Instead of adding a big blacklist in video_detect.c to set
ACPI_VIDEO_BACKLIGHT_DMI_VENDOR correctly, let external modules
promote or demote themselves when they know the generic video
module won't work.
Currently drivers where using acpi_video_unregister() directly
but:
- That didn't respect any acpi_backlight=[video|vendor] parameter
provided by the user.
- Any later call to acpi_video_register() would still re-load the
generic video module (and some gpu drivers are doing that).
This patch fix those two issues.
Signed-off-by: Corentin Chary <[email protected]>
---
drivers/acpi/video_detect.c | 31 +++++++++++++++++++++++++++++--
include/linux/acpi.h | 10 ++++++++++
2 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 45d8097..942fa2a 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -182,8 +182,7 @@ long acpi_video_get_capabilities(acpi_handle graphics_handle)
}
EXPORT_SYMBOL(acpi_video_get_capabilities);
-/* Returns true if video.ko can do backlight switching */
-int acpi_video_backlight_support(void)
+static void acpi_video_caps_check(void)
{
/*
* We must check whether the ACPI graphics device is physically plugged
@@ -191,6 +190,34 @@ int acpi_video_backlight_support(void)
*/
if (!acpi_video_caps_checked)
acpi_video_get_capabilities(NULL);
+}
+
+/* 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
+ * 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;
+}
+EXPORT_SYMBOL(acpi_video_dmi_promote_vendor);
+
+/* To be called when a driver who previously promoted the vendor
+ * interface */
+void acpi_video_dmi_demote_vendor(void)
+{
+ acpi_video_caps_check();
+ acpi_video_support &= ~ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
+}
+EXPORT_SYMBOL(acpi_video_dmi_demote_vendor);
+
+/* 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)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index f421dd8..27ab201 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -190,6 +190,8 @@ extern bool wmi_has_guid(const char *guid);
extern long acpi_video_get_capabilities(acpi_handle graphics_dev_handle);
extern long acpi_is_video_device(struct acpi_device *device);
+extern void acpi_video_dmi_promote_vendor(void);
+extern void acpi_video_dmi_demote_vendor(void);
extern int acpi_video_backlight_support(void);
extern int acpi_video_display_switch_support(void);
@@ -205,6 +207,14 @@ static inline long acpi_is_video_device(struct acpi_device *device)
return 0;
}
+static inline void acpi_video_dmi_promote_vendor(void)
+{
+}
+
+static inline void acpi_video_dmi_demote_vendor(void)
+{
+}
+
static inline int acpi_video_backlight_support(void)
{
return 0;
--
1.7.9.5
Hi Corentin,
On Wed, Jun 13, 2012 at 09:32:01AM +0200, Corentin Chary wrote:
...
> +/* 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
> + * 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;
> +}
> +EXPORT_SYMBOL(acpi_video_dmi_promote_vendor);
I think having the promote_vendor() function do the sanity check on the
acpi_backlight parameter and the unregistering of the acpi_video device
may make the code cleaner and more acpi_video-agnostic in the drivers.
I.e. (untested sample code):
bool acpi_video_promote_vendor(void)
{
if (acpi_video_backlight_support())
return false;
acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
pr_info("Disabling ACPI video driver\n");
acpi_video_unregister();
return true;
}
EXPORT_SYMBOL(acpi_video_promote_vendor);
and in the drivers you do
if (my_drv->broken_acpi_video) {
if (acpi_video_promote_vendor())
do_backlight_init();
} else if (!acpi_video_backlight_support())
do_backlight_init();
or something along these lines.
If you give a boolean parameter to acpi_video_promote_vendor to force
vendor backlight we could make the drivers' code even simpler but that
would change the semantics of the "promotion" to something more of a
"take-over".
PS: I will need to promote backlight control in sony-laptop.ko
eventually as well but I don't have a DMI based list but rather I should
look at a specific handle (or say Method) presence in the SNC Device.
I guess I'm just bothered by the function naming here but not a big
deal. :)
--
mattia
:wq!
On Wed, Jun 27, 2012 at 12:19 AM, Mattia Dongili <[email protected]> wrote:
> Hi Corentin,
>
> On Wed, Jun 13, 2012 at 09:32:01AM +0200, Corentin Chary wrote:
> ...
>> +/* 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
>> + * 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;
>> +}
>> +EXPORT_SYMBOL(acpi_video_dmi_promote_vendor);
>
> I think having the promote_vendor() function do the sanity check on the
> acpi_backlight parameter and the unregistering of the acpi_video device
> may make the code cleaner and more acpi_video-agnostic in the drivers.
> I.e. (untested sample code):
>
> bool acpi_video_promote_vendor(void)
> {
> if (acpi_video_backlight_support())
> return false;
>
> acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
> pr_info("Disabling ACPI video driver\n");
> acpi_video_unregister();
> return true;
> }
> EXPORT_SYMBOL(acpi_video_promote_vendor);
>
> and in the drivers you do
>
> if (my_drv->broken_acpi_video) {
> if (acpi_video_promote_vendor())
> do_backlight_init();
> } else if (!acpi_video_backlight_support())
> do_backlight_init();
>
> or something along these lines.
> If you give a boolean parameter to acpi_video_promote_vendor to force
> vendor backlight we could make the drivers' code even simpler but that
> would change the semantics of the "promotion" to something more of a
> "take-over".
>
> PS: I will need to promote backlight control in sony-laptop.ko
> eventually as well but I don't have a DMI based list but rather I should
> look at a specific handle (or say Method) presence in the SNC Device.
> I guess I'm just bothered by the function naming here but not a big
> deal. :)
> --
> mattia
> :wq!
I had something like that in mind, but it makes acpi.ko use symbols
from video.ko, and video.ko already uses symbols from acpi.ko, and I
didn't want to mess with circular dependencies. But if you manage to
make it work, and if you want to remove dmi_, I'll hapilly Ack it !
--
Corentin Chary
http://xf.iksaif.net