From mboxrd@z Thu Jan 1 00:00:00 1970 From: "S, Deepak" Subject: Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle Date: Thu, 12 Jun 2014 08:14:23 +0530 Message-ID: <53991407.1090402@intel.com> References: <1402001374-2657-1-git-send-email-jbarnes@virtuousgeek.org> <20140606082924.GD27580@intel.com> <20140606080320.20fcd778@jbarnes-desktop> <20140611165611.GM27580@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 5EEE16E32C for ; Wed, 11 Jun 2014 19:44:26 -0700 (PDT) In-Reply-To: <20140611165611.GM27580@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: =?ISO-8859-1?Q?Ville_Syrj=E4l=E4?= , Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 6/11/2014 10:26 PM, Ville Syrj=E4l=E4 wrote: > On Fri, Jun 06, 2014 at 08:03:20AM -0700, Jesse Barnes wrote: >> On Fri, 6 Jun 2014 11:29:24 +0300 >> Ville Syrj=E4l=E4 wrote: >> >>> On Thu, Jun 05, 2014 at 01:49:34PM -0700, Jesse Barnes wrote: >>>> This may take awhile (~10ms), and we don't need to make noise about it. >>>> >>>> References: https://bugs.freedesktop.org/show_bug.cgi?id=3D75244 >>>> Tested-by: huax.lu@intel.com >>>> Signed-off-by: Jesse Barnes >>>> --- >>>> drivers/gpu/drm/i915/intel_pm.c | 4 ---- >>>> 1 file changed, 4 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/in= tel_pm.c >>>> index ee27d74..4eebfd8 100644 >>>> --- a/drivers/gpu/drm/i915/intel_pm.c >>>> +++ b/drivers/gpu/drm/i915/intel_pm.c >>>> @@ -3227,10 +3227,6 @@ static void vlv_set_rps_idle(struct drm_i915_pr= ivate *dev_priv) >>>> vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, >>>> dev_priv->rps.min_freq_softlimit); >>>> >>>> - if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS)) >>>> - & GENFREQSTATUS) =3D=3D 0, 5)) >>>> - DRM_ERROR("timed out waiting for Punit\n"); >>>> - >>> >>> As I recall the entire point of this was to make sure the frequency >>> really drops before we allow turning off the clock. I'm not sure if we >>> can trust that the frequency (and more importantly the voltage) will >>> drop if we allow turning off the clock before the transition is >>> complete. >> >> well, we may need to increase the timeout then... let's see if that >> helps with the bug. > > FYI I played around with my BYT a bit and it doesn't appear to require > this "force gfx clock on" part of the workaround. > > I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to make > sure both render and media wells and the gfx clock remain off, and I > was also monitoring vnn via svid. While that was going on I just rewrote > PUNIT_REG_GPU_FREQ_REQ to make Punit change the frequency, and sure > enough it did, and svid showed me that vnn had also changed. So it > appears there's no need to have the gfx clock on to change its frequency > on this BYT. > > I wonder if this part of the workaround was only needed on older parts. > Deepak, any ideas? Yes ville, this was added initial for older parts and force gfx clock = was part of the workaround. We have not verified this on newer parts. Let me check with hw guys to = see if workaround still exits and when this was fixed. Thanks Deepak