Re: [Intel-gfx] [CI 1/3] drm/i915/gt: Skip rmw for masked registers

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> A masked register does not need rmw to update, and it is best not to use
> such a sequence.
>
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 32 ++++++++++++++-------
>  1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 5a7db279f702..e4c2b6d42f46 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -116,7 +116,8 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa)
>  		} else {
>  			wa_ = &wal->list[mid];
>  
> -			if ((wa->mask & ~wa_->mask) == 0) {
> +			if ((wa->mask | wa_->mask) &&

Don't we want to discard if someone tries to demote a masked
one into a plain?

-Mika

> +			    (wa->mask & ~wa_->mask) == 0) {
>  				DRM_ERROR("Discarding overwritten w/a for reg %04x (mask: %08x, value: %08x)\n",
>  					  i915_mmio_reg_offset(wa_->reg),
>  					  wa_->mask, wa_->val);
> @@ -167,12 +168,6 @@ wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask,
>  	wa_add(wal, reg, mask, val, mask);
>  }
>  
> -static void
> -wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
> -{
> -	wa_write_masked_or(wal, reg, val, _MASKED_BIT_ENABLE(val));
> -}
> -
>  static void
>  wa_write(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
>  {
> @@ -185,14 +180,26 @@ wa_write_or(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
>  	wa_write_masked_or(wal, reg, val, val);
>  }
>  
> +static void
> +wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
> +{
> +	wa_add(wal, reg, 0, _MASKED_BIT_ENABLE(val), val);
> +}
> +
> +static void
> +wa_masked_dis(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
> +{
> +	wa_add(wal, reg, 0, _MASKED_BIT_DISABLE(val), val);
> +}
> +
>  #define WA_SET_BIT_MASKED(addr, mask) \
> -	wa_write_masked_or(wal, (addr), (mask), _MASKED_BIT_ENABLE(mask))
> +	wa_masked_en(wal, (addr), mask)
>  
>  #define WA_CLR_BIT_MASKED(addr, mask) \
> -	wa_write_masked_or(wal, (addr), (mask), _MASKED_BIT_DISABLE(mask))
> +	wa_masked_dis(wal, (addr), mask)
>  
>  #define WA_SET_FIELD_MASKED(addr, mask, value) \
> -	wa_write_masked_or(wal, (addr), (mask), _MASKED_FIELD((mask), (value)))
> +	wa_write_masked_or(wal, (addr), 0, _MASKED_FIELD((mask), (value)))
>  
>  static void gen8_ctx_workarounds_init(struct intel_engine_cs *engine,
>  				      struct i915_wa_list *wal)
> @@ -1020,7 +1027,10 @@ wa_list_apply(struct intel_uncore *uncore, const struct i915_wa_list *wal)
>  	intel_uncore_forcewake_get__locked(uncore, fw);
>  
>  	for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
> -		intel_uncore_rmw_fw(uncore, wa->reg, wa->mask, wa->val);
> +		if (wa->mask)
> +			intel_uncore_rmw_fw(uncore, wa->reg, wa->mask, wa->val);
> +		else
> +			intel_uncore_write_fw(uncore, wa->reg, wa->val);
>  		if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
>  			wa_verify(wa,
>  				  intel_uncore_read_fw(uncore, wa->reg),
> -- 
> 2.25.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx