On Sat, 12 Sep 2026 00:28:09 +0100
Adrián Larumbe <[email protected]> wrote:

> Ideally the probe() function will do as little as possible, and all device
> initialisation and registration should happen inside the panfrost device
> subsystem, just like it's done in Panthor. This also simplifies resource
> unwinding in the error path.
> 
> Do the same thing for DRM driver remove, as in, sweep most of the action
> into panfrost_device_fini(), just like we did for device probe.
> 
> Signed-off-by: Adrián Larumbe <[email protected]>

Reviewed-by: Boris Brezillon <[email protected]>

> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c | 37 +++++++++++++++++++++++
>  drivers/gpu/drm/panfrost/panfrost_drv.c    | 47 
> +-----------------------------
>  2 files changed, 38 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c 
> b/drivers/gpu/drm/panfrost/panfrost_device.c
> index 2fcbd223204c..ec2b1d29a26c 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -8,6 +8,7 @@
>  #include <linux/pm_domain.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regulator/consumer.h>
> +#include <drm/drm_drv.h>
>  
>  #include "panfrost_device.h"
>  #include "panfrost_devfreq.h"
> @@ -230,6 +231,12 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>  {
>       int err;
>  
> +     pfdev->comp = of_device_get_match_data(pfdev->base.dev);
> +     if (!pfdev->comp)
> +             return -ENODEV;
> +
> +     pfdev->coherent = device_get_dma_attr(pfdev->base.dev) == 
> DEV_DMA_COHERENT;
> +
>  #ifdef CONFIG_DEBUG_FS
>       mutex_init(&pfdev->debugfs.gems_lock);
>       INIT_LIST_HEAD(&pfdev->debugfs.gems_list);
> @@ -291,8 +298,31 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>       if (err)
>               goto out_perfcnt;
>  
> +     pm_runtime_set_active(pfdev->base.dev);
> +     pm_runtime_mark_last_busy(pfdev->base.dev);
> +     pm_runtime_get_noresume(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);
> +
> +     /*
> +      * Register the DRM device with the core and the connectors with
> +      * sysfs
> +      */
> +     err = drm_dev_register(&pfdev->base, 0);
> +     if (err < 0)
> +             goto err_disable_rpm;
> +
> +     pm_runtime_put_autosuspend(pfdev->base.dev);
> +
>       return 0;
>  
> +err_disable_rpm:
> +     pm_runtime_dont_use_autosuspend(pfdev->base.dev);
> +     pm_runtime_disable(pfdev->base.dev);
> +     pm_runtime_put_noidle(pfdev->base.dev);
> +     pm_runtime_set_suspended(pfdev->base.dev);
> +     panfrost_gem_fini(pfdev);
>  out_perfcnt:
>       panfrost_perfcnt_fini(pfdev);
>  out_job:
> @@ -316,6 +346,13 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>  
>  void panfrost_device_fini(struct panfrost_device *pfdev)
>  {
> +     pm_runtime_get_sync(pfdev->base.dev);
> +
> +     pm_runtime_dont_use_autosuspend(pfdev->base.dev);
> +     pm_runtime_disable(pfdev->base.dev);
> +     pm_runtime_put_noidle(pfdev->base.dev);
> +     pm_runtime_set_suspended(pfdev->base.dev);
> +
>       panfrost_gem_fini(pfdev);
>       panfrost_perfcnt_fini(pfdev);
>       panfrost_jm_fini(pfdev);
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c 
> b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index a3eff77add55..f77780c72a1a 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -830,7 +830,6 @@ static const struct drm_driver panfrost_drm_driver = {
>  static int panfrost_probe(struct platform_device *pdev)
>  {
>       struct panfrost_device *pfdev;
> -     int err;
>  
>       pfdev = devm_drm_dev_alloc(&pdev->dev, &panfrost_drm_driver,
>                                  struct panfrost_device, base);
> @@ -839,46 +838,7 @@ static int panfrost_probe(struct platform_device *pdev)
>  
>       platform_set_drvdata(pdev, pfdev);
>  
> -     pfdev->comp = of_device_get_match_data(&pdev->dev);
> -     if (!pfdev->comp)
> -             return -ENODEV;
> -
> -     pfdev->coherent = device_get_dma_attr(&pdev->dev) == DEV_DMA_COHERENT;
> -
> -     err = panfrost_device_init(pfdev);
> -     if (err) {
> -             if (err != -EPROBE_DEFER)
> -                     dev_err(&pdev->dev, "Fatal error during GPU init\n");
> -             goto err_out0;
> -     }
> -
> -     pm_runtime_set_active(pfdev->base.dev);
> -     pm_runtime_mark_last_busy(pfdev->base.dev);
> -     pm_runtime_get_noresume(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);
> -
> -     /*
> -      * Register the DRM device with the core and the connectors with
> -      * sysfs
> -      */
> -     err = drm_dev_register(&pfdev->base, 0);
> -     if (err < 0)
> -             goto err_out1;
> -
> -     pm_runtime_put_autosuspend(pfdev->base.dev);
> -
> -     return 0;
> -
> -err_out1:
> -     pm_runtime_dont_use_autosuspend(pfdev->base.dev);
> -     pm_runtime_disable(pfdev->base.dev);
> -     pm_runtime_put_noidle(pfdev->base.dev);
> -     pm_runtime_set_suspended(pfdev->base.dev);
> -     panfrost_device_fini(pfdev);
> -err_out0:
> -     return err;
> +     return panfrost_device_init(pfdev);
>  }
>  
>  static void panfrost_remove(struct platform_device *pdev)
> @@ -887,11 +847,6 @@ static void panfrost_remove(struct platform_device *pdev)
>  
>       drm_dev_unregister(&pfdev->base);
>  
> -     pm_runtime_get_sync(pfdev->base.dev);
> -     pm_runtime_dont_use_autosuspend(pfdev->base.dev);
> -     pm_runtime_put_noidle(pfdev->base.dev);
> -     pm_runtime_disable(pfdev->base.dev);
> -     pm_runtime_set_suspended(pfdev->base.dev);
>       panfrost_device_fini(pfdev);
>  }
>  
> 

Reply via email to