Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1610201 > unrolled thread

[PATCH v6 00/39] i.MX Media Driver

Back to article view | Back to linux.kernel

Contents

  [PATCH v6 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
    [PATCH v6 28/39] media: imx: csi: fix crop rectangle changes in set_fmt Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
    [PATCH v6 10/39] ARM: dts: imx6-sabreauto: create i2cmux for i2c3 Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
    [PATCH v6 37/39] media: imx-csi: add frame size/interval enumeration Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
    [PATCH v6 11/39] ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
    [PATCH v6 21/39] media: imx: Add CSI subdev driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
      [PATCH] [media] imx: csi: retain current field order and  colorimetry setting as default Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 16:00 +0200
        Re: [PATCH] [media] imx: csi: retain current field order and  colorimetry setting as default Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-04-06 16:10 +0200
          Re: [PATCH] [media] imx: csi: retain current field order and  colorimetry setting as default Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 17:10 +0200
            Re: [PATCH] [media] imx: csi: retain current field order and  colorimetry setting as default Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-04-06 17:20 +0200
              Re: [PATCH] [media] imx: csi: retain current field order and  colorimetry setting as default Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 17:30 +0200
        Re: [PATCH] [media] imx: csi: retain current field order and  colorimetry setting as default Hans Verkuil <hverkuil@xs4all.nl> - 2017-04-06 16:30 +0200
          Re: [PATCH] [media] imx: csi: retain current field order and  colorimetry setting as default Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-04-06 16:50 +0200
          Re: [PATCH] [media] imx: csi: retain current field order and  colorimetry setting as default Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 17:00 +0200
            Re: [PATCH] [media] imx: csi: retain current field order and  colorimetry setting as default Hans Verkuil <hverkuil@xs4all.nl> - 2017-04-06 17:50 +0200
              Re: [PATCH] [media] imx: csi: retain current field order and  colorimetry setting as default Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 18:10 +0200
          Re: [PATCH] [media] imx: csi: retain current field order and  colorimetry setting as default Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 17:20 +0200
    [PATCH v6 01/39] [media] dt-bindings: Add bindings for video-multiplexer device Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
      Re: [PATCH v6 01/39] [media] dt-bindings: Add bindings for  video-multiplexer device Rob Herring <robh@kernel.org> - 2017-04-03 16:20 +0200
    [PATCH v6 06/39] ARM: dts: imx6qdl: add capture-subsystem device Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
    [PATCH v6 25/39] ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
    [PATCH v6 05/39] ARM: dts: imx6qdl: Add mipi_ipu1/2 multiplexers, mipi_csi, and their connections Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 02:50 +0200
    [PATCH v6 04/39] ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
    [PATCH v6 14/39] add mux and video interface bridge entity functions Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
    [PATCH v6 15/39] [media] v4l2-mc: add a function to inherit controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
    [PATCH v6 08/39] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
    [PATCH v6 13/39] ARM: dts: imx6-sabreauto: add the ADV7180 video decoder Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
    [PATCH v6 03/39] [media] dt/bindings: Add bindings for OV5640 Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
      Re: [PATCH v6 03/39] [media] dt/bindings: Add bindings for OV5640 Rob Herring <robh@kernel.org> - 2017-04-03 16:20 +0200
    [PATCH v6 12/39] ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-28 03:00 +0200
    Re: [PATCH v6 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-30 13:10 +0200
      Re: [PATCH v6 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-30 18:20 +0200
        Re: [PATCH v6 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-30 18:30 +0200
    [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output  video on pad 1 Philipp Zabel <p.zabel@pengutronix.de> - 2017-03-30 19:30 +0200
      Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output  video on pad 1 Steve Longerbeam <slongerbeam@gmail.com> - 2017-04-05 00:20 +0200
        Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities  output video on pad 1 Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-05 11:50 +0200
      Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities  output video on pad 1 Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-04-05 01:20 +0200
        Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output  video on pad 1 Steve Longerbeam <slongerbeam@gmail.com> - 2017-04-05 02:50 +0200
          Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities  output video on pad 1 Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-04-05 10:30 +0200
            Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities  output video on pad 1 Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-05 11:40 +0200
              Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output  video on pad 1 Javier Martinez Canillas <javier@dowhile0.org> - 2017-04-05 16:00 +0200
              Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities  output video on pad 1 Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-04-05 17:00 +0200
                Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output  video on pad 1 Devin Heitmueller <dheitmueller@kernellabs.com> - 2017-04-05 17:50 +0200
                  Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities  output video on pad 1 Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-04-05 18:20 +0200
                    Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output  video on pad 1 Devin Heitmueller <dheitmueller@kernellabs.com> - 2017-04-05 19:10 +0200
                      Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities  output video on pad 1 Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-04-05 19:20 +0200
                Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities  output video on pad 1 Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 12:10 +0200
        Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output  video on pad 1 Steve Longerbeam <slongerbeam@gmail.com> - 2017-04-05 02:50 +0200
    Re: [PATCH v6 19/39] media: Add i.MX media core driver Pavel Machek <pavel@ucw.cz> - 2017-04-05 13:40 +0200
    Re: [PATCH v6 19/39] media: Add i.MX media core driver Pavel Machek <pavel@ucw.cz> - 2017-04-05 13:40 +0200
    Re: [PATCH v6 19/39] media: Add i.MX media core driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-04-06 11:50 +0200
      Re: [PATCH v6 19/39] media: Add i.MX media core driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-04-07 02:00 +0200

Page 3 of 3 — ← Prev page 1 2 [3]

#1616954 — Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1

Hello Philipp,

On Wed, Apr 5, 2017 at 5:34 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> On Wed, 2017-04-05 at 09:21 +0100, Russell King - ARM Linux wrote:

[snip]

> I think the output part is accurate, as the audio pad is an artifact of
> an unrelated change. I'm not so sure about the VBI pad, but I think that
> shouldn't exist either. The input pad, on the other hand, not having any
> of graph representation in the device tree seems a bit strange. There

Agreed, there should be a OF graph representation (and also a MC
representation) of the input PADs.

The tvp5150 driver currently has hardcoded as input TVP5150_COMPOSITE1
(AIP1B), so it won't work for a board that has the composite connector
wired to TVP5150_COMPOSITE0 (AIP1A) neither will work for S-Video
(AIP1A + AIP1B).

> was a custom binding for the inputs, that got quickly reverted:
> https://patchwork.kernel.org/patch/8395521/
>

Yes, that was my first attempt to have input connector support for
tvp5150. The patches were merged without a detailed review of the DT
bindings and latter were found to be wrong. Instead of having a driver
specific DT binding, a generic binding that could be used by any
device should be defined.

The latest proposal was [0] but that was also found to be inadequate.
After a lot of discussion on #v4l, the best approach we could come
with was something like like [1]. But I was busy with other stuff and
never found time to do the driver changes.

By the way, only the DT bindings portion from the first approach got
reverted but the patch reverting the driver changes never made to
mainline. Could you please test if [2] doesn't cause any issues to you
so the left over can be finally removed?

> regards
> Philipp
>

[0]: https://lkml.org/lkml/2016/4/12/983
[1]: https://hastebin.com/kadagidino.diff
[2]: https://patchwork.kernel.org/patch/9472623/

Best regards,
Javier

[toc] | [prev] | [next] | [standalone]

#1617021 — Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1

Em Wed, 05 Apr 2017 11:34:19 +0200
Philipp Zabel <p.zabel@pengutronix.de> escreveu:

> On Wed, 2017-04-05 at 09:21 +0100, Russell King - ARM Linux wrote:
> [...]
> > > Actually what was I thinking, the TVP5150 is already an example of
> > > such a device.
> > > 
> > > All of this could be solved if there was some direction information
> > > in port nodes.  
> > 
> > I disagree.
> >
> > Philipp identified that the TVP5150 has four pads:
> > 
> > * Input pad
> > * Video output pad
> > * VBI output pad
> > * Audio output pad  
> 
> I didn't think hard enough about this earlier, but there are really only
> two hardware interfaces on TVP5150. The ADC input, which can be
> connected to either of two composite input pins (AIP1A, AIP1B), or use
> both for s-video, and the digital output connected to pins (YOUT[7:0]).
> 
> VBI data can be transferred via the output pins during horizontal or
> vertical blanking, if I understand correctly, or read from a FIFO via
> I2C.
> 
> There is no apparent support for audio data whatsoever, and the only
> mention of audio in the data manual is a vague reference about an "audio
> interface available on the TVP5150" providing a clock to an audio
> interface between an external audio decoder and the backend processor.
> 
> Further, commit 55606310e77f ("[media] tvp5150: create the expected
> number of pads") creates DEMOD_NUM_PADS pads, but doesn't mention or
> initialize the audio pad. It clearly expects the value of DEMOD_NUM_PADS
> to be 3. And indeed the fourth pad was added later in commit
> bddc418787cc ("[media] au0828: use standard demod pads struct").
> 
> So to me it looks like the VBI and audio pads should be removed from
> TVP5150.

There are a number of drivers that can work with different
types of TV demodulators. Typical examples of such hardware can be
found at em28xx, saa7134, cx88 drivers (among lots of other drivers).
Those drivers don't use the subdev API. Instead, they use a generic
helper function with sets the pipelines, based on the pad number.

The problem here is that, currently, both MC API and MC core
lacks a way to identify PAD ports per type, as the only information
that a bridge driver has is a pad number. So, in order for a
generic helper function to work, we had to hardcode pad numbers,
in a way that it would work for all possible types of demods.

It shouldn't be hard to add a "pad_type" information at media_pad
struct, but passing such info to userspace requires a new API
(we're calling it as "properties API"). Sakari was meant to send
us an updated RFC for it[1] with a patchset, back in 2015, but
this never happened.

[1] https://linuxtv.org/news.php?entry=2015-08-17.mchehab

Each vendor chooses the demod using some random criteria 
(usually, they use whatever costs less by the time they create a
hardware model). Newer versions of the same hardware frequently
has a different model.

For example, in the case of em28xx driver, there are currently
several types of demods supported there, like (among other options):

- demods with tda9887, with is actually part of a tuner that has 
  an outputs for IF luminance, IF chrominance and IF audio;

- demods with xc3028, with is an integrated tuner + audio demod,
  with outputs both video as a baseband signal and audio data via IF,
  (or outputs a single IF signal, for Digital TV);

- demods with both xc3028 and msp3400. The last one transforms
  audio IF into I2S;

- demods with saa711x (supported by saa7115 driver), with may or may 
  not have raw VBI and/or sliced VBI, depending on the specific model,
  with is detected during driver's probe.

The generic function should be robust enough to handle all above
cases.

Without a way to report the PAD type to userspace, applications
that need to setup or recognize such pipelines need to hardcode the
pad numbers on userspace. That's, by the way, one of the issues why
it is currently impossible to write a full generic MC plugin at libv4l:
there's currently no way for userspace to recognize what type of
signals each PAD input or output carries.

So, in short, the tvp5150 demod doesn't decode audio, but there
are other demods that do it.

In the case of VBI, tvp5150 has actually two ways of reporting
it:

1) via YOUT[7:0] pins. VBI information is transmitted as a
   set of raw samples, via an ancillary data block, during
   vertical/horizontal blanking intervals. So, yes, it shares
   the same hardware output, although the VBI contents are
   actually multiplexed there. Please notice that not all
   video out PADS encapsulate raw VBI the same way as tvp5150
   (and some devices even don't support raw VBI, like saa7110 and
   some models supported by saa7115 driver).

2) via an interrupt that indicates that it decoded VBI data. The
   VBI information itself is there on FIFO, accessible via a set of
   registers (see "VBI Data processor" chapter at the datasheet).

Currently, the driver doesn't support (2), because, at the time
I wrote the driver, I didn't find a way to read the interrupts generated
by tvp5150 at em28xx[1], due to the lack of em28xx documentation,
but adding support for it shoudn't be hard. I may eventually do it
when I have some time to play with my ISEE hardware.

So, in the case of tvp5150 hardware, have those PADS:

- Input baseband;
- Video + raw VBI output;
- sliced VBI output.

Yet, we need an always unconnected audio output, in order to support
different demods out there.

[1] tvp5150 was written to support some em28xx-based devices

> 
> > So, it has one input and three outputs.  How does marking the direction
> > in the port node (which would indicate that there was a data flow out of
> > TVP5150 into the iMX6 capture) help identify which of those pads should
> > be used?
> >
> > It would eliminate the input pad, but you still have three output pads
> > to choose from.
> > 
> > So no, your idea can't work.  
> 
> In this case, removal of the VBI and audio pads might make this work,
> but in general this is true. In my opinion, to make this truly generic,
> we need an interface to ask the driver which media entity pad a given
> device tree port corresponds to, as there might not even be a single
> media entity corresponding to all ports for more complex devices.

Yes. We also need something like that at the userspace API.

Thanks,
Mauro

[toc] | [prev] | [next] | [standalone]

#1617076 — Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1

> Currently, the driver doesn't support (2), because, at the time
> I wrote the driver, I didn't find a way to read the interrupts generated
> by tvp5150 at em28xx[1], due to the lack of em28xx documentation,
> but adding support for it shoudn't be hard. I may eventually do it
> when I have some time to play with my ISEE hardware.

For what it's worth, I doubt most of the em28xx designs have the
tvp5150 interrupt request line connected in any way.  You would likely
have to poll the FIFO status register via I2C, or use the feature to
embed the sliced data into as VANC data in the 656 output (as
described in sec 3.9 of the tvp5150am1 spec).

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

[toc] | [prev] | [next] | [standalone]

#1617112 — Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1

Em Wed, 5 Apr 2017 11:39:06 -0400
Devin Heitmueller <dheitmueller@kernellabs.com> escreveu:

> > Currently, the driver doesn't support (2), because, at the time
> > I wrote the driver, I didn't find a way to read the interrupts generated
> > by tvp5150 at em28xx[1], due to the lack of em28xx documentation,
> > but adding support for it shoudn't be hard. I may eventually do it
> > when I have some time to play with my ISEE hardware.  
> 
> For what it's worth, I doubt most of the em28xx designs have the
> tvp5150 interrupt request line connected in any way.

True. But, on embedded hardware, such line may be connected into the
SoC. Actually, from the IGEPv3 expansion diagram:

	https://www.isee.biz/support/downloads/item/igepv2-expansion-rc-schematics

The INT line is connected to CAM_IRQ. That's connected to GPIO_154 pin
at OMAP3.

So, on a first glance, it seems possible to use it, instead of polling.

> You would likely
> have to poll the FIFO status register via I2C,

Yes, I considered this option when I wrote the driver. It could work, 
although it would likely have some performance drawback, as the driver
would need to poll it at least 60 times per second.

> or use the feature to
> embed the sliced data into as VANC data in the 656 output (as
> described in sec 3.9 of the tvp5150am1 spec).

True, but the bridge driver would need to handle such data. 

I remember I looked on this when I wrote the driver, but I was
unable to find a way for em28xx to parse (or forward) such
data packets.

Thanks,
Mauro

[toc] | [prev] | [next] | [standalone]

#1617171 — Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1

>> For what it's worth, I doubt most of the em28xx designs have the
>> tvp5150 interrupt request line connected in any way.
>
> True. But, on embedded hardware, such line may be connected into the
> SoC. Actually, from the IGEPv3 expansion diagram:
>
>         https://www.isee.biz/support/downloads/item/igepv2-expansion-rc-schematics
>
> The INT line is connected to CAM_IRQ. That's connected to GPIO_154 pin
> at OMAP3.
>
> So, on a first glance, it seems possible to use it, instead of polling.

To be clear, I wasn't suggesting that the IRQ request line on the
tvp5150 couldn't be supported in general (for example, for those
embedded targets which have it wired up to a host processor).  I'm
just saying you shouldn't expect it to work on most (perhaps all)
em28xx designs which have the tvp5150.  In fact on some em28xx designs
the pin is used as a GPIO output tied to a mux to control input
selection.  Hence blindly enabling the interrupt request line by
default would do all sorts of bad things.

>> You would likely
>> have to poll the FIFO status register via I2C,
>
> Yes, I considered this option when I wrote the driver. It could work,
> although it would likely have some performance drawback, as the driver
> would need to poll it at least 60 times per second.
>
>> or use the feature to
>> embed the sliced data into as VANC data in the 656 output (as
>> described in sec 3.9 of the tvp5150am1 spec).
>
> True, but the bridge driver would need to handle such data.

Correct.

> I remember I looked on this when I wrote the driver, but I was
> unable to find a way for em28xx to parse (or forward) such
> data packets.

I'm pretty sure it's possible, but I haven't looked at the datasheets
in a number of years and don't recall the details.

Hardware VBI splicing is supported by a number of decoders but it's
rarely used on commodity PCs (the Conexant and NXP decoders support it
as well).  That said, I won't argue there might be some value on
really low end platforms.  All I would ask is that if you do introduce
any such functionality into the tvp5150 driver for some embedded
application that you please not break support for devices such as the
em28xx.

Thanks,

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

[toc] | [prev] | [next] | [standalone]

#1617202 — Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1

Em Wed, 5 Apr 2017 13:02:52 -0400
Devin Heitmueller <dheitmueller@kernellabs.com> escreveu:

> > I remember I looked on this when I wrote the driver, but I was
> > unable to find a way for em28xx to parse (or forward) such
> > data packets.  
> 
> I'm pretty sure it's possible, but I haven't looked at the datasheets
> in a number of years and don't recall the details.
> 
> Hardware VBI splicing is supported by a number of decoders but it's
> rarely used on commodity PCs (the Conexant and NXP decoders support it
> as well).  That said, I won't argue there might be some value on
> really low end platforms.  All I would ask is that if you do introduce
> any such functionality into the tvp5150 driver for some embedded
> application that you please not break support for devices such as the
> em28xx.

Yeah, sure. If I write such patchset[1], it will be optional, and
will depend on a DT variable (or platform_data) setup that would
tell what GPIO pin should be used for interrupt.

Not providing it should either disable such feature or enable it
via polling.

[1] Please notice that I don't have any demand of doing it. Yet,
I may do it for fun on my spare time:-)

I added in the past an initial support for sliced VBI API on em28xx, 
with got reverted on changeset 1d179eeedc8cb48712bc236ec82ec6c63af42008, 
mainly due to the lack of such feature on tvp5150. So, such code was never
tested (and likely need fixes/updates), but if I end by adding sliced VBI
support on tvp5150 and on OMAP3, I may add support for it on em28xx too,
using I2C poll. On such case, I'll likely add a modprobe parameter to
enable such feature.

Thanks,
Mauro

[toc] | [prev] | [next] | [standalone]

#1617828 — Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1

On Wed, 2017-04-05 at 11:53 -0300, Mauro Carvalho Chehab wrote:
[...]
> There are a number of drivers that can work with different
> types of TV demodulators. Typical examples of such hardware can be
> found at em28xx, saa7134, cx88 drivers (among lots of other drivers).
> Those drivers don't use the subdev API. Instead, they use a generic
> helper function with sets the pipelines, based on the pad number.
> 
> The problem here is that, currently, both MC API and MC core
> lacks a way to identify PAD ports per type, as the only information
> that a bridge driver has is a pad number. So, in order for a
> generic helper function to work, we had to hardcode pad numbers,
> in a way that it would work for all possible types of demods.
> 
> It shouldn't be hard to add a "pad_type" information at media_pad
> struct, but passing such info to userspace requires a new API
> (we're calling it as "properties API"). Sakari was meant to send
> us an updated RFC for it[1] with a patchset, back in 2015, but
> this never happened.
> 
> [1] https://linuxtv.org/news.php?entry=2015-08-17.mchehab
[...]

That would be most useful.

> So, in short, the tvp5150 demod doesn't decode audio, but there
> are other demods that do it.
> 
> In the case of VBI, tvp5150 has actually two ways of reporting
> it:
> 
> 1) via YOUT[7:0] pins. VBI information is transmitted as a
>    set of raw samples, via an ancillary data block, during
>    vertical/horizontal blanking intervals. So, yes, it shares
>    the same hardware output, although the VBI contents are
>    actually multiplexed there. Please notice that not all
>    video out PADS encapsulate raw VBI the same way as tvp5150
>    (and some devices even don't support raw VBI, like saa7110 and
>    some models supported by saa7115 driver).

This is the physical interface that corresponds to the output port
(should be port@1) in the device tree. It should correspond to the video
output media entity pad.
What is unclear to is me whether the VBI media entity pad also should
correspond to the same physical interface / DT port.

> 2) via an interrupt that indicates that it decoded VBI data. The
>    VBI information itself is there on FIFO, accessible via a set of
>    registers (see "VBI Data processor" chapter at the datasheet).
>
> Currently, the driver doesn't support (2), because, at the time
> I wrote the driver, I didn't find a way to read the interrupts generated
> by tvp5150 at em28xx[1], due to the lack of em28xx documentation,
> but adding support for it shoudn't be hard. I may eventually do it
> when I have some time to play with my ISEE hardware.
> 
> So, in the case of tvp5150 hardware, have those PADS:
> 
> - Input baseband;
> - Video + raw VBI output;
> - sliced VBI output.

This DEMOD_PAD_VBI_OUT, does it correspond to 1) or 2) above?

> Yet, we need an always unconnected audio output, in order to support
> different demods out there.

Are you saying we have to keep pad[DEMOD_PAD_AUDIO_OUT] in tvp5150 even
though it doesn't exist because the framework can't cope with an
audio-less ATV_DECODER that only has three pads?

> [1] tvp5150 was written to support some em28xx-based devices
> 
> > 
> > > So, it has one input and three outputs.  How does marking the direction
> > > in the port node (which would indicate that there was a data flow out of
> > > TVP5150 into the iMX6 capture) help identify which of those pads should
> > > be used?
> > >
> > > It would eliminate the input pad, but you still have three output pads
> > > to choose from.
> > > 
> > > So no, your idea can't work.  
> > 
> > In this case, removal of the VBI and audio pads might make this work,
> > but in general this is true. In my opinion, to make this truly generic,
> > we need an interface to ask the driver which media entity pad a given
> > device tree port corresponds to, as there might not even be a single
> > media entity corresponding to all ports for more complex devices.
> 
> Yes. We also need something like that at the userspace API.

thanks
Philipp

[toc] | [prev] | [next] | [standalone]

#1616478 — Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1

On 04/04/2017 04:10 PM, Russell King - ARM Linux wrote:
> On Thu, Mar 30, 2017 at 07:25:49PM +0200, Philipp Zabel wrote:
>> The TVP5150 DT bindings specify a single output port (port 0) that
>> corresponds to the video output pad (pad 1, DEMOD_PAD_VID_OUT).
>>
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> ---
>> I'm trying to get this to work with a TVP5150 analog TV decoder, and the
>> first problem is that this device doesn't have pad 0 as its single
>> output pad. Instead, as a MEDIA_ENT_F_ATV_DECODER entity, it has for
>> pads (input, video out, vbi out, audio out), and video out is pad 1,
>> whereas the device tree only defines a single port (0).
>
> Looking at the patch, it's highlighted another review point with
> Steve's driver.
>
>> diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
>> index 17e2386a3ca3a..c52d6ca797965 100644
>> --- a/drivers/staging/media/imx/imx-media-dev.c
>> +++ b/drivers/staging/media/imx/imx-media-dev.c
>> @@ -267,6 +267,15 @@ static int imx_media_create_link(struct imx_media_dev *imxmd,
>>  	source_pad = link->local_pad;
>>  	sink_pad = link->remote_pad;
>>
>> +	/*
>> +	 * If the source subdev is an analog video decoder with a single source
>> +	 * port, assume that this port 0 corresponds to the DEMOD_PAD_VID_OUT
>> +	 * entity pad.
>> +	 */
>> +	if (source->entity.function == MEDIA_ENT_F_ATV_DECODER &&
>> +	    local_sd->num_sink_pads == 0 && local_sd->num_src_pads == 1)
>> +		source_pad = DEMOD_PAD_VID_OUT;
>> +
>>  	v4l2_info(&imxmd->v4l2_dev, "%s: %s:%d -> %s:%d\n", __func__,
>>  		  source->name, source_pad, sink->name, sink_pad);
>
> What is "local" and what is "remote" here?  It seems that, in the case of
> a link being created with the sensor(etc), it's all back to front.
>
> Eg, I see locally:
>
> imx-media: imx_media_create_link: imx219 0-0010:0 -> imx6-mipi-csi2:0
>
> So here, "source" is the imx219 (the sensor), and sink is "imx6-mipi-csi2"
> (part of the iMX6 capture.)  However, this makes "local_sd" the subdev of
> the sensor, and "remote_sd" the subdev of the CSI2 interface - which is
> totally back to front - this code is part of the iMX6 capture system,
> so "local" implies that it should be part of that, and the "remote" thing
> would be the sensor.
>
> Hence, this seems completely confused.  I'd suggest that:
>
> (a) the "pad->pad.flags & MEDIA_PAD_FL_SINK" test in imx_media_create_link()
>     is moved into imx_media_create_links(), and placed here instead:
>
> 		for (j = 0; j < num_pads; j++) {
> 			pad = &local_sd->pad[j];
>
> 			if (pad->pad.flags & MEDIA_PAD_FL_SINK)
> 				continue;
>
> 			...
> 		}
>
>     as the pad isn't going to spontaneously change this flag while we
>     consider each individual link.


Sure, I can do that. It would avoid iterating unnecessarily through the
pad's links if the pad is a sink.


>  However, maybe the test should be:
>
> 			if (!(pad->pad.flags & MEDIA_PAD_FL_SOURCE))
>
>     ?
>

maybe that is more intuitive.


> (b) the terms "local" and "remote" in imx_media_create_link() are
>     replaced with "source" and "sink" respectively, since this will
>     now be called with a guaranteed source pad.

Agreed. I'll change the arg and local var names.

>
> As for Philipp's solution, I'm not sure what the correct solution for
> something like this is.  It depends how you view "hardware interface"
> as defined by video-interfaces.txt, and whether the pads on the TVP5150
> represent the hardware interfaces.  If we take the view that the pads
> do represent hardware interfaces, then using the reg= property on the
> port node will solve this problem.

And the missing port nodes would have to actually be defined first.
According to Philipp they aren't, only a single output port 0.


>
> If not, it would mean that we would have to have the iMX capture code
> scan the pads on the source device, looking for outputs - but that
> runs into a problem - if the source device has multiple outputs, does
> the reg= property specify the output pad index or the pad number,

And how do we even know the data direction of a DT port. Is it an input,
an output, bidirectional? The OF graph parsing in imx-media-of.c can't
determine a port's direction if it encounters a device it doesn't
recognize that has multiple ports. For now that is not really a problem
because upstream from the video mux and csi-2 receiver it's expected
there will only be original sources of video with only one source port.
But it can become a limitation later. For example a device that has
multiple output bus interfaces, which would require multiple output
ports.


Steve



> and what if one binding for a device specifies it one way and another
> device's binding specifies it a different way.
>
> There's lots of scope for making things really painful here, and
> ending up with needing sensor-specific code in capture drivers to
> work around different decisions on this.
>
> I think someone needs to nail this down, if it's not already too late.
>

[toc] | [prev] | [next] | [standalone]

#1616857 — Re: [PATCH v6 19/39] media: Add i.MX media core driver

[Multipart message — attachments visible in raw view] — view raw

Hi!

> +References
> +----------
> +
> +[1] "i.MX 6Dual/6Quad Applications Processor Reference Manual"
> +[2] "i.MX 6Solo/6DualLite Applications Processor Reference Manual"

[2] is not present in the text above. [1] is there many times. What is
purpose of this section?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1616863 — Re: [PATCH v6 19/39] media: Add i.MX media core driver

[Multipart message — attachments visible in raw view] — view raw

Hi!


> +https://boundarydevices.com/products/nit6x_5mp

I'd use /product/ in url, as it redirects there.

> +https://boundarydevices.com/product/nit6x_5mp_mipi

..and for consistency.

> +The following example configures a direct conversion pipeline to capture
> +from the OV5640, transmitting on MIPI CSI-2 virtual channel 1. $sensorfmt
> +can be any format supported by the OV5640. $sensordim is the frame
> +dimension part of $sensorfmt (minus the mbus pixel code). $outputfmt can
> +be any format supported by the ipu1_ic_prpenc entity at its output pad:
> +
> +.. code-block:: none
> +
> +   # Setup links
> +   media-ctl -l "'ov5640 1-003c':0 -> 'imx6-mipi-csi2':0[1]"
> +   media-ctl -l "'imx6-mipi-csi2':2 -> 'ipu1_csi1':0[1]"
> +   media-ctl -l "'ipu1_csi1':1 -> 'ipu1_ic_prp':0[1]"
> +   media-ctl -l "'ipu1_ic_prp':1 -> 'ipu1_ic_prpenc':0[1]"
> +   media-ctl -l "'ipu1_ic_prpenc':1 -> 'ipu1_ic_prpenc capture':0[1]"
> +   # Configure pads
> +   media-ctl -V "'ov5640 1-003c':0 [fmt:$sensorfmt field:none]"
> +   media-ctl -V "'imx6-mipi-csi2':2 [fmt:$sensorfmt field:none]"
> +   media-ctl -V "'ipu1_csi1':1 [fmt:AYUV32/$sensordim field:none]"
> +   media-ctl -V "'ipu1_ic_prp':1 [fmt:AYUV32/$sensordim field:none]"
> +   media-ctl -V "'ipu1_ic_prpenc':1 [fmt:$outputfmt field:none]"
> +
> +Streaming can then begin on "ipu1_ic_prpenc capture" node. The v4l2-ctl
> +tool can be used to select any supported YUV or RGB pixelformat on the
> +capture device node.

Nothing for you to fix here, but we should really create some
library-like interface for media-ctl. 
									Pavel
									
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1617817 — Re: [PATCH v6 19/39] media: Add i.MX media core driver

On Mon, 2017-03-27 at 17:40 -0700, Steve Longerbeam wrote:
> Add the core media driver for i.MX SOC.
> 
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
[...]
> diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
> new file mode 100644
> index 0000000..b383be4
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-media-of.c
> @@ -0,0 +1,267 @@
[...]
> +/*
> + * find the remote device node and remote port id (remote pad #)
> + * given local endpoint node
> + */
> +static void of_get_remote_pad(struct device_node *epnode,
> +			      struct device_node **remote_node,
> +			      int *remote_pad)
> +{
> +	struct device_node *rp, *rpp;
> +	struct device_node *remote;
> +
> +	rp = of_graph_get_remote_port(epnode);
> +	rpp = of_graph_get_remote_port_parent(epnode);
> +
> +	if (of_device_is_compatible(rpp, "fsl,imx6q-ipu")) {
> +		/* the remote is one of the CSI ports */
> +		remote = rp;
> +		*remote_pad = 0;
> +		of_node_put(rpp);
> +	} else {
> +		remote = rpp;
> +		of_property_read_u32(rp, "reg", remote_pad);

If this fails because there is no reg property, *remote_pad will keep
the previous value. It should be set to 0 in this case.

----------8<----------
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -85,7 +85,9 @@ static void of_get_remote_pad(struct device_node *epnode,
                of_node_put(rpp);
        } else {
                remote = rpp;
-               of_property_read_u32(rp, "reg", remote_pad);
+               /* FIXME port number and pad index are not the same */
+               if (of_property_read_u32(rp, "reg", remote_pad))
+                       *remote_pad = 0;
                of_node_put(rp);
        }
 
---------->8----------

regards
Philipp

[toc] | [prev] | [next] | [standalone]

#1618410 — Re: [PATCH v6 19/39] media: Add i.MX media core driver

On 04/06/2017 02:43 AM, Philipp Zabel wrote:
> On Mon, 2017-03-27 at 17:40 -0700, Steve Longerbeam wrote:
>> Add the core media driver for i.MX SOC.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> [...]
>> diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
>> new file mode 100644
>> index 0000000..b383be4
>> --- /dev/null
>> +++ b/drivers/staging/media/imx/imx-media-of.c
>> @@ -0,0 +1,267 @@
> [...]
>> +/*
>> + * find the remote device node and remote port id (remote pad #)
>> + * given local endpoint node
>> + */
>> +static void of_get_remote_pad(struct device_node *epnode,
>> +			      struct device_node **remote_node,
>> +			      int *remote_pad)
>> +{
>> +	struct device_node *rp, *rpp;
>> +	struct device_node *remote;
>> +
>> +	rp = of_graph_get_remote_port(epnode);
>> +	rpp = of_graph_get_remote_port_parent(epnode);
>> +
>> +	if (of_device_is_compatible(rpp, "fsl,imx6q-ipu")) {
>> +		/* the remote is one of the CSI ports */
>> +		remote = rp;
>> +		*remote_pad = 0;
>> +		of_node_put(rpp);
>> +	} else {
>> +		remote = rpp;
>> +		of_property_read_u32(rp, "reg", remote_pad);
>
> If this fails because there is no reg property, *remote_pad will keep
> the previous value. It should be set to 0 in this case.

Ok, I will apply this change.

Steve

>
> ----------8<----------
> --- a/drivers/staging/media/imx/imx-media-of.c
> +++ b/drivers/staging/media/imx/imx-media-of.c
> @@ -85,7 +85,9 @@ static void of_get_remote_pad(struct device_node *epnode,
>                 of_node_put(rpp);
>         } else {
>                 remote = rpp;
> -               of_property_read_u32(rp, "reg", remote_pad);
> +               /* FIXME port number and pad index are not the same */
> +               if (of_property_read_u32(rp, "reg", remote_pad))
> +                       *remote_pad = 0;
>                 of_node_put(rp);
>         }
>
> ---------->8----------
>
> regards
> Philipp
>

[toc] | [prev] | [standalone]

Page 3 of 3 — ← Prev page 1 2 [3]

Back to top | Article view | linux.kernel

csiph-web