[PATCH 0/3] Minor Fixes and Refactoring for HDMI PCON stuff

[PATCH 0/3] Minor Fixes and Refactoring for HDMI PCON stuff

[Ankit Nautiyal]

Misc fixes and refactoring in HDMI2.1 PCON helper functions.

Ankit Nautiyal (3):
  drm/i915_hdmi: Fix the definition of intel_hdmi_dsc_get_bpp
  drm/drm_edid: Add helper to get max FRL rate for an HDMI sink
  drm/i915/display: Simplify helpers for getting DSC slices and bpp

 drivers/gpu/drm/drm_edid.c                | 38 +++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.c   | 26 ++++++----------
 drivers/gpu/drm/i915/display/intel_hdmi.c | 26 +++++++++-------
 drivers/gpu/drm/i915/display/intel_hdmi.h |  8 +++--
 include/drm/drm_edid.h                    |  2 ++
 5 files changed, 69 insertions(+), 31 deletions(-)

-- 
2.25.1

[PATCH 1/3] drm/i915_hdmi: Fix the definition of intel_hdmi_dsc_get_bpp

[Ankit Nautiyal]

Fix the data-type of the argument output_format to enum, for the
function intel_hdmi_dsc_get_bpp.

Fixes: 6e6cb758e035 ("drm/i915: Add helper functions for calculating DSC
parameters for HDMI2.1")

Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: "Ville Syrj_l_" <ville.syrjala@linux.intel.com>
Cc: "Jos_ Roberto de Souza" <jose.souza@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Werner Sembach <wse@tuxedocomputers.com>
Cc: Aditya Swarup <aditya.swarup@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: <stable@vger.kernel.org> # v5.12+

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 4 ++--
 drivers/gpu/drm/i915/display/intel_hdmi.h | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 45cf0ab04009..381a9de3a015 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -3126,8 +3126,8 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
  */
 int
 intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width, int num_slices,
-		       int output_format, bool hdmi_all_bpp,
-		       int hdmi_max_chunk_bytes)
+		       enum intel_output_format output_format,
+		       bool hdmi_all_bpp, int hdmi_max_chunk_bytes)
 {
 	int max_dsc_bpp, min_dsc_bpp;
 	int target_bytes;
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h b/drivers/gpu/drm/i915/display/intel_hdmi.h
index b577c38fa90c..fe40e49d2962 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.h
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
@@ -10,6 +10,7 @@
 #include <linux/types.h>
 
 #include "i915_reg.h"
+#include "intel_display_types.h"
 
 struct drm_connector;
 struct drm_encoder;
@@ -49,8 +50,8 @@ bool intel_hdmi_limited_color_range(const struct intel_crtc_state *crtc_state,
 bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
 			     int bpc, bool has_hdmi_sink, bool ycbcr420_output);
 int intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width,
-			   int num_slices, int output_format, bool hdmi_all_bpp,
-			   int hdmi_max_chunk_bytes);
+			   int num_slices, enum intel_output_format output_format,
+			   bool hdmi_all_bpp, int hdmi_max_chunk_bytes);
 int intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
 				  int src_max_slices, int src_max_slice_width,
 				  int hdmi_max_slices, int hdmi_throughput);
-- 
2.25.1

Re: [PATCH 1/3] drm/i915_hdmi: Fix the definition of intel_hdmi_dsc_get_bpp

[Jani Nikula]

On Tue, 25 Jan 2022, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
> Fix the data-type of the argument output_format to enum, for the
> function intel_hdmi_dsc_get_bpp.

The subject prefix should be "drm/i915/hdmi:".

>
> Fixes: 6e6cb758e035 ("drm/i915: Add helper functions for calculating DSC
> parameters for HDMI2.1")

The Fixes: tag should not be wrapped.

>

Superfluous blank line.

> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: "Ville Syrj_l_" <ville.syrjala@linux.intel.com>
> Cc: "Jos_ Roberto de Souza" <jose.souza@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Werner Sembach <wse@tuxedocomputers.com>
> Cc: Aditya Swarup <aditya.swarup@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Please consider trimming the Cc lines manually, this is quite excessive
for a patch this simple.

> Cc: <stable@vger.kernel.org> # v5.12+

No, we don't really want to backport this stuff to stable. There's no
functional impact here.

>

Superfluous blank line.

> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 4 ++--
>  drivers/gpu/drm/i915/display/intel_hdmi.h | 5 +++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 45cf0ab04009..381a9de3a015 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -3126,8 +3126,8 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
>   */
>  int
>  intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width, int num_slices,
> -		       int output_format, bool hdmi_all_bpp,
> -		       int hdmi_max_chunk_bytes)
> +		       enum intel_output_format output_format,
> +		       bool hdmi_all_bpp, int hdmi_max_chunk_bytes)
>  {
>  	int max_dsc_bpp, min_dsc_bpp;
>  	int target_bytes;
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h b/drivers/gpu/drm/i915/display/intel_hdmi.h
> index b577c38fa90c..fe40e49d2962 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.h
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
> @@ -10,6 +10,7 @@
>  #include <linux/types.h>
>  
>  #include "i915_reg.h"
> +#include "intel_display_types.h"

Please avoid including headers from other headers if at all possible.

Here we can forward declare

enum intel_output_format;

along with the other forward declarations below. Strictly speaking it's
not standards compliant to forward declare enums but it does work and we
use it all over the place.

BR,
Jani.

>  
>  struct drm_connector;
>  struct drm_encoder;
> @@ -49,8 +50,8 @@ bool intel_hdmi_limited_color_range(const struct intel_crtc_state *crtc_state,
>  bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
>  			     int bpc, bool has_hdmi_sink, bool ycbcr420_output);
>  int intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width,
> -			   int num_slices, int output_format, bool hdmi_all_bpp,
> -			   int hdmi_max_chunk_bytes);
> +			   int num_slices, enum intel_output_format output_format,
> +			   bool hdmi_all_bpp, int hdmi_max_chunk_bytes);
>  int intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
>  				  int src_max_slices, int src_max_slice_width,
>  				  int hdmi_max_slices, int hdmi_throughput);

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [PATCH 1/3] drm/i915_hdmi: Fix the definition of intel_hdmi_dsc_get_bpp

[Nautiyal, Ankit K]

On 1/25/2022 3:15 PM, Jani Nikula wrote:
> On Tue, 25 Jan 2022, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
>> Fix the data-type of the argument output_format to enum, for the
>> function intel_hdmi_dsc_get_bpp.
> The subject prefix should be "drm/i915/hdmi:".
Thanks for pointing it out. Will fix this.
>
>> Fixes: 6e6cb758e035 ("drm/i915: Add helper functions for calculating DSC
>> parameters for HDMI2.1")
> The Fixes: tag should not be wrapped.
Noted.
>
> Superfluous blank line.

I think I just copy pasted from drm fixes, without taking care of this. 
Will fix this.

>
>> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Cc: Uma Shankar <uma.shankar@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: "Ville Syrj_l_" <ville.syrjala@linux.intel.com>
>> Cc: "Jos_ Roberto de Souza" <jose.souza@intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> Cc: Lyude Paul <lyude@redhat.com>
>> Cc: Werner Sembach <wse@tuxedocomputers.com>
>> Cc: Aditya Swarup <aditya.swarup@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Please consider trimming the Cc lines manually, this is quite excessive
> for a patch this simple.
Noted. Will take care in next version.
>
>> Cc: <stable@vger.kernel.org> # v5.12+
> No, we don't really want to backport this stuff to stable. There's no
> functional impact here.
Got it.
> Superfluous blank line.
Thanks for catching this, will remove.
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_hdmi.c | 4 ++--
>>   drivers/gpu/drm/i915/display/intel_hdmi.h | 5 +++--
>>   2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> index 45cf0ab04009..381a9de3a015 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> @@ -3126,8 +3126,8 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
>>    */
>>   int
>>   intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width, int num_slices,
>> -		       int output_format, bool hdmi_all_bpp,
>> -		       int hdmi_max_chunk_bytes)
>> +		       enum intel_output_format output_format,
>> +		       bool hdmi_all_bpp, int hdmi_max_chunk_bytes)
>>   {
>>   	int max_dsc_bpp, min_dsc_bpp;
>>   	int target_bytes;
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h b/drivers/gpu/drm/i915/display/intel_hdmi.h
>> index b577c38fa90c..fe40e49d2962 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.h
>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
>> @@ -10,6 +10,7 @@
>>   #include <linux/types.h>
>>   
>>   #include "i915_reg.h"
>> +#include "intel_display_types.h"
> Please avoid including headers from other headers if at all possible.
>
> Here we can forward declare
>
> enum intel_output_format;
>
> along with the other forward declarations below. Strictly speaking it's
> not standards compliant to forward declare enums but it does work and we
> use it all over the place.
>
> BR,
> Jani.

Right that make sense, will get rid of the header-file and forward 
declare the enum.

Thanks for the inputs Jani. Will address all of these in the next version.

Regards,

Ankit

>
>>   
>>   struct drm_connector;
>>   struct drm_encoder;
>> @@ -49,8 +50,8 @@ bool intel_hdmi_limited_color_range(const struct intel_crtc_state *crtc_state,
>>   bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
>>   			     int bpc, bool has_hdmi_sink, bool ycbcr420_output);
>>   int intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width,
>> -			   int num_slices, int output_format, bool hdmi_all_bpp,
>> -			   int hdmi_max_chunk_bytes);
>> +			   int num_slices, enum intel_output_format output_format,
>> +			   bool hdmi_all_bpp, int hdmi_max_chunk_bytes);
>>   int intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
>>   				  int src_max_slices, int src_max_slice_width,
>>   				  int hdmi_max_slices, int hdmi_throughput);

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

[Ankit Nautiyal]

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(-)

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

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

[Jani Nikula]

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

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [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@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__ */

[PATCH 3/3] drm/i915/display: Simplify helpers for getting DSC slices and bpp

[Ankit Nautiyal]

Genralize the helper for getting DSC slice count and compressed bpp
for HDMI sink supporting DSC.
This patch:
-Removes the assumption on the bpc and sends it as an argument for
calculating compressed bpc.
-Sends the resolution, and output format as parameters for which the
DSC paremeters are to be calculated instead of crtc_state.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c   |  7 +++++--
 drivers/gpu/drm/i915/display/intel_hdmi.c | 24 ++++++++++++-----------
 drivers/gpu/drm/i915/display/intel_hdmi.h |  5 +++--
 3 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index f7fe7de7e553..17d08f06499b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2357,7 +2357,9 @@ intel_dp_pcon_dsc_enc_slices(struct intel_dp *intel_dp,
 	int pcon_max_slices = drm_dp_pcon_dsc_max_slices(intel_dp->pcon_dsc_dpcd);
 	int pcon_max_slice_width = drm_dp_pcon_dsc_max_slice_width(intel_dp->pcon_dsc_dpcd);
 
-	return intel_hdmi_dsc_get_num_slices(crtc_state, pcon_max_slices,
+	return intel_hdmi_dsc_get_num_slices(&crtc_state->hw.adjusted_mode,
+					     crtc_state->output_format,
+					     pcon_max_slices,
 					     pcon_max_slice_width,
 					     hdmi_max_slices, hdmi_throughput);
 }
@@ -2374,9 +2376,10 @@ intel_dp_pcon_dsc_enc_bpp(struct intel_dp *intel_dp,
 	int pcon_fractional_bpp = drm_dp_pcon_dsc_bpp_incr(intel_dp->pcon_dsc_dpcd);
 	int hdmi_max_chunk_bytes =
 		connector->display_info.hdmi.dsc_cap.total_chunk_kbytes * 1024;
+	int bpc = crtc_state->pipe_bpp / 3;
 
 	return intel_hdmi_dsc_get_bpp(pcon_fractional_bpp, slice_width,
-				      num_slices, output_format, hdmi_all_bpp,
+				      num_slices, output_format, bpc, hdmi_all_bpp,
 				      hdmi_max_chunk_bytes);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 381a9de3a015..f75e2384da63 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -3004,7 +3004,8 @@ int intel_hdmi_dsc_get_slice_height(int vactive)
  * intel_hdmi_dsc_get_num_slices - get no. of dsc slices based on dsc encoder
  * and dsc decoder capabilities
  *
- * @crtc_state: intel crtc_state
+ * @mode: drm_display_mode for which num of slices are needed
+ * @output_format : pipe output format
  * @src_max_slices: maximum slices supported by the DSC encoder
  * @src_max_slice_width: maximum slice width supported by DSC encoder
  * @hdmi_max_slices: maximum slices supported by sink DSC decoder
@@ -3014,7 +3015,8 @@ int intel_hdmi_dsc_get_slice_height(int vactive)
  * and decoder.
  */
 int
-intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
+intel_hdmi_dsc_get_num_slices(const struct drm_display_mode *mode,
+			      enum intel_output_format output_format,
 			      int src_max_slices, int src_max_slice_width,
 			      int hdmi_max_slices, int hdmi_throughput)
 {
@@ -3036,7 +3038,7 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
 	int max_throughput; /* max clock freq. in khz per slice */
 	int max_slice_width;
 	int slice_width;
-	int pixel_clock = crtc_state->hw.adjusted_mode.crtc_clock;
+	int pixel_clock = mode->crtc_clock;
 
 	if (!hdmi_throughput)
 		return 0;
@@ -3047,8 +3049,8 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
 	 * for 4:4:4 is 1.0. Multiplying these factors by 10 and later
 	 * dividing adjusted clock value by 10.
 	 */
-	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 ||
-	    crtc_state->output_format == INTEL_OUTPUT_FORMAT_RGB)
+	if (output_format == INTEL_OUTPUT_FORMAT_YCBCR444 ||
+	    output_format == INTEL_OUTPUT_FORMAT_RGB)
 		kslice_adjust = 10;
 	else
 		kslice_adjust = 5;
@@ -3103,7 +3105,7 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
 		else
 			return 0;
 
-		slice_width = DIV_ROUND_UP(crtc_state->hw.adjusted_mode.hdisplay, target_slices);
+		slice_width = DIV_ROUND_UP(mode->hdisplay, target_slices);
 		if (slice_width >= max_slice_width)
 			min_slices = target_slices + 1;
 	} while (slice_width >= max_slice_width);
@@ -3119,6 +3121,7 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
  * @slice_width: dsc slice width supported by the source and sink
  * @num_slices: num of slices supported by the source and sink
  * @output_format: video output format
+ * @bpc: bits per color
  * @hdmi_all_bpp: sink supports decoding of 1/16th bpp setting
  * @hdmi_max_chunk_bytes: max bytes in a line of chunks supported by sink
  *
@@ -3126,7 +3129,7 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
  */
 int
 intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width, int num_slices,
-		       enum intel_output_format output_format,
+		       enum intel_output_format output_format, int bpc,
 		       bool hdmi_all_bpp, int hdmi_max_chunk_bytes)
 {
 	int max_dsc_bpp, min_dsc_bpp;
@@ -3144,18 +3147,17 @@ intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width, int num_slices,
 	 * for each bpp we check if no of bytes can be supported by HDMI sink
 	 */
 
-	/* Assuming: bpc as 8*/
 	if (output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
 		min_dsc_bpp = 6;
-		max_dsc_bpp = 3 * 4; /* 3*bpc/2 */
+		max_dsc_bpp = 3 * bpc / 2;
 	} else if (output_format == INTEL_OUTPUT_FORMAT_YCBCR444 ||
 		   output_format == INTEL_OUTPUT_FORMAT_RGB) {
 		min_dsc_bpp = 8;
-		max_dsc_bpp = 3 * 8; /* 3*bpc */
+		max_dsc_bpp = 3 * bpc;
 	} else {
 		/* Assuming 4:2:2 encoding */
 		min_dsc_bpp = 7;
-		max_dsc_bpp = 2 * 8; /* 2*bpc */
+		max_dsc_bpp = 2 * bpc;
 	}
 
 	/*
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h b/drivers/gpu/drm/i915/display/intel_hdmi.h
index fe40e49d2962..0866bd9da7ed 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.h
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
@@ -51,8 +51,9 @@ bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
 			     int bpc, bool has_hdmi_sink, bool ycbcr420_output);
 int intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width,
 			   int num_slices, enum intel_output_format output_format,
-			   bool hdmi_all_bpp, int hdmi_max_chunk_bytes);
-int intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
+			   int bpc, bool hdmi_all_bpp, int hdmi_max_chunk_bytes);
+int intel_hdmi_dsc_get_num_slices(const struct drm_display_mode *mode,
+				  enum intel_output_format output_format,
 				  int src_max_slices, int src_max_slice_width,
 				  int hdmi_max_slices, int hdmi_throughput);
 int intel_hdmi_dsc_get_slice_height(int vactive);
-- 
2.25.1