On 2021-07-23 3:14 p.m., Cornij, Nikola wrote:
[AMD Official Use Only]
+Harry
-----Original Message----- From: Daniel Vetter daniel@ffwll.ch Sent: Friday, July 23, 2021 11:23 AM To: Arnd Bergmann arnd@kernel.org Cc: Daniel Vetter daniel@ffwll.ch; Ben Skeggs bskeggs@redhat.com; David Airlie airlied@linux.ie; Lyude Paul lyude@redhat.com; Arnd Bergmann arnd@arndb.de; Ville Syrjälä ville.syrjala@linux.intel.com; Cornij, Nikola Nikola.Cornij@amd.com; dri-devel dri-devel@lists.freedesktop.org; Nouveau Dev nouveau@lists.freedesktop.org; Linux Kernel Mailing List linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/nouveau/kms/nv50-: fix build failure with CONFIG_BACKLIGHT=n
On Fri, Jul 23, 2021 at 12:16:31PM +0200, Arnd Bergmann wrote:
On Fri, Jul 23, 2021 at 11:25 AM Daniel Vetter daniel@ffwll.ch wrote:
On Fri, Jul 23, 2021 at 11:15 AM Arnd Bergmann arnd@kernel.org wrote:
From: Arnd Bergmann arnd@arndb.de
When the backlight support is disabled, the driver fails to build:
drivers/gpu/drm/nouveau/dispnv50/disp.c: In function 'nv50_sor_atomic_disable': drivers/gpu/drm/nouveau/dispnv50/disp.c:1665:59: error: 'struct nouveau_connector' has no member named 'backlight' 1665 | struct nouveau_backlight *backlight = nv_connector->backlight; | ^~ drivers/gpu/drm/nouveau/dispnv50/disp.c:1670:35: error: invalid use of undefined type 'struct nouveau_backlight' 1670 | if (backlight && backlight->uses_dpcd) { | ^~ drivers/gpu/drm/nouveau/dispnv50/disp.c:1671:64: error: invalid use of undefined type 'struct nouveau_backlight' 1671 | ret = drm_edp_backlight_disable(aux, &backlight->edp_info); | ^~
The patch that introduced the problem already contains some #ifdef checks, so just add another one that makes it build again.
Fixes: 6eca310e8924 ("drm/nouveau/kms/nv50-: Add basic DPCD backlight support for nouveau") Signed-off-by: Arnd Bergmann arnd@arndb.de
Can we just toss the idea that BACKTLIGHT=n is a reasonable config for drm drivers using backlights, and add depends BACKLIGHT to all of them?
I mean this is a perfect source of continued patch streams to keep us all busy, but beyond that I really don't see the point ... I frankly have better things to do, and especially with the big drivers we have making backlight optional saves comparitively nothing. -Daniel
Yes! I'd definitely be in favor of that, I've wasted way too much time trying to sort through dependency loops and other problems with backlight support.
Maybe we should leave the drivers/video/fbdev/ drivers untouched in this regard, at least for the moment, but for the drivers/gpu/drm users of backlight that would be a nice simplification, and even the smallest ones are unlikely to be used on systems that are too memory constrained to deal with 4KB extra .text.
Yeah I think we can do this entirely ad-hoc, i.e. any time the backlight wheel wobbles off again we nail it down for good for that driver with a depends on BACKGLIGHT and remove any lingering #ifdef all over.
If you want maybe start out with the biggest drm drivers in a series, I think if nouveau/amdgpu/i915 folks ack this you're good to go to just convert as things get in the way.
Sounds like a good idea to me.
Harry
-Daniel
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch/%3E%3E