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