diff mbox series

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

Message ID 20220125085801.1025521-3-ankit.k.nautiyal@intel.com (mailing list archive)
State New, archived
Headers show
Series Minor Fixes and Refactoring for HDMI PCON stuff | expand

Commit Message

Ankit Nautiyal Jan. 25, 2022, 8:58 a.m. UTC
Move the common function for getting the max FRL rate for an HDMI sink,
from intel_dp.c to drm/drm_edid.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@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(-)

Comments

Jani Nikula Jan. 25, 2022, 9:52 a.m. UTC | #1
On Tue, 25 Jan 2022, Ankit Nautiyal <ankit.k.nautiyal@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.

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.

>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@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.

> + *
> + * 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.

> + *
> + * 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__ */
Ankit Nautiyal Jan. 25, 2022, 12:30 p.m. UTC | #2
On 1/25/2022 3:22 PM, Jani Nikula wrote:
> On Tue, 25 Jan 2022, Ankit Nautiyal <ankit.k.nautiyal@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@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__ */
diff mbox series

Patch

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
+ *
+ * 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
+ *
+ * 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__ */