| Message ID | 1496414235-20098-3-git-send-email-philippe.cornu@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/02/2017 04:37 PM, Philippe CORNU wrote: > This patch adds documentation of device tree bindings for the > Synopsys DesignWare MIPI DSI host DRM bridge driver. > > Signed-off-by: Philippe CORNU <philippe.cornu@st.com> > --- > .../bindings/display/bridge/dw_mipi_dsi.txt | 30 ++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt > > diff --git a/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt > new file mode 100644 > index 0000000..1d7c438 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt > @@ -0,0 +1,30 @@ > +Synopsys DesignWare MIPI DSI host controller > +============================================ > + > +This document defines device tree properties for the Synopsys DesignWare MIPI > +DSI host controller. It doesn't constitue a device tree binding specification ---------------------------------------/\ I'm not sure about this one, and it's already here in the dw_hdmi.txt text. > +by itself but is meant to be referenced by platform-specific device tree > +bindings. > + > +When referenced from platform device tree bindings the properties defined in > +this document are defined as follows. The platform device tree bindings are > +responsible for defining whether each property is required or optional. > + > +- reg: Memory mapped base address and length of the DWC MIPI DSI > + registers. (mandatory) -------------------/\ Why do you specify the mandatory/optional here since it's written it's the responsibility of the platform bindings ? > + > +- clocks: References to all the clocks specified in the clock-names property > + as specified in [1]. (mandatory) > + > +- clock-names: "pclk" is peripheral clock for either AHB and APB. (mandatory) > + > +- resets: References to all the resets specified in the reset-names property > + as specified in [2]. (optional) > + > +- reset-names: string reset name, must be "apb" if used. (optional) > + > +- panel or bridge node: see [3]. (mandatory) > + > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt > +[2] Documentation/devicetree/bindings/reset/reset.txt > +[3] Documentation/devicetree/bindings/display/mipi-dsi-bus.txt >
On Fri, Jun 02, 2017 at 04:37:11PM +0200, Philippe CORNU wrote: > This patch adds documentation of device tree bindings for the > Synopsys DesignWare MIPI DSI host DRM bridge driver. > > Signed-off-by: Philippe CORNU <philippe.cornu@st.com> > --- > .../bindings/display/bridge/dw_mipi_dsi.txt | 30 ++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt > > diff --git a/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt > new file mode 100644 > index 0000000..1d7c438 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt > @@ -0,0 +1,30 @@ > +Synopsys DesignWare MIPI DSI host controller > +============================================ > + > +This document defines device tree properties for the Synopsys DesignWare MIPI > +DSI host controller. It doesn't constitue a device tree binding specification > +by itself but is meant to be referenced by platform-specific device tree > +bindings. > + > +When referenced from platform device tree bindings the properties defined in > +this document are defined as follows. The platform device tree bindings are > +responsible for defining whether each property is required or optional. > + > +- reg: Memory mapped base address and length of the DWC MIPI DSI > + registers. (mandatory) > + > +- clocks: References to all the clocks specified in the clock-names property > + as specified in [1]. (mandatory) > + > +- clock-names: "pclk" is peripheral clock for either AHB and APB. (mandatory) Seems strange there's not also a pixel or bit clock? Or this gets driven from the phy? > + > +- resets: References to all the resets specified in the reset-names property > + as specified in [2]. (optional) > + > +- reset-names: string reset name, must be "apb" if used. (optional) > + > +- panel or bridge node: see [3]. (mandatory) > + > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt > +[2] Documentation/devicetree/bindings/reset/reset.txt > +[3] Documentation/devicetree/bindings/display/mipi-dsi-bus.txt > -- > 1.9.1 >
Hi Philippe, Rob, On 06/08/2017 09:10 PM, Rob Herring wrote: > On Fri, Jun 02, 2017 at 04:37:11PM +0200, Philippe CORNU wrote: >> This patch adds documentation of device tree bindings for the >> Synopsys DesignWare MIPI DSI host DRM bridge driver. >> Could you drop "DRM bridge driver" from the subject and commit message and replace it with just "bridge" or "controller". DT bindings shouldn't mention drivers. >> Signed-off-by: Philippe CORNU <philippe.cornu@st.com> >> --- >> .../bindings/display/bridge/dw_mipi_dsi.txt | 30 ++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >> >> diff --git a/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >> new file mode 100644 >> index 0000000..1d7c438 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >> @@ -0,0 +1,30 @@ >> +Synopsys DesignWare MIPI DSI host controller >> +============================================ >> + >> +This document defines device tree properties for the Synopsys DesignWare MIPI >> +DSI host controller. It doesn't constitue a device tree binding specification s/constitue/constitute >> +by itself but is meant to be referenced by platform-specific device tree >> +bindings. >> + >> +When referenced from platform device tree bindings the properties defined in >> +this document are defined as follows. The platform device tree bindings are >> +responsible for defining whether each property is required or optional. >> + >> +- reg: Memory mapped base address and length of the DWC MIPI DSI >> + registers. (mandatory) >> + >> +- clocks: References to all the clocks specified in the clock-names property >> + as specified in [1]. (mandatory) >> + >> +- clock-names: "pclk" is peripheral clock for either AHB and APB. (mandatory) > > Seems strange there's not also a pixel or bit clock? Or this gets driven > from the phy? Since you mention phy here, I wanted to share a concern with the bindings. These bindings don't have a separate PHY DT node. The PHY is assumed as a part of the IP when integrated by a SoC. There are already rockchip and hisil DSI bindings that use this IP but don't define a PHY node. It's a similar situation with the DW-HDMI bindings. For example, when the DW HDMI is integrated in rockchip or renesas SoC, the bindings "rockchip,rk3288-dw-hdmi" or "renesas,r8a7795-dw-hdmi" are used, and they don't have a separate PHY DT node. I wasn't sure whether this is the right way to proceed or not for such IPs. Some advice would help us here. Thanks, Archit > >> + >> +- resets: References to all the resets specified in the reset-names property >> + as specified in [2]. (optional) >> + >> +- reset-names: string reset name, must be "apb" if used. (optional) >> + >> +- panel or bridge node: see [3]. (mandatory) >> + >> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt >> +[2] Documentation/devicetree/bindings/reset/reset.txt >> +[3] Documentation/devicetree/bindings/display/mipi-dsi-bus.txt >> -- >> 1.9.1 >>
Hello, On 09-06-2017 05:11, Archit Taneja wrote: > Hi Philippe, Rob, > > On 06/08/2017 09:10 PM, Rob Herring wrote: >> Seems strange there's not also a pixel or bit clock? Or this >> gets driven >> from the phy? > > Since you mention phy here, I wanted to share a concern with > the bindings. > These bindings don't have a separate PHY DT node. The PHY is > assumed as a > part of the IP when integrated by a SoC. There are already > rockchip and > hisil DSI bindings that use this IP but don't define a PHY node. > > It's a similar situation with the DW-HDMI bindings. > > For example, when the DW HDMI is integrated in rockchip or > renesas SoC, the > bindings "rockchip,rk3288-dw-hdmi" or "renesas,r8a7795-dw-hdmi" > are used, > and they don't have a separate PHY DT node. > > I wasn't sure whether this is the right way to proceed or not > for such IPs. > Some advice would help us here. > > Thanks, > Archit > I just want to add that read/writes from/to phy are done using the controller (in HDMI and in MIPI DSI Host), so the only way to have a phy driver is that if some custom callbacks are provided or if the memory region is shared. Anyway, I agree with Archit in the sense that phy + controller are highly tied. Also, these two "pieces" are SoC specific and sometimes very different between SoC's because they can be customized so I think a different compatible string suits well here. Best regards, Jose Miguel Abreu
On Fri, Jun 09, 2017 at 10:43:07AM +0100, Jose Abreu wrote: > Hello, > > > On 09-06-2017 05:11, Archit Taneja wrote: > > Hi Philippe, Rob, > > > > On 06/08/2017 09:10 PM, Rob Herring wrote: > >> Seems strange there's not also a pixel or bit clock? Or this > >> gets driven > >> from the phy? > > > > Since you mention phy here, I wanted to share a concern with > > the bindings. > > These bindings don't have a separate PHY DT node. The PHY is > > assumed as a > > part of the IP when integrated by a SoC. There are already > > rockchip and > > hisil DSI bindings that use this IP but don't define a PHY node. > > > > It's a similar situation with the DW-HDMI bindings. > > > > For example, when the DW HDMI is integrated in rockchip or > > renesas SoC, the > > bindings "rockchip,rk3288-dw-hdmi" or "renesas,r8a7795-dw-hdmi" > > are used, > > and they don't have a separate PHY DT node. > > > > I wasn't sure whether this is the right way to proceed or not > > for such IPs. > > Some advice would help us here. > > > > Thanks, > > Archit > > > > I just want to add that read/writes from/to phy are done using > the controller (in HDMI and in MIPI DSI Host), so the only way to > have a phy driver is that if some custom callbacks are provided > or if the memory region is shared. > > Anyway, I agree with Archit in the sense that phy + controller > are highly tied. Also, these two "pieces" are SoC specific and > sometimes very different between SoC's because they can be > customized so I think a different compatible string suits well here. When the phy is integrated like this, I agree that it doesn't make sense to have a separate phy node. Rob
On 6/9/2017 6:31 PM, Rob Herring wrote: > On Fri, Jun 09, 2017 at 10:43:07AM +0100, Jose Abreu wrote: >> Hello, >> >> >> On 09-06-2017 05:11, Archit Taneja wrote: >>> Hi Philippe, Rob, >>> >>> On 06/08/2017 09:10 PM, Rob Herring wrote: >>>> Seems strange there's not also a pixel or bit clock? Or this >>>> gets driven >>>> from the phy? >>> >>> Since you mention phy here, I wanted to share a concern with >>> the bindings. >>> These bindings don't have a separate PHY DT node. The PHY is >>> assumed as a >>> part of the IP when integrated by a SoC. There are already >>> rockchip and >>> hisil DSI bindings that use this IP but don't define a PHY node. >>> >>> It's a similar situation with the DW-HDMI bindings. >>> >>> For example, when the DW HDMI is integrated in rockchip or >>> renesas SoC, the >>> bindings "rockchip,rk3288-dw-hdmi" or "renesas,r8a7795-dw-hdmi" >>> are used, >>> and they don't have a separate PHY DT node. >>> >>> I wasn't sure whether this is the right way to proceed or not >>> for such IPs. >>> Some advice would help us here. >>> >>> Thanks, >>> Archit >>> >> >> I just want to add that read/writes from/to phy are done using >> the controller (in HDMI and in MIPI DSI Host), so the only way to >> have a phy driver is that if some custom callbacks are provided >> or if the memory region is shared. >> >> Anyway, I agree with Archit in the sense that phy + controller >> are highly tied. Also, these two "pieces" are SoC specific and >> sometimes very different between SoC's because they can be >> customized so I think a different compatible string suits well here. > > When the phy is integrated like this, I agree that it doesn't make sense > to have a separate phy node. > Great. Thanks for the clarification Jose and Rob. Archit
On 06/08/2017 05:40 PM, Rob Herring wrote: > On Fri, Jun 02, 2017 at 04:37:11PM +0200, Philippe CORNU wrote: >> This patch adds documentation of device tree bindings for the >> Synopsys DesignWare MIPI DSI host DRM bridge driver. >> >> Signed-off-by: Philippe CORNU <philippe.cornu@st.com> >> --- >> .../bindings/display/bridge/dw_mipi_dsi.txt | 30 ++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >> >> diff --git a/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >> new file mode 100644 >> index 0000000..1d7c438 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >> @@ -0,0 +1,30 @@ >> +Synopsys DesignWare MIPI DSI host controller >> +============================================ >> + >> +This document defines device tree properties for the Synopsys DesignWare MIPI >> +DSI host controller. It doesn't constitue a device tree binding specification >> +by itself but is meant to be referenced by platform-specific device tree >> +bindings. >> + >> +When referenced from platform device tree bindings the properties defined in >> +this document are defined as follows. The platform device tree bindings are >> +responsible for defining whether each property is required or optional. >> + >> +- reg: Memory mapped base address and length of the DWC MIPI DSI >> + registers. (mandatory) >> + >> +- clocks: References to all the clocks specified in the clock-names property >> + as specified in [1]. (mandatory) >> + >> +- clock-names: "pclk" is peripheral clock for either AHB and APB. (mandatory) > > Seems strange there's not also a pixel or bit clock? Or this gets driven > from the phy? > Hi Rob, And many thanks for your comments :) There is a "physical" pixel clock entering into the "DSI controller IP" but the "DSI controller driver" does not need to control (or read) it with the dt because this clock information (the frequency) is also available in panel timings and the drm/kms framework will propagate the panel timings in the drm/kms "crtc/encoder/bridge&panel/connector..." chain. Then, the DSI controller driver will compute phy parameters according to these panel timings. Adding a pixel clock dependency in the dt here is then not necessary as the frequency information comes through the panel timings. Philippe >> + >> +- resets: References to all the resets specified in the reset-names property >> + as specified in [2]. (optional) >> + >> +- reset-names: string reset name, must be "apb" if used. (optional) >> + >> +- panel or bridge node: see [3]. (mandatory) >> + >> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt >> +[2] Documentation/devicetree/bindings/reset/reset.txt >> +[3] Documentation/devicetree/bindings/display/mipi-dsi-bus.txt >> -- >> 1.9.1 >>
On Mon, Jun 19, 2017 at 11:51 AM, Philippe CORNU <philippe.cornu@st.com> wrote: > On 06/08/2017 05:40 PM, Rob Herring wrote: >> On Fri, Jun 02, 2017 at 04:37:11PM +0200, Philippe CORNU wrote: >>> This patch adds documentation of device tree bindings for the >>> Synopsys DesignWare MIPI DSI host DRM bridge driver. >>> >>> Signed-off-by: Philippe CORNU <philippe.cornu@st.com> >>> --- >>> .../bindings/display/bridge/dw_mipi_dsi.txt | 30 ++++++++++++++++++++++ >>> 1 file changed, 30 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >>> >>> diff --git a/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >>> new file mode 100644 >>> index 0000000..1d7c438 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >>> @@ -0,0 +1,30 @@ >>> +Synopsys DesignWare MIPI DSI host controller >>> +============================================ >>> + >>> +This document defines device tree properties for the Synopsys DesignWare MIPI >>> +DSI host controller. It doesn't constitue a device tree binding specification >>> +by itself but is meant to be referenced by platform-specific device tree >>> +bindings. >>> + >>> +When referenced from platform device tree bindings the properties defined in >>> +this document are defined as follows. The platform device tree bindings are >>> +responsible for defining whether each property is required or optional. >>> + >>> +- reg: Memory mapped base address and length of the DWC MIPI DSI >>> + registers. (mandatory) >>> + >>> +- clocks: References to all the clocks specified in the clock-names property >>> + as specified in [1]. (mandatory) >>> + >>> +- clock-names: "pclk" is peripheral clock for either AHB and APB. (mandatory) >> >> Seems strange there's not also a pixel or bit clock? Or this gets driven >> from the phy? >> > Hi Rob, > And many thanks for your comments :) > > There is a "physical" pixel clock entering into the "DSI controller IP" > but the "DSI controller driver" does not need to control (or read) it > with the dt because this clock information (the frequency) is also > available in panel timings and the drm/kms framework will propagate the > panel timings in the drm/kms "crtc/encoder/bridge&panel/connector..." > chain. Then, the DSI controller driver will compute phy parameters > according to these panel timings. > Adding a pixel clock dependency in the dt here is then not necessary as > the frequency information comes through the panel timings. Even if the Linux driver doesn't currently need to control the pixel clock it should still be defined in the binding. Bindings are what you physically have, not just what the driver needs or doesn't need. Rob
diff --git a/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt new file mode 100644 index 0000000..1d7c438 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt @@ -0,0 +1,30 @@ +Synopsys DesignWare MIPI DSI host controller +============================================ + +This document defines device tree properties for the Synopsys DesignWare MIPI +DSI host controller. It doesn't constitue a device tree binding specification +by itself but is meant to be referenced by platform-specific device tree +bindings. + +When referenced from platform device tree bindings the properties defined in +this document are defined as follows. The platform device tree bindings are +responsible for defining whether each property is required or optional. + +- reg: Memory mapped base address and length of the DWC MIPI DSI + registers. (mandatory) + +- clocks: References to all the clocks specified in the clock-names property + as specified in [1]. (mandatory) + +- clock-names: "pclk" is peripheral clock for either AHB and APB. (mandatory) + +- resets: References to all the resets specified in the reset-names property + as specified in [2]. (optional) + +- reset-names: string reset name, must be "apb" if used. (optional) + +- panel or bridge node: see [3]. (mandatory) + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt +[2] Documentation/devicetree/bindings/reset/reset.txt +[3] Documentation/devicetree/bindings/display/mipi-dsi-bus.txt
This patch adds documentation of device tree bindings for the Synopsys DesignWare MIPI DSI host DRM bridge driver. Signed-off-by: Philippe CORNU <philippe.cornu@st.com> --- .../bindings/display/bridge/dw_mipi_dsi.txt | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt