| Message ID | 20190809183223.12031-6-anshuman.gupta@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DC3CO Support for TGL | expand |
On Sat, Aug 10, 2019 at 12:02:19AM +0530, Anshuman Gupta wrote: > We need to have a S/W flag based upon which driver can switch to DC3CO. > If it is only edp display connected and it has psr2 capability, > then set a prefer_dc3co flag to true, which will be used to > switch to dc3co as well as to program DC3CO PSR2 transcoder > early exitline event. > > 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> > --- > drivers/gpu/drm/i915/display/intel_display.c | 5 + > .../drm/i915/display/intel_display_power.c | 105 ++++++++++++++++++ > .../drm/i915/display/intel_display_power.h | 5 + > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_pm.c | 2 +- > drivers/gpu/drm/i915/intel_pm.h | 2 + > 6 files changed, 119 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 647f49ca86ff..1ec204c14a10 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -6448,6 +6448,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, > > if (WARN_ON(intel_crtc->active)) > return; > + /* Enable PSR2 transcoder exit line */ > + if (pipe_config->has_psr2 && dev_priv->csr.prefer_dc3co) > + tgl_enable_psr2_transcoder_exitline(pipe_config); This is part of PSR2 programming, so should be done somewhere below intel_psr_enable() imo. > > intel_encoders_pre_pll_enable(intel_crtc, pipe_config, state); > > @@ -13685,6 +13688,8 @@ static int intel_atomic_check(struct drm_device *dev, > "[modeset]" : "[fastset]"); > } > > + tgl_prefer_dc3co_over_dc5_check(dev_priv, state); I think this belongs to intel_modeset_checks(), where we could also do all necessary pipe locking (since now I can't see how we would prevent allowing DC3CO when an external output is enabled asynchronously). This would be akin to CDCLK rate change, but I defer on this to Ville. > + > return 0; > > fail: > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index 167839060154..04a02c88ff93 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -18,6 +18,7 @@ > #include "intel_hotplug.h" > #include "intel_sideband.h" > #include "intel_tc.h" > +#include "intel_pm.h" > > bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv, > enum i915_power_well_id power_well_id); > @@ -791,6 +792,110 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state) > dev_priv->csr.dc_state = val & mask; > } > > +void tgl_enable_psr2_transcoder_exitline(struct intel_crtc_state *cstate) > +{ > + u32 linetime_us, val, exit_scanlines; > + u32 crtc_vdisplay = cstate->base.adjusted_mode.crtc_vdisplay; > + struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev); > + > + if (WARN_ON(cstate->cpu_transcoder != TRANSCODER_A)) > + return; Where's the TRANSCODER-A restriction coming from? > + > + linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(cstate)); > + if (WARN_ON(!linetime_us)) > + return; > + /* > + * DC3CO Exit time 200us B.Spec 49196 > + * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line time) + 1 > + * Exit line event need to program above calculated scan lines before > + * next VBLANK. > + */ > + exit_scanlines = DIV_ROUND_UP(200, linetime_us) + 1; > + if (WARN_ON(exit_scanlines > crtc_vdisplay)) > + return; > + > + exit_scanlines = crtc_vdisplay - exit_scanlines; > + exit_scanlines <<= EXITLINE_SHIFT; > + val = I915_READ(EXITLINE(cstate->cpu_transcoder)); > + val &= ~(EXITLINE_MASK | EXITLINE_ENABLE); > + val |= exit_scanlines; > + val |= EXITLINE_ENABLE; > + I915_WRITE(EXITLINE(cstate->cpu_transcoder), val); > +} > + > +static bool tgl_is_only_edp_connected(struct intel_crtc_state *crtc_state) > +{ > + struct drm_atomic_state *state = crtc_state->base.state; > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > + struct drm_connector *connector, *edp_connector = NULL; > + struct drm_connector_state *connector_state; > + int i; > + > + for_each_new_connector_in_state(state, connector, connector_state, i) { > + if (connector_state->crtc != &crtc->base) > + continue; > + > + if (connector->status == connector_status_connected && > + connector->connector_type != DRM_MODE_CONNECTOR_eDP) > + return false; > + else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP && > + connector->status == connector_status_connected) > + edp_connector = connector; > + } > + > + if (edp_connector) > + return true; > + > + return false; > +} > + > +/* > + * tgl_prefer_dc3co_over_dc5_check check whether it is worth to choose > + * DC3CO over DC5. Currently it just check crtc psr2 capebilty and only > + * edp display should be connected. > + * TODO: Prefer DC3CO over DC5 only in video playback. > + */ > +void tgl_prefer_dc3co_over_dc5_check(struct drm_i915_private *dev_priv, > + struct intel_atomic_state *state) > +{ > + struct intel_crtc_state *crtc_state, *mode_changed_cstate; > + struct intel_crtc *crtc; > + int i; > + u32 val; > + > + dev_priv->csr.prefer_dc3co = false; > + > + if (!IS_TIGERLAKE(dev_priv)) > + return; > + > + if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)) > + return; > + > + for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) { > + if (crtc->pipe == PIPE_A) > + mode_changed_cstate = crtc_state; Where is the PIPE-A restriction coming from? Couldn't we simply pick the crtc state that has the PSR2 sink? > + if (!crtc_state->has_psr2 && crtc_state->base.active) { > + dev_priv->csr.prefer_dc3co = false; > + return; > + } else if (crtc_state->has_psr2) { > + if (tgl_is_only_edp_connected(crtc_state) && > + crtc_state->base.active) { > + dev_priv->csr.prefer_dc3co = true; > + continue; > + } else { > + dev_priv->csr.prefer_dc3co = false; > + return; > + } > + } > + } > + > + if (dev_priv->csr.prefer_dc3co) { > + val = I915_READ(EXITLINE(mode_changed_cstate->cpu_transcoder)); No HW access during the modeset check phase is the rule. You could just use the old prefer_dc3co value, no? > + if (!(val & EXITLINE_ENABLE)) > + mode_changed_cstate->base.mode_changed = true; > + } > +} > + > static void tgl_allow_dc3co(struct drm_i915_private *dev_priv) > { > gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO); > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > index 97f2562fc5d3..46e1bcfa490a 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > @@ -12,6 +12,8 @@ > > struct drm_i915_private; > struct intel_encoder; > +struct intel_crtc_state; > +struct intel_atomic_state; > > enum intel_display_power_domain { > POWER_DOMAIN_DISPLAY_CORE, > @@ -246,6 +248,9 @@ 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 tgl_prefer_dc3co_over_dc5_check(struct drm_i915_private *dev_priv, > + struct intel_atomic_state *state); > +void tgl_enable_psr2_transcoder_exitline(struct intel_crtc_state *cstate); > > const char * > intel_display_power_domain_str(struct drm_i915_private *i915, > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 3cdb5bf489f2..7ca0703209a4 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -332,6 +332,7 @@ struct intel_csr { > u32 dc_state; > u32 allowed_dc_mask; > intel_wakeref_t wakeref; > + bool prefer_dc3co; > }; > > enum i915_cache_level { > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 780df8db2eba..634e43219164 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4584,7 +4584,7 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency, > return ret; > } > > -static uint_fixed_16_16_t > +uint_fixed_16_16_t > intel_get_linetime_us(const struct intel_crtc_state *crtc_state) > { > u32 pixel_rate; > diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h > index e3573e1e16e3..454e92c06dff 100644 > --- a/drivers/gpu/drm/i915/intel_pm.h > +++ b/drivers/gpu/drm/i915/intel_pm.h > @@ -8,6 +8,7 @@ > > #include <linux/types.h> > > +#include "i915_drv.h" > #include "i915_reg.h" > > struct drm_device; > @@ -76,6 +77,7 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, i915_reg_t reg); > u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, i915_reg_t reg); > > u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1); > +uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *cstate); > > unsigned long i915_chipset_val(struct drm_i915_private *dev_priv); > unsigned long i915_mch_val(struct drm_i915_private *dev_priv); > -- > 2.21.0 >
On Sat, Aug 10, 2019 at 12:02:19AM +0530, Anshuman Gupta wrote: > We need to have a S/W flag based upon which driver can switch to DC3CO. > If it is only edp display connected and it has psr2 capability, > then set a prefer_dc3co flag to true, which will be used to > switch to dc3co as well as to program DC3CO PSR2 transcoder > early exitline event. > > 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> > --- > drivers/gpu/drm/i915/display/intel_display.c | 5 + > .../drm/i915/display/intel_display_power.c | 105 ++++++++++++++++++ > .../drm/i915/display/intel_display_power.h | 5 + > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_pm.c | 2 +- > drivers/gpu/drm/i915/intel_pm.h | 2 + > 6 files changed, 119 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 647f49ca86ff..1ec204c14a10 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -6448,6 +6448,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, > > if (WARN_ON(intel_crtc->active)) > return; > + /* Enable PSR2 transcoder exit line */ > + if (pipe_config->has_psr2 && dev_priv->csr.prefer_dc3co) > + tgl_enable_psr2_transcoder_exitline(pipe_config); > > intel_encoders_pre_pll_enable(intel_crtc, pipe_config, state); > > @@ -13685,6 +13688,8 @@ static int intel_atomic_check(struct drm_device *dev, > "[modeset]" : "[fastset]"); > } > > + tgl_prefer_dc3co_over_dc5_check(dev_priv, state); > + > return 0; > > fail: > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index 167839060154..04a02c88ff93 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -18,6 +18,7 @@ > #include "intel_hotplug.h" > #include "intel_sideband.h" > #include "intel_tc.h" > +#include "intel_pm.h" > > bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv, > enum i915_power_well_id power_well_id); > @@ -791,6 +792,110 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state) > dev_priv->csr.dc_state = val & mask; > } > > +void tgl_enable_psr2_transcoder_exitline(struct intel_crtc_state *cstate) > +{ > + u32 linetime_us, val, exit_scanlines; > + u32 crtc_vdisplay = cstate->base.adjusted_mode.crtc_vdisplay; > + struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev); > + > + if (WARN_ON(cstate->cpu_transcoder != TRANSCODER_A)) > + return; > + > + linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(cstate)); > + if (WARN_ON(!linetime_us)) > + return; > + /* > + * DC3CO Exit time 200us B.Spec 49196 > + * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line time) + 1 > + * Exit line event need to program above calculated scan lines before > + * next VBLANK. > + */ > + exit_scanlines = DIV_ROUND_UP(200, linetime_us) + 1; > + if (WARN_ON(exit_scanlines > crtc_vdisplay)) > + return; > + > + exit_scanlines = crtc_vdisplay - exit_scanlines; > + exit_scanlines <<= EXITLINE_SHIFT; > + val = I915_READ(EXITLINE(cstate->cpu_transcoder)); > + val &= ~(EXITLINE_MASK | EXITLINE_ENABLE); > + val |= exit_scanlines; > + val |= EXITLINE_ENABLE; > + I915_WRITE(EXITLINE(cstate->cpu_transcoder), val); > +} > + > +static bool tgl_is_only_edp_connected(struct intel_crtc_state *crtc_state) > +{ > + struct drm_atomic_state *state = crtc_state->base.state; > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > + struct drm_connector *connector, *edp_connector = NULL; > + struct drm_connector_state *connector_state; > + int i; > + > + for_each_new_connector_in_state(state, connector, connector_state, i) { > + if (connector_state->crtc != &crtc->base) > + continue; > + > + if (connector->status == connector_status_connected && > + connector->connector_type != DRM_MODE_CONNECTOR_eDP) > + return false; > + else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP && > + connector->status == connector_status_connected) > + edp_connector = connector; > + } > + > + if (edp_connector) > + return true; > + > + return false; > +} > + > +/* > + * tgl_prefer_dc3co_over_dc5_check check whether it is worth to choose > + * DC3CO over DC5. Currently it just check crtc psr2 capebilty and only > + * edp display should be connected. > + * TODO: Prefer DC3CO over DC5 only in video playback. > + */ > +void tgl_prefer_dc3co_over_dc5_check(struct drm_i915_private *dev_priv, > + struct intel_atomic_state *state) One more thing here: this function will set prefer_dc3co if there is only a PSR2 eDP output connected. However DC states will be disabled whenever an external display is connected, so I wonder if we could just depend on that and allow DC3CO (instead of the default DC5/6 state) from the PSR2 enabling/disabling and page flip/idle thread functions. > +{ > + struct intel_crtc_state *crtc_state, *mode_changed_cstate; > + struct intel_crtc *crtc; > + int i; > + u32 val; > + > + dev_priv->csr.prefer_dc3co = false; > + > + if (!IS_TIGERLAKE(dev_priv)) > + return; > + > + if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)) > + return; > + > + for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) { > + if (crtc->pipe == PIPE_A) > + mode_changed_cstate = crtc_state; > + if (!crtc_state->has_psr2 && crtc_state->base.active) { > + dev_priv->csr.prefer_dc3co = false; > + return; > + } else if (crtc_state->has_psr2) { > + if (tgl_is_only_edp_connected(crtc_state) && > + crtc_state->base.active) { > + dev_priv->csr.prefer_dc3co = true; > + continue; > + } else { > + dev_priv->csr.prefer_dc3co = false; > + return; > + } > + } > + } > + > + if (dev_priv->csr.prefer_dc3co) { > + val = I915_READ(EXITLINE(mode_changed_cstate->cpu_transcoder)); > + if (!(val & EXITLINE_ENABLE)) > + mode_changed_cstate->base.mode_changed = true; > + } > +} > + > static void tgl_allow_dc3co(struct drm_i915_private *dev_priv) > { > gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO); > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > index 97f2562fc5d3..46e1bcfa490a 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > @@ -12,6 +12,8 @@ > > struct drm_i915_private; > struct intel_encoder; > +struct intel_crtc_state; > +struct intel_atomic_state; > > enum intel_display_power_domain { > POWER_DOMAIN_DISPLAY_CORE, > @@ -246,6 +248,9 @@ 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 tgl_prefer_dc3co_over_dc5_check(struct drm_i915_private *dev_priv, > + struct intel_atomic_state *state); > +void tgl_enable_psr2_transcoder_exitline(struct intel_crtc_state *cstate); > > const char * > intel_display_power_domain_str(struct drm_i915_private *i915, > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 3cdb5bf489f2..7ca0703209a4 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -332,6 +332,7 @@ struct intel_csr { > u32 dc_state; > u32 allowed_dc_mask; > intel_wakeref_t wakeref; > + bool prefer_dc3co; > }; > > enum i915_cache_level { > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 780df8db2eba..634e43219164 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4584,7 +4584,7 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency, > return ret; > } > > -static uint_fixed_16_16_t > +uint_fixed_16_16_t > intel_get_linetime_us(const struct intel_crtc_state *crtc_state) > { > u32 pixel_rate; > diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h > index e3573e1e16e3..454e92c06dff 100644 > --- a/drivers/gpu/drm/i915/intel_pm.h > +++ b/drivers/gpu/drm/i915/intel_pm.h > @@ -8,6 +8,7 @@ > > #include <linux/types.h> > > +#include "i915_drv.h" > #include "i915_reg.h" > > struct drm_device; > @@ -76,6 +77,7 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, i915_reg_t reg); > u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, i915_reg_t reg); > > u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1); > +uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *cstate); > > unsigned long i915_chipset_val(struct drm_i915_private *dev_priv); > unsigned long i915_mch_val(struct drm_i915_private *dev_priv); > -- > 2.21.0 >
On 8/13/2019 9:17 PM, Imre Deak wrote: > On Sat, Aug 10, 2019 at 12:02:19AM +0530, Anshuman Gupta wrote: >> We need to have a S/W flag based upon which driver can switch to DC3CO. >> If it is only edp display connected and it has psr2 capability, >> then set a prefer_dc3co flag to true, which will be used to >> switch to dc3co as well as to program DC3CO PSR2 transcoder >> early exitline event. >> >> 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> >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 5 + >> .../drm/i915/display/intel_display_power.c | 105 ++++++++++++++++++ >> .../drm/i915/display/intel_display_power.h | 5 + >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> drivers/gpu/drm/i915/intel_pm.c | 2 +- >> drivers/gpu/drm/i915/intel_pm.h | 2 + >> 6 files changed, 119 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> index 647f49ca86ff..1ec204c14a10 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -6448,6 +6448,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, >> >> if (WARN_ON(intel_crtc->active)) >> return; >> + /* Enable PSR2 transcoder exit line */ >> + if (pipe_config->has_psr2 && dev_priv->csr.prefer_dc3co) >> + tgl_enable_psr2_transcoder_exitline(pipe_config); > > This is part of PSR2 programming, so should be done somewhere below > intel_psr_enable() imo. > >> >> intel_encoders_pre_pll_enable(intel_crtc, pipe_config, state); >> >> @@ -13685,6 +13688,8 @@ static int intel_atomic_check(struct drm_device *dev, >> "[modeset]" : "[fastset]"); >> } >> >> + tgl_prefer_dc3co_over_dc5_check(dev_priv, state); > > I think this belongs to intel_modeset_checks(), where we could also do > all necessary pipe locking (since now I can't see how we would prevent > allowing DC3CO when an external output is enabled asynchronously). This > would be akin to CDCLK rate change, but I defer on this to Ville. Hi Ville, Could you please provide your inputs on DC3CO design perspective, whether we should have pipe locking in order to maintain consistent state for dc3co. I feel there can be two way to maintain consistent state. 1. Ensure necessary locking on current crtc before calculating prefer_dc3co state, this will block the asynchronous commit. 2. We can make the prefer_dc3co state to false from enable powerwell callback which will trigger by getting a reference count for PG2 power domains if any external output is enabled. (DC3CO will be disallowed from power well enable callback if any external output is enabled akin to DC5/DC6, here we want to make prefer_dc3co atomic state consistent). Thanks , Anshuman Gupta. > >> + >> return 0; >> >> fail: >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c >> index 167839060154..04a02c88ff93 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_power.c >> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c >> @@ -18,6 +18,7 @@ >> #include "intel_hotplug.h" >> #include "intel_sideband.h" >> #include "intel_tc.h" >> +#include "intel_pm.h" >> >> bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv, >> enum i915_power_well_id power_well_id); >> @@ -791,6 +792,110 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state) >> dev_priv->csr.dc_state = val & mask; >> } >> >> +void tgl_enable_psr2_transcoder_exitline(struct intel_crtc_state *cstate) >> +{ >> + u32 linetime_us, val, exit_scanlines; >> + u32 crtc_vdisplay = cstate->base.adjusted_mode.crtc_vdisplay; >> + struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev); >> + >> + if (WARN_ON(cstate->cpu_transcoder != TRANSCODER_A)) >> + return; > > Where's the TRANSCODER-A restriction coming from? > >> + >> + linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(cstate)); >> + if (WARN_ON(!linetime_us)) >> + return; >> + /* >> + * DC3CO Exit time 200us B.Spec 49196 >> + * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line time) + 1 >> + * Exit line event need to program above calculated scan lines before >> + * next VBLANK. >> + */ >> + exit_scanlines = DIV_ROUND_UP(200, linetime_us) + 1; >> + if (WARN_ON(exit_scanlines > crtc_vdisplay)) >> + return; >> + >> + exit_scanlines = crtc_vdisplay - exit_scanlines; >> + exit_scanlines <<= EXITLINE_SHIFT; >> + val = I915_READ(EXITLINE(cstate->cpu_transcoder)); >> + val &= ~(EXITLINE_MASK | EXITLINE_ENABLE); >> + val |= exit_scanlines; >> + val |= EXITLINE_ENABLE; >> + I915_WRITE(EXITLINE(cstate->cpu_transcoder), val); >> +} >> + >> +static bool tgl_is_only_edp_connected(struct intel_crtc_state *crtc_state) >> +{ >> + struct drm_atomic_state *state = crtc_state->base.state; >> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); >> + struct drm_connector *connector, *edp_connector = NULL; >> + struct drm_connector_state *connector_state; >> + int i; >> + >> + for_each_new_connector_in_state(state, connector, connector_state, i) { >> + if (connector_state->crtc != &crtc->base) >> + continue; >> + >> + if (connector->status == connector_status_connected && >> + connector->connector_type != DRM_MODE_CONNECTOR_eDP) >> + return false; >> + else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP && >> + connector->status == connector_status_connected) >> + edp_connector = connector; >> + } >> + >> + if (edp_connector) >> + return true; >> + >> + return false; >> +} >> + >> +/* >> + * tgl_prefer_dc3co_over_dc5_check check whether it is worth to choose >> + * DC3CO over DC5. Currently it just check crtc psr2 capebilty and only >> + * edp display should be connected. >> + * TODO: Prefer DC3CO over DC5 only in video playback. >> + */ >> +void tgl_prefer_dc3co_over_dc5_check(struct drm_i915_private *dev_priv, >> + struct intel_atomic_state *state) >> +{ >> + struct intel_crtc_state *crtc_state, *mode_changed_cstate; >> + struct intel_crtc *crtc; >> + int i; >> + u32 val; >> + >> + dev_priv->csr.prefer_dc3co = false; >> + >> + if (!IS_TIGERLAKE(dev_priv)) >> + return; >> + >> + if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)) >> + return; >> + >> + for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) { >> + if (crtc->pipe == PIPE_A) >> + mode_changed_cstate = crtc_state; > > Where is the PIPE-A restriction coming from? Couldn't we simply pick the > crtc state that has the PSR2 sink? > >> + if (!crtc_state->has_psr2 && crtc_state->base.active) { >> + dev_priv->csr.prefer_dc3co = false; >> + return; >> + } else if (crtc_state->has_psr2) { >> + if (tgl_is_only_edp_connected(crtc_state) && >> + crtc_state->base.active) { >> + dev_priv->csr.prefer_dc3co = true; >> + continue; >> + } else { >> + dev_priv->csr.prefer_dc3co = false; >> + return; >> + } >> + } >> + } >> + >> + if (dev_priv->csr.prefer_dc3co) { >> + val = I915_READ(EXITLINE(mode_changed_cstate->cpu_transcoder)); > > No HW access during the modeset check phase is the rule. You could just > use the old prefer_dc3co value, no? > >> + if (!(val & EXITLINE_ENABLE)) >> + mode_changed_cstate->base.mode_changed = true; >> + } >> +} >> + >> static void tgl_allow_dc3co(struct drm_i915_private *dev_priv) >> { >> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO); >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h >> index 97f2562fc5d3..46e1bcfa490a 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_power.h >> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h >> @@ -12,6 +12,8 @@ >> >> struct drm_i915_private; >> struct intel_encoder; >> +struct intel_crtc_state; >> +struct intel_atomic_state; >> >> enum intel_display_power_domain { >> POWER_DOMAIN_DISPLAY_CORE, >> @@ -246,6 +248,9 @@ 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 tgl_prefer_dc3co_over_dc5_check(struct drm_i915_private *dev_priv, >> + struct intel_atomic_state *state); >> +void tgl_enable_psr2_transcoder_exitline(struct intel_crtc_state *cstate); >> >> const char * >> intel_display_power_domain_str(struct drm_i915_private *i915, >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 3cdb5bf489f2..7ca0703209a4 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -332,6 +332,7 @@ struct intel_csr { >> u32 dc_state; >> u32 allowed_dc_mask; >> intel_wakeref_t wakeref; >> + bool prefer_dc3co; >> }; >> >> enum i915_cache_level { >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index 780df8db2eba..634e43219164 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -4584,7 +4584,7 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency, >> return ret; >> } >> >> -static uint_fixed_16_16_t >> +uint_fixed_16_16_t >> intel_get_linetime_us(const struct intel_crtc_state *crtc_state) >> { >> u32 pixel_rate; >> diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h >> index e3573e1e16e3..454e92c06dff 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.h >> +++ b/drivers/gpu/drm/i915/intel_pm.h >> @@ -8,6 +8,7 @@ >> >> #include <linux/types.h> >> >> +#include "i915_drv.h" >> #include "i915_reg.h" >> >> struct drm_device; >> @@ -76,6 +77,7 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, i915_reg_t reg); >> u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, i915_reg_t reg); >> >> u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1); >> +uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *cstate); >> >> unsigned long i915_chipset_val(struct drm_i915_private *dev_priv); >> unsigned long i915_mch_val(struct drm_i915_private *dev_priv); >> -- >> 2.21.0 >>
On Mon, Aug 19, 2019 at 06:27:48PM +0530, Gupta, Anshuman wrote: > > > On 8/13/2019 9:17 PM, Imre Deak wrote: > > On Sat, Aug 10, 2019 at 12:02:19AM +0530, Anshuman Gupta wrote: > > > We need to have a S/W flag based upon which driver can switch to DC3CO. > > > If it is only edp display connected and it has psr2 capability, > > > then set a prefer_dc3co flag to true, which will be used to > > > switch to dc3co as well as to program DC3CO PSR2 transcoder > > > early exitline event. > > > > > > 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> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 5 + > > > .../drm/i915/display/intel_display_power.c | 105 ++++++++++++++++++ > > > .../drm/i915/display/intel_display_power.h | 5 + > > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > > drivers/gpu/drm/i915/intel_pm.c | 2 +- > > > drivers/gpu/drm/i915/intel_pm.h | 2 + > > > 6 files changed, 119 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > index 647f49ca86ff..1ec204c14a10 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -6448,6 +6448,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, > > > if (WARN_ON(intel_crtc->active)) > > > return; > > > + /* Enable PSR2 transcoder exit line */ > > > + if (pipe_config->has_psr2 && dev_priv->csr.prefer_dc3co) > > > + tgl_enable_psr2_transcoder_exitline(pipe_config); > > > > This is part of PSR2 programming, so should be done somewhere below > > intel_psr_enable() imo. > > > > > intel_encoders_pre_pll_enable(intel_crtc, pipe_config, state); > > > @@ -13685,6 +13688,8 @@ static int intel_atomic_check(struct drm_device *dev, > > > "[modeset]" : "[fastset]"); > > > } > > > + tgl_prefer_dc3co_over_dc5_check(dev_priv, state); > > > > I think this belongs to intel_modeset_checks(), where we could also do > > all necessary pipe locking (since now I can't see how we would prevent > > allowing DC3CO when an external output is enabled asynchronously). This > > would be akin to CDCLK rate change, but I defer on this to Ville. > Hi Ville, > Could you please provide your inputs on DC3CO design perspective, > whether we should have pipe locking in order to maintain consistent state > for dc3co. > > I feel there can be two way to maintain consistent state. > > 1. Ensure necessary locking on current crtc before calculating prefer_dc3co > state, this will block the asynchronous commit. > > 2. We can make the prefer_dc3co state to false from enable powerwell > callback which will trigger by getting a reference count for PG2 power > domains if any external output is enabled. > > (DC3CO will be disallowed from power well enable callback if any external > output is enabled akin to DC5/DC6, here we want to make prefer_dc3co atomic > state consistent). Actually the latest idea based on further discussions was: since all DC states will be disabled if there is an external output enabled, we wouldn't need to compute prefer_dc3co. We could just enable/disable DC3CO (with a new enable_dc3co() API) whenever enabling/disabling PSR2 and during a page-flip/from the page-idle work. That together with the current state of the dc_off power well determines if any of DC5/6 or DC3co is enabled atm. > > Thanks , > Anshuman Gupta. > > > > > + > > > return 0; > > > fail: > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > > > index 167839060154..04a02c88ff93 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > > @@ -18,6 +18,7 @@ > > > #include "intel_hotplug.h" > > > #include "intel_sideband.h" > > > #include "intel_tc.h" > > > +#include "intel_pm.h" > > > bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv, > > > enum i915_power_well_id power_well_id); > > > @@ -791,6 +792,110 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state) > > > dev_priv->csr.dc_state = val & mask; > > > } > > > +void tgl_enable_psr2_transcoder_exitline(struct intel_crtc_state *cstate) > > > +{ > > > + u32 linetime_us, val, exit_scanlines; > > > + u32 crtc_vdisplay = cstate->base.adjusted_mode.crtc_vdisplay; > > > + struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev); > > > + > > > + if (WARN_ON(cstate->cpu_transcoder != TRANSCODER_A)) > > > + return; > > > > Where's the TRANSCODER-A restriction coming from? > > > > > + > > > + linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(cstate)); > > > + if (WARN_ON(!linetime_us)) > > > + return; > > > + /* > > > + * DC3CO Exit time 200us B.Spec 49196 > > > + * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line time) + 1 > > > + * Exit line event need to program above calculated scan lines before > > > + * next VBLANK. > > > + */ > > > + exit_scanlines = DIV_ROUND_UP(200, linetime_us) + 1; > > > + if (WARN_ON(exit_scanlines > crtc_vdisplay)) > > > + return; > > > + > > > + exit_scanlines = crtc_vdisplay - exit_scanlines; > > > + exit_scanlines <<= EXITLINE_SHIFT; > > > + val = I915_READ(EXITLINE(cstate->cpu_transcoder)); > > > + val &= ~(EXITLINE_MASK | EXITLINE_ENABLE); > > > + val |= exit_scanlines; > > > + val |= EXITLINE_ENABLE; > > > + I915_WRITE(EXITLINE(cstate->cpu_transcoder), val); > > > +} > > > + > > > +static bool tgl_is_only_edp_connected(struct intel_crtc_state *crtc_state) > > > +{ > > > + struct drm_atomic_state *state = crtc_state->base.state; > > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > > > + struct drm_connector *connector, *edp_connector = NULL; > > > + struct drm_connector_state *connector_state; > > > + int i; > > > + > > > + for_each_new_connector_in_state(state, connector, connector_state, i) { > > > + if (connector_state->crtc != &crtc->base) > > > + continue; > > > + > > > + if (connector->status == connector_status_connected && > > > + connector->connector_type != DRM_MODE_CONNECTOR_eDP) > > > + return false; > > > + else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP && > > > + connector->status == connector_status_connected) > > > + edp_connector = connector; > > > + } > > > + > > > + if (edp_connector) > > > + return true; > > > + > > > + return false; > > > +} > > > + > > > +/* > > > + * tgl_prefer_dc3co_over_dc5_check check whether it is worth to choose > > > + * DC3CO over DC5. Currently it just check crtc psr2 capebilty and only > > > + * edp display should be connected. > > > + * TODO: Prefer DC3CO over DC5 only in video playback. > > > + */ > > > +void tgl_prefer_dc3co_over_dc5_check(struct drm_i915_private *dev_priv, > > > + struct intel_atomic_state *state) > > > +{ > > > + struct intel_crtc_state *crtc_state, *mode_changed_cstate; > > > + struct intel_crtc *crtc; > > > + int i; > > > + u32 val; > > > + > > > + dev_priv->csr.prefer_dc3co = false; > > > + > > > + if (!IS_TIGERLAKE(dev_priv)) > > > + return; > > > + > > > + if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)) > > > + return; > > > + > > > + for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) { > > > + if (crtc->pipe == PIPE_A) > > > + mode_changed_cstate = crtc_state; > > > > Where is the PIPE-A restriction coming from? Couldn't we simply pick the > > crtc state that has the PSR2 sink? > > > > > + if (!crtc_state->has_psr2 && crtc_state->base.active) { > > > + dev_priv->csr.prefer_dc3co = false; > > > + return; > > > + } else if (crtc_state->has_psr2) { > > > + if (tgl_is_only_edp_connected(crtc_state) && > > > + crtc_state->base.active) { > > > + dev_priv->csr.prefer_dc3co = true; > > > + continue; > > > + } else { > > > + dev_priv->csr.prefer_dc3co = false; > > > + return; > > > + } > > > + } > > > + } > > > + > > > + if (dev_priv->csr.prefer_dc3co) { > > > + val = I915_READ(EXITLINE(mode_changed_cstate->cpu_transcoder)); > > > > No HW access during the modeset check phase is the rule. You could just > > use the old prefer_dc3co value, no? > > > > > + if (!(val & EXITLINE_ENABLE)) > > > + mode_changed_cstate->base.mode_changed = true; > > > + } > > > +} > > > + > > > static void tgl_allow_dc3co(struct drm_i915_private *dev_priv) > > > { > > > gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO); > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > > > index 97f2562fc5d3..46e1bcfa490a 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > > > @@ -12,6 +12,8 @@ > > > struct drm_i915_private; > > > struct intel_encoder; > > > +struct intel_crtc_state; > > > +struct intel_atomic_state; > > > enum intel_display_power_domain { > > > POWER_DOMAIN_DISPLAY_CORE, > > > @@ -246,6 +248,9 @@ 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 tgl_prefer_dc3co_over_dc5_check(struct drm_i915_private *dev_priv, > > > + struct intel_atomic_state *state); > > > +void tgl_enable_psr2_transcoder_exitline(struct intel_crtc_state *cstate); > > > const char * > > > intel_display_power_domain_str(struct drm_i915_private *i915, > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > > index 3cdb5bf489f2..7ca0703209a4 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -332,6 +332,7 @@ struct intel_csr { > > > u32 dc_state; > > > u32 allowed_dc_mask; > > > intel_wakeref_t wakeref; > > > + bool prefer_dc3co; > > > }; > > > enum i915_cache_level { > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > > index 780df8db2eba..634e43219164 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -4584,7 +4584,7 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency, > > > return ret; > > > } > > > -static uint_fixed_16_16_t > > > +uint_fixed_16_16_t > > > intel_get_linetime_us(const struct intel_crtc_state *crtc_state) > > > { > > > u32 pixel_rate; > > > diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h > > > index e3573e1e16e3..454e92c06dff 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.h > > > +++ b/drivers/gpu/drm/i915/intel_pm.h > > > @@ -8,6 +8,7 @@ > > > #include <linux/types.h> > > > +#include "i915_drv.h" > > > #include "i915_reg.h" > > > struct drm_device; > > > @@ -76,6 +77,7 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, i915_reg_t reg); > > > u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, i915_reg_t reg); > > > u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1); > > > +uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *cstate); > > > unsigned long i915_chipset_val(struct drm_i915_private *dev_priv); > > > unsigned long i915_mch_val(struct drm_i915_private *dev_priv); > > > -- > > > 2.21.0 > > >
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 647f49ca86ff..1ec204c14a10 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -6448,6 +6448,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, if (WARN_ON(intel_crtc->active)) return; + /* Enable PSR2 transcoder exit line */ + if (pipe_config->has_psr2 && dev_priv->csr.prefer_dc3co) + tgl_enable_psr2_transcoder_exitline(pipe_config); intel_encoders_pre_pll_enable(intel_crtc, pipe_config, state); @@ -13685,6 +13688,8 @@ static int intel_atomic_check(struct drm_device *dev, "[modeset]" : "[fastset]"); } + tgl_prefer_dc3co_over_dc5_check(dev_priv, state); + return 0; fail: diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index 167839060154..04a02c88ff93 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -18,6 +18,7 @@ #include "intel_hotplug.h" #include "intel_sideband.h" #include "intel_tc.h" +#include "intel_pm.h" bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv, enum i915_power_well_id power_well_id); @@ -791,6 +792,110 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state) dev_priv->csr.dc_state = val & mask; } +void tgl_enable_psr2_transcoder_exitline(struct intel_crtc_state *cstate) +{ + u32 linetime_us, val, exit_scanlines; + u32 crtc_vdisplay = cstate->base.adjusted_mode.crtc_vdisplay; + struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev); + + if (WARN_ON(cstate->cpu_transcoder != TRANSCODER_A)) + return; + + linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(cstate)); + if (WARN_ON(!linetime_us)) + return; + /* + * DC3CO Exit time 200us B.Spec 49196 + * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line time) + 1 + * Exit line event need to program above calculated scan lines before + * next VBLANK. + */ + exit_scanlines = DIV_ROUND_UP(200, linetime_us) + 1; + if (WARN_ON(exit_scanlines > crtc_vdisplay)) + return; + + exit_scanlines = crtc_vdisplay - exit_scanlines; + exit_scanlines <<= EXITLINE_SHIFT; + val = I915_READ(EXITLINE(cstate->cpu_transcoder)); + val &= ~(EXITLINE_MASK | EXITLINE_ENABLE); + val |= exit_scanlines; + val |= EXITLINE_ENABLE; + I915_WRITE(EXITLINE(cstate->cpu_transcoder), val); +} + +static bool tgl_is_only_edp_connected(struct intel_crtc_state *crtc_state) +{ + struct drm_atomic_state *state = crtc_state->base.state; + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); + struct drm_connector *connector, *edp_connector = NULL; + struct drm_connector_state *connector_state; + int i; + + for_each_new_connector_in_state(state, connector, connector_state, i) { + if (connector_state->crtc != &crtc->base) + continue; + + if (connector->status == connector_status_connected && + connector->connector_type != DRM_MODE_CONNECTOR_eDP) + return false; + else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP && + connector->status == connector_status_connected) + edp_connector = connector; + } + + if (edp_connector) + return true; + + return false; +} + +/* + * tgl_prefer_dc3co_over_dc5_check check whether it is worth to choose + * DC3CO over DC5. Currently it just check crtc psr2 capebilty and only + * edp display should be connected. + * TODO: Prefer DC3CO over DC5 only in video playback. + */ +void tgl_prefer_dc3co_over_dc5_check(struct drm_i915_private *dev_priv, + struct intel_atomic_state *state) +{ + struct intel_crtc_state *crtc_state, *mode_changed_cstate; + struct intel_crtc *crtc; + int i; + u32 val; + + dev_priv->csr.prefer_dc3co = false; + + if (!IS_TIGERLAKE(dev_priv)) + return; + + if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)) + return; + + for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) { + if (crtc->pipe == PIPE_A) + mode_changed_cstate = crtc_state; + if (!crtc_state->has_psr2 && crtc_state->base.active) { + dev_priv->csr.prefer_dc3co = false; + return; + } else if (crtc_state->has_psr2) { + if (tgl_is_only_edp_connected(crtc_state) && + crtc_state->base.active) { + dev_priv->csr.prefer_dc3co = true; + continue; + } else { + dev_priv->csr.prefer_dc3co = false; + return; + } + } + } + + if (dev_priv->csr.prefer_dc3co) { + val = I915_READ(EXITLINE(mode_changed_cstate->cpu_transcoder)); + if (!(val & EXITLINE_ENABLE)) + mode_changed_cstate->base.mode_changed = true; + } +} + static void tgl_allow_dc3co(struct drm_i915_private *dev_priv) { gen9_set_dc_state(dev_priv, DC_STATE_EN_DC3CO); diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h index 97f2562fc5d3..46e1bcfa490a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.h +++ b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -12,6 +12,8 @@ struct drm_i915_private; struct intel_encoder; +struct intel_crtc_state; +struct intel_atomic_state; enum intel_display_power_domain { POWER_DOMAIN_DISPLAY_CORE, @@ -246,6 +248,9 @@ 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 tgl_prefer_dc3co_over_dc5_check(struct drm_i915_private *dev_priv, + struct intel_atomic_state *state); +void tgl_enable_psr2_transcoder_exitline(struct intel_crtc_state *cstate); const char * intel_display_power_domain_str(struct drm_i915_private *i915, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3cdb5bf489f2..7ca0703209a4 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -332,6 +332,7 @@ struct intel_csr { u32 dc_state; u32 allowed_dc_mask; intel_wakeref_t wakeref; + bool prefer_dc3co; }; enum i915_cache_level { diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 780df8db2eba..634e43219164 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4584,7 +4584,7 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency, return ret; } -static uint_fixed_16_16_t +uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *crtc_state) { u32 pixel_rate; diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h index e3573e1e16e3..454e92c06dff 100644 --- a/drivers/gpu/drm/i915/intel_pm.h +++ b/drivers/gpu/drm/i915/intel_pm.h @@ -8,6 +8,7 @@ #include <linux/types.h> +#include "i915_drv.h" #include "i915_reg.h" struct drm_device; @@ -76,6 +77,7 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, i915_reg_t reg); u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, i915_reg_t reg); u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1); +uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *cstate); unsigned long i915_chipset_val(struct drm_i915_private *dev_priv); unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
We need to have a S/W flag based upon which driver can switch to DC3CO. If it is only edp display connected and it has psr2 capability, then set a prefer_dc3co flag to true, which will be used to switch to dc3co as well as to program DC3CO PSR2 transcoder early exitline event. 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> --- drivers/gpu/drm/i915/display/intel_display.c | 5 + .../drm/i915/display/intel_display_power.c | 105 ++++++++++++++++++ .../drm/i915/display/intel_display_power.h | 5 + drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 2 +- drivers/gpu/drm/i915/intel_pm.h | 2 + 6 files changed, 119 insertions(+), 1 deletion(-)