* [PATCH] scsi: ufs: Allow RTT negotiation
@ 2024-05-02 13:19 Avri Altman
2024-05-02 19:51 ` Bean Huo
0 siblings, 1 reply; 3+ messages in thread
From: Avri Altman @ 2024-05-02 13:19 UTC (permalink / raw)
To: Martin K . Petersen
Cc: Bart Van Assche, Bean Huo, linux-scsi, linux-kernel, Avri Altman
The rtt-upiu packets preceed any data-out upiu packets, thus
synchronizing the data input to the device: this mostly applies to write
operations, but there are other operations that requires rtt as well.
There are several rules binding this rtt - data-out dialog, specifically
There can be at most outstanding bMaxNumOfRTT such packets. This might
have an effect on write performance (sequential write in particular), as
each data-out upiu must wait for its rtt sibling.
UFSHCI expects bMaxNumOfRTT to be min(bDeviceRTTCap, NORTT). However,
as of today, there does not appears to be no-one who sets it: not the
host controller nor the driver. It wasn't an issue up to now:
bMaxNumOfRTT is set to 2 after manufacturing, and wasn't limiting the
write performance.
UFS4.0, and specifically gear 5 changes this, and requires the device to
be more attentive. This doesn't come free - the device has to allocate
more resources to that end, but the sequential write performance
improvement is significant. Early measurements shows 25% gain when
moving from rtt 2 to 9. Therefore, set bMaxNumOfRTT to be
min(bDeviceRTTCap, NORTT) as UFSHCI expects.
Signed-off-by: Avri Altman <avri.altman@wdc.com>
---
drivers/ufs/core/ufshcd.c | 23 +++++++++++++++++++++++
include/ufs/ufshcd.h | 2 ++
include/ufs/ufshci.h | 1 +
3 files changed, 26 insertions(+)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 0819ddafe7a6..8db9304eb418 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2405,6 +2405,8 @@ static inline int ufshcd_hba_capabilities(struct ufs_hba *hba)
((hba->capabilities & MASK_TASK_MANAGEMENT_REQUEST_SLOTS) >> 16) + 1;
hba->reserved_slot = hba->nutrs - 1;
+ hba->nortt = FIELD_GET(MASK_NUMBER_OUTSTANDING_RTT, hba->capabilities) + 1;
+
/* Read crypto capabilities */
err = ufshcd_hba_init_crypto_capabilities(hba);
if (err) {
@@ -8119,6 +8121,25 @@ static void ufshcd_ext_iid_probe(struct ufs_hba *hba, u8 *desc_buf)
dev_info->b_ext_iid_en = ext_iid_en;
}
+static void ufshcd_rtt_set(struct ufs_hba *hba, u8 *desc_buf)
+{
+ struct ufs_dev_info *dev_info = &hba->dev_info;
+ u32 rtt = 0;
+
+ /* RTT override makes sense only for UFS-4.0 and above */
+ if (dev_info->wspecversion < 0x400)
+ return;
+
+ rtt = min_t(int, desc_buf[DEVICE_DESC_PARAM_RTT_CAP], hba->nortt);
+ /* rtt is 2 after manufacturing */
+ if (rtt < 3)
+ return;
+
+ if (ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
+ QUERY_ATTR_IDN_MAX_NUM_OF_RTT, 0, 0, &rtt))
+ dev_err(hba->dev, "failed writing bMaxNumOfRTT\n");
+}
+
void ufshcd_fixup_dev_quirks(struct ufs_hba *hba,
const struct ufs_dev_quirk *fixups)
{
@@ -8278,6 +8299,8 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
if (hba->ext_iid_sup)
ufshcd_ext_iid_probe(hba, desc_buf);
+ ufshcd_rtt_set(hba, desc_buf);
+
/*
* ufshcd_read_string_desc returns size of the string
* reset the error value
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index bad88bd91995..d74bd2d67b06 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -819,6 +819,7 @@ enum ufshcd_mcq_opr {
* @capabilities: UFS Controller Capabilities
* @mcq_capabilities: UFS Multi Circular Queue capabilities
* @nutrs: Transfer Request Queue depth supported by controller
+ * @nortt - Max outstanding RTTs supported by controller
* @nutmrs: Task Management Queue depth supported by controller
* @reserved_slot: Used to submit device commands. Protected by @dev_cmd.lock.
* @ufs_version: UFS Version to which controller complies
@@ -957,6 +958,7 @@ struct ufs_hba {
u32 capabilities;
int nutrs;
+ int nortt;
u32 mcq_capabilities;
int nutmrs;
u32 reserved_slot;
diff --git a/include/ufs/ufshci.h b/include/ufs/ufshci.h
index 385e1c6b8d60..c50f92bf2e1d 100644
--- a/include/ufs/ufshci.h
+++ b/include/ufs/ufshci.h
@@ -68,6 +68,7 @@ enum {
/* Controller capability masks */
enum {
MASK_TRANSFER_REQUESTS_SLOTS = 0x0000001F,
+ MASK_NUMBER_OUTSTANDING_RTT = 0x0000FF00,
MASK_TASK_MANAGEMENT_REQUEST_SLOTS = 0x00070000,
MASK_EHSLUTRD_SUPPORTED = 0x00400000,
MASK_AUTO_HIBERN8_SUPPORT = 0x00800000,
--
2.42.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] scsi: ufs: Allow RTT negotiation
2024-05-02 13:19 [PATCH] scsi: ufs: Allow RTT negotiation Avri Altman
@ 2024-05-02 19:51 ` Bean Huo
2024-05-03 10:43 ` Avri Altman
0 siblings, 1 reply; 3+ messages in thread
From: Bean Huo @ 2024-05-02 19:51 UTC (permalink / raw)
To: Avri Altman, Martin K . Petersen
Cc: Bart Van Assche, Bean Huo, linux-scsi, linux-kernel
Avri,
On 02.05.24 3:19 PM, Avri Altman wrote:
> + /* RTT override makes sense only for UFS-4.0 and above */
> + if (dev_info->wspecversion < 0x400)
> + return;
> +
> + rtt = min_t(int, desc_buf[DEVICE_DESC_PARAM_RTT_CAP], hba->nortt);
> + /* rtt is 2 after manufacturing */
> + if (rtt < 3)
> + return;
> +
> + if (ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> + QUERY_ATTR_IDN_MAX_NUM_OF_RTT, 0, 0, &rtt))
> + dev_err(hba->dev, "failed writing bMaxNumOfRTT\n");
> +}
bMaxNumOfRTT is Persistent Property, do we need to re-write every time
power cycle?
Kind regards,
Bean
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH] scsi: ufs: Allow RTT negotiation
2024-05-02 19:51 ` Bean Huo
@ 2024-05-03 10:43 ` Avri Altman
0 siblings, 0 replies; 3+ messages in thread
From: Avri Altman @ 2024-05-03 10:43 UTC (permalink / raw)
To: Bean Huo, Martin K . Petersen
Cc: Bart Van Assche, Bean Huo, linux-scsi, linux-kernel
> Avri,
>
> On 02.05.24 3:19 PM, Avri Altman wrote:
> > + /* RTT override makes sense only for UFS-4.0 and above */
> > + if (dev_info->wspecversion < 0x400)
> > + return;
> > +
> > + rtt = min_t(int, desc_buf[DEVICE_DESC_PARAM_RTT_CAP], hba->nortt);
> > + /* rtt is 2 after manufacturing */
> > + if (rtt < 3)
> > + return;
> > +
> > + if (ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> > + QUERY_ATTR_IDN_MAX_NUM_OF_RTT, 0, 0, &rtt))
> > + dev_err(hba->dev, "failed writing bMaxNumOfRTT\n"); }
>
> bMaxNumOfRTT is Persistent Property, do we need to re-write every time
> power cycle?
Correct.
Moreover, don't want to override use-space should it was written.
Will fix.
Thanks,
Avri
>
> Kind regards,
>
> Bean
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-03 10:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-02 13:19 [PATCH] scsi: ufs: Allow RTT negotiation Avri Altman
2024-05-02 19:51 ` Bean Huo
2024-05-03 10:43 ` Avri Altman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).