[RESEND,3/5] soc: mediatek: mtk-cmdq: Add cmdq_pkt_poll_addr() function
Commit Message
Add cmdq_pkt_poll_addr function to support CMDQ user making
an instruction for polling a specific address of hardware rigster
to check the value with or without mask.
POLL is an old operation in GCE, so it does not support SPR and
CMDQ_CODE_LOGIC. CMDQ users need to use GPR and CMDQ_CODE_MASK
to move polling register address to GPR to make an instruction.
This will be done in cmdq_pkt_poll_addr().
Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
---
drivers/soc/mediatek/mtk-cmdq-helper.c | 38 ++++++++++++++++++++++++++
include/linux/soc/mediatek/mtk-cmdq.h | 16 +++++++++++
2 files changed, 54 insertions(+)
Comments
Hi, Jason:
On Fri, 2024-03-01 at 22:44 +0800, Jason-JH.Lin wrote:
> Add cmdq_pkt_poll_addr function to support CMDQ user making
> an instruction for polling a specific address of hardware rigster
> to check the value with or without mask.
>
> POLL is an old operation in GCE, so it does not support SPR and
> CMDQ_CODE_LOGIC. CMDQ users need to use GPR and CMDQ_CODE_MASK
> to move polling register address to GPR to make an instruction.
> This will be done in cmdq_pkt_poll_addr().
>
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> ---
> drivers/soc/mediatek/mtk-cmdq-helper.c | 38
> ++++++++++++++++++++++++++
> include/linux/soc/mediatek/mtk-cmdq.h | 16 +++++++++++
> 2 files changed, 54 insertions(+)
>
> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
> b/drivers/soc/mediatek/mtk-cmdq-helper.c
> index 3a1e47ad8a41..2e9fc9bb1183 100644
> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> @@ -12,6 +12,7 @@
>
> #define CMDQ_WRITE_ENABLE_MASK BIT(0)
> #define CMDQ_POLL_ENABLE_MASK BIT(0)
> +#define CMDQ_POLL_HIGH_ADDR_GPR (14)
I think there are multiple GPR and you use #14 to store high addr. I
would like you to list all GPR and do not limit the usage of each GPR.
The question is, why limit #14 to be high addr? If the GPR is shared by
all threads, there should be a mechanism to manage GPR usage for client
driver to allocate/free GPR.
> #define CMDQ_EOC_IRQ_EN BIT(0)
> #define CMDQ_REG_TYPE 1
> #define CMDQ_JUMP_RELATIVE 1
> @@ -406,6 +407,43 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8
> subsys,
> }
> EXPORT_SYMBOL(cmdq_pkt_poll_mask);
>
> +int cmdq_pkt_poll_addr(struct cmdq_pkt *pkt, dma_addr_t addr, u32
> value, u32 mask)
> +{
> + struct cmdq_instruction inst = { {0} };
> + int err;
> + u8 use_mask = 0;
> +
> + if (mask != U32_MAX) {
> + inst.op = CMDQ_CODE_MASK;
> + inst.mask = ~mask;
> + err = cmdq_pkt_append_command(pkt, inst);
> + if (err != 0)
> + return err;
> + use_mask = CMDQ_POLL_ENABLE_MASK;
> + }
> +
> + /*
> + * POLL is an old operation in GCE and it does not support SPR
> and CMDQ_CODE_LOGIC,
> + * so it can not use cmdq_pkt_assign to keep polling register
> address to SPR.
> + * It needs to use GPR and CMDQ_CODE_MASK to move polling
> register address to GPR.
> + */
> + inst.op = CMDQ_CODE_MASK;
> + inst.dst_t = CMDQ_REG_TYPE;
> + inst.sop = CMDQ_POLL_HIGH_ADDR_GPR;
> + inst.mask = addr;
This is full address, not high address. Why name the GPR to
CMDQ_POLL_HIGH_ADDR_GPR?
The addr is not mask, so I would like inst.value = addr.
Regards,
CK
> + err = cmdq_pkt_append_command(pkt, inst);
> + if (err < 0)
> + return err;
> +
> + inst.op = CMDQ_CODE_POLL;
> + inst.dst_t = CMDQ_REG_TYPE;
> + inst.sop = CMDQ_POLL_HIGH_ADDR_GPR;
> + inst.offset = use_mask;
> + inst.value = value;
> + return cmdq_pkt_append_command(pkt, inst);
> +}
> +EXPORT_SYMBOL(cmdq_pkt_poll_addr);
> +
> int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
> {
> struct cmdq_instruction inst = {};
> diff --git a/include/linux/soc/mediatek/mtk-cmdq.h
> b/include/linux/soc/mediatek/mtk-cmdq.h
> index b6dbe2d8f16a..2fe9be240fbc 100644
> --- a/include/linux/soc/mediatek/mtk-cmdq.h
> +++ b/include/linux/soc/mediatek/mtk-cmdq.h
> @@ -253,6 +253,22 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8
> subsys,
> int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
> u16 offset, u32 value, u32 mask);
>
> +/**
> + * cmdq_pkt_poll_addr() - Append polling command to the CMDQ packet,
> ask GCE to
> + * execute an instruction that wait for a
> specified
> + * address of hardware register to check for the
> value
> + * w/ or w/o mask.
> + * All GCE hardware threads will be blocked by
> this
> + * instruction.
> + * @pkt: the CMDQ packet
> + * @addr: the hardware register address
> + * @value: the specified target register value
> + * @mask: the specified target register mask
> + *
> + * Return: 0 for success; else the error code is returned
> + */
> +int cmdq_pkt_poll_addr(struct cmdq_pkt *pkt, dma_addr_t addr, u32
> value, u32 mask);
> +
> /**
> * cmdq_pkt_assign() - Append logic assign command to the CMDQ
> packet, ask GCE
> * to execute an instruction that set a constant
> value into
Hi CK,
Thanks for the reviews.
On Mon, 2024-03-04 at 03:11 +0000, CK Hu (胡俊光) wrote:
> Hi, Jason:
>
> On Fri, 2024-03-01 at 22:44 +0800, Jason-JH.Lin wrote:
> > Add cmdq_pkt_poll_addr function to support CMDQ user making
> > an instruction for polling a specific address of hardware rigster
> > to check the value with or without mask.
> >
> > POLL is an old operation in GCE, so it does not support SPR and
> > CMDQ_CODE_LOGIC. CMDQ users need to use GPR and CMDQ_CODE_MASK
> > to move polling register address to GPR to make an instruction.
> > This will be done in cmdq_pkt_poll_addr().
> >
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > ---
> > drivers/soc/mediatek/mtk-cmdq-helper.c | 38
> > ++++++++++++++++++++++++++
> > include/linux/soc/mediatek/mtk-cmdq.h | 16 +++++++++++
> > 2 files changed, 54 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index 3a1e47ad8a41..2e9fc9bb1183 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -12,6 +12,7 @@
> >
> > #define CMDQ_WRITE_ENABLE_MASK BIT(0)
> > #define CMDQ_POLL_ENABLE_MASK BIT(0)
> > +#define CMDQ_POLL_HIGH_ADDR_GPR (14)
>
> I think there are multiple GPR and you use #14 to store high addr. I
> would like you to list all GPR and do not limit the usage of each
> GPR.
> The question is, why limit #14 to be high addr? If the GPR is shared
> by
> all threads, there should be a mechanism to manage GPR usage for
> client
> driver to allocate/free GPR.
Yes, there are 16 GPR, from GPR_R0 ~ GPR_R15 and they are shared by all
threads, but GPR_R0 and GPR_R1 is used by GCE HW itself.
I think user may not know which GPR is available, so I think CMDQ
driver should manage the usage of GPR instead of configure by the user.
Currently, we only use 1 dedicated GPR in poll, so I defined it in CMDQ
driver to make it simpler.
>
> > #define CMDQ_EOC_IRQ_EN BIT(0)
> > #define CMDQ_REG_TYPE 1
> > #define CMDQ_JUMP_RELATIVE 1
> > @@ -406,6 +407,43 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt,
> > u8
> > subsys,
> > }
> > EXPORT_SYMBOL(cmdq_pkt_poll_mask);
> >
> > +int cmdq_pkt_poll_addr(struct cmdq_pkt *pkt, dma_addr_t addr, u32
> > value, u32 mask)
> > +{
> > + struct cmdq_instruction inst = { {0} };
> > + int err;
> > + u8 use_mask = 0;
> > +
> > + if (mask != U32_MAX) {
> > + inst.op = CMDQ_CODE_MASK;
> > + inst.mask = ~mask;
> > + err = cmdq_pkt_append_command(pkt, inst);
> > + if (err != 0)
> > + return err;
> > + use_mask = CMDQ_POLL_ENABLE_MASK;
> > + }
> > +
> > + /*
> > + * POLL is an old operation in GCE and it does not support SPR
> > and CMDQ_CODE_LOGIC,
> > + * so it can not use cmdq_pkt_assign to keep polling register
> > address to SPR.
> > + * It needs to use GPR and CMDQ_CODE_MASK to move polling
> > register address to GPR.
> > + */
> > + inst.op = CMDQ_CODE_MASK;
> > + inst.dst_t = CMDQ_REG_TYPE;
> > + inst.sop = CMDQ_POLL_HIGH_ADDR_GPR;
> > + inst.mask = addr;
>
> This is full address, not high address. Why name the GPR to
> CMDQ_POLL_HIGH_ADDR_GPR?
>
My mistake, I'll remove that 'HIGH' word.
> The addr is not mask, so I would like inst.value = addr.
OK, I'll change it.
Regards,
Jason-JH.Lin
>
> Regards,
> CK
>
@@ -12,6 +12,7 @@
#define CMDQ_WRITE_ENABLE_MASK BIT(0)
#define CMDQ_POLL_ENABLE_MASK BIT(0)
+#define CMDQ_POLL_HIGH_ADDR_GPR (14)
#define CMDQ_EOC_IRQ_EN BIT(0)
#define CMDQ_REG_TYPE 1
#define CMDQ_JUMP_RELATIVE 1
@@ -406,6 +407,43 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
}
EXPORT_SYMBOL(cmdq_pkt_poll_mask);
+int cmdq_pkt_poll_addr(struct cmdq_pkt *pkt, dma_addr_t addr, u32 value, u32 mask)
+{
+ struct cmdq_instruction inst = { {0} };
+ int err;
+ u8 use_mask = 0;
+
+ if (mask != U32_MAX) {
+ inst.op = CMDQ_CODE_MASK;
+ inst.mask = ~mask;
+ err = cmdq_pkt_append_command(pkt, inst);
+ if (err != 0)
+ return err;
+ use_mask = CMDQ_POLL_ENABLE_MASK;
+ }
+
+ /*
+ * POLL is an old operation in GCE and it does not support SPR and CMDQ_CODE_LOGIC,
+ * so it can not use cmdq_pkt_assign to keep polling register address to SPR.
+ * It needs to use GPR and CMDQ_CODE_MASK to move polling register address to GPR.
+ */
+ inst.op = CMDQ_CODE_MASK;
+ inst.dst_t = CMDQ_REG_TYPE;
+ inst.sop = CMDQ_POLL_HIGH_ADDR_GPR;
+ inst.mask = addr;
+ err = cmdq_pkt_append_command(pkt, inst);
+ if (err < 0)
+ return err;
+
+ inst.op = CMDQ_CODE_POLL;
+ inst.dst_t = CMDQ_REG_TYPE;
+ inst.sop = CMDQ_POLL_HIGH_ADDR_GPR;
+ inst.offset = use_mask;
+ inst.value = value;
+ return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_poll_addr);
+
int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value)
{
struct cmdq_instruction inst = {};
@@ -253,6 +253,22 @@ int cmdq_pkt_poll(struct cmdq_pkt *pkt, u8 subsys,
int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
+/**
+ * cmdq_pkt_poll_addr() - Append polling command to the CMDQ packet, ask GCE to
+ * execute an instruction that wait for a specified
+ * address of hardware register to check for the value
+ * w/ or w/o mask.
+ * All GCE hardware threads will be blocked by this
+ * instruction.
+ * @pkt: the CMDQ packet
+ * @addr: the hardware register address
+ * @value: the specified target register value
+ * @mask: the specified target register mask
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_poll_addr(struct cmdq_pkt *pkt, dma_addr_t addr, u32 value, u32 mask);
+
/**
* cmdq_pkt_assign() - Append logic assign command to the CMDQ packet, ask GCE
* to execute an instruction that set a constant value into