| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<TYCPR01MB12093F5A839D0C4179CC4DB4DC29EA@TYCPR01MB12093.jpnprd01.prod.outlook.com>
Date: Wed, 21 May 2025 11:02:08 +0000
From: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
To: laurent.pinchart <laurent.pinchart@...asonboard.com>, Prabhakar
<prabhakar.csengg@...il.com>
CC: Biju Das <biju.das.jz@...renesas.com>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>, Geert Uytterhoeven
<geert+renesas@...der.be>, Magnus Damm <magnus.damm@...il.com>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Prabhakar
Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: RE: [PATCH v5 08/12] drm: renesas: rz-du: mipi_dsi: Use mHz for D-PHY
frequency calculations
Hi Laurent,
Thanks for your feedback.
> From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Sent: 20 May 2025 15:24
> Subject: Re: [PATCH v5 08/12] drm: renesas: rz-du: mipi_dsi: Use mHz for D-PHY frequency calculations
>
> Hi Prabhakar,
>
> Thank you for the patch.
>
> On Mon, May 12, 2025 at 07:23:26PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> >
> > Pass the HSFREQ in milli-Hz to the `dphy_init()` callback to improve
> > precision, especially for the RZ/V2H(P) SoC, where PLL dividers require
> > high accuracy.
>
> I have a hard time imagining the need for milliHz accuracy. Could you
> please explain why that is needed on V2H ?
This is needed because of the clock architecture found on the Renesas RZ/V2H,
in combination with a tight tolerance on the HFSFREQ calculation.
The clock architecture found on the RZ/V2H DSI is modelled as below:
______________________________________ ________________________
| | | |
| CPG | | DSI |
| ________ ____________________ | | _____ _____ |
| | | | | | | | | vclk | | |
| | PLLDSI |--->| CSDIV_2to32_PLLDSI |-|-->|-->| PLL |----->| PHY | |
| |________| |____________________| | | |_____| |_____| |
|______________________________________| |________________________|
The PLL found inside the DSI IP is very similar to the PLLDSI found in the
CPG IP block, although the limits for some of the parameters are different.
Also, the PLLDSI is followed by a programmable divider, whereas there is no
such thing in the DSI PLL IP.
The limits for the PLL found within the DSI IP are:
1 <= PLL_P <= 4
64 <= PLL_M <= 1023
0 <= PLL_S <= 5
−32768 <= PLL_K <= 32767
The limits for PLLDSI (found in CPG) are:
1 <= PLL_P <= 4
64 <= PLL_M <= 533
0 <= PLL_S <= 6
−32768 <= PLL_K <= 32767
The limits for the CSDIV_2to32_PLLDSI divider are:
CSDIV = 1/(2 + 2 * n), with n=0,...,15
Using mHz internally for representing VCLK and HSFREQ is vital to allow the DSI
Driver to keep the error between the calculated value of HSFREQ and the actual
value of HSFREQ below a certain threshold (0.5 Hz).
The difficulty in achieving a small error is down to the accuracy of the VCLK
representation.
Since the clock subsystem only allows for Hz, a 0.5 Hz error on the representation of
VCLK (down to the selection of frequencies that cannot be precisely achieved and
related rounding errors) may lead to a very big error in the calculation
of HSFREQ, which uses the below formula:
HSFREQ = (VCLK * bpp) / num_lanes
In the worst-case scenario (1 lane and 24 bpp), a 0.5 Hz error on the representation
of VCLK will lead to an error of 12 Hz(!) on the calculation of HSFREQ, leading
to a non-working video output.
By internally representing VCLK (and HSFREQ) in mHZ, the maximum error we make on
its representation is 0.5 mHz, with a total maximum error of 12 mHz on the
calculation of HSFREQ.
HSFREQ then gets converted back to Hz (bringing the error back to a maximum value
of 0.5 Hz) for calculating the PHY parameters, which at that point is good enough.
We have empirically proven this, so there is no way around it.
The thing that we got lucky about is that we can use the same algorithm to calculate
the parameters for PLLDSI and for the DSI PLL, therefore by sharing a function between
the CPG driver and the DSI driver we can, to some extent, bypass the limitations
of the clock subsystem and achieve the accuracy required by DSI.
Also, RZ/G3E has a similar architecture therefore it is subjected to similar
constraints.
I hope this clarifies things?
Cheers,
Fab
>
> > These changes prepare the driver for upcoming RZ/V2H(P) SoC support.
> >
> > Co-developed-by: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
> > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@...esas.com>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> > Reviewed-by: Biju Das <biju.das.jz@...renesas.com>
> > ---
> > v4->v5:
> > - Added Reviewed tag from Biju
> >
> > v3->v4:
> > - Used MILLI instead of KILO
> > - Made use of mul_u32_u32() for multiplication
> >
> > v2->v3:
> > - Replaced `unsigned long long` with `u64`
> > - Replaced *_mhz with *_millihz` in functions
> >
> > v1->v2:
> > - No changes
> > ---
> > drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 13 ++++++++-----
> > 1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c b/drivers/gpu/drm/renesas/rz-
> du/rzg2l_mipi_dsi.c
> > index 5fc607be0c46..f93519613662 100644
> > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> > @@ -31,7 +31,7 @@
> > struct rzg2l_mipi_dsi;
> >
> > struct rzg2l_mipi_dsi_hw_info {
> > - int (*dphy_init)(struct rzg2l_mipi_dsi *dsi, unsigned long hsfreq);
> > + int (*dphy_init)(struct rzg2l_mipi_dsi *dsi, u64 hsfreq_millihz);
> > void (*dphy_exit)(struct rzg2l_mipi_dsi *dsi);
> > u32 phy_reg_offset;
> > u32 link_reg_offset;
> > @@ -200,8 +200,9 @@ static u32 rzg2l_mipi_dsi_link_read(struct rzg2l_mipi_dsi *dsi, u32 reg)
> > */
> >
> > static int rzg2l_mipi_dsi_dphy_init(struct rzg2l_mipi_dsi *dsi,
> > - unsigned long hsfreq)
> > + u64 hsfreq_millihz)
> > {
> > + unsigned long hsfreq = DIV_ROUND_CLOSEST_ULL(hsfreq_millihz, MILLI);
> > const struct rzg2l_mipi_dsi_timings *dphy_timings;
> > unsigned int i;
> > u32 dphyctrl0;
> > @@ -274,6 +275,7 @@ static int rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi *dsi,
> > const struct drm_display_mode *mode)
> > {
> > unsigned long hsfreq, vclk_rate;
> > + u64 hsfreq_millihz;
> > unsigned int bpp;
> > u32 txsetr;
> > u32 clstptsetr;
> > @@ -305,9 +307,9 @@ static int rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi *dsi,
> > if (vclk_rate != mode->clock * MILLI)
> > dev_info(dsi->dev, "Requested vclk rate %lu, actual %lu mismatch\n",
> > mode->clock * MILLI, vclk_rate);
> > - hsfreq = DIV_ROUND_CLOSEST_ULL(vclk_rate * bpp, dsi->lanes);
> > + hsfreq_millihz = DIV_ROUND_CLOSEST_ULL(mul_u32_u32(vclk_rate, bpp * MILLI), dsi->lanes);
> >
> > - ret = dsi->info->dphy_init(dsi, hsfreq);
> > + ret = dsi->info->dphy_init(dsi, hsfreq_millihz);
> > if (ret < 0)
> > goto err_phy;
> >
> > @@ -315,6 +317,7 @@ static int rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi *dsi,
> > txsetr = TXSETR_DLEN | TXSETR_NUMLANEUSE(dsi->lanes - 1) | TXSETR_CLEN;
> > rzg2l_mipi_dsi_link_write(dsi, TXSETR, txsetr);
> >
> > + hsfreq = DIV_ROUND_CLOSEST_ULL(hsfreq_millihz, MILLI);
> > /*
> > * Global timings characteristic depends on high speed Clock Frequency
> > * Currently MIPI DSI-IF just supports maximum FHD@60 with:
> > @@ -776,7 +779,7 @@ static int rzg2l_mipi_dsi_probe(struct platform_device *pdev)
> > * mode->clock and format are not available. So initialize DPHY with
> > * timing parameters for 80Mbps.
> > */
> > - ret = dsi->info->dphy_init(dsi, 80000000);
> > + ret = dsi->info->dphy_init(dsi, 80000000ULL * MILLI);
> > if (ret < 0)
> > goto err_phy;
> >
>
> --
> Regards,
>
> Laurent Pinchart
Powered by blists - more mailing lists