linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Marco Felsch <m.felsch@pengutronix.de>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: p.zabel@pengutronix.de, mchehab@kernel.org,
	slongerbeam@gmail.com, hverkuil-cisco@xs4all.nl,
	sakari.ailus@linux.intel.com,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org, kernel@pengutronix.de
Subject: Re: [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats
Date: Fri, 30 Apr 2021 08:51:34 +0200	[thread overview]
Message-ID: <20210430065134.jwscxlv3sydo4zgy@pengutronix.de> (raw)
In-Reply-To: <YIsvyz49KvZK6Wg5@pendragon.ideasonboard.com>

Hi Laurent,

On 21-04-30 01:14, Laurent Pinchart wrote:
> Hi Marco,
> 
> On Thu, Apr 29, 2021 at 09:49:03AM +0200, Marco Felsch wrote:
> > On 21-04-29 04:51, Laurent Pinchart wrote:
> > > On Tue, Apr 27, 2021 at 02:06:56PM +0200, Marco Felsch wrote:
> > > > Add special 8/12bit bayer media bus format for the OnSemi AR0237IR
> > > > camera sensor [1]. OnSemi calls this format RGB-IR, the pixel array
> > > > with the interleaved IR pixels looks like:
> > > > 
> > > >         |  G |  R |  G |  B | ...
> > > >         +----+----+----+----+---
> > > >         | IR |  G | IR |  G | ...
> > > >         +----+----+----+----+---
> > > >         |  G |  B |  G |  R | ...
> > > >         +----+----+----+----+---
> > > >         | IR |  G | IR |  G | ...
> > > >         +----+----+----+----+---
> > > >         | .. | .. | .. | .. | ..
> > > > 
> > > > [1] https://www.framos.com/media/pdf/96/ac/8f/AR0237CS-D-PDF-framos.pdf
> > > 
> > > I think we're reaching a limit of the media bus codes model here, due to
> > > a historical mistake. The four possible Bayer patterns, times the
> > > different number of bits per pixel, creates a lot of media bus codes,
> > > and drivers for CSI-2 receivers and IP cores further down the pipeline
> > > have to support them all.
> > 
> > That's correct but it is not bayer related. Currently it is what it is,
> > if a new code is added it must be propagated through all the involved
> > subdevs. On the other hand I wouldn't say that it is better to support
> > new codes per default for all drivers. Since this would add a lot of
> > untested code paths.
> 
> It's not an issue limited to Bayer patterns, but they make the issue
> worse given the number of possible combinations (think about adding DPCM
> and ALAW compression on top of that...).

You're right and again this will apply to all mbus formats...

> > > This is already painful, and if we had a
> > > non-Bayer pattern such as this one,
> > 
> > That's not exactly true since it is a bayer pattern but instead of using
> > 4x4 it uses 8x8 and it as some special pixels.
> > 
> > > we'll open the door to an explosion
> > > of the number of media bus codes (imagine all the different possible
> > > arrangements of this pattern, for instance if you enable horizontal
> > > and/or vertical flipping on the sensor). All drivers would need to be
> > > updated to support these new bus codes, and this really kills the
> > > current model.
> > 
> > Yep, I know what you mean but as I said above I think that adding it
> > explicite is the better abbroach since it involves somone who add _and_
> > test the new code on the particular platform.
> > 
> > > The historical mistake was to tie the Bayer pattern with the media bus
> > > code. We should really have specified raw 8/10/12/14/16 media bus codes,
> > > and conveyed the pattern separately. Most IP cores in the pipeline don't
> > > need to care about the pattern at all, and those who do (that's mostly
> > > ISPs) could be programmed explicitly by userspace as long as we have an
> > > API to retrieve the pattern from the sensor. I believe it's time to bite
> > > the bullet and go in that direction. I'm sorry for this case of yak
> > > shaving, but it really wouldn't be manageable anymore :-(
> > 
> > I got all your points and would agree but this is not a bayer only
> > related problem. You will have this problem with all new other formats
> > as well. I'm with you, most IP cores don't care but I wouldn't
> > guarantee that.
> 
> Sorry, but adding new media bus formats like this one will just not
> scale. We have two options, either fixing the issue, or considering that
> V4L2 is a barely alive API with no future, and merging this without
> caring anymore.

Hm.. you're right that it doesn't scale, as I said I'm absolute on your
side. So let us consider a new approach @Mauro, @Hans, @Sailus what do
you think about?

BTW: IMHO videobuf2 interface isn't that good as well, since you are
blaming ;)

Regards,
  Marco

> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-04-30  6:53 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-27 12:06 [PATCH 0/6] Add new bayer ir formats Marco Felsch
2021-04-27 12:06 ` [PATCH 1/6] media: uapi: Add MEDIA_BUS_FMT_SGRGB_IGIG_GBGR_IGIG media bus formats Marco Felsch
2021-04-29  1:51   ` Laurent Pinchart
2021-04-29  7:49     ` Marco Felsch
2021-04-29  8:44       ` Mauro Carvalho Chehab
2021-04-29 16:53         ` Laurent Pinchart
2021-04-30 12:44           ` Mauro Carvalho Chehab
2021-04-29 22:14       ` Laurent Pinchart
2021-04-30  6:51         ` Marco Felsch [this message]
2021-04-30 12:18           ` Laurent Pinchart
2021-04-30 12:51             ` Mauro Carvalho Chehab
2021-04-30 12:58               ` Laurent Pinchart
2023-02-08 19:44                 ` Marco Felsch
2023-02-09  9:49                   ` Laurent Pinchart
2021-04-27 12:06 ` [PATCH 2/6] media: v4l: Add definition for bayered IR formats Marco Felsch
2021-04-29  1:45   ` Laurent Pinchart
2021-04-29  7:07     ` Marco Felsch
2021-04-27 12:06 ` [PATCH 3/6] media: v4l2-ioctl.c: add V4L2_PIX_FMT_SGRGB_IGIG_GBGR_IGIG to v4l_fill_fmtdesc Marco Felsch
2021-04-27 12:06 ` [PATCH 4/6] media: video-mux: add new SGRGB_IGIG_GBGR_IGIG format support Marco Felsch
2021-04-27 12:07 ` [PATCH 5/6] gpu: ipu-v3: add custom " Marco Felsch
2021-04-27 12:07 ` [PATCH 6/6] media: imx: csi: " Marco Felsch
2021-04-27 12:09 ` [PATCH 0/6] Add new bayer ir formats Marco Felsch
2022-11-24 14:49 ` Hans Verkuil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210430065134.jwscxlv3sydo4zgy@pengutronix.de \
    --to=m.felsch@pengutronix.de \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=kernel@pengutronix.de \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=sakari.ailus@linux.intel.com \
    --cc=slongerbeam@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).