Re: [PATCH RESEND v9 18/18] media: platform: Add jpeg enc feature
From: Xia Jiang <hidden>
Date: 2020-07-24 09:08:32
Also in:
linux-devicetree, linux-media, linux-mediatek, lkml
On Mon, 2020-06-08 at 07:36 +0800, Chun-Kuang Hu wrote:
Hi, Xia: Xia Jiang [off-list ref] 於 2020年6月4日 週四 下午5:21寫道：quoted
Add mtk jpeg encode v4l2 driver based on jpeg decode, because that jpeg decode and encode have great similarities with function operation. Signed-off-by: Xia Jiang <redacted> --- v9: add member variable(struct v4l2_rect) in out_q structure for storing the active crop information. move the renaming exsting functions/defines/variables to a separate patch. --- drivers/media/platform/mtk-jpeg/Makefile | 5 +- .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 845 +++++++++++++++--- .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 44 +- .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 193 ++++ .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h | 123 +++ 5 files changed, 1084 insertions(+), 126 deletions(-) create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.h[snip]
Dear Chun-Kuang, Thanks for your reply. I have uploaded v10 version which contains the changes you mentioned.
quoted
+static void mtk_jpeg_set_enc_default_params(struct mtk_jpeg_ctx *ctx) +{ + struct mtk_jpeg_q_data *q = &ctx->out_q; + struct v4l2_pix_format_mplane *pix_mp; + + pix_mp = kmalloc(sizeof(*pix_mp), GFP_KERNEL); + + ctx->fh.ctrl_handler = &ctx->ctrl_hdl; + ctx->colorspace = V4L2_COLORSPACE_JPEG, + ctx->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; + ctx->quantization = V4L2_QUANTIZATION_DEFAULT; + ctx->xfer_func = V4L2_XFER_FUNC_DEFAULT; + pix_mp->width = MTK_JPEG_MIN_WIDTH; + pix_mp->height = MTK_JPEG_MIN_HEIGHT; + + q->fmt = mtk_jpeg_find_format(V4L2_PIX_FMT_YUYV, + MTK_JPEG_FMT_FLAG_ENC_OUTPUT); + vidioc_try_fmt(container_of(pix_mp, struct v4l2_format, + fmt.pix_mp), q->fmt); + q->w = pix_mp->width; + q->h = pix_mp->height; + q->crop_rect.width = pix_mp->width; + q->crop_rect.height = pix_mp->height; + q->sizeimage[0] = pix_mp->plane_fmt[0].sizeimage; + q->bytesperline[0] = pix_mp->plane_fmt[0].bytesperline; + + q = &ctx->cap_q; + q->fmt = mtk_jpeg_find_format(V4L2_PIX_FMT_JPEG, + MTK_JPEG_FMT_FLAG_ENC_CAPTURE); + pix_mp->width = MTK_JPEG_MIN_WIDTH; + pix_mp->height = MTK_JPEG_MIN_HEIGHT; + vidioc_try_fmt(container_of(pix_mp, struct v4l2_format, + fmt.pix_mp), q->fmt); + q->w = pix_mp->width; + q->h = pix_mp->height; + q->sizeimage[0] = pix_mp->plane_fmt[0].sizeimage; + q->bytesperline[0] = pix_mp->plane_fmt[0].bytesperline; +} + static void mtk_jpeg_set_dec_default_params(struct mtk_jpeg_ctx *ctx) { struct mtk_jpeg_q_data *q = &ctx->out_q; + struct v4l2_pix_format_mplane *pix_mp; int i; + pix_mp = kmalloc(sizeof(*pix_mp), GFP_KERNEL); + + ctx->fh.ctrl_handler = &ctx->ctrl_hdl; ctx->colorspace = V4L2_COLORSPACE_JPEG, ctx->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; ctx->quantization = V4L2_QUANTIZATION_DEFAULT; ctx->xfer_func = V4L2_XFER_FUNC_DEFAULT; - - q->fmt = mtk_jpeg_find_format(ctx, V4L2_PIX_FMT_JPEG, - MTK_JPEG_FMT_TYPE_OUTPUT); - q->w = MTK_JPEG_MIN_WIDTH; - q->h = MTK_JPEG_MIN_HEIGHT; - q->bytesperline[0] = 0; - q->sizeimage[0] = MTK_JPEG_DEFAULT_SIZEIMAGE; + pix_mp->width = MTK_JPEG_MIN_WIDTH; + pix_mp->height = MTK_JPEG_MIN_HEIGHT; + + q->fmt = mtk_jpeg_find_format(V4L2_PIX_FMT_JPEG, + MTK_JPEG_FMT_FLAG_DEC_OUTPUT); + vidioc_try_fmt(container_of(pix_mp, struct v4l2_format, + fmt.pix_mp), q->fmt); + q->w = pix_mp->width; + q->h = pix_mp->height; + q->sizeimage[0] = pix_mp->plane_fmt[0].sizeimage; + q->bytesperline[0] = pix_mp->plane_fmt[0].bytesperline;I would like non-jpeg-enc related modification to be another patch.
done.
quoted
q = &ctx->cap_q; - q->fmt = mtk_jpeg_find_format(ctx, V4L2_PIX_FMT_YUV420M, - MTK_JPEG_FMT_TYPE_CAPTURE); - q->w = MTK_JPEG_MIN_WIDTH; - q->h = MTK_JPEG_MIN_HEIGHT; - + q->fmt = mtk_jpeg_find_format(V4L2_PIX_FMT_YUV420M, + MTK_JPEG_FMT_FLAG_DEC_CAPTURE); + pix_mp->width = MTK_JPEG_MIN_WIDTH; + pix_mp->height = MTK_JPEG_MIN_HEIGHT; + vidioc_try_fmt(container_of(pix_mp, struct v4l2_format, + fmt.pix_mp), q->fmt); + q->w = pix_mp->width; + q->h = pix_mp->height; for (i = 0; i < q->fmt->colplanes; i++) { - u32 stride = q->w * q->fmt->h_sample[i] / 4; - u32 h = q->h * q->fmt->v_sample[i] / 4; + q->sizeimage[i] = pix_mp->plane_fmt[i].sizeimage; + q->bytesperline[i] = pix_mp->plane_fmt[i].bytesperline; + } +} - q->bytesperline[i] = stride; - q->sizeimage[i] = stride * h; +static int mtk_jpeg_enc_open(struct file *file) +{ + struct mtk_jpeg_dev *jpeg = video_drvdata(file); + struct video_device *vfd = video_devdata(file); + struct mtk_jpeg_ctx *ctx; + int ret = 0; + + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + if (mutex_lock_interruptible(&jpeg->lock)) { + ret = -ERESTARTSYS; + goto free; + } + + v4l2_fh_init(&ctx->fh, vfd); + file->private_data = &ctx->fh; + v4l2_fh_add(&ctx->fh); + + ctx->jpeg = jpeg; + ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(jpeg->m2m_dev, ctx, + mtk_jpeg_enc_queue_init); + if (IS_ERR(ctx->fh.m2m_ctx)) { + ret = PTR_ERR(ctx->fh.m2m_ctx); + goto error; } + + ret = mtk_jpeg_enc_ctrls_setup(ctx); + if (ret) { + v4l2_err(&jpeg->v4l2_dev, "Failed to setup jpeg enc controls\n"); + goto error; + } + mtk_jpeg_set_enc_default_params(ctx); + + mutex_unlock(&jpeg->lock); + return 0; + +error: + v4l2_fh_del(&ctx->fh); + v4l2_fh_exit(&ctx->fh); + mutex_unlock(&jpeg->lock); +free: + kfree(ctx); + return ret; } static int mtk_jpeg_dec_open(struct file *file)@@ -953,6 +1507,12 @@ static int mtk_jpeg_dec_open(struct file *file) goto error; } + v4l2_ctrl_handler_init(&ctx->ctrl_hdl, 0); + ret = v4l2_ctrl_handler_setup(&ctx->ctrl_hdl); + if (ret) { + v4l2_err(&jpeg->v4l2_dev, "Failed to setup jpeg dec controls\n"); + goto error; + } mtk_jpeg_set_dec_default_params(ctx); mutex_unlock(&jpeg->lock); return 0;@@ -973,6 +1533,7 @@ static int mtk_jpeg_release(struct file *file) mutex_lock(&jpeg->lock); v4l2_m2m_ctx_release(ctx->fh.m2m_ctx); + v4l2_ctrl_handler_free(&ctx->ctrl_hdl); v4l2_fh_del(&ctx->fh); v4l2_fh_exit(&ctx->fh); kfree(ctx);@@ -980,6 +1541,15 @@ static int mtk_jpeg_release(struct file *file) return 0; } +static const struct v4l2_file_operations mtk_jpeg_enc_fops = { + .owner = THIS_MODULE, + .open = mtk_jpeg_enc_open, + .release = mtk_jpeg_release, + .poll = v4l2_m2m_fop_poll, + .unlocked_ioctl = video_ioctl2, + .mmap = v4l2_m2m_fop_mmap, +}; + static const struct v4l2_file_operations mtk_jpeg_dec_fops = { .owner = THIS_MODULE, .open = mtk_jpeg_dec_open,@@ -993,6 +1563,7 @@ static int mtk_jpeg_clk_init(struct mtk_jpeg_dev *jpeg) { struct device_node *node; struct platform_device *pdev; + int i; node = of_parse_phandle(jpeg->dev->of_node, "mediatek,larb", 0); if (!node)@@ -1006,12 +1577,17 @@ static int mtk_jpeg_clk_init(struct mtk_jpeg_dev *jpeg) jpeg->larb = &pdev->dev; - jpeg->clk_jdec = devm_clk_get(jpeg->dev, "jpgdec"); - if (IS_ERR(jpeg->clk_jdec)) - return PTR_ERR(jpeg->clk_jdec); + for (i = 0; i < jpeg->variant->num_clocks; i++) { + jpeg->clocks[i] = devm_clk_get(jpeg->dev, + jpeg->variant->clk_names[i]); + if (IS_ERR(jpeg->clocks[i])) { + dev_err(&pdev->dev, "failed to get clock: %s\n", + jpeg->variant->clk_names[i]); + return PTR_ERR(jpeg->clocks[i]); + } + } - jpeg->clk_jdec_smi = devm_clk_get(jpeg->dev, "jpgdec-smi"); - return PTR_ERR_OR_ZERO(jpeg->clk_jdec_smi); + return 0; } static int mtk_jpeg_probe(struct platform_device *pdev)@@ -1028,6 +1604,7 @@ static int mtk_jpeg_probe(struct platform_device *pdev) mutex_init(&jpeg->lock); spin_lock_init(&jpeg->hw_lock); jpeg->dev = &pdev->dev; + jpeg->variant = of_device_get_match_data(jpeg->dev); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); jpeg->reg_base = devm_ioremap_resource(&pdev->dev, res);@@ -1042,8 +1619,12 @@ static int mtk_jpeg_probe(struct platform_device *pdev) return jpeg_irq; } - ret = devm_request_irq(&pdev->dev, jpeg_irq, mtk_jpeg_dec_irq, 0, - pdev->name, jpeg); + if (jpeg->variant->is_encoder) + ret = devm_request_irq(&pdev->dev, jpeg_irq, mtk_jpeg_enc_irq, + 0, pdev->name, jpeg); + else + ret = devm_request_irq(&pdev->dev, jpeg_irq, mtk_jpeg_dec_irq, + 0, pdev->name, jpeg); if (ret) { dev_err(&pdev->dev, "Failed to request jpeg_irq %d (%d)\n", jpeg_irq, ret);@@ -1063,7 +1644,10 @@ static int mtk_jpeg_probe(struct platform_device *pdev) goto err_dev_register; } - jpeg->m2m_dev = v4l2_m2m_init(&mtk_jpeg_dec_m2m_ops); + if (jpeg->variant->is_encoder) + jpeg->m2m_dev = v4l2_m2m_init(&mtk_jpeg_enc_m2m_ops); + else + jpeg->m2m_dev = v4l2_m2m_init(&mtk_jpeg_dec_m2m_ops); if (IS_ERR(jpeg->m2m_dev)) { v4l2_err(&jpeg->v4l2_dev, "Failed to init mem2mem device\n"); ret = PTR_ERR(jpeg->m2m_dev);@@ -1076,9 +1660,15 @@ static int mtk_jpeg_probe(struct platform_device *pdev) goto err_vfd_jpeg_alloc; } snprintf(jpeg->vdev->name, sizeof(jpeg->vdev->name), - "%s-dec", MTK_JPEG_NAME); - jpeg->vdev->fops = &mtk_jpeg_dec_fops; - jpeg->vdev->ioctl_ops = &mtk_jpeg_dec_ioctl_ops; + "%s-%s", MTK_JPEG_NAME, + jpeg->variant->is_encoder ? "enc" : "dec"); + if (jpeg->variant->is_encoder) { + jpeg->vdev->fops = &mtk_jpeg_enc_fops; + jpeg->vdev->ioctl_ops = &mtk_jpeg_enc_ioctl_ops; + } else { + jpeg->vdev->fops = &mtk_jpeg_dec_fops; + jpeg->vdev->ioctl_ops = &mtk_jpeg_dec_ioctl_ops; + } jpeg->vdev->minor = -1; jpeg->vdev->release = video_device_release; jpeg->vdev->lock = &jpeg->lock;@@ -1095,8 +1685,9 @@ static int mtk_jpeg_probe(struct platform_device *pdev) video_set_drvdata(jpeg->vdev, jpeg); v4l2_info(&jpeg->v4l2_dev, - "decoder device registered as /dev/video%d (%d,%d)\n", - jpeg->vdev->num, VIDEO_MAJOR, jpeg->vdev->minor); + "jpeg %s device registered as /dev/video%d (%d,%d)\n", + jpeg->variant->is_encoder ? "enc" : "dec", jpeg->vdev->num, + VIDEO_MAJOR, jpeg->vdev->minor); platform_set_drvdata(pdev, jpeg);@@ -1187,14 +1778,36 @@ static const struct dev_pm_ops mtk_jpeg_pm_ops = { SET_RUNTIME_PM_OPS(mtk_jpeg_pm_suspend, mtk_jpeg_pm_resume, NULL) }; +static struct mtk_jpeg_variant mt8173_jpeg_drvdata = { + .is_encoder = false, + .clk_names = {"jpgdec-smi", "jpgdec"}, + .num_clocks = 2, +};I would like non-jpeg-enc related modification to be another patch.
done.
quoted
+ +static struct mtk_jpeg_variant mt2701_jpeg_drvdata = { + .is_encoder = false, + .clk_names = {"jpgdec-smi", "jpgdec"}, + .num_clocks = 2, +};mt2701_jpeg_drvdata is equal to mt8173_jpeg_drvdata, so keep only mt8173_jpeg_drvdata.
done.
quoted
+ +static struct mtk_jpeg_variant mtk_jpeg_drvdata = { + .is_encoder = true, + .clk_names = {"jpgenc"}, + .num_clocks = 1, +}; + static const struct of_device_id mtk_jpeg_match[] = { { .compatible = "mediatek,mt8173-jpgdec", - .data = NULL, + .data = &mt8173_jpeg_drvdata, }, { .compatible = "mediatek,mt2701-jpgdec", - .data = NULL, + .data = &mt2701_jpeg_drvdata, + }, + { + .compatible = "mediatek,mtk-jpgenc", + .data = &mtk_jpeg_drvdata, }, {}, };diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h index 0b59e48495d5..9ec2c3350a16 100644 --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h@@ -3,6 +3,7 @@ * Copyright (c) 2016 MediaTek Inc. * Author: Ming Hsiu Tsai <minghsiu.tsai@mediatek.com> * Rick Chang <rick.chang@mediatek.com> + * Xia Jiang <xia.jiang@mediatek.com> */ #ifndef _MTK_JPEG_CORE_H@@ -16,19 +17,21 @@ #define MTK_JPEG_NAME "mtk-jpeg" #define MTK_JPEG_COMP_MAX 3 +#define MTK_JPEG_MAX_CLOCKS 2 + #define MTK_JPEG_FMT_FLAG_DEC_OUTPUT BIT(0) #define MTK_JPEG_FMT_FLAG_DEC_CAPTURE BIT(1) - -#define MTK_JPEG_FMT_TYPE_OUTPUT 1 -#define MTK_JPEG_FMT_TYPE_CAPTURE 2 +#define MTK_JPEG_FMT_FLAG_ENC_OUTPUT BIT(2) +#define MTK_JPEG_FMT_FLAG_ENC_CAPTURE BIT(3) #define MTK_JPEG_MIN_WIDTH 32U #define MTK_JPEG_MIN_HEIGHT 32U -#define MTK_JPEG_MAX_WIDTH 8192U -#define MTK_JPEG_MAX_HEIGHT 8192U +#define MTK_JPEG_MAX_WIDTH 65535U +#define MTK_JPEG_MAX_HEIGHT 65535UMT8173 use this definition, why do you change this? If the definition is wrong in MT8173, use a patch to fixup this.
yes,the max width/height is also 65535 for MT8173.
quoted
#define MTK_JPEG_DEFAULT_SIZEIMAGE (1 * 1024 * 1024) +#define MTK_JPEG_MAX_EXIF_SIZE (64 * 1024) /** * enum mtk_jpeg_ctx_state - states of the context state machine@@ -42,6 +45,18 @@ enum mtk_jpeg_ctx_state { MTK_JPEG_SOURCE_CHANGE, };
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel