Re: [PATCH 5/5] drm/panel: st7703: Drive XBD599 panel at higher clock rate

Frank Oltmanns
2023-12-20T08:08:32Z

On 2023-12-19 at 18:04:29 +0100, Jernej Škrabec <jernej.skrabec@gmail.com> 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@oltmanns.dev>
>
> 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.

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?

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