On Mon, 27 Mar 2017, Daniel Vetter <daniel@xxxxxxxx> 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@xxxxxxxxx>. 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@xxxxxxxxx >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93578 >> Tested-by: Mads <mads@xxxxxx> >> Tested-by: PJ <foobar@xxxxxxxxxxx> >> Tested-by: François Guerraz <kubrick@xxxxxxxx> >> Tested-by: Lev Popov <leo@xxxxxxxxx> >> Tested-by: Igor Krivenko <igor.s.krivenko@xxxxxxxxx> >> Cc: Clint Taylor <clinton.a.taylor@xxxxxxxxx> >> Cc: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> >> --- >> >> 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. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
