Re: [PATCH v4 00/15] media: rcar: Streams support

[Niklas Söderlund]

Hi Tomi,

On 2026-01-27 10:59:21 +0200, Tomi Valkeinen wrote:
> Hi,
> 
> On 26/01/2026 21:39, Niklas Söderlund wrote:
> > Hi Tomi,
> > 
> > Thanks for digging in this.
> > 
> > On 2026-01-26 16:01:36 +0200, Tomi Valkeinen wrote:
> >> Hi,
> >>
> >> On 31/12/2025 11:57, Niklas Söderlund wrote:
> >>> Hi Tomi,
> >>>
> >>> Thanks for your persistent work on this series!
> >>>
> >>> On 2025-12-16 17:18:17 +0200, Tomi Valkeinen wrote:
> >>>> Add streams support to Renesas rcar platform driver.
> >>>>
> >>>> The series keaps compatibility with the current upstream for a single
> >>>> stream use case. However, in upstream there's a limited custom
> >>>> multi-stream support implemented to the rcar driver, which will be
> >>>> replaced with the upstream's Streams API.
> >>>>
> >>>> I have tested this series on Sparrow-Hawk board, with a few different
> >>>> setups:
> >>>>
> >>>> IMX219 connected to the CSI0 connector
> >>>> - The following patches applied to my test branch in addition to this
> >>>>   series:
> >>>>   1) The v4l2_subdev_get_frame_desc_passthrough dependency
> >>>>   2) Revert of commit e7376745ad5c8548e31d9ea58adfb5a847e017a4 ("media:
> >>>>      rcar-vin: Fix stride setting for RAW8 formats"), as that commit
> >>>>      breaks RAW8
> >>>
> >>> That is so odd, I do grab RAW8 on V4H with a IMX219. In what way is do 
> >>> you see RAW8 breaking?
> >> I think I found it. It's broken for all formats and resolutions, based
> >> on luck:
> >>
> >> VNIS_REG has the lowest 4 bits always 0. From the doc: "These bits
> >> specify the width of the transfer destination memory in 16-pixel
> >> unit."
> >>
> >> We do nothing to comply with that.
> > 
> > Yes we do, but maybe not enough?
> > 
> > In rvin_format_bytesperline() we align for this, we even consider the 
> > special cases for NV12 and NV16.
> > 
> > The value written to VNIS_REG is vin->format.bytesperline / fmt->bpp, 
> > and the value writen to vin->format.bytesperline is ALIGN(pix->width, 
> > align) * fmt->bpp. And for all formats (not NV12 or NV16) is 0x10, so we 
> > do align it to the 16-pixel unit no?
> > 
> > Maybe their is a corner case I have missed or maybe I'm missing some 
> > other angle? And I agree adding and removing the fmt->bpp multiplier is 
> > not the best here. As we have finally moved this driver to media graph 
> > only there are lots of areas things can be cleaned up and improved as we 
> > no longer need to consider all that Gen2 special cases. I bet this can 
> > likely be cleaned up.
> Right, but if we have the stride / 2 code in, we will get strides not
> aligned to 16 bytes unless the bytesperline happened to be aligned to 32
> bytes.
> 
> I sent "[PATCH] media: renesas: vin: Fix RAW8 (again)". I still don't
> understand exactly how the RAW8 processing goes in the hardware, but
> afaics the stride / 2 is required, I cannot get any proper images
> without that. The documentation doesn't really describe it so I'm not
> totally content with all this.
> 
> Anyway, please test the new patch on your end, perhaps we'll finally
> have a conclusion on the RAW8 =).

Reading the comments in that patch and the documentation and I now see 
the other way the docs could be interpreted :-) I agree that if it works 
(I'm sure it does) it also satisfy the documentation. I will give it a 
spin and reply in that patch. It would indeed be super to solve this 
correctly, thanks for your efforts!

> 
>  Tomi
> 

-- 
Kind Regards,
Niklas Söderlund