[v4,75/79] media: ti-vpe: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

Use the new API, in order to cleanup the error check logic.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/platform/ti-vpe/cal-video.c | 4 +++-
 drivers/media/platform/ti-vpe/cal.c       | 8 +++++---
 drivers/media/platform/ti-vpe/vpe.c       | 4 +---
 3 files changed, 9 insertions(+), 7 deletions(-)

On Wed, 28 Apr 2021 16:52:36 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:

> Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")

> added pm_runtime_resume_and_get() in order to automatically handle

> dev->power.usage_count decrement on errors.

> 

> Use the new API, in order to cleanup the error check logic.

> 

> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

> ---

>  drivers/media/platform/ti-vpe/cal-video.c | 4 +++-

>  drivers/media/platform/ti-vpe/cal.c       | 8 +++++---

>  drivers/media/platform/ti-vpe/vpe.c       | 4 +---

>  3 files changed, 9 insertions(+), 7 deletions(-)

> 

> diff --git a/drivers/media/platform/ti-vpe/cal-video.c b/drivers/media/platform/ti-vpe/cal-video.c

> index 7b7436a355ee..15fb5360cf13 100644

> --- a/drivers/media/platform/ti-vpe/cal-video.c

> +++ b/drivers/media/platform/ti-vpe/cal-video.c

> @@ -700,7 +700,9 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)

>  

>  	addr = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);

>  

> -	pm_runtime_get_sync(ctx->cal->dev);

> +	ret = pm_runtime_resume_and_get(ctx->cal->dev);

> +	if (ret < 0)

> +		goto error_pipeline;

>  

>  	cal_ctx_set_dma_addr(ctx, addr);

>  	cal_ctx_start(ctx);

> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c

> index 2e2bef91b2b0..76fe7a8b33f6 100644

> --- a/drivers/media/platform/ti-vpe/cal.c

> +++ b/drivers/media/platform/ti-vpe/cal.c

> @@ -1024,7 +1024,7 @@ static int cal_probe(struct platform_device *pdev)

>  

>  	/* Read the revision and hardware info to verify hardware access. */

>  	pm_runtime_enable(&pdev->dev);

> -	ret = pm_runtime_get_sync(&pdev->dev);

> +	ret = pm_runtime_resume_and_get(&pdev->dev);

>  	if (ret)

>  		goto error_pm_runtime;

>  

> @@ -1098,10 +1098,11 @@ static int cal_remove(struct platform_device *pdev)

>  {

>  	struct cal_dev *cal = platform_get_drvdata(pdev);

>  	unsigned int i;

> +	int ret;

>  

>  	cal_dbg(1, cal, "Removing %s\n", CAL_MODULE_NAME);

>  

> -	pm_runtime_get_sync(&pdev->dev);

> +	ret = pm_runtime_resume_and_get(&pdev->dev);

>  

>  	cal_media_unregister(cal);

>  

> @@ -1115,7 +1116,8 @@ static int cal_remove(struct platform_device *pdev)

>  	for (i = 0; i < cal->data->num_csi2_phy; i++)

>  		cal_camerarx_destroy(cal->phy[i]);

>  

> -	pm_runtime_put_sync(&pdev->dev);

> +	if (ret >= 0)

> +		pm_runtime_put_sync(&pdev->dev);

>  	pm_runtime_disable(&pdev->dev);

>  

>  	return 0;

> diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c

> index 10251b787674..07cb2c140295 100644

> --- a/drivers/media/platform/ti-vpe/vpe.c

> +++ b/drivers/media/platform/ti-vpe/vpe.c

> @@ -2471,10 +2471,8 @@ static int vpe_runtime_get(struct platform_device *pdev)

>  

>  	dev_dbg(&pdev->dev, "vpe_runtime_get\n");

>  

> -	r = pm_runtime_get_sync(&pdev->dev);

> +	r = pm_runtime_resume_and_get(&pdev->dev);

>  	WARN_ON(r < 0);

> -	if (r)

> -		pm_runtime_put_noidle(&pdev->dev);

>  	return r < 0 ? r : 0;

r is <= 0 to zero so this isn't doing anything useful.

>  }

>

Em Fri, 30 Apr 2021 19:03:11 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> escreveu:

> On Wed, 28 Apr 2021 16:52:36 +0200

> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:


> > diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c

> > index 10251b787674..07cb2c140295 100644

> > --- a/drivers/media/platform/ti-vpe/vpe.c

> > +++ b/drivers/media/platform/ti-vpe/vpe.c

> > @@ -2471,10 +2471,8 @@ static int vpe_runtime_get(struct platform_device *pdev)

> >  

> >  	dev_dbg(&pdev->dev, "vpe_runtime_get\n");

> >  

> > -	r = pm_runtime_get_sync(&pdev->dev);

> > +	r = pm_runtime_resume_and_get(&pdev->dev);

> >  	WARN_ON(r < 0);

> > -	if (r)

> > -		pm_runtime_put_noidle(&pdev->dev);

> >  	return r < 0 ? r : 0;  

> r is <= 0 to zero so this isn't doing anything useful.


Not really. pm_runtime*get* routines may return positive values
as well. From Documentation/power/runtime_pm.rst:

  `int pm_runtime_resume(struct device *dev);`
    - execute the subsystem-level resume callback for the device; returns 0 on
      success, 1 if the device's runtime PM status was already 'active' or
      error code on failure, where -EAGAIN means it may be safe to attempt to
      resume the device again in future, but 'power.runtime_error' should be
      checked additionally, and -EACCES means that 'power.disable_depth' is
      different from 0

  `int pm_runtime_resume_and_get(struct device *dev);`
    - run pm_runtime_resume(dev) and if successful, increment the device's
      usage counter; return the result of pm_runtime_resume

  `int pm_runtime_get_sync(struct device *dev);`
    - increment the device's usage counter, run pm_runtime_resume(dev) and
      return its result

So, basically, if the device was already active, it would return 1.

Now, this is called as:

        ret = vpe_runtime_get(pdev);
        if (ret)
                goto rel_m2m;

So the logic could be simplified if the caller would be
doing, instead:

        ret = vpe_runtime_get(pdev);
        if (ret < 0)
                goto rel_m2m;

I'll do such change for the next version.

Thanks,
Mauro