Re: [PATCH v2] drm/i915: Don't forget to mark crtc as inactive after disable

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Jul 13, 2015 at 11:10:51AM +0200, Maarten Lankhorst wrote:
> 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.

Yeah you need to list everyone you want to Cc: explicitly.

> > 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@linux.intel.com>
> >> 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@linux.intel.com>
> >>     Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> >>     Tested-by(IVB): Matt Roper <matthew.d.roper@intel.com>
> >>     Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>
> >> v2: Don't touch disable_shared_dpll()
> >>
> >> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> > 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@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx