* [PATCH v3 0/2] NVIDIA Tegra20 video decoder driver
@ 2017-10-11 20:08 Dmitry Osipenko
2017-10-11 20:08 ` Dmitry Osipenko
2017-10-11 20:08 ` [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node Dmitry Osipenko
0 siblings, 2 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-11 20:08 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
This driver provides accelerated video decoding to NVIDIA Tegra20 SoC's,
it is a result of reverse-engineering efforts. Driver has been tested on
Toshiba AC100 and Acer A500, it should work on any Tegra20 device.
In userspace this driver is utilized by libvdpau-tegra [0] that implements
VDPAU interface, so any video player that supports VDPAU can provide
accelerated video decoding on Tegra20 on Linux.
[0] https://github.com/grate-driver/libvdpau-tegra
Change log:
v3:
- Suppressed compilation warnings reported by 'kbuild test robot'
v2:
- Addressed v1 review comments from Stephen Warren and Dan Carpenter
- Implemented runtime PM
- Miscellaneous code cleanups
- Changed 'TODO'
- CC'd media maintainers for the review as per Greg K-H request,
v1 can be viewed at https://lkml.org/lkml/2017/9/25/606
Dmitry Osipenko (2):
staging: Introduce NVIDIA Tegra20 video decoder driver
ARM: dts: tegra20: Add video decoder node
.../bindings/arm/tegra/nvidia,tegra20-vde.txt | 44 +
arch/arm/boot/dts/tegra20.dtsi | 17 +
drivers/staging/Kconfig | 2 +
drivers/staging/Makefile | 1 +
drivers/staging/tegra-vde/Kconfig | 6 +
drivers/staging/tegra-vde/Makefile | 1 +
drivers/staging/tegra-vde/TODO | 5 +
drivers/staging/tegra-vde/uapi.h | 101 ++
drivers/staging/tegra-vde/vde.c | 1109 ++++++++++++++++++++
9 files changed, 1286 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
create mode 100644 drivers/staging/tegra-vde/Kconfig
create mode 100644 drivers/staging/tegra-vde/Makefile
create mode 100644 drivers/staging/tegra-vde/TODO
create mode 100644 drivers/staging/tegra-vde/uapi.h
create mode 100644 drivers/staging/tegra-vde/vde.c
--
2.14.2
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
2017-10-11 20:08 [PATCH v3 0/2] NVIDIA Tegra20 video decoder driver Dmitry Osipenko
@ 2017-10-11 20:08 ` Dmitry Osipenko
2017-10-11 20:08 ` [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node Dmitry Osipenko
1 sibling, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-11 20:08 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren
Cc: devel, devicetree, linux-kernel, linux-tegra, Dan Carpenter, linux-media
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.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../bindings/arm/tegra/nvidia,tegra20-vde.txt | 44 +
drivers/staging/Kconfig | 2 +
drivers/staging/Makefile | 1 +
drivers/staging/tegra-vde/Kconfig | 6 +
drivers/staging/tegra-vde/Makefile | 1 +
drivers/staging/tegra-vde/TODO | 5 +
drivers/staging/tegra-vde/uapi.h | 101 ++
drivers/staging/tegra-vde/vde.c | 1109 ++++++++++++++++++++
8 files changed, 1269 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
create mode 100644 drivers/staging/tegra-vde/Kconfig
create mode 100644 drivers/staging/tegra-vde/Makefile
create mode 100644 drivers/staging/tegra-vde/TODO
create mode 100644 drivers/staging/tegra-vde/uapi.h
create mode 100644 drivers/staging/tegra-vde/vde.c
diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
new file mode 100644
index 000000000000..c3f847db8167
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
@@ -0,0 +1,44 @@
+NVIDIA Tegra Video Decoder Engine
+
+Required properties:
+- compatible : "nvidia,tegra20-vde"
+- reg : Must contain 2 register ranges: registers and IRAM region that
+ VDE uses for its internal needs and for passing some of decoding
+ parameters.
+- reg-names : Must include the following entries:
+ - regs
+ - iram
+- interrupts : Must contain an entry for each entry in interrupt-names.
+- interrupt-names : Must include the following entries:
+ - ucq-error
+ - sync-token
+ - bsev
+ - bsea
+ - sxe
+- clocks : Must contain an entry for each entry in clock-names.
+ See ../clocks/clock-bindings.txt for details.
+- clock-names : Must include the following entries:
+ - vde
+- resets : Must contain an entry for each entry in reset-names.
+ See ../reset/reset.txt for details.
+- reset-names : Must include the following entries:
+ - vde
+
+Example:
+
+vde@6001a000 {
+ compatible = "nvidia,tegra20-vde";
+ reg = <0x6001a000 0x3D00 /* VDE registers */
+ 0x40000400 0x3FC00>; /* IRAM region */
+ reg-names = "regs", "iram";
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, /* UCQ error interrupt */
+ <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */
+ <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */
+ <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, /* BSE-A interrupt */
+ <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
+ interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
+ clocks = <&tegra_car TEGRA20_CLK_VDE>;
+ clock-names = "vde";
+ resets = <&tegra_car 61>;
+ reset-names = "vde";
+};
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 554683912cff..10c982811093 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -118,4 +118,6 @@ source "drivers/staging/vboxvideo/Kconfig"
source "drivers/staging/pi433/Kconfig"
+source "drivers/staging/tegra-vde/Kconfig"
+
endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 8951c37d8d80..c5ef39767f22 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -49,3 +49,4 @@ obj-$(CONFIG_BCM2835_VCHIQ) += vc04_services/
obj-$(CONFIG_CRYPTO_DEV_CCREE) += ccree/
obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo/
obj-$(CONFIG_PI433) += pi433/
+obj-$(CONFIG_TEGRA_VDE) += tegra-vde/
diff --git a/drivers/staging/tegra-vde/Kconfig b/drivers/staging/tegra-vde/Kconfig
new file mode 100644
index 000000000000..730ee006de66
--- /dev/null
+++ b/drivers/staging/tegra-vde/Kconfig
@@ -0,0 +1,6 @@
+config TEGRA_VDE
+ tristate "NVIDIA Tegra Video Decoder Engine driver"
+ depends on ARCH_TEGRA_2x_SOC || COMPILE_TEST
+ help
+ Say Y here to enable support for the NVIDIA Tegra video decoder
+ driver.
diff --git a/drivers/staging/tegra-vde/Makefile b/drivers/staging/tegra-vde/Makefile
new file mode 100644
index 000000000000..e7c0df1174bf
--- /dev/null
+++ b/drivers/staging/tegra-vde/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_TEGRA_VDE) += vde.o
diff --git a/drivers/staging/tegra-vde/TODO b/drivers/staging/tegra-vde/TODO
new file mode 100644
index 000000000000..e98bbc7b3c19
--- /dev/null
+++ b/drivers/staging/tegra-vde/TODO
@@ -0,0 +1,5 @@
+TODO:
+ - Figure out how generic V4L2 API could be utilized by this driver,
+ implement it.
+
+Contact: Dmitry Osipenko <digetx@gmail.com>
diff --git a/drivers/staging/tegra-vde/uapi.h b/drivers/staging/tegra-vde/uapi.h
new file mode 100644
index 000000000000..36d76278d27c
--- /dev/null
+++ b/drivers/staging/tegra-vde/uapi.h
@@ -0,0 +1,101 @@
+/*
+ * Copyright (C) 2016-2017 Dmitry Osipenko <digetx@gmail.com>
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef _UAPI_TEGRA_VDE_H_
+#define _UAPI_TEGRA_VDE_H_
+
+#include <linux/types.h>
+#include <asm/ioctl.h>
+
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+#define FLAG_B_FRAME (1 << 0)
+#define FLAG_REFERENCE (1 << 1)
+
+struct tegra_vde_h264_frame {
+ __s32 y_fd;
+ __s32 cb_fd;
+ __s32 cr_fd;
+ __s32 aux_fd;
+ __u32 y_offset;
+ __u32 cb_offset;
+ __u32 cr_offset;
+ __u32 aux_offset;
+ __u32 frame_num;
+ __u32 flags;
+
+ __u32 reserved;
+} __attribute__((packed));
+
+struct tegra_vde_h264_decoder_ctx {
+ __s32 bitstream_data_fd;
+ __u32 bitstream_data_offset;
+
+ __u32 dpb_frames_ptr;
+ __u8 dpb_frames_nb;
+ __u8 dpb_ref_frames_with_earlier_poc_nb;
+
+ // SPS
+ __u8 baseline_profile;
+ __u8 level_idc;
+ __u8 log2_max_pic_order_cnt_lsb;
+ __u8 log2_max_frame_num;
+ __u8 pic_order_cnt_type;
+ __u8 direct_8x8_inference_flag;
+ __u8 pic_width_in_mbs;
+ __u8 pic_height_in_mbs;
+
+ // PPS
+ __u8 pic_init_qp;
+ __u8 deblocking_filter_control_present_flag;
+ __u8 constrained_intra_pred_flag;
+ __u8 chroma_qp_index_offset;
+ __u8 pic_order_present_flag;
+
+ // Slice header
+ __u8 num_ref_idx_l0_active_minus1;
+ __u8 num_ref_idx_l1_active_minus1;
+
+ __u32 reserved;
+} __attribute__((packed));
+
+#define VDE_IOCTL_BASE ('v' + 0x20)
+
+#define VDE_IO(nr) _IO(VDE_IOCTL_BASE, nr)
+#define VDE_IOR(nr, type) _IOR(VDE_IOCTL_BASE, nr, type)
+#define VDE_IOW(nr, type) _IOW(VDE_IOCTL_BASE, nr, type)
+#define VDE_IOWR(nr, type) _IOWR(VDE_IOCTL_BASE, nr, type)
+
+#define TEGRA_VDE_DECODE_H264 0x00
+
+#define TEGRA_VDE_IOCTL_DECODE_H264 \
+ VDE_IOW(TEGRA_VDE_DECODE_H264, struct tegra_vde_h264_decoder_ctx)
+
+#if defined(__cplusplus)
+}
+#endif
+
+#endif // _UAPI_TEGRA_VDE_H_
diff --git a/drivers/staging/tegra-vde/vde.c b/drivers/staging/tegra-vde/vde.c
new file mode 100644
index 000000000000..3ef0d5f2295b
--- /dev/null
+++ b/drivers/staging/tegra-vde/vde.c
@@ -0,0 +1,1109 @@
+/*
+ * NVIDIA Tegra20 Video decoder driver
+ *
+ * Copyright (C) 2016-2017 Dmitry Osipenko <digetx@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-buf.h>
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+
+#include <soc/tegra/pmc.h>
+
+#include "uapi.h"
+
+#define SXE(offt) (0x0000 + (offt)) /* Syntax Engine */
+#define BSEV(offt) (0x1000 + (offt)) /* Video Bitstream Engine */
+#define MBE(offt) (0x2000 + (offt)) /* Macroblock Engine */
+#define PPE(offt) (0x2200 + (offt)) /* Post-processing Engine */
+#define MCE(offt) (0x2400 + (offt)) /* Motion Compensation Eng. */
+#define TFE(offt) (0x2600 + (offt)) /* Transform Engine */
+#define VDMA(offt) (0x2A00 + (offt)) /* Video DMA */
+#define FRAMEID(offt) (0x3800 + (offt))
+
+#define ICMDQUE_WR 0x00
+#define CMDQUE_CONTROL 0x08
+#define INTR_STATUS 0x18
+#define BSE_INT_ENB 0x40
+#define BSE_CONFIG 0x44
+
+#define BSE_ICMDQUE_EMPTY BIT(3)
+#define BSE_DMA_BUSY BIT(23)
+
+#define TEGRA_VDE_TIMEOUT msecs_to_jiffies(1000)
+
+#define VDE_WR(__data, __addr) \
+do { \
+ pr_debug("%s: %d: 0x%08X => " #__addr ")\n", \
+ __func__, __LINE__, (u32)(__data)); \
+ writel_relaxed(__data, __addr); \
+} while (0)
+
+struct video_frame {
+ struct dma_buf_attachment *y_dmabuf_attachment;
+ struct dma_buf_attachment *cb_dmabuf_attachment;
+ struct dma_buf_attachment *cr_dmabuf_attachment;
+ struct dma_buf_attachment *aux_dmabuf_attachment;
+ struct sg_table *y_sgt;
+ struct sg_table *cb_sgt;
+ struct sg_table *cr_sgt;
+ struct sg_table *aux_sgt;
+ dma_addr_t y_paddr;
+ dma_addr_t cb_paddr;
+ dma_addr_t cr_paddr;
+ dma_addr_t aux_paddr;
+ u32 frame_num;
+ u32 flags;
+};
+
+struct tegra_vde {
+ phys_addr_t iram_lists_paddr;
+ void __iomem *regs;
+ void __iomem *iram;
+ struct mutex lock;
+ struct miscdevice miscdev;
+ struct reset_control *rst;
+ struct completion decode_completion;
+ struct clk *clk;
+};
+
+static void tegra_vde_set_bits(void __iomem *regs, u32 mask, u32 offset)
+{
+ u32 value = readl_relaxed(regs + offset);
+
+ VDE_WR(value | mask, regs + offset);
+}
+
+static int tegra_vde_wait_mbe(void __iomem *regs)
+{
+ u32 tmp;
+
+ return readl_relaxed_poll_timeout(regs + MBE(0x8C), tmp,
+ (tmp >= 0x10), 1, 100);
+}
+
+static int tegra_vde_setup_mbe_frame_idx(void __iomem *regs,
+ unsigned int refs_nb,
+ bool setup_refs)
+{
+ u32 frame_idx_enb_mask = 0;
+ u32 value;
+ unsigned int frame_idx;
+ unsigned int idx;
+ int err;
+
+ VDE_WR(0xD0000000 | (0 << 23), regs + MBE(0x80));
+ VDE_WR(0xD0200000 | (0 << 23), regs + MBE(0x80));
+
+ err = tegra_vde_wait_mbe(regs);
+ if (err)
+ return err;
+
+ if (!setup_refs)
+ return 0;
+
+ for (idx = 0, frame_idx = 1; idx < refs_nb; idx++, frame_idx++) {
+ VDE_WR(0xD0000000 | (frame_idx << 23), regs + MBE(0x80));
+ VDE_WR(0xD0200000 | (frame_idx << 23), regs + MBE(0x80));
+
+ frame_idx_enb_mask |= frame_idx << (6 * (idx % 4));
+
+ if (idx % 4 == 3 || idx == refs_nb - 1) {
+ value = 0xC0000000;
+ value |= (idx >> 2) << 24;
+ value |= frame_idx_enb_mask;
+
+ VDE_WR(value, regs + MBE(0x80));
+
+ err = tegra_vde_wait_mbe(regs);
+ if (err)
+ return err;
+
+ frame_idx_enb_mask = 0;
+ }
+ }
+
+ return 0;
+}
+
+static void tegra_vde_mbe_set_0xa_reg(void __iomem *regs, int reg, u32 val)
+{
+ VDE_WR(0xA0000000 | (reg << 24) | (val & 0xFFFF), regs + MBE(0x80));
+ VDE_WR(0xA0000000 | ((reg + 1) << 24) | (val >> 16), regs + MBE(0x80));
+}
+
+static int tegra_vde_wait_bsev(struct tegra_vde *vde, bool wait_dma)
+{
+ struct device *dev = vde->miscdev.parent;
+ u32 value;
+ int err;
+
+ err = readl_relaxed_poll_timeout(vde->regs + BSEV(INTR_STATUS), value,
+ !(value & BIT(2)), 1, 100);
+ if (err) {
+ dev_err(dev, "BSEV unknown bit timeout\n");
+ return err;
+ }
+
+ err = readl_relaxed_poll_timeout(vde->regs + BSEV(INTR_STATUS), value,
+ (value & BSE_ICMDQUE_EMPTY), 1, 100);
+ if (err) {
+ dev_err(dev, "BSEV ICMDQUE flush timeout\n");
+ return err;
+ }
+
+ if (!wait_dma)
+ return 0;
+
+ err = readl_relaxed_poll_timeout(vde->regs + BSEV(INTR_STATUS), value,
+ !(value & BSE_DMA_BUSY), 1, 100);
+ if (err) {
+ dev_err(dev, "BSEV DMA timeout\n");
+ return err;
+ }
+
+ return 0;
+}
+
+static int tegra_vde_push_to_bsev_icmdqueue(struct tegra_vde *vde,
+ u32 value, bool wait_dma)
+{
+ VDE_WR(value, vde->regs + BSEV(ICMDQUE_WR));
+
+ return tegra_vde_wait_bsev(vde, wait_dma);
+}
+
+static void tegra_vde_setup_frameid(void __iomem *regs,
+ struct video_frame *frame,
+ unsigned int frameid,
+ u32 mbs_width, u32 mbs_height)
+{
+ u32 y_paddr = frame ? frame->y_paddr : 0xFCDEAD00;
+ u32 cb_paddr = frame ? frame->cb_paddr : 0xFCDEAD00;
+ u32 cr_paddr = frame ? frame->cr_paddr : 0xFCDEAD00;
+ u32 value1 = frame ? ((mbs_width << 16) | mbs_height) : 0;
+ u32 value2 = frame ? ((((mbs_width + 1) >> 1) << 6) | 1) : 0;
+
+ VDE_WR( y_paddr >> 8, regs + FRAMEID(0x000 + frameid * 4));
+ VDE_WR(cb_paddr >> 8, regs + FRAMEID(0x100 + frameid * 4));
+ VDE_WR(cr_paddr >> 8, regs + FRAMEID(0x180 + frameid * 4));
+ VDE_WR(value1, regs + FRAMEID(0x080 + frameid * 4));
+ VDE_WR(value2, regs + FRAMEID(0x280 + frameid * 4));
+}
+
+static void tegra_setup_frameidx(void __iomem *regs,
+ struct video_frame *frames,
+ unsigned int frames_nb,
+ u32 mbs_width, u32 mbs_height)
+{
+ unsigned int idx;
+
+ for (idx = 0; idx < frames_nb; idx++)
+ tegra_vde_setup_frameid(regs, &frames[idx], idx,
+ mbs_width, mbs_height);
+ for (; idx < 17; idx++)
+ tegra_vde_setup_frameid(regs, NULL, idx, 0, 0);
+}
+
+static void tegra_vde_setup_iram_entry(void __iomem *iram_tables,
+ unsigned int table,
+ unsigned int row,
+ u32 value1, u32 value2)
+{
+ VDE_WR(value1, iram_tables + 0x80 * table + row * 8);
+ VDE_WR(value2, iram_tables + 0x80 * table + row * 8 + 4);
+}
+
+static void tegra_vde_setup_iram_tables(void __iomem *iram_tables,
+ struct video_frame *dpb_frames,
+ unsigned int ref_frames_nb,
+ unsigned int with_earlier_poc_nb)
+{
+ struct video_frame *frame;
+ u32 value, aux_paddr;
+ int with_later_poc_nb;
+ unsigned int i, k;
+
+ pr_debug("DPB: Frame 0: frame_num = %d\n", dpb_frames[0].frame_num);
+
+ pr_debug("REF L0:\n");
+
+ for (i = 0; i < 16; i++) {
+ if (i < ref_frames_nb) {
+ frame = &dpb_frames[i + 1];
+
+ aux_paddr = frame->aux_paddr;
+
+ value = (i + 1) << 26;
+ value |= !(frame->flags & FLAG_B_FRAME) << 25;
+ value |= 1 << 24;
+ value |= frame->frame_num;
+
+ pr_debug("\tFrame %d: frame_num = %d B_frame = %d\n",
+ i + 1, frame->frame_num,
+ (frame->flags & FLAG_B_FRAME));
+ } else {
+ aux_paddr = 0xFADEAD00;
+ value = 0;
+ }
+
+ tegra_vde_setup_iram_entry(iram_tables, 0, i, value, aux_paddr);
+ tegra_vde_setup_iram_entry(iram_tables, 1, i, value, aux_paddr);
+ tegra_vde_setup_iram_entry(iram_tables, 2, i, value, aux_paddr);
+ tegra_vde_setup_iram_entry(iram_tables, 3, i, value, aux_paddr);
+ }
+
+ if (!(dpb_frames[0].flags & FLAG_B_FRAME))
+ return;
+
+ if (with_earlier_poc_nb >= ref_frames_nb)
+ return;
+
+ with_later_poc_nb = ref_frames_nb - with_earlier_poc_nb;
+
+ pr_debug("REF L1: with_later_poc_nb %d with_earlier_poc_nb %d\n",
+ with_later_poc_nb, with_earlier_poc_nb);
+
+ for (i = 0, k = with_earlier_poc_nb; i < with_later_poc_nb; i++, k++) {
+ frame = &dpb_frames[k + 1];
+
+ aux_paddr = frame->aux_paddr;
+
+ value = (k + 1) << 26;
+ value |= !(frame->flags & FLAG_B_FRAME) << 25;
+ value |= 1 << 24;
+ value |= frame->frame_num;
+
+ pr_debug("\tFrame %d: frame_num = %d\n",
+ k + 1, frame->frame_num);
+
+ tegra_vde_setup_iram_entry(iram_tables, 2, i, value, aux_paddr);
+ }
+
+ for (k = 0; i < ref_frames_nb; i++, k++) {
+ frame = &dpb_frames[k + 1];
+
+ aux_paddr = frame->aux_paddr;
+
+ value = (k + 1) << 26;
+ value |= !(frame->flags & FLAG_B_FRAME) << 25;
+ value |= 1 << 24;
+ value |= frame->frame_num;
+
+ pr_debug("\tFrame %d: frame_num = %d\n",
+ k + 1, frame->frame_num);
+
+ tegra_vde_setup_iram_entry(iram_tables, 2, i, value, aux_paddr);
+ }
+}
+
+static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
+ struct tegra_vde_h264_decoder_ctx *ctx,
+ struct video_frame *dpb_frames,
+ phys_addr_t bitstream_data_paddr,
+ size_t bitstream_data_size,
+ unsigned int macroblocks_nb)
+{
+ struct device *dev = vde->miscdev.parent;
+ u32 value;
+ int err;
+
+ tegra_vde_set_bits(vde->regs, 0xA, SXE(0xF0));
+ tegra_vde_set_bits(vde->regs, 0xB, BSEV(CMDQUE_CONTROL));
+ tegra_vde_set_bits(vde->regs, 0x8002, MBE(0x50));
+ tegra_vde_set_bits(vde->regs, 0xA, MBE(0xA0));
+ tegra_vde_set_bits(vde->regs, 0xA, PPE(0x14));
+ tegra_vde_set_bits(vde->regs, 0xA, PPE(0x28));
+ tegra_vde_set_bits(vde->regs, 0xA00, MCE(0x08));
+ tegra_vde_set_bits(vde->regs, 0xA, TFE(0x00));
+ tegra_vde_set_bits(vde->regs, 0x5, VDMA(0x04));
+
+ VDE_WR(0x00000000, vde->regs + VDMA(0x1C));
+ VDE_WR(0x00000000, vde->regs + VDMA(0x00));
+ VDE_WR(0x00000007, vde->regs + VDMA(0x04));
+ VDE_WR(0x00000007, vde->regs + FRAMEID(0x200));
+ VDE_WR(0x00000005, vde->regs + TFE(0x04));
+ VDE_WR(0x00000000, vde->regs + MBE(0x84));
+ VDE_WR(0x00000010, vde->regs + SXE(0x08));
+ VDE_WR(0x00000150, vde->regs + SXE(0x54));
+ VDE_WR(0x0000054C, vde->regs + SXE(0x58));
+ VDE_WR(0x00000E34, vde->regs + SXE(0x5C));
+ VDE_WR(0x063C063C, vde->regs + MCE(0x10));
+ VDE_WR(0x0003FC00, vde->regs + BSEV(INTR_STATUS));
+ VDE_WR(0x0000150D, vde->regs + BSEV(BSE_CONFIG));
+ VDE_WR(0x00000100, vde->regs + BSEV(BSE_INT_ENB));
+ VDE_WR(0x00000000, vde->regs + BSEV(0x98));
+ VDE_WR(0x00000060, vde->regs + BSEV(0x9C));
+
+ memset_io(vde->iram + 512, 0, macroblocks_nb / 2);
+
+ tegra_setup_frameidx(vde->regs, dpb_frames, ctx->dpb_frames_nb,
+ ctx->pic_width_in_mbs, ctx->pic_height_in_mbs);
+
+ tegra_vde_setup_iram_tables(vde->iram, dpb_frames,
+ ctx->dpb_frames_nb - 1,
+ ctx->dpb_ref_frames_with_earlier_poc_nb);
+
+ VDE_WR(0x00000000, vde->regs + BSEV(0x8C));
+ VDE_WR(bitstream_data_paddr + bitstream_data_size,
+ vde->regs + BSEV(0x54));
+
+ value = ctx->pic_width_in_mbs << 11 | ctx->pic_height_in_mbs << 3;
+
+ VDE_WR(value, vde->regs + BSEV(0x88));
+
+ err = tegra_vde_wait_bsev(vde, false);
+ if (err)
+ return err;
+
+ err = tegra_vde_push_to_bsev_icmdqueue(vde, 0x800003FC, false);
+ if (err)
+ return err;
+
+ value = 0x01500000;
+ value |= ((vde->iram_lists_paddr + 512) >> 2) & 0xFFFF;
+
+ err = tegra_vde_push_to_bsev_icmdqueue(vde, value, true);
+ if (err)
+ return err;
+
+ err = tegra_vde_push_to_bsev_icmdqueue(vde, 0x840F054C, false);
+ if (err)
+ return err;
+
+ err = tegra_vde_push_to_bsev_icmdqueue(vde, 0x80000080, false);
+ if (err)
+ return err;
+
+ value = 0x0E340000 | ((vde->iram_lists_paddr >> 2) & 0xFFFF);
+
+ err = tegra_vde_push_to_bsev_icmdqueue(vde, value, true);
+ if (err)
+ return err;
+
+ value = 0x00800005;
+ value |= ctx->pic_width_in_mbs << 11;
+ value |= ctx->pic_height_in_mbs << 3;
+
+ VDE_WR(value, vde->regs + SXE(0x10));
+
+ value = !ctx->baseline_profile << 17;
+ value |= ctx->level_idc << 13;
+ value |= ctx->log2_max_pic_order_cnt_lsb << 7;
+ value |= ctx->pic_order_cnt_type << 5;
+ value |= ctx->log2_max_frame_num;
+
+ VDE_WR(value, vde->regs + SXE(0x40));
+
+ value = ctx->pic_init_qp << 25;
+ value |= !!(ctx->deblocking_filter_control_present_flag) << 2;
+ value |= !!ctx->pic_order_present_flag;
+
+ VDE_WR(value, vde->regs + SXE(0x44));
+
+ value = ctx->chroma_qp_index_offset;
+ value |= ctx->num_ref_idx_l0_active_minus1 << 5;
+ value |= ctx->num_ref_idx_l1_active_minus1 << 10;
+ value |= !!ctx->constrained_intra_pred_flag << 15;
+
+ VDE_WR(value, vde->regs + SXE(0x48));
+
+ value = 0x0C000000;
+ value |= !!(dpb_frames[0].flags & FLAG_B_FRAME) << 24;
+
+ VDE_WR(value, vde->regs + SXE(0x4C));
+
+ value = 0x03800000;
+ value |= min_t(size_t, bitstream_data_size, SZ_1M);
+
+ VDE_WR(value, vde->regs + SXE(0x68));
+
+ VDE_WR(bitstream_data_paddr, vde->regs + SXE(0x6C));
+
+ value = (1 << 28) | 5;
+ value |= ctx->pic_width_in_mbs << 11;
+ value |= ctx->pic_height_in_mbs << 3;
+
+ VDE_WR(value, vde->regs + MBE(0x80));
+
+ value = 0x26800000;
+ value |= ctx->level_idc << 4;
+ value |= !ctx->baseline_profile << 1;
+ value |= !!ctx->direct_8x8_inference_flag;
+
+ VDE_WR(value, vde->regs + MBE(0x80));
+
+ VDE_WR(0xF4000001, vde->regs + MBE(0x80));
+ VDE_WR(0x20000000, vde->regs + MBE(0x80));
+ VDE_WR(0xF4000101, vde->regs + MBE(0x80));
+
+ value = 0x20000000;
+ value |= ctx->chroma_qp_index_offset << 8;
+
+ VDE_WR(value, vde->regs + MBE(0x80));
+
+ err = tegra_vde_setup_mbe_frame_idx(vde->regs,
+ ctx->dpb_frames_nb - 1,
+ ctx->pic_order_cnt_type == 0);
+ if (err) {
+ dev_err(dev, "MBE frames setup failed\n");
+ return err;
+ }
+
+ tegra_vde_mbe_set_0xa_reg(vde->regs, 0, 0x000009FC);
+ tegra_vde_mbe_set_0xa_reg(vde->regs, 2, 0xF1DEAD00);
+ tegra_vde_mbe_set_0xa_reg(vde->regs, 4, 0xF2DEAD00);
+ tegra_vde_mbe_set_0xa_reg(vde->regs, 6, 0xF3DEAD00);
+ tegra_vde_mbe_set_0xa_reg(vde->regs, 8, dpb_frames[0].aux_paddr);
+
+ value = 0xFC000000;
+ value |= !!(dpb_frames[0].flags & FLAG_B_FRAME) << 2;
+
+ if (!ctx->baseline_profile)
+ value |= !!(dpb_frames[0].flags & FLAG_REFERENCE) << 1;
+
+ VDE_WR(value, vde->regs + MBE(0x80));
+
+ err = tegra_vde_wait_mbe(vde->regs);
+ if (err) {
+ dev_err(dev, "MBE programming failed\n");
+ return err;
+ }
+
+ return 0;
+}
+
+static void tegra_vde_decode_frame(struct tegra_vde *vde, int macroblocks_nb)
+{
+ reinit_completion(&vde->decode_completion);
+
+ VDE_WR(0x00000001, vde->regs + BSEV(0x8C));
+ VDE_WR(0x20000000 | (macroblocks_nb - 1), vde->regs + SXE(0x00));
+}
+
+static void tegra_vde_detach_and_put_dmabuf(struct dma_buf_attachment *a,
+ struct sg_table *sgt,
+ enum dma_data_direction dma_dir)
+{
+ struct dma_buf *dmabuf = a->dmabuf;
+
+ dma_buf_unmap_attachment(a, sgt, dma_dir);
+ dma_buf_detach(dmabuf, a);
+ dma_buf_put(dmabuf);
+}
+
+static int tegra_vde_attach_dmabuf(struct device *dev,
+ int fd,
+ unsigned long offset,
+ unsigned int min_size,
+ struct dma_buf_attachment **a,
+ phys_addr_t *paddr,
+ struct sg_table **s,
+ size_t *size,
+ enum dma_data_direction dma_dir)
+{
+ struct dma_buf_attachment *attachment;
+ struct dma_buf *dmabuf;
+ struct sg_table *sgt;
+ int err;
+
+ dmabuf = dma_buf_get(fd);
+ if (IS_ERR(dmabuf)) {
+ dev_err(dev, "Invalid dmabuf FD\n");
+ return PTR_ERR(dmabuf);
+ }
+
+ if ((u64)offset + min_size > dmabuf->size) {
+ dev_err(dev, "Too small dmabuf size %zu @0x%lX, "
+ "should be at least %d\n",
+ dmabuf->size, offset, min_size);
+ return -EINVAL;
+ }
+
+ attachment = dma_buf_attach(dmabuf, dev);
+ if (IS_ERR(attachment)) {
+ dev_err(dev, "Failed to attach dmabuf\n");
+ err = PTR_ERR(attachment);
+ goto err_put;
+ }
+
+ sgt = dma_buf_map_attachment(attachment, dma_dir);
+ if (IS_ERR(sgt)) {
+ dev_err(dev, "Failed to get dmabufs sg_table\n");
+ err = PTR_ERR(sgt);
+ goto err_detach;
+ }
+
+ if (sgt->nents != 1) {
+ dev_err(dev, "Sparse DMA region is unsupported\n");
+ err = -EINVAL;
+ goto err_unmap;
+ }
+
+ *paddr = sg_dma_address(sgt->sgl) + offset;
+ *a = attachment;
+ *s = sgt;
+
+ if (size)
+ *size = dmabuf->size - offset;
+
+ return 0;
+
+err_unmap:
+ dma_buf_unmap_attachment(attachment, sgt, dma_dir);
+err_detach:
+ dma_buf_detach(dmabuf, attachment);
+err_put:
+ dma_buf_put(dmabuf);
+
+ return err;
+}
+
+static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
+ struct video_frame *frame,
+ struct tegra_vde_h264_frame *source,
+ enum dma_data_direction dma_dir,
+ bool baseline_profile,
+ size_t csize)
+{
+ int err;
+
+ err = tegra_vde_attach_dmabuf(dev, source->y_fd,
+ source->y_offset, csize * 4,
+ &frame->y_dmabuf_attachment,
+ &frame->y_paddr,
+ &frame->y_sgt,
+ NULL, dma_dir);
+ if (err)
+ return err;
+
+ err = tegra_vde_attach_dmabuf(dev, source->cb_fd,
+ source->cb_offset, csize,
+ &frame->cb_dmabuf_attachment,
+ &frame->cb_paddr,
+ &frame->cb_sgt,
+ NULL, dma_dir);
+ if (err)
+ goto err_release_y;
+
+ err = tegra_vde_attach_dmabuf(dev, source->cr_fd,
+ source->cr_offset, csize,
+ &frame->cr_dmabuf_attachment,
+ &frame->cr_paddr,
+ &frame->cr_sgt,
+ NULL, dma_dir);
+ if (err)
+ goto err_release_cb;
+
+ if (baseline_profile) {
+ frame->aux_paddr = 0xF4DEAD00;
+ } else {
+ err = tegra_vde_attach_dmabuf(dev, source->aux_fd,
+ source->aux_offset, csize,
+ &frame->aux_dmabuf_attachment,
+ &frame->aux_paddr,
+ &frame->aux_sgt,
+ NULL, dma_dir);
+ if (err)
+ goto err_release_cr;
+ }
+
+ return 0;
+
+err_release_cr:
+ tegra_vde_detach_and_put_dmabuf(frame->cr_dmabuf_attachment,
+ frame->cr_sgt, dma_dir);
+err_release_cb:
+ tegra_vde_detach_and_put_dmabuf(frame->cb_dmabuf_attachment,
+ frame->cb_sgt, dma_dir);
+err_release_y:
+ tegra_vde_detach_and_put_dmabuf(frame->y_dmabuf_attachment,
+ frame->y_sgt, dma_dir);
+
+ return err;
+}
+
+static void tegra_vde_deattach_frame_dmabufs(struct video_frame *frame,
+ enum dma_data_direction dma_dir,
+ bool baseline_profile)
+{
+ if (!baseline_profile)
+ tegra_vde_detach_and_put_dmabuf(frame->aux_dmabuf_attachment,
+ frame->aux_sgt, dma_dir);
+
+ tegra_vde_detach_and_put_dmabuf(frame->cr_dmabuf_attachment,
+ frame->cr_sgt, dma_dir);
+
+ tegra_vde_detach_and_put_dmabuf(frame->cb_dmabuf_attachment,
+ frame->cb_sgt, dma_dir);
+
+ tegra_vde_detach_and_put_dmabuf(frame->y_dmabuf_attachment,
+ frame->y_sgt, dma_dir);
+}
+
+static int tegra_vde_copy_and_validate_frame(struct device *dev,
+ struct tegra_vde_h264_frame *frame,
+ unsigned long vaddr)
+{
+ if (copy_from_user(frame, (void __user *)vaddr, sizeof(*frame)))
+ return -EFAULT;
+
+ if (frame->frame_num > 0x7FFFFF) {
+ dev_err(dev, "Bad frame_num %u\n", frame->frame_num);
+ return -EINVAL;
+ }
+
+ if (frame->y_offset & 0xFF) {
+ dev_err(dev, "Bad y_offset 0x%X\n", frame->y_offset);
+ return -EINVAL;
+ }
+
+ if (frame->cb_offset & 0xFF) {
+ dev_err(dev, "Bad cb_offset 0x%X\n", frame->cb_offset);
+ return -EINVAL;
+ }
+
+ if (frame->cr_offset & 0xFF) {
+ dev_err(dev, "Bad cr_offset 0x%X\n", frame->cr_offset);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int tegra_vde_validate_h264_ctx(struct device *dev,
+ struct tegra_vde_h264_decoder_ctx *ctx)
+{
+ if (ctx->dpb_frames_nb == 0 || ctx->dpb_frames_nb > 17) {
+ dev_err(dev, "Bad DPB size %u\n", ctx->dpb_frames_nb);
+ return -EINVAL;
+ }
+
+ if (ctx->level_idc > 15) {
+ dev_err(dev, "Bad level value %u\n", ctx->level_idc);
+ return -EINVAL;
+ }
+
+ if (ctx->pic_init_qp > 52) {
+ dev_err(dev, "Bad pic_init_qp value %u\n", ctx->pic_init_qp);
+ return -EINVAL;
+ }
+
+ if (ctx->log2_max_pic_order_cnt_lsb > 16) {
+ dev_err(dev, "Bad log2_max_pic_order_cnt_lsb value %u\n",
+ ctx->log2_max_pic_order_cnt_lsb);
+ return -EINVAL;
+ }
+
+ if (ctx->log2_max_frame_num > 16) {
+ dev_err(dev, "Bad log2_max_frame_num value %u\n",
+ ctx->log2_max_frame_num);
+ return -EINVAL;
+ }
+
+ if (ctx->chroma_qp_index_offset > 31) {
+ dev_err(dev, "Bad chroma_qp_index_offset value %u\n",
+ ctx->chroma_qp_index_offset);
+ return -EINVAL;
+ }
+
+ if (ctx->pic_order_cnt_type > 2) {
+ dev_err(dev, "Bad pic_order_cnt_type value %u\n",
+ ctx->pic_order_cnt_type);
+ return -EINVAL;
+ }
+
+ if (ctx->num_ref_idx_l0_active_minus1 > 15) {
+ dev_err(dev, "Bad num_ref_idx_l0_active_minus1 value %u\n",
+ ctx->num_ref_idx_l0_active_minus1);
+ return -EINVAL;
+ }
+
+ if (ctx->num_ref_idx_l1_active_minus1 > 15) {
+ dev_err(dev, "Bad num_ref_idx_l1_active_minus1 value %u\n",
+ ctx->num_ref_idx_l1_active_minus1);
+ return -EINVAL;
+ }
+
+ if (!ctx->pic_width_in_mbs || ctx->pic_width_in_mbs > 127) {
+ dev_err(dev, "Bad pic_width_in_mbs value %u\n",
+ ctx->pic_width_in_mbs);
+ return -EINVAL;
+ }
+
+ if (!ctx->pic_height_in_mbs || ctx->pic_height_in_mbs > 127) {
+ dev_err(dev, "Bad pic_height_in_mbs value %u\n",
+ ctx->pic_height_in_mbs);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
+ unsigned long vaddr)
+{
+ struct device *dev = vde->miscdev.parent;
+ struct tegra_vde_h264_decoder_ctx ctx;
+ struct tegra_vde_h264_frame frame;
+ struct video_frame *dpb_frames;
+ struct dma_buf_attachment *bitstream_data_dmabuf_attachment;
+ struct sg_table *bitstream_sgt;
+ enum dma_data_direction dma_dir;
+ phys_addr_t bitstream_data_paddr;
+ phys_addr_t bsev_paddr;
+ size_t bitstream_data_size;
+ unsigned int macroblocks_nb;
+ unsigned int read_bytes;
+ unsigned int i;
+ bool timeout;
+ int ret;
+
+ if (copy_from_user(&ctx, (void __user *)vaddr, sizeof(ctx)))
+ return -EFAULT;
+
+ ret = tegra_vde_validate_h264_ctx(dev, &ctx);
+ if (ret)
+ return -EINVAL;
+
+ ret = tegra_vde_attach_dmabuf(dev, ctx.bitstream_data_fd,
+ ctx.bitstream_data_offset, 0,
+ &bitstream_data_dmabuf_attachment,
+ &bitstream_data_paddr,
+ &bitstream_sgt,
+ &bitstream_data_size,
+ DMA_TO_DEVICE);
+ if (ret)
+ return ret;
+
+ dpb_frames = kcalloc(ctx.dpb_frames_nb, sizeof(*dpb_frames),
+ GFP_KERNEL);
+ if (!dpb_frames) {
+ ret = -ENOMEM;
+ goto err_release_bitstream_dmabuf;
+ }
+
+ macroblocks_nb = ctx.pic_width_in_mbs * ctx.pic_height_in_mbs;
+ vaddr = ctx.dpb_frames_ptr;
+
+ for (i = 0; i < ctx.dpb_frames_nb; i++) {
+ ret = tegra_vde_copy_and_validate_frame(dev, &frame, vaddr);
+ if (ret)
+ goto err_release_dpb_frames;
+
+ dpb_frames[i].flags = frame.flags;
+ dpb_frames[i].frame_num = frame.frame_num;
+
+ dma_dir = (i == 0) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+
+ ret = tegra_vde_attach_dmabufs_to_frame(dev, &dpb_frames[i],
+ &frame, dma_dir,
+ ctx.baseline_profile,
+ macroblocks_nb * 64);
+ if (ret)
+ goto err_release_dpb_frames;
+
+ vaddr += sizeof(frame);
+ }
+
+ ret = mutex_lock_interruptible(&vde->lock);
+ if (ret)
+ goto err_release_dpb_frames;
+
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0)
+ goto err_unlock;
+
+ /*
+ * We rely on the VDE registers reset value, otherwise VDE
+ * causes bus lockup.
+ */
+ ret = reset_control_reset(vde->rst);
+ if (ret) {
+ dev_err(dev, "Failed to reset HW: %d\n", ret);
+ goto err_put_runtime_pm;
+ }
+
+ ret = tegra_vde_setup_hw_context(vde, &ctx, dpb_frames,
+ bitstream_data_paddr,
+ bitstream_data_size,
+ macroblocks_nb);
+ if (ret)
+ goto err_put_runtime_pm;
+
+ tegra_vde_decode_frame(vde, macroblocks_nb);
+
+ timeout = !wait_for_completion_killable_timeout(&vde->decode_completion,
+ TEGRA_VDE_TIMEOUT);
+ if (timeout) {
+ bsev_paddr = readl_relaxed(vde->regs + BSEV(0x10));
+ macroblocks_nb = readl_relaxed(vde->regs + SXE(0xC8)) & 0x1FFF;
+ read_bytes = bsev_paddr ? bsev_paddr - bitstream_data_paddr : 0;
+
+ dev_err(dev, "Decoding failed, "
+ "read 0x%X bytes : %u macroblocks parsed\n",
+ read_bytes, macroblocks_nb);
+
+ ret = -EIO;
+ }
+
+err_put_runtime_pm:
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
+err_unlock:
+ mutex_unlock(&vde->lock);
+
+err_release_dpb_frames:
+ while (i--) {
+ dma_dir = (i == 0) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+
+ tegra_vde_deattach_frame_dmabufs(&dpb_frames[i], dma_dir,
+ ctx.baseline_profile);
+ }
+
+ kfree(dpb_frames);
+
+err_release_bitstream_dmabuf:
+ tegra_vde_detach_and_put_dmabuf(bitstream_data_dmabuf_attachment,
+ bitstream_sgt, DMA_TO_DEVICE);
+
+ return ret;
+}
+
+static long tegra_vde_unlocked_ioctl(struct file *filp,
+ unsigned int cmd, unsigned long arg)
+{
+ struct miscdevice *miscdev = filp->private_data;
+ struct tegra_vde *vde = container_of(miscdev, struct tegra_vde,
+ miscdev);
+
+ switch (cmd) {
+ case TEGRA_VDE_IOCTL_DECODE_H264:
+ return tegra_vde_ioctl_decode_h264(vde, arg);
+ }
+
+ dev_err(miscdev->parent, "Invalid IOCTL command %u\n", cmd);
+
+ return -ENOTTY;
+}
+
+static const struct file_operations tegra_vde_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = tegra_vde_unlocked_ioctl,
+};
+
+static irqreturn_t tegra_vde_isr(int irq, void *data)
+{
+ struct tegra_vde *vde = data;
+
+ tegra_vde_set_bits(vde->regs, 0, FRAMEID(0x208));
+ complete(&vde->decode_completion);
+
+ return IRQ_HANDLED;
+}
+
+static int tegra_vde_runtime_suspend(struct device *dev)
+{
+ struct tegra_vde *vde = dev_get_drvdata(dev);
+ int err;
+
+ err = tegra_powergate_power_off(TEGRA_POWERGATE_VDEC);
+ if (err) {
+ dev_err(dev, "Failed to power down HW: %d\n", err);
+ return err;
+ }
+
+ clk_disable_unprepare(vde->clk);
+
+ return 0;
+}
+
+static int tegra_vde_runtime_resume(struct device *dev)
+{
+ struct tegra_vde *vde = dev_get_drvdata(dev);
+ int err;
+
+ err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VDEC,
+ vde->clk, vde->rst);
+ if (err) {
+ dev_err(dev, "Failed to power up HW : %d\n", err);
+ return err;
+ }
+
+ return 0;
+}
+
+static int tegra_vde_probe(struct platform_device *pdev)
+{
+ struct resource *res_regs, *res_iram;
+ struct device *dev = &pdev->dev;
+ struct tegra_vde *vde;
+ int irq, err;
+
+ vde = devm_kzalloc(&pdev->dev, sizeof(*vde), GFP_KERNEL);
+ if (!vde)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, vde);
+
+ res_regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
+ if (!res_regs)
+ return -ENODEV;
+
+ res_iram = platform_get_resource_byname(pdev, IORESOURCE_MEM, "iram");
+ if (!res_iram)
+ return -ENODEV;
+
+ irq = platform_get_irq_byname(pdev, "sync-token");
+ if (irq < 0)
+ return irq;
+
+ vde->regs = devm_ioremap_resource(dev, res_regs);
+ if (IS_ERR(vde->regs))
+ return PTR_ERR(vde->regs);
+
+ vde->iram = devm_ioremap_resource(dev, res_iram);
+ if (IS_ERR(vde->iram))
+ return PTR_ERR(vde->iram);
+
+ vde->clk = devm_clk_get(dev, "vde");
+ if (IS_ERR(vde->clk)) {
+ err = PTR_ERR(vde->clk);
+ dev_err(dev, "Could not get VDE clk %d\n", err);
+ return err;
+ }
+
+ vde->rst = devm_reset_control_get(dev, "vde");
+ if (IS_ERR(vde->rst)) {
+ err = PTR_ERR(vde->rst);
+ dev_err(dev, "Could not get VDE reset %d\n", err);
+ return err;
+ }
+
+ err = devm_request_irq(dev, irq, tegra_vde_isr, 0,
+ dev_name(dev), vde);
+ if (err) {
+ dev_err(&pdev->dev, "Failed to request IRQ %d\n", err);
+ return err;
+ }
+
+ mutex_init(&vde->lock);
+ init_completion(&vde->decode_completion);
+
+ vde->iram_lists_paddr = res_iram->start;
+ vde->miscdev.minor = MISC_DYNAMIC_MINOR;
+ vde->miscdev.name = "tegra_vde";
+ vde->miscdev.fops = &tegra_vde_fops;
+ vde->miscdev.parent = dev;
+
+ err = misc_register(&vde->miscdev);
+ if (err) {
+ dev_err(dev, "Failed to register misc device: %d\n", err);
+ return err;
+ }
+
+ pm_runtime_enable(dev);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_set_autosuspend_delay(dev, 300);
+
+ if (!pm_runtime_enabled(dev)) {
+ err = tegra_vde_runtime_resume(dev);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static int tegra_vde_remove(struct platform_device *pdev)
+{
+ struct tegra_vde *vde = platform_get_drvdata(pdev);
+ struct device *dev = &pdev->dev;
+ int err;
+
+ if (!pm_runtime_enabled(dev)) {
+ err = tegra_vde_runtime_suspend(dev);
+ if (err)
+ return err;
+ }
+
+ pm_runtime_dont_use_autosuspend(dev);
+ pm_runtime_disable(dev);
+
+ misc_deregister(&vde->miscdev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra_vde_pm_suspend(struct device *dev)
+{
+ struct tegra_vde *vde = dev_get_drvdata(dev);
+ int err;
+
+ mutex_lock(&vde->lock);
+
+ err = pm_runtime_force_suspend(dev);
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static int tegra_vde_pm_resume(struct device *dev)
+{
+ struct tegra_vde *vde = dev_get_drvdata(dev);
+ int err;
+
+ err = pm_runtime_force_resume(dev);
+ if (err < 0)
+ return err;
+
+ mutex_unlock(&vde->lock);
+
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops tegra_vde_pm_ops = {
+ SET_RUNTIME_PM_OPS(tegra_vde_runtime_suspend,
+ tegra_vde_runtime_resume,
+ NULL)
+ SET_SYSTEM_SLEEP_PM_OPS(tegra_vde_pm_suspend,
+ tegra_vde_pm_resume)
+};
+
+static const struct of_device_id tegra_vde_of_match[] = {
+ { .compatible = "nvidia,tegra20-vde", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, tegra_vde_of_match);
+
+static struct platform_driver tegra_vde_driver = {
+ .probe = tegra_vde_probe,
+ .remove = tegra_vde_remove,
+ .driver = {
+ .name = "tegra-vde",
+ .of_match_table = tegra_vde_of_match,
+ .pm = &tegra_vde_pm_ops,
+ },
+};
+module_platform_driver(tegra_vde_driver);
+
+MODULE_DESCRIPTION("NVIDIA Tegra20 Video Decoder driver");
+MODULE_AUTHOR("Dmitry Osipenko");
+MODULE_LICENSE("GPL");
--
2.14.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
@ 2017-10-11 20:08 ` Dmitry Osipenko
0 siblings, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-11 20:08 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
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.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../bindings/arm/tegra/nvidia,tegra20-vde.txt | 44 +
drivers/staging/Kconfig | 2 +
drivers/staging/Makefile | 1 +
drivers/staging/tegra-vde/Kconfig | 6 +
drivers/staging/tegra-vde/Makefile | 1 +
drivers/staging/tegra-vde/TODO | 5 +
drivers/staging/tegra-vde/uapi.h | 101 ++
drivers/staging/tegra-vde/vde.c | 1109 ++++++++++++++++++++
8 files changed, 1269 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
create mode 100644 drivers/staging/tegra-vde/Kconfig
create mode 100644 drivers/staging/tegra-vde/Makefile
create mode 100644 drivers/staging/tegra-vde/TODO
create mode 100644 drivers/staging/tegra-vde/uapi.h
create mode 100644 drivers/staging/tegra-vde/vde.c
diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
new file mode 100644
index 000000000000..c3f847db8167
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
@@ -0,0 +1,44 @@
+NVIDIA Tegra Video Decoder Engine
+
+Required properties:
+- compatible : "nvidia,tegra20-vde"
+- reg : Must contain 2 register ranges: registers and IRAM region that
+ VDE uses for its internal needs and for passing some of decoding
+ parameters.
+- reg-names : Must include the following entries:
+ - regs
+ - iram
+- interrupts : Must contain an entry for each entry in interrupt-names.
+- interrupt-names : Must include the following entries:
+ - ucq-error
+ - sync-token
+ - bsev
+ - bsea
+ - sxe
+- clocks : Must contain an entry for each entry in clock-names.
+ See ../clocks/clock-bindings.txt for details.
+- clock-names : Must include the following entries:
+ - vde
+- resets : Must contain an entry for each entry in reset-names.
+ See ../reset/reset.txt for details.
+- reset-names : Must include the following entries:
+ - vde
+
+Example:
+
+vde@6001a000 {
+ compatible = "nvidia,tegra20-vde";
+ reg = <0x6001a000 0x3D00 /* VDE registers */
+ 0x40000400 0x3FC00>; /* IRAM region */
+ reg-names = "regs", "iram";
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, /* UCQ error interrupt */
+ <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */
+ <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */
+ <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, /* BSE-A interrupt */
+ <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
+ interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
+ clocks = <&tegra_car TEGRA20_CLK_VDE>;
+ clock-names = "vde";
+ resets = <&tegra_car 61>;
+ reset-names = "vde";
+};
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 554683912cff..10c982811093 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -118,4 +118,6 @@ source "drivers/staging/vboxvideo/Kconfig"
source "drivers/staging/pi433/Kconfig"
+source "drivers/staging/tegra-vde/Kconfig"
+
endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 8951c37d8d80..c5ef39767f22 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -49,3 +49,4 @@ obj-$(CONFIG_BCM2835_VCHIQ) += vc04_services/
obj-$(CONFIG_CRYPTO_DEV_CCREE) += ccree/
obj-$(CONFIG_DRM_VBOXVIDEO) += vboxvideo/
obj-$(CONFIG_PI433) += pi433/
+obj-$(CONFIG_TEGRA_VDE) += tegra-vde/
diff --git a/drivers/staging/tegra-vde/Kconfig b/drivers/staging/tegra-vde/Kconfig
new file mode 100644
index 000000000000..730ee006de66
--- /dev/null
+++ b/drivers/staging/tegra-vde/Kconfig
@@ -0,0 +1,6 @@
+config TEGRA_VDE
+ tristate "NVIDIA Tegra Video Decoder Engine driver"
+ depends on ARCH_TEGRA_2x_SOC || COMPILE_TEST
+ help
+ Say Y here to enable support for the NVIDIA Tegra video decoder
+ driver.
diff --git a/drivers/staging/tegra-vde/Makefile b/drivers/staging/tegra-vde/Makefile
new file mode 100644
index 000000000000..e7c0df1174bf
--- /dev/null
+++ b/drivers/staging/tegra-vde/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_TEGRA_VDE) += vde.o
diff --git a/drivers/staging/tegra-vde/TODO b/drivers/staging/tegra-vde/TODO
new file mode 100644
index 000000000000..e98bbc7b3c19
--- /dev/null
+++ b/drivers/staging/tegra-vde/TODO
@@ -0,0 +1,5 @@
+TODO:
+ - Figure out how generic V4L2 API could be utilized by this driver,
+ implement it.
+
+Contact: Dmitry Osipenko <digetx@gmail.com>
diff --git a/drivers/staging/tegra-vde/uapi.h b/drivers/staging/tegra-vde/uapi.h
new file mode 100644
index 000000000000..36d76278d27c
--- /dev/null
+++ b/drivers/staging/tegra-vde/uapi.h
@@ -0,0 +1,101 @@
+/*
+ * Copyright (C) 2016-2017 Dmitry Osipenko <digetx@gmail.com>
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef _UAPI_TEGRA_VDE_H_
+#define _UAPI_TEGRA_VDE_H_
+
+#include <linux/types.h>
+#include <asm/ioctl.h>
+
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+#define FLAG_B_FRAME (1 << 0)
+#define FLAG_REFERENCE (1 << 1)
+
+struct tegra_vde_h264_frame {
+ __s32 y_fd;
+ __s32 cb_fd;
+ __s32 cr_fd;
+ __s32 aux_fd;
+ __u32 y_offset;
+ __u32 cb_offset;
+ __u32 cr_offset;
+ __u32 aux_offset;
+ __u32 frame_num;
+ __u32 flags;
+
+ __u32 reserved;
+} __attribute__((packed));
+
+struct tegra_vde_h264_decoder_ctx {
+ __s32 bitstream_data_fd;
+ __u32 bitstream_data_offset;
+
+ __u32 dpb_frames_ptr;
+ __u8 dpb_frames_nb;
+ __u8 dpb_ref_frames_with_earlier_poc_nb;
+
+ // SPS
+ __u8 baseline_profile;
+ __u8 level_idc;
+ __u8 log2_max_pic_order_cnt_lsb;
+ __u8 log2_max_frame_num;
+ __u8 pic_order_cnt_type;
+ __u8 direct_8x8_inference_flag;
+ __u8 pic_width_in_mbs;
+ __u8 pic_height_in_mbs;
+
+ // PPS
+ __u8 pic_init_qp;
+ __u8 deblocking_filter_control_present_flag;
+ __u8 constrained_intra_pred_flag;
+ __u8 chroma_qp_index_offset;
+ __u8 pic_order_present_flag;
+
+ // Slice header
+ __u8 num_ref_idx_l0_active_minus1;
+ __u8 num_ref_idx_l1_active_minus1;
+
+ __u32 reserved;
+} __attribute__((packed));
+
+#define VDE_IOCTL_BASE ('v' + 0x20)
+
+#define VDE_IO(nr) _IO(VDE_IOCTL_BASE, nr)
+#define VDE_IOR(nr, type) _IOR(VDE_IOCTL_BASE, nr, type)
+#define VDE_IOW(nr, type) _IOW(VDE_IOCTL_BASE, nr, type)
+#define VDE_IOWR(nr, type) _IOWR(VDE_IOCTL_BASE, nr, type)
+
+#define TEGRA_VDE_DECODE_H264 0x00
+
+#define TEGRA_VDE_IOCTL_DECODE_H264 \
+ VDE_IOW(TEGRA_VDE_DECODE_H264, struct tegra_vde_h264_decoder_ctx)
+
+#if defined(__cplusplus)
+}
+#endif
+
+#endif // _UAPI_TEGRA_VDE_H_
diff --git a/drivers/staging/tegra-vde/vde.c b/drivers/staging/tegra-vde/vde.c
new file mode 100644
index 000000000000..3ef0d5f2295b
--- /dev/null
+++ b/drivers/staging/tegra-vde/vde.c
@@ -0,0 +1,1109 @@
+/*
+ * NVIDIA Tegra20 Video decoder driver
+ *
+ * Copyright (C) 2016-2017 Dmitry Osipenko <digetx@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-buf.h>
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+
+#include <soc/tegra/pmc.h>
+
+#include "uapi.h"
+
+#define SXE(offt) (0x0000 + (offt)) /* Syntax Engine */
+#define BSEV(offt) (0x1000 + (offt)) /* Video Bitstream Engine */
+#define MBE(offt) (0x2000 + (offt)) /* Macroblock Engine */
+#define PPE(offt) (0x2200 + (offt)) /* Post-processing Engine */
+#define MCE(offt) (0x2400 + (offt)) /* Motion Compensation Eng. */
+#define TFE(offt) (0x2600 + (offt)) /* Transform Engine */
+#define VDMA(offt) (0x2A00 + (offt)) /* Video DMA */
+#define FRAMEID(offt) (0x3800 + (offt))
+
+#define ICMDQUE_WR 0x00
+#define CMDQUE_CONTROL 0x08
+#define INTR_STATUS 0x18
+#define BSE_INT_ENB 0x40
+#define BSE_CONFIG 0x44
+
+#define BSE_ICMDQUE_EMPTY BIT(3)
+#define BSE_DMA_BUSY BIT(23)
+
+#define TEGRA_VDE_TIMEOUT msecs_to_jiffies(1000)
+
+#define VDE_WR(__data, __addr) \
+do { \
+ pr_debug("%s: %d: 0x%08X => " #__addr ")\n", \
+ __func__, __LINE__, (u32)(__data)); \
+ writel_relaxed(__data, __addr); \
+} while (0)
+
+struct video_frame {
+ struct dma_buf_attachment *y_dmabuf_attachment;
+ struct dma_buf_attachment *cb_dmabuf_attachment;
+ struct dma_buf_attachment *cr_dmabuf_attachment;
+ struct dma_buf_attachment *aux_dmabuf_attachment;
+ struct sg_table *y_sgt;
+ struct sg_table *cb_sgt;
+ struct sg_table *cr_sgt;
+ struct sg_table *aux_sgt;
+ dma_addr_t y_paddr;
+ dma_addr_t cb_paddr;
+ dma_addr_t cr_paddr;
+ dma_addr_t aux_paddr;
+ u32 frame_num;
+ u32 flags;
+};
+
+struct tegra_vde {
+ phys_addr_t iram_lists_paddr;
+ void __iomem *regs;
+ void __iomem *iram;
+ struct mutex lock;
+ struct miscdevice miscdev;
+ struct reset_control *rst;
+ struct completion decode_completion;
+ struct clk *clk;
+};
+
+static void tegra_vde_set_bits(void __iomem *regs, u32 mask, u32 offset)
+{
+ u32 value = readl_relaxed(regs + offset);
+
+ VDE_WR(value | mask, regs + offset);
+}
+
+static int tegra_vde_wait_mbe(void __iomem *regs)
+{
+ u32 tmp;
+
+ return readl_relaxed_poll_timeout(regs + MBE(0x8C), tmp,
+ (tmp >= 0x10), 1, 100);
+}
+
+static int tegra_vde_setup_mbe_frame_idx(void __iomem *regs,
+ unsigned int refs_nb,
+ bool setup_refs)
+{
+ u32 frame_idx_enb_mask = 0;
+ u32 value;
+ unsigned int frame_idx;
+ unsigned int idx;
+ int err;
+
+ VDE_WR(0xD0000000 | (0 << 23), regs + MBE(0x80));
+ VDE_WR(0xD0200000 | (0 << 23), regs + MBE(0x80));
+
+ err = tegra_vde_wait_mbe(regs);
+ if (err)
+ return err;
+
+ if (!setup_refs)
+ return 0;
+
+ for (idx = 0, frame_idx = 1; idx < refs_nb; idx++, frame_idx++) {
+ VDE_WR(0xD0000000 | (frame_idx << 23), regs + MBE(0x80));
+ VDE_WR(0xD0200000 | (frame_idx << 23), regs + MBE(0x80));
+
+ frame_idx_enb_mask |= frame_idx << (6 * (idx % 4));
+
+ if (idx % 4 == 3 || idx == refs_nb - 1) {
+ value = 0xC0000000;
+ value |= (idx >> 2) << 24;
+ value |= frame_idx_enb_mask;
+
+ VDE_WR(value, regs + MBE(0x80));
+
+ err = tegra_vde_wait_mbe(regs);
+ if (err)
+ return err;
+
+ frame_idx_enb_mask = 0;
+ }
+ }
+
+ return 0;
+}
+
+static void tegra_vde_mbe_set_0xa_reg(void __iomem *regs, int reg, u32 val)
+{
+ VDE_WR(0xA0000000 | (reg << 24) | (val & 0xFFFF), regs + MBE(0x80));
+ VDE_WR(0xA0000000 | ((reg + 1) << 24) | (val >> 16), regs + MBE(0x80));
+}
+
+static int tegra_vde_wait_bsev(struct tegra_vde *vde, bool wait_dma)
+{
+ struct device *dev = vde->miscdev.parent;
+ u32 value;
+ int err;
+
+ err = readl_relaxed_poll_timeout(vde->regs + BSEV(INTR_STATUS), value,
+ !(value & BIT(2)), 1, 100);
+ if (err) {
+ dev_err(dev, "BSEV unknown bit timeout\n");
+ return err;
+ }
+
+ err = readl_relaxed_poll_timeout(vde->regs + BSEV(INTR_STATUS), value,
+ (value & BSE_ICMDQUE_EMPTY), 1, 100);
+ if (err) {
+ dev_err(dev, "BSEV ICMDQUE flush timeout\n");
+ return err;
+ }
+
+ if (!wait_dma)
+ return 0;
+
+ err = readl_relaxed_poll_timeout(vde->regs + BSEV(INTR_STATUS), value,
+ !(value & BSE_DMA_BUSY), 1, 100);
+ if (err) {
+ dev_err(dev, "BSEV DMA timeout\n");
+ return err;
+ }
+
+ return 0;
+}
+
+static int tegra_vde_push_to_bsev_icmdqueue(struct tegra_vde *vde,
+ u32 value, bool wait_dma)
+{
+ VDE_WR(value, vde->regs + BSEV(ICMDQUE_WR));
+
+ return tegra_vde_wait_bsev(vde, wait_dma);
+}
+
+static void tegra_vde_setup_frameid(void __iomem *regs,
+ struct video_frame *frame,
+ unsigned int frameid,
+ u32 mbs_width, u32 mbs_height)
+{
+ u32 y_paddr = frame ? frame->y_paddr : 0xFCDEAD00;
+ u32 cb_paddr = frame ? frame->cb_paddr : 0xFCDEAD00;
+ u32 cr_paddr = frame ? frame->cr_paddr : 0xFCDEAD00;
+ u32 value1 = frame ? ((mbs_width << 16) | mbs_height) : 0;
+ u32 value2 = frame ? ((((mbs_width + 1) >> 1) << 6) | 1) : 0;
+
+ VDE_WR( y_paddr >> 8, regs + FRAMEID(0x000 + frameid * 4));
+ VDE_WR(cb_paddr >> 8, regs + FRAMEID(0x100 + frameid * 4));
+ VDE_WR(cr_paddr >> 8, regs + FRAMEID(0x180 + frameid * 4));
+ VDE_WR(value1, regs + FRAMEID(0x080 + frameid * 4));
+ VDE_WR(value2, regs + FRAMEID(0x280 + frameid * 4));
+}
+
+static void tegra_setup_frameidx(void __iomem *regs,
+ struct video_frame *frames,
+ unsigned int frames_nb,
+ u32 mbs_width, u32 mbs_height)
+{
+ unsigned int idx;
+
+ for (idx = 0; idx < frames_nb; idx++)
+ tegra_vde_setup_frameid(regs, &frames[idx], idx,
+ mbs_width, mbs_height);
+ for (; idx < 17; idx++)
+ tegra_vde_setup_frameid(regs, NULL, idx, 0, 0);
+}
+
+static void tegra_vde_setup_iram_entry(void __iomem *iram_tables,
+ unsigned int table,
+ unsigned int row,
+ u32 value1, u32 value2)
+{
+ VDE_WR(value1, iram_tables + 0x80 * table + row * 8);
+ VDE_WR(value2, iram_tables + 0x80 * table + row * 8 + 4);
+}
+
+static void tegra_vde_setup_iram_tables(void __iomem *iram_tables,
+ struct video_frame *dpb_frames,
+ unsigned int ref_frames_nb,
+ unsigned int with_earlier_poc_nb)
+{
+ struct video_frame *frame;
+ u32 value, aux_paddr;
+ int with_later_poc_nb;
+ unsigned int i, k;
+
+ pr_debug("DPB: Frame 0: frame_num = %d\n", dpb_frames[0].frame_num);
+
+ pr_debug("REF L0:\n");
+
+ for (i = 0; i < 16; i++) {
+ if (i < ref_frames_nb) {
+ frame = &dpb_frames[i + 1];
+
+ aux_paddr = frame->aux_paddr;
+
+ value = (i + 1) << 26;
+ value |= !(frame->flags & FLAG_B_FRAME) << 25;
+ value |= 1 << 24;
+ value |= frame->frame_num;
+
+ pr_debug("\tFrame %d: frame_num = %d B_frame = %d\n",
+ i + 1, frame->frame_num,
+ (frame->flags & FLAG_B_FRAME));
+ } else {
+ aux_paddr = 0xFADEAD00;
+ value = 0;
+ }
+
+ tegra_vde_setup_iram_entry(iram_tables, 0, i, value, aux_paddr);
+ tegra_vde_setup_iram_entry(iram_tables, 1, i, value, aux_paddr);
+ tegra_vde_setup_iram_entry(iram_tables, 2, i, value, aux_paddr);
+ tegra_vde_setup_iram_entry(iram_tables, 3, i, value, aux_paddr);
+ }
+
+ if (!(dpb_frames[0].flags & FLAG_B_FRAME))
+ return;
+
+ if (with_earlier_poc_nb >= ref_frames_nb)
+ return;
+
+ with_later_poc_nb = ref_frames_nb - with_earlier_poc_nb;
+
+ pr_debug("REF L1: with_later_poc_nb %d with_earlier_poc_nb %d\n",
+ with_later_poc_nb, with_earlier_poc_nb);
+
+ for (i = 0, k = with_earlier_poc_nb; i < with_later_poc_nb; i++, k++) {
+ frame = &dpb_frames[k + 1];
+
+ aux_paddr = frame->aux_paddr;
+
+ value = (k + 1) << 26;
+ value |= !(frame->flags & FLAG_B_FRAME) << 25;
+ value |= 1 << 24;
+ value |= frame->frame_num;
+
+ pr_debug("\tFrame %d: frame_num = %d\n",
+ k + 1, frame->frame_num);
+
+ tegra_vde_setup_iram_entry(iram_tables, 2, i, value, aux_paddr);
+ }
+
+ for (k = 0; i < ref_frames_nb; i++, k++) {
+ frame = &dpb_frames[k + 1];
+
+ aux_paddr = frame->aux_paddr;
+
+ value = (k + 1) << 26;
+ value |= !(frame->flags & FLAG_B_FRAME) << 25;
+ value |= 1 << 24;
+ value |= frame->frame_num;
+
+ pr_debug("\tFrame %d: frame_num = %d\n",
+ k + 1, frame->frame_num);
+
+ tegra_vde_setup_iram_entry(iram_tables, 2, i, value, aux_paddr);
+ }
+}
+
+static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
+ struct tegra_vde_h264_decoder_ctx *ctx,
+ struct video_frame *dpb_frames,
+ phys_addr_t bitstream_data_paddr,
+ size_t bitstream_data_size,
+ unsigned int macroblocks_nb)
+{
+ struct device *dev = vde->miscdev.parent;
+ u32 value;
+ int err;
+
+ tegra_vde_set_bits(vde->regs, 0xA, SXE(0xF0));
+ tegra_vde_set_bits(vde->regs, 0xB, BSEV(CMDQUE_CONTROL));
+ tegra_vde_set_bits(vde->regs, 0x8002, MBE(0x50));
+ tegra_vde_set_bits(vde->regs, 0xA, MBE(0xA0));
+ tegra_vde_set_bits(vde->regs, 0xA, PPE(0x14));
+ tegra_vde_set_bits(vde->regs, 0xA, PPE(0x28));
+ tegra_vde_set_bits(vde->regs, 0xA00, MCE(0x08));
+ tegra_vde_set_bits(vde->regs, 0xA, TFE(0x00));
+ tegra_vde_set_bits(vde->regs, 0x5, VDMA(0x04));
+
+ VDE_WR(0x00000000, vde->regs + VDMA(0x1C));
+ VDE_WR(0x00000000, vde->regs + VDMA(0x00));
+ VDE_WR(0x00000007, vde->regs + VDMA(0x04));
+ VDE_WR(0x00000007, vde->regs + FRAMEID(0x200));
+ VDE_WR(0x00000005, vde->regs + TFE(0x04));
+ VDE_WR(0x00000000, vde->regs + MBE(0x84));
+ VDE_WR(0x00000010, vde->regs + SXE(0x08));
+ VDE_WR(0x00000150, vde->regs + SXE(0x54));
+ VDE_WR(0x0000054C, vde->regs + SXE(0x58));
+ VDE_WR(0x00000E34, vde->regs + SXE(0x5C));
+ VDE_WR(0x063C063C, vde->regs + MCE(0x10));
+ VDE_WR(0x0003FC00, vde->regs + BSEV(INTR_STATUS));
+ VDE_WR(0x0000150D, vde->regs + BSEV(BSE_CONFIG));
+ VDE_WR(0x00000100, vde->regs + BSEV(BSE_INT_ENB));
+ VDE_WR(0x00000000, vde->regs + BSEV(0x98));
+ VDE_WR(0x00000060, vde->regs + BSEV(0x9C));
+
+ memset_io(vde->iram + 512, 0, macroblocks_nb / 2);
+
+ tegra_setup_frameidx(vde->regs, dpb_frames, ctx->dpb_frames_nb,
+ ctx->pic_width_in_mbs, ctx->pic_height_in_mbs);
+
+ tegra_vde_setup_iram_tables(vde->iram, dpb_frames,
+ ctx->dpb_frames_nb - 1,
+ ctx->dpb_ref_frames_with_earlier_poc_nb);
+
+ VDE_WR(0x00000000, vde->regs + BSEV(0x8C));
+ VDE_WR(bitstream_data_paddr + bitstream_data_size,
+ vde->regs + BSEV(0x54));
+
+ value = ctx->pic_width_in_mbs << 11 | ctx->pic_height_in_mbs << 3;
+
+ VDE_WR(value, vde->regs + BSEV(0x88));
+
+ err = tegra_vde_wait_bsev(vde, false);
+ if (err)
+ return err;
+
+ err = tegra_vde_push_to_bsev_icmdqueue(vde, 0x800003FC, false);
+ if (err)
+ return err;
+
+ value = 0x01500000;
+ value |= ((vde->iram_lists_paddr + 512) >> 2) & 0xFFFF;
+
+ err = tegra_vde_push_to_bsev_icmdqueue(vde, value, true);
+ if (err)
+ return err;
+
+ err = tegra_vde_push_to_bsev_icmdqueue(vde, 0x840F054C, false);
+ if (err)
+ return err;
+
+ err = tegra_vde_push_to_bsev_icmdqueue(vde, 0x80000080, false);
+ if (err)
+ return err;
+
+ value = 0x0E340000 | ((vde->iram_lists_paddr >> 2) & 0xFFFF);
+
+ err = tegra_vde_push_to_bsev_icmdqueue(vde, value, true);
+ if (err)
+ return err;
+
+ value = 0x00800005;
+ value |= ctx->pic_width_in_mbs << 11;
+ value |= ctx->pic_height_in_mbs << 3;
+
+ VDE_WR(value, vde->regs + SXE(0x10));
+
+ value = !ctx->baseline_profile << 17;
+ value |= ctx->level_idc << 13;
+ value |= ctx->log2_max_pic_order_cnt_lsb << 7;
+ value |= ctx->pic_order_cnt_type << 5;
+ value |= ctx->log2_max_frame_num;
+
+ VDE_WR(value, vde->regs + SXE(0x40));
+
+ value = ctx->pic_init_qp << 25;
+ value |= !!(ctx->deblocking_filter_control_present_flag) << 2;
+ value |= !!ctx->pic_order_present_flag;
+
+ VDE_WR(value, vde->regs + SXE(0x44));
+
+ value = ctx->chroma_qp_index_offset;
+ value |= ctx->num_ref_idx_l0_active_minus1 << 5;
+ value |= ctx->num_ref_idx_l1_active_minus1 << 10;
+ value |= !!ctx->constrained_intra_pred_flag << 15;
+
+ VDE_WR(value, vde->regs + SXE(0x48));
+
+ value = 0x0C000000;
+ value |= !!(dpb_frames[0].flags & FLAG_B_FRAME) << 24;
+
+ VDE_WR(value, vde->regs + SXE(0x4C));
+
+ value = 0x03800000;
+ value |= min_t(size_t, bitstream_data_size, SZ_1M);
+
+ VDE_WR(value, vde->regs + SXE(0x68));
+
+ VDE_WR(bitstream_data_paddr, vde->regs + SXE(0x6C));
+
+ value = (1 << 28) | 5;
+ value |= ctx->pic_width_in_mbs << 11;
+ value |= ctx->pic_height_in_mbs << 3;
+
+ VDE_WR(value, vde->regs + MBE(0x80));
+
+ value = 0x26800000;
+ value |= ctx->level_idc << 4;
+ value |= !ctx->baseline_profile << 1;
+ value |= !!ctx->direct_8x8_inference_flag;
+
+ VDE_WR(value, vde->regs + MBE(0x80));
+
+ VDE_WR(0xF4000001, vde->regs + MBE(0x80));
+ VDE_WR(0x20000000, vde->regs + MBE(0x80));
+ VDE_WR(0xF4000101, vde->regs + MBE(0x80));
+
+ value = 0x20000000;
+ value |= ctx->chroma_qp_index_offset << 8;
+
+ VDE_WR(value, vde->regs + MBE(0x80));
+
+ err = tegra_vde_setup_mbe_frame_idx(vde->regs,
+ ctx->dpb_frames_nb - 1,
+ ctx->pic_order_cnt_type == 0);
+ if (err) {
+ dev_err(dev, "MBE frames setup failed\n");
+ return err;
+ }
+
+ tegra_vde_mbe_set_0xa_reg(vde->regs, 0, 0x000009FC);
+ tegra_vde_mbe_set_0xa_reg(vde->regs, 2, 0xF1DEAD00);
+ tegra_vde_mbe_set_0xa_reg(vde->regs, 4, 0xF2DEAD00);
+ tegra_vde_mbe_set_0xa_reg(vde->regs, 6, 0xF3DEAD00);
+ tegra_vde_mbe_set_0xa_reg(vde->regs, 8, dpb_frames[0].aux_paddr);
+
+ value = 0xFC000000;
+ value |= !!(dpb_frames[0].flags & FLAG_B_FRAME) << 2;
+
+ if (!ctx->baseline_profile)
+ value |= !!(dpb_frames[0].flags & FLAG_REFERENCE) << 1;
+
+ VDE_WR(value, vde->regs + MBE(0x80));
+
+ err = tegra_vde_wait_mbe(vde->regs);
+ if (err) {
+ dev_err(dev, "MBE programming failed\n");
+ return err;
+ }
+
+ return 0;
+}
+
+static void tegra_vde_decode_frame(struct tegra_vde *vde, int macroblocks_nb)
+{
+ reinit_completion(&vde->decode_completion);
+
+ VDE_WR(0x00000001, vde->regs + BSEV(0x8C));
+ VDE_WR(0x20000000 | (macroblocks_nb - 1), vde->regs + SXE(0x00));
+}
+
+static void tegra_vde_detach_and_put_dmabuf(struct dma_buf_attachment *a,
+ struct sg_table *sgt,
+ enum dma_data_direction dma_dir)
+{
+ struct dma_buf *dmabuf = a->dmabuf;
+
+ dma_buf_unmap_attachment(a, sgt, dma_dir);
+ dma_buf_detach(dmabuf, a);
+ dma_buf_put(dmabuf);
+}
+
+static int tegra_vde_attach_dmabuf(struct device *dev,
+ int fd,
+ unsigned long offset,
+ unsigned int min_size,
+ struct dma_buf_attachment **a,
+ phys_addr_t *paddr,
+ struct sg_table **s,
+ size_t *size,
+ enum dma_data_direction dma_dir)
+{
+ struct dma_buf_attachment *attachment;
+ struct dma_buf *dmabuf;
+ struct sg_table *sgt;
+ int err;
+
+ dmabuf = dma_buf_get(fd);
+ if (IS_ERR(dmabuf)) {
+ dev_err(dev, "Invalid dmabuf FD\n");
+ return PTR_ERR(dmabuf);
+ }
+
+ if ((u64)offset + min_size > dmabuf->size) {
+ dev_err(dev, "Too small dmabuf size %zu @0x%lX, "
+ "should be at least %d\n",
+ dmabuf->size, offset, min_size);
+ return -EINVAL;
+ }
+
+ attachment = dma_buf_attach(dmabuf, dev);
+ if (IS_ERR(attachment)) {
+ dev_err(dev, "Failed to attach dmabuf\n");
+ err = PTR_ERR(attachment);
+ goto err_put;
+ }
+
+ sgt = dma_buf_map_attachment(attachment, dma_dir);
+ if (IS_ERR(sgt)) {
+ dev_err(dev, "Failed to get dmabufs sg_table\n");
+ err = PTR_ERR(sgt);
+ goto err_detach;
+ }
+
+ if (sgt->nents != 1) {
+ dev_err(dev, "Sparse DMA region is unsupported\n");
+ err = -EINVAL;
+ goto err_unmap;
+ }
+
+ *paddr = sg_dma_address(sgt->sgl) + offset;
+ *a = attachment;
+ *s = sgt;
+
+ if (size)
+ *size = dmabuf->size - offset;
+
+ return 0;
+
+err_unmap:
+ dma_buf_unmap_attachment(attachment, sgt, dma_dir);
+err_detach:
+ dma_buf_detach(dmabuf, attachment);
+err_put:
+ dma_buf_put(dmabuf);
+
+ return err;
+}
+
+static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
+ struct video_frame *frame,
+ struct tegra_vde_h264_frame *source,
+ enum dma_data_direction dma_dir,
+ bool baseline_profile,
+ size_t csize)
+{
+ int err;
+
+ err = tegra_vde_attach_dmabuf(dev, source->y_fd,
+ source->y_offset, csize * 4,
+ &frame->y_dmabuf_attachment,
+ &frame->y_paddr,
+ &frame->y_sgt,
+ NULL, dma_dir);
+ if (err)
+ return err;
+
+ err = tegra_vde_attach_dmabuf(dev, source->cb_fd,
+ source->cb_offset, csize,
+ &frame->cb_dmabuf_attachment,
+ &frame->cb_paddr,
+ &frame->cb_sgt,
+ NULL, dma_dir);
+ if (err)
+ goto err_release_y;
+
+ err = tegra_vde_attach_dmabuf(dev, source->cr_fd,
+ source->cr_offset, csize,
+ &frame->cr_dmabuf_attachment,
+ &frame->cr_paddr,
+ &frame->cr_sgt,
+ NULL, dma_dir);
+ if (err)
+ goto err_release_cb;
+
+ if (baseline_profile) {
+ frame->aux_paddr = 0xF4DEAD00;
+ } else {
+ err = tegra_vde_attach_dmabuf(dev, source->aux_fd,
+ source->aux_offset, csize,
+ &frame->aux_dmabuf_attachment,
+ &frame->aux_paddr,
+ &frame->aux_sgt,
+ NULL, dma_dir);
+ if (err)
+ goto err_release_cr;
+ }
+
+ return 0;
+
+err_release_cr:
+ tegra_vde_detach_and_put_dmabuf(frame->cr_dmabuf_attachment,
+ frame->cr_sgt, dma_dir);
+err_release_cb:
+ tegra_vde_detach_and_put_dmabuf(frame->cb_dmabuf_attachment,
+ frame->cb_sgt, dma_dir);
+err_release_y:
+ tegra_vde_detach_and_put_dmabuf(frame->y_dmabuf_attachment,
+ frame->y_sgt, dma_dir);
+
+ return err;
+}
+
+static void tegra_vde_deattach_frame_dmabufs(struct video_frame *frame,
+ enum dma_data_direction dma_dir,
+ bool baseline_profile)
+{
+ if (!baseline_profile)
+ tegra_vde_detach_and_put_dmabuf(frame->aux_dmabuf_attachment,
+ frame->aux_sgt, dma_dir);
+
+ tegra_vde_detach_and_put_dmabuf(frame->cr_dmabuf_attachment,
+ frame->cr_sgt, dma_dir);
+
+ tegra_vde_detach_and_put_dmabuf(frame->cb_dmabuf_attachment,
+ frame->cb_sgt, dma_dir);
+
+ tegra_vde_detach_and_put_dmabuf(frame->y_dmabuf_attachment,
+ frame->y_sgt, dma_dir);
+}
+
+static int tegra_vde_copy_and_validate_frame(struct device *dev,
+ struct tegra_vde_h264_frame *frame,
+ unsigned long vaddr)
+{
+ if (copy_from_user(frame, (void __user *)vaddr, sizeof(*frame)))
+ return -EFAULT;
+
+ if (frame->frame_num > 0x7FFFFF) {
+ dev_err(dev, "Bad frame_num %u\n", frame->frame_num);
+ return -EINVAL;
+ }
+
+ if (frame->y_offset & 0xFF) {
+ dev_err(dev, "Bad y_offset 0x%X\n", frame->y_offset);
+ return -EINVAL;
+ }
+
+ if (frame->cb_offset & 0xFF) {
+ dev_err(dev, "Bad cb_offset 0x%X\n", frame->cb_offset);
+ return -EINVAL;
+ }
+
+ if (frame->cr_offset & 0xFF) {
+ dev_err(dev, "Bad cr_offset 0x%X\n", frame->cr_offset);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int tegra_vde_validate_h264_ctx(struct device *dev,
+ struct tegra_vde_h264_decoder_ctx *ctx)
+{
+ if (ctx->dpb_frames_nb == 0 || ctx->dpb_frames_nb > 17) {
+ dev_err(dev, "Bad DPB size %u\n", ctx->dpb_frames_nb);
+ return -EINVAL;
+ }
+
+ if (ctx->level_idc > 15) {
+ dev_err(dev, "Bad level value %u\n", ctx->level_idc);
+ return -EINVAL;
+ }
+
+ if (ctx->pic_init_qp > 52) {
+ dev_err(dev, "Bad pic_init_qp value %u\n", ctx->pic_init_qp);
+ return -EINVAL;
+ }
+
+ if (ctx->log2_max_pic_order_cnt_lsb > 16) {
+ dev_err(dev, "Bad log2_max_pic_order_cnt_lsb value %u\n",
+ ctx->log2_max_pic_order_cnt_lsb);
+ return -EINVAL;
+ }
+
+ if (ctx->log2_max_frame_num > 16) {
+ dev_err(dev, "Bad log2_max_frame_num value %u\n",
+ ctx->log2_max_frame_num);
+ return -EINVAL;
+ }
+
+ if (ctx->chroma_qp_index_offset > 31) {
+ dev_err(dev, "Bad chroma_qp_index_offset value %u\n",
+ ctx->chroma_qp_index_offset);
+ return -EINVAL;
+ }
+
+ if (ctx->pic_order_cnt_type > 2) {
+ dev_err(dev, "Bad pic_order_cnt_type value %u\n",
+ ctx->pic_order_cnt_type);
+ return -EINVAL;
+ }
+
+ if (ctx->num_ref_idx_l0_active_minus1 > 15) {
+ dev_err(dev, "Bad num_ref_idx_l0_active_minus1 value %u\n",
+ ctx->num_ref_idx_l0_active_minus1);
+ return -EINVAL;
+ }
+
+ if (ctx->num_ref_idx_l1_active_minus1 > 15) {
+ dev_err(dev, "Bad num_ref_idx_l1_active_minus1 value %u\n",
+ ctx->num_ref_idx_l1_active_minus1);
+ return -EINVAL;
+ }
+
+ if (!ctx->pic_width_in_mbs || ctx->pic_width_in_mbs > 127) {
+ dev_err(dev, "Bad pic_width_in_mbs value %u\n",
+ ctx->pic_width_in_mbs);
+ return -EINVAL;
+ }
+
+ if (!ctx->pic_height_in_mbs || ctx->pic_height_in_mbs > 127) {
+ dev_err(dev, "Bad pic_height_in_mbs value %u\n",
+ ctx->pic_height_in_mbs);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
+ unsigned long vaddr)
+{
+ struct device *dev = vde->miscdev.parent;
+ struct tegra_vde_h264_decoder_ctx ctx;
+ struct tegra_vde_h264_frame frame;
+ struct video_frame *dpb_frames;
+ struct dma_buf_attachment *bitstream_data_dmabuf_attachment;
+ struct sg_table *bitstream_sgt;
+ enum dma_data_direction dma_dir;
+ phys_addr_t bitstream_data_paddr;
+ phys_addr_t bsev_paddr;
+ size_t bitstream_data_size;
+ unsigned int macroblocks_nb;
+ unsigned int read_bytes;
+ unsigned int i;
+ bool timeout;
+ int ret;
+
+ if (copy_from_user(&ctx, (void __user *)vaddr, sizeof(ctx)))
+ return -EFAULT;
+
+ ret = tegra_vde_validate_h264_ctx(dev, &ctx);
+ if (ret)
+ return -EINVAL;
+
+ ret = tegra_vde_attach_dmabuf(dev, ctx.bitstream_data_fd,
+ ctx.bitstream_data_offset, 0,
+ &bitstream_data_dmabuf_attachment,
+ &bitstream_data_paddr,
+ &bitstream_sgt,
+ &bitstream_data_size,
+ DMA_TO_DEVICE);
+ if (ret)
+ return ret;
+
+ dpb_frames = kcalloc(ctx.dpb_frames_nb, sizeof(*dpb_frames),
+ GFP_KERNEL);
+ if (!dpb_frames) {
+ ret = -ENOMEM;
+ goto err_release_bitstream_dmabuf;
+ }
+
+ macroblocks_nb = ctx.pic_width_in_mbs * ctx.pic_height_in_mbs;
+ vaddr = ctx.dpb_frames_ptr;
+
+ for (i = 0; i < ctx.dpb_frames_nb; i++) {
+ ret = tegra_vde_copy_and_validate_frame(dev, &frame, vaddr);
+ if (ret)
+ goto err_release_dpb_frames;
+
+ dpb_frames[i].flags = frame.flags;
+ dpb_frames[i].frame_num = frame.frame_num;
+
+ dma_dir = (i == 0) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+
+ ret = tegra_vde_attach_dmabufs_to_frame(dev, &dpb_frames[i],
+ &frame, dma_dir,
+ ctx.baseline_profile,
+ macroblocks_nb * 64);
+ if (ret)
+ goto err_release_dpb_frames;
+
+ vaddr += sizeof(frame);
+ }
+
+ ret = mutex_lock_interruptible(&vde->lock);
+ if (ret)
+ goto err_release_dpb_frames;
+
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0)
+ goto err_unlock;
+
+ /*
+ * We rely on the VDE registers reset value, otherwise VDE
+ * causes bus lockup.
+ */
+ ret = reset_control_reset(vde->rst);
+ if (ret) {
+ dev_err(dev, "Failed to reset HW: %d\n", ret);
+ goto err_put_runtime_pm;
+ }
+
+ ret = tegra_vde_setup_hw_context(vde, &ctx, dpb_frames,
+ bitstream_data_paddr,
+ bitstream_data_size,
+ macroblocks_nb);
+ if (ret)
+ goto err_put_runtime_pm;
+
+ tegra_vde_decode_frame(vde, macroblocks_nb);
+
+ timeout = !wait_for_completion_killable_timeout(&vde->decode_completion,
+ TEGRA_VDE_TIMEOUT);
+ if (timeout) {
+ bsev_paddr = readl_relaxed(vde->regs + BSEV(0x10));
+ macroblocks_nb = readl_relaxed(vde->regs + SXE(0xC8)) & 0x1FFF;
+ read_bytes = bsev_paddr ? bsev_paddr - bitstream_data_paddr : 0;
+
+ dev_err(dev, "Decoding failed, "
+ "read 0x%X bytes : %u macroblocks parsed\n",
+ read_bytes, macroblocks_nb);
+
+ ret = -EIO;
+ }
+
+err_put_runtime_pm:
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
+err_unlock:
+ mutex_unlock(&vde->lock);
+
+err_release_dpb_frames:
+ while (i--) {
+ dma_dir = (i == 0) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+
+ tegra_vde_deattach_frame_dmabufs(&dpb_frames[i], dma_dir,
+ ctx.baseline_profile);
+ }
+
+ kfree(dpb_frames);
+
+err_release_bitstream_dmabuf:
+ tegra_vde_detach_and_put_dmabuf(bitstream_data_dmabuf_attachment,
+ bitstream_sgt, DMA_TO_DEVICE);
+
+ return ret;
+}
+
+static long tegra_vde_unlocked_ioctl(struct file *filp,
+ unsigned int cmd, unsigned long arg)
+{
+ struct miscdevice *miscdev = filp->private_data;
+ struct tegra_vde *vde = container_of(miscdev, struct tegra_vde,
+ miscdev);
+
+ switch (cmd) {
+ case TEGRA_VDE_IOCTL_DECODE_H264:
+ return tegra_vde_ioctl_decode_h264(vde, arg);
+ }
+
+ dev_err(miscdev->parent, "Invalid IOCTL command %u\n", cmd);
+
+ return -ENOTTY;
+}
+
+static const struct file_operations tegra_vde_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = tegra_vde_unlocked_ioctl,
+};
+
+static irqreturn_t tegra_vde_isr(int irq, void *data)
+{
+ struct tegra_vde *vde = data;
+
+ tegra_vde_set_bits(vde->regs, 0, FRAMEID(0x208));
+ complete(&vde->decode_completion);
+
+ return IRQ_HANDLED;
+}
+
+static int tegra_vde_runtime_suspend(struct device *dev)
+{
+ struct tegra_vde *vde = dev_get_drvdata(dev);
+ int err;
+
+ err = tegra_powergate_power_off(TEGRA_POWERGATE_VDEC);
+ if (err) {
+ dev_err(dev, "Failed to power down HW: %d\n", err);
+ return err;
+ }
+
+ clk_disable_unprepare(vde->clk);
+
+ return 0;
+}
+
+static int tegra_vde_runtime_resume(struct device *dev)
+{
+ struct tegra_vde *vde = dev_get_drvdata(dev);
+ int err;
+
+ err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VDEC,
+ vde->clk, vde->rst);
+ if (err) {
+ dev_err(dev, "Failed to power up HW : %d\n", err);
+ return err;
+ }
+
+ return 0;
+}
+
+static int tegra_vde_probe(struct platform_device *pdev)
+{
+ struct resource *res_regs, *res_iram;
+ struct device *dev = &pdev->dev;
+ struct tegra_vde *vde;
+ int irq, err;
+
+ vde = devm_kzalloc(&pdev->dev, sizeof(*vde), GFP_KERNEL);
+ if (!vde)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, vde);
+
+ res_regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
+ if (!res_regs)
+ return -ENODEV;
+
+ res_iram = platform_get_resource_byname(pdev, IORESOURCE_MEM, "iram");
+ if (!res_iram)
+ return -ENODEV;
+
+ irq = platform_get_irq_byname(pdev, "sync-token");
+ if (irq < 0)
+ return irq;
+
+ vde->regs = devm_ioremap_resource(dev, res_regs);
+ if (IS_ERR(vde->regs))
+ return PTR_ERR(vde->regs);
+
+ vde->iram = devm_ioremap_resource(dev, res_iram);
+ if (IS_ERR(vde->iram))
+ return PTR_ERR(vde->iram);
+
+ vde->clk = devm_clk_get(dev, "vde");
+ if (IS_ERR(vde->clk)) {
+ err = PTR_ERR(vde->clk);
+ dev_err(dev, "Could not get VDE clk %d\n", err);
+ return err;
+ }
+
+ vde->rst = devm_reset_control_get(dev, "vde");
+ if (IS_ERR(vde->rst)) {
+ err = PTR_ERR(vde->rst);
+ dev_err(dev, "Could not get VDE reset %d\n", err);
+ return err;
+ }
+
+ err = devm_request_irq(dev, irq, tegra_vde_isr, 0,
+ dev_name(dev), vde);
+ if (err) {
+ dev_err(&pdev->dev, "Failed to request IRQ %d\n", err);
+ return err;
+ }
+
+ mutex_init(&vde->lock);
+ init_completion(&vde->decode_completion);
+
+ vde->iram_lists_paddr = res_iram->start;
+ vde->miscdev.minor = MISC_DYNAMIC_MINOR;
+ vde->miscdev.name = "tegra_vde";
+ vde->miscdev.fops = &tegra_vde_fops;
+ vde->miscdev.parent = dev;
+
+ err = misc_register(&vde->miscdev);
+ if (err) {
+ dev_err(dev, "Failed to register misc device: %d\n", err);
+ return err;
+ }
+
+ pm_runtime_enable(dev);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_set_autosuspend_delay(dev, 300);
+
+ if (!pm_runtime_enabled(dev)) {
+ err = tegra_vde_runtime_resume(dev);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static int tegra_vde_remove(struct platform_device *pdev)
+{
+ struct tegra_vde *vde = platform_get_drvdata(pdev);
+ struct device *dev = &pdev->dev;
+ int err;
+
+ if (!pm_runtime_enabled(dev)) {
+ err = tegra_vde_runtime_suspend(dev);
+ if (err)
+ return err;
+ }
+
+ pm_runtime_dont_use_autosuspend(dev);
+ pm_runtime_disable(dev);
+
+ misc_deregister(&vde->miscdev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra_vde_pm_suspend(struct device *dev)
+{
+ struct tegra_vde *vde = dev_get_drvdata(dev);
+ int err;
+
+ mutex_lock(&vde->lock);
+
+ err = pm_runtime_force_suspend(dev);
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static int tegra_vde_pm_resume(struct device *dev)
+{
+ struct tegra_vde *vde = dev_get_drvdata(dev);
+ int err;
+
+ err = pm_runtime_force_resume(dev);
+ if (err < 0)
+ return err;
+
+ mutex_unlock(&vde->lock);
+
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops tegra_vde_pm_ops = {
+ SET_RUNTIME_PM_OPS(tegra_vde_runtime_suspend,
+ tegra_vde_runtime_resume,
+ NULL)
+ SET_SYSTEM_SLEEP_PM_OPS(tegra_vde_pm_suspend,
+ tegra_vde_pm_resume)
+};
+
+static const struct of_device_id tegra_vde_of_match[] = {
+ { .compatible = "nvidia,tegra20-vde", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, tegra_vde_of_match);
+
+static struct platform_driver tegra_vde_driver = {
+ .probe = tegra_vde_probe,
+ .remove = tegra_vde_remove,
+ .driver = {
+ .name = "tegra-vde",
+ .of_match_table = tegra_vde_of_match,
+ .pm = &tegra_vde_pm_ops,
+ },
+};
+module_platform_driver(tegra_vde_driver);
+
+MODULE_DESCRIPTION("NVIDIA Tegra20 Video Decoder driver");
+MODULE_AUTHOR("Dmitry Osipenko");
+MODULE_LICENSE("GPL");
--
2.14.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
2017-10-11 20:08 [PATCH v3 0/2] NVIDIA Tegra20 video decoder driver Dmitry Osipenko
2017-10-11 20:08 ` Dmitry Osipenko
@ 2017-10-11 20:08 ` Dmitry Osipenko
2017-10-12 7:43 ` Vladimir Zapolskiy
2017-10-12 8:49 ` Jon Hunter
1 sibling, 2 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-11 20:08 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
Add a device node for the video decoder engine found on Tegra20.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 7c85f97f72ea..1b5d54b6c0cb 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -249,6 +249,23 @@
*/
};
+ vde@6001a000 {
+ compatible = "nvidia,tegra20-vde";
+ reg = <0x6001a000 0x3D00 /* VDE registers */
+ 0x40000400 0x3FC00>; /* IRAM region */
+ reg-names = "regs", "iram";
+ interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, /* UCQ error interrupt */
+ <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */
+ <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */
+ <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, /* BSE-A interrupt */
+ <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
+ interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
+ clocks = <&tegra_car TEGRA20_CLK_VDE>;
+ clock-names = "vde";
+ resets = <&tegra_car 61>;
+ reset-names = "vde";
+ };
+
apbmisc@70000800 {
compatible = "nvidia,tegra20-apbmisc";
reg = <0x70000800 0x64 /* Chip revision */
--
2.14.2
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
2017-10-11 20:08 ` Dmitry Osipenko
@ 2017-10-11 20:47 ` Nicolas Dufresne
-1 siblings, 0 replies; 34+ messages in thread
From: Nicolas Dufresne @ 2017-10-11 20:47 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter,
Greg Kroah-Hartman, Rob Herring, Mauro Carvalho Chehab,
Stephen Warren
Cc: Dan Carpenter, linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 1586 bytes --]
Le mercredi 11 octobre 2017 à 23:08 +0300, Dmitry Osipenko a écrit :
> diff --git a/drivers/staging/tegra-vde/TODO b/drivers/staging/tegra-
> vde/TODO
> new file mode 100644
> index 000000000000..e98bbc7b3c19
> --- /dev/null
> +++ b/drivers/staging/tegra-vde/TODO
> @@ -0,0 +1,5 @@
> +TODO:
> + - Figure out how generic V4L2 API could be utilized by this
> driver,
> + implement it.
> +
That is a very interesting effort, I think it's the first time someone
is proposing an upstream driver for a Tegra platform. When I look
tegra_vde_h264_decoder_ctx, it looks like the only thing that the HW is
not parsing is the media header (pps/sps). Is that correct ?
I wonder how acceptable it would be to parse this inside the driver. It
is no more complex then parsing an EDID. If that was possible, wrapping
this driver as a v4l2 mem2mem should be rather simple. As a side
effect, you'll automatically get some userspace working, notably
GStreamer and FFmpeg.
For the case even parsing the headers is too much from a kernel point
of view, then I think you should have a look at the following effort.
It's a proposal base on yet to be merged Request API. Hugues is also
propose a libv4l2 adapter that makes the driver looks like a normal
v4l2 m2m, hiding all the userspace parsing and table filling. This
though, is long term plan to integrate state-less or parser-less
encoders into linux-media. It seems rather overkill for state-full
driver that requires parsed headers like PPS/SPS.
https://lwn.net/Articles/720797/
regards,
Nicolas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
@ 2017-10-11 20:47 ` Nicolas Dufresne
0 siblings, 0 replies; 34+ messages in thread
From: Nicolas Dufresne @ 2017-10-11 20:47 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter,
Greg Kroah-Hartman, Rob Herring, Mauro Carvalho Chehab,
Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1586 bytes --]
Le mercredi 11 octobre 2017 à 23:08 +0300, Dmitry Osipenko a écrit :
> diff --git a/drivers/staging/tegra-vde/TODO b/drivers/staging/tegra-
> vde/TODO
> new file mode 100644
> index 000000000000..e98bbc7b3c19
> --- /dev/null
> +++ b/drivers/staging/tegra-vde/TODO
> @@ -0,0 +1,5 @@
> +TODO:
> + - Figure out how generic V4L2 API could be utilized by this
> driver,
> + implement it.
> +
That is a very interesting effort, I think it's the first time someone
is proposing an upstream driver for a Tegra platform. When I look
tegra_vde_h264_decoder_ctx, it looks like the only thing that the HW is
not parsing is the media header (pps/sps). Is that correct ?
I wonder how acceptable it would be to parse this inside the driver. It
is no more complex then parsing an EDID. If that was possible, wrapping
this driver as a v4l2 mem2mem should be rather simple. As a side
effect, you'll automatically get some userspace working, notably
GStreamer and FFmpeg.
For the case even parsing the headers is too much from a kernel point
of view, then I think you should have a look at the following effort.
It's a proposal base on yet to be merged Request API. Hugues is also
propose a libv4l2 adapter that makes the driver looks like a normal
v4l2 m2m, hiding all the userspace parsing and table filling. This
though, is long term plan to integrate state-less or parser-less
encoders into linux-media. It seems rather overkill for state-full
driver that requires parsed headers like PPS/SPS.
https://lwn.net/Articles/720797/
regards,
Nicolas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
2017-10-11 20:47 ` Nicolas Dufresne
@ 2017-10-11 22:37 ` Dmitry Osipenko
-1 siblings, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-11 22:37 UTC (permalink / raw)
To: Nicolas Dufresne, Thierry Reding, Jonathan Hunter,
Greg Kroah-Hartman, Rob Herring, Mauro Carvalho Chehab,
Stephen Warren
Cc: Dan Carpenter, linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On 11.10.2017 23:47, Nicolas Dufresne wrote:
> Le mercredi 11 octobre 2017 à 23:08 +0300, Dmitry Osipenko a écrit :
>> diff --git a/drivers/staging/tegra-vde/TODO b/drivers/staging/tegra-
>> vde/TODO
>> new file mode 100644
>> index 000000000000..e98bbc7b3c19
>> --- /dev/null
>> +++ b/drivers/staging/tegra-vde/TODO
>> @@ -0,0 +1,5 @@
>> +TODO:
>> + - Figure out how generic V4L2 API could be utilized by this
>> driver,
>> + implement it.
>> +
>
> That is a very interesting effort, I think it's the first time someone
> is proposing an upstream driver for a Tegra platform.
Thanks!
When I look
> tegra_vde_h264_decoder_ctx, it looks like the only thing that the HW is
> not parsing is the media header (pps/sps). Is that correct ?
>
That's correct. I think it's quite common among embedded (mobile) and
desktop-grade decoders to require some auxiliary info from the media headers.
> I wonder how acceptable it would be to parse this inside the driver. It
> is no more complex then parsing an EDID. If that was possible, wrapping
> this driver as a v4l2 mem2mem should be rather simple. As a side
> effect, you'll automatically get some userspace working, notably
> GStreamer and FFmpeg.
>
Parsing bitstream in kernel feels a bit dirty, although it's up to media
maintainers to decide.
> For the case even parsing the headers is too much from a kernel point
> of view, then I think you should have a look at the following effort.
> It's a proposal base on yet to be merged Request API. Hugues is also
> propose a libv4l2 adapter that makes the driver looks like a normal
> v4l2 m2m, hiding all the userspace parsing and table filling. This
> though, is long term plan to integrate state-less or parser-less
> encoders into linux-media. It seems rather overkill for state-full
> driver that requires parsed headers like PPS/SPS.
>
> https://lwn.net/Articles/720797/
>
I'll take a look at the Request API / libv4l2 adapter, thank you very much for
pointing to it.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
@ 2017-10-11 22:37 ` Dmitry Osipenko
0 siblings, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-11 22:37 UTC (permalink / raw)
To: Nicolas Dufresne, Thierry Reding, Jonathan Hunter,
Greg Kroah-Hartman, Rob Herring, Mauro Carvalho Chehab,
Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
On 11.10.2017 23:47, Nicolas Dufresne wrote:
> Le mercredi 11 octobre 2017 à 23:08 +0300, Dmitry Osipenko a écrit :
>> diff --git a/drivers/staging/tegra-vde/TODO b/drivers/staging/tegra-
>> vde/TODO
>> new file mode 100644
>> index 000000000000..e98bbc7b3c19
>> --- /dev/null
>> +++ b/drivers/staging/tegra-vde/TODO
>> @@ -0,0 +1,5 @@
>> +TODO:
>> + - Figure out how generic V4L2 API could be utilized by this
>> driver,
>> + implement it.
>> +
>
> That is a very interesting effort, I think it's the first time someone
> is proposing an upstream driver for a Tegra platform.
Thanks!
When I look
> tegra_vde_h264_decoder_ctx, it looks like the only thing that the HW is
> not parsing is the media header (pps/sps). Is that correct ?
>
That's correct. I think it's quite common among embedded (mobile) and
desktop-grade decoders to require some auxiliary info from the media headers.
> I wonder how acceptable it would be to parse this inside the driver. It
> is no more complex then parsing an EDID. If that was possible, wrapping
> this driver as a v4l2 mem2mem should be rather simple. As a side
> effect, you'll automatically get some userspace working, notably
> GStreamer and FFmpeg.
>
Parsing bitstream in kernel feels a bit dirty, although it's up to media
maintainers to decide.
> For the case even parsing the headers is too much from a kernel point
> of view, then I think you should have a look at the following effort.
> It's a proposal base on yet to be merged Request API. Hugues is also
> propose a libv4l2 adapter that makes the driver looks like a normal
> v4l2 m2m, hiding all the userspace parsing and table filling. This
> though, is long term plan to integrate state-less or parser-less
> encoders into linux-media. It seems rather overkill for state-full
> driver that requires parsed headers like PPS/SPS.
>
> https://lwn.net/Articles/720797/
>
I'll take a look at the Request API / libv4l2 adapter, thank you very much for
pointing to it.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
2017-10-11 20:08 ` [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node Dmitry Osipenko
@ 2017-10-12 7:43 ` Vladimir Zapolskiy
2017-10-12 8:49 ` Jon Hunter
1 sibling, 0 replies; 34+ messages in thread
From: Vladimir Zapolskiy @ 2017-10-12 7:43 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter,
Greg Kroah-Hartman, Rob Herring, Mauro Carvalho Chehab,
Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
Hello Dmitry,
On 10/11/2017 11:08 PM, Dmitry Osipenko wrote:
> Add a device node for the video decoder engine found on Tegra20.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 7c85f97f72ea..1b5d54b6c0cb 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -249,6 +249,23 @@
> */
> };
>
> + vde@6001a000 {
> + compatible = "nvidia,tegra20-vde";
> + reg = <0x6001a000 0x3D00 /* VDE registers */
> + 0x40000400 0x3FC00>; /* IRAM region */
this notation of a used region in IRAM is non-standard and potentially it
may lead to conflicts for IRAM resource between users.
My proposal is to add a valid device tree node to describe an IRAM region
firstly, then reserve a subregion in it by using a new "iram" property.
----8<----
From: Vladimir Zapolskiy <vz@mleia.com>
Date: Thu, 12 Oct 2017 10:25:45 +0300
Subject: [PATCH] ARM: tegra: add device tree node to describe IRAM on Tegra20
All Tegra20 SoCs contain 256KiB IRAM, which is used to store
resume code and by a video decoder engine.
Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
arch/arm/boot/dts/tegra20.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 7c85f97f72ea..fd2843c90920 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -9,6 +9,14 @@
compatible = "nvidia,tegra20";
interrupt-parent = <&lic>;
+ iram@40000000 {
+ compatible = "mmio-sram";
+ reg = <0x40000000 0x40000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x40000000 0x40000>;
+ };
+
host1x@50000000 {
compatible = "nvidia,tegra20-host1x", "simple-bus";
reg = <0x50000000 0x00024000>;
----8<----
Please add the change above to your next version of the series, or
if you wish I can send it separately for review by Thierry.
After applying that change you do define a region in IRAM for the exclusive
usage by a video decoder engine and add an 'iram' property:
----8<----
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index fd2843c90920..5133fbac2185 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -15,6 +15,11 @@
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x40000000 0x40000>;
+
+ vde_pool: vde {
+ reg = <0x400 0x3fc00>;
+ pool;
+ };
};
host1x@50000000 {
[snip]
+ vde@6001a000 {
+ compatible = "nvidia,tegra20-vde";
+ reg = <0x6001a000 0x3d00>; /* VDE registers */
+ iram = <&vde_pool>; /* IRAM region */
[snip]
----8<----
And finally in the driver you'll use genalloc API to access the IRAM
region, for that you can find ready examples in the kernel source code.
--
With best wishes,
Vladimir
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
@ 2017-10-12 7:43 ` Vladimir Zapolskiy
0 siblings, 0 replies; 34+ messages in thread
From: Vladimir Zapolskiy @ 2017-10-12 7:43 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter,
Greg Kroah-Hartman, Rob Herring, Mauro Carvalho Chehab,
Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
Hello Dmitry,
On 10/11/2017 11:08 PM, Dmitry Osipenko wrote:
> Add a device node for the video decoder engine found on Tegra20.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 7c85f97f72ea..1b5d54b6c0cb 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -249,6 +249,23 @@
> */
> };
>
> + vde@6001a000 {
> + compatible = "nvidia,tegra20-vde";
> + reg = <0x6001a000 0x3D00 /* VDE registers */
> + 0x40000400 0x3FC00>; /* IRAM region */
this notation of a used region in IRAM is non-standard and potentially it
may lead to conflicts for IRAM resource between users.
My proposal is to add a valid device tree node to describe an IRAM region
firstly, then reserve a subregion in it by using a new "iram" property.
----8<----
From: Vladimir Zapolskiy <vz@mleia.com>
Date: Thu, 12 Oct 2017 10:25:45 +0300
Subject: [PATCH] ARM: tegra: add device tree node to describe IRAM on Tegra20
All Tegra20 SoCs contain 256KiB IRAM, which is used to store
resume code and by a video decoder engine.
Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
arch/arm/boot/dts/tegra20.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 7c85f97f72ea..fd2843c90920 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -9,6 +9,14 @@
compatible = "nvidia,tegra20";
interrupt-parent = <&lic>;
+ iram@40000000 {
+ compatible = "mmio-sram";
+ reg = <0x40000000 0x40000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x40000000 0x40000>;
+ };
+
host1x@50000000 {
compatible = "nvidia,tegra20-host1x", "simple-bus";
reg = <0x50000000 0x00024000>;
----8<----
Please add the change above to your next version of the series, or
if you wish I can send it separately for review by Thierry.
After applying that change you do define a region in IRAM for the exclusive
usage by a video decoder engine and add an 'iram' property:
----8<----
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index fd2843c90920..5133fbac2185 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -15,6 +15,11 @@
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x40000000 0x40000>;
+
+ vde_pool: vde {
+ reg = <0x400 0x3fc00>;
+ pool;
+ };
};
host1x@50000000 {
[snip]
+ vde@6001a000 {
+ compatible = "nvidia,tegra20-vde";
+ reg = <0x6001a000 0x3d00>; /* VDE registers */
+ iram = <&vde_pool>; /* IRAM region */
[snip]
----8<----
And finally in the driver you'll use genalloc API to access the IRAM
region, for that you can find ready examples in the kernel source code.
--
With best wishes,
Vladimir
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
2017-10-11 20:08 ` [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node Dmitry Osipenko
@ 2017-10-12 8:49 ` Jon Hunter
2017-10-12 8:49 ` Jon Hunter
1 sibling, 0 replies; 34+ messages in thread
From: Jon Hunter @ 2017-10-12 8:49 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
On 11/10/17 21:08, Dmitry Osipenko wrote:
> Add a device node for the video decoder engine found on Tegra20.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 7c85f97f72ea..1b5d54b6c0cb 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -249,6 +249,23 @@
> */
> };
>
> + vde@6001a000 {
> + compatible = "nvidia,tegra20-vde";
> + reg = <0x6001a000 0x3D00 /* VDE registers */
> + 0x40000400 0x3FC00>; /* IRAM region */
> + reg-names = "regs", "iram";
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, /* UCQ error interrupt */
> + <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */
> + <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */
> + <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, /* BSE-A interrupt */
> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
> + interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
> + clocks = <&tegra_car TEGRA20_CLK_VDE>;
> + clock-names = "vde";
> + resets = <&tegra_car 61>;
> + reset-names = "vde";
> + };
> +
I don't see any binding documentation for this node. We need to make
sure we add this.
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
@ 2017-10-12 8:49 ` Jon Hunter
0 siblings, 0 replies; 34+ messages in thread
From: Jon Hunter @ 2017-10-12 8:49 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
On 11/10/17 21:08, Dmitry Osipenko wrote:
> Add a device node for the video decoder engine found on Tegra20.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 7c85f97f72ea..1b5d54b6c0cb 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -249,6 +249,23 @@
> */
> };
>
> + vde@6001a000 {
> + compatible = "nvidia,tegra20-vde";
> + reg = <0x6001a000 0x3D00 /* VDE registers */
> + 0x40000400 0x3FC00>; /* IRAM region */
> + reg-names = "regs", "iram";
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, /* UCQ error interrupt */
> + <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */
> + <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */
> + <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, /* BSE-A interrupt */
> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
> + interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
> + clocks = <&tegra_car TEGRA20_CLK_VDE>;
> + clock-names = "vde";
> + resets = <&tegra_car 61>;
> + reset-names = "vde";
> + };
> +
I don't see any binding documentation for this node. We need to make
sure we add this.
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
2017-10-12 8:49 ` Jon Hunter
@ 2017-10-12 10:51 ` Dmitry Osipenko
-1 siblings, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-12 10:51 UTC (permalink / raw)
To: Jon Hunter, Thierry Reding, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren
Cc: Dan Carpenter, linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On 12.10.2017 11:49, Jon Hunter wrote:
>
> On 11/10/17 21:08, Dmitry Osipenko wrote:
>> Add a device node for the video decoder engine found on Tegra20.
>>
>> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>> index 7c85f97f72ea..1b5d54b6c0cb 100644
>> --- a/arch/arm/boot/dts/tegra20.dtsi
>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>> @@ -249,6 +249,23 @@
>> */
>> };
>>
>> + vde@6001a000 {
>> + compatible = "nvidia,tegra20-vde";
>> + reg = <0x6001a000 0x3D00 /* VDE registers */
>> + 0x40000400 0x3FC00>; /* IRAM region */
>> + reg-names = "regs", "iram";
>> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, /* UCQ error interrupt */
>> + <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */
>> + <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */
>> + <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, /* BSE-A interrupt */
>> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
>> + interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
>> + clocks = <&tegra_car TEGRA20_CLK_VDE>;
>> + clock-names = "vde";
>> + resets = <&tegra_car 61>;
>> + reset-names = "vde";
>> + };
>> +
>
> I don't see any binding documentation for this node. We need to make
> sure we add this.
>
It's in the first patch.
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
@ 2017-10-12 10:51 ` Dmitry Osipenko
0 siblings, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-12 10:51 UTC (permalink / raw)
To: Jon Hunter, Thierry Reding, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
On 12.10.2017 11:49, Jon Hunter wrote:
>
> On 11/10/17 21:08, Dmitry Osipenko wrote:
>> Add a device node for the video decoder engine found on Tegra20.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>> index 7c85f97f72ea..1b5d54b6c0cb 100644
>> --- a/arch/arm/boot/dts/tegra20.dtsi
>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>> @@ -249,6 +249,23 @@
>> */
>> };
>>
>> + vde@6001a000 {
>> + compatible = "nvidia,tegra20-vde";
>> + reg = <0x6001a000 0x3D00 /* VDE registers */
>> + 0x40000400 0x3FC00>; /* IRAM region */
>> + reg-names = "regs", "iram";
>> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, /* UCQ error interrupt */
>> + <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */
>> + <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */
>> + <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, /* BSE-A interrupt */
>> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
>> + interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
>> + clocks = <&tegra_car TEGRA20_CLK_VDE>;
>> + clock-names = "vde";
>> + resets = <&tegra_car 61>;
>> + reset-names = "vde";
>> + };
>> +
>
> I don't see any binding documentation for this node. We need to make
> sure we add this.
>
It's in the first patch.
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
2017-10-12 10:51 ` Dmitry Osipenko
@ 2017-10-12 10:57 ` Jon Hunter
-1 siblings, 0 replies; 34+ messages in thread
From: Jon Hunter @ 2017-10-12 10:57 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
On 12/10/17 11:51, Dmitry Osipenko wrote:
> On 12.10.2017 11:49, Jon Hunter wrote:
>>
>> On 11/10/17 21:08, Dmitry Osipenko wrote:
>>> Add a device node for the video decoder engine found on Tegra20.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>> index 7c85f97f72ea..1b5d54b6c0cb 100644
>>> --- a/arch/arm/boot/dts/tegra20.dtsi
>>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>>> @@ -249,6 +249,23 @@
>>> */
>>> };
>>>
>>> + vde@6001a000 {
>>> + compatible = "nvidia,tegra20-vde";
>>> + reg = <0x6001a000 0x3D00 /* VDE registers */
>>> + 0x40000400 0x3FC00>; /* IRAM region */
>>> + reg-names = "regs", "iram";
>>> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, /* UCQ error interrupt */
>>> + <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */
>>> + <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */
>>> + <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, /* BSE-A interrupt */
>>> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
>>> + interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
>>> + clocks = <&tegra_car TEGRA20_CLK_VDE>;
>>> + clock-names = "vde";
>>> + resets = <&tegra_car 61>;
>>> + reset-names = "vde";
>>> + };
>>> +
>>
>> I don't see any binding documentation for this node. We need to make
>> sure we add this.
>>
>
> It's in the first patch.
>
> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
>
Ah yes indeed, then that needs to be a separate patch.
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
@ 2017-10-12 10:57 ` Jon Hunter
0 siblings, 0 replies; 34+ messages in thread
From: Jon Hunter @ 2017-10-12 10:57 UTC (permalink / raw)
To: Dmitry Osipenko, Thierry Reding, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
On 12/10/17 11:51, Dmitry Osipenko wrote:
> On 12.10.2017 11:49, Jon Hunter wrote:
>>
>> On 11/10/17 21:08, Dmitry Osipenko wrote:
>>> Add a device node for the video decoder engine found on Tegra20.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>> index 7c85f97f72ea..1b5d54b6c0cb 100644
>>> --- a/arch/arm/boot/dts/tegra20.dtsi
>>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>>> @@ -249,6 +249,23 @@
>>> */
>>> };
>>>
>>> + vde@6001a000 {
>>> + compatible = "nvidia,tegra20-vde";
>>> + reg = <0x6001a000 0x3D00 /* VDE registers */
>>> + 0x40000400 0x3FC00>; /* IRAM region */
>>> + reg-names = "regs", "iram";
>>> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, /* UCQ error interrupt */
>>> + <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */
>>> + <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */
>>> + <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, /* BSE-A interrupt */
>>> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
>>> + interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
>>> + clocks = <&tegra_car TEGRA20_CLK_VDE>;
>>> + clock-names = "vde";
>>> + resets = <&tegra_car 61>;
>>> + reset-names = "vde";
>>> + };
>>> +
>>
>> I don't see any binding documentation for this node. We need to make
>> sure we add this.
>>
>
> It's in the first patch.
>
> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
>
Ah yes indeed, then that needs to be a separate patch.
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
2017-10-12 10:57 ` Jon Hunter
(?)
@ 2017-10-12 11:11 ` Dmitry Osipenko
-1 siblings, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-12 11:11 UTC (permalink / raw)
To: Jon Hunter, Thierry Reding, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
On 12.10.2017 13:57, Jon Hunter wrote:
>
> On 12/10/17 11:51, Dmitry Osipenko wrote:
>> On 12.10.2017 11:49, Jon Hunter wrote:
>>>
>>> On 11/10/17 21:08, Dmitry Osipenko wrote:
>>>> Add a device node for the video decoder engine found on Tegra20.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
>>>> 1 file changed, 17 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>>> index 7c85f97f72ea..1b5d54b6c0cb 100644
>>>> --- a/arch/arm/boot/dts/tegra20.dtsi
>>>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>>>> @@ -249,6 +249,23 @@
>>>> */
>>>> };
>>>>
>>>> + vde@6001a000 {
>>>> + compatible = "nvidia,tegra20-vde";
>>>> + reg = <0x6001a000 0x3D00 /* VDE registers */
>>>> + 0x40000400 0x3FC00>; /* IRAM region */
>>>> + reg-names = "regs", "iram";
>>>> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, /* UCQ error interrupt */
>>>> + <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */
>>>> + <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */
>>>> + <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, /* BSE-A interrupt */
>>>> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
>>>> + interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
>>>> + clocks = <&tegra_car TEGRA20_CLK_VDE>;
>>>> + clock-names = "vde";
>>>> + resets = <&tegra_car 61>;
>>>> + reset-names = "vde";
>>>> + };
>>>> +
>>>
>>> I don't see any binding documentation for this node. We need to make
>>> sure we add this.
>>>
>>
>> It's in the first patch.
>>
>> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
>>
>
> Ah yes indeed, then that needs to be a separate patch.
>
Okay
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
2017-10-12 7:43 ` Vladimir Zapolskiy
@ 2017-10-12 12:06 ` Dmitry Osipenko
-1 siblings, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-12 12:06 UTC (permalink / raw)
To: Vladimir Zapolskiy, Thierry Reding, Jonathan Hunter,
Greg Kroah-Hartman, Rob Herring, Mauro Carvalho Chehab,
Stephen Warren
Cc: Dan Carpenter, linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Hello Vladimir,
On 12.10.2017 10:43, Vladimir Zapolskiy wrote:
> Hello Dmitry,
>
> On 10/11/2017 11:08 PM, Dmitry Osipenko wrote:
>> Add a device node for the video decoder engine found on Tegra20.
>>
>> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>> index 7c85f97f72ea..1b5d54b6c0cb 100644
>> --- a/arch/arm/boot/dts/tegra20.dtsi
>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>> @@ -249,6 +249,23 @@
>> */
>> };
>>
>> + vde@6001a000 {
>> + compatible = "nvidia,tegra20-vde";
>> + reg = <0x6001a000 0x3D00 /* VDE registers */
>> + 0x40000400 0x3FC00>; /* IRAM region */
>
> this notation of a used region in IRAM is non-standard and potentially it
> may lead to conflicts for IRAM resource between users.
>
> My proposal is to add a valid device tree node to describe an IRAM region
> firstly, then reserve a subregion in it by using a new "iram" property.
>
The defined in DT IRAM region used by VDE isn't exactly correct, actually it
should be much smaller. I don't know exactly what parts of IRAM VDE uses, for
now it is just safer to assign the rest of the IRAM region to VDE.
I'm not sure whether it really worthy to use a dynamic allocator for a single
static allocation, but maybe it would come handy later.. Stephen / Jon /
Thierry, what do you think?
> ----8<----
> From: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
> Date: Thu, 12 Oct 2017 10:25:45 +0300
> Subject: [PATCH] ARM: tegra: add device tree node to describe IRAM on Tegra20
>
> All Tegra20 SoCs contain 256KiB IRAM, which is used to store
> resume code and by a video decoder engine.
>
> Signed-off-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
> ---
> arch/arm/boot/dts/tegra20.dtsi | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 7c85f97f72ea..fd2843c90920 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -9,6 +9,14 @@
> compatible = "nvidia,tegra20";
> interrupt-parent = <&lic>;
>
> + iram@40000000 {
> + compatible = "mmio-sram";
> + reg = <0x40000000 0x40000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x40000000 0x40000>;
> + };
> +
> host1x@50000000 {
> compatible = "nvidia,tegra20-host1x", "simple-bus";
> reg = <0x50000000 0x00024000>;
> ----8<----
>
> Please add the change above to your next version of the series, or
> if you wish I can send it separately for review by Thierry.
>
> After applying that change you do define a region in IRAM for the exclusive
> usage by a video decoder engine and add an 'iram' property:
>
Newer Tegra generations also have the IRAM, so I think Tegra30/114/124 DT's
should also include the same IRAM node for consistency. I'll extend your patch
to cover other Tegra's and include it in v4 if you don't mind and if Stephen /
Jon / Thierry would approve your proposal.
> ----8<----
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index fd2843c90920..5133fbac2185 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -15,6 +15,11 @@
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0 0x40000000 0x40000>;
> +
> + vde_pool: vde {
> + reg = <0x400 0x3fc00>;
> + pool;
> + };
> };
>
> host1x@50000000 {
> [snip]
>
> + vde@6001a000 {
> + compatible = "nvidia,tegra20-vde";
> + reg = <0x6001a000 0x3d00>; /* VDE registers */
> + iram = <&vde_pool>; /* IRAM region */
> [snip]
> ----8<----
>
> And finally in the driver you'll use genalloc API to access the IRAM
> region, for that you can find ready examples in the kernel source code.
>
Thank you very much for taking a look at the patch!
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
@ 2017-10-12 12:06 ` Dmitry Osipenko
0 siblings, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-12 12:06 UTC (permalink / raw)
To: Vladimir Zapolskiy, Thierry Reding, Jonathan Hunter,
Greg Kroah-Hartman, Rob Herring, Mauro Carvalho Chehab,
Stephen Warren
Cc: Dan Carpenter, linux-media, linux-tegra, devel, devicetree, linux-kernel
Hello Vladimir,
On 12.10.2017 10:43, Vladimir Zapolskiy wrote:
> Hello Dmitry,
>
> On 10/11/2017 11:08 PM, Dmitry Osipenko wrote:
>> Add a device node for the video decoder engine found on Tegra20.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>> index 7c85f97f72ea..1b5d54b6c0cb 100644
>> --- a/arch/arm/boot/dts/tegra20.dtsi
>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>> @@ -249,6 +249,23 @@
>> */
>> };
>>
>> + vde@6001a000 {
>> + compatible = "nvidia,tegra20-vde";
>> + reg = <0x6001a000 0x3D00 /* VDE registers */
>> + 0x40000400 0x3FC00>; /* IRAM region */
>
> this notation of a used region in IRAM is non-standard and potentially it
> may lead to conflicts for IRAM resource between users.
>
> My proposal is to add a valid device tree node to describe an IRAM region
> firstly, then reserve a subregion in it by using a new "iram" property.
>
The defined in DT IRAM region used by VDE isn't exactly correct, actually it
should be much smaller. I don't know exactly what parts of IRAM VDE uses, for
now it is just safer to assign the rest of the IRAM region to VDE.
I'm not sure whether it really worthy to use a dynamic allocator for a single
static allocation, but maybe it would come handy later.. Stephen / Jon /
Thierry, what do you think?
> ----8<----
> From: Vladimir Zapolskiy <vz@mleia.com>
> Date: Thu, 12 Oct 2017 10:25:45 +0300
> Subject: [PATCH] ARM: tegra: add device tree node to describe IRAM on Tegra20
>
> All Tegra20 SoCs contain 256KiB IRAM, which is used to store
> resume code and by a video decoder engine.
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> ---
> arch/arm/boot/dts/tegra20.dtsi | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 7c85f97f72ea..fd2843c90920 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -9,6 +9,14 @@
> compatible = "nvidia,tegra20";
> interrupt-parent = <&lic>;
>
> + iram@40000000 {
> + compatible = "mmio-sram";
> + reg = <0x40000000 0x40000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x40000000 0x40000>;
> + };
> +
> host1x@50000000 {
> compatible = "nvidia,tegra20-host1x", "simple-bus";
> reg = <0x50000000 0x00024000>;
> ----8<----
>
> Please add the change above to your next version of the series, or
> if you wish I can send it separately for review by Thierry.
>
> After applying that change you do define a region in IRAM for the exclusive
> usage by a video decoder engine and add an 'iram' property:
>
Newer Tegra generations also have the IRAM, so I think Tegra30/114/124 DT's
should also include the same IRAM node for consistency. I'll extend your patch
to cover other Tegra's and include it in v4 if you don't mind and if Stephen /
Jon / Thierry would approve your proposal.
> ----8<----
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index fd2843c90920..5133fbac2185 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -15,6 +15,11 @@
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0 0x40000000 0x40000>;
> +
> + vde_pool: vde {
> + reg = <0x400 0x3fc00>;
> + pool;
> + };
> };
>
> host1x@50000000 {
> [snip]
>
> + vde@6001a000 {
> + compatible = "nvidia,tegra20-vde";
> + reg = <0x6001a000 0x3d00>; /* VDE registers */
> + iram = <&vde_pool>; /* IRAM region */
> [snip]
> ----8<----
>
> And finally in the driver you'll use genalloc API to access the IRAM
> region, for that you can find ready examples in the kernel source code.
>
Thank you very much for taking a look at the patch!
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
2017-10-12 12:06 ` Dmitry Osipenko
@ 2017-10-12 13:25 ` Thierry Reding
-1 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2017-10-12 13:25 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: devel, devicetree, Stephen Warren, Greg Kroah-Hartman,
linux-kernel, Jonathan Hunter, Rob Herring, Dan Carpenter,
linux-tegra, Mauro Carvalho Chehab, Vladimir Zapolskiy,
linux-media
[-- Attachment #1.1: Type: text/plain, Size: 2317 bytes --]
On Thu, Oct 12, 2017 at 03:06:17PM +0300, Dmitry Osipenko wrote:
> Hello Vladimir,
>
> On 12.10.2017 10:43, Vladimir Zapolskiy wrote:
> > Hello Dmitry,
> >
> > On 10/11/2017 11:08 PM, Dmitry Osipenko wrote:
> >> Add a device node for the video decoder engine found on Tegra20.
> >>
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >> ---
> >> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
> >> 1 file changed, 17 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> >> index 7c85f97f72ea..1b5d54b6c0cb 100644
> >> --- a/arch/arm/boot/dts/tegra20.dtsi
> >> +++ b/arch/arm/boot/dts/tegra20.dtsi
> >> @@ -249,6 +249,23 @@
> >> */
> >> };
> >>
> >> + vde@6001a000 {
> >> + compatible = "nvidia,tegra20-vde";
> >> + reg = <0x6001a000 0x3D00 /* VDE registers */
> >> + 0x40000400 0x3FC00>; /* IRAM region */
> >
> > this notation of a used region in IRAM is non-standard and potentially it
> > may lead to conflicts for IRAM resource between users.
> >
> > My proposal is to add a valid device tree node to describe an IRAM region
> > firstly, then reserve a subregion in it by using a new "iram" property.
> >
>
> The defined in DT IRAM region used by VDE isn't exactly correct, actually it
> should be much smaller. I don't know exactly what parts of IRAM VDE uses, for
> now it is just safer to assign the rest of the IRAM region to VDE.
>
> I'm not sure whether it really worthy to use a dynamic allocator for a single
> static allocation, but maybe it would come handy later.. Stephen / Jon /
> Thierry, what do you think?
This sounds like a good idea. I agree that this currently doesn't seem
to be warranted, but consider what would happen if at some point we have
more devices requiring access to the IRAM. Spreading individual reg
properties all across the DT will make it very difficult to ensure they
don't overlap.
Presumably the mmio-sram driver will check that pool don't overlap. Or
even if it doesn't it will make it a lot easier to verify because it's
all in the same DT node and then consumers only reference it.
I like Vladimir's proposal. I also suspect that Rob may want us to stick
to a standardized way referencing such external memory.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
@ 2017-10-12 13:25 ` Thierry Reding
0 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2017-10-12 13:25 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Vladimir Zapolskiy, Jonathan Hunter, Greg Kroah-Hartman,
Rob Herring, Mauro Carvalho Chehab, Stephen Warren,
Dan Carpenter, linux-media, linux-tegra, devel, devicetree,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2317 bytes --]
On Thu, Oct 12, 2017 at 03:06:17PM +0300, Dmitry Osipenko wrote:
> Hello Vladimir,
>
> On 12.10.2017 10:43, Vladimir Zapolskiy wrote:
> > Hello Dmitry,
> >
> > On 10/11/2017 11:08 PM, Dmitry Osipenko wrote:
> >> Add a device node for the video decoder engine found on Tegra20.
> >>
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >> ---
> >> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
> >> 1 file changed, 17 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> >> index 7c85f97f72ea..1b5d54b6c0cb 100644
> >> --- a/arch/arm/boot/dts/tegra20.dtsi
> >> +++ b/arch/arm/boot/dts/tegra20.dtsi
> >> @@ -249,6 +249,23 @@
> >> */
> >> };
> >>
> >> + vde@6001a000 {
> >> + compatible = "nvidia,tegra20-vde";
> >> + reg = <0x6001a000 0x3D00 /* VDE registers */
> >> + 0x40000400 0x3FC00>; /* IRAM region */
> >
> > this notation of a used region in IRAM is non-standard and potentially it
> > may lead to conflicts for IRAM resource between users.
> >
> > My proposal is to add a valid device tree node to describe an IRAM region
> > firstly, then reserve a subregion in it by using a new "iram" property.
> >
>
> The defined in DT IRAM region used by VDE isn't exactly correct, actually it
> should be much smaller. I don't know exactly what parts of IRAM VDE uses, for
> now it is just safer to assign the rest of the IRAM region to VDE.
>
> I'm not sure whether it really worthy to use a dynamic allocator for a single
> static allocation, but maybe it would come handy later.. Stephen / Jon /
> Thierry, what do you think?
This sounds like a good idea. I agree that this currently doesn't seem
to be warranted, but consider what would happen if at some point we have
more devices requiring access to the IRAM. Spreading individual reg
properties all across the DT will make it very difficult to ensure they
don't overlap.
Presumably the mmio-sram driver will check that pool don't overlap. Or
even if it doesn't it will make it a lot easier to verify because it's
all in the same DT node and then consumers only reference it.
I like Vladimir's proposal. I also suspect that Rob may want us to stick
to a standardized way referencing such external memory.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
2017-10-12 13:25 ` Thierry Reding
@ 2017-10-12 13:45 ` Jon Hunter
-1 siblings, 0 replies; 34+ messages in thread
From: Jon Hunter @ 2017-10-12 13:45 UTC (permalink / raw)
To: Thierry Reding, Dmitry Osipenko
Cc: devel, devicetree, Stephen Warren, Greg Kroah-Hartman,
linux-kernel, Rob Herring, Vladimir Zapolskiy, linux-tegra,
Mauro Carvalho Chehab, Dan Carpenter, linux-media
On 12/10/17 14:25, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Thu, Oct 12, 2017 at 03:06:17PM +0300, Dmitry Osipenko wrote:
>> Hello Vladimir,
>>
>> On 12.10.2017 10:43, Vladimir Zapolskiy wrote:
>>> Hello Dmitry,
>>>
>>> On 10/11/2017 11:08 PM, Dmitry Osipenko wrote:
>>>> Add a device node for the video decoder engine found on Tegra20.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
>>>> 1 file changed, 17 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>>> index 7c85f97f72ea..1b5d54b6c0cb 100644
>>>> --- a/arch/arm/boot/dts/tegra20.dtsi
>>>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>>>> @@ -249,6 +249,23 @@
>>>> */
>>>> };
>>>>
>>>> + vde@6001a000 {
>>>> + compatible = "nvidia,tegra20-vde";
>>>> + reg = <0x6001a000 0x3D00 /* VDE registers */
>>>> + 0x40000400 0x3FC00>; /* IRAM region */
>>>
>>> this notation of a used region in IRAM is non-standard and potentially it
>>> may lead to conflicts for IRAM resource between users.
>>>
>>> My proposal is to add a valid device tree node to describe an IRAM region
>>> firstly, then reserve a subregion in it by using a new "iram" property.
>>>
>>
>> The defined in DT IRAM region used by VDE isn't exactly correct, actually it
>> should be much smaller. I don't know exactly what parts of IRAM VDE uses, for
>> now it is just safer to assign the rest of the IRAM region to VDE.
>>
>> I'm not sure whether it really worthy to use a dynamic allocator for a single
>> static allocation, but maybe it would come handy later.. Stephen / Jon /
>> Thierry, what do you think?
>
> This sounds like a good idea. I agree that this currently doesn't seem
> to be warranted, but consider what would happen if at some point we have
> more devices requiring access to the IRAM. Spreading individual reg
> properties all across the DT will make it very difficult to ensure they
> don't overlap.
>
> Presumably the mmio-sram driver will check that pool don't overlap. Or
> even if it doesn't it will make it a lot easier to verify because it's
> all in the same DT node and then consumers only reference it.
>
> I like Vladimir's proposal. I also suspect that Rob may want us to stick
> to a standardized way referencing such external memory.
FWIW I agree. Seems like a nice approach and describes the h/w accurately.
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
@ 2017-10-12 13:45 ` Jon Hunter
0 siblings, 0 replies; 34+ messages in thread
From: Jon Hunter @ 2017-10-12 13:45 UTC (permalink / raw)
To: Thierry Reding, Dmitry Osipenko
Cc: Vladimir Zapolskiy, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren, Dan Carpenter,
linux-media, linux-tegra, devel, devicetree, linux-kernel
On 12/10/17 14:25, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Thu, Oct 12, 2017 at 03:06:17PM +0300, Dmitry Osipenko wrote:
>> Hello Vladimir,
>>
>> On 12.10.2017 10:43, Vladimir Zapolskiy wrote:
>>> Hello Dmitry,
>>>
>>> On 10/11/2017 11:08 PM, Dmitry Osipenko wrote:
>>>> Add a device node for the video decoder engine found on Tegra20.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
>>>> 1 file changed, 17 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>>> index 7c85f97f72ea..1b5d54b6c0cb 100644
>>>> --- a/arch/arm/boot/dts/tegra20.dtsi
>>>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>>>> @@ -249,6 +249,23 @@
>>>> */
>>>> };
>>>>
>>>> + vde@6001a000 {
>>>> + compatible = "nvidia,tegra20-vde";
>>>> + reg = <0x6001a000 0x3D00 /* VDE registers */
>>>> + 0x40000400 0x3FC00>; /* IRAM region */
>>>
>>> this notation of a used region in IRAM is non-standard and potentially it
>>> may lead to conflicts for IRAM resource between users.
>>>
>>> My proposal is to add a valid device tree node to describe an IRAM region
>>> firstly, then reserve a subregion in it by using a new "iram" property.
>>>
>>
>> The defined in DT IRAM region used by VDE isn't exactly correct, actually it
>> should be much smaller. I don't know exactly what parts of IRAM VDE uses, for
>> now it is just safer to assign the rest of the IRAM region to VDE.
>>
>> I'm not sure whether it really worthy to use a dynamic allocator for a single
>> static allocation, but maybe it would come handy later.. Stephen / Jon /
>> Thierry, what do you think?
>
> This sounds like a good idea. I agree that this currently doesn't seem
> to be warranted, but consider what would happen if at some point we have
> more devices requiring access to the IRAM. Spreading individual reg
> properties all across the DT will make it very difficult to ensure they
> don't overlap.
>
> Presumably the mmio-sram driver will check that pool don't overlap. Or
> even if it doesn't it will make it a lot easier to verify because it's
> all in the same DT node and then consumers only reference it.
>
> I like Vladimir's proposal. I also suspect that Rob may want us to stick
> to a standardized way referencing such external memory.
FWIW I agree. Seems like a nice approach and describes the h/w accurately.
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
2017-10-12 13:45 ` Jon Hunter
@ 2017-10-12 15:43 ` Dmitry Osipenko
-1 siblings, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-12 15:43 UTC (permalink / raw)
To: Jon Hunter, Thierry Reding
Cc: devel, devicetree, Stephen Warren, Greg Kroah-Hartman,
linux-kernel, Rob Herring, Vladimir Zapolskiy, linux-tegra,
Mauro Carvalho Chehab, Dan Carpenter, linux-media
On 12.10.2017 16:45, Jon Hunter wrote:
>
> On 12/10/17 14:25, Thierry Reding wrote:
>> * PGP Signed by an unknown key
>>
>> On Thu, Oct 12, 2017 at 03:06:17PM +0300, Dmitry Osipenko wrote:
>>> Hello Vladimir,
>>>
>>> On 12.10.2017 10:43, Vladimir Zapolskiy wrote:
>>>> Hello Dmitry,
>>>>
>>>> On 10/11/2017 11:08 PM, Dmitry Osipenko wrote:
>>>>> Add a device node for the video decoder engine found on Tegra20.
>>>>>
>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>> ---
>>>>> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
>>>>> 1 file changed, 17 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>>>> index 7c85f97f72ea..1b5d54b6c0cb 100644
>>>>> --- a/arch/arm/boot/dts/tegra20.dtsi
>>>>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>>>>> @@ -249,6 +249,23 @@
>>>>> */
>>>>> };
>>>>>
>>>>> + vde@6001a000 {
>>>>> + compatible = "nvidia,tegra20-vde";
>>>>> + reg = <0x6001a000 0x3D00 /* VDE registers */
>>>>> + 0x40000400 0x3FC00>; /* IRAM region */
>>>>
>>>> this notation of a used region in IRAM is non-standard and potentially it
>>>> may lead to conflicts for IRAM resource between users.
>>>>
>>>> My proposal is to add a valid device tree node to describe an IRAM region
>>>> firstly, then reserve a subregion in it by using a new "iram" property.
>>>>
>>>
>>> The defined in DT IRAM region used by VDE isn't exactly correct, actually it
>>> should be much smaller. I don't know exactly what parts of IRAM VDE uses, for
>>> now it is just safer to assign the rest of the IRAM region to VDE.
>>>
>>> I'm not sure whether it really worthy to use a dynamic allocator for a single
>>> static allocation, but maybe it would come handy later.. Stephen / Jon /
>>> Thierry, what do you think?
>>
>> This sounds like a good idea. I agree that this currently doesn't seem
>> to be warranted, but consider what would happen if at some point we have
>> more devices requiring access to the IRAM. Spreading individual reg
>> properties all across the DT will make it very difficult to ensure they
>> don't overlap.
>>
>> Presumably the mmio-sram driver will check that pool don't overlap. Or
>> even if it doesn't it will make it a lot easier to verify because it's
>> all in the same DT node and then consumers only reference it.
>>
>> I like Vladimir's proposal. I also suspect that Rob may want us to stick
>> to a standardized way referencing such external memory.
>
> FWIW I agree. Seems like a nice approach and describes the h/w accurately.
>
Alright :)
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node
@ 2017-10-12 15:43 ` Dmitry Osipenko
0 siblings, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-12 15:43 UTC (permalink / raw)
To: Jon Hunter, Thierry Reding
Cc: Vladimir Zapolskiy, Greg Kroah-Hartman, Rob Herring,
Mauro Carvalho Chehab, Stephen Warren, Dan Carpenter,
linux-media, linux-tegra, devel, devicetree, linux-kernel
On 12.10.2017 16:45, Jon Hunter wrote:
>
> On 12/10/17 14:25, Thierry Reding wrote:
>> * PGP Signed by an unknown key
>>
>> On Thu, Oct 12, 2017 at 03:06:17PM +0300, Dmitry Osipenko wrote:
>>> Hello Vladimir,
>>>
>>> On 12.10.2017 10:43, Vladimir Zapolskiy wrote:
>>>> Hello Dmitry,
>>>>
>>>> On 10/11/2017 11:08 PM, Dmitry Osipenko wrote:
>>>>> Add a device node for the video decoder engine found on Tegra20.
>>>>>
>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>> ---
>>>>> arch/arm/boot/dts/tegra20.dtsi | 17 +++++++++++++++++
>>>>> 1 file changed, 17 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>>>> index 7c85f97f72ea..1b5d54b6c0cb 100644
>>>>> --- a/arch/arm/boot/dts/tegra20.dtsi
>>>>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>>>>> @@ -249,6 +249,23 @@
>>>>> */
>>>>> };
>>>>>
>>>>> + vde@6001a000 {
>>>>> + compatible = "nvidia,tegra20-vde";
>>>>> + reg = <0x6001a000 0x3D00 /* VDE registers */
>>>>> + 0x40000400 0x3FC00>; /* IRAM region */
>>>>
>>>> this notation of a used region in IRAM is non-standard and potentially it
>>>> may lead to conflicts for IRAM resource between users.
>>>>
>>>> My proposal is to add a valid device tree node to describe an IRAM region
>>>> firstly, then reserve a subregion in it by using a new "iram" property.
>>>>
>>>
>>> The defined in DT IRAM region used by VDE isn't exactly correct, actually it
>>> should be much smaller. I don't know exactly what parts of IRAM VDE uses, for
>>> now it is just safer to assign the rest of the IRAM region to VDE.
>>>
>>> I'm not sure whether it really worthy to use a dynamic allocator for a single
>>> static allocation, but maybe it would come handy later.. Stephen / Jon /
>>> Thierry, what do you think?
>>
>> This sounds like a good idea. I agree that this currently doesn't seem
>> to be warranted, but consider what would happen if at some point we have
>> more devices requiring access to the IRAM. Spreading individual reg
>> properties all across the DT will make it very difficult to ensure they
>> don't overlap.
>>
>> Presumably the mmio-sram driver will check that pool don't overlap. Or
>> even if it doesn't it will make it a lot easier to verify because it's
>> all in the same DT node and then consumers only reference it.
>>
>> I like Vladimir's proposal. I also suspect that Rob may want us to stick
>> to a standardized way referencing such external memory.
>
> FWIW I agree. Seems like a nice approach and describes the h/w accurately.
>
Alright :)
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
2017-10-11 20:08 ` Dmitry Osipenko
@ 2017-10-17 20:13 ` Rob Herring
-1 siblings, 0 replies; 34+ messages in thread
From: Rob Herring @ 2017-10-17 20:13 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Thierry Reding, Jonathan Hunter, Greg Kroah-Hartman,
Mauro Carvalho Chehab, Stephen Warren, Dan Carpenter,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Wed, Oct 11, 2017 at 11:08:11PM +0300, Dmitry Osipenko wrote:
> 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.
>
> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> .../bindings/arm/tegra/nvidia,tegra20-vde.txt | 44 +
It's preferred to split bindings to a separate patch. Also, this doesn't
belong under bindings/arm/, but rather bindings/media/.
> drivers/staging/Kconfig | 2 +
> drivers/staging/Makefile | 1 +
> drivers/staging/tegra-vde/Kconfig | 6 +
> drivers/staging/tegra-vde/Makefile | 1 +
> drivers/staging/tegra-vde/TODO | 5 +
> drivers/staging/tegra-vde/uapi.h | 101 ++
> drivers/staging/tegra-vde/vde.c | 1109 ++++++++++++++++++++
> 8 files changed, 1269 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
> create mode 100644 drivers/staging/tegra-vde/Kconfig
> create mode 100644 drivers/staging/tegra-vde/Makefile
> create mode 100644 drivers/staging/tegra-vde/TODO
> create mode 100644 drivers/staging/tegra-vde/uapi.h
> create mode 100644 drivers/staging/tegra-vde/vde.c
>
> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
> new file mode 100644
> index 000000000000..c3f847db8167
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
> @@ -0,0 +1,44 @@
> +NVIDIA Tegra Video Decoder Engine
> +
> +Required properties:
> +- compatible : "nvidia,tegra20-vde"
> +- reg : Must contain 2 register ranges: registers and IRAM region that
> + VDE uses for its internal needs and for passing some of decoding
> + parameters.
Is the IRAM shared with other things. If so, use mmio-sram binding and
define the codec's region.
> +- reg-names : Must include the following entries:
> + - regs
> + - iram
> +- interrupts : Must contain an entry for each entry in interrupt-names.
> +- interrupt-names : Must include the following entries:
> + - ucq-error
> + - sync-token
> + - bsev
> + - bsea
> + - sxe
> +- clocks : Must contain an entry for each entry in clock-names.
> + See ../clocks/clock-bindings.txt for details.
> +- clock-names : Must include the following entries:
> + - vde
> +- resets : Must contain an entry for each entry in reset-names.
> + See ../reset/reset.txt for details.
> +- reset-names : Must include the following entries:
> + - vde
-names is pointless when there is only one.
> +
> +Example:
> +
> +vde@6001a000 {
video-codec@...
> + compatible = "nvidia,tegra20-vde";
> + reg = <0x6001a000 0x3D00 /* VDE registers */
> + 0x40000400 0x3FC00>; /* IRAM region */
Lower case hex please.
> + reg-names = "regs", "iram";
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, /* UCQ error interrupt */
> + <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */
> + <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */
> + <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, /* BSE-A interrupt */
> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
> + interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
> + clocks = <&tegra_car TEGRA20_CLK_VDE>;
> + clock-names = "vde";
> + resets = <&tegra_car 61>;
> + reset-names = "vde";
> +};
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
@ 2017-10-17 20:13 ` Rob Herring
0 siblings, 0 replies; 34+ messages in thread
From: Rob Herring @ 2017-10-17 20:13 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Thierry Reding, Jonathan Hunter, Greg Kroah-Hartman,
Mauro Carvalho Chehab, Stephen Warren, Dan Carpenter,
linux-media, linux-tegra, devel, devicetree, linux-kernel
On Wed, Oct 11, 2017 at 11:08:11PM +0300, Dmitry Osipenko wrote:
> 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.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> .../bindings/arm/tegra/nvidia,tegra20-vde.txt | 44 +
It's preferred to split bindings to a separate patch. Also, this doesn't
belong under bindings/arm/, but rather bindings/media/.
> drivers/staging/Kconfig | 2 +
> drivers/staging/Makefile | 1 +
> drivers/staging/tegra-vde/Kconfig | 6 +
> drivers/staging/tegra-vde/Makefile | 1 +
> drivers/staging/tegra-vde/TODO | 5 +
> drivers/staging/tegra-vde/uapi.h | 101 ++
> drivers/staging/tegra-vde/vde.c | 1109 ++++++++++++++++++++
> 8 files changed, 1269 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
> create mode 100644 drivers/staging/tegra-vde/Kconfig
> create mode 100644 drivers/staging/tegra-vde/Makefile
> create mode 100644 drivers/staging/tegra-vde/TODO
> create mode 100644 drivers/staging/tegra-vde/uapi.h
> create mode 100644 drivers/staging/tegra-vde/vde.c
>
> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
> new file mode 100644
> index 000000000000..c3f847db8167
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
> @@ -0,0 +1,44 @@
> +NVIDIA Tegra Video Decoder Engine
> +
> +Required properties:
> +- compatible : "nvidia,tegra20-vde"
> +- reg : Must contain 2 register ranges: registers and IRAM region that
> + VDE uses for its internal needs and for passing some of decoding
> + parameters.
Is the IRAM shared with other things. If so, use mmio-sram binding and
define the codec's region.
> +- reg-names : Must include the following entries:
> + - regs
> + - iram
> +- interrupts : Must contain an entry for each entry in interrupt-names.
> +- interrupt-names : Must include the following entries:
> + - ucq-error
> + - sync-token
> + - bsev
> + - bsea
> + - sxe
> +- clocks : Must contain an entry for each entry in clock-names.
> + See ../clocks/clock-bindings.txt for details.
> +- clock-names : Must include the following entries:
> + - vde
> +- resets : Must contain an entry for each entry in reset-names.
> + See ../reset/reset.txt for details.
> +- reset-names : Must include the following entries:
> + - vde
-names is pointless when there is only one.
> +
> +Example:
> +
> +vde@6001a000 {
video-codec@...
> + compatible = "nvidia,tegra20-vde";
> + reg = <0x6001a000 0x3D00 /* VDE registers */
> + 0x40000400 0x3FC00>; /* IRAM region */
Lower case hex please.
> + reg-names = "regs", "iram";
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, /* UCQ error interrupt */
> + <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */
> + <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */
> + <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, /* BSE-A interrupt */
> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */
> + interrupt-names = "ucq-error", "sync-token", "bsev", "bsea", "sxe";
> + clocks = <&tegra_car TEGRA20_CLK_VDE>;
> + clock-names = "vde";
> + resets = <&tegra_car 61>;
> + reset-names = "vde";
> +};
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
2017-10-17 20:13 ` Rob Herring
@ 2017-10-17 20:24 ` Thierry Reding
-1 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2017-10-17 20:24 UTC (permalink / raw)
To: Rob Herring
Cc: Dmitry Osipenko, Jonathan Hunter, Greg Kroah-Hartman,
Mauro Carvalho Chehab, Stephen Warren, Dan Carpenter,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 817 bytes --]
On Tue, Oct 17, 2017 at 03:13:54PM -0500, Rob Herring wrote:
[...]
> > diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
[...]
> > +- resets : Must contain an entry for each entry in reset-names.
> > + See ../reset/reset.txt for details.
> > +- reset-names : Must include the following entries:
> > + - vde
>
> -names is pointless when there is only one.
I'd prefer to keep it. In the past we occasionally had to add clocks or
resets to a device tree node where only one had been present (and hence
no -names property) and that caused some awkwardness because verbiage
had to be added to the bindings that clarified that one particular entry
(the original one) always had to come first.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
@ 2017-10-17 20:24 ` Thierry Reding
0 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2017-10-17 20:24 UTC (permalink / raw)
To: Rob Herring
Cc: Dmitry Osipenko, Jonathan Hunter, Greg Kroah-Hartman,
Mauro Carvalho Chehab, Stephen Warren, Dan Carpenter,
linux-media, linux-tegra, devel, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 817 bytes --]
On Tue, Oct 17, 2017 at 03:13:54PM -0500, Rob Herring wrote:
[...]
> > diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
[...]
> > +- resets : Must contain an entry for each entry in reset-names.
> > + See ../reset/reset.txt for details.
> > +- reset-names : Must include the following entries:
> > + - vde
>
> -names is pointless when there is only one.
I'd prefer to keep it. In the past we occasionally had to add clocks or
resets to a device tree node where only one had been present (and hence
no -names property) and that caused some awkwardness because verbiage
had to be added to the bindings that clarified that one particular entry
(the original one) always had to come first.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
2017-10-17 20:24 ` Thierry Reding
(?)
@ 2017-10-17 21:13 ` Rob Herring
[not found] ` <CAL_JsqKtFD9OgO2privwfBAZeCn3zg1JienmefPk=X2W-+ahJQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
-1 siblings, 1 reply; 34+ messages in thread
From: Rob Herring @ 2017-10-17 21:13 UTC (permalink / raw)
To: Thierry Reding
Cc: Dmitry Osipenko, Jonathan Hunter, Greg Kroah-Hartman,
Mauro Carvalho Chehab, Stephen Warren, Dan Carpenter,
linux-media, linux-tegra, devel, devicetree, linux-kernel
On Tue, Oct 17, 2017 at 3:24 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Tue, Oct 17, 2017 at 03:13:54PM -0500, Rob Herring wrote:
> [...]
>> > diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
> [...]
>> > +- resets : Must contain an entry for each entry in reset-names.
>> > + See ../reset/reset.txt for details.
>> > +- reset-names : Must include the following entries:
>> > + - vde
>>
>> -names is pointless when there is only one.
>
> I'd prefer to keep it. In the past we occasionally had to add clocks or
> resets to a device tree node where only one had been present (and hence
> no -names property) and that caused some awkwardness because verbiage
> had to be added to the bindings that clarified that one particular entry
> (the original one) always had to come first.
The order should be specified regardless of -names and the original
one has to come first if you add any. That's not awkwardness, but how
bindings work.
Rob
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
2017-10-17 21:13 ` Rob Herring
@ 2017-10-17 21:26 ` Dmitry Osipenko
0 siblings, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-17 21:26 UTC (permalink / raw)
To: Rob Herring, Thierry Reding
Cc: Jonathan Hunter, Greg Kroah-Hartman, Mauro Carvalho Chehab,
Stephen Warren, Dan Carpenter,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On 18.10.2017 00:13, Rob Herring wrote:
> On Tue, Oct 17, 2017 at 3:24 PM, Thierry Reding
> <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Tue, Oct 17, 2017 at 03:13:54PM -0500, Rob Herring wrote:
>> [...]
>>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
>> [...]
>>>> +- resets : Must contain an entry for each entry in reset-names.
>>>> + See ../reset/reset.txt for details.
>>>> +- reset-names : Must include the following entries:
>>>> + - vde
>>>
>>> -names is pointless when there is only one.
>>
>> I'd prefer to keep it. In the past we occasionally had to add clocks or
>> resets to a device tree node where only one had been present (and hence
>> no -names property) and that caused some awkwardness because verbiage
>> had to be added to the bindings that clarified that one particular entry
>> (the original one) always had to come first.
>
> The order should be specified regardless of -names and the original
> one has to come first if you add any. That's not awkwardness, but how
> bindings work.
Probably it would be okay to remove '-names' from the binding doc, but keep them
in the actual DT, wouldn't it?
--
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
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
@ 2017-10-17 21:26 ` Dmitry Osipenko
0 siblings, 0 replies; 34+ messages in thread
From: Dmitry Osipenko @ 2017-10-17 21:26 UTC (permalink / raw)
To: Rob Herring, Thierry Reding
Cc: Jonathan Hunter, Greg Kroah-Hartman, Mauro Carvalho Chehab,
Stephen Warren, Dan Carpenter, linux-media, linux-tegra, devel,
devicetree, linux-kernel
On 18.10.2017 00:13, Rob Herring wrote:
> On Tue, Oct 17, 2017 at 3:24 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
>> On Tue, Oct 17, 2017 at 03:13:54PM -0500, Rob Herring wrote:
>> [...]
>>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
>> [...]
>>>> +- resets : Must contain an entry for each entry in reset-names.
>>>> + See ../reset/reset.txt for details.
>>>> +- reset-names : Must include the following entries:
>>>> + - vde
>>>
>>> -names is pointless when there is only one.
>>
>> I'd prefer to keep it. In the past we occasionally had to add clocks or
>> resets to a device tree node where only one had been present (and hence
>> no -names property) and that caused some awkwardness because verbiage
>> had to be added to the bindings that clarified that one particular entry
>> (the original one) always had to come first.
>
> The order should be specified regardless of -names and the original
> one has to come first if you add any. That's not awkwardness, but how
> bindings work.
Probably it would be okay to remove '-names' from the binding doc, but keep them
in the actual DT, wouldn't it?
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
2017-10-17 21:26 ` Dmitry Osipenko
@ 2017-10-19 20:52 ` Rob Herring
-1 siblings, 0 replies; 34+ messages in thread
From: Rob Herring @ 2017-10-19 20:52 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: devel, devicetree, Stephen Warren, Greg Kroah-Hartman,
linux-kernel, Jonathan Hunter, Thierry Reding, linux-tegra,
Mauro Carvalho Chehab, Dan Carpenter, linux-media
On Tue, Oct 17, 2017 at 4:26 PM, Dmitry Osipenko <digetx@gmail.com> wrote:
> On 18.10.2017 00:13, Rob Herring wrote:
>> On Tue, Oct 17, 2017 at 3:24 PM, Thierry Reding
>> <thierry.reding@gmail.com> wrote:
>>> On Tue, Oct 17, 2017 at 03:13:54PM -0500, Rob Herring wrote:
>>> [...]
>>>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
>>> [...]
>>>>> +- resets : Must contain an entry for each entry in reset-names.
>>>>> + See ../reset/reset.txt for details.
>>>>> +- reset-names : Must include the following entries:
>>>>> + - vde
>>>>
>>>> -names is pointless when there is only one.
>>>
>>> I'd prefer to keep it. In the past we occasionally had to add clocks or
>>> resets to a device tree node where only one had been present (and hence
>>> no -names property) and that caused some awkwardness because verbiage
>>> had to be added to the bindings that clarified that one particular entry
>>> (the original one) always had to come first.
>>
>> The order should be specified regardless of -names and the original
>> one has to come first if you add any. That's not awkwardness, but how
>> bindings work.
>
> Probably it would be okay to remove '-names' from the binding doc, but keep them
> in the actual DT, wouldn't it?
No. Then where are the names documented?
Rob
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/2] staging: Introduce NVIDIA Tegra20 video decoder driver
@ 2017-10-19 20:52 ` Rob Herring
0 siblings, 0 replies; 34+ messages in thread
From: Rob Herring @ 2017-10-19 20:52 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Thierry Reding, Jonathan Hunter, Greg Kroah-Hartman,
Mauro Carvalho Chehab, Stephen Warren, Dan Carpenter,
linux-media, linux-tegra, devel, devicetree, linux-kernel
On Tue, Oct 17, 2017 at 4:26 PM, Dmitry Osipenko <digetx@gmail.com> wrote:
> On 18.10.2017 00:13, Rob Herring wrote:
>> On Tue, Oct 17, 2017 at 3:24 PM, Thierry Reding
>> <thierry.reding@gmail.com> wrote:
>>> On Tue, Oct 17, 2017 at 03:13:54PM -0500, Rob Herring wrote:
>>> [...]
>>>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-vde.txt
>>> [...]
>>>>> +- resets : Must contain an entry for each entry in reset-names.
>>>>> + See ../reset/reset.txt for details.
>>>>> +- reset-names : Must include the following entries:
>>>>> + - vde
>>>>
>>>> -names is pointless when there is only one.
>>>
>>> I'd prefer to keep it. In the past we occasionally had to add clocks or
>>> resets to a device tree node where only one had been present (and hence
>>> no -names property) and that caused some awkwardness because verbiage
>>> had to be added to the bindings that clarified that one particular entry
>>> (the original one) always had to come first.
>>
>> The order should be specified regardless of -names and the original
>> one has to come first if you add any. That's not awkwardness, but how
>> bindings work.
>
> Probably it would be okay to remove '-names' from the binding doc, but keep them
> in the actual DT, wouldn't it?
No. Then where are the names documented?
Rob
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2017-10-19 20:53 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-11 20:08 [PATCH v3 0/2] NVIDIA Tegra20 video decoder driver Dmitry Osipenko
2017-10-11 20:08 ` [PATCH v3 1/2] staging: Introduce " Dmitry Osipenko
2017-10-11 20:08 ` Dmitry Osipenko
[not found] ` <3d432aa2617977a2b0a8621a1fc2f36f751133e1.1507752381.git.digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-10-11 20:47 ` Nicolas Dufresne
2017-10-11 20:47 ` Nicolas Dufresne
[not found] ` <1507754838.19342.11.camel-dDhyB4GVkw9AFePFGvp55w@public.gmane.org>
2017-10-11 22:37 ` Dmitry Osipenko
2017-10-11 22:37 ` Dmitry Osipenko
2017-10-17 20:13 ` Rob Herring
2017-10-17 20:13 ` Rob Herring
2017-10-17 20:24 ` Thierry Reding
2017-10-17 20:24 ` Thierry Reding
2017-10-17 21:13 ` Rob Herring
[not found] ` <CAL_JsqKtFD9OgO2privwfBAZeCn3zg1JienmefPk=X2W-+ahJQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-10-17 21:26 ` Dmitry Osipenko
2017-10-17 21:26 ` Dmitry Osipenko
2017-10-19 20:52 ` Rob Herring
2017-10-19 20:52 ` Rob Herring
2017-10-11 20:08 ` [PATCH v3 2/2] ARM: dts: tegra20: Add video decoder node Dmitry Osipenko
2017-10-12 7:43 ` Vladimir Zapolskiy
2017-10-12 7:43 ` Vladimir Zapolskiy
[not found] ` <0b6150a7-5b2b-ca4d-eb34-b6614e4833df-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
2017-10-12 12:06 ` Dmitry Osipenko
2017-10-12 12:06 ` Dmitry Osipenko
2017-10-12 13:25 ` Thierry Reding
2017-10-12 13:25 ` Thierry Reding
2017-10-12 13:45 ` Jon Hunter
2017-10-12 13:45 ` Jon Hunter
2017-10-12 15:43 ` Dmitry Osipenko
2017-10-12 15:43 ` Dmitry Osipenko
2017-10-12 8:49 ` Jon Hunter
2017-10-12 8:49 ` Jon Hunter
[not found] ` <f18b6a72-e255-9aa4-6ebd-852ce1a27a4e-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2017-10-12 10:51 ` Dmitry Osipenko
2017-10-12 10:51 ` Dmitry Osipenko
2017-10-12 10:57 ` Jon Hunter
2017-10-12 10:57 ` Jon Hunter
2017-10-12 11:11 ` Dmitry Osipenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.