Re: [PATCH] media: staging/ipu7: snoop ISYS writes to capture buffers

[Sakari Ailus]

Cc Manik and Divyamani.

On Wed, Oct 07, 2026 at 06:33:34PM +0000, Sergey Lebedev wrote:
> The driver sets is_snoop = 0 on the ISYS output pin, so the ISYS writes
> capture buffers without snooping the CPU caches. vb2 syncs each buffer
> for the CPU, but on x86 the DMA API takes the device to be coherent and
> that sync does nothing. A CPU that read a buffer before then reads cache
> lines of that buffer's earlier frame, which show as horizontal streaks
> wherever the scene moves.
> 
> Set is_snoop on the capture pin, not only for metadata as the TODO had
> it: the CPU reads pixel data too. IPU6 sets snoopable on its pins.
> 
> Tested on a Surface Pro 11 (Lunar Lake IPU7), reading every frame with
> the CPU. With a VD55G0 at 644x604 and 320x240 in 8-bit mono, frames
> holding rows of the buffer's previous fill went from 301 of 698 and 684
> of 697 to none. With an OV13858 in raw at 4224x3136, about 790 MB/s, 300
> frames came at 29.95 fps without a gap either way.
> 
> Fixes: a516d36bdc3d ("media: staging/ipu7: add IPU7 input system device driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Sergey Lebedev <lsa.uz@xxxxx>
> ---
> Tested with the bit as a module parameter, both values in one boot, on
> this driver plus Ruslan Koreev's monochrome formats [1], which the
> VD55G0 needs. No row ever equalled the same row of the frame before it.
> A clflush of each buffer before reading, later than any buf_finish could
> run, still left 61 and 24 such rows of 101352 and 46389 (another boot);
> with is_snoop set, none. At 4224x3136 no row was stale even without
> snoop: four 26 MB buffers do not stay in the cache. Not tested on IPU 7.5.
> 
> The ipu6 driver's IPU7 path, drivers/media/pci/intel/ipu6/ipu7-fw-isys.c,
> also used for IPU8, memsets the pin and leaves is_snoop at 0 too.
> Released kernels drive IPU7 only with this driver, hence the stable tag.
> I can send the same change for the ipu6 path, untested here.
> 
> [1] https://lore.kernel.org/linux-media/20260924171820.1179823-5-koreev.r@xxxxxxxxx/
> 
>  drivers/staging/media/ipu7/ipu7-isys-video.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/ipu7/ipu7-isys-video.c b/drivers/staging/media/ipu7/ipu7-isys-video.c
> index 8c6730833f2..13968caf64d 100644
> --- a/drivers/staging/media/ipu7/ipu7-isys-video.c
> +++ b/drivers/staging/media/ipu7/ipu7-isys-video.c
> @@ -407,8 +407,7 @@ static int ipu7_isys_fw_pin_cfg(struct ipu7_isys_video *av,
>  	output_pin->link.pbk_slot_id = IPU_MSG_LINK_PBK_SLOT_ID_DONT_CARE;
>  	output_pin->link.dest = IPU_INSYS_OUTPUT_LINK_DEST_MEM;
>  	output_pin->link.use_sw_managed = 1;
> -	/* TODO: set the snoop bit for metadata capture */
> -	output_pin->link.is_snoop = 0;
> +	output_pin->link.is_snoop = 1;
>  
>  	/* output pin crop */
>  	output_pin->crop.line_top = 0;
> -- 
> 2.54.0
> 
> 

-- 
Sakari Ailus