[alsa-devel] [PATCH 2/4] acpi: allow for an override to set _REV
Dominik Brodowski
linux at dominikbrodowski.net
Sun May 17 19:41:44 CEST 2015
On Thu, May 14, 2015 at 11:55:33 PM Rafael J. Wysocki wrote:
> On Thursday, May 14, 2015 10:36:52 PM Rafael J. Wysocki wrote:
> > So the only value that would really make sense here is 5.
> Overall, what about the appended patch instead of your [2-3/4] (modulo the
> new command line parameter description which is missing here ATM)?
Well, this approach works as well -- limiting it to an override for just 5
seems reasonable; expanding blacklist.c to also cover this case (even though
it's not a blacklisting per se) isn't worth any discussion.
Nonetheless, a few specifics:
> +config ACPI_REV_OVERRIDE_POSSIBLE
Why should that be a config option at all? The code savings should be
really, really tiny; and especially in the beginning we might see a few
machines where testing the override might seem to be a good idea. So I'd
favour having the command line optional, and then only specific quirks
behind a config option: For the Dell XPS 13 it makes sense to disable the
quirk if userspace can manage i2s sound; for other systems, there may not be
such hope. And as this is a machine-specific decision, I fear that we have
to do CONFIG options for each and every such DMI entry.
> +#ifdef CONFIG_ACPI_REV_OVERRIDE_POSSIBLE
> +static bool acpi_rev_override;
> +
> +int __init acpi_rev_override_setup(char *str)
> +{
> + acpi_rev_override = true;
> + return 1;
> +}
> +__setup("acpi_rev_override", acpi_rev_override_setup);
> +#else
> +#define acpi_rev_override false
> +#endif
Why __setup, and not module_param? Should mean a smaller diffstat...
So, here's what I'd do based on your modification:
Documentation/kernel-parameters.txt | 16 ++++++++++++++++
drivers/acpi/Kconfig | 20 ++++++++++++++++++++
drivers/acpi/blacklist.c | 26 ++++++++++++++++++++++++++
drivers/acpi/internal.h | 1 +
drivers/acpi/osl.c | 8 ++++++++
5 files changed, 71 insertions(+)
[PATCH] acpi: override to set _REV, especially on Dell XPS 13 (2015)
By using a module parameter named acpi.override_rev=1, the BIOS
may be told a different supported ACPI revision compared to the default
(which currently is 5, but will be modified to 2 when the revert of
b1ef29725865 is reverted).
Such an override is needed, for example, on a Dell XPS 13 (2015):
Based on what ACPI exports as is supported version (_REV), the firmware
configures the audio device to either work in HDA mode or in I2S mode.
As the latter only works on sufficiently new userspace, add a quirk and
an associated config option to force sound to HDA mode.
Signed-off-by: Dominik Brodowski <linux at dominikbrodowski.net>
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 61ab162..1edb048 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -335,6 +335,22 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
to assume that this machine's pmtimer latches its value
and always returns good values.
+ acpi.rev_override= [HW,ACPI]
+ Export 5 as the supported ACPI REV
+ Format: { "0" | "1" } (0 = DMI-based quirks,
+ 1 = force-enabled)
+ Up to and including Linux v4.1, the BIOS was told which
+ ACPI revision the ACPICA subsystem in Linux actually
+ supports (which was 5 at the time); from v4.2 on, this
+ value will be set statically to 2 to match the behavior
+ of other ACPI implementations. As some BIOS may operate
+ differently depending on which value _REV is set to, this
+ parameter offers the capability to specify that the
+ previously used value 5 is to be exported to the firmware.
+ Note that such changes in the behavior of the BIOS may
+ only be visible after cold booting the system with this
+ parameter _twice_.
+
acpi_sci= [HW,ACPI] ACPI System Control Interrupt trigger mode
Format: { level | edge | high | low }
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index ab2cbb5..a5272c2 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -430,4 +430,24 @@ config XPOWER_PMIC_OPREGION
endif
+config ACPI_REV_OVERRIDE_DELL_XPS_13_2015
+ bool "Dell XPS 13 (2015) quirk to force HDA sound"
+ depends on X86 && SND_HDA
+ default y
+ help
+ Based on what ACPI exports as the supported revision to the firmware,
+ Dell XPS 13 (2015) configures its audio device to either work in HDA
+ mode or in I2S mode, where the former is supposed to be used on Linux
+ until the latter is fully supported (in the kernel as well as in user
+ space).
+
+ This option enables a DMI-based quirk for the above Dell machine (so
+ that HDA audio is exposed by the platform firmware to the kernel) and
+ makes it possible to force the kernel to return "5" as the supported
+ ACPI revision via the "acpi_rev_override" command line switch (when
+ using that switch it may be necessary to carry out a cold reboot
+ _twice_ in a row to make it take effect on the firmware).
+
+ If in doubt, say Y.
+
endif # ACPI
diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c
index 1d17919..ed55ad7 100644
--- a/drivers/acpi/blacklist.c
+++ b/drivers/acpi/blacklist.c
@@ -162,6 +162,15 @@ static int __init dmi_disable_osi_win8(const struct dmi_system_id *d)
acpi_osi_setup("!Windows 2012");
return 0;
}
+#ifdef CONFIG_ACPI_REV_OVERRIDE_DELL_XPS_13_2015
+static int __init dmi_enable_rev_override(const struct dmi_system_id *d)
+{
+ printk(KERN_NOTICE PREFIX "DMI detected: %s (force ACPI _REV to 5)\n",
+ d->ident);
+ acpi_rev_override = true;
+ return 0;
+}
+#endif
static struct dmi_system_id acpi_osi_dmi_table[] __initdata = {
{
@@ -325,6 +334,23 @@ static struct dmi_system_id acpi_osi_dmi_table[] __initdata = {
DMI_MATCH(DMI_PRODUCT_NAME, "1015PX"),
},
},
+
+#ifdef CONFIG_ACPI_REV_OVERRIDE_DELL_XPS_13_2015
+ /*
+ * DELL XPS 13 (2015) switches sound between HDA and I2S
+ * depending on the ACPI _REV callback. If userspace supports
+ * I2S sufficiently (or if you do not care about sound), you
+ * can safely disable this quirk.
+ */
+ {
+ .callback = dmi_enable_rev_override,
+ .ident = "DELL XPS 13 (2015)",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "XPS 13 9343"),
+ },
+ },
+#endif
{}
};
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index ba4a61e..fc8db23 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -23,6 +23,7 @@
#define PREFIX "ACPI: "
+extern bool acpi_rev_override;
acpi_status acpi_os_initialize1(void);
int init_acpi_device_notify(void);
int acpi_scan_init(void);
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 7ccba39..4d020d0 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -534,6 +534,9 @@ acpi_os_get_physical_address(void *virt, acpi_physical_address * phys)
}
#endif
+bool acpi_rev_override;
+module_param_named(rev_override, acpi_rev_override, bool, 0);
+
#define ACPI_MAX_OVERRIDE_LEN 100
static char acpi_os_name[ACPI_MAX_OVERRIDE_LEN];
@@ -552,6 +555,11 @@ acpi_os_predefined_override(const struct acpi_predefined_names *init_val,
*new_val = acpi_os_name;
}
+ if (!memcmp(init_val->name, "_REV", 4) && acpi_rev_override) {
+ printk(KERN_INFO PREFIX "Overriding _REV return value to 5\n");
+ *new_val = (char *)5;
+ }
+
return AE_OK;
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20150517/3d45fd4a/attachment.sig>
More information about the Alsa-devel
mailing list