Re: [PATCH 1/2] drm/i915/display: Add macro for checking 3 DSC engines

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

On 4/15/2025 12:47 PM, Jani Nikula wrote:
> On Mon, 14 Apr 2025, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
>> 3 DSC engines per pipe is currently supported only for BMG.
>> Add a macro to check whether a platform supports 3 DSC engines per pipe.
> Nitpick, feels like a macro returning the number of DSC engines per pipe
> would be more generic. Like, would you also add HAS_DSC_2ENGINES() and
> HAS_DSC_4ENGINES() if you needed to know that? But I guess we can go
> with what you have for the immediate fix.

Yeah NUM_DSC_ENGINES seem better. I was under an impression that newer 
platforms would have 3 DSC engines, which was incorrect, so specific 
case for 3 engines made sense at that time.

If there is any further change, I guess returning number of DSC engines 
as you have suggested is the way to go.

>
> However, adding the tiniest macro and its only user in separate patches,
> for something that needs to be backported to stable, seems like erring
> on the side of splitting up patches too much.

Noted. I agree it could have been just one fix with the macro and the 
usage in same patch.

I have already pushed to drm-intel-next :(. But will keep that in mind.

Thanks Jani for the guidance, thanks Suraj for reviews.


Regards,

Ankit

>
> BR,
> Jani.
>
>
>> v2:Fix Typo in macro argument. (Suraj).
>> Added fixes tag.
>>
>> Bspec: 50175
>> Fixes: be7f5fcdf4a0 ("drm/i915/dp: Enable 3 DSC engines for 12 slices")
>> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Cc: Suraj Kandpal <suraj.kandpal@intel.com>
>> Cc: <stable@vger.kernel.org> # v6.14+
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display_device.h | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
>> index 368b0d3417c2..87c666792c0d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
>> @@ -163,6 +163,7 @@ struct intel_display_platforms {
>>   #define HAS_DP_MST(__display)		(DISPLAY_INFO(__display)->has_dp_mst)
>>   #define HAS_DSB(__display)		(DISPLAY_INFO(__display)->has_dsb)
>>   #define HAS_DSC(__display)		(DISPLAY_RUNTIME_INFO(__display)->has_dsc)
>> +#define HAS_DSC_3ENGINES(__display)	(DISPLAY_VERx100(__display) == 1401 && HAS_DSC(__display))
>>   #define HAS_DSC_MST(__display)		(DISPLAY_VER(__display) >= 12 && HAS_DSC(__display))
>>   #define HAS_FBC(__display)		(DISPLAY_RUNTIME_INFO(__display)->fbc_mask != 0)
>>   #define HAS_FBC_DIRTY_RECT(__display)	(DISPLAY_VER(__display) >= 30)