Created attachment 110360 [details] [review]
Add a module parameter for overriding minimum brightness control

(In reply to Aurabindo J from comment #0)
> Associated commit:
> 
> commit e1c412e75754ab7b7002f3e18a2652d999c40d4b
> Author: Jani Nikula <jani.nikula@intel.com>
> Date:   Wed Nov 5 14:46:31 2014 +0200
> 
>     drm/i915: safeguard against too high minimum brightness
>     
>     Never trust (your interpretation of) the VBT. Regression from
>     
>     commit 6dda730e55f412a6dfb181cae6784822ba463847
>     Author: Jani Nikula <jani.nikula@intel.com>
>     Date:   Tue Jun 24 18:27:40 2014 +0300
>     
>         drm/i915: respect the VBT minimum backlight brightness

The first commit ("drm/i915: respect the VBT minimum backlight brightness") is the real culprit in your case. It starts restricting the minimum brightness based on VBT. The VBT value is supposed to have been selected by the OEM to match the hardware requirements.

The second commit ("drm/i915: safeguard against too high minimum brightness") ensures the limit isn't "too high". It should *not* be effective in your case, where the minimum per VBT is (15/255)*max, i.e. you can't go below 5%.

A few additional things to consider.

First, does 5% brightness running a kernel before the offending commit correspond to lowest brightness running a kernel after the offending commit?

Second, there might be a catch here. The brightness UI, whichever it may be, might provide you with an arbitrary number of steps. The UI might not let you choose the lowest non-zero brightness. E.g. going up from 0, it probably jumps to something other than 1.

See if this gives you more desirable results:

# echo 1 > /sys/class/brightness/intel_backlight/brightness

(In reply to Jani Nikula from comment #2)
> (In reply to Aurabindo J from comment #0)
> > Associated commit:
> > 
> > commit e1c412e75754ab7b7002f3e18a2652d999c40d4b
> > Author: Jani Nikula <jani.nikula@intel.com>
> > Date:   Wed Nov 5 14:46:31 2014 +0200
> > 
> >     drm/i915: safeguard against too high minimum brightness
> >     
> >     Never trust (your interpretation of) the VBT. Regression from
> >     
> >     commit 6dda730e55f412a6dfb181cae6784822ba463847
> >     Author: Jani Nikula <jani.nikula@intel.com>
> >     Date:   Tue Jun 24 18:27:40 2014 +0300
> >     
> >         drm/i915: respect the VBT minimum backlight brightness
> 
> The first commit ("drm/i915: respect the VBT minimum backlight brightness")
> is the real culprit in your case. It starts restricting the minimum
> brightness based on VBT. The VBT value is supposed to have been selected by
> the OEM to match the hardware requirements.

Yes, I noticed it right the first time itself, thats why I included info about both the commits.

> 
> The second commit ("drm/i915: safeguard against too high minimum
> brightness") ensures the limit isn't "too high". It should *not* be
> effective in your case, where the minimum per VBT is (15/255)*max, i.e. you
> can't go below 5%.

my /sys/class/backlight/intel_backlight/max_brightness says 5273. So percentage is calculated relative to this value ? The minimum level I can set is 64 after the offending commit, which is obviously the arbitrary value hardcoded. It is neither the 5%, which should have been ~264, nor 25%, which would be ~1319

If you meant 5% of *255* (and it is), which would be ~ 13, and it is not the new minimum for me, rather 64. So where does 15 come into picture? Wild rounding or anything else? And please clarify what "max" you meant.

Older one also showed proper actual_brightness. But latest kernel always gives 0. By older one, I meant the one without *both* of the referenced commit. I didnt test it in between. Actually 3.10 (the one in centos 7). The newer on is from arch (3.17)

(In reply to Jani Nikula from comment #3)
> A few additional things to consider.
> 
> First, does 5% brightness running a kernel before the offending commit
> correspond to lowest brightness running a kernel after the offending commit?

No, since 5% (of 255)  brightness is at 12 and the new minimum in current kernel is 64.

> 
> Second, there might be a catch here. The brightness UI, whichever it may be,
> might provide you with an arbitrary number of steps. The UI might not let
> you choose the lowest non-zero brightness. E.g. going up from 0, it probably
> jumps to something other than 1.

Yes the brightness UI always goes in steps, though not linearly. It seems to be exponential since at times I see very lower brightness states (before offending commits) between the usually-smallest and turned-off state when I am pressing holding brightness down key from the highest brightness level. I dont get this intermediate level when for going down from other states, for example second lowest.

However, it is letting me choose the smallest level of 64 (in  newer kernels).  Since I cannot read the actual_brightness to know the current level (it is always 0 in new kernel). But I confirmed it by manually echoing the test case values and visually inspecting for any change in brightness for the minimum value supported  by UI. For all values above 64, the UI was able to reduce the brightness by a some amount.

> 
> See if this gives you more desirable results:
> 
> # echo 1 > /sys/class/brightness/intel_backlight/brightness

This still puts the current level to 64, which is a bit on the high side.

Please let me know other details you need

Please attach dmesg with drm.debug=14 module parameter set with a newer kernel (=including both commits).

15 is a value extracted from the VBT (which is in the opregion you attached). It is a coefficient (15/255)*max where max is max_brightness. That translates to the max being about 5% of the max.

The minimum brightness should *not* affect what you can set the brightness interface to. All values 0..max_brightness are allowed, and it should just scale the input suitable for hardware.

(In reply to Aurabindo J from comment #4)
> my /sys/class/backlight/intel_backlight/max_brightness says 5273. So
> percentage is calculated relative to this value ? The minimum level I can
> set is 64 after the offending commit, which is obviously the arbitrary value
> hardcoded.

The commits should have no impact on what you can write to the brightness interface.

> Older one also showed proper actual_brightness. But latest kernel always
> gives 0. By older one, I meant the one without *both* of the referenced
> commit. I didnt test it in between. Actually 3.10 (the one in centos 7). The
> newer on is from arch (3.17)

That's odd. What's the machine?

(In reply to Jani Nikula from comment #7)

> 
> That's odd. What's the machine?

Here is the DMI decode http://pastebin.com/hS5UtPxm

Created attachment 110462 [details]
Dmesg with drm.debug=14

Hi Jani,

This still needs to be in NEEDINFO. Do you need any other details ?

Please try video.use_native_backlight=0 module parameter.

(In reply to Jani Nikula from comment #11)
> Please try video.use_native_backlight=0 module parameter.

I'm afraid that doesn't change anything. The minimum level stays as such, at 64.
new dmesg with drm debug option and video.use_native_backlight=0 is at http://pastebin.com/0nFxW3qP

This bug is preventing us from upgrading the kernel. Please bring back the old behaviour: give us a way to override the restriction on minimum backlight brightness.

Adi H (3aditya3@gmail.com), is this problem still valid?

based on the lack of activity and a response from the submitter to update results with latest configuration the bug will be closed

Hi,

Apologies for the inactivity.

I'd like a more sensible default rather than the solution "minimum brightness is [an arbitrary] 25% of maximum brightness"

Considering this as an enhancement, setting the priority to Low.

Created attachment 145362 [details] [review]
Add module parameter to enable override of minimum brightness

Last time I checked, there was a disagreement about adding a new module parameter. Its been almost 5 years now. I hope you will consider now.

You are reporter of the issue currently having low priority. Do you still see issue. If so, please spesify clearly what is impact to you.

(In reply to Jani Saarinen from comment #19)
> You are reporter of the issue currently having low priority. Do you still
> see issue. If so, please spesify clearly what is impact to you.

Hi Jani,

Yes I still have the issue. The minimum brightness available on my display simply hurt my eyes and without this patch I cant use my laptop. Even if you say that its going to damage the hardware, I would still insist, because I'd rather have my hardware fail first, than my eyes.

I need a way to force at the brightness to go down at least 10% of the max brightness. My safe level is 'echo 100 > /sys/class/backlight/intel_backlight/brightness' which works only if I have the attached patch that allows to override the min brightness value from VBT. I use the value "-1" for the min_brightness module parameter. Please see the attached patch.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/drm/intel/issues/20.