Re: [PATCH] drm/i915: Initialize ring->obj last and use it as a marker

[Daniel Vetter <daniel@ffwll.ch>]

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