| Message ID | 1465182439-27963-1-git-send-email-shawn.lin@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Shawn, Is the intention in this patch to stop using MANUAL_BKOPS for all eMMC5.1+ devices, and only use AUTO_BKOPS? Please see several questions inline. On 6/6/16, 6:07 AM, "linux-mmc-owner@vger.kernel.org on behalf of Shawn Lin" <linux-mmc-owner@vger.kernel.org on behalf of shawn.lin@rock-chips.com> wrote: >JEDEC eMMC v5.1 introduce an autonomously initiated method >for background operations. > >Host that wants to enable the device to perform background >operations during device idle time, should signal the device >by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When >this bit is set, the device may start or stop background operations >whenever it sees fit, without any notification to the host. > >When AUTO_EN bit is set, the host should keep the device power >active. The host may set or clear this bit at any time based on >its power constraints or other considerations. > >Currently the manual bkops is only be used under the async req >circumstances and it's a bit complicated to be controlled as the >perfect method is that we should do some idle monitor just as rpm >and send HPI each time if receiving rd/wr req. But it will impact >performance significantly, especially for random iops since the >weight of executing HPI against r/w small piece of LBAs is >nonnegligible. > >So we now prefer to select the auto one unconditionally if supported >which makes it as simple as possible. It should really good enough >for devices to manage its internal policy for bkops rather than the >host, which makes us believe that we could achieve the best >performance for all the devices implementing auto bkops and the only >thing we should do is to disable it when cutting off the power. > >Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> >--- > > drivers/mmc/core/core.c | 127 +++++++++++++++++++++++++++++++++++++---------- > drivers/mmc/core/mmc.c | 30 ++++++++++- > include/linux/mmc/card.h | 1 + > include/linux/mmc/core.h | 4 +- > include/linux/mmc/mmc.h | 1 + > 5 files changed, 133 insertions(+), 30 deletions(-) > >diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >index e864187..c2e5a66 100644 >--- a/drivers/mmc/core/core.c >+++ b/drivers/mmc/core/core.c >@@ -297,24 +297,13 @@ static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq) > return 0; > } > >-/** >- * mmc_start_bkops - start BKOPS for supported cards >- * @card: MMC card to start BKOPS >- * @form_exception: A flag to indicate if this function was >- * called due to an exception raised by the card >- * >- * Start background operations whenever requested. >- * When the urgent BKOPS bit is set in a R1 command response >- * then background operations should be started immediately. >-*/ >-void mmc_start_bkops(struct mmc_card *card, bool from_exception) >+static void mmc_start_man_bkops(struct mmc_card *card, >+ bool from_exception) > { > int err; > int timeout; > bool use_busy_signal; > >- BUG_ON(!card); >- > if (!card->ext_csd.man_bkops_en || mmc_card_doing_bkops(card)) > return; > >@@ -347,7 +336,7 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception) > EXT_CSD_BKOPS_START, 1, timeout, > use_busy_signal, true, false); > if (err) { >- pr_warn("%s: Error %d starting bkops\n", >+ pr_warn("%s: Error %d starting manual bkops\n", > mmc_hostname(card->host), err); > mmc_retune_release(card->host); > goto out; >@@ -365,6 +354,55 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception) > out: > mmc_release_host(card->host); > } >+ >+static void mmc_start_auto_bkops(struct mmc_card *card) >+{ >+ int err; >+ >+ /* If it's already enable auto_bkops, nothing to do */ >+ if (card->ext_csd.auto_bkops_en) >+ return; >+ >+ mmc_claim_host(card->host); >+ >+ mmc_retune_hold(card->host); >+ >+ err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >+ EXT_CSD_BKOPS_EN, 2, >+ card->ext_csd.generic_cmd6_time); >+ if (err) >+ pr_warn("%s: Error %d starting auto bkops\n", >+ mmc_hostname(card->host), err); >+ >+ card->ext_csd.auto_bkops_en = true; >+ mmc_card_set_doing_bkops(card); >+ mmc_retune_release(card->host); >+ mmc_release_host(card->host); >+} >+ >+ >+/** >+ * mmc_start_bkops - start BKOPS for supported cards >+ * @card: MMC card to start BKOPS >+ * @form_exception: A flag to indicate if this function was >+ * called due to an exception raised by the card >+ * @is_auto: A flag to indicate if we should use auto bkops >+ * >+ * Start background operations whenever requested. >+ * When the urgent BKOPS bit is set in a R1 command response >+ * then background operations should be started immediately, which is >+ * only needed for man_bkops. >+*/ >+void mmc_start_bkops(struct mmc_card *card, bool from_exception, >+ bool is_auto) >+{ >+ BUG_ON(!card); >+ >+ if (is_auto) As far as I understand, the AUTO_BKOPS is enabled during “mmc_init_card()”. If its correct, why the “mmc_start_auto_bkops()” function is needed? >+ return mmc_start_auto_bkops(card); >+ else >+ return mmc_start_man_bkops(card, from_exception); >+} > EXPORT_SYMBOL(mmc_start_bkops); > > /* >@@ -610,7 +648,9 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host, > if (areq) > mmc_post_req(host, areq->mrq, -EINVAL); > >- mmc_start_bkops(host->card, true); >+ /* Prefer to use auto bkops for eMMC 5.1 or later */ If the decision is to use AUTO_BKOPS for all eMMC5.1+ devices, why you need to enable AUTO_BKOPS bit on every R1_EXCEPTION_EVENT? AUTO_BKOPS should be enabled only once... >+ mmc_start_bkops(host->card, true, >+ host->card->ext_csd.rev >= 8); > > /* prepare the request again */ > if (areq) >@@ -751,20 +791,10 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries > > EXPORT_SYMBOL(mmc_wait_for_cmd); > >-/** >- * mmc_stop_bkops - stop ongoing BKOPS >- * @card: MMC card to check BKOPS >- * >- * Send HPI command to stop ongoing background operations to >- * allow rapid servicing of foreground operations, e.g. read/ >- * writes. Wait until the card comes out of the programming state >- * to avoid errors in servicing read/write requests. >- */ >-int mmc_stop_bkops(struct mmc_card *card) >+static int mmc_stop_man_bkops(struct mmc_card *card) > { > int err = 0; > >- BUG_ON(!card); > err = mmc_interrupt_hpi(card); > > /* >@@ -779,6 +809,51 @@ int mmc_stop_bkops(struct mmc_card *card) > > return err; > } >+ >+static int mmc_stop_auto_bkops(struct mmc_card *card) >+{ >+ int err = 0; >+ >+ if (!card->ext_csd.auto_bkops_en) >+ return 0; >+ Shouldn’t the BKOPS_STATUS be checked prior to disabling the BKOPS activity of the device? >+ err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >+ EXT_CSD_BKOPS_EN, 0, MMC_BKOPS_MAX_TIMEOUT, >+ true, true, false); >+ if (err) >+ pr_warn("%s: Error %d stoping auto bkops\n", >+ mmc_hostname(card->host), err); >+ >+ card->ext_csd.auto_bkops_en = false; >+ mmc_card_clr_doing_bkops(card); >+ mmc_retune_release(card->host); >+ >+ return err; >+} >+ >+/** >+ * mmc_stop_bkops - stop ongoing BKOPS >+ * @card: MMC card to check BKOPS >+ * @is_auto: A flag to indicate if we should use auto bkops >+ * >+ * Send HPI command to stop ongoing background operations to >+ * allow rapid servicing of foreground operations, e.g. read/ >+ * writes. Wait until the card comes out of the programming state >+ * to avoid errors in servicing read/write requests. >+ * >+ * But for auto bkops, we could only disable AUTO_EN instead of >+ * sending HPI. And the firmware could do it automatically. >+ * >+ */ >+int mmc_stop_bkops(struct mmc_card *card, bool is_auto) >+{ >+ BUG_ON(!card); >+ >+ if (is_auto) >+ return mmc_stop_auto_bkops(card); >+ else >+ return mmc_stop_man_bkops(card); >+} > EXPORT_SYMBOL(mmc_stop_bkops); > > int mmc_read_bkops_status(struct mmc_card *card) >diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >index 61d6b34..c65bea7 100644 >--- a/drivers/mmc/core/mmc.c >+++ b/drivers/mmc/core/mmc.c >@@ -508,6 +508,13 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) > /* check whether the eMMC card supports BKOPS */ > if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1) { > card->ext_csd.bkops = 1; >+ >+ /* for eMMC v5.1 or later, we could use auto_bkops */ >+ if (card->ext_csd.rev >= 8) >+ card->ext_csd.auto_bkops_en = >+ (ext_csd[EXT_CSD_BKOPS_EN] & >+ EXT_CSD_AUTO_BKOPS_MASK); >+ > card->ext_csd.man_bkops_en = > (ext_csd[EXT_CSD_BKOPS_EN] & > EXT_CSD_MANUAL_BKOPS_MASK); >@@ -516,6 +523,9 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) > if (!card->ext_csd.man_bkops_en) > pr_debug("%s: MAN_BKOPS_EN bit is not set\n", > mmc_hostname(card->host)); >+ if (!card->ext_csd.auto_bkops_en) >+ pr_debug("%s: AUTO_BKOPS_EN bit is not set\n", >+ mmc_hostname(card->host)); > } > > /* check whether the eMMC card supports HPI */ >@@ -1241,7 +1251,7 @@ static int mmc_select_hs400es(struct mmc_card *card) > } > > err = mmc_select_bus_width(card); >- if (IS_ERR_VALUE(err)) >+ if (err < 0) > goto out_err; > > /* Switch card to HS mode */ >@@ -1676,6 +1686,21 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, > mmc_select_powerclass(card); > > /* >+ * Enable auto bkops feature which is mandatory for >+ * eMMC v5.1 or later >+ */ >+ if (card->ext_csd.rev >= 8) { >+ err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >+ EXT_CSD_BKOPS_EN, 2, >+ card->ext_csd.generic_cmd6_time); >+ if (err) >+ pr_warn("%s: Error %d starting auto bkops\n", >+ mmc_hostname(card->host), err); >+ >+ card->ext_csd.auto_bkops_en = true; >+ } >+ >+ /* > * Enable HPI feature (if supported) > */ > if (card->ext_csd.hpi) { >@@ -1898,7 +1923,8 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) > goto out; > > if (mmc_card_doing_bkops(host->card)) { >- err = mmc_stop_bkops(host->card); >+ err = mmc_stop_bkops(host->card, >+ host->card->ext_csd.rev >= 8); > if (err) > goto out; > } >diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h >index 22defc2..f82f9be 100644 >--- a/include/linux/mmc/card.h >+++ b/include/linux/mmc/card.h >@@ -84,6 +84,7 @@ struct mmc_ext_csd { > unsigned int hpi_cmd; /* cmd used as HPI */ > bool bkops; /* background support bit */ > bool man_bkops_en; /* manual bkops enable bit */ >+ bool auto_bkops_en; /* auto bkops enbale bit */ > unsigned int data_sector_size; /* 512 bytes or 4KB */ > unsigned int data_tag_unit_size; /* DATA TAG UNIT size */ > unsigned int boot_ro_lock; /* ro lock support */ >diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h >index b01e77d..45d53ef 100644 >--- a/include/linux/mmc/core.h >+++ b/include/linux/mmc/core.h >@@ -140,7 +140,7 @@ struct mmc_request { > struct mmc_card; > struct mmc_async_req; > >-extern int mmc_stop_bkops(struct mmc_card *); >+extern int mmc_stop_bkops(struct mmc_card *, bool); > extern int mmc_read_bkops_status(struct mmc_card *); > extern struct mmc_async_req *mmc_start_req(struct mmc_host *, > struct mmc_async_req *, int *); >@@ -150,7 +150,7 @@ extern int mmc_wait_for_cmd(struct mmc_host *, struct mmc_command *, int); > extern int mmc_app_cmd(struct mmc_host *, struct mmc_card *); > extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *, > struct mmc_command *, int); >-extern void mmc_start_bkops(struct mmc_card *card, bool from_exception); >+extern void mmc_start_bkops(struct mmc_card *, bool, bool); > extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int); > extern int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error); > extern int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd); >diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h >index c376209..a1cced9 100644 >--- a/include/linux/mmc/mmc.h >+++ b/include/linux/mmc/mmc.h >@@ -436,6 +436,7 @@ struct _mmc_csd { > * BKOPS modes > */ > #define EXT_CSD_MANUAL_BKOPS_MASK 0x01 >+#define EXT_CSD_AUTO_BKOPS_MASK 0x02 > > /* > * MMC_SWITCH access modes >-- >2.3.7 > > >-- >To unsubscribe from this list: send the line "unsubscribe linux-mmc" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Alex, On 2016/6/8 22:46, Alex Lemberg wrote: > Hi Shawn, > > Is the intention in this patch to stop using MANUAL_BKOPS for all eMMC5.1+ devices, and only use AUTO_BKOPS? > Please see several questions inline. > Yup, my intention is to use auto bkops for emmc 5.1+ if possible. > > On 6/6/16, 6:07 AM, "linux-mmc-owner@vger.kernel.org on behalf of Shawn Lin" <linux-mmc-owner@vger.kernel.org on behalf of shawn.lin@rock-chips.com> wrote: > >> JEDEC eMMC v5.1 introduce an autonomously initiated method >> for background operations. >> >> Host that wants to enable the device to perform background >> operations during device idle time, should signal the device >> by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When >> this bit is set, the device may start or stop background operations >> whenever it sees fit, without any notification to the host. >> >> When AUTO_EN bit is set, the host should keep the device power >> active. The host may set or clear this bit at any time based on >> its power constraints or other considerations. >> >> Currently the manual bkops is only be used under the async req >> circumstances and it's a bit complicated to be controlled as the >> perfect method is that we should do some idle monitor just as rpm >> and send HPI each time if receiving rd/wr req. But it will impact >> performance significantly, especially for random iops since the >> weight of executing HPI against r/w small piece of LBAs is >> nonnegligible. >> >> So we now prefer to select the auto one unconditionally if supported >> which makes it as simple as possible. It should really good enough >> for devices to manage its internal policy for bkops rather than the >> host, which makes us believe that we could achieve the best >> performance for all the devices implementing auto bkops and the only >> thing we should do is to disable it when cutting off the power. >> >> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> >> --- >> >> drivers/mmc/core/core.c | 127 +++++++++++++++++++++++++++++++++++++---------- >> drivers/mmc/core/mmc.c | 30 ++++++++++- >> include/linux/mmc/card.h | 1 + >> include/linux/mmc/core.h | 4 +- >> include/linux/mmc/mmc.h | 1 + >> 5 files changed, 133 insertions(+), 30 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index e864187..c2e5a66 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -297,24 +297,13 @@ static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq) >> return 0; >> } >> >> -/** >> - * mmc_start_bkops - start BKOPS for supported cards >> - * @card: MMC card to start BKOPS >> - * @form_exception: A flag to indicate if this function was >> - * called due to an exception raised by the card >> - * >> - * Start background operations whenever requested. >> - * When the urgent BKOPS bit is set in a R1 command response >> - * then background operations should be started immediately. >> -*/ >> -void mmc_start_bkops(struct mmc_card *card, bool from_exception) >> +static void mmc_start_man_bkops(struct mmc_card *card, >> + bool from_exception) >> { >> int err; >> int timeout; >> bool use_busy_signal; >> >> - BUG_ON(!card); >> - >> if (!card->ext_csd.man_bkops_en || mmc_card_doing_bkops(card)) >> return; >> >> @@ -347,7 +336,7 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception) >> EXT_CSD_BKOPS_START, 1, timeout, >> use_busy_signal, true, false); >> if (err) { >> - pr_warn("%s: Error %d starting bkops\n", >> + pr_warn("%s: Error %d starting manual bkops\n", >> mmc_hostname(card->host), err); >> mmc_retune_release(card->host); >> goto out; >> @@ -365,6 +354,55 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception) >> out: >> mmc_release_host(card->host); >> } >> + >> +static void mmc_start_auto_bkops(struct mmc_card *card) >> +{ >> + int err; >> + >> + /* If it's already enable auto_bkops, nothing to do */ >> + if (card->ext_csd.auto_bkops_en) >> + return; >> + >> + mmc_claim_host(card->host); >> + >> + mmc_retune_hold(card->host); >> + >> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >> + EXT_CSD_BKOPS_EN, 2, >> + card->ext_csd.generic_cmd6_time); >> + if (err) >> + pr_warn("%s: Error %d starting auto bkops\n", >> + mmc_hostname(card->host), err); >> + >> + card->ext_csd.auto_bkops_en = true; >> + mmc_card_set_doing_bkops(card); >> + mmc_retune_release(card->host); >> + mmc_release_host(card->host); >> +} >> + >> + >> +/** >> + * mmc_start_bkops - start BKOPS for supported cards >> + * @card: MMC card to start BKOPS >> + * @form_exception: A flag to indicate if this function was >> + * called due to an exception raised by the card >> + * @is_auto: A flag to indicate if we should use auto bkops >> + * >> + * Start background operations whenever requested. >> + * When the urgent BKOPS bit is set in a R1 command response >> + * then background operations should be started immediately, which is >> + * only needed for man_bkops. >> +*/ >> +void mmc_start_bkops(struct mmc_card *card, bool from_exception, >> + bool is_auto) >> +{ >> + BUG_ON(!card); >> + >> + if (is_auto) > > As far as I understand, the AUTO_BKOPS is enabled during “mmc_init_card()”. > If its correct, why the “mmc_start_auto_bkops()” function is needed? > yes it's not used twice for curren code. will remove it. > >> + return mmc_start_auto_bkops(card); >> + else >> + return mmc_start_man_bkops(card, from_exception); >> +} >> EXPORT_SYMBOL(mmc_start_bkops); >> >> /* >> @@ -610,7 +648,9 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host, >> if (areq) >> mmc_post_req(host, areq->mrq, -EINVAL); >> >> - mmc_start_bkops(host->card, true); >> + /* Prefer to use auto bkops for eMMC 5.1 or later */ > > If the decision is to use AUTO_BKOPS for all eMMC5.1+ devices, why you need to enable AUTO_BKOPS bit > on every R1_EXCEPTION_EVENT? > AUTO_BKOPS should be enabled only once... > right, I will remove this enable although it will finally turn back by the checking of auo_bkops_en. > >> + mmc_start_bkops(host->card, true, >> + host->card->ext_csd.rev >= 8); >> >> /* prepare the request again */ >> if (areq) >> @@ -751,20 +791,10 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries >> >> EXPORT_SYMBOL(mmc_wait_for_cmd); >> >> -/** >> - * mmc_stop_bkops - stop ongoing BKOPS >> - * @card: MMC card to check BKOPS >> - * >> - * Send HPI command to stop ongoing background operations to >> - * allow rapid servicing of foreground operations, e.g. read/ >> - * writes. Wait until the card comes out of the programming state >> - * to avoid errors in servicing read/write requests. >> - */ >> -int mmc_stop_bkops(struct mmc_card *card) >> +static int mmc_stop_man_bkops(struct mmc_card *card) >> { >> int err = 0; >> >> - BUG_ON(!card); >> err = mmc_interrupt_hpi(card); >> >> /* >> @@ -779,6 +809,51 @@ int mmc_stop_bkops(struct mmc_card *card) >> >> return err; >> } >> + >> +static int mmc_stop_auto_bkops(struct mmc_card *card) >> +{ >> + int err = 0; >> + >> + if (!card->ext_csd.auto_bkops_en) >> + return 0; >> + > > Shouldn’t the BKOPS_STATUS be checked prior to disabling the BKOPS activity of the device? > Hrmm.. I read the whole section of spec for it, and I did find this requirement for manul bkops but not for the auto one. So what should we do if using the auto one? > >> + err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >> + EXT_CSD_BKOPS_EN, 0, MMC_BKOPS_MAX_TIMEOUT, >> + true, true, false); >> + if (err) >> + pr_warn("%s: Error %d stoping auto bkops\n", >> + mmc_hostname(card->host), err); >> + >> + card->ext_csd.auto_bkops_en = false; >> + mmc_card_clr_doing_bkops(card); >> + mmc_retune_release(card->host); >> + >> + return err; >> +} >> + >> +/** >> + * mmc_stop_bkops - stop ongoing BKOPS >> + * @card: MMC card to check BKOPS >> + * @is_auto: A flag to indicate if we should use auto bkops >> + * >> + * Send HPI command to stop ongoing background operations to >> + * allow rapid servicing of foreground operations, e.g. read/ >> + * writes. Wait until the card comes out of the programming state >> + * to avoid errors in servicing read/write requests. >> + * >> + * But for auto bkops, we could only disable AUTO_EN instead of >> + * sending HPI. And the firmware could do it automatically. >> + * >> + */ >> +int mmc_stop_bkops(struct mmc_card *card, bool is_auto) >> +{ >> + BUG_ON(!card); >> + >> + if (is_auto) >> + return mmc_stop_auto_bkops(card); >> + else >> + return mmc_stop_man_bkops(card); >> +} >> EXPORT_SYMBOL(mmc_stop_bkops); >> >> int mmc_read_bkops_status(struct mmc_card *card) >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index 61d6b34..c65bea7 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -508,6 +508,13 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) >> /* check whether the eMMC card supports BKOPS */ >> if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1) { >> card->ext_csd.bkops = 1; >> + >> + /* for eMMC v5.1 or later, we could use auto_bkops */ >> + if (card->ext_csd.rev >= 8) >> + card->ext_csd.auto_bkops_en = >> + (ext_csd[EXT_CSD_BKOPS_EN] & >> + EXT_CSD_AUTO_BKOPS_MASK); >> + >> card->ext_csd.man_bkops_en = >> (ext_csd[EXT_CSD_BKOPS_EN] & >> EXT_CSD_MANUAL_BKOPS_MASK); >> @@ -516,6 +523,9 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) >> if (!card->ext_csd.man_bkops_en) >> pr_debug("%s: MAN_BKOPS_EN bit is not set\n", >> mmc_hostname(card->host)); >> + if (!card->ext_csd.auto_bkops_en) >> + pr_debug("%s: AUTO_BKOPS_EN bit is not set\n", >> + mmc_hostname(card->host)); >> } >> >> /* check whether the eMMC card supports HPI */ >> @@ -1241,7 +1251,7 @@ static int mmc_select_hs400es(struct mmc_card *card) >> } >> >> err = mmc_select_bus_width(card); >> - if (IS_ERR_VALUE(err)) >> + if (err < 0) >> goto out_err; >> >> /* Switch card to HS mode */ >> @@ -1676,6 +1686,21 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, >> mmc_select_powerclass(card); >> >> /* >> + * Enable auto bkops feature which is mandatory for >> + * eMMC v5.1 or later >> + */ >> + if (card->ext_csd.rev >= 8) { >> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >> + EXT_CSD_BKOPS_EN, 2, >> + card->ext_csd.generic_cmd6_time); >> + if (err) >> + pr_warn("%s: Error %d starting auto bkops\n", >> + mmc_hostname(card->host), err); >> + >> + card->ext_csd.auto_bkops_en = true; >> + } >> + >> + /* >> * Enable HPI feature (if supported) >> */ >> if (card->ext_csd.hpi) { >> @@ -1898,7 +1923,8 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) >> goto out; >> >> if (mmc_card_doing_bkops(host->card)) { >> - err = mmc_stop_bkops(host->card); >> + err = mmc_stop_bkops(host->card, >> + host->card->ext_csd.rev >= 8); >> if (err) >> goto out; >> } >> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h >> index 22defc2..f82f9be 100644 >> --- a/include/linux/mmc/card.h >> +++ b/include/linux/mmc/card.h >> @@ -84,6 +84,7 @@ struct mmc_ext_csd { >> unsigned int hpi_cmd; /* cmd used as HPI */ >> bool bkops; /* background support bit */ >> bool man_bkops_en; /* manual bkops enable bit */ >> + bool auto_bkops_en; /* auto bkops enbale bit */ >> unsigned int data_sector_size; /* 512 bytes or 4KB */ >> unsigned int data_tag_unit_size; /* DATA TAG UNIT size */ >> unsigned int boot_ro_lock; /* ro lock support */ >> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h >> index b01e77d..45d53ef 100644 >> --- a/include/linux/mmc/core.h >> +++ b/include/linux/mmc/core.h >> @@ -140,7 +140,7 @@ struct mmc_request { >> struct mmc_card; >> struct mmc_async_req; >> >> -extern int mmc_stop_bkops(struct mmc_card *); >> +extern int mmc_stop_bkops(struct mmc_card *, bool); >> extern int mmc_read_bkops_status(struct mmc_card *); >> extern struct mmc_async_req *mmc_start_req(struct mmc_host *, >> struct mmc_async_req *, int *); >> @@ -150,7 +150,7 @@ extern int mmc_wait_for_cmd(struct mmc_host *, struct mmc_command *, int); >> extern int mmc_app_cmd(struct mmc_host *, struct mmc_card *); >> extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *, >> struct mmc_command *, int); >> -extern void mmc_start_bkops(struct mmc_card *card, bool from_exception); >> +extern void mmc_start_bkops(struct mmc_card *, bool, bool); >> extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int); >> extern int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error); >> extern int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd); >> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h >> index c376209..a1cced9 100644 >> --- a/include/linux/mmc/mmc.h >> +++ b/include/linux/mmc/mmc.h >> @@ -436,6 +436,7 @@ struct _mmc_csd { >> * BKOPS modes >> */ >> #define EXT_CSD_MANUAL_BKOPS_MASK 0x01 >> +#define EXT_CSD_AUTO_BKOPS_MASK 0x02 >> >> /* >> * MMC_SWITCH access modes >> -- >> 2.3.7 >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >
On 06/06/16 06:07, Shawn Lin wrote: > JEDEC eMMC v5.1 introduce an autonomously initiated method > for background operations. > > Host that wants to enable the device to perform background > operations during device idle time, should signal the device > by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When > this bit is set, the device may start or stop background operations > whenever it sees fit, without any notification to the host. > > When AUTO_EN bit is set, the host should keep the device power > active. The host may set or clear this bit at any time based on > its power constraints or other considerations. > > Currently the manual bkops is only be used under the async req > circumstances and it's a bit complicated to be controlled as the > perfect method is that we should do some idle monitor just as rpm > and send HPI each time if receiving rd/wr req. But it will impact > performance significantly, especially for random iops since the > weight of executing HPI against r/w small piece of LBAs is > nonnegligible. > > So we now prefer to select the auto one unconditionally if supported > which makes it as simple as possible. It should really good enough > for devices to manage its internal policy for bkops rather than the > host, which makes us believe that we could achieve the best > performance for all the devices implementing auto bkops and the only > thing we should do is to disable it when cutting off the power. Do you know if there is really a requirement to do that? Because then, what is the point of power off notification? And why is AUTO_EN persistent across power failure? -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2016/6/13 14:29, Adrian Hunter wrote: > On 06/06/16 06:07, Shawn Lin wrote: >> JEDEC eMMC v5.1 introduce an autonomously initiated method >> for background operations. >> >> Host that wants to enable the device to perform background >> operations during device idle time, should signal the device >> by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When >> this bit is set, the device may start or stop background operations >> whenever it sees fit, without any notification to the host. >> >> When AUTO_EN bit is set, the host should keep the device power >> active. The host may set or clear this bit at any time based on >> its power constraints or other considerations. >> >> Currently the manual bkops is only be used under the async req >> circumstances and it's a bit complicated to be controlled as the >> perfect method is that we should do some idle monitor just as rpm >> and send HPI each time if receiving rd/wr req. But it will impact >> performance significantly, especially for random iops since the >> weight of executing HPI against r/w small piece of LBAs is >> nonnegligible. >> >> So we now prefer to select the auto one unconditionally if supported >> which makes it as simple as possible. It should really good enough >> for devices to manage its internal policy for bkops rather than the >> host, which makes us believe that we could achieve the best >> performance for all the devices implementing auto bkops and the only >> thing we should do is to disable it when cutting off the power. > > Do you know if there is really a requirement to do that? Even without bkops enable, no matter for manual or auto one, FTL should always do bkops like GC internally when needed to guarantee the performance and balance the wear leveling. What I thought to do is to make it more explicitly. Because then, what > is the point of power off notification? When power off notification is sent, bkops will be stopped in _mmc_suspend. So I don't undertand your point here? And why is AUTO_EN persistent across > power failure? Ahh, that's a problem need to be fixed, thanks for reminding. > > > >
On 13/06/16 10:48, Shawn Lin wrote: > On 2016/6/13 14:29, Adrian Hunter wrote: >> On 06/06/16 06:07, Shawn Lin wrote: >>> JEDEC eMMC v5.1 introduce an autonomously initiated method >>> for background operations. >>> >>> Host that wants to enable the device to perform background >>> operations during device idle time, should signal the device >>> by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When >>> this bit is set, the device may start or stop background operations >>> whenever it sees fit, without any notification to the host. >>> >>> When AUTO_EN bit is set, the host should keep the device power >>> active. The host may set or clear this bit at any time based on >>> its power constraints or other considerations. >>> >>> Currently the manual bkops is only be used under the async req >>> circumstances and it's a bit complicated to be controlled as the >>> perfect method is that we should do some idle monitor just as rpm >>> and send HPI each time if receiving rd/wr req. But it will impact >>> performance significantly, especially for random iops since the >>> weight of executing HPI against r/w small piece of LBAs is >>> nonnegligible. >>> >>> So we now prefer to select the auto one unconditionally if supported >>> which makes it as simple as possible. It should really good enough >>> for devices to manage its internal policy for bkops rather than the >>> host, which makes us believe that we could achieve the best >>> performance for all the devices implementing auto bkops and the only >>> thing we should do is to disable it when cutting off the power. >> >> Do you know if there is really a requirement to do that? > > Even without bkops enable, no matter for manual or auto one, FTL should > always do bkops like GC internally when needed to guarantee the > performance and balance the wear leveling. What I thought to do is to > make it more explicitly. > > Because then, what >> is the point of power off notification? > > When power off notification is sent, bkops will be stopped > in _mmc_suspend. So I don't undertand your point here? I am trying to understand why we need to do anything for auto bkops. Since AUTO_EN is persistent, we can leave the decision whether to turn it on to whomever provisions the device. Then we just leave it alone. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
在 2016/6/13 16:17, Adrian Hunter 写道: > On 13/06/16 10:48, Shawn Lin wrote: >> On 2016/6/13 14:29, Adrian Hunter wrote: >>> On 06/06/16 06:07, Shawn Lin wrote: >>>> JEDEC eMMC v5.1 introduce an autonomously initiated method >>>> for background operations. >>>> >>>> Host that wants to enable the device to perform background >>>> operations during device idle time, should signal the device >>>> by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When >>>> this bit is set, the device may start or stop background operations >>>> whenever it sees fit, without any notification to the host. >>>> >>>> When AUTO_EN bit is set, the host should keep the device power >>>> active. The host may set or clear this bit at any time based on >>>> its power constraints or other considerations. >>>> >>>> Currently the manual bkops is only be used under the async req >>>> circumstances and it's a bit complicated to be controlled as the >>>> perfect method is that we should do some idle monitor just as rpm >>>> and send HPI each time if receiving rd/wr req. But it will impact >>>> performance significantly, especially for random iops since the >>>> weight of executing HPI against r/w small piece of LBAs is >>>> nonnegligible. >>>> >>>> So we now prefer to select the auto one unconditionally if supported >>>> which makes it as simple as possible. It should really good enough >>>> for devices to manage its internal policy for bkops rather than the >>>> host, which makes us believe that we could achieve the best >>>> performance for all the devices implementing auto bkops and the only >>>> thing we should do is to disable it when cutting off the power. >>> >>> Do you know if there is really a requirement to do that? >> >> Even without bkops enable, no matter for manual or auto one, FTL should >> always do bkops like GC internally when needed to guarantee the >> performance and balance the wear leveling. What I thought to do is to >> make it more explicitly. >> >> Because then, what >>> is the point of power off notification? >> >> When power off notification is sent, bkops will be stopped >> in _mmc_suspend. So I don't undertand your point here? > > I am trying to understand why we need to do anything for auto bkops. > Since AUTO_EN is persistent, we can leave the decision whether to turn it on > to whomever provisions the device. Then we just leave it alone. > Hrm.. one possible way is to control it by mmc-utils on user space? So we should add a cmd for mmc-utils there? > > >
On 13/06/16 11:58, Shawn Lin wrote: > 在 2016/6/13 16:17, Adrian Hunter 写道: >> On 13/06/16 10:48, Shawn Lin wrote: >>> On 2016/6/13 14:29, Adrian Hunter wrote: >>>> On 06/06/16 06:07, Shawn Lin wrote: >>>>> JEDEC eMMC v5.1 introduce an autonomously initiated method >>>>> for background operations. >>>>> >>>>> Host that wants to enable the device to perform background >>>>> operations during device idle time, should signal the device >>>>> by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When >>>>> this bit is set, the device may start or stop background operations >>>>> whenever it sees fit, without any notification to the host. >>>>> >>>>> When AUTO_EN bit is set, the host should keep the device power >>>>> active. The host may set or clear this bit at any time based on >>>>> its power constraints or other considerations. >>>>> >>>>> Currently the manual bkops is only be used under the async req >>>>> circumstances and it's a bit complicated to be controlled as the >>>>> perfect method is that we should do some idle monitor just as rpm >>>>> and send HPI each time if receiving rd/wr req. But it will impact >>>>> performance significantly, especially for random iops since the >>>>> weight of executing HPI against r/w small piece of LBAs is >>>>> nonnegligible. >>>>> >>>>> So we now prefer to select the auto one unconditionally if supported >>>>> which makes it as simple as possible. It should really good enough >>>>> for devices to manage its internal policy for bkops rather than the >>>>> host, which makes us believe that we could achieve the best >>>>> performance for all the devices implementing auto bkops and the only >>>>> thing we should do is to disable it when cutting off the power. >>>> >>>> Do you know if there is really a requirement to do that? >>> >>> Even without bkops enable, no matter for manual or auto one, FTL should >>> always do bkops like GC internally when needed to guarantee the >>> performance and balance the wear leveling. What I thought to do is to >>> make it more explicitly. >>> >>> Because then, what >>>> is the point of power off notification? >>> >>> When power off notification is sent, bkops will be stopped >>> in _mmc_suspend. So I don't undertand your point here? >> >> I am trying to understand why we need to do anything for auto bkops. >> Since AUTO_EN is persistent, we can leave the decision whether to turn it on >> to whomever provisions the device. Then we just leave it alone. >> > > Hrm.. > > one possible way is to control it by mmc-utils on > user space? So we should add a cmd for mmc-utils > there? That would be consistent with manual bkops. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Shawn, […] >>> + >>> +static int mmc_stop_auto_bkops(struct mmc_card *card) >>> +{ >>> + int err = 0; >>> + >>> + if (!card->ext_csd.auto_bkops_en) >>> + return 0; >>> + >> >> Shouldn’t the BKOPS_STATUS be checked prior to disabling the BKOPS activity of the device? >> > >Hrmm.. I read the whole section of spec for it, and I did find this >requirement for manul bkops but not for the auto one. So what should we >do if using the auto one? > In case of AUTO BKOPS, the eMMC Device should perform internal GC in the same way as in case of MANUAL BKOPS. The only difference is a host awareness. Although there is no requirement in the spec, I think the driver can give some time to the device to perform/complete its internal GC during the idle time. Thus I think we can check the BKOPS_STATUS on Runtime suspend. […] Thanks, Alex
On 06/20/2016 10:33 PM, Alex Lemberg wrote: > Hi Shawn, > > […] > >>>> + >>>> +static int mmc_stop_auto_bkops(struct mmc_card *card) >>>> +{ >>>> + int err = 0; >>>> + >>>> + if (!card->ext_csd.auto_bkops_en) >>>> + return 0; >>>> + >>> >>> Shouldn’t the BKOPS_STATUS be checked prior to disabling the BKOPS activity of the device? >>> >> >> Hrmm.. I read the whole section of spec for it, and I did find this >> requirement for manul bkops but not for the auto one. So what should we >> do if using the auto one? >> > > In case of AUTO BKOPS, the eMMC Device should perform internal GC > in the same way as in case of MANUAL BKOPS. > The only difference is a host awareness. > Although there is no requirement in the spec, I think the driver can > give some time to the device to perform/complete its internal GC during the idle time. > Thus I think we can check the BKOPS_STATUS on Runtime suspend. I'm not sure we can check BKOPS_STATUS when use the AUTO_BKOPS. I have also read the SPEC, but it's not clearly for checking the BKOPS_STATUS. Just i understood that it should be operated without a host's awareness. Best Regards, Jaehoon Chung > > […] > > Thanks, > Alex > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2016/6/20 21:33, Alex Lemberg wrote: > Hi Shawn, > > […] > >>>> + >>>> +static int mmc_stop_auto_bkops(struct mmc_card *card) >>>> +{ >>>> + int err = 0; >>>> + >>>> + if (!card->ext_csd.auto_bkops_en) >>>> + return 0; >>>> + >>> >>> Shouldn’t the BKOPS_STATUS be checked prior to disabling the BKOPS activity of the device? >>> >> >> Hrmm.. I read the whole section of spec for it, and I did find this >> requirement for manul bkops but not for the auto one. So what should we >> do if using the auto one? >> > > In case of AUTO BKOPS, the eMMC Device should perform internal GC > in the same way as in case of MANUAL BKOPS. > The only difference is a host awareness. agree. > Although there is no requirement in the spec, I think the driver can > give some time to the device to perform/complete its internal GC during the idle time. > Thus I think we can check the BKOPS_STATUS on Runtime suspend. We shouldn't diable bkops on *runtime* suspend as it's just the right time for firmware to do GC. We could consider to check and wait for the status when doing poweroff, although it seems firmware should be able to accept the disable cmd and deal the on-going work perfectly when doing bkops without host's awareness, just the same way as suddent power loss cases. Also I don't know whether the firmware will reflect its status on BKOPS_STATUS or not when enabling the auto one. I will do more test. Anyway, thanks for sharing your thought. Also Adrian point out that currently we trigger manual bkosp from userspace via mmc-utils, and I agreed we shouldn't force kernel stack to enable it defaultly. So I'm prone not to update this $SUBJECT and migrate it to mmc-utils later. > > […] > > Thanks, > Alex >
On 13 June 2016 at 14:25, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 13/06/16 11:58, Shawn Lin wrote: >> 在 2016/6/13 16:17, Adrian Hunter 写道: >>> On 13/06/16 10:48, Shawn Lin wrote: >>>> On 2016/6/13 14:29, Adrian Hunter wrote: >>>>> On 06/06/16 06:07, Shawn Lin wrote: >>>>>> JEDEC eMMC v5.1 introduce an autonomously initiated method >>>>>> for background operations. >>>>>> >>>>>> Host that wants to enable the device to perform background >>>>>> operations during device idle time, should signal the device >>>>>> by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When >>>>>> this bit is set, the device may start or stop background operations >>>>>> whenever it sees fit, without any notification to the host. >>>>>> >>>>>> When AUTO_EN bit is set, the host should keep the device power >>>>>> active. The host may set or clear this bit at any time based on >>>>>> its power constraints or other considerations. >>>>>> >>>>>> Currently the manual bkops is only be used under the async req >>>>>> circumstances and it's a bit complicated to be controlled as the >>>>>> perfect method is that we should do some idle monitor just as rpm >>>>>> and send HPI each time if receiving rd/wr req. But it will impact >>>>>> performance significantly, especially for random iops since the >>>>>> weight of executing HPI against r/w small piece of LBAs is >>>>>> nonnegligible. >>>>>> >>>>>> So we now prefer to select the auto one unconditionally if supported >>>>>> which makes it as simple as possible. It should really good enough >>>>>> for devices to manage its internal policy for bkops rather than the >>>>>> host, which makes us believe that we could achieve the best >>>>>> performance for all the devices implementing auto bkops and the only >>>>>> thing we should do is to disable it when cutting off the power. >>>>> >>>>> Do you know if there is really a requirement to do that? >>>> >>>> Even without bkops enable, no matter for manual or auto one, FTL should >>>> always do bkops like GC internally when needed to guarantee the >>>> performance and balance the wear leveling. What I thought to do is to >>>> make it more explicitly. >>>> >>>> Because then, what >>>>> is the point of power off notification? >>>> >>>> When power off notification is sent, bkops will be stopped >>>> in _mmc_suspend. So I don't undertand your point here? >>> >>> I am trying to understand why we need to do anything for auto bkops. >>> Since AUTO_EN is persistent, we can leave the decision whether to turn it on >>> to whomever provisions the device. Then we just leave it alone. >>> >> >> Hrm.. >> >> one possible way is to control it by mmc-utils on >> user space? So we should add a cmd for mmc-utils >> there? > > That would be consistent with manual bkops. > From my first impression I agree, as that is the policy we have been sticking to when writing to persistent EXT_CSD registers. Although, in this case, I am actually wondering on what is the best approach. Is there really ever a case when we don't want auto BKOPS to be default enabled? I think BKOPS is a fundamental feature of an FTL and I can't see a reason to why we need to involve mmc-utils/userspace to enable it. Am I wrong? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
SEkgU2hhd24sDQoNCk9uIDYvMjEvMTYsIDQ6NDQgQU0sICJTaGF3biBMaW4iIDxzaGF3bi5saW5A cm9jay1jaGlwcy5jb20+IHdyb3RlOg0KDQo+T24gMjAxNi82LzIwIDIxOjMzLCBBbGV4IExlbWJl cmcgd3JvdGU6DQo+PiBIaSBTaGF3biwNCj4+DQo+PiBb4oCmXQ0KPj4NCj4+Pj4+ICsNCj4+Pj4+ ICtzdGF0aWMgaW50IG1tY19zdG9wX2F1dG9fYmtvcHMoc3RydWN0IG1tY19jYXJkICpjYXJkKQ0K Pj4+Pj4gK3sNCj4+Pj4+ICsJaW50IGVyciA9IDA7DQo+Pj4+PiArDQo+Pj4+PiArCWlmICghY2Fy ZC0+ZXh0X2NzZC5hdXRvX2Jrb3BzX2VuKQ0KPj4+Pj4gKwkJcmV0dXJuIDA7DQo+Pj4+PiArDQo+ Pj4+DQo+Pj4+IFNob3VsZG7igJl0IHRoZSBCS09QU19TVEFUVVMgYmUgY2hlY2tlZCBwcmlvciB0 byBkaXNhYmxpbmcgdGhlIEJLT1BTIGFjdGl2aXR5IG9mIHRoZSBkZXZpY2U/DQo+Pj4+DQo+Pj4N Cj4+PiBIcm1tLi4gSSByZWFkIHRoZSB3aG9sZSBzZWN0aW9uIG9mIHNwZWMgZm9yIGl0LCBhbmQg SSBkaWQgZmluZCB0aGlzDQo+Pj4gcmVxdWlyZW1lbnQgZm9yIG1hbnVsIGJrb3BzIGJ1dCBub3Qg Zm9yIHRoZSBhdXRvIG9uZS4gU28gd2hhdCBzaG91bGQgd2UNCj4+PiBkbyBpZiB1c2luZyB0aGUg YXV0byBvbmU/DQo+Pj4NCj4+DQo+PiBJbiBjYXNlIG9mIEFVVE8gQktPUFMsIHRoZSBlTU1DIERl dmljZSBzaG91bGQgcGVyZm9ybSBpbnRlcm5hbCBHQw0KPj4gaW4gdGhlIHNhbWUgd2F5IGFzIGlu IGNhc2Ugb2YgTUFOVUFMIEJLT1BTLg0KPj4gVGhlIG9ubHkgZGlmZmVyZW5jZSBpcyBhIGhvc3Qg YXdhcmVuZXNzLg0KPg0KPmFncmVlLg0KPg0KPj4gQWx0aG91Z2ggdGhlcmUgaXMgbm8gcmVxdWly ZW1lbnQgaW4gdGhlIHNwZWMsIEkgdGhpbmsgdGhlIGRyaXZlciBjYW4NCj4+IGdpdmUgc29tZSB0 aW1lIHRvIHRoZSBkZXZpY2UgdG8gcGVyZm9ybS9jb21wbGV0ZSBpdHMgaW50ZXJuYWwgR0MgZHVy aW5nIHRoZSBpZGxlIHRpbWUuDQo+PiBUaHVzIEkgdGhpbmsgd2UgY2FuIGNoZWNrIHRoZSBCS09Q U19TVEFUVVMgb24gUnVudGltZSBzdXNwZW5kLg0KPg0KPldlIHNob3VsZG4ndCBkaWFibGUgYmtv cHMgb24gKnJ1bnRpbWUqIHN1c3BlbmQgYXMgaXQncyBqdXN0IHRoZSByaWdodA0KPnRpbWUgZm9y IGZpcm13YXJlIHRvIGRvIEdDLiBXZSBjb3VsZCBjb25zaWRlciB0byBjaGVjayBhbmQgd2FpdCBm b3INCj50aGUgc3RhdHVzIHdoZW4gZG9pbmcgcG93ZXJvZmYsIGFsdGhvdWdoIGl0IHNlZW1zIGZp cm13YXJlIHNob3VsZCBiZQ0KPmFibGUgdG8gYWNjZXB0IHRoZSBkaXNhYmxlIGNtZCBhbmQgZGVh bCB0aGUgb24tZ29pbmcgd29yayBwZXJmZWN0bHkNCj53aGVuIGRvaW5nIGJrb3BzIHdpdGhvdXQg aG9zdCdzIGF3YXJlbmVzcywganVzdCB0aGUgc2FtZSB3YXkgYXMgc3VkZGVudA0KPnBvd2VyIGxv c3MgY2FzZXMuDQoNCklmIEkgYW0gbm90IHdyb25nLCBpbiBjdXJyZW50IGltcGxlbWVudGF0aW9u IG9mIHJ1bnRpbWUgc3VzcGVuZCwgDQp0aGUgZHJpdmVyIHN0b3BzIEJLT1BTIChzZW5kIEhQSSkg anVzdCBiZWZvcmUgc2VuZGluZyBzbGVlcCBjb21tYW5kLA0Kc2VlIF9tbWNfc3VzcGVuZCgpLCBk ZXBlbmRzIG9uIOKAnE1NQ19DQVBfQUdHUkVTU0lWRV9QTeKAnSBmbGFnLg0KSW4gdGhpcyBjYXNl LCB0aGUgZU1NQyBkZXZpY2Ugd2lsbCBub3QgaGF2ZSBlbm91Z2ggdGltZSB0byBwZXJmb3JtIGlu dGVybmFsIA0KQktPUFMgaW4gYm90aCDigJMgTWFudWFsIGFuZCBBdXRvIEJLT1BTIGNvbmZpZ3Vy YXRpb25zLg0KDQpGb3IgdGhlIHBvd2Vyb2ZmLCBpdCBzaG91bGQgYmUgT0sgd2l0aCBhIGN1cnJl bnQgaW1wbGVtZW50YXRpb24gb2YgDQpQT04gKG1tY19wb3dlcm9mZl9ub3RpZnkoKSkNCg0KPg0K PkFsc28gSSBkb24ndCBrbm93IHdoZXRoZXIgdGhlIGZpcm13YXJlIHdpbGwgcmVmbGVjdCBpdHMg c3RhdHVzIG9uDQo+QktPUFNfU1RBVFVTIG9yIG5vdCB3aGVuIGVuYWJsaW5nIHRoZSBhdXRvIG9u ZS4gSSB3aWxsIGRvIG1vcmUgdGVzdC4NCj4NCj5Bbnl3YXksIHRoYW5rcyBmb3Igc2hhcmluZyB5 b3VyIHRob3VnaHQuDQo+QWxzbyBBZHJpYW4gcG9pbnQgb3V0IHRoYXQgY3VycmVudGx5IHdlIHRy aWdnZXIgbWFudWFsIGJrb3NwIGZyb20NCj51c2Vyc3BhY2UgdmlhIG1tYy11dGlscywgYW5kIEkg YWdyZWVkIHdlIHNob3VsZG4ndCBmb3JjZSBrZXJuZWwgc3RhY2sNCj50byBlbmFibGUgaXQgZGVm YXVsdGx5LiBTbyBJJ20gcHJvbmUgbm90IHRvIHVwZGF0ZSB0aGlzICRTVUJKRUNUIGFuZA0KPm1p Z3JhdGUgaXQgdG8gbW1jLXV0aWxzIGxhdGVyLg0KPg0KPj4NCj4+IFvigKZdDQo+Pg0KPj4gVGhh bmtzLA0KPj4gQWxleA0KPj4NCj4NCj4NCj4tLSANCj5CZXN0IFJlZ2FyZHMNCj5TaGF3biBMaW4N Cj4NCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 6/22/16, 1:21 PM, "Ulf Hansson" <ulf.hansson@linaro.org> wrote: >On 13 June 2016 at 14:25, Adrian Hunter <adrian.hunter@intel.com> wrote: >> On 13/06/16 11:58, Shawn Lin wrote: >>> 在 2016/6/13 16:17, Adrian Hunter 写道: >>>> On 13/06/16 10:48, Shawn Lin wrote: >>>>> On 2016/6/13 14:29, Adrian Hunter wrote: >>>>>> On 06/06/16 06:07, Shawn Lin wrote: >>>>>>> JEDEC eMMC v5.1 introduce an autonomously initiated method >>>>>>> for background operations. >>>>>>> >>>>>>> Host that wants to enable the device to perform background >>>>>>> operations during device idle time, should signal the device >>>>>>> by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When >>>>>>> this bit is set, the device may start or stop background operations >>>>>>> whenever it sees fit, without any notification to the host. >>>>>>> >>>>>>> When AUTO_EN bit is set, the host should keep the device power >>>>>>> active. The host may set or clear this bit at any time based on >>>>>>> its power constraints or other considerations. >>>>>>> >>>>>>> Currently the manual bkops is only be used under the async req >>>>>>> circumstances and it's a bit complicated to be controlled as the >>>>>>> perfect method is that we should do some idle monitor just as rpm >>>>>>> and send HPI each time if receiving rd/wr req. But it will impact >>>>>>> performance significantly, especially for random iops since the >>>>>>> weight of executing HPI against r/w small piece of LBAs is >>>>>>> nonnegligible. >>>>>>> >>>>>>> So we now prefer to select the auto one unconditionally if supported >>>>>>> which makes it as simple as possible. It should really good enough >>>>>>> for devices to manage its internal policy for bkops rather than the >>>>>>> host, which makes us believe that we could achieve the best >>>>>>> performance for all the devices implementing auto bkops and the only >>>>>>> thing we should do is to disable it when cutting off the power. >>>>>> >>>>>> Do you know if there is really a requirement to do that? >>>>> >>>>> Even without bkops enable, no matter for manual or auto one, FTL should >>>>> always do bkops like GC internally when needed to guarantee the >>>>> performance and balance the wear leveling. What I thought to do is to >>>>> make it more explicitly. >>>>> >>>>> Because then, what >>>>>> is the point of power off notification? >>>>> >>>>> When power off notification is sent, bkops will be stopped >>>>> in _mmc_suspend. So I don't undertand your point here? >>>> >>>> I am trying to understand why we need to do anything for auto bkops. >>>> Since AUTO_EN is persistent, we can leave the decision whether to turn it on >>>> to whomever provisions the device. Then we just leave it alone. >>>> >>> >>> Hrm.. >>> >>> one possible way is to control it by mmc-utils on >>> user space? So we should add a cmd for mmc-utils >>> there? >> >> That would be consistent with manual bkops. >> > >From my first impression I agree, as that is the policy we have been >sticking to when writing to persistent EXT_CSD registers. >Although, in this case, I am actually wondering on what is the best approach. > >Is there really ever a case when we don't want auto BKOPS to be default enabled? >I think BKOPS is a fundamental feature of an FTL and I can't see a >reason to why we need to involve mmc-utils/userspace to enable it. Am >I wrong? The even worst case is – involve mmc-utils/userspace to DISABLE it. I think this register need to be set by vendor and no need to be changed on runtime. > >Kind regards >Uffe
On 22 June 2016 at 16:20, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote: > > > On 6/22/16, 1:21 PM, "Ulf Hansson" <ulf.hansson@linaro.org> wrote: > >>On 13 June 2016 at 14:25, Adrian Hunter <adrian.hunter@intel.com> wrote: >>> On 13/06/16 11:58, Shawn Lin wrote: >>>> 在 2016/6/13 16:17, Adrian Hunter 写道: >>>>> On 13/06/16 10:48, Shawn Lin wrote: >>>>>> On 2016/6/13 14:29, Adrian Hunter wrote: >>>>>>> On 06/06/16 06:07, Shawn Lin wrote: >>>>>>>> JEDEC eMMC v5.1 introduce an autonomously initiated method >>>>>>>> for background operations. >>>>>>>> >>>>>>>> Host that wants to enable the device to perform background >>>>>>>> operations during device idle time, should signal the device >>>>>>>> by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When >>>>>>>> this bit is set, the device may start or stop background operations >>>>>>>> whenever it sees fit, without any notification to the host. >>>>>>>> >>>>>>>> When AUTO_EN bit is set, the host should keep the device power >>>>>>>> active. The host may set or clear this bit at any time based on >>>>>>>> its power constraints or other considerations. >>>>>>>> >>>>>>>> Currently the manual bkops is only be used under the async req >>>>>>>> circumstances and it's a bit complicated to be controlled as the >>>>>>>> perfect method is that we should do some idle monitor just as rpm >>>>>>>> and send HPI each time if receiving rd/wr req. But it will impact >>>>>>>> performance significantly, especially for random iops since the >>>>>>>> weight of executing HPI against r/w small piece of LBAs is >>>>>>>> nonnegligible. >>>>>>>> >>>>>>>> So we now prefer to select the auto one unconditionally if supported >>>>>>>> which makes it as simple as possible. It should really good enough >>>>>>>> for devices to manage its internal policy for bkops rather than the >>>>>>>> host, which makes us believe that we could achieve the best >>>>>>>> performance for all the devices implementing auto bkops and the only >>>>>>>> thing we should do is to disable it when cutting off the power. >>>>>>> >>>>>>> Do you know if there is really a requirement to do that? >>>>>> >>>>>> Even without bkops enable, no matter for manual or auto one, FTL should >>>>>> always do bkops like GC internally when needed to guarantee the >>>>>> performance and balance the wear leveling. What I thought to do is to >>>>>> make it more explicitly. >>>>>> >>>>>> Because then, what >>>>>>> is the point of power off notification? >>>>>> >>>>>> When power off notification is sent, bkops will be stopped >>>>>> in _mmc_suspend. So I don't undertand your point here? >>>>> >>>>> I am trying to understand why we need to do anything for auto bkops. >>>>> Since AUTO_EN is persistent, we can leave the decision whether to turn it on >>>>> to whomever provisions the device. Then we just leave it alone. >>>>> >>>> >>>> Hrm.. >>>> >>>> one possible way is to control it by mmc-utils on >>>> user space? So we should add a cmd for mmc-utils >>>> there? >>> >>> That would be consistent with manual bkops. >>> >> > >From my first impression I agree, as that is the policy we have been >>sticking to when writing to persistent EXT_CSD registers. >>Although, in this case, I am actually wondering on what is the best approach. >> >>Is there really ever a case when we don't want auto BKOPS to be default enabled? >>I think BKOPS is a fundamental feature of an FTL and I can't see a >>reason to why we need to involve mmc-utils/userspace to enable it. Am >>I wrong? > > The even worst case is – involve mmc-utils/userspace to DISABLE it. > I think this register need to be set by vendor and no need to be changed on runtime. If it is set by the Vendor, that's of course the best. Are you saying that we shouldn't enable it during the card init sequence from the kernel, in case it is disabled? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 6/22/16, 5:28 PM, "Ulf Hansson" <ulf.hansson@linaro.org> wrote: >On 22 June 2016 at 16:20, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote: >> >> >> On 6/22/16, 1:21 PM, "Ulf Hansson" <ulf.hansson@linaro.org> wrote: >> >>>On 13 June 2016 at 14:25, Adrian Hunter <adrian.hunter@intel.com> wrote: >>>> On 13/06/16 11:58, Shawn Lin wrote: >>>>> 在 2016/6/13 16:17, Adrian Hunter 写道: >>>>>> On 13/06/16 10:48, Shawn Lin wrote: >>>>>>> On 2016/6/13 14:29, Adrian Hunter wrote: >>>>>>>> On 06/06/16 06:07, Shawn Lin wrote: >>>>>>>>> JEDEC eMMC v5.1 introduce an autonomously initiated method >>>>>>>>> for background operations. >>>>>>>>> >>>>>>>>> Host that wants to enable the device to perform background >>>>>>>>> operations during device idle time, should signal the device >>>>>>>>> by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When >>>>>>>>> this bit is set, the device may start or stop background operations >>>>>>>>> whenever it sees fit, without any notification to the host. >>>>>>>>> >>>>>>>>> When AUTO_EN bit is set, the host should keep the device power >>>>>>>>> active. The host may set or clear this bit at any time based on >>>>>>>>> its power constraints or other considerations. >>>>>>>>> >>>>>>>>> Currently the manual bkops is only be used under the async req >>>>>>>>> circumstances and it's a bit complicated to be controlled as the >>>>>>>>> perfect method is that we should do some idle monitor just as rpm >>>>>>>>> and send HPI each time if receiving rd/wr req. But it will impact >>>>>>>>> performance significantly, especially for random iops since the >>>>>>>>> weight of executing HPI against r/w small piece of LBAs is >>>>>>>>> nonnegligible. >>>>>>>>> >>>>>>>>> So we now prefer to select the auto one unconditionally if supported >>>>>>>>> which makes it as simple as possible. It should really good enough >>>>>>>>> for devices to manage its internal policy for bkops rather than the >>>>>>>>> host, which makes us believe that we could achieve the best >>>>>>>>> performance for all the devices implementing auto bkops and the only >>>>>>>>> thing we should do is to disable it when cutting off the power. >>>>>>>> >>>>>>>> Do you know if there is really a requirement to do that? >>>>>>> >>>>>>> Even without bkops enable, no matter for manual or auto one, FTL should >>>>>>> always do bkops like GC internally when needed to guarantee the >>>>>>> performance and balance the wear leveling. What I thought to do is to >>>>>>> make it more explicitly. >>>>>>> >>>>>>> Because then, what >>>>>>>> is the point of power off notification? >>>>>>> >>>>>>> When power off notification is sent, bkops will be stopped >>>>>>> in _mmc_suspend. So I don't undertand your point here? >>>>>> >>>>>> I am trying to understand why we need to do anything for auto bkops. >>>>>> Since AUTO_EN is persistent, we can leave the decision whether to turn it on >>>>>> to whomever provisions the device. Then we just leave it alone. >>>>>> >>>>> >>>>> Hrm.. >>>>> >>>>> one possible way is to control it by mmc-utils on >>>>> user space? So we should add a cmd for mmc-utils >>>>> there? >>>> >>>> That would be consistent with manual bkops. >>>> >>> >> >From my first impression I agree, as that is the policy we have been >>>sticking to when writing to persistent EXT_CSD registers. >>>Although, in this case, I am actually wondering on what is the best approach. >>> >>>Is there really ever a case when we don't want auto BKOPS to be default enabled? >>>I think BKOPS is a fundamental feature of an FTL and I can't see a >>>reason to why we need to involve mmc-utils/userspace to enable it. Am >>>I wrong? >> >> The even worst case is – involve mmc-utils/userspace to DISABLE it. >> I think this register need to be set by vendor and no need to be changed on runtime. > >If it is set by the Vendor, that's of course the best. It can be set by Storage Vendor. According to the spec, the default value of this bit is vendor specific. > >Are you saying that we shouldn't enable it during the card init >sequence from the kernel, in case it is disabled? No. By the spec – a Host that wants to enable the device to perform background operations during device idle time, should signal the device by setting AUTO_EN in BKOPS_EN field [EXT_CSD byte 163] to 1b. > >Kind regards >Uffe
[...] >>> >From my first impression I agree, as that is the policy we have been >>>>sticking to when writing to persistent EXT_CSD registers. >>>>Although, in this case, I am actually wondering on what is the best approach. >>>> >>>>Is there really ever a case when we don't want auto BKOPS to be default enabled? >>>>I think BKOPS is a fundamental feature of an FTL and I can't see a >>>>reason to why we need to involve mmc-utils/userspace to enable it. Am >>>>I wrong? >>> >>> The even worst case is – involve mmc-utils/userspace to DISABLE it. >>> I think this register need to be set by vendor and no need to be changed on runtime. >> >>If it is set by the Vendor, that's of course the best. > > It can be set by Storage Vendor. > According to the spec, the default value of this bit is vendor specific. > >> >>Are you saying that we shouldn't enable it during the card init >>sequence from the kernel, in case it is disabled? > > No. > By the spec – a Host that wants to enable the device to perform > background operations during device idle time, should signal the > device by setting AUTO_EN in BKOPS_EN field [EXT_CSD byte 163] to 1b. Okay, thanks! Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
在 2016/6/22 22:08, Alex Lemberg 写道: > HI Shawn, > > On 6/21/16, 4:44 AM, "Shawn Lin" <shawn.lin@rock-chips.com> wrote: > >> On 2016/6/20 21:33, Alex Lemberg wrote: >>> Hi Shawn, >>> >>> […] >>> >>>>>> + >>>>>> +static int mmc_stop_auto_bkops(struct mmc_card *card) >>>>>> +{ >>>>>> + int err = 0; >>>>>> + >>>>>> + if (!card->ext_csd.auto_bkops_en) >>>>>> + return 0; >>>>>> + >>>>> >>>>> Shouldn’t the BKOPS_STATUS be checked prior to disabling the BKOPS activity of the device? >>>>> >>>> >>>> Hrmm.. I read the whole section of spec for it, and I did find this >>>> requirement for manul bkops but not for the auto one. So what should we >>>> do if using the auto one? >>>> >>> >>> In case of AUTO BKOPS, the eMMC Device should perform internal GC >>> in the same way as in case of MANUAL BKOPS. >>> The only difference is a host awareness. >> >> agree. >> >>> Although there is no requirement in the spec, I think the driver can >>> give some time to the device to perform/complete its internal GC during the idle time. >>> Thus I think we can check the BKOPS_STATUS on Runtime suspend. >> >> We shouldn't diable bkops on *runtime* suspend as it's just the right >> time for firmware to do GC. We could consider to check and wait for >> the status when doing poweroff, although it seems firmware should be >> able to accept the disable cmd and deal the on-going work perfectly >> when doing bkops without host's awareness, just the same way as suddent >> power loss cases. > > If I am not wrong, in current implementation of runtime suspend, > the driver stops BKOPS (send HPI) just before sending sleep command, > see _mmc_suspend(), depends on “MMC_CAP_AGGRESSIVE_PM” flag. > In this case, the eMMC device will not have enough time to perform internal > BKOPS in both – Manual and Auto BKOPS configurations. > ye, so it seems a pre-exiting issue before introducing auto bkops? I think we can push another patch to improve it but not handling it for this $SUBJECT, does it sound ok to you? > For the poweroff, it should be OK with a current implementation of > PON (mmc_poweroff_notify()) > >> >> Also I don't know whether the firmware will reflect its status on >> BKOPS_STATUS or not when enabling the auto one. I will do more test. >> >> Anyway, thanks for sharing your thought. >> Also Adrian point out that currently we trigger manual bkosp from >> userspace via mmc-utils, and I agreed we shouldn't force kernel stack >> to enable it defaultly. So I'm prone not to update this $SUBJECT and >> migrate it to mmc-utils later. >> >>> >>> […] >>> >>> Thanks, >>> Alex >>> >> >> >> -- >> Best Regards >> Shawn Lin >> >
在 2016/6/22 18:21, Ulf Hansson 写道: > On 13 June 2016 at 14:25, Adrian Hunter <adrian.hunter@intel.com> wrote: >> On 13/06/16 11:58, Shawn Lin wrote: >>> 在 2016/6/13 16:17, Adrian Hunter 写道: >>>> On 13/06/16 10:48, Shawn Lin wrote: >>>>> On 2016/6/13 14:29, Adrian Hunter wrote: >>>>>> On 06/06/16 06:07, Shawn Lin wrote: >>>>>>> JEDEC eMMC v5.1 introduce an autonomously initiated method >>>>>>> for background operations. >>>>>>> >>>>>>> Host that wants to enable the device to perform background >>>>>>> operations during device idle time, should signal the device >>>>>>> by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When >>>>>>> this bit is set, the device may start or stop background operations >>>>>>> whenever it sees fit, without any notification to the host. >>>>>>> >>>>>>> When AUTO_EN bit is set, the host should keep the device power >>>>>>> active. The host may set or clear this bit at any time based on >>>>>>> its power constraints or other considerations. >>>>>>> >>>>>>> Currently the manual bkops is only be used under the async req >>>>>>> circumstances and it's a bit complicated to be controlled as the >>>>>>> perfect method is that we should do some idle monitor just as rpm >>>>>>> and send HPI each time if receiving rd/wr req. But it will impact >>>>>>> performance significantly, especially for random iops since the >>>>>>> weight of executing HPI against r/w small piece of LBAs is >>>>>>> nonnegligible. >>>>>>> >>>>>>> So we now prefer to select the auto one unconditionally if supported >>>>>>> which makes it as simple as possible. It should really good enough >>>>>>> for devices to manage its internal policy for bkops rather than the >>>>>>> host, which makes us believe that we could achieve the best >>>>>>> performance for all the devices implementing auto bkops and the only >>>>>>> thing we should do is to disable it when cutting off the power. >>>>>> >>>>>> Do you know if there is really a requirement to do that? >>>>> >>>>> Even without bkops enable, no matter for manual or auto one, FTL should >>>>> always do bkops like GC internally when needed to guarantee the >>>>> performance and balance the wear leveling. What I thought to do is to >>>>> make it more explicitly. >>>>> >>>>> Because then, what >>>>>> is the point of power off notification? >>>>> >>>>> When power off notification is sent, bkops will be stopped >>>>> in _mmc_suspend. So I don't undertand your point here? >>>> >>>> I am trying to understand why we need to do anything for auto bkops. >>>> Since AUTO_EN is persistent, we can leave the decision whether to turn it on >>>> to whomever provisions the device. Then we just leave it alone. >>>> >>> >>> Hrm.. >>> >>> one possible way is to control it by mmc-utils on >>> user space? So we should add a cmd for mmc-utils >>> there? >> >> That would be consistent with manual bkops. >> > >>From my first impression I agree, as that is the policy we have been > sticking to when writing to persistent EXT_CSD persistent . > Although, in this case, I am actually wondering on what is the best approach. I don't know what is the real meaning of "persistent". :) I don't know should we count auto bkops as the persistent registers....HS_TIMING and BUS_WIDTH should also be persistent registers as them are always used after initialization if not changing them? IHMO the more reasonable way is that: IIRC many settings for EXT_CSD should be OTP, like hw-reset(162), reliable write(167) fw-configure(169)..etc, which are marked as R/W. These should be controlled by userpace or even by firmware when flashing emmc, like reliable write... I'm not sure whether should I updete this $SUBJUCT or migirating it to userspace... We need to come to an agreement :) > > Is there really ever a case when we don't want auto BKOPS to be default enabled? > I think BKOPS is a fundamental feature of an FTL and I can't see a > reason to why we need to involve mmc-utils/userspace to enable it. Am > I wrong? > > Kind regards > Uffe > > >
On 23/06/16 04:33, Shawn Lin wrote: > 在 2016/6/22 22:08, Alex Lemberg 写道: >> HI Shawn, >> >> On 6/21/16, 4:44 AM, "Shawn Lin" <shawn.lin@rock-chips.com> wrote: >> >>> On 2016/6/20 21:33, Alex Lemberg wrote: >>>> Hi Shawn, >>>> >>>> […] >>>> >>>>>>> + >>>>>>> +static int mmc_stop_auto_bkops(struct mmc_card *card) >>>>>>> +{ >>>>>>> + int err = 0; >>>>>>> + >>>>>>> + if (!card->ext_csd.auto_bkops_en) >>>>>>> + return 0; >>>>>>> + >>>>>> >>>>>> Shouldn’t the BKOPS_STATUS be checked prior to disabling the BKOPS >>>>>> activity of the device? >>>>>> >>>>> >>>>> Hrmm.. I read the whole section of spec for it, and I did find this >>>>> requirement for manul bkops but not for the auto one. So what should we >>>>> do if using the auto one? >>>>> >>>> >>>> In case of AUTO BKOPS, the eMMC Device should perform internal GC >>>> in the same way as in case of MANUAL BKOPS. >>>> The only difference is a host awareness. >>> >>> agree. >>> >>>> Although there is no requirement in the spec, I think the driver can >>>> give some time to the device to perform/complete its internal GC during >>>> the idle time. >>>> Thus I think we can check the BKOPS_STATUS on Runtime suspend. >>> >>> We shouldn't diable bkops on *runtime* suspend as it's just the right >>> time for firmware to do GC. We could consider to check and wait for >>> the status when doing poweroff, although it seems firmware should be >>> able to accept the disable cmd and deal the on-going work perfectly >>> when doing bkops without host's awareness, just the same way as suddent >>> power loss cases. >> >> If I am not wrong, in current implementation of runtime suspend, >> the driver stops BKOPS (send HPI) just before sending sleep command, >> see _mmc_suspend(), depends on “MMC_CAP_AGGRESSIVE_PM” flag. >> In this case, the eMMC device will not have enough time to perform internal >> BKOPS in both – Manual and Auto BKOPS configurations. >> > > ye, so it seems a pre-exiting issue before introducing auto bkops? > I think we can push another patch to improve it but not handling > it for this $SUBJECT, does it sound ok to you? Runtime suspend for eMMC has a default auto-suspend delay of 3 seconds (refer mmc_blk_probe()). Isn't that when auto bkops would happen? > >> For the poweroff, it should be OK with a current implementation of >> PON (mmc_poweroff_notify()) >> >>> >>> Also I don't know whether the firmware will reflect its status on >>> BKOPS_STATUS or not when enabling the auto one. I will do more test. >>> >>> Anyway, thanks for sharing your thought. >>> Also Adrian point out that currently we trigger manual bkosp from >>> userspace via mmc-utils, and I agreed we shouldn't force kernel stack >>> to enable it defaultly. So I'm prone not to update this $SUBJECT and >>> migrate it to mmc-utils later. >>> >>>> >>>> […] >>>> >>>> Thanks, >>>> Alex >>>> >>> >>> >>> -- >>> Best Regards >>> Shawn Lin >>> >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[...] >>>> one possible way is to control it by mmc-utils on >>>> user space? So we should add a cmd for mmc-utils >>>> there? >>> >>> >>> That would be consistent with manual bkops. >>> >> >>> From my first impression I agree, as that is the policy we have been >> >> sticking to when writing to persistent EXT_CSD persistent . >> Although, in this case, I am actually wondering on what is the best >> approach. > > > I don't know what is the real meaning of "persistent". :) > I don't know should we count auto bkops as the persistent > registers....HS_TIMING and BUS_WIDTH should also be persistent > registers as them are always used after initialization if not changing > them? > > IHMO the more reasonable way is that: > IIRC many settings for EXT_CSD should be OTP, like hw-reset(162), > reliable write(167) fw-configure(169)..etc, which are marked as R/W. > These should be controlled by userpace or even by firmware when > flashing emmc, like reliable write... Yes, that's makes sense to me. OTP registers must only be written to via mmc utils/userspace. > > > I'm not sure whether should I updete this $SUBJUCT or migirating it to > userspace... We need to come to an agreement :) > I don't have any issue to allow all non-OTP registers bits to be written. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[...] >>> If I am not wrong, in current implementation of runtime suspend, >>> the driver stops BKOPS (send HPI) just before sending sleep command, >>> see _mmc_suspend(), depends on “MMC_CAP_AGGRESSIVE_PM” flag. >>> In this case, the eMMC device will not have enough time to perform internal >>> BKOPS in both – Manual and Auto BKOPS configurations. >>> >> >> ye, so it seems a pre-exiting issue before introducing auto bkops? >> I think we can push another patch to improve it but not handling >> it for this $SUBJECT, does it sound ok to you? > > Runtime suspend for eMMC has a default auto-suspend delay of 3 seconds > (refer mmc_blk_probe()). Isn't that when auto bkops would happen? That's correct. So perhaps we should extend the default auto-suspend delay? The SD case actually have the same issue, as I some background operations exists there as well. [...] Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
SGkgVWxmIGFuZCBBZHJpYW4sDQoNCg0KT24gNi8yNy8xNiwgMTI6MDggUE0sICJVbGYgSGFuc3Nv biIgPHVsZi5oYW5zc29uQGxpbmFyby5vcmc+IHdyb3RlOg0KDQo+Wy4uLl0NCj4NCj4+Pj4gSWYg SSBhbSBub3Qgd3JvbmcsIGluIGN1cnJlbnQgaW1wbGVtZW50YXRpb24gb2YgcnVudGltZSBzdXNw ZW5kLA0KPj4+PiB0aGUgZHJpdmVyIHN0b3BzIEJLT1BTIChzZW5kIEhQSSkganVzdCBiZWZvcmUg c2VuZGluZyBzbGVlcCBjb21tYW5kLA0KPj4+PiBzZWUgX21tY19zdXNwZW5kKCksIGRlcGVuZHMg b24g4oCcTU1DX0NBUF9BR0dSRVNTSVZFX1BN4oCdIGZsYWcuDQo+Pj4+IEluIHRoaXMgY2FzZSwg dGhlIGVNTUMgZGV2aWNlIHdpbGwgbm90IGhhdmUgZW5vdWdoIHRpbWUgdG8gcGVyZm9ybSBpbnRl cm5hbA0KPj4+PiBCS09QUyBpbiBib3RoIOKAkyBNYW51YWwgYW5kIEF1dG8gQktPUFMgY29uZmln dXJhdGlvbnMuDQo+Pj4+DQo+Pj4NCj4+PiB5ZSwgc28gaXQgc2VlbXMgYSBwcmUtZXhpdGluZyBp c3N1ZSBiZWZvcmUgaW50cm9kdWNpbmcgYXV0byBia29wcz8NCj4+PiBJIHRoaW5rIHdlIGNhbiBw dXNoIGFub3RoZXIgcGF0Y2ggdG8gaW1wcm92ZSBpdCBidXQgbm90IGhhbmRsaW5nDQo+Pj4gaXQg Zm9yIHRoaXMgJFNVQkpFQ1QsIGRvZXMgaXQgc291bmQgb2sgdG8geW91Pw0KPj4NCj4+IFJ1bnRp bWUgc3VzcGVuZCBmb3IgZU1NQyBoYXMgYSBkZWZhdWx0IGF1dG8tc3VzcGVuZCBkZWxheSBvZiAz IHNlY29uZHMNCj4+IChyZWZlciBtbWNfYmxrX3Byb2JlKCkpLiAgSXNuJ3QgdGhhdCB3aGVuIGF1 dG8gYmtvcHMgd291bGQgaGFwcGVuPw0KPg0KPlRoYXQncyBjb3JyZWN0Lg0KPg0KPlNvIHBlcmhh cHMgd2Ugc2hvdWxkIGV4dGVuZCB0aGUgZGVmYXVsdCBhdXRvLXN1c3BlbmQgZGVsYXk/DQoNCkR1 ZSB0byB0aGUgZGlmZmVyZW5jZXMgaW4gZmxhc2ggbWFuYWdlbWVudCBhbmQgdGVjaG5vbG9naWVz LA0KSSBzdWdnZXN0IHRvIG1ha2UgdGhpcyB2YWx1ZSBjb25maWd1cmFibGUsIGFuZCBub3QgaGFy ZCBjb2RlZC4NCkkgYmVsaWV2ZSBpdCBzaG91bGQgYmUgY29uZmlndXJlZCBwZXIgdmVuZG9yIEJL T1BTIG5lZWRzLg0KVGhlIGRlZmF1bHQgdmFsdWUgc3RpbGwgY2FuIGJlIHNldCB0byAzIHNlY29u ZHMsIGFzIGRvbmUgdG9kYXnigKYNCg0KPg0KPlRoZSBTRCBjYXNlIGFjdHVhbGx5IGhhdmUgdGhl IHNhbWUgaXNzdWUsIGFzIEkgc29tZSBiYWNrZ3JvdW5kDQo+b3BlcmF0aW9ucyBleGlzdHMgdGhl cmUgYXMgd2VsbC4NCj4NCj5bLi4uXQ0KPg0KPktpbmQgcmVnYXJkcw0KPlVmZmUNCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index e864187..c2e5a66 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -297,24 +297,13 @@ static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq) return 0; } -/** - * mmc_start_bkops - start BKOPS for supported cards - * @card: MMC card to start BKOPS - * @form_exception: A flag to indicate if this function was - * called due to an exception raised by the card - * - * Start background operations whenever requested. - * When the urgent BKOPS bit is set in a R1 command response - * then background operations should be started immediately. -*/ -void mmc_start_bkops(struct mmc_card *card, bool from_exception) +static void mmc_start_man_bkops(struct mmc_card *card, + bool from_exception) { int err; int timeout; bool use_busy_signal; - BUG_ON(!card); - if (!card->ext_csd.man_bkops_en || mmc_card_doing_bkops(card)) return; @@ -347,7 +336,7 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception) EXT_CSD_BKOPS_START, 1, timeout, use_busy_signal, true, false); if (err) { - pr_warn("%s: Error %d starting bkops\n", + pr_warn("%s: Error %d starting manual bkops\n", mmc_hostname(card->host), err); mmc_retune_release(card->host); goto out; @@ -365,6 +354,55 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception) out: mmc_release_host(card->host); } + +static void mmc_start_auto_bkops(struct mmc_card *card) +{ + int err; + + /* If it's already enable auto_bkops, nothing to do */ + if (card->ext_csd.auto_bkops_en) + return; + + mmc_claim_host(card->host); + + mmc_retune_hold(card->host); + + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, + EXT_CSD_BKOPS_EN, 2, + card->ext_csd.generic_cmd6_time); + if (err) + pr_warn("%s: Error %d starting auto bkops\n", + mmc_hostname(card->host), err); + + card->ext_csd.auto_bkops_en = true; + mmc_card_set_doing_bkops(card); + mmc_retune_release(card->host); + mmc_release_host(card->host); +} + + +/** + * mmc_start_bkops - start BKOPS for supported cards + * @card: MMC card to start BKOPS + * @form_exception: A flag to indicate if this function was + * called due to an exception raised by the card + * @is_auto: A flag to indicate if we should use auto bkops + * + * Start background operations whenever requested. + * When the urgent BKOPS bit is set in a R1 command response + * then background operations should be started immediately, which is + * only needed for man_bkops. +*/ +void mmc_start_bkops(struct mmc_card *card, bool from_exception, + bool is_auto) +{ + BUG_ON(!card); + + if (is_auto) + return mmc_start_auto_bkops(card); + else + return mmc_start_man_bkops(card, from_exception); +} EXPORT_SYMBOL(mmc_start_bkops); /* @@ -610,7 +648,9 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host, if (areq) mmc_post_req(host, areq->mrq, -EINVAL); - mmc_start_bkops(host->card, true); + /* Prefer to use auto bkops for eMMC 5.1 or later */ + mmc_start_bkops(host->card, true, + host->card->ext_csd.rev >= 8); /* prepare the request again */ if (areq) @@ -751,20 +791,10 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries EXPORT_SYMBOL(mmc_wait_for_cmd); -/** - * mmc_stop_bkops - stop ongoing BKOPS - * @card: MMC card to check BKOPS - * - * Send HPI command to stop ongoing background operations to - * allow rapid servicing of foreground operations, e.g. read/ - * writes. Wait until the card comes out of the programming state - * to avoid errors in servicing read/write requests. - */ -int mmc_stop_bkops(struct mmc_card *card) +static int mmc_stop_man_bkops(struct mmc_card *card) { int err = 0; - BUG_ON(!card); err = mmc_interrupt_hpi(card); /* @@ -779,6 +809,51 @@ int mmc_stop_bkops(struct mmc_card *card) return err; } + +static int mmc_stop_auto_bkops(struct mmc_card *card) +{ + int err = 0; + + if (!card->ext_csd.auto_bkops_en) + return 0; + + err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, + EXT_CSD_BKOPS_EN, 0, MMC_BKOPS_MAX_TIMEOUT, + true, true, false); + if (err) + pr_warn("%s: Error %d stoping auto bkops\n", + mmc_hostname(card->host), err); + + card->ext_csd.auto_bkops_en = false; + mmc_card_clr_doing_bkops(card); + mmc_retune_release(card->host); + + return err; +} + +/** + * mmc_stop_bkops - stop ongoing BKOPS + * @card: MMC card to check BKOPS + * @is_auto: A flag to indicate if we should use auto bkops + * + * Send HPI command to stop ongoing background operations to + * allow rapid servicing of foreground operations, e.g. read/ + * writes. Wait until the card comes out of the programming state + * to avoid errors in servicing read/write requests. + * + * But for auto bkops, we could only disable AUTO_EN instead of + * sending HPI. And the firmware could do it automatically. + * + */ +int mmc_stop_bkops(struct mmc_card *card, bool is_auto) +{ + BUG_ON(!card); + + if (is_auto) + return mmc_stop_auto_bkops(card); + else + return mmc_stop_man_bkops(card); +} EXPORT_SYMBOL(mmc_stop_bkops); int mmc_read_bkops_status(struct mmc_card *card) diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 61d6b34..c65bea7 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -508,6 +508,13 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) /* check whether the eMMC card supports BKOPS */ if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1) { card->ext_csd.bkops = 1; + + /* for eMMC v5.1 or later, we could use auto_bkops */ + if (card->ext_csd.rev >= 8) + card->ext_csd.auto_bkops_en = + (ext_csd[EXT_CSD_BKOPS_EN] & + EXT_CSD_AUTO_BKOPS_MASK); + card->ext_csd.man_bkops_en = (ext_csd[EXT_CSD_BKOPS_EN] & EXT_CSD_MANUAL_BKOPS_MASK); @@ -516,6 +523,9 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) if (!card->ext_csd.man_bkops_en) pr_debug("%s: MAN_BKOPS_EN bit is not set\n", mmc_hostname(card->host)); + if (!card->ext_csd.auto_bkops_en) + pr_debug("%s: AUTO_BKOPS_EN bit is not set\n", + mmc_hostname(card->host)); } /* check whether the eMMC card supports HPI */ @@ -1241,7 +1251,7 @@ static int mmc_select_hs400es(struct mmc_card *card) } err = mmc_select_bus_width(card); - if (IS_ERR_VALUE(err)) + if (err < 0) goto out_err; /* Switch card to HS mode */ @@ -1676,6 +1686,21 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, mmc_select_powerclass(card); /* + * Enable auto bkops feature which is mandatory for + * eMMC v5.1 or later + */ + if (card->ext_csd.rev >= 8) { + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, + EXT_CSD_BKOPS_EN, 2, + card->ext_csd.generic_cmd6_time); + if (err) + pr_warn("%s: Error %d starting auto bkops\n", + mmc_hostname(card->host), err); + + card->ext_csd.auto_bkops_en = true; + } + + /* * Enable HPI feature (if supported) */ if (card->ext_csd.hpi) { @@ -1898,7 +1923,8 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) goto out; if (mmc_card_doing_bkops(host->card)) { - err = mmc_stop_bkops(host->card); + err = mmc_stop_bkops(host->card, + host->card->ext_csd.rev >= 8); if (err) goto out; } diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index 22defc2..f82f9be 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -84,6 +84,7 @@ struct mmc_ext_csd { unsigned int hpi_cmd; /* cmd used as HPI */ bool bkops; /* background support bit */ bool man_bkops_en; /* manual bkops enable bit */ + bool auto_bkops_en; /* auto bkops enbale bit */ unsigned int data_sector_size; /* 512 bytes or 4KB */ unsigned int data_tag_unit_size; /* DATA TAG UNIT size */ unsigned int boot_ro_lock; /* ro lock support */ diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index b01e77d..45d53ef 100644 --- a/include/linux/mmc/core.h +++ b/include/linux/mmc/core.h @@ -140,7 +140,7 @@ struct mmc_request { struct mmc_card; struct mmc_async_req; -extern int mmc_stop_bkops(struct mmc_card *); +extern int mmc_stop_bkops(struct mmc_card *, bool); extern int mmc_read_bkops_status(struct mmc_card *); extern struct mmc_async_req *mmc_start_req(struct mmc_host *, struct mmc_async_req *, int *); @@ -150,7 +150,7 @@ extern int mmc_wait_for_cmd(struct mmc_host *, struct mmc_command *, int); extern int mmc_app_cmd(struct mmc_host *, struct mmc_card *); extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *, struct mmc_command *, int); -extern void mmc_start_bkops(struct mmc_card *card, bool from_exception); +extern void mmc_start_bkops(struct mmc_card *, bool, bool); extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int); extern int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error); extern int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd); diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h index c376209..a1cced9 100644 --- a/include/linux/mmc/mmc.h +++ b/include/linux/mmc/mmc.h @@ -436,6 +436,7 @@ struct _mmc_csd { * BKOPS modes */ #define EXT_CSD_MANUAL_BKOPS_MASK 0x01 +#define EXT_CSD_AUTO_BKOPS_MASK 0x02 /* * MMC_SWITCH access modes
JEDEC eMMC v5.1 introduce an autonomously initiated method for background operations. Host that wants to enable the device to perform background operations during device idle time, should signal the device by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When this bit is set, the device may start or stop background operations whenever it sees fit, without any notification to the host. When AUTO_EN bit is set, the host should keep the device power active. The host may set or clear this bit at any time based on its power constraints or other considerations. Currently the manual bkops is only be used under the async req circumstances and it's a bit complicated to be controlled as the perfect method is that we should do some idle monitor just as rpm and send HPI each time if receiving rd/wr req. But it will impact performance significantly, especially for random iops since the weight of executing HPI against r/w small piece of LBAs is nonnegligible. So we now prefer to select the auto one unconditionally if supported which makes it as simple as possible. It should really good enough for devices to manage its internal policy for bkops rather than the host, which makes us believe that we could achieve the best performance for all the devices implementing auto bkops and the only thing we should do is to disable it when cutting off the power. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- drivers/mmc/core/core.c | 127 +++++++++++++++++++++++++++++++++++++---------- drivers/mmc/core/mmc.c | 30 ++++++++++- include/linux/mmc/card.h | 1 + include/linux/mmc/core.h | 4 +- include/linux/mmc/mmc.h | 1 + 5 files changed, 133 insertions(+), 30 deletions(-)