Re: [PATCH 3/8] media: qcom: camss: vfe-17x: do not gate write master done on IRQ_STATUS_0

[Bryan O'Donoghue]

On 14/09/2026 14:34, Hitesh Patel wrote:
> The VFE 17x interrupt handler reads and clears IRQ_STATUS_0/1 and
> then reads and clears every BUS_IRQ_STATUS register, unconditionally.
> It only acts on the per write master WM_CLIENT_BUF_DONE bits from
> BUS_IRQ_STATUS(1), however, when bit 9 of the previously sampled
> IRQ_STATUS_0 was set. That bit is the ping-pong flag of image master
> 1, which says nothing about the other masters.

This paragraph is disjointed, places a full stop where there was going 
to be a comma ? "IRQ_STATUS_0 was set. That bit is the"
> Those two reads are not atomic either. A buffer done that becomes
> pending after IRQ_STATUS_0 has been sampled but before
> BUS_IRQ_STATUS(1) is read, for example while the handler is entered
> for another line's RDI SOF or REG_UPDATE, is cleared by the bus
> status read and then dropped by the gate: wm_done() is never called
> for it, the buffer is never returned to userspace and that line
> stalls until the next frame happens to line up with the gate again.
>
> With a single RDI streaming the window is rarely hit. As soon as a
> second RDI of the same VFE streams, which is the case when a CSID
> demultiplexes two virtual channels to RDI0 and RDI1, the interrupt
> rate doubles and one of the two lines loses buffer done events
> continuously.
>
> The bus status is read-to-clear in this handler, so once read it is
> the authoritative record of which write masters completed. Act on it
> directly, as the gen2 VFE handler does, and drop the gate.
>
> The write master of a PIX line is skipped: its buffers are completed
> from the composite done above, through vfe_isr_comp_done(), and
> completing them here as well would return two buffers per frame.

Please rewrite this whole commit log in your own language.

Also since your are describing a bug this needs

1. Fixes:
2. A patch title starting with Fix
3. All fixes in a serious should prefix functionality changes.

> Signed-off-by: Hitesh Patel <hitesh@xxxxxxxxxxxxxx>
> ---
>   drivers/media/platform/qcom/camss/camss-vfe-17x.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-17x.c b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> index e5ee7e717..f62fdabbc 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> @@ -363,10 +363,13 @@ static irqreturn_t vfe_isr(int irq, void *dev)
>   		if (vfe_bus_status[0] & STATUS0_COMP_BUF_DONE(i))
>   			vfe->isr_ops.comp_done(vfe, i);
>   
> -	for (wm = 0; wm < MSM_VFE_IMAGE_MASTERS_NUM; wm++)
> -		if (status0 & BIT(9))
> -			if (vfe_bus_status[1] & STATUS1_WM_CLIENT_BUF_DONE(wm))
> -				vfe->isr_ops.wm_done(vfe, wm);
> +	for (wm = 0; wm < MSM_VFE_IMAGE_MASTERS_NUM; wm++) {
> +		if (vfe->wm_output_map[wm] == VFE_LINE_PIX)
> +			continue;

Drop this workaround for pix. We will fix PIX a different way instead of 
for local specific stuff like this.

> +
> +		if (vfe_bus_status[1] & STATUS1_WM_CLIENT_BUF_DONE(wm))
> +			vfe->isr_ops.wm_done(vfe, wm);
> +	}

OK then this becomes a one-liner to not disjunction BIT(9)

>   	return IRQ_HANDLED;
>   }