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.