| Message ID | 20210723202157.2425-2-michael.a.bottini@linux.intel.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | [1/2] acpi: Add acpi_init_properties to ACPI driver code | expand |
On Sat, 2021-07-24 at 11:29 +0300, Andy Shevchenko wrote: > > > On Friday, July 23, 2021, Michael Bottini > <michael.a.bottini@linux.intel.com> wrote: > > Tiger Lake devices have the capability to track the duration > > of time that their Power Supply Units (PSUs) are turned off during > > S0ix. > > This patch adds a debugfs file `pson_residency_usec` to provide > > access to this counter. > > > > In order to determine whether the device is capable of PSON, > > use acpi_init_properties() to reevaluate _DSD. > > > > > > It’s direct abuse of ACPI specification as I read it: > > “_DSD must return the same data each time it is evaluated. Firmware > should not expect it to be evaluated every time (in case it is > implemented as a method).” > > > NAK to the series. Okay, we'll check with the BIOS folks. They are setting this property from _STA. They may not expect OSPM to reevaluate _DSD. But they may have expected that OSPM doesn't attempt to read _DSD until after _STA is executed. David > > > > > > > Signed-off-by: Michael Bottini <michael.a.bottini@linux.intel.com> > > --- > > drivers/platform/x86/intel/pmc/core.c | 46 > > +++++++++++++++++++++++++++ > > drivers/platform/x86/intel/pmc/core.h | 7 ++++ > > 2 files changed, 53 insertions(+) > > > > diff --git a/drivers/platform/x86/intel/pmc/core.c > > b/drivers/platform/x86/intel/pmc/core.c > > index 7c4bf7d22fd5..6cf06aecf368 100644 > > --- a/drivers/platform/x86/intel/pmc/core.c > > +++ b/drivers/platform/x86/intel/pmc/core.c > > @@ -595,6 +595,8 @@ static const struct pmc_reg_map tgl_reg_map = { > > .lpm_sts = tgl_lpm_maps, > > .lpm_status_offset = TGL_LPM_STATUS_OFFSET, > > .lpm_live_status_offset = TGL_LPM_LIVE_STATUS_OFFSET, > > + .pson_residency_offset = TGL_PSON_RESIDENCY_OFFSET, > > + .pson_residency_counter_step = TGL_PSON_RES_COUNTER_STEP, > > .etr3_offset = ETR3_OFFSET, > > }; > > > > @@ -1084,6 +1086,20 @@ static int pmc_core_dev_state_get(void > > *data, > > u64 *val) > > > > DEFINE_DEBUGFS_ATTRIBUTE(pmc_core_dev_state, > > pmc_core_dev_state_get, > > NULL, "%llu\n"); > > > > +static int pmc_core_pson_residency_get(void *data, u64 *val) > > +{ > > + struct pmc_dev *pmcdev = data; > > + const struct pmc_reg_map *map = pmcdev->map; > > + u32 value; > > + > > + value = pmc_core_reg_read(pmcdev, map- > > > pson_residency_offset); > > + *val = (u64)value * pmcdev->map- > > >pson_residency_counter_step; > > + > > + return 0; > > +} > > + > > +DEFINE_DEBUGFS_ATTRIBUTE(pmc_core_pson_residency, > > pmc_core_pson_residency_get, NULL, "%llu\n"); > > + > > static int pmc_core_check_read_lock_bit(struct pmc_dev *pmcdev) > > { > > u32 value; > > @@ -1788,6 +1804,30 @@ static void > > pmc_core_get_low_power_modes(struct pmc_dev *pmcdev) > > } > > } > > > > +static bool pmc_core_is_pson_residency_enabled(struct pmc_dev > > *pmcdev) > > +{ > > + struct platform_device *pdev = pmcdev->pdev; > > + struct acpi_device *adev = ACPI_COMPANION(&pdev->dev); > > + acpi_status status; > > + u8 val; > > + > > + if (!adev) > > + return false; > > + > > + acpi_init_properties(adev); > > + status = acpi_evaluate_object(adev->handle, "PSOP", NULL, > > NULL); > > + > > + if (ACPI_FAILURE(status)) > > + return false; > > + > > + if (fwnode_property_read_u8(acpi_fwnode_handle(adev), > > + "intel-cec-pson-switching- > > enabled-in-s0", > > + &val)) > > + return false; > > + > > + return val == 1; > > +} > > + > > static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) > > { > > debugfs_remove_recursive(pmcdev->dbgfs_dir); > > @@ -1856,6 +1896,11 @@ static void pmc_core_dbgfs_register(struct > > pmc_dev *pmcdev) > > pmcdev->dbgfs_dir, pmcdev, > > > > &pmc_core_substate_req_regs_fops); > > } > > + > > + if (pmcdev->map->pson_residency_offset && > > pmc_core_is_pson_residency_enabled(pmcdev)) { > > + debugfs_create_file("pson_residency_usec", 0444, > > + pmcdev->dbgfs_dir, pmcdev, > > &pmc_core_pson_residency); > > + } > > } > > > > static const struct x86_cpu_id intel_pmc_core_ids[] = { > > @@ -1944,6 +1989,7 @@ static int pmc_core_probe(struct > > platform_device *pdev) > > return -ENOMEM; > > > > platform_set_drvdata(pdev, pmcdev); > > + pmcdev->pdev = pdev; > > > > cpu_id = x86_match_cpu(intel_pmc_core_ids); > > if (!cpu_id) > > diff --git a/drivers/platform/x86/intel/pmc/core.h > > b/drivers/platform/x86/intel/pmc/core.h > > index 333e25981e8e..822d77f49861 100644 > > --- a/drivers/platform/x86/intel/pmc/core.h > > +++ b/drivers/platform/x86/intel/pmc/core.h > > @@ -214,6 +214,10 @@ enum ppfear_regs { > > #define TGL_LPM_PRI_OFFSET 0x1C7C > > #define TGL_LPM_NUM_MAPS 6 > > > > +/* Tigerlake PSON residency register */ > > +#define TGL_PSON_RESIDENCY_OFFSET 0x18f8 > > +#define TGL_PSON_RES_COUNTER_STEP 0x7A > > + > > /* Extended Test Mode Register 3 (CNL and later) */ > > #define ETR3_OFFSET 0x1048 > > #define ETR3_CF9GR BIT(20) > > @@ -301,6 +305,8 @@ struct pmc_reg_map { > > const u32 lpm_residency_offset; > > const u32 lpm_status_offset; > > const u32 lpm_live_status_offset; > > + const u32 pson_residency_offset; > > + const u32 pson_residency_counter_step; > > const u32 etr3_offset; > > }; > > > > @@ -337,6 +343,7 @@ struct pmc_dev { > > int num_lpm_modes; > > int lpm_en_modes[LPM_MAX_NUM_MODES]; > > u32 *lpm_req_regs; > > + struct platform_device *pdev; > > }; > > > > #define pmc_for_each_mode(i, mode, pmcdev) \ > > -- > > 2.25.1 > > > > > >
On Sat, Jul 24, 2021 at 10:49 PM David E. Box <david.e.box@linux.intel.com> wrote: > On Sat, 2021-07-24 at 11:29 +0300, Andy Shevchenko wrote: > > On Friday, July 23, 2021, Michael Bottini > > <michael.a.bottini@linux.intel.com> wrote: > > > Tiger Lake devices have the capability to track the duration > > > of time that their Power Supply Units (PSUs) are turned off during > > > S0ix. > > > This patch adds a debugfs file `pson_residency_usec` to provide > > > access to this counter. > > > > > > In order to determine whether the device is capable of PSON, > > > use acpi_init_properties() to reevaluate _DSD. > > > > > > > > > > It’s direct abuse of ACPI specification as I read it: > > > > “_DSD must return the same data each time it is evaluated. Firmware > > should not expect it to be evaluated every time (in case it is > > implemented as a method).” > > > > > > NAK to the series. > > Okay, we'll check with the BIOS folks. They are setting this property > from _STA. They may not expect OSPM to reevaluate _DSD. Does it matter? > But they may > have expected that OSPM doesn't attempt to read _DSD until after _STA > is executed. They may study a bit of ACPI specification perhaps? Or if there is indeed an issue with the specification language (so native-speaking people misinterpret above) then ECR to ASWG should be submitted?
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c index 7c4bf7d22fd5..6cf06aecf368 100644 --- a/drivers/platform/x86/intel/pmc/core.c +++ b/drivers/platform/x86/intel/pmc/core.c @@ -595,6 +595,8 @@ static const struct pmc_reg_map tgl_reg_map = { .lpm_sts = tgl_lpm_maps, .lpm_status_offset = TGL_LPM_STATUS_OFFSET, .lpm_live_status_offset = TGL_LPM_LIVE_STATUS_OFFSET, + .pson_residency_offset = TGL_PSON_RESIDENCY_OFFSET, + .pson_residency_counter_step = TGL_PSON_RES_COUNTER_STEP, .etr3_offset = ETR3_OFFSET, }; @@ -1084,6 +1086,20 @@ static int pmc_core_dev_state_get(void *data, u64 *val) DEFINE_DEBUGFS_ATTRIBUTE(pmc_core_dev_state, pmc_core_dev_state_get, NULL, "%llu\n"); +static int pmc_core_pson_residency_get(void *data, u64 *val) +{ + struct pmc_dev *pmcdev = data; + const struct pmc_reg_map *map = pmcdev->map; + u32 value; + + value = pmc_core_reg_read(pmcdev, map->pson_residency_offset); + *val = (u64)value * pmcdev->map->pson_residency_counter_step; + + return 0; +} + +DEFINE_DEBUGFS_ATTRIBUTE(pmc_core_pson_residency, pmc_core_pson_residency_get, NULL, "%llu\n"); + static int pmc_core_check_read_lock_bit(struct pmc_dev *pmcdev) { u32 value; @@ -1788,6 +1804,30 @@ static void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev) } } +static bool pmc_core_is_pson_residency_enabled(struct pmc_dev *pmcdev) +{ + struct platform_device *pdev = pmcdev->pdev; + struct acpi_device *adev = ACPI_COMPANION(&pdev->dev); + acpi_status status; + u8 val; + + if (!adev) + return false; + + acpi_init_properties(adev); + status = acpi_evaluate_object(adev->handle, "PSOP", NULL, NULL); + + if (ACPI_FAILURE(status)) + return false; + + if (fwnode_property_read_u8(acpi_fwnode_handle(adev), + "intel-cec-pson-switching-enabled-in-s0", + &val)) + return false; + + return val == 1; +} + static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) { debugfs_remove_recursive(pmcdev->dbgfs_dir); @@ -1856,6 +1896,11 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev) pmcdev->dbgfs_dir, pmcdev, &pmc_core_substate_req_regs_fops); } + + if (pmcdev->map->pson_residency_offset && pmc_core_is_pson_residency_enabled(pmcdev)) { + debugfs_create_file("pson_residency_usec", 0444, + pmcdev->dbgfs_dir, pmcdev, &pmc_core_pson_residency); + } } static const struct x86_cpu_id intel_pmc_core_ids[] = { @@ -1944,6 +1989,7 @@ static int pmc_core_probe(struct platform_device *pdev) return -ENOMEM; platform_set_drvdata(pdev, pmcdev); + pmcdev->pdev = pdev; cpu_id = x86_match_cpu(intel_pmc_core_ids); if (!cpu_id) diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h index 333e25981e8e..822d77f49861 100644 --- a/drivers/platform/x86/intel/pmc/core.h +++ b/drivers/platform/x86/intel/pmc/core.h @@ -214,6 +214,10 @@ enum ppfear_regs { #define TGL_LPM_PRI_OFFSET 0x1C7C #define TGL_LPM_NUM_MAPS 6 +/* Tigerlake PSON residency register */ +#define TGL_PSON_RESIDENCY_OFFSET 0x18f8 +#define TGL_PSON_RES_COUNTER_STEP 0x7A + /* Extended Test Mode Register 3 (CNL and later) */ #define ETR3_OFFSET 0x1048 #define ETR3_CF9GR BIT(20) @@ -301,6 +305,8 @@ struct pmc_reg_map { const u32 lpm_residency_offset; const u32 lpm_status_offset; const u32 lpm_live_status_offset; + const u32 pson_residency_offset; + const u32 pson_residency_counter_step; const u32 etr3_offset; }; @@ -337,6 +343,7 @@ struct pmc_dev { int num_lpm_modes; int lpm_en_modes[LPM_MAX_NUM_MODES]; u32 *lpm_req_regs; + struct platform_device *pdev; }; #define pmc_for_each_mode(i, mode, pmcdev) \
Tiger Lake devices have the capability to track the duration of time that their Power Supply Units (PSUs) are turned off during S0ix. This patch adds a debugfs file `pson_residency_usec` to provide access to this counter. In order to determine whether the device is capable of PSON, use acpi_init_properties() to reevaluate _DSD. Signed-off-by: Michael Bottini <michael.a.bottini@linux.intel.com> --- drivers/platform/x86/intel/pmc/core.c | 46 +++++++++++++++++++++++++++ drivers/platform/x86/intel/pmc/core.h | 7 ++++ 2 files changed, 53 insertions(+)