2023-01-30 06:46:54

by Michael Wu

[permalink] [raw]
Subject: [PATCH] mmc:mmc-cqhci:support interrupt coalescing

Support interrupt coalescing to reduce the frequency of mmc interrupts

Signed-off-by: Michael Wu <[email protected]>
---
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*/
+ 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



2023-01-30 08:31:17

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH] mmc:mmc-cqhci:support interrupt coalescing


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 <[email protected]>
> ---
> 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

2023-01-30 15:59:52

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH] mmc:mmc-cqhci:support interrupt coalescing

On 30/01/23 08:46, Michael Wu wrote:
> Support interrupt coalescing to reduce the frequency of mmc interrupts

There doesn't seem to be any users. The new parameter to
cqhci_init() is always false. New features are not usually
accepted without users.

There needs to be an explanation of why the change is being made.

Also there doesn't seem to be any configuration of the CQIC
register.

>
> Signed-off-by: Michael Wu <[email protected]>
> ---
> 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*/
> + 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,


2023-01-31 03:09:23

by Wenchao Chen

[permalink] [raw]
Subject: Re: [PATCH] mmc:mmc-cqhci:support interrupt coalescing

On Mon, Jan 30, 2023 at 2:49 PM Michael Wu <[email protected]> wrote:
>
> Support interrupt coalescing to reduce the frequency of mmc interrupts
>

Hi Michael
The CQIS register does not have any configuration.
Usually ICCTH needs to be enabled.

> Signed-off-by: Michael Wu <[email protected]>
> ---
> 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*/
> + 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
>

2023-01-31 11:02:14

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] mmc:mmc-cqhci:support interrupt coalescing

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 <[email protected]>
>> ---
>>   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

2023-01-31 13:10:57

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] mmc:mmc-cqhci:support interrupt coalescing

Dear Wenchao,
At present, I am working on the hook function .cqe_enable that the
ICCTH of CQIC is set, and it work well in my test. Actually I want to
confirm why the community does not support this feature. I think it is
necessary to reduce the number of interrupts caused by IO and reducing
interrupt context switching.

On 2023/1/31 11:09, Wenchao Chen wrote:
> On Mon, Jan 30, 2023 at 2:49 PM Michael Wu <[email protected]> wrote:
>>
>> Support interrupt coalescing to reduce the frequency of mmc interrupts
>>
>
> Hi Michael
> The CQIS register does not have any configuration.
> Usually ICCTH needs to be enabled.
>
>> Signed-off-by: Michael Wu <[email protected]>
>> ---
>> 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*/
>> + 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
>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
Regards,
Michael Wu

2023-01-31 13:13:30

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] mmc:mmc-cqhci:support interrupt coalescing

Dear Adrian,

On 2023/1/30 23:59, Adrian Hunter wrote:
> On 30/01/23 08:46, Michael Wu wrote:
>> Support interrupt coalescing to reduce the frequency of mmc interrupts
>
> There doesn't seem to be any users. The new parameter to
> cqhci_init() is always false. New features are not usually
> accepted without users.

At present, I have only supported this feature on the Allwinner platform
because I have not get enough information about other vendors.
>
> There needs to be an explanation of why the change is being made.

I consider reducing the number of interrupt context switches. Can you
tell me why the community does not support this feature? I wonder
whether I'm doing is correct. If it's correct, i will improve this patch
and incorporate it
>
> Also there doesn't seem to be any configuration of the CQIC
> register.

At present, I am working on the hook function .cqe_enable that the ICCTH
of CQIC is set in enable
>
>>
>> Signed-off-by: Michael Wu <[email protected]>
>> ---
>> 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*/
>> + 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,

--
Regards,
Michael Wu

2023-01-31 13:33:20

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH] mmc:mmc-cqhci:support interrupt coalescing

On 31/01/23 15:12, Michael Wu wrote:
> Dear Adrian,
>
> On 2023/1/30 23:59, Adrian Hunter wrote:
>> On 30/01/23 08:46, Michael Wu wrote:
>>> Support interrupt coalescing to reduce the frequency of mmc interrupts
>>
>> There doesn't seem to be any users.  The new parameter to
>> cqhci_init() is always false.  New features are not usually
>> accepted without users.
>
> At present, I have only supported this feature on the Allwinner platform because I have not get enough information about other vendors.
>>
>> There needs to be an explanation of why the change is being made.
>
> I consider reducing the number of interrupt context switches. Can you tell me why the community does not support this feature? I wonder whether I'm doing is correct. If it's correct, i will improve this patch and incorporate it

AFAIK the community has not made any comment on supporting interrupt coalescing for CQHCI, one way or another.
However, it is generally expected that new features will only be added if they demonstrate some benefit.
Just assuming that there is a benefit is not enough.

>>
>> Also there doesn't seem to be any configuration of the CQIC
>> register.
>
> At present, I am working on the hook function .cqe_enable that the ICCTH of CQIC is set in enable
>>
>>>
>>> Signed-off-by: Michael Wu <[email protected]>
>>> ---
>>>   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*/
>>> +    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,
>


2023-02-04 06:05:42

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] mmc:mmc-cqhci:support interrupt coalescing

Dear Adrian,

On 2023/1/31 21:32, Adrian Hunter wrote:
> On 31/01/23 15:12, Michael Wu wrote:
>> Dear Adrian,
>>
>> On 2023/1/30 23:59, Adrian Hunter wrote:
>>> On 30/01/23 08:46, Michael Wu wrote:
>>>> Support interrupt coalescing to reduce the frequency of mmc interrupts
>>>
>>> There doesn't seem to be any users.  The new parameter to
>>> cqhci_init() is always false.  New features are not usually
>>> accepted without users.
>>
>> At present, I have only supported this feature on the Allwinner platform because I have not get enough information about other vendors.
>>>
>>> There needs to be an explanation of why the change is being made.
>>
>> I consider reducing the number of interrupt context switches. Can you tell me why the community does not support this feature? I wonder whether I'm doing is correct. If it's correct, i will improve this patch and incorporate it
>
> AFAIK the community has not made any comment on supporting interrupt coalescing for CQHCI, one way or another.
> However, it is generally expected that new features will only be added if they demonstrate some benefit.
> Just assuming that there is a benefit is not enough.
>

Okay, I'll provide the performance data soon.

>>>
>>> Also there doesn't seem to be any configuration of the CQIC
>>> register.
>>
>> At present, I am working on the hook function .cqe_enable that the ICCTH of CQIC is set in enable
>>>
>>>>
>>>> Signed-off-by: Michael Wu <[email protected]>
>>>> ---
>>>>   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*/
>>>> +    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,
>>

--
Regards,
Michael Wu