<html><body><p> <pre> On Mon, 2024-06-24 at 13:39 +0200, AngeloGioacchino Del Regno wrote: > Il 28/05/24 17:52, Jason-JH Lin (林睿祥) ha scritto: > > Hi Angelo, > > > > On Tue, 2024-05-28 at 12:24 +0200, AngeloGioacchino Del Regno > > wrote: > > > Il 26/05/24 01:08, Jason-JH.Lin ha scritto: > > > > Add cmdq_pkt_logic_command to support math operation. > > > > > > > > cmdq_pkt_logic_command can append logic command to the CMDQ > > > > packet, > > > > ask GCE to execute a arithmetic calculate instruction, > > > > such as add, subtract, multiply, AND, OR and NOT, etc. > > > > > > > > Note that all arithmetic instructions are unsigned > > > > calculations. > > > > If there are any overflows, GCE will sent the invalid IRQ to > > > > notify > > > > CMDQ driver. > > > > > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > > > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com> > > > > --- > > > > drivers/soc/mediatek/mtk-cmdq-helper.c | 36 > > > > ++++++++++++++++++++++ > > > > include/linux/soc/mediatek/mtk-cmdq.h | 42 > > > > ++++++++++++++++++++++++++ > > > > 2 files changed, 78 insertions(+) > > > > > > > > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c > > > > b/drivers/soc/mediatek/mtk-cmdq-helper.c > > > > index 046522664dc1..42fae05f61a8 100644 > > > > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c > > > > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c > > > > @@ -15,10 +15,19 @@ > > > > /* dedicate the last GPR_R15 to assign the register address > > > > to be > > > > poll */ > > > > #define CMDQ_POLL_ADDR_GPR(15) > > > > #define CMDQ_EOC_IRQ_ENBIT(0) > > > > +#define CMDQ_IMMEDIATE_VALUE0 > > > > #define CMDQ_REG_TYPE1 > > > > #define CMDQ_JUMP_RELATIVE0 > > > > #define CMDQ_JUMP_ABSOLUTE1 > > > > > > > > +#define CMDQ_OPERAND_GET_IDX_VALUE(operand) \ > > > > +({ \ > > > > +struct cmdq_operand *op = operand; \ > > > > +op->reg ? op->idx : op->value; \ > > > > +}) > > > > > > I think this CMDQ_OPERAND_GET_IDX_VALUE would be better expressed > > > as > > > a (inline?) > > > function instead... > > > > > > > Yes, I can change them to static inline function to pass their type > > directly. > > > > > static inline u16 cmdq_operand_get_idx_value(struct cmdq_operand > > > *op) > > > { > > > return op->reg ? op->idx : op->value; > > > } > > > > > > and maybe the same for the other definition too.. > > > > > > static inline u16 cmdq_operand_type(struct cmdq_operand *op) > > > { > > > return op->reg ? CMDQ_REG_TYPE : CMDQ_IMMEDIATE_VALUE; > > > } > > > > > > but definitely the first one is what I don't like much, the > > > second > > > one can pass. > > > > > > > You mean '#define CMDQ_OPERAND_GET_IDX_VALUE()' is the first one or > > 'static inline u16 cmdq_operand_get_idx_value()' is the first one? > > > > I'm sorry, your reply slipped through for some reason and I've read > it just now. > > I mean I don't like the macros (#define > CMDQ_OPERAND_GET_IDX_VALUE(..) and > CMDQ_OPERAND_TYPE(..)) :-) > > Please use static inline functions and resend; if you can do that > this week, I > can pick the commit before I send the PR. > > P.S.: And you're right, re-reading my own reply, it was a bit > ambiguous, so > I'm again sorry for that, will be clearer next time. > No problem! Since other patches in this series are still cooking, I'll resend this patch separately. Thanks! Regards, Jason-JH.Lin > Thanks, > Angelo > > > Regards, > > Jason-JH.Lin > > > > > Cheers, > > > Angelo > > > > > > > +#define CMDQ_OPERAND_TYPE(operand) \ > > > > +((operand)->reg ? CMDQ_REG_TYPE : CMDQ_IMMEDIATE_VALUE) > > > > + > > > > struct cmdq_instruction { > > > > union { > > > > u32 value; > > > > @@ -461,6 +470,33 @@ int cmdq_pkt_poll_addr(struct cmdq_pkt > > > > *pkt, > > > > dma_addr_t addr, u32 value, u32 mas > > > > } > > > > EXPORT_SYMBOL(cmdq_pkt_poll_addr); > > > > > > > > +int cmdq_pkt_logic_command(struct cmdq_pkt *pkt, u16 > > > > result_reg_idx, > > > > + struct cmdq_operand *left_operand, > > > > + enum cmdq_logic_op s_op, > > > > + struct cmdq_operand *right_operand) > > > > +{ > > > > +struct cmdq_instruction inst = { {0} }; > > > > +u32 left_idx_value; > > > > +u32 right_idx_value; > > > > + > > > > +if (!left_operand || !right_operand || s_op >= > > > > CMDQ_LOGIC_MAX) > > > > +return -EINVAL; > > > > + > > > > +left_idx_value = > > > > CMDQ_OPERAND_GET_IDX_VALUE(left_operand); > > > > +right_idx_value = > > > > CMDQ_OPERAND_GET_IDX_VALUE(right_operand); > > > > +inst.op = CMDQ_CODE_LOGIC; > > > > +inst.dst_t = CMDQ_REG_TYPE; > > > > +inst.src_t = CMDQ_OPERAND_TYPE(left_operand); > > > > +inst.arg_c_t = CMDQ_OPERAND_TYPE(right_operand); > > > > +inst.sop = s_op; > > > > +inst.reg_dst = result_reg_idx; > > > > +inst.src_reg = left_idx_value; > > > > +inst.arg_c = right_idx_value; > > > > + > > > > +return cmdq_pkt_append_command(pkt, inst); > > > > +} > > > > +EXPORT_SYMBOL(cmdq_pkt_logic_command); > > > > + > > > > 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 d4a8e34505e6..5bee6f7fc400 100644 > > > > --- a/include/linux/soc/mediatek/mtk-cmdq.h > > > > +++ b/include/linux/soc/mediatek/mtk-cmdq.h > > > > @@ -25,6 +25,31 @@ > > > > > > > > struct cmdq_pkt; > > > > > > > > +enum cmdq_logic_op { > > > > +CMDQ_LOGIC_ASSIGN = 0, > > > > +CMDQ_LOGIC_ADD = 1, > > > > +CMDQ_LOGIC_SUBTRACT = 2, > > > > +CMDQ_LOGIC_MULTIPLY = 3, > > > > +CMDQ_LOGIC_XOR = 8, > > > > +CMDQ_LOGIC_NOT = 9, > > > > +CMDQ_LOGIC_OR = 10, > > > > +CMDQ_LOGIC_AND = 11, > > > > +CMDQ_LOGIC_LEFT_SHIFT = 12, > > > > +CMDQ_LOGIC_RIGHT_SHIFT = 13, > > > > +CMDQ_LOGIC_MAX, > > > > +}; > > > > + > > > > +struct cmdq_operand { > > > > +/* register type */ > > > > +bool reg; > > > > +union { > > > > +/* index */ > > > > +u16 idx; > > > > +/* value */ > > > > +u16 value; > > > > +}; > > > > +}; > > > > + > > > > struct cmdq_client_reg { > > > > u8 subsys; > > > > u16 offset; > > > > @@ -272,6 +297,23 @@ 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_logic_command() - Append logic command to the CMDQ > > > > packet, ask GCE to > > > > + * execute an instruction that store the > > > > result > > > > of logic operation > > > > + * with left and right operand into > > > > result_reg_idx. > > > > + * @pkt:the CMDQ packet > > > > + * @result_reg_idx:SPR index that store operation result > > > > of left_operand and right_operand > > > > + * @left_operand:left operand > > > > + * @s_op:the logic operator enum > > > > + * @right_operand:right operand > > > > + * > > > > + * Return: 0 for success; else the error code is returned > > > > + */ > > > > +int cmdq_pkt_logic_command(struct cmdq_pkt *pkt, u16 > > > > result_reg_idx, > > > > + struct cmdq_operand *left_operand, > > > > + enum cmdq_logic_op s_op, > > > > + struct cmdq_operand *right_operand); > > > > + > > > > /** > > > > * cmdq_pkt_assign() - Append logic assign command to the > > > > CMDQ > > > > packet, ask GCE > > > > * to execute an instruction that set a > > > > constant value into > > > > > > > > > > > > > > </pre> </p></body></html><pre>************* MEDIATEK Confidentiality Notice ******************** The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you! </pre>