Re: [PATCH v6 13/21] drm: renesas: rz-du: mipi_dsi: Add RZ_MIPI_DSI_FEATURE_GPO0R feature

Tommaso Merciai
2026-04-08T14:12:41Z

Hi Laurent,
Thanks for your review.

On 4/8/26 14:31, Laurent Pinchart wrote:
> On Wed, Apr 08, 2026 at 12:36:58PM +0200, Tommaso Merciai wrote:
>> The MIPI DSI ip found in the RZ/G3E SoC select the video input clock
>> based on the DU instance actually connected using the GPO0R register.
>>
>> Add this feature to the driver using `RZ_MIPI_DSI_FEATURE_GPO0R`, update
>> the code accordingly to manage the vclk selection with the introduction
>> of `rzg2l_mipi_dsi_get_input_port()`.
>>
>> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
>> ---
>> v5->v6:
>> - Moved rzg2l_mipi_dsi_link_write() into rzv2h_mipi_dsi_dphy_init()
>> + comments from HW Manual.
>>
>> v4->v5:
>> - No changes.
>>
>> v3->v4:
>> - No changes.
>>
>> v2->v3:
>> - No changes.
>>
>> v1->v2:
>> - No changes.
>>
>> .../gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 71 +++++++++++++++++--
>> .../drm/renesas/rz-du/rzg2l_mipi_dsi_regs.h | 3 +
>> 2 files changed, 68 insertions(+), 6 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 be6dbf19a24e..947c8e15fc4b 100644
>> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
>> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
>> @@ -37,7 +37,9 @@ MODULE_IMPORT_NS("RZV2H_CPG");
>>
>> #define RZG2L_DCS_BUF_SIZE 128 /* Maximum DCS buffer size in external memory. */
>>
>> +#define RZ_MIPI_DSI_MAX_INPUT 2
>> #define RZ_MIPI_DSI_FEATURE_16BPP BIT(0)
>> +#define RZ_MIPI_DSI_FEATURE_GPO0R BIT(1)
>>
>> struct rzg2l_mipi_dsi;
>>
>> @@ -81,13 +83,14 @@ struct rzg2l_mipi_dsi {
>> struct drm_bridge bridge;
>> struct drm_bridge *next_bridge;
>>
>> - struct clk *vclk;
>> + struct clk *vclk[RZ_MIPI_DSI_MAX_INPUT];
>> struct clk *lpclk;
>>
>> enum mipi_dsi_pixel_format format;
>> unsigned int num_data_lanes;
>> unsigned int lanes;
>> unsigned long mode_flags;
>> + u8 vclk_idx;
>>
>> struct rzv2h_dsi_mode_calc mode_calc;
>>
>> @@ -543,8 +546,8 @@ static int rzg2l_dphy_conf_clks(struct rzg2l_mipi_dsi *dsi, unsigned long mode_f
>> unsigned long vclk_rate;
>> unsigned int bpp;
>>
>> - clk_set_rate(dsi->vclk, mode_freq * KILO);
>> - vclk_rate = clk_get_rate(dsi->vclk);
>> + clk_set_rate(dsi->vclk[dsi->vclk_idx], mode_freq * KILO);
>> + vclk_rate = clk_get_rate(dsi->vclk[dsi->vclk_idx]);
>> if (vclk_rate != mode_freq * KILO)
>> dev_dbg(dsi->dev, "Requested vclk rate %lu, actual %lu mismatch\n",
>> mode_freq * KILO, vclk_rate);
>> @@ -687,6 +690,19 @@ static int rzv2h_mipi_dsi_dphy_init(struct rzg2l_mipi_dsi *dsi,
>> rzg2l_mipi_dsi_phy_write(dsi, PLLCLKSET1R,
>> FIELD_PREP(PLLCLKSET1R_PLL_K, dsi_parameters->k));
>>
>> + /*
>> + * From RZ/G3E HW manual (Rev.1.15) section 9.5.3 Operation,
>> + * 9.5.3.1 Power on Reset and Initial Settings for All Operations.
>> + * Figure 9.5-4 Power On/Off Sequence show that after writing to
>> + * GPO0R.VICH register we need to wait for more than 1 x tp before
>> + * writing to PLLENR.PLLEN.
>> + *
>> + * Note: GPO0R is a link register, not a PHY register. This setting
>> + * is specific to RZ/G3E.
>> + */
>> + if (dsi->info->features & RZ_MIPI_DSI_FEATURE_GPO0R)
>> + rzg2l_mipi_dsi_link_write(dsi, GPO0R, dsi->vclk_idx);
>> +
>> /*
>> * From RZ/V2H HW manual (Rev.1.20) section 9.5.3 Operation,
>> * (C) After write to D-PHY registers we need to wait for more than 1 x tp
>> @@ -1005,6 +1021,37 @@ static int rzg2l_mipi_dsi_stop_video(struct rzg2l_mipi_dsi *dsi)
>> return ret;
>> }
>>
>> +static int rzg2l_mipi_dsi_get_input_port(struct rzg2l_mipi_dsi *dsi)
>> +{
>> + struct device_node *np = dsi->dev->of_node;
>> + struct device_node *remote_ep, *ep_node;
>> + struct of_endpoint ep;
>> + bool ep_enabled;
>> + int in_port;
>> +
>> + /* DSI can have only one port enabled */
>
> Why is that ? The hardware supports dynamic input selection, why can't
> it be supported at runtime ?

For runtime/dynamic you mean using DT overlay??
like, remove:

Removing - DU0 --> DSI (input 0 | port@0 ) overlay and
install - DU1 --> DSI (input 1 | port@1 ) overlay and
viceversa?

Kind Regards,
Tommaso

>
>> + for_each_endpoint_of_node(np, ep_node) {
>> + of_graph_parse_endpoint(ep_node, &ep);
>> + if (ep.port >= RZ_MIPI_DSI_MAX_INPUT)
>> + break;
>> +
>> + remote_ep = of_graph_get_remote_endpoint(ep_node);
>> + ep_enabled = of_device_is_available(remote_ep);
>> + of_node_put(remote_ep);
>> +
>> + if (ep_enabled) {
>> + in_port = ep.port;
>> + break;
>> + }
>> + }
>> +
>> + if (!ep_enabled)
>> + return -EINVAL;
>> +
>> + dev_dbg(dsi->dev, "input port@%d\n", in_port);
>> + return in_port;
>> +}
>> +
>> /* -----------------------------------------------------------------------------
>> * Bridge
>> */
>> @@ -1425,9 +1472,21 @@ static int rzg2l_mipi_dsi_probe(struct platform_device *pdev)
>> if (IS_ERR(dsi->mmio))
>> return PTR_ERR(dsi->mmio);
>>
>> - dsi->vclk = devm_clk_get(dsi->dev, "vclk");
>> - if (IS_ERR(dsi->vclk))
>> - return PTR_ERR(dsi->vclk);
>> + dsi->vclk[0] = devm_clk_get(dsi->dev, "vclk");
>> + if (IS_ERR(dsi->vclk[0]))
>> + return PTR_ERR(dsi->vclk[0]);
>> +
>> + if (dsi->info->features & RZ_MIPI_DSI_FEATURE_GPO0R) {
>> + dsi->vclk[1] = devm_clk_get(dsi->dev, "vclk2");
>> + if (IS_ERR(dsi->vclk[1]))
>> + return PTR_ERR(dsi->vclk[1]);
>> +
>> + ret = rzg2l_mipi_dsi_get_input_port(dsi);
>> + if (ret < 0)
>> + return dev_err_probe(dsi->dev, -EINVAL,
>> + "No available input port\n");
>> + dsi->vclk_idx = ret;
>> + }
>>
>> dsi->lpclk = devm_clk_get(dsi->dev, "lpclk");
>> if (IS_ERR(dsi->lpclk))
>> diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi_regs.h b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi_regs.h
>> index 2bef20566648..cee2e0bc5dc5 100644
>> --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi_regs.h
>> +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi_regs.h
>> @@ -83,6 +83,9 @@
>> #define LINKSR_SQCHRUN1 BIT(4)
>> #define LINKSR_SQCHRUN0 BIT(0)
>>
>> +/* RZ/G3E General Purpose Output 0 Register */
>> +#define GPO0R 0xc0
>> +
>> /* Tx Set Register */
>> #define TXSETR 0x100
>> #define TXSETR_NUMLANECAP (0x3 << 16)
>