Re: [PATCH v3 4/5] drm/i915/scaler: Check if vblank is sufficient for scaler

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 1/6/2025 10:21 PM, Mitul Golani wrote:
> Check if vblank is too short for scaler prefill latency.
>
> --v2:
> - Use hweight* family of functions for counting bits. [Jani]
> - Update precision handling for hscale and vscale. [Ankit]
> - Consider chroma downscaling factor during latency
> calculation. [Ankit]
> - Replace function name from scaler_prefill_time to
> scaler_prefill_latency.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>   drivers/gpu/drm/i915/display/skl_watermark.c | 31 ++++++++++++++++++++
>   1 file changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 29e8ea91c858..95f60819fe0a 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -2292,6 +2292,36 @@ static int icl_build_plane_wm(struct intel_crtc_state *crtc_state,
>   	return 0;
>   }
>   
> +static int
> +scaler_prefill_latency(const struct intel_crtc_state *crtc_state)
> +{
> +	const struct intel_crtc_scaler_state *scaler_state =
> +					&crtc_state->scaler_state;
> +	int count = hweight32(scaler_state->scaler_users);
> +	int latency;
> +	long long  hscale_k =
> +		mul_u32_u32(scaler_state->scalers[0].hscale, 1000);
> +	long long vscale_k =
> +		mul_u32_u32(scaler_state->scalers[0].vscale, 1000);


I understand you want to retain the precision so multiplying by 1000 
makes sense.

But these values are left shifted by 16 so to do any calculation need to 
right shift by after you are multiplying by 1000.

> +
> +	if (!count)
> +		return 0;
> +
> +	if (count > 1) {
> +		int chroma_downscaling_factor =
> +			crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 ? 2 : 1;

This should be YCBCR420.


> +
> +		latency = (4 * crtc_state->linetime) +
> +			  (chroma_downscaling_factor *
> +			   DIV_ROUND_UP_ULL((4 * crtc_state->linetime * hscale_k * vscale_k),
> +					    USHRT_MAX));

Need to divide by 1000 * 1000 here, since you multiplied these earlier.


Regards,

Ankit


> +	} else {
> +		latency = 4 * crtc_state->linetime;
> +	}
> +
> +	return latency;
> +}
> +
>   static bool
>   skl_is_vblank_too_short(const struct intel_crtc_state *crtc_state,
>   			int wm0_lines, int latency)
> @@ -2302,6 +2332,7 @@ skl_is_vblank_too_short(const struct intel_crtc_state *crtc_state,
>   	/* FIXME missing scaler and DSC pre-fill time */
>   	return crtc_state->framestart_delay +
>   		intel_usecs_to_scanlines(adjusted_mode, latency) +
> +		scaler_prefill_latency(crtc_state) +
>   		wm0_lines >
>   		adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vblank_start;
>   }