lkml.org 
[lkml]   [2023]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] mmc:mmc-cqhci:support interrupt coalescing
From
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 <michael@allwinnertech.com>
>>>> ---
>>>>   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

\
 
 \ /
  Last update: 2023-03-27 00:09    [W:0.066 / U:0.504 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site