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 12:53:37PM +0100, Maarten Lankhorst wrote:
> Op 10-03-16 om 11:33 schreef Ville Syrjälä:
> > 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.
> >
> Ok, since I lack the hardware to test for now this will have to do.. I'd like this to be revisited in the future.
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Thanks for the reviews everyone. Series pushed to dinq.

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