acpi_powerres: Fix turning off power resources on first D-state switch

[obiwac]

Summary

The power resource dependencies for each _PRx object are discovered and cached in ac_prx on the power consumer struct (struct acpi_powerconsumer) when a power consumer is registered. This is done in acpi_pwr_get_power_resources. ACPI guarantees these _PRx objects will evaluate to the same thing each time they are called.

This discovery process also registers those power resources, which were previously only registered when they were referenced by the relevant _PRx object for the target D-state when switching. This meant that the first D-state switch for a power consumer would not turn off any power resources as they wouldn't have been registered yet. This revision fixes this.

ac_prx will be used by subsequent patches.

This was split out from D48294.

Sponsored by: The FreeBSD Foundation

Test Plan

I have tested this on the Framework 13 AMD Ryzen 7040 series. All devices are able to suspend/resume fine.

I don't know if this will make a significant difference, but since devices will now go into D3cold (the power resources under _PR3 wouldn't be turned off previously, which is needed for D3cold), this might make suspended devices use less power (devctl suspend/resume <device>).

[imp]

sys/dev/acpica/acpi_powerres.c

275

reslist_object leaked

279

Style: 4 space indent, not open paren indent, here and elsewhere.

336

You're ignoring errors here. Why have them returned?
And when you can't allocate memory or whatever, what's the right evasive action?

[obiwac]

Fix memory management.

[obiwac]

Thanks for the review @imp !

Have made the necessary changes, will now play around with acpi_pwr_deregister_consumer to see if it works correctly (as it was unused before).

sys/dev/acpica/acpi_powerres.c

275

thanks :)

279

elsewhere in the file uses this style. Should I make this 4 spaces anyways?

336

now deregistering power consumer on error

[obiwac]

okay, acpi_pwr_deregister_consumer seem to work as expected

[obiwac]

sys/dev/acpica/acpi_powerres.c

259–261

I guess this is technically redundant as we're allocating this memory with M_ZERO now. Should I keep this explicit anyway?

[markj]

sys/dev/acpica/acpi_powerres.c

259–261

I think having the explicit initialization is useful for readability.

287

Shouldn't this be allocating an array of size pc->ac_prx[state].prx_count? You can use mallocarray() here.

360

Can we remove this XXX comment?

[obiwac]

Fix bad malloc size

[obiwac]

thanks for the review!

sys/dev/acpica/acpi_powerres.c

287

absolutely, thanks for catching that

I'm not sure how to handle the styling of the wrapping here since aligning the next line to the paren (like in the rest of this file) crosses 80 chars, so I've just done 4 spaces.

360

We're not yet doing this once the consumer has been powered off (only in acpi_pwr_register_consumer when acpi_pwr_get_power_resources fails).

I haven't looked into calling this anywhere else yet.