[RFC PATCH] drm/panfrost: Add support for mapping BOs on GPU page faults

Tomeu Vizoso tomeu at tomeuvizoso.net
Wed Jun 12 12:54:56 UTC 2019


On Mon, 10 Jun 2019 at 19:06, Rob Herring <robh at kernel.org> wrote:
>
> The midgard/bifrost GPUs need to allocate GPU memory which is allocated
> on GPU page faults and not pinned in memory. The vendor driver calls
> this functionality GROW_ON_GPF.
>
> This implementation assumes that BOs allocated with the
> PANFROST_BO_NOMAP flag are never mmapped or exported. Both of those may
> actually work, but I'm unsure if there's some interaction there. It
> would cause the whole object to be pinned in memory which would defeat
> the point of this.
>
> Issues/questions/thoughts:
>
> What's the difference between i_mapping and f_mapping?
>
> What kind of clean-up on close is needed? Based on vgem faults, there
> doesn't seem to be any refcounting. Assume userspace is responsible for
> not freeing the BO while a page fault can occur?

Aren't we taking a reference on all BOs that a job relates to and
unreferencing them once the job is done? I would think that that's
enough, or am I missing something?

> What about evictions? Need to call mapping_set_unevictable()? Maybe we
> want these pages to be swappable, but then we need some notification to
> unmap them.

I'm not sure there's much point in swapping out pages with lifetimes
of a few milliseconds.

> No need for runtime PM, because faults should only occur during job
> submit?

Makes sense to me.

> Need to fault in 2MB sections when possible. It seems this has to be
> done by getting an array of struct pages and then converting to a
> scatterlist. Not sure if there is a better way to do that. There's also
> some errata on T604 needing to fault in at least 8 pages at a time which
> isn't handled.

The old GPUs will be the most interesting to support...

Will give this patch some testing tomorrow.

Thanks,

Tomeu

> Cc: Robin Murphy <robin.murphy at arm.com>
> Cc: Steven Price <steven.price at arm.com>
> Cc: Alyssa Rosenzweig <alyssa at rosenzweig.io>
> Cc: Tomeu Vizoso <tomeu at tomeuvizoso.net>
> Signed-off-by: Rob Herring <robh at kernel.org>
> ---
>
>  drivers/gpu/drm/panfrost/TODO           |  2 -
>  drivers/gpu/drm/panfrost/panfrost_drv.c | 22 +++++--
>  drivers/gpu/drm/panfrost/panfrost_gem.c |  3 +-
>  drivers/gpu/drm/panfrost/panfrost_gem.h |  8 +++
>  drivers/gpu/drm/panfrost/panfrost_mmu.c | 76 +++++++++++++++++++++++--
>  include/uapi/drm/panfrost_drm.h         |  2 +
>  6 files changed, 100 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/panfrost/TODO b/drivers/gpu/drm/panfrost/TODO
> index c2e44add37d8..64129bf73933 100644
> --- a/drivers/gpu/drm/panfrost/TODO
> +++ b/drivers/gpu/drm/panfrost/TODO
> @@ -14,8 +14,6 @@
>    The hard part is handling when more address spaces are needed than what
>    the h/w provides.
>
> -- Support pinning pages on demand (GPU page faults).
> -
>  - Support userspace controlled GPU virtual addresses. Needed for Vulkan. (Tomeu)
>
>  - Support for madvise and a shrinker.
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index d11e2281dde6..f08d41d5186a 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -44,9 +44,11 @@ static int panfrost_ioctl_create_bo(struct drm_device *dev, void *data,
>  {
>         int ret;
>         struct drm_gem_shmem_object *shmem;
> +       struct panfrost_gem_object *bo;
>         struct drm_panfrost_create_bo *args = data;
>
> -       if (!args->size || args->flags || args->pad)
> +       if (!args->size || args->pad ||
> +           (args->flags & ~PANFROST_BO_NOMAP))
>                 return -EINVAL;
>
>         shmem = drm_gem_shmem_create_with_handle(file, dev, args->size,
> @@ -54,11 +56,16 @@ static int panfrost_ioctl_create_bo(struct drm_device *dev, void *data,
>         if (IS_ERR(shmem))
>                 return PTR_ERR(shmem);
>
> -       ret = panfrost_mmu_map(to_panfrost_bo(&shmem->base));
> -       if (ret)
> -               goto err_free;
> +       bo = to_panfrost_bo(&shmem->base);
>
> -       args->offset = to_panfrost_bo(&shmem->base)->node.start << PAGE_SHIFT;
> +       if (!(args->flags & PANFROST_BO_NOMAP)) {
> +               ret = panfrost_mmu_map(bo);
> +               if (ret)
> +                       goto err_free;
> +       } else
> +               bo->no_map = 1;
> +
> +       args->offset = bo->node.start << PAGE_SHIFT;
>
>         return 0;
>
> @@ -269,6 +276,11 @@ static int panfrost_ioctl_mmap_bo(struct drm_device *dev, void *data,
>                 return -ENOENT;
>         }
>
> +       if (to_panfrost_bo(gem_obj)->no_map) {
> +               DRM_DEBUG("Can't mmap 'no_map' GEM BO %d\n", args->handle);
> +               return -EINVAL;
> +       }
> +
>         ret = drm_gem_create_mmap_offset(gem_obj);
>         if (ret == 0)
>                 args->offset = drm_vma_node_offset_addr(&gem_obj->vma_node);
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
> index 886875ae31d3..ed0b4f79610a 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
> @@ -19,7 +19,8 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj)
>         struct panfrost_gem_object *bo = to_panfrost_bo(obj);
>         struct panfrost_device *pfdev = obj->dev->dev_private;
>
> -       panfrost_mmu_unmap(bo);
> +       if (!bo->no_map)
> +               panfrost_mmu_unmap(bo);
>
>         spin_lock(&pfdev->mm_lock);
>         drm_mm_remove_node(&bo->node);
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
> index 045000eb5fcf..aa210d7cbf3f 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
> @@ -11,6 +11,7 @@ struct panfrost_gem_object {
>         struct drm_gem_shmem_object base;
>
>         struct drm_mm_node node;
> +       unsigned no_map: 1;
>  };
>
>  static inline
> @@ -19,6 +20,13 @@ struct  panfrost_gem_object *to_panfrost_bo(struct drm_gem_object *obj)
>         return container_of(to_drm_gem_shmem_obj(obj), struct panfrost_gem_object, base);
>  }
>
> +static inline
> +struct  panfrost_gem_object *drm_mm_node_to_panfrost_bo(struct drm_mm_node *node)
> +{
> +       return container_of(node, struct panfrost_gem_object, node);
> +}
> +
> +
>  struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t size);
>
>  struct drm_gem_object *
> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index 41d184fab07f..5a36495a38f3 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -3,6 +3,7 @@
>  /* Copyright (C) 2019 Arm Ltd. */
>  #include <linux/bitfield.h>
>  #include <linux/delay.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
> @@ -10,6 +11,7 @@
>  #include <linux/iommu.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/shmem_fs.h>
>  #include <linux/sizes.h>
>
>  #include "panfrost_device.h"
> @@ -277,6 +279,60 @@ static const struct iommu_gather_ops mmu_tlb_ops = {
>         .tlb_sync       = mmu_tlb_sync_context,
>  };
>
> +static struct drm_mm_node *addr_to_drm_mm_node(struct panfrost_device *pfdev, int as, u64 addr)
> +{
> +       struct drm_mm_node *node;
> +       u64 offset = addr >> PAGE_SHIFT;
> +
> +       drm_mm_for_each_node(node, &pfdev->mm) {
> +               if (offset >= node->start && offset < (node->start + node->size))
> +                       return node;
> +       }
> +       return NULL;
> +}
> +
> +int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, u64 addr)
> +{
> +       struct page *page;
> +       dma_addr_t dma_addr;
> +       struct drm_mm_node *node;
> +       struct panfrost_gem_object *bo;
> +       struct io_pgtable_ops *ops = pfdev->mmu->pgtbl_ops;
> +       pgoff_t page_offset = addr >> PAGE_SHIFT;
> +
> +       addr &= PAGE_MASK;
> +
> +       node = addr_to_drm_mm_node(pfdev, as, addr);
> +       if (!node)
> +               return -ENOENT;
> +
> +       page_offset -= node->start;
> +       bo = drm_mm_node_to_panfrost_bo(node);
> +       if (WARN_ON(!bo->no_map))
> +               return -EINVAL;
> +
> +       dev_info(pfdev->dev, "mapping page fault @ %llx", addr);
> +
> +       page = shmem_read_mapping_page(bo->base.base.filp->f_mapping,
> +                                      page_offset);
> +       if (IS_ERR(page))
> +               return PTR_ERR(page);
> +
> +       dma_addr = dma_map_page(pfdev->dev, page, 0, PAGE_SIZE, DMA_BIDIRECTIONAL);
> +
> +       mutex_lock(&pfdev->mmu->lock);
> +
> +       ops->map(ops, addr, dma_addr, PAGE_SIZE, IOMMU_WRITE | IOMMU_READ);
> +
> +       mmu_write(pfdev, MMU_INT_CLEAR, BIT(as));
> +
> +       mmu_hw_do_operation(pfdev, as, addr, PAGE_SIZE, AS_COMMAND_FLUSH_PT);
> +
> +       mutex_unlock(&pfdev->mmu->lock);
> +
> +       return 0;
> +}
> +
>  static const char *access_type_name(struct panfrost_device *pfdev,
>                 u32 fault_status)
>  {
> @@ -302,13 +358,11 @@ static irqreturn_t panfrost_mmu_irq_handler(int irq, void *data)
>  {
>         struct panfrost_device *pfdev = data;
>         u32 status = mmu_read(pfdev, MMU_INT_STAT);
> -       int i;
> +       int i, ret;
>
>         if (!status)
>                 return IRQ_NONE;
>
> -       dev_err(pfdev->dev, "mmu irq status=%x\n", status);
> -
>         for (i = 0; status; i++) {
>                 u32 mask = BIT(i) | BIT(i + 16);
>                 u64 addr;
> @@ -329,6 +383,17 @@ static irqreturn_t panfrost_mmu_irq_handler(int irq, void *data)
>                 access_type = (fault_status >> 8) & 0x3;
>                 source_id = (fault_status >> 16);
>
> +               /* Page fault only */
> +               if ((status & mask) == BIT(i)) {
> +                       WARN_ON(exception_type < 0xC1 || exception_type > 0xC4);
> +
> +                       ret = panfrost_mmu_map_fault_addr(pfdev, i, addr);
> +                       if (!ret) {
> +                               status &= ~mask;
> +                               continue;
> +                       }
> +               }
> +
>                 /* terminal fault, print info about the fault */
>                 dev_err(pfdev->dev,
>                         "Unhandled Page fault in AS%d at VA 0x%016llX\n"
> @@ -370,8 +435,9 @@ int panfrost_mmu_init(struct panfrost_device *pfdev)
>         if (irq <= 0)
>                 return -ENODEV;
>
> -       err = devm_request_irq(pfdev->dev, irq, panfrost_mmu_irq_handler,
> -                              IRQF_SHARED, "mmu", pfdev);
> +       err = devm_request_threaded_irq(pfdev->dev, irq, NULL,
> +                                       panfrost_mmu_irq_handler,
> +                                       IRQF_ONESHOT, "mmu", pfdev);
>
>         if (err) {
>                 dev_err(pfdev->dev, "failed to request mmu irq");
> diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
> index a52e0283b90d..6d83baa0e4c1 100644
> --- a/include/uapi/drm/panfrost_drm.h
> +++ b/include/uapi/drm/panfrost_drm.h
> @@ -71,6 +71,8 @@ struct drm_panfrost_wait_bo {
>         __s64 timeout_ns;       /* absolute */
>  };
>
> +#define PANFROST_BO_NOMAP      1
> +
>  /**
>   * struct drm_panfrost_create_bo - ioctl argument for creating Panfrost BOs.
>   *
> --
> 2.20.1
>


More information about the dri-devel mailing list