From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED, USER_AGENT_SANE_2 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 52A60C433FF for ; Sat, 10 Aug 2019 16:12:23 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2746E208C3 for ; Sat, 10 Aug 2019 16:12:22 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="hArSYmWt" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2746E208C3 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=BQbB2Iyy5+Zvy+bDnyavRw+mvdoEaYN6Q+FDIc9Fwxc=; b=hArSYmWthh+9lw lAmmg8dQ+ZOTI66wcZkGPGu/jdzx+n5a5M1z0RvM1N9jCPNHJe3E9KixAv1C3j98WF5SMwMPC9RJb dWt+5zVyWOk5T6Sa7dzIk8jbq7CJLQ4TISOup+pLzPJ8kPujPs/9YRkH4XN6c4uSeSyjYl4s+7Q7V 41H3icrFEm+KuLv3q7ub0wRrawCDkYmWuhxrfcx+6KCzXKe4aWOgc09mmJSBXzmnPJ44n3jVU6e4U MPKpAGc+Zk2a7IpMf/7x2ylWFQSBizYWZeAH7De5wF/GofMdgOpVRVOkiKlDa0rhbvcux+Gs5sMTk Xo7klgw/mKDyprTyMHKQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hwTyd-0006BL-LK; Sat, 10 Aug 2019 16:12:19 +0000 Received: from mailgw01.mediatek.com ([216.200.240.184]) by bombadil.infradead.org with esmtps (Exim 4.92 #3 (Red Hat Linux)) id 1hwTyY-0006Az-UR; Sat, 10 Aug 2019 16:12:17 +0000 X-UUID: d421b4819a79495e8f19d805aefd21df-20190810 X-UUID: d421b4819a79495e8f19d805aefd21df-20190810 Received: from mtkcas67.mediatek.inc [(172.29.193.45)] by mailgw01.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLS) with ESMTP id 1400512989; Sat, 10 Aug 2019 08:12:06 -0800 Received: from MTKMBS31N2.mediatek.inc (172.27.4.87) by MTKMBS62N2.mediatek.inc (172.29.193.42) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Sat, 10 Aug 2019 09:12:04 -0700 Received: from MTKCAS32.mediatek.inc (172.27.4.184) by MTKMBS31N2.mediatek.inc (172.27.4.87) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Sun, 11 Aug 2019 00:12:01 +0800 Received: from [10.17.3.153] (172.27.4.253) by MTKCAS32.mediatek.inc (172.27.4.170) with Microsoft SMTP Server id 15.0.1395.4 via Frontend Transport; Sun, 11 Aug 2019 00:12:00 +0800 Message-ID: <1565453520.19079.17.camel@mhfsdcap03> Subject: Re: [PATCH v11 09/12] soc: mediatek: cmdq: define the instruction struct From: houlong wei To: Bibby Hsieh Date: Sun, 11 Aug 2019 00:12:00 +0800 In-Reply-To: <20190729070106.9332-10-bibby.hsieh@mediatek.com> References: <20190729070106.9332-1-bibby.hsieh@mediatek.com> <20190729070106.9332-10-bibby.hsieh@mediatek.com> X-Mailer: Evolution 3.10.4-0ubuntu2 MIME-Version: 1.0 X-TM-SNTS-SMTP: 36B40D599FEE99665428D035A4900559F5A4021DCEB85292EBDDD62009BA69AC2000:8 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190810_091214_988576_4B971922 X-CRM114-Status: GOOD ( 17.61 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "devicetree@vger.kernel.org" , Nicolas Boichat , Philipp Zabel , srv_heupstream , Daoyuan Huang =?UTF-8?Q?=28=E9=BB=83=E9=81=93=E5=8E=9F=29?= , Sascha Hauer , Jassi Brar , "linux-kernel@vger.kernel.org" , Daniel Kurtz , houlong.wei@mediatek.com, YT Shen =?UTF-8?Q?=28=E6=B2=88=E5=B2=B3=E9=9C=86=29?= , CK Hu =?UTF-8?Q?=28=E8=83=A1=E4=BF=8A=E5=85=89=29?= , Rob Herring , "linux-mediatek@lists.infradead.org" , Ginny Chen =?UTF-8?Q?=28=E9=99=B3=E6=B2=BB=E5=82=91=29?= , Sascha Hauer , Matthias Brugger , Jiaguang Zhang =?UTF-8?Q?=28=E5=BC=A0=E5=8A=A0=E5=B9=BF=29?= , "linux-arm-kernel@lists.infradead.org" , Dennis-YC Hsieh =?UTF-8?Q?=28=E8=AC=9D=E5=AE=87=E5=93=B2=29?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Bibby, I have inline comment in function cmdq_pkt_write_mask(). On Mon, 2019-07-29 at 15:01 +0800, Bibby Hsieh wrote: > Define an instruction structure for gce driver to append command. > This structure can make the client's code more readability. > > Signed-off-by: Bibby Hsieh > Reviewed-by: CK Hu > --- > drivers/soc/mediatek/mtk-cmdq-helper.c | 103 +++++++++++++++-------- > include/linux/mailbox/mtk-cmdq-mailbox.h | 2 + > 2 files changed, 72 insertions(+), 33 deletions(-) > > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c > index 7aa0517ff2f3..0886c4967ca4 100644 > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c > @@ -9,12 +9,24 @@ > #include > #include > > -#define CMDQ_ARG_A_WRITE_MASK 0xffff > #define CMDQ_WRITE_ENABLE_MASK BIT(0) > #define CMDQ_EOC_IRQ_EN BIT(0) > #define CMDQ_EOC_CMD ((u64)((CMDQ_CODE_EOC << CMDQ_OP_CODE_SHIFT)) \ > << 32 | CMDQ_EOC_IRQ_EN) > > +struct cmdq_instruction { > + union { > + u32 value; > + u32 mask; > + }; > + union { > + u16 offset; > + u16 event; > + }; > + u8 subsys; > + u8 op; > +}; > + > static void cmdq_client_timeout(struct timer_list *t) > { > struct cmdq_client *client = from_timer(client, t, timer); > @@ -110,10 +122,8 @@ void cmdq_pkt_destroy(struct cmdq_pkt *pkt) > } > EXPORT_SYMBOL(cmdq_pkt_destroy); > > -static int cmdq_pkt_append_command(struct cmdq_pkt *pkt, enum cmdq_code code, > - u32 arg_a, u32 arg_b) > +static struct cmdq_instruction *cmdq_pkt_append_command(struct cmdq_pkt *pkt) > { > - u64 *cmd_ptr; > > if (unlikely(pkt->cmd_buf_size + CMDQ_INST_SIZE > pkt->buf_size)) { > /* > @@ -127,81 +137,108 @@ static int cmdq_pkt_append_command(struct cmdq_pkt *pkt, enum cmdq_code code, > pkt->cmd_buf_size += CMDQ_INST_SIZE; > WARN_ONCE(1, "%s: buffer size %u is too small !\n", > __func__, (u32)pkt->buf_size); > - return -ENOMEM; > + return NULL; > } > - cmd_ptr = pkt->va_base + pkt->cmd_buf_size; > - (*cmd_ptr) = (u64)((code << CMDQ_OP_CODE_SHIFT) | arg_a) << 32 | arg_b; > + > pkt->cmd_buf_size += CMDQ_INST_SIZE; > > - return 0; > + return pkt->va_base + pkt->cmd_buf_size - CMDQ_INST_SIZE; > } > > int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset, u32 value) > { > - u32 arg_a = (offset & CMDQ_ARG_A_WRITE_MASK) | > - (subsys << CMDQ_SUBSYS_SHIFT); > + struct cmdq_instruction *inst; > + > + inst = cmdq_pkt_append_command(pkt); > + if (!inst) > + return -ENOMEM; > + > + inst->op = CMDQ_CODE_WRITE; > + inst->value = value; > + inst->offset = offset; > + inst->subsys = subsys; > > - return cmdq_pkt_append_command(pkt, CMDQ_CODE_WRITE, arg_a, value); > + return 0; > } > EXPORT_SYMBOL(cmdq_pkt_write); > > int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys, > u16 offset, u32 value, u32 mask) > { > + struct cmdq_instruction *inst; > u32 offset_mask = offset; > - int err = 0; > > if (mask != 0xffffffff) { > - err = cmdq_pkt_append_command(pkt, CMDQ_CODE_MASK, 0, ~mask); > + inst = cmdq_pkt_append_command(pkt); > + if (!inst) > + return -ENOMEM; > + > + inst->op = CMDQ_CODE_MASK; > + inst->mask = ~mask; There were some discussion about how to reduce cmdq buffer allocation times reuse a cmdq packet.Please refer to the below links. https://patchwork.kernel.org/patch/10193349/ https://patchwork.kernel.org/patch/10491161/ If we reuse a cmdq packet, we get the cmdq instruction buffer which may contains previous data. To generate correct instructions, we may need consider how to clear the previous data. 1.Set all members of a cmdq_instruction instance. e.g. Add two lines of code below for this case. inst->offset = 0; inst->subsys = 0; 2. Before reuse a packet, do memset() for the command buffer. How do you think about it? > offset_mask |= CMDQ_WRITE_ENABLE_MASK; > } > - err |= cmdq_pkt_write(pkt, value, subsys, offset_mask); > > - return err; > + return cmdq_pkt_write(pkt, subsys, offset_mask, value); > } > EXPORT_SYMBOL(cmdq_pkt_write_mask); > > int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event) > { > - u32 arg_b; > + struct cmdq_instruction *inst; > > if (event >= CMDQ_MAX_EVENT) > return -EINVAL; > > - /* > - * WFE arg_b > - * bit 0-11: wait value > - * bit 15: 1 - wait, 0 - no wait > - * bit 16-27: update value > - * bit 31: 1 - update, 0 - no update > - */ > - arg_b = CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE; > + inst = cmdq_pkt_append_command(pkt); > + if (!inst) > + return -ENOMEM; > + > + inst->op = CMDQ_CODE_WFE; > + inst->value = CMDQ_WFE_OPTION; > + inst->event = event; > > - return cmdq_pkt_append_command(pkt, CMDQ_CODE_WFE, event, arg_b); > + return 0; > } > EXPORT_SYMBOL(cmdq_pkt_wfe); > > int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, u16 event) > { > + struct cmdq_instruction *inst; > + > if (event >= CMDQ_MAX_EVENT) > return -EINVAL; > > - return cmdq_pkt_append_command(pkt, CMDQ_CODE_WFE, event, > - CMDQ_WFE_UPDATE); > + inst = cmdq_pkt_append_command(pkt); > + if (!inst) > + return -ENOMEM; > + > + inst->op = CMDQ_CODE_WFE; > + inst->value = CMDQ_WFE_UPDATE; > + inst->event = event; > + > + return 0; > } > EXPORT_SYMBOL(cmdq_pkt_clear_event); > > static int cmdq_pkt_finalize(struct cmdq_pkt *pkt) > { > - int err; > + struct cmdq_instruction *inst; > + > + inst = cmdq_pkt_append_command(pkt); > + if (!inst) > + return -ENOMEM; > > - /* insert EOC and generate IRQ for each command iteration */ > - err = cmdq_pkt_append_command(pkt, CMDQ_CODE_EOC, 0, CMDQ_EOC_IRQ_EN); > + inst->op = CMDQ_CODE_EOC; > + inst->value = CMDQ_EOC_IRQ_EN; > > - /* JUMP to end */ > - err |= cmdq_pkt_append_command(pkt, CMDQ_CODE_JUMP, 0, CMDQ_JUMP_PASS); > + inst = cmdq_pkt_append_command(pkt); > + if (!inst) > + return -ENOMEM; > + > + inst->op = CMDQ_CODE_JUMP; > + inst->value = CMDQ_JUMP_PASS; > > - return err; > + return 0; > } > > static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data data) > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h > index 911475da7a53..c8adedefaf42 100644 > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h > @@ -19,6 +19,8 @@ > #define CMDQ_WFE_UPDATE BIT(31) > #define CMDQ_WFE_WAIT BIT(15) > #define CMDQ_WFE_WAIT_VALUE 0x1 > +#define CMDQ_WFE_OPTION (CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | \ > + CMDQ_WFE_WAIT_VALUE) > /** cmdq event maximum */ > #define CMDQ_MAX_EVENT 0x3ff > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel