[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <533b63754b8e4476a799d4b690e37c3e@quicinc.com>
Date: Tue, 31 May 2022 09:23:59 +0000
From: "Sarthak Garg (QUIC)" <quic_sartgarg@...cinc.com>
To: "Kamasali Satyanarayan (Consultant) (QUIC)"
<quic_kamasali@...cinc.com>, 'Ulf Hansson' <ulf.hansson@...aro.org>
CC: "avri.altman@....com" <avri.altman@....com>,
"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
"shawn.lin@...k-chips.com" <shawn.lin@...k-chips.com>,
"s.shtylyov@....ru" <s.shtylyov@....ru>,
"huijin.park@...sung.com" <huijin.park@...sung.com>,
"briannorris@...omium.org" <briannorris@...omium.org>,
"digetx@...il.com" <digetx@...il.com>,
"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH V1] mmc: core: Add partial initialization support
Hi ulf,
Did you get a chance to check the numbers ?
Thanks,
Sarthak
> -----Original Message-----
> From: Sarthak Garg (QUIC)
> Sent: Tuesday, May 24, 2022 8:07 PM
> To: Kamasali Satyanarayan (Consultant) (QUIC) <quic_kamasali@...cinc.com>;
> 'Ulf Hansson' <ulf.hansson@...aro.org>
> Cc: avri.altman@....com; linus.walleij@...aro.org; shawn.lin@...k-chips.com;
> merez@...eaurora.org; s.shtylyov@....ru; huijin.park@...sung.com;
> briannorris@...omium.org; digetx@...il.com; linux-mmc@...r.kernel.org;
> linux-kernel@...r.kernel.org; Veerabhadrarao Badiganti
> <vbadigan@...eaurora.org>; Shaik Sajida Bhanu <sbhanu@...eaurora.org>;
> 'quic_spathi@...cinc.com' <quic_spathi@...cinc.com>
> Subject: RE: [PATCH V1] mmc: core: Add partial initialization support
>
> Hi Ulf,
>
> Please find the gains seen on micron and kingston eMMC parts below with
> partial initialization feature (These are the averaged numbers) :
>
> 1) Micron eMMC (ManfID 0x13)
>
> Partial init Full Init
>
> a) _mmc_resume: _mmc_resume :
>
> Total time : 62ms Total time : 84ms
> (Decrease % from full init = ~26%)
>
> Breakup :
> mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.1ms
> mmc_power_up_time: 33ms mmc_power_up_time: 33ms
> mmc_sleepawake_time: 28ms mmc_init_card_time: 50ms
> mmc_partial_init_time: 1ms
>
> b) _mmc_suspend: _mmc_suspend:
>
> Total time: 5ms Total time: 7.5ms
> mmc_claim_host_time: 0.5ms mmc_claim_host_time: 1ms
> mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 2.5 ms
> mmc_sleep_time: 1.5ms mmc_sleep_time: 2ms
> mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms
>
>
> 2) Kingston eMMC (ManfID 0x70)
>
> Partial init Full Init
>
> a) _mmc_resume: _mmc_resume :
> Total time : 46ms Total time : 62ms
> (Decrease % from full init = ~25%)
>
> Breakup :
> mmc_claim_host_time: 0.2ms mmc_claim_host_time: 0.2ms
> mmc_power_up_time: 30ms mmc_power_up_time: 30ms
> mmc_sleepawake_time: 14ms mmc_init_card_time: 31ms
> mmc_partial_init_time: 2ms
>
>
> b) _mmc_suspend: _mmc_suspend:
> Total time : 5ms Total: 5ms
>
> Breakup :
> mmc_claim_host_time: 0.5ms mmc_claim_host_time: 0.5ms
> mmc_flush_cache_time : 1.5 ms mmc_flush_cache_time : 1.5 ms
> mmc_sleep_time: 1.5ms mmc_sleep_time: 1ms
> mmc_power_off_time: 1.5ms mmc_power_off_time: 1.5ms
>
> Did some minor modifications as well to this patchset as per avri's comment
> which I'll post as V2.
> Please let me know your inputs about these numbers.
>
> Thanks,
> Sarthak
>
> > -----Original Message-----
> > From: Kamasali Satyanarayan (Consultant) (QUIC)
> > <quic_kamasali@...cinc.com>
> > Sent: Tuesday, May 24, 2022 5:29 PM
> > To: 'Ulf Hansson' <ulf.hansson@...aro.org>; Sarthak Garg (QUIC)
> > <quic_sartgarg@...cinc.com>
> > Cc: avri.altman@....com; linus.walleij@...aro.org;
> > shawn.lin@...k-chips.com; merez@...eaurora.org; s.shtylyov@....ru;
> > huijin.park@...sung.com; briannorris@...omium.org; digetx@...il.com;
> > linux-mmc@...r.kernel.org; linux-kernel@...r.kernel.org;
> > Veerabhadrarao Badiganti <vbadigan@...eaurora.org>; Shaik Sajida Bhanu
> > <sbhanu@...eaurora.org>; 'quic_spathi@...cinc.com'
> > <quic_spathi@...cinc.com>
> > Subject: RE: [PATCH V1] mmc: core: Add partial initialization support
> >
> > Hi,
> >
> > These patches will be further taken by Sarthak.
> >
> > Thanks,
> > Satya
> >
> > -----Original Message-----
> > From: Ulf Hansson <ulf.hansson@...aro.org>
> > Sent: Wednesday, April 27, 2022 1:06 PM
> > To: quic_spathi <quic_spathi@...cinc.com>
> > Cc: avri.altman@....com; linus.walleij@...aro.org;
> > shawn.lin@...k-chips.com; merez@...eaurora.org; s.shtylyov@....ru;
> > huijin.park@...sung.com; briannorris@...omium.org; digetx@...il.com;
> > linux-mmc@...r.kernel.org; linux-kernel@...r.kernel.org;
> > Veerabhadrarao Badiganti <vbadigan@...eaurora.org>; Shaik Sajida Bhanu
> > <sbhanu@...eaurora.org>; Kamasali Satyanarayan (Consultant) (QUIC)
> > <quic_kamasali@...cinc.com>
> > Subject: Re: [PATCH V1] mmc: core: Add partial initialization support
> >
> > On Tue, 26 Apr 2022 at 11:04, Srinivasarao Pathipati
> > <quic_spathi@...cinc.com>
> > wrote:
> > >
> > > From: Maya Erez <merez@...eaurora.org>
> > >
> > > This change adds the ability to partially initialize the MMC card by
> > > using card Sleep/Awake sequence (CMD5).
> > > Card will be sent to Sleep state during runtime/system suspend and
> > > will be woken up during runtime/system resume.
> > > By using this sequence the card doesn't need full initialization
> > > which gives time reduction in system/runtime resume path.
> > >
> > > Signed-off-by: Maya Erez <merez@...eaurora.org>
> > > Signed-off-by: Veerabhadrarao Badiganti <vbadigan@...eaurora.org>
> > > Signed-off-by: Shaik Sajida Bhanu <sbhanu@...eaurora.org>
> > > Signed-off-by: kamasali <quic_kamasali@...cinc.com>
> > > Signed-off-by: Srinivasarao Pathipati <quic_spathi@...cinc.com>
> >
> > It seems like this patch has been posted before [1]. Let me repeat my
> > question sent back then.
> >
> > It would be nice if you could provide some more exact numbers of what
> > the gain is for a couple of different eMMCs, to justify the change.
> > Can you please do that?
> >
> > Kind regards
> > Uffe
> >
> > [1]
> > https://patchwork.kernel.org/project/linux-mmc/patch/1591277381-7734-1
> > - git-send-email-vbadigan@...eaurora.org/
> >
> > > ---
> > > drivers/mmc/core/mmc.c | 149
> > ++++++++++++++++++++++++++++++++++++++++++++---
> > > include/linux/mmc/card.h | 4 ++
> > > include/linux/mmc/host.h | 2 +
> > > 3 files changed, 146 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
> > > 9ab915b..8691c00 100644
> > > --- a/drivers/mmc/core/mmc.c
> > > +++ b/drivers/mmc/core/mmc.c
> > > @@ -1942,7 +1942,14 @@ static int mmc_sleep_busy_cb(void *cb_data,
> > > bool
> > *busy)
> > > return 0;
> > > }
> > >
> > > -static int mmc_sleep(struct mmc_host *host)
> > > +static int mmc_can_sleepawake(struct mmc_host *host) {
> > > + return host && (host->caps2 & MMC_CAP2_SLEEP_AWAKE) && host-
> > >card &&
> > > + (host->card->ext_csd.rev >= 3);
> > > +
> > > +}
> > > +
> > > +static int mmc_sleepawake(struct mmc_host *host, bool sleep)
> > > {
> > > struct mmc_command cmd = {};
> > > struct mmc_card *card = host->card; @@ -1953,14 +1960,17 @@
> > > static int mmc_sleep(struct mmc_host *host)
> > > /* Re-tuning can't be done once the card is deselected */
> > > mmc_retune_hold(host);
> > >
> > > - err = mmc_deselect_cards(host);
> > > - if (err)
> > > - goto out_release;
> > > + if (sleep) {
> > > + err = mmc_deselect_cards(host);
> > > + if (err)
> > > + goto out_release;
> > > + }
> > >
> > > cmd.opcode = MMC_SLEEP_AWAKE;
> > > cmd.arg = card->rca << 16;
> > > - cmd.arg |= 1 << 15;
> > > use_r1b_resp = mmc_prepare_busy_cmd(host, &cmd, timeout_ms);
> > > + if (sleep)
> > > + cmd.arg |= 1 << 15;
> > >
> > > err = mmc_wait_for_cmd(host, &cmd, 0);
> > > if (err)
> > > @@ -1982,6 +1992,9 @@ static int mmc_sleep(struct mmc_host *host)
> > >
> > > err = __mmc_poll_for_busy(host, 0, timeout_ms,
> > > &mmc_sleep_busy_cb, host);
> > >
> > > + if (!sleep)
> > > + err = mmc_select_card(card);
> > > +
> > > out_release:
> > > mmc_retune_release(host);
> > > return err;
> > > @@ -2080,6 +2093,66 @@ static int _mmc_flush_cache(struct mmc_host
> > *host)
> > > pr_err("%s: cache flush error %d\n",
> > > mmc_hostname(host), err);
> > > }
> > > + return err;
> > > +}
> > > +
> > > +static int mmc_cache_card_ext_csd(struct mmc_host *host) {
> > > + int err;
> > > + u8 *ext_csd;
> > > + struct mmc_card *card = host->card;
> > > +
> > > + err = mmc_get_ext_csd(card, &ext_csd);
> > > + if (err || !ext_csd) {
> > > + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> > > + mmc_hostname(host), __func__, err);
> > > + return err;
> > > + }
> > > + /* only cache read/write fields that the sw changes */
> > > + card->ext_csd.raw_ext_csd_cmdq =
> > ext_csd[EXT_CSD_CMDQ_MODE_EN];
> > > + card->ext_csd.raw_ext_csd_cache_ctrl =
> > ext_csd[EXT_CSD_CACHE_CTRL];
> > > + card->ext_csd.raw_ext_csd_bus_width =
> > ext_csd[EXT_CSD_BUS_WIDTH];
> > > + card->ext_csd.raw_ext_csd_hs_timing =
> > > + ext_csd[EXT_CSD_HS_TIMING];
> > > +
> > > + kfree(ext_csd);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int mmc_test_awake_ext_csd(struct mmc_host *host) {
> > > + int err;
> > > + u8 *ext_csd;
> > > + struct mmc_card *card = host->card;
> > > +
> > > + err = mmc_get_ext_csd(card, &ext_csd);
> > > + if (err) {
> > > + pr_err("%s: %s: mmc_get_ext_csd failed (%d)\n",
> > > + mmc_hostname(host), __func__, err);
> > > + return err;
> > > + }
> > > +
> > > + /* only compare read/write fields that the sw changes */
> > > + pr_debug("%s: %s: type(cached:current) cmdq(%d:%d)
> > > + cache_ctrl(%d:%d)
> > bus_width (%d:%d) timing(%d:%d)\n",
> > > + mmc_hostname(host), __func__,
> > > + card->ext_csd.raw_ext_csd_cmdq,
> > > + ext_csd[EXT_CSD_CMDQ_MODE_EN],
> > > + card->ext_csd.raw_ext_csd_cache_ctrl,
> > > + ext_csd[EXT_CSD_CACHE_CTRL],
> > > + card->ext_csd.raw_ext_csd_bus_width,
> > > + ext_csd[EXT_CSD_BUS_WIDTH],
> > > + card->ext_csd.raw_ext_csd_hs_timing,
> > > + ext_csd[EXT_CSD_HS_TIMING]);
> > > + err = !((card->ext_csd.raw_ext_csd_cmdq ==
> > > + ext_csd[EXT_CSD_CMDQ_MODE_EN]) &&
> > > + (card->ext_csd.raw_ext_csd_cache_ctrl ==
> > > + ext_csd[EXT_CSD_CACHE_CTRL]) &&
> > > + (card->ext_csd.raw_ext_csd_bus_width ==
> > > + ext_csd[EXT_CSD_BUS_WIDTH]) &&
> > > + (card->ext_csd.raw_ext_csd_hs_timing ==
> > > + ext_csd[EXT_CSD_HS_TIMING]));
> > > +
> > > + kfree(ext_csd);
> > >
> > > return err;
> > > }
> > > @@ -2103,8 +2176,12 @@ static int _mmc_suspend(struct mmc_host
> > > *host,
> > bool is_suspend)
> > > ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend ||
> > > (host->caps2 & MMC_CAP2_FULL_PWR_CYCLE_IN_SUSPEND)))
> > > err = mmc_poweroff_notify(host->card, notify_type);
> > > - else if (mmc_can_sleep(host->card))
> > > - err = mmc_sleep(host);
> > > + if (mmc_can_sleepawake(host)) {
> > > + memcpy(&host->cached_ios, &host->ios, sizeof(host-
> >cached_ios));
> > > + mmc_cache_card_ext_csd(host);
> > > + }
> > > + if (mmc_can_sleep(host->card))
> > > + err = mmc_sleepawake(host, true);
> > > else if (!mmc_host_is_spi(host))
> > > err = mmc_deselect_cards(host);
> > >
> > > @@ -2117,6 +2194,48 @@ static int _mmc_suspend(struct mmc_host
> > > *host,
> > bool is_suspend)
> > > return err;
> > > }
> > >
> > > +static int mmc_partial_init(struct mmc_host *host) {
> > > + int err = 0;
> > > + struct mmc_card *card = host->card;
> > > +
> > > + mmc_set_bus_width(host, host->cached_ios.bus_width);
> > > + mmc_set_timing(host, host->cached_ios.timing);
> > > + if (host->cached_ios.enhanced_strobe) {
> > > + host->ios.enhanced_strobe = true;
> > > + if (host->ops->hs400_enhanced_strobe)
> > > + host->ops->hs400_enhanced_strobe(host, &host->ios);
> > > + }
> > > + mmc_set_clock(host, host->cached_ios.clock);
> > > + mmc_set_bus_mode(host, host->cached_ios.bus_mode);
> > > +
> > > + if (!mmc_card_hs400es(card) &&
> > > + (mmc_card_hs200(card) || mmc_card_hs400(card))) {
> > > + err = mmc_execute_tuning(card);
> > > + if (err) {
> > > + pr_err("%s: %s: Tuning failed (%d)\n",
> > > + mmc_hostname(host), __func__, err);
> > > + goto out;
> > > + }
> > > + }
> > > + /*
> > > + * The ext_csd is read to make sure the card did not went through
> > > + * Power-failure during sleep period.
> > > + * A subset of the W/E_P, W/C_P register will be tested. In case
> > > + * these registers values are different from the values that were
> > > + * cached during suspend, we will conclude that a Power-failure
> occurred
> > > + * and will do full initialization sequence.
> > > + */
> > > + err = mmc_test_awake_ext_csd(host);
> > > + if (err) {
> > > + pr_debug("%s: %s: fail on ext_csd read (%d)\n",
> > > + mmc_hostname(host), __func__, err);
> > > + goto out;
> > > + }
> > > +out:
> > > + return err;
> > > +}
> > > +
> > > /*
> > > * Suspend callback
> > > */
> > > @@ -2139,7 +2258,7 @@ static int mmc_suspend(struct mmc_host *host)
> > > */
> > > static int _mmc_resume(struct mmc_host *host) {
> > > - int err = 0;
> > > + int err = -EINVAL;
> > >
> > > mmc_claim_host(host);
> > >
> > > @@ -2147,7 +2266,19 @@ static int _mmc_resume(struct mmc_host *host)
> > > goto out;
> > >
> > > mmc_power_up(host, host->card->ocr);
> > > - err = mmc_init_card(host, host->card->ocr, host->card);
> > > +
> > > + if (mmc_can_sleepawake(host)) {
> > > + err = mmc_sleepawake(host, false);
> > > + if (!err)
> > > + err = mmc_partial_init(host);
> > > + else
> > > + pr_err("%s: %s: awake failed (%d), fallback to full init\n",
> > > + mmc_hostname(host), __func__, err);
> > > + }
> > > +
> > > + if (err)
> > > + err = mmc_init_card(host, host->card->ocr,
> > > + host->card);
> > > +
> > > mmc_card_clr_suspended(host->card);
> > >
> > > out:
> > > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> > > index
> > > 37f9758..ed7f6f7 100644
> > > --- a/include/linux/mmc/card.h
> > > +++ b/include/linux/mmc/card.h
> > > @@ -86,6 +86,8 @@ struct mmc_ext_csd {
> > > unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
> > > unsigned int boot_ro_lock; /* ro lock support */
> > > bool boot_ro_lockable;
> > > + u8 raw_ext_csd_cmdq; /* 15 */
> > > + u8 raw_ext_csd_cache_ctrl; /* 33 */
> > > bool ffu_capable; /* Firmware upgrade support */
> > > bool cmdq_en; /* Command Queue enabled */
> > > bool cmdq_support; /* Command Queue supported */
> > > @@ -96,7 +98,9 @@ struct mmc_ext_csd {
> > > u8 raw_partition_support; /* 160 */
> > > u8 raw_rpmb_size_mult; /* 168 */
> > > u8 raw_erased_mem_count; /* 181 */
> > > + u8 raw_ext_csd_bus_width; /* 183 */
> > > u8 strobe_support; /* 184 */
> > > + u8 raw_ext_csd_hs_timing; /* 185 */
> > > u8 raw_ext_csd_structure; /* 194 */
> > > u8 raw_card_type; /* 196 */
> > > u8 raw_driver_strength; /* 197 */
> > > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> > > index c38072e..a9ddf7a 100644
> > > --- a/include/linux/mmc/host.h
> > > +++ b/include/linux/mmc/host.h
> > > @@ -422,6 +422,7 @@ struct mmc_host { #else
> > > #define MMC_CAP2_CRYPTO 0
> > > #endif
> > > +#define MMC_CAP2_SLEEP_AWAKE (1 << 29) /* Use Sleep/Awake
> > (CMD5) */
> > > #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28) /* Host with eMMC that
> > has GPT entry at a non-standard location */
> > >
> > > int fixed_drv_type; /* fixed driver type for non-removable
> > media */
> > > @@ -441,6 +442,7 @@ struct mmc_host {
> > > spinlock_t lock; /* lock for claim and bus ops */
> > >
> > > struct mmc_ios ios; /* current io bus settings */
> > > + struct mmc_ios cached_ios;
> > >
> > > /* group bitfields together to minimize padding */
> > > unsigned int use_spi_crc:1;
> > > --
> > > 2.7.4
> > >
Powered by blists - more mailing lists