Hi Biju, Thank you for the review.
On Sun, May 4, 2025 at 1:33 PM Biju Das <biju.das...@bp.renesas.com> wrote: > > Hi Prabhakar, > > > -----Original Message----- > > From: Prabhakar <prabhakar.cse...@gmail.com> > > Sent: 30 April 2025 21:41 > > Subject: [PATCH v4 08/15] drm: renesas: rz-du: mipi_dsi: Use VCLK for > > HSFREQ calculation > > > > From: Lad Prabhakar <prabhakar.mahadev-lad...@bp.renesas.com> > > > > Update the RZ/G2L MIPI DSI driver to calculate HSFREQ using the actual VCLK > > rate instead of the mode > > clock. The relationship between HSCLK and VCLK is: > > > > vclk * bpp <= hsclk * 8 * lanes > > > > Retrieve the VCLK rate using `clk_get_rate(dsi->vclk)`, ensuring that > > HSFREQ accurately reflects the > > clock rate set in hardware, leading to better precision in data > > transmission. > > > > Additionally, use `DIV_ROUND_CLOSEST_ULL` for a more precise division when > > computing `hsfreq`. Also, > > update unit conversions to use correct scaling factors for better clarity > > and correctness. > > > > Since `clk_get_rate()` returns the clock rate in Hz, update the HSFREQ > > threshold comparisons to use Hz > > instead of kHz to ensure correct behavior. > > > > Co-developed-by: Fabrizio Castro <fabrizio.castro...@renesas.com> > > Signed-off-by: Fabrizio Castro <fabrizio.castro...@renesas.com> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad...@bp.renesas.com> > > --- > > v3->v4: > > - Used MILLI instead of KILO > > > > v2->v3: > > - No changes > > > > v1->v2: > > - No changes > > --- > > .../gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 27 ++++++++++--------- > > 1 file changed, 15 insertions(+), 12 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 c5f698cd74f1..911c955a3a76 100644 > > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c > > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c > > @@ -8,6 +8,7 @@ > > #include <linux/delay.h> > > #include <linux/io.h> > > #include <linux/iopoll.h> > > +#include <linux/math.h> > > #include <linux/module.h> > > #include <linux/of.h> > > #include <linux/of_graph.h> > > @@ -15,6 +16,7 @@ > > #include <linux/pm_runtime.h> > > #include <linux/reset.h> > > #include <linux/slab.h> > > +#include <linux/units.h> > > > > #include <drm/drm_atomic.h> > > #include <drm/drm_atomic_helper.h> > > @@ -199,7 +201,7 @@ static int rzg2l_mipi_dsi_dphy_init(struct > > rzg2l_mipi_dsi *dsi, > > /* All DSI global operation timings are set with recommended setting > > */ > > for (i = 0; i < ARRAY_SIZE(rzg2l_mipi_dsi_global_timings); ++i) { > > dphy_timings = &rzg2l_mipi_dsi_global_timings[i]; > > - if (hsfreq <= dphy_timings->hsfreq_max) > > + if (hsfreq <= (dphy_timings->hsfreq_max * MILLI)) > > break; > > } > > > > @@ -258,7 +260,7 @@ static void rzg2l_mipi_dsi_dphy_exit(struct > > rzg2l_mipi_dsi *dsi) static int > > rzg2l_mipi_dsi_startup(struct rzg2l_mipi_dsi *dsi, > > const struct drm_display_mode *mode) { > > - unsigned long hsfreq; > > + unsigned long hsfreq, vclk_rate; > > unsigned int bpp; > > u32 txsetr; > > u32 clstptsetr; > > @@ -269,6 +271,12 @@ static int rzg2l_mipi_dsi_startup(struct > > rzg2l_mipi_dsi *dsi, > > u32 golpbkt; > > int ret; > > > > + ret = pm_runtime_resume_and_get(dsi->dev); > > + if (ret < 0) > > + return ret; > > + > > + clk_set_rate(dsi->vclk, mode->clock * MILLI); > > /* > > * Relationship between hsclk and vclk must follow > > * vclk * bpp = hsclk * 8 * lanes > > @@ -280,13 +288,8 @@ static int rzg2l_mipi_dsi_startup(struct > > rzg2l_mipi_dsi *dsi, > > * hsclk(bit) = hsclk(byte) * 8 = hsfreq > > */ > > bpp = mipi_dsi_pixel_format_to_bpp(dsi->format); > > - hsfreq = (mode->clock * bpp) / dsi->lanes; > > - > > - ret = pm_runtime_resume_and_get(dsi->dev); > > - if (ret < 0) > > - return ret; > > - > > - clk_set_rate(dsi->vclk, mode->clock * 1000); > > + vclk_rate = clk_get_rate(dsi->vclk); > > Not sure, If there is a requested vclk for example, 108.45 MHz and get_rate() > is returning > only 108MHz, do we need to propagate/print this error(like requested vs > actual, if there is a mismatch) > as there is change in resolution? > OK, I'll add the below: if (vclk_rate != mode->clock * MILLI) dev_info(dsi->dev, "Requested vclk rate %lu, actual %lu mismatch\n", mode->clock * MILLI, vclk_rate); > Otherwise LGTM > Reviewed-by: Biju Das <biju.das...@bp.renesas.com> > Cheers, Prabhakar