Message ID | 1402593406-17869-4-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Thu, Jun 12, 2014 at 10:16:41AM -0700, Rodrigo Vivi wrote: > The perfect solution for psr_exit is the hardware tracking the changes and > doing the psr exit by itself. This scenario works for HSW and BDW with some > environments like Gnome and Wayland. > > However there are many other scenarios that this isn't true. Mainly one right > now is KDE users on HSW and BDW with PSR on. User would miss many screen > updates. For instances any key typed could be seen only when mouse cursor is > moved. So this patch introduces the ability of trigger PSR exit on kernel side > on some common cases that. > > Most of the cases are coverred by psr_exit at set_domain. The remaining cases > are coverred by triggering it at set_domain, busy_ioctl, sw_finish and > mark_busy. > > The downside here might be reducing the residency time on the cases this > already work very wall like Gnome environment. But so far let's get focused > on fixinge issues sio PSR couild be used for everybody and we could even > get it enabled by default. Later we can add some alternatives to choose the > level of PSR efficiency over boot flag of even over crtc property. > > Reviewed-by: Vijay Purushothaman <vijay.a.purushothaman@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 3 ++ > drivers/gpu/drm/i915/i915_gem.c | 6 +++ > drivers/gpu/drm/i915/intel_display.c | 20 +++++++- > drivers/gpu/drm/i915/intel_dp.c | 91 ++++++++++++++++++++++++++++++++---- > drivers/gpu/drm/i915/intel_drv.h | 3 ++ > 5 files changed, 111 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index f36d9eb..64d520f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -637,6 +637,9 @@ struct i915_psr { > bool sink_support; > bool source_ok; > bool setup_done; > + bool enabled; > + bool active; > + struct delayed_work work; > }; > > enum intel_pch { > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 1dc4b74..3637add 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1395,6 +1395,8 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, > goto unlock; > } > > + intel_edp_psr_exit(dev, true); > + > /* 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. > @@ -1440,6 +1442,8 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data, > if (ret) > return ret; > > + intel_edp_psr_exit(dev, true); > + > obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); > if (&obj->base == NULL) { > ret = -ENOENT; > @@ -4234,6 +4238,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, > if (ret) > return ret; > > + intel_edp_psr_exit(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/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index b5cbb28..70dc433 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5017,6 +5017,8 @@ 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; > + struct intel_encoder *intel_encoder; > /* All the simple cases only support two dpms states. */ > if (mode != DRM_MODE_DPMS_ON) > mode = DRM_MODE_DPMS_OFF; > @@ -5027,9 +5029,15 @@ void intel_connector_dpms(struct drm_connector *connector, int mode) > connector->dpms = mode; > > /* Only need to change hw state when actually enabled */ > - if (connector->encoder) > - intel_encoder_dpms(to_intel_encoder(connector->encoder), mode); > + if (connector->encoder) { > + > + intel_encoder = to_intel_encoder(connector->encoder); > + intel_encoder_dpms(intel_encoder, mode); > > + if (intel_encoder->type == INTEL_OUTPUT_EDP && > + mode == DRM_MODE_DPMS_OFF) > + intel_edp_psr_exit(dev, false); Nope. intel_connector_dpms is just the generic implementation we have for forwarding the drm core ->dpms interface to our own stuff. Sprinkling random things in here is not ok. This either needs to be in the crtc_disable or in an encoder->disable function. Sorry I didn't spot this earlier. First two patches merged. -Daniel > + } > intel_modeset_check_state(connector->dev); > } > > @@ -8826,12 +8834,15 @@ out: > intel_runtime_pm_put(dev_priv); > } > > + > void intel_mark_fb_busy(struct drm_i915_gem_object *obj, > struct intel_engine_cs *ring) > { > struct drm_device *dev = obj->base.dev; > struct drm_crtc *crtc; > > + intel_edp_psr_exit(dev, true); > + > if (!i915.powersave) > return; > > @@ -9300,6 +9311,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > if (work == NULL) > return -ENOMEM; > > + /* Exit PSR early in page flip */ > + intel_edp_psr_exit(dev, true); > + > work->event = event; > work->crtc = crtc; > work->old_fb_obj = to_intel_framebuffer(old_fb)->obj; > @@ -11491,6 +11505,8 @@ static void intel_setup_outputs(struct drm_device *dev) > if (SUPPORTS_TV(dev)) > intel_tv_init(dev); > > + intel_edp_psr_init(dev); > + > list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) { > encoder->base.possible_crtcs = encoder->crtc_mask; > encoder->base.possible_clones = > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 68289ce..f7b2414 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1797,10 +1797,11 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) > > 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; > > - if (!intel_edp_psr_match_conditions(intel_dp) || > - intel_edp_is_psr_enabled(dev)) > + if (intel_edp_is_psr_enabled(dev)) > return; > > /* Enable PSR on the panel */ > @@ -1808,6 +1809,9 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp) > > /* Enable PSR on the host */ > intel_edp_psr_enable_source(intel_dp); > + > + dev_priv->psr.enabled = true; > + dev_priv->psr.active = true; > } > > void intel_edp_psr_enable(struct intel_dp *intel_dp) > @@ -1827,8 +1831,7 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp) > /* Setup PSR once */ > intel_edp_psr_setup(intel_dp); > > - 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); > } > > @@ -1837,7 +1840,7 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp) > struct drm_device *dev = intel_dp_to_dev(intel_dp); > struct drm_i915_private *dev_priv = dev->dev_private; > > - if (!intel_edp_is_psr_enabled(dev)) > + if (!dev_priv->psr.enabled) > return; > > I915_WRITE(EDP_PSR_CTL(dev), > @@ -1847,13 +1850,13 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp) > if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev)) & > EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10)) > DRM_ERROR("Timed out waiting for PSR Idle State\n"); > + > + dev_priv->psr.enabled = false; > } > > 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; > > if (!HAS_PSR(dev)) > return; > @@ -1861,6 +1864,17 @@ void intel_edp_psr_update(struct drm_device *dev) > if (!dev_priv->psr.setup_done) > return; > > + intel_edp_psr_exit(dev, true); > +} > + > +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; > + struct intel_encoder *encoder; > + struct intel_dp *intel_dp = NULL; > + > 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); > @@ -1868,9 +1882,66 @@ void intel_edp_psr_update(struct drm_device *dev) > if (!intel_edp_psr_match_conditions(intel_dp)) > intel_edp_psr_disable(intel_dp); > else > - if (!intel_edp_is_psr_enabled(dev)) > - intel_edp_psr_do_enable(intel_dp); > + intel_edp_psr_do_enable(intel_dp); > + } > +} > + > +void intel_edp_psr_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); > + > + dev_priv->psr.active = false; > + > + I915_WRITE(EDP_PSR_CTL(dev), I915_READ(EDP_PSR_CTL(dev)) > + & ~EDP_PSR_ENABLE); > } > + } > +} > + > +void intel_edp_psr_exit(struct drm_device *dev, bool schedule_back) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (!HAS_PSR(dev)) > + return; > + > + if (!dev_priv->psr.setup_done) > + return; > + > + cancel_delayed_work_sync(&dev_priv->psr.work); > + > + if (dev_priv->psr.active) > + intel_edp_psr_inactivate(dev); > + > + if (schedule_back) > + schedule_delayed_work(&dev_priv->psr.work, > + msecs_to_jiffies(100)); > +} > + > +void intel_edp_psr_init(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (!HAS_PSR(dev)) > + return; > + > + INIT_DELAYED_WORK(&dev_priv->psr.work, intel_edp_psr_work); > } > > static void intel_disable_dp(struct intel_encoder *encoder) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 1904ef8..bd90661 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -831,6 +831,9 @@ 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_dp_set_drrs_state(struct drm_device *dev, int refresh_rate); > +void intel_edp_psr_exit(struct drm_device *dev, bool schedule_back); > +void intel_edp_psr_init(struct drm_device *dev); > + > > /* intel_dsi.c */ > void intel_dsi_init(struct drm_device *dev); > -- > 1.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f36d9eb..64d520f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -637,6 +637,9 @@ struct i915_psr { bool sink_support; bool source_ok; bool setup_done; + bool enabled; + bool active; + struct delayed_work work; }; enum intel_pch { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 1dc4b74..3637add 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1395,6 +1395,8 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, goto unlock; } + intel_edp_psr_exit(dev, true); + /* 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. @@ -1440,6 +1442,8 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data, if (ret) return ret; + intel_edp_psr_exit(dev, true); + obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); if (&obj->base == NULL) { ret = -ENOENT; @@ -4234,6 +4238,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, if (ret) return ret; + intel_edp_psr_exit(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/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b5cbb28..70dc433 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5017,6 +5017,8 @@ 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; + struct intel_encoder *intel_encoder; /* All the simple cases only support two dpms states. */ if (mode != DRM_MODE_DPMS_ON) mode = DRM_MODE_DPMS_OFF; @@ -5027,9 +5029,15 @@ void intel_connector_dpms(struct drm_connector *connector, int mode) connector->dpms = mode; /* Only need to change hw state when actually enabled */ - if (connector->encoder) - intel_encoder_dpms(to_intel_encoder(connector->encoder), mode); + if (connector->encoder) { + + intel_encoder = to_intel_encoder(connector->encoder); + intel_encoder_dpms(intel_encoder, mode); + if (intel_encoder->type == INTEL_OUTPUT_EDP && + mode == DRM_MODE_DPMS_OFF) + intel_edp_psr_exit(dev, false); + } intel_modeset_check_state(connector->dev); } @@ -8826,12 +8834,15 @@ out: intel_runtime_pm_put(dev_priv); } + void intel_mark_fb_busy(struct drm_i915_gem_object *obj, struct intel_engine_cs *ring) { struct drm_device *dev = obj->base.dev; struct drm_crtc *crtc; + intel_edp_psr_exit(dev, true); + if (!i915.powersave) return; @@ -9300,6 +9311,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, if (work == NULL) return -ENOMEM; + /* Exit PSR early in page flip */ + intel_edp_psr_exit(dev, true); + work->event = event; work->crtc = crtc; work->old_fb_obj = to_intel_framebuffer(old_fb)->obj; @@ -11491,6 +11505,8 @@ static void intel_setup_outputs(struct drm_device *dev) if (SUPPORTS_TV(dev)) intel_tv_init(dev); + intel_edp_psr_init(dev); + list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) { encoder->base.possible_crtcs = encoder->crtc_mask; encoder->base.possible_clones = diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 68289ce..f7b2414 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1797,10 +1797,11 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) 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; - if (!intel_edp_psr_match_conditions(intel_dp) || - intel_edp_is_psr_enabled(dev)) + if (intel_edp_is_psr_enabled(dev)) return; /* Enable PSR on the panel */ @@ -1808,6 +1809,9 @@ static void intel_edp_psr_do_enable(struct intel_dp *intel_dp) /* Enable PSR on the host */ intel_edp_psr_enable_source(intel_dp); + + dev_priv->psr.enabled = true; + dev_priv->psr.active = true; } void intel_edp_psr_enable(struct intel_dp *intel_dp) @@ -1827,8 +1831,7 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp) /* Setup PSR once */ intel_edp_psr_setup(intel_dp); - 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); } @@ -1837,7 +1840,7 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp) struct drm_device *dev = intel_dp_to_dev(intel_dp); struct drm_i915_private *dev_priv = dev->dev_private; - if (!intel_edp_is_psr_enabled(dev)) + if (!dev_priv->psr.enabled) return; I915_WRITE(EDP_PSR_CTL(dev), @@ -1847,13 +1850,13 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp) if (_wait_for((I915_READ(EDP_PSR_STATUS_CTL(dev)) & EDP_PSR_STATUS_STATE_MASK) == 0, 2000, 10)) DRM_ERROR("Timed out waiting for PSR Idle State\n"); + + dev_priv->psr.enabled = false; } 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; if (!HAS_PSR(dev)) return; @@ -1861,6 +1864,17 @@ void intel_edp_psr_update(struct drm_device *dev) if (!dev_priv->psr.setup_done) return; + intel_edp_psr_exit(dev, true); +} + +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; + struct intel_encoder *encoder; + struct intel_dp *intel_dp = NULL; + 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); @@ -1868,9 +1882,66 @@ void intel_edp_psr_update(struct drm_device *dev) if (!intel_edp_psr_match_conditions(intel_dp)) intel_edp_psr_disable(intel_dp); else - if (!intel_edp_is_psr_enabled(dev)) - intel_edp_psr_do_enable(intel_dp); + intel_edp_psr_do_enable(intel_dp); + } +} + +void intel_edp_psr_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); + + dev_priv->psr.active = false; + + I915_WRITE(EDP_PSR_CTL(dev), I915_READ(EDP_PSR_CTL(dev)) + & ~EDP_PSR_ENABLE); } + } +} + +void intel_edp_psr_exit(struct drm_device *dev, bool schedule_back) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + if (!HAS_PSR(dev)) + return; + + if (!dev_priv->psr.setup_done) + return; + + cancel_delayed_work_sync(&dev_priv->psr.work); + + if (dev_priv->psr.active) + intel_edp_psr_inactivate(dev); + + if (schedule_back) + schedule_delayed_work(&dev_priv->psr.work, + msecs_to_jiffies(100)); +} + +void intel_edp_psr_init(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + if (!HAS_PSR(dev)) + return; + + INIT_DELAYED_WORK(&dev_priv->psr.work, intel_edp_psr_work); } static void intel_disable_dp(struct intel_encoder *encoder) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1904ef8..bd90661 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -831,6 +831,9 @@ 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_dp_set_drrs_state(struct drm_device *dev, int refresh_rate); +void intel_edp_psr_exit(struct drm_device *dev, bool schedule_back); +void intel_edp_psr_init(struct drm_device *dev); + /* intel_dsi.c */ void intel_dsi_init(struct drm_device *dev);