| Message ID | 20220609064931.3068601-4-victor.liu@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add some DRM bridge drivers support for i.MX8qm/qxp SoCs | expand |
Hi Liu, Thank you for the patch. On Thu, Jun 09, 2022 at 02:49:20PM +0800, Liu Ying wrote: > This patch adds bindings for i.MX8qm/qxp pixel combiner. > > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Liu Ying <victor.liu@nxp.com> > --- > v7->v8: > * No change. > > v6->v7: > * No change. > > v5->v6: > * No change. > > v4->v5: > * No change. > > v3->v4: > * No change. > > v2->v3: > * Add Rob's R-b tag. > > v1->v2: > * Use graph schema. (Laurent) > * Use enum instead of oneOf + const for the reg property of pixel combiner > channels. (Rob) > > .../bridge/fsl,imx8qxp-pixel-combiner.yaml | 144 ++++++++++++++++++ > 1 file changed, 144 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-combiner.yaml > > diff --git a/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-combiner.yaml b/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-combiner.yaml > new file mode 100644 > index 000000000000..50bae2122183 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-combiner.yaml > @@ -0,0 +1,144 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/fsl,imx8qxp-pixel-combiner.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Freescale i.MX8qm/qxp Pixel Combiner > + > +maintainers: > + - Liu Ying <victor.liu@nxp.com> > + > +description: | > + The Freescale i.MX8qm/qxp Pixel Combiner takes two output streams from a > + single display controller and manipulates the two streams to support a number > + of modes(bypass, pixel combine, YUV444 to YUV422, split_RGB) configured as > + either one screen, two screens, or virtual screens. The pixel combiner is > + also responsible for generating some of the control signals for the pixel link > + output channel. > + > +properties: > + compatible: > + enum: > + - fsl,imx8qm-pixel-combiner > + - fsl,imx8qxp-pixel-combiner > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + const: apb > + > + power-domains: > + maxItems: 1 > + > +patternProperties: > + "^channel@[0-1]$": > + type: object > + description: Represents a display stream of pixel combiner. > + > + properties: > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + reg: > + description: The display stream index. > + enum: [ 0, 1 ] > + > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: Input endpoint of the display stream. > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: Output endpoint of the display stream. When multiple ports are present, they are usually grouped in a "ports" node. Not doing say may work from a schema point of view but makes implementation of generic helpers more difficult. Unless Rob thinks "ports" is really not needed here, I'd add it. This comment applies to all bindings in this series. > + > + required: > + - "#address-cells" > + - "#size-cells" > + - reg > + - port@0 > + - port@1 > + > + additionalProperties: false > + > +required: > + - compatible > + - "#address-cells" > + - "#size-cells" > + - reg > + - clocks > + - clock-names > + - power-domains > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/imx8-lpcg.h> > + #include <dt-bindings/firmware/imx/rsrc.h> > + pixel-combiner@56020000 { > + compatible = "fsl,imx8qxp-pixel-combiner"; > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x56020000 0x10000>; > + clocks = <&dc0_pixel_combiner_lpcg IMX_LPCG_CLK_4>; > + clock-names = "apb"; > + power-domains = <&pd IMX_SC_R_DC_0>; > + > + channel@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0>; > + > + port@0 { > + reg = <0>; > + > + dc0_pixel_combiner_ch0_dc0_dpu_disp0: endpoint { > + remote-endpoint = <&dc0_dpu_disp0_dc0_pixel_combiner_ch0>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + > + dc0_pixel_combiner_ch0_dc0_pixel_link0: endpoint { > + remote-endpoint = <&dc0_pixel_link0_dc0_pixel_combiner_ch0>; > + }; > + }; > + }; > + > + channel@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <1>; > + > + port@0 { > + reg = <0>; > + > + dc0_pixel_combiner_ch1_dc0_dpu_disp1: endpoint { > + remote-endpoint = <&dc0_dpu_disp1_dc0_pixel_combiner_ch1>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + > + dc0_pixel_combiner_ch1_dc0_pixel_link1: endpoint { > + remote-endpoint = <&dc0_pixel_link1_dc0_pixel_combiner_ch1>; > + }; > + }; > + }; > + };
Hi Laurent, On Thu, 2022-06-09 at 11:24 +0300, Laurent Pinchart wrote: > Hi Liu, > > Thank you for the patch. Thank you for the review. > > On Thu, Jun 09, 2022 at 02:49:20PM +0800, Liu Ying wrote: > > This patch adds bindings for i.MX8qm/qxp pixel combiner. > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Liu Ying <victor.liu@nxp.com> > > --- > > v7->v8: > > * No change. > > > > v6->v7: > > * No change. > > > > v5->v6: > > * No change. > > > > v4->v5: > > * No change. > > > > v3->v4: > > * No change. > > > > v2->v3: > > * Add Rob's R-b tag. > > > > v1->v2: > > * Use graph schema. (Laurent) > > * Use enum instead of oneOf + const for the reg property of pixel > > combiner > > channels. (Rob) > > > > .../bridge/fsl,imx8qxp-pixel-combiner.yaml | 144 > > ++++++++++++++++++ > > 1 file changed, 144 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel- > > combiner.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp- > > pixel-combiner.yaml > > b/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp- > > pixel-combiner.yaml > > new file mode 100644 > > index 000000000000..50bae2122183 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp- > > pixel-combiner.yaml > > @@ -0,0 +1,144 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fdisplay%2Fbridge%2Ffsl%2Cimx8qxp-pixel-combiner.yaml%23&data=05%7C01%7Cvictor.liu%40nxp.com%7Ca94c4502ac5748e2357408da49f17304%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637903598635803241%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=RIF8RJtIGU1gCGwSD%2FMsNx%2Bo7Kd9cxkInm310B1npq4%3D&reserved=0 > > +$schema: > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Cvictor.liu%40nxp.com%7Ca94c4502ac5748e2357408da49f17304%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637903598635803241%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=bQy5KKStqN17%2FnK8vGM%2F1nGhlkxdF%2FfYVDwGReWsbes%3D&reserved=0 > > + > > +title: Freescale i.MX8qm/qxp Pixel Combiner > > + > > +maintainers: > > + - Liu Ying <victor.liu@nxp.com> > > + > > +description: | > > + The Freescale i.MX8qm/qxp Pixel Combiner takes two output > > streams from a > > + single display controller and manipulates the two streams to > > support a number > > + of modes(bypass, pixel combine, YUV444 to YUV422, split_RGB) > > configured as > > + either one screen, two screens, or virtual screens. The pixel > > combiner is > > + also responsible for generating some of the control signals for > > the pixel link > > + output channel. > > + > > +properties: > > + compatible: > > + enum: > > + - fsl,imx8qm-pixel-combiner > > + - fsl,imx8qxp-pixel-combiner > > + > > + "#address-cells": > > + const: 1 > > + > > + "#size-cells": > > + const: 0 > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + maxItems: 1 > > + > > + clock-names: > > + const: apb > > + > > + power-domains: > > + maxItems: 1 > > + > > +patternProperties: > > + "^channel@[0-1]$": > > + type: object > > + description: Represents a display stream of pixel combiner. > > + > > + properties: > > + "#address-cells": > > + const: 1 > > + > > + "#size-cells": > > + const: 0 > > + > > + reg: > > + description: The display stream index. > > + enum: [ 0, 1 ] > > + > > + port@0: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: Input endpoint of the display stream. > > + > > + port@1: > > + $ref: /schemas/graph.yaml#/properties/port > > + description: Output endpoint of the display stream. > > When multiple ports are present, they are usually grouped in a > "ports" > node. Not doing say may work from a schema point of view but makes > implementation of generic helpers more difficult. Unless Rob thinks > "ports" is really not needed here, I'd add it. graph.yaml mentions 'ports' node is optional and 'port' node can be a direct property of a device. So, it's legit to use 'port' node without 'ports' and Rob has already provided his R-b tag to this patch. Regarding generic helper, the DPU KMS driver and the PXL2DPI driver(patch 8/14) call of_graph_get_remote_port_parent() to find the remote pixel combiner channel and ldb channel. So, it looks ok. > > This comment applies to all bindings in this series. Besides this pixel combiner binding, the ldb binding(patch 10/14) is the only one this comment applies to. The i.MX8qxp reference manual mentions both ldb and pixel combiner support 2 channels. That's why the bindings use 'channel' nodes to better reflect HW design. Using 'ports' node in 'channel' node would bring more tabs for indentation, so I prefer not to use it. Regards, Liu Ying > > > + > > + required: > > + - "#address-cells" > > + - "#size-cells" > > + - reg > > + - port@0 > > + - port@1 > > + > > + additionalProperties: false > > + > > +required: > > + - compatible > > + - "#address-cells" > > + - "#size-cells" > > + - reg > > + - clocks > > + - clock-names > > + - power-domains > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/imx8-lpcg.h> > > + #include <dt-bindings/firmware/imx/rsrc.h> > > + pixel-combiner@56020000 { > > + compatible = "fsl,imx8qxp-pixel-combiner"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + reg = <0x56020000 0x10000>; > > + clocks = <&dc0_pixel_combiner_lpcg IMX_LPCG_CLK_4>; > > + clock-names = "apb"; > > + power-domains = <&pd IMX_SC_R_DC_0>; > > + > > + channel@0 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + reg = <0>; > > + > > + port@0 { > > + reg = <0>; > > + > > + dc0_pixel_combiner_ch0_dc0_dpu_disp0: endpoint { > > + remote-endpoint = > > <&dc0_dpu_disp0_dc0_pixel_combiner_ch0>; > > + }; > > + }; > > + > > + port@1 { > > + reg = <1>; > > + > > + dc0_pixel_combiner_ch0_dc0_pixel_link0: endpoint { > > + remote-endpoint = > > <&dc0_pixel_link0_dc0_pixel_combiner_ch0>; > > + }; > > + }; > > + }; > > + > > + channel@1 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + reg = <1>; > > + > > + port@0 { > > + reg = <0>; > > + > > + dc0_pixel_combiner_ch1_dc0_dpu_disp1: endpoint { > > + remote-endpoint = > > <&dc0_dpu_disp1_dc0_pixel_combiner_ch1>; > > + }; > > + }; > > + > > + port@1 { > > + reg = <1>; > > + > > + dc0_pixel_combiner_ch1_dc0_pixel_link1: endpoint { > > + remote-endpoint = > > <&dc0_pixel_link1_dc0_pixel_combiner_ch1>; > > + }; > > + }; > > + }; > > + }; > >
diff --git a/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-combiner.yaml b/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-combiner.yaml new file mode 100644 index 000000000000..50bae2122183 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/fsl,imx8qxp-pixel-combiner.yaml @@ -0,0 +1,144 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/bridge/fsl,imx8qxp-pixel-combiner.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Freescale i.MX8qm/qxp Pixel Combiner + +maintainers: + - Liu Ying <victor.liu@nxp.com> + +description: | + The Freescale i.MX8qm/qxp Pixel Combiner takes two output streams from a + single display controller and manipulates the two streams to support a number + of modes(bypass, pixel combine, YUV444 to YUV422, split_RGB) configured as + either one screen, two screens, or virtual screens. The pixel combiner is + also responsible for generating some of the control signals for the pixel link + output channel. + +properties: + compatible: + enum: + - fsl,imx8qm-pixel-combiner + - fsl,imx8qxp-pixel-combiner + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + clock-names: + const: apb + + power-domains: + maxItems: 1 + +patternProperties: + "^channel@[0-1]$": + type: object + description: Represents a display stream of pixel combiner. + + properties: + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + reg: + description: The display stream index. + enum: [ 0, 1 ] + + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: Input endpoint of the display stream. + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: Output endpoint of the display stream. + + required: + - "#address-cells" + - "#size-cells" + - reg + - port@0 + - port@1 + + additionalProperties: false + +required: + - compatible + - "#address-cells" + - "#size-cells" + - reg + - clocks + - clock-names + - power-domains + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/imx8-lpcg.h> + #include <dt-bindings/firmware/imx/rsrc.h> + pixel-combiner@56020000 { + compatible = "fsl,imx8qxp-pixel-combiner"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x56020000 0x10000>; + clocks = <&dc0_pixel_combiner_lpcg IMX_LPCG_CLK_4>; + clock-names = "apb"; + power-domains = <&pd IMX_SC_R_DC_0>; + + channel@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + + port@0 { + reg = <0>; + + dc0_pixel_combiner_ch0_dc0_dpu_disp0: endpoint { + remote-endpoint = <&dc0_dpu_disp0_dc0_pixel_combiner_ch0>; + }; + }; + + port@1 { + reg = <1>; + + dc0_pixel_combiner_ch0_dc0_pixel_link0: endpoint { + remote-endpoint = <&dc0_pixel_link0_dc0_pixel_combiner_ch0>; + }; + }; + }; + + channel@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + + port@0 { + reg = <0>; + + dc0_pixel_combiner_ch1_dc0_dpu_disp1: endpoint { + remote-endpoint = <&dc0_dpu_disp1_dc0_pixel_combiner_ch1>; + }; + }; + + port@1 { + reg = <1>; + + dc0_pixel_combiner_ch1_dc0_pixel_link1: endpoint { + remote-endpoint = <&dc0_pixel_link1_dc0_pixel_combiner_ch1>; + }; + }; + }; + };