On 26.09.2017 08:11, Stephen Warren wrote:

On 09/25/2017 05:45 PM, Dmitry Osipenko wrote:

quoted

On 26.09.2017 02:01, Stephen Warren wrote:

quoted

On 09/25/2017 04:15 PM, Dmitry Osipenko wrote:

quoted

Video decoder, found on NVIDIA Tegra20 SoC, supports a standard set of
video formats like H.264 / MPEG-4 / WMV / VC1. Currently driver supports
decoding of CAVLC H.264 only.

Note: I don't know anything much about video decoding on Tegra (just NV desktop
GPUs, and that was a while ago), but I had a couple small comments on the DT
binding:

quoted

diff --git
a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt

quoted

+NVIDIA Tegra Video Decoder Engine
+
+Required properties:
+- compatible : "nvidia,tegra20-vde"
+- reg : Must contain 2 register ranges: registers and IRAM area.
+- reg-names : Must include the following entries:
+  - regs
+  - iram

I think the IRAM region needs more explanation: What is the region used for and
by what? Can it be moved, and if so does the move need to be co-ordinated with
any other piece of SW?

IRAM region is used by Video Decoder HW for internal use and some of decoding
parameters are supplied via IRAM, like frames order list. AFAIK IRAM addresses
are hardwired in HW and aren't movable, it is not 100% but I'm pretty sure.
Should it be explained in the binding?

I think this should be briefly mentioned, yes. Otherwise at least people
who don't know the VDE HW well (like me) will wonder why on earth VDE
interacts with IRAM at all. I would have assumed all parameters were
supplied via registers or via descriptors in DRAM.

Thanks.

I also forgot to mention that VDE scrubs that IRAM region on HW reset. So yeah,
it's definitely a part of HW definition. I'll add a brief explanation to the
binding.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html