Re: [PATCH 3/6] media: qcom: camss: vfe: Add support for VFE gen4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] media: camss: vfe: AUP_UPDATE hardware desync when queued buffers < 2
--

commit 6e9773967fcca20f990885a6e668a47fa226cbc5
Author: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>

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

This commit adds support for the VFE Gen4 module on the Kaanapali SoC. It 
limits early register updates for Kaanapali, as it requires updates to be 
issued only after all CSID configuration completes.

> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c

[ ... ]

> @@ -552,7 +553,9 @@ int vfe_enable_output_v2(struct vfe_line *line)
>  		output->gen2.active_num++;
>  		ops->vfe_wm_update(vfe, output->wm_idx[0],
>  				   output->buf[i]->addr[0], line);
> -		ops->reg_update(vfe, line->id);
> +
> +		if (!vfe->res->reg_update_after_csid_config)
> +			ops->reg_update(vfe, line->id);

[Severity: High]
Does this cause a hardware desynchronization if fewer than two buffers are
queued before stream-on?

Since min_queued_buffers is 0, V4L2 userspace can queue 0 or 1 buffers
before starting the stream. When vfe_enable_output_v2() skips the
reg_update() here, CSID blindly issues CAMSS_INIT_BUF_COUNT (2) AUP_UPDATEs
during csid_configure_stream().

If the remaining buffers are queued later, vfe_queue_buffer_v2() will issue
an additional reg_update():

drivers/media/platform/qcom/camss/camss-vfe.c:vfe_queue_buffer_v2() {
...
	ops->vfe_wm_update(vfe, output->wm_idx[0], output->buf[1]->addr[0], line);
	ops->reg_update(vfe, line->id);
...
}

This pushes the AUP_UPDATE count to 3 for 2 buffers, which corrupts the
hardware state.

Additionally, if 0 buffers were queued initially, vfe_queue_buffer_v2()
places the newly queued buffer unconditionally in output->buf[1] instead of
buf[0]. When the hardware completes the frame, vfe_buf_done() expects buf[0]
to be valid, leading to a missing ready buf error and leaking the buffer.

>  	}
>  
>  	spin_unlock_irqrestore(&vfe->output_lock, flags);

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