Karol Herbst
2021-Jul-23 16:34 UTC
[Nouveau] [PATCH] drm/nouveau/kms/nv50-: fix build failure with CONFIG_BACKLIGHT=n
On Fri, Jul 23, 2021 at 6:31 PM Randy Dunlap <rdunlap at infradead.org> wrote:> > On 7/23/21 8:15 AM, Karol Herbst wrote: > > On Fri, Jul 23, 2021 at 5:10 PM Randy Dunlap <rdunlap at infradead.org> wrote: > >> > >> On 7/23/21 2:15 AM, Arnd Bergmann wrote: > >>> From: Arnd Bergmann <arnd at 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 at arndb.de> > >>> --- > >>> drivers/gpu/drm/nouveau/dispnv50/disp.c | 11 +++++++---- > >>> 1 file changed, 7 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c > >>> index 093e1f7163b3..fcf53e24db21 100644 > >>> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > >>> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > >>> @@ -1659,20 +1659,23 @@ static void > >>> nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *state) > >>> { > >>> struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder); > >>> - struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev); > >>> struct nouveau_crtc *nv_crtc = nouveau_crtc(nv_encoder->crtc); > >>> struct nouveau_connector *nv_connector = nv50_outp_get_old_connector(state, nv_encoder); > >>> - struct nouveau_backlight *backlight = nv_connector->backlight; > >>> struct drm_dp_aux *aux = &nv_connector->aux; > >>> - int ret; > >>> u8 pwr; > >>> > >>> +#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT > >>> + struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev); > >>> + struct nouveau_backlight *backlight = nv_connector->backlight; > >>> + > >>> if (backlight && backlight->uses_dpcd) { > >>> - ret = drm_edp_backlight_disable(aux, &backlight->edp_info); > >>> + int ret = drm_edp_backlight_disable(aux, &backlight->edp_info); > >>> + > >>> if (ret < 0) > >>> NV_ERROR(drm, "Failed to disable backlight on [CONNECTOR:%d:%s]: %d\n", > >>> nv_connector->base.base.id, nv_connector->base.name, ret); > >>> } > >>> +#endif > >>> > >>> if (nv_encoder->dcb->type == DCB_OUTPUT_DP) { > >>> int ret = drm_dp_dpcd_readb(aux, DP_SET_POWER, &pwr); > >>> > >> > >> Hm, only Lyude Paul replied to this patch: > >> > >> https://lore.kernel.org/lkml/20210714171523.413-1-rdunlap at infradead.org/ > >> > > > > what's actually the use case of compiling with > > CONFIG_DRM_NOUVEAU_BACKLIGHT=n anyway? > > Dunno. In this case it was just a randconfig. Still, it needs to be > handled in some way - such as the other suggestion in this thread. >sure, I was just curious if there was a specific use case or just something random as you mentioned.
Arnd Bergmann
2021-Jul-23 18:40 UTC
[Nouveau] [PATCH] drm/nouveau/kms/nv50-: fix build failure with CONFIG_BACKLIGHT=n
On Fri, Jul 23, 2021 at 6:34 PM Karol Herbst <kherbst at redhat.com> wrote:> On Fri, Jul 23, 2021 at 6:31 PM Randy Dunlap <rdunlap at infradead.org> wrote: > > On 7/23/21 8:15 AM, Karol Herbst wrote: > > > On Fri, Jul 23, 2021 at 5:10 PM Randy Dunlap <rdunlap at infradead.org> wrote: > > > > > > what's actually the use case of compiling with > > > CONFIG_DRM_NOUVEAU_BACKLIGHT=n anyway? > > > > Dunno. In this case it was just a randconfig. Still, it needs to be > > handled in some way - such as the other suggestion in this thread. > > > > sure, I was just curious if there was a specific use case or just > something random as you mentioned.I think this is purely done because of tradition. A long time ago, we had tiny framebuffer drivers and most PCs did not have backlights, so it made sense to leave this optional. This was probably just always carried over. Arnd
Karol Herbst
2021-Jul-23 18:48 UTC
[Nouveau] [PATCH] drm/nouveau/kms/nv50-: fix build failure with CONFIG_BACKLIGHT=n
On Fri, Jul 23, 2021 at 8:40 PM Arnd Bergmann <arnd at kernel.org> wrote:> > On Fri, Jul 23, 2021 at 6:34 PM Karol Herbst <kherbst at redhat.com> wrote: > > On Fri, Jul 23, 2021 at 6:31 PM Randy Dunlap <rdunlap at infradead.org> wrote: > > > On 7/23/21 8:15 AM, Karol Herbst wrote: > > > > On Fri, Jul 23, 2021 at 5:10 PM Randy Dunlap <rdunlap at infradead.org> wrote: > > > > > > > > what's actually the use case of compiling with > > > > CONFIG_DRM_NOUVEAU_BACKLIGHT=n anyway? > > > > > > Dunno. In this case it was just a randconfig. Still, it needs to be > > > handled in some way - such as the other suggestion in this thread. > > > > > > > sure, I was just curious if there was a specific use case or just > > something random as you mentioned. > > I think this is purely done because of tradition. A long time ago, we had > tiny framebuffer drivers and most PCs did not have backlights, so it > made sense to leave this optional. > > This was probably just always carried over. > > Arnd >okay. I think I will write a patch for nouveau then and send it out soonish