| Message ID | 1470874646-70570-6-git-send-email-srinivas.pandruvada@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Srinivas, On Wed, Aug 10, 2016 at 05:17:26PM -0700, Srinivas Pandruvada wrote: > Need to set platform wide _OSC bits CPC version 1 and version 2 bits > so that BIOS presents CPPC objects. > Even though the cppc_acpi supports only version 2, need to set both > _OSC bits for version 1 and version 2, otherwise BIOS ignore _OSC > settings. Does such behaviour go against ACPI specs? If yes, it's better to add comment in the code. > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > --- > drivers/acpi/bus.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index 85b7d07..61643a5 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -330,6 +330,13 @@ static void acpi_bus_osc_support(void) > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT; > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT; > > +#ifdef CONFIG_X86 > + if (boot_cpu_has(X86_FEATURE_HWP)) { > + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT; > + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT; > + } > +#endif Just to check if I understand things correctly. I thought that OSC method is kind of 'handshake'. OS describes features that it supports and then platform indicates back what features it can provide/support. Here in this patch I see only first part and I don't see where you check confirmation from platform that it supports/enabled CPPC. You don't need that, do you? It will be nice to see explanation. Best regards, Alexey. > if (!ghes_disable) > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT; > if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle))) > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Alexey, On Tue, 2016-08-16 at 15:32 +0100, Alexey Klimov wrote: > Hi Srinivas, > > On Wed, Aug 10, 2016 at 05:17:26PM -0700, Srinivas Pandruvada wrote: > > > > Need to set platform wide _OSC bits CPC version 1 and version 2 > > bits > > so that BIOS presents CPPC objects. > > Even though the cppc_acpi supports only version 2, need to set both > > _OSC bits for version 1 and version 2, otherwise BIOS ignore _OSC > > settings. > Does such behaviour go against ACPI specs? > If yes, it's better to add comment in the code. I need to fix the commit description here. The bit 5 is generic CPPC support and bit 6 is CPPC_V2 support capability. So if we set only bit 5 then we can support only CPPC v1. If we set bit 6, we also support CPPC v2. So we have to set both bits for CPPC v2 (Table 6-176 Platform- Wide _OSC Capabilities DWORD 2 ACPI 6.0 spec). > > > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel > > .com> > > --- > > drivers/acpi/bus.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > > index 85b7d07..61643a5 100644 > > --- a/drivers/acpi/bus.c > > +++ b/drivers/acpi/bus.c > > @@ -330,6 +330,13 @@ static void acpi_bus_osc_support(void) > > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT; > > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT; > > > > +#ifdef CONFIG_X86 > > + if (boot_cpu_has(X86_FEATURE_HWP)) { > > + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT; > > + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT; > > + } > > +#endif > Just to check if I understand things correctly. > I thought that OSC method is kind of 'handshake'. OS describes > features > that it supports and then platform indicates back what features it > can > provide/support. > Here in this patch I see only first part and I don't see where you > check > confirmation from platform that it supports/enabled CPPC. > You don't need that, do you? Once we set these bits the BIOS will expose the CPPC tables. This will result in acpi_cppc_processor_probe will be successful. Also _OSC response will set bit 5 and 6 here to confirm the acceptance, but I can't do any meaningful processing with that confirmation till acpi_cppc_processor_probe() returns successfully, which is the confirmation. How the CPPC data is used in x86 as I indicated in the cover page will be a separate series. I submitted CPPC ACPI changes before so that they can be reviewed first. Thanks, Srinivas > It will be nice to see explanation. > > Best regards, > Alexey. > > > > > if (!ghes_disable) > > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT; > > if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle))) -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 85b7d07..61643a5 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -330,6 +330,13 @@ static void acpi_bus_osc_support(void) capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT; capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT; +#ifdef CONFIG_X86 + if (boot_cpu_has(X86_FEATURE_HWP)) { + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT; + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT; + } +#endif + if (!ghes_disable) capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT; if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
Need to set platform wide _OSC bits CPC version 1 and version 2 bits so that BIOS presents CPPC objects. Even though the cppc_acpi supports only version 2, need to set both _OSC bits for version 1 and version 2, otherwise BIOS ignore _OSC settings. Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> --- drivers/acpi/bus.c | 7 +++++++ 1 file changed, 7 insertions(+)