On Thu, Mar 19, 2015 at 04:16:41PM -0300, Paulo Zanoni wrote: > 2015-03-18 19:04 GMT-03:00 Matt Roper <matthew.d.roper@xxxxxxxxx>: > > Determining whether we'll need to wait for vblanks is something we > > should determine during the atomic 'check' phase, not the 'commit' > > phase. Note that we only set these bits in the branch of 'check' where > > intel_crtc->active is true so that we don't try to wait on a disabled > > CRTC. > > > > The whole 'wait for vblank after update' flag should go away in the > > future, once we start handling watermarks in a proper atomic manner. > > Add this to the commit message: > > Regression introduced by: > > commit 2fdd7def16dd7580f297827930126c16b152ec11 > Author: Matt Roper <matthew.d.roper@xxxxxxxxx> > Date: Wed Mar 4 10:49:04 2015 -0800 > drm/i915: Don't clobber plane state on internal disables > > (at least according to QA's bisect) > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > Root-cause-analysis-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89550 > > Testcase: igt/pm_rpm/legacy-planes > > Testcase: igt/pm_rpm/legacy-planes-dpms > > Testcase: igt/pm_rpm/universal-planes > > Testcase: igt/pm_rpm/universal-planes-dpms > > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > > --- > > Paulo, can you test this patch and see if it solves the problem? The only > > platform I have access to (IVB) doesn't have runtime power management, so I > > can't actually run the relevant testcases myself. > > Yes, this patch makes the WARN go away on my BDW machine :) > Tested-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > One of the possible problems with this patch is that, before it, the > wait_vblank was only set for ILK-BDW, but now we set it for all > platforms. Do we really want this? Otherwise, it looks good, although > I haven't been closely following the atomic rework to be able to > assert this is really according to the grand plans. Daniel/Ville could > comment here :) Somethins seems a bit off to me if we end up calling .disable_plane() with a disabled pipe. So fixing things so that won't happen might be a better approach. > > > > > drivers/gpu/drm/i915/intel_sprite.c | 24 ++++++++++-------------- > > 1 file changed, 10 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index a828736..fad1d9f 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -747,13 +747,6 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) > > I915_WRITE(SPRSURF(pipe), 0); > > > > intel_flush_primary_plane(dev_priv, intel_crtc->plane); > > - > > - /* > > - * Avoid underruns when disabling the sprite. > > - * FIXME remove once watermark updates are done properly. > > - */ > > - intel_crtc->atomic.wait_vblank = true; > > - intel_crtc->atomic.update_sprite_watermarks |= (1 << drm_plane_index(plane)); > > } > > > > static int > > @@ -937,13 +930,6 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) > > I915_WRITE(DVSSURF(pipe), 0); > > > > intel_flush_primary_plane(dev_priv, intel_crtc->plane); > > - > > - /* > > - * Avoid underruns when disabling the sprite. > > - * FIXME remove once watermark updates are done properly. > > - */ > > - intel_crtc->atomic.wait_vblank = true; > > - intel_crtc->atomic.update_sprite_watermarks |= (1 << drm_plane_index(plane)); > > } > > > > /** > > @@ -1262,6 +1248,16 @@ finish: > > plane->state->fb->modifier[0] != > > state->base.fb->modifier[0]) > > intel_crtc->atomic.update_wm = true; > > + > > + if (!state->visible) { > > + /* > > + * Avoid underruns when disabling the sprite. > > + * FIXME remove once watermark updates are done properly. > > + */ > > + intel_crtc->atomic.wait_vblank = true; > > + intel_crtc->atomic.update_sprite_watermarks |= > > + (1 << drm_plane_index(plane)); > > + } > > } > > > > return 0; > > -- > > 1.8.5.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
