| Message ID | 1645693637-627-4-git-send-email-kyrie.wu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable two hardware jpeg encoder for MT8195 | expand |
Il 24/02/22 10:07, kyrie.wu ha scritto: > From: kyrie wu <kyrie.wu@mediatek.com> > > manage each hardware information, including irq/clk/power. > the hardware includes HW0 and HW1. > > Signed-off-by: kyrie wu <kyrie.wu@mediatek.com> > --- > drivers/media/platform/mtk-jpeg/Makefile | 11 +- > .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 76 +++++--- > .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 37 ++++ > .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 168 ++++++++++++++++++ > 4 files changed, 267 insertions(+), 25 deletions(-) > Hello Kyrie, despite my v6 review, where I also gave you solutions for an issue with more than one example, this v7 still didn't get one out of the many requested fixes. I'm sure that this was not intentional, so it's not a problem... In any case, this gave me the opportunity to see some more issues inside of this patch: let's get it perfect! ...snip... > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h > index 3e4811a41ba2..31e941ef84bd 100644 > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h > @@ -9,6 +9,7 @@ > #ifndef _MTK_JPEG_CORE_H > #define _MTK_JPEG_CORE_H > > +#include <linux/clk.h> > #include <linux/interrupt.h> > #include <media/v4l2-ctrls.h> > #include <media/v4l2-device.h> > @@ -60,6 +61,7 @@ enum mtk_jpeg_ctx_state { > * @cap_q_default_fourcc: capture queue default fourcc > */ > struct mtk_jpeg_variant { > + bool is_multihw; Thanks for this fix, this name makes it way clearer! > struct clk_bulk_data *clks; > int num_clks; > struct mtk_jpeg_fmt *formats; > @@ -74,6 +76,38 @@ struct mtk_jpeg_variant { > u32 cap_q_default_fourcc; > }; > > +enum mtk_jpegenc_hw_id { > + MTK_JPEGENC_HW0, > + MTK_JPEGENC_HW1, > + MTK_JPEGENC_HW_MAX, > +}; > + > +/** > + * struct mtk_vcodec_clk - Structure used to store vcodec clock information > + */ > +struct mtk_jpegenc_clk { > + struct clk_bulk_data *clks; > + int clk_num; Why is clk_num tabbed? > +}; > + > +/** > + * struct mtk_jpegenc_comp_dev - JPEG COREX abstraction > + * @dev: JPEG device > + * @plat_dev: platform device data > + * @reg_base: JPEG registers mapping > + * @master_dev: mtk_jpeg_dev device > + * @pm: mtk_jpegenc_pm > + * @jpegenc_irq: jpeg encode irq num You're using tabulations *and* spaces.... please use either, not both, as it's not necessary. Besides, this is also producing bad indentation. > + */ > +struct mtk_jpegenc_comp_dev { > + struct device *dev; > + struct platform_device *plat_dev; > + void __iomem *reg_base; > + struct mtk_jpeg_dev *master_dev; > + struct mtk_jpegenc_clk venc_clk; > + int jpegenc_irq; > +}; > + > /** > * struct mtk_jpeg_dev - JPEG IP abstraction > * @lock: the mutex protecting this structure > @@ -100,6 +134,9 @@ struct mtk_jpeg_dev { > void __iomem *reg_base; > struct delayed_work job_timeout_work; > const struct mtk_jpeg_variant *variant; > + > + void __iomem *reg_encbase[MTK_JPEGENC_HW_MAX]; > + struct mtk_jpegenc_comp_dev *enc_hw_dev[MTK_JPEGENC_HW_MAX]; > }; > > /** > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c > index a2b6e1f85c2d..3d967bff1352 100644 > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c > @@ -5,11 +5,27 @@ > * > */ > > +#include <linux/clk.h> > +#include <linux/interrupt.h> > +#include <linux/irq.h> > #include <linux/io.h> > #include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/slab.h> > +#include <media/media-device.h> > #include <media/videobuf2-core.h> > #include <media/videobuf2-dma-contig.h> > +#include <media/videobuf2-v4l2.h> > +#include <media/v4l2-mem2mem.h> > +#include <media/v4l2-dev.h> > +#include <media/v4l2-device.h> > +#include <media/v4l2-fh.h> > +#include <media/v4l2-event.h> > > +#include "mtk_jpeg_core.h" > #include "mtk_jpeg_enc_hw.h" > > static const struct mtk_jpeg_enc_qlt mtk_jpeg_enc_quality[] = { > @@ -30,6 +46,21 @@ static const struct mtk_jpeg_enc_qlt mtk_jpeg_enc_quality[] = { > {.quality_param = 97, .hardware_value = JPEG_ENC_QUALITY_Q97}, > }; > > +#if defined(CONFIG_OF) > +static const struct of_device_id mtk_jpegenc_drv_ids[] = { > + { > + .compatible = "mediatek,mt8195-jpgenc0", > + .data = (void *)MTK_JPEGENC_HW0, > + }, > + { > + .compatible = "mediatek,mt8195-jpgenc1", > + .data = (void *)MTK_JPEGENC_HW1, > + }, I've already pointed out an issue with this in your v6 series: https://patchwork.kernel.org/comment/24726607/ Besides, I want to add up that the SoC distinction is already done in the parent node which, in MT8195's case, is named "mediatek,mt8195-jpgenc", so you really don't have to redo this distinction "from scratch" here in the sub-driver, as you can just get your information from the parent device/node. So, just "mediatek,jpgenc-hw" should be totally enough here. Please fix this for v8. > + {}, > +}; > +MODULE_DEVICE_TABLE(of, mtk_jpegenc_drv_ids); > +#endif > + > void mtk_jpeg_enc_reset(void __iomem *base) > { > writel(0, base + JPEG_ENC_RSTB); ...snip... > + > +static int mtk_jpegenc_hw_probe(struct platform_device *pdev) > +{ > + struct mtk_jpegenc_clk *jpegenc_clk; > + struct mtk_jpeg_dev *master_dev; > + struct mtk_jpegenc_comp_dev *dev; > + int ret, comp_idx; > + > + struct device *decs = &pdev->dev; > + > + if (!decs->parent) > + return -EPROBE_DEFER; > + > + master_dev = dev_get_drvdata(decs->parent); > + if (!master_dev) > + return -EPROBE_DEFER; > + > + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL); > + if (!dev) > + return -ENOMEM; > + > + dev->plat_dev = pdev; > + > + jpegenc_clk = &dev->venc_clk; > + > + jpegenc_clk->clk_num = devm_clk_bulk_get_all(&pdev->dev, > + &jpegenc_clk->clks); Using dev_err_probe() looks more appropriate here: if (jpegenc_clk->clk_num < 0) return dev_err_probe(&pdev->dev, jpegenc_clk->clk_num, "Failed to get jpegenc clocks\n"); > + if (jpegenc_clk->clk_num < 0) { > + dev_err(&pdev->dev, "Failed to get jpegenc clock count\n"); > + return jpegenc_clk->clk_num; > + } > + > + dev->reg_base = > + devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(dev->reg_base)) { > + ret = PTR_ERR(dev->reg_base); > + goto err; There's no need for any goto here, as you're not reverting any operation. Hence, you can just: if (IS_ERR(dev->reg_base)) return PTR_ERR(dev->reg_base); > + } > + > + ret = mtk_jpegenc_hw_init_irq(dev); > + if (ret) { > + dev_err(&pdev->dev, "Failed to register JPEGENC irq handler.\n"); You are already printing an error inside of mtk_jpegenc_hw_init_irq(), so printing another one here is redundant. Either remove the prints in the function or, more appropriately, remove this print. Also, same "goto" comment applies here, you can simply return ret. > + goto err; > + } > + > + comp_idx = (enum mtk_jpegenc_hw_id)of_device_get_match_data(decs); > + if (comp_idx < MTK_JPEGENC_HW_MAX) { `comp_idx` is a bit misleading, this is not using the component framework. ....but this will probably be refactored after following the suggestion that I gave you in v6 and again now. > + master_dev->enc_hw_dev[comp_idx] = dev; > + master_dev->reg_encbase[comp_idx] = dev->reg_base; > + dev->master_dev = master_dev; > + } else { > + dev_err(&pdev->dev, "Failed to get_match_data.\n"); > + goto err; > + } > + > + platform_set_drvdata(pdev, dev); > + pm_runtime_enable(&pdev->dev); > + > + return 0; > + > +err: This label serves no real purpose: please remove. > + return ret; > +} > + Regards, Angelo
On Wed, 2022-03-02 at 10:45 +0100, AngeloGioacchino Del Regno wrote: > Il 24/02/22 10:07, kyrie.wu ha scritto: > > From: kyrie wu <kyrie.wu@mediatek.com> > > > > manage each hardware information, including irq/clk/power. > > the hardware includes HW0 and HW1. > > > > Signed-off-by: kyrie wu <kyrie.wu@mediatek.com> > > --- > > drivers/media/platform/mtk-jpeg/Makefile | 11 +- > > .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 76 +++++--- > > .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 37 ++++ > > .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 168 > > ++++++++++++++++++ > > 4 files changed, 267 insertions(+), 25 deletions(-) > > > > Hello Kyrie, > > despite my v6 review, where I also gave you solutions for an issue > with > more than one example, this v7 still didn't get one out of the many > requested fixes. > > I'm sure that this was not intentional, so it's not a problem... > > In any case, this gave me the opportunity to see some more issues > inside > of this patch: let's get it perfect! > > > ...snip... > > > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h > > b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h > > index 3e4811a41ba2..31e941ef84bd 100644 > > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h > > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h > > @@ -9,6 +9,7 @@ > > #ifndef _MTK_JPEG_CORE_H > > #define _MTK_JPEG_CORE_H > > > > +#include <linux/clk.h> > > #include <linux/interrupt.h> > > #include <media/v4l2-ctrls.h> > > #include <media/v4l2-device.h> > > @@ -60,6 +61,7 @@ enum mtk_jpeg_ctx_state { > > * @cap_q_default_fourcc: capture queue default fourcc > > */ > > struct mtk_jpeg_variant { > > + bool is_multihw; > > Thanks for this fix, this name makes it way clearer! > > > struct clk_bulk_data *clks; > > int num_clks; > > struct mtk_jpeg_fmt *formats; > > @@ -74,6 +76,38 @@ struct mtk_jpeg_variant { > > u32 cap_q_default_fourcc; > > }; > > > > +enum mtk_jpegenc_hw_id { > > + MTK_JPEGENC_HW0, > > + MTK_JPEGENC_HW1, > > + MTK_JPEGENC_HW_MAX, > > +}; > > + > > +/** > > + * struct mtk_vcodec_clk - Structure used to store vcodec clock > > information > > + */ > > +struct mtk_jpegenc_clk { > > + struct clk_bulk_data *clks; > > + int clk_num; > > Why is clk_num tabbed? > > > +}; > > + > > +/** > > + * struct mtk_jpegenc_comp_dev - JPEG COREX abstraction > > + * @dev: JPEG device > > + * @plat_dev: platform device data > > + * @reg_base: JPEG registers mapping > > + * @master_dev: mtk_jpeg_dev device > > + * @pm: mtk_jpegenc_pm > > + * @jpegenc_irq: jpeg encode irq num > > You're using tabulations *and* spaces.... please use either, not > both, as it's > not necessary. Besides, this is also producing bad indentation. > > > + */ > > +struct mtk_jpegenc_comp_dev { > > + struct device *dev; > > + struct platform_device *plat_dev; > > + void __iomem *reg_base; > > + struct mtk_jpeg_dev *master_dev; > > + struct mtk_jpegenc_clk venc_clk; > > + int jpegenc_irq; > > +}; > > + > > /** > > * struct mtk_jpeg_dev - JPEG IP abstraction > > * @lock: the mutex protecting this structure > > @@ -100,6 +134,9 @@ struct mtk_jpeg_dev { > > void __iomem *reg_base; > > struct delayed_work job_timeout_work; > > const struct mtk_jpeg_variant *variant; > > + > > + void __iomem *reg_encbase[MTK_JPEGENC_HW_MAX]; > > + struct mtk_jpegenc_comp_dev *enc_hw_dev[MTK_JPEGENC_HW_MAX]; > > }; > > > > /** > > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c > > b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c > > index a2b6e1f85c2d..3d967bff1352 100644 > > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c > > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c > > @@ -5,11 +5,27 @@ > > * > > */ > > > > +#include <linux/clk.h> > > +#include <linux/interrupt.h> > > +#include <linux/irq.h> > > #include <linux/io.h> > > #include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/of_device.h> > > +#include <linux/pm_runtime.h> > > +#include <linux/slab.h> > > +#include <media/media-device.h> > > #include <media/videobuf2-core.h> > > #include <media/videobuf2-dma-contig.h> > > +#include <media/videobuf2-v4l2.h> > > +#include <media/v4l2-mem2mem.h> > > +#include <media/v4l2-dev.h> > > +#include <media/v4l2-device.h> > > +#include <media/v4l2-fh.h> > > +#include <media/v4l2-event.h> > > > > +#include "mtk_jpeg_core.h" > > #include "mtk_jpeg_enc_hw.h" > > > > static const struct mtk_jpeg_enc_qlt mtk_jpeg_enc_quality[] = { > > @@ -30,6 +46,21 @@ static const struct mtk_jpeg_enc_qlt > > mtk_jpeg_enc_quality[] = { > > {.quality_param = 97, .hardware_value = JPEG_ENC_QUALITY_Q97}, > > }; > > > > +#if defined(CONFIG_OF) > > +static const struct of_device_id mtk_jpegenc_drv_ids[] = { > > + { > > + .compatible = "mediatek,mt8195-jpgenc0", > > + .data = (void *)MTK_JPEGENC_HW0, > > + }, > > + { > > + .compatible = "mediatek,mt8195-jpgenc1", > > + .data = (void *)MTK_JPEGENC_HW1, > > + }, > > I've already pointed out an issue with this in your v6 series: > > https://patchwork.kernel.org/comment/24726607/ > > Besides, I want to add up that the SoC distinction is already done in > the > parent node which, in MT8195's case, is named "mediatek,mt8195- > jpgenc", so > you really don't have to redo this distinction "from scratch" here in > the > sub-driver, as you can just get your information from the parent > device/node. > > So, just "mediatek,jpgenc-hw" should be totally enough here. > > Please fix this for v8. Dear Angelo, Thank you for your prompt reply. I'm terrible sorry for that I got a misunderstanding from your preview comments. The device node have fixed in [RESEND,V7,1/6], base on this patch, I thought that your suggestion is adopt properly. Thanks again for your advice, I will fix driver data in the next version. > > > > + {}, > > +}; > > +MODULE_DEVICE_TABLE(of, mtk_jpegenc_drv_ids); > > +#endif > > + > > void mtk_jpeg_enc_reset(void __iomem *base) > > { > > writel(0, base + JPEG_ENC_RSTB); > > ...snip... > > > + > > +static int mtk_jpegenc_hw_probe(struct platform_device *pdev) > > +{ > > + struct mtk_jpegenc_clk *jpegenc_clk; > > + struct mtk_jpeg_dev *master_dev; > > + struct mtk_jpegenc_comp_dev *dev; > > + int ret, comp_idx; > > + > > + struct device *decs = &pdev->dev; > > + > > + if (!decs->parent) > > + return -EPROBE_DEFER; > > + > > + master_dev = dev_get_drvdata(decs->parent); > > + if (!master_dev) > > + return -EPROBE_DEFER; > > + > > + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL); > > + if (!dev) > > + return -ENOMEM; > > + > > + dev->plat_dev = pdev; > > + > > + jpegenc_clk = &dev->venc_clk; > > + > > + jpegenc_clk->clk_num = devm_clk_bulk_get_all(&pdev->dev, > > + &jpegenc_clk- > > >clks); > > Using dev_err_probe() looks more appropriate here: Dear Angelo, The follow suggestions are same as jpegdec patch in the preview version. I wiil fix all of them. Thanks. Regards, Kyrie > > if (jpegenc_clk->clk_num < 0) > return dev_err_probe(&pdev->dev, jpegenc_clk->clk_num, > "Failed to get jpegenc clocks\n"); > > > > + if (jpegenc_clk->clk_num < 0) { > > + dev_err(&pdev->dev, "Failed to get jpegenc clock > > count\n"); > > + return jpegenc_clk->clk_num; > > + } > > + > > + dev->reg_base = > > + devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(dev->reg_base)) { > > + ret = PTR_ERR(dev->reg_base); > > + goto err; > > There's no need for any goto here, as you're not reverting any > operation. > > Hence, you can just: > > if (IS_ERR(dev->reg_base)) > return PTR_ERR(dev->reg_base); > > > + } > > + > > + ret = mtk_jpegenc_hw_init_irq(dev); > > + if (ret) { > > + dev_err(&pdev->dev, "Failed to register JPEGENC irq > > handler.\n"); > > You are already printing an error inside of > mtk_jpegenc_hw_init_irq(), so printing > another one here is redundant. > Either remove the prints in the function or, more appropriately, > remove this print. > > Also, same "goto" comment applies here, you can simply return ret. > > > + goto err; > > + } > > + > > + comp_idx = (enum > > mtk_jpegenc_hw_id)of_device_get_match_data(decs); > > + if (comp_idx < MTK_JPEGENC_HW_MAX) { > > `comp_idx` is a bit misleading, this is not using the component > framework. > > ....but this will probably be refactored after following the > suggestion that > I gave you in v6 and again now. > > > + master_dev->enc_hw_dev[comp_idx] = dev; > > + master_dev->reg_encbase[comp_idx] = dev->reg_base; > > + dev->master_dev = master_dev; > > + } else { > > + dev_err(&pdev->dev, "Failed to get_match_data.\n"); > > + goto err; > > + } > > + > > + platform_set_drvdata(pdev, dev); > > + pm_runtime_enable(&pdev->dev); > > + > > + return 0; > > + > > +err: > > This label serves no real purpose: please remove. > > > + return ret; > > +} > > + > > > > Regards, > Angelo
diff --git a/drivers/media/platform/mtk-jpeg/Makefile b/drivers/media/platform/mtk-jpeg/Makefile index 76c33aad0f3f..69703db4b0a5 100644 --- a/drivers/media/platform/mtk-jpeg/Makefile +++ b/drivers/media/platform/mtk-jpeg/Makefile @@ -1,6 +1,9 @@ # SPDX-License-Identifier: GPL-2.0-only -mtk_jpeg-objs := mtk_jpeg_core.o \ +obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk_jpeg.o \ + mtk-jpeg-enc-hw.o + +mtk_jpeg-y := mtk_jpeg_core.o \ mtk_jpeg_dec_hw.o \ - mtk_jpeg_dec_parse.o \ - mtk_jpeg_enc_hw.o -obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk_jpeg.o + mtk_jpeg_dec_parse.o + +mtk-jpeg-enc-hw-y := mtk_jpeg_enc_hw.o diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c index d532f86e826e..35fd2b41f446 100644 --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c @@ -1312,31 +1312,39 @@ static int mtk_jpeg_probe(struct platform_device *pdev) spin_lock_init(&jpeg->hw_lock); jpeg->dev = &pdev->dev; jpeg->variant = of_device_get_match_data(jpeg->dev); - INIT_DELAYED_WORK(&jpeg->job_timeout_work, mtk_jpeg_job_timeout_work); - jpeg->reg_base = devm_platform_ioremap_resource(pdev, 0); - if (IS_ERR(jpeg->reg_base)) { - ret = PTR_ERR(jpeg->reg_base); - return ret; - } + if (!jpeg->variant->is_multihw) { + INIT_DELAYED_WORK(&jpeg->job_timeout_work, + mtk_jpeg_job_timeout_work); - jpeg_irq = platform_get_irq(pdev, 0); - if (jpeg_irq < 0) - return jpeg_irq; + jpeg->reg_base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(jpeg->reg_base)) { + ret = PTR_ERR(jpeg->reg_base); + return ret; + } - ret = devm_request_irq(&pdev->dev, jpeg_irq, - jpeg->variant->irq_handler, 0, pdev->name, jpeg); - if (ret) { - dev_err(&pdev->dev, "Failed to request jpeg_irq %d (%d)\n", - jpeg_irq, ret); - goto err_req_irq; - } + jpeg_irq = platform_get_irq(pdev, 0); + if (jpeg_irq < 0) + return jpeg_irq; - ret = devm_clk_bulk_get(jpeg->dev, jpeg->variant->num_clks, - jpeg->variant->clks); - if (ret) { - dev_err(&pdev->dev, "Failed to init clk, err %d\n", ret); - goto err_clk_init; + ret = devm_request_irq(&pdev->dev, + jpeg_irq, + jpeg->variant->irq_handler, + 0, + pdev->name, jpeg); + if (ret) { + dev_err(&pdev->dev, "Failed to request jpeg_irq %d (%d)\n", + jpeg_irq, ret); + goto err_req_irq; + } + + ret = devm_clk_bulk_get(jpeg->dev, + jpeg->variant->num_clks, + jpeg->variant->clks); + if (ret) { + dev_err(&pdev->dev, "Failed to init clk\n"); + goto err_clk_init; + } } ret = v4l2_device_register(&pdev->dev, &jpeg->v4l2_dev); @@ -1383,6 +1391,14 @@ static int mtk_jpeg_probe(struct platform_device *pdev) jpeg->variant->dev_name, jpeg->vdev->num, VIDEO_MAJOR, jpeg->vdev->minor); + if (jpeg->variant->is_multihw) { + ret = devm_of_platform_populate(&pdev->dev); + if (ret) { + v4l2_err(&jpeg->v4l2_dev, "Master of platform populate failed."); + goto err_vfd_jpeg_register; + } + } + platform_set_drvdata(pdev, jpeg); pm_runtime_enable(&pdev->dev); @@ -1494,6 +1510,19 @@ static const struct mtk_jpeg_variant mtk_jpeg_drvdata = { .cap_q_default_fourcc = V4L2_PIX_FMT_JPEG, }; +static const struct mtk_jpeg_variant mtk8195_jpegenc_drvdata = { + .is_multihw = true, + .formats = mtk_jpeg_enc_formats, + .num_formats = MTK_JPEG_ENC_NUM_FORMATS, + .qops = &mtk_jpeg_enc_qops, + .m2m_ops = &mtk_jpeg_enc_m2m_ops, + .dev_name = "mtk-jpeg-enc", + .ioctl_ops = &mtk_jpeg_enc_ioctl_ops, + .out_q_default_fourcc = V4L2_PIX_FMT_YUYV, + .cap_q_default_fourcc = V4L2_PIX_FMT_JPEG, +}; + +#if defined(CONFIG_OF) static const struct of_device_id mtk_jpeg_match[] = { { .compatible = "mediatek,mt8173-jpgdec", @@ -1507,10 +1536,15 @@ static const struct of_device_id mtk_jpeg_match[] = { .compatible = "mediatek,mtk-jpgenc", .data = &mtk_jpeg_drvdata, }, + { + .compatible = "mediatek,mt8195-jpgenc", + .data = &mtk8195_jpegenc_drvdata, + }, {}, }; MODULE_DEVICE_TABLE(of, mtk_jpeg_match); +#endif static struct platform_driver mtk_jpeg_driver = { .probe = mtk_jpeg_probe, diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h index 3e4811a41ba2..31e941ef84bd 100644 --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h @@ -9,6 +9,7 @@ #ifndef _MTK_JPEG_CORE_H #define _MTK_JPEG_CORE_H +#include <linux/clk.h> #include <linux/interrupt.h> #include <media/v4l2-ctrls.h> #include <media/v4l2-device.h> @@ -60,6 +61,7 @@ enum mtk_jpeg_ctx_state { * @cap_q_default_fourcc: capture queue default fourcc */ struct mtk_jpeg_variant { + bool is_multihw; struct clk_bulk_data *clks; int num_clks; struct mtk_jpeg_fmt *formats; @@ -74,6 +76,38 @@ struct mtk_jpeg_variant { u32 cap_q_default_fourcc; }; +enum mtk_jpegenc_hw_id { + MTK_JPEGENC_HW0, + MTK_JPEGENC_HW1, + MTK_JPEGENC_HW_MAX, +}; + +/** + * struct mtk_vcodec_clk - Structure used to store vcodec clock information + */ +struct mtk_jpegenc_clk { + struct clk_bulk_data *clks; + int clk_num; +}; + +/** + * struct mtk_jpegenc_comp_dev - JPEG COREX abstraction + * @dev: JPEG device + * @plat_dev: platform device data + * @reg_base: JPEG registers mapping + * @master_dev: mtk_jpeg_dev device + * @pm: mtk_jpegenc_pm + * @jpegenc_irq: jpeg encode irq num + */ +struct mtk_jpegenc_comp_dev { + struct device *dev; + struct platform_device *plat_dev; + void __iomem *reg_base; + struct mtk_jpeg_dev *master_dev; + struct mtk_jpegenc_clk venc_clk; + int jpegenc_irq; +}; + /** * struct mtk_jpeg_dev - JPEG IP abstraction * @lock: the mutex protecting this structure @@ -100,6 +134,9 @@ struct mtk_jpeg_dev { void __iomem *reg_base; struct delayed_work job_timeout_work; const struct mtk_jpeg_variant *variant; + + void __iomem *reg_encbase[MTK_JPEGENC_HW_MAX]; + struct mtk_jpegenc_comp_dev *enc_hw_dev[MTK_JPEGENC_HW_MAX]; }; /** diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c index a2b6e1f85c2d..3d967bff1352 100644 --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c @@ -5,11 +5,27 @@ * */ +#include <linux/clk.h> +#include <linux/interrupt.h> +#include <linux/irq.h> #include <linux/io.h> #include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/pm_runtime.h> +#include <linux/slab.h> +#include <media/media-device.h> #include <media/videobuf2-core.h> #include <media/videobuf2-dma-contig.h> +#include <media/videobuf2-v4l2.h> +#include <media/v4l2-mem2mem.h> +#include <media/v4l2-dev.h> +#include <media/v4l2-device.h> +#include <media/v4l2-fh.h> +#include <media/v4l2-event.h> +#include "mtk_jpeg_core.h" #include "mtk_jpeg_enc_hw.h" static const struct mtk_jpeg_enc_qlt mtk_jpeg_enc_quality[] = { @@ -30,6 +46,21 @@ static const struct mtk_jpeg_enc_qlt mtk_jpeg_enc_quality[] = { {.quality_param = 97, .hardware_value = JPEG_ENC_QUALITY_Q97}, }; +#if defined(CONFIG_OF) +static const struct of_device_id mtk_jpegenc_drv_ids[] = { + { + .compatible = "mediatek,mt8195-jpgenc0", + .data = (void *)MTK_JPEGENC_HW0, + }, + { + .compatible = "mediatek,mt8195-jpgenc1", + .data = (void *)MTK_JPEGENC_HW1, + }, + {}, +}; +MODULE_DEVICE_TABLE(of, mtk_jpegenc_drv_ids); +#endif + void mtk_jpeg_enc_reset(void __iomem *base) { writel(0, base + JPEG_ENC_RSTB); @@ -159,3 +190,140 @@ void mtk_jpeg_set_enc_params(struct mtk_jpeg_ctx *ctx, void __iomem *base) } EXPORT_SYMBOL_GPL(mtk_jpeg_set_enc_params); +static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv) +{ + struct vb2_v4l2_buffer *src_buf, *dst_buf; + enum vb2_buffer_state buf_state; + struct mtk_jpeg_ctx *ctx; + u32 result_size; + u32 irq_status; + + struct mtk_jpegenc_comp_dev *jpeg = priv; + struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev; + + irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) & + JPEG_ENC_INT_STATUS_MASK_ALLIRQ; + if (irq_status) + writel(0, jpeg->reg_base + JPEG_ENC_INT_STS); + if (!(irq_status & JPEG_ENC_INT_STATUS_DONE)) + return IRQ_NONE; + + ctx = v4l2_m2m_get_curr_priv(master_jpeg->m2m_dev); + if (!ctx) { + v4l2_err(&master_jpeg->v4l2_dev, "Context is NULL\n"); + return IRQ_HANDLED; + } + + src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx); + dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx); + result_size = mtk_jpeg_enc_get_file_size(jpeg->reg_base); + vb2_set_plane_payload(&dst_buf->vb2_buf, 0, result_size); + buf_state = VB2_BUF_STATE_DONE; + v4l2_m2m_buf_done(src_buf, buf_state); + v4l2_m2m_buf_done(dst_buf, buf_state); + v4l2_m2m_job_finish(master_jpeg->m2m_dev, ctx->fh.m2m_ctx); + pm_runtime_put(ctx->jpeg->dev); + + return IRQ_HANDLED; +} + +static int mtk_jpegenc_hw_init_irq(struct mtk_jpegenc_comp_dev *dev) +{ + struct platform_device *pdev = dev->plat_dev; + int ret; + + dev->jpegenc_irq = platform_get_irq(pdev, 0); + if (dev->jpegenc_irq < 0) { + dev_err(&pdev->dev, "Failed to get irq resource"); + return dev->jpegenc_irq; + } + + ret = devm_request_irq(&pdev->dev, + dev->jpegenc_irq, + mtk_jpegenc_hw_irq_handler, + 0, + pdev->name, dev); + if (ret) { + dev_err(&pdev->dev, "Failed to devm_request_irq %d (%d)", + dev->jpegenc_irq, ret); + return -ENOENT; + } + + return 0; +} + +static int mtk_jpegenc_hw_probe(struct platform_device *pdev) +{ + struct mtk_jpegenc_clk *jpegenc_clk; + struct mtk_jpeg_dev *master_dev; + struct mtk_jpegenc_comp_dev *dev; + int ret, comp_idx; + + struct device *decs = &pdev->dev; + + if (!decs->parent) + return -EPROBE_DEFER; + + master_dev = dev_get_drvdata(decs->parent); + if (!master_dev) + return -EPROBE_DEFER; + + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL); + if (!dev) + return -ENOMEM; + + dev->plat_dev = pdev; + + jpegenc_clk = &dev->venc_clk; + + jpegenc_clk->clk_num = devm_clk_bulk_get_all(&pdev->dev, + &jpegenc_clk->clks); + if (jpegenc_clk->clk_num < 0) { + dev_err(&pdev->dev, "Failed to get jpegenc clock count\n"); + return jpegenc_clk->clk_num; + } + + dev->reg_base = + devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(dev->reg_base)) { + ret = PTR_ERR(dev->reg_base); + goto err; + } + + ret = mtk_jpegenc_hw_init_irq(dev); + if (ret) { + dev_err(&pdev->dev, "Failed to register JPEGENC irq handler.\n"); + goto err; + } + + comp_idx = (enum mtk_jpegenc_hw_id)of_device_get_match_data(decs); + if (comp_idx < MTK_JPEGENC_HW_MAX) { + master_dev->enc_hw_dev[comp_idx] = dev; + master_dev->reg_encbase[comp_idx] = dev->reg_base; + dev->master_dev = master_dev; + } else { + dev_err(&pdev->dev, "Failed to get_match_data.\n"); + goto err; + } + + platform_set_drvdata(pdev, dev); + pm_runtime_enable(&pdev->dev); + + return 0; + +err: + return ret; +} + +struct platform_driver mtk_jpegenc_hw_driver = { + .probe = mtk_jpegenc_hw_probe, + .driver = { + .name = "mtk-jpegenc-hw", + .of_match_table = of_match_ptr(mtk_jpegenc_drv_ids), + }, +}; + +module_platform_driver(mtk_jpegenc_hw_driver); + +MODULE_DESCRIPTION("MediaTek JPEG encode HW driver"); +MODULE_LICENSE("GPL v2");