Date:   Wed, 26 Jun 2019 12:00:22 +0200
From:   Andrzej Hajda <a.hajda@...sung.com>
To:     Doug Anderson <dianders@...omium.org>
Cc:     Jernej Skrabec <jernej.skrabec@...l.net>,
        Jonas Karlman <jonas@...boo.se>,
        Maxime Ripard <maxime.ripard@...tlin.com>,
        David Airlie <airlied@...ux.ie>,
        Neil Armstrong <narmstrong@...libre.com>,
        LKML <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        Sean Paul <seanpaul@...omium.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Dylan Reid <dgreid@...omium.org>,
        Jerome Brunet <jbrunet@...libre.com>,
        Sam Ravnborg <sam@...nborg.org>,
        Cheng-Yi Chiang <cychiang@...omium.org>
Subject: Re: [PATCH v2 1/2] drm/bridge/synopsys: dw-hdmi: Handle audio for
 more clock rates

On 26.06.2019 11:56, Andrzej Hajda wrote:
> On 25.06.2019 18:26, Doug Anderson wrote:
>> Hi,
>>
>>
>> On Tue, Jun 25, 2019 at 9:07 AM Andrzej Hajda <a.hajda@...sung.com> wrote:
>>> On 19.06.2019 23:07, Douglas Anderson wrote:
>>>> 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 pick a more ideal value.
>>> Why? I ask because it is against recommendation from HDMI specs.
>> The place where it does matter (and why I originally did this work) is
>> when you don't have auto-CTS.  In such a case you really need "N" and
>> "CTS" to make the math work and both be integral.  This makes sure
>> that you don't slowly accumulate offsets.  I'm hoping that this point
>> should be non-controversial so I won't argue it more.
>>
>> I am an admitted non-expert, but I have a feeling that with Auto-CTS
>> either the old number or the new numbers would produce pretty much the
>> same experience.
>
> Because Auto-CTS mechanism will alternate between two or more CTS values
> every frame, thus it will compensate non-rational clock relationship.
>
>
>>   AKA: anyone using auto-CTS won't notice any change
>> at all.  I guess the question is: with Auto-CTS should you pick the
>> "ideal" 6272 or a value that allows CTS to be the closest to integral
>> as possible.  By reading between the lines of the spec, I decided that
>> it was slightly more important to allow for an integral CTS.  If
>> achieving an integral CTS wasn't a goal then the spec wouldn't even
>> have listed special cases for any of the clock rates.  We would just
>> be using the ideal N and Auto-CTS and be done with it.  The whole
>> point of the tables they list is to make CTS integral.
>
> Specification recommends many contradictory things without explicit
> prioritization, at least I have not found it.
>
> So we should relay on our intuition.
>
> I guess that with auto-cts N we should follow recommendation - I guess
> most sinks have been better tested with recommended values.
>
> So what with non-auto-cts case:
>
> 1. How many devices do not have auto-cts? how many alternative TMDS
> clocks we have? Maybe it is theoretical problem.
>
> 2. Alternating CTS in software is possible, but quite
> complicated/annoying, but at least it will follow recommendation :)
>
>
> Regards
>
> Andrzej
>
>
>>
>>>> 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.
>>>>
>>>> NOTES:
>>>> - 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.
>>>> - This patch makes much less of a difference after the patch
>>>>   ("drm/bridge: dw-hdmi: Use automatic CTS generation mode when using
>>>>   non-AHB audio"), at least if you're using I2S audio.  The main goal
>>>>   of picking a good N is to make it possible to get a nice integral
>>>>   CTS value, but if CTS is automatic then that's much less critical.
>>> As I said above HDMI recommendations are different from those from your
>>> patch. Please elaborate why?
>>>
>>> Btw I've seen your old patches introducing recommended N/CTS calculation
>>> helpers in HDMI framework, unfortunately abandoned due to lack of interest.
>>>
>>> Maybe resurrecting them would be a good idea, with assumption there will
>>> be users :)
>> I have old patches introducing this into the HDMI framework?  I don't
>> remember them / can't find them.  Can you provide a pointer?


And forgot answer this:

My mistake the patches were by Arnaud Pouliquen[1].

[1]: https://patchwork.kernel.org/patch/8906791/


Regards

Andrzej



>>
>> -Doug
>>
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Powered by blists - more mailing lists