Re: [Intel-gfx] [PATCH v3 08/12] drm/ttm: Use drm_memcpy_from_wc_dbm for TTM bo moves

2021-05-26 Thread Christian König

Am 21.05.21 um 17:32 schrieb Thomas Hellström:

Use fast wc memcpy for reading out of wc memory for TTM bo moves.

Cc: Dave Airlie 
Cc: Christian König 
Cc: Daniel Vetter 
Signed-off-by: Thomas Hellström 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/ttm/ttm_bo_util.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 912cbe8e60a2..4a7d3d672f9a 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -31,6 +31,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -91,6 +92,7 @@ void ttm_move_memcpy(struct ttm_buffer_object *bo,
const struct ttm_kmap_iter_ops *src_ops = src_iter->ops;
struct ttm_tt *ttm = bo->ttm;
struct dma_buf_map src_map, dst_map;
+   bool wc_memcpy;
pgoff_t i;
  
  	/* Single TTM move. NOP */

@@ -114,11 +116,16 @@ void ttm_move_memcpy(struct ttm_buffer_object *bo,
return;
}
  
+	wc_memcpy = ((!src_ops->maps_tt || ttm->caching != ttm_cached) &&

+drm_has_memcpy_from_wc());
+
for (i = 0; i < dst_mem->num_pages; ++i) {
dst_ops->map_local(dst_iter, _map, i);
src_ops->map_local(src_iter, _map, i);
  
-		if (!src_map.is_iomem && !dst_map.is_iomem) {

+   if (wc_memcpy) {
+   drm_memcpy_from_wc_dbm(_map, _map, PAGE_SIZE);
+   } else if (!src_map.is_iomem && !dst_map.is_iomem) {
memcpy(dst_map.vaddr, src_map.vaddr, PAGE_SIZE);
} else if (!src_map.is_iomem) {
dma_buf_map_memcpy_to(_map, src_map.vaddr,


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 08/12] drm/ttm: Use drm_memcpy_from_wc_dbm for TTM bo moves

2021-05-24 Thread Thomas Hellström
On Mon, 2021-05-24 at 19:16 +0100, Matthew Auld wrote:
> On Fri, 21 May 2021 at 16:33, Thomas Hellström
>  wrote:
> > 
> > Use fast wc memcpy for reading out of wc memory for TTM bo moves.
> > 
> > Cc: Dave Airlie 
> > Cc: Christian König 
> > Cc: Daniel Vetter 
> > Signed-off-by: Thomas Hellström 
> > ---
> >  drivers/gpu/drm/ttm/ttm_bo_util.c | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> > b/drivers/gpu/drm/ttm/ttm_bo_util.c
> > index 912cbe8e60a2..4a7d3d672f9a 100644
> > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> > @@ -31,6 +31,7 @@
> > 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -91,6 +92,7 @@ void ttm_move_memcpy(struct ttm_buffer_object
> > *bo,
> >     const struct ttm_kmap_iter_ops *src_ops = src_iter->ops;
> >     struct ttm_tt *ttm = bo->ttm;
> >     struct dma_buf_map src_map, dst_map;
> > +   bool wc_memcpy;
> >     pgoff_t i;
> > 
> >     /* Single TTM move. NOP */
> > @@ -114,11 +116,16 @@ void ttm_move_memcpy(struct ttm_buffer_object
> > *bo,
> >     return;
> >     }
> > 
> > +   wc_memcpy = ((!src_ops->maps_tt || ttm->caching !=
> > ttm_cached) &&
> 
> Why do we only consider the caching value for the maps_tt case? Or am
> I misreading this?

Hmm, you're right we should probably check also the iomem caching or
ignore the tt caching. Sometimes these special memcpys tend to be less
cpu cache thrashing and should be used wherever possible, but I guess
we should start out with only using it when source is WC.

> 
> > +    drm_has_memcpy_from_wc());
> > +
> >     for (i = 0; i < dst_mem->num_pages; ++i) {
> >     dst_ops->map_local(dst_iter, _map, i);
> >     src_ops->map_local(src_iter, _map, i);
> > 
> > -   if (!src_map.is_iomem && !dst_map.is_iomem) {
> > +   if (wc_memcpy) {
> > +   drm_memcpy_from_wc_dbm(_map, _map,
> > PAGE_SIZE);
> 
> Do we need to check the return value here? memcpy_from_wc expects
> certain address alignment, or is that always guaranteed here? Maybe
> throw a warning just for paranoia?

It should always be PAGE_SIZE aligned. But I guess it doesn't hurt to
do 
if (wc_memcpy && drm_memcpy_from_wc_dbm())
;

> 
> > +   } else if (!src_map.is_iomem && !dst_map.is_iomem)
> > {
> >     memcpy(dst_map.vaddr, src_map.vaddr,
> > PAGE_SIZE);
> >     } else if (!src_map.is_iomem) {
> >     dma_buf_map_memcpy_to(_map,
> > src_map.vaddr,
> > --
> > 2.31.1
> > 


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 08/12] drm/ttm: Use drm_memcpy_from_wc_dbm for TTM bo moves

2021-05-24 Thread Matthew Auld
On Fri, 21 May 2021 at 16:33, Thomas Hellström
 wrote:
>
> Use fast wc memcpy for reading out of wc memory for TTM bo moves.
>
> Cc: Dave Airlie 
> Cc: Christian König 
> Cc: Daniel Vetter 
> Signed-off-by: Thomas Hellström 
> ---
>  drivers/gpu/drm/ttm/ttm_bo_util.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 912cbe8e60a2..4a7d3d672f9a 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -31,6 +31,7 @@
>
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -91,6 +92,7 @@ void ttm_move_memcpy(struct ttm_buffer_object *bo,
> const struct ttm_kmap_iter_ops *src_ops = src_iter->ops;
> struct ttm_tt *ttm = bo->ttm;
> struct dma_buf_map src_map, dst_map;
> +   bool wc_memcpy;
> pgoff_t i;
>
> /* Single TTM move. NOP */
> @@ -114,11 +116,16 @@ void ttm_move_memcpy(struct ttm_buffer_object *bo,
> return;
> }
>
> +   wc_memcpy = ((!src_ops->maps_tt || ttm->caching != ttm_cached) &&

Why do we only consider the caching value for the maps_tt case? Or am
I misreading this?

> +drm_has_memcpy_from_wc());
> +
> for (i = 0; i < dst_mem->num_pages; ++i) {
> dst_ops->map_local(dst_iter, _map, i);
> src_ops->map_local(src_iter, _map, i);
>
> -   if (!src_map.is_iomem && !dst_map.is_iomem) {
> +   if (wc_memcpy) {
> +   drm_memcpy_from_wc_dbm(_map, _map, PAGE_SIZE);

Do we need to check the return value here? memcpy_from_wc expects
certain address alignment, or is that always guaranteed here? Maybe
throw a warning just for paranoia?

> +   } else if (!src_map.is_iomem && !dst_map.is_iomem) {
> memcpy(dst_map.vaddr, src_map.vaddr, PAGE_SIZE);
> } else if (!src_map.is_iomem) {
> dma_buf_map_memcpy_to(_map, src_map.vaddr,
> --
> 2.31.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v3 08/12] drm/ttm: Use drm_memcpy_from_wc_dbm for TTM bo moves

2021-05-21 Thread Thomas Hellström
Use fast wc memcpy for reading out of wc memory for TTM bo moves.

Cc: Dave Airlie 
Cc: Christian König 
Cc: Daniel Vetter 
Signed-off-by: Thomas Hellström 
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 912cbe8e60a2..4a7d3d672f9a 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -31,6 +31,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -91,6 +92,7 @@ void ttm_move_memcpy(struct ttm_buffer_object *bo,
const struct ttm_kmap_iter_ops *src_ops = src_iter->ops;
struct ttm_tt *ttm = bo->ttm;
struct dma_buf_map src_map, dst_map;
+   bool wc_memcpy;
pgoff_t i;
 
/* Single TTM move. NOP */
@@ -114,11 +116,16 @@ void ttm_move_memcpy(struct ttm_buffer_object *bo,
return;
}
 
+   wc_memcpy = ((!src_ops->maps_tt || ttm->caching != ttm_cached) &&
+drm_has_memcpy_from_wc());
+
for (i = 0; i < dst_mem->num_pages; ++i) {
dst_ops->map_local(dst_iter, _map, i);
src_ops->map_local(src_iter, _map, i);
 
-   if (!src_map.is_iomem && !dst_map.is_iomem) {
+   if (wc_memcpy) {
+   drm_memcpy_from_wc_dbm(_map, _map, PAGE_SIZE);
+   } else if (!src_map.is_iomem && !dst_map.is_iomem) {
memcpy(dst_map.vaddr, src_map.vaddr, PAGE_SIZE);
} else if (!src_map.is_iomem) {
dma_buf_map_memcpy_to(_map, src_map.vaddr,
-- 
2.31.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx