diff mbox series

scsi: ufs: Optimize the WB flush process to save device power consumption

Message ID 20230628070229.61855-1-luhongfei@vivo.com
State New
Headers show
Series scsi: ufs: Optimize the WB flush process to save device power consumption | expand

Commit Message

Lu Hongfei June 28, 2023, 7:02 a.m. UTC 
In the original logic, WB Hibern Flush was always on.
During suspend flow, the host will determine whether the device needs
BKOP or WB flush, and if so, it will keep VCC supply.
WB flush is only a part of BKOP, and device that needs BKOP do not
necessarily need WB flush if the conditions are not met. Therefore,
if WB flush is not needed, it will be better to disable WB Hibern
Flush, which could save device power consumption. When WB Hibern
Flush is needed, enable it again.

In this way, the WB Hibern Flush always on strategy is changed to a dynamic
on/off strategy.

Signed-off-by: Lu Hongfei <luhongfei@vivo.com>
Signed-off-by: Tang Huan <tanghuan@vivo.com>
---
 drivers/ufs/core/ufshcd.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Comments

Avri Altman June 28, 2023, 11 a.m. UTC | #1 
> 
> In the original logic, WB Hibern Flush was always on.
> During suspend flow, the host will determine whether the device needs
> BKOP or WB flush, and if so, it will keep VCC supply.
> WB flush is only a part of BKOP, and device that needs BKOP do not
> necessarily need WB flush if the conditions are not met. Therefore,
> if WB flush is not needed, it will be better to disable WB Hibern
> Flush,..... 
I beg to differ on this conclusion.
If you expect SLC write performance, without fluctuations,
the host should let the device manage its wb buffer on its own.
This is exactly what flush on hibernate does and better leave it be.

Thanks,
Avri
Jinyoung CHOI June 29, 2023, 3:11 a.m. UTC | #2 
>> 
>> In the original logic, WB Hibern Flush was always on.
>> During suspend flow, the host will determine whether the device needs
>> BKOP or WB flush, and if so, it will keep VCC supply.
>> WB flush is only a part of BKOP, and device that needs BKOP do not
>> necessarily need WB flush if the conditions are not met. Therefore,
>> if WB flush is not needed, it will be better to disable WB Hibern
>> Flush,..... 
>I beg to differ on this conclusion.
>If you expect SLC write performance, without fluctuations,
>the host should let the device manage its wb buffer on its own.
>This is exactly what flush on hibernate does and better leave it be.
>
>Thanks,
>Avri

Hi, Lu Hongfei.

I agree with avri.
In fact, I am not sure if there will be a significant improvement in
power consumption.
In addition, for the toggle of the function, a code that compares with
the existing value must be added to prevent unnecessary queries.

Best Regards,
Jinyoung.
diff mbox series

Patch

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 983fae84d9e8..484d7fa96407 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -9592,13 +9592,20 @@  static int __ufshcd_wl_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 		 * If device needs to do BKOP or WB buffer flush during
 		 * Hibern8, keep device power mode as "active power mode"
 		 * and VCC supply.
+		 * If device does not need WB buffer flush now, it's better
+		 * to disable WB flush during H8 to save power consumption.
 		 */
-		hba->dev_info.b_rpm_dev_flush_capable =
-			hba->auto_bkops_enabled ||
-			(((req_link_state == UIC_LINK_HIBERN8_STATE) ||
-			((req_link_state == UIC_LINK_ACTIVE_STATE) &&
-			ufshcd_is_auto_hibern8_enabled(hba))) &&
-			ufshcd_wb_need_flush(hba));
+		hba->dev_info.b_rpm_dev_flush_capable = hba->auto_bkops_enabled;
+		if (((req_link_state == UIC_LINK_HIBERN8_STATE) ||
+		    ((req_link_state == UIC_LINK_ACTIVE_STATE) &&
+		    ufshcd_is_auto_hibern8_enabled(hba))) &&
+		    ufshcd_wb_need_flush(hba)) {
+			ufshcd_wb_toggle_buf_flush_during_h8(hba, true);
+			hba->dev_info.b_rpm_dev_flush_capable = true;
+		} else {
+			ufshcd_wb_toggle_buf_flush_during_h8(hba, false);
+		}
+
 	}
 
 	flush_work(&hba->eeh_work);