| Message ID | 1471462904-842-1-git-send-email-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 17, 2016 at 04:41:44PM -0300, Paulo Zanoni wrote: > From: Chris Wilson <chris@chris-wilson.co.uk> > > intel_fbc_pre_update() depends upon the new state being already pinned > in place in the Global GTT (primarily for both fencing which wants both > an offset and a fence register, if assigned). This requires the call to > intel_fbc_pre_update() be after intel_pin_and_fence_fb() - but commit > e8216e502aca ("drm/i915/fbc: call intel_fbc_pre_update earlier during > page flips") moved the code way too much up in its attempt to call it > before the page flip. > > v2 (from Paulo): > - Point the original bad commit. > - Add a comment to maybe prevent further regressions. > > Fixes: e8216e502aca ("drm/i915/fbc: call intel_fbc_pre_update earlier...") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > Cc: drm-intel-fixes@lists.freedesktop.org If you had just claimed this as your own, I could have reviewed it ;) -Chris
Em Qua, 2016-08-17 às 20:49 +0100, Chris Wilson escreveu: > On Wed, Aug 17, 2016 at 04:41:44PM -0300, Paulo Zanoni wrote: > > > > From: Chris Wilson <chris@chris-wilson.co.uk> > > > > intel_fbc_pre_update() depends upon the new state being already > > pinned > > in place in the Global GTT (primarily for both fencing which wants > > both > > an offset and a fence register, if assigned). This requires the > > call to > > intel_fbc_pre_update() be after intel_pin_and_fence_fb() - but > > commit > > e8216e502aca ("drm/i915/fbc: call intel_fbc_pre_update earlier > > during > > page flips") moved the code way too much up in its attempt to call > > it > > before the page flip. > > > > v2 (from Paulo): > > - Point the original bad commit. > > - Add a comment to maybe prevent further regressions. > > > > Fixes: e8216e502aca ("drm/i915/fbc: call intel_fbc_pre_update > > earlier...") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > > Cc: drm-intel-fixes@lists.freedesktop.org > > If you had just claimed this as your own, I could have reviewed it ;) I don't want to steal your patch, you submitted it first, so you should get the credit. I thought about adding my R-B, but then I remembered reading somewhere that having my Signed-off-by was enough and implicitly meant a R-B. Anyway, feel free to add: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v1) I suppose it would also make sense for you to add R-B: Chris (v2) since I modified the original. Also, feel free to merge this. > -Chris >
On Wed, Aug 17, 2016 at 08:00:09PM +0000, Zanoni, Paulo R wrote: > Em Qua, 2016-08-17 às 20:49 +0100, Chris Wilson escreveu: > > On Wed, Aug 17, 2016 at 04:41:44PM -0300, Paulo Zanoni wrote: > > > > > > From: Chris Wilson <chris@chris-wilson.co.uk> > > > > > > intel_fbc_pre_update() depends upon the new state being already > > > pinned > > > in place in the Global GTT (primarily for both fencing which wants > > > both > > > an offset and a fence register, if assigned). This requires the > > > call to > > > intel_fbc_pre_update() be after intel_pin_and_fence_fb() - but > > > commit > > > e8216e502aca ("drm/i915/fbc: call intel_fbc_pre_update earlier > > > during > > > page flips") moved the code way too much up in its attempt to call > > > it > > > before the page flip. > > > > > > v2 (from Paulo): > > > - Point the original bad commit. > > > - Add a comment to maybe prevent further regressions. > > > > > > Fixes: e8216e502aca ("drm/i915/fbc: call intel_fbc_pre_update > > > earlier...") > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > > > Cc: drm-intel-fixes@lists.freedesktop.org > > > > If you had just claimed this as your own, I could have reviewed it ;) > > I don't want to steal your patch, you submitted it first, so you should > get the credit. Pushed in your name since you did most of the work! -Chris
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8a203b5..7057bd9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12109,9 +12109,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, crtc->primary->fb = fb; update_state_fb(crtc->primary); - intel_fbc_pre_update(intel_crtc, intel_crtc->config, - to_intel_plane_state(primary->state)); - work->pending_flip_obj = i915_gem_object_get(obj); ret = i915_mutex_lock_interruptible(dev); @@ -12157,6 +12154,17 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, work->gtt_offset += intel_crtc->dspaddr_offset; work->rotation = crtc->primary->state->rotation; + /* + * There's the potential that the next frame will not be compatible with + * FBC, so we want to call pre_update() before the actual page flip. + * The problem is that pre_update() caches some information about the fb + * object, so we want to do this only after the object is pinned. Let's + * be on the safe side and do this immediately before scheduling the + * flip. + */ + intel_fbc_pre_update(intel_crtc, intel_crtc->config, + to_intel_plane_state(primary->state)); + if (mmio_flip) { INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);