[v8,4/7] drm/i915/tgl: Do modeset to enable and configure DC3CO exitline.

Submitted by Anshuman Gupta on Sept. 13, 2019, 8:23 a.m.

Details

Message ID 20190913082339.1785-5-anshuman.gupta@intel.com
State New
Headers show
Series "DC3CO Support for TGL" ( rev: 9 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Anshuman Gupta Sept. 13, 2019, 8:23 a.m.
DC3CO enabling B.Specs sequence requires to enable end configure
exit scanlines to TRANS_EXITLINE register, programming this register
has to be part of modeset sequence as this can't be change when
transcoder or port is enabled.
When system boots with only eDP panel there may not be real
modeset as BIOS has already programmed the necessary registers,
therefore it needs to force a modeset to enable and configure
DC3CO exitline.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |   5 +
 .../drm/i915/display/intel_display_power.c    | 104 ++++++++++++++++++
 .../drm/i915/display/intel_display_power.h    |   7 ++
 .../drm/i915/display/intel_display_types.h    |   1 +
 drivers/gpu/drm/i915/intel_pm.c               |   2 +-
 drivers/gpu/drm/i915/intel_pm.h               |   2 +
 6 files changed, 120 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a19f8c73f2e0..6b7b8d2112a5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7409,6 +7409,8 @@  static int intel_crtc_compute_config(struct intel_crtc *crtc,
 	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	int clock_limit = dev_priv->max_dotclk_freq;
 
+	tgl_dc3co_crtc_compute_config(dev_priv, pipe_config);
+
 	if (INTEL_GEN(dev_priv) < 4) {
 		clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
 
@@ -10474,6 +10476,8 @@  static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		pipe_config->pixel_multiplier = 1;
 	}
 
+	tgl_dc3co_crtc_get_config(pipe_config);
+
 out:
 	for_each_power_domain(power_domain, power_domain_mask)
 		intel_display_power_put(dev_priv,
@@ -12739,6 +12743,7 @@  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_BOOL(has_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_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 7965e07257a0..42eabcdecf00 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -19,6 +19,7 @@ 
 #include "intel_hotplug.h"
 #include "intel_sideband.h"
 #include "intel_tc.h"
+#include "intel_pm.h"
 
 bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
 					 enum i915_power_well_id power_well_id);
@@ -772,6 +773,109 @@  static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
 	dev_priv->csr.dc_state = val & mask;
 }
 
+void tgl_disable_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->has_dc3co_exitline)
+		return;
+
+	val = I915_READ(EXITLINE(cstate->cpu_transcoder));
+	val &= ~EXITLINE_ENABLE;
+	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
+}
+
+void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
+{
+	u32 linetime_us, val, exit_scanlines;
+	u32 crtc_vdisplay = cstate->base.adjusted_mode.crtc_vdisplay;
+	struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
+
+	if (!cstate->has_dc3co_exitline)
+		return;
+
+	linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(cstate));
+	if (WARN_ON(!linetime_us))
+		return;
+	/*
+	 * DC3CO Exit time 200us B.Spec 49196
+	 * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line time) + 1
+	 * Exit line event need to program above calculated scan lines before
+	 * next VBLANK.
+	 */
+	exit_scanlines = DIV_ROUND_UP(200, linetime_us) + 1;
+	if (WARN_ON(exit_scanlines > crtc_vdisplay))
+		return;
+
+	exit_scanlines = crtc_vdisplay - exit_scanlines;
+	exit_scanlines <<= EXITLINE_SHIFT;
+	val = I915_READ(EXITLINE(cstate->cpu_transcoder));
+	val &= ~(EXITLINE_MASK | EXITLINE_ENABLE);
+	val |= exit_scanlines;
+	val |= EXITLINE_ENABLE;
+	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
+}
+
+static bool tgl_dc3co_is_edp_connected(struct intel_crtc_state  *crtc_state)
+{
+	struct drm_atomic_state *state = crtc_state->base.state;
+	struct drm_connector *connector;
+	struct drm_connector_state *connector_state;
+	int i;
+
+	for_each_new_connector_in_state(state, connector, connector_state, i) {
+		if (connector->status == connector_status_connected &&
+		    connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
+/*
+ * DC3CO requires to enable exitline and program DC3CO requires
+ * exit scanlines to TRANS_EXITLINE register, which should only
+ * change before transcoder or port are enabled.
+ * This requires to disable the fastset at boot for eDP output.
+ */
+void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
+				   struct intel_crtc_state *crtc_state)
+{
+	if (!IS_TIGERLAKE(dev_priv))
+		return;
+
+	if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
+		return;
+
+	if (crtc_state->has_psr2 && crtc_state->base.active) {
+		if (tgl_dc3co_is_edp_connected(crtc_state))
+			crtc_state->has_dc3co_exitline = true;
+	}
+}
+
+void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
+{
+	u32 val;
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+
+	if (!IS_TIGERLAKE(dev_priv))
+		return;
+
+	if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
+		return;
+
+	/* DC3CO and PSR2 is supported only on TRANSCODER_A */
+	if (crtc_state->cpu_transcoder != TRANSCODER_A)
+		return;
+
+	val = I915_READ(EXITLINE(crtc_state->cpu_transcoder));
+
+	if (val & EXITLINE_ENABLE)
+		crtc_state->has_dc3co_exitline = true;
+}
+
 static void
 allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
index 13fc705799fd..bf68f26a5a37 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.h
+++ b/drivers/gpu/drm/i915/display/intel_display_power.h
@@ -12,6 +12,8 @@ 
 
 struct drm_i915_private;
 struct intel_encoder;
+struct intel_crtc_state;
+struct intel_atomic_state;
 
 enum intel_display_power_domain {
 	POWER_DOMAIN_DISPLAY_CORE,
@@ -258,6 +260,11 @@  void intel_display_power_resume_early(struct drm_i915_private *i915);
 void intel_display_power_suspend(struct drm_i915_private *i915);
 void intel_display_power_resume(struct drm_i915_private *i915);
 void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state);
+void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
+				   struct intel_crtc_state *crtc_state);
+void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state);
+void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
+void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
 
 const char *
 intel_display_power_domain_str(enum intel_display_power_domain domain);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index d5cc4b810d9e..22f8fe0a34d0 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -871,6 +871,7 @@  struct intel_crtc_state {
 
 	bool has_psr;
 	bool has_psr2;
+	bool has_dc3co_exitline;
 
 	/*
 	 * Frequence the dpll for the port should run at. Differs from the
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d0ceb272551f..5679d1c1d560 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4588,7 +4588,7 @@  skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency,
 	return ret;
 }
 
-static uint_fixed_16_16_t
+uint_fixed_16_16_t
 intel_get_linetime_us(const struct intel_crtc_state *crtc_state)
 {
 	u32 pixel_rate;
diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
index e3573e1e16e3..454e92c06dff 100644
--- a/drivers/gpu/drm/i915/intel_pm.h
+++ b/drivers/gpu/drm/i915/intel_pm.h
@@ -8,6 +8,7 @@ 
 
 #include <linux/types.h>
 
+#include "i915_drv.h"
 #include "i915_reg.h"
 
 struct drm_device;
@@ -76,6 +77,7 @@  u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, i915_reg_t reg);
 u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, i915_reg_t reg);
 
 u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1);
+uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *cstate);
 
 unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
 unsigned long i915_mch_val(struct drm_i915_private *dev_priv);

Comments

On Fri, Sep 13, 2019 at 01:53:36PM +0530, Anshuman Gupta wrote:
> DC3CO enabling B.Specs sequence requires to enable end configure
> exit scanlines to TRANS_EXITLINE register, programming this register
> has to be part of modeset sequence as this can't be change when
> transcoder or port is enabled.
> When system boots with only eDP panel there may not be real
> modeset as BIOS has already programmed the necessary registers,
> therefore it needs to force a modeset to enable and configure
> DC3CO exitline.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Animesh Manna <animesh.manna@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>

You missed my review comments on v7:
- Computing the state should be done from a DP encoder compute config
  hook, which will then make tgl_dc3co_is_edp_connected() redundant.
- Instead of has_dc3co_exitline dc3co_exitline should be computed (and
  read out), which could be used then when we need to program it.

> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |   5 +
>  .../drm/i915/display/intel_display_power.c    | 104 ++++++++++++++++++
>  .../drm/i915/display/intel_display_power.h    |   7 ++
>  .../drm/i915/display/intel_display_types.h    |   1 +
>  drivers/gpu/drm/i915/intel_pm.c               |   2 +-
>  drivers/gpu/drm/i915/intel_pm.h               |   2 +
>  6 files changed, 120 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index a19f8c73f2e0..6b7b8d2112a5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7409,6 +7409,8 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	int clock_limit = dev_priv->max_dotclk_freq;
>  
> +	tgl_dc3co_crtc_compute_config(dev_priv, pipe_config);
> +
>  	if (INTEL_GEN(dev_priv) < 4) {
>  		clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
>  
> @@ -10474,6 +10476,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		pipe_config->pixel_multiplier = 1;
>  	}
>  
> +	tgl_dc3co_crtc_get_config(pipe_config);
> +
>  out:
>  	for_each_power_domain(power_domain, power_domain_mask)
>  		intel_display_power_put(dev_priv,
> @@ -12739,6 +12743,7 @@ 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_BOOL(has_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_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 7965e07257a0..42eabcdecf00 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -19,6 +19,7 @@
>  #include "intel_hotplug.h"
>  #include "intel_sideband.h"
>  #include "intel_tc.h"
> +#include "intel_pm.h"
>  
>  bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
>  					 enum i915_power_well_id power_well_id);
> @@ -772,6 +773,109 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
>  	dev_priv->csr.dc_state = val & mask;
>  }
>  
> +void tgl_disable_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->has_dc3co_exitline)
> +		return;
> +
> +	val = I915_READ(EXITLINE(cstate->cpu_transcoder));
> +	val &= ~EXITLINE_ENABLE;
> +	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> +}
> +
> +void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> +{
> +	u32 linetime_us, val, exit_scanlines;
> +	u32 crtc_vdisplay = cstate->base.adjusted_mode.crtc_vdisplay;
> +	struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> +
> +	if (!cstate->has_dc3co_exitline)
> +		return;
> +
> +	linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(cstate));
> +	if (WARN_ON(!linetime_us))
> +		return;
> +	/*
> +	 * DC3CO Exit time 200us B.Spec 49196
> +	 * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line time) + 1
> +	 * Exit line event need to program above calculated scan lines before
> +	 * next VBLANK.
> +	 */
> +	exit_scanlines = DIV_ROUND_UP(200, linetime_us) + 1;
> +	if (WARN_ON(exit_scanlines > crtc_vdisplay))
> +		return;
> +
> +	exit_scanlines = crtc_vdisplay - exit_scanlines;
> +	exit_scanlines <<= EXITLINE_SHIFT;
> +	val = I915_READ(EXITLINE(cstate->cpu_transcoder));
> +	val &= ~(EXITLINE_MASK | EXITLINE_ENABLE);
> +	val |= exit_scanlines;
> +	val |= EXITLINE_ENABLE;
> +	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> +}
> +
> +static bool tgl_dc3co_is_edp_connected(struct intel_crtc_state  *crtc_state)
> +{
> +	struct drm_atomic_state *state = crtc_state->base.state;
> +	struct drm_connector *connector;
> +	struct drm_connector_state *connector_state;
> +	int i;
> +
> +	for_each_new_connector_in_state(state, connector, connector_state, i) {
> +		if (connector->status == connector_status_connected &&
> +		    connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
> +/*
> + * DC3CO requires to enable exitline and program DC3CO requires
> + * exit scanlines to TRANS_EXITLINE register, which should only
> + * change before transcoder or port are enabled.
> + * This requires to disable the fastset at boot for eDP output.
> + */
> +void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> +				   struct intel_crtc_state *crtc_state)
> +{
> +	if (!IS_TIGERLAKE(dev_priv))
> +		return;
> +
> +	if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
> +		return;
> +
> +	if (crtc_state->has_psr2 && crtc_state->base.active) {
> +		if (tgl_dc3co_is_edp_connected(crtc_state))
> +			crtc_state->has_dc3co_exitline = true;
> +	}
> +}
> +
> +void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
> +{
> +	u32 val;
> +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> +
> +	if (!IS_TIGERLAKE(dev_priv))
> +		return;
> +
> +	if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
> +		return;
> +
> +	/* DC3CO and PSR2 is supported only on TRANSCODER_A */
> +	if (crtc_state->cpu_transcoder != TRANSCODER_A)
> +		return;
> +
> +	val = I915_READ(EXITLINE(crtc_state->cpu_transcoder));
> +
> +	if (val & EXITLINE_ENABLE)
> +		crtc_state->has_dc3co_exitline = true;
> +}
> +
>  static void
>  allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> index 13fc705799fd..bf68f26a5a37 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -12,6 +12,8 @@
>  
>  struct drm_i915_private;
>  struct intel_encoder;
> +struct intel_crtc_state;
> +struct intel_atomic_state;
>  
>  enum intel_display_power_domain {
>  	POWER_DOMAIN_DISPLAY_CORE,
> @@ -258,6 +260,11 @@ void intel_display_power_resume_early(struct drm_i915_private *i915);
>  void intel_display_power_suspend(struct drm_i915_private *i915);
>  void intel_display_power_resume(struct drm_i915_private *i915);
>  void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state);
> +void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> +				   struct intel_crtc_state *crtc_state);
> +void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state);
> +void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> +void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
>  
>  const char *
>  intel_display_power_domain_str(enum intel_display_power_domain domain);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index d5cc4b810d9e..22f8fe0a34d0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -871,6 +871,7 @@ struct intel_crtc_state {
>  
>  	bool has_psr;
>  	bool has_psr2;
> +	bool has_dc3co_exitline;
>  
>  	/*
>  	 * Frequence the dpll for the port should run at. Differs from the
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d0ceb272551f..5679d1c1d560 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4588,7 +4588,7 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency,
>  	return ret;
>  }
>  
> -static uint_fixed_16_16_t
> +uint_fixed_16_16_t
>  intel_get_linetime_us(const struct intel_crtc_state *crtc_state)
>  {
>  	u32 pixel_rate;
> diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> index e3573e1e16e3..454e92c06dff 100644
> --- a/drivers/gpu/drm/i915/intel_pm.h
> +++ b/drivers/gpu/drm/i915/intel_pm.h
> @@ -8,6 +8,7 @@
>  
>  #include <linux/types.h>
>  
> +#include "i915_drv.h"
>  #include "i915_reg.h"
>  
>  struct drm_device;
> @@ -76,6 +77,7 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, i915_reg_t reg);
>  u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, i915_reg_t reg);
>  
>  u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1);
> +uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *cstate);
>  
>  unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
>  unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
> -- 
> 2.21.0
>
On 2019-09-23 at 19:26:56 +0300, Imre Deak wrote:
> On Fri, Sep 13, 2019 at 01:53:36PM +0530, Anshuman Gupta wrote:
> > DC3CO enabling B.Specs sequence requires to enable end configure
> > exit scanlines to TRANS_EXITLINE register, programming this register
> > has to be part of modeset sequence as this can't be change when
> > transcoder or port is enabled.
> > When system boots with only eDP panel there may not be real
> > modeset as BIOS has already programmed the necessary registers,
> > therefore it needs to force a modeset to enable and configure
> > DC3CO exitline.
> > 
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> 
> You missed my review comments on v7:
Actually i found this e-mail including earlier v7 couple of review e-mails
received in spam folder, therefore gets unnoticed.
I will fix these comments for next version.
> - Computing the state should be done from a DP encoder compute config
>   hook, which will then make tgl_dc3co_is_edp_connected() redundant.
> - Instead of has_dc3co_exitline dc3co_exitline should be computed (and
>   read out), which could be used then when we need to program it.
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c  |   5 +
> >  .../drm/i915/display/intel_display_power.c    | 104 ++++++++++++++++++
> >  .../drm/i915/display/intel_display_power.h    |   7 ++
> >  .../drm/i915/display/intel_display_types.h    |   1 +
> >  drivers/gpu/drm/i915/intel_pm.c               |   2 +-
> >  drivers/gpu/drm/i915/intel_pm.h               |   2 +
> >  6 files changed, 120 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index a19f8c73f2e0..6b7b8d2112a5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -7409,6 +7409,8 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> >  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> >  	int clock_limit = dev_priv->max_dotclk_freq;
> >  
> > +	tgl_dc3co_crtc_compute_config(dev_priv, pipe_config);
> > +
> >  	if (INTEL_GEN(dev_priv) < 4) {
> >  		clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
> >  
> > @@ -10474,6 +10476,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >  		pipe_config->pixel_multiplier = 1;
> >  	}
> >  
> > +	tgl_dc3co_crtc_get_config(pipe_config);
> > +
> >  out:
> >  	for_each_power_domain(power_domain, power_domain_mask)
> >  		intel_display_power_put(dev_priv,
> > @@ -12739,6 +12743,7 @@ 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_BOOL(has_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_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 7965e07257a0..42eabcdecf00 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -19,6 +19,7 @@
> >  #include "intel_hotplug.h"
> >  #include "intel_sideband.h"
> >  #include "intel_tc.h"
> > +#include "intel_pm.h"
> >  
> >  bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
> >  					 enum i915_power_well_id power_well_id);
> > @@ -772,6 +773,109 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> >  	dev_priv->csr.dc_state = val & mask;
> >  }
> >  
> > +void tgl_disable_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->has_dc3co_exitline)
> > +		return;
> > +
> > +	val = I915_READ(EXITLINE(cstate->cpu_transcoder));
> > +	val &= ~EXITLINE_ENABLE;
> > +	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> > +}
> > +
> > +void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> > +{
> > +	u32 linetime_us, val, exit_scanlines;
> > +	u32 crtc_vdisplay = cstate->base.adjusted_mode.crtc_vdisplay;
> > +	struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> > +
> > +	if (!cstate->has_dc3co_exitline)
> > +		return;
> > +
> > +	linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(cstate));
> > +	if (WARN_ON(!linetime_us))
> > +		return;
> > +	/*
> > +	 * DC3CO Exit time 200us B.Spec 49196
> > +	 * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line time) + 1
> > +	 * Exit line event need to program above calculated scan lines before
> > +	 * next VBLANK.
> > +	 */
> > +	exit_scanlines = DIV_ROUND_UP(200, linetime_us) + 1;
> > +	if (WARN_ON(exit_scanlines > crtc_vdisplay))
> > +		return;
> > +
> > +	exit_scanlines = crtc_vdisplay - exit_scanlines;
> > +	exit_scanlines <<= EXITLINE_SHIFT;
> > +	val = I915_READ(EXITLINE(cstate->cpu_transcoder));
> > +	val &= ~(EXITLINE_MASK | EXITLINE_ENABLE);
> > +	val |= exit_scanlines;
> > +	val |= EXITLINE_ENABLE;
> > +	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> > +}
> > +
> > +static bool tgl_dc3co_is_edp_connected(struct intel_crtc_state  *crtc_state)
> > +{
> > +	struct drm_atomic_state *state = crtc_state->base.state;
> > +	struct drm_connector *connector;
> > +	struct drm_connector_state *connector_state;
> > +	int i;
> > +
> > +	for_each_new_connector_in_state(state, connector, connector_state, i) {
> > +		if (connector->status == connector_status_connected &&
> > +		    connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> > +			return true;
> > +		}
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +/*
> > + * DC3CO requires to enable exitline and program DC3CO requires
> > + * exit scanlines to TRANS_EXITLINE register, which should only
> > + * change before transcoder or port are enabled.
> > + * This requires to disable the fastset at boot for eDP output.
> > + */
> > +void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> > +				   struct intel_crtc_state *crtc_state)
> > +{
> > +	if (!IS_TIGERLAKE(dev_priv))
> > +		return;
> > +
> > +	if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
> > +		return;
> > +
> > +	if (crtc_state->has_psr2 && crtc_state->base.active) {
> > +		if (tgl_dc3co_is_edp_connected(crtc_state))
> > +			crtc_state->has_dc3co_exitline = true;
> > +	}
> > +}
> > +
> > +void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
> > +{
> > +	u32 val;
> > +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > +
> > +	if (!IS_TIGERLAKE(dev_priv))
> > +		return;
> > +
> > +	if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
> > +		return;
> > +
> > +	/* DC3CO and PSR2 is supported only on TRANSCODER_A */
> > +	if (crtc_state->cpu_transcoder != TRANSCODER_A)
> > +		return;
> > +
> > +	val = I915_READ(EXITLINE(crtc_state->cpu_transcoder));
> > +
> > +	if (val & EXITLINE_ENABLE)
> > +		crtc_state->has_dc3co_exitline = true;
> > +}
> > +
> >  static void
> >  allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
> >  {
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index 13fc705799fd..bf68f26a5a37 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > @@ -12,6 +12,8 @@
> >  
> >  struct drm_i915_private;
> >  struct intel_encoder;
> > +struct intel_crtc_state;
> > +struct intel_atomic_state;
> >  
> >  enum intel_display_power_domain {
> >  	POWER_DOMAIN_DISPLAY_CORE,
> > @@ -258,6 +260,11 @@ void intel_display_power_resume_early(struct drm_i915_private *i915);
> >  void intel_display_power_suspend(struct drm_i915_private *i915);
> >  void intel_display_power_resume(struct drm_i915_private *i915);
> >  void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state);
> > +void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> > +				   struct intel_crtc_state *crtc_state);
> > +void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state);
> > +void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> > +void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> >  
> >  const char *
> >  intel_display_power_domain_str(enum intel_display_power_domain domain);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index d5cc4b810d9e..22f8fe0a34d0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -871,6 +871,7 @@ struct intel_crtc_state {
> >  
> >  	bool has_psr;
> >  	bool has_psr2;
> > +	bool has_dc3co_exitline;
> >  
> >  	/*
> >  	 * Frequence the dpll for the port should run at. Differs from the
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index d0ceb272551f..5679d1c1d560 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4588,7 +4588,7 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency,
> >  	return ret;
> >  }
> >  
> > -static uint_fixed_16_16_t
> > +uint_fixed_16_16_t
> >  intel_get_linetime_us(const struct intel_crtc_state *crtc_state)
> >  {
> >  	u32 pixel_rate;
> > diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> > index e3573e1e16e3..454e92c06dff 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.h
> > +++ b/drivers/gpu/drm/i915/intel_pm.h
> > @@ -8,6 +8,7 @@
> >  
> >  #include <linux/types.h>
> >  
> > +#include "i915_drv.h"
> >  #include "i915_reg.h"
> >  
> >  struct drm_device;
> > @@ -76,6 +77,7 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, i915_reg_t reg);
> >  u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, i915_reg_t reg);
> >  
> >  u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1);
> > +uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *cstate);
> >  
> >  unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
> >  unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
> > -- 
> > 2.21.0
> >
On 2019-09-23 at 19:26:56 +0300, Imre Deak wrote:
> On Fri, Sep 13, 2019 at 01:53:36PM +0530, Anshuman Gupta wrote:
> > DC3CO enabling B.Specs sequence requires to enable end configure
> > exit scanlines to TRANS_EXITLINE register, programming this register
> > has to be part of modeset sequence as this can't be change when
> > transcoder or port is enabled.
> > When system boots with only eDP panel there may not be real
> > modeset as BIOS has already programmed the necessary registers,
> > therefore it needs to force a modeset to enable and configure
> > DC3CO exitline.
> > 
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Animesh Manna <animesh.manna@intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> 
> You missed my review comments on v7:
> - Computing the state should be done from a DP encoder compute config
>   hook, which will then make tgl_dc3co_is_edp_connected() redundant.
> - Instead of has_dc3co_exitline dc3co_exitline should be computed (and
>   read out), which could be used then when we need to program it.
Hi Imre,
We need crtc_state pixel rate in order to compute dc3co exitlines,
crtc_state pixel rate is not ready in DP encoder compute config.
It is getting computed in intel_crtc_compute_config().
I have two solution in my mind.
1. compute has_dc3co_exitline in dp compute hook and calculate
   dc3co exitlines in tgl_set_psr2_transcoder_exitline()?
2. call intel_crtc_compute_config() in dp encoder compute config to
   get crtc_state pixel rate?

Please provide your suggestion, will change it accordingly.
Thanks,
Anshuman Gupta.
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c  |   5 +
> >  .../drm/i915/display/intel_display_power.c    | 104 ++++++++++++++++++
> >  .../drm/i915/display/intel_display_power.h    |   7 ++
> >  .../drm/i915/display/intel_display_types.h    |   1 +
> >  drivers/gpu/drm/i915/intel_pm.c               |   2 +-
> >  drivers/gpu/drm/i915/intel_pm.h               |   2 +
> >  6 files changed, 120 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index a19f8c73f2e0..6b7b8d2112a5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -7409,6 +7409,8 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> >  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> >  	int clock_limit = dev_priv->max_dotclk_freq;
> >  
> > +	tgl_dc3co_crtc_compute_config(dev_priv, pipe_config);
> > +
> >  	if (INTEL_GEN(dev_priv) < 4) {
> >  		clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
> >  
> > @@ -10474,6 +10476,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >  		pipe_config->pixel_multiplier = 1;
> >  	}
> >  
> > +	tgl_dc3co_crtc_get_config(pipe_config);
> > +
> >  out:
> >  	for_each_power_domain(power_domain, power_domain_mask)
> >  		intel_display_power_put(dev_priv,
> > @@ -12739,6 +12743,7 @@ 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_BOOL(has_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_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 7965e07257a0..42eabcdecf00 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -19,6 +19,7 @@
> >  #include "intel_hotplug.h"
> >  #include "intel_sideband.h"
> >  #include "intel_tc.h"
> > +#include "intel_pm.h"
> >  
> >  bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
> >  					 enum i915_power_well_id power_well_id);
> > @@ -772,6 +773,109 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> >  	dev_priv->csr.dc_state = val & mask;
> >  }
> >  
> > +void tgl_disable_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->has_dc3co_exitline)
> > +		return;
> > +
> > +	val = I915_READ(EXITLINE(cstate->cpu_transcoder));
> > +	val &= ~EXITLINE_ENABLE;
> > +	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> > +}
> > +
> > +void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> > +{
> > +	u32 linetime_us, val, exit_scanlines;
> > +	u32 crtc_vdisplay = cstate->base.adjusted_mode.crtc_vdisplay;
> > +	struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> > +
> > +	if (!cstate->has_dc3co_exitline)
> > +		return;
> > +
> > +	linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(cstate));
> > +	if (WARN_ON(!linetime_us))
> > +		return;
> > +	/*
> > +	 * DC3CO Exit time 200us B.Spec 49196
> > +	 * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line time) + 1
> > +	 * Exit line event need to program above calculated scan lines before
> > +	 * next VBLANK.
> > +	 */
> > +	exit_scanlines = DIV_ROUND_UP(200, linetime_us) + 1;
> > +	if (WARN_ON(exit_scanlines > crtc_vdisplay))
> > +		return;
> > +
> > +	exit_scanlines = crtc_vdisplay - exit_scanlines;
> > +	exit_scanlines <<= EXITLINE_SHIFT;
> > +	val = I915_READ(EXITLINE(cstate->cpu_transcoder));
> > +	val &= ~(EXITLINE_MASK | EXITLINE_ENABLE);
> > +	val |= exit_scanlines;
> > +	val |= EXITLINE_ENABLE;
> > +	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> > +}
> > +
> > +static bool tgl_dc3co_is_edp_connected(struct intel_crtc_state  *crtc_state)
> > +{
> > +	struct drm_atomic_state *state = crtc_state->base.state;
> > +	struct drm_connector *connector;
> > +	struct drm_connector_state *connector_state;
> > +	int i;
> > +
> > +	for_each_new_connector_in_state(state, connector, connector_state, i) {
> > +		if (connector->status == connector_status_connected &&
> > +		    connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> > +			return true;
> > +		}
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +/*
> > + * DC3CO requires to enable exitline and program DC3CO requires
> > + * exit scanlines to TRANS_EXITLINE register, which should only
> > + * change before transcoder or port are enabled.
> > + * This requires to disable the fastset at boot for eDP output.
> > + */
> > +void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> > +				   struct intel_crtc_state *crtc_state)
> > +{
> > +	if (!IS_TIGERLAKE(dev_priv))
> > +		return;
> > +
> > +	if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
> > +		return;
> > +
> > +	if (crtc_state->has_psr2 && crtc_state->base.active) {
> > +		if (tgl_dc3co_is_edp_connected(crtc_state))
> > +			crtc_state->has_dc3co_exitline = true;
> > +	}
> > +}
> > +
> > +void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
> > +{
> > +	u32 val;
> > +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > +
> > +	if (!IS_TIGERLAKE(dev_priv))
> > +		return;
> > +
> > +	if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
> > +		return;
> > +
> > +	/* DC3CO and PSR2 is supported only on TRANSCODER_A */
> > +	if (crtc_state->cpu_transcoder != TRANSCODER_A)
> > +		return;
> > +
> > +	val = I915_READ(EXITLINE(crtc_state->cpu_transcoder));
> > +
> > +	if (val & EXITLINE_ENABLE)
> > +		crtc_state->has_dc3co_exitline = true;
> > +}
> > +
> >  static void
> >  allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
> >  {
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index 13fc705799fd..bf68f26a5a37 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > @@ -12,6 +12,8 @@
> >  
> >  struct drm_i915_private;
> >  struct intel_encoder;
> > +struct intel_crtc_state;
> > +struct intel_atomic_state;
> >  
> >  enum intel_display_power_domain {
> >  	POWER_DOMAIN_DISPLAY_CORE,
> > @@ -258,6 +260,11 @@ void intel_display_power_resume_early(struct drm_i915_private *i915);
> >  void intel_display_power_suspend(struct drm_i915_private *i915);
> >  void intel_display_power_resume(struct drm_i915_private *i915);
> >  void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state);
> > +void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> > +				   struct intel_crtc_state *crtc_state);
> > +void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state);
> > +void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> > +void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> >  
> >  const char *
> >  intel_display_power_domain_str(enum intel_display_power_domain domain);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index d5cc4b810d9e..22f8fe0a34d0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -871,6 +871,7 @@ struct intel_crtc_state {
> >  
> >  	bool has_psr;
> >  	bool has_psr2;
> > +	bool has_dc3co_exitline;
> >  
> >  	/*
> >  	 * Frequence the dpll for the port should run at. Differs from the
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index d0ceb272551f..5679d1c1d560 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4588,7 +4588,7 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency,
> >  	return ret;
> >  }
> >  
> > -static uint_fixed_16_16_t
> > +uint_fixed_16_16_t
> >  intel_get_linetime_us(const struct intel_crtc_state *crtc_state)
> >  {
> >  	u32 pixel_rate;
> > diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> > index e3573e1e16e3..454e92c06dff 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.h
> > +++ b/drivers/gpu/drm/i915/intel_pm.h
> > @@ -8,6 +8,7 @@
> >  
> >  #include <linux/types.h>
> >  
> > +#include "i915_drv.h"
> >  #include "i915_reg.h"
> >  
> >  struct drm_device;
> > @@ -76,6 +77,7 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, i915_reg_t reg);
> >  u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, i915_reg_t reg);
> >  
> >  u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1);
> > +uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *cstate);
> >  
> >  unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
> >  unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
> > -- 
> > 2.21.0
> >
On Wed, Sep 25, 2019 at 02:40:42PM +0530, Anshuman Gupta wrote:
> On 2019-09-23 at 19:26:56 +0300, Imre Deak wrote:
> > On Fri, Sep 13, 2019 at 01:53:36PM +0530, Anshuman Gupta wrote:
> > > DC3CO enabling B.Specs sequence requires to enable end configure
> > > exit scanlines to TRANS_EXITLINE register, programming this register
> > > has to be part of modeset sequence as this can't be change when
> > > transcoder or port is enabled.
> > > When system boots with only eDP panel there may not be real
> > > modeset as BIOS has already programmed the necessary registers,
> > > therefore it needs to force a modeset to enable and configure
> > > DC3CO exitline.
> > > 
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Cc: Animesh Manna <animesh.manna@intel.com>
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > 
> > You missed my review comments on v7:
> > - Computing the state should be done from a DP encoder compute config
> >   hook, which will then make tgl_dc3co_is_edp_connected() redundant.
> > - Instead of has_dc3co_exitline dc3co_exitline should be computed (and
> >   read out), which could be used then when we need to program it.
> Hi Imre,
> We need crtc_state pixel rate in order to compute dc3co exitlines,
> crtc_state pixel rate is not ready in DP encoder compute config.
> It is getting computed in intel_crtc_compute_config().
> I have two solution in my mind.
> 1. compute has_dc3co_exitline in dp compute hook and calculate
>    dc3co exitlines in tgl_set_psr2_transcoder_exitline()?
> 2. call intel_crtc_compute_config() in dp encoder compute config to
>    get crtc_state pixel rate?

intel_get_linetime_us() is the time needed to fetch one line from
memory, while you need the time needed to scan out one line. You need
the inverse of intel_usecs_to_scanlines().

> 
> Please provide your suggestion, will change it accordingly.
> Thanks,
> Anshuman Gupta.
> > 
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c  |   5 +
> > >  .../drm/i915/display/intel_display_power.c    | 104 ++++++++++++++++++
> > >  .../drm/i915/display/intel_display_power.h    |   7 ++
> > >  .../drm/i915/display/intel_display_types.h    |   1 +
> > >  drivers/gpu/drm/i915/intel_pm.c               |   2 +-
> > >  drivers/gpu/drm/i915/intel_pm.h               |   2 +
> > >  6 files changed, 120 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index a19f8c73f2e0..6b7b8d2112a5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -7409,6 +7409,8 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> > >  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> > >  	int clock_limit = dev_priv->max_dotclk_freq;
> > >  
> > > +	tgl_dc3co_crtc_compute_config(dev_priv, pipe_config);
> > > +
> > >  	if (INTEL_GEN(dev_priv) < 4) {
> > >  		clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
> > >  
> > > @@ -10474,6 +10476,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> > >  		pipe_config->pixel_multiplier = 1;
> > >  	}
> > >  
> > > +	tgl_dc3co_crtc_get_config(pipe_config);
> > > +
> > >  out:
> > >  	for_each_power_domain(power_domain, power_domain_mask)
> > >  		intel_display_power_put(dev_priv,
> > > @@ -12739,6 +12743,7 @@ 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_BOOL(has_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_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > index 7965e07257a0..42eabcdecf00 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > @@ -19,6 +19,7 @@
> > >  #include "intel_hotplug.h"
> > >  #include "intel_sideband.h"
> > >  #include "intel_tc.h"
> > > +#include "intel_pm.h"
> > >  
> > >  bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
> > >  					 enum i915_power_well_id power_well_id);
> > > @@ -772,6 +773,109 @@ static void gen9_set_dc_state(struct drm_i915_private *dev_priv, u32 state)
> > >  	dev_priv->csr.dc_state = val & mask;
> > >  }
> > >  
> > > +void tgl_disable_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->has_dc3co_exitline)
> > > +		return;
> > > +
> > > +	val = I915_READ(EXITLINE(cstate->cpu_transcoder));
> > > +	val &= ~EXITLINE_ENABLE;
> > > +	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> > > +}
> > > +
> > > +void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *cstate)
> > > +{
> > > +	u32 linetime_us, val, exit_scanlines;
> > > +	u32 crtc_vdisplay = cstate->base.adjusted_mode.crtc_vdisplay;
> > > +	struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> > > +
> > > +	if (!cstate->has_dc3co_exitline)
> > > +		return;
> > > +
> > > +	linetime_us = fixed16_to_u32_round_up(intel_get_linetime_us(cstate));
> > > +	if (WARN_ON(!linetime_us))
> > > +		return;
> > > +	/*
> > > +	 * DC3CO Exit time 200us B.Spec 49196
> > > +	 * PSR2 transcoder Early Exit scanlines = ROUNDUP(200 / line time) + 1
> > > +	 * Exit line event need to program above calculated scan lines before
> > > +	 * next VBLANK.
> > > +	 */
> > > +	exit_scanlines = DIV_ROUND_UP(200, linetime_us) + 1;
> > > +	if (WARN_ON(exit_scanlines > crtc_vdisplay))
> > > +		return;
> > > +
> > > +	exit_scanlines = crtc_vdisplay - exit_scanlines;
> > > +	exit_scanlines <<= EXITLINE_SHIFT;
> > > +	val = I915_READ(EXITLINE(cstate->cpu_transcoder));
> > > +	val &= ~(EXITLINE_MASK | EXITLINE_ENABLE);
> > > +	val |= exit_scanlines;
> > > +	val |= EXITLINE_ENABLE;
> > > +	I915_WRITE(EXITLINE(cstate->cpu_transcoder), val);
> > > +}
> > > +
> > > +static bool tgl_dc3co_is_edp_connected(struct intel_crtc_state  *crtc_state)
> > > +{
> > > +	struct drm_atomic_state *state = crtc_state->base.state;
> > > +	struct drm_connector *connector;
> > > +	struct drm_connector_state *connector_state;
> > > +	int i;
> > > +
> > > +	for_each_new_connector_in_state(state, connector, connector_state, i) {
> > > +		if (connector->status == connector_status_connected &&
> > > +		    connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> > > +			return true;
> > > +		}
> > > +	}
> > > +
> > > +	return false;
> > > +}
> > > +
> > > +/*
> > > + * DC3CO requires to enable exitline and program DC3CO requires
> > > + * exit scanlines to TRANS_EXITLINE register, which should only
> > > + * change before transcoder or port are enabled.
> > > + * This requires to disable the fastset at boot for eDP output.
> > > + */
> > > +void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> > > +				   struct intel_crtc_state *crtc_state)
> > > +{
> > > +	if (!IS_TIGERLAKE(dev_priv))
> > > +		return;
> > > +
> > > +	if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
> > > +		return;
> > > +
> > > +	if (crtc_state->has_psr2 && crtc_state->base.active) {
> > > +		if (tgl_dc3co_is_edp_connected(crtc_state))
> > > +			crtc_state->has_dc3co_exitline = true;
> > > +	}
> > > +}
> > > +
> > > +void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state)
> > > +{
> > > +	u32 val;
> > > +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > > +
> > > +	if (!IS_TIGERLAKE(dev_priv))
> > > +		return;
> > > +
> > > +	if (!(dev_priv->csr.allowed_dc_mask & DC_STATE_EN_DC3CO))
> > > +		return;
> > > +
> > > +	/* DC3CO and PSR2 is supported only on TRANSCODER_A */
> > > +	if (crtc_state->cpu_transcoder != TRANSCODER_A)
> > > +		return;
> > > +
> > > +	val = I915_READ(EXITLINE(crtc_state->cpu_transcoder));
> > > +
> > > +	if (val & EXITLINE_ENABLE)
> > > +		crtc_state->has_dc3co_exitline = true;
> > > +}
> > > +
> > >  static void
> > >  allowed_dc_mask_to_target_dc_state(struct drm_i915_private *dev_priv)
> > >  {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > index 13fc705799fd..bf68f26a5a37 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > @@ -12,6 +12,8 @@
> > >  
> > >  struct drm_i915_private;
> > >  struct intel_encoder;
> > > +struct intel_crtc_state;
> > > +struct intel_atomic_state;
> > >  
> > >  enum intel_display_power_domain {
> > >  	POWER_DOMAIN_DISPLAY_CORE,
> > > @@ -258,6 +260,11 @@ void intel_display_power_resume_early(struct drm_i915_private *i915);
> > >  void intel_display_power_suspend(struct drm_i915_private *i915);
> > >  void intel_display_power_resume(struct drm_i915_private *i915);
> > >  void tgl_set_target_dc_state(struct drm_i915_private *dev_priv, u32 state);
> > > +void tgl_dc3co_crtc_compute_config(struct drm_i915_private *dev_priv,
> > > +				   struct intel_crtc_state *crtc_state);
> > > +void tgl_dc3co_crtc_get_config(struct intel_crtc_state *crtc_state);
> > > +void tgl_disable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> > > +void tgl_enable_psr2_transcoder_exitline(const struct intel_crtc_state *state);
> > >  
> > >  const char *
> > >  intel_display_power_domain_str(enum intel_display_power_domain domain);
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index d5cc4b810d9e..22f8fe0a34d0 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -871,6 +871,7 @@ struct intel_crtc_state {
> > >  
> > >  	bool has_psr;
> > >  	bool has_psr2;
> > > +	bool has_dc3co_exitline;
> > >  
> > >  	/*
> > >  	 * Frequence the dpll for the port should run at. Differs from the
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index d0ceb272551f..5679d1c1d560 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -4588,7 +4588,7 @@ skl_wm_method2(u32 pixel_rate, u32 pipe_htotal, u32 latency,
> > >  	return ret;
> > >  }
> > >  
> > > -static uint_fixed_16_16_t
> > > +uint_fixed_16_16_t
> > >  intel_get_linetime_us(const struct intel_crtc_state *crtc_state)
> > >  {
> > >  	u32 pixel_rate;
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> > > index e3573e1e16e3..454e92c06dff 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.h
> > > +++ b/drivers/gpu/drm/i915/intel_pm.h
> > > @@ -8,6 +8,7 @@
> > >  
> > >  #include <linux/types.h>
> > >  
> > > +#include "i915_drv.h"
> > >  #include "i915_reg.h"
> > >  
> > >  struct drm_device;
> > > @@ -76,6 +77,7 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, i915_reg_t reg);
> > >  u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv, i915_reg_t reg);
> > >  
> > >  u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1);
> > > +uint_fixed_16_16_t intel_get_linetime_us(const struct intel_crtc_state *cstate);
> > >  
> > >  unsigned long i915_chipset_val(struct drm_i915_private *dev_priv);
> > >  unsigned long i915_mch_val(struct drm_i915_private *dev_priv);
> > > -- 
> > > 2.21.0
> > >