[PATCH v7 09/20] soc: mediatek: mtk-cmdq: Add pa_base parsing for hardware without subsys ID support

Jason-JH Lin posted 20 patches 5 months, 2 weeks ago
There is a newer version of this series
[PATCH v7 09/20] soc: mediatek: mtk-cmdq: Add pa_base parsing for hardware without subsys ID support
Posted by Jason-JH Lin 5 months, 2 weeks ago
When GCE executes instructions, it typically locates the corresponding
hardware register using the subsys ID. For hardware that does not
support subsys ID, the subsys ID is set to an invalid value, and the
physical address must be used to generate GCE instructions.

The main advantage of using subsys ID is to reduce the number of
instructions. Without subsys ID, an additional `ASSIGN` instruction
is needed to assign the high bytes of the physical address, which can
impact performance if too many instructions are required. However, if
the hardware does not support subsys ID, using the physical address
is the only option to achieve the same functionality.

This commit adds a pa_base parsing flow to the cmdq_client_reg structure
to handle hardware without subsys ID support.

Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
---
 drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++--
 include/linux/soc/mediatek/mtk-cmdq.h  |  3 +++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 4b1591e5b1ae..41e1997cdd53 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -8,6 +8,7 @@
 #include <linux/module.h>
 #include <linux/mailbox_controller.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/soc/mediatek/mtk-cmdq.h>
 
 #define CMDQ_WRITE_ENABLE_MASK	BIT(0)
@@ -60,20 +61,30 @@ int cmdq_dev_get_client_reg(struct device *dev,
 			    struct cmdq_client_reg *client_reg, int idx)
 {
 	struct of_phandle_args spec;
+	struct resource res;
 	int err;
 
 	if (!client_reg)
 		return -ENOENT;
 
+	if (of_address_to_resource(dev->of_node, 0, &res) != 0) {
+		dev_err(dev, "Missing reg in %s node\n", dev->of_node->full_name);
+		return -EINVAL;
+	}
+	client_reg->pa_base = res.start;
+
 	err = of_parse_phandle_with_fixed_args(dev->of_node,
 					       "mediatek,gce-client-reg",
 					       3, idx, &spec);
 	if (err < 0) {
-		dev_warn(dev,
+		dev_dbg(dev,
 			"error %d can't parse gce-client-reg property (%d)",
 			err, idx);
 
-		return err;
+		/* make subsys invalid */
+		client_reg->subsys = CMDQ_SUBSYS_INVALID;
+
+		return 0;
 	}
 
 	client_reg->subsys = (u8)spec.args[0];
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
index 5e3a0e807980..3699229a7375 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -23,6 +23,8 @@
 #define CMDQ_THR_SPR_IDX2	(2)
 #define CMDQ_THR_SPR_IDX3	(3)
 
+#define CMDQ_SUBSYS_INVALID	(U8_MAX)
+
 struct cmdq_pkt;
 
 enum cmdq_logic_op {
@@ -52,6 +54,7 @@ struct cmdq_operand {
 
 struct cmdq_client_reg {
 	u8 subsys;
+	phys_addr_t pa_base;
 	u16 offset;
 	u16 size;
 };
-- 
2.43.0
Re: [PATCH v7 09/20] soc: mediatek: mtk-cmdq: Add pa_base parsing for hardware without subsys ID support
Posted by AngeloGioacchino Del Regno 4 months ago
Il 27/08/25 13:37, Jason-JH Lin ha scritto:
> When GCE executes instructions, it typically locates the corresponding
> hardware register using the subsys ID. For hardware that does not
> support subsys ID, the subsys ID is set to an invalid value, and the
> physical address must be used to generate GCE instructions.
> 
> The main advantage of using subsys ID is to reduce the number of
> instructions. Without subsys ID, an additional `ASSIGN` instruction
> is needed to assign the high bytes of the physical address, which can
> impact performance if too many instructions are required. However, if
> the hardware does not support subsys ID, using the physical address
> is the only option to achieve the same functionality.
> 
> This commit adds a pa_base parsing flow to the cmdq_client_reg structure
> to handle hardware without subsys ID support.
> 
> Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> ---
>   drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++--
>   include/linux/soc/mediatek/mtk-cmdq.h  |  3 +++
>   2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 4b1591e5b1ae..41e1997cdd53 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -8,6 +8,7 @@
>   #include <linux/module.h>
>   #include <linux/mailbox_controller.h>
>   #include <linux/of.h>
> +#include <linux/of_address.h>
>   #include <linux/soc/mediatek/mtk-cmdq.h>
>   
>   #define CMDQ_WRITE_ENABLE_MASK	BIT(0)
> @@ -60,20 +61,30 @@ int cmdq_dev_get_client_reg(struct device *dev,
>   			    struct cmdq_client_reg *client_reg, int idx)
>   {
>   	struct of_phandle_args spec;
> +	struct resource res;
>   	int err;
>   
>   	if (!client_reg)
>   		return -ENOENT;
>   

	err = of_address_to_resource( ... )
	if (err) {
		dev_err(....)
		return;
	}

after which:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> +	if (of_address_to_resource(dev->of_node, 0, &res) != 0) {
> +		dev_err(dev, "Missing reg in %s node\n", dev->of_node->full_name);
> +		return -EINVAL;
> +	}
> +	client_reg->pa_base = res.start;
> +
>   	err = of_parse_phandle_with_fixed_args(dev->of_node,
>   					       "mediatek,gce-client-reg",
>   					       3, idx, &spec);
>   	if (err < 0) {
> -		dev_warn(dev,
> +		dev_dbg(dev,
>   			"error %d can't parse gce-client-reg property (%d)",
>   			err, idx);
>   
> -		return err;
> +		/* make subsys invalid */
> +		client_reg->subsys = CMDQ_SUBSYS_INVALID;
> +
> +		return 0;
>   	}
>   
>   	client_reg->subsys = (u8)spec.args[0];
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
> index 5e3a0e807980..3699229a7375 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -23,6 +23,8 @@
>   #define CMDQ_THR_SPR_IDX2	(2)
>   #define CMDQ_THR_SPR_IDX3	(3)
>   
> +#define CMDQ_SUBSYS_INVALID	(U8_MAX)
> +
>   struct cmdq_pkt;
>   
>   enum cmdq_logic_op {
> @@ -52,6 +54,7 @@ struct cmdq_operand {
>   
>   struct cmdq_client_reg {
>   	u8 subsys;
> +	phys_addr_t pa_base;
>   	u16 offset;
>   	u16 size;
>   };
Re: [PATCH v7 09/20] soc: mediatek: mtk-cmdq: Add pa_base parsing for hardware without subsys ID support
Posted by Jason-JH Lin (林睿祥) 3 months, 4 weeks ago
On Thu, 2025-10-09 at 13:35 +0200, AngeloGioacchino Del Regno wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 27/08/25 13:37, Jason-JH Lin ha scritto:
> > When GCE executes instructions, it typically locates the
> > corresponding
> > hardware register using the subsys ID. For hardware that does not
> > support subsys ID, the subsys ID is set to an invalid value, and
> > the
> > physical address must be used to generate GCE instructions.
> > 
> > The main advantage of using subsys ID is to reduce the number of
> > instructions. Without subsys ID, an additional `ASSIGN` instruction
> > is needed to assign the high bytes of the physical address, which
> > can
> > impact performance if too many instructions are required. However,
> > if
> > the hardware does not support subsys ID, using the physical address
> > is the only option to achieve the same functionality.
> > 
> > This commit adds a pa_base parsing flow to the cmdq_client_reg
> > structure
> > to handle hardware without subsys ID support.
> > 
> > Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> > ---
> >   drivers/soc/mediatek/mtk-cmdq-helper.c | 15 +++++++++++++--
> >   include/linux/soc/mediatek/mtk-cmdq.h  |  3 +++
> >   2 files changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 4b1591e5b1ae..41e1997cdd53 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -8,6 +8,7 @@
> >   #include <linux/module.h>
> >   #include <linux/mailbox_controller.h>
> >   #include <linux/of.h>
> > +#include <linux/of_address.h>
> >   #include <linux/soc/mediatek/mtk-cmdq.h>
> > 
> >   #define CMDQ_WRITE_ENABLE_MASK      BIT(0)
> > @@ -60,20 +61,30 @@ int cmdq_dev_get_client_reg(struct device *dev,
> >                           struct cmdq_client_reg *client_reg, int
> > idx)
> >   {
> >       struct of_phandle_args spec;
> > +     struct resource res;
> >       int err;
> > 
> >       if (!client_reg)
> >               return -ENOENT;
> > 
> 
>         err = of_address_to_resource( ... )
>         if (err) {
>                 dev_err(....)
>                 return;
>         }
> 
> after which:
> 
> Reviewed-by: AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com>
> 
> > +     if (of_address_to_resource(dev->of_node, 0, &res) != 0) {
> > +             dev_err(dev, "Missing reg in %s node\n", dev-
> > >of_node->full_name);
> > +             return -EINVAL;
> > +     }

OK, I'll refine this.
Thanks for the reviews.

Regards,
Jason-JH Lin

> > +     client_reg->pa_base = res.start;
> > +
> >       err = of_parse_phandle_with_fixed_args(dev->of_node,
> >                                              "mediatek,gce-client-
> > reg",
> >                                              3, idx, &spec);
> >       if (err < 0) {
> > -             dev_warn(dev,
> > +             dev_dbg(dev,
> >                       "error %d can't parse gce-client-reg property
> > (%d)",
> >                       err, idx);
> > 
> > -             return err;
> > +             /* make subsys invalid */
> > +             client_reg->subsys = CMDQ_SUBSYS_INVALID;
> > +
> > +             return 0;
> >       }
> > 
> >       client_reg->subsys = (u8)spec.args[0];
> > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
> > b/include/linux/soc/mediatek/mtk-cmdq.h
> > index 5e3a0e807980..3699229a7375 100644
> > --- a/include/linux/soc/mediatek/mtk-cmdq.h
> > +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> > @@ -23,6 +23,8 @@
> >   #define CMDQ_THR_SPR_IDX2   (2)
> >   #define CMDQ_THR_SPR_IDX3   (3)
> > 
> > +#define CMDQ_SUBSYS_INVALID  (U8_MAX)
> > +
> >   struct cmdq_pkt;
> > 
> >   enum cmdq_logic_op {
> > @@ -52,6 +54,7 @@ struct cmdq_operand {
> > 
> >   struct cmdq_client_reg {
> >       u8 subsys;
> > +     phys_addr_t pa_base;
> >       u16 offset;
> >       u16 size;
> >   };
> 
>