[Intel-gfx] [PATCH] drm/i915/dp: reduce link M/N parameters

[Jani Nikula]

On Mon, 27 Mar 2017, Clint Taylor <clinton.a.taylor at intel.com> wrote:
> On 03/27/2017 08:22 AM, Jani Nikula wrote:
>> On Mon, 27 Mar 2017, Daniel Vetter <daniel at ffwll.ch> wrote:
>>> On Mon, Mar 27, 2017 at 02:33:25PM +0300, Jani Nikula wrote:
>>>> Several major vendor USB-C->HDMI converters, in particular the DA200,
>>>> fail to recover a 5.4 GHz 1 lane signal if the link N is greater than
>>>> 0x80000.
>>>>
>>>> The link M and N depend on the pixel clock and link clock ratio. 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. 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.
>>>>
>>>> The DP spec even mentions sources making choices that "allow for static
>>>> and relatively small Mvid and Nvid values", thus reducing the link M/N
>>>> regardless of the sink in question seems justified.
>>>>
>>>> Everything here is based on the work and information gathered by Clint
>>>> Taylor <clinton.a.taylor at intel.com>. This is just an iteration to reduce
>>>> the parameters regardless of lane count, link rate, or sink.
>>>>
>>>> Reference: http://patchwork.freedesktop.org/patch/msgid/1490225256-11667-1-git-send-email-clinton.a.taylor@intel.com
>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93578
>>>> Tested-by: Mads <mads at ab3.no>
>>>> Tested-by: PJ <foobar at pjmodos.net>
>>>> Tested-by: François Guerraz <kubrick at fgv6.net>
>>>> Tested-by: Lev Popov <leo at nabam.net>
>>>> Tested-by: Igor Krivenko <igor.s.krivenko at gmail.com>
>>>> Cc: Clint Taylor <clinton.a.taylor at intel.com>
>>>> Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
>>>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>>>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>>>>
>>>> ---
>>>>
>>>> This is cc: stable material, but due to the slight risk of regressions
>>>> (there's always the risk, however small, when you change parameters that
>>>> affect all sinks) I'd prefer letting this simmer for a while, and asking
>>>> for an explicit stable backport afterwards.
>>>> ---
>>>>  drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
>>>>  1 file changed, 9 insertions(+)
>>>>
>>>> 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);
>>>
>>> Can't we push this into reduce_m_n_ratio?
>>
>> After *ret_m = div_u64((uint64_t) m * *ret_n, n); it's not guaranteed
>> that we could shift at all. The reduction needs to be done on the
>> original m, n being passed in.
>>
>
> Tested-by: Clint Taylor <clinton.a.taylor at intel.com>
> Reviewed-by: Clint Taylor <clinton.a.taylor at intel.com>

Pushed to drm-intel-next-queued, thanks Clint for your work in getting
at the roots of the problem, and everyone for all the review and
testing!

BR,
Jani.


>
>> BR,
>> Jani.
>>
>>
>

-- 
Jani Nikula, Intel Open Source Technology Center