Re: [PATCH v3] drm/i915/guc: don't ever forward VBlank to the GuC

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx