ACPI: D3 states cleanup

There are two ACPI D3 states defined now:
ACPI_STATE_D3 and ACPI_STATE_D3_COLD.

But the uses of these states are not clear/correct in some code. For example, some code refer ACPI_STATE_D3 as D3hot and others refer it as D3cold.

This patch introduces ACPI_STATE_D3_HOT to refer to ACPI D3hot state. And changes ACPI_STATE_D3 to refer to ACPI D3cold state only. Also redefines ACPI_STATE_D3_COLD the same as ACPI_STATE_D3.

Acked-by Rafael J. Wysocki <[email protected]>

Signed-off-by Lin Ming <[email protected]>

---
 drivers/acpi/power.c   |    2 +-
 drivers/acpi/scan.c    |    9 +--------
 drivers/pci/pci-acpi.c |    4 ++--
 include/acpi/actypes.h |    7 ++++---
 4 files changed, 8 insertions(+), 14 deletions(-)

drivers/acpi/power.c +1 -1

index 7049a7d..330bb4d 100644
@@ -631,7 +631,7 @@ int acpi_power_get_inferred_state(struct acpi_device *device, int *state)
 	 * We know a device's inferred power state when all the resources
 	 * required for a given D-state are 'on'.
 	 */
-	for (i = ACPI_STATE_D0; i < ACPI_STATE_D3; i++) {
+	for (i = ACPI_STATE_D0; i < ACPI_STATE_D3_HOT; i++) {
 		list = &device->power.states[i].resources;
 		if (list->count < 1)
 			continue;

drivers/acpi/scan.c +1 -8

index 767e2dc..fbf38ad 100644
@@ -869,7 +869,7 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
 	/*
 	 * Enumerate supported power management states
 	 */
-	for (i = ACPI_STATE_D0; i <= ACPI_STATE_D3; i++) {
+	for (i = ACPI_STATE_D0; i <= ACPI_STATE_D3_HOT; i++) {
 		struct acpi_device_power_state *ps = &device->power.states[i];
 		char object_name[5] = { '_', 'P', 'R', '0' + i, '\0' };
 

@@ -884,13 +884,6 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
 				acpi_bus_add_power_resource(ps->resources.handles[j]);
 		}
 
-		/* The exist of _PR3 indicates D3Cold support */
-		if (i == ACPI_STATE_D3) {
-			status = acpi_get_handle(device->handle, object_name, &handle);
-			if (ACPI_SUCCESS(status))
-				device->power.states[ACPI_STATE_D3_COLD].flags.valid = 1;
-		}
-
 		/* Evaluate "_PSx" to see if we can do explicit sets */
 		object_name[2] = 'S';
 		status = acpi_get_handle(device->handle, object_name, &handle);

drivers/pci/pci-acpi.c +2 -2

index 0f150f2..1929c0c 100644
@@ -200,7 +200,7 @@ static pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
 		return PCI_D1;
 	case ACPI_STATE_D2:
 		return PCI_D2;
-	case ACPI_STATE_D3:
+	case ACPI_STATE_D3_HOT:
 		return PCI_D3hot;
 	case ACPI_STATE_D3_COLD:
 		return PCI_D3cold;

@@ -223,7 +223,7 @@ static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 		[PCI_D0] = ACPI_STATE_D0,
 		[PCI_D1] = ACPI_STATE_D1,
 		[PCI_D2] = ACPI_STATE_D2,
-		[PCI_D3hot] = ACPI_STATE_D3,
+		[PCI_D3hot] = ACPI_STATE_D3_HOT,
 		[PCI_D3cold] = ACPI_STATE_D3
 	};
 	int error = -EINVAL;

include/acpi/actypes.h +4 -3

index eba6604..e8bcc47 100644
@@ -499,9 +499,10 @@ typedef u64 acpi_integer;
 #define ACPI_STATE_D0                   (u8) 0
 #define ACPI_STATE_D1                   (u8) 1
 #define ACPI_STATE_D2                   (u8) 2
-#define ACPI_STATE_D3                   (u8) 3
-#define ACPI_STATE_D3_COLD              (u8) 4
-#define ACPI_D_STATES_MAX               ACPI_STATE_D3_COLD
+#define ACPI_STATE_D3_HOT               (u8) 3
+#define ACPI_STATE_D3                   (u8) 4
+#define ACPI_STATE_D3_COLD              ACPI_STATE_D3
+#define ACPI_D_STATES_MAX               ACPI_STATE_D3
 #define ACPI_D_STATE_COUNT              5
 
 #define ACPI_STATE_C0                   (u8) 0

Signature · 1 line

1.7.2.5

Hi,

Quoted 11 lines · Lin Ming

There are two ACPI D3 states defined now:
ACPI_STATE_D3 and ACPI_STATE_D3_COLD.

But the uses of these states are not clear/correct in some code. For example, some code refer ACPI_STATE_D3 as D3hot and others refer it as D3cold.

This patch introduces ACPI_STATE_D3_HOT to refer to ACPI D3hot state. And changes ACPI_STATE_D3 to refer to ACPI D3cold state only. Also redefines ACPI_STATE_D3_COLD the same as ACPI_STATE_D3.

With this patch now, if a device has _PS3, then we will set its D3hot flag valid. This doesn't feel right to me, since per our discussion the other day, we should assume _PS3 will put the device into D3cold.

Or do you mean: if _PS3 is available, then both D3hot and D3cold is valid from the perspective of acpi, it is the individual driver's responsibility to decide which state is actually valid and will be used.

-Aaron

Quoted 95 lines

Acked-by Rafael J. Wysocki <[email protected]>

Signed-off-by Lin Ming <[email protected]>

---
 drivers/acpi/power.c   |    2 +-
 drivers/acpi/scan.c    |    9 +--------
 drivers/pci/pci-acpi.c |    4 ++--
 include/acpi/actypes.h |    7 ++++---
 4 files changed, 8 insertions(+), 14 deletions(-)

drivers/acpi/power.c +1 -1

index 7049a7d..330bb4d 100644
@@ -631,7 +631,7 @@ int acpi_power_get_inferred_state(struct acpi_device *device, int *state)
 	 * We know a device's inferred power state when all the resources
 	 * required for a given D-state are 'on'.
 	 */
-	for (i = ACPI_STATE_D0; i < ACPI_STATE_D3; i++) {
+	for (i = ACPI_STATE_D0; i < ACPI_STATE_D3_HOT; i++) {
 		list = &device->power.states[i].resources;
 		if (list->count < 1)
 			continue;

drivers/acpi/scan.c +1 -8

index 767e2dc..fbf38ad 100644
@@ -869,7 +869,7 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
 	/*
 	 * Enumerate supported power management states
 	 */
-	for (i = ACPI_STATE_D0; i <= ACPI_STATE_D3; i++) {
+	for (i = ACPI_STATE_D0; i <= ACPI_STATE_D3_HOT; i++) {
 		struct acpi_device_power_state *ps = &device->power.states[i];
 		char object_name[5] = { '_', 'P', 'R', '0' + i, '\0' };
 

@@ -884,13 +884,6 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
 				acpi_bus_add_power_resource(ps->resources.handles[j]);
 		}
 
-		/* The exist of _PR3 indicates D3Cold support */
-		if (i == ACPI_STATE_D3) {
-			status = acpi_get_handle(device->handle, object_name, &handle);
-			if (ACPI_SUCCESS(status))
-				device->power.states[ACPI_STATE_D3_COLD].flags.valid = 1;
-		}
-
 		/* Evaluate "_PSx" to see if we can do explicit sets */
 		object_name[2] = 'S';
 		status = acpi_get_handle(device->handle, object_name, &handle);

drivers/pci/pci-acpi.c +2 -2

index 0f150f2..1929c0c 100644
@@ -200,7 +200,7 @@ static pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
 		return PCI_D1;
 	case ACPI_STATE_D2:
 		return PCI_D2;
-	case ACPI_STATE_D3:
+	case ACPI_STATE_D3_HOT:
 		return PCI_D3hot;
 	case ACPI_STATE_D3_COLD:
 		return PCI_D3cold;

@@ -223,7 +223,7 @@ static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 		[PCI_D0] = ACPI_STATE_D0,
 		[PCI_D1] = ACPI_STATE_D1,
 		[PCI_D2] = ACPI_STATE_D2,
-		[PCI_D3hot] = ACPI_STATE_D3,
+		[PCI_D3hot] = ACPI_STATE_D3_HOT,
 		[PCI_D3cold] = ACPI_STATE_D3
 	};
 	int error = -EINVAL;

include/acpi/actypes.h +4 -3

index eba6604..e8bcc47 100644
@@ -499,9 +499,10 @@ typedef u64 acpi_integer;
 #define ACPI_STATE_D0                   (u8) 0
 #define ACPI_STATE_D1                   (u8) 1
 #define ACPI_STATE_D2                   (u8) 2
-#define ACPI_STATE_D3                   (u8) 3
-#define ACPI_STATE_D3_COLD              (u8) 4
-#define ACPI_D_STATES_MAX               ACPI_STATE_D3_COLD
+#define ACPI_STATE_D3_HOT               (u8) 3
+#define ACPI_STATE_D3                   (u8) 4
+#define ACPI_STATE_D3_COLD              ACPI_STATE_D3
+#define ACPI_D_STATES_MAX               ACPI_STATE_D3
 #define ACPI_D_STATE_COUNT              5
 
 #define ACPI_STATE_C0                   (u8) 0

Signature · 1 line

1.7.2.5

Quoted 19 lines · Aaron Lu

Hi,

Quoted 11 lines · Lin Ming

There are two ACPI D3 states defined now:
ACPI_STATE_D3 and ACPI_STATE_D3_COLD.

But the uses of these states are not clear/correct in some code. For example, some code refer ACPI_STATE_D3 as D3hot and others refer it as D3cold.

This patch introduces ACPI_STATE_D3_HOT to refer to ACPI D3hot state. And changes ACPI_STATE_D3 to refer to ACPI D3cold state only. Also redefines ACPI_STATE_D3_COLD the same as ACPI_STATE_D3.

With this patch now, if a device has _PS3, then we will set its D3hot flag valid. This doesn't feel right to me, since per our discussion the other day, we should assume _PS3 will put the device into D3cold.

Quoted 3 lines

Or do you mean: if _PS3 is available, then both D3hot and D3cold is valid from the perspective of acpi, it is the individual driver's responsibility to decide which state is actually valid and will be used.

Right.

ACPI_STATE_D3(same as ACPI_STATE_D3_COLD now) is always valid.

Lin Ming

Quoted 91 lines

-Aaron

Quoted 88 lines

Acked-by Rafael J. Wysocki <[email protected]>

Signed-off-by Lin Ming <[email protected]>

---
 drivers/acpi/power.c   |    2 +-
 drivers/acpi/scan.c    |    9 +--------
 drivers/pci/pci-acpi.c |    4 ++--
 include/acpi/actypes.h |    7 ++++---
 4 files changed, 8 insertions(+), 14 deletions(-)

drivers/acpi/power.c +1 -1

index 7049a7d..330bb4d 100644
@@ -631,7 +631,7 @@ int acpi_power_get_inferred_state(struct acpi_device *device, int *state)
 	 * We know a device's inferred power state when all the resources
 	 * required for a given D-state are 'on'.
 	 */
-	for (i = ACPI_STATE_D0; i < ACPI_STATE_D3; i++) {
+	for (i = ACPI_STATE_D0; i < ACPI_STATE_D3_HOT; i++) {
 		list = &device->power.states[i].resources;
 		if (list->count < 1)
 			continue;

drivers/acpi/scan.c +1 -8

index 767e2dc..fbf38ad 100644
@@ -869,7 +869,7 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
 	/*
 	 * Enumerate supported power management states
 	 */
-	for (i = ACPI_STATE_D0; i <= ACPI_STATE_D3; i++) {
+	for (i = ACPI_STATE_D0; i <= ACPI_STATE_D3_HOT; i++) {
 		struct acpi_device_power_state *ps = &device->power.states[i];
 		char object_name[5] = { '_', 'P', 'R', '0' + i, '\0' };
 

@@ -884,13 +884,6 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
 				acpi_bus_add_power_resource(ps->resources.handles[j]);
 		}
 
-		/* The exist of _PR3 indicates D3Cold support */
-		if (i == ACPI_STATE_D3) {
-			status = acpi_get_handle(device->handle, object_name, &handle);
-			if (ACPI_SUCCESS(status))
-				device->power.states[ACPI_STATE_D3_COLD].flags.valid = 1;
-		}
-
 		/* Evaluate "_PSx" to see if we can do explicit sets */
 		object_name[2] = 'S';
 		status = acpi_get_handle(device->handle, object_name, &handle);

drivers/pci/pci-acpi.c +2 -2

index 0f150f2..1929c0c 100644
@@ -200,7 +200,7 @@ static pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
 		return PCI_D1;
 	case ACPI_STATE_D2:
 		return PCI_D2;
-	case ACPI_STATE_D3:
+	case ACPI_STATE_D3_HOT:
 		return PCI_D3hot;
 	case ACPI_STATE_D3_COLD:
 		return PCI_D3cold;

@@ -223,7 +223,7 @@ static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 		[PCI_D0] = ACPI_STATE_D0,
 		[PCI_D1] = ACPI_STATE_D1,
 		[PCI_D2] = ACPI_STATE_D2,
-		[PCI_D3hot] = ACPI_STATE_D3,
+		[PCI_D3hot] = ACPI_STATE_D3_HOT,
 		[PCI_D3cold] = ACPI_STATE_D3
 	};
 	int error = -EINVAL;

include/acpi/actypes.h +4 -3

index eba6604..e8bcc47 100644
@@ -499,9 +499,10 @@ typedef u64 acpi_integer;
 #define ACPI_STATE_D0                   (u8) 0
 #define ACPI_STATE_D1                   (u8) 1
 #define ACPI_STATE_D2                   (u8) 2
-#define ACPI_STATE_D3                   (u8) 3
-#define ACPI_STATE_D3_COLD              (u8) 4
-#define ACPI_D_STATES_MAX               ACPI_STATE_D3_COLD
+#define ACPI_STATE_D3_HOT               (u8) 3
+#define ACPI_STATE_D3                   (u8) 4
+#define ACPI_STATE_D3_COLD              ACPI_STATE_D3
+#define ACPI_D_STATES_MAX               ACPI_STATE_D3
 #define ACPI_D_STATE_COUNT              5
 

Quoted 8 lines

Quoted 7 lines

include/acpi/actypes.h +0 -0

 #define ACPI_STATE_C0                   (u8) 0

Signature · 1 line

1.7.2.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 24 lines · Lin Ming

Quoted 22 lines · Aaron Lu

Hi,

Quoted 11 lines · Lin Ming

There are two ACPI D3 states defined now:
ACPI_STATE_D3 and ACPI_STATE_D3_COLD.

But the uses of these states are not clear/correct in some code. For example, some code refer ACPI_STATE_D3 as D3hot and others refer it as D3cold.

This patch introduces ACPI_STATE_D3_HOT to refer to ACPI D3hot state. And changes ACPI_STATE_D3 to refer to ACPI D3cold state only. Also redefines ACPI_STATE_D3_COLD the same as ACPI_STATE_D3.

With this patch now, if a device has _PS3, then we will set its D3hot flag valid. This doesn't feel right to me, since per our discussion the other day, we should assume _PS3 will put the device into D3cold.

Or do you mean: if _PS3 is available, then both D3hot and D3cold is valid from the perspective of acpi, it is the individual driver's responsibility to decide which state is actually valid and will be used.

Quoted 4 lines

Right.

ACPI_STATE_D3(same as ACPI_STATE_D3_COLD now) is always valid.

I mean, if _PS3 is available, can we say D3hot is valid?

Quoted 20 lines

Quoted 20 lines

Quoted 19 lines

Quoted 11 lines

There are two ACPI D3 states defined now:
ACPI_STATE_D3 and ACPI_STATE_D3_COLD.

But the uses of these states are not clear/correct in some code. For example, some code refer ACPI_STATE_D3 as D3hot and others refer it as D3cold.

This patch introduces ACPI_STATE_D3_HOT to refer to ACPI D3hot state. And changes ACPI_STATE_D3 to refer to ACPI D3cold state only. Also redefines ACPI_STATE_D3_COLD the same as ACPI_STATE_D3.

With this patch now, if a device has _PS3, then we will set its D3hot flag valid. This doesn't feel right to me, since per our discussion the other day, we should assume _PS3 will put the device into D3cold.

Or do you mean: if _PS3 is available, then both D3hot and D3cold is valid from the perspective of acpi, it is the individual driver's responsibility to decide which state is actually valid and will be used.

Quoted 6 lines

Quoted 4 lines

Right.

ACPI_STATE_D3(same as ACPI_STATE_D3_COLD now) is always valid.

I mean, if _PS3 is available, can we say D3hot is valid?

Yes.

Hi,

Quoted 20 lines · Lin Ming

Quoted 20 lines

Quoted 20 lines

Quoted 19 lines

Quoted 11 lines

There are two ACPI D3 states defined now:
ACPI_STATE_D3 and ACPI_STATE_D3_COLD.

But the uses of these states are not clear/correct in some code. For example, some code refer ACPI_STATE_D3 as D3hot and others refer it as D3cold.

This patch introduces ACPI_STATE_D3_HOT to refer to ACPI D3hot state. And changes ACPI_STATE_D3 to refer to ACPI D3cold state only. Also redefines ACPI_STATE_D3_COLD the same as ACPI_STATE_D3.

With this patch now, if a device has _PS3, then we will set its D3hot flag valid. This doesn't feel right to me, since per our discussion the other day, we should assume _PS3 will put the device into D3cold.

Or do you mean: if _PS3 is available, then both D3hot and D3cold is valid from the perspective of acpi, it is the individual driver's responsibility to decide which state is actually valid and will be used.

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

Right.

ACPI_STATE_D3(same as ACPI_STATE_D3_COLD now) is always valid.

I mean, if _PS3 is available, can we say D3hot is valid?

Yes.

OK, now I'm confused...

First, let me try to clarify the meaning of acpi power state's valid flag.

By valid, I suppose it means the device can be in that state, instead of we have a way to program this device to go into that state.

e.g. D0 is valid means the device can be in D0 state, and D3_cold is valid means the device can be in D3_cold state. We unconditionally set these two states as valid, because we know every device supports these two states. But we might not be able to put the device into that state in software, since we might not have _PS0 or _PS3 control methods for it.

And if we do have the _PSx or _PRx control methods, we knows we have a way to put the device into that state, and hence the device should be able to support that power state, so we will set that state as valid too.

Is this correct?

For D3hot, obviously not all device supports this state, so we will need to figure it out through the acpi table.
I remembered Rafael said the following words the other day in a reply to my evaluate_ps3_when_entering_d3_cold_patch:

-----------------------------------------------------------------------
I'd rather say that with _PR3 we have the opportunity to avoid removing power completely from the device. In other words, D3_hot is supported (and it is supported _only_ in that case).
-----------------------------------------------------------------------

So I think here is a problem, that if a device has only _PS3, why should we say D3hot is supported? Is there a reason for this that I missed?

Quoted 58 lines · Aaron Lu

Hi,

Quoted 20 lines · Lin Ming

Quoted 20 lines

Quoted 20 lines

Quoted 19 lines

Quoted 11 lines

There are two ACPI D3 states defined now:
ACPI_STATE_D3 and ACPI_STATE_D3_COLD.

But the uses of these states are not clear/correct in some code. For example, some code refer ACPI_STATE_D3 as D3hot and others refer it as D3cold.

This patch introduces ACPI_STATE_D3_HOT to refer to ACPI D3hot state. And changes ACPI_STATE_D3 to refer to ACPI D3cold state only. Also redefines ACPI_STATE_D3_COLD the same as ACPI_STATE_D3.

With this patch now, if a device has _PS3, then we will set its D3hot flag valid. This doesn't feel right to me, since per our discussion the other day, we should assume _PS3 will put the device into D3cold.

Or do you mean: if _PS3 is available, then both D3hot and D3cold is valid from the perspective of acpi, it is the individual driver's responsibility to decide which state is actually valid and will be used.

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

Right.

ACPI_STATE_D3(same as ACPI_STATE_D3_COLD now) is always valid.

I mean, if _PS3 is available, can we say D3hot is valid?

Yes.

OK, now I'm confused...

First, let me try to clarify the meaning of acpi power state's valid flag.

By valid, I suppose it means the device can be in that state, instead of we have a way to program this device to go into that state.

e.g. D0 is valid means the device can be in D0 state, and D3_cold is valid means the device can be in D3_cold state. We unconditionally set these two states as valid, because we know every device supports these two states. But we might not be able to put the device into that state in software, since we might not have _PS0 or _PS3 control methods for it.

And if we do have the _PSx or _PRx control methods, we knows we have a way to put the device into that state, and hence the device should be able to support that power state, so we will set that state as valid too.

Is this correct?

For D3hot, obviously not all device supports this state, so we will need to figure it out through the acpi table.
I remembered Rafael said the following words the other day in a reply to my evaluate_ps3_when_entering_d3_cold_patch:

Quoted 8 lines

-----------------------------------------------------------------------
I'd rather say that with _PR3 we have the opportunity to avoid removing power completely from the device.  In other words, D3_hot is supported (and it is supported _only_ in that case).
-----------------------------------------------------------------------

So I think here is a problem, that if a device has only _PS3, why should we say D3hot is supported? Is there a reason for this that I missed?

OK, I agree.  We need to special case the situation in which _PR3 is not present, but _PS3 is.  IOW, we should do something like this in the loop in acpi_bus_get_power_flags():

               /* State is valid if we have some power control */ if (ps->resources.count
                   || (ps->flags.explicit_set && i < ACPI_STATE_D3)) ps->flags.valid = 1;

Thanks,
Rafael

Quoted 73 lines · Rafael J. Wysocki

Quoted 58 lines · Aaron Lu

Hi,

Quoted 20 lines · Lin Ming

Quoted 20 lines

Quoted 20 lines

Quoted 19 lines

Quoted 11 lines

There are two ACPI D3 states defined now:
ACPI_STATE_D3 and ACPI_STATE_D3_COLD.

But the uses of these states are not clear/correct in some code. For example, some code refer ACPI_STATE_D3 as D3hot and others refer it as D3cold.

This patch introduces ACPI_STATE_D3_HOT to refer to ACPI D3hot state. And changes ACPI_STATE_D3 to refer to ACPI D3cold state only. Also redefines ACPI_STATE_D3_COLD the same as ACPI_STATE_D3.

With this patch now, if a device has _PS3, then we will set its D3hot flag valid. This doesn't feel right to me, since per our discussion the other day, we should assume _PS3 will put the device into D3cold.

Or do you mean: if _PS3 is available, then both D3hot and D3cold is valid from the perspective of acpi, it is the individual driver's responsibility to decide which state is actually valid and will be used.

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

Right.

ACPI_STATE_D3(same as ACPI_STATE_D3_COLD now) is always valid.

I mean, if _PS3 is available, can we say D3hot is valid?

Yes.

OK, now I'm confused...

First, let me try to clarify the meaning of acpi power state's valid flag.

By valid, I suppose it means the device can be in that state, instead of we have a way to program this device to go into that state.

e.g. D0 is valid means the device can be in D0 state, and D3_cold is valid means the device can be in D3_cold state. We unconditionally set these two states as valid, because we know every device supports these two states. But we might not be able to put the device into that state in software, since we might not have _PS0 or _PS3 control methods for it.

And if we do have the _PSx or _PRx control methods, we knows we have a way to put the device into that state, and hence the device should be able to support that power state, so we will set that state as valid too.

Is this correct?

For D3hot, obviously not all device supports this state, so we will need to figure it out through the acpi table.
I remembered Rafael said the following words the other day in a reply to my evaluate_ps3_when_entering_d3_cold_patch:

Quoted 8 lines

-----------------------------------------------------------------------
I'd rather say that with _PR3 we have the opportunity to avoid removing power completely from the device. In other words, D3_hot is supported (and it is supported _only_ in that case).
-----------------------------------------------------------------------

So I think here is a problem, that if a device has only _PS3, why should we say D3hot is supported? Is there a reason for this that I missed?

OK, I agree. We need to special case the situation in which _PR3 is not present, but _PS3 is. IOW, we should do something like this in the loop in acpi_bus_get_power_flags():

Quoted 4 lines

               /* State is valid if we have some power control */
               if (ps->resources.count
                   || (ps->flags.explicit_set && i < ACPI_STATE_D3))
                       ps->flags.valid = 1;

Will add below change.

(quoted diff) +2 -0

+               if (ps->resources.count ||
+                   (ps->flags.explicit_set && i < ACPI_STATE_D3_HOT))
                        ps->flags.valid = 1;

Rafael,

can I still add your ACK when send updated patch?

Quoted 58 lines · Aaron Lu

Hi,

Quoted 20 lines · Lin Ming

Quoted 20 lines

Quoted 20 lines

Quoted 19 lines

Quoted 11 lines

There are two ACPI D3 states defined now:
ACPI_STATE_D3 and ACPI_STATE_D3_COLD.

But the uses of these states are not clear/correct in some code. For example, some code refer ACPI_STATE_D3 as D3hot and others refer it as D3cold.

This patch introduces ACPI_STATE_D3_HOT to refer to ACPI D3hot state. And changes ACPI_STATE_D3 to refer to ACPI D3cold state only. Also redefines ACPI_STATE_D3_COLD the same as ACPI_STATE_D3.

With this patch now, if a device has _PS3, then we will set its D3hot flag valid. This doesn't feel right to me, since per our discussion the other day, we should assume _PS3 will put the device into D3cold.

Or do you mean: if _PS3 is available, then both D3hot and D3cold is valid from the perspective of acpi, it is the individual driver's responsibility to decide which state is actually valid and will be used.

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

Right.

ACPI_STATE_D3(same as ACPI_STATE_D3_COLD now) is always valid.

I mean, if _PS3 is available, can we say D3hot is valid?

Yes.

OK, now I'm confused...

First, let me try to clarify the meaning of acpi power state's valid flag.

By valid, I suppose it means the device can be in that state, instead of we have a way to program this device to go into that state.

e.g. D0 is valid means the device can be in D0 state, and D3_cold is valid means the device can be in D3_cold state. We unconditionally set these two states as valid, because we know every device supports these two states. But we might not be able to put the device into that state in software, since we might not have _PS0 or _PS3 control methods for it.

And if we do have the _PSx or _PRx control methods, we knows we have a way to put the device into that state, and hence the device should be able to support that power state, so we will set that state as valid too.

Is this correct?

For D3hot, obviously not all device supports this state, so we will need to figure it out through the acpi table.
I remembered Rafael said the following words the other day in a reply to my evaluate_ps3_when_entering_d3_cold_patch:

Quoted 8 lines

-----------------------------------------------------------------------
I'd rather say that with _PR3 we have the opportunity to avoid removing power completely from the device. In other words, D3_hot is supported (and it is supported _only_ in that case).
-----------------------------------------------------------------------

So I think here is a problem, that if a device has only _PS3, why should we say D3hot is supported? Is there a reason for this that I missed?

OK, I agree. We need to special case the situation in which _PR3 is not present, but _PS3 is. IOW, we should do something like this in the loop in acpi_bus_get_power_flags():

       /* State is valid if we have some power control */
       if (ps->resources.count
           || (ps->flags.explicit_set && i < ACPI_STATE_D3))
           ps->flags.valid = 1;