Re: [PATCH 02/12] drm/etnaviv: add devicetree bindings

[Lucas Stach <l.stach@pengutronix.de>]

Am Freitag, den 04.12.2015, 10:29 -0600 schrieb Rob Herring:
> On Fri, Dec 04, 2015 at 02:59:54PM +0100, Lucas Stach wrote:
> > Etnaviv follows the same priciple as imx-drm to have a virtual
> > master device node to bind all the individual GPU cores together
> > into one DRM device.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  .../bindings/display/etnaviv/etnaviv-drm.txt       | 46 ++++++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
> > new file mode 100644
> > index 000000000000..19fde29dc1d7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
> > @@ -0,0 +1,46 @@
> > +Etnaviv DRM master device
> > +================================
> > +
> > +The Etnaviv DRM master device is a virtual device needed to list all
> > +Vivante GPU cores that comprise the GPU subsystem.
> > +
> > +Required properties:
> > +- compatible: Should be one of
> > +    "fsl,imx-gpu-subsystem"
> > +    "marvell,dove-gpu-subsystem"
> > +- cores: Should contain a list of phandles pointing to Vivante GPU devices
> > +
> > +example:
> > +
> > +gpu-subsystem {
> > +	compatible = "fsl,imx-gpu-subsystem";
> > +	cores = <&gpu_2d>, <&gpu_3d>;
> > +};
> 
> Yeah, I'm not really a fan of doing this simply because DRM wants 1 
> driver.
> 
I'm aware of that, but I don't see much value in kicking this discussion
around for every DRM driver submission. This is the binding that has
emerged from a lengthy discussion at KS 2013 in Edinburgh and at least
allows us to standardize on _something_. Also ALSA does a similar thing
to bind codecs and CPU interfaces together.

> > +
> > +
> > +Vivante GPU core devices
> > +====================
> > +
> > +Required properties:
> > +- compatible: Should be "vivante,gc"
> 
> This should at least have the specific cores listed like gc-5000 or 
> whatever the numbering is. As is, I can't even tell if this a 2d or 3d 
> core.
> 
There is really no need for this. The cores have identification
registers that are much more accurate than what we could describe with a
compatible value. So using a more specific compatible just provides
redundant (and possibly wrong) information.

> Also, it probably should have an SOC specific property to deal with SOC 
> specific configuration or integration.
> 
Same as above really. Parts of the identification registers are
different for each SoC integration, even if it's the same IP core, so we
can just derive any needed driver behavior differences from that.

> > +- reg: should be register base and length as documented in the
> > +  datasheet
> > +- interrupts: Should contain the cores interrupt line
> > +- clocks: should contain one clock for entry in clock-names
> > +  see Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +- clock-names:
> > +   - "bus":    AXI/register clock
> > +   - "core":   GPU core clock
> > +   - "shader": Shader clock (only required if GPU has feature PIPE_3D)
> > +
> > +example:
> > +
> > +gpu_3d: gpu@00130000 {
> > +	compatible = "vivante,gc";
> > +	reg = <0x00130000 0x4000>;
> > +	interrupts = <0 9 IRQ_TYPE_LEVEL_HIGH>;
> > +	clocks = <&clks IMX6QDL_CLK_GPU3D_AXI>,
> > +	         <&clks IMX6QDL_CLK_GPU3D_CORE>,
> > +	         <&clks IMX6QDL_CLK_GPU3D_SHADER>;
> > +	clock-names = "bus", "core", "shader";
> > +};
> > -- 
> > 2.6.2
> > 

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel