Just one tiny nitpick below:
On Wed, 2022-08-24 at 14:14 +0200, Hans de Goede 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:
> - Add nouveau_acpi_video_backlight_use_native() wrapper to avoid unresolved
> symbol errors on non X86
>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> drivers/gpu/drm/nouveau/nouveau_acpi.c | 5 +++++
> drivers/gpu/drm/nouveau/nouveau_acpi.h | 2 ++
> drivers/gpu/drm/nouveau/nouveau_backlight.c | 6 ++++++
> 3 files changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> index 6140db756d06..1592c9cd7750 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> @@ -386,3 +386,8 @@ nouveau_acpi_edid(struct drm_device *dev, struct drm_connector *connector)
>
> return kmemdup(edid, EDID_LENGTH, GFP_KERNEL);
> }
> +
> +bool nouveau_acpi_video_backlight_use_native(void)
> +{
> + return acpi_video_backlight_use_native();
> +}
> diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.h b/drivers/gpu/drm/nouveau/nouveau_acpi.h
> index 330f9b837066..3c666c30dfca 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_acpi.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.h
> @@ -11,6 +11,7 @@ void nouveau_register_dsm_handler(void);
> void nouveau_unregister_dsm_handler(void);
> void nouveau_switcheroo_optimus_dsm(void);
> void *nouveau_acpi_edid(struct drm_device *, struct drm_connector *);
> +bool nouveau_acpi_video_backlight_use_native(void);
> #else
> static inline bool nouveau_is_optimus(void) { return false; };
> static inline bool nouveau_is_v1_dsm(void) { return false; };
> @@ -18,6 +19,7 @@ static inline void nouveau_register_dsm_handler(void) {}
> static inline void nouveau_unregister_dsm_handler(void) {}
> static inline void nouveau_switcheroo_optimus_dsm(void) {}
> static inline void *nouveau_acpi_edid(struct drm_device *dev, struct drm_connector *connector) { return NULL; }
> +static inline bool nouveau_acpi_video_backlight_use_native(void) { return true; }
> #endif
>
> #endif
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index a2141d3d9b1d..d2b8f8c13db4 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -38,6 +38,7 @@
> #include "nouveau_reg.h"
> #include "nouveau_encoder.h"
> #include "nouveau_connector.h"
> +#include "nouveau_acpi.h"
>
> static struct ida bl_ida;
> #define BL_NAME_SIZE 15 // 12 for name + 2 for digits + 1 for '\0'
> @@ -405,6 +406,11 @@ nouveau_backlight_init(struct drm_connector *connector)
> goto fail_alloc;
> }
>
> + if (!nouveau_acpi_video_backlight_use_native()) {
> + NV_INFO(drm, "Skipping nv_backlight registration\n");
> + goto fail_alloc;
> + }
We should probably make this say something like "No native backlight
interface, using ACPI instead" instead. With that fixed
Reviewed-by: Lyude Paul <lyude@xxxxxxxxxx>
> +
> if (!nouveau_get_backlight_name(backlight_name, bl)) {
> NV_ERROR(drm, "Failed to retrieve a unique name for the backlight interface\n");
> goto fail_alloc;
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
