Re: [PATCH] drm/i915: Don't take fp 0/1 selector into account for pll state

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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älä 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 <aleks@slobodensoftver.org.mk>
> > > Reported-by: Aleks <aleks@slobodensoftver.org.mk>
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > ---
> > >  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/i915/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_crtc *crtc,
> > >  	else
> > >  		fp = pipe_config->dpll_hw_state.fp1;
> > >  
> > > +	/* We don't compute the FPA 0/1 selector. */
> > > +	dpll &= ~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älä
Intel OTC