On Fri, Jul 03, 2020 at 08:45:28PM +0200, Sam Ravnborg wrote:
> The backlight support has two properties that express the state:
> - power
> - state
Tiny nit, but please add "fb_blank" here too, commit message doesn't match
the code anymore. With that:
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
>
> It is un-documented and easy to get wrong.
> Add backlight_is_blank() helper to make it simpler
> for drivers to get the check of the state correct.
>
> A lot of drivers also includes checks for fb_blank.
> This check is redundant when the state is checked
> and thus not needed in this helper function.
> But added anyway to avoid introducing subtle bugs
> due to the creative use of fb_blank in some drivers.
> Introducing this helper will for some drivers results in
> added support for fb_blank. This will be a change in
> functionality, which will improve the backlight driver.
>
> Rolling out this helper to all relevant backlight drivers
> will eliminate almost all accesses to fb_blank.
>
> v4:
> - struct backlight_device * is now const
>
> v3:
> - Clarified that the fb_blank support in
> backlight_is_blank() may result in functionality
> changes for the users (Emil)
>
> v2:
> - Added fb_blank condition (Daniel)
>
> Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Reviewed-by: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
> Reviewed-by: Peter Ujfalusi <peter.ujfalusi@xxxxxx>
> Reviewed-by: Emil Velikov <emil.l.velikov@xxxxxxxxx>
> Cc: Emil Velikov <emil.l.velikov@xxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Lee Jones <lee.jones@xxxxxxxxxx>
> Cc: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
> Cc: Jingoo Han <jingoohan1@xxxxxxxxx>
> ---
> include/linux/backlight.h | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 56e4580d4f55..56e51ebab740 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -175,6 +175,25 @@ static inline void backlight_put(struct backlight_device *bd)
> put_device(&bd->dev);
> }
>
> +/**
> + * backlight_is_blank - Return true if display is expected to be blank
> + * @bd: the backlight device
> + *
> + * Display is expected to be blank if any of these is true::
> + *
> + * 1) if power in not UNBLANK
> + * 2) if fb_blank is not UNBLANK
> + * 3) if state indicate BLANK or SUSPENDED
> + *
> + * Returns true if display is expected to be blank, false otherwise.
> + */
> +static inline bool backlight_is_blank(const struct backlight_device *bd)
> +{
> + return bd->props.power != FB_BLANK_UNBLANK ||
> + bd->props.fb_blank != FB_BLANK_UNBLANK ||
> + bd->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK);
> +}
> +
> extern struct backlight_device *backlight_device_register(const char *name,
> struct device *dev, void *devdata, const struct backlight_ops *ops,
> const struct backlight_properties *props);
> --
> 2.25.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
