On Fri, 23 Sep 2011 08:38:49 -0700, Ben Widawsky <[email protected]> wrote: > On Fri, 23 Sep 2011 08:37:10 +0100 > Chris Wilson <[email protected]> wrote: > > > On Thu, 22 Sep 2011 22:09:39 -0700, Ben Widawsky <[email protected]> > > wrote: > > > On Thu, 22 Sep 2011 21:50:49 -0700 > > > Keith Packard <[email protected]> wrote: > > > > > > > On Thu, 22 Sep 2011 17:11:52 -0700, Ben Widawsky > > > > <[email protected]> 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. > > Crap... This is a bug in the patch.
I'm still can't explain the necessity of forcing the i915_ring_idle() here. If those two lists are empty, then each of the ring write/active lists should also be empty and it will be a no-op. Hence bafflement and time to be slightly afraid. > > > > Also we should not introduce BUG_ON() trivially into user code paths > > (/dev/agp). > > Remove 'em then? Not quite, in this case I think returning -ENODEV is the sensible approach. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
