Hi Niklas,
On Monday, 14 May 2018 13:23:26 EEST Niklas Söderlund wrote:
> On 2018-05-14 09:39:34 +0200, Jacopo Mondi wrote:
> > On Sun, May 13, 2018 at 02:57:55PM +0200, Niklas Söderlund wrote:
> >> On 2018-05-11 12:00:02 +0200, Jacopo Mondi wrote:
> >>> Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> >>> development board.
> >>>
> >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> >>> ---
> >>>
> >>> arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 +++++++++++++++++++
> >>> 1 file changed, 68 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> >>> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index
> >>> d03f194..e0ce462 100644
> >>> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> >>> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
[snip]
> >>> +&vin4 {
> >>> + pinctrl-0 = <&vin4_pins>;
> >>> + pinctrl-names = "default";
> >>> +
> >>> + status = "okay";
> >>> +
> >>> + ports {
> >>> + #address-cells = <1>;
> >>> + #size-cells = <0>;
> >>> +
> >>> + port@0 {
> >>> + reg = <0>;
> >>> +
> >>> + vin4_in: endpoint {
> >>> + hsync-active = <0>;
> >>> + vsync-active = <0>;
> >>
> >> Comparing this to the Gen2 bindings some properties are missing,
> >>
> >> bus-width = <24>;
> >> pclk-sample = <1>;
> >> data-active = <1>;
> >
> > The VIN driver does not parse them, so there is no value in having
> > them there, if not confusing people as it happened to me reading the
> > Gen2 DT.
>
> I have no objection removing them. Trying to understand why the
> description differed from Gen2.
>
> >> This is not a big deal as the VIN driver don't use these properties so
> >> no functional change should come of this but still a difference.
> >
> > Exactly.
> >
> > On a side note. I have not seen a way to configure the pixel clock
> > sampling level in the interface datasheet. The register used to
> > configure synchronism signals polarities is VnDMR2, and there I read
> > we can configure HSYNC/VSYNC and CLOCKENB (which is data enable, not
> > pixel clock) polarities. Is it configured through some other
> > register?
>
> I have not seen such a register no.
>
> >> Over all I'm happy with this change but before I add my tag I would like
> >> to understand why it differs from the Gen2 configuration for the adv7612
> >> properties.
> >>
> >> Also on a side not it is possible with hardware switches on the board
> >> switch the VIN4 source to a completely different pipeline CVBS connector
> >> -> adv7180 -> VIN4. But I think it's best we keep the HDMI as default as
> >> this seems to be how the boards are shipped. But maybe mentioning this
> >> in the commit message would not hurt if you end-up resending the patch.
> >
> > Oh I see. SW-49 to SW-52 enables the HDMI input, SW53-SW54 CVBS one.
> > And actually, reading the 'initial setting of slide switches' in the
> > Draak board manual, it turns out that the board default configuration
> > is with CVBS input selected... What should we do here? reflect
> > defaults in the DT, or prioritize HDMI?
>
> I feel this is a question for Laurent. My feeling for how we handled
> this in other cases is to go with the board default settings. I'm
> however sure there are exceptions to the rule. So maybe we should go
> with the most useful (what ever that is) configuration?
I think I'd go with CVBS as I don't think HDMI would be considered as the most
useful configuration here. The Draak board is unlikely to be used by us as a
reference platform to test HDMI capture, is it ?
This being said, you can instantiate the adv7612 and HDMI connector in DT,
without connecting them to the VIN. That would make it easy to quickly change
the configuration.
> >>> +
> >>> + remote-endpoint = <&adv7612_out>;
> >>> + };
> >>> + };
> >>> + };
> >>> +};
--
Regards,
Laurent Pinchart
