[PATCH] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[PATCH] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Liviu Dudau]

drm_gem_cma_prime_import_sg_table() will fail if the number of entries
in the sg_table > 1. However, you can have a device that uses an IOMMU
engine and can map a discontiguous buffer with multiple entries that
have consecutive sg_dma_addresses, effectively making it contiguous.
Allow for that scenario by testing the entries in the sg_table for
contiguous coverage.

Reviewed-by: Brian Starkey <brian.starkey@arm.com>
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---

Hi,

This patch is the only change I need in order to be able to use existing
IOMMU domain infrastructure with the Mali DP driver. I have tested the
patch and I know it works correctly for my setup, but I would like to get
some comments on whether I am on the right path or if CMA really wants to
see an sg_table with only one entry.

Best regards,
Liviu

 drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 020e7668dfaba..43b179212052d 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device *dev,
 {
 	struct drm_gem_cma_object *cma_obj;
 
-	if (sgt->nents != 1)
-		return ERR_PTR(-EINVAL);
+	if (sgt->nents != 1) {
+		/* check if the entries in the sg_table are contiguous */
+		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
+		struct scatterlist *s;
+		unsigned int i;
+
+		for_each_sg(sgt->sgl, s, sgt->nents, i) {
+			/*
+			 * sg_dma_address(s) is only valid for entries
+			 * that have sg_dma_len(s) != 0
+			 */
+			if (!sg_dma_len(s))
+				continue;
+
+			if (sg_dma_address(s) != next_addr)
+				return ERR_PTR(-EINVAL);
+
+			next_addr = sg_dma_address(s) + sg_dma_len(s);
+		}
+	}
 
 	/* Create a CMA GEM buffer. */
 	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
-- 
2.14.3

[PATCH] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Liviu Dudau]

drm_gem_cma_prime_import_sg_table() will fail if the number of entries
in the sg_table > 1. However, you can have a device that uses an IOMMU
engine and can map a discontiguous buffer with multiple entries that
have consecutive sg_dma_addresses, effectively making it contiguous.
Allow for that scenario by testing the entries in the sg_table for
contiguous coverage.

Reviewed-by: Brian Starkey <brian.starkey@arm.com>
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---

Hi,

This patch is the only change I need in order to be able to use existing
IOMMU domain infrastructure with the Mali DP driver. I have tested the
patch and I know it works correctly for my setup, but I would like to get
some comments on whether I am on the right path or if CMA really wants to
see an sg_table with only one entry.

Best regards,
Liviu

 drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 020e7668dfaba..43b179212052d 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device *dev,
 {
 	struct drm_gem_cma_object *cma_obj;
 
-	if (sgt->nents != 1)
-		return ERR_PTR(-EINVAL);
+	if (sgt->nents != 1) {
+		/* check if the entries in the sg_table are contiguous */
+		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
+		struct scatterlist *s;
+		unsigned int i;
+
+		for_each_sg(sgt->sgl, s, sgt->nents, i) {
+			/*
+			 * sg_dma_address(s) is only valid for entries
+			 * that have sg_dma_len(s) != 0
+			 */
+			if (!sg_dma_len(s))
+				continue;
+
+			if (sg_dma_address(s) != next_addr)
+				return ERR_PTR(-EINVAL);
+
+			next_addr = sg_dma_address(s) + sg_dma_len(s);
+		}
+	}
 
 	/* Create a CMA GEM buffer. */
 	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
-- 
2.14.3

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

Re: [PATCH] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Laurent Pinchart]

Hi Liviu,

Thank you for the patch.

On Wednesday, 1 November 2017 16:14:19 EET Liviu Dudau wrote:
> drm_gem_cma_prime_import_sg_table() will fail if the number of entries
> in the sg_table > 1. However, you can have a device that uses an IOMMU
> engine and can map a discontiguous buffer with multiple entries that
> have consecutive sg_dma_addresses, effectively making it contiguous.
> Allow for that scenario by testing the entries in the sg_table for
> contiguous coverage.
> 
> Reviewed-by: Brian Starkey <brian.starkey@arm.com>
> Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> ---
> 
> Hi,
> 
> This patch is the only change I need in order to be able to use existing
> IOMMU domain infrastructure with the Mali DP driver. I have tested the
> patch and I know it works correctly for my setup, but I would like to get
> some comments on whether I am on the right path or if CMA really wants to
> see an sg_table with only one entry.

CMA, as the memory allocator, doesn't care as it doesn't even see the sg 
table. The drm_gem_cma_helper is badly named as it doesn't depend on CMA, it 
should have been called drm_gem_dma_contig_helper or something similar.

The assumption at the base of that helper library is that the memory is DMA 
contiguous. Your patch guarantees that, so it should be fine. I've quickly 
checked the drivers using drm_gem_cma_prime_import_sg_table and none of them 
use cma_obj->sgt, so I think there's no risk of breakage. However, I would 
prefer if you updated the drm_gem_cma_object structure documentation to 
explicitly state that the sgt can contain multiple entries but that those 
entries are guaranteed to have contiguous DMA addresses.

With the documentation update,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c index 020e7668dfaba..43b179212052d
> 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device
> *dev, {
>  	struct drm_gem_cma_object *cma_obj;
> 
> -	if (sgt->nents != 1)
> -		return ERR_PTR(-EINVAL);
> +	if (sgt->nents != 1) {
> +		/* check if the entries in the sg_table are contiguous */
> +		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
> +		struct scatterlist *s;
> +		unsigned int i;
> +
> +		for_each_sg(sgt->sgl, s, sgt->nents, i) {
> +			/*
> +			 * sg_dma_address(s) is only valid for entries
> +			 * that have sg_dma_len(s) != 0
> +			 */
> +			if (!sg_dma_len(s))
> +				continue;
> +
> +			if (sg_dma_address(s) != next_addr)
> +				return ERR_PTR(-EINVAL);
> +
> +			next_addr = sg_dma_address(s) + sg_dma_len(s);
> +		}
> +	}
> 
>  	/* Create a CMA GEM buffer. */
>  	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Laurent Pinchart]

Hi Liviu,

Thank you for the patch.

On Wednesday, 1 November 2017 16:14:19 EET Liviu Dudau wrote:
> drm_gem_cma_prime_import_sg_table() will fail if the number of entries
> in the sg_table > 1. However, you can have a device that uses an IOMMU
> engine and can map a discontiguous buffer with multiple entries that
> have consecutive sg_dma_addresses, effectively making it contiguous.
> Allow for that scenario by testing the entries in the sg_table for
> contiguous coverage.
> 
> Reviewed-by: Brian Starkey <brian.starkey@arm.com>
> Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> ---
> 
> Hi,
> 
> This patch is the only change I need in order to be able to use existing
> IOMMU domain infrastructure with the Mali DP driver. I have tested the
> patch and I know it works correctly for my setup, but I would like to get
> some comments on whether I am on the right path or if CMA really wants to
> see an sg_table with only one entry.

CMA, as the memory allocator, doesn't care as it doesn't even see the sg 
table. The drm_gem_cma_helper is badly named as it doesn't depend on CMA, it 
should have been called drm_gem_dma_contig_helper or something similar.

The assumption at the base of that helper library is that the memory is DMA 
contiguous. Your patch guarantees that, so it should be fine. I've quickly 
checked the drivers using drm_gem_cma_prime_import_sg_table and none of them 
use cma_obj->sgt, so I think there's no risk of breakage. However, I would 
prefer if you updated the drm_gem_cma_object structure documentation to 
explicitly state that the sgt can contain multiple entries but that those 
entries are guaranteed to have contiguous DMA addresses.

With the documentation update,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c index 020e7668dfaba..43b179212052d
> 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device
> *dev, {
>  	struct drm_gem_cma_object *cma_obj;
> 
> -	if (sgt->nents != 1)
> -		return ERR_PTR(-EINVAL);
> +	if (sgt->nents != 1) {
> +		/* check if the entries in the sg_table are contiguous */
> +		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
> +		struct scatterlist *s;
> +		unsigned int i;
> +
> +		for_each_sg(sgt->sgl, s, sgt->nents, i) {
> +			/*
> +			 * sg_dma_address(s) is only valid for entries
> +			 * that have sg_dma_len(s) != 0
> +			 */
> +			if (!sg_dma_len(s))
> +				continue;
> +
> +			if (sg_dma_address(s) != next_addr)
> +				return ERR_PTR(-EINVAL);
> +
> +			next_addr = sg_dma_address(s) + sg_dma_len(s);
> +		}
> +	}
> 
>  	/* Create a CMA GEM buffer. */
>  	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);

-- 
Regards,

Laurent Pinchart

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

[PATCH v2] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Liviu Dudau]

drm_gem_cma_prime_import_sg_table() will fail if the number of entries
in the sg_table > 1. However, you can have a device that uses an IOMMU
engine and can map a discontiguous buffer with multiple entries that
have consecutive sg_dma_addresses, effectively making it contiguous.
Allow for that scenario by testing the entries in the sg_table for
contiguous coverage.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---

Laurent,

Thanks for the review! I would like to ask for one more favour: if you
are OK with this version, can you pull this patch through the drm-misc tree?

Many thanks,
Liviu

 drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
 include/drm/drm_gem_cma_helper.h     |  4 +++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 020e7668dfaba..43b179212052d 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device *dev,
 {
 	struct drm_gem_cma_object *cma_obj;
 
-	if (sgt->nents != 1)
-		return ERR_PTR(-EINVAL);
+	if (sgt->nents != 1) {
+		/* check if the entries in the sg_table are contiguous */
+		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
+		struct scatterlist *s;
+		unsigned int i;
+
+		for_each_sg(sgt->sgl, s, sgt->nents, i) {
+			/*
+			 * sg_dma_address(s) is only valid for entries
+			 * that have sg_dma_len(s) != 0
+			 */
+			if (!sg_dma_len(s))
+				continue;
+
+			if (sg_dma_address(s) != next_addr)
+				return ERR_PTR(-EINVAL);
+
+			next_addr = sg_dma_address(s) + sg_dma_len(s);
+		}
+	}
 
 	/* Create a CMA GEM buffer. */
 	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
index 58a739bf15f1f..214aa85adc8d5 100644
--- a/include/drm/drm_gem_cma_helper.h
+++ b/include/drm/drm_gem_cma_helper.h
@@ -8,7 +8,9 @@
  * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
  * @base: base GEM object
  * @paddr: physical address of the backing memory
- * @sgt: scatter/gather table for imported PRIME buffers
+ * @sgt: scatter/gather table for imported PRIME buffers. The table can have
+ *       more than one entry but they are guaranteed to have contiguous
+ *       DMA addresses.
  * @vaddr: kernel virtual address of the backing memory
  */
 struct drm_gem_cma_object {
-- 
2.14.3

[PATCH v2] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Liviu Dudau]

drm_gem_cma_prime_import_sg_table() will fail if the number of entries
in the sg_table > 1. However, you can have a device that uses an IOMMU
engine and can map a discontiguous buffer with multiple entries that
have consecutive sg_dma_addresses, effectively making it contiguous.
Allow for that scenario by testing the entries in the sg_table for
contiguous coverage.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
---

Laurent,

Thanks for the review! I would like to ask for one more favour: if you
are OK with this version, can you pull this patch through the drm-misc tree?

Many thanks,
Liviu

 drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
 include/drm/drm_gem_cma_helper.h     |  4 +++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 020e7668dfaba..43b179212052d 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device *dev,
 {
 	struct drm_gem_cma_object *cma_obj;
 
-	if (sgt->nents != 1)
-		return ERR_PTR(-EINVAL);
+	if (sgt->nents != 1) {
+		/* check if the entries in the sg_table are contiguous */
+		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
+		struct scatterlist *s;
+		unsigned int i;
+
+		for_each_sg(sgt->sgl, s, sgt->nents, i) {
+			/*
+			 * sg_dma_address(s) is only valid for entries
+			 * that have sg_dma_len(s) != 0
+			 */
+			if (!sg_dma_len(s))
+				continue;
+
+			if (sg_dma_address(s) != next_addr)
+				return ERR_PTR(-EINVAL);
+
+			next_addr = sg_dma_address(s) + sg_dma_len(s);
+		}
+	}
 
 	/* Create a CMA GEM buffer. */
 	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
index 58a739bf15f1f..214aa85adc8d5 100644
--- a/include/drm/drm_gem_cma_helper.h
+++ b/include/drm/drm_gem_cma_helper.h
@@ -8,7 +8,9 @@
  * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
  * @base: base GEM object
  * @paddr: physical address of the backing memory
- * @sgt: scatter/gather table for imported PRIME buffers
+ * @sgt: scatter/gather table for imported PRIME buffers. The table can have
+ *       more than one entry but they are guaranteed to have contiguous
+ *       DMA addresses.
  * @vaddr: kernel virtual address of the backing memory
  */
 struct drm_gem_cma_object {
-- 
2.14.3

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

✓ Fi.CI.BAT: success for drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Patchwork]

== Series Details ==

Series: drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1
URL   : https://patchwork.freedesktop.org/series/33602/
State : success

== Summary ==

Series 33602v1 drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1
https://patchwork.freedesktop.org/api/1.0/series/33602/revisions/1/mbox/

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:444s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:458s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:382s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:535s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:274s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:494s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:501s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:498s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:489s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:425s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:265s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:430s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:442s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:424s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:476s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:460s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:483s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:522s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:478s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:535s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:572s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:443s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:543s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:565s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:528s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:496s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:458s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:555s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:411s
Blacklisted hosts:
fi-cfl-s         total:289  pass:254  dwarn:3   dfail:0   fail:0   skip:32  time:536s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:552s
fi-glk-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:493s

c36994b067f0771eee4d3136f40c0c6040a8fa77 drm-tip: 2017y-11m-10d-11h-48m-19s UTC integration manifest
b0d0eb465a6f drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7061/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Patchwork]

== Series Details ==

Series: drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1
URL   : https://patchwork.freedesktop.org/series/33602/
State : success

== Summary ==

Test kms_busy:
        Subgroup extended-modeset-hang-oldfb-with-reset-render-c:
                dmesg-warn -> PASS       (shard-hsw) fdo#102249
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-hsw) fdo#100047

fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-hsw        total:2584 pass:1452 dwarn:1   dfail:2   fail:10  skip:1118 time:9292s
Blacklisted hosts:
shard-apl        total:2584 pass:1603 dwarn:3   dfail:2   fail:21  skip:955 time:12880s
shard-kbl        total:2462 pass:1602 dwarn:22  dfail:6   fail:25  skip:804 time:9310s
shard-snb        total:2584 pass:1199 dwarn:1   dfail:2   fail:11  skip:1371 time:7791s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7061/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Laurent Pinchart]

Hi Liviu,

Thank you for the patch.

On Friday, 10 November 2017 15:33:10 EET Liviu Dudau wrote:
> drm_gem_cma_prime_import_sg_table() will fail if the number of entries
> in the sg_table > 1. However, you can have a device that uses an IOMMU
> engine and can map a discontiguous buffer with multiple entries that
> have consecutive sg_dma_addresses, effectively making it contiguous.
> Allow for that scenario by testing the entries in the sg_table for
> contiguous coverage.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> ---
> 
> Laurent,
> 
> Thanks for the review! I would like to ask for one more favour: if you
> are OK with this version, can you pull this patch through the drm-misc tree?

I could, but I'd first need to set dim up, and I'm currently abroad with a bad 
internet connection and a big deadline for the middle of next week (I know, 
lots of excuses), so it's not very convenient for me at this time.

>  drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
>  include/drm/drm_gem_cma_helper.h     |  4 +++-
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c index 020e7668dfaba..43b179212052d
> 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device
> *dev, {
>  	struct drm_gem_cma_object *cma_obj;
> 
> -	if (sgt->nents != 1)
> -		return ERR_PTR(-EINVAL);
> +	if (sgt->nents != 1) {
> +		/* check if the entries in the sg_table are contiguous */
> +		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
> +		struct scatterlist *s;
> +		unsigned int i;
> +
> +		for_each_sg(sgt->sgl, s, sgt->nents, i) {
> +			/*
> +			 * sg_dma_address(s) is only valid for entries
> +			 * that have sg_dma_len(s) != 0
> +			 */
> +			if (!sg_dma_len(s))
> +				continue;
> +
> +			if (sg_dma_address(s) != next_addr)
> +				return ERR_PTR(-EINVAL);
> +
> +			next_addr = sg_dma_address(s) + sg_dma_len(s);
> +		}
> +	}
> 
>  	/* Create a CMA GEM buffer. */
>  	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
> diff --git a/include/drm/drm_gem_cma_helper.h
> b/include/drm/drm_gem_cma_helper.h index 58a739bf15f1f..214aa85adc8d5
> 100644
> --- a/include/drm/drm_gem_cma_helper.h
> +++ b/include/drm/drm_gem_cma_helper.h
> @@ -8,7 +8,9 @@
>   * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
>   * @base: base GEM object
>   * @paddr: physical address of the backing memory
> - * @sgt: scatter/gather table for imported PRIME buffers
> + * @sgt: scatter/gather table for imported PRIME buffers. The table can
> have + *       more than one entry but they are guaranteed to have
> contiguous + *       DMA addresses.
>   * @vaddr: kernel virtual address of the backing memory
>   */
>  struct drm_gem_cma_object {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Laurent Pinchart]

Hi Liviu,

Thank you for the patch.

On Friday, 10 November 2017 15:33:10 EET Liviu Dudau wrote:
> drm_gem_cma_prime_import_sg_table() will fail if the number of entries
> in the sg_table > 1. However, you can have a device that uses an IOMMU
> engine and can map a discontiguous buffer with multiple entries that
> have consecutive sg_dma_addresses, effectively making it contiguous.
> Allow for that scenario by testing the entries in the sg_table for
> contiguous coverage.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> ---
> 
> Laurent,
> 
> Thanks for the review! I would like to ask for one more favour: if you
> are OK with this version, can you pull this patch through the drm-misc tree?

I could, but I'd first need to set dim up, and I'm currently abroad with a bad 
internet connection and a big deadline for the middle of next week (I know, 
lots of excuses), so it's not very convenient for me at this time.

>  drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
>  include/drm/drm_gem_cma_helper.h     |  4 +++-
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c index 020e7668dfaba..43b179212052d
> 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device
> *dev, {
>  	struct drm_gem_cma_object *cma_obj;
> 
> -	if (sgt->nents != 1)
> -		return ERR_PTR(-EINVAL);
> +	if (sgt->nents != 1) {
> +		/* check if the entries in the sg_table are contiguous */
> +		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
> +		struct scatterlist *s;
> +		unsigned int i;
> +
> +		for_each_sg(sgt->sgl, s, sgt->nents, i) {
> +			/*
> +			 * sg_dma_address(s) is only valid for entries
> +			 * that have sg_dma_len(s) != 0
> +			 */
> +			if (!sg_dma_len(s))
> +				continue;
> +
> +			if (sg_dma_address(s) != next_addr)
> +				return ERR_PTR(-EINVAL);
> +
> +			next_addr = sg_dma_address(s) + sg_dma_len(s);
> +		}
> +	}
> 
>  	/* Create a CMA GEM buffer. */
>  	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
> diff --git a/include/drm/drm_gem_cma_helper.h
> b/include/drm/drm_gem_cma_helper.h index 58a739bf15f1f..214aa85adc8d5
> 100644
> --- a/include/drm/drm_gem_cma_helper.h
> +++ b/include/drm/drm_gem_cma_helper.h
> @@ -8,7 +8,9 @@
>   * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
>   * @base: base GEM object
>   * @paddr: physical address of the backing memory
> - * @sgt: scatter/gather table for imported PRIME buffers
> + * @sgt: scatter/gather table for imported PRIME buffers. The table can
> have + *       more than one entry but they are guaranteed to have
> contiguous + *       DMA addresses.
>   * @vaddr: kernel virtual address of the backing memory
>   */
>  struct drm_gem_cma_object {

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v2] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Liviu Dudau]

Hi,

On Sat, Nov 11, 2017 at 02:47:35PM +0200, Laurent Pinchart wrote:
> Hi Liviu,
> 
> Thank you for the patch.
> 
> On Friday, 10 November 2017 15:33:10 EET Liviu Dudau wrote:
> > drm_gem_cma_prime_import_sg_table() will fail if the number of entries
> > in the sg_table > 1. However, you can have a device that uses an IOMMU
> > engine and can map a discontiguous buffer with multiple entries that
> > have consecutive sg_dma_addresses, effectively making it contiguous.
> > Allow for that scenario by testing the entries in the sg_table for
> > contiguous coverage.
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> > ---
> > 
> > Laurent,
> > 
> > Thanks for the review! I would like to ask for one more favour: if you
> > are OK with this version, can you pull this patch through the drm-misc tree?
> 
> I could, but I'd first need to set dim up, and I'm currently abroad with a bad 
> internet connection and a big deadline for the middle of next week (I know, 
> lots of excuses), so it's not very convenient for me at this time.

Any other drm-misc maintainers feeling helpful and willing to take this
patch in? Otherwise I can send it through the mali-dp tree if no one
objects.

Best regards,
Liviu

> 
> >  drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
> >  include/drm/drm_gem_cma_helper.h     |  4 +++-
> >  2 files changed, 23 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> > b/drivers/gpu/drm/drm_gem_cma_helper.c index 020e7668dfaba..43b179212052d
> > 100644
> > --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> > @@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device
> > *dev, {
> >  	struct drm_gem_cma_object *cma_obj;
> > 
> > -	if (sgt->nents != 1)
> > -		return ERR_PTR(-EINVAL);
> > +	if (sgt->nents != 1) {
> > +		/* check if the entries in the sg_table are contiguous */
> > +		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
> > +		struct scatterlist *s;
> > +		unsigned int i;
> > +
> > +		for_each_sg(sgt->sgl, s, sgt->nents, i) {
> > +			/*
> > +			 * sg_dma_address(s) is only valid for entries
> > +			 * that have sg_dma_len(s) != 0
> > +			 */
> > +			if (!sg_dma_len(s))
> > +				continue;
> > +
> > +			if (sg_dma_address(s) != next_addr)
> > +				return ERR_PTR(-EINVAL);
> > +
> > +			next_addr = sg_dma_address(s) + sg_dma_len(s);
> > +		}
> > +	}
> > 
> >  	/* Create a CMA GEM buffer. */
> >  	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
> > diff --git a/include/drm/drm_gem_cma_helper.h
> > b/include/drm/drm_gem_cma_helper.h index 58a739bf15f1f..214aa85adc8d5
> > 100644
> > --- a/include/drm/drm_gem_cma_helper.h
> > +++ b/include/drm/drm_gem_cma_helper.h
> > @@ -8,7 +8,9 @@
> >   * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
> >   * @base: base GEM object
> >   * @paddr: physical address of the backing memory
> > - * @sgt: scatter/gather table for imported PRIME buffers
> > + * @sgt: scatter/gather table for imported PRIME buffers. The table can
> > have + *       more than one entry but they are guaranteed to have
> > contiguous + *       DMA addresses.
> >   * @vaddr: kernel virtual address of the backing memory
> >   */
> >  struct drm_gem_cma_object {
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v2] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Liviu Dudau]

Hi,

On Sat, Nov 11, 2017 at 02:47:35PM +0200, Laurent Pinchart wrote:
> Hi Liviu,
> 
> Thank you for the patch.
> 
> On Friday, 10 November 2017 15:33:10 EET Liviu Dudau wrote:
> > drm_gem_cma_prime_import_sg_table() will fail if the number of entries
> > in the sg_table > 1. However, you can have a device that uses an IOMMU
> > engine and can map a discontiguous buffer with multiple entries that
> > have consecutive sg_dma_addresses, effectively making it contiguous.
> > Allow for that scenario by testing the entries in the sg_table for
> > contiguous coverage.
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> > ---
> > 
> > Laurent,
> > 
> > Thanks for the review! I would like to ask for one more favour: if you
> > are OK with this version, can you pull this patch through the drm-misc tree?
> 
> I could, but I'd first need to set dim up, and I'm currently abroad with a bad 
> internet connection and a big deadline for the middle of next week (I know, 
> lots of excuses), so it's not very convenient for me at this time.

Any other drm-misc maintainers feeling helpful and willing to take this
patch in? Otherwise I can send it through the mali-dp tree if no one
objects.

Best regards,
Liviu

> 
> >  drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
> >  include/drm/drm_gem_cma_helper.h     |  4 +++-
> >  2 files changed, 23 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> > b/drivers/gpu/drm/drm_gem_cma_helper.c index 020e7668dfaba..43b179212052d
> > 100644
> > --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> > @@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device
> > *dev, {
> >  	struct drm_gem_cma_object *cma_obj;
> > 
> > -	if (sgt->nents != 1)
> > -		return ERR_PTR(-EINVAL);
> > +	if (sgt->nents != 1) {
> > +		/* check if the entries in the sg_table are contiguous */
> > +		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
> > +		struct scatterlist *s;
> > +		unsigned int i;
> > +
> > +		for_each_sg(sgt->sgl, s, sgt->nents, i) {
> > +			/*
> > +			 * sg_dma_address(s) is only valid for entries
> > +			 * that have sg_dma_len(s) != 0
> > +			 */
> > +			if (!sg_dma_len(s))
> > +				continue;
> > +
> > +			if (sg_dma_address(s) != next_addr)
> > +				return ERR_PTR(-EINVAL);
> > +
> > +			next_addr = sg_dma_address(s) + sg_dma_len(s);
> > +		}
> > +	}
> > 
> >  	/* Create a CMA GEM buffer. */
> >  	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
> > diff --git a/include/drm/drm_gem_cma_helper.h
> > b/include/drm/drm_gem_cma_helper.h index 58a739bf15f1f..214aa85adc8d5
> > 100644
> > --- a/include/drm/drm_gem_cma_helper.h
> > +++ b/include/drm/drm_gem_cma_helper.h
> > @@ -8,7 +8,9 @@
> >   * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
> >   * @base: base GEM object
> >   * @paddr: physical address of the backing memory
> > - * @sgt: scatter/gather table for imported PRIME buffers
> > + * @sgt: scatter/gather table for imported PRIME buffers. The table can
> > have + *       more than one entry but they are guaranteed to have
> > contiguous + *       DMA addresses.
> >   * @vaddr: kernel virtual address of the backing memory
> >   */
> >  struct drm_gem_cma_object {
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Noralf Trønnes]

Den 15.11.2017 14.04, skrev Liviu Dudau:
> Hi,
>
> On Sat, Nov 11, 2017 at 02:47:35PM +0200, Laurent Pinchart wrote:
>> Hi Liviu,
>>
>> Thank you for the patch.
>>
>> On Friday, 10 November 2017 15:33:10 EET Liviu Dudau wrote:
>>> drm_gem_cma_prime_import_sg_table() will fail if the number of entries
>>> in the sg_table > 1. However, you can have a device that uses an IOMMU
>>> engine and can map a discontiguous buffer with multiple entries that
>>> have consecutive sg_dma_addresses, effectively making it contiguous.
>>> Allow for that scenario by testing the entries in the sg_table for
>>> contiguous coverage.
>>>
>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
>>> ---
>>>
>>> Laurent,
>>>
>>> Thanks for the review! I would like to ask for one more favour: if you
>>> are OK with this version, can you pull this patch through the drm-misc tree?
>> I could, but I'd first need to set dim up, and I'm currently abroad with a bad
>> internet connection and a big deadline for the middle of next week (I know,
>> lots of excuses), so it's not very convenient for me at this time.
> Any other drm-misc maintainers feeling helpful and willing to take this
> patch in?

Sure, I can do it this evening.

Noralf.

>   Otherwise I can send it through the mali-dp tree if no one
> objects.
>
> Best regards,
> Liviu
>
>>>   drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
>>>   include/drm/drm_gem_cma_helper.h     |  4 +++-
>>>   2 files changed, 23 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
>>> b/drivers/gpu/drm/drm_gem_cma_helper.c index 020e7668dfaba..43b179212052d
>>> 100644
>>> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
>>> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
>>> @@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device
>>> *dev, {
>>>   	struct drm_gem_cma_object *cma_obj;
>>>
>>> -	if (sgt->nents != 1)
>>> -		return ERR_PTR(-EINVAL);
>>> +	if (sgt->nents != 1) {
>>> +		/* check if the entries in the sg_table are contiguous */
>>> +		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
>>> +		struct scatterlist *s;
>>> +		unsigned int i;
>>> +
>>> +		for_each_sg(sgt->sgl, s, sgt->nents, i) {
>>> +			/*
>>> +			 * sg_dma_address(s) is only valid for entries
>>> +			 * that have sg_dma_len(s) != 0
>>> +			 */
>>> +			if (!sg_dma_len(s))
>>> +				continue;
>>> +
>>> +			if (sg_dma_address(s) != next_addr)
>>> +				return ERR_PTR(-EINVAL);
>>> +
>>> +			next_addr = sg_dma_address(s) + sg_dma_len(s);
>>> +		}
>>> +	}
>>>
>>>   	/* Create a CMA GEM buffer. */
>>>   	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
>>> diff --git a/include/drm/drm_gem_cma_helper.h
>>> b/include/drm/drm_gem_cma_helper.h index 58a739bf15f1f..214aa85adc8d5
>>> 100644
>>> --- a/include/drm/drm_gem_cma_helper.h
>>> +++ b/include/drm/drm_gem_cma_helper.h
>>> @@ -8,7 +8,9 @@
>>>    * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
>>>    * @base: base GEM object
>>>    * @paddr: physical address of the backing memory
>>> - * @sgt: scatter/gather table for imported PRIME buffers
>>> + * @sgt: scatter/gather table for imported PRIME buffers. The table can
>>> have + *       more than one entry but they are guaranteed to have
>>> contiguous + *       DMA addresses.
>>>    * @vaddr: kernel virtual address of the backing memory
>>>    */
>>>   struct drm_gem_cma_object {
>> -- 
>> Regards,
>>
>> Laurent Pinchart
>>

Re: [PATCH v2] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Noralf Trønnes]

Den 15.11.2017 14.04, skrev Liviu Dudau:
> Hi,
>
> On Sat, Nov 11, 2017 at 02:47:35PM +0200, Laurent Pinchart wrote:
>> Hi Liviu,
>>
>> Thank you for the patch.
>>
>> On Friday, 10 November 2017 15:33:10 EET Liviu Dudau wrote:
>>> drm_gem_cma_prime_import_sg_table() will fail if the number of entries
>>> in the sg_table > 1. However, you can have a device that uses an IOMMU
>>> engine and can map a discontiguous buffer with multiple entries that
>>> have consecutive sg_dma_addresses, effectively making it contiguous.
>>> Allow for that scenario by testing the entries in the sg_table for
>>> contiguous coverage.
>>>
>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
>>> ---
>>>
>>> Laurent,
>>>
>>> Thanks for the review! I would like to ask for one more favour: if you
>>> are OK with this version, can you pull this patch through the drm-misc tree?
>> I could, but I'd first need to set dim up, and I'm currently abroad with a bad
>> internet connection and a big deadline for the middle of next week (I know,
>> lots of excuses), so it's not very convenient for me at this time.
> Any other drm-misc maintainers feeling helpful and willing to take this
> patch in?

Sure, I can do it this evening.

Noralf.

>   Otherwise I can send it through the mali-dp tree if no one
> objects.
>
> Best regards,
> Liviu
>
>>>   drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
>>>   include/drm/drm_gem_cma_helper.h     |  4 +++-
>>>   2 files changed, 23 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
>>> b/drivers/gpu/drm/drm_gem_cma_helper.c index 020e7668dfaba..43b179212052d
>>> 100644
>>> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
>>> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
>>> @@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device
>>> *dev, {
>>>   	struct drm_gem_cma_object *cma_obj;
>>>
>>> -	if (sgt->nents != 1)
>>> -		return ERR_PTR(-EINVAL);
>>> +	if (sgt->nents != 1) {
>>> +		/* check if the entries in the sg_table are contiguous */
>>> +		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
>>> +		struct scatterlist *s;
>>> +		unsigned int i;
>>> +
>>> +		for_each_sg(sgt->sgl, s, sgt->nents, i) {
>>> +			/*
>>> +			 * sg_dma_address(s) is only valid for entries
>>> +			 * that have sg_dma_len(s) != 0
>>> +			 */
>>> +			if (!sg_dma_len(s))
>>> +				continue;
>>> +
>>> +			if (sg_dma_address(s) != next_addr)
>>> +				return ERR_PTR(-EINVAL);
>>> +
>>> +			next_addr = sg_dma_address(s) + sg_dma_len(s);
>>> +		}
>>> +	}
>>>
>>>   	/* Create a CMA GEM buffer. */
>>>   	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
>>> diff --git a/include/drm/drm_gem_cma_helper.h
>>> b/include/drm/drm_gem_cma_helper.h index 58a739bf15f1f..214aa85adc8d5
>>> 100644
>>> --- a/include/drm/drm_gem_cma_helper.h
>>> +++ b/include/drm/drm_gem_cma_helper.h
>>> @@ -8,7 +8,9 @@
>>>    * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
>>>    * @base: base GEM object
>>>    * @paddr: physical address of the backing memory
>>> - * @sgt: scatter/gather table for imported PRIME buffers
>>> + * @sgt: scatter/gather table for imported PRIME buffers. The table can
>>> have + *       more than one entry but they are guaranteed to have
>>> contiguous + *       DMA addresses.
>>>    * @vaddr: kernel virtual address of the backing memory
>>>    */
>>>   struct drm_gem_cma_object {
>> -- 
>> Regards,
>>
>> Laurent Pinchart
>>

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

Re: [PATCH v2] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Liviu Dudau]

On Wed, Nov 15, 2017 at 03:24:41PM +0100, Noralf Trønnes wrote:
> 
> Den 15.11.2017 14.04, skrev Liviu Dudau:
> > Hi,
> > 
> > On Sat, Nov 11, 2017 at 02:47:35PM +0200, Laurent Pinchart wrote:
> > > Hi Liviu,
> > > 
> > > Thank you for the patch.
> > > 
> > > On Friday, 10 November 2017 15:33:10 EET Liviu Dudau wrote:
> > > > drm_gem_cma_prime_import_sg_table() will fail if the number of entries
> > > > in the sg_table > 1. However, you can have a device that uses an IOMMU
> > > > engine and can map a discontiguous buffer with multiple entries that
> > > > have consecutive sg_dma_addresses, effectively making it contiguous.
> > > > Allow for that scenario by testing the entries in the sg_table for
> > > > contiguous coverage.
> > > > 
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> > > > ---
> > > > 
> > > > Laurent,
> > > > 
> > > > Thanks for the review! I would like to ask for one more favour: if you
> > > > are OK with this version, can you pull this patch through the drm-misc tree?
> > > I could, but I'd first need to set dim up, and I'm currently abroad with a bad
> > > internet connection and a big deadline for the middle of next week (I know,
> > > lots of excuses), so it's not very convenient for me at this time.
> > Any other drm-misc maintainers feeling helpful and willing to take this
> > patch in?
> 
> Sure, I can do it this evening.

Cheers, much appreciated!

Best regards,
Liviu

> 
> Noralf.
> 
> >   Otherwise I can send it through the mali-dp tree if no one
> > objects.
> > 
> > Best regards,
> > Liviu
> > 
> > > >   drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
> > > >   include/drm/drm_gem_cma_helper.h     |  4 +++-
> > > >   2 files changed, 23 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> > > > b/drivers/gpu/drm/drm_gem_cma_helper.c index 020e7668dfaba..43b179212052d
> > > > 100644
> > > > --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> > > > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> > > > @@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device
> > > > *dev, {
> > > >   	struct drm_gem_cma_object *cma_obj;
> > > > 
> > > > -	if (sgt->nents != 1)
> > > > -		return ERR_PTR(-EINVAL);
> > > > +	if (sgt->nents != 1) {
> > > > +		/* check if the entries in the sg_table are contiguous */
> > > > +		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
> > > > +		struct scatterlist *s;
> > > > +		unsigned int i;
> > > > +
> > > > +		for_each_sg(sgt->sgl, s, sgt->nents, i) {
> > > > +			/*
> > > > +			 * sg_dma_address(s) is only valid for entries
> > > > +			 * that have sg_dma_len(s) != 0
> > > > +			 */
> > > > +			if (!sg_dma_len(s))
> > > > +				continue;
> > > > +
> > > > +			if (sg_dma_address(s) != next_addr)
> > > > +				return ERR_PTR(-EINVAL);
> > > > +
> > > > +			next_addr = sg_dma_address(s) + sg_dma_len(s);
> > > > +		}
> > > > +	}
> > > > 
> > > >   	/* Create a CMA GEM buffer. */
> > > >   	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
> > > > diff --git a/include/drm/drm_gem_cma_helper.h
> > > > b/include/drm/drm_gem_cma_helper.h index 58a739bf15f1f..214aa85adc8d5
> > > > 100644
> > > > --- a/include/drm/drm_gem_cma_helper.h
> > > > +++ b/include/drm/drm_gem_cma_helper.h
> > > > @@ -8,7 +8,9 @@
> > > >    * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
> > > >    * @base: base GEM object
> > > >    * @paddr: physical address of the backing memory
> > > > - * @sgt: scatter/gather table for imported PRIME buffers
> > > > + * @sgt: scatter/gather table for imported PRIME buffers. The table can
> > > > have + *       more than one entry but they are guaranteed to have
> > > > contiguous + *       DMA addresses.
> > > >    * @vaddr: kernel virtual address of the backing memory
> > > >    */
> > > >   struct drm_gem_cma_object {
> > > -- 
> > > Regards,
> > > 
> > > Laurent Pinchart
> > > 
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH v2] drm: gem_cma_helper.c: Allow importing of contiguous scatterlists with nents > 1

[Liviu Dudau]

On Wed, Nov 15, 2017 at 03:24:41PM +0100, Noralf Trønnes wrote:
> 
> Den 15.11.2017 14.04, skrev Liviu Dudau:
> > Hi,
> > 
> > On Sat, Nov 11, 2017 at 02:47:35PM +0200, Laurent Pinchart wrote:
> > > Hi Liviu,
> > > 
> > > Thank you for the patch.
> > > 
> > > On Friday, 10 November 2017 15:33:10 EET Liviu Dudau wrote:
> > > > drm_gem_cma_prime_import_sg_table() will fail if the number of entries
> > > > in the sg_table > 1. However, you can have a device that uses an IOMMU
> > > > engine and can map a discontiguous buffer with multiple entries that
> > > > have consecutive sg_dma_addresses, effectively making it contiguous.
> > > > Allow for that scenario by testing the entries in the sg_table for
> > > > contiguous coverage.
> > > > 
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > Signed-off-by: Liviu Dudau <liviu.dudau@arm.com>
> > > > ---
> > > > 
> > > > Laurent,
> > > > 
> > > > Thanks for the review! I would like to ask for one more favour: if you
> > > > are OK with this version, can you pull this patch through the drm-misc tree?
> > > I could, but I'd first need to set dim up, and I'm currently abroad with a bad
> > > internet connection and a big deadline for the middle of next week (I know,
> > > lots of excuses), so it's not very convenient for me at this time.
> > Any other drm-misc maintainers feeling helpful and willing to take this
> > patch in?
> 
> Sure, I can do it this evening.

Cheers, much appreciated!

Best regards,
Liviu

> 
> Noralf.
> 
> >   Otherwise I can send it through the mali-dp tree if no one
> > objects.
> > 
> > Best regards,
> > Liviu
> > 
> > > >   drivers/gpu/drm/drm_gem_cma_helper.c | 22 ++++++++++++++++++++--
> > > >   include/drm/drm_gem_cma_helper.h     |  4 +++-
> > > >   2 files changed, 23 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> > > > b/drivers/gpu/drm/drm_gem_cma_helper.c index 020e7668dfaba..43b179212052d
> > > > 100644
> > > > --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> > > > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> > > > @@ -482,8 +482,26 @@ drm_gem_cma_prime_import_sg_table(struct drm_device
> > > > *dev, {
> > > >   	struct drm_gem_cma_object *cma_obj;
> > > > 
> > > > -	if (sgt->nents != 1)
> > > > -		return ERR_PTR(-EINVAL);
> > > > +	if (sgt->nents != 1) {
> > > > +		/* check if the entries in the sg_table are contiguous */
> > > > +		dma_addr_t next_addr = sg_dma_address(sgt->sgl);
> > > > +		struct scatterlist *s;
> > > > +		unsigned int i;
> > > > +
> > > > +		for_each_sg(sgt->sgl, s, sgt->nents, i) {
> > > > +			/*
> > > > +			 * sg_dma_address(s) is only valid for entries
> > > > +			 * that have sg_dma_len(s) != 0
> > > > +			 */
> > > > +			if (!sg_dma_len(s))
> > > > +				continue;
> > > > +
> > > > +			if (sg_dma_address(s) != next_addr)
> > > > +				return ERR_PTR(-EINVAL);
> > > > +
> > > > +			next_addr = sg_dma_address(s) + sg_dma_len(s);
> > > > +		}
> > > > +	}
> > > > 
> > > >   	/* Create a CMA GEM buffer. */
> > > >   	cma_obj = __drm_gem_cma_create(dev, attach->dmabuf->size);
> > > > diff --git a/include/drm/drm_gem_cma_helper.h
> > > > b/include/drm/drm_gem_cma_helper.h index 58a739bf15f1f..214aa85adc8d5
> > > > 100644
> > > > --- a/include/drm/drm_gem_cma_helper.h
> > > > +++ b/include/drm/drm_gem_cma_helper.h
> > > > @@ -8,7 +8,9 @@
> > > >    * struct drm_gem_cma_object - GEM object backed by CMA memory allocations
> > > >    * @base: base GEM object
> > > >    * @paddr: physical address of the backing memory
> > > > - * @sgt: scatter/gather table for imported PRIME buffers
> > > > + * @sgt: scatter/gather table for imported PRIME buffers. The table can
> > > > have + *       more than one entry but they are guaranteed to have
> > > > contiguous + *       DMA addresses.
> > > >    * @vaddr: kernel virtual address of the backing memory
> > > >    */
> > > >   struct drm_gem_cma_object {
> > > -- 
> > > Regards,
> > > 
> > > Laurent Pinchart
> > > 
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx