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@xxxxxxxxxxxxxxxx>
> > ---
> > .../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 */
--
Regards,
Laurent Pinchart
