| Message ID | 1349824101-32574-3-git-send-email-ajaykumar.rs@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Ajay, On 10/10/2012 01:08 AM, Ajay Kumar wrote: > Add documentation for the DT bindings in exynos display port driver. > > Signed-off-by: Ajay Kumar<ajaykumar.rs@samsung.com> > --- > .../devicetree/bindings/video/exynos_dp.txt | 83 ++++++++++++++++++++ > 1 files changed, 83 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/video/exynos_dp.txt > > diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt b/Documentation/devicetree/bindings/video/exynos_dp.txt > new file mode 100644 > index 0000000..a021963 > --- /dev/null > +++ b/Documentation/devicetree/bindings/video/exynos_dp.txt > @@ -0,0 +1,83 @@ > +Exynos display port driver should configure the display port interface > +based on the type of panel connected to it. The bindings are supposed to describe devices, not drivers. So it might be better to say: "The Exynos display port interface should be configured based on the type of panel connected to it." From this documentation it is not clear which properties are required and which are optional for each node. I think, in the property names dashes should be used, rather than underscores. Dashes are much more common among existing bindings. > +We use two nodes: > + -dptx_phy node > + -display-port-controller node > + > +For the dp-phy initialization, we use a dptx_phy node. > +Required properties for dptx_phy: > + -compatible: > + Should be "samsung,dp-phy". Is there a separate dptx-phy driver that is being matched with this compatible property ? Is the dptx-node going to be referenced by other nodes than display-port-controller ? If not, then you could likely drop the compatible property entirely and make the dptx-phy node a child node of display-port-controller, i.e. display-port-controller { compatible = "samsung,exynos5-dp"; reg = <0x145b0000 0x10000>; interrupts =<10 3>; interrupt-parent =<&combiner>; dptx-phy { reg = <0x10040720>; samsung,enable_mask = <1>; }; }; Then the driver could just look for a child node named "dptx-phy" with e.g. of_find_node_by_name(). > + -samsung,dptx_phy_reg: I think it's fine to use just 'reg' instead of this vendor specific name. > + Base address of DP PHY register. > + -samsung,enable_mask: > + The bit-mask used to enable/disable DP PHY. > + > +For the Panel initialization, we read data from display-port-controller node. > +Required properties for display-port-controller: > + -compatible: > + Should be "samsung,exynos5-dp". > + -reg: > + physical base address of the controller and length > + of memory mapped region. > + -interrupts: > + Interrupt combiner values. > + -interrupt-parent: > + phandle to Interrupt combiner node. > + -samsung,dp_phy: > + phandle to dptx_phy node. > + -samsung,color_space: > + input video data format. > + COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2 > + -samsung,dynamic_range: > + dynamic range for input video data. > + VESA = 0, CEA = 1 > + -samsung,ycbcr_coeff: > + YCbCr co-efficients for input video. > + COLOR_YCBCR601 = 0, COLOR_YCBCR709 = 1 > + -samsung,color_depth: > + Number of bits per colour component. > + COLOR_6 = 0, COLOR_8 = 1, COLOR_10 = 2, COLOR_12 = 3 > + -samsung,link_rate: > + link rate supported by the panel. > + LINK_RATE_1_62GBPS = 0x6, LINK_RATE_2_70GBPS = 0x0A > + -samsung,lane_count: > + number of lanes supported by the panel. > + LANE_COUNT1 = 1, LANE_COUNT2 = 2, LANE_COUNT4 = 4 > + -samsung,interlaced: > + Interlace scan mode. > + Progressive if defined, Interlaced if not defined > + -samsung,v_sync_polarity: > + VSYNC polarity configuration. > + High if defined, Low if not defined > + -samsung,h_sync_polarity: > + HSYNC polarity configuration. > + High if defined, Low if not defined So there is no common video bindings for things like these two ? In V4L2 we decided to use vsync-active, hsync-active [1], the video timings bindings [2] use hsync-active-high, hsync-active-high boolean properties. Perhaps it is worth to pick some of those standard definitions and use instead of the vendor specific ones ? > + > +Example: > + > +SOC specific portion: > + dptx_phy: dptx_phy@0x10040720 { > + compatible = "samsung,dp-phy"; > + samsung,dptx_phy_reg =<0x10040720>; reg = <0x10040720>; > + samsung,enable_mask =<1>; > + }; > + > + display-port-controller { > + compatible = "samsung,exynos5-dp"; > + reg =<0x145B0000 0x10000>; I think lower case is preferred. > + interrupts =<10 3>; > + interrupt-parent =<&combiner>; > + samsung,dp_phy =<&dptx_phy>; > + }; > + > +Board Specific portion: > + display-port-controller { > + samsung,color_space =<0>; > + samsung,dynamic_range =<0>; > + samsung,ycbcr_coeff =<0>; > + samsung,color_depth =<1>; > + samsung,link_rate =<0x0a>; > + samsung,lane_count =<2>; > + }; Thanks, Sylwester [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg52743.html [2] http://www.mail-archive.com/linux-media@vger.kernel.org/msg53323.html -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sylwester, On Wed, Oct 10, 2012 at 2:59 AM, Sylwester Nawrocki <sylvester.nawrocki@gmail.com> wrote: > Hi Ajay, > > On 10/10/2012 01:08 AM, Ajay Kumar wrote: >> Add documentation for the DT bindings in exynos display port driver. >> >> Signed-off-by: Ajay Kumar<ajaykumar.rs@samsung.com> >> --- >> .../devicetree/bindings/video/exynos_dp.txt | 83 ++++++++++++++++++++ >> 1 files changed, 83 insertions(+), 0 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/video/exynos_dp.txt >> >> diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt b/Documentation/devicetree/bindings/video/exynos_dp.txt >> new file mode 100644 >> index 0000000..a021963 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/video/exynos_dp.txt >> @@ -0,0 +1,83 @@ >> +Exynos display port driver should configure the display port interface >> +based on the type of panel connected to it. > > The bindings are supposed to describe devices, not drivers. So it > might be better to say: > > "The Exynos display port interface should be configured based on the > type of panel connected to it." Ok. I will change it. > From this documentation it is not clear which properties are required > and which are optional for each node. > I think, in the property names dashes should be used, rather than > underscores. Dashes are much more common among existing bindings. Ok. Will make use of dashes and mention optional properties. >> +We use two nodes: >> + -dptx_phy node >> + -display-port-controller node >> + >> +For the dp-phy initialization, we use a dptx_phy node. >> +Required properties for dptx_phy: >> + -compatible: >> + Should be "samsung,dp-phy". > > Is there a separate dptx-phy driver that is being matched with this > compatible property ? Is the dptx-node going to be referenced by other > nodes than display-port-controller ? If not, then you could likely drop > the compatible property entirely and make the dptx-phy node a child > node of display-port-controller, i.e. > > display-port-controller { > compatible = "samsung,exynos5-dp"; > reg = <0x145b0000 0x10000>; > interrupts =<10 3>; > interrupt-parent =<&combiner>; > > dptx-phy { > reg = <0x10040720>; > samsung,enable_mask = <1>; > }; > }; > > Then the driver could just look for a child node named "dptx-phy" with > e.g. of_find_node_by_name(). Ok. Will change it. >> + -samsung,dptx_phy_reg: > > I think it's fine to use just 'reg' instead of this vendor specific name. Ok. >> + Base address of DP PHY register. >> + -samsung,enable_mask: >> + The bit-mask used to enable/disable DP PHY. >> + >> +For the Panel initialization, we read data from display-port-controller node. >> +Required properties for display-port-controller: >> + -compatible: >> + Should be "samsung,exynos5-dp". >> + -reg: >> + physical base address of the controller and length >> + of memory mapped region. >> + -interrupts: >> + Interrupt combiner values. >> + -interrupt-parent: >> + phandle to Interrupt combiner node. >> + -samsung,dp_phy: >> + phandle to dptx_phy node. >> + -samsung,color_space: >> + input video data format. >> + COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2 >> + -samsung,dynamic_range: >> + dynamic range for input video data. >> + VESA = 0, CEA = 1 >> + -samsung,ycbcr_coeff: >> + YCbCr co-efficients for input video. >> + COLOR_YCBCR601 = 0, COLOR_YCBCR709 = 1 >> + -samsung,color_depth: >> + Number of bits per colour component. >> + COLOR_6 = 0, COLOR_8 = 1, COLOR_10 = 2, COLOR_12 = 3 >> + -samsung,link_rate: >> + link rate supported by the panel. >> + LINK_RATE_1_62GBPS = 0x6, LINK_RATE_2_70GBPS = 0x0A >> + -samsung,lane_count: >> + number of lanes supported by the panel. >> + LANE_COUNT1 = 1, LANE_COUNT2 = 2, LANE_COUNT4 = 4 >> + -samsung,interlaced: >> + Interlace scan mode. >> + Progressive if defined, Interlaced if not defined >> + -samsung,v_sync_polarity: >> + VSYNC polarity configuration. >> + High if defined, Low if not defined >> + -samsung,h_sync_polarity: >> + HSYNC polarity configuration. >> + High if defined, Low if not defined > > So there is no common video bindings for things like these two ? > In V4L2 we decided to use vsync-active, hsync-active [1], the video > timings bindings [2] use hsync-active-high, hsync-active-high boolean > properties. Perhaps it is worth to pick some of those standard > definitions and use instead of the vendor specific ones ? hsync-active-high and vsync-active-high seems to hold good in our case. Also, are you asking us to just use only the standard names or use standard helper functions as well? Since we use only hsync and vsync polarity and no other LCD timing properties, I think we need not use standard helper functions for parsing display timings! >> + >> +Example: >> + >> +SOC specific portion: >> + dptx_phy: dptx_phy@0x10040720 { >> + compatible = "samsung,dp-phy"; >> + samsung,dptx_phy_reg =<0x10040720>; > > reg = <0x10040720>; Ok. >> + samsung,enable_mask =<1>; >> + }; >> + >> + display-port-controller { >> + compatible = "samsung,exynos5-dp"; >> + reg =<0x145B0000 0x10000>; > > I think lower case is preferred. Ok. Will change it. >> + interrupts =<10 3>; >> + interrupt-parent =<&combiner>; >> + samsung,dp_phy =<&dptx_phy>; >> + }; >> + >> +Board Specific portion: >> + display-port-controller { >> + samsung,color_space =<0>; >> + samsung,dynamic_range =<0>; >> + samsung,ycbcr_coeff =<0>; >> + samsung,color_depth =<1>; >> + samsung,link_rate =<0x0a>; >> + samsung,lane_count =<2>; >> + }; > > Thanks, > Sylwester > > [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg52743.html > [2] http://www.mail-archive.com/linux-media@vger.kernel.org/msg53323.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Regards, Ajay -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/11/2012 08:50 AM, Ajay kumar wrote: >>> + -samsung,interlaced: >>> + Interlace scan mode. >>> + Progressive if defined, Interlaced if not defined >>> + -samsung,v_sync_polarity: >>> + VSYNC polarity configuration. >>> + High if defined, Low if not defined >>> + -samsung,h_sync_polarity: >>> + HSYNC polarity configuration. >>> + High if defined, Low if not defined >> >> So there is no common video bindings for things like these two ? >> In V4L2 we decided to use vsync-active, hsync-active [1], the video >> timings bindings [2] use hsync-active-high, hsync-active-high boolean >> properties. Perhaps it is worth to pick some of those standard >> definitions and use instead of the vendor specific ones ? > hsync-active-high and vsync-active-high seems to hold good in our case. > Also, are you asking us to just use only the standard names or use standard > helper functions as well? Since we use only hsync and vsync polarity and no > other LCD timing properties, I think we need not use standard helper functions > for parsing display timings! My point was just to use common property names where possible. Any parsing helpers could be created afterwards, if you would rather avoid doing that right now. BTW, it seems 'interlaced' could also be reused. ... >> [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg52743.html >> [2] http://www.mail-archive.com/linux-media@vger.kernel.org/msg53323.html Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt b/Documentation/devicetree/bindings/video/exynos_dp.txt new file mode 100644 index 0000000..a021963 --- /dev/null +++ b/Documentation/devicetree/bindings/video/exynos_dp.txt @@ -0,0 +1,83 @@ +Exynos display port driver should configure the display port interface +based on the type of panel connected to it. + +We use two nodes: + -dptx_phy node + -display-port-controller node + +For the dp-phy initialization, we use a dptx_phy node. +Required properties for dptx_phy: + -compatible: + Should be "samsung,dp-phy". + -samsung,dptx_phy_reg: + Base address of DP PHY register. + -samsung,enable_mask: + The bit-mask used to enable/disable DP PHY. + +For the Panel initialization, we read data from display-port-controller node. +Required properties for display-port-controller: + -compatible: + Should be "samsung,exynos5-dp". + -reg: + physical base address of the controller and length + of memory mapped region. + -interrupts: + Interrupt combiner values. + -interrupt-parent: + phandle to Interrupt combiner node. + -samsung,dp_phy: + phandle to dptx_phy node. + -samsung,color_space: + input video data format. + COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2 + -samsung,dynamic_range: + dynamic range for input video data. + VESA = 0, CEA = 1 + -samsung,ycbcr_coeff: + YCbCr co-efficients for input video. + COLOR_YCBCR601 = 0, COLOR_YCBCR709 = 1 + -samsung,color_depth: + Number of bits per colour component. + COLOR_6 = 0, COLOR_8 = 1, COLOR_10 = 2, COLOR_12 = 3 + -samsung,link_rate: + link rate supported by the panel. + LINK_RATE_1_62GBPS = 0x6, LINK_RATE_2_70GBPS = 0x0A + -samsung,lane_count: + number of lanes supported by the panel. + LANE_COUNT1 = 1, LANE_COUNT2 = 2, LANE_COUNT4 = 4 + -samsung,interlaced: + Interlace scan mode. + Progressive if defined, Interlaced if not defined + -samsung,v_sync_polarity: + VSYNC polarity configuration. + High if defined, Low if not defined + -samsung,h_sync_polarity: + HSYNC polarity configuration. + High if defined, Low if not defined + +Example: + +SOC specific portion: + dptx_phy: dptx_phy@0x10040720 { + compatible = "samsung,dp-phy"; + samsung,dptx_phy_reg = <0x10040720>; + samsung,enable_mask = <1>; + }; + + display-port-controller { + compatible = "samsung,exynos5-dp"; + reg = <0x145B0000 0x10000>; + interrupts = <10 3>; + interrupt-parent = <&combiner>; + samsung,dp_phy = <&dptx_phy>; + }; + +Board Specific portion: + display-port-controller { + samsung,color_space = <0>; + samsung,dynamic_range = <0>; + samsung,ycbcr_coeff = <0>; + samsung,color_depth = <1>; + samsung,link_rate = <0x0a>; + samsung,lane_count = <2>; + };
Add documentation for the DT bindings in exynos display port driver. Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> --- .../devicetree/bindings/video/exynos_dp.txt | 83 ++++++++++++++++++++ 1 files changed, 83 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/video/exynos_dp.txt