Re: [PATCH 01/10] backlight: Match backlight device against struct fb_info.bl_dev

Thomas Zimmermann
2024-02-20T09:37:57Z

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
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)