Re: [Intel-gfx] [PATCH 01/12] drm/i915: rename display.version to display.ver

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Matt Roper <matthew.d.roper@intel.com>,
	Lucas De Marchi <lucas.demarchi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 01/12] drm/i915: rename display.version to display.ver
Date: Mon, 12 Apr 2021 14:04:58 +0300	[thread overview]
Message-ID: <87fszvkbl1.fsf@intel.com> (raw)
In-Reply-To: <20210411153358.GA1637800@mdroper-desk1.amr.corp.intel.com>

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
>> 

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