Tegra Video Decoder patches for 4.17
Subsystems (3)
cover Tegra Video Decoder patches for 4.17
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(-)
Signature · 1 line
2.16.1
PATCH 1/5 media: staging: tegra-vde: Align bitstream size to 16K
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.
--- drivers/staging/media/tegra-vde/tegra-vde.c | 40 ++++++++++++----------------- 1 file changed, 17 insertions(+), 23 deletions(-)
drivers/staging/media/tegra-vde/tegra-vde.c +17 -23
@@ -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,
Signature · 1 line
2.16.1
PATCH 2/5 media: staging: tegra-vde: Silence some of checkpatch warnings
In reply to Dmitry OsipenkoMake all strings single line to make them grep'able and add a comment to the memory barrier.
--- drivers/staging/media/tegra-vde/tegra-vde.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
drivers/staging/media/tegra-vde/tegra-vde.c +8 -6
@@ -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 ;
Signature · 1 line
2.16.1
PATCH 3/5 media: staging: tegra-vde: Correct minimum size of U/V planes
In reply to Dmitry OsipenkoStride 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.
--- drivers/staging/media/tegra-vde/tegra-vde.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
drivers/staging/media/tegra-vde/tegra-vde.c +9 -3
@@ -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; }
Signature · 1 line
2.16.1
PATCH 4/5 media: staging: tegra-vde: Do not handle spurious interrupts
In reply to Dmitry OsipenkoDo not handle interrupts if we haven't asked for them, potentially that could happen if HW wasn't programmed properly.
--- drivers/staging/media/tegra-vde/tegra-vde.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/staging/media/tegra-vde/tegra-vde.c +3 -0
@@ -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);
Signature · 1 line
2.16.1
PATCH 5/5 media: staging: tegra-vde: Correct included header
In reply to Dmitry OsipenkoThis 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.
--- drivers/staging/media/tegra-vde/tegra-vde.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/media/tegra-vde/tegra-vde.c +1 -1
@@ -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>
Signature · 1 line
2.16.1