diff mbox

[v3,1/7] drm/vc4: Add devicetree bindings for VC4.

Message ID 1444426068-15817-2-git-send-email-eric@anholt.net
State Under Review, archived
Headers show

Commit Message

Eric Anholt Oct. 9, 2015, 9:27 p.m. UTC
VC4 is the GPU (display and 3D) subsystem present on the 2835 and some
other Broadcom SoCs.

This binding follows the model of msm, imx, sti, and others, where
there is a subsystem node for the whole GPU, with nodes for the
individual HW components within it.

Signed-off-by: Eric Anholt <eric@anholt.net>
---

v2: Extend the commit message, fix several nits from Stephen Warren.
v3: Rename the compatibility strings, clean up node names, drop the
    unnecessary lists of components.  Use compatibility strings for
    choosing CRTC HVS channel numbers.  Document the HDMI clock usage.

 .../devicetree/bindings/gpu/brcm,bcm-vc4.txt       | 64 ++++++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt

Comments

Sebastian Reichel Oct. 9, 2015, 11:31 p.m. UTC | #1
Hi,

On Fri, Oct 09, 2015 at 02:27:42PM -0700, Eric Anholt wrote:
> VC4 is the GPU (display and 3D) subsystem present on the 2835 and some
> other Broadcom SoCs.
> 
> This binding follows the model of msm, imx, sti, and others, where
> there is a subsystem node for the whole GPU, with nodes for the
> individual HW components within it.

I think it would be useful to have the acronyms written out one time
in the document (VC4, HVS).

-- Sebastian
Rob Herring Oct. 13, 2015, 1:26 p.m. UTC | #2
On Fri, Oct 9, 2015 at 4:27 PM, Eric Anholt <eric@anholt.net> wrote:
> VC4 is the GPU (display and 3D) subsystem present on the 2835 and some
> other Broadcom SoCs.
>
> This binding follows the model of msm, imx, sti, and others, where
> there is a subsystem node for the whole GPU, with nodes for the

The subsystem node is gone now, right?

> individual HW components within it.
>
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>
> v2: Extend the commit message, fix several nits from Stephen Warren.
> v3: Rename the compatibility strings, clean up node names, drop the
>     unnecessary lists of components.  Use compatibility strings for
>     choosing CRTC HVS channel numbers.  Document the HDMI clock usage.
>
>  .../devicetree/bindings/gpu/brcm,bcm-vc4.txt       | 64 ++++++++++++++++++++++

Can you put this in bindings/display/ instead? Things are moving there in 4.4.


>  1 file changed, 64 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
>
> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
> new file mode 100644
> index 0000000..175bcde
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
> @@ -0,0 +1,64 @@
> +Broadcom VC4 GPU
> +
> +The VC4 device present on the Raspberry Pi includes a display system
> +with HDMI output and the HVS scaler for compositing display planes.
> +
> +Required properties for VC4:
> +- compatible:  Should be "brcm,bcm2835-vc4"

reg property? interrupts? clocks?

> +
> +Required properties for Pixel Valve:
> +- compatible:  Should be one of "brcm,bcm2835-pixelvalve0",
> +               "brcm,bcm2835-pixelvalve1", or "brcm,bcm2835-pixelvalve2"
> +- reg:         Physical base address and length of the PV's registers
> +- interrupts:  The interrupt number
> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
> +
> +Required properties for HVS:
> +- compatible:  Should be "brcm,bcm2835-hvs"
> +- reg:         Physical base address and length of the HVS's registers
> +- interrupts:  The interrupt number
> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
> +
> +Required properties for HDMI

Is HDMI the only output possibility? If not, then you should have OF
graph nodes describing the connection between HDMI block and HVS (or
PV?).

> +- compatible:  Should be "brcm,bcm2835-hdmi"
> +- reg:         Physical base address and length of the two register ranges
> +                 ("HDMI" and "HD", in that order)
> +- interrupts:  The interrupt numbers
> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
> +- ddc:         phandle of the I2C controller used for DDC EDID probing
> +- clocks:      a) hdmi: The HDMI state machine clock
> +               b) pixel: The pixel clock.
> +
> +Optional properties for HDMI:
> +- hpd-gpio:    The GPIO pin for HDMI hotplug detect (if it doesn't appear

*-gpio is deprecated, so "hpd-gpios".

Really, I think this and ddc should be in hdmi-connector binding node.
What has been done for bindings so far is all over the map though.

> +                 as an interrupt/status bit in the HDMI controller
> +                 itself).  See bindings/pinctrl/brcm,bcm2835-gpio.txt
> +
> +Example:
> +pixelvalve@7e807000 {
> +       compatible = "brcm,bcm2835-pixelvalve2";
> +       reg = <0x7e807000 0x100>;
> +       interrupts = <2 10>; /* pixelvalve */
> +};
> +
> +hvs@7e400000 {
> +       compatible = "brcm,bcm2835-hvs";
> +       reg = <0x7e400000 0x6000>;
> +       interrupts = <2 1>;
> +};
> +
> +hdmi: hdmi@7e902000 {
> +       compatible = "brcm,bcm2835-hdmi";
> +       reg = <0x7e902000 0x600>,
> +             <0x7e808000 0x100>;
> +       interrupts = <2 8>, <2 9>;
> +       ddc = <&i2c2>;
> +       hpd-gpio = <&gpio 46 GPIO_ACTIVE_HIGH>;
> +       clocks = <&clocks BCM2835_PLLH_PIX>,
> +                <&clocks BCM2835_CLOCK_HSM>;
> +       clock-names = "pixel", "hdmi";
> +};
> +
> +vc4: gpu@7e4c0000 {
> +       compatible = "brcm,bcm2835-vc4";
> +};
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
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
Eric Anholt Oct. 13, 2015, 5:59 p.m. UTC | #3
Sebastian Reichel <sre@kernel.org> writes:

> Hi,
>
> On Fri, Oct 09, 2015 at 02:27:42PM -0700, Eric Anholt wrote:
>> VC4 is the GPU (display and 3D) subsystem present on the 2835 and some
>> other Broadcom SoCs.
>> 
>> This binding follows the model of msm, imx, sti, and others, where
>> there is a subsystem node for the whole GPU, with nodes for the
>> individual HW components within it.
>
> I think it would be useful to have the acronyms written out one time
> in the document (VC4, HVS).

Done.
Eric Anholt Oct. 13, 2015, 6:17 p.m. UTC | #4
Rob Herring <robh@kernel.org> writes:

> On Fri, Oct 9, 2015 at 4:27 PM, Eric Anholt <eric@anholt.net> wrote:
>> ---
>>
>> v2: Extend the commit message, fix several nits from Stephen Warren.
>> v3: Rename the compatibility strings, clean up node names, drop the
>>     unnecessary lists of components.  Use compatibility strings for
>>     choosing CRTC HVS channel numbers.  Document the HDMI clock usage.
>>
>>  .../devicetree/bindings/gpu/brcm,bcm-vc4.txt       | 64 ++++++++++++++++++++++
>
> Can you put this in bindings/display/ instead? Things are moving there in 4.4.

Sure.

>>  1 file changed, 64 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
>> new file mode 100644
>> index 0000000..175bcde
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
>> @@ -0,0 +1,64 @@
>> +Broadcom VC4 GPU
>> +
>> +The VC4 device present on the Raspberry Pi includes a display system
>> +with HDMI output and the HVS scaler for compositing display planes.
>> +
>> +Required properties for VC4:
>> +- compatible:  Should be "brcm,bcm2835-vc4"
>
> reg property? interrupts? clocks?

This is the subsystem node.  It has no other properties currently.

>> +Required properties for Pixel Valve:
>> +- compatible:  Should be one of "brcm,bcm2835-pixelvalve0",
>> +               "brcm,bcm2835-pixelvalve1", or "brcm,bcm2835-pixelvalve2"
>> +- reg:         Physical base address and length of the PV's registers
>> +- interrupts:  The interrupt number
>> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>> +
>> +Required properties for HVS:
>> +- compatible:  Should be "brcm,bcm2835-hvs"
>> +- reg:         Physical base address and length of the HVS's registers
>> +- interrupts:  The interrupt number
>> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>> +
>> +Required properties for HDMI
>
> Is HDMI the only output possibility? If not, then you should have OF
> graph nodes describing the connection between HDMI block and HVS (or
> PV?).

I'm using compatible strings for the different instances of the module:
brcm,bcm2835-pixelvalve0/1/2.  This lets the connections get wired up
cleanly and understandably within the driver.  I spent a long time
trying to come up with an OF graph-based implementation, and I
eventually gave up.

>> +- compatible:  Should be "brcm,bcm2835-hdmi"
>> +- reg:         Physical base address and length of the two register ranges
>> +                 ("HDMI" and "HD", in that order)
>> +- interrupts:  The interrupt numbers
>> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>> +- ddc:         phandle of the I2C controller used for DDC EDID probing
>> +- clocks:      a) hdmi: The HDMI state machine clock
>> +               b) pixel: The pixel clock.
>> +
>> +Optional properties for HDMI:
>> +- hpd-gpio:    The GPIO pin for HDMI hotplug detect (if it doesn't appear
>
> *-gpio is deprecated, so "hpd-gpios".
>
> Really, I think this and ddc should be in hdmi-connector binding node.
> What has been done for bindings so far is all over the map though.

You say hpd-gpios is deprectated, but that I should use the
hdmi-connector binding that uses hpd-gpios.  Which one is it?  If
hpd-gpios deprecated, what is supposed to be used instead?
Rob Herring Oct. 13, 2015, 9:56 p.m. UTC | #5
On Tue, Oct 13, 2015 at 1:17 PM, Eric Anholt <eric@anholt.net> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> On Fri, Oct 9, 2015 at 4:27 PM, Eric Anholt <eric@anholt.net> wrote:
>>> ---
>>>
>>> v2: Extend the commit message, fix several nits from Stephen Warren.
>>> v3: Rename the compatibility strings, clean up node names, drop the
>>>     unnecessary lists of components.  Use compatibility strings for
>>>     choosing CRTC HVS channel numbers.  Document the HDMI clock usage.
>>>
>>>  .../devicetree/bindings/gpu/brcm,bcm-vc4.txt       | 64 ++++++++++++++++++++++
>>
>> Can you put this in bindings/display/ instead? Things are moving there in 4.4.
>
> Sure.
>
>>>  1 file changed, 64 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
>>> new file mode 100644
>>> index 0000000..175bcde
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
>>> @@ -0,0 +1,64 @@
>>> +Broadcom VC4 GPU
>>> +
>>> +The VC4 device present on the Raspberry Pi includes a display system
>>> +with HDMI output and the HVS scaler for compositing display planes.
>>> +
>>> +Required properties for VC4:
>>> +- compatible:  Should be "brcm,bcm2835-vc4"
>>
>> reg property? interrupts? clocks?
>
> This is the subsystem node.  It has no other properties currently.

In the example, it looks like the gpu. You also have a unit address
which implies you need a reg property.

>>> +Required properties for Pixel Valve:
>>> +- compatible:  Should be one of "brcm,bcm2835-pixelvalve0",
>>> +               "brcm,bcm2835-pixelvalve1", or "brcm,bcm2835-pixelvalve2"
>>> +- reg:         Physical base address and length of the PV's registers
>>> +- interrupts:  The interrupt number
>>> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>> +
>>> +Required properties for HVS:
>>> +- compatible:  Should be "brcm,bcm2835-hvs"
>>> +- reg:         Physical base address and length of the HVS's registers
>>> +- interrupts:  The interrupt number
>>> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>> +
>>> +Required properties for HDMI
>>
>> Is HDMI the only output possibility? If not, then you should have OF
>> graph nodes describing the connection between HDMI block and HVS (or
>> PV?).
>
> I'm using compatible strings for the different instances of the module:
> brcm,bcm2835-pixelvalve0/1/2.  This lets the connections get wired up
> cleanly and understandably within the driver.  I spent a long time
> trying to come up with an OF graph-based implementation, and I
> eventually gave up.

I missed that before, but sorry, that's not how you should be using
compatible strings. What was your issue with OF graph? It can be
difficult to parse. I'd like to improve that with more common parsing
code.


>>> +- compatible:  Should be "brcm,bcm2835-hdmi"
>>> +- reg:         Physical base address and length of the two register ranges
>>> +                 ("HDMI" and "HD", in that order)
>>> +- interrupts:  The interrupt numbers
>>> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>> +- ddc:         phandle of the I2C controller used for DDC EDID probing
>>> +- clocks:      a) hdmi: The HDMI state machine clock
>>> +               b) pixel: The pixel clock.
>>> +
>>> +Optional properties for HDMI:
>>> +- hpd-gpio:    The GPIO pin for HDMI hotplug detect (if it doesn't appear
>>
>> *-gpio is deprecated, so "hpd-gpios".
>>
>> Really, I think this and ddc should be in hdmi-connector binding node.
>> What has been done for bindings so far is all over the map though.
>
> You say hpd-gpios is deprectated, but that I should use the
> hdmi-connector binding that uses hpd-gpios.  Which one is it?  If
> hpd-gpios deprecated, what is supposed to be used instead?

No, I said "hpd-gpio" with no "s" is deprecated. In other words,
always use -gpios whether it is 1 or more gpio.

The connector part is a separate issue of the location of these
properties. If you think about it, the gpio line and I2C bus have
nothing to do with the HDMI node. That's different than cases of HDMI
bridges which have a HPD signal and dedicated I2C controller. Most
examples in the kernel have not followed this and do as you have. I
only have a desire to have common binding and code to handle
connectors at this point, but that is the direction I want to go.

Rob
--
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
Eric Anholt Oct. 21, 2015, 8:57 a.m. UTC | #6
Rob Herring <robh@kernel.org> writes:

> On Tue, Oct 13, 2015 at 1:17 PM, Eric Anholt <eric@anholt.net> wrote:
>> Rob Herring <robh@kernel.org> writes:
>>
>>> On Fri, Oct 9, 2015 at 4:27 PM, Eric Anholt <eric@anholt.net> wrote:
>>>> ---
>>>>
>>>> v2: Extend the commit message, fix several nits from Stephen Warren.
>>>> v3: Rename the compatibility strings, clean up node names, drop the
>>>>     unnecessary lists of components.  Use compatibility strings for
>>>>     choosing CRTC HVS channel numbers.  Document the HDMI clock usage.
>>>>
>>>>  .../devicetree/bindings/gpu/brcm,bcm-vc4.txt       | 64 ++++++++++++++++++++++
>>>
>>> Can you put this in bindings/display/ instead? Things are moving there in 4.4.
>>
>> Sure.
>>
>>>>  1 file changed, 64 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
>>>> new file mode 100644
>>>> index 0000000..175bcde
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
>>>> @@ -0,0 +1,64 @@
>>>> +Broadcom VC4 GPU
>>>> +
>>>> +The VC4 device present on the Raspberry Pi includes a display system
>>>> +with HDMI output and the HVS scaler for compositing display planes.
>>>> +
>>>> +Required properties for VC4:
>>>> +- compatible:  Should be "brcm,bcm2835-vc4"
>>>
>>> reg property? interrupts? clocks?
>>
>> This is the subsystem node.  It has no other properties currently.
>
> In the example, it looks like the gpu. You also have a unit address
> which implies you need a reg property.

I'll drop the unit address.

>>>> +Required properties for Pixel Valve:
>>>> +- compatible:  Should be one of "brcm,bcm2835-pixelvalve0",
>>>> +               "brcm,bcm2835-pixelvalve1", or "brcm,bcm2835-pixelvalve2"
>>>> +- reg:         Physical base address and length of the PV's registers
>>>> +- interrupts:  The interrupt number
>>>> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>>> +
>>>> +Required properties for HVS:
>>>> +- compatible:  Should be "brcm,bcm2835-hvs"
>>>> +- reg:         Physical base address and length of the HVS's registers
>>>> +- interrupts:  The interrupt number
>>>> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>>> +
>>>> +Required properties for HDMI
>>>
>>> Is HDMI the only output possibility? If not, then you should have OF
>>> graph nodes describing the connection between HDMI block and HVS (or
>>> PV?).
>>
>> I'm using compatible strings for the different instances of the module:
>> brcm,bcm2835-pixelvalve0/1/2.  This lets the connections get wired up
>> cleanly and understandably within the driver.  I spent a long time
>> trying to come up with an OF graph-based implementation, and I
>> eventually gave up.
>
> I missed that before, but sorry, that's not how you should be using
> compatible strings. What was your issue with OF graph? It can be
> difficult to parse. I'd like to improve that with more common parsing
> code.

pv2 is definitely different hardware than pv0/1 (some different source
files, not just connections at hw module instantiation).  This seems to
be an obvious case for compatible strings.  I haven't looked enough into
pv0/1 to tell if they're different at a hardware level, since I haven't
added any support for the encoders using them yet.

OF graph: Doesn't appear to solve any problems that the driver has.  The
pv needs to know what kind of encoder is downstream to make choices
about its programming.  The bits in its documentation refer to encoders
by name.  I could write up a ton of DT bindings trying to generate an
abstraction so that the driver could map back to what it has today, but
it would just make the driver more obfuscated and error prone.  It's
much cleaner to let the two driver submodules talk to each other and
sort it out.

>>>> +- compatible:  Should be "brcm,bcm2835-hdmi"
>>>> +- reg:         Physical base address and length of the two register ranges
>>>> +                 ("HDMI" and "HD", in that order)
>>>> +- interrupts:  The interrupt numbers
>>>> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>>> +- ddc:         phandle of the I2C controller used for DDC EDID probing
>>>> +- clocks:      a) hdmi: The HDMI state machine clock
>>>> +               b) pixel: The pixel clock.
>>>> +
>>>> +Optional properties for HDMI:
>>>> +- hpd-gpio:    The GPIO pin for HDMI hotplug detect (if it doesn't appear
>>>
>>> *-gpio is deprecated, so "hpd-gpios".
>>>
>>> Really, I think this and ddc should be in hdmi-connector binding node.
>>> What has been done for bindings so far is all over the map though.
>>
>> You say hpd-gpios is deprectated, but that I should use the
>> hdmi-connector binding that uses hpd-gpios.  Which one is it?  If
>> hpd-gpios deprecated, what is supposed to be used instead?
>
> No, I said "hpd-gpio" with no "s" is deprecated. In other words,
> always use -gpios whether it is 1 or more gpio.

Fixed.

> The connector part is a separate issue of the location of these
> properties. If you think about it, the gpio line and I2C bus have
> nothing to do with the HDMI node. That's different than cases of HDMI
> bridges which have a HPD signal and dedicated I2C controller. Most
> examples in the kernel have not followed this and do as you have. I
> only have a desire to have common binding and code to handle
> connectors at this point, but that is the direction I want to go.

If you come up with common code that makes driver development easier
instead of harder, I'll be interested to see.
Rob Herring Oct. 21, 2015, 10:08 p.m. UTC | #7
On Wed, Oct 21, 2015 at 3:57 AM, Eric Anholt <eric@anholt.net> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> On Tue, Oct 13, 2015 at 1:17 PM, Eric Anholt <eric@anholt.net> wrote:
>>> Rob Herring <robh@kernel.org> writes:
>>>
>>>> On Fri, Oct 9, 2015 at 4:27 PM, Eric Anholt <eric@anholt.net> wrote:

[...]

>>>>> +Required properties for Pixel Valve:
>>>>> +- compatible:  Should be one of "brcm,bcm2835-pixelvalve0",
>>>>> +               "brcm,bcm2835-pixelvalve1", or "brcm,bcm2835-pixelvalve2"
>>>>> +- reg:         Physical base address and length of the PV's registers
>>>>> +- interrupts:  The interrupt number
>>>>> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>>>> +
>>>>> +Required properties for HVS:
>>>>> +- compatible:  Should be "brcm,bcm2835-hvs"
>>>>> +- reg:         Physical base address and length of the HVS's registers
>>>>> +- interrupts:  The interrupt number
>>>>> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>>>> +
>>>>> +Required properties for HDMI
>>>>
>>>> Is HDMI the only output possibility? If not, then you should have OF
>>>> graph nodes describing the connection between HDMI block and HVS (or
>>>> PV?).
>>>
>>> I'm using compatible strings for the different instances of the module:
>>> brcm,bcm2835-pixelvalve0/1/2.  This lets the connections get wired up
>>> cleanly and understandably within the driver.  I spent a long time
>>> trying to come up with an OF graph-based implementation, and I
>>> eventually gave up.
>>
>> I missed that before, but sorry, that's not how you should be using
>> compatible strings. What was your issue with OF graph? It can be
>> difficult to parse. I'd like to improve that with more common parsing
>> code.
>
> pv2 is definitely different hardware than pv0/1 (some different source
> files, not just connections at hw module instantiation).  This seems to
> be an obvious case for compatible strings.  I haven't looked enough into
> pv0/1 to tell if they're different at a hardware level, since I haven't
> added any support for the encoders using them yet.

If you can come up with something more descriptive that would be good.
Certain features or capabilities for example. If not, I guess this is
okay. I worry that the numbering could be different in a future part,
but the IP otherwise unchanged. Then you would not be able to use
these compatible strings.

> OF graph: Doesn't appear to solve any problems that the driver has.  The
> pv needs to know what kind of encoder is downstream to make choices
> about its programming.  The bits in its documentation refer to encoders
> by name.  I could write up a ton of DT bindings trying to generate an
> abstraction so that the driver could map back to what it has today, but
> it would just make the driver more obfuscated and error prone.  It's
> much cleaner to let the two driver submodules talk to each other and
> sort it out.

Okay.

>>>>> +- compatible:  Should be "brcm,bcm2835-hdmi"
>>>>> +- reg:         Physical base address and length of the two register ranges
>>>>> +                 ("HDMI" and "HD", in that order)
>>>>> +- interrupts:  The interrupt numbers
>>>>> +                 See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
>>>>> +- ddc:         phandle of the I2C controller used for DDC EDID probing
>>>>> +- clocks:      a) hdmi: The HDMI state machine clock
>>>>> +               b) pixel: The pixel clock.
>>>>> +
>>>>> +Optional properties for HDMI:
>>>>> +- hpd-gpio:    The GPIO pin for HDMI hotplug detect (if it doesn't appear
>>>>
>>>> *-gpio is deprecated, so "hpd-gpios".
>>>>
>>>> Really, I think this and ddc should be in hdmi-connector binding node.
>>>> What has been done for bindings so far is all over the map though.
>>>
>>> You say hpd-gpios is deprectated, but that I should use the
>>> hdmi-connector binding that uses hpd-gpios.  Which one is it?  If
>>> hpd-gpios deprecated, what is supposed to be used instead?
>>
>> No, I said "hpd-gpio" with no "s" is deprecated. In other words,
>> always use -gpios whether it is 1 or more gpio.
>
> Fixed.
>
>> The connector part is a separate issue of the location of these
>> properties. If you think about it, the gpio line and I2C bus have
>> nothing to do with the HDMI node. That's different than cases of HDMI
>> bridges which have a HPD signal and dedicated I2C controller. Most
>> examples in the kernel have not followed this and do as you have. I
>> only have a desire to have common binding and code to handle
>> connectors at this point, but that is the direction I want to go.
>
> If you come up with common code that makes driver development easier
> instead of harder, I'll be interested to see.

The start is common bindings. :)

Rob
--
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
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
new file mode 100644
index 0000000..175bcde
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
@@ -0,0 +1,64 @@ 
+Broadcom VC4 GPU
+
+The VC4 device present on the Raspberry Pi includes a display system
+with HDMI output and the HVS scaler for compositing display planes.
+
+Required properties for VC4:
+- compatible:	Should be "brcm,bcm2835-vc4"
+
+Required properties for Pixel Valve:
+- compatible:	Should be one of "brcm,bcm2835-pixelvalve0",
+		"brcm,bcm2835-pixelvalve1", or "brcm,bcm2835-pixelvalve2"
+- reg:		Physical base address and length of the PV's registers
+- interrupts:	The interrupt number
+		  See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
+
+Required properties for HVS:
+- compatible:	Should be "brcm,bcm2835-hvs"
+- reg:		Physical base address and length of the HVS's registers
+- interrupts:	The interrupt number
+		  See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
+
+Required properties for HDMI
+- compatible:	Should be "brcm,bcm2835-hdmi"
+- reg:		Physical base address and length of the two register ranges
+		  ("HDMI" and "HD", in that order)
+- interrupts:	The interrupt numbers
+		  See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
+- ddc:		phandle of the I2C controller used for DDC EDID probing
+- clocks:	a) hdmi: The HDMI state machine clock
+		b) pixel: The pixel clock.
+
+Optional properties for HDMI:
+- hpd-gpio:	The GPIO pin for HDMI hotplug detect (if it doesn't appear
+		  as an interrupt/status bit in the HDMI controller
+		  itself).  See bindings/pinctrl/brcm,bcm2835-gpio.txt
+
+Example:
+pixelvalve@7e807000 {
+	compatible = "brcm,bcm2835-pixelvalve2";
+	reg = <0x7e807000 0x100>;
+	interrupts = <2 10>; /* pixelvalve */
+};
+
+hvs@7e400000 {
+	compatible = "brcm,bcm2835-hvs";
+	reg = <0x7e400000 0x6000>;
+	interrupts = <2 1>;
+};
+
+hdmi: hdmi@7e902000 {
+	compatible = "brcm,bcm2835-hdmi";
+	reg = <0x7e902000 0x600>,
+	      <0x7e808000 0x100>;
+	interrupts = <2 8>, <2 9>;
+	ddc = <&i2c2>;
+	hpd-gpio = <&gpio 46 GPIO_ACTIVE_HIGH>;
+	clocks = <&clocks BCM2835_PLLH_PIX>,
+		 <&clocks BCM2835_CLOCK_HSM>;
+	clock-names = "pixel", "hdmi";
+};
+
+vc4: gpu@7e4c0000 {
+	compatible = "brcm,bcm2835-vc4";
+};