Re: [PATCH] samsung-laptop: unregister ACPI video module for somewell known laptops

[Corentin Chary]

On Thu, Mar 22, 2012 at 2:08 PM, Corentin Chary <corentincj@xxxxxxxxxx> wrote:
> On these laptops, the ACPI video is not functional, and very unlikely
> to be fixed by the vendor. Note that intel_backlight works for some
> of these laptops, and the backlight from samsung-laptop always work.
>
> The good news is that newer laptops have functional ACPI video device
> and won't end up growing this list.
>
> Signed-off-by: Corentin Chary <corentincj@xxxxxxxxxx>
> ---
> Âdrivers/platform/x86/samsung-laptop.c | Â 80 +++++++++++++++++++++++++++++++--
> Â1 files changed, 76 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
> index 4787afd..e2a34b4 100644
> --- a/drivers/platform/x86/samsung-laptop.c
> +++ b/drivers/platform/x86/samsung-laptop.c
> @@ -26,6 +26,9 @@
> Â#include <linux/seq_file.h>
> Â#include <linux/debugfs.h>
> Â#include <linux/ctype.h>
> +#if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
> +#include <acpi/video.h>
> +#endif
>
> Â/*
> Â* This driver is needed because a number of Samsung laptops do not hook
> @@ -336,6 +339,7 @@ struct samsung_laptop {
> Â Â Â Âstruct work_struct kbd_led_work;
>
> Â Â Â Âstruct samsung_laptop_debug debug;
> + Â Â Â struct samsung_quirks *quirks;
>
> Â Â Â Âbool handle_backlight;
> Â Â Â Âbool has_stepping_quirk;
> @@ -343,7 +347,15 @@ struct samsung_laptop {
> Â Â Â Âchar sdiag[64];
> Â};
>
> +struct samsung_quirks {
> + Â Â Â bool broken_acpi_video;
> +};
> +
> +static struct samsung_quirks samsung_unknown = {};
>
> +static struct samsung_quirks samsung_broken_acpi_video = {
> + Â Â Â .broken_acpi_video = true,
> +};
>
> Âstatic bool force;
> Âmodule_param(force, bool, 0);
> @@ -1416,6 +1428,14 @@ static int __init samsung_platform_init(struct samsung_laptop *samsung)
> Â Â Â Âreturn 0;
> Â}
>
> +static struct samsung_quirks *quirks;
> +
> +static int __init samsung_dmi_matched(const struct dmi_system_id *d)
> +{
> + Â Â Â quirks = d->driver_data;
> + Â Â Â return 0;
> +}
> +
> Âstatic struct dmi_system_id __initdata samsung_dmi_table[] = {
> Â Â Â Â{
> Â Â Â Â Â Â Â Â.matches = {
> @@ -1445,6 +1465,47 @@ static struct dmi_system_id __initdata samsung_dmi_table[] = {
> Â Â Â Â Â Â Â Â Â Â Â ÂDMI_MATCH(DMI_CHASSIS_TYPE, "14"), /* Sub-Notebook */
> Â Â Â Â Â Â Â Â},
> Â Â Â Â},
> + Â Â Â /* Specific DMI ids for laptop with quirks */
> + Â Â Â {
> + Â Â Â Â.callback = samsung_dmi_matched,
> + Â Â Â Â.ident = "N150P",
> + Â Â Â Â.matches = {
> + Â Â Â Â Â Â Â DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> + Â Â Â Â Â Â Â DMI_MATCH(DMI_PRODUCT_NAME, "N150P"),
> + Â Â Â Â Â Â Â DMI_MATCH(DMI_BOARD_NAME, "N150P"),
> + Â Â Â Â Â Â Â },
> + Â Â Â Â.driver_data = &samsung_broken_acpi_video,
> + Â Â Â },
> + Â Â Â {
> + Â Â Â Â.callback = samsung_dmi_matched,
> + Â Â Â Â.ident = "N145P/N250P/N260P",
> + Â Â Â Â.matches = {
> + Â Â Â Â Â Â Â DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> + Â Â Â Â Â Â Â DMI_MATCH(DMI_PRODUCT_NAME, "N145P/N250P/N260P"),
> + Â Â Â Â Â Â Â DMI_MATCH(DMI_BOARD_NAME, "N145P/N250P/N260P"),
> + Â Â Â Â Â Â Â },
> + Â Â Â Â.driver_data = &samsung_broken_acpi_video,
> + Â Â Â },
> + Â Â Â {
> + Â Â Â Â.callback = samsung_dmi_matched,
> + Â Â Â Â.ident = "N150/N210/N220",
> + Â Â Â Â.matches = {
> + Â Â Â Â Â Â Â DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> + Â Â Â Â Â Â Â DMI_MATCH(DMI_PRODUCT_NAME, "N150/N210/N220"),
> + Â Â Â Â Â Â Â DMI_MATCH(DMI_BOARD_NAME, "N150/N210/N220"),
> + Â Â Â Â Â Â Â },
> + Â Â Â Â.driver_data = &samsung_broken_acpi_video,
> + Â Â Â },
> + Â Â Â {
> + Â Â Â Â.callback = samsung_dmi_matched,
> + Â Â Â Â.ident = "NF110/NF210/NF310",
> + Â Â Â Â.matches = {
> + Â Â Â Â Â Â Â DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> + Â Â Â Â Â Â Â DMI_MATCH(DMI_PRODUCT_NAME, "NF110/NF210/NF310"),
> + Â Â Â Â Â Â Â DMI_MATCH(DMI_BOARD_NAME, "NF110/NF210/NF310"),
> + Â Â Â Â Â Â Â },
> + Â Â Â Â.driver_data = &samsung_broken_acpi_video,
> + Â Â Â },
> Â Â Â Â{ },
> Â};
> ÂMODULE_DEVICE_TABLE(dmi, samsung_dmi_table);
> @@ -1456,6 +1517,7 @@ static int __init samsung_init(void)
> Â Â Â Âstruct samsung_laptop *samsung;
> Â Â Â Âint ret;
>
> + Â Â Â quirks = &samsung_unknown;
> Â Â Â Âif (!force && !dmi_check_system(samsung_dmi_table))
> Â Â Â Â Â Â Â Âreturn -ENODEV;
>
> @@ -1465,12 +1527,21 @@ static int __init samsung_init(void)
>
> Â Â Â Âmutex_init(&samsung->sabi_mutex);
> Â Â Â Âsamsung->handle_backlight = true;
> + Â Â Â samsung->quirks = quirks;
>
> -#ifdef CONFIG_ACPI
> +
> +#if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
> Â Â Â Â/* Don't handle backlight here if the acpi video already handle it */
> - Â Â Â if (acpi_video_backlight_support())
> - Â Â Â Â Â Â Â samsung->handle_backlight = false;
> + Â Â Â if (acpi_video_backlight_support()) {
> + Â Â Â Â Â Â Â if (samsung->quirks->broken_acpi_video) {
> + Â Â Â Â Â Â Â Â Â Â Â pr_info("Disabling ACPI video driver\n");
> + Â Â Â Â Â Â Â Â Â Â Â acpi_video_unregister();
> + Â Â Â Â Â Â Â } else {
> + Â Â Â Â Â Â Â Â Â Â Â samsung->handle_backlight = false;
> + Â Â Â Â Â Â Â }
> + Â Â Â }
> Â#endif
> +
> Â Â Â Âret = samsung_platform_init(samsung);
> Â Â Â Âif (ret)
> Â Â Â Â Â Â Â Âgoto error_platform;
> @@ -1481,7 +1552,8 @@ static int __init samsung_init(void)
>
> Â#ifdef CONFIG_ACPI
> Â Â Â Â/* Only log that if we are really on a sabi platform */
> - Â Â Â if (acpi_video_backlight_support())
> + Â Â Â if (acpi_video_backlight_support() &&
> + Â Â Â Â Â !samsung->quirks->broken_acpi_video)
> Â Â Â Â Â Â Â Âpr_info("Backlight controlled by ACPI video driver\n");
> Â#endif
>
> --
> 1.7.3.4
>

Matthew, is this one ok ? I really hope this patch can go in 3.4 so we
don't introduce a regression for old laptops.
Thanks,

-- 
Corentin Chary
http://xf.iksaif.net
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/