All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nouveau: use ttm populate mapping functions. (v2)
@ 2020-07-28  3:25 Dave Airlie
  2020-07-28 16:07 ` Ruhl, Michael J
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Airlie @ 2020-07-28  3:25 UTC (permalink / raw)
  To: dri-devel; +Cc: bskeggs

From: Dave Airlie <airlied@redhat.com>

Instead of rolling driver copies of them.

v2: cleanup return handling (Ben)
Signed-off-by: Dave Airlie <airlied@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);
 }
 
 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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [PATCH] nouveau: use ttm populate mapping functions. (v2)
  2020-07-28  3:25 [PATCH] nouveau: use ttm populate mapping functions. (v2) Dave Airlie
@ 2020-07-28 16:07 ` Ruhl, Michael J
  2020-07-28 20:15   ` Ben Skeggs
  0 siblings, 1 reply; 5+ messages in thread
From: Ruhl, Michael J @ 2020-07-28 16:07 UTC (permalink / raw)
  To: Dave Airlie, dri-devel; +Cc: bskeggs

>-----Original Message-----
>From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>Dave Airlie
>Sent: Monday, July 27, 2020 11:26 PM
>To: dri-devel@lists.freedesktop.org
>Cc: bskeggs@redhat.com
>Subject: [PATCH] nouveau: use ttm populate mapping functions. (v2)
>
>From: Dave Airlie <airlied@redhat.com>
>
>Instead of rolling driver copies of them.
>
>v2: cleanup return handling (Ben)
>Signed-off-by: Dave Airlie <airlied@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?

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] nouveau: use ttm populate mapping functions. (v2)
  2020-07-28 16:07 ` Ruhl, Michael J
@ 2020-07-28 20:15   ` Ben Skeggs
  2020-07-28 20:32     ` Ruhl, Michael J
  0 siblings, 1 reply; 5+ messages in thread
From: Ben Skeggs @ 2020-07-28 20:15 UTC (permalink / raw)
  To: Ruhl, Michael J; +Cc: bskeggs, dri-devel

On Wed, 29 Jul 2020 at 02:08, Ruhl, Michael J <michael.j.ruhl@intel.com> wrote:
>
> >-----Original Message-----
> >From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> >Dave Airlie
> >Sent: Monday, July 27, 2020 11:26 PM
> >To: dri-devel@lists.freedesktop.org
> >Cc: bskeggs@redhat.com
> >Subject: [PATCH] nouveau: use ttm populate mapping functions. (v2)
> >
> >From: Dave Airlie <airlied@redhat.com>
> >
> >Instead of rolling driver copies of them.
> >
> >v2: cleanup return handling (Ben)
> >Signed-off-by: Dave Airlie <airlied@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.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] nouveau: use ttm populate mapping functions. (v2)
  2020-07-28 20:15   ` Ben Skeggs
@ 2020-07-28 20:32     ` Ruhl, Michael J
  2020-07-29  6:00       ` Ben Skeggs
  0 siblings, 1 reply; 5+ messages in thread
From: Ruhl, Michael J @ 2020-07-28 20:32 UTC (permalink / raw)
  To: Ben Skeggs; +Cc: bskeggs, dri-devel

>-----Original Message-----
>From: Ben Skeggs <skeggsb@gmail.com>
>Sent: Tuesday, July 28, 2020 4:16 PM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>
>Cc: Dave Airlie <airlied@gmail.com>; dri-devel@lists.freedesktop.org;
>bskeggs@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.ruhl@intel.com>
>wrote:
>>
>> >-----Original Message-----
>> >From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>> >Dave Airlie
>> >Sent: Monday, July 27, 2020 11:26 PM
>> >To: dri-devel@lists.freedesktop.org
>> >Cc: bskeggs@redhat.com
>> >Subject: [PATCH] nouveau: use ttm populate mapping functions. (v2)
>> >
>> >From: Dave Airlie <airlied@redhat.com>
>> >
>> >Instead of rolling driver copies of them.
>> >
>> >v2: cleanup return handling (Ben)
>> >Signed-off-by: Dave Airlie <airlied@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.ruhl@intel.com>

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] nouveau: use ttm populate mapping functions. (v2)
  2020-07-28 20:32     ` Ruhl, Michael J
@ 2020-07-29  6:00       ` Ben Skeggs
  0 siblings, 0 replies; 5+ messages in thread
From: Ben Skeggs @ 2020-07-29  6:00 UTC (permalink / raw)
  To: Ruhl, Michael J; +Cc: bskeggs, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 5403 bytes --]

On Wed, 29 Jul 2020 at 06:33, Ruhl, Michael J <michael.j.ruhl@intel.com>
wrote:

> >-----Original Message-----
> >From: Ben Skeggs <skeggsb@gmail.com>
> >Sent: Tuesday, July 28, 2020 4:16 PM
> >To: Ruhl, Michael J <michael.j.ruhl@intel.com>
> >Cc: Dave Airlie <airlied@gmail.com>; dri-devel@lists.freedesktop.org;
> >bskeggs@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.ruhl@intel.com>
> >wrote:
> >>
> >> >-----Original Message-----
> >> >From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> >> >Dave Airlie
> >> >Sent: Monday, July 27, 2020 11:26 PM
> >> >To: dri-devel@lists.freedesktop.org
> >> >Cc: bskeggs@redhat.com
> >> >Subject: [PATCH] nouveau: use ttm populate mapping functions. (v2)
> >> >
> >> >From: Dave Airlie <airlied@redhat.com>
> >> >
> >> >Instead of rolling driver copies of them.
> >> >
> >> >v2: cleanup return handling (Ben)
> >> >Signed-off-by: Dave Airlie <airlied@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.ruhl@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
>

[-- Attachment #1.2: Type: text/html, Size: 8690 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-07-29  6:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-28  3:25 [PATCH] nouveau: use ttm populate mapping functions. (v2) Dave Airlie
2020-07-28 16:07 ` Ruhl, Michael J
2020-07-28 20:15   ` Ben Skeggs
2020-07-28 20:32     ` Ruhl, Michael J
2020-07-29  6:00       ` Ben Skeggs

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.