* [PATCH/RFC 2/2] drm: rcar-du: Allow importing non-contiguous dma-buf with VSP
@ 2017-11-13 10:32 ` Laurent Pinchart
0 siblings, 0 replies; 17+ messages in thread
From: Laurent Pinchart @ 2017-11-13 10:32 UTC (permalink / raw)
To: dri-devel; +Cc: linux-renesas-soc, Daniel Vetter, Liviu Dudau, Kieran Bingham
When the DU sources its frames from a VSP, it performs no memory access
and thus has no requirements on imported dma-buf memory types. In
particular the DU could import a physically non-contiguous buffer that
would later be mapped contiguously through the VSP IOMMU.
This use case isn't supported at the moment as the GEM CMA helpers will
reject any non-contiguous buffer, and the DU isn't connected to an IOMMU
that can make the buffer contiguous for DMA. Fix this by implementing a
custom .gem_prime_import_sg_table() operation that accepts all imported
dma-buf regardless of the number of scatterlist entries.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 39 +++++++++++++++++++++++++++++++++++
drivers/gpu/drm/rcar-du/rcar_du_kms.h | 7 +++++++
3 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 48c166f925a3..d999231f98c7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -289,7 +289,7 @@ static struct drm_driver rcar_du_driver = {
.gem_prime_import = drm_gem_prime_import,
.gem_prime_export = drm_gem_prime_export,
.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
- .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
+ .gem_prime_import_sg_table = rcar_du_gem_prime_import_sg_table,
.gem_prime_vmap = drm_gem_cma_prime_vmap,
.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
.gem_prime_mmap = drm_gem_cma_prime_mmap,
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 566d1a948c8f..2dd0c2ba047d 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -20,6 +20,7 @@
#include <drm/drm_gem_cma_helper.h>
#include <drm/drm_gem_framebuffer_helper.h>
+#include <linux/dma-buf.h>
#include <linux/of_graph.h>
#include <linux/wait.h>
@@ -148,6 +149,44 @@ const struct rcar_du_format_info *rcar_du_format_info(u32 fourcc)
* Frame buffer
*/
+struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct drm_device *dev,
+ struct dma_buf_attachment *attach,
+ struct sg_table *sgt)
+{
+ struct rcar_du_device *rcdu = dev->dev_private;
+ struct drm_gem_cma_object *cma_obj;
+ struct drm_gem_object *gem_obj;
+ int ret;
+
+ if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
+ return drm_gem_cma_prime_import_sg_table(dev, attach, sgt);
+
+ /* Create a CMA GEM buffer. */
+ cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
+ if (!cma_obj)
+ return ERR_PTR(-ENOMEM);
+ gem_obj = &cma_obj->base;
+
+ ret = drm_gem_object_init(dev, gem_obj, attach->dmabuf->size);
+ if (ret)
+ goto error;
+
+ ret = drm_gem_create_mmap_offset(gem_obj);
+ if (ret) {
+ drm_gem_object_release(gem_obj);
+ goto error;
+ }
+
+ cma_obj->paddr = 0;
+ cma_obj->sgt = sgt;
+
+ return gem_obj;
+
+error:
+ kfree(cma_obj);
+ return ERR_PTR(ret);
+}
+
int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
struct drm_mode_create_dumb *args)
{
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.h b/drivers/gpu/drm/rcar-du/rcar_du_kms.h
index 07951d5fe38b..10b2bb0f0df9 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.h
@@ -16,10 +16,13 @@
#include <linux/types.h>
+struct dma_buf_attachment;
struct drm_file;
struct drm_device;
+struct drm_gem_object;
struct drm_mode_create_dumb;
struct rcar_du_device;
+struct sg_table;
struct rcar_du_format_info {
u32 fourcc;
@@ -36,4 +39,8 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu);
int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
struct drm_mode_create_dumb *args);
+struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct drm_device *dev,
+ struct dma_buf_attachment *attach,
+ struct sg_table *sgt);
+
#endif /* __RCAR_DU_KMS_H__ */
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH/RFC 2/2] drm: rcar-du: Allow importing non-contiguous dma-buf with VSP
2017-11-13 10:32 ` Laurent Pinchart
@ 2017-11-13 11:53 ` Liviu Dudau
-1 siblings, 0 replies; 17+ messages in thread
From: Liviu Dudau @ 2017-11-13 11:53 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel, linux-renesas-soc, Kieran Bingham, Daniel Vetter
On Mon, Nov 13, 2017 at 12:32:28PM +0200, Laurent Pinchart wrote:
> When the DU sources its frames from a VSP, it performs no memory access
> and thus has no requirements on imported dma-buf memory types. In
> particular the DU could import a physically non-contiguous buffer that
> would later be mapped contiguously through the VSP IOMMU.
>
> This use case isn't supported at the moment as the GEM CMA helpers will
> reject any non-contiguous buffer, and the DU isn't connected to an IOMMU
> that can make the buffer contiguous for DMA. Fix this by implementing a
> custom .gem_prime_import_sg_table() operation that accepts all imported
> dma-buf regardless of the number of scatterlist entries.
This patch raises the question of why use CMA at all if you can accept
any kind of buffers.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 39 +++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/rcar-du/rcar_du_kms.h | 7 +++++++
> 3 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 48c166f925a3..d999231f98c7 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -289,7 +289,7 @@ static struct drm_driver rcar_du_driver = {
> .gem_prime_import = drm_gem_prime_import,
> .gem_prime_export = drm_gem_prime_export,
> .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> + .gem_prime_import_sg_table = rcar_du_gem_prime_import_sg_table,
> .gem_prime_vmap = drm_gem_cma_prime_vmap,
> .gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> .gem_prime_mmap = drm_gem_cma_prime_mmap,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index 566d1a948c8f..2dd0c2ba047d 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -20,6 +20,7 @@
> #include <drm/drm_gem_cma_helper.h>
> #include <drm/drm_gem_framebuffer_helper.h>
>
> +#include <linux/dma-buf.h>
> #include <linux/of_graph.h>
> #include <linux/wait.h>
>
> @@ -148,6 +149,44 @@ const struct rcar_du_format_info *rcar_du_format_info(u32 fourcc)
> * Frame buffer
> */
>
> +struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct drm_device *dev,
> + struct dma_buf_attachment *attach,
> + struct sg_table *sgt)
> +{
> + struct rcar_du_device *rcdu = dev->dev_private;
> + struct drm_gem_cma_object *cma_obj;
> + struct drm_gem_object *gem_obj;
> + int ret;
> +
> + if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
> + return drm_gem_cma_prime_import_sg_table(dev, attach, sgt);
> +
> + /* Create a CMA GEM buffer. */
> + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
> + if (!cma_obj)
> + return ERR_PTR(-ENOMEM);
> + gem_obj = &cma_obj->base;
> +
> + ret = drm_gem_object_init(dev, gem_obj, attach->dmabuf->size);
> + if (ret)
> + goto error;
> +
> + ret = drm_gem_create_mmap_offset(gem_obj);
> + if (ret) {
> + drm_gem_object_release(gem_obj);
> + goto error;
> + }
> +
> + cma_obj->paddr = 0;
This is going to break drm_gem_cma_describe() if you are using it plus
the rcar_du_plane_setup_scanout() unless I'm missing something besides
familiarity with the RCAR driver code :)
This function looks very similar to what I tried to do for mali-dp to
allow the import of contiguous DMA buffers that have more than 1 sgt
entries. In the end I gave up as I kept finding issues and went for the
drm_gem_cma_prime_import_sg_table() changes. Maybe you need to do a
similar change in the function to bypass some requirements if the driver
signals that it can accept relaxed requirements?
Best regards,
Liviu
> + cma_obj->sgt = sgt;
> +
> + return gem_obj;
> +
> +error:
> + kfree(cma_obj);
> + return ERR_PTR(ret);
> +}
> +
> int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
> struct drm_mode_create_dumb *args)
> {
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.h b/drivers/gpu/drm/rcar-du/rcar_du_kms.h
> index 07951d5fe38b..10b2bb0f0df9 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.h
> @@ -16,10 +16,13 @@
>
> #include <linux/types.h>
>
> +struct dma_buf_attachment;
> struct drm_file;
> struct drm_device;
> +struct drm_gem_object;
> struct drm_mode_create_dumb;
> struct rcar_du_device;
> +struct sg_table;
>
> struct rcar_du_format_info {
> u32 fourcc;
> @@ -36,4 +39,8 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu);
> int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
> struct drm_mode_create_dumb *args);
>
> +struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct drm_device *dev,
> + struct dma_buf_attachment *attach,
> + struct sg_table *sgt);
> +
> #endif /* __RCAR_DU_KMS_H__ */
> --
> Regards,
>
> Laurent Pinchart
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH/RFC 2/2] drm: rcar-du: Allow importing non-contiguous dma-buf with VSP
@ 2017-11-13 11:53 ` Liviu Dudau
0 siblings, 0 replies; 17+ messages in thread
From: Liviu Dudau @ 2017-11-13 11:53 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-renesas-soc, Daniel Vetter, Kieran Bingham, dri-devel
On Mon, Nov 13, 2017 at 12:32:28PM +0200, Laurent Pinchart wrote:
> When the DU sources its frames from a VSP, it performs no memory access
> and thus has no requirements on imported dma-buf memory types. In
> particular the DU could import a physically non-contiguous buffer that
> would later be mapped contiguously through the VSP IOMMU.
>
> This use case isn't supported at the moment as the GEM CMA helpers will
> reject any non-contiguous buffer, and the DU isn't connected to an IOMMU
> that can make the buffer contiguous for DMA. Fix this by implementing a
> custom .gem_prime_import_sg_table() operation that accepts all imported
> dma-buf regardless of the number of scatterlist entries.
This patch raises the question of why use CMA at all if you can accept
any kind of buffers.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 39 +++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/rcar-du/rcar_du_kms.h | 7 +++++++
> 3 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 48c166f925a3..d999231f98c7 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -289,7 +289,7 @@ static struct drm_driver rcar_du_driver = {
> .gem_prime_import = drm_gem_prime_import,
> .gem_prime_export = drm_gem_prime_export,
> .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> + .gem_prime_import_sg_table = rcar_du_gem_prime_import_sg_table,
> .gem_prime_vmap = drm_gem_cma_prime_vmap,
> .gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> .gem_prime_mmap = drm_gem_cma_prime_mmap,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index 566d1a948c8f..2dd0c2ba047d 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -20,6 +20,7 @@
> #include <drm/drm_gem_cma_helper.h>
> #include <drm/drm_gem_framebuffer_helper.h>
>
> +#include <linux/dma-buf.h>
> #include <linux/of_graph.h>
> #include <linux/wait.h>
>
> @@ -148,6 +149,44 @@ const struct rcar_du_format_info *rcar_du_format_info(u32 fourcc)
> * Frame buffer
> */
>
> +struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct drm_device *dev,
> + struct dma_buf_attachment *attach,
> + struct sg_table *sgt)
> +{
> + struct rcar_du_device *rcdu = dev->dev_private;
> + struct drm_gem_cma_object *cma_obj;
> + struct drm_gem_object *gem_obj;
> + int ret;
> +
> + if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
> + return drm_gem_cma_prime_import_sg_table(dev, attach, sgt);
> +
> + /* Create a CMA GEM buffer. */
> + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
> + if (!cma_obj)
> + return ERR_PTR(-ENOMEM);
> + gem_obj = &cma_obj->base;
> +
> + ret = drm_gem_object_init(dev, gem_obj, attach->dmabuf->size);
> + if (ret)
> + goto error;
> +
> + ret = drm_gem_create_mmap_offset(gem_obj);
> + if (ret) {
> + drm_gem_object_release(gem_obj);
> + goto error;
> + }
> +
> + cma_obj->paddr = 0;
This is going to break drm_gem_cma_describe() if you are using it plus
the rcar_du_plane_setup_scanout() unless I'm missing something besides
familiarity with the RCAR driver code :)
This function looks very similar to what I tried to do for mali-dp to
allow the import of contiguous DMA buffers that have more than 1 sgt
entries. In the end I gave up as I kept finding issues and went for the
drm_gem_cma_prime_import_sg_table() changes. Maybe you need to do a
similar change in the function to bypass some requirements if the driver
signals that it can accept relaxed requirements?
Best regards,
Liviu
> + cma_obj->sgt = sgt;
> +
> + return gem_obj;
> +
> +error:
> + kfree(cma_obj);
> + return ERR_PTR(ret);
> +}
> +
> int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
> struct drm_mode_create_dumb *args)
> {
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.h b/drivers/gpu/drm/rcar-du/rcar_du_kms.h
> index 07951d5fe38b..10b2bb0f0df9 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.h
> @@ -16,10 +16,13 @@
>
> #include <linux/types.h>
>
> +struct dma_buf_attachment;
> struct drm_file;
> struct drm_device;
> +struct drm_gem_object;
> struct drm_mode_create_dumb;
> struct rcar_du_device;
> +struct sg_table;
>
> struct rcar_du_format_info {
> u32 fourcc;
> @@ -36,4 +39,8 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu);
> int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
> struct drm_mode_create_dumb *args);
>
> +struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct drm_device *dev,
> + struct dma_buf_attachment *attach,
> + struct sg_table *sgt);
> +
> #endif /* __RCAR_DU_KMS_H__ */
> --
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH/RFC 2/2] drm: rcar-du: Allow importing non-contiguous dma-buf with VSP
2017-11-13 11:53 ` Liviu Dudau
@ 2017-11-14 3:34 ` Laurent Pinchart
-1 siblings, 0 replies; 17+ messages in thread
From: Laurent Pinchart @ 2017-11-14 3:34 UTC (permalink / raw)
To: Liviu Dudau
Cc: Laurent Pinchart, dri-devel, linux-renesas-soc, Kieran Bingham,
Daniel Vetter
Hi Liviu,
On Monday, 13 November 2017 13:53:14 EET Liviu Dudau wrote:
> On Mon, Nov 13, 2017 at 12:32:28PM +0200, Laurent Pinchart wrote:
> > When the DU sources its frames from a VSP, it performs no memory access
> > and thus has no requirements on imported dma-buf memory types. In
> > particular the DU could import a physically non-contiguous buffer that
> > would later be mapped contiguously through the VSP IOMMU.
> >
> > This use case isn't supported at the moment as the GEM CMA helpers will
> > reject any non-contiguous buffer, and the DU isn't connected to an IOMMU
> > that can make the buffer contiguous for DMA. Fix this by implementing a
> > custom .gem_prime_import_sg_table() operation that accepts all imported
> > dma-buf regardless of the number of scatterlist entries.
>
> This patch raises the question of why use CMA at all if you can accept
> any kind of buffers.
Both the DU and VSP require DMA contiguous memory. On R-Car Gen2 the DU
performs DMA, and thus uses the GEM CMA helpers. On Gen3 it delegates DMA to
the external VSP composer, and still uses the GEM CMA helpers to ensure that
memory will be DMA contiguous for the VSP. The problem arises when physically
non-contiguous dma-buf buffers are imported, they can be mapped contiguously
to the VSP (assuming an IOMMU is present) but not to the DU (as there's no
IOMMU).
The situation is particularly messy due to the fact that I have one VSP
instance per CRTC, each connected to its own IOMMU channel. The driver thus
doesn't know when allocating GEM objects to which struct device they need to
be mapped. The DRM helpers don't support delayed mapping of memory. I'd like
to know whether that's something I should fix properly (which would likely
involve major reworking of the helpers) or hack around it in the DU driver.
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >
> > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
> > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 39 ++++++++++++++++++++++++++++++
> > drivers/gpu/drm/rcar-du/rcar_du_kms.h | 7 +++++++
> > 3 files changed, 47 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 48c166f925a3..d999231f98c7
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > @@ -289,7 +289,7 @@ static struct drm_driver rcar_du_driver = {
> > .gem_prime_import = drm_gem_prime_import,
> > .gem_prime_export = drm_gem_prime_export,
> > .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> > - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> > + .gem_prime_import_sg_table = rcar_du_gem_prime_import_sg_table,
> > .gem_prime_vmap = drm_gem_cma_prime_vmap,
> > .gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> > .gem_prime_mmap = drm_gem_cma_prime_mmap,
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 566d1a948c8f..2dd0c2ba047d
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
[snip]
> > +struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct
> > drm_device *dev,
> > + struct dma_buf_attachment *attach,
> > + struct sg_table *sgt)
> > +{
> > + struct rcar_du_device *rcdu = dev->dev_private;
> > + struct drm_gem_cma_object *cma_obj;
> > + struct drm_gem_object *gem_obj;
> > + int ret;
> > +
> > + if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
> > + return drm_gem_cma_prime_import_sg_table(dev, attach, sgt);
> > +
> > + /* Create a CMA GEM buffer. */
> > + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
> > + if (!cma_obj)
> > + return ERR_PTR(-ENOMEM);
> > + gem_obj = &cma_obj->base;
> > +
> > + ret = drm_gem_object_init(dev, gem_obj, attach->dmabuf->size);
> > + if (ret)
> > + goto error;
> > +
> > + ret = drm_gem_create_mmap_offset(gem_obj);
> > + if (ret) {
> > + drm_gem_object_release(gem_obj);
> > + goto error;
> > + }
> > +
> > + cma_obj->paddr = 0;
>
> This is going to break drm_gem_cma_describe() if you are using it
As far as I can tell drm_gem_cma_describe() will just print paddr = 0, it
won't crash.
> plus the rcar_du_plane_setup_scanout() unless I'm missing something besides
> familiarity with the RCAR driver code :)
rcar_du_plane_setup_scanout() is only called when !rcar_du_has(rcdu,
RCAR_DU_FEATURE_VSP1_SOURCE) (that is on Gen3 hardware, or on Gen2 when I
artificially disable all DMA from the DU and use external composers to emulate
Gen3 behaviour for testing purpose), in which case this function calls
drm_gem_cma_prime_import_sg_table().
> This function looks very similar to what I tried to do for mali-dp to
> allow the import of contiguous DMA buffers that have more than 1 sgt
> entries. In the end I gave up as I kept finding issues and went for the
> drm_gem_cma_prime_import_sg_table() changes. Maybe you need to do a
> similar change in the function to bypass some requirements if the driver
> signals that it can accept relaxed requirements?
As explained above I've considered reworking the helpers (to do it properly I
think I'll likely need to rework drm_prime.c too), but I'd first like to know
whether there's an interest for that.
> > + cma_obj->sgt = sgt;
> > +
> > + return gem_obj;
> > +
> > +error:
> > + kfree(cma_obj);
> > + return ERR_PTR(ret);
> > +}
[snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH/RFC 2/2] drm: rcar-du: Allow importing non-contiguous dma-buf with VSP
@ 2017-11-14 3:34 ` Laurent Pinchart
0 siblings, 0 replies; 17+ messages in thread
From: Laurent Pinchart @ 2017-11-14 3:34 UTC (permalink / raw)
To: Liviu Dudau
Cc: linux-renesas-soc, Daniel Vetter, Laurent Pinchart,
Kieran Bingham, dri-devel
Hi Liviu,
On Monday, 13 November 2017 13:53:14 EET Liviu Dudau wrote:
> On Mon, Nov 13, 2017 at 12:32:28PM +0200, Laurent Pinchart wrote:
> > When the DU sources its frames from a VSP, it performs no memory access
> > and thus has no requirements on imported dma-buf memory types. In
> > particular the DU could import a physically non-contiguous buffer that
> > would later be mapped contiguously through the VSP IOMMU.
> >
> > This use case isn't supported at the moment as the GEM CMA helpers will
> > reject any non-contiguous buffer, and the DU isn't connected to an IOMMU
> > that can make the buffer contiguous for DMA. Fix this by implementing a
> > custom .gem_prime_import_sg_table() operation that accepts all imported
> > dma-buf regardless of the number of scatterlist entries.
>
> This patch raises the question of why use CMA at all if you can accept
> any kind of buffers.
Both the DU and VSP require DMA contiguous memory. On R-Car Gen2 the DU
performs DMA, and thus uses the GEM CMA helpers. On Gen3 it delegates DMA to
the external VSP composer, and still uses the GEM CMA helpers to ensure that
memory will be DMA contiguous for the VSP. The problem arises when physically
non-contiguous dma-buf buffers are imported, they can be mapped contiguously
to the VSP (assuming an IOMMU is present) but not to the DU (as there's no
IOMMU).
The situation is particularly messy due to the fact that I have one VSP
instance per CRTC, each connected to its own IOMMU channel. The driver thus
doesn't know when allocating GEM objects to which struct device they need to
be mapped. The DRM helpers don't support delayed mapping of memory. I'd like
to know whether that's something I should fix properly (which would likely
involve major reworking of the helpers) or hack around it in the DU driver.
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >
> > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
> > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 39 ++++++++++++++++++++++++++++++
> > drivers/gpu/drm/rcar-du/rcar_du_kms.h | 7 +++++++
> > 3 files changed, 47 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 48c166f925a3..d999231f98c7
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > @@ -289,7 +289,7 @@ static struct drm_driver rcar_du_driver = {
> > .gem_prime_import = drm_gem_prime_import,
> > .gem_prime_export = drm_gem_prime_export,
> > .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> > - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> > + .gem_prime_import_sg_table = rcar_du_gem_prime_import_sg_table,
> > .gem_prime_vmap = drm_gem_cma_prime_vmap,
> > .gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> > .gem_prime_mmap = drm_gem_cma_prime_mmap,
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 566d1a948c8f..2dd0c2ba047d
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
[snip]
> > +struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct
> > drm_device *dev,
> > + struct dma_buf_attachment *attach,
> > + struct sg_table *sgt)
> > +{
> > + struct rcar_du_device *rcdu = dev->dev_private;
> > + struct drm_gem_cma_object *cma_obj;
> > + struct drm_gem_object *gem_obj;
> > + int ret;
> > +
> > + if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
> > + return drm_gem_cma_prime_import_sg_table(dev, attach, sgt);
> > +
> > + /* Create a CMA GEM buffer. */
> > + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
> > + if (!cma_obj)
> > + return ERR_PTR(-ENOMEM);
> > + gem_obj = &cma_obj->base;
> > +
> > + ret = drm_gem_object_init(dev, gem_obj, attach->dmabuf->size);
> > + if (ret)
> > + goto error;
> > +
> > + ret = drm_gem_create_mmap_offset(gem_obj);
> > + if (ret) {
> > + drm_gem_object_release(gem_obj);
> > + goto error;
> > + }
> > +
> > + cma_obj->paddr = 0;
>
> This is going to break drm_gem_cma_describe() if you are using it
As far as I can tell drm_gem_cma_describe() will just print paddr = 0, it
won't crash.
> plus the rcar_du_plane_setup_scanout() unless I'm missing something besides
> familiarity with the RCAR driver code :)
rcar_du_plane_setup_scanout() is only called when !rcar_du_has(rcdu,
RCAR_DU_FEATURE_VSP1_SOURCE) (that is on Gen3 hardware, or on Gen2 when I
artificially disable all DMA from the DU and use external composers to emulate
Gen3 behaviour for testing purpose), in which case this function calls
drm_gem_cma_prime_import_sg_table().
> This function looks very similar to what I tried to do for mali-dp to
> allow the import of contiguous DMA buffers that have more than 1 sgt
> entries. In the end I gave up as I kept finding issues and went for the
> drm_gem_cma_prime_import_sg_table() changes. Maybe you need to do a
> similar change in the function to bypass some requirements if the driver
> signals that it can accept relaxed requirements?
As explained above I've considered reworking the helpers (to do it properly I
think I'll likely need to rework drm_prime.c too), but I'd first like to know
whether there's an interest for that.
> > + cma_obj->sgt = sgt;
> > +
> > + return gem_obj;
> > +
> > +error:
> > + kfree(cma_obj);
> > + return ERR_PTR(ret);
> > +}
[snip]
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH/RFC 2/2] drm: rcar-du: Allow importing non-contiguous dma-buf with VSP
2017-11-14 3:34 ` Laurent Pinchart
@ 2017-11-14 9:27 ` Liviu Dudau
-1 siblings, 0 replies; 17+ messages in thread
From: Liviu Dudau @ 2017-11-14 9:27 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Laurent Pinchart, dri-devel, linux-renesas-soc, Kieran Bingham,
Daniel Vetter
On Tue, Nov 14, 2017 at 05:34:14AM +0200, Laurent Pinchart wrote:
> Hi Liviu,
Hi Laurent,
>
> On Monday, 13 November 2017 13:53:14 EET Liviu Dudau wrote:
> > On Mon, Nov 13, 2017 at 12:32:28PM +0200, Laurent Pinchart wrote:
> > > When the DU sources its frames from a VSP, it performs no memory access
> > > and thus has no requirements on imported dma-buf memory types. In
> > > particular the DU could import a physically non-contiguous buffer that
> > > would later be mapped contiguously through the VSP IOMMU.
> > >
> > > This use case isn't supported at the moment as the GEM CMA helpers will
> > > reject any non-contiguous buffer, and the DU isn't connected to an IOMMU
> > > that can make the buffer contiguous for DMA. Fix this by implementing a
> > > custom .gem_prime_import_sg_table() operation that accepts all imported
> > > dma-buf regardless of the number of scatterlist entries.
> >
> > This patch raises the question of why use CMA at all if you can accept
> > any kind of buffers.
>
> Both the DU and VSP require DMA contiguous memory. On R-Car Gen2 the DU
> performs DMA, and thus uses the GEM CMA helpers. On Gen3 it delegates DMA to
> the external VSP composer, and still uses the GEM CMA helpers to ensure that
> memory will be DMA contiguous for the VSP. The problem arises when physically
> non-contiguous dma-buf buffers are imported, they can be mapped contiguously
> to the VSP (assuming an IOMMU is present) but not to the DU (as there's no
> IOMMU).
>
> The situation is particularly messy due to the fact that I have one VSP
> instance per CRTC, each connected to its own IOMMU channel. The driver thus
> doesn't know when allocating GEM objects to which struct device they need to
> be mapped. The DRM helpers don't support delayed mapping of memory. I'd like
> to know whether that's something I should fix properly (which would likely
> involve major reworking of the helpers) or hack around it in the DU driver.
I understand. Looks like you have your hands full with an "interesting"
architecture.
>
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > >
> > > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
> > > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 39 ++++++++++++++++++++++++++++++
> > > drivers/gpu/drm/rcar-du/rcar_du_kms.h | 7 +++++++
> > > 3 files changed, 47 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 48c166f925a3..d999231f98c7
> > > 100644
> > > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > > @@ -289,7 +289,7 @@ static struct drm_driver rcar_du_driver = {
> > > .gem_prime_import = drm_gem_prime_import,
> > > .gem_prime_export = drm_gem_prime_export,
> > > .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> > > - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> > > + .gem_prime_import_sg_table = rcar_du_gem_prime_import_sg_table,
> > > .gem_prime_vmap = drm_gem_cma_prime_vmap,
> > > .gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> > > .gem_prime_mmap = drm_gem_cma_prime_mmap,
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 566d1a948c8f..2dd0c2ba047d
> > > 100644
> > > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>
> [snip]
>
> > > +struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct
> > > drm_device *dev,
> > > + struct dma_buf_attachment *attach,
> > > + struct sg_table *sgt)
> > > +{
> > > + struct rcar_du_device *rcdu = dev->dev_private;
> > > + struct drm_gem_cma_object *cma_obj;
> > > + struct drm_gem_object *gem_obj;
> > > + int ret;
> > > +
> > > + if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
> > > + return drm_gem_cma_prime_import_sg_table(dev, attach, sgt);
> > > +
> > > + /* Create a CMA GEM buffer. */
> > > + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
> > > + if (!cma_obj)
> > > + return ERR_PTR(-ENOMEM);
> > > + gem_obj = &cma_obj->base;
> > > +
> > > + ret = drm_gem_object_init(dev, gem_obj, attach->dmabuf->size);
> > > + if (ret)
> > > + goto error;
> > > +
> > > + ret = drm_gem_create_mmap_offset(gem_obj);
> > > + if (ret) {
> > > + drm_gem_object_release(gem_obj);
> > > + goto error;
> > > + }
> > > +
> > > + cma_obj->paddr = 0;
> >
> > This is going to break drm_gem_cma_describe() if you are using it
>
> As far as I can tell drm_gem_cma_describe() will just print paddr = 0, it
> won't crash.
I'm sorry, didn't meant it will crash, just print (possibly useless)
zero value.
>
> > plus the rcar_du_plane_setup_scanout() unless I'm missing something besides
> > familiarity with the RCAR driver code :)
>
> rcar_du_plane_setup_scanout() is only called when !rcar_du_has(rcdu,
> RCAR_DU_FEATURE_VSP1_SOURCE) (that is on Gen3 hardware, or on Gen2 when I
> artificially disable all DMA from the DU and use external composers to emulate
> Gen3 behaviour for testing purpose), in which case this function calls
> drm_gem_cma_prime_import_sg_table().
Thanks for the explanation!
>
> > This function looks very similar to what I tried to do for mali-dp to
> > allow the import of contiguous DMA buffers that have more than 1 sgt
> > entries. In the end I gave up as I kept finding issues and went for the
> > drm_gem_cma_prime_import_sg_table() changes. Maybe you need to do a
> > similar change in the function to bypass some requirements if the driver
> > signals that it can accept relaxed requirements?
>
> As explained above I've considered reworking the helpers (to do it properly I
> think I'll likely need to rework drm_prime.c too), but I'd first like to know
> whether there's an interest for that.
Probably if you can express somehow the fact that the DU doesn't do any
memory access it might be interesting to other drivers. Otherwise you're
probably tied to having a driver specific version of the hook.
Best regards,
Liviu
>
> > > + cma_obj->sgt = sgt;
> > > +
> > > + return gem_obj;
> > > +
> > > +error:
> > > + kfree(cma_obj);
> > > + return ERR_PTR(ret);
> > > +}
>
> [snip]
>
> --
> Regards,
>
> Laurent Pinchart
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH/RFC 2/2] drm: rcar-du: Allow importing non-contiguous dma-buf with VSP
@ 2017-11-14 9:27 ` Liviu Dudau
0 siblings, 0 replies; 17+ messages in thread
From: Liviu Dudau @ 2017-11-14 9:27 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-renesas-soc, Daniel Vetter, Laurent Pinchart,
Kieran Bingham, dri-devel
On Tue, Nov 14, 2017 at 05:34:14AM +0200, Laurent Pinchart wrote:
> Hi Liviu,
Hi Laurent,
>
> On Monday, 13 November 2017 13:53:14 EET Liviu Dudau wrote:
> > On Mon, Nov 13, 2017 at 12:32:28PM +0200, Laurent Pinchart wrote:
> > > When the DU sources its frames from a VSP, it performs no memory access
> > > and thus has no requirements on imported dma-buf memory types. In
> > > particular the DU could import a physically non-contiguous buffer that
> > > would later be mapped contiguously through the VSP IOMMU.
> > >
> > > This use case isn't supported at the moment as the GEM CMA helpers will
> > > reject any non-contiguous buffer, and the DU isn't connected to an IOMMU
> > > that can make the buffer contiguous for DMA. Fix this by implementing a
> > > custom .gem_prime_import_sg_table() operation that accepts all imported
> > > dma-buf regardless of the number of scatterlist entries.
> >
> > This patch raises the question of why use CMA at all if you can accept
> > any kind of buffers.
>
> Both the DU and VSP require DMA contiguous memory. On R-Car Gen2 the DU
> performs DMA, and thus uses the GEM CMA helpers. On Gen3 it delegates DMA to
> the external VSP composer, and still uses the GEM CMA helpers to ensure that
> memory will be DMA contiguous for the VSP. The problem arises when physically
> non-contiguous dma-buf buffers are imported, they can be mapped contiguously
> to the VSP (assuming an IOMMU is present) but not to the DU (as there's no
> IOMMU).
>
> The situation is particularly messy due to the fact that I have one VSP
> instance per CRTC, each connected to its own IOMMU channel. The driver thus
> doesn't know when allocating GEM objects to which struct device they need to
> be mapped. The DRM helpers don't support delayed mapping of memory. I'd like
> to know whether that's something I should fix properly (which would likely
> involve major reworking of the helpers) or hack around it in the DU driver.
I understand. Looks like you have your hands full with an "interesting"
architecture.
>
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > >
> > > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
> > > drivers/gpu/drm/rcar-du/rcar_du_kms.c | 39 ++++++++++++++++++++++++++++++
> > > drivers/gpu/drm/rcar-du/rcar_du_kms.h | 7 +++++++
> > > 3 files changed, 47 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 48c166f925a3..d999231f98c7
> > > 100644
> > > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > > @@ -289,7 +289,7 @@ static struct drm_driver rcar_du_driver = {
> > > .gem_prime_import = drm_gem_prime_import,
> > > .gem_prime_export = drm_gem_prime_export,
> > > .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
> > > - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
> > > + .gem_prime_import_sg_table = rcar_du_gem_prime_import_sg_table,
> > > .gem_prime_vmap = drm_gem_cma_prime_vmap,
> > > .gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> > > .gem_prime_mmap = drm_gem_cma_prime_mmap,
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > > b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 566d1a948c8f..2dd0c2ba047d
> > > 100644
> > > --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>
> [snip]
>
> > > +struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct
> > > drm_device *dev,
> > > + struct dma_buf_attachment *attach,
> > > + struct sg_table *sgt)
> > > +{
> > > + struct rcar_du_device *rcdu = dev->dev_private;
> > > + struct drm_gem_cma_object *cma_obj;
> > > + struct drm_gem_object *gem_obj;
> > > + int ret;
> > > +
> > > + if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
> > > + return drm_gem_cma_prime_import_sg_table(dev, attach, sgt);
> > > +
> > > + /* Create a CMA GEM buffer. */
> > > + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
> > > + if (!cma_obj)
> > > + return ERR_PTR(-ENOMEM);
> > > + gem_obj = &cma_obj->base;
> > > +
> > > + ret = drm_gem_object_init(dev, gem_obj, attach->dmabuf->size);
> > > + if (ret)
> > > + goto error;
> > > +
> > > + ret = drm_gem_create_mmap_offset(gem_obj);
> > > + if (ret) {
> > > + drm_gem_object_release(gem_obj);
> > > + goto error;
> > > + }
> > > +
> > > + cma_obj->paddr = 0;
> >
> > This is going to break drm_gem_cma_describe() if you are using it
>
> As far as I can tell drm_gem_cma_describe() will just print paddr = 0, it
> won't crash.
I'm sorry, didn't meant it will crash, just print (possibly useless)
zero value.
>
> > plus the rcar_du_plane_setup_scanout() unless I'm missing something besides
> > familiarity with the RCAR driver code :)
>
> rcar_du_plane_setup_scanout() is only called when !rcar_du_has(rcdu,
> RCAR_DU_FEATURE_VSP1_SOURCE) (that is on Gen3 hardware, or on Gen2 when I
> artificially disable all DMA from the DU and use external composers to emulate
> Gen3 behaviour for testing purpose), in which case this function calls
> drm_gem_cma_prime_import_sg_table().
Thanks for the explanation!
>
> > This function looks very similar to what I tried to do for mali-dp to
> > allow the import of contiguous DMA buffers that have more than 1 sgt
> > entries. In the end I gave up as I kept finding issues and went for the
> > drm_gem_cma_prime_import_sg_table() changes. Maybe you need to do a
> > similar change in the function to bypass some requirements if the driver
> > signals that it can accept relaxed requirements?
>
> As explained above I've considered reworking the helpers (to do it properly I
> think I'll likely need to rework drm_prime.c too), but I'd first like to know
> whether there's an interest for that.
Probably if you can express somehow the fact that the DU doesn't do any
memory access it might be interesting to other drivers. Otherwise you're
probably tied to having a driver specific version of the hook.
Best regards,
Liviu
>
> > > + cma_obj->sgt = sgt;
> > > +
> > > + return gem_obj;
> > > +
> > > +error:
> > > + kfree(cma_obj);
> > > + return ERR_PTR(ret);
> > > +}
>
> [snip]
>
> --
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH/RFC v1.1] drm: rcar-du: Allow importing non-contiguous dma-buf with VSP
2017-11-13 10:32 ` Laurent Pinchart
(?)
(?)
@ 2017-12-13 22:46 ` Laurent Pinchart
-1 siblings, 0 replies; 17+ messages in thread
From: Laurent Pinchart @ 2017-12-13 22:46 UTC (permalink / raw)
To: dri-devel; +Cc: linux-renesas-soc, Daniel Vetter, Liviu Dudau, Kieran Bingham
When the DU sources its frames from a VSP, it performs no memory access
and thus has no requirements on imported dma-buf memory types. In
particular the DU could import a physically non-contiguous buffer that
would later be mapped contiguously through the VSP IOMMU.
This use case isn't supported at the moment as the GEM CMA helpers will
reject any non-contiguous buffer, and the DU isn't connected to an IOMMU
that can make the buffer contiguous for DMA. Fix this by implementing a
custom .gem_prime_import_sg_table() operation that accepts all imported
dma-buf regardless of the number of scatterlist entries.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
Changes since v1:
- Duplicate the imported scatter gather table in
rcar_du_vsp_plane_prepare_fb()
This patch fixes a bug of the previous version and is posted in case anyone
would like to test the implementation. I still plan to give Noralf's GEM
library a try as an alternative approach to this series.
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +-
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 39 +++++++++++++++++++++++++++++++++++
drivers/gpu/drm/rcar-du/rcar_du_kms.h | 7 +++++++
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 34 ++++++++++++++++++++++++++----
4 files changed, 77 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 48c166f925a3..d999231f98c7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -289,7 +289,7 @@ static struct drm_driver rcar_du_driver = {
.gem_prime_import = drm_gem_prime_import,
.gem_prime_export = drm_gem_prime_export,
.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
- .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
+ .gem_prime_import_sg_table = rcar_du_gem_prime_import_sg_table,
.gem_prime_vmap = drm_gem_cma_prime_vmap,
.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
.gem_prime_mmap = drm_gem_cma_prime_mmap,
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 566d1a948c8f..2dd0c2ba047d 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -20,6 +20,7 @@
#include <drm/drm_gem_cma_helper.h>
#include <drm/drm_gem_framebuffer_helper.h>
+#include <linux/dma-buf.h>
#include <linux/of_graph.h>
#include <linux/wait.h>
@@ -148,6 +149,44 @@ const struct rcar_du_format_info *rcar_du_format_info(u32 fourcc)
* Frame buffer
*/
+struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct drm_device *dev,
+ struct dma_buf_attachment *attach,
+ struct sg_table *sgt)
+{
+ struct rcar_du_device *rcdu = dev->dev_private;
+ struct drm_gem_cma_object *cma_obj;
+ struct drm_gem_object *gem_obj;
+ int ret;
+
+ if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
+ return drm_gem_cma_prime_import_sg_table(dev, attach, sgt);
+
+ /* Create a CMA GEM buffer. */
+ cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
+ if (!cma_obj)
+ return ERR_PTR(-ENOMEM);
+ gem_obj = &cma_obj->base;
+
+ ret = drm_gem_object_init(dev, gem_obj, attach->dmabuf->size);
+ if (ret)
+ goto error;
+
+ ret = drm_gem_create_mmap_offset(gem_obj);
+ if (ret) {
+ drm_gem_object_release(gem_obj);
+ goto error;
+ }
+
+ cma_obj->paddr = 0;
+ cma_obj->sgt = sgt;
+
+ return gem_obj;
+
+error:
+ kfree(cma_obj);
+ return ERR_PTR(ret);
+}
+
int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
struct drm_mode_create_dumb *args)
{
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.h b/drivers/gpu/drm/rcar-du/rcar_du_kms.h
index 07951d5fe38b..10b2bb0f0df9 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.h
@@ -16,10 +16,13 @@
#include <linux/types.h>
+struct dma_buf_attachment;
struct drm_file;
struct drm_device;
+struct drm_gem_object;
struct drm_mode_create_dumb;
struct rcar_du_device;
+struct sg_table;
struct rcar_du_format_info {
u32 fourcc;
@@ -36,4 +39,8 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu);
int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
struct drm_mode_create_dumb *args);
+struct drm_gem_object *rcar_du_gem_prime_import_sg_table(struct drm_device *dev,
+ struct dma_buf_attachment *attach,
+ struct sg_table *sgt);
+
#endif /* __RCAR_DU_KMS_H__ */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 2c260c33840b..73fdc814aa39 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -224,10 +224,36 @@ static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
drm_fb_cma_get_gem_obj(state->fb, i);
struct sg_table *sgt = &rstate->sg_tables[i];
- ret = dma_get_sgtable(rcdu->dev, sgt, gem->vaddr, gem->paddr,
- gem->base.size);
- if (ret)
- goto fail;
+ if (gem->sgt) {
+ struct scatterlist *src;
+ struct scatterlist *dst;
+
+ /*
+ * If the GEM buffer has a scatter gather table, it has
+ * been imported from a dma-buf and has no physical
+ * address as it might not be physically contiguous.
+ * Copy the original scatter gather table to map it to
+ * the VSP.
+ */
+ ret = sg_alloc_table(sgt, gem->sgt->orig_nents,
+ GFP_KERNEL);
+ if (ret)
+ goto fail;
+
+ src = gem->sgt->sgl;
+ dst = sgt->sgl;
+ for (i = 0; i < gem->sgt->orig_nents; ++i) {
+ sg_set_page(dst, sg_page(src), src->length,
+ src->offset);
+ src = sg_next(src);
+ dst = sg_next(dst);
+ }
+ } else {
+ ret = dma_get_sgtable(rcdu->dev, sgt, gem->vaddr,
+ gem->paddr, gem->base.size);
+ if (ret)
+ goto fail;
+ }
ret = vsp1_du_map_sg(vsp->vsp, sgt);
if (!ret) {
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 17+ messages in thread