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

[Jani Nikula <jani.nikula@intel.com>]

On Thu, 23 Mar 2017, Clint Taylor <clinton.a.taylor@intel.com> wrote:
> I would prefer a solution for B (rules for M/N), but the code doesn't 
> appear to be broken and I don't believe we should "Fix" something that 
> is working. The device also works by changing the roundup_pow_of_two() 
> to rounddown_pow_of_two() however that would apply the change to every 
> device connected.

Looking at this again, the problem must be in the (external) link M/N,
not (internal) data M/N. Thus it's only about the pixel clock / link
clock ratio. Right?

With current code link N exceeds 0x80000 only when link clock >= 540000
kHz. Except for the eDP intermediate link clocks, at least the four
least significant bits are always zero. But just one bit shift right
would be enough to bring even the DP 1.4 810000 kHz link clock under
0x80000 link N. The pixel clock for modes that require a link clock >=
540000 kHz would also have several least significant bits zero.

Unless the user provides a mode with an odd pixel clock value, we can
reduce the numbers to reach the goal, with no loss in precision, and it
doesn't even feel like a hack. The DP spec even mentions sources making
choices that "allow for static and relatively small Mvid and Nvid
values".

Of course, all of this hinges on the problem being specific to the link
M/N, and independent of data M/N.

BR,
Jani.


diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9a28a8917dc1..55bb6cf2a2d3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6337,6 +6337,15 @@ intel_reduce_m_n_ratio(uint32_t *num, uint32_t *den)
 static void compute_m_n(unsigned int m, unsigned int n,
 			uint32_t *ret_m, uint32_t *ret_n)
 {
+	/*
+	 * Reduce M/N as much as possible without loss in precision. Several DP
+	 * dongles in particular seem to be fussy about too large M/N values.
+	 */
+	while ((m & 1) == 0 && (n & 1) == 0) {
+		m >>= 1;
+		n >>= 1;
+	}
+
 	*ret_n = min_t(unsigned int, roundup_pow_of_two(n), DATA_LINK_N_MAX);
 	*ret_m = div_u64((uint64_t) m * *ret_n, n);
 	intel_reduce_m_n_ratio(ret_m, ret_n);
---

And the hack on top that ensures we're below 0x80000 link N independent
of the pixel clock. Note that the only loss in precision here is the one
bit in pixel clock; the other values passed in will always be even.

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 55bb6cf2a2d3..b51b836b9538 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6341,6 +6341,8 @@ static void compute_m_n(unsigned int m, unsigned int n,
 	 * Reduce M/N as much as possible without loss in precision. Several DP
 	 * dongles in particular seem to be fussy about too large M/N values.
 	 */
+	m >>= 1;
+	n >>= 1;
 	while ((m & 1) == 0 && (n & 1) == 0) {
 		m >>= 1;
 		n >>= 1;



-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx