All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup
@ 2022-11-09 11:15 Animesh Manna
  2022-11-09 11:15 ` [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario Animesh Manna
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Animesh Manna @ 2022-11-09 11:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

Simplified pps_get_register() which use get_pps_idx() hook to derive the
pps instance and get_pps_idx() will be initialized at pps_init().

v1: Initial version. Got r-b from Jani.
v2: Corrected unintentional change around memset() call. [Jani]

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_types.h |  1 +
 drivers/gpu/drm/i915/display/intel_pps.c           | 14 +++++++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index c6abaaa46e17..87163ef32983 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1698,6 +1698,7 @@ struct intel_dp {
 	u8 (*preemph_max)(struct intel_dp *intel_dp);
 	u8 (*voltage_max)(struct intel_dp *intel_dp,
 			  const struct intel_crtc_state *crtc_state);
+	int (*get_pps_idx)(struct intel_dp *intel_dp);
 
 	/* Displayport compliance testing */
 	struct intel_dp_compliance compliance;
diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
index 22f5e08d396b..3949fb449353 100644
--- a/drivers/gpu/drm/i915/display/intel_pps.c
+++ b/drivers/gpu/drm/i915/display/intel_pps.c
@@ -366,11 +366,8 @@ static void intel_pps_get_registers(struct intel_dp *intel_dp,
 	int pps_idx = 0;
 
 	memset(regs, 0, sizeof(*regs));
-
-	if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv))
-		pps_idx = bxt_power_sequencer_idx(intel_dp);
-	else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		pps_idx = vlv_power_sequencer_pipe(intel_dp);
+	if (intel_dp->get_pps_idx)
+		pps_idx = intel_dp->get_pps_idx(intel_dp);
 
 	regs->pp_ctrl = PP_CONTROL(pps_idx);
 	regs->pp_stat = PP_STATUS(pps_idx);
@@ -1433,6 +1430,13 @@ void intel_pps_init(struct intel_dp *intel_dp)
 	intel_dp->pps.initializing = true;
 	INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work);
 
+	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915))
+		intel_dp->get_pps_idx = bxt_power_sequencer_idx;
+	else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
+		intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
+	else
+		intel_dp->get_pps_idx = NULL;
+
 	pps_init_timestamps(intel_dp);
 
 	with_intel_pps_lock(intel_dp, wakeref) {
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
  2022-11-09 11:15 [Intel-gfx] [PATCH 1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup Animesh Manna
@ 2022-11-09 11:15 ` Animesh Manna
  2022-11-09 11:42   ` Jani Nikula
  2022-11-09 11:15 ` [Intel-gfx] [PATCH 3/3] drm/i915/edp: Fix warning as vdd went down without driver knowledge Animesh Manna
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Animesh Manna @ 2022-11-09 11:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

From display gen12 onwards to support dual EDP two instances of pps added.
Currently backlight controller and pps instance can be mapped together
for a specific panel. Currently dual PPS support is broken. This patch
fixes it and enables for display 12+.

v1: Iniital revision.
v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani]
v3: Set pps_id to -1 for pnpid type of panel which will be used by
bxt_power_sequencer_idx() to set 2nd pps instance as default for
2nd EDP panel. [Jani]
v4: Early return for PANEL_TYPE_FALLBACK. [Jani]
v5: Removed additional pps_id variable and reused backlight
controller. [Jani]

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c |  9 ++++++++-
 drivers/gpu/drm/i915/display/intel_bios.h |  2 +-
 drivers/gpu/drm/i915/display/intel_dp.c   | 10 +++++++---
 drivers/gpu/drm/i915/display/intel_pps.c  | 12 +++++++++++-
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index c2987f2c2b2e..1c1eea061fbb 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -3183,7 +3183,7 @@ void intel_bios_init(struct drm_i915_private *i915)
 	kfree(oprom_vbt);
 }
 
-void intel_bios_init_panel(struct drm_i915_private *i915,
+bool intel_bios_init_panel(struct drm_i915_private *i915,
 			   struct intel_panel *panel,
 			   const struct intel_bios_encoder_data *devdata,
 			   const struct edid *edid)
@@ -3192,6 +3192,11 @@ void intel_bios_init_panel(struct drm_i915_private *i915,
 
 	panel->vbt.panel_type = get_panel_type(i915, devdata, edid);
 
+	if (panel->vbt.panel_type == PANEL_TYPE_FALLBACK && !edid) {
+		panel->vbt.backlight.controller = -1;
+		return true;
+	}
+
 	parse_panel_options(i915, panel);
 	parse_generic_dtd(i915, panel);
 	parse_lfp_data(i915, panel);
@@ -3203,6 +3208,8 @@ void intel_bios_init_panel(struct drm_i915_private *i915,
 	parse_psr(i915, panel);
 	parse_mipi_config(i915, panel);
 	parse_mipi_sequence(i915, panel);
+
+	return false;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h
index e375405a7828..f8ef0274f3ee 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.h
+++ b/drivers/gpu/drm/i915/display/intel_bios.h
@@ -232,7 +232,7 @@ struct mipi_pps_data {
 } __packed;
 
 void intel_bios_init(struct drm_i915_private *dev_priv);
-void intel_bios_init_panel(struct drm_i915_private *dev_priv,
+bool intel_bios_init_panel(struct drm_i915_private *dev_priv,
 			   struct intel_panel *panel,
 			   const struct intel_bios_encoder_data *devdata,
 			   const struct edid *edid);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 7400d6b4c587..78cf3a77f026 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5235,6 +5235,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 	bool has_dpcd;
 	struct edid *edid;
+	bool retry;
 
 	if (!intel_dp_is_edp(intel_dp))
 		return true;
@@ -5254,6 +5255,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 		return false;
 	}
 
+	retry = intel_bios_init_panel(dev_priv, &intel_connector->panel,
+				      encoder->devdata, NULL);
+
 	intel_pps_init(intel_dp);
 
 	/* Cache DPCD and EDID for edp. */
@@ -5288,9 +5292,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 		edid = ERR_PTR(-ENOENT);
 	}
 	intel_connector->edid = edid;
-
-	intel_bios_init_panel(dev_priv, &intel_connector->panel,
-			      encoder->devdata, IS_ERR(edid) ? NULL : edid);
+	if (retry)
+		intel_bios_init_panel(dev_priv, &intel_connector->panel,
+				      encoder->devdata, IS_ERR(edid) ? NULL : edid);
 
 	intel_panel_add_edid_fixed_modes(intel_connector, true);
 
diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
index 3949fb449353..5738af154bd2 100644
--- a/drivers/gpu/drm/i915/display/intel_pps.c
+++ b/drivers/gpu/drm/i915/display/intel_pps.c
@@ -219,6 +219,16 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp)
 	/* We should never land here with regular DP ports */
 	drm_WARN_ON(&dev_priv->drm, !intel_dp_is_edp(intel_dp));
 
+	if (backlight_controller == -1) {
+		/*
+		 * Use 2nd PPS instance as default for 2nd EDP panel.
+		 */
+		if (connector->encoder->port == PORT_A)
+			return 0;
+		else
+			return 1;
+	}
+
 	if (!intel_dp->pps.pps_reset)
 		return backlight_controller;
 
@@ -1430,7 +1440,7 @@ void intel_pps_init(struct intel_dp *intel_dp)
 	intel_dp->pps.initializing = true;
 	INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work);
 
-	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915))
+	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12)
 		intel_dp->get_pps_idx = bxt_power_sequencer_idx;
 	else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
 		intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Intel-gfx] [PATCH 3/3] drm/i915/edp: Fix warning as vdd went down without driver knowledge
  2022-11-09 11:15 [Intel-gfx] [PATCH 1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup Animesh Manna
  2022-11-09 11:15 ` [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario Animesh Manna
@ 2022-11-09 11:15 ` Animesh Manna
  2022-11-09 11:46   ` Jani Nikula
  2022-11-09 16:41 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup Patchwork
  2022-11-09 17:05 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  3 siblings, 1 reply; 15+ messages in thread
From: Animesh Manna @ 2022-11-09 11:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

Kernel warning triggered as vdd went down after certain time during
aux transfer in connector init sequence. To solve the kernel
warning adjust power domain and vdd wakeref count.
Currently issue seen on ADL so add the above adjustment part of
ADL platform check, if needed will extend for future platform.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_pps.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
index 5738af154bd2..ebc03c8f73c5 100644
--- a/drivers/gpu/drm/i915/display/intel_pps.c
+++ b/drivers/gpu/drm/i915/display/intel_pps.c
@@ -597,8 +597,15 @@ bool intel_pps_vdd_on_unlocked(struct intel_dp *intel_dp)
 	cancel_delayed_work(&intel_dp->pps.panel_vdd_work);
 	intel_dp->pps.want_panel_vdd = true;
 
-	if (edp_have_panel_vdd(intel_dp))
+	if (edp_have_panel_vdd(intel_dp)) {
 		return need_to_disable;
+	} else {
+		if ((IS_ALDERLAKE_S(dev_priv) || IS_ALDERLAKE_P(dev_priv)) &&
+		    intel_dp->pps.vdd_wakeref)
+			intel_display_power_put(dev_priv,
+						intel_aux_power_domain(dig_port),
+						fetch_and_zero(&intel_dp->pps.vdd_wakeref));
+	}
 
 	drm_WARN_ON(&dev_priv->drm, intel_dp->pps.vdd_wakeref);
 	intel_dp->pps.vdd_wakeref = intel_display_power_get(dev_priv,
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
  2022-11-09 11:15 ` [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario Animesh Manna
@ 2022-11-09 11:42   ` Jani Nikula
  2022-11-09 13:54     ` Manna, Animesh
  0 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2022-11-09 11:42 UTC (permalink / raw)
  To: Animesh Manna, intel-gfx

On Wed, 09 Nov 2022, Animesh Manna <animesh.manna@intel.com> wrote:
> From display gen12 onwards to support dual EDP two instances of pps added.
> Currently backlight controller and pps instance can be mapped together
> for a specific panel. Currently dual PPS support is broken. This patch
> fixes it and enables for display 12+.
>
> v1: Iniital revision.
> v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani]
> v3: Set pps_id to -1 for pnpid type of panel which will be used by
> bxt_power_sequencer_idx() to set 2nd pps instance as default for
> 2nd EDP panel. [Jani]
> v4: Early return for PANEL_TYPE_FALLBACK. [Jani]
> v5: Removed additional pps_id variable and reused backlight
> controller. [Jani]
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c |  9 ++++++++-
>  drivers/gpu/drm/i915/display/intel_bios.h |  2 +-
>  drivers/gpu/drm/i915/display/intel_dp.c   | 10 +++++++---
>  drivers/gpu/drm/i915/display/intel_pps.c  | 12 +++++++++++-
>  4 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index c2987f2c2b2e..1c1eea061fbb 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -3183,7 +3183,7 @@ void intel_bios_init(struct drm_i915_private *i915)
>  	kfree(oprom_vbt);
>  }
>  
> -void intel_bios_init_panel(struct drm_i915_private *i915,
> +bool intel_bios_init_panel(struct drm_i915_private *i915,
>  			   struct intel_panel *panel,
>  			   const struct intel_bios_encoder_data *devdata,
>  			   const struct edid *edid)
> @@ -3192,6 +3192,11 @@ void intel_bios_init_panel(struct drm_i915_private *i915,
>  
>  	panel->vbt.panel_type = get_panel_type(i915, devdata, edid);
>  
> +	if (panel->vbt.panel_type == PANEL_TYPE_FALLBACK && !edid) {

As Ville pointed out, "panel->vbt.panel_type == PANEL_TYPE_FALLBACK" is
comparing apples to oranges, and we hit this path only in the rare case
that panel_type == 3, regardless of where the panel type originated.

This is probably the reason patches 1&2 don't fix the original issue
like it should.

I remind you that the issue is two eDP having their own pps software
states but using the same pps hardware due to conflicting pps index
(which is the same as panel->vbt.backlight.controller). This is what we
need to solve here.

We get the wakeref imbalance because of two eDP trampling on the same
pps hardware. Patch 3 does nothing to address that issue, it just tries
to hide the problem.


BR,
Jani.


> +		panel->vbt.backlight.controller = -1;
> +		return true;
> +	}
> +
>  	parse_panel_options(i915, panel);
>  	parse_generic_dtd(i915, panel);
>  	parse_lfp_data(i915, panel);
> @@ -3203,6 +3208,8 @@ void intel_bios_init_panel(struct drm_i915_private *i915,
>  	parse_psr(i915, panel);
>  	parse_mipi_config(i915, panel);
>  	parse_mipi_sequence(i915, panel);
> +
> +	return false;
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h
> index e375405a7828..f8ef0274f3ee 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.h
> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> @@ -232,7 +232,7 @@ struct mipi_pps_data {
>  } __packed;
>  
>  void intel_bios_init(struct drm_i915_private *dev_priv);
> -void intel_bios_init_panel(struct drm_i915_private *dev_priv,
> +bool intel_bios_init_panel(struct drm_i915_private *dev_priv,
>  			   struct intel_panel *panel,
>  			   const struct intel_bios_encoder_data *devdata,
>  			   const struct edid *edid);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7400d6b4c587..78cf3a77f026 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5235,6 +5235,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>  	bool has_dpcd;
>  	struct edid *edid;
> +	bool retry;
>  
>  	if (!intel_dp_is_edp(intel_dp))
>  		return true;
> @@ -5254,6 +5255,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  		return false;
>  	}
>  
> +	retry = intel_bios_init_panel(dev_priv, &intel_connector->panel,
> +				      encoder->devdata, NULL);
> +
>  	intel_pps_init(intel_dp);
>  
>  	/* Cache DPCD and EDID for edp. */
> @@ -5288,9 +5292,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  		edid = ERR_PTR(-ENOENT);
>  	}
>  	intel_connector->edid = edid;
> -
> -	intel_bios_init_panel(dev_priv, &intel_connector->panel,
> -			      encoder->devdata, IS_ERR(edid) ? NULL : edid);
> +	if (retry)
> +		intel_bios_init_panel(dev_priv, &intel_connector->panel,
> +				      encoder->devdata, IS_ERR(edid) ? NULL : edid);
>  
>  	intel_panel_add_edid_fixed_modes(intel_connector, true);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index 3949fb449353..5738af154bd2 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -219,6 +219,16 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp)
>  	/* We should never land here with regular DP ports */
>  	drm_WARN_ON(&dev_priv->drm, !intel_dp_is_edp(intel_dp));
>  
> +	if (backlight_controller == -1) {
> +		/*
> +		 * Use 2nd PPS instance as default for 2nd EDP panel.
> +		 */
> +		if (connector->encoder->port == PORT_A)
> +			return 0;
> +		else
> +			return 1;
> +	}
> +
>  	if (!intel_dp->pps.pps_reset)
>  		return backlight_controller;
>  
> @@ -1430,7 +1440,7 @@ void intel_pps_init(struct intel_dp *intel_dp)
>  	intel_dp->pps.initializing = true;
>  	INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work);
>  
> -	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915))
> +	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12)
>  		intel_dp->get_pps_idx = bxt_power_sequencer_idx;
>  	else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>  		intel_dp->get_pps_idx = vlv_power_sequencer_pipe;

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Intel-gfx] [PATCH 3/3] drm/i915/edp: Fix warning as vdd went down without driver knowledge
  2022-11-09 11:15 ` [Intel-gfx] [PATCH 3/3] drm/i915/edp: Fix warning as vdd went down without driver knowledge Animesh Manna
@ 2022-11-09 11:46   ` Jani Nikula
  0 siblings, 0 replies; 15+ messages in thread
From: Jani Nikula @ 2022-11-09 11:46 UTC (permalink / raw)
  To: Animesh Manna, intel-gfx

On Wed, 09 Nov 2022, Animesh Manna <animesh.manna@intel.com> wrote:
> Kernel warning triggered as vdd went down after certain time during
> aux transfer in connector init sequence. To solve the kernel
> warning adjust power domain and vdd wakeref count.
> Currently issue seen on ADL so add the above adjustment part of
> ADL platform check, if needed will extend for future platform.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_pps.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index 5738af154bd2..ebc03c8f73c5 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -597,8 +597,15 @@ bool intel_pps_vdd_on_unlocked(struct intel_dp *intel_dp)
>  	cancel_delayed_work(&intel_dp->pps.panel_vdd_work);
>  	intel_dp->pps.want_panel_vdd = true;
>  
> -	if (edp_have_panel_vdd(intel_dp))
> +	if (edp_have_panel_vdd(intel_dp)) {
>  		return need_to_disable;
> +	} else {
> +		if ((IS_ALDERLAKE_S(dev_priv) || IS_ALDERLAKE_P(dev_priv)) &&
> +		    intel_dp->pps.vdd_wakeref)
> +			intel_display_power_put(dev_priv,
> +						intel_aux_power_domain(dig_port),
> +						fetch_and_zero(&intel_dp->pps.vdd_wakeref));
> +	}

Already replied to patch 2, but repeating here:

We have two eDP, each with their own intel_dp->pps, but both checking
the same PPS hardware in edp_have_panel_vdd().

This is not the fix.

The fix is to ensure they both use their own PPS harware
instance. That's what the goal with patches 1&2 is, but there's still an
issue there.


BR,
Jani.


>  
>  	drm_WARN_ON(&dev_priv->drm, intel_dp->pps.vdd_wakeref);
>  	intel_dp->pps.vdd_wakeref = intel_display_power_get(dev_priv,

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
  2022-11-09 11:42   ` Jani Nikula
@ 2022-11-09 13:54     ` Manna, Animesh
  2022-11-09 14:04       ` Jani Nikula
  0 siblings, 1 reply; 15+ messages in thread
From: Manna, Animesh @ 2022-11-09 13:54 UTC (permalink / raw)
  To: Nikula, Jani, intel-gfx



> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Wednesday, November 9, 2022 5:12 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Manna, Animesh <animesh.manna@intel.com>; Ville Syrjälä
> <ville.syrjala@linux.intel.com>; Shankar, Uma <uma.shankar@intel.com>
> Subject: Re: [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
> 
> On Wed, 09 Nov 2022, Animesh Manna <animesh.manna@intel.com> wrote:
> > From display gen12 onwards to support dual EDP two instances of pps added.
> > Currently backlight controller and pps instance can be mapped together
> > for a specific panel. Currently dual PPS support is broken. This patch
> > fixes it and enables for display 12+.
> >
> > v1: Iniital revision.
> > v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init.
> > [Jani]
> > v3: Set pps_id to -1 for pnpid type of panel which will be used by
> > bxt_power_sequencer_idx() to set 2nd pps instance as default for 2nd
> > EDP panel. [Jani]
> > v4: Early return for PANEL_TYPE_FALLBACK. [Jani]
> > v5: Removed additional pps_id variable and reused backlight
> > controller. [Jani]
> >
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Uma Shankar <uma.shankar@intel.com>
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_bios.c |  9 ++++++++-
> > drivers/gpu/drm/i915/display/intel_bios.h |  2 +-
> >  drivers/gpu/drm/i915/display/intel_dp.c   | 10 +++++++---
> >  drivers/gpu/drm/i915/display/intel_pps.c  | 12 +++++++++++-
> >  4 files changed, 27 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
> > b/drivers/gpu/drm/i915/display/intel_bios.c
> > index c2987f2c2b2e..1c1eea061fbb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> > @@ -3183,7 +3183,7 @@ void intel_bios_init(struct drm_i915_private *i915)
> >  	kfree(oprom_vbt);
> >  }
> >
> > -void intel_bios_init_panel(struct drm_i915_private *i915,
> > +bool intel_bios_init_panel(struct drm_i915_private *i915,
> >  			   struct intel_panel *panel,
> >  			   const struct intel_bios_encoder_data *devdata,
> >  			   const struct edid *edid)
> > @@ -3192,6 +3192,11 @@ void intel_bios_init_panel(struct
> > drm_i915_private *i915,
> >
> >  	panel->vbt.panel_type = get_panel_type(i915, devdata, edid);
> >
> > +	if (panel->vbt.panel_type == PANEL_TYPE_FALLBACK && !edid) {
> 
> As Ville pointed out, "panel->vbt.panel_type == PANEL_TYPE_FALLBACK" is
> comparing apples to oranges, and we hit this path only in the rare case that
> panel_type == 3, regardless of where the panel type originated.

Got it, tried returning some random negative number (for example -2) from fallback_get_panel_type() and checking the same here is fixing the WARN_ON. 

> 
> This is probably the reason patches 1&2 don't fix the original issue like it should.
> 
> I remind you that the issue is two eDP having their own pps software states but
> using the same pps hardware due to conflicting pps index (which is the same as
> panel->vbt.backlight.controller). This is what we need to solve here.

In this patch we decide to use the specific pps based on port number. Once we identify it is PORT B always 2nd instance of PPS will be used. All pps register get instantiated with pps_idx 1 
	if (backlight_controller == -1) {
		/*
		 * Use 2nd PPS instance as default for 2nd EDP panel.
		 */
		if (connector->encoder->port == PORT_A)
			return 0;
		else
			return 1;
	}
Not sure what is toggling the VDD. Anyways I agree patch 3 is not needed. Thanks for review.

Regards,
Animesh

> 
> We get the wakeref imbalance because of two eDP trampling on the same pps
> hardware. Patch 3 does nothing to address that issue, it just tries to hide the
> problem.
> 
> 
> BR,
> Jani.
> 
> 
> > +		panel->vbt.backlight.controller = -1;
> > +		return true;
> > +	}
> > +
> >  	parse_panel_options(i915, panel);
> >  	parse_generic_dtd(i915, panel);
> >  	parse_lfp_data(i915, panel);
> > @@ -3203,6 +3208,8 @@ void intel_bios_init_panel(struct drm_i915_private
> *i915,
> >  	parse_psr(i915, panel);
> >  	parse_mipi_config(i915, panel);
> >  	parse_mipi_sequence(i915, panel);
> > +
> > +	return false;
> >  }
> >
> >  /**
> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.h
> > b/drivers/gpu/drm/i915/display/intel_bios.h
> > index e375405a7828..f8ef0274f3ee 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bios.h
> > +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> > @@ -232,7 +232,7 @@ struct mipi_pps_data {  } __packed;
> >
> >  void intel_bios_init(struct drm_i915_private *dev_priv); -void
> > intel_bios_init_panel(struct drm_i915_private *dev_priv,
> > +bool intel_bios_init_panel(struct drm_i915_private *dev_priv,
> >  			   struct intel_panel *panel,
> >  			   const struct intel_bios_encoder_data *devdata,
> >  			   const struct edid *edid);
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 7400d6b4c587..78cf3a77f026 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5235,6 +5235,7 @@ static bool intel_edp_init_connector(struct intel_dp
> *intel_dp,
> >  	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> >  	bool has_dpcd;
> >  	struct edid *edid;
> > +	bool retry;
> >
> >  	if (!intel_dp_is_edp(intel_dp))
> >  		return true;
> > @@ -5254,6 +5255,9 @@ static bool intel_edp_init_connector(struct intel_dp
> *intel_dp,
> >  		return false;
> >  	}
> >
> > +	retry = intel_bios_init_panel(dev_priv, &intel_connector->panel,
> > +				      encoder->devdata, NULL);
> > +
> >  	intel_pps_init(intel_dp);
> >
> >  	/* Cache DPCD and EDID for edp. */
> > @@ -5288,9 +5292,9 @@ static bool intel_edp_init_connector(struct intel_dp
> *intel_dp,
> >  		edid = ERR_PTR(-ENOENT);
> >  	}
> >  	intel_connector->edid = edid;
> > -
> > -	intel_bios_init_panel(dev_priv, &intel_connector->panel,
> > -			      encoder->devdata, IS_ERR(edid) ? NULL : edid);
> > +	if (retry)
> > +		intel_bios_init_panel(dev_priv, &intel_connector->panel,
> > +				      encoder->devdata, IS_ERR(edid) ? NULL :
> edid);
> >
> >  	intel_panel_add_edid_fixed_modes(intel_connector, true);
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c
> > b/drivers/gpu/drm/i915/display/intel_pps.c
> > index 3949fb449353..5738af154bd2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_pps.c
> > +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> > @@ -219,6 +219,16 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp)
> >  	/* We should never land here with regular DP ports */
> >  	drm_WARN_ON(&dev_priv->drm, !intel_dp_is_edp(intel_dp));
> >
> > +	if (backlight_controller == -1) {
> > +		/*
> > +		 * Use 2nd PPS instance as default for 2nd EDP panel.
> > +		 */
> > +		if (connector->encoder->port == PORT_A)
> > +			return 0;
> > +		else
> > +			return 1;
> > +	}
> > +
> >  	if (!intel_dp->pps.pps_reset)
> >  		return backlight_controller;
> >
> > @@ -1430,7 +1440,7 @@ void intel_pps_init(struct intel_dp *intel_dp)
> >  	intel_dp->pps.initializing = true;
> >  	INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work,
> > edp_panel_vdd_work);
> >
> > -	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915))
> > +	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >=
> > +12)
> >  		intel_dp->get_pps_idx = bxt_power_sequencer_idx;
> >  	else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> >  		intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
> 
> --
> Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
  2022-11-09 13:54     ` Manna, Animesh
@ 2022-11-09 14:04       ` Jani Nikula
  0 siblings, 0 replies; 15+ messages in thread
From: Jani Nikula @ 2022-11-09 14:04 UTC (permalink / raw)
  To: Manna, Animesh, intel-gfx

On Wed, 09 Nov 2022, "Manna, Animesh" <animesh.manna@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani <jani.nikula@intel.com>
>> Sent: Wednesday, November 9, 2022 5:12 PM
>> To: Manna, Animesh <animesh.manna@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Manna, Animesh <animesh.manna@intel.com>; Ville Syrjälä
>> <ville.syrjala@linux.intel.com>; Shankar, Uma <uma.shankar@intel.com>
>> Subject: Re: [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
>>
>> On Wed, 09 Nov 2022, Animesh Manna <animesh.manna@intel.com> wrote:
>> > From display gen12 onwards to support dual EDP two instances of pps added.
>> > Currently backlight controller and pps instance can be mapped together
>> > for a specific panel. Currently dual PPS support is broken. This patch
>> > fixes it and enables for display 12+.
>> >
>> > v1: Iniital revision.
>> > v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init.
>> > [Jani]
>> > v3: Set pps_id to -1 for pnpid type of panel which will be used by
>> > bxt_power_sequencer_idx() to set 2nd pps instance as default for 2nd
>> > EDP panel. [Jani]
>> > v4: Early return for PANEL_TYPE_FALLBACK. [Jani]
>> > v5: Removed additional pps_id variable and reused backlight
>> > controller. [Jani]
>> >
>> > Cc: Jani Nikula <jani.nikula@intel.com>
>> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > Cc: Uma Shankar <uma.shankar@intel.com>
>> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_bios.c |  9 ++++++++-
>> > drivers/gpu/drm/i915/display/intel_bios.h |  2 +-
>> >  drivers/gpu/drm/i915/display/intel_dp.c   | 10 +++++++---
>> >  drivers/gpu/drm/i915/display/intel_pps.c  | 12 +++++++++++-
>> >  4 files changed, 27 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
>> > b/drivers/gpu/drm/i915/display/intel_bios.c
>> > index c2987f2c2b2e..1c1eea061fbb 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> > @@ -3183,7 +3183,7 @@ void intel_bios_init(struct drm_i915_private *i915)
>> >     kfree(oprom_vbt);
>> >  }
>> >
>> > -void intel_bios_init_panel(struct drm_i915_private *i915,
>> > +bool intel_bios_init_panel(struct drm_i915_private *i915,
>> >                        struct intel_panel *panel,
>> >                        const struct intel_bios_encoder_data *devdata,
>> >                        const struct edid *edid)
>> > @@ -3192,6 +3192,11 @@ void intel_bios_init_panel(struct
>> > drm_i915_private *i915,
>> >
>> >     panel->vbt.panel_type = get_panel_type(i915, devdata, edid);
>> >
>> > +   if (panel->vbt.panel_type == PANEL_TYPE_FALLBACK && !edid) {
>>
>> As Ville pointed out, "panel->vbt.panel_type == PANEL_TYPE_FALLBACK" is
>> comparing apples to oranges, and we hit this path only in the rare case that
>> panel_type == 3, regardless of where the panel type originated.
>
> Got it, tried returning some random negative number (for example -2) from fallback_get_panel_type() and checking the same here is fixing the WARN_ON.
>
>>
>> This is probably the reason patches 1&2 don't fix the original issue like it should.
>>
>> I remind you that the issue is two eDP having their own pps software states but
>> using the same pps hardware due to conflicting pps index (which is the same as
>> panel->vbt.backlight.controller). This is what we need to solve here.
>
> In this patch we decide to use the specific pps based on port number. Once we identify it is PORT B always 2nd instance of PPS will be used. All pps register get instantiated with pps_idx 1
>         if (backlight_controller == -1) {
>                 /*
>                  * Use 2nd PPS instance as default for 2nd EDP panel.
>                  */
>                 if (connector->encoder->port == PORT_A)
>                         return 0;
>                 else
>                         return 1;
>         }
> Not sure what is toggling the VDD.

The 2nd eDP obviously!

BR,
Jani.


> Anyways I agree patch 3 is not
> needed. Thanks for review.
>
> Regards,
> Animesh
>
>>
>> We get the wakeref imbalance because of two eDP trampling on the same pps
>> hardware. Patch 3 does nothing to address that issue, it just tries to hide the
>> problem.
>>
>>
>> BR,
>> Jani.
>>
>>
>> > +           panel->vbt.backlight.controller = -1;
>> > +           return true;
>> > +   }
>> > +
>> >     parse_panel_options(i915, panel);
>> >     parse_generic_dtd(i915, panel);
>> >     parse_lfp_data(i915, panel);
>> > @@ -3203,6 +3208,8 @@ void intel_bios_init_panel(struct drm_i915_private
>> *i915,
>> >     parse_psr(i915, panel);
>> >     parse_mipi_config(i915, panel);
>> >     parse_mipi_sequence(i915, panel);
>> > +
>> > +   return false;
>> >  }
>> >
>> >  /**
>> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.h
>> > b/drivers/gpu/drm/i915/display/intel_bios.h
>> > index e375405a7828..f8ef0274f3ee 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_bios.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_bios.h
>> > @@ -232,7 +232,7 @@ struct mipi_pps_data {  } __packed;
>> >
>> >  void intel_bios_init(struct drm_i915_private *dev_priv); -void
>> > intel_bios_init_panel(struct drm_i915_private *dev_priv,
>> > +bool intel_bios_init_panel(struct drm_i915_private *dev_priv,
>> >                        struct intel_panel *panel,
>> >                        const struct intel_bios_encoder_data *devdata,
>> >                        const struct edid *edid);
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> > b/drivers/gpu/drm/i915/display/intel_dp.c
>> > index 7400d6b4c587..78cf3a77f026 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> > @@ -5235,6 +5235,7 @@ static bool intel_edp_init_connector(struct intel_dp
>> *intel_dp,
>> >     struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> >     bool has_dpcd;
>> >     struct edid *edid;
>> > +   bool retry;
>> >
>> >     if (!intel_dp_is_edp(intel_dp))
>> >             return true;
>> > @@ -5254,6 +5255,9 @@ static bool intel_edp_init_connector(struct intel_dp
>> *intel_dp,
>> >             return false;
>> >     }
>> >
>> > +   retry = intel_bios_init_panel(dev_priv, &intel_connector->panel,
>> > +                                 encoder->devdata, NULL);
>> > +
>> >     intel_pps_init(intel_dp);
>> >
>> >     /* Cache DPCD and EDID for edp. */
>> > @@ -5288,9 +5292,9 @@ static bool intel_edp_init_connector(struct intel_dp
>> *intel_dp,
>> >             edid = ERR_PTR(-ENOENT);
>> >     }
>> >     intel_connector->edid = edid;
>> > -
>> > -   intel_bios_init_panel(dev_priv, &intel_connector->panel,
>> > -                         encoder->devdata, IS_ERR(edid) ? NULL : edid);
>> > +   if (retry)
>> > +           intel_bios_init_panel(dev_priv, &intel_connector->panel,
>> > +                                 encoder->devdata, IS_ERR(edid) ? NULL :
>> edid);
>> >
>> >     intel_panel_add_edid_fixed_modes(intel_connector, true);
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c
>> > b/drivers/gpu/drm/i915/display/intel_pps.c
>> > index 3949fb449353..5738af154bd2 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_pps.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_pps.c
>> > @@ -219,6 +219,16 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp)
>> >     /* We should never land here with regular DP ports */
>> >     drm_WARN_ON(&dev_priv->drm, !intel_dp_is_edp(intel_dp));
>> >
>> > +   if (backlight_controller == -1) {
>> > +           /*
>> > +            * Use 2nd PPS instance as default for 2nd EDP panel.
>> > +            */
>> > +           if (connector->encoder->port == PORT_A)
>> > +                   return 0;
>> > +           else
>> > +                   return 1;
>> > +   }
>> > +
>> >     if (!intel_dp->pps.pps_reset)
>> >             return backlight_controller;
>> >
>> > @@ -1430,7 +1440,7 @@ void intel_pps_init(struct intel_dp *intel_dp)
>> >     intel_dp->pps.initializing = true;
>> >     INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work,
>> > edp_panel_vdd_work);
>> >
>> > -   if (IS_GEMINILAKE(i915) || IS_BROXTON(i915))
>> > +   if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >=
>> > +12)
>> >             intel_dp->get_pps_idx = bxt_power_sequencer_idx;
>> >     else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>> >             intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup
  2022-11-09 11:15 [Intel-gfx] [PATCH 1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup Animesh Manna
  2022-11-09 11:15 ` [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario Animesh Manna
  2022-11-09 11:15 ` [Intel-gfx] [PATCH 3/3] drm/i915/edp: Fix warning as vdd went down without driver knowledge Animesh Manna
@ 2022-11-09 16:41 ` Patchwork
  2022-11-09 17:05 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  3 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2022-11-09 16:41 UTC (permalink / raw)
  To: Manna, Animesh; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup
URL   : https://patchwork.freedesktop.org/series/110694/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup
  2022-11-09 11:15 [Intel-gfx] [PATCH 1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup Animesh Manna
                   ` (2 preceding siblings ...)
  2022-11-09 16:41 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup Patchwork
@ 2022-11-09 17:05 ` Patchwork
  3 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2022-11-09 17:05 UTC (permalink / raw)
  To: Manna, Animesh; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 15382 bytes --]

== Series Details ==

Series: series starting with [1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup
URL   : https://patchwork.freedesktop.org/series/110694/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12362 -> Patchwork_110694v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_110694v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_110694v1, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/index.html

Participating hosts (37 -> 38)
------------------------------

  Additional (3): fi-hsw-4770 fi-rkl-11600 bat-dg1-6 
  Missing    (2): fi-ctg-p8600 fi-bxt-dsi 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_110694v1:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_psr@primary_mmap_gtt:
    - bat-adlp-4:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12362/bat-adlp-4/igt@kms_psr@primary_mmap_gtt.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-adlp-4/igt@kms_psr@primary_mmap_gtt.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - {bat-rplp-1}:       [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12362/bat-rplp-1/igt@i915_pm_rpm@basic-pci-d3-state.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-rplp-1/igt@i915_pm_rpm@basic-pci-d3-state.html

  
Known issues
------------

  Here are the changes found in Patchwork_110694v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-rkl-11600:       NOTRUN -> [SKIP][5] ([i915#2190])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-rkl-11600/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - fi-rkl-11600:       NOTRUN -> [SKIP][6] ([i915#4613]) +3 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-rkl-11600/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@gem_mmap@basic:
    - bat-dg1-6:          NOTRUN -> [SKIP][7] ([i915#4083])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@gem_mmap@basic.html

  * igt@gem_render_tiled_blits@basic:
    - bat-dg1-6:          NOTRUN -> [SKIP][8] ([i915#4079]) +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@gem_render_tiled_blits@basic.html

  * igt@gem_tiled_fence_blits@basic:
    - bat-dg1-6:          NOTRUN -> [SKIP][9] ([i915#4077]) +2 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@gem_tiled_fence_blits@basic.html

  * igt@gem_tiled_pread_basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][10] ([i915#3282])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-rkl-11600/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - bat-dg1-6:          NOTRUN -> [SKIP][11] ([i915#1155])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@i915_pm_backlight@basic-brightness.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][12] ([i915#3012])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-rkl-11600/igt@i915_pm_backlight@basic-brightness.html
    - fi-hsw-4770:        NOTRUN -> [SKIP][13] ([fdo#109271] / [i915#3012])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-hsw-4770/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rps@basic-api:
    - bat-dg1-6:          NOTRUN -> [SKIP][14] ([i915#6621])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@gt_engines:
    - bat-dg1-6:          NOTRUN -> [INCOMPLETE][15] ([i915#4418])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@i915_selftest@live@gt_engines.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-rkl-11600:       NOTRUN -> [INCOMPLETE][16] ([i915#4817])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - fi-hsw-4770:        NOTRUN -> [SKIP][17] ([fdo#109271]) +9 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-hsw-4770/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
    - bat-dg1-6:          NOTRUN -> [SKIP][18] ([i915#4215])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@kms_addfb_basic@basic-y-tiled-legacy.html

  * igt@kms_addfb_basic@tile-pitch-mismatch:
    - bat-dg1-6:          NOTRUN -> [SKIP][19] ([i915#4212]) +7 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@kms_addfb_basic@tile-pitch-mismatch.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-hsw-4770:        NOTRUN -> [SKIP][20] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-hsw-4770/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_chamelium@hdmi-crc-fast:
    - bat-dg1-6:          NOTRUN -> [SKIP][21] ([fdo#111827]) +7 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@kms_chamelium@hdmi-crc-fast.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-rkl-11600:       NOTRUN -> [SKIP][22] ([fdo#111827]) +7 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-rkl-11600/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
    - fi-rkl-11600:       NOTRUN -> [SKIP][23] ([i915#4103])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html
    - bat-dg1-6:          NOTRUN -> [SKIP][24] ([i915#4103] / [i915#4213])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-rkl-11600:       NOTRUN -> [SKIP][25] ([fdo#109285] / [i915#4098])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-rkl-11600/igt@kms_force_connector_basic@force-load-detect.html
    - bat-dg1-6:          NOTRUN -> [SKIP][26] ([fdo#109285])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
    - fi-ivb-3770:        NOTRUN -> [SKIP][27] ([fdo#109271])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-ivb-3770/igt@kms_pipe_crc_basic@suspend-read-crc.html

  * igt@kms_psr@sprite_plane_onoff:
    - bat-dg1-6:          NOTRUN -> [SKIP][28] ([i915#1072] / [i915#4078]) +3 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@kms_psr@sprite_plane_onoff.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][29] ([i915#1072]) +3 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-rkl-11600/igt@kms_psr@sprite_plane_onoff.html
    - fi-hsw-4770:        NOTRUN -> [SKIP][30] ([fdo#109271] / [i915#1072]) +3 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-hsw-4770/igt@kms_psr@sprite_plane_onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-dg1-6:          NOTRUN -> [SKIP][31] ([i915#3555])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@kms_setmode@basic-clone-single-crtc.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][32] ([i915#3555] / [i915#4098])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-gtt:
    - bat-dg1-6:          NOTRUN -> [SKIP][33] ([i915#3708] / [i915#4077]) +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@prime_vgem@basic-gtt.html

  * igt@prime_vgem@basic-read:
    - bat-dg1-6:          NOTRUN -> [SKIP][34] ([i915#3708]) +3 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@prime_vgem@basic-read.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][35] ([fdo#109295] / [i915#3291] / [i915#3708]) +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-rkl-11600/igt@prime_vgem@basic-read.html

  * igt@prime_vgem@basic-userptr:
    - bat-dg1-6:          NOTRUN -> [SKIP][36] ([i915#3708] / [i915#4873])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@prime_vgem@basic-userptr.html
    - fi-rkl-11600:       NOTRUN -> [SKIP][37] ([fdo#109295] / [i915#3301] / [i915#3708])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-rkl-11600/igt@prime_vgem@basic-userptr.html

  * igt@runner@aborted:
    - bat-dg1-6:          NOTRUN -> [FAIL][38] ([i915#4312])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-dg1-6/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@gem_exec_gttfill@basic:
    - fi-pnv-d510:        [FAIL][39] ([i915#7229]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12362/fi-pnv-d510/igt@gem_exec_gttfill@basic.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-pnv-d510/igt@gem_exec_gttfill@basic.html

  * igt@i915_selftest@live@gt_pm:
    - {bat-adln-1}:       [DMESG-FAIL][41] ([i915#4258]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12362/bat-adln-1/igt@i915_selftest@live@gt_pm.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-adln-1/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@requests:
    - {bat-rpls-1}:       [INCOMPLETE][43] ([i915#4983] / [i915#6257]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12362/bat-rpls-1/igt@i915_selftest@live@requests.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-rpls-1/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@slpc:
    - {bat-adln-1}:       [DMESG-FAIL][45] ([i915#6997]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12362/bat-adln-1/igt@i915_selftest@live@slpc.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/bat-adln-1/igt@i915_selftest@live@slpc.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size:
    - fi-bsw-kefka:       [FAIL][47] ([i915#6298]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12362/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-hdmi-a-1:
    - fi-skl-6700k2:      [INCOMPLETE][49] ([i915#7266]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12362/fi-skl-6700k2/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-hdmi-a-1.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/fi-skl-6700k2/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-hdmi-a-1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4418]: https://gitlab.freedesktop.org/drm/intel/issues/4418
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817
  [i915#4873]: https://gitlab.freedesktop.org/drm/intel/issues/4873
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5153]: https://gitlab.freedesktop.org/drm/intel/issues/5153
  [i915#6257]: https://gitlab.freedesktop.org/drm/intel/issues/6257
  [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
  [i915#7229]: https://gitlab.freedesktop.org/drm/intel/issues/7229
  [i915#7266]: https://gitlab.freedesktop.org/drm/intel/issues/7266
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456


Build changes
-------------

  * Linux: CI_DRM_12362 -> Patchwork_110694v1

  CI-20190529: 20190529
  CI_DRM_12362: 8a360df54c04866499d763c87704f7a50953462e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7048: 5edd5c539f1fdf1c02157bf43fa1fd22d4ad2c75 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_110694v1: 8a360df54c04866499d763c87704f7a50953462e @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

565f6aed7e7c drm/i915/edp: Fix warning as vdd went down without driver knowledge
3a359b509ba2 drm/i915/pps: Enable 2nd pps for dual EDP scenario
883196c05cce drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110694v1/index.html

[-- Attachment #2: Type: text/html, Size: 18145 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
  2022-11-09 10:41         ` Jani Nikula
@ 2022-11-09 11:09           ` Manna, Animesh
  0 siblings, 0 replies; 15+ messages in thread
From: Manna, Animesh @ 2022-11-09 11:09 UTC (permalink / raw)
  To: Nikula, Jani, Ville Syrjälä; +Cc: intel-gfx



> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Wednesday, November 9, 2022 4:12 PM
> To: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>
> Subject: Re: [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
> 
> On Wed, 09 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Nov 09, 2022 at 12:25:59PM +0200, Jani Nikula wrote:
> >> On Wed, 09 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >> > On Wed, Nov 09, 2022 at 02:48:21AM +0530, Animesh Manna wrote:
> >> >> >From display gen12 onwards to support dual EDP two instances of pps
> added.
> >> >> Currently backlight controller and pps instance can be mapped
> >> >> together for a specific panel. Extended support for gen12 for dual EDP
> usage.
> >> >>
> >> >> v1: Iniital revision
> >> >> v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init.
> >> >> [Jani]
> >> >>
> >> >> Cc: Jani Nikula <jani.nikula@intel.com>
> >> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >> Cc: Uma Shankar <uma.shankar@intel.com>
> >> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> >> >> ---
> >> >>  drivers/gpu/drm/i915/display/intel_bios.c | 7 -------
> >> >> drivers/gpu/drm/i915/display/intel_bios.h | 7 +++++++
> >> >>  drivers/gpu/drm/i915/display/intel_dp.c   | 9 ++++++---
> >> >>  drivers/gpu/drm/i915/display/intel_pps.c  | 2 +-
> >> >>  4 files changed, 14 insertions(+), 11 deletions(-)
> >> >>
> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
> >> >> b/drivers/gpu/drm/i915/display/intel_bios.c
> >> >> index c2987f2c2b2e..fca44be9bab8 100644
> >> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> >> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> >> >> @@ -706,13 +706,6 @@ static int fallback_get_panel_type(struct
> drm_i915_private *i915,
> >> >>  	return 0;
> >> >>  }
> >> >>
> >> >> -enum panel_type {
> >> >> -	PANEL_TYPE_OPREGION,
> >> >> -	PANEL_TYPE_VBT,
> >> >> -	PANEL_TYPE_PNPID,
> >> >> -	PANEL_TYPE_FALLBACK,
> >> >> -};
> >> >> -
> >> >>  static int get_panel_type(struct drm_i915_private *i915,
> >> >>  			  const struct intel_bios_encoder_data *devdata,
> >> >>  			  const struct edid *edid)
> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h
> >> >> b/drivers/gpu/drm/i915/display/intel_bios.h
> >> >> index e375405a7828..da01b13260ae 100644
> >> >> --- a/drivers/gpu/drm/i915/display/intel_bios.h
> >> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> >> >> @@ -231,6 +231,13 @@ struct mipi_pps_data {
> >> >>  	u16 panel_power_cycle_delay;
> >> >>  } __packed;
> >> >>
> >> >> +enum panel_type {
> >> >> +	PANEL_TYPE_OPREGION,
> >> >> +	PANEL_TYPE_VBT,
> >> >> +	PANEL_TYPE_PNPID,
> >> >> +	PANEL_TYPE_FALLBACK,
> >> >> +};
> >> >> +
> >> >>  void intel_bios_init(struct drm_i915_private *dev_priv);  void
> >> >> intel_bios_init_panel(struct drm_i915_private *dev_priv,
> >> >>  			   struct intel_panel *panel,
> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> >> >> b/drivers/gpu/drm/i915/display/intel_dp.c
> >> >> index 7400d6b4c587..08ece347f7cb 100644
> >> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> >> @@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct
> intel_dp *intel_dp,
> >> >>  		return false;
> >> >>  	}
> >> >>
> >> >> +	intel_bios_init_panel(dev_priv, &intel_connector->panel,
> >> >> +			      encoder->devdata, NULL);
> >> >> +
> >> >>  	intel_pps_init(intel_dp);
> >> >>
> >> >>  	/* Cache DPCD and EDID for edp. */ @@ -5288,9 +5291,9 @@ static
> >> >> bool intel_edp_init_connector(struct intel_dp *intel_dp,
> >> >>  		edid = ERR_PTR(-ENOENT);
> >> >>  	}
> >> >>  	intel_connector->edid = edid;
> >> >> -
> >> >> -	intel_bios_init_panel(dev_priv, &intel_connector->panel,
> >> >> -			      encoder->devdata, IS_ERR(edid) ? NULL : edid);
> >> >> +	if (intel_connector->panel.vbt.panel_type ==
> >> >> +PANEL_TYPE_FALLBACK)
> >> >
> >> > vbt.panel_type does _not_ contain enum panel_type (a bit of an
> >> > unfortunate name selection by me there I guess).
> >> >
> >> >> +		intel_bios_init_panel(dev_priv, &intel_connector->panel,
> >> >> +				      encoder->devdata, IS_ERR(edid) ? NULL :
> edid);
> >> >
> >> > I suspect just calling this twice might leak all kinds of stuff.
> >>
> >> Yeah, what's up with this, this is some old version that was fixed
> >> way back too. I thought this was good to go:
> >>
> >> https://lore.kernel.org/r/20221018083921.23239-2-animesh.manna@intel.
> >> com
> >
> > That look broken in exactly the same way wrt. enum panel_type.
> 
> You're right, of course.

My Bad, will rectify and send the new version right away ..
JFYI, With the correct initial series [1] warning is seen on [2]

[1] https://patchwork.freedesktop.org/series/109820/
[2] https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109820v2/bat-adlp-4/igt@i915_module_load@load.html

Regards,
Animesh

> 
> BR,
> Jani.
> 
> 
> >
> >>
> >> >
> >> >>
> >> >>  	intel_panel_add_edid_fixed_modes(intel_connector, true);
> >> >>
> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c
> >> >> b/drivers/gpu/drm/i915/display/intel_pps.c
> >> >> index 3949fb449353..0975e49f8d03 100644
> >> >> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> >> >> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> >> >> @@ -1430,7 +1430,7 @@ void intel_pps_init(struct intel_dp *intel_dp)
> >> >>  	intel_dp->pps.initializing = true;
> >> >>  	INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work,
> >> >> edp_panel_vdd_work);
> >> >>
> >> >> -	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915))
> >> >> +	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915)
> >> >> +>= 12)
> >> >>  		intel_dp->get_pps_idx = bxt_power_sequencer_idx;
> >> >>  	else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> >> >>  		intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
> >> >> --
> >> >> 2.29.0
> >>
> >> --
> >> Jani Nikula, Intel Open Source Graphics Center
> 
> --
> Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
  2022-11-09 10:29       ` Ville Syrjälä
@ 2022-11-09 10:41         ` Jani Nikula
  2022-11-09 11:09           ` Manna, Animesh
  0 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2022-11-09 10:41 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Wed, 09 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Nov 09, 2022 at 12:25:59PM +0200, Jani Nikula wrote:
>> On Wed, 09 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > On Wed, Nov 09, 2022 at 02:48:21AM +0530, Animesh Manna wrote:
>> >> >From display gen12 onwards to support dual EDP two instances of pps added.
>> >> Currently backlight controller and pps instance can be mapped together
>> >> for a specific panel. Extended support for gen12 for dual EDP usage.
>> >> 
>> >> v1: Iniital revision
>> >> v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani]
>> >> 
>> >> Cc: Jani Nikula <jani.nikula@intel.com>
>> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> Cc: Uma Shankar <uma.shankar@intel.com>
>> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/display/intel_bios.c | 7 -------
>> >>  drivers/gpu/drm/i915/display/intel_bios.h | 7 +++++++
>> >>  drivers/gpu/drm/i915/display/intel_dp.c   | 9 ++++++---
>> >>  drivers/gpu/drm/i915/display/intel_pps.c  | 2 +-
>> >>  4 files changed, 14 insertions(+), 11 deletions(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> >> index c2987f2c2b2e..fca44be9bab8 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> >> @@ -706,13 +706,6 @@ static int fallback_get_panel_type(struct drm_i915_private *i915,
>> >>  	return 0;
>> >>  }
>> >>  
>> >> -enum panel_type {
>> >> -	PANEL_TYPE_OPREGION,
>> >> -	PANEL_TYPE_VBT,
>> >> -	PANEL_TYPE_PNPID,
>> >> -	PANEL_TYPE_FALLBACK,
>> >> -};
>> >> -
>> >>  static int get_panel_type(struct drm_i915_private *i915,
>> >>  			  const struct intel_bios_encoder_data *devdata,
>> >>  			  const struct edid *edid)
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h
>> >> index e375405a7828..da01b13260ae 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_bios.h
>> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
>> >> @@ -231,6 +231,13 @@ struct mipi_pps_data {
>> >>  	u16 panel_power_cycle_delay;
>> >>  } __packed;
>> >>  
>> >> +enum panel_type {
>> >> +	PANEL_TYPE_OPREGION,
>> >> +	PANEL_TYPE_VBT,
>> >> +	PANEL_TYPE_PNPID,
>> >> +	PANEL_TYPE_FALLBACK,
>> >> +};
>> >> +
>> >>  void intel_bios_init(struct drm_i915_private *dev_priv);
>> >>  void intel_bios_init_panel(struct drm_i915_private *dev_priv,
>> >>  			   struct intel_panel *panel,
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> >> index 7400d6b4c587..08ece347f7cb 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> >> @@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>> >>  		return false;
>> >>  	}
>> >>  
>> >> +	intel_bios_init_panel(dev_priv, &intel_connector->panel,
>> >> +			      encoder->devdata, NULL);
>> >> +
>> >>  	intel_pps_init(intel_dp);
>> >>  
>> >>  	/* Cache DPCD and EDID for edp. */
>> >> @@ -5288,9 +5291,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>> >>  		edid = ERR_PTR(-ENOENT);
>> >>  	}
>> >>  	intel_connector->edid = edid;
>> >> -
>> >> -	intel_bios_init_panel(dev_priv, &intel_connector->panel,
>> >> -			      encoder->devdata, IS_ERR(edid) ? NULL : edid);
>> >> +	if (intel_connector->panel.vbt.panel_type == PANEL_TYPE_FALLBACK)
>> >
>> > vbt.panel_type does _not_ contain enum panel_type (a bit of an 
>> > unfortunate name selection by me there I guess).
>> >
>> >> +		intel_bios_init_panel(dev_priv, &intel_connector->panel,
>> >> +				      encoder->devdata, IS_ERR(edid) ? NULL : edid);
>> >
>> > I suspect just calling this twice might leak all kinds of stuff.
>> 
>> Yeah, what's up with this, this is some old version that was fixed way
>> back too. I thought this was good to go:
>> 
>> https://lore.kernel.org/r/20221018083921.23239-2-animesh.manna@intel.com
>
> That look broken in exactly the same way wrt. enum panel_type.

You're right, of course.

BR,
Jani.


>
>> 
>> >
>> >>  
>> >>  	intel_panel_add_edid_fixed_modes(intel_connector, true);
>> >>  
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
>> >> index 3949fb449353..0975e49f8d03 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_pps.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
>> >> @@ -1430,7 +1430,7 @@ void intel_pps_init(struct intel_dp *intel_dp)
>> >>  	intel_dp->pps.initializing = true;
>> >>  	INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work);
>> >>  
>> >> -	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915))
>> >> +	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12)
>> >>  		intel_dp->get_pps_idx = bxt_power_sequencer_idx;
>> >>  	else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>> >>  		intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
>> >> -- 
>> >> 2.29.0
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
  2022-11-09 10:25     ` Jani Nikula
@ 2022-11-09 10:29       ` Ville Syrjälä
  2022-11-09 10:41         ` Jani Nikula
  0 siblings, 1 reply; 15+ messages in thread
From: Ville Syrjälä @ 2022-11-09 10:29 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, Nov 09, 2022 at 12:25:59PM +0200, Jani Nikula wrote:
> On Wed, 09 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Wed, Nov 09, 2022 at 02:48:21AM +0530, Animesh Manna wrote:
> >> >From display gen12 onwards to support dual EDP two instances of pps added.
> >> Currently backlight controller and pps instance can be mapped together
> >> for a specific panel. Extended support for gen12 for dual EDP usage.
> >> 
> >> v1: Iniital revision
> >> v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani]
> >> 
> >> Cc: Jani Nikula <jani.nikula@intel.com>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Cc: Uma Shankar <uma.shankar@intel.com>
> >> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_bios.c | 7 -------
> >>  drivers/gpu/drm/i915/display/intel_bios.h | 7 +++++++
> >>  drivers/gpu/drm/i915/display/intel_dp.c   | 9 ++++++---
> >>  drivers/gpu/drm/i915/display/intel_pps.c  | 2 +-
> >>  4 files changed, 14 insertions(+), 11 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> >> index c2987f2c2b2e..fca44be9bab8 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> >> @@ -706,13 +706,6 @@ static int fallback_get_panel_type(struct drm_i915_private *i915,
> >>  	return 0;
> >>  }
> >>  
> >> -enum panel_type {
> >> -	PANEL_TYPE_OPREGION,
> >> -	PANEL_TYPE_VBT,
> >> -	PANEL_TYPE_PNPID,
> >> -	PANEL_TYPE_FALLBACK,
> >> -};
> >> -
> >>  static int get_panel_type(struct drm_i915_private *i915,
> >>  			  const struct intel_bios_encoder_data *devdata,
> >>  			  const struct edid *edid)
> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h
> >> index e375405a7828..da01b13260ae 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_bios.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> >> @@ -231,6 +231,13 @@ struct mipi_pps_data {
> >>  	u16 panel_power_cycle_delay;
> >>  } __packed;
> >>  
> >> +enum panel_type {
> >> +	PANEL_TYPE_OPREGION,
> >> +	PANEL_TYPE_VBT,
> >> +	PANEL_TYPE_PNPID,
> >> +	PANEL_TYPE_FALLBACK,
> >> +};
> >> +
> >>  void intel_bios_init(struct drm_i915_private *dev_priv);
> >>  void intel_bios_init_panel(struct drm_i915_private *dev_priv,
> >>  			   struct intel_panel *panel,
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index 7400d6b4c587..08ece347f7cb 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> >>  		return false;
> >>  	}
> >>  
> >> +	intel_bios_init_panel(dev_priv, &intel_connector->panel,
> >> +			      encoder->devdata, NULL);
> >> +
> >>  	intel_pps_init(intel_dp);
> >>  
> >>  	/* Cache DPCD and EDID for edp. */
> >> @@ -5288,9 +5291,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> >>  		edid = ERR_PTR(-ENOENT);
> >>  	}
> >>  	intel_connector->edid = edid;
> >> -
> >> -	intel_bios_init_panel(dev_priv, &intel_connector->panel,
> >> -			      encoder->devdata, IS_ERR(edid) ? NULL : edid);
> >> +	if (intel_connector->panel.vbt.panel_type == PANEL_TYPE_FALLBACK)
> >
> > vbt.panel_type does _not_ contain enum panel_type (a bit of an 
> > unfortunate name selection by me there I guess).
> >
> >> +		intel_bios_init_panel(dev_priv, &intel_connector->panel,
> >> +				      encoder->devdata, IS_ERR(edid) ? NULL : edid);
> >
> > I suspect just calling this twice might leak all kinds of stuff.
> 
> Yeah, what's up with this, this is some old version that was fixed way
> back too. I thought this was good to go:
> 
> https://lore.kernel.org/r/20221018083921.23239-2-animesh.manna@intel.com

That look broken in exactly the same way wrt. enum panel_type.

> 
> >
> >>  
> >>  	intel_panel_add_edid_fixed_modes(intel_connector, true);
> >>  
> >> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> >> index 3949fb449353..0975e49f8d03 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> >> @@ -1430,7 +1430,7 @@ void intel_pps_init(struct intel_dp *intel_dp)
> >>  	intel_dp->pps.initializing = true;
> >>  	INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work);
> >>  
> >> -	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915))
> >> +	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12)
> >>  		intel_dp->get_pps_idx = bxt_power_sequencer_idx;
> >>  	else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> >>  		intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
> >> -- 
> >> 2.29.0
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
  2022-11-09 10:11   ` Ville Syrjälä
@ 2022-11-09 10:25     ` Jani Nikula
  2022-11-09 10:29       ` Ville Syrjälä
  0 siblings, 1 reply; 15+ messages in thread
From: Jani Nikula @ 2022-11-09 10:25 UTC (permalink / raw)
  To: Ville Syrjälä, Animesh Manna; +Cc: intel-gfx

On Wed, 09 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Nov 09, 2022 at 02:48:21AM +0530, Animesh Manna wrote:
>> >From display gen12 onwards to support dual EDP two instances of pps added.
>> Currently backlight controller and pps instance can be mapped together
>> for a specific panel. Extended support for gen12 for dual EDP usage.
>> 
>> v1: Iniital revision
>> v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani]
>> 
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_bios.c | 7 -------
>>  drivers/gpu/drm/i915/display/intel_bios.h | 7 +++++++
>>  drivers/gpu/drm/i915/display/intel_dp.c   | 9 ++++++---
>>  drivers/gpu/drm/i915/display/intel_pps.c  | 2 +-
>>  4 files changed, 14 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> index c2987f2c2b2e..fca44be9bab8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> @@ -706,13 +706,6 @@ static int fallback_get_panel_type(struct drm_i915_private *i915,
>>  	return 0;
>>  }
>>  
>> -enum panel_type {
>> -	PANEL_TYPE_OPREGION,
>> -	PANEL_TYPE_VBT,
>> -	PANEL_TYPE_PNPID,
>> -	PANEL_TYPE_FALLBACK,
>> -};
>> -
>>  static int get_panel_type(struct drm_i915_private *i915,
>>  			  const struct intel_bios_encoder_data *devdata,
>>  			  const struct edid *edid)
>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h
>> index e375405a7828..da01b13260ae 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bios.h
>> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
>> @@ -231,6 +231,13 @@ struct mipi_pps_data {
>>  	u16 panel_power_cycle_delay;
>>  } __packed;
>>  
>> +enum panel_type {
>> +	PANEL_TYPE_OPREGION,
>> +	PANEL_TYPE_VBT,
>> +	PANEL_TYPE_PNPID,
>> +	PANEL_TYPE_FALLBACK,
>> +};
>> +
>>  void intel_bios_init(struct drm_i915_private *dev_priv);
>>  void intel_bios_init_panel(struct drm_i915_private *dev_priv,
>>  			   struct intel_panel *panel,
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 7400d6b4c587..08ece347f7cb 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>  		return false;
>>  	}
>>  
>> +	intel_bios_init_panel(dev_priv, &intel_connector->panel,
>> +			      encoder->devdata, NULL);
>> +
>>  	intel_pps_init(intel_dp);
>>  
>>  	/* Cache DPCD and EDID for edp. */
>> @@ -5288,9 +5291,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>>  		edid = ERR_PTR(-ENOENT);
>>  	}
>>  	intel_connector->edid = edid;
>> -
>> -	intel_bios_init_panel(dev_priv, &intel_connector->panel,
>> -			      encoder->devdata, IS_ERR(edid) ? NULL : edid);
>> +	if (intel_connector->panel.vbt.panel_type == PANEL_TYPE_FALLBACK)
>
> vbt.panel_type does _not_ contain enum panel_type (a bit of an 
> unfortunate name selection by me there I guess).
>
>> +		intel_bios_init_panel(dev_priv, &intel_connector->panel,
>> +				      encoder->devdata, IS_ERR(edid) ? NULL : edid);
>
> I suspect just calling this twice might leak all kinds of stuff.

Yeah, what's up with this, this is some old version that was fixed way
back too. I thought this was good to go:

https://lore.kernel.org/r/20221018083921.23239-2-animesh.manna@intel.com

>
>>  
>>  	intel_panel_add_edid_fixed_modes(intel_connector, true);
>>  
>> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
>> index 3949fb449353..0975e49f8d03 100644
>> --- a/drivers/gpu/drm/i915/display/intel_pps.c
>> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
>> @@ -1430,7 +1430,7 @@ void intel_pps_init(struct intel_dp *intel_dp)
>>  	intel_dp->pps.initializing = true;
>>  	INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work);
>>  
>> -	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915))
>> +	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12)
>>  		intel_dp->get_pps_idx = bxt_power_sequencer_idx;
>>  	else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>>  		intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
>> -- 
>> 2.29.0

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
  2022-11-08 21:18 ` [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario Animesh Manna
@ 2022-11-09 10:11   ` Ville Syrjälä
  2022-11-09 10:25     ` Jani Nikula
  0 siblings, 1 reply; 15+ messages in thread
From: Ville Syrjälä @ 2022-11-09 10:11 UTC (permalink / raw)
  To: Animesh Manna; +Cc: Jani Nikula, intel-gfx

On Wed, Nov 09, 2022 at 02:48:21AM +0530, Animesh Manna wrote:
> >From display gen12 onwards to support dual EDP two instances of pps added.
> Currently backlight controller and pps instance can be mapped together
> for a specific panel. Extended support for gen12 for dual EDP usage.
> 
> v1: Iniital revision
> v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani]
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 7 -------
>  drivers/gpu/drm/i915/display/intel_bios.h | 7 +++++++
>  drivers/gpu/drm/i915/display/intel_dp.c   | 9 ++++++---
>  drivers/gpu/drm/i915/display/intel_pps.c  | 2 +-
>  4 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index c2987f2c2b2e..fca44be9bab8 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -706,13 +706,6 @@ static int fallback_get_panel_type(struct drm_i915_private *i915,
>  	return 0;
>  }
>  
> -enum panel_type {
> -	PANEL_TYPE_OPREGION,
> -	PANEL_TYPE_VBT,
> -	PANEL_TYPE_PNPID,
> -	PANEL_TYPE_FALLBACK,
> -};
> -
>  static int get_panel_type(struct drm_i915_private *i915,
>  			  const struct intel_bios_encoder_data *devdata,
>  			  const struct edid *edid)
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h
> index e375405a7828..da01b13260ae 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.h
> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> @@ -231,6 +231,13 @@ struct mipi_pps_data {
>  	u16 panel_power_cycle_delay;
>  } __packed;
>  
> +enum panel_type {
> +	PANEL_TYPE_OPREGION,
> +	PANEL_TYPE_VBT,
> +	PANEL_TYPE_PNPID,
> +	PANEL_TYPE_FALLBACK,
> +};
> +
>  void intel_bios_init(struct drm_i915_private *dev_priv);
>  void intel_bios_init_panel(struct drm_i915_private *dev_priv,
>  			   struct intel_panel *panel,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7400d6b4c587..08ece347f7cb 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  		return false;
>  	}
>  
> +	intel_bios_init_panel(dev_priv, &intel_connector->panel,
> +			      encoder->devdata, NULL);
> +
>  	intel_pps_init(intel_dp);
>  
>  	/* Cache DPCD and EDID for edp. */
> @@ -5288,9 +5291,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  		edid = ERR_PTR(-ENOENT);
>  	}
>  	intel_connector->edid = edid;
> -
> -	intel_bios_init_panel(dev_priv, &intel_connector->panel,
> -			      encoder->devdata, IS_ERR(edid) ? NULL : edid);
> +	if (intel_connector->panel.vbt.panel_type == PANEL_TYPE_FALLBACK)

vbt.panel_type does _not_ contain enum panel_type (a bit of an 
unfortunate name selection by me there I guess).

> +		intel_bios_init_panel(dev_priv, &intel_connector->panel,
> +				      encoder->devdata, IS_ERR(edid) ? NULL : edid);

I suspect just calling this twice might leak all kinds of stuff.

>  
>  	intel_panel_add_edid_fixed_modes(intel_connector, true);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index 3949fb449353..0975e49f8d03 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -1430,7 +1430,7 @@ void intel_pps_init(struct intel_dp *intel_dp)
>  	intel_dp->pps.initializing = true;
>  	INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work);
>  
> -	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915))
> +	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12)
>  		intel_dp->get_pps_idx = bxt_power_sequencer_idx;
>  	else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>  		intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
> -- 
> 2.29.0

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario
  2022-11-08 21:18 [Intel-gfx] [PATCH 1/3] " Animesh Manna
@ 2022-11-08 21:18 ` Animesh Manna
  2022-11-09 10:11   ` Ville Syrjälä
  0 siblings, 1 reply; 15+ messages in thread
From: Animesh Manna @ 2022-11-08 21:18 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

From display gen12 onwards to support dual EDP two instances of pps added.
Currently backlight controller and pps instance can be mapped together
for a specific panel. Extended support for gen12 for dual EDP usage.

v1: Iniital revision
v2: Called intel_bios_panel_init w/o PNPID before intel_pps_init. [Jani]

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 7 -------
 drivers/gpu/drm/i915/display/intel_bios.h | 7 +++++++
 drivers/gpu/drm/i915/display/intel_dp.c   | 9 ++++++---
 drivers/gpu/drm/i915/display/intel_pps.c  | 2 +-
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index c2987f2c2b2e..fca44be9bab8 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -706,13 +706,6 @@ static int fallback_get_panel_type(struct drm_i915_private *i915,
 	return 0;
 }
 
-enum panel_type {
-	PANEL_TYPE_OPREGION,
-	PANEL_TYPE_VBT,
-	PANEL_TYPE_PNPID,
-	PANEL_TYPE_FALLBACK,
-};
-
 static int get_panel_type(struct drm_i915_private *i915,
 			  const struct intel_bios_encoder_data *devdata,
 			  const struct edid *edid)
diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h
index e375405a7828..da01b13260ae 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.h
+++ b/drivers/gpu/drm/i915/display/intel_bios.h
@@ -231,6 +231,13 @@ struct mipi_pps_data {
 	u16 panel_power_cycle_delay;
 } __packed;
 
+enum panel_type {
+	PANEL_TYPE_OPREGION,
+	PANEL_TYPE_VBT,
+	PANEL_TYPE_PNPID,
+	PANEL_TYPE_FALLBACK,
+};
+
 void intel_bios_init(struct drm_i915_private *dev_priv);
 void intel_bios_init_panel(struct drm_i915_private *dev_priv,
 			   struct intel_panel *panel,
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 7400d6b4c587..08ece347f7cb 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5254,6 +5254,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 		return false;
 	}
 
+	intel_bios_init_panel(dev_priv, &intel_connector->panel,
+			      encoder->devdata, NULL);
+
 	intel_pps_init(intel_dp);
 
 	/* Cache DPCD and EDID for edp. */
@@ -5288,9 +5291,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 		edid = ERR_PTR(-ENOENT);
 	}
 	intel_connector->edid = edid;
-
-	intel_bios_init_panel(dev_priv, &intel_connector->panel,
-			      encoder->devdata, IS_ERR(edid) ? NULL : edid);
+	if (intel_connector->panel.vbt.panel_type == PANEL_TYPE_FALLBACK)
+		intel_bios_init_panel(dev_priv, &intel_connector->panel,
+				      encoder->devdata, IS_ERR(edid) ? NULL : edid);
 
 	intel_panel_add_edid_fixed_modes(intel_connector, true);
 
diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
index 3949fb449353..0975e49f8d03 100644
--- a/drivers/gpu/drm/i915/display/intel_pps.c
+++ b/drivers/gpu/drm/i915/display/intel_pps.c
@@ -1430,7 +1430,7 @@ void intel_pps_init(struct intel_dp *intel_dp)
 	intel_dp->pps.initializing = true;
 	INIT_DELAYED_WORK(&intel_dp->pps.panel_vdd_work, edp_panel_vdd_work);
 
-	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915))
+	if (IS_GEMINILAKE(i915) || IS_BROXTON(i915) || DISPLAY_VER(i915) >= 12)
 		intel_dp->get_pps_idx = bxt_power_sequencer_idx;
 	else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
 		intel_dp->get_pps_idx = vlv_power_sequencer_pipe;
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-11-09 17:05 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-09 11:15 [Intel-gfx] [PATCH 1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup Animesh Manna
2022-11-09 11:15 ` [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario Animesh Manna
2022-11-09 11:42   ` Jani Nikula
2022-11-09 13:54     ` Manna, Animesh
2022-11-09 14:04       ` Jani Nikula
2022-11-09 11:15 ` [Intel-gfx] [PATCH 3/3] drm/i915/edp: Fix warning as vdd went down without driver knowledge Animesh Manna
2022-11-09 11:46   ` Jani Nikula
2022-11-09 16:41 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/pps: Add get_pps_idx() hook as part of pps_get_register() cleanup Patchwork
2022-11-09 17:05 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2022-11-08 21:18 [Intel-gfx] [PATCH 1/3] " Animesh Manna
2022-11-08 21:18 ` [Intel-gfx] [PATCH 2/3] drm/i915/pps: Enable 2nd pps for dual EDP scenario Animesh Manna
2022-11-09 10:11   ` Ville Syrjälä
2022-11-09 10:25     ` Jani Nikula
2022-11-09 10:29       ` Ville Syrjälä
2022-11-09 10:41         ` Jani Nikula
2022-11-09 11:09           ` Manna, Animesh

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.