| Message ID | 20220712193910.439171-4-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/kms: Stop registering multiple /sys/class/backlight devs for a single display | expand |
On Tue, Jul 12, 2022 at 3:39 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Before this commit when we want userspace to use the acpi_video backlight > device we register both the GPU's native backlight device and acpi_video's > firmware acpi_video# backlight device. This relies on userspace preferring > firmware type backlight devices over native ones. > > Registering 2 backlight devices for a single display really is > undesirable, don't register the GPU's native backlight device when > another backlight device should be used. > > Changes in v2: > - To avoid linker errors when amdgpu is builtin and video_detect.c is in > a module, select ACPI_VIDEO and its deps if ACPI && X86 are enabled. > When these are not set, ACPI_VIDEO is disabled, ensuring the stubs > from acpi/video.h will be used. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/Kconfig | 6 ++++++ > drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 7 +++++++ > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 +++++++ > 3 files changed, 20 insertions(+) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index aaa7ad1f0614..d65119860760 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -258,6 +258,12 @@ config DRM_AMDGPU > select HWMON > select BACKLIGHT_CLASS_DEVICE > select INTERVAL_TREE > + # amdgpu depends on ACPI_VIDEO when X86 and ACPI are both enabled > + # for select to work, ACPI_VIDEO's dependencies must also be selected > + select INPUT if ACPI && X86 > + select X86_PLATFORM_DEVICES if ACPI && X86 > + select ACPI_WMI if ACPI && X86 > + select ACPI_VIDEO if ACPI && X86 > help > Choose this option if you have a recent AMD Radeon graphics card. > > diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c > index fa7421afb9a6..abf209e36fca 100644 > --- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c > +++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c > @@ -26,6 +26,8 @@ > > #include <linux/pci.h> > > +#include <acpi/video.h> > + > #include <drm/drm_crtc_helper.h> > #include <drm/amdgpu_drm.h> > #include "amdgpu.h" > @@ -184,6 +186,11 @@ void amdgpu_atombios_encoder_init_backlight(struct amdgpu_encoder *amdgpu_encode > if (!(adev->mode_info.firmware_flags & ATOM_BIOS_INFO_BL_CONTROLLED_BY_GPU)) > return; > > + if (!acpi_video_backlight_use_native()) { > + DRM_INFO("Skipping amdgpu atom DIG backlight registration\n"); > + return; > + } > + > pdata = kmalloc(sizeof(struct amdgpu_backlight_privdata), GFP_KERNEL); > if (!pdata) { > DRM_ERROR("Memory allocation failed\n"); > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 5eb111d35793..3b03a95e59a8 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -86,6 +86,8 @@ > #include <drm/drm_audio_component.h> > #include <drm/drm_gem_atomic_helper.h> > > +#include <acpi/video.h> > + > #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h" > > #include "dcn/dcn_1_0_offset.h" > @@ -4050,6 +4052,11 @@ amdgpu_dm_register_backlight_device(struct amdgpu_display_manager *dm) > amdgpu_dm_update_backlight_caps(dm, dm->num_of_edps); > dm->brightness[dm->num_of_edps] = AMDGPU_MAX_BL_LEVEL; > > + if (!acpi_video_backlight_use_native()) { > + DRM_INFO("Skipping amdgpu DM backlight registration\n"); > + return; > + } > + > props.max_brightness = AMDGPU_MAX_BL_LEVEL; > props.brightness = AMDGPU_MAX_BL_LEVEL; > props.type = BACKLIGHT_RAW; > -- > 2.36.0 >
On Wed, Jul 20, 2022 at 12:44 PM Alex Deucher <alexdeucher@gmail.com> wrote: > > On Tue, Jul 12, 2022 at 3:39 PM Hans de Goede <hdegoede@redhat.com> wrote: > > > > Before this commit when we want userspace to use the acpi_video backlight > > device we register both the GPU's native backlight device and acpi_video's > > firmware acpi_video# backlight device. This relies on userspace preferring > > firmware type backlight devices over native ones. > > > > Registering 2 backlight devices for a single display really is > > undesirable, don't register the GPU's native backlight device when > > another backlight device should be used. > > > > Changes in v2: > > - To avoid linker errors when amdgpu is builtin and video_detect.c is in > > a module, select ACPI_VIDEO and its deps if ACPI && X86 are enabled. > > When these are not set, ACPI_VIDEO is disabled, ensuring the stubs > > from acpi/video.h will be used. > > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Acked-by: Alex Deucher <alexander.deucher@amd.com> Actually, can you use dev_info for the messages below rather than DRM_INFO? That makes it easier to tell which GPU is affected in a multi-GPU system. With that changed, Acked-by: Alex Deucher <alexander.deucher@amd.com> > > > --- > > drivers/gpu/drm/Kconfig | 6 ++++++ > > drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 7 +++++++ > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 +++++++ > > 3 files changed, 20 insertions(+) > > > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > > index aaa7ad1f0614..d65119860760 100644 > > --- a/drivers/gpu/drm/Kconfig > > +++ b/drivers/gpu/drm/Kconfig > > @@ -258,6 +258,12 @@ config DRM_AMDGPU > > select HWMON > > select BACKLIGHT_CLASS_DEVICE > > select INTERVAL_TREE > > + # amdgpu depends on ACPI_VIDEO when X86 and ACPI are both enabled > > + # for select to work, ACPI_VIDEO's dependencies must also be selected > > + select INPUT if ACPI && X86 > > + select X86_PLATFORM_DEVICES if ACPI && X86 > > + select ACPI_WMI if ACPI && X86 > > + select ACPI_VIDEO if ACPI && X86 > > help > > Choose this option if you have a recent AMD Radeon graphics card. > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c > > index fa7421afb9a6..abf209e36fca 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c > > +++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c > > @@ -26,6 +26,8 @@ > > > > #include <linux/pci.h> > > > > +#include <acpi/video.h> > > + > > #include <drm/drm_crtc_helper.h> > > #include <drm/amdgpu_drm.h> > > #include "amdgpu.h" > > @@ -184,6 +186,11 @@ void amdgpu_atombios_encoder_init_backlight(struct amdgpu_encoder *amdgpu_encode > > if (!(adev->mode_info.firmware_flags & ATOM_BIOS_INFO_BL_CONTROLLED_BY_GPU)) > > return; > > > > + if (!acpi_video_backlight_use_native()) { > > + DRM_INFO("Skipping amdgpu atom DIG backlight registration\n"); > > + return; > > + } > > + > > pdata = kmalloc(sizeof(struct amdgpu_backlight_privdata), GFP_KERNEL); > > if (!pdata) { > > DRM_ERROR("Memory allocation failed\n"); > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > index 5eb111d35793..3b03a95e59a8 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > > @@ -86,6 +86,8 @@ > > #include <drm/drm_audio_component.h> > > #include <drm/drm_gem_atomic_helper.h> > > > > +#include <acpi/video.h> > > + > > #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h" > > > > #include "dcn/dcn_1_0_offset.h" > > @@ -4050,6 +4052,11 @@ amdgpu_dm_register_backlight_device(struct amdgpu_display_manager *dm) > > amdgpu_dm_update_backlight_caps(dm, dm->num_of_edps); > > dm->brightness[dm->num_of_edps] = AMDGPU_MAX_BL_LEVEL; > > > > + if (!acpi_video_backlight_use_native()) { > > + DRM_INFO("Skipping amdgpu DM backlight registration\n"); > > + return; > > + } > > + > > props.max_brightness = AMDGPU_MAX_BL_LEVEL; > > props.brightness = AMDGPU_MAX_BL_LEVEL; > > props.type = BACKLIGHT_RAW; > > -- > > 2.36.0 > >
Hi, On 7/20/22 18:46, Alex Deucher wrote: > On Wed, Jul 20, 2022 at 12:44 PM Alex Deucher <alexdeucher@gmail.com> wrote: >> >> On Tue, Jul 12, 2022 at 3:39 PM Hans de Goede <hdegoede@redhat.com> wrote: >>> >>> Before this commit when we want userspace to use the acpi_video backlight >>> device we register both the GPU's native backlight device and acpi_video's >>> firmware acpi_video# backlight device. This relies on userspace preferring >>> firmware type backlight devices over native ones. >>> >>> Registering 2 backlight devices for a single display really is >>> undesirable, don't register the GPU's native backlight device when >>> another backlight device should be used. >>> >>> Changes in v2: >>> - To avoid linker errors when amdgpu is builtin and video_detect.c is in >>> a module, select ACPI_VIDEO and its deps if ACPI && X86 are enabled. >>> When these are not set, ACPI_VIDEO is disabled, ensuring the stubs >>> from acpi/video.h will be used. >>> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> >> Acked-by: Alex Deucher <alexander.deucher@amd.com> > > Actually, can you use dev_info for the messages below rather than > DRM_INFO? That makes it easier to tell which GPU is affected in a > multi-GPU system. With that changed, > Acked-by: Alex Deucher <alexander.deucher@amd.com> Thank you for the ack. I've changed the DRM_INFO(...) calls with drm_info(drm_dev, ...) calls for the version 3 of this series which I'm preparing. Regards, Hans > >> >>> --- >>> drivers/gpu/drm/Kconfig | 6 ++++++ >>> drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 7 +++++++ >>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 +++++++ >>> 3 files changed, 20 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig >>> index aaa7ad1f0614..d65119860760 100644 >>> --- a/drivers/gpu/drm/Kconfig >>> +++ b/drivers/gpu/drm/Kconfig >>> @@ -258,6 +258,12 @@ config DRM_AMDGPU >>> select HWMON >>> select BACKLIGHT_CLASS_DEVICE >>> select INTERVAL_TREE >>> + # amdgpu depends on ACPI_VIDEO when X86 and ACPI are both enabled >>> + # for select to work, ACPI_VIDEO's dependencies must also be selected >>> + select INPUT if ACPI && X86 >>> + select X86_PLATFORM_DEVICES if ACPI && X86 >>> + select ACPI_WMI if ACPI && X86 >>> + select ACPI_VIDEO if ACPI && X86 >>> help >>> Choose this option if you have a recent AMD Radeon graphics card. >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c >>> index fa7421afb9a6..abf209e36fca 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c >>> @@ -26,6 +26,8 @@ >>> >>> #include <linux/pci.h> >>> >>> +#include <acpi/video.h> >>> + >>> #include <drm/drm_crtc_helper.h> >>> #include <drm/amdgpu_drm.h> >>> #include "amdgpu.h" >>> @@ -184,6 +186,11 @@ void amdgpu_atombios_encoder_init_backlight(struct amdgpu_encoder *amdgpu_encode >>> if (!(adev->mode_info.firmware_flags & ATOM_BIOS_INFO_BL_CONTROLLED_BY_GPU)) >>> return; >>> >>> + if (!acpi_video_backlight_use_native()) { >>> + DRM_INFO("Skipping amdgpu atom DIG backlight registration\n"); >>> + return; >>> + } >>> + >>> pdata = kmalloc(sizeof(struct amdgpu_backlight_privdata), GFP_KERNEL); >>> if (!pdata) { >>> DRM_ERROR("Memory allocation failed\n"); >>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> index 5eb111d35793..3b03a95e59a8 100644 >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> @@ -86,6 +86,8 @@ >>> #include <drm/drm_audio_component.h> >>> #include <drm/drm_gem_atomic_helper.h> >>> >>> +#include <acpi/video.h> >>> + >>> #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h" >>> >>> #include "dcn/dcn_1_0_offset.h" >>> @@ -4050,6 +4052,11 @@ amdgpu_dm_register_backlight_device(struct amdgpu_display_manager *dm) >>> amdgpu_dm_update_backlight_caps(dm, dm->num_of_edps); >>> dm->brightness[dm->num_of_edps] = AMDGPU_MAX_BL_LEVEL; >>> >>> + if (!acpi_video_backlight_use_native()) { >>> + DRM_INFO("Skipping amdgpu DM backlight registration\n"); >>> + return; >>> + } >>> + >>> props.max_brightness = AMDGPU_MAX_BL_LEVEL; >>> props.brightness = AMDGPU_MAX_BL_LEVEL; >>> props.type = BACKLIGHT_RAW; >>> -- >>> 2.36.0 >>> >
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index aaa7ad1f0614..d65119860760 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -258,6 +258,12 @@ config DRM_AMDGPU select HWMON select BACKLIGHT_CLASS_DEVICE select INTERVAL_TREE + # amdgpu depends on ACPI_VIDEO when X86 and ACPI are both enabled + # for select to work, ACPI_VIDEO's dependencies must also be selected + select INPUT if ACPI && X86 + select X86_PLATFORM_DEVICES if ACPI && X86 + select ACPI_WMI if ACPI && X86 + select ACPI_VIDEO if ACPI && X86 help Choose this option if you have a recent AMD Radeon graphics card. diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c index fa7421afb9a6..abf209e36fca 100644 --- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c +++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c @@ -26,6 +26,8 @@ #include <linux/pci.h> +#include <acpi/video.h> + #include <drm/drm_crtc_helper.h> #include <drm/amdgpu_drm.h> #include "amdgpu.h" @@ -184,6 +186,11 @@ void amdgpu_atombios_encoder_init_backlight(struct amdgpu_encoder *amdgpu_encode if (!(adev->mode_info.firmware_flags & ATOM_BIOS_INFO_BL_CONTROLLED_BY_GPU)) return; + if (!acpi_video_backlight_use_native()) { + DRM_INFO("Skipping amdgpu atom DIG backlight registration\n"); + return; + } + pdata = kmalloc(sizeof(struct amdgpu_backlight_privdata), GFP_KERNEL); if (!pdata) { DRM_ERROR("Memory allocation failed\n"); diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 5eb111d35793..3b03a95e59a8 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -86,6 +86,8 @@ #include <drm/drm_audio_component.h> #include <drm/drm_gem_atomic_helper.h> +#include <acpi/video.h> + #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h" #include "dcn/dcn_1_0_offset.h" @@ -4050,6 +4052,11 @@ amdgpu_dm_register_backlight_device(struct amdgpu_display_manager *dm) amdgpu_dm_update_backlight_caps(dm, dm->num_of_edps); dm->brightness[dm->num_of_edps] = AMDGPU_MAX_BL_LEVEL; + if (!acpi_video_backlight_use_native()) { + DRM_INFO("Skipping amdgpu DM backlight registration\n"); + return; + } + props.max_brightness = AMDGPU_MAX_BL_LEVEL; props.brightness = AMDGPU_MAX_BL_LEVEL; props.type = BACKLIGHT_RAW;
Before this commit when we want userspace to use the acpi_video backlight device we register both the GPU's native backlight device and acpi_video's firmware acpi_video# backlight device. This relies on userspace preferring firmware type backlight devices over native ones. Registering 2 backlight devices for a single display really is undesirable, don't register the GPU's native backlight device when another backlight device should be used. Changes in v2: - To avoid linker errors when amdgpu is builtin and video_detect.c is in a module, select ACPI_VIDEO and its deps if ACPI && X86 are enabled. When these are not set, ACPI_VIDEO is disabled, ensuring the stubs from acpi/video.h will be used. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/gpu/drm/Kconfig | 6 ++++++ drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 7 +++++++ drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 +++++++ 3 files changed, 20 insertions(+)