On Wed, 2020-01-22 at 18:49 +0530, Anshuman Gupta wrote: > On 2020-01-21 at 12:13:14 -0800, José Roberto de Souza wrote: > > A recent change in BSpec allow us to change EXTLINE while > > transcoder > > is enabled so this allow us to change it even when doing the first > > fastset after taking over previous hardware state set by BIOS. > > BIOS don't enable PSR, so if sink supports PSR it will be enabled > > on > > the first fastset, so moving the EXTLINE compute and set to PSR > > flows > > allow us to simplfy a bunch of code. > > > > This will save a lot of time in all the IGT tests that uses CRC, as > > when PSR2 is enabled CRCs are not generated, so we switch to PSR1, > > so > > the previous code would compute dc3co_exitline=0 causing a full > > modeset that would shutdown pipe, enable and train link. > > > > v2: only programming EXTLINE when DC3CO is enabled > > > > BSpec: 49196 > > Cc: Imre Deak <[email protected]> > > Cc: Anshuman Gupta <[email protected]> > > Signed-off-by: José Roberto de Souza <[email protected]> > > --- > > drivers/gpu/drm/i915/display/intel_ddi.c | 86 ---------------- > > ---- > > drivers/gpu/drm/i915/display/intel_display.c | 1 - > > drivers/gpu/drm/i915/display/intel_psr.c | 46 +++++++++++ > > 3 files changed, 46 insertions(+), 87 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > > b/drivers/gpu/drm/i915/display/intel_ddi.c > > index bbf1c0a243a2..e52c3cae2965 100644 > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > @@ -3344,86 +3344,6 @@ static void > > intel_ddi_disable_fec_state(struct intel_encoder *encoder, > > POSTING_READ(intel_dp->regs.dp_tp_ctl); > > } > > > > -static void > > -tgl_clear_psr2_transcoder_exitline(const struct intel_crtc_state > > *cstate) > > -{ > > - struct drm_i915_private *dev_priv = to_i915(cstate->uapi.crtc- > > >dev); > > - u32 val; > > - > > - if (!cstate->dc3co_exitline) > > - return; > > - > > - val = I915_READ(EXITLINE(cstate->cpu_transcoder)); > > - val &= ~(EXITLINE_MASK | EXITLINE_ENABLE); > > - I915_WRITE(EXITLINE(cstate->cpu_transcoder), val); > > -} > > - > > -static void > > -tgl_set_psr2_transcoder_exitline(const struct intel_crtc_state > > *cstate) > > -{ > > - u32 val, exit_scanlines; > > - struct drm_i915_private *dev_priv = to_i915(cstate->uapi.crtc- > > >dev); > > - > > - if (!cstate->dc3co_exitline) > > - return; > > - > > - exit_scanlines = cstate->dc3co_exitline; > > - 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 void tgl_dc3co_exitline_compute_config(struct intel_encoder > > *encoder, > > - struct intel_crtc_state > > *cstate) > > -{ > > - u32 exit_scanlines; > > - struct drm_i915_private *dev_priv = to_i915(cstate->uapi.crtc- > > >dev); > > - u32 crtc_vdisplay = cstate->hw.adjusted_mode.crtc_vdisplay; > > - > > - cstate->dc3co_exitline = 0; > > - > > - if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)) > > - return; > > - > > - /* B.Specs:49196 DC3CO only works with pipeA and DDIA.*/ > > - if (to_intel_crtc(cstate->uapi.crtc)->pipe != PIPE_A || > > - encoder->port != PORT_A) > > - return; > > - > > - if (!cstate->has_psr2 || !cstate->hw.active) > > - return; > > - > > - /* > > - * DC3CO Exit time 200us B.Spec 49196 > > - * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line > > time) + 1 > > - */ > > - exit_scanlines = > > - intel_usecs_to_scanlines(&cstate->hw.adjusted_mode, > > 200) + 1; > > - > > - if (WARN_ON(exit_scanlines > crtc_vdisplay)) > > - return; > > - > > - cstate->dc3co_exitline = crtc_vdisplay - exit_scanlines; > > - DRM_DEBUG_KMS("DC3CO exit scanlines %d\n", cstate- > > >dc3co_exitline); > > -} > > - > > -static void tgl_dc3co_exitline_get_config(struct intel_crtc_state > > *crtc_state) > > -{ > > - u32 val; > > - struct drm_i915_private *dev_priv = to_i915(crtc_state- > > >uapi.crtc->dev); > > - > > - if (INTEL_GEN(dev_priv) < 12) > > - return; > > - > > - val = I915_READ(EXITLINE(crtc_state->cpu_transcoder)); > > - > > - if (val & EXITLINE_ENABLE) > > - crtc_state->dc3co_exitline = val & EXITLINE_MASK; > > -} > > - > > static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder, > > const struct intel_crtc_state > > *crtc_state, > > const struct drm_connector_state > > *conn_state) > > @@ -3436,7 +3356,6 @@ static void tgl_ddi_pre_enable_dp(struct > > intel_encoder *encoder, > > int level = intel_ddi_dp_level(intel_dp); > > enum transcoder transcoder = crtc_state->cpu_transcoder; > > > > - tgl_set_psr2_transcoder_exitline(crtc_state); > > intel_dp_set_link_params(intel_dp, crtc_state->port_clock, > > crtc_state->lane_count, is_mst); > > > > @@ -3822,7 +3741,6 @@ static void intel_ddi_post_disable_dp(struct > > intel_encoder *encoder, > > dig_port- > > >ddi_io_power_domain); > > > > intel_ddi_clk_disable(encoder); > > - tgl_clear_psr2_transcoder_exitline(old_crtc_state); > > } > > > > static void intel_ddi_post_disable_hdmi(struct intel_encoder > > *encoder, > > @@ -4406,9 +4324,6 @@ void intel_ddi_get_config(struct > > intel_encoder *encoder, > > break; > > } > > > > - if (encoder->type == INTEL_OUTPUT_EDP) > > - tgl_dc3co_exitline_get_config(pipe_config); > > - > > pipe_config->has_audio = > > intel_ddi_is_audio_enabled(dev_priv, cpu_transcoder); > > > > @@ -4490,7 +4405,6 @@ static int intel_ddi_compute_config(struct > > intel_encoder *encoder, > > ret = intel_hdmi_compute_config(encoder, pipe_config, > > conn_state); > > } else { > > ret = intel_dp_compute_config(encoder, pipe_config, > > conn_state); > > - tgl_dc3co_exitline_compute_config(encoder, > > pipe_config); > > } > > > > if (ret) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > > b/drivers/gpu/drm/i915/display/intel_display.c > > index 427a2a4e4ce6..0eddcdef3cad 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -13578,7 +13578,6 @@ intel_pipe_config_compare(const struct > > intel_crtc_state *current_config, > > > > PIPE_CONF_CHECK_I(pixel_multiplier); > > PIPE_CONF_CHECK_I(output_format); > > - PIPE_CONF_CHECK_I(dc3co_exitline); > > PIPE_CONF_CHECK_BOOL(has_hdmi_sink); > > if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) || > > IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index fdd7f93953b1..eb3d10036bef 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -600,6 +600,37 @@ static void > > tgl_disallow_dc3co_on_psr2_exit(struct drm_i915_private *dev_priv) > > tgl_psr2_disable_dc3co(dev_priv); > > } > > > > +static void > > +tgl_dc3co_exitline_compute_config(struct intel_dp *intel_dp, > > + struct intel_crtc_state *crtc_state) > > +{ > > + const u32 crtc_vdisplay = crtc_state- > > >uapi.adjusted_mode.crtc_vdisplay; > As there is splitup in uapi and hw, shall we use hw.adjusted_mode, > (one more place below in this function) > I am not sure about it, what is ur opinion on it? > Rest of patch looks good to me, with above comment.
For eDP/PSR code it do not matters which one we use, it is used to handle HW state in the special modes where 2 or more pipes joins to output in 8k+ displays. So going to keep it as is to avoid more changes. Thnaks for the reviews. > Reviewed-by: Anshuman Gupta <[email protected]> > > + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > + u32 exit_scanlines; > > + > > + if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO)) > > + return; > > + > > + /* B.Specs:49196 DC3CO only works with pipeA and DDIA.*/ > > + if (to_intel_crtc(crtc_state->uapi.crtc)->pipe != PIPE_A || > > + dig_port->base.port != PORT_A) > > + return; > > + > > + /* > > + * DC3CO Exit time 200us B.Spec 49196 > > + * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line > > time) + 1 > > + */ > > + exit_scanlines = > > + intel_usecs_to_scanlines(&crtc_state- > > >uapi.adjusted_mode, 200) + 1; > > + > > + if (WARN_ON(exit_scanlines > crtc_vdisplay)) > > + return; > > + > > + crtc_state->dc3co_exitline = crtc_vdisplay - exit_scanlines; > > + DRM_DEBUG_KMS("DC3CO exit scanlines %d\n", crtc_state- > > >dc3co_exitline); > > +} > > + > > static bool intel_psr2_config_valid(struct intel_dp *intel_dp, > > struct intel_crtc_state > > *crtc_state) > > { > > @@ -671,6 +702,7 @@ static bool intel_psr2_config_valid(struct > > intel_dp *intel_dp, > > return false; > > } > > > > + tgl_dc3co_exitline_compute_config(intel_dp, crtc_state); > > return true; > > } > > > > @@ -788,6 +820,20 @@ static void intel_psr_enable_source(struct > > intel_dp *intel_dp, > > I915_WRITE(EDP_PSR_DEBUG(dev_priv->psr.transcoder), mask); > > > > psr_irq_control(dev_priv); > > + > > + if (crtc_state->dc3co_exitline) { > > + u32 val; > > + > > + /* > > + * TODO: if future platforms supports DC3CO in more > > than one > > + * transcoder, EXITLINE will need to be unset when > > disabling PSR > > + */ > > + val = I915_READ(EXITLINE(cpu_transcoder)); > > + val &= ~EXITLINE_MASK; > > + val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT; > > + val |= EXITLINE_ENABLE; > > + I915_WRITE(EXITLINE(cpu_transcoder), val); > > + } > > } > > > > static void intel_psr_enable_locked(struct drm_i915_private > > *dev_priv, > > -- > > 2.25.0 > > _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
