| Message ID | 20240212162645.5661-2-tzimmermann@suse.de |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | backlight: Replace struct fb_info in interfaces | expand |
On Mon, Feb 12, 2024 at 05:16:34PM +0100, Thomas Zimmermann wrote: > Framebuffer drivers for devices with dedicated backlight are supposed > to set struct fb_info.bl_dev to the backlight's respective device. Use > the value to match backlight and framebuffer in the backlight core code. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> Daniel.
Thomas Zimmermann <tzimmermann@suse.de> writes: Hello Thomas, > Framebuffer drivers for devices with dedicated backlight are supposed > to set struct fb_info.bl_dev to the backlight's respective device. Use > the value to match backlight and framebuffer in the backlight core code. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/video/backlight/backlight.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c > index 86e1cdc8e3697..48844a4f28ad3 100644 > --- a/drivers/video/backlight/backlight.c > +++ b/drivers/video/backlight/backlight.c > @@ -98,7 +98,8 @@ static int fb_notifier_callback(struct notifier_block *self, > { > struct backlight_device *bd; > struct fb_event *evdata = data; > - int node = evdata->info->node; > + struct fb_info *info = evdata->info; > + int node = info->node; > int fb_blank = 0; > > /* If we aren't interested in this event, skip it immediately ... */ > @@ -110,8 +111,12 @@ static int fb_notifier_callback(struct notifier_block *self, > > if (!bd->ops) > goto out; > - if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info)) > + else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info)) > goto out; > +#if IS_ENABLED(CONFIG_FB_BACKLIGHT) > + else if (info->bl_dev && info->bl_dev != bd) If the driver doesn't provide a struct backlight_ops .check_fb callback, I think that having an info->bl_dev should be mandatory ? Or at least maybe there should be a warning if info->bl_dev isn't set ? The would be a driver bug, right ? Regardless, the change makes sense to me. Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Hi Am 20.02.24 um 10:17 schrieb Javier Martinez Canillas: > Thomas Zimmermann <tzimmermann@suse.de> writes: > > Hello Thomas, > >> Framebuffer drivers for devices with dedicated backlight are supposed >> to set struct fb_info.bl_dev to the backlight's respective device. Use >> the value to match backlight and framebuffer in the backlight core code. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/video/backlight/backlight.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c >> index 86e1cdc8e3697..48844a4f28ad3 100644 >> --- a/drivers/video/backlight/backlight.c >> +++ b/drivers/video/backlight/backlight.c >> @@ -98,7 +98,8 @@ static int fb_notifier_callback(struct notifier_block *self, >> { >> struct backlight_device *bd; >> struct fb_event *evdata = data; >> - int node = evdata->info->node; >> + struct fb_info *info = evdata->info; >> + int node = info->node; >> int fb_blank = 0; >> >> /* If we aren't interested in this event, skip it immediately ... */ >> @@ -110,8 +111,12 @@ static int fb_notifier_callback(struct notifier_block *self, >> >> if (!bd->ops) >> goto out; >> - if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info)) >> + else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info)) >> goto out; >> +#if IS_ENABLED(CONFIG_FB_BACKLIGHT) >> + else if (info->bl_dev && info->bl_dev != bd) > If the driver doesn't provide a struct backlight_ops .check_fb callback, I > think that having an info->bl_dev should be mandatory ? Or at least maybe > there should be a warning if info->bl_dev isn't set ? bl_dev can only be used for display drivers that set an explicit backlight device; otherwise it's NULL. There seem to be systems where backlight and display are distinct. And the docs for check_fb say that by default the backlight matches against any display. I tried to keep this semantics by silently succeeding if neither check_fb nor bl_dev have bene set. > > The would be a driver bug, right ? I assume that some systems create the backlight instance from platform data or DT and the display driver has no means of knowing about it. Best regards Thomas > > Regardless, the change makes sense to me. > > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> >
Thomas Zimmermann <tzimmermann@suse.de> writes: > Hi > > Am 20.02.24 um 10:17 schrieb Javier Martinez Canillas: >> Thomas Zimmermann <tzimmermann@suse.de> writes: >> >> Hello Thomas, >> >>> Framebuffer drivers for devices with dedicated backlight are supposed >>> to set struct fb_info.bl_dev to the backlight's respective device. Use >>> the value to match backlight and framebuffer in the backlight core code. >>> >>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>> --- >>> drivers/video/backlight/backlight.c | 9 +++++++-- >>> 1 file changed, 7 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c >>> index 86e1cdc8e3697..48844a4f28ad3 100644 >>> --- a/drivers/video/backlight/backlight.c >>> +++ b/drivers/video/backlight/backlight.c >>> @@ -98,7 +98,8 @@ static int fb_notifier_callback(struct notifier_block *self, >>> { >>> struct backlight_device *bd; >>> struct fb_event *evdata = data; >>> - int node = evdata->info->node; >>> + struct fb_info *info = evdata->info; >>> + int node = info->node; >>> int fb_blank = 0; >>> >>> /* If we aren't interested in this event, skip it immediately ... */ >>> @@ -110,8 +111,12 @@ static int fb_notifier_callback(struct notifier_block *self, >>> >>> if (!bd->ops) >>> goto out; >>> - if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info)) >>> + else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info)) >>> goto out; >>> +#if IS_ENABLED(CONFIG_FB_BACKLIGHT) >>> + else if (info->bl_dev && info->bl_dev != bd) >> If the driver doesn't provide a struct backlight_ops .check_fb callback, I >> think that having an info->bl_dev should be mandatory ? Or at least maybe >> there should be a warning if info->bl_dev isn't set ? > > bl_dev can only be used for display drivers that set an explicit > backlight device; otherwise it's NULL. There seem to be systems where > backlight and display are distinct. And the docs for check_fb say that > by default the backlight matches against any display. I tried to keep > this semantics by silently succeeding if neither check_fb nor bl_dev > have bene set. > >> >> The would be a driver bug, right ? > > I assume that some systems create the backlight instance from platform > data or DT and the display driver has no means of knowing about it. > Ok. I thought that in that case a (platform specific) .check_fb callback would have to be provided then. But if the semantic is that none could be missing, then I guess is OK to silently succeeding. I wonder if at least a debug printout is worth it. But maybe a follow-up. > Best regards > Thomas >
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c index 86e1cdc8e3697..48844a4f28ad3 100644 --- a/drivers/video/backlight/backlight.c +++ b/drivers/video/backlight/backlight.c @@ -98,7 +98,8 @@ static int fb_notifier_callback(struct notifier_block *self, { struct backlight_device *bd; struct fb_event *evdata = data; - int node = evdata->info->node; + struct fb_info *info = evdata->info; + int node = info->node; int fb_blank = 0; /* If we aren't interested in this event, skip it immediately ... */ @@ -110,8 +111,12 @@ static int fb_notifier_callback(struct notifier_block *self, if (!bd->ops) goto out; - if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info)) + else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info)) goto out; +#if IS_ENABLED(CONFIG_FB_BACKLIGHT) + else if (info->bl_dev && info->bl_dev != bd) + goto out; +#endif fb_blank = *(int *)evdata->data; if (fb_blank == FB_BLANK_UNBLANK && !bd->fb_bl_on[node]) {
Framebuffer drivers for devices with dedicated backlight are supposed to set struct fb_info.bl_dev to the backlight's respective device. Use the value to match backlight and framebuffer in the backlight core code. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/video/backlight/backlight.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)