linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Mans Rullgard <mans@mansr.com>,
	linux-fbdev@vger.kernel.org, Jingoo Han <jingoohan1@gmail.com>,
	Helge Deller <deller@gmx.de>, Lee Jones <lee@kernel.org>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Tomi Valkeinen <tomi.valkeinen@ti.com>
Subject: Re: [PATCH] backlight: led_bl: fix initial power state
Date: Tue, 4 Jul 2023 19:07:31 +0200	[thread overview]
Message-ID: <20230704170731.GB940443@ravnborg.org> (raw)
In-Reply-To: <20230704150310.GA385243@aspen.lan>

Hi Daniel,

> > @@ -200,8 +200,8 @@ static int led_bl_probe(struct platform_device *pdev)
> >  	props.type = BACKLIGHT_RAW;
> >  	props.max_brightness = priv->max_brightness;
> >  	props.brightness = priv->default_brightness;
> > -	props.power = (priv->default_brightness > 0) ? FB_BLANK_POWERDOWN :
> > -		      FB_BLANK_UNBLANK;
> > +	props.power = (priv->default_brightness > 0) ? FB_BLANK_UNBLANK :
> > +		      FB_BLANK_POWERDOWN;
> 
> The logic was wrong before but I think will still be wrong after the
> change too (e.g. the bogus logic is probably avoiding backlight flicker
> in some use cases).
> 
> The logic here needs to be similar to what pwm_bl.c implements in
> pwm_backlight_initial_power_state(). Whilst it might be better
> to implement this in led_bl_get_leds() let me show what I mean
> in code that fits in the current line:
> 
> 	/*
> 	 * Activate the backlight if the LEDs are already lit *or*
> 	 * there is no phandle link (meaning the backlight power
> 	 * state cannot be synced with the display state).
> 	 */
> 	props.power = (active_at_boot || !dev->node->phandle) ?
> 			FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
> 
The following code does the same using helpers:

	if (active_at_boot || !dev->node->phandle))
		backlight_enable(bd);
	else
		backlight_disable(bd);

The code needs to execute after backlight_device_register() so maybe not
so great an idea?!?

	Sam

  parent reply	other threads:[~2023-07-04 17:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-04 14:07 [PATCH] backlight: led_bl: fix initial power state Mans Rullgard
2023-07-04 15:03 ` Daniel Thompson
2023-07-04 15:31   ` Måns Rullgård
2023-07-04 15:40     ` Daniel Thompson
2023-07-04 17:07   ` Sam Ravnborg [this message]
2023-07-05 14:07     ` Daniel Thompson
2023-07-05 17:56       ` Sam Ravnborg
  -- strict thread matches above, loose matches on Subject: below --
2023-07-05 14:24 Mans Rullgard
2023-07-05 14:33 ` Daniel Thompson
2023-07-05 14:36   ` Måns Rullgård
2023-07-05 14:44     ` Daniel Thompson
2023-07-05 14:51       ` Måns Rullgård

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230704170731.GB940443@ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=daniel.thompson@linaro.org \
    --cc=deller@gmx.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jingoohan1@gmail.com \
    --cc=lee@kernel.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mans@mansr.com \
    --cc=tomi.valkeinen@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).