Re: [PATCH] drm/bridge/synopsys: dw-hdmi: Handle audio for more clock rates

[Doug Anderson]

Hi,

On Wed, Jun 19, 2019 at 8:23 AM Neil Armstrong <narmstrong@xxxxxxxxxxxx> wrote:
>
> On 18/06/2019 19:23, Jernej Åkrabec wrote:
> > Hi!
> >
> > Dne torek, 18. junij 2019 ob 01:55:58 CEST je Douglas Anderson napisal(a):
> >> Let's add some better support for HDMI audio to dw_hdmi.
> >> Specifically:
> >>
> >> 1. For 44.1 kHz audio the old code made the assumption that an N of
> >> 6272 was right most of the time.  That wasn't true and the new table
> >> should give better 44.1 kHz audio for many more rates.
> >>
> >> 2. The new table has values from the HDMI spec for 297 MHz and 594
> >> MHz.
> >>
> >> 3. There is now code to try to come up with a more idea N/CTS for
> >> clock rates that aren't in the table.  This code is a bit slow because
> >> it iterates over every possible value of N and picks the best one, but
> >> it should make a good fallback.
> >>
> >> 4. The new code allows for platforms that know they make a clock rate
> >> slightly differently to pick different N/CTS values.  For instance on
> >> rk3288 we can make 25,176,471 Hz instead of 25,174,825.1748... Hz
> >> (25.2 MHz / 1.001).  A future patch to the rk3288 platform code could
> >> enable support for this clock rate and specify the N/CTS that would be
> >> ideal.
> >>
> >> NOTE: the oddest part of this patch comes about because computing the
> >> ideal N/CTS means knowing the _exact_ clock rate, not a rounded
> >> version of it.  The drm framework makes this harder by rounding rates
> >> to kHz, but even if it didn't there might be cases where the ideal
> >> rate could only be calculated if we knew the real (non-integral) rate.
> >> This means that in cases where we know (or believe) that the true rate
> >> is something other than the rate we are told by drm.
> >>
> >> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> >
> > Which bus is used for audio transfer on your device? If it is I2S, which is
> > commonly used, then please be aware of this patch:
> > https://lists.freedesktop.org/archives/dri-devel/2019-June/221539.html
> >
> > It avoids exact N/CTS calculation by enabling auto detection. It is well
> > tested on multiple SoCs from Allwinner, Amlogic and Rockchip.
> >
> > Best regards,
> > Jernej
> >
> >
> Hi Douglas,
>
> Thanks for your work !
>
> If you could rebase on top of https://lists.freedesktop.org/archives/dri-devel/2019-June/221539.html
> so we can make use of your extended N table with automatic CTS HW calculation, it would be great !

Thanks to you and Jernej for pointing me at this.  It seems likely
that patch by itself would solve problems we found and I'll pick that
into my tree.

Probably my patch is no longer quite as useful atop yours, but I'll
still post a v2 since (in theory) folks that aren't using I2S might
find it useful.  I guess it's also possible (?) that picking an N
where CTS would be able to be integral has some type of advantage,
even with auto CTS?


> Finally could you add the plat_data tmds table as a separate patch to simplify review ?

Sure.  I'm probably not going to be able to post the patch to actually
use it, so I guess we could just not bother applying the 2nd patch
unless someone ever needs it.

-Doug