From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH] drm/i915: Ironlake GPU with VT-d fix Date: Fri, 23 Sep 2011 08:37:10 +0100 Message-ID: References: <1316736712-5887-1-git-send-email-ben@bwidawsk.net> <20110922220939.3c78a1be@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 8D1B89E97F for ; Fri, 23 Sep 2011 00:37:15 -0700 (PDT) In-Reply-To: <20110922220939.3c78a1be@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ben Widawsky , Keith Packard Cc: Dave Airlie , intel-gfx@lists.freedesktop.org, Woodhouse , David@freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 22 Sep 2011 22:09:39 -0700, Ben Widawsky wrote: > On Thu, 22 Sep 2011 21:50:49 -0700 > Keith Packard wrote: > > > On Thu, 22 Sep 2011 17:11:52 -0700, Ben Widawsky > > wrote: > > > > > It requires an additional IOMMU patch. > > > > Can we collect those two patches into one sequence? > > > > Yes. Although not sure who would do the pull request to Linus. > > > > + if ((gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_HB || > > > + gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG) && > > > + intel_private.base.needs_dmar) > > > + intel_private.base.do_idle_maps = 1; > > > + > > > > I'd like to make this conditional on whether IOMMU is actually in use; > > needs_dmar is based solely on whether the DMA_API is compiled into the > > kernel and the GTT gen is > 2. > > David did mention a way for me to detect it. I wasn't sure if there was a > deadline to get this patch out so I submitted it now. I will work on the > detection portion of it while the rest gets review. It's something like, check > if the bus address == the physical address and if not, assume the IOMMU is in > use. And for starters, we can check no_iommu. But really we want intel_iommu.c to report whether or not it enabled VTd on the gfx. > > > > > - if (lists_empty) > > > + if (lists_empty && !!dev_priv->mm.gtt->do_idle_maps) > > > return 0; > > > > Is it necessary to change the semantic of this function in cases which > > aren't related to GTT remapping? Seems like you're imposing a fairly > > high cost on operations which don't actually need it. > > > > Not sure I follow. On that specific hunk it's apparently needed. I tried the > patch without this and we got some data back which suggested it didn't work. It > would be nice if we could reproduce this locally... Keith is right, there should be no reason to touch that function, and especially not for the normal code paths. Also we should not introduce BUG_ON() trivially into user code paths (/dev/agp). -Chris -- Chris Wilson, Intel Open Source Technology Centre