Hi Sam, On Sun, 17 May 2020 at 20:02, Sam Ravnborg <sam@xxxxxxxxxxxx> wrote: > > Increase readability of fb_notifier_callback() by removing > a few indent levels. > No functional change. > > Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx> > Cc: Lee Jones <lee.jones@xxxxxxxxxx> > Cc: Daniel Thompson <daniel.thompson@xxxxxxxxxx> > Cc: Jingoo Han <jingoohan1@xxxxxxxxx> > --- > drivers/video/backlight/backlight.c | 43 +++++++++++++++-------------- > 1 file changed, 22 insertions(+), 21 deletions(-) > > diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c > index cac3e35d7630..17f04cff50ab 100644 > --- a/drivers/video/backlight/backlight.c > +++ b/drivers/video/backlight/backlight.c > @@ -58,28 +58,29 @@ static int fb_notifier_callback(struct notifier_block *self, > > bd = container_of(self, struct backlight_device, fb_notif); > mutex_lock(&bd->ops_lock); > - if (bd->ops) > - if (!bd->ops->check_fb || > - bd->ops->check_fb(bd, evdata->info)) { > - fb_blank = *(int *)evdata->data; > - if (fb_blank == FB_BLANK_UNBLANK && > - !bd->fb_bl_on[node]) { > - bd->fb_bl_on[node] = true; > - if (!bd->use_count++) { > - bd->props.state &= ~BL_CORE_FBBLANK; > - bd->props.fb_blank = FB_BLANK_UNBLANK; > - backlight_update_status(bd); > - } > - } else if (fb_blank != FB_BLANK_UNBLANK && > - bd->fb_bl_on[node]) { > - bd->fb_bl_on[node] = false; > - if (!(--bd->use_count)) { > - bd->props.state |= BL_CORE_FBBLANK; > - bd->props.fb_blank = fb_blank; > - backlight_update_status(bd); > - } > - } > + > + if (!bd->ops) > + goto out; > + if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info)) Mildly related: Would be a nice to define which ops are mandatory and which aren't. That plus enforcement in backlight_device_register. But that's for another patchset. > + goto out; > + > + fb_blank = *(int *)evdata->data; > + if (fb_blank == FB_BLANK_UNBLANK && !bd->fb_bl_on[node]) { > + bd->fb_bl_on[node] = true; > + if (!bd->use_count++) { > + bd->props.state &= ~BL_CORE_FBBLANK; > + bd->props.fb_blank = FB_BLANK_UNBLANK; > + backlight_update_status(bd); > + } > + } else if (fb_blank != FB_BLANK_UNBLANK && bd->fb_bl_on[node]) { > + bd->fb_bl_on[node] = false; > + if (!(--bd->use_count)) { > + bd->props.state |= BL_CORE_FBBLANK; > + bd->props.fb_blank = fb_blank; > + backlight_update_status(bd); > } Something like the following reads better, plus one could simplify it with follow-on patch. if (fb_blank == FB_BLANK_UNBLANK) if (!bd->fb_bl_on[node] && !bd->use_count++) { bd->props.state &= ~BL_CORE_FBBLANK; bd->props.fb_blank = FB_BLANK_UNBLANK; backlight_update_status(bd); // above is backlight_enable() } bd->fb_bl_on[node] = true; } else { if (bd->fb_bl_on[node] && !(--bd->use_count)) { bd->props.state |= BL_CORE_FBBLANK; bd->props.fb_blank = fb_blank; backlight_update_status(bd); // above is backlight_disable() } bd->fb_bl_on[node] = false; } As-is, one cannot use the backlight helpers indicated, since it touches .power. First one should ensure the drivers honour .power - by using the helper introduced later. -Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel