| Message ID | 1399637360-4277-5-git-send-email-oscar.mateo@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 09, 2014 at 01:08:34PM +0100, oscar.mateo@intel.com wrote: > From: Ben Widawsky <benjamin.widawsky@intel.com> > > It's beneficial to be able to get a name, base, and id before we've > actually initialized the rings. This ability was effectively destroyed > in the ringbuffer fire which Daniel started. > > With the simple early init function, that ability is restored. > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > v2: The Full PPGTT series have moved things around a little bit. > Also, don't forget the VEBOX. > > v3: Checking ring->dev is not a good way to test if a ring is > initialized... > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> Needs to be updated for VEBOX2. Also I don't really see the point, where exactly do we need this? Ripping apart the ring init like this doesn't look too great imo. -Daniel > --- > drivers/gpu/drm/i915/i915_gem.c | 2 ++ > drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- > drivers/gpu/drm/i915/intel_ringbuffer.c | 60 ++++++++++++++++++--------------- > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + > 4 files changed, 37 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index ce941cf..6ef53bd 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4502,6 +4502,8 @@ int i915_gem_init(struct drm_device *dev) > > i915_gem_init_global_gtt(dev); > > + intel_init_rings_early(dev); > + > ret = i915_gem_context_init(dev); > if (ret) { > mutex_unlock(&dev->struct_mutex); > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > index 2d81985..8f37238 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -886,7 +886,7 @@ static void i915_gem_record_rings(struct drm_device *dev, > for (i = 0; i < I915_NUM_RINGS; i++) { > struct intel_ring_buffer *ring = &dev_priv->ring[i]; > > - if (ring->dev == NULL) > + if (!intel_ring_initialized(ring)) > continue; > > error->ring[i].valid = true; > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index a112971..fc737c8 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1417,7 +1417,6 @@ static int intel_init_ring_buffer(struct drm_device *dev, > { > int ret; > > - ring->dev = dev; > INIT_LIST_HEAD(&ring->active_list); > INIT_LIST_HEAD(&ring->request_list); > ring->size = 32 * PAGE_SIZE; > @@ -1908,10 +1907,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_ring_buffer *ring = &dev_priv->ring[RCS]; > > - ring->name = "render ring"; > - ring->id = RCS; > - ring->mmio_base = RENDER_RING_BASE; > - > if (INTEL_INFO(dev)->gen >= 6) { > ring->add_request = gen6_add_request; > ring->flush = gen7_render_ring_flush; > @@ -2019,10 +2014,6 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size) > struct intel_ring_buffer *ring = &dev_priv->ring[RCS]; > int ret; > > - ring->name = "render ring"; > - ring->id = RCS; > - ring->mmio_base = RENDER_RING_BASE; > - > if (INTEL_INFO(dev)->gen >= 6) { > /* non-kms not supported on gen6+ */ > return -ENODEV; > @@ -2056,7 +2047,6 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size) > ring->init = init_render_ring; > ring->cleanup = render_ring_cleanup; > > - ring->dev = dev; > INIT_LIST_HEAD(&ring->active_list); > INIT_LIST_HEAD(&ring->request_list); > > @@ -2086,12 +2076,8 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_ring_buffer *ring = &dev_priv->ring[VCS]; > > - ring->name = "bsd ring"; > - ring->id = VCS; > - > ring->write_tail = ring_write_tail; > if (INTEL_INFO(dev)->gen >= 6) { > - ring->mmio_base = GEN6_BSD_RING_BASE; > /* gen6 bsd needs a special wa for tail updates */ > if (IS_GEN6(dev)) > ring->write_tail = gen6_bsd_ring_write_tail; > @@ -2132,7 +2118,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) > ring->semaphore.mbox.signal[VECS] = GEN6_VEVSYNC; > ring->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC; > } else { > - ring->mmio_base = BSD_RING_BASE; > ring->flush = bsd_ring_flush; > ring->add_request = i9xx_add_request; > ring->get_seqno = ring_get_seqno; > @@ -2167,11 +2152,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev) > return -EINVAL; > } > > - ring->name = "bds2_ring"; > - ring->id = VCS2; > - > ring->write_tail = ring_write_tail; > - ring->mmio_base = GEN8_BSD2_RING_BASE; > ring->flush = gen6_bsd_ring_flush; > ring->add_request = gen6_add_request; > ring->get_seqno = gen6_ring_get_seqno; > @@ -2210,10 +2191,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_ring_buffer *ring = &dev_priv->ring[BCS]; > > - ring->name = "blitter ring"; > - ring->id = BCS; > - > - ring->mmio_base = BLT_RING_BASE; > ring->write_tail = ring_write_tail; > ring->flush = gen6_ring_flush; > ring->add_request = gen6_add_request; > @@ -2259,10 +2236,6 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_ring_buffer *ring = &dev_priv->ring[VECS]; > > - ring->name = "video enhancement ring"; > - ring->id = VECS; > - > - ring->mmio_base = VEBOX_RING_BASE; > ring->write_tail = ring_write_tail; > ring->flush = gen6_ring_flush; > ring->add_request = gen6_add_request; > @@ -2351,3 +2324,36 @@ intel_stop_ring_buffer(struct intel_ring_buffer *ring) > > stop_ring(ring); > } > + > +void intel_init_rings_early(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + dev_priv->ring[RCS].name = "render ring"; > + dev_priv->ring[RCS].id = RCS; > + dev_priv->ring[RCS].mmio_base = RENDER_RING_BASE; > + dev_priv->ring[RCS].dev = dev; > + > + dev_priv->ring[BCS].name = "blitter ring"; > + dev_priv->ring[BCS].id = BCS; > + dev_priv->ring[BCS].mmio_base = BLT_RING_BASE; > + dev_priv->ring[BCS].dev = dev; > + > + dev_priv->ring[VCS].name = "bsd ring"; > + dev_priv->ring[VCS].id = VCS; > + if (INTEL_INFO(dev)->gen >= 6) > + dev_priv->ring[VCS].mmio_base = GEN6_BSD_RING_BASE; > + else > + dev_priv->ring[VCS].mmio_base = BSD_RING_BASE; > + dev_priv->ring[VCS].dev = dev; > + > + dev_priv->ring[VCS2].name = "bds2_ring"; > + dev_priv->ring[VCS2].id = VCS2; > + dev_priv->ring[VCS2].mmio_base = GEN8_BSD2_RING_BASE; > + dev_priv->ring[VCS2].dev = dev; > + > + dev_priv->ring[VECS].name = "video enhancement ring"; > + dev_priv->ring[VECS].id = VECS; > + dev_priv->ring[VECS].mmio_base = VEBOX_RING_BASE; > + dev_priv->ring[VECS].dev = dev; > +} > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 72c3c15..b1bf767 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -297,6 +297,7 @@ void intel_ring_init_seqno(struct intel_ring_buffer *ring, u32 seqno); > int intel_ring_flush_all_caches(struct intel_ring_buffer *ring); > int intel_ring_invalidate_all_caches(struct intel_ring_buffer *ring); > > +void intel_init_rings_early(struct drm_device *dev); > int intel_init_render_ring_buffer(struct drm_device *dev); > int intel_init_bsd_ring_buffer(struct drm_device *dev); > int intel_init_bsd2_ring_buffer(struct drm_device *dev); > -- > 1.9.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, May 13, 2014 at 03:26:51PM +0200, Daniel Vetter wrote: > On Fri, May 09, 2014 at 01:08:34PM +0100, oscar.mateo@intel.com wrote: > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > > index 2d81985..8f37238 100644 > > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > > @@ -886,7 +886,7 @@ static void i915_gem_record_rings(struct drm_device *dev, > > for (i = 0; i < I915_NUM_RINGS; i++) { > > struct intel_ring_buffer *ring = &dev_priv->ring[i]; > > > > - if (ring->dev == NULL) > > + if (!intel_ring_initialized(ring)) > > continue; Besides this was deliberately written not to use intel_ring_initialized(). -Chris
> -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter > Sent: Tuesday, May 13, 2014 2:27 PM > To: Mateo Lozano, Oscar > Cc: intel-gfx@lists.freedesktop.org; Ben Widawsky; Widawsky, Benjamin > Subject: Re: [Intel-gfx] [PATCH 04/50] drm/i915: Extract trivial parts of ring init > (early init) > > On Fri, May 09, 2014 at 01:08:34PM +0100, oscar.mateo@intel.com wrote: > > From: Ben Widawsky <benjamin.widawsky@intel.com> > > > > It's beneficial to be able to get a name, base, and id before we've > > actually initialized the rings. This ability was effectively destroyed > > in the ringbuffer fire which Daniel started. > > > > With the simple early init function, that ability is restored. > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > > > v2: The Full PPGTT series have moved things around a little bit. > > Also, don't forget the VEBOX. > > > > v3: Checking ring->dev is not a good way to test if a ring is > > initialized... > > > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > > Needs to be updated for VEBOX2. Also I don't really see the point, where > exactly do we need this? Ripping apart the ring init like this doesn't look too > great imo. VEBOX2?? Not one week ago I updated it for BSD2 :( Anyway, this patch is a legacy carry over from previous versions, I can perfectly make do without it. Thanks!
On Wed, May 14, 2014 at 11:53:46AM +0000, Mateo Lozano, Oscar wrote: > > -----Original Message----- > > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter > > Sent: Tuesday, May 13, 2014 2:27 PM > > To: Mateo Lozano, Oscar > > Cc: intel-gfx@lists.freedesktop.org; Ben Widawsky; Widawsky, Benjamin > > Subject: Re: [Intel-gfx] [PATCH 04/50] drm/i915: Extract trivial parts of ring init > > (early init) > > > > On Fri, May 09, 2014 at 01:08:34PM +0100, oscar.mateo@intel.com wrote: > > > From: Ben Widawsky <benjamin.widawsky@intel.com> > > > > > > It's beneficial to be able to get a name, base, and id before we've > > > actually initialized the rings. This ability was effectively destroyed > > > in the ringbuffer fire which Daniel started. > > > > > > With the simple early init function, that ability is restored. > > > > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > > > > > v2: The Full PPGTT series have moved things around a little bit. > > > Also, don't forget the VEBOX. > > > > > > v3: Checking ring->dev is not a good way to test if a ring is > > > initialized... > > > > > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > > > > Needs to be updated for VEBOX2. Also I don't really see the point, where > > exactly do we need this? Ripping apart the ring init like this doesn't look too > > great imo. > > VEBOX2?? Not one week ago I updated it for BSD2 :( > Anyway, this patch is a legacy carry over from previous versions, I can perfectly make do without it. Oops, I've meant VCS2, must have been asleep while reading it ;-) -Daniel
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ce941cf..6ef53bd 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4502,6 +4502,8 @@ int i915_gem_init(struct drm_device *dev) i915_gem_init_global_gtt(dev); + intel_init_rings_early(dev); + ret = i915_gem_context_init(dev); if (ret) { mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 2d81985..8f37238 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -886,7 +886,7 @@ static void i915_gem_record_rings(struct drm_device *dev, for (i = 0; i < I915_NUM_RINGS; i++) { struct intel_ring_buffer *ring = &dev_priv->ring[i]; - if (ring->dev == NULL) + if (!intel_ring_initialized(ring)) continue; error->ring[i].valid = true; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index a112971..fc737c8 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1417,7 +1417,6 @@ static int intel_init_ring_buffer(struct drm_device *dev, { int ret; - ring->dev = dev; INIT_LIST_HEAD(&ring->active_list); INIT_LIST_HEAD(&ring->request_list); ring->size = 32 * PAGE_SIZE; @@ -1908,10 +1907,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_ring_buffer *ring = &dev_priv->ring[RCS]; - ring->name = "render ring"; - ring->id = RCS; - ring->mmio_base = RENDER_RING_BASE; - if (INTEL_INFO(dev)->gen >= 6) { ring->add_request = gen6_add_request; ring->flush = gen7_render_ring_flush; @@ -2019,10 +2014,6 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size) struct intel_ring_buffer *ring = &dev_priv->ring[RCS]; int ret; - ring->name = "render ring"; - ring->id = RCS; - ring->mmio_base = RENDER_RING_BASE; - if (INTEL_INFO(dev)->gen >= 6) { /* non-kms not supported on gen6+ */ return -ENODEV; @@ -2056,7 +2047,6 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size) ring->init = init_render_ring; ring->cleanup = render_ring_cleanup; - ring->dev = dev; INIT_LIST_HEAD(&ring->active_list); INIT_LIST_HEAD(&ring->request_list); @@ -2086,12 +2076,8 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_ring_buffer *ring = &dev_priv->ring[VCS]; - ring->name = "bsd ring"; - ring->id = VCS; - ring->write_tail = ring_write_tail; if (INTEL_INFO(dev)->gen >= 6) { - ring->mmio_base = GEN6_BSD_RING_BASE; /* gen6 bsd needs a special wa for tail updates */ if (IS_GEN6(dev)) ring->write_tail = gen6_bsd_ring_write_tail; @@ -2132,7 +2118,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) ring->semaphore.mbox.signal[VECS] = GEN6_VEVSYNC; ring->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC; } else { - ring->mmio_base = BSD_RING_BASE; ring->flush = bsd_ring_flush; ring->add_request = i9xx_add_request; ring->get_seqno = ring_get_seqno; @@ -2167,11 +2152,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev) return -EINVAL; } - ring->name = "bds2_ring"; - ring->id = VCS2; - ring->write_tail = ring_write_tail; - ring->mmio_base = GEN8_BSD2_RING_BASE; ring->flush = gen6_bsd_ring_flush; ring->add_request = gen6_add_request; ring->get_seqno = gen6_ring_get_seqno; @@ -2210,10 +2191,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_ring_buffer *ring = &dev_priv->ring[BCS]; - ring->name = "blitter ring"; - ring->id = BCS; - - ring->mmio_base = BLT_RING_BASE; ring->write_tail = ring_write_tail; ring->flush = gen6_ring_flush; ring->add_request = gen6_add_request; @@ -2259,10 +2236,6 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_ring_buffer *ring = &dev_priv->ring[VECS]; - ring->name = "video enhancement ring"; - ring->id = VECS; - - ring->mmio_base = VEBOX_RING_BASE; ring->write_tail = ring_write_tail; ring->flush = gen6_ring_flush; ring->add_request = gen6_add_request; @@ -2351,3 +2324,36 @@ intel_stop_ring_buffer(struct intel_ring_buffer *ring) stop_ring(ring); } + +void intel_init_rings_early(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + dev_priv->ring[RCS].name = "render ring"; + dev_priv->ring[RCS].id = RCS; + dev_priv->ring[RCS].mmio_base = RENDER_RING_BASE; + dev_priv->ring[RCS].dev = dev; + + dev_priv->ring[BCS].name = "blitter ring"; + dev_priv->ring[BCS].id = BCS; + dev_priv->ring[BCS].mmio_base = BLT_RING_BASE; + dev_priv->ring[BCS].dev = dev; + + dev_priv->ring[VCS].name = "bsd ring"; + dev_priv->ring[VCS].id = VCS; + if (INTEL_INFO(dev)->gen >= 6) + dev_priv->ring[VCS].mmio_base = GEN6_BSD_RING_BASE; + else + dev_priv->ring[VCS].mmio_base = BSD_RING_BASE; + dev_priv->ring[VCS].dev = dev; + + dev_priv->ring[VCS2].name = "bds2_ring"; + dev_priv->ring[VCS2].id = VCS2; + dev_priv->ring[VCS2].mmio_base = GEN8_BSD2_RING_BASE; + dev_priv->ring[VCS2].dev = dev; + + dev_priv->ring[VECS].name = "video enhancement ring"; + dev_priv->ring[VECS].id = VECS; + dev_priv->ring[VECS].mmio_base = VEBOX_RING_BASE; + dev_priv->ring[VECS].dev = dev; +} diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 72c3c15..b1bf767 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -297,6 +297,7 @@ void intel_ring_init_seqno(struct intel_ring_buffer *ring, u32 seqno); int intel_ring_flush_all_caches(struct intel_ring_buffer *ring); int intel_ring_invalidate_all_caches(struct intel_ring_buffer *ring); +void intel_init_rings_early(struct drm_device *dev); int intel_init_render_ring_buffer(struct drm_device *dev); int intel_init_bsd_ring_buffer(struct drm_device *dev); int intel_init_bsd2_ring_buffer(struct drm_device *dev);