Re: [PATCH v2 1/2] media: rkisp1-isp: Set correct data mode for YUV bypass

[Alen Karnil <alen.karnil@ideasonboard.com>]

Hi all,

Taking over from Isaac but have some points to discuss.

On 05/02/2026 10:32, Isaac Scott wrote:
> The rkisp1 features a 'bypass' mode for RAW and YUV formats. This
> disables all ISP blocks, and makes the rkisp1 display input data from
> the MIPI CSI receiver at the output, unmodified.
> 
> To determine whether we can activate bypass, we can detect whether both
> the source and sink formats are YUV. If they are, we must configure the
> ISP to expect a YUV input, interpreting H/VSYNC signals as data
> enable / disable.
> 
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---
> 
> Changelog since v1:
> - Removed in_bypass flag
> - Renamed the patch to better represent the functionality of the patch
> 
> ---
>   drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index 2311672cedb1..21bfa0edbaf1 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -240,7 +240,9 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp,
>   		}
>   	} else if (sink_fmt->pixel_enc == V4L2_PIXEL_ENC_YUV) {
>   		acq_mult = 2;
> -		if (mbus_type == V4L2_MBUS_CSI2_DPHY) {
> +		if (src_fmt->pixel_enc == V4L2_PIXEL_ENC_YUV) {
As Laurent states, this condition is always true, this will need to be 
reworked.
> +			isp_ctrl = RKISP1_CIF_ISP_CTRL_ISP_MODE_DATA_MODE;
Having tested this, DATA_MODE is correct, I am seeing a garbled image if 
using ITU601, definitely needs a comment if it goes against the 
documentation.

If we drop the earlier always true if statement, then the fix is to 
replace ITU601 selection with DATA_MODE, but there two paths that lead 
to ITU601 being selected, and I can only test one of them.

So the question is should both be replaced?
> +		} else if (mbus_type == V4L2_MBUS_CSI2_DPHY) {
>   			isp_ctrl = RKISP1_CIF_ISP_CTRL_ISP_MODE_ITU601;
>   		} else {
>   			if (mbus_type == V4L2_MBUS_BT656)

-- 
Thanks,
Alen Karnil