Re: [PATCH] mmc:mmc-cqhci:support interrupt coalescing

Michael Wu <michael@xxxxxxxxxxxxxxxxx> · Tue, 31 Jan 2023 19:01:58 +0800

Dear Michal，
    Thanks for your advices, I'll provide the performance data soon.

On 2023/1/30 16:30, Michal Simek wrote:

If you look at commits on drivers/mmc/host/cqhci-core.c you will see 
that subject style is
mmc: cqhci:<space>

Also some tools are checking this style. Please fix it.



On 1/30/23 07:46, Michael Wu wrote:

Support interrupt coalescing to reduce the frequency of mmc interrupts

Missing dot at the end of sentence. I expect that you are doing it for a 
reason.
I would guess that it increase performance but without any details.
Should I enable it to get for example 20% better performance?

Commit message should be IMHO much bigger with more details and some 
information about how coaleascing works would be also worth.


Signed-off-by: Michael Wu <michael@xxxxxxxxxxxxxxxxx>
---
  drivers/mmc/host/cqhci-core.c      | 20 +++++++++++++++-----
  drivers/mmc/host/cqhci.h           |  5 ++++-
  drivers/mmc/host/mtk-sd.c          |  2 +-
  drivers/mmc/host/sdhci-brcmstb.c   |  2 +-
  drivers/mmc/host/sdhci-esdhc-imx.c |  2 +-
  drivers/mmc/host/sdhci-msm.c       |  2 +-
  drivers/mmc/host/sdhci-of-arasan.c |  2 +-
  drivers/mmc/host/sdhci-pci-core.c  |  2 +-
  drivers/mmc/host/sdhci-pci-gli.c   |  2 +-
  drivers/mmc/host/sdhci-tegra.c     |  2 +-
  drivers/mmc/host/sdhci_am654.c     |  2 +-
  11 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/cqhci-core.c 
b/drivers/mmc/host/cqhci-core.c
index b3d7d6d8d654..f9cdf9f04bfc 100644
--- a/drivers/mmc/host/cqhci-core.c
+++ b/drivers/mmc/host/cqhci-core.c
@@ -420,7 +420,7 @@ static void cqhci_disable(struct mmc_host *mmc)
  }

  static void cqhci_prep_task_desc(struct mmc_request *mrq,
-                                struct cqhci_host *cq_host, int tag)
+                                struct cqhci_host *cq_host, int tag, 
int intr)
  {
         __le64 *task_desc = (__le64 __force *)get_desc(cq_host, tag);
         u32 req_flags = mrq->data->flags;
@@ -428,7 +428,7 @@ static void cqhci_prep_task_desc(struct 
mmc_request *mrq,

         desc0 = CQHCI_VALID(1) |
                 CQHCI_END(1) |
-               CQHCI_INT(1) |
+               CQHCI_INT(intr) |
                 CQHCI_ACT(0x5) |
                 CQHCI_FORCED_PROG(!!(req_flags & MMC_DATA_FORCED_PRG)) |
                 CQHCI_DATA_TAG(!!(req_flags & MMC_DATA_DAT_TAG)) |
@@ -621,7 +621,7 @@ static int cqhci_request(struct mmc_host *mmc, 
struct mmc_request *mrq)
         }

         if (mrq->data) {
-               cqhci_prep_task_desc(mrq, cq_host, tag);
+               cqhci_prep_task_desc(mrq, cq_host, tag, 
(cq_host->intr_clsc ? 0 : 1));

                 err = cqhci_prep_tran_desc(mrq, cq_host, tag);
                 if (err) {
@@ -812,7 +812,7 @@ static void cqhci_finish_mrq(struct mmc_host *mmc, 
unsigned int tag)
  irqreturn_t cqhci_irq(struct mmc_host *mmc, u32 intmask, int cmd_error,
                       int data_error)
  {
-       u32 status;
+       u32 status, rval;
         unsigned long tag = 0, comp_status;
         struct cqhci_host *cq_host = mmc->cqe_private;

@@ -856,6 +856,15 @@ irqreturn_t cqhci_irq(struct mmc_host *mmc, u32 
intmask, int cmd_error,
                 spin_unlock(&cq_host->lock);
         }

+       if (cq_host->intr_clsc) {
+               rval = cqhci_readl(cq_host, CQHCI_IC);
+               rval |= CQHCI_IC_RESET;
+               cqhci_writel(cq_host, rval, CQHCI_IC);
+               rval = cqhci_readl(cq_host, CQHCI_IC);
+               rval &= (~CQHCI_IC_RESET);
+               cqhci_writel(cq_host, rval, CQHCI_IC);
+       }
+
         if (status & CQHCI_IS_TCL)
                 wake_up(&cq_host->wait_queue);

@@ -1172,11 +1181,12 @@ static unsigned int cqhci_ver_minor(struct 
cqhci_host *cq_host)
  }

  int cqhci_init(struct cqhci_host *cq_host, struct mmc_host *mmc,
-             bool dma64)
+             bool dma64, bool intr_clsc)
  {
         int err;

         cq_host->dma64 = dma64;
+       cq_host->intr_clsc = intr_clsc;
         cq_host->mmc = mmc;
         cq_host->mmc->cqe_private = cq_host;

diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
index ba9387ed90eb..acf90773c30a 100644
--- a/drivers/mmc/host/cqhci.h
+++ b/drivers/mmc/host/cqhci.h
@@ -227,6 +227,9 @@ struct cqhci_host {

         /* 64 bit DMA */
         bool dma64;
+
+       /* interrupt coalescing*/

missing space.

+       bool intr_clsc;
         int num_slots;
         int qcnt;

@@ -312,7 +315,7 @@ struct platform_device;

  irqreturn_t cqhci_irq(struct mmc_host *mmc, u32 intmask, int cmd_error,
                       int data_error);
-int cqhci_init(struct cqhci_host *cq_host, struct mmc_host *mmc, bool 
dma64);
+int cqhci_init(struct cqhci_host *cq_host, struct mmc_host *mmc, bool 
dma64, bool intr_clsc);
  struct cqhci_host *cqhci_pltfm_init(struct platform_device *pdev);
  int cqhci_deactivate(struct mmc_host *mmc);
  static inline int cqhci_suspend(struct mmc_host *mmc)
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index edade0e54a0c..2c18f954d4b8 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -2796,7 +2796,7 @@ static int msdc_drv_probe(struct platform_device 
*pdev)
                 host->cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
                 host->cq_host->mmio = host->base + 0x800;
                 host->cq_host->ops = &msdc_cmdq_ops;
-               ret = cqhci_init(host->cq_host, mmc, true);
+               ret = cqhci_init(host->cq_host, mmc, true, false);
                 if (ret)
                         goto host_free;
                 mmc->max_segs = 128;
diff --git a/drivers/mmc/host/sdhci-brcmstb.c 
b/drivers/mmc/host/sdhci-brcmstb.c
index f2cf3d70db79..4aeaeddbbf25 100644
--- a/drivers/mmc/host/sdhci-brcmstb.c
+++ b/drivers/mmc/host/sdhci-brcmstb.c
@@ -231,7 +231,7 @@ static int sdhci_brcmstb_add_host(struct 
sdhci_host *host,
                 cq_host->caps |= CQHCI_TASK_DESC_SZ_128;
         }

-       ret = cqhci_init(cq_host, host->mmc, dma64);
+       ret = cqhci_init(cq_host, host->mmc, dma64, false);
         if (ret)
                 goto cleanup;

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
b/drivers/mmc/host/sdhci-esdhc-imx.c
index 9e73c34b6401..7aef7abe71f1 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1712,7 +1712,7 @@ static int sdhci_esdhc_imx_probe(struct 
platform_device *pdev)
                 cq_host->mmio = host->ioaddr + ESDHC_CQHCI_ADDR_OFFSET;
                 cq_host->ops = &esdhc_cqhci_ops;

-               err = cqhci_init(cq_host, host->mmc, false);
+               err = cqhci_init(cq_host, host->mmc, false, false);
                 if (err)
                         goto disable_ahb_clk;
         }
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 4ac8651d0b29..b6549d1e43ec 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -2153,7 +2153,7 @@ static int sdhci_msm_cqe_add_host(struct 
sdhci_host *host,
         if (ret)
                 goto cleanup;

-       ret = cqhci_init(cq_host, host->mmc, dma64);
+       ret = cqhci_init(cq_host, host->mmc, dma64, false);
         if (ret) {
                 dev_err(&pdev->dev, "%s: CQE init: failed (%d)\n",
                                 mmc_hostname(host->mmc), ret);
diff --git a/drivers/mmc/host/sdhci-of-arasan.c 
b/drivers/mmc/host/sdhci-of-arasan.c
index 89c431a34c43..811f8686532d 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -1610,7 +1610,7 @@ static int sdhci_arasan_add_host(struct 
sdhci_arasan_data *sdhci_arasan)
         if (dma64)
                 cq_host->caps |= CQHCI_TASK_DESC_SZ_128;

-       ret = cqhci_init(cq_host, host->mmc, dma64);
+       ret = cqhci_init(cq_host, host->mmc, dma64, false);
         if (ret)
                 goto cleanup;

diff --git a/drivers/mmc/host/sdhci-pci-core.c 
b/drivers/mmc/host/sdhci-pci-core.c
index c359f867df0a..6f6cae6355a7 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -964,7 +964,7 @@ static int glk_emmc_add_host(struct sdhci_pci_slot 
*slot)
         if (dma64)
                 cq_host->caps |= CQHCI_TASK_DESC_SZ_128;

-       ret = cqhci_init(cq_host, host->mmc, dma64);
+       ret = cqhci_init(cq_host, host->mmc, dma64, false);
         if (ret)
                 goto cleanup;

diff --git a/drivers/mmc/host/sdhci-pci-gli.c 
b/drivers/mmc/host/sdhci-pci-gli.c
index 633a8ee8f8c5..6917ba339aa9 100644
--- a/drivers/mmc/host/sdhci-pci-gli.c
+++ b/drivers/mmc/host/sdhci-pci-gli.c
@@ -908,7 +908,7 @@ static int gl9763e_add_host(struct sdhci_pci_slot 
*slot)
         if (dma64)
                 cq_host->caps |= CQHCI_TASK_DESC_SZ_128;

-       ret = cqhci_init(cq_host, host->mmc, dma64);
+       ret = cqhci_init(cq_host, host->mmc, dma64, false);
         if (ret)
                 goto cleanup;

diff --git a/drivers/mmc/host/sdhci-tegra.c 
b/drivers/mmc/host/sdhci-tegra.c
index bff084f178c9..f98a468e8f43 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -1620,7 +1620,7 @@ static int sdhci_tegra_add_host(struct 
sdhci_host *host)
         if (dma64)
                 cq_host->caps |= CQHCI_TASK_DESC_SZ_128;

-       ret = cqhci_init(cq_host, host->mmc, dma64);
+       ret = cqhci_init(cq_host, host->mmc, dma64, false);
         if (ret)
                 goto cleanup;

diff --git a/drivers/mmc/host/sdhci_am654.c 
b/drivers/mmc/host/sdhci_am654.c
index 7ef828942df3..8e7fbee70e16 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -568,7 +568,7 @@ static int sdhci_am654_cqe_add_host(struct 
sdhci_host *host)

         host->mmc->caps2 |= MMC_CAP2_CQE;

-       return cqhci_init(cq_host, host->mmc, 1);
+       return cqhci_init(cq_host, host->mmc, 1, false);
  }

  static int sdhci_am654_get_otap_delay(struct sdhci_host *host,
--
2.29.0


M

--
Regards,
Michael Wu






