diff mbox

[v3,3/8] dt-bindings: display: xlnx: Add ZynqMP DP subsystem bindings

Message ID 1516067831-11382-3-git-send-email-hyun.kwon@xilinx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hyun Kwon Jan. 16, 2018, 1:57 a.m. UTC
This add a dt binding for ZynqMP DP subsystem.

Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com>
---
v2
- Group multiple ports under 'ports'
- Replace linux specific terms with generic hardware descriptions
---
---
 .../bindings/display/xlnx/xlnx,zynqmp-dpsub.txt    | 98 ++++++++++++++++++++++
 1 file changed, 98 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt

--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Comments

Rob Herring (Arm) Jan. 20, 2018, 12:31 a.m. UTC | #1
On Mon, Jan 15, 2018 at 05:57:06PM -0800, Hyun Kwon wrote:
> This add a dt binding for ZynqMP DP subsystem.
> 
> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com>
> ---
> v2
> - Group multiple ports under 'ports'
> - Replace linux specific terms with generic hardware descriptions
> ---
> ---
>  .../bindings/display/xlnx/xlnx,zynqmp-dpsub.txt    | 98 ++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt
> new file mode 100644
> index 0000000..dbcbde5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt
> @@ -0,0 +1,98 @@
> +Xilinx ZynqMP DisplayPort subsystem
> +-----------------------------------
> +
> +Required properties:
> +
> +- compatible: Must be "xlnx,zynqmp-dpsub-1.7".
> +
> +- reg: Physical base address and length of the registers set for the device.
> +- reg-names: Must be "dp", "blend", "av_buf", and "aud" to map logical register
> +  partitions.
> +
> +- interrupts: Interrupt number.
> +- interrupts-parent: phandle for interrupt controller.
> +
> +- clocks: phandles for axi, audio, non-live video, and live video clocks.
> +  axi clock is required. Audio clock is optional. If not present, audio will
> +  be disabled. One of non-live or live video clock should be present.
> +- clock-names: The identification strings are required. "aclk" for axi clock.
> +  "dp_aud_clk" for audio clock. "dp_vtc_pixel_clk_in" for non-live video clock.
> +  "dp_live_video_in_clk" for live video clock (clock from programmable logic).
> +
> +- phys: phandles for phy specifier.

phandles? How many?

> +- phy-names: The identifier strings. "dp-phy" followed by index.
> +
> +- power-domains: phandle for the corresponding power domain
> +
> +- ports: There are 2 logical blocks in the IP: display controller and
> +  DisplayPort encoder. Each block can be used / connected independently with

I'm confused. The block described in this doc is just DisplayPort 
encoder or both display controller and DisplayPort? Or something else?

> +  external device, hence ports for each block are required using DT bindings
> +  defined in Documentation/devicetree/bindings/graph.txt. Refer to
> +  ./xlnx,display.txt for how topology for entire pipeline is described.
> +
> +- vid-layer, gfx-layer: Required to represent available layers
> +
> +Required layer properties
> +
> +- dmas: phandles for DMA channels as defined in
> +  Documentation/devicetree/bindings/dma/dma.txt.
> +- dma-names: The identifier strings are required. "graphics0" for graphics
> +  layer. "video" followed by index for video layer
> +
> +Optional child node
> +
> +- The driver populates any child device node in this node. This can be used,
> +  for example, to populate the sound device from the DisplayPort subsystem
> +  driver.
> +
> +Example:
> +       zynqmp_dpsub: zynqmp_dpsub@fd4a0000 {

video-codec@... if this the DP encoder.

> +               compatible = "xlnx,zynqmp-dpsub-1.7";
> +               reg = <0x0 0xfd4a0000 0x0 0x1000>,
> +                     <0x0 0xfd4aa000 0x0 0x1000>,
> +                     <0x0 0xfd4ab000 0x0 0x1000>,
> +                     <0x0 0xfd4ac000 0x0 0x1000>;
> +               reg-names = "dp", "blend", "av_buf", "aud";
> +               interrupts = <0 119 4>;
> +               interrupt-parent = <&gic>;
> +
> +               clock-names = "dp_apb_clk", "dp_aud_clk", "dp_live_video_in_clk";
> +               clocks = <&dp_aclk>, <&clkc 17>, <&si570_1>;
> +
> +               phys = <&lane1>, <&lane0>;
> +               phy-names = "dp-phy0", "dp-phy1";
> +
> +               power-domains = <&pd_dp>;
> +
> +               vid-layer {
> +                       dma-names = "vid0", "vid1", "vid2";

Documentation says these should be "videoX".

> +                       dmas = <&xlnx_dpdma 0>,
> +                              <&xlnx_dpdma 1>,
> +                              <&xlnx_dpdma 2>;
> +               };
> +
> +               gfx-layer {
> +                       dma-names = "gfx0";

And "graphics0"

> +                       dmas = <&xlnx_dpdma 3>;
> +               };

You don't need these child nodes. Just do:

dma-names = "vid0", "vid1", "vid2", "gfx0";

Or gfx0 first.

> +
> +               ports {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +
> +                       display_port: port@0 {
> +                               reg = <0>;
> +                               display_port: endpoint {
> +                                       remote-endpoint = <&dp_port>;
> +                               };
> +                       };
> +                       port@1 {
> +                               reg = <1>;
> +                               dp_port: endpoint {
> +                                       remote-endpoint = <&display_port>;
> +                               };
> +                       };
> +               }
> +       };
> +};
> +
> --
> 2.7.4
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hyun Kwon Jan. 20, 2018, 2:22 a.m. UTC | #2
Hi Rob,

Thanks for the review.

> -----Original Message-----

> From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf

> Of Rob Herring

> Sent: Friday, January 19, 2018 4:31 PM

> To: Hyun Kwon <hyunk@xilinx.com>

> Cc: devicetree@vger.kernel.org; Laurent Pinchart

> <laurent.pinchart@ideasonboard.com>; Michal Simek

> <michal.simek@xilinx.com>; dri-devel@lists.freedesktop.org; Daniel Vetter

> <daniel.vetter@ffwll.ch>

> Subject: Re: [PATCH v3 3/8] dt-bindings: display: xlnx: Add ZynqMP DP

> subsystem bindings

> 

> On Mon, Jan 15, 2018 at 05:57:06PM -0800, Hyun Kwon wrote:

> > This add a dt binding for ZynqMP DP subsystem.

> >

> > Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com>

> > ---

> > v2

> > - Group multiple ports under 'ports'

> > - Replace linux specific terms with generic hardware descriptions

> > ---

> > ---

> >  .../bindings/display/xlnx/xlnx,zynqmp-dpsub.txt    | 98

> ++++++++++++++++++++++

> >  1 file changed, 98 insertions(+)

> >  create mode 100644

> Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt

> >

> > diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-

> dpsub.txt b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-

> dpsub.txt

> > new file mode 100644

> > index 0000000..dbcbde5

> > --- /dev/null

> > +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-

> dpsub.txt

> > @@ -0,0 +1,98 @@

> > +Xilinx ZynqMP DisplayPort subsystem

> > +-----------------------------------

> > +

> > +Required properties:

> > +

> > +- compatible: Must be "xlnx,zynqmp-dpsub-1.7".

> > +

> > +- reg: Physical base address and length of the registers set for the device.

> > +- reg-names: Must be "dp", "blend", "av_buf", and "aud" to map logical

> register

> > +  partitions.

> > +

> > +- interrupts: Interrupt number.

> > +- interrupts-parent: phandle for interrupt controller.

> > +

> > +- clocks: phandles for axi, audio, non-live video, and live video clocks.

> > +  axi clock is required. Audio clock is optional. If not present, audio will

> > +  be disabled. One of non-live or live video clock should be present.

> > +- clock-names: The identification strings are required. "aclk" for axi clock.

> > +  "dp_aud_clk" for audio clock. "dp_vtc_pixel_clk_in" for non-live video

> clock.

> > +  "dp_live_video_in_clk" for live video clock (clock from programmable

> logic).

> > +

> > +- phys: phandles for phy specifier.

> 

> phandles? How many?

> 


Configurable between 1 or 2 lanes. Will add more details.

> > +- phy-names: The identifier strings. "dp-phy" followed by index.

> > +

> > +- power-domains: phandle for the corresponding power domain

> > +

> > +- ports: There are 2 logical blocks in the IP: display controller and

> > +  DisplayPort encoder. Each block can be used / connected independently

> with

> 

> I'm confused. The block described in this doc is just DisplayPort

> encoder or both display controller and DisplayPort? Or something else?

> 


The DP subsystem includes both block, protocol indipendent controller in front
and the DisplayPort transmitter in the back. I'll try to elaborate / clarify
better.

> > +  external device, hence ports for each block are required using DT

> bindings

> > +  defined in Documentation/devicetree/bindings/graph.txt. Refer to

> > +  ./xlnx,display.txt for how topology for entire pipeline is described.

> > +

> > +- vid-layer, gfx-layer: Required to represent available layers

> > +

> > +Required layer properties

> > +

> > +- dmas: phandles for DMA channels as defined in

> > +  Documentation/devicetree/bindings/dma/dma.txt.

> > +- dma-names: The identifier strings are required. "graphics0" for graphics

> > +  layer. "video" followed by index for video layer

> > +

> > +Optional child node

> > +

> > +- The driver populates any child device node in this node. This can be

> used,

> > +  for example, to populate the sound device from the DisplayPort

> subsystem

> > +  driver.

> > +

> > +Example:

> > +       zynqmp_dpsub: zynqmp_dpsub@fd4a0000 {

> 

> video-codec@... if this the DP encoder.

> 

> > +               compatible = "xlnx,zynqmp-dpsub-1.7";

> > +               reg = <0x0 0xfd4a0000 0x0 0x1000>,

> > +                     <0x0 0xfd4aa000 0x0 0x1000>,

> > +                     <0x0 0xfd4ab000 0x0 0x1000>,

> > +                     <0x0 0xfd4ac000 0x0 0x1000>;

> > +               reg-names = "dp", "blend", "av_buf", "aud";

> > +               interrupts = <0 119 4>;

> > +               interrupt-parent = <&gic>;

> > +

> > +               clock-names = "dp_apb_clk", "dp_aud_clk",

> "dp_live_video_in_clk";

> > +               clocks = <&dp_aclk>, <&clkc 17>, <&si570_1>;

> > +

> > +               phys = <&lane1>, <&lane0>;

> > +               phy-names = "dp-phy0", "dp-phy1";

> > +

> > +               power-domains = <&pd_dp>;

> > +

> > +               vid-layer {

> > +                       dma-names = "vid0", "vid1", "vid2";

> 

> Documentation says these should be "videoX".

> 


Will fix.

> > +                       dmas = <&xlnx_dpdma 0>,

> > +                              <&xlnx_dpdma 1>,

> > +                              <&xlnx_dpdma 2>;

> > +               };

> > +

> > +               gfx-layer {

> > +                       dma-names = "gfx0";

> 

> And "graphics0"

> 


Will fix.

> > +                       dmas = <&xlnx_dpdma 3>;

> > +               };

> 

> You don't need these child nodes. Just do:

> 

> dma-names = "vid0", "vid1", "vid2", "gfx0";

> 

> Or gfx0 first.

> 


Sure. As mentioned earlier, each layer will have to be referenced independently
by other nodes in upcoming patches. But I can follow your suggestion for now,
and make change when it's needed.

Thanks,
-hyun
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt
new file mode 100644
index 0000000..dbcbde5
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt
@@ -0,0 +1,98 @@ 
+Xilinx ZynqMP DisplayPort subsystem
+-----------------------------------
+
+Required properties:
+
+- compatible: Must be "xlnx,zynqmp-dpsub-1.7".
+
+- reg: Physical base address and length of the registers set for the device.
+- reg-names: Must be "dp", "blend", "av_buf", and "aud" to map logical register
+  partitions.
+
+- interrupts: Interrupt number.
+- interrupts-parent: phandle for interrupt controller.
+
+- clocks: phandles for axi, audio, non-live video, and live video clocks.
+  axi clock is required. Audio clock is optional. If not present, audio will
+  be disabled. One of non-live or live video clock should be present.
+- clock-names: The identification strings are required. "aclk" for axi clock.
+  "dp_aud_clk" for audio clock. "dp_vtc_pixel_clk_in" for non-live video clock.
+  "dp_live_video_in_clk" for live video clock (clock from programmable logic).
+
+- phys: phandles for phy specifier.
+- phy-names: The identifier strings. "dp-phy" followed by index.
+
+- power-domains: phandle for the corresponding power domain
+
+- ports: There are 2 logical blocks in the IP: display controller and
+  DisplayPort encoder. Each block can be used / connected independently with
+  external device, hence ports for each block are required using DT bindings
+  defined in Documentation/devicetree/bindings/graph.txt. Refer to
+  ./xlnx,display.txt for how topology for entire pipeline is described.
+
+- vid-layer, gfx-layer: Required to represent available layers
+
+Required layer properties
+
+- dmas: phandles for DMA channels as defined in
+  Documentation/devicetree/bindings/dma/dma.txt.
+- dma-names: The identifier strings are required. "graphics0" for graphics
+  layer. "video" followed by index for video layer
+
+Optional child node
+
+- The driver populates any child device node in this node. This can be used,
+  for example, to populate the sound device from the DisplayPort subsystem
+  driver.
+
+Example:
+       zynqmp_dpsub: zynqmp_dpsub@fd4a0000 {
+               compatible = "xlnx,zynqmp-dpsub-1.7";
+               reg = <0x0 0xfd4a0000 0x0 0x1000>,
+                     <0x0 0xfd4aa000 0x0 0x1000>,
+                     <0x0 0xfd4ab000 0x0 0x1000>,
+                     <0x0 0xfd4ac000 0x0 0x1000>;
+               reg-names = "dp", "blend", "av_buf", "aud";
+               interrupts = <0 119 4>;
+               interrupt-parent = <&gic>;
+
+               clock-names = "dp_apb_clk", "dp_aud_clk", "dp_live_video_in_clk";
+               clocks = <&dp_aclk>, <&clkc 17>, <&si570_1>;
+
+               phys = <&lane1>, <&lane0>;
+               phy-names = "dp-phy0", "dp-phy1";
+
+               power-domains = <&pd_dp>;
+
+               vid-layer {
+                       dma-names = "vid0", "vid1", "vid2";
+                       dmas = <&xlnx_dpdma 0>,
+                              <&xlnx_dpdma 1>,
+                              <&xlnx_dpdma 2>;
+               };
+
+               gfx-layer {
+                       dma-names = "gfx0";
+                       dmas = <&xlnx_dpdma 3>;
+               };
+
+               ports {
+                       #address-cells = <1>;
+                       #size-cells = <0>;
+
+                       display_port: port@0 {
+                               reg = <0>;
+                               display_port: endpoint {
+                                       remote-endpoint = <&dp_port>;
+                               };
+                       };
+                       port@1 {
+                               reg = <1>;
+                               dp_port: endpoint {
+                                       remote-endpoint = <&display_port>;
+                               };
+                       };
+               }
+       };
+};
+