This series adds support for multi hardware decode into mtk-vcodec, by first
adding component framework to manage each hardware information: interrupt,
clock, register bases and power. Secondly add core thread to deal with core
hardware message, at the same time, add msg queue for different hardware
share messages. Lastly, the architecture of different specs are not the same,
using specs type to separate them.
This series has been tested with both MT8183 and MT8173. Decoding was working
for both chips.
Patches 1,2 rewrite get register bases and power on/off interface.
Patch 3-5 add component framework to support multi hardware.
Patches 6-14 add interfaces to support core hardware.
----
This patch dependents on "media: mtk-vcodec: support for MT8183 decoder"[1].
Multi hardware decode is based on stateless decoder, MT8183 is the first
time to add stateless decoder. Otherwise it will cause conflict.
Please also accept this patch together with [1].
[1]https://lore.kernel.org/patchwork/project/lkml/list/?series=507084
----
Yunfei Dong (14):
media: mtk-vcodec: Get numbers of register bases from DT
media: mtk-vcodec: Refactor vcodec pm interface
media: mtk-vcodec: Use component framework to manage each hardware
information
dt-bindings: media: mtk-vcodec: Separate video encoder and decoder
dt-bindings
media: mtk-vcodec: Use pure single core for MT8183
media: mtk-vcodec: Add irq interface for core hardware
media: mtk-vcodec: Add msg queue feature for lat and core architecture
media: mtk-vcodec: Generalize power and clock on/off interfaces
media: mtk-vcodec: Add new interface to lock different hardware
media: mtk-vcodec: Add core thread
media: mtk-vcodec: Support 34bits dma address for vdec
dt-bindings: media: mtk-vcodec: Adds decoder dt-bindings for mt8192
media: mtk-vcodec: Add core dec and dec end ipi msg
media: mtk-vcodec: Use codec type to separate different hardware
.../media/mediatek-vcodec-comp-decoder.txt | 93 ++++++
.../media/mediatek-vcodec-decoder.txt | 169 +++++++++++
.../media/mediatek-vcodec-encoder.txt | 73 +++++
drivers/media/platform/mtk-vcodec/Makefile | 2 +
.../platform/mtk-vcodec/mtk_vcodec_dec.c | 4 +-
.../platform/mtk-vcodec/mtk_vcodec_dec.h | 4 +
.../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 286 +++++++++++++++---
.../platform/mtk-vcodec/mtk_vcodec_dec_hw.c | 193 ++++++++++++
.../platform/mtk-vcodec/mtk_vcodec_dec_hw.h | 51 ++++
.../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 98 ++++--
.../platform/mtk-vcodec/mtk_vcodec_dec_pm.h | 13 +-
.../mtk-vcodec/mtk_vcodec_dec_stateful.c | 1 +
.../mtk-vcodec/mtk_vcodec_dec_stateless.c | 1 +
.../platform/mtk-vcodec/mtk_vcodec_drv.h | 69 ++++-
.../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 1 -
.../platform/mtk-vcodec/mtk_vcodec_intr.c | 30 ++
.../platform/mtk-vcodec/mtk_vcodec_intr.h | 2 +
.../platform/mtk-vcodec/mtk_vcodec_util.c | 87 +++++-
.../platform/mtk-vcodec/mtk_vcodec_util.h | 8 +-
.../media/platform/mtk-vcodec/vdec_drv_if.c | 21 +-
.../media/platform/mtk-vcodec/vdec_ipi_msg.h | 16 +-
.../platform/mtk-vcodec/vdec_msg_queue.c | 266 ++++++++++++++++
.../platform/mtk-vcodec/vdec_msg_queue.h | 136 +++++++++
.../media/platform/mtk-vcodec/vdec_vpu_if.c | 46 ++-
.../media/platform/mtk-vcodec/vdec_vpu_if.h | 22 ++
25 files changed, 1582 insertions(+), 110 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/mediatek-vcodec-comp-decoder.txt
create mode 100644 Documentation/devicetree/bindings/media/mediatek-vcodec-decoder.txt
create mode 100644 Documentation/devicetree/bindings/media/mediatek-vcodec-encoder.txt
create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_hw.c
create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_hw.h
create mode 100644 drivers/media/platform/mtk-vcodec/vdec_msg_queue.c
create mode 100644 drivers/media/platform/mtk-vcodec/vdec_msg_queue.h
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Different platform may has different numbers of register bases. Gets the
numbers of register bases from DT (sizeof(u32) * 4 bytes for each).
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 38 ++++++++++++++-----
1 file changed, 29 insertions(+), 9 deletions(-)
@@ -23,14 +23,50 @@#include"mtk_vcodec_util.h"#include"mtk_vcodec_fw.h"-#define VDEC_HW_ACTIVE 0x10-#define VDEC_IRQ_CFG 0x11-#define VDEC_IRQ_CLR 0x10-#define VDEC_IRQ_CFG_REG 0xa4-module_param(mtk_v4l2_dbg_level,int,0644);module_param(mtk_vcodec_dbg,bool,0644);+staticstructof_device_idmtk_vdec_drv_ids[]={+{+.compatible="mediatek,mtk-vcodec-lat",+.data=(void*)MTK_VDEC_LAT0,+},+{+.compatible="mediatek,mtk-vcodec-core",+.data=(void*)MTK_VDEC_CORE,+},+{},+};++staticinlineintmtk_vdec_compare_of(structdevice*dev,void*data)+{+returndev->of_node==data;+}++staticinlinevoidmtk_vdec_release_of(structdevice*dev,void*data)+{+of_node_put(data);+}++staticinlineintmtk_vdec_bind(structdevice*dev)+{+structmtk_vcodec_dev*data=dev_get_drvdata(dev);++returncomponent_bind_all(dev,data);+}++staticinlinevoidmtk_vdec_unbind(structdevice*dev)+{+structmtk_vcodec_dev*data=dev_get_drvdata(dev);++component_unbind_all(dev,data);+}++staticconststructcomponent_master_opsmtk_vdec_ops={+.bind=mtk_vdec_bind,+.unbind=mtk_vdec_unbind,+};+/* Wake up context wait_queue */staticvoidwake_up_ctx(structmtk_vcodec_ctx*ctx){
@@ -38,6 +74,47 @@ static void wake_up_ctx(struct mtk_vcodec_ctx *ctx)wake_up_interruptible(&ctx->queue);}+staticstructcomponent_match*mtk_vcodec_match_add(+structmtk_vcodec_dev*vdec_dev)+{+structplatform_device*pdev=vdec_dev->plat_dev;+structcomponent_match*match=NULL;+inti;++for(i=0;i<ARRAY_SIZE(mtk_vdec_drv_ids);i++){+structdevice_node*comp_node;+enummtk_vdec_hw_idcomp_idx;+conststructof_device_id*of_id;++comp_node=of_find_compatible_node(NULL,NULL,+mtk_vdec_drv_ids[i].compatible);+if(!comp_node)+continue;++if(!of_device_is_available(comp_node)){+of_node_put(comp_node);+dev_err(&pdev->dev,"Fail to get MMSYS node\n");+continue;+}++of_id=of_match_node(mtk_vdec_drv_ids,comp_node);+if(!of_id){+dev_err(&pdev->dev,"Failed to get match node\n");+returnERR_PTR(-EINVAL);+}++comp_idx=(enummtk_vdec_hw_id)of_id->data;+mtk_v4l2_debug(4,"Get component:hw_id(%d),vdec_dev(0x%p),comp_node(0x%p)\n",+comp_idx,vdec_dev,comp_node);+vdec_dev->component_node[comp_idx]=comp_node;++component_match_add_release(&pdev->dev,&match,mtk_vdec_release_of,+mtk_vdec_compare_of,comp_node);+}++returnmatch;+}+staticirqreturn_tmtk_vcodec_dec_irq_handler(intirq,void*priv){structmtk_vcodec_dev*dev=priv;
@@ -103,6 +180,62 @@ static int mtk_vcodec_get_reg_bases(struct mtk_vcodec_dev *dev)returnret;}+staticintmtk_vcodec_init_master(structmtk_vcodec_dev*dev)+{+structplatform_device*pdev=dev->plat_dev;+structcomponent_match*match;+intret=0;++match=mtk_vcodec_match_add(dev);+if(IS_ERR_OR_NULL(match))+return-EINVAL;++platform_set_drvdata(pdev,dev);+ret=component_master_add_with_match(&pdev->dev,&mtk_vdec_ops,match);+if(ret<0)+returnret;++return0;+}++staticintmtk_vcodec_init_dec_params(structmtk_vcodec_dev*dev)+{+structplatform_device*pdev=dev->plat_dev;+structresource*res;+intret=0;++if(!dev->is_support_comp){+res=platform_get_resource(pdev,IORESOURCE_IRQ,0);+if(res==NULL){+dev_err(&pdev->dev,"failed to get irq resource");+return-ENOENT;+}++dev->dec_irq=platform_get_irq(dev->plat_dev,0);+irq_set_status_flags(dev->dec_irq,IRQ_NOAUTOEN);+ret=devm_request_irq(&pdev->dev,dev->dec_irq,+mtk_vcodec_dec_irq_handler,0,pdev->name,dev);+if(ret){+dev_err(&pdev->dev,"failed to install dev->dec_irq %d (%d)",+dev->dec_irq,+ret);+returnret;+}++ret=mtk_vcodec_init_dec_pm(dev->plat_dev,&dev->pm);+if(ret<0){+dev_err(&pdev->dev,"failed to get mt vcodec clock source");+returnret;+}+}++ret=mtk_vcodec_get_reg_bases(dev);+if(ret&&!dev->is_support_comp)+mtk_vcodec_release_dec_pm(&dev->pm);++returnret;+}+staticintfops_vcodec_open(structfile*file){structmtk_vcodec_dev*dev=video_drvdata(file);
@@ -228,7 +361,6 @@ static int mtk_vcodec_probe(struct platform_device *pdev){structmtk_vcodec_dev*dev;structvideo_device*vfd_dec;-structresource*res;phandlerproc_phandle;enummtk_vcodec_fw_typefw_type;intret;
@@ -257,31 +389,14 @@ static int mtk_vcodec_probe(struct platform_device *pdev)if(IS_ERR(dev->fw_handler))returnPTR_ERR(dev->fw_handler);-ret=mtk_vcodec_init_dec_pm(dev->plat_dev,&dev->pm);-if(ret<0){-dev_err(&pdev->dev,"Failed to get mt vcodec clock source");-gotoerr_dec_pm;-}--ret=mtk_vcodec_get_reg_bases(dev);-if(ret)-gotoerr_res;--res=platform_get_resource(pdev,IORESOURCE_IRQ,0);-if(res==NULL){-dev_err(&pdev->dev,"failed to get irq resource");-ret=-ENOENT;-gotoerr_res;-}+if(!of_find_compatible_node(NULL,NULL,"mediatek,mtk-vcodec-core"))+dev->is_support_comp=false;+else+dev->is_support_comp=true;-dev->dec_irq=platform_get_irq(pdev,0);-irq_set_status_flags(dev->dec_irq,IRQ_NOAUTOEN);-ret=devm_request_irq(&pdev->dev,dev->dec_irq,-mtk_vcodec_dec_irq_handler,0,pdev->name,dev);-if(ret){-dev_err(&pdev->dev,"Failed to install dev->dec_irq %d (%d)",-dev->dec_irq,-ret);+if(mtk_vcodec_init_dec_params(dev)){+dev_err(&pdev->dev,"Failed to init pm and registers");+ret=-EINVAL;gotoerr_res;}
@@ -319,7 +434,6 @@ static int mtk_vcodec_probe(struct platform_device *pdev)MTK_VCODEC_DEC_NAME);video_set_drvdata(vfd_dec,dev);dev->vfd_dec=vfd_dec;-platform_set_drvdata(pdev,dev);dev->m2m_dev_dec=v4l2_m2m_init(&mtk_vdec_m2m_ops);if(IS_ERR((__forcevoid*)dev->m2m_dev_dec)){
@@ -370,8 +484,17 @@ static int mtk_vcodec_probe(struct platform_device *pdev)mtk_v4l2_debug(0,"decoder registered as /dev/video%d",vfd_dec->num);-return0;+if(dev->is_support_comp){+ret=mtk_vcodec_init_master(dev);+if(ret<0)+gotoerr_component_match;+}else{+platform_set_drvdata(pdev,dev);+}+return0;+err_component_match:+video_unregister_device(vfd_dec);err_dec_reg:if(dev->vdec_pdata->uses_stateless_api)media_device_unregister(&dev->mdev_dec);
@@ -387,9 +510,8 @@ static int mtk_vcodec_probe(struct platform_device *pdev)err_dec_alloc:v4l2_device_unregister(&dev->v4l2_dev);err_res:-mtk_vcodec_release_dec_pm(&dev->pm);-err_dec_pm:mtk_vcodec_fw_release(dev->fw_handler);+returnret;}
@@ -0,0 +1,191 @@+// SPDX-License-Identifier: GPL-2.0+/*+*Copyright(c)2021MediaTekInc.+*Author:YunfeiDong<yunfei.dong@mediatek.com>+*/++#include<linux/interrupt.h>+#include<linux/irq.h>+#include<linux/module.h>+#include<linux/of.h>+#include<linux/of_device.h>+#include<linux/slab.h>++#include"mtk_vcodec_drv.h"+#include"mtk_vcodec_dec.h"+#include"mtk_vcodec_dec_hw.h"+#include"mtk_vcodec_dec_pm.h"+#include"mtk_vcodec_intr.h"+#include"mtk_vcodec_util.h"++staticintmtk_vdec_comp_bind(structdevice*dev,structdevice*master,+void*data)+{+structmtk_vdec_comp_dev*comp_dev=dev_get_drvdata(dev);+structmtk_vcodec_dev*master_dev=data;+inti;++for(i=0;i<MTK_VDEC_HW_MAX;i++){+if(dev->of_node!=master_dev->component_node[i])+continue;++master_dev->comp_dev[i]=comp_dev;+comp_dev->comp_idx=i;+comp_dev->master_dev=master_dev;+break;+}++if(i==MTK_VDEC_HW_MAX){+dev_err(dev,"Failed to get component node\n");+return-EINVAL;+}++comp_dev->reg_base[VDEC_COMP_SYS]=+master_dev->reg_base[VDEC_COMP_SYS];+return0;+}++staticvoidmtk_vdec_comp_unbind(structdevice*dev,structdevice*master,+void*data)+{+structmtk_vdec_comp_dev*comp_dev=dev_get_drvdata(dev);++comp_dev->reg_base[VDEC_COMP_SYS]=NULL;+}++staticconststructcomponent_opsmtk_vdec_hw_component_ops={+.bind=mtk_vdec_comp_bind,+.unbind=mtk_vdec_comp_unbind,+};++/* Wake up context wait_queue */+staticvoidmtk_vdec_comp_wake_up_ctx(structmtk_vcodec_ctx*ctx)+{+ctx->int_cond=1;+wake_up_interruptible(&ctx->queue);+}++staticirqreturn_tmtk_vdec_comp_irq_handler(intirq,void*priv)+{+structmtk_vdec_comp_dev*dev=priv;+structmtk_vcodec_ctx*ctx;+u32cg_status;+unsignedintdec_done_status;+void__iomem*vdec_misc_addr=dev->reg_base[VDEC_COMP_MISC]++VDEC_IRQ_CFG_REG;++ctx=mtk_vcodec_get_curr_ctx(dev->master_dev);++/* check if HW active or not */+cg_status=readl(dev->reg_base[VDEC_COMP_SYS]);+if((cg_status&VDEC_HW_ACTIVE)!=0){+mtk_v4l2_err("vdec active is not 0x0 (0x%08x)",+cg_status);+returnIRQ_HANDLED;+}++dec_done_status=readl(vdec_misc_addr);+if((dec_done_status&MTK_VDEC_IRQ_STATUS_DEC_SUCCESS)!=+MTK_VDEC_IRQ_STATUS_DEC_SUCCESS)+returnIRQ_HANDLED;++/* clear interrupt */+writel((readl(vdec_misc_addr)|VDEC_IRQ_CFG),vdec_misc_addr);+writel((readl(vdec_misc_addr)&~VDEC_IRQ_CLR),vdec_misc_addr);++mtk_vdec_comp_wake_up_ctx(ctx);++mtk_v4l2_debug(3,"wake up ctx %d, dec_done_status=%x",+ctx->id,dec_done_status);++returnIRQ_HANDLED;+}++staticintmtk_vdec_comp_init_irq(structmtk_vdec_comp_dev*dev)+{+structplatform_device*pdev=dev->plat_dev;+intret;++dev->dec_irq=platform_get_irq(pdev,0);+if(dev->dec_irq<0){+dev_err(&pdev->dev,"Failed to get irq resource");+returndev->dec_irq;+}++ret=devm_request_irq(&pdev->dev,dev->dec_irq,+mtk_vdec_comp_irq_handler,+0,pdev->name,dev);+if(ret){+dev_err(&pdev->dev,"Failed to install dev->dec_irq %d (%d)",+dev->dec_irq,ret);+return-ENOENT;+}++disable_irq(dev->dec_irq);+return0;+}++staticintmtk_vdec_comp_probe(structplatform_device*pdev)+{+structmtk_vdec_comp_dev*dev;+intret;++dev=devm_kzalloc(&pdev->dev,sizeof(*dev),GFP_KERNEL);+if(!dev)+return-ENOMEM;++dev->plat_dev=pdev;+spin_lock_init(&dev->irqlock);++ret=mtk_vcodec_init_dec_pm(dev->plat_dev,&dev->pm);+if(ret){+dev_err(&pdev->dev,"Failed to get mt vcodec clock source");+returnret;+}++dev->reg_base[VDEC_COMP_MISC]=+devm_platform_ioremap_resource(pdev,0);+if(IS_ERR((__forcevoid*)dev->reg_base[VDEC_COMP_MISC])){+ret=PTR_ERR((__forcevoid*)dev->reg_base[VDEC_COMP_MISC]);+gotoerr;+}++if(of_get_property(pdev->dev.of_node,"dma-ranges",NULL))+dma_set_mask_and_coherent(&pdev->dev,DMA_BIT_MASK(34));++ret=mtk_vdec_comp_init_irq(dev);+if(ret)+gotoerr;++platform_set_drvdata(pdev,dev);++ret=component_add(&pdev->dev,&mtk_vdec_hw_component_ops);+if(ret){+dev_err(&pdev->dev,"Failed to add component: %d\n",ret);+gotoerr;+}++return0;+err:+mtk_vcodec_release_dec_pm(&dev->pm);+returnret;+}++staticconststructof_device_idmtk_vdec_comp_ids[]={+{+.compatible="mediatek,mtk-vcodec-lat",+},+{+.compatible="mediatek,mtk-vcodec-core",+},+{},+};+MODULE_DEVICE_TABLE(of,mtk_vdec_comp_ids);++structplatform_drivermtk_vdec_comp_driver={+.probe=mtk_vdec_comp_probe,+.driver={+.name="mtk-vdec-comp",+.of_match_table=mtk_vdec_comp_ids,+},+};
Need a DT binding document patch for the attribute.
Does it really need to call of_find_compatible_node() for parsing an
attribute? If so, it needs to call of_node_put() afterward.
The newly added code in the file doesn't look like it needs anything
from component.h and io.h.
quoted hunk
@@ -404,6 +422,7 @@ struct mtk_vcodec_enc_pdata { * * @fw_handler: used to communicate with the firmware. * @id_counter: used to identify current opened instance+ * @is_support_comp: 1: using compoent framework, 0: not support
is_support_comp is a boolean. Use true and false instead of 1 and 0.
quoted hunk
@@ -422,6 +441,10 @@ struct mtk_vcodec_enc_pdata { * @pm: power management control * @dec_capability: used to identify decode capability, ex: 4k * @enc_capability: used to identify encode capability+ *+ * comp_dev: component hardware device+ * component_node: component node+ * comp_idx: component index
@@ -134,7 +134,7 @@ static irqreturn_t mtk_vcodec_dec_irq_handler(int irq, void *priv)void__iomem*vdec_misc_addr=dev->reg_base[VDEC_MISC]+VDEC_IRQ_CFG_REG;-ctx=mtk_vcodec_get_curr_ctx(dev);+ctx=mtk_vcodec_get_curr_ctx(dev,MTK_VDEC_CORE);/* check if HW active or not */cg_status=readl(dev->reg_base[0]);
@@ -299,7 +299,7 @@ static int fops_vcodec_open(struct file *file)mtk_vcodec_dec_set_default_params(ctx);if(v4l2_fh_is_singular(&ctx->fh)){-ret=mtk_vcodec_dec_pw_on(&dev->pm);+ret=mtk_vcodec_dec_pw_on(dev,MTK_VDEC_LAT0);if(ret<0)gotoerr_load_fw;/*
@@ -360,7 +360,7 @@ static int fops_vcodec_release(struct file *file)mtk_vcodec_dec_release(ctx);if(v4l2_fh_is_singular(&ctx->fh))-mtk_vcodec_dec_pw_off(&dev->pm);+mtk_vcodec_dec_pw_off(dev,MTK_VDEC_LAT0);v4l2_fh_del(&ctx->fh);v4l2_fh_exit(&ctx->fh);v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
@@ -75,7 +75,7 @@ static irqreturn_t mtk_vdec_comp_irq_handler(int irq, void *priv)void__iomem*vdec_misc_addr=dev->reg_base[VDEC_COMP_MISC]+VDEC_IRQ_CFG_REG;-ctx=mtk_vcodec_get_curr_ctx(dev->master_dev);+ctx=mtk_vcodec_get_curr_ctx(dev->master_dev,dev->comp_idx);/* check if HW active or not */cg_status=readl(dev->reg_base[VDEC_COMP_SYS]);
@@ -84,10 +86,23 @@ void mtk_vcodec_release_dec_pm(struct mtk_vcodec_pm *pm)put_device(pm->larbvdec);}-intmtk_vcodec_dec_pw_on(structmtk_vcodec_pm*pm)+intmtk_vcodec_dec_pw_on(structmtk_vcodec_dev*vdec_dev,intcomp_idx){+structmtk_vdec_comp_dev*comp_dev;+structmtk_vcodec_pm*pm;intret;+if(vdec_dev->is_support_comp){+comp_dev=mtk_vcodec_get_hw_dev(vdec_dev,comp_idx);+if(!comp_dev){+mtk_v4l2_err("Failed to get hw dev\n");+return-EINVAL;+}+pm=&comp_dev->pm;+}else{+pm=&vdec_dev->pm;+}+ret=pm_runtime_resume_and_get(pm->dev);if(ret)mtk_v4l2_err("pm_runtime_resume_and_get fail %d",ret);
@@ -95,20 +110,49 @@ int mtk_vcodec_dec_pw_on(struct mtk_vcodec_pm *pm)returnret;}-voidmtk_vcodec_dec_pw_off(structmtk_vcodec_pm*pm)+voidmtk_vcodec_dec_pw_off(structmtk_vcodec_dev*vdec_dev,intcomp_idx){+structmtk_vdec_comp_dev*comp_dev;+structmtk_vcodec_pm*pm;intret;+if(vdec_dev->is_support_comp){+comp_dev=mtk_vcodec_get_hw_dev(vdec_dev,comp_idx);+if(!comp_dev){+mtk_v4l2_err("Failed to get hw dev\n");+return;+}+pm=&comp_dev->pm;+}else{+pm=&vdec_dev->pm;+}+ret=pm_runtime_put_sync(pm->dev);if(ret)mtk_v4l2_err("pm_runtime_put_sync fail %d",ret);}-voidmtk_vcodec_dec_clock_on(structmtk_vcodec_pm*pm)+voidmtk_vcodec_dec_clock_on(structmtk_vcodec_dev*vdec_dev,intcomp_idx){-structmtk_vcodec_clk*dec_clk=&pm->vdec_clk;-intret,i=0;+structmtk_vdec_comp_dev*comp_dev;+structmtk_vcodec_pm*pm;+structmtk_vcodec_clk*dec_clk;+intret,i;++if(vdec_dev->is_support_comp){+comp_dev=mtk_vcodec_get_hw_dev(vdec_dev,comp_idx);+if(!comp_dev){+mtk_v4l2_err("Failed to get hw dev\n");+return;+}+pm=&comp_dev->pm;+enable_irq(comp_dev->dec_irq);+}else{+pm=&vdec_dev->pm;+enable_irq(vdec_dev->dec_irq);+}+dec_clk=&pm->vdec_clk;for(i=0;i<dec_clk->clk_num;i++){ret=clk_prepare_enable(dec_clk->clk_info[i].vcodec_clk);if(ret){
@@ -130,11 +174,27 @@ void mtk_vcodec_dec_clock_on(struct mtk_vcodec_pm *pm)clk_disable_unprepare(dec_clk->clk_info[i].vcodec_clk);}-voidmtk_vcodec_dec_clock_off(structmtk_vcodec_pm*pm)+voidmtk_vcodec_dec_clock_off(structmtk_vcodec_dev*vdec_dev,intcomp_idx){-structmtk_vcodec_clk*dec_clk=&pm->vdec_clk;-inti=0;+structmtk_vdec_comp_dev*comp_dev;+structmtk_vcodec_pm*pm;+structmtk_vcodec_clk*dec_clk;+inti;+if(vdec_dev->is_support_comp){+comp_dev=mtk_vcodec_get_hw_dev(vdec_dev,comp_idx);+if(!comp_dev){+mtk_v4l2_err("Failed to get hw dev\n");+return;+}+pm=&comp_dev->pm;+disable_irq(comp_dev->dec_irq);+}else{+pm=&vdec_dev->pm;+disable_irq(vdec_dev->dec_irq);+}++dec_clk=&pm->vdec_clk;mtk_smi_larb_put(pm->larbvdec);for(i=dec_clk->clk_num-1;i>=0;i--)clk_disable_unprepare(dec_clk->clk_info[i].vcodec_clk);
@@ -81,25 +84,87 @@ void mtk_vcodec_mem_free(struct mtk_vcodec_ctx *data,}EXPORT_SYMBOL(mtk_vcodec_mem_free);-voidmtk_vcodec_set_curr_ctx(structmtk_vcodec_dev*dev,-structmtk_vcodec_ctx*ctx)+void*mtk_vcodec_get_hw_dev(structmtk_vcodec_dev*dev,intcomp_idx){-unsignedlongflags;+structplatform_device*hw_pdev;+structdevice_node*node;+structmtk_vdec_comp_dev*master_dev;++if(comp_idx>=MTK_VDEC_HW_MAX||comp_idx<0){+mtk_v4l2_err("Comp idx is out of range:%d",comp_idx);+returnNULL;+}++if(dev->comp_dev[comp_idx])+returndev->comp_dev[comp_idx];++node=dev->component_node[comp_idx];+if(!node){+mtk_v4l2_err("Get lat node fail:%d",comp_idx);+returnNULL;+}++hw_pdev=of_find_device_by_node(node);+of_node_put(node);++if(WARN_ON(!hw_pdev)){+mtk_v4l2_err("Get hw id(%d) node fail",comp_idx);+returnNULL;+}++master_dev=platform_get_drvdata(hw_pdev);+if(!master_dev){+mtk_v4l2_err("Get hw id(%d) pdev fail",comp_idx);+returnNULL;+}-spin_lock_irqsave(&dev->irqlock,flags);-dev->curr_ctx=ctx;-spin_unlock_irqrestore(&dev->irqlock,flags);+dev->comp_dev[master_dev->comp_idx]=master_dev;+returnmaster_dev;+}+EXPORT_SYMBOL(mtk_vcodec_get_hw_dev);++voidmtk_vcodec_set_curr_ctx(structmtk_vcodec_dev*vdec_dev,+structmtk_vcodec_ctx*ctx,intcomp_idx)+{+unsignedlongflags;+structmtk_vdec_comp_dev*comp_dev;++spin_lock_irqsave(&vdec_dev->irqlock,flags);+if(vdec_dev->is_support_comp){+comp_dev=mtk_vcodec_get_hw_dev(vdec_dev,comp_idx);+if(!comp_dev){+mtk_v4l2_err("Failed to get hw dev");+spin_unlock_irqrestore(&vdec_dev->irqlock,flags);+return;+}+comp_dev->curr_ctx=ctx;+}else{+vdec_dev->curr_ctx=ctx;+}+spin_unlock_irqrestore(&vdec_dev->irqlock,flags);}EXPORT_SYMBOL(mtk_vcodec_set_curr_ctx);-structmtk_vcodec_ctx*mtk_vcodec_get_curr_ctx(structmtk_vcodec_dev*dev)+structmtk_vcodec_ctx*mtk_vcodec_get_curr_ctx(structmtk_vcodec_dev*vdec_dev,+unsignedintcomp_idx){unsignedlongflags;structmtk_vcodec_ctx*ctx;--spin_lock_irqsave(&dev->irqlock,flags);-ctx=dev->curr_ctx;-spin_unlock_irqrestore(&dev->irqlock,flags);+structmtk_vdec_comp_dev*comp_dev;++spin_lock_irqsave(&vdec_dev->irqlock,flags);+if(vdec_dev->is_support_comp){+comp_dev=mtk_vcodec_get_hw_dev(vdec_dev,comp_idx);+if(!comp_dev){+mtk_v4l2_err("Failed to get hw dev");+spin_unlock_irqrestore(&vdec_dev->irqlock,flags);+returnNULL;+}+ctx=comp_dev->curr_ctx;+}else{+ctx=vdec_dev->curr_ctx;+}+spin_unlock_irqrestore(&vdec_dev->irqlock,flags);returnctx;}EXPORT_SYMBOL(mtk_vcodec_get_curr_ctx);
@@ -0,0 +1,169 @@+Mediatek Video Decoder++Mediatek Video Decoder is the video decode hw present in Mediatek SoCs which+supports high resolution decoding functionalities. With component framework+require master and component node properties:++Required properties(no component framework):+- compatible : must be one of the following string:+ "mediatek,mt8173-vcodec-dec" for MT8173 decoder.+ "mediatek,mt8183-vcodec-dec" for MT8183 decoder.+- reg : Physical base address of the video decoder registers and length of+ memory mapped region.+- interrupts : interrupt number to the cpu.+- mediatek,larb : must contain the local arbiters in the current Socs.+- clocks : list of clock specifiers, corresponding to entries in+ the clock-names property.+- clock-names: must contain "vcodecpll", "univpll_d2", "clk_cci400_sel"+ "vdec_sel", "vdecpll", "vencpll", "venc_lt_sel", "vdec_bus_clk_src".+- iommus : should point to the respective IOMMU block with master port as+ argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt+ for details.+- dma-ranges : describes the dma address range space that the codec hw access.+One of the two following nodes:+- mediatek,vpu : the node of the video processor unit, if using VPU.+- mediatek,scp : the node of the SCP unit, if using SCP.++Master properties(component framework):+- compatible :+ "mediatek,mt8192-vcodec-dec" for MT8192 decoder.+- reg : Physical base address of the video decoder registers and length of+ memory mapped region.+- iommus : should point to the respective IOMMU block with master port as+ argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt+ for details.+One of the two following nodes:+- mediatek,vpu : the node of the video processor unit, if using VPU.+- mediatek,scp : the node of the SCP unit, if using SCP.++component properties(lat/core):+- compatible :+ "mediatek,mtk-vcodec-lat" for lat hardware decoder.+ "mediatek,mtk-vcodec-core" core hardware decoder+- reg : Physical base address of the video decoder registers and length of+ memory mapped region.+- interrupts : interrupt number to the cpu.+- clocks : list of clock specifiers, corresponding to entries in+ the clock-names property.+- clock-names: "vdec-soc-vdec", "vdec-soc-lat", "vdec-vdec", "vdec-top".+- iommus : should point to the respective IOMMU block with master port as+ argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt+ for details.+One of the two following nodes:+- mediatek,vpu : the node of the video processor unit, if using VPU.+- mediatek,scp : the node of the SCP unit, if using SCP.++Example:++No component framework:+vcodec_dec: vcodec@16000000 {+ compatible = "mediatek,mt8173-vcodec-dec";+ reg = <0 0x16000000 0 0x100>, /*VDEC_SYS*/+ <0 0x16020000 0 0x1000>, /*VDEC_MISC*/+ <0 0x16021000 0 0x800>, /*VDEC_LD*/+ <0 0x16021800 0 0x800>, /*VDEC_TOP*/+ <0 0x16022000 0 0x1000>, /*VDEC_CM*/+ <0 0x16023000 0 0x1000>, /*VDEC_AD*/+ <0 0x16024000 0 0x1000>, /*VDEC_AV*/+ <0 0x16025000 0 0x1000>, /*VDEC_PP*/+ <0 0x16026800 0 0x800>, /*VP8_VD*/+ <0 0x16027000 0 0x800>, /*VP6_VD*/+ <0 0x16027800 0 0x800>, /*VP8_VL*/+ <0 0x16028400 0 0x400>; /*VP9_VD*/+ interrupts = <GIC_SPI 204 IRQ_TYPE_LEVEL_LOW>;+ mediatek,larb = <&larb1>;+ iommus = <&iommu M4U_PORT_HW_VDEC_MC_EXT>,+ <&iommu M4U_PORT_HW_VDEC_PP_EXT>,+ <&iommu M4U_PORT_HW_VDEC_AVC_MV_EXT>,+ <&iommu M4U_PORT_HW_VDEC_PRED_RD_EXT>,+ <&iommu M4U_PORT_HW_VDEC_PRED_WR_EXT>,+ <&iommu M4U_PORT_HW_VDEC_UFO_EXT>,+ <&iommu M4U_PORT_HW_VDEC_VLD_EXT>,+ <&iommu M4U_PORT_HW_VDEC_VLD2_EXT>;+ mediatek,vpu = <&vpu>;+ power-domains = <&scpsys MT8173_POWER_DOMAIN_VDEC>;+ clocks = <&apmixedsys CLK_APMIXED_VCODECPLL>,+ <&topckgen CLK_TOP_UNIVPLL_D2>,+ <&topckgen CLK_TOP_CCI400_SEL>,+ <&topckgen CLK_TOP_VDEC_SEL>,+ <&topckgen CLK_TOP_VCODECPLL>,+ <&apmixedsys CLK_APMIXED_VENCPLL>,+ <&topckgen CLK_TOP_VENC_LT_SEL>,+ <&topckgen CLK_TOP_VCODECPLL_370P5>;+ clock-names = "vcodecpll",+ "univpll_d2",+ "clk_cci400_sel",+ "vdec_sel",+ "vdecpll",+ "vencpll",+ "venc_lt_sel",+ "vdec_bus_clk_src";+ assigned-clocks = <&topckgen CLK_TOP_VENC_LT_SEL>,+ <&topckgen CLK_TOP_CCI400_SEL>,+ <&topckgen CLK_TOP_VDEC_SEL>,+ <&apmixedsys CLK_APMIXED_VCODECPLL>,+ <&apmixedsys CLK_APMIXED_VENCPLL>;+ assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL_370P5>,+ <&topckgen CLK_TOP_UNIVPLL_D2>,+ <&topckgen CLK_TOP_VCODECPLL>;+ assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>;+ };++With component framework:+vcodec_dec: vcodec_dec@16000000 {+ compatible = "mediatek,mt8192-vcodec-dec";+ reg = <0 0x16000000 0 0x1000>; /* VDEC_SYS */+ mediatek,scp = <&scp>;+ iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>;+ };++vcodec_lat: vcodec_lat@0x16010000 {+ compatible = "mediatek,mtk-vcodec-lat";+ reg = <0 0x16010000 0 0x800>; /* VDEC_MISC */+ interrupts = <GIC_SPI 426 IRQ_TYPE_LEVEL_HIGH 0>;+ iommus = <&iommu0 M4U_PORT_L5_VDEC_LAT0_VLD_EXT>,+ <&iommu0 M4U_PORT_L5_VDEC_LAT0_VLD2_EXT>,+ <&iommu0 M4U_PORT_L5_VDEC_LAT0_AVC_MV_EXT>,+ <&iommu0 M4U_PORT_L5_VDEC_LAT0_PRED_RD_EXT>,+ <&iommu0 M4U_PORT_L5_VDEC_LAT0_TILE_EXT>,+ <&iommu0 M4U_PORT_L5_VDEC_LAT0_WDMA_EXT>,+ <&iommu0 M4U_PORT_L5_VDEC_LAT0_RG_CTRL_DMA_EXT>,+ <&iommu0 M4U_PORT_L5_VDEC_UFO_ENC_EXT>;+ clocks = <&topckgen CLK_TOP_VDEC_SEL>,+ <&vdecsys_soc CLK_VDEC_SOC_VDEC>,+ <&vdecsys_soc CLK_VDEC_SOC_LAT>,+ <&vdecsys_soc CLK_VDEC_SOC_LARB1>,+ <&topckgen CLK_TOP_MAINPLL_D4>;+ clock-names = "vdec-sel", "vdec-soc-vdec", "vdec-soc-lat",+ "vdec-vdec", "vdec-top";+ assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>;+ assigned-clock-parents = <&topckgen CLK_TOP_MAINPLL_D4>;+ power-domains = <&spm MT8192_POWER_DOMAIN_VDEC>;+ };++vcodec_core: vcodec_core@0x16025000 {+ compatible = "mediatek,mtk-vcodec-core";+ reg = <0 0x16025000 0 0x1000>; /* VDEC_CORE_MISC */+ interrupts = <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH 0>;+ iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>,+ <&iommu0 M4U_PORT_L4_VDEC_UFO_EXT>,+ <&iommu0 M4U_PORT_L4_VDEC_PP_EXT>,+ <&iommu0 M4U_PORT_L4_VDEC_PRED_RD_EXT>,+ <&iommu0 M4U_PORT_L4_VDEC_PRED_WR_EXT>,+ <&iommu0 M4U_PORT_L4_VDEC_PPWRAP_EXT>,+ <&iommu0 M4U_PORT_L4_VDEC_TILE_EXT>,+ <&iommu0 M4U_PORT_L4_VDEC_VLD_EXT>,+ <&iommu0 M4U_PORT_L4_VDEC_VLD2_EXT>,+ <&iommu0 M4U_PORT_L4_VDEC_AVC_MV_EXT>,+ <&iommu0 M4U_PORT_L4_VDEC_RG_CTRL_DMA_EXT>;+ clocks = <&topckgen CLK_TOP_VDEC_SEL>,+ <&vdecsys CLK_VDEC_VDEC>,+ <&vdecsys CLK_VDEC_LAT>,+ <&vdecsys CLK_VDEC_LARB1>,+ <&topckgen CLK_TOP_MAINPLL_D4>;+ clock-names = "vdec-sel", "vdec-soc-vdec", "vdec-soc-lat",+ "vdec-vdec", "vdec-top";+ assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>;+ assigned-clock-parents = <&topckgen CLK_TOP_MAINPLL_D4>;+ power-domains = <&spm MT8192_POWER_DOMAIN_VDEC2>;+ };
@@ -0,0 +1,73 @@+Mediatek Video Encoder++Mediatek Video Encoder is the video encode hw present in Mediatek SoCs which+supports high resolution encoding functionalities.++Required properties:+- compatible : must be one of the following string:+ "mediatek,mt8173-vcodec-enc-vp8" for mt8173 vp8 encoder.+ "mediatek,mt8173-vcodec-enc" for mt8173 avc encoder.+ "mediatek,mt8183-vcodec-enc" for MT8183 encoder.+ "mediatek,mt8192-vcodec-enc" for MT8192 encoder.+- reg : Physical base address of the video encoder registers and length of+ memory mapped region.+- interrupts : interrupt number to the cpu.+- mediatek,larb : must contain the local arbiters in the current Socs.+- clocks : list of clock specifiers, corresponding to entries in+ the clock-names property.+- clock-names: avc encoder must contain "venc_sel", vp8 encoder must+ contain "venc_lt_sel".+- iommus : should point to the respective IOMMU block with master port as+ argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt+ for details.+- dma-ranges : describes the dma address range space that the codec hw access.+One of the two following nodes:+- mediatek,vpu : the node of the video processor unit, if using VPU.+- mediatek,scp : the node of the SCP unit, if using SCP.+++Example:++vcodec_enc_avc: vcodec@18002000 {+ compatible = "mediatek,mt8173-vcodec-enc";+ reg = <0 0x18002000 0 0x1000>;+ interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;+ iommus = <&iommu M4U_PORT_VENC_RCPU>,+ <&iommu M4U_PORT_VENC_REC>,+ <&iommu M4U_PORT_VENC_BSDMA>,+ <&iommu M4U_PORT_VENC_SV_COMV>,+ <&iommu M4U_PORT_VENC_RD_COMV>,+ <&iommu M4U_PORT_VENC_CUR_LUMA>,+ <&iommu M4U_PORT_VENC_CUR_CHROMA>,+ <&iommu M4U_PORT_VENC_REF_LUMA>,+ <&iommu M4U_PORT_VENC_REF_CHROMA>,+ <&iommu M4U_PORT_VENC_NBM_RDMA>,+ <&iommu M4U_PORT_VENC_NBM_WDMA>;+ mediatek,larb = <&larb3>;+ mediatek,vpu = <&vpu>;+ clocks = <&topckgen CLK_TOP_VENC_SEL>;+ clock-names = "venc_sel";+ assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>;+ assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL>;+ };++vcodec_enc_vp8: vcodec@19002000 {+ compatible = "mediatek,mt8173-vcodec-enc-vp8";+ reg = <0 0x19002000 0 0x1000>; /* VENC_LT_SYS */+ interrupts = <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;+ iommus = <&iommu M4U_PORT_VENC_RCPU_SET2>,+ <&iommu M4U_PORT_VENC_REC_FRM_SET2>,+ <&iommu M4U_PORT_VENC_BSDMA_SET2>,+ <&iommu M4U_PORT_VENC_SV_COMA_SET2>,+ <&iommu M4U_PORT_VENC_RD_COMA_SET2>,+ <&iommu M4U_PORT_VENC_CUR_LUMA_SET2>,+ <&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>,+ <&iommu M4U_PORT_VENC_REF_LUMA_SET2>,+ <&iommu M4U_PORT_VENC_REC_CHROMA_SET2>;+ mediatek,larb = <&larb5>;+ mediatek,vpu = <&vpu>;+ clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;+ clock-names = "venc_lt_sel";+ assigned-clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;+ assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL_370P5>;+ };
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
The patch is weird. Its title says "separate" but the changes are all
creating new content.
Would expect the patch to remove content from some files (e.g.
Documentation/devicetree/bindings/media/mediatek-vcodec.txt) and
separate into 2 files.
Also would expect the patch is a refactor which shouldn't introduce
any new things.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
The patch is weird. Its title says "separate" but the changes are all
creating new content.
Would expect the patch to remove content from some files (e.g.
Documentation/devicetree/bindings/media/mediatek-vcodec.txt) and
separate into 2 files.
Except h/w doesn't change. Using the component framework is not a reason
to change bindings.
Also, note that any major changes or additions should be in schema
format now.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
For add new hardware, not only need to lock lat hardware, also
need to lock core hardware in case of different instance start
to decoder at the same time.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c | 4 ++--
drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 5 +++--
drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h | 2 +-
3 files changed, 6 insertions(+), 5 deletions(-)
Use the dma_set_mask_and_coherent helper to set vdec
DMA bit mask to support 34bits iova space(16GB) that
the mt8192 iommu HW support.
Whole the iova range separate to 0~4G/4G~8G/8G~12G/12G~16G,
regarding which iova range VDEC actually locate, it
depends on the dma-ranges property of vdec dtsi node.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 3 +++
1 file changed, 3 insertions(+)
For lat and core architecture, lat thread will send message to core
thread when lat decode done. Core hardware will use the message
from lat to decode, then free message to lat thread when decode done.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
drivers/media/platform/mtk-vcodec/Makefile | 1 +
.../platform/mtk-vcodec/mtk_vcodec_drv.h | 15 ++
.../platform/mtk-vcodec/vdec_msg_queue.c | 234 ++++++++++++++++++
.../platform/mtk-vcodec/vdec_msg_queue.h | 130 ++++++++++
4 files changed, 380 insertions(+)
create mode 100644 drivers/media/platform/mtk-vcodec/vdec_msg_queue.c
create mode 100644 drivers/media/platform/mtk-vcodec/vdec_msg_queue.h
+bool vdec_msg_queue_wait_lat_buf_full(struct vdec_msg_queue *msg_queue)
+{
+ long timeout_jiff;
+ int ret, i;
+
+ for (i = 0; i < NUM_BUFFER_COUNT + 2; i++) {
+ timeout_jiff = msecs_to_jiffies(1000);
+ ret = wait_event_timeout(msg_queue->lat_read,
+ msg_queue->num_lat == NUM_BUFFER_COUNT, timeout_jiff);
+ if (ret) {
+ mtk_v4l2_debug(3, "success to get lat buf: %d",
+ msg_queue->num_lat);
+ return true;
+ }
+ }
Why does it need the loop? i is unused.
+void vdec_msg_queue_deinit(
+ struct mtk_vcodec_ctx *ctx,
+ struct vdec_msg_queue *msg_queue)
+{
+ struct vdec_lat_buf *lat_buf;
+ struct mtk_vcodec_mem *mem;
+ int i;
+
+ mem = &msg_queue->wdma_addr;
+ if (mem->va)
+ mtk_vcodec_mem_free(ctx, mem);
+ for (i = 0; i < NUM_BUFFER_COUNT; i++) {
+ lat_buf = &msg_queue->lat_buf[i];
+
+ mem = &lat_buf->wdma_err_addr;
+ if (mem->va)
+ mtk_vcodec_mem_free(ctx, mem);
+
+ mem = &lat_buf->slice_bc_addr;
+ if (mem->va)
+ mtk_vcodec_mem_free(ctx, mem);
+
+ if (lat_buf->private_data)
+ kfree(lat_buf->private_data);
+ }
+
+ msg_queue->init_done = false;
Have no idea what init_done does in the code. It is not included in
any branch condition.
+/**
+ * vdec_msg_queue_init - init lat buffer information.
+ * @ctx: v4l2 ctx
+ * @msg_queue: used to store the lat buffer information
+ * @core_decode: core decode callback for each codec
+ * @private_size: the private data size used to share with core
+ */
+int vdec_msg_queue_init(
+ struct mtk_vcodec_ctx *ctx,
+ struct vdec_msg_queue *msg_queue,
+ core_decode_cb_t core_decode,
+ int private_size);
Would prefer to have *msg_queue as the first argument (also applies to
all operators of vdec_msg_queue).
+/**
+ * vdec_msg_queue_get_core_buf - get used core buffer for lat decode.
+ * @dev: mtk vcodec device
+ */
+struct vdec_lat_buf *vdec_msg_queue_get_core_buf(
+ struct mtk_vcodec_dev *dev);
This is weird: vdec_msg_queue's operator but manipulating mtk_vcodec_dev?
+
+/**
+ * vdec_msg_queue_buf_to_core - queue buf to the core for core decode.
+ * @dev: mtk vcodec device
+ * @buf: current lat buffer
+ */
+void vdec_msg_queue_buf_to_core(struct mtk_vcodec_dev *dev,
+ struct vdec_lat_buf *buf);
Also weird.
+/**
+ * vdec_msg_queue_buf_to_lat - queue buf to lat for lat decode.
+ * @buf: current lat buffer
+ */
+void vdec_msg_queue_buf_to_lat(struct vdec_lat_buf *buf);
It should at least accept a struct vdec_msg_queue argument (or which
msg queue should the buf put into?).
+/**
+ * vdec_msg_queue_update_ube_rptr - used to updata the ube read point.
Typo.
+/**
+ * vdec_msg_queue_update_ube_wptr - used to updata the ube write point.
Typo.
+/**
+ * vdec_msg_queue_deinit - deinit lat buffer information.
+ * @ctx: v4l2 ctx
+ * @msg_queue: used to store the lat buffer information
+ */
+void vdec_msg_queue_deinit(
+ struct mtk_vcodec_ctx *ctx,
+ struct vdec_msg_queue *msg_queue);
Would prefer to have *msg_queue as the first argument.
The position of struct vdec_msg_queue is weird. It looks like the msg
queue is only for struct vdec_lat_buf. If so, would vdec_msg_queue be
better to call vdec_lat_queue or something similar?
It shouldn't touch the core queue in mtk_vcodec_dev anyway. Is it
possible to generalize the queue-related code for both lat and core
queues?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Tzung-Bi,
Thanks for your detail feedback.
I add the description according to your each comments.
On Fri, 2021-07-09 at 17:39 +0800, Tzung-Bi Shih wrote:
On Wed, Jul 7, 2021 at 2:22 PM Yunfei Dong [off-list ref] wrote:
quoted
@@ -464,6 +469,11 @@ struct mtk_vcodec_enc_pdata { * comp_dev: component hardware device * component_node: component node * comp_idx: component index+ *+ * core_read: Wait queue used to signalize when core get useful lat buffer+ * core_queue: List of V4L2 lat_buf
To be neat, replace "Wait" to "wait" and "List" to "list".
Doesn't it need to call mtk_vcodec_mem_free() and kfree() for any failure paths?
When allocate memory fail, will call deinit function auto, then free all allocated memory.
quoted
+struct vdec_lat_buf *vdec_msg_queue_get_core_buf(
+ struct mtk_vcodec_dev *dev)
+{
+ struct vdec_lat_buf *buf;
+ int ret;
+
+ spin_lock(&dev->core_lock);
+ if (list_empty(&dev->core_queue)) {
+ mtk_v4l2_debug(3, "core queue is NULL, num_core = %d", dev->num_core);
+ spin_unlock(&dev->core_lock);
+ ret = wait_event_freezable(dev->core_read,
+ !list_empty(&dev->core_queue));
+ if (ret)
+ return NULL;
Should be !ret?
According the definidtion, when condition is true, return value is 0.
#define wait_event_freezable(wq_head, condition) \
({ \
int __ret = 0; \
might_sleep(); \
if (!(condition)) \
__ret = __wait_event_freezable(wq_head, condition); \
__ret; \
})
+bool vdec_msg_queue_wait_lat_buf_full(struct vdec_msg_queue *msg_queue)
+{
+ long timeout_jiff;
+ int ret, i;
+
+ for (i = 0; i < NUM_BUFFER_COUNT + 2; i++) {
+ timeout_jiff = msecs_to_jiffies(1000);
+ ret = wait_event_timeout(msg_queue->lat_read,
+ msg_queue->num_lat == NUM_BUFFER_COUNT, timeout_jiff);
+ if (ret) {
+ mtk_v4l2_debug(3, "success to get lat buf: %d",
+ msg_queue->num_lat);
+ return true;
+ }
+ }
Why does it need the loop? i is unused.
Core maybe decode timeout, need to wait all core buffer process
completely.
quoted
+void vdec_msg_queue_deinit(
+ struct mtk_vcodec_ctx *ctx,
+ struct vdec_msg_queue *msg_queue)
+{
+ struct vdec_lat_buf *lat_buf;
+ struct mtk_vcodec_mem *mem;
+ int i;
+
+ mem = &msg_queue->wdma_addr;
+ if (mem->va)
+ mtk_vcodec_mem_free(ctx, mem);
+ for (i = 0; i < NUM_BUFFER_COUNT; i++) {
+ lat_buf = &msg_queue->lat_buf[i];
+
+ mem = &lat_buf->wdma_err_addr;
+ if (mem->va)
+ mtk_vcodec_mem_free(ctx, mem);
+
+ mem = &lat_buf->slice_bc_addr;
+ if (mem->va)
+ mtk_vcodec_mem_free(ctx, mem);
+
+ if (lat_buf->private_data)
+ kfree(lat_buf->private_data);
+ }
+
+ msg_queue->init_done = false;
Have no idea what init_done does in the code. It is not included in
any branch condition.
When call vdec_msg_queue_init will set this parameter to true.
quoted
+/**
+ * vdec_msg_queue_init - init lat buffer information.
+ * @ctx: v4l2 ctx
+ * @msg_queue: used to store the lat buffer information
+ * @core_decode: core decode callback for each codec
+ * @private_size: the private data size used to share with core
+ */
+int vdec_msg_queue_init(
+ struct mtk_vcodec_ctx *ctx,
+ struct vdec_msg_queue *msg_queue,
+ core_decode_cb_t core_decode,
+ int private_size);
Would prefer to have *msg_queue as the first argument (also applies to
all operators of vdec_msg_queue).
Can fix.
quoted
+/**
+ * vdec_msg_queue_get_core_buf - get used core buffer for lat decode.
+ * @dev: mtk vcodec device
+ */
+struct vdec_lat_buf *vdec_msg_queue_get_core_buf(
+ struct mtk_vcodec_dev *dev);
This is weird: vdec_msg_queue's operator but manipulating mtk_vcodec_dev?
vdec_msg_queue is used to share message between lat and core, for each
instance has its lat msg queue list, but all instance share one core msg
queue list. When try to get core buffer need to get it from core queue
list. Then queue it to lat queue list when core decode done.
quoted
+
+/**
+ * vdec_msg_queue_buf_to_core - queue buf to the core for core decode.
+ * @dev: mtk vcodec device
+ * @buf: current lat buffer
+ */
+void vdec_msg_queue_buf_to_core(struct mtk_vcodec_dev *dev,
+ struct vdec_lat_buf *buf);
Also weird.
quoted
+/**
+ * vdec_msg_queue_buf_to_lat - queue buf to lat for lat decode.
+ * @buf: current lat buffer
+ */
+void vdec_msg_queue_buf_to_lat(struct vdec_lat_buf *buf);
It should at least accept a struct vdec_msg_queue argument (or which
msg queue should the buf put into?).
All buffer is struct vdec_lat_buf, used to share info between lat and core queue list.
quoted
+/**
+ * vdec_msg_queue_update_ube_rptr - used to updata the ube read point.
Typo.
quoted
+/**
+ * vdec_msg_queue_update_ube_wptr - used to updata the ube write point.
Typo.
quoted
+/**
+ * vdec_msg_queue_deinit - deinit lat buffer information.
+ * @ctx: v4l2 ctx
+ * @msg_queue: used to store the lat buffer information
+ */
+void vdec_msg_queue_deinit(
+ struct mtk_vcodec_ctx *ctx,
+ struct vdec_msg_queue *msg_queue);
Would prefer to have *msg_queue as the first argument.
Yes, can fix.
The position of struct vdec_msg_queue is weird. It looks like the msg
queue is only for struct vdec_lat_buf. If so, would vdec_msg_queue be
better to call vdec_lat_queue or something similar?
It shouldn't touch the core queue in mtk_vcodec_dev anyway. Is it
possible to generalize the queue-related code for both lat and core
queues?
Lat queue list is separately for each instance, but only has one core
queue list.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Jul 12, 2021 at 3:28 PM mtk12024 [off-list ref] wrote:
On Fri, 2021-07-09 at 17:39 +0800, Tzung-Bi Shih wrote:
quoted
On Wed, Jul 7, 2021 at 2:22 PM Yunfei Dong [off-list ref] wrote:
Doesn't it need to call mtk_vcodec_mem_free() and kfree() for any failure paths?
When allocate memory fail, will call deinit function auto, then free all allocated memory.
I guess you mean: if vdec_msg_queue_init() fails,
vdec_msg_queue_deinit() should be called?
If so:
- It is not "auto". It depends on callers to invoke _deinit() if _init() fails.
- The API usage would be a bit weird: if the object hasn't been
initialized, shall we de-initialize it?
quoted
quoted
+struct vdec_lat_buf *vdec_msg_queue_get_core_buf(
+ struct mtk_vcodec_dev *dev)
+{
+ struct vdec_lat_buf *buf;
+ int ret;
+
+ spin_lock(&dev->core_lock);
+ if (list_empty(&dev->core_queue)) {
+ mtk_v4l2_debug(3, "core queue is NULL, num_core = %d", dev->num_core);
+ spin_unlock(&dev->core_lock);
+ ret = wait_event_freezable(dev->core_read,
+ !list_empty(&dev->core_queue));
+ if (ret)
+ return NULL;
Should be !ret?
According the definidtion, when condition is true, return value is 0.
Yeah, you're right. I was confused a bit with wait_event_timeout().
quoted
quoted
+bool vdec_msg_queue_wait_lat_buf_full(struct vdec_msg_queue *msg_queue)
+{
+ long timeout_jiff;
+ int ret, i;
+
+ for (i = 0; i < NUM_BUFFER_COUNT + 2; i++) {
+ timeout_jiff = msecs_to_jiffies(1000);
+ ret = wait_event_timeout(msg_queue->lat_read,
+ msg_queue->num_lat == NUM_BUFFER_COUNT, timeout_jiff);
+ if (ret) {
+ mtk_v4l2_debug(3, "success to get lat buf: %d",
+ msg_queue->num_lat);
+ return true;
+ }
+ }
Why does it need the loop? i is unused.
Core maybe decode timeout, need to wait all core buffer process
completely.
The point is: the i is unused. If it needs more time to complete,
could it just wait for (NUM_BUFFER_COUNT + 2) * 1000 msecs?
quoted
quoted
+ msg_queue->init_done = false;
Have no idea what init_done does in the code. It is not included in
any branch condition.
When call vdec_msg_queue_init will set this parameter to true.
The point is: if init_done doesn't change any code branch but just a
flag, does it really need the flag?
For example usages:
- If see the msg_queue->init_done has already been set to true in
vdec_msg_queue_init(), return errors.
- If see the msg_queue->init_done has already been set to false in
vdec_msg_queue_deinit(), return errors.
In the cases, I believe it brings very limited benefit (i.e. the
msg_queue is likely to _init and _deinit only once).
quoted
quoted
+/**
+ * vdec_msg_queue_get_core_buf - get used core buffer for lat decode.
+ * @dev: mtk vcodec device
+ */
+struct vdec_lat_buf *vdec_msg_queue_get_core_buf(
+ struct mtk_vcodec_dev *dev);
This is weird: vdec_msg_queue's operator but manipulating mtk_vcodec_dev?
vdec_msg_queue is used to share message between lat and core, for each
instance has its lat msg queue list, but all instance share one core msg
queue list. When try to get core buffer need to get it from core queue
list. Then queue it to lat queue list when core decode done.
I guess you mean: during runtime, it has n lat queues and 1 core queue.
If so, would it be intuitive and simple by:
msg_queue *core_q;
msg_queue *lat_q[LAT_N];
vdec_msg_queue_dequeue(core_q) if it wants to get from core queue.
vdec_msg_queue_enqueue(lat_q[X], data) if it wants to put data to lat queue X.
quoted
quoted
+/**
+ * vdec_msg_queue_buf_to_lat - queue buf to lat for lat decode.
+ * @buf: current lat buffer
+ */
+void vdec_msg_queue_buf_to_lat(struct vdec_lat_buf *buf);
It should at least accept a struct vdec_msg_queue argument (or which
msg queue should the buf put into?).
All buffer is struct vdec_lat_buf, used to share info between lat and core queue list.
The API semantic needs to provide a way to specify which msg_queue the
buf would put into.
quoted
The position of struct vdec_msg_queue is weird. It looks like the msg
queue is only for struct vdec_lat_buf. If so, would vdec_msg_queue be
better to call vdec_lat_queue or something similar?
It shouldn't touch the core queue in mtk_vcodec_dev anyway. Is it
possible to generalize the queue-related code for both lat and core
queues?
Lat queue list is separately for each instance, but only has one core
queue list.
Suggested to generalize the vdec_msg_queue to handle both lat and core
(and maybe furthermore). See comment above.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
@@ -43,3 +43,33 @@ int mtk_vcodec_wait_for_done_ctx(struct mtk_vcodec_ctx *ctx, int command,returnstatus;}EXPORT_SYMBOL(mtk_vcodec_wait_for_done_ctx);++intmtk_vcodec_wait_for_comp_done_ctx(structmtk_vcodec_ctx*ctx,+intcommand,unsignedinttimeout_ms,unsignedhw_id)+{+longtimeout_jiff,ret;+intstatus=0;++timeout_jiff=msecs_to_jiffies(timeout_ms);+ret=wait_event_interruptible_timeout(ctx->core_queue[hw_id],+ctx->int_core_cond[hw_id],+timeout_jiff);++if(!ret){+status=-1;/* timeout */+mtk_v4l2_err("[%d] cmd=%d, type=%d, dec timeout=%ums (%d %d)",+ctx->id,command,ctx->type,timeout_ms,+ctx->int_core_cond[hw_id],ctx->int_core_type[hw_id]);+}elseif(-ERESTARTSYS==ret){+status=-1;+mtk_v4l2_err("[%d] cmd=%d, type=%d, dec inter fail (%d %d)",+ctx->id,command,ctx->type,+ctx->int_core_cond[hw_id],ctx->int_core_type[hw_id]);+}++ctx->int_core_cond[hw_id]=0;+ctx->int_core_type[hw_id]=0;++returnstatus;+}+EXPORT_SYMBOL(mtk_vcodec_wait_for_comp_done_ctx);
Hi Tzung-Bi,
Thanks for your detail feedback.
I add the description according to your each comments.
On Fri, 2021-07-09 at 15:59 +0800, Tzung-Bi Shih wrote:
On Wed, Jul 7, 2021 at 2:22 PM Yunfei Dong [off-list ref] wrote:
quoted
+static int mtk_vcodec_get_hw_count(struct mtk_vcodec_dev *dev)
+{
+ if (dev->vdec_pdata->hw_arch == MTK_VDEC_PURE_SINGLE_CORE)
+ return 1;
+ else if (dev->vdec_pdata->hw_arch == MTK_VDEC_LAT_SINGLE_CORE)
+ return 2;
+ else
+ return 0;
+}
Use a switch .. case .. would be easier to read.
Yes
Would it be better to use some macro or enums for the magic numbers?
There are just one core thread, in order to separeate different
hardware, using codec type to separeate it in scp driver.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../media/platform/mtk-vcodec/vdec_ipi_msg.h | 12 ++++---
.../media/platform/mtk-vcodec/vdec_vpu_if.c | 34 ++++++++++++++++---
.../media/platform/mtk-vcodec/vdec_vpu_if.h | 4 +++
3 files changed, 41 insertions(+), 9 deletions(-)
@@ -257,7 +257,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)if(IS_ERR(dev->fw_handler))returnPTR_ERR(dev->fw_handler);-ret=mtk_vcodec_init_dec_pm(dev);+ret=mtk_vcodec_init_dec_pm(dev->plat_dev,&dev->pm);if(ret<0){dev_err(&pdev->dev,"Failed to get mt vcodec clock source");gotoerr_dec_pm;
@@ -387,7 +387,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)err_dec_alloc:v4l2_device_unregister(&dev->v4l2_dev);err_res:-mtk_vcodec_release_dec_pm(dev);+mtk_vcodec_release_dec_pm(&dev->pm);err_dec_pm:mtk_vcodec_fw_release(dev->fw_handler);returnret;
@@ -430,7 +430,7 @@ static int mtk_vcodec_dec_remove(struct platform_device *pdev)video_unregister_device(dev->vfd_dec);v4l2_device_unregister(&dev->v4l2_dev);-mtk_vcodec_release_dec_pm(dev);+mtk_vcodec_release_dec_pm(&dev->pm);mtk_vcodec_fw_release(dev->fw_handler);return0;}