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

[Alexander Stein]

Hi Marek,

Am Mittwoch, 5. Juni 2024, 18:25:13 CEST schrieb Marek Vasut:
> On 6/5/24 12:52 PM, Alexander Stein wrote:
> > Hi Marek,
> 
> Hi,
> 
> [snip]
> > Ah, right. That seems simple. But I noticed another thing:
> > The TC9595 PLL is configured to 147333333 while the lcdif configures
> > the display clock to 147333000, the value actually stored in
> > adjusted_mode.clock. I do not know if this small difference can be neglected.
> 
> Good point, please see (*) below.
> 
> >>> No, sorry. I'm not sure about those VFIFO overruns/underruns you mentioned
> >>> in the commit message. Does this maybe only apply to DPI input?
> >>
> >> No, this actually happens with DSI input in both DPI and (e)DP output
> >> modes, it is only really well visible in some resolutions it seems.
> > 
> > Ah, i see.
> > 
> >>> 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.

> > 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.

> 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.

> >> 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.

> >>>>> 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.

> [...]
> 
> > 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 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.
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.

Are you running the DSIM host from 24MHz refclock?

> 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.

Best regards,
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/