On Wed, 17 Sep 2025 21:45:55 +0000 Chris Brandt <chris.bra...@renesas.com> wrote:
> 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. Hi Chris, no problem. Just to let you know, I tested your 2 patches on our custom board and the panel is still working well. Hugo. > -----Original Message----- > From: Hugo Villeneuve <h...@hugovil.com> > Sent: Wednesday, September 17, 2025 4:29 PM > To: Chris Brandt <chris.bra...@renesas.com> > Cc: Geert Uytterhoeven <geert+rene...@glider.be>; Michael Turquette > <mturque...@baylibre.com>; Stephen Boyd <sb...@kernel.org>; Biju Das > <biju.das...@bp.renesas.com>; Maarten Lankhorst > <maarten.lankho...@linux.intel.com>; Maxime Ripard <mrip...@kernel.org>; > Thomas Zimmermann <tzimmerm...@suse.de>; David Airlie <airl...@gmail.com>; > Simona Vetter <sim...@ffwll.ch>; Hien Huynh <hien.huynh...@renesas.com>; > Nghia Vo <nghia.vo...@renesas.com>; linux-renesas-...@vger.kernel.org; > linux-...@vger.kernel.org; dri-devel@lists.freedesktop.org > 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 <chris.bra...@renesas.com> wrote: > > > Before the MIPI DSI clock source can be configured, the target divide > > ratio needs to be known. > > > > Signed-off-by: Chris Brandt <chris.bra...@renesas.com> > > > > --- > > 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 > -- Hugo Villeneuve
