| Message ID | 20190930174137.15233-6-anshuman.gupta@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DC3CO Support for TGL | expand |
On Mon, Sep 30, 2019 at 11:11:36PM +0530, Anshuman Gupta wrote: > DC3CO is useful power state, when DMC detects PSR2 idle frame > while an active video playback, playing 30fps video on 60hz panel > is the classic example of this use case. > > B.Specs:49196 has a restriction to enable DC3CO only for Video Playback. > It will be worthy to enable DC3CO after completion of each pageflip > and switch back to DC5 when display is idle because driver doesn't > differentiate between video playback and a normal pageflip. > We will use Frontbuffer flush call tgl_dc3co_flush() to enable DC3CO > state only for ORIGIN_FLIP flush call, because DC3CO state has primarily > targeted for VPB use case. We are not interested here for frontbuffer > invalidates calls because that triggers PSR2 exit, which will > explicitly disable DC3CO. > > DC5 and DC6 saves more power, but can't be entered during video > playback because there are not enough idle frames in a row to meet > most PSR2 panel deep sleep entry requirement typically 4 frames. > As PSR2 existing implementation is using minimum 6 idle frames for > deep sleep, it is safer to enable DC5/6 after 6 idle frames > (By scheduling a delayed work of 6 idle frames, once DC3CO has been > enabled after a pageflip). > > After manually waiting for 6 idle frames DC5/6 will be enabled and > PSR2 deep sleep idle frames will be restored to 6 idle frames, at this > point DMC will triggers DC5/6 once PSR2 enters to deep sleep after > 6 idle frames. > In future when we will enable S/W PSR2 tracking, we can change the > PSR2 required deep sleep idle frames to 1 so DMC can trigger the > DC5/6 immediately after S/W manual waiting of 6 idle frames get > complete. > > v2: calculated s/w state to switch over dc3co when there is an > update. [Imre] > Used cancel_delayed_work_sync() in order to avoid any race > with already scheduled delayed work. [Imre] > v3: Cancel_delayed_work_sync() may blocked the commit work. > hence dropping it, dc5_idle_thread() checks the valid wakeref before > putting the reference count, which avoids any chances of dropping > a zero wakeref. [Imre (IRC)] > v4: Used frontbuffer flush mechanism. [Imre] > v5: Used psr.pipe to extract frontbuffer busy bits. [Imre] > Used cancel_delayed_work_sync() in encoder disable path. [Imre] > Used mod_delayed_work() instead of cancelling and scheduling a > delayed work. [Imre] > Used psr.lock in tgl_dc5_idle_thread() to enable psr2 deep > sleep. [Imre] > Removed DC5_REQ_IDLE_FRAMES macro. [Imre] > v6: Used dc3co_exitline check instead of TGL and dc3co allowed_dc_mask > checks, used delayed_work_pending with the psr lock and removed the > psr2_deep_slp_disabled flag. [Imre] > v7: Code refactoring moved the most of functional code to inte_psr.c [Imre] > Using frontbuffer_bits on psr.pipe check instead of > busy_frontbuffer_bits. [Imre] > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Animesh Manna <animesh.manna@intel.com> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > --- > .../drm/i915/display/intel_display_power.c | 45 ++++++++ > .../drm/i915/display/intel_display_power.h | 2 + > drivers/gpu/drm/i915/display/intel_psr.c | 109 +++++++++++++++++- > drivers/gpu/drm/i915/i915_drv.h | 2 + > 4 files changed, 157 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index 67ba92dd8366..9fddebfda169 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -886,6 +886,51 @@ lookup_power_well(struct drm_i915_private *dev_priv, > return &dev_priv->power_domains.power_wells[0]; > } > > +/** > + * intel_display_power_set_target_dc_state - Set target dc state. > + * @dev_priv: i915 device > + * @state: state which needs to be set as target_dc_state. > + * > + * This function set the "DC off" power well target_dc_state, > + * based upon this target_dc_stste, "DC off" power well will > + * enable desired DC state. > + */ > +void intel_display_power_set_target_dc_state(struct drm_i915_private *dev_priv, > + u32 state) > +{ > + struct i915_power_well *power_well; > + bool dc_off_enabled; > + struct i915_power_domains *power_domains = &dev_priv->power_domains; > + > + mutex_lock(&power_domains->lock); > + power_well = lookup_power_well(dev_priv, SKL_DISP_DC_OFF); > + > + if (WARN_ON(!power_well)) > + goto unlock; > + > + state = sanitize_target_dc_state(dev_priv, state); > + > + if (state == dev_priv->csr.target_dc_state) > + goto unlock; > + > + dc_off_enabled = power_well->desc->ops->is_enabled(dev_priv, > + power_well); > + /* > + * If DC off power well is disabled, need to enable and disable the > + * DC off power well to effect target DC state. > + */ > + if (!dc_off_enabled) > + power_well->desc->ops->enable(dev_priv, power_well); > + > + dev_priv->csr.target_dc_state = state; > + > + if (!dc_off_enabled) > + power_well->desc->ops->disable(dev_priv, power_well); > + > +unlock: > + mutex_unlock(&power_domains->lock); > +} > + > static void assert_can_enable_dc5(struct drm_i915_private *dev_priv) > { > bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv, > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > index 7d72faf474b2..1da04f3e0fb3 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > @@ -257,6 +257,8 @@ void intel_display_power_suspend_late(struct drm_i915_private *i915); > void intel_display_power_resume_early(struct drm_i915_private *i915); > void intel_display_power_suspend(struct drm_i915_private *i915); > void intel_display_power_resume(struct drm_i915_private *i915); > +void intel_display_power_set_target_dc_state(struct drm_i915_private *dev_priv, > + u32 state); > > const char * > intel_display_power_domain_str(enum intel_display_power_domain domain); > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > index b3c7eef53bf3..6a6f1031d29b 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -534,6 +534,68 @@ transcoder_has_psr2(struct drm_i915_private *dev_priv, enum transcoder trans) > return trans == TRANSCODER_EDP; > } > > +static void psr2_program_idle_frames(struct drm_i915_private *dev_priv, > + u32 idle_frames) > +{ > + u32 val; > + > + idle_frames <<= EDP_PSR2_IDLE_FRAME_SHIFT; > + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)); > + val &= ~EDP_PSR2_IDLE_FRAME_MASK; > + val |= idle_frames; > + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val); > +} > + > +static void tgl_psr2_deep_sleep_disable(struct drm_i915_private *dev_priv) > +{ > + psr2_program_idle_frames(dev_priv, 0); > +} > + > +static void tgl_psr2_deep_sleep_enable(struct drm_i915_private *dev_priv) > +{ > + int idle_frames; > + > + /* > + * Let's use 6 as the minimum to cover all known cases including the > + * off-by-one issue that HW has in some cases. > + */ > + idle_frames = max(6, dev_priv->vbt.psr.idle_frames); > + idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1); > + psr2_program_idle_frames(dev_priv, idle_frames); > +} > + > +static void tgl_enable_psr2_deep_sleep_dc6(struct drm_i915_private *dev_priv) > +{ > + intel_display_power_set_target_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6); > + tgl_psr2_deep_sleep_enable(dev_priv); > +} > + > +static void tgl_dc5_idle_thread(struct work_struct *work) > +{ > + struct drm_i915_private *dev_priv = > + container_of(work, typeof(*dev_priv), psr.idle_work.work); > + > + mutex_lock(&dev_priv->psr.lock); > + /* If delayed work is pending, it is not idle */ > + if (delayed_work_pending(&dev_priv->psr.idle_work)) > + goto unlock; > + > + DRM_DEBUG_KMS("DC5/6 idle thread\n"); > + tgl_enable_psr2_deep_sleep_dc6(dev_priv); > +unlock: > + mutex_unlock(&dev_priv->psr.lock); > +} > + > +static void tgl_disallow_dc3co_on_psr2_exit(struct drm_i915_private *dev_priv) > +{ > + if (!dev_priv->psr.dc3co_exitline) > + return; > + > + cancel_delayed_work(&dev_priv->psr.idle_work); > + /* Before PSR2 exit disallow dc3co*/ > + tgl_enable_psr2_deep_sleep_dc6(dev_priv); > +} > + > static bool intel_psr2_config_valid(struct intel_dp *intel_dp, > struct intel_crtc_state *crtc_state) > { > @@ -746,6 +808,7 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv, > dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state); > dev_priv->psr.busy_frontbuffer_bits = 0; > dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)->pipe; > + dev_priv->psr.dc3co_exitline = crtc_state->dc3co_exitline; Here we only need to know if DC3co is enabled so for clarity you could track that with a psr.dc3co_enabled bool instead of psr.dc3co_exitline. With moving the dc3co_exit_delay calculation to here the patchset is: Reviewed-by: Imre Deak <imre.deak@intel.com> > dev_priv->psr.transcoder = crtc_state->cpu_transcoder; > > /* > @@ -829,6 +892,7 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv) > } > > if (dev_priv->psr.psr2_enabled) { > + tgl_disallow_dc3co_on_psr2_exit(dev_priv); > val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)); > WARN_ON(!(val & EDP_PSR2_ENABLE)); > val &= ~EDP_PSR2_ENABLE; > @@ -901,6 +965,7 @@ void intel_psr_disable(struct intel_dp *intel_dp, > > mutex_unlock(&dev_priv->psr.lock); > cancel_work_sync(&dev_priv->psr.work); > + cancel_delayed_work_sync(&dev_priv->psr.idle_work); > } > > static void psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv) > @@ -1208,6 +1273,45 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv, > mutex_unlock(&dev_priv->psr.lock); > } > > +/* > + * When we will be completely rely on PSR2 S/W tracking in future, > + * intel_psr_flush() will invalidate and flush the PSR for ORIGIN_FLIP > + * event also therefore tgl_dc3co_flush() require to be changed > + * accrodingly in future. > + */ > +static void > +tgl_dc3co_flush(struct drm_i915_private *dev_priv, > + unsigned int frontbuffer_bits, enum fb_op_origin origin) > +{ > + u32 delay; > + > + mutex_lock(&dev_priv->psr.lock); > + > + if (!dev_priv->psr.dc3co_exitline) > + goto unlock; > + > + if (!dev_priv->psr.psr2_enabled || !dev_priv->psr.active) > + goto unlock; > + > + /* > + * At every frontbuffer flush flip event modified delay of delayed work, > + * when delayed work schedules that means display has been idle. > + */ > + if (!(frontbuffer_bits & > + INTEL_FRONTBUFFER_ALL_MASK(dev_priv->psr.pipe))) > + goto unlock; > + > + tgl_psr2_deep_sleep_disable(dev_priv); > + intel_display_power_set_target_dc_state(dev_priv, DC_STATE_EN_DC3CO); > + /* DC5/DC6 required idle frames = 6 */ > + delay = 6 * dev_priv->psr.dc3co_frame_time_us; > + mod_delayed_work(system_wq, &dev_priv->psr.idle_work, > + usecs_to_jiffies(delay)); > + > +unlock: > + mutex_unlock(&dev_priv->psr.lock); > +} > + > /** > * intel_psr_flush - Flush PSR > * @dev_priv: i915 device > @@ -1227,8 +1331,10 @@ void intel_psr_flush(struct drm_i915_private *dev_priv, > if (!CAN_PSR(dev_priv)) > return; > > - if (origin == ORIGIN_FLIP) > + if (origin == ORIGIN_FLIP) { > + tgl_dc3co_flush(dev_priv, frontbuffer_bits, origin); > return; > + } > > mutex_lock(&dev_priv->psr.lock); > if (!dev_priv->psr.enabled) { > @@ -1284,6 +1390,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv) > dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link; > > INIT_WORK(&dev_priv->psr.work, intel_psr_work); > + INIT_DELAYED_WORK(&dev_priv->psr.idle_work, tgl_dc5_idle_thread); > mutex_init(&dev_priv->psr.lock); > } > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 7b2318c5c7a0..980af06a0607 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -502,6 +502,8 @@ struct i915_psr { > bool sink_not_reliable; > bool irq_aux_error; > u16 su_x_granularity; > + u32 dc3co_exitline; > + struct delayed_work idle_work; > }; > > #define QUIRK_LVDS_SSC_DISABLE (1<<1) > -- > 2.21.0 >
On Mon, Sep 30, 2019 at 11:11:36PM +0530, Anshuman Gupta wrote: > DC3CO is useful power state, when DMC detects PSR2 idle frame > while an active video playback, playing 30fps video on 60hz panel > is the classic example of this use case. > > B.Specs:49196 has a restriction to enable DC3CO only for Video Playback. > It will be worthy to enable DC3CO after completion of each pageflip > and switch back to DC5 when display is idle because driver doesn't > differentiate between video playback and a normal pageflip. > We will use Frontbuffer flush call tgl_dc3co_flush() to enable DC3CO > state only for ORIGIN_FLIP flush call, because DC3CO state has primarily > targeted for VPB use case. We are not interested here for frontbuffer > invalidates calls because that triggers PSR2 exit, which will > explicitly disable DC3CO. > > DC5 and DC6 saves more power, but can't be entered during video > playback because there are not enough idle frames in a row to meet > most PSR2 panel deep sleep entry requirement typically 4 frames. > As PSR2 existing implementation is using minimum 6 idle frames for > deep sleep, it is safer to enable DC5/6 after 6 idle frames > (By scheduling a delayed work of 6 idle frames, once DC3CO has been > enabled after a pageflip). > > After manually waiting for 6 idle frames DC5/6 will be enabled and > PSR2 deep sleep idle frames will be restored to 6 idle frames, at this > point DMC will triggers DC5/6 once PSR2 enters to deep sleep after > 6 idle frames. > In future when we will enable S/W PSR2 tracking, we can change the > PSR2 required deep sleep idle frames to 1 so DMC can trigger the > DC5/6 immediately after S/W manual waiting of 6 idle frames get > complete. > > v2: calculated s/w state to switch over dc3co when there is an > update. [Imre] > Used cancel_delayed_work_sync() in order to avoid any race > with already scheduled delayed work. [Imre] > v3: Cancel_delayed_work_sync() may blocked the commit work. > hence dropping it, dc5_idle_thread() checks the valid wakeref before > putting the reference count, which avoids any chances of dropping > a zero wakeref. [Imre (IRC)] > v4: Used frontbuffer flush mechanism. [Imre] > v5: Used psr.pipe to extract frontbuffer busy bits. [Imre] > Used cancel_delayed_work_sync() in encoder disable path. [Imre] > Used mod_delayed_work() instead of cancelling and scheduling a > delayed work. [Imre] > Used psr.lock in tgl_dc5_idle_thread() to enable psr2 deep > sleep. [Imre] > Removed DC5_REQ_IDLE_FRAMES macro. [Imre] > v6: Used dc3co_exitline check instead of TGL and dc3co allowed_dc_mask > checks, used delayed_work_pending with the psr lock and removed the > psr2_deep_slp_disabled flag. [Imre] > v7: Code refactoring moved the most of functional code to inte_psr.c [Imre] > Using frontbuffer_bits on psr.pipe check instead of > busy_frontbuffer_bits. [Imre] > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Animesh Manna <animesh.manna@intel.com> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > --- > .../drm/i915/display/intel_display_power.c | 45 ++++++++ > .../drm/i915/display/intel_display_power.h | 2 + > drivers/gpu/drm/i915/display/intel_psr.c | 109 +++++++++++++++++- > drivers/gpu/drm/i915/i915_drv.h | 2 + > 4 files changed, 157 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index 67ba92dd8366..9fddebfda169 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -886,6 +886,51 @@ lookup_power_well(struct drm_i915_private *dev_priv, > return &dev_priv->power_domains.power_wells[0]; > } > > +/** > + * intel_display_power_set_target_dc_state - Set target dc state. > + * @dev_priv: i915 device > + * @state: state which needs to be set as target_dc_state. > + * > + * This function set the "DC off" power well target_dc_state, > + * based upon this target_dc_stste, "DC off" power well will > + * enable desired DC state. > + */ > +void intel_display_power_set_target_dc_state(struct drm_i915_private *dev_priv, > + u32 state) > +{ > + struct i915_power_well *power_well; > + bool dc_off_enabled; > + struct i915_power_domains *power_domains = &dev_priv->power_domains; > + > + mutex_lock(&power_domains->lock); > + power_well = lookup_power_well(dev_priv, SKL_DISP_DC_OFF); > + > + if (WARN_ON(!power_well)) > + goto unlock; > + > + state = sanitize_target_dc_state(dev_priv, state); > + > + if (state == dev_priv->csr.target_dc_state) > + goto unlock; > + > + dc_off_enabled = power_well->desc->ops->is_enabled(dev_priv, > + power_well); > + /* > + * If DC off power well is disabled, need to enable and disable the > + * DC off power well to effect target DC state. > + */ > + if (!dc_off_enabled) > + power_well->desc->ops->enable(dev_priv, power_well); > + > + dev_priv->csr.target_dc_state = state; > + > + if (!dc_off_enabled) > + power_well->desc->ops->disable(dev_priv, power_well); > + > +unlock: > + mutex_unlock(&power_domains->lock); > +} > + > static void assert_can_enable_dc5(struct drm_i915_private *dev_priv) > { > bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv, > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > index 7d72faf474b2..1da04f3e0fb3 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > @@ -257,6 +257,8 @@ void intel_display_power_suspend_late(struct drm_i915_private *i915); > void intel_display_power_resume_early(struct drm_i915_private *i915); > void intel_display_power_suspend(struct drm_i915_private *i915); > void intel_display_power_resume(struct drm_i915_private *i915); > +void intel_display_power_set_target_dc_state(struct drm_i915_private *dev_priv, > + u32 state); > > const char * > intel_display_power_domain_str(enum intel_display_power_domain domain); > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > index b3c7eef53bf3..6a6f1031d29b 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -534,6 +534,68 @@ transcoder_has_psr2(struct drm_i915_private *dev_priv, enum transcoder trans) > return trans == TRANSCODER_EDP; > } > > +static void psr2_program_idle_frames(struct drm_i915_private *dev_priv, > + u32 idle_frames) > +{ > + u32 val; > + > + idle_frames <<= EDP_PSR2_IDLE_FRAME_SHIFT; > + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)); > + val &= ~EDP_PSR2_IDLE_FRAME_MASK; > + val |= idle_frames; > + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val); > +} Some minor issues, could be also addressed as a follow-up: > + > +static void tgl_psr2_deep_sleep_disable(struct drm_i915_private *dev_priv) > +{ This is better named tgl_psr2_enable_dc3co(), moving the intel_display_power_set_target_dc_state(dev_priv, DC_STATE_EN_DC3CO); call to this function too. > + psr2_program_idle_frames(dev_priv, 0); > +} > + > +static void tgl_psr2_deep_sleep_enable(struct drm_i915_private *dev_priv) > +{ > + int idle_frames; > + > + /* > + * Let's use 6 as the minimum to cover all known cases including the > + * off-by-one issue that HW has in some cases. > + */ > + idle_frames = max(6, dev_priv->vbt.psr.idle_frames); > + idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1); > + psr2_program_idle_frames(dev_priv, idle_frames); > +} > + > +static void tgl_enable_psr2_deep_sleep_dc6(struct drm_i915_private *dev_priv) > +{ This is better named tgl_psr2_disable_dc3co(). > + intel_display_power_set_target_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6); > + tgl_psr2_deep_sleep_enable(dev_priv); tgl_psr2_deep_sleep_enable() could be inlined here, since there is no other user for it. That would make the function more symmetric with tgl_psr2_enable_dc3co(). > +} > + > +static void tgl_dc5_idle_thread(struct work_struct *work) > +{ > + struct drm_i915_private *dev_priv = > + container_of(work, typeof(*dev_priv), psr.idle_work.work); > + > + mutex_lock(&dev_priv->psr.lock); > + /* If delayed work is pending, it is not idle */ > + if (delayed_work_pending(&dev_priv->psr.idle_work)) > + goto unlock; > + > + DRM_DEBUG_KMS("DC5/6 idle thread\n"); > + tgl_enable_psr2_deep_sleep_dc6(dev_priv); > +unlock: > + mutex_unlock(&dev_priv->psr.lock); > +} > + > +static void tgl_disallow_dc3co_on_psr2_exit(struct drm_i915_private *dev_priv) > +{ > + if (!dev_priv->psr.dc3co_exitline) > + return; > + > + cancel_delayed_work(&dev_priv->psr.idle_work); > + /* Before PSR2 exit disallow dc3co*/ > + tgl_enable_psr2_deep_sleep_dc6(dev_priv); > +} > + > static bool intel_psr2_config_valid(struct intel_dp *intel_dp, > struct intel_crtc_state *crtc_state) > { > @@ -746,6 +808,7 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv, > dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state); > dev_priv->psr.busy_frontbuffer_bits = 0; > dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)->pipe; > + dev_priv->psr.dc3co_exitline = crtc_state->dc3co_exitline; > dev_priv->psr.transcoder = crtc_state->cpu_transcoder; > > /* > @@ -829,6 +892,7 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv) > } > > if (dev_priv->psr.psr2_enabled) { > + tgl_disallow_dc3co_on_psr2_exit(dev_priv); > val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)); > WARN_ON(!(val & EDP_PSR2_ENABLE)); > val &= ~EDP_PSR2_ENABLE; > @@ -901,6 +965,7 @@ void intel_psr_disable(struct intel_dp *intel_dp, > > mutex_unlock(&dev_priv->psr.lock); > cancel_work_sync(&dev_priv->psr.work); > + cancel_delayed_work_sync(&dev_priv->psr.idle_work); > } > > static void psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv) > @@ -1208,6 +1273,45 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv, > mutex_unlock(&dev_priv->psr.lock); > } > > +/* > + * When we will be completely rely on PSR2 S/W tracking in future, > + * intel_psr_flush() will invalidate and flush the PSR for ORIGIN_FLIP > + * event also therefore tgl_dc3co_flush() require to be changed > + * accrodingly in future. > + */ > +static void > +tgl_dc3co_flush(struct drm_i915_private *dev_priv, > + unsigned int frontbuffer_bits, enum fb_op_origin origin) > +{ > + u32 delay; > + > + mutex_lock(&dev_priv->psr.lock); > + > + if (!dev_priv->psr.dc3co_exitline) > + goto unlock; > + > + if (!dev_priv->psr.psr2_enabled || !dev_priv->psr.active) > + goto unlock; > + > + /* > + * At every frontbuffer flush flip event modified delay of delayed work, > + * when delayed work schedules that means display has been idle. > + */ > + if (!(frontbuffer_bits & > + INTEL_FRONTBUFFER_ALL_MASK(dev_priv->psr.pipe))) > + goto unlock; > + > + tgl_psr2_deep_sleep_disable(dev_priv); > + intel_display_power_set_target_dc_state(dev_priv, DC_STATE_EN_DC3CO); > + /* DC5/DC6 required idle frames = 6 */ > + delay = 6 * dev_priv->psr.dc3co_frame_time_us; > + mod_delayed_work(system_wq, &dev_priv->psr.idle_work, > + usecs_to_jiffies(delay)); > + > +unlock: > + mutex_unlock(&dev_priv->psr.lock); > +} > + > /** > * intel_psr_flush - Flush PSR > * @dev_priv: i915 device > @@ -1227,8 +1331,10 @@ void intel_psr_flush(struct drm_i915_private *dev_priv, > if (!CAN_PSR(dev_priv)) > return; > > - if (origin == ORIGIN_FLIP) > + if (origin == ORIGIN_FLIP) { > + tgl_dc3co_flush(dev_priv, frontbuffer_bits, origin); > return; > + } > > mutex_lock(&dev_priv->psr.lock); > if (!dev_priv->psr.enabled) { > @@ -1284,6 +1390,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv) > dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link; > > INIT_WORK(&dev_priv->psr.work, intel_psr_work); > + INIT_DELAYED_WORK(&dev_priv->psr.idle_work, tgl_dc5_idle_thread); > mutex_init(&dev_priv->psr.lock); > } > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 7b2318c5c7a0..980af06a0607 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -502,6 +502,8 @@ struct i915_psr { > bool sink_not_reliable; > bool irq_aux_error; > u16 su_x_granularity; > + u32 dc3co_exitline; > + struct delayed_work idle_work; > }; > > #define QUIRK_LVDS_SSC_DISABLE (1<<1) > -- > 2.21.0 >
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index 67ba92dd8366..9fddebfda169 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -886,6 +886,51 @@ lookup_power_well(struct drm_i915_private *dev_priv, return &dev_priv->power_domains.power_wells[0]; } +/** + * intel_display_power_set_target_dc_state - Set target dc state. + * @dev_priv: i915 device + * @state: state which needs to be set as target_dc_state. + * + * This function set the "DC off" power well target_dc_state, + * based upon this target_dc_stste, "DC off" power well will + * enable desired DC state. + */ +void intel_display_power_set_target_dc_state(struct drm_i915_private *dev_priv, + u32 state) +{ + struct i915_power_well *power_well; + bool dc_off_enabled; + struct i915_power_domains *power_domains = &dev_priv->power_domains; + + mutex_lock(&power_domains->lock); + power_well = lookup_power_well(dev_priv, SKL_DISP_DC_OFF); + + if (WARN_ON(!power_well)) + goto unlock; + + state = sanitize_target_dc_state(dev_priv, state); + + if (state == dev_priv->csr.target_dc_state) + goto unlock; + + dc_off_enabled = power_well->desc->ops->is_enabled(dev_priv, + power_well); + /* + * If DC off power well is disabled, need to enable and disable the + * DC off power well to effect target DC state. + */ + if (!dc_off_enabled) + power_well->desc->ops->enable(dev_priv, power_well); + + dev_priv->csr.target_dc_state = state; + + if (!dc_off_enabled) + power_well->desc->ops->disable(dev_priv, power_well); + +unlock: + mutex_unlock(&power_domains->lock); +} + static void assert_can_enable_dc5(struct drm_i915_private *dev_priv) { bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv, diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h index 7d72faf474b2..1da04f3e0fb3 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.h +++ b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -257,6 +257,8 @@ void intel_display_power_suspend_late(struct drm_i915_private *i915); void intel_display_power_resume_early(struct drm_i915_private *i915); void intel_display_power_suspend(struct drm_i915_private *i915); void intel_display_power_resume(struct drm_i915_private *i915); +void intel_display_power_set_target_dc_state(struct drm_i915_private *dev_priv, + u32 state); const char * intel_display_power_domain_str(enum intel_display_power_domain domain); diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index b3c7eef53bf3..6a6f1031d29b 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -534,6 +534,68 @@ transcoder_has_psr2(struct drm_i915_private *dev_priv, enum transcoder trans) return trans == TRANSCODER_EDP; } +static void psr2_program_idle_frames(struct drm_i915_private *dev_priv, + u32 idle_frames) +{ + u32 val; + + idle_frames <<= EDP_PSR2_IDLE_FRAME_SHIFT; + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)); + val &= ~EDP_PSR2_IDLE_FRAME_MASK; + val |= idle_frames; + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val); +} + +static void tgl_psr2_deep_sleep_disable(struct drm_i915_private *dev_priv) +{ + psr2_program_idle_frames(dev_priv, 0); +} + +static void tgl_psr2_deep_sleep_enable(struct drm_i915_private *dev_priv) +{ + int idle_frames; + + /* + * Let's use 6 as the minimum to cover all known cases including the + * off-by-one issue that HW has in some cases. + */ + idle_frames = max(6, dev_priv->vbt.psr.idle_frames); + idle_frames = max(idle_frames, dev_priv->psr.sink_sync_latency + 1); + psr2_program_idle_frames(dev_priv, idle_frames); +} + +static void tgl_enable_psr2_deep_sleep_dc6(struct drm_i915_private *dev_priv) +{ + intel_display_power_set_target_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6); + tgl_psr2_deep_sleep_enable(dev_priv); +} + +static void tgl_dc5_idle_thread(struct work_struct *work) +{ + struct drm_i915_private *dev_priv = + container_of(work, typeof(*dev_priv), psr.idle_work.work); + + mutex_lock(&dev_priv->psr.lock); + /* If delayed work is pending, it is not idle */ + if (delayed_work_pending(&dev_priv->psr.idle_work)) + goto unlock; + + DRM_DEBUG_KMS("DC5/6 idle thread\n"); + tgl_enable_psr2_deep_sleep_dc6(dev_priv); +unlock: + mutex_unlock(&dev_priv->psr.lock); +} + +static void tgl_disallow_dc3co_on_psr2_exit(struct drm_i915_private *dev_priv) +{ + if (!dev_priv->psr.dc3co_exitline) + return; + + cancel_delayed_work(&dev_priv->psr.idle_work); + /* Before PSR2 exit disallow dc3co*/ + tgl_enable_psr2_deep_sleep_dc6(dev_priv); +} + static bool intel_psr2_config_valid(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_state) { @@ -746,6 +808,7 @@ static void intel_psr_enable_locked(struct drm_i915_private *dev_priv, dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state); dev_priv->psr.busy_frontbuffer_bits = 0; dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)->pipe; + dev_priv->psr.dc3co_exitline = crtc_state->dc3co_exitline; dev_priv->psr.transcoder = crtc_state->cpu_transcoder; /* @@ -829,6 +892,7 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv) } if (dev_priv->psr.psr2_enabled) { + tgl_disallow_dc3co_on_psr2_exit(dev_priv); val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)); WARN_ON(!(val & EDP_PSR2_ENABLE)); val &= ~EDP_PSR2_ENABLE; @@ -901,6 +965,7 @@ void intel_psr_disable(struct intel_dp *intel_dp, mutex_unlock(&dev_priv->psr.lock); cancel_work_sync(&dev_priv->psr.work); + cancel_delayed_work_sync(&dev_priv->psr.idle_work); } static void psr_force_hw_tracking_exit(struct drm_i915_private *dev_priv) @@ -1208,6 +1273,45 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv, mutex_unlock(&dev_priv->psr.lock); } +/* + * When we will be completely rely on PSR2 S/W tracking in future, + * intel_psr_flush() will invalidate and flush the PSR for ORIGIN_FLIP + * event also therefore tgl_dc3co_flush() require to be changed + * accrodingly in future. + */ +static void +tgl_dc3co_flush(struct drm_i915_private *dev_priv, + unsigned int frontbuffer_bits, enum fb_op_origin origin) +{ + u32 delay; + + mutex_lock(&dev_priv->psr.lock); + + if (!dev_priv->psr.dc3co_exitline) + goto unlock; + + if (!dev_priv->psr.psr2_enabled || !dev_priv->psr.active) + goto unlock; + + /* + * At every frontbuffer flush flip event modified delay of delayed work, + * when delayed work schedules that means display has been idle. + */ + if (!(frontbuffer_bits & + INTEL_FRONTBUFFER_ALL_MASK(dev_priv->psr.pipe))) + goto unlock; + + tgl_psr2_deep_sleep_disable(dev_priv); + intel_display_power_set_target_dc_state(dev_priv, DC_STATE_EN_DC3CO); + /* DC5/DC6 required idle frames = 6 */ + delay = 6 * dev_priv->psr.dc3co_frame_time_us; + mod_delayed_work(system_wq, &dev_priv->psr.idle_work, + usecs_to_jiffies(delay)); + +unlock: + mutex_unlock(&dev_priv->psr.lock); +} + /** * intel_psr_flush - Flush PSR * @dev_priv: i915 device @@ -1227,8 +1331,10 @@ void intel_psr_flush(struct drm_i915_private *dev_priv, if (!CAN_PSR(dev_priv)) return; - if (origin == ORIGIN_FLIP) + if (origin == ORIGIN_FLIP) { + tgl_dc3co_flush(dev_priv, frontbuffer_bits, origin); return; + } mutex_lock(&dev_priv->psr.lock); if (!dev_priv->psr.enabled) { @@ -1284,6 +1390,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv) dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link; INIT_WORK(&dev_priv->psr.work, intel_psr_work); + INIT_DELAYED_WORK(&dev_priv->psr.idle_work, tgl_dc5_idle_thread); mutex_init(&dev_priv->psr.lock); } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 7b2318c5c7a0..980af06a0607 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -502,6 +502,8 @@ struct i915_psr { bool sink_not_reliable; bool irq_aux_error; u16 su_x_granularity; + u32 dc3co_exitline; + struct delayed_work idle_work; }; #define QUIRK_LVDS_SSC_DISABLE (1<<1)
DC3CO is useful power state, when DMC detects PSR2 idle frame while an active video playback, playing 30fps video on 60hz panel is the classic example of this use case. B.Specs:49196 has a restriction to enable DC3CO only for Video Playback. It will be worthy to enable DC3CO after completion of each pageflip and switch back to DC5 when display is idle because driver doesn't differentiate between video playback and a normal pageflip. We will use Frontbuffer flush call tgl_dc3co_flush() to enable DC3CO state only for ORIGIN_FLIP flush call, because DC3CO state has primarily targeted for VPB use case. We are not interested here for frontbuffer invalidates calls because that triggers PSR2 exit, which will explicitly disable DC3CO. DC5 and DC6 saves more power, but can't be entered during video playback because there are not enough idle frames in a row to meet most PSR2 panel deep sleep entry requirement typically 4 frames. As PSR2 existing implementation is using minimum 6 idle frames for deep sleep, it is safer to enable DC5/6 after 6 idle frames (By scheduling a delayed work of 6 idle frames, once DC3CO has been enabled after a pageflip). After manually waiting for 6 idle frames DC5/6 will be enabled and PSR2 deep sleep idle frames will be restored to 6 idle frames, at this point DMC will triggers DC5/6 once PSR2 enters to deep sleep after 6 idle frames. In future when we will enable S/W PSR2 tracking, we can change the PSR2 required deep sleep idle frames to 1 so DMC can trigger the DC5/6 immediately after S/W manual waiting of 6 idle frames get complete. v2: calculated s/w state to switch over dc3co when there is an update. [Imre] Used cancel_delayed_work_sync() in order to avoid any race with already scheduled delayed work. [Imre] v3: Cancel_delayed_work_sync() may blocked the commit work. hence dropping it, dc5_idle_thread() checks the valid wakeref before putting the reference count, which avoids any chances of dropping a zero wakeref. [Imre (IRC)] v4: Used frontbuffer flush mechanism. [Imre] v5: Used psr.pipe to extract frontbuffer busy bits. [Imre] Used cancel_delayed_work_sync() in encoder disable path. [Imre] Used mod_delayed_work() instead of cancelling and scheduling a delayed work. [Imre] Used psr.lock in tgl_dc5_idle_thread() to enable psr2 deep sleep. [Imre] Removed DC5_REQ_IDLE_FRAMES macro. [Imre] v6: Used dc3co_exitline check instead of TGL and dc3co allowed_dc_mask checks, used delayed_work_pending with the psr lock and removed the psr2_deep_slp_disabled flag. [Imre] v7: Code refactoring moved the most of functional code to inte_psr.c [Imre] Using frontbuffer_bits on psr.pipe check instead of busy_frontbuffer_bits. [Imre] Cc: Jani Nikula <jani.nikula@intel.com> Cc: Imre Deak <imre.deak@intel.com> Cc: Animesh Manna <animesh.manna@intel.com> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> --- .../drm/i915/display/intel_display_power.c | 45 ++++++++ .../drm/i915/display/intel_display_power.h | 2 + drivers/gpu/drm/i915/display/intel_psr.c | 109 +++++++++++++++++- drivers/gpu/drm/i915/i915_drv.h | 2 + 4 files changed, 157 insertions(+), 1 deletion(-)