On 26/05/22 10:08, jasonlai.genesyslogic@xxxxxxxxx wrote: > From: Jason Lai <jasonlai.genesyslogic@xxxxxxxxx> > > 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@xxxxxxxxx> > Signed-off-by: Jason Lai <jason.lai@xxxxxxxxxxxxxxxxxxx> > --- > 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), > };
