From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: Update PSR on resume. Date: Mon, 11 Aug 2014 19:31:45 +0200 Message-ID: <20140811173145.GU10500@phenom.ffwll.local> References: <20140610152457.GY5821@phenom.ffwll.local> <1407518374-7886-1-git-send-email-rodrigo.vivi@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f41.google.com (mail-wg0-f41.google.com [74.125.82.41]) by gabe.freedesktop.org (Postfix) with ESMTP id EEDE66E42F for ; Mon, 11 Aug 2014 10:31:33 -0700 (PDT) Received: by mail-wg0-f41.google.com with SMTP id z12so8864838wgg.12 for ; Mon, 11 Aug 2014 10:31:33 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Rodrigo Vivi Cc: Daniel Vetter , intel-gfx , Rodrigo Vivi List-Id: intel-gfx@lists.freedesktop.org On Mon, Aug 11, 2014 at 09:57:12AM -0700, Rodrigo Vivi wrote: > Well, this fix the issue Linus faced. > > Actually the issue I was aware of and trying to fix with this patch for a > long time was reported by chromeos guys saying the psr wasn't propperly > working after suspend/resume. They got the screen back but never got psr > back again. > > The original patch fix suspend/resume issues with psr and I decided to keep > the same and subject for reference of what the problem this fixes and what > was the patch history, but changed the place for the full setup to > crtc_enable per your recommendation. So if I read your patch correctly then we no do the initial psr setup in crtc_enable, but only after driver load/resume. Before we do it always when crtc_enable is called. That doesn't make sense. Also note that Linus said it's also sometimes broken when using X normally. Otoh if there's really something we need to in crtc_enable in addition to what we do already, then we _must_ do it always. Otherwise runtime pm (which is pretty much the same as system resume) will not work. -Daniel > > Thanks, > Rodrigo. > > > > On Sat, Aug 9, 2014 at 12:40 AM, Daniel Vetter > wrote: > > > On Fri, Aug 8, 2014 at 7:19 PM, Rodrigo Vivi > > wrote: > > > From: Rodrigo Vivi > > > > > > Some registers set during setup might not be persistent after > > suspend/resume. > > > This was causing bugs for some people that was unable to get PSR entry > > state > > > after resume cycle. > > > > > > v2: Adding some comments and better commit message explaining why this > > is needed. > > > v3: Getting back old setup_done variable and move from resume to > > crtc_enable > > > as Daniel requested. > > > > > > Cc: Daniel Vetter > > > Signed-off-by: Rodrigo Vivi > > > > I'm confused ... whats the use of this? Afaict that's exactly what the > > code currently does. > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > > > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch