Re: [PATCH 04/53] drm/i915: Extract ringbuffer destroy & make alloc outside accesible

["Volkin, Bradley D" <bradley.d.volkin@intel.com>]

On Fri, Jun 13, 2014 at 08:37:22AM -0700, oscar.mateo@intel.com wrote:
> From: Oscar Mateo <oscar.mateo@intel.com>
> 
> We are going to start creating a lot of extra ringbuffers soon, so
> these functions are handy.
> 
> No functional changes.
> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 26 ++++++++++++++++----------
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  4 ++++
>  2 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 279488a..915f3d5 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1378,15 +1378,25 @@ static int init_phys_status_page(struct intel_engine_cs *ring)
>  	return 0;
>  }
>  
> -static int allocate_ring_buffer(struct intel_engine_cs *ring)
> +void intel_destroy_ring_buffer(struct intel_ringbuffer *ringbuf)
> +{
> +	if (!ringbuf->obj)
> +		return;
> +
> +	iounmap(ringbuf->virtual_start);
> +	i915_gem_object_ggtt_unpin(ringbuf->obj);
> +	drm_gem_object_unreference(&ringbuf->obj->base);
> +	ringbuf->obj = NULL;
> +}
> +
> +int intel_allocate_ring_buffer(struct drm_device *dev,
> +			       struct intel_ringbuffer *ringbuf)

A bikeshed, but maybe intel_alloc_ringbuffer_obj() since we're only
allocating the backing object, and to mirror the earlier
i915_gem_alloc_context_obj() with similar purpose. Otherwise, looks
fine to me.

Brad

>  {
> -	struct drm_device *dev = ring->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_ringbuffer *ringbuf = ring->buffer;
>  	struct drm_i915_gem_object *obj;
>  	int ret;
>  
> -	if (intel_ring_initialized(ring))
> +	if (ringbuf->obj)
>  		return 0;
>  
>  	obj = NULL;
> @@ -1455,7 +1465,7 @@ static int intel_init_ring_buffer(struct drm_device *dev,
>  			goto error;
>  	}
>  
> -	ret = allocate_ring_buffer(ring);
> +	ret = intel_allocate_ring_buffer(dev, ringbuf);
>  	if (ret) {
>  		DRM_ERROR("Failed to allocate ringbuffer %s: %d\n", ring->name, ret);
>  		goto error;
> @@ -1496,11 +1506,7 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
>  	intel_stop_ring_buffer(ring);
>  	WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & MODE_IDLE) == 0);
>  
> -	iounmap(ringbuf->virtual_start);
> -
> -	i915_gem_object_ggtt_unpin(ringbuf->obj);
> -	drm_gem_object_unreference(&ringbuf->obj->base);
> -	ringbuf->obj = NULL;
> +	intel_destroy_ring_buffer(ringbuf);
>  	ring->preallocated_lazy_request = NULL;
>  	ring->outstanding_lazy_seqno = 0;
>  
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 910c83c..dee5b37 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -318,6 +318,10 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev);
>  u64 intel_ring_get_active_head(struct intel_engine_cs *ring);
>  void intel_ring_setup_status_page(struct intel_engine_cs *ring);
>  
> +void intel_destroy_ring_buffer(struct intel_ringbuffer *ringbuf);
> +int intel_allocate_ring_buffer(struct drm_device *dev,
> +			       struct intel_ringbuffer *ringbuf);
> +
>  static inline u32 intel_ring_get_tail(struct intel_engine_cs *ring)
>  {
>  	return ring->buffer->tail;
> -- 
> 1.9.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx