Re: [PATCH 4/4] mmc: dw_mmc: Convert descriptor ring buffer to per-instance configurable

[Ulf Hansson]

On Thu, 9 Apr 2026 at 09:48, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote:
>
> Replace the hardcoded DESC_RING_BUF_SZ macro with a per-instance
> ring_size member in struct dw_mci. This change provides greater
> flexibility and prepares the driver for future configuration options.
>
> Variant host controllers can now override the default ring_size via
> the struct dw_mci_drv_data::init() callback, allowing them to tune
> the descriptor ring buffer size for their specific use cases. This
> is particularly beneficial for improving performance in large-block
> sequential read/write scenarios.

I am not sure what you mean by "use cases" here, but generally, we
should avoid optimizing by tweaking buffer sizes at the host driver
level.

Although, perhaps if you show how this is intended to work with a
user, it becomes easier to discuss?

>
> Empirical testing shows that increasing ring_size can significantly
> improve request efficiency. For example, the block count per request
> can increase from 0x800 (1 MiB) to 0x2000 (4 MiB), as demonstrated
> by trace data:
>
>     dd-706     [004] .....   106.017566: mmc_request_start: mmc1: start
>     struct mmc_request[0000000066f43a37]: ... sbc_arg=0x800
>
>     dd-697     [001] .....    15.227953: mmc_request_start: mmc1: start
>     struct mmc_request[00000000d82bf187]: ... sbc_arg=0x2000
>
> While increasing the request size improves sequential I/O throughput,
> it also introduces trade-offs: larger requests can delay other pending
> I/O operations. Therefore, this configuration should be balanced
> according to the specific workload and not hardcoded globally.
>
> The default ring_size is initialized to PAGE_SIZE in dw_mci_alloc_host(),
> preserving existing behavior. All buffer size calculations now use
> host->ring_size instead of the hardcoded macro.
>
> No functional changes are introduced for existing platforms.
>
> Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>

Kind regards
Uffe

>
> ---
>
>  drivers/mmc/host/dw_mmc.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index df6daa6..61f10e7 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -50,8 +50,6 @@
>                                  SDMMC_IDMAC_INT_FBE | SDMMC_IDMAC_INT_RI | \
>                                  SDMMC_IDMAC_INT_TI)
>
> -#define DESC_RING_BUF_SZ       PAGE_SIZE
> -
>  struct idmac_desc_64addr {
>         u32             des0;   /* Control Descriptor */
>  #define IDMAC_OWN_CLR64(x) \
> @@ -493,7 +491,7 @@ static int dw_mci_idmac_init(struct dw_mci *host)
>                 struct idmac_desc_64addr *p;
>
>                 host->desc_num =
> -                       DESC_RING_BUF_SZ / sizeof(struct idmac_desc_64addr);
> +                       host->ring_size / sizeof(struct idmac_desc_64addr);
>
>                 /* Forward link the descriptor list */
>                 for (i = 0, p = host->sg_cpu; i < host->desc_num - 1;
> @@ -521,7 +519,7 @@ static int dw_mci_idmac_init(struct dw_mci *host)
>                 struct idmac_desc *p;
>
>                 host->desc_num =
> -                       DESC_RING_BUF_SZ / sizeof(struct idmac_desc);
> +                       host->ring_size / sizeof(struct idmac_desc);
>
>                 /* Forward link the descriptor list */
>                 for (i = 0, p = host->sg_cpu;
> @@ -653,7 +651,7 @@ static inline int dw_mci_prepare_desc(struct dw_mci *host, struct mmc_data *data
>  err_own_bit:
>         /* restore the descriptor chain as it's polluted */
>         dev_dbg(host->dev, "descriptor is still owned by IDMAC.\n");
> -       memset(host->sg_cpu, 0, DESC_RING_BUF_SZ);
> +       memset(host->sg_cpu, 0, host->ring_size);
>         dw_mci_idmac_init(host);
>         return -EINVAL;
>  }
> @@ -2954,7 +2952,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
>
>                 /* Alloc memory for sg translation */
>                 host->sg_cpu = dmam_alloc_coherent(host->dev,
> -                                                  DESC_RING_BUF_SZ,
> +                                                  host->ring_size,
>                                                    &host->sg_dma, GFP_KERNEL);
>                 if (!host->sg_cpu) {
>                         dev_err(host->dev,
> @@ -3185,6 +3183,7 @@ struct dw_mci *dw_mci_alloc_host(struct device *dev)
>         host = mmc_priv(mmc);
>         host->mmc = mmc;
>         host->dev = dev;
> +       host->ring_size = PAGE_SIZE;
>
>         return host;
>  }
> --
> 2.7.4
>