| Message ID | 20230113060133.9394-1-yong.wu@mediatek.com |
|---|---|
| Headers | show |
| Series | Adjust the dma-ranges for MTK IOMMU | expand |
On Fri, 2023-01-13 at 09:25 +0100, Krzysztof Kozlowski wrote: > On 13/01/2023 07:01, Yong Wu wrote: > > MediaTek iommu has already controlled the masters' iova ranges by > > the > > master's larb/port id. then the dma-ranges property is unnecessary > > for > Sentences in English always start with a capital letter, however also > they do not start with "Then". Make it a proper a proper sentence. Sorry for the syntax issues. I think it is "," before "then". > > the master's node. the master is vcodec here. > > Unnecessary or invalid? For mt8195, It is unnecessary. For the other SoC which doesn't use parent/child node, the property is invalid, however, there is no vcodec node have this property in this case in the current upstream dts nodes. > Don't you depend now on some feature of driver > added for example recently? No. It doesn't depend on any the other patches. Just depend on the code changing in this patchset. I just put the dt-binding at the beginning of this series. > > > > Cc: Tiffany Lin <tiffany.lin@mediatek.com> > > Cc: Andrew-CT Chen <andrew-ct.chen@mediatek.com> > > Cc: Yunfei Dong <yunfei.dong@mediatek.com> > > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > > There is little point in storing output of get_maintainers.pl forever > in > the git log. If you need it for some reason, please keep it after - > --. I did get the list from get_maintainers.pl. Sorry that I didn't differentiate. Mainly I changed the iommu code but changed the media dt-binding. Just expect the media owners/reviewers to take a look at this. Thanks. > > Cc: Matthias Brugger <matthias.bgg@gmail.com> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > --- > > .../devicetree/bindings/media/mediatek,vcodec-decoder.yaml | 5 - > > ---- > > .../devicetree/bindings/media/mediatek,vcodec-encoder.yaml | 5 - > > ---- > > .../bindings/media/mediatek,vcodec-subdev-decoder.yaml | 7 - > > ------ > > 3 files changed, 17 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/media/mediatek,vcodec- > > decoder.yaml > > b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > decoder.yaml > > index aa55ca65d6ed..fad59b486d5d 100644 > > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec- > > decoder.yaml > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > decoder.yaml > > @@ -56,11 +56,6 @@ properties: > > 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,vpu: > > $ref: /schemas/types.yaml#/definitions/phandle > > description: > > diff --git > > a/Documentation/devicetree/bindings/media/mediatek,vcodec- > > encoder.yaml > > b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > encoder.yaml > > index 0f2ea8d9a10c..a2051b31fa29 100644 > > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec- > > encoder.yaml > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > encoder.yaml > > @@ -49,11 +49,6 @@ properties: > > 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,vpu: > > $ref: /schemas/types.yaml#/definitions/phandle > > description: > > diff --git > > a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev- > > decoder.yaml > > b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev- > > decoder.yaml > > index c4f20acdc1f8..290594bc91cc 100644 > > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec- > > subdev-decoder.yaml > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > subdev-decoder.yaml > > @@ -76,11 +76,6 @@ properties: > > The node of system control processor (SCP), using > > the remoteproc & rpmsg framework. > > > > - dma-ranges: > > - maxItems: 1 > > - description: | > > - Describes the physical address space of IOMMU maps to > > memory. > > - > > "#address-cells": > > const: 2 > > > > @@ -203,7 +198,6 @@ required: > > - reg > > - iommus > > - mediatek,scp > > - - dma-ranges > > - ranges > > > > if: > > @@ -236,7 +230,6 @@ examples: > > compatible = "mediatek,mt8192-vcodec-dec"; > > mediatek,scp = <&scp>; > > iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>; > > - dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>; > > #address-cells = <2>; > > #size-cells = <2>; > > ranges = <0 0 0 0x16000000 0 0x40000>; > > Best regards, > Krzysztof > >
Il 13/01/23 07:01, Yong Wu ha scritto: > Currenly masters can not indicate its special dma-ranges. Prepare > for vcodec. some vcodec end address is address + size, if our size > is 4G, the end address may be 0x2_0000_0000. and the > register is u32, then it may get zero. thus add a gap(8M) for > all the regions. > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> I definitely agree on the fact that we do *need* this series... but this particular commit looks like a hack. I'm not convinced: I have a hunch that this one will sooner or later backfire on us and break things again... at the same time, I'm not sure how to do this properly at this point (I didn't do any research, anyway). Ideas? Regards, Angelo
On Mon, 2023-01-16 at 10:46 +0100, AngeloGioacchino Del Regno wrote: > Il 13/01/23 07:01, Yong Wu ha scritto: > > Currenly masters can not indicate its special dma-ranges. Prepare > > for vcodec. some vcodec end address is address + size, if our size > > is 4G, the end address may be 0x2_0000_0000. and the > > register is u32, then it may get zero. thus add a gap(8M) for > > all the regions. > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > I definitely agree on the fact that we do *need* this series... Thanks very much for your review. > but this particular commit looks like a hack. > > I'm not convinced: I have a hunch that this one will sooner or later > backfire > on us and break things again... at the same time, I'm not sure how to > do this > properly at this point (I didn't do any research, anyway). I got a real vcodec issue described in the commit message. As you may see in the vcodec's dt-binding example[1/10] or the dts node[9/10], their length is 0xfff00000 that means they use 1M as the gap. Vcodec use this for a long time. After this patchset, this property is unused, then I have to take care of this in the iommu, therefore this patch is required, and I just give a bigger gap(8M) here. > > Ideas? > > Regards, > Angelo >
On 16/01/2023 10:16, Yong Wu (吴勇) wrote: >>>>> >>>>> Cc: Tiffany Lin <tiffany.lin@mediatek.com> >>>>> Cc: Andrew-CT Chen <andrew-ct.chen@mediatek.com> >>>>> Cc: Yunfei Dong <yunfei.dong@mediatek.com> >>>>> Cc: Mauro Carvalho Chehab <mchehab@kernel.org> >>>>> Cc: Rob Herring <robh+dt@kernel.org> >>>>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> >>>> >>>> There is little point in storing output of get_maintainers.pl >>>> forever >>>> in >>>> the git log. If you need it for some reason, please keep it after >>>> - >>>> --. >>> >>> I did get the list from get_maintainers.pl. Sorry that I didn't >>> differentiate. >> >> Getting the list from get_maintainers.pl is correct but storing it >> forever in git log is really unnecessary. It's not useful. It's just >> automated output, reproducible at any given time. > > This patchset crosses several domains. This patch is about vcodec, the > next one is about jpeg and the later ones are about iommu. > The reviewers may be different, thus I use "Cc:" here. is this OK in > this case? I guess we do not talk about the same thing. It does not matter that reviewers are different. They are all different. Please show me the direct benefit of storing automated output from a tool in Git log. > or I should remove this, and put all of them in the cc list > of the mail. I gave you the instruction at beginning, some mails ago... Best regards, Krzysztof
After commit f1ad5338a4d5 ("of: Fix "dma-ranges" handling for bus controllers"), the dma-ranges is not allowed for dts leaf node. but we still would like to separate the different masters into different iova regions. Thus we adjust the internal flow, separate the 16GB iova range by the master HW larbid/portid and add the dma-ranges property in the parent "soc" node. This also could avoid the users forget/abuse the iova regions. The commit f1ad5338a4d5 doesn't introduce the regression for us in upstream, currently mt8195 vcodec/jpeg use the parent/child node. thus I don't add "Fixes:" tag. In this series, I add functions for mt8192/mt8195/mt8186, mt8188 will be in its special patchset. and the previous mt8173/mt8183...support 0-4GB only, no need this function. Base on v6.2-rc3. Yong Wu (10): dt-bindings: media: mediatek,vcodec: Remove dma-ranges property dt-bindings: media: mediatek,jpeg: Remove dma-ranges property iommu/mediatek: Get regionid from larb/port id iommu/mediatek: mt8195: Add larb_region_msk iommu/mediatek: mt8186: add larb_region_msk iommu/mediatek: mt8192: add larb_region_msk iommu/mediatek: Add a gap for the iova regions arm64: dts: mt8195: Add dma-ranges for the parent "soc" node arm64: dts: mt8195: Remove the unnecessary dma-ranges arm64: dts: mt8186: Add dma-ranges for the parent "soc" node .../media/mediatek,mt8195-jpegdec.yaml | 7 -- .../media/mediatek,mt8195-jpegenc.yaml | 7 -- .../media/mediatek,vcodec-decoder.yaml | 5 - .../media/mediatek,vcodec-encoder.yaml | 5 - .../media/mediatek,vcodec-subdev-decoder.yaml | 7 -- .../bindings/media/mediatek-jpeg-encoder.yaml | 5 - arch/arm64/boot/dts/mediatek/mt8186.dtsi | 1 + arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +- drivers/iommu/mtk_iommu.c | 95 ++++++++++++++----- 9 files changed, 72 insertions(+), 62 deletions(-)