From mboxrd@z Thu Jan 1 00:00:00 1970 From: Deepak S Subject: Re: [PATCH 27/71] drm/i915/chv: Enable Render Standby (RC6) for Cheeryview Date: Sun, 13 Apr 2014 21:01:00 +0530 Message-ID: <534AADB4.7090409@linux.intel.com> References: <1397039349-10639-1-git-send-email-ville.syrjala@linux.intel.com> <1397039349-10639-28-git-send-email-ville.syrjala@linux.intel.com> <87a9bt162w.fsf@intel.com> <20140410170653.GA18465@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 B81BF6E0EE for ; Sun, 13 Apr 2014 08:31:05 -0700 (PDT) In-Reply-To: <20140410170653.GA18465@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?= , Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org Thanks for the feedback. I will address all the comments and I will post cherryview rc6/turbo updated patches within couple of days. I think the patches need little bit of cleanup. Thanks Deepak On Thursday 10 April 2014 10:36 PM, Ville Syrj=E4l=E4 wrote: > On Thu, Apr 10, 2014 at 07:51:03PM +0300, Jani Nikula wrote: >> On Wed, 09 Apr 2014, ville.syrjala@linux.intel.com wrote: >>> From: Deepak S >>> >>> v2: Configure PCBR if BIOS fails allocate pcbr (deepak) >>> >>> Signed-off-by: Deepak S >>> --- >>> drivers/gpu/drm/i915/intel_pm.c | 101 +++++++++++++++++++++++++++++++= +++++++-- >>> 1 file changed, 98 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/int= el_pm.c >>> index 0889af7..909cc0a 100644 >>> --- a/drivers/gpu/drm/i915/intel_pm.c >>> +++ b/drivers/gpu/drm/i915/intel_pm.c >>> @@ -3184,6 +3184,18 @@ static void gen6_disable_rps(struct drm_device *= dev) >>> gen6_disable_rps_interrupts(dev); >>> } >>> = >>> +static void cherryview_disable_rps(struct drm_device *dev) >>> +{ >>> + struct drm_i915_private *dev_priv =3D dev->dev_private; >>> + >>> + I915_WRITE(GEN6_RC_CONTROL, 0); >>> + >>> + if (dev_priv->vlv_pctx) { >>> + drm_gem_object_unreference(&dev_priv->vlv_pctx->base); >>> + dev_priv->vlv_pctx =3D NULL; >>> + } >>> +} >>> + >>> static void valleyview_disable_rps(struct drm_device *dev) >>> { >>> struct drm_i915_private *dev_priv =3D dev->dev_private; >>> @@ -3551,6 +3563,29 @@ int valleyview_rps_min_freq(struct drm_i915_priv= ate *dev_priv) >>> return vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff; >>> } >>> = >>> +static void cherryview_setup_pctx(struct drm_device *dev) >>> +{ >>> + struct drm_i915_private *dev_priv =3D dev->dev_private; >>> + unsigned long pctx_paddr; >>> + struct i915_gtt *gtt =3D &dev_priv->gtt; >>> + u32 pcbr; >>> + int pctx_size =3D 32*1024; >>> + >>> + pcbr =3D I915_READ(VLV_PCBR); >>> + if (pcbr >> 12 =3D=3D 0) { >>> + /* >>> + * From the Gunit register HAS: >>> + * The Gfx driver is expected to program this register and ensure >>> + * proper allocation within Gfx stolen memory. For example, this >>> + * register should be programmed such than the PCBR range does not >>> + * overlap with other relevant ranges. >>> + */ >>> + pctx_paddr =3D (dev_priv->mm.stolen_base + gtt->stolen_size - pctx_s= ize); >>> + I915_WRITE(VLV_PCBR, pctx_paddr); >>> + } >>> +} >>> + >>> + >>> static void valleyview_setup_pctx(struct drm_device *dev) >>> { >>> struct drm_i915_private *dev_priv =3D dev->dev_private; >>> @@ -3595,6 +3630,61 @@ out: >>> dev_priv->vlv_pctx =3D pctx; >>> } >>> = >>> +static void cherryview_enable_rps(struct drm_device *dev) >>> +{ >>> + struct drm_i915_private *dev_priv =3D dev->dev_private; >>> + struct intel_ring_buffer *ring; >>> + u32 gtfifodbg, rc6_mode =3D 0, pcbr; >>> + int i; >>> + >>> + WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock)); >>> + >>> + if ((gtfifodbg =3D I915_READ(GTFIFODBG))) { >> Please no assignment within if; this one's easy to split. >> >> There's a bunch of other checkpatch issues in the series; I don't >> personally care about most of them but you might want to run it and see >> if you want to do something about it. > Looks like it's a straight up copy-paste from the gen6 and vlv code. So > someone might want to clean those out as well. > > And maybe we should just drop this check for CHV since the GT wake FIFO > isn't used anymore. But I'm not sure if the register still hold something > sensible or not. > >> BR, >> Jani. >> >> >>> + DRM_DEBUG_DRIVER("GT fifo had a previous error %x\n", >>> + gtfifodbg); >>> + I915_WRITE(GTFIFODBG, gtfifodbg); >>> + } >>> + >>> + cherryview_setup_pctx(dev); >>> + >>> + /* 1a & 1b: Get forcewake during program sequence. Although the driver >>> + * hasn't enabled a state yet where we need forcewake, BIOS may have.= */ >>> + gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL); >>> + >>> + /* 2a: Program RC6 thresholds.*/ >>> + I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16); >>> + I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); /* 12500 * 1280ns */ >>> + I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25); /* 25 * 1280ns */ >>> + >>> + for_each_ring(ring, dev_priv, i) >>> + I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10); >>> + >>> + I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */ >>> + >>> + /* allows RC6 residency counter to work */ >>> + I915_WRITE(VLV_COUNTER_CONTROL, >>> + _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH | >>> + VLV_MEDIA_RC6_COUNT_EN | >>> + VLV_RENDER_RC6_COUNT_EN)); >>> + >>> + /* Todo: If BIOS has not configured PCBR >>> + * then allocate in BIOS Reserved */ >>> + >>> + /* For now we assume BIOS is allocating and populating the PCBR */ >>> + pcbr =3D I915_READ(VLV_PCBR); >>> + >>> + DRM_DEBUG_DRIVER("PCBR offset : 0x%x\n", pcbr); >>> + >>> + /* 3: Enable RC6 */ >>> + if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE & (pcbr >> 12)) >>> + rc6_mode =3D GEN6_RC_CTL_EI_MODE(1) | VLV_RC_CTL_CTX_RST_PARALLEL; >>> + >>> + I915_WRITE(GEN6_RC_CONTROL, rc6_mode); >>> + >>> + gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL); >>> +} >>> + >>> + >>> static void valleyview_enable_rps(struct drm_device *dev) >>> { >>> struct drm_i915_private *dev_priv =3D dev->dev_private; >>> @@ -4437,7 +4527,9 @@ void intel_disable_gt_powersave(struct drm_device= *dev) >>> cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work); >>> cancel_work_sync(&dev_priv->rps.work); >>> mutex_lock(&dev_priv->rps.hw_lock); >>> - if (IS_VALLEYVIEW(dev)) >>> + if (IS_CHERRYVIEW(dev)) >>> + cherryview_disable_rps(dev); >>> + else if (IS_VALLEYVIEW(dev)) >>> valleyview_disable_rps(dev); >>> else >>> gen6_disable_rps(dev); >>> @@ -4455,7 +4547,9 @@ static void intel_gen6_powersave_work(struct work= _struct *work) >>> = >>> mutex_lock(&dev_priv->rps.hw_lock); >>> = >>> - if (IS_VALLEYVIEW(dev)) { >>> + if (IS_CHERRYVIEW(dev)) { >>> + cherryview_enable_rps(dev); >>> + } else if (IS_VALLEYVIEW(dev)) { >>> valleyview_enable_rps(dev); >>> } else if (IS_BROADWELL(dev)) { >>> gen8_enable_rps(dev); >>> @@ -4476,7 +4570,7 @@ void intel_enable_gt_powersave(struct drm_device = *dev) >>> ironlake_enable_drps(dev); >>> ironlake_enable_rc6(dev); >>> intel_init_emon(dev); >>> - } else if (IS_GEN6(dev) || IS_GEN7(dev)) { >>> + } else if (INTEL_INFO(dev)->gen >=3D 6) { >>> if (IS_VALLEYVIEW(dev)) >>> valleyview_setup_pctx(dev); >>> /* >>> @@ -5051,6 +5145,7 @@ static void valleyview_init_clock_gating(struct d= rm_device *dev) >>> dev_priv->mem_freq =3D 1333; >>> break; >>> } >>> + >>> DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq); >>> = >>> dev_priv->vlv_cdclk_freq =3D valleyview_cur_cdclk(dev_priv); >>> -- = >>> 1.8.3.2 >>> >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> -- = >> Jani Nikula, Intel Open Source Technology Center