| Message ID | 20241003133037.3398144-2-adrian.larumbe@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] drm/panfrost: Add missing OPP table refcnt decremental | expand |
On 03/10/2024 14:30, Adrián Larumbe wrote: > Make sure in case of errors between the first fetch of an OPP in > panthor_devfreq_init and its successive put, the error path decrements its > reference count to avoid OPP object leaks when removing the device. > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > Fixes: fac9b22df4b1 ("drm/panthor: Add the devfreq logical block") Reviewed-by: Steven Price <steven.price@arm.com> > --- > drivers/gpu/drm/panthor/panthor_devfreq.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c > index 9d0f891b9b53..ce0ac4563f65 100644 > --- a/drivers/gpu/drm/panthor/panthor_devfreq.c > +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c > @@ -197,7 +197,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev) > if (ret && ret != -ENODEV) { > if (ret != -EPROBE_DEFER) > DRM_DEV_ERROR(dev, "Couldn't retrieve/enable sram supply\n"); > - return ret; > + goto opp_err; > } > > /* > @@ -207,7 +207,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev) > ret = dev_pm_opp_set_opp(dev, opp); > if (ret) { > DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n"); > - return ret; > + goto opp_err; > } > > dev_pm_opp_put(opp); > @@ -242,6 +242,10 @@ int panthor_devfreq_init(struct panthor_device *ptdev) > DRM_DEV_INFO(dev, "Failed to register cooling device\n"); > > return 0; > + > +opp_err: > + dev_pm_opp_put(opp); > + return ret; > } > > int panthor_devfreq_resume(struct panthor_device *ptdev)
On Thu, 3 Oct 2024 14:30:29 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > Make sure in case of errors between the first fetch of an OPP in > panthor_devfreq_init and its successive put, the error path decrements its > reference count to avoid OPP object leaks when removing the device. > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > Fixes: fac9b22df4b1 ("drm/panthor: Add the devfreq logical block") > --- > drivers/gpu/drm/panthor/panthor_devfreq.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c > index 9d0f891b9b53..ce0ac4563f65 100644 > --- a/drivers/gpu/drm/panthor/panthor_devfreq.c > +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c > @@ -197,7 +197,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev) > if (ret && ret != -ENODEV) { > if (ret != -EPROBE_DEFER) > DRM_DEV_ERROR(dev, "Couldn't retrieve/enable sram supply\n"); > - return ret; > + goto opp_err; > } > > /* > @@ -207,7 +207,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev) > ret = dev_pm_opp_set_opp(dev, opp); > if (ret) { > DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n"); > - return ret; > + goto opp_err; > } > > dev_pm_opp_put(opp); > @@ -242,6 +242,10 @@ int panthor_devfreq_init(struct panthor_device *ptdev) > DRM_DEV_INFO(dev, "Failed to register cooling device\n"); > > return 0; > + > +opp_err: > + dev_pm_opp_put(opp); > + return ret; If you re-order things (see the following diff), you shouldn't need this error path. --->8--- diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c index 9d0f891b9b53..4f1a30f29c06 100644 --- a/drivers/gpu/drm/panthor/panthor_devfreq.c +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c @@ -163,13 +163,6 @@ int panthor_devfreq_init(struct panthor_device *ptdev) cur_freq = clk_get_rate(ptdev->clks.core); - opp = devfreq_recommended_opp(dev, &cur_freq, 0); - if (IS_ERR(opp)) - return PTR_ERR(opp); - - panthor_devfreq_profile.initial_freq = cur_freq; - ptdev->current_frequency = cur_freq; - /* Regulator coupling only takes care of synchronizing/balancing voltage * updates, but the coupled regulator needs to be enabled manually. * @@ -200,17 +193,24 @@ int panthor_devfreq_init(struct panthor_device *ptdev) return ret; } + opp = devfreq_recommended_opp(dev, &cur_freq, 0); + if (IS_ERR(opp)) + return PTR_ERR(opp); + + panthor_devfreq_profile.initial_freq = cur_freq; + ptdev->current_frequency = cur_freq; + /* * Set the recommend OPP this will enable and configure the regulator * if any and will avoid a switch off by regulator_late_cleanup() */ ret = dev_pm_opp_set_opp(dev, opp); + dev_pm_opp_put(opp); if (ret) { DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n"); return ret; } - dev_pm_opp_put(opp); /* Find the fastest defined rate */ opp = dev_pm_opp_find_freq_floor(dev, &freq);
On Thu, 3 Oct 2024 14:30:29 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > Make sure in case of errors between the first fetch of an OPP in > panthor_devfreq_init and its successive put, the error path decrements its > reference count to avoid OPP object leaks when removing the device. > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > Fixes: fac9b22df4b1 ("drm/panthor: Add the devfreq logical block") It doesn't apply on top of drm-misc-fixes. Could you send a v3 based on drm-misc-fixes please? > --- > drivers/gpu/drm/panthor/panthor_devfreq.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c > index 9d0f891b9b53..ce0ac4563f65 100644 > --- a/drivers/gpu/drm/panthor/panthor_devfreq.c > +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c > @@ -197,7 +197,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev) > if (ret && ret != -ENODEV) { > if (ret != -EPROBE_DEFER) > DRM_DEV_ERROR(dev, "Couldn't retrieve/enable sram supply\n"); > - return ret; > + goto opp_err; > } > > /* > @@ -207,7 +207,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev) > ret = dev_pm_opp_set_opp(dev, opp); > if (ret) { > DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n"); > - return ret; > + goto opp_err; > } > > dev_pm_opp_put(opp); > @@ -242,6 +242,10 @@ int panthor_devfreq_init(struct panthor_device *ptdev) > DRM_DEV_INFO(dev, "Failed to register cooling device\n"); > > return 0; > + > +opp_err: > + dev_pm_opp_put(opp); > + return ret; > } > > int panthor_devfreq_resume(struct panthor_device *ptdev)
On Wed, 30 Oct 2024 16:13:58 +0100 Boris Brezillon <boris.brezillon@collabora.com> wrote: > On Thu, 3 Oct 2024 14:30:29 +0100 > Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > > > Make sure in case of errors between the first fetch of an OPP in > > panthor_devfreq_init and its successive put, the error path decrements its > > reference count to avoid OPP object leaks when removing the device. > > > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > > Fixes: fac9b22df4b1 ("drm/panthor: Add the devfreq logical block") > > It doesn't apply on top of drm-misc-fixes. Could you send a v3 based > on drm-misc-fixes please? Sorry, I meant v4. v3 exists, but doesn't apply correctly on drm-misc-fixes. > > > --- > > drivers/gpu/drm/panthor/panthor_devfreq.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c > > index 9d0f891b9b53..ce0ac4563f65 100644 > > --- a/drivers/gpu/drm/panthor/panthor_devfreq.c > > +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c > > @@ -197,7 +197,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev) > > if (ret && ret != -ENODEV) { > > if (ret != -EPROBE_DEFER) > > DRM_DEV_ERROR(dev, "Couldn't retrieve/enable sram supply\n"); > > - return ret; > > + goto opp_err; > > } > > > > /* > > @@ -207,7 +207,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev) > > ret = dev_pm_opp_set_opp(dev, opp); > > if (ret) { > > DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n"); > > - return ret; > > + goto opp_err; > > } > > > > dev_pm_opp_put(opp); > > @@ -242,6 +242,10 @@ int panthor_devfreq_init(struct panthor_device *ptdev) > > DRM_DEV_INFO(dev, "Failed to register cooling device\n"); > > > > return 0; > > + > > +opp_err: > > + dev_pm_opp_put(opp); > > + return ret; > > } > > > > int panthor_devfreq_resume(struct panthor_device *ptdev) >
diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c b/drivers/gpu/drm/panthor/panthor_devfreq.c index 9d0f891b9b53..ce0ac4563f65 100644 --- a/drivers/gpu/drm/panthor/panthor_devfreq.c +++ b/drivers/gpu/drm/panthor/panthor_devfreq.c @@ -197,7 +197,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev) if (ret && ret != -ENODEV) { if (ret != -EPROBE_DEFER) DRM_DEV_ERROR(dev, "Couldn't retrieve/enable sram supply\n"); - return ret; + goto opp_err; } /* @@ -207,7 +207,7 @@ int panthor_devfreq_init(struct panthor_device *ptdev) ret = dev_pm_opp_set_opp(dev, opp); if (ret) { DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n"); - return ret; + goto opp_err; } dev_pm_opp_put(opp); @@ -242,6 +242,10 @@ int panthor_devfreq_init(struct panthor_device *ptdev) DRM_DEV_INFO(dev, "Failed to register cooling device\n"); return 0; + +opp_err: + dev_pm_opp_put(opp); + return ret; } int panthor_devfreq_resume(struct panthor_device *ptdev)
Make sure in case of errors between the first fetch of an OPP in panthor_devfreq_init and its successive put, the error path decrements its reference count to avoid OPP object leaks when removing the device. Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> Fixes: fac9b22df4b1 ("drm/panthor: Add the devfreq logical block") --- drivers/gpu/drm/panthor/panthor_devfreq.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)