Hi Keke,
let me ask Sakari's and Laurent's opinion on this specific
issue
On Mon, Nov 11, 2024 at 11:00:07AM +0800, Keke Li wrote:
> Hi Jacopo
>
> Thanks very much for your reply.
>
> On 2024/11/8 22:04, Jacopo Mondi wrote:
> > [ EXTERNAL EMAIL ]
> >
> > Hi Keke
> >
> > On Fri, Nov 08, 2024 at 08:34:41PM +0800, Keke Li wrote:
> > > Hi Jacopo
> > >
> > > Thanks very much for your reply.
> > >
> > > On 2024/11/8 00:03, Jacopo Mondi wrote:
> > > > [ EXTERNAL EMAIL ]
> > > >
> > > > Hi Keke
> > > >
> > > > a first pass of review without going into details about the
> > > > ISP parameters and stats but mostly on architecture.
> > > >
> > > > On Wed, Sep 18, 2024 at 02:07:18PM +0800, Keke Li via B4 Relay wrote:
> > > > > From: Keke Li <keke.li@xxxxxxxxxxx>
> > > > >
> > > > > The C3 ISP supports multi-camera and muti-exposure
> > > > > high dynamic range (HDR). It brings together some
> > > > > advanced imaging technologies to provide good image quality.
> > > > > This driver mainly responsible for driving ISP pipeline
> > > > > to process raw image.
> > > > >
> > > > > Signed-off-by: Keke Li <keke.li@xxxxxxxxxxx>
> > > > > ---
> > > > > drivers/media/platform/amlogic/Kconfig | 1 +
[snip]
> > > > > > +++ b/drivers/media/platform/amlogic/c3-isp/c3-isp-capture.c
> > > > > @@ -0,0 +1,759 @@
> > > > > +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
> > > > > +/*
> > > > > + * Copyright (C) 2024 Amlogic, Inc. All rights reserved
> > > > > + */
> > > > > +
> > > > > +#include <linux/cleanup.h>
> > > > > +#include <linux/pm_runtime.h>
> > > > > +
> > > > > +#include <media/v4l2-ctrls.h>
> > > > > +#include <media/v4l2-event.h>
> > > > > +#include <media/v4l2-ioctl.h>
> > > > > +#include <media/v4l2-mc.h>
> > > > > +#include <media/videobuf2-dma-contig.h>
> > > > > +
> > > > > +#include "c3-isp-common.h"
> > > > > +#include "c3-isp-regs.h"
> > > > > +
> > > > > +static const struct c3_isp_capture_format cap_formats[] = {
> > > > > + {
> > > > > + .mbus_code = MEDIA_BUS_FMT_Y8_1X8,
> > > > > + .fourcc = V4L2_PIX_FMT_GREY,
> > > > > + .depth = 8,
> > > > > + },
> > > > > + {
> > > > > + .mbus_code = MEDIA_BUS_FMT_YUYV8_1_5X8,
> > > > Does the 5X8 version represents the format on the internal bus between
> > > > the resizers and the capture device ?
> > > >
> > > > How does format propagation work from the ISP to the resizers and the
> > > > capture devices ? I mean, is there an internal bus where the number of
> > > > samples (5X8 vs 2X8) changes depending on the output format ?
> > > >
> > > There is no internal bus between the resizer and the capture device.
> > >
> > I presume there is at least a data path, maybe as internal memory
> > buffers, between the different components of the ISP though.
>
>
> Yes, there is a data path between resizer and capture node.
>
> > > The output format should be only configured in capture device.
> > >
> > Ok, what I'm after here is finding out the reason why you have
> > different mbus codes associated with different output formats. Same
> > thing as per the below question about the mbus codes used between the
> > ISP and the resizers.
> >
> > The mbus code should describe the format as it actually is in the data
> > path between the different ISP components. In example, if your
> > demosaicing block outputs data in a specific format with a specific
> > bit-depth (in example RGB data with a 30 bits wide bus) you should use
> > MEDIA_BUS_FMT_RGB101010_1X30.
> >
> > However, if this information is not available, or not relevant as it
> > doesn't influence the behaviour of any of the ISP blocks, I think
> > using MEDIA_BUS_FMT_FIXED should be fine.
> >
> > Otherwise, if there are reasons to use MEDIA_BUS_FMT_YUYV8_1_5X8 over
> > MEDIA_BUS_FMT_YUYV8_2X8 it's fine, but please explain them :)
> >
> > Thanks
> > j
>
>
> I have checked the ISP datasheet, there is no need to use mbus code.
>
> So decided to use MEDIA_BUS_FMT_FIXED.
>
> > > > > + .fourcc = V4L2_PIX_FMT_NV12,
> > > > > + .depth = 12,
> > > > > + }, {
> > > > > + .mbus_code = MEDIA_BUS_FMT_YUYV8_1_5X8,
> > > > > + .fourcc = V4L2_PIX_FMT_NV21,
> > > > > + .depth = 12,
> > > > > + }, {
> > > > > + .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8,
> > > > > + .fourcc = V4L2_PIX_FMT_NV16,
> > > > > + .depth = 16,
> > > > > + }, {
> > > > > + .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8,
> > > > > + .fourcc = V4L2_PIX_FMT_NV61,
> > > > > + .depth = 16,
> > > > > + },
> > > > > +};
