diff mbox series

[01/12] drm/i915: rename display.version to display.ver

Message ID 20210408045227.985408-2-lucas.demarchi@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Extend GEN renames to the rest of the driver | expand

Commit Message

Lucas De Marchi April 8, 2021, 4:52 a.m. UTC
The macro we use to check is called DISPLAY_VER(). While using this
macro and the new ones being added in following changes I made the
mistake multiple times when mixing both "ver" and "version". Although
it's usually better to prefer the complete name, the shorhand
DISPLAY_VER() / GRAPHICS_VER / MEDIA_VER are clear and cause less
visual polution.

Another issue is when copying the variable to other places.
"display.version" would be copied to a "display_version" variable which
is long and would make people abbreviate as "version", or "display_ver".
In the first case it's not always clear what version refers to, and in
the second case it just hints it should be the name in the first place.

So, in the same way use used "gen" rather than "generation", use "ver"
instead of "version".

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          | 2 +-
 drivers/gpu/drm/i915/i915_pci.c          | 4 ++--
 drivers/gpu/drm/i915/intel_device_info.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

Comments

Souza, Jose April 10, 2021, 12:07 a.m. UTC | #1
On Wed, 2021-04-07 at 21:52 -0700, Lucas De Marchi wrote:
> The macro we use to check is called DISPLAY_VER(). While using this
> macro and the new ones being added in following changes I made the
> mistake multiple times when mixing both "ver" and "version". Although
> it's usually better to prefer the complete name, the shorhand
> DISPLAY_VER() / GRAPHICS_VER / MEDIA_VER are clear and cause less
> visual polution.
> 
> Another issue is when copying the variable to other places.
> "display.version" would be copied to a "display_version" variable which
> is long and would make people abbreviate as "version", or "display_ver".
> In the first case it's not always clear what version refers to, and in
> the second case it just hints it should be the name in the first place.
> 
> So, in the same way use used "gen" rather than "generation", use "ver"
> instead of "version".
> 

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          | 2 +-
>  drivers/gpu/drm/i915/i915_pci.c          | 4 ++--
>  drivers/gpu/drm/i915/intel_device_info.h | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 69e43bf91a15..8c62bb2abd31 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1237,7 +1237,7 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
>  #define INTEL_GEN(dev_priv)	(INTEL_INFO(dev_priv)->gen)
>  #define INTEL_DEVID(dev_priv)	(RUNTIME_INFO(dev_priv)->device_id)
>  
> 
> 
> 
> -#define DISPLAY_VER(i915)	(INTEL_INFO(i915)->display.version)
> +#define DISPLAY_VER(i915)	(INTEL_INFO(i915)->display.ver)
>  #define IS_DISPLAY_RANGE(i915, from, until) \
>  	(DISPLAY_VER(i915) >= (from) && DISPLAY_VER(i915) <= (until))
>  #define IS_DISPLAY_VER(i915, v) (DISPLAY_VER(i915) == (v))
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 480553746794..ce5cbeaf036d 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -36,7 +36,7 @@
>  #include "i915_selftest.h"
>  
> 
> 
> 
>  #define PLATFORM(x) .platform = (x)
> -#define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1), .display.version = (x)
> +#define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1), .display.ver = (x)
>  
> 
> 
> 
>  #define I845_PIPE_OFFSETS \
>  	.pipe_offsets = { \
> @@ -723,7 +723,7 @@ static const struct intel_device_info bxt_info = {
>  static const struct intel_device_info glk_info = {
>  	GEN9_LP_FEATURES,
>  	PLATFORM(INTEL_GEMINILAKE),
> -	.display.version = 10,
> +	.display.ver = 10,
>  	.ddb_size = 1024,
>  	GLK_COLORS,
>  };
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 2f442d418a15..b16c75927a12 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -189,7 +189,7 @@ struct intel_device_info {
>  #undef DEFINE_FLAG
>  
> 
> 
> 
>  	struct {
> -		u8 version;
> +		u8 ver;
>  
> 
> 
> 
>  #define DEFINE_FLAG(name) u8 name:1
>  		DEV_INFO_DISPLAY_FOR_EACH_FLAG(DEFINE_FLAG);
Matt Roper April 11, 2021, 3:33 p.m. UTC | #2
On Wed, Apr 07, 2021 at 09:52:16PM -0700, Lucas De Marchi wrote:
> The macro we use to check is called DISPLAY_VER(). While using this
> macro and the new ones being added in following changes I made the
> mistake multiple times when mixing both "ver" and "version". Although
> it's usually better to prefer the complete name, the shorhand
> DISPLAY_VER() / GRAPHICS_VER / MEDIA_VER are clear and cause less
> visual polution.
> 
> Another issue is when copying the variable to other places.
> "display.version" would be copied to a "display_version" variable which
> is long and would make people abbreviate as "version", or "display_ver".
> In the first case it's not always clear what version refers to, and in
> the second case it just hints it should be the name in the first place.
> 
> So, in the same way use used "gen" rather than "generation", use "ver"
> instead of "version".

I'm indifferent on "ver" versus "version" but Jani suggested using the
full word "version" when we were first adding this, so you should
probably get an ack from him.


Matt

> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          | 2 +-
>  drivers/gpu/drm/i915/i915_pci.c          | 4 ++--
>  drivers/gpu/drm/i915/intel_device_info.h | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 69e43bf91a15..8c62bb2abd31 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1237,7 +1237,7 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
>  #define INTEL_GEN(dev_priv)	(INTEL_INFO(dev_priv)->gen)
>  #define INTEL_DEVID(dev_priv)	(RUNTIME_INFO(dev_priv)->device_id)
>  
> -#define DISPLAY_VER(i915)	(INTEL_INFO(i915)->display.version)
> +#define DISPLAY_VER(i915)	(INTEL_INFO(i915)->display.ver)
>  #define IS_DISPLAY_RANGE(i915, from, until) \
>  	(DISPLAY_VER(i915) >= (from) && DISPLAY_VER(i915) <= (until))
>  #define IS_DISPLAY_VER(i915, v) (DISPLAY_VER(i915) == (v))
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 480553746794..ce5cbeaf036d 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -36,7 +36,7 @@
>  #include "i915_selftest.h"
>  
>  #define PLATFORM(x) .platform = (x)
> -#define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1), .display.version = (x)
> +#define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1), .display.ver = (x)
>  
>  #define I845_PIPE_OFFSETS \
>  	.pipe_offsets = { \
> @@ -723,7 +723,7 @@ static const struct intel_device_info bxt_info = {
>  static const struct intel_device_info glk_info = {
>  	GEN9_LP_FEATURES,
>  	PLATFORM(INTEL_GEMINILAKE),
> -	.display.version = 10,
> +	.display.ver = 10,
>  	.ddb_size = 1024,
>  	GLK_COLORS,
>  };
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 2f442d418a15..b16c75927a12 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -189,7 +189,7 @@ struct intel_device_info {
>  #undef DEFINE_FLAG
>  
>  	struct {
> -		u8 version;
> +		u8 ver;
>  
>  #define DEFINE_FLAG(name) u8 name:1
>  		DEV_INFO_DISPLAY_FOR_EACH_FLAG(DEFINE_FLAG);
> -- 
> 2.31.1
>
Jani Nikula April 12, 2021, 11:04 a.m. UTC | #3
On Sun, 11 Apr 2021, Matt Roper <matthew.d.roper@intel.com> wrote:
> On Wed, Apr 07, 2021 at 09:52:16PM -0700, Lucas De Marchi wrote:
>> The macro we use to check is called DISPLAY_VER(). While using this
>> macro and the new ones being added in following changes I made the
>> mistake multiple times when mixing both "ver" and "version". Although
>> it's usually better to prefer the complete name, the shorhand
>> DISPLAY_VER() / GRAPHICS_VER / MEDIA_VER are clear and cause less
>> visual polution.
>> 
>> Another issue is when copying the variable to other places.
>> "display.version" would be copied to a "display_version" variable which
>> is long and would make people abbreviate as "version", or "display_ver".
>> In the first case it's not always clear what version refers to, and in
>> the second case it just hints it should be the name in the first place.
>> 
>> So, in the same way use used "gen" rather than "generation", use "ver"
>> instead of "version".
>
> I'm indifferent on "ver" versus "version" but Jani suggested using the
> full word "version" when we were first adding this, so you should
> probably get an ack from him.

For display version there were precious few places where we actually
needed to refer to it, so I thought the longer version was better for
clarity.

I realize graphics_ver is used much more (although not necessarily in
reference to intel_device_info) and it may get tedious to use the longer
version. And it's better to have all of them unified.

Acked-by: Jani Nikula <jani.nikula@intel.com>

>
>
> Matt
>
>> 
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h          | 2 +-
>>  drivers/gpu/drm/i915/i915_pci.c          | 4 ++--
>>  drivers/gpu/drm/i915/intel_device_info.h | 2 +-
>>  3 files changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 69e43bf91a15..8c62bb2abd31 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1237,7 +1237,7 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
>>  #define INTEL_GEN(dev_priv)	(INTEL_INFO(dev_priv)->gen)
>>  #define INTEL_DEVID(dev_priv)	(RUNTIME_INFO(dev_priv)->device_id)
>>  
>> -#define DISPLAY_VER(i915)	(INTEL_INFO(i915)->display.version)
>> +#define DISPLAY_VER(i915)	(INTEL_INFO(i915)->display.ver)
>>  #define IS_DISPLAY_RANGE(i915, from, until) \
>>  	(DISPLAY_VER(i915) >= (from) && DISPLAY_VER(i915) <= (until))
>>  #define IS_DISPLAY_VER(i915, v) (DISPLAY_VER(i915) == (v))
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> index 480553746794..ce5cbeaf036d 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -36,7 +36,7 @@
>>  #include "i915_selftest.h"
>>  
>>  #define PLATFORM(x) .platform = (x)
>> -#define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1), .display.version = (x)
>> +#define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1), .display.ver = (x)
>>  
>>  #define I845_PIPE_OFFSETS \
>>  	.pipe_offsets = { \
>> @@ -723,7 +723,7 @@ static const struct intel_device_info bxt_info = {
>>  static const struct intel_device_info glk_info = {
>>  	GEN9_LP_FEATURES,
>>  	PLATFORM(INTEL_GEMINILAKE),
>> -	.display.version = 10,
>> +	.display.ver = 10,
>>  	.ddb_size = 1024,
>>  	GLK_COLORS,
>>  };
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
>> index 2f442d418a15..b16c75927a12 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.h
>> +++ b/drivers/gpu/drm/i915/intel_device_info.h
>> @@ -189,7 +189,7 @@ struct intel_device_info {
>>  #undef DEFINE_FLAG
>>  
>>  	struct {
>> -		u8 version;
>> +		u8 ver;
>>  
>>  #define DEFINE_FLAG(name) u8 name:1
>>  		DEV_INFO_DISPLAY_FOR_EACH_FLAG(DEFINE_FLAG);
>> -- 
>> 2.31.1
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 69e43bf91a15..8c62bb2abd31 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1237,7 +1237,7 @@  static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
 #define INTEL_GEN(dev_priv)	(INTEL_INFO(dev_priv)->gen)
 #define INTEL_DEVID(dev_priv)	(RUNTIME_INFO(dev_priv)->device_id)
 
-#define DISPLAY_VER(i915)	(INTEL_INFO(i915)->display.version)
+#define DISPLAY_VER(i915)	(INTEL_INFO(i915)->display.ver)
 #define IS_DISPLAY_RANGE(i915, from, until) \
 	(DISPLAY_VER(i915) >= (from) && DISPLAY_VER(i915) <= (until))
 #define IS_DISPLAY_VER(i915, v) (DISPLAY_VER(i915) == (v))
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 480553746794..ce5cbeaf036d 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -36,7 +36,7 @@ 
 #include "i915_selftest.h"
 
 #define PLATFORM(x) .platform = (x)
-#define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1), .display.version = (x)
+#define GEN(x) .gen = (x), .gen_mask = BIT((x) - 1), .display.ver = (x)
 
 #define I845_PIPE_OFFSETS \
 	.pipe_offsets = { \
@@ -723,7 +723,7 @@  static const struct intel_device_info bxt_info = {
 static const struct intel_device_info glk_info = {
 	GEN9_LP_FEATURES,
 	PLATFORM(INTEL_GEMINILAKE),
-	.display.version = 10,
+	.display.ver = 10,
 	.ddb_size = 1024,
 	GLK_COLORS,
 };
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 2f442d418a15..b16c75927a12 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -189,7 +189,7 @@  struct intel_device_info {
 #undef DEFINE_FLAG
 
 	struct {
-		u8 version;
+		u8 ver;
 
 #define DEFINE_FLAG(name) u8 name:1
 		DEV_INFO_DISPLAY_FOR_EACH_FLAG(DEFINE_FLAG);