[Intel-gfx] [PATCH] drm/i915: Don't clear all watermarks when updating.

[Damien Lespiau]

On Wed, Jul 08, 2015 at 09:05:53AM -0700, Bob Paauwe wrote:
> Clearing the watermarks for all pipes/planes when updating the
> watermarks for a single CRTC change seems like the wrong thing to
> do here. As is, this code will update any pipe/plane watermarks
> that need updating and leave the remaining set to zero.  Later, the
> watermark checks in check_wm_state() will flag these zero'd out pipe/plane
> watermarks and throw errors.
> 
> By not clearing all pipe/plane watermark values, only those that
> require changes are changed and the remaining stay unchanged.
> 
> Signed-off-by: Bob Paauwe <bob.j.paauwe at intel.com>

Hi Bob,

The spirit of this patch looks good to me (thanks for looking into
this!), but we I think we still need to clear the data for the pipe
we're are updating as it will contain stale values from the previous
skl_update_wm().

For instance we don't reset ->dirty[pipe] to false after the register
write and were relying on that memset. 2 options I can see:

  - selectively zero all the fields for that pipe (not helped by the
    fact we have arrays indexed by the pipe (array of structure Vs
    structure of arrays))
  - make sure we don't rely on any field from the previous call (eg.
    update ->dirty in skl_write_wm_values())

I have a slight preference for the first solution as it seems a bit more
fool proof, but don't mind either way.

Thanks,

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c2f8956..25fc319 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3734,8 +3734,6 @@ static void skl_update_wm(struct drm_crtc *crtc)
>  	struct skl_pipe_wm pipe_wm = {};
>  	struct intel_wm_config config = {};
>  
> -	memset(results, 0, sizeof(*results));
> -
>  	skl_compute_wm_global_parameters(dev, &config);
>  
>  	if (!skl_update_pipe_wm(crtc, &params, &config,
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx