As reported by Guilherme:
The rtc-cmos interrupt setting was changed in the commit 079062b28fb4
("rtc: cmos: prevent kernel warning on IRQ flags mismatch") in order
to allow shared interrupts; according to that commit's description,
some machine got kernel warnings due to the interrupt line being shared
between rtc-cmos and other hardware, and rtc-cmos didn't allow IRQ sharing
that time.
After the aforementioned commit though it was observed a huge increase
in lost HPET interrupts in some systems, observed through the following
kernel message:
[...] hpet1: lost 35 rtc interrupts
After investigation, it was narrowed down to the shared interrupts
usage when having the kernel option "irqpoll" enabled. In this case,
all IRQ handlers are called for non-timer interrupts, if such handlers
are setup in shared IRQ lines. The rtc-cmos IRQ handler could be set to
hpet_rtc_interrupt(), which will produce the kernel "lost interrupts"
message after doing work - lots of readl/writel to HPET registers, which
are known to be slow.
This patch changes this behavior by preventing shared interrupts for
everything, but Microsoft Surface 3 as stated in the culprit commit message.
Although "irqpoll" is not a default kernel option, it's used in some contexts,
one being the kdump kernel (which is an already "impaired" kernel usually
running with 1 CPU available), so the performance burden could be considerable.
Also, the same issue would happen (in a shorter extent though) when using
"irqfixup" kernel option.
In a quick experiment, a virtual machine with uptime of 2 minutes produced
>300 calls to hpet_rtc_interrupt() when "irqpoll" was set, whereas without
sharing interrupts this number reduced to 1 interrupt. Machines with more
hardware than a VM should generate even more unnecessary HPET interrupts
in this scenario.
Fixes: 079062b28fb4 ("rtc: cmos: prevent kernel warning on IRQ flags mismatch")
Reported-by: Guilherme G. Piccoli <[email protected]>
Suggested-by: Thomas Gleixner <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/rtc/rtc-cmos.c | 21 +++++++++++++++++++--
include/linux/mc146818rtc.h | 4 +++-
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 033303708c8b..09b7cdda9f55 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -27,6 +27,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/dmi.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>
@@ -40,7 +41,6 @@
#ifdef CONFIG_X86
#include <asm/i8259.h>
#include <asm/processor.h>
-#include <linux/dmi.h>
#endif
/* this is for "generic access to PC-style RTC" using CMOS_READ/CMOS_WRITE */
@@ -836,6 +836,7 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq)
if (is_valid_irq(rtc_irq)) {
irq_handler_t rtc_cmos_int_handler;
+ unsigned long irq_flags = 0;
if (use_hpet_alarm()) {
rtc_cmos_int_handler = hpet_rtc_interrupt;
@@ -849,8 +850,11 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq)
} else
rtc_cmos_int_handler = cmos_interrupt;
+ if (flags & CMOS_RTC_FLAGS_SHARED_IRQ)
+ irq_flags |= IRQF_SHARED;
+
retval = request_irq(rtc_irq, rtc_cmos_int_handler,
- IRQF_SHARED, dev_name(&cmos_rtc.rtc->dev),
+ irq_flags, dev_name(&cmos_rtc.rtc->dev),
cmos_rtc.rtc);
if (retval < 0) {
dev_dbg(dev, "IRQ %d is already in use\n", rtc_irq);
@@ -1215,6 +1219,16 @@ static void use_acpi_alarm_quirks(void)
static inline void use_acpi_alarm_quirks(void) { }
#endif
+static const struct dmi_system_id rtc_cmos_surface3_table[] = {
+ {
+ .ident = "Microsoft Surface 3",
+ .matches = {
+ DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
+ },
+ },
+ {}
+};
+
/* Every ACPI platform has a mc146818 compatible "cmos rtc". Here we find
* its device node and pass extra config data. This helps its driver use
* capabilities that the now-obsolete mc146818 didn't have, and informs it
@@ -1229,6 +1243,9 @@ static void cmos_wake_setup(struct device *dev)
use_acpi_alarm_quirks();
+ if (dmi_check_system(rtc_cmos_surface3_table))
+ acpi_rtc_info.flags |= CMOS_RTC_FLAGS_SHARED_IRQ;
+
rtc_wake_setup(dev);
acpi_rtc_info.wake_on = rtc_wake_on;
acpi_rtc_info.wake_off = rtc_wake_off;
diff --git a/include/linux/mc146818rtc.h b/include/linux/mc146818rtc.h
index 0661af17a758..d62d69b48b3e 100644
--- a/include/linux/mc146818rtc.h
+++ b/include/linux/mc146818rtc.h
@@ -35,7 +35,9 @@ struct cmos_rtc_board_info {
void (*wake_off)(struct device *dev);
u32 flags;
-#define CMOS_RTC_FLAGS_NOFREQ (1 << 0)
+#define CMOS_RTC_FLAGS_NOFREQ (1 << 0)
+#define CMOS_RTC_FLAGS_SHARED_IRQ (1 << 1)
+
int address_space;
u8 rtc_day_alarm; /* zero, or register index */
--
2.24.1
Since we have a common x86 quirk that provides an exported variable,
use it instead of local DMI table match.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/platform/x86/surface3-wmi.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/platform/x86/surface3-wmi.c b/drivers/platform/x86/surface3-wmi.c
index 130b6f52a600..5eeedc4ddb8a 100644
--- a/drivers/platform/x86/surface3-wmi.c
+++ b/drivers/platform/x86/surface3-wmi.c
@@ -11,9 +11,9 @@
#include <linux/slab.h>
#include <linux/acpi.h>
-#include <linux/dmi.h>
#include <linux/input.h>
#include <linux/mutex.h>
+#include <linux/platform_data/x86/machine.h>
#include <linux/platform_device.h>
#include <linux/spi/spi.h>
@@ -29,18 +29,6 @@ MODULE_LICENSE("GPL");
MODULE_ALIAS("wmi:" SURFACE3_LID_GUID);
-static const struct dmi_system_id surface3_dmi_table[] = {
-#if defined(CONFIG_X86)
- {
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
- },
- },
-#endif
- { }
-};
-
struct surface3_wmi {
struct acpi_device *touchscreen_adev;
struct acpi_device *pnp0c0d_adev;
@@ -201,7 +189,7 @@ static int __init s3_wmi_probe(struct platform_device *pdev)
{
int error;
- if (!dmi_check_system(surface3_dmi_table))
+ if (!x86_microsoft_surface_3_machine)
return -ENODEV;
memset(&s3_wmi, 0, sizeof(s3_wmi));
--
2.24.1
Since we have a common x86 quirk that provides an exported variable,
use it instead of local DMI table match.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/rtc/rtc-cmos.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 09b7cdda9f55..b8b67a98c47a 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -33,6 +33,7 @@
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/spinlock.h>
+#include <linux/platform_data/x86/machine.h>
#include <linux/platform_device.h>
#include <linux/log2.h>
#include <linux/pm.h>
@@ -1219,16 +1220,6 @@ static void use_acpi_alarm_quirks(void)
static inline void use_acpi_alarm_quirks(void) { }
#endif
-static const struct dmi_system_id rtc_cmos_surface3_table[] = {
- {
- .ident = "Microsoft Surface 3",
- .matches = {
- DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
- },
- },
- {}
-};
-
/* Every ACPI platform has a mc146818 compatible "cmos rtc". Here we find
* its device node and pass extra config data. This helps its driver use
* capabilities that the now-obsolete mc146818 didn't have, and informs it
@@ -1243,7 +1234,7 @@ static void cmos_wake_setup(struct device *dev)
use_acpi_alarm_quirks();
- if (dmi_check_system(rtc_cmos_surface3_table))
+ if (x86_microsoft_surface_3_machine)
acpi_rtc_info.flags |= CMOS_RTC_FLAGS_SHARED_IRQ;
rtc_wake_setup(dev);
--
2.24.1
Static checker is not happy with
.../kernel/quirks.c:666:6: warning: symbol 'x86_apple_machine' was not declared. Should it be static?
This is due to missed inclusion. Add it to satisfy the static checker.
Signed-off-by: Andy Shevchenko <[email protected]>
---
arch/x86/kernel/quirks.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
index 1daf8f2aa21f..5b96654aacc0 100644
--- a/arch/x86/kernel/quirks.c
+++ b/arch/x86/kernel/quirks.c
@@ -4,6 +4,7 @@
*/
#include <linux/dmi.h>
#include <linux/pci.h>
+#include <linux/platform_data/x86/machine.h>
#include <linux/irq.h>
#include <asm/hpet.h>
--
2.24.1
In order to extend the DMI based quirks, convert them to a table.
Signed-off-by: Andy Shevchenko <[email protected]>
---
arch/x86/kernel/quirks.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
index 5b96654aacc0..447d4fba8516 100644
--- a/arch/x86/kernel/quirks.c
+++ b/arch/x86/kernel/quirks.c
@@ -667,8 +667,31 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2083, quirk_intel_purley_xeon_ras
bool x86_apple_machine;
EXPORT_SYMBOL(x86_apple_machine);
+static int apple_machine_cb(const struct dmi_system_id *id)
+{
+ x86_apple_machine = true;
+ return 1;
+}
+
+static const struct dmi_system_id x86_machine_table[] = {
+ {
+ .ident = "x86 Apple Macintosh",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+ },
+ .callback = apple_machine_cb,
+ },
+ {
+ .ident = "x86 Apple Macintosh",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Computer, Inc."),
+ },
+ .callback = apple_machine_cb,
+ },
+ {}
+};
+
void __init early_platform_quirks(void)
{
- x86_apple_machine = dmi_match(DMI_SYS_VENDOR, "Apple Inc.") ||
- dmi_match(DMI_SYS_VENDOR, "Apple Computer, Inc.");
+ dmi_check_system(x86_machine_table);
}
--
2.24.1
Add a DMI quirk for Microsoft Surface 3 which will be utilized by few drivers.
Cc: Cezary Rojewski <[email protected]>
Cc: Pierre-Louis Bossart <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Jie Yang <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: [email protected]
Signed-off-by: Andy Shevchenko <[email protected]>
---
arch/x86/kernel/quirks.c | 16 ++++++++++++++++
include/linux/platform_data/x86/machine.h | 5 +++++
2 files changed, 21 insertions(+)
diff --git a/arch/x86/kernel/quirks.c b/arch/x86/kernel/quirks.c
index 447d4fba8516..9574ef7eaa66 100644
--- a/arch/x86/kernel/quirks.c
+++ b/arch/x86/kernel/quirks.c
@@ -673,6 +673,15 @@ static int apple_machine_cb(const struct dmi_system_id *id)
return 1;
}
+bool x86_microsoft_surface_3_machine;
+EXPORT_SYMBOL(x86_microsoft_surface_3_machine);
+
+static int microsoft_surface_3_machine_cb(const struct dmi_system_id *id)
+{
+ x86_microsoft_surface_3_machine = true;
+ return 1;
+}
+
static const struct dmi_system_id x86_machine_table[] = {
{
.ident = "x86 Apple Macintosh",
@@ -688,6 +697,13 @@ static const struct dmi_system_id x86_machine_table[] = {
},
.callback = apple_machine_cb,
},
+ {
+ .ident = "Microsoft Surface 3",
+ .matches = {
+ DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
+ },
+ .callback = microsoft_surface_3_machine_cb,
+ },
{}
};
diff --git a/include/linux/platform_data/x86/machine.h b/include/linux/platform_data/x86/machine.h
index b1e7a560a046..9bdf5a06b490 100644
--- a/include/linux/platform_data/x86/machine.h
+++ b/include/linux/platform_data/x86/machine.h
@@ -8,8 +8,13 @@
* x86_apple_machine - whether the machine is an x86 Apple Macintosh
*/
extern bool x86_apple_machine;
+/**
+ * x86_microsoft_surface_3_machine - whether the machine is Microsoft Surface 3
+ */
+extern bool x86_microsoft_surface_3_machine;
#else
#define x86_apple_machine false
+#define x86_microsoft_surface_3_machine false
#endif
#endif /* PLATFORM_DATA_X86_MACHINE_H */
--
2.24.1
Rename linux/platform_data/x86/apple.h to linux/platform_data/x86/machine.h
in order to add new quirks later on. For sake of being less intrusive,
leave former file that includes a latter one.
While here, add include to linux/types.h due to bool type in use.
Signed-off-by: Andy Shevchenko <[email protected]>
---
include/linux/platform_data/x86/apple.h | 14 +-------------
include/linux/platform_data/x86/machine.h | 15 +++++++++++++++
2 files changed, 16 insertions(+), 13 deletions(-)
create mode 100644 include/linux/platform_data/x86/machine.h
diff --git a/include/linux/platform_data/x86/apple.h b/include/linux/platform_data/x86/apple.h
index 079e816c3c21..1fd0af6ffea9 100644
--- a/include/linux/platform_data/x86/apple.h
+++ b/include/linux/platform_data/x86/apple.h
@@ -1,13 +1 @@
-#ifndef PLATFORM_DATA_X86_APPLE_H
-#define PLATFORM_DATA_X86_APPLE_H
-
-#ifdef CONFIG_X86
-/**
- * x86_apple_machine - whether the machine is an x86 Apple Macintosh
- */
-extern bool x86_apple_machine;
-#else
-#define x86_apple_machine false
-#endif
-
-#endif
+#include <linux/platform_data/x86/machine.h>
diff --git a/include/linux/platform_data/x86/machine.h b/include/linux/platform_data/x86/machine.h
new file mode 100644
index 000000000000..b1e7a560a046
--- /dev/null
+++ b/include/linux/platform_data/x86/machine.h
@@ -0,0 +1,15 @@
+#ifndef PLATFORM_DATA_X86_MACHINE_H
+#define PLATFORM_DATA_X86_MACHINE_H
+
+#include <linux/types.h>
+
+#ifdef CONFIG_X86
+/**
+ * x86_apple_machine - whether the machine is an x86 Apple Macintosh
+ */
+extern bool x86_apple_machine;
+#else
+#define x86_apple_machine false
+#endif
+
+#endif /* PLATFORM_DATA_X86_MACHINE_H */
--
2.24.1
Since we have a common x86 quirk that provides an exported variable,
use it instead of local DMI table match.
Cc: Cezary Rojewski <[email protected]>
Cc: Pierre-Louis Bossart <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Jie Yang <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: [email protected]
Signed-off-by: Andy Shevchenko <[email protected]>
---
.../intel/common/soc-acpi-intel-cht-match.c | 28 ++-----------------
1 file changed, 3 insertions(+), 25 deletions(-)
diff --git a/sound/soc/intel/common/soc-acpi-intel-cht-match.c b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
index d0fb43c2b9f6..833d2e130e6e 100644
--- a/sound/soc/intel/common/soc-acpi-intel-cht-match.c
+++ b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
@@ -5,31 +5,11 @@
* Copyright (c) 2017, Intel Corporation.
*/
-#include <linux/dmi.h>
+#include <linux/platform_data/x86/machine.h>
+
#include <sound/soc-acpi.h>
#include <sound/soc-acpi-intel-match.h>
-static unsigned long cht_machine_id;
-
-#define CHT_SURFACE_MACH 1
-
-static int cht_surface_quirk_cb(const struct dmi_system_id *id)
-{
- cht_machine_id = CHT_SURFACE_MACH;
- return 1;
-}
-
-static const struct dmi_system_id cht_table[] = {
- {
- .callback = cht_surface_quirk_cb,
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
- },
- },
- { }
-};
-
static struct snd_soc_acpi_mach cht_surface_mach = {
.id = "10EC5640",
.drv_name = "cht-bsw-rt5645",
@@ -43,9 +23,7 @@ static struct snd_soc_acpi_mach *cht_quirk(void *arg)
{
struct snd_soc_acpi_mach *mach = arg;
- dmi_check_system(cht_table);
-
- if (cht_machine_id == CHT_SURFACE_MACH)
+ if (x86_microsoft_surface_3_machine)
return &cht_surface_mach;
else
return mach;
--
2.24.1
On 1/17/20 11:56 AM, Andy Shevchenko wrote:
> Since we have a common x86 quirk that provides an exported variable,
> use it instead of local DMI table match.
>
> Cc: Cezary Rojewski <[email protected]>
> Cc: Pierre-Louis Bossart <[email protected]>
> Cc: Liam Girdwood <[email protected]>
> Cc: Jie Yang <[email protected]>
> Cc: Mark Brown <[email protected]>
> Cc: [email protected]
> Signed-off-by: Andy Shevchenko <[email protected]>
Thanks Andy.
Acked-by: Pierre-Louis Bossart <[email protected]>
> ---
> .../intel/common/soc-acpi-intel-cht-match.c | 28 ++-----------------
> 1 file changed, 3 insertions(+), 25 deletions(-)
>
> diff --git a/sound/soc/intel/common/soc-acpi-intel-cht-match.c b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> index d0fb43c2b9f6..833d2e130e6e 100644
> --- a/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> +++ b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> @@ -5,31 +5,11 @@
> * Copyright (c) 2017, Intel Corporation.
> */
>
> -#include <linux/dmi.h>
> +#include <linux/platform_data/x86/machine.h>
> +
> #include <sound/soc-acpi.h>
> #include <sound/soc-acpi-intel-match.h>
>
> -static unsigned long cht_machine_id;
> -
> -#define CHT_SURFACE_MACH 1
> -
> -static int cht_surface_quirk_cb(const struct dmi_system_id *id)
> -{
> - cht_machine_id = CHT_SURFACE_MACH;
> - return 1;
> -}
> -
> -static const struct dmi_system_id cht_table[] = {
> - {
> - .callback = cht_surface_quirk_cb,
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
> - },
> - },
> - { }
> -};
> -
> static struct snd_soc_acpi_mach cht_surface_mach = {
> .id = "10EC5640",
> .drv_name = "cht-bsw-rt5645",
> @@ -43,9 +23,7 @@ static struct snd_soc_acpi_mach *cht_quirk(void *arg)
> {
> struct snd_soc_acpi_mach *mach = arg;
>
> - dmi_check_system(cht_table);
> -
> - if (cht_machine_id == CHT_SURFACE_MACH)
> + if (x86_microsoft_surface_3_machine)
> return &cht_surface_mach;
> else
> return mach;
>
On Fri, Jan 17, 2020 at 01:10:55PM -0600, Pierre-Louis Bossart wrote:
> On 1/17/20 11:56 AM, Andy Shevchenko wrote:
> > Since we have a common x86 quirk that provides an exported variable,
> > use it instead of local DMI table match.
> >
> > Cc: Cezary Rojewski <[email protected]>
> > Cc: Pierre-Louis Bossart <[email protected]>
> > Cc: Liam Girdwood <[email protected]>
> > Cc: Jie Yang <[email protected]>
> > Cc: Mark Brown <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Andy Shevchenko <[email protected]>
>
> Thanks Andy.
>
> Acked-by: Pierre-Louis Bossart <[email protected]>
Thank you. Though I think I'll re-do this a bit, i.e.
- convert the cht_quirk() to oneliner that is using ternary operator
- convert also codec driver to use variable instead of DMI match
>
> > ---
> > .../intel/common/soc-acpi-intel-cht-match.c | 28 ++-----------------
> > 1 file changed, 3 insertions(+), 25 deletions(-)
> >
> > diff --git a/sound/soc/intel/common/soc-acpi-intel-cht-match.c b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> > index d0fb43c2b9f6..833d2e130e6e 100644
> > --- a/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> > +++ b/sound/soc/intel/common/soc-acpi-intel-cht-match.c
> > @@ -5,31 +5,11 @@
> > * Copyright (c) 2017, Intel Corporation.
> > */
> > -#include <linux/dmi.h>
> > +#include <linux/platform_data/x86/machine.h>
> > +
> > #include <sound/soc-acpi.h>
> > #include <sound/soc-acpi-intel-match.h>
> > -static unsigned long cht_machine_id;
> > -
> > -#define CHT_SURFACE_MACH 1
> > -
> > -static int cht_surface_quirk_cb(const struct dmi_system_id *id)
> > -{
> > - cht_machine_id = CHT_SURFACE_MACH;
> > - return 1;
> > -}
> > -
> > -static const struct dmi_system_id cht_table[] = {
> > - {
> > - .callback = cht_surface_quirk_cb,
> > - .matches = {
> > - DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> > - DMI_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
> > - },
> > - },
> > - { }
> > -};
> > -
> > static struct snd_soc_acpi_mach cht_surface_mach = {
> > .id = "10EC5640",
> > .drv_name = "cht-bsw-rt5645",
> > @@ -43,9 +23,7 @@ static struct snd_soc_acpi_mach *cht_quirk(void *arg)
> > {
> > struct snd_soc_acpi_mach *mach = arg;
> > - dmi_check_system(cht_table);
> > -
> > - if (cht_machine_id == CHT_SURFACE_MACH)
> > + if (x86_microsoft_surface_3_machine)
> > return &cht_surface_mach;
> > else
> > return mach;
> >
--
With Best Regards,
Andy Shevchenko
On Fri, Jan 17, 2020 at 07:56:23PM +0200, Andy Shevchenko wrote:
> Add a DMI quirk for Microsoft Surface 3 which will be utilized by few drivers.
I have patches 5 and 7 with no other context, what's going on with
dependencies here?
On Mon, Jan 20, 2020 at 12:09:33PM +0000, Mark Brown wrote:
> On Fri, Jan 17, 2020 at 07:56:23PM +0200, Andy Shevchenko wrote:
> > Add a DMI quirk for Microsoft Surface 3 which will be utilized by few drivers.
>
> I have patches 5 and 7 with no other context, what's going on with
> dependencies here?
Other patches has no relation to ASoC or vise versa.
I will Cc you for entire series in next version.
--
With Best Regards,
Andy Shevchenko
On 17/01/2020 14:56, Andy Shevchenko wrote:
> As reported by Guilherme:
>
> The rtc-cmos interrupt setting was changed in the commit 079062b28fb4
> ("rtc: cmos: prevent kernel warning on IRQ flags mismatch") in order
> to allow shared interrupts; according to that commit's description,
> some machine got kernel warnings due to the interrupt line being shared
> between rtc-cmos and other hardware, and rtc-cmos didn't allow IRQ sharing
> that time.
>
> After the aforementioned commit though it was observed a huge increase
> in lost HPET interrupts in some systems, observed through the following
> kernel message:
>
> [...] hpet1: lost 35 rtc interrupts
>
> After investigation, it was narrowed down to the shared interrupts
> usage when having the kernel option "irqpoll" enabled. In this case,
> all IRQ handlers are called for non-timer interrupts, if such handlers
> are setup in shared IRQ lines. The rtc-cmos IRQ handler could be set to
> hpet_rtc_interrupt(), which will produce the kernel "lost interrupts"
> message after doing work - lots of readl/writel to HPET registers, which
> are known to be slow.
>
> This patch changes this behavior by preventing shared interrupts for
> everything, but Microsoft Surface 3 as stated in the culprit commit message.
> Although "irqpoll" is not a default kernel option, it's used in some contexts,
> one being the kdump kernel (which is an already "impaired" kernel usually
> running with 1 CPU available), so the performance burden could be considerable.
> Also, the same issue would happen (in a shorter extent though) when using
> "irqfixup" kernel option.
>
> In a quick experiment, a virtual machine with uptime of 2 minutes produced
>> 300 calls to hpet_rtc_interrupt() when "irqpoll" was set, whereas without
> sharing interrupts this number reduced to 1 interrupt. Machines with more
> hardware than a VM should generate even more unnecessary HPET interrupts
> in this scenario.
>
> Fixes: 079062b28fb4 ("rtc: cmos: prevent kernel warning on IRQ flags mismatch")
> Reported-by: Guilherme G. Piccoli <[email protected]>
> Suggested-by: Thomas Gleixner <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
Andy, thanks for the great patch! It works for me, I've tested it on top
of 5.5-rc7, no more RTC lost interrupts while using the "irqpoll"
parameter. So, feel free to add my:
Tested-by: Guilherme G. Piccoli <[email protected]>
Cheers,
Guilherme
On Wed, Jan 22, 2020 at 10:27:18AM -0300, Guilherme G. Piccoli wrote:
> On 17/01/2020 14:56, Andy Shevchenko wrote:
> Andy, thanks for the great patch! It works for me, I've tested it on top
> of 5.5-rc7, no more RTC lost interrupts while using the "irqpoll"
> parameter. So, feel free to add my:
>
> Tested-by: Guilherme G. Piccoli <[email protected]>
Thank you for testing!
(Un)fortunately I dug a bit into the history of the patches and into the ACPI
tables of MS Surface 3. I have another (better) solution which I will send
separately from this series.
--
With Best Regards,
Andy Shevchenko
On Wed, Jan 22, 2020 at 10:39 AM Andy Shevchenko
<[email protected]> wrote:
> [...]
> Thank you for testing!
>
> (Un)fortunately I dug a bit into the history of the patches and into the ACPI
> tables of MS Surface 3. I have another (better) solution which I will send
> separately from this series.
>
OK, if possible, loop me in and I can test that too.
Appreciate your effort on this!
Thanks,
Guilherme