[PATCH 0/2] media: vsp1: V3U VSP-X/VSP-D Support

[PATCH 0/2] media: vsp1: V3U VSP-X/VSP-D Support

[Kieran Bingham]

A couple of patches to discuss the enablement of the VSP on the V3U
(r8a779a0) platform.

The VSPX patch is purely RFC, howeverc the VSPD patch may be enough to
support that component on the new platform.

As yet, these patches are untested while I await hardware to arrive -
but they're potentially worth discussing already.


Kieran Bingham (2):
  media: vsp1: Add support for the V3U VSPD
  [RFC]: media: vsp1: Add support for the V3U VSPX

 drivers/media/platform/vsp1/vsp1.h      |  2 ++
 drivers/media/platform/vsp1/vsp1_drv.c  | 18 ++++++++++++++++++
 drivers/media/platform/vsp1/vsp1_regs.h |  5 +++++
 3 files changed, 25 insertions(+)

-- 
2.25.1

[PATCH 1/2] media: vsp1: Add support for the V3U VSPD

[Kieran Bingham]

The V3U provides two VSPD instances, with a new update to the version
register to detect the new SoC.

Add the new version and model detection, and detail the features
available in this module.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/media/platform/vsp1/vsp1_drv.c  | 10 ++++++++++
 drivers/media/platform/vsp1/vsp1_regs.h |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
index dc62533cf32c..c59e865dfef2 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -785,6 +785,16 @@ static const struct vsp1_device_info vsp1_device_infos[] = {
 		.uif_count = 2,
 		.wpf_count = 2,
 		.num_bru_inputs = 5,
+	}, {
+		.version = VI6_IP_VERSION_MODEL_VSPD_V3U,
+		.model = "VSP2-D",
+		.gen = 3,
+		.features = VSP1_HAS_BRU | VSP1_HAS_CLU | VSP1_HAS_EXT_DL,
+		.lif_count = 1,
+		.rpf_count = 5,
+		.uif_count = 2,
+		.wpf_count = 1,
+		.num_bru_inputs = 5,
 	},
 };
 
diff --git a/drivers/media/platform/vsp1/vsp1_regs.h b/drivers/media/platform/vsp1/vsp1_regs.h
index fe3130db1fa2..b378ea4451ce 100644
--- a/drivers/media/platform/vsp1/vsp1_regs.h
+++ b/drivers/media/platform/vsp1/vsp1_regs.h
@@ -766,6 +766,8 @@
 #define VI6_IP_VERSION_MODEL_VSPD_V3	(0x18 << 8)
 #define VI6_IP_VERSION_MODEL_VSPDL_GEN3	(0x19 << 8)
 #define VI6_IP_VERSION_MODEL_VSPBS_GEN3	(0x1a << 8)
+#define VI6_IP_VERSION_MODEL_VSPD_V3U	(0x1c << 8)
+
 #define VI6_IP_VERSION_SOC_MASK		(0xff << 0)
 #define VI6_IP_VERSION_SOC_H2		(0x01 << 0)
 #define VI6_IP_VERSION_SOC_V2H		(0x01 << 0)
@@ -777,6 +779,7 @@
 #define VI6_IP_VERSION_SOC_D3		(0x04 << 0)
 #define VI6_IP_VERSION_SOC_M3N		(0x04 << 0)
 #define VI6_IP_VERSION_SOC_E3		(0x04 << 0)
+#define VI6_IP_VERSION_SOC_V3U		(0x05 << 0)
 
 /* -----------------------------------------------------------------------------
  * RPF CLUT Registers
-- 
2.25.1

[PATCH 2/2] [RFC]: media: vsp1: Add support for the V3U VSPX

[Kieran Bingham]

The V3U provides four VSPX instances. This module brings in the IIF, to
process images through the ISP.

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

---

We need to set the VI6_DPR_ROUTE_IIFSEL bit on the VI6_DPR_BRU_ROUTE to
enable output on the IIF.

At the moment I'm envisaging adding a new entity to keep our existing
model, and map the IIF in the media graph, but it might be that we only
need to set this one bit to output on the IIF - so there might not be a
need for such over engineering.

In this patch I've provisionally added a VSP1_HAS_IIF feature, thinking
it would be needed, but as I've moved on - I don't actually see that it
would be required, so this likely gets dropped.

We will have some Bayer specific formats to handle, but they can be
flagged based on the SoC type or a HAS_BAYER formats rather than the IIF
if needed.

Furthermore, table 32.39 (Registers VSPX supports) indicates that the
VSPX still needs to configure the WPF0 when used (and that the BRU must
be connected, which we can handle in the routing tables).

So we may have to instantiate an instance of WPF specifically to handle
the IIF, even though the IIF is actually a function of the BRU ...

Still seems a bit too early to tell - so I think this patch really is
just for potential discussions - and not destined at all for
integration.
---
 drivers/media/platform/vsp1/vsp1.h      | 2 ++
 drivers/media/platform/vsp1/vsp1_drv.c  | 8 ++++++++
 drivers/media/platform/vsp1/vsp1_regs.h | 2 ++
 3 files changed, 12 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1.h b/drivers/media/platform/vsp1/vsp1.h
index 37cf33c7e6ca..a2fecd9392fd 100644
--- a/drivers/media/platform/vsp1/vsp1.h
+++ b/drivers/media/platform/vsp1/vsp1.h
@@ -54,12 +54,14 @@ struct vsp1_uif;
 #define VSP1_HAS_HGT		BIT(8)
 #define VSP1_HAS_BRS		BIT(9)
 #define VSP1_HAS_EXT_DL		BIT(10)
+#define VSP1_HAS_IIF		BIT(11)
 
 struct vsp1_device_info {
 	u32 version;
 	const char *model;
 	unsigned int gen;
 	unsigned int features;
+	unsigned int iif_count;
 	unsigned int lif_count;
 	unsigned int rpf_count;
 	unsigned int uds_count;
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
index c59e865dfef2..15e327516828 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -795,6 +795,14 @@ static const struct vsp1_device_info vsp1_device_infos[] = {
 		.uif_count = 2,
 		.wpf_count = 1,
 		.num_bru_inputs = 5,
+	}, {
+		.version = VI6_IP_VERSION_MODEL_VSPX_V3U,
+		.model = "VSP2-X",
+		.gen = 3,
+		.features = VSP1_HAS_IIF | VSP1_HAS_BRU | VSP1_HAS_EXT_DL,
+		.iif_count = 1,
+		.rpf_count = 4,
+		.num_bru_inputs = 4,
 	},
 };
 
diff --git a/drivers/media/platform/vsp1/vsp1_regs.h b/drivers/media/platform/vsp1/vsp1_regs.h
index b378ea4451ce..700ede1241e0 100644
--- a/drivers/media/platform/vsp1/vsp1_regs.h
+++ b/drivers/media/platform/vsp1/vsp1_regs.h
@@ -364,6 +364,7 @@
 #define VI6_DPR_HST_ROUTE		0x2044
 #define VI6_DPR_HSI_ROUTE		0x2048
 #define VI6_DPR_BRU_ROUTE		0x204c
+#define VI6_DPR_ROUTE_IIFSEL		BIT(28)
 #define VI6_DPR_ILV_BRS_ROUTE		0x2050
 #define VI6_DPR_ROUTE_BRSSEL		BIT(28)
 #define VI6_DPR_ROUTE_FXA_MASK		(0xff << 16)
@@ -767,6 +768,7 @@
 #define VI6_IP_VERSION_MODEL_VSPDL_GEN3	(0x19 << 8)
 #define VI6_IP_VERSION_MODEL_VSPBS_GEN3	(0x1a << 8)
 #define VI6_IP_VERSION_MODEL_VSPD_V3U	(0x1c << 8)
+#define VI6_IP_VERSION_MODEL_VSPX_V3U	(0x1d << 8)
 
 #define VI6_IP_VERSION_SOC_MASK		(0xff << 0)
 #define VI6_IP_VERSION_SOC_H2		(0x01 << 0)
-- 
2.25.1

Re: [PATCH 2/2] [RFC]: media: vsp1: Add support for the V3U VSPX

[Laurent Pinchart]

Hi Kieran,

I've just realized I've never replied to this.

On Thu, Dec 17, 2020 at 05:13:49PM +0000, Kieran Bingham wrote:
> The V3U provides four VSPX instances. This module brings in the IIF, to
> process images through the ISP.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> ---
> 
> We need to set the VI6_DPR_ROUTE_IIFSEL bit on the VI6_DPR_BRU_ROUTE to
> enable output on the IIF.
> 
> At the moment I'm envisaging adding a new entity to keep our existing
> model, and map the IIF in the media graph, but it might be that we only
> need to set this one bit to output on the IIF - so there might not be a
> need for such over engineering.
> 
> In this patch I've provisionally added a VSP1_HAS_IIF feature, thinking
> it would be needed, but as I've moved on - I don't actually see that it
> would be required, so this likely gets dropped.
> 
> We will have some Bayer specific formats to handle, but they can be
> flagged based on the SoC type or a HAS_BAYER formats rather than the IIF
> if needed.

The hardware seems to map raw formats to RGB formats, for instance using
RGB332 for RAW8. I wonder if this means that a RAW8 byte gets split to
the internal 32-bit ARGB bus, with the bits then extracted by the ISP.
We'll have to figure out how to handle that.

> Furthermore, table 32.39 (Registers VSPX supports) indicates that the
> VSPX still needs to configure the WPF0 when used (and that the BRU must
> be connected, which we can handle in the routing tables).

Hmmmm... It's not entirely clear. We certainly have to set the IIFSEL
bit, but I wonder if the mention of the RT[5:0] field being set to 56
(WPF0) wouldn't be about the WPF output use case only.

Another question is how to model the connection to the ISP in the media
graph, and whether the VPSX and ISP need to be part of the same graph.
That would be interesting, and would need to be taken into account in
the DT bindings. As a first step, an IIF entity to model the path
towards the ISP (with a sink pad but no source pad) could be a way to
configure the routing to either the ISP or the WPF.

> So we may have to instantiate an instance of WPF specifically to handle
> the IIF, even though the IIF is actually a function of the BRU ...
> 
> Still seems a bit too early to tell - so I think this patch really is
> just for potential discussions - and not destined at all for
> integration.

I think we need to experiment :-) Now that Niklas has submitted a driver
for the ISP routing, maybe we'll be able to test this ?

> ---
>  drivers/media/platform/vsp1/vsp1.h      | 2 ++
>  drivers/media/platform/vsp1/vsp1_drv.c  | 8 ++++++++
>  drivers/media/platform/vsp1/vsp1_regs.h | 2 ++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1.h b/drivers/media/platform/vsp1/vsp1.h
> index 37cf33c7e6ca..a2fecd9392fd 100644
> --- a/drivers/media/platform/vsp1/vsp1.h
> +++ b/drivers/media/platform/vsp1/vsp1.h
> @@ -54,12 +54,14 @@ struct vsp1_uif;
>  #define VSP1_HAS_HGT		BIT(8)
>  #define VSP1_HAS_BRS		BIT(9)
>  #define VSP1_HAS_EXT_DL		BIT(10)
> +#define VSP1_HAS_IIF		BIT(11)
>  
>  struct vsp1_device_info {
>  	u32 version;
>  	const char *model;
>  	unsigned int gen;
>  	unsigned int features;
> +	unsigned int iif_count;
>  	unsigned int lif_count;
>  	unsigned int rpf_count;
>  	unsigned int uds_count;
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
> index c59e865dfef2..15e327516828 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -795,6 +795,14 @@ static const struct vsp1_device_info vsp1_device_infos[] = {
>  		.uif_count = 2,
>  		.wpf_count = 1,
>  		.num_bru_inputs = 5,
> +	}, {
> +		.version = VI6_IP_VERSION_MODEL_VSPX_V3U,
> +		.model = "VSP2-X",
> +		.gen = 3,
> +		.features = VSP1_HAS_IIF | VSP1_HAS_BRU | VSP1_HAS_EXT_DL,
> +		.iif_count = 1,
> +		.rpf_count = 4,
> +		.num_bru_inputs = 4,
>  	},
>  };
>  
> diff --git a/drivers/media/platform/vsp1/vsp1_regs.h b/drivers/media/platform/vsp1/vsp1_regs.h
> index b378ea4451ce..700ede1241e0 100644
> --- a/drivers/media/platform/vsp1/vsp1_regs.h
> +++ b/drivers/media/platform/vsp1/vsp1_regs.h
> @@ -364,6 +364,7 @@
>  #define VI6_DPR_HST_ROUTE		0x2044
>  #define VI6_DPR_HSI_ROUTE		0x2048
>  #define VI6_DPR_BRU_ROUTE		0x204c
> +#define VI6_DPR_ROUTE_IIFSEL		BIT(28)
>  #define VI6_DPR_ILV_BRS_ROUTE		0x2050
>  #define VI6_DPR_ROUTE_BRSSEL		BIT(28)
>  #define VI6_DPR_ROUTE_FXA_MASK		(0xff << 16)
> @@ -767,6 +768,7 @@
>  #define VI6_IP_VERSION_MODEL_VSPDL_GEN3	(0x19 << 8)
>  #define VI6_IP_VERSION_MODEL_VSPBS_GEN3	(0x1a << 8)
>  #define VI6_IP_VERSION_MODEL_VSPD_V3U	(0x1c << 8)
> +#define VI6_IP_VERSION_MODEL_VSPX_V3U	(0x1d << 8)
>  
>  #define VI6_IP_VERSION_SOC_MASK		(0xff << 0)
>  #define VI6_IP_VERSION_SOC_H2		(0x01 << 0)

-- 
Regards,

Laurent Pinchart