dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Garrett <mjg59@srcf.ucam.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Pan@freedesktop.org, "Karol Herbst" <kherbst@redhat.com>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	platform-driver-x86@vger.kernel.org,
	"Dmitry Osipenko" <digetx@gmail.com>,
	amd-gfx@lists.freedesktop.org, linux-acpi@vger.kernel.org,
	"Ben Skeggs" <bskeggs@redhat.com>,
	"David Airlie" <airlied@redhat.com>,
	"Len Brown" <lenb@kernel.org>, "Daniel Dadap" <ddadap@nvidia.com>,
	"Jani Nikula" <jani.nikula@intel.com>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	"Mark Gross" <markgross@kernel.org>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Mika Westerberg" <mika.westerberg@linux.intel.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
	Xinhui <Xinhui.Pan@amd.com>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>
Subject: Re: [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)
Date: Wed, 26 Oct 2022 00:40:40 +0100	[thread overview]
Message-ID: <20221025234040.GA27673@srcf.ucam.org> (raw)
In-Reply-To: <cb5add36-c13c-ccd5-1b4b-71b45163a170@redhat.com>

On Wed, Oct 26, 2022 at 01:27:25AM +0200, Hans de Goede wrote:

> this code should actually set the ACPI_VIDEO_BACKLIGHT flag:
> drivers/acpi/scan.c:
> 
> static acpi_status
> acpi_backlight_cap_match(acpi_handle handle, u32 level, void *context,
>                           void **return_value)
> {
>         long *cap = context;
> 
>         if (acpi_has_method(handle, "_BCM") &&
>             acpi_has_method(handle, "_BCL")) {
>                 acpi_handle_debug(handle, "Found generic backlight support\n");
>                 *cap |= ACPI_VIDEO_BACKLIGHT;
>                 /* We have backlight support, no need to scan further */
>                 return AE_CTRL_TERMINATE;
>         }
>         return 0;
> }

Ah, yeah, my local tree no longer matches the upstream behaviour because 
I've hacked the EC firmware to remove the backlight trigger because it 
had an extremely poor brightness curve and also automatically changed it 
on AC events - as a result I removed the backlight code from the DSDT 
and just fell back to the native control. Like I said I'm a long way 
from the normal setup, but this did previously work.

The "right" logic here seems pretty simple: if ACPI backlight control is 
expected to work, use it. If it isn't, but there's a vendor interface, 
use it. If there's no vendor interface, use the native interface. The 
problem you're dealing with is that the knowledge of whether or not 
there's a vendor interface isn't something the core kernel code knows 
about. What you're proposing here is effectively for us to expose 
additional information about whether or not there's a vendor interface 
in the system firmware, but since we're talking in some cases about 
hardware that's almost 20 years old, we're not realistically going to 
get those old machines fixed. So, it feels like there's two choices:

1) Make a default policy decision, but then allow that decision to be 
altered later on (eg, when a vendor-specific platform driver has been 
loaded) - you've said this poses additional complexities.

2) Move the knowledge of whether or not there's a vendor interface into 
the core code. Basically take every platform driver that exposes a 
vendor interface, and move the detection code into the core.

I think any other approach is going to result in machines that 
previously worked no longer working (and you can't just make the 
vendor/native split dependent on the Coreboot DMI BIOS string, because 
there are some Coreboot platforms that implement the vendor interface 
for compatibility, and you also can't ask all Coreboot users to update 
their firmware to fix things)

  reply	other threads:[~2022-10-25 23:40 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-25 14:36 [PATCH v5 00/31] drm/kms: Stop registering multiple /sys/class/backlight devs for a single display Hans de Goede
2022-08-25 14:36 ` [PATCH v5 01/31] ACPI: video: Add acpi_video_backlight_use_native() helper Hans de Goede
2022-08-25 14:36 ` [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2) Hans de Goede
2022-09-25 23:39   ` Dmitry Osipenko
2022-09-27 11:04     ` Hans de Goede
2022-10-24 20:30       ` Matthew Garrett
2022-10-25 18:50         ` Hans de Goede
2022-10-25 19:32           ` Matthew Garrett
2022-10-25 20:25             ` Hans de Goede
2022-10-25 20:31               ` Limonciello, Mario
2022-10-25 20:32               ` Hans de Goede
2022-10-25 20:40               ` Matthew Garrett
2022-10-25 23:27                 ` Hans de Goede
2022-10-25 23:40                   ` Matthew Garrett [this message]
2022-10-26  9:59                     ` Hans de Goede
2022-10-26 20:49                       ` Matthew Garrett
2022-10-27  8:51                         ` Hans de Goede
2022-10-27  9:11                           ` Matthew Garrett
2022-10-27  9:39                             ` Hans de Goede
2022-10-27  9:52                               ` Matthew Garrett
2022-10-27 10:37                                 ` Hans de Goede
2022-10-27 12:09                                   ` Rafael J. Wysocki
2022-10-27 12:17                                     ` Hans de Goede
2022-10-27 12:19                                       ` Rafael J. Wysocki
2022-11-04 16:23                                     ` Hans de Goede
2022-08-25 14:36 ` [PATCH v5 03/31] drm/amdgpu: Don't register backlight when another backlight should be used (v3) Hans de Goede
2022-08-25 14:36 ` [PATCH v5 04/31] drm/radeon: " Hans de Goede
2022-08-25 14:37 ` [PATCH v5 05/31] drm/nouveau: Don't register backlight when another backlight should be used (v2) Hans de Goede
2022-08-25 14:37 ` [PATCH v5 06/31] ACPI: video: Drop backlight_device_get_by_type() call from acpi_video_get_backlight_type() Hans de Goede
2022-08-25 14:37 ` [PATCH v5 07/31] ACPI: video: Remove acpi_video_bus from list before tearing it down Hans de Goede
2022-08-25 14:37 ` [PATCH v5 08/31] ACPI: video: Simplify acpi_video_unregister_backlight() Hans de Goede
2022-08-25 14:37 ` [PATCH v5 09/31] ACPI: video: Make backlight class device registration a separate step (v2) Hans de Goede
2022-08-25 14:37 ` [PATCH v5 10/31] ACPI: video: Remove code to unregister acpi_video backlight when a native backlight registers Hans de Goede
2022-08-25 14:37 ` [PATCH v5 11/31] drm/i915: Call acpi_video_register_backlight() (v3) Hans de Goede
2022-09-02 13:20   ` Jani Nikula
2022-08-25 14:37 ` [PATCH v5 12/31] drm/nouveau: Register ACPI video backlight when nv_backlight registration fails (v2) Hans de Goede
2022-08-25 14:37 ` [PATCH v5 13/31] drm/amdgpu: Register ACPI video backlight when skipping amdgpu backlight registration Hans de Goede
2022-08-25 14:37 ` [PATCH v5 14/31] drm/radeon: Register ACPI video backlight when skipping radeon " Hans de Goede
2022-08-25 14:37 ` [PATCH v5 15/31] platform/x86: nvidia-wmi-ec-backlight: Move fw interface definitions to a header (v2) Hans de Goede
2022-08-25 22:11   ` Daniel Dadap
2022-08-25 14:37 ` [PATCH v5 16/31] ACPI: video: Refactor acpi_video_get_backlight_type() a bit Hans de Goede
2022-08-25 14:37 ` [PATCH v5 17/31] ACPI: video: Add Nvidia WMI EC brightness control detection (v3) Hans de Goede
2022-08-25 22:21   ` Daniel Dadap
2022-08-29 11:41     ` Hans de Goede
2022-08-29 16:43       ` Daniel Dadap
2022-08-25 14:37 ` [PATCH v5 18/31] ACPI: video: Add Apple GMUX brightness control detection Hans de Goede
2022-08-25 14:37 ` [PATCH v5 19/31] platform/x86: nvidia-wmi-ec-backlight: Use acpi_video_get_backlight_type() Hans de Goede
2022-08-25 14:37 ` [PATCH v5 20/31] platform/x86: apple-gmux: Stop calling acpi/video.h functions Hans de Goede
2022-08-25 14:37 ` [PATCH v5 21/31] platform/x86: toshiba_acpi: Stop using acpi_video_set_dmi_backlight_type() Hans de Goede
2022-08-25 14:37 ` [PATCH v5 22/31] platform/x86: acer-wmi: Move backlight DMI quirks to acpi/video_detect.c Hans de Goede
2022-08-25 14:37 ` [PATCH v5 23/31] platform/x86: asus-wmi: Drop DMI chassis-type check from backlight handling Hans de Goede
2022-08-25 14:37 ` [PATCH v5 24/31] platform/x86: asus-wmi: Move acpi_backlight=vendor quirks to ACPI video_detect.c Hans de Goede
2022-08-25 14:37 ` [PATCH v5 25/31] platform/x86: asus-wmi: Move acpi_backlight=native " Hans de Goede
2022-08-25 14:37 ` [PATCH v5 26/31] platform/x86: samsung-laptop: Move acpi_backlight=[vendor|native] " Hans de Goede
2022-08-25 14:37 ` [PATCH v5 27/31] ACPI: video: Remove acpi_video_set_dmi_backlight_type() Hans de Goede
2022-08-25 14:37 ` [PATCH v5 28/31] ACPI: video: Drop "Samsung X360" acpi_backlight=native quirk Hans de Goede
2022-08-25 14:37 ` [PATCH v5 29/31] ACPI: video: Drop NL5x?U, PF4NU1F and PF5?U?? acpi_backlight=native quirks Hans de Goede
2022-08-25 14:37 ` [PATCH v5 30/31] ACPI: video: Fix indentation of video_detect_dmi_table[] entries Hans de Goede
2022-08-25 14:37 ` [PATCH v5 31/31] drm/todo: Add entry about dealing with brightness control on devices with > 1 panel Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221025234040.GA27673@srcf.ucam.org \
    --to=mjg59@srcf.ucam.org \
    --cc=Pan@freedesktop.org \
    --cc=Xinhui.Pan@amd.com \
    --cc=airlied@redhat.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=andy@kernel.org \
    --cc=bskeggs@redhat.com \
    --cc=christian.koenig@amd.com \
    --cc=ddadap@nvidia.com \
    --cc=digetx@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hdegoede@redhat.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=kherbst@redhat.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=markgross@kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=nouveau@lists.freedesktop.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=tvrtko.ursulin@linux.intel.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).