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

Daniel Vetter
2014-06-16T18:45:41Z

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@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?

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