Hi Hugo, Thank you for your review.
> > +rzg2l_cpg_dsi_div_set_divider(mipi_dsi_pixel_format_to_bpp(dsi->forma > > +t) / dsi->lanes, 1); > > What is this "1" value meaning? This is hard to decipher. That is true (unless you know to look in the other file) > If it is related to PLL5_TARGET_DSI, then these PLL5_TARGET_* macros should > be added to the renesas.h header file and used here. I was not clear how much I should be adding to that renesas.h file. But like you said, it would make the code easier to read. I was also waiting to hear what Geert thought about adding a new API to the clock driver. Chris -----Original Message----- From: Hugo Villeneuve <[email protected]> Sent: Wednesday, September 17, 2025 4:29 PM To: Chris Brandt <[email protected]> Cc: Geert Uytterhoeven <[email protected]>; Michael Turquette <[email protected]>; Stephen Boyd <[email protected]>; Biju Das <[email protected]>; Maarten Lankhorst <[email protected]>; Maxime Ripard <[email protected]>; Thomas Zimmermann <[email protected]>; David Airlie <[email protected]>; Simona Vetter <[email protected]>; Hien Huynh <[email protected]>; Nghia Vo <[email protected]>; [email protected]; [email protected]; [email protected] Subject: Re: [PATCH v2 2/2] drm: renesas: rz-du: Set DSI divider based on target MIPI device On Fri, 12 Sep 2025 10:20:56 -0400 Chris Brandt <[email protected]> wrote: > Before the MIPI DSI clock source can be configured, the target divide > ratio needs to be known. > > Signed-off-by: Chris Brandt <[email protected]> > > --- > v1->v2: > - Add spaces around '/' in comments > - Add target argument in new API > --- > drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 18 > ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > 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 f87337c3cbb5..ca0de93d5a1a 100644 > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c > @@ -7,6 +7,7 @@ > > #include <linux/bitfield.h> > #include <linux/clk.h> > +#include <linux/clk/renesas.h> > #include <linux/delay.h> > #include <linux/dma-mapping.h> > #include <linux/io.h> > @@ -732,6 +733,23 @@ static int rzg2l_mipi_dsi_host_attach(struct > mipi_dsi_host *host, > > drm_bridge_add(&dsi->bridge); > > + /* > + * Report required division ratio setting for the MIPI clock > +dividers Add missing dot at end of sentence. > + * Assume the default clock source is FOUTPOSTDIV (PLL/2) being fed to > the DSI-PHY, but also > + * the DSI-PHY must be 16x the MIPI-DSI HS clock. > + * > + * pllclk / 2 = vclk * DSI divider > + * pllclk = vclk * DSI divider * 2 > + * > + * hsclk = (vclk * DSI divider * 2) / 16 > + * > + * vclk * bpp = hsclk * 8 * num_lanes > + * vclk * bpp = ((vclk * DSI divider * 2) / 16) * 8 * num_lanes > + * which simplifies to... > + * DSI divider = bpp / num_lanes > + */ > + > +rzg2l_cpg_dsi_div_set_divider(mipi_dsi_pixel_format_to_bpp(dsi->forma > +t) / dsi->lanes, 1); What is this "1" value meaning? This is hard to decipher. If it is related to PLL5_TARGET_DSI, then these PLL5_TARGET_* macros should be added to the renesas.h header file and used here. > + > return 0; > } > > -- > 2.50.1 > > -- Hugo Villeneuve
