Re: [PATCH 02/12] drm/i915/dp: Return early if DSC not supported

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

On 11/27/2024 11:13 AM, Kandpal, Suraj wrote:
>
>> -----Original Message-----
>> From: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
>> Sent: Wednesday, November 20, 2024 4:08 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: intel-xe@lists.freedesktop.org; Kandpal, Suraj <suraj.kandpal@intel.com>;
>> jani.nikula@linux.intel.com; Deak, Imre <imre.deak@intel.com>
>> Subject: [PATCH 02/12] drm/i915/dp: Return early if DSC not supported
>>
>> Check for DSC support before computing link config with DSC.
>> For DP MST we are already doing the same.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> b/drivers/gpu/drm/i915/display/intel_dp.c
>> index db9ddbcdd159..dee15a05e7fd 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -2378,9 +2378,6 @@ int intel_dp_dsc_compute_config(struct intel_dp
>> *intel_dp,
>>   		 intel_dp_supports_fec(intel_dp, connector, pipe_config) &&
>>   		 !intel_dp_is_uhbr(pipe_config));
>>
>> -	if (!intel_dp_supports_dsc(intel_dp, connector, pipe_config))
>> -		return -EINVAL;
>> -
>>   	if (!intel_dp_dsc_supports_format(connector, pipe_config-
>>> output_format))
>>   		return -EINVAL;
>>
>> @@ -2643,6 +2640,9 @@ 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));
>>
>> +		if (!intel_dp_supports_dsc(intel_dp, connector, pipe_config))
>> +			return -EINVAL;
>> +
> Mostly looks good to me but I was thinking what if we made intel_dp_supports_dsc one of the conditions that
> Determines if dsc is needed or not.


Thanks Suraj for looking into the series once again.

I think that mixing intel_dp_supports_dsc with dsc_needed will 
complicate the check.

Currently dsc_is_needed is set: if dsc is forced or if its needed for 
joiner case or if its needed because bandwidth is not sufficient for the 
given mode.

If dsc is not needed, we dont need to check DSC support.

If DSC is indeed required, first logical thing to do should be to check 
if DSC is supported.


Regards,

Ankit

>
> Regards,
> Suraj Kandpal
>
>>   		if (!intel_dp_compute_config_limits(intel_dp, pipe_config,
>>
>> respect_downstream_limits,
>>   						    true,
>> --
>> 2.45.2