On 2020-05-10 17:55, Clément Péron wrote:
Convert busy_count to a simple int protected by spinlock.

A little more reasoning might be nice.

Signed-off-by: Clément Péron <peron.c...@gmail.com>
---
[...]
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.h 
b/drivers/gpu/drm/panfrost/panfrost_devfreq.h
index 0697f8d5aa34..e6629900a618 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,17 @@ 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, for example by the GP
+        * and PP interrupts.
+        */

Nit: this comment is clearly wrong, since we only have Job, GPU and MMU interrupts here. I guess if there is a race it would be between submission/completion/timeout on different job slots.

Given that, should this actually be considered a fix for 9e62b885f715 ("drm/panfrost: Simplify devfreq utilisation tracking")?

Robin.

Reply via email to