All of lore.kernel.org
 help / color / mirror / Atom feed
From: Aaron Lu <aaron.lu@intel.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Jani Nikula <jani.nikula@intel.com>,
	Borislav Petkov <bp@alien8.de>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	lkml <linux-kernel@vger.kernel.org>,
	"Rafael J. Wysocki" <rjw@sisk.pl>,
	ACPI Devel Mailing List <linux-acpi@vger.kernel.org>
Subject: Re: i915 backlight
Date: Fri, 02 Aug 2013 09:16:03 +0800	[thread overview]
Message-ID: <51FB0853.9090105@intel.com> (raw)
In-Reply-To: <51FA2537.5040208@intel.com>

On 08/01/2013 05:07 PM, Aaron Lu wrote:
> On 08/01/2013 04:12 PM, Borislav Petkov wrote:
>> On Thu, Aug 01, 2013 at 09:13:35AM +0800, Aaron Lu wrote:
>>> Can you please run acpi_listen and then press the Fn-Fx key, see if the
>>> events are correctly sent out?
>>
>> Like this?
>>
>> # acpi_listen
>> video/brightnessdown BRTDN 00000087 00000000
>> video/brightnessup BRTUP 00000086 00000000
>> video/brightnessdown BRTDN 00000087 00000000
>> video/brightnessup BRTUP 00000086 00000000
>> video/brightnessdown BRTDN 00000087 00000000
>> video/brightnessup BRTUP 00000086 00000000
>> video/brightnessdown BRTDN 00000087 00000000
>> video/brightnessup BRTUP 00000086 00000000
>> video/brightnessdown BRTDN 00000087 00000000
>> ^C
> 
> Yes, so the event is correctly sent out.
> 
>>
>>> From the bug page:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=51231#c80
>>> I got the impression that both the acpi_video interface and the vendor
>>> interface thinkpad_screen are broken. So adding this cmdline here works
>>> suggests that either thinkpad_screen works or thinkpad vendor driver
>>> doesn't get loaded or doesn't create that interface for some reason.
>>>
>>> Alternatively, if the intel_backlight interface works(highly possible),
>>> you can use xorg.conf to specify the that backlight interface for X.
>>>
>>> Section "Device"
>>>         Option     "Backlight"	"intel_backlight"
>>> 	Identifier  "Card0"
>>> 	Driver      "intel"
>>> 	BusID       "PCI:0:2:0"
>>> EndSection
>>
>> Yeah, that didn't work *but* manually writing to both:
>>
>> /sys/class/backlight/acpi_video0/brightness
>>
>> and
>>
>> /sys/class/backlight/intel_backlight/brightness
>>
>> works.
> 
> Err...we have the event sent out on hotkey press and the interface also
> works, but still, using hotkey to adjust brightness level is broken...
> 
> I just found an old acer laptop that has similar issue(or even worse: on
> X starts, an almost black screen is shown and hotkey adjust doesn't
> work), I'll look into this.

Hi Jani & Daniel,

It turned out there is an integer overflow problem, and the below patch
fixed this problem on Acer Aspire 4732Z and thinkpad R61i.

From: Aaron Lu <aaron.lu@intel.com>
Subject: [PATCH] drm/i915: avoid brightness overflow when doing scale

Some card's max brightness level is pretty large, e.g. on Acer Aspire
4732Z, the max level is 989910. If user space set a large enough level
then the current scale done in intel_panel_set_backlight will cause an
integer overflow and the scaled level will be mistakenly small, leaving
user with an almost black screen. This patch fixes this problem.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
Since the only external user of intel_panel_set_backlight is operation
region code where the max will be a constant of 255, this patch fixes
the problem by comparing freq and max and then do things accordingly
instead of converting to 64 bits.

 drivers/gpu/drm/i915/intel_panel.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 67e2c1f..7c674f0 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -498,7 +498,10 @@ void intel_panel_set_backlight(struct drm_device *dev, u32 level, u32 max)
 	}
 
 	/* scale to hardware */
-	level = level * freq / max;
+	if (freq < max)
+		level = level * freq / max;
+	else
+		level = freq / max * level;
 
 	dev_priv->backlight.level = level;
 	if (dev_priv->backlight.device)
-- 
1.8.3.1


Hi Boris,

Since the sysfs interface works on your system, I think your problem
should be different. Can you please file a bug for this? I can provide
you with a debug patch and then see what happened. Please attach
acpidump when filing the bug.

https://bugzilla.kernel.org, ACPI/Power-Video.

Thanks,
Aaron

> 
>>
>> The ranges are different, though:
>>
>> intel_backlight/actual_brightness:1000
>> intel_backlight/bl_power:0
>> intel_backlight/brightness:1000
>> intel_backlight/max_brightness:4437
>> intel_backlight/type:raw
>>
>> acpi_video0/actual_brightness:41
>> acpi_video0/bl_power:0
>> acpi_video0/brightness:41
>> acpi_video0/max_brightness:100
>> acpi_video0/type:firmware
> 
> Yes, different interface has different brightness ranges and a value in
> one range may turn out to be the same actual brightness level of another
> value in another range.
> 
>>
>> I guess I need to write me a dirty script for now ... :-)
> 
> :-)
> 
>>
>> Thanks guys.
>>
> Thanks,
> Aaron
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


  reply	other threads:[~2013-08-02  1:15 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-31 16:22 i915 INFO: trying to register non-static key Borislav Petkov
2013-07-31 16:36 ` i915 backlight Borislav Petkov
2013-07-31 21:16   ` Rafael J. Wysocki
2013-08-01  8:07     ` Borislav Petkov
2013-08-02  6:00       ` Aaron Lu
2013-08-02  6:25         ` Josep Lladonosa
2013-08-02  6:36           ` Aaron Lu
2013-08-02  7:25             ` Josep Lladonosa
2013-08-02  6:48           ` Felipe Contreras
2013-08-02 14:03             ` Rafael J. Wysocki
2013-08-02 18:58               ` Felipe Contreras
2013-08-02 20:03                 ` Josep Lladonosa
2013-08-02 20:08                   ` Felipe Contreras
2013-08-02 20:11                     ` Josep Lladonosa
2013-08-02 20:19                       ` Borislav Petkov
2013-08-02 21:25                       ` Felipe Contreras
2013-08-02 22:23                         ` Josep Lladonosa
2013-08-02 23:35                 ` Rafael J. Wysocki
2013-08-03  1:01                   ` Felipe Contreras
2013-08-02  8:16         ` Borislav Petkov
2013-08-01  1:13   ` Aaron Lu
2013-08-01  8:12     ` Borislav Petkov
2013-08-01  9:07       ` Aaron Lu
2013-08-01  9:07         ` Aaron Lu
2013-08-02  1:16         ` Aaron Lu [this message]
2013-08-02  8:34           ` Borislav Petkov
2013-08-05  7:34           ` Daniel Vetter
2013-08-04 23:06 ` i915 INFO: trying to register non-static key Daniel Vetter
2013-08-04 23:06   ` Daniel Vetter
2013-08-05  9:26   ` Borislav Petkov
2013-08-05 13:23     ` Johannes Stezenbach
2013-08-05 13:27       ` Borislav Petkov
2013-08-05 13:48         ` Johannes Stezenbach

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=51FB0853.9090105@intel.com \
    --to=aaron.lu@intel.com \
    --cc=bp@alien8.de \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rjw@sisk.pl \
    /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.