[PATCH 00/12] DP DSC min/max src bpc fixes

[PATCH 00/12] DP DSC min/max src bpc fixes

[Ankit Nautiyal]

Use helpers for source min/max src bpc appropriately for dp mst case and
to limit max_requested_bpc property min/max values. 

Rev2: Drop patch to limit max_requested_bpc based on src DSC bpc
limits. Instead add change to ignore max_requested_bpc if its
too low for DSC.

Rev3: Update patch#1 commit message.

Rev4: Rebase.

Rev5: Address Jani's comment on patch#3.

Rev6: Rebase.

Rev7: Add patch to fix return type for dsc_min/max_src bpc helpers to
int.

Rev8:
-Drop the first patch.
-Refactor helpers for fec support.
-Add patches to refactor pipe_bpp limits and link limits.

Ankit Nautiyal (12):
  drm/i915/dp: Refactor FEC support check in intel_dp_supports_dsc
  drm/i915/dp: Return early if DSC not supported
  drm/i915/dp: Separate out helper for compute fec_enable
  drm/i915/dp: Remove HAS_DSC macro for intel_dp_dsc_max_src_input_bpc
  drm/i915/dp: Return int from dsc_max/min_src_input_bpc helpers
  drm/i915/dp_mst: Use helpers to get dsc min/max input bpc
  drm/i915/dp: Drop max_requested_bpc for dsc pipe_min/max bpp
  drm/i915/dp: Refactor pipe_bpp limits with dsc
  drm/i915/dp_mst: Refactor pipe_bpp limits with dsc for mst
  drm/i915/dp: Use clamp for pipe_bpp limits with DSC
  drm/i915/dp: Make dsc helpers accept const crtc_state pointers
  drm/i915/dp: Set the DSC link limits
    intel_dp_compute_config_link_bpp_limits

 drivers/gpu/drm/i915/display/intel_dp.c     | 189 +++++++++++---------
 drivers/gpu/drm/i915/display/intel_dp.h     |  14 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c |  18 +-
 3 files changed, 120 insertions(+), 101 deletions(-)

-- 
2.45.2

[PATCH 01/12] drm/i915/dp: Refactor FEC support check in intel_dp_supports_dsc

[Ankit Nautiyal]

Forward Error Correction is required for DP if we are using DSC but
is optional for eDP.

Currently the helper intel_dp_supports_dsc checks if fec_enable is set for
DP or not. The helper is called after fec_enable is set in crtc_state.

Instead of this a better approach would be to:
first, call intel_dp_supports_dsc to check for DSC support
(along with FEC requirement for DP) and then set fec_enable for DP
(if not already set) in crtc_state.

To achieve this, remove the check for fec_enable in the helper and instead
check for FEC support for DP. With this change the helper
intel_dp_supports_dsc can be called earlier and return early if DSC is
not supported. The structure intel_dp is added to the helper to get the
FEC support for DP.

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

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 053a9a4182e7..db9ddbcdd159 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1632,13 +1632,15 @@ bool intel_dp_supports_fec(struct intel_dp *intel_dp,
 		drm_dp_sink_supports_fec(connector->dp.fec_capability);
 }
 
-bool intel_dp_supports_dsc(const struct intel_connector *connector,
+bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
+			   const struct intel_connector *connector,
 			   const struct intel_crtc_state *crtc_state)
 {
 	if (!intel_dp_has_dsc(connector))
 		return false;
 
-	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP) && !crtc_state->fec_enable)
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP) &&
+	    !intel_dp_supports_fec(intel_dp, connector, crtc_state))
 		return false;
 
 	return intel_dsc_source_support(crtc_state);
@@ -2376,7 +2378,7 @@ 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(connector, 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))
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 48f10876be65..4ae54e9718ce 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -170,7 +170,8 @@ bool intel_dp_supports_fec(struct intel_dp *intel_dp,
 			   const struct intel_connector *connector,
 			   const struct intel_crtc_state *pipe_config);
 
-bool intel_dp_supports_dsc(const struct intel_connector *connector,
+bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
+			   const struct intel_connector *connector,
 			   const struct intel_crtc_state *crtc_state);
 
 u32 intel_dp_dsc_nearest_valid_bpp(struct drm_i915_private *i915, u32 bpp, u32 pipe_bpp);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index f058360a2641..0662736849ac 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -498,12 +498,13 @@ adjust_limits_for_dsc_hblank_expansion_quirk(const struct intel_connector *conne
 	struct intel_display *display = to_intel_display(connector);
 	const struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	int min_bpp_x16 = limits->link.min_bpp_x16;
+	struct intel_dp *intel_dp = connector->mst_port;
 
 	if (!hblank_expansion_quirk_needs_dsc(connector, crtc_state, limits))
 		return true;
 
 	if (!dsc) {
-		if (intel_dp_supports_dsc(connector, crtc_state)) {
+		if (intel_dp_supports_dsc(intel_dp, connector, crtc_state)) {
 			drm_dbg_kms(display->drm,
 				    "[CRTC:%d:%s][CONNECTOR:%d:%s] DSC needed by hblank expansion quirk\n",
 				    crtc->base.base.id, crtc->base.name,
@@ -648,7 +649,7 @@ static int mst_stream_compute_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(connector, pipe_config))
+		if (!intel_dp_supports_dsc(intel_dp, connector, pipe_config))
 			return -EINVAL;
 
 		if (!mst_stream_compute_config_limits(intel_dp, connector,
-- 
2.45.2

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

[Ankit Nautiyal]

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;
+
 		if (!intel_dp_compute_config_limits(intel_dp, pipe_config,
 						    respect_downstream_limits,
 						    true,
-- 
2.45.2

[PATCH 03/12] drm/i915/dp: Separate out helper for compute fec_enable

[Ankit Nautiyal]

Make a separate function for setting fec_enable in crtc_state.
Drop the check for FEC support as its already checked while checking for
DSC support.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 30 +++++++++++++++++--------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index dee15a05e7fd..d82e25d0dc5a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2352,6 +2352,26 @@ static int intel_edp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 	return 0;
 }
 
+static void intel_dp_compute_fec_config(struct intel_dp *intel_dp,
+					struct intel_crtc_state *pipe_config)
+{
+	if (pipe_config->fec_enable)
+		return;
+
+	/*
+	 * Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional.
+	 * Since, FEC is a bandwidth overhead, continue to not enable it for
+	 * eDP. Until, there is a good reason to do so.
+	 */
+	if (intel_dp_is_edp(intel_dp))
+		return;
+
+	if (intel_dp_is_uhbr(pipe_config))
+		return;
+
+	pipe_config->fec_enable = true;
+}
+
 int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 				struct intel_crtc_state *pipe_config,
 				struct drm_connector_state *conn_state,
@@ -2368,15 +2388,7 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 	int num_joined_pipes = intel_crtc_num_joined_pipes(pipe_config);
 	int ret;
 
-	/*
-	 * Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional.
-	 * Since, FEC is a bandwidth overhead, continue to not enable it for
-	 * eDP. Until, there is a good reason to do so.
-	 */
-	pipe_config->fec_enable = pipe_config->fec_enable ||
-		(!intel_dp_is_edp(intel_dp) &&
-		 intel_dp_supports_fec(intel_dp, connector, pipe_config) &&
-		 !intel_dp_is_uhbr(pipe_config));
+	intel_dp_compute_fec_config(intel_dp, pipe_config);
 
 	if (!intel_dp_dsc_supports_format(connector, pipe_config->output_format))
 		return -EINVAL;
-- 
2.45.2

[PATCH 04/12] drm/i915/dp: Remove HAS_DSC macro for intel_dp_dsc_max_src_input_bpc

[Ankit Nautiyal]

DSC support is already checked before the helper
intel_dp_dsc_max_src_input_bpc is called.
Remove the check from the helper.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index d82e25d0dc5a..dd60ca532ae3 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2195,7 +2195,7 @@ static
 u8 intel_dp_dsc_min_src_input_bpc(struct drm_i915_private *i915)
 {
 	/* Min DSC Input BPC for ICL+ is 8 */
-	return HAS_DSC(i915) ? 8 : 0;
+	return 8;
 }
 
 static
-- 
2.45.2

[PATCH 05/12] drm/i915/dp: Return int from dsc_max/min_src_input_bpc helpers

[Ankit Nautiyal]

Use ints for dsc_max/min_bpc instead of u8 in
dsc_max/min_src_input_bpc helpers and their callers.
This will also help replace min_t/max_t macros with min/max ones.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index dd60ca532ae3..10fb2b9e6bc1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1789,7 +1789,7 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 }
 
 static
-u8 intel_dp_dsc_max_src_input_bpc(struct drm_i915_private *i915)
+int intel_dp_dsc_max_src_input_bpc(struct drm_i915_private *i915)
 {
 	/* Max DSC Input BPC for ICL is 10 and for TGL+ is 12 */
 	if (DISPLAY_VER(i915) >= 12)
@@ -1806,14 +1806,14 @@ int intel_dp_dsc_compute_max_bpp(const struct intel_connector *connector,
 	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 	int i, num_bpc;
 	u8 dsc_bpc[3] = {};
-	u8 dsc_max_bpc;
+	int dsc_max_bpc;
 
 	dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(i915);
 
 	if (!dsc_max_bpc)
 		return dsc_max_bpc;
 
-	dsc_max_bpc = min_t(u8, dsc_max_bpc, max_req_bpc);
+	dsc_max_bpc = min(dsc_max_bpc, max_req_bpc);
 
 	num_bpc = drm_dp_dsc_sink_supported_input_bpcs(connector->dp.dsc_dpcd,
 						       dsc_bpc);
@@ -2192,7 +2192,7 @@ static int dsc_compute_compressed_bpp(struct intel_dp *intel_dp,
 }
 
 static
-u8 intel_dp_dsc_min_src_input_bpc(struct drm_i915_private *i915)
+int intel_dp_dsc_min_src_input_bpc(struct drm_i915_private *i915)
 {
 	/* Min DSC Input BPC for ICL+ is 8 */
 	return 8;
@@ -2204,7 +2204,7 @@ bool is_dsc_pipe_bpp_sufficient(struct drm_i915_private *i915,
 				struct link_config_limits *limits,
 				int pipe_bpp)
 {
-	u8 dsc_max_bpc, dsc_min_bpc, dsc_max_pipe_bpp, dsc_min_pipe_bpp;
+	int dsc_max_bpc, dsc_min_bpc, dsc_max_pipe_bpp, dsc_min_pipe_bpp;
 
 	dsc_max_bpc = min(intel_dp_dsc_max_src_input_bpc(i915), conn_state->max_requested_bpc);
 	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(i915);
@@ -2249,9 +2249,9 @@ static int intel_dp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	const struct intel_connector *connector =
 		to_intel_connector(conn_state->connector);
-	u8 max_req_bpc = conn_state->max_requested_bpc;
-	u8 dsc_max_bpc, dsc_max_bpp;
-	u8 dsc_min_bpc, dsc_min_bpp;
+	int max_req_bpc = conn_state->max_requested_bpc;
+	int dsc_max_bpc, dsc_max_bpp;
+	int dsc_min_bpc, dsc_min_bpp;
 	u8 dsc_bpc[3] = {};
 	int forced_bpp, pipe_bpp;
 	int num_bpc, i, ret;
@@ -2271,7 +2271,7 @@ static int intel_dp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 	if (!dsc_max_bpc)
 		return -EINVAL;
 
-	dsc_max_bpc = min_t(u8, dsc_max_bpc, max_req_bpc);
+	dsc_max_bpc = min(dsc_max_bpc, max_req_bpc);
 	dsc_max_bpp = min(dsc_max_bpc * 3, limits->pipe.max_bpp);
 
 	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(i915);
-- 
2.45.2

[PATCH 06/12] drm/i915/dp_mst: Use helpers to get dsc min/max input bpc

[Ankit Nautiyal]

Use helpers for source min/max input bpc with DSC.
While at it, make them return int instead of u8 and use struct
intel_display.

v2: Make the helpers return int instead of u8. (Jani)
v3: Use min/max macros instead of min_t/max_t. (Jani)
v4: Use struct intel_display.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c     | 25 ++++++++++-----------
 drivers/gpu/drm/i915/display/intel_dp.h     |  3 +++
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 13 +++++------
 3 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 10fb2b9e6bc1..e6dacdf00e0b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1788,13 +1788,12 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 	return -EINVAL;
 }
 
-static
-int intel_dp_dsc_max_src_input_bpc(struct drm_i915_private *i915)
+int intel_dp_dsc_max_src_input_bpc(struct intel_display *display)
 {
 	/* Max DSC Input BPC for ICL is 10 and for TGL+ is 12 */
-	if (DISPLAY_VER(i915) >= 12)
+	if (DISPLAY_VER(display) >= 12)
 		return 12;
-	if (DISPLAY_VER(i915) == 11)
+	if (DISPLAY_VER(display) == 11)
 		return 10;
 
 	return 0;
@@ -1803,12 +1802,12 @@ int intel_dp_dsc_max_src_input_bpc(struct drm_i915_private *i915)
 int intel_dp_dsc_compute_max_bpp(const struct intel_connector *connector,
 				 u8 max_req_bpc)
 {
-	struct drm_i915_private *i915 = to_i915(connector->base.dev);
+	struct intel_display *display = to_intel_display(connector);
 	int i, num_bpc;
 	u8 dsc_bpc[3] = {};
 	int dsc_max_bpc;
 
-	dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(i915);
+	dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(display);
 
 	if (!dsc_max_bpc)
 		return dsc_max_bpc;
@@ -2191,8 +2190,7 @@ static int dsc_compute_compressed_bpp(struct intel_dp *intel_dp,
 					   dsc_max_bpp, dsc_min_bpp, pipe_bpp, timeslots);
 }
 
-static
-int intel_dp_dsc_min_src_input_bpc(struct drm_i915_private *i915)
+int intel_dp_dsc_min_src_input_bpc(struct intel_display *display)
 {
 	/* Min DSC Input BPC for ICL+ is 8 */
 	return 8;
@@ -2204,10 +2202,11 @@ bool is_dsc_pipe_bpp_sufficient(struct drm_i915_private *i915,
 				struct link_config_limits *limits,
 				int pipe_bpp)
 {
+	struct intel_display *display = to_intel_display(&i915->drm);
 	int dsc_max_bpc, dsc_min_bpc, dsc_max_pipe_bpp, dsc_min_pipe_bpp;
 
-	dsc_max_bpc = min(intel_dp_dsc_max_src_input_bpc(i915), conn_state->max_requested_bpc);
-	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(i915);
+	dsc_max_bpc = min(intel_dp_dsc_max_src_input_bpc(display), conn_state->max_requested_bpc);
+	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(display);
 
 	dsc_max_pipe_bpp = min(dsc_max_bpc * 3, limits->pipe.max_bpp);
 	dsc_min_pipe_bpp = max(dsc_min_bpc * 3, limits->pipe.min_bpp);
@@ -2246,7 +2245,7 @@ static int intel_dp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 					 struct link_config_limits *limits,
 					 int timeslots)
 {
-	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+	struct intel_display *display = to_intel_display(intel_dp);
 	const struct intel_connector *connector =
 		to_intel_connector(conn_state->connector);
 	int max_req_bpc = conn_state->max_requested_bpc;
@@ -2267,14 +2266,14 @@ static int intel_dp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 		}
 	}
 
-	dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(i915);
+	dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(display);
 	if (!dsc_max_bpc)
 		return -EINVAL;
 
 	dsc_max_bpc = min(dsc_max_bpc, max_req_bpc);
 	dsc_max_bpp = min(dsc_max_bpc * 3, limits->pipe.max_bpp);
 
-	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(i915);
+	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(display);
 	dsc_min_bpp = max(dsc_min_bpc * 3, limits->pipe.min_bpp);
 
 	/*
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 4ae54e9718ce..e5a25e5cbc25 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -20,6 +20,7 @@ struct intel_atomic_state;
 struct intel_connector;
 struct intel_crtc_state;
 struct intel_digital_port;
+struct intel_display;
 struct intel_dp;
 struct intel_encoder;
 
@@ -207,5 +208,7 @@ bool intel_dp_link_params_valid(struct intel_dp *intel_dp, int link_rate,
 				u8 lane_count);
 bool intel_dp_has_connector(struct intel_dp *intel_dp,
 			    const struct drm_connector_state *conn_state);
+int intel_dp_dsc_max_src_input_bpc(struct intel_display *display);
+int intel_dp_dsc_min_src_input_bpc(struct intel_display *display);
 
 #endif /* __INTEL_DP_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 0662736849ac..9568924d143b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -366,17 +366,14 @@ static int mst_stream_dsc_compute_link_config(struct intel_encoder *encoder,
 	int i, num_bpc;
 	u8 dsc_bpc[3] = {};
 	int min_bpp, max_bpp, sink_min_bpp, sink_max_bpp;
-	u8 dsc_max_bpc;
+	int dsc_max_bpc, dsc_min_bpc;
 	int min_compressed_bpp, max_compressed_bpp;
 
-	/* Max DSC Input BPC for ICL is 10 and for TGL+ is 12 */
-	if (DISPLAY_VER(display) >= 12)
-		dsc_max_bpc = min_t(u8, 12, conn_state->max_requested_bpc);
-	else
-		dsc_max_bpc = min_t(u8, 10, conn_state->max_requested_bpc);
+	dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(display);
+	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(display);
 
-	max_bpp = min_t(u8, dsc_max_bpc * 3, limits->pipe.max_bpp);
-	min_bpp = limits->pipe.min_bpp;
+	max_bpp = min(dsc_max_bpc * 3, limits->pipe.max_bpp);
+	min_bpp = max(dsc_min_bpc * 3, limits->pipe.min_bpp);
 
 	num_bpc = drm_dp_dsc_sink_supported_input_bpcs(connector->dp.dsc_dpcd,
 						       dsc_bpc);
-- 
2.45.2

[PATCH 07/12] drm/i915/dp: Drop max_requested_bpc for dsc pipe_min/max bpp

[Ankit Nautiyal]

Currently we are including both max_requested_bpc and
limits->pipe.bpp_max while computing maximum possible pipe bpp with dsc.
However, while setting limits->pipe.max_bpp, the max_requested_bpc is
already taken into account.

Drop the redundant check for max_requested_bpc and use only
limits->pipe.bpp_max. This will also result in dropping conn_state
argument in functions where it was used only to get max_requested_bpc.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index e6dacdf00e0b..0af58ddbe69a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2198,14 +2198,13 @@ int intel_dp_dsc_min_src_input_bpc(struct intel_display *display)
 
 static
 bool is_dsc_pipe_bpp_sufficient(struct drm_i915_private *i915,
-				struct drm_connector_state *conn_state,
 				struct link_config_limits *limits,
 				int pipe_bpp)
 {
 	struct intel_display *display = to_intel_display(&i915->drm);
 	int dsc_max_bpc, dsc_min_bpc, dsc_max_pipe_bpp, dsc_min_pipe_bpp;
 
-	dsc_max_bpc = min(intel_dp_dsc_max_src_input_bpc(display), conn_state->max_requested_bpc);
+	dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(display);
 	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(display);
 
 	dsc_max_pipe_bpp = min(dsc_max_bpc * 3, limits->pipe.max_bpp);
@@ -2217,7 +2216,6 @@ bool is_dsc_pipe_bpp_sufficient(struct drm_i915_private *i915,
 
 static
 int intel_dp_force_dsc_pipe_bpp(struct intel_dp *intel_dp,
-				struct drm_connector_state *conn_state,
 				struct link_config_limits *limits)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
@@ -2228,7 +2226,7 @@ int intel_dp_force_dsc_pipe_bpp(struct intel_dp *intel_dp,
 
 	forced_bpp = intel_dp->force_dsc_bpc * 3;
 
-	if (is_dsc_pipe_bpp_sufficient(i915, conn_state, limits, forced_bpp)) {
+	if (is_dsc_pipe_bpp_sufficient(i915, limits, forced_bpp)) {
 		drm_dbg_kms(&i915->drm, "Input DSC BPC forced to %d\n", intel_dp->force_dsc_bpc);
 		return forced_bpp;
 	}
@@ -2248,14 +2246,13 @@ static int intel_dp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 	struct intel_display *display = to_intel_display(intel_dp);
 	const struct intel_connector *connector =
 		to_intel_connector(conn_state->connector);
-	int max_req_bpc = conn_state->max_requested_bpc;
 	int dsc_max_bpc, dsc_max_bpp;
 	int dsc_min_bpc, dsc_min_bpp;
 	u8 dsc_bpc[3] = {};
 	int forced_bpp, pipe_bpp;
 	int num_bpc, i, ret;
 
-	forced_bpp = intel_dp_force_dsc_pipe_bpp(intel_dp, conn_state, limits);
+	forced_bpp = intel_dp_force_dsc_pipe_bpp(intel_dp, limits);
 
 	if (forced_bpp) {
 		ret = dsc_compute_compressed_bpp(intel_dp, connector, pipe_config,
@@ -2270,7 +2267,6 @@ static int intel_dp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 	if (!dsc_max_bpc)
 		return -EINVAL;
 
-	dsc_max_bpc = min(dsc_max_bpc, max_req_bpc);
 	dsc_max_bpp = min(dsc_max_bpc * 3, limits->pipe.max_bpp);
 
 	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(display);
@@ -2310,16 +2306,16 @@ static int intel_edp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 	int dsc_src_min_bpp, dsc_sink_min_bpp, dsc_min_bpp;
 	int dsc_src_max_bpp, dsc_sink_max_bpp, dsc_max_bpp;
 
-	forced_bpp = intel_dp_force_dsc_pipe_bpp(intel_dp, conn_state, limits);
+	forced_bpp = intel_dp_force_dsc_pipe_bpp(intel_dp, limits);
 
 	if (forced_bpp) {
 		pipe_bpp = forced_bpp;
 	} else {
-		int max_bpc = min(limits->pipe.max_bpp / 3, (int)conn_state->max_requested_bpc);
+		int max_bpc = limits->pipe.max_bpp / 3;
 
 		/* For eDP use max bpp that can be supported with DSC. */
 		pipe_bpp = intel_dp_dsc_compute_max_bpp(connector, max_bpc);
-		if (!is_dsc_pipe_bpp_sufficient(i915, conn_state, limits, pipe_bpp)) {
+		if (!is_dsc_pipe_bpp_sufficient(i915, limits, pipe_bpp)) {
 			drm_dbg_kms(&i915->drm,
 				    "Computed BPC is not in DSC BPC limits\n");
 			return -EINVAL;
-- 
2.45.2

[PATCH 08/12] drm/i915/dp: Refactor pipe_bpp limits with dsc

[Ankit Nautiyal]

With DSC there are additional limits for pipe_bpp. Currently these are
scattered in different places.
Instead set the limits->pipe.max/min_bpp in one place and use them
wherever required.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 49 ++++++++++++-------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 0af58ddbe69a..b6d5e8eb75f5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2197,21 +2197,11 @@ int intel_dp_dsc_min_src_input_bpc(struct intel_display *display)
 }
 
 static
-bool is_dsc_pipe_bpp_sufficient(struct drm_i915_private *i915,
-				struct link_config_limits *limits,
+bool is_dsc_pipe_bpp_sufficient(struct link_config_limits *limits,
 				int pipe_bpp)
 {
-	struct intel_display *display = to_intel_display(&i915->drm);
-	int dsc_max_bpc, dsc_min_bpc, dsc_max_pipe_bpp, dsc_min_pipe_bpp;
-
-	dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(display);
-	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(display);
-
-	dsc_max_pipe_bpp = min(dsc_max_bpc * 3, limits->pipe.max_bpp);
-	dsc_min_pipe_bpp = max(dsc_min_bpc * 3, limits->pipe.min_bpp);
-
-	return pipe_bpp >= dsc_min_pipe_bpp &&
-	       pipe_bpp <= dsc_max_pipe_bpp;
+	return pipe_bpp >= limits->pipe.min_bpp &&
+	       pipe_bpp <= limits->pipe.max_bpp;
 }
 
 static
@@ -2226,7 +2216,7 @@ int intel_dp_force_dsc_pipe_bpp(struct intel_dp *intel_dp,
 
 	forced_bpp = intel_dp->force_dsc_bpc * 3;
 
-	if (is_dsc_pipe_bpp_sufficient(i915, limits, forced_bpp)) {
+	if (is_dsc_pipe_bpp_sufficient(limits, forced_bpp)) {
 		drm_dbg_kms(&i915->drm, "Input DSC BPC forced to %d\n", intel_dp->force_dsc_bpc);
 		return forced_bpp;
 	}
@@ -2243,11 +2233,10 @@ static int intel_dp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 					 struct link_config_limits *limits,
 					 int timeslots)
 {
-	struct intel_display *display = to_intel_display(intel_dp);
 	const struct intel_connector *connector =
 		to_intel_connector(conn_state->connector);
-	int dsc_max_bpc, dsc_max_bpp;
-	int dsc_min_bpc, dsc_min_bpp;
+	int dsc_max_bpp;
+	int dsc_min_bpp;
 	u8 dsc_bpc[3] = {};
 	int forced_bpp, pipe_bpp;
 	int num_bpc, i, ret;
@@ -2263,14 +2252,8 @@ static int intel_dp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 		}
 	}
 
-	dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(display);
-	if (!dsc_max_bpc)
-		return -EINVAL;
-
-	dsc_max_bpp = min(dsc_max_bpc * 3, limits->pipe.max_bpp);
-
-	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(display);
-	dsc_min_bpp = max(dsc_min_bpc * 3, limits->pipe.min_bpp);
+	dsc_max_bpp = limits->pipe.max_bpp;
+	dsc_min_bpp = limits->pipe.min_bpp;
 
 	/*
 	 * Get the maximum DSC bpc that will be supported by any valid
@@ -2315,7 +2298,7 @@ static int intel_edp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 
 		/* For eDP use max bpp that can be supported with DSC. */
 		pipe_bpp = intel_dp_dsc_compute_max_bpp(connector, max_bpc);
-		if (!is_dsc_pipe_bpp_sufficient(i915, limits, pipe_bpp)) {
+		if (!is_dsc_pipe_bpp_sufficient(limits, pipe_bpp)) {
 			drm_dbg_kms(&i915->drm,
 				    "Computed BPC is not in DSC BPC limits\n");
 			return -EINVAL;
@@ -2529,6 +2512,18 @@ intel_dp_compute_config_link_bpp_limits(struct intel_dp *intel_dp,
 	return true;
 }
 
+static void
+intel_dp_dsc_compute_pipe_bpp_limits(struct intel_dp *intel_dp,
+				     struct link_config_limits *limits)
+{
+	struct intel_display *display = to_intel_display(intel_dp);
+	int dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(display);
+	int dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(display);
+
+	limits->pipe.max_bpp = min(limits->pipe.max_bpp, dsc_max_bpc * 3);
+	limits->pipe.min_bpp = max(limits->pipe.min_bpp, dsc_min_bpc * 3);
+}
+
 static bool
 intel_dp_compute_config_limits(struct intel_dp *intel_dp,
 			       struct intel_crtc_state *crtc_state,
@@ -2549,6 +2544,8 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp,
 	limits->pipe.min_bpp = intel_dp_min_bpp(crtc_state->output_format);
 	limits->pipe.max_bpp = intel_dp_max_bpp(intel_dp, crtc_state,
 						     respect_downstream_limits);
+	if (dsc)
+		intel_dp_dsc_compute_pipe_bpp_limits(intel_dp, limits);
 
 	if (intel_dp->use_max_params) {
 		/*
-- 
2.45.2

[PATCH 09/12] drm/i915/dp_mst: Refactor pipe_bpp limits with dsc for mst

[Ankit Nautiyal]

Similar to DP, set the dsc limits->pipe.max/min_bpp early for MST too.
Use the limits while computing the compressed bpp.

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

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index b6d5e8eb75f5..827368b6cdb9 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2512,7 +2512,7 @@ intel_dp_compute_config_link_bpp_limits(struct intel_dp *intel_dp,
 	return true;
 }
 
-static void
+void
 intel_dp_dsc_compute_pipe_bpp_limits(struct intel_dp *intel_dp,
 				     struct link_config_limits *limits)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index e5a25e5cbc25..973b2aa0da1d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -200,6 +200,9 @@ intel_dp_compute_config_link_bpp_limits(struct intel_dp *intel_dp,
 					const struct intel_crtc_state *crtc_state,
 					bool dsc,
 					struct link_config_limits *limits);
+void
+intel_dp_dsc_compute_pipe_bpp_limits(struct intel_dp *intel_dp,
+				     struct link_config_limits *limits);
 
 void intel_dp_get_dsc_sink_cap(u8 dpcd_rev, struct intel_connector *connector);
 bool intel_dp_has_gamut_metadata_dip(struct intel_encoder *encoder);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 9568924d143b..78cb65f7cb2b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -366,14 +366,10 @@ static int mst_stream_dsc_compute_link_config(struct intel_encoder *encoder,
 	int i, num_bpc;
 	u8 dsc_bpc[3] = {};
 	int min_bpp, max_bpp, sink_min_bpp, sink_max_bpp;
-	int dsc_max_bpc, dsc_min_bpc;
 	int min_compressed_bpp, max_compressed_bpp;
 
-	dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(display);
-	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(display);
-
-	max_bpp = min(dsc_max_bpc * 3, limits->pipe.max_bpp);
-	min_bpp = max(dsc_min_bpc * 3, limits->pipe.min_bpp);
+	max_bpp = limits->pipe.max_bpp;
+	min_bpp = limits->pipe.min_bpp;
 
 	num_bpc = drm_dp_dsc_sink_supported_input_bpcs(connector->dp.dsc_dpcd,
 						       dsc_bpc);
@@ -576,6 +572,9 @@ mst_stream_compute_config_limits(struct intel_dp *intel_dp,
 
 	intel_dp_test_compute_config(intel_dp, crtc_state, limits);
 
+	if (dsc)
+		intel_dp_dsc_compute_pipe_bpp_limits(intel_dp, limits);
+
 	if (!intel_dp_compute_config_link_bpp_limits(intel_dp,
 						     crtc_state,
 						     dsc,
-- 
2.45.2

[PATCH 10/12] drm/i915/dp: Use clamp for pipe_bpp limits with DSC

[Ankit Nautiyal]

Currently to get the max pipe_bpp with dsc we take the min of
limits->pipe.max_bpp and dsc max bpp (dsc max bpc * 3). This can result
in problems when limits->pipe.max_bpp is less than the computed dsc min bpp
(dsc min bpc * 3).

Replace the min/max functions with clamp while computing
limits->pipe.max/min_bpp to ensure that the pipe_bpp limits are constrained
within the DSC-defined minimum and maximum values.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 827368b6cdb9..6cd3ae2db1a9 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2520,8 +2520,11 @@ intel_dp_dsc_compute_pipe_bpp_limits(struct intel_dp *intel_dp,
 	int dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(display);
 	int dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(display);
 
-	limits->pipe.max_bpp = min(limits->pipe.max_bpp, dsc_max_bpc * 3);
-	limits->pipe.min_bpp = max(limits->pipe.min_bpp, dsc_min_bpc * 3);
+	limits->pipe.max_bpp = clamp(limits->pipe.max_bpp,
+				     dsc_min_bpc * 3, dsc_max_bpc * 3);
+
+	limits->pipe.min_bpp = clamp(limits->pipe.min_bpp,
+				     dsc_min_bpc * 3, dsc_max_bpc * 3);
 }
 
 static bool
-- 
2.45.2

[PATCH 11/12] drm/i915/dp: Make dsc helpers accept const crtc_state pointers

[Ankit Nautiyal]

Modify the dsc helpers to get max/min compressed bpp to accept
`const struct intel_crtc_state *` pointers instead of
`struct intel_crtc_state *`.

These helpers are not supposed to modify `crtc_state`.
Accepting const pointers will allow these helpers to be called from
functions that have const pointer to crtc_state.

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

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 6cd3ae2db1a9..1adde61c5b76 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1980,7 +1980,7 @@ static int dsc_compute_link_config(struct intel_dp *intel_dp,
 
 static
 u16 intel_dp_dsc_max_sink_compressed_bppx16(const struct intel_connector *connector,
-					    struct intel_crtc_state *pipe_config,
+					    const struct intel_crtc_state *pipe_config,
 					    int bpc)
 {
 	u16 max_bppx16 = drm_edp_dsc_sink_output_bpp(connector->dp.dsc_dpcd);
@@ -2005,7 +2005,7 @@ u16 intel_dp_dsc_max_sink_compressed_bppx16(const struct intel_connector *connec
 	return 0;
 }
 
-int intel_dp_dsc_sink_min_compressed_bpp(struct intel_crtc_state *pipe_config)
+int intel_dp_dsc_sink_min_compressed_bpp(const struct intel_crtc_state *pipe_config)
 {
 	/* From Mandatory bit rate range Support Table 2-157 (DP v2.0) */
 	switch (pipe_config->output_format) {
@@ -2023,7 +2023,7 @@ int intel_dp_dsc_sink_min_compressed_bpp(struct intel_crtc_state *pipe_config)
 }
 
 int intel_dp_dsc_sink_max_compressed_bpp(const struct intel_connector *connector,
-					 struct intel_crtc_state *pipe_config,
+					 const struct intel_crtc_state *pipe_config,
 					 int bpc)
 {
 	return intel_dp_dsc_max_sink_compressed_bppx16(connector,
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 973b2aa0da1d..eac517c1d445 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -145,9 +145,9 @@ u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
 					enum intel_output_format output_format,
 					u32 pipe_bpp,
 					u32 timeslots);
-int intel_dp_dsc_sink_min_compressed_bpp(struct intel_crtc_state *pipe_config);
+int intel_dp_dsc_sink_min_compressed_bpp(const struct intel_crtc_state *pipe_config);
 int intel_dp_dsc_sink_max_compressed_bpp(const struct intel_connector *connector,
-					 struct intel_crtc_state *pipe_config,
+					 const struct intel_crtc_state *pipe_config,
 					 int bpc);
 u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
 				int mode_clock, int mode_hdisplay,
-- 
2.45.2

[PATCH 12/12] drm/i915/dp: Set the DSC link limits intel_dp_compute_config_link_bpp_limits

[Ankit Nautiyal]

The helper intel_dp_compute_config_link_bpp_limits is the correct place
to set the DSC link limits. Move the code to this function and remove
the #TODO item.

v2: Add argument intel_connector to the helper to get correct connector
for DP MST. (Imre)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c     | 60 +++++++++++----------
 drivers/gpu/drm/i915/display/intel_dp.h     |  1 +
 drivers/gpu/drm/i915/display/intel_dp_mst.c |  1 +
 3 files changed, 34 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 1adde61c5b76..21cde5487e1a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2161,21 +2161,16 @@ static int dsc_compute_compressed_bpp(struct intel_dp *intel_dp,
 {
 	const struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
-	int dsc_src_min_bpp, dsc_sink_min_bpp, dsc_min_bpp;
-	int dsc_src_max_bpp, dsc_sink_max_bpp, dsc_max_bpp;
+	int dsc_min_bpp;
+	int dsc_max_bpp;
 	int dsc_joiner_max_bpp;
 	int num_joined_pipes = intel_crtc_num_joined_pipes(pipe_config);
 
-	dsc_src_min_bpp = dsc_src_min_compressed_bpp();
-	dsc_sink_min_bpp = intel_dp_dsc_sink_min_compressed_bpp(pipe_config);
-	dsc_min_bpp = max(dsc_src_min_bpp, dsc_sink_min_bpp);
-	dsc_min_bpp = max(dsc_min_bpp, fxp_q4_to_int_roundup(limits->link.min_bpp_x16));
+	dsc_min_bpp = fxp_q4_to_int_roundup(limits->link.min_bpp_x16);
 
-	dsc_src_max_bpp = dsc_src_max_compressed_bpp(intel_dp);
-	dsc_sink_max_bpp = intel_dp_dsc_sink_max_compressed_bpp(connector,
-								pipe_config,
-								pipe_bpp / 3);
-	dsc_max_bpp = dsc_sink_max_bpp ? min(dsc_sink_max_bpp, dsc_src_max_bpp) : dsc_src_max_bpp;
+	dsc_max_bpp = intel_dp_dsc_sink_max_compressed_bpp(connector,
+							   pipe_config,
+							   pipe_bpp / 3);
 
 	dsc_joiner_max_bpp = get_max_compressed_bpp_with_joiner(i915, adjusted_mode->clock,
 								adjusted_mode->hdisplay,
@@ -2286,8 +2281,8 @@ static int intel_edp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 	struct intel_connector *connector =
 		to_intel_connector(conn_state->connector);
 	int pipe_bpp, forced_bpp;
-	int dsc_src_min_bpp, dsc_sink_min_bpp, dsc_min_bpp;
-	int dsc_src_max_bpp, dsc_sink_max_bpp, dsc_max_bpp;
+	int dsc_min_bpp;
+	int dsc_max_bpp;
 
 	forced_bpp = intel_dp_force_dsc_pipe_bpp(intel_dp, limits);
 
@@ -2307,16 +2302,12 @@ static int intel_edp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
 	pipe_config->port_clock = limits->max_rate;
 	pipe_config->lane_count = limits->max_lane_count;
 
-	dsc_src_min_bpp = dsc_src_min_compressed_bpp();
-	dsc_sink_min_bpp = intel_dp_dsc_sink_min_compressed_bpp(pipe_config);
-	dsc_min_bpp = max(dsc_src_min_bpp, dsc_sink_min_bpp);
-	dsc_min_bpp = max(dsc_min_bpp, fxp_q4_to_int_roundup(limits->link.min_bpp_x16));
+	dsc_min_bpp = fxp_q4_to_int_roundup(limits->link.min_bpp_x16);
+
+	dsc_max_bpp = intel_dp_dsc_sink_max_compressed_bpp(connector,
+							   pipe_config,
+							   pipe_bpp / 3);
 
-	dsc_src_max_bpp = dsc_src_max_compressed_bpp(intel_dp);
-	dsc_sink_max_bpp = intel_dp_dsc_sink_max_compressed_bpp(connector,
-								pipe_config,
-								pipe_bpp / 3);
-	dsc_max_bpp = dsc_sink_max_bpp ? min(dsc_sink_max_bpp, dsc_src_max_bpp) : dsc_src_max_bpp;
 	dsc_max_bpp = min(dsc_max_bpp, fxp_q4_to_int(limits->link.max_bpp_x16));
 
 	/* Compressed BPP should be less than the Input DSC bpp */
@@ -2455,6 +2446,7 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 /**
  * intel_dp_compute_config_link_bpp_limits - compute output link bpp limits
  * @intel_dp: intel DP
+ * @connector: intel connector
  * @crtc_state: crtc state
  * @dsc: DSC compression mode
  * @limits: link configuration limits
@@ -2466,6 +2458,7 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
  */
 bool
 intel_dp_compute_config_link_bpp_limits(struct intel_dp *intel_dp,
+					const struct intel_connector *connector,
 					const struct intel_crtc_state *crtc_state,
 					bool dsc,
 					struct link_config_limits *limits)
@@ -2488,12 +2481,22 @@ intel_dp_compute_config_link_bpp_limits(struct intel_dp *intel_dp,
 
 		limits->link.min_bpp_x16 = fxp_q4_from_int(limits->pipe.min_bpp);
 	} else {
-		/*
-		 * TODO: set the DSC link limits already here, atm these are
-		 * initialized only later in intel_edp_dsc_compute_pipe_bpp() /
-		 * intel_dp_dsc_compute_pipe_bpp()
-		 */
-		limits->link.min_bpp_x16 = 0;
+		int dsc_src_min_bpp, dsc_sink_min_bpp, dsc_min_bpp;
+		int dsc_src_max_bpp, dsc_sink_max_bpp, dsc_max_bpp;
+
+		dsc_src_min_bpp = dsc_src_min_compressed_bpp();
+		dsc_sink_min_bpp = intel_dp_dsc_sink_min_compressed_bpp(crtc_state);
+		dsc_min_bpp = max(dsc_src_min_bpp, dsc_sink_min_bpp);
+		limits->link.min_bpp_x16 = fxp_q4_from_int(dsc_min_bpp);
+
+		dsc_src_max_bpp = dsc_src_max_compressed_bpp(intel_dp);
+		dsc_sink_max_bpp = intel_dp_dsc_sink_max_compressed_bpp(connector,
+									crtc_state,
+									limits->pipe.max_bpp / 3);
+		dsc_max_bpp = dsc_sink_max_bpp ?
+			      min(dsc_sink_max_bpp, dsc_src_max_bpp) : dsc_src_max_bpp;
+
+		max_link_bpp_x16 = min(max_link_bpp_x16, fxp_q4_from_int(dsc_max_bpp));
 	}
 
 	limits->link.max_bpp_x16 = max_link_bpp_x16;
@@ -2566,6 +2569,7 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp,
 	intel_dp_test_compute_config(intel_dp, crtc_state, limits);
 
 	return intel_dp_compute_config_link_bpp_limits(intel_dp,
+						       intel_dp->attached_connector,
 						       crtc_state,
 						       dsc,
 						       limits);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index eac517c1d445..5981ab36fa8d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -197,6 +197,7 @@ int intel_dp_output_bpp(enum intel_output_format output_format, int bpp);
 
 bool
 intel_dp_compute_config_link_bpp_limits(struct intel_dp *intel_dp,
+					const struct intel_connector *connector,
 					const struct intel_crtc_state *crtc_state,
 					bool dsc,
 					struct link_config_limits *limits);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 78cb65f7cb2b..356c328aba72 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -576,6 +576,7 @@ mst_stream_compute_config_limits(struct intel_dp *intel_dp,
 		intel_dp_dsc_compute_pipe_bpp_limits(intel_dp, limits);
 
 	if (!intel_dp_compute_config_link_bpp_limits(intel_dp,
+						     connector,
 						     crtc_state,
 						     dsc,
 						     limits))
-- 
2.45.2

✓ Fi.CI.BAT: success for DP DSC min/max src bpc fixes (rev9)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 3750 bytes --]

== Series Details ==

Series: DP DSC min/max src bpc fixes (rev9)
URL   : https://patchwork.freedesktop.org/series/125571/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_15722 -> Patchwork_125571v9
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125571v9/index.html

Participating hosts (46 -> 45)
------------------------------

  Missing    (1): fi-snb-2520m 

Known issues
------------

  Here are the changes found in Patchwork_125571v9 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - bat-adls-6:         [PASS][1] -> [FAIL][2] ([i915#12903])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15722/bat-adls-6/igt@i915_pm_rpm@module-reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125571v9/bat-adls-6/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live:
    - bat-mtlp-6:         [PASS][3] -> [ABORT][4] ([i915#12829])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15722/bat-mtlp-6/igt@i915_selftest@live.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125571v9/bat-mtlp-6/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-mtlp-6:         [PASS][5] -> [ABORT][6] ([i915#12915])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15722/bat-mtlp-6/igt@i915_selftest@live@workarounds.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125571v9/bat-mtlp-6/igt@i915_selftest@live@workarounds.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - bat-dg1-7:          [FAIL][7] ([i915#12903]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15722/bat-dg1-7/igt@i915_pm_rpm@module-reload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125571v9/bat-dg1-7/igt@i915_pm_rpm@module-reload.html
    - bat-rpls-4:         [FAIL][9] ([i915#12903]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15722/bat-rpls-4/igt@i915_pm_rpm@module-reload.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125571v9/bat-rpls-4/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live:
    - bat-arlh-3:         [ABORT][11] ([i915#12829]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15722/bat-arlh-3/igt@i915_selftest@live.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125571v9/bat-arlh-3/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-arlh-3:         [ABORT][13] ([i915#12061]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15722/bat-arlh-3/igt@i915_selftest@live@workarounds.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125571v9/bat-arlh-3/igt@i915_selftest@live@workarounds.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#12829]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12829
  [i915#12903]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12903
  [i915#12915]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12915


Build changes
-------------

  * Linux: CI_DRM_15722 -> Patchwork_125571v9

  CI-20190529: 20190529
  CI_DRM_15722: ccf64319402f90f68549e2047a7f985a46436e41 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8118: 17707095f1e5d3c30f463b43022f01c0160579b6 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_125571v9: ccf64319402f90f68549e2047a7f985a46436e41 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_125571v9/index.html

[-- Attachment #2: Type: text/html, Size: 4718 bytes --]

Re: [PATCH 03/12] drm/i915/dp: Separate out helper for compute fec_enable

[Jani Nikula]

On Wed, 20 Nov 2024, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
> Make a separate function for setting fec_enable in crtc_state.
> Drop the check for FEC support as its already checked while checking for
> DSC support.

That's two changes that generally shouldn't be bundled together.

Aim for separating non-functional refactoring and functional
changes. (Well, arguably dropping the FEC check should also be
non-functional, but you know what I mean.)

> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 30 +++++++++++++++++--------
>  1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index dee15a05e7fd..d82e25d0dc5a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2352,6 +2352,26 @@ static int intel_edp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
>  	return 0;
>  }
>  
> +static void intel_dp_compute_fec_config(struct intel_dp *intel_dp,
> +					struct intel_crtc_state *pipe_config)

I think all encoder->callback_name hooks should be named
something_something_callback_name(), and the same goes for helpers
specifically aimed at this.

This would make the function intel_dp_fec_compute_config().

Yes, in many ways "compute fec config" reads better, but there's value
in being able to search for "_compute_config", and to know this is only
for he ->compute_config path.

BR,
Jani.


> +{
> +	if (pipe_config->fec_enable)
> +		return;
> +
> +	/*
> +	 * Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional.
> +	 * Since, FEC is a bandwidth overhead, continue to not enable it for
> +	 * eDP. Until, there is a good reason to do so.
> +	 */
> +	if (intel_dp_is_edp(intel_dp))
> +		return;
> +
> +	if (intel_dp_is_uhbr(pipe_config))
> +		return;
> +
> +	pipe_config->fec_enable = true;
> +}
> +
>  int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  				struct intel_crtc_state *pipe_config,
>  				struct drm_connector_state *conn_state,
> @@ -2368,15 +2388,7 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  	int num_joined_pipes = intel_crtc_num_joined_pipes(pipe_config);
>  	int ret;
>  
> -	/*
> -	 * Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional.
> -	 * Since, FEC is a bandwidth overhead, continue to not enable it for
> -	 * eDP. Until, there is a good reason to do so.
> -	 */
> -	pipe_config->fec_enable = pipe_config->fec_enable ||
> -		(!intel_dp_is_edp(intel_dp) &&
> -		 intel_dp_supports_fec(intel_dp, connector, pipe_config) &&
> -		 !intel_dp_is_uhbr(pipe_config));
> +	intel_dp_compute_fec_config(intel_dp, pipe_config);
>  
>  	if (!intel_dp_dsc_supports_format(connector, pipe_config->output_format))
>  		return -EINVAL;

-- 
Jani Nikula, Intel

Re: [PATCH 03/12] drm/i915/dp: Separate out helper for compute fec_enable

[Nautiyal, Ankit K]

On 11/20/2024 5:22 PM, Jani Nikula wrote:
> On Wed, 20 Nov 2024, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
>> Make a separate function for setting fec_enable in crtc_state.
>> Drop the check for FEC support as its already checked while checking for
>> DSC support.
> That's two changes that generally shouldn't be bundled together.
>
> Aim for separating non-functional refactoring and functional
> changes. (Well, arguably dropping the FEC check should also be
> non-functional, but you know what I mean.)

Moving to separate helper should indeed have non functional change and 
dropping the check can be another patch.

Initially I was going with a separate patch for dropping the FEC check, 
but couldn't make up my mind, and merged the two things. :)

Will do as suggested.


>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c | 30 +++++++++++++++++--------
>>   1 file changed, 21 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index dee15a05e7fd..d82e25d0dc5a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -2352,6 +2352,26 @@ static int intel_edp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
>>   	return 0;
>>   }
>>   
>> +static void intel_dp_compute_fec_config(struct intel_dp *intel_dp,
>> +					struct intel_crtc_state *pipe_config)
> I think all encoder->callback_name hooks should be named
> something_something_callback_name(), and the same goes for helpers
> specifically aimed at this.
>
> This would make the function intel_dp_fec_compute_config().
>
> Yes, in many ways "compute fec config" reads better, but there's value
> in being able to search for "_compute_config", and to know this is only
> for he ->compute_config path.

Makes sense to have _fec_compute_config. Will change this.

Thanks Jani, for the comments and suggestions.

Regards,

Ankit



>
> BR,
> Jani.
>
>
>> +{
>> +	if (pipe_config->fec_enable)
>> +		return;
>> +
>> +	/*
>> +	 * Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional.
>> +	 * Since, FEC is a bandwidth overhead, continue to not enable it for
>> +	 * eDP. Until, there is a good reason to do so.
>> +	 */
>> +	if (intel_dp_is_edp(intel_dp))
>> +		return;
>> +
>> +	if (intel_dp_is_uhbr(pipe_config))
>> +		return;
>> +
>> +	pipe_config->fec_enable = true;
>> +}
>> +
>>   int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>>   				struct intel_crtc_state *pipe_config,
>>   				struct drm_connector_state *conn_state,
>> @@ -2368,15 +2388,7 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>>   	int num_joined_pipes = intel_crtc_num_joined_pipes(pipe_config);
>>   	int ret;
>>   
>> -	/*
>> -	 * Though eDP v1.5 supports FEC with DSC, unlike DP, it is optional.
>> -	 * Since, FEC is a bandwidth overhead, continue to not enable it for
>> -	 * eDP. Until, there is a good reason to do so.
>> -	 */
>> -	pipe_config->fec_enable = pipe_config->fec_enable ||
>> -		(!intel_dp_is_edp(intel_dp) &&
>> -		 intel_dp_supports_fec(intel_dp, connector, pipe_config) &&
>> -		 !intel_dp_is_uhbr(pipe_config));
>> +	intel_dp_compute_fec_config(intel_dp, pipe_config);
>>   
>>   	if (!intel_dp_dsc_supports_format(connector, pipe_config->output_format))
>>   		return -EINVAL;

Re: [PATCH 03/12] drm/i915/dp: Separate out helper for compute fec_enable

[Jani Nikula]

On Wed, 20 Nov 2024, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
>>> +static void intel_dp_compute_fec_config(struct intel_dp *intel_dp,
>>> +					struct intel_crtc_state *pipe_config)

PS. For new stuff please go with crtc_state instead of pipe_config.


-- 
Jani Nikula, Intel

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

[Kandpal, Suraj]

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

Regards,
Suraj Kandpal

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

RE: [PATCH 04/12] drm/i915/dp: Remove HAS_DSC macro for intel_dp_dsc_max_src_input_bpc

[Kandpal, Suraj]

> -----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 04/12] drm/i915/dp: Remove HAS_DSC macro for
> intel_dp_dsc_max_src_input_bpc
> 
> DSC support is already checked before the helper
> intel_dp_dsc_max_src_input_bpc is called.
> Remove the check from the helper.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index d82e25d0dc5a..dd60ca532ae3 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2195,7 +2195,7 @@ static
>  u8 intel_dp_dsc_min_src_input_bpc(struct drm_i915_private *i915)  {
>  	/* Min DSC Input BPC for ICL+ is 8 */
> -	return HAS_DSC(i915) ? 8 : 0;
> +	return 8;

With this change we don't really need to pass drm_i915_private to this function how about we make it a void function
This also helps us move away from intel_display

Regards,
Suraj Kandpal

>  }
> 
>  static
> --
> 2.45.2

RE: [PATCH 09/12] drm/i915/dp_mst: Refactor pipe_bpp limits with dsc for mst

[Kandpal, Suraj]

> -----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 09/12] drm/i915/dp_mst: Refactor pipe_bpp limits with dsc
> for mst
> 
> Similar to DP, set the dsc limits->pipe.max/min_bpp early for MST too.
> Use the limits while computing the compressed bpp.
> 

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c     |  2 +-
>  drivers/gpu/drm/i915/display/intel_dp.h     |  3 +++
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 11 +++++------
>  3 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index b6d5e8eb75f5..827368b6cdb9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2512,7 +2512,7 @@ intel_dp_compute_config_link_bpp_limits(struct
> intel_dp *intel_dp,
>  	return true;
>  }
> 
> -static void
> +void
>  intel_dp_dsc_compute_pipe_bpp_limits(struct intel_dp *intel_dp,
>  				     struct link_config_limits *limits)  { diff --git
> a/drivers/gpu/drm/i915/display/intel_dp.h
> b/drivers/gpu/drm/i915/display/intel_dp.h
> index e5a25e5cbc25..973b2aa0da1d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -200,6 +200,9 @@ intel_dp_compute_config_link_bpp_limits(struct
> intel_dp *intel_dp,
>  					const struct intel_crtc_state
> *crtc_state,
>  					bool dsc,
>  					struct link_config_limits *limits);
> +void
> +intel_dp_dsc_compute_pipe_bpp_limits(struct intel_dp *intel_dp,
> +				     struct link_config_limits *limits);
> 
>  void intel_dp_get_dsc_sink_cap(u8 dpcd_rev, struct intel_connector
> *connector);  bool intel_dp_has_gamut_metadata_dip(struct intel_encoder
> *encoder); diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 9568924d143b..78cb65f7cb2b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -366,14 +366,10 @@ static int
> mst_stream_dsc_compute_link_config(struct intel_encoder *encoder,
>  	int i, num_bpc;
>  	u8 dsc_bpc[3] = {};
>  	int min_bpp, max_bpp, sink_min_bpp, sink_max_bpp;
> -	int dsc_max_bpc, dsc_min_bpc;
>  	int min_compressed_bpp, max_compressed_bpp;
> 
> -	dsc_max_bpc = intel_dp_dsc_max_src_input_bpc(display);
> -	dsc_min_bpc = intel_dp_dsc_min_src_input_bpc(display);
> -
> -	max_bpp = min(dsc_max_bpc * 3, limits->pipe.max_bpp);
> -	min_bpp = max(dsc_min_bpc * 3, limits->pipe.min_bpp);
> +	max_bpp = limits->pipe.max_bpp;
> +	min_bpp = limits->pipe.min_bpp;
> 
>  	num_bpc = drm_dp_dsc_sink_supported_input_bpcs(connector-
> >dp.dsc_dpcd,
>  						       dsc_bpc);
> @@ -576,6 +572,9 @@ mst_stream_compute_config_limits(struct intel_dp
> *intel_dp,
> 
>  	intel_dp_test_compute_config(intel_dp, crtc_state, limits);
> 
> +	if (dsc)
> +		intel_dp_dsc_compute_pipe_bpp_limits(intel_dp, limits);
> +
>  	if (!intel_dp_compute_config_link_bpp_limits(intel_dp,
>  						     crtc_state,
>  						     dsc,
> --
> 2.45.2

RE: [PATCH 11/12] drm/i915/dp: Make dsc helpers accept const crtc_state pointers

[Kandpal, Suraj]

> -----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 11/12] drm/i915/dp: Make dsc helpers accept const crtc_state
> pointers
> 
> Modify the dsc helpers to get max/min compressed bpp to accept `const
> struct intel_crtc_state *` pointers instead of `struct intel_crtc_state *`.
> 
> These helpers are not supposed to modify `crtc_state`.
> Accepting const pointers will allow these helpers to be called from functions
> that have const pointer to crtc_state.
> 

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 6 +++---
> drivers/gpu/drm/i915/display/intel_dp.h | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 6cd3ae2db1a9..1adde61c5b76 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1980,7 +1980,7 @@ static int dsc_compute_link_config(struct intel_dp
> *intel_dp,
> 
>  static
>  u16 intel_dp_dsc_max_sink_compressed_bppx16(const struct
> intel_connector *connector,
> -					    struct intel_crtc_state *pipe_config,
> +					    const struct intel_crtc_state
> *pipe_config,
>  					    int bpc)
>  {
>  	u16 max_bppx16 = drm_edp_dsc_sink_output_bpp(connector-
> >dp.dsc_dpcd);
> @@ -2005,7 +2005,7 @@ u16
> intel_dp_dsc_max_sink_compressed_bppx16(const struct intel_connector
> *connec
>  	return 0;
>  }
> 
> -int intel_dp_dsc_sink_min_compressed_bpp(struct intel_crtc_state
> *pipe_config)
> +int intel_dp_dsc_sink_min_compressed_bpp(const struct intel_crtc_state
> +*pipe_config)
>  {
>  	/* From Mandatory bit rate range Support Table 2-157 (DP v2.0) */
>  	switch (pipe_config->output_format) {
> @@ -2023,7 +2023,7 @@ int intel_dp_dsc_sink_min_compressed_bpp(struct
> intel_crtc_state *pipe_config)  }
> 
>  int intel_dp_dsc_sink_max_compressed_bpp(const struct intel_connector
> *connector,
> -					 struct intel_crtc_state *pipe_config,
> +					 const struct intel_crtc_state
> *pipe_config,
>  					 int bpc)
>  {
>  	return intel_dp_dsc_max_sink_compressed_bppx16(connector,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> b/drivers/gpu/drm/i915/display/intel_dp.h
> index 973b2aa0da1d..eac517c1d445 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -145,9 +145,9 @@ u16 intel_dp_dsc_get_max_compressed_bpp(struct
> drm_i915_private *i915,
>  					enum intel_output_format
> output_format,
>  					u32 pipe_bpp,
>  					u32 timeslots);
> -int intel_dp_dsc_sink_min_compressed_bpp(struct intel_crtc_state
> *pipe_config);
> +int intel_dp_dsc_sink_min_compressed_bpp(const struct intel_crtc_state
> +*pipe_config);
>  int intel_dp_dsc_sink_max_compressed_bpp(const struct intel_connector
> *connector,
> -					 struct intel_crtc_state *pipe_config,
> +					 const struct intel_crtc_state
> *pipe_config,
>  					 int bpc);
>  u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
>  				int mode_clock, int mode_hdisplay,
> --
> 2.45.2

Re: [PATCH 04/12] drm/i915/dp: Remove HAS_DSC macro for intel_dp_dsc_max_src_input_bpc

[Nautiyal, Ankit K]

On 11/27/2024 11:15 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 04/12] drm/i915/dp: Remove HAS_DSC macro for
>> intel_dp_dsc_max_src_input_bpc
>>
>> DSC support is already checked before the helper
>> intel_dp_dsc_max_src_input_bpc is called.
>> Remove the check from the helper.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> b/drivers/gpu/drm/i915/display/intel_dp.c
>> index d82e25d0dc5a..dd60ca532ae3 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -2195,7 +2195,7 @@ static
>>   u8 intel_dp_dsc_min_src_input_bpc(struct drm_i915_private *i915)  {
>>   	/* Min DSC Input BPC for ICL+ is 8 */
>> -	return HAS_DSC(i915) ? 8 : 0;
>> +	return 8;
> With this change we don't really need to pass drm_i915_private to this function how about we make it a void function
> This also helps us move away from intel_display

Yeah I agree we dont need to pass drm_i915_private. Will fix this.

Regards,

Ankit

>
> Regards,
> Suraj Kandpal
>
>>   }
>>
>>   static
>> --
>> 2.45.2

Re: [PATCH 01/12] drm/i915/dp: Refactor FEC support check in intel_dp_supports_dsc

[Jani Nikula]

On Wed, 20 Nov 2024, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
> Forward Error Correction is required for DP if we are using DSC but
> is optional for eDP.
>
> Currently the helper intel_dp_supports_dsc checks if fec_enable is set for
> DP or not. The helper is called after fec_enable is set in crtc_state.
>
> Instead of this a better approach would be to:
> first, call intel_dp_supports_dsc to check for DSC support
> (along with FEC requirement for DP) and then set fec_enable for DP
> (if not already set) in crtc_state.
>
> To achieve this, remove the check for fec_enable in the helper and instead
> check for FEC support for DP. With this change the helper
> intel_dp_supports_dsc can be called earlier and return early if DSC is
> not supported. The structure intel_dp is added to the helper to get the
> FEC support for DP.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c     | 8 +++++---
>  drivers/gpu/drm/i915/display/intel_dp.h     | 3 ++-
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 +++--
>  3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 053a9a4182e7..db9ddbcdd159 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1632,13 +1632,15 @@ bool intel_dp_supports_fec(struct intel_dp *intel_dp,
>  		drm_dp_sink_supports_fec(connector->dp.fec_capability);
>  }
>  
> -bool intel_dp_supports_dsc(const struct intel_connector *connector,
> +bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
> +			   const struct intel_connector *connector,
>  			   const struct intel_crtc_state *crtc_state)
>  {
>  	if (!intel_dp_has_dsc(connector))
>  		return false;
>  
> -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP) && !crtc_state->fec_enable)
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP) &&
> +	    !intel_dp_supports_fec(intel_dp, connector, crtc_state))
>  		return false;
>  
>  	return intel_dsc_source_support(crtc_state);
> @@ -2376,7 +2378,7 @@ 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(connector, 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))
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 48f10876be65..4ae54e9718ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -170,7 +170,8 @@ bool intel_dp_supports_fec(struct intel_dp *intel_dp,
>  			   const struct intel_connector *connector,
>  			   const struct intel_crtc_state *pipe_config);
>  
> -bool intel_dp_supports_dsc(const struct intel_connector *connector,
> +bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
> +			   const struct intel_connector *connector,
>  			   const struct intel_crtc_state *crtc_state);
>  
>  u32 intel_dp_dsc_nearest_valid_bpp(struct drm_i915_private *i915, u32 bpp, u32 pipe_bpp);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index f058360a2641..0662736849ac 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -498,12 +498,13 @@ adjust_limits_for_dsc_hblank_expansion_quirk(const struct intel_connector *conne
>  	struct intel_display *display = to_intel_display(connector);
>  	const struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	int min_bpp_x16 = limits->link.min_bpp_x16;
> +	struct intel_dp *intel_dp = connector->mst_port;

Maybe better to pass in intel_dp here too for clarity. The caller has it
from to_primary_dp().

BR,
Jani.

>  
>  	if (!hblank_expansion_quirk_needs_dsc(connector, crtc_state, limits))
>  		return true;
>  
>  	if (!dsc) {
> -		if (intel_dp_supports_dsc(connector, crtc_state)) {
> +		if (intel_dp_supports_dsc(intel_dp, connector, crtc_state)) {
>  			drm_dbg_kms(display->drm,
>  				    "[CRTC:%d:%s][CONNECTOR:%d:%s] DSC needed by hblank expansion quirk\n",
>  				    crtc->base.base.id, crtc->base.name,
> @@ -648,7 +649,7 @@ static int mst_stream_compute_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(connector, pipe_config))
> +		if (!intel_dp_supports_dsc(intel_dp, connector, pipe_config))
>  			return -EINVAL;
>  
>  		if (!mst_stream_compute_config_limits(intel_dp, connector,

-- 
Jani Nikula, Intel

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

[Nautiyal, Ankit K]

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

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

[Kandpal, Suraj]

> -----Original Message-----
> From: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Sent: Tuesday, December 3, 2024 2:02 PM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: intel-xe@lists.freedesktop.org; jani.nikula@linux.intel.com; Deak, Imre
> <imre.deak@intel.com>
> Subject: Re: [PATCH 02/12] drm/i915/dp: Return early if DSC not supported
> 
> 
> 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.
> 
> 

Okay then it LGTM
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

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