During recent development of the Mali CSF GPU Panthor driver, a user
noticed that GPU frequency values as reported by fdinfo were
incorrect. This was traced back to incorrect handling of frequency value
updates. The same problem was seen in Panfrost.
Also one should consider GEM objects from a dma-buf import as being
resident in system memory, so that this can be reflected on fdinfo.
Adrián Larumbe (2):
drm/panfrost: Consider dma-buf imported objects as resident
drm/panfrost: Fix incorrect updating of current device frequency
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 17 +++++++++++++++--
drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +-
2 files changed, 16 insertions(+), 3 deletions(-)
base-commit: 38f922a563aac3148ac73e73689805917f034cb5
--
2.42.0
It was noticed when setting the Panfrost's DVFS device to the performant
governor, GPU frequency as reported by fdinfo had dropped to 0 permamently.
There are two separate issues causing this behaviour:
- Not initialising the device's current_frequency variable to its original
value during device probe().
- Updating said variable in Panfrost devfreq's get_dev_status() rather
than after the new OPP's frequency had been retrieved in target(), which
meant the old frequency would be assigned instead.
Signed-off-by: Adrián Larumbe <[email protected]>
Fixes: f11b0417eec2 ("drm/panfrost: Add fdinfo support GPU load metrics")
---
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index f59c82ea8870..2d30da38c2c3 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -29,14 +29,20 @@ static void panfrost_devfreq_update_utilization(struct panfrost_devfreq *pfdevfr
static int panfrost_devfreq_target(struct device *dev, unsigned long *freq,
u32 flags)
{
+ struct panfrost_device *ptdev = dev_get_drvdata(dev);
struct dev_pm_opp *opp;
+ int err;
opp = devfreq_recommended_opp(dev, freq, flags);
if (IS_ERR(opp))
return PTR_ERR(opp);
dev_pm_opp_put(opp);
- return dev_pm_opp_set_rate(dev, *freq);
+ err = dev_pm_opp_set_rate(dev, *freq);
+ if (!err)
+ ptdev->pfdevfreq.current_frequency = *freq;
+
+ return err;
}
static void panfrost_devfreq_reset(struct panfrost_devfreq *pfdevfreq)
@@ -58,7 +64,6 @@ static int panfrost_devfreq_get_dev_status(struct device *dev,
spin_lock_irqsave(&pfdevfreq->lock, irqflags);
panfrost_devfreq_update_utilization(pfdevfreq);
- pfdevfreq->current_frequency = status->current_frequency;
status->total_time = ktime_to_ns(ktime_add(pfdevfreq->busy_time,
pfdevfreq->idle_time));
@@ -164,6 +169,14 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
panfrost_devfreq_profile.initial_freq = cur_freq;
+ /*
+ * We could wait until panfrost_devfreq_target() to set this value, but
+ * since the simple_ondemand governor works asynchronously, there's a
+ * chance by the time someone opens the device's fdinfo file, current
+ * frequency hasn't been updated yet, so let's just do an early set.
+ */
+ pfdevfreq->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()
--
2.42.0
On 25/11/2023 20:52, Adrián Larumbe wrote:
> It was noticed when setting the Panfrost's DVFS device to the performant
> governor, GPU frequency as reported by fdinfo had dropped to 0 permamently.
>
> There are two separate issues causing this behaviour:
> - Not initialising the device's current_frequency variable to its original
> value during device probe().
> - Updating said variable in Panfrost devfreq's get_dev_status() rather
> than after the new OPP's frequency had been retrieved in target(), which
> meant the old frequency would be assigned instead.
Good catch - I'd not looked at the performance governor. I'd assumed
that one was "too simple to be wrong" ;)
Reviewed-by: Steven Price <[email protected]>
>
> Signed-off-by: Adrián Larumbe <[email protected]>
> Fixes: f11b0417eec2 ("drm/panfrost: Add fdinfo support GPU load metrics")
> ---
> drivers/gpu/drm/panfrost/panfrost_devfreq.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> index f59c82ea8870..2d30da38c2c3 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> @@ -29,14 +29,20 @@ static void panfrost_devfreq_update_utilization(struct panfrost_devfreq *pfdevfr
> static int panfrost_devfreq_target(struct device *dev, unsigned long *freq,
> u32 flags)
> {
> + struct panfrost_device *ptdev = dev_get_drvdata(dev);
> struct dev_pm_opp *opp;
> + int err;
>
> opp = devfreq_recommended_opp(dev, freq, flags);
> if (IS_ERR(opp))
> return PTR_ERR(opp);
> dev_pm_opp_put(opp);
>
> - return dev_pm_opp_set_rate(dev, *freq);
> + err = dev_pm_opp_set_rate(dev, *freq);
> + if (!err)
> + ptdev->pfdevfreq.current_frequency = *freq;
> +
> + return err;
> }
>
> static void panfrost_devfreq_reset(struct panfrost_devfreq *pfdevfreq)
> @@ -58,7 +64,6 @@ static int panfrost_devfreq_get_dev_status(struct device *dev,
> spin_lock_irqsave(&pfdevfreq->lock, irqflags);
>
> panfrost_devfreq_update_utilization(pfdevfreq);
> - pfdevfreq->current_frequency = status->current_frequency;
>
> status->total_time = ktime_to_ns(ktime_add(pfdevfreq->busy_time,
> pfdevfreq->idle_time));
> @@ -164,6 +169,14 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
>
> panfrost_devfreq_profile.initial_freq = cur_freq;
>
> + /*
> + * We could wait until panfrost_devfreq_target() to set this value, but
> + * since the simple_ondemand governor works asynchronously, there's a
> + * chance by the time someone opens the device's fdinfo file, current
> + * frequency hasn't been updated yet, so let's just do an early set.
> + */
> + pfdevfreq->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()
On 25/11/2023 20:52, Adrián Larumbe wrote:
> During recent development of the Mali CSF GPU Panthor driver, a user
> noticed that GPU frequency values as reported by fdinfo were
> incorrect. This was traced back to incorrect handling of frequency value
> updates. The same problem was seen in Panfrost.
>
> Also one should consider GEM objects from a dma-buf import as being
> resident in system memory, so that this can be reflected on fdinfo.
>
> Adrián Larumbe (2):
> drm/panfrost: Consider dma-buf imported objects as resident
> drm/panfrost: Fix incorrect updating of current device frequency
>
> drivers/gpu/drm/panfrost/panfrost_devfreq.c | 17 +++++++++++++++--
> drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +-
> 2 files changed, 16 insertions(+), 3 deletions(-)
>
>
> base-commit: 38f922a563aac3148ac73e73689805917f034cb5
Pushed to drm-misc-fixes
Thanks,
Steve