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

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

Hi Biju,
Thanks for your patch.

On Wed, Aug 26, 2026 at 06:48:38PM +0100, Biju wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
> 
> 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 <biju.das.jz@bp.renesas.com>
> ---
> 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

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().

Kind regards,
Tommaso

> +}
> +
>  static int rzg2l_mipi_dsi_probe(struct platform_device *pdev)
>  {
>  	unsigned int num_data_lanes;
> @@ -1454,6 +1488,10 @@ static int rzg2l_mipi_dsi_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, dsi);
>  
> +	ret = rzg2l_mipi_dsi_pwrrdy_init(dsi);
> +	if (ret)
> +		return ret;
> +
>  	pm_runtime_enable(dsi->dev);
>  
>  	ret = pm_runtime_resume_and_get(dsi->dev);
> -- 
> 2.43.0
>