On Wed, 29 Jul 2020 at 06:33, Ruhl, Michael J <michael.j.r...@intel.com>
wrote:
> >-----Original Message-----
> >From: Ben Skeggs <skeg...@gmail.com>
> >Sent: Tuesday, July 28, 2020 4:16 PM
> >To: Ruhl, Michael J <michael.j.r...@intel.com>
> >Cc: Dave Airlie <airl...@gmail.com>; dri-devel@lists.freedesktop.org;
> >bske...@redhat.com
> >Subject: Re: [PATCH] nouveau: use ttm populate mapping functions. (v2)
> >
> >On Wed, 29 Jul 2020 at 02:08, Ruhl, Michael J <michael.j.r...@intel.com>
> >wrote:
> >>
> >> >-----Original Message-----
> >> >From: dri-devel <dri-devel-boun...@lists.freedesktop.org> On Behalf Of
> >> >Dave Airlie
> >> >Sent: Monday, July 27, 2020 11:26 PM
> >> >To: dri-devel@lists.freedesktop.org
> >> >Cc: bske...@redhat.com
> >> >Subject: [PATCH] nouveau: use ttm populate mapping functions. (v2)
> >> >
> >> >From: Dave Airlie <airl...@redhat.com>
> >> >
> >> >Instead of rolling driver copies of them.
> >> >
> >> >v2: cleanup return handling (Ben)
> >> >Signed-off-by: Dave Airlie <airl...@redhat.com>
> >> >---
> >> > drivers/gpu/drm/nouveau/nouveau_bo.c | 38 ++--------------------------
> >> > 1 file changed, 2 insertions(+), 36 deletions(-)
> >> >
> >> >diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> >> >b/drivers/gpu/drm/nouveau/nouveau_bo.c
> >> >index 7806278dce57..6ef5085c9a91 100644
> >> >--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> >> >+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> >> >@@ -1231,8 +1231,6 @@ nouveau_ttm_tt_populate(struct ttm_tt *ttm,
> >> >struct ttm_operation_ctx *ctx)
> >> > struct ttm_dma_tt *ttm_dma = (void *)ttm;
> >> > struct nouveau_drm *drm;
> >> > struct device *dev;
> >> >- unsigned i;
> >> >- int r;
> >> > bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
> >> >
> >> > if (ttm->state != tt_unpopulated)
> >> >@@ -1260,31 +1258,7 @@ nouveau_ttm_tt_populate(struct ttm_tt *ttm,
> >> >struct ttm_operation_ctx *ctx)
> >> > return ttm_dma_populate((void *)ttm, dev, ctx);
> >> > }
> >> > #endif
> >> >-
> >> >- r = ttm_pool_populate(ttm, ctx);
> >> >- if (r) {
> >> >- return r;
> >> >- }
> >> >-
> >> >- for (i = 0; i < ttm->num_pages; i++) {
> >> >- dma_addr_t addr;
> >> >-
> >> >- addr = dma_map_page(dev, ttm->pages[i], 0, PAGE_SIZE,
> >> >- DMA_BIDIRECTIONAL);
> >> >-
> >> >- if (dma_mapping_error(dev, addr)) {
> >> >- while (i--) {
> >> >- dma_unmap_page(dev, ttm_dma-
> >> >>dma_address[i],
> >> >- PAGE_SIZE,
> >> >DMA_BIDIRECTIONAL);
> >> >- ttm_dma->dma_address[i] = 0;
> >> >- }
> >> >- ttm_pool_unpopulate(ttm);
> >> >- return -EFAULT;
> >> >- }
> >> >-
> >> >- ttm_dma->dma_address[i] = addr;
> >> >- }
> >> >- return 0;
> >> >+ return ttm_populate_and_map_pages(dev, ttm_dma, ctx);
> >>
> >> This is not a completely straight code replacement.
> >>
> >> ttm_populate_and_map_pages() also has code to deal with pages that are
> >> contiguous (consolidates them).
> >>
> >> Is it possible that the nouveau HW can't handle a contiguous buffer
> larger
> >> than PAG_SIZE?
> >I think it's fine. The function appears to consolidate the pages for
> >the dma_map_page() call, but otherwise leave dma_address[] in
> >PAGE_SIZE chunks, I don't believe anything else in the driver will
> >care.
>
> Ahh.. I misread it. This is limiting the calls to dma_map_page().
> Instead
> of calling it for each page, just do it for the first one...
>
> Thanks for setting me straight. 😊
>
> Reviewed-by: Michael J. Ruhl <michael.j.r...@intel.com>
>
Thanks for the review! I've got the patch in my tree.
Ben.
>
> Mike
>
>
> >Ben.
> >
> >>
> >> Thanks,
> >>
> >> Mike
> >>
> >> > }
> >> >
> >> > static void
> >> >@@ -1293,7 +1267,6 @@ nouveau_ttm_tt_unpopulate(struct ttm_tt *ttm)
> >> > struct ttm_dma_tt *ttm_dma = (void *)ttm;
> >> > struct nouveau_drm *drm;
> >> > struct device *dev;
> >> >- unsigned i;
> >> > bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
> >> >
> >> > if (slave)
> >> >@@ -1316,14 +1289,7 @@ nouveau_ttm_tt_unpopulate(struct ttm_tt
> >*ttm)
> >> > }
> >> > #endif
> >> >
> >> >- for (i = 0; i < ttm->num_pages; i++) {
> >> >- if (ttm_dma->dma_address[i]) {
> >> >- dma_unmap_page(dev, ttm_dma->dma_address[i],
> >> >PAGE_SIZE,
> >> >- DMA_BIDIRECTIONAL);
> >> >- }
> >> >- }
> >> >-
> >> >- ttm_pool_unpopulate(ttm);
> >> >+ ttm_unmap_and_unpopulate_pages(dev, ttm_dma);
> >> > }
> >> >
> >> > void
> >> >--
> >> >2.26.2
> >> >
> >> >_______________________________________________
> >> >dri-devel mailing list
> >> >dri-devel@lists.freedesktop.org
> >> >https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
