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=-4.7 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,UNPARSEABLE_RELAY,UNWANTED_LANGUAGE_BODY,URIBL_BLOCKED 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 2AA92C43387 for ; Wed, 26 Dec 2018 02:57:19 +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 EC1EA21726 for ; Wed, 26 Dec 2018 02:57:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="W5WwhNL8" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EC1EA21726 Authentication-Results: mail.kernel.org; dmarc=none (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=zrIpKBPa9DS2XA9elTCg0FQrAQHBEfTFuvZcdYkexHg=; b=W5WwhNL8Xpww24 LNFG+YqYicQM65IRFM2qPMKoVEvMmPCdg2ZXIlr2u6msYGDeELl/iGGP+MwGg5HEDJqMEjW9+9xVZ fea2/gML4FyxGMjhliqbH27JBvh8m1ykwmGAWW1rNC7mTMQuZYTtTfwOSQ7qA4EUYZeT/q8734+uR 4x9Aj7/6/16aLi3xVsRRBvSfNQ/NRCd4Q1SJzqM7G57to2+weVaN+B37mHdLWxmt2PoeQjoZ+bXX6 8P6pBViwZUo0YGU2blkUHPnoFfxc8+O1YKLKkLpSZljosV/QwjtiFpoNdh3S9EegGhNrxr3vLmjIg iCHhc4SH6vNaw8wseeUQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gbzNl-0005cJ-9C; Wed, 26 Dec 2018 02:57:17 +0000 Received: from [1.203.163.81] (helo=mailgw02.mediatek.com) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gbzNf-0005Fg-DY; Wed, 26 Dec 2018 02:57:15 +0000 X-UUID: edf04048d0304c6f8a222e14acd23963-20181226 X-UUID: edf04048d0304c6f8a222e14acd23963-20181226 Received: from mtkcas35.mediatek.inc [(172.27.4.250)] by mailgw02.mediatek.com (envelope-from ) (mailgw01.mediatek.com ESMTP with TLS) with ESMTP id 296912647; Wed, 26 Dec 2018 10:56:58 +0800 Received: from MTKCAS06.mediatek.inc (172.21.101.30) by MTKMBS31DR.mediatek.inc (172.27.6.102) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Wed, 26 Dec 2018 10:56:54 +0800 Received: from [172.21.77.4] (172.21.77.4) by MTKCAS06.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1395.4 via Frontend Transport; Wed, 26 Dec 2018 10:56:54 +0800 Message-ID: <1545793014.14496.21.camel@mtksdaap41> Subject: Re: [PATCH 02/18] drm/mediatek: add mutex mod and sof into ddp private data From: CK Hu To: Yongqiang Niu Date: Wed, 26 Dec 2018 10:56:54 +0800 In-Reply-To: <1545638931-24938-3-git-send-email-yongqiang.niu@mediatek.com> References: <1545638931-24938-1-git-send-email-yongqiang.niu@mediatek.com> <1545638931-24938-3-git-send-email-yongqiang.niu@mediatek.com> X-Mailer: Evolution 3.10.4-0ubuntu2 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20181225_185711_915306_A5470C7A X-CRM114-Status: GOOD ( 19.11 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , devicetree@vger.kernel.org, David Airlie , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Rob Herring , linux-mediatek@lists.infradead.org, Philipp Zabel , Matthias Brugger , linux-arm-kernel@lists.infradead.org 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, Yongqiang: On Mon, 2018-12-24 at 16:08 +0800, Yongqiang Niu wrote: > This patch add mutex mod and sof into ddp private data Usually, the commit title shows 'WHAT' does this patch do, commit message shows 'WHY' does this patch do, and commit body shows 'HOW' does this patch do. This commit message just show WHAT does this patch do but does not show WHY. Maybe this is trivial for you but not for everyone. So describe more about why you do this. In addition, 'add mutex mode' and 'add sof' are two things, so break these two modification into two patches. > > Signed-off-by: Yongqiang Niu > --- > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 117 ++++++++++++++++++++++++++------- > 1 file changed, 94 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > index 579ce28..adb37e4 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > @@ -41,11 +41,14 @@ > #define DISP_REG_CONFIG_DSI_SEL 0x050 > #define DISP_REG_CONFIG_DPI_SEL 0x064 > > +#define MT2701_DISP_MUTEX0_MOD0 0x2C Lower case for hex number. > +#define MT2701_DISP_MUTEX0_SOF0 0x30 Align 0x2c and 0x30 > + > #define DISP_REG_MUTEX_EN(n) (0x20 + 0x20 * (n)) > #define DISP_REG_MUTEX(n) (0x24 + 0x20 * (n)) > #define DISP_REG_MUTEX_RST(n) (0x28 + 0x20 * (n)) > -#define DISP_REG_MUTEX_MOD(n) (0x2c + 0x20 * (n)) > -#define DISP_REG_MUTEX_SOF(n) (0x30 + 0x20 * (n)) > +#define DISP_REG_MUTEX_MOD(data, n) ((data)->mutex_mod_reg + 0x20 * (n)) > +#define DISP_REG_MUTEX_SOF(data, n) ((data)->mutex_sof_reg + 0x20 * (n)) > #define DISP_REG_MUTEX_MOD2(n) (0x34 + 0x20 * (n)) > > #define INT_MUTEX BIT(1) > @@ -147,12 +150,30 @@ struct mtk_disp_mutex { > bool claimed; > }; > > +enum mtk_ddp_mutex_sof_id { > + DDP_MUTEX_SOF_SINGLE_MODE, > + DDP_MUTEX_SOF_DSI0, > + DDP_MUTEX_SOF_DSI1, > + DDP_MUTEX_SOF_DPI0, > + DDP_MUTEX_SOF_DPI1, > + DDP_MUTEX_SOF_DSI2, > + DDP_MUTEX_SOF_DSI3, > + DDP_MUTEX_SOF_MAX, > +}; > + > +struct mtk_ddp_data { > + const unsigned int *mutex_mod; > + const unsigned int *mutex_sof; > + unsigned int mutex_mod_reg; > + unsigned int mutex_sof_reg; > +}; > + > struct mtk_ddp { > struct device *dev; > struct clk *clk; > void __iomem *regs; > struct mtk_disp_mutex mutex[10]; > - const unsigned int *mutex_mod; > + const struct mtk_ddp_data *data; > }; > > static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = { > @@ -202,6 +223,51 @@ struct mtk_ddp { > [DDP_COMPONENT_WDMA1] = MT8173_MUTEX_MOD_DISP_WDMA1, > }; > > +static const unsigned int mt2701_mutex_sof[DDP_MUTEX_SOF_MAX] = { > + [DDP_MUTEX_SOF_SINGLE_MODE] = MUTEX_SOF_SINGLE_MODE, > + [DDP_MUTEX_SOF_DSI0] = MUTEX_SOF_DSI0, > + [DDP_MUTEX_SOF_DSI1] = MUTEX_SOF_DSI1, > + [DDP_MUTEX_SOF_DPI0] = MUTEX_SOF_DPI0, > +}; > + > +static const unsigned int mt2712_mutex_sof[DDP_MUTEX_SOF_MAX] = { > + [DDP_MUTEX_SOF_SINGLE_MODE] = MUTEX_SOF_SINGLE_MODE, > + [DDP_MUTEX_SOF_DSI0] = MUTEX_SOF_DSI0, > + [DDP_MUTEX_SOF_DSI1] = MUTEX_SOF_DSI1, > + [DDP_MUTEX_SOF_DPI0] = MUTEX_SOF_DPI0, > + [DDP_MUTEX_SOF_DPI1] = MUTEX_SOF_DPI1, > + [DDP_MUTEX_SOF_DSI2] = MUTEX_SOF_DSI2, > + [DDP_MUTEX_SOF_DSI3] = MUTEX_SOF_DSI3, > +}; > + > +static const unsigned int mt8173_mutex_sof[DDP_MUTEX_SOF_MAX] = { > + [DDP_MUTEX_SOF_SINGLE_MODE] = MUTEX_SOF_SINGLE_MODE, > + [DDP_MUTEX_SOF_DSI0] = MUTEX_SOF_DSI0, > + [DDP_MUTEX_SOF_DSI1] = MUTEX_SOF_DSI1, > + [DDP_MUTEX_SOF_DPI0] = MUTEX_SOF_DPI0, > +}; It looks like that both mt8173_mutex_sof and mt2701_mutex_sof are subset of mt2712_mutex_sof, so I think you could keep only mt2712_mutex_sof and mt8173 and mt2701 also use this one. > + > +static const struct mtk_ddp_data mt2701_ddp_driver_data = { > + .mutex_mod = mt2701_mutex_mod, > + .mutex_sof = mt2701_mutex_sof, > + .mutex_mod_reg = MT2701_DISP_MUTEX0_MOD0, > + .mutex_sof_reg = MT2701_DISP_MUTEX0_SOF0, > +}; > + > +static const struct mtk_ddp_data mt2712_ddp_driver_data = { > + .mutex_mod = mt2712_mutex_mod, > + .mutex_sof = mt2712_mutex_sof, > + .mutex_mod_reg = MT2701_DISP_MUTEX0_MOD0, > + .mutex_sof_reg = MT2701_DISP_MUTEX0_SOF0, > +}; > + > +static const struct mtk_ddp_data mt8173_ddp_driver_data = { > + .mutex_mod = mt8173_mutex_mod, > + .mutex_sof = mt8173_mutex_sof, > + .mutex_mod_reg = MT2701_DISP_MUTEX0_MOD0, > + .mutex_sof_reg = MT2701_DISP_MUTEX0_SOF0, > +}; > + > static unsigned int mtk_ddp_mout_en(enum mtk_ddp_comp_id cur, > enum mtk_ddp_comp_id next, > unsigned int *addr) > @@ -446,39 +512,40 @@ void mtk_disp_mutex_add_comp(struct mtk_disp_mutex *mutex, > > switch (id) { > case DDP_COMPONENT_DSI0: > - reg = MUTEX_SOF_DSI0; > + reg = DDP_MUTEX_SOF_DSI0; I would like you to change 'reg' to 'id' because you change the usage of this variable. Regards, CK > break; > case DDP_COMPONENT_DSI1: > - reg = MUTEX_SOF_DSI0; > + reg = DDP_MUTEX_SOF_DSI0; > break; > case DDP_COMPONENT_DSI2: > - reg = MUTEX_SOF_DSI2; > + reg = DDP_MUTEX_SOF_DSI2; > break; > case DDP_COMPONENT_DSI3: > - reg = MUTEX_SOF_DSI3; > + reg = DDP_MUTEX_SOF_DSI3; > break; > case DDP_COMPONENT_DPI0: > - reg = MUTEX_SOF_DPI0; > + reg = DDP_MUTEX_SOF_DPI0; > break; > case DDP_COMPONENT_DPI1: > - reg = MUTEX_SOF_DPI1; > + reg = DDP_MUTEX_SOF_DPI1; > break; > default: > - if (ddp->mutex_mod[id] < 32) { > - offset = DISP_REG_MUTEX_MOD(mutex->id); > + if (ddp->data->mutex_mod[id] < 32) { > + offset = DISP_REG_MUTEX_MOD(ddp->data, mutex->id); > reg = readl_relaxed(ddp->regs + offset); > - reg |= 1 << ddp->mutex_mod[id]; > + reg |= 1 << ddp->data->mutex_mod[id]; > writel_relaxed(reg, ddp->regs + offset); > } else { > offset = DISP_REG_MUTEX_MOD2(mutex->id); > reg = readl_relaxed(ddp->regs + offset); > - reg |= 1 << (ddp->mutex_mod[id] - 32); > + reg |= 1 << (ddp->data->mutex_mod[id] - 32); > writel_relaxed(reg, ddp->regs + offset); > } > return; > } > > - writel_relaxed(reg, ddp->regs + DISP_REG_MUTEX_SOF(mutex->id)); > + writel_relaxed(ddp->data->mutex_sof[reg], > + ddp->regs + DISP_REG_MUTEX_SOF(ddp->data, mutex->id)); > } > > void mtk_disp_mutex_remove_comp(struct mtk_disp_mutex *mutex, > @@ -499,18 +566,19 @@ void mtk_disp_mutex_remove_comp(struct mtk_disp_mutex *mutex, > case DDP_COMPONENT_DPI0: > case DDP_COMPONENT_DPI1: > writel_relaxed(MUTEX_SOF_SINGLE_MODE, > - ddp->regs + DISP_REG_MUTEX_SOF(mutex->id)); > + ddp->regs + > + DISP_REG_MUTEX_SOF(ddp->data, mutex->id)); > break; > default: > - if (ddp->mutex_mod[id] < 32) { > - offset = DISP_REG_MUTEX_MOD(mutex->id); > + if (ddp->data->mutex_mod[id] < 32) { > + offset = DISP_REG_MUTEX_MOD(ddp->data, mutex->id); > reg = readl_relaxed(ddp->regs + offset); > - reg &= ~(1 << ddp->mutex_mod[id]); > + reg &= ~(1 << ddp->data->mutex_mod[id]); > writel_relaxed(reg, ddp->regs + offset); > } else { > offset = DISP_REG_MUTEX_MOD2(mutex->id); > reg = readl_relaxed(ddp->regs + offset); > - reg &= ~(1 << (ddp->mutex_mod[id] - 32)); > + reg &= ~(1 << (ddp->data->mutex_mod[id] - 32)); > writel_relaxed(reg, ddp->regs + offset); > } > break; > @@ -585,7 +653,7 @@ static int mtk_ddp_probe(struct platform_device *pdev) > return PTR_ERR(ddp->regs); > } > > - ddp->mutex_mod = of_device_get_match_data(dev); > + ddp->data = of_device_get_match_data(dev); > > platform_set_drvdata(pdev, ddp); > > @@ -598,9 +666,12 @@ static int mtk_ddp_remove(struct platform_device *pdev) > } > > static const struct of_device_id ddp_driver_dt_match[] = { > - { .compatible = "mediatek,mt2701-disp-mutex", .data = mt2701_mutex_mod}, > - { .compatible = "mediatek,mt2712-disp-mutex", .data = mt2712_mutex_mod}, > - { .compatible = "mediatek,mt8173-disp-mutex", .data = mt8173_mutex_mod}, > + { .compatible = "mediatek,mt2701-disp-mutex", > + .data = &mt2701_ddp_driver_data}, > + { .compatible = "mediatek,mt2712-disp-mutex", > + .data = &mt2712_ddp_driver_data}, > + { .compatible = "mediatek,mt8173-disp-mutex", > + .data = &mt8173_ddp_driver_data}, > {}, > }; > MODULE_DEVICE_TABLE(of, ddp_driver_dt_match); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel