Re: [PATCH RFC 7/8] dt-bindings: display: add Unisoc's generic mipi panel bindings

[Maxime Ripard]

On Tue, Dec 10, 2019 at 04:36:34PM +0800, Kevin Tang wrote:
> From: Kevin Tang <kevin.tang@xxxxxxxxxx>
>
> Adds generic MIPI panel support for Unisoc's display subsystem.
>
> Cc: Orson Zhai <orsonzhai@xxxxxxxxx>
> Cc: Baolin Wang <baolin.wang@xxxxxxxxxx>
> Cc: Chunyan Zhang <zhang.lyra@xxxxxxxxx>
> Signed-off-by: Kevin Tang <kevin.tang@xxxxxxxxxx>
> ---
>  .../devicetree/bindings/display/sprd/panel.txt     | 110 +++++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/sprd/panel.txt
>
> diff --git a/Documentation/devicetree/bindings/display/sprd/panel.txt b/Documentation/devicetree/bindings/display/sprd/panel.txt
> new file mode 100644
> index 0000000..a4017af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/sprd/panel.txt
> @@ -0,0 +1,110 @@
> +Unisoc Generic MIPI Panel
> +================================================================
> +
> +Required properties:
> +  - compatible: must be "sprd,generic-mipi-panel"
> +  - reg: panel ID
> +  - #address-cells, #size-cells: should be set respectively to <1> and <0>
> +  - port: video port for DPI input
> +  - sprd,dsi-work-mode: the following dsi mode can be select:
> +	0: cmd mode,
> +	1: video burst mode,
> +	2: video non-burst mode with sync pulse,
> +	3: video non-burst mode with sync event
> +  - sprd,dsi-lane-number: number of dsi lanes to use, default is 4
> +  - sprd,dsi-color-format: data format for video stream transmission, currently
> +	supports "rgb888", "rgb666", "rgb666_packed", "rgb565" and "dsc", defaults rgb888
> +  - sprd,phy-bit-clock: the transmission rate of the clock lane for High-Speed,
> +	the unit is Kbps, and the default value is 500Mbps
> +  - sprd,phy-escape-clock: the transmission rate of the clock lane for Low-Power,
> +	the unit is Kbps, and the default value is 20Mbps
> +
> +
> +Optional properties:
> +  - width-mm: see [2] for details
> +  - height-mm: see [2] for details
> +  - sprd,esd-check-enable: esd check function enable switch
> +  - sprd,esd-check-mode: esd detection method, default is register
> +	0: register,
> +	1: TE signal
> +  - sprd,esd-check-period: esd detection cycle, unit ms, default 1000ms
> +  - sprd,esd-check-register: if register detection is used, this attribute must be configured
> +  - sprd,esd-check-value: if register detection is used, this attribute must be configured
> +  - sprd,reset-on-sequence: timing of the reset pin when the lcd power on
> +	<1 5>, <0 5> means first keep high for 5ms, then keep low for 5ms
> +  - sprd,reset-on-sequence: timing of the reset pin when the lcd power off
> +  - sprd,use-dcs-write: bool attribute, indicating whether to use the dcs to send inital & sleep cmds,
> +	default use generic
> +  - sprd,initial-command: lcd initialization command set
> +  - sprd,sleep-in-command: lcd suspend command set
> +  - sprd,sleep-out-command: lcd resume command set
> +  - display-timings: see [1] for details

I can't say for sure since I'm not the panel maintainer, but I'm not
sure it's something that we want.

Panels are much more complicated than that, and DT is usually to store
data, not code (unlike the initial-command property you have).

The best way to support this would be to use the panel infrastructure.

> +
> +	[1] Documentation/devicetree/bindings/display/panel/display-timing.txt
> +	[2] Documentation/devicetree/bindings/display/panel/panel-common.yaml
> +
> +Example
> +-------
> +
> +Panel specific DT entry:
> +
> +	&dsi {
> +		panel {
> +			compatible = "sprd,generic-mipi-panel";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0>;
> +
> +			port {
> +				reg = <1>;
> +				panel_in: endpoint {
> +					remote-endpoint = <&dphy_out>;
> +				};
> +			};
> +		};
> +	};
> +
> +	/ { lcds {
> +        lcd_mipi_hd: lcd_mipi_hd {
> +                sprd,dsi-work-mode = <1>;
> +                sprd,dsi-lane-number = <4>;
> +                sprd,dsi-color-format = "rgb888";
> +                sprd,phy-bit-clock = <1100000>;
> +                sprd,phy-escape-clock = <20000>;
> +                width-mm = <68>;
> +                height-mm = <121>;
> +                sprd,esd-check-enable = <0>;
> +                sprd,esd-check-mode = <0>;
> +                sprd,esd-check-period = <1000>;
> +                sprd,esd-check-register = <0x0A>;
> +                sprd,esd-check-value = <0x9C>;
> +                sprd,reset-on-sequence = <1 5>, <0 5>, <1 20>;
> +                sprd,reset-off-sequence = <0 5>;
> +                sprd,use-dcs-write;
> +                sprd,initial-command = [
> +                        39 00 00 02 b0 00
> +                        39 00 00 04 B3 31 00 06
> +                        ];
> +                sprd,sleep-in-command = [
> +                        13 0A 00 01 28
> +                        13 78 00 01 10
> +                        ];
> +                sprd,sleep-out-command = [
> +                        13 78 00 01 11
> +                        13 32 00 01 29
> +                        ];
> +                display-timings {
> +                        timing0 {
> +                                clock-frequency = <64000000>;
> +                                hactive = <720>;
> +                                vactive = <1280>;
> +                                hback-porch = <31>;
> +                                hfront-porch = <31>;
> +                                vback-porch = <32>;
> +                                vfront-porch = <16>;
> +                                hsync-len = <20>;
> +                                vsync-len = <2>;
> +                        };
> +                };
> +        };

This example doesn't match the binding described above?

Thanks!
Maxime

Attachment: signature.asc
Description: PGP signature