From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?U3TDqXBoYW5lIEFOQ0VMT1Q=?= Subject: Re: [PATCH v6] drm/i915: respect the VBT minimum backlight brightness again Date: Thu, 12 Nov 2015 09:40:03 +0100 Message-ID: <56445063.2060105@free.fr> References: <1447051331-11829-1-git-send-email-sylee@canonical.com> <1447307039-11147-1-git-send-email-sylee@canonical.com> <564448EE.9000908@free.fr> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2121421848==" Return-path: Received: from smtp3-g21.free.fr (smtp3-g21.free.fr [212.27.42.3]) by gabe.freedesktop.org (Postfix) with ESMTPS id 340566E2B1 for ; Thu, 12 Nov 2015 00:40:07 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Shih-Yuan Lee (FourDollars)" Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org This is a multi-part message in MIME format. --===============2121421848== Content-Type: multipart/alternative; boundary="------------030800050004010908030105" This is a multi-part message in MIME format. --------------030800050004010908030105 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable On 12/11/2015 09:28, Shih-Yuan Lee (FourDollars) wrote: > I think the first step is to identify which backlight interface is=20 > able to actually change the brightness. > Usually there are two backlight interfaces under=20 > /sys/class/backlight/, i.e. /sys/class/backlight/acpi_video0 and=20 > /sys/class/backlight/intel_backlight. > However you may see other different backlight interfaces under=20 > /sys/class/backlight/. It depends on what hardware you are using. > Executing `cat max_brightness` under those folders can see the max=20 > brightness and then you can execute `echo $(($(cat max_brightness)/2))=20 > | sudo tee brightness` to adjust it to about 50% brightness. > only /sys/class/backlight/intel_backlight is present. max_brightness is 937 these commands have no effect on display brightness: echo 0 >brightness echo 468 >brightness echo 937 >brightness Regards, Steph > On Thu, Nov 12, 2015 at 4:08 PM, St=C3=A9phane ANCELOT > wrote: > > Hi, > I have seen you were working on brightness. Right, but this is > useful only if many worlwide users would be able to activate it. > I have never been able to make it working on my intel systemes > (core i5 - 4300u , core i3 ...) > Very difficult to check where the problem is coming from, altough > you have either an intel_backlight or acpi_backlight folder in the > sys directory > Regarding many many posts on the web, this sounds tricky and > difficult to know why it has not worked, or simply the > documentation is lacking .... > > Regards, > S.Ancelot > > > On 12/11/2015 06:43, Shih-Yuan Lee (FourDollars) wrote: > > There was a wonderful period after > > commit 6dda730e55f412a6dfb181cae6784822ba463847 > Author: Jani Nikula > > Date: Tue Jun 24 18:27:40 2014 +0300 > > drm/i915: respect the VBT minimum backlight brightness > > The backlight class 0 brightness means the PWM min and it does > not turn > off the backlight. After kernel 3.18, the backlight class 0 > brightness > is used to turn off the backlight and the PWM min is missing. > > Because of > > commit e6755fb78e8f20ecadf2a4080084121336624ad9 > Author: Jani Nikula > > Date: Tue Aug 12 17:11:42 2014 +0300 > > drm/i915: switch off backlight for backlight class 0 > brightness > > Use "VBT backlight PWM modulation frequency 200 Hz, active > high, min > brightness 10, level 255" as an example. It means the VBT min > is 10 > out of [0..255] and the PWM max is 937 from other place so the > corresponding PWM min should be 37 (10 * 937 / 255). > > When we set backlight class 0 brightness, the backlight is > turned off. > Althought the PWM value is 37 when the backlight is off but it = is > useless. When we set set backlight class 1 brightness, the > backlight is > on but the PWM value is 38 and it doesn't match the > corresponding PWM > min of the VBT minimum backlight. > > And it has another minor issue that there are some backlight cl= ass > brightness values are mapped to the same PWM value because the > backlight > class brightness range is larger than the valid PWM brightness > range. > > The backlight class brightness range [0..937] > The valid PWM brightness range [37..937] > > This commit makes that backlight class 1 brightness means the > PWM min > and backlight class max_brightness means the PWM max, and the > ranges > become > > The backlight class brightness range [0..901] > The valid PWM brightness range [36..937] > > That's no backlight class brightness value mapped to the same > PWM value. > > Signed-off-by: Shih-Yuan Lee (FourDollars) > > > --- > drivers/gpu/drm/i915/intel_panel.c | 26 > ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > b/drivers/gpu/drm/i915/intel_panel.c > index b05c6d9..8efa199 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -1194,10 +1194,9 @@ static int > intel_backlight_device_register(struct intel_connector *connect= or) > props.type =3D BACKLIGHT_RAW; > /* > - * Note: Everything should work even if the backlight > device max > - * presented to the userspace is arbitrarily chosen. > + * Expose the whole valid PWM brightness range to the > backlight class. > */ > - props.max_brightness =3D panel->backlight.max; > + props.max_brightness =3D panel->backlight.max - > panel->backlight.min; > props.brightness =3D scale_hw_to_user(connector, > panel->backlight.level, > props.max_brightness); > @@ -1400,7 +1399,8 @@ static u32 get_backlight_min_vbt(struct > intel_connector *connector) > struct drm_device *dev =3D connector->base.dev; > struct drm_i915_private *dev_priv =3D dev->dev_private; > struct intel_panel *panel =3D &connector->panel; > - int min; > + int vbt_min; > + u32 pwm_min; > WARN_ON(panel->backlight.max =3D=3D 0); > @@ -1411,14 +1411,24 @@ static u32 > get_backlight_min_vbt(struct intel_connector *connector) > * against this by letting the minimum be at most > (arbitrarily chosen) > * 25% of the max. > */ > - min =3D clamp_t(int, > dev_priv->vbt.backlight.min_brightness, 0, 64); > - if (min !=3D dev_priv->vbt.backlight.min_brightness) { > + vbt_min =3D clamp_t(int, > dev_priv->vbt.backlight.min_brightness, 0, 64); > + if (vbt_min !=3D dev_priv->vbt.backlight.min_brightness= ) { > DRM_DEBUG_KMS("clamping VBT min backlight > %d/255 to %d/255\n", > - dev_priv->vbt.backlight.min_brightness, min); > + dev_priv->vbt.backlight.min_brightness, vbt_min); > } > /* vbt value is a coefficient in range [0..255] */ > - return scale(min, 0, 255, 0, panel->backlight.max); > + pwm_min =3D scale(vbt_min, 0, 255, 0, panel->backlight.= max); > + > + /* > + * Because backlight class brightness 0 is used to > turn off the backlight, we > + * need to step down a little bit here to make > backlight class brightness 1 > + * match the real PWM min. > + */ > + if (pwm_min > 0) > + return pwm_min - 1; > + else > + return 0; > } > static int lpt_setup_backlight(struct intel_connector > *connector, enum pipe unused) > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > --------------030800050004010908030105 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable
On 12/11/2015 09:28, Shih-Yuan Lee (FourDollars) wrote:
I think the first step is to identify which backlight interface is able to actually change the brightness.
Usually there are two backlight=C2=A0interfaces=C2=A0under /sys/class/backlight/, i.e. /sys/class/backlight/acpi_video0 and /sys/class/backlight/intel_backlight.
However you may see other different backlight=C2=A0interface= s under /sys/class/backlight/. It depends on what hardware you are using.
Executing `cat=C2=A0max_brightness` under those folders can = see the max brightness and then you can execute `echo $(($(cat max_brightness)/2)) | sudo tee brightness` to adjust it to about 50% brightness.

only /sys/class/backlight/intel_backlight is present.
max_brightness is 937

these commands have no effect on display brightness:
echo 0 >brightness
echo 468 >brightness
echo 937 >brightness

Regards,
Steph


On Thu, Nov 12, 2015 at 4:08 PM, St=C3=A9phane ANCELOT <sancelot@free.fr> wrote:
Hi,
I have seen you were working on brightness. Right, but this is useful only if many worlwide users would be able to activate it.
I have never been able to make it working on my intel systemes (core i5 - 4300u , core i3 ...)
Very difficult to check where the problem is coming from, altough you have either an intel_backlight or acpi_backlight folder in the sys directory
Regarding many many posts on the web, this sounds tricky and difficult to know why it has not worked, or simply the documentation is lacking ....

Regards,
S.Ancelot


On 12/11/2015 06:43, Shih-Yuan Lee (FourDollars) wrote:
There was a wonderful period after

commit 6dda730e55f412a6dfb181cae6784822ba463847
Author: Jani Nikula <jani.nikula at intel.com>= ;
Date:=C2=A0 =C2=A0Tue Jun 24 18:27:40 2014 +0300

=C2=A0 =C2=A0 =C2=A0drm/i915: respect the VBT minimum b= acklight brightness

The backlight class 0 brightness means the PWM min and it does not turn
off the backlight. After kernel 3.18, the backlight class 0 brightness
is used to turn off the backlight and the PWM min is missing.

Because of

commit e6755fb78e8f20ecadf2a4080084121336624ad9
Author: Jani Nikula <jani.nikula at intel.com>= ;
Date:=C2=A0 =C2=A0Tue Aug 12 17:11:42 2014 +0300

=C2=A0 =C2=A0 =C2=A0drm/i915: switch off backlight for = backlight class 0 brightness

Use "VBT backlight PWM modulation frequency 200 Hz, active high, min
brightness 10, level 255" as an example. It means the VBT min is 10
out of [0..255] and the PWM max is 937 from other place so the
corresponding PWM min should be 37 (10 * 937 / 255).
When we set backlight class 0 brightness, the backlight is turned off.
Althought the PWM value is 37 when the backlight is off but it is
useless. When we set set backlight class 1 brightness, the backlight is
on but the PWM value is 38 and it doesn't match the corresponding PWM
min of the VBT minimum backlight.

And it has another minor issue that there are some backlight class
brightness values are mapped to the same PWM value because the backlight
class brightness range is larger than the valid PWM brightness range.

The backlight class brightness range [0..937]
The valid PWM brightness range [37..937]

This commit makes that backlight class 1 brightness means the PWM min
and backlight class max_brightness means the PWM max, and the ranges
become

The backlight class brightness range [0..901]
The valid PWM brightness range [36..937]

That's no backlight class brightness value mapped to the same PWM value.

Signed-off-by: Shih-Yuan Lee (FourDollars) <sylee@canonical.com>
---
=C2=A0 drivers/gpu/drm/i915/intel_panel.c | 26 ++++++++++++++++++--------
=C2=A0 1 file changed, 18 insertions(+), 8 deletions(-)=

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index b05c6d9..8efa199 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1194,10 +1194,9 @@ static int intel_backlight_device_register(struct intel_connector *connector)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 props.type =3D BACKLIGHT_RA= W;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 /*
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 * Note: Everything should = work even if the backlight device max
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 * presented to the userspa= ce is arbitrarily chosen.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 * Expose the whole valid P= WM brightness range to the backlight class.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
-=C2=A0 =C2=A0 =C2=A0 =C2=A0props.max_brightness =3D panel->backlight.max;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0props.max_brightness =3D pa= nel->backlight.max - panel->backlight.min;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 props.brightness =3D scale_= hw_to_user(connector,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 panel->backlight.level,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 props.max_brightness);
@@ -1400,7 +1399,8 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 struct drm_device *dev =3D connector->base.dev;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 struct drm_i915_private *de= v_priv =3D dev->dev_private;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 struct intel_panel *panel =3D &connector->panel;
-=C2=A0 =C2=A0 =C2=A0 =C2=A0int min;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0int vbt_min;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0u32 pwm_min;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 WARN_ON(panel->backlight= .max =3D=3D 0);
=C2=A0 @@ -1411,14 +1411,24 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* against this by let= ting the minimum be at most (arbitrarily chosen)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* 25% of the max.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
-=C2=A0 =C2=A0 =C2=A0 =C2=A0min =3D clamp_t(int, dev_priv->vbt.backlight.min_brightness, 0, 64);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0if (min !=3D dev_priv->vbt.backlight.min_brightness) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0vbt_min =3D clamp_t(int, dev_priv->vbt.backlight.min_brightness, 0, 64);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0if (vbt_min !=3D dev_priv->vbt.backlight.min_brightness) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= DRM_DEBUG_KMS("clamping VBT min backlight %d/255 to %d/255\n",
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev_priv->vbt.backlight.min_brightness, min);<= br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev_priv->vbt.backlight.min_brightness, vbt_mi= n);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* vbt value is a coefficie= nt in range [0..255] */
-=C2=A0 =C2=A0 =C2=A0 =C2=A0return scale(min, 0, 255, 0= , panel->backlight.max);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0pwm_min =3D scale(vbt_min, = 0, 255, 0, panel->backlight.max);
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0/*
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 * Because backlight class = brightness 0 is used to turn off the backlight, we
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 * need to step down a litt= le bit here to make backlight class brightness 1
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 * match the real PWM min.<= br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 */
+=C2=A0 =C2=A0 =C2=A0 =C2=A0if (pwm_min > 0)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= return pwm_min - 1;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0else
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= return 0;
=C2=A0 }
=C2=A0 =C2=A0 static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unused)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedeskt= op.org/mailman/listinfo/intel-gfx


--------------030800050004010908030105-- --===============2121421848== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============2121421848==--