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

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

[Sergey Lebedev]

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@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sergey Lebedev <lsa.uz@pm.me>
---
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@gmail.com/

 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

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@vger.kernel.org
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Sergey Lebedev <lsa.uz@pm.me>
> ---
> 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@gmail.com/
> 
>  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