Re: [PATCH] drm/i915/dp: Correct max compressed bpp bounds by using link bpp

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

On 1/16/2025 11:13 PM, Imre Deak wrote:
> On Thu, Jan 16, 2025 at 03:24:16PM +0530, Ankit Nautiyal wrote:
>> While setting the bounds for compressed bpp we ensure that the
>> compressed bpp is less than the pipe bpp.
>>
>> This gives an issue with 420 output format where effective link bpp is
>> half that of the pipe bpp. Therefore instead of pipe bpp use link bpp to
>> set the bounds for the compressed bpp.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 1ad25c37f3c2..47fcfbcb893b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -2068,9 +2068,12 @@ icl_dsc_compute_link_config(struct intel_dp *intel_dp,
>>   			    int timeslots)
>>   {
>>   	int i, ret;
>> +	int link_bpp;
>> +
>> +	link_bpp = intel_dp_output_bpp(pipe_config->output_format, pipe_bpp);
> Here and in the xelpd version, using output_bpp instead of link_bpp or
> just inlining intel_dp_output_bpp() at the single place it's used would
> be better, but in any case the patch looks ok:
>
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Iniling the helper for xelpd one, where we are using fractional bpp is 
becoming a bit complicated.

Using identifier output_bpp instead of link_bpp seems better, since we 
are deriving it using intel_dp_output_bpp().

Will send the patch with the 'output_bpp'.

Thanks & Regards,

Ankit


>
>>   
>>   	/* Compressed BPP should be less than the Input DSC bpp */
>> -	dsc_max_bpp = min(dsc_max_bpp, pipe_bpp - 1);
>> +	dsc_max_bpp = min(dsc_max_bpp, link_bpp - 1);
>>   
>>   	for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp); i++) {
>>   		if (valid_dsc_bpp[i] < dsc_min_bpp)
>> @@ -2113,6 +2116,7 @@ xelpd_dsc_compute_link_config(struct intel_dp *intel_dp,
>>   	u8 bppx16_incr = drm_dp_dsc_sink_bpp_incr(connector->dp.dsc_dpcd);
>>   	u16 compressed_bppx16;
>>   	u8 bppx16_step;
>> +	int link_bpp;
>>   	int ret;
>>   
>>   	if (DISPLAY_VER(display) < 14 || bppx16_incr <= 1)
>> @@ -2120,8 +2124,10 @@ xelpd_dsc_compute_link_config(struct intel_dp *intel_dp,
>>   	else
>>   		bppx16_step = 16 / bppx16_incr;
>>   
>> +	link_bpp = intel_dp_output_bpp(pipe_config->output_format, pipe_bpp);
>> +
>>   	/* Compressed BPP should be less than the Input DSC bpp */
>> -	dsc_max_bpp = min(dsc_max_bpp << 4, (pipe_bpp << 4) - bppx16_step);
>> +	dsc_max_bpp = min(dsc_max_bpp << 4, (link_bpp << 4) - bppx16_step);
>>   	dsc_min_bpp = dsc_min_bpp << 4;
>>   
>>   	for (compressed_bppx16 = dsc_max_bpp;
>> -- 
>> 2.45.2
>>