Re: [Intel-gfx] [PATCH] drm/i915: Clamp linetime wm to <64usec

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Thu, Jun 25, 2020 at 11:00:03PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The linetime watermark is a 9 bit value, which gives us
> a maximum linetime of just below 64 usec. If the linetime
> exceeds that value we currently just discard the high bits
> and program the rest into the register, which angers the
> state checker.
> 
> To avoid that let's just clamp the value to the max. I believe
> it should be perfectly fine to program a smaller linetime wm
> than strictly required, just means the hardware may fetch data
> sooner than strictly needed. We are further reassured by the
> fact that with DRRS the spec tells us to program the smaller
> of the two linetimes corresponding to the two refresh rates.
> 
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index a11bb675f9b3..d486d675166f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -12581,12 +12581,15 @@ static u16 hsw_linetime_wm(const struct intel_crtc_state *crtc_state)
>  {
>  	const struct drm_display_mode *adjusted_mode =
>  		&crtc_state->hw.adjusted_mode;
> +	int linetime_wm;
>  
>  	if (!crtc_state->hw.enable)
>  		return 0;
>  
> -	return DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
> -				 adjusted_mode->crtc_clock);
> +	linetime_wm = DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
> +					adjusted_mode->crtc_clock);
> +
> +	return min(linetime_wm, 0x1ff);

Are we actually doing the right thing here? I just mean that we get value
543 in the bug because pixel rate is 14874 which doesn't seem correct.

Stan
>  }
>  
>  static u16 hsw_ips_linetime_wm(const struct intel_crtc_state *crtc_state,
> @@ -12594,12 +12597,15 @@ static u16 hsw_ips_linetime_wm(const struct intel_crtc_state *crtc_state,
>  {
>  	const struct drm_display_mode *adjusted_mode =
>  		&crtc_state->hw.adjusted_mode;
> +	int linetime_wm;
>  
>  	if (!crtc_state->hw.enable)
>  		return 0;
>  
> -	return DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
> -				 cdclk_state->logical.cdclk);
> +	linetime_wm = DIV_ROUND_CLOSEST(adjusted_mode->crtc_htotal * 1000 * 8,
> +					cdclk_state->logical.cdclk);
> +
> +	return min(linetime_wm, 0x1ff);
>  }
>  
>  static u16 skl_linetime_wm(const struct intel_crtc_state *crtc_state)
> @@ -12608,7 +12614,7 @@ static u16 skl_linetime_wm(const struct intel_crtc_state *crtc_state)
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	const struct drm_display_mode *adjusted_mode =
>  		&crtc_state->hw.adjusted_mode;
> -	u16 linetime_wm;
> +	int linetime_wm;
>  
>  	if (!crtc_state->hw.enable)
>  		return 0;
> @@ -12620,7 +12626,7 @@ static u16 skl_linetime_wm(const struct intel_crtc_state *crtc_state)
>  	if (IS_GEN9_LP(dev_priv) && dev_priv->ipc_enabled)
>  		linetime_wm /= 2;
>  
> -	return linetime_wm;
> +	return min(linetime_wm, 0x1ff);
>  }
>  
>  static int hsw_compute_linetime_wm(struct intel_atomic_state *state,
> -- 
> 2.26.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx