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

[Clint Taylor]

On 03/24/2017 04:25 AM, Jani Nikula wrote:
> On Thu, 23 Mar 2017, Clint Taylor <clinton.a.taylor at 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?
>

Correct, Pixel/Link clock ratio only.

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

As long as the precision is maintained a lower value should be fine.
>
> 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;
>
>
>

The above code at 1 lane @ 5.4 Ghz 1080P at 60 (148.400Mhz pixel clock) 
makes GMCH Data N,M = 0x40000, 0x34CCC and Link N,M = 0x40000, 0x11999. 
The offending dongle appears to work fine with the new values as 
expected. I saw Nvid values of 0x10000 at 2048x1152 and the dongle still 
worked.

-Clint