| Message ID | 20170719125502.25696-10-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Daniel Vetter (2017-07-19 13:55:02) > The core already does this in setup_commit(). With this we can also > remove the unpin_work_count since it's the last user. Also note that the loop only existed for the legacy pageflip support, with that removed it becomes entirely redundant. -Chris
On Wed, Jul 19, 2017 at 02:09:02PM +0100, Chris Wilson wrote: > Quoting Daniel Vetter (2017-07-19 13:55:02) > > The core already does this in setup_commit(). With this we can also > > remove the unpin_work_count since it's the last user. > > Also note that the loop only existed for the legacy pageflip support, > with that removed it becomes entirely redundant. Yeah, I just wanted to make clear that removing this isn't a bit of code that we failed to move over to atomic. It's been dead since a while. -Daniel
Op 19-07-17 om 14:55 schreef Daniel Vetter: > The core already does this in setup_commit(). With this we can also > remove the unpin_work_count since it's the last user. > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/intel_display.c | 13 +------------ > drivers/gpu/drm/i915/intel_drv.h | 2 -- > 2 files changed, 1 insertion(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index e52a2adbaaa5..351208b7b1ad 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11825,18 +11825,7 @@ static int intel_atomic_check(struct drm_device *dev, > static int intel_atomic_prepare_commit(struct drm_device *dev, > struct drm_atomic_state *state) > { > - struct drm_i915_private *dev_priv = to_i915(dev); > - struct drm_crtc_state *crtc_state; > - struct drm_crtc *crtc; > - int i, ret; > - > - for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > - if (state->legacy_cursor_update) > - continue; > - > - if (atomic_read(&to_intel_crtc(crtc)->unpin_work_count) >= 2) > - flush_workqueue(dev_priv->wq); > - } > + int ret; > > ret = mutex_lock_interruptible(&dev->struct_mutex); > if (ret) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 9cb7e781e863..96402c06e295 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -798,8 +798,6 @@ struct intel_crtc { > unsigned long long enabled_power_domains; > struct intel_overlay *overlay; > > - atomic_t unpin_work_count; > - > /* Display surface base address adjustement for pageflips. Note that on > * gen4+ this only adjusts up to a tile, offsets within a tile are > * handled in the hw itself (with the TILEOFF register). */ I like red diffs.. For patch 1, 4 (with updated commit message), 6-9: Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
On Wed, Jul 19, 2017 at 04:01:03PM +0200, Maarten Lankhorst wrote: > Op 19-07-17 om 14:55 schreef Daniel Vetter: > > The core already does this in setup_commit(). With this we can also > > remove the unpin_work_count since it's the last user. > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > --- > > drivers/gpu/drm/i915/intel_display.c | 13 +------------ > > drivers/gpu/drm/i915/intel_drv.h | 2 -- > > 2 files changed, 1 insertion(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index e52a2adbaaa5..351208b7b1ad 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -11825,18 +11825,7 @@ static int intel_atomic_check(struct drm_device *dev, > > static int intel_atomic_prepare_commit(struct drm_device *dev, > > struct drm_atomic_state *state) > > { > > - struct drm_i915_private *dev_priv = to_i915(dev); > > - struct drm_crtc_state *crtc_state; > > - struct drm_crtc *crtc; > > - int i, ret; > > - > > - for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > > - if (state->legacy_cursor_update) > > - continue; > > - > > - if (atomic_read(&to_intel_crtc(crtc)->unpin_work_count) >= 2) > > - flush_workqueue(dev_priv->wq); > > - } > > + int ret; > > > > ret = mutex_lock_interruptible(&dev->struct_mutex); > > if (ret) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 9cb7e781e863..96402c06e295 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -798,8 +798,6 @@ struct intel_crtc { > > unsigned long long enabled_power_domains; > > struct intel_overlay *overlay; > > > > - atomic_t unpin_work_count; > > - > > /* Display surface base address adjustement for pageflips. Note that on > > * gen4+ this only adjusts up to a tile, offsets within a tile are > > * handled in the hw itself (with the TILEOFF register). */ > > I like red diffs.. > > For patch 1, 4 (with updated commit message), 6-9: > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Ok I merged patches 1&2, that take care of this for a lot of users/systems, and the remaining stuff needs a notch more polish, and a lot more testing anyway. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e52a2adbaaa5..351208b7b1ad 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11825,18 +11825,7 @@ static int intel_atomic_check(struct drm_device *dev, static int intel_atomic_prepare_commit(struct drm_device *dev, struct drm_atomic_state *state) { - struct drm_i915_private *dev_priv = to_i915(dev); - struct drm_crtc_state *crtc_state; - struct drm_crtc *crtc; - int i, ret; - - for_each_new_crtc_in_state(state, crtc, crtc_state, i) { - if (state->legacy_cursor_update) - continue; - - if (atomic_read(&to_intel_crtc(crtc)->unpin_work_count) >= 2) - flush_workqueue(dev_priv->wq); - } + int ret; ret = mutex_lock_interruptible(&dev->struct_mutex); if (ret) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 9cb7e781e863..96402c06e295 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -798,8 +798,6 @@ struct intel_crtc { unsigned long long enabled_power_domains; struct intel_overlay *overlay; - atomic_t unpin_work_count; - /* Display surface base address adjustement for pageflips. Note that on * gen4+ this only adjusts up to a tile, offsets within a tile are * handled in the hw itself (with the TILEOFF register). */
The core already does this in setup_commit(). With this we can also remove the unpin_work_count since it's the last user. Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/intel_display.c | 13 +------------ drivers/gpu/drm/i915/intel_drv.h | 2 -- 2 files changed, 1 insertion(+), 14 deletions(-)