On Mon, Jun 01, 2020 at 05:46:26PM +0530, Kaaira Gupta wrote:
> On Fri, May 29, 2020 at 05:43:57PM +0200, Dafna Hirschfeld wrote:
> > Hi,
> > Thanks for the patch
> >
> > I don't know how real devices handle ARGB formats,
> > I wonder if it should be the part of the debayer.
>
> Hi! qcam tries to support BA24 as it is one of the formats that vimc
> lists as its supported formats wih --list-formats. Shouldn't BA24 be
> possible to capture with vimc?
>
> If yes, which entity should support it, if not debayer? Should there be
> a separate conversion entity, or should we keep the support in debayer
> itself for efficiency issues?
At the hardware level, the de-bayering block usually produces RGB with 8
or more bits per colour components (so 3xn, 24 bits for 8-bit depths).
The conversion to 32-bit ARGB usually happens at the DMA engine level,
in the formatter right in front of the DMA engine. Ideally the vimc
pipeline should expose the same.
>From a performance point of view, it makes little sense to process the
image in vimc through multiple steps. I think it would be best to
generate the final image directly at the output of the pipeline.
> > On 28.05.20 20:57, Kaaira Gupta wrote:
> > > Running qcam for pixelformat 0x34324142 showed that vimc debayer does
> > > not support it. Hence, add the support for Alpha (255).
> >
> > I would change the commit log to:
> >
> > Add support for V4L2_PIX_FMT_RGB24 format in the debayer
> > and set the alpha channel to constant 255.
> >
> > > Signed-off-by: Kaaira Gupta <kgupta@xxxxxxxxxxxxx>
> > > ---
> > > .../media/test-drivers/vimc/vimc-debayer.c | 27 ++++++++++++-------
> > > 1 file changed, 18 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/media/test-drivers/vimc/vimc-debayer.c b/drivers/media/test-drivers/vimc/vimc-debayer.c
> > > index c3f6fef34f68..f34148717a40 100644
> > > --- a/drivers/media/test-drivers/vimc/vimc-debayer.c
> > > +++ b/drivers/media/test-drivers/vimc/vimc-debayer.c
> > > @@ -62,6 +62,7 @@ static const u32 vimc_deb_src_mbus_codes[] = {
> > > MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> > > MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA,
> > > MEDIA_BUS_FMT_RGB888_1X32_PADHI,
> > > + MEDIA_BUS_FMT_ARGB8888_1X32
> > > };
> > > static const struct vimc_deb_pix_map vimc_deb_pix_map_list[] = {
> > > @@ -322,15 +323,23 @@ static void vimc_deb_process_rgb_frame(struct vimc_deb_device *vdeb,
> > > unsigned int i, index;
> > > vpix = vimc_pix_map_by_code(vdeb->src_code);
> > > - index = VIMC_FRAME_INDEX(lin, col, vdeb->sink_fmt.width, 3);
> > > - for (i = 0; i < 3; i++) {
> > > - switch (vpix->pixelformat) {
> > > - case V4L2_PIX_FMT_RGB24:
> > > - vdeb->src_frame[index + i] = rgb[i];
> > > - break;
> > > - case V4L2_PIX_FMT_BGR24:
> > > - vdeb->src_frame[index + i] = rgb[2 - i];
> > > - break;
> > > +
> > > + if (vpix->pixelformat == V4L2_PIX_FMT_ARGB32) {
> > > + index = VIMC_FRAME_INDEX(lin, col, vdeb->sink_fmt.width, 4);
> > > + vdeb->src_frame[index] = 255;
> > > + for (i = 0; i < 3; i++)
> > > + vdeb->src_frame[index + i + 1] = rgb[i];
> > > + } else {
> > > + index = VIMC_FRAME_INDEX(lin, col, vdeb->sink_fmt.width, 3);
> > > + for (i = 0; i < 3; i++) {
> > > + switch (vpix->pixelformat) {
> > > + case V4L2_PIX_FMT_RGB24:
> > > + vdeb->src_frame[index + i] = rgb[i];
> > > + break;
> > > + case V4L2_PIX_FMT_BGR24:
> > > + vdeb->src_frame[index + i] = rgb[2 - i];
> > > + break;
> > > + }
> > > }
> > > }
> > > }
--
Regards,
Laurent Pinchart
