[Intel-gfx] [PATCH 2/3] drm/drm_edid: Add helper to get max FRL rate for an HDMI sink

[Nautiyal, Ankit K]

On 1/25/2022 3:22 PM, Jani Nikula wrote:
> On Tue, 25 Jan 2022, Ankit Nautiyal <ankit.k.nautiyal at intel.com> wrote:
>> Move the common function for getting the max FRL rate for an HDMI sink,
>> from intel_dp.c to drm/drm_edid.
> The subject prefix should be "drm/edid:"
>
> But I'm not sure these functions belong in drm_edid.c though. If you see
> a function prefixed drm_hdmi_, this is not where you'd expect to find
> it. Not sure what the right place should be though.

Hmm. Another candidate which came to my mind was drm_connector,

but since the parsing of the edid got these values, I was thinking this 
might be the correct place.

>
> Please split this to two patches, adding the helpers in drm and using
> them in i915. It's generally easier to manage that way if there's no
> other reason to keep the changes together.
Alright, that make sense. Will split into two patches.
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>> ---
>>   drivers/gpu/drm/drm_edid.c              | 38 +++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/display/intel_dp.c | 19 ++++---------
>>   include/drm/drm_edid.h                  |  2 ++
>>   3 files changed, 45 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index eb61a1a92dc0..75b538b4c87f 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -6176,3 +6176,41 @@ void drm_update_tile_info(struct drm_connector *connector,
>>   		connector->tile_group = NULL;
>>   	}
>>   }
>> +
>> +/**
>> + * drm_hdmi_sink_max_frl - get the max frl rate from HDMI2.1 sink
>> + * @connector - connector with HDMI2.1 sink
> Do you need to first make sure it's a HDMI 2.1 sink? That's what the
> documentation makes you believe.

The thing is that, these fields are the one that helps us to understand 
if the sink is HDMI2.1.

Perhaps I should fix the documentation here:

drm_hdmi_sink_max_frl - get the max frl rate, if supported.
@connector - connector with HDMI sink

>> + *
>> + * RETURNS:
>> + * max frl rate supported by the HDMI2.1 sink, 0 if FRL not supported
>> + */
>> +int drm_hdmi_sink_max_frl(struct drm_connector *connector)
>> +{
>> +	int max_lanes = connector->display_info.hdmi.max_lanes;
>> +	int rate_per_lane = connector->display_info.hdmi.max_frl_rate_per_lane;
>> +
>> +	return max_lanes * rate_per_lane;
>> +}
>> +EXPORT_SYMBOL(drm_hdmi_sink_max_frl);
>> +
>> +/**
>> + * drm_hdmi_sink_dsc_max_frl - get the max frl rate from HDMI2.1 sink
>> + * with DSC1.2 compression.
>> + * @connector - connector with HDMI2.1 sink
> Ditto.

Similar to the above lines, here also the documentation can be fixed.

Thanks & Regards,

Ankit


>
>> + *
>> + * RETURNS:
>> + * max frl rate supported by the HDMI2.1 sink with DSC1.2, 0 if FRL not supported
>> + */
>> +int drm_hdmi_sink_dsc_max_frl(struct drm_connector *connector)
>> +{
>> +	int max_dsc_lanes, dsc_rate_per_lane;
>> +
>> +	if (!connector->display_info.hdmi.dsc_cap.v_1p2)
>> +		return 0;
>> +
>> +	max_dsc_lanes = connector->display_info.hdmi.dsc_cap.max_lanes;
>> +	dsc_rate_per_lane = connector->display_info.hdmi.dsc_cap.max_frl_rate_per_lane;
>> +
>> +	return max_dsc_lanes * dsc_rate_per_lane;
>> +}
>> +EXPORT_SYMBOL(drm_hdmi_sink_dsc_max_frl);
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 4d4579a301f6..f7fe7de7e553 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -2190,22 +2190,13 @@ static int intel_dp_hdmi_sink_max_frl(struct intel_dp *intel_dp)
>>   {
>>   	struct intel_connector *intel_connector = intel_dp->attached_connector;
>>   	struct drm_connector *connector = &intel_connector->base;
>> -	int max_frl_rate;
>> -	int max_lanes, rate_per_lane;
>> -	int max_dsc_lanes, dsc_rate_per_lane;
>> +	int max_frl = drm_hdmi_sink_max_frl(connector);
>> +	int dsc_max_frl = drm_hdmi_sink_dsc_max_frl(connector);
>>   
>> -	max_lanes = connector->display_info.hdmi.max_lanes;
>> -	rate_per_lane = connector->display_info.hdmi.max_frl_rate_per_lane;
>> -	max_frl_rate = max_lanes * rate_per_lane;
>> +	if (dsc_max_frl)
>> +		return min(max_frl, dsc_max_frl);
>>   
>> -	if (connector->display_info.hdmi.dsc_cap.v_1p2) {
>> -		max_dsc_lanes = connector->display_info.hdmi.dsc_cap.max_lanes;
>> -		dsc_rate_per_lane = connector->display_info.hdmi.dsc_cap.max_frl_rate_per_lane;
>> -		if (max_dsc_lanes && dsc_rate_per_lane)
>> -			max_frl_rate = min(max_frl_rate, max_dsc_lanes * dsc_rate_per_lane);
>> -	}
>> -
>> -	return max_frl_rate;
>> +	return max_frl;
>>   }
>>   
>>   static bool
>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
>> index 18f6c700f6d0..5003e1254c44 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -592,6 +592,8 @@ drm_display_mode_from_cea_vic(struct drm_device *dev,
>>   			      u8 video_code);
>>   const u8 *drm_find_edid_extension(const struct edid *edid,
>>   				  int ext_id, int *ext_index);
>> +int drm_hdmi_sink_max_frl(struct drm_connector *connector);
>> +int drm_hdmi_sink_dsc_max_frl(struct drm_connector *connector);
>>   
>>   
>>   #endif /* __DRM_EDID_H__ */