* [Intel-gfx] [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changed
2019-11-01 0:14 ` [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changed José Roberto de Souza
@ 2019-11-01 0:14 ` José Roberto de Souza
2019-11-13 11:38 ` Anshuamn Gupta
2019-11-17 12:53 ` Anshuamn Gupta
2 siblings, 0 replies; 24+ messages in thread
From: José Roberto de Souza @ 2019-11-01 0:14 UTC (permalink / raw)
To: intel-gfx
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.
BSpec: 49196
Cc: Imre Deak <imre.deak@intel.com>
Cc: Anshuman Gupta <anshuman.gupta@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
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 | 47 +++++++++++
3 files changed, 47 insertions(+), 87 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index b51f244ad7a5..f52fb7619d46 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3339,86 +3339,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->base.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->base.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->base.crtc->dev);
- u32 crtc_vdisplay = cstate->base.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->base.crtc)->pipe != PIPE_A ||
- encoder->port != PORT_A)
- return;
-
- if (!cstate->has_psr2 || !cstate->base.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->base.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->base.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)
@@ -3431,7 +3351,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);
@@ -3760,7 +3679,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,
@@ -4308,9 +4226,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);
@@ -4392,7 +4307,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 348ce0456696..6c1c93b2a81f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -13091,7 +13091,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 bb9b5349b72a..06a0a1c9621e 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -596,6 +596,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->base.adjusted_mode.crtc_vdisplay;
+ 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->base.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->base.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)
{
@@ -658,6 +689,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;
}
@@ -775,6 +807,21 @@ 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 (INTEL_GEN(dev_priv) >= 12 && dev_priv->psr.psr2_enabled) {
+ 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 | EXITLINE_ENABLE);
+ val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
+ if (crtc_state->dc3co_exitline)
+ val |= EXITLINE_ENABLE;
+ I915_WRITE(EXITLINE(cpu_transcoder), val);
+ }
}
static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changed
2019-11-01 0:14 ` [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changed José Roberto de Souza
2019-11-01 0:14 ` [Intel-gfx] " José Roberto de Souza
@ 2019-11-13 11:38 ` Anshuamn Gupta
2019-11-13 11:38 ` [Intel-gfx] " Anshuamn Gupta
2019-11-13 12:00 ` Imre Deak
2019-11-17 12:53 ` Anshuamn Gupta
2 siblings, 2 replies; 24+ messages in thread
From: Anshuamn Gupta @ 2019-11-13 11:38 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
On 2019-10-31 at 17:14:22 -0700, 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.
>
> BSpec: 49196
I am not able to find it explicitly that we can change
EXITLINE while transcoder is enabled in index 49196.
Which change actually u are referring?
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> 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 | 47 +++++++++++
> 3 files changed, 47 insertions(+), 87 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index b51f244ad7a5..f52fb7619d46 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3339,86 +3339,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->base.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->base.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->base.crtc->dev);
> - u32 crtc_vdisplay = cstate->base.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->base.crtc)->pipe != PIPE_A ||
> - encoder->port != PORT_A)
> - return;
> -
> - if (!cstate->has_psr2 || !cstate->base.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->base.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->base.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)
> @@ -3431,7 +3351,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);
>
> @@ -3760,7 +3679,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,
> @@ -4308,9 +4226,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);
>
> @@ -4392,7 +4307,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 348ce0456696..6c1c93b2a81f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13091,7 +13091,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 bb9b5349b72a..06a0a1c9621e 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -596,6 +596,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->base.adjusted_mode.crtc_vdisplay;
> + 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->base.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->base.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)
> {
> @@ -658,6 +689,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;
> }
>
> @@ -775,6 +807,21 @@ 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 (INTEL_GEN(dev_priv) >= 12 && dev_priv->psr.psr2_enabled) {
> + 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 | EXITLINE_ENABLE);
> + val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
> + if (crtc_state->dc3co_exitline)
> + val |= EXITLINE_ENABLE;
> + I915_WRITE(EXITLINE(cpu_transcoder), val);
> + }
> }
>
> static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> --
> 2.23.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Intel-gfx] [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changed
2019-11-13 11:38 ` Anshuamn Gupta
@ 2019-11-13 11:38 ` Anshuamn Gupta
2019-11-13 12:00 ` Imre Deak
1 sibling, 0 replies; 24+ messages in thread
From: Anshuamn Gupta @ 2019-11-13 11:38 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
On 2019-10-31 at 17:14:22 -0700, 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.
>
> BSpec: 49196
I am not able to find it explicitly that we can change
EXITLINE while transcoder is enabled in index 49196.
Which change actually u are referring?
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> 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 | 47 +++++++++++
> 3 files changed, 47 insertions(+), 87 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index b51f244ad7a5..f52fb7619d46 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3339,86 +3339,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->base.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->base.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->base.crtc->dev);
> - u32 crtc_vdisplay = cstate->base.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->base.crtc)->pipe != PIPE_A ||
> - encoder->port != PORT_A)
> - return;
> -
> - if (!cstate->has_psr2 || !cstate->base.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->base.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->base.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)
> @@ -3431,7 +3351,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);
>
> @@ -3760,7 +3679,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,
> @@ -4308,9 +4226,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);
>
> @@ -4392,7 +4307,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 348ce0456696..6c1c93b2a81f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13091,7 +13091,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 bb9b5349b72a..06a0a1c9621e 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -596,6 +596,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->base.adjusted_mode.crtc_vdisplay;
> + 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->base.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->base.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)
> {
> @@ -658,6 +689,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;
> }
>
> @@ -775,6 +807,21 @@ 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 (INTEL_GEN(dev_priv) >= 12 && dev_priv->psr.psr2_enabled) {
> + 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 | EXITLINE_ENABLE);
> + val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
> + if (crtc_state->dc3co_exitline)
> + val |= EXITLINE_ENABLE;
> + I915_WRITE(EXITLINE(cpu_transcoder), val);
> + }
> }
>
> static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> --
> 2.23.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changed
2019-11-13 11:38 ` Anshuamn Gupta
2019-11-13 11:38 ` [Intel-gfx] " Anshuamn Gupta
@ 2019-11-13 12:00 ` Imre Deak
2019-11-13 12:00 ` [Intel-gfx] " Imre Deak
1 sibling, 1 reply; 24+ messages in thread
From: Imre Deak @ 2019-11-13 12:00 UTC (permalink / raw)
To: Anshuamn Gupta; +Cc: intel-gfx
On Wed, Nov 13, 2019 at 05:08:21PM +0530, Anshuamn Gupta wrote:
> On 2019-10-31 at 17:14:22 -0700, 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.
> >
> > BSpec: 49196
>
> I am not able to find it explicitly that we can change EXITLINE while
> transcoder is enabled in index 49196. Which change actually u are
> referring?
Looks like a recent change at
https://gfxspecs.intel.com/Predator/Home/Index/50497
explicitly allows programming it anytime before DC3co is enabled.
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > 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 | 47 +++++++++++
> > 3 files changed, 47 insertions(+), 87 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index b51f244ad7a5..f52fb7619d46 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3339,86 +3339,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->base.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->base.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->base.crtc->dev);
> > - u32 crtc_vdisplay = cstate->base.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->base.crtc)->pipe != PIPE_A ||
> > - encoder->port != PORT_A)
> > - return;
> > -
> > - if (!cstate->has_psr2 || !cstate->base.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->base.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->base.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)
> > @@ -3431,7 +3351,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);
> >
> > @@ -3760,7 +3679,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,
> > @@ -4308,9 +4226,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);
> >
> > @@ -4392,7 +4307,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 348ce0456696..6c1c93b2a81f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -13091,7 +13091,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 bb9b5349b72a..06a0a1c9621e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -596,6 +596,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->base.adjusted_mode.crtc_vdisplay;
> > + 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->base.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->base.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)
> > {
> > @@ -658,6 +689,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;
> > }
> >
> > @@ -775,6 +807,21 @@ 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 (INTEL_GEN(dev_priv) >= 12 && dev_priv->psr.psr2_enabled) {
> > + 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 | EXITLINE_ENABLE);
> > + val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
> > + if (crtc_state->dc3co_exitline)
> > + val |= EXITLINE_ENABLE;
> > + I915_WRITE(EXITLINE(cpu_transcoder), val);
> > + }
> > }
> >
> > static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> > --
> > 2.23.0
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Intel-gfx] [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changed
2019-11-13 12:00 ` Imre Deak
@ 2019-11-13 12:00 ` Imre Deak
0 siblings, 0 replies; 24+ messages in thread
From: Imre Deak @ 2019-11-13 12:00 UTC (permalink / raw)
To: Anshuamn Gupta; +Cc: intel-gfx
On Wed, Nov 13, 2019 at 05:08:21PM +0530, Anshuamn Gupta wrote:
> On 2019-10-31 at 17:14:22 -0700, 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.
> >
> > BSpec: 49196
>
> I am not able to find it explicitly that we can change EXITLINE while
> transcoder is enabled in index 49196. Which change actually u are
> referring?
Looks like a recent change at
https://gfxspecs.intel.com/Predator/Home/Index/50497
explicitly allows programming it anytime before DC3co is enabled.
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > 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 | 47 +++++++++++
> > 3 files changed, 47 insertions(+), 87 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index b51f244ad7a5..f52fb7619d46 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3339,86 +3339,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->base.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->base.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->base.crtc->dev);
> > - u32 crtc_vdisplay = cstate->base.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->base.crtc)->pipe != PIPE_A ||
> > - encoder->port != PORT_A)
> > - return;
> > -
> > - if (!cstate->has_psr2 || !cstate->base.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->base.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->base.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)
> > @@ -3431,7 +3351,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);
> >
> > @@ -3760,7 +3679,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,
> > @@ -4308,9 +4226,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);
> >
> > @@ -4392,7 +4307,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 348ce0456696..6c1c93b2a81f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -13091,7 +13091,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 bb9b5349b72a..06a0a1c9621e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -596,6 +596,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->base.adjusted_mode.crtc_vdisplay;
> > + 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->base.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->base.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)
> > {
> > @@ -658,6 +689,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;
> > }
> >
> > @@ -775,6 +807,21 @@ 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 (INTEL_GEN(dev_priv) >= 12 && dev_priv->psr.psr2_enabled) {
> > + 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 | EXITLINE_ENABLE);
> > + val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
> > + if (crtc_state->dc3co_exitline)
> > + val |= EXITLINE_ENABLE;
> > + I915_WRITE(EXITLINE(cpu_transcoder), val);
> > + }
> > }
> >
> > static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> > --
> > 2.23.0
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changed
2019-11-01 0:14 ` [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changed José Roberto de Souza
2019-11-01 0:14 ` [Intel-gfx] " José Roberto de Souza
2019-11-13 11:38 ` Anshuamn Gupta
@ 2019-11-17 12:53 ` Anshuamn Gupta
2019-11-17 12:53 ` [Intel-gfx] " Anshuamn Gupta
2019-11-18 18:42 ` Souza, Jose
2 siblings, 2 replies; 24+ messages in thread
From: Anshuamn Gupta @ 2019-11-17 12:53 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
On 2019-10-31 at 17:14:22 -0700, 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.
>
> BSpec: 49196
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> 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 | 47 +++++++++++
> 3 files changed, 47 insertions(+), 87 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index b51f244ad7a5..f52fb7619d46 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3339,86 +3339,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->base.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->base.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->base.crtc->dev);
> - u32 crtc_vdisplay = cstate->base.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->base.crtc)->pipe != PIPE_A ||
> - encoder->port != PORT_A)
> - return;
> -
> - if (!cstate->has_psr2 || !cstate->base.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->base.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->base.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)
> @@ -3431,7 +3351,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);
>
> @@ -3760,7 +3679,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,
> @@ -4308,9 +4226,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);
>
> @@ -4392,7 +4307,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 348ce0456696..6c1c93b2a81f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13091,7 +13091,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 bb9b5349b72a..06a0a1c9621e 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -596,6 +596,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->base.adjusted_mode.crtc_vdisplay;
> + 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;
We wanted to explicitly clear dc3co_exitline in case it is not
supported.
> +
> + 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->base.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->base.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)
> {
> @@ -658,6 +689,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;
> }
>
> @@ -775,6 +807,21 @@ 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 (INTEL_GEN(dev_priv) >= 12 && dev_priv->psr.psr2_enabled) {
IMHO we can use dc3co_enabled flag here instead of psr2_enabled, and it would be
nice to have a seperate function to set the exitline here.
Thanks,
Anshuman Gupta.
> + 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 | EXITLINE_ENABLE);
> + val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
> + if (crtc_state->dc3co_exitline)
> + val |= EXITLINE_ENABLE;
> + I915_WRITE(EXITLINE(cpu_transcoder), val);
> + }
> }
>
> static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> --
> 2.23.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Intel-gfx] [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changed
2019-11-17 12:53 ` Anshuamn Gupta
@ 2019-11-17 12:53 ` Anshuamn Gupta
2019-11-18 18:42 ` Souza, Jose
1 sibling, 0 replies; 24+ messages in thread
From: Anshuamn Gupta @ 2019-11-17 12:53 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
On 2019-10-31 at 17:14:22 -0700, 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.
>
> BSpec: 49196
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
> 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 | 47 +++++++++++
> 3 files changed, 47 insertions(+), 87 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index b51f244ad7a5..f52fb7619d46 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3339,86 +3339,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->base.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->base.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->base.crtc->dev);
> - u32 crtc_vdisplay = cstate->base.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->base.crtc)->pipe != PIPE_A ||
> - encoder->port != PORT_A)
> - return;
> -
> - if (!cstate->has_psr2 || !cstate->base.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->base.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->base.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)
> @@ -3431,7 +3351,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);
>
> @@ -3760,7 +3679,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,
> @@ -4308,9 +4226,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);
>
> @@ -4392,7 +4307,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 348ce0456696..6c1c93b2a81f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13091,7 +13091,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 bb9b5349b72a..06a0a1c9621e 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -596,6 +596,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->base.adjusted_mode.crtc_vdisplay;
> + 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;
We wanted to explicitly clear dc3co_exitline in case it is not
supported.
> +
> + 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->base.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->base.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)
> {
> @@ -658,6 +689,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;
> }
>
> @@ -775,6 +807,21 @@ 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 (INTEL_GEN(dev_priv) >= 12 && dev_priv->psr.psr2_enabled) {
IMHO we can use dc3co_enabled flag here instead of psr2_enabled, and it would be
nice to have a seperate function to set the exitline here.
Thanks,
Anshuman Gupta.
> + 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 | EXITLINE_ENABLE);
> + val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
> + if (crtc_state->dc3co_exitline)
> + val |= EXITLINE_ENABLE;
> + I915_WRITE(EXITLINE(cpu_transcoder), val);
> + }
> }
>
> static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
> --
> 2.23.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changed
2019-11-17 12:53 ` Anshuamn Gupta
2019-11-17 12:53 ` [Intel-gfx] " Anshuamn Gupta
@ 2019-11-18 18:42 ` Souza, Jose
2019-11-18 18:42 ` [Intel-gfx] " Souza, Jose
2019-12-30 14:21 ` [Intel-gfx] [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changedy Anshuamn Gupta
1 sibling, 2 replies; 24+ messages in thread
From: Souza, Jose @ 2019-11-18 18:42 UTC (permalink / raw)
To: Gupta, Anshuman; +Cc: intel-gfx@lists.freedesktop.org
On Sun, 2019-11-17 at 18:23 +0530, Anshuamn Gupta wrote:
> On 2019-10-31 at 17:14:22 -0700, 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.
> >
> > BSpec: 49196
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > 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 | 47 +++++++++++
> > 3 files changed, 47 insertions(+), 87 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index b51f244ad7a5..f52fb7619d46 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3339,86 +3339,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->base.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->base.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->base.crtc-
> > >dev);
> > - u32 crtc_vdisplay = cstate->base.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->base.crtc)->pipe != PIPE_A ||
> > - encoder->port != PORT_A)
> > - return;
> > -
> > - if (!cstate->has_psr2 || !cstate->base.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->base.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-
> > >base.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)
> > @@ -3431,7 +3351,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);
> >
> > @@ -3760,7 +3679,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,
> > @@ -4308,9 +4226,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);
> >
> > @@ -4392,7 +4307,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 348ce0456696..6c1c93b2a81f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -13091,7 +13091,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 bb9b5349b72a..06a0a1c9621e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -596,6 +596,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-
> > >base.adjusted_mode.crtc_vdisplay;
> > + 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;
> We wanted to explicitly clear dc3co_exitline in case it is not
> supported.
The whole intel_crtc_state is zeroed before the compute functions are
called so no need to set dc3co_exitline to zero.
> > +
> > + 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->base.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-
> > >base.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)
> > {
> > @@ -658,6 +689,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;
> > }
> >
> > @@ -775,6 +807,21 @@ 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 (INTEL_GEN(dev_priv) >= 12 && dev_priv->psr.psr2_enabled) {
> IMHO we can use dc3co_enabled flag here instead of psr2_enabled, and
> it would be
> nice to have a seperate function to set the exitline here.
> Thanks,
> Anshuman Gupta.
Here it is checking for PSR2 because in case DC3CO is disabled it will
set EXITLINE to 0 but we could keep it set too without any drawbacks.
Thoughts Imre?
I don't think we need a separated function it is just RMW into one
registers.
> > + 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 | EXITLINE_ENABLE);
> > + val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
> > + if (crtc_state->dc3co_exitline)
> > + val |= EXITLINE_ENABLE;
> > + I915_WRITE(EXITLINE(cpu_transcoder), val);
> > + }
> > }
> >
> > static void intel_psr_enable_locked(struct drm_i915_private
> > *dev_priv,
> > --
> > 2.23.0
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Intel-gfx] [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changed
2019-11-18 18:42 ` Souza, Jose
@ 2019-11-18 18:42 ` Souza, Jose
2019-12-30 14:21 ` [Intel-gfx] [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changedy Anshuamn Gupta
1 sibling, 0 replies; 24+ messages in thread
From: Souza, Jose @ 2019-11-18 18:42 UTC (permalink / raw)
To: Gupta, Anshuman; +Cc: intel-gfx@lists.freedesktop.org
On Sun, 2019-11-17 at 18:23 +0530, Anshuamn Gupta wrote:
> On 2019-10-31 at 17:14:22 -0700, 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.
> >
> > BSpec: 49196
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > 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 | 47 +++++++++++
> > 3 files changed, 47 insertions(+), 87 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index b51f244ad7a5..f52fb7619d46 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3339,86 +3339,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->base.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->base.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->base.crtc-
> > >dev);
> > - u32 crtc_vdisplay = cstate->base.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->base.crtc)->pipe != PIPE_A ||
> > - encoder->port != PORT_A)
> > - return;
> > -
> > - if (!cstate->has_psr2 || !cstate->base.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->base.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-
> > >base.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)
> > @@ -3431,7 +3351,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);
> >
> > @@ -3760,7 +3679,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,
> > @@ -4308,9 +4226,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);
> >
> > @@ -4392,7 +4307,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 348ce0456696..6c1c93b2a81f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -13091,7 +13091,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 bb9b5349b72a..06a0a1c9621e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -596,6 +596,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-
> > >base.adjusted_mode.crtc_vdisplay;
> > + 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;
> We wanted to explicitly clear dc3co_exitline in case it is not
> supported.
The whole intel_crtc_state is zeroed before the compute functions are
called so no need to set dc3co_exitline to zero.
> > +
> > + 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->base.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-
> > >base.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)
> > {
> > @@ -658,6 +689,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;
> > }
> >
> > @@ -775,6 +807,21 @@ 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 (INTEL_GEN(dev_priv) >= 12 && dev_priv->psr.psr2_enabled) {
> IMHO we can use dc3co_enabled flag here instead of psr2_enabled, and
> it would be
> nice to have a seperate function to set the exitline here.
> Thanks,
> Anshuman Gupta.
Here it is checking for PSR2 because in case DC3CO is disabled it will
set EXITLINE to 0 but we could keep it set too without any drawbacks.
Thoughts Imre?
I don't think we need a separated function it is just RMW into one
registers.
> > + 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 | EXITLINE_ENABLE);
> > + val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
> > + if (crtc_state->dc3co_exitline)
> > + val |= EXITLINE_ENABLE;
> > + I915_WRITE(EXITLINE(cpu_transcoder), val);
> > + }
> > }
> >
> > static void intel_psr_enable_locked(struct drm_i915_private
> > *dev_priv,
> > --
> > 2.23.0
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Intel-gfx] [PATCH 3/3] drm/i915/dc3co: Avoid full modeset when EXITLINE needs to be changedy
2019-11-18 18:42 ` Souza, Jose
2019-11-18 18:42 ` [Intel-gfx] " Souza, Jose
@ 2019-12-30 14:21 ` Anshuamn Gupta
1 sibling, 0 replies; 24+ messages in thread
From: Anshuamn Gupta @ 2019-12-30 14:21 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org
On 2019-11-19 at 00:12:35 +0530, Souza, Jose wrote:
> On Sun, 2019-11-17 at 18:23 +0530, Anshuamn Gupta wrote:
> > On 2019-10-31 at 17:14:22 -0700, 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.
> > >
> > > BSpec: 49196
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > > 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 | 47 +++++++++++
> > > 3 files changed, 47 insertions(+), 87 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index b51f244ad7a5..f52fb7619d46 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -3339,86 +3339,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->base.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->base.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->base.crtc-
> > > >dev);
> > > - u32 crtc_vdisplay = cstate->base.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->base.crtc)->pipe != PIPE_A ||
> > > - encoder->port != PORT_A)
> > > - return;
> > > -
> > > - if (!cstate->has_psr2 || !cstate->base.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->base.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-
> > > >base.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)
> > > @@ -3431,7 +3351,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);
> > >
> > > @@ -3760,7 +3679,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,
> > > @@ -4308,9 +4226,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);
> > >
> > > @@ -4392,7 +4307,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 348ce0456696..6c1c93b2a81f 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -13091,7 +13091,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 bb9b5349b72a..06a0a1c9621e 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -596,6 +596,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-
> > > >base.adjusted_mode.crtc_vdisplay;
> > > + 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;
> > We wanted to explicitly clear dc3co_exitline in case it is not
> > supported.
>
>
> The whole intel_crtc_state is zeroed before the compute functions are
> called so no need to set dc3co_exitline to zero.
>
> > > +
> > > + 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->base.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-
> > > >base.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)
> > > {
> > > @@ -658,6 +689,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;
> > > }
> > >
> > > @@ -775,6 +807,21 @@ 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 (INTEL_GEN(dev_priv) >= 12 && dev_priv->psr.psr2_enabled) {
> > IMHO we can use dc3co_enabled flag here instead of psr2_enabled, and
> > it would be
> > nice to have a seperate function to set the exitline here.
> > Thanks,
> > Anshuman Gupta.
>
> Here it is checking for PSR2 because in case DC3CO is disabled it will
> set EXITLINE to 0 but we could keep it set too without any drawbacks.
IMHO EXITLINE is only used to exit the DC3CO until a programmed exit line event,
this optimally align the frame update command position within the
repeated video frame. so there is no use case of EXITLINE without DC3CO.
Thanks ,
Anshuman Gupta.
> Thoughts Imre?
>
> I don't think we need a separated function it is just RMW into one
> registers.
>
> > > + 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 | EXITLINE_ENABLE);
> > > + val |= crtc_state->dc3co_exitline << EXITLINE_SHIFT;
> > > + if (crtc_state->dc3co_exitline)
> > > + val |= EXITLINE_ENABLE;
> > > + I915_WRITE(EXITLINE(cpu_transcoder), val);
> > > + }
> > > }
> > >
> > > static void intel_psr_enable_locked(struct drm_i915_private
> > > *dev_priv,
> > > --
> > > 2.23.0
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread