Re: [PATCH 3/3] drm/i915: Revert backlight cargo-culting

From: Daniel Vetter <daniel@ffwll.ch>
To: Takashi Iwai <tiwai@suse.de>
Cc: Jani Nikula <jani.nikula@intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 3/3] drm/i915: Revert backlight cargo-culting
Date: Fri, 22 Mar 2013 15:01:29 +0100	[thread overview]
Message-ID: <20130322140129.GU9021@phenom.ffwll.local> (raw)
In-Reply-To: <s5hobebacry.wl%tiwai@suse.de>

On Fri, Mar 22, 2013 at 12:34:41PM +0100, Takashi Iwai wrote:
> At Fri, 22 Mar 2013 12:22:42 +0100,
> Daniel Vetter wrote:
> > 
> > On Fri, Mar 22, 2013 at 11:42 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > > At Fri, 22 Mar 2013 11:22:57 +0100,
> > > Daniel Vetter wrote:
> > >> On Fri, Mar 22, 2013 at 11:13 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >> > On Fri, Mar 22, 2013 at 11:00 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > >> >> At Fri, 22 Mar 2013 10:53:41 +0100,
> > >> >> Daniel Vetter wrote:
> > >> >>>
> > >> >>> This reverts the following commits:
> > >> >>>
> > >> >>> cf0a6584aa6d382f802 drm/i915: write backlight harder
> > >> >>> 770c12312ad617172b1 drm/i915: Fix blank panel at reopening lid
> > >> >>>
> > >> >>> We've come full-circle in this mess and now broke the originally fixed
> > >> >>> machines again with the new trick. So remove it all and start over.
> > >> >>>
> > >> >>> References: http://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg18788.html
> > >> >>> Cc: Jani Nikula <jani.nikula@intel.com>
> > >> >>> Cc: Takashi Iwai <tiwai@suse.de>
> > >> >>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >> >>
> > >> >> Well, although I fully agree that it's better to clear all voodoo once
> > >> >> and fix from scratch, my concern is that it'll cause mess for stable
> > >> >> kernel.  This path won't fix anything by itself, so it's not suitable
> > >> >> for stable alone.  If any, it should be together with the real fix
> > >> >> patch.
> > >> >>
> > >> >> I guess you'll submit the proper fix patch(es) to stable later on?
> > >> >
> > >> > Oops, I've missed that 770c12312ad617172 itself is a regression fix,
> > >> > but lacks the relevant commit citations. I'll resend with more
> > >> > aggressive reverting.
> > >>
> > >> Actually the only other patch I've found is 6db65cbb941f, but no one
> > >> reported a regression against that one yet. So I think we can leave
> > >> that in for now. So I still think that this is the patch I want for
> > >> 3.9&stable, since oldest regression wins.
> > >
> > > Yeah, the situation is messy.  Let me write down as far as I remember:
> > >
> > > - 770c12312ad617172 itself fixed the backlight at resume originally
> > >   with 3.6 kernel.  The backlight worked at boot time, IIRC.  It had
> > >   no mention of regressing commit since it was a series of changes,
> > >   IIRC.  It fixed the same problem for a few Lenovo and HP laptops.
> > 
> > I think the important question is whether those Lenovo/HPs ever
> > worked?
> 
> IIRC, the boot screen worked on 3.6-rc but the resume was broken -- at
> least on HP machines.  On Lenovo, it might be different.  Sorry, I
> don't remember.  Need to dig into LKML archive.

Hm, right. And iirc (my google fu is lacking atm) we never figured out
what's really broken, only noticed that your patch fixes it. So I think I
should only revert cf0a6584aa6d382f802 drm/i915: write backlight harder.

Which leaves us with another regression, but afaics there's no way to win
this game here right now. I really hope that cleaning up the code and
introducing proper locking helps a bit to win this game of whack-a-mole.

I'll send the new revert asap.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch