On Mon, 27 Mar 2017, Clint Taylor <[email protected]> wrote: > On 03/27/2017 08:22 AM, Jani Nikula wrote: >> On Mon, 27 Mar 2017, Daniel Vetter <[email protected]> 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 <[email protected]>. This is just an iteration to reduce >>>> the parameters regardless of lane count, link rate, or sink. >>>> >>>> Reference: >>>> http://patchwork.freedesktop.org/patch/msgid/[email protected] >>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93578 >>>> Tested-by: Mads <[email protected]> >>>> Tested-by: PJ <[email protected]> >>>> Tested-by: François Guerraz <[email protected]> >>>> Tested-by: Lev Popov <[email protected]> >>>> Tested-by: Igor Krivenko <[email protected]> >>>> Cc: Clint Taylor <[email protected]> >>>> Cc: Anusha Srivatsa <[email protected]> >>>> Cc: Ville Syrjälä <[email protected]> >>>> Cc: Daniel Vetter <[email protected]> >>>> Signed-off-by: Jani Nikula <[email protected]> >>>> >>>> --- >>>> >>>> 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 <[email protected]> > Reviewed-by: Clint Taylor <[email protected]>
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 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
