Hi Daniel & Gordon & Thomas:
Just found that, my bad. Thanks for giving so much introduction for me, a beginner. :) Let me cook a patch for it.
BTW: CTXT_SR_CTRL register looks like a register needs mask. Does the bit 0 restore inhibit also needs _MASKED_BIT_ENABLE here? It looks like bit 0 is not set with _MASKED_BIT_ENABLE().
-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel Vetter
Sent: Wednesday, February 11, 2015 6:19 AM
To: Gordon, David S
Cc: Wang, Zhi A; Daniel, Thomas; Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: About CTX_CONTEXT_CONTROL initialization in populate_lr_context() intel_lrc.c
On Tue, Feb 10, 2015 at 04:25:20PM +0000, Dave Gordon wrote:
> On 09/02/15 15:05, Wang, Zhi A wrote:
> > Hi Gurus:
> > Forgive my junior HW knowledge, I just found that in execlist
> > context initialization function populate_lr_context(), this line:
> >
> > reg_state[CTX_CONTEXT_CONTROL+1] =
> > _MASKED_BIT_ENABLE((1<<3) |
> > MI_RESTORE_INHIBIT);
> >
> > It seems the "Inhibit Synchronous Context Switch" bit is not set
> > here, so when HW is trying to wait some events, e.g. semaphore,
> > according to Bspec, per my basic understanding, it will switch out
> > current context with some reason bit set. Here comes my question, I
> > think if this situation happen, should i915 remember this context
> > and try to re-schedule it in a proper time, e.g. before submitting a
> > new context until the context_completed bit in CSB is set? I don't
> > find a register that will remember the context switched out by
> > waiting event, so I think it should be i915 to handle this situation
> > or just set "Inhibit Synchronous Context Switch" bit here?...
>
> But that's exactly what it does already ... it sets bit 3, which is
> the "Inhibit Synchronous Context Switch" bit you refer to.
>
> What's wrong here is that it should use a #define for this bit, and
> for the "Restore Inhibit" bit -- for which it's actually using a value
> defined in a completely different context (no pun intended), namely
> the bits in a MI_SET_CONTEXT *instruction*. It just happens to work
> because they have the same numerical value. So, what we need is:
>
> <intel_lrc.h>
>
> #define RING_CONTEXT_CONTROL(ring) ((ring)->mmio_base+0x244)
> #define CTX_CTRL_INHIBIT_SYN_CTX_SWITCH (1 << 3)
> #define CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT (1 << 0)
>
> and then change the line you pointed out to use the correct symbolic names.
Can I have this in patch form please? I.e. convert to diff, paste the above into commit message and add sob line.
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
