On 26/08/2019 23:33, Rob Herring wrote: > There's a few issues with the runtime PM initialization. > > The documentation states pm_runtime_set_active() should be called before > pm_runtime_enable(). The pm_runtime_put_autosuspend() could suspend the GPU > before panfrost_perfcnt_init() is called which touches the h/w. The > autosuspend delay keeps things from breaking. There's no need explicitly > power off the GPU only to wake back up with pm_runtime_get_sync(). Just > delaying pm_runtime_enable to the end of probe is sufficient. > > Lets move all the runtime PM calls into the probe() function so they are > all in one place and are done after all initialization. > > Cc: Tomeu Vizoso <tomeu.viz...@collabora.com> > Cc: Steven Price <steven.pr...@arm.com> > Cc: David Airlie <airl...@linux.ie> > Cc: Daniel Vetter <dan...@ffwll.ch> > Acked-by: Alyssa Rosenzweig <alyssa.rosenzw...@collabora.com> > Signed-off-by: Rob Herring <r...@kernel.org>
Reviewed-by: Steven Price <steven.pr...@arm.com> Steve > --- > v3: > - Move autosuspend setup after pm_runtime_enable as only autosuspend changes > trigger suspend. > - Fix autosuspend delay to 50ms instead of 0. > > drivers/gpu/drm/panfrost/panfrost_device.c | 9 --------- > drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++++++---- > 2 files changed, 6 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c > b/drivers/gpu/drm/panfrost/panfrost_device.c > index 4da71bb56c20..73805210834e 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c > @@ -5,7 +5,6 @@ > #include <linux/clk.h> > #include <linux/reset.h> > #include <linux/platform_device.h> > -#include <linux/pm_runtime.h> > #include <linux/regulator/consumer.h> > > #include "panfrost_device.h" > @@ -166,14 +165,6 @@ int panfrost_device_init(struct panfrost_device *pfdev) > if (err) > goto err_out4; > > - /* runtime PM will wake us up later */ > - panfrost_gpu_power_off(pfdev); > - > - pm_runtime_set_active(pfdev->dev); > - pm_runtime_get_sync(pfdev->dev); > - pm_runtime_mark_last_busy(pfdev->dev); > - pm_runtime_put_autosuspend(pfdev->dev); > - > err = panfrost_perfcnt_init(pfdev); > if (err) > goto err_out5; > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c > b/drivers/gpu/drm/panfrost/panfrost_drv.c > index d74442d71048..bc2ddeb55f5d 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -523,10 +523,6 @@ static int panfrost_probe(struct platform_device *pdev) > mutex_init(&pfdev->shrinker_lock); > INIT_LIST_HEAD(&pfdev->shrinker_list); > > - pm_runtime_use_autosuspend(pfdev->dev); > - pm_runtime_set_autosuspend_delay(pfdev->dev, 50); /* ~3 frames */ > - pm_runtime_enable(pfdev->dev); > - > err = panfrost_device_init(pfdev); > if (err) { > if (err != -EPROBE_DEFER) > @@ -541,6 +537,12 @@ static int panfrost_probe(struct platform_device *pdev) > goto err_out1; > } > > + pm_runtime_set_active(pfdev->dev); > + pm_runtime_mark_last_busy(pfdev->dev); > + pm_runtime_enable(pfdev->dev); > + pm_runtime_set_autosuspend_delay(pfdev->dev, 50); /* ~3 frames */ > + pm_runtime_use_autosuspend(pfdev->dev); > + > /* > * Register the DRM device with the core and the connectors with > * sysfs > -- > 2.20.1 > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel