| From foo@baz Mon Sep 17 12:33:31 CEST 2018 |
| From: Sylwester Nawrocki <s.nawrocki@samsung.com> |
| Date: Tue, 5 Jun 2018 09:33:59 -0400 |
| Subject: media: s5p-mfc: Fix buffer look up in s5p_mfc_handle_frame_{new, copy_time} functions |
| |
| From: Sylwester Nawrocki <s.nawrocki@samsung.com> |
| |
| [ Upstream commit 4faeaf9c0f4581667ce5826f9c90c4fd463ef086 ] |
| |
| Look up of buffers in s5p_mfc_handle_frame_new, s5p_mfc_handle_frame_copy_time |
| functions is not working properly for DMA addresses above 2 GiB. As a result |
| flags and timestamp of returned buffers are not set correctly and it breaks |
| operation of GStreamer/OMX plugins which rely on the CAPTURE buffer queue |
| flags. |
| |
| Due to improper return type of the get_dec_y_adr, get_dspl_y_adr callbacks |
| and sign bit extension these callbacks return incorrect address values, |
| e.g. 0xfffffffffefc0000 instead of 0x00000000fefc0000. Then the statement: |
| |
| "if (vb2_dma_contig_plane_dma_addr(&dst_buf->b->vb2_buf, 0) == dec_y_addr)" |
| |
| is always false, which breaks looking up capture queue buffers. |
| |
| To ensure proper matching by address u32 type is used for the DMA |
| addresses. This should work on all related SoCs, since the MFC DMA |
| address width is not larger than 32-bit. |
| |
| Changes done in this patch are minimal as there is a larger patch series |
| pending refactoring the whole driver. |
| |
| Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> |
| Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> |
| Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/media/platform/s5p-mfc/s5p_mfc.c | 23 ++++++++++++----------- |
| 1 file changed, 12 insertions(+), 11 deletions(-) |
| |
| --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c |
| +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c |
| @@ -254,24 +254,24 @@ static void s5p_mfc_handle_frame_all_ext |
| static void s5p_mfc_handle_frame_copy_time(struct s5p_mfc_ctx *ctx) |
| { |
| struct s5p_mfc_dev *dev = ctx->dev; |
| - struct s5p_mfc_buf *dst_buf, *src_buf; |
| - size_t dec_y_addr; |
| + struct s5p_mfc_buf *dst_buf, *src_buf; |
| + u32 dec_y_addr; |
| unsigned int frame_type; |
| |
| /* Make sure we actually have a new frame before continuing. */ |
| frame_type = s5p_mfc_hw_call(dev->mfc_ops, get_dec_frame_type, dev); |
| if (frame_type == S5P_FIMV_DECODE_FRAME_SKIPPED) |
| return; |
| - dec_y_addr = s5p_mfc_hw_call(dev->mfc_ops, get_dec_y_adr, dev); |
| + dec_y_addr = (u32)s5p_mfc_hw_call(dev->mfc_ops, get_dec_y_adr, dev); |
| |
| /* Copy timestamp / timecode from decoded src to dst and set |
| appropriate flags. */ |
| src_buf = list_entry(ctx->src_queue.next, struct s5p_mfc_buf, list); |
| list_for_each_entry(dst_buf, &ctx->dst_queue, list) { |
| - if (vb2_dma_contig_plane_dma_addr(&dst_buf->b->vb2_buf, 0) |
| - == dec_y_addr) { |
| - dst_buf->b->timecode = |
| - src_buf->b->timecode; |
| + u32 addr = (u32)vb2_dma_contig_plane_dma_addr(&dst_buf->b->vb2_buf, 0); |
| + |
| + if (addr == dec_y_addr) { |
| + dst_buf->b->timecode = src_buf->b->timecode; |
| dst_buf->b->vb2_buf.timestamp = |
| src_buf->b->vb2_buf.timestamp; |
| dst_buf->b->flags &= |
| @@ -307,10 +307,10 @@ static void s5p_mfc_handle_frame_new(str |
| { |
| struct s5p_mfc_dev *dev = ctx->dev; |
| struct s5p_mfc_buf *dst_buf; |
| - size_t dspl_y_addr; |
| + u32 dspl_y_addr; |
| unsigned int frame_type; |
| |
| - dspl_y_addr = s5p_mfc_hw_call(dev->mfc_ops, get_dspl_y_adr, dev); |
| + dspl_y_addr = (u32)s5p_mfc_hw_call(dev->mfc_ops, get_dspl_y_adr, dev); |
| if (IS_MFCV6_PLUS(dev)) |
| frame_type = s5p_mfc_hw_call(dev->mfc_ops, |
| get_disp_frame_type, ctx); |
| @@ -329,9 +329,10 @@ static void s5p_mfc_handle_frame_new(str |
| /* The MFC returns address of the buffer, now we have to |
| * check which videobuf does it correspond to */ |
| list_for_each_entry(dst_buf, &ctx->dst_queue, list) { |
| + u32 addr = (u32)vb2_dma_contig_plane_dma_addr(&dst_buf->b->vb2_buf, 0); |
| + |
| /* Check if this is the buffer we're looking for */ |
| - if (vb2_dma_contig_plane_dma_addr(&dst_buf->b->vb2_buf, 0) |
| - == dspl_y_addr) { |
| + if (addr == dspl_y_addr) { |
| list_del(&dst_buf->list); |
| ctx->dst_queue_cnt--; |
| dst_buf->b->sequence = ctx->sequence; |