From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: Don't lock panel registers when downclocking Date: Mon, 13 Feb 2012 17:39:49 +0100 Message-ID: <20120213163949.GG5301@phenom.ffwll.local> References: <1328135490-382-1-git-send-email-seanpaul@chromium.org> <20120213100831.GC5301@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f177.google.com (mail-wi0-f177.google.com [209.85.212.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 909179F36D for ; Mon, 13 Feb 2012 08:39:35 -0800 (PST) Received: by wico1 with SMTP id o1so4157940wic.36 for ; Mon, 13 Feb 2012 08:39:34 -0800 (PST) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Sean Paul Cc: intel-gfx@lists.freedesktop.org, jglasgow@chromium.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Feb 13, 2012 at 10:13:02AM -0500, Sean Paul wrote: > On Mon, Feb 13, 2012 at 5:08 AM, Daniel Vetter 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 > > > > 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