On 29/09/2026 04:44, Adrián Larumbe 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. > > Reviewed-by: Boris Brezillon <[email protected]> > Signed-off-by: Adrián Larumbe <[email protected]>
Reviewed-by: Steven Price <[email protected]> > --- > drivers/gpu/drm/panfrost/panfrost_device.c | 49 ++++++++++++++++++++++++++++ > drivers/gpu/drm/panfrost/panfrost_drv.c | 52 > +----------------------------- > 2 files changed, 50 insertions(+), 51 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c > b/drivers/gpu/drm/panfrost/panfrost_device.c > index 9f2b1967a398..c6bf3d0663df 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" > @@ -228,8 +229,15 @@ static int panfrost_pm_domain_init(struct > panfrost_device *pfdev) > > int panfrost_device_init(struct panfrost_device *pfdev) > { > + bool device_initialised = false; > 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 +299,35 @@ int panfrost_device_init(struct panfrost_device *pfdev) > if (err) > goto out_perfcnt; > > + device_initialised = true; > + > + /* The reason we must manually set the PM status and usage counter is > + * we have just powered the device up but did not go through the PM > + * runtime resume callback, so we need to update these ourselves. > + */ > + pm_runtime_get_noresume(pfdev->base.dev); > + 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); > + > + /* > + * 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); > + panfrost_gem_fini(pfdev); > out_perfcnt: > panfrost_perfcnt_fini(pfdev); > out_job: > @@ -311,11 +346,22 @@ int panfrost_device_init(struct panfrost_device *pfdev) > panfrost_reset_fini(pfdev); > out_pm_domain: > panfrost_pm_domain_fini(pfdev); > + > + if (device_initialised) { > + pm_runtime_set_suspended(pfdev->base.dev); > + pm_runtime_put_noidle(pfdev->base.dev); > + } > + > return err; > } > > 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); > + > panfrost_gem_fini(pfdev); > panfrost_perfcnt_fini(pfdev); > panfrost_jm_fini(pfdev); > @@ -326,6 +372,9 @@ void panfrost_device_fini(struct panfrost_device *pfdev) > panfrost_clk_fini(pfdev); > panfrost_reset_fini(pfdev); > panfrost_pm_domain_fini(pfdev); > + > + pm_runtime_set_suspended(pfdev->base.dev); > + pm_runtime_put_noidle(pfdev->base.dev); > } > > #define PANFROST_EXCEPTION(id) \ > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c > b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 907d4a14a0b5..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,50 +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; > - } > - > - /* The reason we must manually set the PM status and usage counter is > - * we have just powered the device up but did not go through the PM > - * runtime resume callback, so we need to update these ourselves. > - */ > - pm_runtime_get_noresume(pfdev->base.dev); > - 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); > - > - /* > - * 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); > - panfrost_device_fini(pfdev); > - pm_runtime_set_suspended(pfdev->base.dev); > - pm_runtime_put_noidle(pfdev->base.dev); > -err_out0: > - return err; > + return panfrost_device_init(pfdev); > } > > static void panfrost_remove(struct platform_device *pdev) > @@ -891,13 +847,7 @@ 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_disable(pfdev->base.dev); > panfrost_device_fini(pfdev); > - pm_runtime_set_suspended(pfdev->base.dev); > - pm_runtime_put_noidle(pfdev->base.dev); > - > } > > static ssize_t profiling_show(struct device *dev, >
