On 5/29/19 1:04 PM, Ian Arkver wrote: > Hi, > > On 29/05/2019 11:41, Marek Vasut wrote: >> On 5/29/19 8:28 AM, Jacopo Mondi wrote: >> >> [...] >> >>>>>>> [1] >>>>>>> https://www.renesas.com/eu/en/products/audio-video/video-decoders-encoders/video-decoders/device/ISL79987.html >>>>>>> >>>>>>> >>>>>>>> +Required Properties: >>>>>>>> +- compatible: value should be "isil,isl79987" >>>>> >>>>> And here you might want to have 2 different compatibles for 79987 and >>>>> 79988. >>>> >>>> The 79988 is not supported yet, do we want to have it in the binding >>>> doc? >>>> >>> >>> I got mislead by the isl7998x naming scheme you used... >>> >>> I would say that's up to you, the two chips seems very similar, >>> and it might make sense to provide bindings that support both. At the >>> same time, as long as the here defined bindings does not prevent >>> future expansions to include the ISL79988, its support could be safely >>> post-poned. In that case please s/isl7998x/isl79987/ in this document >>> and do not mention BT565 in the description. >> >> Right >> >>>> [...] >>>> >>>>>>> I see from the example you only support one output port? How do you >>>>>>> model the input ones. >>>>>> >>>>>> I don't . Do we model analog inputs now somehow ? >>>>> >>>>> I really think so, please see: >>>>> Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt >>>>> >>>>> >>>>> And as an example of a board device tree using connectors to model >>>>> analog input see how the cvbs input on Salvator-X is described: >>>>> >>>>> cvbs-in { >>>>> compatible = "composite-video-connector"; >>>>> label = "CVBS IN"; >>>>> >>>>> port { >>>>> cvbs_con: endpoint { >>>>> remote-endpoint = <&adv7482_ain7>; >>>>> }; >>>>> }; >>>>> }; >>>>> >>>>> I think you should provide 4 input ports, where to connect input from >>>>> the analog connectors, and derive the number of enabled inputs from >>>>> the number of endpoints connected to an active remote. >>>> >>>> Deriving the number of active physical inputs from some existing >>>> binding >>>> makes sense. >>>> >>>> However unlike the adv7482, the isl79987 does not support remapping the >>>> physical inputs to ADCs in the chip. It does support some remapping of >>>> physical inputs to MIPI CSI2 channels, but that's probably not very >>>> useful. >>>> >>> >>> I understand, but I will now use against you the argument you have >>> correctly pointed out here below that DT should describe hardware, and >>> the hardware has indeed 4 input ports.. >> >> My question here is whether it makes sense to describe the ports even if >> they cannot be muxed to different ADC. Does it ? > > Each input port can be either differential CVBS or single ended with a > 2:1 input select mux. It would be nice to be able to describe this. Where do you see that ? > You cannot remap the inputs to different ADCs, but you can remap the > ADCs to different VC IDs using the > ISL7998x_REG_P5_LI_ENGINE_VC_ASSIGNMENT register. Describing each input > would proivde somewhere to specify the vc-id. I think Jacopo mentioned above the input muxing and the MIPI CSI2 VC muxing are two separate things. But I have to wonder, do we have a way of muxing the VCs in the DT or via the media controller yet ? -- Best regards, Marek Vasut
