Hi,
This serie cleans and adds regulator support to Panfrost devfreq. This is mostly based on comment for the freshly introduced lima devfreq.
We need to add regulator support because on Allwinner the GPU OPP table defines both frequencies and voltages.
First patches [01-07] should not change the actual behavior and introduce a proper panfrost_devfreq struct.
Regards, Clément
Clément Péron (14): drm/panfrost: avoid static declaration drm/panfrost: clean headers in devfreq drm/panfrost: don't use pfdevfreq.busy_count to know if hw is idle drm/panfrost: introduce panfrost_devfreq struct drm/panfrost: use spinlock instead of atomic drm/panfrost: properly handle error in probe drm/panfrost: rename error labels in device_init drm/panfrost: move devfreq_init()/fini() in device drm/panfrost: dynamically alloc regulators drm/panfrost: add regulators to devfreq arm64: defconfig: Enable devfreq cooling device arm64: dts: allwinner: h6: Add cooling map for GPU [DO NOT MERGE] arm64: dts: allwinner: h6: Add GPU OPP table [DO NOT MERGE] arm64: dts: allwinner: force GPU regulator to be always
.../dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 + arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 102 +++++++++++ arch/arm64/configs/defconfig | 1 + drivers/gpu/drm/panfrost/panfrost_devfreq.c | 165 ++++++++++++------ drivers/gpu/drm/panfrost/panfrost_devfreq.h | 30 +++- drivers/gpu/drm/panfrost/panfrost_device.c | 61 ++++--- drivers/gpu/drm/panfrost/panfrost_device.h | 14 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 15 +- drivers/gpu/drm/panfrost/panfrost_job.c | 10 +- 9 files changed, 290 insertions(+), 109 deletions(-)
This declaration can be avoided so change it.
Signed-off-by: Clément Péron peron.clem@gmail.com Reviewed-by: Steven Price steven.price@arm.com --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 38 ++++++++++----------- 1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index 413987038fbf..1b560b903ea6 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -14,7 +14,24 @@ #include "panfrost_gpu.h" #include "panfrost_regs.h"
-static void panfrost_devfreq_update_utilization(struct panfrost_device *pfdev); +static void panfrost_devfreq_update_utilization(struct panfrost_device *pfdev) +{ + ktime_t now; + ktime_t last; + + if (!pfdev->devfreq.devfreq) + return; + + now = ktime_get(); + last = pfdev->devfreq.time_last_update; + + if (atomic_read(&pfdev->devfreq.busy_count) > 0) + pfdev->devfreq.busy_time += ktime_sub(now, last); + else + pfdev->devfreq.idle_time += ktime_sub(now, last); + + pfdev->devfreq.time_last_update = now; +}
static int panfrost_devfreq_target(struct device *dev, unsigned long *freq, u32 flags) @@ -139,25 +156,6 @@ void panfrost_devfreq_suspend(struct panfrost_device *pfdev) devfreq_suspend_device(pfdev->devfreq.devfreq); }
-static void panfrost_devfreq_update_utilization(struct panfrost_device *pfdev) -{ - ktime_t now; - ktime_t last; - - if (!pfdev->devfreq.devfreq) - return; - - now = ktime_get(); - last = pfdev->devfreq.time_last_update; - - if (atomic_read(&pfdev->devfreq.busy_count) > 0) - pfdev->devfreq.busy_time += ktime_sub(now, last); - else - pfdev->devfreq.idle_time += ktime_sub(now, last); - - pfdev->devfreq.time_last_update = now; -} - void panfrost_devfreq_record_busy(struct panfrost_device *pfdev) { panfrost_devfreq_update_utilization(pfdev);
Don't include not required headers and sort them.
Signed-off-by: Clément Péron peron.clem@gmail.com Reviewed-by: Steven Price steven.price@arm.com --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index 1b560b903ea6..df7b71da9a84 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -1,18 +1,14 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright 2019 Collabora ltd. */ + +#include <linux/clk.h> #include <linux/devfreq.h> #include <linux/devfreq_cooling.h> #include <linux/platform_device.h> #include <linux/pm_opp.h> -#include <linux/clk.h> -#include <linux/regulator/consumer.h>
#include "panfrost_device.h" #include "panfrost_devfreq.h" -#include "panfrost_features.h" -#include "panfrost_issues.h" -#include "panfrost_gpu.h" -#include "panfrost_regs.h"
static void panfrost_devfreq_update_utilization(struct panfrost_device *pfdev) {
This use devfreq variable that will be lock with spinlock in future patches. We should either introduce a function to access this one but as devfreq is optional let's just remove it.
Signed-off-by: Clément Péron peron.clem@gmail.com Reviewed-by: Steven Price steven.price@arm.com --- drivers/gpu/drm/panfrost/panfrost_job.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 7914b1570841..63e32a9f2749 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -581,10 +581,6 @@ int panfrost_job_is_idle(struct panfrost_device *pfdev) struct panfrost_job_slot *js = pfdev->js; int i;
- /* Check whether the hardware is idle */ - if (atomic_read(&pfdev->devfreq.busy_count)) - return false; - for (i = 0; i < NUM_JOB_SLOTS; i++) { /* If there are any jobs in the HW queue, we're not idle */ if (atomic_read(&js->queue[i].sched.hw_rq_count))
Introduce a proper panfrost_devfreq to deal with devfreq variables.
Signed-off-by: Clément Péron peron.clem@gmail.com Reviewed-by: Steven Price steven.price@arm.com --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 76 ++++++++++++--------- drivers/gpu/drm/panfrost/panfrost_devfreq.h | 20 +++++- drivers/gpu/drm/panfrost/panfrost_device.h | 11 +-- drivers/gpu/drm/panfrost/panfrost_job.c | 6 +- 4 files changed, 66 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index df7b71da9a84..962550363391 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -10,23 +10,23 @@ #include "panfrost_device.h" #include "panfrost_devfreq.h"
-static void panfrost_devfreq_update_utilization(struct panfrost_device *pfdev) +static void panfrost_devfreq_update_utilization(struct panfrost_devfreq *pfdevfreq) { ktime_t now; ktime_t last;
- if (!pfdev->devfreq.devfreq) + if (!pfdevfreq->devfreq) return;
now = ktime_get(); - last = pfdev->devfreq.time_last_update; + last = pfdevfreq->time_last_update;
- if (atomic_read(&pfdev->devfreq.busy_count) > 0) - pfdev->devfreq.busy_time += ktime_sub(now, last); + if (atomic_read(&pfdevfreq->busy_count) > 0) + pfdevfreq->busy_time += ktime_sub(now, last); else - pfdev->devfreq.idle_time += ktime_sub(now, last); + pfdevfreq->idle_time += ktime_sub(now, last);
- pfdev->devfreq.time_last_update = now; + pfdevfreq->time_last_update = now; }
static int panfrost_devfreq_target(struct device *dev, unsigned long *freq, @@ -47,30 +47,31 @@ static int panfrost_devfreq_target(struct device *dev, unsigned long *freq, return 0; }
-static void panfrost_devfreq_reset(struct panfrost_device *pfdev) +static void panfrost_devfreq_reset(struct panfrost_devfreq *pfdevfreq) { - pfdev->devfreq.busy_time = 0; - pfdev->devfreq.idle_time = 0; - pfdev->devfreq.time_last_update = ktime_get(); + pfdevfreq->busy_time = 0; + pfdevfreq->idle_time = 0; + pfdevfreq->time_last_update = ktime_get(); }
static int panfrost_devfreq_get_dev_status(struct device *dev, struct devfreq_dev_status *status) { struct panfrost_device *pfdev = dev_get_drvdata(dev); + struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq;
- panfrost_devfreq_update_utilization(pfdev); + panfrost_devfreq_update_utilization(pfdevfreq);
status->current_frequency = clk_get_rate(pfdev->clock); - status->total_time = ktime_to_ns(ktime_add(pfdev->devfreq.busy_time, - pfdev->devfreq.idle_time)); + status->total_time = ktime_to_ns(ktime_add(pfdevfreq->busy_time, + pfdevfreq->idle_time));
- status->busy_time = ktime_to_ns(pfdev->devfreq.busy_time); + status->busy_time = ktime_to_ns(pfdevfreq->busy_time);
- panfrost_devfreq_reset(pfdev); + panfrost_devfreq_reset(pfdevfreq);
- dev_dbg(pfdev->dev, "busy %lu total %lu %lu %% freq %lu MHz\n", status->busy_time, - status->total_time, + dev_dbg(pfdev->dev, "busy %lu total %lu %lu %% freq %lu MHz\n", + status->busy_time, status->total_time, status->busy_time / (status->total_time / 100), status->current_frequency / 1000 / 1000);
@@ -91,6 +92,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) struct device *dev = &pfdev->pdev->dev; struct devfreq *devfreq; struct thermal_cooling_device *cooling; + struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq;
ret = dev_pm_opp_of_add_table(dev); if (ret == -ENODEV) /* Optional, continue without devfreq */ @@ -98,7 +100,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) else if (ret) return ret;
- panfrost_devfreq_reset(pfdev); + panfrost_devfreq_reset(pfdevfreq);
cur_freq = clk_get_rate(pfdev->clock);
@@ -116,53 +118,59 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) dev_pm_opp_of_remove_table(dev); return PTR_ERR(devfreq); } - pfdev->devfreq.devfreq = devfreq; + pfdevfreq->devfreq = devfreq;
cooling = of_devfreq_cooling_register(dev->of_node, devfreq); if (IS_ERR(cooling)) DRM_DEV_INFO(dev, "Failed to register cooling device\n"); else - pfdev->devfreq.cooling = cooling; + pfdevfreq->cooling = cooling;
return 0; }
void panfrost_devfreq_fini(struct panfrost_device *pfdev) { - if (pfdev->devfreq.cooling) - devfreq_cooling_unregister(pfdev->devfreq.cooling); + struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq; + + if (pfdevfreq->cooling) + devfreq_cooling_unregister(pfdevfreq->cooling); dev_pm_opp_of_remove_table(&pfdev->pdev->dev); }
void panfrost_devfreq_resume(struct panfrost_device *pfdev) { - if (!pfdev->devfreq.devfreq) + struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq; + + if (!pfdevfreq->devfreq) return;
- panfrost_devfreq_reset(pfdev); + panfrost_devfreq_reset(pfdevfreq);
- devfreq_resume_device(pfdev->devfreq.devfreq); + devfreq_resume_device(pfdevfreq->devfreq); }
void panfrost_devfreq_suspend(struct panfrost_device *pfdev) { - if (!pfdev->devfreq.devfreq) + struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq; + + if (!pfdevfreq->devfreq) return;
- devfreq_suspend_device(pfdev->devfreq.devfreq); + devfreq_suspend_device(pfdevfreq->devfreq); }
-void panfrost_devfreq_record_busy(struct panfrost_device *pfdev) +void panfrost_devfreq_record_busy(struct panfrost_devfreq *pfdevfreq) { - panfrost_devfreq_update_utilization(pfdev); - atomic_inc(&pfdev->devfreq.busy_count); + panfrost_devfreq_update_utilization(pfdevfreq); + atomic_inc(&pfdevfreq->busy_count); }
-void panfrost_devfreq_record_idle(struct panfrost_device *pfdev) +void panfrost_devfreq_record_idle(struct panfrost_devfreq *pfdevfreq) { int count;
- panfrost_devfreq_update_utilization(pfdev); - count = atomic_dec_if_positive(&pfdev->devfreq.busy_count); + panfrost_devfreq_update_utilization(pfdevfreq); + count = atomic_dec_if_positive(&pfdevfreq->busy_count); WARN_ON(count < 0); } diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.h b/drivers/gpu/drm/panfrost/panfrost_devfreq.h index 0611beffc8d0..0697f8d5aa34 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.h +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.h @@ -4,13 +4,29 @@ #ifndef __PANFROST_DEVFREQ_H__ #define __PANFROST_DEVFREQ_H__
+#include <linux/ktime.h> + +struct devfreq; +struct thermal_cooling_device; + +struct panfrost_device; + +struct panfrost_devfreq { + struct devfreq *devfreq; + struct thermal_cooling_device *cooling; + ktime_t busy_time; + ktime_t idle_time; + ktime_t time_last_update; + atomic_t busy_count; +}; + int panfrost_devfreq_init(struct panfrost_device *pfdev); void panfrost_devfreq_fini(struct panfrost_device *pfdev);
void panfrost_devfreq_resume(struct panfrost_device *pfdev); void panfrost_devfreq_suspend(struct panfrost_device *pfdev);
-void panfrost_devfreq_record_busy(struct panfrost_device *pfdev); -void panfrost_devfreq_record_idle(struct panfrost_device *pfdev); +void panfrost_devfreq_record_busy(struct panfrost_devfreq *devfreq); +void panfrost_devfreq_record_idle(struct panfrost_devfreq *devfreq);
#endif /* __PANFROST_DEVFREQ_H__ */ diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index c30c719a8059..2efa59c9d1c5 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -13,6 +13,8 @@ #include <drm/drm_mm.h> #include <drm/gpu_scheduler.h>
+#include "panfrost_devfreq.h" + struct panfrost_device; struct panfrost_mmu; struct panfrost_job_slot; @@ -107,14 +109,7 @@ struct panfrost_device { struct list_head shrinker_list; struct shrinker shrinker;
- struct { - struct devfreq *devfreq; - struct thermal_cooling_device *cooling; - ktime_t busy_time; - ktime_t idle_time; - ktime_t time_last_update; - atomic_t busy_count; - } devfreq; + struct panfrost_devfreq pfdevfreq; };
struct panfrost_mmu { diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 63e32a9f2749..a67f3eac6a58 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -155,7 +155,7 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) }
cfg = panfrost_mmu_as_get(pfdev, &job->file_priv->mmu); - panfrost_devfreq_record_busy(pfdev); + panfrost_devfreq_record_busy(&pfdev->pfdevfreq);
job_write(pfdev, JS_HEAD_NEXT_LO(js), jc_head & 0xFFFFFFFF); job_write(pfdev, JS_HEAD_NEXT_HI(js), jc_head >> 32); @@ -415,7 +415,7 @@ static void panfrost_job_timedout(struct drm_sched_job *sched_job) } spin_unlock_irqrestore(&pfdev->js->job_lock, flags);
- panfrost_devfreq_record_idle(pfdev); + panfrost_devfreq_record_idle(&pfdev->pfdevfreq); panfrost_device_reset(pfdev);
for (i = 0; i < NUM_JOB_SLOTS; i++) @@ -478,7 +478,7 @@ static irqreturn_t panfrost_job_irq_handler(int irq, void *data) pfdev->jobs[j] = NULL;
panfrost_mmu_as_put(pfdev, &job->file_priv->mmu); - panfrost_devfreq_record_idle(pfdev); + panfrost_devfreq_record_idle(&pfdev->pfdevfreq);
dma_fence_signal_locked(job->done_fence); pm_runtime_put_autosuspend(pfdev->dev);
Convert busy_count to a simple int protected by spinlock.
Signed-off-by: Clément Péron peron.clem@gmail.com Reviewed-by: Steven Price steven.price@arm.com --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 43 +++++++++++++++------ drivers/gpu/drm/panfrost/panfrost_devfreq.h | 9 ++++- 2 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index 962550363391..78753cfb59fb 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -12,16 +12,12 @@
static void panfrost_devfreq_update_utilization(struct panfrost_devfreq *pfdevfreq) { - ktime_t now; - ktime_t last; - - if (!pfdevfreq->devfreq) - return; + ktime_t now, last;
now = ktime_get(); last = pfdevfreq->time_last_update;
- if (atomic_read(&pfdevfreq->busy_count) > 0) + if (pfdevfreq->busy_count > 0) pfdevfreq->busy_time += ktime_sub(now, last); else pfdevfreq->idle_time += ktime_sub(now, last); @@ -59,10 +55,14 @@ static int panfrost_devfreq_get_dev_status(struct device *dev, { struct panfrost_device *pfdev = dev_get_drvdata(dev); struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq; + unsigned long irqflags; + + status->current_frequency = clk_get_rate(pfdev->clock); + + spin_lock_irqsave(&pfdevfreq->lock, irqflags);
panfrost_devfreq_update_utilization(pfdevfreq);
- status->current_frequency = clk_get_rate(pfdev->clock); status->total_time = ktime_to_ns(ktime_add(pfdevfreq->busy_time, pfdevfreq->idle_time));
@@ -70,6 +70,8 @@ static int panfrost_devfreq_get_dev_status(struct device *dev,
panfrost_devfreq_reset(pfdevfreq);
+ spin_unlock_irqrestore(&pfdevfreq->lock, irqflags); + dev_dbg(pfdev->dev, "busy %lu total %lu %lu %% freq %lu MHz\n", status->busy_time, status->total_time, status->busy_time / (status->total_time / 100), @@ -100,6 +102,8 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) else if (ret) return ret;
+ spin_lock_init(&pfdevfreq->lock); + panfrost_devfreq_reset(pfdevfreq);
cur_freq = clk_get_rate(pfdev->clock); @@ -162,15 +166,32 @@ void panfrost_devfreq_suspend(struct panfrost_device *pfdev)
void panfrost_devfreq_record_busy(struct panfrost_devfreq *pfdevfreq) { + unsigned long irqflags; + + if (!pfdevfreq->devfreq) + return; + + spin_lock_irqsave(&pfdevfreq->lock, irqflags); + panfrost_devfreq_update_utilization(pfdevfreq); - atomic_inc(&pfdevfreq->busy_count); + + pfdevfreq->busy_count++; + + spin_unlock_irqrestore(&pfdevfreq->lock, irqflags); }
void panfrost_devfreq_record_idle(struct panfrost_devfreq *pfdevfreq) { - int count; + unsigned long irqflags; + + if (!pfdevfreq->devfreq) + return; + + spin_lock_irqsave(&pfdevfreq->lock, irqflags);
panfrost_devfreq_update_utilization(pfdevfreq); - count = atomic_dec_if_positive(&pfdevfreq->busy_count); - WARN_ON(count < 0); + + WARN_ON(--pfdevfreq->busy_count < 0); + + spin_unlock_irqrestore(&pfdevfreq->lock, irqflags); } diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.h b/drivers/gpu/drm/panfrost/panfrost_devfreq.h index 0697f8d5aa34..3392df1020be 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.h +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.h @@ -4,6 +4,7 @@ #ifndef __PANFROST_DEVFREQ_H__ #define __PANFROST_DEVFREQ_H__
+#include <linux/spinlock.h> #include <linux/ktime.h>
struct devfreq; @@ -14,10 +15,16 @@ struct panfrost_device; struct panfrost_devfreq { struct devfreq *devfreq; struct thermal_cooling_device *cooling; + ktime_t busy_time; ktime_t idle_time; ktime_t time_last_update; - atomic_t busy_count; + int busy_count; + /* + * Protect busy_time, idle_time, time_last_update and busy_count + * because these can be updated concurrently between multiple jobs. + */ + spinlock_t lock; };
int panfrost_devfreq_init(struct panfrost_device *pfdev);
Introduce a boolean to know if opp table has been added.
With this, we can call panfrost_devfreq_fini() in case of error and release what has been initialised.
Signed-off-by: Clément Péron peron.clem@gmail.com Reviewed-by: Steven Price steven.price@arm.com --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 25 ++++++++++++++++----- drivers/gpu/drm/panfrost/panfrost_devfreq.h | 1 + 2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index 78753cfb59fb..d9007f44b772 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -101,6 +101,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) return 0; else if (ret) return ret; + pfdevfreq->opp_of_table_added = true;
spin_lock_init(&pfdevfreq->lock);
@@ -109,8 +110,10 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) cur_freq = clk_get_rate(pfdev->clock);
opp = devfreq_recommended_opp(dev, &cur_freq, 0); - if (IS_ERR(opp)) - return PTR_ERR(opp); + if (IS_ERR(opp)) { + ret = PTR_ERR(opp); + goto err_fini; + }
panfrost_devfreq_profile.initial_freq = cur_freq; dev_pm_opp_put(opp); @@ -119,8 +122,8 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) DEVFREQ_GOV_SIMPLE_ONDEMAND, NULL); if (IS_ERR(devfreq)) { DRM_DEV_ERROR(dev, "Couldn't initialize GPU devfreq\n"); - dev_pm_opp_of_remove_table(dev); - return PTR_ERR(devfreq); + ret = PTR_ERR(devfreq); + goto err_fini; } pfdevfreq->devfreq = devfreq;
@@ -131,15 +134,25 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) pfdevfreq->cooling = cooling;
return 0; + +err_fini: + panfrost_devfreq_fini(pfdev); + return ret; }
void panfrost_devfreq_fini(struct panfrost_device *pfdev) { struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq;
- if (pfdevfreq->cooling) + if (pfdevfreq->cooling) { devfreq_cooling_unregister(pfdevfreq->cooling); - dev_pm_opp_of_remove_table(&pfdev->pdev->dev); + pfdevfreq->cooling = NULL; + } + + if (pfdevfreq->opp_of_table_added) { + dev_pm_opp_of_remove_table(&pfdev->pdev->dev); + pfdevfreq->opp_of_table_added = false; + } }
void panfrost_devfreq_resume(struct panfrost_device *pfdev) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.h b/drivers/gpu/drm/panfrost/panfrost_devfreq.h index 3392df1020be..210269944687 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.h +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.h @@ -15,6 +15,7 @@ struct panfrost_device; struct panfrost_devfreq { struct devfreq *devfreq; struct thermal_cooling_device *cooling; + bool opp_of_table_added;
ktime_t busy_time; ktime_t idle_time;
Rename goto labels in device_init it will be easier to maintain.
Signed-off-by: Clément Péron peron.clem@gmail.com --- drivers/gpu/drm/panfrost/panfrost_device.c | 30 +++++++++++----------- 1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 8136babd3ba9..cc16d102b275 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -215,57 +215,57 @@ int panfrost_device_init(struct panfrost_device *pfdev) err = panfrost_regulator_init(pfdev); if (err) { dev_err(pfdev->dev, "regulator init failed %d\n", err); - goto err_out0; + goto out_clk; }
err = panfrost_reset_init(pfdev); if (err) { dev_err(pfdev->dev, "reset init failed %d\n", err); - goto err_out1; + goto out_regulator; }
err = panfrost_pm_domain_init(pfdev); if (err) - goto err_out2; + goto out_reset;
res = platform_get_resource(pfdev->pdev, IORESOURCE_MEM, 0); pfdev->iomem = devm_ioremap_resource(pfdev->dev, res); if (IS_ERR(pfdev->iomem)) { dev_err(pfdev->dev, "failed to ioremap iomem\n"); err = PTR_ERR(pfdev->iomem); - goto err_out3; + goto out_pm_domain; }
err = panfrost_gpu_init(pfdev); if (err) - goto err_out3; + goto out_pm_domain;
err = panfrost_mmu_init(pfdev); if (err) - goto err_out4; + goto out_gpu;
err = panfrost_job_init(pfdev); if (err) - goto err_out5; + goto out_mmu;
err = panfrost_perfcnt_init(pfdev); if (err) - goto err_out6; + goto out_job;
return 0; -err_out6: +out_job: panfrost_job_fini(pfdev); -err_out5: +out_mmu: panfrost_mmu_fini(pfdev); -err_out4: +out_gpu: panfrost_gpu_fini(pfdev); -err_out3: +out_pm_domain: panfrost_pm_domain_fini(pfdev); -err_out2: +out_reset: panfrost_reset_fini(pfdev); -err_out1: +out_regulator: panfrost_regulator_fini(pfdev); -err_out0: +out_clk: panfrost_clk_fini(pfdev); return err; }
Later we will introduce devfreq probing regulator if they are present. As regulator should be probe only one time we need to get this logic in the device_init().
panfrost_device is already taking care of devfreq_resume() and devfreq_suspend(), so it's not totally illogic to move the devfreq_init() and devfreq_fini() here.
Signed-off-by: Clément Péron peron.clem@gmail.com --- drivers/gpu/drm/panfrost/panfrost_device.c | 12 +++++++++++- drivers/gpu/drm/panfrost/panfrost_drv.c | 15 ++------------- 2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index cc16d102b275..464da1646398 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -212,10 +212,17 @@ int panfrost_device_init(struct panfrost_device *pfdev) return err; }
+ err = panfrost_devfreq_init(pfdev); + if (err) { + if (err != -EPROBE_DEFER) + dev_err(pfdev->dev, "devfreq init failed %d\n", err); + goto out_clk; + } + err = panfrost_regulator_init(pfdev); if (err) { dev_err(pfdev->dev, "regulator init failed %d\n", err); - goto out_clk; + goto out_devfreq; }
err = panfrost_reset_init(pfdev); @@ -265,6 +272,8 @@ int panfrost_device_init(struct panfrost_device *pfdev) panfrost_reset_fini(pfdev); out_regulator: panfrost_regulator_fini(pfdev); +out_devfreq: + panfrost_devfreq_fini(pfdev); out_clk: panfrost_clk_fini(pfdev); return err; @@ -278,6 +287,7 @@ void panfrost_device_fini(struct panfrost_device *pfdev) panfrost_gpu_fini(pfdev); panfrost_pm_domain_fini(pfdev); panfrost_reset_fini(pfdev); + panfrost_devfreq_fini(pfdev); panfrost_regulator_fini(pfdev); panfrost_clk_fini(pfdev); } diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 882fecc33fdb..4dda68689015 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -14,7 +14,6 @@ #include <drm/drm_utils.h>
#include "panfrost_device.h" -#include "panfrost_devfreq.h" #include "panfrost_gem.h" #include "panfrost_mmu.h" #include "panfrost_job.h" @@ -606,13 +605,6 @@ static int panfrost_probe(struct platform_device *pdev) goto err_out0; }
- err = panfrost_devfreq_init(pfdev); - if (err) { - if (err != -EPROBE_DEFER) - dev_err(&pdev->dev, "Fatal error during devfreq init\n"); - goto err_out1; - } - pm_runtime_set_active(pfdev->dev); pm_runtime_mark_last_busy(pfdev->dev); pm_runtime_enable(pfdev->dev); @@ -625,16 +617,14 @@ static int panfrost_probe(struct platform_device *pdev) */ err = drm_dev_register(ddev, 0); if (err < 0) - goto err_out2; + goto err_out1;
panfrost_gem_shrinker_init(ddev);
return 0;
-err_out2: - pm_runtime_disable(pfdev->dev); - panfrost_devfreq_fini(pfdev); err_out1: + pm_runtime_disable(pfdev->dev); panfrost_device_fini(pfdev); err_out0: drm_dev_put(ddev); @@ -650,7 +640,6 @@ static int panfrost_remove(struct platform_device *pdev) panfrost_gem_shrinker_cleanup(ddev);
pm_runtime_get_sync(pfdev->dev); - panfrost_devfreq_fini(pfdev); panfrost_device_fini(pfdev); pm_runtime_put_sync_suspend(pfdev->dev); pm_runtime_disable(pfdev->dev);
We will later introduce regulators managed by OPP.
Only alloc regulators when it's needed. This also help use to release the regulators only when they are allocated.
Signed-off-by: Clément Péron peron.clem@gmail.com Reviewed-by: Steven Price steven.price@arm.com --- drivers/gpu/drm/panfrost/panfrost_device.c | 14 +++++++++----- drivers/gpu/drm/panfrost/panfrost_device.h | 3 +-- 2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 464da1646398..0b0fb45aee82 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -90,9 +90,11 @@ static int panfrost_regulator_init(struct panfrost_device *pfdev) { int ret, i;
- if (WARN(pfdev->comp->num_supplies > ARRAY_SIZE(pfdev->regulators), - "Too many supplies in compatible structure.\n")) - return -EINVAL; + pfdev->regulators = devm_kcalloc(pfdev->dev, pfdev->comp->num_supplies, + sizeof(*pfdev->regulators), + GFP_KERNEL); + if (!pfdev->regulators) + return -ENOMEM;
for (i = 0; i < pfdev->comp->num_supplies; i++) pfdev->regulators[i].supply = pfdev->comp->supply_names[i]; @@ -117,8 +119,10 @@ static int panfrost_regulator_init(struct panfrost_device *pfdev)
static void panfrost_regulator_fini(struct panfrost_device *pfdev) { - regulator_bulk_disable(pfdev->comp->num_supplies, - pfdev->regulators); + if (!pfdev->regulators) + return; + + regulator_bulk_disable(pfdev->comp->num_supplies, pfdev->regulators); }
static void panfrost_pm_domain_fini(struct panfrost_device *pfdev) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index 2efa59c9d1c5..953f7536a773 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -22,7 +22,6 @@ struct panfrost_job; struct panfrost_perfcnt;
#define NUM_JOB_SLOTS 3 -#define MAX_REGULATORS 2 #define MAX_PM_DOMAINS 3
struct panfrost_features { @@ -81,7 +80,7 @@ struct panfrost_device { void __iomem *iomem; struct clk *clock; struct clk *bus_clock; - struct regulator_bulk_data regulators[MAX_REGULATORS]; + struct regulator_bulk_data *regulators; struct reset_control *rstc; /* pm_domains for devices with more than one. */ struct device *pm_domain_devs[MAX_PM_DOMAINS];
Some OPP tables specify voltage for each frequency. Devfreq can handle these regulators but they should be get only 1 time to avoid issue and know who is in charge.
If OPP table is probe don't init regulator.
Signed-off-by: Clément Péron peron.clem@gmail.com Reviewed-by: Steven Price steven.price@arm.com --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 19 +++++++++++++++++++ drivers/gpu/drm/panfrost/panfrost_devfreq.h | 2 ++ drivers/gpu/drm/panfrost/panfrost_device.c | 11 +++++++---- 3 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index d9007f44b772..d1e3e9d00a48 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -93,6 +93,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) unsigned long cur_freq; struct device *dev = &pfdev->pdev->dev; struct devfreq *devfreq; + struct opp_table *opp_table; struct thermal_cooling_device *cooling; struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq;
@@ -105,6 +106,19 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
spin_lock_init(&pfdevfreq->lock);
+ opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names, + pfdev->comp->num_supplies); + if (IS_ERR(opp_table)) { + ret = PTR_ERR(opp_table); + /* Continue if the optional regulator is missing */ + if (ret != -ENODEV) { + DRM_DEV_ERROR(dev, "Couldn't set OPP regulators\n"); + goto err_fini; + } + } else { + pfdevfreq->regulators_opp_table = opp_table; + } + panfrost_devfreq_reset(pfdevfreq);
cur_freq = clk_get_rate(pfdev->clock); @@ -153,6 +167,11 @@ void panfrost_devfreq_fini(struct panfrost_device *pfdev) dev_pm_opp_of_remove_table(&pfdev->pdev->dev); pfdevfreq->opp_of_table_added = false; } + + if (pfdevfreq->regulators_opp_table) { + dev_pm_opp_put_regulators(pfdevfreq->regulators_opp_table); + pfdevfreq->regulators_opp_table = NULL; + } }
void panfrost_devfreq_resume(struct panfrost_device *pfdev) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.h b/drivers/gpu/drm/panfrost/panfrost_devfreq.h index 210269944687..db6ea48e21f9 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.h +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.h @@ -8,12 +8,14 @@ #include <linux/ktime.h>
struct devfreq; +struct opp_table; struct thermal_cooling_device;
struct panfrost_device;
struct panfrost_devfreq { struct devfreq *devfreq; + struct opp_table *regulators_opp_table; struct thermal_cooling_device *cooling; bool opp_of_table_added;
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 0b0fb45aee82..1b5fc9221828 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -223,10 +223,13 @@ int panfrost_device_init(struct panfrost_device *pfdev) goto out_clk; }
- err = panfrost_regulator_init(pfdev); - if (err) { - dev_err(pfdev->dev, "regulator init failed %d\n", err); - goto out_devfreq; + /* OPP will handle regulators */ + if (!pfdev->pfdevfreq.opp_of_table_added) { + err = panfrost_regulator_init(pfdev); + if (err) { + dev_err(pfdev->dev, "regulator init failed %d\n", err); + goto out_devfreq; + } }
err = panfrost_reset_init(pfdev);
Devfreq cooling device framework is used in Panfrost to throttle GPU in order to regulate its temperature.
Enable this driver for ARM64 SoC.
Signed-off-by: Clément Péron peron.clem@gmail.com --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 883e8bace3ed..1b7f9ffdc314 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -501,6 +501,7 @@ CONFIG_SENSORS_INA2XX=m CONFIG_SENSORS_INA3221=m CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y CONFIG_CPU_THERMAL=y +CONFIG_DEVFREQ_THERMAL=y CONFIG_THERMAL_EMULATION=y CONFIG_QORIQ_THERMAL=m CONFIG_SUN8I_THERMAL=y
Add a simple cooling map for the GPU.
Signed-off-by: Clément Péron peron.clem@gmail.com --- arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 22 ++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi index 78b1361dfbb9..8f514a2169aa 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi @@ -174,6 +174,7 @@ gpu: gpu@1800000 { clocks = <&ccu CLK_GPU>, <&ccu CLK_BUS_GPU>; clock-names = "core", "bus"; resets = <&ccu RST_BUS_GPU>; + #cooling-cells = <2>; status = "disabled"; };
@@ -1012,6 +1013,27 @@ gpu-thermal { polling-delay-passive = <0>; polling-delay = <0>; thermal-sensors = <&ths 1>; + + trips { + gpu_alert: gpu-alert { + temperature = <85000>; + hysteresis = <2000>; + type = "passive"; + }; + + gpu-crit { + temperature = <100000>; + hysteresis = <0>; + type = "critical"; + }; + }; + + cooling-maps { + map0 { + trip = <&gpu_alert>; + cooling-device = <&gpu THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; + }; + }; }; }; };
Add an Operating Performance Points table for the GPU to enable Dynamic Voltage & Frequency Scaling on the H6.
The voltage range is set with minival voltage set to the target and the maximal voltage set to 1.2V. This allow DVFS framework to work properly on board with fixed regulator.
Signed-off-by: Clément Péron peron.clem@gmail.com --- arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 80 ++++++++++++++++++++ 1 file changed, 80 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi index 8f514a2169aa..a69f9e09a829 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi @@ -174,6 +174,7 @@ gpu: gpu@1800000 { clocks = <&ccu CLK_GPU>, <&ccu CLK_BUS_GPU>; clock-names = "core", "bus"; resets = <&ccu RST_BUS_GPU>; + operating-points-v2 = <&gpu_opp_table>; #cooling-cells = <2>; status = "disabled"; }; @@ -1036,4 +1037,83 @@ map0 { }; }; }; + + gpu_opp_table: gpu-opp-table { + compatible = "operating-points-v2"; + + opp@216000000 { + opp-hz = /bits/ 64 <216000000>; + opp-microvolt = <810000 810000 1200000>; + }; + + opp@264000000 { + opp-hz = /bits/ 64 <264000000>; + opp-microvolt = <810000 810000 1200000>; + }; + + opp@312000000 { + opp-hz = /bits/ 64 <312000000>; + opp-microvolt = <810000 810000 1200000>; + }; + + opp@336000000 { + opp-hz = /bits/ 64 <336000000>; + opp-microvolt = <810000 810000 1200000>; + }; + + opp@360000000 { + opp-hz = /bits/ 64 <360000000>; + opp-microvolt = <820000 820000 1200000>; + }; + + opp@384000000 { + opp-hz = /bits/ 64 <384000000>; + opp-microvolt = <830000 830000 1200000>; + }; + + opp@408000000 { + opp-hz = /bits/ 64 <408000000>; + opp-microvolt = <840000 840000 1200000>; + }; + + opp@420000000 { + opp-hz = /bits/ 64 <420000000>; + opp-microvolt = <850000 850000 1200000>; + }; + + opp@432000000 { + opp-hz = /bits/ 64 <432000000>; + opp-microvolt = <860000 860000 1200000>; + }; + + opp@456000000 { + opp-hz = /bits/ 64 <456000000>; + opp-microvolt = <870000 870000 1200000>; + }; + + opp@504000000 { + opp-hz = /bits/ 64 <504000000>; + opp-microvolt = <890000 890000 1200000>; + }; + + opp@540000000 { + opp-hz = /bits/ 64 <540000000>; + opp-microvolt = <910000 910000 1200000>; + }; + + opp@576000000 { + opp-hz = /bits/ 64 <576000000>; + opp-microvolt = <930000 930000 1200000>; + }; + + opp@624000000 { + opp-hz = /bits/ 64 <624000000>; + opp-microvolt = <950000 950000 1200000>; + }; + + opp@756000000 { + opp-hz = /bits/ 64 <756000000>; + opp-microvolt = <1040000 1040000 1200000>; + }; + }; };
Hi,
On Sat, Jul 04, 2020 at 12:25:34PM +0200, Clément Péron wrote:
Add an Operating Performance Points table for the GPU to enable Dynamic Voltage & Frequency Scaling on the H6.
The voltage range is set with minival voltage set to the target and the maximal voltage set to 1.2V. This allow DVFS framework to work properly on board with fixed regulator.
Signed-off-by: Clément Péron peron.clem@gmail.com
That patch seems reasonable, why shouldn't we merge it?
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 80 ++++++++++++++++++++ 1 file changed, 80 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi index 8f514a2169aa..a69f9e09a829 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi @@ -174,6 +174,7 @@ gpu: gpu@1800000 { clocks = <&ccu CLK_GPU>, <&ccu CLK_BUS_GPU>; clock-names = "core", "bus"; resets = <&ccu RST_BUS_GPU>;
operating-points-v2 = <&gpu_opp_table>; #cooling-cells = <2>; status = "disabled";@@ -1036,4 +1037,83 @@ map0 { }; }; };
- gpu_opp_table: gpu-opp-table {
compatible = "operating-points-v2";opp@216000000 {opp-hz = /bits/ 64 <216000000>;opp-microvolt = <810000 810000 1200000>;};
All those nodes will create DTC warnings though.
Maxime
Hi Maxime,
On Sat, 4 Jul 2020 at 14:13, Maxime Ripard maxime@cerno.tech wrote:
Hi,
On Sat, Jul 04, 2020 at 12:25:34PM +0200, Clément Péron wrote:
Add an Operating Performance Points table for the GPU to enable Dynamic Voltage & Frequency Scaling on the H6.
The voltage range is set with minival voltage set to the target and the maximal voltage set to 1.2V. This allow DVFS framework to work properly on board with fixed regulator.
Signed-off-by: Clément Péron peron.clem@gmail.com
That patch seems reasonable, why shouldn't we merge it?
I didn't test it a lot and last time I did, some frequencies looked unstable. https://lore.kernel.org/patchwork/cover/1239739/
This series adds regulator support to Panfrost devfreq, I will send a new one if DVFS on the H6 GPU is stable.
I got this running glmark2 last time # glmark2-es2-drm ======================================================= glmark2 2017.07 ======================================================= OpenGL Information GL_VENDOR: Panfrost GL_RENDERER: Mali T720 (Panfrost) GL_VERSION: OpenGL ES 2.0 Mesa 20.0.5 =======================================================
[ 93.550063] panfrost 1800000.gpu: GPU Fault 0x00000088 (UNKNOWN) at 0x0000000080117100 [ 94.045401] panfrost 1800000.gpu: gpu sched timeout, js=0, config=0x3700, status=0x8, head=0x21d6c00, tail=0x21d6c00, sched_job=00000000e3c2132f
[ 328.871070] panfrost 1800000.gpu: Unhandled Page fault in AS0 at VA 0x0000000000000000 [ 328.871070] Reason: TODO [ 328.871070] raw fault status: 0xAA0003C2 [ 328.871070] decoded fault status: SLAVE FAULT [ 328.871070] exception type 0xC2: TRANSLATION_FAULT_LEVEL2 [ 328.871070] access type 0x3: WRITE [ 328.871070] source id 0xAA00 [ 329.373327] panfrost 1800000.gpu: gpu sched timeout, js=1, config=0x3700, status=0x8, head=0xa1a4900, tail=0xa1a4900, sched_job=000000007ac31097 [ 329.386527] panfrost 1800000.gpu: js fault, js=0, status=DATA_INVALID_FAULT, head=0xa1a4c00, tail=0xa1a4c00 [ 329.396293] panfrost 1800000.gpu: gpu sched timeout, js=0, config=0x3700, status=0x58, head=0xa1a4c00, tail=0xa1a4c00, sched_job=0000000004c90381 [ 329.411521] panfrost 1800000.gpu: Unhandled Page fault in AS0 at VA 0x0000000000000000 [ 329.411521] Reason: TODO [ 329.411521] raw fault status: 0xAA0003C2 [ 329.411521] decoded fault status: SLAVE FAULT [ 329.411521] exception type 0xC2: TRANSLATION_FAULT_LEVEL2 [ 329.411521] access type 0x3: WRITE [ 329.411521] source id 0xAA00
Regards, Clement
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 80 ++++++++++++++++++++ 1 file changed, 80 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi index 8f514a2169aa..a69f9e09a829 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi @@ -174,6 +174,7 @@ gpu: gpu@1800000 { clocks = <&ccu CLK_GPU>, <&ccu CLK_BUS_GPU>; clock-names = "core", "bus"; resets = <&ccu RST_BUS_GPU>;
operating-points-v2 = <&gpu_opp_table>; #cooling-cells = <2>; status = "disabled"; };@@ -1036,4 +1037,83 @@ map0 { }; }; };
gpu_opp_table: gpu-opp-table {compatible = "operating-points-v2";opp@216000000 {opp-hz = /bits/ 64 <216000000>;opp-microvolt = <810000 810000 1200000>;};All those nodes will create DTC warnings though.
Maxime
Hi Maxime and All,
On Sat, 4 Jul 2020 at 16:56, Clément Péron peron.clem@gmail.com wrote:
Hi Maxime,
On Sat, 4 Jul 2020 at 14:13, Maxime Ripard maxime@cerno.tech wrote:
Hi,
On Sat, Jul 04, 2020 at 12:25:34PM +0200, Clément Péron wrote:
Add an Operating Performance Points table for the GPU to enable Dynamic Voltage & Frequency Scaling on the H6.
The voltage range is set with minival voltage set to the target and the maximal voltage set to 1.2V. This allow DVFS framework to work properly on board with fixed regulator.
Signed-off-by: Clément Péron peron.clem@gmail.com
That patch seems reasonable, why shouldn't we merge it?
I didn't test it a lot and last time I did, some frequencies looked unstable. https://lore.kernel.org/patchwork/cover/1239739/
This series adds regulator support to Panfrost devfreq, I will send a new one if DVFS on the H6 GPU is stable.
I got this running glmark2 last time
# glmark2-es2-drm
glmark2 2017.07======================================================= OpenGL Information GL_VENDOR: Panfrost GL_RENDERER: Mali T720 (Panfrost) GL_VERSION: OpenGL ES 2.0 Mesa 20.0.5 =======================================================
[ 93.550063] panfrost 1800000.gpu: GPU Fault 0x00000088 (UNKNOWN) at 0x0000000080117100 [ 94.045401] panfrost 1800000.gpu: gpu sched timeout, js=0, config=0x3700, status=0x8, head=0x21d6c00, tail=0x21d6c00, sched_job=00000000e3c2132f
[ 328.871070] panfrost 1800000.gpu: Unhandled Page fault in AS0 at VA 0x0000000000000000 [ 328.871070] Reason: TODO [ 328.871070] raw fault status: 0xAA0003C2 [ 328.871070] decoded fault status: SLAVE FAULT [ 328.871070] exception type 0xC2: TRANSLATION_FAULT_LEVEL2 [ 328.871070] access type 0x3: WRITE [ 328.871070] source id 0xAA00 [ 329.373327] panfrost 1800000.gpu: gpu sched timeout, js=1, config=0x3700, status=0x8, head=0xa1a4900, tail=0xa1a4900, sched_job=000000007ac31097 [ 329.386527] panfrost 1800000.gpu: js fault, js=0, status=DATA_INVALID_FAULT, head=0xa1a4c00, tail=0xa1a4c00 [ 329.396293] panfrost 1800000.gpu: gpu sched timeout, js=0, config=0x3700, status=0x58, head=0xa1a4c00, tail=0xa1a4c00, sched_job=0000000004c90381 [ 329.411521] panfrost 1800000.gpu: Unhandled Page fault in AS0 at VA 0x0000000000000000 [ 329.411521] Reason: TODO [ 329.411521] raw fault status: 0xAA0003C2 [ 329.411521] decoded fault status: SLAVE FAULT [ 329.411521] exception type 0xC2: TRANSLATION_FAULT_LEVEL2 [ 329.411521] access type 0x3: WRITE [ 329.411521] source id 0xAA00
Just to keep a track of this issue.
Piotr Oniszczuk give more test and seems to be software related: https://www.spinics.net/lists/dri-devel/msg264279.html
Ondrej gave a great explanation about a possible origin of this issue: https://freenode.irclog.whitequark.org/linux-sunxi/2020-07-11
20:12 <megi> looks like gpu pll on H6 is NKMP clock, and those are implemented in such a way in mainline that they are prone to overshooting the frequency during output divider reduction 20:13 <megi> so disabling P divider may help 20:13 <megi> or fixing the dividers 20:14 <megi> and just allowing N to change 20:22 <megi> hmm, I haven't looked at this for quite some time, but H6 BSP way of setting PLL factors actually makes the most sense out of everything I've seen/tested so far 20:23 <megi> it waits for lock not after setting NK factors, but after reducing the M factor (pre-divider) 20:24 <megi> I might as well re-run my CPU PLL tester with this algorithm, to see if it fixes the lockups 20:26 <megi> it makes sense to wait for PLL to stabilize "after" changing all the factors that actually affect the VCO, and not just some of them 20:27 <megi> warpme_: ^ 20:28 <megi> it may be the same thing that plagues the CPU PLL rate changes at runtime
Regards, Clement
Regards, Clement
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 80 ++++++++++++++++++++ 1 file changed, 80 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi index 8f514a2169aa..a69f9e09a829 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi @@ -174,6 +174,7 @@ gpu: gpu@1800000 { clocks = <&ccu CLK_GPU>, <&ccu CLK_BUS_GPU>; clock-names = "core", "bus"; resets = <&ccu RST_BUS_GPU>;
operating-points-v2 = <&gpu_opp_table>; #cooling-cells = <2>; status = "disabled"; };@@ -1036,4 +1037,83 @@ map0 { }; }; };
gpu_opp_table: gpu-opp-table {compatible = "operating-points-v2";opp@216000000 {opp-hz = /bits/ 64 <216000000>;opp-microvolt = <810000 810000 1200000>;};All those nodes will create DTC warnings though.
Maxime
Hi Clement,
On Mon, Aug 03, 2020 at 09:54:05AM +0200, Clément Péron wrote:
Hi Maxime and All,
On Sat, 4 Jul 2020 at 16:56, Clément Péron peron.clem@gmail.com wrote:
Hi Maxime,
On Sat, 4 Jul 2020 at 14:13, Maxime Ripard maxime@cerno.tech wrote:
Hi,
On Sat, Jul 04, 2020 at 12:25:34PM +0200, Clément Péron wrote:
Add an Operating Performance Points table for the GPU to enable Dynamic Voltage & Frequency Scaling on the H6.
The voltage range is set with minival voltage set to the target and the maximal voltage set to 1.2V. This allow DVFS framework to work properly on board with fixed regulator.
Signed-off-by: Clément Péron peron.clem@gmail.com
That patch seems reasonable, why shouldn't we merge it?
I didn't test it a lot and last time I did, some frequencies looked unstable. https://lore.kernel.org/patchwork/cover/1239739/
This series adds regulator support to Panfrost devfreq, I will send a new one if DVFS on the H6 GPU is stable.
I got this running glmark2 last time
# glmark2-es2-drm
glmark2 2017.07======================================================= OpenGL Information GL_VENDOR: Panfrost GL_RENDERER: Mali T720 (Panfrost) GL_VERSION: OpenGL ES 2.0 Mesa 20.0.5 =======================================================
[ 93.550063] panfrost 1800000.gpu: GPU Fault 0x00000088 (UNKNOWN) at 0x0000000080117100 [ 94.045401] panfrost 1800000.gpu: gpu sched timeout, js=0, config=0x3700, status=0x8, head=0x21d6c00, tail=0x21d6c00, sched_job=00000000e3c2132f
[ 328.871070] panfrost 1800000.gpu: Unhandled Page fault in AS0 at VA 0x0000000000000000 [ 328.871070] Reason: TODO [ 328.871070] raw fault status: 0xAA0003C2 [ 328.871070] decoded fault status: SLAVE FAULT [ 328.871070] exception type 0xC2: TRANSLATION_FAULT_LEVEL2 [ 328.871070] access type 0x3: WRITE [ 328.871070] source id 0xAA00 [ 329.373327] panfrost 1800000.gpu: gpu sched timeout, js=1, config=0x3700, status=0x8, head=0xa1a4900, tail=0xa1a4900, sched_job=000000007ac31097 [ 329.386527] panfrost 1800000.gpu: js fault, js=0, status=DATA_INVALID_FAULT, head=0xa1a4c00, tail=0xa1a4c00 [ 329.396293] panfrost 1800000.gpu: gpu sched timeout, js=0, config=0x3700, status=0x58, head=0xa1a4c00, tail=0xa1a4c00, sched_job=0000000004c90381 [ 329.411521] panfrost 1800000.gpu: Unhandled Page fault in AS0 at VA 0x0000000000000000 [ 329.411521] Reason: TODO [ 329.411521] raw fault status: 0xAA0003C2 [ 329.411521] decoded fault status: SLAVE FAULT [ 329.411521] exception type 0xC2: TRANSLATION_FAULT_LEVEL2 [ 329.411521] access type 0x3: WRITE [ 329.411521] source id 0xAA00
Just to keep a track of this issue.
Piotr Oniszczuk give more test and seems to be software related: https://www.spinics.net/lists/dri-devel/msg264279.html
Ondrej gave a great explanation about a possible origin of this issue: https://freenode.irclog.whitequark.org/linux-sunxi/2020-07-11
20:12 <megi> looks like gpu pll on H6 is NKMP clock, and those are implemented in such a way in mainline that they are prone to overshooting the frequency during output divider reduction 20:13 <megi> so disabling P divider may help 20:13 <megi> or fixing the dividers 20:14 <megi> and just allowing N to change 20:22 <megi> hmm, I haven't looked at this for quite some time, but H6 BSP way of setting PLL factors actually makes the most sense out of everything I've seen/tested so far 20:23 <megi> it waits for lock not after setting NK factors, but after reducing the M factor (pre-divider) 20:24 <megi> I might as well re-run my CPU PLL tester with this algorithm, to see if it fixes the lockups 20:26 <megi> it makes sense to wait for PLL to stabilize "after" changing all the factors that actually affect the VCO, and not just some of them 20:27 <megi> warpme_: ^ 20:28 <megi> it may be the same thing that plagues the CPU PLL rate changes at runtime
I guess it's one of the bugs we never heard of...
It would be a good idea to test it on another platform (like Rockchip?) to rule out any driver issue?
What do you think?
Maxime
Hi Maxime,
On Tue, 25 Aug 2020 at 15:35, Maxime Ripard maxime@cerno.tech wrote:
Hi Clement,
On Mon, Aug 03, 2020 at 09:54:05AM +0200, Clément Péron wrote:
Hi Maxime and All,
On Sat, 4 Jul 2020 at 16:56, Clément Péron peron.clem@gmail.com wrote:
Hi Maxime,
On Sat, 4 Jul 2020 at 14:13, Maxime Ripard maxime@cerno.tech wrote:
Hi,
On Sat, Jul 04, 2020 at 12:25:34PM +0200, Clément Péron wrote:
Add an Operating Performance Points table for the GPU to enable Dynamic Voltage & Frequency Scaling on the H6.
The voltage range is set with minival voltage set to the target and the maximal voltage set to 1.2V. This allow DVFS framework to work properly on board with fixed regulator.
Signed-off-by: Clément Péron peron.clem@gmail.com
That patch seems reasonable, why shouldn't we merge it?
I didn't test it a lot and last time I did, some frequencies looked unstable. https://lore.kernel.org/patchwork/cover/1239739/
This series adds regulator support to Panfrost devfreq, I will send a new one if DVFS on the H6 GPU is stable.
I got this running glmark2 last time
# glmark2-es2-drm
glmark2 2017.07======================================================= OpenGL Information GL_VENDOR: Panfrost GL_RENDERER: Mali T720 (Panfrost) GL_VERSION: OpenGL ES 2.0 Mesa 20.0.5 =======================================================
[ 93.550063] panfrost 1800000.gpu: GPU Fault 0x00000088 (UNKNOWN) at 0x0000000080117100 [ 94.045401] panfrost 1800000.gpu: gpu sched timeout, js=0, config=0x3700, status=0x8, head=0x21d6c00, tail=0x21d6c00, sched_job=00000000e3c2132f
[ 328.871070] panfrost 1800000.gpu: Unhandled Page fault in AS0 at VA 0x0000000000000000 [ 328.871070] Reason: TODO [ 328.871070] raw fault status: 0xAA0003C2 [ 328.871070] decoded fault status: SLAVE FAULT [ 328.871070] exception type 0xC2: TRANSLATION_FAULT_LEVEL2 [ 328.871070] access type 0x3: WRITE [ 328.871070] source id 0xAA00 [ 329.373327] panfrost 1800000.gpu: gpu sched timeout, js=1, config=0x3700, status=0x8, head=0xa1a4900, tail=0xa1a4900, sched_job=000000007ac31097 [ 329.386527] panfrost 1800000.gpu: js fault, js=0, status=DATA_INVALID_FAULT, head=0xa1a4c00, tail=0xa1a4c00 [ 329.396293] panfrost 1800000.gpu: gpu sched timeout, js=0, config=0x3700, status=0x58, head=0xa1a4c00, tail=0xa1a4c00, sched_job=0000000004c90381 [ 329.411521] panfrost 1800000.gpu: Unhandled Page fault in AS0 at VA 0x0000000000000000 [ 329.411521] Reason: TODO [ 329.411521] raw fault status: 0xAA0003C2 [ 329.411521] decoded fault status: SLAVE FAULT [ 329.411521] exception type 0xC2: TRANSLATION_FAULT_LEVEL2 [ 329.411521] access type 0x3: WRITE [ 329.411521] source id 0xAA00
Just to keep a track of this issue.
Piotr Oniszczuk give more test and seems to be software related: https://www.spinics.net/lists/dri-devel/msg264279.html
Ondrej gave a great explanation about a possible origin of this issue: https://freenode.irclog.whitequark.org/linux-sunxi/2020-07-11
20:12 <megi> looks like gpu pll on H6 is NKMP clock, and those are implemented in such a way in mainline that they are prone to overshooting the frequency during output divider reduction 20:13 <megi> so disabling P divider may help 20:13 <megi> or fixing the dividers 20:14 <megi> and just allowing N to change 20:22 <megi> hmm, I haven't looked at this for quite some time, but H6 BSP way of setting PLL factors actually makes the most sense out of everything I've seen/tested so far 20:23 <megi> it waits for lock not after setting NK factors, but after reducing the M factor (pre-divider) 20:24 <megi> I might as well re-run my CPU PLL tester with this algorithm, to see if it fixes the lockups 20:26 <megi> it makes sense to wait for PLL to stabilize "after" changing all the factors that actually affect the VCO, and not just some of them 20:27 <megi> warpme_: ^ 20:28 <megi> it may be the same thing that plagues the CPU PLL rate changes at runtime
I guess it's one of the bugs we never heard of...
It would be a good idea to test it on another platform (like Rockchip?) to rule out any driver issue?
What do you think?
I can't exclude a bug in the driver, but if it was the case LE community or Panfrost maintainer would have heard of that.
Megi's explanations match what I observed. NKMP drivers seem the perfect guilty here or maybe it's a combination of both...
Jernej sent me this patch to test: https://github.com/clementperon/linux/commit/56bde359beaf8e827ce53ede1fe4a0a... But it didn't fix the issue, If someone want to have a look at it :)
Regards, Clement
Maxime
On Fri, Aug 28, 2020 at 02:16:36PM +0200, Clément Péron wrote:
Hi Maxime,
On Tue, 25 Aug 2020 at 15:35, Maxime Ripard maxime@cerno.tech wrote:
Hi Clement,
On Mon, Aug 03, 2020 at 09:54:05AM +0200, Clément Péron wrote:
Hi Maxime and All,
On Sat, 4 Jul 2020 at 16:56, Clément Péron peron.clem@gmail.com wrote:
Hi Maxime,
On Sat, 4 Jul 2020 at 14:13, Maxime Ripard maxime@cerno.tech wrote:
Hi,
On Sat, Jul 04, 2020 at 12:25:34PM +0200, Clément Péron wrote:
Add an Operating Performance Points table for the GPU to enable Dynamic Voltage & Frequency Scaling on the H6.
The voltage range is set with minival voltage set to the target and the maximal voltage set to 1.2V. This allow DVFS framework to work properly on board with fixed regulator.
Signed-off-by: Clément Péron peron.clem@gmail.com
That patch seems reasonable, why shouldn't we merge it?
I didn't test it a lot and last time I did, some frequencies looked unstable. https://lore.kernel.org/patchwork/cover/1239739/
This series adds regulator support to Panfrost devfreq, I will send a new one if DVFS on the H6 GPU is stable.
I got this running glmark2 last time
# glmark2-es2-drm
glmark2 2017.07======================================================= OpenGL Information GL_VENDOR: Panfrost GL_RENDERER: Mali T720 (Panfrost) GL_VERSION: OpenGL ES 2.0 Mesa 20.0.5 =======================================================
[ 93.550063] panfrost 1800000.gpu: GPU Fault 0x00000088 (UNKNOWN) at 0x0000000080117100 [ 94.045401] panfrost 1800000.gpu: gpu sched timeout, js=0, config=0x3700, status=0x8, head=0x21d6c00, tail=0x21d6c00, sched_job=00000000e3c2132f
[ 328.871070] panfrost 1800000.gpu: Unhandled Page fault in AS0 at VA 0x0000000000000000 [ 328.871070] Reason: TODO [ 328.871070] raw fault status: 0xAA0003C2 [ 328.871070] decoded fault status: SLAVE FAULT [ 328.871070] exception type 0xC2: TRANSLATION_FAULT_LEVEL2 [ 328.871070] access type 0x3: WRITE [ 328.871070] source id 0xAA00 [ 329.373327] panfrost 1800000.gpu: gpu sched timeout, js=1, config=0x3700, status=0x8, head=0xa1a4900, tail=0xa1a4900, sched_job=000000007ac31097 [ 329.386527] panfrost 1800000.gpu: js fault, js=0, status=DATA_INVALID_FAULT, head=0xa1a4c00, tail=0xa1a4c00 [ 329.396293] panfrost 1800000.gpu: gpu sched timeout, js=0, config=0x3700, status=0x58, head=0xa1a4c00, tail=0xa1a4c00, sched_job=0000000004c90381 [ 329.411521] panfrost 1800000.gpu: Unhandled Page fault in AS0 at VA 0x0000000000000000 [ 329.411521] Reason: TODO [ 329.411521] raw fault status: 0xAA0003C2 [ 329.411521] decoded fault status: SLAVE FAULT [ 329.411521] exception type 0xC2: TRANSLATION_FAULT_LEVEL2 [ 329.411521] access type 0x3: WRITE [ 329.411521] source id 0xAA00
Just to keep a track of this issue.
Piotr Oniszczuk give more test and seems to be software related: https://www.spinics.net/lists/dri-devel/msg264279.html
Ondrej gave a great explanation about a possible origin of this issue: https://freenode.irclog.whitequark.org/linux-sunxi/2020-07-11
20:12 <megi> looks like gpu pll on H6 is NKMP clock, and those are implemented in such a way in mainline that they are prone to overshooting the frequency during output divider reduction 20:13 <megi> so disabling P divider may help 20:13 <megi> or fixing the dividers 20:14 <megi> and just allowing N to change 20:22 <megi> hmm, I haven't looked at this for quite some time, but H6 BSP way of setting PLL factors actually makes the most sense out of everything I've seen/tested so far 20:23 <megi> it waits for lock not after setting NK factors, but after reducing the M factor (pre-divider) 20:24 <megi> I might as well re-run my CPU PLL tester with this algorithm, to see if it fixes the lockups 20:26 <megi> it makes sense to wait for PLL to stabilize "after" changing all the factors that actually affect the VCO, and not just some of them 20:27 <megi> warpme_: ^ 20:28 <megi> it may be the same thing that plagues the CPU PLL rate changes at runtime
I guess it's one of the bugs we never heard of...
It would be a good idea to test it on another platform (like Rockchip?) to rule out any driver issue?
What do you think?
I can't exclude a bug in the driver, but if it was the case LE community or Panfrost maintainer would have heard of that.
Megi's explanations match what I observed. NKMP drivers seem the perfect guilty here or maybe it's a combination of both...
Jernej sent me this patch to test: https://github.com/clementperon/linux/commit/56bde359beaf8e827ce53ede1fe4a0a... But it didn't fix the issue, If someone want to have a look at it :)
Not sure how that patch is supposed to work, but it seems to apply all factors at once to me.
regards, o.
Regards, Clement
Maxime
Dne petek, 28. avgust 2020 ob 14:21:19 CEST je Ondřej Jirman napisal(a):
On Fri, Aug 28, 2020 at 02:16:36PM +0200, Clément Péron wrote:
Hi Maxime,
On Tue, 25 Aug 2020 at 15:35, Maxime Ripard maxime@cerno.tech wrote:
Hi Clement,
On Mon, Aug 03, 2020 at 09:54:05AM +0200, Clément Péron wrote:
Hi Maxime and All,
On Sat, 4 Jul 2020 at 16:56, Clément Péron peron.clem@gmail.com
wrote:
Hi Maxime,
On Sat, 4 Jul 2020 at 14:13, Maxime Ripard maxime@cerno.tech
wrote:
Hi,
On Sat, Jul 04, 2020 at 12:25:34PM +0200, Clément Péron wrote: > Add an Operating Performance Points table for the GPU to > enable Dynamic Voltage & Frequency Scaling on the H6. > > The voltage range is set with minival voltage set to the target > and the maximal voltage set to 1.2V. This allow DVFS framework > to > work properly on board with fixed regulator. > > Signed-off-by: Clément Péron peron.clem@gmail.com
That patch seems reasonable, why shouldn't we merge it?
I didn't test it a lot and last time I did, some frequencies looked unstable. https://lore.kernel.org/patchwork/cover/1239739/
This series adds regulator support to Panfrost devfreq, I will send a new one if DVFS on the H6 GPU is stable.
I got this running glmark2 last time
# glmark2-es2-drm
glmark2 2017.07=======================================================
OpenGL Information GL_VENDOR: Panfrost GL_RENDERER: Mali T720 (Panfrost) GL_VERSION: OpenGL ES 2.0 Mesa 20.0.5=======================================================
[ 93.550063] panfrost 1800000.gpu: GPU Fault 0x00000088 (UNKNOWN) at 0x0000000080117100 [ 94.045401] panfrost 1800000.gpu: gpu sched timeout, js=0, config=0x3700, status=0x8, head=0x21d6c00, tail=0x21d6c00, sched_job=00000000e3c2132f
[ 328.871070] panfrost 1800000.gpu: Unhandled Page fault in AS0 at VA 0x0000000000000000 [ 328.871070] Reason: TODO [ 328.871070] raw fault status: 0xAA0003C2 [ 328.871070] decoded fault status: SLAVE FAULT [ 328.871070] exception type 0xC2: TRANSLATION_FAULT_LEVEL2 [ 328.871070] access type 0x3: WRITE [ 328.871070] source id 0xAA00 [ 329.373327] panfrost 1800000.gpu: gpu sched timeout, js=1, config=0x3700, status=0x8, head=0xa1a4900, tail=0xa1a4900, sched_job=000000007ac31097 [ 329.386527] panfrost 1800000.gpu: js fault, js=0, status=DATA_INVALID_FAULT, head=0xa1a4c00, tail=0xa1a4c00 [ 329.396293] panfrost 1800000.gpu: gpu sched timeout, js=0, config=0x3700, status=0x58, head=0xa1a4c00, tail=0xa1a4c00, sched_job=0000000004c90381 [ 329.411521] panfrost 1800000.gpu: Unhandled Page fault in AS0 at VA 0x0000000000000000 [ 329.411521] Reason: TODO [ 329.411521] raw fault status: 0xAA0003C2 [ 329.411521] decoded fault status: SLAVE FAULT [ 329.411521] exception type 0xC2: TRANSLATION_FAULT_LEVEL2 [ 329.411521] access type 0x3: WRITE [ 329.411521] source id 0xAA00
Just to keep a track of this issue.
Piotr Oniszczuk give more test and seems to be software related: https://www.spinics.net/lists/dri-devel/msg264279.html
Ondrej gave a great explanation about a possible origin of this issue: https://freenode.irclog.whitequark.org/linux-sunxi/2020-07-11
20:12 <megi> looks like gpu pll on H6 is NKMP clock, and those are implemented in such a way in mainline that they are prone to overshooting the frequency during output divider reduction 20:13 <megi> so disabling P divider may help 20:13 <megi> or fixing the dividers 20:14 <megi> and just allowing N to change 20:22 <megi> hmm, I haven't looked at this for quite some time, but H6 BSP way of setting PLL factors actually makes the most sense out of everything I've seen/tested so far 20:23 <megi> it waits for lock not after setting NK factors, but after reducing the M factor (pre-divider) 20:24 <megi> I might as well re-run my CPU PLL tester with this algorithm, to see if it fixes the lockups 20:26 <megi> it makes sense to wait for PLL to stabilize "after" changing all the factors that actually affect the VCO, and not just some of them 20:27 <megi> warpme_: ^ 20:28 <megi> it may be the same thing that plagues the CPU PLL rate changes at runtime
I guess it's one of the bugs we never heard of...
It would be a good idea to test it on another platform (like Rockchip?) to rule out any driver issue?
What do you think?
I can't exclude a bug in the driver, but if it was the case LE community or Panfrost maintainer would have heard of that.
Megi's explanations match what I observed. NKMP drivers seem the perfect guilty here or maybe it's a combination of both...
Jernej sent me this patch to test: https://github.com/clementperon/linux/commit/56bde359beaf8e827ce53ede1fe4a 0ad233cb79b But it didn't fix the issue, If someone want to have a look at it :)
Not sure how that patch is supposed to work, but it seems to apply all factors at once to me.
It's hackish but in essence, it is what vendor clock driver does. From A83T or so onwards, vendor driver uses "lock_mode = PLL_LOCK_NEW_MODE" and this hack tries to mimick that locking behaviour (good enough for tests).
And yes, this code still applies all factors at once, because vendor driver does that too on H6. Only case where vendor driver applies factors one by one is for V3s SoC.
Btw, only recently I noticed following block in h6 clk driver: https://elixir.bootlin.com/linux/latest/source/drivers/clk/sunxi-ng/ccu-sun5...
Other clk drivers don't have it. Maybe it has some influence in this matter.
Best regards, Jernej
regards, o.
Regards, Clement
Maxime
Signed-off-by: Clément Péron peron.clem@gmail.com --- arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts index 3f7ceeb1a767..14257f7476b8 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts @@ -245,6 +245,7 @@ reg_dcdca: dcdca { };
reg_dcdcc: dcdcc { + regulator-always-on; regulator-enable-ramp-delay = <32000>; regulator-min-microvolt = <810000>; regulator-max-microvolt = <1080000>;
Hi,
On Sat, 4 Jul 2020 at 12:25, Clément Péron peron.clem@gmail.com wrote:
Signed-off-by: Clément Péron peron.clem@gmail.com
arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts index 3f7ceeb1a767..14257f7476b8 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts @@ -245,6 +245,7 @@ reg_dcdca: dcdca { };
reg_dcdcc: dcdcc {
regulator-always-on;
This patch is normally no more required since this serie: https://lore.kernel.org/linux-pm/cover.1589528491.git.viresh.kumar@linaro.or...
regulator-enable-ramp-delay = <32000>; regulator-min-microvolt = <810000>; regulator-max-microvolt = <1080000>;-- 2.25.1
Hi,
On Sat, 4 Jul 2020 at 12:25, Clément Péron peron.clem@gmail.com wrote:
Hi,
This serie cleans and adds regulator support to Panfrost devfreq. This is mostly based on comment for the freshly introduced lima devfreq.
We need to add regulator support because on Allwinner the GPU OPP table defines both frequencies and voltages.
First patches [01-07] should not change the actual behavior and introduce a proper panfrost_devfreq struct.
Regards, Clément
Changes since v1: - Collect Steven Price reviewed-by tags - Fix spinlock comment - Drop OPP clock-name path - Drop device_property_test patch - Add rename error labels patch
Clément Péron (14): drm/panfrost: avoid static declaration drm/panfrost: clean headers in devfreq drm/panfrost: don't use pfdevfreq.busy_count to know if hw is idle drm/panfrost: introduce panfrost_devfreq struct drm/panfrost: use spinlock instead of atomic drm/panfrost: properly handle error in probe drm/panfrost: rename error labels in device_init drm/panfrost: move devfreq_init()/fini() in device drm/panfrost: dynamically alloc regulators drm/panfrost: add regulators to devfreq arm64: defconfig: Enable devfreq cooling device arm64: dts: allwinner: h6: Add cooling map for GPU [DO NOT MERGE] arm64: dts: allwinner: h6: Add GPU OPP table [DO NOT MERGE] arm64: dts: allwinner: force GPU regulator to be always
.../dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 + arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 102 +++++++++++ arch/arm64/configs/defconfig | 1 + drivers/gpu/drm/panfrost/panfrost_devfreq.c | 165 ++++++++++++------ drivers/gpu/drm/panfrost/panfrost_devfreq.h | 30 +++- drivers/gpu/drm/panfrost/panfrost_device.c | 61 ++++--- drivers/gpu/drm/panfrost/panfrost_device.h | 14 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 15 +- drivers/gpu/drm/panfrost/panfrost_job.c | 10 +- 9 files changed, 290 insertions(+), 109 deletions(-)
-- 2.25.1
Patches 1-12 are
Reviewed-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com
Thank you!
On Sat, Jul 04, 2020 at 12:25:21PM +0200, Clément Péron wrote:
Hi,
This serie cleans and adds regulator support to Panfrost devfreq. This is mostly based on comment for the freshly introduced lima devfreq.
We need to add regulator support because on Allwinner the GPU OPP table defines both frequencies and voltages.
First patches [01-07] should not change the actual behavior and introduce a proper panfrost_devfreq struct.
Regards, Clément
Clément Péron (14): drm/panfrost: avoid static declaration drm/panfrost: clean headers in devfreq drm/panfrost: don't use pfdevfreq.busy_count to know if hw is idle drm/panfrost: introduce panfrost_devfreq struct drm/panfrost: use spinlock instead of atomic drm/panfrost: properly handle error in probe drm/panfrost: rename error labels in device_init drm/panfrost: move devfreq_init()/fini() in device drm/panfrost: dynamically alloc regulators drm/panfrost: add regulators to devfreq arm64: defconfig: Enable devfreq cooling device arm64: dts: allwinner: h6: Add cooling map for GPU [DO NOT MERGE] arm64: dts: allwinner: h6: Add GPU OPP table [DO NOT MERGE] arm64: dts: allwinner: force GPU regulator to be always
.../dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 + arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 102 +++++++++++ arch/arm64/configs/defconfig | 1 + drivers/gpu/drm/panfrost/panfrost_devfreq.c | 165 ++++++++++++------ drivers/gpu/drm/panfrost/panfrost_devfreq.h | 30 +++- drivers/gpu/drm/panfrost/panfrost_device.c | 61 ++++--- drivers/gpu/drm/panfrost/panfrost_device.h | 14 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 15 +- drivers/gpu/drm/panfrost/panfrost_job.c | 10 +- 9 files changed, 290 insertions(+), 109 deletions(-)
-- 2.25.1
Hello Clément,
On Sat, Jul 04, 2020 at 12:25:21PM +0200, Clément Péron wrote:
Hi,
This serie cleans and adds regulator support to Panfrost devfreq. This is mostly based on comment for the freshly introduced lima devfreq.
I tried to test the series, but I'm unsure what it's meant to be based on.
It doesn't appply on top of linux-next and while it applies on top of 5.8-rc3, it fails to run due to ordering of
dev_pm_opp_set_regulators and dev_pm_opp_of_add_table
where this patch series places
dev_pm_opp_of_add_table after dev_pm_opp_of_add_table
which fails with this warning:
https://elixir.bootlin.com/linux/v5.8-rc3/source/drivers/opp/core.c#L1696
[ 0.155455] ------------[ cut here ]------------ [ 0.155473] WARNING: CPU: 2 PID: 1 at drivers/opp/core.c:1696 dev_pm_opp_set_regulators+0x134/0x1f0 [ 0.155476] Modules linked in: [ 0.155487] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc4-00328-gf89269f4a65c #12 [ 0.155489] Hardware name: OrangePi 3 (DT) [ 0.155496] pstate: 20000005 (nzCv daif -PAN -UAO BTYPE=--) [ 0.155502] pc : dev_pm_opp_set_regulators+0x134/0x1f0 [ 0.155507] lr : dev_pm_opp_set_regulators+0x28/0x1f0 [ 0.155510] sp : ffffffc01002bb00 [ 0.155512] x29: ffffffc01002bb00 x28: 0000000000000000 [ 0.155518] x27: ffffffc0113b03b0 x26: ffffffc011431960 [ 0.155523] x25: ffffffc011397a70 x24: ffffff807b6a2410 [ 0.155528] x23: 0000000000000001 x22: ffffffc010f290a0 [ 0.155533] x21: ffffff80789e3880 x20: ffffff80789e3ac8 [ 0.155538] x19: ffffff80789e4400 x18: 00000000fffffffe [ 0.155543] x17: 0000000000000001 x16: 0000000000000019 [ 0.155548] x15: 0000000000000001 x14: ffffffffffffffff [ 0.155553] x13: ffffffc01169fe00 x12: 0000000000000005 [ 0.155558] x11: 0000000000000007 x10: 0101010101010101 [ 0.155563] x9 : ffffffffffffffff x8 : 7f7f7f7f7f7f7f7f [ 0.155568] x7 : fefefeff646c606d x6 : 01111d48f3f5f3f0 [ 0.155573] x5 : 70737573481d1101 x4 : 0000000000000000 [ 0.155577] x3 : ffffff80789e4450 x2 : 0000000000000000 [ 0.155582] x1 : ffffff807b490000 x0 : ffffff8078c2fe00 [ 0.155587] Call trace: [ 0.155595] dev_pm_opp_set_regulators+0x134/0x1f0 [ 0.155603] panfrost_devfreq_init+0x70/0x178 [ 0.155608] panfrost_device_init+0x108/0x5d8 [ 0.155613] panfrost_probe+0xa4/0x178 [ 0.155619] platform_drv_probe+0x50/0xa0 [ 0.155626] really_probe+0xd4/0x318 [ 0.155631] driver_probe_device+0x54/0xb0 [ 0.155638] device_driver_attach+0x6c/0x78 [ 0.155643] __driver_attach+0x54/0xd0 [ 0.155649] bus_for_each_dev+0x5c/0x98 [ 0.155654] driver_attach+0x20/0x28 [ 0.155660] bus_add_driver+0x140/0x1e8 [ 0.155666] driver_register+0x60/0x110 [ 0.155670] __platform_driver_register+0x44/0x50 [ 0.155678] panfrost_driver_init+0x18/0x20 [ 0.155685] do_one_initcall+0x3c/0x160 [ 0.155691] kernel_init_freeable+0x20c/0x2b0 [ 0.155698] kernel_init+0x10/0x104 [ 0.155703] ret_from_fork+0x10/0x1c [ 0.155712] ---[ end trace ed26920b0484a95e ]--- [ 0.155725] panfrost 1800000.gpu: [drm:panfrost_devfreq_init] *ERROR* Couldn't set OPP regulators [ 0.156710] panfrost 1800000.gpu: devfreq init failed -16 [ 0.156725] panfrost 1800000.gpu: Fatal error during GPU init [ 0.156795] panfrost: probe of 1800000.gpu failed with error -16 [ 0.157158] cacheinfo: Unable to detect cache hierarchy for CPU 0
thank you and regards, o.
We need to add regulator support because on Allwinner the GPU OPP table defines both frequencies and voltages.
First patches [01-07] should not change the actual behavior and introduce a proper panfrost_devfreq struct.
Regards, Clément
Clément Péron (14): drm/panfrost: avoid static declaration drm/panfrost: clean headers in devfreq drm/panfrost: don't use pfdevfreq.busy_count to know if hw is idle drm/panfrost: introduce panfrost_devfreq struct drm/panfrost: use spinlock instead of atomic drm/panfrost: properly handle error in probe drm/panfrost: rename error labels in device_init drm/panfrost: move devfreq_init()/fini() in device drm/panfrost: dynamically alloc regulators drm/panfrost: add regulators to devfreq arm64: defconfig: Enable devfreq cooling device arm64: dts: allwinner: h6: Add cooling map for GPU [DO NOT MERGE] arm64: dts: allwinner: h6: Add GPU OPP table [DO NOT MERGE] arm64: dts: allwinner: force GPU regulator to be always
.../dts/allwinner/sun50i-h6-beelink-gs1.dts | 1 + arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 102 +++++++++++ arch/arm64/configs/defconfig | 1 + drivers/gpu/drm/panfrost/panfrost_devfreq.c | 165 ++++++++++++------ drivers/gpu/drm/panfrost/panfrost_devfreq.h | 30 +++- drivers/gpu/drm/panfrost/panfrost_device.c | 61 ++++--- drivers/gpu/drm/panfrost/panfrost_device.h | 14 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 15 +- drivers/gpu/drm/panfrost/panfrost_job.c | 10 +- 9 files changed, 290 insertions(+), 109 deletions(-)
-- 2.25.1
dri-devel@lists.freedesktop.org
-
Alyssa Rosenzweig -
Clément Péron -
Jernej Škrabec -
Maxime Ripard -
Ondřej Jirman -
Ondřej Jirman