diff mbox series

[1/1,SRU,OEM-5.10] UBUNTU: SAUCE: platform/x86: intel_pmc: Ignore GBE LTR on Tiger Lake platforms

Message ID 20210315065328.54680-2-acelan.kao@canonical.com
State New
Headers show
Series Intel e1000e blocks the system to enter PC10 on TGL platform | expand

Commit Message

AceLan Kao March 15, 2021, 6:53 a.m. UTC
From: "David E. Box" <david.e.box@linux.intel.com>

BugLink: https://bugs.launchpad.net/bugs/1919116

Due to a HW limitation, the Latency Tolerance Reporting (LTR) value
programmed in the Tiger Lake GBE controller is not large enough to allow
the platform to enter Package C10, which in turn prevents the platform from
achieving its low power target during suspend-to-idle.  Ignore the GBE LTR
value on Tiger Lake. LTR ignore functionality is currently performed solely
by a debugfs write call. Split out the LTR code into its own function that
can be called by both the debugfs writer and by this work around.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Sasha Neftin <sasha.neftin@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
(cherry picked from
https://patchwork.kernel.org/project/platform-driver-x86/patch/20210305190608.1834164-1-david.e.box@linux.intel.com/)
Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
---
 drivers/platform/x86/intel_pmc_core.c | 55 ++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 13 deletions(-)

Comments

Andrea Righi March 15, 2021, 3:58 p.m. UTC | #1
On Mon, Mar 15, 2021 at 02:53:28PM +0800, AceLan Kao wrote:
> From: "David E. Box" <david.e.box@linux.intel.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1919116
> 
> Due to a HW limitation, the Latency Tolerance Reporting (LTR) value
> programmed in the Tiger Lake GBE controller is not large enough to allow
> the platform to enter Package C10, which in turn prevents the platform from
> achieving its low power target during suspend-to-idle.  Ignore the GBE LTR
> value on Tiger Lake. LTR ignore functionality is currently performed solely
> by a debugfs write call. Split out the LTR code into its own function that
> can be called by both the debugfs writer and by this work around.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> Reviewed-by: Sasha Neftin <sasha.neftin@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> (cherry picked from
> https://patchwork.kernel.org/project/platform-driver-x86/patch/20210305190608.1834164-1-david.e.box@linux.intel.com/)
> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> ---
>  drivers/platform/x86/intel_pmc_core.c | 55 ++++++++++++++++++++-------
>  1 file changed, 42 insertions(+), 13 deletions(-)

...
> +	/*
> +	 * On TGL, due to a hardware limitation, the GBE LTR blocks PC10 when
> +	 * a cable is attached. Tell the PMC to ignore it.
> +	 */
> +	if (pmcdev->map == &tgl_reg_map) {

Someone on the list suggested: if (pmcdev->map >= &tgl_reg_map)

Should we include also this change?

Thanks,
-Andrea

> +		dev_dbg(&pdev->dev, "ignoring GBE LTR\n");
> +		quirk_ltr_ignore(1U << 3);
> +	}
> +
>  	pmc_core_dbgfs_register(pmcdev);
>  
>  	device_initialized = true;
> -- 
> 2.25.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Timo Aaltonen March 18, 2021, 7:36 p.m. UTC | #2
On 15.3.2021 17.58, Andrea Righi wrote:
> On Mon, Mar 15, 2021 at 02:53:28PM +0800, AceLan Kao wrote:
>> From: "David E. Box" <david.e.box@linux.intel.com>
>>
>> BugLink: https://bugs.launchpad.net/bugs/1919116
>>
>> Due to a HW limitation, the Latency Tolerance Reporting (LTR) value
>> programmed in the Tiger Lake GBE controller is not large enough to allow
>> the platform to enter Package C10, which in turn prevents the platform from
>> achieving its low power target during suspend-to-idle.  Ignore the GBE LTR
>> value on Tiger Lake. LTR ignore functionality is currently performed solely
>> by a debugfs write call. Split out the LTR code into its own function that
>> can be called by both the debugfs writer and by this work around.
>>
>> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
>> Reviewed-by: Sasha Neftin <sasha.neftin@intel.com>
>> Cc: intel-wired-lan@lists.osuosl.org
>> (cherry picked from
>> https://patchwork.kernel.org/project/platform-driver-x86/patch/20210305190608.1834164-1-david.e.box@linux.intel.com/)
>> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
>> ---
>>   drivers/platform/x86/intel_pmc_core.c | 55 ++++++++++++++++++++-------
>>   1 file changed, 42 insertions(+), 13 deletions(-)
> 
> ...
>> +	/*
>> +	 * On TGL, due to a hardware limitation, the GBE LTR blocks PC10 when
>> +	 * a cable is attached. Tell the PMC to ignore it.
>> +	 */
>> +	if (pmcdev->map == &tgl_reg_map) {
> 
> Someone on the list suggested: if (pmcdev->map >= &tgl_reg_map)
> 
> Should we include also this change?
> 

this was considered to be a bad idea
diff mbox series

Patch

diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
index 3e5fe66333f1..38ef87cfd1cc 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -863,34 +863,45 @@  static int pmc_core_pll_show(struct seq_file *s, void *unused)
 }
 DEFINE_SHOW_ATTRIBUTE(pmc_core_pll);
 
-static ssize_t pmc_core_ltr_ignore_write(struct file *file,
-					 const char __user *userbuf,
-					 size_t count, loff_t *ppos)
+static int pmc_core_write_ltr_ignore(u32 value)
 {
 	struct pmc_dev *pmcdev = &pmc;
 	const struct pmc_reg_map *map = pmcdev->map;
-	u32 val, buf_size, fd;
-	int err;
-
-	buf_size = count < 64 ? count : 64;
-
-	err = kstrtou32_from_user(userbuf, buf_size, 10, &val);
-	if (err)
-		return err;
+	u32 fd;
+	int err = 0;
 
 	mutex_lock(&pmcdev->lock);
 
-	if (val > map->ltr_ignore_max) {
+	if (fls(value) > map->ltr_ignore_max) {
 		err = -EINVAL;
 		goto out_unlock;
 	}
 
 	fd = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
-	fd |= (1U << val);
+	fd |= value;
 	pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, fd);
 
 out_unlock:
 	mutex_unlock(&pmcdev->lock);
+
+	return err;
+}
+
+static ssize_t pmc_core_ltr_ignore_write(struct file *file,
+					 const char __user *userbuf,
+					 size_t count, loff_t *ppos)
+{
+	u32 buf_size, val;
+	int err;
+
+	buf_size = count < 64 ? count : 64;
+
+	err = kstrtou32_from_user(userbuf, buf_size, 10, &val);
+	if (err)
+		return err;
+
+	err = pmc_core_write_ltr_ignore(1U << val);
+
 	return err == 0 ? count : err;
 }
 
@@ -1189,6 +1200,15 @@  static int quirk_xtal_ignore(const struct dmi_system_id *id)
 	return 0;
 }
 
+static int quirk_ltr_ignore(u32 val)
+{
+	int err;
+
+	err = pmc_core_write_ltr_ignore(val);
+
+	return err;
+}
+
 static const struct dmi_system_id pmc_core_dmi_table[]  = {
 	{
 	.callback = quirk_xtal_ignore,
@@ -1244,6 +1264,15 @@  static int pmc_core_probe(struct platform_device *pdev)
 	pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit();
 	dmi_check_system(pmc_core_dmi_table);
 
+	/*
+	 * On TGL, due to a hardware limitation, the GBE LTR blocks PC10 when
+	 * a cable is attached. Tell the PMC to ignore it.
+	 */
+	if (pmcdev->map == &tgl_reg_map) {
+		dev_dbg(&pdev->dev, "ignoring GBE LTR\n");
+		quirk_ltr_ignore(1U << 3);
+	}
+
 	pmc_core_dbgfs_register(pmcdev);
 
 	device_initialized = true;