[Intel-gfx] [PATCH v2 12/14] drm/i915: Define multicast registers as a new type

[Jani Nikula]

On Tue, 04 Oct 2022, Jani Nikula <jani.nikula at linux.intel.com> wrote:
> On Fri, 30 Sep 2022, Matt Roper <matthew.d.roper at intel.com> wrote:
>> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
>> index 8f486f77609f..e823869b9afd 100644
>> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
>> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
>> @@ -104,22 +104,16 @@ typedef struct {
>>  
>>  #define _MMIO(r) ((const i915_reg_t){ .reg = (r) })
>>  
>> -#define INVALID_MMIO_REG _MMIO(0)
>> -
>> -static __always_inline u32 i915_mmio_reg_offset(i915_reg_t reg)
>> -{
>> -	return reg.reg;
>> -}
>> +typedef struct {
>> +	u32 reg;
>> +} i915_mcr_reg_t;
>>  
>> -static inline bool i915_mmio_reg_equal(i915_reg_t a, i915_reg_t b)
>> -{
>> -	return i915_mmio_reg_offset(a) == i915_mmio_reg_offset(b);
>> -}
>> +#define INVALID_MMIO_REG _MMIO(0)
>>  
>> -static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>> -{
>> -	return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG);
>> -}
>> +/* These macros can be used on either i915_reg_t or i915_mcr_reg_t */
>> +#define i915_mmio_reg_offset(r) (r.reg)
>> +#define i915_mmio_reg_equal(a, b) (i915_mmio_reg_offset(a) == i915_mmio_reg_offset(b))
>> +#define i915_mmio_reg_valid(r) (!i915_mmio_reg_equal(r, INVALID_MMIO_REG))
>>  
>
> I don't really like losing the type safety here. The whole and only
> purpose of typedeffing i915_reg_t as a struct instead of just u32 was
> the strict type safety.

PS. Changes like this should really be highlighted better, in the commit
subject and title. Now it feels like it's hidden within a big commit
within a big series, without even mentioning it in the commit message.


BR,
Jani.


>
> BR,
> Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center