[PATCH 1/2] drm: rcar-du: Add Standby Mode support

[PATCH 1/2] drm: rcar-du: Add Standby Mode support

[Nguyen An Hoan]

From: Hoan Nguyen An <na-hoan@jinso.co.jp>

Add function support for Module Standby Mode

Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 26 ++++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h |  1 +
 2 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 90dacab..935cdd1 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -651,6 +651,32 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
 	rcar_du_group_start_stop(rcrtc->group, false);
 }
 
+int rcar_du_crtc_standby(struct rcar_du_crtc *rcrtc)
+{
+	u32 val_reg;
+	unsigned int i = 0;
+
+	/* Enter Standby mode */
+	rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_DRES | DSYSR_DEN, 0);
+
+	for(i = 100; i > 0; i--)
+	{
+		val_reg = rcar_du_crtc_read(rcrtc, DSSR);
+		if(!(val_reg & DSSR_VBK))
+			break;
+		udelay(10);
+	}
+
+	if(!(i > 0))
+		return -ETIMEDOUT;
+
+	/* Stop clock */
+	clk_set_rate(rcrtc->extclock, 0);
+	clk_set_rate(rcrtc->clock, 0);
+
+	return 0;
+}
+
 /* -----------------------------------------------------------------------------
  * CRTC Functions
  */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index 59ac6e7..6e0de42 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -99,6 +99,7 @@ enum rcar_du_output {
 
 int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
 			unsigned int hwindex);
+int rcar_du_crtc_standby(struct rcar_du_crtc *rcrtc);
 void rcar_du_crtc_suspend(struct rcar_du_crtc *rcrtc);
 void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc);
 
-- 
2.7.4

[PATCH 2/2] drm: rcar-du: Add Standby Mode to suspend()

[Nguyen An Hoan]

From: Hoan Nguyen An <na-hoan@jinso.co.jp>

Enter Standby Mode before suspend by drm
We will not need to "Release from Module Standby Mode"
because after resume() is called, this is done.

Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 32dba13..1ab3f56 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -426,6 +426,20 @@ static struct drm_driver rcar_du_driver = {
 static int rcar_du_pm_suspend(struct device *dev)
 {
 	struct rcar_du_device *rcdu = dev_get_drvdata(dev);
+	int ret;
+	unsigned int i = 0;
+
+	for (i = 0; i < rcdu->num_crtcs; i++)
+	{
+		struct rcar_du_crtc *rcrtc = &rcdu->crtcs[i];
+
+		ret = rcar_du_crtc_standby(rcrtc);
+		if(ret)
+		{
+			dev_err(dev, "failed to enter Standby mode\n");
+			return ret;
+		}
+	}
 
 	return drm_mode_config_helper_suspend(rcdu->ddev);
 }
-- 
2.7.4

Re: [PATCH 2/2] drm: rcar-du: Add Standby Mode to suspend()

[Laurent Pinchart]

Hello Hoan-san,

On Friday, 14 December 2018 09:25:18 EET Nguyen An Hoan wrote:
> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
> 
> Enter Standby Mode before suspend by drm
> We will not need to "Release from Module Standby Mode"
> because after resume() is called, this is done.
> 
> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 32dba13..1ab3f56 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -426,6 +426,20 @@ static struct drm_driver rcar_du_driver = {
>  static int rcar_du_pm_suspend(struct device *dev)
>  {
>  	struct rcar_du_device *rcdu = dev_get_drvdata(dev);
> +	int ret;
> +	unsigned int i = 0;
> +
> +	for (i = 0; i < rcdu->num_crtcs; i++)
> +	{
> +		struct rcar_du_crtc *rcrtc = &rcdu->crtcs[i];
> +
> +		ret = rcar_du_crtc_standby(rcrtc);
> +		if(ret)
> +		{
> +			dev_err(dev, "failed to enter Standby mode\n");
> +			return ret;
> +		}
> +	}

Could you please explain why this is needed ? The call to 
drm_mode_config_helper_suspend() should disable all CRTCs, so there shouldn't 
be a need to disable them manually first.

>  	return drm_mode_config_helper_suspend(rcdu->ddev);
>  }

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: rcar-du: Add Standby Mode support

[Sergei Shtylyov]

Hello!

On 14.12.2018 10:25, Nguyen An Hoan wrote:

> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
> 
> Add function support for Module Standby Mode
> 
> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> ---
>   drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 26 ++++++++++++++++++++++++++
>   drivers/gpu/drm/rcar-du/rcar_du_crtc.h |  1 +
>   2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 90dacab..935cdd1 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -651,6 +651,32 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>   	rcar_du_group_start_stop(rcrtc->group, false);
>   }
>   
> +int rcar_du_crtc_standby(struct rcar_du_crtc *rcrtc)
> +{
> +	u32 val_reg;
> +	unsigned int i = 0;
> +
> +	/* Enter Standby mode */
> +	rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_DRES | DSYSR_DEN, 0);
> +
> +	for(i = 100; i > 0; i--)
> +	{

    Please run your patches thru scripts/checkpatch.pl before posting.

> +		val_reg = rcar_du_crtc_read(rcrtc, DSSR);
> +		if(!(val_reg & DSSR_VBK))
> +			break;
> +		udelay(10);
> +	}
> +
> +	if(!(i > 0))
> +		return -ETIMEDOUT;
> +
> +	/* Stop clock */
> +	clk_set_rate(rcrtc->extclock, 0);
> +	clk_set_rate(rcrtc->clock, 0);
> +
> +	return 0;
> +}
> +
>   /* -----------------------------------------------------------------------------
>    * CRTC Functions
>    */
[...]

MBR, Sergei

Re: [PATCH 1/2] drm: rcar-du: Add Standby Mode support

[Geert Uytterhoeven]

Hi Hoan,

On Fri, Dec 14, 2018 at 8:25 AM Nguyen An Hoan <na-hoan@jinso.co.jp> wrote:
> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
>
> Add function support for Module Standby Mode
>
> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>

Thanks for your patch!

> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -651,6 +651,32 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>         rcar_du_group_start_stop(rcrtc->group, false);
>  }
>
> +int rcar_du_crtc_standby(struct rcar_du_crtc *rcrtc)
> +{
> +       u32 val_reg;
> +       unsigned int i = 0;
> +
> +       /* Enter Standby mode */
> +       rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_DRES | DSYSR_DEN, 0);
> +
> +       for(i = 100; i > 0; i--)
> +       {
> +               val_reg = rcar_du_crtc_read(rcrtc, DSSR);
> +               if(!(val_reg & DSSR_VBK))
> +                       break;
> +               udelay(10);
> +       }
> +
> +       if(!(i > 0))
> +               return -ETIMEDOUT;
> +
> +       /* Stop clock */
> +       clk_set_rate(rcrtc->extclock, 0);
> +       clk_set_rate(rcrtc->clock, 0);

That's not the API to stop a clock.
Why do you set the clock rates to 0?

> +
> +       return 0;
> +}

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds