Re: [PATCH v2 2/9] media: rkisp1: Allow setting color space on ISP sink pad

[paul.elder@ideasonboard.com]

On Tue, Aug 23, 2022 at 08:18:33PM +0300, 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>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
> Changes since v1:
> 
> - Fix swapped default color spaces for YUV and Bayer
> - Improve coherency in usage of ternary operator ? :
> ---
>  .../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..b5bdf427c7e1 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 ? :
> +			       (is_yuv ? V4L2_COLORSPACE_SRGB :
> +				V4L2_COLORSPACE_RAW);
> +	sink_fmt->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 */