diff mbox

[v2,1/4] drm: arm: Add DT bindings documentation for HDLCD driver.

Message ID 1447258010-2234-2-git-send-email-Liviu.Dudau@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Liviu Dudau Nov. 11, 2015, 4:06 p.m. UTC
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 .../devicetree/bindings/drm/arm/arm,hdlcd.txt      | 74 ++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt

Comments

Rob Herring (Arm) Nov. 11, 2015, 6:48 p.m. UTC | #1
On Wed, Nov 11, 2015 at 04:06:47PM +0000, Liviu Dudau wrote:
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> 
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>

Looks pretty good, but a few comments.

> ---
>  .../devicetree/bindings/drm/arm/arm,hdlcd.txt      | 74 ++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
> 
> diff --git a/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
> new file mode 100644
> index 0000000..b57f1b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
> @@ -0,0 +1,74 @@
> +ARM HDLCD
> +
> +This is a display controller found on several development platforms produced
> +by ARM Ltd and in more modern of its' Fast Models. The HDLCD is an RGB
> +streamer that reads the data from a framebuffer and sends it to a single
> +digital encoder (DVI or HDMI).
> +
> +Required properties:
> +  - compatible: "arm,hdlcd"

Kind of generic. Something more specific please.

> +  - reg: Physical base address and length of the controller's registers.
> +    If a second pair of address and length values is present this specifies
> +    the presence of a DMA coherent memory area that the HDLCD can use as
> +    framebuffer instead of normal CMA memory.

This is on-chip RAM or nornal system RAM? We already have bindings for 
both.

> +  - interrupts: One interrupt used by the display controller to notify the
> +    interrupt controller when any of the interrupt sources programmed in
> +    the interrupt mask register have activated.
> +  - clocks: A list of phandle + clock-specifier pairs, one for each
> +    entry in 'clock-names'.
> +  - clock-names: A list of clock names. For HDLD it should contain:
> +      - "pxlclk" for the clock feeding the output PLL of the controller.
> +  - port: The HDLCD connection to an encoder chip. The connection is modelled
> +    using the OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
Liviu Dudau Nov. 12, 2015, 10:42 a.m. UTC | #2
On Wed, Nov 11, 2015 at 12:48:50PM -0600, Rob Herring wrote:
> On Wed, Nov 11, 2015 at 04:06:47PM +0000, Liviu Dudau wrote:
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Pawel Moll <pawel.moll@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> > Cc: Kumar Gala <galak@codeaurora.org>
> > 
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> 
> Looks pretty good, but a few comments.
> 
> > ---
> >  .../devicetree/bindings/drm/arm/arm,hdlcd.txt      | 74 ++++++++++++++++++++++
> >  1 file changed, 74 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
> > new file mode 100644
> > index 0000000..b57f1b9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
> > @@ -0,0 +1,74 @@
> > +ARM HDLCD
> > +
> > +This is a display controller found on several development platforms produced
> > +by ARM Ltd and in more modern of its' Fast Models. The HDLCD is an RGB
> > +streamer that reads the data from a framebuffer and sends it to a single
> > +digital encoder (DVI or HDMI).
> > +
> > +Required properties:
> > +  - compatible: "arm,hdlcd"
> 
> Kind of generic. Something more specific please.

"There can be only one!" (hdlcd) :) This is going to be a "one version only" HW part.
ARM has now switched to a new display hardware that has more features and a new name,
and work on mainlining support for that will start once I get the HDLCD driver accepted. 

> 
> > +  - reg: Physical base address and length of the controller's registers.
> > +    If a second pair of address and length values is present this specifies
> > +    the presence of a DMA coherent memory area that the HDLCD can use as
> > +    framebuffer instead of normal CMA memory.
> 
> This is on-chip RAM or nornal system RAM? We already have bindings for 
> both.

Juno has a set of TLX (ThinLinks) connectors on the board where an FPGA can be attached. On r1
the code running on FPGA can even participate as an AXI master with full coherency. The FPGA
has local memory that we want to share with the HDLCD to be used as a framebuffer.

> 
> > +  - interrupts: One interrupt used by the display controller to notify the
> > +    interrupt controller when any of the interrupt sources programmed in
> > +    the interrupt mask register have activated.
> > +  - clocks: A list of phandle + clock-specifier pairs, one for each
> > +    entry in 'clock-names'.
> > +  - clock-names: A list of clock names. For HDLD it should contain:
> > +      - "pxlclk" for the clock feeding the output PLL of the controller.
> > +  - port: The HDLCD connection to an encoder chip. The connection is modelled
> > +    using the OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> 

Thanks for reviewing this,
Liviu
Jon Medhurst (Tixy) Nov. 12, 2015, 10:52 a.m. UTC | #3
On Thu, 2015-11-12 at 10:42 +0000, Liviu Dudau wrote:
> > This is on-chip RAM or nornal system RAM? We already have bindings
> for 
> > both.
>
> Juno has a set of TLX (ThinLinks) connectors on the board where an
> FPGA can be attached. On r1
> the code running on FPGA can even participate as an AXI master with
> full coherency. The FPGA
> has local memory that we want to share with the HDLCD to be used as a
> framebuffer.

The HDLCD on the Juno chip or one implemented in the FPGA? I assume you
mean the latter but just wanted to check.
Liviu Dudau Nov. 12, 2015, 11:09 a.m. UTC | #4
On Thu, Nov 12, 2015 at 10:52:25AM +0000, Jon Medhurst (Tixy) wrote:
> On Thu, 2015-11-12 at 10:42 +0000, Liviu Dudau wrote:
> > > This is on-chip RAM or nornal system RAM? We already have bindings
> > for 
> > > both.
> >
> > Juno has a set of TLX (ThinLinks) connectors on the board where an
> > FPGA can be attached. On r1
> > the code running on FPGA can even participate as an AXI master with
> > full coherency. The FPGA
> > has local memory that we want to share with the HDLCD to be used as a
> > framebuffer.
> 
> The HDLCD on the Juno chip or one implemented in the FPGA? I assume you
> mean the latter but just wanted to check.

You can have a GPU on the FPGA, and you want to read the framebuffer
off the FPGA's memory to render it on screen using the SoC's HDLCD. Hope
this makes sense.

Best regards,
Liviu

> 
> -- 
> Tixy
>
Rob Herring (Arm) Nov. 13, 2015, 3:30 a.m. UTC | #5
On Thu, Nov 12, 2015 at 4:42 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Wed, Nov 11, 2015 at 12:48:50PM -0600, Rob Herring wrote:
>> On Wed, Nov 11, 2015 at 04:06:47PM +0000, Liviu Dudau wrote:
>> > Cc: Rob Herring <robh+dt@kernel.org>
>> > Cc: Pawel Moll <pawel.moll@arm.com>
>> > Cc: Mark Rutland <mark.rutland@arm.com>
>> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>> > Cc: Kumar Gala <galak@codeaurora.org>
>> >
>> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>>
>> Looks pretty good, but a few comments.
>>
>> > ---
>> >  .../devicetree/bindings/drm/arm/arm,hdlcd.txt      | 74 ++++++++++++++++++++++
>> >  1 file changed, 74 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
>> > new file mode 100644
>> > index 0000000..b57f1b9
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
>> > @@ -0,0 +1,74 @@
>> > +ARM HDLCD
>> > +
>> > +This is a display controller found on several development platforms produced
>> > +by ARM Ltd and in more modern of its' Fast Models. The HDLCD is an RGB
>> > +streamer that reads the data from a framebuffer and sends it to a single
>> > +digital encoder (DVI or HDMI).
>> > +
>> > +Required properties:
>> > +  - compatible: "arm,hdlcd"
>>
>> Kind of generic. Something more specific please.
>
> "There can be only one!" (hdlcd) :) This is going to be a "one version only" HW part.
> ARM has now switched to a new display hardware that has more features and a new name,
> and work on mainlining support for that will start once I get the HDLCD driver accepted.

So there is never going to be a single difference across platforms.
Variations in max clock for different FPGAs?


>> > +  - reg: Physical base address and length of the controller's registers.
>> > +    If a second pair of address and length values is present this specifies
>> > +    the presence of a DMA coherent memory area that the HDLCD can use as
>> > +    framebuffer instead of normal CMA memory.
>>
>> This is on-chip RAM or nornal system RAM? We already have bindings for
>> both.
>
> Juno has a set of TLX (ThinLinks) connectors on the board where an FPGA can be attached. On r1
> the code running on FPGA can even participate as an AXI master with full coherency. The FPGA
> has local memory that we want to share with the HDLCD to be used as a framebuffer.

So describe the memory region and then use a memory-region phandle to
the memory here.

>> > +  - interrupts: One interrupt used by the display controller to notify the
>> > +    interrupt controller when any of the interrupt sources programmed in
>> > +    the interrupt mask register have activated.
>> > +  - clocks: A list of phandle + clock-specifier pairs, one for each
>> > +    entry in 'clock-names'.
>> > +  - clock-names: A list of clock names. For HDLD it should contain:

typo: HDLD

>> > +      - "pxlclk" for the clock feeding the output PLL of the controller.
>> > +  - port: The HDLCD connection to an encoder chip. The connection is modelled

s/modelled/modeled/

Rob
Liviu Dudau Nov. 13, 2015, 10:11 a.m. UTC | #6
On Thu, Nov 12, 2015 at 09:30:31PM -0600, Rob Herring wrote:
> On Thu, Nov 12, 2015 at 4:42 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > On Wed, Nov 11, 2015 at 12:48:50PM -0600, Rob Herring wrote:
> >> On Wed, Nov 11, 2015 at 04:06:47PM +0000, Liviu Dudau wrote:
> >> > Cc: Rob Herring <robh+dt@kernel.org>
> >> > Cc: Pawel Moll <pawel.moll@arm.com>
> >> > Cc: Mark Rutland <mark.rutland@arm.com>
> >> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> >> > Cc: Kumar Gala <galak@codeaurora.org>
> >> >
> >> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> >>
> >> Looks pretty good, but a few comments.
> >>
> >> > ---
> >> >  .../devicetree/bindings/drm/arm/arm,hdlcd.txt      | 74 ++++++++++++++++++++++
> >> >  1 file changed, 74 insertions(+)
> >> >  create mode 100644 Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
> >> > new file mode 100644
> >> > index 0000000..b57f1b9
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
> >> > @@ -0,0 +1,74 @@
> >> > +ARM HDLCD
> >> > +
> >> > +This is a display controller found on several development platforms produced
> >> > +by ARM Ltd and in more modern of its' Fast Models. The HDLCD is an RGB
> >> > +streamer that reads the data from a framebuffer and sends it to a single
> >> > +digital encoder (DVI or HDMI).
> >> > +
> >> > +Required properties:
> >> > +  - compatible: "arm,hdlcd"
> >>
> >> Kind of generic. Something more specific please.
> >
> > "There can be only one!" (hdlcd) :) This is going to be a "one version only" HW part.
> > ARM has now switched to a new display hardware that has more features and a new name,
> > and work on mainlining support for that will start once I get the HDLCD driver accepted.
> 
> So there is never going to be a single difference across platforms.

Correct, there is only one implementation available. AFAIK there are no plans to make changes to it.

> Variations in max clock for different FPGAs?

The clock is external to the part and there is a check in the driver if we can set the frequency
to match the requested dotclock by the video_mode. It does not affect the hardware or how it presents
itself to the driver.

> 
> 
> >> > +  - reg: Physical base address and length of the controller's registers.
> >> > +    If a second pair of address and length values is present this specifies
> >> > +    the presence of a DMA coherent memory area that the HDLCD can use as
> >> > +    framebuffer instead of normal CMA memory.
> >>
> >> This is on-chip RAM or nornal system RAM? We already have bindings for
> >> both.
> >
> > Juno has a set of TLX (ThinLinks) connectors on the board where an FPGA can be attached. On r1
> > the code running on FPGA can even participate as an AXI master with full coherency. The FPGA
> > has local memory that we want to share with the HDLCD to be used as a framebuffer.
> 
> So describe the memory region and then use a memory-region phandle to
> the memory here.

OK, I will.

> 
> >> > +  - interrupts: One interrupt used by the display controller to notify the
> >> > +    interrupt controller when any of the interrupt sources programmed in
> >> > +    the interrupt mask register have activated.
> >> > +  - clocks: A list of phandle + clock-specifier pairs, one for each
> >> > +    entry in 'clock-names'.
> >> > +  - clock-names: A list of clock names. For HDLD it should contain:
> 
> typo: HDLD

oops, thanks, will fix.

> 
> >> > +      - "pxlclk" for the clock feeding the output PLL of the controller.
> >> > +  - port: The HDLCD connection to an encoder chip. The connection is modelled
> 
> s/modelled/modeled/

ditto.

Thanks for reviewing it,
Liviu

> 
> Rob
>
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
new file mode 100644
index 0000000..b57f1b9
--- /dev/null
+++ b/Documentation/devicetree/bindings/drm/arm/arm,hdlcd.txt
@@ -0,0 +1,74 @@ 
+ARM HDLCD
+
+This is a display controller found on several development platforms produced
+by ARM Ltd and in more modern of its' Fast Models. The HDLCD is an RGB
+streamer that reads the data from a framebuffer and sends it to a single
+digital encoder (DVI or HDMI).
+
+Required properties:
+  - compatible: "arm,hdlcd"
+  - reg: Physical base address and length of the controller's registers.
+    If a second pair of address and length values is present this specifies
+    the presence of a DMA coherent memory area that the HDLCD can use as
+    framebuffer instead of normal CMA memory.
+  - interrupts: One interrupt used by the display controller to notify the
+    interrupt controller when any of the interrupt sources programmed in
+    the interrupt mask register have activated.
+  - clocks: A list of phandle + clock-specifier pairs, one for each
+    entry in 'clock-names'.
+  - clock-names: A list of clock names. For HDLD it should contain:
+      - "pxlclk" for the clock feeding the output PLL of the controller.
+  - port: The HDLCD connection to an encoder chip. The connection is modelled
+    using the OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+
+Example:
+
+/ {
+	...
+
+	hdlcd@2b000000 {
+		compatible = "arm,hdlcd";
+		reg = <0 0x2b000000 0 0x1000>;
+		interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&oscclk5>;
+		clock-names = "pxlclk";
+		port {
+			hdlcd_output: endpoint@0 {
+				remote-endpoint = <&hdmi_enc_input>;
+			};
+		};
+	};
+
+	/* HDMI encoder on I2C bus */
+	i2c@7ffa0000 {
+		....
+		hdmi-transmitter@70 {
+			compatible = ".....";
+			reg = <0x70>;
+			video-ports = <0x234501>;
+			port@0 {
+				hdmi_enc_input: endpoint {
+					remote-endpoint = <&hdlcd_output>;
+				};
+
+				hdmi_enc_output: endpoint {
+					remote-endpoint = <&hdmi_1_port>;
+				};
+			};
+		};
+
+	};
+
+	hdmi1: connector@1 {
+		compatible = "hdmi-connector";
+		type = "a";
+		port {
+			hdmi_1_port: endpoint {
+				remote-endpoint = <&hdmi_enc_output>;
+			};
+		};
+	};
+
+	...
+};