[Intel-gfx] [PATCH] drm/i915: Eliminate dead code in intel_sanitize_enable_ppgtt()

[Chris Wilson]

Quoting Imre Deak (2017-07-25 22:12:36)
> From: Damien Lespiau <damien.lespiau at intel.com>
> 
> We exit early if has_aliasing_ppgtt is 0, so towards the end of the
> function has_aliasing_ppgtt can only be 1.
> 
> Also:
> 
>         if (foo)
>                 return 1;
>         else
>                 return 0;
> 
> when foo is already a bool is really just:
> 
>         return foo;
> 
> v2:
> - Simplify more using the fact that i915.enable_execlists requires that
>   GEN >= 8. (Chris)
> 
> Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> [Imre: updated to v2]
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 10aa7762d9a6..f401318430ed 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -133,7 +133,7 @@ static inline void i915_ggtt_invalidate(struct drm_i915_private *i915)
>  }
>  
>  int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
> -                               int enable_ppgtt)
> +                               int enable_ppgtt)
>  {
>         bool has_aliasing_ppgtt;
>         bool has_full_ppgtt;
> @@ -180,10 +180,14 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
>                 return 0;
>         }
>  
> -       if (INTEL_GEN(dev_priv) >= 8 && i915.enable_execlists && has_full_ppgtt)
> -               return has_full_48bit_ppgtt ? 3 : 2;
> -       else
> -               return has_aliasing_ppgtt ? 1 : 0;
> +       if (!has_full_ppgtt)
> +               return 1;
> +
> +       /* full-ppgtt doesn't yet work reliably in legacy ringbuffer mode */
> +       if (!i915.enable_execlists)
> +               return 1;
> +
> +       return has_full_48bit_ppgtt ? 3 : 2;
>  }

I was looking at a more complete overhaul, say

nt intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
-                               int enable_ppgtt)
+                               int enable_ppgtt)
 {
-       bool has_aliasing_ppgtt;
-       bool has_full_ppgtt;
-       bool has_full_48bit_ppgtt;
+       unsigned int min, max;
 
-       has_aliasing_ppgtt = dev_priv->info.has_aliasing_ppgtt;
-       has_full_ppgtt = dev_priv->info.has_full_ppgtt;
-       has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt;
-
-       if (intel_vgpu_active(dev_priv)) {
-               /* emulation is too hard */
-               has_full_ppgtt = false;
-               has_full_48bit_ppgtt = false;
-       }
-
-       if (!has_aliasing_ppgtt)
-               return 0;
-
-       /*
-        * We don't allow disabling PPGTT for gen9+ as it's a requirement for
-        * execlists, the sole mechanism available to submit work.
-        */
-       if (enable_ppgtt == 0 && INTEL_GEN(dev_priv) < 9)
-               return 0;
-
-       if (enable_ppgtt == 1)
-               return 1;
-
-       if (enable_ppgtt == 2 && has_full_ppgtt)
-               return 2;
-
-       if (enable_ppgtt == 3 && has_full_48bit_ppgtt)
-               return 3;
+       if (dev_priv->info.has_full_48bit_ppgtt)
+               max = 3;
+       else if (dev_priv->info.has_full_ppgtt)
+               max = 2;
+       else if (dev_priv->info.has_aliasing_ppgtt)
+               max = 1;
+       else
+               max = 0;
 
        /* Disable ppgtt on SNB if VT-d is on. */
        if (IS_GEN6(dev_priv) && intel_vtd_active()) {
                DRM_INFO("Disabling PPGTT because VT-d is on\n");
-               return 0;
+               max = 0;
        }
 
        /* Early VLV doesn't have this */
        if (IS_VALLEYVIEW(dev_priv) && dev_priv->drm.pdev->revision < 0xb) {
                DRM_DEBUG_DRIVER("disabling PPGTT on pre-B3 step VLV\n");
-               return 0;
+               max = 0;
        }
 
-       if (INTEL_GEN(dev_priv) >= 8 && i915.enable_execlists && has_full_ppgtt)
-               return has_full_48bit_ppgtt ? 3 : 2;
-       else
-               return has_aliasing_ppgtt ? 1 : 0;
+       /* full-ppgtt doesn't yet work reliably in legacy ringbuffer mode */
+       if (!i915.enable_execlists)
+               max = min(max, 1u);
+
+       /* emulation of full-ppgtt is too hard */
+       if (intel_vgpu_active(dev_priv))
+               max = min(max, 1u);
+
+       /*
+        * We don't allow disabling PPGTT for gen9+ as it's a requirement for
+        * execlists, the sole mechanism available to submit work.
+        */
+       min = 0;
+       if (INTEL_GEN(dev_priv) >= 9)
+               min = 1;
+
+       return clamp_t(unsigned int, enable_ppgtt, min, max);
 }
 
 static int ppgtt_bind_vma(struct i915_vma *vma,


i.e.

int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
                                int enable_ppgtt)
{
        unsigned int min, max;

        if (dev_priv->info.has_full_48bit_ppgtt)
                max = 3;
        else if (dev_priv->info.has_full_ppgtt)
                max = 2;
        else if (dev_priv->info.has_aliasing_ppgtt)
                max = 1;
        else
                max = 0;

        /* Disable ppgtt on SNB if VT-d is on. */
        if (IS_GEN6(dev_priv) && intel_vtd_active()) {
                DRM_INFO("Disabling PPGTT because VT-d is on\n");
                max = 0;
        }

        /* Early VLV doesn't have this */
        if (IS_VALLEYVIEW(dev_priv) && dev_priv->drm.pdev->revision < 0xb) {
                DRM_DEBUG_DRIVER("disabling PPGTT on pre-B3 step VLV\n");
                max = 0;
        }

        /* full-ppgtt doesn't yet work reliably in legacy ringbuffer mode */
        if (!i915.enable_execlists)
                max = min(max, 1u);

        /* emulation of full-ppgtt is too hard */
        if (intel_vgpu_active(dev_priv))
                max = min(max, 1u);

        /*
         * We don't allow disabling PPGTT for gen9+ as it's a requirement for
         * execlists, the sole mechanism available to submit work.
         */
        min = 0;
        if (INTEL_GEN(dev_priv) >= 9)
                min = 1;

        return clamp_t(unsigned int, enable_ppgtt, min, max);
}