diff mbox series

[2/2] drm/i915: Save some lines of source code in workarounds

Message ID 20190104114053.15706-2-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915: Move workaround infrastructure code up | expand

Commit Message

Tvrtko Ursulin Jan. 4, 2019, 11:40 a.m. UTC 
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

No functional or code size change - just notice we can compact the source
by re-using a single helper for adding workarounds.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_workarounds.c | 32 +++++-------------------
 1 file changed, 6 insertions(+), 26 deletions(-)

Comments

Chris Wilson Jan. 4, 2019, 12:01 p.m. UTC | #1 
Quoting Tvrtko Ursulin (2019-01-04 11:40:53)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> No functional or code size change - just notice we can compact the source
> by re-using a single helper for adding workarounds.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_workarounds.c | 32 +++++-------------------
>  1 file changed, 6 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> index ffc96c8b849b..a8161324108d 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -142,7 +142,8 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa)
>  }
>  
>  static void
> -__wa_add(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, u32 val)
> +wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
> +                  u32 val)

This looked odd, since I was thinking that __wa_add() remained the
better name for adding the actual i915_wa_list, but __wa_add() is just
perplexingly the wrapper for _wa_add()

For both,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Tvrtko Ursulin Jan. 4, 2019, 1:21 p.m. UTC | #2 
On 04/01/2019 12:01, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-01-04 11:40:53)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> No functional or code size change - just notice we can compact the source
>> by re-using a single helper for adding workarounds.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_workarounds.c | 32 +++++-------------------
>>   1 file changed, 6 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
>> index ffc96c8b849b..a8161324108d 100644
>> --- a/drivers/gpu/drm/i915/intel_workarounds.c
>> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
>> @@ -142,7 +142,8 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa)
>>   }
>>   
>>   static void
>> -__wa_add(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, u32 val)
>> +wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
>> +                  u32 val)
> 
> This looked odd, since I was thinking that __wa_add() remained the
> better name for adding the actual i915_wa_list, but __wa_add() is just
> perplexingly the wrapper for _wa_add()
> 
> For both,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

I am not too proud with my used of single and double underscores here. 
:I And I was also thinking about why not just keep __wa_add as the 
common adder. Even had a version with _wa_add renamed to __wa_add, and 
then _wa_add etc. Maybe I need to have another go at it.

Regards,

Tvrtko
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index ffc96c8b849b..a8161324108d 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -142,7 +142,8 @@  static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa)
 }
 
 static void
-__wa_add(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, u32 val)
+wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
+		   u32 val)
 {
 	struct i915_wa wa = {
 		.reg = reg,
@@ -156,26 +157,7 @@  __wa_add(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, u32 val)
 static void
 wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
 {
-	struct i915_wa wa = {
-		.reg = reg,
-		.mask = val,
-		.val = _MASKED_BIT_ENABLE(val)
-	};
-
-	_wa_add(wal, &wa);
-}
-
-static void
-wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
-		   u32 val)
-{
-	struct i915_wa wa = {
-		.reg = reg,
-		.mask = mask,
-		.val = val
-	};
-
-	_wa_add(wal, &wa);
+	wa_write_masked_or(wal, reg, val, _MASKED_BIT_ENABLE(val));
 }
 
 static void
@@ -190,16 +172,14 @@  wa_write_or(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
 	wa_write_masked_or(wal, reg, val, val);
 }
 
-#define WA_REG(addr, mask, val) __wa_add(wal, (addr), (mask), (val))
-
 #define WA_SET_BIT_MASKED(addr, mask) \
-	WA_REG(addr, (mask), _MASKED_BIT_ENABLE(mask))
+	wa_write_masked_or(wal, (addr), (mask), _MASKED_BIT_ENABLE(mask))
 
 #define WA_CLR_BIT_MASKED(addr, mask) \
-	WA_REG(addr, (mask), _MASKED_BIT_DISABLE(mask))
+	wa_write_masked_or(wal, (addr), (mask), _MASKED_BIT_DISABLE(mask))
 
 #define WA_SET_FIELD_MASKED(addr, mask, value) \
-	WA_REG(addr, (mask), _MASKED_FIELD(mask, value))
+	wa_write_masked_or(wal, (addr), (mask), _MASKED_FIELD((mask), (value)))
 
 static void gen8_ctx_workarounds_init(struct intel_engine_cs *engine)
 {