Re: [PATCH v5 05/11] drm/panfrost: Move all device power up and down into RPM callbacks

[Boris Brezillon <boris.brezillon@collabora.com>]

On Tue, 11 Aug 2026 22:42:14 +0100
Adrián Larumbe <adrian.larumbe@collabora.com> wrote:

> During device probe(), failure to do a PM get() will leave the usage_count
> set to 0, which is the value assigned at device creation time. That means
> when the autosuspend delay expires, runtime suspend callback won't be
> invoked, so the device will remain powered on forever.
> 
> On top of that, failure to call PM put() during device unplug means
> Panfrost device's PM usage_count increases monotonically for every new
> module reload.
> 
> The combined outcome of both of the above was that devfreq OPP transition
> notifications would be printed all the time, even when no jobs are being
> submitted. This quickly fills the kernel ring buffer with junk.
> 
> Even direr than that was the fact MMU interrupts are only enabled when
> the device is reset, so after device probe() the very first job targeting
> the tiler heap BO would always time out, because the driver's PM runtime
> resume callback would not be invoked.
> 
> Fix all that by moving all GPU enabling and disabling into RPM resume and
> suspend callbacks, and making sure we resume the device right before
> touching any HW registers. This is done in imitation of the Panthor model.
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> Fixes: 635430797d3f ("drm/panfrost: Rework runtime PM initialization")
> Fixes: 876b15d2c88d ("drm/panfrost: Fix module unload")
> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c | 389 ++++++++++++++++-------------
>  drivers/gpu/drm/panfrost/panfrost_device.h |   8 +
>  drivers/gpu/drm/panfrost/panfrost_drv.c    |  11 +-
>  drivers/gpu/drm/panfrost/panfrost_gpu.c    |   9 +-
>  drivers/gpu/drm/panfrost/panfrost_gpu.h    |   1 -
>  drivers/gpu/drm/panfrost/panfrost_job.c    |   7 +-
>  drivers/gpu/drm/panfrost/panfrost_mmu.c    |   9 +-
>  drivers/gpu/drm/panfrost/panfrost_mmu.h    |   1 -
>  8 files changed, 236 insertions(+), 199 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index 74992deb0b3a..52f4b8c6a05f 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -26,11 +26,46 @@ static int panfrost_reset_init(struct panfrost_device *pfdev)
>  		return PTR_ERR(pfdev->rstc);
>  	}
>  
> -	return reset_control_deassert(pfdev->rstc);
> +	return 0;
>  }
>  
> -static void panfrost_reset_fini(struct panfrost_device *pfdev)
> +static int panfrost_clk_enable_deassert_reset(struct panfrost_device *pfdev)
>  {
> +	int err;
> +
> +	err = reset_control_deassert(pfdev->rstc);
> +	if (err)
> +		return err;
> +
> +	err = clk_enable(pfdev->clock);

I'd go for clk_prepare_enable() (and disable_unprepare() in
clk_disable_assert_reset()), just so you don't end up with a
prepare count lower than your enable count if the rpm get/put
section is covering the clk_fini() one. Note that the extra
prepare is cheap (just a refcnt increment since the clk has
been prepared already in clk_init()).

> +	if (err)
> +		goto assert_reset;
> +
> +	err = clk_enable(pfdev->bus_clock);
> +	if (err)
> +		goto disable_clock;
> +
> +	err = clk_enable(pfdev->bus_ace_clock);
> +	if (err)
> +		goto disable_bus_clock;
> +
> +	return 0;
> +
> +disable_bus_clock:
> +	clk_disable(pfdev->bus_clock);
> +disable_clock:
> +	clk_disable(pfdev->clock);
> +assert_reset:
> +	reset_control_assert(pfdev->rstc);
> +
> +	return err;
> +}
> +
> +static void panfrost_clk_disable_assert_reset(struct panfrost_device *pfdev)
> +{
> +	clk_disable(pfdev->bus_ace_clock);
> +	clk_disable(pfdev->bus_clock);
> +	clk_disable(pfdev->clock);
>  	reset_control_assert(pfdev->rstc);
>  }
>  
> @@ -48,7 +83,7 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
>  	rate = clk_get_rate(pfdev->clock);
>  	dev_info(pfdev->base.dev, "clock rate = %lu\n", rate);
>  
> -	err = clk_prepare_enable(pfdev->clock);
> +	err = clk_prepare(pfdev->clock);
>  	if (err)
>  		return err;
>  
> @@ -57,44 +92,55 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
>  		dev_err(pfdev->base.dev, "get bus_clock failed %ld\n",
>  			PTR_ERR(pfdev->bus_clock));
>  		err = PTR_ERR(pfdev->bus_clock);
> -		goto disable_clock;
> +		goto unprepare_clock;
>  	}
>  
>  	if (pfdev->bus_clock) {
>  		rate = clk_get_rate(pfdev->bus_clock);
>  		dev_info(pfdev->base.dev, "bus_clock rate = %lu\n", rate);
>  
> -		err = clk_prepare_enable(pfdev->bus_clock);
> +		err = clk_prepare(pfdev->bus_clock);
>  		if (err)
> -			goto disable_clock;
> +			goto unprepare_clock;
>  	}
>  
>  	pfdev->bus_ace_clock = devm_clk_get_optional(pfdev->base.dev, "bus_ace");
>  	if (IS_ERR(pfdev->bus_ace_clock)) {
>  		err = PTR_ERR(pfdev->bus_ace_clock);
>  		dev_err(pfdev->base.dev, "get bus_ace_clock failed %d\n", err);
> -		goto disable_bus_clock;
> +		goto unprepare_bus_clock;
>  	}
>  
> -	err = clk_prepare_enable(pfdev->bus_ace_clock);
> +	err = clk_prepare(pfdev->bus_ace_clock);
>  	if (err)
> -		goto disable_bus_clock;
> +		goto unprepare_bus_clock;
> +
> +	if (!(pfdev->comp->pm_features & BIT(GPU_PM_RT))) {
> +		err = panfrost_clk_enable_deassert_reset(pfdev);
> +		if (err)
> +			goto unprepare_bus_ace_clock;
> +	}
>  
>  	return 0;
>  
> -disable_bus_clock:
> -	clk_disable_unprepare(pfdev->bus_clock);
> -disable_clock:
> -	clk_disable_unprepare(pfdev->clock);
> +unprepare_bus_ace_clock:
> +	clk_unprepare(pfdev->bus_ace_clock);
> +unprepare_bus_clock:
> +	clk_unprepare(pfdev->bus_clock);
> +unprepare_clock:
> +	clk_unprepare(pfdev->clock);
>  
>  	return err;
>  }
>  
>  static void panfrost_clk_fini(struct panfrost_device *pfdev)
>  {
> -	clk_disable_unprepare(pfdev->bus_ace_clock);
> -	clk_disable_unprepare(pfdev->bus_clock);
> -	clk_disable_unprepare(pfdev->clock);
> +	if (!(pfdev->comp->pm_features & BIT(GPU_PM_RT)))
> +		panfrost_clk_disable_assert_reset(pfdev);
> +
> +	clk_unprepare(pfdev->bus_ace_clock);
> +	clk_unprepare(pfdev->bus_clock);
> +	clk_unprepare(pfdev->clock);
>  }
>  
>  static int panfrost_regulator_init(struct panfrost_device *pfdev)
> @@ -212,6 +258,127 @@ static int panfrost_pm_domain_init(struct panfrost_device *pfdev)
>  	return err;
>  }
>  
> +static int panfrost_device_runtime_resume(struct device *dev)
> +{
> +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
> +		ret = panfrost_clk_enable_deassert_reset(pfdev);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	panfrost_devfreq_resume(pfdev);
> +
> +	if (panfrost_device_started(pfdev))
> +		panfrost_device_reset(pfdev, true);
> +
> +	return 0;
> +}
> +
> +static int panfrost_device_runtime_suspend(struct device *dev)
> +{
> +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +
> +	if (panfrost_device_started(pfdev) &&
> +	    !panfrost_jm_is_idle(pfdev))

Uh, if this is being called and JM is not idle, there's a serious issue
that needs fixing (JM should hold a PM ref when it's active). It probably
deserves a WARN_ON()

> +		return -EBUSY;
> +
> +	panfrost_jm_suspend_irq(pfdev);
> +	panfrost_mmu_suspend_irq(pfdev);
> +	panfrost_gpu_suspend_irq(pfdev);
> +	panfrost_gpu_power_off(pfdev);
> +	panfrost_devfreq_suspend(pfdev);

I've seen extra checks added to panfrost_gpu_suspend_irq() to cover
for some early suspend call. What I think we should do instead is
skip those sub-component calls if the device is not fully initialized
(panfrost_device_started() == true). And then, in the _fini() helpers,
you make sure to suspend/disable stuff, so that, if they're called
in from the device_init() error path, things are undone as the should
without relying on the runtime PM stuff for that.

> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> +		panfrost_clk_disable_assert_reset(pfdev);
> +
> +	return 0;
> +}
> +
> +static int panfrost_device_resume(struct device *dev)
> +{
> +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) {
> +		unsigned long freq = pfdev->pfdevfreq.fast_rate;
> +		struct dev_pm_opp *opp;
> +
> +		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> +		if (IS_ERR(opp))
> +			return PTR_ERR(opp);
> +		dev_pm_opp_set_opp(dev, opp);
> +		dev_pm_opp_put(opp);
> +	}
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> +		ret = clk_enable(pfdev->clock);
> +		if (ret)
> +			goto err_clk;
> +
> +		if (pfdev->bus_clock) {
> +			ret = clk_enable(pfdev->bus_clock);
> +			if (ret)
> +				goto err_bus_clk;
> +		}
> +	}
> +
> +	ret = pm_runtime_force_resume(dev);
> +	if (ret)
> +		goto err_resume;
> +
> +	return 0;
> +
> +err_resume:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS) && pfdev->bus_clock)
> +		clk_disable(pfdev->bus_clock);
> +err_bus_clk:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS))
> +		clk_disable(pfdev->clock);
> +err_clk:
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> +		dev_pm_opp_set_opp(dev, NULL);
> +	return ret;
> +}
> +
> +static int panfrost_device_suspend(struct device *dev)
> +{
> +	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = pm_runtime_force_suspend(dev);
> +	if (ret)
> +		return ret;
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> +		if (pfdev->bus_clock)
> +			clk_disable(pfdev->bus_clock);
> +
> +		clk_disable(pfdev->clock);
> +	}
> +
> +	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> +		dev_pm_opp_set_opp(dev, NULL);
> +
> +	return 0;
> +}
> +
> +EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = {
> +	RUNTIME_PM_OPS(panfrost_device_runtime_suspend, panfrost_device_runtime_resume, NULL)
> +	SYSTEM_SLEEP_PM_OPS(panfrost_device_suspend, panfrost_device_resume)
> +};
> +
> +void panfrost_try_suspend_device(struct panfrost_device *pfdev)
> +{
> +	pm_runtime_put_sync_suspend(pfdev->base.dev);
> +
> +	/* If PM is disabled, we need to call the suspend handler manually. */
> +	if (!IS_ENABLED(CONFIG_PM))
> +		panfrost_device_runtime_suspend(pfdev->base.dev);

I'm not too sure it's wise to combine the two things in the same
helper. pm_runtime_put_sync_suspend() is the helper you call when
you're done interacting with the HW in some code section and want it to
enter suspend if there's no other users left.

The conditional !PM panfrost_device_runtime_suspend() call is supposed
to be in the device_remove() path in pair with the conditional
panfrost_device_runtime_resume() that exists in the probe() path.

> +}
> +
>  int panfrost_device_init(struct panfrost_device *pfdev)
>  {
>  	int err;
> @@ -242,7 +409,7 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>  	err = panfrost_clk_init(pfdev);
>  	if (err) {
>  		dev_err(pfdev->base.dev, "clk init failed %d\n", err);
> -		goto out_reset;
> +		goto out_pm_domain;
>  	}
>  
>  	err = panfrost_devfreq_init(pfdev);
> @@ -265,60 +432,70 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>  		goto out_regulator;
>  	}
>  
> -	err = panfrost_gpu_init(pfdev);
> +	err = devm_pm_runtime_enable(pfdev->base.dev);
>  	if (err)
>  		goto out_regulator;
>  
> -	err = panfrost_mmu_init(pfdev);
> +	err = pm_runtime_resume_and_get(pfdev->base.dev);

I don't see a pm_runtime_put to go with that runtime_get in the
device_init() function. To me, it looks like this keeps the device
active until the device_fini() function is called, which is not what we
want.

>  	if (err)
> -		goto out_gpu;
> +		goto out_regulator;
>  
> -	err = panfrost_jm_init(pfdev);
> +	/* If PM is disabled, we need to call panfrost_device_runtime_resume() manually. */
> +	if (!IS_ENABLED(CONFIG_PM)) {
> +		err = panfrost_device_runtime_resume(pfdev->base.dev);
> +		if (err)
> +			goto out_regulator;
> +	}
> +
> +	err = panfrost_gpu_init(pfdev);
>  	if (err)
> -		goto out_mmu;
> +		goto out_rpm_put;
> +
> +	err = panfrost_mmu_init(pfdev);
> +	if (err)
> +		goto out_rpm_put;
>  
>  	err = panfrost_perfcnt_init(pfdev);
>  	if (err)
> -		goto out_job;
> +		goto out_rpm_put;
>  
>  	err = panfrost_gem_init(pfdev);
>  	if (err)
>  		goto out_perfcnt;
>  
> +	err = panfrost_jm_init(pfdev);
> +	if (err)
> +		goto out_gem;
> +
>  	return 0;
>  
> +out_gem:
> +	panfrost_gem_fini(pfdev);
>  out_perfcnt:
>  	panfrost_perfcnt_fini(pfdev);
> -out_job:
> -	panfrost_jm_fini(pfdev);
> -out_mmu:
> -	panfrost_mmu_fini(pfdev);
> -out_gpu:
> -	panfrost_gpu_fini(pfdev);
> +out_rpm_put:
> +	panfrost_try_suspend_device(pfdev);
>  out_regulator:
>  	panfrost_regulator_fini(pfdev);
>  out_devfreq:
>  	panfrost_devfreq_fini(pfdev);
>  out_clk:
>  	panfrost_clk_fini(pfdev);
> -out_reset:
> -	panfrost_reset_fini(pfdev);
>  out_pm_domain:
>  	panfrost_pm_domain_fini(pfdev);
>  	return err;
>  }
>  
> -void panfrost_device_fini(struct panfrost_device *pfdev)
> +void
> +panfrost_device_fini(struct panfrost_device *pfdev)
>  {

There should be pm_runtime_resume_and_get() here...

> +	panfrost_jm_fini(pfdev);
>  	panfrost_gem_fini(pfdev);
>  	panfrost_perfcnt_fini(pfdev);
> -	panfrost_jm_fini(pfdev);
> -	panfrost_mmu_fini(pfdev);
> -	panfrost_gpu_fini(pfdev);
> -	panfrost_devfreq_fini(pfdev);

... and a pm_runtime_put_sync() here. We can probably even used a

	scoped_cond_guard(pm_runtime_active_try_enabled,
			  /* FIXME: can't resume fallback */,
			  pfdev->base.dev) {
	}

	/* If PM is disabled, we need to call the suspend handler manually. */
	if (!IS_ENABLED(CONFIG_PM))
		panfrost_device_runtime_suspend(pfdev->base.dev);

> +	panfrost_try_suspend_device(pfdev);
>  	panfrost_regulator_fini(pfdev);
> +	panfrost_devfreq_fini(pfdev);
>  	panfrost_clk_fini(pfdev);
> -	panfrost_reset_fini(pfdev);
>  	panfrost_pm_domain_fini(pfdev);
>  }
>  
> @@ -425,147 +602,9 @@ bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev,
>  void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int)
>  {
>  	panfrost_gpu_soft_reset(pfdev);
> -
>  	panfrost_gpu_power_on(pfdev);
>  	panfrost_mmu_reset(pfdev);
> -

Looks like unrelated cosmetic changes.

>  	panfrost_jm_reset_interrupts(pfdev);
>  	if (enable_job_int)
>  		panfrost_jm_enable_interrupts(pfdev);
>  }
> -
> -static int panfrost_device_runtime_resume(struct device *dev)
> -{
> -	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> -	int ret;
> -
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
> -		ret = reset_control_deassert(pfdev->rstc);
> -		if (ret)
> -			return ret;
> -
> -		ret = clk_enable(pfdev->clock);
> -		if (ret)
> -			goto err_clk;
> -
> -		ret = clk_enable(pfdev->bus_clock);
> -		if (ret)
> -			goto err_bus_clk;
> -
> -		ret = clk_enable(pfdev->bus_ace_clock);
> -		if (ret)
> -			goto err_bus_ace_clk;
> -	}
> -
> -	panfrost_device_reset(pfdev, true);
> -	panfrost_devfreq_resume(pfdev);
> -
> -	return 0;
> -
> -err_bus_ace_clk:
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> -		clk_disable(pfdev->bus_clock);
> -err_bus_clk:
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> -		clk_disable(pfdev->clock);
> -err_clk:
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT))
> -		reset_control_assert(pfdev->rstc);
> -	return ret;
> -}
> -
> -static int panfrost_device_runtime_suspend(struct device *dev)
> -{
> -	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> -
> -	if (!panfrost_jm_is_idle(pfdev))
> -		return -EBUSY;
> -
> -	panfrost_devfreq_suspend(pfdev);
> -	panfrost_jm_suspend_irq(pfdev);
> -	panfrost_mmu_suspend_irq(pfdev);
> -	panfrost_gpu_suspend_irq(pfdev);
> -	panfrost_gpu_power_off(pfdev);
> -
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) {
> -		clk_disable(pfdev->bus_ace_clock);
> -		clk_disable(pfdev->bus_clock);
> -		clk_disable(pfdev->clock);
> -		reset_control_assert(pfdev->rstc);
> -	}
> -
> -	return 0;
> -}
> -
> -static int panfrost_device_resume(struct device *dev)
> -{
> -	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> -	int ret;
> -
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF)) {
> -		unsigned long freq = pfdev->pfdevfreq.fast_rate;
> -		struct dev_pm_opp *opp;
> -
> -		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
> -		if (IS_ERR(opp))
> -			return PTR_ERR(opp);
> -		dev_pm_opp_set_opp(dev, opp);
> -		dev_pm_opp_put(opp);
> -	}
> -
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> -		ret = clk_enable(pfdev->clock);
> -		if (ret)
> -			goto err_clk;
> -
> -		if (pfdev->bus_clock) {
> -			ret = clk_enable(pfdev->bus_clock);
> -			if (ret)
> -				goto err_bus_clk;
> -		}
> -	}
> -
> -	ret = pm_runtime_force_resume(dev);
> -	if (ret)
> -		goto err_resume;
> -
> -	return 0;
> -
> -err_resume:
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS) && pfdev->bus_clock)
> -		clk_disable(pfdev->bus_clock);
> -err_bus_clk:
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS))
> -		clk_disable(pfdev->clock);
> -err_clk:
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> -		dev_pm_opp_set_opp(dev, NULL);
> -	return ret;
> -}
> -
> -static int panfrost_device_suspend(struct device *dev)
> -{
> -	struct panfrost_device *pfdev = dev_get_drvdata(dev);
> -	int ret;
> -
> -	ret = pm_runtime_force_suspend(dev);
> -	if (ret)
> -		return ret;
> -
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_CLK_DIS)) {
> -		if (pfdev->bus_clock)
> -			clk_disable(pfdev->bus_clock);
> -
> -		clk_disable(pfdev->clock);
> -	}
> -
> -	if (pfdev->comp->pm_features & BIT(GPU_PM_VREG_OFF))
> -		dev_pm_opp_set_opp(dev, NULL);
> -
> -	return 0;
> -}
> -
> -EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = {
> -	RUNTIME_PM_OPS(panfrost_device_runtime_suspend, panfrost_device_runtime_resume, NULL)
> -	SYSTEM_SLEEP_PM_OPS(panfrost_device_suspend, panfrost_device_resume)
> -};
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
> index ec55c136b1b6..0fd33bc5b86f 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.h
> @@ -253,6 +253,8 @@ int panfrost_device_init(struct panfrost_device *pfdev);
>  void panfrost_device_fini(struct panfrost_device *pfdev);
>  void panfrost_device_reset(struct panfrost_device *pfdev, bool enable_job_int);
>  
> +void panfrost_try_suspend_device(struct panfrost_device *pfdev);
> +
>  extern const struct dev_pm_ops panfrost_pm_ops;
>  
>  enum drm_panfrost_exception_type {
> @@ -342,4 +344,10 @@ panfrost_device_schedule_reset(struct panfrost_device *pfdev)
>  	queue_work(pfdev->reset.wq, &pfdev->reset.work);
>  }
>  
> +static inline bool
> +panfrost_device_started(struct panfrost_device *pfdev)
> +{
> +	return pfdev->js;
> +}
> +
>  #endif
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 331a3bd5b98c..8410de95e364 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -988,9 +988,6 @@ static int panfrost_probe(struct platform_device *pdev)
>  		goto err_out0;
>  	}
>  
> -	pm_runtime_set_active(pfdev->base.dev);
> -	pm_runtime_mark_last_busy(pfdev->base.dev);
> -	pm_runtime_enable(pfdev->base.dev);
>  	pm_runtime_set_autosuspend_delay(pfdev->base.dev, 50); /* ~3 frames */
>  	pm_runtime_use_autosuspend(pfdev->base.dev);

I'd recommend moving those to panfrost_device_init(), since this is were
the rest of the PM related initialization happens.

>  
> @@ -1002,13 +999,12 @@ static int panfrost_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto err_out1;
>  
> +	pm_runtime_put_autosuspend(pfdev->base.dev);

Oh, so here is the runtime_put_autosuspend() I was looking for
in panfrost_device_init().

>  
>  	return 0;
>  
>  err_out1:
> -	pm_runtime_disable(pfdev->base.dev);
>  	panfrost_device_fini(pfdev);
> -	pm_runtime_set_suspended(pfdev->base.dev);
>  err_out0:
>  	return err;
>  }
> @@ -1019,10 +1015,9 @@ static void panfrost_remove(struct platform_device *pdev)
>  
>  	drm_dev_unregister(&pfdev->base);
>  
> -	pm_runtime_get_sync(pfdev->base.dev);
> -	pm_runtime_disable(pfdev->base.dev);
> +	drm_WARN_ON(&pfdev->base, pm_runtime_get_sync(pfdev->base.dev) < 0);

And here's the pm_runtime_get() that was missing in panthor_device_fini().
Let's move anything PM related to device_{init,fini}() to clarify things.

Also, we probably want a resume_and_get() instead of get_sync(), and some
fallback in case the resume fails (though I'm not too sure what the fallback
could be).

> +
>  	panfrost_device_fini(pfdev);
> -	pm_runtime_set_suspended(pfdev->base.dev);

I assume this is now handled by the action registered by
devm_pm_runtime_enable().

>  }
>  
>  static ssize_t profiling_show(struct device *dev,
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> index 7d555e63e21a..0a3a68b561c9 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> @@ -509,7 +509,9 @@ void panfrost_gpu_suspend_irq(struct panfrost_device *pfdev)
>  	set_bit(PANFROST_COMP_BIT_GPU, pfdev->is_suspended);
>  
>  	gpu_write(pfdev, GPU_INT_MASK, 0);
> -	synchronize_irq(pfdev->gpu_irq);
> +
> +	if (pfdev->gpu_irq > 0)
> +		synchronize_irq(pfdev->gpu_irq);

As mentioned above, I don't think panfrost_gpu_suspend_irq() should be called
before panthor_gpu_init() (applies to all xxx_suspend_irq() helpers actually),
and that's something to address at the RPM implementation level (skip those
when the device is not yet initialized/started).