Re: [PATCH RFC 3/4] media: qcom: camss: vfe: Add the demux + BLS stage to the pixel pipe

[bod]

On 2026-10-13 09:44 +0100, William Bright wrote:
> The pixel pipe never subtracts the sensor black level, so blacks come
> out lifted and a demosaic that is sensitive to a green pedestal mixes
> colours.
> 
> Add the demux + BLS block ahead of demosaic as an opt in stage. It
> takes the CFA order from the sink format, subtracts a static black
> level and stretches the rest back to full scale. Its layout was worked
> out on the IFE 780 against CamX's demux + BLS 1.4.1 settings; the
> IFE 880 keeps it off until it is checked there.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: William Bright <william.bright@xxxxxxxxxxx>
> ---
>  drivers/media/platform/qcom/camss/camss-vfe-780.c  |  3 +-
>  .../media/platform/qcom/camss/camss-vfe-880-pp.c   | 65 +++++++++++++++++++++-
>  .../media/platform/qcom/camss/camss-vfe-880-pp.h   |  3 +-
>  3 files changed, 68 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-780.c b/drivers/media/platform/qcom/camss/camss-vfe-780.c
> index c2ffe39f929a..032786c4ed74 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-780.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-780.c
> @@ -247,7 +247,8 @@ static void vfe_line_prepare(struct vfe_line *line)
>  	struct vfe_device *vfe = to_vfe(line);
>  
>  	if (line->is_pix && pix->pixelformat == V4L2_PIX_FMT_NV12)
> -		vfe_880_setup_pixel_pipe(vfe, vfe_pp_desc(vfe), pix->width, pix->height);
> +		vfe_880_setup_pixel_pipe(vfe, vfe_pp_desc(vfe), pix->width, pix->height,
> +					 line->fmt[MSM_VFE_PAD_SINK].code);
>  }
>  
>  static const struct camss_video_ops vfe_video_ops_780 = {
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-880-pp.c b/drivers/media/platform/qcom/camss/camss-vfe-880-pp.c
> index f962016ef3c6..87e6a241ec05 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-880-pp.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-880-pp.c
> @@ -11,6 +11,7 @@
>  
>  #include <linux/io.h>
>  #include <linux/kernel.h>
> +#include <linux/media-bus-format.h>
>  
>  #include "camss.h"
>  #include "camss-vfe.h"
> @@ -18,6 +19,17 @@
>  
>  /* Register offsets, named as in the IFE register map */
>  #define IFE_TOP_CORE_CFG_0                                       0x00024
> +#define IFE_DEMUX_BLS_MODULE_CFG                                 0x03b60
> +#define		IFE_DEMUX_BLS_PERIOD(n)				((n) << 20)
> +#define IFE_DEMUX_BLS_EVEN_CFG                                   0x03b68
> +#define IFE_DEMUX_BLS_ODD_CFG                                    0x03b6c
> +#define IFE_DEMUX_BLS_GAIN_CFG_0                                 0x03b70
> +#define IFE_DEMUX_BLS_GAIN_CFG_1                                 0x03b74
> +#define IFE_DEMUX_BLS_SUB_G_EVEN                                 0x03b78
> +#define IFE_DEMUX_BLS_SUB_G_ODD                                  0x03b7c
> +#define IFE_DEMUX_BLS_SUB_B                                      0x03b80
> +#define IFE_DEMUX_BLS_SUB_R                                      0x03b84
> +#define		IFE_DEMUX_BLS_SUB(v)				((v) << 20)
>  #define IFE_WB_GAIN_MODULE_CFG                                   0x04560
>  #define IFE_WB_GAIN_WB_CFG_0                                     0x04568
>  #define IFE_WB_GAIN_WB_CFG_1                                     0x0456c
> @@ -313,6 +325,55 @@ static void vfe_setup_chroma_downscale_nv12(struct vfe_device *vfe, u32 w, u32 h
>  	writel_relaxed(0x00000000, vfe->base + IFE_DOWNSCALE_MN_C_VID_OUT_DOWNSCALE_MN_C_V_PHASE_CFG);
>  }
>  
> +/* 12 bit black level subtracted ahead of demosaic, 42 at 10 bit */
> +#define IFE_780_BLACK_LEVEL	168
> +
> +/* Demux + BLS: CFA order from the sink format, black level subtracted */
> +static void vfe_setup_demux_bls(struct vfe_device *vfe, u32 code)
> +{
> +	u32 even, odd, gain, bl = IFE_780_BLACK_LEVEL;
> +
> +	switch (code) {
> +	case MEDIA_BUS_FMT_SRGGB8_1X8:
> +	case MEDIA_BUS_FMT_SRGGB10_1X10:
> +	case MEDIA_BUS_FMT_SRGGB12_1X12:
> +	case MEDIA_BUS_FMT_SRGGB14_1X14:
> +		even = 0xc9;
> +		odd = 0xac;
> +		break;
> +	case MEDIA_BUS_FMT_SGBRG8_1X8:
> +	case MEDIA_BUS_FMT_SGBRG10_1X10:
> +	case MEDIA_BUS_FMT_SGBRG12_1X12:
> +	case MEDIA_BUS_FMT_SGBRG14_1X14:
> +		even = 0xca;
> +		odd = 0x9c;
> +		break;
> +	case MEDIA_BUS_FMT_SBGGR8_1X8:
> +	case MEDIA_BUS_FMT_SBGGR10_1X10:
> +	case MEDIA_BUS_FMT_SBGGR12_1X12:
> +	case MEDIA_BUS_FMT_SBGGR14_1X14:
> +		even = 0xac;
> +		odd = 0xc9;
> +		break;
> +	default:
> +		even = 0x9c;
> +		odd = 0xca;
> +		break;
> +	}
> +
> +	gain = DIV_ROUND_CLOSEST(0x400 * 4095, 4095 - bl);
> +
> +	writel_relaxed(even, vfe->base + IFE_DEMUX_BLS_EVEN_CFG);
> +	writel_relaxed(odd, vfe->base + IFE_DEMUX_BLS_ODD_CFG);
> +	writel_relaxed(gain << 16 | gain, vfe->base + IFE_DEMUX_BLS_GAIN_CFG_0);
> +	writel_relaxed(gain << 16 | gain, vfe->base + IFE_DEMUX_BLS_GAIN_CFG_1);
> +	writel_relaxed(IFE_DEMUX_BLS_SUB(bl), vfe->base + IFE_DEMUX_BLS_SUB_G_EVEN);
> +	writel_relaxed(IFE_DEMUX_BLS_SUB(bl), vfe->base + IFE_DEMUX_BLS_SUB_G_ODD);
> +	writel_relaxed(IFE_DEMUX_BLS_SUB(bl), vfe->base + IFE_DEMUX_BLS_SUB_B);
> +	writel_relaxed(IFE_DEMUX_BLS_SUB(bl), vfe->base + IFE_DEMUX_BLS_SUB_R);
> +	writel_relaxed(IFE_DEMUX_BLS_PERIOD(1) | 1, vfe->base + IFE_DEMUX_BLS_MODULE_CFG);
> +}

This looks pretty close to correct. I might cherry-pick this and fold on top of V1.
---
bod