[Intel-gfx] [PATCH v3 2/2] drivers: i915: Default max backlight brightness value

[Bryan Freed]

On Tue, Nov 8, 2011 at 5:49 PM, Olof Johansson <olof at lixom.net> wrote:
> Hi,
>
> On Tue, Nov 8, 2011 at 3:11 PM, Matthew Garrett <mjg59 at srcf.ucam.org> wrote:
>> On Tue, Nov 08, 2011 at 03:02:00PM -0800, Olof Johansson wrote:
>>
>>> How about a DMI table check that overrides whatever is setup (or not
>>> setup) from the video bios? We know exactly what platforms need this
>>> so that table would be easy to specify.
>>
>> dmi's horribly unscalable. It's much better to have a communication
>> channel that doesn't require new code for new models of the same
>> platform.
>
> Yeah, agreed.
>
>>> I'm not sure how well this would fit into our platform layer code, it
>>> would be pretty nasty to have to export the default backlight variable
>>> from the i915 driver and modify it from there as well, and I'm sure
>>> noone wants to see any kind of chromeos-specific code paths in the 915
>>> driver (myself included).
>>
>> Well right now this path is (effectively) chromeos-specific. Refactoring
>> the code so we just have the register readback as a single information
>> source and allow the existing platform-specific code to hook in would be
>> conceptually cleaner. But then maybe this is grotesque over-engineering
>> and we should just hack this case.

I tried to make some progress on this today.  By existing
platform-specific code, are you referring to chromeos_acpi.c?
I did not see a clean way to do this.  We can easily add variables to
chromeos_acpi, but how does one cleanly make use of them in
intel_panel.c?

>
>
> Actually, I didn't look closely enough to the original patches from
> Simon for this and your original feedback.
>
> Looks to me like filling in the original block of 'XXX add code'
> fallback is the way to go here.
>
> Getting the raw pch clock isn't hard, and setting a reasonable value
> for the modulation frequency and the corresponding max duty cycle
> based on that should solve our problem.

I think the original comment was a bit of a guess on how to proceed.
Knowing internal clocks does not help us figure out what what the
display limitations are, like what is the right frequency to set the
backlight to.  That is why this is typically set by VBIOS rather than
calculated in the driver.

Since we try to run without VBIOS or VBT, we get critical defaults
from init_vbt_defaults().
Since this is our first critical register not touched by VBT
processing, init_vbt_defaults() does not help much.
The new native backlight driver in intel_panel.c detects this
uninitialized register case but does not provide a default (other than
a hopefully previously saved value) itself.
The quick fix for chromebooks would be to use as the default what our
old homespun native backlight driver hardwired into the register,
0x1000.
Making that a module parameter allows us to put it in the "kernel
blob" that our firmware parses to find command line parameters.  And
other BIOSes that do not run VBIOS (if there are any) can similarly
provide their appropriate default.

I understand module parameters are now frowned upon, but I do not see
a cleaner solution.

bryan.

>
> -Olof
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>