| Message ID | 20210105231947.31235-3-daniele.ceraolospurio@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Split GuC submission from execlists submission | expand |
Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:44) > GuC owns the execlists state and the context IDs used for submission, so > the status of the ports and the CSB entries are not something we control > or can decode from the i915 side, therefore we can avoid dumping it. A > follow-up patch will also stop setting the csb pointers when using GuC > submission. > > GuC dumps all the required events in the GuC logs when verbosity is set > high enough. Would not be worth including, or is it not very helpful for debugging curious engine stalls? > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: John Harrison <John.C.Harrison@Intel.com> > Cc: Matthew Brost <matthew.brost@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On 1/5/2021 4:58 PM, Chris Wilson wrote: > Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:44) >> GuC owns the execlists state and the context IDs used for submission, so >> the status of the ports and the CSB entries are not something we control >> or can decode from the i915 side, therefore we can avoid dumping it. A >> follow-up patch will also stop setting the csb pointers when using GuC >> submission. >> >> GuC dumps all the required events in the GuC logs when verbosity is set >> high enough. > Would not be worth including, or is it not very helpful for debugging > curious engine stalls? GuC is going to reset the engine if it stalls, so we should get the GuC logs and the engine state included in the error state. Daniele > >> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Cc: John Harrison <John.C.Harrison@Intel.com> >> Cc: Matthew Brost <matthew.brost@intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > -Chris
Quoting Daniele Ceraolo Spurio (2021-01-06 02:32:28) > > > On 1/5/2021 4:58 PM, Chris Wilson wrote: > > Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:44) > >> GuC owns the execlists state and the context IDs used for submission, so > >> the status of the ports and the CSB entries are not something we control > >> or can decode from the i915 side, therefore we can avoid dumping it. A > >> follow-up patch will also stop setting the csb pointers when using GuC > >> submission. > >> > >> GuC dumps all the required events in the GuC logs when verbosity is set > >> high enough. > > Would not be worth including, or is it not very helpful for debugging > > curious engine stalls? > > GuC is going to reset the engine if it stalls, so we should get the GuC > logs and the engine state included in the error state. Here we would be focusing on "why hasn't a request been submitted/executed". A bad request is usually self-evident, but a missing one is tricky. -Chris
On 1/5/2021 6:55 PM, Chris Wilson wrote: > Quoting Daniele Ceraolo Spurio (2021-01-06 02:32:28) >> >> On 1/5/2021 4:58 PM, Chris Wilson wrote: >>> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:44) >>>> GuC owns the execlists state and the context IDs used for submission, so >>>> the status of the ports and the CSB entries are not something we control >>>> or can decode from the i915 side, therefore we can avoid dumping it. A >>>> follow-up patch will also stop setting the csb pointers when using GuC >>>> submission. >>>> >>>> GuC dumps all the required events in the GuC logs when verbosity is set >>>> high enough. >>> Would not be worth including, or is it not very helpful for debugging >>> curious engine stalls? >> GuC is going to reset the engine if it stalls, so we should get the GuC >> logs and the engine state included in the error state. > Here we would be focusing on "why hasn't a request been submitted/executed". > A bad request is usually self-evident, but a missing one is tricky. Agreed, but I still don't think we could use the CSB info even if we dumped it. We currently can't map CSB events in GuC submission mode to specific contexts, because the ctx IDs used by the GuC do not map to the ones used by i915. We've asked the GuC team to expose a way to do such a mapping, but that's still under discussion. In the meantime we plan to add a few traces to make sure the requests reach the GuC and use the GuC logs for what goes on inside the FW (GuC logs include the context IDs it uses for submission and all CSB events on high verbosity). Daniele > -Chris
Quoting Daniele Ceraolo Spurio (2021-01-06 17:21:16) > > > On 1/5/2021 6:55 PM, Chris Wilson wrote: > > Quoting Daniele Ceraolo Spurio (2021-01-06 02:32:28) > >> > >> On 1/5/2021 4:58 PM, Chris Wilson wrote: > >>> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:44) > >>>> GuC owns the execlists state and the context IDs used for submission, so > >>>> the status of the ports and the CSB entries are not something we control > >>>> or can decode from the i915 side, therefore we can avoid dumping it. A > >>>> follow-up patch will also stop setting the csb pointers when using GuC > >>>> submission. > >>>> > >>>> GuC dumps all the required events in the GuC logs when verbosity is set > >>>> high enough. > >>> Would not be worth including, or is it not very helpful for debugging > >>> curious engine stalls? > >> GuC is going to reset the engine if it stalls, so we should get the GuC > >> logs and the engine state included in the error state. > > Here we would be focusing on "why hasn't a request been submitted/executed". > > A bad request is usually self-evident, but a missing one is tricky. > > Agreed, but I still don't think we could use the CSB info even if we > dumped it. We currently can't map CSB events in GuC submission mode to > specific contexts, because the ctx IDs used by the GuC do not map to the > ones used by i915. We've asked the GuC team to expose a way to do such a > mapping, but that's still under discussion. In the meantime we plan to > add a few traces to make sure the requests reach the GuC and use the GuC > logs for what goes on inside the FW (GuC logs include the context IDs it > uses for submission and all CSB events on high verbosity). I was more reflecting on what could be here instead. Details of the ctb? It would be great to have a snapshot of some relevant guc state, merely wonder if we could extract anything from the log automatically. As well as the usual what have we submitted to the guc. -Chris
On 1/6/2021 11:43 AM, Chris Wilson wrote: > Quoting Daniele Ceraolo Spurio (2021-01-06 17:21:16) >> >> On 1/5/2021 6:55 PM, Chris Wilson wrote: >>> Quoting Daniele Ceraolo Spurio (2021-01-06 02:32:28) >>>> On 1/5/2021 4:58 PM, Chris Wilson wrote: >>>>> Quoting Daniele Ceraolo Spurio (2021-01-05 23:19:44) >>>>>> GuC owns the execlists state and the context IDs used for submission, so >>>>>> the status of the ports and the CSB entries are not something we control >>>>>> or can decode from the i915 side, therefore we can avoid dumping it. A >>>>>> follow-up patch will also stop setting the csb pointers when using GuC >>>>>> submission. >>>>>> >>>>>> GuC dumps all the required events in the GuC logs when verbosity is set >>>>>> high enough. >>>>> Would not be worth including, or is it not very helpful for debugging >>>>> curious engine stalls? >>>> GuC is going to reset the engine if it stalls, so we should get the GuC >>>> logs and the engine state included in the error state. >>> Here we would be focusing on "why hasn't a request been submitted/executed". >>> A bad request is usually self-evident, but a missing one is tricky. >> Agreed, but I still don't think we could use the CSB info even if we >> dumped it. We currently can't map CSB events in GuC submission mode to >> specific contexts, because the ctx IDs used by the GuC do not map to the >> ones used by i915. We've asked the GuC team to expose a way to do such a >> mapping, but that's still under discussion. In the meantime we plan to >> add a few traces to make sure the requests reach the GuC and use the GuC >> logs for what goes on inside the FW (GuC logs include the context IDs it >> uses for submission and all CSB events on high verbosity). > I was more reflecting on what could be here instead. Details of the ctb? > It would be great to have a snapshot of some relevant guc state, merely > wonder if we could extract anything from the log automatically. As well > as the usual what have we submitted to the guc. > -Chris Just realized I hadn't replied to this. We're still discussing with the GuC team about what type of GuC status we can extract and/or ask GuC to provide. Request list aside, most of the i915-side of the GuC info is going to be global (single ctb channel, single submission queue into GuC), so it'll likely end up in a different printer function. Daniele
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 1847d3c2ea99..f62303bf80b8 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -1470,7 +1470,9 @@ static void intel_engine_print_registers(struct intel_engine_cs *engine, drm_printf(m, "\tIPEHR: 0x%08x\n", ENGINE_READ(engine, IPEHR)); } - if (HAS_EXECLISTS(dev_priv)) { + if (intel_engine_in_guc_submission_mode(engine)) { + /* nothing to print yet */ + } else if (HAS_EXECLISTS(dev_priv)) { struct i915_request * const *port, *rq; const u32 *hws = &engine->status_page.addr[I915_HWS_CSB_BUF0_INDEX];
GuC owns the execlists state and the context IDs used for submission, so the status of the ports and the CSB entries are not something we control or can decode from the i915 side, therefore we can avoid dumping it. A follow-up patch will also stop setting the csb pointers when using GuC submission. GuC dumps all the required events in the GuC logs when verbosity is set high enough. Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Cc: Matthew Brost <matthew.brost@intel.com> --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)