From: Irui Wang <irui.wang@mediatek.com>
Add MT8196 encoder compatible string, which will reference VCP device.
Signed-off-by: Irui Wang <irui.wang@mediatek.com>
---
.../bindings/media/mediatek,vcodec-encoder.yaml | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml
index ebc615584f92..bb4dbf23ccc5 100644
--- a/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml
@@ -24,6 +24,7 @@ properties:
- mediatek,mt8188-vcodec-enc
- mediatek,mt8192-vcodec-enc
- mediatek,mt8195-vcodec-enc
+ - mediatek,mt8196-vcodec-enc
- items:
- const: mediatek,mt8186-vcodec-enc
- const: mediatek,mt8183-vcodec-enc
@@ -58,6 +59,11 @@ properties:
description:
Describes point to scp.
+ mediatek,vcp:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Describes point to vcp.
+
power-domains:
maxItems: 1
@@ -76,6 +82,17 @@ required:
- iommus
allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - mediatek,mt8196-vcodec-enc
+
+ then:
+ required:
+ - mediatek,vcp
+
- if:
properties:
compatible:
--
2.46.0On Thu, Aug 14, 2025 at 04:56:41PM +0800, Kyrie Wu wrote: > From: Irui Wang <irui.wang@mediatek.com> > > Add MT8196 encoder compatible string, which will reference VCP device. You ignored comments from v2. > > Signed-off-by: Irui Wang <irui.wang@mediatek.com> Incorrect SoB chain. > --- > .../bindings/media/mediatek,vcodec-encoder.yaml | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml > index ebc615584f92..bb4dbf23ccc5 100644 > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-encoder.yaml > @@ -24,6 +24,7 @@ properties: > - mediatek,mt8188-vcodec-enc > - mediatek,mt8192-vcodec-enc > - mediatek,mt8195-vcodec-enc > + - mediatek,mt8196-vcodec-enc > - items: > - const: mediatek,mt8186-vcodec-enc > - const: mediatek,mt8183-vcodec-enc > @@ -58,6 +59,11 @@ properties: > description: > Describes point to scp. > > + mediatek,vcp: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + Describes point to vcp. For what? You just repeated the property name. You must say here something useful instead: why this is needed, what is its purpose. > + > power-domains: > maxItems: 1 > > @@ -76,6 +82,17 @@ required: > - iommus > > allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - mediatek,mt8196-vcodec-enc > + > + then: > + required: > + - mediatek,vcp else mediatek,vcp: false Best regards, Krzysztof
Dear Krzysztof, Thanks for your reviewing. On Fri, 2025-08-15 at 10:54 +0200, Krzysztof Kozlowski wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > On Thu, Aug 14, 2025 at 04:56:41PM +0800, Kyrie Wu wrote: > > From: Irui Wang <irui.wang@mediatek.com> > > > > Add MT8196 encoder compatible string, which will reference VCP > > device. > > You ignored comments from v2. I think I misunderstood the v2 comments, I rewrote the title because it said dt-bindings and encoder twice, this is not enough, right? we need to describe more information in Body text? > > > > > Signed-off-by: Irui Wang <irui.wang@mediatek.com> > > Incorrect SoB chain. > > > --- > > .../bindings/media/mediatek,vcodec-encoder.yaml | 17 > > +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/media/mediatek,vcodec- > > encoder.yaml > > b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > encoder.yaml > > index ebc615584f92..bb4dbf23ccc5 100644 > > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec- > > encoder.yaml > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > encoder.yaml > > @@ -24,6 +24,7 @@ properties: > > - mediatek,mt8188-vcodec-enc > > - mediatek,mt8192-vcodec-enc > > - mediatek,mt8195-vcodec-enc > > + - mediatek,mt8196-vcodec-enc > > - items: > > - const: mediatek,mt8186-vcodec-enc > > - const: mediatek,mt8183-vcodec-enc > > @@ -58,6 +59,11 @@ properties: > > description: > > Describes point to scp. > > > > + mediatek,vcp: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: > > + Describes point to vcp. > > For what? You just repeated the property name. You must say here > something useful instead: why this is needed, what is its purpose. I would like to say: "define this 'mediatek,vcp' property here, this is a phandle point to vcp device, for platforms using vcp firmware." Is this OK for you? > > > > + > > power-domains: > > maxItems: 1 > > > > @@ -76,6 +82,17 @@ required: > > - iommus > > > > allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - mediatek,mt8196-vcodec-enc > > + > > + then: > > + required: > > + - mediatek,vcp > > else > > mediatek,vcp: false I think the else statement is no need here. Different platforms are using different firmware phandle, vpu, scp, and vcp. so we use if-then to describe the required property for these platforms. > > > Best regards, > Krzysztof > Thanks again for your kindly reviewing. Best Regards
On 20/08/2025 04:55, Irui Wang (王瑞) wrote: > Dear Krzysztof, > > Thanks for your reviewing. > > On Fri, 2025-08-15 at 10:54 +0200, Krzysztof Kozlowski wrote: >> External email : Please do not click links or open attachments until >> you have verified the sender or the content. >> >> >> On Thu, Aug 14, 2025 at 04:56:41PM +0800, Kyrie Wu wrote: >>> From: Irui Wang <irui.wang@mediatek.com> >>> >>> Add MT8196 encoder compatible string, which will reference VCP >>> device. >> >> You ignored comments from v2. > > I think I misunderstood the v2 comments, I rewrote the title because it > said dt-bindings and encoder twice, this is not enough, right? we need > to describe more information in Body text? where are lore links in the changelog? Why aren't you using b4 to submit patches? > >> >>> >>> Signed-off-by: Irui Wang <irui.wang@mediatek.com> >> >> Incorrect SoB chain. >> >>> --- >>> .../bindings/media/mediatek,vcodec-encoder.yaml | 17 >>> +++++++++++++++++ >>> 1 file changed, 17 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/media/mediatek,vcodec- >>> encoder.yaml >>> b/Documentation/devicetree/bindings/media/mediatek,vcodec- >>> encoder.yaml >>> index ebc615584f92..bb4dbf23ccc5 100644 >>> --- a/Documentation/devicetree/bindings/media/mediatek,vcodec- >>> encoder.yaml >>> +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec- >>> encoder.yaml >>> @@ -24,6 +24,7 @@ properties: >>> - mediatek,mt8188-vcodec-enc >>> - mediatek,mt8192-vcodec-enc >>> - mediatek,mt8195-vcodec-enc >>> + - mediatek,mt8196-vcodec-enc >>> - items: >>> - const: mediatek,mt8186-vcodec-enc >>> - const: mediatek,mt8183-vcodec-enc >>> @@ -58,6 +59,11 @@ properties: >>> description: >>> Describes point to scp. >>> >>> + mediatek,vcp: >>> + $ref: /schemas/types.yaml#/definitions/phandle >>> + description: >>> + Describes point to vcp. >> >> For what? You just repeated the property name. You must say here >> something useful instead: why this is needed, what is its purpose. > > I would like to say: > "define this 'mediatek,vcp' property here, this is a phandle point to Don't explain to us what DT is. Above is 100% redundant. > vcp device, for platforms using vcp firmware." Explain what is the purpose of this in hardware, how hardware uses it, what problem for hardware this addresses. > > Is this OK for you? >> >> > >>> + >>> power-domains: >>> maxItems: 1 >>> >>> @@ -76,6 +82,17 @@ required: >>> - iommus >>> >>> allOf: >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - mediatek,mt8196-vcodec-enc >>> + >>> + then: >>> + required: >>> + - mediatek,vcp >> >> else >> >> mediatek,vcp: false > > I think the else statement is no need here. Different platforms are > using different firmware phandle, vpu, scp, and vcp. so we use if-then > to describe the required property for these platforms. Hm? I just told you it is needed. Otherwise, explain why each variant/device has now VCP. You have entire commit msg to explain all this. Best regards, Krzysztof
Dear Krzysztof, Thanks for your reviewing. On Wed, 2025-08-20 at 07:55 +0200, Krzysztof Kozlowski wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > On 20/08/2025 04:55, Irui Wang (王瑞) wrote: > > Dear Krzysztof, > > > > Thanks for your reviewing. > > > > On Fri, 2025-08-15 at 10:54 +0200, Krzysztof Kozlowski wrote: > > > External email : Please do not click links or open attachments > > > until > > > you have verified the sender or the content. > > > > > > > > > On Thu, Aug 14, 2025 at 04:56:41PM +0800, Kyrie Wu wrote: > > > > From: Irui Wang <irui.wang@mediatek.com> > > > > > > > > Add MT8196 encoder compatible string, which will reference VCP > > > > device. > > > > > > You ignored comments from v2. > > > > I think I misunderstood the v2 comments, I rewrote the title > > because it > > said dt-bindings and encoder twice, this is not enough, right? we > > need > > to describe more information in Body text? > > where are lore links in the changelog? We need put the lore links in the change log, I mean put the previous version(v2) patchset lore link? We should to indicate the changes in each version in the change log and we did the same. If each version lore links or URL is needed, we will start add them in next patch. > Why aren't you using b4 to submit > patches? We have been using 'git send-email' to submit patches to community, b4 is not deployed ready on my server yet, we can study it first. > > > > > > > > > > > > > Signed-off-by: Irui Wang <irui.wang@mediatek.com> > > > > > > Incorrect SoB chain. > > > > > > > --- > > > > .../bindings/media/mediatek,vcodec-encoder.yaml | 17 > > > > +++++++++++++++++ > > > > 1 file changed, 17 insertions(+) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/media/mediatek,vcodec- > > > > encoder.yaml > > > > b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > > > encoder.yaml > > > > index ebc615584f92..bb4dbf23ccc5 100644 > > > > --- a/Documentation/devicetree/bindings/media/mediatek,vcodec- > > > > encoder.yaml > > > > +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec- > > > > encoder.yaml > > > > @@ -24,6 +24,7 @@ properties: > > > > - mediatek,mt8188-vcodec-enc > > > > - mediatek,mt8192-vcodec-enc > > > > - mediatek,mt8195-vcodec-enc > > > > + - mediatek,mt8196-vcodec-enc > > > > - items: > > > > - const: mediatek,mt8186-vcodec-enc > > > > - const: mediatek,mt8183-vcodec-enc > > > > @@ -58,6 +59,11 @@ properties: > > > > description: > > > > Describes point to scp. > > > > > > > > + mediatek,vcp: > > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > > + description: > > > > + Describes point to vcp. > > > > > > For what? You just repeated the property name. You must say here > > > something useful instead: why this is needed, what is its > > > purpose. > > > > I would like to say: > > "define this 'mediatek,vcp' property here, this is a phandle point > > to > > Don't explain to us what DT is. Above is 100% redundant. > > > vcp device, for platforms using vcp firmware." > > Explain what is the purpose of this in hardware, how hardware uses > it, > what problem for hardware this addresses. Maybe I would like to say: phandle to the VCP node, the video encoder kernel driver uses this phandle to coordinate with the encoder software driver which running in VCP firmware for command submission and interrupt handling during encoding process. > > > > > > Is this OK for you? > > > > > > > > > > + > > > > power-domains: > > > > maxItems: 1 > > > > > > > > @@ -76,6 +82,17 @@ required: > > > > - iommus > > > > > > > > allOf: > > > > + - if: > > > > + properties: > > > > + compatible: > > > > + contains: > > > > + enum: > > > > + - mediatek,mt8196-vcodec-enc > > > > + > > > > + then: > > > > + required: > > > > + - mediatek,vcp > > > > > > else > > > > > > mediatek,vcp: false > > > > I think the else statement is no need here. Different platforms are > > using different firmware phandle, vpu, scp, and vcp. so we use if- > > then > > to describe the required property for these platforms. > > Hm? I just told you it is needed. Otherwise, explain why each > variant/device has now VCP. Currently, just mt8196 use VCP devices, so 'mediatek,vcp' property is required on mt8196 platform, sorry I didn't get your opinion why 'each variant/device has now VCP.' > > You have entire commit msg to explain all this. > > > Best regards, > Krzysztof Thanks again for your kindly reviewing. Best regards
© 2016 - 2025 Red Hat, Inc.