diff mbox series

[01/10] dt-bindings: media: mediatek,vcodec: Remove dma-ranges property

Message ID 20230113060133.9394-2-yong.wu@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Adjust the dma-ranges for MTK IOMMU | expand

Commit Message

Yong Wu (吴勇) Jan. 13, 2023, 6:01 a.m. UTC
MediaTek iommu has already controlled the masters' iova ranges by the
master's larb/port id. then the dma-ranges property is unnecessary for
the master's node. the master is vcodec here.

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>
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(-)

Comments

Krzysztof Kozlowski Jan. 13, 2023, 8:25 a.m. UTC | #1
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.

> the master's node. the master is vcodec here.

Unnecessary or invalid? Don't you depend now on some feature of driver
added for example recently?

> 
> 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 ---.

> 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
Yong Wu (吴勇) Jan. 16, 2023, 8:01 a.m. UTC | #2
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
> 
>
Krzysztof Kozlowski Jan. 16, 2023, 8:06 a.m. UTC | #3
On 16/01/2023 09:01, Yong Wu (吴勇) wrote:
> 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.

So this is an ABI change where you expect no upstream users to be
affected? Why you do not clarify it in commit msg?

> 
>>>
>>> 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.

> 


Best regards,
Krzysztof
Yong Wu (吴勇) Jan. 16, 2023, 9:16 a.m. UTC | #4
On Mon, 2023-01-16 at 09:06 +0100, Krzysztof Kozlowski wrote:
> On 16/01/2023 09:01, Yong Wu (吴勇) wrote:
> > 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.
> 
> So this is an ABI change where you expect no upstream users to be
> affected? Why you do not clarify it in commit msg?

Sorry I missed a venc node. In [9/10] of this series, I deleted this
property for mt8195 venc node, this has a little affect. 

VENC would like to locate the IOVA range of 4G-8G. Without this
patchset, It will fallback to 0-4GB. But this is not a fatal issue. It 
also work fine with 0-4GB iova. I will comment this in the commit
message in the next version.

> > > > 
> > > > 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? or I should remove this, and put all of them in the cc list
of the mail.

Thanks.

> > 
> 
> Best regards,
> Krzysztof
>
AngeloGioacchino Del Regno Jan. 16, 2023, 9:47 a.m. UTC | #5
Il 13/01/23 07:01, Yong Wu ha scritto:
> MediaTek iommu has already controlled the masters' iova ranges by the
> master's larb/port id. then the dma-ranges property is unnecessary for
> the master's node. the master is vcodec here.
> 
> 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>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Krzysztof Kozlowski Jan. 17, 2023, 10:44 a.m. UTC | #6
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
diff mbox series

Patch

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>;