On Thu, Oct 04, 2018 at 04:15:58PM -0700, Paulo Zanoni wrote:
> The transition watermarks ask for Selected Result Blocks (the real
> value), not Result Blocks (the integer value). Given how ceilings are
> applied in both the non-transition and the transition watermarks
> calculations, we can get away with assuming that Selected Result
> Blocks is actually Result Blocks minus 1 without any rounding errors.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zan...@intel.com>

Matches current bspec.

Reviewed-by: Matt Roper <matthew.d.ro...@intel.com>

I wonder if we should make trans_amount configurable via debugfs so that
we can experiment with the impact of transitional watermarks.


Matt

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 40ce99c455f3..14f13a371989 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4874,7 +4874,7 @@ static void skl_compute_transition_wm(struct 
> intel_crtc_state *cstate,
>       const struct drm_i915_private *dev_priv = to_i915(dev);
>       uint16_t trans_min, trans_y_tile_min;
>       const uint16_t trans_amount = 10; /* This is configurable amount */
> -     uint16_t trans_offset_b, res_blocks;
> +     uint16_t wm0_sel_res_b, trans_offset_b, res_blocks;
>  
>       if (!cstate->base.active)
>               goto exit;
> @@ -4893,13 +4893,25 @@ static void skl_compute_transition_wm(struct 
> intel_crtc_state *cstate,
>  
>       trans_offset_b = trans_min + trans_amount;
>  
> +     /*
> +      * The spec asks for Selected Result Blocks for wm0 (the real value),
> +      * not Result Blocks (the integer value). Pay attention to the capital
> +      * letters. The value wm_l0->plane_res_b is actually Result Blocks, but
> +      * since Result Blocks is the ceiling of Selected Result Blocks plus 1,
> +      * and since we later will have to get the ceiling of the sum in the
> +      * transition watermarks calculation, we can just pretend Selected
> +      * Result Blocks is Result Blocks minus 1 and it should work for the
> +      * current platforms.
> +      */
> +     wm0_sel_res_b = wm_l0->plane_res_b - 1;
> +
>       if (wp->y_tiled) {
>               trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2,
>                                                       wp->y_tile_minimum);
> -             res_blocks = max(wm_l0->plane_res_b, trans_y_tile_min) +
> +             res_blocks = max(wm0_sel_res_b, trans_y_tile_min) +
>                               trans_offset_b;
>       } else {
> -             res_blocks = wm_l0->plane_res_b + trans_offset_b;
> +             res_blocks = wm0_sel_res_b + trans_offset_b;
>  
>               /* WA BUG:1938466 add one block for non y-tile planes */
>               if (IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0))
> -- 
> 2.14.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to