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=-16.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,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 464D0C433EF for ; Fri, 3 Sep 2021 04:02:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 28535603E9 for ; Fri, 3 Sep 2021 04:02:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231892AbhICEDs (ORCPT ); Fri, 3 Sep 2021 00:03:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37070 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231748AbhICEDo (ORCPT ); Fri, 3 Sep 2021 00:03:44 -0400 Received: from mail-lj1-x229.google.com (mail-lj1-x229.google.com [IPv6:2a00:1450:4864:20::229]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 75986C061757 for ; Thu, 2 Sep 2021 21:02:44 -0700 (PDT) Received: by mail-lj1-x229.google.com with SMTP id y6so7544762lje.2 for ; Thu, 02 Sep 2021 21:02:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hsqolKVO4nOJoFFGBqwd8L+i7kJ81PMtNw0t/34Bbxc=; b=a+IMKpSwOaBkBCR2jhE3eDJFkMxdZGfrTPA91s5BiAXKK1vbq/JCB59IkKv6KCUtql f9BVn1U4h9ANDqU8CgHzLZporyLsDXCdf9/2C09SchnyqGNjRUj1gU+lBRQT2AnAqztU 9rW80S9PDgEp8TtrnhFEFze2kWMWQAwV71cl4= 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=hsqolKVO4nOJoFFGBqwd8L+i7kJ81PMtNw0t/34Bbxc=; b=hUKkpMfr+mN1apCUwt6fu5NJMHDvztbZWZE1uIT1/qqzmTz0MJGgWl9kAO7HqUwubY 3NZw3aP5xpEqJjvF/jB4OAZvlcQZV4Uj+/uQLHgl2oCc+H1lWjTM280y6YUaZw27XJF5 iBSOD0DpfLmxZpYLRyGMwt0naW4wyzSqsP3rRx1VWrO8LyWbXvewTWS/HkJ23+CzXl1M p6jLromrXk4Ecb9Q50SpTfAvR0qdc2Ew8w5xjdJr8QEvfIwsWENNgVlFfsGLE5aI0IUD j0IWr4qNbRF1gx1ruohzAaaMTqf3B76m2oy8+rtv3p4q1+yDB2okhy11nnl68LlmqNvf Or7w== X-Gm-Message-State: AOAM533pnrNNTRuo4T6cKh78KSyYjXoofhiZgE0A6oM5fhkspFsrV7dh ALgAe6KXu/NoItsWqPC25KJIUhPtjMDCj5kMOfnnRg== X-Google-Smtp-Source: ABdhPJxar47W9Hxbv7flNSitG5YspCZFWGxl4kNdEZYVIQGwUadVu8hYMZ5WnWzdLG9zy7rzurwuo3xhXN7yB8eeMJg= X-Received: by 2002:a2e:7d0e:: with SMTP id y14mr1247904ljc.251.1630641762596; Thu, 02 Sep 2021 21:02:42 -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> <9c3f871b3b58b8456e40c43fe402a7a4acb66660.camel@mediatek.com> In-Reply-To: <9c3f871b3b58b8456e40c43fe402a7a4acb66660.camel@mediatek.com> From: Chen-Yu Tsai Date: Fri, 3 Sep 2021 12:02:31 +0800 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: Ezequiel Garcia , 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 Thu, Sep 2, 2021 at 2:09 PM yunfei.dong@mediatek.com wrote: > > 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 > > 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. > > > Thanks. > > > > 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. > > > You can see the block diagram in patch: > > https://patchwork.linuxtv.org/project/linux-media/patch/20210901083215.25984-14-yunfei.dong@mediatek.com/ 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 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 339AFC433EF for ; Fri, 3 Sep 2021 04:03:23 +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 DDFB061059 for ; Fri, 3 Sep 2021 04:03:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org DDFB061059 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org 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=1cmgy41ZenQMFI66tghpCnkgS0Zib8/VDa7q8QeQMZs=; b=oWVmEc5M79MS51 fogQPxCAMsaNrrB1Cg0fPGNDJzXy77QLrmW3UaYnGvEfhCQ+vs+4MSZeBgf0rn1c3LSHAz5bv7wn+ sRyHkeusGaMuO4yDSNJw5Hgf1i8nbLci+8z/V5mDY/4S2x1ezUfvUEJ4Xli9B1K3b0bHqlVIIs05M z5gcrN9pYpdHG/UdCOsNtKSWFi6ZANPo+a7AFWPC8g5oZVIMGtcy+g9N8XWRXeyg5llAxNIyCn9qg WoTKo+pq47A3V+bLHX09N4jYA6/8krtbNXtzpMovWQiKGao9CSLR28YC8dEWg0+CbU/HN0P28/ht9 NZM+yku9Tvl23gzakIag==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mM0Q0-00AvDw-HC; Fri, 03 Sep 2021 04:03:08 +0000 Received: from mail-lj1-x236.google.com ([2a00:1450:4864:20::236]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mM0Pf-00Av9W-BR for linux-mediatek@lists.infradead.org; Fri, 03 Sep 2021 04:02:50 +0000 Received: by mail-lj1-x236.google.com with SMTP id s3so7488127ljp.11 for ; Thu, 02 Sep 2021 21:02:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hsqolKVO4nOJoFFGBqwd8L+i7kJ81PMtNw0t/34Bbxc=; b=a+IMKpSwOaBkBCR2jhE3eDJFkMxdZGfrTPA91s5BiAXKK1vbq/JCB59IkKv6KCUtql f9BVn1U4h9ANDqU8CgHzLZporyLsDXCdf9/2C09SchnyqGNjRUj1gU+lBRQT2AnAqztU 9rW80S9PDgEp8TtrnhFEFze2kWMWQAwV71cl4= 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=hsqolKVO4nOJoFFGBqwd8L+i7kJ81PMtNw0t/34Bbxc=; b=ItBdsSrszAf34clEUm5PM4FYqYaX6UwYkHsKDIZElT905+0a8/Uxe6As0FQQdiBrlh JJhUm0lLYME2w0sOXZisN4YpogQPgrNB/GjYbGEh7nYldY3oTeYuoRbn1KBAxrZGYWAw xaa8TWtBLRonzxAwRDxRu1o45G1gkGC+CQ6ntwq22LwO7Ka4ylx3kfziS4bnUKaJYetI puwCtfKuu9ZJCCBHGPSqvcWQWpSA6akF38vlm9cA2j9CFFho/63jj/Sw0U/caCT8RiIl /eFQfDaeiJAjgyOdW/UaHh5XOwIgkVWYQUE6TjZLK7ETP6M25XqhOWksRu2y98lN1Wkv q4qg== X-Gm-Message-State: AOAM531I08jqcXot8NWm2wu+2o82XftBafdLGyyWJc1Q8RiGYVLFTybX D7QsJWPHuKEx7FhHj+fx5/ABXxXXHUkElE3zSll3ig== X-Google-Smtp-Source: ABdhPJxar47W9Hxbv7flNSitG5YspCZFWGxl4kNdEZYVIQGwUadVu8hYMZ5WnWzdLG9zy7rzurwuo3xhXN7yB8eeMJg= X-Received: by 2002:a2e:7d0e:: with SMTP id y14mr1247904ljc.251.1630641762596; Thu, 02 Sep 2021 21:02:42 -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> <9c3f871b3b58b8456e40c43fe402a7a4acb66660.camel@mediatek.com> In-Reply-To: <9c3f871b3b58b8456e40c43fe402a7a4acb66660.camel@mediatek.com> From: Chen-Yu Tsai Date: Fri, 3 Sep 2021 12:02:31 +0800 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: Ezequiel Garcia , 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-20210902_210247_429325_39CEC1C3 X-CRM114-Status: GOOD ( 48.56 ) 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 Thu, Sep 2, 2021 at 2:09 PM yunfei.dong@mediatek.com wrote: > > 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 > > 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. > > > Thanks. > > > > 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. > > > You can see the block diagram in patch: > > https://patchwork.linuxtv.org/project/linux-media/patch/20210901083215.25984-14-yunfei.dong@mediatek.com/ 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 _______________________________________________ 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 31A23C433F5 for ; Fri, 3 Sep 2021 04:05:30 +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 BA52C61058 for ; Fri, 3 Sep 2021 04:05:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org BA52C61058 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org 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=Y+3SBoP1Au2c9yklv7Cs00c7dyA2JNbkQGVqO3jmNgo=; b=nsIBFaILSEwuS8 1/0OzPFKWm50QoxyrMDLF9atHI4p1Y+kwREAQ3VHXq5Q6PPbnxFBh1OJICg29BN8EQj5ryYS9mh/L ocrRhCaVio/CKpiI0tqlIuG+4SJ35nlCIQNLwVaOxn5MAqQJkmxMNQluBOlnM8rWXkqDpKSqZS0IO X8VBRjj5uc3S1XyS2ng8zWWYy7GEHizGmlGtHzh483fHavFzGdMxuRp8V7oXTc+KiRFH3Lx3bQVG9 08QRDWjhT1Jck0vbqlk0jqfrmwTv0yuInIWP6INsIamz4TWvC6u9Xa69CUwU5SoVV7PWhflNK2VfL g5iFYYc+gk5uPGmOpMMA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mM0Pi-00AvC4-MT; Fri, 03 Sep 2021 04:02:50 +0000 Received: from mail-lj1-x234.google.com ([2a00:1450:4864:20::234]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mM0Pd-00Av9V-9k for linux-arm-kernel@lists.infradead.org; Fri, 03 Sep 2021 04:02:47 +0000 Received: by mail-lj1-x234.google.com with SMTP id p15so7549142ljn.3 for ; Thu, 02 Sep 2021 21:02:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hsqolKVO4nOJoFFGBqwd8L+i7kJ81PMtNw0t/34Bbxc=; b=a+IMKpSwOaBkBCR2jhE3eDJFkMxdZGfrTPA91s5BiAXKK1vbq/JCB59IkKv6KCUtql f9BVn1U4h9ANDqU8CgHzLZporyLsDXCdf9/2C09SchnyqGNjRUj1gU+lBRQT2AnAqztU 9rW80S9PDgEp8TtrnhFEFze2kWMWQAwV71cl4= 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=hsqolKVO4nOJoFFGBqwd8L+i7kJ81PMtNw0t/34Bbxc=; b=GyGozqHsazWHcrSL6vj4GwnJ6vX4xvc1jaddCrNQeK23Jo8h+Wp6Ouy+NzeWjmr/dL qu3UuB76LegdJoFsBhEkELcbZlMFBWIYvLahX4Yhn3f3tgKd/A+lpnAAkuzmPYlYDoUw 5KSiMYk7Agtcpjp76AUatx8YCHDyYO/nmOdkCKYBKa0PKZgZ/c9jz5mNnxis9nk0NfjK vSwFDXGzSNqCYbFFQk8CQ0BYtCivSXvtKRDAuFymGdFSyANCkPVRdcF8P+kY+joAp2cj InE0LGhKUJWqhPWVezUGRaVgOVzpogwF79WE4v/vOLK0zswMVB4XCYOQ4+uIIqH8DyTL Dw1g== X-Gm-Message-State: AOAM530+napxS7oO0yJCPSjxHLjcxKPcJ2Ft9+MiLp0/HDe9wp9mEs74 1qUulRA6RrAu7/zmAjQAGAM8AeQ5NQYxUrGvSmL2BQ== X-Google-Smtp-Source: ABdhPJxar47W9Hxbv7flNSitG5YspCZFWGxl4kNdEZYVIQGwUadVu8hYMZ5WnWzdLG9zy7rzurwuo3xhXN7yB8eeMJg= X-Received: by 2002:a2e:7d0e:: with SMTP id y14mr1247904ljc.251.1630641762596; Thu, 02 Sep 2021 21:02:42 -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> <9c3f871b3b58b8456e40c43fe402a7a4acb66660.camel@mediatek.com> In-Reply-To: <9c3f871b3b58b8456e40c43fe402a7a4acb66660.camel@mediatek.com> From: Chen-Yu Tsai Date: Fri, 3 Sep 2021 12:02:31 +0800 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: Ezequiel Garcia , 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-20210902_210245_417990_87D3161D X-CRM114-Status: GOOD ( 49.74 ) 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 Thu, Sep 2, 2021 at 2:09 PM yunfei.dong@mediatek.com wrote: > > 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 > > 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. > > > Thanks. > > > > 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. > > > You can see the block diagram in patch: > > https://patchwork.linuxtv.org/project/linux-media/patch/20210901083215.25984-14-yunfei.dong@mediatek.com/ 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel