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~3 rewrite get register bases and power on/off interface.
Patch 4 add component framework to support multi hardware.
Patch 5 separate video encoder and decoder document
Patches 6-15 add interfaces to support core hardware.
----
This patch dependents on : "media: mtk-vcodec: support for MT8183 decoder"[1] and
"Mediatek MT8192 clock support"[2].
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].
2: The definition of decoder clocks are in mt8192-clk.h, need to include them in
case of build fail [2].
[1]https://patchwork.linuxtv.org/project/linux-media/list/?series=6057
[2]https://patchwork.kernel.org/project/linux-mediatek/list/?series=511175
----
Changes compared with v4:
- Fix comments for patch 4/15
>> + if (dev->is_comp_supported) {
>> + ret = mtk_vcodec_init_master(dev);
>> + if (ret < 0)
>> + goto err_component_match;
>> + } else {
>> + platform_set_drvdata(pdev, dev);
>> + }
Fix platform_set_drvdata.
- Fix build error for patch 9/15
- Add depend patch in case of error header file for patch 13/15
Changes compared with v3:
- Fix return value for patch 1/15
- Fix comments for patch 4/15
> Looking up "mediatek,mtk-vcodec-core" to determine if it uses component framwork sounds like...
Add prameter in pdata, for all platform will use compoent after mt8183
>> + if (dev->is_comp_supported) {
>> + ret = mtk_vcodec_init_master(dev);
>> + if (ret < 0)
>> + goto err_component_match;
>> + } else {
>> + platform_set_drvdata(pdev, dev);
>> + }
> + Has asked the same question in [1]. Why it removes the
> +platform_set_drvdata() above? mtk_vcodec_init_master() also calls platform_set_drvdata().
Must call component_master_add_with_match after platform_set_drvdata for component architecture.
- Fix yaml files check fail for patch 5/15
- Fix yaml file check fail for patch 14/15
Changes compared with v1:
- Fix many comments for patch 3/14
- Remove unnecessary code for patch 4/14
- Using enum mtk_vdec_hw_count instead of magic numbers for patch 6/14
- Reconstructed get/put lat buffer for lat and core hardware for patch 7/14
- Using yaml format to instead of txt file for patch 12/14
Yunfei Dong (15):
media: mtk-vcodec: Get numbers of register bases from DT
media: mtk-vcodec: Align vcodec wake up interrupt interface
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 multi 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.yaml | 172 +++++++++++
.../media/mediatek,vcodec-decoder.yaml | 175 +++++++++++
.../media/mediatek,vcodec-encoder.yaml | 185 +++++++++++
.../bindings/media/mediatek-vcodec.txt | 130 --------
drivers/media/platform/mtk-vcodec/Makefile | 2 +
.../platform/mtk-vcodec/mtk_vcodec_dec.c | 4 +-
.../platform/mtk-vcodec/mtk_vcodec_dec.h | 1 +
.../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 276 ++++++++++++++---
.../platform/mtk-vcodec/mtk_vcodec_dec_hw.c | 184 +++++++++++
.../platform/mtk-vcodec/mtk_vcodec_dec_hw.h | 53 ++++
.../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 | 2 +
.../mtk-vcodec/mtk_vcodec_dec_stateless.c | 2 +
.../platform/mtk-vcodec/mtk_vcodec_drv.h | 71 ++++-
.../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 12 +-
.../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 1 -
.../platform/mtk-vcodec/mtk_vcodec_intr.c | 27 +-
.../platform/mtk-vcodec/mtk_vcodec_intr.h | 4 +-
.../platform/mtk-vcodec/mtk_vcodec_util.c | 87 +++++-
.../platform/mtk-vcodec/mtk_vcodec_util.h | 8 +-
.../platform/mtk-vcodec/vdec/vdec_h264_if.c | 2 +-
.../mtk-vcodec/vdec/vdec_h264_req_if.c | 2 +-
.../platform/mtk-vcodec/vdec/vdec_vp8_if.c | 2 +-
.../platform/mtk-vcodec/vdec/vdec_vp9_if.c | 2 +-
.../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 | 290 ++++++++++++++++++
.../platform/mtk-vcodec/vdec_msg_queue.h | 157 ++++++++++
.../media/platform/mtk-vcodec/vdec_vpu_if.c | 46 ++-
.../media/platform/mtk-vcodec/vdec_vpu_if.h | 22 ++
.../platform/mtk-vcodec/venc/venc_h264_if.c | 2 +-
.../platform/mtk-vcodec/venc/venc_vp8_if.c | 2 +-
33 files changed, 1792 insertions(+), 279 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml
create mode 100644 Documentation/devicetree/bindings/media/mediatek,vcodec-decoder.yaml
create mode 100644 Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml
delete mode 100644 Documentation/devicetree/bindings/media/mediatek-vcodec.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.25.1
@@ -18,19 +19,90 @@#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"#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,+};++staticstructcomponent_match*mtk_vcodec_match_add(+structmtk_vcodec_dev*vdec_dev)+{+structplatform_device*pdev=vdec_dev->plat_dev;+structcomponent_match*match=NULL;+structdevice_node*comp_node;+enummtk_vdec_hw_idcomp_idx;+conststructof_device_id*of_id;+inti;++for(i=0;i<ARRAY_SIZE(mtk_vdec_drv_ids);i++){+of_id=&mtk_vdec_drv_ids[i];+comp_node=of_find_compatible_node(NULL,NULL,+of_id->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;+}++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;
@@ -95,6 +167,58 @@ static int mtk_vcodec_get_reg_bases(struct mtk_vcodec_dev *dev)return0;}+staticintmtk_vcodec_init_master(structmtk_vcodec_dev*dev)+{+structplatform_device*pdev=dev->plat_dev;+structcomponent_match*match;+intret;++match=mtk_vcodec_match_add(dev);+if(IS_ERR_OR_NULL(match))+return-EINVAL;++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;+intret;++ret=mtk_vcodec_get_reg_bases(dev);+if(ret)+returnret;++if(!dev->vdec_pdata->is_comp_supported){+dev->dec_irq=platform_get_irq(pdev,0);+if(dev->dec_irq<0){+dev_err(&pdev->dev,"failed to get irq number");+returndev->dec_irq;+}++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(pdev,&dev->pm);+if(ret<0){+dev_err(&pdev->dev,"failed to get mt vcodec clock source");+returnret;+}+}++return0;+}+staticintfops_vcodec_open(structfile*file){structmtk_vcodec_dev*dev=video_drvdata(file);
@@ -220,7 +344,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;
@@ -249,32 +372,10 @@ 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;-}--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);+ret=mtk_vcodec_init_dec_params(dev);if(ret){-dev_err(&pdev->dev,"Failed to install dev->dec_irq %d (%d)",-dev->dec_irq,-ret);-gotoerr_res;+dev_err(&pdev->dev,"Failed to init pm and registers");+gotoerr_dec_pm;}mutex_init(&dev->dec_mutex);
@@ -362,8 +463,14 @@ 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->vdec_pdata->is_comp_supported){+ret=mtk_vcodec_init_master(dev);+if(ret<0)+gotoerr_component_match;+}+return0;+err_component_match:err_dec_reg:if(dev->vdec_pdata->uses_stateless_api)media_device_unregister(&dev->mdev_dec);
@@ -382,6 +489,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)mtk_vcodec_release_dec_pm(&dev->pm);err_dec_pm:mtk_vcodec_fw_release(dev->fw_handler);+returnret;}
@@ -0,0 +1,184 @@+// SPDX-License-Identifier: GPL-2.0+/*+*Copyright(c)2021MediaTekInc.+*Author:YunfeiDong<yunfei.dong@mediatek.com>+*/++#include<linux/component.h>+#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,+};++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(dec_done_status|VDEC_IRQ_CFG,vdec_misc_addr);+writel(dec_done_status&~VDEC_IRQ_CLR,vdec_misc_addr);++wake_up_ctx(ctx,MTK_INST_IRQ_RECEIVED);++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;+}++irq_set_status_flags(dev->dec_irq,IRQ_NOAUTOEN);+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);+returnret;+}++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;+ret=mtk_vcodec_init_dec_pm(pdev,&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){+dev_err(&pdev->dev,"Failed to register irq handler.\n");+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,+},+};
@@ -0,0 +1,185 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/iommu/mediatek,vcodec-encoder.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Mediatek Video Encode Accelerator With Component++maintainers:+-Yunfei Dong <yunfei.dong@mediatek.com>++description:|++Mediatek Video Encode is the video encode hardware present in Mediatek+SoCs which supports high resolution encoding functionalities.++properties:+compatible:+enum:+-mediatek,mt8173-vcodec-enc-vp8+-mediatek,mt8173-vcodec-enc+-mediatek,mt8183-vcodec-enc+-mediatek,mt8192-vcodec-enc++reg:+maxItems:1++interrupts:+maxItems:1++clocks:+minItems:1+maxItems:5++clock-names:+minItems:1+maxItems:5++assigned-clocks:true++assigned-clock-parents:true++iommus:+minItems:1+maxItems:32+description:|+List of the hardware port in respective IOMMU block for current Socs.+Refer to bindings/iommu/mediatek,iommu.yaml.++dma-ranges:+maxItems:1+description:|+Describes the physical address space of IOMMU maps to memory.++mediatek,larb:+$ref:/schemas/types.yaml#/definitions/phandle+maxItems:1+description:|+Must contain the local arbiters in the current Socs.++mediatek,vpu:+$ref:/schemas/types.yaml#/definitions/phandle+maxItems:1+description:+Describes point to vpu.++mediatek,scp:+$ref:/schemas/types.yaml#/definitions/phandle+maxItems:1+description:+Describes point to scp.++required:+-compatible+-reg+-interrupts+-clocks+-clock-names+-iommus+-assigned-clocks+-assigned-clock-parents++allOf:+-if:+properties:+compatible:+contains:+enum:+-mediatek,mt8183-vcodec-enc+-mediatek,mt8192-vcodec-enc++then:+required:+-mediatek,scp++-if:+properties:+compatible:+contains:+enum:+-mediatek,mt8173-vcodec-enc-vp8+-mediatek,mt8173-vcodec-enc++then:+required:+-mediatek,vpu++-if:+properties:+compatible:+enum:+-mediatek,mt8173-vcodec-enc+-mediatek,mt8192-vcodec-enc+-mediatek,mt8173-vcodec-enc++then:+properties:+clock:+items:+minItems:1+maxItems:1+clock-names:+items:+-const:venc_sel+else:# for vp8 hw decoder+properties:+clock:+items:+minItems:1+maxItems:1+clock-names:+items:+-const:venc_lt_sel++additionalProperties:false++examples:+-|+#include <dt-bindings/interrupt-controller/arm-gic.h>+#include <dt-bindings/clock/mt8173-clk.h>+#include <dt-bindings/memory/mt8173-larb-port.h>+#include <dt-bindings/interrupt-controller/irq.h>++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>;+};
@@ -249,7 +249,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;
@@ -379,7 +379,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;
@@ -422,7 +422,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;}
@@ -11,34 +11,31 @@#include"mtk_vcodec_intr.h"#include"mtk_vcodec_util.h"-intmtk_vcodec_wait_for_done_ctx(structmtk_vcodec_ctx*ctx,intcommand,-unsignedinttimeout_ms)+intmtk_vcodec_wait_for_done_ctx(structmtk_vcodec_ctx*ctx,+intcommand,unsignedinttimeout_ms,unsignedhw_id){-wait_queue_head_t*waitqueue;longtimeout_jiff,ret;intstatus=0;-waitqueue=(wait_queue_head_t*)&ctx->queue;timeout_jiff=msecs_to_jiffies(timeout_ms);--ret=wait_event_interruptible_timeout(*waitqueue,-ctx->int_cond,+ret=wait_event_interruptible_timeout(ctx->queue[hw_id],+ctx->int_cond[hw_id],timeout_jiff);if(!ret){status=-1;/* timeout */-mtk_v4l2_err("[%d] ctx->type=%d, cmd=%d, wait_event_interruptible_timeout time=%ums out %d %d!",-ctx->id,ctx->type,command,timeout_ms,-ctx->int_cond,ctx->int_type);+mtk_v4l2_err("[%d] cmd=%d, type=%d, dec timeout=%ums (%d %d)",+ctx->id,command,ctx->type,timeout_ms,+ctx->int_cond[hw_id],ctx->int_type[hw_id]);}elseif(-ERESTARTSYS==ret){-mtk_v4l2_err("[%d] ctx->type=%d, cmd=%d, wait_event_interruptible_timeout interrupted by a signal %d %d",-ctx->id,ctx->type,command,ctx->int_cond,-ctx->int_type);status=-1;+mtk_v4l2_err("[%d] cmd=%d, type=%d, dec inter fail (%d %d)",+ctx->id,command,ctx->type,+ctx->int_cond[hw_id],ctx->int_type[hw_id]);}-ctx->int_cond=0;-ctx->int_type=0;+ctx->int_cond[hw_id]=0;+ctx->int_type[hw_id]=0;returnstatus;}
@@ -126,7 +126,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]);
@@ -286,7 +286,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;/*
@@ -347,7 +347,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);
@@ -68,7 +68,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->vdec_pdata->is_comp_supported){+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->vdec_pdata->is_comp_supported){+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->vdec_pdata->is_comp_supported){+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->vdec_pdata->is_comp_supported){+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->vdec_pdata->is_comp_supported){+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->vdec_pdata->is_comp_supported){+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);
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need to include them in case of build fail [1].
[1]https://patchwork.kernel.org/project/linux-mediatek/list/?series=511175
---
.../media/mediatek,vcodec-comp-decoder.yaml | 172 ++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml
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>
---
v5: no changes
---
drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 3 +++
1 file changed, 3 insertions(+)
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>
---
v5: no changes
---
drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c | 4 ++--
drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 6 +++---
drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
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>
---
v5: no changes
---
.../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 37 ++++++++++++++-----
1 file changed, 28 insertions(+), 9 deletions(-)
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>
---
v5: no changes
---
.../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(-)
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>
---
v5: no changes
---
drivers/media/platform/mtk-vcodec/Makefile | 1 +
.../platform/mtk-vcodec/mtk_vcodec_drv.h | 5 +
.../platform/mtk-vcodec/vdec_msg_queue.c | 258 ++++++++++++++++++
.../platform/mtk-vcodec/vdec_msg_queue.h | 151 ++++++++++
4 files changed, 415 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
Vdec and venc can use the same function to wake up interrupt event.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Reviewed-by: Tzung-Bi Shih <redacted>
---
v5: no changes
---
drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 9 +--------
drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h | 8 ++++++++
drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 8 --------
3 files changed, 9 insertions(+), 16 deletions(-)
@@ -31,13 +31,6 @@module_param(mtk_v4l2_dbg_level,int,0644);module_param(mtk_vcodec_dbg,bool,0644);-/* Wake up context wait_queue */-staticvoidwake_up_ctx(structmtk_vcodec_ctx*ctx)-{-ctx->int_cond=1;-wake_up_interruptible(&ctx->queue);-}-staticirqreturn_tmtk_vcodec_dec_irq_handler(intirq,void*priv){structmtk_vcodec_dev*dev=priv;
From: Rob Herring <robh@kernel.org> Date: 2021-08-11 17:25:11
On Wed, 11 Aug 2021 10:57:59 +0800, Yunfei Dong wrote:
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need to include them in case of build fail [1].
[1]https://patchwork.kernel.org/project/linux-mediatek/list/?series=511175
---
.../media/mediatek,vcodec-comp-decoder.yaml | 172 ++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml: $id: relative path/filename doesn't match actual path or filename
expected: http://devicetree.org/schemas/media/mediatek,vcodec-comp-decoder.yaml#
Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.example.dts:22:18: fatal error: dt-bindings/clock/mt8192-clk.h: No such file or directory
22 | #include <dt-bindings/clock/mt8192-clk.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:380: Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1419: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1515556
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
From: Rob Herring <robh@kernel.org> Date: 2021-08-11 17:41:24
On Wed, Aug 11, 2021 at 10:57:59AM +0800, Yunfei Dong wrote:
quoted hunk
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need to include them in case of build fail [1].
[1]https://patchwork.kernel.org/project/linux-mediatek/list/?series=511175
---
.../media/mediatek,vcodec-comp-decoder.yaml | 172 ++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml
@@ -0,0 +1,172 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/iommu/mediatek,vcodec-comp-decoder.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Mediatek Video Decode Accelerator With Component++maintainers:+-Yunfei Dong <yunfei.dong@mediatek.com>++description:|++Mediatek Video Decode is the video decode hardware present in Mediatek+SoCs which supports high resolution decoding functionalities. Required+master and component node.++properties:+compatible:+oneOf:
Hi Yunfei,
Thank you for the patch.
On Wed, Aug 11, 2021 at 10:57:59AM +0800, Yunfei Dong wrote:
quoted hunk
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need to include them in case of build fail [1].
[1]https://patchwork.kernel.org/project/linux-mediatek/list/?series=511175
---
.../media/mediatek,vcodec-comp-decoder.yaml | 172 ++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml
@@ -0,0 +1,172 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/iommu/mediatek,vcodec-comp-decoder.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Mediatek Video Decode Accelerator With Component++maintainers:+-Yunfei Dong <yunfei.dong@mediatek.com>++description:|++Mediatek Video Decode is the video decode hardware present in Mediatek+SoCs which supports high resolution decoding functionalities. Required+master and component node.
This should explain how the three IP cores relate to each other.
I'm a bit late in the game, reviewing v5 only, but I'm wondering if
those IP cores need to be modelled in separate nodes. It would be much
easier, from a software point of view, to have a single node, with
multiple register ranges.
Are some of those IP cores used in different SoCs, combined in different
ways, that make a modular design better ?
--
Regards,
Laurent Pinchart
Hi Rob,
Thanks for your detail suggestion.
For the header file(dt-bindings/clock/mt8192-clk.h) is included in:
https://patchwork.kernel.org/project/linux-mediatek/list/?series=511175
Need to depend on this patch, do you have any suggestion about how to
add it?
Thanks.
On Wed, 2021-08-11 at 11:24 -0600, Rob Herring wrote:
On Wed, 11 Aug 2021 10:57:59 +0800, Yunfei Dong wrote:
quoted
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need to
include them in case of build fail [1].
[1]
https://patchwork.kernel.org/project/linux-mediatek/list/?series=511175
---
.../media/mediatek,vcodec-comp-decoder.yaml | 172
++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644
Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.yaml
My bot found errors running 'make DT_CHECKER_FLAGS=-m
dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.yaml: $id: relative path/filename doesn't match actual path
or filename
expected:
http://devicetree.org/schemas/media/mediatek,vcodec-comp-decoder.yaml#
Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.example.dts:22:18: fatal error: dt-bindings/clock/mt8192-
clk.h: No such file or directory
22 | #include <dt-bindings/clock/mt8192-clk.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:380:
Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1419: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1515556
This check can fail if there are any dependencies. The base for a
patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up
to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
Hi Laurent,
Thanks for your detail suggestion.
On Wed, 2021-08-11 at 20:59 +0300, Laurent Pinchart wrote:
Hi Yunfei,
Thank you for the patch.
On Wed, Aug 11, 2021 at 10:57:59AM +0800, Yunfei Dong wrote:
quoted
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need to
include them in case of build fail [1].
[1]
https://patchwork.kernel.org/project/linux-mediatek/list/?series=511175
---
.../media/mediatek,vcodec-comp-decoder.yaml | 172
++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644
Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.yaml
diff --git
a/Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.yaml
b/Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.yaml
new file mode 100644
index 000000000000..083c89933917
I'm a bit late in the game, reviewing v5 only, but I'm wondering if
those IP cores need to be modelled in separate nodes. It would be
much
easier, from a software point of view, to have a single node, with
multiple register ranges.
Are some of those IP cores used in different SoCs, combined in
different
ways, that make a modular design better ?
Different platform has different hardware, for mt8192 only has three
nodes. but mt8195 will has five nodes. and the clk/power/irq/iommu are
different. It is not easy to manage all hardware at the same time in
one node, need to enable different hardware at the same time, the logic
will be very complex.
It is much easier to handle different hardware using component, enable
different hardware when we need it.
+danvet
Hi,
On Tue, 10 Aug 2021 at 23:58, Yunfei Dong [off-list ref] wrote:
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.
I don't think it's a good idea to introduce the component API in the
media subsystem. It doesn't seem to be maintained, IRC there's not even
a maintainer for it, and it has some issues that were never addressed.
It would be really important to avoid it. Is it really needed in the
first place?
Thanks,
Ezequiel
Hi Ezequiel,
Thanks for your suggestion.
On Wed, 2021-08-18 at 11:11 -0300, Ezequiel Garcia wrote:
+danvet
Hi,
On Tue, 10 Aug 2021 at 23:58, Yunfei Dong [off-list ref]
wrote:
quoted
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.
I don't think it's a good idea to introduce the component API in the
media subsystem. It doesn't seem to be maintained, IRC there's not
even
a maintainer for it, and it has some issues that were never
addressed.
It would be really important to avoid it. Is it really needed in the
first place?
Thanks,
Ezequiel
For there are many hardware need to use, mt8192 is three and mt8195 is
five. Maybe need more to be used in the feature.
Each hardware has independent clk/power/iommu port/irq.
Use component interface in prob to get each component's information.
Just enable the hardware when need to use it, very convenient and
simple.
I found that there are many modules use component to manage hardware
information, such as iommu and drm etc.
Do you have any other suggestion for this architecture?
Thanks
Yunfei Dong
On Thu, 19 Aug 2021 at 04:13, yunfei.dong@mediatek.com
[off-list ref] wrote:
Hi Ezequiel,
Thanks for your suggestion.
On Wed, 2021-08-18 at 11:11 -0300, Ezequiel Garcia wrote:
quoted
+danvet
Hi,
On Tue, 10 Aug 2021 at 23:58, Yunfei Dong [off-list ref]
wrote:
quoted
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.
I don't think it's a good idea to introduce the component API in the
media subsystem. It doesn't seem to be maintained, IRC there's not
even
a maintainer for it, and it has some issues that were never
addressed.
It would be really important to avoid it. Is it really needed in the
first place?
Thanks,
Ezequiel
For there are many hardware need to use, mt8192 is three and mt8195 is
five. Maybe need more to be used in the feature.
Each hardware has independent clk/power/iommu port/irq.
Use component interface in prob to get each component's information.
Just enable the hardware when need to use it, very convenient and
simple.
I found that there are many modules use component to manage hardware
information, such as iommu and drm etc.
Many drivers support multiple hardware variants, where each variant
has a different number of clocks or interrupts, see for instance
struct hantro_variant which allows to expose different codec cores,
some having both decoder/encoder, and some having just a decoder.
The component API is mostly used by DRM to aggregate independent
subdevices (called components) into an aggregated driver.
For instance, a DRM driver needs to glue together the HDMI, MIPI,
and plany controller, or any other hardware arrangement where
devices can be described independently.
The component API may look simple but has some issues, it's not easy
to debug, and can cause troubles if not used as expected [1].
It's worth making sure you actually need a framework
to glue different devices together.
Do you have any other suggestion for this architecture?
Hi Ezequiel,
Thanks for your detail feedback.
On Thu, 2021-08-19 at 11:10 -0300, Ezequiel Garcia wrote:
On Thu, 19 Aug 2021 at 04:13, yunfei.dong@mediatek.com
[off-list ref] wrote:
quoted
Hi Ezequiel,
Thanks for your suggestion.
On Wed, 2021-08-18 at 11:11 -0300, Ezequiel Garcia wrote:
quoted
+danvet
Hi,
On Tue, 10 Aug 2021 at 23:58, Yunfei Dong [off-list ref]
wrote:
quoted
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.
I don't think it's a good idea to introduce the component API in the
media subsystem. It doesn't seem to be maintained, IRC there's not
even
a maintainer for it, and it has some issues that were never
addressed.
It would be really important to avoid it. Is it really needed in the
first place?
Thanks,
Ezequiel
For there are many hardware need to use, mt8192 is three and mt8195 is
five. Maybe need more to be used in the feature.
Each hardware has independent clk/power/iommu port/irq.
Use component interface in prob to get each component's information.
Just enable the hardware when need to use it, very convenient and
simple.
I found that there are many modules use component to manage hardware
information, such as iommu and drm etc.
Many drivers support multiple hardware variants, where each variant
has a different number of clocks or interrupts, see for instance
struct hantro_variant which allows to expose different codec cores,
some having both decoder/encoder, and some having just a decoder.
The component API is mostly used by DRM to aggregate independent
subdevices (called components) into an aggregated driver.
For instance, a DRM driver needs to glue together the HDMI, MIPI,
and plany controller, or any other hardware arrangement where
devices can be described independently.
The usage scenario is very similar with drm and iommu, So decide to use
component framework.
Decode has three/five or more hardwares, these hardware are independent.
For mt8183 just need core hardware to decode, but mt8192 has lat,soc and
core hardware to decode. When lat need to use, just enable lat hardware,
core is the same.And mt8195 will has two cores, each core can work well
independent.
For each component device just used to open their power/clk/iommu
port/irq when master need to enable it. The main logic is in master
device.
The component API may look simple but has some issues, it's not easy
to debug, and can cause troubles if not used as expected [1].
It's worth making sure you actually need a framework
to glue different devices together.
Each hardware has its index, master can get hardware information
according these index, looks not complex. What do you mean about not
easy to debug?
quoted
Do you have any other suggestion for this architecture?
Looking at the different patchsets that are posted, it's not clear
to me what exactly are the different architectures that you intend
to support, can you some documentation which clarifies that?
Have five hardwares lat,soc,core0,core1 and main. Lat thread can use lat
soc and main, core thread can use soc,lat, core0 and core1. Core thread
can be used or not for different project. Also Need to use these
hardware dynamic at the same time. So I use component framework, just
need to know the used hardware index according to different
project.Need not to do complex logic to manage these hardwares.
On Fri, 20 Aug 2021 at 04:59, yunfei.dong@mediatek.com
[off-list ref] wrote:
Hi Ezequiel,
Thanks for your detail feedback.
On Thu, 2021-08-19 at 11:10 -0300, Ezequiel Garcia wrote:
quoted
On Thu, 19 Aug 2021 at 04:13, yunfei.dong@mediatek.com
[off-list ref] wrote:
quoted
Hi Ezequiel,
Thanks for your suggestion.
On Wed, 2021-08-18 at 11:11 -0300, Ezequiel Garcia wrote:
quoted
+danvet
Hi,
On Tue, 10 Aug 2021 at 23:58, Yunfei Dong [off-list ref]
wrote:
quoted
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.
I don't think it's a good idea to introduce the component API in the
media subsystem. It doesn't seem to be maintained, IRC there's not
even
a maintainer for it, and it has some issues that were never
addressed.
It would be really important to avoid it. Is it really needed in the
first place?
Thanks,
Ezequiel
For there are many hardware need to use, mt8192 is three and mt8195 is
five. Maybe need more to be used in the feature.
Each hardware has independent clk/power/iommu port/irq.
Use component interface in prob to get each component's information.
Just enable the hardware when need to use it, very convenient and
simple.
I found that there are many modules use component to manage hardware
information, such as iommu and drm etc.
Many drivers support multiple hardware variants, where each variant
has a different number of clocks or interrupts, see for instance
struct hantro_variant which allows to expose different codec cores,
some having both decoder/encoder, and some having just a decoder.
The component API is mostly used by DRM to aggregate independent
subdevices (called components) into an aggregated driver.
For instance, a DRM driver needs to glue together the HDMI, MIPI,
and plany controller, or any other hardware arrangement where
devices can be described independently.
The usage scenario is very similar with drm and iommu, So decide to use
component framework.
Decode has three/five or more hardwares, these hardware are independent.
For mt8183 just need core hardware to decode, but mt8192 has lat,soc and
core hardware to decode. When lat need to use, just enable lat hardware,
core is the same.And mt8195 will has two cores, each core can work well
independent.
For each component device just used to open their power/clk/iommu
port/irq when master need to enable it. The main logic is in master
device.
quoted
The component API may look simple but has some issues, it's not easy
to debug, and can cause troubles if not used as expected [1].
It's worth making sure you actually need a framework
to glue different devices together.
Each hardware has its index, master can get hardware information
according these index, looks not complex. What do you mean about not
easy to debug?
quoted
quoted
Do you have any other suggestion for this architecture?
Looking at the different patchsets that are posted, it's not clear
to me what exactly are the different architectures that you intend
to support, can you some documentation which clarifies that?
Have five hardwares lat,soc,core0,core1 and main. Lat thread can use lat
soc and main, core thread can use soc,lat, core0 and core1. Core thread
can be used or not for different project.
Can you explain what are these lat,soc and core threads for?
Also Need to use these
hardware dynamic at the same time. So I use component framework, just
need to know the used hardware index according to different
project.Need not to do complex logic to manage these hardwares.
I am not thrilled to see the component framework introduced to the
media subsystem. Like I said, it has no clear maintainer, and it's not
easy to use.
The media subsystem has some support which AFAIK does the same thing,
see v4l2-async, which is maintained by media people.
Please push a branch based on media/master containing these changes.
I see there are other patch series for this device, but it's hard to track
which goes first, etc.
Thanks,
Ezequiel
From: Daniel Vetter <hidden> Date: 2021-08-22 16:50:40
On Wed, Aug 18, 2021 at 4:12 PM Ezequiel Garcia
[off-list ref] wrote:
+danvet
Hi,
On Tue, 10 Aug 2021 at 23:58, Yunfei Dong [off-list ref] wrote:
quoted
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.
I don't think it's a good idea to introduce the component API in the
media subsystem. It doesn't seem to be maintained, IRC there's not even
a maintainer for it, and it has some issues that were never addressed.
Defacto dri-devel folks are maintainer component.c, but also I'm not
aware of anything missing there?
There has been discussions that in various drm subsystems like
drm_bridge or drm_panel a few things are missing, which prevent
drivers from moving _away_ from component.c to the more specific
solutions for panel/bridges. But nothing that's preventing them from
using component.c itself.
I'm happy to merge a MAINTAINERS patch to clarify the situation if
that's needed.
-Daniel
It would be really important to avoid it. Is it really needed in the
first place?
Thanks,
Ezequiel
On Sun, 22 Aug 2021 at 13:50, Daniel Vetter [off-list ref] wrote:
On Wed, Aug 18, 2021 at 4:12 PM Ezequiel Garcia
[off-list ref] wrote:
quoted
+danvet
Hi,
On Tue, 10 Aug 2021 at 23:58, Yunfei Dong [off-list ref] wrote:
quoted
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.
I don't think it's a good idea to introduce the component API in the
media subsystem. It doesn't seem to be maintained, IRC there's not even
a maintainer for it, and it has some issues that were never addressed.
Defacto dri-devel folks are maintainer component.c, but also I'm not
aware of anything missing there?
A while ago, I tried to fix a crash in the Rockchip DRM driver
(I was then told there can be similar issues on the IMX driver too,
but I forgot the details of that).
I sent a patchset trying to address it and got total silence back.
Although you could argue the issue is in how drivers use the component
API, AFAICR the abuse is spreaded across a few drivers, so it felt
more reasonable to improve the component API itself, instead of changing
all the drivers.
See below:
https://patchwork.kernel.org/project/linux-rockchip/cover/20200120170602.3832-1-ezequiel@collabora.com/
There has been discussions that in various drm subsystems like
drm_bridge or drm_panel a few things are missing, which prevent
drivers from moving _away_ from component.c to the more specific
solutions for panel/bridges. But nothing that's preventing them from
using component.c itself.
I'm happy to merge a MAINTAINERS patch to clarify the situation if
that's needed.
Hi Ezequiel,
Thanks for your suggestion.
On Sun, 2021-08-22 at 11:32 -0300, Ezequiel Garcia wrote:
On Fri, 20 Aug 2021 at 04:59, yunfei.dong@mediatek.com
[off-list ref] wrote:
quoted
Hi Ezequiel,
Thanks for your detail feedback.
On Thu, 2021-08-19 at 11:10 -0300, Ezequiel Garcia wrote:
quoted
On Thu, 19 Aug 2021 at 04:13, yunfei.dong@mediatek.com
[off-list ref] wrote:
quoted
Hi Ezequiel,
Thanks for your suggestion.
On Wed, 2021-08-18 at 11:11 -0300, Ezequiel Garcia wrote:
quoted
+danvet
Hi,
On Tue, 10 Aug 2021 at 23:58, Yunfei Dong <
yunfei.dong@mediatek.com>
wrote:
quoted
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.
I don't think it's a good idea to introduce the component API
in the
media subsystem. It doesn't seem to be maintained, IRC
there's not
even
a maintainer for it, and it has some issues that were never
addressed.
It would be really important to avoid it. Is it really needed
in the
first place?
Thanks,
Ezequiel
For there are many hardware need to use, mt8192 is three and
mt8195 is
five. Maybe need more to be used in the feature.
Each hardware has independent clk/power/iommu port/irq.
Use component interface in prob to get each component's
information.
Just enable the hardware when need to use it, very convenient
and
simple.
I found that there are many modules use component to manage
hardware
information, such as iommu and drm etc.
Many drivers support multiple hardware variants, where each
variant
has a different number of clocks or interrupts, see for instance
struct hantro_variant which allows to expose different codec
cores,
some having both decoder/encoder, and some having just a decoder.
The component API is mostly used by DRM to aggregate independent
subdevices (called components) into an aggregated driver.
For instance, a DRM driver needs to glue together the HDMI, MIPI,
and plany controller, or any other hardware arrangement where
devices can be described independently.
The usage scenario is very similar with drm and iommu, So decide to
use
component framework.
Decode has three/five or more hardwares, these hardware are
independent.
For mt8183 just need core hardware to decode, but mt8192 has
lat,soc and
core hardware to decode. When lat need to use, just enable lat
hardware,
core is the same.And mt8195 will has two cores, each core can work
well
independent.
For each component device just used to open their power/clk/iommu
port/irq when master need to enable it. The main logic is in master
device.
quoted
The component API may look simple but has some issues, it's not
easy
to debug, and can cause troubles if not used as expected [1].
It's worth making sure you actually need a framework
to glue different devices together.
Each hardware has its index, master can get hardware information
according these index, looks not complex. What do you mean about
not
easy to debug?
quoted
quoted
Do you have any other suggestion for this architecture?
Looking at the different patchsets that are posted, it's not
clear
to me what exactly are the different architectures that you
intend
to support, can you some documentation which clarifies that?
Have five hardwares lat,soc,core0,core1 and main. Lat thread can
use lat
soc and main, core thread can use soc,lat, core0 and core1. Core
thread
can be used or not for different project.
Can you explain what are these lat,soc and core threads for?
You can regards lat,soc and core as hardware, each hardware can work
independent. Lat and core threads used to control hardware to decode.
quoted
Also Need to use these
hardware dynamic at the same time. So I use component framework,
just
need to know the used hardware index according to different
project.Need not to do complex logic to manage these hardwares.
I am not thrilled to see the component framework introduced to the
media subsystem. Like I said, it has no clear maintainer, and it's
not
easy to use.
How do you think about Deniel Vetter's mail ? It looks that there are
maintainer for it.
The media subsystem has some support which AFAIK does the same thing,
see v4l2-async, which is maintained by media people.
If component can be used, I prefer to use it. At the other hand, I will
try to use these method as compared.
Please push a branch based on media/master containing these changes.
I see there are other patch series for this device, but it's hard to
track
which goes first, etc.
I need time to push a branch, or you can get the latest kernel and git
am these patches, maybe much quicker.
Hi Ezequiel,
You can get the dtsi information from patch 13, it is decoder yaml file
about component architecture:
[PATCH v4, 13/15] dt-bindings: media: mtk-vcodec: Adds decoder dt-
bindings for mt8192
Thanks
Yunfei Dong
On Tue, 2021-08-24 at 18:21 +0800, yunfei.dong@mediatek.com wrote:
Hi Ezequiel,
Thanks for your suggestion.
On Sun, 2021-08-22 at 11:32 -0300, Ezequiel Garcia wrote:
quoted
On Fri, 20 Aug 2021 at 04:59, yunfei.dong@mediatek.com
[off-list ref] wrote:
quoted
Hi Ezequiel,
Thanks for your detail feedback.
On Thu, 2021-08-19 at 11:10 -0300, Ezequiel Garcia wrote:
quoted
On Thu, 19 Aug 2021 at 04:13, yunfei.dong@mediatek.com
[off-list ref] wrote:
quoted
Hi Ezequiel,
Thanks for your suggestion.
On Wed, 2021-08-18 at 11:11 -0300, Ezequiel Garcia wrote:
quoted
+danvet
Hi,
On Tue, 10 Aug 2021 at 23:58, Yunfei Dong <
yunfei.dong@mediatek.com>
wrote:
quoted
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.
I don't think it's a good idea to introduce the component
API
in the
media subsystem. It doesn't seem to be maintained, IRC
there's not
even
a maintainer for it, and it has some issues that were never
addressed.
It would be really important to avoid it. Is it really
needed
in the
first place?
Thanks,
Ezequiel
For there are many hardware need to use, mt8192 is three and
mt8195 is
five. Maybe need more to be used in the feature.
Each hardware has independent clk/power/iommu port/irq.
Use component interface in prob to get each component's
information.
Just enable the hardware when need to use it, very convenient
and
simple.
I found that there are many modules use component to manage
hardware
information, such as iommu and drm etc.
Many drivers support multiple hardware variants, where each
variant
has a different number of clocks or interrupts, see for
instance
struct hantro_variant which allows to expose different codec
cores,
some having both decoder/encoder, and some having just a
decoder.
The component API is mostly used by DRM to aggregate
independent
subdevices (called components) into an aggregated driver.
For instance, a DRM driver needs to glue together the HDMI,
MIPI,
and plany controller, or any other hardware arrangement where
devices can be described independently.
The usage scenario is very similar with drm and iommu, So decide
to
use
component framework.
Decode has three/five or more hardwares, these hardware are
independent.
For mt8183 just need core hardware to decode, but mt8192 has
lat,soc and
core hardware to decode. When lat need to use, just enable lat
hardware,
core is the same.And mt8195 will has two cores, each core can
work
well
independent.
For each component device just used to open their power/clk/iommu
port/irq when master need to enable it. The main logic is in
master
device.
quoted
The component API may look simple but has some issues, it's not
easy
to debug, and can cause troubles if not used as expected [1].
It's worth making sure you actually need a framework
to glue different devices together.
Each hardware has its index, master can get hardware information
according these index, looks not complex. What do you mean about
not
easy to debug?
quoted
quoted
Do you have any other suggestion for this architecture?
Looking at the different patchsets that are posted, it's not
clear
to me what exactly are the different architectures that you
intend
to support, can you some documentation which clarifies that?
Have five hardwares lat,soc,core0,core1 and main. Lat thread can
use lat
soc and main, core thread can use soc,lat, core0 and core1. Core
thread
can be used or not for different project.
Can you explain what are these lat,soc and core threads for?
You can regards lat,soc and core as hardware, each hardware can work
independent. Lat and core threads used to control hardware to decode.
quoted
quoted
Also Need to use these
hardware dynamic at the same time. So I use component framework,
just
need to know the used hardware index according to different
project.Need not to do complex logic to manage these hardwares.
I am not thrilled to see the component framework introduced to the
media subsystem. Like I said, it has no clear maintainer, and it's
not
easy to use.
How do you think about Deniel Vetter's mail ? It looks that there are
maintainer for it.
quoted
The media subsystem has some support which AFAIK does the same
thing,
see v4l2-async, which is maintained by media people.
If component can be used, I prefer to use it. At the other hand, I
will
try to use these method as compared.
quoted
Please push a branch based on media/master containing these
changes.
I see there are other patch series for this device, but it's hard
to
track
which goes first, etc.
I need time to push a branch, or you can get the latest kernel and
git
am these patches, maybe much quicker.
From: Daniel Vetter <hidden> Date: 2021-08-26 09:14:16
On Sun, Aug 22, 2021 at 02:57:15PM -0300, Ezequiel Garcia wrote:
On Sun, 22 Aug 2021 at 13:50, Daniel Vetter [off-list ref] wrote:
quoted
On Wed, Aug 18, 2021 at 4:12 PM Ezequiel Garcia
[off-list ref] wrote:
quoted
+danvet
Hi,
On Tue, 10 Aug 2021 at 23:58, Yunfei Dong [off-list ref] wrote:
quoted
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.
I don't think it's a good idea to introduce the component API in the
media subsystem. It doesn't seem to be maintained, IRC there's not even
a maintainer for it, and it has some issues that were never addressed.
Defacto dri-devel folks are maintainer component.c, but also I'm not
aware of anything missing there?
A while ago, I tried to fix a crash in the Rockchip DRM driver
(I was then told there can be similar issues on the IMX driver too,
but I forgot the details of that).
I sent a patchset trying to address it and got total silence back.
Although you could argue the issue is in how drivers use the component
API, AFAICR the abuse is spreaded across a few drivers, so it felt
more reasonable to improve the component API itself, instead of changing
all the drivers.
See below:
https://patchwork.kernel.org/project/linux-rockchip/cover/20200120170602.3832-1-ezequiel@collabora.com/
Patches get lost on the mailing list, and rockchip is one of the lesser
maintained drivers. You need to ping this stuff.
For bridge/panel I still think we should work towards removing component.c
use from them.
quoted
There has been discussions that in various drm subsystems like
drm_bridge or drm_panel a few things are missing, which prevent
drivers from moving _away_ from component.c to the more specific
solutions for panel/bridges. But nothing that's preventing them from
using component.c itself.
I'm happy to merge a MAINTAINERS patch to clarify the situation if
that's needed.
Indeed, that would be good.
Ok I'm going to type something.
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On Wed, 11 Aug 2021 at 14:59, Laurent Pinchart
[off-list ref] wrote:
Hi Yunfei,
Thank you for the patch.
On Wed, Aug 11, 2021 at 10:57:59AM +0800, Yunfei Dong wrote:
quoted
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need to include them in case of build fail [1].
[1]https://patchwork.kernel.org/project/linux-mediatek/list/?series=511175
---
.../media/mediatek,vcodec-comp-decoder.yaml | 172 ++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml
@@ -0,0 +1,172 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/iommu/mediatek,vcodec-comp-decoder.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Mediatek Video Decode Accelerator With Component++maintainers:+-Yunfei Dong <yunfei.dong@mediatek.com>++description:|++Mediatek Video Decode is the video decode hardware present in Mediatek+SoCs which supports high resolution decoding functionalities. Required+master and component node.
This should explain how the three IP cores relate to each other.
I'm a bit late in the game, reviewing v5 only, but I'm wondering if
those IP cores need to be modelled in separate nodes. It would be much
easier, from a software point of view, to have a single node, with
multiple register ranges.
Are some of those IP cores used in different SoCs, combined in different
ways, that make a modular design better ?
Yeah, I agree with Laurent here. This way of modelling the different parts
of the CODEC as different pieces is the reason why you need a framework
to pull them together, such as the component API (I guess v4l2-async
could have been used as well).
I normally don't bother with driver internals, as its up to each driver
author to decide what is best. However, I believe this design is too
convoluted, and it may lead other developers to follow the same pattern,
so please avoid it.
There are several ways of modelling this and initializing or probing
the sub-devices,
without using any async framework.
Thanks,
Ezequiel
On Tue, 17 Aug 2021 at 00:52, yunfei.dong@mediatek.com
[off-list ref] wrote:
Hi Laurent,
Thanks for your detail suggestion.
On Wed, 2021-08-11 at 20:59 +0300, Laurent Pinchart wrote:
quoted
Hi Yunfei,
Thank you for the patch.
On Wed, Aug 11, 2021 at 10:57:59AM +0800, Yunfei Dong wrote:
quoted
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need to
include them in case of build fail [1].
[1]
https://patchwork.kernel.org/project/linux-mediatek/list/?series=511175
---
.../media/mediatek,vcodec-comp-decoder.yaml | 172
++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644
Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.yaml
diff --git
a/Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.yaml
b/Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.yaml
new file mode 100644
index 000000000000..083c89933917
I'm a bit late in the game, reviewing v5 only, but I'm wondering if
those IP cores need to be modelled in separate nodes. It would be
much
easier, from a software point of view, to have a single node, with
multiple register ranges.
Are some of those IP cores used in different SoCs, combined in
different
ways, that make a modular design better ?
Different platform has different hardware, for mt8192 only has three
nodes. but mt8195 will has five nodes. and the clk/power/irq/iommu are
different. It is not easy to manage all hardware at the same time in
one node, need to enable different hardware at the same time, the logic
will be very complex.
It is much easier to handle different hardware using component, enable
different hardware when we need it.
You can still have one device-tree node for each device, which means
you can still manage your resources (clk/power/irq/iommu) easily, but doing
this so that it avoids an async framework to pull the parts together.
I gave you this feedback several times, and you have been objecting it
every time without even trying to consider a different approach.
Thanks,
Ezequiel
On Sun, 2021-08-29 at 17:50 -0300, Ezequiel Garcia wrote:
On Wed, 11 Aug 2021 at 14:59, Laurent Pinchart
[off-list ref] wrote:
quoted
Hi Yunfei,
Thank you for the patch.
On Wed, Aug 11, 2021 at 10:57:59AM +0800, Yunfei Dong wrote:
quoted
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need to
include them in case of build fail [1].
[1]
https://patchwork.kernel.org/project/linux-mediatek/list/?series=511175
---
.../media/mediatek,vcodec-comp-decoder.yaml | 172
++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644
Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.yaml
diff --git
a/Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.yaml
b/Documentation/devicetree/bindings/media/mediatek,vcodec-comp-
decoder.yaml
new file mode 100644
index 000000000000..083c89933917
I'm a bit late in the game, reviewing v5 only, but I'm wondering if
those IP cores need to be modelled in separate nodes. It would be
much
easier, from a software point of view, to have a single node, with
multiple register ranges.
Are some of those IP cores used in different SoCs, combined in
different
ways, that make a modular design better ?
1. Each a node should respect a HW node. Defining a complex node that
contain multiple register is not better, for they belong to different
hardware. Different platforms has different hardware count, mt8195 has
five hardwares.
2. Another reason is from the IOMMU point, the vcodec HW include core
and lat hardwares, each of them connect to the independent IOMMU
hardware for mt8195, can't write all iommu ports together, or hardware
can't access dram data, so we must separate them.
Yeah, I agree with Laurent here. This way of modelling the different
parts
of the CODEC as different pieces is the reason why you need a
framework
to pull them together, such as the component API (I guess v4l2-async
could have been used as well).
We must separate each hardware node, only master need to register video
and media device, component just used for store clk/power/irq/register
information for current hardware.
I normally don't bother with driver internals, as its up to each
driver
author to decide what is best. However, I believe this design is too
convoluted, and it may lead other developers to follow the same
pattern,
so please avoid it.
According to your information, it looks that component and v4l2 async
are ok for this architecture, only component is too convoluted(iommu
driver also use component).
For we are not very familiar with v4l2 async, out architecture is
designed for component include next forty patches. It is a very big
change for out driver. I will try to spend a lot of time to change it.
There are several ways of modelling this and initializing or probing
the sub-devices,
without using any async framework.
You may misunderstand, I'm not objecting your suggestion every time,
for our driver is designed for component api, need to spend a lot of
time to change it in many platforms. I will change it if v4l2 async has
many advantages then component as you said. I will try to use v4l2
async as compared.
Hi Yunfei,
On Mon, Aug 30, 2021 at 02:07:44PM +0800, yunfei.dong@mediatek.com wrote:
On Sun, 2021-08-29 at 17:50 -0300, Ezequiel Garcia wrote:
quoted
On Wed, 11 Aug 2021 at 14:59, Laurent Pinchart wrote:
quoted
On Wed, Aug 11, 2021 at 10:57:59AM +0800, Yunfei Dong wrote:
quoted
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need to
include them in case of build fail [1].
[1]
https://patchwork.kernel.org/project/linux-mediatek/list/?series=511175
---
.../media/mediatek,vcodec-comp-decoder.yaml | 172 ++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644
Documentation/devicetree/bindings/media/mediatek,vcodec-comp-decoder.yaml
@@ -0,0 +1,172 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.2+---+$id:http://devicetree.org/schemas/iommu/mediatek,vcodec-comp-decoder.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Mediatek Video Decode Accelerator With Component++maintainers:+-Yunfei Dong <yunfei.dong@mediatek.com>++description:|++Mediatek Video Decode is the video decode hardware present in Mediatek+SoCs which supports high resolution decoding functionalities. Required+master and component node.
This should explain how the three IP cores relate to each other.
I'm a bit late in the game, reviewing v5 only, but I'm wondering if
those IP cores need to be modelled in separate nodes. It would be much
easier, from a software point of view, to have a single node, with
multiple register ranges.
Are some of those IP cores used in different SoCs, combined in different
ways, that make a modular design better ?
1. Each a node should respect a HW node. Defining a complex node that
contain multiple register is not better, for they belong to different
hardware. Different platforms has different hardware count, mt8195 has
five hardwares.
2. Another reason is from the IOMMU point, the vcodec HW include core
and lat hardwares, each of them connect to the independent IOMMU
hardware for mt8195, can't write all iommu ports together, or hardware
can't access dram data, so we must separate them.
Maybe it could help understanding the situation if the DT bindings could
include more information about the hardware architecture, such as a
block diagram for instance, and how the pieces interact with each other
?
quoted
Yeah, I agree with Laurent here. This way of modelling the different parts
of the CODEC as different pieces is the reason why you need a framework
to pull them together, such as the component API (I guess v4l2-async
could have been used as well).
We must separate each hardware node, only master need to register video
and media device, component just used for store clk/power/irq/register
information for current hardware.
quoted
I normally don't bother with driver internals, as its up to each driver
author to decide what is best. However, I believe this design is too
convoluted, and it may lead other developers to follow the same pattern,
so please avoid it.
According to your information, it looks that component and v4l2 async
are ok for this architecture, only component is too convoluted(iommu
driver also use component).
For we are not very familiar with v4l2 async, out architecture is
designed for component include next forty patches. It is a very big
change for out driver. I will try to spend a lot of time to change it.
quoted
There are several ways of modelling this and initializing or probing
the sub-devices,
without using any async framework.
You may misunderstand, I'm not objecting your suggestion every time,
for our driver is designed for component api, need to spend a lot of
time to change it in many platforms. I will change it if v4l2 async has
many advantages then component as you said. I will try to use v4l2
async as compared.
Hi Ezequiel,
Thanks for your feedback.
On Sun, 2021-08-29 at 17:54 -0300, Ezequiel Garcia wrote:
On Tue, 17 Aug 2021 at 00:52, yunfei.dong@mediatek.com
[off-list ref] wrote:
quoted
Hi Laurent,
Thanks for your detail suggestion.
On Wed, 2021-08-11 at 20:59 +0300, Laurent Pinchart wrote:
quoted
Hi Yunfei,
Thank you for the patch.
On Wed, Aug 11, 2021 at 10:57:59AM +0800, Yunfei Dong wrote:
quoted
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need to
include them in case of build fail [1].
[1]
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek Video Decode Accelerator With Component
+
+maintainers:
+ - Yunfei Dong [off-list ref]
+
+description: |+
+ Mediatek Video Decode is the video decode hardware present
in
Mediatek
+ SoCs which supports high resolution decoding
functionalities.
Required
+ master and component node.
This should explain how the three IP cores relate to each other.
I'm a bit late in the game, reviewing v5 only, but I'm wondering
if
those IP cores need to be modelled in separate nodes. It would be
much
easier, from a software point of view, to have a single node,
with
multiple register ranges.
Are some of those IP cores used in different SoCs, combined in
different
ways, that make a modular design better ?
Different platform has different hardware, for mt8192 only has
three
nodes. but mt8195 will has five nodes. and the clk/power/irq/iommu
are
different. It is not easy to manage all hardware at the same time
in
one node, need to enable different hardware at the same time, the
logic
will be very complex.
It is much easier to handle different hardware using component,
enable
different hardware when we need it.
You can still have one device-tree node for each device, which means
you can still manage your resources (clk/power/irq/iommu) easily, but
doing
this so that it avoids an async framework to pull the parts together.
It mean that v4l2 async also can't be used?
I gave you this feedback several times, and you have been objecting
it
every time without even trying to consider a different approach.
I'm not objecting your suggestion every time, for our driver is
designed for component api, need to spend a lot of time to change it in
many platforms. We need to get a final solution for this architecture,
or it's not easy to change all patches in the feature.
Thanks,
Ezequiel
1. Each a node should respect a HW node. Defining a complex node that
contain multiple register is not better, for they belong to different
hardware. Different platforms has different hardware count, mt8195 has
five hardwares.
2. Another reason is from the IOMMU point, the vcodec HW include core
and lat hardwares, each of them connect to the independent IOMMU
hardware for mt8195, can't write all iommu ports together, or hardware
can't access dram data, so we must separate them.
For these limitation, can't combine all hardware together. I will write
the hardware diagram in next patch.
Thanks,
Yunfei Dong
On Wed, 1 Sept 2021 at 00:49, yunfei.dong@mediatek.com
[off-list ref] wrote:
Hi Ezequiel,
Thanks for your feedback.
On Sun, 2021-08-29 at 17:54 -0300, Ezequiel Garcia wrote:
quoted
On Tue, 17 Aug 2021 at 00:52, yunfei.dong@mediatek.com
[off-list ref] wrote:
quoted
Hi Laurent,
Thanks for your detail suggestion.
On Wed, 2021-08-11 at 20:59 +0300, Laurent Pinchart wrote:
quoted
Hi Yunfei,
Thank you for the patch.
On Wed, Aug 11, 2021 at 10:57:59AM +0800, Yunfei Dong wrote:
quoted
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need to
include them in case of build fail [1].
[1]
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek Video Decode Accelerator With Component
+
+maintainers:
+ - Yunfei Dong [off-list ref]
+
+description: |+
+ Mediatek Video Decode is the video decode hardware present
in
Mediatek
+ SoCs which supports high resolution decoding
functionalities.
Required
+ master and component node.
This should explain how the three IP cores relate to each other.
I'm a bit late in the game, reviewing v5 only, but I'm wondering
if
those IP cores need to be modelled in separate nodes. It would be
much
easier, from a software point of view, to have a single node,
with
multiple register ranges.
Are some of those IP cores used in different SoCs, combined in
different
ways, that make a modular design better ?
Different platform has different hardware, for mt8192 only has
three
nodes. but mt8195 will has five nodes. and the clk/power/irq/iommu
are
different. It is not easy to manage all hardware at the same time
in
one node, need to enable different hardware at the same time, the
logic
will be very complex.
It is much easier to handle different hardware using component,
enable
different hardware when we need it.
You can still have one device-tree node for each device, which means
you can still manage your resources (clk/power/irq/iommu) easily, but
doing
this so that it avoids an async framework to pull the parts together.
It mean that v4l2 async also can't be used?
No, I don't think it's needed. I will take a look at the device tree
and see if I can show you an alternative approach that doesn't
require an async framework.
quoted
I gave you this feedback several times, and you have been objecting
it
every time without even trying to consider a different approach.
I'm not objecting your suggestion every time, for our driver is
designed for component api, need to spend a lot of time to change it in
many platforms. We need to get a final solution for this architecture,
or it's not easy to change all patches in the feature.
quoted
Thanks,
Ezequiel
1. Each a node should respect a HW node. Defining a complex node that
contain multiple register is not better, for they belong to different
hardware. Different platforms has different hardware count, mt8195 has
five hardwares.
2. Another reason is from the IOMMU point, the vcodec HW include core
and lat hardwares, each of them connect to the independent IOMMU
hardware for mt8195, can't write all iommu ports together, or hardware
can't access dram data, so we must separate them.
For these limitation, can't combine all hardware together. I will write
the hardware diagram in next patch.
No need for another patch, you can write that hardware diagram
now, so we can help with some design ideas.
Thanks!
Ezequiel
Hi Ezequiel,
Thanks for your suggestion.
On Wed, 2021-09-01 at 08:50 -0300, Ezequiel Garcia wrote:
On Wed, 1 Sept 2021 at 00:49, yunfei.dong@mediatek.com
[off-list ref] wrote:
quoted
Hi Ezequiel,
Thanks for your feedback.
On Sun, 2021-08-29 at 17:54 -0300, Ezequiel Garcia wrote:
quoted
On Tue, 17 Aug 2021 at 00:52, yunfei.dong@mediatek.com
[off-list ref] wrote:
quoted
Hi Laurent,
Thanks for your detail suggestion.
On Wed, 2021-08-11 at 20:59 +0300, Laurent Pinchart wrote:
quoted
Hi Yunfei,
Thank you for the patch.
On Wed, Aug 11, 2021 at 10:57:59AM +0800, Yunfei Dong wrote:
quoted
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need
to
include them in case of build fail [1].
[1]
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek Video Decode Accelerator With Component
+
+maintainers:
+ - Yunfei Dong [off-list ref]
+
+description: |+
+ Mediatek Video Decode is the video decode hardware
present
in
Mediatek
+ SoCs which supports high resolution decoding
functionalities.
Required
+ master and component node.
This should explain how the three IP cores relate to each
other.
I'm a bit late in the game, reviewing v5 only, but I'm
wondering
if
those IP cores need to be modelled in separate nodes. It
would be
much
easier, from a software point of view, to have a single node,
with
multiple register ranges.
Are some of those IP cores used in different SoCs, combined
in
different
ways, that make a modular design better ?
Different platform has different hardware, for mt8192 only has
three
nodes. but mt8195 will has five nodes. and the
clk/power/irq/iommu
are
different. It is not easy to manage all hardware at the same
time
in
one node, need to enable different hardware at the same time,
the
logic
will be very complex.
It is much easier to handle different hardware using component,
enable
different hardware when we need it.
You can still have one device-tree node for each device, which
means
you can still manage your resources (clk/power/irq/iommu) easily,
but
doing
this so that it avoids an async framework to pull the parts
together.
It mean that v4l2 async also can't be used?
No, I don't think it's needed. I will take a look at the device tree
and see if I can show you an alternative approach that doesn't
require an async framework.
Thanks.
quoted
quoted
I gave you this feedback several times, and you have been
objecting
it
every time without even trying to consider a different approach.
I'm not objecting your suggestion every time, for our driver is
designed for component api, need to spend a lot of time to change
it in
many platforms. We need to get a final solution for this
architecture,
or it's not easy to change all patches in the feature.
quoted
Thanks,
Ezequiel
1. Each a node should respect a HW node. Defining a complex node
that
contain multiple register is not better, for they belong to
different
hardware. Different platforms has different hardware count, mt8195
has
five hardwares.
2. Another reason is from the IOMMU point, the vcodec HW include
core
and lat hardwares, each of them connect to the independent IOMMU
hardware for mt8195, can't write all iommu ports together, or
hardware
can't access dram data, so we must separate them.
For these limitation, can't combine all hardware together. I will
write
the hardware diagram in next patch.
No need for another patch, you can write that hardware diagram
now, so we can help with some design ideas.
On Thu, Sep 2, 2021 at 2:09 PM yunfei.dong@mediatek.com
[off-list ref] wrote:
Hi Ezequiel,
Thanks for your suggestion.
On Wed, 2021-09-01 at 08:50 -0300, Ezequiel Garcia wrote:
quoted
On Wed, 1 Sept 2021 at 00:49, yunfei.dong@mediatek.com
[off-list ref] wrote:
quoted
Hi Ezequiel,
Thanks for your feedback.
On Sun, 2021-08-29 at 17:54 -0300, Ezequiel Garcia wrote:
quoted
On Tue, 17 Aug 2021 at 00:52, yunfei.dong@mediatek.com
[off-list ref] wrote:
quoted
Hi Laurent,
Thanks for your detail suggestion.
On Wed, 2021-08-11 at 20:59 +0300, Laurent Pinchart wrote:
quoted
Hi Yunfei,
Thank you for the patch.
On Wed, Aug 11, 2021 at 10:57:59AM +0800, Yunfei Dong wrote:
quoted
Adds decoder dt-bindings for mt8192.
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
v5: no changes
This patch depends on "Mediatek MT8192 clock support"[1].
The definition of decoder clocks are in mt8192-clk.h, need
to
include them in case of build fail [1].
[1]
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek Video Decode Accelerator With Component
+
+maintainers:
+ - Yunfei Dong [off-list ref]
+
+description: |+
+ Mediatek Video Decode is the video decode hardware
present
in
Mediatek
+ SoCs which supports high resolution decoding
functionalities.
Required
+ master and component node.
This should explain how the three IP cores relate to each
other.
I'm a bit late in the game, reviewing v5 only, but I'm
wondering
if
those IP cores need to be modelled in separate nodes. It
would be
much
easier, from a software point of view, to have a single node,
with
multiple register ranges.
Are some of those IP cores used in different SoCs, combined
in
different
ways, that make a modular design better ?
Different platform has different hardware, for mt8192 only has
three
nodes. but mt8195 will has five nodes. and the
clk/power/irq/iommu
are
different. It is not easy to manage all hardware at the same
time
in
one node, need to enable different hardware at the same time,
the
logic
will be very complex.
It is much easier to handle different hardware using component,
enable
different hardware when we need it.
You can still have one device-tree node for each device, which
means
you can still manage your resources (clk/power/irq/iommu) easily,
but
doing
this so that it avoids an async framework to pull the parts
together.
It mean that v4l2 async also can't be used?
No, I don't think it's needed. I will take a look at the device tree
and see if I can show you an alternative approach that doesn't
require an async framework.
Thanks.
quoted
quoted
quoted
I gave you this feedback several times, and you have been
objecting
it
every time without even trying to consider a different approach.
I'm not objecting your suggestion every time, for our driver is
designed for component api, need to spend a lot of time to change
it in
many platforms. We need to get a final solution for this
architecture,
or it's not easy to change all patches in the feature.
quoted
Thanks,
Ezequiel
1. Each a node should respect a HW node. Defining a complex node
that
contain multiple register is not better, for they belong to
different
hardware. Different platforms has different hardware count, mt8195
has
five hardwares.
2. Another reason is from the IOMMU point, the vcodec HW include
core
and lat hardwares, each of them connect to the independent IOMMU
hardware for mt8195, can't write all iommu ports together, or
hardware
can't access dram data, so we must separate them.
For these limitation, can't combine all hardware together. I will
write
the hardware diagram in next patch.
No need for another patch, you can write that hardware diagram
now, so we can help with some design ideas.
This isn't what was really requested though. This is a diagram of how you
designed the software to operate the hardware. Ezequiel is asking for a
diagram of how the hardware is actually designed, how the different blocks
fit together, and how the hardware itself operates. Something like the
block diagrams commonly seen in the datasheets.
ChenYu