Re: [PATCH] drm/i915: Reduce Data Link N value for 1 lane DP->hdmi converters

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/23/2017 05:30 AM, Jani Nikula wrote:
On Thu, 23 Mar 2017, clinton.a.taylor@xxxxxxxxx wrote:
From: Clint Taylor <clinton.a.taylor@xxxxxxxxx>

Several major vendor USB-C->HDMI converters fail to recover a 5.4 GHz 1 lane
signal if the Data Link N is greater than 0x80000.
Patch detects when 1 lane 5.4 GHz signal is being used and makes the maximum
value 20 bit instead of the maximum specification supported 24 bit value.

Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
Cc: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>


Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93578

I will add to the commit message.


Signed-off-by: Clint Taylor <clinton.a.taylor@xxxxxxxxx>
---
 drivers/gpu/drm/i915/i915_reg.h      |    2 ++
 drivers/gpu/drm/i915/intel_display.c |   15 +++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 04c8f69..838d8d5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4869,6 +4869,8 @@ enum {

 #define  DATA_LINK_M_N_MASK	(0xffffff)
 #define  DATA_LINK_N_MAX	(0x800000)
+/* Maximum N value useable on some DP->HDMI converters */
+#define  DATA_LINK_REDUCED_N_MAX (0x80000)

 #define _PIPEA_DATA_N_G4X	0x70054
 #define _PIPEB_DATA_N_G4X	0x71054
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 010e5dd..6e1fdd2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6315,9 +6315,10 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 }

 static void compute_m_n(unsigned int m, unsigned int n,
-			uint32_t *ret_m, uint32_t *ret_n)
+			uint32_t *ret_m, uint32_t *ret_n,
+			uint32_t max_link_n)
 {
-	*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
+	*ret_n = min_t(unsigned int, roundup_pow_of_two(n), max_link_n);

If there's evidence suggesting "certain other operating systems" always
use a max (or fixed value) of 0x80000, perhaps we should just follow
suit? Simpler and less magical.


The other OS's don't appear to be fixed to 0x80000. The calculation in i915 rounds up to the nearest power of 2 and the other OS's might have a slightly different calculation to the nearest power of 2. Of course I haven't seen the other OS's code to know their exact formula. HBR3 will cause a higher value to be calculated and having a fixed value may cause issues. The i915 formula works and reducing the value can cause precision issues in the ratio with the pixel clock.

 	*ret_m = div_u64((uint64_t) m * *ret_n, n);
 	intel_reduce_m_n_ratio(ret_m, ret_n);
 }
@@ -6327,14 +6328,20 @@ static void compute_m_n(unsigned int m, unsigned int n,
 		       int pixel_clock, int link_clock,
 		       struct intel_link_m_n *m_n)
 {
+	uint32_t max_link_n = DATA_LINK_N_MAX;
 	m_n->tu = 64;

+	if ((nlanes==1) && (link_clock >= 540000))

Is the problem really dependent on these conditions? You can get the
same problematic N value with nlanes == 2 && link_clock == 270000 too.


The offending device only supports a single DP lane up to HBR2.5. This check matches the datasheet for the part. The offending device works with our current calculation at 1 lane HBR (270000).

BR,
Jani.

+		max_link_n = DATA_LINK_REDUCED_N_MAX;
+
 	compute_m_n(bits_per_pixel * pixel_clock,
 		    link_clock * nlanes * 8,
-		    &m_n->gmch_m, &m_n->gmch_n);
+		    &m_n->gmch_m, &m_n->gmch_n,
+		    max_link_n);

 	compute_m_n(pixel_clock, link_clock,
-		    &m_n->link_m, &m_n->link_n);
+		    &m_n->link_m, &m_n->link_n,
+		    max_link_n);
 }

 static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux