RE: [PATCH v32 4/4] scsi: ufs: Add HPB 2.0 support
> Hi,
> > if (dev_info->wspecversion >= UFS_DEV_HPB_SUPPORT_VERSION &&
> > (b_ufs_feature_sup & UFS_DEV_HPB_SUPPORT)) {
> > - dev_info->hpb_enabled = true;
> > + bool hpb_en = false;
> > +
> > ufshpb_get_dev_info(hba, desc_buf);
> > +
> > + if (!ufshpb_is_legacy(hba))
> > + err = ufshcd_query_flag_retry(hba,
> > +
> > UPIU_QUERY_OPCODE_READ_FLAG,
> > +
> > QUERY_FLAG_IDN_HPB_EN, 0,
> > + _en);
> > +
> > + if (ufshpb_is_legacy(hba) || (!err && hpb_en))
> > + dev_info->hpb_enabled = true;
> > }
> I think there is a confusion concerning fHPBEn flag.
> The spec say: "If host wants to enable HPB, host set the fHPBEn flag as ‘1’."
> And its default value is '0'.
> So upon successful init, we should set this flag and not read it.
After some further thinking, I wish to withdraw from my comment above.
The spec doesn't really say how this flag should be used, but provides the
"system"
With a mean to disable hpb.
So I tend to agree with your interpretation, that this flag should be
configured by the OEMs,
Along with all other hpb configurables, should they choose to enable it.
As it is of a persistent nature, we might even consider in the future,
to add some logic that will allow to use this flag to disable hpb.
Thanks,
Avri
>
> I wouldn't rush to fix it however, before we see what Martin/Greg are planning
> for this feature.
> Thanks,
> Avri
RE: [PATCH v32 4/4] scsi: ufs: Add HPB 2.0 support
Hi,
> if (dev_info->wspecversion >= UFS_DEV_HPB_SUPPORT_VERSION &&
> (b_ufs_feature_sup & UFS_DEV_HPB_SUPPORT)) {
> - dev_info->hpb_enabled = true;
> + bool hpb_en = false;
> +
> ufshpb_get_dev_info(hba, desc_buf);
> +
> + if (!ufshpb_is_legacy(hba))
> + err = ufshcd_query_flag_retry(hba,
> +
> UPIU_QUERY_OPCODE_READ_FLAG,
> + QUERY_FLAG_IDN_HPB_EN,
> 0,
> + _en);
> +
> + if (ufshpb_is_legacy(hba) || (!err && hpb_en))
> + dev_info->hpb_enabled = true;
> }
I think there is a confusion concerning fHPBEn flag.
The spec say: "If host wants to enable HPB, host set the fHPBEn flag as ‘1’."
And its default value is '0'.
So upon successful init, we should set this flag and not read it.
I wouldn't rush to fix it however, before we see what Martin/Greg are planning
for this feature.
Thanks,
Avri
RE: [PATCH v32 4/4] scsi: ufs: Add HPB 2.0 support
> From: Daejun Park
>
> @@ -1692,6 +2188,7 @@ static void ufshpb_hpb_lu_prepared(struct ufs_hba
> *hba)
> ufshpb_set_state(hpb, HPB_PRESENT);
> if ((hpb->lu_pinned_end - hpb->lu_pinned_start) > 0)
> queue_work(ufshpb_wq, >map_work);
> + ufshpb_issue_umap_all_req(hpb);
> HPB-WRITE-BUFFER with buffer-id = 0x3h is supported in device control mode
> only
> so I think it is necessary to distinguish device mode and host mode.
You are correct.
Since Daejun's driver's scope is device mode only - see ufshpb_get_dev_info,
This needs to be fixed in the host mode series.
I will fix it in my next version.
Thanks,
Avri
> } else {
> dev_err(hba->dev, "destroy HPB lu %d\n", hpb->lun);
> ufshpb_destroy_lu(hba, sdev);
>
Re: [PATCH v32 4/4] scsi: ufs: Add HPB 2.0 support
On 2021-03-31 09:18, Daejun Park wrote: This patch supports the HPB 2.0. The HPB 2.0 supports read of varying sizes from 4KB to 512KB. In the case of Read (<= 32KB) is supported as single HPB read. In the case of Read (36KB ~ 512KB) is supported by as a combination of write buffer command and HPB read command to deliver more PPN. The write buffer commands may not be issued immediately due to busy tags. To use HPB read more aggressively, the driver can requeue the write buffer command. The requeue threshold is implemented as timeout and can be modified with requeue_timeout_ms entry in sysfs. Reviewed-by: Can Guo Reviewed-by: Bean Huo Signed-off-by: Daejun Park --- Please allow me a few more days in April to have this whole series tested one more time. Thanks, Can Guo.
[PATCH v32 4/4] scsi: ufs: Add HPB 2.0 support
This patch supports the HPB 2.0. The HPB 2.0 supports read of varying sizes from 4KB to 512KB. In the case of Read (<= 32KB) is supported as single HPB read. In the case of Read (36KB ~ 512KB) is supported by as a combination of write buffer command and HPB read command to deliver more PPN. The write buffer commands may not be issued immediately due to busy tags. To use HPB read more aggressively, the driver can requeue the write buffer command. The requeue threshold is implemented as timeout and can be modified with requeue_timeout_ms entry in sysfs. Reviewed-by: Can Guo Reviewed-by: Bean Huo Signed-off-by: Daejun Park --- Documentation/ABI/testing/sysfs-driver-ufs | 47 +- drivers/scsi/ufs/ufs-sysfs.c | 4 + drivers/scsi/ufs/ufs.h | 3 +- drivers/scsi/ufs/ufshcd.c | 25 +- drivers/scsi/ufs/ufshcd.h | 7 + drivers/scsi/ufs/ufshpb.c | 623 +++-- drivers/scsi/ufs/ufshpb.h | 67 ++- 7 files changed, 696 insertions(+), 80 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs index 528bf89fc98b..419adf450b89 100644 --- a/Documentation/ABI/testing/sysfs-driver-ufs +++ b/Documentation/ABI/testing/sysfs-driver-ufs @@ -1253,14 +1253,14 @@ Description:This entry shows the number of HPB pinned regions assigned to The file is read only. -What: /sys/class/scsi_device/*/device/hpb_sysfs/hit_cnt +What: /sys/class/scsi_device/*/device/hpb_stat_sysfs/hit_cnt Date: March 2021 Contact: Daejun Park Description: This entry shows the number of reads that changed to HPB read. The file is read only. -What: /sys/class/scsi_device/*/device/hpb_sysfs/miss_cnt +What: /sys/class/scsi_device/*/device/hpb_stat_sysfs/miss_cnt Date: March 2021 Contact: Daejun Park Description: This entry shows the number of reads that cannot be changed to @@ -1268,7 +1268,7 @@ Description: This entry shows the number of reads that cannot be changed to The file is read only. -What: /sys/class/scsi_device/*/device/hpb_sysfs/rb_noti_cnt +What: /sys/class/scsi_device/*/device/hpb_stat_sysfs/rb_noti_cnt Date: March 2021 Contact: Daejun Park Description: This entry shows the number of response UPIUs that has @@ -1276,7 +1276,7 @@ Description: This entry shows the number of response UPIUs that has The file is read only. -What: /sys/class/scsi_device/*/device/hpb_sysfs/rb_active_cnt +What: /sys/class/scsi_device/*/device/hpb_stat_sysfs/rb_active_cnt Date: March 2021 Contact: Daejun Park Description: This entry shows the number of active sub-regions recommended by @@ -1284,7 +1284,7 @@ Description: This entry shows the number of active sub-regions recommended by The file is read only. -What: /sys/class/scsi_device/*/device/hpb_sysfs/rb_inactive_cnt +What: /sys/class/scsi_device/*/device/hpb_stat_sysfs/rb_inactive_cnt Date: March 2021 Contact: Daejun Park Description: This entry shows the number of inactive regions recommended by @@ -1292,10 +1292,45 @@ Description:This entry shows the number of inactive regions recommended by The file is read only. -What: /sys/class/scsi_device/*/device/hpb_sysfs/map_req_cnt +What: /sys/class/scsi_device/*/device/hpb_stat_sysfs/map_req_cnt Date: March 2021 Contact: Daejun Park Description: This entry shows the number of read buffer commands for activating sub-regions recommended by response UPIUs. The file is read only. + +What: /sys/class/scsi_device/*/device/hpb_param_sysfs/requeue_timeout_ms +Date: March 2021 +Contact: Daejun Park +Description: This entry shows the requeue timeout threshold for write buffer + command in ms. This value can be changed by writing proper integer to + this entry. + +What: /sys/bus/platform/drivers/ufshcd/*/attributes/max_data_size_hpb_single_cmd +Date: March 2021 +Contact: Daejun Park +Description: This entry shows the maximum HPB data size for using single HPB + command. + + === + 00h 4KB + 01h 8KB + 02h 12KB + ... + FFh 1024KB + === + + The file is read only. + +What: /sys/bus/platform/drivers/ufshcd/*/flags/wb_enable +Date: March 2021 +Contact: Daejun Park +Description: This entry shows the status of HPB. + + == + 0 HPB is not enabled. + 1 HPB is enabled +
