From mboxrd@z Thu Jan 1 00:00:00 1970 From: Deepak S Subject: Re: [PATCH v2 1/3] drm/i915: gmch: factor out intel_set_memory_cxsr Date: Tue, 01 Jul 2014 09:18:48 +0530 Message-ID: <53B22FA0.2000500@linux.intel.com> References: <1402660461-32294-1-git-send-email-imre.deak@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id AAD266E127 for ; Sun, 29 Jun 2014 20:53:59 -0700 (PDT) In-Reply-To: <1402660461-32294-1-git-send-email-imre.deak@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Imre Deak , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Friday 13 June 2014 05:24 PM, Imre Deak wrote: > This functionality will be also needed by an upcoming patch, so factor > it out. As a bonus this also makes things a bit more uniform across > platforms. Note that this also changes the register read-modify-write > to a simple write during disabling. This is what we do during enabling > anyway and according to the spec all the relevant bits are reserved-MBZ > or reserved with a 0 default value. > > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_pm.c | 75 ++++++++++++++++++++--------------------- > 2 files changed, 39 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index f36d9eb..211a173 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2590,6 +2590,8 @@ extern void gen6_set_rps(struct drm_device *dev, u8 val); > extern void valleyview_set_rps(struct drm_device *dev, u8 val); > extern int valleyview_rps_max_freq(struct drm_i915_private *dev_priv); > extern int valleyview_rps_min_freq(struct drm_i915_private *dev_priv); > +extern void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, > + bool enable); > extern void intel_detect_pch(struct drm_device *dev); > extern int intel_trans_dp_port_sel(struct drm_crtc *crtc); > extern int intel_enable_rc6(const struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 0b088fe..e55622e 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -789,12 +789,33 @@ static const struct cxsr_latency *intel_get_cxsr_latency(int is_desktop, > return NULL; > } > > -static void pineview_disable_cxsr(struct drm_device *dev) > +void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable) > { > - struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_device *dev = dev_priv->dev; > + u32 val; > > - /* deactivate cxsr */ > - I915_WRITE(DSPFW3, I915_READ(DSPFW3) & ~PINEVIEW_SELF_REFRESH_EN); > + if (IS_VALLEYVIEW(dev)) { > + I915_WRITE(FW_BLC_SELF_VLV, enable ? FW_CSPWRDWNEN : 0); > + } else if (IS_G4X(dev) || IS_CRESTLINE(dev)) { > + I915_WRITE(FW_BLC_SELF, enable ? FW_BLC_SELF_EN : 0); > + } else if (IS_PINEVIEW(dev)) { > + val = I915_READ(DSPFW3) & ~PINEVIEW_SELF_REFRESH_EN; > + val |= enable ? PINEVIEW_SELF_REFRESH_EN : 0; > + I915_WRITE(DSPFW3, val); > + } else if (IS_I945G(dev) || IS_I945GM(dev)) { > + val = enable ? _MASKED_BIT_ENABLE(FW_BLC_SELF_EN) : > + _MASKED_BIT_DISABLE(FW_BLC_SELF_EN); > + I915_WRITE(FW_BLC_SELF, val); > + } else if (IS_I915GM(dev)) { > + val = enable ? _MASKED_BIT_ENABLE(INSTPM_SELF_EN) : > + _MASKED_BIT_DISABLE(INSTPM_SELF_EN); > + I915_WRITE(INSTPM, val); > + } else { > + return; > + } > + > + DRM_DEBUG_KMS("memory self-refresh is %s\n", > + enable ? "enabled" : "disabled"); > } > > /* > @@ -1033,7 +1054,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc) > dev_priv->fsb_freq, dev_priv->mem_freq); > if (!latency) { > DRM_DEBUG_KMS("Unknown FSB/MEM found, disable CxSR\n"); > - pineview_disable_cxsr(dev); > + intel_set_memory_cxsr(dev_priv, false); > return; > } > > @@ -1085,12 +1106,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc) > DRM_DEBUG_KMS("DSPFW3 register is %x\n", reg); > > /* activate cxsr */ > - I915_WRITE(DSPFW3, > - I915_READ(DSPFW3) | PINEVIEW_SELF_REFRESH_EN); > - DRM_DEBUG_KMS("Self-refresh is enabled\n"); > - } else { > - pineview_disable_cxsr(dev); > - DRM_DEBUG_KMS("Self-refresh is disabled\n"); > + intel_set_memory_cxsr(dev_priv, true); I think we need to pass "false" here to disable cxsr right? Apart for this everything else looks good. Reviewed-by: Deepak S > } > } > > @@ -1342,10 +1358,9 @@ static void valleyview_update_wm(struct drm_crtc *crtc) > &valleyview_wm_info, > &valleyview_cursor_wm_info, > &ignore_plane_sr, &cursor_sr)) { > - I915_WRITE(FW_BLC_SELF_VLV, FW_CSPWRDWNEN); > + intel_set_memory_cxsr(dev_priv, true); > } else { > - I915_WRITE(FW_BLC_SELF_VLV, > - I915_READ(FW_BLC_SELF_VLV) & ~FW_CSPWRDWNEN); > + intel_set_memory_cxsr(dev_priv, false); > plane_sr = cursor_sr = 0; > } > > @@ -1394,10 +1409,9 @@ static void g4x_update_wm(struct drm_crtc *crtc) > &g4x_wm_info, > &g4x_cursor_wm_info, > &plane_sr, &cursor_sr)) { > - I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN); > + intel_set_memory_cxsr(dev_priv, true); > } else { > - I915_WRITE(FW_BLC_SELF, > - I915_READ(FW_BLC_SELF) & ~FW_BLC_SELF_EN); > + intel_set_memory_cxsr(dev_priv, false); > plane_sr = cursor_sr = 0; > } > > @@ -1468,13 +1482,10 @@ static void i965_update_wm(struct drm_crtc *unused_crtc) > DRM_DEBUG_KMS("self-refresh watermark: display plane %d " > "cursor %d\n", srwm, cursor_sr); > > - if (IS_CRESTLINE(dev)) > - I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN); > + intel_set_memory_cxsr(dev_priv, true); > } else { > /* Turn off self refresh if both pipes are enabled */ > - if (IS_CRESTLINE(dev)) > - I915_WRITE(FW_BLC_SELF, I915_READ(FW_BLC_SELF) > - & ~FW_BLC_SELF_EN); > + intel_set_memory_cxsr(dev_priv, false); > } > > DRM_DEBUG_KMS("Setting FIFO watermarks - A: 8, B: 8, C: 8, SR %d\n", > @@ -1560,10 +1571,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc) > cwm = 2; > > /* Play safe and disable self-refresh before adjusting watermarks. */ > - if (IS_I945G(dev) || IS_I945GM(dev)) > - I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN_MASK | 0); > - else if (IS_I915GM(dev)) > - I915_WRITE(INSTPM, _MASKED_BIT_DISABLE(INSTPM_SELF_EN)); > + intel_set_memory_cxsr(dev_priv, false); > > /* Calc sr entries for one plane configs */ > if (HAS_FW_BLC(dev) && enabled) { > @@ -1609,17 +1617,8 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc) > I915_WRITE(FW_BLC, fwater_lo); > I915_WRITE(FW_BLC2, fwater_hi); > > - if (HAS_FW_BLC(dev)) { > - if (enabled) { > - if (IS_I945G(dev) || IS_I945GM(dev)) > - I915_WRITE(FW_BLC_SELF, > - FW_BLC_SELF_EN_MASK | FW_BLC_SELF_EN); > - else if (IS_I915GM(dev)) > - I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_SELF_EN)); > - DRM_DEBUG_KMS("memory self refresh enabled\n"); > - } else > - DRM_DEBUG_KMS("memory self refresh disabled\n"); > - } > + if (enabled) > + intel_set_memory_cxsr(dev_priv, true); > } > > static void i845_update_wm(struct drm_crtc *unused_crtc) > @@ -6662,7 +6661,7 @@ void intel_init_pm(struct drm_device *dev) > (dev_priv->is_ddr3 == 1) ? "3" : "2", > dev_priv->fsb_freq, dev_priv->mem_freq); > /* Disable CxSR and never update its watermark again */ > - pineview_disable_cxsr(dev); > + intel_set_memory_cxsr(dev_priv, false); > dev_priv->display.update_wm = NULL; > } else > dev_priv->display.update_wm = pineview_update_wm;