Re: [PATCH 13/49] drm/i915/bdw: Execlists ring tail writing

Mateo Lozano, Oscar
2014-03-27T17:15:49Z

I already got a review from Brad Volkin on this that I agree with: change the "write_tail" vfunc name to something different, like "submit". If no one disagrees, I´ll change it in the next submission.

> -----Original Message-----
> From: Mateo Lozano, Oscar
> Sent: Thursday, March 27, 2014 6:00 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Mateo Lozano, Oscar
> Subject: [PATCH 13/49] drm/i915/bdw: Execlists ring tail writing
>
> From: Oscar Mateo <oscar.mateo@intel.com>
>
> The write tail function is a very special place for execlists: since all access to
> the ring is mediated through requests (thanks to Chris Wilson's "Write
> RING_TAIL once per-request" for that) and all requests end up with a write tail,
> this is the place we are going to use to submit contexts for execution.
>
> For the moment, just mark the place (we still need to do a lot of preparation
> before execlists are ready to start submitting things).
>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 35e022f..a18dcf7 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -413,6 +413,12 @@ static void ring_write_tail(struct intel_engine *ring,
> I915_WRITE_TAIL(ring, value);
> }
>
> +static void gen8_write_tail_lrc(struct intel_engine *ring,
> + u32 value)
> +{
> + DRM_ERROR("Execlists still not ready!\n"); }
> +
> u32 intel_ring_get_active_head(struct intel_engine *ring) {
> drm_i915_private_t *dev_priv = ring->dev->dev_private; @@ -1907,12
> +1913,15 @@ int intel_init_render_ring(struct drm_device *dev)
> drm_i915_private_t *dev_priv = dev->dev_private;
> struct intel_engine *ring = &dev_priv->ring[RCS];
>
> + ring->write_tail = ring_write_tail;
> if (INTEL_INFO(dev)->gen >= 6) {
> ring->add_request = gen6_add_request;
> ring->flush = gen7_render_ring_flush;
> if (INTEL_INFO(dev)->gen == 6)
> ring->flush = gen6_render_ring_flush;
> if (INTEL_INFO(dev)->gen >= 8) {
> + if (dev_priv->lrc_enabled)
> + ring->write_tail = gen8_write_tail_lrc;
> ring->flush = gen8_render_ring_flush;
> ring->irq_get = gen8_ring_get_irq;
> ring->irq_put = gen8_ring_put_irq;
> @@ -1958,7 +1967,7 @@ int intel_init_render_ring(struct drm_device *dev)
> }
> ring->irq_enable_mask = I915_USER_INTERRUPT;
> }
> - ring->write_tail = ring_write_tail;
> +
> if (IS_HASWELL(dev))
> ring->dispatch_execbuffer = hsw_ring_dispatch_execbuffer;
> else if (IS_GEN8(dev))
> @@ -2079,6 +2088,8 @@ int intel_init_bsd_ring(struct drm_device *dev)
> ring->get_seqno = gen6_ring_get_seqno;
> ring->set_seqno = ring_set_seqno;
> if (INTEL_INFO(dev)->gen >= 8) {
> + if (dev_priv->lrc_enabled)
> + ring->write_tail = gen8_write_tail_lrc;
> ring->irq_enable_mask =
> GT_RENDER_USER_INTERRUPT <<
> GEN8_VCS1_IRQ_SHIFT;
> ring->irq_get = gen8_ring_get_irq;
> @@ -2133,6 +2144,8 @@ int intel_init_blt_ring(struct drm_device *dev)
> ring->get_seqno = gen6_ring_get_seqno;
> ring->set_seqno = ring_set_seqno;
> if (INTEL_INFO(dev)->gen >= 8) {
> + if (dev_priv->lrc_enabled)
> + ring->write_tail = gen8_write_tail_lrc;
> ring->irq_enable_mask =
> GT_RENDER_USER_INTERRUPT <<
> GEN8_BCS_IRQ_SHIFT;
> ring->irq_get = gen8_ring_get_irq;
> @@ -2170,6 +2183,8 @@ int intel_init_vebox_ring(struct drm_device *dev)
> ring->set_seqno = ring_set_seqno;
>
> if (INTEL_INFO(dev)->gen >= 8) {
> + if (dev_priv->lrc_enabled)
> + ring->write_tail = gen8_write_tail_lrc;
> ring->irq_enable_mask =
> GT_RENDER_USER_INTERRUPT <<
> GEN8_VECS_IRQ_SHIFT;
> ring->irq_get = gen8_ring_get_irq;
> --
> 1.9.0