[PATCH v10 00/18] drm: Add Samsung MIPI DSIM bridge

[Maxime Ripard]

On Wed, Jan 04, 2023 at 04:08:47PM +0100, Marek Vasut wrote:
> On 1/3/23 11:59, Alexander Stein wrote:
> > Hi,
> > 
> > Am Sonntag, 18. Dezember 2022, 23:28:20 CET schrieb Marek Vasut:
> > > On 12/18/22 23:24, Adam Ford wrote:
> > > > On Sat, Dec 17, 2022 at 10:33 PM Marek Vasut <marex at denx.de> wrote:
> > > > > On 12/18/22 05:23, Adam Ford wrote:
> > > > > > On Sat, Dec 17, 2022 at 5:56 PM Marek Vasut <marex at denx.de> wrote:
> > > > > > > On 12/16/22 14:25, Alexander Stein wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > [...]
> > > > > > > 
> > > > > > > > Oh, nice, thanks for the pointer. When setting
> > > > > > > > 
> > > > > > > > > samsung,burst-clock-frequency = <668250000>;
> > > > > > > > 
> > > > > > > > in imx8mm.dtsi
> > > > > > > > I get a non-flickering display using 4 lanes. Although admittedly this
> > > > > > > > is just random guessing. I'm not sure which clock exactly has to be
> > > > > > > > in the range CHA_DSI_CLK_RANGE is configured to. With 4 lanes
> > > > > > > > SN65DSI84 is configured for>>>>>
> > > > > > > > 205-210 MHz (0x29), while I get these PLL PMS settings on DSIM:
> > > > > > > > > samsung-dsim 32e10000.dsi: PLL freq 668250000, (p 4, m 99, s 0)
> > > > > > > > > samsung-dsim 32e10000.dsi: hs_clk = 668250000, byte_clk = 83531250,
> > > > > > > > > esc_clk
> > > > > > > > 
> > > > > > > > = 16706250
> > > > > > > 
> > > > > > > If I recall it right, minimum PLL frequency is:
> > > > > > > 
> > > > > > > fPMS=1.2*width*height*bpp*fps=1.2*800*480*24*60=663.5 MHz
> > > > > > > 
> > > > > > > the link frequency is then
> > > > > > > 
> > > > > > > fHS=fPMS/lanes/2=82.9 MHz (on the DDR clock lane)
> > > > > > > 
> > > > > > > So DSI83 should be in the range of 80..85 MHz input clock if I
> > > > > > > calculate
> > > > > > > this right. Can you check what is the value of mode->clock, the
> > > > > > > mipi_dsi_panel_format_to_bpp() return value, ctx->dsi->lanes in dsi83
> > > > > > > sm65dsi83_get_dsi_range() ?
> > > > > > > 
> > > > > > > > AFAICS DSIM bridge is configurung hs_clk, byte_clk and esc_clk just
> > > > > > > > from DT
> > > > > > > > properties, while SN65DSI84 is using display mode and number of lanes.
> > > > > > > > 
> > > > > > > > Is it expected that the DSIM PLL frequencies are set in DT for a
> > > > > > > > specific
> > > > > > > > bridge/display setup?
> > > > > > > 
> > > > > > > No, there should be negotiation between the host and bridge/panel, I
> > > > > > > tried to propose two variants, but they were all rejected.
> > > > > > 
> > > > > > For one of Jagan's previous revisions, I added some code to let the
> > > > > > PHY auto adjust the frequencies instead of being fixed.  NXP had this
> > > > > > in their downstream kernel, but with this patch and another, I was
> > > > > > able to set a variety of pixel clocks from my HDMI monitor and my
> > > > > > DSI83. I haven't had time to re-base my work on Jagan's latest work,
> > > > > > but you can link to the patch I did for the older stuff here:
> > > > > > 
> > > > > > https://github.com/aford173/linux/commit/e845274b0f22ba3b24813ffd6bb3cb8
> > > > > > 8ab4b67e4 and
> > > > > > https://github.com/aford173/linux/commit/3f90057eb608f96d106029ef6398134
> > > > > > 75241936f
> > > > > > 
> > > > > > I've been traveling a lot lately, so I haven't had time to evaluate
> > > > > > his series, but I hope to get something like those re-based once the
> > > > > > DSI stuff has been accepted.
> > > > > 
> > > > > I have these two attempts, both rejected:
> > > > > 
> > > > > https://patchwork.freedesktop.org/patch/475207/
> > > > > https://patchwork.freedesktop.org/patch/496049/
> > > > 
> > > > I have some patches re-based to Jagan's latest branch.  It doesn't
> > > > impact any drivers other than the new samsung-dsim driver, and it
> > > > doesn't touch any of the drm helper functions either.  It adjusts hs
> > > > clock based on the connected device.  I am not sure what the impact
> > > > will have on the attached Exynos devices, so I am expecting some
> > > > iterations.  Right now it's working with my DSI83 chip, but I need to
> > > > get it working with my adv7535 part as well.  On the older branch, I
> > > > was able to sync the ad7535 with a variety of resolutions using
> > > > different pixel clock rates.
> > > > 
> > > > Once I get it working again with my adv7535 and cleaned up, I'll
> > > > submit the patches to the drm group, and I'll CC you, Jagan and Marek
> > > > Szyprowski with a reference to Jagan's series so people wanting to try
> > > > it can apply it to his branch.
> > > 
> > > The negotiation has to happen between the host and the bridge/panel,
> > > otherwise you won't be able to support bridge/panel devices which
> > > require specific clock rate on the DSI. Only the bridge/panel driver
> > > knows about such requirement.
> > 
> > AFAICS using Adam's patch the dynamic DPHY config is done in atomic_pre_enable
> > callback. So at this point the negotiation has to be finished already.
> > Wouldn't it be possible to setup 'dsi->format' within a atomic_check for
> > samsung_dsim? But I don't know how you would get the expected clock frequency
> > from the downward bridge.

I have zero context there, so I'm not sure what proposals have been made
already, but why isn't it possible to do it the other way around and
make the bridge ask the upstream driver if it can provide the clock
frequency the bridge require?

It's pretty much how the common clock framework works too: you set the
rate on the leaf clock, and it propagates upwards in your tree,
adjusting for any constraint we have along the way.

It does need to happen at atomic_check time though, otherwise it would
be terrible for anyone attempting to use that driver.

Maxime