mbox series

[v7,00/16] introduce more MDP3 components in MT8195

Message ID 20231012084037.19376-1-moudy.ho@mediatek.com
Headers show
Series introduce more MDP3 components in MT8195 | expand

Message

Moudy Ho Oct. 12, 2023, 8:40 a.m. UTC
Changes since v6:
- Rebase on v6.6-rc5.
- Dependent dtsi files:
  https://patchwork.kernel.org/project/linux-mediatek/list/?series=792079
- Depends on:
  Message ID = 20231006073831.10402-5-shawn.sung@mediatek.com
- Discard splitting RDMA's common properties and instead use 'allOf' to
  isolate different platform features.
- Revise the incorrect properties in FG, HDR, STITCH, TCC and TDAP bindings.
- Adding SoC-specific compatible string to components, like WROT and RSZ,
  that are inherited from MT8183.
- Fixed typos in TCC patch and enhancing its hardware description.

Changes since v5:
- Rebase on v6.6-rc2.
- Dependent dtsi files:
  https://patchwork.kernel.org/project/linux-mediatek/list/?series=786511
- Depends on:
  Message ID = 20230911074233.31556-5-shawn.sung@mediatek.com
- Split out common propertis for RDMA.
- Split each component into independent patches.

Changes since v4:
- Rebase on v6.6-rc1
- Organize identical hardware components into their respective files.

Hi,

The purpose of this patch is to separate the MDP3-related bindings from
the original mailing list mentioned below:
https://lore.kernel.org/all/20230208092209.19472-1-moudy.ho@mediatek.com/
Those binding files describe additional components that
are present in the mt8195.

Moudy Ho (16):
  dt-bindings: media: mediatek: mdp3: correct RDMA and WROT node with
    generic names
  dt-bindings: media: mediatek: mdp3: merge the indentical RDMA under
    display
  dt-bindings: media: mediatek: mdp3: add config for MT8195 RDMA
  dt-bindings: media: mediatek: mdp3: add compatible for MT8195 RSZ
  dt-bindings: media: mediatek: mdp3: add compatible for MT8195 WROT
  dt-bindings: media: mediatek: mdp3: add component FG for MT8195
  dt-bindings: media: mediatek: mdp3: add component HDR for MT8195
  dt-bindings: media: mediatek: mdp3: add component STITCH for MT8195
  dt-bindings: media: mediatek: mdp3: add component TCC for MT8195
  dt-bindings: media: mediatek: mdp3: add component TDSHP for MT8195
  dt-bindings: display: mediatek: aal: add compatible for MT8195
  dt-bindings: display: mediatek: color: add compatible for MT8195
  dt-bindings: display: mediatek: merge: add compatible for MT8195
  dt-bindings: display: mediatek: ovl: add compatible for MT8195
  dt-bindings: display: mediatek: split: add compatible for MT8195
  dt-bindings: display: mediatek: padding: add compatible for MT8195

 .../display/mediatek/mediatek,aal.yaml        |   1 +
 .../display/mediatek/mediatek,color.yaml      |   1 +
 .../display/mediatek/mediatek,mdp-rdma.yaml   |  88 --------------
 .../display/mediatek/mediatek,merge.yaml      |   1 +
 .../display/mediatek/mediatek,ovl.yaml        |   1 +
 .../display/mediatek/mediatek,padding.yaml    |   4 +-
 .../display/mediatek/mediatek,split.yaml      |  27 +++++
 .../bindings/media/mediatek,mdp3-fg.yaml      |  61 ++++++++++
 .../bindings/media/mediatek,mdp3-hdr.yaml     |  60 ++++++++++
 .../bindings/media/mediatek,mdp3-rdma.yaml    | 108 ++++++++++++++----
 .../bindings/media/mediatek,mdp3-rsz.yaml     |   6 +-
 .../bindings/media/mediatek,mdp3-stitch.yaml  |  61 ++++++++++
 .../bindings/media/mediatek,mdp3-tcc.yaml     |  62 ++++++++++
 .../bindings/media/mediatek,mdp3-tdshp.yaml   |  61 ++++++++++
 .../bindings/media/mediatek,mdp3-wrot.yaml    |  29 +++--
 15 files changed, 449 insertions(+), 122 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-fg.yaml
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-hdr.yaml
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-stitch.yaml
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-tcc.yaml
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-tdshp.yaml

Comments

Moudy Ho Oct. 13, 2023, 6:05 a.m. UTC | #1
On Thu, 2023-10-12 at 14:35 +0200, AngeloGioacchino Del Regno wrote:
> Il 12/10/23 10:40, Moudy Ho ha scritto:
> > Add a compatible string for the PAD block in MediaTek MT8195 that
> > is controlled by MDP3.
> > 
> > Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> > ---
> >   .../bindings/display/mediatek/mediatek,padding.yaml           | 4
> > +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,paddi
> > ng.yaml
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,paddi
> > ng.yaml
> > index db24801ebc48..636b69133acc 100644
> > ---
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,paddi
> > ng.yaml
> > +++
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,paddi
> > ng.yaml
> > @@ -20,7 +20,9 @@ description:
> >   
> >   properties:
> >     compatible:
> > -    const: mediatek,mt8188-padding
> > +    enum:
> > +      - mediatek,mt8188-padding
> > +      - mediatek,mt8195-mdp3-pad
> 
> mediatek,mt8195-mdp3-padding please!
> 
> Thanks,
> Angelo
> 

Hi Angelo,

Thanks for the reminder. I'll correct it to have consistent naming and
make corresponding modifications in the DTSI.

Sincerely,
Moudy

> >   
> >     reg:
> >       maxItems: 1
> 
>
Krzysztof Kozlowski Oct. 13, 2023, 6:49 a.m. UTC | #2
On 12/10/2023 10:40, Moudy Ho wrote:
> Add the fundamental hardware configuration of component FG,
> which is controlled by MDP3 on MT8195.
> 
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 13, 2023, 6:50 a.m. UTC | #3
On 12/10/2023 10:40, Moudy Ho wrote:
> Add the fundamental hardware configuration of component TDSHP,
> which is controlled by MDP3 on MT8195.
> 
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 16, 2023, 4:21 p.m. UTC | #4
On 16/10/2023 10:01, AngeloGioacchino Del Regno wrote:
> Il 13/10/23 08:52, Krzysztof Kozlowski ha scritto:
>> On 12/10/2023 10:40, Moudy Ho wrote:
>>> Add compatible string and GCE property for MT8195 SPLIT, of
>>> which is operated by MDP3.
>>>
>>> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
>>
>>
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: mediatek,mt8195-mdp3-split
>>> +
>>> +    then:
>>> +      required:
>>> +        - mediatek,gce-client-reg
>>
>> else:
>>    mediatek,gce-client-reg: false
>>
>>
> 
> Technically, all of the display components do support GCE, using it is
> a matter of preference, so disallowing gce-client-reg on anything that
> is not mt8195-mdp3-split is *technically* wrong, as much as not having
> that from the beginning was also technically wrong... :-)
> 
> P.S.: The driver for the display split component doesn't use GCE yet,
> only mdp3 for now, but again, it's the driver - while the HW is actually
> capable of using that

Hm, fine with me then.

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 16, 2023, 4:21 p.m. UTC | #5
On 12/10/2023 10:40, Moudy Ho wrote:
> Add compatible string and GCE property for MT8195 SPLIT, of
> which is operated by MDP3.
> 
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---

After feedback from Angelo:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Moudy Ho Oct. 18, 2023, 3:06 a.m. UTC | #6
On Fri, 2023-10-13 at 08:46 +0200, Krzysztof Kozlowski wrote:
>  	 

Hi Krzysztof,

Thank you for assisting with the review.

> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 12/10/2023 10:40, Moudy Ho wrote:
> 
> >  
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: mediatek,mt8183-mdp3-rdma
> > +
> > +    then:
> > +      properties:
> > +        clocks:
> > +          items:
> > +            - description: RDMA clock
> > +            - description: RSZ clock (shared SRAM with RDMA)
> > +
> > +        mboxes:
> > +          items:
> > +            - description: used for 1st data pipe from RDMA
> > +            - description: used for 2nd data pipe from RDMA
> 
> interrupts:
>   false
> 

As Angelo provided additional clarification in [15/16], explaining that
certain conditions in [2/16] and [3/16] were intentionally omitted due
to the need to integrate the same IP with different operations.
Apologies for any inconvenience this has caused you.

> > +
> > +      required:
> > +        - mboxes
> > +        - mediatek,gce-events
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: mediatek,mt8195-vdo1-rdma
> > +
> > +    then:
> > +      properties:
> > +        clocks:
> > +          items:
> > +            - description: RDMA clock
> 
> mboxes: false
> mediatek,gce-events: false
> 
> I am not so sure it is actually "simpler" to merge these. They are
> quite
> different. You will end up with unmanageable allOf  with a lot of
> branches (which supposedly you want to remove).
> 
> 

Upon examining the minor hardware changes in MDP for MT8183 and MT8195
RDMA ([3/16]), it appears that branching cannot be avoided. However,
consolidating these changes has the additional advantage of addressing
Rob's concerns from v4. Perhaps we can consider the current changes as
a form of progress.

Sincerely,
Moudy

> > +
> >  additionalProperties: false
> >  
> >  examples:
> 
> Best regards,
> Krzysztof
>
AngeloGioacchino Del Regno Oct. 18, 2023, 9:29 a.m. UTC | #7
Il 18/10/23 05:06, Moudy Ho (何宗原) ha scritto:
> On Fri, 2023-10-13 at 08:46 +0200, Krzysztof Kozlowski wrote:
>>   	
> 
> Hi Krzysztof,
> 
> Thank you for assisting with the review.
> 
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>   On 12/10/2023 10:40, Moudy Ho wrote:
>>
>>>   
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: mediatek,mt8183-mdp3-rdma
>>> +
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          items:
>>> +            - description: RDMA clock
>>> +            - description: RSZ clock (shared SRAM with RDMA)
>>> +
>>> +        mboxes:
>>> +          items:
>>> +            - description: used for 1st data pipe from RDMA
>>> +            - description: used for 2nd data pipe from RDMA
>>
>> interrupts:
>>    false
>>
> 
> As Angelo provided additional clarification in [15/16], explaining that
> certain conditions in [2/16] and [3/16] were intentionally omitted due
> to the need to integrate the same IP with different operations.
> Apologies for any inconvenience this has caused you.
> 

MT8183's MDP3 RDMA interrupt property was omitted in the devicetree that we
have upstream because it was either unused in the driver, or MTK didn't want
to actually use it for reasons, but that SoC *definitely does* have a mdp_rdma0
IRQ and a mdp_rdma1 IRQ.

That's the same for MT8186 and MT8188... and it's probably the same for all
MediaTek SoCs, so interrupts shouldn't be disallowed in this binding.

>>> +
>>> +      required:
>>> +        - mboxes
>>> +        - mediatek,gce-events
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: mediatek,mt8195-vdo1-rdma
>>> +
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          items:
>>> +            - description: RDMA clock
>>
>> mboxes: false
>> mediatek,gce-events: false
>>
>> I am not so sure it is actually "simpler" to merge these. They are
>> quite
>> different. You will end up with unmanageable allOf  with a lot of
>> branches (which supposedly you want to remove).
>>

It's the same thing as "split"... All of the display and mdp/mdp3 components of
MediaTek SoC do support GCE mailboxes by HW, so it's not limited to "split", but
literally all of them.

Disallowing mboxes and/or mediatek,gce-events on *any* of those is actually wrong.

Cheers,
Angelo

>>
> 
> Upon examining the minor hardware changes in MDP for MT8183 and MT8195
> RDMA ([3/16]), it appears that branching cannot be avoided. However,
> consolidating these changes has the additional advantage of addressing
> Rob's concerns from v4. Perhaps we can consider the current changes as
> a form of progress.
> 
> Sincerely,
> Moudy
> 
>>> +
>>>   additionalProperties: false
>>>   
>>>   examples:
>>
>> Best regards,
>> Krzysztof
>>