[v2] drm/i915: Check require bandwidth did not exceed LSPCON limitation
diff mbox series

Message ID 20200121132621.22194-1-shawn.c.lee@intel.com
State New
Headers show
Series
  • [v2] drm/i915: Check require bandwidth did not exceed LSPCON limitation
Related show

Commit Message

Lee Shawn C Jan. 21, 2020, 1:26 p.m. UTC
While mode setting, driver would calculate mode rate based on
resolution and bpp. And choose the best bpp that did not exceed
DP bandwidtd.

But LSPCON had more restriction due to it convert DP to HDMI.
Driver should respect HDMI's bandwidth limitation if LSPCON
was active. This change would ignore the bpp when its required
output bandwidth already over HDMI 2.0 or 1.4 spec.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Sam McNally <sammc@google.com>
Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>

v2: move lspcon_max_rate() into intel_lspcon.c.
---
 drivers/gpu/drm/i915/display/intel_dp.c     |  5 +++++
 drivers/gpu/drm/i915/display/intel_lspcon.c | 10 ++++++++++
 drivers/gpu/drm/i915/display/intel_lspcon.h |  1 +
 3 files changed, 16 insertions(+)

Comments

Sharma, Shashank Jan. 21, 2020, 6:14 a.m. UTC | #1
[AMD Official Use Only - Internal Distribution Only]

Hello Shawn, 

-----Original Message-----
From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Lee Shawn C
Sent: Tuesday, January 21, 2020 6:56 PM
To: intel-gfx@lists.freedesktop.org
Cc: Cooper Chiou <cooper.chiou@intel.com>; Sam McNally <sammc@google.com>
Subject: [Intel-gfx] [PATCH v2] drm/i915: Check require bandwidth did not exceed LSPCON limitation

While mode setting, driver would calculate mode rate based on resolution and bpp. And choose the best bpp that did not exceed DP bandwidtd.

But LSPCON had more restriction due to it convert DP to HDMI.
Driver should respect HDMI's bandwidth limitation if LSPCON was active. This change would ignore the bpp when its required output bandwidth already over HDMI 2.0 or 1.4 spec.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Sam McNally <sammc@google.com>
Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>

v2: move lspcon_max_rate() into intel_lspcon.c.
---
 drivers/gpu/drm/i915/display/intel_dp.c     |  5 +++++
 drivers/gpu/drm/i915/display/intel_lspcon.c | 10 ++++++++++  drivers/gpu/drm/i915/display/intel_lspcon.h |  1 +
 3 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index c7424e2a04a3..6796055ace69 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1983,6 +1983,7 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 				  const struct link_config_limits *limits)  {
 	struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
+	struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
 	int bpp, clock, lane_count;
 	int mode_rate, link_clock, link_avail;
 
@@ -1992,6 +1993,10 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
 						   output_bpp);
 
+		/* Bypass this mode if require bandwidth over HDMI spec when LSPCON active */
+		if (lspcon->active && mode_rate > lspcon_max_rate(lspcon))
+			continue;
+
 		for (clock = limits->min_clock; clock <= limits->max_clock; clock++) {
 			for (lane_count = limits->min_lane_count;
 			     lane_count <= limits->max_lane_count; diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
index d807c5648c87..5657e949aabf 100644
--- a/drivers/gpu/drm/i915/display/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
@@ -518,6 +518,16 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
 				  buf, ret);
 }
 
+int lspcon_max_rate(struct intel_lspcon *lspcon) {
+	enum drm_lspcon_mode current_mode = lspcon_get_current_mode(lspcon);
+
+	if (lspcon_current_mode == DRM_LSPCON_MODE_LS)
+		return DIV_ROUND_UP(340000 * 24, 8);
+
+	return DIV_ROUND_UP(600000 * 24, 8);
Instead of assuming max clock 340 or 600, my suggestion would be to introduce a new variable in struct intel_lspcon called max_rate, and then initialize that properly at lspcon_probe/init, this will make the solution generic for any LSPCON type dongle on Intel board. We have seen few dp->hdmi type2 convertors also on-board on ICL chips, which behave like LSPCON, but can be HDMI 1.4 or HDMI 2.0 depending on the board config.   

- Shashank 
+}
+
 u32 lspcon_infoframes_enabled(struct intel_encoder *encoder,
 			      const struct intel_crtc_state *pipe_config)  { diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.h b/drivers/gpu/drm/i915/display/intel_lspcon.h
index 37cfddf8a9c5..b584c02ab33b 100644
--- a/drivers/gpu/drm/i915/display/intel_lspcon.h
+++ b/drivers/gpu/drm/i915/display/intel_lspcon.h
@@ -18,6 +18,7 @@ struct intel_lspcon;
 bool lspcon_init(struct intel_digital_port *intel_dig_port);  void lspcon_resume(struct intel_lspcon *lspcon);  void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
+int lspcon_max_rate(struct intel_lspcon *lspcon);
 void lspcon_write_infoframe(struct intel_encoder *encoder,
 			    const struct intel_crtc_state *crtc_state,
 			    unsigned int type,
--
2.17.1
Sharma, Shashank Jan. 21, 2020, 6:16 a.m. UTC | #2
[AMD Official Use Only - Internal Distribution Only]



-----Original Message-----
From: Sharma, Shashank 
Sent: Tuesday, January 21, 2020 11:44 AM
To: Lee Shawn C <shawn.c.lee@intel.com>; intel-gfx@lists.freedesktop.org
Cc: Cooper Chiou <cooper.chiou@intel.com>; Sam McNally <sammc@google.com>
Subject: RE: [Intel-gfx] [PATCH v2] drm/i915: Check require bandwidth did not exceed LSPCON limitation

[AMD Official Use Only - Internal Distribution Only]

Hello Shawn, 

-----Original Message-----
From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Lee Shawn C
Sent: Tuesday, January 21, 2020 6:56 PM
To: intel-gfx@lists.freedesktop.org
Cc: Cooper Chiou <cooper.chiou@intel.com>; Sam McNally <sammc@google.com>
Subject: [Intel-gfx] [PATCH v2] drm/i915: Check require bandwidth did not exceed LSPCON limitation

While mode setting, driver would calculate mode rate based on resolution and bpp. And choose the best bpp that did not exceed DP bandwidtd.

But LSPCON had more restriction due to it convert DP to HDMI.
Driver should respect HDMI's bandwidth limitation if LSPCON was active. This change would ignore the bpp when its required output bandwidth already over HDMI 2.0 or 1.4 spec.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Sam McNally <sammc@google.com>
Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>

v2: move lspcon_max_rate() into intel_lspcon.c.
---
 drivers/gpu/drm/i915/display/intel_dp.c     |  5 +++++
 drivers/gpu/drm/i915/display/intel_lspcon.c | 10 ++++++++++  drivers/gpu/drm/i915/display/intel_lspcon.h |  1 +
 3 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index c7424e2a04a3..6796055ace69 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1983,6 +1983,7 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 				  const struct link_config_limits *limits)  {
 	struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
+	struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
 	int bpp, clock, lane_count;
 	int mode_rate, link_clock, link_avail;
 
@@ -1992,6 +1993,10 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
 						   output_bpp);
 
+		/* Bypass this mode if require bandwidth over HDMI spec when LSPCON active */
+		if (lspcon->active && mode_rate > lspcon_max_rate(lspcon))
+			continue;
+
 		for (clock = limits->min_clock; clock <= limits->max_clock; clock++) {
 			for (lane_count = limits->min_lane_count;
 			     lane_count <= limits->max_lane_count; diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
index d807c5648c87..5657e949aabf 100644
--- a/drivers/gpu/drm/i915/display/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
@@ -518,6 +518,16 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
 				  buf, ret);
 }
 
+int lspcon_max_rate(struct intel_lspcon *lspcon) {
+	enum drm_lspcon_mode current_mode = lspcon_get_current_mode(lspcon);
+
+	if (lspcon_current_mode == DRM_LSPCON_MODE_LS)
Also, if I recall correctly, we are driving always on PCON mode. So if LSPCON is active, it would be in PCON mode. Do you see scenarios where you are finding it in LS mode ? 

- Shashank
+		return DIV_ROUND_UP(340000 * 24, 8);
+
+	return DIV_ROUND_UP(600000 * 24, 8);
Instead of assuming max clock 340 or 600, my suggestion would be to introduce a new variable in struct intel_lspcon called max_rate, and then initialize that properly at lspcon_probe/init, this will make the solution generic for any LSPCON type dongle on Intel board. We have seen few dp->hdmi type2 convertors also on-board on ICL chips, which behave like LSPCON, but can be HDMI 1.4 or HDMI 2.0 depending on the board config.   

- Shashank 
+}
+
 u32 lspcon_infoframes_enabled(struct intel_encoder *encoder,
 			      const struct intel_crtc_state *pipe_config)  { diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.h b/drivers/gpu/drm/i915/display/intel_lspcon.h
index 37cfddf8a9c5..b584c02ab33b 100644
--- a/drivers/gpu/drm/i915/display/intel_lspcon.h
+++ b/drivers/gpu/drm/i915/display/intel_lspcon.h
@@ -18,6 +18,7 @@ struct intel_lspcon;
 bool lspcon_init(struct intel_digital_port *intel_dig_port);  void lspcon_resume(struct intel_lspcon *lspcon);  void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
+int lspcon_max_rate(struct intel_lspcon *lspcon);
 void lspcon_write_infoframe(struct intel_encoder *encoder,
 			    const struct intel_crtc_state *crtc_state,
 			    unsigned int type,
--
2.17.1

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index c7424e2a04a3..6796055ace69 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1983,6 +1983,7 @@  intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 				  const struct link_config_limits *limits)
 {
 	struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
+	struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
 	int bpp, clock, lane_count;
 	int mode_rate, link_clock, link_avail;
 
@@ -1992,6 +1993,10 @@  intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
 						   output_bpp);
 
+		/* Bypass this mode if require bandwidth over HDMI spec when LSPCON active */
+		if (lspcon->active && mode_rate > lspcon_max_rate(lspcon))
+			continue;
+
 		for (clock = limits->min_clock; clock <= limits->max_clock; clock++) {
 			for (lane_count = limits->min_lane_count;
 			     lane_count <= limits->max_lane_count;
diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c b/drivers/gpu/drm/i915/display/intel_lspcon.c
index d807c5648c87..5657e949aabf 100644
--- a/drivers/gpu/drm/i915/display/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
@@ -518,6 +518,16 @@  void lspcon_set_infoframes(struct intel_encoder *encoder,
 				  buf, ret);
 }
 
+int lspcon_max_rate(struct intel_lspcon *lspcon)
+{
+	enum drm_lspcon_mode current_mode = lspcon_get_current_mode(lspcon);
+
+	if (lspcon_current_mode == DRM_LSPCON_MODE_LS)
+		return DIV_ROUND_UP(340000 * 24, 8);
+
+	return DIV_ROUND_UP(600000 * 24, 8);
+}
+
 u32 lspcon_infoframes_enabled(struct intel_encoder *encoder,
 			      const struct intel_crtc_state *pipe_config)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.h b/drivers/gpu/drm/i915/display/intel_lspcon.h
index 37cfddf8a9c5..b584c02ab33b 100644
--- a/drivers/gpu/drm/i915/display/intel_lspcon.h
+++ b/drivers/gpu/drm/i915/display/intel_lspcon.h
@@ -18,6 +18,7 @@  struct intel_lspcon;
 bool lspcon_init(struct intel_digital_port *intel_dig_port);
 void lspcon_resume(struct intel_lspcon *lspcon);
 void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
+int lspcon_max_rate(struct intel_lspcon *lspcon);
 void lspcon_write_infoframe(struct intel_encoder *encoder,
 			    const struct intel_crtc_state *crtc_state,
 			    unsigned int type,