On Wed Apr 30 02:29:00 2025 +0300, Laurent Pinchart wrote:
> The vsp1 driver implements very partial colour space support: it
> hardcodes the colorspace field on all video devices and subdevices to
> V4L2_COLORSPACE_SRGB, regardless of the configured format. The
> xfer_func, ycbcr_enc and quantization fields are not set (except for
> hsv_enc for HSV formats on video devices). This doesn't match the
> hardware configuration, which handles YUV data as encoding in BT.601
> with limited range.
>
> As a first step towards colour space configuration, keep the colour
> space fields hardcoded, but set them based on the selected format type
> (RGB, YUV or HSV).
>
> While at it, remove an extra blank line.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+rene...@ideasonboard.com>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen+rene...@ideasonboard.com>
> Link:
> https://lore.kernel.org/r/20250429232904.26413-6-laurent.pinchart+rene...@ideasonboard.com
> Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> Signed-off-by: Hans Verkuil <hverk...@xs4all.nl>
Patch committed.
Thanks,
Hans Verkuil
drivers/media/platform/renesas/vsp1/vsp1_brx.c | 9 ++++-
drivers/media/platform/renesas/vsp1/vsp1_entity.c | 22 +++++++++++-
drivers/media/platform/renesas/vsp1/vsp1_entity.h | 2 ++
drivers/media/platform/renesas/vsp1/vsp1_hsit.c | 11 +++++-
drivers/media/platform/renesas/vsp1/vsp1_pipe.c | 44 +++++++++++++++++++++++
drivers/media/platform/renesas/vsp1/vsp1_pipe.h | 2 ++
drivers/media/platform/renesas/vsp1/vsp1_rwpf.c | 11 +++++-
drivers/media/platform/renesas/vsp1/vsp1_sru.c | 9 ++++-
drivers/media/platform/renesas/vsp1/vsp1_uds.c | 9 ++++-
drivers/media/platform/renesas/vsp1/vsp1_video.c | 7 ++--
10 files changed, 115 insertions(+), 11 deletions(-)
---
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_brx.c
b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
index 5dee0490c593..5fc2e5a3bb30 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_brx.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_brx.c
@@ -15,6 +15,7 @@
#include "vsp1.h"
#include "vsp1_brx.h"
#include "vsp1_dl.h"
+#include "vsp1_entity.h"
#include "vsp1_pipe.h"
#include "vsp1_rwpf.h"
#include "vsp1_video.h"
@@ -108,6 +109,8 @@ static void brx_try_format(struct vsp1_brx *brx,
if (fmt->code != MEDIA_BUS_FMT_ARGB8888_1X32 &&
fmt->code != MEDIA_BUS_FMT_AYUV8_1X32)
fmt->code = MEDIA_BUS_FMT_AYUV8_1X32;
+
+ vsp1_entity_adjust_color_space(fmt);
break;
default:
@@ -115,13 +118,17 @@ static void brx_try_format(struct vsp1_brx *brx,
format = v4l2_subdev_state_get_format(sd_state,
BRX_PAD_SINK(0));
fmt->code = format->code;
+
+ fmt->colorspace = format->colorspace;
+ fmt->xfer_func = format->xfer_func;
+ fmt->ycbcr_enc = format->ycbcr_enc;
+ fmt->quantization = format->quantization;
break;
}
fmt->width = clamp(fmt->width, BRX_MIN_SIZE, BRX_MAX_SIZE);
fmt->height = clamp(fmt->height, BRX_MIN_SIZE, BRX_MAX_SIZE);
fmt->field = V4L2_FIELD_NONE;
- fmt->colorspace = V4L2_COLORSPACE_SRGB;
}
static int brx_set_format(struct v4l2_subdev *subdev,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index 2096a09a1278..a6680d531872 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -104,6 +104,20 @@ void vsp1_entity_configure_partition(struct vsp1_entity
*entity,
dl, dlb);
}
+void vsp1_entity_adjust_color_space(struct v4l2_mbus_framefmt *format)
+{
+ u8 xfer_func = format->xfer_func;
+ u8 ycbcr_enc = format->ycbcr_enc;
+ u8 quantization = format->quantization;
+
+ vsp1_adjust_color_space(format->code, &format->colorspace, &xfer_func,
+ &ycbcr_enc, &quantization);
+
+ format->xfer_func = xfer_func;
+ format->ycbcr_enc = ycbcr_enc;
+ format->quantization = quantization;
+}
+
/*
-----------------------------------------------------------------------------
* V4L2 Subdevice Operations
*/
@@ -334,7 +348,13 @@ int vsp1_subdev_set_pad_format(struct v4l2_subdev *subdev,
format->height = clamp_t(unsigned int, fmt->format.height,
min_height, max_height);
format->field = V4L2_FIELD_NONE;
- format->colorspace = V4L2_COLORSPACE_SRGB;
+
+ format->colorspace = fmt->format.colorspace;
+ format->xfer_func = fmt->format.xfer_func;
+ format->ycbcr_enc = fmt->format.ycbcr_enc;
+ format->quantization = fmt->format.quantization;
+
+ vsp1_entity_adjust_color_space(format);
fmt->format = *format;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
index bdcb780a79da..b7c72d0b7f8e 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.h
@@ -171,6 +171,8 @@ void vsp1_entity_configure_partition(struct vsp1_entity
*entity,
struct vsp1_dl_list *dl,
struct vsp1_dl_body *dlb);
+void vsp1_entity_adjust_color_space(struct v4l2_mbus_framefmt *format);
+
struct media_pad *vsp1_entity_remote_pad(struct media_pad *pad);
int vsp1_subdev_get_pad_format(struct v4l2_subdev *subdev,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
index 8ba2a7c7305c..1fcd1967d3b2 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_hsit.c
@@ -14,6 +14,7 @@
#include "vsp1.h"
#include "vsp1_dl.h"
+#include "vsp1_entity.h"
#include "vsp1_hsit.h"
#define HSIT_MIN_SIZE 4U
@@ -96,7 +97,13 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
format->height = clamp_t(unsigned int, fmt->format.height,
HSIT_MIN_SIZE, HSIT_MAX_SIZE);
format->field = V4L2_FIELD_NONE;
- format->colorspace = V4L2_COLORSPACE_SRGB;
+
+ format->colorspace = fmt->format.colorspace;
+ format->xfer_func = fmt->format.xfer_func;
+ format->ycbcr_enc = fmt->format.ycbcr_enc;
+ format->quantization = fmt->format.quantization;
+
+ vsp1_entity_adjust_color_space(format);
fmt->format = *format;
@@ -106,6 +113,8 @@ static int hsit_set_format(struct v4l2_subdev *subdev,
format->code = hsit->inverse ? MEDIA_BUS_FMT_ARGB8888_1X32
: MEDIA_BUS_FMT_AHSV8888_1X32;
+ vsp1_entity_adjust_color_space(format);
+
done:
mutex_unlock(&hsit->entity.lock);
return ret;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
index c8ec5bfa7944..94875c499df4 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c
@@ -346,6 +346,50 @@ vsp1_get_format_info_by_index(struct vsp1_device *vsp1,
unsigned int index,
return NULL;
}
+/**
+ * vsp1_adjust_color_space - Adjust color space fields in a format
+ * @code: the media bus code
+ * @colorspace: the colorspace
+ * @xfer_func: the transfer function
+ * @encoding: the encoding
+ * @quantization: the quantization
+ *
+ * This function adjusts all color space fields of a video device of subdev
+ * format structure, taking into account the requested format, requested color
+ * space and limitations of the VSP1. It should be used in the video device and
+ * subdev set format handlers.
+ *
+ * For now, simply hardcode the color space fields to the VSP1 defaults based
+ * on the media bus code.
+ */
+void vsp1_adjust_color_space(u32 code, u32 *colorspace, u8 *xfer_func,
+ u8 *encoding, u8 *quantization)
+{
+ switch (code) {
+ case MEDIA_BUS_FMT_ARGB8888_1X32:
+ default:
+ *colorspace = V4L2_COLORSPACE_SRGB;
+ *xfer_func = V4L2_XFER_FUNC_SRGB;
+ *encoding = V4L2_YCBCR_ENC_601;
+ *quantization = V4L2_QUANTIZATION_FULL_RANGE;
+ break;
+
+ case MEDIA_BUS_FMT_AHSV8888_1X32:
+ *colorspace = V4L2_COLORSPACE_SRGB;
+ *xfer_func = V4L2_XFER_FUNC_SRGB;
+ *encoding = V4L2_HSV_ENC_256;
+ *quantization = V4L2_QUANTIZATION_FULL_RANGE;
+ break;
+
+ case MEDIA_BUS_FMT_AYUV8_1X32:
+ *colorspace = V4L2_COLORSPACE_SMPTE170M;
+ *xfer_func = V4L2_XFER_FUNC_709;
+ *encoding = V4L2_YCBCR_ENC_601;
+ *quantization = V4L2_QUANTIZATION_LIM_RANGE;
+ break;
+ }
+}
+
/*
-----------------------------------------------------------------------------
* Pipeline Management
*/
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
index 383951c5bd90..7f623b8cbe5c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
+++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.h
@@ -183,5 +183,7 @@ const struct vsp1_format_info *vsp1_get_format_info(struct
vsp1_device *vsp1,
const struct vsp1_format_info *
vsp1_get_format_info_by_index(struct vsp1_device *vsp1, unsigned int index,
u32 code);
+void vsp1_adjust_color_space(u32 code, u32 *colorspace, u8 *xfer_func,
+ u8 *encoding, u8 *quantization);
#endif /* __VSP1_PIPE_H__ */
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index 1b4bac7b7cfa..4e8bcf6a59ad 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -10,6 +10,7 @@
#include <media/v4l2-subdev.h>
#include "vsp1.h"
+#include "vsp1_entity.h"
#include "vsp1_rwpf.h"
#include "vsp1_video.h"
@@ -90,6 +91,8 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
else
format->code = sink_format->code;
+ vsp1_entity_adjust_color_space(format);
+
fmt->format = *format;
goto done;
}
@@ -100,7 +103,13 @@ static int vsp1_rwpf_set_format(struct v4l2_subdev *subdev,
format->height = clamp_t(unsigned int, fmt->format.height,
RWPF_MIN_HEIGHT, rwpf->max_height);
format->field = V4L2_FIELD_NONE;
- format->colorspace = V4L2_COLORSPACE_SRGB;
+
+ format->colorspace = fmt->format.colorspace;
+ format->xfer_func = fmt->format.xfer_func;
+ format->ycbcr_enc = fmt->format.ycbcr_enc;
+ format->quantization = fmt->format.quantization;
+
+ vsp1_entity_adjust_color_space(format);
fmt->format = *format;
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
index 1759ce642e6e..bba2872afaf2 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_sru.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_sru.c
@@ -14,6 +14,7 @@
#include "vsp1.h"
#include "vsp1_dl.h"
+#include "vsp1_entity.h"
#include "vsp1_pipe.h"
#include "vsp1_sru.h"
@@ -178,6 +179,8 @@ static void sru_try_format(struct vsp1_sru *sru,
fmt->code != MEDIA_BUS_FMT_AYUV8_1X32)
fmt->code = MEDIA_BUS_FMT_AYUV8_1X32;
+ vsp1_entity_adjust_color_space(fmt);
+
fmt->width = clamp(fmt->width, SRU_MIN_SIZE, SRU_MAX_SIZE);
fmt->height = clamp(fmt->height, SRU_MIN_SIZE, SRU_MAX_SIZE);
break;
@@ -187,6 +190,11 @@ static void sru_try_format(struct vsp1_sru *sru,
format = v4l2_subdev_state_get_format(sd_state, SRU_PAD_SINK);
fmt->code = format->code;
+ fmt->colorspace = format->colorspace;
+ fmt->xfer_func = format->xfer_func;
+ fmt->ycbcr_enc = format->ycbcr_enc;
+ fmt->quantization = format->quantization;
+
/*
* We can upscale by 2 in both direction, but not independently.
* Compare the input and output rectangles areas (avoiding
@@ -211,7 +219,6 @@ static void sru_try_format(struct vsp1_sru *sru,
}
fmt->field = V4L2_FIELD_NONE;
- fmt->colorspace = V4L2_COLORSPACE_SRGB;
}
static int sru_set_format(struct v4l2_subdev *subdev,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
index c5a38478cf8c..2db473b6f83c 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_uds.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_uds.c
@@ -14,6 +14,7 @@
#include "vsp1.h"
#include "vsp1_dl.h"
+#include "vsp1_entity.h"
#include "vsp1_pipe.h"
#include "vsp1_uds.h"
@@ -177,6 +178,8 @@ static void uds_try_format(struct vsp1_uds *uds,
fmt->code != MEDIA_BUS_FMT_AYUV8_1X32)
fmt->code = MEDIA_BUS_FMT_AYUV8_1X32;
+ vsp1_entity_adjust_color_space(fmt);
+
fmt->width = clamp(fmt->width, UDS_MIN_SIZE, UDS_MAX_SIZE);
fmt->height = clamp(fmt->height, UDS_MIN_SIZE, UDS_MAX_SIZE);
break;
@@ -186,6 +189,11 @@ static void uds_try_format(struct vsp1_uds *uds,
format = v4l2_subdev_state_get_format(sd_state, UDS_PAD_SINK);
fmt->code = format->code;
+ fmt->colorspace = format->colorspace;
+ fmt->xfer_func = format->xfer_func;
+ fmt->ycbcr_enc = format->ycbcr_enc;
+ fmt->quantization = format->quantization;
+
uds_output_limits(format->width, &minimum, &maximum);
fmt->width = clamp(fmt->width, minimum, maximum);
uds_output_limits(format->height, &minimum, &maximum);
@@ -194,7 +202,6 @@ static void uds_try_format(struct vsp1_uds *uds,
}
fmt->field = V4L2_FIELD_NONE;
- fmt->colorspace = V4L2_COLORSPACE_SRGB;
}
static int uds_set_format(struct v4l2_subdev *subdev,
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c
b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index da578993f472..68d495c20a84 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -127,12 +127,10 @@ static int __vsp1_video_try_format(struct vsp1_video
*video,
info = vsp1_get_format_info(video->vsp1, VSP1_VIDEO_DEF_FORMAT);
pix->pixelformat = info->fourcc;
- pix->colorspace = V4L2_COLORSPACE_SRGB;
pix->field = V4L2_FIELD_NONE;
- if (info->fourcc == V4L2_PIX_FMT_HSV24 ||
- info->fourcc == V4L2_PIX_FMT_HSV32)
- pix->hsv_enc = V4L2_HSV_ENC_256;
+ vsp1_adjust_color_space(info->mbus, &pix->colorspace, &pix->xfer_func,
+ &pix->ycbcr_enc, &pix->quantization);
memset(pix->reserved, 0, sizeof(pix->reserved));
@@ -891,7 +889,6 @@ vsp1_video_querycap(struct file *file, void *fh, struct
v4l2_capability *cap)
| V4L2_CAP_IO_MC | V4L2_CAP_VIDEO_CAPTURE_MPLANE
| V4L2_CAP_VIDEO_OUTPUT_MPLANE;
-
strscpy(cap->driver, "vsp1", sizeof(cap->driver));
strscpy(cap->card, video->video.name, sizeof(cap->card));
