| Message ID | 20170121172905.1646-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On la, 2017-01-21 at 17:29 +0000, Chris Wilson wrote: > When execlists signals the context completion, it also provides the > context id for the status event. Assert that id matches the one we expect. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> <SNIP> > @@ -595,6 +595,10 @@ static void intel_lrc_irq_handler(unsigned long data) > if (!(status & GEN8_CTX_STATUS_COMPLETED_MASK)) > continue; > > + /* Check the context id for this event matches */ > + GEM_BUG_ON(readl(buf + 2 * idx + 1) != > + port[0].request->ctx->hw_id); > + > GEM_BUG_ON(port[0].count == 0); > if (--port[0].count == 0) { > GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED); This is good for now, Mika has to update it for the SVM code, though. (Counterpart at intel_lr_context_descriptor_update). Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
On Mon, Jan 23, 2017 at 10:26:00AM +0200, Joonas Lahtinen wrote: > On la, 2017-01-21 at 17:29 +0000, Chris Wilson wrote: > > When execlists signals the context completion, it also provides the > > context id for the status event. Assert that id matches the one we expect. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > <SNIP> > > > @@ -595,6 +595,10 @@ static void intel_lrc_irq_handler(unsigned long data) > > if (!(status & GEN8_CTX_STATUS_COMPLETED_MASK)) > > continue; > > > > + /* Check the context id for this event matches */ > > + GEM_BUG_ON(readl(buf + 2 * idx + 1) != > > + port[0].request->ctx->hw_id); > > + > > GEM_BUG_ON(port[0].count == 0); > > if (--port[0].count == 0) { > > GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED); > > This is good for now, Mika has to update it for the SVM code, though. > > (Counterpart at intel_lr_context_descriptor_update). One suggestion you made was that hw_id might carry the pasid in its upper 12bits (matching lrc_desc construction). Or an alternative is that we use upper_32_bits(port[0]->request->ctx->engine[engine->id].lrc_desc) here. That's probably a better description if that is what the hw is doing - which I can't remember, I have vague memories of it being a direct copy of the high dword, but the description of it I read over the weekend just referred to it as context_id. So does the context status carry the upper dword of elsp or just the context id? Easy way to find out -- today everything say use group 1! -Chris
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 432ee495dec2..6aa8761c333a 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -595,6 +595,10 @@ static void intel_lrc_irq_handler(unsigned long data) if (!(status & GEN8_CTX_STATUS_COMPLETED_MASK)) continue; + /* Check the context id for this event matches */ + GEM_BUG_ON(readl(buf + 2 * idx + 1) != + port[0].request->ctx->hw_id); + GEM_BUG_ON(port[0].count == 0); if (--port[0].count == 0) { GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED);
When execlists signals the context completion, it also provides the context id for the status event. Assert that id matches the one we expect. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_lrc.c | 4 ++++ 1 file changed, 4 insertions(+)