[Intel-gfx] [PATCH 1/7] drm/i915: Record GT workarounds in a list

Chris Wilson chris at chris-wilson.co.uk
Fri Nov 30 21:51:50 UTC 2018


Quoting Tvrtko Ursulin (2018-11-30 17:44:06)
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> To enable later verification of GT workaround state at various stages of
> driver lifetime, we record the list of applicable ones per platforms to a
> list, from which they are also applied.
> 
> The added data structure is a simple array of register, mask and value
> items, which is allocated on demand as workarounds are added to the list.
> 
> This is a temporary implementation which later in the series gets fused
> with the existing per context workaround list handling. It is separated at
> this stage since the following patch fixes a bug which needs to be as easy
> to backport as possible.
> 
> Also, since in the following patch we will be adding a new class of
> workarounds (per engine) which can be applied from interrupt context, we
> straight away make the provision for safe read-modify-write cycle.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c          |   1 +
>  drivers/gpu/drm/i915/i915_drv.h          |   2 +
>  drivers/gpu/drm/i915/i915_gem.c          |   2 +
>  drivers/gpu/drm/i915/intel_workarounds.c | 443 +++++++++++++++--------
>  drivers/gpu/drm/i915/intel_workarounds.h |  22 ++
>  5 files changed, 327 insertions(+), 143 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e39016713464..2f3dc1cf83a6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1453,6 +1453,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>  
>         intel_uncore_sanitize(dev_priv);
>  
> +       intel_gt_workarounds_init(dev_priv);
>         i915_gem_load_init_fences(dev_priv);
>  
>         /* On the 945G/GM, the chipset reports the MSI capability on the
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 43ac6873a2bb..9ddbcc1f3554 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -69,6 +69,7 @@
>  #include "intel_ringbuffer.h"
>  #include "intel_uncore.h"
>  #include "intel_wopcm.h"
> +#include "intel_workarounds.h"
>  #include "intel_uc.h"
>  
>  #include "i915_gem.h"
> @@ -1652,6 +1653,7 @@ struct drm_i915_private {
>         int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
>  
>         struct i915_workarounds workarounds;
> +       struct i915_wa_list gt_wa_list;
>  
>         struct i915_frontbuffer_tracking fb_tracking;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c55b1f75c980..18adb3dd1fcd 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5706,6 +5706,8 @@ void i915_gem_fini(struct drm_i915_private *dev_priv)
>         i915_gem_contexts_fini(dev_priv);
>         mutex_unlock(&dev_priv->drm.struct_mutex);
>  
> +       intel_wa_list_free(&dev_priv->gt_wa_list);
> +
>         intel_cleanup_gt_powersave(dev_priv);
>  
>         intel_uc_fini_misc(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> index e5cd6c6c66c3..ff20ebf9e040 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -48,6 +48,18 @@
>   * - Public functions to init or apply the given workaround type.
>   */
>  
> +static void wa_init_start(struct i915_wa_list *wal, const char *name)
> +{
> +       wal->name = name;
> +}
> +
> +static void wa_init_finish(struct i915_wa_list *wal)
> +{
> +       if (wal->count)
> +               DRM_DEBUG_DRIVER("Initialized %u %s workarounds\n",
> +                                wal->count, wal->name);
> +}
> +
>  static void wa_add(struct drm_i915_private *i915,
>                    i915_reg_t reg, const u32 mask, const u32 val)
>  {
> @@ -575,28 +587,88 @@ int intel_ctx_workarounds_emit(struct i915_request *rq)
>         return 0;
>  }
>  
> -static void bdw_gt_workarounds_apply(struct drm_i915_private *dev_priv)
> +static void
> +wal_add(struct i915_wa_list *wal, const struct i915_wa *wa)
> +{
> +       const unsigned int grow = 4;
> +
> +       if (wal->__size == wal->count) {
> +               struct i915_wa *list;
> +
> +               list = kcalloc(wal->__size + grow, sizeof(*wa), GFP_KERNEL);
> +               if (!list) {
> +                       DRM_ERROR("No space for workaround init!\n");
> +                       return;
> +               }
> +
> +               if (wal->list)
> +                       memcpy(list, wal->list, sizeof(*wa) * wal->count);
> +
> +               wal->list = list;
> +               wal->__size += grow;
> +       }
> +
> +       memcpy(&wal->list[wal->count], wa, sizeof(*wa));
> +       wal->count++;
> +}
> +
> +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)
> +       };
> +
> +       wal_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
> +       };
> +
> +       wal_add(wal, &wa);
>  }
>  
> -static void chv_gt_workarounds_apply(struct drm_i915_private *dev_priv)
> +static void
> +wa_write(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
>  {
> +       wa_write_masked_or(wal, reg, ~0, val);
>  }
>  
> -static void gen9_gt_workarounds_apply(struct drm_i915_private *dev_priv)
> +static void
> +wa_write_or(struct i915_wa_list *wal, i915_reg_t reg, u32 val)
>  {
> +       wa_write_masked_or(wal, reg, val, val);
> +}
> +
> +static void gen9_gt_workarounds_init(struct drm_i915_private *dev_priv)

Sneaky observation is that we now don't need dev_priv throughout the
init path, so a bonus s/dev_priv/i915/.

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris


More information about the Intel-gfx mailing list