Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440

[Pali RohÃr]

On Wednesday 24 September 2014 10:59:41 Pali RohÃr wrote:
> On Wednesday 24 September 2014 10:19:38 Hans de Goede wrote:
> > Hi,
> > 
> > On 09/23/2014 10:44 PM, Pali RohÃr wrote:
> > > On Tuesday 23 September 2014 22:31:31 you wrote:
> > >> Hi,
> > >> 
> > >> On 09/23/2014 10:06 PM, Pali RohÃr wrote:
> > >>> Hello,
> > >>> 
> > >>> after big changes in acpi video/i915 code I cannot
> > >>> change display brightness on my Dell Latitude E6440
> > >>> with kernel 3.17-rc6. With kernel 3.13 everything
> > >>> worked fine.
> > >>> 
> > >>> More information about this problem:
> > >>> 
> > >>> For configuring brightness on Dell laptops there are 4
> > >>> ways: 1) via acpi video driver
> > >>> 2) via dell-laptop driver
> > >>> 3) via i915 drm driver
> > >>> 4) from userspace with special dell SMI call
> > >>> 
> > >>>     (e.g with program dellLcdBrightness from libsmbios
> > >>>     package)
> > >>> 
> > >>> Methods 2) and 4) are same, both making special SMI call
> > >>> and Bios handing this request (just 2 is from kernel and
> > >>> 4 from userspace)
> > >>> 
> > >>> Method 1) via acpi video driver working, but is not
> > >>> perfect. Driver can be used to change brightness (but
> > >>> only some levels, probably this depends on acpi/DSDT
> > >>> tables), but cannot be used to retrieve current
> > >>> brightness (when BIOS/SMI change brightness acpi driver
> > >>> report old incorrect value). So I prefer dell-laptop
> > >>> driver instead acpi video.
> > >>> 
> > >>> Method 3) working even with 3.17-rc6 kernel but because
> > >>> that backlight device exported by i915 is marked as raw,
> > >>> desktop programs prefer to use other devices.
> > >>> 
> > >>> Moreover it looks like that methods 1) 2) and 4) just
> > >>> forward request to method 3). So in any cased brightness
> > >>> is changed by i915 drm driver.
> > >>> 
> > >>> I'm not sure (correct me if I'm wrong!) but I think that
> > >>> intel i915 drm driver accept changes (file
> > >>> intel_opregion.c) only if acpi function
> > >>> acpi_video_verify_backlight_support() returns true.
> > >>> 
> > >>> Function acpi_video_verify_backlight_support() returns
> > >>> true iff: function acpi_video_backlight_support()
> > >>> returns true AND at least one of these functions
> > >>> returns false: acpi_osi_is_win8()
> > >>> acpi_video_use_native_backlight()
> > >>> backlight_device_registered(BACKLIGHT_RAW)
> > >>> 
> > >>> On my notebook acpi_osi_is_win8() returns true (as is
> > >>> win8 compliant),
> > >>> backlight_device_registered(BACKLIGHT_RAW) returns true
> > >>> as I'm using intel i915 drm driver with raw backlight
> > >>> device and acpi_video_use_native_backlight() returns
> > >>> true/false depending on
> > >>> "video.use_native_backlight" kernel param. Default is
> > >>> true.
> > >>> 
> > >>> So if I want to have working acpi video driver with
> > >>> display brightness support I need to boot kernel with
> > >>> param: "video.use_native_backlight=0". I tested it with
> > >>> kernel 3.17-rc6 and this param really enabled display
> > >>> brightness support via acpi video driver -- which is
> > >>> good.
> > >>> 
> > >>> Driver dell-laptop creating backligh device for
> > >>> brightness control only if
> > >>> acpi_video_backlight_support() returns false. There is
> > >>> complicated condition for it and when kernel is booted
> > >>> with "video.use_native_backlight=0" that function
> > >>> returns true.
> > >>> 
> > >>> So conclusion is: With current code in kernel 3.17-rc6
> > >>> it is not possible to control brightness of display
> > >>> with native driver dell-laptop on Dell Latitude E6440
> > >>> (and probably on others too)!!!
> > >>> 
> > >>> And Because laptop is win8 compliant and you create
> > >>> decision to use native driver (instead acpi one) it is
> > >>> not possible to control display brightness without
> > >>> tweeks in kernel cmdline.
> > >>> 
> > >>> As I wrote I would rather to use native dell-laptop
> > >>> driver for controlling brightness, but it is not
> > >>> possible.
> > >>> 
> > >>> So how to solve this problem?
> > >>> 
> > >>> Quick solution would be to set use_native_backlight
> > >>> false for some Dell laptops which means, that acpi
> > >>> video will be used and in this case intel i915 driver
> > >>> will *not* drop backlight change request.
> > >>> 
> > >>> Another solution could be to disable check in
> > >>> dell_laptop driver and add use_native_backlight=0 to
> > >>> hooks. But this create two backlight interfaces (which
> > >>> is not good), but only way (for now) how to make
> > >>> dell_laptop working again.
> > >>> 
> > >>> Better and maybe only one proper solution would be to
> > >>> teach intel drm i915 driver to not drop backlight change
> > >>> request for Dell laptops (or all??). (This allows to
> > >>> work both acpi video and dell_laptop drivers without
> > >>> any change and with *any* value in param
> > >>> use_native_backlight). I think that problematic code is
> > >>> in function asle_set_backlight() in file
> > >>> intel_opregion.c (but I'm not sure). My idea is that
> > >>> "drop" event function it is caused by this commit
> > >>> 0b9f7d93ca6109048a4eb06332b666b6e29df4fe (but I'm not
> > >>> sure).
> > >>> 
> > >>> At least something must be done as I think that I'm not
> > >>> only one who has Dell laptop and brightness
> > >>> configuration is really important...
> > >> 
> > >> I don't understand your problem, the kernel is selecting
> > >> the i915 backlight driver, making that the only one
> > >> available to userspace, so the one problem you state with
> > >> the i915 driver, that it is "raw" is not an issue, as
> > >> userspace will pick it when it is the only one.
> > > 
> > > It is not only one.
> > 
> > Are you sure, if you do not specify any commandline
> > parameters, then neither dell-laptop nor acpi-video should
> > register a backlight interface.
> > 
> > dell-laptop.c has:
> > 
> > #ifdef CONFIG_ACPI
> > 
> >         /* In the event of an ACPI backlight being
> >         available,
> > 
> > don't * register the platform controller.
> > 
> >          */
> >         
> >         if (acpi_video_backlight_support())
> >         
> >                 return 0;
> > 
> > #endif
> > 
> > And acpi_video_backlight_support() will (normally) return
> > true on acpi-backlight capable systems independent of
> > use_native_backlight.
> > 
> > And drivers/acpi/video.c has this:
> >         /* no warning message if acpi_backlight=vendor or a
> > 
> > quirk is used */ if (!acpi_video_verify_backlight_support())
> > 
> >                 return;
> > 
> > ...
> > 
> > bool acpi_video_verify_backlight_support(void)
> > {
> > 
> >         if (acpi_osi_is_win8() &&
> > 
> > acpi_video_use_native_backlight() &&
> > backlight_device_registered(BACKLIGHT_RAW)) return false;
> > 
> >         return acpi_video_backlight_support();
> > 
> > }
> > 
> > So unlike the check in dell-laptop this one does depend on
> > the use_native_backlight setting.
> 
> It depends (see my previous mail). Here is code:
> 
> static bool acpi_video_use_native_backlight(void)
> {
> 	if (use_native_backlight_param != -1)
> 		return use_native_backlight_param;
> 	else
> 		return use_native_backlight_dmi;
> }
> 
> > I've just checked 3.17 on my E6430 and there this works as
> > it should, I only get the intel_backlight in
> > /sys/class/backlight
> > 
> > > Also dell-laptop (or acpi video) backlight
> > > interface is created (depends on use_native_backlight
> > > param). And userspace will pick dell-laptop (or acpi
> > > video) to use (which cannot change brightness).
> > > 
> > >> Why would you want to use dell-laptop despite the i915
> > >> driver working fine ?
> > > 
> > > i915 working only if I compile kernel without acpi video
> > > dell- laptop support (distributions compile kernel with
> > > these drivers) and i915 is not good for usage. First it
> > > provides more then thousand brightness levels and lot of
> > > (with low numbers) completely turn display off. And if
> > > userspace map these thousand levels into 5-10 levels (as
> > > nobody want to press brightness key up/down 1000x) two
> > > lowest levels cause display off.
> > 
> > More and more laptops will only have working backlight
> > control at all when using i915, so userspace will need to
> > learn to better deal with backlight controls with these
> > ranges. And low pwm levels turning the backlight of is
> > normal for raw interfaces, if userspace does not want this,
> > then they should not go so low.
> 
> So then kernel should report which low levels turn backlight
> off so userspace will know which levels should avoid. But
> this is not implemented yet.
> 
> > I suggest that you file a bug against your desktop
> > environment that it is not using the backlight controls in
> > an optimal manner, from the kernel pov everything is
> > working fine.
> 
> Once I will know which levels should not DE use I can report
> bug.
> 
> > > With acpi
> > > video and dell-laptop driver levels are mapped into small
> > > level space in perfect way. So this is reason I want to
> > > use dell-laptop for controlling brightness.
> > 
> > If you want to use dell-laptop, specify
> > acpi_backlight=vendor on the kernel commandline, that will
> > give you dell_laptop + intel_backlight as backlight
> > interfaces, and userspace should prefer dell_laptop.
> 
> With acpi_backlight=vendor dell-laptop is not working, see my
> previous mail. In this case intel i915 drm driver ignore bios
> events for changing brightness. And userspace prefer to use
> dell_laptop which do nothing!
> 

Ok, that problematic commit is:

ACPI / i915: ignore firmware requests for backlight change
0b9f7d93ca6109048a4eb06332b666b6e29df4fe

When I reverted it acpi_backlight=vendor started working again 
(via dell_laptop). Without reverting that commit dell_laptop 
simply do nothing.

Tested and on my laptop Dell Latitude E6440 driver dell_laptop 
does not work with above commit.

> > But IMHO it would be better to file a bug
> > with your desktop environment, and get that fixed to work
> > properly with intel_backlight (or with raw backlight
> > interfaces in general).
> > 
> > Regards,
> > 
> > Hans
> > 
> > >> Regards,
> > >> 
> > >> Hans

-- 
Pali RohÃr
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part.