[RFC] drm/msm/adreno: Balance pm_runtime enable

[RFC] drm/msm/adreno: Balance pm_runtime enable

[Bjorn Andersson]

When any of the components in the mdss hierarchy fails to bind,
previously bound components are being unbound again.

One such case happens when the DP controller fails to find its bridge or
panel, where adreno_unbind() will be invoked without adreno_load_gpu()
being called to invoke pm_runtime_enable().

The result is that once everything is bound the pm_runtime_get_sync()
call find the power-domain to have a positive disable_depth, fails
with -EACCESS and prevents the GPU device to be powered up.

Move the pm_runtime_enable() to adreno_bind(), in order to balance it
with any calls to adreno_unbind().

Fixes: 4b18299b3365 ("drm/msm/adreno: Defer enabling runpm until hw_init()")
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 36f062c7582f..ca38b837dedb 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -432,15 +432,6 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
 	if (ret)
 		return NULL;
 
-	/*
-	 * Now that we have firmware loaded, and are ready to begin
-	 * booting the gpu, go ahead and enable runpm:
-	 */
-	pm_runtime_enable(&pdev->dev);
-
-	/* Make sure pm runtime is active and reset any previous errors */
-	pm_runtime_set_active(&pdev->dev);
-
 	ret = pm_runtime_get_sync(&pdev->dev);
 	if (ret < 0) {
 		pm_runtime_put_sync(&pdev->dev);
@@ -548,6 +539,8 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
 		return PTR_ERR(gpu);
 	}
 
+	pm_runtime_enable(dev);
+
 	return 0;
 }
 
-- 
2.25.1

Re: [RFC] drm/msm/adreno: Balance pm_runtime enable

[Rob Clark]

On Fri, Feb 3, 2023 at 10:12 AM Bjorn Andersson
<quic_bjorande@quicinc.com> wrote:
>
> When any of the components in the mdss hierarchy fails to bind,
> previously bound components are being unbound again.
>
> One such case happens when the DP controller fails to find its bridge or
> panel, where adreno_unbind() will be invoked without adreno_load_gpu()
> being called to invoke pm_runtime_enable().
>
> The result is that once everything is bound the pm_runtime_get_sync()
> call find the power-domain to have a positive disable_depth, fails
> with -EACCESS and prevents the GPU device to be powered up.
>
> Move the pm_runtime_enable() to adreno_bind(), in order to balance it
> with any calls to adreno_unbind().

I think instead we want to move where the pm_runtime_disable() is..
since pm_runtime_enable() was moved because we can't actually enable
runpm until we have GMU firmware

BR,
-R

> Fixes: 4b18299b3365 ("drm/msm/adreno: Defer enabling runpm until hw_init()")
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_device.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 36f062c7582f..ca38b837dedb 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -432,15 +432,6 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
>         if (ret)
>                 return NULL;
>
> -       /*
> -        * Now that we have firmware loaded, and are ready to begin
> -        * booting the gpu, go ahead and enable runpm:
> -        */
> -       pm_runtime_enable(&pdev->dev);
> -
> -       /* Make sure pm runtime is active and reset any previous errors */
> -       pm_runtime_set_active(&pdev->dev);
> -
>         ret = pm_runtime_get_sync(&pdev->dev);
>         if (ret < 0) {
>                 pm_runtime_put_sync(&pdev->dev);
> @@ -548,6 +539,8 @@ static int adreno_bind(struct device *dev, struct device *master, void *data)
>                 return PTR_ERR(gpu);
>         }
>
> +       pm_runtime_enable(dev);
> +
>         return 0;
>  }
>
> --
> 2.25.1
>

Re: [RFC] drm/msm/adreno: Balance pm_runtime enable

[Johan Hovold]

On Fri, Feb 10, 2023 at 08:10:00AM -0800, Rob Clark wrote:
> On Fri, Feb 3, 2023 at 10:12 AM Bjorn Andersson
> <quic_bjorande@quicinc.com> wrote:
> >
> > When any of the components in the mdss hierarchy fails to bind,
> > previously bound components are being unbound again.
> >
> > One such case happens when the DP controller fails to find its bridge or
> > panel, where adreno_unbind() will be invoked without adreno_load_gpu()
> > being called to invoke pm_runtime_enable().
> >
> > The result is that once everything is bound the pm_runtime_get_sync()
> > call find the power-domain to have a positive disable_depth, fails
> > with -EACCESS and prevents the GPU device to be powered up.
> >
> > Move the pm_runtime_enable() to adreno_bind(), in order to balance it
> > with any calls to adreno_unbind().
> 
> I think instead we want to move where the pm_runtime_disable() is..
> since pm_runtime_enable() was moved because we can't actually enable
> runpm until we have GMU firmware

Here's a fix for this issue which does not move pm_runtime_enable() from
first open():

	https://lore.kernel.org/lkml/20230221101430.14546-1-johan+linaro@kernel.org/

Johan