From: Jani Nikula <jani.nikula@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Allow minimum brightness upon backlight enable
Date: Tue, 20 Oct 2015 13:32:52 +0300 [thread overview]
Message-ID: <871tcpzv6z.fsf@intel.com> (raw)
In-Reply-To: <20151002090057.GQ9929@nuc-i3427.alporthouse.com>
On Fri, 02 Oct 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Fri, Oct 02, 2015 at 10:50:50AM +0300, Jani Nikula wrote:
>> On Fri, 02 Oct 2015, clinton.a.taylor@intel.com wrote:
>> > From: Clint Taylor <clinton.a.taylor@intel.com>
>> >
>> > backlight minimum is a valid value so you don't need to set maximum.
>> >
>> > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/intel_panel.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
>> > index 4d28c7b..d509904 100644
>> > --- a/drivers/gpu/drm/i915/intel_panel.c
>> > +++ b/drivers/gpu/drm/i915/intel_panel.c
>> > @@ -1069,7 +1069,7 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
>> >
>> > WARN_ON(panel->backlight.max == 0);
>> >
>> > - if (panel->backlight.level <= panel->backlight.min) {
>> > + if (panel->backlight.level < panel->backlight.min) {
>> > panel->backlight.level = panel->backlight.max;
>> > if (panel->backlight.device)
>> > panel->backlight.device->props.brightness =
>>
>> This is policy in action in the kernel. We have this whole if block here
>> because some, uh, misguided userspace sets brightness to minimum before
>> switching the backlight off, and assumes enabling backlight cranks up
>> the brightness as well. (*)
>
> If you mean X, no. It has to set the backlight because on some intel
> models the backlight is not routed through i915 and so it needs to
> explicitly control it. On resume, it sets the previous value. This is
> just a broken kernel.
Okay, I guess I'm willing to give this a shot. This may result in
regression reports, but let's see.
However, the meaning of the whole check changes. What Clint now checks
(level below minimum) shouldn't happen, something failed already. Maybe
we should set the level to minimum instead, with debug logging?
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
prev parent reply other threads:[~2015-10-20 10:29 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-01 23:36 [PATCH] drm/i915: Allow minimum brightness upon backlight enable clinton.a.taylor
2015-10-02 7:50 ` Jani Nikula
2015-10-02 9:00 ` Chris Wilson
2015-10-20 10:32 ` Jani Nikula [this message]
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=871tcpzv6z.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=Intel-gfx@lists.freedesktop.org \
--cc=chris@chris-wilson.co.uk \
/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