diff mbox series

[6/6,SRU,OEM-6.0] ACPI: video: Prefer native over vendor

Message ID 20230427074116.188543-7-acelan.kao@canonical.com
State New
Headers show
Series both dell_backlight and nvidia_0 backlight interface appear, and can't adjust the display brightness | expand

Commit Message

AceLan Kao April 27, 2023, 7:41 a.m. UTC
From: Hans de Goede <hdegoede@redhat.com>

BugLink: https://launchpad.net/bugs/2017774

When available prefer native backlight control over vendor backlight
control.

Testing has shown that there are quite a few laptop models which rely
on native backlight control (they don't have ACPI video bus backlight
control) and on which acpi_osi_is_win8() returns false.

Currently __acpi_video_get_backlight_type() returns vendor on these
laptops, leading to an empty /sys/class/backlight.

As a workaround for this acpi_video_backlight_use_native() has been
temporarily changed to always return true.

This re-introduces the problem of having multiple backlight
devices under /sys/class/backlight for a single panel.

Change __acpi_video_get_backlight_type() to prefer native over vendor
when available. So that it returns native on these models.

And change acpi_video_backlight_use_native() back to only return
true when __acpi_video_get_backlight_type() returns native.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
(cherry picked from commit fb1836c91317e0770950260dfa91eb9b2170cb27)
Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
---
 drivers/acpi/video_detect.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

Comments

Andrei Gherzan April 28, 2023, 10 a.m. UTC | #1
On 23/04/27 03:41PM, AceLan Kao wrote:
> From: Hans de Goede <hdegoede@redhat.com>
> 
> BugLink: https://launchpad.net/bugs/2017774
> 
> When available prefer native backlight control over vendor backlight
> control.
> 
> Testing has shown that there are quite a few laptop models which rely
> on native backlight control (they don't have ACPI video bus backlight
> control) and on which acpi_osi_is_win8() returns false.
> 
> Currently __acpi_video_get_backlight_type() returns vendor on these
> laptops, leading to an empty /sys/class/backlight.
> 
> As a workaround for this acpi_video_backlight_use_native() has been
> temporarily changed to always return true.
> 
> This re-introduces the problem of having multiple backlight
> devices under /sys/class/backlight for a single panel.
> 
> Change __acpi_video_get_backlight_type() to prefer native over vendor
> when available. So that it returns native on these models.
> 
> And change acpi_video_backlight_use_native() back to only return
> true when __acpi_video_get_backlight_type() returns native.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> (cherry picked from commit fb1836c91317e0770950260dfa91eb9b2170cb27)

There are two follow-up fixes with DMI quirks:
9dcb34234b8235144c96103266317da33321077e
e6b3086fddc0065a5ffb947d4d29dd0e6efc327b

I think it would make sense to pull them too.

> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> ---
>  drivers/acpi/video_detect.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> index 37aaf541bca1..3c29d003a3a8 100644
> --- a/drivers/acpi/video_detect.c
> +++ b/drivers/acpi/video_detect.c
> @@ -680,10 +680,10 @@ static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
>  		return acpi_backlight_video;
>  
>  	/* Use native if available */
> -	if (native_available && prefer_native_over_acpi_video())
> +	if (native_available)
>  		return acpi_backlight_native;
>  
> -	/* No ACPI video (old hw), use vendor specific fw methods. */
> +	/* No ACPI video/native (old hw), use vendor specific fw methods. */
>  	return acpi_backlight_vendor;
>  }
>  
> @@ -695,19 +695,7 @@ EXPORT_SYMBOL(acpi_video_get_backlight_type);
>  
>  bool acpi_video_backlight_use_native(void)
>  {
> -	/*
> -	 * Call __acpi_video_get_backlight_type() to let it know that
> -	 * a native backlight is available.
> -	 */
> -	__acpi_video_get_backlight_type(true);
> -
> -	/*
> -	 * For now just always return true. There is a whole bunch of laptop
> -	 * models where (video_caps & ACPI_VIDEO_BACKLIGHT) is false causing
> -	 * __acpi_video_get_backlight_type() to return vendor, while these
> -	 * models only have a native backlight control.
> -	 */
> -	return true;
> +	return __acpi_video_get_backlight_type(true) == acpi_backlight_native;
>  }
>  EXPORT_SYMBOL(acpi_video_backlight_use_native);
>  
> -- 
> 2.34.1
AceLan Kao May 8, 2023, 6:40 a.m. UTC | #2
Hi Andrei,

Adding those "Fixes" patches and resolving the conflicts has led to a
significant increase in the number of patches (from 6 to 21). Although
there are still some DMI quirks that have not been included, I did not
pull all of them in because OEM-6.0 is soon to reach its end of life
and this kernel only focuses on our projects. I believe that the
number of patches that have been added should be sufficient for most
cases.

Please review the v2, thanks.
https://lists.ubuntu.com/archives/kernel-team/2023-May/139269.html

Andrei Gherzan <andrei.gherzan@canonical.com> 於 2023年4月28日 週五 下午6:01寫道:
>
> On 23/04/27 03:41PM, AceLan Kao wrote:
> > From: Hans de Goede <hdegoede@redhat.com>
> >
> > BugLink: https://launchpad.net/bugs/2017774
> >
> > When available prefer native backlight control over vendor backlight
> > control.
> >
> > Testing has shown that there are quite a few laptop models which rely
> > on native backlight control (they don't have ACPI video bus backlight
> > control) and on which acpi_osi_is_win8() returns false.
> >
> > Currently __acpi_video_get_backlight_type() returns vendor on these
> > laptops, leading to an empty /sys/class/backlight.
> >
> > As a workaround for this acpi_video_backlight_use_native() has been
> > temporarily changed to always return true.
> >
> > This re-introduces the problem of having multiple backlight
> > devices under /sys/class/backlight for a single panel.
> >
> > Change __acpi_video_get_backlight_type() to prefer native over vendor
> > when available. So that it returns native on these models.
> >
> > And change acpi_video_backlight_use_native() back to only return
> > true when __acpi_video_get_backlight_type() returns native.
> >
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > (cherry picked from commit fb1836c91317e0770950260dfa91eb9b2170cb27)
>
> There are two follow-up fixes with DMI quirks:
> 9dcb34234b8235144c96103266317da33321077e
> e6b3086fddc0065a5ffb947d4d29dd0e6efc327b
>
> I think it would make sense to pull them too.
>
> > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> > ---
> >  drivers/acpi/video_detect.c | 18 +++---------------
> >  1 file changed, 3 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> > index 37aaf541bca1..3c29d003a3a8 100644
> > --- a/drivers/acpi/video_detect.c
> > +++ b/drivers/acpi/video_detect.c
> > @@ -680,10 +680,10 @@ static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
> >               return acpi_backlight_video;
> >
> >       /* Use native if available */
> > -     if (native_available && prefer_native_over_acpi_video())
> > +     if (native_available)
> >               return acpi_backlight_native;
> >
> > -     /* No ACPI video (old hw), use vendor specific fw methods. */
> > +     /* No ACPI video/native (old hw), use vendor specific fw methods. */
> >       return acpi_backlight_vendor;
> >  }
> >
> > @@ -695,19 +695,7 @@ EXPORT_SYMBOL(acpi_video_get_backlight_type);
> >
> >  bool acpi_video_backlight_use_native(void)
> >  {
> > -     /*
> > -      * Call __acpi_video_get_backlight_type() to let it know that
> > -      * a native backlight is available.
> > -      */
> > -     __acpi_video_get_backlight_type(true);
> > -
> > -     /*
> > -      * For now just always return true. There is a whole bunch of laptop
> > -      * models where (video_caps & ACPI_VIDEO_BACKLIGHT) is false causing
> > -      * __acpi_video_get_backlight_type() to return vendor, while these
> > -      * models only have a native backlight control.
> > -      */
> > -     return true;
> > +     return __acpi_video_get_backlight_type(true) == acpi_backlight_native;
> >  }
> >  EXPORT_SYMBOL(acpi_video_backlight_use_native);
> >
> > --
> > 2.34.1
>
> --
> Andrei Gherzan
diff mbox series

Patch

diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 37aaf541bca1..3c29d003a3a8 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -680,10 +680,10 @@  static enum acpi_backlight_type __acpi_video_get_backlight_type(bool native)
 		return acpi_backlight_video;
 
 	/* Use native if available */
-	if (native_available && prefer_native_over_acpi_video())
+	if (native_available)
 		return acpi_backlight_native;
 
-	/* No ACPI video (old hw), use vendor specific fw methods. */
+	/* No ACPI video/native (old hw), use vendor specific fw methods. */
 	return acpi_backlight_vendor;
 }
 
@@ -695,19 +695,7 @@  EXPORT_SYMBOL(acpi_video_get_backlight_type);
 
 bool acpi_video_backlight_use_native(void)
 {
-	/*
-	 * Call __acpi_video_get_backlight_type() to let it know that
-	 * a native backlight is available.
-	 */
-	__acpi_video_get_backlight_type(true);
-
-	/*
-	 * For now just always return true. There is a whole bunch of laptop
-	 * models where (video_caps & ACPI_VIDEO_BACKLIGHT) is false causing
-	 * __acpi_video_get_backlight_type() to return vendor, while these
-	 * models only have a native backlight control.
-	 */
-	return true;
+	return __acpi_video_get_backlight_type(true) == acpi_backlight_native;
 }
 EXPORT_SYMBOL(acpi_video_backlight_use_native);