Re: [PATCH 1/2] drm/i915: extract ring sync code

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed, 21 Mar 2012 17:19:12 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> We want to use this function elsewhere...
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.h            |   19 +++++++++
>  drivers/gpu/drm/i915/i915_gem.c            |   43 ++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   60 +---------------------------
>  3 files changed, 63 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b6098b0..ee691ce 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -38,6 +38,7 @@
>  #include <linux/i2c-algo-bit.h>
>  #include <drm/intel-gtt.h>
>  #include <linux/backlight.h>
> +#include <linux/intel-iommu.h>
>  
>  /* General customization:
>   */
> @@ -1188,6 +1189,8 @@ void i915_gem_lastclose(struct drm_device *dev);
>  
>  int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
>  int __must_check i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj);
> +int i915_gem_object_sync(struct drm_i915_gem_object *obj,
> +			 struct intel_ring_buffer *to);
>  void i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
>  				    struct intel_ring_buffer *ring,
>  				    u32 seqno);
> @@ -1282,6 +1285,22 @@ i915_gem_get_unfenced_gtt_alignment(struct drm_device *dev,
>  int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
>  				    enum i915_cache_level cache_level);
>  
> +static inline bool
> +intel_enable_semaphores(struct drm_device *dev)
> +{
> +	if (INTEL_INFO(dev)->gen < 6)
> +		return 0;
> +
> +	if (i915_semaphores >= 0)
> +		return i915_semaphores;
> +
> +	/* Enable semaphores on SNB when IO remapping is off */
> +	if (INTEL_INFO(dev)->gen == 6)
> +		return !intel_iommu_enabled;
> +
> +	return 1;
> +}

This doesn't need to go in a header as it is still only used in one
location, unless you are planning more?

> +
>  /* i915_gem_gtt.c */
>  int __must_check i915_gem_init_aliasing_ppgtt(struct drm_device *dev);
>  void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 863e14a..ce2fee5 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2026,6 +2026,49 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj)
>  	return 0;
>  }
>  
> +int
> +i915_gem_object_sync(struct drm_i915_gem_object *obj,
> +		     struct intel_ring_buffer *to)
> +{
> +	struct intel_ring_buffer *from = obj->ring;
> +	u32 seqno;
> +	int ret, idx;
> +
> +	if (from == NULL || to == from)
> +		return 0;
> +
> +	/* XXX gpu semaphores are implicated in various hard hangs on SNB */
This comment is antiquated as we now know the exact hw snafu that
trigger those hangs. And since you already have explained this comment in
intel_enable_semaphores() we can kill it here.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre