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


Groups > linux.kernel > #1616762

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

From Philipp Zabel <p.zabel@pengutronix.de>
Newsgroups linux.kernel
Subject Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
Date 2017-04-05 11:40 +0200
Message-ID <tsPzX-Ri-1@gated-at.bofh.it> (permalink)
References (2 earlier) <tqM3v-1dT-7@gated-at.bofh.it> <tsFTX-32H-13@gated-at.bofh.it> <tsHj3-3Oe-5@gated-at.bofh.it> <tsHj3-3Oe-3@gated-at.bofh.it> <tsOue-bB-23@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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.

> 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.

> As I already stated, I believe that this case is already covered by
> video-interfaces.txt:
> 
>   If more than
>   one port is present in a device node or there is more than one endpoint at a
>   port, or port node needs to be associated with a selected hardware interface,
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   a common scheme using '#address-cells', '#size-cells' and 'reg' properties is
>   used.
> 
> So, according to that, you do not need to have more than one port node
> to use the reg property - it's _either_ more than one port _or_ to
> select the hardware interface.

I don't think enforcing a 1:1 correspondence between device tree node
and media entity and enforcing port reg property == entity pad index is
a good idea in the long run. Binding errors are going to be made that
will have to be worked around at the driver level, and more complex
devices might have to create multiple media entities / v4l2 subdevices
with external ports that interface with the of graph.

> It all hinges on whether you consider the video output, VBI output or
> audio output to be separate hardware interfaces that the singular
> specified "port" node needs to select between.
> 
> There's another reason why the TVP5150 binding looks wrong and broken,
> however.  How does the audio output get routed to other parts of the
> system if you're using the video output, and how is that relationship
> defined?  It's a v4l2 subdev pad, so it would need to be part of the
> v4l2 subdev graph.  It sounds to me like the binding was created with
> a narrow focused "this is the board in front of me, it only has video
> wired up, I'm not going to consider other use cases" blinkered view.

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
was a custom binding for the inputs, that got quickly reverted:
https://patchwork.kernel.org/patch/8395521/

regards
Philipp

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[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

csiph-web