All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Initialize ring->obj last and use it as a marker
@ 2012-04-14 11:39 Chris Wilson
  2012-04-14 22:15 ` Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2012-04-14 11:39 UTC (permalink / raw)
  To: intel-gfx

Use the assignment of ring->obj as a marker that the ring is active, and
so be careful not to initialise that value too early in case we need to
perform some workarounds that would ordinarily require touching the ring
whilst prepping the object.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c         |   12 +++++++-----
 drivers/gpu/drm/i915/intel_ringbuffer.c |   20 ++++++++++++--------
 drivers/gpu/drm/i915/intel_ringbuffer.h |    3 ++-
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ccfdc81..6019caa 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -838,15 +838,17 @@ int i915_reset(struct drm_device *dev, u8 flags)
 	 */
 	if (drm_core_check_feature(dev, DRIVER_MODESET) ||
 			!dev_priv->mm.suspended) {
+		int i;
+
 		dev_priv->mm.suspended = 0;
 
 		i915_gem_init_swizzling(dev);
 
-		dev_priv->ring[RCS].init(&dev_priv->ring[RCS]);
-		if (HAS_BSD(dev))
-		    dev_priv->ring[VCS].init(&dev_priv->ring[VCS]);
-		if (HAS_BLT(dev))
-		    dev_priv->ring[BCS].init(&dev_priv->ring[BCS]);
+		for (i = 0; i < I915_NUM_RINGS; i++) {
+			if (dev_priv->ring[i].obj)
+				dev_priv->ring[i].init(&dev_priv->ring[i],
+						       dev_priv->ring[i].obj);
+		}
 
 		i915_gem_init_ppgtt(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 6c14457..9bd659c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -247,10 +247,10 @@ u32 intel_ring_get_active_head(struct intel_ring_buffer *ring)
 	return I915_READ(acthd_reg);
 }
 
-static int init_ring_common(struct intel_ring_buffer *ring)
+static int init_ring_common(struct intel_ring_buffer *ring,
+			    struct drm_i915_gem_object *obj)
 {
 	drm_i915_private_t *dev_priv = ring->dev->dev_private;
-	struct drm_i915_gem_object *obj = ring->obj;
 	u32 head;
 
 	/* Stop the ring if it's running. */
@@ -377,11 +377,16 @@ cleanup_pipe_control(struct intel_ring_buffer *ring)
 	ring->private = NULL;
 }
 
-static int init_render_ring(struct intel_ring_buffer *ring)
+static int init_render_ring(struct intel_ring_buffer *ring,
+			    struct drm_i915_gem_object *obj)
 {
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int ret = init_ring_common(ring);
+	int ret;
+
+	ret = init_ring_common(ring, obj);
+	if (ret)
+		return ret;
 
 	if (INTEL_INFO(dev)->gen > 3) {
 		int mode = VS_TIMER_DISPATCH << 16 | VS_TIMER_DISPATCH;
@@ -919,8 +924,6 @@ int intel_init_ring_buffer(struct drm_device *dev,
 		goto err_hws;
 	}
 
-	ring->obj = obj;
-
 	ret = i915_gem_object_pin(obj, PAGE_SIZE, true);
 	if (ret)
 		goto err_unref;
@@ -939,7 +942,8 @@ int intel_init_ring_buffer(struct drm_device *dev,
 	}
 
 	ring->virtual_start = ring->map.handle;
-	ret = ring->init(ring);
+
+	ret = ring->init(ring, obj);
 	if (ret)
 		goto err_unmap;
 
@@ -951,6 +955,7 @@ int intel_init_ring_buffer(struct drm_device *dev,
 	if (IS_I830(ring->dev))
 		ring->effective_size -= 128;
 
+	ring->obj = obj;
 	return 0;
 
 err_unmap:
@@ -959,7 +964,6 @@ err_unpin:
 	i915_gem_object_unpin(obj);
 err_unref:
 	drm_gem_object_unreference(&obj->base);
-	ring->obj = NULL;
 err_hws:
 	cleanup_status_page(ring);
 	return ret;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 06a66ad..c3c464d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -66,7 +66,8 @@ struct  intel_ring_buffer {
 	bool __must_check (*irq_get)(struct intel_ring_buffer *ring);
 	void		(*irq_put)(struct intel_ring_buffer *ring);
 
-	int		(*init)(struct intel_ring_buffer *ring);
+	int		(*init)(struct intel_ring_buffer *ring,
+				struct drm_i915_gem_object *obj);
 
 	void		(*write_tail)(struct intel_ring_buffer *ring,
 				      u32 value);
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Initialize ring->obj last and use it as a marker
  2012-04-14 11:39 [PATCH] drm/i915: Initialize ring->obj last and use it as a marker Chris Wilson
@ 2012-04-14 22:15 ` Daniel Vetter
  2012-04-14 22:27   ` Chris Wilson
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Vetter @ 2012-04-14 22:15 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Sat, Apr 14, 2012 at 12:39:44PM +0100, Chris Wilson wrote:
> Use the assignment of ring->obj as a marker that the ring is active, and
> so be careful not to initialise that value too early in case we need to
> perform some workarounds that would ordinarily require touching the ring
> whilst prepping the object.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

I guess we don't need this with the actual workaround that needs this
patch (i.e. that try-to-make-i845 patch)? Or is other stuff in the works?
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.c         |   12 +++++++-----
>  drivers/gpu/drm/i915/intel_ringbuffer.c |   20 ++++++++++++--------
>  drivers/gpu/drm/i915/intel_ringbuffer.h |    3 ++-
>  3 files changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ccfdc81..6019caa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -838,15 +838,17 @@ int i915_reset(struct drm_device *dev, u8 flags)
>  	 */
>  	if (drm_core_check_feature(dev, DRIVER_MODESET) ||
>  			!dev_priv->mm.suspended) {
> +		int i;
> +
>  		dev_priv->mm.suspended = 0;
>  
>  		i915_gem_init_swizzling(dev);
>  
> -		dev_priv->ring[RCS].init(&dev_priv->ring[RCS]);
> -		if (HAS_BSD(dev))
> -		    dev_priv->ring[VCS].init(&dev_priv->ring[VCS]);
> -		if (HAS_BLT(dev))
> -		    dev_priv->ring[BCS].init(&dev_priv->ring[BCS]);
> +		for (i = 0; i < I915_NUM_RINGS; i++) {
> +			if (dev_priv->ring[i].obj)
> +				dev_priv->ring[i].init(&dev_priv->ring[i],
> +						       dev_priv->ring[i].obj);
> +		}
>  
>  		i915_gem_init_ppgtt(dev);
>  
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 6c14457..9bd659c 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -247,10 +247,10 @@ u32 intel_ring_get_active_head(struct intel_ring_buffer *ring)
>  	return I915_READ(acthd_reg);
>  }
>  
> -static int init_ring_common(struct intel_ring_buffer *ring)
> +static int init_ring_common(struct intel_ring_buffer *ring,
> +			    struct drm_i915_gem_object *obj)
>  {
>  	drm_i915_private_t *dev_priv = ring->dev->dev_private;
> -	struct drm_i915_gem_object *obj = ring->obj;
>  	u32 head;
>  
>  	/* Stop the ring if it's running. */
> @@ -377,11 +377,16 @@ cleanup_pipe_control(struct intel_ring_buffer *ring)
>  	ring->private = NULL;
>  }
>  
> -static int init_render_ring(struct intel_ring_buffer *ring)
> +static int init_render_ring(struct intel_ring_buffer *ring,
> +			    struct drm_i915_gem_object *obj)
>  {
>  	struct drm_device *dev = ring->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int ret = init_ring_common(ring);
> +	int ret;
> +
> +	ret = init_ring_common(ring, obj);
> +	if (ret)
> +		return ret;
>  
>  	if (INTEL_INFO(dev)->gen > 3) {
>  		int mode = VS_TIMER_DISPATCH << 16 | VS_TIMER_DISPATCH;
> @@ -919,8 +924,6 @@ int intel_init_ring_buffer(struct drm_device *dev,
>  		goto err_hws;
>  	}
>  
> -	ring->obj = obj;
> -
>  	ret = i915_gem_object_pin(obj, PAGE_SIZE, true);
>  	if (ret)
>  		goto err_unref;
> @@ -939,7 +942,8 @@ int intel_init_ring_buffer(struct drm_device *dev,
>  	}
>  
>  	ring->virtual_start = ring->map.handle;
> -	ret = ring->init(ring);
> +
> +	ret = ring->init(ring, obj);
>  	if (ret)
>  		goto err_unmap;
>  
> @@ -951,6 +955,7 @@ int intel_init_ring_buffer(struct drm_device *dev,
>  	if (IS_I830(ring->dev))
>  		ring->effective_size -= 128;
>  
> +	ring->obj = obj;
>  	return 0;
>  
>  err_unmap:
> @@ -959,7 +964,6 @@ err_unpin:
>  	i915_gem_object_unpin(obj);
>  err_unref:
>  	drm_gem_object_unreference(&obj->base);
> -	ring->obj = NULL;
>  err_hws:
>  	cleanup_status_page(ring);
>  	return ret;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 06a66ad..c3c464d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -66,7 +66,8 @@ struct  intel_ring_buffer {
>  	bool __must_check (*irq_get)(struct intel_ring_buffer *ring);
>  	void		(*irq_put)(struct intel_ring_buffer *ring);
>  
> -	int		(*init)(struct intel_ring_buffer *ring);
> +	int		(*init)(struct intel_ring_buffer *ring,
> +				struct drm_i915_gem_object *obj);
>  
>  	void		(*write_tail)(struct intel_ring_buffer *ring,
>  				      u32 value);
> -- 
> 1.7.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Initialize ring->obj last and use it as a marker
  2012-04-14 22:15 ` Daniel Vetter
@ 2012-04-14 22:27   ` Chris Wilson
  0 siblings, 0 replies; 3+ messages in thread
From: Chris Wilson @ 2012-04-14 22:27 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Sun, 15 Apr 2012 00:15:24 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Sat, Apr 14, 2012 at 12:39:44PM +0100, Chris Wilson wrote:
> > Use the assignment of ring->obj as a marker that the ring is active, and
> > so be careful not to initialise that value too early in case we need to
> > perform some workarounds that would ordinarily require touching the ring
> > whilst prepping the object.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> I guess we don't need this with the actual workaround that needs this
> patch (i.e. that try-to-make-i845 patch)? Or is other stuff in the works?

It's relevant for the quirk where we might need to idle the gpu whilst
manipulating the GATT, but I'm still chasing the magic to make it work
for 845g. I also thought it relevant in the light of your other
ringbuffer cleanups. There's probably a few other places where we
could make the code more architecture agnostic and just operate on what
has been setup.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-04-14 22:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-14 11:39 [PATCH] drm/i915: Initialize ring->obj last and use it as a marker Chris Wilson
2012-04-14 22:15 ` Daniel Vetter
2012-04-14 22:27   ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.