diff mbox

[1/5] drm/displayid: Enhance version reporting

Message ID 1462307812-3341-2-git-send-email-airlied@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Airlie May 3, 2016, 8:36 p.m. UTC 
From: Tomas Bzatek <tomas@bzatek.net>

Cosmetic change, let's report more precise revisions and IDs.

https://bugs.freedesktop.org/show_bug.cgi?id=95207

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_edid.c  | 6 +++---
 include/drm/drm_displayid.h | 6 ++++--
 2 files changed, 7 insertions(+), 5 deletions(-)

Comments

Ville Syrjälä May 4, 2016, 9:10 a.m. UTC | #1 
On Wed, May 04, 2016 at 06:36:48AM +1000, Dave Airlie wrote:
> From: Tomas Bzatek <tomas@bzatek.net>
> 
> Cosmetic change, let's report more precise revisions and IDs.
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=95207
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/drm_edid.c  | 6 +++---
>  include/drm/drm_displayid.h | 6 ++++--
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 9a9be9a..c8a3a55 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4168,8 +4168,8 @@ static int drm_parse_display_id(struct drm_connector *connector,
>  
>  	base = (struct displayid_hdr *)&displayid[idx];
>  
> -	DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
> -		      base->rev, base->bytes, base->prod_id, base->ext_count);
> +	DRM_DEBUG_KMS("base revision v%d.%d, edid length %d, bytes %d, prod_id %d ext_count %d\n",
> +		      base->ver, base->rev, length, base->bytes, base->prod_id, base->ext_count);
>  
>  	if (base->bytes + 5 > length - idx)
>  		return -EINVAL;
> @@ -4183,7 +4183,7 @@ static int drm_parse_display_id(struct drm_connector *connector,
>  	}
>  
>  	block = (struct displayid_block *)&displayid[idx + 4];
> -	DRM_DEBUG_KMS("block id %d, rev %d, len %d\n",
> +	DRM_DEBUG_KMS("block id 0x%x, rev %d, len %d\n",
>  		      block->tag, block->rev, block->num_bytes);
>  
>  	switch (block->tag) {
> diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h
> index 623b4e9..042f9fc 100644
> --- a/include/drm/drm_displayid.h
> +++ b/include/drm/drm_displayid.h
> @@ -52,7 +52,8 @@
>  #define PRODUCT_TYPE_DIRECT_DRIVE 6
>  
>  struct displayid_hdr {
> -	u8 rev;
> +	u8 rev:4;
> +	u8 ver:4;
>  	u8 bytes;
>  	u8 prod_id;
>  	u8 ext_count;
> @@ -60,7 +61,8 @@ struct displayid_hdr {
>  
>  struct displayid_block {
>  	u8 tag;
> -	u8 rev;
> +	u8 rev:3;
> +	u8 reserved:5;
>  	u8 num_bytes;
>  } __packed;

Using bitfields in an architecture independent structure doesn't
feel like an entirely good idea to me.
Michel Dänzer May 9, 2016, 9:52 a.m. UTC | #2 
On 04.05.2016 18:10, Ville Syrjälä wrote:
> On Wed, May 04, 2016 at 06:36:48AM +1000, Dave Airlie wrote:
>> From: Tomas Bzatek <tomas@bzatek.net>
>>
>> Cosmetic change, let's report more precise revisions and IDs.
>>
>> https://bugs.freedesktop.org/show_bug.cgi?id=95207
>>
>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>> ---
>>  drivers/gpu/drm/drm_edid.c  | 6 +++---
>>  include/drm/drm_displayid.h | 6 ++++--
>>  2 files changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 9a9be9a..c8a3a55 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -4168,8 +4168,8 @@ static int drm_parse_display_id(struct drm_connector *connector,
>>  
>>  	base = (struct displayid_hdr *)&displayid[idx];
>>  
>> -	DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
>> -		      base->rev, base->bytes, base->prod_id, base->ext_count);
>> +	DRM_DEBUG_KMS("base revision v%d.%d, edid length %d, bytes %d, prod_id %d ext_count %d\n",
>> +		      base->ver, base->rev, length, base->bytes, base->prod_id, base->ext_count);
>>  
>>  	if (base->bytes + 5 > length - idx)
>>  		return -EINVAL;
>> @@ -4183,7 +4183,7 @@ static int drm_parse_display_id(struct drm_connector *connector,
>>  	}
>>  
>>  	block = (struct displayid_block *)&displayid[idx + 4];
>> -	DRM_DEBUG_KMS("block id %d, rev %d, len %d\n",
>> +	DRM_DEBUG_KMS("block id 0x%x, rev %d, len %d\n",
>>  		      block->tag, block->rev, block->num_bytes);
>>  
>>  	switch (block->tag) {
>> diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h
>> index 623b4e9..042f9fc 100644
>> --- a/include/drm/drm_displayid.h
>> +++ b/include/drm/drm_displayid.h
>> @@ -52,7 +52,8 @@
>>  #define PRODUCT_TYPE_DIRECT_DRIVE 6
>>  
>>  struct displayid_hdr {
>> -	u8 rev;
>> +	u8 rev:4;
>> +	u8 ver:4;
>>  	u8 bytes;
>>  	u8 prod_id;
>>  	u8 ext_count;
>> @@ -60,7 +61,8 @@ struct displayid_hdr {
>>  
>>  struct displayid_block {
>>  	u8 tag;
>> -	u8 rev;
>> +	u8 rev:3;
>> +	u8 reserved:5;
>>  	u8 num_bytes;
>>  } __packed;
> 
> Using bitfields in an architecture independent structure doesn't
> feel like an entirely good idea to me.

Yeah, this won't work as expected on some architectures.
Dave Airlie May 10, 2016, 1:16 a.m. UTC | #3 
>>> index 623b4e9..042f9fc 100644
>>> --- a/include/drm/drm_displayid.h
>>> +++ b/include/drm/drm_displayid.h
>>> @@ -52,7 +52,8 @@
>>>  #define PRODUCT_TYPE_DIRECT_DRIVE 6
>>>
>>>  struct displayid_hdr {
>>> -    u8 rev;
>>> +    u8 rev:4;
>>> +    u8 ver:4;
>>>      u8 bytes;
>>>      u8 prod_id;
>>>      u8 ext_count;
>>> @@ -60,7 +61,8 @@ struct displayid_hdr {
>>>
>>>  struct displayid_block {
>>>      u8 tag;
>>> -    u8 rev;
>>> +    u8 rev:3;
>>> +    u8 reserved:5;
>>>      u8 num_bytes;
>>>  } __packed;
>>
>> Using bitfields in an architecture independent structure doesn't
>> feel like an entirely good idea to me.
>
> Yeah, this won't work as expected on some architectures.
>
Indeed, I'll drop this for now.

Dave.
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9a9be9a..c8a3a55 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4168,8 +4168,8 @@  static int drm_parse_display_id(struct drm_connector *connector,
 
 	base = (struct displayid_hdr *)&displayid[idx];
 
-	DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
-		      base->rev, base->bytes, base->prod_id, base->ext_count);
+	DRM_DEBUG_KMS("base revision v%d.%d, edid length %d, bytes %d, prod_id %d ext_count %d\n",
+		      base->ver, base->rev, length, base->bytes, base->prod_id, base->ext_count);
 
 	if (base->bytes + 5 > length - idx)
 		return -EINVAL;
@@ -4183,7 +4183,7 @@  static int drm_parse_display_id(struct drm_connector *connector,
 	}
 
 	block = (struct displayid_block *)&displayid[idx + 4];
-	DRM_DEBUG_KMS("block id %d, rev %d, len %d\n",
+	DRM_DEBUG_KMS("block id 0x%x, rev %d, len %d\n",
 		      block->tag, block->rev, block->num_bytes);
 
 	switch (block->tag) {
diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h
index 623b4e9..042f9fc 100644
--- a/include/drm/drm_displayid.h
+++ b/include/drm/drm_displayid.h
@@ -52,7 +52,8 @@ 
 #define PRODUCT_TYPE_DIRECT_DRIVE 6
 
 struct displayid_hdr {
-	u8 rev;
+	u8 rev:4;
+	u8 ver:4;
 	u8 bytes;
 	u8 prod_id;
 	u8 ext_count;
@@ -60,7 +61,8 @@  struct displayid_hdr {
 
 struct displayid_block {
 	u8 tag;
-	u8 rev;
+	u8 rev:3;
+	u8 reserved:5;
 	u8 num_bytes;
 } __packed;