Karol Herbst
2021-Jul-23 22:46 UTC
[Nouveau] [PATCH] nouveau: make backlight support non optional
In the past this only led to compilation issues. Also the small amount of extra .text shouldn't really matter compared to the entire nouveau driver anyway. Cc: Arnd Bergmann <arnd at kernel.org> Cc: Lyude Paul <lyude at redhat.com> Cc: Ben Skeggs <bskeggs at redhat.com> Cc: Randy Dunlap <rdunlap at infradead.org> Cc: Daniel Vetter <daniel at ffwll.ch> Cc: nouveau at lists.freedesktop.org Cc: dri-devel at lists.freedesktop.org Fixes: 6eca310e8924 ("drm/nouveau/kms/nv50-: Add basic DPCD backlight support for nouveau") Signed-off-by: Karol Herbst <kherbst at redhat.com> --- drivers/gpu/drm/nouveau/Kbuild | 2 +- drivers/gpu/drm/nouveau/Kconfig | 13 ++--------- drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 ---- drivers/gpu/drm/nouveau/nouveau_connector.h | 24 --------------------- 4 files changed, 3 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/nouveau/Kbuild b/drivers/gpu/drm/nouveau/Kbuild index 60586fb8275e..f6957e7ad807 100644 --- a/drivers/gpu/drm/nouveau/Kbuild +++ b/drivers/gpu/drm/nouveau/Kbuild @@ -49,7 +49,7 @@ nouveau-y += nouveau_ttm.o nouveau-y += nouveau_vmm.o # DRM - modesetting -nouveau-$(CONFIG_DRM_NOUVEAU_BACKLIGHT) += nouveau_backlight.o +nouveau-y += nouveau_backlight.o nouveau-y += nouveau_bios.o nouveau-y += nouveau_connector.o nouveau-y += nouveau_display.o diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig index 9436310d0854..2e159b0ea7fb 100644 --- a/drivers/gpu/drm/nouveau/Kconfig +++ b/drivers/gpu/drm/nouveau/Kconfig @@ -7,14 +7,13 @@ config DRM_NOUVEAU select DRM_KMS_HELPER select DRM_TTM select DRM_TTM_HELPER - select BACKLIGHT_CLASS_DEVICE if DRM_NOUVEAU_BACKLIGHT - select ACPI_VIDEO if ACPI && X86 && BACKLIGHT_CLASS_DEVICE && INPUT + select BACKLIGHT_CLASS_DEVICE + select ACPI_VIDEO if ACPI && X86 && INPUT select X86_PLATFORM_DEVICES if ACPI && X86 select ACPI_WMI if ACPI && X86 select MXM_WMI if ACPI && X86 select POWER_SUPPLY # Similar to i915, we need to select ACPI_VIDEO and it's dependencies - select BACKLIGHT_CLASS_DEVICE if ACPI && X86 select INPUT if ACPI && X86 select THERMAL if ACPI && X86 select ACPI_VIDEO if ACPI && X86 @@ -85,14 +84,6 @@ config NOUVEAU_DEBUG_PUSH Say Y here if you want to enable verbose push buffer debug output and sanity checks. -config DRM_NOUVEAU_BACKLIGHT - bool "Support for backlight control" - depends on DRM_NOUVEAU - default y - help - Say Y here if you want to control the backlight of your display - (e.g. a laptop panel). - config DRM_NOUVEAU_SVM bool "(EXPERIMENTAL) Enable SVM (Shared Virtual Memory) support" depends on DEVICE_PRIVATE diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 093e1f7163b3..b30fd0f4a541 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -1712,9 +1712,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *sta struct drm_device *dev = encoder->dev; struct nouveau_drm *drm = nouveau_drm(dev); struct nouveau_connector *nv_connector; -#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT struct nouveau_backlight *backlight; -#endif struct nvbios *bios = &drm->vbios; bool hda = false; u8 proto = NV507D_SOR_SET_CONTROL_PROTOCOL_CUSTOM; @@ -1790,12 +1788,10 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder, struct drm_atomic_state *sta nv50_audio_enable(encoder, nv_crtc, nv_connector, state, mode); -#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT backlight = nv_connector->backlight; if (backlight && backlight->uses_dpcd) drm_edp_backlight_enable(&nv_connector->aux, &backlight->edp_info, (u16)backlight->dev->props.brightness); -#endif break; default: diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h b/drivers/gpu/drm/nouveau/nouveau_connector.h index 40f90e353540..88ed64efe9e9 100644 --- a/drivers/gpu/drm/nouveau/nouveau_connector.h +++ b/drivers/gpu/drm/nouveau/nouveau_connector.h @@ -45,7 +45,6 @@ struct nvkm_i2c_port; struct dcb_output; -#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT struct nouveau_backlight { struct backlight_device *dev; @@ -54,7 +53,6 @@ struct nouveau_backlight { int id; }; -#endif #define nouveau_conn_atom(p) \ container_of((p), struct nouveau_conn_atom, state) @@ -133,9 +131,7 @@ struct nouveau_connector { struct nouveau_encoder *detected_encoder; struct edid *edid; struct drm_display_mode *native_mode; -#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT struct nouveau_backlight *backlight; -#endif /* * Our connector property code expects a nouveau_conn_atom struct * even on pre-nv50 where we do not support atomic. This embedded @@ -220,29 +216,9 @@ nouveau_conn_mode_clock_valid(const struct drm_display_mode *, const unsigned max_clock, unsigned *clock); -#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT extern int nouveau_backlight_init(struct drm_connector *); extern void nouveau_backlight_fini(struct drm_connector *); extern void nouveau_backlight_ctor(void); extern void nouveau_backlight_dtor(void); -#else -static inline int -nouveau_backlight_init(struct drm_connector *connector) -{ - return 0; -} - -static inline void -nouveau_backlight_fini(struct drm_connector *connector) { -} - -static inline void -nouveau_backlight_ctor(void) { -} - -static inline void -nouveau_backlight_dtor(void) { -} -#endif #endif /* __NOUVEAU_CONNECTOR_H__ */ -- 2.31.1
Arnd Bergmann
2021-Jul-24 06:55 UTC
[Nouveau] [PATCH] nouveau: make backlight support non optional
On Sat, Jul 24, 2021 at 12:47 AM Karol Herbst <kherbst at redhat.com> wrote:> > In the past this only led to compilation issues. Also the small amount of > extra .text shouldn't really matter compared to the entire nouveau driver > anyway. >> select DRM_TTM_HELPER > - select BACKLIGHT_CLASS_DEVICE if DRM_NOUVEAU_BACKLIGHT > - select ACPI_VIDEO if ACPI && X86 && BACKLIGHT_CLASS_DEVICE && INPUT > + select BACKLIGHT_CLASS_DEVICE > + select ACPI_VIDEO if ACPI && X86 && INPUT > select X86_PLATFORM_DEVICES if ACPI && X86 > select ACPI_WMI if ACPI && X86I think the logic needs to be the reverse: instead of 'select BACKLIGHT_CLASS_DEVICE', this should be 'depends on BACKLIGHT_CLASS_DEVICE', and the same for ACPI_VIDEO. We may want to add 'default DRM || FB' to BACKLIGHT_CLASS_DEVICE in the process so we don't lose it for users doing 'make oldconfig' or 'make defconfig' The rest of the patch looks good to me. Arnd
Karol Herbst
2021-Jul-24 09:54 UTC
[Nouveau] [PATCH] nouveau: make backlight support non optional
On Sat, Jul 24, 2021 at 8:55 AM Arnd Bergmann <arnd at kernel.org> wrote:> > On Sat, Jul 24, 2021 at 12:47 AM Karol Herbst <kherbst at redhat.com> wrote: > > > > In the past this only led to compilation issues. Also the small amount of > > extra .text shouldn't really matter compared to the entire nouveau driver > > anyway. > > > > > select DRM_TTM_HELPER > > - select BACKLIGHT_CLASS_DEVICE if DRM_NOUVEAU_BACKLIGHT > > - select ACPI_VIDEO if ACPI && X86 && BACKLIGHT_CLASS_DEVICE && INPUT > > + select BACKLIGHT_CLASS_DEVICE > > + select ACPI_VIDEO if ACPI && X86 && INPUT > > select X86_PLATFORM_DEVICES if ACPI && X86 > > select ACPI_WMI if ACPI && X86 > > I think the logic needs to be the reverse: instead of 'select > BACKLIGHT_CLASS_DEVICE', > this should be 'depends on BACKLIGHT_CLASS_DEVICE', and the same for ACPI_VIDEO. > > We may want to add 'default DRM || FB' to BACKLIGHT_CLASS_DEVICE in the > process so we don't lose it for users doing 'make oldconfig' or 'make defconfig' >yeah.. not sure. I tested it locally (config without backlight enabled) and olddefconfig just worked. I think the problem with "depends" is that the user needs to enable backlight support first before even seeing nouveau and I don't know if that makes sense. But maybe "default" is indeed helping here in this case.> The rest of the patch looks good to me. > > Arnd >