Re: [PATCH] drm/i915: A better maximum brightness for users.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Mon, Nov 9, 2015 at 6:51 PM, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
On Mon, 09 Nov 2015, "Shih-Yuan Lee (FourDollars)" <sylee@xxxxxxxxxxxxx> wrote:
> On Mon, Nov 9, 2015 at 6:17 PM, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> wrote:
>
>> On Mon, 09 Nov 2015, "Shih-Yuan Lee (FourDollars)" <sylee@xxxxxxxxxxxxx>
>> wrote:
>> > The PWM brightness level of Dell XPS 13 (2015) is from 10 to 937 however
>> > the sysfs brightness level always starts from 0 so it is better to use
>> > 927 as the sysfs maximum brightness level and it becomes easier to map
>> > from the PWM brightness level to the sysfs brightness level.
>>
>> We've been thinking we should provide a fixed range to userspace
>> instead. Say, 0-100.
>>
>> BR,
>> Jani.
>>
>>
>>
> That might not be a good idea for the backward compatibility.

Please explain how you think your change is good and a fixed range 0-100
is bad in terms of backward compatibility. Both just arbitrarily change
the max.
The original sysfs brightness range is from 0 to 937. Let's define it as A = {0, 1, 2, ..., 937}.
The PWM brightness range is from 10 to 937. Let's define it as B = {10, 11, 12, ..., 937}.
|A| = 938, |B| = 928
|A| != |B|
It implies A and B is not an 1-1 mapping.

My patch is not a arbitrarily change.
It makes A' = {0, 1, 2, ..., 927}. |A'| = 928
You can see |A'| = |B| so it implies A' and B is an 1-1 mapping.
It means any value in A' can be mapped to a different value in B and visa versa.

C = {0, 100} has the same mapping problem.

Besides, we've changed the max for some platforms before, and the ABI
for backlight sysfs is you can stick a value between 0 and
max_brightness to the brightness attribute. Any userspace that relies on
anything else is broken.

> However I saw some message as the following.
> [    3.402233] [drm:parse_lfp_backlight] VBT backlight PWM modulation
> frequency 200 Hz, active high, min brightness 10, level 255
>
>
> Does it mean the brightness range is also defined in the BIOS?

The minimum and the PWM modulation frequency are defined in BIOS. The
modulation frequency is an attribute for the hardware; I think that was
originally exposed as the max was just for implementation convenience.
I don't mean the minimum or the PWM modulation frequency.
I mean "level 255".
Does it mean the brightness range or something else?

Regards,
$4

BR,
Jani.


>
> Regards,
> $4
>
>>
>> >
>> > Signed-off-by: Shih-Yuan Lee (FourDollars) <sylee@xxxxxxxxxxxxx>
>> > ---
>> >  drivers/gpu/drm/i915/intel_panel.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_panel.c
>> b/drivers/gpu/drm/i915/intel_panel.c
>> > index a24df35..697fd4d 100644
>> > --- a/drivers/gpu/drm/i915/intel_panel.c
>> > +++ b/drivers/gpu/drm/i915/intel_panel.c
>> > @@ -1211,7 +1211,7 @@ static int intel_backlight_device_register(struct
>> intel_connector *connector)
>> >        * Note: Everything should work even if the backlight device max
>> >        * presented to the userspace is arbitrarily chosen.
>> >        */
>> > -     props.max_brightness = panel->backlight.max;
>> > +     props.max_brightness = panel->backlight.max - panel->backlight.min;
>> >       props.brightness = scale_hw_to_user(connector,
>> >                                           panel->backlight.level,
>> >                                           props.max_brightness);
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
>>

--
Jani Nikula, Intel Open Source Technology Center



--
Shih-Yuan Lee (FourDollars) | Software Engineer | Commercial Engineering - PC & Core Taipei | Ubuntu Engineering and Services | Canonical
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux