Re: [PATCH] drm/i915: Sanitize the enable_ppgtt module option once

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 22 Apr 2014, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Otherwise we'll end up spamming dmesg on every context creation on snb
> with vt-d enabled. This regression was introduced in
>
> commit 246cbfb5fb9a1ca0997fbb135464c1ff5bb9c549
> Author: Ben Widawsky <benjamin.widawsky@intel.com>
> Date:   Fri Dec 6 14:11:14 2013 -0800
>
>     drm/i915: Reorganize intel_enable_ppgtt
>
> References: https://lkml.org/lkml/2014/4/17/599
> Cc: Alessandro Suardi <alessandro.suardi@gmail.com>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 0d514ff9b94c..47491c4a1181 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -34,25 +34,35 @@ static void gen8_setup_private_ppat(struct drm_i915_private *dev_priv);
>  
>  bool intel_enable_ppgtt(struct drm_device *dev, bool full)
>  {
> -	if (i915.enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
> +	if (i915.enable_ppgtt == 0)
>  		return false;
>  
>  	if (i915.enable_ppgtt == 1 && full)
>  		return false;
>  
> +	return true;
> +}
> +
> +static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
> +{
> +	if (enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev))
> +		return 0;
> +
> +	if (i915.enable_ppgtt == 1)
> +		return 1;
> +
> +	if (i915.enable_ppgtt == 2 && HAS_PPGTT(dev))
> +		return 2;

You should probably either pass enable_ppgtt as parameter and use that
exclusively, or not pass it and refer to i915.enable_ppgtt directly, but
not mix them.

> +
>  #ifdef CONFIG_INTEL_IOMMU
>  	/* Disable ppgtt on SNB if VT-d is on. */
>  	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped) {
>  		DRM_INFO("Disabling PPGTT because VT-d is on\n");
> -		return false;
> +		return 0;
>  	}
>  #endif
>  
> -	/* Full ppgtt disabled by default for now due to issues. */
> -	if (full)
> -		return HAS_PPGTT(dev) && (i915.enable_ppgtt == 2);
> -	else
> -		return HAS_ALIASING_PPGTT(dev);
> +	return HAS_ALIASING_PPGTT(dev) ? 1 : 0;


This conflicts in -fixes due to
commit 8d214b7d9c45f4af23ce41b2bc74f79c44f760de
Author: Ben Widawsky <benjamin.widawsky@linux.intel.com>
Date:   Mon Mar 24 18:06:00 2014 -0700

    drm/i915: Allow full PPGTT with param override

Should I incorporate that in the conflict resolution for simplicity,
letting 3.15 users also play with full ppgtt with the module param?


BR,
Jani.


>  }
>  
>  
> @@ -1157,6 +1167,8 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ret = 0;
>  
> +	i915.enable_ppgtt = sanitize_enable_ppgtt(dev, i915.enable_ppgtt);
> +
>  	ppgtt->base.dev = dev;
>  	ppgtt->base.scratch = dev_priv->gtt.base.scratch;
>  
> -- 
> 1.9.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center