Re: [PATCH v7 3/5] dt-binding: mt8183: Add Mediatek MDP3 dt-bindings

[moudy ho]

On Wed, 2021-09-01 at 18:16 +0800, Chen-Yu Tsai wrote:
> On Wed, Sep 1, 2021 at 5:04 PM moudy ho <moudy.ho@xxxxxxxxxxxx>
> wrote:
> > 
> > On Mon, 2021-08-30 at 10:05 -0500, Rob Herring wrote:
> > > On Mon, Aug 30, 2021 at 2:58 AM moudy ho <moudy.ho@xxxxxxxxxxxx>
> > > wrote:
> > > > 
> > > > On Tue, 2021-08-24 at 13:02 -0500, Rob Herring wrote:
> > > > > On Tue, Aug 24, 2021 at 06:00:25PM +0800, Moudy Ho wrote:
> > > > > > This patch adds DT binding document for Media Data Path 3
> > > > > > (MDP3)
> > > > > > a unit in multimedia system used for scaling and color
> > > > > > format
> > > > > > convert.
> > > > > > 
> > > > > > Signed-off-by: Moudy Ho <moudy.ho@xxxxxxxxxxxx>
> > > > > > ---
> > > > > >  .../bindings/media/mediatek,mdp3-ccorr.yaml   |  57 +++++
> > > > > >  .../bindings/media/mediatek,mdp3-rdma.yaml    | 207
> > > > > > ++++++++++++++++++
> > > > > >  .../bindings/media/mediatek,mdp3-rsz.yaml     |  65 ++++++
> > > > > >  .../bindings/media/mediatek,mdp3-wdma.yaml    |  71 ++++++
> > > > > >  .../bindings/media/mediatek,mdp3-wrot.yaml    |  71 ++++++
> > > > > >  5 files changed, 471 insertions(+)
> > > > > >  create mode 100644
> > > > > > Documentation/devicetree/bindings/media/mediatek,mdp3-
> > > > > > ccorr.yaml
> > > > > >  create mode 100644
> > > > > > Documentation/devicetree/bindings/media/mediatek,mdp3-
> > > > > > rdma.yaml
> > > > > >  create mode 100644
> > > > > > Documentation/devicetree/bindings/media/mediatek,mdp3-
> > > > > > rsz.yaml
> > > > > >  create mode 100644
> > > > > > Documentation/devicetree/bindings/media/mediatek,mdp3-
> > > > > > wdma.yaml
> > > > > >  create mode 100644
> > > > > > Documentation/devicetree/bindings/media/mediatek,mdp3-
> > > > > > wrot.yaml
> > > > > > 
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/media/mediatek,mdp3-
> > > > > > ccorr.yaml
> > > > > > b/Documentation/devicetree/bindings/media/mediatek,mdp3-
> > > > > > ccorr.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..59fd68b46022
> > > > > > --- /dev/null
> > > > > > +++
> > > > > > b/Documentation/devicetree/bindings/media/mediatek,mdp3-
> > > > > > ccorr.yaml
> > > > > > @@ -0,0 +1,57 @@
> > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id:
> > > > > > 
> > 
> > 
https://urldefense.com/v3/__http://devicetree.org/schemas/media/mediatek,mdp3-ccorr.yaml*__;Iw!!CTRNKA9wMg0ARbw!1C0ChLqzi7Zq8D2d4_S4IqCEei4GXdgy3_VCQg8MdsJP7n8TlxbGyajipusfH8hi$
> > > > > > 
> > > > > > +$schema:
> > > > > > 
> > 
> > 
https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!1C0ChLqzi7Zq8D2d4_S4IqCEei4GXdgy3_VCQg8MdsJP7n8TlxbGyajipi-OInix$
> > > > > > 
> > > > > > +
> > > > > > +title: Mediatek Media Data Path 3 CCORR Device Tree
> > > > > > Bindings
> > > > > > +
> > > > > > +maintainers:
> > > > > > +  - Daoyuan Huang <daoyuan.huang@xxxxxxxxxxxx>
> > > > > > +  - Moudy Ho <moudy.ho@xxxxxxxxxxxx>
> > > > > > +
> > > > > > +description: |
> > > > > > +  One of Media Data Path 3 (MDP3) components used to do
> > > > > > color
> > > > > > correction with 3X3 matrix.
> > > > > > +
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +    items:
> > > > > > +      - enum:
> > > > > > +        - mediatek,mt8183-mdp3-ccorr
> > > > > > +
> > > > > > +  mediatek,mdp3-id:
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > +    maxItems: 1
> > > > > > +    description: |
> > > > > > +      HW index to distinguish same functionality modules.
> > > > > 
> > > > > If we wanted h/w indexes in DT, we'd have a standard
> > > > > property.
> > > > > Why
> > > > > do
> > > > > you need this?
> > > > > 
> > > > 
> > > > I'm sorry not quite sure what HW indexes means (something like
> > > > aliases?)
> > > 
> > > It means whatever you said in your description.
> > > 
> > > And no, I'm not suggesting you use aliases.
> > 
> > Sorry for the inaccuracy described here, the comment i mentioned
> > before
> > should be "standard property" instead of "HW index".
> > 
> > > > It was originally used to mark multiple identical modules in
> > > > the
> > > > MDP
> > > > data path algorithm, so that appropriate paths can be
> > > > dynamically
> > > > dispatched.
> > > 
> > > If they are identical, then why do you need to distinguish them
> > > in
> > > DT?
> > > If there's some difference you need to know about such as
> > > connections
> > > to other blocks, then describe that. Another common example is
> > > needing
> > > to know what bits/registers to access in a syscon phandle. For
> > > that,
> > > make the register offset or bits be args to the phandle property.
> > > 
> > > Rob
> > 
> > Integrating the previous discussion, maybe I can revise the
> > description
> > to the following:
> >     description: |
> >       There may be multiple blocks with the same function but
> > different
> >       addresses in MDP3. In order to distinguish the connection
> > with
> >       other blocks, a unique ID is needed to dynamically use one or
> >       more identical blocks to implement multiple pipelines.
> 
> With display pipelines it is common to describe the pipeline with an
> OF
> graph. With the pipeline drawn out, you also get ways to derive
> identifiers
> for otherwise identical blocks, such as from port IDs.
> 
> See Documentation/devicetree/bindings/display/allwinner,sun4i-a10-
> display-engine.yaml
> and arch/arm/boot/dts/sun9i-a80.dtsi for such an example.
> 
> 
> ChenYu

From the MDP routing table defined in mt8183-mmsys.h
([v7,2/5] soc: mediatek: mmsys: Add support for MDP),
It can be seen that each component has its own independent
one-to-many mapping table, and its next-level receiver may
have its own multiple inputs and is difficult to briefly
describe these relationships.
 
In addition, due to hardware limitations, RSZ0 (for example)
cannot be completely regarded as RSZ1 in mmsys, and must be
regarded as an independent entity, an example mentioned
in line #67 from mtk-mdp3-cmdq.c
([v7,5/5] media: platform: mtk-mdp3: Add Mediatek MDP3 driver).

Moudy