On Thu, Nov 6, 2014 at 1:32 PM, R, Durgadoss <durgadoss.r@xxxxxxxxx> wrote: >>-----Original Message----- >>From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of >>Rodrigo Vivi >>Sent: Wednesday, October 29, 2014 12:16 AM >>To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx >>Cc: Vivi, Rodrigo >>Subject: [PATCH 04/10] drm/i915: PSR VLV/CHV: Introduce setup, >>enable and disable functions >> >>The biggest difference from HSW/BDW PSR here is that VLV enable_source >>function enables PSR but let it in Inactive state. So it might be called >>on early stage along with setup and enable_sink ones. >> >>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> >>--- >> drivers/gpu/drm/i915/intel_dp.c | 161 >>++++++++++++++++++++++++++++++++++++---- >> 1 file changed, 145 insertions(+), 16 deletions(-) >> >>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >>index ac70b3a..335a711 100644 >>--- a/drivers/gpu/drm/i915/intel_dp.c >>+++ b/drivers/gpu/drm/i915/intel_dp.c >>@@ -2134,7 +2134,44 @@ static void intel_edp_psr_write_vsc(struct intel_dp >>*intel_dp, >> POSTING_READ(ctl_reg); >> } >> >>-static void intel_edp_psr_setup_vsc(struct intel_dp *intel_dp) >>+/** >>+ * vlv_edp_psr_setup_vsc - Setup PSR related VSC aux registers on VLV+. >>+ * @intel_dp: DP struct >>+ * >>+ * On VLV we don't need to generate VSC. It auto generates according >>+ * EDP 1.3 spec, Table 3.10. >>+ * HB0 - Secondary Data Packet ID = 0 >>+ * HB1 - Secondary Data Packet Type = 07h >>+ * HB2 - Bits 4:0 = Revision Number = 02h >>+ * Bits 7:5 = Reserved (all 0s) >>+ * HB3 - Bits 4:0 Number of Valid Data Bytes = 08h >>+ * Bits 7:5 Reserved >>+ * DB0 - Bits 7:4 Stereo Interface Method Specific Paramenter >>+ * Bits 3:0 Stereo Interface Method Code >>+ * DB1 - Bit 0 - PSR State (0 when Inactive / 1 when Active) >>+ * Bit 1 - Update RFB. (0 do not update RFB / 1 update RFB) >>+ * Bit 2 - CRC Valid. VLV always send 0 which >>+ * means CRC value in DB7:2 invalid >>+ * Bits 7:3 - Reserved >>+ * DB2-7 - CRC - don't care because DB1[2] always tells this is invalid. >>+ */ >>+static void vlv_edp_psr_setup_vsc(struct intel_dp *intel_dp) >>+{ >>+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); >>+ struct drm_device *dev = intel_dig_port->base.base.dev; >>+ struct drm_i915_private *dev_priv = dev->dev_private; >>+ struct drm_crtc *crtc = intel_dig_port->base.base.crtc; >>+ enum pipe pipe = to_intel_crtc(crtc)->pipe; >>+ uint32_t val; >>+ >>+ /* VLV auto-generate VSC package as per EDP 1.3 spec, Table 3.10 */ >>+ val = I915_READ(VLV_VSCSDP(pipe)); >>+ val &= ~VLV_EDP_PSR_SDP_FREQ_MASK; >>+ val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME; >>+ I915_WRITE(VLV_VSCSDP(pipe), val); >>+} >>+ >>+static void hsw_edp_psr_setup_vsc(struct intel_dp *intel_dp) >> { >> struct edp_vsc_psr psr_vsc; >> >>@@ -2147,7 +2184,20 @@ static void intel_edp_psr_setup_vsc(struct intel_dp >>*intel_dp) >> intel_edp_psr_write_vsc(intel_dp, &psr_vsc); >> } >> >>-static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp) >>+/** >>+ * vlv_edp_psr_enable_sink - Enable PSR on Panel for VLV. >>+ * @intel_dp: DP struct >>+ * >>+ * This function enable PSR on Panel (Sink) side over DPCD write. >>+ */ >>+static void vlv_edp_psr_enable_sink(struct intel_dp *intel_dp) >>+{ >>+ /* Enable PSR in sink */ >>+ drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, >>+ DP_PSR_ENABLE); >>+} >>+ >>+static void hsw_edp_psr_enable_sink(struct intel_dp *intel_dp) >> { >> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); >> struct drm_device *dev = dig_port->base.base.dev; >>@@ -2191,7 +2241,28 @@ static void intel_edp_psr_enable_sink(struct intel_dp >>*intel_dp) >> (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT)); >> } >> >>-static void intel_edp_psr_enable_source(struct intel_dp *intel_dp) >>+/** >>+ * vlv_edp_psr_enable_source - Enable PSR on VLV without activate it. >>+ * @intel_dp: DP struct >>+ * >>+ * This function do the transition from PSR_state 0 to PSR_state 1 that is >>+ * PSR Inactive one.. >>+ */ >>+static void vlv_edp_psr_enable_source(struct intel_dp *intel_dp) >>+{ >>+ struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); >>+ struct drm_device *dev = dig_port->base.base.dev; >>+ struct drm_i915_private *dev_priv = dev->dev_private; >>+ struct drm_crtc *crtc = dig_port->base.base.crtc; >>+ enum pipe pipe = to_intel_crtc(crtc)->pipe; >>+ >>+ I915_WRITE(VLV_PSRCTL(pipe), >>+ VLV_EDP_PSR_SRC_TRANSMITTER_STATE | > > I see we are keeping the source active during PSR. > Making this '0' may provide better power savings, > but we need to manage a lot of hassle during PSR exit.. > (like Link training, DPIO/PLL bring up etc..) Also I think this blocked sink crc to work. So we wouldn't be able to validate it. Although there are other things blocking that tests :( But I can double check that anyway. > >>+ VLV_EDP_PSR_MODE_SW_TIMER | >>+ VLV_EDP_PSR_ENABLE); >>+} >>+ >>+static void hsw_edp_psr_enable_source(struct intel_dp *intel_dp) >> { >> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); >> struct drm_device *dev = dig_port->base.base.dev; >>@@ -2276,7 +2347,7 @@ static void intel_edp_psr_do_enable(struct intel_dp >>*intel_dp) >> lockdep_assert_held(&dev_priv->psr.lock); >> >> /* Enable/Re-enable PSR on the host */ >>- intel_edp_psr_enable_source(intel_dp); >>+ hsw_edp_psr_enable_source(intel_dp); >> >> dev_priv->psr.active = true; >> } >>@@ -2307,30 +2378,71 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp) >> >> dev_priv->psr.busy_frontbuffer_bits = 0; >> >>- intel_edp_psr_setup_vsc(intel_dp); >>+ if (HAS_DDI(dev)) { >>+ hsw_edp_psr_setup_vsc(intel_dp); >>+ >>+ /* Avoid continuous PSR exit by masking memup and hpd */ >>+ I915_WRITE(EDP_PSR_DEBUG_CTL(dev), >>EDP_PSR_DEBUG_MASK_MEMUP | >>+ EDP_PSR_DEBUG_MASK_HPD | >>EDP_PSR_DEBUG_MASK_LPSP); >> >>- /* Avoid continuous PSR exit by masking memup and hpd */ >>- I915_WRITE(EDP_PSR_DEBUG_CTL(dev), >>EDP_PSR_DEBUG_MASK_MEMUP | >>- EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP); >>+ /* Enable PSR on the panel */ >>+ hsw_edp_psr_enable_sink(intel_dp); >>+ } else { >>+ vlv_edp_psr_setup_vsc(intel_dp); >> >>- /* Enable PSR on the panel */ >>- intel_edp_psr_enable_sink(intel_dp); >>+ /* Enable PSR on the panel */ >>+ vlv_edp_psr_enable_sink(intel_dp); >>+ >>+ /* On HSW/BDW enable_source also means go to PSR entry/active >>+ * state as soon as idle_frame achieved and here would be >>+ * to soon. However on VLV enable_source just enable PSR >>+ * but let it on inactive state. So we might do this prior >>+ * to active transition, i.e. here. >>+ */ >>+ vlv_edp_psr_enable_source(intel_dp); >>+ } >> >> dev_priv->psr.enabled = intel_dp; >> unlock: >> mutex_unlock(&dev_priv->psr.lock); >> } >> >>-void intel_edp_psr_disable(struct intel_dp *intel_dp) >>+/** >>+ * vlv_edp_psr_disable - Disable PSR on VLV >>+ * @intel_dp: DP struct >>+ * >>+ * This function puts PSR VLV back to PSR_state 0 that is PSR Disabled. > > I hope you meant 'puts VLV PSR'.. Yes thanks! > >>+ */ >>+static void vlv_edp_psr_disable(struct intel_dp *intel_dp) >> { >>- struct drm_device *dev = intel_dp_to_dev(intel_dp); >>+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); >>+ struct drm_device *dev = intel_dig_port->base.base.dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >>+ struct intel_crtc *intel_crtc = >>+ to_intel_crtc(intel_dig_port->base.base.crtc); >>+ uint32_t val; >> >>- mutex_lock(&dev_priv->psr.lock); >>- if (!dev_priv->psr.enabled) { >>- mutex_unlock(&dev_priv->psr.lock); >>- return; >>+ if (dev_priv->psr.active) { >>+ if (wait_for((I915_READ(VLV_PSRSTAT(intel_crtc->pipe)) & >>+ VLV_EDP_PSR_IN_TRANS) == 0, 250)) >>+ WARN(1, "PSR transition took longer than expected\n"); > > The spec claims this waiting time as 250 micro seconds. > Looks like we are waiting for 250 ms.. No ? This is true. I'll fix that and test tomorrow. > > Just curious, did you see this transition during testing ? No, I never checked. > > Thanks, > Durga > >>+ >>+ val = I915_READ(VLV_PSRCTL(intel_crtc->pipe)); >>+ val &= ~VLV_EDP_PSR_ACTIVE_ENTRY; >>+ val &= ~VLV_EDP_PSR_ENABLE; >>+ val &= ~VLV_EDP_PSR_MODE_MASK; >>+ I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), val); >>+ >>+ dev_priv->psr.active = false; >>+ } else { >>+ WARN_ON(vlv_edp_is_psr_enabled_on_pipe(dev, intel_crtc- >>>pipe)); >> } >>+} >>+ >>+static void hsw_edp_psr_disable(struct intel_dp *intel_dp) >>+{ >>+ struct drm_device *dev = intel_dp_to_dev(intel_dp); >>+ struct drm_i915_private *dev_priv = dev->dev_private; >> >> if (dev_priv->psr.active) { >> I915_WRITE(EDP_PSR_CTL(dev), >>@@ -2345,6 +2457,23 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp) >> } else { >> WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE); >> } >>+} >>+ >>+void intel_edp_psr_disable(struct intel_dp *intel_dp) >>+{ >>+ struct drm_device *dev = intel_dp_to_dev(intel_dp); >>+ struct drm_i915_private *dev_priv = dev->dev_private; >>+ >>+ mutex_lock(&dev_priv->psr.lock); >>+ if (!dev_priv->psr.enabled) { >>+ mutex_unlock(&dev_priv->psr.lock); >>+ return; >>+ } >>+ >>+ if (HAS_DDI(dev)) >>+ hsw_edp_psr_disable(intel_dp); >>+ else >>+ vlv_edp_psr_disable(intel_dp); >> >> dev_priv->psr.enabled = NULL; >> mutex_unlock(&dev_priv->psr.lock); >>-- >>1.9.3 >> >>_______________________________________________ >>Intel-gfx mailing list >>Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >>http://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
