All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Pali Rohár" <pali.rohar@gmail.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Zhang Rui <rui.zhang@intel.com>, Len Brown <lenb@kernel.org>,
	linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
	Aaron Lu <aaron.lu@intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	David Airlie <airlied@linux.ie>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: ACPI/i915: Cannot configure display brightness on Dell Latitude E6440
Date: Wed, 24 Sep 2014 10:59:41 +0200	[thread overview]
Message-ID: <201409241059.41465@pali> (raw)
In-Reply-To: <54227E9A.7080107@redhat.com>

[-- Attachment #1: Type: Text/Plain, Size: 9487 bytes --]

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!

> 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@gmail.com

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

  reply	other threads:[~2014-09-24  8:59 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-23 20:06 ACPI/i915: Cannot configure display brightness on Dell Latitude E6440 Pali Rohár
2014-09-23 20:31 ` Hans de Goede
2014-09-23 20:44   ` Pali Rohár
2014-09-24  8:19     ` Hans de Goede
2014-09-24  8:19       ` Hans de Goede
2014-09-24  8:59       ` Pali Rohár [this message]
2014-09-24  9:14         ` Pali Rohár
2014-09-24 12:04           ` Hans de Goede
2014-09-24 12:04             ` Hans de Goede
2014-09-24 12:53             ` Pali Rohár
2014-09-24 14:34               ` Hans de Goede
2014-09-24 14:34                 ` Hans de Goede
2014-09-24 18:21                 ` Pali Rohár
2014-09-24 18:21                   ` Pali Rohár
2014-09-25  3:15                   ` Aaron Lu
2014-09-25  3:15                     ` Aaron Lu
2014-09-25 14:23                     ` Pali Rohár
2014-09-26  2:30                       ` [PATCH] ACPI / i915: Update the condition to ignore firmware backlight change request Aaron Lu
2014-09-26  2:30                         ` Aaron Lu
2014-09-26 21:52                         ` Rafael J. Wysocki
2014-09-26 21:52                           ` Rafael J. Wysocki
2014-09-29  7:16                           ` Daniel Vetter
2014-09-29  7:16                             ` Daniel Vetter
2014-09-25 19:58                     ` ACPI/i915: Cannot configure display brightness on Dell Latitude E6440 Rafael J. Wysocki
2014-09-25 19:58                       ` Rafael J. Wysocki
2014-09-26  2:20                       ` Aaron Lu
2014-09-26  2:20                         ` Aaron Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201409241059.41465@pali \
    --to=pali.rohar@gmail.com \
    --cc=aaron.lu@intel.com \
    --cc=airlied@linux.ie \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hdegoede@redhat.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rui.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.