linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/11] R-Car DU Interlaced support through VSP1
@ 2018-07-17 20:35 Kieran Bingham
  2018-07-17 20:35 ` [PATCH v5 01/11] media: vsp1: drm: Fix minor grammar error Kieran Bingham
                   ` (10 more replies)
  0 siblings, 11 replies; 18+ messages in thread
From: Kieran Bingham @ 2018-07-17 20:35 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham
  Cc: linux-renesas-soc, linux-media, dri-devel, Kieran Bingham

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

The Gen3 R-Car DU devices make use of the VSP to handle frame processing.

In this series we implement support for handling interlaced pipelines by using
the auto-fld feature of the VSP hardware.

The implementation is preceded by some cleanup work and refactoring, through
patches 1 to 6. These are trivial and could be collected earlier and
independently if this series requires further revisions.

Patch 7 makes a key distinctive change to remove all existing support for
headerless display lists throughout the VSP1 driver, and ensures that all
pipelines use the same code path. This simplifies the code and reduces
opportunity for untested code paths to exist.

Patches 8, 9 and 10 implement the relevant support in the VSP1 driver, before
patch 11 finally enables the feature through the drm R-Car DU driver.

This series is based upon the linux-media tree (master branch), and is
available from the following URL:

  git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git
  tags/vsp1/du/interlaced/v5

ChangeLog:

v5:
 * media: vsp1: Use header display lists for all WPF outputs linked to the DU
   - Fixed comments
   - Re-aligned header_offset calculation
 * media: vsp1: Add support for extended display list headers
   - Rename vsp1_dl_ext_header field names
   - Rename @extended -> @extension
   - Remove unnecessary VI6_DL_SWAP changes
   - Rename @cmd_opcode -> @opcode
   - Drop unused @data_size field
   - Move iteration of WPF's inside vsp1_dlm_setup
   - Rename vsp1_dl_ext_cmd_header -> vsp1_pre_ext_dl_body
   - Rename vsp1_pre_ext_dl_body->cmd to vsp1_pre_ext_dl_body->opcode
   - Rename vsp1_dl_ext_header->reserved0 to vsp1_dl_ext_header->padding
   - vsp1_pre_ext_dl_body: Rename 'data' to 'address_set'
   - vsp1_pre_ext_dl_body: Add struct documentation
   - Document ordering of 16bit accesses for flags in vsp1_dl_ext_header
 * media: vsp1: Provide support for extended command pools
   - Rename vsp1_dl_ext_cmd_header -> vsp1_pre_ext_dl_body
   - fixup vsp1_cmd_pool structure documentation
   - Rename dlm->autofld_cmds dlm->cmdpool
   - Separate out the instatiation of vsp1_extended_commands
   - Move initialisation of lock, and lists in vsp1_dl_cmd_pool_create to
     immediately after allocation
   - simplify vsp1_dlm_get_autofld_cmd
   - Rename vsp1_dl_get_autofld_cmd() to vsp1_dl_get_pre_cmd() and moved
     to "Display List Extended Command Management" section of vsp1_dl
 * media: vsp1: Support Interlaced display pipelines
   - Obtain autofld cmd in vsp1_rpf_configure_autofld()
   - Move VSP1_DL_EXT_AUTOFLD_INT to vsp1_regs.h
   - Rename VSP1_DL_EXT_AUTOFLD_INT -> VI6_DL_EXT_AUTOFLD_INT
   - move interlaced configuration parameter to pipe object
   - autofld: Set cmd->flags in one single expression.
 * drm: rcar-du: Support interlaced video output through vsp1
   - Fix commit title
   - Document change to DSMR
   - Configure through vsp1_du_setup_lif(), rather than
     vsp1_du_atomic_update()

v4.5:
 - Rebase to latest linux-media/master

v4:
 - Move configure_partition() call changes into tlb-optimise-v9

v3:
 - Rebased on top of TLB Optimise rework v8
 - Added the DL parameter back into the configure_partition() calls.
   - This change could be moved into the TLB-Optimise series.
 - Document interlaced field in struct vsp1_du_atomic_config

v2:
 - media: vsp1: use kernel __packed for structures
    became:
   media: vsp1: Remove packed attributes from aligned structures

 - media: vsp1: Add support for extended display list headers
   - No longer declares structs __packed

 - media: vsp1: Provide support for extended command pools
   - Fix spelling typo in commit message
   - constify, and staticify the instantiation of vsp1_extended_commands
   - s/autfld_cmds/autofld_cmds/
   - staticify cmd pool functions (Thanks kbuild-bot)

 - media: vsp1: Support Interlaced display pipelines
   - fix erroneous BIT value which enabled interlaced
   - fix field handling at frame_end interrupt

Kieran Bingham (11):
  media: vsp1: drm: Fix minor grammar error
  media: vsp1: Remove packed attributes from aligned structures
  media: vsp1: Rename dl_child to dl_next
  media: vsp1: Remove unused display list structure field
  media: vsp1: Clean up DLM objects on error
  media: vsp1: Provide VSP1 feature helper macro
  media: vsp1: Use header display lists for all WPF outputs linked to the DU
  media: vsp1: Add support for extended display list headers
  media: vsp1: Provide support for extended command pools
  media: vsp1: Support Interlaced display pipelines
  drm: rcar-du: Support interlaced video output through vsp1

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |   1 +-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c   |   1 +-
 drivers/media/platform/vsp1/vsp1.h      |   3 +-
 drivers/media/platform/vsp1/vsp1_dl.c   | 432 +++++++++++++++++++------
 drivers/media/platform/vsp1/vsp1_dl.h   |  28 ++-
 drivers/media/platform/vsp1/vsp1_drm.c  |  18 +-
 drivers/media/platform/vsp1/vsp1_drv.c  |  20 +-
 drivers/media/platform/vsp1/vsp1_pipe.h |   2 +-
 drivers/media/platform/vsp1/vsp1_regs.h |   5 +-
 drivers/media/platform/vsp1/vsp1_rpf.c  |  72 +++-
 drivers/media/platform/vsp1/vsp1_wpf.c  |   6 +-
 include/media/vsp1.h                    |   2 +-
 12 files changed, 480 insertions(+), 110 deletions(-)

base-commit: 666e994aa2278e948e2492ee9d81b4df241e7222
-- 
git-series 0.9.1

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

* [PATCH v5 01/11] media: vsp1: drm: Fix minor grammar error
  2018-07-17 20:35 [PATCH v5 00/11] R-Car DU Interlaced support through VSP1 Kieran Bingham
@ 2018-07-17 20:35 ` Kieran Bingham
  2018-07-17 20:35 ` [PATCH v5 02/11] media: vsp1: Remove packed attributes from aligned structures Kieran Bingham
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2018-07-17 20:35 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham
  Cc: linux-renesas-soc, linux-media, dri-devel, Kieran Bingham

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

The pixel format is 'unsupported'. Fix the small debug message which
incorrectly declares this.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/vsp1/vsp1_drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
index edb35a5c57ea..a16856856789 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -806,7 +806,7 @@ int vsp1_du_atomic_update(struct device *dev, unsigned int pipe_index,
 	 */
 	fmtinfo = vsp1_get_format_info(vsp1, cfg->pixelformat);
 	if (!fmtinfo) {
-		dev_dbg(vsp1->dev, "Unsupport pixel format %08x for RPF\n",
+		dev_dbg(vsp1->dev, "Unsupported pixel format %08x for RPF\n",
 			cfg->pixelformat);
 		return -EINVAL;
 	}
-- 
git-series 0.9.1

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

* [PATCH v5 02/11] media: vsp1: Remove packed attributes from aligned structures
  2018-07-17 20:35 [PATCH v5 00/11] R-Car DU Interlaced support through VSP1 Kieran Bingham
  2018-07-17 20:35 ` [PATCH v5 01/11] media: vsp1: drm: Fix minor grammar error Kieran Bingham
@ 2018-07-17 20:35 ` Kieran Bingham
  2018-07-17 20:35 ` [PATCH v5 03/11] media: vsp1: Rename dl_child to dl_next Kieran Bingham
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2018-07-17 20:35 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham
  Cc: linux-renesas-soc, linux-media, dri-devel, Kieran Bingham

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

The use of the packed attribute can cause a performance penalty for
all accesses to the struct members, as the compiler will assume that the
structure has the potential to have an unaligned base.

These structures are all correctly aligned and contain no holes, thus
the attribute is redundant and negatively impacts performance, so we
remove the attributes entirely.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2
 - Remove attributes entirely

 drivers/media/platform/vsp1/vsp1_dl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
index d9b9cdd8fbe2..5da7988dd616 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -25,19 +25,19 @@
 struct vsp1_dl_header_list {
 	u32 num_bytes;
 	u32 addr;
-} __attribute__((__packed__));
+};
 
 struct vsp1_dl_header {
 	u32 num_lists;
 	struct vsp1_dl_header_list lists[8];
 	u32 next_header;
 	u32 flags;
-} __attribute__((__packed__));
+};
 
 struct vsp1_dl_entry {
 	u32 addr;
 	u32 data;
-} __attribute__((__packed__));
+};
 
 /**
  * struct vsp1_dl_body - Display list body
-- 
git-series 0.9.1

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

* [PATCH v5 03/11] media: vsp1: Rename dl_child to dl_next
  2018-07-17 20:35 [PATCH v5 00/11] R-Car DU Interlaced support through VSP1 Kieran Bingham
  2018-07-17 20:35 ` [PATCH v5 01/11] media: vsp1: drm: Fix minor grammar error Kieran Bingham
  2018-07-17 20:35 ` [PATCH v5 02/11] media: vsp1: Remove packed attributes from aligned structures Kieran Bingham
@ 2018-07-17 20:35 ` Kieran Bingham
  2018-07-17 20:35 ` [PATCH v5 04/11] media: vsp1: Remove unused display list structure field Kieran Bingham
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2018-07-17 20:35 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham
  Cc: linux-renesas-soc, linux-media, dri-devel, Kieran Bingham

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Both vsp1_dl_list_commit() and __vsp1_dl_list_put() walk the display
list chain referencing the nodes as children, when in reality they are
siblings.

Update the terminology to 'dl_next' to be consistent with the
vsp1_video_pipeline_run() usage.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/vsp1/vsp1_dl.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
index 5da7988dd616..851ac26860ae 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -398,7 +398,7 @@ struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm)
 /* This function must be called with the display list manager lock held.*/
 static void __vsp1_dl_list_put(struct vsp1_dl_list *dl)
 {
-	struct vsp1_dl_list *dl_child;
+	struct vsp1_dl_list *dl_next;
 
 	if (!dl)
 		return;
@@ -408,8 +408,8 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl)
 	 * hardware operation.
 	 */
 	if (dl->has_chain) {
-		list_for_each_entry(dl_child, &dl->chain, chain)
-			__vsp1_dl_list_put(dl_child);
+		list_for_each_entry(dl_next, &dl->chain, chain)
+			__vsp1_dl_list_put(dl_next);
 	}
 
 	dl->has_chain = false;
@@ -673,17 +673,17 @@ static void vsp1_dl_list_commit_singleshot(struct vsp1_dl_list *dl)
 void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool internal)
 {
 	struct vsp1_dl_manager *dlm = dl->dlm;
-	struct vsp1_dl_list *dl_child;
+	struct vsp1_dl_list *dl_next;
 	unsigned long flags;
 
 	if (dlm->mode == VSP1_DL_MODE_HEADER) {
 		/* Fill the header for the head and chained display lists. */
 		vsp1_dl_list_fill_header(dl, list_empty(&dl->chain));
 
-		list_for_each_entry(dl_child, &dl->chain, chain) {
-			bool last = list_is_last(&dl_child->chain, &dl->chain);
+		list_for_each_entry(dl_next, &dl->chain, chain) {
+			bool last = list_is_last(&dl_next->chain, &dl->chain);
 
-			vsp1_dl_list_fill_header(dl_child, last);
+			vsp1_dl_list_fill_header(dl_next, last);
 		}
 	}
 
-- 
git-series 0.9.1

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

* [PATCH v5 04/11] media: vsp1: Remove unused display list structure field
  2018-07-17 20:35 [PATCH v5 00/11] R-Car DU Interlaced support through VSP1 Kieran Bingham
                   ` (2 preceding siblings ...)
  2018-07-17 20:35 ` [PATCH v5 03/11] media: vsp1: Rename dl_child to dl_next Kieran Bingham
@ 2018-07-17 20:35 ` Kieran Bingham
  2018-07-17 20:35 ` [PATCH v5 05/11] media: vsp1: Clean up DLM objects on error Kieran Bingham
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2018-07-17 20:35 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham
  Cc: linux-renesas-soc, linux-media, dri-devel, Kieran Bingham

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

The vsp1 reference in the vsp1_dl_body structure is not used.
Remove it.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/vsp1/vsp1_dl.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
index 851ac26860ae..da73881a975b 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -44,7 +44,6 @@ struct vsp1_dl_entry {
  * @list: entry in the display list list of bodies
  * @free: entry in the pool free body list
  * @pool: pool to which this body belongs
- * @vsp1: the VSP1 device
  * @entries: array of entries
  * @dma: DMA address of the entries
  * @size: size of the DMA memory in bytes
@@ -58,7 +57,6 @@ struct vsp1_dl_body {
 	refcount_t refcnt;
 
 	struct vsp1_dl_body_pool *pool;
-	struct vsp1_device *vsp1;
 
 	struct vsp1_dl_entry *entries;
 	dma_addr_t dma;
-- 
git-series 0.9.1

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

* [PATCH v5 05/11] media: vsp1: Clean up DLM objects on error
  2018-07-17 20:35 [PATCH v5 00/11] R-Car DU Interlaced support through VSP1 Kieran Bingham
                   ` (3 preceding siblings ...)
  2018-07-17 20:35 ` [PATCH v5 04/11] media: vsp1: Remove unused display list structure field Kieran Bingham
@ 2018-07-17 20:35 ` Kieran Bingham
  2018-07-17 20:35 ` [PATCH v5 06/11] media: vsp1: Provide VSP1 feature helper macro Kieran Bingham
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2018-07-17 20:35 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham
  Cc: linux-renesas-soc, linux-media, dri-devel, Kieran Bingham

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

If there is an error allocating a display list within a DLM object
the existing display lists are not free'd, and neither is the DL body
pool.

Use the existing vsp1_dlm_destroy() function to clean up on error.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/vsp1/vsp1_dl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
index da73881a975b..3bf5b0b871b6 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -857,8 +857,10 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
 		struct vsp1_dl_list *dl;
 
 		dl = vsp1_dl_list_alloc(dlm);
-		if (!dl)
+		if (!dl) {
+			vsp1_dlm_destroy(dlm);
 			return NULL;
+		}
 
 		list_add_tail(&dl->list, &dlm->free);
 	}
-- 
git-series 0.9.1

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

* [PATCH v5 06/11] media: vsp1: Provide VSP1 feature helper macro
  2018-07-17 20:35 [PATCH v5 00/11] R-Car DU Interlaced support through VSP1 Kieran Bingham
                   ` (4 preceding siblings ...)
  2018-07-17 20:35 ` [PATCH v5 05/11] media: vsp1: Clean up DLM objects on error Kieran Bingham
@ 2018-07-17 20:35 ` Kieran Bingham
  2018-07-17 20:35 ` [PATCH v5 07/11] media: vsp1: Use header display lists for all WPF outputs linked to the DU Kieran Bingham
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2018-07-17 20:35 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham
  Cc: linux-renesas-soc, linux-media, dri-devel, Kieran Bingham

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

The VSP1 devices define their specific capabilities through features
marked in their device info structure. Various parts of the code read
this info structure to infer if the features are available.

Wrap this into a more readable vsp1_feature(vsp1, f) macro to ensure
that usage is consistent throughout the driver.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/vsp1/vsp1.h     |  2 ++
 drivers/media/platform/vsp1/vsp1_drv.c | 16 ++++++++--------
 drivers/media/platform/vsp1/vsp1_wpf.c |  6 +++---
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1.h b/drivers/media/platform/vsp1/vsp1.h
index 33f632331474..f0d21cc8e9ab 100644
--- a/drivers/media/platform/vsp1/vsp1.h
+++ b/drivers/media/platform/vsp1/vsp1.h
@@ -68,6 +68,8 @@ struct vsp1_device_info {
 	bool uapi;
 };
 
+#define vsp1_feature(vsp1, f) ((vsp1)->info->features & (f))
+
 struct vsp1_device {
 	struct device *dev;
 	const struct vsp1_device_info *info;
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
index 5d82f6ee56ea..3367c2ba990d 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -265,7 +265,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 	}
 
 	/* Instantiate all the entities. */
-	if (vsp1->info->features & VSP1_HAS_BRS) {
+	if (vsp1_feature(vsp1, VSP1_HAS_BRS)) {
 		vsp1->brs = vsp1_brx_create(vsp1, VSP1_ENTITY_BRS);
 		if (IS_ERR(vsp1->brs)) {
 			ret = PTR_ERR(vsp1->brs);
@@ -275,7 +275,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 		list_add_tail(&vsp1->brs->entity.list_dev, &vsp1->entities);
 	}
 
-	if (vsp1->info->features & VSP1_HAS_BRU) {
+	if (vsp1_feature(vsp1, VSP1_HAS_BRU)) {
 		vsp1->bru = vsp1_brx_create(vsp1, VSP1_ENTITY_BRU);
 		if (IS_ERR(vsp1->bru)) {
 			ret = PTR_ERR(vsp1->bru);
@@ -285,7 +285,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 		list_add_tail(&vsp1->bru->entity.list_dev, &vsp1->entities);
 	}
 
-	if (vsp1->info->features & VSP1_HAS_CLU) {
+	if (vsp1_feature(vsp1, VSP1_HAS_CLU)) {
 		vsp1->clu = vsp1_clu_create(vsp1);
 		if (IS_ERR(vsp1->clu)) {
 			ret = PTR_ERR(vsp1->clu);
@@ -311,7 +311,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 
 	list_add_tail(&vsp1->hst->entity.list_dev, &vsp1->entities);
 
-	if (vsp1->info->features & VSP1_HAS_HGO && vsp1->info->uapi) {
+	if (vsp1_feature(vsp1, VSP1_HAS_HGO) && vsp1->info->uapi) {
 		vsp1->hgo = vsp1_hgo_create(vsp1);
 		if (IS_ERR(vsp1->hgo)) {
 			ret = PTR_ERR(vsp1->hgo);
@@ -322,7 +322,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 			      &vsp1->entities);
 	}
 
-	if (vsp1->info->features & VSP1_HAS_HGT && vsp1->info->uapi) {
+	if (vsp1_feature(vsp1, VSP1_HAS_HGT) && vsp1->info->uapi) {
 		vsp1->hgt = vsp1_hgt_create(vsp1);
 		if (IS_ERR(vsp1->hgt)) {
 			ret = PTR_ERR(vsp1->hgt);
@@ -353,7 +353,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 		}
 	}
 
-	if (vsp1->info->features & VSP1_HAS_LUT) {
+	if (vsp1_feature(vsp1, VSP1_HAS_LUT)) {
 		vsp1->lut = vsp1_lut_create(vsp1);
 		if (IS_ERR(vsp1->lut)) {
 			ret = PTR_ERR(vsp1->lut);
@@ -387,7 +387,7 @@ static int vsp1_create_entities(struct vsp1_device *vsp1)
 		}
 	}
 
-	if (vsp1->info->features & VSP1_HAS_SRU) {
+	if (vsp1_feature(vsp1, VSP1_HAS_SRU)) {
 		vsp1->sru = vsp1_sru_create(vsp1);
 		if (IS_ERR(vsp1->sru)) {
 			ret = PTR_ERR(vsp1->sru);
@@ -537,7 +537,7 @@ static int vsp1_device_init(struct vsp1_device *vsp1)
 	vsp1_write(vsp1, VI6_DPR_HSI_ROUTE, VI6_DPR_NODE_UNUSED);
 	vsp1_write(vsp1, VI6_DPR_BRU_ROUTE, VI6_DPR_NODE_UNUSED);
 
-	if (vsp1->info->features & VSP1_HAS_BRS)
+	if (vsp1_feature(vsp1, VSP1_HAS_BRS))
 		vsp1_write(vsp1, VI6_DPR_ILV_BRS_ROUTE, VI6_DPR_NODE_UNUSED);
 
 	vsp1_write(vsp1, VI6_DPR_HGO_SMPPT, (7 << VI6_DPR_SMPPT_TGW_SHIFT) |
diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c b/drivers/media/platform/vsp1/vsp1_wpf.c
index 23c8f706b3f2..c2a1a7f97e26 100644
--- a/drivers/media/platform/vsp1/vsp1_wpf.c
+++ b/drivers/media/platform/vsp1/vsp1_wpf.c
@@ -141,13 +141,13 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf)
 	if (wpf->entity.index != 0) {
 		/* Only WPF0 supports flipping. */
 		num_flip_ctrls = 0;
-	} else if (vsp1->info->features & VSP1_HAS_WPF_HFLIP) {
+	} else if (vsp1_feature(vsp1, VSP1_HAS_WPF_HFLIP)) {
 		/*
 		 * When horizontal flip is supported the WPF implements three
 		 * controls (horizontal flip, vertical flip and rotation).
 		 */
 		num_flip_ctrls = 3;
-	} else if (vsp1->info->features & VSP1_HAS_WPF_VFLIP) {
+	} else if (vsp1_feature(vsp1, VSP1_HAS_WPF_VFLIP)) {
 		/*
 		 * When only vertical flip is supported the WPF implements a
 		 * single control (vertical flip).
@@ -276,7 +276,7 @@ static void wpf_configure_stream(struct vsp1_entity *entity,
 
 		vsp1_wpf_write(wpf, dlb, VI6_WPF_DSWAP, fmtinfo->swap);
 
-		if (vsp1->info->features & VSP1_HAS_WPF_HFLIP &&
+		if (vsp1_feature(vsp1, VSP1_HAS_WPF_HFLIP) &&
 		    wpf->entity.index == 0)
 			vsp1_wpf_write(wpf, dlb, VI6_WPF_ROT_CTRL,
 				       VI6_WPF_ROT_CTRL_LN16 |
-- 
git-series 0.9.1

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

* [PATCH v5 07/11] media: vsp1: Use header display lists for all WPF outputs linked to the DU
  2018-07-17 20:35 [PATCH v5 00/11] R-Car DU Interlaced support through VSP1 Kieran Bingham
                   ` (5 preceding siblings ...)
  2018-07-17 20:35 ` [PATCH v5 06/11] media: vsp1: Provide VSP1 feature helper macro Kieran Bingham
@ 2018-07-17 20:35 ` Kieran Bingham
  2018-07-17 20:35 ` [PATCH v5 08/11] media: vsp1: Add support for extended display list headers Kieran Bingham
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2018-07-17 20:35 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham
  Cc: linux-renesas-soc, linux-media, dri-devel, Kieran Bingham

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Header mode display lists are now supported on all WPF outputs. To
support extended headers and auto-fld capabilities for interlaced mode
handling only header mode display lists can be used.

Disable the headerless display list configuration, and remove the dead
code.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---
v5:
 - Fixed comments
 - Re-aligned header_offset calculation

 drivers/media/platform/vsp1/vsp1_dl.c | 108 ++++++---------------------
 1 file changed, 27 insertions(+), 81 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
index 3bf5b0b871b6..90e0a11c29b5 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -94,7 +94,7 @@ struct vsp1_dl_body_pool {
  * struct vsp1_dl_list - Display list
  * @list: entry in the display list manager lists
  * @dlm: the display list manager
- * @header: display list header, NULL for headerless lists
+ * @header: display list header
  * @dma: DMA address for the header
  * @body0: first display list body
  * @bodies: list of extra display list bodies
@@ -118,15 +118,9 @@ struct vsp1_dl_list {
 	bool internal;
 };
 
-enum vsp1_dl_mode {
-	VSP1_DL_MODE_HEADER,
-	VSP1_DL_MODE_HEADERLESS,
-};
-
 /**
  * struct vsp1_dl_manager - Display List manager
  * @index: index of the related WPF
- * @mode: display list operation mode (header or headerless)
  * @singleshot: execute the display list in single-shot mode
  * @vsp1: the VSP1 device
  * @lock: protects the free, active, queued, and pending lists
@@ -138,7 +132,6 @@ enum vsp1_dl_mode {
  */
 struct vsp1_dl_manager {
 	unsigned int index;
-	enum vsp1_dl_mode mode;
 	bool singleshot;
 	struct vsp1_device *vsp1;
 
@@ -318,6 +311,7 @@ void vsp1_dl_body_write(struct vsp1_dl_body *dlb, u32 reg, u32 data)
 static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm)
 {
 	struct vsp1_dl_list *dl;
+	size_t header_offset;
 
 	dl = kzalloc(sizeof(*dl), GFP_KERNEL);
 	if (!dl)
@@ -330,16 +324,14 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm)
 	dl->body0 = vsp1_dl_body_get(dlm->pool);
 	if (!dl->body0)
 		return NULL;
-	if (dlm->mode == VSP1_DL_MODE_HEADER) {
-		size_t header_offset = dl->body0->max_entries
-				     * sizeof(*dl->body0->entries);
 
-		dl->header = ((void *)dl->body0->entries) + header_offset;
-		dl->dma = dl->body0->dma + header_offset;
+	header_offset = dl->body0->max_entries * sizeof(*dl->body0->entries);
 
-		memset(dl->header, 0, sizeof(*dl->header));
-		dl->header->lists[0].addr = dl->body0->dma;
-	}
+	dl->header = ((void *)dl->body0->entries) + header_offset;
+	dl->dma = dl->body0->dma + header_offset;
+
+	memset(dl->header, 0, sizeof(*dl->header));
+	dl->header->lists[0].addr = dl->body0->dma;
 
 	return dl;
 }
@@ -471,16 +463,9 @@ struct vsp1_dl_body *vsp1_dl_list_get_body0(struct vsp1_dl_list *dl)
  *
  * The reference must be explicitly released by a call to vsp1_dl_body_put()
  * when the body isn't needed anymore.
- *
- * Additional bodies are only usable for display lists in header mode.
- * Attempting to add a body to a header-less display list will return an error.
  */
 int vsp1_dl_list_add_body(struct vsp1_dl_list *dl, struct vsp1_dl_body *dlb)
 {
-	/* Multi-body lists are only available in header mode. */
-	if (dl->dlm->mode != VSP1_DL_MODE_HEADER)
-		return -EINVAL;
-
 	refcount_inc(&dlb->refcnt);
 
 	list_add_tail(&dlb->list, &dl->bodies);
@@ -501,17 +486,10 @@ int vsp1_dl_list_add_body(struct vsp1_dl_list *dl, struct vsp1_dl_body *dlb)
  * Adding a display list to a chain passes ownership of the display list to
  * the head display list item. The chain is released when the head dl item is
  * put back with __vsp1_dl_list_put().
- *
- * Chained display lists are only usable in header mode. Attempts to add a
- * display list to a chain in header-less mode will return an error.
  */
 int vsp1_dl_list_add_chain(struct vsp1_dl_list *head,
 			   struct vsp1_dl_list *dl)
 {
-	/* Chained lists are only available in header mode. */
-	if (head->dlm->mode != VSP1_DL_MODE_HEADER)
-		return -EINVAL;
-
 	head->has_chain = true;
 	list_add_tail(&dl->chain, &head->chain);
 	return 0;
@@ -579,17 +557,10 @@ static bool vsp1_dl_list_hw_update_pending(struct vsp1_dl_manager *dlm)
 		return false;
 
 	/*
-	 * Check whether the VSP1 has taken the update. In headerless mode the
-	 * hardware indicates this by clearing the UPD bit in the DL_BODY_SIZE
-	 * register, and in header mode by clearing the UPDHDR bit in the CMD
-	 * register.
+	 * Check whether the VSP1 has taken the update. The hardware indicates
+	 * this by clearing the UPDHDR bit in the CMD register.
 	 */
-	if (dlm->mode == VSP1_DL_MODE_HEADERLESS)
-		return !!(vsp1_read(vsp1, VI6_DL_BODY_SIZE)
-			  & VI6_DL_BODY_SIZE_UPD);
-	else
-		return !!(vsp1_read(vsp1, VI6_CMD(dlm->index))
-			  & VI6_CMD_UPDHDR);
+	return !!(vsp1_read(vsp1, VI6_CMD(dlm->index)) & VI6_CMD_UPDHDR);
 }
 
 static void vsp1_dl_list_hw_enqueue(struct vsp1_dl_list *dl)
@@ -597,26 +568,14 @@ static void vsp1_dl_list_hw_enqueue(struct vsp1_dl_list *dl)
 	struct vsp1_dl_manager *dlm = dl->dlm;
 	struct vsp1_device *vsp1 = dlm->vsp1;
 
-	if (dlm->mode == VSP1_DL_MODE_HEADERLESS) {
-		/*
-		 * In headerless mode, program the hardware directly with the
-		 * display list body address and size and set the UPD bit. The
-		 * bit will be cleared by the hardware when the display list
-		 * processing starts.
-		 */
-		vsp1_write(vsp1, VI6_DL_HDR_ADDR(0), dl->body0->dma);
-		vsp1_write(vsp1, VI6_DL_BODY_SIZE, VI6_DL_BODY_SIZE_UPD |
-			(dl->body0->num_entries * sizeof(*dl->header->lists)));
-	} else {
-		/*
-		 * In header mode, program the display list header address. If
-		 * the hardware is idle (single-shot mode or first frame in
-		 * continuous mode) it will then be started independently. If
-		 * the hardware is operating, the VI6_DL_HDR_REF_ADDR register
-		 * will be updated with the display list address.
-		 */
-		vsp1_write(vsp1, VI6_DL_HDR_ADDR(dlm->index), dl->dma);
-	}
+	/*
+	 * Program the display list header address. If the hardware is idle
+	 * (single-shot mode or first frame in continuous mode) it will then be
+	 * started independently. If the hardware is operating, the
+	 * VI6_DL_HDR_REF_ADDR register will be updated with the display list
+	 * address.
+	 */
+	vsp1_write(vsp1, VI6_DL_HDR_ADDR(dlm->index), dl->dma);
 }
 
 static void vsp1_dl_list_commit_continuous(struct vsp1_dl_list *dl)
@@ -674,15 +633,13 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool internal)
 	struct vsp1_dl_list *dl_next;
 	unsigned long flags;
 
-	if (dlm->mode == VSP1_DL_MODE_HEADER) {
-		/* Fill the header for the head and chained display lists. */
-		vsp1_dl_list_fill_header(dl, list_empty(&dl->chain));
+	/* Fill the header for the head and chained display lists. */
+	vsp1_dl_list_fill_header(dl, list_empty(&dl->chain));
 
-		list_for_each_entry(dl_next, &dl->chain, chain) {
-			bool last = list_is_last(&dl_next->chain, &dl->chain);
+	list_for_each_entry(dl_next, &dl->chain, chain) {
+		bool last = list_is_last(&dl_next->chain, &dl->chain);
 
-			vsp1_dl_list_fill_header(dl_next, last);
-		}
+		vsp1_dl_list_fill_header(dl_next, last);
 	}
 
 	dl->internal = internal;
@@ -711,7 +668,7 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool internal)
  * has completed at frame end. If the flag is not returned display list
  * completion has been delayed by one frame because the display list commit
  * raced with the frame end interrupt. The function always returns with the flag
- * set in header mode as display list processing is then not continuous and
+ * set in single-shot mode as display list processing is then not continuous and
  * races never occur.
  *
  * The VSP1_DL_FRAME_END_INTERNAL flag indicates that the previous display list
@@ -783,13 +740,6 @@ void vsp1_dlm_setup(struct vsp1_device *vsp1)
 		 | VI6_DL_CTRL_DC2 | VI6_DL_CTRL_DC1 | VI6_DL_CTRL_DC0
 		 | VI6_DL_CTRL_DLE;
 
-	/*
-	 * The DRM pipeline operates with display lists in Continuous Frame
-	 * Mode, all other pipelines use manual start.
-	 */
-	if (vsp1->drm)
-		ctrl |= VI6_DL_CTRL_CFM0 | VI6_DL_CTRL_NH0;
-
 	vsp1_write(vsp1, VI6_DL_CTRL, ctrl);
 	vsp1_write(vsp1, VI6_DL_SWAP, VI6_DL_SWAP_LWS);
 }
@@ -829,8 +779,6 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
 		return NULL;
 
 	dlm->index = index;
-	dlm->mode = index == 0 && !vsp1->info->uapi
-		  ? VSP1_DL_MODE_HEADERLESS : VSP1_DL_MODE_HEADER;
 	dlm->singleshot = vsp1->info->uapi;
 	dlm->vsp1 = vsp1;
 
@@ -839,14 +787,12 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
 
 	/*
 	 * Initialize the display list body and allocate DMA memory for the body
-	 * and the optional header. Both are allocated together to avoid memory
+	 * and the header. Both are allocated together to avoid memory
 	 * fragmentation, with the header located right after the body in
 	 * memory. An extra body is allocated on top of the prealloc to account
 	 * for the cached body used by the vsp1_pipeline object.
 	 */
-	header_size = dlm->mode == VSP1_DL_MODE_HEADER
-		    ? ALIGN(sizeof(struct vsp1_dl_header), 8)
-		    : 0;
+	header_size = ALIGN(sizeof(struct vsp1_dl_header), 8);
 
 	dlm->pool = vsp1_dl_body_pool_create(vsp1, prealloc + 1,
 					     VSP1_DL_NUM_ENTRIES, header_size);
-- 
git-series 0.9.1

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

* [PATCH v5 08/11] media: vsp1: Add support for extended display list headers
  2018-07-17 20:35 [PATCH v5 00/11] R-Car DU Interlaced support through VSP1 Kieran Bingham
                   ` (6 preceding siblings ...)
  2018-07-17 20:35 ` [PATCH v5 07/11] media: vsp1: Use header display lists for all WPF outputs linked to the DU Kieran Bingham
@ 2018-07-17 20:35 ` Kieran Bingham
  2018-08-02 14:39   ` Laurent Pinchart
  2018-07-17 20:35 ` [PATCH v5 09/11] media: vsp1: Provide support for extended command pools Kieran Bingham
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Kieran Bingham @ 2018-07-17 20:35 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham
  Cc: linux-renesas-soc, linux-media, dri-devel, Kieran Bingham

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Extended display list headers allow pre and post command lists to be
executed by the VSP pipeline. This provides the base support for
features such as AUTO_FLD (for interlaced support) and AUTO_DISP (for
supporting continuous camera preview pipelines.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

---

v2:
 - remove __packed attributes

v5:
 - Rename vsp1_dl_ext_header field names
 - Rename @extended -> @extension
 - Remove unnecessary VI6_DL_SWAP changes
 - Rename @cmd_opcode -> @opcode
 - Drop unused @data_size field
 - Move iteration of WPF's inside vsp1_dlm_setup
 - Rename vsp1_dl_ext_cmd_header -> vsp1_pre_ext_dl_body
 - Rename vsp1_pre_ext_dl_body->cmd to vsp1_pre_ext_dl_body->opcode
 - Rename vsp1_dl_ext_header->reserved0 to vsp1_dl_ext_header->padding
 - vsp1_pre_ext_dl_body: Rename 'data' to 'address_set'
 - vsp1_pre_ext_dl_body: Add struct documentation
 - Document ordering of 16bit accesses for flags in vsp1_dl_ext_header

 drivers/media/platform/vsp1/vsp1.h      |   1 +-
 drivers/media/platform/vsp1/vsp1_dl.c   | 104 ++++++++++++++++++++++++-
 drivers/media/platform/vsp1/vsp1_dl.h   |  25 ++++++-
 drivers/media/platform/vsp1/vsp1_drv.c  |   4 +-
 drivers/media/platform/vsp1/vsp1_regs.h |   2 +-
 5 files changed, 132 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1.h b/drivers/media/platform/vsp1/vsp1.h
index f0d21cc8e9ab..56c62122a81a 100644
--- a/drivers/media/platform/vsp1/vsp1.h
+++ b/drivers/media/platform/vsp1/vsp1.h
@@ -53,6 +53,7 @@ struct vsp1_uif;
 #define VSP1_HAS_HGO		(1 << 7)
 #define VSP1_HAS_HGT		(1 << 8)
 #define VSP1_HAS_BRS		(1 << 9)
+#define VSP1_HAS_EXT_DL		(1 << 10)
 
 struct vsp1_device_info {
 	u32 version;
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
index 90e0a11c29b5..2fffe977aa35 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -22,6 +22,9 @@
 #define VSP1_DLH_INT_ENABLE		(1 << 1)
 #define VSP1_DLH_AUTO_START		(1 << 0)
 
+#define VSP1_DLH_EXT_PRE_CMD_EXEC	(1 << 9)
+#define VSP1_DLH_EXT_POST_CMD_EXEC	(1 << 8)
+
 struct vsp1_dl_header_list {
 	u32 num_bytes;
 	u32 addr;
@@ -34,12 +37,59 @@ struct vsp1_dl_header {
 	u32 flags;
 };
 
+/**
+ * struct vsp1_dl_ext_header - Extended display list header
+ * @padding: padding zero bytes for alignment
+ * @pre_ext_dl_num_cmd: number of pre-extended command bodies to parse
+ * @flags: enables or disables execution of the pre and post command
+ * @pre_ext_dl_plist: start address of pre-extended display list bodies
+ * @post_ext_dl_num_cmd: number of post-extended command bodies to parse
+ * @post_ext_dl_plist: start address of post-extended display list bodies
+ */
+struct vsp1_dl_ext_header {
+	u32 padding;
+
+	/*
+	 * The datasheet represents flags as stored before pre_ext_dl_num_cmd,
+	 * expecting 32-bit accesses. The flags are appropriate to the whole
+	 * header, not just the pre_ext command, and thus warrant being
+	 * separated out. Due to byte ordering, and representing as 16 bit
+	 * values here, the flags must be positioned after the
+	 * pre_ext_dl_num_cmd.
+	 */
+	u16 pre_ext_dl_num_cmd;
+	u16 flags;
+	u32 pre_ext_dl_plist;
+
+	u32 post_ext_dl_num_cmd;
+	u32 post_ext_dl_plist;
+};
+
+struct vsp1_dl_header_extended {
+	struct vsp1_dl_header header;
+	struct vsp1_dl_ext_header ext;
+};
+
 struct vsp1_dl_entry {
 	u32 addr;
 	u32 data;
 };
 
 /**
+ * struct vsp1_pre_ext_dl_body - Pre Extended Display List Body
+ * @opcode: Extended display list command operation code
+ * @flags: Pre-extended command flags. These are specific to each command
+ * @address_set: Source address set pointer. Must have 16 byte alignment
+ * @reserved: Zero bits for alignment.
+ */
+struct vsp1_pre_ext_dl_body {
+	u32 opcode;
+	u32 flags;
+	u32 address_set;
+	u32 reserved;
+};
+
+/**
  * struct vsp1_dl_body - Display list body
  * @list: entry in the display list list of bodies
  * @free: entry in the pool free body list
@@ -95,9 +145,12 @@ struct vsp1_dl_body_pool {
  * @list: entry in the display list manager lists
  * @dlm: the display list manager
  * @header: display list header
+ * @extension: extended display list header. NULL for normal lists
  * @dma: DMA address for the header
  * @body0: first display list body
  * @bodies: list of extra display list bodies
+ * @pre_cmd: pre command to be issued through extended dl header
+ * @post_cmd: post command to be issued through extended dl header
  * @has_chain: if true, indicates that there's a partition chain
  * @chain: entry in the display list partition chain
  * @internal: whether the display list is used for internal purpose
@@ -107,11 +160,15 @@ struct vsp1_dl_list {
 	struct vsp1_dl_manager *dlm;
 
 	struct vsp1_dl_header *header;
+	struct vsp1_dl_ext_header *extension;
 	dma_addr_t dma;
 
 	struct vsp1_dl_body *body0;
 	struct list_head bodies;
 
+	struct vsp1_dl_ext_cmd *pre_cmd;
+	struct vsp1_dl_ext_cmd *post_cmd;
+
 	bool has_chain;
 	struct list_head chain;
 
@@ -495,6 +552,14 @@ int vsp1_dl_list_add_chain(struct vsp1_dl_list *head,
 	return 0;
 }
 
+static void vsp1_dl_ext_cmd_fill_header(struct vsp1_dl_ext_cmd *cmd)
+{
+	cmd->cmds[0].opcode = cmd->opcode;
+	cmd->cmds[0].flags = cmd->flags;
+	cmd->cmds[0].address_set = cmd->data_dma;
+	cmd->cmds[0].reserved = 0;
+}
+
 static void vsp1_dl_list_fill_header(struct vsp1_dl_list *dl, bool is_last)
 {
 	struct vsp1_dl_manager *dlm = dl->dlm;
@@ -547,6 +612,27 @@ static void vsp1_dl_list_fill_header(struct vsp1_dl_list *dl, bool is_last)
 		 */
 		dl->header->flags = VSP1_DLH_INT_ENABLE;
 	}
+
+	if (!dl->extension)
+		return;
+
+	dl->extension->flags = 0;
+
+	if (dl->pre_cmd) {
+		dl->extension->pre_ext_dl_plist = dl->pre_cmd->cmd_dma;
+		dl->extension->pre_ext_dl_num_cmd = dl->pre_cmd->num_cmds;
+		dl->extension->flags |= VSP1_DLH_EXT_PRE_CMD_EXEC;
+
+		vsp1_dl_ext_cmd_fill_header(dl->pre_cmd);
+	}
+
+	if (dl->post_cmd) {
+		dl->extension->post_ext_dl_plist = dl->post_cmd->cmd_dma;
+		dl->extension->post_ext_dl_num_cmd = dl->post_cmd->num_cmds;
+		dl->extension->flags |= VSP1_DLH_EXT_POST_CMD_EXEC;
+
+		vsp1_dl_ext_cmd_fill_header(dl->post_cmd);
+	}
 }
 
 static bool vsp1_dl_list_hw_update_pending(struct vsp1_dl_manager *dlm)
@@ -736,9 +822,16 @@ unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
 /* Hardware Setup */
 void vsp1_dlm_setup(struct vsp1_device *vsp1)
 {
+	unsigned int i;
 	u32 ctrl = (256 << VI6_DL_CTRL_AR_WAIT_SHIFT)
 		 | VI6_DL_CTRL_DC2 | VI6_DL_CTRL_DC1 | VI6_DL_CTRL_DC0
 		 | VI6_DL_CTRL_DLE;
+	u32 ext_dl = (0x02 << VI6_DL_EXT_CTRL_POLINT_SHIFT) |
+		      VI6_DL_EXT_CTRL_DLPRI | VI6_DL_EXT_CTRL_EXT;
+
+	if (vsp1_feature(vsp1, VSP1_HAS_EXT_DL))
+		for (i = 0; i < vsp1->info->wpf_count; ++i)
+			vsp1_write(vsp1, VI6_DL_EXT_CTRL(i), ext_dl);
 
 	vsp1_write(vsp1, VI6_DL_CTRL, ctrl);
 	vsp1_write(vsp1, VI6_DL_SWAP, VI6_DL_SWAP_LWS);
@@ -792,7 +885,11 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
 	 * memory. An extra body is allocated on top of the prealloc to account
 	 * for the cached body used by the vsp1_pipeline object.
 	 */
-	header_size = ALIGN(sizeof(struct vsp1_dl_header), 8);
+	header_size = vsp1_feature(vsp1, VSP1_HAS_EXT_DL) ?
+			sizeof(struct vsp1_dl_header_extended) :
+			sizeof(struct vsp1_dl_header);
+
+	header_size = ALIGN(header_size, 8);
 
 	dlm->pool = vsp1_dl_body_pool_create(vsp1, prealloc + 1,
 					     VSP1_DL_NUM_ENTRIES, header_size);
@@ -808,6 +905,11 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
 			return NULL;
 		}
 
+		/* The extended header immediately follows the header. */
+		if (vsp1_feature(vsp1, VSP1_HAS_EXT_DL))
+			dl->extension = (void *)dl->header
+				      + sizeof(*dl->header);
+
 		list_add_tail(&dl->list, &dlm->free);
 	}
 
diff --git a/drivers/media/platform/vsp1/vsp1_dl.h b/drivers/media/platform/vsp1/vsp1_dl.h
index 7dba0469c92e..afefd5bfa136 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.h
+++ b/drivers/media/platform/vsp1/vsp1_dl.h
@@ -20,6 +20,31 @@ struct vsp1_dl_manager;
 #define VSP1_DL_FRAME_END_COMPLETED		BIT(0)
 #define VSP1_DL_FRAME_END_INTERNAL		BIT(1)
 
+/**
+ * struct vsp1_dl_ext_cmd - Extended Display command
+ * @free: entry in the pool of free commands list
+ * @opcode: command type opcode
+ * @flags: flags used by the command
+ * @cmds: array of command bodies for this extended cmd
+ * @num_cmds: quantity of commands in @cmds array
+ * @cmd_dma: DMA address of the command body
+ * @data: memory allocation for command-specific data
+ * @data_dma: DMA address for command-specific data
+ */
+struct vsp1_dl_ext_cmd {
+	struct list_head free;
+
+	u8 opcode;
+	u32 flags;
+
+	struct vsp1_pre_ext_dl_body *cmds;
+	unsigned int num_cmds;
+	dma_addr_t cmd_dma;
+
+	void *data;
+	dma_addr_t data_dma;
+};
+
 void vsp1_dlm_setup(struct vsp1_device *vsp1);
 
 struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
index 3367c2ba990d..b6619c9c18bb 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -754,7 +754,7 @@ static const struct vsp1_device_info vsp1_device_infos[] = {
 		.version = VI6_IP_VERSION_MODEL_VSPD_GEN3,
 		.model = "VSP2-D",
 		.gen = 3,
-		.features = VSP1_HAS_BRU | VSP1_HAS_WPF_VFLIP,
+		.features = VSP1_HAS_BRU | VSP1_HAS_WPF_VFLIP | VSP1_HAS_EXT_DL,
 		.lif_count = 1,
 		.rpf_count = 5,
 		.uif_count = 1,
@@ -774,7 +774,7 @@ static const struct vsp1_device_info vsp1_device_infos[] = {
 		.version = VI6_IP_VERSION_MODEL_VSPDL_GEN3,
 		.model = "VSP2-DL",
 		.gen = 3,
-		.features = VSP1_HAS_BRS | VSP1_HAS_BRU,
+		.features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_EXT_DL,
 		.lif_count = 2,
 		.rpf_count = 5,
 		.uif_count = 2,
diff --git a/drivers/media/platform/vsp1/vsp1_regs.h b/drivers/media/platform/vsp1/vsp1_regs.h
index 0d249ff9f564..5ea9f9070cf3 100644
--- a/drivers/media/platform/vsp1/vsp1_regs.h
+++ b/drivers/media/platform/vsp1/vsp1_regs.h
@@ -72,7 +72,7 @@
 #define VI6_DL_SWAP_WDS			(1 << 1)
 #define VI6_DL_SWAP_BTS			(1 << 0)
 
-#define VI6_DL_EXT_CTRL			0x011c
+#define VI6_DL_EXT_CTRL(n)		(0x011c + (n) * 36)
 #define VI6_DL_EXT_CTRL_NWE		(1 << 16)
 #define VI6_DL_EXT_CTRL_POLINT_MASK	(0x3f << 8)
 #define VI6_DL_EXT_CTRL_POLINT_SHIFT	8
-- 
git-series 0.9.1

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

* [PATCH v5 09/11] media: vsp1: Provide support for extended command pools
  2018-07-17 20:35 [PATCH v5 00/11] R-Car DU Interlaced support through VSP1 Kieran Bingham
                   ` (7 preceding siblings ...)
  2018-07-17 20:35 ` [PATCH v5 08/11] media: vsp1: Add support for extended display list headers Kieran Bingham
@ 2018-07-17 20:35 ` Kieran Bingham
  2018-08-02 14:58   ` Laurent Pinchart
  2018-07-17 20:35 ` [PATCH v5 10/11] media: vsp1: Support Interlaced display pipelines Kieran Bingham
  2018-07-17 20:35 ` [PATCH v5 11/11] drm: rcar-du: Support interlaced video output through vsp1 Kieran Bingham
  10 siblings, 1 reply; 18+ messages in thread
From: Kieran Bingham @ 2018-07-17 20:35 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham
  Cc: linux-renesas-soc, linux-media, dri-devel, Kieran Bingham

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

VSPD and VSP-DL devices can provide extended display lists supporting
extended command display list objects.

These extended commands require their own dma memory areas for a header
and body specific to the command type.

Implement a command pool to allocate all necessary memory in a single
DMA allocation to reduce pressure on the TLB, and provide convenient
re-usable command objects for the entities to utilise.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

---
v2:
 - Fix spelling typo in commit message
 - constify, and staticify the instantiation of vsp1_extended_commands
 - s/autfld_cmds/autofld_cmds/
 - staticify cmd pool functions (Thanks kbuild-bot)

v5:
 - Rename vsp1_dl_ext_cmd_header -> vsp1_pre_ext_dl_body
 - fixup vsp1_cmd_pool structure documentation
 - Rename dlm->autofld_cmds dlm->cmdpool
 - Separate out the instatiation of vsp1_extended_commands
 - Move initialisation of lock, and lists in vsp1_dl_cmd_pool_create to
   immediately after allocation
 - simplify vsp1_dlm_get_autofld_cmd
 - Rename vsp1_dl_get_autofld_cmd() to vsp1_dl_get_pre_cmd() and moved
   to "Display List Extended Command Management" section of vsp1_dl

 drivers/media/platform/vsp1/vsp1_dl.c | 194 +++++++++++++++++++++++++++-
 drivers/media/platform/vsp1/vsp1_dl.h |   3 +-
 2 files changed, 197 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
index 2fffe977aa35..d5b3c24d160c 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -141,6 +141,30 @@ struct vsp1_dl_body_pool {
 };
 
 /**
+ * struct vsp1_cmd_pool - Display List commands pool
+ * @dma: DMA address of the entries
+ * @size: size of the full DMA memory pool in bytes
+ * @mem: CPU memory pointer for the pool
+ * @cmds: Array of command structures for the pool
+ * @free: Free pool entries
+ * @lock: Protects the free list
+ * @vsp1: the VSP1 device
+ */
+struct vsp1_dl_cmd_pool {
+	/* DMA allocation */
+	dma_addr_t dma;
+	size_t size;
+	void *mem;
+
+	struct vsp1_dl_ext_cmd *cmds;
+	struct list_head free;
+
+	spinlock_t lock;
+
+	struct vsp1_device *vsp1;
+};
+
+/**
  * struct vsp1_dl_list - Display list
  * @list: entry in the display list manager lists
  * @dlm: the display list manager
@@ -186,6 +210,7 @@ struct vsp1_dl_list {
  * @queued: list queued to the hardware (written to the DL registers)
  * @pending: list waiting to be queued to the hardware
  * @pool: body pool for the display list bodies
+ * @autofld_cmds: command pool to support auto-fld interlaced mode
  */
 struct vsp1_dl_manager {
 	unsigned int index;
@@ -199,6 +224,7 @@ struct vsp1_dl_manager {
 	struct vsp1_dl_list *pending;
 
 	struct vsp1_dl_body_pool *pool;
+	struct vsp1_dl_cmd_pool *cmdpool;
 };
 
 /* -----------------------------------------------------------------------------
@@ -362,6 +388,157 @@ void vsp1_dl_body_write(struct vsp1_dl_body *dlb, u32 reg, u32 data)
 }
 
 /* -----------------------------------------------------------------------------
+ * Display List Extended Command Management
+ */
+
+enum vsp1_extcmd_type {
+	VSP1_EXTCMD_AUTODISP,
+	VSP1_EXTCMD_AUTOFLD,
+};
+
+struct vsp1_extended_command_info {
+	u16 opcode;
+	size_t body_size;
+};
+
+static const struct vsp1_extended_command_info vsp1_extended_commands[] = {
+	[VSP1_EXTCMD_AUTODISP] = { 0x02, 96 },
+	[VSP1_EXTCMD_AUTOFLD]  = { 0x03, 160 },
+};
+
+/**
+ * vsp1_dl_cmd_pool_create - Create a pool of commands from a single allocation
+ * @vsp1: The VSP1 device
+ * @type: The command pool type
+ * @num_cmds: The number of commands to allocate
+ *
+ * Allocate a pool of commands each with enough memory to contain the private
+ * data of each command. The allocation sizes are dependent upon the command
+ * type.
+ *
+ * Return a pointer to the pool on success or NULL if memory can't be allocated.
+ */
+static struct vsp1_dl_cmd_pool *
+vsp1_dl_cmd_pool_create(struct vsp1_device *vsp1, enum vsp1_extcmd_type type,
+			unsigned int num_cmds)
+{
+	struct vsp1_dl_cmd_pool *pool;
+	unsigned int i;
+	size_t cmd_size;
+
+	pool = kzalloc(sizeof(*pool), GFP_KERNEL);
+	if (!pool)
+		return NULL;
+
+	spin_lock_init(&pool->lock);
+	INIT_LIST_HEAD(&pool->free);
+
+	pool->cmds = kcalloc(num_cmds, sizeof(*pool->cmds), GFP_KERNEL);
+	if (!pool->cmds) {
+		kfree(pool);
+		return NULL;
+	}
+
+	cmd_size = sizeof(struct vsp1_pre_ext_dl_body) +
+		   vsp1_extended_commands[type].body_size;
+	cmd_size = ALIGN(cmd_size, 16);
+
+	pool->size = cmd_size * num_cmds;
+	pool->mem = dma_alloc_wc(vsp1->bus_master, pool->size, &pool->dma,
+				 GFP_KERNEL);
+	if (!pool->mem) {
+		kfree(pool->cmds);
+		kfree(pool);
+		return NULL;
+	}
+
+	for (i = 0; i < num_cmds; ++i) {
+		struct vsp1_dl_ext_cmd *cmd = &pool->cmds[i];
+		size_t cmd_offset = i * cmd_size;
+		/* data_offset must be 16 byte aligned for DMA. */
+		size_t data_offset = sizeof(struct vsp1_pre_ext_dl_body) +
+				     cmd_offset;
+
+		cmd->pool = pool;
+		cmd->opcode = vsp1_extended_commands[type].opcode;
+
+		/*
+		 * TODO: Auto-disp can utilise more than one extended body
+		 * command per cmd.
+		 */
+		cmd->num_cmds = 1;
+		cmd->cmds = pool->mem + cmd_offset;
+		cmd->cmd_dma = pool->dma + cmd_offset;
+
+		cmd->data = pool->mem + data_offset;
+		cmd->data_dma = pool->dma + data_offset;
+
+		list_add_tail(&cmd->free, &pool->free);
+	}
+
+	return pool;
+}
+
+static
+struct vsp1_dl_ext_cmd *vsp1_dl_ext_cmd_get(struct vsp1_dl_cmd_pool *pool)
+{
+	struct vsp1_dl_ext_cmd *cmd = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pool->lock, flags);
+
+	if (!list_empty(&pool->free)) {
+		cmd = list_first_entry(&pool->free, struct vsp1_dl_ext_cmd,
+				       free);
+		list_del(&cmd->free);
+	}
+
+	spin_unlock_irqrestore(&pool->lock, flags);
+
+	return cmd;
+}
+
+static void vsp1_dl_ext_cmd_put(struct vsp1_dl_ext_cmd *cmd)
+{
+	unsigned long flags;
+
+	if (!cmd)
+		return;
+
+	/* Reset flags, these mark data usage. */
+	cmd->flags = 0;
+
+	spin_lock_irqsave(&cmd->pool->lock, flags);
+	list_add_tail(&cmd->free, &cmd->pool->free);
+	spin_unlock_irqrestore(&cmd->pool->lock, flags);
+}
+
+static void vsp1_dl_ext_cmd_pool_destroy(struct vsp1_dl_cmd_pool *pool)
+{
+	if (!pool)
+		return;
+
+	if (pool->mem)
+		dma_free_wc(pool->vsp1->bus_master, pool->size, pool->mem,
+			    pool->dma);
+
+	kfree(pool->cmds);
+	kfree(pool);
+}
+
+struct vsp1_dl_ext_cmd *vsp1_dl_get_pre_cmd(struct vsp1_dl_list *dl)
+{
+	struct vsp1_dl_manager *dlm = dl->dlm;
+
+	if (dl->pre_cmd)
+		return dl->pre_cmd;
+
+	dl->pre_cmd = vsp1_dl_ext_cmd_get(dlm->cmdpool);
+
+	return dl->pre_cmd;
+}
+
+/* ----------------------------------------------------------------------------
  * Display List Transaction Management
  */
 
@@ -463,6 +640,12 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl)
 
 	vsp1_dl_list_bodies_put(dl);
 
+	vsp1_dl_ext_cmd_put(dl->pre_cmd);
+	vsp1_dl_ext_cmd_put(dl->post_cmd);
+
+	dl->pre_cmd = NULL;
+	dl->post_cmd = NULL;
+
 	/*
 	 * body0 is reused as as an optimisation as presently every display list
 	 * has at least one body, thus we reinitialise the entries list.
@@ -913,6 +1096,15 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
 		list_add_tail(&dl->list, &dlm->free);
 	}
 
+	if (vsp1_feature(vsp1, VSP1_HAS_EXT_DL)) {
+		dlm->cmdpool = vsp1_dl_cmd_pool_create(vsp1,
+					VSP1_EXTCMD_AUTOFLD, prealloc);
+		if (!dlm->cmdpool) {
+			vsp1_dlm_destroy(dlm);
+			return NULL;
+		}
+	}
+
 	return dlm;
 }
 
@@ -929,4 +1121,6 @@ void vsp1_dlm_destroy(struct vsp1_dl_manager *dlm)
 	}
 
 	vsp1_dl_body_pool_destroy(dlm->pool);
+	vsp1_dl_ext_cmd_pool_destroy(dlm->cmdpool);
 }
+
diff --git a/drivers/media/platform/vsp1/vsp1_dl.h b/drivers/media/platform/vsp1/vsp1_dl.h
index afefd5bfa136..125750dc8b5c 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.h
+++ b/drivers/media/platform/vsp1/vsp1_dl.h
@@ -22,6 +22,7 @@ struct vsp1_dl_manager;
 
 /**
  * struct vsp1_dl_ext_cmd - Extended Display command
+ * @pool: pool to which this command belongs
  * @free: entry in the pool of free commands list
  * @opcode: command type opcode
  * @flags: flags used by the command
@@ -32,6 +33,7 @@ struct vsp1_dl_manager;
  * @data_dma: DMA address for command-specific data
  */
 struct vsp1_dl_ext_cmd {
+	struct vsp1_dl_cmd_pool *pool;
 	struct list_head free;
 
 	u8 opcode;
@@ -58,6 +60,7 @@ struct vsp1_dl_body *vsp1_dlm_dl_body_get(struct vsp1_dl_manager *dlm);
 struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm);
 void vsp1_dl_list_put(struct vsp1_dl_list *dl);
 struct vsp1_dl_body *vsp1_dl_list_get_body0(struct vsp1_dl_list *dl);
+struct vsp1_dl_ext_cmd *vsp1_dl_get_pre_cmd(struct vsp1_dl_list *dl);
 void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool internal);
 
 struct vsp1_dl_body_pool *
-- 
git-series 0.9.1

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

* [PATCH v5 10/11] media: vsp1: Support Interlaced display pipelines
  2018-07-17 20:35 [PATCH v5 00/11] R-Car DU Interlaced support through VSP1 Kieran Bingham
                   ` (8 preceding siblings ...)
  2018-07-17 20:35 ` [PATCH v5 09/11] media: vsp1: Provide support for extended command pools Kieran Bingham
@ 2018-07-17 20:35 ` Kieran Bingham
  2018-08-02 22:44   ` Laurent Pinchart
  2018-07-17 20:35 ` [PATCH v5 11/11] drm: rcar-du: Support interlaced video output through vsp1 Kieran Bingham
  10 siblings, 1 reply; 18+ messages in thread
From: Kieran Bingham @ 2018-07-17 20:35 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham
  Cc: linux-renesas-soc, linux-media, dri-devel, Kieran Bingham

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Calculate the top and bottom fields for the interlaced frames and
utilise the extended display list command feature to implement the
auto-field operations. This allows the DU to update the VSP2 registers
dynamically based upon the currently processing field.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

---

v2:
 - fix erroneous BIT value which enabled interlaced
 - fix field handling at frame_end interrupt

v3:
 - Pass DL through partition calls to allow autocmd's to be retrieved
 - Document interlaced field in struct vsp1_du_atomic_config

v5:
 - Obtain autofld cmd in vsp1_rpf_configure_autofld()
 - Move VSP1_DL_EXT_AUTOFLD_INT to vsp1_regs.h
 - Rename VSP1_DL_EXT_AUTOFLD_INT -> VI6_DL_EXT_AUTOFLD_INT
 - move interlaced configuration parameter to pipe object
 - autofld: Set cmd->flags in one single expression.

 drivers/media/platform/vsp1/vsp1_dl.c   | 10 ++++-
 drivers/media/platform/vsp1/vsp1_drm.c  | 16 +++++-
 drivers/media/platform/vsp1/vsp1_pipe.h |  2 +-
 drivers/media/platform/vsp1/vsp1_regs.h |  3 +-
 drivers/media/platform/vsp1/vsp1_rpf.c  | 72 ++++++++++++++++++++++++--
 include/media/vsp1.h                    |  2 +-
 6 files changed, 100 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
index d5b3c24d160c..4963acac73f8 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -946,6 +946,8 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool internal)
  */
 unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
 {
+	struct vsp1_device *vsp1 = dlm->vsp1;
+	u32 status = vsp1_read(vsp1, VI6_STATUS);
 	unsigned int flags = 0;
 
 	spin_lock(&dlm->lock);
@@ -971,6 +973,14 @@ unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
 		goto done;
 
 	/*
+	 * Progressive streams report only TOP fields. If we have a BOTTOM
+	 * field, we are interlaced, and expect the frame to complete on the
+	 * next frame end interrupt.
+	 */
+	if (status & VI6_STATUS_FLD_STD(dlm->index))
+		goto done;
+
+	/*
 	 * The device starts processing the queued display list right after the
 	 * frame end interrupt. The display list thus becomes active.
 	 */
diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c
index a16856856789..efdc9a676728 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -671,8 +671,20 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
 	drm_pipe->width = cfg->width;
 	drm_pipe->height = cfg->height;
 
-	dev_dbg(vsp1->dev, "%s: configuring LIF%u with format %ux%u\n",
-		__func__, pipe_index, cfg->width, cfg->height);
+	if (cfg->interlaced && !vsp1_feature(vsp1, VSP1_HAS_EXT_DL)) {
+		/*
+		 * Interlaced support requires extended display lists to
+		 * provide the auto-fld feature with the DU.
+		 */
+		dev_dbg(vsp1->dev, "Interlaced unsupported on this pipeline\n");
+		return -EINVAL;
+	}
+
+	pipe->interlaced = cfg->interlaced;
+
+	dev_dbg(vsp1->dev, "%s: configuring LIF%u with format %ux%u%s\n",
+		__func__, pipe_index, cfg->width, cfg->height,
+		pipe->interlaced ? "i" : "");
 
 	mutex_lock(&vsp1->drm->lock);
 
diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h
index 743d8f0db45c..ae646c9ef337 100644
--- a/drivers/media/platform/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/vsp1/vsp1_pipe.h
@@ -104,6 +104,7 @@ struct vsp1_partition {
  * @entities: list of entities in the pipeline
  * @stream_config: cached stream configuration for video pipelines
  * @configured: when false the @stream_config shall be written to the hardware
+ * @interlaced: True when the pipeline is configured in interlaced mode
  * @partitions: The number of partitions used to process one frame
  * @partition: The current partition for configuration to process
  * @part_table: The pre-calculated partitions used by the pipeline
@@ -142,6 +143,7 @@ struct vsp1_pipeline {
 
 	struct vsp1_dl_body *stream_config;
 	bool configured;
+	bool interlaced;
 
 	unsigned int partitions;
 	struct vsp1_partition *partition;
diff --git a/drivers/media/platform/vsp1/vsp1_regs.h b/drivers/media/platform/vsp1/vsp1_regs.h
index 5ea9f9070cf3..3738ff2f7b85 100644
--- a/drivers/media/platform/vsp1/vsp1_regs.h
+++ b/drivers/media/platform/vsp1/vsp1_regs.h
@@ -28,6 +28,7 @@
 #define VI6_SRESET_SRTS(n)		(1 << (n))
 
 #define VI6_STATUS			0x0038
+#define VI6_STATUS_FLD_STD(n)		(1 << ((n) + 28))
 #define VI6_STATUS_SYS_ACT(n)		(1 << ((n) + 8))
 
 #define VI6_WPF_IRQ_ENB(n)		(0x0048 + (n) * 12)
@@ -80,6 +81,8 @@
 #define VI6_DL_EXT_CTRL_EXPRI		(1 << 4)
 #define VI6_DL_EXT_CTRL_EXT		(1 << 0)
 
+#define VI6_DL_EXT_AUTOFLD_INT		BIT(0)
+
 #define VI6_DL_BODY_SIZE		0x0120
 #define VI6_DL_BODY_SIZE_UPD		(1 << 24)
 #define VI6_DL_BODY_SIZE_BS_MASK	(0x1ffff << 0)
diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c
index 69e5fe6e6b50..c728c193e09c 100644
--- a/drivers/media/platform/vsp1/vsp1_rpf.c
+++ b/drivers/media/platform/vsp1/vsp1_rpf.c
@@ -20,6 +20,18 @@
 #define RPF_MAX_WIDTH				8190
 #define RPF_MAX_HEIGHT				8190
 
+/* Pre extended display list command data structure. */
+struct vsp1_extcmd_auto_fld_body {
+	u32 top_y0;
+	u32 bottom_y0;
+	u32 top_c0;
+	u32 bottom_c0;
+	u32 top_c1;
+	u32 bottom_c1;
+	u32 reserved0;
+	u32 reserved1;
+};
+
 /* -----------------------------------------------------------------------------
  * Device Access
  */
@@ -64,6 +76,14 @@ static void rpf_configure_stream(struct vsp1_entity *entity,
 		pstride |= format->plane_fmt[1].bytesperline
 			<< VI6_RPF_SRCM_PSTRIDE_C_SHIFT;
 
+	/*
+	 * pstride has both STRIDE_Y and STRIDE_C, but multiplying the whole
+	 * of pstride by 2 is conveniently OK here as we are multiplying both
+	 * values.
+	 */
+	if (pipe->interlaced)
+		pstride *= 2;
+
 	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_PSTRIDE, pstride);
 
 	/* Format */
@@ -100,6 +120,9 @@ static void rpf_configure_stream(struct vsp1_entity *entity,
 		top = compose->top;
 	}
 
+	if (pipe->interlaced)
+		top /= 2;
+
 	vsp1_rpf_write(rpf, dlb, VI6_RPF_LOC,
 		       (left << VI6_RPF_LOC_HCOORD_SHIFT) |
 		       (top << VI6_RPF_LOC_VCOORD_SHIFT));
@@ -169,6 +192,36 @@ static void rpf_configure_stream(struct vsp1_entity *entity,
 
 }
 
+static void vsp1_rpf_configure_autofld(struct vsp1_rwpf *rpf,
+				       struct vsp1_dl_list *dl)
+{
+	const struct v4l2_pix_format_mplane *format = &rpf->format;
+	struct vsp1_dl_ext_cmd *cmd;
+	struct vsp1_extcmd_auto_fld_body *auto_fld;
+	u32 offset_y, offset_c;
+
+	cmd = vsp1_dl_get_pre_cmd(dl);
+	if (WARN_ONCE(!cmd, "Failed to obtain an autofld cmd"))
+		return;
+
+	/* Re-index our auto_fld to match the current RPF. */
+	auto_fld = cmd->data;
+	auto_fld = &auto_fld[rpf->entity.index];
+
+	auto_fld->top_y0 = rpf->mem.addr[0];
+	auto_fld->top_c0 = rpf->mem.addr[1];
+	auto_fld->top_c1 = rpf->mem.addr[2];
+
+	offset_y = format->plane_fmt[0].bytesperline;
+	offset_c = format->plane_fmt[1].bytesperline;
+
+	auto_fld->bottom_y0 = rpf->mem.addr[0] + offset_y;
+	auto_fld->bottom_c0 = rpf->mem.addr[1] + offset_c;
+	auto_fld->bottom_c1 = rpf->mem.addr[2] + offset_c;
+
+	cmd->flags |= VI6_DL_EXT_AUTOFLD_INT | BIT(16 + rpf->entity.index);
+}
+
 static void rpf_configure_frame(struct vsp1_entity *entity,
 				struct vsp1_pipeline *pipe,
 				struct vsp1_dl_list *dl,
@@ -221,6 +274,11 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
 		crop.left += pipe->partition->rpf.left;
 	}
 
+	if (pipe->interlaced) {
+		crop.height = round_down(crop.height / 2, fmtinfo->vsub);
+		crop.top = round_down(crop.top / 2, fmtinfo->vsub);
+	}
+
 	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRC_BSIZE,
 		       (crop.width << VI6_RPF_SRC_BSIZE_BHSIZE_SHIFT) |
 		       (crop.height << VI6_RPF_SRC_BSIZE_BVSIZE_SHIFT));
@@ -249,9 +307,17 @@ static void rpf_configure_partition(struct vsp1_entity *entity,
 	    fmtinfo->swap_uv)
 		swap(mem.addr[1], mem.addr[2]);
 
-	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_Y, mem.addr[0]);
-	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_C0, mem.addr[1]);
-	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_C1, mem.addr[2]);
+	/*
+	 * Interlaced pipelines will use the extended pre-cmd to process
+	 * SRCM_ADDR_{Y,C0,C1}
+	 */
+	if (pipe->interlaced) {
+		vsp1_rpf_configure_autofld(rpf, dl);
+	} else {
+		vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_Y, mem.addr[0]);
+		vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_C0, mem.addr[1]);
+		vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_C1, mem.addr[2]);
+	}
 }
 
 static void rpf_partition(struct vsp1_entity *entity,
diff --git a/include/media/vsp1.h b/include/media/vsp1.h
index 678c24de1ac6..3093b9cb9067 100644
--- a/include/media/vsp1.h
+++ b/include/media/vsp1.h
@@ -25,6 +25,7 @@ int vsp1_du_init(struct device *dev);
  * struct vsp1_du_lif_config - VSP LIF configuration
  * @width: output frame width
  * @height: output frame height
+ * @interlaced: true for interlaced pipelines
  * @callback: frame completion callback function (optional). When a callback
  *	      is provided, the VSP driver guarantees that it will be called once
  *	      and only once for each vsp1_du_atomic_flush() call.
@@ -33,6 +34,7 @@ int vsp1_du_init(struct device *dev);
 struct vsp1_du_lif_config {
 	unsigned int width;
 	unsigned int height;
+	bool interlaced;
 
 	void (*callback)(void *data, bool completed, u32 crc);
 	void *callback_data;
-- 
git-series 0.9.1

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

* [PATCH v5 11/11] drm: rcar-du: Support interlaced video output through vsp1
  2018-07-17 20:35 [PATCH v5 00/11] R-Car DU Interlaced support through VSP1 Kieran Bingham
                   ` (9 preceding siblings ...)
  2018-07-17 20:35 ` [PATCH v5 10/11] media: vsp1: Support Interlaced display pipelines Kieran Bingham
@ 2018-07-17 20:35 ` Kieran Bingham
  2018-08-02 22:46   ` Laurent Pinchart
  10 siblings, 1 reply; 18+ messages in thread
From: Kieran Bingham @ 2018-07-17 20:35 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham
  Cc: linux-renesas-soc, linux-media, dri-devel, Kieran Bingham

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Use the newly exposed VSP1 interface to enable interlaced frame support
through the VSP1 LIF pipelines.

The DSMR register is updated to set the ODEV flag on interlaced
pipelines, thus defining an interlaced stream as having the ODD field
located in the second half (BOTTOM) of the frame buffer.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

---
v5
 - Fix commit title
 - Document change to DSMR
 - Configure through vsp1_du_setup_lif(), rather than
   vsp1_du_atomic_update()

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 1 +
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 15dc9caa128b..b52b3e817b93 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -289,6 +289,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 	/* Signal polarities */
 	value = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
 	      | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? DSMR_HSL : 0)
+	      | ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? DSMR_ODEV : 0)
 	      | DSMR_DIPM_DISP | DSMR_CSPM;
 	rcar_du_crtc_write(rcrtc, DSMR, value);
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 72eebeda518e..a042f116731b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -52,6 +52,7 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
 	struct vsp1_du_lif_config cfg = {
 		.width = mode->hdisplay,
 		.height = mode->vdisplay,
+		.interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE,
 		.callback = rcar_du_vsp_complete,
 		.callback_data = crtc,
 	};
-- 
git-series 0.9.1

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

* Re: [PATCH v5 08/11] media: vsp1: Add support for extended display list headers
  2018-07-17 20:35 ` [PATCH v5 08/11] media: vsp1: Add support for extended display list headers Kieran Bingham
@ 2018-08-02 14:39   ` Laurent Pinchart
  2018-08-03  9:47     ` Kieran Bingham
  0 siblings, 1 reply; 18+ messages in thread
From: Laurent Pinchart @ 2018-08-02 14:39 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: Kieran Bingham, linux-renesas-soc, linux-media, dri-devel,
	Kieran Bingham

Hi Kieran,

Thank you for the patch.

On Tuesday, 17 July 2018 23:35:50 EEST Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> Extended display list headers allow pre and post command lists to be
> executed by the VSP pipeline. This provides the base support for
> features such as AUTO_FLD (for interlaced support) and AUTO_DISP (for
> supporting continuous camera preview pipelines.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> ---
> 
> v2:
>  - remove __packed attributes
> 
> v5:
>  - Rename vsp1_dl_ext_header field names
>  - Rename @extended -> @extension
>  - Remove unnecessary VI6_DL_SWAP changes
>  - Rename @cmd_opcode -> @opcode
>  - Drop unused @data_size field
>  - Move iteration of WPF's inside vsp1_dlm_setup
>  - Rename vsp1_dl_ext_cmd_header -> vsp1_pre_ext_dl_body
>  - Rename vsp1_pre_ext_dl_body->cmd to vsp1_pre_ext_dl_body->opcode
>  - Rename vsp1_dl_ext_header->reserved0 to vsp1_dl_ext_header->padding
>  - vsp1_pre_ext_dl_body: Rename 'data' to 'address_set'
>  - vsp1_pre_ext_dl_body: Add struct documentation
>  - Document ordering of 16bit accesses for flags in vsp1_dl_ext_header
> 
>  drivers/media/platform/vsp1/vsp1.h      |   1 +-
>  drivers/media/platform/vsp1/vsp1_dl.c   | 104 ++++++++++++++++++++++++-
>  drivers/media/platform/vsp1/vsp1_dl.h   |  25 ++++++-
>  drivers/media/platform/vsp1/vsp1_drv.c  |   4 +-
>  drivers/media/platform/vsp1/vsp1_regs.h |   2 +-
>  5 files changed, 132 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1.h
> b/drivers/media/platform/vsp1/vsp1.h index f0d21cc8e9ab..56c62122a81a
> 100644
> --- a/drivers/media/platform/vsp1/vsp1.h
> +++ b/drivers/media/platform/vsp1/vsp1.h
> @@ -53,6 +53,7 @@ struct vsp1_uif;
>  #define VSP1_HAS_HGO		(1 << 7)
>  #define VSP1_HAS_HGT		(1 << 8)
>  #define VSP1_HAS_BRS		(1 << 9)
> +#define VSP1_HAS_EXT_DL		(1 << 10)
> 
>  struct vsp1_device_info {
>  	u32 version;
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c index 90e0a11c29b5..2fffe977aa35
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -22,6 +22,9 @@
>  #define VSP1_DLH_INT_ENABLE		(1 << 1)
>  #define VSP1_DLH_AUTO_START		(1 << 0)
> 
> +#define VSP1_DLH_EXT_PRE_CMD_EXEC	(1 << 9)
> +#define VSP1_DLH_EXT_POST_CMD_EXEC	(1 << 8)
> +
>  struct vsp1_dl_header_list {
>  	u32 num_bytes;
>  	u32 addr;
> @@ -34,12 +37,59 @@ struct vsp1_dl_header {
>  	u32 flags;
>  };
> 
> +/**
> + * struct vsp1_dl_ext_header - Extended display list header
> + * @padding: padding zero bytes for alignment
> + * @pre_ext_dl_num_cmd: number of pre-extended command bodies to parse
> + * @flags: enables or disables execution of the pre and post command
> + * @pre_ext_dl_plist: start address of pre-extended display list bodies
> + * @post_ext_dl_num_cmd: number of post-extended command bodies to parse
> + * @post_ext_dl_plist: start address of post-extended display list bodies
> + */
> +struct vsp1_dl_ext_header {
> +	u32 padding;
> +
> +	/*
> +	 * The datasheet represents flags as stored before pre_ext_dl_num_cmd,
> +	 * expecting 32-bit accesses. The flags are appropriate to the whole
> +	 * header, not just the pre_ext command, and thus warrant being
> +	 * separated out. Due to byte ordering, and representing as 16 bit
> +	 * values here, the flags must be positioned after the
> +	 * pre_ext_dl_num_cmd.
> +	 */
> +	u16 pre_ext_dl_num_cmd;
> +	u16 flags;
> +	u32 pre_ext_dl_plist;
> +
> +	u32 post_ext_dl_num_cmd;
> +	u32 post_ext_dl_plist;
> +};
> +
> +struct vsp1_dl_header_extended {
> +	struct vsp1_dl_header header;
> +	struct vsp1_dl_ext_header ext;
> +};
> +
>  struct vsp1_dl_entry {
>  	u32 addr;
>  	u32 data;
>  };
> 
>  /**
> + * struct vsp1_pre_ext_dl_body - Pre Extended Display List Body
> + * @opcode: Extended display list command operation code
> + * @flags: Pre-extended command flags. These are specific to each command
> + * @address_set: Source address set pointer. Must have 16 byte alignment

s/byte/bytes/

> + * @reserved: Zero bits for alignment.
> + */
> +struct vsp1_pre_ext_dl_body {
> +	u32 opcode;
> +	u32 flags;
> +	u32 address_set;
> +	u32 reserved;
> +};
> +
> +/**
>   * struct vsp1_dl_body - Display list body
>   * @list: entry in the display list list of bodies
>   * @free: entry in the pool free body list
> @@ -95,9 +145,12 @@ struct vsp1_dl_body_pool {
>   * @list: entry in the display list manager lists
>   * @dlm: the display list manager
>   * @header: display list header
> + * @extension: extended display list header. NULL for normal lists
>   * @dma: DMA address for the header
>   * @body0: first display list body
>   * @bodies: list of extra display list bodies
> + * @pre_cmd: pre command to be issued through extended dl header
> + * @post_cmd: post command to be issued through extended dl header
>   * @has_chain: if true, indicates that there's a partition chain
>   * @chain: entry in the display list partition chain
>   * @internal: whether the display list is used for internal purpose
> @@ -107,11 +160,15 @@ struct vsp1_dl_list {
>  	struct vsp1_dl_manager *dlm;
> 
>  	struct vsp1_dl_header *header;
> +	struct vsp1_dl_ext_header *extension;
>  	dma_addr_t dma;
> 
>  	struct vsp1_dl_body *body0;
>  	struct list_head bodies;
> 
> +	struct vsp1_dl_ext_cmd *pre_cmd;
> +	struct vsp1_dl_ext_cmd *post_cmd;
> +
>  	bool has_chain;
>  	struct list_head chain;
> 
> @@ -495,6 +552,14 @@ int vsp1_dl_list_add_chain(struct vsp1_dl_list *head,
>  	return 0;
>  }
> 
> +static void vsp1_dl_ext_cmd_fill_header(struct vsp1_dl_ext_cmd *cmd)
> +{
> +	cmd->cmds[0].opcode = cmd->opcode;
> +	cmd->cmds[0].flags = cmd->flags;
> +	cmd->cmds[0].address_set = cmd->data_dma;
> +	cmd->cmds[0].reserved = 0;
> +}
> +
>  static void vsp1_dl_list_fill_header(struct vsp1_dl_list *dl, bool is_last)
> {
>  	struct vsp1_dl_manager *dlm = dl->dlm;
> @@ -547,6 +612,27 @@ static void vsp1_dl_list_fill_header(struct
> vsp1_dl_list *dl, bool is_last) */
>  		dl->header->flags = VSP1_DLH_INT_ENABLE;
>  	}
> +
> +	if (!dl->extension)
> +		return;
> +
> +	dl->extension->flags = 0;
> +
> +	if (dl->pre_cmd) {
> +		dl->extension->pre_ext_dl_plist = dl->pre_cmd->cmd_dma;
> +		dl->extension->pre_ext_dl_num_cmd = dl->pre_cmd->num_cmds;
> +		dl->extension->flags |= VSP1_DLH_EXT_PRE_CMD_EXEC;
> +
> +		vsp1_dl_ext_cmd_fill_header(dl->pre_cmd);
> +	}
> +
> +	if (dl->post_cmd) {
> +		dl->extension->post_ext_dl_plist = dl->post_cmd->cmd_dma;
> +		dl->extension->post_ext_dl_num_cmd = dl->post_cmd->num_cmds;
> +		dl->extension->flags |= VSP1_DLH_EXT_POST_CMD_EXEC;
> +
> +		vsp1_dl_ext_cmd_fill_header(dl->post_cmd);
> +	}
>  }
> 
>  static bool vsp1_dl_list_hw_update_pending(struct vsp1_dl_manager *dlm)
> @@ -736,9 +822,16 @@ unsigned int vsp1_dlm_irq_frame_end(struct
> vsp1_dl_manager *dlm) /* Hardware Setup */
>  void vsp1_dlm_setup(struct vsp1_device *vsp1)
>  {
> +	unsigned int i;
>  	u32 ctrl = (256 << VI6_DL_CTRL_AR_WAIT_SHIFT)
>  		 | VI6_DL_CTRL_DC2 | VI6_DL_CTRL_DC1 | VI6_DL_CTRL_DC0
>  		 | VI6_DL_CTRL_DLE;
> +	u32 ext_dl = (0x02 << VI6_DL_EXT_CTRL_POLINT_SHIFT) |
> +		      VI6_DL_EXT_CTRL_DLPRI | VI6_DL_EXT_CTRL_EXT;

To match the style of the file, can you move the trailing | under the = ?

> +
> +	if (vsp1_feature(vsp1, VSP1_HAS_EXT_DL))
> +		for (i = 0; i < vsp1->info->wpf_count; ++i)
> +			vsp1_write(vsp1, VI6_DL_EXT_CTRL(i), ext_dl);

And add {} to the if ?

With these small updates,

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

If there's no other issue with v5 that would require a v6, I'll fix this when 
applying.

>  	vsp1_write(vsp1, VI6_DL_CTRL, ctrl);
>  	vsp1_write(vsp1, VI6_DL_SWAP, VI6_DL_SWAP_LWS);
> @@ -792,7 +885,11 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct
> vsp1_device *vsp1, * memory. An extra body is allocated on top of the
> prealloc to account * for the cached body used by the vsp1_pipeline object.
>  	 */
> -	header_size = ALIGN(sizeof(struct vsp1_dl_header), 8);
> +	header_size = vsp1_feature(vsp1, VSP1_HAS_EXT_DL) ?
> +			sizeof(struct vsp1_dl_header_extended) :
> +			sizeof(struct vsp1_dl_header);
> +
> +	header_size = ALIGN(header_size, 8);
> 
>  	dlm->pool = vsp1_dl_body_pool_create(vsp1, prealloc + 1,
>  					     VSP1_DL_NUM_ENTRIES, header_size);
> @@ -808,6 +905,11 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct
> vsp1_device *vsp1, return NULL;
>  		}
> 
> +		/* The extended header immediately follows the header. */
> +		if (vsp1_feature(vsp1, VSP1_HAS_EXT_DL))
> +			dl->extension = (void *)dl->header
> +				      + sizeof(*dl->header);
> +
>  		list_add_tail(&dl->list, &dlm->free);
>  	}
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.h
> b/drivers/media/platform/vsp1/vsp1_dl.h index 7dba0469c92e..afefd5bfa136
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.h
> +++ b/drivers/media/platform/vsp1/vsp1_dl.h
> @@ -20,6 +20,31 @@ struct vsp1_dl_manager;
>  #define VSP1_DL_FRAME_END_COMPLETED		BIT(0)
>  #define VSP1_DL_FRAME_END_INTERNAL		BIT(1)
> 
> +/**
> + * struct vsp1_dl_ext_cmd - Extended Display command
> + * @free: entry in the pool of free commands list
> + * @opcode: command type opcode
> + * @flags: flags used by the command
> + * @cmds: array of command bodies for this extended cmd
> + * @num_cmds: quantity of commands in @cmds array
> + * @cmd_dma: DMA address of the command body
> + * @data: memory allocation for command-specific data
> + * @data_dma: DMA address for command-specific data
> + */
> +struct vsp1_dl_ext_cmd {
> +	struct list_head free;
> +
> +	u8 opcode;
> +	u32 flags;
> +
> +	struct vsp1_pre_ext_dl_body *cmds;
> +	unsigned int num_cmds;
> +	dma_addr_t cmd_dma;
> +
> +	void *data;
> +	dma_addr_t data_dma;
> +};
> +
>  void vsp1_dlm_setup(struct vsp1_device *vsp1);
> 
>  struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
> b/drivers/media/platform/vsp1/vsp1_drv.c index 3367c2ba990d..b6619c9c18bb
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -754,7 +754,7 @@ static const struct vsp1_device_info vsp1_device_infos[]
> = { .version = VI6_IP_VERSION_MODEL_VSPD_GEN3,
>  		.model = "VSP2-D",
>  		.gen = 3,
> -		.features = VSP1_HAS_BRU | VSP1_HAS_WPF_VFLIP,
> +		.features = VSP1_HAS_BRU | VSP1_HAS_WPF_VFLIP | VSP1_HAS_EXT_DL,
>  		.lif_count = 1,
>  		.rpf_count = 5,
>  		.uif_count = 1,
> @@ -774,7 +774,7 @@ static const struct vsp1_device_info vsp1_device_infos[]
> = { .version = VI6_IP_VERSION_MODEL_VSPDL_GEN3,
>  		.model = "VSP2-DL",
>  		.gen = 3,
> -		.features = VSP1_HAS_BRS | VSP1_HAS_BRU,
> +		.features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_EXT_DL,
>  		.lif_count = 2,
>  		.rpf_count = 5,
>  		.uif_count = 2,
> diff --git a/drivers/media/platform/vsp1/vsp1_regs.h
> b/drivers/media/platform/vsp1/vsp1_regs.h index 0d249ff9f564..5ea9f9070cf3
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_regs.h
> +++ b/drivers/media/platform/vsp1/vsp1_regs.h
> @@ -72,7 +72,7 @@
>  #define VI6_DL_SWAP_WDS			(1 << 1)
>  #define VI6_DL_SWAP_BTS			(1 << 0)
> 
> -#define VI6_DL_EXT_CTRL			0x011c
> +#define VI6_DL_EXT_CTRL(n)		(0x011c + (n) * 36)
>  #define VI6_DL_EXT_CTRL_NWE		(1 << 16)
>  #define VI6_DL_EXT_CTRL_POLINT_MASK	(0x3f << 8)
>  #define VI6_DL_EXT_CTRL_POLINT_SHIFT	8


-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v5 09/11] media: vsp1: Provide support for extended command pools
  2018-07-17 20:35 ` [PATCH v5 09/11] media: vsp1: Provide support for extended command pools Kieran Bingham
@ 2018-08-02 14:58   ` Laurent Pinchart
  0 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2018-08-02 14:58 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: Kieran Bingham, linux-renesas-soc, linux-media, dri-devel,
	Kieran Bingham

Hi Kieran,

Thank you for the patch.

On Tuesday, 17 July 2018 23:35:51 EEST Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> VSPD and VSP-DL devices can provide extended display lists supporting
> extended command display list objects.
> 
> These extended commands require their own dma memory areas for a header
> and body specific to the command type.
> 
> Implement a command pool to allocate all necessary memory in a single
> DMA allocation to reduce pressure on the TLB, and provide convenient
> re-usable command objects for the entities to utilise.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> ---
> v2:
>  - Fix spelling typo in commit message
>  - constify, and staticify the instantiation of vsp1_extended_commands
>  - s/autfld_cmds/autofld_cmds/
>  - staticify cmd pool functions (Thanks kbuild-bot)
> 
> v5:
>  - Rename vsp1_dl_ext_cmd_header -> vsp1_pre_ext_dl_body
>  - fixup vsp1_cmd_pool structure documentation
>  - Rename dlm->autofld_cmds dlm->cmdpool
>  - Separate out the instatiation of vsp1_extended_commands
>  - Move initialisation of lock, and lists in vsp1_dl_cmd_pool_create to
>    immediately after allocation
>  - simplify vsp1_dlm_get_autofld_cmd
>  - Rename vsp1_dl_get_autofld_cmd() to vsp1_dl_get_pre_cmd() and moved
>    to "Display List Extended Command Management" section of vsp1_dl
> 
>  drivers/media/platform/vsp1/vsp1_dl.c | 194 +++++++++++++++++++++++++++-
>  drivers/media/platform/vsp1/vsp1_dl.h |   3 +-
>  2 files changed, 197 insertions(+)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c index 2fffe977aa35..d5b3c24d160c
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -141,6 +141,30 @@ struct vsp1_dl_body_pool {
>  };
> 
>  /**
> + * struct vsp1_cmd_pool - Display List commands pool
> + * @dma: DMA address of the entries
> + * @size: size of the full DMA memory pool in bytes
> + * @mem: CPU memory pointer for the pool
> + * @cmds: Array of command structures for the pool
> + * @free: Free pool entries
> + * @lock: Protects the free list
> + * @vsp1: the VSP1 device
> + */
> +struct vsp1_dl_cmd_pool {
> +	/* DMA allocation */
> +	dma_addr_t dma;
> +	size_t size;
> +	void *mem;
> +
> +	struct vsp1_dl_ext_cmd *cmds;
> +	struct list_head free;
> +
> +	spinlock_t lock;
> +
> +	struct vsp1_device *vsp1;
> +};
> +
> +/**
>   * struct vsp1_dl_list - Display list
>   * @list: entry in the display list manager lists
>   * @dlm: the display list manager
> @@ -186,6 +210,7 @@ struct vsp1_dl_list {
>   * @queued: list queued to the hardware (written to the DL registers)
>   * @pending: list waiting to be queued to the hardware
>   * @pool: body pool for the display list bodies
> + * @autofld_cmds: command pool to support auto-fld interlaced mode
>   */
>  struct vsp1_dl_manager {
>  	unsigned int index;
> @@ -199,6 +224,7 @@ struct vsp1_dl_manager {
>  	struct vsp1_dl_list *pending;
> 
>  	struct vsp1_dl_body_pool *pool;
> +	struct vsp1_dl_cmd_pool *cmdpool;
>  };
> 
>  /*
> ---------------------------------------------------------------------------
> -- @@ -362,6 +388,157 @@ void vsp1_dl_body_write(struct vsp1_dl_body *dlb,
> u32 reg, u32 data) }
> 
>  /*
> ---------------------------------------------------------------------------
> -- + * Display List Extended Command Management
> + */
> +
> +enum vsp1_extcmd_type {
> +	VSP1_EXTCMD_AUTODISP,
> +	VSP1_EXTCMD_AUTOFLD,
> +};
> +
> +struct vsp1_extended_command_info {
> +	u16 opcode;
> +	size_t body_size;
> +};
> +
> +static const struct vsp1_extended_command_info vsp1_extended_commands[] = {
> +	[VSP1_EXTCMD_AUTODISP] = { 0x02, 96 },
> +	[VSP1_EXTCMD_AUTOFLD]  = { 0x03, 160 },
> +};
> +
> +/**
> + * vsp1_dl_cmd_pool_create - Create a pool of commands from a single
> allocation + * @vsp1: The VSP1 device
> + * @type: The command pool type
> + * @num_cmds: The number of commands to allocate
> + *
> + * Allocate a pool of commands each with enough memory to contain the
> private + * data of each command. The allocation sizes are dependent upon
> the command + * type.
> + *
> + * Return a pointer to the pool on success or NULL if memory can't be
> allocated. + */
> +static struct vsp1_dl_cmd_pool *
> +vsp1_dl_cmd_pool_create(struct vsp1_device *vsp1, enum vsp1_extcmd_type
> type, +			unsigned int num_cmds)
> +{
> +	struct vsp1_dl_cmd_pool *pool;
> +	unsigned int i;
> +	size_t cmd_size;
> +
> +	pool = kzalloc(sizeof(*pool), GFP_KERNEL);
> +	if (!pool)
> +		return NULL;
> +
> +	spin_lock_init(&pool->lock);
> +	INIT_LIST_HEAD(&pool->free);
> +
> +	pool->cmds = kcalloc(num_cmds, sizeof(*pool->cmds), GFP_KERNEL);
> +	if (!pool->cmds) {
> +		kfree(pool);
> +		return NULL;
> +	}
> +
> +	cmd_size = sizeof(struct vsp1_pre_ext_dl_body) +
> +		   vsp1_extended_commands[type].body_size;
> +	cmd_size = ALIGN(cmd_size, 16);
> +
> +	pool->size = cmd_size * num_cmds;
> +	pool->mem = dma_alloc_wc(vsp1->bus_master, pool->size, &pool->dma,
> +				 GFP_KERNEL);
> +	if (!pool->mem) {
> +		kfree(pool->cmds);
> +		kfree(pool);
> +		return NULL;
> +	}
> +
> +	for (i = 0; i < num_cmds; ++i) {
> +		struct vsp1_dl_ext_cmd *cmd = &pool->cmds[i];
> +		size_t cmd_offset = i * cmd_size;
> +		/* data_offset must be 16 byte aligned for DMA. */
> +		size_t data_offset = sizeof(struct vsp1_pre_ext_dl_body) +
> +				     cmd_offset;
> +
> +		cmd->pool = pool;
> +		cmd->opcode = vsp1_extended_commands[type].opcode;
> +
> +		/*
> +		 * TODO: Auto-disp can utilise more than one extended body
> +		 * command per cmd.
> +		 */
> +		cmd->num_cmds = 1;
> +		cmd->cmds = pool->mem + cmd_offset;
> +		cmd->cmd_dma = pool->dma + cmd_offset;
> +
> +		cmd->data = pool->mem + data_offset;
> +		cmd->data_dma = pool->dma + data_offset;
> +
> +		list_add_tail(&cmd->free, &pool->free);
> +	}
> +
> +	return pool;
> +}
> +
> +static
> +struct vsp1_dl_ext_cmd *vsp1_dl_ext_cmd_get(struct vsp1_dl_cmd_pool *pool)
> +{
> +	struct vsp1_dl_ext_cmd *cmd = NULL;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&pool->lock, flags);
> +
> +	if (!list_empty(&pool->free)) {
> +		cmd = list_first_entry(&pool->free, struct vsp1_dl_ext_cmd,
> +				       free);
> +		list_del(&cmd->free);
> +	}
> +
> +	spin_unlock_irqrestore(&pool->lock, flags);
> +
> +	return cmd;
> +}
> +
> +static void vsp1_dl_ext_cmd_put(struct vsp1_dl_ext_cmd *cmd)
> +{
> +	unsigned long flags;
> +
> +	if (!cmd)
> +		return;
> +
> +	/* Reset flags, these mark data usage. */
> +	cmd->flags = 0;
> +
> +	spin_lock_irqsave(&cmd->pool->lock, flags);
> +	list_add_tail(&cmd->free, &cmd->pool->free);
> +	spin_unlock_irqrestore(&cmd->pool->lock, flags);
> +}
> +
> +static void vsp1_dl_ext_cmd_pool_destroy(struct vsp1_dl_cmd_pool *pool)
> +{
> +	if (!pool)
> +		return;
> +
> +	if (pool->mem)
> +		dma_free_wc(pool->vsp1->bus_master, pool->size, pool->mem,
> +			    pool->dma);
> +
> +	kfree(pool->cmds);
> +	kfree(pool);
> +}
> +
> +struct vsp1_dl_ext_cmd *vsp1_dl_get_pre_cmd(struct vsp1_dl_list *dl)
> +{
> +	struct vsp1_dl_manager *dlm = dl->dlm;
> +
> +	if (dl->pre_cmd)
> +		return dl->pre_cmd;
> +
> +	dl->pre_cmd = vsp1_dl_ext_cmd_get(dlm->cmdpool);
> +
> +	return dl->pre_cmd;
> +}
> +
> +/*
> ---------------------------------------------------------------------------
> - * Display List Transaction Management
>   */
> 
> @@ -463,6 +640,12 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl)
> 
>  	vsp1_dl_list_bodies_put(dl);
> 
> +	vsp1_dl_ext_cmd_put(dl->pre_cmd);
> +	vsp1_dl_ext_cmd_put(dl->post_cmd);
> +
> +	dl->pre_cmd = NULL;
> +	dl->post_cmd = NULL;
> +
>  	/*
>  	 * body0 is reused as as an optimisation as presently every display list
>  	 * has at least one body, thus we reinitialise the entries list.
> @@ -913,6 +1096,15 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct
> vsp1_device *vsp1, list_add_tail(&dl->list, &dlm->free);
>  	}
> 
> +	if (vsp1_feature(vsp1, VSP1_HAS_EXT_DL)) {
> +		dlm->cmdpool = vsp1_dl_cmd_pool_create(vsp1,
> +					VSP1_EXTCMD_AUTOFLD, prealloc);
> +		if (!dlm->cmdpool) {
> +			vsp1_dlm_destroy(dlm);
> +			return NULL;
> +		}
> +	}
> +
>  	return dlm;
>  }
> 
> @@ -929,4 +1121,6 @@ void vsp1_dlm_destroy(struct vsp1_dl_manager *dlm)
>  	}
> 
>  	vsp1_dl_body_pool_destroy(dlm->pool);
> +	vsp1_dl_ext_cmd_pool_destroy(dlm->cmdpool);
>  }
> +

Extra blank line. Apart from that,

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

> diff --git a/drivers/media/platform/vsp1/vsp1_dl.h
> b/drivers/media/platform/vsp1/vsp1_dl.h index afefd5bfa136..125750dc8b5c
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.h
> +++ b/drivers/media/platform/vsp1/vsp1_dl.h
> @@ -22,6 +22,7 @@ struct vsp1_dl_manager;
> 
>  /**
>   * struct vsp1_dl_ext_cmd - Extended Display command
> + * @pool: pool to which this command belongs
>   * @free: entry in the pool of free commands list
>   * @opcode: command type opcode
>   * @flags: flags used by the command
> @@ -32,6 +33,7 @@ struct vsp1_dl_manager;
>   * @data_dma: DMA address for command-specific data
>   */
>  struct vsp1_dl_ext_cmd {
> +	struct vsp1_dl_cmd_pool *pool;
>  	struct list_head free;
> 
>  	u8 opcode;
> @@ -58,6 +60,7 @@ struct vsp1_dl_body *vsp1_dlm_dl_body_get(struct
> vsp1_dl_manager *dlm); struct vsp1_dl_list *vsp1_dl_list_get(struct
> vsp1_dl_manager *dlm); void vsp1_dl_list_put(struct vsp1_dl_list *dl);
>  struct vsp1_dl_body *vsp1_dl_list_get_body0(struct vsp1_dl_list *dl);
> +struct vsp1_dl_ext_cmd *vsp1_dl_get_pre_cmd(struct vsp1_dl_list *dl);
>  void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool internal);
> 
>  struct vsp1_dl_body_pool *

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v5 10/11] media: vsp1: Support Interlaced display pipelines
  2018-07-17 20:35 ` [PATCH v5 10/11] media: vsp1: Support Interlaced display pipelines Kieran Bingham
@ 2018-08-02 22:44   ` Laurent Pinchart
  2018-08-03 10:29     ` Kieran Bingham
  0 siblings, 1 reply; 18+ messages in thread
From: Laurent Pinchart @ 2018-08-02 22:44 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: Kieran Bingham, linux-renesas-soc, linux-media, dri-devel,
	Kieran Bingham

Hi Kieran,

Thank you for the patch.

On Tuesday, 17 July 2018 23:35:52 EEST Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> Calculate the top and bottom fields for the interlaced frames and
> utilise the extended display list command feature to implement the
> auto-field operations. This allows the DU to update the VSP2 registers
> dynamically based upon the currently processing field.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> ---
> 
> v2:
>  - fix erroneous BIT value which enabled interlaced
>  - fix field handling at frame_end interrupt
> 
> v3:
>  - Pass DL through partition calls to allow autocmd's to be retrieved
>  - Document interlaced field in struct vsp1_du_atomic_config
> 
> v5:
>  - Obtain autofld cmd in vsp1_rpf_configure_autofld()
>  - Move VSP1_DL_EXT_AUTOFLD_INT to vsp1_regs.h
>  - Rename VSP1_DL_EXT_AUTOFLD_INT -> VI6_DL_EXT_AUTOFLD_INT
>  - move interlaced configuration parameter to pipe object
>  - autofld: Set cmd->flags in one single expression.
> 
>  drivers/media/platform/vsp1/vsp1_dl.c   | 10 ++++-
>  drivers/media/platform/vsp1/vsp1_drm.c  | 16 +++++-
>  drivers/media/platform/vsp1/vsp1_pipe.h |  2 +-
>  drivers/media/platform/vsp1/vsp1_regs.h |  3 +-
>  drivers/media/platform/vsp1/vsp1_rpf.c  | 72 ++++++++++++++++++++++++--
>  include/media/vsp1.h                    |  2 +-
>  6 files changed, 100 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c index d5b3c24d160c..4963acac73f8
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -946,6 +946,8 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool
> internal) */
>  unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
>  {
> +	struct vsp1_device *vsp1 = dlm->vsp1;
> +	u32 status = vsp1_read(vsp1, VI6_STATUS);
>  	unsigned int flags = 0;
> 
>  	spin_lock(&dlm->lock);
> @@ -971,6 +973,14 @@ unsigned int vsp1_dlm_irq_frame_end(struct
> vsp1_dl_manager *dlm) goto done;
> 
>  	/*
> +	 * Progressive streams report only TOP fields. If we have a BOTTOM
> +	 * field, we are interlaced, and expect the frame to complete on the
> +	 * next frame end interrupt.
> +	 */
> +	if (status & VI6_STATUS_FLD_STD(dlm->index))
> +		goto done;
> +
> +	/*
>  	 * The device starts processing the queued display list right after the
>  	 * frame end interrupt. The display list thus becomes active.
>  	 */
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> b/drivers/media/platform/vsp1/vsp1_drm.c index a16856856789..efdc9a676728
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -671,8 +671,20 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int
> pipe_index, drm_pipe->width = cfg->width;
>  	drm_pipe->height = cfg->height;
> 
> -	dev_dbg(vsp1->dev, "%s: configuring LIF%u with format %ux%u\n",
> -		__func__, pipe_index, cfg->width, cfg->height);
> +	if (cfg->interlaced && !vsp1_feature(vsp1, VSP1_HAS_EXT_DL)) {
> +		/*
> +		 * Interlaced support requires extended display lists to
> +		 * provide the auto-fld feature with the DU.
> +		 */
> +		dev_dbg(vsp1->dev, "Interlaced unsupported on this pipeline\n");
> +		return -EINVAL;
> +	}

As mentioned in the v4 review, this check should be moved to the DRM driver, 
in the atomic check handler.

> +
> +	pipe->interlaced = cfg->interlaced;
> +
> +	dev_dbg(vsp1->dev, "%s: configuring LIF%u with format %ux%u%s\n",
> +		__func__, pipe_index, cfg->width, cfg->height,
> +		pipe->interlaced ? "i" : "");
> 
>  	mutex_lock(&vsp1->drm->lock);
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h
> b/drivers/media/platform/vsp1/vsp1_pipe.h index 743d8f0db45c..ae646c9ef337
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_pipe.h
> +++ b/drivers/media/platform/vsp1/vsp1_pipe.h
> @@ -104,6 +104,7 @@ struct vsp1_partition {
>   * @entities: list of entities in the pipeline
>   * @stream_config: cached stream configuration for video pipelines
>   * @configured: when false the @stream_config shall be written to the
> hardware + * @interlaced: True when the pipeline is configured in
> interlaced mode * @partitions: The number of partitions used to process one
> frame * @partition: The current partition for configuration to process *
> @part_table: The pre-calculated partitions used by the pipeline @@ -142,6
> +143,7 @@ struct vsp1_pipeline {
> 
>  	struct vsp1_dl_body *stream_config;
>  	bool configured;
> +	bool interlaced;
> 
>  	unsigned int partitions;
>  	struct vsp1_partition *partition;
> diff --git a/drivers/media/platform/vsp1/vsp1_regs.h
> b/drivers/media/platform/vsp1/vsp1_regs.h index 5ea9f9070cf3..3738ff2f7b85
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_regs.h
> +++ b/drivers/media/platform/vsp1/vsp1_regs.h
> @@ -28,6 +28,7 @@
>  #define VI6_SRESET_SRTS(n)		(1 << (n))
> 
>  #define VI6_STATUS			0x0038
> +#define VI6_STATUS_FLD_STD(n)		(1 << ((n) + 28))
>  #define VI6_STATUS_SYS_ACT(n)		(1 << ((n) + 8))
> 
>  #define VI6_WPF_IRQ_ENB(n)		(0x0048 + (n) * 12)
> @@ -80,6 +81,8 @@
>  #define VI6_DL_EXT_CTRL_EXPRI		(1 << 4)
>  #define VI6_DL_EXT_CTRL_EXT		(1 << 0)
> 
> +#define VI6_DL_EXT_AUTOFLD_INT		BIT(0)
> +
>  #define VI6_DL_BODY_SIZE		0x0120
>  #define VI6_DL_BODY_SIZE_UPD		(1 << 24)
>  #define VI6_DL_BODY_SIZE_BS_MASK	(0x1ffff << 0)
> diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c
> b/drivers/media/platform/vsp1/vsp1_rpf.c index 69e5fe6e6b50..c728c193e09c
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rpf.c
> @@ -20,6 +20,18 @@
>  #define RPF_MAX_WIDTH				8190
>  #define RPF_MAX_HEIGHT				8190
> 
> +/* Pre extended display list command data structure. */
> +struct vsp1_extcmd_auto_fld_body {
> +	u32 top_y0;
> +	u32 bottom_y0;
> +	u32 top_c0;
> +	u32 bottom_c0;
> +	u32 top_c1;
> +	u32 bottom_c1;
> +	u32 reserved0;
> +	u32 reserved1;
> +};
> +
>  /*
> ---------------------------------------------------------------------------
> -- * Device Access
>   */
> @@ -64,6 +76,14 @@ static void rpf_configure_stream(struct vsp1_entity
> *entity, pstride |= format->plane_fmt[1].bytesperline
>  			<< VI6_RPF_SRCM_PSTRIDE_C_SHIFT;
> 
> +	/*
> +	 * pstride has both STRIDE_Y and STRIDE_C, but multiplying the whole
> +	 * of pstride by 2 is conveniently OK here as we are multiplying both
> +	 * values.
> +	 */
> +	if (pipe->interlaced)
> +		pstride *= 2;
> +
>  	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_PSTRIDE, pstride);
> 
>  	/* Format */
> @@ -100,6 +120,9 @@ static void rpf_configure_stream(struct vsp1_entity
> *entity, top = compose->top;
>  	}
> 
> +	if (pipe->interlaced)
> +		top /= 2;
> +
>  	vsp1_rpf_write(rpf, dlb, VI6_RPF_LOC,
>  		       (left << VI6_RPF_LOC_HCOORD_SHIFT) |
>  		       (top << VI6_RPF_LOC_VCOORD_SHIFT));
> @@ -169,6 +192,36 @@ static void rpf_configure_stream(struct vsp1_entity
> *entity,
> 
>  }
> 
> +static void vsp1_rpf_configure_autofld(struct vsp1_rwpf *rpf,
> +				       struct vsp1_dl_list *dl)
> +{
> +	const struct v4l2_pix_format_mplane *format = &rpf->format;
> +	struct vsp1_dl_ext_cmd *cmd;
> +	struct vsp1_extcmd_auto_fld_body *auto_fld;
> +	u32 offset_y, offset_c;
> +
> +	cmd = vsp1_dl_get_pre_cmd(dl);
> +	if (WARN_ONCE(!cmd, "Failed to obtain an autofld cmd"))
> +		return;
> +
> +	/* Re-index our auto_fld to match the current RPF. */
> +	auto_fld = cmd->data;
> +	auto_fld = &auto_fld[rpf->entity.index];
> +
> +	auto_fld->top_y0 = rpf->mem.addr[0];
> +	auto_fld->top_c0 = rpf->mem.addr[1];
> +	auto_fld->top_c1 = rpf->mem.addr[2];
> +
> +	offset_y = format->plane_fmt[0].bytesperline;
> +	offset_c = format->plane_fmt[1].bytesperline;
> +
> +	auto_fld->bottom_y0 = rpf->mem.addr[0] + offset_y;
> +	auto_fld->bottom_c0 = rpf->mem.addr[1] + offset_c;
> +	auto_fld->bottom_c1 = rpf->mem.addr[2] + offset_c;
> +
> +	cmd->flags |= VI6_DL_EXT_AUTOFLD_INT | BIT(16 + rpf->entity.index);
> +}
> +
>  static void rpf_configure_frame(struct vsp1_entity *entity,
>  				struct vsp1_pipeline *pipe,
>  				struct vsp1_dl_list *dl,
> @@ -221,6 +274,11 @@ static void rpf_configure_partition(struct vsp1_entity
> *entity, crop.left += pipe->partition->rpf.left;
>  	}
> 
> +	if (pipe->interlaced) {
> +		crop.height = round_down(crop.height / 2, fmtinfo->vsub);
> +		crop.top = round_down(crop.top / 2, fmtinfo->vsub);
> +	}
> +
>  	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRC_BSIZE,
>  		       (crop.width << VI6_RPF_SRC_BSIZE_BHSIZE_SHIFT) |
>  		       (crop.height << VI6_RPF_SRC_BSIZE_BVSIZE_SHIFT));
> @@ -249,9 +307,17 @@ static void rpf_configure_partition(struct vsp1_entity
> *entity, fmtinfo->swap_uv)
>  		swap(mem.addr[1], mem.addr[2]);
> 
> -	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_Y, mem.addr[0]);
> -	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_C0, mem.addr[1]);
> -	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_C1, mem.addr[2]);
> +	/*
> +	 * Interlaced pipelines will use the extended pre-cmd to process
> +	 * SRCM_ADDR_{Y,C0,C1}
> +	 */
> +	if (pipe->interlaced) {
> +		vsp1_rpf_configure_autofld(rpf, dl);
> +	} else {
> +		vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_Y, mem.addr[0]);
> +		vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_C0, mem.addr[1]);
> +		vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_C1, mem.addr[2]);
> +	}
>  }
> 
>  static void rpf_partition(struct vsp1_entity *entity,
> diff --git a/include/media/vsp1.h b/include/media/vsp1.h
> index 678c24de1ac6..3093b9cb9067 100644
> --- a/include/media/vsp1.h
> +++ b/include/media/vsp1.h
> @@ -25,6 +25,7 @@ int vsp1_du_init(struct device *dev);
>   * struct vsp1_du_lif_config - VSP LIF configuration
>   * @width: output frame width
>   * @height: output frame height
> + * @interlaced: true for interlaced pipelines
>   * @callback: frame completion callback function (optional). When a
> callback *	      is provided, the VSP driver guarantees that it will be
> called once *	      and only once for each vsp1_du_atomic_flush() call.
> @@ -33,6 +34,7 @@ int vsp1_du_init(struct device *dev);
>  struct vsp1_du_lif_config {
>  	unsigned int width;
>  	unsigned int height;
> +	bool interlaced;
> 
>  	void (*callback)(void *data, bool completed, u32 crc);
>  	void *callback_data;

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v5 11/11] drm: rcar-du: Support interlaced video output through vsp1
  2018-07-17 20:35 ` [PATCH v5 11/11] drm: rcar-du: Support interlaced video output through vsp1 Kieran Bingham
@ 2018-08-02 22:46   ` Laurent Pinchart
  0 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2018-08-02 22:46 UTC (permalink / raw)
  To: Kieran Bingham
  Cc: Kieran Bingham, linux-renesas-soc, linux-media, dri-devel,
	Kieran Bingham

Hi Kieran,

Thank you for the patch.

On Tuesday, 17 July 2018 23:35:53 EEST Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> Use the newly exposed VSP1 interface to enable interlaced frame support
> through the VSP1 LIF pipelines.
> 
> The DSMR register is updated to set the ODEV flag on interlaced
> pipelines, thus defining an interlaced stream as having the ODD field
> located in the second half (BOTTOM) of the frame buffer.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

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

> ---
> v5
>  - Fix commit title
>  - Document change to DSMR
>  - Configure through vsp1_du_setup_lif(), rather than
>    vsp1_du_atomic_update()
> 
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 1 +
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 15dc9caa128b..b52b3e817b93
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -289,6 +289,7 @@ static void rcar_du_crtc_set_display_timing(struct
> rcar_du_crtc *rcrtc) /* Signal polarities */
>  	value = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
> 
>  	      | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? DSMR_HSL : 0)
> 
> +	      | ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? DSMR_ODEV : 0)
> 
>  	      | DSMR_DIPM_DISP | DSMR_CSPM;
> 
>  	rcar_du_crtc_write(rcrtc, DSMR, value);
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 72eebeda518e..a042f116731b
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -52,6 +52,7 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
>  	struct vsp1_du_lif_config cfg = {
>  		.width = mode->hdisplay,
>  		.height = mode->vdisplay,
> +		.interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE,
>  		.callback = rcar_du_vsp_complete,
>  		.callback_data = crtc,
>  	};

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v5 08/11] media: vsp1: Add support for extended display list headers
  2018-08-02 14:39   ` Laurent Pinchart
@ 2018-08-03  9:47     ` Kieran Bingham
  0 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2018-08-03  9:47 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Kieran Bingham, linux-renesas-soc, linux-media, dri-devel,
	Kieran Bingham

Hi Laurent,

On 02/08/18 15:39, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.

Thank you for (hopefully the last) review on this series :)



> On Tuesday, 17 July 2018 23:35:50 EEST Kieran Bingham wrote:
>> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>> Extended display list headers allow pre and post command lists to be
>> executed by the VSP pipeline. This provides the base support for
>> features such as AUTO_FLD (for interlaced support) and AUTO_DISP (for
>> supporting continuous camera preview pipelines.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>> ---
>>
>> v2:
>>  - remove __packed attributes
>>
>> v5:
>>  - Rename vsp1_dl_ext_header field names
>>  - Rename @extended -> @extension
>>  - Remove unnecessary VI6_DL_SWAP changes
>>  - Rename @cmd_opcode -> @opcode
>>  - Drop unused @data_size field
>>  - Move iteration of WPF's inside vsp1_dlm_setup
>>  - Rename vsp1_dl_ext_cmd_header -> vsp1_pre_ext_dl_body
>>  - Rename vsp1_pre_ext_dl_body->cmd to vsp1_pre_ext_dl_body->opcode
>>  - Rename vsp1_dl_ext_header->reserved0 to vsp1_dl_ext_header->padding
>>  - vsp1_pre_ext_dl_body: Rename 'data' to 'address_set'
>>  - vsp1_pre_ext_dl_body: Add struct documentation
>>  - Document ordering of 16bit accesses for flags in vsp1_dl_ext_header
>>
>>  drivers/media/platform/vsp1/vsp1.h      |   1 +-
>>  drivers/media/platform/vsp1/vsp1_dl.c   | 104 ++++++++++++++++++++++++-
>>  drivers/media/platform/vsp1/vsp1_dl.h   |  25 ++++++-
>>  drivers/media/platform/vsp1/vsp1_drv.c  |   4 +-
>>  drivers/media/platform/vsp1/vsp1_regs.h |   2 +-
>>  5 files changed, 132 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1.h
>> b/drivers/media/platform/vsp1/vsp1.h index f0d21cc8e9ab..56c62122a81a
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1.h
>> +++ b/drivers/media/platform/vsp1/vsp1.h
>> @@ -53,6 +53,7 @@ struct vsp1_uif;
>>  #define VSP1_HAS_HGO		(1 << 7)
>>  #define VSP1_HAS_HGT		(1 << 8)
>>  #define VSP1_HAS_BRS		(1 << 9)
>> +#define VSP1_HAS_EXT_DL		(1 << 10)
>>
>>  struct vsp1_device_info {
>>  	u32 version;
>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
>> b/drivers/media/platform/vsp1/vsp1_dl.c index 90e0a11c29b5..2fffe977aa35
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_dl.c
>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
>> @@ -22,6 +22,9 @@
>>  #define VSP1_DLH_INT_ENABLE		(1 << 1)
>>  #define VSP1_DLH_AUTO_START		(1 << 0)
>>
>> +#define VSP1_DLH_EXT_PRE_CMD_EXEC	(1 << 9)
>> +#define VSP1_DLH_EXT_POST_CMD_EXEC	(1 << 8)
>> +
>>  struct vsp1_dl_header_list {
>>  	u32 num_bytes;
>>  	u32 addr;
>> @@ -34,12 +37,59 @@ struct vsp1_dl_header {
>>  	u32 flags;
>>  };
>>
>> +/**
>> + * struct vsp1_dl_ext_header - Extended display list header
>> + * @padding: padding zero bytes for alignment
>> + * @pre_ext_dl_num_cmd: number of pre-extended command bodies to parse
>> + * @flags: enables or disables execution of the pre and post command
>> + * @pre_ext_dl_plist: start address of pre-extended display list bodies
>> + * @post_ext_dl_num_cmd: number of post-extended command bodies to parse
>> + * @post_ext_dl_plist: start address of post-extended display list bodies
>> + */
>> +struct vsp1_dl_ext_header {
>> +	u32 padding;
>> +
>> +	/*
>> +	 * The datasheet represents flags as stored before pre_ext_dl_num_cmd,
>> +	 * expecting 32-bit accesses. The flags are appropriate to the whole
>> +	 * header, not just the pre_ext command, and thus warrant being
>> +	 * separated out. Due to byte ordering, and representing as 16 bit
>> +	 * values here, the flags must be positioned after the
>> +	 * pre_ext_dl_num_cmd.
>> +	 */
>> +	u16 pre_ext_dl_num_cmd;
>> +	u16 flags;
>> +	u32 pre_ext_dl_plist;
>> +
>> +	u32 post_ext_dl_num_cmd;
>> +	u32 post_ext_dl_plist;
>> +};
>> +
>> +struct vsp1_dl_header_extended {
>> +	struct vsp1_dl_header header;
>> +	struct vsp1_dl_ext_header ext;
>> +};
>> +
>>  struct vsp1_dl_entry {
>>  	u32 addr;
>>  	u32 data;
>>  };
>>
>>  /**
>> + * struct vsp1_pre_ext_dl_body - Pre Extended Display List Body
>> + * @opcode: Extended display list command operation code
>> + * @flags: Pre-extended command flags. These are specific to each command
>> + * @address_set: Source address set pointer. Must have 16 byte alignment
> 
> s/byte/bytes/

I've made this "16-byte alignment" - I don't think you can align to a
plural, or at least it doesn't sound right in my head.

It's an interesting grammatical question that I couldn't find the answer
to, so I've asked online @ ell.stackexchange.com [0]


[0]
https://ell.stackexchange.com/questions/175063/what-is-the-correct-use-of-a-plural-term-and-alignment


> 
>> + * @reserved: Zero bits for alignment.
>> + */
>> +struct vsp1_pre_ext_dl_body {
>> +	u32 opcode;
>> +	u32 flags;
>> +	u32 address_set;
>> +	u32 reserved;
>> +};
>> +
>> +/**
>>   * struct vsp1_dl_body - Display list body
>>   * @list: entry in the display list list of bodies
>>   * @free: entry in the pool free body list
>> @@ -95,9 +145,12 @@ struct vsp1_dl_body_pool {
>>   * @list: entry in the display list manager lists
>>   * @dlm: the display list manager
>>   * @header: display list header
>> + * @extension: extended display list header. NULL for normal lists
>>   * @dma: DMA address for the header
>>   * @body0: first display list body
>>   * @bodies: list of extra display list bodies
>> + * @pre_cmd: pre command to be issued through extended dl header
>> + * @post_cmd: post command to be issued through extended dl header
>>   * @has_chain: if true, indicates that there's a partition chain
>>   * @chain: entry in the display list partition chain
>>   * @internal: whether the display list is used for internal purpose
>> @@ -107,11 +160,15 @@ struct vsp1_dl_list {
>>  	struct vsp1_dl_manager *dlm;
>>
>>  	struct vsp1_dl_header *header;
>> +	struct vsp1_dl_ext_header *extension;
>>  	dma_addr_t dma;
>>
>>  	struct vsp1_dl_body *body0;
>>  	struct list_head bodies;
>>
>> +	struct vsp1_dl_ext_cmd *pre_cmd;
>> +	struct vsp1_dl_ext_cmd *post_cmd;
>> +
>>  	bool has_chain;
>>  	struct list_head chain;
>>
>> @@ -495,6 +552,14 @@ int vsp1_dl_list_add_chain(struct vsp1_dl_list *head,
>>  	return 0;
>>  }
>>
>> +static void vsp1_dl_ext_cmd_fill_header(struct vsp1_dl_ext_cmd *cmd)
>> +{
>> +	cmd->cmds[0].opcode = cmd->opcode;
>> +	cmd->cmds[0].flags = cmd->flags;
>> +	cmd->cmds[0].address_set = cmd->data_dma;
>> +	cmd->cmds[0].reserved = 0;
>> +}
>> +
>>  static void vsp1_dl_list_fill_header(struct vsp1_dl_list *dl, bool is_last)
>> {
>>  	struct vsp1_dl_manager *dlm = dl->dlm;
>> @@ -547,6 +612,27 @@ static void vsp1_dl_list_fill_header(struct
>> vsp1_dl_list *dl, bool is_last) */
>>  		dl->header->flags = VSP1_DLH_INT_ENABLE;
>>  	}
>> +
>> +	if (!dl->extension)
>> +		return;
>> +
>> +	dl->extension->flags = 0;
>> +
>> +	if (dl->pre_cmd) {
>> +		dl->extension->pre_ext_dl_plist = dl->pre_cmd->cmd_dma;
>> +		dl->extension->pre_ext_dl_num_cmd = dl->pre_cmd->num_cmds;
>> +		dl->extension->flags |= VSP1_DLH_EXT_PRE_CMD_EXEC;
>> +
>> +		vsp1_dl_ext_cmd_fill_header(dl->pre_cmd);
>> +	}
>> +
>> +	if (dl->post_cmd) {
>> +		dl->extension->post_ext_dl_plist = dl->post_cmd->cmd_dma;
>> +		dl->extension->post_ext_dl_num_cmd = dl->post_cmd->num_cmds;
>> +		dl->extension->flags |= VSP1_DLH_EXT_POST_CMD_EXEC;
>> +
>> +		vsp1_dl_ext_cmd_fill_header(dl->post_cmd);
>> +	}
>>  }
>>
>>  static bool vsp1_dl_list_hw_update_pending(struct vsp1_dl_manager *dlm)
>> @@ -736,9 +822,16 @@ unsigned int vsp1_dlm_irq_frame_end(struct
>> vsp1_dl_manager *dlm) /* Hardware Setup */
>>  void vsp1_dlm_setup(struct vsp1_device *vsp1)
>>  {
>> +	unsigned int i;
>>  	u32 ctrl = (256 << VI6_DL_CTRL_AR_WAIT_SHIFT)
>>  		 | VI6_DL_CTRL_DC2 | VI6_DL_CTRL_DC1 | VI6_DL_CTRL_DC0
>>  		 | VI6_DL_CTRL_DLE;
>> +	u32 ext_dl = (0x02 << VI6_DL_EXT_CTRL_POLINT_SHIFT) |
>> +		      VI6_DL_EXT_CTRL_DLPRI | VI6_DL_EXT_CTRL_EXT;
> 
> To match the style of the file, can you move the trailing | under the = ?

Done.


> 
>> +
>> +	if (vsp1_feature(vsp1, VSP1_HAS_EXT_DL))
>> +		for (i = 0; i < vsp1->info->wpf_count; ++i)
>> +			vsp1_write(vsp1, VI6_DL_EXT_CTRL(i), ext_dl);
> 
> And add {} to the if ?

Added.


> With these small updates,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Collected.

> 
> If there's no other issue with v5 that would require a v6, I'll fix this when 
> applying.
> 
>>  	vsp1_write(vsp1, VI6_DL_CTRL, ctrl);
>>  	vsp1_write(vsp1, VI6_DL_SWAP, VI6_DL_SWAP_LWS);
>> @@ -792,7 +885,11 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct
>> vsp1_device *vsp1, * memory. An extra body is allocated on top of the
>> prealloc to account * for the cached body used by the vsp1_pipeline object.
>>  	 */
>> -	header_size = ALIGN(sizeof(struct vsp1_dl_header), 8);
>> +	header_size = vsp1_feature(vsp1, VSP1_HAS_EXT_DL) ?
>> +			sizeof(struct vsp1_dl_header_extended) :
>> +			sizeof(struct vsp1_dl_header);
>> +
>> +	header_size = ALIGN(header_size, 8);
>>
>>  	dlm->pool = vsp1_dl_body_pool_create(vsp1, prealloc + 1,
>>  					     VSP1_DL_NUM_ENTRIES, header_size);
>> @@ -808,6 +905,11 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct
>> vsp1_device *vsp1, return NULL;
>>  		}
>>
>> +		/* The extended header immediately follows the header. */
>> +		if (vsp1_feature(vsp1, VSP1_HAS_EXT_DL))
>> +			dl->extension = (void *)dl->header
>> +				      + sizeof(*dl->header);
>> +
>>  		list_add_tail(&dl->list, &dlm->free);
>>  	}
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.h
>> b/drivers/media/platform/vsp1/vsp1_dl.h index 7dba0469c92e..afefd5bfa136
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_dl.h
>> +++ b/drivers/media/platform/vsp1/vsp1_dl.h
>> @@ -20,6 +20,31 @@ struct vsp1_dl_manager;
>>  #define VSP1_DL_FRAME_END_COMPLETED		BIT(0)
>>  #define VSP1_DL_FRAME_END_INTERNAL		BIT(1)
>>
>> +/**
>> + * struct vsp1_dl_ext_cmd - Extended Display command
>> + * @free: entry in the pool of free commands list
>> + * @opcode: command type opcode
>> + * @flags: flags used by the command
>> + * @cmds: array of command bodies for this extended cmd
>> + * @num_cmds: quantity of commands in @cmds array
>> + * @cmd_dma: DMA address of the command body
>> + * @data: memory allocation for command-specific data
>> + * @data_dma: DMA address for command-specific data
>> + */
>> +struct vsp1_dl_ext_cmd {
>> +	struct list_head free;
>> +
>> +	u8 opcode;
>> +	u32 flags;
>> +
>> +	struct vsp1_pre_ext_dl_body *cmds;
>> +	unsigned int num_cmds;
>> +	dma_addr_t cmd_dma;
>> +
>> +	void *data;
>> +	dma_addr_t data_dma;
>> +};
>> +
>>  void vsp1_dlm_setup(struct vsp1_device *vsp1);
>>
>>  struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1,
>> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
>> b/drivers/media/platform/vsp1/vsp1_drv.c index 3367c2ba990d..b6619c9c18bb
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_drv.c
>> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
>> @@ -754,7 +754,7 @@ static const struct vsp1_device_info vsp1_device_infos[]
>> = { .version = VI6_IP_VERSION_MODEL_VSPD_GEN3,
>>  		.model = "VSP2-D",
>>  		.gen = 3,
>> -		.features = VSP1_HAS_BRU | VSP1_HAS_WPF_VFLIP,
>> +		.features = VSP1_HAS_BRU | VSP1_HAS_WPF_VFLIP | VSP1_HAS_EXT_DL,
>>  		.lif_count = 1,
>>  		.rpf_count = 5,
>>  		.uif_count = 1,
>> @@ -774,7 +774,7 @@ static const struct vsp1_device_info vsp1_device_infos[]
>> = { .version = VI6_IP_VERSION_MODEL_VSPDL_GEN3,
>>  		.model = "VSP2-DL",
>>  		.gen = 3,
>> -		.features = VSP1_HAS_BRS | VSP1_HAS_BRU,
>> +		.features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_EXT_DL,
>>  		.lif_count = 2,
>>  		.rpf_count = 5,
>>  		.uif_count = 2,
>> diff --git a/drivers/media/platform/vsp1/vsp1_regs.h
>> b/drivers/media/platform/vsp1/vsp1_regs.h index 0d249ff9f564..5ea9f9070cf3
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_regs.h
>> +++ b/drivers/media/platform/vsp1/vsp1_regs.h
>> @@ -72,7 +72,7 @@
>>  #define VI6_DL_SWAP_WDS			(1 << 1)
>>  #define VI6_DL_SWAP_BTS			(1 << 0)
>>
>> -#define VI6_DL_EXT_CTRL			0x011c
>> +#define VI6_DL_EXT_CTRL(n)		(0x011c + (n) * 36)
>>  #define VI6_DL_EXT_CTRL_NWE		(1 << 16)
>>  #define VI6_DL_EXT_CTRL_POLINT_MASK	(0x3f << 8)
>>  #define VI6_DL_EXT_CTRL_POLINT_SHIFT	8
> 
> 


-- 
Regards
--
Kieran

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

* Re: [PATCH v5 10/11] media: vsp1: Support Interlaced display pipelines
  2018-08-02 22:44   ` Laurent Pinchart
@ 2018-08-03 10:29     ` Kieran Bingham
  0 siblings, 0 replies; 18+ messages in thread
From: Kieran Bingham @ 2018-08-03 10:29 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Kieran Bingham, linux-renesas-soc, linux-media, dri-devel,
	Kieran Bingham

On 02/08/18 23:44, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Tuesday, 17 July 2018 23:35:52 EEST Kieran Bingham wrote:
>> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>> Calculate the top and bottom fields for the interlaced frames and
>> utilise the extended display list command feature to implement the
>> auto-field operations. This allows the DU to update the VSP2 registers
>> dynamically based upon the currently processing field.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>> ---
>>
>> v2:
>>  - fix erroneous BIT value which enabled interlaced
>>  - fix field handling at frame_end interrupt
>>
>> v3:
>>  - Pass DL through partition calls to allow autocmd's to be retrieved
>>  - Document interlaced field in struct vsp1_du_atomic_config
>>
>> v5:
>>  - Obtain autofld cmd in vsp1_rpf_configure_autofld()
>>  - Move VSP1_DL_EXT_AUTOFLD_INT to vsp1_regs.h
>>  - Rename VSP1_DL_EXT_AUTOFLD_INT -> VI6_DL_EXT_AUTOFLD_INT
>>  - move interlaced configuration parameter to pipe object
>>  - autofld: Set cmd->flags in one single expression.
>>
>>  drivers/media/platform/vsp1/vsp1_dl.c   | 10 ++++-
>>  drivers/media/platform/vsp1/vsp1_drm.c  | 16 +++++-
>>  drivers/media/platform/vsp1/vsp1_pipe.h |  2 +-
>>  drivers/media/platform/vsp1/vsp1_regs.h |  3 +-
>>  drivers/media/platform/vsp1/vsp1_rpf.c  | 72 ++++++++++++++++++++++++--
>>  include/media/vsp1.h                    |  2 +-
>>  6 files changed, 100 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
>> b/drivers/media/platform/vsp1/vsp1_dl.c index d5b3c24d160c..4963acac73f8
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_dl.c
>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
>> @@ -946,6 +946,8 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool
>> internal) */
>>  unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
>>  {
>> +	struct vsp1_device *vsp1 = dlm->vsp1;
>> +	u32 status = vsp1_read(vsp1, VI6_STATUS);
>>  	unsigned int flags = 0;
>>
>>  	spin_lock(&dlm->lock);
>> @@ -971,6 +973,14 @@ unsigned int vsp1_dlm_irq_frame_end(struct
>> vsp1_dl_manager *dlm) goto done;
>>
>>  	/*
>> +	 * Progressive streams report only TOP fields. If we have a BOTTOM
>> +	 * field, we are interlaced, and expect the frame to complete on the
>> +	 * next frame end interrupt.
>> +	 */
>> +	if (status & VI6_STATUS_FLD_STD(dlm->index))
>> +		goto done;
>> +
>> +	/*
>>  	 * The device starts processing the queued display list right after the
>>  	 * frame end interrupt. The display list thus becomes active.
>>  	 */
>> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
>> b/drivers/media/platform/vsp1/vsp1_drm.c index a16856856789..efdc9a676728
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_drm.c
>> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
>> @@ -671,8 +671,20 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int
>> pipe_index, drm_pipe->width = cfg->width;
>>  	drm_pipe->height = cfg->height;
>>
>> -	dev_dbg(vsp1->dev, "%s: configuring LIF%u with format %ux%u\n",
>> -		__func__, pipe_index, cfg->width, cfg->height);
>> +	if (cfg->interlaced && !vsp1_feature(vsp1, VSP1_HAS_EXT_DL)) {
>> +		/*
>> +		 * Interlaced support requires extended display lists to
>> +		 * provide the auto-fld feature with the DU.
>> +		 */
>> +		dev_dbg(vsp1->dev, "Interlaced unsupported on this pipeline\n");
>> +		return -EINVAL;
>> +	}
> 
> As mentioned in the v4 review, this check should be moved to the DRM driver, 
> in the atomic check handler.

Ahh yes, this hunk was simply meant to be dropped.

Only VSPD and VSPDL entities are linked with the DU, and they all
support VSP_HAS_EXT_DL.

Any limitations from the DU side can simply be handled in the DU.

>> +
>> +	pipe->interlaced = cfg->interlaced;
>> +
>> +	dev_dbg(vsp1->dev, "%s: configuring LIF%u with format %ux%u%s\n",
>> +		__func__, pipe_index, cfg->width, cfg->height,
>> +		pipe->interlaced ? "i" : "");
>>
>>  	mutex_lock(&vsp1->drm->lock);
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h
>> b/drivers/media/platform/vsp1/vsp1_pipe.h index 743d8f0db45c..ae646c9ef337
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_pipe.h
>> +++ b/drivers/media/platform/vsp1/vsp1_pipe.h
>> @@ -104,6 +104,7 @@ struct vsp1_partition {
>>   * @entities: list of entities in the pipeline
>>   * @stream_config: cached stream configuration for video pipelines
>>   * @configured: when false the @stream_config shall be written to the
>> hardware + * @interlaced: True when the pipeline is configured in
>> interlaced mode * @partitions: The number of partitions used to process one
>> frame * @partition: The current partition for configuration to process *
>> @part_table: The pre-calculated partitions used by the pipeline @@ -142,6
>> +143,7 @@ struct vsp1_pipeline {
>>
>>  	struct vsp1_dl_body *stream_config;
>>  	bool configured;
>> +	bool interlaced;
>>
>>  	unsigned int partitions;
>>  	struct vsp1_partition *partition;
>> diff --git a/drivers/media/platform/vsp1/vsp1_regs.h
>> b/drivers/media/platform/vsp1/vsp1_regs.h index 5ea9f9070cf3..3738ff2f7b85
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_regs.h
>> +++ b/drivers/media/platform/vsp1/vsp1_regs.h
>> @@ -28,6 +28,7 @@
>>  #define VI6_SRESET_SRTS(n)		(1 << (n))
>>
>>  #define VI6_STATUS			0x0038
>> +#define VI6_STATUS_FLD_STD(n)		(1 << ((n) + 28))
>>  #define VI6_STATUS_SYS_ACT(n)		(1 << ((n) + 8))
>>
>>  #define VI6_WPF_IRQ_ENB(n)		(0x0048 + (n) * 12)
>> @@ -80,6 +81,8 @@
>>  #define VI6_DL_EXT_CTRL_EXPRI		(1 << 4)
>>  #define VI6_DL_EXT_CTRL_EXT		(1 << 0)
>>
>> +#define VI6_DL_EXT_AUTOFLD_INT		BIT(0)
>> +
>>  #define VI6_DL_BODY_SIZE		0x0120
>>  #define VI6_DL_BODY_SIZE_UPD		(1 << 24)
>>  #define VI6_DL_BODY_SIZE_BS_MASK	(0x1ffff << 0)
>> diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c
>> b/drivers/media/platform/vsp1/vsp1_rpf.c index 69e5fe6e6b50..c728c193e09c
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_rpf.c
>> +++ b/drivers/media/platform/vsp1/vsp1_rpf.c
>> @@ -20,6 +20,18 @@
>>  #define RPF_MAX_WIDTH				8190
>>  #define RPF_MAX_HEIGHT				8190
>>
>> +/* Pre extended display list command data structure. */
>> +struct vsp1_extcmd_auto_fld_body {
>> +	u32 top_y0;
>> +	u32 bottom_y0;
>> +	u32 top_c0;
>> +	u32 bottom_c0;
>> +	u32 top_c1;
>> +	u32 bottom_c1;
>> +	u32 reserved0;
>> +	u32 reserved1;
>> +};
>> +
>>  /*
>> ---------------------------------------------------------------------------
>> -- * Device Access
>>   */
>> @@ -64,6 +76,14 @@ static void rpf_configure_stream(struct vsp1_entity
>> *entity, pstride |= format->plane_fmt[1].bytesperline
>>  			<< VI6_RPF_SRCM_PSTRIDE_C_SHIFT;
>>
>> +	/*
>> +	 * pstride has both STRIDE_Y and STRIDE_C, but multiplying the whole
>> +	 * of pstride by 2 is conveniently OK here as we are multiplying both
>> +	 * values.
>> +	 */
>> +	if (pipe->interlaced)
>> +		pstride *= 2;
>> +
>>  	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_PSTRIDE, pstride);
>>
>>  	/* Format */
>> @@ -100,6 +120,9 @@ static void rpf_configure_stream(struct vsp1_entity
>> *entity, top = compose->top;
>>  	}
>>
>> +	if (pipe->interlaced)
>> +		top /= 2;
>> +
>>  	vsp1_rpf_write(rpf, dlb, VI6_RPF_LOC,
>>  		       (left << VI6_RPF_LOC_HCOORD_SHIFT) |
>>  		       (top << VI6_RPF_LOC_VCOORD_SHIFT));
>> @@ -169,6 +192,36 @@ static void rpf_configure_stream(struct vsp1_entity
>> *entity,
>>
>>  }
>>
>> +static void vsp1_rpf_configure_autofld(struct vsp1_rwpf *rpf,
>> +				       struct vsp1_dl_list *dl)
>> +{
>> +	const struct v4l2_pix_format_mplane *format = &rpf->format;
>> +	struct vsp1_dl_ext_cmd *cmd;
>> +	struct vsp1_extcmd_auto_fld_body *auto_fld;
>> +	u32 offset_y, offset_c;
>> +
>> +	cmd = vsp1_dl_get_pre_cmd(dl);
>> +	if (WARN_ONCE(!cmd, "Failed to obtain an autofld cmd"))
>> +		return;
>> +
>> +	/* Re-index our auto_fld to match the current RPF. */
>> +	auto_fld = cmd->data;
>> +	auto_fld = &auto_fld[rpf->entity.index];
>> +
>> +	auto_fld->top_y0 = rpf->mem.addr[0];
>> +	auto_fld->top_c0 = rpf->mem.addr[1];
>> +	auto_fld->top_c1 = rpf->mem.addr[2];
>> +
>> +	offset_y = format->plane_fmt[0].bytesperline;
>> +	offset_c = format->plane_fmt[1].bytesperline;
>> +
>> +	auto_fld->bottom_y0 = rpf->mem.addr[0] + offset_y;
>> +	auto_fld->bottom_c0 = rpf->mem.addr[1] + offset_c;
>> +	auto_fld->bottom_c1 = rpf->mem.addr[2] + offset_c;
>> +
>> +	cmd->flags |= VI6_DL_EXT_AUTOFLD_INT | BIT(16 + rpf->entity.index);
>> +}
>> +
>>  static void rpf_configure_frame(struct vsp1_entity *entity,
>>  				struct vsp1_pipeline *pipe,
>>  				struct vsp1_dl_list *dl,
>> @@ -221,6 +274,11 @@ static void rpf_configure_partition(struct vsp1_entity
>> *entity, crop.left += pipe->partition->rpf.left;
>>  	}
>>
>> +	if (pipe->interlaced) {
>> +		crop.height = round_down(crop.height / 2, fmtinfo->vsub);
>> +		crop.top = round_down(crop.top / 2, fmtinfo->vsub);
>> +	}
>> +
>>  	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRC_BSIZE,
>>  		       (crop.width << VI6_RPF_SRC_BSIZE_BHSIZE_SHIFT) |
>>  		       (crop.height << VI6_RPF_SRC_BSIZE_BVSIZE_SHIFT));
>> @@ -249,9 +307,17 @@ static void rpf_configure_partition(struct vsp1_entity
>> *entity, fmtinfo->swap_uv)
>>  		swap(mem.addr[1], mem.addr[2]);
>>
>> -	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_Y, mem.addr[0]);
>> -	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_C0, mem.addr[1]);
>> -	vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_C1, mem.addr[2]);
>> +	/*
>> +	 * Interlaced pipelines will use the extended pre-cmd to process
>> +	 * SRCM_ADDR_{Y,C0,C1}
>> +	 */
>> +	if (pipe->interlaced) {
>> +		vsp1_rpf_configure_autofld(rpf, dl);
>> +	} else {
>> +		vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_Y, mem.addr[0]);
>> +		vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_C0, mem.addr[1]);
>> +		vsp1_rpf_write(rpf, dlb, VI6_RPF_SRCM_ADDR_C1, mem.addr[2]);
>> +	}
>>  }
>>
>>  static void rpf_partition(struct vsp1_entity *entity,
>> diff --git a/include/media/vsp1.h b/include/media/vsp1.h
>> index 678c24de1ac6..3093b9cb9067 100644
>> --- a/include/media/vsp1.h
>> +++ b/include/media/vsp1.h
>> @@ -25,6 +25,7 @@ int vsp1_du_init(struct device *dev);
>>   * struct vsp1_du_lif_config - VSP LIF configuration
>>   * @width: output frame width
>>   * @height: output frame height
>> + * @interlaced: true for interlaced pipelines
>>   * @callback: frame completion callback function (optional). When a
>> callback *	      is provided, the VSP driver guarantees that it will be
>> called once *	      and only once for each vsp1_du_atomic_flush() call.
>> @@ -33,6 +34,7 @@ int vsp1_du_init(struct device *dev);
>>  struct vsp1_du_lif_config {
>>  	unsigned int width;
>>  	unsigned int height;
>> +	bool interlaced;
>>
>>  	void (*callback)(void *data, bool completed, u32 crc);
>>  	void *callback_data;
> 

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

end of thread, other threads:[~2018-08-03 12:25 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-17 20:35 [PATCH v5 00/11] R-Car DU Interlaced support through VSP1 Kieran Bingham
2018-07-17 20:35 ` [PATCH v5 01/11] media: vsp1: drm: Fix minor grammar error Kieran Bingham
2018-07-17 20:35 ` [PATCH v5 02/11] media: vsp1: Remove packed attributes from aligned structures Kieran Bingham
2018-07-17 20:35 ` [PATCH v5 03/11] media: vsp1: Rename dl_child to dl_next Kieran Bingham
2018-07-17 20:35 ` [PATCH v5 04/11] media: vsp1: Remove unused display list structure field Kieran Bingham
2018-07-17 20:35 ` [PATCH v5 05/11] media: vsp1: Clean up DLM objects on error Kieran Bingham
2018-07-17 20:35 ` [PATCH v5 06/11] media: vsp1: Provide VSP1 feature helper macro Kieran Bingham
2018-07-17 20:35 ` [PATCH v5 07/11] media: vsp1: Use header display lists for all WPF outputs linked to the DU Kieran Bingham
2018-07-17 20:35 ` [PATCH v5 08/11] media: vsp1: Add support for extended display list headers Kieran Bingham
2018-08-02 14:39   ` Laurent Pinchart
2018-08-03  9:47     ` Kieran Bingham
2018-07-17 20:35 ` [PATCH v5 09/11] media: vsp1: Provide support for extended command pools Kieran Bingham
2018-08-02 14:58   ` Laurent Pinchart
2018-07-17 20:35 ` [PATCH v5 10/11] media: vsp1: Support Interlaced display pipelines Kieran Bingham
2018-08-02 22:44   ` Laurent Pinchart
2018-08-03 10:29     ` Kieran Bingham
2018-07-17 20:35 ` [PATCH v5 11/11] drm: rcar-du: Support interlaced video output through vsp1 Kieran Bingham
2018-08-02 22:46   ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).