| Message ID | 1466780277-23435-1-git-send-email-david.s.gordon@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 24/06/16 15:57, Dave Gordon wrote: > If a context waiting for VBlank were switched out, switching > in the next context and generating a CSB event in the process, > then the GuC would have to put the context back in the queue, > and then observe the subsequent VBlank interrupt so that it > could resubmit the suspended context. > > However, we always set the CTX_CTRL_INHIBIT_SYN_CTX_SWITCH bit > in the RING_CONTEXT_CONTROL register, so this case cannot occur. > Furthermore we don't use the GuC's internal scheduler or allow > it to auto-resubmit workloads. Consequently, the GuC doesn't > need to see VBlanks, and by sending them to it we may be waking > it up unnecessarily, which might reduce RC6 residency and > increase power consumption. > > So this patch removes the setting of the GFC_FORWARD_VBLANK > field from the code that diverts interrupts towards the GuC. > (The code to direct interrupts to the host, OTOH, continues to > explicitly set the field to "never send VBlanks to the GuC".) > > v3: > Remove the line of code completely (original set the field > to ALWAYS forward, v1 changed it to CONDITIONAL forwarding, > v2 explicitly set it to NEVER, v3 just doesn't touch it at > all, as we know it's already set to NEVER). > > Signed-off-by: Dave Gordon <david.s.gordon@intel.com> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (previous version) > --- > drivers/gpu/drm/i915/intel_guc_loader.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c > index 8fe96a2..4f6311a 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -87,7 +87,7 @@ static void direct_interrupts_to_host(struct drm_i915_private *dev_priv) > struct intel_engine_cs *engine; > int irqs; > > - /* tell all command streamers NOT to forward interrupts and vblank to GuC */ > + /* tell all command streamers NOT to forward interrupts or vblank to GuC */ > irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_NEVER); > irqs |= _MASKED_BIT_DISABLE(GFX_INTERRUPT_STEERING); > for_each_engine(engine, dev_priv) > @@ -105,9 +105,8 @@ static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv) > int irqs; > u32 tmp; > > - /* tell all command streamers to forward interrupts and vblank to GuC */ > - irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_ALWAYS); > - irqs |= _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING); > + /* tell all command streamers to forward interrupts (but not vblank) to GuC */ > + irqs = _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING); > for_each_engine(engine, dev_priv) > I915_WRITE(RING_MODE_GEN7(engine), irqs); > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
On 24/06/16 16:25, Patchwork wrote: > == Series Details == > > Series: drm/i915/guc: don't ever forward VBlank to the GuC > URL : https://patchwork.freedesktop.org/series/9145/ > State : success > > == Summary == > > Series 9145v1 drm/i915/guc: don't ever forward VBlank to the GuC > http://patchwork.freedesktop.org/api/1.0/series/9145/revisions/1/mbox > > Test gem_exec_flush: > Subgroup basic-batch-kernel-default-cmd: > fail -> PASS (ro-byt-n2820) > Test kms_pipe_crc_basic: > Subgroup suspend-read-crc-pipe-b: > dmesg-warn -> SKIP (ro-bdw-i5-5250u) > Subgroup suspend-read-crc-pipe-c: > dmesg-warn -> SKIP (ro-bdw-i5-5250u) > > fi-hsw-i7-4770k total:228 pass:194 dwarn:0 dfail:0 fail:1 skip:33 > fi-skl-i5-6260u total:228 pass:202 dwarn:0 dfail:0 fail:1 skip:25 > fi-snb-i7-2600 total:228 pass:174 dwarn:0 dfail:0 fail:1 skip:53 > ro-bdw-i5-5250u total:227 pass:201 dwarn:1 dfail:1 fail:1 skip:23 > ro-bdw-i7-5600u total:227 pass:189 dwarn:0 dfail:1 fail:0 skip:37 > ro-bsw-n3050 total:227 pass:175 dwarn:1 dfail:1 fail:2 skip:48 > ro-byt-n2820 total:227 pass:178 dwarn:0 dfail:1 fail:3 skip:45 > ro-hsw-i3-4010u total:227 pass:194 dwarn:0 dfail:1 fail:1 skip:31 > ro-hsw-i7-4770r total:227 pass:194 dwarn:0 dfail:1 fail:1 skip:31 > ro-ilk-i7-620lm total:227 pass:154 dwarn:0 dfail:1 fail:2 skip:70 > ro-ilk1-i5-650 total:222 pass:154 dwarn:0 dfail:1 fail:2 skip:65 > ro-ivb-i7-3770 total:227 pass:185 dwarn:0 dfail:1 fail:1 skip:40 > ro-ivb2-i7-3770 total:227 pass:189 dwarn:0 dfail:1 fail:1 skip:36 > ro-skl3-i5-6260u total:227 pass:205 dwarn:1 dfail:1 fail:1 skip:19 > ro-snb-i7-2620M total:227 pass:178 dwarn:0 dfail:1 fail:1 skip:47 > fi-kbl-qkkr failed to connect after reboot > fi-skl-i7-6700k failed to connect after reboot > ro-bdw-i7-5557U failed to connect after reboot > > Results at /archive/results/CI_IGT_test/RO_Patchwork_1301/ > > 66e29f4 drm-intel-nightly: 2016y-06m-24d-13h-47m-00s UTC integration manifest > 403064e drm/i915/guc: don't ever forward VBlank to the GuC Merged to dinq, thanks for the patch. Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 8fe96a2..4f6311a 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -87,7 +87,7 @@ static void direct_interrupts_to_host(struct drm_i915_private *dev_priv) struct intel_engine_cs *engine; int irqs; - /* tell all command streamers NOT to forward interrupts and vblank to GuC */ + /* tell all command streamers NOT to forward interrupts or vblank to GuC */ irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_NEVER); irqs |= _MASKED_BIT_DISABLE(GFX_INTERRUPT_STEERING); for_each_engine(engine, dev_priv) @@ -105,9 +105,8 @@ static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv) int irqs; u32 tmp; - /* tell all command streamers to forward interrupts and vblank to GuC */ - irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_ALWAYS); - irqs |= _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING); + /* tell all command streamers to forward interrupts (but not vblank) to GuC */ + irqs = _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING); for_each_engine(engine, dev_priv) I915_WRITE(RING_MODE_GEN7(engine), irqs);
If a context waiting for VBlank were switched out, switching in the next context and generating a CSB event in the process, then the GuC would have to put the context back in the queue, and then observe the subsequent VBlank interrupt so that it could resubmit the suspended context. However, we always set the CTX_CTRL_INHIBIT_SYN_CTX_SWITCH bit in the RING_CONTEXT_CONTROL register, so this case cannot occur. Furthermore we don't use the GuC's internal scheduler or allow it to auto-resubmit workloads. Consequently, the GuC doesn't need to see VBlanks, and by sending them to it we may be waking it up unnecessarily, which might reduce RC6 residency and increase power consumption. So this patch removes the setting of the GFC_FORWARD_VBLANK field from the code that diverts interrupts towards the GuC. (The code to direct interrupts to the host, OTOH, continues to explicitly set the field to "never send VBlanks to the GuC".) v3: Remove the line of code completely (original set the field to ALWAYS forward, v1 changed it to CONDITIONAL forwarding, v2 explicitly set it to NEVER, v3 just doesn't touch it at all, as we know it's already set to NEVER). Signed-off-by: Dave Gordon <david.s.gordon@intel.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (previous version) --- drivers/gpu/drm/i915/intel_guc_loader.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)