All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Only ignore eDP ports that are connected
@ 2016-06-01  7:27 Chris Wilson
  2016-06-01  8:08 ` ✗ Ro.CI.BAT: warning for " Patchwork
  2016-06-01  8:20 ` [PATCH] " Ville Syrjälä
  0 siblings, 2 replies; 5+ messages in thread
From: Chris Wilson @ 2016-06-01  7:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter

If the VBT says that a certain port should be eDP (and hence fused off
from HDMI), but in reality it isn't, we need to try and acquire the HDMI
connection instead. So only trust the VBT edp setting if we can connect
to an eDP device on that port.

Fixes: d2182a6608 (drm/i915: Don't register HDMI connectors for eDP ports on VLV/CHV)
References: https://bugs.freedesktop.org/show_bug.cgi?id=96288
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Phidias Chiang <phidias.chiang@canonical.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++----------
 drivers/gpu/drm/i915/intel_dp.c      | 13 ++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 82796d1a87d7..0be8cede60e3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14642,6 +14642,8 @@ static void intel_setup_outputs(struct drm_device *dev)
 		if (I915_READ(PCH_DP_D) & DP_DETECTED)
 			intel_dp_init(dev, PCH_DP_D, PORT_D);
 	} else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
+		bool has_edp;
+
 		/*
 		 * The DP_DETECTED bit is the latched state of the DDC
 		 * SDA pin at boot. However since eDP doesn't require DDC
@@ -14651,19 +14653,17 @@ static void intel_setup_outputs(struct drm_device *dev)
 		 * eDP ports. Consult the VBT as well as DP_DETECTED to
 		 * detect eDP ports.
 		 */
-		if (I915_READ(VLV_HDMIB) & SDVO_DETECTED &&
-		    !intel_dp_is_edp(dev, PORT_B))
+		has_edp = intel_dp_is_edp(dev, PORT_B);
+		if (I915_READ(VLV_DP_B) & DP_DETECTED || has_edp)
+			has_edp &= intel_dp_init(dev, VLV_DP_B, PORT_B);
+		if (I915_READ(VLV_HDMIB) & SDVO_DETECTED && !has_edp)
 			intel_hdmi_init(dev, VLV_HDMIB, PORT_B);
-		if (I915_READ(VLV_DP_B) & DP_DETECTED ||
-		    intel_dp_is_edp(dev, PORT_B))
-			intel_dp_init(dev, VLV_DP_B, PORT_B);
 
-		if (I915_READ(VLV_HDMIC) & SDVO_DETECTED &&
-		    !intel_dp_is_edp(dev, PORT_C))
+		has_edp = intel_dp_is_edp(dev, PORT_C);
+		if (I915_READ(VLV_DP_C) & DP_DETECTED || has_edp)
+			has_edp &= intel_dp_init(dev, VLV_DP_C, PORT_C);
+		if (I915_READ(VLV_HDMIC) & SDVO_DETECTED && !has_edp)
 			intel_hdmi_init(dev, VLV_HDMIC, PORT_C);
-		if (I915_READ(VLV_DP_C) & DP_DETECTED ||
-		    intel_dp_is_edp(dev, PORT_C))
-			intel_dp_init(dev, VLV_DP_C, PORT_C);
 
 		if (IS_CHERRYVIEW(dev)) {
 			/* eDP not supported on port D, so don't check VBT */
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1d3824721c0a..d043b1384345 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5565,9 +5565,9 @@ fail:
 	return false;
 }
 
-void
-intel_dp_init(struct drm_device *dev,
-	      i915_reg_t output_reg, enum port port)
+bool intel_dp_init(struct drm_device *dev,
+		   i915_reg_t output_reg,
+		   enum port port)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_digital_port *intel_dig_port;
@@ -5577,7 +5577,7 @@ intel_dp_init(struct drm_device *dev,
 
 	intel_dig_port = kzalloc(sizeof(*intel_dig_port), GFP_KERNEL);
 	if (!intel_dig_port)
-		return;
+		return false;
 
 	intel_connector = intel_connector_alloc();
 	if (!intel_connector)
@@ -5634,7 +5634,7 @@ intel_dp_init(struct drm_device *dev,
 	if (!intel_dp_init_connector(intel_dig_port, intel_connector))
 		goto err_init_connector;
 
-	return;
+	return true;
 
 err_init_connector:
 	drm_encoder_cleanup(encoder);
@@ -5642,8 +5642,7 @@ err_encoder_init:
 	kfree(intel_connector);
 err_connector_alloc:
 	kfree(intel_dig_port);
-
-	return;
+	return false;
 }
 
 void intel_dp_mst_suspend(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bef4ee90ca67..cb9dc596fb7e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1328,7 +1328,7 @@ void intel_csr_ucode_suspend(struct drm_i915_private *);
 void intel_csr_ucode_resume(struct drm_i915_private *);
 
 /* intel_dp.c */
-void intel_dp_init(struct drm_device *dev, i915_reg_t output_reg, enum port port);
+bool intel_dp_init(struct drm_device *dev, i915_reg_t output_reg, enum port port);
 bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 			     struct intel_connector *intel_connector);
 void intel_dp_set_link_params(struct intel_dp *intel_dp,
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Ro.CI.BAT: warning for drm/i915: Only ignore eDP ports that are connected
  2016-06-01  7:27 [PATCH] drm/i915: Only ignore eDP ports that are connected Chris Wilson
@ 2016-06-01  8:08 ` Patchwork
  2016-06-01  8:20 ` [PATCH] " Ville Syrjälä
  1 sibling, 0 replies; 5+ messages in thread
From: Patchwork @ 2016-06-01  8:08 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Only ignore eDP ports that are connected
URL   : https://patchwork.freedesktop.org/series/8080/
State : warning

== Summary ==

Series 8080v1 drm/i915: Only ignore eDP ports that are connected
http://patchwork.freedesktop.org/api/1.0/series/8080/revisions/1/mbox

Test gem_exec_flush:
        Subgroup basic-uc-pro-default:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-uc-set-default:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup basic-wb-pro-default:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test gem_mmap_gtt:
        Subgroup basic-copy:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup basic-write:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup basic-write-gtt-no-prefault:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test gem_tiled_pread_basic:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test kms_addfb_basic:
        Subgroup addfb25-y-tiled-small:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup bad-pitch-32:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup bad-pitch-63:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup bad-pitch-65536:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup size-max:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup small-bo:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test kms_psr_sink_crc:
        Subgroup psr_basic:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)

fi-bdw-i7-5557u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8  
fi-hsw-i7-4770k  total:102  pass:86   dwarn:0   dfail:0   fail:0   skip:15 
fi-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-i5-6260u  total:209  pass:198  dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8  
ro-bdw-i7-5600u  total:102  pass:75   dwarn:0   dfail:0   fail:0   skip:26 
ro-bsw-n3050     total:89   pass:67   dwarn:0   dfail:0   fail:2   skip:19 
ro-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:102  pass:82   dwarn:0   dfail:0   fail:0   skip:19 
ro-ilk-i7-620lm  total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:102  pass:75   dwarn:0   dfail:0   fail:0   skip:26 
ro-ivb2-i7-3770  total:102  pass:45   dwarn:34  dfail:0   fail:0   skip:22 
ro-skl-i7-6700hq total:204  pass:173  dwarn:10  dfail:0   fail:0   skip:21 
ro-snb-i7-2620M  total:102  pass:72   dwarn:0   dfail:0   fail:0   skip:29 
fi-bsw-n3050 failed to connect after reboot
fi-byt-n2820 failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1073/

d8eea9f drm-intel-nightly: 2016y-05m-31d-23h-16m-57s UTC integration manifest
8297f0f drm/i915: Only ignore eDP ports that are connected

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: Only ignore eDP ports that are connected
  2016-06-01  7:27 [PATCH] drm/i915: Only ignore eDP ports that are connected Chris Wilson
  2016-06-01  8:08 ` ✗ Ro.CI.BAT: warning for " Patchwork
@ 2016-06-01  8:20 ` Ville Syrjälä
  2016-06-01  8:44   ` Chris Wilson
  2016-06-01  8:50   ` Chris Wilson
  1 sibling, 2 replies; 5+ messages in thread
From: Ville Syrjälä @ 2016-06-01  8:20 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Wed, Jun 01, 2016 at 08:27:50AM +0100, Chris Wilson wrote:
> If the VBT says that a certain port should be eDP (and hence fused off
> from HDMI), but in reality it isn't, we need to try and acquire the HDMI
> connection instead. So only trust the VBT edp setting if we can connect
> to an eDP device on that port.
> 
> Fixes: d2182a6608 (drm/i915: Don't register HDMI connectors for eDP ports on VLV/CHV)
> References: https://bugs.freedesktop.org/show_bug.cgi?id=96288
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Tested-by: Phidias Chiang <phidias.chiang@canonical.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++----------
>  drivers/gpu/drm/i915/intel_dp.c      | 13 ++++++-------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +-
>  3 files changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 82796d1a87d7..0be8cede60e3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14642,6 +14642,8 @@ static void intel_setup_outputs(struct drm_device *dev)
>  		if (I915_READ(PCH_DP_D) & DP_DETECTED)
>  			intel_dp_init(dev, PCH_DP_D, PORT_D);
>  	} else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> +		bool has_edp;
> +
>  		/*
>  		 * The DP_DETECTED bit is the latched state of the DDC
>  		 * SDA pin at boot. However since eDP doesn't require DDC
> @@ -14651,19 +14653,17 @@ static void intel_setup_outputs(struct drm_device *dev)
>  		 * eDP ports. Consult the VBT as well as DP_DETECTED to
>  		 * detect eDP ports.
>  		 */
> -		if (I915_READ(VLV_HDMIB) & SDVO_DETECTED &&
> -		    !intel_dp_is_edp(dev, PORT_B))
> +		has_edp = intel_dp_is_edp(dev, PORT_B);
> +		if (I915_READ(VLV_DP_B) & DP_DETECTED || has_edp)
> +			has_edp &= intel_dp_init(dev, VLV_DP_B, PORT_B);
> +		if (I915_READ(VLV_HDMIB) & SDVO_DETECTED && !has_edp)
>  			intel_hdmi_init(dev, VLV_HDMIB, PORT_B);
> -		if (I915_READ(VLV_DP_B) & DP_DETECTED ||
> -		    intel_dp_is_edp(dev, PORT_B))
> -			intel_dp_init(dev, VLV_DP_B, PORT_B);

This will change the order in which the connectors/encoders are 
registered. Shouldn't really matter. We use the DP,HDMI order on
some other platforms as well. I wonder if we should change everything
to use that order for consistency?

Also if the eDP init fails we'll not register a regular DP port, but
that's sort of OK since our eDP init doesn't actually check if the sink
is really eDP. Instead it just checks that DPCD can be read. So if we
would have a board with a normal user accesible DP connector which the
VBT claims to be eDP, we'd run into some hilarity when the user boots
w/o a display connected, yanks the cable after boot, or hotplugs in
another display. But I'm thinking we can just ignore that scenario
until we encounter such a board.

I'm happy enough with this as is, so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
> -		if (I915_READ(VLV_HDMIC) & SDVO_DETECTED &&
> -		    !intel_dp_is_edp(dev, PORT_C))
> +		has_edp = intel_dp_is_edp(dev, PORT_C);
> +		if (I915_READ(VLV_DP_C) & DP_DETECTED || has_edp)
> +			has_edp &= intel_dp_init(dev, VLV_DP_C, PORT_C);
> +		if (I915_READ(VLV_HDMIC) & SDVO_DETECTED && !has_edp)
>  			intel_hdmi_init(dev, VLV_HDMIC, PORT_C);
> -		if (I915_READ(VLV_DP_C) & DP_DETECTED ||
> -		    intel_dp_is_edp(dev, PORT_C))
> -			intel_dp_init(dev, VLV_DP_C, PORT_C);
>  
>  		if (IS_CHERRYVIEW(dev)) {
>  			/* eDP not supported on port D, so don't check VBT */
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1d3824721c0a..d043b1384345 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5565,9 +5565,9 @@ fail:
>  	return false;
>  }
>  
> -void
> -intel_dp_init(struct drm_device *dev,
> -	      i915_reg_t output_reg, enum port port)
> +bool intel_dp_init(struct drm_device *dev,
> +		   i915_reg_t output_reg,
> +		   enum port port)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_digital_port *intel_dig_port;
> @@ -5577,7 +5577,7 @@ intel_dp_init(struct drm_device *dev,
>  
>  	intel_dig_port = kzalloc(sizeof(*intel_dig_port), GFP_KERNEL);
>  	if (!intel_dig_port)
> -		return;
> +		return false;
>  
>  	intel_connector = intel_connector_alloc();
>  	if (!intel_connector)
> @@ -5634,7 +5634,7 @@ intel_dp_init(struct drm_device *dev,
>  	if (!intel_dp_init_connector(intel_dig_port, intel_connector))
>  		goto err_init_connector;
>  
> -	return;
> +	return true;
>  
>  err_init_connector:
>  	drm_encoder_cleanup(encoder);
> @@ -5642,8 +5642,7 @@ err_encoder_init:
>  	kfree(intel_connector);
>  err_connector_alloc:
>  	kfree(intel_dig_port);
> -
> -	return;
> +	return false;
>  }
>  
>  void intel_dp_mst_suspend(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bef4ee90ca67..cb9dc596fb7e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1328,7 +1328,7 @@ void intel_csr_ucode_suspend(struct drm_i915_private *);
>  void intel_csr_ucode_resume(struct drm_i915_private *);
>  
>  /* intel_dp.c */
> -void intel_dp_init(struct drm_device *dev, i915_reg_t output_reg, enum port port);
> +bool intel_dp_init(struct drm_device *dev, i915_reg_t output_reg, enum port port);
>  bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  			     struct intel_connector *intel_connector);
>  void intel_dp_set_link_params(struct intel_dp *intel_dp,
> -- 
> 2.8.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: Only ignore eDP ports that are connected
  2016-06-01  8:20 ` [PATCH] " Ville Syrjälä
@ 2016-06-01  8:44   ` Chris Wilson
  2016-06-01  8:50   ` Chris Wilson
  1 sibling, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2016-06-01  8:44 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Wed, Jun 01, 2016 at 11:20:04AM +0300, Ville Syrjälä wrote:
> On Wed, Jun 01, 2016 at 08:27:50AM +0100, Chris Wilson wrote:
> > If the VBT says that a certain port should be eDP (and hence fused off
> > from HDMI), but in reality it isn't, we need to try and acquire the HDMI
> > connection instead. So only trust the VBT edp setting if we can connect
> > to an eDP device on that port.
> > 
> > Fixes: d2182a6608 (drm/i915: Don't register HDMI connectors for eDP ports on VLV/CHV)
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=96288
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Tested-by: Phidias Chiang <phidias.chiang@canonical.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++----------
> >  drivers/gpu/drm/i915/intel_dp.c      | 13 ++++++-------
> >  drivers/gpu/drm/i915/intel_drv.h     |  2 +-
> >  3 files changed, 17 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 82796d1a87d7..0be8cede60e3 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14642,6 +14642,8 @@ static void intel_setup_outputs(struct drm_device *dev)
> >  		if (I915_READ(PCH_DP_D) & DP_DETECTED)
> >  			intel_dp_init(dev, PCH_DP_D, PORT_D);
> >  	} else if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > +		bool has_edp;
> > +
> >  		/*
> >  		 * The DP_DETECTED bit is the latched state of the DDC
> >  		 * SDA pin at boot. However since eDP doesn't require DDC
> > @@ -14651,19 +14653,17 @@ static void intel_setup_outputs(struct drm_device *dev)
> >  		 * eDP ports. Consult the VBT as well as DP_DETECTED to
> >  		 * detect eDP ports.
> >  		 */
> > -		if (I915_READ(VLV_HDMIB) & SDVO_DETECTED &&
> > -		    !intel_dp_is_edp(dev, PORT_B))
> > +		has_edp = intel_dp_is_edp(dev, PORT_B);
> > +		if (I915_READ(VLV_DP_B) & DP_DETECTED || has_edp)
> > +			has_edp &= intel_dp_init(dev, VLV_DP_B, PORT_B);
> > +		if (I915_READ(VLV_HDMIB) & SDVO_DETECTED && !has_edp)
> >  			intel_hdmi_init(dev, VLV_HDMIB, PORT_B);
> > -		if (I915_READ(VLV_DP_B) & DP_DETECTED ||
> > -		    intel_dp_is_edp(dev, PORT_B))
> > -			intel_dp_init(dev, VLV_DP_B, PORT_B);
> 
> This will change the order in which the connectors/encoders are 
> registered. Shouldn't really matter. We use the DP,HDMI order on
> some other platforms as well. I wonder if we should change everything
> to use that order for consistency?

I'm tempted to, just to solve the same problem of false eDP disabling
valid outputs on other platforms, e.g. ivb.  However, changing the order
feels like a "it can't possibly break anything, but does!" situation, so
gradual is best.  Certainly, on the older sdvo chipsets, the order is
more constrained, but on the ddi platforms where hdmi/dp are
interchangeable, it should be flexible.

> Also if the eDP init fails we'll not register a regular DP port, but
> that's sort of OK since our eDP init doesn't actually check if the sink
> is really eDP. Instead it just checks that DPCD can be read. So if we
> would have a board with a normal user accesible DP connector which the
> VBT claims to be eDP, we'd run into some hilarity when the user boots
> w/o a display connected, yanks the cable after boot, or hotplugs in
> another display. But I'm thinking we can just ignore that scenario
> until we encounter such a board.

Right. That we could probably resolve just by dropping the dp->is_edp
flag if intel_edp_init_connector fails and just leaving the connector
established for DP - but only if it is reported as detected (rather than
the VBT override). Then we would change the HDMI setup to only ignore
known eDP links (for which we would need a stronger is-eDP
verification). The challenge is "who would this regress"?

The older bug is https://bugs.freedesktop.org/show_bug.cgi?id=88331 if
anyone feels inspired on handling HDMI vs eDP vs DP more consistently
between platforms (if that is even possible!).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: Only ignore eDP ports that are connected
  2016-06-01  8:20 ` [PATCH] " Ville Syrjälä
  2016-06-01  8:44   ` Chris Wilson
@ 2016-06-01  8:50   ` Chris Wilson
  1 sibling, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2016-06-01  8:50 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Jani Nikula, Daniel Vetter, intel-gfx

On Wed, Jun 01, 2016 at 11:20:04AM +0300, Ville Syrjälä wrote:
> On Wed, Jun 01, 2016 at 08:27:50AM +0100, Chris Wilson wrote:
> > If the VBT says that a certain port should be eDP (and hence fused off
> > from HDMI), but in reality it isn't, we need to try and acquire the HDMI
> > connection instead. So only trust the VBT edp setting if we can connect
> > to an eDP device on that port.
> > 
> > Fixes: d2182a6608 (drm/i915: Don't register HDMI connectors for eDP ports on VLV/CHV)
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=96288
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Tested-by: Phidias Chiang <phidias.chiang@canonical.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

[snip]

> I'm happy enough with this as is, so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

The only way to get enough test coverage to see if this regresses is to
push and wait 12-24 months. So here goes...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2016-06-01  8:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-01  7:27 [PATCH] drm/i915: Only ignore eDP ports that are connected Chris Wilson
2016-06-01  8:08 ` ✗ Ro.CI.BAT: warning for " Patchwork
2016-06-01  8:20 ` [PATCH] " Ville Syrjälä
2016-06-01  8:44   ` Chris Wilson
2016-06-01  8:50   ` Chris Wilson

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.