Re: [PATCH] drm/i915: Don't lock panel registers when downclocking

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Feb 13, 2012 at 10:13:02AM -0500, Sean Paul wrote:
> On Mon, Feb 13, 2012 at 5:08 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Wed, Feb 01, 2012 at 05:31:30PM -0500, Sean Paul wrote:
> >> This patch removes the locking from the downclock routines since we are no
> >> longer locking the registers at all. See ed10fca9 for the original commit
> >> changing this philosophy.
> >>
> >> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> >
> > I've thought this was due to paranoia because we don't trust our own code
> > and because we don't trust the bios to randomly lock this again. Without
> > any reasons to the contrary, I'll prefer to keep this.
> 
> Thanks for the explanation, Daniel, however I'd ask that you
> reconsider this patch.
> 
> The state coming into the downclock functions is unlocked and without
> this patch, the state coming out is locked. This causes at least one
> warning in the code from assert_panel_unlocked.

Ah, that's a pretty important thing missing from the commit message. I've
checked the code and we have indeed an issue there. Can you please:
- Extend your commit message to mention that you're actually hitting the
  assert_panel_unlocked assert (and how this happens). Maybe explain that
  you need lvds downclocking, which is disabled by default.
- Add an assert_panel_unlocked call instead of unlocking the panel in your
  patch (we do need to be paranoid about these things).

Then I think your patch is good to go into -next.

Yours, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48