Re: [PATCH V6 0/5] phy: freescale: fsl-samsung-hdmi: Expand phy clock options

[Frieder Schrempf <frieder.schrempf@kontron.de>]

On 05.09.24 2:35 PM, Adam Ford wrote:
> On Thu, Sep 5, 2024 at 2:49 AM Frieder Schrempf
> <frieder.schrempf@kontron.de> wrote:
>>
>> On 05.09.24 1:30 AM, Adam Ford wrote:
>>> Currently, there is a look-up-table to describe all the clock options the HDMI PHY
>>> can use.  Some of these entries in the LUT are using a fractional divider which does
>>> not have a well documented algorithm for determinging values, but the the integer
>>> divider can use an algorithm to calculate the integer divder values dynamically
>>> beyond those listed in the LUT and also duplicates some of the entries.
>>>
>>> The first two patches do not do anything functionally other than simplify
>>> some of the register accesses and de-duplicates some of the register look-ups.
>>>
>>> The third patch adds support for the integer divider and uses it whenever the
>>> clock request is an exact match.  Otherwise, it will use the LUT as before.
>>> The rouding is still based on the LUT if the integer clock isn't an exact match.
>>>
>>> The forth patch updates thes set_rate and round_rate functions to use either
>>> the fractional clock LUT or the the integer divder mechanism to determine
>>> which ever clock rate might be closest match.
>>>
>>> The last patch removes the integer divider entries from the LUT since by then
>>> it'll be comparing both the integer divider calculator and the closest value
>>> in the LUT.
>>>
>>> In my testing with a AOC 4K monitor, I was able to add 4 entries in my modetest
>>> table.  I do not have an HDMI analyzer, so I just used my monitor to determine
>>> if this series worked.
>>
>> So I tested this series and it works fine. With Dominique's patch to
>> allow for 0.5% deviation for the clock, all the 24 modes of my monitor
>> and 30 out of 42 modes of my HDMI grabber are working now.
>>
>> I still have some issues with LCDIF underrun errors on modeswitch with
>> v6.11-rc6 but these are unrelated to this series.
> 
> I was comparing the LCDIF driver from the NXP downstream and the
> mainline, and I noticed the panic threshold values are different in
> the NXP downstream for the LCDIF3 which generates the video for the
> HDMI.
> 
> The downstream threshold states the default low value is 1/3 and the
> default high value is 2/3
> It appears the mainline matches these values [1].
> However, there is an option to override the defaults and change the
> values to 1/2 and 3/4.  I don't really understand what these values
> do, but it might be worth some investigation to see if playing with
> these values helps or not.  The notes in both versions state the panic
> isn't designed to trigger an interrupt, but to get the Noc and QoS
> modules to handle it.

I will try that. Although I doubt that it is related to the thresholds.
This should be relevant in cases where bus priorities play some role
(high load, conflicts with other peripherals, etc.).

In my case the target is basically idle and other peripherals are not
used. Also it happens only when switching modes or turning on/off.

> 
> Either way, it's unrelated to this patch, but my monitor doesn't
> always sync values from the LUT, but sometimes it does, but I can't
> tell if we have an underflow or not.

You can check by reading register 0x32fc6024 and see if the second bit
(UNDERRUN) is set.

> 
> adam
> 
> [1] - https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/mxsfb/lcdif_kms.c?h=next-20240905#n350
> 
>>
>> Thanks Adam and Dominique for the great work!
> 
> I enjoy puzzles.  Reading through the tables and looking for patterns
> was fun.  I did spend some time trying to understand the fractional
> divider stuff, but I didn't make much progress.

I tried myself and I made some further progress today. But I'm not quite
there, yet. The goal would be to be able to calculate some more table
entries. Let's see.