Re: [PATCH v3 2/6] dt-bindings: display: Add Synopsys DW MIPI DSI DRM bridge driver

[Rob Herring]

On Mon, Jun 19, 2017 at 11:51 AM, Philippe CORNU  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 
>>> ---
>>>   .../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 000..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/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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/6] dt-bindings: display: Add Synopsys DW MIPI DSI DRM bridge driver

[Philippe CORNU]

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 
>> ---
>>   .../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 000..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/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
>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/6] dt-bindings: display: Add Synopsys DW MIPI DSI DRM bridge driver

[Jose Abreu]

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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/6] dt-bindings: display: Add Synopsys DW MIPI DSI DRM bridge driver

[Archit Taneja]

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

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/6] dt-bindings: display: Add Synopsys DW MIPI DSI DRM bridge driver

[Rob Herring]

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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/6] dt-bindings: display: Add Synopsys DW MIPI DSI DRM bridge driver

[Archit Taneja]

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 
---
  .../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 000..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



--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/6] dt-bindings: display: Add Synopsys DW MIPI DSI DRM bridge driver

[Rob Herring]

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 
> ---
>  .../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 000..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
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/6] dt-bindings: display: Add Synopsys DW MIPI DSI DRM bridge driver

[Neil Armstrong]

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 
> ---
>  .../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 000..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
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 2/6] dt-bindings: display: Add Synopsys DW MIPI DSI DRM bridge driver

[Philippe CORNU]

This patch adds documentation of device tree bindings for the
Synopsys DesignWare MIPI DSI host DRM bridge driver.

Signed-off-by: Philippe CORNU 
---
 .../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 000..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
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel