From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CBC9CC432BE for ; Wed, 1 Sep 2021 11:51:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 99B2F60F6C for ; Wed, 1 Sep 2021 11:51:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243882AbhIALwE (ORCPT ); Wed, 1 Sep 2021 07:52:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48766 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S243198AbhIALwC (ORCPT ); Wed, 1 Sep 2021 07:52:02 -0400 Received: from mail-ej1-x629.google.com (mail-ej1-x629.google.com [IPv6:2a00:1450:4864:20::629]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 66A4BC061575 for ; Wed, 1 Sep 2021 04:51:05 -0700 (PDT) Received: by mail-ej1-x629.google.com with SMTP id t19so5979772ejr.8 for ; Wed, 01 Sep 2021 04:51:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vanguardiasur-com-ar.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=uOKShc+WYnMIyB9CjzG3Z7cKQEUyUYeyvbUWTQN1OA8=; b=A2VXQJcZyRdsMmFVn55x/9V+Wes21QUSR4y2zTr3dlLBLRC4nV3ov0V5JB3Nv2rktX Vj5Es6BLGHEN6UGYgEcZnjy/luVor5j2rGxsyR8e3qRz6TsA5tSGL1LQu6xbDViMYJfB UZd5MuTs2n8nNWPb4SiFnZ7JmsegQa3H7Gi5r8unB3BNkKsb8JKlxczHXyswWD+XSKR3 V6A6wz0xB8JwcimJGFXJwkR33UB9JFum2Z6Ezxiq8dOs6rRp6EXHgQDN5P2ITugUW79q uoPMIYSdmolesOPcseqhtPVljinE3Z+m+Nl1ANCBVwZzIuUQRR4YayIACa2KCDEhyJYl r1pQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=uOKShc+WYnMIyB9CjzG3Z7cKQEUyUYeyvbUWTQN1OA8=; b=gPcRbffQVvpIoUHpl+c5IwnqVM4/WysxznQX0VByfoGaHqO+ZKXtzv29NcKHfjvm+w oHGDDQEYMHDqTqr33/2QRR+zs4/eow98/DS+Z/lVqj08NAtF0oOW2vqLum2X7j0Qpn0X r0EACZ9NJe0KnIhol0iPHPgzB/qU6cJhMdE5FU1QARPc9e7DaWdL8u470Txb+41X/vrw pWvlhk8oYdIiezWsb2PVQqda3OQdcmVi5M6qv2xDZ+bhbpleoq0Kiwm/1OdviJ6s8/Ch OE5elEz++YBrBuoD71aw2PRCQe0m5W3n7YM07GlgnpCZzczcgywFdCjzCyr7eOaMD1zf foAQ== X-Gm-Message-State: AOAM532yMgb2oVHK1S4pdYCtxjNjm1u3bJjPZrjAnM5f+ycIfT5m/sW+ U5qDhgZcBbGq7MIgVGS9dTDmAuPMbK1WWip6UGghzg== X-Google-Smtp-Source: ABdhPJwjFkys34+/Kq9OTrga8NV7WQe+xOMBu0yoIqzEl2gh/Thkm/Ncu6x6UaVnu4XPasukDVHMUjbCXCYs0832saM= X-Received: by 2002:a17:906:408c:: with SMTP id u12mr37218674ejj.413.1630497063860; Wed, 01 Sep 2021 04:51:03 -0700 (PDT) MIME-Version: 1.0 References: <20210811025801.21597-1-yunfei.dong@mediatek.com> <20210811025801.21597-14-yunfei.dong@mediatek.com> <952c219de7595f7f814d3006fbe25b8089a35212.camel@mediatek.com> In-Reply-To: From: Ezequiel Garcia Date: Wed, 1 Sep 2021 08:50:52 -0300 Message-ID: Subject: Re: [PATCH v5, 13/15] dt-bindings: media: mtk-vcodec: Adds decoder dt-bindings for mt8192 To: "yunfei.dong@mediatek.com" Cc: Laurent Pinchart , Alexandre Courbot , Hans Verkuil , Tzung-Bi Shih , Tiffany Lin , Andrew-CT Chen , Mauro Carvalho Chehab , Rob Herring , Matthias Brugger , Tomasz Figa , Hsin-Yi Wang , Fritz Koenig , Irui Wang , linux-media , devicetree , Linux Kernel Mailing List , linux-arm-kernel , srv_heupstream , "moderated list:ARM/Mediatek SoC support" , Project_Global_Chrome_Upstream_Group , George Sun Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 1 Sept 2021 at 00:49, yunfei.dong@mediatek.com wrote: > > 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 > > wrote: > > > > > > 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: > > > > > Adds decoder dt-bindings for mt8192. > > > > > > > > > > Signed-off-by: Yunfei Dong > > > > > --- > > > > > 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 > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > > > > comp- > > > > > decoder.yaml > > > > > @@ -0,0 +1,172 @@ > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > +%YAML 1.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 > > > > > + > > > > > +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 will explain it in next patch. > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > + oneOf: > > > > > + - enum: > > > > > + - mediatek,mt8192-vcodec-dec # for lat hardware > > > > > + - mediatek,mtk-vcodec-lat # for core hardware > > > > > + - mediatek,mtk-vcodec-core > > > > > + > > > > > + reg: > > > > > + maxItems: 1 > > > > > + > > > > > + interrupts: > > > > > + maxItems: 1 > > > > > + > > > > > + clocks: > > > > > + maxItems: 5 > > > > > + > > > > > + clock-names: > > > > > + items: > > > > > + - const: vdec-sel > > > > > + - const: vdec-soc-vdec > > > > > + - const: vdec-soc-lat > > > > > + - const: vdec-vdec > > > > > + - const: vdec-top > > > > > + > > > > > + assigned-clocks: true > > > > > + > > > > > + assigned-clock-parents: true > > > > > + > > > > > + power-domains: > > > > > + maxItems: 1 > > > > > + > > > > > + 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,scp: > > > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > > > + maxItems: 1 > > > > > + description: > > > > > + Describes point to scp. > > > > > + > > > > > +required: > > > > > + - compatible > > > > > + - reg > > > > > + - iommus > > > > > + - dma-ranges > > > > > + > > > > > +allOf: > > > > > + - if: #master node > > > > > + properties: > > > > > + compatible: > > > > > + contains: > > > > > + enum: > > > > > + - mediatek,mt8192-vcodec-dec # for lat hardware > > > > > + > > > > > + then: > > > > > + required: > > > > > + - mediatek,scp > > > > > + > > > > > + - if: #component node > > > > > + properties: > > > > > + compatible: > > > > > + contains: > > > > > + enum: > > > > > + - mediatek,mtk-vcodec-lat # for core > > > > > hardware > > > > > + - mediatek,mtk-vcodec-core > > > > > + > > > > > + then: > > > > > + required: > > > > > + - interrupts > > > > > + - clocks > > > > > + - clock-names > > > > > + - assigned-clocks > > > > > + - assigned-clock-parents > > > > > + - power-domains > > > > > + > > > > > + > > > > > +additionalProperties: false > > > > > + > > > > > +examples: > > > > > + - | > > > > > + #include > > > > > + #include > > > > > + #include > > > > > + #include > > > > > + #include > > > > > + > > > > > + vcodec_dec: vcodec_dec@16000000 { > > > > > + compatible = "mediatek,mt8192-vcodec-dec"; > > > > > + reg = <0 0x16000000 0 0x1000>; /* VDEC_SYS > > > > > */ > > > > > + mediatek,scp = <&scp>; > > > > > + iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>; > > > > > + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>; > > > > > + }; > > > > > + > > > > > + vcodec_lat: vcodec_lat@0x16010000 { > > > > > + compatible = "mediatek,mtk-vcodec-lat"; > > > > > + reg = <0 0x16010000 0 0x800>; /* > > > > > VDEC_MISC */ > > > > > + interrupts = ; > > > > > + iommus = <&iommu0 M4U_PORT_L5_VDEC_LAT0_VLD_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_VLD2_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_AVC_MV_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_PRED_RD_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_TILE_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_WDMA_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_RG_CTRL_DMA_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_UFO_ENC_EXT>; > > > > > + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>; > > > > > + clocks = <&topckgen CLK_TOP_VDEC_SEL>, > > > > > + <&vdecsys_soc CLK_VDEC_SOC_VDEC>, > > > > > + <&vdecsys_soc CLK_VDEC_SOC_LAT>, > > > > > + <&vdecsys_soc CLK_VDEC_SOC_LARB1>, > > > > > + <&topckgen CLK_TOP_MAINPLL_D4>; > > > > > + clock-names = "vdec-sel", "vdec-soc-vdec", "vdec-soc- > > > > > lat", > > > > > + "vdec-vdec", "vdec-top"; > > > > > + assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>; > > > > > + assigned-clock-parents = <&topckgen > > > > > CLK_TOP_MAINPLL_D4>; > > > > > + power-domains = <&spm MT8192_POWER_DOMAIN_VDEC>; > > > > > + }; > > > > > + > > > > > + vcodec_core: vcodec_core@0x16025000 { > > > > > + compatible = "mediatek,mtk-vcodec-core"; > > > > > + reg = <0 0x16025000 0 0x1000>; /* > > > > > VDEC_CORE_MISC */ > > > > > + interrupts = ; > > > > > + iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_UFO_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_PP_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_PRED_RD_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_PRED_WR_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_PPWRAP_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_TILE_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_VLD_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_VLD2_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_AVC_MV_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_RG_CTRL_DMA_EXT>; > > > > > + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>; > > > > > + clocks = <&topckgen CLK_TOP_VDEC_SEL>, > > > > > + <&vdecsys CLK_VDEC_VDEC>, > > > > > + <&vdecsys CLK_VDEC_LAT>, > > > > > + <&vdecsys CLK_VDEC_LARB1>, > > > > > + <&topckgen CLK_TOP_MAINPLL_D4>; > > > > > + clock-names = "vdec-sel", "vdec-soc-vdec", "vdec-soc- > > > > > lat", > > > > > + "vdec-vdec", "vdec-top"; > > > > > + assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>; > > > > > + assigned-clock-parents = <&topckgen > > > > > CLK_TOP_MAINPLL_D4>; > > > > > + power-domains = <&spm MT8192_POWER_DOMAIN_VDEC2>; > > > > > + }; > > > > > > > > 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. > > 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. > No need for another patch, you can write that hardware diagram now, so we can help with some design ideas. Thanks! Ezequiel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 28B6DC432BE for ; Wed, 1 Sep 2021 11:51:26 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D036661008 for ; Wed, 1 Sep 2021 11:51:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org D036661008 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=vanguardiasur.com.ar Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=UjDtS/Wy3af5nZ9I+ZshdjW7PT2ptgPtkrrRc1iqcsc=; b=d+xwaz6dh23Agr +emUIT0WvGIFLKZcafj1hExQZW59/9mExR2TWnX17ye6l41inYYpOhQ1yf79B+cgLOF93tOuyzKG7 lFJwjUwLhi3N0ZENELxgpUbv7ALfbhfy4QbxwNKdUnDl5u+0WrEeWfDxspEqQK1Sw3qIlktL7Gat/ VId9wpjdlc5DbsRSNxjRdwGLEy3CLr9dc369YnBCXuzSNmPge0b4PKfWzAmUW2czbVf7OLbjZFcS3 ludZq5Z66oNI0fe4dN/0kzfI5oFGoH1nF3bGlSKLM0/hIyDzyHDh9viCt0utawweo1d74Yak1y0HL TLycNax+lBA6gVMcwwbw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mLOlu-005gKO-7U; Wed, 01 Sep 2021 11:51:14 +0000 Received: from mail-ej1-x629.google.com ([2a00:1450:4864:20::629]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mLOlp-005gIq-4u for linux-mediatek@lists.infradead.org; Wed, 01 Sep 2021 11:51:13 +0000 Received: by mail-ej1-x629.google.com with SMTP id a25so5999047ejv.6 for ; Wed, 01 Sep 2021 04:51:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vanguardiasur-com-ar.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=uOKShc+WYnMIyB9CjzG3Z7cKQEUyUYeyvbUWTQN1OA8=; b=A2VXQJcZyRdsMmFVn55x/9V+Wes21QUSR4y2zTr3dlLBLRC4nV3ov0V5JB3Nv2rktX Vj5Es6BLGHEN6UGYgEcZnjy/luVor5j2rGxsyR8e3qRz6TsA5tSGL1LQu6xbDViMYJfB UZd5MuTs2n8nNWPb4SiFnZ7JmsegQa3H7Gi5r8unB3BNkKsb8JKlxczHXyswWD+XSKR3 V6A6wz0xB8JwcimJGFXJwkR33UB9JFum2Z6Ezxiq8dOs6rRp6EXHgQDN5P2ITugUW79q uoPMIYSdmolesOPcseqhtPVljinE3Z+m+Nl1ANCBVwZzIuUQRR4YayIACa2KCDEhyJYl r1pQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=uOKShc+WYnMIyB9CjzG3Z7cKQEUyUYeyvbUWTQN1OA8=; b=j1777YytlqEWi5R/5JvwyCgEaOkUcJetpIKcnrO2KvpXrbH84j//gDSFsbNyqDYRo9 HwqayqoWMkR1Lwwn5OW1laIZVdgCre+EWFbEDAgX6FPW/uLA/kVRnQJq7kYPPs+Nu5hx BhAY9Zv7MmzsMozB1YkqpLrtgHKZY192WkBUgKFFAh6JqcNUZN32btUY41md4fjAtxo4 x7kQ44CFKttu9mWLl2sKe4tu2Jj/ywHh6jKfoJAowtUQkc+eYzG04wHEZyvDwgFEUJoN bMhM67lK6IXOnUsTeSQOhmua/5oGl3oJWDBsYL/QB/5IsObyiVxRQGsR+hyOSe4L0SLW L4AQ== X-Gm-Message-State: AOAM531BYpVIN3OloSkt1aYw/zrnEkyzyu3iLtpCk+MGy8PjAGwkVO5G ZGLsqgh0isEBrnAt1/G2Pfv0yr+ulfdHd9zK0qxiLw== X-Google-Smtp-Source: ABdhPJwjFkys34+/Kq9OTrga8NV7WQe+xOMBu0yoIqzEl2gh/Thkm/Ncu6x6UaVnu4XPasukDVHMUjbCXCYs0832saM= X-Received: by 2002:a17:906:408c:: with SMTP id u12mr37218674ejj.413.1630497063860; Wed, 01 Sep 2021 04:51:03 -0700 (PDT) MIME-Version: 1.0 References: <20210811025801.21597-1-yunfei.dong@mediatek.com> <20210811025801.21597-14-yunfei.dong@mediatek.com> <952c219de7595f7f814d3006fbe25b8089a35212.camel@mediatek.com> In-Reply-To: From: Ezequiel Garcia Date: Wed, 1 Sep 2021 08:50:52 -0300 Message-ID: Subject: Re: [PATCH v5, 13/15] dt-bindings: media: mtk-vcodec: Adds decoder dt-bindings for mt8192 To: "yunfei.dong@mediatek.com" Cc: Laurent Pinchart , Alexandre Courbot , Hans Verkuil , Tzung-Bi Shih , Tiffany Lin , Andrew-CT Chen , Mauro Carvalho Chehab , Rob Herring , Matthias Brugger , Tomasz Figa , Hsin-Yi Wang , Fritz Koenig , Irui Wang , linux-media , devicetree , Linux Kernel Mailing List , linux-arm-kernel , srv_heupstream , "moderated list:ARM/Mediatek SoC support" , Project_Global_Chrome_Upstream_Group , George Sun X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210901_045109_225282_B5386603 X-CRM114-Status: GOOD ( 56.60 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Wed, 1 Sept 2021 at 00:49, yunfei.dong@mediatek.com wrote: > > 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 > > wrote: > > > > > > 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: > > > > > Adds decoder dt-bindings for mt8192. > > > > > > > > > > Signed-off-by: Yunfei Dong > > > > > --- > > > > > 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 > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > > > > comp- > > > > > decoder.yaml > > > > > @@ -0,0 +1,172 @@ > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > +%YAML 1.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 > > > > > + > > > > > +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 will explain it in next patch. > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > + oneOf: > > > > > + - enum: > > > > > + - mediatek,mt8192-vcodec-dec # for lat hardware > > > > > + - mediatek,mtk-vcodec-lat # for core hardware > > > > > + - mediatek,mtk-vcodec-core > > > > > + > > > > > + reg: > > > > > + maxItems: 1 > > > > > + > > > > > + interrupts: > > > > > + maxItems: 1 > > > > > + > > > > > + clocks: > > > > > + maxItems: 5 > > > > > + > > > > > + clock-names: > > > > > + items: > > > > > + - const: vdec-sel > > > > > + - const: vdec-soc-vdec > > > > > + - const: vdec-soc-lat > > > > > + - const: vdec-vdec > > > > > + - const: vdec-top > > > > > + > > > > > + assigned-clocks: true > > > > > + > > > > > + assigned-clock-parents: true > > > > > + > > > > > + power-domains: > > > > > + maxItems: 1 > > > > > + > > > > > + 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,scp: > > > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > > > + maxItems: 1 > > > > > + description: > > > > > + Describes point to scp. > > > > > + > > > > > +required: > > > > > + - compatible > > > > > + - reg > > > > > + - iommus > > > > > + - dma-ranges > > > > > + > > > > > +allOf: > > > > > + - if: #master node > > > > > + properties: > > > > > + compatible: > > > > > + contains: > > > > > + enum: > > > > > + - mediatek,mt8192-vcodec-dec # for lat hardware > > > > > + > > > > > + then: > > > > > + required: > > > > > + - mediatek,scp > > > > > + > > > > > + - if: #component node > > > > > + properties: > > > > > + compatible: > > > > > + contains: > > > > > + enum: > > > > > + - mediatek,mtk-vcodec-lat # for core > > > > > hardware > > > > > + - mediatek,mtk-vcodec-core > > > > > + > > > > > + then: > > > > > + required: > > > > > + - interrupts > > > > > + - clocks > > > > > + - clock-names > > > > > + - assigned-clocks > > > > > + - assigned-clock-parents > > > > > + - power-domains > > > > > + > > > > > + > > > > > +additionalProperties: false > > > > > + > > > > > +examples: > > > > > + - | > > > > > + #include > > > > > + #include > > > > > + #include > > > > > + #include > > > > > + #include > > > > > + > > > > > + vcodec_dec: vcodec_dec@16000000 { > > > > > + compatible = "mediatek,mt8192-vcodec-dec"; > > > > > + reg = <0 0x16000000 0 0x1000>; /* VDEC_SYS > > > > > */ > > > > > + mediatek,scp = <&scp>; > > > > > + iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>; > > > > > + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>; > > > > > + }; > > > > > + > > > > > + vcodec_lat: vcodec_lat@0x16010000 { > > > > > + compatible = "mediatek,mtk-vcodec-lat"; > > > > > + reg = <0 0x16010000 0 0x800>; /* > > > > > VDEC_MISC */ > > > > > + interrupts = ; > > > > > + iommus = <&iommu0 M4U_PORT_L5_VDEC_LAT0_VLD_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_VLD2_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_AVC_MV_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_PRED_RD_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_TILE_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_WDMA_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_RG_CTRL_DMA_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_UFO_ENC_EXT>; > > > > > + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>; > > > > > + clocks = <&topckgen CLK_TOP_VDEC_SEL>, > > > > > + <&vdecsys_soc CLK_VDEC_SOC_VDEC>, > > > > > + <&vdecsys_soc CLK_VDEC_SOC_LAT>, > > > > > + <&vdecsys_soc CLK_VDEC_SOC_LARB1>, > > > > > + <&topckgen CLK_TOP_MAINPLL_D4>; > > > > > + clock-names = "vdec-sel", "vdec-soc-vdec", "vdec-soc- > > > > > lat", > > > > > + "vdec-vdec", "vdec-top"; > > > > > + assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>; > > > > > + assigned-clock-parents = <&topckgen > > > > > CLK_TOP_MAINPLL_D4>; > > > > > + power-domains = <&spm MT8192_POWER_DOMAIN_VDEC>; > > > > > + }; > > > > > + > > > > > + vcodec_core: vcodec_core@0x16025000 { > > > > > + compatible = "mediatek,mtk-vcodec-core"; > > > > > + reg = <0 0x16025000 0 0x1000>; /* > > > > > VDEC_CORE_MISC */ > > > > > + interrupts = ; > > > > > + iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_UFO_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_PP_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_PRED_RD_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_PRED_WR_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_PPWRAP_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_TILE_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_VLD_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_VLD2_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_AVC_MV_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_RG_CTRL_DMA_EXT>; > > > > > + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>; > > > > > + clocks = <&topckgen CLK_TOP_VDEC_SEL>, > > > > > + <&vdecsys CLK_VDEC_VDEC>, > > > > > + <&vdecsys CLK_VDEC_LAT>, > > > > > + <&vdecsys CLK_VDEC_LARB1>, > > > > > + <&topckgen CLK_TOP_MAINPLL_D4>; > > > > > + clock-names = "vdec-sel", "vdec-soc-vdec", "vdec-soc- > > > > > lat", > > > > > + "vdec-vdec", "vdec-top"; > > > > > + assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>; > > > > > + assigned-clock-parents = <&topckgen > > > > > CLK_TOP_MAINPLL_D4>; > > > > > + power-domains = <&spm MT8192_POWER_DOMAIN_VDEC2>; > > > > > + }; > > > > > > > > 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. > > 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. > No need for another patch, you can write that hardware diagram now, so we can help with some design ideas. Thanks! Ezequiel _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2645EC432BE for ; Wed, 1 Sep 2021 11:53:57 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id EC38860F6C for ; Wed, 1 Sep 2021 11:53:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org EC38860F6C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=vanguardiasur.com.ar Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=TjLzYOf55lqjlpefQna/cBC9Wq/MuioWxeYpVnG8e0M=; b=vIcMWkXtY5Lmlo W+3niIzed1mjrnANpYTlPkqyGK8kSeYfLnztAZyWNAas9GDhDKcSutDDPmSDalChlnkyRbcTmJDHg vW2PPA724axL8Q9o01jkrZxPTCvjqEBSkvRvfb+cPzdGYf03xZa2LYRPupOVe2iqbkjUBNXdd0I+a URZL9rG2x/kUDP9iqPFkPakpv8gNmt9aXVNSl3lKuwvyf3hOzYi1qZuERVbJ0Nwa1dXVTeaxoS0GZ 9akAsZ+pG7zxaU8aTUxwfPHwuG0S0G2iH3VrcEH3KC06yBhjoCNQ3L0WwaSqV4vKNF1VMW/S4+9y/ rDgduPBMZ2vhWkmlTdmA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mLOlw-005gLB-8X; Wed, 01 Sep 2021 11:51:16 +0000 Received: from mail-ej1-x630.google.com ([2a00:1450:4864:20::630]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mLOlp-005gIp-5e for linux-arm-kernel@lists.infradead.org; Wed, 01 Sep 2021 11:51:14 +0000 Received: by mail-ej1-x630.google.com with SMTP id lc21so5991035ejc.7 for ; Wed, 01 Sep 2021 04:51:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vanguardiasur-com-ar.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=uOKShc+WYnMIyB9CjzG3Z7cKQEUyUYeyvbUWTQN1OA8=; b=A2VXQJcZyRdsMmFVn55x/9V+Wes21QUSR4y2zTr3dlLBLRC4nV3ov0V5JB3Nv2rktX Vj5Es6BLGHEN6UGYgEcZnjy/luVor5j2rGxsyR8e3qRz6TsA5tSGL1LQu6xbDViMYJfB UZd5MuTs2n8nNWPb4SiFnZ7JmsegQa3H7Gi5r8unB3BNkKsb8JKlxczHXyswWD+XSKR3 V6A6wz0xB8JwcimJGFXJwkR33UB9JFum2Z6Ezxiq8dOs6rRp6EXHgQDN5P2ITugUW79q uoPMIYSdmolesOPcseqhtPVljinE3Z+m+Nl1ANCBVwZzIuUQRR4YayIACa2KCDEhyJYl r1pQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=uOKShc+WYnMIyB9CjzG3Z7cKQEUyUYeyvbUWTQN1OA8=; b=Sr6gb95MiSA5QO8vFZRr35Qsr+a6cqzNAqfGyBvHJMVw8SdFjqHgMVNDK2Ya0S29ei u7/9Wo/+kyXTd8mu+VuKEG5+I1opN/8dLwLYbGvXwZlSjs6QC1uODNSFiPb5spsty3fE IylLvlMIMdwbGmRiCL4T8g7J218tYyiPMPu2z7w0IJ7coG/K+nVtPADwysNp822evnwX c7AgRso5yYvIUjb9v/GRyMb8Rvfy6xyYbDlcbmIDQIRCTFEMmHJBW5arSk92Xs1v9Hmy XE+TUCagio4hd+aHxPb0jI/q1Odl5Uk+IL33cLz/TNi4LpkAAcQLv5fNhP07paSnXRF3 /XSA== X-Gm-Message-State: AOAM533ZZp6eXgGRbSDuOPfPeeK1wxC993HMp08aAhukaYUnG9H0tFoi TLBNm55U+sS+ygXvqetMzh+LgqPh5IGWGFeNnbjSvg== X-Google-Smtp-Source: ABdhPJwjFkys34+/Kq9OTrga8NV7WQe+xOMBu0yoIqzEl2gh/Thkm/Ncu6x6UaVnu4XPasukDVHMUjbCXCYs0832saM= X-Received: by 2002:a17:906:408c:: with SMTP id u12mr37218674ejj.413.1630497063860; Wed, 01 Sep 2021 04:51:03 -0700 (PDT) MIME-Version: 1.0 References: <20210811025801.21597-1-yunfei.dong@mediatek.com> <20210811025801.21597-14-yunfei.dong@mediatek.com> <952c219de7595f7f814d3006fbe25b8089a35212.camel@mediatek.com> In-Reply-To: From: Ezequiel Garcia Date: Wed, 1 Sep 2021 08:50:52 -0300 Message-ID: Subject: Re: [PATCH v5, 13/15] dt-bindings: media: mtk-vcodec: Adds decoder dt-bindings for mt8192 To: "yunfei.dong@mediatek.com" Cc: Laurent Pinchart , Alexandre Courbot , Hans Verkuil , Tzung-Bi Shih , Tiffany Lin , Andrew-CT Chen , Mauro Carvalho Chehab , Rob Herring , Matthias Brugger , Tomasz Figa , Hsin-Yi Wang , Fritz Koenig , Irui Wang , linux-media , devicetree , Linux Kernel Mailing List , linux-arm-kernel , srv_heupstream , "moderated list:ARM/Mediatek SoC support" , Project_Global_Chrome_Upstream_Group , George Sun X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210901_045109_245623_13CD0A1A X-CRM114-Status: GOOD ( 58.01 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, 1 Sept 2021 at 00:49, yunfei.dong@mediatek.com wrote: > > 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 > > wrote: > > > > > > 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: > > > > > Adds decoder dt-bindings for mt8192. > > > > > > > > > > Signed-off-by: Yunfei Dong > > > > > --- > > > > > 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 > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > > > > comp- > > > > > decoder.yaml > > > > > @@ -0,0 +1,172 @@ > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > +%YAML 1.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 > > > > > + > > > > > +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 will explain it in next patch. > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > + oneOf: > > > > > + - enum: > > > > > + - mediatek,mt8192-vcodec-dec # for lat hardware > > > > > + - mediatek,mtk-vcodec-lat # for core hardware > > > > > + - mediatek,mtk-vcodec-core > > > > > + > > > > > + reg: > > > > > + maxItems: 1 > > > > > + > > > > > + interrupts: > > > > > + maxItems: 1 > > > > > + > > > > > + clocks: > > > > > + maxItems: 5 > > > > > + > > > > > + clock-names: > > > > > + items: > > > > > + - const: vdec-sel > > > > > + - const: vdec-soc-vdec > > > > > + - const: vdec-soc-lat > > > > > + - const: vdec-vdec > > > > > + - const: vdec-top > > > > > + > > > > > + assigned-clocks: true > > > > > + > > > > > + assigned-clock-parents: true > > > > > + > > > > > + power-domains: > > > > > + maxItems: 1 > > > > > + > > > > > + 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,scp: > > > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > > > + maxItems: 1 > > > > > + description: > > > > > + Describes point to scp. > > > > > + > > > > > +required: > > > > > + - compatible > > > > > + - reg > > > > > + - iommus > > > > > + - dma-ranges > > > > > + > > > > > +allOf: > > > > > + - if: #master node > > > > > + properties: > > > > > + compatible: > > > > > + contains: > > > > > + enum: > > > > > + - mediatek,mt8192-vcodec-dec # for lat hardware > > > > > + > > > > > + then: > > > > > + required: > > > > > + - mediatek,scp > > > > > + > > > > > + - if: #component node > > > > > + properties: > > > > > + compatible: > > > > > + contains: > > > > > + enum: > > > > > + - mediatek,mtk-vcodec-lat # for core > > > > > hardware > > > > > + - mediatek,mtk-vcodec-core > > > > > + > > > > > + then: > > > > > + required: > > > > > + - interrupts > > > > > + - clocks > > > > > + - clock-names > > > > > + - assigned-clocks > > > > > + - assigned-clock-parents > > > > > + - power-domains > > > > > + > > > > > + > > > > > +additionalProperties: false > > > > > + > > > > > +examples: > > > > > + - | > > > > > + #include > > > > > + #include > > > > > + #include > > > > > + #include > > > > > + #include > > > > > + > > > > > + vcodec_dec: vcodec_dec@16000000 { > > > > > + compatible = "mediatek,mt8192-vcodec-dec"; > > > > > + reg = <0 0x16000000 0 0x1000>; /* VDEC_SYS > > > > > */ > > > > > + mediatek,scp = <&scp>; > > > > > + iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>; > > > > > + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>; > > > > > + }; > > > > > + > > > > > + vcodec_lat: vcodec_lat@0x16010000 { > > > > > + compatible = "mediatek,mtk-vcodec-lat"; > > > > > + reg = <0 0x16010000 0 0x800>; /* > > > > > VDEC_MISC */ > > > > > + interrupts = ; > > > > > + iommus = <&iommu0 M4U_PORT_L5_VDEC_LAT0_VLD_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_VLD2_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_AVC_MV_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_PRED_RD_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_TILE_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_WDMA_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_LAT0_RG_CTRL_DMA_EXT>, > > > > > + <&iommu0 M4U_PORT_L5_VDEC_UFO_ENC_EXT>; > > > > > + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>; > > > > > + clocks = <&topckgen CLK_TOP_VDEC_SEL>, > > > > > + <&vdecsys_soc CLK_VDEC_SOC_VDEC>, > > > > > + <&vdecsys_soc CLK_VDEC_SOC_LAT>, > > > > > + <&vdecsys_soc CLK_VDEC_SOC_LARB1>, > > > > > + <&topckgen CLK_TOP_MAINPLL_D4>; > > > > > + clock-names = "vdec-sel", "vdec-soc-vdec", "vdec-soc- > > > > > lat", > > > > > + "vdec-vdec", "vdec-top"; > > > > > + assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>; > > > > > + assigned-clock-parents = <&topckgen > > > > > CLK_TOP_MAINPLL_D4>; > > > > > + power-domains = <&spm MT8192_POWER_DOMAIN_VDEC>; > > > > > + }; > > > > > + > > > > > + vcodec_core: vcodec_core@0x16025000 { > > > > > + compatible = "mediatek,mtk-vcodec-core"; > > > > > + reg = <0 0x16025000 0 0x1000>; /* > > > > > VDEC_CORE_MISC */ > > > > > + interrupts = ; > > > > > + iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_UFO_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_PP_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_PRED_RD_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_PRED_WR_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_PPWRAP_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_TILE_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_VLD_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_VLD2_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_AVC_MV_EXT>, > > > > > + <&iommu0 M4U_PORT_L4_VDEC_RG_CTRL_DMA_EXT>; > > > > > + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>; > > > > > + clocks = <&topckgen CLK_TOP_VDEC_SEL>, > > > > > + <&vdecsys CLK_VDEC_VDEC>, > > > > > + <&vdecsys CLK_VDEC_LAT>, > > > > > + <&vdecsys CLK_VDEC_LARB1>, > > > > > + <&topckgen CLK_TOP_MAINPLL_D4>; > > > > > + clock-names = "vdec-sel", "vdec-soc-vdec", "vdec-soc- > > > > > lat", > > > > > + "vdec-vdec", "vdec-top"; > > > > > + assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>; > > > > > + assigned-clock-parents = <&topckgen > > > > > CLK_TOP_MAINPLL_D4>; > > > > > + power-domains = <&spm MT8192_POWER_DOMAIN_VDEC2>; > > > > > + }; > > > > > > > > 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. > > 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. > No need for another patch, you can write that hardware diagram now, so we can help with some design ideas. Thanks! Ezequiel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel