Re: [PATCH 4/6] media: qcom: camss: vfe: Add support for VFE 900

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] media: camss: incorrect FRAME_INCR address shift for VFE 900
--

commit dd5f8f2cb07db9a03dde1f94e9ccbfb47aa55597
Author: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>

media: qcom: camss: vfe: Add support for VFE 900

This commit introduces support for the VFE 900 hardware variant on the Nord
platform. It adds a new configuration structure to handle register base
differences and updates the image address shift in vfe_wm_update().

> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-gen4.c b/drivers/media/platform/qcom/camss/camss-vfe-gen4.c
> index d73d70898710a..f1429a6d2be54 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-gen4.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-gen4.c

[ ... ]

> @@ -83,18 +147,36 @@
>   * RDI3			3
>   * GAMMA		4
>   * STATES_BE		5
> + *
> + * v900 full IFE write master client map: the RDI clients start at 28,
> + * after the 28 image/stats clients (0-27):
> + *
> + * MAIN C0/C1/C2/UV		0-3
> + * PIXEL_RAW			4
> + * W_IR				5
> + * AI_1 C0/C1/C2/UV		6-9
> + * AI_2 C0/C1/C2/UV		10-13
> + * HV_DS16 / HV_DS4		14-15
> + * STATS BG_IR..BLTM		16-27
> + * RDI0				28
> + * RDI1				29
> + * RDI2				30
> + * ... RDI11			39
> + *
> + * v900 IFE Lite matches the generic map above (RDI0 at client 0).
>   */
> -#define RDI_WM(n) ((vfe_is_lite(vfe) ? 0x0 : 0x17) + (n))
>  
>  static void vfe_wm_start(struct vfe_device *vfe, u8 wm, struct vfe_line *line)
>  {
>  	struct v4l2_pix_format_mplane *pix =
>  		&line->video_out.active_fmt.fmt.pix_mp;
>  
> -	wm = RDI_WM(wm);
> +	wm = vfe_gen4_rdi_wm(vfe, wm);
>  
>  	/* no clock gating at bus input */
> -	writel(WM_CGC_OVERRIDE_ALL, vfe->base + VFE_BUS_WM_CGC_OVERRIDE);
> +	if (vfe_gen4_get_hw_info(vfe)->has_wm_cgc_override)
> +		writel(WM_CGC_OVERRIDE_ALL,
> +		       vfe->base + VFE_BUS_WM_CGC_OVERRIDE);
>  
>  	writel(0x0, vfe->base + VFE_BUS_WM_TEST_BUS_CTRL);
>  

[Severity: High]
Should VFE_BUS_WM_FRAME_INCR also be updated to use the new addr_shift?

Just below this in vfe_wm_start(), the frame increment is still
hardcoded to shift by 8:

	writel(ALIGN(pix->plane_fmt[0].bytesperline, 16) * pix->height >> 8,
	       vfe->base + VFE_BUS_WM_FRAME_INCR(wm));

Since VFE 900 uses an addr_shift of 0, leaving this hardcoded shift means
the frame increment will be 256 times smaller than required. During
multi-frame captures on VFE 900, will subsequent frames be written to
incorrect offsets and overwrite previous frame data?

[ ... ]

> @@ -123,15 +205,19 @@ static void vfe_wm_start(struct vfe_device *vfe, u8 wm, struct vfe_line *line)
>  
>  static void vfe_wm_stop(struct vfe_device *vfe, u8 wm)
>  {
> -	wm = RDI_WM(wm);
> +	wm = vfe_gen4_rdi_wm(vfe, wm);
>  	writel(0, vfe->base + VFE_BUS_WM_CFG(wm));
>  }
>  
>  static void vfe_wm_update(struct vfe_device *vfe, u8 wm, u32 addr,
>  			  struct vfe_line *line)
>  {
> -	wm = RDI_WM(wm);
> -	writel(addr >> 8, vfe->base + VFE_BUS_WM_IMAGE_ADDR(wm));
> +	const struct vfe_gen4_hw_info *hw_info = vfe_gen4_get_hw_info(vfe);
> +
> +	wm = vfe_gen4_rdi_wm(vfe, wm);
> +
> +	writel(addr >> hw_info->addr_shift,
> +	       vfe->base + VFE_BUS_WM_IMAGE_ADDR(wm));
>  
>  	dev_dbg(vfe->camss->dev, "wm:%d, image buf addr:0x%x\n", wm, addr);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-nord-v1-0-20db41a7f1ff@oss.qualcomm.com?part=4