| Message ID | 20180321172625.6415-7-jeff.mcgee@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 21, 2018 at 10:26:23AM -0700, jeff.mcgee@intel.com wrote: > From: Jeff McGee <jeff.mcgee@intel.com> > > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com> > --- > drivers/gpu/drm/i915/intel_lrc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index beb81f13a3cc..cec4e1653daf 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1009,7 +1009,7 @@ static void execlists_submission_tasklet(unsigned long data) > * imposing the cost of a locked atomic transaction when submitting a > * new request (outside of the context-switch interrupt). > */ > - if (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) > + while (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) Assuming that this accidentally went missing in the refactor. Chris? > process_csb(engine); > > if (!execlists_is_active(&engine->execlists, EXECLISTS_ACTIVE_PREEMPT)) > -- > 2.16.2 >
Quoting Jeff McGee (2018-03-21 17:33:04) > On Wed, Mar 21, 2018 at 10:26:23AM -0700, jeff.mcgee@intel.com wrote: > > From: Jeff McGee <jeff.mcgee@intel.com> > > > > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com> > > --- > > drivers/gpu/drm/i915/intel_lrc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > > index beb81f13a3cc..cec4e1653daf 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -1009,7 +1009,7 @@ static void execlists_submission_tasklet(unsigned long data) > > * imposing the cost of a locked atomic transaction when submitting a > > * new request (outside of the context-switch interrupt). > > */ > > - if (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) > > + while (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) > Assuming that this accidentally went missing in the refactor. Chris? No. process_csb became a do{} while. The caller did a test_bit to avoid the function call for normal rescheduling paths. -Chris
On Wed, Mar 21, 2018 at 06:06:44PM +0000, Chris Wilson wrote: > Quoting Jeff McGee (2018-03-21 17:33:04) > > On Wed, Mar 21, 2018 at 10:26:23AM -0700, jeff.mcgee@intel.com wrote: > > > From: Jeff McGee <jeff.mcgee@intel.com> > > > > > > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_lrc.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > > > index beb81f13a3cc..cec4e1653daf 100644 > > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > > @@ -1009,7 +1009,7 @@ static void execlists_submission_tasklet(unsigned long data) > > > * imposing the cost of a locked atomic transaction when submitting a > > > * new request (outside of the context-switch interrupt). > > > */ > > > - if (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) > > > + while (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) > > Assuming that this accidentally went missing in the refactor. Chris? > > No. process_csb became a do{} while. The caller did a test_bit to avoid > the function call for normal rescheduling paths. > -Chris But there is no loop in process_csb(). -Jeff
Quoting Jeff McGee (2018-03-21 18:29:46) > On Wed, Mar 21, 2018 at 06:06:44PM +0000, Chris Wilson wrote: > > Quoting Jeff McGee (2018-03-21 17:33:04) > > > On Wed, Mar 21, 2018 at 10:26:23AM -0700, jeff.mcgee@intel.com wrote: > > > > From: Jeff McGee <jeff.mcgee@intel.com> > > > > > > > > Signed-off-by: Jeff McGee <jeff.mcgee@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/intel_lrc.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > > > > index beb81f13a3cc..cec4e1653daf 100644 > > > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > > > @@ -1009,7 +1009,7 @@ static void execlists_submission_tasklet(unsigned long data) > > > > * imposing the cost of a locked atomic transaction when submitting a > > > > * new request (outside of the context-switch interrupt). > > > > */ > > > > - if (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) > > > > + while (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) > > > Assuming that this accidentally went missing in the refactor. Chris? > > > > No. process_csb became a do{} while. The caller did a test_bit to avoid > > the function call for normal rescheduling paths. > > -Chris > > But there is no loop in process_csb(). There is in my copy. I was trying different things and removing the loop masked a different issue with tasklet scheduling. Strictly we don't need a loop here as we will always be re-run following an interrupt. But since we may need to grab forcewake and whatnot, it seems prudent to keep the loop around. -Chris >
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index beb81f13a3cc..cec4e1653daf 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1009,7 +1009,7 @@ static void execlists_submission_tasklet(unsigned long data) * imposing the cost of a locked atomic transaction when submitting a * new request (outside of the context-switch interrupt). */ - if (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) + while (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) process_csb(engine); if (!execlists_is_active(&engine->execlists, EXECLISTS_ACTIVE_PREEMPT))