[PATCH v2 0/4] Add mediatek,gce-props.yaml for other bindings reference mediatek,gce-events

[PATCH v2 0/4] Add mediatek,gce-props.yaml for other bindings reference mediatek,gce-events

[Jason-JH.Lin]

From: Jason-jh Lin <jason-jh.lin@mediatek.corp-partner.google.com>

Since mediatek,gce-events property is a HW event signal to GCE,
it can be used for both mailbox producers and consumers.

Add a new mediatek,gce-props.yaml to define this kind of GCE properties
and move mediatek,gce-events property existed in other bindings
to reference mediatek,gce-props.yaml.

Jason-JH.Lin (4):
  dt-bindings: mailbox: Add mediatek,gce-props.yaml
  dt-bindings: mailbox: mediatek: gce-mailbox: Add reference to
    gce-props.yaml
  dt-bindings: media: mediatek: mdp: Change mediatek,gce-events to
    reference
  dt-bindings: soc: mediatek: Change mediatek,gce-events to refernece

 .../mailbox/mediatek,gce-mailbox.yaml         |  6 ++-
 .../bindings/mailbox/mediatek,gce-props.yaml  | 41 +++++++++++++++++++
 .../bindings/media/mediatek,mdp3-rdma.yaml    | 11 ++---
 .../bindings/media/mediatek,mdp3-rsz.yaml     | 12 ++----
 .../bindings/media/mediatek,mdp3-wrot.yaml    | 12 ++----
 .../bindings/soc/mediatek/mediatek,ccorr.yaml | 12 ++----
 .../bindings/soc/mediatek/mediatek,mutex.yaml | 11 ++---
 .../bindings/soc/mediatek/mediatek,wdma.yaml  | 12 ++----
 8 files changed, 67 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml

-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/4] dt-bindings: mailbox: Add mediatek,gce-props.yaml

[Jason-JH.Lin]

Add mediatek,gce-props.yaml for specific GCE properties for both
Mailbox Providers and Consumers.

Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
---
 .../bindings/mailbox/mediatek,gce-props.yaml  | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
new file mode 100644
index 000000000000..aac776b74e88
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/mediatek,gce-props.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek Global Command Engine common propertes for both Mailbox Providers and Consumers.
+
+maintainers:
+  - Houlong Wei <houlong.wei@mediatek.com>
+
+description:
+  The Global Command Engine (GCE) is used to implement a Command Queue (CMDQ)
+  driver using the Linux Mailbox framework. The GCE is an instruction based,
+  multi-threaded, single-core command dispatcher for MediaTek hardware.
+  We use GCE Mailbox binding to define GCE core properties for GCE Mailbox Provider.
+  A device that uses the CMDQ driver to confige its hardware registers by requesting
+  the Linux Mailbox Channels in the GCE Mailbox Controller is a Mailbox Consumer.
+  This binding defines the common GCE properties for both Mailbox Providers and Consumers.
+
+properties:
+  mediatek,gce-events:
+    description:
+      Each gce-events is an event id corresponding to a specific hardware event
+      signal sent to GCE. The event id is defined in the GCE header
+      include/dt-bindings/gce/<chip>-gce.h of each chips.
+      CMDQ client drivers have two usage of GCE event signals,
+      one is sfotware tokens and the other is hardware events.
+      Software tokens refer to GCE event signals triggered by drivers.
+      e.g. software drivers append GCE commands to set a GCE event after specific
+      GCE commands. Or CMDQ client driver uses the CPU to write the event id
+      into GCE register to trigger the GCE event corresponding to the event id.
+      Hardware events refer to GCE event signals triggered by hardware engines.
+      e.g. When OVL fetches all the data in the frame buffer, OVL will send
+      a frame done irq and send a frame done GCE event via hardware bus directly
+      at the same time.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 1024
+
+additionalProperties: true
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/4] dt-bindings: mailbox: Add mediatek,gce-props.yaml

[Krzysztof Kozlowski]

On 10/01/2024 07:35, Jason-JH.Lin wrote:
> Add mediatek,gce-props.yaml for specific GCE properties for both
> Mailbox Providers and Consumers.
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> ---
>  .../bindings/mailbox/mediatek,gce-props.yaml  | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
> new file mode 100644
> index 000000000000..aac776b74e88
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mailbox/mediatek,gce-props.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Global Command Engine common propertes for both Mailbox Providers and Consumers.

That's way too long. Please observe coding style wrapping. Just MediaTek
Global Command Engine Common Propertes
(pay attention to capitalization)

> +
> +maintainers:
> +  - Houlong Wei <houlong.wei@mediatek.com>
> +
> +description:
> +  The Global Command Engine (GCE) is used to implement a Command Queue (CMDQ)
> +  driver using the Linux Mailbox framework. The GCE is an instruction based,
> +  multi-threaded, single-core command dispatcher for MediaTek hardware.
> +  We use GCE Mailbox binding to define GCE core properties for GCE Mailbox Provider.
> +  A device that uses the CMDQ driver to confige its hardware registers by requesting

configure?

> +  the Linux Mailbox Channels in the GCE Mailbox Controller is a Mailbox Consumer.
> +  This binding defines the common GCE properties for both Mailbox Providers and Consumers.

Last four sentences: a lot of text yet absolutely no meaning. I still do
not understand why same property is shared between provider and consumer.

Also, wrap according to Linux coding style.

> +
> +properties:
> +  mediatek,gce-events:
> +    description:
> +      Each gce-events is an event id corresponding to a specific hardware event
> +      signal sent to GCE. The event id is defined in the GCE header
> +      include/dt-bindings/gce/<chip>-gce.h of each chips.
> +      CMDQ client drivers have two usage of GCE event signals,
> +      one is sfotware tokens and the other is hardware events.
> +      Software tokens refer to GCE event signals triggered by drivers.
> +      e.g. software drivers append GCE commands to set a GCE event after specific
> +      GCE commands. Or CMDQ client driver uses the CPU to write the event id
> +      into GCE register to trigger the GCE event corresponding to the event id.
> +      Hardware events refer to GCE event signals triggered by hardware engines.
> +      e.g. When OVL fetches all the data in the frame buffer, OVL will send

There is no full stop before "e.g." and no new sentence. Please run this
through grammar check, like Google, ChatGPT or human.

> +      a frame done irq and send a frame done GCE event via hardware bus directly
> +      at the same time.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 1024


Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/4] dt-bindings: mailbox: Add mediatek,gce-props.yaml

[Jason-JH Lin (林睿祥)]

Hi Krzysztof,

Thanks for the reviews.

On Fri, 2024-01-12 at 08:20 +0100, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 10/01/2024 07:35, Jason-JH.Lin wrote:
> > Add mediatek,gce-props.yaml for specific GCE properties for both
> > Mailbox Providers and Consumers.
> > 
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > ---
> >  .../bindings/mailbox/mediatek,gce-props.yaml  | 41
> +++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
> > 
> > diff --git
> a/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
> b/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
> > new file mode 100644
> > index 000000000000..aac776b74e88
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> props.yaml
> > @@ -0,0 +1,41 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: 
> http://devicetree.org/schemas/mailbox/mediatek,gce-props.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek Global Command Engine common propertes for both
> Mailbox Providers and Consumers.
> 
> That's way too long. Please observe coding style wrapping. Just
> MediaTek
> Global Command Engine Common Propertes
> (pay attention to capitalization)
> 

OK, Thanks for the fixing title text.
I'll change it.

> > +
> > +maintainers:
> > +  - Houlong Wei <houlong.wei@mediatek.com>
> > +
> > +description:
> > +  The Global Command Engine (GCE) is used to implement a Command
> Queue (CMDQ)
> > +  driver using the Linux Mailbox framework. The GCE is an
> instruction based,
> > +  multi-threaded, single-core command dispatcher for MediaTek
> hardware.
> > +  We use GCE Mailbox binding to define GCE core properties for GCE
> Mailbox Provider.
> > +  A device that uses the CMDQ driver to confige its hardware
> registers by requesting
> 
> configure?

Yes, that typo didn't scan out.
Thanks for the correction.

> 
> > +  the Linux Mailbox Channels in the GCE Mailbox Controller is a
> Mailbox Consumer.
> > +  This binding defines the common GCE properties for both Mailbox
> Providers and Consumers.
> 
> Last four sentences: a lot of text yet absolutely no meaning. I still
> do
> not understand why same property is shared between provider and
> consumer.

OK, I'll add more description for that.

The main reason is that the property is used for GCE HW event ID
corresponding to a HW event signal. If the provider wants to get or set
an event value of the event ID by operating GCE HW register. It needs
to know what the event ID is.

> 
> Also, wrap according to Linux coding style.
> 
OK, I'll shrink them to less then 80 columns.

> > +
> > +properties:
> > +  mediatek,gce-events:
> > +    description:
> > +      Each gce-events is an event id corresponding to a specific
> hardware event
> > +      signal sent to GCE. The event id is defined in the GCE
> header
> > +      include/dt-bindings/gce/<chip>-gce.h of each chips.
> > +      CMDQ client drivers have two usage of GCE event signals,
> > +      one is sfotware tokens and the other is hardware events.
> > +      Software tokens refer to GCE event signals triggered by
> drivers.
> > +      e.g. software drivers append GCE commands to set a GCE event
> after specific
> > +      GCE commands. Or CMDQ client driver uses the CPU to write
> the event id
> > +      into GCE register to trigger the GCE event corresponding to
> the event id.
> > +      Hardware events refer to GCE event signals triggered by
> hardware engines.
> > +      e.g. When OVL fetches all the data in the frame buffer, OVL
> will send
> 
> There is no full stop before "e.g." and no new sentence. Please run
> this
> through grammar check, like Google, ChatGPT or human.
> 
OK. Thanks for the suggestion.
I'll correct them.

Regards,
Jason-JH.Lin

> > +      a frame done irq and send a frame done GCE event via
> hardware bus directly
> > +      at the same time.
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    minItems: 1
> > +    maxItems: 1024
> 
> 
> Best regards,
> Krzysztof
> 
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/4] dt-bindings: mailbox: mediatek: gce-mailbox: Add reference to gce-props.yaml

[Jason-JH.Lin]

1. Add "Provider" to the title to make it clearer.
2. Add reference to gce-props.yaml for adding mediatek,gce-events property.

Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
---
 .../devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml   | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
index cef9d7601398..728dc93117a6 100644
--- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
+++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/mailbox/mediatek,gce-mailbox.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Mediatek Global Command Engine Mailbox
+title: MediaTek Global Command Engine Mailbox Provider
 
 maintainers:
   - Houlong Wei <houlong.wei@mediatek.com>
@@ -57,6 +57,8 @@ required:
   - clocks
 
 allOf:
+  - $ref: mediatek,gce-props.yaml
+
   - if:
       not:
         properties:
@@ -67,7 +69,7 @@ allOf:
       required:
         - clock-names
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/4] dt-bindings: mailbox: mediatek: gce-mailbox: Add reference to gce-props.yaml

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1616 bytes --]

On Wed, Jan 10, 2024 at 02:35:30PM +0800, Jason-JH.Lin wrote:
> 1. Add "Provider" to the title to make it clearer.
> 2. Add reference to gce-props.yaml for adding mediatek,gce-events property.

I can see this from the diff. There's still no explanation here as to
why the mailbox provider needs to have a gce-event id. NAK until you can
explain that.

Cheers,
Conor.

> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> ---
>  .../devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml   | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> index cef9d7601398..728dc93117a6 100644
> --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> @@ -4,7 +4,7 @@
>  $id: http://devicetree.org/schemas/mailbox/mediatek,gce-mailbox.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Mediatek Global Command Engine Mailbox
> +title: MediaTek Global Command Engine Mailbox Provider
>  
>  maintainers:
>    - Houlong Wei <houlong.wei@mediatek.com>
> @@ -57,6 +57,8 @@ required:
>    - clocks
>  
>  allOf:
> +  - $ref: mediatek,gce-props.yaml
> +
>    - if:
>        not:
>          properties:
> @@ -67,7 +69,7 @@ allOf:
>        required:
>          - clock-names
>  
> -additionalProperties: false
> +unevaluatedProperties: false
>  
>  examples:
>    - |
> -- 
> 2.18.0
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/4] dt-bindings: mailbox: mediatek: gce-mailbox: Add reference to gce-props.yaml

[Jason-JH Lin (林睿祥)]

Hi Conor,

Thanks for the reviews.

On Wed, 2024-01-10 at 10:36 +0000, Conor Dooley wrote:
> On Wed, Jan 10, 2024 at 02:35:30PM +0800, Jason-JH.Lin wrote:
> > 1. Add "Provider" to the title to make it clearer.
> > 2. Add reference to gce-props.yaml for adding mediatek,gce-events
> > property.
> 
> I can see this from the diff. There's still no explanation here as to
> why the mailbox provider needs to have a gce-event id. NAK until you
> can
> explain that.
> 
Sorry for missing the reason in commit message, I'll add it in the next
version.

There are 2 reasons why the mailbox provider needs gce-events:
1. The mailbox provider here is CMDQ mailbox driver. It configures GCE
hardware register by CPU directly. If we want to set the default value
from 0 to 1 for specific gce-events during the initialization of CMDQ
driver. We need to tell CMDQ driver what gce-events need to be set and
I think such GCE hardware setting can get from its device node.

2. We'll have the secure CMDQ mailbox driver in the future patch [1].
It will request or reserve a mailbox channel, which is a dedicate GCE
thread, as a secure IRQ handler. This GCE thread executes a looping
instruction set that keeps waiting for the gce-event set from another
GCE thread in the secure world. So we also need to tell the CMDQ driver
what gce-event need to be waited.


[1] cmdq_sec_irq_notify_start() is where the GCE thread is requested to
prepare a looping instruction set to wait for the gce-event.
- 
https://patchwork.kernel.org/project/linux-mediatek/patch/20231222045228.27826-9-jason-jh.lin@mediatek.com/

Regards,
Jason-JH.Lin

> Cheers,
> Conor.
> 
> > 
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > ---
> >  .../devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml   | 6
> > ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > mailbox.yaml
> > b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > mailbox.yaml
> > index cef9d7601398..728dc93117a6 100644
> > --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > mailbox.yaml
> > +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > mailbox.yaml
> > @@ -4,7 +4,7 @@
> >  $id: 
> > http://devicetree.org/schemas/mailbox/mediatek,gce-mailbox.yaml#
> >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> >  
> > -title: Mediatek Global Command Engine Mailbox
> > +title: MediaTek Global Command Engine Mailbox Provider
> >  
> >  maintainers:
> >    - Houlong Wei <houlong.wei@mediatek.com>
> > @@ -57,6 +57,8 @@ required:
> >    - clocks
> >  
> >  allOf:
> > +  - $ref: mediatek,gce-props.yaml
> > +
> >    - if:
> >        not:
> >          properties:
> > @@ -67,7 +69,7 @@ allOf:
> >        required:
> >          - clock-names
> >  
> > -additionalProperties: false
> > +unevaluatedProperties: false
> >  
> >  examples:
> >    - |
> > -- 
> > 2.18.0
> > 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/4] dt-bindings: mailbox: mediatek: gce-mailbox: Add reference to gce-props.yaml

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 3862 bytes --]

On Wed, Jan 10, 2024 at 04:36:20PM +0000, Jason-JH Lin (林睿祥) wrote:
> Hi Conor,
> 
> Thanks for the reviews.
> 
> On Wed, 2024-01-10 at 10:36 +0000, Conor Dooley wrote:
> > On Wed, Jan 10, 2024 at 02:35:30PM +0800, Jason-JH.Lin wrote:
> > > 1. Add "Provider" to the title to make it clearer.
> > > 2. Add reference to gce-props.yaml for adding mediatek,gce-events
> > > property.
> > 
> > I can see this from the diff. There's still no explanation here as to
> > why the mailbox provider needs to have a gce-event id. NAK until you
> > can
> > explain that.
> > 
> Sorry for missing the reason in commit message, I'll add it in the next
> version.
> 
> There are 2 reasons why the mailbox provider needs gce-events:
> 1. The mailbox provider here is CMDQ mailbox driver. It configures GCE
> hardware register by CPU directly. If we want to set the default value
> from 0 to 1 for specific gce-events during the initialization of CMDQ
> driver. We need to tell CMDQ driver what gce-events need to be set and
> I think such GCE hardware setting can get from its device node.

Why would someone want to set it to 1 or 0?
At what level will that vary? Per SoC? Per board? Something else?

> 2. We'll have the secure CMDQ mailbox driver in the future patch [1].
> It will request or reserve a mailbox channel, which is a dedicate GCE
> thread, as a secure IRQ handler. This GCE thread executes a looping
> instruction set that keeps waiting for the gce-event set from another
> GCE thread in the secure world. So we also need to tell the CMDQ driver
> what gce-event need to be waited.

Ditto here, what level does this vary at? Do different SoCs or different
boards/platforms dictate the value?
Could this channel be determined from the soc-specific compatible?

In other words, please explain in your commit message why this requires
a property and is not detectable from any existing mechanism. From
reading this I don't know what is preventing the secure mailbox channel
from picking a "random" unused channel.

Thanks,
Conor.

> [1] cmdq_sec_irq_notify_start() is where the GCE thread is requested to
> prepare a looping instruction set to wait for the gce-event.
> - 
> https://patchwork.kernel.org/project/linux-mediatek/patch/20231222045228.27826-9-jason-jh.lin@mediatek.com/
> 
> Regards,
> Jason-JH.Lin
> 
> > Cheers,
> > Conor.
> > 
> > > 
> > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > > ---
> > >  .../devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml   | 6
> > > ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > mailbox.yaml
> > > b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > mailbox.yaml
> > > index cef9d7601398..728dc93117a6 100644
> > > --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > mailbox.yaml
> > > +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > mailbox.yaml
> > > @@ -4,7 +4,7 @@
> > >  $id: 
> > > http://devicetree.org/schemas/mailbox/mediatek,gce-mailbox.yaml#
> > >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> > >  
> > > -title: Mediatek Global Command Engine Mailbox
> > > +title: MediaTek Global Command Engine Mailbox Provider
> > >  
> > >  maintainers:
> > >    - Houlong Wei <houlong.wei@mediatek.com>
> > > @@ -57,6 +57,8 @@ required:
> > >    - clocks
> > >  
> > >  allOf:
> > > +  - $ref: mediatek,gce-props.yaml
> > > +
> > >    - if:
> > >        not:
> > >          properties:
> > > @@ -67,7 +69,7 @@ allOf:
> > >        required:
> > >          - clock-names
> > >  
> > > -additionalProperties: false
> > > +unevaluatedProperties: false
> > >  
> > >  examples:
> > >    - |
> > > -- 
> > > 2.18.0
> > > 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/4] dt-bindings: mailbox: mediatek: gce-mailbox: Add reference to gce-props.yaml

[Jason-JH Lin (林睿祥)]

On Thu, 2024-01-11 at 17:31 +0000, Conor Dooley wrote:
> On Wed, Jan 10, 2024 at 04:36:20PM +0000, Jason-JH Lin (林睿祥) wrote:
> > Hi Conor,
> > 
> > Thanks for the reviews.
> > 
> > On Wed, 2024-01-10 at 10:36 +0000, Conor Dooley wrote:
> > > On Wed, Jan 10, 2024 at 02:35:30PM +0800, Jason-JH.Lin wrote:
> > > > 1. Add "Provider" to the title to make it clearer.
> > > > 2. Add reference to gce-props.yaml for adding mediatek,gce-
> > > > events
> > > > property.
> > > 
> > > I can see this from the diff. There's still no explanation here
> > > as to
> > > why the mailbox provider needs to have a gce-event id. NAK until
> > > you
> > > can
> > > explain that.
> > > 
> > 
> > Sorry for missing the reason in commit message, I'll add it in the
> > next
> > version.
> > 
> > There are 2 reasons why the mailbox provider needs gce-events:
> > 1. The mailbox provider here is CMDQ mailbox driver. It configures
> > GCE
> > hardware register by CPU directly. If we want to set the default
> > value
> > from 0 to 1 for specific gce-events during the initialization of
> > CMDQ
> > driver. We need to tell CMDQ driver what gce-events need to be set
> > and
> > I think such GCE hardware setting can get from its device node.
> 
> Why would someone want to set it to 1 or 0?

GCE HW have an event table in SRAM. Each event ID has a boolean event
value and the default value is 0. There are 1024 event IDs (0~1023) in
the event table. The mediatek,gce-events is the property to get the
event IDs.

E.g.
In some camera suspend/resume use cases, the may set the event ID: 100
to 1 before suspend. When CMDQ driver resumes, all the event value of
event IDs will be clear to 0. But camera driver won't know the event
IDs:100 has been cleared to 0 and still send a cmd to wait for the
event ID:100. So CMDQ driver may need to keep the value of event ID:
100 before suspend and set back the value to 1 after CMDQ driver
clearing all the event value of event IDs.

CMDQ driver will need to know which event IDs' values need to be
backupped and restored in that camera suspend/resume use case.

> At what level will that vary? Per SoC? Per board? Something else?
> 

I think the SoC supports camera feature may need this property.

> > 2. We'll have the secure CMDQ mailbox driver in the future patch
> > [1].
> > It will request or reserve a mailbox channel, which is a dedicate
> > GCE
> > thread, as a secure IRQ handler. This GCE thread executes a looping
> > instruction set that keeps waiting for the gce-event set from
> > another
> > GCE thread in the secure world. So we also need to tell the CMDQ
> > driver
> > what gce-event need to be waited.
> 
> Ditto here, what level does this vary at? Do different SoCs or
> different
> boards/platforms dictate the value?

It's a SoC level, the SoC supports secure feature will need this
property.

> Could this channel be determined from the soc-specific compatible?
> 
> In other words, please explain in your commit message why this
> requires
> a property and is not detectable from any existing mechanism. From
> reading this I don't know what is preventing the secure mailbox
> channel
> from picking a "random" unused channel.

The secure channel could be dedicated from the soc-specific compatible,
but the event ID couldn't.

The same event signal corresponding event ID may changes in different
SoC.
E.g.
The HW event signal for CMDQ_EVENT_VDO0_MUTEX_STREAM_DONE_0 is
corresponding to GCE event ID: 574 in MT8188, but it's corresponding to
eventID: 597 in MT8195.

I can take any of the "unused" mailbox channel for the secure irq
handler. But without the property mediatek,gce-events, the secure
channel might not know what event ID to wait for.

Regards,
Jason-JH.Lin
> 
> Thanks,
> Conor.
> 
> > [1] cmdq_sec_irq_notify_start() is where the GCE thread is
> > requested to
> > prepare a looping instruction set to wait for the gce-event.
> > - 
> > 
https://patchwork.kernel.org/project/linux-mediatek/patch/20231222045228.27826-9-jason-jh.lin@mediatek.com/
> > 
> > Regards,
> > Jason-JH.Lin
> > 
> > > Cheers,
> > > Conor.
> > > 
> > > > 
> > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > > > ---
> > > >  .../devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml   |
> > > > 6
> > > > ++++--
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > mailbox.yaml
> > > > b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > mailbox.yaml
> > > > index cef9d7601398..728dc93117a6 100644
> > > > --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > mailbox.yaml
> > > > +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > mailbox.yaml
> > > > @@ -4,7 +4,7 @@
> > > >  $id: 
> > > > 
http://devicetree.org/schemas/mailbox/mediatek,gce-mailbox.yaml#
> > > >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> > > >  
> > > > -title: Mediatek Global Command Engine Mailbox
> > > > +title: MediaTek Global Command Engine Mailbox Provider
> > > >  
> > > >  maintainers:
> > > >    - Houlong Wei <houlong.wei@mediatek.com>
> > > > @@ -57,6 +57,8 @@ required:
> > > >    - clocks
> > > >  
> > > >  allOf:
> > > > +  - $ref: mediatek,gce-props.yaml
> > > > +
> > > >    - if:
> > > >        not:
> > > >          properties:
> > > > @@ -67,7 +69,7 @@ allOf:
> > > >        required:
> > > >          - clock-names
> > > >  
> > > > -additionalProperties: false
> > > > +unevaluatedProperties: false
> > > >  
> > > >  examples:
> > > >    - |
> > > > -- 
> > > > 2.18.0
> > > > 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/4] dt-bindings: mailbox: mediatek: gce-mailbox: Add reference to gce-props.yaml

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 7071 bytes --]

On Fri, Jan 12, 2024 at 07:44:13AM +0000, Jason-JH Lin (林睿祥) wrote:
> On Thu, 2024-01-11 at 17:31 +0000, Conor Dooley wrote:
> > On Wed, Jan 10, 2024 at 04:36:20PM +0000, Jason-JH Lin (林睿祥) wrote:
> > > Hi Conor,
> > > 
> > > Thanks for the reviews.
> > > 
> > > On Wed, 2024-01-10 at 10:36 +0000, Conor Dooley wrote:
> > > > On Wed, Jan 10, 2024 at 02:35:30PM +0800, Jason-JH.Lin wrote:
> > > > > 1. Add "Provider" to the title to make it clearer.
> > > > > 2. Add reference to gce-props.yaml for adding mediatek,gce-
> > > > > events
> > > > > property.
> > > > 
> > > > I can see this from the diff. There's still no explanation here
> > > > as to
> > > > why the mailbox provider needs to have a gce-event id. NAK until
> > > > you
> > > > can
> > > > explain that.
> > > > 
> > > 
> > > Sorry for missing the reason in commit message, I'll add it in the
> > > next
> > > version.
> > > 
> > > There are 2 reasons why the mailbox provider needs gce-events:
> > > 1. The mailbox provider here is CMDQ mailbox driver. It configures
> > > GCE
> > > hardware register by CPU directly. If we want to set the default
> > > value
> > > from 0 to 1 for specific gce-events during the initialization of
> > > CMDQ
> > > driver. We need to tell CMDQ driver what gce-events need to be set
> > > and
> > > I think such GCE hardware setting can get from its device node.
> > 
> > Why would someone want to set it to 1 or 0?
> 
> GCE HW have an event table in SRAM. Each event ID has a boolean event
> value and the default value is 0. There are 1024 event IDs (0~1023) in
> the event table. The mediatek,gce-events is the property to get the
> event IDs.
> 
> E.g.
> In some camera suspend/resume use cases, the may set the event ID: 100
> to 1 before suspend. When CMDQ driver resumes, all the event value of
> event IDs will be clear to 0. But camera driver won't know the event
> IDs:100 has been cleared to 0 and still send a cmd to wait for the
> event ID:100. So CMDQ driver may need to keep the value of event ID:
> 100 before suspend and set back the value to 1 after CMDQ driver
> clearing all the event value of event IDs.
> 
> CMDQ driver will need to know which event IDs' values need to be
> backupped and restored in that camera suspend/resume use case.

Unfortunately "some use case" doesn't really help me here, it is hard
for me to tell whether these use cases are software policy or an
attribute of the hardware. If I had the same exact camera but was using
it for a different reason, might I set it to 1 before suspend in one
case but not in the other?

I also don't really understand how having this property helps in this
case either. If a camera is a mailbox consumer, it should have a
gce-event property for the event ID that it is using in its node.

Why is that not sufficient and a gce-event also needs to be present in
the consumer node? It seems to me like having it in the consumer alone
should be sufficient, and the registration process would inform the
mailbox provider driver which gce-event ID was being used by the camera.

> 
> > At what level will that vary? Per SoC? Per board? Something else?
> > 
> 
> I think the SoC supports camera feature may need this property.
> 
> > > 2. We'll have the secure CMDQ mailbox driver in the future patch
> > > [1].
> > > It will request or reserve a mailbox channel, which is a dedicate
> > > GCE
> > > thread, as a secure IRQ handler. This GCE thread executes a looping
> > > instruction set that keeps waiting for the gce-event set from
> > > another
> > > GCE thread in the secure world. So we also need to tell the CMDQ
> > > driver
> > > what gce-event need to be waited.
> > 
> > Ditto here, what level does this vary at? Do different SoCs or
> > different
> > boards/platforms dictate the value?
> 
> It's a SoC level, the SoC supports secure feature will need this
> property.
> 
> > Could this channel be determined from the soc-specific compatible?
> > 
> > In other words, please explain in your commit message why this
> > requires
> > a property and is not detectable from any existing mechanism. From
> > reading this I don't know what is preventing the secure mailbox
> > channel
> > from picking a "random" unused channel.
> 
> The secure channel could be dedicated from the soc-specific compatible,
> but the event ID couldn't.
> 
> The same event signal corresponding event ID may changes in different
> SoC.
> E.g.
> The HW event signal for CMDQ_EVENT_VDO0_MUTEX_STREAM_DONE_0 is
> corresponding to GCE event ID: 574 in MT8188, but it's corresponding to
> eventID: 597 in MT8195.

Is it always 574 in MT8188 and always 597 in MT8195?

Thanks,
Conor.

> I can take any of the "unused" mailbox channel for the secure irq
> handler. But without the property mediatek,gce-events, the secure
> channel might not know what event ID to wait for.
> 
> Regards,
> Jason-JH.Lin
> > 
> > Thanks,
> > Conor.
> > 
> > > [1] cmdq_sec_irq_notify_start() is where the GCE thread is
> > > requested to
> > > prepare a looping instruction set to wait for the gce-event.
> > > - 
> > > 
> https://patchwork.kernel.org/project/linux-mediatek/patch/20231222045228.27826-9-jason-jh.lin@mediatek.com/
> > > 
> > > Regards,
> > > Jason-JH.Lin
> > > 
> > > > Cheers,
> > > > Conor.
> > > > 
> > > > > 
> > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > > > > ---
> > > > >  .../devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml   |
> > > > > 6
> > > > > ++++--
> > > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > mailbox.yaml
> > > > > b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > mailbox.yaml
> > > > > index cef9d7601398..728dc93117a6 100644
> > > > > --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > mailbox.yaml
> > > > > +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > mailbox.yaml
> > > > > @@ -4,7 +4,7 @@
> > > > >  $id: 
> > > > > 
> http://devicetree.org/schemas/mailbox/mediatek,gce-mailbox.yaml#
> > > > >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > >  
> > > > > -title: Mediatek Global Command Engine Mailbox
> > > > > +title: MediaTek Global Command Engine Mailbox Provider
> > > > >  
> > > > >  maintainers:
> > > > >    - Houlong Wei <houlong.wei@mediatek.com>
> > > > > @@ -57,6 +57,8 @@ required:
> > > > >    - clocks
> > > > >  
> > > > >  allOf:
> > > > > +  - $ref: mediatek,gce-props.yaml
> > > > > +
> > > > >    - if:
> > > > >        not:
> > > > >          properties:
> > > > > @@ -67,7 +69,7 @@ allOf:
> > > > >        required:
> > > > >          - clock-names
> > > > >  
> > > > > -additionalProperties: false
> > > > > +unevaluatedProperties: false
> > > > >  
> > > > >  examples:
> > > > >    - |
> > > > > -- 
> > > > > 2.18.0
> > > > > 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/4] dt-bindings: mailbox: mediatek: gce-mailbox: Add reference to gce-props.yaml

[Jason-JH Lin (林睿祥)]

On Mon, 2024-01-15 at 17:23 +0000, Conor Dooley wrote:
> On Fri, Jan 12, 2024 at 07:44:13AM +0000, Jason-JH Lin (林睿祥) wrote:
> > On Thu, 2024-01-11 at 17:31 +0000, Conor Dooley wrote:
> > > On Wed, Jan 10, 2024 at 04:36:20PM +0000, Jason-JH Lin (林睿祥)
> > > wrote:
> > > > Hi Conor,
> > > > 
> > > > Thanks for the reviews.
> > > > 
> > > > On Wed, 2024-01-10 at 10:36 +0000, Conor Dooley wrote:
> > > > > On Wed, Jan 10, 2024 at 02:35:30PM +0800, Jason-JH.Lin wrote:
> > > > > > 1. Add "Provider" to the title to make it clearer.
> > > > > > 2. Add reference to gce-props.yaml for adding mediatek,gce-
> > > > > > events
> > > > > > property.
> > > > > 
> > > > > I can see this from the diff. There's still no explanation
> > > > > here
> > > > > as to
> > > > > why the mailbox provider needs to have a gce-event id. NAK
> > > > > until
> > > > > you
> > > > > can
> > > > > explain that.
> > > > > 
> > > > 
> > > > Sorry for missing the reason in commit message, I'll add it in
> > > > the
> > > > next
> > > > version.
> > > > 
> > > > There are 2 reasons why the mailbox provider needs gce-events:
> > > > 1. The mailbox provider here is CMDQ mailbox driver. It
> > > > configures
> > > > GCE
> > > > hardware register by CPU directly. If we want to set the
> > > > default
> > > > value
> > > > from 0 to 1 for specific gce-events during the initialization
> > > > of
> > > > CMDQ
> > > > driver. We need to tell CMDQ driver what gce-events need to be
> > > > set
> > > > and
> > > > I think such GCE hardware setting can get from its device node.
> > > 
> > > Why would someone want to set it to 1 or 0?
> > 
> > GCE HW have an event table in SRAM. Each event ID has a boolean
> > event
> > value and the default value is 0. There are 1024 event IDs (0~1023)
> > in
> > the event table. The mediatek,gce-events is the property to get the
> > event IDs.
> > 
> > E.g.
> > In some camera suspend/resume use cases, the may set the event ID:
> > 100
> > to 1 before suspend. When CMDQ driver resumes, all the event value
> > of
> > event IDs will be clear to 0. But camera driver won't know the
> > event
> > IDs:100 has been cleared to 0 and still send a cmd to wait for the
> > event ID:100. So CMDQ driver may need to keep the value of event
> > ID:
> > 100 before suspend and set back the value to 1 after CMDQ driver
> > clearing all the event value of event IDs.
> > 
> > CMDQ driver will need to know which event IDs' values need to be
> > backupped and restored in that camera suspend/resume use case.
> 
> Unfortunately "some use case" doesn't really help me here, it is hard
> for me to tell whether these use cases are software policy or an
> attribute of the hardware. If I had the same exact camera but was
> using
> it for a different reason, might I set it to 1 before suspend in one
> case but not in the other?
> 

It depends on the scenario, not the camera. If the same camera will use
the value of event ID after resume, it should backup the value of event
ID before suspend.

> I also don't really understand how having this property helps in this
> case either. If a camera is a mailbox consumer, it should have a
> gce-event property for the event ID that it is using in its node.
> 
> Why is that not sufficient and a gce-event also needs to be present
> in
> the consumer node? It seems to me like having it in the consumer
> alone
> should be sufficient, and the registration process would inform the
> mailbox provider driver which gce-event ID was being used by the
> camera.

Hmm...
In this camera scenario, I think gce-events can be set in consumer's
device node cloud be sufficient.

The CMDQ mailbox driver can open an API for camera driver to set its
event ID to be backup, so we don't need to get this event ID from
mailbox provider's device node.

> 
> > 
> > > At what level will that vary? Per SoC? Per board? Something else?
> > > 
> > 
> > I think the SoC supports camera feature may need this property.
> > 
> > > > 2. We'll have the secure CMDQ mailbox driver in the future
> > > > patch
> > > > [1].
> > > > It will request or reserve a mailbox channel, which is a
> > > > dedicate
> > > > GCE
> > > > thread, as a secure IRQ handler. This GCE thread executes a
> > > > looping
> > > > instruction set that keeps waiting for the gce-event set from
> > > > another
> > > > GCE thread in the secure world. So we also need to tell the
> > > > CMDQ
> > > > driver
> > > > what gce-event need to be waited.
> > > 
> > > Ditto here, what level does this vary at? Do different SoCs or
> > > different
> > > boards/platforms dictate the value?
> > 
> > It's a SoC level, the SoC supports secure feature will need this
> > property.
> > 
> > > Could this channel be determined from the soc-specific
> > > compatible?
> > > 
> > > In other words, please explain in your commit message why this
> > > requires
> > > a property and is not detectable from any existing mechanism.
> > > From
> > > reading this I don't know what is preventing the secure mailbox
> > > channel
> > > from picking a "random" unused channel.
> > 
> > The secure channel could be dedicated from the soc-specific
> > compatible,
> > but the event ID couldn't.
> > 
> > The same event signal corresponding event ID may changes in
> > different
> > SoC.
> > E.g.
> > The HW event signal for CMDQ_EVENT_VDO0_MUTEX_STREAM_DONE_0 is
> > corresponding to GCE event ID: 574 in MT8188, but it's
> > corresponding to
> > eventID: 597 in MT8195.
> 
> Is it always 574 in MT8188 and always 597 in MT8195?
> 
Yes, some gce-events are hardware bound and they can not change by
software. For example, in MT8195, when VDO0_MUTEX is stream done,
VDO_MUTEX will send an event signal to GCE, and the value of event
ID:597 will be set to 1. In MT8188, the value of event ID: 574 will be
set to 1 when VOD0_MUTEX is stream done.

Some of gce-events are not hardware bound and they can change by
software. For example, in MT8188, we can take the event ID: 855 that is
not bound to any hardware to set its value to 1 when the driver in
secure world completes a task. But in MT8195, the event ID: 855 is
already bound to VDEC_LAT1, so we have to take another event ID to
achieve the same purpose.
This event ID can be change any IDs that is not bound to any hardware
and is not use in any software driver yet.
We can see if the event ID is bound to the hardware or is used by
software driver in the header
include/de-bindings/mailbox/mediatek,mt8188-gce.h.

Regards,
Jason-JH.Lin

> Thanks,
> Conor.
> 
> > I can take any of the "unused" mailbox channel for the secure irq
> > handler. But without the property mediatek,gce-events, the secure
> > channel might not know what event ID to wait for.
> > 
> > Regards,
> > Jason-JH.Lin
> > > 
> > > Thanks,
> > > Conor.
> > > 
> > > > [1] cmdq_sec_irq_notify_start() is where the GCE thread is
> > > > requested to
> > > > prepare a looping instruction set to wait for the gce-event.
> > > > - 
> > > > 
> > 
> > 
https://patchwork.kernel.org/project/linux-mediatek/patch/20231222045228.27826-9-jason-jh.lin@mediatek.com/
> > > > 
> > > > Regards,
> > > > Jason-JH.Lin
> > > > 
> > > > > Cheers,
> > > > > Conor.
> > > > > 
> > > > > > 
> > > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > > > > > ---
> > > > > >  .../devicetree/bindings/mailbox/mediatek,gce-
> > > > > > mailbox.yaml   |
> > > > > > 6
> > > > > > ++++--
> > > > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > > mailbox.yaml
> > > > > > b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > > mailbox.yaml
> > > > > > index cef9d7601398..728dc93117a6 100644
> > > > > > ---
> > > > > > a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > > mailbox.yaml
> > > > > > +++
> > > > > > b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > > mailbox.yaml
> > > > > > @@ -4,7 +4,7 @@
> > > > > >  $id: 
> > > > > > 
> > 
> > http://devicetree.org/schemas/mailbox/mediatek,gce-mailbox.yaml#
> > > > > >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > >  
> > > > > > -title: Mediatek Global Command Engine Mailbox
> > > > > > +title: MediaTek Global Command Engine Mailbox Provider
> > > > > >  
> > > > > >  maintainers:
> > > > > >    - Houlong Wei <houlong.wei@mediatek.com>
> > > > > > @@ -57,6 +57,8 @@ required:
> > > > > >    - clocks
> > > > > >  
> > > > > >  allOf:
> > > > > > +  - $ref: mediatek,gce-props.yaml
> > > > > > +
> > > > > >    - if:
> > > > > >        not:
> > > > > >          properties:
> > > > > > @@ -67,7 +69,7 @@ allOf:
> > > > > >        required:
> > > > > >          - clock-names
> > > > > >  
> > > > > > -additionalProperties: false
> > > > > > +unevaluatedProperties: false
> > > > > >  
> > > > > >  examples:
> > > > > >    - |
> > > > > > -- 
> > > > > > 2.18.0
> > > > > > 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/4] dt-bindings: mailbox: mediatek: gce-mailbox: Add reference to gce-props.yaml

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 3173 bytes --]

On Tue, Jan 16, 2024 at 08:21:15AM +0000, Jason-JH Lin (林睿祥) wrote:
> On Mon, 2024-01-15 at 17:23 +0000, Conor Dooley wrote:
> > On Fri, Jan 12, 2024 at 07:44:13AM +0000, Jason-JH Lin (林睿祥) wrote:
> > > On Thu, 2024-01-11 at 17:31 +0000, Conor Dooley wrote:
> > > > On Wed, Jan 10, 2024 at 04:36:20PM +0000, Jason-JH Lin (林睿祥)

> > > > > 2. We'll have the secure CMDQ mailbox driver in the future
> > > > > patch
> > > > > [1].
> > > > > It will request or reserve a mailbox channel, which is a
> > > > > dedicate
> > > > > GCE
> > > > > thread, as a secure IRQ handler. This GCE thread executes a
> > > > > looping
> > > > > instruction set that keeps waiting for the gce-event set from
> > > > > another
> > > > > GCE thread in the secure world. So we also need to tell the
> > > > > CMDQ
> > > > > driver
> > > > > what gce-event need to be waited.
> > > > 
> > > > Ditto here, what level does this vary at? Do different SoCs or
> > > > different
> > > > boards/platforms dictate the value?
> > > 
> > > It's a SoC level, the SoC supports secure feature will need this
> > > property.
> > > 
> > > > Could this channel be determined from the soc-specific
> > > > compatible?
> > > > 
> > > > In other words, please explain in your commit message why this
> > > > requires
> > > > a property and is not detectable from any existing mechanism.
> > > > From
> > > > reading this I don't know what is preventing the secure mailbox
> > > > channel
> > > > from picking a "random" unused channel.
> > > 
> > > The secure channel could be dedicated from the soc-specific
> > > compatible,
> > > but the event ID couldn't.
> > > 
> > > The same event signal corresponding event ID may changes in
> > > different
> > > SoC.
> > > E.g.
> > > The HW event signal for CMDQ_EVENT_VDO0_MUTEX_STREAM_DONE_0 is
> > > corresponding to GCE event ID: 574 in MT8188, but it's
> > > corresponding to
> > > eventID: 597 in MT8195.
> > 
> > Is it always 574 in MT8188 and always 597 in MT8195?
> > 
> Yes, some gce-events are hardware bound and they can not change by
> software. For example, in MT8195, when VDO0_MUTEX is stream done,
> VDO_MUTEX will send an event signal to GCE, and the value of event
> ID:597 will be set to 1. In MT8188, the value of event ID: 574 will be
> set to 1 when VOD0_MUTEX is stream done.
> 
> Some of gce-events are not hardware bound and they can change by
> software. For example, in MT8188, we can take the event ID: 855 that is
> not bound to any hardware to set its value to 1 when the driver in
> secure world completes a task. But in MT8195, the event ID: 855 is
> already bound to VDEC_LAT1, so we have to take another event ID to
> achieve the same purpose.
> This event ID can be change any IDs that is not bound to any hardware
> and is not use in any software driver yet.
> We can see if the event ID is bound to the hardware or is used by
> software driver in the header
> include/de-bindings/mailbox/mediatek,mt8188-gce.h.

I see. Bring this particular patch back with your future series that
adds support for the secure channel then.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/4] dt-bindings: mailbox: mediatek: gce-mailbox: Add reference to gce-props.yaml

[Jason-JH Lin (林睿祥)]

On Tue, 2024-01-16 at 17:22 +0000, Conor Dooley wrote:
> On Tue, Jan 16, 2024 at 08:21:15AM +0000, Jason-JH Lin (林睿祥) wrote:
> > On Mon, 2024-01-15 at 17:23 +0000, Conor Dooley wrote:
> > > On Fri, Jan 12, 2024 at 07:44:13AM +0000, Jason-JH Lin (林睿祥)
> > > wrote:
> > > > On Thu, 2024-01-11 at 17:31 +0000, Conor Dooley wrote:
> > > > > On Wed, Jan 10, 2024 at 04:36:20PM +0000, Jason-JH Lin (林睿祥)
> > > > > > 2. We'll have the secure CMDQ mailbox driver in the future
> > > > > > patch
> > > > > > [1].
> > > > > > It will request or reserve a mailbox channel, which is a
> > > > > > dedicate
> > > > > > GCE
> > > > > > thread, as a secure IRQ handler. This GCE thread executes a
> > > > > > looping
> > > > > > instruction set that keeps waiting for the gce-event set
> > > > > > from
> > > > > > another
> > > > > > GCE thread in the secure world. So we also need to tell the
> > > > > > CMDQ
> > > > > > driver
> > > > > > what gce-event need to be waited.
> > > > > 
> > > > > Ditto here, what level does this vary at? Do different SoCs
> > > > > or
> > > > > different
> > > > > boards/platforms dictate the value?
> > > > 
> > > > It's a SoC level, the SoC supports secure feature will need
> > > > this
> > > > property.
> > > > 
> > > > > Could this channel be determined from the soc-specific
> > > > > compatible?
> > > > > 
> > > > > In other words, please explain in your commit message why
> > > > > this
> > > > > requires
> > > > > a property and is not detectable from any existing mechanism.
> > > > > From
> > > > > reading this I don't know what is preventing the secure
> > > > > mailbox
> > > > > channel
> > > > > from picking a "random" unused channel.
> > > > 
> > > > The secure channel could be dedicated from the soc-specific
> > > > compatible,
> > > > but the event ID couldn't.
> > > > 
> > > > The same event signal corresponding event ID may changes in
> > > > different
> > > > SoC.
> > > > E.g.
> > > > The HW event signal for CMDQ_EVENT_VDO0_MUTEX_STREAM_DONE_0 is
> > > > corresponding to GCE event ID: 574 in MT8188, but it's
> > > > corresponding to
> > > > eventID: 597 in MT8195.
> > > 
> > > Is it always 574 in MT8188 and always 597 in MT8195?
> > > 
> > 
> > Yes, some gce-events are hardware bound and they can not change by
> > software. For example, in MT8195, when VDO0_MUTEX is stream done,
> > VDO_MUTEX will send an event signal to GCE, and the value of event
> > ID:597 will be set to 1. In MT8188, the value of event ID: 574 will
> > be
> > set to 1 when VOD0_MUTEX is stream done.
> > 
> > Some of gce-events are not hardware bound and they can change by
> > software. For example, in MT8188, we can take the event ID: 855
> > that is
> > not bound to any hardware to set its value to 1 when the driver in
> > secure world completes a task. But in MT8195, the event ID: 855 is
> > already bound to VDEC_LAT1, so we have to take another event ID to
> > achieve the same purpose.
> > This event ID can be change to any IDs that is not bound to any
> > hardware
> > and is not used in any software driver yet.
> > We can see if the event ID is bound to the hardware or is used by
> > software driver in the header
> > include/de-bindings/mailbox/mediatek,mt8188-gce.h.
> 
> I see. Bring this particular patch back with your future series that
> adds support for the secure channel then.
> 

OK, I'll move this particular patch to the future secure series that
adds support for the secure channel. Thanks!

Regards,
Jason-JH.Lin

> Thanks,
> Conor.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/4] dt-bindings: media: mediatek: mdp: Change mediatek,gce-events to reference

[Jason-JH.Lin]

Change mediatek,gce-events property to reference mediatek,gce-props.yaml
instead of defining itself.

Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
---
 .../bindings/media/mediatek,mdp3-rdma.yaml           | 11 +++--------
 .../devicetree/bindings/media/mediatek,mdp3-rsz.yaml | 12 ++++--------
 .../bindings/media/mediatek,mdp3-wrot.yaml           | 12 ++++--------
 3 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
index 59db8306485b..1ba70b9a5843 100644
--- a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
@@ -44,13 +44,6 @@ properties:
       4 arguments defined in this property. Each GCE subsys id is mapping to
       a client defined in the header include/dt-bindings/gce/<chip>-gce.h.
 
-  mediatek,gce-events:
-    description:
-      The event id which is mapping to the specific hardware event signal
-      to gce. The event id is defined in the gce header
-      include/dt-bindings/gce/<chip>-gce.h of each chips.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
-
   mediatek,scp:
     $ref: /schemas/types.yaml#/definitions/phandle
     description:
@@ -96,6 +89,8 @@ required:
   - '#dma-cells'
 
 allOf:
+  - $ref: ../mailbox/mediatek,gce-props.yaml
+
   - if:
       properties:
         compatible:
@@ -142,7 +137,7 @@ allOf:
         clocks:
           maxItems: 1
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |
diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml
index f5676bec4326..13219cc946a9 100644
--- a/Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml
@@ -38,13 +38,6 @@ properties:
       4 arguments defined in this property. Each GCE subsys id is mapping to
       a client defined in the header include/dt-bindings/gce/<chip>-gce.h.
 
-  mediatek,gce-events:
-    description:
-      The event id which is mapping to the specific hardware event signal
-      to gce. The event id is defined in the gce header
-      include/dt-bindings/gce/<chip>-gce.h of each chips.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
-
   clocks:
     minItems: 1
 
@@ -55,7 +48,10 @@ required:
   - mediatek,gce-events
   - clocks
 
-additionalProperties: false
+allOf:
+  - $ref: ../mailbox/mediatek,gce-props.yaml
+
+unevaluatedProperties: false
 
 examples:
   - |
diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml
index 53a679338402..40da895800e7 100644
--- a/Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml
@@ -38,13 +38,6 @@ properties:
       4 arguments defined in this property. Each GCE subsys id is mapping to
       a client defined in the header include/dt-bindings/gce/<chip>-gce.h.
 
-  mediatek,gce-events:
-    description:
-      The event id which is mapping to the specific hardware event signal
-      to gce. The event id is defined in the gce header
-      include/dt-bindings/gce/<chip>-gce.h of each chips.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
-
   power-domains:
     maxItems: 1
 
@@ -67,7 +60,10 @@ required:
   - iommus
   - '#dma-cells'
 
-additionalProperties: false
+allOf:
+  - $ref: ../mailbox/mediatek,gce-props.yaml
+
+unevaluatedProperties: false
 
 examples:
   - |
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/4] dt-bindings: media: mediatek: mdp: Change mediatek,gce-events to reference

[Krzysztof Kozlowski]

On 10/01/2024 07:35, Jason-JH.Lin wrote:
> Change mediatek,gce-events property to reference mediatek,gce-props.yaml
> instead of defining itself.
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> ---
>  .../bindings/media/mediatek,mdp3-rdma.yaml           | 11 +++--------
>  .../devicetree/bindings/media/mediatek,mdp3-rsz.yaml | 12 ++++--------
>  .../bindings/media/mediatek,mdp3-wrot.yaml           | 12 ++++--------
>  3 files changed, 11 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
> index 59db8306485b..1ba70b9a5843 100644
> --- a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
> @@ -44,13 +44,6 @@ properties:
>        4 arguments defined in this property. Each GCE subsys id is mapping to
>        a client defined in the header include/dt-bindings/gce/<chip>-gce.h.
>  
> -  mediatek,gce-events:
> -    description:
> -      The event id which is mapping to the specific hardware event signal
> -      to gce. The event id is defined in the gce header
> -      include/dt-bindings/gce/<chip>-gce.h of each chips.
> -    $ref: /schemas/types.yaml#/definitions/uint32-array
> -
>    mediatek,scp:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description:
> @@ -96,6 +89,8 @@ required:
>    - '#dma-cells'
>  
>  allOf:
> +  - $ref: ../mailbox/mediatek,gce-props.yaml

You need full path, so /schemas/mailbox/

Applies to all the patches.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/4] dt-bindings: media: mediatek: mdp: Change mediatek,gce-events to reference

[Jason-JH Lin (林睿祥)]

Hi Krzysztof,

Thanks for the reivews.

On Fri, 2024-01-12 at 08:22 +0100, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 10/01/2024 07:35, Jason-JH.Lin wrote:
> > Change mediatek,gce-events property to reference mediatek,gce-
> props.yaml
> > instead of defining itself.
> > 
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > ---
> >  .../bindings/media/mediatek,mdp3-rdma.yaml           | 11 +++-----
> ---
> >  .../devicetree/bindings/media/mediatek,mdp3-rsz.yaml | 12 ++++--
> ------
> >  .../bindings/media/mediatek,mdp3-wrot.yaml           | 12 ++++--
> ------
> >  3 files changed, 11 insertions(+), 24 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-
> rdma.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-
> rdma.yaml
> > index 59db8306485b..1ba70b9a5843 100644
> > --- a/Documentation/devicetree/bindings/media/mediatek,mdp3-
> rdma.yaml
> > +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-
> rdma.yaml
> > @@ -44,13 +44,6 @@ properties:
> >        4 arguments defined in this property. Each GCE subsys id is
> mapping to
> >        a client defined in the header include/dt-
> bindings/gce/<chip>-gce.h.
> >  
> > -  mediatek,gce-events:
> > -    description:
> > -      The event id which is mapping to the specific hardware event
> signal
> > -      to gce. The event id is defined in the gce header
> > -      include/dt-bindings/gce/<chip>-gce.h of each chips.
> > -    $ref: /schemas/types.yaml#/definitions/uint32-array
> > -
> >    mediatek,scp:
> >      $ref: /schemas/types.yaml#/definitions/phandle
> >      description:
> > @@ -96,6 +89,8 @@ required:
> >    - '#dma-cells'
> >  
> >  allOf:
> > +  - $ref: ../mailbox/mediatek,gce-props.yaml
> 
> You need full path, so /schemas/mailbox/
> 
> Applies to all the patches.
> 

OK, I'll apply it to all the patches.

Regards,
Jason-JH.Lin

> Best regards,
> Krzysztof
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/4] dt-bindings: soc: mediatek: Change mediatek,gce-events to refernece

[Jason-JH.Lin]

Change mediatek,gce-events property to reference mediatek,gce-props.yaml
instead of defining itself.

Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
---
 .../bindings/soc/mediatek/mediatek,ccorr.yaml        | 12 ++++--------
 .../bindings/soc/mediatek/mediatek,mutex.yaml        | 11 +++--------
 .../bindings/soc/mediatek/mediatek,wdma.yaml         | 12 ++++--------
 3 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
index 4380b98b0dfe..9e03de3c721b 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
+++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
@@ -34,13 +34,6 @@ properties:
       4 arguments defined in this property. Each GCE subsys id is mapping to
       a client defined in the header include/dt-bindings/gce/<chip>-gce.h.
 
-  mediatek,gce-events:
-    description:
-      The event id which is mapping to the specific hardware event signal
-      to gce. The event id is defined in the gce header
-      include/dt-bindings/gce/<chip>-gce.h of each chips.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
-
   clocks:
     minItems: 1
 
@@ -51,7 +44,10 @@ required:
   - mediatek,gce-events
   - clocks
 
-additionalProperties: false
+allOf:
+  - $ref: ../../mailbox/mediatek,gce-props.yaml#
+
+unevaluatedProperties: false
 
 examples:
   - |
diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
index ba2014a8725c..6c3dafe7f9a5 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
+++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
@@ -53,13 +53,6 @@ properties:
     items:
       - description: MUTEX Clock
 
-  mediatek,gce-events:
-    description:
-      The event id which is mapping to the specific hardware event signal
-      to gce. The event id is defined in the gce header
-      include/dt-bindings/gce/<chip>-gce.h of each chips.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
-
   mediatek,gce-client-reg:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     items:
@@ -73,6 +66,8 @@ properties:
       a client defined in the header include/dt-bindings/gce/<chip>-gce.h.
 
 allOf:
+  - $ref: ../../mailbox/mediatek,gce-props.yaml#
+
   - if:
       properties:
         compatible:
@@ -97,7 +92,7 @@ required:
   - interrupts
   - power-domains
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |
diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml
index 69afb329e5f4..a8d393784554 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml
+++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml
@@ -35,13 +35,6 @@ properties:
       4 arguments defined in this property. Each GCE subsys id is mapping to
       a client defined in the header include/dt-bindings/gce/<chip>-gce.h.
 
-  mediatek,gce-events:
-    description:
-      The event id which is mapping to the specific hardware event signal
-      to gce. The event id is defined in the gce header
-      include/dt-bindings/gce/<chip>-gce.h of each chips.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
-
   power-domains:
     maxItems: 1
 
@@ -60,7 +53,10 @@ required:
   - clocks
   - iommus
 
-additionalProperties: false
+allOf:
+  - $ref: ../../mailbox/mediatek,gce-props.yaml#
+
+unevaluatedProperties: false
 
 examples:
   - |
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel