[PATCH 2/6] drm/bridge: tc358767: Use tc_pxl_pll_calc() to correct adjusted_mode clock

[Marek Vasut]

On 6/6/24 12:10 PM, Alexander Stein wrote:
> Hi Marek,

Hello Alexander,

sorry for the delay.

>>>>> At least for 148.5MHz (1080p) apparently it is not possible to that
>>>>> exact clock anyway.
>>>>
>>>> Right, which sucks, but the TC9595 datasheet explicitly states that the
>>>> FRMSYNC mode should always be enabled (and is apparently force-enabled
>>>> on newer bridge chips with no option to disable it) unless the Pixel
>>>> clock are sourced from DSI clock (which is never the case with this
>>>> driver). That's what the [1] patch does.
>>>>
>>>> But messing with the HFP isn't really working for all resolutions, so
>>>> this patch instead adjusts the input pixel clock to fit the Pixel PLL
>>>> limits, which avoids the VFIFO issues altogether. And that makes the
>>>> FRMSYNC mode work for all kinds of resolutions.
>>>
>>> I can't tell if VFIFO issue arise here, because I'm currently trying to
>>> get a reliable and stable output for DP.
>>
>> What kind of problem do you observe on your side exactly ? (I think the
>> answer to this is at the end). I've spent quite a while on this, so
>> maybe I ran into that before.
> 
> DP output either works as expected or not at all. The monitor stays in
> sleep mode then. I only notice that VFUEN is not cleared in this case.

Let's continue this one at the end of this thread.

>>> The documentation is somewhat
>>> limited, but FRAMSYNC seems almost necessary in any case, unless you
>>> utilize DSI bus 100% for this device to get the correct display timings
>>> using DSI packets.
>>
>> That's not quite what it says.
>>
>> I don't know what kind of datasheet you have and for which exact bridge,
>> but look at Section 4, block diagram.
> 
> I have the datasheet 0.1 and 1.1 for TC9595 available. Also the
> TC358767A_867XBG v37 spreadsheet.

I have rev. 0.1 and rev. 1.3 datasheets for TC9595 .
I also have TC9595 DSI-to-DPI timing spreadsheet.

>> The FRMSYNC operates on the LS_Clk domain side, right of V_FiFo . It
>> reads lines from the VFIFO and sends them out of the DP as DP packets.
>> As long as the input into the VFIFO delivers the lines such that the
>> VFIFO does not overflow or underflow, everything is fine.
>>
>> The DSI side starts to write line into the VFIFO on HSS (Hsync start),
>> see "DSI Packets for Video Transmission". HSE (Hsync end) packet is
>> ignored, see "Video Transmission" . Two consecutive HSS must be sent
>> with the same time between the two as time between two lines configured
>> into FRMSYNC timing generator.
>>
>> However, I _think_ (*) if the time between two HSS is slightly longer
>> than what the FRMSYNC expects, the FRMSYNC stretches the HFP slightly
>> for each frame and we won't run into any problems (see 4.12 Clocks and
>> search for keyword "approximated", they talk about close approximation
>> there).
> 
> Which datasheet do you have available? It might just be a typo, but the
> Clocks section is 4.13 in my case.

Here I used the rev. 1.3 from 20230727 . Definitely section 4.12 in this 
datasheet, maybe they removed a chapter since ? Here is section 4 list:

- MIPI DSI Rx
- MIPI DPI Rx
- I2S Audio Rx
- DisplayPortTM Tx
- Parallel Output Mode
- GPIO Interface
- I2C Slave Interface
- Interrupt Interface
- Internal Test Pattern (Color Bar) Generator
- Reset
- Boot-Strap & State of TC9595XBG chip after Reset
- Clocks

>>>> You might be wondering why not source pixel clock from DSI HS clock and
>>>> disable FRMSYNC. For one thing, this would limit the ability to pick
>>>> faster DSI HS clock and make good use of the DSI burst mode (the DSI
>>>> link can go into LP state after transmitting entire line of pixel data
>>>> for longer with faster DSI HS clock). The other thing is, to drive the
>>>> Pixel PLL (not to be confused with pixel clock) from DSI HS clock, the
>>>> DSI HS clock have to be set to specific clock rates (13*4*7=364 MHz and
>>>> 13*4*9=468 MHz), which is really limiting.
>>
>> btw. those 13 MHz match one of RefClk input options, the *4 is from
>> HSBY4 divider and 7 and 9 come from MODE[1] strap option dividers.
>>
>>> This is not mentioned in the datasheet at all, but just in a small note
>>> in the calculation sheet, without any description. On a different platform
>>> DSI HS clock running at 445.5MHz seems also possible, even if unsupported.
>>
>> You can use arbitrary DSI HS clock as long as you don't derive RefClk
>> from DSI HS clock (and RefClk feeds Pixel PLL).
> 
> Yes, that's what I would expect as well, but that other platform was
> configured to derive RefClk from DSI/4.

I think that's the HSCKBY4 thing, the DSI HS clock are divided by 4 and 
then by either 7 or 9 to achieve 13 MHz RefClk input.

This may be achievable with some effort for DSI-to-DPI mode, but for the 
DSI-to-(e)DP mode with aux channel this is currently far away. And with 
the limitations this imposes on the DSI clock, I am not particularly 
interested in this mode of operation, sourcing the RefClk from Xtal as 
the driver assumes right now does provide much more flexibility.

>>>>>>> BTW: Which platform are you testing on?
>>>>>>
>>>>>> MX8MP with LCDIFv3 -> DSIM -> TC9595 -> DP output.
>>>>>>
>>>>>> The TC9595 is 2nd generation (automotive?) replacement for TC358767 (1st
>>>>>> generation replacement is TC358867) .
>>>>>
>>>>> Same here. But fail to get output on a DP monitor if I'm running from
>>>>> external refclk. Using DSICLK/4 seems necessary for some reason, but it
>>>>> is very unreliable to get a proper image.
>>>>
>>>> Do you have all the patches in place ? This is what you should have:
>>>>
>>>> drm/bridge: tc358767: Enable FRMSYNC timing generator
>>>> drm: bridge: samsung-dsim: Initialize bridge on attach
>>>> drm/bridge: tc358767: Reset chip again on attach
>>>> clk: imx: clk-imx8mp: Allow media_disp pixel clock reconfigure parent rate
>>>> drm: lcdif: Use adjusted_mode .clock instead of .crtc_clock
>>>> drm/bridge: tc358767: Fix comment in tc_edp_mode_valid
>>>> drm/bridge: tc358767: Check if fully initialized before signalling HPD
>>>> event via IRQ
>>>> drm/bridge: tc358767: Split tc_pxl_pll_en() into parameter calculation
>>>> and enablement
>>>> drm/bridge: tc358767: Use tc_pxl_pll_calc() to correct adjusted_mode clock
>>>> drm/bridge: tc358767: Drop line_pixel_subtract
>>>> drm/bridge: tc358767: Disable MIPI_DSI_CLOCK_NON_CONTINUOUS
>>>> drm/bridge: tc358767: Set LSCLK divider for SYSCLK to 1
>>>> Revert "drm/bridge: tc358767: Set default CLRSIPO count"
>>>> drm/bridge: tc358767: Add configurable default preemphasis
>>>
>>> Thanks, I was missing the lcdif and clk-imx8mp patches. I saw them separately
>>> on the mailing list, but didn't realize would need them.
>>>
>>>> I only use external refclk, at 26 MHz.
>>>
>>> Same for me, difference is I'm using a crystal oscillator instead of
>>> CCM_CLKOUT. And yes, this is the TQ platform TQMa8MPxL/MBa8MPxL.
>>
>> I also have a SoM modified to use Xtal, but it seems unnecessary, the
>> CCM output saves on BoM too.
> 
> That's right, but Xtal is what I have right now. A modification for
> CCM_CLKOUT wouldbe possible if clock drift is an issue here.

I think the TC9595 does have rather stringent requirements on the Xtal 
indeed, although it also seems the bridge is very tolerant about those 
input clock :)

>> [...]
>>
>>> Thanks for sharing, despite the fixed-clock and IMX8MP_CLK_CLKOUT2 this
>>> looks very much the same. Unfortunately I get an output on DP just sometimes.
>>> As Bit 0 in register 0x464 is not cleared, I suspect the bridge is not
>>> recognizing DSI (VSYNC) packets properly :(
>>> At some point this bridge is lenient, but picky otherwise.
>>
>> You did strap the bridge such that it sources RefClk from the Xtal,
>> right (MODE[0] = 0 -> RefClk)? I haven't seen VFUEN[0] lock up like that
>> before.
> 
> Yes, MODE[0] is pulled down. Otherwise I2C is not even possible.
> I've tried using MODE[0]=1 with a fixed DSI HS clock, but without luck.

You would have to have DSI clock running before releasing the bridge 
from reset. I did manage to get this working at some point with 
STM32MP15xx DSI host using crude hackage, but I don't have any interest 
in supporting this mode. All the hardware I have available has external 
RefClk.

>> You could try and force the bridge to generate test pattern instead of
>> DSI_RX, edit the tc358767.c -> locate tc_dsi_rx_enable() -> locate
>> tc_test_pattern -> make sure the branch option value |=
>> DP0_VIDSRC_COLOR_BAR is activated . Rebuild and retest .
>>
>> This will allow you to validate the TC358767<->DP connection separately
>> from the DSI<->TC358767 connection . This is what I did when I was
>> debugging DSI issues on another board with TC9595 . Once you are sure
>> one side of the connection is stable, you can focus on the other side.
>>
>> Also, with the COLOR_BAR mode in place, try experimenting with the DP
>> preemphasis tuning and possibly the initial differential voltage tuning
>> (it is in the same registers as the preemphasis). Maybe you have link
>> quality issues and the link is borderline, it does train and link up,
>> but fails right after. Increasing the preemphasis and differential
>> voltage might help stabilize it.
> 
> Thanks, but as far I can tell I don't have any issues on the DP side.
> With command line parameter 'tc358767.test=1' in place I get that test
> pattern with 100% success rate. So I consider my problem on the DSI side only.

Try tuning the CLRSIPO values up and down (probably between 3..25), does 
that help ?

> As FVUEN is cleared at the next VSYNC event I suspect the DSI timings
> are (slightly) off, but unfortunately I don't have equipment to check
> DSI signal quality/timings.

As long as the LCDIFv3 pixel clock are equal or slightly slower than 
what the TC9595 PixelPLL generates, AND, DSIM serializer has enough 
bandwidth on the DSI bus (i.e. set the bus to 1 GHz, the TC9595 DSI RX 
cannot go any faster), you should have no issues on that end.

When in doubt, try and use i2ctransfer to read out register 0x300 
repeatedly, that's DSI RX error counter register. See if the DSI error 
count increments.

> Are you running the DSIM host from 24MHz refclock?

Yes, I did not modify imx8mp.dtsi mipi_dsi node 
samsung,pll-clock-frequency = <24000000>; in any way , so that's 24 MHz 
base.

>> Also, I use these to fiddle with the TC9595 (in my case) DP timing
>> registers, the first one reads them all out, the second one writes them.
>> Aadjust values as needed, the values below are from whatever experiment
>> so they are likely invalid. Make specific note of the last 0x01 on the
>> second line, that's the VFUEN update:
>> $ i2ctransfer -f -y 2 w2 at 0xf 0x4 0x50 r24
>> $ i2ctransfer -f -y 2 w26 at 0xf 0x4 0x50   0x10 0x01 0x50 0x01   0x20 0x00
>> 0x50 0x00   0x80 0x07 0x20 0x00   0x06 0x00 0x1a 0x00   0xb0 0x04 0x03
>> 0x00   0x01 0x00 0x00 0x00
>>
>> [...]
> 
> Thanks I'll keep that at hand, if they might come handy.

Glad I could help.