Re: [PATCH 07/11] drm/i915: Warn if there's a cdclk change in progess

Daniel Vetter
2014-07-07T09:26:28Z

On Wed, Jun 25, 2014 at 10:34:48PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 25, 2014 at 11:55:58AM -0700, Jesse Barnes wrote:
> > On Fri, 13 Jun 2014 13:37:53 +0300
> > ville.syrjala@linux.intel.com wrote:
> >
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > If someone is interested in the current cdclk frquency it should
> > > be stable and not in process of changing frquency. Warn if the current
> > > and requested cdclk don't match in .get_display_clock_spee() on vlv.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 29dddec..601e97e 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5234,6 +5234,10 @@ static int valleyview_get_display_clock_speed(struct drm_device *dev)
> > >
> > > divider = val & DISPLAY_FREQUENCY_VALUES;
> > >
> > > + WARN((val & DISPLAY_FREQUENCY_STATUS) !=
> > > + (divider << DISPLAY_FREQUENCY_STATUS_SHIFT),
> > > + "cdclk change in progress\n");
> > > +
> > > return DIV_ROUND_CLOSEST(vco << 1, divider + 1);
> > > }
> > >
> >
> > Hm, there's not much we can do in this case, so rather than warn maybe
> > we should try a wait instead, and only warn if it times out? Even then
> > there's not much we can do aside from poking the PUnit folks.
>
> This shouldn't happen unless we somehow messed up and triggered a cdclk
> change and didn't wait for it to complete, which would be a driver bug.
> So I think a simple WARN seems sufficient.

I concur with Ville here so merged the patch. If we hit this we can figure
out where exactly we've been wrong and whether there's a legitimate case
where we need to wait for cdclk to settle.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch