[v10,2/4] Documentation/dt: Add descriptions for loongson display controller

Commit Message

Sui Jingfeng Feb. 20, 2022, 2:55 p.m. UTC

From: suijingfeng <[e-mail]>

Add DT documentation for loongson display controller found in LS2K1000,
LS2K0500 and LS7A1000.

v2: DT binding docs and includes should be a separate patch,
    fix a warnning because of that.

v3: split dt-bindings from other changes into a separate patch.

v4: fix warnings and errors when running make dt_binding_check

Reported-by: Rob Herring <[e-mail]>
Reported-by: Krzysztof Kozlowski <[e-mail]>
Signed-off-by: suijingfeng <[e-mail]>
Signed-off-by: Sui Jingfeng <[e-mail]>
---
 .../loongson/loongson,display-controller.yaml | 122 ++++++++++++++++++
 1 file changed, 122 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml

Comments

Jiaxun Yang Feb. 20, 2022, 6:22 p.m. UTC | #1

在 2022/2/20 14:55, Sui Jingfeng 写道:
> From: suijingfeng <[e-mail]>
>
> Add DT documentation for loongson display controller found in LS2K1000,
> LS2K0500 and LS7A1000.
>
> v2: DT binding docs and includes should be a separate patch,
>      fix a warnning because of that.
>
> v3: split dt-bindings from other changes into a separate patch.
>
> v4: fix warnings and errors when running make dt_binding_check
>
> Reported-by: Rob Herring <[e-mail]>
> Reported-by: Krzysztof Kozlowski <[e-mail]>
> Signed-off-by: suijingfeng <[e-mail]>
> Signed-off-by: Sui Jingfeng <[e-mail]>
> ---
>   .../loongson/loongson,display-controller.yaml | 122 ++++++++++++++++++
>   1 file changed, 122 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml b/Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml
> new file mode 100644
> index 000000000000..ee1a59b91943
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml
> @@ -0,0 +1,122 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/loongson/loongson,display-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson LS7A1000/LS2K1000/LS2K0500 Display Controller Device Tree Bindings
> +
> +maintainers:
> +  - Sui Jingfeng <[e-mail]>
> +
> +description: |+
> +
> +  Loongson display controllers are simple which require scanout buffers
> +  to be physically contiguous. LS2K1000/LS2K0500 is a SOC, only system
> +  memory is available. LS7A1000/LS7A2000 is bridge chip which is equipped
> +  with a dedicated video ram which is 64MB or more.
> +
> +  For LS7A1000, there are 4 dedicated GPIOs whose control register is
> +  located at the DC register space. They are used to emulate two way i2c,
> +  One for DVO0, another for DVO1.
> +
> +  LS2K1000 and LS2K0500 SoC grab i2c adapter from other module, either
> +  general purpose GPIO emulated i2c or hardware i2c in the SoC.
> +
> +  LSDC has two display pipes, each way has a DVO interface which provide
> +  RGB888 signals, vertical & horizontal synchronisations, data enable and
> +  the pixel clock. LSDC has two CRTC, each CRTC is able to scanout from
> +  1920x1080 resolution at 60Hz. Each CRTC has two FB address registers.
> +
> +  LSDC's display pipeline have several components as below description,
> +
> +  The display controller in LS7A1000:
> +    ___________________                                     _________
> +    |            -------|                                   |         |
> +    |  CRTC0 --> | DVO0 ----> Encoder0 ---> Connector0 ---> | Monitor |
> +    |  _   _     -------|        ^             ^            |_________|
> +    | | | | |    -------|        |             |
> +    | |_| |_|    | i2c0 <--------+-------------+
> +    |            -------|
> +    |   DC IN LS7A1000  |
> +    |  _   _     -------|
> +    | | | | |    | i2c1 <--------+-------------+
> +    | |_| |_|    -------|        |             |             _________
> +    |            -------|        |             |            |         |
> +    |  CRTC1 --> | DVO1 ----> Encoder1 ---> Connector1 ---> |  Panel  |
> +    |            -------|                                   |_________|
> +    |___________________|
> +
> +  Simple usage of LS7A1000 with LS3A4000 CPU:
> +
> +    +------+            +-----------------------------------+
> +    | DDR4 |            |  +-------------------+            |
> +    +------+            |  | PCIe Root complex |   LS7A1000 |
> +       || MC0           |  +--++---------++----+            |
> +  +----------+  HT 3.0  |     ||         ||                 |
> +  | LS3A4000 |<-------->| +---++---+  +--++--+    +---------+   +------+
> +  |   CPU    |<-------->| | GC1000 |  | LSDC |<-->| DDR3 MC |<->| VRAM |
> +  +----------+          | +--------+  +-+--+-+    +---------+   +------+
> +       || MC1           +---------------|--|----------------+
> +    +------+                            |  |
> +    | DDR4 |          +-------+   DVO0  |  |  DVO1   +------+
> +    +------+   VGA <--|ADV7125|<--------+  +-------->|TFP410|--> DVI/HDMI
> +                      +-------+                      +------+
> +
> +  The display controller in LS2K1000/LS2K0500:
> +     ___________________                                     _________
> +    |            -------|                                   |         |
> +    |  CRTC0 --> | DVO0 ----> Encoder0 ---> Connector0 ---> | Monitor |
> +    |  _   _     -------|        ^              ^           |_________|
> +    | | | | |           |        |              |
> +    | |_| |_|           |     +------+          |
> +    |                   <---->| i2c0 |<---------+
> +    |   DC IN LS2K1000  |     +------+
> +    |  _   _            |     +------+
> +    | | | | |           <---->| i2c1 |----------+
> +    | |_| |_|           |     +------+          |            _________
> +    |            -------|        |              |           |         |
> +    |  CRTC1 --> | DVO1 ----> Encoder1 ---> Connector1 ---> |  Panel  |
> +    |            -------|                                   |_________|
> +    |___________________|
> +
> +properties:
> +  $nodename:
> +    pattern: "^display-controller@[0-9a-f],[0-9a-f]$"
> +
> +  compatible:
> +    enum:
> +      - loongson,ls7a1000-dc
> +      - loongson,ls2k1000-dc
> +      - loongson,ls2k0500-dc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
Given that it is possible to have output subnodes I guess 
additionalProperties
should be allowed?

Thanks.
- Jiaxun
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    bus {
> +
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +        #interrupt-cells = <2>;
> +
> +        lsdc: display-controller@6,1 {
> +            compatible = "loongson,ls7a1000-dc";
> +            reg = <0x3100 0x0 0x0 0x0 0x0>;
> +            interrupts = <28 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-parent = <&pic>;
> +        };
> +    };
> +...

Krzysztof Kozlowski Feb. 21, 2022, 7:31 a.m. UTC | #2

On 20/02/2022 19:22, Jiaxun Yang wrote:
> 
> 
> 在 2022/2/20 14:55, Sui Jingfeng 写道:
>> From: suijingfeng <[e-mail]>
>>
>> Add DT documentation for loongson display controller found in LS2K1000,
>> LS2K0500 and LS7A1000.
>>
>> v2: DT binding docs and includes should be a separate patch,
>>      fix a warnning because of that.
>>
>> v3: split dt-bindings from other changes into a separate patch.
>>
>> v4: fix warnings and errors when running make dt_binding_check
>>
>> Reported-by: Rob Herring <[e-mail]>
>> Reported-by: Krzysztof Kozlowski <[e-mail]>
>> Signed-off-by: suijingfeng <[e-mail]>
>> Signed-off-by: Sui Jingfeng <[e-mail]>
>> ---
>>   .../loongson/loongson,display-controller.yaml | 122 ++++++++++++++++++
>>   1 file changed, 122 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml b/Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml
>> new file mode 100644
>> index 000000000000..ee1a59b91943
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml
>> @@ -0,0 +1,122 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/loongson/loongson,display-controller.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Loongson LS7A1000/LS2K1000/LS2K0500 Display Controller Device Tree Bindings
>> +
>> +maintainers:
>> +  - Sui Jingfeng <[e-mail]>
>> +
>> +description: |+
>> +
>> +  Loongson display controllers are simple which require scanout buffers
>> +  to be physically contiguous. LS2K1000/LS2K0500 is a SOC, only system
>> +  memory is available. LS7A1000/LS7A2000 is bridge chip which is equipped
>> +  with a dedicated video ram which is 64MB or more.
>> +
>> +  For LS7A1000, there are 4 dedicated GPIOs whose control register is
>> +  located at the DC register space. They are used to emulate two way i2c,
>> +  One for DVO0, another for DVO1.
>> +
>> +  LS2K1000 and LS2K0500 SoC grab i2c adapter from other module, either
>> +  general purpose GPIO emulated i2c or hardware i2c in the SoC.
>> +
>> +  LSDC has two display pipes, each way has a DVO interface which provide
>> +  RGB888 signals, vertical & horizontal synchronisations, data enable and
>> +  the pixel clock. LSDC has two CRTC, each CRTC is able to scanout from
>> +  1920x1080 resolution at 60Hz. Each CRTC has two FB address registers.
>> +
>> +  LSDC's display pipeline have several components as below description,
>> +
>> +  The display controller in LS7A1000:
>> +    ___________________                                     _________
>> +    |            -------|                                   |         |
>> +    |  CRTC0 --> | DVO0 ----> Encoder0 ---> Connector0 ---> | Monitor |
>> +    |  _   _     -------|        ^             ^            |_________|
>> +    | | | | |    -------|        |             |
>> +    | |_| |_|    | i2c0 <--------+-------------+
>> +    |            -------|
>> +    |   DC IN LS7A1000  |
>> +    |  _   _     -------|
>> +    | | | | |    | i2c1 <--------+-------------+
>> +    | |_| |_|    -------|        |             |             _________
>> +    |            -------|        |             |            |         |
>> +    |  CRTC1 --> | DVO1 ----> Encoder1 ---> Connector1 ---> |  Panel  |
>> +    |            -------|                                   |_________|
>> +    |___________________|
>> +
>> +  Simple usage of LS7A1000 with LS3A4000 CPU:
>> +
>> +    +------+            +-----------------------------------+
>> +    | DDR4 |            |  +-------------------+            |
>> +    +------+            |  | PCIe Root complex |   LS7A1000 |
>> +       || MC0           |  +--++---------++----+            |
>> +  +----------+  HT 3.0  |     ||         ||                 |
>> +  | LS3A4000 |<-------->| +---++---+  +--++--+    +---------+   +------+
>> +  |   CPU    |<-------->| | GC1000 |  | LSDC |<-->| DDR3 MC |<->| VRAM |
>> +  +----------+          | +--------+  +-+--+-+    +---------+   +------+
>> +       || MC1           +---------------|--|----------------+
>> +    +------+                            |  |
>> +    | DDR4 |          +-------+   DVO0  |  |  DVO1   +------+
>> +    +------+   VGA <--|ADV7125|<--------+  +-------->|TFP410|--> DVI/HDMI
>> +                      +-------+                      +------+
>> +
>> +  The display controller in LS2K1000/LS2K0500:
>> +     ___________________                                     _________
>> +    |            -------|                                   |         |
>> +    |  CRTC0 --> | DVO0 ----> Encoder0 ---> Connector0 ---> | Monitor |
>> +    |  _   _     -------|        ^              ^           |_________|
>> +    | | | | |           |        |              |
>> +    | |_| |_|           |     +------+          |
>> +    |                   <---->| i2c0 |<---------+
>> +    |   DC IN LS2K1000  |     +------+
>> +    |  _   _            |     +------+
>> +    | | | | |           <---->| i2c1 |----------+
>> +    | |_| |_|           |     +------+          |            _________
>> +    |            -------|        |              |           |         |
>> +    |  CRTC1 --> | DVO1 ----> Encoder1 ---> Connector1 ---> |  Panel  |
>> +    |            -------|                                   |_________|
>> +    |___________________|
>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^display-controller@[0-9a-f],[0-9a-f]$"
>> +
>> +  compatible:
>> +    enum:
>> +      - loongson,ls7a1000-dc
>> +      - loongson,ls2k1000-dc
>> +      - loongson,ls2k0500-dc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +
>> +additionalProperties: false
> Given that it is possible to have output subnodes I guess 
> additionalProperties
> should be allowed?

subnodes should be instead listed. Either with strict name (if these are
e.g. ports) or with some more or less relaxed pattern.


Best regards,
Krzysztof

Rob Herring (Arm) Feb. 22, 2022, 11:02 p.m. UTC | #3

On Sun, Feb 20, 2022 at 10:55:52PM +0800, Sui Jingfeng wrote:
> From: suijingfeng <[e-mail]>

Follow the conventions of the subsystem for patch subjects. It should be 
evident with 'git log --oneline Documentation/devicetree/bindings/display'.

Something like this:

dt-bindings: display: Add Loongson display controller

> 
> Add DT documentation for loongson display controller found in LS2K1000,
> LS2K0500 and LS7A1000.
> 
> v2: DT binding docs and includes should be a separate patch,
>     fix a warnning because of that.
> 
> v3: split dt-bindings from other changes into a separate patch.
> 
> v4: fix warnings and errors when running make dt_binding_check

What happened in versions 5-10? You've sent 10 versions in 3 weeks. 
That's not a rate that gives people time to review. Also, it looks like 
all the changes you've made are either documented things to do or not do 
or are errors the tools (checkpatch.pl, 'make dt_binding_check') will 
tell you to fix. 

>
> Reported-by: Rob Herring <[e-mail]>
> Reported-by: Krzysztof Kozlowski <[e-mail]>

What did we report? That's generally for fixes.

> Signed-off-by: suijingfeng <[e-mail]>
> Signed-off-by: Sui Jingfeng <[e-mail]>
> ---
>  .../loongson/loongson,display-controller.yaml | 122 ++++++++++++++++++
>  1 file changed, 122 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml b/Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml
> new file mode 100644
> index 000000000000..ee1a59b91943
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml
> @@ -0,0 +1,122 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/loongson/loongson,display-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson LS7A1000/LS2K1000/LS2K0500 Display Controller Device Tree Bindings
> +
> +maintainers:
> +  - Sui Jingfeng <[e-mail]>
> +
> +description: |+
> +
> +  Loongson display controllers are simple which require scanout buffers
> +  to be physically contiguous. LS2K1000/LS2K0500 is a SOC, only system
> +  memory is available. LS7A1000/LS7A2000 is bridge chip which is equipped
> +  with a dedicated video ram which is 64MB or more.
> +
> +  For LS7A1000, there are 4 dedicated GPIOs whose control register is
> +  located at the DC register space. They are used to emulate two way i2c,
> +  One for DVO0, another for DVO1.
> +
> +  LS2K1000 and LS2K0500 SoC grab i2c adapter from other module, either
> +  general purpose GPIO emulated i2c or hardware i2c in the SoC.
> +
> +  LSDC has two display pipes, each way has a DVO interface which provide
> +  RGB888 signals, vertical & horizontal synchronisations, data enable and
> +  the pixel clock. LSDC has two CRTC, each CRTC is able to scanout from
> +  1920x1080 resolution at 60Hz. Each CRTC has two FB address registers.
> +
> +  LSDC's display pipeline have several components as below description,
> +
> +  The display controller in LS7A1000:
> +    ___________________                                     _________
> +    |            -------|                                   |         |
> +    |  CRTC0 --> | DVO0 ----> Encoder0 ---> Connector0 ---> | Monitor |
> +    |  _   _     -------|        ^             ^            |_________|
> +    | | | | |    -------|        |             |
> +    | |_| |_|    | i2c0 <--------+-------------+
> +    |            -------|
> +    |   DC IN LS7A1000  |
> +    |  _   _     -------|
> +    | | | | |    | i2c1 <--------+-------------+
> +    | |_| |_|    -------|        |             |             _________
> +    |            -------|        |             |            |         |
> +    |  CRTC1 --> | DVO1 ----> Encoder1 ---> Connector1 ---> |  Panel  |
> +    |            -------|                                   |_________|
> +    |___________________|
> +
> +  Simple usage of LS7A1000 with LS3A4000 CPU:
> +
> +    +------+            +-----------------------------------+
> +    | DDR4 |            |  +-------------------+            |
> +    +------+            |  | PCIe Root complex |   LS7A1000 |
> +       || MC0           |  +--++---------++----+            |
> +  +----------+  HT 3.0  |     ||         ||                 |
> +  | LS3A4000 |<-------->| +---++---+  +--++--+    +---------+   +------+
> +  |   CPU    |<-------->| | GC1000 |  | LSDC |<-->| DDR3 MC |<->| VRAM |
> +  +----------+          | +--------+  +-+--+-+    +---------+   +------+

How do you know how much VRAM you have? That's going to need a binding 
for the VRAM.

> +       || MC1           +---------------|--|----------------+
> +    +------+                            |  |
> +    | DDR4 |          +-------+   DVO0  |  |  DVO1   +------+
> +    +------+   VGA <--|ADV7125|<--------+  +-------->|TFP410|--> DVI/HDMI
> +                      +-------+                      +------+
> +
> +  The display controller in LS2K1000/LS2K0500:
> +     ___________________                                     _________
> +    |            -------|                                   |         |
> +    |  CRTC0 --> | DVO0 ----> Encoder0 ---> Connector0 ---> | Monitor |
> +    |  _   _     -------|        ^              ^           |_________|
> +    | | | | |           |        |              |
> +    | |_| |_|           |     +------+          |
> +    |                   <---->| i2c0 |<---------+
> +    |   DC IN LS2K1000  |     +------+
> +    |  _   _            |     +------+
> +    | | | | |           <---->| i2c1 |----------+
> +    | |_| |_|           |     +------+          |            _________
> +    |            -------|        |              |           |         |
> +    |  CRTC1 --> | DVO1 ----> Encoder1 ---> Connector1 ---> |  Panel  |
> +    |            -------|                                   |_________|
> +    |___________________|
> +
> +properties:
> +  $nodename:
> +    pattern: "^display-controller@[0-9a-f],[0-9a-f]$"
> +
> +  compatible:
> +    enum:
> +      - loongson,ls7a1000-dc
> +      - loongson,ls2k1000-dc
> +      - loongson,ls2k0500-dc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    bus {
> +
> +        #address-cells = <3>;
> +        #size-cells = <2>;
> +        #interrupt-cells = <2>;
> +
> +        lsdc: display-controller@6,1 {
> +            compatible = "loongson,ls7a1000-dc";
> +            reg = <0x3100 0x0 0x0 0x0 0x0>;
> +            interrupts = <28 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-parent = <&pic>;

Don't need 'interrupt-parent' in examples.

This certainly looks incomplete given you have 2 outputs you are going 
to need to describe what's connected to them.

> +        };
> +    };
> +...
> -- 
> 2.25.1
> 
>

Sui Jingfeng Feb. 23, 2022, 1:56 p.m. UTC | #4

> -----Original Messages-----
&gt; From: "Rob Herring" <[e-mail]>
&gt; Sent Time: 2022-02-23 07:02:34 (Wednesday)
&gt; To: "Sui Jingfeng" &lt;[e-mail]&gt;
&gt; Cc: "Maxime Ripard" <[e-mail]>, "Thomas Zimmermann" <[e-mail]>, "Roland Scheidegger" <[e-mail]>, "Zack Rusin" <[e-mail]>, "Christian Gmeiner" <[e-mail]>, "David Airlie" <[e-mail]>, "Daniel Vetter" <[e-mail]>, "Thomas Bogendoerfer" <[e-mail]>, "Dan Carpenter" <[e-mail]>, "Krzysztof Kozlowski" <[e-mail]>, "Andrey Zhizhikin" <[e-mail]>, "Sam Ravnborg" <[e-mail]>, "David S . Miller" <[e-mail]>, "Jiaxun Yang" <[e-mail]>, "Lucas Stach" <[e-mail]>, "Maarten Lankhorst" <[e-mail]>, "Ilia Mirkin" <[e-mail]>, "Qing Zhang" <[e-mail]>, suijingfeng <[e-mail]>, [e-mail], [e-mail], [e-mail], [e-mail]
&gt; Subject: Re: [PATCH v10 2/4] Documentation/dt: Add descriptions for loongson display controller
&gt; 
&gt; On Sun, Feb 20, 2022 at 10:55:52PM +0800, Sui Jingfeng wrote:
&gt; &gt; From: suijingfeng <[e-mail]>
&gt; 
&gt; Follow the conventions of the subsystem for patch subjects. It should be 
&gt; evident with 'git log --oneline Documentation/devicetree/bindings/display'.
&gt; 
&gt; Something like this:
&gt; 
&gt; dt-bindings: display: Add Loongson display controller
&gt; 

Hi, 

We are not a platform device driver, there is no
of_device_id defined in my driver. In other word, 
my driver will not bind against devices whose compatible
is "loongson,ls7a1000-dc". We just parse the device tree
actively, find necessary information of interest. 
In this case, can I use the word "dt-bindings" in the commit title? 

I want to follow the conventions, but get some push back,
Krzysztof say that he can not see any bindings, these are not bindings.</[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]>

本邮件及其附件含有龙芯中科的商业秘密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制或散发）本邮件及其附件中的信息。如果您错收本邮件，请您立即电话或邮件通知发件人并删除本邮件。 
This email and its attachments contain confidential information from Loongson Technology , which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this email in error, please notify the sender by phone or email immediately and delete it.

Sui Jingfeng Feb. 23, 2022, 3:35 p.m. UTC | #5

On 2022/2/23 21:56, 隋景峰 wrote:
> Something like this:
>   
> dt-bindings: display: Add Loongson display controller

Hi,

We are not a platform device driver, there is no
of_device_id defined in my driver. In other word,
my driver will not bind against devices whose compatible
is "loongson,ls7a1000-dc". We just parse the device tree
actively, find necessary information of interest.

What's the meaning of dt-bindings by definition ?
In this case, can I use the word "dt-bindings" in the commit title?

I want to follow the conventions, but get some push back,
Krzysztof say that he can not see any bindings, these are not bindings.

Krzysztof Kozlowski Feb. 23, 2022, 3:40 p.m. UTC | #6

On 23/02/2022 14:56, 隋景峰 wrote:
> 
> 
> 
> > -----Original Messages-----
> &gt; From: "Rob Herring" <[e-mail]>
> &gt; Sent Time: 2022-02-23 07:02:34 (Wednesday)
> &gt; To: "Sui Jingfeng" &lt;[e-mail]&gt;
> &gt; Cc: "Maxime Ripard" <[e-mail]>, "Thomas Zimmermann" <[e-mail]>, "Roland Scheidegger" <[e-mail]>, "Zack Rusin" <[e-mail]>, "Christian Gmeiner" <[e-mail]>, "David Airlie" <[e-mail]>, "Daniel Vetter" <[e-mail]>, "Thomas Bogendoerfer" <[e-mail]>, "Dan Carpenter" <[e-mail]>, "Krzysztof Kozlowski" <[e-mail]>, "Andrey Zhizhikin" <[e-mail]>, "Sam Ravnborg" <[e-mail]>, "David S . Miller" <[e-mail]>, "Jiaxun Yang" <[e-mail]>, "Lucas Stach" <[e-mail]>, "Maarten Lankhorst" <[e-mail]>, "Ilia Mirkin" <[e-mail]>, "Qing Zhang" <[e-mail]>, suijingfeng <[e-mail]>, [e-mail], [e-mail], [e-mail], [e-mail]
> &gt; Subject: Re: [PATCH v10 2/4] Documentation/dt: Add descriptions for loongson display controller
> &gt; 
> &gt; On Sun, Feb 20, 2022 at 10:55:52PM +0800, Sui Jingfeng wrote:
> &gt; &gt; From: suijingfeng <[e-mail]>
> &gt; 
> &gt; Follow the conventions of the subsystem for patch subjects. It should be 
> &gt; evident with 'git log --oneline Documentation/devicetree/bindings/display'.
> &gt; 
> &gt; Something like this:
> &gt; 
> &gt; dt-bindings: display: Add Loongson display controller
> &gt; 
> 
> Hi, 
> 
> We are not a platform device driver, there is no
> of_device_id defined in my driver. In other word, 
> my driver will not bind against devices whose compatible
> is "loongson,ls7a1000-dc". We just parse the device tree
> actively, find necessary information of interest. 
> In this case, can I use the word "dt-bindings" in the commit title?

This is a patch for specific subsystem, so as Rob said, it should follow
subsystem conventions.

The patch itself is a dt-bindings patch, so there is nothing here
special which would encourage for any exception.


> 
> I want to follow the conventions, but get some push back,
> Krzysztof say that he can not see any bindings, these are not bindings.

I said in comment to your patch with DTS, which you called bindings,
that there are no bindings at all in it. Because in your patch with DTS
you did not include bindings, but you called it bindings.

Here, this is a patch with bindings, so your comment "these are not
bindings" is not true.


</[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]></[e-mail]>

This link does not work...

> This email and its attachments contain confidential information from Loongson Technology , which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this email in error, please notify the sender by phone or email immediately and delete it. 

Such automatic footers do not help. Could you work on a way to avoid them?



Best regards,
Krzysztof

Krzysztof Kozlowski Feb. 23, 2022, 3:44 p.m. UTC | #7

On 23/02/2022 16:35, Sui Jingfeng wrote:
> 
> On 2022/2/23 21:56, 隋景峰 wrote:
>> Something like this:
>>   
>> dt-bindings: display: Add Loongson display controller
> 
> Hi,

Thanks for resending in a proper format. I already replied to your
original post, so let me paste it here as well.

> 
> We are not a platform device driver, there is no
> of_device_id defined in my driver. In other word,
> my driver will not bind against devices whose compatible
> is "loongson,ls7a1000-dc". We just parse the device tree
> actively, find necessary information of interest.
> 
> What's the meaning of dt-bindings by definition ?
> In this case, can I use the word "dt-bindings" in the commit title?

This is a patch for specific subsystem, so as Rob said, it should follow
subsystem conventions.

The patch itself is a dt-bindings patch, so there is nothing here
special which would encourage for any exception.

> 
> I want to follow the conventions, but get some push back,
> Krzysztof say that he can not see any bindings, these are not bindings.

I said in comment to your patch with DTS, which you called bindings,
that there are no bindings at all in it. Because in your patch with DTS
you did not include bindings, but you called it bindings.

Here, this is a patch with bindings, so your comment "these are not
bindings" is not true.

Best regards,
Krzysztof

Patch

diff --git a/Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml b/Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml
new file mode 100644
index 000000000000..ee1a59b91943
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml
@@ -0,0 +1,122 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/loongson/loongson,display-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Loongson LS7A1000/LS2K1000/LS2K0500 Display Controller Device Tree Bindings
+
+maintainers:
+  - Sui Jingfeng <[e-mail]>
+
+description: |+
+
+  Loongson display controllers are simple which require scanout buffers
+  to be physically contiguous. LS2K1000/LS2K0500 is a SOC, only system
+  memory is available. LS7A1000/LS7A2000 is bridge chip which is equipped
+  with a dedicated video ram which is 64MB or more.
+
+  For LS7A1000, there are 4 dedicated GPIOs whose control register is
+  located at the DC register space. They are used to emulate two way i2c,
+  One for DVO0, another for DVO1.
+
+  LS2K1000 and LS2K0500 SoC grab i2c adapter from other module, either
+  general purpose GPIO emulated i2c or hardware i2c in the SoC.
+
+  LSDC has two display pipes, each way has a DVO interface which provide
+  RGB888 signals, vertical & horizontal synchronisations, data enable and
+  the pixel clock. LSDC has two CRTC, each CRTC is able to scanout from
+  1920x1080 resolution at 60Hz. Each CRTC has two FB address registers.
+
+  LSDC's display pipeline have several components as below description,
+
+  The display controller in LS7A1000:
+    ___________________                                     _________
+    |            -------|                                   |         |
+    |  CRTC0 --> | DVO0 ----> Encoder0 ---> Connector0 ---> | Monitor |
+    |  _   _     -------|        ^             ^            |_________|
+    | | | | |    -------|        |             |
+    | |_| |_|    | i2c0 <--------+-------------+
+    |            -------|
+    |   DC IN LS7A1000  |
+    |  _   _     -------|
+    | | | | |    | i2c1 <--------+-------------+
+    | |_| |_|    -------|        |             |             _________
+    |            -------|        |             |            |         |
+    |  CRTC1 --> | DVO1 ----> Encoder1 ---> Connector1 ---> |  Panel  |
+    |            -------|                                   |_________|
+    |___________________|
+
+  Simple usage of LS7A1000 with LS3A4000 CPU:
+
+    +------+            +-----------------------------------+
+    | DDR4 |            |  +-------------------+            |
+    +------+            |  | PCIe Root complex |   LS7A1000 |
+       || MC0           |  +--++---------++----+            |
+  +----------+  HT 3.0  |     ||         ||                 |
+  | LS3A4000 |<-------->| +---++---+  +--++--+    +---------+   +------+
+  |   CPU    |<-------->| | GC1000 |  | LSDC |<-->| DDR3 MC |<->| VRAM |
+  +----------+          | +--------+  +-+--+-+    +---------+   +------+
+       || MC1           +---------------|--|----------------+
+    +------+                            |  |
+    | DDR4 |          +-------+   DVO0  |  |  DVO1   +------+
+    +------+   VGA <--|ADV7125|<--------+  +-------->|TFP410|--> DVI/HDMI
+                      +-------+                      +------+
+
+  The display controller in LS2K1000/LS2K0500:
+     ___________________                                     _________
+    |            -------|                                   |         |
+    |  CRTC0 --> | DVO0 ----> Encoder0 ---> Connector0 ---> | Monitor |
+    |  _   _     -------|        ^              ^           |_________|
+    | | | | |           |        |              |
+    | |_| |_|           |     +------+          |
+    |                   <---->| i2c0 |<---------+
+    |   DC IN LS2K1000  |     +------+
+    |  _   _            |     +------+
+    | | | | |           <---->| i2c1 |----------+
+    | |_| |_|           |     +------+          |            _________
+    |            -------|        |              |           |         |
+    |  CRTC1 --> | DVO1 ----> Encoder1 ---> Connector1 ---> |  Panel  |
+    |            -------|                                   |_________|
+    |___________________|
+
+properties:
+  $nodename:
+    pattern: "^display-controller@[0-9a-f],[0-9a-f]$"
+
+  compatible:
+    enum:
+      - loongson,ls7a1000-dc
+      - loongson,ls2k1000-dc
+      - loongson,ls2k0500-dc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    bus {
+
+        #address-cells = <3>;
+        #size-cells = <2>;
+        #interrupt-cells = <2>;
+
+        lsdc: display-controller@6,1 {
+            compatible = "loongson,ls7a1000-dc";
+            reg = <0x3100 0x0 0x0 0x0 0x0>;
+            interrupts = <28 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-parent = <&pic>;
+        };
+    };
+...