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

Maarten Lankhorst
2016-03-10T09:09:40Z

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.

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