| Message ID | 20240301111126.22035-4-jason-jh.lin@mediatek.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add CMDQ API for upcoming ISP feature | expand |
Il 01/03/24 12:11, Jason-JH.Lin ha scritto: > 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 POLL is a legacy operation in GCE, so it does not support ..... > 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> > Change-Id: I91ff98e06570dc4501187eb29de64aaa65b1cf13 > --- > 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) > #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; /* Describe what you're doing here as a comment please */ if (mask < GENMASK(31, 0)) { inst.mask = ~mask; err = cmdq_pkt_append_command ... use_mask ... } /* POLL is a legacy operation in GCE and ... etc etc */ inst.mask = addr; inst.dst_t = ... inst.sop = ... err = cmdq_pkt_append... etc etc etc > + 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; This is a reassignment of the same value. You can do it like /* dst_t and sop must be CMDQ_REG_TYPE, CMDQ_POLL_HIGH_ADDR_GPR */ inst.op = CMDQ_CODE_POLL; inst.offset ... inst.value .... (but you're also inheriting the same `inst.mask`, was that intended?) err = cmdq_pkt_append .... if (err < 0) return err; return 0 } > + 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. /** * cmdq_pkt_poll_addr() - Append blocking POLL command to CMDQ packet * @pkt: the CMDQ packet * @addr: the hardware register address * @value: the specified target register value * @mask: the specified target register mask * * Appends a polling (POLL) command to the CMDQ packet and asks the GCE * to execute an instruction that checks for the specified `value` (with * or without `mask`) to appear in the specified hardware register `addr`. * All GCE threads will be blocked by this instruction. * * Return: 0 for success or negative error code */ That's better, isn't it? :-) Cheers, Angelo
Hi Angelo, Thanks for the reviews. On Mon, 2024-03-04 at 11:05 +0100, AngeloGioacchino Del Regno wrote: > Il 01/03/24 12:11, Jason-JH.Lin ha scritto: > > 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 > > POLL is a legacy operation in GCE, so it does not support ..... > OK, I'll change that. > > 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> > > Change-Id: I91ff98e06570dc4501187eb29de64aaa65b1cf13 > > --- > > 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) > > #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; > > /* Describe what you're doing here as a comment please */ > if (mask < GENMASK(31, 0)) { > inst.mask = ~mask; > err = cmdq_pkt_append_command ... > use_mask ... > } OK, I'll change the U32_MAX to GENMASK(31, 0). > > /* POLL is a legacy operation in GCE and ... etc etc */ > inst.mask = addr; > inst.dst_t = ... > inst.sop = ... > err = cmdq_pkt_append... etc etc etc > OK, I'll change like this and I'll use inst.value = addr; because addr is not mask. > > > + 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; > > This is a reassignment of the same value. You can do it like > > /* dst_t and sop must be CMDQ_REG_TYPE, CMDQ_POLL_HIGH_ADDR_GPR */ > inst.op = CMDQ_CODE_POLL; > inst.offset ... > inst.value .... > inst is used to generate an instruction for GCE, so it will be reassigned every time after calling cmdq_pkt_append_command(pkt, inst); to append a generated inst into the cmd buffer. > (but you're also inheriting the same `inst.mask`, was that intended?) > The definition of cmdq_instruction is: struct cmdq_instruction { union { u32 value; u32 mask; struct { u16 arg_c; u16 src_reg; }; }; union { u16 offset; u16 event; u16 reg_dst; }; ... }; so inst.mask and inst.value are the same u32 in inst. That means inst.mask has been reassigned as the inst.value here. > err = cmdq_pkt_append .... > if (err < 0) > return err; > > return 0 > } OK, I'll change it to return 0. > > > + 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. > > /** > * cmdq_pkt_poll_addr() - Append blocking POLL command to CMDQ > packet > * @pkt: the CMDQ packet > * @addr: the hardware register address > * @value: the specified target register value > * @mask: the specified target register mask > * > * Appends a polling (POLL) command to the CMDQ packet and asks the > GCE > * to execute an instruction that checks for the specified `value` > (with > * or without `mask`) to appear in the specified hardware register > `addr`. > * All GCE threads will be blocked by this instruction. > * > * Return: 0 for success or negative error code > */ > > That's better, isn't it? :-) > OK, I'll change that. Thanks for making the entire sample. Regards, Jason-JH.Lin > Cheers, > Angelo > >
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) #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 = {}; 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
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> Change-Id: I91ff98e06570dc4501187eb29de64aaa65b1cf13 --- drivers/soc/mediatek/mtk-cmdq-helper.c | 38 ++++++++++++++++++++++++++ include/linux/soc/mediatek/mtk-cmdq.h | 16 +++++++++++ 2 files changed, 54 insertions(+)