intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: fix VBT parsing for SDVO child device mapping
@ 2015-08-18 11:28 Jani Nikula
  2015-08-18 13:57 ` Ville Syrjälä
  2015-08-28  3:15 ` shuang.he
  0 siblings, 2 replies; 4+ messages in thread
From: Jani Nikula @ 2015-08-18 11:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

commit 75067ddecf21271631bc018d2fb23ddd09b66aae
Author: Antti Koskipaa <antti.koskipaa@linux.intel.com>
Date:   Fri Jul 10 14:10:55 2015 +0300

    drm/i915: Per-DDI I_boost override

increased size of union child_device_config without taking into account
the size check in parse_sdvo_device_mapping(). Switch the function over
to using the legacy struct only.

Fixes: 75067ddecf21 ("drm/i915: Per-DDI I_boost override")
Cc: Antti Koskipaa <antti.koskipaa@linux.intel.com>
Cc: David Weinehall <david.weinehall@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 50 +++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 8e46149bafdd..1b7e1a591a37 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -401,7 +401,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
 {
 	struct sdvo_device_mapping *p_mapping;
 	const struct bdb_general_definitions *p_defs;
-	const union child_device_config *p_child;
+	const struct old_child_dev_config *child; /* legacy */
 	int i, child_device_num, count;
 	u16	block_size;
 
@@ -410,14 +410,14 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
 		DRM_DEBUG_KMS("No general definition block is found, unable to construct sdvo mapping.\n");
 		return;
 	}
-	/* judge whether the size of child device meets the requirements.
-	 * If the child device size obtained from general definition block
-	 * is different with sizeof(struct child_device_config), skip the
-	 * parsing of sdvo device info
+
+	/*
+	 * Only parse SDVO mappings when the general definitions block child
+	 * device size matches that of the *legacy* child device config
+	 * struct. Thus, SDVO mapping will be skipped for newer VBT.
 	 */
-	if (p_defs->child_dev_size != sizeof(*p_child)) {
-		/* different child dev size . Ignore it */
-		DRM_DEBUG_KMS("different child size is found. Invalid.\n");
+	if (p_defs->child_dev_size != sizeof(*child)) {
+		DRM_DEBUG_KMS("Unsupported child device size for SDVO mapping.\n");
 		return;
 	}
 	/* get the block size of general definitions */
@@ -427,37 +427,37 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
 		p_defs->child_dev_size;
 	count = 0;
 	for (i = 0; i < child_device_num; i++) {
-		p_child = child_device_ptr(p_defs, i);
-		if (!p_child->old.device_type) {
+		child = &child_device_ptr(p_defs, i)->old;
+		if (!child->device_type) {
 			/* skip the device block if device type is invalid */
 			continue;
 		}
-		if (p_child->old.slave_addr != SLAVE_ADDR1 &&
-			p_child->old.slave_addr != SLAVE_ADDR2) {
+		if (child->slave_addr != SLAVE_ADDR1 &&
+		    child->slave_addr != SLAVE_ADDR2) {
 			/*
 			 * If the slave address is neither 0x70 nor 0x72,
 			 * it is not a SDVO device. Skip it.
 			 */
 			continue;
 		}
-		if (p_child->old.dvo_port != DEVICE_PORT_DVOB &&
-			p_child->old.dvo_port != DEVICE_PORT_DVOC) {
+		if (child->dvo_port != DEVICE_PORT_DVOB &&
+		    child->dvo_port != DEVICE_PORT_DVOC) {
 			/* skip the incorrect SDVO port */
 			DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
 			continue;
 		}
 		DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
-				" %s port\n",
-				p_child->old.slave_addr,
-				(p_child->old.dvo_port == DEVICE_PORT_DVOB) ?
-					"SDVOB" : "SDVOC");
-		p_mapping = &(dev_priv->sdvo_mappings[p_child->old.dvo_port - 1]);
+			      " %s port\n",
+			      child->slave_addr,
+			      (child->dvo_port == DEVICE_PORT_DVOB) ?
+			      "SDVOB" : "SDVOC");
+		p_mapping = &(dev_priv->sdvo_mappings[child->dvo_port - 1]);
 		if (!p_mapping->initialized) {
-			p_mapping->dvo_port = p_child->old.dvo_port;
-			p_mapping->slave_addr = p_child->old.slave_addr;
-			p_mapping->dvo_wiring = p_child->old.dvo_wiring;
-			p_mapping->ddc_pin = p_child->old.ddc_pin;
-			p_mapping->i2c_pin = p_child->old.i2c_pin;
+			p_mapping->dvo_port = child->dvo_port;
+			p_mapping->slave_addr = child->slave_addr;
+			p_mapping->dvo_wiring = child->dvo_wiring;
+			p_mapping->ddc_pin = child->ddc_pin;
+			p_mapping->i2c_pin = child->i2c_pin;
 			p_mapping->initialized = 1;
 			DRM_DEBUG_KMS("SDVO device: dvo=%x, addr=%x, wiring=%d, ddc_pin=%d, i2c_pin=%d\n",
 				      p_mapping->dvo_port,
@@ -469,7 +469,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
 			DRM_DEBUG_KMS("Maybe one SDVO port is shared by "
 					 "two SDVO device.\n");
 		}
-		if (p_child->old.slave2_addr) {
+		if (child->slave2_addr) {
 			/* Maybe this is a SDVO device with multiple inputs */
 			/* And the mapping info is not added */
 			DRM_DEBUG_KMS("there exists the slave2_addr. Maybe this"
-- 
2.1.4

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

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

* Re: [PATCH] drm/i915: fix VBT parsing for SDVO child device mapping
  2015-08-18 11:28 [PATCH] drm/i915: fix VBT parsing for SDVO child device mapping Jani Nikula
@ 2015-08-18 13:57 ` Ville Syrjälä
  2015-08-19  6:54   ` Jani Nikula
  2015-08-28  3:15 ` shuang.he
  1 sibling, 1 reply; 4+ messages in thread
From: Ville Syrjälä @ 2015-08-18 13:57 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Tue, Aug 18, 2015 at 02:28:55PM +0300, Jani Nikula wrote:
> commit 75067ddecf21271631bc018d2fb23ddd09b66aae
> Author: Antti Koskipaa <antti.koskipaa@linux.intel.com>
> Date:   Fri Jul 10 14:10:55 2015 +0300
> 
>     drm/i915: Per-DDI I_boost override
> 
> increased size of union child_device_config without taking into account
> the size check in parse_sdvo_device_mapping(). Switch the function over
> to using the legacy struct only.
> 
> Fixes: 75067ddecf21 ("drm/i915: Per-DDI I_boost override")
> Cc: Antti Koskipaa <antti.koskipaa@linux.intel.com>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Also gave it a spin on my 946gz so:
Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_bios.c | 50 +++++++++++++++++++--------------------
>  1 file changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 8e46149bafdd..1b7e1a591a37 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -401,7 +401,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
>  {
>  	struct sdvo_device_mapping *p_mapping;
>  	const struct bdb_general_definitions *p_defs;
> -	const union child_device_config *p_child;
> +	const struct old_child_dev_config *child; /* legacy */
>  	int i, child_device_num, count;
>  	u16	block_size;
>  
> @@ -410,14 +410,14 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
>  		DRM_DEBUG_KMS("No general definition block is found, unable to construct sdvo mapping.\n");
>  		return;
>  	}
> -	/* judge whether the size of child device meets the requirements.
> -	 * If the child device size obtained from general definition block
> -	 * is different with sizeof(struct child_device_config), skip the
> -	 * parsing of sdvo device info
> +
> +	/*
> +	 * Only parse SDVO mappings when the general definitions block child
> +	 * device size matches that of the *legacy* child device config
> +	 * struct. Thus, SDVO mapping will be skipped for newer VBT.
>  	 */
> -	if (p_defs->child_dev_size != sizeof(*p_child)) {
> -		/* different child dev size . Ignore it */
> -		DRM_DEBUG_KMS("different child size is found. Invalid.\n");
> +	if (p_defs->child_dev_size != sizeof(*child)) {
> +		DRM_DEBUG_KMS("Unsupported child device size for SDVO mapping.\n");
>  		return;
>  	}
>  	/* get the block size of general definitions */
> @@ -427,37 +427,37 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
>  		p_defs->child_dev_size;
>  	count = 0;
>  	for (i = 0; i < child_device_num; i++) {
> -		p_child = child_device_ptr(p_defs, i);
> -		if (!p_child->old.device_type) {
> +		child = &child_device_ptr(p_defs, i)->old;
> +		if (!child->device_type) {
>  			/* skip the device block if device type is invalid */
>  			continue;
>  		}
> -		if (p_child->old.slave_addr != SLAVE_ADDR1 &&
> -			p_child->old.slave_addr != SLAVE_ADDR2) {
> +		if (child->slave_addr != SLAVE_ADDR1 &&
> +		    child->slave_addr != SLAVE_ADDR2) {
>  			/*
>  			 * If the slave address is neither 0x70 nor 0x72,
>  			 * it is not a SDVO device. Skip it.
>  			 */
>  			continue;
>  		}
> -		if (p_child->old.dvo_port != DEVICE_PORT_DVOB &&
> -			p_child->old.dvo_port != DEVICE_PORT_DVOC) {
> +		if (child->dvo_port != DEVICE_PORT_DVOB &&
> +		    child->dvo_port != DEVICE_PORT_DVOC) {
>  			/* skip the incorrect SDVO port */
>  			DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
>  			continue;
>  		}
>  		DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
> -				" %s port\n",
> -				p_child->old.slave_addr,
> -				(p_child->old.dvo_port == DEVICE_PORT_DVOB) ?
> -					"SDVOB" : "SDVOC");
> -		p_mapping = &(dev_priv->sdvo_mappings[p_child->old.dvo_port - 1]);
> +			      " %s port\n",
> +			      child->slave_addr,
> +			      (child->dvo_port == DEVICE_PORT_DVOB) ?
> +			      "SDVOB" : "SDVOC");
> +		p_mapping = &(dev_priv->sdvo_mappings[child->dvo_port - 1]);
>  		if (!p_mapping->initialized) {
> -			p_mapping->dvo_port = p_child->old.dvo_port;
> -			p_mapping->slave_addr = p_child->old.slave_addr;
> -			p_mapping->dvo_wiring = p_child->old.dvo_wiring;
> -			p_mapping->ddc_pin = p_child->old.ddc_pin;
> -			p_mapping->i2c_pin = p_child->old.i2c_pin;
> +			p_mapping->dvo_port = child->dvo_port;
> +			p_mapping->slave_addr = child->slave_addr;
> +			p_mapping->dvo_wiring = child->dvo_wiring;
> +			p_mapping->ddc_pin = child->ddc_pin;
> +			p_mapping->i2c_pin = child->i2c_pin;
>  			p_mapping->initialized = 1;
>  			DRM_DEBUG_KMS("SDVO device: dvo=%x, addr=%x, wiring=%d, ddc_pin=%d, i2c_pin=%d\n",
>  				      p_mapping->dvo_port,
> @@ -469,7 +469,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
>  			DRM_DEBUG_KMS("Maybe one SDVO port is shared by "
>  					 "two SDVO device.\n");
>  		}
> -		if (p_child->old.slave2_addr) {
> +		if (child->slave2_addr) {
>  			/* Maybe this is a SDVO device with multiple inputs */
>  			/* And the mapping info is not added */
>  			DRM_DEBUG_KMS("there exists the slave2_addr. Maybe this"
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH] drm/i915: fix VBT parsing for SDVO child device mapping
  2015-08-18 13:57 ` Ville Syrjälä
@ 2015-08-19  6:54   ` Jani Nikula
  0 siblings, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2015-08-19  6:54 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, 18 Aug 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Aug 18, 2015 at 02:28:55PM +0300, Jani Nikula wrote:
>> commit 75067ddecf21271631bc018d2fb23ddd09b66aae
>> Author: Antti Koskipaa <antti.koskipaa@linux.intel.com>
>> Date:   Fri Jul 10 14:10:55 2015 +0300
>> 
>>     drm/i915: Per-DDI I_boost override
>> 
>> increased size of union child_device_config without taking into account
>> the size check in parse_sdvo_device_mapping(). Switch the function over
>> to using the legacy struct only.
>> 
>> Fixes: 75067ddecf21 ("drm/i915: Per-DDI I_boost override")
>> Cc: Antti Koskipaa <antti.koskipaa@linux.intel.com>
>> Cc: David Weinehall <david.weinehall@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Also gave it a spin on my 946gz so:
> Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pushed to drm-intel-next-fixes, thanks for the review and testing.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/intel_bios.c | 50 +++++++++++++++++++--------------------
>>  1 file changed, 25 insertions(+), 25 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 8e46149bafdd..1b7e1a591a37 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -401,7 +401,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
>>  {
>>  	struct sdvo_device_mapping *p_mapping;
>>  	const struct bdb_general_definitions *p_defs;
>> -	const union child_device_config *p_child;
>> +	const struct old_child_dev_config *child; /* legacy */
>>  	int i, child_device_num, count;
>>  	u16	block_size;
>>  
>> @@ -410,14 +410,14 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
>>  		DRM_DEBUG_KMS("No general definition block is found, unable to construct sdvo mapping.\n");
>>  		return;
>>  	}
>> -	/* judge whether the size of child device meets the requirements.
>> -	 * If the child device size obtained from general definition block
>> -	 * is different with sizeof(struct child_device_config), skip the
>> -	 * parsing of sdvo device info
>> +
>> +	/*
>> +	 * Only parse SDVO mappings when the general definitions block child
>> +	 * device size matches that of the *legacy* child device config
>> +	 * struct. Thus, SDVO mapping will be skipped for newer VBT.
>>  	 */
>> -	if (p_defs->child_dev_size != sizeof(*p_child)) {
>> -		/* different child dev size . Ignore it */
>> -		DRM_DEBUG_KMS("different child size is found. Invalid.\n");
>> +	if (p_defs->child_dev_size != sizeof(*child)) {
>> +		DRM_DEBUG_KMS("Unsupported child device size for SDVO mapping.\n");
>>  		return;
>>  	}
>>  	/* get the block size of general definitions */
>> @@ -427,37 +427,37 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
>>  		p_defs->child_dev_size;
>>  	count = 0;
>>  	for (i = 0; i < child_device_num; i++) {
>> -		p_child = child_device_ptr(p_defs, i);
>> -		if (!p_child->old.device_type) {
>> +		child = &child_device_ptr(p_defs, i)->old;
>> +		if (!child->device_type) {
>>  			/* skip the device block if device type is invalid */
>>  			continue;
>>  		}
>> -		if (p_child->old.slave_addr != SLAVE_ADDR1 &&
>> -			p_child->old.slave_addr != SLAVE_ADDR2) {
>> +		if (child->slave_addr != SLAVE_ADDR1 &&
>> +		    child->slave_addr != SLAVE_ADDR2) {
>>  			/*
>>  			 * If the slave address is neither 0x70 nor 0x72,
>>  			 * it is not a SDVO device. Skip it.
>>  			 */
>>  			continue;
>>  		}
>> -		if (p_child->old.dvo_port != DEVICE_PORT_DVOB &&
>> -			p_child->old.dvo_port != DEVICE_PORT_DVOC) {
>> +		if (child->dvo_port != DEVICE_PORT_DVOB &&
>> +		    child->dvo_port != DEVICE_PORT_DVOC) {
>>  			/* skip the incorrect SDVO port */
>>  			DRM_DEBUG_KMS("Incorrect SDVO port. Skip it\n");
>>  			continue;
>>  		}
>>  		DRM_DEBUG_KMS("the SDVO device with slave addr %2x is found on"
>> -				" %s port\n",
>> -				p_child->old.slave_addr,
>> -				(p_child->old.dvo_port == DEVICE_PORT_DVOB) ?
>> -					"SDVOB" : "SDVOC");
>> -		p_mapping = &(dev_priv->sdvo_mappings[p_child->old.dvo_port - 1]);
>> +			      " %s port\n",
>> +			      child->slave_addr,
>> +			      (child->dvo_port == DEVICE_PORT_DVOB) ?
>> +			      "SDVOB" : "SDVOC");
>> +		p_mapping = &(dev_priv->sdvo_mappings[child->dvo_port - 1]);
>>  		if (!p_mapping->initialized) {
>> -			p_mapping->dvo_port = p_child->old.dvo_port;
>> -			p_mapping->slave_addr = p_child->old.slave_addr;
>> -			p_mapping->dvo_wiring = p_child->old.dvo_wiring;
>> -			p_mapping->ddc_pin = p_child->old.ddc_pin;
>> -			p_mapping->i2c_pin = p_child->old.i2c_pin;
>> +			p_mapping->dvo_port = child->dvo_port;
>> +			p_mapping->slave_addr = child->slave_addr;
>> +			p_mapping->dvo_wiring = child->dvo_wiring;
>> +			p_mapping->ddc_pin = child->ddc_pin;
>> +			p_mapping->i2c_pin = child->i2c_pin;
>>  			p_mapping->initialized = 1;
>>  			DRM_DEBUG_KMS("SDVO device: dvo=%x, addr=%x, wiring=%d, ddc_pin=%d, i2c_pin=%d\n",
>>  				      p_mapping->dvo_port,
>> @@ -469,7 +469,7 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
>>  			DRM_DEBUG_KMS("Maybe one SDVO port is shared by "
>>  					 "two SDVO device.\n");
>>  		}
>> -		if (p_child->old.slave2_addr) {
>> +		if (child->slave2_addr) {
>>  			/* Maybe this is a SDVO device with multiple inputs */
>>  			/* And the mapping info is not added */
>>  			DRM_DEBUG_KMS("there exists the slave2_addr. Maybe this"
>> -- 
>> 2.1.4
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: fix VBT parsing for SDVO child device mapping
  2015-08-18 11:28 [PATCH] drm/i915: fix VBT parsing for SDVO child device mapping Jani Nikula
  2015-08-18 13:57 ` Ville Syrjälä
@ 2015-08-28  3:15 ` shuang.he
  1 sibling, 0 replies; 4+ messages in thread
From: shuang.he @ 2015-08-28  3:15 UTC (permalink / raw)
  To: shuang.he, julianx.dumez, christophe.sureau, lei.a.liu, intel-gfx,
	jani.nikula

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 7227
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                 -2              302/302              300/302
SNB                                  315/315              315/315
IVB                                  336/336              336/336
BYT                 -1              283/283              282/283
HSW                                  378/378              378/378
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@kms_flip@flip-vs-dpms-interruptible      PASS(1)      DMESG_WARN(1)
*ILK  igt@kms_flip@rcs-wf_vblank-vs-dpms-interruptible      PASS(1)      DMESG_WARN(1)
*BYT  igt@gem_partial_pwrite_pread@reads-uncached      PASS(1)      FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2015-08-28  3:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-18 11:28 [PATCH] drm/i915: fix VBT parsing for SDVO child device mapping Jani Nikula
2015-08-18 13:57 ` Ville Syrjälä
2015-08-19  6:54   ` Jani Nikula
2015-08-28  3:15 ` shuang.he

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).