diff mbox

[1/2] drm/i915/guc: Support engine busy stats

Message ID 20180222060732.4381-1-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tvrtko Ursulin Feb. 22, 2018, 6:07 a.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Place context in/out hooks into the GuC backend, when contexts are
assigned to ports, and removed from them, in order to be able to
provide engine busy stats in GuC mode.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Testcase: igt/perf_pmu/busy-accuracy-*-*
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Chris Wilson Feb. 22, 2018, 7:51 a.m. UTC | #1
Quoting Tvrtko Ursulin (2018-02-22 06:07:32)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Place context in/out hooks into the GuC backend, when contexts are
> assigned to ports, and removed from them, in order to be able to
> provide engine busy stats in GuC mode.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Testcase: igt/perf_pmu/busy-accuracy-*-*
> ---
>  drivers/gpu/drm/i915/intel_guc_submission.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 649113c7a3c2..8e99f8fd6da2 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -673,6 +673,7 @@ static void guc_dequeue(struct intel_engine_cs *engine)
>         struct intel_engine_execlists * const execlists = &engine->execlists;
>         struct execlist_port *port = execlists->port;
>         struct i915_request *last = NULL;
> +       struct i915_gem_context *last_ctx = NULL;
>         const struct execlist_port * const last_port =
>                 &execlists->port[execlists->port_mask];
>         bool submit = false;
> @@ -718,8 +719,13 @@ static void guc_dequeue(struct intel_engine_cs *engine)
>                                         goto done;
>                                 }
>  
> -                               if (submit)
> +                               if (submit) {
>                                         port_assign(port, last);
> +                                       if (last->ctx != last_ctx) {
> +                                               intel_engine_context_in(last->engine);
> +                                               last_ctx = last->ctx;
> +                                       }
> +                               }
>                                 port++;
>                         }
>  
> @@ -741,6 +747,8 @@ static void guc_dequeue(struct intel_engine_cs *engine)
>         execlists->first = rb;
>         if (submit) {
>                 port_assign(port, last);
> +               if (last->ctx != last_ctx)
> +                       intel_engine_context_in(last->engine);
>                 execlists_set_active(execlists, EXECLISTS_ACTIVE_USER);
>                 guc_submit(engine);
>         }
> @@ -763,6 +771,7 @@ static void guc_submission_tasklet(unsigned long data)
>  
>         rq = port_request(&port[0]);
>         while (rq && i915_request_completed(rq)) {
> +               intel_engine_context_out(rq->engine);

If we only emit context_in once for 2 consecutive rq with the same
context, we would need to do the same for context_out.

Will be interesting to see if this explodes, or we may need yet another
test :)
-Chris
Tvrtko Ursulin Feb. 22, 2018, 8:26 a.m. UTC | #2
On 22/02/2018 07:51, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-02-22 06:07:32)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Place context in/out hooks into the GuC backend, when contexts are
>> assigned to ports, and removed from them, in order to be able to
>> provide engine busy stats in GuC mode.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Testcase: igt/perf_pmu/busy-accuracy-*-*
>> ---
>>   drivers/gpu/drm/i915/intel_guc_submission.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
>> index 649113c7a3c2..8e99f8fd6da2 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
>> @@ -673,6 +673,7 @@ static void guc_dequeue(struct intel_engine_cs *engine)
>>          struct intel_engine_execlists * const execlists = &engine->execlists;
>>          struct execlist_port *port = execlists->port;
>>          struct i915_request *last = NULL;
>> +       struct i915_gem_context *last_ctx = NULL;
>>          const struct execlist_port * const last_port =
>>                  &execlists->port[execlists->port_mask];
>>          bool submit = false;
>> @@ -718,8 +719,13 @@ static void guc_dequeue(struct intel_engine_cs *engine)
>>                                          goto done;
>>                                  }
>>   
>> -                               if (submit)
>> +                               if (submit) {
>>                                          port_assign(port, last);
>> +                                       if (last->ctx != last_ctx) {
>> +                                               intel_engine_context_in(last->engine);
>> +                                               last_ctx = last->ctx;
>> +                                       }
>> +                               }
>>                                  port++;
>>                          }
>>   
>> @@ -741,6 +747,8 @@ static void guc_dequeue(struct intel_engine_cs *engine)
>>          execlists->first = rb;
>>          if (submit) {
>>                  port_assign(port, last);
>> +               if (last->ctx != last_ctx)
>> +                       intel_engine_context_in(last->engine);
>>                  execlists_set_active(execlists, EXECLISTS_ACTIVE_USER);
>>                  guc_submit(engine);
>>          }
>> @@ -763,6 +771,7 @@ static void guc_submission_tasklet(unsigned long data)
>>   
>>          rq = port_request(&port[0]);
>>          while (rq && i915_request_completed(rq)) {
>> +               intel_engine_context_out(rq->engine);
> 
> If we only emit context_in once for 2 consecutive rq with the same
> context, we would need to do the same for context_out.
> 
> Will be interesting to see if this explodes, or we may need yet another
> test :)


I thought two consecutive requests with the same context is one port, so 
one context_in and one context_out. But maybe I'm wrong. Lets wait and 
see as you say. :)

Regards,

Tvrtko
Chris Wilson Feb. 22, 2018, 8:31 a.m. UTC | #3
Quoting Tvrtko Ursulin (2018-02-22 08:26:58)
> 
> On 22/02/2018 07:51, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-02-22 06:07:32)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Place context in/out hooks into the GuC backend, when contexts are
> >> assigned to ports, and removed from them, in order to be able to
> >> provide engine busy stats in GuC mode.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> Testcase: igt/perf_pmu/busy-accuracy-*-*
> >> ---
> >>   drivers/gpu/drm/i915/intel_guc_submission.c | 13 ++++++++++---
> >>   1 file changed, 10 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> >> index 649113c7a3c2..8e99f8fd6da2 100644
> >> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> >> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> >> @@ -673,6 +673,7 @@ static void guc_dequeue(struct intel_engine_cs *engine)
> >>          struct intel_engine_execlists * const execlists = &engine->execlists;
> >>          struct execlist_port *port = execlists->port;
> >>          struct i915_request *last = NULL;
> >> +       struct i915_gem_context *last_ctx = NULL;
> >>          const struct execlist_port * const last_port =
> >>                  &execlists->port[execlists->port_mask];
> >>          bool submit = false;
> >> @@ -718,8 +719,13 @@ static void guc_dequeue(struct intel_engine_cs *engine)
> >>                                          goto done;
> >>                                  }
> >>   
> >> -                               if (submit)
> >> +                               if (submit) {
> >>                                          port_assign(port, last);
> >> +                                       if (last->ctx != last_ctx) {
> >> +                                               intel_engine_context_in(last->engine);
> >> +                                               last_ctx = last->ctx;
> >> +                                       }
> >> +                               }
> >>                                  port++;
> >>                          }
> >>   
> >> @@ -741,6 +747,8 @@ static void guc_dequeue(struct intel_engine_cs *engine)
> >>          execlists->first = rb;
> >>          if (submit) {
> >>                  port_assign(port, last);
> >> +               if (last->ctx != last_ctx)
> >> +                       intel_engine_context_in(last->engine);
> >>                  execlists_set_active(execlists, EXECLISTS_ACTIVE_USER);
> >>                  guc_submit(engine);
> >>          }
> >> @@ -763,6 +771,7 @@ static void guc_submission_tasklet(unsigned long data)
> >>   
> >>          rq = port_request(&port[0]);
> >>          while (rq && i915_request_completed(rq)) {
> >> +               intel_engine_context_out(rq->engine);
> > 
> > If we only emit context_in once for 2 consecutive rq with the same
> > context, we would need to do the same for context_out.
> > 
> > Will be interesting to see if this explodes, or we may need yet another
> > test :)
> 
> 
> I thought two consecutive requests with the same context is one port, so 
> one context_in and one context_out. But maybe I'm wrong. Lets wait and 
> see as you say. :)

The difference is we don't do lite-restore in the guc. I think you want
to move context_in to guc_submit().
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 649113c7a3c2..8e99f8fd6da2 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -673,6 +673,7 @@  static void guc_dequeue(struct intel_engine_cs *engine)
 	struct intel_engine_execlists * const execlists = &engine->execlists;
 	struct execlist_port *port = execlists->port;
 	struct i915_request *last = NULL;
+	struct i915_gem_context *last_ctx = NULL;
 	const struct execlist_port * const last_port =
 		&execlists->port[execlists->port_mask];
 	bool submit = false;
@@ -718,8 +719,13 @@  static void guc_dequeue(struct intel_engine_cs *engine)
 					goto done;
 				}
 
-				if (submit)
+				if (submit) {
 					port_assign(port, last);
+					if (last->ctx != last_ctx) {
+						intel_engine_context_in(last->engine);
+						last_ctx = last->ctx;
+					}
+				}
 				port++;
 			}
 
@@ -741,6 +747,8 @@  static void guc_dequeue(struct intel_engine_cs *engine)
 	execlists->first = rb;
 	if (submit) {
 		port_assign(port, last);
+		if (last->ctx != last_ctx)
+			intel_engine_context_in(last->engine);
 		execlists_set_active(execlists, EXECLISTS_ACTIVE_USER);
 		guc_submit(engine);
 	}
@@ -763,6 +771,7 @@  static void guc_submission_tasklet(unsigned long data)
 
 	rq = port_request(&port[0]);
 	while (rq && i915_request_completed(rq)) {
+		intel_engine_context_out(rq->engine);
 		trace_i915_request_out(rq);
 		i915_request_put(rq);
 
@@ -1192,8 +1201,6 @@  int intel_guc_submission_enable(struct intel_guc *guc)
 		execlists->tasklet.func = guc_submission_tasklet;
 		engine->park = guc_submission_park;
 		engine->unpark = guc_submission_unpark;
-
-		engine->flags &= ~I915_ENGINE_SUPPORTS_STATS;
 	}
 
 	return 0;