[PATCH 0/3] HDMI optimization series

[PATCH 0/3] HDMI optimization series

[Sonika Jindal]

This series adds some optimization related to reading of edid only when
required and when live status says so.
The comments in the patches explain more.

v2:
   Some refactoring is with this series.

   Also, right now this is done for platforms gen7 and above because we
   couldn't test with older platforms. For newer platforms it works
   reliably.

   For HPD and live status to work on SKL, following patch is required:
   "drm/i915: Handle HPD when it has actually occurred"

Shashank Sharma (2):
  drm/i915: add attached connector to hdmi container
  drm/i915: Add HDMI probe function

Sonika Jindal (1):
  drm/i915: Check live status before reading edid

 drivers/gpu/drm/i915/intel_dp.c   |    4 +-
 drivers/gpu/drm/i915/intel_drv.h  |    3 ++
 drivers/gpu/drm/i915/intel_hdmi.c |   93 +++++++++++++++++++++++++++++++++----
 3 files changed, 88 insertions(+), 12 deletions(-)

-- 
1.7.10.4

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

[PATCH 1/3] drm/i915: add attached connector to hdmi container

[Sonika Jindal]

From: Shashank Sharma <shashank.sharma@intel.com>

This patch adds the intel_connector initialized to intel_hdmi
display, during the init phase, just like the other encoders do.
This attachment is very useful when we need to extract the connector
pointer during the hotplug handler function

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  |    1 +
 drivers/gpu/drm/i915/intel_hdmi.c |    1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e90c743..a47fbc3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -659,6 +659,7 @@ struct intel_hdmi {
 	enum hdmi_force_audio force_audio;
 	bool rgb_quant_range_selectable;
 	enum hdmi_picture_aspect aspect_ratio;
+	struct intel_connector *attached_connector;
 	void (*write_infoframe)(struct drm_encoder *encoder,
 				enum hdmi_infoframe_type type,
 				const void *frame, ssize_t len);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 00c4b40..af4d1e6 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2000,6 +2000,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
 	drm_connector_register(connector);
+	intel_hdmi->attached_connector = intel_connector;
 
 	/* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be written
 	 * 0xd.  Failure to do so will result in spurious interrupts being
-- 
1.7.10.4

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

[PATCH 2/3] drm/i915: Add HDMI probe function

[Sonika Jindal]

From: Shashank Sharma <shashank.sharma@intel.com>

This patch adds a separate probe function for HDMI
EDID read over DDC channel. This function has been
registered as a .hot_plug handler for HDMI encoder.

The current implementation of hdmi_detect()
function re-sets the cached HDMI edid (in connector->detect_edid) in
every detect call.This function gets called many times, sometimes
directly from userspace probes, forcing drivers to read EDID every
detect function call.This causes several problems like:

1. Race conditions in multiple hot_plug / unplug cases, between
   interrupts bottom halves and userspace detections.
2. Many Un-necessary EDID reads for single hotplug/unplug
3. HDMI complaince failures which expects only one EDID read per hotplug

This function will be serving the purpose of really reading the EDID
by really probing the DDC channel, and updating the cached EDID.

The plan is to:
1. i915 IRQ handler bottom half function already calls
   intel_encoder->hotplug() function. Adding This probe function which
   will read the EDID only in case of a hotplug / unplug.
2. During init_connector this probe will be called to read the edid
3. Reuse the cached EDID in hdmi_detect() function.

The "< gen7" check is there because this was tested only for >=gen7
platforms. For older platforms the hotplug/reading edid path remains same.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c |   49 ++++++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index af4d1e6..c4b82ce 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1340,23 +1340,56 @@ intel_hdmi_set_edid(struct drm_connector *connector)
 	return connected;
 }
 
+void intel_hdmi_probe(struct intel_encoder *intel_encoder)
+{
+	struct intel_hdmi *intel_hdmi =
+			enc_to_intel_hdmi(&intel_encoder->base);
+	struct intel_connector *intel_connector =
+				intel_hdmi->attached_connector;
+	/*
+	 * We are here, means there is a hotplug or a force
+	 * detection. Clear the cached EDID and probe the
+	 * DDC bus to check the current status of HDMI.
+	 */
+	intel_hdmi_unset_edid(&intel_connector->base);
+	if (intel_hdmi_set_edid(&intel_connector->base))
+		DRM_DEBUG_DRIVER("DDC probe: got EDID\n");
+	else
+		DRM_DEBUG_DRIVER("DDC probe: no EDID\n");
+}
+
 static enum drm_connector_status
 intel_hdmi_detect(struct drm_connector *connector, bool force)
 {
 	enum drm_connector_status status;
+	struct intel_connector *intel_connector =
+				to_intel_connector(connector);
+	struct drm_device *dev = connector->dev;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
+	/*
+	 * There are many userspace calls which probe EDID from
+	 * detect path. In case of multiple hotplug/unplug, these
+	 * can cause race conditions while probing EDID. Also its
+	 * waste of CPU cycles to read the EDID again and again
+	 * unless there is a real hotplug.
+	 * So, reading edid in detect only for older platforms (< gen7)
+	 * Letting the newer platforms rely on hotplugs and init to read edid.
+	 * Check connector status based on availability of cached EDID.
+	 */
+	if (INTEL_INFO(dev)->gen < 7)
+		intel_hdmi_probe(intel_connector->encoder);
 
-	intel_hdmi_unset_edid(connector);
-
-	if (intel_hdmi_set_edid(connector)) {
+	if (intel_connector->detect_edid) {
 		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
-
-		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
 		status = connector_status_connected;
-	} else
+		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
+		DRM_DEBUG_DRIVER("hdmi status = connected\n");
+	} else {
 		status = connector_status_disconnected;
+		DRM_DEBUG_DRIVER("hdmi status = disconnected\n");
+	}
 
 	return status;
 }
@@ -1997,11 +2030,15 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	intel_connector->unregister = intel_connector_unregister;
 
 	intel_hdmi_add_properties(intel_hdmi, connector);
+	intel_encoder->hot_plug = intel_hdmi_probe;
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
 	drm_connector_register(connector);
 	intel_hdmi->attached_connector = intel_connector;
 
+	/* Set edid during init */
+	intel_hdmi_probe(intel_encoder);
+
 	/* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be written
 	 * 0xd.  Failure to do so will result in spurious interrupts being
 	 * generated on the port when a cable is not attached.
-- 
1.7.10.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Check live status before reading edid

[Sonika Jindal]

Adding this for SKL onwards.

v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
to check digital port status. Adding a separate function to get bxt live
status (Daniel)

Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c   |    4 ++--
 drivers/gpu/drm/i915/intel_drv.h  |    2 ++
 drivers/gpu/drm/i915/intel_hdmi.c |   43 +++++++++++++++++++++++++++++++++----
 3 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4ebfc3a..7b54b9d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
 	return intel_dp_detect_dpcd(intel_dp);
 }
 
-static int g4x_digital_port_connected(struct drm_device *dev,
-				       struct intel_digital_port *intel_dig_port)
+int g4x_digital_port_connected(struct drm_device *dev,
+			       struct intel_digital_port *intel_dig_port)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t bit;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a47fbc3..30c8dd6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1187,6 +1187,8 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
 void intel_edp_drrs_invalidate(struct drm_device *dev,
 		unsigned frontbuffer_bits);
 void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
+int g4x_digital_port_connected(struct drm_device *dev,
+			       struct intel_digital_port *intel_dig_port);
 
 /* intel_dp_mst.c */
 int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index c4b82ce..402be54 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1300,6 +1300,40 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
 	to_intel_connector(connector)->detect_edid = NULL;
 }
 
+static bool bxt_port_connected(struct drm_i915_private *dev_priv,
+			       struct intel_digital_port *port)
+{
+	u32 temp = I915_READ(GEN8_DE_PORT_ISR);
+
+	/* TODO: Add bxt A0/A1 wa related to hpd pin swap */
+	switch (port->port) {
+	case PORT_B:
+		return temp & BXT_DE_PORT_HP_DDIB;
+
+	case PORT_C:
+		return temp & BXT_DE_PORT_HP_DDIC;
+
+	default:
+		return false;
+
+	}
+}
+
+static bool intel_hdmi_live_status(struct intel_digital_port *intel_dig_port)
+{
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	if (IS_VALLEYVIEW(dev))
+		return g4x_digital_port_connected(dev, intel_dig_port);
+	else if (IS_SKYLAKE(dev))
+		return ibx_digital_port_connected(dev_priv, intel_dig_port);
+	else if (IS_BROXTON(dev))
+		return bxt_port_connected(dev_priv, intel_dig_port);
+
+	return true;
+}
+
 static bool
 intel_hdmi_set_edid(struct drm_connector *connector)
 {
@@ -1308,15 +1342,16 @@ intel_hdmi_set_edid(struct drm_connector *connector)
 	struct intel_encoder *intel_encoder =
 		&hdmi_to_dig_port(intel_hdmi)->base;
 	enum intel_display_power_domain power_domain;
-	struct edid *edid;
+	struct edid *edid = NULL;
 	bool connected = false;
 
 	power_domain = intel_display_port_power_domain(intel_encoder);
 	intel_display_power_get(dev_priv, power_domain);
 
-	edid = drm_get_edid(connector,
-			    intel_gmbus_get_adapter(dev_priv,
-						    intel_hdmi->ddc_bus));
+	if (intel_hdmi_live_status(hdmi_to_dig_port(intel_hdmi)))
+		edid = drm_get_edid(connector,
+				intel_gmbus_get_adapter(dev_priv,
+					intel_hdmi->ddc_bus));
 
 	intel_display_power_put(dev_priv, power_domain);
 
-- 
1.7.10.4

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

Re: [PATCH 3/3] drm/i915: Check live status before reading edid

[Imre Deak]

On ti, 2015-07-14 at 17:21 +0530, Sonika Jindal wrote:
> Adding this for SKL onwards.
> 
> v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
> to check digital port status. Adding a separate function to get bxt live
> status (Daniel)
> 
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c   |    4 ++--
>  drivers/gpu/drm/i915/intel_drv.h  |    2 ++
>  drivers/gpu/drm/i915/intel_hdmi.c |   43 +++++++++++++++++++++++++++++++++----
>  3 files changed, 43 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4ebfc3a..7b54b9d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
>  	return intel_dp_detect_dpcd(intel_dp);
>  }
>  
> -static int g4x_digital_port_connected(struct drm_device *dev,
> -				       struct intel_digital_port *intel_dig_port)
> +int g4x_digital_port_connected(struct drm_device *dev,
> +			       struct intel_digital_port *intel_dig_port)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t bit;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a47fbc3..30c8dd6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1187,6 +1187,8 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
>  void intel_edp_drrs_invalidate(struct drm_device *dev,
>  		unsigned frontbuffer_bits);
>  void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
> +int g4x_digital_port_connected(struct drm_device *dev,
> +			       struct intel_digital_port *intel_dig_port);
>  
>  /* intel_dp_mst.c */
>  int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index c4b82ce..402be54 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1300,6 +1300,40 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
>  	to_intel_connector(connector)->detect_edid = NULL;
>  }
>  
> +static bool bxt_port_connected(struct drm_i915_private *dev_priv,
> +			       struct intel_digital_port *port)
> +{
> +	u32 temp = I915_READ(GEN8_DE_PORT_ISR);
> +
> +	/* TODO: Add bxt A0/A1 wa related to hpd pin swap */

This doesn't seem to work on my BXT A1 RVP. For me GEN8_DE_PORT_ISR
reads all 0 even while an HDMI monitor is plugged to port B.

> +	switch (port->port) {
> +	case PORT_B:
> +		return temp & BXT_DE_PORT_HP_DDIB;
> +
> +	case PORT_C:
> +		return temp & BXT_DE_PORT_HP_DDIC;
> +
> +	default:
> +		return false;
> +
> +	}
> +}
> +
> +static bool intel_hdmi_live_status(struct intel_digital_port *intel_dig_port)
> +{
> +	struct drm_device *dev = intel_dig_port->base.base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +
> +	if (IS_VALLEYVIEW(dev))
> +		return g4x_digital_port_connected(dev, intel_dig_port);
> +	else if (IS_SKYLAKE(dev))
> +		return ibx_digital_port_connected(dev_priv, intel_dig_port);
> +	else if (IS_BROXTON(dev))
> +		return bxt_port_connected(dev_priv, intel_dig_port);
> +
> +	return true;
> +}
> +
>  static bool
>  intel_hdmi_set_edid(struct drm_connector *connector)
>  {
> @@ -1308,15 +1342,16 @@ intel_hdmi_set_edid(struct drm_connector *connector)
>  	struct intel_encoder *intel_encoder =
>  		&hdmi_to_dig_port(intel_hdmi)->base;
>  	enum intel_display_power_domain power_domain;
> -	struct edid *edid;
> +	struct edid *edid = NULL;
>  	bool connected = false;
>  
>  	power_domain = intel_display_port_power_domain(intel_encoder);
>  	intel_display_power_get(dev_priv, power_domain);
>  
> -	edid = drm_get_edid(connector,
> -			    intel_gmbus_get_adapter(dev_priv,
> -						    intel_hdmi->ddc_bus));
> +	if (intel_hdmi_live_status(hdmi_to_dig_port(intel_hdmi)))
> +		edid = drm_get_edid(connector,
> +				intel_gmbus_get_adapter(dev_priv,
> +					intel_hdmi->ddc_bus));
>  
>  	intel_display_power_put(dev_priv, power_domain);
>  


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

Re: [PATCH 3/3] drm/i915: Check live status before reading edid

[Jindal, Sonika]

On 7/14/2015 7:59 PM, Imre Deak wrote:
> On ti, 2015-07-14 at 17:21 +0530, Sonika Jindal wrote:
>> Adding this for SKL onwards.
>>
>> v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
>> to check digital port status. Adding a separate function to get bxt live
>> status (Daniel)
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c   |    4 ++--
>>   drivers/gpu/drm/i915/intel_drv.h  |    2 ++
>>   drivers/gpu/drm/i915/intel_hdmi.c |   43 +++++++++++++++++++++++++++++++++----
>>   3 files changed, 43 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 4ebfc3a..7b54b9d 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
>>   	return intel_dp_detect_dpcd(intel_dp);
>>   }
>>
>> -static int g4x_digital_port_connected(struct drm_device *dev,
>> -				       struct intel_digital_port *intel_dig_port)
>> +int g4x_digital_port_connected(struct drm_device *dev,
>> +			       struct intel_digital_port *intel_dig_port)
>>   {
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>   	uint32_t bit;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index a47fbc3..30c8dd6 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1187,6 +1187,8 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
>>   void intel_edp_drrs_invalidate(struct drm_device *dev,
>>   		unsigned frontbuffer_bits);
>>   void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
>> +int g4x_digital_port_connected(struct drm_device *dev,
>> +			       struct intel_digital_port *intel_dig_port);
>>
>>   /* intel_dp_mst.c */
>>   int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index c4b82ce..402be54 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1300,6 +1300,40 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
>>   	to_intel_connector(connector)->detect_edid = NULL;
>>   }
>>
>> +static bool bxt_port_connected(struct drm_i915_private *dev_priv,
>> +			       struct intel_digital_port *port)
>> +{
>> +	u32 temp = I915_READ(GEN8_DE_PORT_ISR);
>> +
>> +	/* TODO: Add bxt A0/A1 wa related to hpd pin swap */
>
> This doesn't seem to work on my BXT A1 RVP. For me GEN8_DE_PORT_ISR
> reads all 0 even while an HDMI monitor is plugged to port B.
>
This is really strange, I checked on atleast 3 different A1 boards and 3 
different monitors (also on two different branches) and I see 0x44440 as 
0x8 after the HPD swap patch when monitor is connected and it is 0 when 
no hdmi monitor is present.
Do you think something to do with the board config?

>> +	switch (port->port) {
>> +	case PORT_B:
>> +		return temp & BXT_DE_PORT_HP_DDIB;
>> +
>> +	case PORT_C:
>> +		return temp & BXT_DE_PORT_HP_DDIC;
>> +
>> +	default:
>> +		return false;
>> +
>> +	}
>> +}
>> +
>> +static bool intel_hdmi_live_status(struct intel_digital_port *intel_dig_port)
>> +{
>> +	struct drm_device *dev = intel_dig_port->base.base.dev;
>> +	struct drm_i915_private *dev_priv = to_i915(dev);
>> +
>> +	if (IS_VALLEYVIEW(dev))
>> +		return g4x_digital_port_connected(dev, intel_dig_port);
>> +	else if (IS_SKYLAKE(dev))
>> +		return ibx_digital_port_connected(dev_priv, intel_dig_port);
>> +	else if (IS_BROXTON(dev))
>> +		return bxt_port_connected(dev_priv, intel_dig_port);
>> +
>> +	return true;
>> +}
>> +
>>   static bool
>>   intel_hdmi_set_edid(struct drm_connector *connector)
>>   {
>> @@ -1308,15 +1342,16 @@ intel_hdmi_set_edid(struct drm_connector *connector)
>>   	struct intel_encoder *intel_encoder =
>>   		&hdmi_to_dig_port(intel_hdmi)->base;
>>   	enum intel_display_power_domain power_domain;
>> -	struct edid *edid;
>> +	struct edid *edid = NULL;
>>   	bool connected = false;
>>
>>   	power_domain = intel_display_port_power_domain(intel_encoder);
>>   	intel_display_power_get(dev_priv, power_domain);
>>
>> -	edid = drm_get_edid(connector,
>> -			    intel_gmbus_get_adapter(dev_priv,
>> -						    intel_hdmi->ddc_bus));
>> +	if (intel_hdmi_live_status(hdmi_to_dig_port(intel_hdmi)))
>> +		edid = drm_get_edid(connector,
>> +				intel_gmbus_get_adapter(dev_priv,
>> +					intel_hdmi->ddc_bus));
>>
>>   	intel_display_power_put(dev_priv, power_domain);
>>
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Check live status before reading edid

[Imre Deak]

On Wed, 2015-07-15 at 09:25 +0530, Jindal, Sonika wrote:
> 
> On 7/14/2015 7:59 PM, Imre Deak wrote:
> > On ti, 2015-07-14 at 17:21 +0530, Sonika Jindal wrote:
> >> Adding this for SKL onwards.
> >>
> >> v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
> >> to check digital port status. Adding a separate function to get bxt live
> >> status (Daniel)
> >>
> >> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_dp.c   |    4 ++--
> >>   drivers/gpu/drm/i915/intel_drv.h  |    2 ++
> >>   drivers/gpu/drm/i915/intel_hdmi.c |   43 +++++++++++++++++++++++++++++++++----
> >>   3 files changed, 43 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 4ebfc3a..7b54b9d 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
> >>   	return intel_dp_detect_dpcd(intel_dp);
> >>   }
> >>
> >> -static int g4x_digital_port_connected(struct drm_device *dev,
> >> -				       struct intel_digital_port *intel_dig_port)
> >> +int g4x_digital_port_connected(struct drm_device *dev,
> >> +			       struct intel_digital_port *intel_dig_port)
> >>   {
> >>   	struct drm_i915_private *dev_priv = dev->dev_private;
> >>   	uint32_t bit;
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >> index a47fbc3..30c8dd6 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -1187,6 +1187,8 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
> >>   void intel_edp_drrs_invalidate(struct drm_device *dev,
> >>   		unsigned frontbuffer_bits);
> >>   void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
> >> +int g4x_digital_port_connected(struct drm_device *dev,
> >> +			       struct intel_digital_port *intel_dig_port);
> >>
> >>   /* intel_dp_mst.c */
> >>   int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
> >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >> index c4b82ce..402be54 100644
> >> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >> @@ -1300,6 +1300,40 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
> >>   	to_intel_connector(connector)->detect_edid = NULL;
> >>   }
> >>
> >> +static bool bxt_port_connected(struct drm_i915_private *dev_priv,
> >> +			       struct intel_digital_port *port)
> >> +{
> >> +	u32 temp = I915_READ(GEN8_DE_PORT_ISR);
> >> +
> >> +	/* TODO: Add bxt A0/A1 wa related to hpd pin swap */
> >
> > This doesn't seem to work on my BXT A1 RVP. For me GEN8_DE_PORT_ISR
> > reads all 0 even while an HDMI monitor is plugged to port B.
> >
> This is really strange, I checked on atleast 3 different A1 boards and 3 
> different monitors (also on two different branches) and I see 0x44440 as 
> 0x8 after the HPD swap patch when monitor is connected and it is 0 when 
> no hdmi monitor is present.
> Do you think something to do with the board config?

Upgrading my BIOS from version W25 to W29 has solved this issue,
GEN8_DE_PORT_ISR bit 3 is set now correctly whenever the monitor is
connected. My guess is that some GPIO/PIN configuration change done by
the BIOS has fixed this, although I haven't found the reason by looking
at the PAD_CFG register contents and BIOS release notes.


> >> +	switch (port->port) {
> >> +	case PORT_B:
> >> +		return temp & BXT_DE_PORT_HP_DDIB;
> >> +
> >> +	case PORT_C:
> >> +		return temp & BXT_DE_PORT_HP_DDIC;
> >> +
> >> +	default:
> >> +		return false;
> >> +
> >> +	}
> >> +}
> >> +
> >> +static bool intel_hdmi_live_status(struct intel_digital_port *intel_dig_port)
> >> +{
> >> +	struct drm_device *dev = intel_dig_port->base.base.dev;
> >> +	struct drm_i915_private *dev_priv = to_i915(dev);
> >> +
> >> +	if (IS_VALLEYVIEW(dev))
> >> +		return g4x_digital_port_connected(dev, intel_dig_port);
> >> +	else if (IS_SKYLAKE(dev))
> >> +		return ibx_digital_port_connected(dev_priv, intel_dig_port);
> >> +	else if (IS_BROXTON(dev))
> >> +		return bxt_port_connected(dev_priv, intel_dig_port);
> >> +
> >> +	return true;
> >> +}
> >> +
> >>   static bool
> >>   intel_hdmi_set_edid(struct drm_connector *connector)
> >>   {
> >> @@ -1308,15 +1342,16 @@ intel_hdmi_set_edid(struct drm_connector *connector)
> >>   	struct intel_encoder *intel_encoder =
> >>   		&hdmi_to_dig_port(intel_hdmi)->base;
> >>   	enum intel_display_power_domain power_domain;
> >> -	struct edid *edid;
> >> +	struct edid *edid = NULL;
> >>   	bool connected = false;
> >>
> >>   	power_domain = intel_display_port_power_domain(intel_encoder);
> >>   	intel_display_power_get(dev_priv, power_domain);
> >>
> >> -	edid = drm_get_edid(connector,
> >> -			    intel_gmbus_get_adapter(dev_priv,
> >> -						    intel_hdmi->ddc_bus));
> >> +	if (intel_hdmi_live_status(hdmi_to_dig_port(intel_hdmi)))
> >> +		edid = drm_get_edid(connector,
> >> +				intel_gmbus_get_adapter(dev_priv,
> >> +					intel_hdmi->ddc_bus));
> >>
> >>   	intel_display_power_put(dev_priv, power_domain);
> >>
> >
> >


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

Re: [PATCH 3/3] drm/i915: Check live status before reading edid

[Sivakumar Thulasimani]

On 7/14/2015 5:21 PM, Sonika Jindal wrote:
> Adding this for SKL onwards.
>
> v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
> to check digital port status. Adding a separate function to get bxt live
> status (Daniel)
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c   |    4 ++--
>   drivers/gpu/drm/i915/intel_drv.h  |    2 ++
>   drivers/gpu/drm/i915/intel_hdmi.c |   43 +++++++++++++++++++++++++++++++++----
>   3 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4ebfc3a..7b54b9d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
>   	return intel_dp_detect_dpcd(intel_dp);
>   }
>   
> -static int g4x_digital_port_connected(struct drm_device *dev,
> -				       struct intel_digital_port *intel_dig_port)
> +int g4x_digital_port_connected(struct drm_device *dev,
> +			       struct intel_digital_port *intel_dig_port)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	uint32_t bit;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a47fbc3..30c8dd6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1187,6 +1187,8 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
>   void intel_edp_drrs_invalidate(struct drm_device *dev,
>   		unsigned frontbuffer_bits);
>   void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
> +int g4x_digital_port_connected(struct drm_device *dev,
> +			       struct intel_digital_port *intel_dig_port);
>   
>   /* intel_dp_mst.c */
>   int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index c4b82ce..402be54 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1300,6 +1300,40 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
>   	to_intel_connector(connector)->detect_edid = NULL;
>   }
>   
> +static bool bxt_port_connected(struct drm_i915_private *dev_priv,
> +			       struct intel_digital_port *port)
> +{
> +	u32 temp = I915_READ(GEN8_DE_PORT_ISR);
> +
> +	/* TODO: Add bxt A0/A1 wa related to hpd pin swap */
> +	switch (port->port) {
why not pass the encoder and use its hpd_pin ? that will avoid the need 
to do
A0/A1 related checks ?
> +	case PORT_B:
> +		return temp & BXT_DE_PORT_HP_DDIB;
> +
> +	case PORT_C:
> +		return temp & BXT_DE_PORT_HP_DDIC;
> +
> +	default:
> +		return false;
> +
> +	}
> +}
> +
> +static bool intel_hdmi_live_status(struct intel_digital_port *intel_dig_port)
> +{
> +	struct drm_device *dev = intel_dig_port->base.base.dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +
> +	if (IS_VALLEYVIEW(dev))
> +		return g4x_digital_port_connected(dev, intel_dig_port);
> +	else if (IS_SKYLAKE(dev))
> +		return ibx_digital_port_connected(dev_priv, intel_dig_port);
> +	else if (IS_BROXTON(dev))
> +		return bxt_port_connected(dev_priv, intel_dig_port);
> +
> +	return true;
> +}
> +
>   static bool
>   intel_hdmi_set_edid(struct drm_connector *connector)
>   {
> @@ -1308,15 +1342,16 @@ intel_hdmi_set_edid(struct drm_connector *connector)
>   	struct intel_encoder *intel_encoder =
>   		&hdmi_to_dig_port(intel_hdmi)->base;
>   	enum intel_display_power_domain power_domain;
> -	struct edid *edid;
> +	struct edid *edid = NULL;
>   	bool connected = false;
>   
>   	power_domain = intel_display_port_power_domain(intel_encoder);
>   	intel_display_power_get(dev_priv, power_domain);
>   
> -	edid = drm_get_edid(connector,
> -			    intel_gmbus_get_adapter(dev_priv,
> -						    intel_hdmi->ddc_bus));
> +	if (intel_hdmi_live_status(hdmi_to_dig_port(intel_hdmi)))
> +		edid = drm_get_edid(connector,
> +				intel_gmbus_get_adapter(dev_priv,
> +					intel_hdmi->ddc_bus));
>   
>   	intel_display_power_put(dev_priv, power_domain);
>   

-- 
regards,
Sivakumar

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

Re: [PATCH 3/3] drm/i915: Check live status before reading edid

[Jindal, Sonika]

On 8/10/2015 11:14 AM, Sivakumar Thulasimani wrote:
>
>
> On 7/14/2015 5:21 PM, Sonika Jindal wrote:
>> Adding this for SKL onwards.
>>
>> v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
>> to check digital port status. Adding a separate function to get bxt live
>> status (Daniel)
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dp.c   |    4 ++--
>>   drivers/gpu/drm/i915/intel_drv.h  |    2 ++
>>   drivers/gpu/drm/i915/intel_hdmi.c |   43
>> +++++++++++++++++++++++++++++++++----
>>   3 files changed, 43 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index 4ebfc3a..7b54b9d 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
>>       return intel_dp_detect_dpcd(intel_dp);
>>   }
>> -static int g4x_digital_port_connected(struct drm_device *dev,
>> -                       struct intel_digital_port *intel_dig_port)
>> +int g4x_digital_port_connected(struct drm_device *dev,
>> +                   struct intel_digital_port *intel_dig_port)
>>   {
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>       uint32_t bit;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index a47fbc3..30c8dd6 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1187,6 +1187,8 @@ void intel_edp_drrs_disable(struct intel_dp
>> *intel_dp);
>>   void intel_edp_drrs_invalidate(struct drm_device *dev,
>>           unsigned frontbuffer_bits);
>>   void intel_edp_drrs_flush(struct drm_device *dev, unsigned
>> frontbuffer_bits);
>> +int g4x_digital_port_connected(struct drm_device *dev,
>> +                   struct intel_digital_port *intel_dig_port);
>>   /* intel_dp_mst.c */
>>   int intel_dp_mst_encoder_init(struct intel_digital_port
>> *intel_dig_port, int conn_id);
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
>> b/drivers/gpu/drm/i915/intel_hdmi.c
>> index c4b82ce..402be54 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1300,6 +1300,40 @@ intel_hdmi_unset_edid(struct drm_connector
>> *connector)
>>       to_intel_connector(connector)->detect_edid = NULL;
>>   }
>> +static bool bxt_port_connected(struct drm_i915_private *dev_priv,
>> +                   struct intel_digital_port *port)
>> +{
>> +    u32 temp = I915_READ(GEN8_DE_PORT_ISR);
>> +
>> +    /* TODO: Add bxt A0/A1 wa related to hpd pin swap */
>> +    switch (port->port) {
> why not pass the encoder and use its hpd_pin ? that will avoid the need
> to do
> A0/A1 related checks ?
This was kept to make it inline with the other functions which does the 
same thing. But because of the work around i think it will make sense to 
try the way you suggest.

>> +    case PORT_B:
>> +        return temp & BXT_DE_PORT_HP_DDIB;
>> +
>> +    case PORT_C:
>> +        return temp & BXT_DE_PORT_HP_DDIC;
>> +
>> +    default:
>> +        return false;
>> +
>> +    }
>> +}
>> +
>> +static bool intel_hdmi_live_status(struct intel_digital_port
>> *intel_dig_port)
>> +{
>> +    struct drm_device *dev = intel_dig_port->base.base.dev;
>> +    struct drm_i915_private *dev_priv = to_i915(dev);
>> +
>> +    if (IS_VALLEYVIEW(dev))
>> +        return g4x_digital_port_connected(dev, intel_dig_port);
>> +    else if (IS_SKYLAKE(dev))
>> +        return ibx_digital_port_connected(dev_priv, intel_dig_port);
>> +    else if (IS_BROXTON(dev))
>> +        return bxt_port_connected(dev_priv, intel_dig_port);
>> +
>> +    return true;
>> +}
>> +
>>   static bool
>>   intel_hdmi_set_edid(struct drm_connector *connector)
>>   {
>> @@ -1308,15 +1342,16 @@ intel_hdmi_set_edid(struct drm_connector
>> *connector)
>>       struct intel_encoder *intel_encoder =
>>           &hdmi_to_dig_port(intel_hdmi)->base;
>>       enum intel_display_power_domain power_domain;
>> -    struct edid *edid;
>> +    struct edid *edid = NULL;
>>       bool connected = false;
>>       power_domain = intel_display_port_power_domain(intel_encoder);
>>       intel_display_power_get(dev_priv, power_domain);
>> -    edid = drm_get_edid(connector,
>> -                intel_gmbus_get_adapter(dev_priv,
>> -                            intel_hdmi->ddc_bus));
>> +    if (intel_hdmi_live_status(hdmi_to_dig_port(intel_hdmi)))
>> +        edid = drm_get_edid(connector,
>> +                intel_gmbus_get_adapter(dev_priv,
>> +                    intel_hdmi->ddc_bus));
>>       intel_display_power_put(dev_priv, power_domain);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] HDMI optimization series

[Daniel Vetter]

On Tue, Jul 14, 2015 at 05:21:20PM +0530, Sonika Jindal wrote:
> This series adds some optimization related to reading of edid only when
> required and when live status says so.
> The comments in the patches explain more.

This series breaks my funky ivb machine with the broken hpd bits: When I
unplug the screen the system never invalidates the edid and so never
notices that it's gone.

Now iirc you've discovered an issue in our hpd irq handler which can cause
lost hpd bits, but that patch isn't in this series. And iirc I asked you
to resend everything since that hw fix also wasn't in the last series. And
I can't find it any more. Did I just dream about this other patch to fix
hpd on big core?

Thanks, Daniel

> v2:
>    Some refactoring is with this series.
> 
>    Also, right now this is done for platforms gen7 and above because we
>    couldn't test with older platforms. For newer platforms it works
>    reliably.
> 
>    For HPD and live status to work on SKL, following patch is required:
>    "drm/i915: Handle HPD when it has actually occurred"
> 
> Shashank Sharma (2):
>   drm/i915: add attached connector to hdmi container
>   drm/i915: Add HDMI probe function
> 
> Sonika Jindal (1):
>   drm/i915: Check live status before reading edid
> 
>  drivers/gpu/drm/i915/intel_dp.c   |    4 +-
>  drivers/gpu/drm/i915/intel_drv.h  |    3 ++
>  drivers/gpu/drm/i915/intel_hdmi.c |   93 +++++++++++++++++++++++++++++++++----
>  3 files changed, 88 insertions(+), 12 deletions(-)
> 
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] HDMI optimization series

[Jindal, Sonika]

Hi Daniel,

That patch was already merged:
http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.html

For SKL, the above patch helped in getting the correct ISR bits set.
One option is to enable the HDMI optimization from VLV onwards.
I don't have an ivb machine to try out the issue.

Regards,
Sonika

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Friday, August 7, 2015 6:54 PM
To: Jindal, Sonika
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series

On Tue, Jul 14, 2015 at 05:21:20PM +0530, Sonika Jindal wrote:
> This series adds some optimization related to reading of edid only 
> when required and when live status says so.
> The comments in the patches explain more.

This series breaks my funky ivb machine with the broken hpd bits: When I unplug the screen the system never invalidates the edid and so never notices that it's gone.

Now iirc you've discovered an issue in our hpd irq handler which can cause lost hpd bits, but that patch isn't in this series. And iirc I asked you to resend everything since that hw fix also wasn't in the last series. And I can't find it any more. Did I just dream about this other patch to fix hpd on big core?

Thanks, Daniel

> v2:
>    Some refactoring is with this series.
> 
>    Also, right now this is done for platforms gen7 and above because we
>    couldn't test with older platforms. For newer platforms it works
>    reliably.
> 
>    For HPD and live status to work on SKL, following patch is required:
>    "drm/i915: Handle HPD when it has actually occurred"
> 
> Shashank Sharma (2):
>   drm/i915: add attached connector to hdmi container
>   drm/i915: Add HDMI probe function
> 
> Sonika Jindal (1):
>   drm/i915: Check live status before reading edid
> 
>  drivers/gpu/drm/i915/intel_dp.c   |    4 +-
>  drivers/gpu/drm/i915/intel_drv.h  |    3 ++
>  drivers/gpu/drm/i915/intel_hdmi.c |   93 +++++++++++++++++++++++++++++++++----
>  3 files changed, 88 insertions(+), 12 deletions(-)
> 
> --
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] HDMI optimization series

[Daniel Vetter]

On Mon, Aug 10, 2015 at 04:50:37AM +0000, Jindal, Sonika wrote:
> Hi Daniel,
> 
> That patch was already merged:
> http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.html
> 
> For SKL, the above patch helped in getting the correct ISR bits set.
> One option is to enable the HDMI optimization from VLV onwards.
> I don't have an ivb machine to try out the issue.

ivb is simple the machine I have here, but when we tried this the last
time around we had reports for all platforms (your patch still doesn't
cite the relevant sha1 btw). I think there are 3 possible explanations:

a) we do something wrong with hpd handling on these platforms. That seems
to be the explanation you favour (with the gen >= 7 checks and all that),
but I think it's very unlikely: On each platform where we had reports of
hpd being broken there was also machines where hpd works perfectly fine.

b) There's broken HDMI (or DVI) sinks out there. If that's the case we can
never merge your patch.

c) There's something in vbt or somewhere else that tells the windows
driver whether using hpd or not is ok (and the hpd problem is actually an
issue with certain OEM machines ...).

I hoped that with your hpd handling fix we'd have some indication that our
hpd troubles are of type a). But since I tested with your patch that
didn't work out.

And until we have some evidence that our hpd troubles aren't type b) I
really don't want to merge any patch which relies upon hpd bits for hdmi.
-Daniel

> 
> Regards,
> Sonika
> 
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Friday, August 7, 2015 6:54 PM
> To: Jindal, Sonika
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series
> 
> On Tue, Jul 14, 2015 at 05:21:20PM +0530, Sonika Jindal wrote:
> > This series adds some optimization related to reading of edid only 
> > when required and when live status says so.
> > The comments in the patches explain more.
> 
> This series breaks my funky ivb machine with the broken hpd bits: When I unplug the screen the system never invalidates the edid and so never notices that it's gone.
> 
> Now iirc you've discovered an issue in our hpd irq handler which can cause lost hpd bits, but that patch isn't in this series. And iirc I asked you to resend everything since that hw fix also wasn't in the last series. And I can't find it any more. Did I just dream about this other patch to fix hpd on big core?
> 
> Thanks, Daniel
> 
> > v2:
> >    Some refactoring is with this series.
> > 
> >    Also, right now this is done for platforms gen7 and above because we
> >    couldn't test with older platforms. For newer platforms it works
> >    reliably.
> > 
> >    For HPD and live status to work on SKL, following patch is required:
> >    "drm/i915: Handle HPD when it has actually occurred"
> > 
> > Shashank Sharma (2):
> >   drm/i915: add attached connector to hdmi container
> >   drm/i915: Add HDMI probe function
> > 
> > Sonika Jindal (1):
> >   drm/i915: Check live status before reading edid
> > 
> >  drivers/gpu/drm/i915/intel_dp.c   |    4 +-
> >  drivers/gpu/drm/i915/intel_drv.h  |    3 ++
> >  drivers/gpu/drm/i915/intel_hdmi.c |   93 +++++++++++++++++++++++++++++++++----
> >  3 files changed, 88 insertions(+), 12 deletions(-)
> > 
> > --
> > 1.7.10.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] HDMI optimization series

[Jindal, Sonika]

On 8/10/2015 1:38 PM, Daniel Vetter wrote:
> On Mon, Aug 10, 2015 at 04:50:37AM +0000, Jindal, Sonika wrote:
>> Hi Daniel,
>>
>> That patch was already merged:
>> http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.html
>>
>> For SKL, the above patch helped in getting the correct ISR bits set.
>> One option is to enable the HDMI optimization from VLV onwards.
>> I don't have an ivb machine to try out the issue.
>
> ivb is simple the machine I have here, but when we tried this the last
> time around we had reports for all platforms (your patch still doesn't
> cite the relevant sha1 btw). I think there are 3 possible explanations:
>
Yes, I don't know which were those patches and how to find them..

> a) we do something wrong with hpd handling on these platforms. That seems
> to be the explanation you favour (with the gen >= 7 checks and all that),
> but I think it's very unlikely: On each platform where we had reports of
> hpd being broken there was also machines where hpd works perfectly fine.
>
Not sure, I will find one ivb system and try on that.

> b) There's broken HDMI (or DVI) sinks out there. If that's the case we can
> never merge your patch.
I doubt this because we have tested these patches with many sinks in the 
past with VLV/CHV.
>
> c) There's something in vbt or somewhere else that tells the windows
> driver whether using hpd or not is ok (and the hpd problem is actually an
> issue with certain OEM machines ...).
>
No, nothing like that.

> I hoped that with your hpd handling fix we'd have some indication that our
> hpd troubles are of type a). But since I tested with your patch that
> didn't work out.
>
> And until we have some evidence that our hpd troubles aren't type b) I
> really don't want to merge any patch which relies upon hpd bits for hdmi.
> -Daniel
>
I will try on ivb.

Regards
Sonika

>>
>> Regards,
>> Sonika
>>
>> -----Original Message-----
>> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
>> Sent: Friday, August 7, 2015 6:54 PM
>> To: Jindal, Sonika
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series
>>
>> On Tue, Jul 14, 2015 at 05:21:20PM +0530, Sonika Jindal wrote:
>>> This series adds some optimization related to reading of edid only
>>> when required and when live status says so.
>>> The comments in the patches explain more.
>>
>> This series breaks my funky ivb machine with the broken hpd bits: When I unplug the screen the system never invalidates the edid and so never notices that it's gone.
>>
>> Now iirc you've discovered an issue in our hpd irq handler which can cause lost hpd bits, but that patch isn't in this series. And iirc I asked you to resend everything since that hw fix also wasn't in the last series. And I can't find it any more. Did I just dream about this other patch to fix hpd on big core?
>>
>> Thanks, Daniel
>>
>>> v2:
>>>     Some refactoring is with this series.
>>>
>>>     Also, right now this is done for platforms gen7 and above because we
>>>     couldn't test with older platforms. For newer platforms it works
>>>     reliably.
>>>
>>>     For HPD and live status to work on SKL, following patch is required:
>>>     "drm/i915: Handle HPD when it has actually occurred"
>>>
>>> Shashank Sharma (2):
>>>    drm/i915: add attached connector to hdmi container
>>>    drm/i915: Add HDMI probe function
>>>
>>> Sonika Jindal (1):
>>>    drm/i915: Check live status before reading edid
>>>
>>>   drivers/gpu/drm/i915/intel_dp.c   |    4 +-
>>>   drivers/gpu/drm/i915/intel_drv.h  |    3 ++
>>>   drivers/gpu/drm/i915/intel_hdmi.c |   93 +++++++++++++++++++++++++++++++++----
>>>   3 files changed, 88 insertions(+), 12 deletions(-)
>>>
>>> --
>>> 1.7.10.4
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] HDMI optimization series

[Daniel Vetter]

On Mon, Aug 10, 2015 at 02:05:47PM +0530, Jindal, Sonika wrote:
> 
> 
> On 8/10/2015 1:38 PM, Daniel Vetter wrote:
> >On Mon, Aug 10, 2015 at 04:50:37AM +0000, Jindal, Sonika wrote:
> >>Hi Daniel,
> >>
> >>That patch was already merged:
> >>http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.html
> >>
> >>For SKL, the above patch helped in getting the correct ISR bits set.
> >>One option is to enable the HDMI optimization from VLV onwards.
> >>I don't have an ivb machine to try out the issue.
> >
> >ivb is simple the machine I have here, but when we tried this the last
> >time around we had reports for all platforms (your patch still doesn't
> >cite the relevant sha1 btw). I think there are 3 possible explanations:
> >
> Yes, I don't know which were those patches and how to find them..

git blame (recursively) and git log on intel_hdmi.c will tell you the
story. I require all these extensive git commit messages because I dig
them out daily. Also git log --pickaxe (well I use the equivalent in the
gitk gui). If these tools are generally unknown in the vpg display team
then we absolutely need to do a training session about them, they're
extremely powerful and useful to dig out the history of the i915 code.

> >a) we do something wrong with hpd handling on these platforms. That seems
> >to be the explanation you favour (with the gen >= 7 checks and all that),
> >but I think it's very unlikely: On each platform where we had reports of
> >hpd being broken there was also machines where hpd works perfectly fine.
> >
> Not sure, I will find one ivb system and try on that.
> 
> >b) There's broken HDMI (or DVI) sinks out there. If that's the case we can
> >never merge your patch.
> I doubt this because we have tested these patches with many sinks in the
> past with VLV/CHV.
> >
> >c) There's something in vbt or somewhere else that tells the windows
> >driver whether using hpd or not is ok (and the hpd problem is actually an
> >issue with certain OEM machines ...).
> >
> No, nothing like that.
> 
> >I hoped that with your hpd handling fix we'd have some indication that our
> >hpd troubles are of type a). But since I tested with your patch that
> >didn't work out.
> >
> >And until we have some evidence that our hpd troubles aren't type b) I
> >really don't want to merge any patch which relies upon hpd bits for hdmi.
> >-Daniel
> >
> I will try on ivb.

I don't think it's ivb specific, since we do have ivb machines where hpd
works perfectly fine. Same for every other platform. The only thing which
seems common is that DP+ connectors work, but some of the hdmi-only
connectors fail. That's why I wondered whether there's something i915 does
different than the windows driver. In case you can get hold of one, the
broken laptop I have here is an Asus UX31A with ivb.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] HDMI optimization series

[Sharma, Shashank]

HI Daniel, 

Looks like we are not getting an IVB machine here. I think instead of blocking the patch set for the whole series, we can just block it for the platforms we don’t get success for.
We are shipping many VLV/CHV systems with these patches applied, and they are passing all Android test suits and test benches, so we don’t doubt the solution as such.

We have done testing for following platforms (gen 9 is tested by Imre also):
1. CHV / BSW
2. SKL
3. BXT
4. BDW

Why don’t we go ahead with this solution for gen 8 onwards, and keep the rest of the code un touched, this will serve both the purpose.

Regards
Shashank 
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Daniel Vetter
Sent: Tuesday, August 11, 2015 3:12 PM
To: Jindal, Sonika
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series

On Mon, Aug 10, 2015 at 02:05:47PM +0530, Jindal, Sonika wrote:
> 
> 
> On 8/10/2015 1:38 PM, Daniel Vetter wrote:
> >On Mon, Aug 10, 2015 at 04:50:37AM +0000, Jindal, Sonika wrote:
> >>Hi Daniel,
> >>
> >>That patch was already merged:
> >>http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.htm
> >>l
> >>
> >>For SKL, the above patch helped in getting the correct ISR bits set.
> >>One option is to enable the HDMI optimization from VLV onwards.
> >>I don't have an ivb machine to try out the issue.
> >
> >ivb is simple the machine I have here, but when we tried this the 
> >last time around we had reports for all platforms (your patch still 
> >doesn't cite the relevant sha1 btw). I think there are 3 possible explanations:
> >
> Yes, I don't know which were those patches and how to find them..

git blame (recursively) and git log on intel_hdmi.c will tell you the story. I require all these extensive git commit messages because I dig them out daily. Also git log --pickaxe (well I use the equivalent in the gitk gui). If these tools are generally unknown in the vpg display team then we absolutely need to do a training session about them, they're extremely powerful and useful to dig out the history of the i915 code.

> >a) we do something wrong with hpd handling on these platforms. That 
> >seems to be the explanation you favour (with the gen >= 7 checks and 
> >all that), but I think it's very unlikely: On each platform where we 
> >had reports of hpd being broken there was also machines where hpd works perfectly fine.
> >
> Not sure, I will find one ivb system and try on that.
> 
> >b) There's broken HDMI (or DVI) sinks out there. If that's the case 
> >we can never merge your patch.
> I doubt this because we have tested these patches with many sinks in 
> the past with VLV/CHV.
> >
> >c) There's something in vbt or somewhere else that tells the windows 
> >driver whether using hpd or not is ok (and the hpd problem is 
> >actually an issue with certain OEM machines ...).
> >
> No, nothing like that.
> 
> >I hoped that with your hpd handling fix we'd have some indication 
> >that our hpd troubles are of type a). But since I tested with your 
> >patch that didn't work out.
> >
> >And until we have some evidence that our hpd troubles aren't type b) 
> >I really don't want to merge any patch which relies upon hpd bits for hdmi.
> >-Daniel
> >
> I will try on ivb.

I don't think it's ivb specific, since we do have ivb machines where hpd works perfectly fine. Same for every other platform. The only thing which seems common is that DP+ connectors work, but some of the hdmi-only connectors fail. That's why I wondered whether there's something i915 does different than the windows driver. In case you can get hold of one, the broken laptop I have here is an Asus UX31A with ivb.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] HDMI optimization series

[Daniel Vetter]

On Tue, Aug 11, 2015 at 11:03:54AM +0000, Sharma, Shashank wrote:
> HI Daniel, 
> 
> Looks like we are not getting an IVB machine here. I think instead of blocking the patch set for the whole series, we can just block it for the platforms we don’t get success for.
> We are shipping many VLV/CHV systems with these patches applied, and they are passing all Android test suits and test benches, so we don’t doubt the solution as such.
> 
> We have done testing for following platforms (gen 9 is tested by Imre also):
> 1. CHV / BSW
> 2. SKL
> 3. BXT
> 4. BDW

I know that it works on piles of other platforms too. The problem is
figuring out why it doesn't work on some and what we're doing wrong.

> Why don’t we go ahead with this solution for gen 8 onwards, and keep the
> rest of the code un touched, this will serve both the purpose.

I guess we can, but if we get a regression report on bdw because of this
I'll be furious.
-Daniel

> 
> Regards
> Shashank 
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Daniel Vetter
> Sent: Tuesday, August 11, 2015 3:12 PM
> To: Jindal, Sonika
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series
> 
> On Mon, Aug 10, 2015 at 02:05:47PM +0530, Jindal, Sonika wrote:
> > 
> > 
> > On 8/10/2015 1:38 PM, Daniel Vetter wrote:
> > >On Mon, Aug 10, 2015 at 04:50:37AM +0000, Jindal, Sonika wrote:
> > >>Hi Daniel,
> > >>
> > >>That patch was already merged:
> > >>http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.htm
> > >>l
> > >>
> > >>For SKL, the above patch helped in getting the correct ISR bits set.
> > >>One option is to enable the HDMI optimization from VLV onwards.
> > >>I don't have an ivb machine to try out the issue.
> > >
> > >ivb is simple the machine I have here, but when we tried this the 
> > >last time around we had reports for all platforms (your patch still 
> > >doesn't cite the relevant sha1 btw). I think there are 3 possible explanations:
> > >
> > Yes, I don't know which were those patches and how to find them..
> 
> git blame (recursively) and git log on intel_hdmi.c will tell you the story. I require all these extensive git commit messages because I dig them out daily. Also git log --pickaxe (well I use the equivalent in the gitk gui). If these tools are generally unknown in the vpg display team then we absolutely need to do a training session about them, they're extremely powerful and useful to dig out the history of the i915 code.
> 
> > >a) we do something wrong with hpd handling on these platforms. That 
> > >seems to be the explanation you favour (with the gen >= 7 checks and 
> > >all that), but I think it's very unlikely: On each platform where we 
> > >had reports of hpd being broken there was also machines where hpd works perfectly fine.
> > >
> > Not sure, I will find one ivb system and try on that.
> > 
> > >b) There's broken HDMI (or DVI) sinks out there. If that's the case 
> > >we can never merge your patch.
> > I doubt this because we have tested these patches with many sinks in 
> > the past with VLV/CHV.
> > >
> > >c) There's something in vbt or somewhere else that tells the windows 
> > >driver whether using hpd or not is ok (and the hpd problem is 
> > >actually an issue with certain OEM machines ...).
> > >
> > No, nothing like that.
> > 
> > >I hoped that with your hpd handling fix we'd have some indication 
> > >that our hpd troubles are of type a). But since I tested with your 
> > >patch that didn't work out.
> > >
> > >And until we have some evidence that our hpd troubles aren't type b) 
> > >I really don't want to merge any patch which relies upon hpd bits for hdmi.
> > >-Daniel
> > >
> > I will try on ivb.
> 
> I don't think it's ivb specific, since we do have ivb machines where hpd works perfectly fine. Same for every other platform. The only thing which seems common is that DP+ connectors work, but some of the hdmi-only connectors fail. That's why I wondered whether there's something i915 does different than the windows driver. In case you can get hold of one, the broken laptop I have here is an Asus UX31A with ivb.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] HDMI optimization series

[Sharma, Shashank]

Regards
Shashank

On 8/12/2015 5:55 PM, Daniel Vetter wrote:
> On Tue, Aug 11, 2015 at 11:03:54AM +0000, Sharma, Shashank wrote:
>> HI Daniel,
>>
>> Looks like we are not getting an IVB machine here. I think instead of blocking the patch set for the whole series, we can just block it for the platforms we don’t get success for.
>> We are shipping many VLV/CHV systems with these patches applied, and they are passing all Android test suits and test benches, so we don’t doubt the solution as such.
>>
>> We have done testing for following platforms (gen 9 is tested by Imre also):
>> 1. CHV / BSW
>> 2. SKL
>> 3. BXT
>> 4. BDW
>
> I know that it works on piles of other platforms too. The problem is
> figuring out why it doesn't work on some and what we're doing wrong.
>
>> Why don’t we go ahead with this solution for gen 8 onwards, and keep the
>> rest of the code un touched, this will serve both the purpose.
>
> I guess we can, but if we get a regression report on bdw because of this
> I'll be furious.
> -Daniel
>
I completely agree, and understand your concern about adding regression. 
How about this, we perform an extended set of testing on BDW also, and 
only after seeing the test reports, we can take a call on merging these 
patches.
>>
>> Regards
>> Shashank
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Daniel Vetter
>> Sent: Tuesday, August 11, 2015 3:12 PM
>> To: Jindal, Sonika
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series
>>
>> On Mon, Aug 10, 2015 at 02:05:47PM +0530, Jindal, Sonika wrote:
>>>
>>>
>>> On 8/10/2015 1:38 PM, Daniel Vetter wrote:
>>>> On Mon, Aug 10, 2015 at 04:50:37AM +0000, Jindal, Sonika wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> That patch was already merged:
>>>>> http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.htm
>>>>> l
>>>>>
>>>>> For SKL, the above patch helped in getting the correct ISR bits set.
>>>>> One option is to enable the HDMI optimization from VLV onwards.
>>>>> I don't have an ivb machine to try out the issue.
>>>>
>>>> ivb is simple the machine I have here, but when we tried this the
>>>> last time around we had reports for all platforms (your patch still
>>>> doesn't cite the relevant sha1 btw). I think there are 3 possible explanations:
>>>>
>>> Yes, I don't know which were those patches and how to find them..
>>
>> git blame (recursively) and git log on intel_hdmi.c will tell you the story. I require all these extensive git commit messages because I dig them out daily. Also git log --pickaxe (well I use the equivalent in the gitk gui). If these tools are generally unknown in the vpg display team then we absolutely need to do a training session about them, they're extremely powerful and useful to dig out the history of the i915 code.
>>
>>>> a) we do something wrong with hpd handling on these platforms. That
>>>> seems to be the explanation you favour (with the gen >= 7 checks and
>>>> all that), but I think it's very unlikely: On each platform where we
>>>> had reports of hpd being broken there was also machines where hpd works perfectly fine.
>>>>
>>> Not sure, I will find one ivb system and try on that.
>>>
>>>> b) There's broken HDMI (or DVI) sinks out there. If that's the case
>>>> we can never merge your patch.
>>> I doubt this because we have tested these patches with many sinks in
>>> the past with VLV/CHV.
>>>>
>>>> c) There's something in vbt or somewhere else that tells the windows
>>>> driver whether using hpd or not is ok (and the hpd problem is
>>>> actually an issue with certain OEM machines ...).
>>>>
>>> No, nothing like that.
>>>
>>>> I hoped that with your hpd handling fix we'd have some indication
>>>> that our hpd troubles are of type a). But since I tested with your
>>>> patch that didn't work out.
>>>>
>>>> And until we have some evidence that our hpd troubles aren't type b)
>>>> I really don't want to merge any patch which relies upon hpd bits for hdmi.
>>>> -Daniel
>>>>
>>> I will try on ivb.
>>
>> I don't think it's ivb specific, since we do have ivb machines where hpd works perfectly fine. Same for every other platform. The only thing which seems common is that DP+ connectors work, but some of the hdmi-only connectors fail. That's why I wondered whether there's something i915 does different than the windows driver. In case you can get hold of one, the broken laptop I have here is an Asus UX31A with ivb.
>> -Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] HDMI optimization series

[Daniel Vetter]

On Wed, Aug 12, 2015 at 06:04:50PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 8/12/2015 5:55 PM, Daniel Vetter wrote:
> >On Tue, Aug 11, 2015 at 11:03:54AM +0000, Sharma, Shashank wrote:
> >>HI Daniel,
> >>
> >>Looks like we are not getting an IVB machine here. I think instead of blocking the patch set for the whole series, we can just block it for the platforms we don’t get success for.
> >>We are shipping many VLV/CHV systems with these patches applied, and they are passing all Android test suits and test benches, so we don’t doubt the solution as such.
> >>
> >>We have done testing for following platforms (gen 9 is tested by Imre also):
> >>1. CHV / BSW
> >>2. SKL
> >>3. BXT
> >>4. BDW
> >
> >I know that it works on piles of other platforms too. The problem is
> >figuring out why it doesn't work on some and what we're doing wrong.
> >
> >>Why don’t we go ahead with this solution for gen 8 onwards, and keep the
> >>rest of the code un touched, this will serve both the purpose.
> >
> >I guess we can, but if we get a regression report on bdw because of this
> >I'll be furious.
> >-Daniel
> >
> I completely agree, and understand your concern about adding regression. How
> about this, we perform an extended set of testing on BDW also, and only
> after seeing the test reports, we can take a call on merging these patches.

The problem is that no one seems to have machines with broken hpd, I
stumbled over the broken ivb here purely by accident. The only way to get
testing is to merge the patches and then wait for someone to complain.
Trying to do exhaustive testing beforehand is imo impossible.

The only thing which would alleviate my concerns is fixing the hpd
handling bugs we know about already, since in the past we've just done "oh
doesn't work on genX, try on genX+1 only then" and that never worked
out. And we've tried this for _every_ _single_ platform since gen4. But we
can try once more, simply be aware that if it breaks again I will not
accept a patch to restrict to gen9 but instead will rip it out again.
-Daniel

> >>
> >>Regards
> >>Shashank
> >>-----Original Message-----
> >>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Daniel Vetter
> >>Sent: Tuesday, August 11, 2015 3:12 PM
> >>To: Jindal, Sonika
> >>Cc: intel-gfx@lists.freedesktop.org
> >>Subject: Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series
> >>
> >>On Mon, Aug 10, 2015 at 02:05:47PM +0530, Jindal, Sonika wrote:
> >>>
> >>>
> >>>On 8/10/2015 1:38 PM, Daniel Vetter wrote:
> >>>>On Mon, Aug 10, 2015 at 04:50:37AM +0000, Jindal, Sonika wrote:
> >>>>>Hi Daniel,
> >>>>>
> >>>>>That patch was already merged:
> >>>>>http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.htm
> >>>>>l
> >>>>>
> >>>>>For SKL, the above patch helped in getting the correct ISR bits set.
> >>>>>One option is to enable the HDMI optimization from VLV onwards.
> >>>>>I don't have an ivb machine to try out the issue.
> >>>>
> >>>>ivb is simple the machine I have here, but when we tried this the
> >>>>last time around we had reports for all platforms (your patch still
> >>>>doesn't cite the relevant sha1 btw). I think there are 3 possible explanations:
> >>>>
> >>>Yes, I don't know which were those patches and how to find them..
> >>
> >>git blame (recursively) and git log on intel_hdmi.c will tell you the story. I require all these extensive git commit messages because I dig them out daily. Also git log --pickaxe (well I use the equivalent in the gitk gui). If these tools are generally unknown in the vpg display team then we absolutely need to do a training session about them, they're extremely powerful and useful to dig out the history of the i915 code.
> >>
> >>>>a) we do something wrong with hpd handling on these platforms. That
> >>>>seems to be the explanation you favour (with the gen >= 7 checks and
> >>>>all that), but I think it's very unlikely: On each platform where we
> >>>>had reports of hpd being broken there was also machines where hpd works perfectly fine.
> >>>>
> >>>Not sure, I will find one ivb system and try on that.
> >>>
> >>>>b) There's broken HDMI (or DVI) sinks out there. If that's the case
> >>>>we can never merge your patch.
> >>>I doubt this because we have tested these patches with many sinks in
> >>>the past with VLV/CHV.
> >>>>
> >>>>c) There's something in vbt or somewhere else that tells the windows
> >>>>driver whether using hpd or not is ok (and the hpd problem is
> >>>>actually an issue with certain OEM machines ...).
> >>>>
> >>>No, nothing like that.
> >>>
> >>>>I hoped that with your hpd handling fix we'd have some indication
> >>>>that our hpd troubles are of type a). But since I tested with your
> >>>>patch that didn't work out.
> >>>>
> >>>>And until we have some evidence that our hpd troubles aren't type b)
> >>>>I really don't want to merge any patch which relies upon hpd bits for hdmi.
> >>>>-Daniel
> >>>>
> >>>I will try on ivb.
> >>
> >>I don't think it's ivb specific, since we do have ivb machines where hpd works perfectly fine. Same for every other platform. The only thing which seems common is that DP+ connectors work, but some of the hdmi-only connectors fail. That's why I wondered whether there's something i915 does different than the windows driver. In case you can get hold of one, the broken laptop I have here is an Asus UX31A with ivb.
> >>-Daniel
> >>--
> >>Daniel Vetter
> >>Software Engineer, Intel Corporation
> >>http://blog.ffwll.ch
> >>_______________________________________________
> >>Intel-gfx mailing list
> >>Intel-gfx@lists.freedesktop.org
> >>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] HDMI optimization series

[Sharma, Shashank]

Sure, I understood the point. 
We will do another round of testing with gen8, and when we are good with that, will submit the patches again.

Regards
Shashank
-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Wednesday, August 12, 2015 6:11 PM
To: Sharma, Shashank
Cc: Daniel Vetter; Jindal, Sonika; intel-gfx@lists.freedesktop.org; Mukherjee, Indranil
Subject: Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series

On Wed, Aug 12, 2015 at 06:04:50PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> 
> On 8/12/2015 5:55 PM, Daniel Vetter wrote:
> >On Tue, Aug 11, 2015 at 11:03:54AM +0000, Sharma, Shashank wrote:
> >>HI Daniel,
> >>
> >>Looks like we are not getting an IVB machine here. I think instead of blocking the patch set for the whole series, we can just block it for the platforms we don’t get success for.
> >>We are shipping many VLV/CHV systems with these patches applied, and they are passing all Android test suits and test benches, so we don’t doubt the solution as such.
> >>
> >>We have done testing for following platforms (gen 9 is tested by Imre also):
> >>1. CHV / BSW
> >>2. SKL
> >>3. BXT
> >>4. BDW
> >
> >I know that it works on piles of other platforms too. The problem is 
> >figuring out why it doesn't work on some and what we're doing wrong.
> >
> >>Why don’t we go ahead with this solution for gen 8 onwards, and keep 
> >>the rest of the code un touched, this will serve both the purpose.
> >
> >I guess we can, but if we get a regression report on bdw because of 
> >this I'll be furious.
> >-Daniel
> >
> I completely agree, and understand your concern about adding 
> regression. How about this, we perform an extended set of testing on 
> BDW also, and only after seeing the test reports, we can take a call on merging these patches.

The problem is that no one seems to have machines with broken hpd, I stumbled over the broken ivb here purely by accident. The only way to get testing is to merge the patches and then wait for someone to complain.
Trying to do exhaustive testing beforehand is imo impossible.

The only thing which would alleviate my concerns is fixing the hpd handling bugs we know about already, since in the past we've just done "oh doesn't work on genX, try on genX+1 only then" and that never worked out. And we've tried this for _every_ _single_ platform since gen4. But we can try once more, simply be aware that if it breaks again I will not accept a patch to restrict to gen9 but instead will rip it out again.
-Daniel

> >>
> >>Regards
> >>Shashank
> >>-----Original Message-----
> >>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On 
> >>Behalf Of Daniel Vetter
> >>Sent: Tuesday, August 11, 2015 3:12 PM
> >>To: Jindal, Sonika
> >>Cc: intel-gfx@lists.freedesktop.org
> >>Subject: Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series
> >>
> >>On Mon, Aug 10, 2015 at 02:05:47PM +0530, Jindal, Sonika wrote:
> >>>
> >>>
> >>>On 8/10/2015 1:38 PM, Daniel Vetter wrote:
> >>>>On Mon, Aug 10, 2015 at 04:50:37AM +0000, Jindal, Sonika wrote:
> >>>>>Hi Daniel,
> >>>>>
> >>>>>That patch was already merged:
> >>>>>http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.
> >>>>>htm
> >>>>>l
> >>>>>
> >>>>>For SKL, the above patch helped in getting the correct ISR bits set.
> >>>>>One option is to enable the HDMI optimization from VLV onwards.
> >>>>>I don't have an ivb machine to try out the issue.
> >>>>
> >>>>ivb is simple the machine I have here, but when we tried this the 
> >>>>last time around we had reports for all platforms (your patch 
> >>>>still doesn't cite the relevant sha1 btw). I think there are 3 possible explanations:
> >>>>
> >>>Yes, I don't know which were those patches and how to find them..
> >>
> >>git blame (recursively) and git log on intel_hdmi.c will tell you the story. I require all these extensive git commit messages because I dig them out daily. Also git log --pickaxe (well I use the equivalent in the gitk gui). If these tools are generally unknown in the vpg display team then we absolutely need to do a training session about them, they're extremely powerful and useful to dig out the history of the i915 code.
> >>
> >>>>a) we do something wrong with hpd handling on these platforms. 
> >>>>That seems to be the explanation you favour (with the gen >= 7 
> >>>>checks and all that), but I think it's very unlikely: On each 
> >>>>platform where we had reports of hpd being broken there was also machines where hpd works perfectly fine.
> >>>>
> >>>Not sure, I will find one ivb system and try on that.
> >>>
> >>>>b) There's broken HDMI (or DVI) sinks out there. If that's the 
> >>>>case we can never merge your patch.
> >>>I doubt this because we have tested these patches with many sinks 
> >>>in the past with VLV/CHV.
> >>>>
> >>>>c) There's something in vbt or somewhere else that tells the 
> >>>>windows driver whether using hpd or not is ok (and the hpd problem 
> >>>>is actually an issue with certain OEM machines ...).
> >>>>
> >>>No, nothing like that.
> >>>
> >>>>I hoped that with your hpd handling fix we'd have some indication 
> >>>>that our hpd troubles are of type a). But since I tested with your 
> >>>>patch that didn't work out.
> >>>>
> >>>>And until we have some evidence that our hpd troubles aren't type 
> >>>>b) I really don't want to merge any patch which relies upon hpd bits for hdmi.
> >>>>-Daniel
> >>>>
> >>>I will try on ivb.
> >>
> >>I don't think it's ivb specific, since we do have ivb machines where hpd works perfectly fine. Same for every other platform. The only thing which seems common is that DP+ connectors work, but some of the hdmi-only connectors fail. That's why I wondered whether there's something i915 does different than the windows driver. In case you can get hold of one, the broken laptop I have here is an Asus UX31A with ivb.
> >>-Daniel
> >>--
> >>Daniel Vetter
> >>Software Engineer, Intel Corporation http://blog.ffwll.ch 
> >>_______________________________________________
> >>Intel-gfx mailing list
> >>Intel-gfx@lists.freedesktop.org
> >>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] HDMI optimization series

[David Weinehall]

On Tue, Aug 11, 2015 at 11:41:42AM +0200, Daniel Vetter wrote:
> On Mon, Aug 10, 2015 at 02:05:47PM +0530, Jindal, Sonika wrote:
> > 
> > 
> > On 8/10/2015 1:38 PM, Daniel Vetter wrote:
> > >On Mon, Aug 10, 2015 at 04:50:37AM +0000, Jindal, Sonika wrote:
> > >>Hi Daniel,
> > >>
> > >>That patch was already merged:
> > >>http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.html
> > >>
> > >>For SKL, the above patch helped in getting the correct ISR bits set.
> > >>One option is to enable the HDMI optimization from VLV onwards.
> > >>I don't have an ivb machine to try out the issue.
> > >
> > >ivb is simple the machine I have here, but when we tried this the last
> > >time around we had reports for all platforms (your patch still doesn't
> > >cite the relevant sha1 btw). I think there are 3 possible explanations:
> > >
> > Yes, I don't know which were those patches and how to find them..
> 
> git blame (recursively) and git log on intel_hdmi.c will tell you the
> story. I require all these extensive git commit messages because I dig
> them out daily. Also git log --pickaxe (well I use the equivalent in the
> gitk gui). If these tools are generally unknown in the vpg display team
> then we absolutely need to do a training session about them, they're
> extremely powerful and useful to dig out the history of the i915 code.

git blame I use a lot to find whom to ask for clarification.
pickaxe, however, was new to me. Thanks for the tip, will look into it.

As far as training sessions about git goes, I think a lot of people could
benefit from it; git has a LOT of hidden gems, and it seems almost
everyone has different workflows.


Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx