Re: [PATCH 3/5] drm/i915: Fix watermarks for VLV/CHV

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Mar 10, 2016 at 10:09:40AM +0100, Maarten Lankhorst wrote:
> Op 09-03-16 om 18:07 schreef ville.syrjala@linux.intel.com:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > commit 92826fcdfc14 ("drm/i915: Calculate watermark related members in the crtc_state, v4.")
> > broke thigns by removing the pre vs. post wm update distinction. We also
> > lost the pre plane wm update entirely for VLV/CHV from the crtc enable
> > path.
> >
> > This caused underruns on modeset and plane enable/disable on CHV,
> > and often those can lead to a dead pipe.
> >
> > So let's bring back the pre vs. post thing, and let's toss in an
> > explicit wm update to valleyview_crtc_enable() to avoid having to
> > put it into the common code.
> >
> > This is more or less a partial revert of the offending commit.
> >
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: drm-intel-fixes@lists.freedesktop.org
> > Fixes: 92826fcdfc14 ("drm/i915: Calculate watermark related members in the crtc_state, v4.")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_atomic.c  |  3 ++-
> >  drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++++++----------
> >  drivers/gpu/drm/i915/intel_drv.h     |  2 +-
> >  3 files changed, 22 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> > index 6a661e796328..79448f1c8b8d 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > @@ -96,7 +96,8 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
> >  	crtc_state->update_pipe = false;
> >  	crtc_state->disable_lp_wm = false;
> >  	crtc_state->disable_cxsr = false;
> > -	crtc_state->wm_changed = false;
> > +	crtc_state->update_wm_pre = false;
> > +	crtc_state->update_wm_post = false;
> >  	crtc_state->fb_changed = false;
> >  	crtc_state->wm.need_postvbl_update = false;
> There's already a wm.need_postvbl_update.
> could we use it for non-atomic platforms, and add a wm.need_intermediate_update ?
> This could probably be used for ILK too.

Dunno. All I know is the regression is now on it's way 4.5 and needs to
be dealt with ASAP. So no major overhaul at this point.

> 
> I believe that if a driver provides the atomic wm functions those functions should also set the wm.*_update members themselves,
> while for the watermarks that aren't converted yet it should be provided by the core.
> 
> ~Maarten

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx