* [RESEND] mmc: sdhci-pci-gli: Improve Random 4K Read Performance of GL9763E
@ 2022-05-26 7:08 jasonlai.genesyslogic
2022-05-27 8:54 ` Adrian Hunter
0 siblings, 1 reply; 4+ messages in thread
From: jasonlai.genesyslogic @ 2022-05-26 7:08 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter
Cc: linux-mmc, ben.chuang, greg.tu, SeanHY.chen, jason.lai,
jasonlai.genesyslogic, victor.shih, Renius Chen
From: Jason Lai <jasonlai.genesyslogic@gmail.com>
Resend this patch due to code base updated to 5.18.0-rc3.
This patch is based on patch [1] and remove data transfer length checking.
Due to flaws in hardware design, GL9763E takes long time to exit from L1
state. The I/O performance will suffer severe impact if it often enter and
and exit L1 state.
Unfortunately, entering and exiting L1 state is signal handshake in
physical layer, software knows nothiong about it. The only way to stop
entering L1 state is to disable hardware LPM negotiation on GL9763E.
To improve read performance and take battery life into account, we reject
L1 negotiation while executing MMC_READ_MULTIPLE_BLOCK command and enable L1
negotiation again when receiving non-MMC_READ_MULTIPLE_BLOCK command.
[1]
https://patchwork.kernel.org/project/linux-mmc/list/?series=510801&archive
=both
Signed-off-by: Renius Chen <reniuschengl@gmail.com>
Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
---
drivers/mmc/host/sdhci-pci-gli.c | 60 +++++++++++++++++++++++++++++++-
1 file changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
index d09728c37d03..86200b73c0b0 100644
--- a/drivers/mmc/host/sdhci-pci-gli.c
+++ b/drivers/mmc/host/sdhci-pci-gli.c
@@ -95,9 +95,12 @@
#define PCIE_GLI_9763E_SCR 0x8E0
#define GLI_9763E_SCR_AXI_REQ BIT(9)
+#define PCIE_GLI_9763E_CFG 0x8A0
+#define GLI_9763E_CFG_LPSN_DIS BIT(12)
+
#define PCIE_GLI_9763E_CFG2 0x8A4
#define GLI_9763E_CFG2_L1DLY GENMASK(28, 19)
-#define GLI_9763E_CFG2_L1DLY_MID 0x54
+#define GLI_9763E_CFG2_L1DLY_MID 0x54 // Set L1 entry delay time to 21us
#define PCIE_GLI_9763E_MMC_CTRL 0x960
#define GLI_9763E_HS400_SLOW BIT(3)
@@ -144,6 +147,10 @@
#define GLI_MAX_TUNING_LOOP 40
+struct gli_host {
+ bool lpm_negotiation_enabled;
+};
+
/* Genesys Logic chipset */
static inline void gl9750_wt_on(struct sdhci_host *host)
{
@@ -818,6 +825,53 @@ static void sdhci_gl9763e_dumpregs(struct mmc_host *mmc)
sdhci_dumpregs(mmc_priv(mmc));
}
+static void gl9763e_set_low_power_negotiation(struct sdhci_pci_slot *slot, bool enable)
+{
+ struct pci_dev *pdev = slot->chip->pdev;
+ u32 value;
+
+ pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value);
+ value &= ~GLI_9763E_VHS_REV;
+ value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_W);
+ pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value);
+
+ pci_read_config_dword(pdev, PCIE_GLI_9763E_CFG, &value);
+
+ if (enable)
+ value &= ~GLI_9763E_CFG_LPSN_DIS;
+ else
+ value |= GLI_9763E_CFG_LPSN_DIS;
+
+ pci_write_config_dword(pdev, PCIE_GLI_9763E_CFG, value);
+
+ pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value);
+ value &= ~GLI_9763E_VHS_REV;
+ value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_R);
+ pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value);
+}
+
+static void gl9763e_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+ struct sdhci_host *host = mmc_priv(mmc);
+ struct mmc_command *cmd;
+ struct sdhci_pci_slot *slot = sdhci_priv(host);
+ struct gli_host *gli_host = sdhci_pci_priv(slot);
+
+ cmd = mrq->cmd;
+
+ if (cmd && (cmd->opcode == MMC_READ_MULTIPLE_BLOCK) && gli_host->lpm_negotiation_enabled) {
+ gl9763e_set_low_power_negotiation(slot, false);
+ gli_host->lpm_negotiation_enabled = false;
+ } else {
+ if (gli_host->lpm_negotiation_enabled == false) {
+ gl9763e_set_low_power_negotiation(slot, true);
+ gli_host->lpm_negotiation_enabled = true;
+ }
+ }
+
+ sdhci_request(mmc, mrq);
+}
+
static void sdhci_gl9763e_cqe_pre_enable(struct mmc_host *mmc)
{
struct cqhci_host *cq_host = mmc->cqe_private;
@@ -1016,6 +1070,9 @@ static int gli_probe_slot_gl9763e(struct sdhci_pci_slot *slot)
gli_pcie_enable_msi(slot);
host->mmc_host_ops.hs400_enhanced_strobe =
gl9763e_hs400_enhanced_strobe;
+
+ host->mmc_host_ops.request = gl9763e_request;
+
gli_set_gl9763e(slot);
sdhci_enable_v4_mode(host);
@@ -1109,4 +1166,5 @@ const struct sdhci_pci_fixes sdhci_gl9763e = {
.allow_runtime_pm = true,
#endif
.add_host = gl9763e_add_host,
+ .priv_size = sizeof(struct gli_host),
};
--
2.36.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RESEND] mmc: sdhci-pci-gli: Improve Random 4K Read Performance of GL9763E
2022-05-26 7:08 [RESEND] mmc: sdhci-pci-gli: Improve Random 4K Read Performance of GL9763E jasonlai.genesyslogic
@ 2022-05-27 8:54 ` Adrian Hunter
2022-05-30 3:11 ` Lai Jason
0 siblings, 1 reply; 4+ messages in thread
From: Adrian Hunter @ 2022-05-27 8:54 UTC (permalink / raw)
To: jasonlai.genesyslogic, ulf.hansson, adrian.hunter
Cc: linux-mmc, ben.chuang, greg.tu, SeanHY.chen, jason.lai,
victor.shih, Renius Chen
On 26/05/22 10:08, jasonlai.genesyslogic@gmail.com wrote:
> From: Jason Lai <jasonlai.genesyslogic@gmail.com>
>
> Resend this patch due to code base updated to 5.18.0-rc3.
>
> This patch is based on patch [1] and remove data transfer length checking.
>
> Due to flaws in hardware design, GL9763E takes long time to exit from L1
> state. The I/O performance will suffer severe impact if it often enter and
> and exit L1 state.
>
> Unfortunately, entering and exiting L1 state is signal handshake in
> physical layer, software knows nothiong about it. The only way to stop
> entering L1 state is to disable hardware LPM negotiation on GL9763E.
>
> To improve read performance and take battery life into account, we reject
> L1 negotiation while executing MMC_READ_MULTIPLE_BLOCK command and enable L1
> negotiation again when receiving non-MMC_READ_MULTIPLE_BLOCK command.
>
> [1]
> https://patchwork.kernel.org/project/linux-mmc/list/?series=510801&archive
> =both
Really needs Ulf's response, but a minor comment below.
>
> Signed-off-by: Renius Chen <reniuschengl@gmail.com>
> Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
> ---
> drivers/mmc/host/sdhci-pci-gli.c | 60 +++++++++++++++++++++++++++++++-
> 1 file changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
> index d09728c37d03..86200b73c0b0 100644
> --- a/drivers/mmc/host/sdhci-pci-gli.c
> +++ b/drivers/mmc/host/sdhci-pci-gli.c
> @@ -95,9 +95,12 @@
> #define PCIE_GLI_9763E_SCR 0x8E0
> #define GLI_9763E_SCR_AXI_REQ BIT(9)
>
> +#define PCIE_GLI_9763E_CFG 0x8A0
> +#define GLI_9763E_CFG_LPSN_DIS BIT(12)
> +
> #define PCIE_GLI_9763E_CFG2 0x8A4
> #define GLI_9763E_CFG2_L1DLY GENMASK(28, 19)
> -#define GLI_9763E_CFG2_L1DLY_MID 0x54
> +#define GLI_9763E_CFG2_L1DLY_MID 0x54 // Set L1 entry delay time to 21us
>
> #define PCIE_GLI_9763E_MMC_CTRL 0x960
> #define GLI_9763E_HS400_SLOW BIT(3)
> @@ -144,6 +147,10 @@
>
> #define GLI_MAX_TUNING_LOOP 40
>
> +struct gli_host {
> + bool lpm_negotiation_enabled;
> +};
> +
> /* Genesys Logic chipset */
> static inline void gl9750_wt_on(struct sdhci_host *host)
> {
> @@ -818,6 +825,53 @@ static void sdhci_gl9763e_dumpregs(struct mmc_host *mmc)
> sdhci_dumpregs(mmc_priv(mmc));
> }
>
> +static void gl9763e_set_low_power_negotiation(struct sdhci_pci_slot *slot, bool enable)
> +{
> + struct pci_dev *pdev = slot->chip->pdev;
> + u32 value;
> +
> + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value);
> + value &= ~GLI_9763E_VHS_REV;
> + value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_W);
> + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value);
> +
> + pci_read_config_dword(pdev, PCIE_GLI_9763E_CFG, &value);
> +
> + if (enable)
> + value &= ~GLI_9763E_CFG_LPSN_DIS;
> + else
> + value |= GLI_9763E_CFG_LPSN_DIS;
> +
> + pci_write_config_dword(pdev, PCIE_GLI_9763E_CFG, value);
> +
> + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value);
> + value &= ~GLI_9763E_VHS_REV;
> + value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_R);
> + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value);
> +}
> +
> +static void gl9763e_request(struct mmc_host *mmc, struct mmc_request *mrq)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct mmc_command *cmd;
> + struct sdhci_pci_slot *slot = sdhci_priv(host);
> + struct gli_host *gli_host = sdhci_pci_priv(slot);
> +
> + cmd = mrq->cmd;
> +
> + if (cmd && (cmd->opcode == MMC_READ_MULTIPLE_BLOCK) && gli_host->lpm_negotiation_enabled) {
> + gl9763e_set_low_power_negotiation(slot, false);
> + gli_host->lpm_negotiation_enabled = false;
> + } else {
> + if (gli_host->lpm_negotiation_enabled == false) {
Is this logic right? Wouldn't it also get here with
cmd->opcode == MMC_READ_MULTIPLE_BLOCK &&
gli_host->lpm_negotiation_enabled == false
and then you don't want the following?
> + gl9763e_set_low_power_negotiation(slot, true);
> + gli_host->lpm_negotiation_enabled = true;
> + }
> + }
> +
> + sdhci_request(mmc, mrq);
> +}
> +
> static void sdhci_gl9763e_cqe_pre_enable(struct mmc_host *mmc)
> {
> struct cqhci_host *cq_host = mmc->cqe_private;
> @@ -1016,6 +1070,9 @@ static int gli_probe_slot_gl9763e(struct sdhci_pci_slot *slot)
> gli_pcie_enable_msi(slot);
> host->mmc_host_ops.hs400_enhanced_strobe =
> gl9763e_hs400_enhanced_strobe;
> +
> + host->mmc_host_ops.request = gl9763e_request;
> +
> gli_set_gl9763e(slot);
> sdhci_enable_v4_mode(host);
>
> @@ -1109,4 +1166,5 @@ const struct sdhci_pci_fixes sdhci_gl9763e = {
> .allow_runtime_pm = true,
> #endif
> .add_host = gl9763e_add_host,
> + .priv_size = sizeof(struct gli_host),
> };
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RESEND] mmc: sdhci-pci-gli: Improve Random 4K Read Performance of GL9763E
2022-05-27 8:54 ` Adrian Hunter
@ 2022-05-30 3:11 ` Lai Jason
2022-05-30 4:46 ` Adrian Hunter
0 siblings, 1 reply; 4+ messages in thread
From: Lai Jason @ 2022-05-30 3:11 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Ben Chuang,
GregTu[杜啟軒],
SeanHY.chen, Jason Lai, victor.shih, Renius Chen
Hi Adrian,
Please see my reply below.
On Fri, May 27, 2022 at 4:54 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 26/05/22 10:08, jasonlai.genesyslogic@gmail.com wrote:
> > From: Jason Lai <jasonlai.genesyslogic@gmail.com>
> >
> > Resend this patch due to code base updated to 5.18.0-rc3.
> >
> > This patch is based on patch [1] and remove data transfer length checking.
> >
> > Due to flaws in hardware design, GL9763E takes long time to exit from L1
> > state. The I/O performance will suffer severe impact if it often enter and
> > and exit L1 state.
> >
> > Unfortunately, entering and exiting L1 state is signal handshake in
> > physical layer, software knows nothiong about it. The only way to stop
> > entering L1 state is to disable hardware LPM negotiation on GL9763E.
> >
> > To improve read performance and take battery life into account, we reject
> > L1 negotiation while executing MMC_READ_MULTIPLE_BLOCK command and enable L1
> > negotiation again when receiving non-MMC_READ_MULTIPLE_BLOCK command.
> >
> > [1]
> > https://patchwork.kernel.org/project/linux-mmc/list/?series=510801&archive
> > =both
>
> Really needs Ulf's response, but a minor comment below.
>
> >
> > Signed-off-by: Renius Chen <reniuschengl@gmail.com>
> > Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
> > ---
> > drivers/mmc/host/sdhci-pci-gli.c | 60 +++++++++++++++++++++++++++++++-
> > 1 file changed, 59 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
> > index d09728c37d03..86200b73c0b0 100644
> > --- a/drivers/mmc/host/sdhci-pci-gli.c
> > +++ b/drivers/mmc/host/sdhci-pci-gli.c
> > @@ -95,9 +95,12 @@
> > #define PCIE_GLI_9763E_SCR 0x8E0
> > #define GLI_9763E_SCR_AXI_REQ BIT(9)
> >
> > +#define PCIE_GLI_9763E_CFG 0x8A0
> > +#define GLI_9763E_CFG_LPSN_DIS BIT(12)
> > +
> > #define PCIE_GLI_9763E_CFG2 0x8A4
> > #define GLI_9763E_CFG2_L1DLY GENMASK(28, 19)
> > -#define GLI_9763E_CFG2_L1DLY_MID 0x54
> > +#define GLI_9763E_CFG2_L1DLY_MID 0x54 // Set L1 entry delay time to 21us
> >
> > #define PCIE_GLI_9763E_MMC_CTRL 0x960
> > #define GLI_9763E_HS400_SLOW BIT(3)
> > @@ -144,6 +147,10 @@
> >
> > #define GLI_MAX_TUNING_LOOP 40
> >
> > +struct gli_host {
> > + bool lpm_negotiation_enabled;
> > +};
> > +
> > /* Genesys Logic chipset */
> > static inline void gl9750_wt_on(struct sdhci_host *host)
> > {
> > @@ -818,6 +825,53 @@ static void sdhci_gl9763e_dumpregs(struct mmc_host *mmc)
> > sdhci_dumpregs(mmc_priv(mmc));
> > }
> >
> > +static void gl9763e_set_low_power_negotiation(struct sdhci_pci_slot *slot, bool enable)
> > +{
> > + struct pci_dev *pdev = slot->chip->pdev;
> > + u32 value;
> > +
> > + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value);
> > + value &= ~GLI_9763E_VHS_REV;
> > + value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_W);
> > + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value);
> > +
> > + pci_read_config_dword(pdev, PCIE_GLI_9763E_CFG, &value);
> > +
> > + if (enable)
> > + value &= ~GLI_9763E_CFG_LPSN_DIS;
> > + else
> > + value |= GLI_9763E_CFG_LPSN_DIS;
> > +
> > + pci_write_config_dword(pdev, PCIE_GLI_9763E_CFG, value);
> > +
> > + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value);
> > + value &= ~GLI_9763E_VHS_REV;
> > + value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_R);
> > + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value);
> > +}
> > +
> > +static void gl9763e_request(struct mmc_host *mmc, struct mmc_request *mrq)
> > +{
> > + struct sdhci_host *host = mmc_priv(mmc);
> > + struct mmc_command *cmd;
> > + struct sdhci_pci_slot *slot = sdhci_priv(host);
> > + struct gli_host *gli_host = sdhci_pci_priv(slot);
> > +
> > + cmd = mrq->cmd;
> > +
> > + if (cmd && (cmd->opcode == MMC_READ_MULTIPLE_BLOCK) && gli_host->lpm_negotiation_enabled) {
> > + gl9763e_set_low_power_negotiation(slot, false);
> > + gli_host->lpm_negotiation_enabled = false;
> > + } else {
> > + if (gli_host->lpm_negotiation_enabled == false) {
>
> Is this logic right? Wouldn't it also get here with
No, the logic is wrong. The original intention of my design is keeping LPM
negotiation disabled from arriving of READ_MULTIPLE_BLOCK command to
arriving of non-READ_MULTIPLE_BLOCK command. So The piece of code should be
written as below:
if (cmd && (cmd->opcode == MMC_READ_MULTIPLE_BLOCK)) {
if (gli_host->lpm_negotiation_enabled) {
gl9763e_set_low_power_negotiation(slot, false);
gli_host->lpm_negotiation_enabled = false;
}
} else {
if (gli_host->lpm_negotiation_enabled == false) {
gl9763e_set_low_power_negotiation(slot, true);
gli_host->lpm_negotiation_enabled = true;
}
}
Am I correct?
regards,
Jason Lai
> cmd->opcode == MMC_READ_MULTIPLE_BLOCK &&
> gli_host->lpm_negotiation_enabled == false
>
> and then you don't want the following?
>
> > + gl9763e_set_low_power_negotiation(slot, true);
> > + gli_host->lpm_negotiation_enabled = true;
> > + }
> > + }
> > +
> > + sdhci_request(mmc, mrq);
> > +}
> > +
> > static void sdhci_gl9763e_cqe_pre_enable(struct mmc_host *mmc)
> > {
> > struct cqhci_host *cq_host = mmc->cqe_private;
> > @@ -1016,6 +1070,9 @@ static int gli_probe_slot_gl9763e(struct sdhci_pci_slot *slot)
> > gli_pcie_enable_msi(slot);
> > host->mmc_host_ops.hs400_enhanced_strobe =
> > gl9763e_hs400_enhanced_strobe;
> > +
> > + host->mmc_host_ops.request = gl9763e_request;
> > +
> > gli_set_gl9763e(slot);
> > sdhci_enable_v4_mode(host);
> >
> > @@ -1109,4 +1166,5 @@ const struct sdhci_pci_fixes sdhci_gl9763e = {
> > .allow_runtime_pm = true,
> > #endif
> > .add_host = gl9763e_add_host,
> > + .priv_size = sizeof(struct gli_host),
> > };
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RESEND] mmc: sdhci-pci-gli: Improve Random 4K Read Performance of GL9763E
2022-05-30 3:11 ` Lai Jason
@ 2022-05-30 4:46 ` Adrian Hunter
0 siblings, 0 replies; 4+ messages in thread
From: Adrian Hunter @ 2022-05-30 4:46 UTC (permalink / raw)
To: Lai Jason
Cc: Ulf Hansson, linux-mmc, Ben Chuang,
GregTu[杜啟軒],
SeanHY.chen, Jason Lai, victor.shih, Renius Chen
On 30/05/22 06:11, Lai Jason wrote:
> Hi Adrian,
>
> Please see my reply below.
>
> On Fri, May 27, 2022 at 4:54 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> On 26/05/22 10:08, jasonlai.genesyslogic@gmail.com wrote:
>>> From: Jason Lai <jasonlai.genesyslogic@gmail.com>
>>>
>>> Resend this patch due to code base updated to 5.18.0-rc3.
>>>
>>> This patch is based on patch [1] and remove data transfer length checking.
>>>
>>> Due to flaws in hardware design, GL9763E takes long time to exit from L1
>>> state. The I/O performance will suffer severe impact if it often enter and
>>> and exit L1 state.
>>>
>>> Unfortunately, entering and exiting L1 state is signal handshake in
>>> physical layer, software knows nothiong about it. The only way to stop
>>> entering L1 state is to disable hardware LPM negotiation on GL9763E.
>>>
>>> To improve read performance and take battery life into account, we reject
>>> L1 negotiation while executing MMC_READ_MULTIPLE_BLOCK command and enable L1
>>> negotiation again when receiving non-MMC_READ_MULTIPLE_BLOCK command.
>>>
>>> [1]
>>> https://patchwork.kernel.org/project/linux-mmc/list/?series=510801&archive
>>> =both
>>
>> Really needs Ulf's response, but a minor comment below.
>>
>>>
>>> Signed-off-by: Renius Chen <reniuschengl@gmail.com>
>>> Signed-off-by: Jason Lai <jason.lai@genesyslogic.com.tw>
>>> ---
>>> drivers/mmc/host/sdhci-pci-gli.c | 60 +++++++++++++++++++++++++++++++-
>>> 1 file changed, 59 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
>>> index d09728c37d03..86200b73c0b0 100644
>>> --- a/drivers/mmc/host/sdhci-pci-gli.c
>>> +++ b/drivers/mmc/host/sdhci-pci-gli.c
>>> @@ -95,9 +95,12 @@
>>> #define PCIE_GLI_9763E_SCR 0x8E0
>>> #define GLI_9763E_SCR_AXI_REQ BIT(9)
>>>
>>> +#define PCIE_GLI_9763E_CFG 0x8A0
>>> +#define GLI_9763E_CFG_LPSN_DIS BIT(12)
>>> +
>>> #define PCIE_GLI_9763E_CFG2 0x8A4
>>> #define GLI_9763E_CFG2_L1DLY GENMASK(28, 19)
>>> -#define GLI_9763E_CFG2_L1DLY_MID 0x54
>>> +#define GLI_9763E_CFG2_L1DLY_MID 0x54 // Set L1 entry delay time to 21us
>>>
>>> #define PCIE_GLI_9763E_MMC_CTRL 0x960
>>> #define GLI_9763E_HS400_SLOW BIT(3)
>>> @@ -144,6 +147,10 @@
>>>
>>> #define GLI_MAX_TUNING_LOOP 40
>>>
>>> +struct gli_host {
>>> + bool lpm_negotiation_enabled;
>>> +};
>>> +
>>> /* Genesys Logic chipset */
>>> static inline void gl9750_wt_on(struct sdhci_host *host)
>>> {
>>> @@ -818,6 +825,53 @@ static void sdhci_gl9763e_dumpregs(struct mmc_host *mmc)
>>> sdhci_dumpregs(mmc_priv(mmc));
>>> }
>>>
>>> +static void gl9763e_set_low_power_negotiation(struct sdhci_pci_slot *slot, bool enable)
>>> +{
>>> + struct pci_dev *pdev = slot->chip->pdev;
>>> + u32 value;
>>> +
>>> + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value);
>>> + value &= ~GLI_9763E_VHS_REV;
>>> + value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_W);
>>> + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value);
>>> +
>>> + pci_read_config_dword(pdev, PCIE_GLI_9763E_CFG, &value);
>>> +
>>> + if (enable)
>>> + value &= ~GLI_9763E_CFG_LPSN_DIS;
>>> + else
>>> + value |= GLI_9763E_CFG_LPSN_DIS;
>>> +
>>> + pci_write_config_dword(pdev, PCIE_GLI_9763E_CFG, value);
>>> +
>>> + pci_read_config_dword(pdev, PCIE_GLI_9763E_VHS, &value);
>>> + value &= ~GLI_9763E_VHS_REV;
>>> + value |= FIELD_PREP(GLI_9763E_VHS_REV, GLI_9763E_VHS_REV_R);
>>> + pci_write_config_dword(pdev, PCIE_GLI_9763E_VHS, value);
>>> +}
>>> +
>>> +static void gl9763e_request(struct mmc_host *mmc, struct mmc_request *mrq)
>>> +{
>>> + struct sdhci_host *host = mmc_priv(mmc);
>>> + struct mmc_command *cmd;
>>> + struct sdhci_pci_slot *slot = sdhci_priv(host);
>>> + struct gli_host *gli_host = sdhci_pci_priv(slot);
>>> +
>>> + cmd = mrq->cmd;
>>> +
>>> + if (cmd && (cmd->opcode == MMC_READ_MULTIPLE_BLOCK) && gli_host->lpm_negotiation_enabled) {
>>> + gl9763e_set_low_power_negotiation(slot, false);
>>> + gli_host->lpm_negotiation_enabled = false;
>>> + } else {
>>> + if (gli_host->lpm_negotiation_enabled == false) {
>>
>> Is this logic right? Wouldn't it also get here with
>
> No, the logic is wrong. The original intention of my design is keeping LPM
> negotiation disabled from arriving of READ_MULTIPLE_BLOCK command to
> arriving of non-READ_MULTIPLE_BLOCK command. So The piece of code should be
> written as below:
> if (cmd && (cmd->opcode == MMC_READ_MULTIPLE_BLOCK)) {
> if (gli_host->lpm_negotiation_enabled) {
> gl9763e_set_low_power_negotiation(slot, false);
> gli_host->lpm_negotiation_enabled = false;
> }
> } else {
> if (gli_host->lpm_negotiation_enabled == false) {
> gl9763e_set_low_power_negotiation(slot, true);
> gli_host->lpm_negotiation_enabled = true;
> }
> }
>
> Am I correct?
Looks better. You might want to consider a wrapper function like:
static void gl9763e_set_lpm_negotiation(struct sdhci_pci_slot *slot, bool enable)
{
if (gli_host->lpm_negotiation_enabled == enable)
return;
gli_host->lpm_negotiation_enabled = enable;
gl9763e_set_low_power_negotiation(slot, enable);
}
>
> regards,
> Jason Lai
>
>> cmd->opcode == MMC_READ_MULTIPLE_BLOCK &&
>> gli_host->lpm_negotiation_enabled == false
>>
>> and then you don't want the following?
>>
>>> + gl9763e_set_low_power_negotiation(slot, true);
>>> + gli_host->lpm_negotiation_enabled = true;
>>> + }
>>> + }
>>> +
>>> + sdhci_request(mmc, mrq);
>>> +}
>>> +
>>> static void sdhci_gl9763e_cqe_pre_enable(struct mmc_host *mmc)
>>> {
>>> struct cqhci_host *cq_host = mmc->cqe_private;
>>> @@ -1016,6 +1070,9 @@ static int gli_probe_slot_gl9763e(struct sdhci_pci_slot *slot)
>>> gli_pcie_enable_msi(slot);
>>> host->mmc_host_ops.hs400_enhanced_strobe =
>>> gl9763e_hs400_enhanced_strobe;
>>> +
>>> + host->mmc_host_ops.request = gl9763e_request;
>>> +
>>> gli_set_gl9763e(slot);
>>> sdhci_enable_v4_mode(host);
>>>
>>> @@ -1109,4 +1166,5 @@ const struct sdhci_pci_fixes sdhci_gl9763e = {
>>> .allow_runtime_pm = true,
>>> #endif
>>> .add_host = gl9763e_add_host,
>>> + .priv_size = sizeof(struct gli_host),
>>> };
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-30 4:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-26 7:08 [RESEND] mmc: sdhci-pci-gli: Improve Random 4K Read Performance of GL9763E jasonlai.genesyslogic
2022-05-27 8:54 ` Adrian Hunter
2022-05-30 3:11 ` Lai Jason
2022-05-30 4:46 ` Adrian Hunter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.