Em Fri, 3 Aug 2018 09:29:53 +0200 Marco Felsch <m.felsch@xxxxxxxxxxxxxx> escreveu: > Hi Rob, > > first of all, thanks for the review. After some discussion with the > media guys I have a question about the dt-bindings. > > On 18-07-03 17:23, Rob Herring wrote: > > On Thu, Jun 28, 2018 at 06:20:52PM +0200, Marco Felsch wrote: > > > The TVP5150/1 decoders support different video input sources to their > > > AIP1A/B pins. > > > > > > Possible configurations are as follows: > > > - Analog Composite signal connected to AIP1A. > > > - Analog Composite signal connected to AIP1B. > > > - Analog S-Video Y (luminance) and C (chrominance) > > > signals connected to AIP1A and AIP1B respectively. > > > > > > This patch extends the device tree bindings documentation to describe > > > how the input connectors for these devices should be defined in a DT. > > > > > > Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx> > > > --- > > > .../devicetree/bindings/media/i2c/tvp5150.txt | 118 +++++++++++++++++- > > > 1 file changed, 113 insertions(+), 5 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt > > > index 8c0fc1a26bf0..feed8c911c5e 100644 > > > --- a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt > > > +++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt > > > @@ -12,11 +12,23 @@ Optional Properties: > > > - pdn-gpios: phandle for the GPIO connected to the PDN pin, if any. > > > - reset-gpios: phandle for the GPIO connected to the RESETB pin, if any. > > > > > > -The device node must contain one 'port' child node for its digital output > > > -video port, in accordance with the video interface bindings defined in > > > -Documentation/devicetree/bindings/media/video-interfaces.txt. > > > +The device node must contain one 'port' child node per device input and output > > > +port, in accordance with the video interface bindings defined in > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes > > > +are numbered as follows > > > > > > -Required Endpoint Properties for parallel synchronization: > > > + Name Type Port > > > + -------------------------------------- > > > + AIP1A sink 0 > > > + AIP1B sink 1 > > > + S-VIDEO sink 2 > > > + Y-OUT src 3 > > Do you think it's correct to have a seperate port for each binding? > Since the S-Video port is a combination of AIP1A and AIP1B. After a > discussion with Mauro [1] the TVP5150 should have only 3 pads. Since the > pads are directly mappped to the dt-ports this will correspond to three > ports (2 in, 1 out). Now the svideo connector will be mapped to a second > endpoint in each port: > > port@0 > endpoint@0 -----------> Comp0-Con > endpoint@1 -----+-----> Svideo-Con > port@1 | > endpoint@0 -----|-----> Comp1-Con > endpoint@1 -----+ > port@2 > endpoint For tvp5150, the model is like the above, so just one port at the S-video connector. Yet, for more complex devices that would allow switching the endpoints at the Svideo connector, the model would be: port@0 endpoint@0 (AIP1A) -----------> Comp0-Con endpoint@1 (AIP1B) -----------> Svideo-Con port@0 (luminance) port@1 endpoint@0 (AIP1A) -----------> Comp1-Con endpoint@1 (AIP1B) -----------> Svideo-Con port@1 (chrominance) port@2 endpoint (video bitstream output) E. g. the S-Video connector will also have two ports, one for the chrominance signal and another one for the luminance one. > I don't like that solution that much, since the mapping is now signal > based. We also don't map each line of a parallel port. > > A quick grep shows that currently each device using a svideo connector > seperates them in a own port as I did. No. I've no idea about how you did the grep, but this is not how other drivers handle it currently. Right now, on all hardware where connectors are mapped, there is just one input port and multiple connectors linked to it. You can see some examples here: https://www.infradead.org/~mchehab/mc-next-gen/au0828_hvr950q.png https://www.infradead.org/~mchehab/mc-next-gen/cx231xx_hvr930c_hd.png https://www.infradead.org/~mchehab/mc-next-gen/em28xx_hvr950.png https://www.infradead.org/~mchehab/mc-next-gen/playtv_usb.png https://www.infradead.org/~mchehab/mc-next-gen/saa7134-asus-p7131-dual.png https://www.infradead.org/~mchehab/mc-next-gen/wintv_usb2.png The problem with this approach is that it doesn't reflect how the hardware is actually wired. On some hardware similar to tvp5150, it may be possible, for example, to wire the S-Video both ways, e. g., something equivalent to (using tvp5150 terminology): Luminance -> AIP1A Chrominance -> AIP1B or Luminance -> AIP1B Chrominance -> AIP1A So, just one pad wouldn't allow this kind of config. Having three pads is equally wrong, as there's no S-Video port on tvp5150. All it has physically are two inputs: AIP1A and AIP1B. If you want to see the discussions we had, they are at: https://linuxtv.org/irc/irclogger_log/media-maint?date=2018-08-02,Thu I'm preparing right now a summary of them. will copy you once I finish it. > IMHO this is a uncomplicate > solution, but don't abstract the HW correctly. What is your opinion > about that? Is it correct to have seperate (virtual) port or should I > map the svideo connector as shown above? > > [1] https://www.spinics.net/lists/devicetree/msg242825.html Anyway, I'm writing a summary of our discussions Thanks, Mauro
