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

Ville Syrjälä
2014-06-25T19:35:18Z

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.

--
Ville Syrjälä
Intel OTC