On Mon, Jun 16, 2014 at 09:20:35PM +0300, Ville Syrjälä wrote: > 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@xxxxxxxxxxxxxxxxxxxxxx> > > > > Reported-by: Aleks <aleks@xxxxxxxxxxxxxxxxxxxxxx> > > > > Cc: stable@xxxxxxxxxxxxxxx > > > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > > > --- > > > > 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? Hm, I guess I hide in shame and will leave this topic to people with clue ;-) Thanks for looking at this anyway. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html