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=-12.5 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,UNWANTED_LANGUAGE_BODY, USER_AGENT_SANE_1 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 08542C11F68 for ; Wed, 30 Jun 2021 12:18:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E56EE61624 for ; Wed, 30 Jun 2021 12:18:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234539AbhF3MUm (ORCPT ); Wed, 30 Jun 2021 08:20:42 -0400 Received: from mga05.intel.com ([192.55.52.43]:10895 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234376AbhF3MUk (ORCPT ); Wed, 30 Jun 2021 08:20:40 -0400 X-IronPort-AV: E=McAfee;i="6200,9189,10030"; a="293973467" X-IronPort-AV: E=Sophos;i="5.83,311,1616482800"; d="scan'208";a="293973467" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Jun 2021 05:18:09 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.83,311,1616482800"; d="scan'208";a="419972036" Received: from ahunter-desktop.fi.intel.com (HELO [10.237.72.79]) ([10.237.72.79]) by fmsmga007.fm.intel.com with ESMTP; 30 Jun 2021 05:18:01 -0700 Subject: Re: [PATCH] [RESEND] mmc: sdhci-pci-gli: Improve Random 4K Read Performance of GL9763E To: Renius Chen , ulf.hansson@linaro.org Cc: linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org, Ben.Chuang@genesyslogic.com.tw References: <20210624032543.101861-1-reniuschengl@gmail.com> From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki, Business Identity Code: 0357606 - 4, Domiciled in Helsinki Message-ID: <916ea01b-4265-0964-9139-8f50b80ea66e@intel.com> Date: Wed, 30 Jun 2021 15:18:19 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210624032543.101861-1-reniuschengl@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 24/06/21 6:25 am, Renius Chen wrote: > During a sequence of random 4K read operations, the performance will be > reduced due to spending much time on entering/exiting the low power state > between requests. We disable the low power state negotiation of GL9763E > during a sequence of random 4K read operations to improve the performance > and enable it again after the operations have finished. > > Signed-off-by: Renius Chen > --- > drivers/mmc/host/sdhci-pci-gli.c | 86 ++++++++++++++++++++++++++++++++ > 1 file changed, 86 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c > index 302a7579a9b3..0105f728ccc4 100644 > --- a/drivers/mmc/host/sdhci-pci-gli.c > +++ b/drivers/mmc/host/sdhci-pci-gli.c > @@ -88,6 +88,9 @@ > #define PCIE_GLI_9763E_SCR 0x8E0 > #define GLI_9763E_SCR_AXI_REQ BIT(9) > > +#define PCIE_GLI_9763E_CFG 0x8A0 > +#define GLI_9763E_CFG_LPSN_DIS BIT(12) > + > #define PCIE_GLI_9763E_CFG2 0x8A4 > #define GLI_9763E_CFG2_L1DLY GENMASK(28, 19) > #define GLI_9763E_CFG2_L1DLY_MID 0x54 > @@ -691,6 +694,86 @@ static void sdhci_gl9763e_dumpregs(struct mmc_host *mmc) > sdhci_dumpregs(mmc_priv(mmc)); > } > > +static void gl9763e_request(struct mmc_host *mmc, struct mmc_request *mrq) > +{ > + struct sdhci_host *host = mmc_priv(mmc); > + struct mmc_command *cmd; > + struct sdhci_pci_slot *slot = sdhci_priv(host); > + struct pci_dev *pdev = slot->chip->pdev; > + u32 value; > + static bool start_4k_r; > + static int continuous_4k_r; Please do not use static variables. Instead define a struct for them and set .priv_size in sdhci_pci_fixes e.g. struct sdhci_pci_gli { static bool start_4k_r; static int continuous_4k_r; }; const struct sdhci_pci_fixes sdhci_gl9763e = { .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC, .probe_slot = gli_probe_slot_gl9763e, .ops = &sdhci_gl9763e_ops, #ifdef CONFIG_PM_SLEEP .resume = sdhci_cqhci_gli_resume, .suspend = sdhci_cqhci_gli_suspend, #endif .add_host = gl9763e_add_host, .priv_size = sizeof(struct sdhci_pci_gli); }; And then you can get a pointer to it: struct sdhci_pci_gli *sdhci_pci_gli = sdhci_pci_priv(slot); > + > + cmd = (mrq->sbc && !(host->flags & SDHCI_AUTO_CMD23)) ? mrq->sbc : mrq->cmd; mrq->cmd is always the main command, so why look at mrq->sbc? > + > + if (cmd->opcode == MMC_READ_MULTIPLE_BLOCK) { > + if (cmd->data->blocks == 8) { > + continuous_4k_r++; > + > + if ((!start_4k_r) && (continuous_4k_r >= 3)) { > + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value); > + value &= ~GLI_9763E_VHS_REV; > + value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_W); > + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value); > + > + pci_read_config_dword(pdev, PCIE_GLI_9763E_CFG, &value); > + value |= GLI_9763E_CFG_LPSN_DIS; > + pci_write_config_dword(pdev, PCIE_GLI_9763E_CFG, value); > + > + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value); > + value &= ~GLI_9763E_VHS_REV; > + value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_R); > + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value); > + > + start_4k_r = true; > + } > + } else { > + continuous_4k_r = 0; > + > + if (start_4k_r) { > + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value); > + value &= ~GLI_9763E_VHS_REV; > + value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_W); > + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value); > + > + pci_read_config_dword(pdev, PCIE_GLI_9763E_CFG, &value); > + value &= ~GLI_9763E_CFG_LPSN_DIS; > + pci_write_config_dword(pdev, PCIE_GLI_9763E_CFG, value); > + > + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value); > + value &= ~GLI_9763E_VHS_REV; > + value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_R); > + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value); > + > + start_4k_r = false; > + } > + } > + } else { > + continuous_4k_r = 0; > + > + if (start_4k_r) { > + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value); > + value &= ~GLI_9763E_VHS_REV; > + value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_W); > + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value); > + > + pci_read_config_dword(pdev, PCIE_GLI_9763E_CFG, &value); > + value &= ~GLI_9763E_CFG_LPSN_DIS; > + pci_write_config_dword(pdev, PCIE_GLI_9763E_CFG, value); > + > + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value); > + value &= ~GLI_9763E_VHS_REV; > + value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_R); > + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value); > + > + start_4k_r = false; > + } The code chunk above is the same as the previous and almost the same as the one before that. Please make a separate function > + } > + > + sdhci_request(mmc, mrq); > +} > + > + > static void sdhci_gl9763e_cqe_pre_enable(struct mmc_host *mmc) > { > struct cqhci_host *cq_host = mmc->cqe_private; > @@ -848,6 +931,9 @@ static int gli_probe_slot_gl9763e(struct sdhci_pci_slot *slot) > gli_pcie_enable_msi(slot); > host->mmc_host_ops.hs400_enhanced_strobe = > gl9763e_hs400_enhanced_strobe; > + > + host->mmc_host_ops.request = gl9763e_request; > + > gli_set_gl9763e(slot); > sdhci_enable_v4_mode(host); > >