| Message ID | 1420444624-8313-1-git-send-email-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Monday, January 05, 2015 08:57:04 AM Hans de Goede wrote: > The L521X variant of the Dell XPS15 has integrated nvidia graphics, and > backlight control does not work properly when using the native interfaces. > > BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1163574 > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Applied, thanks! > --- > drivers/acpi/video.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c > index c72e79d2c5..032db45 100644 > --- a/drivers/acpi/video.c > +++ b/drivers/acpi/video.c > @@ -522,6 +522,16 @@ static struct dmi_system_id video_dmi_table[] __initdata = { > DMI_MATCH(DMI_PRODUCT_NAME, "370R4E/370R4V/370R5E/3570RE/370R5V"), > }, > }, > + > + { > + /* https://bugzilla.redhat.com/show_bug.cgi?id=1163574 */ > + .callback = video_disable_native_backlight, > + .ident = "Dell XPS15 L521X", > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), > + DMI_MATCH(DMI_PRODUCT_NAME, "XPS L521X"), > + }, > + }, > {} > }; > >
On Mon, Jan 05, 2015 at 08:57:04AM +0100, Hans de Goede wrote: > The L521X variant of the Dell XPS15 has integrated nvidia graphics, and > backlight control does not work properly when using the native interfaces. I'm not convinced that the assessment of this bug is correct. Is the system using the nvidia graphics device? If so, why is userspace writing to the intel backlight device?
Hi, On 11-01-15 07:16, Matthew Garrett wrote: > On Mon, Jan 05, 2015 at 08:57:04AM +0100, Hans de Goede wrote: >> The L521X variant of the Dell XPS15 has integrated nvidia graphics, and >> backlight control does not work properly when using the native interfaces. > > I'm not convinced that the assessment of this bug is correct. Is the > system using the nvidia graphics device? I do not know. > If so, why is userspace writing to the intel backlight device? Userspace is writing to the intel_backlight device because it is the only one present under /sys/class/backlight, as we disable acpi-video on win8 ready laptops by default now. What this commit does is disable the disabling of acpi-video, and after that userspace will see an acpi_video0 backlight under /sys/class/backlight, use that and then things work, this has been confirmed by the bug reporter. The commit message may not be 100% accurate, although I believe this is related to the laptop having nvidia graphics, but the code / fix is correct regardless. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 11, 2015 at 11:37:52AM +0100, Hans de Goede wrote: > Userspace is writing to the intel_backlight device because it is the > only one present under /sys/class/backlight, as we disable acpi-video > on win8 ready laptops by default now. Why is nouveau not providing a backlight device? > The commit message may not be 100% accurate, although I believe this is > related to the laptop having nvidia graphics, but the code / fix is correct > regardless. I don't think we have enough evidence to know whether this fix is correct.
Hi, On 12-01-15 03:44, Matthew Garrett wrote: > On Sun, Jan 11, 2015 at 11:37:52AM +0100, Hans de Goede wrote: > >> Userspace is writing to the intel_backlight device because it is the >> only one present under /sys/class/backlight, as we disable acpi-video >> on win8 ready laptops by default now. > > Why is nouveau not providing a backlight device? I do not know. >> The commit message may not be 100% accurate, although I believe this is >> related to the laptop having nvidia graphics, but the code / fix is correct >> regardless. > > I don't think we have enough evidence to know whether this fix is > correct. This fix fixes a regression introduced from kernel 3.15 to 3.16, when we switched video.use_native_brightness's default from 0 to 1, all this patch does is restore the previous behaviour on this specific model as the new behaviour is broken. So this is in essence a model targeted "revert" fixing a regression and as such most certainly is correct, given the clear no regressions policy the kernel has. Might it be good to dive deeper as to why the intel_backlight interface does not work, yes certainly. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 12, 2015 at 10:12:57AM +0100, Hans de Goede wrote: > Hi, > > On 12-01-15 03:44, Matthew Garrett wrote: > >On Sun, Jan 11, 2015 at 11:37:52AM +0100, Hans de Goede wrote: > > > >>Userspace is writing to the intel_backlight device because it is the > >>only one present under /sys/class/backlight, as we disable acpi-video > >>on win8 ready laptops by default now. > > > >Why is nouveau not providing a backlight device? > > I do not know. We should figure that out. One possibility here is that it's correct for Intel to be disabling the ACPI device, but not correct for nouveau to be. We should possibly extend the interface to only unregister devices that correspond to the PCI device provided by the raw interface. > >I don't think we have enough evidence to know whether this fix is > >correct. > > This fix fixes a regression introduced from kernel 3.15 to 3.16, when we > switched video.use_native_brightness's default from 0 to 1, all this patch > does is restore the previous behaviour on this specific model as the new > behaviour is broken. But it does so in a way that discourages us from ever fixing the actual problem, and leaves an unknown number of similar systems broken. > So this is in essence a model targeted "revert" fixing a regression and as > such most certainly is correct, given the clear no regressions policy the > kernel has. If we want to avoid regressions and a patch has broken a system without us having a good idea as to why, we should revert the entire patch in order to unbreak other systems.
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index c72e79d2c5..032db45 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -522,6 +522,16 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "370R4E/370R4V/370R5E/3570RE/370R5V"), }, }, + + { + /* https://bugzilla.redhat.com/show_bug.cgi?id=1163574 */ + .callback = video_disable_native_backlight, + .ident = "Dell XPS15 L521X", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_NAME, "XPS L521X"), + }, + }, {} };
The L521X variant of the Dell XPS15 has integrated nvidia graphics, and backlight control does not work properly when using the native interfaces. BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1163574 Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/acpi/video.c | 10 ++++++++++ 1 file changed, 10 insertions(+)