[Intel-gfx] [PATCH 10/13 v2] drm/i915: Needs_dmar, not

Rodrigo Vivi rodrigo.vivi at gmail.com
Thu Jan 17 14:50:26 CET 2013


Reviewed-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>

On Wed, Jan 16, 2013 at 4:22 PM, Ben Widawsky <ben at bwidawsk.net> wrote:
> The reasoning behind our code taking two paths depending upon whether or
> not we may have been configured for IOMMU isn't clear to me. It should
> always be safe to use the pci mapping functions as they are designed to
> abstract the decision we were handling in i915.
>
> Aside from simpler code, removing another member for the intel_gtt
> struct is a nice motivation.
>
> I ran this by Chris, and he wasn't concerned about the extra kzalloc,
> and memory references vs. page_to_phys calculation in the case without
> IOMMU.
>
> v2: Update commit message
>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  drivers/char/agp/intel-gtt.c        | 10 ++++++----
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 35 +++++++++++++----------------------
>  include/drm/intel-gtt.h             |  2 --
>  3 files changed, 19 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> index eb05eb5..a531377 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -84,6 +84,8 @@ static struct _intel_private {
>          * this is not the full gtt. */
>         unsigned int gtt_mappable_entries;
>         phys_addr_t gma_bus_addr;
> +       /* Whether i915 needs to use the dmar apis or not. */
> +       unsigned int needs_dmar : 1;
>  } intel_private;
>
>  #define INTEL_GTT_GEN  intel_private.driver->gen
> @@ -299,7 +301,7 @@ static int intel_gtt_setup_scratch_page(void)
>         get_page(page);
>         set_pages_uc(page, 1);
>
> -       if (intel_private.base.needs_dmar) {
> +       if (intel_private.needs_dmar) {
>                 dma_addr = pci_map_page(intel_private.pcidev, page, 0,
>                                     PAGE_SIZE, PCI_DMA_BIDIRECTIONAL);
>                 if (pci_dma_mapping_error(intel_private.pcidev, dma_addr))
> @@ -615,7 +617,7 @@ static int intel_gtt_init(void)
>
>         intel_private.stolen_size = intel_gtt_stolen_size();
>
> -       intel_private.base.needs_dmar = USE_PCI_DMA_API && INTEL_GTT_GEN > 2;
> +       intel_private.needs_dmar = USE_PCI_DMA_API && INTEL_GTT_GEN > 2;
>
>         ret = intel_gtt_setup_scratch_page();
>         if (ret != 0) {
> @@ -875,7 +877,7 @@ static int intel_fake_agp_insert_entries(struct agp_memory *mem,
>         if (!mem->is_flushed)
>                 global_cache_flush();
>
> -       if (intel_private.base.needs_dmar) {
> +       if (intel_private.needs_dmar) {
>                 struct sg_table st;
>
>                 ret = intel_gtt_map_memory(mem->pages, mem->page_count, &st);
> @@ -919,7 +921,7 @@ static int intel_fake_agp_remove_entries(struct agp_memory *mem,
>
>         intel_gtt_clear_range(pg_start, mem->page_count);
>
> -       if (intel_private.base.needs_dmar) {
> +       if (intel_private.needs_dmar) {
>                 intel_gtt_unmap_memory(mem->sg_list, mem->num_sg);
>                 mem->sg_list = NULL;
>                 mem->num_sg = 0;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index a92d8cd..ae96835 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -139,28 +139,23 @@ int i915_gem_init_aliasing_ppgtt(struct drm_device *dev)
>                         goto err_pt_alloc;
>         }
>
> -       if (dev_priv->mm.gtt->needs_dmar) {
> -               ppgtt->pt_dma_addr = kzalloc(sizeof(dma_addr_t)
> -                                               *ppgtt->num_pd_entries,
> -                                            GFP_KERNEL);
> -               if (!ppgtt->pt_dma_addr)
> -                       goto err_pt_alloc;
> +       ppgtt->pt_dma_addr = kzalloc(sizeof(dma_addr_t) *ppgtt->num_pd_entries,
> +                                    GFP_KERNEL);
> +       if (!ppgtt->pt_dma_addr)
> +               goto err_pt_alloc;
>
> -               for (i = 0; i < ppgtt->num_pd_entries; i++) {
> -                       dma_addr_t pt_addr;
> +       for (i = 0; i < ppgtt->num_pd_entries; i++) {
> +               dma_addr_t pt_addr;
>
> -                       pt_addr = pci_map_page(dev->pdev, ppgtt->pt_pages[i],
> -                                              0, 4096,
> -                                              PCI_DMA_BIDIRECTIONAL);
> +               pt_addr = pci_map_page(dev->pdev, ppgtt->pt_pages[i], 0, 4096,
> +                                      PCI_DMA_BIDIRECTIONAL);
>
> -                       if (pci_dma_mapping_error(dev->pdev,
> -                                                 pt_addr)) {
> -                               ret = -EIO;
> -                               goto err_pd_pin;
> +               if (pci_dma_mapping_error(dev->pdev, pt_addr)) {
> +                       ret = -EIO;
> +                       goto err_pd_pin;
>
> -                       }
> -                       ppgtt->pt_dma_addr[i] = pt_addr;
>                 }
> +               ppgtt->pt_dma_addr[i] = pt_addr;
>         }
>
>         ppgtt->scratch_page_dma_addr = dev_priv->mm.gtt->scratch_page_dma;
> @@ -295,11 +290,7 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
>         for (i = 0; i < ppgtt->num_pd_entries; i++) {
>                 dma_addr_t pt_addr;
>
> -               if (dev_priv->mm.gtt->needs_dmar)
> -                       pt_addr = ppgtt->pt_dma_addr[i];
> -               else
> -                       pt_addr = page_to_phys(ppgtt->pt_pages[i]);
> -
> +               pt_addr = ppgtt->pt_dma_addr[i];
>                 pd_entry = GEN6_PDE_ADDR_ENCODE(pt_addr);
>                 pd_entry |= GEN6_PDE_VALID;
>
> diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
> index 6f53ecd..63157c5 100644
> --- a/include/drm/intel-gtt.h
> +++ b/include/drm/intel-gtt.h
> @@ -4,8 +4,6 @@
>  #define        _DRM_INTEL_GTT_H
>
>  struct intel_gtt {
> -       /*  Whether i915 needs to use the dmar apis or not. */
> -       unsigned int needs_dmar : 1;
>         /* Whether we idle the gpu before mapping/unmapping */
>         unsigned int do_idle_maps : 1;
>         /* Share the scratch page dma with ppgtts. */
> --
> 1.8.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br



More information about the Intel-gfx mailing list