Re: [PATCH v6 08/14] dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for mt8196

On Mon, 2026-08-10 at 08:31 +0200, Krzysztof Kozlowski wrote:
> On 10/08/2026 05:09, Kyrie Wu (吴晗) wrote:
> > On Mon, 2026-07-20 at 12:19 +0200, Krzysztof Kozlowski wrote:
> > > On 20/07/2026 08:26, Kyrie Wu (吴晗) wrote:
> > > > On Mon, 2026-07-20 at 07:49 +0200, Krzysztof Kozlowski wrote:
> > > > > On 20/07/2026 07:46, Krzysztof Kozlowski wrote:
> > > > > > On Mon, Jul 20, 2026 at 09:20:50AM +0800, Kyrie Wu wrote:
> > > > > > > Add the MT8196 compatible string to the MediaTek vcodec
> > > > > > > subdev
> > > > > > > decoder binding.
> > > > > > > 
> > > > > > > Compared to previous ICs, the MT8196 supports a 10-bit
> > > > > > > decoder
> > > > > > > and has a decoding capability of 4K@120fps. It also
> > > > > > > supports
> > > > > > > 36-bit DRAM IOVA address and Video Power Control to
> > > > > > > optimize
> > > > > > > bandwidth and voltage usage.
> > > > > > 
> > > > > > Please wrap commit message according to Linux coding style
> > > > > > /
> > > > > > submission
> > > > > > process (neither too early nor over the limit):
> > > > > > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst*L597__;Iw!!CTRNKA9wMg0ARbw!gDjbq0EZNkhLTzuJM_w5PopnDFej1mG4qnkRFgyoiU7Dvhm7tj3MeG5eZYNGA0L0G_61j-cmdIxORA$
> > > > > >  
> > > > > > 
> > > > > > > 
> > > > > > > Signed-off-by: Kyrie Wu <[email protected]>
> > > > > > > Acked-by: Nicolas Dufresne
> > > > > > > <[email protected]>
> > > > > > > ---
> > > > > > >  .../bindings/media/mediatek,vcodec-subdev-
> > > > > > > decoder.yaml          
> > > > > > > > 1 +
> > > > > > >  1 file changed, 1 insertion(+)
> > > > > > 
> > > > > > Acked-by: Krzysztof Kozlowski
> > > > > > <[email protected]>
> > > > > 
> > > > > Actually no. I checked previous comments and you received
> > > > > feedback
> > > > > which
> > > > > you basically ignored, so explain why this model has flexible
> > > > > number
> > > > > of
> > > > > clocks
> > > > > 
> > > > > Best regards,
> > > > > Krzysztof
> > > > 
> > > > Dear Krzysztof,
> > > > 
> > > > As mentioned in the previous version of the communication, the
> > > > information regarding clk was incorrect, and I have corrected
> > > > it in
> > > > this version. 
> > > > 
> > > > Thank you for your attention and reminder.
> > > 
> > > Read my message carefully instead of responding whatever
> > > necessary to
> > > make me go away.
> > > 
> > > NAK
> > > 
> > > Where did you explain that device has flexible number of clocks?
> > > 
> > > Best regards,
> > > Krzysztof
> > 
> > Hi Krzysztof,
> > 
> > You are right, my previous reply was not sufficient. Sorry about
> > that.
> > 
> > MT8196 has a fixed set of clock inputs: 9 clocks for the decoder
> > hardware
> > and 3 clocks for the VCP interface. By "flexible number of clocks",
> > I meant that clock control can be split between the kernel driver
> > and VCP firmware. This is a software implementation detail,
> > not a hardware or DT binding property. My wording was incorrect.
> > 
> > The binding should not imply that the number of clocks is flexible.
> > I have updated the commit message to clarify this.
> 
> SW detail so not relevant to the bindings? Then my NAK stays.
> 
> Drop my tag and fix the patch. If you do not intend to fix the patch
> - I
> really could not get from above whether you agree with previous
> feedback
> or not - then this should not be applied.
> 
> Best regards,
> Krzysztof

Hi Krzysztof,

I reviewed the binding and the MT8196 DTS again. My previous
explanation
was incorrect.

MT8196 has two decoder child nodes, one for LAT and one for CORE. Each
node has a fixed set of four clocks, with the following clock-names:

    "sel", "vdec", "lat", "top"
	
The clock count must therefore not be flexible. I plan to add an MT8196
specific schema constraint in the binding, equivalent to:

    - if:
        properties:
          compatible:
            contains:
              const: mediatek,mt8196-vcodec-dec
			  
	 then:
        patternProperties:
          '^video-codec@[0-9a-f]+$':
            properties:
              clocks:
                minItems: 4
                maxItems: 4
              clock-names:
                minItems: 4
                maxItems: 4
                items:
                  - const: sel
                  - const: vdec
                  - const: lat
                  - const: top

I will use the existing child-node pattern from the binding rather than
introducing a new one.

Would this be the appropriate way to describe the fixed clock
requirements for MT8196? If so, I will send a new revision with this
change and without your Acked-by tag.

Sorry for not properly addressing your previous feedback.

Best regards,
Kyrie