From dbd18f067e1b38dc43a04c592116b1956bf92da3 Mon Sep 17 00:00:00 2001 From: Mirela Rabulea Date: Tue, 16 Apr 2019 17:36:28 +0300 Subject: [PATCH] MLK-21459: mxc-jpeg: Jpeg encoder/decoder multi-instance crash Reserve a slot for each context and allocate the slot data at open, deallocate slot data at release and free that slot. The device was already holding the array of slots. The maximum number of slots allowed per device is currently limited to 1 for encoder and 1 for decoder, but the hardware supports 4+4. Use GFP_ATOMIC for dma_zalloc_coherent, as we are holding a lock. Also fix potential decoder bug when stm_ctrl is not properly cleared before setting a new image format. This could be observed when decoding an yuv420 image (img_fmt=0000b) after an yuv444 image (0011b), but only if the descriptor is being reused. Removed some debug logs and updated a comment. Signed-off-by: Mirela Rabulea Reviewed-by: Robert Chiras --- drivers/media/platform/imx8/mxc-jpeg.c | 153 ++++++++++++++----------- drivers/media/platform/imx8/mxc-jpeg.h | 3 +- 2 files changed, 90 insertions(+), 66 deletions(-) diff --git a/drivers/media/platform/imx8/mxc-jpeg.c b/drivers/media/platform/imx8/mxc-jpeg.c index cddf227950cb..dda8662e46a0 100644 --- a/drivers/media/platform/imx8/mxc-jpeg.c +++ b/drivers/media/platform/imx8/mxc-jpeg.c @@ -1001,6 +1001,7 @@ static int mxc_jpeg_parse(struct mxc_jpeg_ctx *ctx, kfree(user_format_name); img_fmt = mxc_jpeg_fourcc_to_imgfmt(q_data_cap->fmt->fourcc); } + desc->stm_ctrl &= ~STM_CTRL_IMAGE_FORMAT(0xF); /* clear image format */ desc->stm_ctrl |= STM_CTRL_IMAGE_FORMAT(img_fmt); desc->line_pitch = mxc_jpeg_get_line_pitch(dev, &sof, img_fmt); q_data_cap->stride = desc->line_pitch; @@ -1127,81 +1128,82 @@ static int mxc_jpeg_queue_init(void *priv, struct vb2_queue *src_vq, return ret; } -static int mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg) +static int mxc_get_free_slot(struct mxc_jpeg_slot_data slot_data[], int n) { - int slot; - - for (slot = 0; slot < MXC_MAX_SLOTS; slot++) { - /* allocate descriptor for decoding/encoding phase */ - jpeg->slot_data[slot].desc = dma_zalloc_coherent(jpeg->dev, - sizeof(struct mxc_jpeg_desc), - &(jpeg->slot_data[slot].desc_handle), 0); - if (!jpeg->slot_data[slot].desc) - goto err; - dev_dbg(jpeg->dev, "Descriptor for dec/enc: %p 0x%llx\n", - jpeg->slot_data[slot].desc, - jpeg->slot_data[slot].desc_handle); - - /* allocate descriptor for configuration phase (encoder only) */ - jpeg->slot_data[slot].cfg_desc = dma_zalloc_coherent(jpeg->dev, - sizeof(struct mxc_jpeg_desc), - &jpeg->slot_data[slot].cfg_desc_handle, 0); - if (!jpeg->slot_data[slot].cfg_desc) - goto err; - dev_dbg(jpeg->dev, "Descriptor for config phase: %p 0x%llx\n", - jpeg->slot_data[slot].cfg_desc, - jpeg->slot_data[slot].cfg_desc_handle); - - /* allocate configuration stream */ - jpeg->slot_data[slot].cfg_stream_vaddr = dma_zalloc_coherent( - jpeg->dev, - sizeof(hactbl), - &jpeg->slot_data[slot].cfg_stream_handle, 0); - if (!jpeg->slot_data[slot].cfg_stream_vaddr) - goto err; - dev_dbg(jpeg->dev, "Configuration stream: %p 0x%llx\n", - jpeg->slot_data[slot].cfg_stream_vaddr, - jpeg->slot_data[slot].cfg_stream_handle); - - /* initial set-up for configuration stream - * TODO: fixup the sizes, currently harcoded to 64x64) - */ - memcpy(jpeg->slot_data[slot].cfg_stream_vaddr, - &hactbl, sizeof(hactbl)); - } - return 0; + int free_slot = 0; + + while (slot_data[free_slot].used && free_slot < n) + free_slot++; + + return free_slot; /* >=n when there are no more free slots */ +} + +static bool mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg, int slot) +{ + /* allocate descriptor for decoding/encoding phase */ + jpeg->slot_data[slot].desc = dma_zalloc_coherent(jpeg->dev, + sizeof(struct mxc_jpeg_desc), + &jpeg->slot_data[slot].desc_handle, GFP_ATOMIC); + if (!jpeg->slot_data[slot].desc) + goto err; + + /* allocate descriptor for configuration phase (encoder only) */ + jpeg->slot_data[slot].cfg_desc = dma_zalloc_coherent(jpeg->dev, + sizeof(struct mxc_jpeg_desc), + &jpeg->slot_data[slot].cfg_desc_handle, GFP_ATOMIC); + if (!jpeg->slot_data[slot].cfg_desc) + goto err; + + /* allocate configuration stream */ + jpeg->slot_data[slot].cfg_stream_vaddr = dma_zalloc_coherent( + jpeg->dev, + sizeof(hactbl), + &jpeg->slot_data[slot].cfg_stream_handle, GFP_ATOMIC); + if (!jpeg->slot_data[slot].cfg_stream_vaddr) + goto err; + + /* initial set-up for configuration stream */ + memcpy(jpeg->slot_data[slot].cfg_stream_vaddr, + &hactbl, sizeof(hactbl)); + + jpeg->slot_data[slot].used = true; + + return true; err: - dev_err(jpeg->dev, "Could not allocate descriptors\n"); - return 1; + dev_err(jpeg->dev, "Could not allocate descriptors for slot %d", slot); + return false; } -static int mxc_jpeg_free_slot_data(struct mxc_jpeg_dev *jpeg) +static void mxc_jpeg_free_slot_data(struct mxc_jpeg_dev *jpeg, int slot) { - int slot; - - for (slot = 0; slot < MXC_MAX_SLOTS; slot++) { - /* free descriptor for decoding/encoding phase */ - dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc), - jpeg->slot_data[slot].desc, - jpeg->slot_data[slot].desc_handle); - - /* free descriptor for configuration phase (encoder only) */ - dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc), - jpeg->slot_data[slot].cfg_desc, - jpeg->slot_data[slot].cfg_desc_handle); - - /* free configuration stream */ - dma_free_coherent(jpeg->dev, sizeof(hactbl), - jpeg->slot_data[slot].cfg_stream_vaddr, - jpeg->slot_data[slot].cfg_stream_handle); + if (slot >= MXC_MAX_SLOTS) { + dev_err(jpeg->dev, "Invalid slot %d, nothing to free.", slot); + return; } - return 0; + + /* free descriptor for decoding/encoding phase */ + dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc), + jpeg->slot_data[slot].desc, + jpeg->slot_data[slot].desc_handle); + + /* free descriptor for configuration phase (encoder only) */ + dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc), + jpeg->slot_data[slot].cfg_desc, + jpeg->slot_data[slot].cfg_desc_handle); + + /* free configuration stream */ + dma_free_coherent(jpeg->dev, sizeof(hactbl), + jpeg->slot_data[slot].cfg_stream_vaddr, + jpeg->slot_data[slot].cfg_stream_handle); + + jpeg->slot_data[slot].used = false; } static int mxc_jpeg_open(struct file *file) { struct mxc_jpeg_dev *mxc_jpeg = video_drvdata(file); struct video_device *mxc_vfd = video_devdata(file); + struct device *dev = mxc_jpeg->dev; struct mxc_jpeg_ctx *ctx; struct mxc_jpeg_fmt *out_fmt, *cap_fmt; int ret = 0; @@ -1240,9 +1242,23 @@ static int mxc_jpeg_open(struct file *file) goto error; } - if (mxc_jpeg_alloc_slot_data(mxc_jpeg)) + ctx->slot = mxc_get_free_slot(mxc_jpeg->slot_data, MXC_MAX_SLOTS); + if (ctx->slot >= MXC_MAX_SLOTS) { + dev_err(dev, "No more free slots\n"); + ret = -EBUSY; goto error; + } + if (!mxc_jpeg_alloc_slot_data(mxc_jpeg, ctx->slot)) { + ret = -ENOMEM; + goto error; + } + if (mxc_jpeg->mode == MXC_JPEG_DECODE) + dev_dbg(dev, "Opened JPEG decoder instance on slot %d.", + ctx->slot); + else + dev_dbg(dev, "Opened JPEG encoder instance on slot %d.", + ctx->slot); mutex_unlock(&mxc_jpeg->lock); return 0; @@ -1584,9 +1600,16 @@ static int mxc_jpeg_release(struct file *file) { struct mxc_jpeg_dev *mxc_jpeg = video_drvdata(file); struct mxc_jpeg_ctx *ctx = mxc_jpeg_fh_to_ctx(file->private_data); + struct device *dev = mxc_jpeg->dev; mutex_lock(&mxc_jpeg->lock); - mxc_jpeg_free_slot_data(mxc_jpeg); + if (mxc_jpeg->mode == MXC_JPEG_DECODE) + dev_dbg(dev, "Release JPEG decoder instance on slot %d.", + ctx->slot); + else + dev_dbg(dev, "Release JPEG encoder instance on slot %d.", + ctx->slot); + mxc_jpeg_free_slot_data(mxc_jpeg, ctx->slot); v4l2_m2m_ctx_release(ctx->fh.m2m_ctx); v4l2_fh_del(&ctx->fh); v4l2_fh_exit(&ctx->fh); diff --git a/drivers/media/platform/imx8/mxc-jpeg.h b/drivers/media/platform/imx8/mxc-jpeg.h index 9f039264b093..163a5ea4f8be 100644 --- a/drivers/media/platform/imx8/mxc-jpeg.h +++ b/drivers/media/platform/imx8/mxc-jpeg.h @@ -91,10 +91,11 @@ struct mxc_jpeg_ctx { unsigned int enc_state; unsigned int aborting; unsigned int stopping; + unsigned int slot; }; struct mxc_jpeg_slot_data { - int used; + bool used; struct mxc_jpeg_desc *desc; // enc/dec descriptor struct mxc_jpeg_desc *cfg_desc; // configuration descriptor void *cfg_stream_vaddr; // configuration bitstream virtual address -- 2.17.1