Re: [Intel-gfx] [PATCH] drm/i915: Fix timeslots argument for DP DSC SST case

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Jan 02, 2023 at 03:23:06PM +0200, Stanislav Lisovskiy wrote:
> We now accept timeslots param exactly how the variable
> sounds: amount of timeslots, but not ratio timeslots/64.
> So for SST case(when we have all timeslots for use), it
> should be 64, but not 1.

I noticed that at intel_dp_dsc_get_output_bpp() we have this comment:

         * for SST -> TimeSlotsPerMTP is 1,

and there's a bunch of math used with this timeslots, but none of them
is a direct division by 64.

So I wonder if a refactor to reflect the "perMTP" is not needed there.
Or the reverse math instead of passing the 64 directly.

> This caused some issues in the tests.

could you also expand on what tests?
any "References:" link to cibuglog or so?

Oh, any "Fixes:" tag as well?

Thanks for the patch,
Rodrigo.

> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 13baf3cb5f934..362fb394d613c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1696,7 +1696,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  			    str_yes_no(ret), str_yes_no(joiner_needs_dsc),
>  			    str_yes_no(intel_dp->force_dsc_en));
>  		ret = intel_dp_dsc_compute_config(intel_dp, pipe_config,
> -						  conn_state, &limits, 1, true);
> +						  conn_state, &limits, 64, true);
>  		if (ret < 0)
>  			return ret;
>  	}
> -- 
> 2.37.3
>