Linux MultiMedia Card development
 help / color / mirror / Atom feed
* [PATCH] mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read performance
@ 2025-02-17 11:06 ziniu.wang_1
  2025-02-17 11:52 ` Bough Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: ziniu.wang_1 @ 2025-02-17 11:06 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson
  Cc: haibo.chen, shawnguo, s.hauer, kernel, festevam, imx, linux-mmc,
	s32, linux-arm-kernel, linux-kernel

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(+)

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH] mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read performance
  2025-02-17 11:06 [PATCH] mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read performance ziniu.wang_1
@ 2025-02-17 11:52 ` Bough Chen
  2025-02-17 11:56 ` Fabio Estevam
  2025-02-17 11:58 ` Lucas Stach
  2 siblings, 0 replies; 6+ messages in thread
From: Bough Chen @ 2025-02-17 11:52 UTC (permalink / raw)
  To: Luke Wang, adrian.hunter@intel.com, ulf.hansson@linaro.org
  Cc: shawnguo@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, imx@lists.linux.dev,
	linux-mmc@vger.kernel.org, dl-S32,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org

> -----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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read performance
  2025-02-17 11:06 [PATCH] mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read performance ziniu.wang_1
  2025-02-17 11:52 ` Bough Chen
@ 2025-02-17 11:56 ` Fabio Estevam
  2025-02-17 11:58 ` Lucas Stach
  2 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2025-02-17 11:56 UTC (permalink / raw)
  To: ziniu.wang_1
  Cc: adrian.hunter, ulf.hansson, haibo.chen, shawnguo, s.hauer, kernel,
	imx, linux-mmc, s32, linux-arm-kernel, linux-kernel

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?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read performance
  2025-02-17 11:06 [PATCH] mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read performance ziniu.wang_1
  2025-02-17 11:52 ` Bough Chen
  2025-02-17 11:56 ` Fabio Estevam
@ 2025-02-17 11:58 ` Lucas Stach
  2025-02-17 12:02   ` Christian Loehle
  2025-02-18  3:09   ` [EXT] " Luke Wang
  2 siblings, 2 replies; 6+ messages in thread
From: Lucas Stach @ 2025-02-17 11:58 UTC (permalink / raw)
  To: ziniu.wang_1, adrian.hunter, ulf.hansson
  Cc: imx, s32, shawnguo, s.hauer, linux-mmc, linux-kernel, haibo.chen,
	kernel, festevam, linux-arm-kernel

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 */ }


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read performance
  2025-02-17 11:58 ` Lucas Stach
@ 2025-02-17 12:02   ` Christian Loehle
  2025-02-18  3:09   ` [EXT] " Luke Wang
  1 sibling, 0 replies; 6+ messages in thread
From: Christian Loehle @ 2025-02-17 12:02 UTC (permalink / raw)
  To: Lucas Stach, ziniu.wang_1, adrian.hunter, ulf.hansson
  Cc: imx, s32, shawnguo, s.hauer, linux-mmc, linux-kernel, haibo.chen,
	kernel, festevam, linux-arm-kernel

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?


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [EXT] Re: [PATCH] mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read performance
  2025-02-17 11:58 ` Lucas Stach
  2025-02-17 12:02   ` Christian Loehle
@ 2025-02-18  3:09   ` Luke Wang
  1 sibling, 0 replies; 6+ messages in thread
From: Luke Wang @ 2025-02-18  3:09 UTC (permalink / raw)
  To: Lucas Stach, adrian.hunter@intel.com, ulf.hansson@linaro.org
  Cc: imx@lists.linux.dev, dl-S32, shawnguo@kernel.org,
	s.hauer@pengutronix.de, linux-mmc@vger.kernel.org,
	linux-kernel@vger.kernel.org, Bough Chen, kernel@pengutronix.de,
	festevam@gmail.com, linux-arm-kernel@lists.infradead.org

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 */ }


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-02-18  3:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-17 11:06 [PATCH] mmc: sdhci-esdhc-imx: improve imx8mq emmc/sd read performance ziniu.wang_1
2025-02-17 11:52 ` Bough Chen
2025-02-17 11:56 ` Fabio Estevam
2025-02-17 11:58 ` Lucas Stach
2025-02-17 12:02   ` Christian Loehle
2025-02-18  3:09   ` [EXT] " Luke Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox