From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH] drm/i915: Ironlake GPU with VT-d fix Date: Thu, 22 Sep 2011 22:09:39 -0700 Message-ID: <20110922220939.3c78a1be@bwidawsk.net> References: <1316736712-5887-1-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id 7513D9ED8D for ; Thu, 22 Sep 2011 22:09:28 -0700 (PDT) In-Reply-To: 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: 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 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. > > > - 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... Ben