From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: Don't take fp 0/1 selector into account for pll state Date: Mon, 16 Jun 2014 21:20:35 +0300 Message-ID: <20140616182035.GJ27580@intel.com> References: <1402836942-31319-1-git-send-email-daniel.vetter@ffwll.ch> <20140616083239.GG27580@intel.com> <20140616180125.GD5821@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 897E66E5BE for ; Mon, 16 Jun 2014 11:20:39 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140616180125.GD5821@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: Daniel Vetter , Intel Graphics Development , Aleks , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Jun 16, 2014 at 08:01:26PM +0200, Daniel Vetter wrote: > On Mon, Jun 16, 2014 at 11:32:39AM +0300, Ville Syrj=E4l=E4 wrote: > > On Sun, Jun 15, 2014 at 02:55:42PM +0200, Daniel Vetter wrote: > > > It changes at runtime and so should be ignored for pipe state checks. > > > Note that we don't yet read out the full DRRS state, so there's some > > > gaps. Bu DRRS is also not yet enabled for LVDS by default. > > > = > > > Cc: Aleks > > > Reported-by: Aleks > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Daniel Vetter > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > = > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i= 915/intel_display.c > > > index ba1d9aac3958..1ccf660e67d9 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -8549,6 +8549,9 @@ static void i9xx_crtc_clock_get(struct intel_cr= tc *crtc, > > > else > > > fp =3D pipe_config->dpll_hw_state.fp1; > > > = > > > + /* We don't compute the FPA 0/1 selector. */ > > > + dpll &=3D ~DISPLAY_RATE_SELECT_FPA1; > > > + > > = > > But we still compute port_clock based on the currently active FPA > > register. Won't that make the clock checks fail as well? > = > Well yeah, but that should get solved as part of the DRRS stuff I think. > Imo ignore the frequency selector for the dpll state is the right thing. > = > DRRS with state readout is still in-flux and unsolved wrt fastbooting. > = > Should I add a caveat to the commit message that this isn't everything or > not worth it as-is? Well, after a better look I see that this patch does absolutely nothing. You already picked the FPA register before you cleared the select bit. So you need to clear it a bit earlier in this function, or even go as far as clearing it when we read out the dpll state. Maybe the latter is better in case we want to start checking the entire dpll state? -- = Ville Syrj=E4l=E4 Intel OTC