Re: [PATCH 3/3] drm/i915: Remove i915.semaphores modparam

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 28-10-17 om 13:27 schreef Chris Wilson:
> Having disabled the broken semaphores on Sandybridge, there is no need
> for a modparam any more, so remove it in favour of a simple
> HAS_LEGACY_SEMAPHORES() guard.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |  7 +------
>  drivers/gpu/drm/i915/i915_drv.h         |  4 ++--
>  drivers/gpu/drm/i915/i915_gem.c         | 11 -----------
>  drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
>  drivers/gpu/drm/i915/i915_params.c      |  4 ----
>  drivers/gpu/drm/i915/i915_params.h      |  1 -
>  drivers/gpu/drm/i915/intel_engine_cs.c  |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  4 ++--
>  8 files changed, 7 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e021ae585f70..a23b3edaca23 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -321,7 +321,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  		value = USES_PPGTT(dev_priv);
>  		break;
>  	case I915_PARAM_HAS_SEMAPHORES:
> -		value = i915_modparams.semaphores;
> +		value = HAS_LEGACY_SEMAPHORES(dev_priv);
>  		break;
>  	case I915_PARAM_HAS_SECURE_BATCHES:
>  		value = capable(CAP_SYS_ADMIN);
> @@ -1056,11 +1056,6 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
>  					    i915_modparams.enable_ppgtt);
>  	DRM_DEBUG_DRIVER("ppgtt mode: %i\n", i915_modparams.enable_ppgtt);
>  
> -	i915_modparams.semaphores =
> -		intel_sanitize_semaphores(dev_priv, i915_modparams.semaphores);
> -	DRM_DEBUG_DRIVER("use GPU semaphores? %s\n",
> -			 yesno(i915_modparams.semaphores));
> -
>  	intel_uc_sanitize_options(dev_priv);
>  
>  	intel_gvt_sanitize_options(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cd0bd8d16ef0..326a06f844f9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3127,6 +3127,8 @@ intel_info(const struct drm_i915_private *dev_priv)
>  #define HAS_BLT(dev_priv)	HAS_ENGINE(dev_priv, BCS)
>  #define HAS_VEBOX(dev_priv)	HAS_ENGINE(dev_priv, VECS)
>  
> +#define HAS_LEGACY_SEMAPHORES(dev_priv) IS_GEN7(dev_priv)
> +
>  #define HAS_LLC(dev_priv)	((dev_priv)->info.has_llc)
>  #define HAS_SNOOP(dev_priv)	((dev_priv)->info.has_snoop)
>  #define HAS_EDRAM(dev_priv)	(!!((dev_priv)->edram_cap & EDRAM_ENABLED))
> @@ -3291,8 +3293,6 @@ intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *dev_priv)
>  int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
>  				int enable_ppgtt);
>  
> -bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value);
> -
>  /* i915_drv.c */
>  void __printf(3, 4)
>  __i915_printk(struct drm_i915_private *dev_priv, const char *level,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index eee699dd376e..ab4a0a2b99fe 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4948,17 +4948,6 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
>  	return ret;
>  }
>  
> -bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
> -{
> -	if (!IS_GEN7(dev_priv))
> -		return false;
> -
> -	if (value >= 0)
> -		return value;
> -
> -	return true;
> -}
> -
>  int i915_gem_init(struct drm_i915_private *dev_priv)
>  {
>  	int ret;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index a843e35f00db..103a633754f3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -596,7 +596,7 @@ mi_set_context(struct drm_i915_gem_request *req, u32 flags)
>  	enum intel_engine_id id;
>  	const int num_rings =
>  		/* Use an extended w/a on gen7 if signalling from other rings */
> -		(i915_modparams.semaphores && INTEL_GEN(dev_priv) == 7) ?
> +		(HAS_LEGACY_SEMAPHORES(dev_priv) && IS_GEN7(dev_priv)) ?
The HAS_LEGACY_SEMAPHORES becomes redundant here. :)

Also typo in commit 1, sempahores -> semaphores

If you can get an ack from someone who knows more about snb semaphores and if this actually gets through CI then

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx