diff mbox series

[v2,1/2] drm/panfrost: Add missing OPP table refcnt decremental

Message ID 20241003133037.3398144-1-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

Commit Message

Adrián Larumbe Oct. 3, 2024, 1:30 p.m. UTC
Commit f11b0417eec2 ("drm/panfrost: Add fdinfo support GPU load metrics")
retrieves the OPP for the maximum device clock frequency, but forgets to
keep the reference count balanced by putting the returned OPP object. This
eventually leads to an OPP core warning when removing the device.

Fix it by putting OPP objects as many times as they're retrieved. That
includes putting the OPP object in case setting it fails.

Also remove an unnecessary whitespace.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
Fixes: f11b0417eec2 ("drm/panfrost: Add fdinfo support GPU load metrics")
---
 drivers/gpu/drm/panfrost/panfrost_devfreq.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Steven Price Oct. 3, 2024, 2:50 p.m. UTC | #1
On 03/10/2024 14:30, Adrián Larumbe wrote:
> Commit f11b0417eec2 ("drm/panfrost: Add fdinfo support GPU load metrics")
> retrieves the OPP for the maximum device clock frequency, but forgets to
> keep the reference count balanced by putting the returned OPP object. This
> eventually leads to an OPP core warning when removing the device.
> 
> Fix it by putting OPP objects as many times as they're retrieved. That
> includes putting the OPP object in case setting it fails.
> 
> Also remove an unnecessary whitespace.
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> Fixes: f11b0417eec2 ("drm/panfrost: Add fdinfo support GPU load metrics")

Reviewed-by: Steven Price <steven.price@arm.com>

> ---
>  drivers/gpu/drm/panfrost/panfrost_devfreq.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> index 2d30da38c2c3..4b8840a41779 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> @@ -38,7 +38,7 @@ static int panfrost_devfreq_target(struct device *dev, unsigned long *freq,
>  		return PTR_ERR(opp);
>  	dev_pm_opp_put(opp);
>  
> -	err =  dev_pm_opp_set_rate(dev, *freq);
> +	err = dev_pm_opp_set_rate(dev, *freq);
>  	if (!err)
>  		ptdev->pfdevfreq.current_frequency = *freq;
>  
> @@ -184,9 +184,12 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
>  	ret = dev_pm_opp_set_opp(dev, opp);
>  	if (ret) {
>  		DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n");
> +		dev_pm_opp_put(opp);
>  		return ret;
>  	}
>  
> +	dev_pm_opp_put(opp);
> +
>  	/* Find the fastest defined rate  */
>  	opp = dev_pm_opp_find_freq_floor(dev, &freq);
>  	if (IS_ERR(opp))
Boris Brezillon Oct. 3, 2024, 2:56 p.m. UTC | #2
On Thu,  3 Oct 2024 14:30:28 +0100
Adrián Larumbe <adrian.larumbe@collabora.com> wrote:

> Commit f11b0417eec2 ("drm/panfrost: Add fdinfo support GPU load metrics")
> retrieves the OPP for the maximum device clock frequency, but forgets to
> keep the reference count balanced by putting the returned OPP object. This
> eventually leads to an OPP core warning when removing the device.
> 
> Fix it by putting OPP objects as many times as they're retrieved. That
> includes putting the OPP object in case setting it fails.
> 
> Also remove an unnecessary whitespace.
> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> Fixes: f11b0417eec2 ("drm/panfrost: Add fdinfo support GPU load metrics")
> ---
>  drivers/gpu/drm/panfrost/panfrost_devfreq.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> index 2d30da38c2c3..4b8840a41779 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> @@ -38,7 +38,7 @@ static int panfrost_devfreq_target(struct device *dev, unsigned long *freq,
>  		return PTR_ERR(opp);
>  	dev_pm_opp_put(opp);
>  
> -	err =  dev_pm_opp_set_rate(dev, *freq);
> +	err = dev_pm_opp_set_rate(dev, *freq);
>  	if (!err)
>  		ptdev->pfdevfreq.current_frequency = *freq;
>  
> @@ -184,9 +184,12 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
>  	ret = dev_pm_opp_set_opp(dev, opp);

Put the dev_pm_opp_put(opp) here, and you can drop the one in the error
path.

>  	if (ret) {
>  		DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n");
> +		dev_pm_opp_put(opp);
>  		return ret;
>  	}
>  
> +	dev_pm_opp_put(opp);
> +
>  	/* Find the fastest defined rate  */
>  	opp = dev_pm_opp_find_freq_floor(dev, &freq);
>  	if (IS_ERR(opp))
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 2d30da38c2c3..4b8840a41779 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -38,7 +38,7 @@  static int panfrost_devfreq_target(struct device *dev, unsigned long *freq,
 		return PTR_ERR(opp);
 	dev_pm_opp_put(opp);
 
-	err =  dev_pm_opp_set_rate(dev, *freq);
+	err = dev_pm_opp_set_rate(dev, *freq);
 	if (!err)
 		ptdev->pfdevfreq.current_frequency = *freq;
 
@@ -184,9 +184,12 @@  int panfrost_devfreq_init(struct panfrost_device *pfdev)
 	ret = dev_pm_opp_set_opp(dev, opp);
 	if (ret) {
 		DRM_DEV_ERROR(dev, "Couldn't set recommended OPP\n");
+		dev_pm_opp_put(opp);
 		return ret;
 	}
 
+	dev_pm_opp_put(opp);
+
 	/* Find the fastest defined rate  */
 	opp = dev_pm_opp_find_freq_floor(dev, &freq);
 	if (IS_ERR(opp))