* [PATCH 0/4] Support PUIS and DEVSLP feature
@ 2018-06-01 5:39 Ruiyu Ni
2018-06-01 5:39 ` [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices Ruiyu Ni
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Ruiyu Ni @ 2018-06-01 5:39 UTC (permalink / raw)
To: edk2-devel
Ruiyu Ni (4):
MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices
MdeModulePkg: Add AtaAtapiPolicy protocol definition
MdeModulePkg/AtaAtapiPassThru: enable/disable PUIS per policy
MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP per policy
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 391 ++++++++++++++++++++-
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h | 23 +-
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 19 +-
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 6 +-
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf | 3 +-
MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h | 59 ++++
MdeModulePkg/MdeModulePkg.dec | 3 +
7 files changed, 495 insertions(+), 9 deletions(-)
create mode 100644 MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h
--
2.16.1.windows.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices
2018-06-01 5:39 [PATCH 0/4] Support PUIS and DEVSLP feature Ruiyu Ni
@ 2018-06-01 5:39 ` Ruiyu Ni
2018-06-04 3:51 ` Chiu, Chasel
2018-06-04 5:53 ` Wu, Hao A
2018-06-01 5:39 ` [PATCH 2/4] MdeModulePkg: Add AtaAtapiPolicy protocol definition Ruiyu Ni
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Ruiyu Ni @ 2018-06-01 5:39 UTC (permalink / raw)
To: edk2-devel; +Cc: Chasel Chiu, Hao A Wu
The patch adds support to certain devices that support PUIS (Power
up in Standby).
For those devices that supports SET_FEATURE spin up, SW needs to
send SET_FEATURE subcommand to spin up the devices.
For those devices that doesn't support SET_FEATURE spin up, SW needs
to send read sectors command to spin up the devices.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 129 ++++++++++++++++++++-
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 3 +-
2 files changed, 127 insertions(+), 5 deletions(-)
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index e6de5d65bc..6ab7870570 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -1,7 +1,7 @@
/** @file
The file for AHCI mode of ATA host controller.
- Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -1841,7 +1841,8 @@ AhciDeviceSetFeature (
IN UINT8 Port,
IN UINT8 PortMultiplier,
IN UINT16 Feature,
- IN UINT32 FeatureSpecificData
+ IN UINT32 FeatureSpecificData,
+ IN UINT64 Timeout
)
{
EFI_STATUS Status;
@@ -1868,7 +1869,7 @@ AhciDeviceSetFeature (
0,
&AtaCommandBlock,
&AtaStatusBlock,
- ATA_ATAPI_TIMEOUT,
+ Timeout,
NULL
);
@@ -2216,6 +2217,104 @@ Error6:
return Status;
}
+
+/**
+ Spin-up disk if IDD was incomplete or PUIS feature is enabled
+
+ @param PciIo The PCI IO protocol instance.
+ @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
+ @param Port The number of port.
+ @param PortMultiplier The multiplier of port.
+ @param IdentifyData A pointer to data buffer which is used to contain IDENTIFY data.
+
+**/
+EFI_STATUS
+AhciSpinUpDisk (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN EFI_AHCI_REGISTERS *AhciRegisters,
+ IN UINT8 Port,
+ IN UINT8 PortMultiplier,
+ IN OUT EFI_IDENTIFY_DATA *IdentifyData
+ )
+{
+ EFI_STATUS Status;
+ EFI_ATA_COMMAND_BLOCK AtaCommandBlock;
+ EFI_ATA_STATUS_BLOCK AtaStatusBlock;
+ UINT8 Buffer[512];
+
+ if (IdentifyData->AtaData.specific_config == ATA_SPINUP_CFG_REQUIRED_IDD_INCOMPLETE) {
+ //
+ // Use SET_FEATURE subcommand to spin up the device.
+ //
+ Status = AhciDeviceSetFeature (
+ PciIo, AhciRegisters, Port, PortMultiplier,
+ ATA_SUB_CMD_PUIS_SET_DEVICE_SPINUP, 0x00, ATA_SPINUP_TIMEOUT
+ );
+ DEBUG ((DEBUG_INFO, "CMD_PUIS_SET_DEVICE_SPINUP for device at port [%d] PortMultiplier [%d] - %r!\n",
+ Port, PortMultiplier, Status));
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ } else {
+ ASSERT (IdentifyData->AtaData.specific_config == ATA_SPINUP_CFG_NOT_REQUIRED_IDD_INCOMPLETE);
+
+ //
+ // Use READ_SECTORS to spin up the device if SpinUp SET FEATURE subcommand is not supported
+ //
+ ZeroMem (&AtaCommandBlock, sizeof (EFI_ATA_COMMAND_BLOCK));
+ ZeroMem (&AtaStatusBlock, sizeof (EFI_ATA_STATUS_BLOCK));
+ //
+ // Perform READ SECTORS PIO Data-In command to Read LBA 0
+ //
+ AtaCommandBlock.AtaCommand = ATA_CMD_READ_SECTORS;
+ AtaCommandBlock.AtaSectorCount = 0x1;
+
+ Status = AhciPioTransfer (
+ PciIo,
+ AhciRegisters,
+ Port,
+ PortMultiplier,
+ NULL,
+ 0,
+ TRUE,
+ &AtaCommandBlock,
+ &AtaStatusBlock,
+ &Buffer,
+ sizeof (Buffer),
+ ATA_SPINUP_TIMEOUT,
+ NULL
+ );
+ DEBUG ((DEBUG_INFO, "Read LBA 0 for device at port [%d] PortMultiplier [%d] - %r!\n",
+ Port, PortMultiplier, Status));
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ }
+
+ //
+ // Read the complete IDENTIFY DEVICE data.
+ //
+ ZeroMem (IdentifyData, sizeof (*IdentifyData));
+ Status = AhciIdentify (PciIo, AhciRegisters, Port, PortMultiplier, IdentifyData);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Read IDD failed for device at port [%d] PortMultiplier [%d] - %r!\n",
+ Port, PortMultiplier, Status));
+ return Status;
+ }
+
+ DEBUG ((DEBUG_INFO, "IDENTIFY DEVICE: [0] = %016x, [2] = %016x, [83] = %016x, [86] = %016x\n",
+ IdentifyData->AtaData.config, IdentifyData->AtaData.specific_config,
+ IdentifyData->AtaData.command_set_supported_83, IdentifyData->AtaData.command_set_feature_enb_86));
+ //
+ // Check if IDD is incomplete
+ //
+ if ((IdentifyData->AtaData.config & BIT2) != 0) {
+ return EFI_DEVICE_ERROR;
+ }
+
+ return EFI_SUCCESS;
+}
+
/**
Initialize ATA host controller at AHCI mode.
@@ -2458,6 +2557,28 @@ AhciModeInitialization (
continue;
}
+ DEBUG ((
+ DEBUG_INFO, "IDENTIFY DEVICE: [0] = %016x, [2] = %016x, [83] = %016x, [86] = %016x\n",
+ Buffer.AtaData.config, Buffer.AtaData.specific_config,
+ Buffer.AtaData.command_set_supported_83, Buffer.AtaData.command_set_feature_enb_86
+ ));
+ if ((Buffer.AtaData.config & BIT2) != 0) {
+ //
+ // SpinUp disk if device reported incomplete IDENTIFY DEVICE.
+ //
+ Status = AhciSpinUpDisk (
+ PciIo,
+ AhciRegisters,
+ Port,
+ 0,
+ &Buffer
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Spin up standby device failed - %r\n", Status));
+ continue;
+ }
+ }
+
DeviceType = EfiIdeHarddisk;
} else {
continue;
@@ -2523,7 +2644,7 @@ AhciModeInitialization (
TransferMode.ModeNumber = (UINT8) SupportedModes->MultiWordDmaMode.Mode;
}
- Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port, 0, 0x03, (UINT32)(*(UINT8 *)&TransferMode));
+ Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port, 0, 0x03, (UINT32)(*(UINT8 *)&TransferMode), ATA_ATAPI_TIMEOUT); // RPPO-SKL-TBD
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "Set transfer Mode Fail, Status = %r\n", Status));
continue;
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
index 85e5a55539..31b005f2f6 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -1,7 +1,7 @@
/** @file
Header file for ATA/ATAPI PASS THRU driver.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -148,6 +148,7 @@ struct _ATA_NONBLOCK_TASK {
// It means 3 second span.
//
#define ATA_ATAPI_TIMEOUT EFI_TIMER_PERIOD_SECONDS(3)
+#define ATA_SPINUP_TIMEOUT EFI_TIMER_PERIOD_SECONDS(10)
#define IS_ALIGNED(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/4] MdeModulePkg: Add AtaAtapiPolicy protocol definition
2018-06-01 5:39 [PATCH 0/4] Support PUIS and DEVSLP feature Ruiyu Ni
2018-06-01 5:39 ` [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices Ruiyu Ni
@ 2018-06-01 5:39 ` Ruiyu Ni
2018-06-04 6:29 ` Zeng, Star
2018-06-01 5:39 ` [PATCH 3/4] MdeModulePkg/AtaAtapiPassThru: enable/disable PUIS per policy Ruiyu Ni
2018-06-01 5:39 ` [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP " Ruiyu Ni
3 siblings, 1 reply; 15+ messages in thread
From: Ruiyu Ni @ 2018-06-01 5:39 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng
The patch adds AtaAtapiPolicy protocol which is produced by platform
and consumed by AtaAtapiPassThruDxe driver.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h | 59 ++++++++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 ++
2 files changed, 62 insertions(+)
create mode 100644 MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h
diff --git a/MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h b/MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h
new file mode 100644
index 0000000000..86a6213dc9
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h
@@ -0,0 +1,59 @@
+/** @file
+ ATA ATAPI Policy protocol is produced by platform and consumed by AtaAtapiPassThruDxe
+ driver.
+
+ Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+#ifndef __ATA_ATAPI_POLICY_H__
+#define __ATA_ATAPI_POLICY_H__
+
+#define EDKII_ATA_ATAPI_POLICY_PROTOCOL_GUID \
+ { \
+ 0xe59cd769, 0x5083, 0x4f26,{ 0x90, 0x94, 0x6c, 0x91, 0x9f, 0x91, 0x6c, 0x4e } \
+ }
+
+typedef struct {
+ ///
+ /// Protocol version.
+ ///
+ UINT32 Version;
+
+ ///
+ /// 0: Disable Power-up in Standby;
+ /// 1: Enable Power-up in Standby;
+ /// others: Since PUIS setting is non-volatile, platform can use other value than 0/1 to keep hardware PUIS setting.
+ ///
+ UINT8 PuisEnable;
+
+ ///
+ /// 0: Disable Device Sleep;
+ /// 1: Enable Device Sleep;
+ ///
+ UINT8 DeviceSleepEnable;
+
+ ///
+ /// 0: Disable Aggressive Device Sleep;
+ /// 1: Enable Aggressive Device Sleep.
+ ///
+ UINT8 AggressiveDeviceSleepEnable;
+
+ UINT8 Reserved;
+} EDKII_ATA_ATAPI_POLICY_PROTOCOL;
+
+#define EDKII_ATA_ATAPI_POLICY_VERSION 0x00010000
+
+
+extern EFI_GUID gEdkiiAtaAtapiPolicyProtocolGuid;
+
+
+
+#endif
+
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index cc397185f7..3d8376ffdf 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -595,6 +595,9 @@ [Protocols]
gEdkiiPlatformSpecificResetFilterProtocolGuid = { 0x695d7835, 0x8d47, 0x4c11, { 0xab, 0x22, 0xfa, 0x8a, 0xcc, 0xe7, 0xae, 0x7a } }
## Include/Protocol/PlatformSpecificResetHandler.h
gEdkiiPlatformSpecificResetHandlerProtocolGuid = { 0x2df6ba0b, 0x7092, 0x440d, { 0xbd, 0x4, 0xfb, 0x9, 0x1e, 0xc3, 0xf3, 0xc1 } }
+
+ ## Include/Protocol/AtaAtapiPolicy.h
+ gEdkiiAtaAtapiPolicyProtocolGuid = { 0xe59cd769, 0x5083, 0x4f26,{ 0x90, 0x94, 0x6c, 0x91, 0x9f, 0x91, 0x6c, 0x4e } }
#
# [Error.gEfiMdeModulePkgTokenSpaceGuid]
# 0x80000001 | Invalid value provided.
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/4] MdeModulePkg/AtaAtapiPassThru: enable/disable PUIS per policy
2018-06-01 5:39 [PATCH 0/4] Support PUIS and DEVSLP feature Ruiyu Ni
2018-06-01 5:39 ` [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices Ruiyu Ni
2018-06-01 5:39 ` [PATCH 2/4] MdeModulePkg: Add AtaAtapiPolicy protocol definition Ruiyu Ni
@ 2018-06-01 5:39 ` Ruiyu Ni
2018-06-04 3:51 ` Chiu, Chasel
2018-06-01 5:39 ` [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP " Ruiyu Ni
3 siblings, 1 reply; 15+ messages in thread
From: Ruiyu Ni @ 2018-06-01 5:39 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Chasel Chiu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
---
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 48 ++++++++++++++++++++++
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 19 ++++++++-
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 3 ++
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf | 3 +-
4 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index 6ab7870570..46d5c68996 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -2315,6 +2315,38 @@ AhciSpinUpDisk (
return EFI_SUCCESS;
}
+/**
+ Enable/disable/skip PUIS of the disk according to policy.
+
+ @param PciIo The PCI IO protocol instance.
+ @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
+ @param Port The number of port.
+ @param PortMultiplier The multiplier of port.
+
+**/
+EFI_STATUS
+AhciPuisEnable (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN EFI_AHCI_REGISTERS *AhciRegisters,
+ IN UINT8 Port,
+ IN UINT8 PortMultiplier
+ )
+{
+ EFI_STATUS Status;
+
+ Status = EFI_SUCCESS;
+ if (mAtaAtapiPolicy->PuisEnable == 0) {
+ Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port, PortMultiplier, ATA_SUB_CMD_DISABLE_PUIS, 0x00, ATA_ATAPI_TIMEOUT);
+ } else if (mAtaAtapiPolicy->PuisEnable == 1) {
+ Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port, PortMultiplier, ATA_SUB_CMD_ENABLE_PUIS, 0x00, ATA_ATAPI_TIMEOUT);
+ }
+ DEBUG ((DEBUG_INFO, "%a PUIS feature at port [%d] PortMultiplier [%d] - %r!\n",
+ (mAtaAtapiPolicy->PuisEnable == 0) ? "Disable" : (
+ (mAtaAtapiPolicy->PuisEnable == 1) ? "Enable" : "Skip"
+ ), Port, PortMultiplier, Status));
+ return Status;
+}
+
/**
Initialize ATA host controller at AHCI mode.
@@ -2657,6 +2689,22 @@ AhciModeInitialization (
if (DeviceType == EfiIdeHarddisk) {
REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_PERIPHERAL_FIXED_MEDIA | EFI_P_PC_ENABLE));
}
+
+ //
+ // Enable/disable PUIS according to policy setting if PUIS is capable (Word[83].BIT5 is set).
+ //
+ if ((Buffer.AtaData.command_set_supported_83 & BIT5) != 0) {
+ Status = AhciPuisEnable (
+ PciIo,
+ AhciRegisters,
+ Port,
+ 0
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "PUIS enable/disable failed, Status = %r\n", Status));
+ continue;
+ }
+ }
}
}
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
index a48b295d26..aab704bcd3 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
@@ -2,7 +2,7 @@
This file implements ATA_PASSTHRU_PROCTOCOL and EXT_SCSI_PASSTHRU_PROTOCOL interfaces
for managed ATA controllers.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -142,6 +142,15 @@ UINT8 mScsiId[TARGET_MAX_BYTES] = {
0xFF, 0xFF, 0xFF, 0xFF
};
+EDKII_ATA_ATAPI_POLICY_PROTOCOL *mAtaAtapiPolicy;
+EDKII_ATA_ATAPI_POLICY_PROTOCOL mDefaultAtaAtapiPolicy = {
+ EDKII_ATA_ATAPI_POLICY_VERSION,
+ 2, // PuisEnable
+ 0, // DeviceSleepEnable
+ 0, // AggressiveDeviceSleepEnable
+ 0 // Reserved
+};
+
/**
Sends an ATA command to an ATA device that is attached to the ATA controller. This function
supports both blocking I/O and non-blocking I/O. The blocking I/O functionality is required,
@@ -739,6 +748,14 @@ AtaAtapiPassThruStart (
goto ErrorExit;
}
+ Status = gBS->LocateProtocol (&gEdkiiAtaAtapiPolicyProtocolGuid, NULL, (VOID **)&mAtaAtapiPolicy);
+ if (EFI_ERROR (Status)) {
+ //
+ // If there is no AtaAtapiPolicy exposed, use the default policy.
+ //
+ mAtaAtapiPolicy = &mDefaultAtaAtapiPolicy;
+ }
+
//
// Allocate a buffer to store the ATA_ATAPI_PASS_THRU_INSTANCE data structure
//
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
index 31b005f2f6..b07bcbbb3e 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -24,6 +24,7 @@
#include <Protocol/IdeControllerInit.h>
#include <Protocol/AtaPassThru.h>
#include <Protocol/ScsiPassThruExt.h>
+#include <Protocol/AtaAtapiPolicy.h>
#include <Library/DebugLib.h>
#include <Library/BaseLib.h>
@@ -45,6 +46,8 @@ extern EFI_DRIVER_BINDING_PROTOCOL gAtaAtapiPassThruDriverBinding;
extern EFI_COMPONENT_NAME_PROTOCOL gAtaAtapiPassThruComponentName;
extern EFI_COMPONENT_NAME2_PROTOCOL gAtaAtapiPassThruComponentName2;
+extern EDKII_ATA_ATAPI_POLICY_PROTOCOL *mAtaAtapiPolicy;
+
#define ATA_ATAPI_PASS_THRU_SIGNATURE SIGNATURE_32 ('a', 'a', 'p', 't')
#define ATA_ATAPI_DEVICE_SIGNATURE SIGNATURE_32 ('a', 'd', 'e', 'v')
#define ATA_NONBLOCKING_TASK_SIGNATURE SIGNATURE_32 ('a', 't', 's', 'k')
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
index 82d5f7a46c..d1ce859091 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
@@ -4,7 +4,7 @@
# This driver installs AtaPassThru and ExtScsiPassThru protocol in each ide/sata controller
# to access to all attached Ata/Atapi devices.
#
-# Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -67,6 +67,7 @@ [Protocols]
gEfiIdeControllerInitProtocolGuid ## TO_START
gEfiDevicePathProtocolGuid ## TO_START
gEfiPciIoProtocolGuid ## TO_START
+ gEdkiiAtaAtapiPolicyProtocolGuid ## CONSUMES
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdAtaSmartEnable ## SOMETIMES_CONSUMES
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP per policy
2018-06-01 5:39 [PATCH 0/4] Support PUIS and DEVSLP feature Ruiyu Ni
` (2 preceding siblings ...)
2018-06-01 5:39 ` [PATCH 3/4] MdeModulePkg/AtaAtapiPassThru: enable/disable PUIS per policy Ruiyu Ni
@ 2018-06-01 5:39 ` Ruiyu Ni
2018-06-04 3:51 ` Chiu, Chasel
2018-06-04 6:20 ` Wu, Hao A
3 siblings, 2 replies; 15+ messages in thread
From: Ruiyu Ni @ 2018-06-01 5:39 UTC (permalink / raw)
To: edk2-devel; +Cc: Chasel Chiu, Hao A Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 214 +++++++++++++++++++++++
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h | 23 ++-
2 files changed, 235 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index 46d5c68996..3741e3b782 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -2217,6 +2217,213 @@ Error6:
return Status;
}
+/**
+ Read logs from SATA device.
+
+ @param PciIo The PCI IO protocol instance.
+ @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
+ @param Port The number of port.
+ @param PortMultiplier The timeout value of stop.
+ @param Buffer The data buffer to store IDENTIFY PACKET data.
+
+ @retval EFI_DEVICE_ERROR The cmd abort with error occurs.
+ @retval EFI_TIMEOUT The operation is time out.
+ @retval EFI_UNSUPPORTED The device is not ready for executing.
+ @retval EFI_SUCCESS The cmd executes successfully.
+
+**/
+EFI_STATUS
+AhciReadLogExt (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN EFI_AHCI_REGISTERS *AhciRegisters,
+ IN UINT8 Port,
+ IN UINT8 PortMultiplier,
+ IN OUT UINT8 *Buffer,
+ IN UINT8 LogNumber,
+ IN UINT8 PageNumber
+ )
+{
+ EFI_ATA_COMMAND_BLOCK AtaCommandBlock;
+ EFI_ATA_STATUS_BLOCK AtaStatusBlock;
+
+ if (PciIo == NULL || AhciRegisters == NULL || Buffer == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ ///
+ /// Read log from device
+ ///
+ ZeroMem (&AtaCommandBlock, sizeof (EFI_ATA_COMMAND_BLOCK));
+ ZeroMem (&AtaStatusBlock, sizeof (EFI_ATA_STATUS_BLOCK));
+ ZeroMem (Buffer, 512);
+
+ AtaCommandBlock.AtaCommand = ATA_CMD_READ_LOG_EXT;
+ AtaCommandBlock.AtaSectorCount = 1;
+ AtaCommandBlock.AtaSectorNumber = LogNumber;
+ AtaCommandBlock.AtaCylinderLow = PageNumber;
+
+ return AhciPioTransfer (
+ PciIo,
+ AhciRegisters,
+ Port,
+ PortMultiplier,
+ NULL,
+ 0,
+ TRUE,
+ &AtaCommandBlock,
+ &AtaStatusBlock,
+ Buffer,
+ 512,
+ ATA_ATAPI_TIMEOUT,
+ NULL
+ );
+}
+
+/**
+ Enable DEVSLP command of the disk if supported.
+ Support for S3 resume to be added later
+
+ @param PciIo The PCI IO protocol instance.
+ @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
+ @param Port The number of port.
+ @param PortMultiplier The multiplier of port.
+ @param IdentifyData A pointer to data buffer which is used to contain IDENTIFY data.
+
+**/
+EFI_STATUS
+AhciEnableDevSlp (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN EFI_AHCI_REGISTERS *AhciRegisters,
+ IN UINT8 Port,
+ IN UINT8 PortMultiplier,
+ IN EFI_IDENTIFY_DATA *IdentifyData
+ )
+{
+ EFI_STATUS Status;
+ UINT32 Offset;
+ UINT32 Capability2;
+ UINT8 LogData[512];
+ DEVSLP_TIMING_VARIABLES DevSlpTiming;
+ UINT32 PortCmd;
+ UINT32 PortDevSlp;
+
+ if (mAtaAtapiPolicy->DeviceSleepEnable == 0) {
+ return EFI_SUCCESS;
+ }
+
+ //
+ // Do not enable DevSlp if DevSlp is not supported.
+ //
+ Capability2 = AhciReadReg (PciIo, AHCI_CAPABILITY2_OFFSET);
+ DEBUG ((DEBUG_INFO, "AHCI CAPABILITY2 = %08x\n", Capability2));
+ if ((Capability2 & AHCI_CAP2_SDS) == 0) {
+ return EFI_UNSUPPORTED;
+ }
+
+ //
+ // Do not enable DevSlp if DevSlp is not present
+ // Do not enable DevSlp if Hot Plug or Mechanical Presence Switch is supported
+ //
+ Offset = EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH;
+ PortCmd = AhciReadReg (PciIo, Offset + EFI_AHCI_PORT_CMD);
+ PortDevSlp = AhciReadReg (PciIo, Offset + AHCI_PORT_DEVSLP);
+ DEBUG ((DEBUG_ERROR, "Port CMD/DEVSLP = %08x / %08x\n", PortCmd, PortDevSlp));
+ if (((PortDevSlp & AHCI_PORT_DEVSLP_DSP) == 0) ||
+ ((PortCmd & (EFI_AHCI_PORT_CMD_HPCP | EFI_AHCI_PORT_CMD_MPSP)) != 0)
+ ) {
+ return EFI_UNSUPPORTED;
+ }
+
+ //
+ // Do not enable DevSlp if the device doesn't support DevSlp
+ //
+ DEBUG ((DEBUG_INFO, "IDENTIFY DEVICE: [77] = %04x, [78] = %04x, [79] = %04x\n",
+ IdentifyData->AtaData.reserved_77,
+ IdentifyData->AtaData.serial_ata_features_supported, IdentifyData->AtaData.serial_ata_features_enabled));
+ if ((IdentifyData->AtaData.serial_ata_features_supported & BIT8) == 0) {
+ DEBUG ((DEBUG_INFO, "DevSlp feature is not supported for device at port [%d] PortMultiplier [%d]!\n",
+ Port, PortMultiplier));
+ return EFI_UNSUPPORTED;
+ }
+
+ //
+ // Enable DevSlp when it is not enabled.
+ //
+ if ((IdentifyData->AtaData.serial_ata_features_enabled & BIT8) != 0) {
+ Status = AhciDeviceSetFeature (
+ PciIo, AhciRegisters, Port, 0, ATA_SUB_CMD_ENABLE_SATA_FEATURE, 0x09, ATA_ATAPI_TIMEOUT
+ );
+ DEBUG ((DEBUG_INFO, "DevSlp set feature for device at port [%d] PortMultiplier [%d] - %r\n",
+ Port, PortMultiplier, Status));
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ }
+
+ Status = AhciReadLogExt(PciIo, AhciRegisters, Port, PortMultiplier, LogData, 0x30, 0x08);
+
+ //
+ // Clear PxCMD.ST and PxDEVSLP.ADSE before updating PxDEVSLP.DITO and PxDEVSLP.MDAT.
+ //
+ AhciWriteReg (PciIo, Offset + EFI_AHCI_PORT_CMD, PortCmd & ~EFI_AHCI_PORT_CMD_ST);
+ PortDevSlp &= ~AHCI_PORT_DEVSLP_ADSE;
+ AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
+
+ //
+ // Set PxDEVSLP.DETO and PxDEVSLP.MDAT to 0.
+ //
+ PortDevSlp &= ~AHCI_PORT_DEVSLP_DETO_MASK;
+ PortDevSlp &= ~AHCI_PORT_DEVSLP_MDAT_MASK;
+ AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
+ DEBUG ((DEBUG_INFO, "Read Log Ext at port [%d] PortMultiplier [%d] - %r\n", Port, PortMultiplier, Status));
+ if (EFI_ERROR (Status)) {
+ //
+ // Assume DEVSLP TIMING VARIABLES is not supported if the Identify Device Data log (30h, 8) fails
+ //
+ DevSlpTiming.Supported = 0;
+ } else {
+ CopyMem (&DevSlpTiming, &LogData[48], sizeof (DevSlpTiming));
+ DEBUG ((DEBUG_INFO, "DevSlpTiming: Supported(%d), Deto(%d), Madt(%d)\n",
+ DevSlpTiming.Supported, DevSlpTiming.Deto, DevSlpTiming.Madt));
+ }
+
+ //
+ // Use 20ms as default DETO when DEVSLP TIMING VARIABLES is not supported or the DETO is 0.
+ //
+ if ((DevSlpTiming.Supported == 0) || (DevSlpTiming.Deto == 0)) {
+ DevSlpTiming.Deto = 20;
+ }
+
+ //
+ // Use 10ms as default MADT when DEVSLP TIMING VARIABLES is not supported or the MADT is 0.
+ //
+ if ((DevSlpTiming.Supported == 0) || (DevSlpTiming.Madt == 0)) {
+ DevSlpTiming.Madt = 10;
+ }
+
+ PortDevSlp |= DevSlpTiming.Deto << 2;
+ PortDevSlp |= DevSlpTiming.Madt << 10;
+ AhciOrReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
+
+ if (mAtaAtapiPolicy->AggressiveDeviceSleepEnable == 1) {
+ if ((Capability2 & AHCI_CAP2_SADM) != 0) {
+ PortDevSlp &= ~AHCI_PORT_DEVSLP_DITO_MASK;
+ PortDevSlp |= (625 << 15);
+ AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
+
+ PortDevSlp |= AHCI_PORT_DEVSLP_ADSE;
+ AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
+ }
+ }
+
+
+ AhciWriteReg (PciIo, Offset + EFI_AHCI_PORT_CMD, PortCmd);
+
+ DEBUG ((DEBUG_INFO, "Enabled DevSlp feature at port [%d] PortMultiplier [%d], Port CMD/DEVSLP = %08x / %08x\n",
+ Port, PortMultiplier, PortCmd, PortDevSlp));
+
+ return EFI_SUCCESS;
+}
/**
Spin-up disk if IDD was incomplete or PUIS feature is enabled
@@ -2688,6 +2895,13 @@ AhciModeInitialization (
CreateNewDeviceInfo (Instance, Port, 0xFFFF, DeviceType, &Buffer);
if (DeviceType == EfiIdeHarddisk) {
REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_PERIPHERAL_FIXED_MEDIA | EFI_P_PC_ENABLE));
+ AhciEnableDevSlp (
+ PciIo,
+ AhciRegisters,
+ Port,
+ 0,
+ &Buffer
+ );
}
//
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
index 809bcc307f..0ef749b4c7 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
@@ -1,7 +1,7 @@
/** @file
Header file for AHCI mode of ATA host controller.
- Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -29,6 +29,10 @@
#define EFI_AHCI_MAX_PORTS 32
+#define AHCI_CAPABILITY2_OFFSET 0x0024
+#define AHCI_CAP2_SDS BIT3
+#define AHCI_CAP2_SADM BIT4
+
typedef struct {
UINT32 Lower32;
UINT32 Upper32;
@@ -182,7 +186,13 @@ typedef union {
#define EFI_AHCI_PORT_SACT 0x0034
#define EFI_AHCI_PORT_CI 0x0038
#define EFI_AHCI_PORT_SNTF 0x003C
-
+#define AHCI_PORT_DEVSLP 0x0044
+#define AHCI_PORT_DEVSLP_ADSE BIT0
+#define AHCI_PORT_DEVSLP_DSP BIT1
+#define AHCI_PORT_DEVSLP_DETO_MASK 0x000003FC
+#define AHCI_PORT_DEVSLP_MDAT_MASK 0x00007C00
+#define AHCI_PORT_DEVSLP_DITO_MASK 0x01FF8000
+#define AHCI_PORT_DEVSLP_DM_MASK 0x1E000000
#pragma pack(1)
//
@@ -283,6 +293,15 @@ typedef struct {
UINT8 AhciUnknownFisRsvd[0x60];
} EFI_AHCI_RECEIVED_FIS;
+typedef struct {
+ UINT8 Madt : 5;
+ UINT8 Reserved_5 : 3;
+ UINT8 Deto;
+ UINT16 Reserved_16;
+ UINT32 Reserved_32 : 31;
+ UINT32 Supported : 1;
+} DEVSLP_TIMING_VARIABLES;
+
#pragma pack()
typedef struct {
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP per policy
2018-06-01 5:39 ` [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP " Ruiyu Ni
@ 2018-06-04 3:51 ` Chiu, Chasel
2018-06-04 6:20 ` Wu, Hao A
1 sibling, 0 replies; 15+ messages in thread
From: Chiu, Chasel @ 2018-06-04 3:51 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Wu, Hao A
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
-----Original Message-----
From: Ni, Ruiyu
Sent: Friday, June 1, 2018 1:39 PM
To: edk2-devel@lists.01.org
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
Subject: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP per policy
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 214 +++++++++++++++++++++++ MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h | 23 ++-
2 files changed, 235 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index 46d5c68996..3741e3b782 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -2217,6 +2217,213 @@ Error6:
return Status;
}
+/**
+ Read logs from SATA device.
+
+ @param PciIo The PCI IO protocol instance.
+ @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
+ @param Port The number of port.
+ @param PortMultiplier The timeout value of stop.
+ @param Buffer The data buffer to store IDENTIFY PACKET data.
+
+ @retval EFI_DEVICE_ERROR The cmd abort with error occurs.
+ @retval EFI_TIMEOUT The operation is time out.
+ @retval EFI_UNSUPPORTED The device is not ready for executing.
+ @retval EFI_SUCCESS The cmd executes successfully.
+
+**/
+EFI_STATUS
+AhciReadLogExt (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN EFI_AHCI_REGISTERS *AhciRegisters,
+ IN UINT8 Port,
+ IN UINT8 PortMultiplier,
+ IN OUT UINT8 *Buffer,
+ IN UINT8 LogNumber,
+ IN UINT8 PageNumber
+ )
+{
+ EFI_ATA_COMMAND_BLOCK AtaCommandBlock;
+ EFI_ATA_STATUS_BLOCK AtaStatusBlock;
+
+ if (PciIo == NULL || AhciRegisters == NULL || Buffer == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ ///
+ /// Read log from device
+ ///
+ ZeroMem (&AtaCommandBlock, sizeof (EFI_ATA_COMMAND_BLOCK)); ZeroMem
+ (&AtaStatusBlock, sizeof (EFI_ATA_STATUS_BLOCK)); ZeroMem (Buffer,
+ 512);
+
+ AtaCommandBlock.AtaCommand = ATA_CMD_READ_LOG_EXT;
+ AtaCommandBlock.AtaSectorCount = 1;
+ AtaCommandBlock.AtaSectorNumber = LogNumber;
+ AtaCommandBlock.AtaCylinderLow = PageNumber;
+
+ return AhciPioTransfer (
+ PciIo,
+ AhciRegisters,
+ Port,
+ PortMultiplier,
+ NULL,
+ 0,
+ TRUE,
+ &AtaCommandBlock,
+ &AtaStatusBlock,
+ Buffer,
+ 512,
+ ATA_ATAPI_TIMEOUT,
+ NULL
+ );
+}
+
+/**
+ Enable DEVSLP command of the disk if supported.
+ Support for S3 resume to be added later
+
+ @param PciIo The PCI IO protocol instance.
+ @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
+ @param Port The number of port.
+ @param PortMultiplier The multiplier of port.
+ @param IdentifyData A pointer to data buffer which is used to contain IDENTIFY data.
+
+**/
+EFI_STATUS
+AhciEnableDevSlp (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN EFI_AHCI_REGISTERS *AhciRegisters,
+ IN UINT8 Port,
+ IN UINT8 PortMultiplier,
+ IN EFI_IDENTIFY_DATA *IdentifyData
+ )
+{
+ EFI_STATUS Status;
+ UINT32 Offset;
+ UINT32 Capability2;
+ UINT8 LogData[512];
+ DEVSLP_TIMING_VARIABLES DevSlpTiming;
+ UINT32 PortCmd;
+ UINT32 PortDevSlp;
+
+ if (mAtaAtapiPolicy->DeviceSleepEnable == 0) {
+ return EFI_SUCCESS;
+ }
+
+ //
+ // Do not enable DevSlp if DevSlp is not supported.
+ //
+ Capability2 = AhciReadReg (PciIo, AHCI_CAPABILITY2_OFFSET); DEBUG
+ ((DEBUG_INFO, "AHCI CAPABILITY2 = %08x\n", Capability2)); if
+ ((Capability2 & AHCI_CAP2_SDS) == 0) {
+ return EFI_UNSUPPORTED;
+ }
+
+ //
+ // Do not enable DevSlp if DevSlp is not present // Do not enable
+ DevSlp if Hot Plug or Mechanical Presence Switch is supported //
+ Offset = EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH;
+ PortCmd = AhciReadReg (PciIo, Offset + EFI_AHCI_PORT_CMD);
+ PortDevSlp = AhciReadReg (PciIo, Offset + AHCI_PORT_DEVSLP); DEBUG
+ ((DEBUG_ERROR, "Port CMD/DEVSLP = %08x / %08x\n", PortCmd,
+ PortDevSlp)); if (((PortDevSlp & AHCI_PORT_DEVSLP_DSP) == 0) ||
+ ((PortCmd & (EFI_AHCI_PORT_CMD_HPCP | EFI_AHCI_PORT_CMD_MPSP)) != 0)
+ ) {
+ return EFI_UNSUPPORTED;
+ }
+
+ //
+ // Do not enable DevSlp if the device doesn't support DevSlp //
+ DEBUG ((DEBUG_INFO, "IDENTIFY DEVICE: [77] = %04x, [78] = %04x, [79] = %04x\n",
+ IdentifyData->AtaData.reserved_77,
+ IdentifyData->AtaData.serial_ata_features_supported,
+ IdentifyData->AtaData.serial_ata_features_enabled));
+ if ((IdentifyData->AtaData.serial_ata_features_supported & BIT8) == 0) {
+ DEBUG ((DEBUG_INFO, "DevSlp feature is not supported for device at port [%d] PortMultiplier [%d]!\n",
+ Port, PortMultiplier));
+ return EFI_UNSUPPORTED;
+ }
+
+ //
+ // Enable DevSlp when it is not enabled.
+ //
+ if ((IdentifyData->AtaData.serial_ata_features_enabled & BIT8) != 0) {
+ Status = AhciDeviceSetFeature (
+ PciIo, AhciRegisters, Port, 0, ATA_SUB_CMD_ENABLE_SATA_FEATURE, 0x09, ATA_ATAPI_TIMEOUT
+ );
+ DEBUG ((DEBUG_INFO, "DevSlp set feature for device at port [%d] PortMultiplier [%d] - %r\n",
+ Port, PortMultiplier, Status));
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ }
+
+ Status = AhciReadLogExt(PciIo, AhciRegisters, Port, PortMultiplier,
+ LogData, 0x30, 0x08);
+
+ //
+ // Clear PxCMD.ST and PxDEVSLP.ADSE before updating PxDEVSLP.DITO and PxDEVSLP.MDAT.
+ //
+ AhciWriteReg (PciIo, Offset + EFI_AHCI_PORT_CMD, PortCmd &
+ ~EFI_AHCI_PORT_CMD_ST); PortDevSlp &= ~AHCI_PORT_DEVSLP_ADSE;
+ AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
+
+ //
+ // Set PxDEVSLP.DETO and PxDEVSLP.MDAT to 0.
+ //
+ PortDevSlp &= ~AHCI_PORT_DEVSLP_DETO_MASK; PortDevSlp &=
+ ~AHCI_PORT_DEVSLP_MDAT_MASK; AhciWriteReg (PciIo, Offset +
+ AHCI_PORT_DEVSLP, PortDevSlp); DEBUG ((DEBUG_INFO, "Read Log Ext at
+ port [%d] PortMultiplier [%d] - %r\n", Port, PortMultiplier, Status));
+ if (EFI_ERROR (Status)) {
+ //
+ // Assume DEVSLP TIMING VARIABLES is not supported if the Identify Device Data log (30h, 8) fails
+ //
+ DevSlpTiming.Supported = 0;
+ } else {
+ CopyMem (&DevSlpTiming, &LogData[48], sizeof (DevSlpTiming));
+ DEBUG ((DEBUG_INFO, "DevSlpTiming: Supported(%d), Deto(%d), Madt(%d)\n",
+ DevSlpTiming.Supported, DevSlpTiming.Deto,
+ DevSlpTiming.Madt)); }
+
+ //
+ // Use 20ms as default DETO when DEVSLP TIMING VARIABLES is not supported or the DETO is 0.
+ //
+ if ((DevSlpTiming.Supported == 0) || (DevSlpTiming.Deto == 0)) {
+ DevSlpTiming.Deto = 20;
+ }
+
+ //
+ // Use 10ms as default MADT when DEVSLP TIMING VARIABLES is not supported or the MADT is 0.
+ //
+ if ((DevSlpTiming.Supported == 0) || (DevSlpTiming.Madt == 0)) {
+ DevSlpTiming.Madt = 10;
+ }
+
+ PortDevSlp |= DevSlpTiming.Deto << 2; PortDevSlp |=
+ DevSlpTiming.Madt << 10; AhciOrReg (PciIo, Offset + AHCI_PORT_DEVSLP,
+ PortDevSlp);
+
+ if (mAtaAtapiPolicy->AggressiveDeviceSleepEnable == 1) {
+ if ((Capability2 & AHCI_CAP2_SADM) != 0) {
+ PortDevSlp &= ~AHCI_PORT_DEVSLP_DITO_MASK;
+ PortDevSlp |= (625 << 15);
+ AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
+
+ PortDevSlp |= AHCI_PORT_DEVSLP_ADSE;
+ AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
+ }
+ }
+
+
+ AhciWriteReg (PciIo, Offset + EFI_AHCI_PORT_CMD, PortCmd);
+
+ DEBUG ((DEBUG_INFO, "Enabled DevSlp feature at port [%d] PortMultiplier [%d], Port CMD/DEVSLP = %08x / %08x\n",
+ Port, PortMultiplier, PortCmd, PortDevSlp));
+
+ return EFI_SUCCESS;
+}
/**
Spin-up disk if IDD was incomplete or PUIS feature is enabled @@ -2688,6 +2895,13 @@ AhciModeInitialization (
CreateNewDeviceInfo (Instance, Port, 0xFFFF, DeviceType, &Buffer);
if (DeviceType == EfiIdeHarddisk) {
REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_PERIPHERAL_FIXED_MEDIA | EFI_P_PC_ENABLE));
+ AhciEnableDevSlp (
+ PciIo,
+ AhciRegisters,
+ Port,
+ 0,
+ &Buffer
+ );
}
//
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
index 809bcc307f..0ef749b4c7 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
@@ -1,7 +1,7 @@
/** @file
Header file for AHCI mode of ATA host controller.
- Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights
+ reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -29,6 +29,10 @@
#define EFI_AHCI_MAX_PORTS 32
+#define AHCI_CAPABILITY2_OFFSET 0x0024
+#define AHCI_CAP2_SDS BIT3
+#define AHCI_CAP2_SADM BIT4
+
typedef struct {
UINT32 Lower32;
UINT32 Upper32;
@@ -182,7 +186,13 @@ typedef union {
#define EFI_AHCI_PORT_SACT 0x0034
#define EFI_AHCI_PORT_CI 0x0038
#define EFI_AHCI_PORT_SNTF 0x003C
-
+#define AHCI_PORT_DEVSLP 0x0044
+#define AHCI_PORT_DEVSLP_ADSE BIT0
+#define AHCI_PORT_DEVSLP_DSP BIT1
+#define AHCI_PORT_DEVSLP_DETO_MASK 0x000003FC
+#define AHCI_PORT_DEVSLP_MDAT_MASK 0x00007C00
+#define AHCI_PORT_DEVSLP_DITO_MASK 0x01FF8000
+#define AHCI_PORT_DEVSLP_DM_MASK 0x1E000000
#pragma pack(1)
//
@@ -283,6 +293,15 @@ typedef struct {
UINT8 AhciUnknownFisRsvd[0x60];
} EFI_AHCI_RECEIVED_FIS;
+typedef struct {
+ UINT8 Madt : 5;
+ UINT8 Reserved_5 : 3;
+ UINT8 Deto;
+ UINT16 Reserved_16;
+ UINT32 Reserved_32 : 31;
+ UINT32 Supported : 1;
+} DEVSLP_TIMING_VARIABLES;
+
#pragma pack()
typedef struct {
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices
2018-06-01 5:39 ` [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices Ruiyu Ni
@ 2018-06-04 3:51 ` Chiu, Chasel
2018-06-04 5:53 ` Wu, Hao A
1 sibling, 0 replies; 15+ messages in thread
From: Chiu, Chasel @ 2018-06-04 3:51 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Wu, Hao A
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
-----Original Message-----
From: Ni, Ruiyu
Sent: Friday, June 1, 2018 1:39 PM
To: edk2-devel@lists.01.org
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
Subject: [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices
The patch adds support to certain devices that support PUIS (Power up in Standby).
For those devices that supports SET_FEATURE spin up, SW needs to send SET_FEATURE subcommand to spin up the devices.
For those devices that doesn't support SET_FEATURE spin up, SW needs to send read sectors command to spin up the devices.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 129 ++++++++++++++++++++-
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 3 +-
2 files changed, 127 insertions(+), 5 deletions(-)
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index e6de5d65bc..6ab7870570 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -1,7 +1,7 @@
/** @file
The file for AHCI mode of ATA host controller.
- Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights
+ reserved.<BR>
(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License @@ -1841,7 +1841,8 @@ AhciDeviceSetFeature (
IN UINT8 Port,
IN UINT8 PortMultiplier,
IN UINT16 Feature,
- IN UINT32 FeatureSpecificData
+ IN UINT32 FeatureSpecificData,
+ IN UINT64 Timeout
)
{
EFI_STATUS Status;
@@ -1868,7 +1869,7 @@ AhciDeviceSetFeature (
0,
&AtaCommandBlock,
&AtaStatusBlock,
- ATA_ATAPI_TIMEOUT,
+ Timeout,
NULL
);
@@ -2216,6 +2217,104 @@ Error6:
return Status;
}
+
+/**
+ Spin-up disk if IDD was incomplete or PUIS feature is enabled
+
+ @param PciIo The PCI IO protocol instance.
+ @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
+ @param Port The number of port.
+ @param PortMultiplier The multiplier of port.
+ @param IdentifyData A pointer to data buffer which is used to contain IDENTIFY data.
+
+**/
+EFI_STATUS
+AhciSpinUpDisk (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN EFI_AHCI_REGISTERS *AhciRegisters,
+ IN UINT8 Port,
+ IN UINT8 PortMultiplier,
+ IN OUT EFI_IDENTIFY_DATA *IdentifyData
+ )
+{
+ EFI_STATUS Status;
+ EFI_ATA_COMMAND_BLOCK AtaCommandBlock;
+ EFI_ATA_STATUS_BLOCK AtaStatusBlock;
+ UINT8 Buffer[512];
+
+ if (IdentifyData->AtaData.specific_config == ATA_SPINUP_CFG_REQUIRED_IDD_INCOMPLETE) {
+ //
+ // Use SET_FEATURE subcommand to spin up the device.
+ //
+ Status = AhciDeviceSetFeature (
+ PciIo, AhciRegisters, Port, PortMultiplier,
+ ATA_SUB_CMD_PUIS_SET_DEVICE_SPINUP, 0x00, ATA_SPINUP_TIMEOUT
+ );
+ DEBUG ((DEBUG_INFO, "CMD_PUIS_SET_DEVICE_SPINUP for device at port [%d] PortMultiplier [%d] - %r!\n",
+ Port, PortMultiplier, Status));
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ } else {
+ ASSERT (IdentifyData->AtaData.specific_config ==
+ ATA_SPINUP_CFG_NOT_REQUIRED_IDD_INCOMPLETE);
+
+ //
+ // Use READ_SECTORS to spin up the device if SpinUp SET FEATURE subcommand is not supported
+ //
+ ZeroMem (&AtaCommandBlock, sizeof (EFI_ATA_COMMAND_BLOCK));
+ ZeroMem (&AtaStatusBlock, sizeof (EFI_ATA_STATUS_BLOCK));
+ //
+ // Perform READ SECTORS PIO Data-In command to Read LBA 0
+ //
+ AtaCommandBlock.AtaCommand = ATA_CMD_READ_SECTORS;
+ AtaCommandBlock.AtaSectorCount = 0x1;
+
+ Status = AhciPioTransfer (
+ PciIo,
+ AhciRegisters,
+ Port,
+ PortMultiplier,
+ NULL,
+ 0,
+ TRUE,
+ &AtaCommandBlock,
+ &AtaStatusBlock,
+ &Buffer,
+ sizeof (Buffer),
+ ATA_SPINUP_TIMEOUT,
+ NULL
+ );
+ DEBUG ((DEBUG_INFO, "Read LBA 0 for device at port [%d] PortMultiplier [%d] - %r!\n",
+ Port, PortMultiplier, Status));
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ }
+
+ //
+ // Read the complete IDENTIFY DEVICE data.
+ //
+ ZeroMem (IdentifyData, sizeof (*IdentifyData)); Status =
+ AhciIdentify (PciIo, AhciRegisters, Port, PortMultiplier,
+ IdentifyData); if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Read IDD failed for device at port [%d] PortMultiplier [%d] - %r!\n",
+ Port, PortMultiplier, Status));
+ return Status;
+ }
+
+ DEBUG ((DEBUG_INFO, "IDENTIFY DEVICE: [0] = %016x, [2] = %016x, [83] = %016x, [86] = %016x\n",
+ IdentifyData->AtaData.config, IdentifyData->AtaData.specific_config,
+ IdentifyData->AtaData.command_set_supported_83,
+ IdentifyData->AtaData.command_set_feature_enb_86));
+ //
+ // Check if IDD is incomplete
+ //
+ if ((IdentifyData->AtaData.config & BIT2) != 0) {
+ return EFI_DEVICE_ERROR;
+ }
+
+ return EFI_SUCCESS;
+}
+
/**
Initialize ATA host controller at AHCI mode.
@@ -2458,6 +2557,28 @@ AhciModeInitialization (
continue;
}
+ DEBUG ((
+ DEBUG_INFO, "IDENTIFY DEVICE: [0] = %016x, [2] = %016x, [83] = %016x, [86] = %016x\n",
+ Buffer.AtaData.config, Buffer.AtaData.specific_config,
+ Buffer.AtaData.command_set_supported_83, Buffer.AtaData.command_set_feature_enb_86
+ ));
+ if ((Buffer.AtaData.config & BIT2) != 0) {
+ //
+ // SpinUp disk if device reported incomplete IDENTIFY DEVICE.
+ //
+ Status = AhciSpinUpDisk (
+ PciIo,
+ AhciRegisters,
+ Port,
+ 0,
+ &Buffer
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Spin up standby device failed - %r\n", Status));
+ continue;
+ }
+ }
+
DeviceType = EfiIdeHarddisk;
} else {
continue;
@@ -2523,7 +2644,7 @@ AhciModeInitialization (
TransferMode.ModeNumber = (UINT8) SupportedModes->MultiWordDmaMode.Mode;
}
- Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port, 0, 0x03, (UINT32)(*(UINT8 *)&TransferMode));
+ Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port, 0,
+ 0x03, (UINT32)(*(UINT8 *)&TransferMode), ATA_ATAPI_TIMEOUT); //
+ RPPO-SKL-TBD
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "Set transfer Mode Fail, Status = %r\n", Status));
continue;
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
index 85e5a55539..31b005f2f6 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -1,7 +1,7 @@
/** @file
Header file for ATA/ATAPI PASS THRU driver.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights
+ reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at @@ -148,6 +148,7 @@ struct _ATA_NONBLOCK_TASK { // It means 3 second span.
//
#define ATA_ATAPI_TIMEOUT EFI_TIMER_PERIOD_SECONDS(3)
+#define ATA_SPINUP_TIMEOUT EFI_TIMER_PERIOD_SECONDS(10)
#define IS_ALIGNED(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] MdeModulePkg/AtaAtapiPassThru: enable/disable PUIS per policy
2018-06-01 5:39 ` [PATCH 3/4] MdeModulePkg/AtaAtapiPassThru: enable/disable PUIS per policy Ruiyu Ni
@ 2018-06-04 3:51 ` Chiu, Chasel
0 siblings, 0 replies; 15+ messages in thread
From: Chiu, Chasel @ 2018-06-04 3:51 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Zeng, Star
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
-----Original Message-----
From: Ni, Ruiyu
Sent: Friday, June 1, 2018 1:39 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
Subject: [PATCH 3/4] MdeModulePkg/AtaAtapiPassThru: enable/disable PUIS per policy
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
---
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 48 ++++++++++++++++++++++
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 19 ++++++++-
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 3 ++
.../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf | 3 +-
4 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index 6ab7870570..46d5c68996 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -2315,6 +2315,38 @@ AhciSpinUpDisk (
return EFI_SUCCESS;
}
+/**
+ Enable/disable/skip PUIS of the disk according to policy.
+
+ @param PciIo The PCI IO protocol instance.
+ @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
+ @param Port The number of port.
+ @param PortMultiplier The multiplier of port.
+
+**/
+EFI_STATUS
+AhciPuisEnable (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN EFI_AHCI_REGISTERS *AhciRegisters,
+ IN UINT8 Port,
+ IN UINT8 PortMultiplier
+ )
+{
+ EFI_STATUS Status;
+
+ Status = EFI_SUCCESS;
+ if (mAtaAtapiPolicy->PuisEnable == 0) {
+ Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port,
+PortMultiplier, ATA_SUB_CMD_DISABLE_PUIS, 0x00, ATA_ATAPI_TIMEOUT);
+ } else if (mAtaAtapiPolicy->PuisEnable == 1) {
+ Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port,
+PortMultiplier, ATA_SUB_CMD_ENABLE_PUIS, 0x00, ATA_ATAPI_TIMEOUT);
+ }
+ DEBUG ((DEBUG_INFO, "%a PUIS feature at port [%d] PortMultiplier [%d] - %r!\n",
+ (mAtaAtapiPolicy->PuisEnable == 0) ? "Disable" : (
+ (mAtaAtapiPolicy->PuisEnable == 1) ? "Enable" : "Skip"
+ ), Port, PortMultiplier, Status));
+ return Status;
+}
+
/**
Initialize ATA host controller at AHCI mode.
@@ -2657,6 +2689,22 @@ AhciModeInitialization (
if (DeviceType == EfiIdeHarddisk) {
REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_PERIPHERAL_FIXED_MEDIA | EFI_P_PC_ENABLE));
}
+
+ //
+ // Enable/disable PUIS according to policy setting if PUIS is capable (Word[83].BIT5 is set).
+ //
+ if ((Buffer.AtaData.command_set_supported_83 & BIT5) != 0) {
+ Status = AhciPuisEnable (
+ PciIo,
+ AhciRegisters,
+ Port,
+ 0
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "PUIS enable/disable failed, Status = %r\n", Status));
+ continue;
+ }
+ }
}
}
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
index a48b295d26..aab704bcd3 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
@@ -2,7 +2,7 @@
This file implements ATA_PASSTHRU_PROCTOCOL and EXT_SCSI_PASSTHRU_PROTOCOL interfaces
for managed ATA controllers.
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2018, Intel Corporation. All rights
+ reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at @@ -142,6 +142,15 @@ UINT8 mScsiId[TARGET_MAX_BYTES] = {
0xFF, 0xFF, 0xFF, 0xFF
};
+EDKII_ATA_ATAPI_POLICY_PROTOCOL *mAtaAtapiPolicy;
+EDKII_ATA_ATAPI_POLICY_PROTOCOL mDefaultAtaAtapiPolicy = {
+ EDKII_ATA_ATAPI_POLICY_VERSION,
+ 2, // PuisEnable
+ 0, // DeviceSleepEnable
+ 0, // AggressiveDeviceSleepEnable
+ 0 // Reserved
+};
+
/**
Sends an ATA command to an ATA device that is attached to the ATA controller. This function
supports both blocking I/O and non-blocking I/O. The blocking I/O functionality is required, @@ -739,6 +748,14 @@ AtaAtapiPassThruStart (
goto ErrorExit;
}
+ Status = gBS->LocateProtocol (&gEdkiiAtaAtapiPolicyProtocolGuid,
+ NULL, (VOID **)&mAtaAtapiPolicy); if (EFI_ERROR (Status)) {
+ //
+ // If there is no AtaAtapiPolicy exposed, use the default policy.
+ //
+ mAtaAtapiPolicy = &mDefaultAtaAtapiPolicy; }
+
//
// Allocate a buffer to store the ATA_ATAPI_PASS_THRU_INSTANCE data structure
//
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
index 31b005f2f6..b07bcbbb3e 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -24,6 +24,7 @@
#include <Protocol/IdeControllerInit.h> #include <Protocol/AtaPassThru.h> #include <Protocol/ScsiPassThruExt.h>
+#include <Protocol/AtaAtapiPolicy.h>
#include <Library/DebugLib.h>
#include <Library/BaseLib.h>
@@ -45,6 +46,8 @@ extern EFI_DRIVER_BINDING_PROTOCOL gAtaAtapiPassThruDriverBinding; extern EFI_COMPONENT_NAME_PROTOCOL gAtaAtapiPassThruComponentName; extern EFI_COMPONENT_NAME2_PROTOCOL gAtaAtapiPassThruComponentName2;
+extern EDKII_ATA_ATAPI_POLICY_PROTOCOL *mAtaAtapiPolicy;
+
#define ATA_ATAPI_PASS_THRU_SIGNATURE SIGNATURE_32 ('a', 'a', 'p', 't')
#define ATA_ATAPI_DEVICE_SIGNATURE SIGNATURE_32 ('a', 'd', 'e', 'v')
#define ATA_NONBLOCKING_TASK_SIGNATURE SIGNATURE_32 ('a', 't', 's', 'k') diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
index 82d5f7a46c..d1ce859091 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
@@ -4,7 +4,7 @@
# This driver installs AtaPassThru and ExtScsiPassThru protocol in each ide/sata controller # to access to all attached Ata/Atapi devices.
#
-# Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2010 - 2018, Intel Corporation. All rights
+reserved.<BR>
#
# This program and the accompanying materials # are licensed and made available under the terms and conditions of the BSD License @@ -67,6 +67,7 @@ [Protocols]
gEfiIdeControllerInitProtocolGuid ## TO_START
gEfiDevicePathProtocolGuid ## TO_START
gEfiPciIoProtocolGuid ## TO_START
+ gEdkiiAtaAtapiPolicyProtocolGuid ## CONSUMES
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdAtaSmartEnable ## SOMETIMES_CONSUMES
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices
2018-06-01 5:39 ` [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices Ruiyu Ni
2018-06-04 3:51 ` Chiu, Chasel
@ 2018-06-04 5:53 ` Wu, Hao A
1 sibling, 0 replies; 15+ messages in thread
From: Wu, Hao A @ 2018-06-04 5:53 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
One minor comment below.
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Friday, June 01, 2018 1:39 PM
> To: edk2-devel@lists.01.org
> Cc: Chiu, Chasel; Wu, Hao A
> Subject: [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up
> in Standby devices
>
> The patch adds support to certain devices that support PUIS (Power
> up in Standby).
> For those devices that supports SET_FEATURE spin up, SW needs to
> send SET_FEATURE subcommand to spin up the devices.
> For those devices that doesn't support SET_FEATURE spin up, SW needs
> to send read sectors command to spin up the devices.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> ---
> MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 129
> ++++++++++++++++++++-
> .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 3 +-
> 2 files changed, 127 insertions(+), 5 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> index e6de5d65bc..6ab7870570 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> @@ -1,7 +1,7 @@
> /** @file
> The file for AHCI mode of ATA host controller.
>
> - Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
> (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -1841,7 +1841,8 @@ AhciDeviceSetFeature (
> IN UINT8 Port,
> IN UINT8 PortMultiplier,
> IN UINT16 Feature,
> - IN UINT32 FeatureSpecificData
> + IN UINT32 FeatureSpecificData,
> + IN UINT64 Timeout
> )
> {
> EFI_STATUS Status;
> @@ -1868,7 +1869,7 @@ AhciDeviceSetFeature (
> 0,
> &AtaCommandBlock,
> &AtaStatusBlock,
> - ATA_ATAPI_TIMEOUT,
> + Timeout,
> NULL
> );
>
> @@ -2216,6 +2217,104 @@ Error6:
> return Status;
> }
>
> +
> +/**
> + Spin-up disk if IDD was incomplete or PUIS feature is enabled
> +
> + @param PciIo The PCI IO protocol instance.
> + @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
> + @param Port The number of port.
> + @param PortMultiplier The multiplier of port.
> + @param IdentifyData A pointer to data buffer which is used to contain
> IDENTIFY data.
> +
> +**/
> +EFI_STATUS
> +AhciSpinUpDisk (
> + IN EFI_PCI_IO_PROTOCOL *PciIo,
> + IN EFI_AHCI_REGISTERS *AhciRegisters,
> + IN UINT8 Port,
> + IN UINT8 PortMultiplier,
> + IN OUT EFI_IDENTIFY_DATA *IdentifyData
> + )
> +{
> + EFI_STATUS Status;
> + EFI_ATA_COMMAND_BLOCK AtaCommandBlock;
> + EFI_ATA_STATUS_BLOCK AtaStatusBlock;
> + UINT8 Buffer[512];
> +
> + if (IdentifyData->AtaData.specific_config ==
> ATA_SPINUP_CFG_REQUIRED_IDD_INCOMPLETE) {
> + //
> + // Use SET_FEATURE subcommand to spin up the device.
> + //
> + Status = AhciDeviceSetFeature (
> + PciIo, AhciRegisters, Port, PortMultiplier,
> + ATA_SUB_CMD_PUIS_SET_DEVICE_SPINUP, 0x00,
> ATA_SPINUP_TIMEOUT
> + );
> + DEBUG ((DEBUG_INFO, "CMD_PUIS_SET_DEVICE_SPINUP for device at
> port [%d] PortMultiplier [%d] - %r!\n",
> + Port, PortMultiplier, Status));
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> + } else {
> + ASSERT (IdentifyData->AtaData.specific_config ==
> ATA_SPINUP_CFG_NOT_REQUIRED_IDD_INCOMPLETE);
> +
> + //
> + // Use READ_SECTORS to spin up the device if SpinUp SET FEATURE
> subcommand is not supported
> + //
> + ZeroMem (&AtaCommandBlock, sizeof (EFI_ATA_COMMAND_BLOCK));
> + ZeroMem (&AtaStatusBlock, sizeof (EFI_ATA_STATUS_BLOCK));
> + //
> + // Perform READ SECTORS PIO Data-In command to Read LBA 0
> + //
> + AtaCommandBlock.AtaCommand = ATA_CMD_READ_SECTORS;
> + AtaCommandBlock.AtaSectorCount = 0x1;
> +
> + Status = AhciPioTransfer (
> + PciIo,
> + AhciRegisters,
> + Port,
> + PortMultiplier,
> + NULL,
> + 0,
> + TRUE,
> + &AtaCommandBlock,
> + &AtaStatusBlock,
> + &Buffer,
> + sizeof (Buffer),
> + ATA_SPINUP_TIMEOUT,
> + NULL
> + );
> + DEBUG ((DEBUG_INFO, "Read LBA 0 for device at port [%d] PortMultiplier
> [%d] - %r!\n",
> + Port, PortMultiplier, Status));
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> + }
> +
> + //
> + // Read the complete IDENTIFY DEVICE data.
> + //
> + ZeroMem (IdentifyData, sizeof (*IdentifyData));
> + Status = AhciIdentify (PciIo, AhciRegisters, Port, PortMultiplier,
> IdentifyData);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Read IDD failed for device at port [%d]
> PortMultiplier [%d] - %r!\n",
> + Port, PortMultiplier, Status));
> + return Status;
> + }
> +
> + DEBUG ((DEBUG_INFO, "IDENTIFY DEVICE: [0] = %016x, [2] = %016x, [83]
> = %016x, [86] = %016x\n",
> + IdentifyData->AtaData.config, IdentifyData->AtaData.specific_config,
> + IdentifyData->AtaData.command_set_supported_83, IdentifyData-
> >AtaData.command_set_feature_enb_86));
> + //
> + // Check if IDD is incomplete
> + //
> + if ((IdentifyData->AtaData.config & BIT2) != 0) {
> + return EFI_DEVICE_ERROR;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> /**
> Initialize ATA host controller at AHCI mode.
>
> @@ -2458,6 +2557,28 @@ AhciModeInitialization (
> continue;
> }
>
> + DEBUG ((
> + DEBUG_INFO, "IDENTIFY DEVICE: [0] = %016x, [2] = %016x, [83]
> = %016x, [86] = %016x\n",
> + Buffer.AtaData.config, Buffer.AtaData.specific_config,
> + Buffer.AtaData.command_set_supported_83,
> Buffer.AtaData.command_set_feature_enb_86
> + ));
> + if ((Buffer.AtaData.config & BIT2) != 0) {
> + //
> + // SpinUp disk if device reported incomplete IDENTIFY DEVICE.
> + //
> + Status = AhciSpinUpDisk (
> + PciIo,
> + AhciRegisters,
> + Port,
> + 0,
> + &Buffer
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Spin up standby device failed - %r\n",
> Status));
> + continue;
> + }
> + }
> +
> DeviceType = EfiIdeHarddisk;
> } else {
> continue;
> @@ -2523,7 +2644,7 @@ AhciModeInitialization (
> TransferMode.ModeNumber = (UINT8) SupportedModes-
> >MultiWordDmaMode.Mode;
> }
>
> - Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port, 0, 0x03,
> (UINT32)(*(UINT8 *)&TransferMode));
> + Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port, 0, 0x03,
> (UINT32)(*(UINT8 *)&TransferMode), ATA_ATAPI_TIMEOUT); // RPPO-SKL-
> TBD
Please help to remove the comment here.
With this change,
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
> if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_ERROR, "Set transfer Mode Fail, Status = %r\n", Status));
> continue;
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> index 85e5a55539..31b005f2f6 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> @@ -1,7 +1,7 @@
> /** @file
> Header file for ATA/ATAPI PASS THRU driver.
>
> - Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at
> @@ -148,6 +148,7 @@ struct _ATA_NONBLOCK_TASK {
> // It means 3 second span.
> //
> #define ATA_ATAPI_TIMEOUT EFI_TIMER_PERIOD_SECONDS(3)
> +#define ATA_SPINUP_TIMEOUT EFI_TIMER_PERIOD_SECONDS(10)
>
> #define IS_ALIGNED(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
>
> --
> 2.16.1.windows.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP per policy
2018-06-01 5:39 ` [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP " Ruiyu Ni
2018-06-04 3:51 ` Chiu, Chasel
@ 2018-06-04 6:20 ` Wu, Hao A
2018-06-04 6:36 ` Ni, Ruiyu
1 sibling, 1 reply; 15+ messages in thread
From: Wu, Hao A @ 2018-06-04 6:20 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
Hi Ray,
Some comments:
1. Please help to add the 'EFI_' prefix for the new definitions added in .h
files.
2. Do we need to handle the case when AhciEnableDevSlp() returns with an error?
3. Please help to adjust the debug level for the below line within function
AhciEnableDevSlp():
DEBUG ((DEBUG_ERROR, "Port CMD/DEVSLP = %08x / %08x\n", PortCmd, PortDevSlp));
4. The function description for AhciReadLogExt() does not match it prototype.
5. Please add the return value descriptions for function AhciEnableDevSlp().
Also, could you help to provide what kind of tests have been performed for this
patch and for the whole series? Thanks.
Best Regards,
Hao Wu
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Friday, June 01, 2018 1:39 PM
> To: edk2-devel@lists.01.org
> Cc: Chiu, Chasel; Wu, Hao A
> Subject: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable
> DEVSLP per policy
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> ---
> MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 214
> +++++++++++++++++++++++
> MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h | 23 ++-
> 2 files changed, 235 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> index 46d5c68996..3741e3b782 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> @@ -2217,6 +2217,213 @@ Error6:
> return Status;
> }
>
> +/**
> + Read logs from SATA device.
> +
> + @param PciIo The PCI IO protocol instance.
> + @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
> + @param Port The number of port.
> + @param PortMultiplier The timeout value of stop.
> + @param Buffer The data buffer to store IDENTIFY PACKET data.
> +
> + @retval EFI_DEVICE_ERROR The cmd abort with error occurs.
> + @retval EFI_TIMEOUT The operation is time out.
> + @retval EFI_UNSUPPORTED The device is not ready for executing.
> + @retval EFI_SUCCESS The cmd executes successfully.
> +
> +**/
> +EFI_STATUS
> +AhciReadLogExt (
> + IN EFI_PCI_IO_PROTOCOL *PciIo,
> + IN EFI_AHCI_REGISTERS *AhciRegisters,
> + IN UINT8 Port,
> + IN UINT8 PortMultiplier,
> + IN OUT UINT8 *Buffer,
> + IN UINT8 LogNumber,
> + IN UINT8 PageNumber
> + )
> +{
> + EFI_ATA_COMMAND_BLOCK AtaCommandBlock;
> + EFI_ATA_STATUS_BLOCK AtaStatusBlock;
> +
> + if (PciIo == NULL || AhciRegisters == NULL || Buffer == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + ///
> + /// Read log from device
> + ///
> + ZeroMem (&AtaCommandBlock, sizeof (EFI_ATA_COMMAND_BLOCK));
> + ZeroMem (&AtaStatusBlock, sizeof (EFI_ATA_STATUS_BLOCK));
> + ZeroMem (Buffer, 512);
> +
> + AtaCommandBlock.AtaCommand = ATA_CMD_READ_LOG_EXT;
> + AtaCommandBlock.AtaSectorCount = 1;
> + AtaCommandBlock.AtaSectorNumber = LogNumber;
> + AtaCommandBlock.AtaCylinderLow = PageNumber;
> +
> + return AhciPioTransfer (
> + PciIo,
> + AhciRegisters,
> + Port,
> + PortMultiplier,
> + NULL,
> + 0,
> + TRUE,
> + &AtaCommandBlock,
> + &AtaStatusBlock,
> + Buffer,
> + 512,
> + ATA_ATAPI_TIMEOUT,
> + NULL
> + );
> +}
> +
> +/**
> + Enable DEVSLP command of the disk if supported.
> + Support for S3 resume to be added later
> +
> + @param PciIo The PCI IO protocol instance.
> + @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
> + @param Port The number of port.
> + @param PortMultiplier The multiplier of port.
> + @param IdentifyData A pointer to data buffer which is used to contain
> IDENTIFY data.
> +
> +**/
> +EFI_STATUS
> +AhciEnableDevSlp (
> + IN EFI_PCI_IO_PROTOCOL *PciIo,
> + IN EFI_AHCI_REGISTERS *AhciRegisters,
> + IN UINT8 Port,
> + IN UINT8 PortMultiplier,
> + IN EFI_IDENTIFY_DATA *IdentifyData
> + )
> +{
> + EFI_STATUS Status;
> + UINT32 Offset;
> + UINT32 Capability2;
> + UINT8 LogData[512];
> + DEVSLP_TIMING_VARIABLES DevSlpTiming;
> + UINT32 PortCmd;
> + UINT32 PortDevSlp;
> +
> + if (mAtaAtapiPolicy->DeviceSleepEnable == 0) {
> + return EFI_SUCCESS;
> + }
> +
> + //
> + // Do not enable DevSlp if DevSlp is not supported.
> + //
> + Capability2 = AhciReadReg (PciIo, AHCI_CAPABILITY2_OFFSET);
> + DEBUG ((DEBUG_INFO, "AHCI CAPABILITY2 = %08x\n", Capability2));
> + if ((Capability2 & AHCI_CAP2_SDS) == 0) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + //
> + // Do not enable DevSlp if DevSlp is not present
> + // Do not enable DevSlp if Hot Plug or Mechanical Presence Switch is
> supported
> + //
> + Offset = EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH;
> + PortCmd = AhciReadReg (PciIo, Offset + EFI_AHCI_PORT_CMD);
> + PortDevSlp = AhciReadReg (PciIo, Offset + AHCI_PORT_DEVSLP);
> + DEBUG ((DEBUG_ERROR, "Port CMD/DEVSLP = %08x / %08x\n", PortCmd,
> PortDevSlp));
> + if (((PortDevSlp & AHCI_PORT_DEVSLP_DSP) == 0) ||
> + ((PortCmd & (EFI_AHCI_PORT_CMD_HPCP |
> EFI_AHCI_PORT_CMD_MPSP)) != 0)
> + ) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + //
> + // Do not enable DevSlp if the device doesn't support DevSlp
> + //
> + DEBUG ((DEBUG_INFO, "IDENTIFY DEVICE: [77] = %04x, [78] = %04x, [79]
> = %04x\n",
> + IdentifyData->AtaData.reserved_77,
> + IdentifyData->AtaData.serial_ata_features_supported, IdentifyData-
> >AtaData.serial_ata_features_enabled));
> + if ((IdentifyData->AtaData.serial_ata_features_supported & BIT8) == 0) {
> + DEBUG ((DEBUG_INFO, "DevSlp feature is not supported for device at
> port [%d] PortMultiplier [%d]!\n",
> + Port, PortMultiplier));
> + return EFI_UNSUPPORTED;
> + }
> +
> + //
> + // Enable DevSlp when it is not enabled.
> + //
> + if ((IdentifyData->AtaData.serial_ata_features_enabled & BIT8) != 0) {
> + Status = AhciDeviceSetFeature (
> + PciIo, AhciRegisters, Port, 0, ATA_SUB_CMD_ENABLE_SATA_FEATURE,
> 0x09, ATA_ATAPI_TIMEOUT
> + );
> + DEBUG ((DEBUG_INFO, "DevSlp set feature for device at port [%d]
> PortMultiplier [%d] - %r\n",
> + Port, PortMultiplier, Status));
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> + }
> +
> + Status = AhciReadLogExt(PciIo, AhciRegisters, Port, PortMultiplier, LogData,
> 0x30, 0x08);
> +
> + //
> + // Clear PxCMD.ST and PxDEVSLP.ADSE before updating PxDEVSLP.DITO
> and PxDEVSLP.MDAT.
> + //
> + AhciWriteReg (PciIo, Offset + EFI_AHCI_PORT_CMD, PortCmd &
> ~EFI_AHCI_PORT_CMD_ST);
> + PortDevSlp &= ~AHCI_PORT_DEVSLP_ADSE;
> + AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
> +
> + //
> + // Set PxDEVSLP.DETO and PxDEVSLP.MDAT to 0.
> + //
> + PortDevSlp &= ~AHCI_PORT_DEVSLP_DETO_MASK;
> + PortDevSlp &= ~AHCI_PORT_DEVSLP_MDAT_MASK;
> + AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
> + DEBUG ((DEBUG_INFO, "Read Log Ext at port [%d] PortMultiplier [%d] -
> %r\n", Port, PortMultiplier, Status));
> + if (EFI_ERROR (Status)) {
> + //
> + // Assume DEVSLP TIMING VARIABLES is not supported if the Identify
> Device Data log (30h, 8) fails
> + //
> + DevSlpTiming.Supported = 0;
> + } else {
> + CopyMem (&DevSlpTiming, &LogData[48], sizeof (DevSlpTiming));
> + DEBUG ((DEBUG_INFO, "DevSlpTiming: Supported(%d), Deto(%d),
> Madt(%d)\n",
> + DevSlpTiming.Supported, DevSlpTiming.Deto, DevSlpTiming.Madt));
> + }
> +
> + //
> + // Use 20ms as default DETO when DEVSLP TIMING VARIABLES is not
> supported or the DETO is 0.
> + //
> + if ((DevSlpTiming.Supported == 0) || (DevSlpTiming.Deto == 0)) {
> + DevSlpTiming.Deto = 20;
> + }
> +
> + //
> + // Use 10ms as default MADT when DEVSLP TIMING VARIABLES is not
> supported or the MADT is 0.
> + //
> + if ((DevSlpTiming.Supported == 0) || (DevSlpTiming.Madt == 0)) {
> + DevSlpTiming.Madt = 10;
> + }
> +
> + PortDevSlp |= DevSlpTiming.Deto << 2;
> + PortDevSlp |= DevSlpTiming.Madt << 10;
> + AhciOrReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
> +
> + if (mAtaAtapiPolicy->AggressiveDeviceSleepEnable == 1) {
> + if ((Capability2 & AHCI_CAP2_SADM) != 0) {
> + PortDevSlp &= ~AHCI_PORT_DEVSLP_DITO_MASK;
> + PortDevSlp |= (625 << 15);
> + AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
> +
> + PortDevSlp |= AHCI_PORT_DEVSLP_ADSE;
> + AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
> + }
> + }
> +
> +
> + AhciWriteReg (PciIo, Offset + EFI_AHCI_PORT_CMD, PortCmd);
> +
> + DEBUG ((DEBUG_INFO, "Enabled DevSlp feature at port [%d]
> PortMultiplier [%d], Port CMD/DEVSLP = %08x / %08x\n",
> + Port, PortMultiplier, PortCmd, PortDevSlp));
> +
> + return EFI_SUCCESS;
> +}
>
> /**
> Spin-up disk if IDD was incomplete or PUIS feature is enabled
> @@ -2688,6 +2895,13 @@ AhciModeInitialization (
> CreateNewDeviceInfo (Instance, Port, 0xFFFF, DeviceType, &Buffer);
> if (DeviceType == EfiIdeHarddisk) {
> REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
> (EFI_PERIPHERAL_FIXED_MEDIA | EFI_P_PC_ENABLE));
> + AhciEnableDevSlp (
> + PciIo,
> + AhciRegisters,
> + Port,
> + 0,
> + &Buffer
> + );
> }
>
> //
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> index 809bcc307f..0ef749b4c7 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> @@ -1,7 +1,7 @@
> /** @file
> Header file for AHCI mode of ATA host controller.
>
> - Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at
> @@ -29,6 +29,10 @@
>
> #define EFI_AHCI_MAX_PORTS 32
>
> +#define AHCI_CAPABILITY2_OFFSET 0x0024
> +#define AHCI_CAP2_SDS BIT3
> +#define AHCI_CAP2_SADM BIT4
> +
> typedef struct {
> UINT32 Lower32;
> UINT32 Upper32;
> @@ -182,7 +186,13 @@ typedef union {
> #define EFI_AHCI_PORT_SACT 0x0034
> #define EFI_AHCI_PORT_CI 0x0038
> #define EFI_AHCI_PORT_SNTF 0x003C
> -
> +#define AHCI_PORT_DEVSLP 0x0044
> +#define AHCI_PORT_DEVSLP_ADSE BIT0
> +#define AHCI_PORT_DEVSLP_DSP BIT1
> +#define AHCI_PORT_DEVSLP_DETO_MASK 0x000003FC
> +#define AHCI_PORT_DEVSLP_MDAT_MASK 0x00007C00
> +#define AHCI_PORT_DEVSLP_DITO_MASK 0x01FF8000
> +#define AHCI_PORT_DEVSLP_DM_MASK 0x1E000000
>
> #pragma pack(1)
> //
> @@ -283,6 +293,15 @@ typedef struct {
> UINT8 AhciUnknownFisRsvd[0x60];
> } EFI_AHCI_RECEIVED_FIS;
>
> +typedef struct {
> + UINT8 Madt : 5;
> + UINT8 Reserved_5 : 3;
> + UINT8 Deto;
> + UINT16 Reserved_16;
> + UINT32 Reserved_32 : 31;
> + UINT32 Supported : 1;
> +} DEVSLP_TIMING_VARIABLES;
> +
> #pragma pack()
>
> typedef struct {
> --
> 2.16.1.windows.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] MdeModulePkg: Add AtaAtapiPolicy protocol definition
2018-06-01 5:39 ` [PATCH 2/4] MdeModulePkg: Add AtaAtapiPolicy protocol definition Ruiyu Ni
@ 2018-06-04 6:29 ` Zeng, Star
2018-06-04 6:49 ` Ni, Ruiyu
0 siblings, 1 reply; 15+ messages in thread
From: Zeng, Star @ 2018-06-04 6:29 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Zeng, Star
I see below definition for PuisEnable.
+ /// others: Since PUIS setting is non-volatile, platform can use other value than 0/1 to keep hardware PUIS setting.
+ ///
+ UINT8 PuisEnable;
What will the code do for DeviceSleepEnable and AggressiveDeviceSleepEnable when their value is other value than 0/1?
Could that information be also stated?
Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu
Sent: Friday, June 1, 2018 1:39 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>
Subject: [PATCH 2/4] MdeModulePkg: Add AtaAtapiPolicy protocol definition
The patch adds AtaAtapiPolicy protocol which is produced by platform and consumed by AtaAtapiPassThruDxe driver.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h | 59 ++++++++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 ++
2 files changed, 62 insertions(+)
create mode 100644 MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h
diff --git a/MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h b/MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h
new file mode 100644
index 0000000000..86a6213dc9
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h
@@ -0,0 +1,59 @@
+/** @file
+ ATA ATAPI Policy protocol is produced by platform and consumed by
+AtaAtapiPassThruDxe
+ driver.
+
+ Copyright (c) 2018, Intel Corporation. All rights reserved.<BR> This
+ program and the accompanying materials are licensed and made
+ available under the terms and conditions of the BSD License which
+ accompanies this distribution. The full text of the license may be
+ found at http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+#ifndef __ATA_ATAPI_POLICY_H__
+#define __ATA_ATAPI_POLICY_H__
+
+#define EDKII_ATA_ATAPI_POLICY_PROTOCOL_GUID \
+ { \
+ 0xe59cd769, 0x5083, 0x4f26,{ 0x90, 0x94, 0x6c, 0x91, 0x9f, 0x91,
+0x6c, 0x4e } \
+ }
+
+typedef struct {
+ ///
+ /// Protocol version.
+ ///
+ UINT32 Version;
+
+ ///
+ /// 0: Disable Power-up in Standby;
+ /// 1: Enable Power-up in Standby;
+ /// others: Since PUIS setting is non-volatile, platform can use other value than 0/1 to keep hardware PUIS setting.
+ ///
+ UINT8 PuisEnable;
+
+ ///
+ /// 0: Disable Device Sleep;
+ /// 1: Enable Device Sleep;
+ ///
+ UINT8 DeviceSleepEnable;
+
+ ///
+ /// 0: Disable Aggressive Device Sleep; /// 1: Enable Aggressive
+ Device Sleep.
+ ///
+ UINT8 AggressiveDeviceSleepEnable;
+
+ UINT8 Reserved;
+} EDKII_ATA_ATAPI_POLICY_PROTOCOL;
+
+#define EDKII_ATA_ATAPI_POLICY_VERSION 0x00010000
+
+
+extern EFI_GUID gEdkiiAtaAtapiPolicyProtocolGuid;
+
+
+
+#endif
+
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index cc397185f7..3d8376ffdf 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -595,6 +595,9 @@ [Protocols]
gEdkiiPlatformSpecificResetFilterProtocolGuid = { 0x695d7835, 0x8d47, 0x4c11, { 0xab, 0x22, 0xfa, 0x8a, 0xcc, 0xe7, 0xae, 0x7a } }
## Include/Protocol/PlatformSpecificResetHandler.h
gEdkiiPlatformSpecificResetHandlerProtocolGuid = { 0x2df6ba0b, 0x7092, 0x440d, { 0xbd, 0x4, 0xfb, 0x9, 0x1e, 0xc3, 0xf3, 0xc1 } }
+
+ ## Include/Protocol/AtaAtapiPolicy.h
+ gEdkiiAtaAtapiPolicyProtocolGuid = { 0xe59cd769, 0x5083, 0x4f26,{
+ 0x90, 0x94, 0x6c, 0x91, 0x9f, 0x91, 0x6c, 0x4e } }
#
# [Error.gEfiMdeModulePkgTokenSpaceGuid]
# 0x80000001 | Invalid value provided.
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP per policy
2018-06-04 6:20 ` Wu, Hao A
@ 2018-06-04 6:36 ` Ni, Ruiyu
2018-06-04 6:45 ` Wu, Hao A
0 siblings, 1 reply; 15+ messages in thread
From: Ni, Ruiyu @ 2018-06-04 6:36 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org
Hao,
Thanks for the comments.
Reply in below.
Thanks/Ray
> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, June 4, 2018 2:21 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Chiu, Chasel <chasel.chiu@intel.com>
> Subject: RE: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru:
> Enable/disable DEVSLP per policy
>
> Hi Ray,
>
> Some comments:
> 1. Please help to add the 'EFI_' prefix for the new definitions added in .h
> files.
The "EFI_" prefix was removed by purpose.
The offset and the bits location don't change even for non EFI.
I think "EFI_" can only be used for UEFI/PI spec defined protocols/macros.
I don't want to change existing macros. But for new added macros, I'd like
to follow this guide line.
> 2. Do we need to handle the case when AhciEnableDevSlp() returns with an
> error?
It's fine for DEVSLP not enabled. HOST controllers or devices may not support it.
> 3. Please help to adjust the debug level for the below line within function
> AhciEnableDevSlp():
> DEBUG ((DEBUG_ERROR, "Port CMD/DEVSLP = %08x / %08x\n", PortCmd,
> PortDevSlp));
Thanks. I will change to DEBUG_INFO.
> 4. The function description for AhciReadLogExt() does not
> match it prototype.
Thanks. I will update the function header comments.
> 5. Please add the return value descriptions for function AhciEnableDevSlp().
Thanks. I will update the function header comments.
I will run ECC for all code changes.
>
> Also, could you help to provide what kind of tests have been performed for
> this patch and for the whole series? Thanks.
Test performed:
1. Use Seagate/Matrox/WestDigital non-SSD hard drive to test enable PUIS
2. Use the same hard drive to boot. Make sure code can properly spin up the harddrive from PUIS state.
3. Use Intel 530 SSD to test DEVSLP enabling logic.
>
> Best Regards,
> Hao Wu
>
> > -----Original Message-----
> > From: Ni, Ruiyu
> > Sent: Friday, June 01, 2018 1:39 PM
> > To: edk2-devel@lists.01.org
> > Cc: Chiu, Chasel; Wu, Hao A
> > Subject: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru:
> Enable/disable
> > DEVSLP per policy
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> > Cc: Chasel Chiu <chasel.chiu@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > ---
> > MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 214
> > +++++++++++++++++++++++
> > MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h | 23 ++-
> > 2 files changed, 235 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > index 46d5c68996..3741e3b782 100644
> > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > @@ -2217,6 +2217,213 @@ Error6:
> > return Status;
> > }
> >
> > +/**
> > + Read logs from SATA device.
> > +
> > + @param PciIo The PCI IO protocol instance.
> > + @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
> > + @param Port The number of port.
> > + @param PortMultiplier The timeout value of stop.
> > + @param Buffer The data buffer to store IDENTIFY PACKET data.
> > +
> > + @retval EFI_DEVICE_ERROR The cmd abort with error occurs.
> > + @retval EFI_TIMEOUT The operation is time out.
> > + @retval EFI_UNSUPPORTED The device is not ready for executing.
> > + @retval EFI_SUCCESS The cmd executes successfully.
> > +
> > +**/
> > +EFI_STATUS
> > +AhciReadLogExt (
> > + IN EFI_PCI_IO_PROTOCOL *PciIo,
> > + IN EFI_AHCI_REGISTERS *AhciRegisters,
> > + IN UINT8 Port,
> > + IN UINT8 PortMultiplier,
> > + IN OUT UINT8 *Buffer,
> > + IN UINT8 LogNumber,
> > + IN UINT8 PageNumber
> > + )
> > +{
> > + EFI_ATA_COMMAND_BLOCK AtaCommandBlock;
> > + EFI_ATA_STATUS_BLOCK AtaStatusBlock;
> > +
> > + if (PciIo == NULL || AhciRegisters == NULL || Buffer == NULL) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + ///
> > + /// Read log from device
> > + ///
> > + ZeroMem (&AtaCommandBlock, sizeof (EFI_ATA_COMMAND_BLOCK));
> > + ZeroMem (&AtaStatusBlock, sizeof (EFI_ATA_STATUS_BLOCK));
> ZeroMem
> > + (Buffer, 512);
> > +
> > + AtaCommandBlock.AtaCommand = ATA_CMD_READ_LOG_EXT;
> > + AtaCommandBlock.AtaSectorCount = 1;
> > + AtaCommandBlock.AtaSectorNumber = LogNumber;
> > + AtaCommandBlock.AtaCylinderLow = PageNumber;
> > +
> > + return AhciPioTransfer (
> > + PciIo,
> > + AhciRegisters,
> > + Port,
> > + PortMultiplier,
> > + NULL,
> > + 0,
> > + TRUE,
> > + &AtaCommandBlock,
> > + &AtaStatusBlock,
> > + Buffer,
> > + 512,
> > + ATA_ATAPI_TIMEOUT,
> > + NULL
> > + );
> > +}
> > +
> > +/**
> > + Enable DEVSLP command of the disk if supported.
> > + Support for S3 resume to be added later
> > +
> > + @param PciIo The PCI IO protocol instance.
> > + @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
> > + @param Port The number of port.
> > + @param PortMultiplier The multiplier of port.
> > + @param IdentifyData A pointer to data buffer which is used to
> contain
> > IDENTIFY data.
> > +
> > +**/
> > +EFI_STATUS
> > +AhciEnableDevSlp (
> > + IN EFI_PCI_IO_PROTOCOL *PciIo,
> > + IN EFI_AHCI_REGISTERS *AhciRegisters,
> > + IN UINT8 Port,
> > + IN UINT8 PortMultiplier,
> > + IN EFI_IDENTIFY_DATA *IdentifyData
> > + )
> > +{
> > + EFI_STATUS Status;
> > + UINT32 Offset;
> > + UINT32 Capability2;
> > + UINT8 LogData[512];
> > + DEVSLP_TIMING_VARIABLES DevSlpTiming;
> > + UINT32 PortCmd;
> > + UINT32 PortDevSlp;
> > +
> > + if (mAtaAtapiPolicy->DeviceSleepEnable == 0) {
> > + return EFI_SUCCESS;
> > + }
> > +
> > + //
> > + // Do not enable DevSlp if DevSlp is not supported.
> > + //
> > + Capability2 = AhciReadReg (PciIo, AHCI_CAPABILITY2_OFFSET); DEBUG
> > + ((DEBUG_INFO, "AHCI CAPABILITY2 = %08x\n", Capability2)); if
> > + ((Capability2 & AHCI_CAP2_SDS) == 0) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > + //
> > + // Do not enable DevSlp if DevSlp is not present // Do not enable
> > + DevSlp if Hot Plug or Mechanical Presence Switch is
> > supported
> > + //
> > + Offset = EFI_AHCI_PORT_START + Port * EFI_AHCI_PORT_REG_WIDTH;
> > + PortCmd = AhciReadReg (PciIo, Offset + EFI_AHCI_PORT_CMD);
> > + PortDevSlp = AhciReadReg (PciIo, Offset + AHCI_PORT_DEVSLP); DEBUG
> > + ((DEBUG_ERROR, "Port CMD/DEVSLP = %08x / %08x\n", PortCmd,
> > PortDevSlp));
> > + if (((PortDevSlp & AHCI_PORT_DEVSLP_DSP) == 0) ||
> > + ((PortCmd & (EFI_AHCI_PORT_CMD_HPCP |
> > EFI_AHCI_PORT_CMD_MPSP)) != 0)
> > + ) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > + //
> > + // Do not enable DevSlp if the device doesn't support DevSlp //
> > + DEBUG ((DEBUG_INFO, "IDENTIFY DEVICE: [77] = %04x, [78] = %04x, [79]
> > = %04x\n",
> > + IdentifyData->AtaData.reserved_77,
> > + IdentifyData->AtaData.serial_ata_features_supported,
> > + IdentifyData-
> > >AtaData.serial_ata_features_enabled));
> > + if ((IdentifyData->AtaData.serial_ata_features_supported & BIT8) == 0) {
> > + DEBUG ((DEBUG_INFO, "DevSlp feature is not supported for device
> > + at
> > port [%d] PortMultiplier [%d]!\n",
> > + Port, PortMultiplier));
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > + //
> > + // Enable DevSlp when it is not enabled.
> > + //
> > + if ((IdentifyData->AtaData.serial_ata_features_enabled & BIT8) != 0) {
> > + Status = AhciDeviceSetFeature (
> > + PciIo, AhciRegisters, Port, 0, ATA_SUB_CMD_ENABLE_SATA_FEATURE,
> > 0x09, ATA_ATAPI_TIMEOUT
> > + );
> > + DEBUG ((DEBUG_INFO, "DevSlp set feature for device at port [%d]
> > PortMultiplier [%d] - %r\n",
> > + Port, PortMultiplier, Status));
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + }
> > +
> > + Status = AhciReadLogExt(PciIo, AhciRegisters, Port, PortMultiplier,
> > + LogData,
> > 0x30, 0x08);
> > +
> > + //
> > + // Clear PxCMD.ST and PxDEVSLP.ADSE before updating PxDEVSLP.DITO
> > and PxDEVSLP.MDAT.
> > + //
> > + AhciWriteReg (PciIo, Offset + EFI_AHCI_PORT_CMD, PortCmd &
> > ~EFI_AHCI_PORT_CMD_ST);
> > + PortDevSlp &= ~AHCI_PORT_DEVSLP_ADSE; AhciWriteReg (PciIo, Offset
> > + + AHCI_PORT_DEVSLP, PortDevSlp);
> > +
> > + //
> > + // Set PxDEVSLP.DETO and PxDEVSLP.MDAT to 0.
> > + //
> > + PortDevSlp &= ~AHCI_PORT_DEVSLP_DETO_MASK; PortDevSlp &=
> > + ~AHCI_PORT_DEVSLP_MDAT_MASK; AhciWriteReg (PciIo, Offset +
> > + AHCI_PORT_DEVSLP, PortDevSlp); DEBUG ((DEBUG_INFO, "Read Log Ext
> at
> > + port [%d] PortMultiplier [%d] -
> > %r\n", Port, PortMultiplier, Status));
> > + if (EFI_ERROR (Status)) {
> > + //
> > + // Assume DEVSLP TIMING VARIABLES is not supported if the
> > + Identify
> > Device Data log (30h, 8) fails
> > + //
> > + DevSlpTiming.Supported = 0;
> > + } else {
> > + CopyMem (&DevSlpTiming, &LogData[48], sizeof (DevSlpTiming));
> > + DEBUG ((DEBUG_INFO, "DevSlpTiming: Supported(%d), Deto(%d),
> > Madt(%d)\n",
> > + DevSlpTiming.Supported, DevSlpTiming.Deto,
> > + DevSlpTiming.Madt)); }
> > +
> > + //
> > + // Use 20ms as default DETO when DEVSLP TIMING VARIABLES is not
> > supported or the DETO is 0.
> > + //
> > + if ((DevSlpTiming.Supported == 0) || (DevSlpTiming.Deto == 0)) {
> > + DevSlpTiming.Deto = 20;
> > + }
> > +
> > + //
> > + // Use 10ms as default MADT when DEVSLP TIMING VARIABLES is not
> > supported or the MADT is 0.
> > + //
> > + if ((DevSlpTiming.Supported == 0) || (DevSlpTiming.Madt == 0)) {
> > + DevSlpTiming.Madt = 10;
> > + }
> > +
> > + PortDevSlp |= DevSlpTiming.Deto << 2; PortDevSlp |=
> > + DevSlpTiming.Madt << 10; AhciOrReg (PciIo, Offset +
> > + AHCI_PORT_DEVSLP, PortDevSlp);
> > +
> > + if (mAtaAtapiPolicy->AggressiveDeviceSleepEnable == 1) {
> > + if ((Capability2 & AHCI_CAP2_SADM) != 0) {
> > + PortDevSlp &= ~AHCI_PORT_DEVSLP_DITO_MASK;
> > + PortDevSlp |= (625 << 15);
> > + AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
> > +
> > + PortDevSlp |= AHCI_PORT_DEVSLP_ADSE;
> > + AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
> > + }
> > + }
> > +
> > +
> > + AhciWriteReg (PciIo, Offset + EFI_AHCI_PORT_CMD, PortCmd);
> > +
> > + DEBUG ((DEBUG_INFO, "Enabled DevSlp feature at port [%d]
> > PortMultiplier [%d], Port CMD/DEVSLP = %08x / %08x\n",
> > + Port, PortMultiplier, PortCmd, PortDevSlp));
> > +
> > + return EFI_SUCCESS;
> > +}
> >
> > /**
> > Spin-up disk if IDD was incomplete or PUIS feature is enabled @@
> > -2688,6 +2895,13 @@ AhciModeInitialization (
> > CreateNewDeviceInfo (Instance, Port, 0xFFFF, DeviceType, &Buffer);
> > if (DeviceType == EfiIdeHarddisk) {
> > REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
> > (EFI_PERIPHERAL_FIXED_MEDIA | EFI_P_PC_ENABLE));
> > + AhciEnableDevSlp (
> > + PciIo,
> > + AhciRegisters,
> > + Port,
> > + 0,
> > + &Buffer
> > + );
> > }
> >
> > //
> > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> > index 809bcc307f..0ef749b4c7 100644
> > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> > @@ -1,7 +1,7 @@
> > /** @file
> > Header file for AHCI mode of ATA host controller.
> >
> > - Copyright (c) 2010 - 2014, Intel Corporation. All rights
> > reserved.<BR>
> > + Copyright (c) 2010 - 2018, Intel Corporation. All rights
> > + reserved.<BR>
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of
> > the BSD License
> > which accompanies this distribution. The full text of the license
> > may be found at @@ -29,6 +29,10 @@
> >
> > #define EFI_AHCI_MAX_PORTS 32
> >
> > +#define AHCI_CAPABILITY2_OFFSET 0x0024
> > +#define AHCI_CAP2_SDS BIT3
> > +#define AHCI_CAP2_SADM BIT4
> > +
> > typedef struct {
> > UINT32 Lower32;
> > UINT32 Upper32;
> > @@ -182,7 +186,13 @@ typedef union {
> > #define EFI_AHCI_PORT_SACT 0x0034
> > #define EFI_AHCI_PORT_CI 0x0038
> > #define EFI_AHCI_PORT_SNTF 0x003C
> > -
> > +#define AHCI_PORT_DEVSLP 0x0044
> > +#define AHCI_PORT_DEVSLP_ADSE BIT0
> > +#define AHCI_PORT_DEVSLP_DSP BIT1
> > +#define AHCI_PORT_DEVSLP_DETO_MASK 0x000003FC
> > +#define AHCI_PORT_DEVSLP_MDAT_MASK 0x00007C00
> > +#define AHCI_PORT_DEVSLP_DITO_MASK 0x01FF8000
> > +#define AHCI_PORT_DEVSLP_DM_MASK 0x1E000000
> >
> > #pragma pack(1)
> > //
> > @@ -283,6 +293,15 @@ typedef struct {
> > UINT8 AhciUnknownFisRsvd[0x60];
> > } EFI_AHCI_RECEIVED_FIS;
> >
> > +typedef struct {
> > + UINT8 Madt : 5;
> > + UINT8 Reserved_5 : 3;
> > + UINT8 Deto;
> > + UINT16 Reserved_16;
> > + UINT32 Reserved_32 : 31;
> > + UINT32 Supported : 1;
> > +} DEVSLP_TIMING_VARIABLES;
> > +
> > #pragma pack()
> >
> > typedef struct {
> > --
> > 2.16.1.windows.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP per policy
2018-06-04 6:36 ` Ni, Ruiyu
@ 2018-06-04 6:45 ` Wu, Hao A
2018-06-04 6:48 ` Ni, Ruiyu
0 siblings, 1 reply; 15+ messages in thread
From: Wu, Hao A @ 2018-06-04 6:45 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Monday, June 04, 2018 2:37 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Chiu, Chasel
> Subject: RE: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru:
> Enable/disable DEVSLP per policy
>
> Hao,
> Thanks for the comments.
> Reply in below.
>
> Thanks/Ray
>
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Monday, June 4, 2018 2:21 PM
> > To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> > Cc: Chiu, Chasel <chasel.chiu@intel.com>
> > Subject: RE: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru:
> > Enable/disable DEVSLP per policy
> >
> > Hi Ray,
> >
> > Some comments:
> > 1. Please help to add the 'EFI_' prefix for the new definitions added in .h
> > files.
> The "EFI_" prefix was removed by purpose.
> The offset and the bits location don't change even for non EFI.
> I think "EFI_" can only be used for UEFI/PI spec defined protocols/macros.
> I don't want to change existing macros. But for new added macros, I'd like
> to follow this guide line.
>
> > 2. Do we need to handle the case when AhciEnableDevSlp() returns with an
> > error?
> It's fine for DEVSLP not enabled. HOST controllers or devices may not support
> it.
Got it.
But what if the error status returned is not related with the support of DEVSLP.
For example, if a call to the AhciDeviceSetFeature() to enable the DEVSLP
feature?
Maybe, adding a debug error message for the above case is enough?
Best Regards,
Hao Wu
>
> > 3. Please help to adjust the debug level for the below line within function
> > AhciEnableDevSlp():
> > DEBUG ((DEBUG_ERROR, "Port CMD/DEVSLP = %08x / %08x\n", PortCmd,
> > PortDevSlp));
> Thanks. I will change to DEBUG_INFO.
>
> > 4. The function description for AhciReadLogExt() does not
> > match it prototype.
> Thanks. I will update the function header comments.
> > 5. Please add the return value descriptions for function AhciEnableDevSlp().
> Thanks. I will update the function header comments.
> I will run ECC for all code changes.
> >
> > Also, could you help to provide what kind of tests have been performed for
> > this patch and for the whole series? Thanks.
> Test performed:
> 1. Use Seagate/Matrox/WestDigital non-SSD hard drive to test enable PUIS
> 2. Use the same hard drive to boot. Make sure code can properly spin up the
> harddrive from PUIS state.
> 3. Use Intel 530 SSD to test DEVSLP enabling logic.
>
> >
> > Best Regards,
> > Hao Wu
> >
> > > -----Original Message-----
> > > From: Ni, Ruiyu
> > > Sent: Friday, June 01, 2018 1:39 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Chiu, Chasel; Wu, Hao A
> > > Subject: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru:
> > Enable/disable
> > > DEVSLP per policy
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> > > Cc: Chasel Chiu <chasel.chiu@intel.com>
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > ---
> > > MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 214
> > > +++++++++++++++++++++++
> > > MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h | 23 ++-
> > > 2 files changed, 235 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > index 46d5c68996..3741e3b782 100644
> > > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > @@ -2217,6 +2217,213 @@ Error6:
> > > return Status;
> > > }
> > >
> > > +/**
> > > + Read logs from SATA device.
> > > +
> > > + @param PciIo The PCI IO protocol instance.
> > > + @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
> > > + @param Port The number of port.
> > > + @param PortMultiplier The timeout value of stop.
> > > + @param Buffer The data buffer to store IDENTIFY PACKET data.
> > > +
> > > + @retval EFI_DEVICE_ERROR The cmd abort with error occurs.
> > > + @retval EFI_TIMEOUT The operation is time out.
> > > + @retval EFI_UNSUPPORTED The device is not ready for executing.
> > > + @retval EFI_SUCCESS The cmd executes successfully.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +AhciReadLogExt (
> > > + IN EFI_PCI_IO_PROTOCOL *PciIo,
> > > + IN EFI_AHCI_REGISTERS *AhciRegisters,
> > > + IN UINT8 Port,
> > > + IN UINT8 PortMultiplier,
> > > + IN OUT UINT8 *Buffer,
> > > + IN UINT8 LogNumber,
> > > + IN UINT8 PageNumber
> > > + )
> > > +{
> > > + EFI_ATA_COMMAND_BLOCK AtaCommandBlock;
> > > + EFI_ATA_STATUS_BLOCK AtaStatusBlock;
> > > +
> > > + if (PciIo == NULL || AhciRegisters == NULL || Buffer == NULL) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > + ///
> > > + /// Read log from device
> > > + ///
> > > + ZeroMem (&AtaCommandBlock, sizeof
> (EFI_ATA_COMMAND_BLOCK));
> > > + ZeroMem (&AtaStatusBlock, sizeof (EFI_ATA_STATUS_BLOCK));
> > ZeroMem
> > > + (Buffer, 512);
> > > +
> > > + AtaCommandBlock.AtaCommand = ATA_CMD_READ_LOG_EXT;
> > > + AtaCommandBlock.AtaSectorCount = 1;
> > > + AtaCommandBlock.AtaSectorNumber = LogNumber;
> > > + AtaCommandBlock.AtaCylinderLow = PageNumber;
> > > +
> > > + return AhciPioTransfer (
> > > + PciIo,
> > > + AhciRegisters,
> > > + Port,
> > > + PortMultiplier,
> > > + NULL,
> > > + 0,
> > > + TRUE,
> > > + &AtaCommandBlock,
> > > + &AtaStatusBlock,
> > > + Buffer,
> > > + 512,
> > > + ATA_ATAPI_TIMEOUT,
> > > + NULL
> > > + );
> > > +}
> > > +
> > > +/**
> > > + Enable DEVSLP command of the disk if supported.
> > > + Support for S3 resume to be added later
> > > +
> > > + @param PciIo The PCI IO protocol instance.
> > > + @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
> > > + @param Port The number of port.
> > > + @param PortMultiplier The multiplier of port.
> > > + @param IdentifyData A pointer to data buffer which is used to
> > contain
> > > IDENTIFY data.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +AhciEnableDevSlp (
> > > + IN EFI_PCI_IO_PROTOCOL *PciIo,
> > > + IN EFI_AHCI_REGISTERS *AhciRegisters,
> > > + IN UINT8 Port,
> > > + IN UINT8 PortMultiplier,
> > > + IN EFI_IDENTIFY_DATA *IdentifyData
> > > + )
> > > +{
> > > + EFI_STATUS Status;
> > > + UINT32 Offset;
> > > + UINT32 Capability2;
> > > + UINT8 LogData[512];
> > > + DEVSLP_TIMING_VARIABLES DevSlpTiming;
> > > + UINT32 PortCmd;
> > > + UINT32 PortDevSlp;
> > > +
> > > + if (mAtaAtapiPolicy->DeviceSleepEnable == 0) {
> > > + return EFI_SUCCESS;
> > > + }
> > > +
> > > + //
> > > + // Do not enable DevSlp if DevSlp is not supported.
> > > + //
> > > + Capability2 = AhciReadReg (PciIo, AHCI_CAPABILITY2_OFFSET); DEBUG
> > > + ((DEBUG_INFO, "AHCI CAPABILITY2 = %08x\n", Capability2)); if
> > > + ((Capability2 & AHCI_CAP2_SDS) == 0) {
> > > + return EFI_UNSUPPORTED;
> > > + }
> > > +
> > > + //
> > > + // Do not enable DevSlp if DevSlp is not present // Do not enable
> > > + DevSlp if Hot Plug or Mechanical Presence Switch is
> > > supported
> > > + //
> > > + Offset = EFI_AHCI_PORT_START + Port *
> EFI_AHCI_PORT_REG_WIDTH;
> > > + PortCmd = AhciReadReg (PciIo, Offset + EFI_AHCI_PORT_CMD);
> > > + PortDevSlp = AhciReadReg (PciIo, Offset + AHCI_PORT_DEVSLP);
> DEBUG
> > > + ((DEBUG_ERROR, "Port CMD/DEVSLP = %08x / %08x\n", PortCmd,
> > > PortDevSlp));
> > > + if (((PortDevSlp & AHCI_PORT_DEVSLP_DSP) == 0) ||
> > > + ((PortCmd & (EFI_AHCI_PORT_CMD_HPCP |
> > > EFI_AHCI_PORT_CMD_MPSP)) != 0)
> > > + ) {
> > > + return EFI_UNSUPPORTED;
> > > + }
> > > +
> > > + //
> > > + // Do not enable DevSlp if the device doesn't support DevSlp //
> > > + DEBUG ((DEBUG_INFO, "IDENTIFY DEVICE: [77] = %04x, [78] = %04x, [79]
> > > = %04x\n",
> > > + IdentifyData->AtaData.reserved_77,
> > > + IdentifyData->AtaData.serial_ata_features_supported,
> > > + IdentifyData-
> > > >AtaData.serial_ata_features_enabled));
> > > + if ((IdentifyData->AtaData.serial_ata_features_supported & BIT8) == 0)
> {
> > > + DEBUG ((DEBUG_INFO, "DevSlp feature is not supported for device
> > > + at
> > > port [%d] PortMultiplier [%d]!\n",
> > > + Port, PortMultiplier));
> > > + return EFI_UNSUPPORTED;
> > > + }
> > > +
> > > + //
> > > + // Enable DevSlp when it is not enabled.
> > > + //
> > > + if ((IdentifyData->AtaData.serial_ata_features_enabled & BIT8) != 0) {
> > > + Status = AhciDeviceSetFeature (
> > > + PciIo, AhciRegisters, Port, 0,
> ATA_SUB_CMD_ENABLE_SATA_FEATURE,
> > > 0x09, ATA_ATAPI_TIMEOUT
> > > + );
> > > + DEBUG ((DEBUG_INFO, "DevSlp set feature for device at port [%d]
> > > PortMultiplier [%d] - %r\n",
> > > + Port, PortMultiplier, Status));
> > > + if (EFI_ERROR (Status)) {
> > > + return Status;
> > > + }
> > > + }
> > > +
> > > + Status = AhciReadLogExt(PciIo, AhciRegisters, Port, PortMultiplier,
> > > + LogData,
> > > 0x30, 0x08);
> > > +
> > > + //
> > > + // Clear PxCMD.ST and PxDEVSLP.ADSE before updating
> PxDEVSLP.DITO
> > > and PxDEVSLP.MDAT.
> > > + //
> > > + AhciWriteReg (PciIo, Offset + EFI_AHCI_PORT_CMD, PortCmd &
> > > ~EFI_AHCI_PORT_CMD_ST);
> > > + PortDevSlp &= ~AHCI_PORT_DEVSLP_ADSE; AhciWriteReg (PciIo,
> Offset
> > > + + AHCI_PORT_DEVSLP, PortDevSlp);
> > > +
> > > + //
> > > + // Set PxDEVSLP.DETO and PxDEVSLP.MDAT to 0.
> > > + //
> > > + PortDevSlp &= ~AHCI_PORT_DEVSLP_DETO_MASK; PortDevSlp &=
> > > + ~AHCI_PORT_DEVSLP_MDAT_MASK; AhciWriteReg (PciIo, Offset +
> > > + AHCI_PORT_DEVSLP, PortDevSlp); DEBUG ((DEBUG_INFO, "Read Log
> Ext
> > at
> > > + port [%d] PortMultiplier [%d] -
> > > %r\n", Port, PortMultiplier, Status));
> > > + if (EFI_ERROR (Status)) {
> > > + //
> > > + // Assume DEVSLP TIMING VARIABLES is not supported if the
> > > + Identify
> > > Device Data log (30h, 8) fails
> > > + //
> > > + DevSlpTiming.Supported = 0;
> > > + } else {
> > > + CopyMem (&DevSlpTiming, &LogData[48], sizeof (DevSlpTiming));
> > > + DEBUG ((DEBUG_INFO, "DevSlpTiming: Supported(%d), Deto(%d),
> > > Madt(%d)\n",
> > > + DevSlpTiming.Supported, DevSlpTiming.Deto,
> > > + DevSlpTiming.Madt)); }
> > > +
> > > + //
> > > + // Use 20ms as default DETO when DEVSLP TIMING VARIABLES is not
> > > supported or the DETO is 0.
> > > + //
> > > + if ((DevSlpTiming.Supported == 0) || (DevSlpTiming.Deto == 0)) {
> > > + DevSlpTiming.Deto = 20;
> > > + }
> > > +
> > > + //
> > > + // Use 10ms as default MADT when DEVSLP TIMING VARIABLES is not
> > > supported or the MADT is 0.
> > > + //
> > > + if ((DevSlpTiming.Supported == 0) || (DevSlpTiming.Madt == 0)) {
> > > + DevSlpTiming.Madt = 10;
> > > + }
> > > +
> > > + PortDevSlp |= DevSlpTiming.Deto << 2; PortDevSlp |=
> > > + DevSlpTiming.Madt << 10; AhciOrReg (PciIo, Offset +
> > > + AHCI_PORT_DEVSLP, PortDevSlp);
> > > +
> > > + if (mAtaAtapiPolicy->AggressiveDeviceSleepEnable == 1) {
> > > + if ((Capability2 & AHCI_CAP2_SADM) != 0) {
> > > + PortDevSlp &= ~AHCI_PORT_DEVSLP_DITO_MASK;
> > > + PortDevSlp |= (625 << 15);
> > > + AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
> > > +
> > > + PortDevSlp |= AHCI_PORT_DEVSLP_ADSE;
> > > + AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
> > > + }
> > > + }
> > > +
> > > +
> > > + AhciWriteReg (PciIo, Offset + EFI_AHCI_PORT_CMD, PortCmd);
> > > +
> > > + DEBUG ((DEBUG_INFO, "Enabled DevSlp feature at port [%d]
> > > PortMultiplier [%d], Port CMD/DEVSLP = %08x / %08x\n",
> > > + Port, PortMultiplier, PortCmd, PortDevSlp));
> > > +
> > > + return EFI_SUCCESS;
> > > +}
> > >
> > > /**
> > > Spin-up disk if IDD was incomplete or PUIS feature is enabled @@
> > > -2688,6 +2895,13 @@ AhciModeInitialization (
> > > CreateNewDeviceInfo (Instance, Port, 0xFFFF, DeviceType, &Buffer);
> > > if (DeviceType == EfiIdeHarddisk) {
> > > REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
> > > (EFI_PERIPHERAL_FIXED_MEDIA | EFI_P_PC_ENABLE));
> > > + AhciEnableDevSlp (
> > > + PciIo,
> > > + AhciRegisters,
> > > + Port,
> > > + 0,
> > > + &Buffer
> > > + );
> > > }
> > >
> > > //
> > > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> > > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> > > index 809bcc307f..0ef749b4c7 100644
> > > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> > > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> > > @@ -1,7 +1,7 @@
> > > /** @file
> > > Header file for AHCI mode of ATA host controller.
> > >
> > > - Copyright (c) 2010 - 2014, Intel Corporation. All rights
> > > reserved.<BR>
> > > + Copyright (c) 2010 - 2018, Intel Corporation. All rights
> > > + reserved.<BR>
> > > This program and the accompanying materials
> > > are licensed and made available under the terms and conditions of
> > > the BSD License
> > > which accompanies this distribution. The full text of the license
> > > may be found at @@ -29,6 +29,10 @@
> > >
> > > #define EFI_AHCI_MAX_PORTS 32
> > >
> > > +#define AHCI_CAPABILITY2_OFFSET 0x0024
> > > +#define AHCI_CAP2_SDS BIT3
> > > +#define AHCI_CAP2_SADM BIT4
> > > +
> > > typedef struct {
> > > UINT32 Lower32;
> > > UINT32 Upper32;
> > > @@ -182,7 +186,13 @@ typedef union {
> > > #define EFI_AHCI_PORT_SACT 0x0034
> > > #define EFI_AHCI_PORT_CI 0x0038
> > > #define EFI_AHCI_PORT_SNTF 0x003C
> > > -
> > > +#define AHCI_PORT_DEVSLP 0x0044
> > > +#define AHCI_PORT_DEVSLP_ADSE BIT0
> > > +#define AHCI_PORT_DEVSLP_DSP BIT1
> > > +#define AHCI_PORT_DEVSLP_DETO_MASK 0x000003FC
> > > +#define AHCI_PORT_DEVSLP_MDAT_MASK 0x00007C00
> > > +#define AHCI_PORT_DEVSLP_DITO_MASK 0x01FF8000
> > > +#define AHCI_PORT_DEVSLP_DM_MASK 0x1E000000
> > >
> > > #pragma pack(1)
> > > //
> > > @@ -283,6 +293,15 @@ typedef struct {
> > > UINT8 AhciUnknownFisRsvd[0x60];
> > > } EFI_AHCI_RECEIVED_FIS;
> > >
> > > +typedef struct {
> > > + UINT8 Madt : 5;
> > > + UINT8 Reserved_5 : 3;
> > > + UINT8 Deto;
> > > + UINT16 Reserved_16;
> > > + UINT32 Reserved_32 : 31;
> > > + UINT32 Supported : 1;
> > > +} DEVSLP_TIMING_VARIABLES;
> > > +
> > > #pragma pack()
> > >
> > > typedef struct {
> > > --
> > > 2.16.1.windows.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP per policy
2018-06-04 6:45 ` Wu, Hao A
@ 2018-06-04 6:48 ` Ni, Ruiyu
0 siblings, 0 replies; 15+ messages in thread
From: Ni, Ruiyu @ 2018-06-04 6:48 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org
Thanks/Ray
> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, June 4, 2018 2:45 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Chiu, Chasel <chasel.chiu@intel.com>
> Subject: RE: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru:
> Enable/disable DEVSLP per policy
>
> > -----Original Message-----
> > From: Ni, Ruiyu
> > Sent: Monday, June 04, 2018 2:37 PM
> > To: Wu, Hao A; edk2-devel@lists.01.org
> > Cc: Chiu, Chasel
> > Subject: RE: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru:
> > Enable/disable DEVSLP per policy
> >
> > Hao,
> > Thanks for the comments.
> > Reply in below.
> >
> > Thanks/Ray
> >
> > > -----Original Message-----
> > > From: Wu, Hao A
> > > Sent: Monday, June 4, 2018 2:21 PM
> > > To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> > > Cc: Chiu, Chasel <chasel.chiu@intel.com>
> > > Subject: RE: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru:
> > > Enable/disable DEVSLP per policy
> > >
> > > Hi Ray,
> > >
> > > Some comments:
> > > 1. Please help to add the 'EFI_' prefix for the new definitions added in .h
> > > files.
> > The "EFI_" prefix was removed by purpose.
> > The offset and the bits location don't change even for non EFI.
> > I think "EFI_" can only be used for UEFI/PI spec defined protocols/macros.
> > I don't want to change existing macros. But for new added macros, I'd
> > like to follow this guide line.
> >
> > > 2. Do we need to handle the case when AhciEnableDevSlp() returns
> > > with an error?
> > It's fine for DEVSLP not enabled. HOST controllers or devices may not
> > support it.
>
> Got it.
> But what if the error status returned is not related with the support of
> DEVSLP.
> For example, if a call to the AhciDeviceSetFeature() to enable the DEVSLP
> feature?
>
> Maybe, adding a debug error message for the above case is enough?
I have a debug message for the set feature failure:
DEBUG ((DEBUG_INFO, "DevSlp set feature for device at port [%d] PortMultiplier [%d] - %r\n",
Port, PortMultiplier, Status));
>
> Best Regards,
> Hao Wu
>
> >
> > > 3. Please help to adjust the debug level for the below line within function
> > > AhciEnableDevSlp():
> > > DEBUG ((DEBUG_ERROR, "Port CMD/DEVSLP = %08x / %08x\n",
> PortCmd,
> > > PortDevSlp));
> > Thanks. I will change to DEBUG_INFO.
> >
> > > 4. The function description for AhciReadLogExt() does not match it
> > > prototype.
> > Thanks. I will update the function header comments.
> > > 5. Please add the return value descriptions for function
> AhciEnableDevSlp().
> > Thanks. I will update the function header comments.
> > I will run ECC for all code changes.
> > >
> > > Also, could you help to provide what kind of tests have been
> > > performed for this patch and for the whole series? Thanks.
> > Test performed:
> > 1. Use Seagate/Matrox/WestDigital non-SSD hard drive to test enable
> > PUIS 2. Use the same hard drive to boot. Make sure code can properly
> > spin up the harddrive from PUIS state.
> > 3. Use Intel 530 SSD to test DEVSLP enabling logic.
> >
> > >
> > > Best Regards,
> > > Hao Wu
> > >
> > > > -----Original Message-----
> > > > From: Ni, Ruiyu
> > > > Sent: Friday, June 01, 2018 1:39 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Chiu, Chasel; Wu, Hao A
> > > > Subject: [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru:
> > > Enable/disable
> > > > DEVSLP per policy
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> > > > Cc: Chasel Chiu <chasel.chiu@intel.com>
> > > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > > ---
> > > > MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 214
> > > > +++++++++++++++++++++++
> > > > MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h | 23 ++-
> > > > 2 files changed, 235 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > > index 46d5c68996..3741e3b782 100644
> > > > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > > @@ -2217,6 +2217,213 @@ Error6:
> > > > return Status;
> > > > }
> > > >
> > > > +/**
> > > > + Read logs from SATA device.
> > > > +
> > > > + @param PciIo The PCI IO protocol instance.
> > > > + @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
> > > > + @param Port The number of port.
> > > > + @param PortMultiplier The timeout value of stop.
> > > > + @param Buffer The data buffer to store IDENTIFY PACKET data.
> > > > +
> > > > + @retval EFI_DEVICE_ERROR The cmd abort with error occurs.
> > > > + @retval EFI_TIMEOUT The operation is time out.
> > > > + @retval EFI_UNSUPPORTED The device is not ready for executing.
> > > > + @retval EFI_SUCCESS The cmd executes successfully.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +AhciReadLogExt (
> > > > + IN EFI_PCI_IO_PROTOCOL *PciIo,
> > > > + IN EFI_AHCI_REGISTERS *AhciRegisters,
> > > > + IN UINT8 Port,
> > > > + IN UINT8 PortMultiplier,
> > > > + IN OUT UINT8 *Buffer,
> > > > + IN UINT8 LogNumber,
> > > > + IN UINT8 PageNumber
> > > > + )
> > > > +{
> > > > + EFI_ATA_COMMAND_BLOCK AtaCommandBlock;
> > > > + EFI_ATA_STATUS_BLOCK AtaStatusBlock;
> > > > +
> > > > + if (PciIo == NULL || AhciRegisters == NULL || Buffer == NULL) {
> > > > + return EFI_INVALID_PARAMETER; }
> > > > +
> > > > + ///
> > > > + /// Read log from device
> > > > + ///
> > > > + ZeroMem (&AtaCommandBlock, sizeof
> > (EFI_ATA_COMMAND_BLOCK));
> > > > + ZeroMem (&AtaStatusBlock, sizeof (EFI_ATA_STATUS_BLOCK));
> > > ZeroMem
> > > > + (Buffer, 512);
> > > > +
> > > > + AtaCommandBlock.AtaCommand = ATA_CMD_READ_LOG_EXT;
> > > > + AtaCommandBlock.AtaSectorCount = 1;
> > > > + AtaCommandBlock.AtaSectorNumber = LogNumber;
> > > > + AtaCommandBlock.AtaCylinderLow = PageNumber;
> > > > +
> > > > + return AhciPioTransfer (
> > > > + PciIo,
> > > > + AhciRegisters,
> > > > + Port,
> > > > + PortMultiplier,
> > > > + NULL,
> > > > + 0,
> > > > + TRUE,
> > > > + &AtaCommandBlock,
> > > > + &AtaStatusBlock,
> > > > + Buffer,
> > > > + 512,
> > > > + ATA_ATAPI_TIMEOUT,
> > > > + NULL
> > > > + );
> > > > +}
> > > > +
> > > > +/**
> > > > + Enable DEVSLP command of the disk if supported.
> > > > + Support for S3 resume to be added later
> > > > +
> > > > + @param PciIo The PCI IO protocol instance.
> > > > + @param AhciRegisters The pointer to the EFI_AHCI_REGISTERS.
> > > > + @param Port The number of port.
> > > > + @param PortMultiplier The multiplier of port.
> > > > + @param IdentifyData A pointer to data buffer which is used to
> > > contain
> > > > IDENTIFY data.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +AhciEnableDevSlp (
> > > > + IN EFI_PCI_IO_PROTOCOL *PciIo,
> > > > + IN EFI_AHCI_REGISTERS *AhciRegisters,
> > > > + IN UINT8 Port,
> > > > + IN UINT8 PortMultiplier,
> > > > + IN EFI_IDENTIFY_DATA *IdentifyData
> > > > + )
> > > > +{
> > > > + EFI_STATUS Status;
> > > > + UINT32 Offset;
> > > > + UINT32 Capability2;
> > > > + UINT8 LogData[512];
> > > > + DEVSLP_TIMING_VARIABLES DevSlpTiming;
> > > > + UINT32 PortCmd;
> > > > + UINT32 PortDevSlp;
> > > > +
> > > > + if (mAtaAtapiPolicy->DeviceSleepEnable == 0) {
> > > > + return EFI_SUCCESS;
> > > > + }
> > > > +
> > > > + //
> > > > + // Do not enable DevSlp if DevSlp is not supported.
> > > > + //
> > > > + Capability2 = AhciReadReg (PciIo, AHCI_CAPABILITY2_OFFSET);
> > > > + DEBUG ((DEBUG_INFO, "AHCI CAPABILITY2 = %08x\n", Capability2));
> > > > + if
> > > > + ((Capability2 & AHCI_CAP2_SDS) == 0) {
> > > > + return EFI_UNSUPPORTED;
> > > > + }
> > > > +
> > > > + //
> > > > + // Do not enable DevSlp if DevSlp is not present // Do not
> > > > + enable DevSlp if Hot Plug or Mechanical Presence Switch is
> > > > supported
> > > > + //
> > > > + Offset = EFI_AHCI_PORT_START + Port *
> > EFI_AHCI_PORT_REG_WIDTH;
> > > > + PortCmd = AhciReadReg (PciIo, Offset + EFI_AHCI_PORT_CMD);
> > > > + PortDevSlp = AhciReadReg (PciIo, Offset + AHCI_PORT_DEVSLP);
> > DEBUG
> > > > + ((DEBUG_ERROR, "Port CMD/DEVSLP = %08x / %08x\n", PortCmd,
> > > > PortDevSlp));
> > > > + if (((PortDevSlp & AHCI_PORT_DEVSLP_DSP) == 0) ||
> > > > + ((PortCmd & (EFI_AHCI_PORT_CMD_HPCP |
> > > > EFI_AHCI_PORT_CMD_MPSP)) != 0)
> > > > + ) {
> > > > + return EFI_UNSUPPORTED;
> > > > + }
> > > > +
> > > > + //
> > > > + // Do not enable DevSlp if the device doesn't support DevSlp
> > > > + // DEBUG ((DEBUG_INFO, "IDENTIFY DEVICE: [77] = %04x, [78] =
> > > > + %04x, [79]
> > > > = %04x\n",
> > > > + IdentifyData->AtaData.reserved_77,
> > > > + IdentifyData->AtaData.serial_ata_features_supported,
> > > > + IdentifyData-
> > > > >AtaData.serial_ata_features_enabled));
> > > > + if ((IdentifyData->AtaData.serial_ata_features_supported &
> > > > + BIT8) == 0)
> > {
> > > > + DEBUG ((DEBUG_INFO, "DevSlp feature is not supported for
> > > > + device at
> > > > port [%d] PortMultiplier [%d]!\n",
> > > > + Port, PortMultiplier));
> > > > + return EFI_UNSUPPORTED;
> > > > + }
> > > > +
> > > > + //
> > > > + // Enable DevSlp when it is not enabled.
> > > > + //
> > > > + if ((IdentifyData->AtaData.serial_ata_features_enabled & BIT8) != 0)
> {
> > > > + Status = AhciDeviceSetFeature (
> > > > + PciIo, AhciRegisters, Port, 0,
> > ATA_SUB_CMD_ENABLE_SATA_FEATURE,
> > > > 0x09, ATA_ATAPI_TIMEOUT
> > > > + );
> > > > + DEBUG ((DEBUG_INFO, "DevSlp set feature for device at port
> > > > + [%d]
> > > > PortMultiplier [%d] - %r\n",
> > > > + Port, PortMultiplier, Status));
> > > > + if (EFI_ERROR (Status)) {
> > > > + return Status;
> > > > + }
> > > > + }
> > > > +
> > > > + Status = AhciReadLogExt(PciIo, AhciRegisters, Port,
> > > > + PortMultiplier, LogData,
> > > > 0x30, 0x08);
> > > > +
> > > > + //
> > > > + // Clear PxCMD.ST and PxDEVSLP.ADSE before updating
> > PxDEVSLP.DITO
> > > > and PxDEVSLP.MDAT.
> > > > + //
> > > > + AhciWriteReg (PciIo, Offset + EFI_AHCI_PORT_CMD, PortCmd &
> > > > ~EFI_AHCI_PORT_CMD_ST);
> > > > + PortDevSlp &= ~AHCI_PORT_DEVSLP_ADSE; AhciWriteReg (PciIo,
> > Offset
> > > > + + AHCI_PORT_DEVSLP, PortDevSlp);
> > > > +
> > > > + //
> > > > + // Set PxDEVSLP.DETO and PxDEVSLP.MDAT to 0.
> > > > + //
> > > > + PortDevSlp &= ~AHCI_PORT_DEVSLP_DETO_MASK; PortDevSlp &=
> > > > + ~AHCI_PORT_DEVSLP_MDAT_MASK; AhciWriteReg (PciIo, Offset +
> > > > + AHCI_PORT_DEVSLP, PortDevSlp); DEBUG ((DEBUG_INFO, "Read Log
> > Ext
> > > at
> > > > + port [%d] PortMultiplier [%d] -
> > > > %r\n", Port, PortMultiplier, Status));
> > > > + if (EFI_ERROR (Status)) {
> > > > + //
> > > > + // Assume DEVSLP TIMING VARIABLES is not supported if the
> > > > + Identify
> > > > Device Data log (30h, 8) fails
> > > > + //
> > > > + DevSlpTiming.Supported = 0;
> > > > + } else {
> > > > + CopyMem (&DevSlpTiming, &LogData[48], sizeof (DevSlpTiming));
> > > > + DEBUG ((DEBUG_INFO, "DevSlpTiming: Supported(%d), Deto(%d),
> > > > Madt(%d)\n",
> > > > + DevSlpTiming.Supported, DevSlpTiming.Deto,
> > > > + DevSlpTiming.Madt)); }
> > > > +
> > > > + //
> > > > + // Use 20ms as default DETO when DEVSLP TIMING VARIABLES is not
> > > > supported or the DETO is 0.
> > > > + //
> > > > + if ((DevSlpTiming.Supported == 0) || (DevSlpTiming.Deto == 0)) {
> > > > + DevSlpTiming.Deto = 20;
> > > > + }
> > > > +
> > > > + //
> > > > + // Use 10ms as default MADT when DEVSLP TIMING VARIABLES is not
> > > > supported or the MADT is 0.
> > > > + //
> > > > + if ((DevSlpTiming.Supported == 0) || (DevSlpTiming.Madt == 0)) {
> > > > + DevSlpTiming.Madt = 10;
> > > > + }
> > > > +
> > > > + PortDevSlp |= DevSlpTiming.Deto << 2; PortDevSlp |=
> > > > + DevSlpTiming.Madt << 10; AhciOrReg (PciIo, Offset +
> > > > + AHCI_PORT_DEVSLP, PortDevSlp);
> > > > +
> > > > + if (mAtaAtapiPolicy->AggressiveDeviceSleepEnable == 1) {
> > > > + if ((Capability2 & AHCI_CAP2_SADM) != 0) {
> > > > + PortDevSlp &= ~AHCI_PORT_DEVSLP_DITO_MASK;
> > > > + PortDevSlp |= (625 << 15);
> > > > + AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP,
> > > > + PortDevSlp);
> > > > +
> > > > + PortDevSlp |= AHCI_PORT_DEVSLP_ADSE;
> > > > + AhciWriteReg (PciIo, Offset + AHCI_PORT_DEVSLP, PortDevSlp);
> > > > + }
> > > > + }
> > > > +
> > > > +
> > > > + AhciWriteReg (PciIo, Offset + EFI_AHCI_PORT_CMD, PortCmd);
> > > > +
> > > > + DEBUG ((DEBUG_INFO, "Enabled DevSlp feature at port [%d]
> > > > PortMultiplier [%d], Port CMD/DEVSLP = %08x / %08x\n",
> > > > + Port, PortMultiplier, PortCmd, PortDevSlp));
> > > > +
> > > > + return EFI_SUCCESS;
> > > > +}
> > > >
> > > > /**
> > > > Spin-up disk if IDD was incomplete or PUIS feature is enabled
> > > > @@
> > > > -2688,6 +2895,13 @@ AhciModeInitialization (
> > > > CreateNewDeviceInfo (Instance, Port, 0xFFFF, DeviceType,
> &Buffer);
> > > > if (DeviceType == EfiIdeHarddisk) {
> > > > REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
> > > > (EFI_PERIPHERAL_FIXED_MEDIA | EFI_P_PC_ENABLE));
> > > > + AhciEnableDevSlp (
> > > > + PciIo,
> > > > + AhciRegisters,
> > > > + Port,
> > > > + 0,
> > > > + &Buffer
> > > > + );
> > > > }
> > > >
> > > > //
> > > > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> > > > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> > > > index 809bcc307f..0ef749b4c7 100644
> > > > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> > > > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.h
> > > > @@ -1,7 +1,7 @@
> > > > /** @file
> > > > Header file for AHCI mode of ATA host controller.
> > > >
> > > > - Copyright (c) 2010 - 2014, Intel Corporation. All rights
> > > > reserved.<BR>
> > > > + Copyright (c) 2010 - 2018, Intel Corporation. All rights
> > > > + reserved.<BR>
> > > > This program and the accompanying materials
> > > > are licensed and made available under the terms and conditions
> > > > of the BSD License
> > > > which accompanies this distribution. The full text of the
> > > > license may be found at @@ -29,6 +29,10 @@
> > > >
> > > > #define EFI_AHCI_MAX_PORTS 32
> > > >
> > > > +#define AHCI_CAPABILITY2_OFFSET 0x0024
> > > > +#define AHCI_CAP2_SDS BIT3
> > > > +#define AHCI_CAP2_SADM BIT4
> > > > +
> > > > typedef struct {
> > > > UINT32 Lower32;
> > > > UINT32 Upper32;
> > > > @@ -182,7 +186,13 @@ typedef union {
> > > > #define EFI_AHCI_PORT_SACT 0x0034
> > > > #define EFI_AHCI_PORT_CI 0x0038
> > > > #define EFI_AHCI_PORT_SNTF 0x003C
> > > > -
> > > > +#define AHCI_PORT_DEVSLP 0x0044
> > > > +#define AHCI_PORT_DEVSLP_ADSE BIT0
> > > > +#define AHCI_PORT_DEVSLP_DSP BIT1
> > > > +#define AHCI_PORT_DEVSLP_DETO_MASK 0x000003FC
> > > > +#define AHCI_PORT_DEVSLP_MDAT_MASK 0x00007C00
> > > > +#define AHCI_PORT_DEVSLP_DITO_MASK 0x01FF8000
> > > > +#define AHCI_PORT_DEVSLP_DM_MASK 0x1E000000
> > > >
> > > > #pragma pack(1)
> > > > //
> > > > @@ -283,6 +293,15 @@ typedef struct {
> > > > UINT8 AhciUnknownFisRsvd[0x60];
> > > > } EFI_AHCI_RECEIVED_FIS;
> > > >
> > > > +typedef struct {
> > > > + UINT8 Madt : 5;
> > > > + UINT8 Reserved_5 : 3;
> > > > + UINT8 Deto;
> > > > + UINT16 Reserved_16;
> > > > + UINT32 Reserved_32 : 31;
> > > > + UINT32 Supported : 1;
> > > > +} DEVSLP_TIMING_VARIABLES;
> > > > +
> > > > #pragma pack()
> > > >
> > > > typedef struct {
> > > > --
> > > > 2.16.1.windows.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] MdeModulePkg: Add AtaAtapiPolicy protocol definition
2018-06-04 6:29 ` Zeng, Star
@ 2018-06-04 6:49 ` Ni, Ruiyu
0 siblings, 0 replies; 15+ messages in thread
From: Ni, Ruiyu @ 2018-06-04 6:49 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org
Ok. I will update the comments.
Thanks/Ray
> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, June 4, 2018 2:30 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: RE: [PATCH 2/4] MdeModulePkg: Add AtaAtapiPolicy protocol
> definition
>
> I see below definition for PuisEnable.
> + /// others: Since PUIS setting is non-volatile, platform can use other value
> than 0/1 to keep hardware PUIS setting.
> + ///
> + UINT8 PuisEnable;
>
>
> What will the code do for DeviceSleepEnable and
> AggressiveDeviceSleepEnable when their value is other value than 0/1?
> Could that information be also stated?
>
>
>
> Thanks,
> Star
>
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Friday, June 1, 2018 1:39 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH 2/4] MdeModulePkg: Add AtaAtapiPolicy protocol definition
>
> The patch adds AtaAtapiPolicy protocol which is produced by platform and
> consumed by AtaAtapiPassThruDxe driver.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> ---
> MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h | 59
> ++++++++++++++++++++++++++
> MdeModulePkg/MdeModulePkg.dec | 3 ++
> 2 files changed, 62 insertions(+)
> create mode 100644 MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h
>
> diff --git a/MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h
> b/MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h
> new file mode 100644
> index 0000000000..86a6213dc9
> --- /dev/null
> +++ b/MdeModulePkg/Include/Protocol/AtaAtapiPolicy.h
> @@ -0,0 +1,59 @@
> +/** @file
> + ATA ATAPI Policy protocol is produced by platform and consumed by
> +AtaAtapiPassThruDxe
> + driver.
> +
> + Copyright (c) 2018, Intel Corporation. All rights reserved.<BR> This
> + program and the accompanying materials are licensed and made
> + available under the terms and conditions of the BSD License which
> + accompanies this distribution. The full text of the license may be
> + found at http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +
> +**/
> +#ifndef __ATA_ATAPI_POLICY_H__
> +#define __ATA_ATAPI_POLICY_H__
> +
> +#define EDKII_ATA_ATAPI_POLICY_PROTOCOL_GUID \
> + { \
> + 0xe59cd769, 0x5083, 0x4f26,{ 0x90, 0x94, 0x6c, 0x91, 0x9f, 0x91,
> +0x6c, 0x4e } \
> + }
> +
> +typedef struct {
> + ///
> + /// Protocol version.
> + ///
> + UINT32 Version;
> +
> + ///
> + /// 0: Disable Power-up in Standby;
> + /// 1: Enable Power-up in Standby;
> + /// others: Since PUIS setting is non-volatile, platform can use other value
> than 0/1 to keep hardware PUIS setting.
> + ///
> + UINT8 PuisEnable;
> +
> + ///
> + /// 0: Disable Device Sleep;
> + /// 1: Enable Device Sleep;
> + ///
> + UINT8 DeviceSleepEnable;
> +
> + ///
> + /// 0: Disable Aggressive Device Sleep; /// 1: Enable Aggressive
> + Device Sleep.
> + ///
> + UINT8 AggressiveDeviceSleepEnable;
> +
> + UINT8 Reserved;
> +} EDKII_ATA_ATAPI_POLICY_PROTOCOL;
> +
> +#define EDKII_ATA_ATAPI_POLICY_VERSION 0x00010000
> +
> +
> +extern EFI_GUID gEdkiiAtaAtapiPolicyProtocolGuid;
> +
> +
> +
> +#endif
> +
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec index cc397185f7..3d8376ffdf 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -595,6 +595,9 @@ [Protocols]
> gEdkiiPlatformSpecificResetFilterProtocolGuid = { 0x695d7835, 0x8d47,
> 0x4c11, { 0xab, 0x22, 0xfa, 0x8a, 0xcc, 0xe7, 0xae, 0x7a } }
> ## Include/Protocol/PlatformSpecificResetHandler.h
> gEdkiiPlatformSpecificResetHandlerProtocolGuid = { 0x2df6ba0b, 0x7092,
> 0x440d, { 0xbd, 0x4, 0xfb, 0x9, 0x1e, 0xc3, 0xf3, 0xc1 } }
> +
> + ## Include/Protocol/AtaAtapiPolicy.h
> + gEdkiiAtaAtapiPolicyProtocolGuid = { 0xe59cd769, 0x5083, 0x4f26,{
> + 0x90, 0x94, 0x6c, 0x91, 0x9f, 0x91, 0x6c, 0x4e } }
> #
> # [Error.gEfiMdeModulePkgTokenSpaceGuid]
> # 0x80000001 | Invalid value provided.
> --
> 2.16.1.windows.1
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-06-04 6:49 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-01 5:39 [PATCH 0/4] Support PUIS and DEVSLP feature Ruiyu Ni
2018-06-01 5:39 ` [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices Ruiyu Ni
2018-06-04 3:51 ` Chiu, Chasel
2018-06-04 5:53 ` Wu, Hao A
2018-06-01 5:39 ` [PATCH 2/4] MdeModulePkg: Add AtaAtapiPolicy protocol definition Ruiyu Ni
2018-06-04 6:29 ` Zeng, Star
2018-06-04 6:49 ` Ni, Ruiyu
2018-06-01 5:39 ` [PATCH 3/4] MdeModulePkg/AtaAtapiPassThru: enable/disable PUIS per policy Ruiyu Ni
2018-06-04 3:51 ` Chiu, Chasel
2018-06-01 5:39 ` [PATCH 4/4] MdeModulePkg/Ata/AtaAtapiPassThru: Enable/disable DEVSLP " Ruiyu Ni
2018-06-04 3:51 ` Chiu, Chasel
2018-06-04 6:20 ` Wu, Hao A
2018-06-04 6:36 ` Ni, Ruiyu
2018-06-04 6:45 ` Wu, Hao A
2018-06-04 6:48 ` Ni, Ruiyu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox