[PATCH v2 0/5] misc improvements for j721e-csi2rx

[PATCH v2 0/5] misc improvements for j721e-csi2rx

[Julien Massot]

Change in V2:

- Add Fixes tag for patch 1 as suggested by Jai
- Add RGB patches from Jai

RGB patches 4, 5 have been tested with the MAX96714F pattern generator.

This small series:
- Fix media link issue when the remote subdevice has multiple output ports
- add support for Grey format patch 2 and 3
- add support for RGB formats patch 4 and 5

Jai Luthra (2):
  media: cadence: csi2rx: add support for RGB formats
  media: ti: j721e-csi2rx: add support for RGB formats

Julien Massot (3):
  media: cadence: csi2rx: use match fwnode for media link
  media: cadence: csi2rx: add Y8_1X8 format
  media: ti: j721e-csi2rx: add GREY format

 drivers/media/platform/cadence/cdns-csi2rx.c  |  6 ++++-
 .../platform/ti/j721e-csi2rx/j721e-csi2rx.c   | 24 +++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH v2 1/5] media: cadence: csi2rx: use match fwnode for media link

[Julien Massot]

On some subdev the fwnode is the device node and not the endpoint node.
Using the subdev fwnode doesn't allow to fetch the subdev
pad we are connected to when the remote subdev has multiple
output pads.

Fixes: 1fc3b37f34f69 ("media: v4l: cadence: Add Cadence MIPI-CSI2 RX driver")
Signed-off-by: Julien Massot <julien.massot@collabora.com>
Reviewed-by: Jai Luthra <j-luthra@ti.com>
---
 drivers/media/platform/cadence/cdns-csi2rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 91ef22e9109a..e83b877c48da 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -467,7 +467,7 @@ static int csi2rx_async_bound(struct v4l2_async_notifier *notifier,
 	struct csi2rx_priv *csi2rx = v4l2_subdev_to_csi2rx(subdev);
 
 	csi2rx->source_pad = media_entity_get_fwnode_pad(&s_subdev->entity,
-							 s_subdev->fwnode,
+							 asd->match.fwnode,
 							 MEDIA_PAD_FL_SOURCE);
 	if (csi2rx->source_pad < 0) {
 		dev_err(csi2rx->dev, "Couldn't find output pad for subdev %s\n",
-- 
2.43.0

Re: [PATCH v2 1/5] media: cadence: csi2rx: use match fwnode for media link

[Sakari Ailus]

Hi Julien,

Thanks for the patch.

On Mon, Dec 18, 2023 at 10:56:00AM +0100, Julien Massot wrote:
> On some subdev the fwnode is the device node and not the endpoint node.
> Using the subdev fwnode doesn't allow to fetch the subdev
> pad we are connected to when the remote subdev has multiple
> output pads.

I'd change the commit message, endpoint-to-endpoint matching used to be the
preferred way to do things not long ago.

> 
> Fixes: 1fc3b37f34f69 ("media: v4l: cadence: Add Cadence MIPI-CSI2 RX driver")

I think the driver was originally fine but I missed making the below change
when changing async matching. Instead I think you should have here:

Fixes: 1029939b3782 ("media: v4l: async: Simplify async sub-device fwnode matching")

IOW applying your patch without 1029939b3782 will also break cdns-csi2rx
driver.

> Signed-off-by: Julien Massot <julien.massot@collabora.com>
> Reviewed-by: Jai Luthra <j-luthra@ti.com>
> ---
>  drivers/media/platform/cadence/cdns-csi2rx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
> index 91ef22e9109a..e83b877c48da 100644
> --- a/drivers/media/platform/cadence/cdns-csi2rx.c
> +++ b/drivers/media/platform/cadence/cdns-csi2rx.c
> @@ -467,7 +467,7 @@ static int csi2rx_async_bound(struct v4l2_async_notifier *notifier,
>  	struct csi2rx_priv *csi2rx = v4l2_subdev_to_csi2rx(subdev);
>  
>  	csi2rx->source_pad = media_entity_get_fwnode_pad(&s_subdev->entity,
> -							 s_subdev->fwnode,
> +							 asd->match.fwnode,
>  							 MEDIA_PAD_FL_SOURCE);
>  	if (csi2rx->source_pad < 0) {
>  		dev_err(csi2rx->dev, "Couldn't find output pad for subdev %s\n",

-- 
Regards,

Sakari Ailus

Re: [PATCH v2 1/5] media: cadence: csi2rx: use match fwnode for media link

[Julien Massot]

Hi Sakari

On 12/18/23 12:09, Sakari Ailus wrote:
> Hi Julien,
> 
> Thanks for the patch.
> 
> On Mon, Dec 18, 2023 at 10:56:00AM +0100, Julien Massot wrote:
>> On some subdev the fwnode is the device node and not the endpoint node.
>> Using the subdev fwnode doesn't allow to fetch the subdev
>> pad we are connected to when the remote subdev has multiple
>> output pads.
> 
> I'd change the commit message, endpoint-to-endpoint matching used to be the
> preferred way to do things not long ago.
> 
>>
>> Fixes: 1fc3b37f34f69 ("media: v4l: cadence: Add Cadence MIPI-CSI2 RX driver")
> 
> I think the driver was originally fine but I missed making the below change
> when changing async matching. Instead I think you should have here:
> 
> Fixes: 1029939b3782 ("media: v4l: async: Simplify async sub-device fwnode matching")
> 
> IOW applying your patch without 1029939b3782 will also break cdns-csi2rx
> driver.

I just replaced the fix tag, and the commit message as suggested in V3.

Thank you,
Julien

[PATCH v2 2/5] media: cadence: csi2rx: add Y8_1X8 format

[Julien Massot]

Add support for MEDIA_BUS_FMT_Y8_1X8 format.

Signed-off-by: Julien Massot <julien.massot@collabora.com>
Acked-by: Jai Luthra <j-luthra@ti.com>
---
 drivers/media/platform/cadence/cdns-csi2rx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index e83b877c48da..2ca4a8cde75e 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -114,6 +114,7 @@ static const struct csi2rx_fmt formats[] = {
 	{ .code	= MEDIA_BUS_FMT_SGBRG8_1X8, .bpp = 8, },
 	{ .code	= MEDIA_BUS_FMT_SGRBG8_1X8, .bpp = 8, },
 	{ .code	= MEDIA_BUS_FMT_SRGGB8_1X8, .bpp = 8, },
+	{ .code	= MEDIA_BUS_FMT_Y8_1X8,     .bpp = 8, },
 	{ .code	= MEDIA_BUS_FMT_SBGGR10_1X10, .bpp = 10, },
 	{ .code	= MEDIA_BUS_FMT_SGBRG10_1X10, .bpp = 10, },
 	{ .code	= MEDIA_BUS_FMT_SGRBG10_1X10, .bpp = 10, },
-- 
2.43.0

[PATCH v2 3/5] media: ti: j721e-csi2rx: add GREY format

[Julien Massot]

Add Grey format to properly select RAW8 data type.

Tested with a ST VG5661 sensor.

Signed-off-by: Julien Massot <julien.massot@collabora.com>
Acked-by: Jai Luthra <j-luthra@ti.com>
---
 drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
index 9518c78cee4a..6e58112957ce 100644
--- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
+++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
@@ -158,6 +158,12 @@ static const struct ti_csi2rx_fmt ti_csi2rx_formats[] = {
 		.csi_dt			= MIPI_CSI2_DT_RAW8,
 		.bpp			= 8,
 		.size			= SHIM_DMACNTX_SIZE_8,
+	}, {
+		.fourcc			= V4L2_PIX_FMT_GREY,
+		.code			= MEDIA_BUS_FMT_Y8_1X8,
+		.csi_dt			= MIPI_CSI2_DT_RAW8,
+		.bpp			= 8,
+		.size			= SHIM_DMACNTX_SIZE_8,
 	}, {
 		.fourcc			= V4L2_PIX_FMT_SBGGR10,
 		.code			= MEDIA_BUS_FMT_SBGGR10_1X10,
-- 
2.43.0

[PATCH v2 4/5] media: cadence: csi2rx: add support for RGB formats

[Julien Massot]

From: Jai Luthra <j-luthra@ti.com>

Add support for RGB565, RGB888 and BGR888 media bus formats.

Signed-off-by: Jai Luthra <j-luthra@ti.com>
Signed-off-by: Julien Massot <julien.massot@collabora.com>
Tested-by: Julien Massot <julien.massot@collabora.com>
---
 drivers/media/platform/cadence/cdns-csi2rx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 2ca4a8cde75e..aa08733172a5 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -119,6 +119,9 @@ static const struct csi2rx_fmt formats[] = {
 	{ .code	= MEDIA_BUS_FMT_SGBRG10_1X10, .bpp = 10, },
 	{ .code	= MEDIA_BUS_FMT_SGRBG10_1X10, .bpp = 10, },
 	{ .code	= MEDIA_BUS_FMT_SRGGB10_1X10, .bpp = 10, },
+	{ .code	= MEDIA_BUS_FMT_RGB565_1X16,  .bpp = 16, },
+	{ .code	= MEDIA_BUS_FMT_RGB888_1X24,  .bpp = 24, },
+	{ .code	= MEDIA_BUS_FMT_BGR888_1X24,  .bpp = 24, },
 };
 
 static const struct csi2rx_fmt *csi2rx_get_fmt_by_code(u32 code)
-- 
2.43.0

[PATCH v2 5/5] media: ti: j721e-csi2rx: add support for RGB formats

[Julien Massot]

From: Jai Luthra <j-luthra@ti.com>

Add support for RGB565, RGB888 and BGR888 media bus formats.

The pixel formats for these have a different byte-order than the MIPI
bus formats, given the SHIM IP [1] unpacks them into memory as follows:

MIPI RGB888 unpacks to:
BYTE3		BYTE2		BYTE1		BYTE0
00000000	RRRRRRRR	GGGGGGGG	BBBBBBBB

MIPI RGB565 unpacks to:
BIT 15-11	BIT 10-5	BIT 4-0
RRRRR		GGGGGG		BBBBB

[1]: AM62x TRM: 12.6.1.4.5 "CSI_RX_IF Data Memory Organization Details"

Link: https://www.ti.com/lit/pdf/spruiv7
Signed-off-by: Jai Luthra <j-luthra@ti.com>
Signed-off-by: Julien Massot <julien.massot@collabora.com>
Tested-by: Julien Massot <julien.massot@collabora.com>
---
 .../platform/ti/j721e-csi2rx/j721e-csi2rx.c    | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
index 6e58112957ce..15ddc851737a 100644
--- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
+++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
@@ -188,6 +188,24 @@ static const struct ti_csi2rx_fmt ti_csi2rx_formats[] = {
 		.csi_dt			= MIPI_CSI2_DT_RAW10,
 		.bpp			= 16,
 		.size			= SHIM_DMACNTX_SIZE_16,
+	}, {
+		.fourcc			= V4L2_PIX_FMT_RGB565X,
+		.code			= MEDIA_BUS_FMT_RGB565_1X16,
+		.csi_dt			= MIPI_CSI2_DT_RGB565,
+		.bpp			= 16,
+		.size			= SHIM_DMACNTX_SIZE_16,
+	}, {
+		.fourcc			= V4L2_PIX_FMT_XBGR32,
+		.code			= MEDIA_BUS_FMT_RGB888_1X24,
+		.csi_dt			= MIPI_CSI2_DT_RGB888,
+		.bpp			= 32,
+		.size			= SHIM_DMACNTX_SIZE_32,
+	}, {
+		.fourcc			= V4L2_PIX_FMT_RGBX32,
+		.code			= MEDIA_BUS_FMT_BGR888_1X24,
+		.csi_dt			= MIPI_CSI2_DT_RGB888,
+		.bpp			= 32,
+		.size			= SHIM_DMACNTX_SIZE_32,
 	},
 
 	/* More formats can be supported but they are not listed for now. */
-- 
2.43.0