Change in coreboot[master]: soc/intel/skylake: acpi: drop HWP's dependency on EIST

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44137 )

Change subject: soc/intel/skylake: acpi: drop HWP's dependency on EIST
......................................................................

Patch Set 3:

Matt,

thanks for sharing your thoughts!
> Patch Set 3:
>
> (1 comment)
>
> The basis of the change (i.e., that the chip features are independent) is valid, though I'm not clear on the benefit/need unless there's some Intel chips that now support HWP but not EIST.  Most of the Intel-based systems that don't set "eist_enable" in their devicetree are probably doing it by accidental oversight, e.g.:
>
> https://review.coreboot.org/c/coreboot/+/29664
>
> particularly since coreboot is most often used by Linux (where the pstate driver typical forces on EIST or HWP) vs Windows (which expects the features to be advertised/enable by the firmware before the OS attempts to use them).  In the past there's been some informal chats before about whether eist_enable should be enabled by default or disappear entirely.  The current code does force an unnecessary relationship between HWP and EIST though it does help to force people to remember to enable EIST.

IMHO we shouldn't force people to do the right thing by establishing technically wrong dependencies, which lead to even more confusion.
> What I describe here is mostly trying to provide some background/content on the situation rather than be an argument against the change.  IMO, the the current way these features are structured in coreboot aren't ideal but addressing them would be a much bigger change that's probably outside the scope of what anyone wants to do right now.