Re: [PATCH 08/43] drm/i915/bdw: Add a context and an engine pointers to the ringbuffer

Daniel Vetter
2014-08-11T14:20:20Z

On Mon, Aug 11, 2014 at 04:14:13PM +0200, Daniel Vetter wrote:
> On Thu, Jul 24, 2014 at 05:04:16PM +0100, Thomas Daniel wrote:
> > From: Oscar Mateo <oscar.mateo@intel.com>
> >
> > Any given ringbuffer is unequivocally tied to one context and one engine.
> > By setting the appropriate pointers to them, the ringbuffer struct holds
> > all the infromation you might need to submit a workload for processing,
> > Execlists style.
> >
> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_lrc.c | 2 ++
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
> > drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +++
> > 3 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index 0a12b8c..2eb7db6 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -132,6 +132,8 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
> > return ret;
> > }
> >
> > + ringbuf->ring = ring;
> > + ringbuf->ctx = ctx;
> > ringbuf->size = 32 * PAGE_SIZE;
> > ringbuf->effective_size = ringbuf->size;
> > ringbuf->head = 0;
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 01e9840..279dda4 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -1570,6 +1570,8 @@ static int intel_init_ring_buffer(struct drm_device *dev,
> > INIT_LIST_HEAD(&ring->active_list);
> > INIT_LIST_HEAD(&ring->request_list);
> > ringbuf->size = 32 * PAGE_SIZE;
> > + ringbuf->ring = ring;
> > + ringbuf->ctx = ring->default_context;
>
> That doesn't make a terribly lot of sense tbh. I fear it's one of these
> slight confusions which will take tons of patches to clean up. Why exactly
> do we need the ring->ctx pointer?
>
> If we only need this for lrc I want to name it accordingly, to make sure
> legacy code doesn't grow stupid ideas. And also we should only initialize
> this in the lrc ctx init then.
>
> All patches up to this one merged.

Ok, I've discussed this quickly with Damien on irc. We decided to cut away
the ring->ctx part of this patch for now to be able to move on.
-Daniel

> -Daniel
>
> > memset(ring->semaphore.sync_seqno, 0, sizeof(ring->semaphore.sync_seqno));
> >
> > init_waitqueue_head(&ring->irq_queue);
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > index 053d004..be40788 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > @@ -88,6 +88,9 @@ struct intel_ringbuffer {
> > struct drm_i915_gem_object *obj;
> > void __iomem *virtual_start;
> >
> > + struct intel_engine_cs *ring;
> > + struct intel_context *ctx;
> > +
> > u32 head;
> > u32 tail;
> > int space;
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch