Hi GVRao, Thank you for the patch. On Tue, Aug 11, 2020 at 06:16:16AM +0530, Venkateshwar Rao Gannavarapu wrote: > The Xilinx MIPI DSI (Display Serial Interface) Transmitter subsystem > implements the Mobile Industry Processor Interface (MIPI) based display > interface. It supports the interface with programmable logic (FPGA). > > Signed-off-by: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xxxxxxxxxx> > --- > .../devicetree/bindings/display/xlnx/xlnx,dsi.yaml | 147 +++++++++++++++++++++ > 1 file changed, 147 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi.yaml > > diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi.yaml b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi.yaml > new file mode 100644 > index 0000000..73da0d8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi.yaml > @@ -0,0 +1,147 @@ > +# SPDX-License-Identifier: GPL-2.0 New bindings must be licensed as (GPL-2.0-only OR BSD-2-Clause). > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/xlnx/xlnx,dsi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Xilinx Programmable DSI-TX Subsystem > + > +description: | > + The programmable MIPI DSI controller of Xilinx implements display pipeline > + based on DSI v1.3 specification. The subsystem includes multiple functional > + blocks as below > + > + +---------------+ +-----------------------------------------------+ > + | | | +------------------+ | > + | | | v----------->+AXI CROSBAR |XXX | > + |FRAME BUFFER | AXI STREAM | | | X X | > + |(DMA) | +------->+ +------------+ +------------------+ XX | > + | +<------+ | |MIPI | X | > + | | | |DSI-TX | X | > + | | | |Controller | +------------+ | > + | | | | +------------->D-PHY | | > + +---------------+ | | | | | | > + S_AXIS_ACLK | +-------------<------------+ | | > + +---------------->+ | | | > + | | | | > + DPHY_CLK_200M | | | | > + +---------------->+ | | | > + + | +------------+ | > + | | > + | MIPI DSI TX SUBSYSTEM | > + +-----------------------------------------------+ > + > + The DSI TX controller consists of multiple layers such as lane management layer, > + low-level protocol and pixel-to-byte conversion. The DSI TX controller core > + receives stream of image data through an input stream interface. The subsystem > + driver supports upto 4 lane support and generates PPI trasfers towards DPHY DT bindings shouldn't mention drivers, you can just say "The subsystem supports up to 4 data lanes ...". s/trasfers/transfers/ > + with continuous clock. It supports Burst, non-burst modes and command modes. > + > +maintainers: > + - Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xxxxxxxxxx> > + > +properties: > + compatible: > + const: xlnx,dsi-1.0 I think calling it just "dsi-1.0" isn't specific enough, as it could also be a DSI RX. "xlnx,dsi-tx-1.0" would be a better value. Is this binding for v1.0 of the IP core ? There's a v2.0 too. Only v2.0 supports command mode as far as I can tell, so with the xlnx,dsi-cmd-mode property below, this seems to match v2.0. > + > + reg: > + maxItems: 1 This should specify in the description if only the DSI TX registers are covered, or if the D-PHY registers are included too. > + > + clocks: > + description: List of clock specifiers > + items: > + - description: AXI Lite clock > + - description: Video DPHY clock > + > + clock-names: > + items: > + - const: s_axis_aclk > + - const: dphy_clk_200M > + > + xlnx,dsi-num-lanes: > + description: Maximum number of lanes that IP configured with. > + possible values are 1, 2, 4. You can drop the second sentence as it's implied by the enum below. > + > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: [1, 2, 4] Do we need this property ? The protocol configuration register has an Active Lanes field that reports the number of lanes. Could we read the information from the register, and drop this property ? > + > + xlnx,dsi-data-type: > + description: MIPI DSI pixel format. > + possible values are 0, 1, 2, 3. Same here. > + > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: [0, 1, 2, 3] Same comment as above, should this be read from the Pixel Format field instead of being specified in DT ? > + > + xlnx,dsi-cmd-mode: > + description: denotes command mode support > + > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: [0, 1] I don't see a command mode user parameter in the datasheet, it seems that command mode is a runtime configuration parameter (bit 3 in the Core Configuration Register). It shouldn't be specified in DT, but queried at runtime from the connected panel. > + > + ports: > + type: object > + > + properties: > + port@0: > + type: object > + description: | > + output / source port node, endpoint describing modules > + connected the DSI TX subsystem > + > + properties: > + reg: > + const: 0 > + > + endpoint: > + type: object > + > + properties: > + > + remote-endpoint: true > + > + required: > + - remote-endpoint > + > + additionalProperties: false > + > + additionalProperties: false There should also be a port for the AXI4-Stream interface. The common practice is to number the input port@0 and the output port@1. Shouldn't ports specify required: - port@0 - port@1 ? > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - xlnx,dsi-num-lanes > + - xlnx,dsi-data-type > + - xlnx,dsi-cmd-mode > + - ports > + > +additionalProperties: false > + > +examples: > + - | > + mipi_dsi_tx_subsystem@80000000 { > + compatible = "xlnx,dsi-1.0"; > + reg = <0x0 0x80000000 0x0 0x10000>; DT examples are compiled in a context where #address-cells and #size-cells are both set to 1. The reg property should thus be <0x80000000 0x10000>; in the example, even if it doesn't match the hardware. Could you please validate the bindings with make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/xlnx/xlnx,dsi.yaml as explained in Documentation/devicetree/writing-schema.rst ? This will also compile the example to make sure that everything is right. > + clocks = <&misc_clk_0>, <&misc_clk_1>; > + clock-names = "s_axis_aclk", "dphy_clk_200M"; > + xlnx,dsi-num-lanes = <4>; > + xlnx,dsi-data-type = <1>; > + xlnx,dsi-cmd-mode = <0>; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0x0>; > + dsi_tx_out: endpoint { > + remote-endpoint = <&panel_in>; > + }; > + }; > + }; > + }; > + > +... -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
