[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<TY3PR01MB1134657A6D7A36FC387938AF7867DA@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Thu, 19 Jun 2025 05:05:30 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
CC: Geert Uytterhoeven <geert+renesas@...der.be>, Andrzej Hajda
<andrzej.hajda@...el.com>, Neil Armstrong <neil.armstrong@...aro.org>, Robert
Foss <rfoss@...nel.org>, laurent.pinchart
<laurent.pinchart@...asonboard.com>, Jonas Karlman <jonas@...boo.se>, Jernej
Skrabec <jernej.skrabec@...il.com>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Rob Herring <robh@...nel.org>, Krzysztof
Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Michael
Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, Magnus
Damm <magnus.damm@...il.com>, "dri-devel@...ts.freedesktop.org"
<dri-devel@...ts.freedesktop.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "linux-renesas-soc@...r.kernel.org"
<linux-renesas-soc@...r.kernel.org>, "linux-clk@...r.kernel.org"
<linux-clk@...r.kernel.org>, Fabrizio Castro
<fabrizio.castro.jz@...esas.com>, Prabhakar Mahadev Lad
<prabhakar.mahadev-lad.rj@...renesas.com>
Subject: RE: [PATCH v6 1/4] clk: renesas: rzv2h-cpg: Add support for DSI
clocks
Hi Prabhakar,
> -----Original Message-----
> From: Biju Das
> Sent: 18 June 2025 14:26
> Subject: RE: [PATCH v6 1/4] clk: renesas: rzv2h-cpg: Add support for DSI clocks
>
> Hi Prabhakar,
>
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@...ts.freedesktop.org> On Behalf Of
> > Lad, Prabhakar
> > Sent: 16 June 2025 11:45
> > Subject: Re: [PATCH v6 1/4] clk: renesas: rzv2h-cpg: Add support for
> > DSI clocks
> >
> > Hi Biju,
> >
> > Thank you for the review.
> >
> > On Fri, Jun 13, 2025 at 6:57 AM Biju Das <biju.das.jz@...renesas.com> wrote:
> > >
> > > Hi Prabhakar,
> > >
> > > > -----Original Message-----
> > > > From: Prabhakar <prabhakar.csengg@...il.com>
> > > > Sent: 30 May 2025 18:19
> > > .castro.jz@...esas.com>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> > > > lad.rj@...renesas.com>
> > > > Subject: [PATCH v6 1/4] clk: renesas: rzv2h-cpg: Add support for
> > > > DSI clocks
> > > >
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> > > >
> > > > Add support for PLLDSI and PLLDSI divider clocks.
> > > >
> > > > Introduce the `renesas-rzv2h-dsi.h` header to centralize and share
> > > > PLLDSI-related data structures, limits, and algorithms between the RZ/V2H CPG and DSI drivers.
> > > >
> > > > The DSI PLL is functionally similar to the CPG's PLLDSI, but has
> > > > slightly different parameter limits and omits the programmable
> > > > divider present in CPG. To ensure precise frequency
> > > > calculations-especially for milliHz-level accuracy needed by the
> > > > DSI driver-the shared algorithm
> > allows both drivers to compute PLL parameters consistently using the same logic and input clock.
> > > >
> > > > 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>
> > > > ---
> > > > v5->v6:
> > > > - Renamed CPG_PLL_STBY_SSCGEN_WEN to CPG_PLL_STBY_SSC_EN_WEN
> > > > - Updated CPG_PLL_CLK1_DIV_K, CPG_PLL_CLK1_DIV_M, and
> > > > CPG_PLL_CLK1_DIV_P macros to use GENMASK
> > > > - Updated req->rate in rzv2h_cpg_plldsi_div_determine_rate()
> > > > - Dropped the cast in rzv2h_cpg_plldsi_div_set_rate()
> > > > - Dropped rzv2h_cpg_plldsi_round_rate() and implemented
> > > > rzv2h_cpg_plldsi_determine_rate() instead
> > > > - Made use of FIELD_PREP()
> > > > - Moved CPG_CSDIV1 macro in patch 2/4
> > > > - Dropped two_pow_s in rzv2h_dsi_get_pll_parameters_values()
> > > > - Used mul_u32_u32() while calculating output_m and output_k_range
> > > > - Used div_s64() instead of div64_s64() while calculating
> > > > pll_k
> > > > - Used mul_u32_u32() while calculating fvco and fvco checks
> > > > - Rounded the final output using DIV_U64_ROUND_CLOSEST()
> > > >
> > > > v4->v5:
> > > > - No changes
> > > >
> > > > v3->v4:
> > > > - Corrected parameter name in rzv2h_dsi_get_pll_parameters_values()
> > > > description freq_millihz
> > > >
> > > > v2->v3:
> > > > - Update the commit message to clarify the purpose of `renesas-rzv2h-dsi.h`
> > > > header
> > > > - Used mul_u32_u32() in rzv2h_cpg_plldsi_div_determine_rate()
> > > > - Replaced *_mhz to *_millihz for clarity
> > > > - Updated u64->u32 for fvco limits
> > > > - Initialized the members in declaration order for
> > > > RZV2H_CPG_PLL_DSI_LIMITS() macro
> > > > - Used clk_div_mask() in rzv2h_cpg_plldsi_div_recalc_rate()
> > > > - Replaced `unsigned long long` with u64
> > > > - Dropped rzv2h_cpg_plldsi_clk_recalc_rate() and reused
> > > > rzv2h_cpg_pll_clk_recalc_rate() instead
> > > > - In rzv2h_cpg_plldsi_div_set_rate() followed the same style
> > > > of RMW-operation as done in the other functions
> > > > - Renamed rzv2h_cpg_plldsi_set_rate() to rzv2h_cpg_pll_set_rate()
> > > > - Dropped rzv2h_cpg_plldsi_clk_register() and reused
> > > > rzv2h_cpg_pll_clk_register() instead
> > > > - Added a gaurd in renesas-rzv2h-dsi.h header
> > > >
> > > > v1->v2:
> > > > - No changes
> > > > ---
> > > > drivers/clk/renesas/rzv2h-cpg.c | 278 +++++++++++++++++++++++++-
> > > > drivers/clk/renesas/rzv2h-cpg.h | 13 ++
> > > > include/linux/clk/renesas-rzv2h-dsi.h | 210 +++++++++++++++++++
> > > > 3 files changed, 492 insertions(+), 9 deletions(-) create mode
> > > > 100644 include/linux/clk/renesas- rzv2h-dsi.h
> > > >
> > > > diff --git a/drivers/clk/renesas/rzv2h-cpg.c
> > > > b/drivers/clk/renesas/rzv2h-cpg.c index
> > > > 761da3bf77ce..d590f9f47371 100644
> > > > --- a/drivers/clk/renesas/rzv2h-cpg.c
> > > > +++ b/drivers/clk/renesas/rzv2h-cpg.c
> > > > @@ -14,9 +14,13 @@
> > > > #include <linux/bitfield.h>
> > > > #include <linux/clk.h>
> > > > #include <linux/clk-provider.h>
> > > > +#include <linux/clk/renesas-rzv2h-dsi.h>
> > > > #include <linux/delay.h>
> > > > #include <linux/init.h>
> > > > #include <linux/iopoll.h>
> > > > +#include <linux/math.h>
> > >
> > >
> > >
> > > > + req->rate =
> > > > + DIV_ROUND_CLOSEST_ULL(dsi_dividers->freq_millihz,
> > > > + MILLI);
> > > > +
> > > > + return 0;
> > > > +};
> > > > +
> > > > +static int rzv2h_cpg_plldsi_div_set_rate(struct clk_hw *hw,
> > > > + unsigned long rate,
> > > > + unsigned long parent_rate) {
> > > > + struct rzv2h_plldsi_div_clk *dsi_div = to_plldsi_div_clk(hw);
> > > > + struct rzv2h_cpg_priv *priv = dsi_div->priv;
> > > > + struct rzv2h_plldsi_parameters *dsi_dividers = &priv->plldsi_div_parameters;
> > > > + struct ddiv ddiv = dsi_div->ddiv;
> > > > + const struct clk_div_table *clkt;
> > > > + bool div_found = false;
> > > > + u32 val, shift, div;
> > > > +
> > > > + div = dsi_dividers->csdiv;
> > > > + for (clkt = dsi_div->dtable; clkt->div; clkt++) {
> > > > + if (clkt->div == div) {
> > > > + div_found = true;
> > > > + break;
> > > > + }
> > > > + }
> > > > +
> > > > + if (!div_found)
> > > > + return -EINVAL;
> > >
> > > This check can be done in determine rate and cache the divider??
> > >
> > Ok, I'll drop this check as the divider is already cached. The for
> > loop above is to determine the val which is used below to program the registers.
>
> If you are caching actual divider value, then the check is not required here.
> Otherwise the above code is fine.
>
> Assume the csdiv you found, have no corresponding match in the table.
1) By looking at RZ/G3E, can we make this code more scalable?
RZ/G3E has 2 PLL-DSI's
PLL-DSI1 supports DUAL LVDS, Single LVDS and DSI
PLL-DSI2 supports single LVDS, DPI and DSI
In total there will be 4 limit tables (DSI, single LVDS, Dual LVDS and DPI)
Based on the display output, each PLL needs to pick the appropriate limit table
to compute PLL parameters.
2) Can we drop DSI divider limits from the limit table and use the values from dsi divider table
itself which is passed in DEF_PLLDSI_DIV?
Note:
LVDS does not use DSI dividers as it is connected to a Mux and always has fixed Divider 7.
DPI uses the DSI divider without any constraints like DSI
Video clock and DSI HS Byte clock must follow the relationship.
Video clock Frequency [Hz] × Video Pixel Bit Depth [bit]
<= DSI HS Byte clock Frequency [Hz] × 8 [bit] × Number of DSI HS Data Lane
Cheers,
Biju
Powered by blists - more mailing lists