<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 9, 2015 at 6:51 PM, Jani Nikula <span dir="ltr"><<a href="mailto:jani.nikula@linux.intel.com" target="_blank">jani.nikula@linux.intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">On Mon, 09 Nov 2015, "Shih-Yuan Lee (FourDollars)" <<a href="mailto:sylee@canonical.com">sylee@canonical.com</a>> wrote:<br>
> On Mon, Nov 9, 2015 at 6:17 PM, Jani Nikula <<a href="mailto:jani.nikula@linux.intel.com">jani.nikula@linux.intel.com</a>><br>
> wrote:<br>
><br>
>> On Mon, 09 Nov 2015, "Shih-Yuan Lee (FourDollars)" <<a href="mailto:sylee@canonical.com">sylee@canonical.com</a>><br>
>> wrote:<br>
>> > The PWM brightness level of Dell XPS 13 (2015) is from 10 to 937 however<br>
>> > the sysfs brightness level always starts from 0 so it is better to use<br>
>> > 927 as the sysfs maximum brightness level and it becomes easier to map<br>
>> > from the PWM brightness level to the sysfs brightness level.<br>
>><br>
>> We've been thinking we should provide a fixed range to userspace<br>
>> instead. Say, 0-100.<br>
>><br>
>> BR,<br>
>> Jani.<br>
>><br>
>><br>
>><br>
> That might not be a good idea for the backward compatibility.<br>
<br>
</span>Please explain how you think your change is good and a fixed range 0-100<br>
is bad in terms of backward compatibility. Both just arbitrarily change<br>
the max.<br></blockquote><div>The original sysfs brightness range is from 0 to 937. Let's define it as A = {0, 1, 2, ..., 937}.</div><div>The PWM brightness range is from 10 to 937. Let's define it as B = {10, 11, 12, ..., 937}.</div><div>|A| = 938, |B| = 928</div><div>|A| != |B|</div><div>It implies A and B is not an 1-1 mapping.</div><div><br></div><div>My patch is not a arbitrarily change.</div><div>It makes A' = {0, 1, 2, ..., 927}. |A'| = 928</div><div>You can see |A'| = |B| so it implies A' and B is an 1-1 mapping.</div><div>It means any value in A' can be mapped to a different value in B and visa versa.</div><div><br></div><div>C = {0, 100} has the same mapping problem.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
Besides, we've changed the max for some platforms before, and the ABI<br>
for backlight sysfs is you can stick a value between 0 and<br>
max_brightness to the brightness attribute. Any userspace that relies on<br>
anything else is broken.<br>
<span class=""><br>
> However I saw some message as the following.<br>
> [    3.402233] [drm:parse_lfp_backlight] VBT backlight PWM modulation<br>
> frequency 200 Hz, active high, min brightness 10, level 255<br>
><br>
><br>
> Does it mean the brightness range is also defined in the BIOS?<br>
<br>
</span>The minimum and the PWM modulation frequency are defined in BIOS. The<br>
modulation frequency is an attribute for the hardware; I think that was<br>
originally exposed as the max was just for implementation convenience.<br></blockquote><div>I don't mean the minimum or the PWM modulation frequency.</div><div>I mean "level 255".</div><div>Does it mean the brightness range or something else?</div><div><br></div><div>Regards,</div><div>$4</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
BR,<br>
Jani.<br>
<div class=""><div class="h5"><br>
<br>
><br>
> Regards,<br>
> $4<br>
><br>
>><br>
>> ><br>
>> > Signed-off-by: Shih-Yuan Lee (FourDollars) <<a href="mailto:sylee@canonical.com">sylee@canonical.com</a>><br>
>> > ---<br>
>> >  drivers/gpu/drm/i915/intel_panel.c | 2 +-<br>
>> >  1 file changed, 1 insertion(+), 1 deletion(-)<br>
>> ><br>
>> > diff --git a/drivers/gpu/drm/i915/intel_panel.c<br>
>> b/drivers/gpu/drm/i915/intel_panel.c<br>
>> > index a24df35..697fd4d 100644<br>
>> > --- a/drivers/gpu/drm/i915/intel_panel.c<br>
>> > +++ b/drivers/gpu/drm/i915/intel_panel.c<br>
>> > @@ -1211,7 +1211,7 @@ static int intel_backlight_device_register(struct<br>
>> intel_connector *connector)<br>
>> >        * Note: Everything should work even if the backlight device max<br>
>> >        * presented to the userspace is arbitrarily chosen.<br>
>> >        */<br>
>> > -     props.max_brightness = panel->backlight.max;<br>
>> > +     props.max_brightness = panel->backlight.max - panel->backlight.min;<br>
>> >       props.brightness = scale_hw_to_user(connector,<br>
>> >                                           panel->backlight.level,<br>
>> >                                           props.max_brightness);<br>
>><br>
>> --<br>
>> Jani Nikula, Intel Open Source Technology Center<br>
>><br>
<br>
--<br>
Jani Nikula, Intel Open Source Technology Center<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">Shih-Yuan Lee (FourDollars) | Software Engineer | Commercial Engineering - PC & Core Taipei | Ubuntu Engineering and Services | Canonical</div>
</div></div>