From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 07/11] drm/i915: Warn if there's a cdclk change in progess Date: Mon, 7 Jul 2014 11:26:37 +0200 Message-ID: <20140707092637.GN5821@phenom.ffwll.local> References: <1402655877-6460-1-git-send-email-ville.syrjala@linux.intel.com> <1402655877-6460-8-git-send-email-ville.syrjala@linux.intel.com> <20140625115558.645bb288@jbarnes-desktop> <20140625193448.GB27580@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wg0-f42.google.com (mail-wg0-f42.google.com [74.125.82.42]) by gabe.freedesktop.org (Postfix) with ESMTP id 3F5396E2A4 for ; Mon, 7 Jul 2014 02:26:28 -0700 (PDT) Received: by mail-wg0-f42.google.com with SMTP id n12so2294192wgh.25 for ; Mon, 07 Jul 2014 02:26:27 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140625193448.GB27580@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Jun 25, 2014 at 10:34:48PM +0300, Ville Syrj=E4l=E4 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=E4l=E4 > > > = > > > 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=E4l=E4 > > > --- > > > 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/i= 915/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 =3D val & DISPLAY_FREQUENCY_VALUES; > > > = > > > + WARN((val & DISPLAY_FREQUENCY_STATUS) !=3D > > > + (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