| Message ID | 1417506124-18626-1-git-send-email-andy.yan@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andy, Am Dienstag, den 02.12.2014, 15:42 +0800 schrieb Andy Yan: > diff --git a/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt > new file mode 100644 > index 0000000..107c1ca > --- /dev/null > +++ b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt > @@ -0,0 +1,40 @@ > +DesignWare HDMI bridge bindings > + > +Required properities: > +- compatible: platform specific such as: > + * "fsl,imx6q-hdmi" > + * "fsl,imx6dl-hdmi" > + * "rockchip,rk3288-dw-hdmi" I think we should add a common compatible value "snps,dw-hdmi-tx" here: compatible = "fsl,imx6q-hdmi", "snps,dw-hdmi-tx"; > +- reg: Physical base address and length of the controller's registers. > +- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing Better make ddc-i2c-bus optional, see the other thread about the ddc i2c master. > +- interrupts: The HDMI interrupt number > + > +Optional properties > +- reg-io-width: the width of the reg:1,4, default set to 1 if not present > + > +Example: > + hdmi: hdmi@0120000 { > + compatible = "fsl,imx6q-hdmi"; > + reg = <0x00120000 0x9000>; > + interrupts = <0 115 0x04>; > + gpr = <&gpr>; > + clocks = <&clks 123>, <&clks 124>; > + clock-names = "iahb", "isfr"; > + ddc-i2c-bus = <&i2c2>; > + > + port@0 { > + reg = <0>; > + > + hdmi_mux_0: endpoint { > + remote-endpoint = <&ipu1_di0_hdmi>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + > + hdmi_mux_1: endpoint { > + remote-endpoint = <&ipu1_di1_hdmi>; > + }; > + }; > + }; regards Philipp
Hi Philipp: On 2014?12?03? 02:23, Philipp Zabel wrote: > Hi Andy, > > Am Dienstag, den 02.12.2014, 15:42 +0800 schrieb Andy Yan: >> diff --git a/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt >> new file mode 100644 >> index 0000000..107c1ca >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt >> @@ -0,0 +1,40 @@ >> +DesignWare HDMI bridge bindings >> + >> +Required properities: >> +- compatible: platform specific such as: >> + * "fsl,imx6q-hdmi" >> + * "fsl,imx6dl-hdmi" >> + * "rockchip,rk3288-dw-hdmi" > I think we should add a common compatible value "snps,dw-hdmi-tx" here: > > compatible = "fsl,imx6q-hdmi", "snps,dw-hdmi-tx"; > How about "snps,dw-hdmi", because the driver is not only about hdmi tx, but also include hdmi phy. If we add such compatible value, do we have to implement another platform driver like dw_hdmi-pltfm.c with the compatible="snps,dw-hdmi" , or just include the compatible value in dw_hdmi-imx.c and dw_hdmi-rockchip.c? >> +- reg: Physical base address and length of the controller's registers. >> +- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing > Better make ddc-i2c-bus optional, see the other thread about the ddc i2c > master. > >> +- interrupts: The HDMI interrupt number >> + >> +Optional properties >> +- reg-io-width: the width of the reg:1,4, default set to 1 if not present >> + >> +Example: >> + hdmi: hdmi@0120000 { >> + compatible = "fsl,imx6q-hdmi"; >> + reg = <0x00120000 0x9000>; >> + interrupts = <0 115 0x04>; >> + gpr = <&gpr>; >> + clocks = <&clks 123>, <&clks 124>; >> + clock-names = "iahb", "isfr"; >> + ddc-i2c-bus = <&i2c2>; >> + >> + port@0 { >> + reg = <0>; >> + >> + hdmi_mux_0: endpoint { >> + remote-endpoint = <&ipu1_di0_hdmi>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + >> + hdmi_mux_1: endpoint { >> + remote-endpoint = <&ipu1_di1_hdmi>; >> + }; >> + }; >> + }; > regards > Philipp > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip > > >
Hi Andy, Am Mittwoch, den 03.12.2014, 08:54 +0800 schrieb Andy Yan: > >> +Required properities: > >> +- compatible: platform specific such as: > >> + * "fsl,imx6q-hdmi" > >> + * "fsl,imx6dl-hdmi" > >> + * "rockchip,rk3288-dw-hdmi" > > I think we should add a common compatible value "snps,dw-hdmi-tx" here: > > > > compatible = "fsl,imx6q-hdmi", "snps,dw-hdmi-tx"; > > > How about "snps,dw-hdmi", because the driver is not only about > hdmi tx, but also include hdmi phy. Synopsys call the whole module "DesignWare HDMI Transmitter (TX) IP Solution": https://www.synopsys.com/dw/ipdir.php?ds=dwc_hdmi_14_csds_tx https://www.synopsys.com/dw/ipdir.php?ds=dwc_hdmi_20_csds_tx That includes the PHY. I'd prefer keeping the -tx in there to differentiate from a possible future "snps,dw-hdmi-rx": https://www.synopsys.com/dw/ipdir.php?ds=dwc_hdmi_14_csds_rx https://www.synopsys.com/dw/ipdir.php?ds=dwc_hdmi_20_csds_rx > If we add such compatible value, do we have to implement another > platform driver like dw_hdmi-pltfm.c with the > compatible="snps,dw-hdmi" , > or just include the compatible value in dw_hdmi-imx.c and > dw_hdmi-rockchip.c? That common compatible doesn't have to be used by any driver. It's just there to show these are the same/similar IP core. If a common driver without any SoC specific knowledge could be written, that one would match against the common compatible. regards Philipp
Hi Philipp: On 2014?12?03? 17:19, Philipp Zabel wrote: > Hi Andy, > > Am Mittwoch, den 03.12.2014, 08:54 +0800 schrieb Andy Yan: >>>> +Required properities: >>>> +- compatible: platform specific such as: >>>> + * "fsl,imx6q-hdmi" >>>> + * "fsl,imx6dl-hdmi" >>>> + * "rockchip,rk3288-dw-hdmi" >>> I think we should add a common compatible value "snps,dw-hdmi-tx" here: >>> >>> compatible = "fsl,imx6q-hdmi", "snps,dw-hdmi-tx"; >>> >> How about "snps,dw-hdmi", because the driver is not only about >> hdmi tx, but also include hdmi phy. > Synopsys call the whole module > "DesignWare HDMI Transmitter (TX) IP Solution": > > https://www.synopsys.com/dw/ipdir.php?ds=dwc_hdmi_14_csds_tx > https://www.synopsys.com/dw/ipdir.php?ds=dwc_hdmi_20_csds_tx > > That includes the PHY. I'd prefer keeping the -tx in there to > differentiate from a possible future "snps,dw-hdmi-rx": > > https://www.synopsys.com/dw/ipdir.php?ds=dwc_hdmi_14_csds_rx > https://www.synopsys.com/dw/ipdir.php?ds=dwc_hdmi_20_csds_rx Ok, I will add the compatible "snps, dw-hdmi-tx", So do I need to add this value to imx6dl.dtsi? >> If we add such compatible value, do we have to implement another >> platform driver like dw_hdmi-pltfm.c with the >> compatible="snps,dw-hdmi" , >> or just include the compatible value in dw_hdmi-imx.c and >> dw_hdmi-rockchip.c? > That common compatible doesn't have to be used by any driver. It's just > there to show these are the same/similar IP core. > If a common driver without any SoC specific knowledge could be written, > that one would match against the common compatible. > > regards > Philipp > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip > > >
On 2014?12?03? 02:23, Philipp Zabel wrote: > Hi Andy, > > Am Dienstag, den 02.12.2014, 15:42 +0800 schrieb Andy Yan: >> diff --git a/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt >> new file mode 100644 >> index 0000000..107c1ca >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt >> @@ -0,0 +1,40 @@ >> +DesignWare HDMI bridge bindings >> + >> +Required properities: >> +- compatible: platform specific such as: >> + * "fsl,imx6q-hdmi" >> + * "fsl,imx6dl-hdmi" >> + * "rockchip,rk3288-dw-hdmi" > I think we should add a common compatible value "snps,dw-hdmi-tx" here: > > compatible = "fsl,imx6q-hdmi", "snps,dw-hdmi-tx"; > >> +- reg: Physical base address and length of the controller's registers. >> +- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing > Better make ddc-i2c-bus optional, see the other thread about the ddc i2c > master. I have the same idea too, but the patch about ddc i2c master has not landed yet, can we change the ddc-i2c-bus to optional after the ddc i2c master patch land? >> +- interrupts: The HDMI interrupt number >> + >> +Optional properties >> +- reg-io-width: the width of the reg:1,4, default set to 1 if not present >> + >> +Example: >> + hdmi: hdmi@0120000 { >> + compatible = "fsl,imx6q-hdmi"; >> + reg = <0x00120000 0x9000>; >> + interrupts = <0 115 0x04>; >> + gpr = <&gpr>; >> + clocks = <&clks 123>, <&clks 124>; >> + clock-names = "iahb", "isfr"; >> + ddc-i2c-bus = <&i2c2>; >> + >> + port@0 { >> + reg = <0>; >> + >> + hdmi_mux_0: endpoint { >> + remote-endpoint = <&ipu1_di0_hdmi>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + >> + hdmi_mux_1: endpoint { >> + remote-endpoint = <&ipu1_di1_hdmi>; >> + }; >> + }; >> + }; > regards > Philipp > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip > > >
Am Mittwoch, den 03.12.2014, 17:46 +0800 schrieb Andy Yan: > On 2014?12?03? 02:23, Philipp Zabel wrote: > > Hi Andy, > > > > Am Dienstag, den 02.12.2014, 15:42 +0800 schrieb Andy Yan: > >> diff --git a/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt > >> new file mode 100644 > >> index 0000000..107c1ca > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt > >> @@ -0,0 +1,40 @@ > >> +DesignWare HDMI bridge bindings > >> + > >> +Required properities: > >> +- compatible: platform specific such as: > >> + * "fsl,imx6q-hdmi" > >> + * "fsl,imx6dl-hdmi" > >> + * "rockchip,rk3288-dw-hdmi" > > I think we should add a common compatible value "snps,dw-hdmi-tx" here: > > > > compatible = "fsl,imx6q-hdmi", "snps,dw-hdmi-tx"; > > > >> +- reg: Physical base address and length of the controller's registers. > >> +- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing > > Better make ddc-i2c-bus optional, see the other thread about the ddc i2c > > master. > I have the same idea too, but the patch about ddc i2c master has not > landed yet, can we change the ddc-i2c-bus to optional after the ddc > i2c master > patch land? Check out Documentation/devicetree/bindings/drm/imx/hdmi.txt, it was already marked as optional. We can't make it required now. regards Philipp
On 2014?12?03? 19:52, Philipp Zabel wrote: > Am Mittwoch, den 03.12.2014, 17:46 +0800 schrieb Andy Yan: >> On 2014?12?03? 02:23, Philipp Zabel wrote: >>> Hi Andy, >>> >>> Am Dienstag, den 02.12.2014, 15:42 +0800 schrieb Andy Yan: >>>> diff --git a/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt >>>> new file mode 100644 >>>> index 0000000..107c1ca >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt >>>> @@ -0,0 +1,40 @@ >>>> +DesignWare HDMI bridge bindings >>>> + >>>> +Required properities: >>>> +- compatible: platform specific such as: >>>> + * "fsl,imx6q-hdmi" >>>> + * "fsl,imx6dl-hdmi" >>>> + * "rockchip,rk3288-dw-hdmi" >>> I think we should add a common compatible value "snps,dw-hdmi-tx" here: >>> >>> compatible = "fsl,imx6q-hdmi", "snps,dw-hdmi-tx"; >>> >>>> +- reg: Physical base address and length of the controller's registers. >>>> +- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing >>> Better make ddc-i2c-bus optional, see the other thread about the ddc i2c >>> master. >> I have the same idea too, but the patch about ddc i2c master has not >> landed yet, can we change the ddc-i2c-bus to optional after the ddc >> i2c master >> patch land? > Check out Documentation/devicetree/bindings/drm/imx/hdmi.txt, it was > already marked as optional. We can't make it required now. > > regards > Philipp > > > OK, got it, thanks
diff --git a/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt new file mode 100644 index 0000000..107c1ca --- /dev/null +++ b/Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt @@ -0,0 +1,40 @@ +DesignWare HDMI bridge bindings + +Required properities: +- compatible: platform specific such as: + * "fsl,imx6q-hdmi" + * "fsl,imx6dl-hdmi" + * "rockchip,rk3288-dw-hdmi" +- reg: Physical base address and length of the controller's registers. +- ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing +- interrupts: The HDMI interrupt number + +Optional properties +- reg-io-width: the width of the reg:1,4, default set to 1 if not present + +Example: + hdmi: hdmi@0120000 { + compatible = "fsl,imx6q-hdmi"; + reg = <0x00120000 0x9000>; + interrupts = <0 115 0x04>; + gpr = <&gpr>; + clocks = <&clks 123>, <&clks 124>; + clock-names = "iahb", "isfr"; + ddc-i2c-bus = <&i2c2>; + + port@0 { + reg = <0>; + + hdmi_mux_0: endpoint { + remote-endpoint = <&ipu1_di0_hdmi>; + }; + }; + + port@1 { + reg = <1>; + + hdmi_mux_1: endpoint { + remote-endpoint = <&ipu1_di1_hdmi>; + }; + }; + };
Signed-off-by: Andy Yan <andy.yan@rock-chips.com> --- Changes in v15: None Changes in v14: None Changes in v13: None Changes in v12: None Changes in v11: None Changes in v10: None Changes in v9: None Changes in v8: - correct some spelling mistake - modify ddc-i2c-bus and interrupt description Changes in v7: None Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None .../devicetree/bindings/drm/bridge/dw_hdmi.txt | 40 ++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/drm/bridge/dw_hdmi.txt