[PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic
Hans de Goede
hdegoede at redhat.com
Thu Jun 11 02:19:20 PDT 2015
Hi,
On 11-06-15 11:00, Aaron Lu wrote:
> On Wed, Jun 10, 2015 at 03:01:07PM +0200, Hans de Goede wrote:
>> Currently we have 2 kernel commandline options + dmi-quirks in 3 places all
>> interacting (in interesting ways) to select which which backlight interface
>> to use. On the commandline we've acpi_backlight=[video|vendor] and
>> video.use_native_backlight=[0|1]. DMI quirks we have in
>> acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c .
>>
>> This commit is the first step to cleaning this up, replacing the 2 cmdline
>> options with just acpi_video.backlight=[video|vendor|native|none], and
>> adding a new API to video_detect.c to reflect this.
>
> I like backlight=[firmware|platform|raw|none] better, but that would
> require to put the selection/quirk logic to backlight core. What do you
> think?
I think you are asking 3 separate questions:
1) Should the backlight selection logic maybe be moved to the backlight
class code ?
My answer to that one is: no it should not as it is x86/acpi specific.
2) What would be a good kernel cmdline option name for this?
Given the answer to one I think it is good to keep acpi in the name
space of the cmdline option. Also note that moving the code to the
backlight class code does not magically give us a backlight= cmdline
option. None module-name prefixed cmdline options are only available
to code which is always builtin, and the backlight class code can be
build as a module, so putting the cmdline option in the backlight
class will give us: backlight.backlight=
So all in all I think acpi_video.backlight= is actually pretty ok.
3) What would be a good kernel cmdline option values for this?
I agree that the old vendor/video/native value labels are not
necessarily the best labels for this, but I would like to keep
them as those are what people are used to. Ideally I would like
to have just kept acpi_backlight=[video|vendor|native|none]
but that is not possible without making all of the acpi_video
code always builtin.
Regards,
Hans
>> Follow up commits will also move other related code, like unregistering the
>> acpi_video backlight interface if it was registered before other drivers
>> which take priority over it are loaded, to video_detect.c where this
>> logic really belongs.
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>> drivers/acpi/video_detect.c | 172 +++++++++++++++++++++-----------------------
>> include/acpi/video.h | 17 +++++
>> 2 files changed, 100 insertions(+), 89 deletions(-)
>>
>> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
>> index 0bc8b98..01c8c46 100644
>> --- a/drivers/acpi/video_detect.c
>> +++ b/drivers/acpi/video_detect.c
>> @@ -1,4 +1,6 @@
>> /*
>> + * Copyright (C) 2015 Red Hat Inc.
>> + * Hans de Goede <hdegoede at redhat.com>
>> * Copyright (C) 2008 SuSE Linux Products GmbH
>> * Thomas Renninger <trenn at suse.de>
>> *
>> @@ -9,28 +11,24 @@
>> * acpi_get_pci_dev() can be called to identify ACPI graphics
>> * devices for which a real graphics card is plugged in
>> *
>> - * Now acpi_video_get_capabilities() can be called to check which
>> - * capabilities the graphics cards plugged in support. The check for general
>> - * video capabilities will be triggered by the first caller of
>> - * acpi_video_get_capabilities(NULL); which will happen when the first
>> - * backlight switching supporting driver calls:
>> - * acpi_video_backlight_support();
>> - *
>> * Depending on whether ACPI graphics extensions (cmp. ACPI spec Appendix B)
>> * are available, video.ko should be used to handle the device.
>> *
>> * Otherwise vendor specific drivers like thinkpad_acpi, asus-laptop,
>> * sony_acpi,... can take care about backlight brightness.
>> *
>> - * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m)
>> - * this file will not be compiled, acpi_video_get_capabilities() and
>> - * acpi_video_backlight_support() will always return 0 and vendor specific
>> - * drivers always can handle backlight.
>> + * Backlight drivers can use acpi_video_get_backlight_type() to determine
>> + * which driver should handle the backlight.
>> *
>> + * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m)
>> + * this file will not be compiled and acpi_video_get_backlight_type() will
>> + * always return acpi_backlight_vendor.
>> */
>>
>> +#include <acpi/video.h>
>> #include <linux/export.h>
>> #include <linux/acpi.h>
>> +#include <linux/backlight.h>
>> #include <linux/dmi.h>
>> #include <linux/module.h>
>> #include <linux/pci.h>
>> @@ -38,20 +36,24 @@
>> ACPI_MODULE_NAME("video");
>> #define _COMPONENT ACPI_VIDEO_COMPONENT
>>
>> -static long acpi_video_support;
>> -static bool acpi_video_caps_checked;
>> +static enum acpi_backlight_type acpi_backlight_cmdline = acpi_backlight_undef;
>> +static enum acpi_backlight_type acpi_backlight_dmi = acpi_backlight_undef;
>>
>> static char acpi_backlight_str[16];
>> module_param_string(backlight, acpi_backlight_str, 16, 0444);
>> MODULE_PARM_DESC(backlight,
>> - "Select which backlight interface to use [vendor|video]");
>> + "Select which backlight interface to use [vendor|video|native]");
>>
>> static void acpi_video_parse_cmdline(void)
>> {
>> if (!strcmp("vendor", acpi_backlight_str))
>> - acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
>> + acpi_backlight_cmdline = acpi_backlight_vendor;
>> if (!strcmp("video", acpi_backlight_str))
>> - acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO;
>> + acpi_backlight_cmdline = acpi_backlight_video;
>> + if (!strcmp("native", acpi_backlight_str))
>> + acpi_backlight_cmdline = acpi_backlight_native;
>> + if (!strcmp("none", acpi_backlight_str))
>> + acpi_backlight_cmdline = acpi_backlight_none;
>> }
>>
>> static acpi_status
>> @@ -82,7 +84,7 @@ find_video(acpi_handle handle, u32 lvl, void *context, void **rv)
>> * buggy */
>> static int video_detect_force_vendor(const struct dmi_system_id *d)
>> {
>> - acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
>> + acpi_backlight_dmi = acpi_backlight_vendor;
>> return 0;
>> }
>>
>> @@ -130,99 +132,91 @@ static struct dmi_system_id video_detect_dmi_table[] = {
>> };
>>
>> /*
>> - * Returns the video capabilities of a specific ACPI graphics device
>> + * Determine which type of backlight interface to use on this system,
>> + * First check cmdline, then dmi quirks, then do autodetect.
>> + *
>> + * The autodetect order is:
>> + * 1) Is the acpi-video backlight interface supported ->
>> + * no, use a vendor interface
>> + * 2) Is this a win8 "ready" BIOS and do we have a native interface ->
>> + * yes, use a native interface
>> + * 3) Else use the acpi-video interface
>> *
>> - * if NULL is passed as argument all ACPI devices are enumerated and
>> - * all graphics capabilities of physically present devices are
>> - * summarized and returned. This is cached and done only once.
>> + * Arguably the native on win8 check should be done first, but that would
>> + * be a behavior change, which may causes issues.
>> */
>> -static long acpi_video_get_capabilities(acpi_handle graphics_handle)
>> +enum acpi_backlight_type acpi_video_get_backlight_type(void)
>> {
>> - long caps = 0;
>> - struct acpi_device *tmp_dev;
>> - acpi_status status;
>> -
>> - if (acpi_video_caps_checked && graphics_handle == NULL)
>> - return acpi_video_support;
>> -
>> - if (!graphics_handle) {
>> - /* Only do the global walk through all graphics devices once */
>> - acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
>> - ACPI_UINT32_MAX, find_video, NULL,
>> - &caps, NULL);
>> - /* There might be boot param flags set already... */
>> - acpi_video_support |= caps;
>> - acpi_video_caps_checked = 1;
>> - /* Add blacklists here. Be careful to use the right *DMI* bits
>> - * to still be able to override logic via boot params, e.g.:
>> - *
>> - * if (dmi_name_in_vendors("XY")) {
>> - * acpi_video_support |=
>> - * ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
>> - *}
>> - */
>> + static DEFINE_MUTEX(init_mutex);
>> + static bool init_done;
>> + static long video_caps;
>>
>> + /* Parse cmdline, dmi and acpi only once */
>> + mutex_lock(&init_mutex);
>> + if (!init_done) {
>> + acpi_video_parse_cmdline();
>> dmi_check_system(video_detect_dmi_table);
>> - } else {
>> - status = acpi_bus_get_device(graphics_handle, &tmp_dev);
>> - if (ACPI_FAILURE(status)) {
>> - ACPI_EXCEPTION((AE_INFO, status, "Invalid device"));
>> - return 0;
>> - }
>> - acpi_walk_namespace(ACPI_TYPE_DEVICE, graphics_handle,
>> + acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
>> ACPI_UINT32_MAX, find_video, NULL,
>> - &caps, NULL);
>> + &video_caps, NULL);
>> + init_done = true;
>> }
>> - ACPI_DEBUG_PRINT((ACPI_DB_INFO, "We have 0x%lX video support %s %s\n",
>> - graphics_handle ? caps : acpi_video_support,
>> - graphics_handle ? "on device " : "in general",
>> - graphics_handle ? acpi_device_bid(tmp_dev) : ""));
>> - return caps;
>> + mutex_unlock(&init_mutex);
>> +
>> + if (acpi_backlight_cmdline != acpi_backlight_undef)
>> + return acpi_backlight_cmdline;
>> +
>> + if (acpi_backlight_dmi != acpi_backlight_undef)
>> + return acpi_backlight_dmi;
>> +
>> + if (!(video_caps & ACPI_VIDEO_BACKLIGHT))
>> + return acpi_backlight_vendor;
>> +
>> + if (acpi_osi_is_win8() && backlight_device_registered(BACKLIGHT_RAW))
>> + return acpi_backlight_native;
>> +
>> + return acpi_backlight_video;
>> }
>> +EXPORT_SYMBOL(acpi_video_get_backlight_type);
>>
>> -static void acpi_video_caps_check(void)
>> +/*
>> + * Set the preferred backlight interface type based on DMI info.
>> + * This function allows DMI blacklists to be implemented by external
>> + * platform drivers instead of putting a big blacklist in video_detect.c
>> + */
>> +void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type)
>> {
>> - /*
>> - * We must check whether the ACPI graphics device is physically plugged
>> - * in. Therefore this must be called after binding PCI and ACPI devices
>> - */
>> - if (!acpi_video_caps_checked) {
>> - acpi_video_parse_cmdline();
>> - acpi_video_get_capabilities(NULL);
>> - }
>> + acpi_backlight_dmi = type;
>> }
>> +EXPORT_SYMBOL(acpi_video_set_dmi_backlight_type);
>>
>> -/* Promote the vendor interface instead of the generic video module.
>> - * This function allow DMI blacklists to be implemented by externals
>> - * platform drivers instead of putting a big blacklist in video_detect.c
>> +/*
>> + * Compatiblity function, this is going away as soon as all drivers are
>> + * converted to acpi_video_set_dmi_backlight_type().
>> + *
>> + * Promote the vendor interface instead of the generic video module.
>> * After calling this function you will probably want to call
>> * acpi_video_unregister() to make sure the video module is not loaded
>> */
>> void acpi_video_dmi_promote_vendor(void)
>> {
>> - acpi_video_caps_check();
>> - acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
>> + acpi_video_set_dmi_backlight_type(acpi_backlight_vendor);
>> }
>> EXPORT_SYMBOL(acpi_video_dmi_promote_vendor);
>>
>> -/* Returns true if video.ko can do backlight switching */
>> +/*
>> + * Compatiblity function, this is going away as soon as all drivers are
>> + * converted to acpi_video_get_backlight_type().
>> + *
>> + * Returns true if video.ko can do backlight switching.
>> + */
>> int acpi_video_backlight_support(void)
>> {
>> - acpi_video_caps_check();
>> -
>> - /* First check for boot param -> highest prio */
>> - if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR)
>> - return 0;
>> - else if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO)
>> - return 1;
>> -
>> - /* Then check for DMI blacklist -> second highest prio */
>> - if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_DMI_VENDOR)
>> - return 0;
>> - else if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_DMI_VIDEO)
>> - return 1;
>> -
>> - /* Then go the default way */
>> - return acpi_video_support & ACPI_VIDEO_BACKLIGHT;
>> + /*
>> + * This is done this way since vendor drivers call this to see
>> + * if they should load, and we do not want them to load for both
>> + * the acpi_backlight_video and acpi_backlight_native cases.
>> + */
>> + return acpi_video_get_backlight_type() != acpi_backlight_vendor;
>> }
>> EXPORT_SYMBOL(acpi_video_backlight_support);
>> diff --git a/include/acpi/video.h b/include/acpi/video.h
>> index 843ef1a..01b5cc7 100644
>> --- a/include/acpi/video.h
>> +++ b/include/acpi/video.h
>> @@ -16,6 +16,14 @@ struct acpi_device;
>> #define ACPI_VIDEO_DISPLAY_LEGACY_PANEL 0x0110
>> #define ACPI_VIDEO_DISPLAY_LEGACY_TV 0x0200
>>
>> +enum acpi_backlight_type {
>> + acpi_backlight_undef = -1,
>> + acpi_backlight_none = 0,
>> + acpi_backlight_video,
>> + acpi_backlight_vendor,
>> + acpi_backlight_native,
>> +};
>> +
>> #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
>> extern int acpi_video_register(void);
>> extern void acpi_video_unregister(void);
>> @@ -23,6 +31,8 @@ extern void acpi_video_unregister_backlight(void);
>> extern int acpi_video_get_edid(struct acpi_device *device, int type,
>> int device_id, void **edid);
>> extern bool acpi_video_verify_backlight_support(void);
>> +extern enum acpi_backlight_type acpi_video_get_backlight_type(void);
>> +extern void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type);
>> #else
>> static inline int acpi_video_register(void) { return 0; }
>> static inline void acpi_video_unregister(void) { return; }
>> @@ -33,6 +43,13 @@ static inline int acpi_video_get_edid(struct acpi_device *device, int type,
>> return -ENODEV;
>> }
>> static inline bool acpi_video_verify_backlight_support(void) { return false; }
>> +static inline enum acpi_backlight_type acpi_video_get_backlight_type(void)
>> +{
>> + return acpi_backlight_vendor;
>> +}
>> +static void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type)
>> +{
>> +}
>> #endif
>>
>> #endif
>> --
>> 2.4.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
More information about the dri-devel
mailing list