RE: [PATCH v3 04/13] drm: renesas: rzg2l_mipi_dsi: Add support for DSI PWRRDY

Hi Tommaso,

Thanks for the feedback.

> -----Original Message-----
> From: Tommaso Merciai <[email protected]>
> Sent: 27 August 2026 11:29
> Subject: Re: [PATCH v3 04/13] drm: renesas: rzg2l_mipi_dsi: Add support for DSI PWRRDY
> 
> Hi Biju,
> Thanks for your patch.
> 
> On Wed, Aug 26, 2026 at 06:48:38PM +0100, Biju wrote:
> > From: Biju Das <[email protected]>
> >
> > The register for controlling power to the DSI region is in the SYSC
> > (System Controller) block. Add support for controlling the DSI PWRRDY
> > signal so the driver can efficiently manage power to the DSI region.
> >
> > Signed-off-by: Biju Das <[email protected]>
> > ---
> > v2->v3:
> >  * Updated powerseq API names.
> > v1->v2:
> >  * Switched to power sequence for handling pwrrdy signal.
> >  * Dropped header file regmap.h and mfd/syscon.h
> >  * Added header file pwrseq/consumer.h
> > ---
> >  .../gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c    | 38 +++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >
> > 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 d67e1230b70c..7df5f4a4161b 100644
> > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/of_graph.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> > +#include <linux/pwrseq/consumer.h>
> >  #include <linux/reset.h>
> >  #include <linux/slab.h>
> >  #include <linux/units.h>
> > @@ -63,6 +64,7 @@ struct rzg2l_mipi_dsi_hw_info {
> >  	unsigned long max_dclk;
> >  	u16 activation_dly;
> >  	u8 features;
> > +	bool pwrrdy;
> >  };
> >
> >  struct rzv2h_dsi_mode_calc {
> > @@ -87,6 +89,8 @@ struct rzg2l_mipi_dsi {
> >  	struct clk *vclk;
> >  	struct clk *lpclk;
> >
> > +	struct pwrseq_desc *pwrseq;
> > +
> >  	enum mipi_dsi_pixel_format format;
> >  	unsigned int num_data_lanes;
> >  	unsigned int lanes;
> > @@ -1396,6 +1400,36 @@ static const struct dev_pm_ops rzg2l_mipi_pm_ops = {
> >   * Probe & Remove
> >   */
> >
> > +static void rzg2l_mipi_dsi_pwrrdy_off(void *data) {
> > +	pwrseq_disable(data);
> > +}
> > +
> > +static int rzg2l_mipi_dsi_pwrrdy_init(struct rzg2l_mipi_dsi *dsi) {
> > +	int ret;
> > +
> > +	if (!dsi->info->pwrrdy)
> 
> Maybe better to introduce a new feature flag like:
> 
> 	RZ_MIPI_DSI_FEATURE_PWRRDY

Ok.

> 
> And drop bool pwrrdy from rzg2l_mipi_dsi_hw_info?
> 
> > +		return 0;
> > +
> > +	dsi->pwrseq = devm_pwrseq_get(dsi->dev, "dsi-pwrrdy");
> > +	if (IS_ERR(dsi->pwrseq)) {
> > +		/*
> > +		 * This platform requires a sequencer. If we can't get it, we
> > +		 * must return the error (including -EPROBE_DEFER to wait for
> > +		 * the provider to appear)
> > +		 */
> > +		return dev_err_probe(dsi->dev, PTR_ERR(dsi->pwrseq),
> > +				     "Failed to get required power sequencer\n");
> > +	}
> > +
> > +	ret = pwrseq_enable(dsi->pwrseq);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return devm_add_action_or_reset(dsi->dev, rzg2l_mipi_dsi_pwrrdy_off,
> > +dsi->pwrseq);
> 
> I think we can drop devm_add_action_or_reset() as devm_pwrseq_get() already register devm_pwrseq_put() ->
> pwrseq_put() which already call pwrseq_disable() if desc->powered_on.
> 
> Then we can drop rzg2l_mipi_dsi_pwrrdy_off().

OK will fix this in next version.

Cheers,
Biju