diff mbox series

mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read performance

Message ID 20250217110623.2383142-1-ziniu.wang_1@nxp.com (mailing list archive)
State New
Headers show
Series mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read performance | expand

Commit Message

Luke Wang Feb. 17, 2025, 11:06 a.m. UTC 
From: Luke Wang <ziniu.wang_1@nxp.com>

Compared with kernel 6.1, imx8mq eMMC/SD read performance drops by about
30% with kernel 6.6.

The eMMC/SD read thread will be put to sleep until the hardware completes
data transfer. Normally, the read thread will be woken up immediately
when the data transfer is completed. However, due to a known ic bug, if
imx8mq is in cpuidle, it will take a long time (about 500us) to exit
cpuidle. As a result, the read thread cannot immediately read the next
data block, affecting the read performance.

Kernel 6.6 uses EEVDF as the new scheduler, which affects cpu scheduling
and cpuidle behavior. With kernel 6.6, the cpu which the read thread
resides has a greater probability in cpuidle (about 80%), while with
kernel 6.1, the probability is only about 20-30%. For other platforms,
this does not have a significant impact on read performance because the
cpuidle exit time is very short (for example, imx93 is about 60us). But
for imx8mq, this results in longer waits for the thread to be woken up
while reading eMMC/SD, which drops performance.

So for imx8mq, use the ESDHC_FLAG_PMQOS flag to request the cpu latency
QoS constraint. This can prevent entering cpuidle during data transfer.

Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Bough Chen Feb. 17, 2025, 11:52 a.m. UTC | #1 
> -----Original Message-----
> From: Luke Wang <ziniu.wang_1@nxp.com>
> Sent: 2025年2月17日 19:06
> To: adrian.hunter@intel.com; ulf.hansson@linaro.org
> Cc: Bough Chen <haibo.chen@nxp.com>; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> imx@lists.linux.dev; linux-mmc@vger.kernel.org; dl-S32 <S32@nxp.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [PATCH] mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read
> performance
> 
> From: Luke Wang <ziniu.wang_1@nxp.com>
> 
> Compared with kernel 6.1, imx8mq eMMC/SD read performance drops by about
> 30% with kernel 6.6.
> 
> The eMMC/SD read thread will be put to sleep until the hardware completes
> data transfer. Normally, the read thread will be woken up immediately when the
> data transfer is completed. However, due to a known ic bug, if imx8mq is in
> cpuidle, it will take a long time (about 500us) to exit cpuidle. As a result, the
> read thread cannot immediately read the next data block, affecting the read
> performance.
> 
> Kernel 6.6 uses EEVDF as the new scheduler, which affects cpu scheduling and
> cpuidle behavior. With kernel 6.6, the cpu which the read thread resides has a
> greater probability in cpuidle (about 80%), while with kernel 6.1, the probability
> is only about 20-30%. For other platforms, this does not have a significant impact
> on read performance because the cpuidle exit time is very short (for example,
> imx93 is about 60us). But for imx8mq, this results in longer waits for the thread
> to be woken up while reading eMMC/SD, which drops performance.
> 
> So for imx8mq, use the ESDHC_FLAG_PMQOS flag to request the cpu latency
> QoS constraint. This can prevent entering cpuidle during data transfer.

Reviewed-by: Haibo Chen <haibo.chen@nxp.com>

Regards
Haibo Chen
> 
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index ff78a7c6a04c..b3bf9c171d46 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -337,6 +337,15 @@ static struct esdhc_soc_data usdhc_imx8mm_data = {
>  	.quirks = SDHCI_QUIRK_NO_LED,
>  };
> 
> +static struct esdhc_soc_data usdhc_imx8mq_data = {
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> +			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> +			| ESDHC_FLAG_HS400 | ESDHC_FLAG_PMQOS
> +			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
> +			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
> +	.quirks = SDHCI_QUIRK_NO_LED,
> +};
> +
>  struct pltfm_imx_data {
>  	u32 scratchpad;
>  	struct pinctrl *pinctrl;
> @@ -381,6 +390,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = {
>  	{ .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, },
>  	{ .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, },
>  	{ .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, },
> +	{ .compatible = "fsl,imx8mq-usdhc", .data = &usdhc_imx8mq_data, },
>  	{ .compatible = "fsl,imxrt1050-usdhc", .data = &usdhc_imxrt1050_data, },
>  	{ .compatible = "nxp,s32g2-usdhc", .data = &usdhc_s32g2_data, },
>  	{ /* sentinel */ }
> --
> 2.34.1
Fabio Estevam Feb. 17, 2025, 11:56 a.m. UTC | #2 
On Mon, Feb 17, 2025 at 8:05 AM <ziniu.wang_1@nxp.com> wrote:
>
> From: Luke Wang <ziniu.wang_1@nxp.com>
>
> Compared with kernel 6.1, imx8mq eMMC/SD read performance drops by about
> 30% with kernel 6.6.

Since this fixes a significant performance regression, what about
adding a Fixes tag?
Lucas Stach Feb. 17, 2025, 11:58 a.m. UTC | #3 
Hi Luke,

Am Montag, dem 17.02.2025 um 19:06 +0800 schrieb ziniu.wang_1@nxp.com:
> From: Luke Wang <ziniu.wang_1@nxp.com>
> 
> Compared with kernel 6.1, imx8mq eMMC/SD read performance drops by about
> 30% with kernel 6.6.
> 
> The eMMC/SD read thread will be put to sleep until the hardware completes
> data transfer. Normally, the read thread will be woken up immediately
> when the data transfer is completed. However, due to a known ic bug, if
> imx8mq is in cpuidle, it will take a long time (about 500us) to exit
> cpuidle. As a result, the read thread cannot immediately read the next
> data block, affecting the read performance.
> 
Is this really a problem with the upstream kernel? i.MX8MQ upstream
does not use the deeper PSCI idle states, but only uses WFI, so I doubt
that upstream is affected by this issue.

Regards,
Lucas

> Kernel 6.6 uses EEVDF as the new scheduler, which affects cpu scheduling
> and cpuidle behavior. With kernel 6.6, the cpu which the read thread
> resides has a greater probability in cpuidle (about 80%), while with
> kernel 6.1, the probability is only about 20-30%. For other platforms,
> this does not have a significant impact on read performance because the
> cpuidle exit time is very short (for example, imx93 is about 60us). But
> for imx8mq, this results in longer waits for the thread to be woken up
> while reading eMMC/SD, which drops performance.
> 
> So for imx8mq, use the ESDHC_FLAG_PMQOS flag to request the cpu latency
> QoS constraint. This can prevent entering cpuidle during data transfer.
> 
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index ff78a7c6a04c..b3bf9c171d46 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -337,6 +337,15 @@ static struct esdhc_soc_data usdhc_imx8mm_data = {
>  	.quirks = SDHCI_QUIRK_NO_LED,
>  };
>  
> +static struct esdhc_soc_data usdhc_imx8mq_data = {
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> +			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> +			| ESDHC_FLAG_HS400 | ESDHC_FLAG_PMQOS
> +			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
> +			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
> +	.quirks = SDHCI_QUIRK_NO_LED,
> +};
> +
>  struct pltfm_imx_data {
>  	u32 scratchpad;
>  	struct pinctrl *pinctrl;
> @@ -381,6 +390,7 @@ static const struct of_device_id imx_esdhc_dt_ids[] = {
>  	{ .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, },
>  	{ .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, },
>  	{ .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, },
> +	{ .compatible = "fsl,imx8mq-usdhc", .data = &usdhc_imx8mq_data, },
>  	{ .compatible = "fsl,imxrt1050-usdhc", .data = &usdhc_imxrt1050_data, },
>  	{ .compatible = "nxp,s32g2-usdhc", .data = &usdhc_s32g2_data, },
>  	{ /* sentinel */ }
Christian Loehle Feb. 17, 2025, 12:02 p.m. UTC | #4 
On 2/17/25 11:58, Lucas Stach wrote:
> Hi Luke,
> 
> Am Montag, dem 17.02.2025 um 19:06 +0800 schrieb ziniu.wang_1@nxp.com:
>> From: Luke Wang <ziniu.wang_1@nxp.com>
>>
>> Compared with kernel 6.1, imx8mq eMMC/SD read performance drops by about
>> 30% with kernel 6.6.
>>
>> The eMMC/SD read thread will be put to sleep until the hardware completes
>> data transfer. Normally, the read thread will be woken up immediately
>> when the data transfer is completed. However, due to a known ic bug, if
>> imx8mq is in cpuidle, it will take a long time (about 500us) to exit
>> cpuidle. As a result, the read thread cannot immediately read the next
>> data block, affecting the read performance.
>>
> Is this really a problem with the upstream kernel? i.MX8MQ upstream
> does not use the deeper PSCI idle states, but only uses WFI, so I doubt
> that upstream is affected by this issue.
> 
> Regards,
> Lucas

Furthermore if that were to be the case the correct solution would probably
to have that reflected in the dts, too?
Luke Wang Feb. 18, 2025, 3:09 a.m. UTC | #5 
Hi Lucas,

You are right. 
This issue is observed on local kernel. I checked that the local kernel does use a deeper PSCI idle state (power off).
So, the upstream kernel will not be affected by this.

Thank you,
Luke

> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: Monday, February 17, 2025 7:58 PM
> To: Luke Wang <ziniu.wang_1@nxp.com>; adrian.hunter@intel.com;
> ulf.hansson@linaro.org
> Cc: imx@lists.linux.dev; dl-S32 <S32@nxp.com>; shawnguo@kernel.org;
> s.hauer@pengutronix.de; linux-mmc@vger.kernel.org; linux-
> kernel@vger.kernel.org; Bough Chen <haibo.chen@nxp.com>;
> kernel@pengutronix.de; festevam@gmail.com; linux-arm-
> kernel@lists.infradead.org
> Subject: [EXT] Re: [PATCH] mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd
> read performance
> 
> [You don't often get email from l.stach@pengutronix.de. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> Hi Luke,
> 
> Am Montag, dem 17.02.2025 um 19:06 +0800 schrieb
> ziniu.wang_1@nxp.com:
> > From: Luke Wang <ziniu.wang_1@nxp.com>
> >
> > Compared with kernel 6.1, imx8mq eMMC/SD read performance drops by
> about
> > 30% with kernel 6.6.
> >
> > The eMMC/SD read thread will be put to sleep until the hardware
> completes
> > data transfer. Normally, the read thread will be woken up immediately
> > when the data transfer is completed. However, due to a known ic bug, if
> > imx8mq is in cpuidle, it will take a long time (about 500us) to exit
> > cpuidle. As a result, the read thread cannot immediately read the next
> > data block, affecting the read performance.
> >
> Is this really a problem with the upstream kernel? i.MX8MQ upstream
> does not use the deeper PSCI idle states, but only uses WFI, so I doubt
> that upstream is affected by this issue.
> 
> Regards,
> Lucas
> 
> > Kernel 6.6 uses EEVDF as the new scheduler, which affects cpu scheduling
> > and cpuidle behavior. With kernel 6.6, the cpu which the read thread
> > resides has a greater probability in cpuidle (about 80%), while with
> > kernel 6.1, the probability is only about 20-30%. For other platforms,
> > this does not have a significant impact on read performance because the
> > cpuidle exit time is very short (for example, imx93 is about 60us). But
> > for imx8mq, this results in longer waits for the thread to be woken up
> > while reading eMMC/SD, which drops performance.
> >
> > So for imx8mq, use the ESDHC_FLAG_PMQOS flag to request the cpu
> latency
> > QoS constraint. This can prevent entering cpuidle during data transfer.
> >
> > Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-
> esdhc-imx.c
> > index ff78a7c6a04c..b3bf9c171d46 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -337,6 +337,15 @@ static struct esdhc_soc_data usdhc_imx8mm_data
> = {
> >       .quirks = SDHCI_QUIRK_NO_LED,
> >  };
> >
> > +static struct esdhc_soc_data usdhc_imx8mq_data = {
> > +     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> > +                     | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> > +                     | ESDHC_FLAG_HS400 | ESDHC_FLAG_PMQOS
> > +                     | ESDHC_FLAG_STATE_LOST_IN_LPMODE
> > +                     | ESDHC_FLAG_BROKEN_AUTO_CMD23,
> > +     .quirks = SDHCI_QUIRK_NO_LED,
> > +};
> > +
> >  struct pltfm_imx_data {
> >       u32 scratchpad;
> >       struct pinctrl *pinctrl;
> > @@ -381,6 +390,7 @@ static const struct of_device_id imx_esdhc_dt_ids[]
> = {
> >       { .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, },
> >       { .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, },
> >       { .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, },
> > +     { .compatible = "fsl,imx8mq-usdhc", .data = &usdhc_imx8mq_data, },
> >       { .compatible = "fsl,imxrt1050-usdhc", .data =
> &usdhc_imxrt1050_data, },
> >       { .compatible = "nxp,s32g2-usdhc", .data = &usdhc_s32g2_data, },
> >       { /* sentinel */ }
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index ff78a7c6a04c..b3bf9c171d46 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -337,6 +337,15 @@  static struct esdhc_soc_data usdhc_imx8mm_data = {
 	.quirks = SDHCI_QUIRK_NO_LED,
 };
 
+static struct esdhc_soc_data usdhc_imx8mq_data = {
+	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
+			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
+			| ESDHC_FLAG_HS400 | ESDHC_FLAG_PMQOS
+			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
+			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
+	.quirks = SDHCI_QUIRK_NO_LED,
+};
+
 struct pltfm_imx_data {
 	u32 scratchpad;
 	struct pinctrl *pinctrl;
@@ -381,6 +390,7 @@  static const struct of_device_id imx_esdhc_dt_ids[] = {
 	{ .compatible = "fsl,imx7ulp-usdhc", .data = &usdhc_imx7ulp_data, },
 	{ .compatible = "fsl,imx8qxp-usdhc", .data = &usdhc_imx8qxp_data, },
 	{ .compatible = "fsl,imx8mm-usdhc", .data = &usdhc_imx8mm_data, },
+	{ .compatible = "fsl,imx8mq-usdhc", .data = &usdhc_imx8mq_data, },
 	{ .compatible = "fsl,imxrt1050-usdhc", .data = &usdhc_imxrt1050_data, },
 	{ .compatible = "nxp,s32g2-usdhc", .data = &usdhc_s32g2_data, },
 	{ /* sentinel */ }