Dne sreda, 20. december 2023 ob 08:14:27 CET je Frank Oltmanns napisal(a): > > On 2023-12-19 at 18:04:29 +0100, Jernej Škrabec <jernej.skrabec@xxxxxxxxx> wrote: > > Dne ponedeljek, 18. december 2023 ob 14:35:23 CET je Frank Oltmanns napisal(a): > >> This panel is used in the pinephone that runs on a Allwinner A64 SOC. > >> Acoording to it's datasheet, the SOC requires PLL-MIPI to run at more > >> than 500 MHz. > >> > >> Therefore, change [hv]sync_(start|end) so that we reach a clock rate > >> that is high enough to drive PLL-MIPI within its limits. > >> > >> Signed-off-by: Frank Oltmanns <frank@xxxxxxxxxxxx> > > > > I'm not too sure about this patch. I see that PLL_MIPI doesn't have set > > minimum frequency limit in clock driver. If you add it, clock framework > > should find rate that is high enough and divisible with target rate. > > This one is really a tough nut. Unfortunately, the PLL_MIPI clock for > this panel has to run exactly at 6 * panel clock. Let me start by > showing the relevant part of the clock tree (this is on the pinephone > after applying the patches): > pll-video0 393600000 > pll-mipi 500945454 > tcon0 500945454 > tcon-data-clock 125236363 > > To elaborate, tcon-data-clock has to run at 1/4 the DSI per-lane bit > rate [1]. It's a fixed divisor > > The panel I'm proposing to change is defined as this: > > static const struct st7703_panel_desc xbd599_desc = { > .mode = &xbd599_mode, > .lanes = 4, > .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE, > .format = MIPI_DSI_FMT_RGB888, > .init_sequence = xbd599_init_sequence, > }; > > So, we have 24 bpp and 4 lanes. Therefore, the resulting requested > tcon-data-clock rate is > crtc_clock * 1000 * (24 / 4) / 4 > > tcon-data-clock therefore requests a parent rate of > 4 * (crtc_clock * 1000 * (24 / 4) / 4) > > The initial 4 is the fixed divisor between tcon0 and tcon-data-clock. > Since tcon0 is a ccu_mux, the rate of tcon0 equals the rate of pll-mipi. > > Since PLL-MIPI has to run at at least at 500MHz this forces us to have a > crtc_clock >= 83.333 MHz. The mode I'm prorposing results in a rate of > 83.502 MHz. This is much better explanation why this change is needed. Still, I think adding min and max rate to PLL_MIPI would make sense, so proper rates are guaranteed. Anyway, do you know where are all those old values come from? And how did you come up with new ones? I guess you can't just simply change timings, there are probably some HW limitations? Do you know if BSP kernel support this panel and how this situation is solved there? > > If we only changed the constraints on the PLL_MIPI without changing the > panel mode, we end up with a mismatch. This, in turn, would result in > dropped frames, right? >From what I read, I think frame rate would be higher than 60 fps. What exactly would happen depends on the panel. Best regards, Jernej > > Best regards, > Frank > > [1] Source: > https://elixir.bootlin.com/linux/v6.6.7/source/drivers/gpu/drm/sun4i/sun4i_tcon.c#L346 > > > > > Best regards, > > Jernej > > > >> --- > >> drivers/gpu/drm/panel/panel-sitronix-st7703.c | 14 +++++++------- > >> 1 file changed, 7 insertions(+), 7 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7703.c b/drivers/gpu/drm/panel/panel-sitronix-st7703.c > >> index b55bafd1a8be..6886fd7f765e 100644 > >> --- a/drivers/gpu/drm/panel/panel-sitronix-st7703.c > >> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7703.c > >> @@ -320,14 +320,14 @@ static int xbd599_init_sequence(struct st7703 *ctx) > >> > >> static const struct drm_display_mode xbd599_mode = { > >> .hdisplay = 720, > >> - .hsync_start = 720 + 40, > >> - .hsync_end = 720 + 40 + 40, > >> - .htotal = 720 + 40 + 40 + 40, > >> + .hsync_start = 720 + 65, > >> + .hsync_end = 720 + 65 + 65, > >> + .htotal = 720 + 65 + 65 + 65, > >> .vdisplay = 1440, > >> - .vsync_start = 1440 + 18, > >> - .vsync_end = 1440 + 18 + 10, > >> - .vtotal = 1440 + 18 + 10 + 17, > >> - .clock = 69000, > >> + .vsync_start = 1440 + 30, > >> + .vsync_end = 1440 + 30 + 22, > >> + .vtotal = 1440 + 30 + 22 + 29, > >> + .clock = (720 + 65 + 65 + 65) * (1440 + 30 + 22 + 29) * 60 / 1000, > >> .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, > >> .width_mm = 68, > >> .height_mm = 136, > >> > >> >