| Message ID | 20230829002346.2104251-1-srinivas.pandruvada@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | thermal: processor_thermal: Suport workload hint | expand |
On Tue, Aug 29, 2023 at 2:23 AM Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > > Add support for Meteor Lake workload hints. Before adding this support, > some reorganization and clean up is required. > First four changes are for clean up and to reorganize code to add > support for workload hint. The last patch adds a test program as part > of self tests. > > v3: > Changes in the commit log > Rename of files for using WT instead of WLT > Address comments from Rafael on v2 > > v2: > Changes in comments and commit log > Self test program is improved to disable workloadtype notification > on exit > > Srinivas Pandruvada (7): > thermal: int340x: processor_thermal: Move mailbox code to common > module > thermal: int340x: processor_thermal: Add interrupt configuration > thermal: int340x: processor_thermal: Use non MSI interrupts by default > thermal/drivers/int340x: Remove PROC_THERMAL_FEATURE_WLT_REQ for > Meteor Lake > thermal: int340x: processor_thermal: Add workload type hint interface > thermal/drivers/int340x: Support workload hint interrupts > selftests/thermel/intel: Add test to read workload hint > > .../driver-api/thermal/intel_dptf.rst | 51 ++++ > .../thermal/intel/int340x_thermal/Makefile | 2 + > .../processor_thermal_device.c | 17 +- > .../processor_thermal_device.h | 21 +- > .../processor_thermal_device_pci.c | 79 ++++-- > .../processor_thermal_device_pci_legacy.c | 3 +- > .../int340x_thermal/processor_thermal_mbox.c | 179 ++++--------- > .../processor_thermal_wt_hint.c | 252 ++++++++++++++++++ > .../processor_thermal_wt_req.c | 136 ++++++++++ > tools/testing/selftests/Makefile | 1 + > .../thermal/intel/workload_hint/Makefile | 12 + > .../intel/workload_hint/workload_hint_test.c | 157 +++++++++++ > 12 files changed, 752 insertions(+), 158 deletions(-) > create mode 100644 drivers/thermal/intel/int340x_thermal/processor_thermal_wt_hint.c > create mode 100644 drivers/thermal/intel/int340x_thermal/processor_thermal_wt_req.c > create mode 100644 tools/testing/selftests/thermal/intel/workload_hint/Makefile > create mode 100644 tools/testing/selftests/thermal/intel/workload_hint/workload_hint_test.c > > -- There is a slight issue with the patch ordering in this series, because the interface to enable the interrupt should only be provided after implementing the interrupt handlers. I don't think that anyone will apply the series partially and try to enable the feature, though. Also, I'm not actually sure if proc_thermal_wt_intr_callback() can run safely against the work item scheduled in proc_thermal_irq_handler() in case the workload hint one triggers along with a thermal threshold one. I think that the access to MMIO is cached, so what if they both try to update the same cache line at the same time? Or are they guaranteed to be different cache lines? Anyway, tentatively applied as 6.7 material, but I've changed the second patch somewhat, because I couldn't convince myself that the implicit type conversions in processor_thermal_mbox_interrupt_config() would always do the right thing regardless of the numbers involved, so please check the result in my bleeding-edge branch. Thanks!
Hi Rafael, On Tue, 2023-09-12 at 16:09 +0200, Rafael J. Wysocki wrote: > On Tue, Aug 29, 2023 at 2:23 AM Srinivas Pandruvada > <srinivas.pandruvada@linux.intel.com> wrote: > > > > [...] > > -- > > There is a slight issue with the patch ordering in this series, > because the interface to enable the interrupt should only be provided > after implementing the interrupt handlers. I don't think that anyone > will apply the series partially and try to enable the feature, > though. Thanks! > > Also, I'm not actually sure if proc_thermal_wt_intr_callback() can > run > safely against the work item scheduled in proc_thermal_irq_handler() > in case the workload hint one triggers along with a thermal threshold > one. I think that the access to MMIO is cached, so what if they both > try to update the same cache line at the same time? Or are they > guaranteed to be different cache lines? These two registers are 90 cache lines apart. Looking at all the registers on this bar for status offsets, they are several cache lines apart. Also this bar is non prefetchable, so continuous data can't be fetched ahead. > > Anyway, tentatively applied as 6.7 material, but I've changed the > second patch somewhat, because I couldn't convince myself that the > implicit type conversions in > processor_thermal_mbox_interrupt_config() > would always do the right thing regardless of the numbers involved, > so > please check the result in my bleeding-edge branch. > If I diff, there is only one change in processor_thermal_mbox.c. Tested that change and works fine. Thanks, Srinivas > Thanks!
On Tue, Sep 12, 2023 at 9:44 PM srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > > Hi Rafael, > > On Tue, 2023-09-12 at 16:09 +0200, Rafael J. Wysocki wrote: > > On Tue, Aug 29, 2023 at 2:23 AM Srinivas Pandruvada > > <srinivas.pandruvada@linux.intel.com> wrote: > > > > > > > > [...] > > > > -- > > > > There is a slight issue with the patch ordering in this series, > > because the interface to enable the interrupt should only be provided > > after implementing the interrupt handlers. I don't think that anyone > > will apply the series partially and try to enable the feature, > > though. > Thanks! > > > > > Also, I'm not actually sure if proc_thermal_wt_intr_callback() can > > run > > safely against the work item scheduled in proc_thermal_irq_handler() > > in case the workload hint one triggers along with a thermal threshold > > one. I think that the access to MMIO is cached, so what if they both > > try to update the same cache line at the same time? Or are they > > guaranteed to be different cache lines? > These two registers are 90 cache lines apart. Looking at all the > registers on this bar for status offsets, they are several cache lines > apart. Also this bar is non prefetchable, so continuous data can't be > fetched ahead. OK > > > > Anyway, tentatively applied as 6.7 material, but I've changed the > > second patch somewhat, because I couldn't convince myself that the > > implicit type conversions in > > processor_thermal_mbox_interrupt_config() > > would always do the right thing regardless of the numbers involved, > > so > > please check the result in my bleeding-edge branch. > > > If I diff, there is only one change in processor_thermal_mbox.c. Tested > that change and works fine. Good, thanks!
Add support for Meteor Lake workload hints. Before adding this support, some reorganization and clean up is required. First four changes are for clean up and to reorganize code to add support for workload hint. The last patch adds a test program as part of self tests. v3: Changes in the commit log Rename of files for using WT instead of WLT Address comments from Rafael on v2 v2: Changes in comments and commit log Self test program is improved to disable workloadtype notification on exit Srinivas Pandruvada (7): thermal: int340x: processor_thermal: Move mailbox code to common module thermal: int340x: processor_thermal: Add interrupt configuration thermal: int340x: processor_thermal: Use non MSI interrupts by default thermal/drivers/int340x: Remove PROC_THERMAL_FEATURE_WLT_REQ for Meteor Lake thermal: int340x: processor_thermal: Add workload type hint interface thermal/drivers/int340x: Support workload hint interrupts selftests/thermel/intel: Add test to read workload hint .../driver-api/thermal/intel_dptf.rst | 51 ++++ .../thermal/intel/int340x_thermal/Makefile | 2 + .../processor_thermal_device.c | 17 +- .../processor_thermal_device.h | 21 +- .../processor_thermal_device_pci.c | 79 ++++-- .../processor_thermal_device_pci_legacy.c | 3 +- .../int340x_thermal/processor_thermal_mbox.c | 179 ++++--------- .../processor_thermal_wt_hint.c | 252 ++++++++++++++++++ .../processor_thermal_wt_req.c | 136 ++++++++++ tools/testing/selftests/Makefile | 1 + .../thermal/intel/workload_hint/Makefile | 12 + .../intel/workload_hint/workload_hint_test.c | 157 +++++++++++ 12 files changed, 752 insertions(+), 158 deletions(-) create mode 100644 drivers/thermal/intel/int340x_thermal/processor_thermal_wt_hint.c create mode 100644 drivers/thermal/intel/int340x_thermal/processor_thermal_wt_req.c create mode 100644 tools/testing/selftests/thermal/intel/workload_hint/Makefile create mode 100644 tools/testing/selftests/thermal/intel/workload_hint/workload_hint_test.c