On Fri, 2020-02-28 at 22:35 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
>
> Currently we're comparing the watermarks between the old and new
> states
> before we've fully computed the new watermarks. In particular
> skl_build_pipe_wm() will not account for the amount of ddb space
> we'll
> have. That information is only available during skl_compute_ddb()
> which will proceed to zero out any watermark level exceeding the
> ddb allocation. If we're short on ddb space this will end up
> adding the plane to the state due erronously determining that the
> watermarks have changed. Fix the problem by deferring
> skl_wm_add_affected_planes() until we have the final watermarks
> computed.
>
> Noticed this when trying enable transition watermarks on glk.
> We now computed the trans_wm as 28, but we only had 14 blocks
> of ddb, and thus skl_compute_ddb() ended up disabling the cursor
> trans_wm every time. Thus we ended up adding the cursor to every
> commit that didn't actually affect the cursor at all.
>
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 39299811b650..a3d76e69caae 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5762,16 +5762,24 @@ skl_compute_wm(struct intel_atomic_state
> *state)
> ret = skl_build_pipe_wm(new_crtc_state);
> if (ret)
> return ret;
> -
> - ret = skl_wm_add_affected_planes(state, crtc);
> - if (ret)
> - return ret;
> }
>
> ret = skl_compute_ddb(state);
> if (ret)
> return ret;
>
> + /*
> + * skl_compute_ddb() will have adjusted the final watermarks
> + * based on how much ddb is available. Now we can actually
> + * check if the final watermarks changed.
> + */
> + for_each_oldnew_intel_crtc_in_state(state, crtc,
> old_crtc_state,
> + new_crtc_state, i) {
> + ret = skl_wm_add_affected_planes(state, crtc);
> + if (ret)
> + return ret;
> + }
skl_compute_ddb() is already calling skl_wm_add_affected_planes() after
do the ddb allocation for each pipe, so we could remove this chunk,
with that:
Reviewed-by: José Roberto de Souza <jose.so...@intel.com>
> +
> skl_print_wm_changes(state);
>
> return 0;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
