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@xxxxxxxxx>
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, ¶ms, &config,
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
