[PATCH 0/6] drm/i915: remove pch_edp encoder state variable

[PATCH 0/6] drm/i915: remove pch_edp encoder state variable

[Imre Deak]

Since whether a specific eDP port is on CPU or on PCH is fixed and
depends only on which digital port it is, we don't really need state
tracking for it. So remove it and instead check for the port and if
needed the machine type (for ValleyView).

The assumption in the code before and after the change:

Before ILK there is no eDP. Starting from ILK we can have eDP ports on
port A and D:

Port A and for ValleyView port C is always a CPU side eDP port, if the
port exists.

Port D is a PCH side eDP port if there is a PCH and VBT says it's an eDP
port.

Tested only on IVB.

Imre Deak (6):
  drm/i915: use enc_to_intel_dp() instead of opencoding the same
  drm/i915: hsw: replace !is_pch_edp() with port==PORT_A
  drm/i915: ilk-ivb: replace is_pch_edp() with port==PORT_D
  drm/i915: stop using is_pch_edp() in intel_dp_init_connector()
  drm/i915: stop using is_pch_edp() in is_cpu_edp()
  drm/i915: remove is_pch_edp() helpers and state variable

 drivers/gpu/drm/i915/intel_display.c |    4 +-
 drivers/gpu/drm/i915/intel_dp.c      |   79 +++++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h     |   14 +++---
 3 files changed, 36 insertions(+), 61 deletions(-)

-- 
1.7.10.4

[PATCH 1/6] drm/i915: use enc_to_intel_dp() instead of opencoding the same

[Imre Deak]

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6e01393..c24ee02 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -621,19 +621,17 @@ static inline struct intel_encoder *intel_attached_encoder(struct drm_connector
 	return to_intel_connector(connector)->encoder;
 }
 
-static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
-{
-	struct intel_digital_port *intel_dig_port =
-		container_of(encoder, struct intel_digital_port, base.base);
-	return &intel_dig_port->dp;
-}
-
 static inline struct intel_digital_port *
 enc_to_dig_port(struct drm_encoder *encoder)
 {
 	return container_of(encoder, struct intel_digital_port, base.base);
 }
 
+static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
+{
+	return &enc_to_dig_port(encoder)->dp;
+}
+
 static inline struct intel_digital_port *
 dp_to_dig_port(struct intel_dp *intel_dp)
 {
-- 
1.7.10.4

[PATCH 2/6] drm/i915: hsw: replace !is_pch_edp() with port==PORT_A

[Imre Deak]

On HSW the CPU side eDP is always on port-A, the PCH side eDP is always
on port-D.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0ed764d..5b689b2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5976,7 +5976,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
 		switch (encoder->type) {
 		case INTEL_OUTPUT_EDP:
-			if (!intel_encoder_is_pch_edp(&encoder->base))
+			if (enc_to_dig_port(&encoder->base)->port == PORT_A)
 				is_cpu_edp = true;
 			break;
 		}
-- 
1.7.10.4

[PATCH 3/6] drm/i915: ilk-ivb: replace is_pch_edp() with port==PORT_D

[Imre Deak]

On ILK-IVB the PCH side eDP is always on port-D.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5b689b2..bdf07b7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5067,7 +5067,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
 			break;
 		case INTEL_OUTPUT_EDP:
 			has_panel = true;
-			if (intel_encoder_is_pch_edp(&encoder->base))
+			if (enc_to_dig_port(&encoder->base)->port == PORT_D)
 				has_pch_edp = true;
 			else
 				has_cpu_edp = true;
-- 
1.7.10.4

[PATCH 4/6] drm/i915: stop using is_pch_edp() in intel_dp_init_connector()

[Imre Deak]

is_pch_edp() will be removed in a follow-up patch, so replace it
with a check for the port and VBT info (for port-D eDP).

Also make things a bit clearer by using a switch on the ports.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d5a3717..31c6129 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3010,24 +3010,34 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		if (intel_dpd_is_edp(dev))
 			intel_dp->is_pch_edp = true;
 
+	type = DRM_MODE_CONNECTOR_DisplayPort;
 	/*
 	 * FIXME : We need to initialize built-in panels before external panels.
 	 * For X0, DP_C is fixed as eDP. Revisit this as part of VLV eDP cleanup
 	 */
-	if (IS_VALLEYVIEW(dev) && port == PORT_C) {
-		type = DRM_MODE_CONNECTOR_eDP;
-		intel_encoder->type = INTEL_OUTPUT_EDP;
-	} else if (port == PORT_A || is_pch_edp(intel_dp)) {
+	switch (port) {
+	case PORT_A:
 		type = DRM_MODE_CONNECTOR_eDP;
-		intel_encoder->type = INTEL_OUTPUT_EDP;
-	} else {
-		/* The intel_encoder->type value may be INTEL_OUTPUT_UNKNOWN for
-		 * DDI or INTEL_OUTPUT_DISPLAYPORT for the older gens, so don't
-		 * rewrite it.
-		 */
-		type = DRM_MODE_CONNECTOR_DisplayPort;
+		break;
+	case PORT_C:
+		if (IS_VALLEYVIEW(dev))
+			type = DRM_MODE_CONNECTOR_eDP;
+		break;
+	case PORT_D:
+		if (HAS_PCH_SPLIT(dev) && intel_dpd_is_edp(dev))
+			type = DRM_MODE_CONNECTOR_eDP;
+		break;
+	default:	/* silence GCC warning */
+		break;
 	}
 
+	/* The intel_encoder->type value may be INTEL_OUTPUT_UNKNOWN for
+	 * DDI or INTEL_OUTPUT_DISPLAYPORT for the older gens, so don't
+	 * rewrite it.
+	 */
+	if (type == DRM_MODE_CONNECTOR_eDP)
+		intel_encoder->type = INTEL_OUTPUT_EDP;
+
 	drm_connector_init(dev, connector, &intel_dp_connector_funcs, type);
 	drm_connector_helper_add(connector, &intel_dp_connector_helper_funcs);
 
-- 
1.7.10.4

[PATCH 5/6] drm/i915: stop using is_pch_edp() in is_cpu_edp()

[Imre Deak]

is_pch_edp() will be removed by the next patch, so replace it by a check
for the port and device type.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 31c6129..df1a446 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -65,6 +65,13 @@ static bool is_pch_edp(struct intel_dp *intel_dp)
 	return intel_dp->is_pch_edp;
 }
 
+static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+
+	return intel_dig_port->base.base.dev;
+}
+
 /**
  * is_cpu_edp - is the port on the CPU and attached to an eDP panel?
  * @intel_dp: DP struct
@@ -73,14 +80,12 @@ static bool is_pch_edp(struct intel_dp *intel_dp)
  */
 static bool is_cpu_edp(struct intel_dp *intel_dp)
 {
-	return is_edp(intel_dp) && !is_pch_edp(intel_dp);
-}
-
-static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
-{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	enum port port = intel_dig_port->port;
 
-	return intel_dig_port->base.base.dev;
+	return is_edp(intel_dp) &&
+		(port == PORT_A || (port == PORT_C && IS_VALLEYVIEW(dev)));
 }
 
 static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
-- 
1.7.10.4

[PATCH 6/6] drm/i915: remove is_pch_edp() helpers and state variable

[Imre Deak]

There are no more users for these, so remove them.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  |   36 ------------------------------------
 drivers/gpu/drm/i915/intel_drv.h |    2 --
 2 files changed, 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index df1a446..a8c329b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -52,19 +52,6 @@ static bool is_edp(struct intel_dp *intel_dp)
 	return intel_dig_port->base.type == INTEL_OUTPUT_EDP;
 }
 
-/**
- * is_pch_edp - is the port on the PCH and attached to an eDP panel?
- * @intel_dp: DP struct
- *
- * Returns true if the given DP struct corresponds to a PCH DP port attached
- * to an eDP panel, false otherwise.  Helpful for determining whether we
- * may need FDI resources for a given DP output or not.
- */
-static bool is_pch_edp(struct intel_dp *intel_dp)
-{
-	return intel_dp->is_pch_edp;
-}
-
 static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -93,25 +80,6 @@ static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
 	return enc_to_intel_dp(&intel_attached_encoder(connector)->base);
 }
 
-/**
- * intel_encoder_is_pch_edp - is the given encoder a PCH attached eDP?
- * @encoder: DRM encoder
- *
- * Return true if @encoder corresponds to a PCH attached eDP panel.  Needed
- * by intel_display.c.
- */
-bool intel_encoder_is_pch_edp(struct drm_encoder *encoder)
-{
-	struct intel_dp *intel_dp;
-
-	if (!encoder)
-		return false;
-
-	intel_dp = enc_to_intel_dp(encoder);
-
-	return is_pch_edp(intel_dp);
-}
-
 static void intel_dp_link_down(struct intel_dp *intel_dp);
 
 static int
@@ -3011,10 +2979,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	intel_dp->DP = I915_READ(intel_dp->output_reg);
 	intel_dp->attached_connector = intel_connector;
 
-	if (HAS_PCH_SPLIT(dev) && port == PORT_D)
-		if (intel_dpd_is_edp(dev))
-			intel_dp->is_pch_edp = true;
-
 	type = DRM_MODE_CONNECTOR_DisplayPort;
 	/*
 	 * FIXME : We need to initialize built-in panels before external panels.
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c24ee02..388a394 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -451,7 +451,6 @@ struct intel_dp {
 	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
 	struct i2c_adapter adapter;
 	struct i2c_algo_dp_aux_data algo;
-	bool is_pch_edp;
 	uint8_t train_set[4];
 	int panel_power_up_delay;
 	int panel_power_down_delay;
@@ -566,7 +565,6 @@ extern void ironlake_edp_panel_on(struct intel_dp *intel_dp);
 extern void ironlake_edp_panel_off(struct intel_dp *intel_dp);
 extern void ironlake_edp_panel_vdd_on(struct intel_dp *intel_dp);
 extern void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
-extern bool intel_encoder_is_pch_edp(struct drm_encoder *encoder);
 extern int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
 extern void intel_flush_display_plane(struct drm_i915_private *dev_priv,
 				      enum plane plane);
-- 
1.7.10.4

Re: [PATCH 3/6] drm/i915: ilk-ivb: replace is_pch_edp() with port==PORT_D

[Daniel Vetter]

On Tue, May 7, 2013 at 5:24 PM, Imre Deak <imre.deak@intel.com> wrote:
> On ILK-IVB the PCH side eDP is always on port-D.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5b689b2..bdf07b7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5067,7 +5067,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
>                         break;
>                 case INTEL_OUTPUT_EDP:
>                         has_panel = true;
> -                       if (intel_encoder_is_pch_edp(&encoder->base))
> +                       if (enc_to_dig_port(&encoder->base)->port == PORT_D)
>                                 has_pch_edp = true;
>                         else
>                                 has_cpu_edp = true;

The only place we use that is for a debug output. Imo it's better to
kill this here and move any relevant debug output to intel_dp_init (if
there's anything missing).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/6] drm/i915: stop using is_pch_edp() in intel_dp_init_connector()

[Ville Syrjälä]

On Tue, May 07, 2013 at 06:24:11PM +0300, Imre Deak wrote:
> is_pch_edp() will be removed in a follow-up patch, so replace it
> with a check for the port and VBT info (for port-D eDP).
> 
> Also make things a bit clearer by using a switch on the ports.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d5a3717..31c6129 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3010,24 +3010,34 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  		if (intel_dpd_is_edp(dev))
>  			intel_dp->is_pch_edp = true;
>  
> +	type = DRM_MODE_CONNECTOR_DisplayPort;
>  	/*
>  	 * FIXME : We need to initialize built-in panels before external panels.
>  	 * For X0, DP_C is fixed as eDP. Revisit this as part of VLV eDP cleanup
>  	 */
> -	if (IS_VALLEYVIEW(dev) && port == PORT_C) {
> -		type = DRM_MODE_CONNECTOR_eDP;
> -		intel_encoder->type = INTEL_OUTPUT_EDP;
> -	} else if (port == PORT_A || is_pch_edp(intel_dp)) {
> +	switch (port) {
> +	case PORT_A:
>  		type = DRM_MODE_CONNECTOR_eDP;
> -		intel_encoder->type = INTEL_OUTPUT_EDP;
> -	} else {
> -		/* The intel_encoder->type value may be INTEL_OUTPUT_UNKNOWN for
> -		 * DDI or INTEL_OUTPUT_DISPLAYPORT for the older gens, so don't
> -		 * rewrite it.
> -		 */
> -		type = DRM_MODE_CONNECTOR_DisplayPort;
> +		break;
> +	case PORT_C:
> +		if (IS_VALLEYVIEW(dev))
> +			type = DRM_MODE_CONNECTOR_eDP;
> +		break;
> +	case PORT_D:
> +		if (HAS_PCH_SPLIT(dev) && intel_dpd_is_edp(dev))
> +			type = DRM_MODE_CONNECTOR_eDP;
> +		break;
> +	default:	/* silence GCC warning */
> +		break;
>  	}
>  
> +	/* The intel_encoder->type value may be INTEL_OUTPUT_UNKNOWN for
> +	 * DDI or INTEL_OUTPUT_DISPLAYPORT for the older gens, so don't
> +	 * rewrite it.
> +	 */
> +	if (type == DRM_MODE_CONNECTOR_eDP)
> +		intel_encoder->type = INTEL_OUTPUT_EDP;

The comment is a bit confusing since now it's in the code path that can
rewrite it. Perhaps add some mention of eDP to the comment as well, so
that people understand that it's still valid for other kinds of
connectors.

> +
>  	drm_connector_init(dev, connector, &intel_dp_connector_funcs, type);
>  	drm_connector_helper_add(connector, &intel_dp_connector_helper_funcs);
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH v2 0/7] drm/i915: remove pch_edp encoder state variable

[Imre Deak]

Since whether a specific eDP port is on CPU or on PCH is fixed and
depends only on which digital port it is, we don't really need state
tracking for it. So remove it and instead check for the port and if
needed the machine type (for ValleyView).

The assumption in the code before and after the change:

Before ILK there is no eDP. Starting from ILK we can have eDP ports on
port A and D:

Port A and for ValleyView port C is always a CPU side eDP port, if the
port exists.

Port D is a PCH side eDP port if there is a PCH and VBT says it's an eDP
port.

Tested only on IVB.

v2:
- reduce debug verbosity (Daniel)
- make comment about not setting the encoder type clearer (Ville)

Imre Deak (7):
  drm/i915: use enc_to_intel_dp() instead of opencoding the same
  drm/i915: hsw: replace !is_pch_edp() with port==PORT_A
  drm/i915: ilk-ivb: replace !is_pch_edp() with port==PORT_A
  drm/i915: stop using is_pch_edp() in intel_dp_init_connector()
  drm/i915: stop using is_pch_edp() in is_cpu_edp()
  drm/i915: remove is_pch_edp() helpers and state variable
  drm/i915: print DP init debug messages from a single place

 drivers/gpu/drm/i915/intel_display.c |   24 +++-------
 drivers/gpu/drm/i915/intel_dp.c      |   84 ++++++++++++++--------------------
 drivers/gpu/drm/i915/intel_drv.h     |   14 ++----
 3 files changed, 46 insertions(+), 76 deletions(-)

-- 
1.7.10.4

[PATCH v2 1/7] drm/i915: use enc_to_intel_dp() instead of opencoding the same

[Imre Deak]

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6e01393..c24ee02 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -621,19 +621,17 @@ static inline struct intel_encoder *intel_attached_encoder(struct drm_connector
 	return to_intel_connector(connector)->encoder;
 }
 
-static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
-{
-	struct intel_digital_port *intel_dig_port =
-		container_of(encoder, struct intel_digital_port, base.base);
-	return &intel_dig_port->dp;
-}
-
 static inline struct intel_digital_port *
 enc_to_dig_port(struct drm_encoder *encoder)
 {
 	return container_of(encoder, struct intel_digital_port, base.base);
 }
 
+static inline struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
+{
+	return &enc_to_dig_port(encoder)->dp;
+}
+
 static inline struct intel_digital_port *
 dp_to_dig_port(struct intel_dp *intel_dp)
 {
-- 
1.7.10.4

[PATCH v2 2/7] drm/i915: hsw: replace !is_pch_edp() with port==PORT_A

[Imre Deak]

On HSW the CPU side eDP is always on port-A, the PCH side eDP is always
on port-D.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0ed764d..5b689b2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5976,7 +5976,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
 		switch (encoder->type) {
 		case INTEL_OUTPUT_EDP:
-			if (!intel_encoder_is_pch_edp(&encoder->base))
+			if (enc_to_dig_port(&encoder->base)->port == PORT_A)
 				is_cpu_edp = true;
 			break;
 		}
-- 
1.7.10.4

[PATCH v2 3/7] drm/i915: ilk-ivb: replace !is_pch_edp() with port==PORT_A

[Imre Deak]

On ILK-IVB the CPU side eDP is always on port-A.

Also reduce somewhat the debug verbosity.

v2:
- reduce debug verbosity

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5b689b2..bcadc19 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5052,7 +5052,6 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
 	u32 val, final;
 	bool has_lvds = false;
 	bool has_cpu_edp = false;
-	bool has_pch_edp = false;
 	bool has_panel = false;
 	bool has_ck505 = false;
 	bool can_ssc = false;
@@ -5067,9 +5066,7 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
 			break;
 		case INTEL_OUTPUT_EDP:
 			has_panel = true;
-			if (intel_encoder_is_pch_edp(&encoder->base))
-				has_pch_edp = true;
-			else
+			if (enc_to_dig_port(&encoder->base)->port == PORT_A)
 				has_cpu_edp = true;
 			break;
 		}
@@ -5083,9 +5080,8 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
 		can_ssc = true;
 	}
 
-	DRM_DEBUG_KMS("has_panel %d has_lvds %d has_pch_edp %d has_cpu_edp %d has_ck505 %d\n",
-		      has_panel, has_lvds, has_pch_edp, has_cpu_edp,
-		      has_ck505);
+	DRM_DEBUG_KMS("has_panel %d has_lvds %d has_ck505 %d\n",
+		      has_panel, has_lvds, has_ck505);
 
 	/* Ironlake: try to setup display ref clock before DPLL
 	 * enabling. This is only under driver's control after
-- 
1.7.10.4

[PATCH v2 4/7] drm/i915: stop using is_pch_edp() in intel_dp_init_connector()

[Imre Deak]

is_pch_edp() will be removed in a follow-up patch, so replace it
with a check for the port and VBT info (for port-D eDP).

Also make things a bit clearer by using a switch on the ports.

v2:
- make the comment about not setting the conder type for DP clearer
  (Ville)

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d5a3717..42c11dd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3010,24 +3010,35 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		if (intel_dpd_is_edp(dev))
 			intel_dp->is_pch_edp = true;
 
+	type = DRM_MODE_CONNECTOR_DisplayPort;
 	/*
 	 * FIXME : We need to initialize built-in panels before external panels.
 	 * For X0, DP_C is fixed as eDP. Revisit this as part of VLV eDP cleanup
 	 */
-	if (IS_VALLEYVIEW(dev) && port == PORT_C) {
-		type = DRM_MODE_CONNECTOR_eDP;
-		intel_encoder->type = INTEL_OUTPUT_EDP;
-	} else if (port == PORT_A || is_pch_edp(intel_dp)) {
+	switch (port) {
+	case PORT_A:
 		type = DRM_MODE_CONNECTOR_eDP;
-		intel_encoder->type = INTEL_OUTPUT_EDP;
-	} else {
-		/* The intel_encoder->type value may be INTEL_OUTPUT_UNKNOWN for
-		 * DDI or INTEL_OUTPUT_DISPLAYPORT for the older gens, so don't
-		 * rewrite it.
-		 */
-		type = DRM_MODE_CONNECTOR_DisplayPort;
+		break;
+	case PORT_C:
+		if (IS_VALLEYVIEW(dev))
+			type = DRM_MODE_CONNECTOR_eDP;
+		break;
+	case PORT_D:
+		if (HAS_PCH_SPLIT(dev) && intel_dpd_is_edp(dev))
+			type = DRM_MODE_CONNECTOR_eDP;
+		break;
+	default:	/* silence GCC warning */
+		break;
 	}
 
+	/*
+	 * For eDP we always set the encoder type to INTEL_OUTPUT_EDP, but
+	 * for DP the encoder type can be set by the caller to
+	 * INTEL_OUTPUT_UNKNOWN for DDI, so don't rewrite it.
+	 */
+	if (type == DRM_MODE_CONNECTOR_eDP)
+		intel_encoder->type = INTEL_OUTPUT_EDP;
+
 	drm_connector_init(dev, connector, &intel_dp_connector_funcs, type);
 	drm_connector_helper_add(connector, &intel_dp_connector_helper_funcs);
 
-- 
1.7.10.4

[PATCH v2 5/7] drm/i915: stop using is_pch_edp() in is_cpu_edp()

[Imre Deak]

is_pch_edp() will be removed by the next patch, so replace it by a check
for the port and device type.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 42c11dd..38b9cd2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -65,6 +65,13 @@ static bool is_pch_edp(struct intel_dp *intel_dp)
 	return intel_dp->is_pch_edp;
 }
 
+static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+
+	return intel_dig_port->base.base.dev;
+}
+
 /**
  * is_cpu_edp - is the port on the CPU and attached to an eDP panel?
  * @intel_dp: DP struct
@@ -73,14 +80,12 @@ static bool is_pch_edp(struct intel_dp *intel_dp)
  */
 static bool is_cpu_edp(struct intel_dp *intel_dp)
 {
-	return is_edp(intel_dp) && !is_pch_edp(intel_dp);
-}
-
-static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
-{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	enum port port = intel_dig_port->port;
 
-	return intel_dig_port->base.base.dev;
+	return is_edp(intel_dp) &&
+		(port == PORT_A || (port == PORT_C && IS_VALLEYVIEW(dev)));
 }
 
 static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
-- 
1.7.10.4

[PATCH v2 6/7] drm/i915: remove is_pch_edp() helpers and state variable

[Imre Deak]

There are no more users for these, so remove them.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  |   36 ------------------------------------
 drivers/gpu/drm/i915/intel_drv.h |    2 --
 2 files changed, 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 38b9cd2..42ceff5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -52,19 +52,6 @@ static bool is_edp(struct intel_dp *intel_dp)
 	return intel_dig_port->base.type == INTEL_OUTPUT_EDP;
 }
 
-/**
- * is_pch_edp - is the port on the PCH and attached to an eDP panel?
- * @intel_dp: DP struct
- *
- * Returns true if the given DP struct corresponds to a PCH DP port attached
- * to an eDP panel, false otherwise.  Helpful for determining whether we
- * may need FDI resources for a given DP output or not.
- */
-static bool is_pch_edp(struct intel_dp *intel_dp)
-{
-	return intel_dp->is_pch_edp;
-}
-
 static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -93,25 +80,6 @@ static struct intel_dp *intel_attached_dp(struct drm_connector *connector)
 	return enc_to_intel_dp(&intel_attached_encoder(connector)->base);
 }
 
-/**
- * intel_encoder_is_pch_edp - is the given encoder a PCH attached eDP?
- * @encoder: DRM encoder
- *
- * Return true if @encoder corresponds to a PCH attached eDP panel.  Needed
- * by intel_display.c.
- */
-bool intel_encoder_is_pch_edp(struct drm_encoder *encoder)
-{
-	struct intel_dp *intel_dp;
-
-	if (!encoder)
-		return false;
-
-	intel_dp = enc_to_intel_dp(encoder);
-
-	return is_pch_edp(intel_dp);
-}
-
 static void intel_dp_link_down(struct intel_dp *intel_dp);
 
 static int
@@ -3011,10 +2979,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	intel_dp->DP = I915_READ(intel_dp->output_reg);
 	intel_dp->attached_connector = intel_connector;
 
-	if (HAS_PCH_SPLIT(dev) && port == PORT_D)
-		if (intel_dpd_is_edp(dev))
-			intel_dp->is_pch_edp = true;
-
 	type = DRM_MODE_CONNECTOR_DisplayPort;
 	/*
 	 * FIXME : We need to initialize built-in panels before external panels.
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c24ee02..388a394 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -451,7 +451,6 @@ struct intel_dp {
 	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
 	struct i2c_adapter adapter;
 	struct i2c_algo_dp_aux_data algo;
-	bool is_pch_edp;
 	uint8_t train_set[4];
 	int panel_power_up_delay;
 	int panel_power_down_delay;
@@ -566,7 +565,6 @@ extern void ironlake_edp_panel_on(struct intel_dp *intel_dp);
 extern void ironlake_edp_panel_off(struct intel_dp *intel_dp);
 extern void ironlake_edp_panel_vdd_on(struct intel_dp *intel_dp);
 extern void ironlake_edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
-extern bool intel_encoder_is_pch_edp(struct drm_encoder *encoder);
 extern int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
 extern void intel_flush_display_plane(struct drm_i915_private *dev_priv,
 				      enum plane plane);
-- 
1.7.10.4

[PATCH v2 7/7] drm/i915: print DP init debug messages from a single place

[Imre Deak]

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   12 +++---------
 drivers/gpu/drm/i915/intel_dp.c      |    4 ++++
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bcadc19..9472ebc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8828,10 +8828,8 @@ static void intel_setup_outputs(struct drm_device *dev)
 				intel_hdmi_init(dev, GEN4_HDMIB, PORT_B);
 			}
 
-			if (!found && SUPPORTS_INTEGRATED_DP(dev)) {
-				DRM_DEBUG_KMS("probing DP_B\n");
+			if (!found && SUPPORTS_INTEGRATED_DP(dev))
 				intel_dp_init(dev, DP_B, PORT_B);
-			}
 		}
 
 		/* Before G4X SDVOC doesn't have its own detect register */
@@ -8847,17 +8845,13 @@ static void intel_setup_outputs(struct drm_device *dev)
 				DRM_DEBUG_KMS("probing HDMI on SDVOC\n");
 				intel_hdmi_init(dev, GEN4_HDMIC, PORT_C);
 			}
-			if (SUPPORTS_INTEGRATED_DP(dev)) {
-				DRM_DEBUG_KMS("probing DP_C\n");
+			if (SUPPORTS_INTEGRATED_DP(dev))
 				intel_dp_init(dev, DP_C, PORT_C);
-			}
 		}
 
 		if (SUPPORTS_INTEGRATED_DP(dev) &&
-		    (I915_READ(DP_D) & DP_DETECTED)) {
-			DRM_DEBUG_KMS("probing DP_D\n");
+		    (I915_READ(DP_D) & DP_DETECTED))
 			intel_dp_init(dev, DP_D, PORT_D);
-		}
 	} else if (IS_GEN2(dev))
 		intel_dvo_init(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 42ceff5..92d1334 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3008,6 +3008,10 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	if (type == DRM_MODE_CONNECTOR_eDP)
 		intel_encoder->type = INTEL_OUTPUT_EDP;
 
+	DRM_DEBUG_KMS("Adding %s connector on port %c\n",
+			type == DRM_MODE_CONNECTOR_eDP ? "eDP" : "DP",
+			port_name(port));
+
 	drm_connector_init(dev, connector, &intel_dp_connector_funcs, type);
 	drm_connector_helper_add(connector, &intel_dp_connector_helper_funcs);
 
-- 
1.7.10.4

Re: [PATCH v2 5/7] drm/i915: stop using is_pch_edp() in is_cpu_edp()

[Daniel Vetter]

On Wed, May 08, 2013 at 01:14:06PM +0300, Imre Deak wrote:
> is_pch_edp() will be removed by the next patch, so replace it by a check
> for the port and device type.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 42c11dd..38b9cd2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -65,6 +65,13 @@ static bool is_pch_edp(struct intel_dp *intel_dp)
>  	return intel_dp->is_pch_edp;
>  }
>  
> +static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +
> +	return intel_dig_port->base.base.dev;
> +}
> +
>  /**
>   * is_cpu_edp - is the port on the CPU and attached to an eDP panel?
>   * @intel_dp: DP struct
> @@ -73,14 +80,12 @@ static bool is_pch_edp(struct intel_dp *intel_dp)
>   */
>  static bool is_cpu_edp(struct intel_dp *intel_dp)
>  {
> -	return is_edp(intel_dp) && !is_pch_edp(intel_dp);
> -}
> -
> -static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
> -{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	enum port port = intel_dig_port->port;
>  
> -	return intel_dig_port->base.base.dev;
> +	return is_edp(intel_dp) &&
> +		(port == PORT_A || (port == PORT_C && IS_VALLEYVIEW(dev)));

Valleyview is a giant mess here, I guess we should split the vlv case
from the pch_split cpu edp case. Maybe just check for port A in relevant
cases and factor out vlv cases completely.

My bet is that this massive confusion is the reason that vlv dp (i.e.
non-edp) is totally broken atm.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v2 5/7] drm/i915: stop using is_pch_edp() in is_cpu_edp()

[Imre Deak]

On Fri, 2013-05-10 at 10:49 +0200, Daniel Vetter wrote:
> On Wed, May 08, 2013 at 01:14:06PM +0300, Imre Deak wrote:
> > is_pch_edp() will be removed by the next patch, so replace it by a check
> > for the port and device type.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c |   17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 42c11dd..38b9cd2 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -65,6 +65,13 @@ static bool is_pch_edp(struct intel_dp *intel_dp)
> >  	return intel_dp->is_pch_edp;
> >  }
> >  
> > +static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
> > +{
> > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > +
> > +	return intel_dig_port->base.base.dev;
> > +}
> > +
> >  /**
> >   * is_cpu_edp - is the port on the CPU and attached to an eDP panel?
> >   * @intel_dp: DP struct
> > @@ -73,14 +80,12 @@ static bool is_pch_edp(struct intel_dp *intel_dp)
> >   */
> >  static bool is_cpu_edp(struct intel_dp *intel_dp)
> >  {
> > -	return is_edp(intel_dp) && !is_pch_edp(intel_dp);
> > -}
> > -
> > -static struct drm_device *intel_dp_to_dev(struct intel_dp *intel_dp)
> > -{
> > +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > +	enum port port = intel_dig_port->port;
> >  
> > -	return intel_dig_port->base.base.dev;
> > +	return is_edp(intel_dp) &&
> > +		(port == PORT_A || (port == PORT_C && IS_VALLEYVIEW(dev)));
> 
> Valleyview is a giant mess here, I guess we should split the vlv case
> from the pch_split cpu edp case. Maybe just check for port A in relevant
> cases and factor out vlv cases completely.
> 
> My bet is that this massive confusion is the reason that vlv dp (i.e.
> non-edp) is totally broken atm.

Could we do that as a follow-up to this? That work would have also the
benefit of removing is_cpu_edp() as you hoped earlier, so it could stand
on its own..

--Imre