Re: [PATCH RESEND v9 18/18] media: platform: Add jpeg enc feature

From: Chun-Kuang Hu
Date: Sun Jun 07 2020 - 19:36:39 EST

Next message: David Miller: "Re: [PATCH] net/mlx5: Add a missing macro undefinition"
Previous message: Prakhar Srivastava: "[v1 PATCH 1/2] Refactoring carrying over IMA measuremnet logs over Kexec."
In reply to: Xia Jiang: "[PATCH RESEND v9 18/18] media: platform: Add jpeg enc feature"
Next in thread: Xia Jiang: "[PATCH RESEND v9 11/18] media: platform: Use generic rounding helpers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi, Xia:

Xia Jiang <xia.jiang@xxxxxxxxxxxx> æ 2020å6æ4æ éå äå5:21åéï
>
> 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 <xia.jiang@xxxxxxxxxxxx>
> ---
> 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]

>
> +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.

>
> 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.

> +
> +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.

> +
> +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@xxxxxxxxxxxx>
> * Rick Chang <rick.chang@xxxxxxxxxxxx>
> + * Xia Jiang <xia.jiang@xxxxxxxxxxxx>
> */
>
> #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 65535U

MT8173 use this definition, why do you change this?
If the definition is wrong in MT8173, use a patch to fixup this.

>
> #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,
> };
>

Next message: David Miller: "Re: [PATCH] net/mlx5: Add a missing macro undefinition"
Previous message: Prakhar Srivastava: "[v1 PATCH 1/2] Refactoring carrying over IMA measuremnet logs over Kexec."
In reply to: Xia Jiang: "[PATCH RESEND v9 18/18] media: platform: Add jpeg enc feature"
Next in thread: Xia Jiang: "[PATCH RESEND v9 11/18] media: platform: Use generic rounding helpers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]