Re: [PATCH 4/5] drm/v3d: Create function to update a set of GPU stats

2024-04-15 Thread Chema Casanova

El 3/4/24 a las 22:24, Maíra Canal escribió:

Given a set of GPU stats, that is, a `struct v3d_stats` related to a
queue in a given context, create a function that can update all this set of
GPU stats.

Signed-off-by: Maíra Canal 
---
  drivers/gpu/drm/v3d/v3d_sched.c | 20 
  1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index ea5f5a84b55b..754107b80f67 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -118,6 +118,16 @@ v3d_job_start_stats(struct v3d_job *job, enum v3d_queue 
queue)
global_stats->start_ns = now;
  }
  
+static void

+v3d_stats_update(struct v3d_stats *stats)
+{
+   u64 now = local_clock();

I understand that with this change, we would be calling twice local_clock()
for each stat update, once for local and one for global stats. I don't know
the performance impact of the extra local_clock(), but I understand as 
you are

always updating global_stats after local_stats we would be losing the
correspondence that global_stats is the sum of all local_stats for all 
process.

With this approach, it will be always greater or equal.

Maybe it makes sense to pass an extra parameter now so save one
local_clock() and the sum of global and local stats is the same when you 
only

have one process running.



+
+   stats->enabled_ns += now - stats->start_ns;
+   stats->jobs_sent++;
+   stats->start_ns = 0;
+}
+
  void
  v3d_job_update_stats(struct v3d_job *job, enum v3d_queue queue)
  {
@@ -125,15 +135,9 @@ v3d_job_update_stats(struct v3d_job *job, enum v3d_queue 
queue)
struct v3d_file_priv *file = job->file->driver_priv;
struct v3d_stats *global_stats = &v3d->queue[queue].stats;
struct v3d_stats *local_stats = &file->stats[queue];
-   u64 now = local_clock();
-
-   local_stats->enabled_ns += now - local_stats->start_ns;
-   local_stats->jobs_sent++;
-   local_stats->start_ns = 0;
  
-	global_stats->enabled_ns += now - global_stats->start_ns;

-   global_stats->jobs_sent++;
-   global_stats->start_ns = 0;
+   v3d_stats_update(local_stats);
+   v3d_stats_update(global_stats);
  }
  
  static struct dma_fence *v3d_bin_job_run(struct drm_sched_job *sched_job)


Re: [PATCH 4/5] drm/v3d: Create function to update a set of GPU stats

2024-04-15 Thread Tvrtko Ursulin



On 03/04/2024 21:24, Maíra Canal wrote:

Given a set of GPU stats, that is, a `struct v3d_stats` related to a
queue in a given context, create a function that can update all this set of
GPU stats.

Signed-off-by: Maíra Canal 
---
  drivers/gpu/drm/v3d/v3d_sched.c | 20 
  1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index ea5f5a84b55b..754107b80f67 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -118,6 +118,16 @@ v3d_job_start_stats(struct v3d_job *job, enum v3d_queue 
queue)
global_stats->start_ns = now;
  }
  
+static void

+v3d_stats_update(struct v3d_stats *stats)
+{
+   u64 now = local_clock();
+
+   stats->enabled_ns += now - stats->start_ns;
+   stats->jobs_sent++;
+   stats->start_ns = 0;
+}
+
  void
  v3d_job_update_stats(struct v3d_job *job, enum v3d_queue queue)
  {
@@ -125,15 +135,9 @@ v3d_job_update_stats(struct v3d_job *job, enum v3d_queue 
queue)
struct v3d_file_priv *file = job->file->driver_priv;
struct v3d_stats *global_stats = &v3d->queue[queue].stats;
struct v3d_stats *local_stats = &file->stats[queue];
-   u64 now = local_clock();
-
-   local_stats->enabled_ns += now - local_stats->start_ns;
-   local_stats->jobs_sent++;
-   local_stats->start_ns = 0;
  
-	global_stats->enabled_ns += now - global_stats->start_ns;

-   global_stats->jobs_sent++;
-   global_stats->start_ns = 0;
+   v3d_stats_update(local_stats);
+   v3d_stats_update(global_stats);
  }
  
  static struct dma_fence *v3d_bin_job_run(struct drm_sched_job *sched_job)


Reviewed-by: Tvrtko Ursulin 

Regards,

Tvrtko


[PATCH 4/5] drm/v3d: Create function to update a set of GPU stats

2024-04-03 Thread Maíra Canal
Given a set of GPU stats, that is, a `struct v3d_stats` related to a
queue in a given context, create a function that can update all this set of
GPU stats.

Signed-off-by: Maíra Canal 
---
 drivers/gpu/drm/v3d/v3d_sched.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index ea5f5a84b55b..754107b80f67 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -118,6 +118,16 @@ v3d_job_start_stats(struct v3d_job *job, enum v3d_queue 
queue)
global_stats->start_ns = now;
 }
 
+static void
+v3d_stats_update(struct v3d_stats *stats)
+{
+   u64 now = local_clock();
+
+   stats->enabled_ns += now - stats->start_ns;
+   stats->jobs_sent++;
+   stats->start_ns = 0;
+}
+
 void
 v3d_job_update_stats(struct v3d_job *job, enum v3d_queue queue)
 {
@@ -125,15 +135,9 @@ v3d_job_update_stats(struct v3d_job *job, enum v3d_queue 
queue)
struct v3d_file_priv *file = job->file->driver_priv;
struct v3d_stats *global_stats = &v3d->queue[queue].stats;
struct v3d_stats *local_stats = &file->stats[queue];
-   u64 now = local_clock();
-
-   local_stats->enabled_ns += now - local_stats->start_ns;
-   local_stats->jobs_sent++;
-   local_stats->start_ns = 0;
 
-   global_stats->enabled_ns += now - global_stats->start_ns;
-   global_stats->jobs_sent++;
-   global_stats->start_ns = 0;
+   v3d_stats_update(local_stats);
+   v3d_stats_update(global_stats);
 }
 
 static struct dma_fence *v3d_bin_job_run(struct drm_sched_job *sched_job)
-- 
2.44.0