| Message ID | 20220815065235.23797-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 6844cebbf60ac61296a96f1bf57966f98d8d2d6a |
| Headers | show |
| Series | media: rkisp1: Fix and improve color space support | expand |
On 15.08.2022 09:52, Laurent Pinchart wrote: >The ISP accepts different color spaces on its input: for YUV input, it >doesn't set any restrictions, and for Bayer inputs, any color primaries >or transfer function can be accepted (YCbCr encoding isn't applicable >there, and quantization range can only be full). > >Allow setting a color space on the ISP sink pad, with the aforementioned >restrictions. The settings don't influence hardware yet (only the YUV >quantization range will, anything else has no direct effect on the ISP >configuration), but can already be set to allow color space information >to be coherent across the ISP sink link. > >Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >--- > .../platform/rockchip/rkisp1/rkisp1-isp.c | 31 +++++++++++++++++++ > 1 file changed, 31 insertions(+) > >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c >index a52b22824739..32114d1e8ad1 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c >@@ -705,6 +705,7 @@ static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp, > const struct rkisp1_mbus_info *mbus_info; > struct v4l2_mbus_framefmt *sink_fmt; > struct v4l2_rect *sink_crop; >+ bool is_yuv; > > sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state, > RKISP1_ISP_PAD_SINK_VIDEO, >@@ -725,6 +726,36 @@ static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp, > RKISP1_ISP_MIN_HEIGHT, > RKISP1_ISP_MAX_HEIGHT); > >+ /* >+ * Adjust the color space fields. Accept any color primaries and >+ * transfer function for both YUV and Bayer. For YUV any YCbCr encoding >+ * and quantization range is also accepted. For Bayer formats, the YCbCr >+ * encoding isn't applicable, and the quantization range can only be >+ * full. >+ */ >+ is_yuv = mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV; >+ >+ sink_fmt->colorspace = format->colorspace ? format->colorspace : >+ (is_yuv ? V4L2_COLORSPACE_RAW : I don't understand enough of the different colorspaces, why is 'raw' chosen for yuv input? Thanks, Dafna >+ V4L2_COLORSPACE_SRGB); >+ sink_fmt->xfer_func = format->xfer_func ? format->xfer_func : >+ V4L2_MAP_XFER_FUNC_DEFAULT(sink_fmt->colorspace); >+ if (is_yuv) { >+ sink_fmt->ycbcr_enc = format->ycbcr_enc ? : >+ V4L2_MAP_YCBCR_ENC_DEFAULT(sink_fmt->colorspace); >+ sink_fmt->quantization = format->quantization ? : >+ V4L2_MAP_QUANTIZATION_DEFAULT(false, sink_fmt->colorspace, >+ sink_fmt->ycbcr_enc); >+ } else { >+ /* >+ * The YCbCr encoding isn't applicable for non-YUV formats, but >+ * V4L2 has no "no encoding" value. Hardcode it to Rec. 601, it >+ * should be ignored by userspace. >+ */ >+ sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_601; >+ sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE; >+ } >+ > *format = *sink_fmt; > > /* Propagate to in crop */ >-- >Regards, > >Laurent Pinchart >
Hi Dafna, On Thu, Aug 18, 2022 at 07:00:27AM +0300, Dafna Hirschfeld wrote: > On 15.08.2022 09:52, Laurent Pinchart wrote: > > The ISP accepts different color spaces on its input: for YUV input, it > > doesn't set any restrictions, and for Bayer inputs, any color primaries > > or transfer function can be accepted (YCbCr encoding isn't applicable > > there, and quantization range can only be full). > > > > Allow setting a color space on the ISP sink pad, with the aforementioned > > restrictions. The settings don't influence hardware yet (only the YUV > > quantization range will, anything else has no direct effect on the ISP > > configuration), but can already be set to allow color space information > > to be coherent across the ISP sink link. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > .../platform/rockchip/rkisp1/rkisp1-isp.c | 31 +++++++++++++++++++ > > 1 file changed, 31 insertions(+) > > > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > > index a52b22824739..32114d1e8ad1 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > > @@ -705,6 +705,7 @@ static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp, > > const struct rkisp1_mbus_info *mbus_info; > > struct v4l2_mbus_framefmt *sink_fmt; > > struct v4l2_rect *sink_crop; > > + bool is_yuv; > > > > sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state, > > RKISP1_ISP_PAD_SINK_VIDEO, > > @@ -725,6 +726,36 @@ static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp, > > RKISP1_ISP_MIN_HEIGHT, > > RKISP1_ISP_MAX_HEIGHT); > > > > + /* > > + * Adjust the color space fields. Accept any color primaries and > > + * transfer function for both YUV and Bayer. For YUV any YCbCr encoding > > + * and quantization range is also accepted. For Bayer formats, the YCbCr > > + * encoding isn't applicable, and the quantization range can only be > > + * full. > > + */ > > + is_yuv = mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV; > > + > > + sink_fmt->colorspace = format->colorspace ? format->colorspace : > > + (is_yuv ? V4L2_COLORSPACE_RAW : > > I don't understand enough of the different colorspaces, why is 'raw' > chosen for yuv input? You clearly understand the topic as you've spotted a bug :-) It should be the other way around, for Bayer input the driver should default to RAW, and for YUV input, to SRGB (which in V4L2 is used to describe limited-range, Rec. 601 encoded sRGB RGB data). > > + V4L2_COLORSPACE_SRGB); > > + sink_fmt->xfer_func = format->xfer_func ? format->xfer_func : > > + V4L2_MAP_XFER_FUNC_DEFAULT(sink_fmt->colorspace); > > + if (is_yuv) { > > + sink_fmt->ycbcr_enc = format->ycbcr_enc ? : > > + V4L2_MAP_YCBCR_ENC_DEFAULT(sink_fmt->colorspace); > > + sink_fmt->quantization = format->quantization ? : > > + V4L2_MAP_QUANTIZATION_DEFAULT(false, sink_fmt->colorspace, > > + sink_fmt->ycbcr_enc); > > + } else { > > + /* > > + * The YCbCr encoding isn't applicable for non-YUV formats, but > > + * V4L2 has no "no encoding" value. Hardcode it to Rec. 601, it > > + * should be ignored by userspace. > > + */ > > + sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_601; > > + sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE; > > + } > > + > > *format = *sink_fmt; > > > > /* Propagate to in crop */
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c index a52b22824739..32114d1e8ad1 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c @@ -705,6 +705,7 @@ static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp, const struct rkisp1_mbus_info *mbus_info; struct v4l2_mbus_framefmt *sink_fmt; struct v4l2_rect *sink_crop; + bool is_yuv; sink_fmt = rkisp1_isp_get_pad_fmt(isp, sd_state, RKISP1_ISP_PAD_SINK_VIDEO, @@ -725,6 +726,36 @@ static void rkisp1_isp_set_sink_fmt(struct rkisp1_isp *isp, RKISP1_ISP_MIN_HEIGHT, RKISP1_ISP_MAX_HEIGHT); + /* + * Adjust the color space fields. Accept any color primaries and + * transfer function for both YUV and Bayer. For YUV any YCbCr encoding + * and quantization range is also accepted. For Bayer formats, the YCbCr + * encoding isn't applicable, and the quantization range can only be + * full. + */ + is_yuv = mbus_info->pixel_enc == V4L2_PIXEL_ENC_YUV; + + sink_fmt->colorspace = format->colorspace ? format->colorspace : + (is_yuv ? V4L2_COLORSPACE_RAW : + V4L2_COLORSPACE_SRGB); + sink_fmt->xfer_func = format->xfer_func ? format->xfer_func : + V4L2_MAP_XFER_FUNC_DEFAULT(sink_fmt->colorspace); + if (is_yuv) { + sink_fmt->ycbcr_enc = format->ycbcr_enc ? : + V4L2_MAP_YCBCR_ENC_DEFAULT(sink_fmt->colorspace); + sink_fmt->quantization = format->quantization ? : + V4L2_MAP_QUANTIZATION_DEFAULT(false, sink_fmt->colorspace, + sink_fmt->ycbcr_enc); + } else { + /* + * The YCbCr encoding isn't applicable for non-YUV formats, but + * V4L2 has no "no encoding" value. Hardcode it to Rec. 601, it + * should be ignored by userspace. + */ + sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_601; + sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE; + } + *format = *sink_fmt; /* Propagate to in crop */
The ISP accepts different color spaces on its input: for YUV input, it doesn't set any restrictions, and for Bayer inputs, any color primaries or transfer function can be accepted (YCbCr encoding isn't applicable there, and quantization range can only be full). Allow setting a color space on the ISP sink pad, with the aforementioned restrictions. The settings don't influence hardware yet (only the YUV quantization range will, anything else has no direct effect on the ISP configuration), but can already be set to allow color space information to be coherent across the ISP sink link. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- .../platform/rockchip/rkisp1/rkisp1-isp.c | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+)