Re: [PATCH 12/15] drm/i915: Interrupt routing for GuC submission

Daniel Vetter
2015-06-24T09:59:36Z

On Tue, Jun 23, 2015 at 04:48:11PM -0700, Yu Dai wrote:
>
>
> On 06/23/2015 04:33 AM, Dave Gordon wrote:
> >On 17/06/15 13:41, Daniel Vetter wrote:
> >> On Wed, Jun 17, 2015 at 02:22:19PM +0200, Daniel Vetter wrote:
> >>> On Wed, Jun 17, 2015 at 09:20:44AM +0100, Dave Gordon wrote:
> >>>> On 16/06/15 10:24, Chris Wilson wrote:
> >>>>> On Mon, Jun 15, 2015 at 07:36:30PM +0100, Dave Gordon wrote:
> >>>>>> +static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv)
> >>>>>> +{
> >>>>>> + struct intel_engine_cs *ring;
> >>>>>> + int i, irqs;
> >>>>>> +
> >>>>>> + /* 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);
> >>>>>> + for_each_ring(ring, dev_priv, i)
> >>>>>> + I915_WRITE(RING_MODE_GEN7(ring), irqs);
> >>>>>> +
> >>>>>> + /* tell DE to send (all) flip_done to GuC */
> >>>>>> + irqs = DERRMR_PIPEA_PRI_FLIP_DONE | DERRMR_PIPEA_SPR_FLIP_DONE |
> >>>>>> + DERRMR_PIPEB_PRI_FLIP_DONE | DERRMR_PIPEB_SPR_FLIP_DONE |
> >>>>>> + DERRMR_PIPEC_PRI_FLIP_DONE | DERRMR_PIPEC_SPR_FLIP_DONE;
> >>>>>> + /* Unmasked bits will cause GuC response message to be sent */
> >>>>>> + I915_WRITE(DE_GUCRMR, ~irqs);
> >>>>>
> >>>>> That's scary since userspace depends on a few more DERRMR events
> >>>>> (wait-for-scanline). Where will they end up?
> >>>>> -Chris
> >>>>
> >>>> This doesn't change any bits in DE_RRMR, or set the VBLANK or SCANLINE
> >>>> bits in the DE_GUCRMR, so those events should be unaffected. The GuC
> >>>> isn't interested in those, only in flip done.
> >>>
> >>> Why does the guc care about flip_done? With atomic it'll get exactly none
> >>> of those, ever ...
> >>
> >> Well I forgot that mmio writes also generate interrupts. Still strange
> >> that GuC is interested in this. Would be really interesting to know what
> >> GuC is up to here.
> >
> >Maybe Alex knows ... otherwise we can ask the GuC f/w team ...
> >
> The SLPC (Single Loop Power Control) within GuC needs these. However, to
> enable it or not is yet determined because architecture review is not done.

Well if guc needs to know about display activity, the kernel can tell it
directly. And it could even tell the guc when we've missed a frame, which
is something the guc has absolutely no idea about since with atomic that's
all implemented on the kernel side and never goes through the rings.

Sounds like a "not engineerd for linux kernel" feature :(
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation