[PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP

[PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP

[Laurent Pinchart]

Hello,

This patch series fixes the rcar-du-drm driver to support VSP plane sources
with an IOMMU. It is available for convenience at

	git://linuxtv.org/pinchartl/media.git iommu/devel/du

On R-Car Gen3 the DU has no direct memory access but sources planes through
VSP instances. When an IOMMU is inserted between the VSP and memory, the DU
framebuffers need to be DMA mapped using the VSP device, not the DU device as
currently done. The same situation can also be reproduced on Gen2 hardware by
linking the VSP to the DU in DT [1], effectively disabling direct memory
access by the DU.

The situation is made quite complex by the fact that different planes can be
connected to different DU instances, and thus served by different IOMMUs (or,
in practice on existing hardware, by the same IOMMU but through different
micro-TLBs). We thus can't allocate and map buffers to the right device in a
single dma_alloc_wc() operation as done in the DRM CMA GEM helpers.

However, on such setups, the DU DT node doesn't reference IOMMUs as the DU
does not perform any direct memory access. We can thus keep the GEM object
allocation unchanged, and the DMA addresses that we receive in the DU driver
will be physical addresses. Those buffers then need to be mapped to the VSP
device when they are associated with planes. Fortunately the atomic framework
provides two plane helper operations, .prepare_fb() and .cleanup_fb() that we
can use for this purpose.

The reality is slightly more complex than this on Gen3, as an FCP device
instance sits between VSP instances and memory. It is the FCP devices that are
connected to the IOMMUs, and buffer mapping thus need to be performed using
the FCP devices. This isn't required on Gen2 as the platforms don't have any
FCPs.

Patches 1/6 and 2/6 unconstify the state argument to the .prepare_fb() and
.cleanup_fb() operations, to allow storing the mapped buffer addresses in the
state. Patches 3/6 and 4/6 then extend the rcar-fcp driver API to expose the
FCP struct device. Patch 5/6 extends the vsp1 driver API to allow mapping a
scatter-gather list to the VSP, with the implementation using the FCP devices
instead when available. Patch 6/6 then use the vsp1 mapping API in the
rcar-du-drm driver to map and unmap buffers when needed.

The series has been tested on Gen2 (Lager) only as the Gen3 IOMMU is known to
be broken.

A possible improvement is to modify the GEM object allocation mechanism to use
non-contiguous memory when the DU driver detects that all the VSP instances it
is connected to use an IOMMU (possibly through FCP devices).

An issue has been noticed with synchronization between page flip and VSP
operation. Buffers get unmapped (and possibly freed) before the VSP is done
reading them. The problem isn't new, but is much more noticeable with IOMMU
support enabled as any hardware access to unmapped memory generates an IOMMU
page fault immediately.

The series unfortunately contain a dependency between DRM and V4L2 patches,
complicating upstream merge. As there's no urgency to merge patch 6/6 due to
the IOMMU being broken on Gen3 at the moment, I propose merging patches
1/6-2/6 and 3/6-5/6 independently for the next kernel release.

I would particularly appreciate feedback on the APIs introduced by patches 4/6
and 5/6.

[1] https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg06589.html

Laurent Pinchart (6):
  drm: Don't implement empty prepare_fb()/cleanup_fb()
  drm: Unconstify state argument to prepare_fb()/cleanup_fb()
  v4l: rcar-fcp: Don't get/put module reference
  v4l: rcar-fcp: Add an API to retrieve the FCP device
  v4l: vsp1: Add API to map and unmap DRM buffers through the VSP
  drm: rcar-du: Map memory through the VSP device

 drivers/gpu/drm/arc/arcpgu_crtc.c               |  2 -
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  4 +-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     | 15 -----
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 15 -----
 drivers/gpu/drm/i915/intel_display.c            |  4 +-
 drivers/gpu/drm/i915/intel_drv.h                |  4 +-
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c       |  4 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c       |  4 +-
 drivers/gpu/drm/omapdrm/omap_plane.c            |  4 +-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c           | 74 +++++++++++++++++++++++--
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h           |  2 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c     |  4 +-
 drivers/gpu/drm/tegra/dc.c                      | 17 ------
 drivers/gpu/drm/vc4/vc4_plane.c                 |  2 -
 drivers/media/platform/rcar-fcp.c               | 17 +++---
 drivers/media/platform/vsp1/vsp1_drm.c          | 24 ++++++++
 include/drm/drm_modeset_helper_vtables.h        |  4 +-
 include/media/rcar-fcp.h                        |  5 ++
 include/media/vsp1.h                            |  3 +
 19 files changed, 126 insertions(+), 82 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP

[Laurent Pinchart]

Hello,

This patch series fixes the rcar-du-drm driver to support VSP plane sources
with an IOMMU. It is available for convenience at

	git://linuxtv.org/pinchartl/media.git iommu/devel/du

On R-Car Gen3 the DU has no direct memory access but sources planes through
VSP instances. When an IOMMU is inserted between the VSP and memory, the DU
framebuffers need to be DMA mapped using the VSP device, not the DU device as
currently done. The same situation can also be reproduced on Gen2 hardware by
linking the VSP to the DU in DT [1], effectively disabling direct memory
access by the DU.

The situation is made quite complex by the fact that different planes can be
connected to different DU instances, and thus served by different IOMMUs (or,
in practice on existing hardware, by the same IOMMU but through different
micro-TLBs). We thus can't allocate and map buffers to the right device in a
single dma_alloc_wc() operation as done in the DRM CMA GEM helpers.

However, on such setups, the DU DT node doesn't reference IOMMUs as the DU
does not perform any direct memory access. We can thus keep the GEM object
allocation unchanged, and the DMA addresses that we receive in the DU driver
will be physical addresses. Those buffers then need to be mapped to the VSP
device when they are associated with planes. Fortunately the atomic framework
provides two plane helper operations, .prepare_fb() and .cleanup_fb() that we
can use for this purpose.

The reality is slightly more complex than this on Gen3, as an FCP device
instance sits between VSP instances and memory. It is the FCP devices that are
connected to the IOMMUs, and buffer mapping thus need to be performed using
the FCP devices. This isn't required on Gen2 as the platforms don't have any
FCPs.

Patches 1/6 and 2/6 unconstify the state argument to the .prepare_fb() and
.cleanup_fb() operations, to allow storing the mapped buffer addresses in the
state. Patches 3/6 and 4/6 then extend the rcar-fcp driver API to expose the
FCP struct device. Patch 5/6 extends the vsp1 driver API to allow mapping a
scatter-gather list to the VSP, with the implementation using the FCP devices
instead when available. Patch 6/6 then use the vsp1 mapping API in the
rcar-du-drm driver to map and unmap buffers when needed.

The series has been tested on Gen2 (Lager) only as the Gen3 IOMMU is known to
be broken.

A possible improvement is to modify the GEM object allocation mechanism to use
non-contiguous memory when the DU driver detects that all the VSP instances it
is connected to use an IOMMU (possibly through FCP devices).

An issue has been noticed with synchronization between page flip and VSP
operation. Buffers get unmapped (and possibly freed) before the VSP is done
reading them. The problem isn't new, but is much more noticeable with IOMMU
support enabled as any hardware access to unmapped memory generates an IOMMU
page fault immediately.

The series unfortunately contain a dependency between DRM and V4L2 patches,
complicating upstream merge. As there's no urgency to merge patch 6/6 due to
the IOMMU being broken on Gen3 at the moment, I propose merging patches
1/6-2/6 and 3/6-5/6 independently for the next kernel release.

I would particularly appreciate feedback on the APIs introduced by patches 4/6
and 5/6.

[1] https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg06589.html

Laurent Pinchart (6):
  drm: Don't implement empty prepare_fb()/cleanup_fb()
  drm: Unconstify state argument to prepare_fb()/cleanup_fb()
  v4l: rcar-fcp: Don't get/put module reference
  v4l: rcar-fcp: Add an API to retrieve the FCP device
  v4l: vsp1: Add API to map and unmap DRM buffers through the VSP
  drm: rcar-du: Map memory through the VSP device

 drivers/gpu/drm/arc/arcpgu_crtc.c               |  2 -
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  4 +-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     | 15 -----
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 15 -----
 drivers/gpu/drm/i915/intel_display.c            |  4 +-
 drivers/gpu/drm/i915/intel_drv.h                |  4 +-
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c       |  4 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c       |  4 +-
 drivers/gpu/drm/omapdrm/omap_plane.c            |  4 +-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c           | 74 +++++++++++++++++++++++--
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h           |  2 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c     |  4 +-
 drivers/gpu/drm/tegra/dc.c                      | 17 ------
 drivers/gpu/drm/vc4/vc4_plane.c                 |  2 -
 drivers/media/platform/rcar-fcp.c               | 17 +++---
 drivers/media/platform/vsp1/vsp1_drm.c          | 24 ++++++++
 include/drm/drm_modeset_helper_vtables.h        |  4 +-
 include/media/rcar-fcp.h                        |  5 ++
 include/media/vsp1.h                            |  3 +
 19 files changed, 126 insertions(+), 82 deletions(-)

-- 
Regards,

Laurent Pinchart

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

[PATCH 1/6] drm: Don't implement empty prepare_fb()/cleanup_fb()

[Laurent Pinchart]

The plane .prepare_fb() and .cleanup_fb() helpers are optional, there's
no need to implement empty stubs, and no need to explicitly set the
function pointers to NULL either.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/arc/arcpgu_crtc.c               |  2 --
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     | 15 ---------------
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 15 ---------------
 drivers/gpu/drm/tegra/dc.c                      | 17 -----------------
 drivers/gpu/drm/vc4/vc4_plane.c                 |  2 --
 5 files changed, 51 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index ee0a61c2861b..7130b044b004 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -183,8 +183,6 @@ static void arc_pgu_plane_atomic_update(struct drm_plane *plane,
 }
 
 static const struct drm_plane_helper_funcs arc_pgu_plane_helper_funcs = {
-	.prepare_fb = NULL,
-	.cleanup_fb = NULL,
 	.atomic_update = arc_pgu_plane_atomic_update,
 };
 
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index e50467a0deb0..a7e5486bd1e9 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -169,25 +169,10 @@ static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
 	return;
 }
 
-static void
-fsl_dcu_drm_plane_cleanup_fb(struct drm_plane *plane,
-			     const struct drm_plane_state *new_state)
-{
-}
-
-static int
-fsl_dcu_drm_plane_prepare_fb(struct drm_plane *plane,
-			     const struct drm_plane_state *new_state)
-{
-	return 0;
-}
-
 static const struct drm_plane_helper_funcs fsl_dcu_drm_plane_helper_funcs = {
 	.atomic_check = fsl_dcu_drm_plane_atomic_check,
 	.atomic_disable = fsl_dcu_drm_plane_atomic_disable,
 	.atomic_update = fsl_dcu_drm_plane_atomic_update,
-	.cleanup_fb = fsl_dcu_drm_plane_cleanup_fb,
-	.prepare_fb = fsl_dcu_drm_plane_prepare_fb,
 };
 
 static void fsl_dcu_drm_plane_destroy(struct drm_plane *plane)
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
index c3707d47cd89..635ead039b8b 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
@@ -815,19 +815,6 @@ static void ade_disable_channel(struct ade_plane *aplane)
 	ade_compositor_routing_disable(base, ch);
 }
 
-static int ade_plane_prepare_fb(struct drm_plane *plane,
-				const struct drm_plane_state *new_state)
-{
-	/* do nothing */
-	return 0;
-}
-
-static void ade_plane_cleanup_fb(struct drm_plane *plane,
-				 const struct drm_plane_state *old_state)
-{
-	/* do nothing */
-}
-
 static int ade_plane_atomic_check(struct drm_plane *plane,
 				  struct drm_plane_state *state)
 {
@@ -895,8 +882,6 @@ static void ade_plane_atomic_disable(struct drm_plane *plane,
 }
 
 static const struct drm_plane_helper_funcs ade_plane_helper_funcs = {
-	.prepare_fb = ade_plane_prepare_fb,
-	.cleanup_fb = ade_plane_cleanup_fb,
 	.atomic_check = ade_plane_atomic_check,
 	.atomic_update = ade_plane_atomic_update,
 	.atomic_disable = ade_plane_atomic_disable,
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 8495bd01b544..3de7ce33d3d4 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -480,17 +480,6 @@ static const struct drm_plane_funcs tegra_primary_plane_funcs = {
 	.atomic_destroy_state = tegra_plane_atomic_destroy_state,
 };
 
-static int tegra_plane_prepare_fb(struct drm_plane *plane,
-				  const struct drm_plane_state *new_state)
-{
-	return 0;
-}
-
-static void tegra_plane_cleanup_fb(struct drm_plane *plane,
-				   const struct drm_plane_state *old_fb)
-{
-}
-
 static int tegra_plane_state_add(struct tegra_plane *plane,
 				 struct drm_plane_state *state)
 {
@@ -624,8 +613,6 @@ static void tegra_plane_atomic_disable(struct drm_plane *plane,
 }
 
 static const struct drm_plane_helper_funcs tegra_primary_plane_helper_funcs = {
-	.prepare_fb = tegra_plane_prepare_fb,
-	.cleanup_fb = tegra_plane_cleanup_fb,
 	.atomic_check = tegra_plane_atomic_check,
 	.atomic_update = tegra_plane_atomic_update,
 	.atomic_disable = tegra_plane_atomic_disable,
@@ -796,8 +783,6 @@ static const struct drm_plane_funcs tegra_cursor_plane_funcs = {
 };
 
 static const struct drm_plane_helper_funcs tegra_cursor_plane_helper_funcs = {
-	.prepare_fb = tegra_plane_prepare_fb,
-	.cleanup_fb = tegra_plane_cleanup_fb,
 	.atomic_check = tegra_cursor_atomic_check,
 	.atomic_update = tegra_cursor_atomic_update,
 	.atomic_disable = tegra_cursor_atomic_disable,
@@ -866,8 +851,6 @@ static const uint32_t tegra_overlay_plane_formats[] = {
 };
 
 static const struct drm_plane_helper_funcs tegra_overlay_plane_helper_funcs = {
-	.prepare_fb = tegra_plane_prepare_fb,
-	.cleanup_fb = tegra_plane_cleanup_fb,
 	.atomic_check = tegra_plane_atomic_check,
 	.atomic_update = tegra_plane_atomic_update,
 	.atomic_disable = tegra_plane_atomic_disable,
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 29e4b400e25e..881bf489478b 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -735,8 +735,6 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb)
 }
 
 static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
-	.prepare_fb = NULL,
-	.cleanup_fb = NULL,
 	.atomic_check = vc4_plane_atomic_check,
 	.atomic_update = vc4_plane_atomic_update,
 };
-- 
Regards,

Laurent Pinchart

[PATCH 1/6] drm: Don't implement empty prepare_fb()/cleanup_fb()

[Laurent Pinchart]

The plane .prepare_fb() and .cleanup_fb() helpers are optional, there's
no need to implement empty stubs, and no need to explicitly set the
function pointers to NULL either.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/arc/arcpgu_crtc.c               |  2 --
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     | 15 ---------------
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 15 ---------------
 drivers/gpu/drm/tegra/dc.c                      | 17 -----------------
 drivers/gpu/drm/vc4/vc4_plane.c                 |  2 --
 5 files changed, 51 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
index ee0a61c2861b..7130b044b004 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -183,8 +183,6 @@ static void arc_pgu_plane_atomic_update(struct drm_plane *plane,
 }
 
 static const struct drm_plane_helper_funcs arc_pgu_plane_helper_funcs = {
-	.prepare_fb = NULL,
-	.cleanup_fb = NULL,
 	.atomic_update = arc_pgu_plane_atomic_update,
 };
 
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index e50467a0deb0..a7e5486bd1e9 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -169,25 +169,10 @@ static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
 	return;
 }
 
-static void
-fsl_dcu_drm_plane_cleanup_fb(struct drm_plane *plane,
-			     const struct drm_plane_state *new_state)
-{
-}
-
-static int
-fsl_dcu_drm_plane_prepare_fb(struct drm_plane *plane,
-			     const struct drm_plane_state *new_state)
-{
-	return 0;
-}
-
 static const struct drm_plane_helper_funcs fsl_dcu_drm_plane_helper_funcs = {
 	.atomic_check = fsl_dcu_drm_plane_atomic_check,
 	.atomic_disable = fsl_dcu_drm_plane_atomic_disable,
 	.atomic_update = fsl_dcu_drm_plane_atomic_update,
-	.cleanup_fb = fsl_dcu_drm_plane_cleanup_fb,
-	.prepare_fb = fsl_dcu_drm_plane_prepare_fb,
 };
 
 static void fsl_dcu_drm_plane_destroy(struct drm_plane *plane)
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
index c3707d47cd89..635ead039b8b 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
@@ -815,19 +815,6 @@ static void ade_disable_channel(struct ade_plane *aplane)
 	ade_compositor_routing_disable(base, ch);
 }
 
-static int ade_plane_prepare_fb(struct drm_plane *plane,
-				const struct drm_plane_state *new_state)
-{
-	/* do nothing */
-	return 0;
-}
-
-static void ade_plane_cleanup_fb(struct drm_plane *plane,
-				 const struct drm_plane_state *old_state)
-{
-	/* do nothing */
-}
-
 static int ade_plane_atomic_check(struct drm_plane *plane,
 				  struct drm_plane_state *state)
 {
@@ -895,8 +882,6 @@ static void ade_plane_atomic_disable(struct drm_plane *plane,
 }
 
 static const struct drm_plane_helper_funcs ade_plane_helper_funcs = {
-	.prepare_fb = ade_plane_prepare_fb,
-	.cleanup_fb = ade_plane_cleanup_fb,
 	.atomic_check = ade_plane_atomic_check,
 	.atomic_update = ade_plane_atomic_update,
 	.atomic_disable = ade_plane_atomic_disable,
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 8495bd01b544..3de7ce33d3d4 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -480,17 +480,6 @@ static const struct drm_plane_funcs tegra_primary_plane_funcs = {
 	.atomic_destroy_state = tegra_plane_atomic_destroy_state,
 };
 
-static int tegra_plane_prepare_fb(struct drm_plane *plane,
-				  const struct drm_plane_state *new_state)
-{
-	return 0;
-}
-
-static void tegra_plane_cleanup_fb(struct drm_plane *plane,
-				   const struct drm_plane_state *old_fb)
-{
-}
-
 static int tegra_plane_state_add(struct tegra_plane *plane,
 				 struct drm_plane_state *state)
 {
@@ -624,8 +613,6 @@ static void tegra_plane_atomic_disable(struct drm_plane *plane,
 }
 
 static const struct drm_plane_helper_funcs tegra_primary_plane_helper_funcs = {
-	.prepare_fb = tegra_plane_prepare_fb,
-	.cleanup_fb = tegra_plane_cleanup_fb,
 	.atomic_check = tegra_plane_atomic_check,
 	.atomic_update = tegra_plane_atomic_update,
 	.atomic_disable = tegra_plane_atomic_disable,
@@ -796,8 +783,6 @@ static const struct drm_plane_funcs tegra_cursor_plane_funcs = {
 };
 
 static const struct drm_plane_helper_funcs tegra_cursor_plane_helper_funcs = {
-	.prepare_fb = tegra_plane_prepare_fb,
-	.cleanup_fb = tegra_plane_cleanup_fb,
 	.atomic_check = tegra_cursor_atomic_check,
 	.atomic_update = tegra_cursor_atomic_update,
 	.atomic_disable = tegra_cursor_atomic_disable,
@@ -866,8 +851,6 @@ static const uint32_t tegra_overlay_plane_formats[] = {
 };
 
 static const struct drm_plane_helper_funcs tegra_overlay_plane_helper_funcs = {
-	.prepare_fb = tegra_plane_prepare_fb,
-	.cleanup_fb = tegra_plane_cleanup_fb,
 	.atomic_check = tegra_plane_atomic_check,
 	.atomic_update = tegra_plane_atomic_update,
 	.atomic_disable = tegra_plane_atomic_disable,
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 29e4b400e25e..881bf489478b 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -735,8 +735,6 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb)
 }
 
 static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
-	.prepare_fb = NULL,
-	.cleanup_fb = NULL,
 	.atomic_check = vc4_plane_atomic_check,
 	.atomic_update = vc4_plane_atomic_update,
 };
-- 
Regards,

Laurent Pinchart

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

[PATCH 2/6] drm: Unconstify state argument to prepare_fb()/cleanup_fb()

[Laurent Pinchart]

The functions might need to modify the state to store memory-related
data.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 4 ++--
 drivers/gpu/drm/i915/intel_display.c            | 4 ++--
 drivers/gpu/drm/i915/intel_drv.h                | 4 ++--
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c       | 4 ++--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c       | 4 ++--
 drivers/gpu/drm/omapdrm/omap_plane.c            | 4 ++--
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c     | 4 ++--
 include/drm/drm_modeset_helper_vtables.h        | 4 ++--
 8 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 016c191221f3..ae2b31dd9487 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -755,7 +755,7 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
 }
 
 static int atmel_hlcdc_plane_prepare_fb(struct drm_plane *p,
-					const struct drm_plane_state *new_state)
+					struct drm_plane_state *new_state)
 {
 	/*
 	 * FIXME: we should avoid this const -> non-const cast but it's
@@ -780,7 +780,7 @@ static int atmel_hlcdc_plane_prepare_fb(struct drm_plane *p,
 }
 
 static void atmel_hlcdc_plane_cleanup_fb(struct drm_plane *p,
-				const struct drm_plane_state *old_state)
+				struct drm_plane_state *old_state)
 {
 	/*
 	 * FIXME: we should avoid this const -> non-const cast but it's
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dcf93b3d4fb6..cf70de704b2a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13974,7 +13974,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
  */
 int
 intel_prepare_plane_fb(struct drm_plane *plane,
-		       const struct drm_plane_state *new_state)
+		       struct drm_plane_state *new_state)
 {
 	struct drm_device *dev = plane->dev;
 	struct drm_framebuffer *fb = new_state->fb;
@@ -14058,7 +14058,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
  */
 void
 intel_cleanup_plane_fb(struct drm_plane *plane,
-		       const struct drm_plane_state *old_state)
+		       struct drm_plane_state *old_state)
 {
 	struct drm_device *dev = plane->dev;
 	struct intel_plane_state *old_intel_state;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cc937a19b1ba..aa0d6679eff3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1238,9 +1238,9 @@ void intel_finish_page_flip_cs(struct drm_i915_private *dev_priv, int pipe);
 void intel_finish_page_flip_mmio(struct drm_i915_private *dev_priv, int pipe);
 void intel_check_page_flip(struct drm_i915_private *dev_priv, int pipe);
 int intel_prepare_plane_fb(struct drm_plane *plane,
-			   const struct drm_plane_state *new_state);
+			   struct drm_plane_state *new_state);
 void intel_cleanup_plane_fb(struct drm_plane *plane,
-			    const struct drm_plane_state *old_state);
+			    struct drm_plane_state *old_state);
 int intel_plane_atomic_get_property(struct drm_plane *plane,
 				    const struct drm_plane_state *state,
 				    struct drm_property *property,
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
index 9f96dfe67769..0689a06bec8e 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
@@ -99,7 +99,7 @@ static const struct drm_plane_funcs mdp4_plane_funcs = {
 };
 
 static int mdp4_plane_prepare_fb(struct drm_plane *plane,
-		const struct drm_plane_state *new_state)
+		struct drm_plane_state *new_state)
 {
 	struct mdp4_plane *mdp4_plane = to_mdp4_plane(plane);
 	struct mdp4_kms *mdp4_kms = get_kms(plane);
@@ -113,7 +113,7 @@ static int mdp4_plane_prepare_fb(struct drm_plane *plane,
 }
 
 static void mdp4_plane_cleanup_fb(struct drm_plane *plane,
-		const struct drm_plane_state *old_state)
+		struct drm_plane_state *old_state)
 {
 	struct mdp4_plane *mdp4_plane = to_mdp4_plane(plane);
 	struct mdp4_kms *mdp4_kms = get_kms(plane);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 432c09836b0e..1ef024df51a8 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -250,7 +250,7 @@ static const struct drm_plane_funcs mdp5_plane_funcs = {
 };
 
 static int mdp5_plane_prepare_fb(struct drm_plane *plane,
-		const struct drm_plane_state *new_state)
+		struct drm_plane_state *new_state)
 {
 	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
 	struct mdp5_kms *mdp5_kms = get_kms(plane);
@@ -264,7 +264,7 @@ static int mdp5_plane_prepare_fb(struct drm_plane *plane,
 }
 
 static void mdp5_plane_cleanup_fb(struct drm_plane *plane,
-		const struct drm_plane_state *old_state)
+		struct drm_plane_state *old_state)
 {
 	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
 	struct mdp5_kms *mdp5_kms = get_kms(plane);
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 5252ab720e70..bdfbb6181398 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -60,7 +60,7 @@ to_omap_plane_state(struct drm_plane_state *state)
 }
 
 static int omap_plane_prepare_fb(struct drm_plane *plane,
-				 const struct drm_plane_state *new_state)
+				 struct drm_plane_state *new_state)
 {
 	if (!new_state->fb)
 		return 0;
@@ -69,7 +69,7 @@ static int omap_plane_prepare_fb(struct drm_plane *plane,
 }
 
 static void omap_plane_cleanup_fb(struct drm_plane *plane,
-				  const struct drm_plane_state *old_state)
+				  struct drm_plane_state *old_state)
 {
 	if (old_state->fb)
 		omap_framebuffer_unpin(old_state->fb);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 91305eb7d312..a2466e37b0a1 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -569,7 +569,7 @@ static void vop_plane_destroy(struct drm_plane *plane)
 }
 
 static int vop_plane_prepare_fb(struct drm_plane *plane,
-				const struct drm_plane_state *new_state)
+				struct drm_plane_state *new_state)
 {
 	if (plane->state->fb)
 		drm_framebuffer_reference(plane->state->fb);
@@ -578,7 +578,7 @@ static int vop_plane_prepare_fb(struct drm_plane *plane,
 }
 
 static void vop_plane_cleanup_fb(struct drm_plane *plane,
-				 const struct drm_plane_state *old_state)
+				 struct drm_plane_state *old_state)
 {
 	if (old_state->fb)
 		drm_framebuffer_unreference(old_state->fb);
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index b55f21857a98..bfe145e602d2 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -826,7 +826,7 @@ struct drm_plane_helper_funcs {
 	 * everything else must complete successfully.
 	 */
 	int (*prepare_fb)(struct drm_plane *plane,
-			  const struct drm_plane_state *new_state);
+			  struct drm_plane_state *new_state);
 	/**
 	 * @cleanup_fb:
 	 *
@@ -837,7 +837,7 @@ struct drm_plane_helper_funcs {
 	 * transitional plane helpers, but it is optional.
 	 */
 	void (*cleanup_fb)(struct drm_plane *plane,
-			   const struct drm_plane_state *old_state);
+			   struct drm_plane_state *old_state);
 
 	/**
 	 * @atomic_check:
-- 
Regards,

Laurent Pinchart

[PATCH 2/6] drm: Unconstify state argument to prepare_fb()/cleanup_fb()

[Laurent Pinchart]

The functions might need to modify the state to store memory-related
data.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 4 ++--
 drivers/gpu/drm/i915/intel_display.c            | 4 ++--
 drivers/gpu/drm/i915/intel_drv.h                | 4 ++--
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c       | 4 ++--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c       | 4 ++--
 drivers/gpu/drm/omapdrm/omap_plane.c            | 4 ++--
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c     | 4 ++--
 include/drm/drm_modeset_helper_vtables.h        | 4 ++--
 8 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 016c191221f3..ae2b31dd9487 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -755,7 +755,7 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
 }
 
 static int atmel_hlcdc_plane_prepare_fb(struct drm_plane *p,
-					const struct drm_plane_state *new_state)
+					struct drm_plane_state *new_state)
 {
 	/*
 	 * FIXME: we should avoid this const -> non-const cast but it's
@@ -780,7 +780,7 @@ static int atmel_hlcdc_plane_prepare_fb(struct drm_plane *p,
 }
 
 static void atmel_hlcdc_plane_cleanup_fb(struct drm_plane *p,
-				const struct drm_plane_state *old_state)
+				struct drm_plane_state *old_state)
 {
 	/*
 	 * FIXME: we should avoid this const -> non-const cast but it's
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dcf93b3d4fb6..cf70de704b2a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13974,7 +13974,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
  */
 int
 intel_prepare_plane_fb(struct drm_plane *plane,
-		       const struct drm_plane_state *new_state)
+		       struct drm_plane_state *new_state)
 {
 	struct drm_device *dev = plane->dev;
 	struct drm_framebuffer *fb = new_state->fb;
@@ -14058,7 +14058,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
  */
 void
 intel_cleanup_plane_fb(struct drm_plane *plane,
-		       const struct drm_plane_state *old_state)
+		       struct drm_plane_state *old_state)
 {
 	struct drm_device *dev = plane->dev;
 	struct intel_plane_state *old_intel_state;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cc937a19b1ba..aa0d6679eff3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1238,9 +1238,9 @@ void intel_finish_page_flip_cs(struct drm_i915_private *dev_priv, int pipe);
 void intel_finish_page_flip_mmio(struct drm_i915_private *dev_priv, int pipe);
 void intel_check_page_flip(struct drm_i915_private *dev_priv, int pipe);
 int intel_prepare_plane_fb(struct drm_plane *plane,
-			   const struct drm_plane_state *new_state);
+			   struct drm_plane_state *new_state);
 void intel_cleanup_plane_fb(struct drm_plane *plane,
-			    const struct drm_plane_state *old_state);
+			    struct drm_plane_state *old_state);
 int intel_plane_atomic_get_property(struct drm_plane *plane,
 				    const struct drm_plane_state *state,
 				    struct drm_property *property,
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
index 9f96dfe67769..0689a06bec8e 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
@@ -99,7 +99,7 @@ static const struct drm_plane_funcs mdp4_plane_funcs = {
 };
 
 static int mdp4_plane_prepare_fb(struct drm_plane *plane,
-		const struct drm_plane_state *new_state)
+		struct drm_plane_state *new_state)
 {
 	struct mdp4_plane *mdp4_plane = to_mdp4_plane(plane);
 	struct mdp4_kms *mdp4_kms = get_kms(plane);
@@ -113,7 +113,7 @@ static int mdp4_plane_prepare_fb(struct drm_plane *plane,
 }
 
 static void mdp4_plane_cleanup_fb(struct drm_plane *plane,
-		const struct drm_plane_state *old_state)
+		struct drm_plane_state *old_state)
 {
 	struct mdp4_plane *mdp4_plane = to_mdp4_plane(plane);
 	struct mdp4_kms *mdp4_kms = get_kms(plane);
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 432c09836b0e..1ef024df51a8 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -250,7 +250,7 @@ static const struct drm_plane_funcs mdp5_plane_funcs = {
 };
 
 static int mdp5_plane_prepare_fb(struct drm_plane *plane,
-		const struct drm_plane_state *new_state)
+		struct drm_plane_state *new_state)
 {
 	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
 	struct mdp5_kms *mdp5_kms = get_kms(plane);
@@ -264,7 +264,7 @@ static int mdp5_plane_prepare_fb(struct drm_plane *plane,
 }
 
 static void mdp5_plane_cleanup_fb(struct drm_plane *plane,
-		const struct drm_plane_state *old_state)
+		struct drm_plane_state *old_state)
 {
 	struct mdp5_plane *mdp5_plane = to_mdp5_plane(plane);
 	struct mdp5_kms *mdp5_kms = get_kms(plane);
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 5252ab720e70..bdfbb6181398 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -60,7 +60,7 @@ to_omap_plane_state(struct drm_plane_state *state)
 }
 
 static int omap_plane_prepare_fb(struct drm_plane *plane,
-				 const struct drm_plane_state *new_state)
+				 struct drm_plane_state *new_state)
 {
 	if (!new_state->fb)
 		return 0;
@@ -69,7 +69,7 @@ static int omap_plane_prepare_fb(struct drm_plane *plane,
 }
 
 static void omap_plane_cleanup_fb(struct drm_plane *plane,
-				  const struct drm_plane_state *old_state)
+				  struct drm_plane_state *old_state)
 {
 	if (old_state->fb)
 		omap_framebuffer_unpin(old_state->fb);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 91305eb7d312..a2466e37b0a1 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -569,7 +569,7 @@ static void vop_plane_destroy(struct drm_plane *plane)
 }
 
 static int vop_plane_prepare_fb(struct drm_plane *plane,
-				const struct drm_plane_state *new_state)
+				struct drm_plane_state *new_state)
 {
 	if (plane->state->fb)
 		drm_framebuffer_reference(plane->state->fb);
@@ -578,7 +578,7 @@ static int vop_plane_prepare_fb(struct drm_plane *plane,
 }
 
 static void vop_plane_cleanup_fb(struct drm_plane *plane,
-				 const struct drm_plane_state *old_state)
+				 struct drm_plane_state *old_state)
 {
 	if (old_state->fb)
 		drm_framebuffer_unreference(old_state->fb);
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index b55f21857a98..bfe145e602d2 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -826,7 +826,7 @@ struct drm_plane_helper_funcs {
 	 * everything else must complete successfully.
 	 */
 	int (*prepare_fb)(struct drm_plane *plane,
-			  const struct drm_plane_state *new_state);
+			  struct drm_plane_state *new_state);
 	/**
 	 * @cleanup_fb:
 	 *
@@ -837,7 +837,7 @@ struct drm_plane_helper_funcs {
 	 * transitional plane helpers, but it is optional.
 	 */
 	void (*cleanup_fb)(struct drm_plane *plane,
-			   const struct drm_plane_state *old_state);
+			   struct drm_plane_state *old_state);
 
 	/**
 	 * @atomic_check:
-- 
Regards,

Laurent Pinchart

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

[PATCH 3/6] v4l: rcar-fcp: Don't get/put module reference

[Laurent Pinchart]

Direct callers of the FCP API hold a reference to the FCP module due to
module linkage, there's no need to take another one manually. Take a
reference to the device instead to ensure that it won't disappear being
the caller's back.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/rcar-fcp.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/rcar-fcp.c b/drivers/media/platform/rcar-fcp.c
index f7bcbf7677a0..7427be1c3741 100644
--- a/drivers/media/platform/rcar-fcp.c
+++ b/drivers/media/platform/rcar-fcp.c
@@ -53,14 +53,7 @@ struct rcar_fcp_device *rcar_fcp_get(const struct device_node *np)
 		if (fcp->dev->of_node != np)
 			continue;
 
-		/*
-		 * Make sure the module won't be unloaded behind our back. This
-		 * is a poor man's safety net, the module should really not be
-		 * unloaded while FCP users can be active.
-		 */
-		if (!try_module_get(fcp->dev->driver->owner))
-			fcp = NULL;
-
+		get_device(fcp->dev);
 		goto done;
 	}
 
@@ -81,7 +74,7 @@ EXPORT_SYMBOL_GPL(rcar_fcp_get);
 void rcar_fcp_put(struct rcar_fcp_device *fcp)
 {
 	if (fcp)
-		module_put(fcp->dev->driver->owner);
+		put_device(fcp->dev);
 }
 EXPORT_SYMBOL_GPL(rcar_fcp_put);
 
-- 
Regards,

Laurent Pinchart

[PATCH 3/6] v4l: rcar-fcp: Don't get/put module reference

[Laurent Pinchart]

Direct callers of the FCP API hold a reference to the FCP module due to
module linkage, there's no need to take another one manually. Take a
reference to the device instead to ensure that it won't disappear being
the caller's back.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/rcar-fcp.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/rcar-fcp.c b/drivers/media/platform/rcar-fcp.c
index f7bcbf7677a0..7427be1c3741 100644
--- a/drivers/media/platform/rcar-fcp.c
+++ b/drivers/media/platform/rcar-fcp.c
@@ -53,14 +53,7 @@ struct rcar_fcp_device *rcar_fcp_get(const struct device_node *np)
 		if (fcp->dev->of_node != np)
 			continue;
 
-		/*
-		 * Make sure the module won't be unloaded behind our back. This
-		 * is a poor man's safety net, the module should really not be
-		 * unloaded while FCP users can be active.
-		 */
-		if (!try_module_get(fcp->dev->driver->owner))
-			fcp = NULL;
-
+		get_device(fcp->dev);
 		goto done;
 	}
 
@@ -81,7 +74,7 @@ EXPORT_SYMBOL_GPL(rcar_fcp_get);
 void rcar_fcp_put(struct rcar_fcp_device *fcp)
 {
 	if (fcp)
-		module_put(fcp->dev->driver->owner);
+		put_device(fcp->dev);
 }
 EXPORT_SYMBOL_GPL(rcar_fcp_put);
 
-- 
Regards,

Laurent Pinchart

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

[PATCH 4/6] v4l: rcar-fcp: Add an API to retrieve the FCP device

[Laurent Pinchart]

The new rcar_fcp_get_device() function retrieves the struct device
related to the FCP device. This is useful to handle DMA mapping through
the right device.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/rcar-fcp.c | 6 ++++++
 include/media/rcar-fcp.h          | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/media/platform/rcar-fcp.c b/drivers/media/platform/rcar-fcp.c
index 7427be1c3741..60e7ec17d4e2 100644
--- a/drivers/media/platform/rcar-fcp.c
+++ b/drivers/media/platform/rcar-fcp.c
@@ -78,6 +78,12 @@ void rcar_fcp_put(struct rcar_fcp_device *fcp)
 }
 EXPORT_SYMBOL_GPL(rcar_fcp_put);
 
+struct device *rcar_fcp_get_device(struct rcar_fcp_device *fcp)
+{
+	return fcp->dev;
+}
+EXPORT_SYMBOL_GPL(rcar_fcp_get_device);
+
 /**
  * rcar_fcp_enable - Enable an FCP
  * @fcp: The FCP instance
diff --git a/include/media/rcar-fcp.h b/include/media/rcar-fcp.h
index 8723f05c6321..b60a7b176c37 100644
--- a/include/media/rcar-fcp.h
+++ b/include/media/rcar-fcp.h
@@ -19,6 +19,7 @@ struct rcar_fcp_device;
 #if IS_ENABLED(CONFIG_VIDEO_RENESAS_FCP)
 struct rcar_fcp_device *rcar_fcp_get(const struct device_node *np);
 void rcar_fcp_put(struct rcar_fcp_device *fcp);
+struct device *rcar_fcp_get_device(struct rcar_fcp_device *fcp);
 int rcar_fcp_enable(struct rcar_fcp_device *fcp);
 void rcar_fcp_disable(struct rcar_fcp_device *fcp);
 #else
@@ -27,6 +28,10 @@ static inline struct rcar_fcp_device *rcar_fcp_get(const struct device_node *np)
 	return ERR_PTR(-ENOENT);
 }
 static inline void rcar_fcp_put(struct rcar_fcp_device *fcp) { }
+static inline struct device *rcar_fcp_get_device(struct rcar_fcp_device *fcp)
+{
+	return NULL;
+}
 static inline int rcar_fcp_enable(struct rcar_fcp_device *fcp)
 {
 	return 0;
-- 
Regards,

Laurent Pinchart

[PATCH 4/6] v4l: rcar-fcp: Add an API to retrieve the FCP device

[Laurent Pinchart]

The new rcar_fcp_get_device() function retrieves the struct device
related to the FCP device. This is useful to handle DMA mapping through
the right device.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/rcar-fcp.c | 6 ++++++
 include/media/rcar-fcp.h          | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/media/platform/rcar-fcp.c b/drivers/media/platform/rcar-fcp.c
index 7427be1c3741..60e7ec17d4e2 100644
--- a/drivers/media/platform/rcar-fcp.c
+++ b/drivers/media/platform/rcar-fcp.c
@@ -78,6 +78,12 @@ void rcar_fcp_put(struct rcar_fcp_device *fcp)
 }
 EXPORT_SYMBOL_GPL(rcar_fcp_put);
 
+struct device *rcar_fcp_get_device(struct rcar_fcp_device *fcp)
+{
+	return fcp->dev;
+}
+EXPORT_SYMBOL_GPL(rcar_fcp_get_device);
+
 /**
  * rcar_fcp_enable - Enable an FCP
  * @fcp: The FCP instance
diff --git a/include/media/rcar-fcp.h b/include/media/rcar-fcp.h
index 8723f05c6321..b60a7b176c37 100644
--- a/include/media/rcar-fcp.h
+++ b/include/media/rcar-fcp.h
@@ -19,6 +19,7 @@ struct rcar_fcp_device;
 #if IS_ENABLED(CONFIG_VIDEO_RENESAS_FCP)
 struct rcar_fcp_device *rcar_fcp_get(const struct device_node *np);
 void rcar_fcp_put(struct rcar_fcp_device *fcp);
+struct device *rcar_fcp_get_device(struct rcar_fcp_device *fcp);
 int rcar_fcp_enable(struct rcar_fcp_device *fcp);
 void rcar_fcp_disable(struct rcar_fcp_device *fcp);
 #else
@@ -27,6 +28,10 @@ static inline struct rcar_fcp_device *rcar_fcp_get(const struct device_node *np)
 	return ERR_PTR(-ENOENT);
 }
 static inline void rcar_fcp_put(struct rcar_fcp_device *fcp) { }
+static inline struct device *rcar_fcp_get_device(struct rcar_fcp_device *fcp)
+{
+	return NULL;
+}
 static inline int rcar_fcp_enable(struct rcar_fcp_device *fcp)
 {
 	return 0;
-- 
Regards,

Laurent Pinchart

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

[PATCH 5/6] v4l: vsp1: Add API to map and unmap DRM buffers through the VSP

[Laurent Pinchart]

The display buffers must be mapped for DMA through the device that
performs memory access. Expose an API to map and unmap memory through
the VSP device to be used by the DU.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/vsp1/vsp1_drm.c | 24 ++++++++++++++++++++++++
 include/media/vsp1.h                   |  3 +++
 2 files changed, 27 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
index f76131b192a4..0472cb4bc2e4 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -12,9 +12,11 @@
  */
 
 #include <linux/device.h>
+#include <linux/dma-mapping.h>
 #include <linux/slab.h>
 
 #include <media/media-entity.h>
+#include <media/rcar-fcp.h>
 #include <media/v4l2-subdev.h>
 #include <media/vsp1.h>
 
@@ -521,6 +523,28 @@ void vsp1_du_atomic_flush(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(vsp1_du_atomic_flush);
 
+int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt)
+{
+	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+	struct device *map_dev;
+
+	map_dev = vsp1->fcp ? rcar_fcp_get_device(vsp1->fcp) : dev;
+
+	return dma_map_sg(map_dev, sgt->sgl, sgt->nents, DMA_TO_DEVICE);
+}
+EXPORT_SYMBOL_GPL(vsp1_du_map_sg);
+
+void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt)
+{
+	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+	struct device *map_dev;
+
+	map_dev = vsp1->fcp ? rcar_fcp_get_device(vsp1->fcp) : dev;
+
+	dma_unmap_sg(map_dev, sgt->sgl, sgt->nents, DMA_TO_DEVICE);
+}
+EXPORT_SYMBOL_GPL(vsp1_du_unmap_sg);
+
 /* -----------------------------------------------------------------------------
  * Initialization
  */
diff --git a/include/media/vsp1.h b/include/media/vsp1.h
index 458b400373d4..8d3d07a3715e 100644
--- a/include/media/vsp1.h
+++ b/include/media/vsp1.h
@@ -13,6 +13,7 @@
 #ifndef __MEDIA_VSP1_H__
 #define __MEDIA_VSP1_H__
 
+#include <linux/scatterlist.h>
 #include <linux/types.h>
 #include <linux/videodev2.h>
 
@@ -37,5 +38,7 @@ void vsp1_du_atomic_begin(struct device *dev);
 int vsp1_du_atomic_update(struct device *dev, unsigned int rpf,
 			  const struct vsp1_du_atomic_config *cfg);
 void vsp1_du_atomic_flush(struct device *dev);
+int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt);
+void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt);
 
 #endif /* __MEDIA_VSP1_H__ */
-- 
Regards,

Laurent Pinchart

[PATCH 5/6] v4l: vsp1: Add API to map and unmap DRM buffers through the VSP

[Laurent Pinchart]

The display buffers must be mapped for DMA through the device that
performs memory access. Expose an API to map and unmap memory through
the VSP device to be used by the DU.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/vsp1/vsp1_drm.c | 24 ++++++++++++++++++++++++
 include/media/vsp1.h                   |  3 +++
 2 files changed, 27 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
index f76131b192a4..0472cb4bc2e4 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -12,9 +12,11 @@
  */
 
 #include <linux/device.h>
+#include <linux/dma-mapping.h>
 #include <linux/slab.h>
 
 #include <media/media-entity.h>
+#include <media/rcar-fcp.h>
 #include <media/v4l2-subdev.h>
 #include <media/vsp1.h>
 
@@ -521,6 +523,28 @@ void vsp1_du_atomic_flush(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(vsp1_du_atomic_flush);
 
+int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt)
+{
+	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+	struct device *map_dev;
+
+	map_dev = vsp1->fcp ? rcar_fcp_get_device(vsp1->fcp) : dev;
+
+	return dma_map_sg(map_dev, sgt->sgl, sgt->nents, DMA_TO_DEVICE);
+}
+EXPORT_SYMBOL_GPL(vsp1_du_map_sg);
+
+void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt)
+{
+	struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+	struct device *map_dev;
+
+	map_dev = vsp1->fcp ? rcar_fcp_get_device(vsp1->fcp) : dev;
+
+	dma_unmap_sg(map_dev, sgt->sgl, sgt->nents, DMA_TO_DEVICE);
+}
+EXPORT_SYMBOL_GPL(vsp1_du_unmap_sg);
+
 /* -----------------------------------------------------------------------------
  * Initialization
  */
diff --git a/include/media/vsp1.h b/include/media/vsp1.h
index 458b400373d4..8d3d07a3715e 100644
--- a/include/media/vsp1.h
+++ b/include/media/vsp1.h
@@ -13,6 +13,7 @@
 #ifndef __MEDIA_VSP1_H__
 #define __MEDIA_VSP1_H__
 
+#include <linux/scatterlist.h>
 #include <linux/types.h>
 #include <linux/videodev2.h>
 
@@ -37,5 +38,7 @@ void vsp1_du_atomic_begin(struct device *dev);
 int vsp1_du_atomic_update(struct device *dev, unsigned int rpf,
 			  const struct vsp1_du_atomic_config *cfg);
 void vsp1_du_atomic_flush(struct device *dev);
+int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt);
+void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt);
 
 #endif /* __MEDIA_VSP1_H__ */
-- 
Regards,

Laurent Pinchart

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

[PATCH 6/6] drm: rcar-du: Map memory through the VSP device

[Laurent Pinchart]

For planes handled by a VSP instance, map the framebuffer memory through
the VSP to ensure proper IOMMU handling.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 74 ++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h |  2 +
 2 files changed, 70 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 83ebd162f3ef..851c2e78de0a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -19,7 +19,9 @@
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_plane_helper.h>
 
+#include <linux/dma-mapping.h>
 #include <linux/of_platform.h>
+#include <linux/scatterlist.h>
 #include <linux/videodev2.h>
 
 #include <media/vsp1.h>
@@ -166,12 +168,9 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 	cfg.dst.width = state->state.crtc_w;
 	cfg.dst.height = state->state.crtc_h;
 
-	for (i = 0; i < state->format->planes; ++i) {
-		struct drm_gem_cma_object *gem;
-
-		gem = drm_fb_cma_get_gem_obj(fb, i);
-		cfg.mem[i] = gem->paddr + fb->offsets[i];
-	}
+	for (i = 0; i < state->format->planes; ++i)
+		cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl)
+			   + fb->offsets[i];
 
 	for (i = 0; i < ARRAY_SIZE(formats_kms); ++i) {
 		if (formats_kms[i] == state->format->fourcc) {
@@ -183,6 +182,67 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 	vsp1_du_atomic_update(plane->vsp->vsp, plane->index, &cfg);
 }
 
+static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
+					struct drm_plane_state *state)
+{
+	struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
+	struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp;
+	struct rcar_du_device *rcdu = vsp->dev;
+	unsigned int i;
+	int ret;
+
+	if (!state->fb)
+		return 0;
+
+	for (i = 0; i < rstate->format->planes; ++i) {
+		struct drm_gem_cma_object *gem =
+			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;
+
+		ret = vsp1_du_map_sg(vsp->vsp, sgt);
+		if (!ret) {
+			sg_free_table(sgt);
+			ret = -ENOMEM;
+			goto fail;
+		}
+	}
+
+	return 0;
+
+fail:
+	for (i--; i >= 0; i--) {
+		struct sg_table *sgt = &rstate->sg_tables[i];
+
+		vsp1_du_unmap_sg(vsp->vsp, sgt);
+		sg_free_table(sgt);
+	}
+
+	return ret;
+}
+
+static void rcar_du_vsp_plane_cleanup_fb(struct drm_plane *plane,
+					 struct drm_plane_state *state)
+{
+	struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
+	struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp;
+	unsigned int i;
+
+	if (!state->fb)
+		return;
+
+	for (i = 0; i < rstate->format->planes; ++i) {
+		struct sg_table *sgt = &rstate->sg_tables[i];
+
+		vsp1_du_unmap_sg(vsp->vsp, sgt);
+		sg_free_table(sgt);
+	}
+}
+
 static int rcar_du_vsp_plane_atomic_check(struct drm_plane *plane,
 					  struct drm_plane_state *state)
 {
@@ -223,6 +283,8 @@ static void rcar_du_vsp_plane_atomic_update(struct drm_plane *plane,
 }
 
 static const struct drm_plane_helper_funcs rcar_du_vsp_plane_helper_funcs = {
+	.prepare_fb = rcar_du_vsp_plane_prepare_fb,
+	.cleanup_fb = rcar_du_vsp_plane_cleanup_fb,
 	.atomic_check = rcar_du_vsp_plane_atomic_check,
 	.atomic_update = rcar_du_vsp_plane_atomic_update,
 };
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
index 510dcc9c6816..bbb41610e38a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
@@ -43,6 +43,7 @@ static inline struct rcar_du_vsp_plane *to_rcar_vsp_plane(struct drm_plane *p)
  * struct rcar_du_vsp_plane_state - Driver-specific plane state
  * @state: base DRM plane state
  * @format: information about the pixel format used by the plane
+ * @sg_tables: scatter-gather tables for the frame buffer memory
  * @alpha: value of the plane alpha property
  * @zpos: value of the plane zpos property
  */
@@ -50,6 +51,7 @@ struct rcar_du_vsp_plane_state {
 	struct drm_plane_state state;
 
 	const struct rcar_du_format_info *format;
+	struct sg_table sg_tables[3];
 
 	unsigned int alpha;
 	unsigned int zpos;
-- 
Regards,

Laurent Pinchart

[PATCH 6/6] drm: rcar-du: Map memory through the VSP device

[Laurent Pinchart]

For planes handled by a VSP instance, map the framebuffer memory through
the VSP to ensure proper IOMMU handling.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 74 ++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h |  2 +
 2 files changed, 70 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 83ebd162f3ef..851c2e78de0a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -19,7 +19,9 @@
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_plane_helper.h>
 
+#include <linux/dma-mapping.h>
 #include <linux/of_platform.h>
+#include <linux/scatterlist.h>
 #include <linux/videodev2.h>
 
 #include <media/vsp1.h>
@@ -166,12 +168,9 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 	cfg.dst.width = state->state.crtc_w;
 	cfg.dst.height = state->state.crtc_h;
 
-	for (i = 0; i < state->format->planes; ++i) {
-		struct drm_gem_cma_object *gem;
-
-		gem = drm_fb_cma_get_gem_obj(fb, i);
-		cfg.mem[i] = gem->paddr + fb->offsets[i];
-	}
+	for (i = 0; i < state->format->planes; ++i)
+		cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl)
+			   + fb->offsets[i];
 
 	for (i = 0; i < ARRAY_SIZE(formats_kms); ++i) {
 		if (formats_kms[i] == state->format->fourcc) {
@@ -183,6 +182,67 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
 	vsp1_du_atomic_update(plane->vsp->vsp, plane->index, &cfg);
 }
 
+static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
+					struct drm_plane_state *state)
+{
+	struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
+	struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp;
+	struct rcar_du_device *rcdu = vsp->dev;
+	unsigned int i;
+	int ret;
+
+	if (!state->fb)
+		return 0;
+
+	for (i = 0; i < rstate->format->planes; ++i) {
+		struct drm_gem_cma_object *gem =
+			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;
+
+		ret = vsp1_du_map_sg(vsp->vsp, sgt);
+		if (!ret) {
+			sg_free_table(sgt);
+			ret = -ENOMEM;
+			goto fail;
+		}
+	}
+
+	return 0;
+
+fail:
+	for (i--; i >= 0; i--) {
+		struct sg_table *sgt = &rstate->sg_tables[i];
+
+		vsp1_du_unmap_sg(vsp->vsp, sgt);
+		sg_free_table(sgt);
+	}
+
+	return ret;
+}
+
+static void rcar_du_vsp_plane_cleanup_fb(struct drm_plane *plane,
+					 struct drm_plane_state *state)
+{
+	struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
+	struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp;
+	unsigned int i;
+
+	if (!state->fb)
+		return;
+
+	for (i = 0; i < rstate->format->planes; ++i) {
+		struct sg_table *sgt = &rstate->sg_tables[i];
+
+		vsp1_du_unmap_sg(vsp->vsp, sgt);
+		sg_free_table(sgt);
+	}
+}
+
 static int rcar_du_vsp_plane_atomic_check(struct drm_plane *plane,
 					  struct drm_plane_state *state)
 {
@@ -223,6 +283,8 @@ static void rcar_du_vsp_plane_atomic_update(struct drm_plane *plane,
 }
 
 static const struct drm_plane_helper_funcs rcar_du_vsp_plane_helper_funcs = {
+	.prepare_fb = rcar_du_vsp_plane_prepare_fb,
+	.cleanup_fb = rcar_du_vsp_plane_cleanup_fb,
 	.atomic_check = rcar_du_vsp_plane_atomic_check,
 	.atomic_update = rcar_du_vsp_plane_atomic_update,
 };
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
index 510dcc9c6816..bbb41610e38a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
@@ -43,6 +43,7 @@ static inline struct rcar_du_vsp_plane *to_rcar_vsp_plane(struct drm_plane *p)
  * struct rcar_du_vsp_plane_state - Driver-specific plane state
  * @state: base DRM plane state
  * @format: information about the pixel format used by the plane
+ * @sg_tables: scatter-gather tables for the frame buffer memory
  * @alpha: value of the plane alpha property
  * @zpos: value of the plane zpos property
  */
@@ -50,6 +51,7 @@ struct rcar_du_vsp_plane_state {
 	struct drm_plane_state state;
 
 	const struct rcar_du_format_info *format;
+	struct sg_table sg_tables[3];
 
 	unsigned int alpha;
 	unsigned int zpos;
-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 3/6] v4l: rcar-fcp: Don't get/put module reference

[Geert Uytterhoeven]

On Fri, Aug 19, 2016 at 10:39 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Direct callers of the FCP API hold a reference to the FCP module due to
> module linkage, there's no need to take another one manually. Take a
> reference to the device instead to ensure that it won't disappear being

... behind

> the caller's back.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/6] v4l: rcar-fcp: Don't get/put module reference

[Geert Uytterhoeven]

On Fri, Aug 19, 2016 at 10:39 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Direct callers of the FCP API hold a reference to the FCP module due to
> module linkage, there's no need to take another one manually. Take a
> reference to the device instead to ensure that it won't disappear being

... behind

> the caller's back.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/6] drm: Don't implement empty prepare_fb()/cleanup_fb()

[Daniel Vetter]

On Fri, Aug 19, 2016 at 11:39:29AM +0300, Laurent Pinchart wrote:
> The plane .prepare_fb() and .cleanup_fb() helpers are optional, there's
> no need to implement empty stubs, and no need to explicitly set the
> function pointers to NULL either.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Rebased onto Chris' patch (which I had already in the queue) and applied
to drm-misc.

Thanks, Daniel

> ---
>  drivers/gpu/drm/arc/arcpgu_crtc.c               |  2 --
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     | 15 ---------------
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 15 ---------------
>  drivers/gpu/drm/tegra/dc.c                      | 17 -----------------
>  drivers/gpu/drm/vc4/vc4_plane.c                 |  2 --
>  5 files changed, 51 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> index ee0a61c2861b..7130b044b004 100644
> --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> @@ -183,8 +183,6 @@ static void arc_pgu_plane_atomic_update(struct drm_plane *plane,
>  }
>  
>  static const struct drm_plane_helper_funcs arc_pgu_plane_helper_funcs = {
> -	.prepare_fb = NULL,
> -	.cleanup_fb = NULL,
>  	.atomic_update = arc_pgu_plane_atomic_update,
>  };
>  
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> index e50467a0deb0..a7e5486bd1e9 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> @@ -169,25 +169,10 @@ static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
>  	return;
>  }
>  
> -static void
> -fsl_dcu_drm_plane_cleanup_fb(struct drm_plane *plane,
> -			     const struct drm_plane_state *new_state)
> -{
> -}
> -
> -static int
> -fsl_dcu_drm_plane_prepare_fb(struct drm_plane *plane,
> -			     const struct drm_plane_state *new_state)
> -{
> -	return 0;
> -}
> -
>  static const struct drm_plane_helper_funcs fsl_dcu_drm_plane_helper_funcs = {
>  	.atomic_check = fsl_dcu_drm_plane_atomic_check,
>  	.atomic_disable = fsl_dcu_drm_plane_atomic_disable,
>  	.atomic_update = fsl_dcu_drm_plane_atomic_update,
> -	.cleanup_fb = fsl_dcu_drm_plane_cleanup_fb,
> -	.prepare_fb = fsl_dcu_drm_plane_prepare_fb,
>  };
>  
>  static void fsl_dcu_drm_plane_destroy(struct drm_plane *plane)
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index c3707d47cd89..635ead039b8b 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -815,19 +815,6 @@ static void ade_disable_channel(struct ade_plane *aplane)
>  	ade_compositor_routing_disable(base, ch);
>  }
>  
> -static int ade_plane_prepare_fb(struct drm_plane *plane,
> -				const struct drm_plane_state *new_state)
> -{
> -	/* do nothing */
> -	return 0;
> -}
> -
> -static void ade_plane_cleanup_fb(struct drm_plane *plane,
> -				 const struct drm_plane_state *old_state)
> -{
> -	/* do nothing */
> -}
> -
>  static int ade_plane_atomic_check(struct drm_plane *plane,
>  				  struct drm_plane_state *state)
>  {
> @@ -895,8 +882,6 @@ static void ade_plane_atomic_disable(struct drm_plane *plane,
>  }
>  
>  static const struct drm_plane_helper_funcs ade_plane_helper_funcs = {
> -	.prepare_fb = ade_plane_prepare_fb,
> -	.cleanup_fb = ade_plane_cleanup_fb,
>  	.atomic_check = ade_plane_atomic_check,
>  	.atomic_update = ade_plane_atomic_update,
>  	.atomic_disable = ade_plane_atomic_disable,
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 8495bd01b544..3de7ce33d3d4 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -480,17 +480,6 @@ static const struct drm_plane_funcs tegra_primary_plane_funcs = {
>  	.atomic_destroy_state = tegra_plane_atomic_destroy_state,
>  };
>  
> -static int tegra_plane_prepare_fb(struct drm_plane *plane,
> -				  const struct drm_plane_state *new_state)
> -{
> -	return 0;
> -}
> -
> -static void tegra_plane_cleanup_fb(struct drm_plane *plane,
> -				   const struct drm_plane_state *old_fb)
> -{
> -}
> -
>  static int tegra_plane_state_add(struct tegra_plane *plane,
>  				 struct drm_plane_state *state)
>  {
> @@ -624,8 +613,6 @@ static void tegra_plane_atomic_disable(struct drm_plane *plane,
>  }
>  
>  static const struct drm_plane_helper_funcs tegra_primary_plane_helper_funcs = {
> -	.prepare_fb = tegra_plane_prepare_fb,
> -	.cleanup_fb = tegra_plane_cleanup_fb,
>  	.atomic_check = tegra_plane_atomic_check,
>  	.atomic_update = tegra_plane_atomic_update,
>  	.atomic_disable = tegra_plane_atomic_disable,
> @@ -796,8 +783,6 @@ static const struct drm_plane_funcs tegra_cursor_plane_funcs = {
>  };
>  
>  static const struct drm_plane_helper_funcs tegra_cursor_plane_helper_funcs = {
> -	.prepare_fb = tegra_plane_prepare_fb,
> -	.cleanup_fb = tegra_plane_cleanup_fb,
>  	.atomic_check = tegra_cursor_atomic_check,
>  	.atomic_update = tegra_cursor_atomic_update,
>  	.atomic_disable = tegra_cursor_atomic_disable,
> @@ -866,8 +851,6 @@ static const uint32_t tegra_overlay_plane_formats[] = {
>  };
>  
>  static const struct drm_plane_helper_funcs tegra_overlay_plane_helper_funcs = {
> -	.prepare_fb = tegra_plane_prepare_fb,
> -	.cleanup_fb = tegra_plane_cleanup_fb,
>  	.atomic_check = tegra_plane_atomic_check,
>  	.atomic_update = tegra_plane_atomic_update,
>  	.atomic_disable = tegra_plane_atomic_disable,
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 29e4b400e25e..881bf489478b 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -735,8 +735,6 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb)
>  }
>  
>  static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
> -	.prepare_fb = NULL,
> -	.cleanup_fb = NULL,
>  	.atomic_check = vc4_plane_atomic_check,
>  	.atomic_update = vc4_plane_atomic_update,
>  };
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/6] drm: Don't implement empty prepare_fb()/cleanup_fb()

[Daniel Vetter]

On Fri, Aug 19, 2016 at 11:39:29AM +0300, Laurent Pinchart wrote:
> The plane .prepare_fb() and .cleanup_fb() helpers are optional, there's
> no need to implement empty stubs, and no need to explicitly set the
> function pointers to NULL either.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Rebased onto Chris' patch (which I had already in the queue) and applied
to drm-misc.

Thanks, Daniel

> ---
>  drivers/gpu/drm/arc/arcpgu_crtc.c               |  2 --
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     | 15 ---------------
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 15 ---------------
>  drivers/gpu/drm/tegra/dc.c                      | 17 -----------------
>  drivers/gpu/drm/vc4/vc4_plane.c                 |  2 --
>  5 files changed, 51 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> index ee0a61c2861b..7130b044b004 100644
> --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> @@ -183,8 +183,6 @@ static void arc_pgu_plane_atomic_update(struct drm_plane *plane,
>  }
>  
>  static const struct drm_plane_helper_funcs arc_pgu_plane_helper_funcs = {
> -	.prepare_fb = NULL,
> -	.cleanup_fb = NULL,
>  	.atomic_update = arc_pgu_plane_atomic_update,
>  };
>  
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> index e50467a0deb0..a7e5486bd1e9 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> @@ -169,25 +169,10 @@ static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
>  	return;
>  }
>  
> -static void
> -fsl_dcu_drm_plane_cleanup_fb(struct drm_plane *plane,
> -			     const struct drm_plane_state *new_state)
> -{
> -}
> -
> -static int
> -fsl_dcu_drm_plane_prepare_fb(struct drm_plane *plane,
> -			     const struct drm_plane_state *new_state)
> -{
> -	return 0;
> -}
> -
>  static const struct drm_plane_helper_funcs fsl_dcu_drm_plane_helper_funcs = {
>  	.atomic_check = fsl_dcu_drm_plane_atomic_check,
>  	.atomic_disable = fsl_dcu_drm_plane_atomic_disable,
>  	.atomic_update = fsl_dcu_drm_plane_atomic_update,
> -	.cleanup_fb = fsl_dcu_drm_plane_cleanup_fb,
> -	.prepare_fb = fsl_dcu_drm_plane_prepare_fb,
>  };
>  
>  static void fsl_dcu_drm_plane_destroy(struct drm_plane *plane)
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index c3707d47cd89..635ead039b8b 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -815,19 +815,6 @@ static void ade_disable_channel(struct ade_plane *aplane)
>  	ade_compositor_routing_disable(base, ch);
>  }
>  
> -static int ade_plane_prepare_fb(struct drm_plane *plane,
> -				const struct drm_plane_state *new_state)
> -{
> -	/* do nothing */
> -	return 0;
> -}
> -
> -static void ade_plane_cleanup_fb(struct drm_plane *plane,
> -				 const struct drm_plane_state *old_state)
> -{
> -	/* do nothing */
> -}
> -
>  static int ade_plane_atomic_check(struct drm_plane *plane,
>  				  struct drm_plane_state *state)
>  {
> @@ -895,8 +882,6 @@ static void ade_plane_atomic_disable(struct drm_plane *plane,
>  }
>  
>  static const struct drm_plane_helper_funcs ade_plane_helper_funcs = {
> -	.prepare_fb = ade_plane_prepare_fb,
> -	.cleanup_fb = ade_plane_cleanup_fb,
>  	.atomic_check = ade_plane_atomic_check,
>  	.atomic_update = ade_plane_atomic_update,
>  	.atomic_disable = ade_plane_atomic_disable,
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 8495bd01b544..3de7ce33d3d4 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -480,17 +480,6 @@ static const struct drm_plane_funcs tegra_primary_plane_funcs = {
>  	.atomic_destroy_state = tegra_plane_atomic_destroy_state,
>  };
>  
> -static int tegra_plane_prepare_fb(struct drm_plane *plane,
> -				  const struct drm_plane_state *new_state)
> -{
> -	return 0;
> -}
> -
> -static void tegra_plane_cleanup_fb(struct drm_plane *plane,
> -				   const struct drm_plane_state *old_fb)
> -{
> -}
> -
>  static int tegra_plane_state_add(struct tegra_plane *plane,
>  				 struct drm_plane_state *state)
>  {
> @@ -624,8 +613,6 @@ static void tegra_plane_atomic_disable(struct drm_plane *plane,
>  }
>  
>  static const struct drm_plane_helper_funcs tegra_primary_plane_helper_funcs = {
> -	.prepare_fb = tegra_plane_prepare_fb,
> -	.cleanup_fb = tegra_plane_cleanup_fb,
>  	.atomic_check = tegra_plane_atomic_check,
>  	.atomic_update = tegra_plane_atomic_update,
>  	.atomic_disable = tegra_plane_atomic_disable,
> @@ -796,8 +783,6 @@ static const struct drm_plane_funcs tegra_cursor_plane_funcs = {
>  };
>  
>  static const struct drm_plane_helper_funcs tegra_cursor_plane_helper_funcs = {
> -	.prepare_fb = tegra_plane_prepare_fb,
> -	.cleanup_fb = tegra_plane_cleanup_fb,
>  	.atomic_check = tegra_cursor_atomic_check,
>  	.atomic_update = tegra_cursor_atomic_update,
>  	.atomic_disable = tegra_cursor_atomic_disable,
> @@ -866,8 +851,6 @@ static const uint32_t tegra_overlay_plane_formats[] = {
>  };
>  
>  static const struct drm_plane_helper_funcs tegra_overlay_plane_helper_funcs = {
> -	.prepare_fb = tegra_plane_prepare_fb,
> -	.cleanup_fb = tegra_plane_cleanup_fb,
>  	.atomic_check = tegra_plane_atomic_check,
>  	.atomic_update = tegra_plane_atomic_update,
>  	.atomic_disable = tegra_plane_atomic_disable,
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 29e4b400e25e..881bf489478b 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -735,8 +735,6 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb)
>  }
>  
>  static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
> -	.prepare_fb = NULL,
> -	.cleanup_fb = NULL,
>  	.atomic_check = vc4_plane_atomic_check,
>  	.atomic_update = vc4_plane_atomic_update,
>  };
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP

[Magnus Damm]

Hi Laurent,

Thanks for your help with this. Good to see that the DU driver is
getting closer to work with the IPMMU hardware! Please see below for
some feedback from me.

On Fri, Aug 19, 2016 at 5:39 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Hello,
>
> This patch series fixes the rcar-du-drm driver to support VSP plane sources
> with an IOMMU. It is available for convenience at
>
>         git://linuxtv.org/pinchartl/media.git iommu/devel/du
>
> On R-Car Gen3 the DU has no direct memory access but sources planes through
> VSP instances. When an IOMMU is inserted between the VSP and memory, the DU
> framebuffers need to be DMA mapped using the VSP device, not the DU device as
> currently done. The same situation can also be reproduced on Gen2 hardware by
> linking the VSP to the DU in DT [1], effectively disabling direct memory
> access by the DU.
>
> The situation is made quite complex by the fact that different planes can be
> connected to different DU instances, and thus served by different IOMMUs (or,
> in practice on existing hardware, by the same IOMMU but through different
> micro-TLBs). We thus can't allocate and map buffers to the right device in a
> single dma_alloc_wc() operation as done in the DRM CMA GEM helpers.
>
> However, on such setups, the DU DT node doesn't reference IOMMUs as the DU
> does not perform any direct memory access. We can thus keep the GEM object
> allocation unchanged, and the DMA addresses that we receive in the DU driver
> will be physical addresses. Those buffers then need to be mapped to the VSP
> device when they are associated with planes. Fortunately the atomic framework
> provides two plane helper operations, .prepare_fb() and .cleanup_fb() that we
> can use for this purpose.
>
> The reality is slightly more complex than this on Gen3, as an FCP device
> instance sits between VSP instances and memory. It is the FCP devices that are
> connected to the IOMMUs, and buffer mapping thus need to be performed using
> the FCP devices. This isn't required on Gen2 as the platforms don't have any
> FCPs.
>
> Patches 1/6 and 2/6 unconstify the state argument to the .prepare_fb() and
> .cleanup_fb() operations, to allow storing the mapped buffer addresses in the
> state. Patches 3/6 and 4/6 then extend the rcar-fcp driver API to expose the
> FCP struct device. Patch 5/6 extends the vsp1 driver API to allow mapping a
> scatter-gather list to the VSP, with the implementation using the FCP devices
> instead when available. Patch 6/6 then use the vsp1 mapping API in the
> rcar-du-drm driver to map and unmap buffers when needed.
>
> The series has been tested on Gen2 (Lager) only as the Gen3 IOMMU is known to
> be broken.

Slight clarification, the R-Car Gen3 family as a whole does not have
broken IPMMU hardware. Early R-Car H3 revisions do require some errata
handling though, but M3-W and later ES versions and MP of H3 will be
fine. Given the early R-Car H3 errata I agree it makes sense to
develop and test this series on R-Car Gen2 though.

> A possible improvement is to modify the GEM object allocation mechanism to use
> non-contiguous memory when the DU driver detects that all the VSP instances it
> is connected to use an IOMMU (possibly through FCP devices).
>
> An issue has been noticed with synchronization between page flip and VSP
> operation. Buffers get unmapped (and possibly freed) before the VSP is done
> reading them. The problem isn't new, but is much more noticeable with IOMMU
> support enabled as any hardware access to unmapped memory generates an IOMMU
> page fault immediately.
>
> The series unfortunately contain a dependency between DRM and V4L2 patches,
> complicating upstream merge. As there's no urgency to merge patch 6/6 due to
> the IOMMU being broken on Gen3 at the moment, I propose merging patches
> 1/6-2/6 and 3/6-5/6 independently for the next kernel release.
>
> I would particularly appreciate feedback on the APIs introduced by patches 4/6
> and 5/6.

The code in general looks fine to me. The APIs introduced by patches
4/6 and 5/6 seem quite straightforward. Is there something I can do to
help with those?

> [1] https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg06589.html
>
> Laurent Pinchart (6):
>   drm: Don't implement empty prepare_fb()/cleanup_fb()
>   drm: Unconstify state argument to prepare_fb()/cleanup_fb()
>   v4l: rcar-fcp: Don't get/put module reference
>   v4l: rcar-fcp: Add an API to retrieve the FCP device
>   v4l: vsp1: Add API to map and unmap DRM buffers through the VSP
>   drm: rcar-du: Map memory through the VSP device
>
>  drivers/gpu/drm/arc/arcpgu_crtc.c               |  2 -
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  4 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     | 15 -----
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 15 -----
>  drivers/gpu/drm/i915/intel_display.c            |  4 +-
>  drivers/gpu/drm/i915/intel_drv.h                |  4 +-
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c       |  4 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c       |  4 +-
>  drivers/gpu/drm/omapdrm/omap_plane.c            |  4 +-
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c           | 74 +++++++++++++++++++++++--
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.h           |  2 +
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c     |  4 +-
>  drivers/gpu/drm/tegra/dc.c                      | 17 ------
>  drivers/gpu/drm/vc4/vc4_plane.c                 |  2 -
>  drivers/media/platform/rcar-fcp.c               | 17 +++---
>  drivers/media/platform/vsp1/vsp1_drm.c          | 24 ++++++++
>  include/drm/drm_modeset_helper_vtables.h        |  4 +-
>  include/media/rcar-fcp.h                        |  5 ++
>  include/media/vsp1.h                            |  3 +
>  19 files changed, 126 insertions(+), 82 deletions(-)

So I've spent some time to test this series on R-Car H3. In particular
I've tested the code in renesas-drivers-2016-08-23-v4.8-rc3.

Since I did some early prototyping to enable the DU with IPMMU myself
I noticed that some further changes may be needed. For instance, the
Display List code and VB2 queue both need a struct device from
somewhere. I propose something like the below, using the API from
patch 4/6 in this series:

--- 0001/drivers/media/platform/vsp1/vsp1_dl.c
+++ work/drivers/media/platform/vsp1/vsp1_dl.c 2016-09-01
06:18:17.140607110 +0900
@@ -17,6 +17,8 @@
 #include <linux/slab.h>
 #include <linux/workqueue.h>

+#include <media/rcar-fcp.h>
+
 #include "vsp1.h"
 #include "vsp1_dl.h"

@@ -130,12 +132,12 @@ static int vsp1_dl_body_init(struct vsp1
      size_t extra_size)
 {
  size_t size = num_entries * sizeof(*dlb->entries) + extra_size;
+ struct device *fcp = rcar_fcp_get_device(vsp1->fcp);

  dlb->vsp1 = vsp1;
  dlb->size = size;
-
- dlb->entries = dma_alloc_wc(vsp1->dev, dlb->size, &dlb->dma,
-    GFP_KERNEL);
+ dlb->entries = dma_alloc_wc(fcp ? fcp : vsp1->dev,
+    dlb->size, &dlb->dma, GFP_KERNEL);
  if (!dlb->entries)
  return -ENOMEM;

@@ -147,7 +149,10 @@ static int vsp1_dl_body_init(struct vsp1
  */
 static void vsp1_dl_body_cleanup(struct vsp1_dl_body *dlb)
 {
- dma_free_wc(dlb->vsp1->dev, dlb->size, dlb->entries, dlb->dma);
+ struct device *fcp = rcar_fcp_get_device(dlb->vsp1->fcp);
+
+ dma_free_wc(fcp ? fcp : dlb->vsp1->dev,
+    dlb->size, dlb->entries, dlb->dma);
 }

 /**
--- 0001/drivers/media/platform/vsp1/vsp1_video.c
+++ work/drivers/media/platform/vsp1/vsp1_video.c 2016-09-01
06:20:02.940607110 +0900
@@ -27,6 +27,8 @@
 #include <media/videobuf2-v4l2.h>
 #include <media/videobuf2-dma-contig.h>

+#include <media/rcar-fcp.h>
+
 #include "vsp1.h"
 #include "vsp1_bru.h"
 #include "vsp1_dl.h"
@@ -939,6 +941,7 @@ struct vsp1_video *vsp1_video_create(str
 {
  struct vsp1_video *video;
  const char *direction;
+ struct device *fcp;
  int ret;

  video = devm_kzalloc(vsp1->dev, sizeof(*video), GFP_KERNEL);
@@ -996,7 +999,8 @@ struct vsp1_video *vsp1_video_create(str
  video->queue.ops = &vsp1_video_queue_qops;
  video->queue.mem_ops = &vb2_dma_contig_memops;
  video->queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
- video->queue.dev = video->vsp1->dev;
+ fcp = rcar_fcp_get_device(vsp1->fcp);
+ video->queue.dev = fcp ? fcp : video->vsp1->dev;
  ret = vb2_queue_init(&video->queue);
  if (ret < 0) {
  dev_err(video->vsp1->dev, "failed to initialize vb2 queue\n");

Can you please consider to include or rework the above code in your
next version of this series?

Not sure if R-Car Gen2 is affected or not, but without the above code
I get the following trap during boot on r8a7795 Salvator-X:

ipmmu-vmsa febd0000.mmu: Unhandled faut: status 0x00000101 iova 0x7f09a000

Thanks,

/ magnus

Re: [PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP

[Magnus Damm]

Hi Laurent,

Thanks for your help with this. Good to see that the DU driver is
getting closer to work with the IPMMU hardware! Please see below for
some feedback from me.

On Fri, Aug 19, 2016 at 5:39 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Hello,
>
> This patch series fixes the rcar-du-drm driver to support VSP plane sources
> with an IOMMU. It is available for convenience at
>
>         git://linuxtv.org/pinchartl/media.git iommu/devel/du
>
> On R-Car Gen3 the DU has no direct memory access but sources planes through
> VSP instances. When an IOMMU is inserted between the VSP and memory, the DU
> framebuffers need to be DMA mapped using the VSP device, not the DU device as
> currently done. The same situation can also be reproduced on Gen2 hardware by
> linking the VSP to the DU in DT [1], effectively disabling direct memory
> access by the DU.
>
> The situation is made quite complex by the fact that different planes can be
> connected to different DU instances, and thus served by different IOMMUs (or,
> in practice on existing hardware, by the same IOMMU but through different
> micro-TLBs). We thus can't allocate and map buffers to the right device in a
> single dma_alloc_wc() operation as done in the DRM CMA GEM helpers.
>
> However, on such setups, the DU DT node doesn't reference IOMMUs as the DU
> does not perform any direct memory access. We can thus keep the GEM object
> allocation unchanged, and the DMA addresses that we receive in the DU driver
> will be physical addresses. Those buffers then need to be mapped to the VSP
> device when they are associated with planes. Fortunately the atomic framework
> provides two plane helper operations, .prepare_fb() and .cleanup_fb() that we
> can use for this purpose.
>
> The reality is slightly more complex than this on Gen3, as an FCP device
> instance sits between VSP instances and memory. It is the FCP devices that are
> connected to the IOMMUs, and buffer mapping thus need to be performed using
> the FCP devices. This isn't required on Gen2 as the platforms don't have any
> FCPs.
>
> Patches 1/6 and 2/6 unconstify the state argument to the .prepare_fb() and
> .cleanup_fb() operations, to allow storing the mapped buffer addresses in the
> state. Patches 3/6 and 4/6 then extend the rcar-fcp driver API to expose the
> FCP struct device. Patch 5/6 extends the vsp1 driver API to allow mapping a
> scatter-gather list to the VSP, with the implementation using the FCP devices
> instead when available. Patch 6/6 then use the vsp1 mapping API in the
> rcar-du-drm driver to map and unmap buffers when needed.
>
> The series has been tested on Gen2 (Lager) only as the Gen3 IOMMU is known to
> be broken.

Slight clarification, the R-Car Gen3 family as a whole does not have
broken IPMMU hardware. Early R-Car H3 revisions do require some errata
handling though, but M3-W and later ES versions and MP of H3 will be
fine. Given the early R-Car H3 errata I agree it makes sense to
develop and test this series on R-Car Gen2 though.

> A possible improvement is to modify the GEM object allocation mechanism to use
> non-contiguous memory when the DU driver detects that all the VSP instances it
> is connected to use an IOMMU (possibly through FCP devices).
>
> An issue has been noticed with synchronization between page flip and VSP
> operation. Buffers get unmapped (and possibly freed) before the VSP is done
> reading them. The problem isn't new, but is much more noticeable with IOMMU
> support enabled as any hardware access to unmapped memory generates an IOMMU
> page fault immediately.
>
> The series unfortunately contain a dependency between DRM and V4L2 patches,
> complicating upstream merge. As there's no urgency to merge patch 6/6 due to
> the IOMMU being broken on Gen3 at the moment, I propose merging patches
> 1/6-2/6 and 3/6-5/6 independently for the next kernel release.
>
> I would particularly appreciate feedback on the APIs introduced by patches 4/6
> and 5/6.

The code in general looks fine to me. The APIs introduced by patches
4/6 and 5/6 seem quite straightforward. Is there something I can do to
help with those?

> [1] https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg06589.html
>
> Laurent Pinchart (6):
>   drm: Don't implement empty prepare_fb()/cleanup_fb()
>   drm: Unconstify state argument to prepare_fb()/cleanup_fb()
>   v4l: rcar-fcp: Don't get/put module reference
>   v4l: rcar-fcp: Add an API to retrieve the FCP device
>   v4l: vsp1: Add API to map and unmap DRM buffers through the VSP
>   drm: rcar-du: Map memory through the VSP device
>
>  drivers/gpu/drm/arc/arcpgu_crtc.c               |  2 -
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  4 +-
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     | 15 -----
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 15 -----
>  drivers/gpu/drm/i915/intel_display.c            |  4 +-
>  drivers/gpu/drm/i915/intel_drv.h                |  4 +-
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c       |  4 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c       |  4 +-
>  drivers/gpu/drm/omapdrm/omap_plane.c            |  4 +-
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c           | 74 +++++++++++++++++++++++--
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.h           |  2 +
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c     |  4 +-
>  drivers/gpu/drm/tegra/dc.c                      | 17 ------
>  drivers/gpu/drm/vc4/vc4_plane.c                 |  2 -
>  drivers/media/platform/rcar-fcp.c               | 17 +++---
>  drivers/media/platform/vsp1/vsp1_drm.c          | 24 ++++++++
>  include/drm/drm_modeset_helper_vtables.h        |  4 +-
>  include/media/rcar-fcp.h                        |  5 ++
>  include/media/vsp1.h                            |  3 +
>  19 files changed, 126 insertions(+), 82 deletions(-)

So I've spent some time to test this series on R-Car H3. In particular
I've tested the code in renesas-drivers-2016-08-23-v4.8-rc3.

Since I did some early prototyping to enable the DU with IPMMU myself
I noticed that some further changes may be needed. For instance, the
Display List code and VB2 queue both need a struct device from
somewhere. I propose something like the below, using the API from
patch 4/6 in this series:

--- 0001/drivers/media/platform/vsp1/vsp1_dl.c
+++ work/drivers/media/platform/vsp1/vsp1_dl.c 2016-09-01
06:18:17.140607110 +0900
@@ -17,6 +17,8 @@
 #include <linux/slab.h>
 #include <linux/workqueue.h>

+#include <media/rcar-fcp.h>
+
 #include "vsp1.h"
 #include "vsp1_dl.h"

@@ -130,12 +132,12 @@ static int vsp1_dl_body_init(struct vsp1
      size_t extra_size)
 {
  size_t size = num_entries * sizeof(*dlb->entries) + extra_size;
+ struct device *fcp = rcar_fcp_get_device(vsp1->fcp);

  dlb->vsp1 = vsp1;
  dlb->size = size;
-
- dlb->entries = dma_alloc_wc(vsp1->dev, dlb->size, &dlb->dma,
-    GFP_KERNEL);
+ dlb->entries = dma_alloc_wc(fcp ? fcp : vsp1->dev,
+    dlb->size, &dlb->dma, GFP_KERNEL);
  if (!dlb->entries)
  return -ENOMEM;

@@ -147,7 +149,10 @@ static int vsp1_dl_body_init(struct vsp1
  */
 static void vsp1_dl_body_cleanup(struct vsp1_dl_body *dlb)
 {
- dma_free_wc(dlb->vsp1->dev, dlb->size, dlb->entries, dlb->dma);
+ struct device *fcp = rcar_fcp_get_device(dlb->vsp1->fcp);
+
+ dma_free_wc(fcp ? fcp : dlb->vsp1->dev,
+    dlb->size, dlb->entries, dlb->dma);
 }

 /**
--- 0001/drivers/media/platform/vsp1/vsp1_video.c
+++ work/drivers/media/platform/vsp1/vsp1_video.c 2016-09-01
06:20:02.940607110 +0900
@@ -27,6 +27,8 @@
 #include <media/videobuf2-v4l2.h>
 #include <media/videobuf2-dma-contig.h>

+#include <media/rcar-fcp.h>
+
 #include "vsp1.h"
 #include "vsp1_bru.h"
 #include "vsp1_dl.h"
@@ -939,6 +941,7 @@ struct vsp1_video *vsp1_video_create(str
 {
  struct vsp1_video *video;
  const char *direction;
+ struct device *fcp;
  int ret;

  video = devm_kzalloc(vsp1->dev, sizeof(*video), GFP_KERNEL);
@@ -996,7 +999,8 @@ struct vsp1_video *vsp1_video_create(str
  video->queue.ops = &vsp1_video_queue_qops;
  video->queue.mem_ops = &vb2_dma_contig_memops;
  video->queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
- video->queue.dev = video->vsp1->dev;
+ fcp = rcar_fcp_get_device(vsp1->fcp);
+ video->queue.dev = fcp ? fcp : video->vsp1->dev;
  ret = vb2_queue_init(&video->queue);
  if (ret < 0) {
  dev_err(video->vsp1->dev, "failed to initialize vb2 queue\n");

Can you please consider to include or rework the above code in your
next version of this series?

Not sure if R-Car Gen2 is affected or not, but without the above code
I get the following trap during boot on r8a7795 Salvator-X:

ipmmu-vmsa febd0000.mmu: Unhandled faut: status 0x00000101 iova 0x7f09a000

Thanks,

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

Re: [PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP

[Geert Uytterhoeven]

Hi Laurent,

On Fri, Aug 19, 2016 at 10:39 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> This patch series fixes the rcar-du-drm driver to support VSP plane sources
> with an IOMMU. It is available for convenience at
>
>         git://linuxtv.org/pinchartl/media.git iommu/devel/du

Dropped from renesas-drivers, as this branch is based on a very old tree
(v4.8-rc2), and many (but not all!) commits have found their way upstream.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP

[Laurent Pinchart]

Hi Magnus,

On Wednesday 07 Sep 2016 17:01:06 Magnus Damm wrote:
> Hi Laurent,
> 
> Thanks for your help with this. Good to see that the DU driver is
> getting closer to work with the IPMMU hardware! Please see below for
> some feedback from me.
> 
> On Fri, Aug 19, 2016 at 5:39 PM, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series fixes the rcar-du-drm driver to support VSP plane
> > sources with an IOMMU. It is available for convenience at
> > 
> >         git://linuxtv.org/pinchartl/media.git iommu/devel/du
> > 
> > On R-Car Gen3 the DU has no direct memory access but sources planes
> > through VSP instances. When an IOMMU is inserted between the VSP and
> > memory, the DU framebuffers need to be DMA mapped using the VSP device,
> > not the DU device as currently done. The same situation can also be
> > reproduced on Gen2 hardware by linking the VSP to the DU in DT [1],
> > effectively disabling direct memory access by the DU.
> > 
> > The situation is made quite complex by the fact that different planes can
> > be connected to different DU instances, and thus served by different
> > IOMMUs (or, in practice on existing hardware, by the same IOMMU but
> > through different micro-TLBs). We thus can't allocate and map buffers to
> > the right device in a single dma_alloc_wc() operation as done in the DRM
> > CMA GEM helpers.
> > 
> > However, on such setups, the DU DT node doesn't reference IOMMUs as the DU
> > does not perform any direct memory access. We can thus keep the GEM object
> > allocation unchanged, and the DMA addresses that we receive in the DU
> > driver will be physical addresses. Those buffers then need to be mapped
> > to the VSP device when they are associated with planes. Fortunately the
> > atomic framework provides two plane helper operations, .prepare_fb() and
> > .cleanup_fb() that we can use for this purpose.
> > 
> > The reality is slightly more complex than this on Gen3, as an FCP device
> > instance sits between VSP instances and memory. It is the FCP devices that
> > are connected to the IOMMUs, and buffer mapping thus need to be performed
> > using the FCP devices. This isn't required on Gen2 as the platforms don't
> > have any FCPs.
> > 
> > Patches 1/6 and 2/6 unconstify the state argument to the .prepare_fb() and
> > .cleanup_fb() operations, to allow storing the mapped buffer addresses in
> > the state. Patches 3/6 and 4/6 then extend the rcar-fcp driver API to
> > expose the FCP struct device. Patch 5/6 extends the vsp1 driver API to
> > allow mapping a scatter-gather list to the VSP, with the implementation
> > using the FCP devices instead when available. Patch 6/6 then use the vsp1
> > mapping API in the rcar-du-drm driver to map and unmap buffers when
> > needed.
> > 
> > The series has been tested on Gen2 (Lager) only as the Gen3 IOMMU is known
> > to be broken.
> 
> Slight clarification, the R-Car Gen3 family as a whole does not have
> broken IPMMU hardware. Early R-Car H3 revisions do require some errata
> handling though, but M3-W and later ES versions and MP of H3 will be
> fine. Given the early R-Car H3 errata I agree it makes sense to
> develop and test this series on R-Car Gen2 though.
> 
> > A possible improvement is to modify the GEM object allocation mechanism to
> > use non-contiguous memory when the DU driver detects that all the VSP
> > instances it is connected to use an IOMMU (possibly through FCP devices).
> > 
> > An issue has been noticed with synchronization between page flip and VSP
> > operation. Buffers get unmapped (and possibly freed) before the VSP is
> > done reading them. The problem isn't new, but is much more noticeable with
> > IOMMU support enabled as any hardware access to unmapped memory generates
> > an IOMMU page fault immediately.
> > 
> > The series unfortunately contain a dependency between DRM and V4L2
> > patches, complicating upstream merge. As there's no urgency to merge patch
> > 6/6 due to the IOMMU being broken on Gen3 at the moment, I propose merging
> > patches 1/6-2/6 and 3/6-5/6 independently for the next kernel release.
> > 
> > I would particularly appreciate feedback on the APIs introduced by patches
> > 4/6 and 5/6.
> 
> The code in general looks fine to me. The APIs introduced by patches
> 4/6 and 5/6 seem quite straightforward. Is there something I can do to
> help with those?
> 
> > [1]
> > https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg06589.h
> > tml
> > Laurent Pinchart (6):
> >   drm: Don't implement empty prepare_fb()/cleanup_fb()
> >   drm: Unconstify state argument to prepare_fb()/cleanup_fb()
> >   v4l: rcar-fcp: Don't get/put module reference
> >   v4l: rcar-fcp: Add an API to retrieve the FCP device
> >   v4l: vsp1: Add API to map and unmap DRM buffers through the VSP
> >   drm: rcar-du: Map memory through the VSP device
> >  
> >  drivers/gpu/drm/arc/arcpgu_crtc.c               |  2 -
> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  4 +-
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     | 15 -----
> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 15 -----
> >  drivers/gpu/drm/i915/intel_display.c            |  4 +-
> >  drivers/gpu/drm/i915/intel_drv.h                |  4 +-
> >  drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c       |  4 +-
> >  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c       |  4 +-
> >  drivers/gpu/drm/omapdrm/omap_plane.c            |  4 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_vsp.c           | 74 ++++++++++++++++++--
> >  drivers/gpu/drm/rcar-du/rcar_du_vsp.h           |  2 +
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c     |  4 +-
> >  drivers/gpu/drm/tegra/dc.c                      | 17 ------
> >  drivers/gpu/drm/vc4/vc4_plane.c                 |  2 -
> >  drivers/media/platform/rcar-fcp.c               | 17 +++---
> >  drivers/media/platform/vsp1/vsp1_drm.c          | 24 ++++++++
> >  include/drm/drm_modeset_helper_vtables.h        |  4 +-
> >  include/media/rcar-fcp.h                        |  5 ++
> >  include/media/vsp1.h                            |  3 +
> >  19 files changed, 126 insertions(+), 82 deletions(-)
> 
> So I've spent some time to test this series on R-Car H3. In particular
> I've tested the code in renesas-drivers-2016-08-23-v4.8-rc3.
> 
> Since I did some early prototyping to enable the DU with IPMMU myself
> I noticed that some further changes may be needed. For instance, the
> Display List code and VB2 queue both need a struct device from
> somewhere. I propose something like the below, using the API from
> patch 4/6 in this series:
> 
> --- 0001/drivers/media/platform/vsp1/vsp1_dl.c
> +++ work/drivers/media/platform/vsp1/vsp1_dl.c 2016-09-01
> 06:18:17.140607110 +0900
> @@ -17,6 +17,8 @@
>  #include <linux/slab.h>
>  #include <linux/workqueue.h>
> 
> +#include <media/rcar-fcp.h>
> +
>  #include "vsp1.h"
>  #include "vsp1_dl.h"
> 
> @@ -130,12 +132,12 @@ static int vsp1_dl_body_init(struct vsp1
>       size_t extra_size)
>  {
>   size_t size = num_entries * sizeof(*dlb->entries) + extra_size;
> + struct device *fcp = rcar_fcp_get_device(vsp1->fcp);
> 
>   dlb->vsp1 = vsp1;
>   dlb->size = size;
> -
> - dlb->entries = dma_alloc_wc(vsp1->dev, dlb->size, &dlb->dma,
> -    GFP_KERNEL);
> + dlb->entries = dma_alloc_wc(fcp ? fcp : vsp1->dev,
> +    dlb->size, &dlb->dma, GFP_KERNEL);
>   if (!dlb->entries)
>   return -ENOMEM;
> 
> @@ -147,7 +149,10 @@ static int vsp1_dl_body_init(struct vsp1
>   */
>  static void vsp1_dl_body_cleanup(struct vsp1_dl_body *dlb)
>  {
> - dma_free_wc(dlb->vsp1->dev, dlb->size, dlb->entries, dlb->dma);
> + struct device *fcp = rcar_fcp_get_device(dlb->vsp1->fcp);
> +
> + dma_free_wc(fcp ? fcp : dlb->vsp1->dev,
> +    dlb->size, dlb->entries, dlb->dma);
>  }
> 
>  /**
> --- 0001/drivers/media/platform/vsp1/vsp1_video.c
> +++ work/drivers/media/platform/vsp1/vsp1_video.c 2016-09-01
> 06:20:02.940607110 +0900
> @@ -27,6 +27,8 @@
>  #include <media/videobuf2-v4l2.h>
>  #include <media/videobuf2-dma-contig.h>
> 
> +#include <media/rcar-fcp.h>
> +
>  #include "vsp1.h"
>  #include "vsp1_bru.h"
>  #include "vsp1_dl.h"
> @@ -939,6 +941,7 @@ struct vsp1_video *vsp1_video_create(str
>  {
>   struct vsp1_video *video;
>   const char *direction;
> + struct device *fcp;
>   int ret;
> 
>   video = devm_kzalloc(vsp1->dev, sizeof(*video), GFP_KERNEL);
> @@ -996,7 +999,8 @@ struct vsp1_video *vsp1_video_create(str
>   video->queue.ops = &vsp1_video_queue_qops;
>   video->queue.mem_ops = &vb2_dma_contig_memops;
>   video->queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> - video->queue.dev = video->vsp1->dev;
> + fcp = rcar_fcp_get_device(vsp1->fcp);
> + video->queue.dev = fcp ? fcp : video->vsp1->dev;
>   ret = vb2_queue_init(&video->queue);
>   if (ret < 0) {
>   dev_err(video->vsp1->dev, "failed to initialize vb2 queue\n");
> 
> Can you please consider to include or rework the above code in your
> next version of this series?
> 
> Not sure if R-Car Gen2 is affected or not, but without the above code
> I get the following trap during boot on r8a7795 Salvator-X:
> 
> ipmmu-vmsa febd0000.mmu: Unhandled faut: status 0x00000101 iova 0x7f09a000

I'm finally getting back to this. I'll include a modified version of the above 
code in the next version of the series and will make sure to test it on both 
Gen2 and Gen3 systems with and without the IOMMU enabled.

Unless you'd prefer otherwise, I'll mark you as the author of the code. Can I 
get your SoB ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP

[Laurent Pinchart]

Hi Magnus,

On Wednesday 07 Sep 2016 17:01:06 Magnus Damm wrote:
> Hi Laurent,
> 
> Thanks for your help with this. Good to see that the DU driver is
> getting closer to work with the IPMMU hardware! Please see below for
> some feedback from me.
> 
> On Fri, Aug 19, 2016 at 5:39 PM, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series fixes the rcar-du-drm driver to support VSP plane
> > sources with an IOMMU. It is available for convenience at
> > 
> >         git://linuxtv.org/pinchartl/media.git iommu/devel/du
> > 
> > On R-Car Gen3 the DU has no direct memory access but sources planes
> > through VSP instances. When an IOMMU is inserted between the VSP and
> > memory, the DU framebuffers need to be DMA mapped using the VSP device,
> > not the DU device as currently done. The same situation can also be
> > reproduced on Gen2 hardware by linking the VSP to the DU in DT [1],
> > effectively disabling direct memory access by the DU.
> > 
> > The situation is made quite complex by the fact that different planes can
> > be connected to different DU instances, and thus served by different
> > IOMMUs (or, in practice on existing hardware, by the same IOMMU but
> > through different micro-TLBs). We thus can't allocate and map buffers to
> > the right device in a single dma_alloc_wc() operation as done in the DRM
> > CMA GEM helpers.
> > 
> > However, on such setups, the DU DT node doesn't reference IOMMUs as the DU
> > does not perform any direct memory access. We can thus keep the GEM object
> > allocation unchanged, and the DMA addresses that we receive in the DU
> > driver will be physical addresses. Those buffers then need to be mapped
> > to the VSP device when they are associated with planes. Fortunately the
> > atomic framework provides two plane helper operations, .prepare_fb() and
> > .cleanup_fb() that we can use for this purpose.
> > 
> > The reality is slightly more complex than this on Gen3, as an FCP device
> > instance sits between VSP instances and memory. It is the FCP devices that
> > are connected to the IOMMUs, and buffer mapping thus need to be performed
> > using the FCP devices. This isn't required on Gen2 as the platforms don't
> > have any FCPs.
> > 
> > Patches 1/6 and 2/6 unconstify the state argument to the .prepare_fb() and
> > .cleanup_fb() operations, to allow storing the mapped buffer addresses in
> > the state. Patches 3/6 and 4/6 then extend the rcar-fcp driver API to
> > expose the FCP struct device. Patch 5/6 extends the vsp1 driver API to
> > allow mapping a scatter-gather list to the VSP, with the implementation
> > using the FCP devices instead when available. Patch 6/6 then use the vsp1
> > mapping API in the rcar-du-drm driver to map and unmap buffers when
> > needed.
> > 
> > The series has been tested on Gen2 (Lager) only as the Gen3 IOMMU is known
> > to be broken.
> 
> Slight clarification, the R-Car Gen3 family as a whole does not have
> broken IPMMU hardware. Early R-Car H3 revisions do require some errata
> handling though, but M3-W and later ES versions and MP of H3 will be
> fine. Given the early R-Car H3 errata I agree it makes sense to
> develop and test this series on R-Car Gen2 though.
> 
> > A possible improvement is to modify the GEM object allocation mechanism to
> > use non-contiguous memory when the DU driver detects that all the VSP
> > instances it is connected to use an IOMMU (possibly through FCP devices).
> > 
> > An issue has been noticed with synchronization between page flip and VSP
> > operation. Buffers get unmapped (and possibly freed) before the VSP is
> > done reading them. The problem isn't new, but is much more noticeable with
> > IOMMU support enabled as any hardware access to unmapped memory generates
> > an IOMMU page fault immediately.
> > 
> > The series unfortunately contain a dependency between DRM and V4L2
> > patches, complicating upstream merge. As there's no urgency to merge patch
> > 6/6 due to the IOMMU being broken on Gen3 at the moment, I propose merging
> > patches 1/6-2/6 and 3/6-5/6 independently for the next kernel release.
> > 
> > I would particularly appreciate feedback on the APIs introduced by patches
> > 4/6 and 5/6.
> 
> The code in general looks fine to me. The APIs introduced by patches
> 4/6 and 5/6 seem quite straightforward. Is there something I can do to
> help with those?
> 
> > [1]
> > https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg06589.h
> > tml
> > Laurent Pinchart (6):
> >   drm: Don't implement empty prepare_fb()/cleanup_fb()
> >   drm: Unconstify state argument to prepare_fb()/cleanup_fb()
> >   v4l: rcar-fcp: Don't get/put module reference
> >   v4l: rcar-fcp: Add an API to retrieve the FCP device
> >   v4l: vsp1: Add API to map and unmap DRM buffers through the VSP
> >   drm: rcar-du: Map memory through the VSP device
> >  
> >  drivers/gpu/drm/arc/arcpgu_crtc.c               |  2 -
> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  4 +-
> >  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c     | 15 -----
> >  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 15 -----
> >  drivers/gpu/drm/i915/intel_display.c            |  4 +-
> >  drivers/gpu/drm/i915/intel_drv.h                |  4 +-
> >  drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c       |  4 +-
> >  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c       |  4 +-
> >  drivers/gpu/drm/omapdrm/omap_plane.c            |  4 +-
> >  drivers/gpu/drm/rcar-du/rcar_du_vsp.c           | 74 ++++++++++++++++++--
> >  drivers/gpu/drm/rcar-du/rcar_du_vsp.h           |  2 +
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c     |  4 +-
> >  drivers/gpu/drm/tegra/dc.c                      | 17 ------
> >  drivers/gpu/drm/vc4/vc4_plane.c                 |  2 -
> >  drivers/media/platform/rcar-fcp.c               | 17 +++---
> >  drivers/media/platform/vsp1/vsp1_drm.c          | 24 ++++++++
> >  include/drm/drm_modeset_helper_vtables.h        |  4 +-
> >  include/media/rcar-fcp.h                        |  5 ++
> >  include/media/vsp1.h                            |  3 +
> >  19 files changed, 126 insertions(+), 82 deletions(-)
> 
> So I've spent some time to test this series on R-Car H3. In particular
> I've tested the code in renesas-drivers-2016-08-23-v4.8-rc3.
> 
> Since I did some early prototyping to enable the DU with IPMMU myself
> I noticed that some further changes may be needed. For instance, the
> Display List code and VB2 queue both need a struct device from
> somewhere. I propose something like the below, using the API from
> patch 4/6 in this series:
> 
> --- 0001/drivers/media/platform/vsp1/vsp1_dl.c
> +++ work/drivers/media/platform/vsp1/vsp1_dl.c 2016-09-01
> 06:18:17.140607110 +0900
> @@ -17,6 +17,8 @@
>  #include <linux/slab.h>
>  #include <linux/workqueue.h>
> 
> +#include <media/rcar-fcp.h>
> +
>  #include "vsp1.h"
>  #include "vsp1_dl.h"
> 
> @@ -130,12 +132,12 @@ static int vsp1_dl_body_init(struct vsp1
>       size_t extra_size)
>  {
>   size_t size = num_entries * sizeof(*dlb->entries) + extra_size;
> + struct device *fcp = rcar_fcp_get_device(vsp1->fcp);
> 
>   dlb->vsp1 = vsp1;
>   dlb->size = size;
> -
> - dlb->entries = dma_alloc_wc(vsp1->dev, dlb->size, &dlb->dma,
> -    GFP_KERNEL);
> + dlb->entries = dma_alloc_wc(fcp ? fcp : vsp1->dev,
> +    dlb->size, &dlb->dma, GFP_KERNEL);
>   if (!dlb->entries)
>   return -ENOMEM;
> 
> @@ -147,7 +149,10 @@ static int vsp1_dl_body_init(struct vsp1
>   */
>  static void vsp1_dl_body_cleanup(struct vsp1_dl_body *dlb)
>  {
> - dma_free_wc(dlb->vsp1->dev, dlb->size, dlb->entries, dlb->dma);
> + struct device *fcp = rcar_fcp_get_device(dlb->vsp1->fcp);
> +
> + dma_free_wc(fcp ? fcp : dlb->vsp1->dev,
> +    dlb->size, dlb->entries, dlb->dma);
>  }
> 
>  /**
> --- 0001/drivers/media/platform/vsp1/vsp1_video.c
> +++ work/drivers/media/platform/vsp1/vsp1_video.c 2016-09-01
> 06:20:02.940607110 +0900
> @@ -27,6 +27,8 @@
>  #include <media/videobuf2-v4l2.h>
>  #include <media/videobuf2-dma-contig.h>
> 
> +#include <media/rcar-fcp.h>
> +
>  #include "vsp1.h"
>  #include "vsp1_bru.h"
>  #include "vsp1_dl.h"
> @@ -939,6 +941,7 @@ struct vsp1_video *vsp1_video_create(str
>  {
>   struct vsp1_video *video;
>   const char *direction;
> + struct device *fcp;
>   int ret;
> 
>   video = devm_kzalloc(vsp1->dev, sizeof(*video), GFP_KERNEL);
> @@ -996,7 +999,8 @@ struct vsp1_video *vsp1_video_create(str
>   video->queue.ops = &vsp1_video_queue_qops;
>   video->queue.mem_ops = &vb2_dma_contig_memops;
>   video->queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> - video->queue.dev = video->vsp1->dev;
> + fcp = rcar_fcp_get_device(vsp1->fcp);
> + video->queue.dev = fcp ? fcp : video->vsp1->dev;
>   ret = vb2_queue_init(&video->queue);
>   if (ret < 0) {
>   dev_err(video->vsp1->dev, "failed to initialize vb2 queue\n");
> 
> Can you please consider to include or rework the above code in your
> next version of this series?
> 
> Not sure if R-Car Gen2 is affected or not, but without the above code
> I get the following trap during boot on r8a7795 Salvator-X:
> 
> ipmmu-vmsa febd0000.mmu: Unhandled faut: status 0x00000101 iova 0x7f09a000

I'm finally getting back to this. I'll include a modified version of the above 
code in the next version of the series and will make sure to test it on both 
Gen2 and Gen3 systems with and without the IOMMU enabled.

Unless you'd prefer otherwise, I'll mark you as the author of the code. Can I 
get your SoB ?

-- 
Regards,

Laurent Pinchart

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