| Message ID | 1391627071-10097-1-git-send-email-rodrigo.vivi@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 05, 2014 at 05:04:31PM -0200, Rodrigo Vivi wrote: > This patch adds PSR Support to Baytrail. > > Baytrail cannot easily detect screen updates and force PSR exit. > So we inactivate it on {busy_ioctl, set_domain, sw_finish and mark_busy} > and update to enable it back on next display mark_idle. > > v2: Also inactivate PSR on cursor update. > v3: Inactivate PSR on mark_busy, dset_domain and sw_finish_ioctl, and > early on page flip besides avoid initializing inactive/active flag > more than once. > v4: Fix identation issues. > v5: Rebase and add Baytrail per pipe support although leaving PIPE_B > support disabled by for now since it isn't working properly yet. > v6: Removing forgotten comment and useless clkgating definition. > v7: Remove inactivate from set_domain. Chris warned this was semanticaly > wrong. > v8: Accept Ville's suggestions: Use register's names matching spec and > warn if transition took longer than it should. > v9: New version with delayed work to get PSR back. Disabling it on > set_domain but not rescheduing it back until next finish_page_flip. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 36 ++++- > drivers/gpu/drm/i915/i915_drv.h | 5 +- > drivers/gpu/drm/i915/i915_gem.c | 12 ++ > drivers/gpu/drm/i915/i915_reg.h | 37 +++++ > drivers/gpu/drm/i915/i915_suspend.c | 2 +- > drivers/gpu/drm/i915/intel_display.c | 18 ++- > drivers/gpu/drm/i915/intel_dp.c | 256 ++++++++++++++++++++++++++++++----- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 8 files changed, 323 insertions(+), 44 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index bc8707f..2949c48 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1900,6 +1900,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) > struct drm_device *dev = node->minor->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > u32 psrperf = 0; > + u32 statA = 0; > + u32 statB = 0; > bool enabled = false; > > intel_runtime_pm_get(dev_priv); > @@ -1907,14 +1909,38 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) > seq_printf(m, "Sink_Support: %s\n", yesno(dev_priv->psr.sink_support)); > seq_printf(m, "Source_OK: %s\n", yesno(dev_priv->psr.source_ok)); > > - enabled = HAS_PSR(dev) && > - I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; > - seq_printf(m, "Enabled: %s\n", yesno(enabled)); > + if (HAS_PSR(dev)) { > + if (IS_VALLEYVIEW(dev)) { > + statA = I915_READ(VLV_PSRSTAT(PIPE_A)) & > + VLV_EDP_PSR_CURR_STATE_MASK; > + statB = I915_READ(VLV_PSRSTAT(PIPE_B)) & > + VLV_EDP_PSR_CURR_STATE_MASK; > + enabled = ((statA == VLV_EDP_PSR_ACTIVE_NORFB_UP) || > + (statA == VLV_EDP_PSR_ACTIVE_SF_UPDATE) || > + (statB == VLV_EDP_PSR_ACTIVE_NORFB_UP) || > + (statB == VLV_EDP_PSR_ACTIVE_SF_UPDATE)); > + } else > + enabled = I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; > + } > + seq_printf(m, "Enabled: %s", yesno(enabled)); > > - if (HAS_PSR(dev)) > + if (IS_VALLEYVIEW(dev)) { > + if ((statA == VLV_EDP_PSR_ACTIVE_NORFB_UP) || > + (statA == VLV_EDP_PSR_ACTIVE_SF_UPDATE)) > + seq_puts(m, " pipe A"); > + if ((statB == VLV_EDP_PSR_ACTIVE_NORFB_UP) || > + (statB == VLV_EDP_PSR_ACTIVE_SF_UPDATE)) > + seq_puts(m, " pipe B"); > + } > + > + seq_puts(m, "\n"); > + > + /* VLV PSR has no kind of performance counter */ > + if (HAS_PSR(dev) && !IS_VALLEYVIEW(dev)) { > psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) & > EDP_PSR_PERF_CNT_MASK; > - seq_printf(m, "Performance_Counter: %u\n", psrperf); > + seq_printf(m, "Performance_Counter: %u\n", psrperf); > + } > > intel_runtime_pm_put(dev_priv); > return 0; > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 21470be..87c346a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -747,7 +747,9 @@ struct i915_psr { > bool sink_support; > bool source_ok; > bool setup_done; > + bool active; > struct mutex lock; > + struct delayed_work work; > }; > > enum intel_pch { > @@ -1867,7 +1869,8 @@ struct drm_i915_file_private { > > #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi) > #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg) > -#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev)) > +#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \ > + IS_VALLEYVIEW(dev)) > #define HAS_PC8(dev) (IS_HASWELL(dev)) /* XXX HSW:ULX */ > #define HAS_RUNTIME_PM(dev) (IS_HASWELL(dev)) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 39770f7..5ef1380 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1256,6 +1256,14 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, > goto unlock; > } > > + /* Here we don't reschedule work to get PSR back because userspace > + * can set domain and take as much as time it wants to write it. > + * We only reschedule it back on next finish_page_flip. There is the > + * risk of PSR be inactive forever depending on how compositor works, > + * but this is the safest way to avoid loosing screen updates. > + */ > + intel_edp_psr_inactivate(dev, false); This will still block until PSR is deactivated (aux communication and all). That still seems like a bad idea to do while holding struct_mutex. > + > /* Try to flush the object off the GPU without holding the lock. > * We will repeat the flush holding the lock in the normal manner > * to catch cases where we are gazumped. > @@ -1299,6 +1307,8 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data, > if (ret) > return ret; > > + intel_edp_psr_inactivate(dev, true); > + > obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); > if (&obj->base == NULL) { > ret = -ENOENT; > @@ -4059,6 +4069,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, > if (ret) > return ret; > > + intel_edp_psr_inactivate(dev, true); > + > obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); > if (&obj->base == NULL) { > ret = -ENOENT; > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index cbbaf26..a5815d0 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1968,6 +1968,43 @@ > #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B) > #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B) > > +/* VLV eDP PSR registers */ > +#define _PSRCTLA (VLV_DISPLAY_BASE + 0x60090) > +#define _PSRCTLB (VLV_DISPLAY_BASE + 0x61090) > +#define VLV_EDP_PSR_ENABLE (1<<0) > +#define VLV_EDP_PSR_RESET (1<<1) > +#define VLV_EDP_PSR_MODE_MASK (7<<2) > +#define VLV_EDP_PSR_MODE_HW_TIMER (1<<3) > +#define VLV_EDP_PSR_MODE_SW_TIMER (1<<2) > +#define VLV_EDP_PSR_SINGLE_FRAME_UPDATE (1<<7) > +#define VLV_EDP_PSR_ACTIVE_ENTRY (1<<8) > +#define VLV_EDP_PSR_SRC_TRANSMITTER_STATE (1<<9) > +#define VLV_EDP_PSR_DBL_FRAME (1<<10) > +#define VLV_EDP_PSR_FRAME_COUNT_MASK (0xff<<16) > +#define VLV_EDP_PSR_IDLE_FRAME_SHIFT 16 > +#define VLV_EDP_PSR_INT_TRANSITION (1<<24) > +#define VLV_PSRCTL(pipe) _PIPE(pipe, _PSRCTLA, _PSRCTLB) > + > +#define _VSCSDPA (VLV_DISPLAY_BASE + 0x600a0) > +#define _VSCSDPB (VLV_DISPLAY_BASE + 0x610a0) > +#define VLV_EDP_PSR_SDP_FREQ_MASK (3<<30) > +#define VLV_EDP_PSR_SDP_FREQ_ONCE (1<<31) > +#define VLV_EDP_PSR_SDP_FREQ_EVFRAME (1<<30) > +#define VLV_VSCSDP(pipe) _PIPE(pipe, _VSCSDPA, _VSCSDPB) > + > +#define _PSRSTATA (VLV_DISPLAY_BASE + 0x60094) > +#define _PSRSTATB (VLV_DISPLAY_BASE + 0x61094) > +#define VLV_EDP_PSR_LAST_STATE_MASK (7<<3) > +#define VLV_EDP_PSR_CURR_STATE_MASK 7 > +#define VLV_EDP_PSR_DISABLED (0<<0) > +#define VLV_EDP_PSR_INACTIVE (1<<0) > +#define VLV_EDP_PSR_IN_TRANS_TO_ACTIVE (2<<0) > +#define VLV_EDP_PSR_ACTIVE_NORFB_UP (3<<0) > +#define VLV_EDP_PSR_ACTIVE_SF_UPDATE (4<<0) > +#define VLV_EDP_PSR_EXIT (5<<0) > +#define VLV_EDP_PSR_IN_TRANS (1<<7) > +#define VLV_PSRSTAT(pipe) _PIPE(pipe, _PSRSTATA, _PSRSTATB) > + > /* HSW+ eDP PSR registers */ > #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800) > #define EDP_PSR_CTL(dev) (EDP_PSR_BASE(dev) + 0) > diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c > index ffcba21..6b31020 100644 > --- a/drivers/gpu/drm/i915/i915_suspend.c > +++ b/drivers/gpu/drm/i915/i915_suspend.c > @@ -289,8 +289,8 @@ static void i915_restore_display(struct drm_device *dev) > } > > /* Force a full PSR setup on resume */ > + intel_edp_psr_inactivate(dev, false); > dev_priv->psr.setup_done = false; > - intel_edp_psr_update(dev); > > /* only restore FBC info on the platform that supports FBC*/ > intel_disable_fbc(dev); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 1a9aa19..92d6df4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4451,9 +4451,13 @@ static void intel_connector_check_state(struct intel_connector *connector) > * consider. */ > void intel_connector_dpms(struct drm_connector *connector, int mode) > { > + struct drm_device *dev = connector->dev; > + > /* All the simple cases only support two dpms states. */ > - if (mode != DRM_MODE_DPMS_ON) > + if (mode != DRM_MODE_DPMS_ON) { > mode = DRM_MODE_DPMS_OFF; > + intel_edp_psr_inactivate(dev, false); Maybe check if this connector even has PSR enabled? > + } > > if (mode == connector->dpms) > return; > @@ -7501,6 +7505,8 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, > u32 base = 0, pos = 0; > bool visible; > > + intel_edp_psr_inactivate(dev, true); Don't need to inactivate unless the cursor is or was visible. > + > if (on) > base = intel_crtc->cursor_addr; > > @@ -8230,12 +8236,15 @@ void intel_mark_idle(struct drm_device *dev) > gen6_rps_idle(dev->dev_private); > } > > + > void intel_mark_fb_busy(struct drm_i915_gem_object *obj, > struct intel_ring_buffer *ring) > { > struct drm_device *dev = obj->base.dev; > struct drm_crtc *crtc; > > + intel_edp_psr_inactivate(dev, true); > + > if (!i915.powersave) > return; > > @@ -8336,6 +8345,10 @@ static void do_intel_finish_page_flip(struct drm_device *dev, > queue_work(dev_priv->wq, &work->work); > > trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj); > + > + /* Get PSR back after set_domain inactivated it without rescheduling > + * it back. */ > + schedule_delayed_work(&dev_priv->psr.work, msecs_to_jiffies(5000)); > } > > void intel_finish_page_flip(struct drm_device *dev, int pipe) > @@ -8688,6 +8701,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > if (work == NULL) > return -ENOMEM; > > + /* Inactivate PSR early in page flip */ > + intel_edp_psr_inactivate(dev, true); > + > work->event = event; > work->crtc = crtc; > work->old_fb_obj = to_intel_framebuffer(old_fb)->obj; > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 80054bb..576c204 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1578,21 +1578,35 @@ static void intel_dp_get_config(struct intel_encoder *encoder, > } > } > > -static bool is_edp_psr(struct drm_device *dev) > +static bool is_edp_psr(struct intel_dp *intel_dp) > +{ > + return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED; > +} > + > +static bool vlv_edp_is_psr_enabled_on_pipe(struct drm_device *dev, int pipe) > { > struct drm_i915_private *dev_priv = dev->dev_private; > + uint32_t val; > > - return dev_priv->psr.sink_support; > + val = I915_READ(VLV_PSRSTAT(pipe)) & > + VLV_EDP_PSR_CURR_STATE_MASK; > + return (val == VLV_EDP_PSR_ACTIVE_NORFB_UP) || > + (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE); > } > > static bool intel_edp_is_psr_enabled(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > > - if (!HAS_PSR(dev)) > - return false; > + if (HAS_PSR(dev)) { > + if (IS_VALLEYVIEW(dev)) > + return vlv_edp_is_psr_enabled_on_pipe(dev, PIPE_A) || > + vlv_edp_is_psr_enabled_on_pipe(dev, PIPE_B); > + else > + return I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; > + } > > - return I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; > + return false; > } > > static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp, > @@ -1624,32 +1638,60 @@ static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp, > POSTING_READ(ctl_reg); > } > > +static void intel_edp_psr_work(struct work_struct *work) > +{ > + struct drm_i915_private *dev_priv = > + container_of(work, typeof(*dev_priv), psr.work.work); > + struct drm_device *dev = dev_priv->dev; > + > + intel_edp_psr_update(dev); > +} > + > static void intel_edp_psr_setup(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 edp_vsc_psr psr_vsc; > + uint32_t val; > > if (dev_priv->psr.setup_done) > return; > > - mutex_init(&dev_priv->psr.lock); > - > - /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */ > - memset(&psr_vsc, 0, sizeof(psr_vsc)); > - psr_vsc.sdp_header.HB0 = 0; > - psr_vsc.sdp_header.HB1 = 0x7; > - psr_vsc.sdp_header.HB2 = 0x2; > - psr_vsc.sdp_header.HB3 = 0x8; > - intel_edp_psr_write_vsc(intel_dp, &psr_vsc); > - > - /* 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); > + if (IS_VALLEYVIEW(dev)) { > + val = I915_READ(VLV_VSCSDP(PIPE_A)); > + val &= ~VLV_EDP_PSR_SDP_FREQ_MASK; > + val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME; > + I915_WRITE(VLV_VSCSDP(PIPE_A), val); > + > + val = I915_READ(VLV_VSCSDP(PIPE_B)); > + val &= ~VLV_EDP_PSR_SDP_FREQ_MASK; > + val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME; > + I915_WRITE(VLV_VSCSDP(PIPE_B), val); > + } else { > + /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */ > + memset(&psr_vsc, 0, sizeof(psr_vsc)); > + psr_vsc.sdp_header.HB0 = 0; > + psr_vsc.sdp_header.HB1 = 0x7; > + psr_vsc.sdp_header.HB2 = 0x2; > + psr_vsc.sdp_header.HB3 = 0x8; > + intel_edp_psr_write_vsc(intel_dp, &psr_vsc); > + > + /* 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); > + } > > dev_priv->psr.setup_done = true; > } > > +static void vlv_edp_psr_enable_sink(struct intel_dp *intel_dp) > +{ > + /* Enable PSR in sink */ > + intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, > + DP_PSR_ENABLE); > +} > + > static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp) > { > struct drm_device *dev = intel_dp_to_dev(intel_dp); > @@ -1680,6 +1722,27 @@ 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 vlv_edp_psr_enable_source(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 intel_crtc *intel_crtc = > + to_intel_crtc(intel_dig_port->base.base.crtc); > + > + uint32_t idle_frames = 1; > + uint32_t val = 0; > + > + val |= VLV_EDP_PSR_ENABLE; > + val &= ~VLV_EDP_PSR_MODE_MASK; > + > + val |= VLV_EDP_PSR_MODE_HW_TIMER; > + val &= ~VLV_EDP_PSR_FRAME_COUNT_MASK; > + val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; > + > + I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), val); > +} > + > static void intel_edp_psr_enable_source(struct intel_dp *intel_dp) > { > struct drm_device *dev = intel_dp_to_dev(intel_dp); > @@ -1721,8 +1784,8 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) > return false; > } > > - if ((intel_encoder->type != INTEL_OUTPUT_EDP) || > - (dig_port->port != PORT_A)) { > + if (HAS_DDI(dev) && ((intel_encoder->type != INTEL_OUTPUT_EDP) || > + (dig_port->port != PORT_A))) { > DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n"); > return false; > } > @@ -1767,23 +1830,45 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) > return false; > } > > + /* Baytrail supports per-pipe PSR configuration, however PSR on > + * PIPE_B isn't working properly. So let's keep it disabled for now. */ > + if (IS_VALLEYVIEW(dev) && intel_crtc->pipe != PIPE_A) { > + DRM_DEBUG_KMS("PSR on BYT isn't enabled on pipe B.\n"); > + return false; > + } > + > dev_priv->psr.source_ok = true; > return true; > } > > static void intel_edp_psr_do_enable(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); > > - if (!intel_edp_psr_match_conditions(intel_dp) || > - intel_edp_is_psr_enabled(dev)) > - return; > + if (IS_VALLEYVIEW(dev)) { > + if (vlv_edp_is_psr_enabled_on_pipe(dev, intel_crtc->pipe)) > + return; > + } else > + if (intel_edp_is_psr_enabled(dev)) > + return; > > /* Enable PSR on the panel */ > - intel_edp_psr_enable_sink(intel_dp); > + if (IS_VALLEYVIEW(dev)) > + vlv_edp_psr_enable_sink(intel_dp); > + else > + intel_edp_psr_enable_sink(intel_dp); > > /* Enable PSR on the host */ > - intel_edp_psr_enable_source(intel_dp); > + if (IS_VALLEYVIEW(dev)) > + vlv_edp_psr_enable_source(intel_dp); > + else > + intel_edp_psr_enable_source(intel_dp); > + > + dev_priv->psr.active = true; > } > > void intel_edp_psr_enable(struct intel_dp *intel_dp) > @@ -1791,16 +1876,43 @@ void intel_edp_psr_enable(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 (!is_edp_psr(intel_dp)) > + return; > + > /* Setup PSR once */ > intel_edp_psr_setup(intel_dp); > > mutex_lock(&dev_priv->psr.lock); > - if (intel_edp_psr_match_conditions(intel_dp) && > - !intel_edp_is_psr_enabled(dev)) > + if (intel_edp_psr_match_conditions(intel_dp)) > intel_edp_psr_do_enable(intel_dp); > mutex_unlock(&dev_priv->psr.lock); > } > > +void vlv_edp_psr_disable(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 intel_crtc *intel_crtc = > + to_intel_crtc(intel_dig_port->base.base.crtc); > + uint32_t val; > + > + if (!dev_priv->psr.setup_done) > + return; > + > + intel_edp_psr_inactivate(dev, false); > + > + 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"); > + > + 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); > +} > + > void intel_edp_psr_disable(struct intel_dp *intel_dp) > { > struct drm_device *dev = intel_dp_to_dev(intel_dp); > @@ -1823,6 +1935,7 @@ void intel_edp_psr_update(struct drm_device *dev) > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_encoder *encoder; > struct intel_dp *intel_dp = NULL; > + struct intel_crtc *intel_crtc; > > if (!dev_priv->psr.setup_done) > return; > @@ -1830,26 +1943,93 @@ void intel_edp_psr_update(struct drm_device *dev) > list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) > if (encoder->type == INTEL_OUTPUT_EDP) { > intel_dp = enc_to_intel_dp(&encoder->base); > - > - if (!is_edp_psr(dev)) > - return; > + intel_crtc = to_intel_crtc(encoder->base.crtc); > > mutex_lock(&dev_priv->psr.lock); > - if (!intel_edp_psr_match_conditions(intel_dp)) > - intel_edp_psr_disable(intel_dp); > - else > + if (!intel_edp_psr_match_conditions(intel_dp)) { > + if (IS_VALLEYVIEW(dev)) > + vlv_edp_psr_disable(intel_dp); > + else > + intel_edp_psr_disable(intel_dp); > + } else > if (!intel_edp_is_psr_enabled(dev)) > intel_edp_psr_do_enable(intel_dp); > mutex_unlock(&dev_priv->psr.lock); > } > } > > +void intel_edp_psr_do_inactivate(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_connector *connector; > + struct intel_encoder *encoder; > + struct intel_crtc *intel_crtc; > + struct intel_dp *intel_dp = NULL; > + > + list_for_each_entry(connector, &dev->mode_config.connector_list, > + base.head) { > + > + if (connector->base.dpms != DRM_MODE_DPMS_ON) > + continue; > + > + encoder = to_intel_encoder(connector->base.encoder); > + if (encoder->type == INTEL_OUTPUT_EDP) { > + > + intel_dp = enc_to_intel_dp(&encoder->base); > + intel_crtc = to_intel_crtc(encoder->base.crtc); Still digging through the encoder and crtc pointers w/o holding any modeset locks. Feels dangerous. intel_edp_psr_match_conditions() does the same kind of stuff, and that too now gets called outside modeset codepaths. > + > + if (!vlv_edp_is_psr_enabled_on_pipe(dev, > + intel_crtc->pipe)) > + continue; > + > + dev_priv->psr.active = false; > + > + I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), > + VLV_EDP_PSR_RESET); > + /* WaClearPSRReset:vlv */ > + I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), 0); > + > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > + } > + } > +} > + > +void intel_edp_psr_inactivate(struct drm_device *dev, bool schedule_back) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (!IS_VALLEYVIEW(dev)) > + return; > + > + if (!dev_priv->psr.setup_done) > + return; > + > + /* If it was requested to not turn psr back delayed work must be > + * canceled even if it is already on inactivated state. */ > + if (!schedule_back) > + cancel_delayed_work_sync(&dev_priv->psr.work); > + > + if (!dev_priv->psr.active) > + return; What if psr gets re-activated just after you checked above? > + > + cancel_delayed_work_sync(&dev_priv->psr.work); > + > + intel_edp_psr_do_inactivate(dev); > + > + if (schedule_back) > + schedule_delayed_work(&dev_priv->psr.work, > + msecs_to_jiffies(5000)); > +} > + > static void intel_disable_dp(struct intel_encoder *encoder) > { > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > enum port port = dp_to_dig_port(intel_dp)->port; > struct drm_device *dev = encoder->base.dev; > > + if (IS_VALLEYVIEW(dev)) > + vlv_edp_psr_disable(intel_dp); > + > /* Make sure the panel is off before trying to change the mode. But also > * ensure that we have vdd while we switch off the panel. */ > intel_edp_backlight_off(intel_dp); > @@ -1906,6 +2086,7 @@ static void vlv_enable_dp(struct intel_encoder *encoder) > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > > intel_edp_backlight_on(intel_dp); > + intel_edp_psr_enable(intel_dp); > } > > static void g4x_pre_enable_dp(struct intel_encoder *encoder) > @@ -3846,8 +4027,11 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > BUG(); > } > > - if (is_edp(intel_dp)) > + if (is_edp(intel_dp)) { > intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq); > + INIT_DELAYED_WORK(&dev_priv->psr.work, intel_edp_psr_work); > + mutex_init(&dev_priv->psr.lock); Initializing global state from a per-connector init function seems a bit weird. > + } > > error = intel_dp_i2c_init(intel_dp, intel_connector, name); > WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n", > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 71c1371..d8103f9 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -748,6 +748,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp); > void intel_edp_psr_enable(struct intel_dp *intel_dp); > void intel_edp_psr_disable(struct intel_dp *intel_dp); > void intel_edp_psr_update(struct drm_device *dev); > +void intel_edp_psr_inactivate(struct drm_device *dev, bool schedule_back); > > > /* intel_dsi.c */ > -- > 1.8.1.2
On Fri, Feb 7, 2014 at 3:24 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Wed, Feb 05, 2014 at 05:04:31PM -0200, Rodrigo Vivi wrote: >> This patch adds PSR Support to Baytrail. >> >> Baytrail cannot easily detect screen updates and force PSR exit. >> So we inactivate it on {busy_ioctl, set_domain, sw_finish and mark_busy} >> and update to enable it back on next display mark_idle. >> >> v2: Also inactivate PSR on cursor update. >> v3: Inactivate PSR on mark_busy, dset_domain and sw_finish_ioctl, and >> early on page flip besides avoid initializing inactive/active flag >> more than once. >> v4: Fix identation issues. >> v5: Rebase and add Baytrail per pipe support although leaving PIPE_B >> support disabled by for now since it isn't working properly yet. >> v6: Removing forgotten comment and useless clkgating definition. >> v7: Remove inactivate from set_domain. Chris warned this was semanticaly >> wrong. >> v8: Accept Ville's suggestions: Use register's names matching spec and >> warn if transition took longer than it should. >> v9: New version with delayed work to get PSR back. Disabling it on >> set_domain but not rescheduing it back until next finish_page_flip. >> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 36 ++++- >> drivers/gpu/drm/i915/i915_drv.h | 5 +- >> drivers/gpu/drm/i915/i915_gem.c | 12 ++ >> drivers/gpu/drm/i915/i915_reg.h | 37 +++++ >> drivers/gpu/drm/i915/i915_suspend.c | 2 +- >> drivers/gpu/drm/i915/intel_display.c | 18 ++- >> drivers/gpu/drm/i915/intel_dp.c | 256 ++++++++++++++++++++++++++++++----- >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> 8 files changed, 323 insertions(+), 44 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >> index bc8707f..2949c48 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -1900,6 +1900,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) >> struct drm_device *dev = node->minor->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> u32 psrperf = 0; >> + u32 statA = 0; >> + u32 statB = 0; >> bool enabled = false; >> >> intel_runtime_pm_get(dev_priv); >> @@ -1907,14 +1909,38 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) >> seq_printf(m, "Sink_Support: %s\n", yesno(dev_priv->psr.sink_support)); >> seq_printf(m, "Source_OK: %s\n", yesno(dev_priv->psr.source_ok)); >> >> - enabled = HAS_PSR(dev) && >> - I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; >> - seq_printf(m, "Enabled: %s\n", yesno(enabled)); >> + if (HAS_PSR(dev)) { >> + if (IS_VALLEYVIEW(dev)) { >> + statA = I915_READ(VLV_PSRSTAT(PIPE_A)) & >> + VLV_EDP_PSR_CURR_STATE_MASK; >> + statB = I915_READ(VLV_PSRSTAT(PIPE_B)) & >> + VLV_EDP_PSR_CURR_STATE_MASK; >> + enabled = ((statA == VLV_EDP_PSR_ACTIVE_NORFB_UP) || >> + (statA == VLV_EDP_PSR_ACTIVE_SF_UPDATE) || >> + (statB == VLV_EDP_PSR_ACTIVE_NORFB_UP) || >> + (statB == VLV_EDP_PSR_ACTIVE_SF_UPDATE)); >> + } else >> + enabled = I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; >> + } >> + seq_printf(m, "Enabled: %s", yesno(enabled)); >> >> - if (HAS_PSR(dev)) >> + if (IS_VALLEYVIEW(dev)) { >> + if ((statA == VLV_EDP_PSR_ACTIVE_NORFB_UP) || >> + (statA == VLV_EDP_PSR_ACTIVE_SF_UPDATE)) >> + seq_puts(m, " pipe A"); >> + if ((statB == VLV_EDP_PSR_ACTIVE_NORFB_UP) || >> + (statB == VLV_EDP_PSR_ACTIVE_SF_UPDATE)) >> + seq_puts(m, " pipe B"); >> + } >> + >> + seq_puts(m, "\n"); >> + >> + /* VLV PSR has no kind of performance counter */ >> + if (HAS_PSR(dev) && !IS_VALLEYVIEW(dev)) { >> psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) & >> EDP_PSR_PERF_CNT_MASK; >> - seq_printf(m, "Performance_Counter: %u\n", psrperf); >> + seq_printf(m, "Performance_Counter: %u\n", psrperf); >> + } >> >> intel_runtime_pm_put(dev_priv); >> return 0; >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 21470be..87c346a 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -747,7 +747,9 @@ struct i915_psr { >> bool sink_support; >> bool source_ok; >> bool setup_done; >> + bool active; >> struct mutex lock; >> + struct delayed_work work; >> }; >> >> enum intel_pch { >> @@ -1867,7 +1869,8 @@ struct drm_i915_file_private { >> >> #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi) >> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg) >> -#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev)) >> +#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \ >> + IS_VALLEYVIEW(dev)) >> #define HAS_PC8(dev) (IS_HASWELL(dev)) /* XXX HSW:ULX */ >> #define HAS_RUNTIME_PM(dev) (IS_HASWELL(dev)) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index 39770f7..5ef1380 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -1256,6 +1256,14 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, >> goto unlock; >> } >> >> + /* Here we don't reschedule work to get PSR back because userspace >> + * can set domain and take as much as time it wants to write it. >> + * We only reschedule it back on next finish_page_flip. There is the >> + * risk of PSR be inactive forever depending on how compositor works, >> + * but this is the safest way to avoid loosing screen updates. >> + */ >> + intel_edp_psr_inactivate(dev, false); > > This will still block until PSR is deactivated (aux communication and > all). That still seems like a bad idea to do while holding struct_mutex. So, what do you suggest here? > >> + >> /* Try to flush the object off the GPU without holding the lock. >> * We will repeat the flush holding the lock in the normal manner >> * to catch cases where we are gazumped. >> @@ -1299,6 +1307,8 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data, >> if (ret) >> return ret; >> >> + intel_edp_psr_inactivate(dev, true); >> + >> obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); >> if (&obj->base == NULL) { >> ret = -ENOENT; >> @@ -4059,6 +4069,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, >> if (ret) >> return ret; >> >> + intel_edp_psr_inactivate(dev, true); >> + >> obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); >> if (&obj->base == NULL) { >> ret = -ENOENT; >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index cbbaf26..a5815d0 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -1968,6 +1968,43 @@ >> #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B) >> #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B) >> >> +/* VLV eDP PSR registers */ >> +#define _PSRCTLA (VLV_DISPLAY_BASE + 0x60090) >> +#define _PSRCTLB (VLV_DISPLAY_BASE + 0x61090) >> +#define VLV_EDP_PSR_ENABLE (1<<0) >> +#define VLV_EDP_PSR_RESET (1<<1) >> +#define VLV_EDP_PSR_MODE_MASK (7<<2) >> +#define VLV_EDP_PSR_MODE_HW_TIMER (1<<3) >> +#define VLV_EDP_PSR_MODE_SW_TIMER (1<<2) >> +#define VLV_EDP_PSR_SINGLE_FRAME_UPDATE (1<<7) >> +#define VLV_EDP_PSR_ACTIVE_ENTRY (1<<8) >> +#define VLV_EDP_PSR_SRC_TRANSMITTER_STATE (1<<9) >> +#define VLV_EDP_PSR_DBL_FRAME (1<<10) >> +#define VLV_EDP_PSR_FRAME_COUNT_MASK (0xff<<16) >> +#define VLV_EDP_PSR_IDLE_FRAME_SHIFT 16 >> +#define VLV_EDP_PSR_INT_TRANSITION (1<<24) >> +#define VLV_PSRCTL(pipe) _PIPE(pipe, _PSRCTLA, _PSRCTLB) >> + >> +#define _VSCSDPA (VLV_DISPLAY_BASE + 0x600a0) >> +#define _VSCSDPB (VLV_DISPLAY_BASE + 0x610a0) >> +#define VLV_EDP_PSR_SDP_FREQ_MASK (3<<30) >> +#define VLV_EDP_PSR_SDP_FREQ_ONCE (1<<31) >> +#define VLV_EDP_PSR_SDP_FREQ_EVFRAME (1<<30) >> +#define VLV_VSCSDP(pipe) _PIPE(pipe, _VSCSDPA, _VSCSDPB) >> + >> +#define _PSRSTATA (VLV_DISPLAY_BASE + 0x60094) >> +#define _PSRSTATB (VLV_DISPLAY_BASE + 0x61094) >> +#define VLV_EDP_PSR_LAST_STATE_MASK (7<<3) >> +#define VLV_EDP_PSR_CURR_STATE_MASK 7 >> +#define VLV_EDP_PSR_DISABLED (0<<0) >> +#define VLV_EDP_PSR_INACTIVE (1<<0) >> +#define VLV_EDP_PSR_IN_TRANS_TO_ACTIVE (2<<0) >> +#define VLV_EDP_PSR_ACTIVE_NORFB_UP (3<<0) >> +#define VLV_EDP_PSR_ACTIVE_SF_UPDATE (4<<0) >> +#define VLV_EDP_PSR_EXIT (5<<0) >> +#define VLV_EDP_PSR_IN_TRANS (1<<7) >> +#define VLV_PSRSTAT(pipe) _PIPE(pipe, _PSRSTATA, _PSRSTATB) >> + >> /* HSW+ eDP PSR registers */ >> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800) >> #define EDP_PSR_CTL(dev) (EDP_PSR_BASE(dev) + 0) >> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c >> index ffcba21..6b31020 100644 >> --- a/drivers/gpu/drm/i915/i915_suspend.c >> +++ b/drivers/gpu/drm/i915/i915_suspend.c >> @@ -289,8 +289,8 @@ static void i915_restore_display(struct drm_device *dev) >> } >> >> /* Force a full PSR setup on resume */ >> + intel_edp_psr_inactivate(dev, false); >> dev_priv->psr.setup_done = false; >> - intel_edp_psr_update(dev); >> >> /* only restore FBC info on the platform that supports FBC*/ >> intel_disable_fbc(dev); >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 1a9aa19..92d6df4 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -4451,9 +4451,13 @@ static void intel_connector_check_state(struct intel_connector *connector) >> * consider. */ >> void intel_connector_dpms(struct drm_connector *connector, int mode) >> { >> + struct drm_device *dev = connector->dev; >> + >> /* All the simple cases only support two dpms states. */ >> - if (mode != DRM_MODE_DPMS_ON) >> + if (mode != DRM_MODE_DPMS_ON) { >> mode = DRM_MODE_DPMS_OFF; >> + intel_edp_psr_inactivate(dev, false); > > Maybe check if this connector even has PSR enabled? the psr_inactivate already does that by checking psr.setup_done and psr.active. > >> + } >> >> if (mode == connector->dpms) >> return; >> @@ -7501,6 +7505,8 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, >> u32 base = 0, pos = 0; >> bool visible; >> >> + intel_edp_psr_inactivate(dev, true); > > Don't need to inactivate unless the cursor is or was visible. so, below next retur after check visible and cursor_visible should be enough, right? > >> + >> if (on) >> base = intel_crtc->cursor_addr; >> >> @@ -8230,12 +8236,15 @@ void intel_mark_idle(struct drm_device *dev) >> gen6_rps_idle(dev->dev_private); >> } >> >> + >> void intel_mark_fb_busy(struct drm_i915_gem_object *obj, >> struct intel_ring_buffer *ring) >> { >> struct drm_device *dev = obj->base.dev; >> struct drm_crtc *crtc; >> >> + intel_edp_psr_inactivate(dev, true); >> + >> if (!i915.powersave) >> return; >> >> @@ -8336,6 +8345,10 @@ static void do_intel_finish_page_flip(struct drm_device *dev, >> queue_work(dev_priv->wq, &work->work); >> >> trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj); >> + >> + /* Get PSR back after set_domain inactivated it without rescheduling >> + * it back. */ >> + schedule_delayed_work(&dev_priv->psr.work, msecs_to_jiffies(5000)); >> } >> >> void intel_finish_page_flip(struct drm_device *dev, int pipe) >> @@ -8688,6 +8701,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, >> if (work == NULL) >> return -ENOMEM; >> >> + /* Inactivate PSR early in page flip */ >> + intel_edp_psr_inactivate(dev, true); >> + >> work->event = event; >> work->crtc = crtc; >> work->old_fb_obj = to_intel_framebuffer(old_fb)->obj; >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 80054bb..576c204 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -1578,21 +1578,35 @@ static void intel_dp_get_config(struct intel_encoder *encoder, >> } >> } >> >> -static bool is_edp_psr(struct drm_device *dev) >> +static bool is_edp_psr(struct intel_dp *intel_dp) >> +{ >> + return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED; >> +} >> + >> +static bool vlv_edp_is_psr_enabled_on_pipe(struct drm_device *dev, int pipe) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> + uint32_t val; >> >> - return dev_priv->psr.sink_support; >> + val = I915_READ(VLV_PSRSTAT(pipe)) & >> + VLV_EDP_PSR_CURR_STATE_MASK; >> + return (val == VLV_EDP_PSR_ACTIVE_NORFB_UP) || >> + (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE); >> } >> >> static bool intel_edp_is_psr_enabled(struct drm_device *dev) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> >> - if (!HAS_PSR(dev)) >> - return false; >> + if (HAS_PSR(dev)) { >> + if (IS_VALLEYVIEW(dev)) >> + return vlv_edp_is_psr_enabled_on_pipe(dev, PIPE_A) || >> + vlv_edp_is_psr_enabled_on_pipe(dev, PIPE_B); >> + else >> + return I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; >> + } >> >> - return I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; >> + return false; >> } >> >> static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp, >> @@ -1624,32 +1638,60 @@ static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp, >> POSTING_READ(ctl_reg); >> } >> >> +static void intel_edp_psr_work(struct work_struct *work) >> +{ >> + struct drm_i915_private *dev_priv = >> + container_of(work, typeof(*dev_priv), psr.work.work); >> + struct drm_device *dev = dev_priv->dev; >> + >> + intel_edp_psr_update(dev); >> +} >> + >> static void intel_edp_psr_setup(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 edp_vsc_psr psr_vsc; >> + uint32_t val; >> >> if (dev_priv->psr.setup_done) >> return; >> >> - mutex_init(&dev_priv->psr.lock); >> - >> - /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */ >> - memset(&psr_vsc, 0, sizeof(psr_vsc)); >> - psr_vsc.sdp_header.HB0 = 0; >> - psr_vsc.sdp_header.HB1 = 0x7; >> - psr_vsc.sdp_header.HB2 = 0x2; >> - psr_vsc.sdp_header.HB3 = 0x8; >> - intel_edp_psr_write_vsc(intel_dp, &psr_vsc); >> - >> - /* 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); >> + if (IS_VALLEYVIEW(dev)) { >> + val = I915_READ(VLV_VSCSDP(PIPE_A)); >> + val &= ~VLV_EDP_PSR_SDP_FREQ_MASK; >> + val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME; >> + I915_WRITE(VLV_VSCSDP(PIPE_A), val); >> + >> + val = I915_READ(VLV_VSCSDP(PIPE_B)); >> + val &= ~VLV_EDP_PSR_SDP_FREQ_MASK; >> + val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME; >> + I915_WRITE(VLV_VSCSDP(PIPE_B), val); >> + } else { >> + /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */ >> + memset(&psr_vsc, 0, sizeof(psr_vsc)); >> + psr_vsc.sdp_header.HB0 = 0; >> + psr_vsc.sdp_header.HB1 = 0x7; >> + psr_vsc.sdp_header.HB2 = 0x2; >> + psr_vsc.sdp_header.HB3 = 0x8; >> + intel_edp_psr_write_vsc(intel_dp, &psr_vsc); >> + >> + /* 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); >> + } >> >> dev_priv->psr.setup_done = true; >> } >> >> +static void vlv_edp_psr_enable_sink(struct intel_dp *intel_dp) >> +{ >> + /* Enable PSR in sink */ >> + intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, >> + DP_PSR_ENABLE); >> +} >> + >> static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp) >> { >> struct drm_device *dev = intel_dp_to_dev(intel_dp); >> @@ -1680,6 +1722,27 @@ 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 vlv_edp_psr_enable_source(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 intel_crtc *intel_crtc = >> + to_intel_crtc(intel_dig_port->base.base.crtc); >> + >> + uint32_t idle_frames = 1; >> + uint32_t val = 0; >> + >> + val |= VLV_EDP_PSR_ENABLE; >> + val &= ~VLV_EDP_PSR_MODE_MASK; >> + >> + val |= VLV_EDP_PSR_MODE_HW_TIMER; >> + val &= ~VLV_EDP_PSR_FRAME_COUNT_MASK; >> + val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; >> + >> + I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), val); >> +} >> + >> static void intel_edp_psr_enable_source(struct intel_dp *intel_dp) >> { >> struct drm_device *dev = intel_dp_to_dev(intel_dp); >> @@ -1721,8 +1784,8 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) >> return false; >> } >> >> - if ((intel_encoder->type != INTEL_OUTPUT_EDP) || >> - (dig_port->port != PORT_A)) { >> + if (HAS_DDI(dev) && ((intel_encoder->type != INTEL_OUTPUT_EDP) || >> + (dig_port->port != PORT_A))) { >> DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n"); >> return false; >> } >> @@ -1767,23 +1830,45 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) >> return false; >> } >> >> + /* Baytrail supports per-pipe PSR configuration, however PSR on >> + * PIPE_B isn't working properly. So let's keep it disabled for now. */ >> + if (IS_VALLEYVIEW(dev) && intel_crtc->pipe != PIPE_A) { >> + DRM_DEBUG_KMS("PSR on BYT isn't enabled on pipe B.\n"); >> + return false; >> + } >> + >> dev_priv->psr.source_ok = true; >> return true; >> } >> >> static void intel_edp_psr_do_enable(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); >> >> - if (!intel_edp_psr_match_conditions(intel_dp) || >> - intel_edp_is_psr_enabled(dev)) >> - return; >> + if (IS_VALLEYVIEW(dev)) { >> + if (vlv_edp_is_psr_enabled_on_pipe(dev, intel_crtc->pipe)) >> + return; >> + } else >> + if (intel_edp_is_psr_enabled(dev)) >> + return; >> >> /* Enable PSR on the panel */ >> - intel_edp_psr_enable_sink(intel_dp); >> + if (IS_VALLEYVIEW(dev)) >> + vlv_edp_psr_enable_sink(intel_dp); >> + else >> + intel_edp_psr_enable_sink(intel_dp); >> >> /* Enable PSR on the host */ >> - intel_edp_psr_enable_source(intel_dp); >> + if (IS_VALLEYVIEW(dev)) >> + vlv_edp_psr_enable_source(intel_dp); >> + else >> + intel_edp_psr_enable_source(intel_dp); >> + >> + dev_priv->psr.active = true; >> } >> >> void intel_edp_psr_enable(struct intel_dp *intel_dp) >> @@ -1791,16 +1876,43 @@ void intel_edp_psr_enable(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 (!is_edp_psr(intel_dp)) >> + return; >> + >> /* Setup PSR once */ >> intel_edp_psr_setup(intel_dp); >> >> mutex_lock(&dev_priv->psr.lock); >> - if (intel_edp_psr_match_conditions(intel_dp) && >> - !intel_edp_is_psr_enabled(dev)) >> + if (intel_edp_psr_match_conditions(intel_dp)) >> intel_edp_psr_do_enable(intel_dp); >> mutex_unlock(&dev_priv->psr.lock); >> } >> >> +void vlv_edp_psr_disable(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 intel_crtc *intel_crtc = >> + to_intel_crtc(intel_dig_port->base.base.crtc); >> + uint32_t val; >> + >> + if (!dev_priv->psr.setup_done) >> + return; >> + >> + intel_edp_psr_inactivate(dev, false); >> + >> + 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"); >> + >> + 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); >> +} >> + >> void intel_edp_psr_disable(struct intel_dp *intel_dp) >> { >> struct drm_device *dev = intel_dp_to_dev(intel_dp); >> @@ -1823,6 +1935,7 @@ void intel_edp_psr_update(struct drm_device *dev) >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct intel_encoder *encoder; >> struct intel_dp *intel_dp = NULL; >> + struct intel_crtc *intel_crtc; >> >> if (!dev_priv->psr.setup_done) >> return; >> @@ -1830,26 +1943,93 @@ void intel_edp_psr_update(struct drm_device *dev) >> list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) >> if (encoder->type == INTEL_OUTPUT_EDP) { >> intel_dp = enc_to_intel_dp(&encoder->base); >> - >> - if (!is_edp_psr(dev)) >> - return; >> + intel_crtc = to_intel_crtc(encoder->base.crtc); >> >> mutex_lock(&dev_priv->psr.lock); >> - if (!intel_edp_psr_match_conditions(intel_dp)) >> - intel_edp_psr_disable(intel_dp); >> - else >> + if (!intel_edp_psr_match_conditions(intel_dp)) { >> + if (IS_VALLEYVIEW(dev)) >> + vlv_edp_psr_disable(intel_dp); >> + else >> + intel_edp_psr_disable(intel_dp); >> + } else >> if (!intel_edp_is_psr_enabled(dev)) >> intel_edp_psr_do_enable(intel_dp); >> mutex_unlock(&dev_priv->psr.lock); >> } >> } >> >> +void intel_edp_psr_do_inactivate(struct drm_device *dev) >> +{ >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + struct intel_connector *connector; >> + struct intel_encoder *encoder; >> + struct intel_crtc *intel_crtc; >> + struct intel_dp *intel_dp = NULL; >> + >> + list_for_each_entry(connector, &dev->mode_config.connector_list, >> + base.head) { >> + >> + if (connector->base.dpms != DRM_MODE_DPMS_ON) >> + continue; >> + >> + encoder = to_intel_encoder(connector->base.encoder); >> + if (encoder->type == INTEL_OUTPUT_EDP) { >> + >> + intel_dp = enc_to_intel_dp(&encoder->base); >> + intel_crtc = to_intel_crtc(encoder->base.crtc); > > Still digging through the encoder and crtc pointers w/o holding any > modeset locks. Feels dangerous. intel_edp_psr_match_conditions() does > the same kind of stuff, and that too now gets called outside modeset > codepaths. I don't see an easy way to solve this with current sturcture here since we need the pipe and intel_dp pointer. Maybe on the future rework putiing psr inside pipe_config and in a scruct with all needed info without require to interate anywhere. But on the current structure I don't see an easy solution, do you? >> + >> + if (!vlv_edp_is_psr_enabled_on_pipe(dev, >> + intel_crtc->pipe)) >> + continue; >> + >> + dev_priv->psr.active = false; >> + >> + I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), >> + VLV_EDP_PSR_RESET); >> + /* WaClearPSRReset:vlv */ >> + I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), 0); >> + >> + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); >> + } >> + } >> +} >> + >> +void intel_edp_psr_inactivate(struct drm_device *dev, bool schedule_back) >> +{ >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + >> + if (!IS_VALLEYVIEW(dev)) >> + return; >> + >> + if (!dev_priv->psr.setup_done) >> + return; >> + >> + /* If it was requested to not turn psr back delayed work must be >> + * canceled even if it is already on inactivated state. */ >> + if (!schedule_back) >> + cancel_delayed_work_sync(&dev_priv->psr.work); >> + >> + if (!dev_priv->psr.active) >> + return; > > What if psr gets re-activated just after you checked above? Shouldn't be dangerus... psr.activate idea is to minimize I915_WRITE, mainly when calling busy ioctls in sequence... But I'm open to suggestions. > >> + >> + cancel_delayed_work_sync(&dev_priv->psr.work); >> + >> + intel_edp_psr_do_inactivate(dev); >> + >> + if (schedule_back) >> + schedule_delayed_work(&dev_priv->psr.work, >> + msecs_to_jiffies(5000)); >> +} >> + >> static void intel_disable_dp(struct intel_encoder *encoder) >> { >> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); >> enum port port = dp_to_dig_port(intel_dp)->port; >> struct drm_device *dev = encoder->base.dev; >> >> + if (IS_VALLEYVIEW(dev)) >> + vlv_edp_psr_disable(intel_dp); >> + >> /* Make sure the panel is off before trying to change the mode. But also >> * ensure that we have vdd while we switch off the panel. */ >> intel_edp_backlight_off(intel_dp); >> @@ -1906,6 +2086,7 @@ static void vlv_enable_dp(struct intel_encoder *encoder) >> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); >> >> intel_edp_backlight_on(intel_dp); >> + intel_edp_psr_enable(intel_dp); >> } >> >> static void g4x_pre_enable_dp(struct intel_encoder *encoder) >> @@ -3846,8 +4027,11 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, >> BUG(); >> } >> >> - if (is_edp(intel_dp)) >> + if (is_edp(intel_dp)) { >> intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq); >> + INIT_DELAYED_WORK(&dev_priv->psr.work, intel_edp_psr_work); >> + mutex_init(&dev_priv->psr.lock); > > Initializing global state from a per-connector init function seems a bit > weird. I thought it as well, but didn't find another place. My first attempt was at psr_setup, but it is called again after suspend/resume and it was breaking things up. suggestions? > >> + } >> >> error = intel_dp_i2c_init(intel_dp, intel_connector, name); >> WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n", >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 71c1371..d8103f9 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -748,6 +748,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp); >> void intel_edp_psr_enable(struct intel_dp *intel_dp); >> void intel_edp_psr_disable(struct intel_dp *intel_dp); >> void intel_edp_psr_update(struct drm_device *dev); >> +void intel_edp_psr_inactivate(struct drm_device *dev, bool schedule_back); >> >> >> /* intel_dsi.c */ >> -- >> 1.8.1.2 > > -- > Ville Syrjälä > Intel OTC Hi Ville, thank you very much for your comments. Coding is improving a lot along with stability here. Please help me to get this structure merged than we rework all psr later to avoid that loop over connectors and encoders.... Thanks,
On Fri, Feb 07, 2014 at 04:05:26PM -0200, Rodrigo Vivi wrote: > On Fri, Feb 7, 2014 at 3:24 PM, Ville Syrjälä > <ville.syrjala@linux.intel.com> wrote: > > On Wed, Feb 05, 2014 at 05:04:31PM -0200, Rodrigo Vivi wrote: > >> This patch adds PSR Support to Baytrail. > >> > >> Baytrail cannot easily detect screen updates and force PSR exit. > >> So we inactivate it on {busy_ioctl, set_domain, sw_finish and mark_busy} > >> and update to enable it back on next display mark_idle. > >> > >> v2: Also inactivate PSR on cursor update. > >> v3: Inactivate PSR on mark_busy, dset_domain and sw_finish_ioctl, and > >> early on page flip besides avoid initializing inactive/active flag > >> more than once. > >> v4: Fix identation issues. > >> v5: Rebase and add Baytrail per pipe support although leaving PIPE_B > >> support disabled by for now since it isn't working properly yet. > >> v6: Removing forgotten comment and useless clkgating definition. > >> v7: Remove inactivate from set_domain. Chris warned this was semanticaly > >> wrong. > >> v8: Accept Ville's suggestions: Use register's names matching spec and > >> warn if transition took longer than it should. > >> v9: New version with delayed work to get PSR back. Disabling it on > >> set_domain but not rescheduing it back until next finish_page_flip. > >> > >> Cc: Chris Wilson <chris@chris-wilson.co.uk> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> > >> --- > >> drivers/gpu/drm/i915/i915_debugfs.c | 36 ++++- > >> drivers/gpu/drm/i915/i915_drv.h | 5 +- > >> drivers/gpu/drm/i915/i915_gem.c | 12 ++ > >> drivers/gpu/drm/i915/i915_reg.h | 37 +++++ > >> drivers/gpu/drm/i915/i915_suspend.c | 2 +- > >> drivers/gpu/drm/i915/intel_display.c | 18 ++- > >> drivers/gpu/drm/i915/intel_dp.c | 256 ++++++++++++++++++++++++++++++----- > >> drivers/gpu/drm/i915/intel_drv.h | 1 + > >> 8 files changed, 323 insertions(+), 44 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > >> index bc8707f..2949c48 100644 > >> --- a/drivers/gpu/drm/i915/i915_debugfs.c > >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c > >> @@ -1900,6 +1900,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) > >> struct drm_device *dev = node->minor->dev; > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> u32 psrperf = 0; > >> + u32 statA = 0; > >> + u32 statB = 0; > >> bool enabled = false; > >> > >> intel_runtime_pm_get(dev_priv); > >> @@ -1907,14 +1909,38 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) > >> seq_printf(m, "Sink_Support: %s\n", yesno(dev_priv->psr.sink_support)); > >> seq_printf(m, "Source_OK: %s\n", yesno(dev_priv->psr.source_ok)); > >> > >> - enabled = HAS_PSR(dev) && > >> - I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; > >> - seq_printf(m, "Enabled: %s\n", yesno(enabled)); > >> + if (HAS_PSR(dev)) { > >> + if (IS_VALLEYVIEW(dev)) { > >> + statA = I915_READ(VLV_PSRSTAT(PIPE_A)) & > >> + VLV_EDP_PSR_CURR_STATE_MASK; > >> + statB = I915_READ(VLV_PSRSTAT(PIPE_B)) & > >> + VLV_EDP_PSR_CURR_STATE_MASK; > >> + enabled = ((statA == VLV_EDP_PSR_ACTIVE_NORFB_UP) || > >> + (statA == VLV_EDP_PSR_ACTIVE_SF_UPDATE) || > >> + (statB == VLV_EDP_PSR_ACTIVE_NORFB_UP) || > >> + (statB == VLV_EDP_PSR_ACTIVE_SF_UPDATE)); > >> + } else > >> + enabled = I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; > >> + } > >> + seq_printf(m, "Enabled: %s", yesno(enabled)); > >> > >> - if (HAS_PSR(dev)) > >> + if (IS_VALLEYVIEW(dev)) { > >> + if ((statA == VLV_EDP_PSR_ACTIVE_NORFB_UP) || > >> + (statA == VLV_EDP_PSR_ACTIVE_SF_UPDATE)) > >> + seq_puts(m, " pipe A"); > >> + if ((statB == VLV_EDP_PSR_ACTIVE_NORFB_UP) || > >> + (statB == VLV_EDP_PSR_ACTIVE_SF_UPDATE)) > >> + seq_puts(m, " pipe B"); > >> + } > >> + > >> + seq_puts(m, "\n"); > >> + > >> + /* VLV PSR has no kind of performance counter */ > >> + if (HAS_PSR(dev) && !IS_VALLEYVIEW(dev)) { > >> psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) & > >> EDP_PSR_PERF_CNT_MASK; > >> - seq_printf(m, "Performance_Counter: %u\n", psrperf); > >> + seq_printf(m, "Performance_Counter: %u\n", psrperf); > >> + } > >> > >> intel_runtime_pm_put(dev_priv); > >> return 0; > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >> index 21470be..87c346a 100644 > >> --- a/drivers/gpu/drm/i915/i915_drv.h > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > >> @@ -747,7 +747,9 @@ struct i915_psr { > >> bool sink_support; > >> bool source_ok; > >> bool setup_done; > >> + bool active; > >> struct mutex lock; > >> + struct delayed_work work; > >> }; > >> > >> enum intel_pch { > >> @@ -1867,7 +1869,8 @@ struct drm_i915_file_private { > >> > >> #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi) > >> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg) > >> -#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev)) > >> +#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \ > >> + IS_VALLEYVIEW(dev)) > >> #define HAS_PC8(dev) (IS_HASWELL(dev)) /* XXX HSW:ULX */ > >> #define HAS_RUNTIME_PM(dev) (IS_HASWELL(dev)) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > >> index 39770f7..5ef1380 100644 > >> --- a/drivers/gpu/drm/i915/i915_gem.c > >> +++ b/drivers/gpu/drm/i915/i915_gem.c > >> @@ -1256,6 +1256,14 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, > >> goto unlock; > >> } > >> > >> + /* Here we don't reschedule work to get PSR back because userspace > >> + * can set domain and take as much as time it wants to write it. > >> + * We only reschedule it back on next finish_page_flip. There is the > >> + * risk of PSR be inactive forever depending on how compositor works, > >> + * but this is the safest way to avoid loosing screen updates. > >> + */ > >> + intel_edp_psr_inactivate(dev, false); > > > > This will still block until PSR is deactivated (aux communication and > > all). That still seems like a bad idea to do while holding struct_mutex. > > So, what do you suggest here? Same as before. The disabling could be done from a workqueue. > > > > >> + > >> /* Try to flush the object off the GPU without holding the lock. > >> * We will repeat the flush holding the lock in the normal manner > >> * to catch cases where we are gazumped. > >> @@ -1299,6 +1307,8 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data, > >> if (ret) > >> return ret; > >> > >> + intel_edp_psr_inactivate(dev, true); > >> + > >> obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); > >> if (&obj->base == NULL) { > >> ret = -ENOENT; > >> @@ -4059,6 +4069,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, > >> if (ret) > >> return ret; > >> > >> + intel_edp_psr_inactivate(dev, true); > >> + > >> obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); > >> if (&obj->base == NULL) { > >> ret = -ENOENT; > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > >> index cbbaf26..a5815d0 100644 > >> --- a/drivers/gpu/drm/i915/i915_reg.h > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > >> @@ -1968,6 +1968,43 @@ > >> #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B) > >> #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B) > >> > >> +/* VLV eDP PSR registers */ > >> +#define _PSRCTLA (VLV_DISPLAY_BASE + 0x60090) > >> +#define _PSRCTLB (VLV_DISPLAY_BASE + 0x61090) > >> +#define VLV_EDP_PSR_ENABLE (1<<0) > >> +#define VLV_EDP_PSR_RESET (1<<1) > >> +#define VLV_EDP_PSR_MODE_MASK (7<<2) > >> +#define VLV_EDP_PSR_MODE_HW_TIMER (1<<3) > >> +#define VLV_EDP_PSR_MODE_SW_TIMER (1<<2) > >> +#define VLV_EDP_PSR_SINGLE_FRAME_UPDATE (1<<7) > >> +#define VLV_EDP_PSR_ACTIVE_ENTRY (1<<8) > >> +#define VLV_EDP_PSR_SRC_TRANSMITTER_STATE (1<<9) > >> +#define VLV_EDP_PSR_DBL_FRAME (1<<10) > >> +#define VLV_EDP_PSR_FRAME_COUNT_MASK (0xff<<16) > >> +#define VLV_EDP_PSR_IDLE_FRAME_SHIFT 16 > >> +#define VLV_EDP_PSR_INT_TRANSITION (1<<24) > >> +#define VLV_PSRCTL(pipe) _PIPE(pipe, _PSRCTLA, _PSRCTLB) > >> + > >> +#define _VSCSDPA (VLV_DISPLAY_BASE + 0x600a0) > >> +#define _VSCSDPB (VLV_DISPLAY_BASE + 0x610a0) > >> +#define VLV_EDP_PSR_SDP_FREQ_MASK (3<<30) > >> +#define VLV_EDP_PSR_SDP_FREQ_ONCE (1<<31) > >> +#define VLV_EDP_PSR_SDP_FREQ_EVFRAME (1<<30) > >> +#define VLV_VSCSDP(pipe) _PIPE(pipe, _VSCSDPA, _VSCSDPB) > >> + > >> +#define _PSRSTATA (VLV_DISPLAY_BASE + 0x60094) > >> +#define _PSRSTATB (VLV_DISPLAY_BASE + 0x61094) > >> +#define VLV_EDP_PSR_LAST_STATE_MASK (7<<3) > >> +#define VLV_EDP_PSR_CURR_STATE_MASK 7 > >> +#define VLV_EDP_PSR_DISABLED (0<<0) > >> +#define VLV_EDP_PSR_INACTIVE (1<<0) > >> +#define VLV_EDP_PSR_IN_TRANS_TO_ACTIVE (2<<0) > >> +#define VLV_EDP_PSR_ACTIVE_NORFB_UP (3<<0) > >> +#define VLV_EDP_PSR_ACTIVE_SF_UPDATE (4<<0) > >> +#define VLV_EDP_PSR_EXIT (5<<0) > >> +#define VLV_EDP_PSR_IN_TRANS (1<<7) > >> +#define VLV_PSRSTAT(pipe) _PIPE(pipe, _PSRSTATA, _PSRSTATB) > >> + > >> /* HSW+ eDP PSR registers */ > >> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800) > >> #define EDP_PSR_CTL(dev) (EDP_PSR_BASE(dev) + 0) > >> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c > >> index ffcba21..6b31020 100644 > >> --- a/drivers/gpu/drm/i915/i915_suspend.c > >> +++ b/drivers/gpu/drm/i915/i915_suspend.c > >> @@ -289,8 +289,8 @@ static void i915_restore_display(struct drm_device *dev) > >> } > >> > >> /* Force a full PSR setup on resume */ > >> + intel_edp_psr_inactivate(dev, false); > >> dev_priv->psr.setup_done = false; > >> - intel_edp_psr_update(dev); > >> > >> /* only restore FBC info on the platform that supports FBC*/ > >> intel_disable_fbc(dev); > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >> index 1a9aa19..92d6df4 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -4451,9 +4451,13 @@ static void intel_connector_check_state(struct intel_connector *connector) > >> * consider. */ > >> void intel_connector_dpms(struct drm_connector *connector, int mode) > >> { > >> + struct drm_device *dev = connector->dev; > >> + > >> /* All the simple cases only support two dpms states. */ > >> - if (mode != DRM_MODE_DPMS_ON) > >> + if (mode != DRM_MODE_DPMS_ON) { > >> mode = DRM_MODE_DPMS_OFF; > >> + intel_edp_psr_inactivate(dev, false); > > > > Maybe check if this connector even has PSR enabled? > > the psr_inactivate already does that by checking psr.setup_done and psr.active. I meant that it will now disable PSR for all displays even if the DPMS mode was changed for some other display. Well, I guess it's somewhat rare to only do DPMS for a subset of active displays. > > > > >> + } > >> > >> if (mode == connector->dpms) > >> return; > >> @@ -7501,6 +7505,8 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, > >> u32 base = 0, pos = 0; > >> bool visible; > >> > >> + intel_edp_psr_inactivate(dev, true); > > > > Don't need to inactivate unless the cursor is or was visible. > > so, below next retur after check visible and cursor_visible should be > enough, right? Yeah that should be fine. > > > >> + > >> if (on) > >> base = intel_crtc->cursor_addr; > >> > >> @@ -8230,12 +8236,15 @@ void intel_mark_idle(struct drm_device *dev) > >> gen6_rps_idle(dev->dev_private); > >> } > >> > >> + > >> void intel_mark_fb_busy(struct drm_i915_gem_object *obj, > >> struct intel_ring_buffer *ring) > >> { > >> struct drm_device *dev = obj->base.dev; > >> struct drm_crtc *crtc; > >> > >> + intel_edp_psr_inactivate(dev, true); > >> + > >> if (!i915.powersave) > >> return; > >> > >> @@ -8336,6 +8345,10 @@ static void do_intel_finish_page_flip(struct drm_device *dev, > >> queue_work(dev_priv->wq, &work->work); > >> > >> trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj); > >> + > >> + /* Get PSR back after set_domain inactivated it without rescheduling > >> + * it back. */ > >> + schedule_delayed_work(&dev_priv->psr.work, msecs_to_jiffies(5000)); > >> } > >> > >> void intel_finish_page_flip(struct drm_device *dev, int pipe) > >> @@ -8688,6 +8701,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > >> if (work == NULL) > >> return -ENOMEM; > >> > >> + /* Inactivate PSR early in page flip */ > >> + intel_edp_psr_inactivate(dev, true); > >> + > >> work->event = event; > >> work->crtc = crtc; > >> work->old_fb_obj = to_intel_framebuffer(old_fb)->obj; > >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > >> index 80054bb..576c204 100644 > >> --- a/drivers/gpu/drm/i915/intel_dp.c > >> +++ b/drivers/gpu/drm/i915/intel_dp.c > >> @@ -1578,21 +1578,35 @@ static void intel_dp_get_config(struct intel_encoder *encoder, > >> } > >> } > >> > >> -static bool is_edp_psr(struct drm_device *dev) > >> +static bool is_edp_psr(struct intel_dp *intel_dp) > >> +{ > >> + return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED; > >> +} > >> + > >> +static bool vlv_edp_is_psr_enabled_on_pipe(struct drm_device *dev, int pipe) > >> { > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> + uint32_t val; > >> > >> - return dev_priv->psr.sink_support; > >> + val = I915_READ(VLV_PSRSTAT(pipe)) & > >> + VLV_EDP_PSR_CURR_STATE_MASK; > >> + return (val == VLV_EDP_PSR_ACTIVE_NORFB_UP) || > >> + (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE); > >> } > >> > >> static bool intel_edp_is_psr_enabled(struct drm_device *dev) > >> { > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> > >> - if (!HAS_PSR(dev)) > >> - return false; > >> + if (HAS_PSR(dev)) { > >> + if (IS_VALLEYVIEW(dev)) > >> + return vlv_edp_is_psr_enabled_on_pipe(dev, PIPE_A) || > >> + vlv_edp_is_psr_enabled_on_pipe(dev, PIPE_B); > >> + else > >> + return I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; > >> + } > >> > >> - return I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; > >> + return false; > >> } > >> > >> static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp, > >> @@ -1624,32 +1638,60 @@ static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp, > >> POSTING_READ(ctl_reg); > >> } > >> > >> +static void intel_edp_psr_work(struct work_struct *work) > >> +{ > >> + struct drm_i915_private *dev_priv = > >> + container_of(work, typeof(*dev_priv), psr.work.work); > >> + struct drm_device *dev = dev_priv->dev; > >> + > >> + intel_edp_psr_update(dev); > >> +} > >> + > >> static void intel_edp_psr_setup(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 edp_vsc_psr psr_vsc; > >> + uint32_t val; > >> > >> if (dev_priv->psr.setup_done) > >> return; > >> > >> - mutex_init(&dev_priv->psr.lock); > >> - > >> - /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */ > >> - memset(&psr_vsc, 0, sizeof(psr_vsc)); > >> - psr_vsc.sdp_header.HB0 = 0; > >> - psr_vsc.sdp_header.HB1 = 0x7; > >> - psr_vsc.sdp_header.HB2 = 0x2; > >> - psr_vsc.sdp_header.HB3 = 0x8; > >> - intel_edp_psr_write_vsc(intel_dp, &psr_vsc); > >> - > >> - /* 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); > >> + if (IS_VALLEYVIEW(dev)) { > >> + val = I915_READ(VLV_VSCSDP(PIPE_A)); > >> + val &= ~VLV_EDP_PSR_SDP_FREQ_MASK; > >> + val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME; > >> + I915_WRITE(VLV_VSCSDP(PIPE_A), val); > >> + > >> + val = I915_READ(VLV_VSCSDP(PIPE_B)); > >> + val &= ~VLV_EDP_PSR_SDP_FREQ_MASK; > >> + val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME; > >> + I915_WRITE(VLV_VSCSDP(PIPE_B), val); > >> + } else { > >> + /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */ > >> + memset(&psr_vsc, 0, sizeof(psr_vsc)); > >> + psr_vsc.sdp_header.HB0 = 0; > >> + psr_vsc.sdp_header.HB1 = 0x7; > >> + psr_vsc.sdp_header.HB2 = 0x2; > >> + psr_vsc.sdp_header.HB3 = 0x8; > >> + intel_edp_psr_write_vsc(intel_dp, &psr_vsc); > >> + > >> + /* 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); > >> + } > >> > >> dev_priv->psr.setup_done = true; > >> } > >> > >> +static void vlv_edp_psr_enable_sink(struct intel_dp *intel_dp) > >> +{ > >> + /* Enable PSR in sink */ > >> + intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, > >> + DP_PSR_ENABLE); > >> +} > >> + > >> static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp) > >> { > >> struct drm_device *dev = intel_dp_to_dev(intel_dp); > >> @@ -1680,6 +1722,27 @@ 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 vlv_edp_psr_enable_source(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 intel_crtc *intel_crtc = > >> + to_intel_crtc(intel_dig_port->base.base.crtc); > >> + > >> + uint32_t idle_frames = 1; > >> + uint32_t val = 0; > >> + > >> + val |= VLV_EDP_PSR_ENABLE; > >> + val &= ~VLV_EDP_PSR_MODE_MASK; > >> + > >> + val |= VLV_EDP_PSR_MODE_HW_TIMER; > >> + val &= ~VLV_EDP_PSR_FRAME_COUNT_MASK; > >> + val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; > >> + > >> + I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), val); > >> +} > >> + > >> static void intel_edp_psr_enable_source(struct intel_dp *intel_dp) > >> { > >> struct drm_device *dev = intel_dp_to_dev(intel_dp); > >> @@ -1721,8 +1784,8 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) > >> return false; > >> } > >> > >> - if ((intel_encoder->type != INTEL_OUTPUT_EDP) || > >> - (dig_port->port != PORT_A)) { > >> + if (HAS_DDI(dev) && ((intel_encoder->type != INTEL_OUTPUT_EDP) || > >> + (dig_port->port != PORT_A))) { > >> DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n"); > >> return false; > >> } > >> @@ -1767,23 +1830,45 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) > >> return false; > >> } > >> > >> + /* Baytrail supports per-pipe PSR configuration, however PSR on > >> + * PIPE_B isn't working properly. So let's keep it disabled for now. */ > >> + if (IS_VALLEYVIEW(dev) && intel_crtc->pipe != PIPE_A) { > >> + DRM_DEBUG_KMS("PSR on BYT isn't enabled on pipe B.\n"); > >> + return false; > >> + } > >> + > >> dev_priv->psr.source_ok = true; > >> return true; > >> } > >> > >> static void intel_edp_psr_do_enable(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); > >> > >> - if (!intel_edp_psr_match_conditions(intel_dp) || > >> - intel_edp_is_psr_enabled(dev)) > >> - return; > >> + if (IS_VALLEYVIEW(dev)) { > >> + if (vlv_edp_is_psr_enabled_on_pipe(dev, intel_crtc->pipe)) > >> + return; > >> + } else > >> + if (intel_edp_is_psr_enabled(dev)) > >> + return; > >> > >> /* Enable PSR on the panel */ > >> - intel_edp_psr_enable_sink(intel_dp); > >> + if (IS_VALLEYVIEW(dev)) > >> + vlv_edp_psr_enable_sink(intel_dp); > >> + else > >> + intel_edp_psr_enable_sink(intel_dp); > >> > >> /* Enable PSR on the host */ > >> - intel_edp_psr_enable_source(intel_dp); > >> + if (IS_VALLEYVIEW(dev)) > >> + vlv_edp_psr_enable_source(intel_dp); > >> + else > >> + intel_edp_psr_enable_source(intel_dp); > >> + > >> + dev_priv->psr.active = true; > >> } > >> > >> void intel_edp_psr_enable(struct intel_dp *intel_dp) > >> @@ -1791,16 +1876,43 @@ void intel_edp_psr_enable(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 (!is_edp_psr(intel_dp)) > >> + return; > >> + > >> /* Setup PSR once */ > >> intel_edp_psr_setup(intel_dp); > >> > >> mutex_lock(&dev_priv->psr.lock); > >> - if (intel_edp_psr_match_conditions(intel_dp) && > >> - !intel_edp_is_psr_enabled(dev)) > >> + if (intel_edp_psr_match_conditions(intel_dp)) > >> intel_edp_psr_do_enable(intel_dp); > >> mutex_unlock(&dev_priv->psr.lock); > >> } > >> > >> +void vlv_edp_psr_disable(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 intel_crtc *intel_crtc = > >> + to_intel_crtc(intel_dig_port->base.base.crtc); > >> + uint32_t val; > >> + > >> + if (!dev_priv->psr.setup_done) > >> + return; > >> + > >> + intel_edp_psr_inactivate(dev, false); > >> + > >> + 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"); > >> + > >> + 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); > >> +} > >> + > >> void intel_edp_psr_disable(struct intel_dp *intel_dp) > >> { > >> struct drm_device *dev = intel_dp_to_dev(intel_dp); > >> @@ -1823,6 +1935,7 @@ void intel_edp_psr_update(struct drm_device *dev) > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> struct intel_encoder *encoder; > >> struct intel_dp *intel_dp = NULL; > >> + struct intel_crtc *intel_crtc; > >> > >> if (!dev_priv->psr.setup_done) > >> return; > >> @@ -1830,26 +1943,93 @@ void intel_edp_psr_update(struct drm_device *dev) > >> list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) > >> if (encoder->type == INTEL_OUTPUT_EDP) { > >> intel_dp = enc_to_intel_dp(&encoder->base); > >> - > >> - if (!is_edp_psr(dev)) > >> - return; > >> + intel_crtc = to_intel_crtc(encoder->base.crtc); > >> > >> mutex_lock(&dev_priv->psr.lock); > >> - if (!intel_edp_psr_match_conditions(intel_dp)) > >> - intel_edp_psr_disable(intel_dp); > >> - else > >> + if (!intel_edp_psr_match_conditions(intel_dp)) { > >> + if (IS_VALLEYVIEW(dev)) > >> + vlv_edp_psr_disable(intel_dp); > >> + else > >> + intel_edp_psr_disable(intel_dp); > >> + } else > >> if (!intel_edp_is_psr_enabled(dev)) > >> intel_edp_psr_do_enable(intel_dp); > >> mutex_unlock(&dev_priv->psr.lock); > >> } > >> } > >> > >> +void intel_edp_psr_do_inactivate(struct drm_device *dev) > >> +{ > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + struct intel_connector *connector; > >> + struct intel_encoder *encoder; > >> + struct intel_crtc *intel_crtc; > >> + struct intel_dp *intel_dp = NULL; > >> + > >> + list_for_each_entry(connector, &dev->mode_config.connector_list, > >> + base.head) { > >> + > >> + if (connector->base.dpms != DRM_MODE_DPMS_ON) > >> + continue; > >> + > >> + encoder = to_intel_encoder(connector->base.encoder); > >> + if (encoder->type == INTEL_OUTPUT_EDP) { > >> + > >> + intel_dp = enc_to_intel_dp(&encoder->base); > >> + intel_crtc = to_intel_crtc(encoder->base.crtc); > > > > Still digging through the encoder and crtc pointers w/o holding any > > modeset locks. Feels dangerous. intel_edp_psr_match_conditions() does > > the same kind of stuff, and that too now gets called outside modeset > > codepaths. > > I don't see an easy way to solve this with current sturcture here > since we need the pipe and intel_dp pointer. > Maybe on the future rework putiing psr inside pipe_config and in a > scruct with all needed info without require to interate anywhere. > But on the current structure I don't see an easy solution, do you? Well, if it was done from a workqueue anyway, it could just grab all modeset locks. Would just need to make sure we wouldn't cause deadlocks by doing that. > > >> + > >> + if (!vlv_edp_is_psr_enabled_on_pipe(dev, > >> + intel_crtc->pipe)) > >> + continue; > >> + > >> + dev_priv->psr.active = false; > >> + > >> + I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), > >> + VLV_EDP_PSR_RESET); > >> + /* WaClearPSRReset:vlv */ > >> + I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), 0); > >> + > >> + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > >> + } > >> + } > >> +} > >> + > >> +void intel_edp_psr_inactivate(struct drm_device *dev, bool schedule_back) > >> +{ > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + > >> + if (!IS_VALLEYVIEW(dev)) > >> + return; > >> + > >> + if (!dev_priv->psr.setup_done) > >> + return; > >> + > >> + /* If it was requested to not turn psr back delayed work must be > >> + * canceled even if it is already on inactivated state. */ > >> + if (!schedule_back) > >> + cancel_delayed_work_sync(&dev_priv->psr.work); > >> + > >> + if (!dev_priv->psr.active) > >> + return; > > > > What if psr gets re-activated just after you checked above? > > Shouldn't be dangerus... psr.activate idea is to minimize I915_WRITE, > mainly when calling busy ioctls in sequence... > But I'm open to suggestions. I think if this did happen and there are no more updates to the screen for a while, this would leave PSR enabled, with potentially stale screen contents. > > > > >> + > >> + cancel_delayed_work_sync(&dev_priv->psr.work); > >> + > >> + intel_edp_psr_do_inactivate(dev); > >> + > >> + if (schedule_back) > >> + schedule_delayed_work(&dev_priv->psr.work, > >> + msecs_to_jiffies(5000)); > >> +} > >> + > >> static void intel_disable_dp(struct intel_encoder *encoder) > >> { > >> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > >> enum port port = dp_to_dig_port(intel_dp)->port; > >> struct drm_device *dev = encoder->base.dev; > >> > >> + if (IS_VALLEYVIEW(dev)) > >> + vlv_edp_psr_disable(intel_dp); > >> + > >> /* Make sure the panel is off before trying to change the mode. But also > >> * ensure that we have vdd while we switch off the panel. */ > >> intel_edp_backlight_off(intel_dp); > >> @@ -1906,6 +2086,7 @@ static void vlv_enable_dp(struct intel_encoder *encoder) > >> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > >> > >> intel_edp_backlight_on(intel_dp); > >> + intel_edp_psr_enable(intel_dp); > >> } > >> > >> static void g4x_pre_enable_dp(struct intel_encoder *encoder) > >> @@ -3846,8 +4027,11 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > >> BUG(); > >> } > >> > >> - if (is_edp(intel_dp)) > >> + if (is_edp(intel_dp)) { > >> intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq); > >> + INIT_DELAYED_WORK(&dev_priv->psr.work, intel_edp_psr_work); > >> + mutex_init(&dev_priv->psr.lock); > > > > Initializing global state from a per-connector init function seems a bit > > weird. > > I thought it as well, but didn't find another place. > My first attempt was at psr_setup, but it is called again after > suspend/resume and it was breaking things up. > suggestions? Some modeset init function sounds like the right place. > > > > >> + } > >> > >> error = intel_dp_i2c_init(intel_dp, intel_connector, name); > >> WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n", > >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > >> index 71c1371..d8103f9 100644 > >> --- a/drivers/gpu/drm/i915/intel_drv.h > >> +++ b/drivers/gpu/drm/i915/intel_drv.h > >> @@ -748,6 +748,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp); > >> void intel_edp_psr_enable(struct intel_dp *intel_dp); > >> void intel_edp_psr_disable(struct intel_dp *intel_dp); > >> void intel_edp_psr_update(struct drm_device *dev); > >> +void intel_edp_psr_inactivate(struct drm_device *dev, bool schedule_back); > >> > >> > >> /* intel_dsi.c */ > >> -- > >> 1.8.1.2 > > > > -- > > Ville Syrjälä > > Intel OTC > > Hi Ville, thank you very much for your comments. Coding is improving a > lot along with stability here. > Please help me to get this structure merged than we rework all psr > later to avoid that loop over connectors and encoders.... > > Thanks, > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index bc8707f..2949c48 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1900,6 +1900,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) struct drm_device *dev = node->minor->dev; struct drm_i915_private *dev_priv = dev->dev_private; u32 psrperf = 0; + u32 statA = 0; + u32 statB = 0; bool enabled = false; intel_runtime_pm_get(dev_priv); @@ -1907,14 +1909,38 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) seq_printf(m, "Sink_Support: %s\n", yesno(dev_priv->psr.sink_support)); seq_printf(m, "Source_OK: %s\n", yesno(dev_priv->psr.source_ok)); - enabled = HAS_PSR(dev) && - I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; - seq_printf(m, "Enabled: %s\n", yesno(enabled)); + if (HAS_PSR(dev)) { + if (IS_VALLEYVIEW(dev)) { + statA = I915_READ(VLV_PSRSTAT(PIPE_A)) & + VLV_EDP_PSR_CURR_STATE_MASK; + statB = I915_READ(VLV_PSRSTAT(PIPE_B)) & + VLV_EDP_PSR_CURR_STATE_MASK; + enabled = ((statA == VLV_EDP_PSR_ACTIVE_NORFB_UP) || + (statA == VLV_EDP_PSR_ACTIVE_SF_UPDATE) || + (statB == VLV_EDP_PSR_ACTIVE_NORFB_UP) || + (statB == VLV_EDP_PSR_ACTIVE_SF_UPDATE)); + } else + enabled = I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; + } + seq_printf(m, "Enabled: %s", yesno(enabled)); - if (HAS_PSR(dev)) + if (IS_VALLEYVIEW(dev)) { + if ((statA == VLV_EDP_PSR_ACTIVE_NORFB_UP) || + (statA == VLV_EDP_PSR_ACTIVE_SF_UPDATE)) + seq_puts(m, " pipe A"); + if ((statB == VLV_EDP_PSR_ACTIVE_NORFB_UP) || + (statB == VLV_EDP_PSR_ACTIVE_SF_UPDATE)) + seq_puts(m, " pipe B"); + } + + seq_puts(m, "\n"); + + /* VLV PSR has no kind of performance counter */ + if (HAS_PSR(dev) && !IS_VALLEYVIEW(dev)) { psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) & EDP_PSR_PERF_CNT_MASK; - seq_printf(m, "Performance_Counter: %u\n", psrperf); + seq_printf(m, "Performance_Counter: %u\n", psrperf); + } intel_runtime_pm_put(dev_priv); return 0; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 21470be..87c346a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -747,7 +747,9 @@ struct i915_psr { bool sink_support; bool source_ok; bool setup_done; + bool active; struct mutex lock; + struct delayed_work work; }; enum intel_pch { @@ -1867,7 +1869,8 @@ struct drm_i915_file_private { #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi) #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg) -#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev)) +#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \ + IS_VALLEYVIEW(dev)) #define HAS_PC8(dev) (IS_HASWELL(dev)) /* XXX HSW:ULX */ #define HAS_RUNTIME_PM(dev) (IS_HASWELL(dev)) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 39770f7..5ef1380 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1256,6 +1256,14 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, goto unlock; } + /* Here we don't reschedule work to get PSR back because userspace + * can set domain and take as much as time it wants to write it. + * We only reschedule it back on next finish_page_flip. There is the + * risk of PSR be inactive forever depending on how compositor works, + * but this is the safest way to avoid loosing screen updates. + */ + intel_edp_psr_inactivate(dev, false); + /* Try to flush the object off the GPU without holding the lock. * We will repeat the flush holding the lock in the normal manner * to catch cases where we are gazumped. @@ -1299,6 +1307,8 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data, if (ret) return ret; + intel_edp_psr_inactivate(dev, true); + obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); if (&obj->base == NULL) { ret = -ENOENT; @@ -4059,6 +4069,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, if (ret) return ret; + intel_edp_psr_inactivate(dev, true); + obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); if (&obj->base == NULL) { ret = -ENOENT; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index cbbaf26..a5815d0 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1968,6 +1968,43 @@ #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B) #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B) +/* VLV eDP PSR registers */ +#define _PSRCTLA (VLV_DISPLAY_BASE + 0x60090) +#define _PSRCTLB (VLV_DISPLAY_BASE + 0x61090) +#define VLV_EDP_PSR_ENABLE (1<<0) +#define VLV_EDP_PSR_RESET (1<<1) +#define VLV_EDP_PSR_MODE_MASK (7<<2) +#define VLV_EDP_PSR_MODE_HW_TIMER (1<<3) +#define VLV_EDP_PSR_MODE_SW_TIMER (1<<2) +#define VLV_EDP_PSR_SINGLE_FRAME_UPDATE (1<<7) +#define VLV_EDP_PSR_ACTIVE_ENTRY (1<<8) +#define VLV_EDP_PSR_SRC_TRANSMITTER_STATE (1<<9) +#define VLV_EDP_PSR_DBL_FRAME (1<<10) +#define VLV_EDP_PSR_FRAME_COUNT_MASK (0xff<<16) +#define VLV_EDP_PSR_IDLE_FRAME_SHIFT 16 +#define VLV_EDP_PSR_INT_TRANSITION (1<<24) +#define VLV_PSRCTL(pipe) _PIPE(pipe, _PSRCTLA, _PSRCTLB) + +#define _VSCSDPA (VLV_DISPLAY_BASE + 0x600a0) +#define _VSCSDPB (VLV_DISPLAY_BASE + 0x610a0) +#define VLV_EDP_PSR_SDP_FREQ_MASK (3<<30) +#define VLV_EDP_PSR_SDP_FREQ_ONCE (1<<31) +#define VLV_EDP_PSR_SDP_FREQ_EVFRAME (1<<30) +#define VLV_VSCSDP(pipe) _PIPE(pipe, _VSCSDPA, _VSCSDPB) + +#define _PSRSTATA (VLV_DISPLAY_BASE + 0x60094) +#define _PSRSTATB (VLV_DISPLAY_BASE + 0x61094) +#define VLV_EDP_PSR_LAST_STATE_MASK (7<<3) +#define VLV_EDP_PSR_CURR_STATE_MASK 7 +#define VLV_EDP_PSR_DISABLED (0<<0) +#define VLV_EDP_PSR_INACTIVE (1<<0) +#define VLV_EDP_PSR_IN_TRANS_TO_ACTIVE (2<<0) +#define VLV_EDP_PSR_ACTIVE_NORFB_UP (3<<0) +#define VLV_EDP_PSR_ACTIVE_SF_UPDATE (4<<0) +#define VLV_EDP_PSR_EXIT (5<<0) +#define VLV_EDP_PSR_IN_TRANS (1<<7) +#define VLV_PSRSTAT(pipe) _PIPE(pipe, _PSRSTATA, _PSRSTATB) + /* HSW+ eDP PSR registers */ #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800) #define EDP_PSR_CTL(dev) (EDP_PSR_BASE(dev) + 0) diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c index ffcba21..6b31020 100644 --- a/drivers/gpu/drm/i915/i915_suspend.c +++ b/drivers/gpu/drm/i915/i915_suspend.c @@ -289,8 +289,8 @@ static void i915_restore_display(struct drm_device *dev) } /* Force a full PSR setup on resume */ + intel_edp_psr_inactivate(dev, false); dev_priv->psr.setup_done = false; - intel_edp_psr_update(dev); /* only restore FBC info on the platform that supports FBC*/ intel_disable_fbc(dev); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1a9aa19..92d6df4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4451,9 +4451,13 @@ static void intel_connector_check_state(struct intel_connector *connector) * consider. */ void intel_connector_dpms(struct drm_connector *connector, int mode) { + struct drm_device *dev = connector->dev; + /* All the simple cases only support two dpms states. */ - if (mode != DRM_MODE_DPMS_ON) + if (mode != DRM_MODE_DPMS_ON) { mode = DRM_MODE_DPMS_OFF; + intel_edp_psr_inactivate(dev, false); + } if (mode == connector->dpms) return; @@ -7501,6 +7505,8 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, u32 base = 0, pos = 0; bool visible; + intel_edp_psr_inactivate(dev, true); + if (on) base = intel_crtc->cursor_addr; @@ -8230,12 +8236,15 @@ void intel_mark_idle(struct drm_device *dev) gen6_rps_idle(dev->dev_private); } + void intel_mark_fb_busy(struct drm_i915_gem_object *obj, struct intel_ring_buffer *ring) { struct drm_device *dev = obj->base.dev; struct drm_crtc *crtc; + intel_edp_psr_inactivate(dev, true); + if (!i915.powersave) return; @@ -8336,6 +8345,10 @@ static void do_intel_finish_page_flip(struct drm_device *dev, queue_work(dev_priv->wq, &work->work); trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj); + + /* Get PSR back after set_domain inactivated it without rescheduling + * it back. */ + schedule_delayed_work(&dev_priv->psr.work, msecs_to_jiffies(5000)); } void intel_finish_page_flip(struct drm_device *dev, int pipe) @@ -8688,6 +8701,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, if (work == NULL) return -ENOMEM; + /* Inactivate PSR early in page flip */ + intel_edp_psr_inactivate(dev, true); + work->event = event; work->crtc = crtc; work->old_fb_obj = to_intel_framebuffer(old_fb)->obj; diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 80054bb..576c204 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1578,21 +1578,35 @@ static void intel_dp_get_config(struct intel_encoder *encoder, } } -static bool is_edp_psr(struct drm_device *dev) +static bool is_edp_psr(struct intel_dp *intel_dp) +{ + return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED; +} + +static bool vlv_edp_is_psr_enabled_on_pipe(struct drm_device *dev, int pipe) { struct drm_i915_private *dev_priv = dev->dev_private; + uint32_t val; - return dev_priv->psr.sink_support; + val = I915_READ(VLV_PSRSTAT(pipe)) & + VLV_EDP_PSR_CURR_STATE_MASK; + return (val == VLV_EDP_PSR_ACTIVE_NORFB_UP) || + (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE); } static bool intel_edp_is_psr_enabled(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - if (!HAS_PSR(dev)) - return false; + if (HAS_PSR(dev)) { + if (IS_VALLEYVIEW(dev)) + return vlv_edp_is_psr_enabled_on_pipe(dev, PIPE_A) || + vlv_edp_is_psr_enabled_on_pipe(dev, PIPE_B); + else + return I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; + } - return I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE; + return false; } static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp, @@ -1624,32 +1638,60 @@ static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp, POSTING_READ(ctl_reg); } +static void intel_edp_psr_work(struct work_struct *work) +{ + struct drm_i915_private *dev_priv = + container_of(work, typeof(*dev_priv), psr.work.work); + struct drm_device *dev = dev_priv->dev; + + intel_edp_psr_update(dev); +} + static void intel_edp_psr_setup(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 edp_vsc_psr psr_vsc; + uint32_t val; if (dev_priv->psr.setup_done) return; - mutex_init(&dev_priv->psr.lock); - - /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */ - memset(&psr_vsc, 0, sizeof(psr_vsc)); - psr_vsc.sdp_header.HB0 = 0; - psr_vsc.sdp_header.HB1 = 0x7; - psr_vsc.sdp_header.HB2 = 0x2; - psr_vsc.sdp_header.HB3 = 0x8; - intel_edp_psr_write_vsc(intel_dp, &psr_vsc); - - /* 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); + if (IS_VALLEYVIEW(dev)) { + val = I915_READ(VLV_VSCSDP(PIPE_A)); + val &= ~VLV_EDP_PSR_SDP_FREQ_MASK; + val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME; + I915_WRITE(VLV_VSCSDP(PIPE_A), val); + + val = I915_READ(VLV_VSCSDP(PIPE_B)); + val &= ~VLV_EDP_PSR_SDP_FREQ_MASK; + val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME; + I915_WRITE(VLV_VSCSDP(PIPE_B), val); + } else { + /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */ + memset(&psr_vsc, 0, sizeof(psr_vsc)); + psr_vsc.sdp_header.HB0 = 0; + psr_vsc.sdp_header.HB1 = 0x7; + psr_vsc.sdp_header.HB2 = 0x2; + psr_vsc.sdp_header.HB3 = 0x8; + intel_edp_psr_write_vsc(intel_dp, &psr_vsc); + + /* 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); + } dev_priv->psr.setup_done = true; } +static void vlv_edp_psr_enable_sink(struct intel_dp *intel_dp) +{ + /* Enable PSR in sink */ + intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG, + DP_PSR_ENABLE); +} + static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); @@ -1680,6 +1722,27 @@ 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 vlv_edp_psr_enable_source(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 intel_crtc *intel_crtc = + to_intel_crtc(intel_dig_port->base.base.crtc); + + uint32_t idle_frames = 1; + uint32_t val = 0; + + val |= VLV_EDP_PSR_ENABLE; + val &= ~VLV_EDP_PSR_MODE_MASK; + + val |= VLV_EDP_PSR_MODE_HW_TIMER; + val &= ~VLV_EDP_PSR_FRAME_COUNT_MASK; + val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; + + I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), val); +} + static void intel_edp_psr_enable_source(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); @@ -1721,8 +1784,8 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) return false; } - if ((intel_encoder->type != INTEL_OUTPUT_EDP) || - (dig_port->port != PORT_A)) { + if (HAS_DDI(dev) && ((intel_encoder->type != INTEL_OUTPUT_EDP) || + (dig_port->port != PORT_A))) { DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n"); return false; } @@ -1767,23 +1830,45 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) return false; } + /* Baytrail supports per-pipe PSR configuration, however PSR on + * PIPE_B isn't working properly. So let's keep it disabled for now. */ + if (IS_VALLEYVIEW(dev) && intel_crtc->pipe != PIPE_A) { + DRM_DEBUG_KMS("PSR on BYT isn't enabled on pipe B.\n"); + return false; + } + dev_priv->psr.source_ok = true; return true; } static void intel_edp_psr_do_enable(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); - if (!intel_edp_psr_match_conditions(intel_dp) || - intel_edp_is_psr_enabled(dev)) - return; + if (IS_VALLEYVIEW(dev)) { + if (vlv_edp_is_psr_enabled_on_pipe(dev, intel_crtc->pipe)) + return; + } else + if (intel_edp_is_psr_enabled(dev)) + return; /* Enable PSR on the panel */ - intel_edp_psr_enable_sink(intel_dp); + if (IS_VALLEYVIEW(dev)) + vlv_edp_psr_enable_sink(intel_dp); + else + intel_edp_psr_enable_sink(intel_dp); /* Enable PSR on the host */ - intel_edp_psr_enable_source(intel_dp); + if (IS_VALLEYVIEW(dev)) + vlv_edp_psr_enable_source(intel_dp); + else + intel_edp_psr_enable_source(intel_dp); + + dev_priv->psr.active = true; } void intel_edp_psr_enable(struct intel_dp *intel_dp) @@ -1791,16 +1876,43 @@ void intel_edp_psr_enable(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 (!is_edp_psr(intel_dp)) + return; + /* Setup PSR once */ intel_edp_psr_setup(intel_dp); mutex_lock(&dev_priv->psr.lock); - if (intel_edp_psr_match_conditions(intel_dp) && - !intel_edp_is_psr_enabled(dev)) + if (intel_edp_psr_match_conditions(intel_dp)) intel_edp_psr_do_enable(intel_dp); mutex_unlock(&dev_priv->psr.lock); } +void vlv_edp_psr_disable(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 intel_crtc *intel_crtc = + to_intel_crtc(intel_dig_port->base.base.crtc); + uint32_t val; + + if (!dev_priv->psr.setup_done) + return; + + intel_edp_psr_inactivate(dev, false); + + 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"); + + 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); +} + void intel_edp_psr_disable(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); @@ -1823,6 +1935,7 @@ void intel_edp_psr_update(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_encoder *encoder; struct intel_dp *intel_dp = NULL; + struct intel_crtc *intel_crtc; if (!dev_priv->psr.setup_done) return; @@ -1830,26 +1943,93 @@ void intel_edp_psr_update(struct drm_device *dev) list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) if (encoder->type == INTEL_OUTPUT_EDP) { intel_dp = enc_to_intel_dp(&encoder->base); - - if (!is_edp_psr(dev)) - return; + intel_crtc = to_intel_crtc(encoder->base.crtc); mutex_lock(&dev_priv->psr.lock); - if (!intel_edp_psr_match_conditions(intel_dp)) - intel_edp_psr_disable(intel_dp); - else + if (!intel_edp_psr_match_conditions(intel_dp)) { + if (IS_VALLEYVIEW(dev)) + vlv_edp_psr_disable(intel_dp); + else + intel_edp_psr_disable(intel_dp); + } else if (!intel_edp_is_psr_enabled(dev)) intel_edp_psr_do_enable(intel_dp); mutex_unlock(&dev_priv->psr.lock); } } +void intel_edp_psr_do_inactivate(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_connector *connector; + struct intel_encoder *encoder; + struct intel_crtc *intel_crtc; + struct intel_dp *intel_dp = NULL; + + list_for_each_entry(connector, &dev->mode_config.connector_list, + base.head) { + + if (connector->base.dpms != DRM_MODE_DPMS_ON) + continue; + + encoder = to_intel_encoder(connector->base.encoder); + if (encoder->type == INTEL_OUTPUT_EDP) { + + intel_dp = enc_to_intel_dp(&encoder->base); + intel_crtc = to_intel_crtc(encoder->base.crtc); + + if (!vlv_edp_is_psr_enabled_on_pipe(dev, + intel_crtc->pipe)) + continue; + + dev_priv->psr.active = false; + + I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), + VLV_EDP_PSR_RESET); + /* WaClearPSRReset:vlv */ + I915_WRITE(VLV_PSRCTL(intel_crtc->pipe), 0); + + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); + } + } +} + +void intel_edp_psr_inactivate(struct drm_device *dev, bool schedule_back) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + if (!IS_VALLEYVIEW(dev)) + return; + + if (!dev_priv->psr.setup_done) + return; + + /* If it was requested to not turn psr back delayed work must be + * canceled even if it is already on inactivated state. */ + if (!schedule_back) + cancel_delayed_work_sync(&dev_priv->psr.work); + + if (!dev_priv->psr.active) + return; + + cancel_delayed_work_sync(&dev_priv->psr.work); + + intel_edp_psr_do_inactivate(dev); + + if (schedule_back) + schedule_delayed_work(&dev_priv->psr.work, + msecs_to_jiffies(5000)); +} + static void intel_disable_dp(struct intel_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); enum port port = dp_to_dig_port(intel_dp)->port; struct drm_device *dev = encoder->base.dev; + if (IS_VALLEYVIEW(dev)) + vlv_edp_psr_disable(intel_dp); + /* Make sure the panel is off before trying to change the mode. But also * ensure that we have vdd while we switch off the panel. */ intel_edp_backlight_off(intel_dp); @@ -1906,6 +2086,7 @@ static void vlv_enable_dp(struct intel_encoder *encoder) struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); intel_edp_backlight_on(intel_dp); + intel_edp_psr_enable(intel_dp); } static void g4x_pre_enable_dp(struct intel_encoder *encoder) @@ -3846,8 +4027,11 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, BUG(); } - if (is_edp(intel_dp)) + if (is_edp(intel_dp)) { intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq); + INIT_DELAYED_WORK(&dev_priv->psr.work, intel_edp_psr_work); + mutex_init(&dev_priv->psr.lock); + } error = intel_dp_i2c_init(intel_dp, intel_connector, name); WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n", diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 71c1371..d8103f9 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -748,6 +748,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp); void intel_edp_psr_enable(struct intel_dp *intel_dp); void intel_edp_psr_disable(struct intel_dp *intel_dp); void intel_edp_psr_update(struct drm_device *dev); +void intel_edp_psr_inactivate(struct drm_device *dev, bool schedule_back); /* intel_dsi.c */
This patch adds PSR Support to Baytrail. Baytrail cannot easily detect screen updates and force PSR exit. So we inactivate it on {busy_ioctl, set_domain, sw_finish and mark_busy} and update to enable it back on next display mark_idle. v2: Also inactivate PSR on cursor update. v3: Inactivate PSR on mark_busy, dset_domain and sw_finish_ioctl, and early on page flip besides avoid initializing inactive/active flag more than once. v4: Fix identation issues. v5: Rebase and add Baytrail per pipe support although leaving PIPE_B support disabled by for now since it isn't working properly yet. v6: Removing forgotten comment and useless clkgating definition. v7: Remove inactivate from set_domain. Chris warned this was semanticaly wrong. v8: Accept Ville's suggestions: Use register's names matching spec and warn if transition took longer than it should. v9: New version with delayed work to get PSR back. Disabling it on set_domain but not rescheduing it back until next finish_page_flip. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 36 ++++- drivers/gpu/drm/i915/i915_drv.h | 5 +- drivers/gpu/drm/i915/i915_gem.c | 12 ++ drivers/gpu/drm/i915/i915_reg.h | 37 +++++ drivers/gpu/drm/i915/i915_suspend.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 18 ++- drivers/gpu/drm/i915/intel_dp.c | 256 ++++++++++++++++++++++++++++++----- drivers/gpu/drm/i915/intel_drv.h | 1 + 8 files changed, 323 insertions(+), 44 deletions(-)