[PATCH resend 0/4] Make video.use_native_backlight=1 work properly with nouveau

[PATCH resend 0/4] Make video.use_native_backlight=1 work properly with nouveau

[Hans de Goede]

Hi All,

I know it has not been that long since the last send of this series, but
it has been very quiet, and I would like to see some discussion on it
(or it being applied at once, that is fine too :)

This patch-set adds backlight device (un)registration notification and
makes acpi-video listen to it, so that video.use_native_backlight=1 still
works if a raw interface gets loaded *after* acpi-video has been initialized.

It also changes nouveau to always register its raw interface, as all the other
kms drivers do, acpi_video_backlight_support() is only intended to avoid the
loading of multiple (possibly conflicting) firmware drivers, not to avoid
loading raw drivers.

In the mean time I've gotten feedback from a user with a laptop which needs
video.use_native_backlight=1 and uses nouveau, and he has confirmed that this
patch-set works as advertised, see:
https://bugzilla.redhat.com/show_bug.cgi?id\x1093171

Regards,

Hans

[PATCH resend 1/4] nouveau: Don't check acpi_video_backlight_support() before registering backlight

[Hans de Goede]

acpi_video_backlight_support() is supposed to be called by other (vendor
specific) firmware backlight controls, not by native / raw backlight controls
like nv_backlight.

Userspace will normally prefer firmware interfaces over raw interfaces, so
if acpi_video backlight support is present it will use that even if
nv_backlight is registered as well.

Except when video.use_native_backlight is present on the kernel cmdline
(or enabled through a dmi based quirk). As the name indicates the goal here
is to make only the raw interface available to userspace so that it will use
that (it only does this when it sees a win8 compliant bios).

This is done by:
1) Not registering any acpi_video# backlight devices; and
2) Making acpi_video_backlight_support() return true so that other firmware
drivers, ie acer_wmi, thinkpad_acpi, dell_laptop, etc. Don't register their
own vender specific interfaces.

Currently nouveau breaks this setup, as when acpi_video_backlight_support()
returns true, it does not register itself, resulting in no backlight control
at all.

This is esp. going to be a problem with 3.16 which will default to
video.use_native_backlight=1, and thus nouveau based laptops with a win8 bios
will get no backlight control at all.

This also likely explains why the previous attempt to make
video.use_native_backlight=1 the default was not a success, as without this
patch having a default of video.use_native_backlight=1 will cause regressions.

Note this effectively reverts commit 5bead799

Also see: https://bugzilla.redhat.com/show_bug.cgi?id\x1093171

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/nouveau/nouveau_backlight.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index 630f6e8..2c1e4aa 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -31,7 +31,6 @@
  */
 
 #include <linux/backlight.h>
-#include <linux/acpi.h>
 
 #include "nouveau_drm.h"
 #include "nouveau_reg.h"
@@ -222,14 +221,6 @@ nouveau_backlight_init(struct drm_device *dev)
 	struct nouveau_device *device = nv_device(drm->device);
 	struct drm_connector *connector;
 
-#ifdef CONFIG_ACPI
-	if (acpi_video_backlight_support()) {
-		NV_INFO(drm, "ACPI backlight interface available, "
-			     "not registering our own\n");
-		return 0;
-	}
-#endif
-
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 		if (connector->connector_type != DRM_MODE_CONNECTOR_LVDS &&
 		    connector->connector_type != DRM_MODE_CONNECTOR_eDP)
-- 
1.9.0

[PATCH resend 2/4] backlight: Add backlight device (un)registration notification

[Hans de Goede]

Some firmware drivers, ie acpi-video want to get themselves out of the
way (in some cases) when their also is a raw backlight device available.

Due to module loading ordering being unknown, acpi-video cannot be certain
that the backlight_device_registered(BACKLIGHT_RAW) it does for this is
the final verdict wrt there being a BACKLIGHT_RAW device.

By adding notification acpi-video can listen for backlight devices showing
up after it has loaded, and unregister its backlight device if desired.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/backlight/backlight.c | 40 +++++++++++++++++++++++++++++++++++++
 include/linux/backlight.h           |  7 +++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index bd2172c..4280890 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -23,6 +23,7 @@
 
 static struct list_head backlight_dev_list;
 static struct mutex backlight_dev_list_mutex;
+static struct blocking_notifier_head backlight_notifier;
 
 static const char *const backlight_types[] = {
 	[BACKLIGHT_RAW] = "raw",
@@ -370,6 +371,9 @@ struct backlight_device *backlight_device_register(const char *name,
 	list_add(&new_bd->entry, &backlight_dev_list);
 	mutex_unlock(&backlight_dev_list_mutex);
 
+	blocking_notifier_call_chain(&backlight_notifier,
+				     BACKLIGHT_REGISTERED, new_bd);
+
 	return new_bd;
 }
 EXPORT_SYMBOL(backlight_device_register);
@@ -413,6 +417,10 @@ void backlight_device_unregister(struct backlight_device *bd)
 		pmac_backlight = NULL;
 	mutex_unlock(&pmac_backlight_mutex);
 #endif
+
+	blocking_notifier_call_chain(&backlight_notifier,
+				     BACKLIGHT_UNREGISTERED, bd);
+
 	mutex_lock(&bd->ops_lock);
 	bd->ops = NULL;
 	mutex_unlock(&bd->ops_lock);
@@ -438,6 +446,36 @@ static int devm_backlight_device_match(struct device *dev, void *res,
 }
 
 /**
+ * backlight_register_notifier - get notified of backlight (un)registration
+ * @nb: notifier block with the notifier to call on backlight (un)registration
+ *
+ * @return 0 on success, otherwise a negative error code
+ *
+ * Register a notifier to get notified when backlight devices get registered
+ * or unregistered.
+ */
+int backlight_register_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&backlight_notifier, nb);
+}
+EXPORT_SYMBOL(backlight_register_notifier);
+
+/**
+ * backlight_unregister_notifier - unregister a backlight notifier
+ * @nb: notifier block to unregister
+ *
+ * @return 0 on success, otherwise a negative error code
+ *
+ * Register a notifier to get notified when backlight devices get registered
+ * or unregistered.
+ */
+int backlight_unregister_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&backlight_notifier, nb);
+}
+EXPORT_SYMBOL(backlight_unregister_notifier);
+
+/**
  * devm_backlight_device_register - resource managed backlight_device_register()
  * @dev: the device to register
  * @name: the name of the device
@@ -544,6 +582,8 @@ static int __init backlight_class_init(void)
 	backlight_class->pm = &backlight_class_dev_pm_ops;
 	INIT_LIST_HEAD(&backlight_dev_list);
 	mutex_init(&backlight_dev_list_mutex);
+	BLOCKING_INIT_NOTIFIER_HEAD(&backlight_notifier);
+
 	return 0;
 }
 
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 7264742..adb14a8 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -40,6 +40,11 @@ enum backlight_type {
 	BACKLIGHT_TYPE_MAX,
 };
 
+enum backlight_notification {
+	BACKLIGHT_REGISTERED,
+	BACKLIGHT_UNREGISTERED,
+};
+
 struct backlight_device;
 struct fb_info;
 
@@ -133,6 +138,8 @@ extern void devm_backlight_device_unregister(struct device *dev,
 extern void backlight_force_update(struct backlight_device *bd,
 				   enum backlight_update_reason reason);
 extern bool backlight_device_registered(enum backlight_type type);
+extern int backlight_register_notifier(struct notifier_block *nb);
+extern int backlight_unregister_notifier(struct notifier_block *nb);
 
 #define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)
 
-- 
1.9.0

[PATCH resend 3/4] acpi-video: Unregister the backlight device if a raw one shows up later

[Hans de Goede]

When video.use_native_backlight=1 and non intel gfx are in use, the raw
backlight device of the gfx driver will show up after acpi-video has done its
acpi_video_verify_backlight_support() check.

This causes video.use_native_backlight=1 to not have the desired result.

This patch fixes this by adding a backlight notifier and when a raw
backlight is registered or unregistered re-doing the
acpi_video_verify_backlight_support() check.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/video.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index ba6e4d7..47b7e21 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -151,6 +151,7 @@ struct acpi_video_enumerated_device {
 struct acpi_video_bus {
 	struct acpi_device *device;
 	bool backlight_registered;
+	bool backlight_notifier_registered;
 	u8 dos_setting;
 	struct acpi_video_enumerated_device *attached_array;
 	u8 attached_count;
@@ -162,6 +163,7 @@ struct acpi_video_bus {
 	struct input_dev *input;
 	char phys[32];	/* for input device */
 	struct notifier_block pm_nb;
+	struct notifier_block backlight_nb;
 };
 
 struct acpi_video_device_flags {
@@ -1828,6 +1830,9 @@ static int acpi_video_bus_register_backlight(struct acpi_video_bus *video)
 {
 	struct acpi_video_device *dev;
 
+	if (video->backlight_registered)
+		return 0;
+
 	if (!acpi_video_verify_backlight_support())
 		return 0;
 
@@ -1972,6 +1977,56 @@ static void acpi_video_bus_remove_notify_handler(struct acpi_video_bus *video)
 	video->input = NULL;
 }
 
+static int acpi_video_backlight_notify(struct notifier_block *nb,
+					unsigned long val, void *bd)
+{
+	struct backlight_device *backlight = bd;
+	struct acpi_video_bus *video;
+
+	/* acpi_video_verify_backlight_support only cares about raw devices */
+	if (backlight->props.type != BACKLIGHT_RAW)
+		return NOTIFY_DONE;
+
+	video = container_of(nb, struct acpi_video_bus, backlight_nb);
+
+	switch (val) {
+	case BACKLIGHT_REGISTERED:
+		if (!acpi_video_verify_backlight_support())
+			acpi_video_bus_unregister_backlight(video);
+		break;
+	case BACKLIGHT_UNREGISTERED:
+		acpi_video_bus_register_backlight(video);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static int acpi_video_bus_add_backlight_notify_handler(
+						struct acpi_video_bus *video)
+{
+	int error;
+
+	video->backlight_nb.notifier_call = acpi_video_backlight_notify;
+	video->backlight_nb.priority = 0;
+	error = backlight_register_notifier(&video->backlight_nb);
+	if (error = 0)
+		video->backlight_notifier_registered = true;
+
+	return error;
+}
+
+static int acpi_video_bus_remove_backlight_notify_handler(
+						struct acpi_video_bus *video)
+{
+	if (!video->backlight_notifier_registered)
+		return 0;
+
+	video->backlight_notifier_registered = false;
+
+	return backlight_unregister_notifier(&video->backlight_nb);
+}
+
 static int acpi_video_bus_put_devices(struct acpi_video_bus *video)
 {
 	struct acpi_video_device *dev, *next;
@@ -2053,6 +2108,7 @@ static int acpi_video_bus_add(struct acpi_device *device)
 
 	acpi_video_bus_register_backlight(video);
 	acpi_video_bus_add_notify_handler(video);
+	acpi_video_bus_add_backlight_notify_handler(video);
 
 	return 0;
 
@@ -2076,6 +2132,7 @@ static int acpi_video_bus_remove(struct acpi_device *device)
 
 	video = acpi_driver_data(device);
 
+	acpi_video_bus_remove_backlight_notify_handler(video);
 	acpi_video_bus_remove_notify_handler(video);
 	acpi_video_bus_unregister_backlight(video);
 	acpi_video_bus_put_devices(video);
-- 
1.9.0

[PATCH resend 4/4] acpi-video: Add use native backlight quirk for the ThinkPad W530

[Hans de Goede]

Like all of the other *30 ThinkPad models, the W530 has a broken acpi-video
backlight control. Note in order for this to actually fix things on the
ThinkPad W530 the commit titled:
"nouveau: Don't check acpi_video_backlight_support() before registering backlight"
is also needed.

https://bugzilla.redhat.com/show_bug.cgi?id\x1093171

Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/video.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 47b7e21..8309100 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -515,6 +515,14 @@ static struct dmi_system_id video_dmi_table[] __initdata = {
 		},
 	},
 	{
+	 .callback = video_set_use_native_backlight,
+	 .ident = "ThinkPad W530",
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+		DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad W530"),
+		},
+	},
+	{
 	.callback = video_set_use_native_backlight,
 	.ident = "ThinkPad X1 Carbon",
 	.matches = {
-- 
1.9.0

Re: [PATCH resend 1/4] nouveau: Don't check acpi_video_backlight_support() before registering backli

[Rafael J. Wysocki]

On Wednesday, May 21, 2014 03:39:53 PM Hans de Goede wrote:
> acpi_video_backlight_support() is supposed to be called by other (vendor
> specific) firmware backlight controls, not by native / raw backlight controls
> like nv_backlight.
> 
> Userspace will normally prefer firmware interfaces over raw interfaces, so
> if acpi_video backlight support is present it will use that even if
> nv_backlight is registered as well.
> 
> Except when video.use_native_backlight is present on the kernel cmdline
> (or enabled through a dmi based quirk). As the name indicates the goal here
> is to make only the raw interface available to userspace so that it will use
> that (it only does this when it sees a win8 compliant bios).
> 
> This is done by:
> 1) Not registering any acpi_video# backlight devices; and
> 2) Making acpi_video_backlight_support() return true so that other firmware
> drivers, ie acer_wmi, thinkpad_acpi, dell_laptop, etc. Don't register their
> own vender specific interfaces.
> 
> Currently nouveau breaks this setup, as when acpi_video_backlight_support()
> returns true, it does not register itself, resulting in no backlight control
> at all.
> 
> This is esp. going to be a problem with 3.16 which will default to
> video.use_native_backlight=1, and thus nouveau based laptops with a win8 bios
> will get no backlight control at all.
> 
> This also likely explains why the previous attempt to make
> video.use_native_backlight=1 the default was not a success, as without this
> patch having a default of video.use_native_backlight=1 will cause regressions.
> 
> Note this effectively reverts commit 5bead799
> 
> Also see: https://bugzilla.redhat.com/show_bug.cgi?id\x1093171
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

It would be good to have an ACK from the nouveau people for this one.

> ---
>  drivers/gpu/drm/nouveau/nouveau_backlight.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index 630f6e8..2c1e4aa 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -31,7 +31,6 @@
>   */
>  
>  #include <linux/backlight.h>
> -#include <linux/acpi.h>
>  
>  #include "nouveau_drm.h"
>  #include "nouveau_reg.h"
> @@ -222,14 +221,6 @@ nouveau_backlight_init(struct drm_device *dev)
>  	struct nouveau_device *device = nv_device(drm->device);
>  	struct drm_connector *connector;
>  
> -#ifdef CONFIG_ACPI
> -	if (acpi_video_backlight_support()) {
> -		NV_INFO(drm, "ACPI backlight interface available, "
> -			     "not registering our own\n");
> -		return 0;
> -	}
> -#endif
> -
>  	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>  		if (connector->connector_type != DRM_MODE_CONNECTOR_LVDS &&
>  		    connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH resend 2/4] backlight: Add backlight device (un)registration notification

[Rafael J. Wysocki]

On Wednesday, May 21, 2014 03:39:54 PM Hans de Goede wrote:
> Some firmware drivers, ie acpi-video want to get themselves out of the
> way (in some cases) when their also is a raw backlight device available.
> 
> Due to module loading ordering being unknown, acpi-video cannot be certain
> that the backlight_device_registered(BACKLIGHT_RAW) it does for this is
> the final verdict wrt there being a BACKLIGHT_RAW device.
> 
> By adding notification acpi-video can listen for backlight devices showing
> up after it has loaded, and unregister its backlight device if desired.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Backlight maintainer's ACK is requisite here.

> ---
>  drivers/video/backlight/backlight.c | 40 +++++++++++++++++++++++++++++++++++++
>  include/linux/backlight.h           |  7 +++++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index bd2172c..4280890 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -23,6 +23,7 @@
>  
>  static struct list_head backlight_dev_list;
>  static struct mutex backlight_dev_list_mutex;
> +static struct blocking_notifier_head backlight_notifier;
>  
>  static const char *const backlight_types[] = {
>  	[BACKLIGHT_RAW] = "raw",
> @@ -370,6 +371,9 @@ struct backlight_device *backlight_device_register(const char *name,
>  	list_add(&new_bd->entry, &backlight_dev_list);
>  	mutex_unlock(&backlight_dev_list_mutex);
>  
> +	blocking_notifier_call_chain(&backlight_notifier,
> +				     BACKLIGHT_REGISTERED, new_bd);
> +
>  	return new_bd;
>  }
>  EXPORT_SYMBOL(backlight_device_register);
> @@ -413,6 +417,10 @@ void backlight_device_unregister(struct backlight_device *bd)
>  		pmac_backlight = NULL;
>  	mutex_unlock(&pmac_backlight_mutex);
>  #endif
> +
> +	blocking_notifier_call_chain(&backlight_notifier,
> +				     BACKLIGHT_UNREGISTERED, bd);
> +
>  	mutex_lock(&bd->ops_lock);
>  	bd->ops = NULL;
>  	mutex_unlock(&bd->ops_lock);
> @@ -438,6 +446,36 @@ static int devm_backlight_device_match(struct device *dev, void *res,
>  }
>  
>  /**
> + * backlight_register_notifier - get notified of backlight (un)registration
> + * @nb: notifier block with the notifier to call on backlight (un)registration
> + *
> + * @return 0 on success, otherwise a negative error code
> + *
> + * Register a notifier to get notified when backlight devices get registered
> + * or unregistered.
> + */
> +int backlight_register_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&backlight_notifier, nb);
> +}
> +EXPORT_SYMBOL(backlight_register_notifier);
> +
> +/**
> + * backlight_unregister_notifier - unregister a backlight notifier
> + * @nb: notifier block to unregister
> + *
> + * @return 0 on success, otherwise a negative error code
> + *
> + * Register a notifier to get notified when backlight devices get registered
> + * or unregistered.
> + */
> +int backlight_unregister_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&backlight_notifier, nb);
> +}
> +EXPORT_SYMBOL(backlight_unregister_notifier);
> +
> +/**
>   * devm_backlight_device_register - resource managed backlight_device_register()
>   * @dev: the device to register
>   * @name: the name of the device
> @@ -544,6 +582,8 @@ static int __init backlight_class_init(void)
>  	backlight_class->pm = &backlight_class_dev_pm_ops;
>  	INIT_LIST_HEAD(&backlight_dev_list);
>  	mutex_init(&backlight_dev_list_mutex);
> +	BLOCKING_INIT_NOTIFIER_HEAD(&backlight_notifier);
> +
>  	return 0;
>  }
>  
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 7264742..adb14a8 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -40,6 +40,11 @@ enum backlight_type {
>  	BACKLIGHT_TYPE_MAX,
>  };
>  
> +enum backlight_notification {
> +	BACKLIGHT_REGISTERED,
> +	BACKLIGHT_UNREGISTERED,
> +};
> +
>  struct backlight_device;
>  struct fb_info;
>  
> @@ -133,6 +138,8 @@ extern void devm_backlight_device_unregister(struct device *dev,
>  extern void backlight_force_update(struct backlight_device *bd,
>  				   enum backlight_update_reason reason);
>  extern bool backlight_device_registered(enum backlight_type type);
> +extern int backlight_register_notifier(struct notifier_block *nb);
> +extern int backlight_unregister_notifier(struct notifier_block *nb);
>  
>  #define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)
>  
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH resend 1/4] nouveau: Don't check acpi_video_backlight_support() before registering backli

[Hans de Goede]

Hi,

On 05/22/2014 01:30 AM, Rafael J. Wysocki wrote:
> On Wednesday, May 21, 2014 03:39:53 PM Hans de Goede wrote:
>> acpi_video_backlight_support() is supposed to be called by other (vendor
>> specific) firmware backlight controls, not by native / raw backlight controls
>> like nv_backlight.
>>
>> Userspace will normally prefer firmware interfaces over raw interfaces, so
>> if acpi_video backlight support is present it will use that even if
>> nv_backlight is registered as well.
>>
>> Except when video.use_native_backlight is present on the kernel cmdline
>> (or enabled through a dmi based quirk). As the name indicates the goal here
>> is to make only the raw interface available to userspace so that it will use
>> that (it only does this when it sees a win8 compliant bios).
>>
>> This is done by:
>> 1) Not registering any acpi_video# backlight devices; and
>> 2) Making acpi_video_backlight_support() return true so that other firmware
>> drivers, ie acer_wmi, thinkpad_acpi, dell_laptop, etc. Don't register their
>> own vender specific interfaces.
>>
>> Currently nouveau breaks this setup, as when acpi_video_backlight_support()
>> returns true, it does not register itself, resulting in no backlight control
>> at all.
>>
>> This is esp. going to be a problem with 3.16 which will default to
>> video.use_native_backlight=1, and thus nouveau based laptops with a win8 bios
>> will get no backlight control at all.
>>
>> This also likely explains why the previous attempt to make
>> video.use_native_backlight=1 the default was not a success, as without this
>> patch having a default of video.use_native_backlight=1 will cause regressions.
>>
>> Note this effectively reverts commit 5bead799
>>
>> Also see: https://bugzilla.redhat.com/show_bug.cgi?id\x1093171
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> It would be good to have an ACK from the nouveau people for this one.

Right, it could / should even go in through the drm tree I guess.

Regards,

Hans

Re: [PATCH resend 2/4] backlight: Add backlight device (un)registration notification

[Hans de Goede]

Hi,

On 05/22/2014 01:31 AM, Rafael J. Wysocki wrote:
> On Wednesday, May 21, 2014 03:39:54 PM Hans de Goede wrote:
>> Some firmware drivers, ie acpi-video want to get themselves out of the
>> way (in some cases) when their also is a raw backlight device available.
>>
>> Due to module loading ordering being unknown, acpi-video cannot be certain
>> that the backlight_device_registered(BACKLIGHT_RAW) it does for this is
>> the final verdict wrt there being a BACKLIGHT_RAW device.
>>
>> By adding notification acpi-video can listen for backlight devices showing
>> up after it has loaded, and unregister its backlight device if desired.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Backlight maintainer's ACK is requisite here.

Agreed, which is why I send this set to all 3 the backlight maintainers
directly on both postings.

What may be helpful for them is to hear from you if you're ok with the
acpi-video bits which are actually going to use this, since those will
be the only user of the new backlight api (for now).

Thanks & Regardsm

Hans




> 
>> ---
>>  drivers/video/backlight/backlight.c | 40 +++++++++++++++++++++++++++++++++++++
>>  include/linux/backlight.h           |  7 +++++++
>>  2 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
>> index bd2172c..4280890 100644
>> --- a/drivers/video/backlight/backlight.c
>> +++ b/drivers/video/backlight/backlight.c
>> @@ -23,6 +23,7 @@
>>  
>>  static struct list_head backlight_dev_list;
>>  static struct mutex backlight_dev_list_mutex;
>> +static struct blocking_notifier_head backlight_notifier;
>>  
>>  static const char *const backlight_types[] = {
>>  	[BACKLIGHT_RAW] = "raw",
>> @@ -370,6 +371,9 @@ struct backlight_device *backlight_device_register(const char *name,
>>  	list_add(&new_bd->entry, &backlight_dev_list);
>>  	mutex_unlock(&backlight_dev_list_mutex);
>>  
>> +	blocking_notifier_call_chain(&backlight_notifier,
>> +				     BACKLIGHT_REGISTERED, new_bd);
>> +
>>  	return new_bd;
>>  }
>>  EXPORT_SYMBOL(backlight_device_register);
>> @@ -413,6 +417,10 @@ void backlight_device_unregister(struct backlight_device *bd)
>>  		pmac_backlight = NULL;
>>  	mutex_unlock(&pmac_backlight_mutex);
>>  #endif
>> +
>> +	blocking_notifier_call_chain(&backlight_notifier,
>> +				     BACKLIGHT_UNREGISTERED, bd);
>> +
>>  	mutex_lock(&bd->ops_lock);
>>  	bd->ops = NULL;
>>  	mutex_unlock(&bd->ops_lock);
>> @@ -438,6 +446,36 @@ static int devm_backlight_device_match(struct device *dev, void *res,
>>  }
>>  
>>  /**
>> + * backlight_register_notifier - get notified of backlight (un)registration
>> + * @nb: notifier block with the notifier to call on backlight (un)registration
>> + *
>> + * @return 0 on success, otherwise a negative error code
>> + *
>> + * Register a notifier to get notified when backlight devices get registered
>> + * or unregistered.
>> + */
>> +int backlight_register_notifier(struct notifier_block *nb)
>> +{
>> +	return blocking_notifier_chain_register(&backlight_notifier, nb);
>> +}
>> +EXPORT_SYMBOL(backlight_register_notifier);
>> +
>> +/**
>> + * backlight_unregister_notifier - unregister a backlight notifier
>> + * @nb: notifier block to unregister
>> + *
>> + * @return 0 on success, otherwise a negative error code
>> + *
>> + * Register a notifier to get notified when backlight devices get registered
>> + * or unregistered.
>> + */
>> +int backlight_unregister_notifier(struct notifier_block *nb)
>> +{
>> +	return blocking_notifier_chain_unregister(&backlight_notifier, nb);
>> +}
>> +EXPORT_SYMBOL(backlight_unregister_notifier);
>> +
>> +/**
>>   * devm_backlight_device_register - resource managed backlight_device_register()
>>   * @dev: the device to register
>>   * @name: the name of the device
>> @@ -544,6 +582,8 @@ static int __init backlight_class_init(void)
>>  	backlight_class->pm = &backlight_class_dev_pm_ops;
>>  	INIT_LIST_HEAD(&backlight_dev_list);
>>  	mutex_init(&backlight_dev_list_mutex);
>> +	BLOCKING_INIT_NOTIFIER_HEAD(&backlight_notifier);
>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
>> index 7264742..adb14a8 100644
>> --- a/include/linux/backlight.h
>> +++ b/include/linux/backlight.h
>> @@ -40,6 +40,11 @@ enum backlight_type {
>>  	BACKLIGHT_TYPE_MAX,
>>  };
>>  
>> +enum backlight_notification {
>> +	BACKLIGHT_REGISTERED,
>> +	BACKLIGHT_UNREGISTERED,
>> +};
>> +
>>  struct backlight_device;
>>  struct fb_info;
>>  
>> @@ -133,6 +138,8 @@ extern void devm_backlight_device_unregister(struct device *dev,
>>  extern void backlight_force_update(struct backlight_device *bd,
>>  				   enum backlight_update_reason reason);
>>  extern bool backlight_device_registered(enum backlight_type type);
>> +extern int backlight_register_notifier(struct notifier_block *nb);
>> +extern int backlight_unregister_notifier(struct notifier_block *nb);
>>  
>>  #define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)
>>  
>>
> 

Re: [PATCH resend 2/4] backlight: Add backlight device (un)registration notification

[Lee Jones]

> >> Some firmware drivers, ie acpi-video want to get themselves out of the
> >> way (in some cases) when their also is a raw backlight device available.
> >>
> >> Due to module loading ordering being unknown, acpi-video cannot be certain
> >> that the backlight_device_registered(BACKLIGHT_RAW) it does for this is
> >> the final verdict wrt there being a BACKLIGHT_RAW device.
> >>
> >> By adding notification acpi-video can listen for backlight devices showing
> >> up after it has loaded, and unregister its backlight device if desired.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > 
> > Backlight maintainer's ACK is requisite here.
> 
> Agreed, which is why I send this set to all 3 the backlight maintainers
> directly on both postings.
> 
> What may be helpful for them is to hear from you if you're ok with the
> acpi-video bits which are actually going to use this, since those will
> be the only user of the new backlight api (for now).

I'm happy to apply any Backlight patches which have either Bryan or
Jingoo's Ack, as they are the reviewers for the BL subsystem.

> >> ---
> >>  drivers/video/backlight/backlight.c | 40 +++++++++++++++++++++++++++++++++++++
> >>  include/linux/backlight.h           |  7 +++++++
> >>  2 files changed, 47 insertions(+)
> >>
> >> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> >> index bd2172c..4280890 100644
> >> --- a/drivers/video/backlight/backlight.c
> >> +++ b/drivers/video/backlight/backlight.c
> >> @@ -23,6 +23,7 @@
> >>  
> >>  static struct list_head backlight_dev_list;
> >>  static struct mutex backlight_dev_list_mutex;
> >> +static struct blocking_notifier_head backlight_notifier;
> >>  
> >>  static const char *const backlight_types[] = {
> >>  	[BACKLIGHT_RAW] = "raw",
> >> @@ -370,6 +371,9 @@ struct backlight_device *backlight_device_register(const char *name,
> >>  	list_add(&new_bd->entry, &backlight_dev_list);
> >>  	mutex_unlock(&backlight_dev_list_mutex);
> >>  
> >> +	blocking_notifier_call_chain(&backlight_notifier,
> >> +				     BACKLIGHT_REGISTERED, new_bd);
> >> +
> >>  	return new_bd;
> >>  }
> >>  EXPORT_SYMBOL(backlight_device_register);
> >> @@ -413,6 +417,10 @@ void backlight_device_unregister(struct backlight_device *bd)
> >>  		pmac_backlight = NULL;
> >>  	mutex_unlock(&pmac_backlight_mutex);
> >>  #endif
> >> +
> >> +	blocking_notifier_call_chain(&backlight_notifier,
> >> +				     BACKLIGHT_UNREGISTERED, bd);
> >> +
> >>  	mutex_lock(&bd->ops_lock);
> >>  	bd->ops = NULL;
> >>  	mutex_unlock(&bd->ops_lock);
> >> @@ -438,6 +446,36 @@ static int devm_backlight_device_match(struct device *dev, void *res,
> >>  }
> >>  
> >>  /**
> >> + * backlight_register_notifier - get notified of backlight (un)registration
> >> + * @nb: notifier block with the notifier to call on backlight (un)registration
> >> + *
> >> + * @return 0 on success, otherwise a negative error code
> >> + *
> >> + * Register a notifier to get notified when backlight devices get registered
> >> + * or unregistered.
> >> + */
> >> +int backlight_register_notifier(struct notifier_block *nb)
> >> +{
> >> +	return blocking_notifier_chain_register(&backlight_notifier, nb);
> >> +}
> >> +EXPORT_SYMBOL(backlight_register_notifier);
> >> +
> >> +/**
> >> + * backlight_unregister_notifier - unregister a backlight notifier
> >> + * @nb: notifier block to unregister
> >> + *
> >> + * @return 0 on success, otherwise a negative error code
> >> + *
> >> + * Register a notifier to get notified when backlight devices get registered
> >> + * or unregistered.
> >> + */
> >> +int backlight_unregister_notifier(struct notifier_block *nb)
> >> +{
> >> +	return blocking_notifier_chain_unregister(&backlight_notifier, nb);
> >> +}
> >> +EXPORT_SYMBOL(backlight_unregister_notifier);
> >> +
> >> +/**
> >>   * devm_backlight_device_register - resource managed backlight_device_register()
> >>   * @dev: the device to register
> >>   * @name: the name of the device
> >> @@ -544,6 +582,8 @@ static int __init backlight_class_init(void)
> >>  	backlight_class->pm = &backlight_class_dev_pm_ops;
> >>  	INIT_LIST_HEAD(&backlight_dev_list);
> >>  	mutex_init(&backlight_dev_list_mutex);
> >> +	BLOCKING_INIT_NOTIFIER_HEAD(&backlight_notifier);
> >> +
> >>  	return 0;
> >>  }
> >>  
> >> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> >> index 7264742..adb14a8 100644
> >> --- a/include/linux/backlight.h
> >> +++ b/include/linux/backlight.h
> >> @@ -40,6 +40,11 @@ enum backlight_type {
> >>  	BACKLIGHT_TYPE_MAX,
> >>  };
> >>  
> >> +enum backlight_notification {
> >> +	BACKLIGHT_REGISTERED,
> >> +	BACKLIGHT_UNREGISTERED,
> >> +};
> >> +
> >>  struct backlight_device;
> >>  struct fb_info;
> >>  
> >> @@ -133,6 +138,8 @@ extern void devm_backlight_device_unregister(struct device *dev,
> >>  extern void backlight_force_update(struct backlight_device *bd,
> >>  				   enum backlight_update_reason reason);
> >>  extern bool backlight_device_registered(enum backlight_type type);
> >> +extern int backlight_register_notifier(struct notifier_block *nb);
> >> +extern int backlight_unregister_notifier(struct notifier_block *nb);
> >>  
> >>  #define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)
> >>  
> >>
> > 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH resend 1/4] nouveau: Don't check acpi_video_backlight_support() before registering backli

[Ben Skeggs]

On Thu, May 22, 2014 at 9:30 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Wednesday, May 21, 2014 03:39:53 PM Hans de Goede wrote:
>> acpi_video_backlight_support() is supposed to be called by other (vendor
>> specific) firmware backlight controls, not by native / raw backlight controls
>> like nv_backlight.
>>
>> Userspace will normally prefer firmware interfaces over raw interfaces, so
>> if acpi_video backlight support is present it will use that even if
>> nv_backlight is registered as well.
>>
>> Except when video.use_native_backlight is present on the kernel cmdline
>> (or enabled through a dmi based quirk). As the name indicates the goal here
>> is to make only the raw interface available to userspace so that it will use
>> that (it only does this when it sees a win8 compliant bios).
>>
>> This is done by:
>> 1) Not registering any acpi_video# backlight devices; and
>> 2) Making acpi_video_backlight_support() return true so that other firmware
>> drivers, ie acer_wmi, thinkpad_acpi, dell_laptop, etc. Don't register their
>> own vender specific interfaces.
>>
>> Currently nouveau breaks this setup, as when acpi_video_backlight_support()
>> returns true, it does not register itself, resulting in no backlight control
>> at all.
>>
>> This is esp. going to be a problem with 3.16 which will default to
>> video.use_native_backlight=1, and thus nouveau based laptops with a win8 bios
>> will get no backlight control at all.
>>
>> This also likely explains why the previous attempt to make
>> video.use_native_backlight=1 the default was not a success, as without this
>> patch having a default of video.use_native_backlight=1 will cause regressions.
>>
>> Note this effectively reverts commit 5bead799
>>
>> Also see: https://bugzilla.redhat.com/show_bug.cgi?id\x1093171
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> It would be good to have an ACK from the nouveau people for this one.
Acked-by: Ben Skeggs <bskeggs@redhat.com>

;)

>
>> ---
>>  drivers/gpu/drm/nouveau/nouveau_backlight.c | 9 ---------
>>  1 file changed, 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
>> index 630f6e8..2c1e4aa 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
>> @@ -31,7 +31,6 @@
>>   */
>>
>>  #include <linux/backlight.h>
>> -#include <linux/acpi.h>
>>
>>  #include "nouveau_drm.h"
>>  #include "nouveau_reg.h"
>> @@ -222,14 +221,6 @@ nouveau_backlight_init(struct drm_device *dev)
>>       struct nouveau_device *device = nv_device(drm->device);
>>       struct drm_connector *connector;
>>
>> -#ifdef CONFIG_ACPI
>> -     if (acpi_video_backlight_support()) {
>> -             NV_INFO(drm, "ACPI backlight interface available, "
>> -                          "not registering our own\n");
>> -             return 0;
>> -     }
>> -#endif
>> -
>>       list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>>               if (connector->connector_type != DRM_MODE_CONNECTOR_LVDS &&
>>                   connector->connector_type != DRM_MODE_CONNECTOR_eDP)
>>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH resend 2/4] backlight: Add backlight device (un)registration notification

[Jingoo Han]

On Thursday, May 22, 2014 6:02 PM, Lee Jones wrote:
> On Thursday, May 22, 2014 5:45 PM, Hans de Goede wrote:
> > On Thursday, May 22, 2014 8:31 AM, Rafael J. Wysocki wrote:
> > > On Wednesday, May 21, 2014 10:40 PM, Hans de Goede wrote:
> > >> Some firmware drivers, ie acpi-video want to get themselves out of the
> > >> way (in some cases) when their also is a raw backlight device available.
> > >>
> > >> Due to module loading ordering being unknown, acpi-video cannot be certain
> > >> that the backlight_device_registered(BACKLIGHT_RAW) it does for this is
> > >> the final verdict wrt there being a BACKLIGHT_RAW device.
> > >>
> > >> By adding notification acpi-video can listen for backlight devices showing
> > >> up after it has loaded, and unregister its backlight device if desired.
> > >>
> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > >
> > > Backlight maintainer's ACK is requisite here.
> >
> > Agreed, which is why I send this set to all 3 the backlight maintainers
> > directly on both postings.
> >
> > What may be helpful for them is to hear from you if you're ok with the
> > acpi-video bits which are actually going to use this, since those will
> > be the only user of the new backlight api (for now).
> 
> I'm happy to apply any Backlight patches which have either Bryan or
> Jingoo's Ack, as they are the reviewers for the BL subsystem.

Acked-by: Jingoo Han <jg1.han@samsung.com>

Lee Jones,
Would you merge this patch into your backlight git tree?
Thank you.

Best regards,
Jingoo Han

> 
> > >> ---
> > >>  drivers/video/backlight/backlight.c | 40 +++++++++++++++++++++++++++++++++++++
> > >>  include/linux/backlight.h           |  7 +++++++
> > >>  2 files changed, 47 insertions(+)
> > >>
> > >> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> > >> index bd2172c..4280890 100644
> > >> --- a/drivers/video/backlight/backlight.c
> > >> +++ b/drivers/video/backlight/backlight.c
> > >> @@ -23,6 +23,7 @@
> > >>
> > >>  static struct list_head backlight_dev_list;
> > >>  static struct mutex backlight_dev_list_mutex;
> > >> +static struct blocking_notifier_head backlight_notifier;
> > >>
> > >>  static const char *const backlight_types[] = {
> > >>  	[BACKLIGHT_RAW] = "raw",
> > >> @@ -370,6 +371,9 @@ struct backlight_device *backlight_device_register(const char *name,
> > >>  	list_add(&new_bd->entry, &backlight_dev_list);
> > >>  	mutex_unlock(&backlight_dev_list_mutex);
> > >>
> > >> +	blocking_notifier_call_chain(&backlight_notifier,
> > >> +				     BACKLIGHT_REGISTERED, new_bd);
> > >> +
> > >>  	return new_bd;
> > >>  }
> > >>  EXPORT_SYMBOL(backlight_device_register);
> > >> @@ -413,6 +417,10 @@ void backlight_device_unregister(struct backlight_device *bd)
> > >>  		pmac_backlight = NULL;
> > >>  	mutex_unlock(&pmac_backlight_mutex);
> > >>  #endif
> > >> +
> > >> +	blocking_notifier_call_chain(&backlight_notifier,
> > >> +				     BACKLIGHT_UNREGISTERED, bd);
> > >> +
> > >>  	mutex_lock(&bd->ops_lock);
> > >>  	bd->ops = NULL;
> > >>  	mutex_unlock(&bd->ops_lock);
> > >> @@ -438,6 +446,36 @@ static int devm_backlight_device_match(struct device *dev, void *res,
> > >>  }
> > >>
> > >>  /**
> > >> + * backlight_register_notifier - get notified of backlight (un)registration
> > >> + * @nb: notifier block with the notifier to call on backlight (un)registration
> > >> + *
> > >> + * @return 0 on success, otherwise a negative error code
> > >> + *
> > >> + * Register a notifier to get notified when backlight devices get registered
> > >> + * or unregistered.
> > >> + */
> > >> +int backlight_register_notifier(struct notifier_block *nb)
> > >> +{
> > >> +	return blocking_notifier_chain_register(&backlight_notifier, nb);
> > >> +}
> > >> +EXPORT_SYMBOL(backlight_register_notifier);
> > >> +
> > >> +/**
> > >> + * backlight_unregister_notifier - unregister a backlight notifier
> > >> + * @nb: notifier block to unregister
> > >> + *
> > >> + * @return 0 on success, otherwise a negative error code
> > >> + *
> > >> + * Register a notifier to get notified when backlight devices get registered
> > >> + * or unregistered.
> > >> + */
> > >> +int backlight_unregister_notifier(struct notifier_block *nb)
> > >> +{
> > >> +	return blocking_notifier_chain_unregister(&backlight_notifier, nb);
> > >> +}
> > >> +EXPORT_SYMBOL(backlight_unregister_notifier);
> > >> +
> > >> +/**
> > >>   * devm_backlight_device_register - resource managed backlight_device_register()
> > >>   * @dev: the device to register
> > >>   * @name: the name of the device
> > >> @@ -544,6 +582,8 @@ static int __init backlight_class_init(void)
> > >>  	backlight_class->pm = &backlight_class_dev_pm_ops;
> > >>  	INIT_LIST_HEAD(&backlight_dev_list);
> > >>  	mutex_init(&backlight_dev_list_mutex);
> > >> +	BLOCKING_INIT_NOTIFIER_HEAD(&backlight_notifier);
> > >> +
> > >>  	return 0;
> > >>  }
> > >>
> > >> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > >> index 7264742..adb14a8 100644
> > >> --- a/include/linux/backlight.h
> > >> +++ b/include/linux/backlight.h
> > >> @@ -40,6 +40,11 @@ enum backlight_type {
> > >>  	BACKLIGHT_TYPE_MAX,
> > >>  };
> > >>
> > >> +enum backlight_notification {
> > >> +	BACKLIGHT_REGISTERED,
> > >> +	BACKLIGHT_UNREGISTERED,
> > >> +};
> > >> +
> > >>  struct backlight_device;
> > >>  struct fb_info;
> > >>
> > >> @@ -133,6 +138,8 @@ extern void devm_backlight_device_unregister(struct device *dev,
> > >>  extern void backlight_force_update(struct backlight_device *bd,
> > >>  				   enum backlight_update_reason reason);
> > >>  extern bool backlight_device_registered(enum backlight_type type);
> > >> +extern int backlight_register_notifier(struct notifier_block *nb);
> > >> +extern int backlight_unregister_notifier(struct notifier_block *nb);
> > >>
> > >>  #define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)

Re: [PATCH resend 2/4] backlight: Add backlight device (un)registration notification

[Rafael J. Wysocki]

On Monday, May 26, 2014 12:03:43 PM Jingoo Han wrote:
> On Thursday, May 22, 2014 6:02 PM, Lee Jones wrote:
> > On Thursday, May 22, 2014 5:45 PM, Hans de Goede wrote:
> > > On Thursday, May 22, 2014 8:31 AM, Rafael J. Wysocki wrote:
> > > > On Wednesday, May 21, 2014 10:40 PM, Hans de Goede wrote:
> > > >> Some firmware drivers, ie acpi-video want to get themselves out of the
> > > >> way (in some cases) when their also is a raw backlight device available.
> > > >>
> > > >> Due to module loading ordering being unknown, acpi-video cannot be certain
> > > >> that the backlight_device_registered(BACKLIGHT_RAW) it does for this is
> > > >> the final verdict wrt there being a BACKLIGHT_RAW device.
> > > >>
> > > >> By adding notification acpi-video can listen for backlight devices showing
> > > >> up after it has loaded, and unregister its backlight device if desired.
> > > >>
> > > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > >
> > > > Backlight maintainer's ACK is requisite here.
> > >
> > > Agreed, which is why I send this set to all 3 the backlight maintainers
> > > directly on both postings.
> > >
> > > What may be helpful for them is to hear from you if you're ok with the
> > > acpi-video bits which are actually going to use this, since those will
> > > be the only user of the new backlight api (for now).
> > 
> > I'm happy to apply any Backlight patches which have either Bryan or
> > Jingoo's Ack, as they are the reviewers for the BL subsystem.
> 
> Acked-by: Jingoo Han <jg1.han@samsung.com>
> 
> Lee Jones,
> Would you merge this patch into your backlight git tree?

Hans, does this series depend on things that I've applied already?  If so,
I'd very much prefer to take this series too as a whole.

Rafael

Re: [PATCH resend 2/4] backlight: Add backlight device (un)registration notification

[Hans de Goede]

Hi,

On 05/26/2014 01:03 PM, Rafael J. Wysocki wrote:
> On Monday, May 26, 2014 12:03:43 PM Jingoo Han wrote:
>> On Thursday, May 22, 2014 6:02 PM, Lee Jones wrote:
>>> On Thursday, May 22, 2014 5:45 PM, Hans de Goede wrote:
>>>> On Thursday, May 22, 2014 8:31 AM, Rafael J. Wysocki wrote:
>>>>> On Wednesday, May 21, 2014 10:40 PM, Hans de Goede wrote:
>>>>>> Some firmware drivers, ie acpi-video want to get themselves out of the
>>>>>> way (in some cases) when their also is a raw backlight device available.
>>>>>>
>>>>>> Due to module loading ordering being unknown, acpi-video cannot be certain
>>>>>> that the backlight_device_registered(BACKLIGHT_RAW) it does for this is
>>>>>> the final verdict wrt there being a BACKLIGHT_RAW device.
>>>>>>
>>>>>> By adding notification acpi-video can listen for backlight devices showing
>>>>>> up after it has loaded, and unregister its backlight device if desired.
>>>>>>
>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>
>>>>> Backlight maintainer's ACK is requisite here.
>>>>
>>>> Agreed, which is why I send this set to all 3 the backlight maintainers
>>>> directly on both postings.
>>>>
>>>> What may be helpful for them is to hear from you if you're ok with the
>>>> acpi-video bits which are actually going to use this, since those will
>>>> be the only user of the new backlight api (for now).
>>>
>>> I'm happy to apply any Backlight patches which have either Bryan or
>>> Jingoo's Ack, as they are the reviewers for the BL subsystem.
>>
>> Acked-by: Jingoo Han <jg1.han@samsung.com>
>>
>> Lee Jones,
>> Would you merge this patch into your backlight git tree?
> 
> Hans, does this series depend on things that I've applied already?  If so,
> I'd very much prefer to take this series too as a whole.

The 3th patch in this series:
" acpi-video: Unregister the backlight device if a raw one shows up later"
depends on my "acpi-video: Add an acpi_video_unregister_backlight function"
patch, which you've applied to your linux-next branch already.

As well as on the 2nd patch in this series:
"backlight: Add backlight device (un)registration notification"

So I agree that it is a good idea to take the whole series through your tree.

Thanks & Regards,

Hans

Re: [PATCH resend 2/4] backlight: Add backlight device (un)registration notification

[Lee Jones]

> On 05/26/2014 01:03 PM, Rafael J. Wysocki wrote:
> > On Monday, May 26, 2014 12:03:43 PM Jingoo Han wrote:
> >> On Thursday, May 22, 2014 6:02 PM, Lee Jones wrote:
> >>> On Thursday, May 22, 2014 5:45 PM, Hans de Goede wrote:
> >>>> On Thursday, May 22, 2014 8:31 AM, Rafael J. Wysocki wrote:
> >>>>> On Wednesday, May 21, 2014 10:40 PM, Hans de Goede wrote:
> >>>>>> Some firmware drivers, ie acpi-video want to get themselves out of the
> >>>>>> way (in some cases) when their also is a raw backlight device available.
> >>>>>>
> >>>>>> Due to module loading ordering being unknown, acpi-video cannot be certain
> >>>>>> that the backlight_device_registered(BACKLIGHT_RAW) it does for this is
> >>>>>> the final verdict wrt there being a BACKLIGHT_RAW device.
> >>>>>>
> >>>>>> By adding notification acpi-video can listen for backlight devices showing
> >>>>>> up after it has loaded, and unregister its backlight device if desired.
> >>>>>>
> >>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>>>>
> >>>>> Backlight maintainer's ACK is requisite here.
> >>>>
> >>>> Agreed, which is why I send this set to all 3 the backlight maintainers
> >>>> directly on both postings.
> >>>>
> >>>> What may be helpful for them is to hear from you if you're ok with the
> >>>> acpi-video bits which are actually going to use this, since those will
> >>>> be the only user of the new backlight api (for now).
> >>>
> >>> I'm happy to apply any Backlight patches which have either Bryan or
> >>> Jingoo's Ack, as they are the reviewers for the BL subsystem.
> >>
> >> Acked-by: Jingoo Han <jg1.han@samsung.com>
> >>
> >> Lee Jones,
> >> Would you merge this patch into your backlight git tree?
> > 
> > Hans, does this series depend on things that I've applied already?  If so,
> > I'd very much prefer to take this series too as a whole.
> 
> The 3th patch in this series:
> " acpi-video: Unregister the backlight device if a raw one shows up later"
> depends on my "acpi-video: Add an acpi_video_unregister_backlight function"
> patch, which you've applied to your linux-next branch already.
> 
> As well as on the 2nd patch in this series:
> "backlight: Add backlight device (un)registration notification"
> 
> So I agree that it is a good idea to take the whole series through your tree.

I'm fine with that.

Rafael, could you apply the set onto an immutable branch and send me a
signed pull-request please?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH resend 2/4] backlight: Add backlight device (un)registration notification

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 3044 bytes --]

On Tuesday, May 27, 2014 10:20:33 AM Lee Jones wrote:
> > On 05/26/2014 01:03 PM, Rafael J. Wysocki wrote:
> > > On Monday, May 26, 2014 12:03:43 PM Jingoo Han wrote:
> > >> On Thursday, May 22, 2014 6:02 PM, Lee Jones wrote:
> > >>> On Thursday, May 22, 2014 5:45 PM, Hans de Goede wrote:
> > >>>> On Thursday, May 22, 2014 8:31 AM, Rafael J. Wysocki wrote:
> > >>>>> On Wednesday, May 21, 2014 10:40 PM, Hans de Goede wrote:
> > >>>>>> Some firmware drivers, ie acpi-video want to get themselves out of the
> > >>>>>> way (in some cases) when their also is a raw backlight device available.
> > >>>>>>
> > >>>>>> Due to module loading ordering being unknown, acpi-video cannot be certain
> > >>>>>> that the backlight_device_registered(BACKLIGHT_RAW) it does for this is
> > >>>>>> the final verdict wrt there being a BACKLIGHT_RAW device.
> > >>>>>>
> > >>>>>> By adding notification acpi-video can listen for backlight devices showing
> > >>>>>> up after it has loaded, and unregister its backlight device if desired.
> > >>>>>>
> > >>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > >>>>>
> > >>>>> Backlight maintainer's ACK is requisite here.
> > >>>>
> > >>>> Agreed, which is why I send this set to all 3 the backlight maintainers
> > >>>> directly on both postings.
> > >>>>
> > >>>> What may be helpful for them is to hear from you if you're ok with the
> > >>>> acpi-video bits which are actually going to use this, since those will
> > >>>> be the only user of the new backlight api (for now).
> > >>>
> > >>> I'm happy to apply any Backlight patches which have either Bryan or
> > >>> Jingoo's Ack, as they are the reviewers for the BL subsystem.
> > >>
> > >> Acked-by: Jingoo Han <jg1.han@samsung.com>
> > >>
> > >> Lee Jones,
> > >> Would you merge this patch into your backlight git tree?
> > > 
> > > Hans, does this series depend on things that I've applied already?  If so,
> > > I'd very much prefer to take this series too as a whole.
> > 
> > The 3th patch in this series:
> > " acpi-video: Unregister the backlight device if a raw one shows up later"
> > depends on my "acpi-video: Add an acpi_video_unregister_backlight function"
> > patch, which you've applied to your linux-next branch already.
> > 
> > As well as on the 2nd patch in this series:
> > "backlight: Add backlight device (un)registration notification"
> > 
> > So I agree that it is a good idea to take the whole series through your tree.
> 
> I'm fine with that.
> 
> Rafael, could you apply the set onto an immutable branch and send me a
> signed pull-request please?

You can find this patch on the branch at

git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git acpi-video

The top-most commit is 0dc6b96ac20c (ACPI / video: Add 4 new models to the
use_native_backlight DMI list).

Please feel free to pull from there if necessary, it is not going to be rebased.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH resend 2/4] backlight: Add backlight device (un)registration notification

[Lee Jones]

On Sun, 01 Jun 2014, Rafael J. Wysocki wrote:
> On Tuesday, May 27, 2014 10:20:33 AM Lee Jones wrote:
> > > On 05/26/2014 01:03 PM, Rafael J. Wysocki wrote:
> > > > On Monday, May 26, 2014 12:03:43 PM Jingoo Han wrote:
> > > >> On Thursday, May 22, 2014 6:02 PM, Lee Jones wrote:
> > > >>> On Thursday, May 22, 2014 5:45 PM, Hans de Goede wrote:
> > > >>>> On Thursday, May 22, 2014 8:31 AM, Rafael J. Wysocki wrote:
> > > >>>>> On Wednesday, May 21, 2014 10:40 PM, Hans de Goede wrote:
> > > >>>>>> Some firmware drivers, ie acpi-video want to get themselves out of the
> > > >>>>>> way (in some cases) when their also is a raw backlight device available.
> > > >>>>>>
> > > >>>>>> Due to module loading ordering being unknown, acpi-video cannot be certain
> > > >>>>>> that the backlight_device_registered(BACKLIGHT_RAW) it does for this is
> > > >>>>>> the final verdict wrt there being a BACKLIGHT_RAW device.
> > > >>>>>>
> > > >>>>>> By adding notification acpi-video can listen for backlight devices showing
> > > >>>>>> up after it has loaded, and unregister its backlight device if desired.
> > > >>>>>>
> > > >>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > >>>>>
> > > >>>>> Backlight maintainer's ACK is requisite here.
> > > >>>>
> > > >>>> Agreed, which is why I send this set to all 3 the backlight maintainers
> > > >>>> directly on both postings.
> > > >>>>
> > > >>>> What may be helpful for them is to hear from you if you're ok with the
> > > >>>> acpi-video bits which are actually going to use this, since those will
> > > >>>> be the only user of the new backlight api (for now).
> > > >>>
> > > >>> I'm happy to apply any Backlight patches which have either Bryan or
> > > >>> Jingoo's Ack, as they are the reviewers for the BL subsystem.
> > > >>
> > > >> Acked-by: Jingoo Han <jg1.han@samsung.com>
> > > >>
> > > >> Lee Jones,
> > > >> Would you merge this patch into your backlight git tree?
> > > > 
> > > > Hans, does this series depend on things that I've applied already?  If so,
> > > > I'd very much prefer to take this series too as a whole.
> > > 
> > > The 3th patch in this series:
> > > " acpi-video: Unregister the backlight device if a raw one shows up later"
> > > depends on my "acpi-video: Add an acpi_video_unregister_backlight function"
> > > patch, which you've applied to your linux-next branch already.
> > > 
> > > As well as on the 2nd patch in this series:
> > > "backlight: Add backlight device (un)registration notification"
> > > 
> > > So I agree that it is a good idea to take the whole series through your tree.
> > 
> > I'm fine with that.
> > 
> > Rafael, could you apply the set onto an immutable branch and send me a
> > signed pull-request please?
> 
> You can find this patch on the branch at
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git acpi-video
> 
> The top-most commit is 0dc6b96ac20c (ACPI / video: Add 4 new models to the
> use_native_backlight DMI list).
> 
> Please feel free to pull from there if necessary, it is not going to be rebased.

acpi-video contains 14 patches!  If we share patches in the future,
the branches really need to contain as few patches as possible.  I'm
happy to set-up a special 'mfd-pm' immutable branch for future
releases to save either one of use pulling in more patches into our
respective trees than is necessary.

Rather than pull all those patches in to the MFD tree, I'll simply run
the risk of a merge conflict this time.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog