On Thu, Mar 19, 2015 at 09:36:28PM +0200, Ville Syrjälä wrote: > 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> Queued for -next, thanks for the patch. > > 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. Well we have some serious trouble still in these paths with the set_base stuck somewhere in the middle where it shouldn't really hang out when everything is off. So definitely more work to do in any case. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
