Re: [PATCH v5 4/4] drm: renesas: rz-du: mipi_dsi: Add support for RZ/V2H(P) SoC

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Prabhakar, Fabrizio,

On Mon, 12 May 2025 at 20:43, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add DSI support for Renesas RZ/V2H(P) SoC.
>
> Co-developed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2022 Renesas Electronics Corporation
>   */
>  #include <linux/clk.h>
> +#include <linux/clk/renesas-rzv2h-dsi.h>
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
> @@ -30,6 +31,9 @@
>
>  #define RZ_MIPI_DSI_FEATURE_16BPP      BIT(0)
>
> +#define RZV2H_MIPI_DPHY_FOUT_MIN_IN_MEGA       (80 * MEGA)
> +#define RZV2H_MIPI_DPHY_FOUT_MAX_IN_MEGA       (1500 * MEGA)

RZV2H_MIPI_DPHY_FOUT_M{IN,AX}_IN_MHZ?

> +
>  struct rzg2l_mipi_dsi;
>
>  struct rzg2l_mipi_dsi_hw_info {
> @@ -40,6 +44,7 @@ struct rzg2l_mipi_dsi_hw_info {
>                               u64 *hsfreq_millihz);
>         unsigned int (*dphy_mode_clk_check)(struct rzg2l_mipi_dsi *dsi,
>                                             unsigned long mode_freq);
> +       const struct rzv2h_pll_div_limits *cpg_dsi_limits;
>         u32 phy_reg_offset;
>         u32 link_reg_offset;
>         unsigned long max_dclk;
> @@ -47,6 +52,11 @@ struct rzg2l_mipi_dsi_hw_info {
>         u8 features;
>  };
>
> +struct rzv2h_dsi_mode_calc {
> +       unsigned long mode_freq;
> +       u64 mode_freq_hz;

Interesting... I guess mode_freq is not in Hz?

> +};
> +
>  struct rzg2l_mipi_dsi {
>         struct device *dev;
>         void __iomem *mmio;

> +static u16 rzv2h_dphy_find_ulpsexit(unsigned long freq)
> +{
> +       static const unsigned long hsfreq[] = {
> +               1953125UL,
> +               3906250UL,
> +               7812500UL,
> +               15625000UL,
> +       };
> +       static const u16 ulpsexit[] = {49, 98, 195, 391};
> +       unsigned int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(hsfreq); i++) {
> +               if (freq <= hsfreq[i])
> +                       break;
> +       }
> +
> +       if (i == ARRAY_SIZE(hsfreq))
> +               i -= 1;

i--

> +
> +       return ulpsexit[i];
> +}
> +
> +static u16 rzv2h_dphy_find_timings_val(unsigned long freq, u8 index)
> +{
> +       const struct rzv2h_mipi_dsi_timings *timings;
> +       u16 i;
> +
> +       timings = &rzv2h_dsi_timings_tables[index];
> +       for (i = 0; i < timings->len; i++) {
> +               unsigned long hsfreq = timings->hsfreq[i] * 10000000UL;

(I wanted to say "MEGA", but then I noticed the 7th zero ;-)

10 * MEGA?

> +
> +               if (freq <= hsfreq)
> +                       break;
> +       }
> +
> +       if (i == timings->len)
> +               i -= 1;

i--

> +
> +       return timings->start_index + i;
> +};
> +
>  static void rzg2l_mipi_dsi_phy_write(struct rzg2l_mipi_dsi *dsi, u32 reg, u32 data)
>  {
>         iowrite32(data, dsi->mmio + dsi->info->phy_reg_offset + reg);
> @@ -308,6 +479,158 @@ static int rzg2l_dphy_conf_clks(struct rzg2l_mipi_dsi *dsi, unsigned long mode_f
>         return 0;
>  }
>
> +static unsigned int rzv2h_dphy_mode_clk_check(struct rzg2l_mipi_dsi *dsi,
> +                                             unsigned long mode_freq)
> +{
> +       struct rzv2h_plldsi_parameters *dsi_parameters = &dsi->dsi_parameters;
> +       u64 hsfreq_millihz, mode_freq_hz, mode_freq_millihz;
> +       struct rzv2h_plldsi_parameters cpg_dsi_parameters;
> +       unsigned int bpp, i;
> +
> +       bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> +
> +       for (i = 0; i < 10; i += 1) {
> +               unsigned long hsfreq;
> +               bool parameters_found;
> +
> +               mode_freq_hz = mode_freq * MILLI + i;

KILO?

And I guess you want to use mul_u32_u32(), as mode_freq_hz is u64?

> +               mode_freq_millihz = mode_freq_hz * MILLI * 1ULL;

Why * 1ULL?

> +               parameters_found = rzv2h_dsi_get_pll_parameters_values(dsi->info->cpg_dsi_limits,
> +                                                                      &cpg_dsi_parameters,
> +                                                                      mode_freq_millihz);
> +               if (!parameters_found)
> +                       continue;
> +
> +               hsfreq_millihz = DIV_ROUND_CLOSEST_ULL(cpg_dsi_parameters.freq_millihz * bpp,
> +                                                      dsi->lanes);
> +               parameters_found = rzv2h_dsi_get_pll_parameters_values(&rzv2h_plldsi_div_limits,
> +                                                                      dsi_parameters,
> +                                                                      hsfreq_millihz);
> +               if (!parameters_found)
> +                       continue;
> +
> +               if (abs(dsi_parameters->error_millihz) >= 500)
> +                       continue;
> +
> +               hsfreq = DIV_ROUND_CLOSEST_ULL(hsfreq_millihz, MILLI);
> +               if (hsfreq >= RZV2H_MIPI_DPHY_FOUT_MIN_IN_MEGA &&
> +                   hsfreq <= RZV2H_MIPI_DPHY_FOUT_MAX_IN_MEGA) {
> +                       dsi->mode_calc.mode_freq_hz = mode_freq_hz;
> +                       dsi->mode_calc.mode_freq = mode_freq;
> +                       return MODE_OK;
> +               }
> +       }
> +
> +       return MODE_CLOCK_RANGE;
> +}

> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi_regs.h
> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi_regs.h
> @@ -40,6 +40,39 @@
>  #define DSIDPHYTIM3_THS_TRAIL(x)       ((x) << 8)
>  #define DSIDPHYTIM3_THS_ZERO(x)                ((x) << 0)
>
> +/* RZ/V2H DPHY Registers */
> +#define PLLENR                         0x000
> +#define PLLENR_PLLEN                   BIT(0)
> +
> +#define PHYRSTR                                0x004
> +#define PHYRSTR_PHYMRSTN               BIT(0)
> +
> +#define PLLCLKSET0R                    0x010
> +#define PLLCLKSET0R_PLL_S(x)           ((x) << 0)

 #define PLLCLKSET0R_PLL_S GENMASK(2, 0)

and after that you can use FIELD_PREP(PLLCLKSET0R_PLL_S, x) in the code.
More opportunities for masks below...

> +#define PLLCLKSET0R_PLL_P(x)           ((x) << 8)
> +#define PLLCLKSET0R_PLL_M(x)           ((x) << 16)
> +
> +#define PLLCLKSET1R                    0x014
> +#define PLLCLKSET1R_PLL_K(x)           ((x) << 0)
> +
> +#define PHYTCLKSETR                    0x020
> +#define PHYTCLKSETR_TCLKTRAILCTL(x)    ((x) << 0)
> +#define PHYTCLKSETR_TCLKPOSTCTL(x)     ((x) << 8)
> +#define PHYTCLKSETR_TCLKZEROCTL(x)     ((x) << 16)
> +#define PHYTCLKSETR_TCLKPRPRCTL(x)     ((x) << 24)
> +
> +#define PHYTHSSETR                     0x024
> +#define PHYTHSSETR_THSEXITCTL(x)       ((x) << 0)
> +#define PHYTHSSETR_THSTRAILCTL(x)      ((x) << 8)
> +#define PHYTHSSETR_THSZEROCTL(x)       ((x) << 16)
> +#define PHYTHSSETR_THSPRPRCTL(x)       ((x) << 24)
> +
> +#define PHYTLPXSETR                    0x028
> +#define PHYTLPXSETR_TLPXCTL(x)         ((x) << 0)
> +
> +#define PHYCR                          0x030
> +#define PHYCR_ULPSEXIT(x)              ((x) << 0)
> +
>  /* --------------------------------------------------------*/
>
>  /* Link Status Register */

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds