[PATCH v1 0/5] Tegra Video Decoder patches for 4.17

public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH v1 0/5] Tegra Video Decoder patches for 4.17
@ 2018-03-17 15:28 Dmitry Osipenko
  2018-03-17 15:28 ` [PATCH v1 1/5] media: staging: tegra-vde: Align bitstream size to 16K Dmitry Osipenko
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2018-03-17 15:28 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Greg Kroah-Hartman, linux-media, devel, linux-tegra, linux-kernel

Hello media maintainers,

I've been postponing sending out these patches for awhile because I was
waiting for a review for the Tegra memory controller patches that would
allow to reset VDE HW properly and was hoping that they will get into 4.17,
but it's getting quite late now and seems 4.18 is the best bet now for the
proper VDE reset. So here is a small patchset that addresses couple of
minor issues that I've spotted over time.

Dmitry Osipenko (5):
  media: staging: tegra-vde: Align bitstream size to 16K
  media: staging: tegra-vde: Silence some of checkpatch warnings
  media: staging: tegra-vde: Correct minimum size of U/V planes
  media: staging: tegra-vde: Do not handle spurious interrupts
  media: staging: tegra-vde: Correct included header

 drivers/staging/media/tegra-vde/tegra-vde.c | 63 ++++++++++++++++-------------
 1 file changed, 34 insertions(+), 29 deletions(-)

-- 
2.16.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v1 1/5] media: staging: tegra-vde: Align bitstream size to 16K
  2018-03-17 15:28 [PATCH v1 0/5] Tegra Video Decoder patches for 4.17 Dmitry Osipenko
@ 2018-03-17 15:28 ` Dmitry Osipenko
  2018-03-17 15:28 ` [PATCH v1 2/5] media: staging: tegra-vde: Silence some of checkpatch warnings Dmitry Osipenko
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2018-03-17 15:28 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Greg Kroah-Hartman, linux-media, devel, linux-tegra, linux-kernel

I've noticed that decoding fails sometime if size of bitstream buffer
isn't aligned to 16K, probably because HW fetches data from memory in
a 16K granularity and if the last chunk of data isn't aligned, HW reads
garbage data beyond the dmabuf and tries to parse it.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 40 ++++++++++++-----------------
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c b/drivers/staging/media/tegra-vde/tegra-vde.c
index c47659e96089..c2ff2071b23c 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -440,7 +440,7 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
 	VDE_WR(value, vde->sxe + 0x4C);
 
 	value = 0x03800000;
-	value |= min_t(size_t, bitstream_data_size, SZ_1M);
+	value |= bitstream_data_size & GENMASK(19, 15);
 
 	VDE_WR(value, vde->sxe + 0x68);
 
@@ -522,7 +522,8 @@ static void tegra_vde_detach_and_put_dmabuf(struct dma_buf_attachment *a,
 static int tegra_vde_attach_dmabuf(struct device *dev,
 				   int fd,
 				   unsigned long offset,
-				   unsigned int min_size,
+				   size_t min_size,
+				   size_t align_size,
 				   struct dma_buf_attachment **a,
 				   dma_addr_t *addr,
 				   struct sg_table **s,
@@ -540,9 +541,16 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
 		return PTR_ERR(dmabuf);
 	}
 
+	if (dmabuf->size & (align_size - 1)) {
+		dev_err(dev, "Unaligned dmabuf 0x%zX, "
+			     "should be aligned to 0x%zX\n",
+			dmabuf->size, align_size);
+		return -EINVAL;
+	}
+
 	if ((u64)offset + min_size > dmabuf->size) {
 		dev_err(dev, "Too small dmabuf size %zu @0x%lX, "
-			     "should be at least %d\n",
+			     "should be at least %zu\n",
 			dmabuf->size, offset, min_size);
 		return -EINVAL;
 	}
@@ -596,7 +604,7 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
 	int err;
 
 	err = tegra_vde_attach_dmabuf(dev, src->y_fd,
-				      src->y_offset, csize * 4,
+				      src->y_offset, csize * 4, SZ_256,
 				      &frame->y_dmabuf_attachment,
 				      &frame->y_addr,
 				      &frame->y_sgt,
@@ -605,7 +613,7 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
 		return err;
 
 	err = tegra_vde_attach_dmabuf(dev, src->cb_fd,
-				      src->cb_offset, csize,
+				      src->cb_offset, csize, SZ_256,
 				      &frame->cb_dmabuf_attachment,
 				      &frame->cb_addr,
 				      &frame->cb_sgt,
@@ -614,7 +622,7 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
 		goto err_release_y;
 
 	err = tegra_vde_attach_dmabuf(dev, src->cr_fd,
-				      src->cr_offset, csize,
+				      src->cr_offset, csize, SZ_256,
 				      &frame->cr_dmabuf_attachment,
 				      &frame->cr_addr,
 				      &frame->cr_sgt,
@@ -628,7 +636,7 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
 	}
 
 	err = tegra_vde_attach_dmabuf(dev, src->aux_fd,
-				      src->aux_offset, csize,
+				      src->aux_offset, csize, SZ_256,
 				      &frame->aux_dmabuf_attachment,
 				      &frame->aux_addr,
 				      &frame->aux_sgt,
@@ -677,21 +685,6 @@ static int tegra_vde_validate_frame(struct device *dev,
 		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;
 }
 
@@ -792,7 +785,8 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 		return ret;
 
 	ret = tegra_vde_attach_dmabuf(dev, ctx.bitstream_data_fd,
-				      ctx.bitstream_data_offset, 0,
+				      ctx.bitstream_data_offset,
+				      SZ_16K, SZ_16K,
 				      &bitstream_data_dmabuf_attachment,
 				      &bitstream_data_addr,
 				      &bitstream_sgt,
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v1 2/5] media: staging: tegra-vde: Silence some of checkpatch warnings
  2018-03-17 15:28 [PATCH v1 0/5] Tegra Video Decoder patches for 4.17 Dmitry Osipenko
  2018-03-17 15:28 ` [PATCH v1 1/5] media: staging: tegra-vde: Align bitstream size to 16K Dmitry Osipenko
@ 2018-03-17 15:28 ` Dmitry Osipenko
  2018-03-17 15:28 ` [PATCH v1 3/5] media: staging: tegra-vde: Correct minimum size of U/V planes Dmitry Osipenko
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2018-03-17 15:28 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Greg Kroah-Hartman, linux-media, devel, linux-tegra, linux-kernel

Make all strings single line to make them grep'able and add a comment
to the memory barrier.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c b/drivers/staging/media/tegra-vde/tegra-vde.c
index c2ff2071b23c..14899c887d58 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -368,6 +368,11 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
 	tegra_vde_setup_iram_tables(vde, dpb_frames,
 				    ctx->dpb_frames_nb - 1,
 				    ctx->dpb_ref_frames_with_earlier_poc_nb);
+
+	/*
+	 * The IRAM mapping is write-combine, ensure that CPU buffers have
+	 * been flushed at this point.
+	 */
 	wmb();
 
 	VDE_WR(0x00000000, vde->bsev + 0x8C);
@@ -542,15 +547,13 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
 	}
 
 	if (dmabuf->size & (align_size - 1)) {
-		dev_err(dev, "Unaligned dmabuf 0x%zX, "
-			     "should be aligned to 0x%zX\n",
+		dev_err(dev, "Unaligned dmabuf 0x%zX, should be aligned to 0x%zX\n",
 			dmabuf->size, align_size);
 		return -EINVAL;
 	}
 
 	if ((u64)offset + min_size > dmabuf->size) {
-		dev_err(dev, "Too small dmabuf size %zu @0x%lX, "
-			     "should be at least %zu\n",
+		dev_err(dev, "Too small dmabuf size %zu @0x%lX, should be at least %zu\n",
 			dmabuf->size, offset, min_size);
 		return -EINVAL;
 	}
@@ -863,8 +866,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 		macroblocks_nb = readl_relaxed(vde->sxe + 0xC8) & 0x1FFF;
 		read_bytes = bsev_ptr ? bsev_ptr - bitstream_data_addr : 0;
 
-		dev_err(dev, "Decoding failed: "
-				"read 0x%X bytes, %u macroblocks parsed\n",
+		dev_err(dev, "Decoding failed: read 0x%X bytes, %u macroblocks parsed\n",
 			read_bytes, macroblocks_nb);
 
 		ret = -EIO;
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v1 3/5] media: staging: tegra-vde: Correct minimum size of U/V planes
  2018-03-17 15:28 [PATCH v1 0/5] Tegra Video Decoder patches for 4.17 Dmitry Osipenko
  2018-03-17 15:28 ` [PATCH v1 1/5] media: staging: tegra-vde: Align bitstream size to 16K Dmitry Osipenko
  2018-03-17 15:28 ` [PATCH v1 2/5] media: staging: tegra-vde: Silence some of checkpatch warnings Dmitry Osipenko
@ 2018-03-17 15:28 ` Dmitry Osipenko
  2018-03-17 15:28 ` [PATCH v1 4/5] media: staging: tegra-vde: Do not handle spurious interrupts Dmitry Osipenko
  2018-03-17 15:28 ` [PATCH v1 5/5] media: staging: tegra-vde: Correct included header Dmitry Osipenko
  4 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2018-03-17 15:28 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Greg Kroah-Hartman, linux-media, devel, linux-tegra, linux-kernel

Stride of U/V planes must be aligned to 16 bytes (2 macroblocks). This
needs to be taken into account, otherwise it is possible to get a silent
memory corruption if dmabuf size is less than the size of decoded video
frame.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c b/drivers/staging/media/tegra-vde/tegra-vde.c
index 14899c887d58..94b4db55cdb5 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -602,12 +602,12 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
 					     struct tegra_vde_h264_frame *src,
 					     enum dma_data_direction dma_dir,
 					     bool baseline_profile,
-					     size_t csize)
+					     size_t lsize, size_t csize)
 {
 	int err;
 
 	err = tegra_vde_attach_dmabuf(dev, src->y_fd,
-				      src->y_offset, csize * 4, SZ_256,
+				      src->y_offset, lsize, SZ_256,
 				      &frame->y_dmabuf_attachment,
 				      &frame->y_addr,
 				      &frame->y_sgt,
@@ -773,9 +773,11 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 	enum dma_data_direction dma_dir;
 	dma_addr_t bitstream_data_addr;
 	dma_addr_t bsev_ptr;
+	size_t lsize, csize;
 	size_t bitstream_data_size;
 	unsigned int macroblocks_nb;
 	unsigned int read_bytes;
+	unsigned int cstride;
 	unsigned int i;
 	long timeout;
 	int ret, err;
@@ -814,6 +816,10 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 		goto free_dpb_frames;
 	}
 
+	cstride = ALIGN(ctx.pic_width_in_mbs * 8, 16);
+	csize = cstride * ctx.pic_height_in_mbs * 8;
+	lsize = macroblocks_nb * 256;
+
 	for (i = 0; i < ctx.dpb_frames_nb; i++) {
 		ret = tegra_vde_validate_frame(dev, &frames[i]);
 		if (ret)
@@ -827,7 +833,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 		ret = tegra_vde_attach_dmabufs_to_frame(dev, &dpb_frames[i],
 							&frames[i], dma_dir,
 							ctx.baseline_profile,
-							macroblocks_nb * 64);
+							lsize, csize);
 		if (ret)
 			goto release_dpb_frames;
 	}
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v1 4/5] media: staging: tegra-vde: Do not handle spurious interrupts
  2018-03-17 15:28 [PATCH v1 0/5] Tegra Video Decoder patches for 4.17 Dmitry Osipenko
                   ` (2 preceding siblings ...)
  2018-03-17 15:28 ` [PATCH v1 3/5] media: staging: tegra-vde: Correct minimum size of U/V planes Dmitry Osipenko
@ 2018-03-17 15:28 ` Dmitry Osipenko
  2018-03-17 15:28 ` [PATCH v1 5/5] media: staging: tegra-vde: Correct included header Dmitry Osipenko
  4 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2018-03-17 15:28 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Greg Kroah-Hartman, linux-media, devel, linux-tegra, linux-kernel

Do not handle interrupts if we haven't asked for them, potentially that
could happen if HW wasn't programmed properly.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c b/drivers/staging/media/tegra-vde/tegra-vde.c
index 94b4db55cdb5..9e542c6288f1 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -935,6 +935,9 @@ static irqreturn_t tegra_vde_isr(int irq, void *data)
 {
 	struct tegra_vde *vde = data;
 
+	if (completion_done(&vde->decode_completion))
+		return IRQ_NONE;
+
 	tegra_vde_set_bits(vde, 0, vde->frameid + 0x208);
 	complete(&vde->decode_completion);
 
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v1 5/5] media: staging: tegra-vde: Correct included header
  2018-03-17 15:28 [PATCH v1 0/5] Tegra Video Decoder patches for 4.17 Dmitry Osipenko
                   ` (3 preceding siblings ...)
  2018-03-17 15:28 ` [PATCH v1 4/5] media: staging: tegra-vde: Do not handle spurious interrupts Dmitry Osipenko
@ 2018-03-17 15:28 ` Dmitry Osipenko
  4 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2018-03-17 15:28 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Greg Kroah-Hartman, linux-media, devel, linux-tegra, linux-kernel

This is Open Firmware driver, hence 'of_device.h' should be included
instead of 'platform_device.h'. Right now OF headers happen to be included
indirectly and this may break in the future, so let's correct the header.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c b/drivers/staging/media/tegra-vde/tegra-vde.c
index 9e542c6288f1..90177a59b97c 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -16,7 +16,7 @@
 #include <linux/iopoll.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
-#include <linux/platform_device.h>
+#include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-03-17 15:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-17 15:28 [PATCH v1 0/5] Tegra Video Decoder patches for 4.17 Dmitry Osipenko
2018-03-17 15:28 ` [PATCH v1 1/5] media: staging: tegra-vde: Align bitstream size to 16K Dmitry Osipenko
2018-03-17 15:28 ` [PATCH v1 2/5] media: staging: tegra-vde: Silence some of checkpatch warnings Dmitry Osipenko
2018-03-17 15:28 ` [PATCH v1 3/5] media: staging: tegra-vde: Correct minimum size of U/V planes Dmitry Osipenko
2018-03-17 15:28 ` [PATCH v1 4/5] media: staging: tegra-vde: Do not handle spurious interrupts Dmitry Osipenko
2018-03-17 15:28 ` [PATCH v1 5/5] media: staging: tegra-vde: Correct included header Dmitry Osipenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox