Op 10-07-15 om 13:22 schreef Damien Lespiau: > Hi Patrik, > > Please do Cc the patch author and reviewer when finding a regression, > they are superb candidates for the review, especially when they are busy > rewriting the display code. > > On Wed, Jul 08, 2015 at 03:31:52PM +0200, Patrik Jakobsson wrote: >> Watermark calculations depend on the intel_crtc->active flag to be set >> properly. Suspend/resume is broken on SKL and we also get DDB mismatches >> without this patch. >> >> The regression was introduced in: >> >> commit eddfcbcdc27fbecb33bff098967bbdd7ca75bfa6 >> Author: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >> Date: Mon Jun 15 12:33:53 2015 +0200 >> >> drm/i915: Update less state during modeset. >> >> No need to repeatedly call update_watermarks, or update_fbc. >> Down to a single call to update_watermarks in .crtc_enable >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >> Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx> >> Tested-by(IVB): Matt Roper <matthew.d.roper@xxxxxxxxx> >> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> >> >> v2: Don't touch disable_shared_dpll() >> >> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@xxxxxxxxxxxxxxx> > We do need to update the watermarks in disable() as well, to repartition > the DDB and update the watermarks based on the new allocation. > > Maarten, Matt, I've no clue where you want the crtc state update, where > the atomic WM work is at, could you comment? > I'd rather have we had a better way of updating watermarks, but keeping it in the .crtc_disable hook looks good to me right now. Some proper atomic solution will eventually have to be done. :) Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
