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

[Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>]

Hi Laurent,
Thanks for your comments.

On 4/8/26 17:08, Laurent Pinchart wrote:
> On Wed, Apr 08, 2026 at 04:58:01PM +0200, Tommaso Merciai wrote:
>> On 4/8/26 16:17, Laurent Pinchart wrote:
>>> On Wed, Apr 08, 2026 at 04:12:22PM +0200, Tommaso Merciai wrote:
>>>> 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?
>>>
>>> No, I mean configurable by userspace, with two CRTCs sharing one DSI
>>> encoder.
>>
>> Sorry, question:
>>    - Is it possible to create CRTC from user space?
> 
> No, the CRTCs are created by the driver, but you can have one DRM device
> that covers two LCDCs, with one CRTC each, both connected to the same
> DSI encoder (and apparently this applies to the LVDS encoder too).
> Userspace then selects which CRTC drives which connector.

Which user space tool would you suggest I use for testing this?

And also, which user space tool is the user supposed to use at
runtime on his final/production system to perform that selection?


Kind Regards,
Tommaso


> 
>>   From hardware point only one DSI input is selectable out of 2 LCDC's at
>> a time.
>>
>> References:
>>    - 9.5.2.2.3 9.5 MIPI DSI Interface (DSI)
>>      General Purpose Output 0 Register (DSI_LINK_GPO0R)
>>
>>    - 9.5 MIPI DSI Interface (DSI)
>>      9.5.1.2 Block Diagram
>>      Figure 9.5-1 Video Input Interface
>>
>>>>>> +	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)
>