2023-09-22 15:51:07

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v6 12/16] dt-bindings: display: mediatek: color: add compatible for MT8195

On Fri, Sep 22, 2023 at 03:21:12PM +0800, Moudy Ho wrote:
> Add a compatible string for the COLOR block in MediaTek MT8195 that
> is controlled by MDP3.
>
> Signed-off-by: Moudy Ho <[email protected]>
> ---
> .../devicetree/bindings/display/mediatek/mediatek,color.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
> index f21e44092043..b886ca0d89ea 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
> @@ -26,6 +26,7 @@ properties:
> - mediatek,mt2701-disp-color
> - mediatek,mt8167-disp-color
> - mediatek,mt8173-disp-color
> + - mediatek,mt8195-mdp3-color

How come this one is a "mdp3" not a "disp"?

> - items:
> - enum:
> - mediatek,mt7623-disp-color
> --
> 2.18.0
>


Attachments:
(No filename) (1.03 kB)
signature.asc (235.00 B)
Download all attachments

2023-09-22 17:28:40

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v6 12/16] dt-bindings: display: mediatek: color: add compatible for MT8195

On Fri, Sep 22, 2023 at 04:49:14PM +0100, Conor Dooley wrote:
> On Fri, Sep 22, 2023 at 03:21:12PM +0800, Moudy Ho wrote:
> > Add a compatible string for the COLOR block in MediaTek MT8195 that
> > is controlled by MDP3.
> >
> > Signed-off-by: Moudy Ho <[email protected]>
> > ---
> > .../devicetree/bindings/display/mediatek/mediatek,color.yaml | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
> > index f21e44092043..b886ca0d89ea 100644
> > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
> > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
> > @@ -26,6 +26,7 @@ properties:
> > - mediatek,mt2701-disp-color
> > - mediatek,mt8167-disp-color
> > - mediatek,mt8173-disp-color
> > + - mediatek,mt8195-mdp3-color
>
> How come this one is a "mdp3" not a "disp"?

I don't know what mdp3 means & googling gives me no answers. What's the
"disp" one controlled by, since it isn't controlled by mdp3?

>
> > - items:
> > - enum:
> > - mediatek,mt7623-disp-color
> > --
> > 2.18.0
> >



Attachments:
(No filename) (1.29 kB)
signature.asc (235.00 B)
Download all attachments

2023-09-27 08:31:10

by Moudy Ho (何宗原)

[permalink] [raw]
Subject: Re: [PATCH v6 12/16] dt-bindings: display: mediatek: color: add compatible for MT8195

On Fri, 2023-09-22 at 16:51 +0100, Conor Dooley wrote:
> On Fri, Sep 22, 2023 at 04:49:14PM +0100, Conor Dooley wrote:
> > On Fri, Sep 22, 2023 at 03:21:12PM +0800, Moudy Ho wrote:
> > > Add a compatible string for the COLOR block in MediaTek MT8195
> > > that
> > > is controlled by MDP3.
> > >
> > > Signed-off-by: Moudy Ho <[email protected]>
> > > ---
> > > .../devicetree/bindings/display/mediatek/mediatek,color.yaml
> > > | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/mediatek/mediatek,col
> > > or.yaml
> > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,col
> > > or.yaml
> > > index f21e44092043..b886ca0d89ea 100644
> > > ---
> > > a/Documentation/devicetree/bindings/display/mediatek/mediatek,col
> > > or.yaml
> > > +++
> > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,col
> > > or.yaml
> > > @@ -26,6 +26,7 @@ properties:
> > > - mediatek,mt2701-disp-color
> > > - mediatek,mt8167-disp-color
> > > - mediatek,mt8173-disp-color
> > > + - mediatek,mt8195-mdp3-color
> >
> > How come this one is a "mdp3" not a "disp"?
>
> I don't know what mdp3 means & googling gives me no answers. What's
> the
> "disp" one controlled by, since it isn't controlled by mdp3?
>

Hi Conor,

Mediatek's Media Data Path ver.3 (MDP3) is associated with MMSYS and
acts as an independent driver that operates between VDEC and DISP.
By controlling multiple components, it carries out tasks like
converting color formats, resizing, and applying specific Picture
Quality (PQ) effects.
The driver can be found at "driver/media/platform/mediatek/mdp3".
Since the same hardware components are configured in both MDP3 and
DISP, considering previous discussions, I attemped to integrate into a
single binding, named after the controlling user.

Sincerely,
Moudy
> >
> > > - items:
> > > - enum:
> > > - mediatek,mt7623-disp-color
> > > --
> > > 2.18.0
> > >
>
>

2023-09-27 13:39:27

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v6 12/16] dt-bindings: display: mediatek: color: add compatible for MT8195

On Wed, Sep 27, 2023 at 07:19:28AM +0000, Moudy Ho (何宗原) wrote:
> On Fri, 2023-09-22 at 16:51 +0100, Conor Dooley wrote:
> > On Fri, Sep 22, 2023 at 04:49:14PM +0100, Conor Dooley wrote:
> > > On Fri, Sep 22, 2023 at 03:21:12PM +0800, Moudy Ho wrote:
> > > > Add a compatible string for the COLOR block in MediaTek MT8195
> > > > that
> > > > is controlled by MDP3.
> > > >
> > > > Signed-off-by: Moudy Ho <[email protected]>
> > > > ---
> > > > .../devicetree/bindings/display/mediatek/mediatek,color.yaml
> > > > | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/display/mediatek/mediatek,col
> > > > or.yaml
> > > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,col
> > > > or.yaml
> > > > index f21e44092043..b886ca0d89ea 100644
> > > > ---
> > > > a/Documentation/devicetree/bindings/display/mediatek/mediatek,col
> > > > or.yaml
> > > > +++
> > > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,col
> > > > or.yaml
> > > > @@ -26,6 +26,7 @@ properties:
> > > > - mediatek,mt2701-disp-color
> > > > - mediatek,mt8167-disp-color
> > > > - mediatek,mt8173-disp-color
> > > > + - mediatek,mt8195-mdp3-color
> > >
> > > How come this one is a "mdp3" not a "disp"?
> >
> > I don't know what mdp3 means & googling gives me no answers. What's
> > the
> > "disp" one controlled by, since it isn't controlled by mdp3?
> >
>
> Hi Conor,
>
> Mediatek's Media Data Path ver.3 (MDP3) is associated with MMSYS and
> acts as an independent driver that operates between VDEC and DISP.
> By controlling multiple components, it carries out tasks like
> converting color formats, resizing, and applying specific Picture
> Quality (PQ) effects.
> The driver can be found at "driver/media/platform/mediatek/mdp3".
> Since the same hardware components are configured in both MDP3 and
> DISP, considering previous discussions, I attemped to integrate into a
> single binding, named after the controlling user.

I'm still kinda struggling to understand this. Do you mean that the
hardware can be controlled by either of the disp and mdp3 drivers, and
a compatible containing "disp" would use one driver, and one containing
"mdp3" would use another?


Attachments:
(No filename) (2.30 kB)
signature.asc (235.00 B)
Download all attachments

2023-09-28 12:40:58

by Moudy Ho (何宗原)

[permalink] [raw]
Subject: Re: [PATCH v6 12/16] dt-bindings: display: mediatek: color: add compatible for MT8195

On Wed, 2023-09-27 at 10:47 +0100, Conor Dooley wrote:
> On Wed, Sep 27, 2023 at 07:19:28AM +0000, Moudy Ho (何宗原) wrote:
> > On Fri, 2023-09-22 at 16:51 +0100, Conor Dooley wrote:
> > > On Fri, Sep 22, 2023 at 04:49:14PM +0100, Conor Dooley wrote:
> > > > On Fri, Sep 22, 2023 at 03:21:12PM +0800, Moudy Ho wrote:
> > > > > Add a compatible string for the COLOR block in MediaTek
> > > > > MT8195
> > > > > that
> > > > > is controlled by MDP3.
> > > > >
> > > > > Signed-off-by: Moudy Ho <[email protected]>
> > > > > ---
> > > > > .../devicetree/bindings/display/mediatek/mediatek,color.yaml
> > > > >
> > > > > | 1 +
> > > > > 1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/display/mediatek/mediatek
> > > > > ,col
> > > > > or.yaml
> > > > > b/Documentation/devicetree/bindings/display/mediatek/mediatek
> > > > > ,col
> > > > > or.yaml
> > > > > index f21e44092043..b886ca0d89ea 100644
> > > > > ---
> > > > > a/Documentation/devicetree/bindings/display/mediatek/mediatek
> > > > > ,col
> > > > > or.yaml
> > > > > +++
> > > > > b/Documentation/devicetree/bindings/display/mediatek/mediatek
> > > > > ,col
> > > > > or.yaml
> > > > > @@ -26,6 +26,7 @@ properties:
> > > > > - mediatek,mt2701-disp-color
> > > > > - mediatek,mt8167-disp-color
> > > > > - mediatek,mt8173-disp-color
> > > > > + - mediatek,mt8195-mdp3-color
> > > >
> > > > How come this one is a "mdp3" not a "disp"?
> > >
> > > I don't know what mdp3 means & googling gives me no answers.
> > > What's
> > > the
> > > "disp" one controlled by, since it isn't controlled by mdp3?
> > >
> >
> > Hi Conor,
> >
> > Mediatek's Media Data Path ver.3 (MDP3) is associated with MMSYS
> > and
> > acts as an independent driver that operates between VDEC and DISP.
> > By controlling multiple components, it carries out tasks like
> > converting color formats, resizing, and applying specific Picture
> > Quality (PQ) effects.
> > The driver can be found at "driver/media/platform/mediatek/mdp3".
> > Since the same hardware components are configured in both MDP3 and
> > DISP, considering previous discussions, I attemped to integrate
> > into a
> > single binding, named after the controlling user.
>
> I'm still kinda struggling to understand this. Do you mean that the
> hardware can be controlled by either of the disp and mdp3 drivers,
> and
> a compatible containing "disp" would use one driver, and one
> containing
> "mdp3" would use another?
>

Hi Conor,

Sorry for any confusion caused by the software information. In the
video pipeline, after decoding, the data flows sequentially through two
subsystems: MDP and DISP. Each subsystems has multiple IPs, with some
serving the same functionality as COLOR mentioned here. However, these
IPs cannot be controlled by different subsystems. Therefore, I included
the name of the subsystem after SoC to identify the configuration's
location. Is this approach feasible?

Sincerely,
Moudy

2023-09-28 17:08:10

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v6 12/16] dt-bindings: display: mediatek: color: add compatible for MT8195

On Thu, Sep 28, 2023 at 02:52:23AM +0000, Moudy Ho (何宗原) wrote:
> On Wed, 2023-09-27 at 10:47 +0100, Conor Dooley wrote:
> > On Wed, Sep 27, 2023 at 07:19:28AM +0000, Moudy Ho (何宗原) wrote:
> > > On Fri, 2023-09-22 at 16:51 +0100, Conor Dooley wrote:
> > > > On Fri, Sep 22, 2023 at 04:49:14PM +0100, Conor Dooley wrote:
> > > > > On Fri, Sep 22, 2023 at 03:21:12PM +0800, Moudy Ho wrote:
> > > > > > Add a compatible string for the COLOR block in MediaTek
> > > > > > MT8195
> > > > > > that
> > > > > > is controlled by MDP3.
> > > > > >
> > > > > > Signed-off-by: Moudy Ho <[email protected]>
> > > > > > ---
> > > > > > .../devicetree/bindings/display/mediatek/mediatek,color.yaml
> > > > > >
> > > > > > | 1 +
> > > > > > 1 file changed, 1 insertion(+)
> > > > > >
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/display/mediatek/mediatek
> > > > > > ,col
> > > > > > or.yaml
> > > > > > b/Documentation/devicetree/bindings/display/mediatek/mediatek
> > > > > > ,col
> > > > > > or.yaml
> > > > > > index f21e44092043..b886ca0d89ea 100644
> > > > > > ---
> > > > > > a/Documentation/devicetree/bindings/display/mediatek/mediatek
> > > > > > ,col
> > > > > > or.yaml
> > > > > > +++
> > > > > > b/Documentation/devicetree/bindings/display/mediatek/mediatek
> > > > > > ,col
> > > > > > or.yaml
> > > > > > @@ -26,6 +26,7 @@ properties:
> > > > > > - mediatek,mt2701-disp-color
> > > > > > - mediatek,mt8167-disp-color
> > > > > > - mediatek,mt8173-disp-color
> > > > > > + - mediatek,mt8195-mdp3-color
> > > > >
> > > > > How come this one is a "mdp3" not a "disp"?
> > > >
> > > > I don't know what mdp3 means & googling gives me no answers.
> > > > What's
> > > > the
> > > > "disp" one controlled by, since it isn't controlled by mdp3?
> > > >

> > > Mediatek's Media Data Path ver.3 (MDP3) is associated with MMSYS
> > > and
> > > acts as an independent driver that operates between VDEC and DISP.
> > > By controlling multiple components, it carries out tasks like
> > > converting color formats, resizing, and applying specific Picture
> > > Quality (PQ) effects.
> > > The driver can be found at "driver/media/platform/mediatek/mdp3".
> > > Since the same hardware components are configured in both MDP3 and
> > > DISP, considering previous discussions, I attemped to integrate
> > > into a
> > > single binding, named after the controlling user.
> >
> > I'm still kinda struggling to understand this. Do you mean that the
> > hardware can be controlled by either of the disp and mdp3 drivers,
> > and
> > a compatible containing "disp" would use one driver, and one
> > containing
> > "mdp3" would use another?
> >

> Sorry for any confusion caused by the software information. In the
> video pipeline, after decoding, the data flows sequentially through two
> subsystems: MDP and DISP. Each subsystems has multiple IPs, with some
> serving the same functionality as COLOR mentioned here. However, these
> IPs cannot be controlled by different subsystems. Therefore, I included
> the name of the subsystem after SoC to identify the configuration's
> location. Is this approach feasible?

I'll have to leave things to the likes of Laurent to comment here I
think. I don't understand this hardware well enough to have a useful
opinion. It would seem like a different part of the datapath is a
different device that should be documented separately, but I don't know
enough to say for sure, sorry.


Attachments:
(No filename) (3.50 kB)
signature.asc (235.00 B)
Download all attachments
Subject: Re: [PATCH v6 12/16] dt-bindings: display: mediatek: color: add compatible for MT8195

Il 28/09/23 18:49, Conor Dooley ha scritto:
> On Thu, Sep 28, 2023 at 02:52:23AM +0000, Moudy Ho (何宗原) wrote:
>> On Wed, 2023-09-27 at 10:47 +0100, Conor Dooley wrote:
>>> On Wed, Sep 27, 2023 at 07:19:28AM +0000, Moudy Ho (何宗原) wrote:
>>>> On Fri, 2023-09-22 at 16:51 +0100, Conor Dooley wrote:
>>>>> On Fri, Sep 22, 2023 at 04:49:14PM +0100, Conor Dooley wrote:
>>>>>> On Fri, Sep 22, 2023 at 03:21:12PM +0800, Moudy Ho wrote:
>>>>>>> Add a compatible string for the COLOR block in MediaTek
>>>>>>> MT8195
>>>>>>> that
>>>>>>> is controlled by MDP3.
>>>>>>>
>>>>>>> Signed-off-by: Moudy Ho <[email protected]>
>>>>>>> ---
>>>>>>> .../devicetree/bindings/display/mediatek/mediatek,color.yaml
>>>>>>>
>>>>>>> | 1 +
>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/display/mediatek/mediatek
>>>>>>> ,col
>>>>>>> or.yaml
>>>>>>> b/Documentation/devicetree/bindings/display/mediatek/mediatek
>>>>>>> ,col
>>>>>>> or.yaml
>>>>>>> index f21e44092043..b886ca0d89ea 100644
>>>>>>> ---
>>>>>>> a/Documentation/devicetree/bindings/display/mediatek/mediatek
>>>>>>> ,col
>>>>>>> or.yaml
>>>>>>> +++
>>>>>>> b/Documentation/devicetree/bindings/display/mediatek/mediatek
>>>>>>> ,col
>>>>>>> or.yaml
>>>>>>> @@ -26,6 +26,7 @@ properties:
>>>>>>> - mediatek,mt2701-disp-color
>>>>>>> - mediatek,mt8167-disp-color
>>>>>>> - mediatek,mt8173-disp-color
>>>>>>> + - mediatek,mt8195-mdp3-color
>>>>>>
>>>>>> How come this one is a "mdp3" not a "disp"?
>>>>>
>>>>> I don't know what mdp3 means & googling gives me no answers.
>>>>> What's
>>>>> the
>>>>> "disp" one controlled by, since it isn't controlled by mdp3?
>>>>>
>
>>>> Mediatek's Media Data Path ver.3 (MDP3) is associated with MMSYS
>>>> and
>>>> acts as an independent driver that operates between VDEC and DISP.
>>>> By controlling multiple components, it carries out tasks like
>>>> converting color formats, resizing, and applying specific Picture
>>>> Quality (PQ) effects.
>>>> The driver can be found at "driver/media/platform/mediatek/mdp3".
>>>> Since the same hardware components are configured in both MDP3 and
>>>> DISP, considering previous discussions, I attemped to integrate
>>>> into a
>>>> single binding, named after the controlling user.
>>>
>>> I'm still kinda struggling to understand this. Do you mean that the
>>> hardware can be controlled by either of the disp and mdp3 drivers,
>>> and
>>> a compatible containing "disp" would use one driver, and one
>>> containing
>>> "mdp3" would use another?
>>>
>
>> Sorry for any confusion caused by the software information. In the
>> video pipeline, after decoding, the data flows sequentially through two
>> subsystems: MDP and DISP. Each subsystems has multiple IPs, with some
>> serving the same functionality as COLOR mentioned here. However, these
>> IPs cannot be controlled by different subsystems. Therefore, I included
>> the name of the subsystem after SoC to identify the configuration's
>> location. Is this approach feasible?
>
> I'll have to leave things to the likes of Laurent to comment here I
> think. I don't understand this hardware well enough to have a useful
> opinion. It would seem like a different part of the datapath is a
> different device that should be documented separately, but I don't know
> enough to say for sure, sorry.

Hardware speaking, it's not a different device: those all reside in the
same block, except they are configured to route their I/O *either* to the
display pipeline, *or* to the MDP3 pipeline.

I would agree though in that this could be more flexible, as in, not
having a requirement to say "mdp3" or "disp", and managing the COLOR
blocks generically and letting the drivers to choose the actual path
transparently from what the devicetree compatible is, but there's no
practical point in doing this in the end, because there is an enough
number of (for example, COLOR) blocks such that one can be completely
reserved to MDP3 and one completely reserved to DISP.

So, we don't *need* this flexibility, but would be nice to have for
different (unexistant, basically) usecases...

The thing is, if we go for the maximum flexibility, the drawback is
that we'd see a number of nodes like

shared_block: something@somewhere {
compatible = "mediatek,something";
}

mdp3: dma-controller@14001000 {
......
mediatek,color = <&color0>;
mediatek,stitch = <&stitch0>;
mediatek,hdr = <&hdr0>;
mediatek,aal = <&aal0>;
....
long list of another 10 components
}

display: something@somewhere {
......
an even longer list than the MDP3 one
}

...or perhaps even a graph, which is even longer in the end.

I'm not against this kind of structure, but I wonder if it's worth it.

Cheers,
Angelo