| Message ID | 1358285181-2128-12-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 15, 2013 at 7:26 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > And, move it to where the rest of the logic is. > > Cc: Dave Airlie <airlied@redhat.com> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/char/agp/intel-gtt.c | 27 --------------------------- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem_gtt.c | 27 ++++++++++++++++++++++++--- > include/drm/intel-gtt.h | 2 -- > 4 files changed, 25 insertions(+), 32 deletions(-) > > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c > index a531377..f54ddc0 100644 > --- a/drivers/char/agp/intel-gtt.c > +++ b/drivers/char/agp/intel-gtt.c > @@ -849,9 +849,6 @@ static int intel_fake_agp_insert_entries(struct agp_memory *mem, > { > int ret = -EINVAL; > > - if (intel_private.base.do_idle_maps) > - return -ENODEV; > - > if (intel_private.clear_fake_agp) { > int start = intel_private.stolen_size / PAGE_SIZE; > int end = intel_private.gtt_mappable_entries; > @@ -916,9 +913,6 @@ static int intel_fake_agp_remove_entries(struct agp_memory *mem, > if (mem->page_count == 0) > return 0; > > - if (intel_private.base.do_idle_maps) > - return -ENODEV; > - I'm not convinced new logic replaces these do_idle_maps asserts > intel_gtt_clear_range(pg_start, mem->page_count); > > if (intel_private.needs_dmar) { > @@ -1078,24 +1072,6 @@ static void i965_write_entry(dma_addr_t addr, > writel(addr | pte_flags, intel_private.gtt + entry); > } > > -/* Certain Gen5 chipsets require require idling the GPU before > - * unmapping anything from the GTT when VT-d is enabled. > - */ > -static inline int needs_idle_maps(void) > -{ > -#ifdef CONFIG_INTEL_IOMMU > - const unsigned short gpu_devid = intel_private.pcidev->device; > - > - /* Query intel_iommu to see if we need the workaround. Presumably that > - * was loaded first. > - */ > - if ((gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_HB || > - gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG) && > - intel_iommu_gfx_mapped) > - return 1; > -#endif > - return 0; > -} > > static int i9xx_setup(void) > { > @@ -1124,9 +1100,6 @@ static int i9xx_setup(void) > break; > } > > - if (needs_idle_maps()) > - intel_private.base.do_idle_maps = 1; > - > intel_i9xx_setup_flush(); > > return 0; > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 201da6d..6c8b0b8 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -385,6 +385,7 @@ struct i915_gtt { > /** "Graphics Stolen Memory" holds the global PTEs */ > void __iomem *gsm; > > + bool do_idle_maps; > }; > #define gtt_total_entries(gtt) ((gtt).total >> PAGE_SHIFT) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 2acca75..afd5ec7 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -330,11 +330,30 @@ void i915_gem_init_ppgtt(struct drm_device *dev) > } > } > > +extern int intel_iommu_gfx_mapped; > +/* Certain Gen5 chipsets require require idling the GPU before > + * unmapping anything from the GTT when VT-d is enabled. > + */ > +static inline bool needs_idle_maps(struct drm_device *dev) > +{ > +#ifdef CONFIG_INTEL_IOMMU > + const u16 gpu_devid = dev->pdev->device; > + > + /* Query intel_iommu to see if we need the workaround. Presumably that > + * was loaded first. > + */ > + if ((gpu_devid == 0x0044 || gpu_devid == 0x46) && I would prefer to stay with defied names instead of direct ids. > + intel_iommu_gfx_mapped) > + return true; > +#endif > + return false; > +} > + > static bool do_idling(struct drm_i915_private *dev_priv) > { > bool ret = dev_priv->mm.interruptible; > > - if (unlikely(dev_priv->mm.gtt->do_idle_maps)) { > + if (unlikely(dev_priv->gtt.do_idle_maps)) { > dev_priv->mm.interruptible = false; > if (i915_gpu_idle(dev_priv->dev)) { > DRM_ERROR("Couldn't idle GPU\n"); > @@ -348,11 +367,10 @@ static bool do_idling(struct drm_i915_private *dev_priv) > > static void undo_idling(struct drm_i915_private *dev_priv, bool interruptible) > { > - if (unlikely(dev_priv->mm.gtt->do_idle_maps)) > + if (unlikely(dev_priv->gtt.do_idle_maps)) > dev_priv->mm.interruptible = interruptible; > } > > - > static void i915_ggtt_clear_range(struct drm_device *dev, > unsigned first_entry, > unsigned num_entries) > @@ -701,6 +719,9 @@ int i915_gem_gtt_init(struct drm_device *dev) > intel_gmch_remove(); > return -ENODEV; > } > + > + dev_priv->gtt.do_idle_maps = needs_idle_maps(dev); > + > return 0; > } > > diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h > index 63157c5..4982dca 100644 > --- a/include/drm/intel-gtt.h > +++ b/include/drm/intel-gtt.h > @@ -5,8 +5,6 @@ > > struct intel_gtt { > /* Whether we idle the gpu before mapping/unmapping */ > - unsigned int do_idle_maps : 1; > - /* Share the scratch page dma with ppgtts. */ > dma_addr_t scratch_page_dma; > struct page *scratch_page; > } *intel_gtt_get(void); > -- > 1.8.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx Other than that, feel free to use: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> -- Rodrigo Vivi Blog: http://blog.vivi.eng.br
On Wed, 16 Jan 2013 21:09:32 -0200 Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote: > On Tue, Jan 15, 2013 at 7:26 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > > And, move it to where the rest of the logic is. > > > > Cc: Dave Airlie <airlied@redhat.com> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/char/agp/intel-gtt.c | 27 --------------------------- > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > drivers/gpu/drm/i915/i915_gem_gtt.c | 27 ++++++++++++++++++++++++--- > > include/drm/intel-gtt.h | 2 -- > > 4 files changed, 25 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c > > index a531377..f54ddc0 100644 > > --- a/drivers/char/agp/intel-gtt.c > > +++ b/drivers/char/agp/intel-gtt.c > > @@ -849,9 +849,6 @@ static int intel_fake_agp_insert_entries(struct agp_memory *mem, > > { > > int ret = -EINVAL; > > > > - if (intel_private.base.do_idle_maps) > > - return -ENODEV; > > - > > if (intel_private.clear_fake_agp) { > > int start = intel_private.stolen_size / PAGE_SIZE; > > int end = intel_private.gtt_mappable_entries; > > @@ -916,9 +913,6 @@ static int intel_fake_agp_remove_entries(struct agp_memory *mem, > > if (mem->page_count == 0) > > return 0; > > > > - if (intel_private.base.do_idle_maps) > > - return -ENODEV; > > - > > I'm not convinced new logic replaces these do_idle_maps asserts As the author of that original logic, it should have always just been a sanity check. I think Chris even mentioned in the original patch we shouldn't bother with the check, but meh. Anyway, you are right that it's not replaced, but that should be fine. > > > intel_gtt_clear_range(pg_start, mem->page_count); > > > > if (intel_private.needs_dmar) { > > @@ -1078,24 +1072,6 @@ static void i965_write_entry(dma_addr_t addr, > > writel(addr | pte_flags, intel_private.gtt + entry); > > } > > > > -/* Certain Gen5 chipsets require require idling the GPU before > > - * unmapping anything from the GTT when VT-d is enabled. > > - */ > > -static inline int needs_idle_maps(void) > > -{ > > -#ifdef CONFIG_INTEL_IOMMU > > - const unsigned short gpu_devid = intel_private.pcidev->device; > > - > > - /* Query intel_iommu to see if we need the workaround. Presumably that > > - * was loaded first. > > - */ > > - if ((gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_HB || > > - gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG) && > > - intel_iommu_gfx_mapped) > > - return 1; > > -#endif > > - return 0; > > -} > > > > static int i9xx_setup(void) > > { > > @@ -1124,9 +1100,6 @@ static int i9xx_setup(void) > > break; > > } > > > > - if (needs_idle_maps()) > > - intel_private.base.do_idle_maps = 1; > > - > > intel_i9xx_setup_flush(); > > > > return 0; > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 201da6d..6c8b0b8 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -385,6 +385,7 @@ struct i915_gtt { > > /** "Graphics Stolen Memory" holds the global PTEs */ > > void __iomem *gsm; > > > > + bool do_idle_maps; > > }; > > #define gtt_total_entries(gtt) ((gtt).total >> PAGE_SHIFT) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index 2acca75..afd5ec7 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -330,11 +330,30 @@ void i915_gem_init_ppgtt(struct drm_device *dev) > > } > > } > > > > +extern int intel_iommu_gfx_mapped; > > +/* Certain Gen5 chipsets require require idling the GPU before > > + * unmapping anything from the GTT when VT-d is enabled. > > + */ > > +static inline bool needs_idle_maps(struct drm_device *dev) > > +{ > > +#ifdef CONFIG_INTEL_IOMMU > > + const u16 gpu_devid = dev->pdev->device; > > + > > + /* Query intel_iommu to see if we need the workaround. Presumably that > > + * was loaded first. > > + */ > > + if ((gpu_devid == 0x0044 || gpu_devid == 0x46) && > > I would prefer to stay with defied names instead of direct ids. > There is one problem, apparently we don't support 0x44 in our kernel, and I didn't want to have the workaround there for an undefined chipset. I see no reason to add 0x44 if Intel never shipped one. So I'd be happy to use an existing define for 0x46, and leave 0x44 hardcoded if that pleases people. > > + intel_iommu_gfx_mapped) > > + return true; > > +#endif > > + return false; > > +} > > + > > static bool do_idling(struct drm_i915_private *dev_priv) > > { > > bool ret = dev_priv->mm.interruptible; > > > > - if (unlikely(dev_priv->mm.gtt->do_idle_maps)) { > > + if (unlikely(dev_priv->gtt.do_idle_maps)) { > > dev_priv->mm.interruptible = false; > > if (i915_gpu_idle(dev_priv->dev)) { > > DRM_ERROR("Couldn't idle GPU\n"); > > @@ -348,11 +367,10 @@ static bool do_idling(struct drm_i915_private *dev_priv) > > > > static void undo_idling(struct drm_i915_private *dev_priv, bool interruptible) > > { > > - if (unlikely(dev_priv->mm.gtt->do_idle_maps)) > > + if (unlikely(dev_priv->gtt.do_idle_maps)) > > dev_priv->mm.interruptible = interruptible; > > } > > > > - > > static void i915_ggtt_clear_range(struct drm_device *dev, > > unsigned first_entry, > > unsigned num_entries) > > @@ -701,6 +719,9 @@ int i915_gem_gtt_init(struct drm_device *dev) > > intel_gmch_remove(); > > return -ENODEV; > > } > > + > > + dev_priv->gtt.do_idle_maps = needs_idle_maps(dev); > > + > > return 0; > > } > > > > diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h > > index 63157c5..4982dca 100644 > > --- a/include/drm/intel-gtt.h > > +++ b/include/drm/intel-gtt.h > > @@ -5,8 +5,6 @@ > > > > struct intel_gtt { > > /* Whether we idle the gpu before mapping/unmapping */ > > - unsigned int do_idle_maps : 1; > > - /* Share the scratch page dma with ppgtts. */ > > dma_addr_t scratch_page_dma; > > struct page *scratch_page; > > } *intel_gtt_get(void); > > -- > > 1.8.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > Other than that, feel free to use: > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c index a531377..f54ddc0 100644 --- a/drivers/char/agp/intel-gtt.c +++ b/drivers/char/agp/intel-gtt.c @@ -849,9 +849,6 @@ static int intel_fake_agp_insert_entries(struct agp_memory *mem, { int ret = -EINVAL; - if (intel_private.base.do_idle_maps) - return -ENODEV; - if (intel_private.clear_fake_agp) { int start = intel_private.stolen_size / PAGE_SIZE; int end = intel_private.gtt_mappable_entries; @@ -916,9 +913,6 @@ static int intel_fake_agp_remove_entries(struct agp_memory *mem, if (mem->page_count == 0) return 0; - if (intel_private.base.do_idle_maps) - return -ENODEV; - intel_gtt_clear_range(pg_start, mem->page_count); if (intel_private.needs_dmar) { @@ -1078,24 +1072,6 @@ static void i965_write_entry(dma_addr_t addr, writel(addr | pte_flags, intel_private.gtt + entry); } -/* Certain Gen5 chipsets require require idling the GPU before - * unmapping anything from the GTT when VT-d is enabled. - */ -static inline int needs_idle_maps(void) -{ -#ifdef CONFIG_INTEL_IOMMU - const unsigned short gpu_devid = intel_private.pcidev->device; - - /* Query intel_iommu to see if we need the workaround. Presumably that - * was loaded first. - */ - if ((gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_HB || - gpu_devid == PCI_DEVICE_ID_INTEL_IRONLAKE_M_IG) && - intel_iommu_gfx_mapped) - return 1; -#endif - return 0; -} static int i9xx_setup(void) { @@ -1124,9 +1100,6 @@ static int i9xx_setup(void) break; } - if (needs_idle_maps()) - intel_private.base.do_idle_maps = 1; - intel_i9xx_setup_flush(); return 0; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 201da6d..6c8b0b8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -385,6 +385,7 @@ struct i915_gtt { /** "Graphics Stolen Memory" holds the global PTEs */ void __iomem *gsm; + bool do_idle_maps; }; #define gtt_total_entries(gtt) ((gtt).total >> PAGE_SHIFT) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 2acca75..afd5ec7 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -330,11 +330,30 @@ void i915_gem_init_ppgtt(struct drm_device *dev) } } +extern int intel_iommu_gfx_mapped; +/* Certain Gen5 chipsets require require idling the GPU before + * unmapping anything from the GTT when VT-d is enabled. + */ +static inline bool needs_idle_maps(struct drm_device *dev) +{ +#ifdef CONFIG_INTEL_IOMMU + const u16 gpu_devid = dev->pdev->device; + + /* Query intel_iommu to see if we need the workaround. Presumably that + * was loaded first. + */ + if ((gpu_devid == 0x0044 || gpu_devid == 0x46) && + intel_iommu_gfx_mapped) + return true; +#endif + return false; +} + static bool do_idling(struct drm_i915_private *dev_priv) { bool ret = dev_priv->mm.interruptible; - if (unlikely(dev_priv->mm.gtt->do_idle_maps)) { + if (unlikely(dev_priv->gtt.do_idle_maps)) { dev_priv->mm.interruptible = false; if (i915_gpu_idle(dev_priv->dev)) { DRM_ERROR("Couldn't idle GPU\n"); @@ -348,11 +367,10 @@ static bool do_idling(struct drm_i915_private *dev_priv) static void undo_idling(struct drm_i915_private *dev_priv, bool interruptible) { - if (unlikely(dev_priv->mm.gtt->do_idle_maps)) + if (unlikely(dev_priv->gtt.do_idle_maps)) dev_priv->mm.interruptible = interruptible; } - static void i915_ggtt_clear_range(struct drm_device *dev, unsigned first_entry, unsigned num_entries) @@ -701,6 +719,9 @@ int i915_gem_gtt_init(struct drm_device *dev) intel_gmch_remove(); return -ENODEV; } + + dev_priv->gtt.do_idle_maps = needs_idle_maps(dev); + return 0; } diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h index 63157c5..4982dca 100644 --- a/include/drm/intel-gtt.h +++ b/include/drm/intel-gtt.h @@ -5,8 +5,6 @@ struct intel_gtt { /* Whether we idle the gpu before mapping/unmapping */ - unsigned int do_idle_maps : 1; - /* Share the scratch page dma with ppgtts. */ dma_addr_t scratch_page_dma; struct page *scratch_page; } *intel_gtt_get(void);
And, move it to where the rest of the logic is. Cc: Dave Airlie <airlied@redhat.com> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/char/agp/intel-gtt.c | 27 --------------------------- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem_gtt.c | 27 ++++++++++++++++++++++++--- include/drm/intel-gtt.h | 2 -- 4 files changed, 25 insertions(+), 32 deletions(-)