From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH] drm/i915: Sanitize the enable_ppgtt module option once Date: Wed, 23 Apr 2014 10:44:01 -0700 Message-ID: <20140423174401.GC25941@bwidawsk.net> References: <1398197870-20488-1-git-send-email-daniel.vetter@ffwll.ch> <20140422212215.GB22645@nuc-i3427.alporthouse.com> <20140423071803.GU10722@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail.bwidawsk.net (bwidawsk.net [166.78.191.112]) by gabe.freedesktop.org (Postfix) with ESMTP id EEB586E02B for ; Wed, 23 Apr 2014 10:44:16 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140423071803.GU10722@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: Daniel Vetter , Intel Graphics Development , Alessandro Suardi List-Id: intel-gfx@lists.freedesktop.org On Wed, Apr 23, 2014 at 09:18:03AM +0200, Daniel Vetter wrote: > On Tue, Apr 22, 2014 at 10:22:15PM +0100, Chris Wilson wrote: > > On Tue, Apr 22, 2014 at 10:17:50PM +0200, Daniel Vetter 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 > > > Date: Fri Dec 6 14:11:14 2013 -0800 > > > > > > drm/i915: Reorganize intel_enable_ppgtt > > > > I started to consider what would happen if i915.enable_ppgtt changed on > > the fly, but then saw that it is 0400 and this pre-initialisation makes > > a lot of sense. So maybe we could mention that here: > > > > As the i915.enable_ppgtt is read-only it cannot be changed after the > > module is loaded and so we can perform an early sanitization of the > > values. > > > > > References: https://lkml.org/lkml/2014/4/17/599 > > > Cc: Alessandro Suardi > > > Cc: Ben Widawsky > > > Signed-off-by: Daniel Vetter > > > > If you care to add in some of the comments, > > Reviewed-by: Chris Wilson > > Fully agreed on both since the first thing I've checked is that the > enable_ppgtt option is indeed 0400 ;-) > > Jani, can you please apply Chris' commit message text and comment when > merging? > -Daniel Here as well: Reviewed-by: Ben Widawsky > > > > > > 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; > > > +} > > > + > > > > /* i915.enable_ppgtt is read-only, so do an early pass to validate > > * the user's requested state against the hardware/driver capabilities. > > * We do this now so that we can print out any log messages once rather > > * than every time we check intel_enable_ppgtt(). > > */ > > > +static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt) > > > +{ > > -Chris > > > > -- > > Chris Wilson, Intel Open Source Technology Centre > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Ben Widawsky, Intel Open Source Technology Center