[Intel-gfx] [PATCH 2/2] drm/i915: Don't rely that 2 VDSC engines are always enough for pixel rate

[Nautiyal, Ankit K]

On 7/4/2023 6:47 PM, Stanislav Lisovskiy wrote:
> We are currently having FIFO underruns happening for kms_dsc test case,
> problem is that, we check if curreny cdclk is >= pixel rate only if
> there is a single VDSC engine enabled(i.e dsc_split=false) however if
> we happen to have 2 VDSC engines enabled, we just kinda rely that this
> would be automatically enough.
> However pixel rate can be even >= than VDSC clock(cdclk) * 2, so in that
> case even with 2 VDSC engines enabled, we still need to tweak it up.
> So lets compare pixel rate with cdclk * slice count(VDSC engine count) and

Since we are not using slice count, we can just mention VDSC engine count.


> check if it still requires bumping up.
> Previously we had to bump up CDCLK many times for similar reasons.
>
> v2: - Use new intel_dsc_get_num_vdsc_instances to determine number of VDSC
>        engines, instead of slice count(Ankit Nautiyal)
> v3: - s/u8/int/ (Jani Nikula)
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_cdclk.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 4207863b7b2a..bfa1c5d589ba 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -37,6 +37,7 @@
>   #include "intel_pci_config.h"
>   #include "intel_pcode.h"
>   #include "intel_psr.h"
> +#include "intel_vdsc.h"
>   #include "vlv_sideband.h"
>   
>   /**
> @@ -2607,9 +2608,17 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>   	 * When we decide to use only one VDSC engine, since
>   	 * each VDSC operates with 1 ppc throughput, pixel clock
>   	 * cannot be higher than the VDSC clock (cdclk)
> +	 * If there 2 VDSC engines, then pixel clock can't be higher than
> +	 * VDSC clock(cdclk) * 2. However even that can still be not enough.
> +	 * Slice count reflects amount of VDSC engines,
As mentioned above, we can remove slice_count, as we are using VDSC 
engine count.
> +	 * so lets use that to determine, if need still need to tweak CDCLK higher.


>   	 */
> -	if (crtc_state->dsc.compression_enable && !crtc_state->dsc.dsc_split)
> -		min_cdclk = max(min_cdclk, (int)crtc_state->pixel_rate);
> +	if (crtc_state->dsc.compression_enable) {
> +		int num_vdsc_instances = intel_dsc_get_num_vdsc_instances(crtc_state);
> +
> +		min_cdclk = max_t(int, min_cdclk,
> +			          crtc_state->pixel_rate / num_vdsc_instances);

I was wondering if we should use DIV_ROUND_UP(crtc_state->pixel_rate / 
num_vdsc_instances), since min_cdclk should be more than this value.

Though practically Pixel rate in Khz / num of vdsc instances, wont need 
to roundup, so perhaps we might not require this. I leave it up to you.


With the above changes in documentation, this is:

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>


> +	}
>   
>   	/*
>   	 * HACK. Currently for TGL/DG2 platforms we calculate