linux-kernel - Re: [PATCH v2 08/10] dt-bindings: media: Document bindings for the Sunxi-Cedrus VPU driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 20 Apr 2018 09:22:20 +0200
From:   Paul Kocialkowski <paul.kocialkowski@...tlin.com>
To:     Tomasz Figa <tfiga@...omium.org>,
        Philipp Zabel <p.zabel@...gutronix.de>
Cc:     Linux Media Mailing List <linux-media@...r.kernel.org>,
        devicetree@...r.kernel.org,
        "list@....net:IOMMU DRIVERS <iommu@...ts.linux-foundation.org>, Joerg "
         "Roedel <joro@...tes.org>," <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-sunxi@...glegroups.com,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Maxime Ripard <maxime.ripard@...tlin.com>, wens@...e.org,
        Pawel Osciak <pawel@...iak.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Arnd Bergmann <arnd@...db.de>,
        Alexandre Courbot <acourbot@...omium.org>
Subject: Re: [PATCH v2 08/10] dt-bindings: media: Document bindings for the
 Sunxi-Cedrus VPU driver

Hi and thanks for the review,

On Fri, 2018-04-20 at 01:31 +0000, Tomasz Figa wrote:
> Hi Paul, Philipp,
> 
> On Fri, Apr 20, 2018 at 1:04 AM Philipp Zabel <p.zabel@...gutronix.de>
> wrote:
> 
> > Hi Paul,
> > On Thu, 2018-04-19 at 17:45 +0200, Paul Kocialkowski wrote:
> > > This adds a device-tree binding document that specifies the
> > > properties
> > > used by the Sunxi-Cedurs VPU driver, as well as examples.
> > > 
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
> > > ---
> > >  .../devicetree/bindings/media/sunxi-cedrus.txt     | 50
> 
> ++++++++++++++++++++++
> > >  1 file changed, 50 insertions(+)
> > >  create mode 100644
> 
> Documentation/devicetree/bindings/media/sunxi-cedrus.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/sunxi-
> > > cedrus.txt
> 
> b/Documentation/devicetree/bindings/media/sunxi-cedrus.txt
> > > new file mode 100644
> > > index 000000000000..71ad3f9c3352
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/sunxi-cedrus.txt
> > > @@ -0,0 +1,50 @@
> > > +Device-tree bindings for the VPU found in Allwinner SoCs,
> > > referred to
> 
> as the
> > > +Video Engine (VE) in Allwinner literature.
> > > +
> > > +The VPU can only access the first 256 MiB of DRAM, that are DMA-
> > > mapped
> 
> starting
> > > +from the DRAM base. This requires specific memory allocation and
> 
> handling.
> 
> And no IOMMU? Brings back memories.

Exactly, no IOMMU so we don't have much choice but cope with that
hardware limitation...

> > > +
> > > +Required properties:
> > > +- compatible         : "allwinner,sun4i-a10-video-engine";
> > > +- memory-region         : DMA pool for buffers allocation;
> > > +- clocks             : list of clock specifiers, corresponding to
> 
> entries in
> > > +                          the clock-names property;
> > > +- clock-names                : should contain "ahb", "mod" and
> > > "ram"
> 
> entries;
> > > +- assigned-clocks       : list of clocks assigned to the VE;
> > > +- assigned-clocks-rates : list of clock rates for the clocks
> > > assigned
> 
> to the VE;
> > > +- resets             : phandle for reset;
> > > +- interrupts         : should contain VE interrupt number;
> > > +- reg                        : should contain register base and
> > > length
> 
> of VE.
> > > +
> > > +Example:
> > > +
> > > +reserved-memory {
> > > +     #address-cells = <1>;
> > > +     #size-cells = <1>;
> > > +     ranges;
> > > +
> > > +     /* Address must be kept in the lower 256 MiBs of DRAM for
> > > VE. */
> > > +     ve_memory: cma@...00000 {
> > > +             compatible = "shared-dma-pool";
> > > +             reg = <0x4a000000 0x6000000>;
> > > +             no-map;
> > > +             linux,cma-default;
> > > +     };
> > > +};
> > > +
> > > +video-engine@...e000 {
> > This is not really required by any specification, and not as common
> > as
> > gpu@..., but could this reasonably be called "vpu@...e000" to follow
> > somewhat-common practice?
> 
> AFAIR the name is supposed to be somewhat readable for someone that
> doesn't know the hardware. To me, "video-engine" sounds more obvious
> than "vpu", but we actually use "codec" already, in case of MFC and
> JPEG codec on Exynos. If encode/decode is the only functionality of
> this block, I'd personally go with "codec". If it can do other things,
> e.g. scaling/rotation without encode/decode, I'd probably call it
> "video-processor".

I agree that the term VPU is more commonly associated with video
decoding, while video engine could mean a number of things.

The reason I went with "video-engine" here (while still presenting the
driver as a VPU driver) is that Video Engine is the term used in
Allwinner's litterature. Other nodes in Allwinner device-trees generally
stick to these terms (for instance, we have "display-engine" nodes).
This also makes it easier to find the matching parts in the
documentation.

Cheers,

-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)