Re: [Freedreno] [PATCH v2 4/4] drm/msm/a6xx: Use the DMA API for GMU memory objects
On Tue, Feb 25, 2020 at 3:54 PM John Stultz wrote:
>
> On Thu, Feb 20, 2020 at 10:27 AM Jordan Crouse wrote:
> >
> > The GMU has very few memory allocations and uses a flat memory space so
> > there is no good reason to go out of our way to bypass the DMA APIs which
> > were basically designed for this exact scenario.
> >
> > v2: Pass force_dma false to of_dma_configure to require that the DMA
> > region be set up and return error from of_dma_configure to fail probe.
> >
> > Signed-off-by: Jordan Crouse
> > ---
> >
> > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 112
> > +++---
> > drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 5 +-
> > 2 files changed, 11 insertions(+), 106 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > index 983afea..c36b38b 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> ...
> > - count = bo->size >> PAGE_SHIFT;
> > + bo->virt = dma_alloc_attrs(gmu->dev, bo->size, &bo->iova,
> > GFP_KERNEL,
> > + bo->attrs);
> >
> ...
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> > index 2af91ed..31bd1987 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> > @@ -13,7 +13,7 @@ struct a6xx_gmu_bo {
> > void *virt;
> > size_t size;
> > u64 iova;
> > - struct page **pages;
> > + unsigned long attrs;
> > };
>
> As a head up, Todd reported that this patch is causing build trouble
> w/ arm32, as the iova needs to be a dma_attr_t.
>
> I've got a patch for the android-mainline tree to fix this, but you
> might want to spin a v3 to address this.
> https://android-review.googlesource.com/c/kernel/common/+/1243928
>
I guess based on robher's comments on the bindings, there will be a v3..
BR,
-R
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v2 4/4] drm/msm/a6xx: Use the DMA API for GMU memory objects
On Thu, Feb 20, 2020 at 10:27 AM Jordan Crouse wrote:
>
> The GMU has very few memory allocations and uses a flat memory space so
> there is no good reason to go out of our way to bypass the DMA APIs which
> were basically designed for this exact scenario.
>
> v2: Pass force_dma false to of_dma_configure to require that the DMA
> region be set up and return error from of_dma_configure to fail probe.
>
> Signed-off-by: Jordan Crouse
> ---
>
> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 112
> +++---
> drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 5 +-
> 2 files changed, 11 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 983afea..c36b38b 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
...
> - count = bo->size >> PAGE_SHIFT;
> + bo->virt = dma_alloc_attrs(gmu->dev, bo->size, &bo->iova, GFP_KERNEL,
> + bo->attrs);
>
...
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> index 2af91ed..31bd1987 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> @@ -13,7 +13,7 @@ struct a6xx_gmu_bo {
> void *virt;
> size_t size;
> u64 iova;
> - struct page **pages;
> + unsigned long attrs;
> };
As a head up, Todd reported that this patch is causing build trouble
w/ arm32, as the iova needs to be a dma_attr_t.
I've got a patch for the android-mainline tree to fix this, but you
might want to spin a v3 to address this.
https://android-review.googlesource.com/c/kernel/common/+/1243928
thanks
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 4/4] drm/msm/a6xx: Use the DMA API for GMU memory objects
The GMU has very few memory allocations and uses a flat memory space so
there is no good reason to go out of our way to bypass the DMA APIs which
were basically designed for this exact scenario.
v2: Pass force_dma false to of_dma_configure to require that the DMA
region be set up and return error from of_dma_configure to fail probe.
Signed-off-by: Jordan Crouse
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 112 +++---
drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 5 +-
2 files changed, 11 insertions(+), 106 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 983afea..c36b38b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -2,6 +2,7 @@
/* Copyright (c) 2017-2019 The Linux Foundation. All rights reserved. */
#include
+#include
#include
#include
#include
@@ -895,21 +896,10 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
static void a6xx_gmu_memory_free(struct a6xx_gmu *gmu, struct a6xx_gmu_bo *bo)
{
- int count, i;
- u64 iova;
-
if (IS_ERR_OR_NULL(bo))
return;
- count = bo->size >> PAGE_SHIFT;
- iova = bo->iova;
-
- for (i = 0; i < count; i++, iova += PAGE_SIZE) {
- iommu_unmap(gmu->domain, iova, PAGE_SIZE);
- __free_pages(bo->pages[i], 0);
- }
-
- kfree(bo->pages);
+ dma_free_attrs(gmu->dev, bo->size, bo->virt, bo->iova, bo->attrs);
kfree(bo);
}
@@ -917,94 +907,23 @@ static struct a6xx_gmu_bo *a6xx_gmu_memory_alloc(struct
a6xx_gmu *gmu,
size_t size)
{
struct a6xx_gmu_bo *bo;
- int ret, count, i;
bo = kzalloc(sizeof(*bo), GFP_KERNEL);
if (!bo)
return ERR_PTR(-ENOMEM);
bo->size = PAGE_ALIGN(size);
+ bo->attrs = DMA_ATTR_WRITE_COMBINE;
- count = bo->size >> PAGE_SHIFT;
+ bo->virt = dma_alloc_attrs(gmu->dev, bo->size, &bo->iova, GFP_KERNEL,
+ bo->attrs);
- bo->pages = kcalloc(count, sizeof(struct page *), GFP_KERNEL);
- if (!bo->pages) {
+ if (!bo->virt) {
kfree(bo);
return ERR_PTR(-ENOMEM);
}
- for (i = 0; i < count; i++) {
- bo->pages[i] = alloc_page(GFP_KERNEL);
- if (!bo->pages[i])
- goto err;
- }
-
- bo->iova = gmu->uncached_iova_base;
-
- for (i = 0; i < count; i++) {
- ret = iommu_map(gmu->domain,
- bo->iova + (PAGE_SIZE * i),
- page_to_phys(bo->pages[i]), PAGE_SIZE,
- IOMMU_READ | IOMMU_WRITE);
-
- if (ret) {
- DRM_DEV_ERROR(gmu->dev, "Unable to map GMU buffer
object\n");
-
- for (i = i - 1 ; i >= 0; i--)
- iommu_unmap(gmu->domain,
- bo->iova + (PAGE_SIZE * i),
- PAGE_SIZE);
-
- goto err;
- }
- }
-
- bo->virt = vmap(bo->pages, count, VM_IOREMAP,
- pgprot_writecombine(PAGE_KERNEL));
- if (!bo->virt)
- goto err;
-
- /* Align future IOVA addresses on 1MB boundaries */
- gmu->uncached_iova_base += ALIGN(size, SZ_1M);
-
return bo;
-
-err:
- for (i = 0; i < count; i++) {
- if (bo->pages[i])
- __free_pages(bo->pages[i], 0);
- }
-
- kfree(bo->pages);
- kfree(bo);
-
- return ERR_PTR(-ENOMEM);
-}
-
-static int a6xx_gmu_memory_probe(struct a6xx_gmu *gmu)
-{
- int ret;
-
- /*
-* The GMU address space is hardcoded to treat the range
-* 0x6000 - 0x8000 as un-cached memory. All buffers shared
-* between the GMU and the CPU will live in this space
-*/
- gmu->uncached_iova_base = 0x6000;
-
-
- gmu->domain = iommu_domain_alloc(&platform_bus_type);
- if (!gmu->domain)
- return -ENODEV;
-
- ret = iommu_attach_device(gmu->domain, gmu->dev);
-
- if (ret) {
- iommu_domain_free(gmu->domain);
- gmu->domain = NULL;
- }
-
- return ret;
}
/* Return the 'arc-level' for the given frequency */
@@ -1264,10 +1183,6 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
a6xx_gmu_memory_free(gmu, gmu->hfi);
- iommu_detach_device(gmu->domain, gmu->dev);
-
- iommu_domain_free(gmu->domain);
-
free_irq(gmu->gmu_irq, gmu);
free_irq(gmu->hfi_irq, gmu);
@@ -1288,7 +1203,10 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct
device_node *node)
gmu->dev = &pdev->dev;
- of_dma_configure(gmu->dev, node, true);
+ /* Pass force_dma false to require the DT to set the dma region */
+ ret = of_dma_configure(gmu->dev, node, false);
+
