[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

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

[German Pablo Lindo]

As Sergey Lebedev, i experienced the same error in the same hardware regarding the IR camera, for some reason, the image displayed had horizontal lines from the previous frame, ruining the image

I have tested this patch and i havent seen the error again in qcam, i have also tested the patch on howdy (wich after the needed configuration and tweaking, now seems more precise or at least more responsive in detecting the user even in positions different from the original one configured) and if it had affected the other IPU7 cameras of the device (The other two cameras stayed as previous versions without the patch)

Reported-by: German Pablo Lindo <germanpapulindez@gmail.com>
Tested-by: German Pablo Lindo <germanpapulindez@gmail.com>

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

[Sergey Lebedev]

Hi Manik, Divyamani,

Sakari added you to this patch in October. Is is_snoop = 1 on the
capture pin right for IPU7, and for IPU 7.5, which I could not test?

Christian Murphy sent a different fix for the same rows in August,
which I missed: a clflush of each capture buffer in buf_prepare and
buf_finish, with is_snoop left at 0. If the snoop bit cannot be relied
on, his is the one to take:
https://lore.kernel.org/all/20260824224139.21256-1-christian@themurphys.eu/

With is_snoop = 1, frames holding rows of the buffer's previous fill
went from 301 of 698 and 684 of 697 to none, reading every frame with
the CPU, and 4224x3136 raw at about 790 MB/s still ran at 29.95 fps.
The patch applies to v7.4-rc1 unchanged.

It carries an Assisted-by tag, which 7.4-rc1 no longer accepts; Sakari,
please drop it when applying, or I will resend without it.

Sergey