drm/i915: Move intel_engine_context_in/out into intel_lrc.c
diff mbox series

Message ID 20191025090952.10135-1-tvrtko.ursulin@linux.intel.com
State New
Headers show
Series
  • drm/i915: Move intel_engine_context_in/out into intel_lrc.c
Related show

Commit Message

Tvrtko Ursulin Oct. 25, 2019, 9:09 a.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Intel_lrc.c is the only caller and so to avoid some header file ordering
issues in future patches move these two over there.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine.h | 55 --------------------------
 drivers/gpu/drm/i915/gt/intel_lrc.c    | 55 ++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 55 deletions(-)

Comments

Chris Wilson Oct. 25, 2019, 9:13 a.m. UTC | #1
Quoting Tvrtko Ursulin (2019-10-25 10:09:52)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Intel_lrc.c is the only caller and so to avoid some header file ordering
> issues in future patches move these two over there.

How much pain would you feel if we did intel_lrc.c +
intel_execlists_submission.c earlier rather than later?

> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine.h | 55 --------------------------
>  drivers/gpu/drm/i915/gt/intel_lrc.c    | 55 ++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index 97bbdd9773c9..c6895938b626 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -290,61 +290,6 @@ void intel_engine_dump(struct intel_engine_cs *engine,
>                        struct drm_printer *m,
>                        const char *header, ...);
>  
> -static inline void intel_engine_context_in(struct intel_engine_cs *engine)
> -{
> -       unsigned long flags;
> -
> -       if (READ_ONCE(engine->stats.enabled) == 0)
> -               return;
> -
> -       write_seqlock_irqsave(&engine->stats.lock, flags);
> -
> -       if (engine->stats.enabled > 0) {
> -               if (engine->stats.active++ == 0)
> -                       engine->stats.start = ktime_get();
> -               GEM_BUG_ON(engine->stats.active == 0);
> -       }
> -
> -       write_sequnlock_irqrestore(&engine->stats.lock, flags);
> -}
> -
> -static inline void intel_engine_context_out(struct intel_engine_cs *engine)
> -{
> -       unsigned long flags;
> -
> -       if (READ_ONCE(engine->stats.enabled) == 0)
> -               return;
> -
> -       write_seqlock_irqsave(&engine->stats.lock, flags);
> -
> -       if (engine->stats.enabled > 0) {
> -               ktime_t last;
> -
> -               if (engine->stats.active && --engine->stats.active == 0) {
> -                       /*
> -                        * Decrement the active context count and in case GPU
> -                        * is now idle add up to the running total.
> -                        */
> -                       last = ktime_sub(ktime_get(), engine->stats.start);
> -
> -                       engine->stats.total = ktime_add(engine->stats.total,
> -                                                       last);
> -               } else if (engine->stats.active == 0) {
> -                       /*
> -                        * After turning on engine stats, context out might be
> -                        * the first event in which case we account from the
> -                        * time stats gathering was turned on.
> -                        */
> -                       last = ktime_sub(ktime_get(), engine->stats.enabled_at);
> -
> -                       engine->stats.total = ktime_add(engine->stats.total,
> -                                                       last);
> -               }
> -       }
> -
> -       write_sequnlock_irqrestore(&engine->stats.lock, flags);
> -}
> -
>  int intel_enable_engine_stats(struct intel_engine_cs *engine);
>  void intel_disable_engine_stats(struct intel_engine_cs *engine);
>  
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 73eae85a2cc9..523de1fd4452 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -944,6 +944,61 @@ execlists_context_status_change(struct i915_request *rq, unsigned long status)
>                                    status, rq);
>  }
>  
> +static void intel_engine_context_in(struct intel_engine_cs *engine)

stats_in() / stats_out() ?

Now that's it entirely local and we may end up doing other per-context
in/out ops?

Purely mechanical, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Tvrtko Ursulin Oct. 25, 2019, 12:43 p.m. UTC | #2
On 25/10/2019 10:13, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-10-25 10:09:52)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Intel_lrc.c is the only caller and so to avoid some header file ordering
>> issues in future patches move these two over there.
> 
> How much pain would you feel if we did intel_lrc.c +
> intel_execlists_submission.c earlier rather than later?

Par for course as you like to say. :)

>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_engine.h | 55 --------------------------
>>   drivers/gpu/drm/i915/gt/intel_lrc.c    | 55 ++++++++++++++++++++++++++
>>   2 files changed, 55 insertions(+), 55 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
>> index 97bbdd9773c9..c6895938b626 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
>> @@ -290,61 +290,6 @@ void intel_engine_dump(struct intel_engine_cs *engine,
>>                         struct drm_printer *m,
>>                         const char *header, ...);
>>   
>> -static inline void intel_engine_context_in(struct intel_engine_cs *engine)
>> -{
>> -       unsigned long flags;
>> -
>> -       if (READ_ONCE(engine->stats.enabled) == 0)
>> -               return;
>> -
>> -       write_seqlock_irqsave(&engine->stats.lock, flags);
>> -
>> -       if (engine->stats.enabled > 0) {
>> -               if (engine->stats.active++ == 0)
>> -                       engine->stats.start = ktime_get();
>> -               GEM_BUG_ON(engine->stats.active == 0);
>> -       }
>> -
>> -       write_sequnlock_irqrestore(&engine->stats.lock, flags);
>> -}
>> -
>> -static inline void intel_engine_context_out(struct intel_engine_cs *engine)
>> -{
>> -       unsigned long flags;
>> -
>> -       if (READ_ONCE(engine->stats.enabled) == 0)
>> -               return;
>> -
>> -       write_seqlock_irqsave(&engine->stats.lock, flags);
>> -
>> -       if (engine->stats.enabled > 0) {
>> -               ktime_t last;
>> -
>> -               if (engine->stats.active && --engine->stats.active == 0) {
>> -                       /*
>> -                        * Decrement the active context count and in case GPU
>> -                        * is now idle add up to the running total.
>> -                        */
>> -                       last = ktime_sub(ktime_get(), engine->stats.start);
>> -
>> -                       engine->stats.total = ktime_add(engine->stats.total,
>> -                                                       last);
>> -               } else if (engine->stats.active == 0) {
>> -                       /*
>> -                        * After turning on engine stats, context out might be
>> -                        * the first event in which case we account from the
>> -                        * time stats gathering was turned on.
>> -                        */
>> -                       last = ktime_sub(ktime_get(), engine->stats.enabled_at);
>> -
>> -                       engine->stats.total = ktime_add(engine->stats.total,
>> -                                                       last);
>> -               }
>> -       }
>> -
>> -       write_sequnlock_irqrestore(&engine->stats.lock, flags);
>> -}
>> -
>>   int intel_enable_engine_stats(struct intel_engine_cs *engine);
>>   void intel_disable_engine_stats(struct intel_engine_cs *engine);
>>   
>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> index 73eae85a2cc9..523de1fd4452 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> @@ -944,6 +944,61 @@ execlists_context_status_change(struct i915_request *rq, unsigned long status)
>>                                     status, rq);
>>   }
>>   
>> +static void intel_engine_context_in(struct intel_engine_cs *engine)
> 
> stats_in() / stats_out() ?
> 
> Now that's it entirely local and we may end up doing other per-context
> in/out ops?

Yeah, could make sense. I did rename it to intel_context_in/out in a 
local patch which adds per ce stats. Lets see when I un-bit-rot that 
work how it will look.

> Purely mechanical, so
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

In the meantime I have pushed this so at least header file is smaller. 
Thanks.

Regards,

Tvrtko

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index 97bbdd9773c9..c6895938b626 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -290,61 +290,6 @@  void intel_engine_dump(struct intel_engine_cs *engine,
 		       struct drm_printer *m,
 		       const char *header, ...);
 
-static inline void intel_engine_context_in(struct intel_engine_cs *engine)
-{
-	unsigned long flags;
-
-	if (READ_ONCE(engine->stats.enabled) == 0)
-		return;
-
-	write_seqlock_irqsave(&engine->stats.lock, flags);
-
-	if (engine->stats.enabled > 0) {
-		if (engine->stats.active++ == 0)
-			engine->stats.start = ktime_get();
-		GEM_BUG_ON(engine->stats.active == 0);
-	}
-
-	write_sequnlock_irqrestore(&engine->stats.lock, flags);
-}
-
-static inline void intel_engine_context_out(struct intel_engine_cs *engine)
-{
-	unsigned long flags;
-
-	if (READ_ONCE(engine->stats.enabled) == 0)
-		return;
-
-	write_seqlock_irqsave(&engine->stats.lock, flags);
-
-	if (engine->stats.enabled > 0) {
-		ktime_t last;
-
-		if (engine->stats.active && --engine->stats.active == 0) {
-			/*
-			 * Decrement the active context count and in case GPU
-			 * is now idle add up to the running total.
-			 */
-			last = ktime_sub(ktime_get(), engine->stats.start);
-
-			engine->stats.total = ktime_add(engine->stats.total,
-							last);
-		} else if (engine->stats.active == 0) {
-			/*
-			 * After turning on engine stats, context out might be
-			 * the first event in which case we account from the
-			 * time stats gathering was turned on.
-			 */
-			last = ktime_sub(ktime_get(), engine->stats.enabled_at);
-
-			engine->stats.total = ktime_add(engine->stats.total,
-							last);
-		}
-	}
-
-	write_sequnlock_irqrestore(&engine->stats.lock, flags);
-}
-
 int intel_enable_engine_stats(struct intel_engine_cs *engine);
 void intel_disable_engine_stats(struct intel_engine_cs *engine);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 73eae85a2cc9..523de1fd4452 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -944,6 +944,61 @@  execlists_context_status_change(struct i915_request *rq, unsigned long status)
 				   status, rq);
 }
 
+static void intel_engine_context_in(struct intel_engine_cs *engine)
+{
+	unsigned long flags;
+
+	if (READ_ONCE(engine->stats.enabled) == 0)
+		return;
+
+	write_seqlock_irqsave(&engine->stats.lock, flags);
+
+	if (engine->stats.enabled > 0) {
+		if (engine->stats.active++ == 0)
+			engine->stats.start = ktime_get();
+		GEM_BUG_ON(engine->stats.active == 0);
+	}
+
+	write_sequnlock_irqrestore(&engine->stats.lock, flags);
+}
+
+static void intel_engine_context_out(struct intel_engine_cs *engine)
+{
+	unsigned long flags;
+
+	if (READ_ONCE(engine->stats.enabled) == 0)
+		return;
+
+	write_seqlock_irqsave(&engine->stats.lock, flags);
+
+	if (engine->stats.enabled > 0) {
+		ktime_t last;
+
+		if (engine->stats.active && --engine->stats.active == 0) {
+			/*
+			 * Decrement the active context count and in case GPU
+			 * is now idle add up to the running total.
+			 */
+			last = ktime_sub(ktime_get(), engine->stats.start);
+
+			engine->stats.total = ktime_add(engine->stats.total,
+							last);
+		} else if (engine->stats.active == 0) {
+			/*
+			 * After turning on engine stats, context out might be
+			 * the first event in which case we account from the
+			 * time stats gathering was turned on.
+			 */
+			last = ktime_sub(ktime_get(), engine->stats.enabled_at);
+
+			engine->stats.total = ktime_add(engine->stats.total,
+							last);
+		}
+	}
+
+	write_sequnlock_irqrestore(&engine->stats.lock, flags);
+}
+
 static inline struct intel_engine_cs *
 __execlists_schedule_in(struct i915_request *rq)
 {