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

Hi Hans,

Thanks for following up and explaining the situation to Pali.

On 09/25/2014 02:21 AM, Pali Rohár wrote:
> On Wednesday 24 September 2014 16:34:21 Hans de Goede wrote:
>> Ok, so the dell-laptop interface is just an obsolete wrapper
>> around the i915 opregion code, which shows that the right
>> interface to use is the i915 one, which we do if you don't
>> specify any kernel commandline parameters, case closed.
>>
>> Regards,
>>
>> Hans
> 
> Nope, its not closed.
> 
> Still i915 interface has problem with setting backlight. It 
> exports lot of levels which turning display off. Which breaking 
> exiting applications for configuring display brightness. This is 
> still big regression as black screen is not want people want to 
> see.
> 
> Driver dell-laptop has exported only few - not thousands level 
> (which is insane) and only usefull levels (not lot of levels 
> which turn display off).
> 
> So for this reason using i915 backlight interface is not possible 
> and also Dell (for E6440) set kernel param acpi_backlight=vendor 
> to use dell_laptop module for controlling brightness.
> 
> On my laptop E6440 is better for using dell-laptop and not acpi 
> or i915.

Hi Pali,

Please test this patch:

diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index ca52ad2ae7d1..15534345bd57 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -396,6 +396,24 @@ int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
 	return -EINVAL;
 }
 
+/*
+ * Some of the Thinkpads' firmware will issue a backlight change operation
+ * region request unconditionally on AC plug/unplug, this is undesirable and
+ * should be ignored. Then there is a Dell laptop whose vendor backlight
+ * interface also makes use of operation region request to change backlight
+ * level and we have to keep it work. The rule used here is: if the vendor
+ * backlight interface is not in use and the ACPI backlight interface is
+ * broken, we ignore the requests; oterwise, we keep processing them.
+ */
+static bool should_ignore_backlight_request(void)
+{
+	if (acpi_video_backlight_support() &&
+	    !acpi_video_verify_backlight_support())
+		return true;
+
+	return false;
+}
+
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -404,11 +422,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 
 	DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
 
-	/*
-	 * If the acpi_video interface is not supposed to be used, don't
-	 * bother processing backlight level change requests from firmware.
-	 */
-	if (!acpi_video_verify_backlight_support()) {
+	if (should_ignore_backlight_request()) {
 		DRM_DEBUG_KMS("opregion backlight request ignored\n");
 		return 0;
 	}

WARNING: multiple messages have this Message-ID (diff)
From: Aaron Lu <aaron.lu@intel.com>
To: "Pali Rohár" <pali.rohar@gmail.com>,
	"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,
	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: Thu, 25 Sep 2014 11:15:35 +0800	[thread overview]
Message-ID: <542388D7.1010008@intel.com> (raw)
In-Reply-To: <201409242021.10107@pali>

Hi Hans,

Thanks for following up and explaining the situation to Pali.

On 09/25/2014 02:21 AM, Pali Rohár wrote:
> On Wednesday 24 September 2014 16:34:21 Hans de Goede wrote:
>> Ok, so the dell-laptop interface is just an obsolete wrapper
>> around the i915 opregion code, which shows that the right
>> interface to use is the i915 one, which we do if you don't
>> specify any kernel commandline parameters, case closed.
>>
>> Regards,
>>
>> Hans
> 
> Nope, its not closed.
> 
> Still i915 interface has problem with setting backlight. It 
> exports lot of levels which turning display off. Which breaking 
> exiting applications for configuring display brightness. This is 
> still big regression as black screen is not want people want to 
> see.
> 
> Driver dell-laptop has exported only few - not thousands level 
> (which is insane) and only usefull levels (not lot of levels 
> which turn display off).
> 
> So for this reason using i915 backlight interface is not possible 
> and also Dell (for E6440) set kernel param acpi_backlight=vendor 
> to use dell_laptop module for controlling brightness.
> 
> On my laptop E6440 is better for using dell-laptop and not acpi 
> or i915.

Hi Pali,

Please test this patch:

diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index ca52ad2ae7d1..15534345bd57 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -396,6 +396,24 @@ int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
 	return -EINVAL;
 }
 
+/*
+ * Some of the Thinkpads' firmware will issue a backlight change operation
+ * region request unconditionally on AC plug/unplug, this is undesirable and
+ * should be ignored. Then there is a Dell laptop whose vendor backlight
+ * interface also makes use of operation region request to change backlight
+ * level and we have to keep it work. The rule used here is: if the vendor
+ * backlight interface is not in use and the ACPI backlight interface is
+ * broken, we ignore the requests; oterwise, we keep processing them.
+ */
+static bool should_ignore_backlight_request(void)
+{
+	if (acpi_video_backlight_support() &&
+	    !acpi_video_verify_backlight_support())
+		return true;
+
+	return false;
+}
+
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -404,11 +422,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 
 	DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
 
-	/*
-	 * If the acpi_video interface is not supposed to be used, don't
-	 * bother processing backlight level change requests from firmware.
-	 */
-	if (!acpi_video_verify_backlight_support()) {
+	if (should_ignore_backlight_request()) {
 		DRM_DEBUG_KMS("opregion backlight request ignored\n");
 		return 0;
 	}

  reply	other threads:[~2014-09-25  3:15 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
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 [this message]
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=542388D7.1010008@intel.com \
    --to=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=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pali.rohar@gmail.com \
    --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.