From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Lee, Chon Ming" Subject: Re: [PATCH] drm/i915/vlv: drop punit freq staus read after setting idle Date: Thu, 12 Jun 2014 16:19:42 +0800 Message-ID: <20140612081942.GH1002@clee30-mobl2.gar.corp.intel.com> References: <1402001374-2657-1-git-send-email-jbarnes@virtuousgeek.org> <20140606082924.GD27580@intel.com> <20140606080320.20fcd778@jbarnes-desktop> <20140611165611.GM27580@intel.com> <53991407.1090402@intel.com> <20140612073718.GA1046@clee30-mobl2.gar.corp.intel.com> <20140612081108.GO27580@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 49ED16E4BF for ; Thu, 12 Jun 2014 01:21:36 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140612081108.GO27580@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: "S, Deepak" , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 06/12 11:11, Ville Syrj=E4l=E4 wrote: > On Thu, Jun 12, 2014 at 03:37:18PM +0800, Lee, Chon Ming wrote: > > On 06/12 08:14, S, Deepak wrote: > > > = > > > = > > > 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 abo= ut 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/i9= 15/intel_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_i9= 15_private *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 i= f we > > > >>>can trust that the frequency (and more importantly the voltage) wi= ll > > > >>>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 requi= re > > > >this "force gfx clock on" part of the workaround. > > > > > > > >I was polling VLV_GTLC_SURVIVABILITY_REG and VLV_GTLC_PW_STATUS to m= ake > > > >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 rew= rote > > > >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 frequ= ency > > > >on this BYT. > > > > > > > >I wonder if this part of the workaround was only needed on older par= ts. > > > >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 > > = > > This is a bit side topic. In the vlv_set_rps_idle, it try to mask the = turbo > > interrupt by writing to GEN6_PMINTRMSK (offset 0xA168). But based on t= he bspec, > > the PMIMR for BYT/GEN6 should be 0x44024 (GEN6_PMIMR). = > > = > > Not sure I miss anything. If there offset is wrong, mean the turbo has= not mask at > > all. > = > There are two mask registers for this stuff. PMINTRMSK is some kind > of lower level mask, and GEN6_PMIMR is the regular interrupt mask. > I believe masking the interrupt in PMINTRMSK prevents it from being > visible in GEN6_PMISR even. > = Thanks for the info. Got that, we can't use GEN6PMIMR to mask interrupt, n= eed to use PMINTRMSK. Regards, Chon Ming > -- = > Ville Syrj=E4l=E4 > Intel OTC