Re: [PATCH] thermal/drivers/intel/int340x: Add DLVR support
From: srinivas pandruvada
Date: Tue Apr 04 2023 - 12:40:23 EST
On Mon, 2023-04-03 at 20:37 +0200, Rafael J. Wysocki wrote:
> On Fri, Mar 31, 2023 at 6:53 PM Srinivas Pandruvada
> <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
> >
> > Add support for DLVR (Digital Linear Voltage Regulator) attributes,
> > which can be used to control RFIM.
> > Here instead of "fivr" another directory "dlvr" is created with
> > DLVR
> > attributes:
> >
> > /sys/bus/pci/devices/0000:00:04.0/dlvr
> > ├── dlvr_freq_mhz
> > ├── dlvr_freq_select
> > ├── dlvr_hardware_rev
> > ├── dlvr_pll_busy
> > ├── dlvr_rfim_enable
> > └── dlvr_spread_spectrum_pct
> >
> > Attributes
> > dlvr_freq_mhz (RO):
> > Current DLVR PLL frequency in MHz.
> >
> > dlvr_freq_select (RW):
> > Sets DLVR PLL clock frequency.
> >
> > dlvr_hardware_rev (RO):
> > DLVR hardware revision.
> >
> > dlvr_pll_busy (RO):
> > PLL can't accept frequency change when set.
> >
> > dlvr_rfim_enable (RW):
> > 0: Disable RF frequency hopping, 1: Enable RF frequency hopping.
> >
> > dlvr_spread_spectrum_pct (RW)
> > A write to this register updates the DLVR spread spectrum percent
> > value.
>
> How is this attribute going to be used by user space in practice?
Spread spectrum percent helps to reduce the DLVR clock noise to meet
regulatory compliance. This spreading % increases bandwidth of signal
transmission and hence reduces the effects of interference, noise, and
signal fading.
> Also should it be split like the frequency one (for consistency)?
This is a RW field and is applied immediately unlike frequency, where
it is two step process. First you specify and enable and then see the
effect. So they are two fields.
Thanks,
Srinivas
>
> > Signed-off-by: Srinivas Pandruvada
> > <srinivas.pandruvada@xxxxxxxxxxxxxxx>
> > ---
> > .../driver-api/thermal/intel_dptf.rst | 25 ++++++
> > .../processor_thermal_device.c | 3 +-
> > .../processor_thermal_device.h | 1 +
> > .../processor_thermal_device_pci.c | 2 +-
> > .../int340x_thermal/processor_thermal_rfim.c | 80
> > +++++++++++++++++--
> > 5 files changed, 104 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/driver-api/thermal/intel_dptf.rst
> > b/Documentation/driver-api/thermal/intel_dptf.rst
> > index f5c193cccbda..da906deb6a7d 100644
> > --- a/Documentation/driver-api/thermal/intel_dptf.rst
> > +++ b/Documentation/driver-api/thermal/intel_dptf.rst
> > @@ -264,6 +264,31 @@ DVFS attributes
> > ``rfi_disable (RW)``
> > Disable DDR rate change feature
> >
> > +DLVR attributes
> > +
> > +:file:`/sys/bus/pci/devices/0000\:00\:04.0/dlvr/`
> > +
> > +``dlvr_hardware_rev`` (RO)
> > + DLVR hardware revision.
> > +
> > +``dlvr_freq_mhz`` (RO)
> > + Current DLVR PLL frequency in MHz.
> > +
> > +``dlvr_freq_select`` (RW)
> > + Sets DLVR PLL clock frequency. Once set, and enabled via
> > + dlvr_rfim_enable, the dlvr_freq_mhz will show the current
> > + DLVR PLL frequency.
> > +
> > +``dlvr_pll_busy`` (RO)
> > + PLL can't accept frequency change when set.
> > +
> > +``dlvr_rfim_enable`` (RW)
> > + 0: Disable RF frequency hopping, 1: Enable RF frequency
> > hopping.
> > +
> > +``dlvr_spread_spectrum_pct`` (RW)
> > + Sets DLVR spread spectrum percent value.
> > +
> > +
> > DPTF Power supply and Battery Interface
> > ----------------------------------------
> >
> > diff --git
> > a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> > b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> > index a1dc18be7609..3ca0a2f5937f 100644
> > ---
> > a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> > +++
> > b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> > @@ -337,7 +337,8 @@ int proc_thermal_mmio_add(struct pci_dev *pdev,
> > }
> >
> > if (feature_mask & PROC_THERMAL_FEATURE_FIVR ||
> > - feature_mask & PROC_THERMAL_FEATURE_DVFS) {
> > + feature_mask & PROC_THERMAL_FEATURE_DVFS ||
> > + feature_mask & PROC_THERMAL_FEATURE_DLVR) {
> > ret = proc_thermal_rfim_add(pdev, proc_priv);
> > if (ret) {
> > dev_err(&pdev->dev, "failed to add RFIM
> > interface\n");
> > diff --git
> > a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.h
> > b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.h
> > index 7d52fcff4937..7acaa8f1b896 100644
> > ---
> > a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.h
> > +++
> > b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.h
> > @@ -60,6 +60,7 @@ struct rapl_mmio_regs {
> > #define PROC_THERMAL_FEATURE_FIVR 0x02
> > #define PROC_THERMAL_FEATURE_DVFS 0x04
> > #define PROC_THERMAL_FEATURE_MBOX 0x08
> > +#define PROC_THERMAL_FEATURE_DLVR 0x10
> >
> > #if IS_ENABLED(CONFIG_PROC_THERMAL_MMIO_RAPL)
> > int proc_thermal_rapl_add(struct pci_dev *pdev, struct
> > proc_thermal_device *proc_priv);
> > diff --git
> > a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pc
> > i.c
> > b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pc
> > i.c
> > index 90526f46c9b1..aaff67377250 100644
> > ---
> > a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pc
> > i.c
> > +++
> > b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pc
> > i.c
> > @@ -352,7 +352,7 @@ static SIMPLE_DEV_PM_OPS(proc_thermal_pci_pm,
> > proc_thermal_pci_suspend,
> >
> > static const struct pci_device_id proc_thermal_pci_ids[] = {
> > { PCI_DEVICE_DATA(INTEL, ADL_THERMAL,
> > PROC_THERMAL_FEATURE_RAPL | PROC_THERMAL_FEATURE_FIVR |
> > PROC_THERMAL_FEATURE_DVFS | PROC_THERMAL_FEATURE_MBOX) },
> > - { PCI_DEVICE_DATA(INTEL, MTLP_THERMAL,
> > PROC_THERMAL_FEATURE_RAPL | PROC_THERMAL_FEATURE_FIVR |
> > PROC_THERMAL_FEATURE_DVFS | PROC_THERMAL_FEATURE_MBOX) },
> > + { PCI_DEVICE_DATA(INTEL, MTLP_THERMAL,
> > PROC_THERMAL_FEATURE_RAPL | PROC_THERMAL_FEATURE_FIVR |
> > PROC_THERMAL_FEATURE_DVFS | PROC_THERMAL_FEATURE_MBOX |
> > PROC_THERMAL_FEATURE_DLVR) },
> > { PCI_DEVICE_DATA(INTEL, RPL_THERMAL,
> > PROC_THERMAL_FEATURE_RAPL | PROC_THERMAL_FEATURE_FIVR |
> > PROC_THERMAL_FEATURE_DVFS | PROC_THERMAL_FEATURE_MBOX) },
> > { },
> > };
> > diff --git
> > a/drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c
> > b/drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c
> > index 92ed1213fe37..951a0869982a 100644
> > ---
> > a/drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c
> > +++
> > b/drivers/thermal/intel/int340x_thermal/processor_thermal_rfim.c
> > @@ -39,6 +39,25 @@ static const struct mmio_reg
> > tgl_fivr_mmio_regs[] = {
> > { 1, 0x5A14, 2, 0x3, 1}, /* fivr_fffc_rev */
> > };
> >
> > +static const char * const dlvr_strings[] = {
> > + "dlvr_spread_spectrum_pct",
> > + "dlvr_rfim_enable",
> > + "dlvr_freq_select",
> > + "dlvr_hardware_rev",
> > + "dlvr_freq_mhz",
> > + "dlvr_pll_busy",
> > + NULL
> > +};
> > +
> > +static const struct mmio_reg dlvr_mmio_regs[] = {
> > + { 0, 0x15A08, 5, 0x1F, 0}, /* dlvr_spread_spectrum_pct */
> > + { 0, 0x15A08, 1, 0x1, 7}, /* dlvr_rfim_enable */
> > + { 0, 0x15A08, 12, 0xFFF, 8}, /* dlvr_freq_select */
> > + { 1, 0x15A10, 2, 0x3, 30}, /* dlvr_hardware_rev */
> > + { 1, 0x15A10, 16, 0xFFFF, 0}, /* dlvr_freq_mhz */
> > + { 1, 0x15A10, 1, 0x1, 16}, /* dlvr_pll_busy */
> > +};
> > +
> > /* These will represent sysfs attribute names */
> > static const char * const dvfs_strings[] = {
> > "rfi_restriction_run_busy",
> > @@ -78,14 +97,16 @@ static ssize_t suffix##_show(struct device
> > *dev,\
> > int ret;\
> > \
> > proc_priv = pci_get_drvdata(pdev);\
> > - if (table) {\
> > + if (table == 1) {\
> > match_strs = (const char **)dvfs_strings;\
> > mmio_regs = adl_dvfs_mmio_regs;\
> > - } else { \
> > + } else if (table == 2) { \
> > + match_strs = (const char **)dlvr_strings;\
> > + mmio_regs = dlvr_mmio_regs;\
> > + } else {\
> > match_strs = (const char **)fivr_strings;\
> > mmio_regs = tgl_fivr_mmio_regs;\
> > } \
> > - \
> > ret = match_string(match_strs, -1, attr->attr.name);\
> > if (ret < 0)\
> > return ret;\
> > @@ -109,10 +130,13 @@ static ssize_t suffix##_store(struct device
> > *dev,\
> > u32 mask;\
> > \
> > proc_priv = pci_get_drvdata(pdev);\
> > - if (table) {\
> > + if (table == 1) {\
> > match_strs = (const char **)dvfs_strings;\
> > mmio_regs = adl_dvfs_mmio_regs;\
> > - } else { \
> > + } else if (table == 2) { \
> > + match_strs = (const char **)dlvr_strings;\
> > + mmio_regs = dlvr_mmio_regs;\
> > + } else {\
> > match_strs = (const char **)fivr_strings;\
> > mmio_regs = tgl_fivr_mmio_regs;\
> > } \
> > @@ -147,6 +171,39 @@ RFIM_STORE(spread_spectrum_clk_enable, 0)
> > RFIM_STORE(rfi_vco_ref_code, 0)
> > RFIM_STORE(fivr_fffc_rev, 0)
> >
> > +RFIM_SHOW(dlvr_spread_spectrum_pct, 2)
> > +RFIM_SHOW(dlvr_hardware_rev, 2)
> > +RFIM_SHOW(dlvr_freq_mhz, 2)
> > +RFIM_SHOW(dlvr_pll_busy, 2)
> > +RFIM_SHOW(dlvr_freq_select, 2)
> > +RFIM_SHOW(dlvr_rfim_enable, 2)
> > +
> > +RFIM_STORE(dlvr_spread_spectrum_pct, 2)
> > +RFIM_STORE(dlvr_rfim_enable, 2)
> > +RFIM_STORE(dlvr_freq_select, 2)
> > +
> > +static DEVICE_ATTR_RW(dlvr_spread_spectrum_pct);
> > +static DEVICE_ATTR_RW(dlvr_freq_select);
> > +static DEVICE_ATTR_RO(dlvr_hardware_rev);
> > +static DEVICE_ATTR_RO(dlvr_freq_mhz);
> > +static DEVICE_ATTR_RO(dlvr_pll_busy);
> > +static DEVICE_ATTR_RW(dlvr_rfim_enable);
> > +
> > +static struct attribute *dlvr_attrs[] = {
> > + &dev_attr_dlvr_spread_spectrum_pct.attr,
> > + &dev_attr_dlvr_freq_select.attr,
> > + &dev_attr_dlvr_hardware_rev.attr,
> > + &dev_attr_dlvr_freq_mhz.attr,
> > + &dev_attr_dlvr_pll_busy.attr,
> > + &dev_attr_dlvr_rfim_enable.attr,
> > + NULL
> > +};
> > +
> > +static const struct attribute_group dlvr_attribute_group = {
> > + .attrs = dlvr_attrs,
> > + .name = "dlvr"
> > +};
> > +
> > static DEVICE_ATTR_RW(vco_ref_code_lo);
> > static DEVICE_ATTR_RW(vco_ref_code_hi);
> > static DEVICE_ATTR_RW(spread_spectrum_pct);
> > @@ -277,12 +334,22 @@ int proc_thermal_rfim_add(struct pci_dev
> > *pdev, struct proc_thermal_device *proc
> > return ret;
> > }
> >
> > + if (proc_priv->mmio_feature_mask &
> > PROC_THERMAL_FEATURE_DLVR) {
> > + ret = sysfs_create_group(&pdev->dev.kobj,
> > &dlvr_attribute_group);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > if (proc_priv->mmio_feature_mask &
> > PROC_THERMAL_FEATURE_DVFS) {
> > ret = sysfs_create_group(&pdev->dev.kobj,
> > &dvfs_attribute_group);
> > if (ret && proc_priv->mmio_feature_mask &
> > PROC_THERMAL_FEATURE_FIVR) {
> > sysfs_remove_group(&pdev->dev.kobj,
> > &fivr_attribute_group);
> > return ret;
> > }
> > + if (ret && proc_priv->mmio_feature_mask &
> > PROC_THERMAL_FEATURE_DLVR) {
> > + sysfs_remove_group(&pdev->dev.kobj,
> > &dlvr_attribute_group);
> > + return ret;
> > + }
> > }
> >
> > return 0;
> > @@ -296,6 +363,9 @@ void proc_thermal_rfim_remove(struct pci_dev
> > *pdev)
> > if (proc_priv->mmio_feature_mask &
> > PROC_THERMAL_FEATURE_FIVR)
> > sysfs_remove_group(&pdev->dev.kobj,
> > &fivr_attribute_group);
> >
> > + if (proc_priv->mmio_feature_mask &
> > PROC_THERMAL_FEATURE_DLVR)
> > + sysfs_remove_group(&pdev->dev.kobj,
> > &dlvr_attribute_group);
> > +
> > if (proc_priv->mmio_feature_mask &
> > PROC_THERMAL_FEATURE_DVFS)
> > sysfs_remove_group(&pdev->dev.kobj,
> > &dvfs_attribute_group);
> > }
> > --
> > 2.34.1
> >