Hi Tomi,
On 2025-05-30 15:08:56 +0300, Tomi Valkeinen wrote:
> Hi,
>
> On 02/04/2025 21:33, Niklas Söderlund wrote:
> > Earlier versions of the datasheet where unclear about the stride setting
> > for RAW8 capture formats. Later datasheets clarifies that the stride
> > only process in this mode for non-image data. For image data the full
> > stride shall be used. Compare section "RAW: 8 Bits and Embedded 8-Bit
> > Non-Image Data, User Defined 8-bit Data" vs "RAW: 8 Bits".
>
> Looking at the V4H manual, I see:
>
> "
> RAW: 8 Bits and Embedded 8-Bit Non-Image Data, User Defined 8-bit Data
>
> The 8-bit RAW image data format, Embedded 8-bit non image data and User
> Defined 8-bit data format are shown below. Notes: 1. These data are
> transferred by 2-Byte unit.
> "
>
> and
>
> "
> RAW: 8 Bits (16 Bits/Pixel) Data
>
> The 8-bit RAW (16 bits/pixel) image data format is shown below.
> Notes: 1. These data are transferred by 2-Byte unit.
> "
>
> I don't see "RAW: 8 Bits". In both cases above the text says "2-byte unit".
I agree this is all priorly documented. My understanding and this fix
comes from the two diagrams attached to the two different sections you
point out.
In the first case "(12) RAW: 8 Bits and Embedded 8-Bit Non-Image Data,
User Defined 8-bit Data" the diagram (for even number of pixels) shows
two pixels being transferred in a 16-bit space, concatenated one after
the other.
In the second case "(21) RAW: 8 Bits (16 Bits/Pixel) Data" the diagram
shows one pixel being transferred in a 16-bit space, and the upper 8
bits being set to 0.
Without this fix the 8-bit raw frames I can grab from IMX219 and IMX462
on V4H are jumbled. I suspect maybe we need to teach the VIN driver to
support both modes as this have an effect on your setup. How do your
output images look with this fix?
What I can't figure out is what is different at the VIN stage. All RAW
8-bit media bus formats sets VnMC.INF to b100 (Embedded 8-bit non Image
Data or RAW8 input) which would suggest the first case is always used.
The only case where the second case could be used is for the RAW10
formats where VnMC.INF is set to b111 (RAW8 (16 bits/pixel) or RAW10 or
RAW12 or RAW14 or RAW16 or RGB565).
The only source I can think of is that our sensors somehow transmit
8-bit data differently, but I'm not sure how we can check that theory.
But if we can then we could use different media bus codes to address
both scenarios. What do you think?
>
> Tomi
>
> >
> > Remove the special case from pixel formats that carry image data and
> > treat it as any other image format.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > ---
> > .../media/platform/renesas/rcar-vin/rcar-dma.c | 16 ----------------
> > 1 file changed, 16 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> > index f8394be8a922..fdf0f86c801f 100644
> > --- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> > @@ -680,22 +680,6 @@ void rvin_crop_scale_comp(struct rvin_dev *vin)
> >
> > fmt = rvin_format_from_pixel(vin, vin->format.pixelformat);
> > stride = vin->format.bytesperline / fmt->bpp;
> > -
> > - /* For RAW8 format bpp is 1, but the hardware process RAW8
> > - * format in 2 pixel unit hence configure VNIS_REG as stride / 2.
> > - */
> > - switch (vin->format.pixelformat) {
> > - case V4L2_PIX_FMT_SBGGR8:
> > - case V4L2_PIX_FMT_SGBRG8:
> > - case V4L2_PIX_FMT_SGRBG8:
> > - case V4L2_PIX_FMT_SRGGB8:
> > - case V4L2_PIX_FMT_GREY:
> > - stride /= 2;
> > - break;
> > - default:
> > - break;
> > - }
> > -
> > rvin_write(vin, stride, VNIS_REG);
> > }
> >
>
--
Kind Regards,
Niklas Söderlund
