Re: [PATCH 3/3] drm: renesas: rz-du: Add support for RZ/G3L LVDS encoder

[Claudiu Beznea]

Hi,

On 4/19/26 18:58, Dmitry Baryshkov wrote:
> On Fri, Apr 17, 2026 at 06:52:30PM +0100, Biju wrote:
> > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> >
> > Add support for the RZ/G3L LVDS encoder driver. It operates in single-link
> > mode with 4 lanes (Data) + 1 lane (Clock) and supports pixel clock rates
> > from 25 to 87 MHz. The LVDS module cannot be used at the same time as
> > MIPI-DSI. However, LVDS and the DSI interface share a peripheral clock and
> > the MIPI_DSI_PRESET_N reset signal. Also, the MIPI_DSI_CMN_RSTB and
> > MIPI_DSI_ARESET_N reset signals must be asserted before using the LVDS
> > module.
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@xxxxxxxxxxxxxx>
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---

[ ...]

> > +/* -----------------------------------------------------------------------------
> > + * Bridge
> > + */
> > +static void rzg3l_lvds_atomic_enable(struct drm_bridge *bridge,
> > +				     struct drm_atomic_state *state)
> > +{
> > +	struct rzg3l_lvds *lvds = bridge_to_rzg3l_lvds(bridge);
> > +	const struct drm_bridge_state *bridge_state;
> > +	int ret;
> > +	u32 fmt;
> > +
> > +	/* Get the LVDS format from the bridge state. */
> > +	bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
> > +	if (!bridge_state) {
> > +		dev_err(lvds->dev, "failed to get bridge state\n");
> > +		return;
> > +	}
> > +
> > +	switch (bridge_state->output_bus_cfg.format) {
> > +	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
> > +		fmt = RZG3L_LVDS_MODE_JEIDA;
> > +		break;
> > +	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
> > +		fmt = RZG3L_LVDS_MODE_VESA;
> > +		break;
> > +	default:
> > +		fmt = RZG3L_LVDS_MODE_VESA;
> > +		dev_warn(lvds->dev, "Unsupported bus fmt 0x%04x\n",
> > +			 bridge_state->output_bus_cfg.format);
> > +		break;
> > +	}
> > +
> > +	ret = pm_runtime_resume_and_get(lvds->dev);
>
> If this  fails for any reason, the atomic_disable() would still be
> called and it will decrement the counter, potentially undeflowing it.
> Consider switching to pm_runtime_get_sync(), which suits better here.

AFAIK, the clocks of this HW blocks have MSTOP functionality. HW manual of 
RZ/G3S [1] (should be the same for RZ/G3L as well) mentions the following in the 
chapter 41.2.1. "If the master accesses a module that has the clock stopped and 
the MSTOP bit set, a bus error will occur". [1]
MSTOP is set though the clock enable/disable APIs.

The clocks on RZ/G3L are part of clock power domains. If the 
pm_runtime_resume_and_get() fails (or any runtime PM resume calls), the clocks 
will be off and MSTOP set. In this case, calling atomic_disable() or any API 
setting HW registers will lead to sync aborts.

Thank you,
Claudiu

[1] 
https://renesasweb.s3.us-west-2.amazonaws.com/prod/docs/X014/X0141882/r01uh1014ej0120-rzg3s.pdf?response-content-disposition=inline%3B%20filename%3D%22r01uh1014ej0120-rzg3s.pdf%22&response-content-type=application%2Fpdf&X-Amz-Content-Sha256=UNSIGNED-PAYLOAD&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIA3WXFIBJWSDVTHJX3%2F20260421%2Fus-west-2%2Fs3%2Faws4_request&X-Amz-Date=20260421T075408Z&X-Amz-SignedHeaders=host&X-Amz-Expires=86700&X-Amz-Signature=435eb49a37a080094aecf291bc3dc2d5b7ee62d3de8075151082e9db6c6acbe7