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

Maarten Lankhorst
2017-10-30T10:22:05Z

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>