Restore workarounds after engine reset and unify their handling
Subsystems (2)
v3 cover Restore workarounds after engine reset and unify their handling
From: Tvrtko Ursulin <[email protected]>
First two patches in this series fix losing of workarounds after engine reset
(https://bugzilla.freedesktop.org/show_bug.cgi?id=107945) which started happening after 59b449d5c82a ("drm/i915: Split out functions for different kinds of workarounds").
But since it was discovered to be unsafe to simply re-apply all of them, against a possibly active GPU, and potentially from IRQ context, the approach taken was to split GT workarounds and per-engine workarounds. Latter so far contain the ones living in the 0x2xxx and 0xbxxx range, which were empirically shown to be lost after RCS reset.
This way only a smaller set of affected workarounds can be applied after engine resetm, which is done with irq safe read-modify-write cycle.
The series is structured like this so first two patches are as standalone as possible so it is easy (easier) to backport them. The rest of the series cleans up the whole workaround handling by moving all four classes of them to a common framework.
v2:
* One patch less due removing verification after engine reset.
* See patch change logs.
v3:
* Further review comments as per individual change logs.
Tvrtko Ursulin (7):
drm/i915: Record GT workarounds in a list
drm/i915: Introduce per-engine workarounds
drm/i915: Verify GT workaround state after GPU init drm/i915/selftests: Add tests for GT and engine workaround
verification
drm/i915: Move register white-listing to the common workaround
framework
drm/i915: Fuse per-context workaround handling with the common
framework
drm/i915: Trim unused workaround list entries
drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_debugfs.c | 12 +- drivers/gpu/drm/i915/i915_drv.c | 2 + drivers/gpu/drm/i915/i915_drv.h | 17 +- drivers/gpu/drm/i915/i915_gem.c | 7 +- drivers/gpu/drm/i915/i915_gem_context.c | 6 +- drivers/gpu/drm/i915/intel_engine_cs.c | 4 + drivers/gpu/drm/i915/intel_lrc.c | 11 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.h | 4 + drivers/gpu/drm/i915/intel_workarounds.c | 987 +++++++++++------- drivers/gpu/drm/i915/intel_workarounds.h | 36 +- drivers/gpu/drm/i915/selftests/igt_reset.c | 44 + drivers/gpu/drm/i915/selftests/igt_reset.h | 15 + .../gpu/drm/i915/selftests/intel_hangcheck.c | 51 +- .../drm/i915/selftests/intel_workarounds.c | 182 +++- 16 files changed, 882 insertions(+), 499 deletions(-) create mode 100644 drivers/gpu/drm/i915/selftests/igt_reset.c create mode 100644 drivers/gpu/drm/i915/selftests/igt_reset.h
Signature · 6 lines
2.19.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v3 1/7 drm/i915: Record GT workarounds in a list
From: Tvrtko Ursulin <[email protected]>
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.
v2:
* Change dev_priv to i915 along the init path. (Chris Wilson)
* API rename. (Chris Wilson)
v3:
* Remove explicit list size tracking in favour of growing the allocation in power of two chunks. (Chris Wilson)
--- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_gem.c | 4 +- drivers/gpu/drm/i915/intel_workarounds.c | 491 +++++++++++++++-------- drivers/gpu/drm/i915/intel_workarounds.h | 23 +- 5 files changed, 353 insertions(+), 168 deletions(-)
drivers/gpu/drm/i915/i915_drv.c +1 -0
@@ -1453,6 +1453,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)intel_uncore_sanitize (dev_priv); +intel_gt_init_workarounds (dev_priv);i915_gem_load_init_fences (dev_priv);/* On the 945G/GM, the chipset reports the MSI capability on the
drivers/gpu/drm/i915/i915_drv.h +2 -0
@@ -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;
drivers/gpu/drm/i915/i915_gem.c +3 -1
@@ -5334,7 +5334,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)I915_WRITE (MI_PREDICATE_RESULT_2 ,IS_HSW_GT3 (dev_priv) ?LOWER_SLICE_ENABLED :LOWER_SLICE_DISABLED ); -intel_gt_workarounds_apply (dev_priv); +intel_gt_apply_workarounds (dev_priv);i915_gem_init_swizzling (dev_priv);
@@ -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);
drivers/gpu/drm/i915/intel_workarounds.c +325 -166
655 lines not on this page · raw message
drivers/gpu/drm/i915/intel_workarounds.h +22 -1
@@ -7,10 +7,31 @@#ifndef _I915_WORKAROUNDS_H_#define _I915_WORKAROUNDS_H_ +#include <linux/slab.h> + +struct i915_wa { +i915_reg_t reg; +u32 mask; +u32 val; +}; + +struct i915_wa_list { +const char *name; +unsigned int count; +struct i915_wa *list; +}; + +static inline void intel_wa_list_free (struct i915_wa_list *wal) +{ +kfree (wal->list); +memset (wal,0 ,sizeof (*wal)); +} +int intel_ctx_workarounds_init (struct drm_i915_private *dev_priv);int intel_ctx_workarounds_emit (struct i915_request *rq); -void intel_gt_workarounds_apply (struct drm_i915_private *dev_priv); +void intel_gt_init_workarounds (struct drm_i915_private *dev_priv); +void intel_gt_apply_workarounds (struct drm_i915_private *dev_priv);void intel_whitelist_workarounds_apply (struct intel_engine_cs *engine);
Signature · 6 lines
2.19.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 1/7] drm/i915: Record GT workarounds in a list
Quoting Tvrtko Ursulin (2018-12-03 11:46:11)
Quoted 98 lines
From: Tvrtko Ursulin <[email protected]>
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.
v2:
* Changedev_privto i915 along the init path. (Chris Wilson)
* API rename. (Chris Wilson)v3:
* Remove explicit list size tracking in favour of growing the allocation in power of two chunks. (Chris Wilson)Signed-off-by Tvrtko Ursulin <[email protected]>Reviewed-by Chris Wilson <[email protected]> # v2--- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_gem.c | 4 +- drivers/gpu/drm/i915/intel_workarounds.c | 491 +++++++++++++++-------- drivers/gpu/drm/i915/intel_workarounds.h | 23 +- 5 files changed, 353 insertions(+), 168 deletions(-)drivers/gpu/drm/i915/i915_drv.c +1
-0@@ -1453,6 +1453,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)intel_uncore_sanitize (dev_priv); +intel_gt_init_workarounds (dev_priv);i915_gem_load_init_fences (dev_priv);/* On the 945G/GM, the chipset reports the MSI capability on the drivers/gpu/drm/i915/i915_drv.h +2
-0@@ -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;drivers/gpu/drm/i915/i915_gem.c +3
-1@@ -5334,7 +5334,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)I915_WRITE (MI_PREDICATE_RESULT_2 ,IS_HSW_GT3 (dev_priv) ?LOWER_SLICE_ENABLED :LOWER_SLICE_DISABLED ); -intel_gt_workarounds_apply (dev_priv); +intel_gt_apply_workarounds (dev_priv);i915_gem_init_swizzling (dev_priv);@@ -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);drivers/gpu/drm/i915/intel_workarounds.c +0
-0@@ -48,6 +48,18 @@* - Public functions to init or apply the given workaround type. */
drivers/gpu/drm/i915/intel_workarounds.c line 60
Quoted 10 lines
drivers/gpu/drm/i915/intel_workarounds.c +10
-0+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);
Does this become if (!wal->count) return; later?
Quoted 8 lines
drivers/gpu/drm/i915/intel_workarounds.c +2
-0+} +static void wa_add (struct drm_i915_private *i915,i915_reg_t reg,const u32 mask,const u32 val) {@@ -575,160 +587,240 @@ int intel_ctx_workarounds_emit(struct i915_request *rq)return 0 ; }
drivers/gpu/drm/i915/intel_workarounds.c int intel_ctx_workarounds_emit(struct i915_request *rq) · line 597
Quoted 10 lines
drivers/gpu/drm/i915/intel_workarounds.c +8
-1-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 =1 <<4 ; + +GEM_BUG_ON (!is_power_of_2 (grow)); + +if (IS_ALIGNED (wal->count, grow)) {/* Either uninitialized or full. */
Neat.
drivers/gpu/drm/i915/intel_workarounds.c int intel_ctx_workarounds_emit(struct i915_request *rq) · line 601
Quoted 4 lines
drivers/gpu/drm/i915/intel_workarounds.c +4
-0+struct i915_wa *list; + + list =kcalloc (ALIGN (wal->count +1 , grow),sizeof (*wa), +GFP_KERNEL );
(Quietly comments on the calloc here ;)
drivers/gpu/drm/i915/intel_workarounds.c int intel_ctx_workarounds_emit(struct i915_request *rq) · line 614
Quoted 13 lines
drivers/gpu/drm/i915/intel_workarounds.c +13
-0+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; + } + +memcpy (&wal->list[wal->count], wa,sizeof (*wa)); + wal->count++;
I'd push for
wal->list[wal->count++] = wa;
Might as well use static type checking.
drivers/gpu/drm/i915/intel_workarounds.c int intel_ctx_workarounds_emit(struct i915_request *rq) · line 618
Quoted 4 lines
drivers/gpu/drm/i915/intel_workarounds.c +4
-0+struct i915_wa_list { +const char *name; +unsigned int count; +struct i915_wa *list;
Oh well, didn't save anything after all.
-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 1/7] drm/i915: Record GT workarounds in a list
Quoted 99 lines · Chris Wilson
Quoting Tvrtko Ursulin (2018-12-03 11:46:11)
Quoted 98 lines
From: Tvrtko Ursulin <[email protected]>
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.
v2:
* Changedev_privto i915 along the init path. (Chris Wilson)
* API rename. (Chris Wilson)v3:
* Remove explicit list size tracking in favour of growing the allocation in power of two chunks. (Chris Wilson)Signed-off-by Tvrtko Ursulin <[email protected]>Reviewed-by Chris Wilson <[email protected]> # v2--- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_gem.c | 4 +- drivers/gpu/drm/i915/intel_workarounds.c | 491 +++++++++++++++-------- drivers/gpu/drm/i915/intel_workarounds.h | 23 +- 5 files changed, 353 insertions(+), 168 deletions(-)drivers/gpu/drm/i915/i915_drv.c +1
-0@@ -1453,6 +1453,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)intel_uncore_sanitize (dev_priv); +intel_gt_init_workarounds (dev_priv);i915_gem_load_init_fences (dev_priv);/* On the 945G/GM, the chipset reports the MSI capability on the drivers/gpu/drm/i915/i915_drv.h +2
-0@@ -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;drivers/gpu/drm/i915/i915_gem.c +3
-1@@ -5334,7 +5334,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)I915_WRITE (MI_PREDICATE_RESULT_2 ,IS_HSW_GT3 (dev_priv) ?LOWER_SLICE_ENABLED :LOWER_SLICE_DISABLED ); -intel_gt_workarounds_apply (dev_priv); +intel_gt_apply_workarounds (dev_priv);i915_gem_init_swizzling (dev_priv);@@ -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);drivers/gpu/drm/i915/intel_workarounds.c +0
-0@@ -48,6 +48,18 @@* - Public functions to init or apply the given workaround type. */
Quoted 12 lines
drivers/gpu/drm/i915/intel_workarounds.c line 60
Quoted 10 lines
drivers/gpu/drm/i915/intel_workarounds.c +10
-0+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);Does this become if (!wal->count) return; later?
Done.
Quoted 21 lines
Quoted 8 lines
drivers/gpu/drm/i915/intel_workarounds.c +2
-0+} +static void wa_add (struct drm_i915_private *i915,i915_reg_t reg,const u32 mask,const u32 val) {@@ -575,160 +587,240 @@ int intel_ctx_workarounds_emit(struct i915_request *rq)return 0 ; }drivers/gpu/drm/i915/intel_workarounds.c int intel_ctx_workarounds_emit(struct i915_request *rq) · line 597
Quoted 10 lines
drivers/gpu/drm/i915/intel_workarounds.c +8
-1-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 =1 <<4 ; + +GEM_BUG_ON (!is_power_of_2 (grow)); + +if (IS_ALIGNED (wal->count, grow)) {/* Either uninitialized or full. */ Neat.
Quoted 6 lines
drivers/gpu/drm/i915/intel_workarounds.c int intel_ctx_workarounds_emit(struct i915_request *rq) · line 601
Quoted 4 lines
drivers/gpu/drm/i915/intel_workarounds.c +4
-0+struct i915_wa *list; + + list =kcalloc (ALIGN (wal->count +1 , grow),sizeof (*wa), +GFP_KERNEL );(Quietly comments on the calloc here ;)
Oh I don't want to complicate things with zeroing the tail. Or you wouldn't bother with zeroing at all since I always copy over used entries? So unused, who cares about them?
Quoted 16 lines
Quoted 4 lines
drivers/gpu/drm/i915/intel_workarounds.c +3
-0+if (!list) { +DRM_ERROR ("No space for workaround init!\n" ); +return ;drivers/gpu/drm/i915/intel_workarounds.c int intel_ctx_workarounds_emit(struct i915_request *rq) · line 614
Quoted 10 lines
drivers/gpu/drm/i915/intel_workarounds.c +10
-0+ } + +if (wal->list) +memcpy (list, wal->list,sizeof (*wa) * wal->count); + + wal->list = list; + } + +memcpy (&wal->list[wal->count], wa,sizeof (*wa)); + wal->count++;I'd push for
Quoted 2 lines
wal->list[wal->count++] = wa;Might as well use static type checking.
Ok.
Quoted 6 lines
drivers/gpu/drm/i915/intel_workarounds.c int intel_ctx_workarounds_emit(struct i915_request *rq) · line 618
Quoted 4 lines
drivers/gpu/drm/i915/intel_workarounds.c +4
-0+struct i915_wa_list { +const char *name; +unsigned int count; +struct i915_wa *list;Oh well, didn't save anything after all.
How nothing, one unsigned int per wa_list instance! :)
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 1/7] drm/i915: Record GT workarounds in a list
Quoting Tvrtko Ursulin (2018-12-03 12:34:24)
Quoted 25 lines · Chris Wilson
Quoted 23 lines
Quoting Tvrtko Ursulin (2018-12-03 11:46:11)
Quoted 4 lines
From: Tvrtko Ursulin <[email protected]>
drivers/gpu/drm/i915/intel_workarounds.c +0
-0@@ -575,160 +587,240 @@ int intel_ctx_workarounds_emit(struct i915_request *rq)return 0 ; }drivers/gpu/drm/i915/intel_workarounds.c int intel_ctx_workarounds_emit(struct i915_request *rq) · line 597
Quoted 10 lines
drivers/gpu/drm/i915/intel_workarounds.c +8
-1-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 =1 <<4 ; + +GEM_BUG_ON (!is_power_of_2 (grow)); + +if (IS_ALIGNED (wal->count, grow)) {/* Either uninitialized or full. */ Neat.
Quoted 4 lines
drivers/gpu/drm/i915/intel_workarounds.c +4
-0+struct i915_wa *list; + + list =kcalloc (ALIGN (wal->count +1 , grow),sizeof (*wa), +GFP_KERNEL );
Quoted 5 lines
Quoted 1 line
(Quietly comments on the calloc here ;)
Oh I don't want to complicate things with zeroing the tail. Or you wouldn't bother with zeroing at all since I always copy over used entries? So unused, who cares about them?
Exactly, nothing after wal->count is accessed, so no need to zero it as it will always be initialised by wal_add().
Quoted 8 lines
Quoted 6 lines
drivers/gpu/drm/i915/intel_workarounds.h line 21
Quoted 4 lines
drivers/gpu/drm/i915/intel_workarounds.h +4
-0+struct i915_wa_list { +const char *name; +unsigned int count; +struct i915_wa *list;Oh well, didn't save anything after all.
How nothing, one unsigned int per
wa_listinstance! :)
struct is padded to 8 bytes for the pointer. There's a hole after count, even if we place it at the end. :|
-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 1/7] drm/i915: Record GT workarounds in a list
Quoted 26 lines · Chris Wilson
Quoting Tvrtko Ursulin (2018-12-03 12:34:24)
Quoted 25 lines · Chris Wilson
Quoted 23 lines
Quoting Tvrtko Ursulin (2018-12-03 11:46:11)
Quoted 4 lines
From: Tvrtko Ursulin <[email protected]>
drivers/gpu/drm/i915/intel_workarounds.c +0
-0@@ -575,160 +587,240 @@ int intel_ctx_workarounds_emit(struct i915_request *rq)return 0 ; }drivers/gpu/drm/i915/intel_workarounds.c int intel_ctx_workarounds_emit(struct i915_request *rq) · line 597
Quoted 10 lines
drivers/gpu/drm/i915/intel_workarounds.c +8
-1-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 =1 <<4 ; + +GEM_BUG_ON (!is_power_of_2 (grow)); + +if (IS_ALIGNED (wal->count, grow)) {/* Either uninitialized or full. */ Neat.
Quoted 4 lines
drivers/gpu/drm/i915/intel_workarounds.c +4
-0+struct i915_wa *list; + + list =kcalloc (ALIGN (wal->count +1 , grow),sizeof (*wa), +GFP_KERNEL );
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
(Quietly comments on the calloc here ;)
Oh I don't want to complicate things with zeroing the tail. Or you wouldn't bother with zeroing at all since I always copy over used entries? So unused, who cares about them?
Exactly, nothing after
wal->countis accessed, so no need to zero it as it will always be initialised bywal_add().
Ok.
Quoted 11 lines
Quoted 8 lines
Quoted 6 lines
drivers/gpu/drm/i915/intel_workarounds.h line 21
Quoted 4 lines
drivers/gpu/drm/i915/intel_workarounds.h +4
-0+struct i915_wa_list { +const char *name; +unsigned int count; +struct i915_wa *list;Oh well, didn't save anything after all.
How nothing, one unsigned int per
wa_listinstance! :)struct is padded to 8 bytes for the pointer. There's a hole after count, even if we place it at the end. :|
Another unsigned int gets in there later in the series - for the benefit of being able to log the exact, and not merged, number of workarounds. But it makes sense to move the count at the end anyway so I'll do that.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v3 2/7 drm/i915: Introduce per-engine workarounds
In reply to Tvrtko UrsulinFrom: Tvrtko Ursulin <[email protected]>
We stopped re-applying the GT workarounds after engine reset since commit 59b449d5c82a ("drm/i915: Split out functions for different kinds of workarounds").
Issue with this is that some of the GT workarounds live in the MMIO space which gets lost during engine resets. So far the registers in 0x2xxx and 0xbxxx address range have been identified to be affected.
This losing of applied workarounds has obvious negative effects and can even lead to hard system hangs (see the linked Bugzilla).
Rather than just restoring this re-application, because we have also observed that it is not safe to just re-write all GT workarounds after engine resets (GPU might be live and weird hardware states can happen), we introduce a new class of per-engine workarounds and move only the affected GT workarounds over.
Using the framework introduced in the previous patch, we therefore after engine reset, re-apply only the workarounds living in the affected MMIO address ranges.
v2:
* Move Wa_1406609255:icl to engine workarounds as well.
* Rename API. (Chris Wilson)
* Drop redundant IS_KABYLAKE. (Chris Wilson)
* Re-order engine wa/ init so latest platforms are first. (Rodrigo Vivi)
Bugzilla: https://bugzilla.freedesktop.org/show_bug.cgi?id=107945
--- drivers/gpu/drm/i915/intel_engine_cs.c | 2 + drivers/gpu/drm/i915/intel_lrc.c | 4 + drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + drivers/gpu/drm/i915/intel_workarounds.c | 261 ++++++++++++----------- drivers/gpu/drm/i915/intel_workarounds.h | 3 + 5 files changed, 153 insertions(+), 119 deletions(-)
drivers/gpu/drm/i915/intel_engine_cs.c +2 -0
@@ -723,6 +723,8 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)__intel_context_unpin (i915->kernel_context, engine);i915_timeline_fini (&engine->timeline); + +intel_wa_list_free (&engine->wa_list); }u64 intel_engine_get_active_head (const struct intel_engine_cs *engine)
drivers/gpu/drm/i915/intel_lrc.c +4 -0
@@ -1617,6 +1617,8 @@ static bool unexpected_starting_state(struct intel_engine_cs *engine)static int gen8_init_common_ring (struct intel_engine_cs *engine) { +intel_engine_apply_workarounds (engine); +intel_mocs_init_engine (engine);intel_engine_reset_breadcrumbs (engine);
@@ -2314,6 +2316,8 @@ int logical_render_ring_init(struct intel_engine_cs *engine) ret); } +intel_engine_init_workarounds (engine); +return 0 ; err_cleanup_common:
drivers/gpu/drm/i915/intel_ringbuffer.h +2 -0
@@ -15,6 +15,7 @@#include "i915_selftest.h" #include "i915_timeline.h" #include "intel_gpu_commands.h" +#include "intel_workarounds.h" struct drm_printer ;struct i915_sched_attr ;
@@ -451,6 +452,7 @@ struct intel_engine_cs {struct intel_hw_status_page status_page;struct i915_ctx_workarounds wa_ctx; +struct i915_wa_list wa_list;struct i915_vma *scratch;u32 irq_keep_mask;/* always keep these interrupts */
drivers/gpu/drm/i915/intel_workarounds.c +142 -119
344 lines not on this page · raw message
drivers/gpu/drm/i915/intel_workarounds.h +3 -0
@@ -35,4 +35,7 @@ void intel_gt_apply_workarounds(struct drm_i915_private *dev_priv);void intel_whitelist_workarounds_apply (struct intel_engine_cs *engine); +void intel_engine_init_workarounds (struct intel_engine_cs *engine); +void intel_engine_apply_workarounds (struct intel_engine_cs *engine); +#endif
Signature · 6 lines
2.19.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v3 3/7 drm/i915: Verify GT workaround state after GPU init
In reply to Tvrtko UrsulinFrom: Tvrtko Ursulin <[email protected]>
Since we now have all the GT workarounds in a table, by adding a simple shared helper function we can now verify that their values are still applied after some interesting events in the lifetime of the driver.
Initially we only do this after GPU initialization.
v2:
Chris Wilson:
* Simplify verification by realizing it's a simple xor and and.
* Remove verification from engine reset path.
* Return bool straight away from the verify API.
v3:
* API rename. (Chris Wilson)
--- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_gem.c | 3 +++ drivers/gpu/drm/i915/intel_workarounds.c | 34 ++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_workarounds.h | 2 ++ 4 files changed, 40 insertions(+)
drivers/gpu/drm/i915/i915_drv.c +1 -0
@@ -53,6 +53,7 @@#include "i915_vgpu.h" #include "intel_drv.h" #include "intel_uc.h" +#include "intel_workarounds.h" static struct drm_driver driver;
drivers/gpu/drm/i915/i915_gem.c +3 -0
@@ -5334,7 +5334,10 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)I915_WRITE (MI_PREDICATE_RESULT_2 ,IS_HSW_GT3 (dev_priv) ?LOWER_SLICE_ENABLED :LOWER_SLICE_DISABLED ); +/* Apply the GT workarounds... */ intel_gt_apply_workarounds (dev_priv); +/* ...and determine whether they are sticking. */ +intel_gt_verify_workarounds (dev_priv,"init" );i915_gem_init_swizzling (dev_priv);
drivers/gpu/drm/i915/intel_workarounds.c +34 -0
@@ -976,6 +976,40 @@ void intel_gt_apply_workarounds(struct drm_i915_private *dev_priv)wa_list_apply (dev_priv, &dev_priv->gt_wa_list); } +static bool +wa_verify (const struct i915_wa *wa,u32 cur,const char *name,const char *from) +{ +if ((cur ^ wa->val) & wa->mask) { +DRM_ERROR ("%s workaround lost on %s! (%x=%x/%x, expected %x, mask=%x)\n" , + name, from,i915_mmio_reg_offset (wa->reg), cur, + cur & wa->mask, wa->val, wa->mask); + +return false; + } + +return true; +} + +static bool wa_list_verify (struct drm_i915_private *dev_priv, +const struct i915_wa_list *wal, +const char *from) +{ +struct i915_wa *wa; +unsigned int i; +bool ok = true; + +for (i =0 , wa = wal->list; i < wal->count; i++, wa++) + ok &=wa_verify (wa,I915_READ (wa->reg), wal->name, from); + +return ok; +} + +bool intel_gt_verify_workarounds (struct drm_i915_private *dev_priv, +const char *from) +{ +return wa_list_verify (dev_priv, &dev_priv->gt_wa_list, from); +} +struct whitelist {i915_reg_t reg[RING_MAX_NONPRIV_SLOTS ];unsigned int count;
drivers/gpu/drm/i915/intel_workarounds.h +2 -0
@@ -32,6 +32,8 @@ int intel_ctx_workarounds_emit(struct i915_request *rq);void intel_gt_init_workarounds (struct drm_i915_private *dev_priv);void intel_gt_apply_workarounds (struct drm_i915_private *dev_priv); +bool intel_gt_verify_workarounds (struct drm_i915_private *dev_priv, +const char *from);void intel_whitelist_workarounds_apply (struct intel_engine_cs *engine);
Signature · 6 lines
2.19.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v3 4/7 drm/i915/selftests: Add tests for GT and engine workaround verification
In reply to Tvrtko UrsulinFrom: Tvrtko Ursulin <[email protected]>
Two simple selftests which test that both GT and engine workarounds are not lost after either a full GPU reset, or after the per-engine ones.
(Including checks that one engine reset is not affecting workarounds not belonging to itself.)
v2:
* Rebase for series refactoring.
* Add spinner for actual engine reset!
* Add idle reset test as well. (Chris Wilson)
* Share existing global_reset_lock. (Chris Wilson)
v3:
* intel_engine_verify_workarounds can be static.
* API rename. (Chris Wilson)
* Move global reset lock out of the loop. (Chris Wilson)
--- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/intel_workarounds.c | 6 + drivers/gpu/drm/i915/selftests/igt_reset.c | 44 ++++++ drivers/gpu/drm/i915/selftests/igt_reset.h | 15 ++ .../gpu/drm/i915/selftests/intel_hangcheck.c | 51 ++----- .../drm/i915/selftests/intel_workarounds.c | 142 +++++++++++++++++- 6 files changed, 212 insertions(+), 47 deletions(-) create mode 100644 drivers/gpu/drm/i915/selftests/igt_reset.c create mode 100644 drivers/gpu/drm/i915/selftests/igt_reset.h
drivers/gpu/drm/i915/Makefile +1 -0
@@ -166,6 +166,7 @@ i915-$(CONFIG_DRM_I915_SELFTEST) += \ selftests/i915_random.o \ selftests/i915_selftest.o \ selftests/igt_flush_test.o \ + selftests/igt_reset.o \ selftests/igt_spinner.o # virtual gpu code
drivers/gpu/drm/i915/intel_workarounds.c +6 -0
@@ -1302,5 +1302,11 @@ void intel_engine_apply_workarounds(struct intel_engine_cs *engine) }#if IS_ENABLED (CONFIG_DRM_I915_SELFTEST ) +static bool intel_engine_verify_workarounds (struct intel_engine_cs *engine, +const char *from) +{ +return wa_list_verify (engine->i915, &engine->wa_list, from); +} +#include "selftests/intel_workarounds.c" #endif
drivers/gpu/drm/i915/selftests/igt_reset.c new file +44 -0
44 lines not on this page · raw message
drivers/gpu/drm/i915/selftests/igt_reset.h new file +15 -0
@@ -0,0 +1,15 @@ +/* +* SPDX-License-Identifier: MIT +* +* Copyright © 2018 Intel Corporation +*/ + +#ifndef __I915_SELFTESTS_IGT_RESET_H__ +#define __I915_SELFTESTS_IGT_RESET_H__ + +#include "../i915_drv.h" + +void igt_global_reset_lock (struct drm_i915_private *i915); +void igt_global_reset_unlock (struct drm_i915_private *i915); + +#endif
drivers/gpu/drm/i915/selftests/intel_hangcheck.c +9 -42
111 lines not on this page · raw message
drivers/gpu/drm/i915/selftests/intel_workarounds.c +137 -5
173 lines not on this page · raw message
Signature · 6 lines
2.19.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 4/7] drm/i915/selftests: Add tests for GT and engine workaround verification
Quoting Tvrtko Ursulin (2018-12-03 11:46:14)
Quoted 3 lines
drivers/gpu/drm/i915/selftests/intel_workarounds.c +3
-0+static int +live_engine_reset_gt_engine_workarounds (void *arg) +{
...
Quoted 6 lines
drivers/gpu/drm/i915/selftests/intel_workarounds.c +6
-0+ ok =verify_gt_engine_wa (i915,"after idle reset" ); +if (!ok) { + ret = -ESRCH ; + goto err; + } +
drivers/gpu/drm/i915/selftests/intel_workarounds.c static int live_reset_whitelist(void *arg) · line 424
Quoted 10 lines
drivers/gpu/drm/i915/selftests/intel_workarounds.c +10
-0+ ret =igt_spinner_init (&spin, i915); +if (ret) + goto err; + +intel_runtime_pm_get (i915); + + rq =igt_spinner_create_request (&spin, ctx, engine,MI_NOOP ); +if (IS_ERR (rq)) { + ret =PTR_ERR (rq); +igt_spinner_fini (&spin);
Missing rpm_put
drivers/gpu/drm/i915/selftests/intel_workarounds.c static int live_reset_whitelist(void *arg) · line 433
Quoted 9 lines
drivers/gpu/drm/i915/selftests/intel_workarounds.c +9
-0+ goto err; + } + +i915_request_add (rq); + +if (!igt_wait_for_spinner (&spin, rq)) { +pr_err ("Spinner failed to start\n" ); +igt_spinner_fini (&spin); + ret = -ETIMEDOUT ;
Missing rpm_put
I need to convince people to take the wakeref tracking to make these leaks more obvious :)
-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v3 5/7 drm/i915: Move register white-listing to the common workaround framework
In reply to Tvrtko UrsulinFrom: Tvrtko Ursulin <[email protected]>
Instead of having a separate list of white-listed registers we can trivially move this to the common workarounds framework.
This brings us one step closer to the goal of driving all workaround classes using the same code.
v2:
* Use GEM_DEBUG_WARN_ON for the sanity check. (Chris Wilson)
v3:
* API rename. (Chris Wilson)
--- drivers/gpu/drm/i915/intel_engine_cs.c | 1 + drivers/gpu/drm/i915/intel_lrc.c | 5 +- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + drivers/gpu/drm/i915/intel_workarounds.c | 83 ++++++++----------- drivers/gpu/drm/i915/intel_workarounds.h | 3 +- .../drm/i915/selftests/intel_workarounds.c | 40 ++++----- 6 files changed, 60 insertions(+), 73 deletions(-)
drivers/gpu/drm/i915/intel_engine_cs.c +1 -0
@@ -725,6 +725,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)i915_timeline_fini (&engine->timeline);intel_wa_list_free (&engine->wa_list); +intel_wa_list_free (&engine->whitelist); }u64 intel_engine_get_active_head (const struct intel_engine_cs *engine)
drivers/gpu/drm/i915/intel_lrc.c +3 -2
@@ -1643,7 +1643,7 @@ static int gen8_init_render_ring(struct intel_engine_cs *engine)if (ret)return ret; -intel_whitelist_workarounds_apply (engine); +intel_engine_apply_whitelist (engine);/* We need to disable the AsyncFlip performance optimisations in order * to use MI_WAIT_FOR_EVENT within the CS. It should already be
@@ -1666,7 +1666,7 @@ static int gen9_init_render_ring(struct intel_engine_cs *engine)if (ret)return ret; -intel_whitelist_workarounds_apply (engine); +intel_engine_apply_whitelist (engine);return 0 ; }
@@ -2316,6 +2316,7 @@ int logical_render_ring_init(struct intel_engine_cs *engine) ret); } +intel_engine_init_whitelist (engine);intel_engine_init_workarounds (engine);return 0 ;
drivers/gpu/drm/i915/intel_ringbuffer.h +1 -0
@@ -453,6 +453,7 @@ struct intel_engine_cs {struct intel_hw_status_page status_page;struct i915_ctx_workarounds wa_ctx;struct i915_wa_list wa_list; +struct i915_wa_list whitelist;struct i915_vma *scratch;u32 irq_keep_mask;/* always keep these interrupts */
drivers/gpu/drm/i915/intel_workarounds.c +33 -50
165 lines not on this page · raw message
drivers/gpu/drm/i915/intel_workarounds.h +2 -1
@@ -35,7 +35,8 @@ void intel_gt_apply_workarounds(struct drm_i915_private *dev_priv);bool intel_gt_verify_workarounds (struct drm_i915_private *dev_priv,const char *from); -void intel_whitelist_workarounds_apply (struct intel_engine_cs *engine); +void intel_engine_init_whitelist (struct intel_engine_cs *engine); +void intel_engine_apply_whitelist (struct intel_engine_cs *engine);void intel_engine_init_workarounds (struct intel_engine_cs *engine);void intel_engine_apply_workarounds (struct intel_engine_cs *engine);
drivers/gpu/drm/i915/selftests/intel_workarounds.c +20 -20
@@ -94,17 +94,23 @@ read_nonprivs(struct i915_gem_context *ctx, struct intel_engine_cs *engine)return ERR_PTR (err); } -static u32 get_whitelist_reg (const struct whitelist *w,unsigned int i) +static u32 +get_whitelist_reg (const struct intel_engine_cs *engine,unsigned int i) { -return i < w->count ?i915_mmio_reg_offset (w->reg[i]) : w->nopid; +i915_reg_t reg = i < engine->whitelist.count ? + engine->whitelist.list[i].reg : +RING_NOPID (engine->mmio_base); + +return i915_mmio_reg_offset (reg); } -static void print_results (const struct whitelist *w,const u32 *results) +static void +print_results (const struct intel_engine_cs *engine,const u32 *results) {unsigned int i;for (i =0 ; i <RING_MAX_NONPRIV_SLOTS ; i++) { -u32 expected =get_whitelist_reg (w, i); +u32 expected =get_whitelist_reg (engine, i);u32 actual = results[i]; pr_info("RING_NONPRIV[%d]: expected 0x%08x, found 0x%08x\n",
@@ -112,8 +118,7 @@ static void print_results(const struct whitelist *w, const u32 *results) } } -static int check_whitelist (const struct whitelist *w, -struct i915_gem_context *ctx, +static int check_whitelist (struct i915_gem_context *ctx,struct intel_engine_cs *engine) {struct drm_i915_gem_object *results;
@@ -141,11 +146,11 @@ static int check_whitelist(const struct whitelist *w, }for (i =0 ; i <RING_MAX_NONPRIV_SLOTS ; i++) { -u32 expected =get_whitelist_reg (w, i); +u32 expected =get_whitelist_reg (engine, i);u32 actual = vaddr[i];if (expected != actual) { -print_results (w, vaddr); +print_results (engine, vaddr);pr_err ("Invalid RING_NONPRIV[%d], expected 0x%08x, found 0x%08x\n" , i, expected, actual);
@@ -217,7 +222,6 @@ switch_to_scratch_context(struct intel_engine_cs *engine,static int check_whitelist_across_reset (struct intel_engine_cs *engine,int (*reset)(struct intel_engine_cs *), -const struct whitelist *w,const char *name) {struct drm_i915_private *i915 = engine->i915;
@@ -227,7 +231,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,int err;pr_info ("Checking %d whitelisted registers (RING_NONPRIV) [%s]\n" , - w->count, name); + engine->whitelist.count, name);if (want_spin) { err =igt_spinner_init (&spin, i915);
@@ -239,7 +243,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,if (IS_ERR (ctx))return PTR_ERR (ctx); - err =check_whitelist (w, ctx, engine); + err =check_whitelist (ctx, engine);if (err) {pr_err ("Invalid whitelist *before* %s reset!\n" , name); goto out;
@@ -263,7 +267,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine, goto out; } - err =check_whitelist (w, ctx, engine); + err =check_whitelist (ctx, engine);if (err) {pr_err ("Whitelist not preserved in context across %s reset!\n" , name);
@@ -276,7 +280,7 @@ static int check_whitelist_across_reset(struct intel_engine_cs *engine,if (IS_ERR (ctx))return PTR_ERR (ctx); - err =check_whitelist (w, ctx, engine); + err =check_whitelist (ctx, engine);if (err) {pr_err ("Invalid whitelist *after* %s reset in fresh context!\n" , name);
@@ -292,22 +296,18 @@ static int live_reset_whitelist(void *arg) {struct drm_i915_private *i915 = arg;struct intel_engine_cs *engine = i915->engine[RCS ]; -struct whitelist w;int err =0 ;/* If we reset the gpu, we should not lose the RING_NONPRIV */ -if (!engine) -return 0 ; - -if (!whitelist_build (engine, &w)) +if (!engine || engine->whitelist.count ==0 )return 0 ;igt_global_reset_lock (i915);if (intel_has_reset_engine (i915)) { err =check_whitelist_across_reset (engine, - do_engine_reset, &w, + do_engine_reset,"engine" );if (err) goto out;
@@ -315,7 +315,7 @@ static int live_reset_whitelist(void *arg)if (intel_has_gpu_reset (i915)) { err =check_whitelist_across_reset (engine, - do_device_reset, &w, + do_device_reset,"device" );if (err) goto out;
Signature · 6 lines
2.19.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v3 6/7 drm/i915: Fuse per-context workaround handling with the common framework
In reply to Tvrtko UrsulinFrom: Tvrtko Ursulin <[email protected]>
Convert the per context workaround handling code to run against the newly introduced common workaround framework and fuse the two to use the existing smarter list add helper, the one which does the sorted insert and merges registers where possible.
This completes migration of all four classes of workarounds onto the common framework.
Existing macros are kept untouched for smaller code churn.
v2:
* Rename to list name ctx_wa_list and move from dev_priv to engine.
v3:
* API rename and parameters tweaking. (Chris Wilson)
--- drivers/gpu/drm/i915/i915_debugfs.c | 12 +- drivers/gpu/drm/i915/i915_drv.h | 15 - drivers/gpu/drm/i915/i915_gem_context.c | 6 +- drivers/gpu/drm/i915/intel_engine_cs.c | 1 + drivers/gpu/drm/i915/intel_lrc.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + drivers/gpu/drm/i915/intel_workarounds.c | 334 +++++++++++------------ drivers/gpu/drm/i915/intel_workarounds.h | 5 +- 9 files changed, 168 insertions(+), 210 deletions(-)
drivers/gpu/drm/i915/i915_debugfs.c +7 -5
@@ -3375,13 +3375,15 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)static int i915_wa_registers (struct seq_file *m,void *unused) { -struct i915_workarounds *wa = &node_to_i915 (m->private)->workarounds; -int i; +struct drm_i915_private *i915 =node_to_i915 (m->private); +const struct i915_wa_list *wal = &i915->engine[RCS ]->ctx_wa_list; +struct i915_wa *wa; +unsigned int i; -seq_printf (m,"Workarounds applied: %d\n" , wa->count); -for (i =0 ; i < wa->count; ++i) +seq_printf (m,"Workarounds applied: %u\n" , wal->count); +for (i =0 , wa = wal->list; i < wal->count; i++, wa++)seq_printf (m,"0x%X: 0x%08X, mask: 0x%08X\n" , - wa->reg[i].addr, wa->reg[i].value, wa->reg[i].mask); +i915_mmio_reg_offset (wa->reg), wa->val, wa->mask);return 0 ; }
drivers/gpu/drm/i915/i915_drv.h +0 -15
@@ -1189,20 +1189,6 @@ struct i915_frontbuffer_tracking {unsigned flip_bits; }; -struct i915_wa_reg { -u32 addr; -u32 value; -/* bitmask representing WA bits */ -u32 mask; -}; - -#define I915_MAX_WA_REGS 16 - -struct i915_workarounds { -struct i915_wa_reg reg[I915_MAX_WA_REGS ]; -u32 count; -}; -struct i915_virtual_gpu {bool active;u32 caps;
@@ -1652,7 +1638,6 @@ 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;
drivers/gpu/drm/i915/i915_gem_context.c +1 -5
@@ -535,16 +535,12 @@ static bool needs_preempt_context(struct drm_i915_private *i915)int i915_gem_contexts_init (struct drm_i915_private *dev_priv) {struct i915_gem_context *ctx; -int ret;/* Reassure ourselves we are only called once */ GEM_BUG_ON (dev_priv->kernel_context);GEM_BUG_ON (dev_priv->preempt_context); - ret =intel_ctx_workarounds_init (dev_priv); -if (ret) -return ret; - +intel_engine_init_ctx_wa (dev_priv->engine[RCS ]);init_contexts (dev_priv);/* lowest priority; idle task */
drivers/gpu/drm/i915/intel_engine_cs.c +1 -0
@@ -724,6 +724,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)i915_timeline_fini (&engine->timeline); +intel_wa_list_free (&engine->ctx_wa_list);intel_wa_list_free (&engine->wa_list);intel_wa_list_free (&engine->whitelist); }
drivers/gpu/drm/i915/intel_lrc.c +1 -1
@@ -2078,7 +2078,7 @@ static int gen8_init_rcs_context(struct i915_request *rq) {int ret; - ret =intel_ctx_workarounds_emit (rq); + ret =intel_engine_emit_ctx_wa (rq);if (ret)return ret;
drivers/gpu/drm/i915/intel_ringbuffer.c +1 -1
@@ -649,7 +649,7 @@ static int intel_rcs_ctx_init(struct i915_request *rq) {int ret; - ret =intel_ctx_workarounds_emit (rq); + ret =intel_engine_emit_ctx_wa (rq);if (ret !=0 )return ret;
drivers/gpu/drm/i915/intel_ringbuffer.h +1 -0
@@ -452,6 +452,7 @@ struct intel_engine_cs {struct intel_hw_status_page status_page;struct i915_ctx_workarounds wa_ctx; +struct i915_wa_list ctx_wa_list;struct i915_wa_list wa_list;struct i915_wa_list whitelist;struct i915_vma *scratch;
drivers/gpu/drm/i915/intel_workarounds.c +153 -181
571 lines not on this page · raw message
drivers/gpu/drm/i915/intel_workarounds.h +3 -2
@@ -18,6 +18,7 @@ struct i915_wa {struct i915_wa_list {const char *name;unsigned int count; +unsigned int wa_count;struct i915_wa *list; };
@@ -27,8 +28,8 @@ static inline void intel_wa_list_free(struct i915_wa_list *wal)memset (wal,0 ,sizeof (*wal)); } -int intel_ctx_workarounds_init (struct drm_i915_private *dev_priv); -int intel_ctx_workarounds_emit (struct i915_request *rq); +void intel_engine_init_ctx_wa (struct intel_engine_cs *engine); +int intel_engine_emit_ctx_wa (struct i915_request *rq);void intel_gt_init_workarounds (struct drm_i915_private *dev_priv);void intel_gt_apply_workarounds (struct drm_i915_private *dev_priv);
Signature · 6 lines
2.19.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 6/7] drm/i915: Fuse per-context workaround handling with the common framework
Quoting Tvrtko Ursulin (2018-12-03 11:46:16)
Quoted 36 lines
From: Tvrtko Ursulin <[email protected]>
Convert the per context workaround handling code to run against the newly introduced common workaround framework and fuse the two to use the existing smarter list add helper, the one which does the sorted insert and merges registers where possible.
This completes migration of all four classes of workarounds onto the common framework.
Existing macros are kept untouched for smaller code churn.
v2:
* Rename to list namectx_wa_listand move fromdev_privto engine.v3:
* API rename and parameters tweaking. (Chris Wilson)Signed-off-by Tvrtko Ursulin <[email protected]>--- drivers/gpu/drm/i915/i915_debugfs.c | 12 +- drivers/gpu/drm/i915/i915_drv.h | 15 - drivers/gpu/drm/i915/i915_gem_context.c | 6 +- drivers/gpu/drm/i915/intel_engine_cs.c | 1 + drivers/gpu/drm/i915/intel_lrc.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + drivers/gpu/drm/i915/intel_workarounds.c | 334 +++++++++++------------ drivers/gpu/drm/i915/intel_workarounds.h | 5 +- 9 files changed, 168 insertions(+), 210 deletions(-)drivers/gpu/drm/i915/i915_debugfs.c +0
-0
drivers/gpu/drm/i915/i915_debugfs.c static int i915_shared_dplls_info(struct seq_file *m, void *unused) · line 3381
Quoted 9 lines
drivers/gpu/drm/i915/i915_debugfs.c +4
-2@@ -3375,13 +3375,15 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)static int i915_wa_registers (struct seq_file *m,void *unused) { -struct i915_workarounds *wa = &node_to_i915 (m->private)->workarounds; -int i; +struct drm_i915_private *i915 =node_to_i915 (m->private); +const struct i915_wa_list *wal = &i915->engine[RCS ]->ctx_wa_list; +struct i915_wa *wa; +unsigned int i;
You keep telling me to be defensive around the existence of RCS :)
drivers/gpu/drm/i915/intel_workarounds.c
Quoted 15 lines
drivers/gpu/drm/i915/i915_debugfs.c +3
-3-seq_printf (m,"Workarounds applied: %d\n" , wa->count); -for (i =0 ; i < wa->count; ++i) +seq_printf (m,"Workarounds applied: %u\n" , wal->count); +for (i =0 , wa = wal->list; i < wal->count; i++, wa++)seq_printf (m,"0x%X: 0x%08X, mask: 0x%08X\n" , - wa->reg[i].addr, wa->reg[i].value, wa->reg[i].mask); +i915_mmio_reg_offset (wa->reg), wa->val, wa->mask);return 0 ; }drivers/gpu/drm/i915/intel_workarounds.c +0
-0
Lgtm,
-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v3 7/7 drm/i915: Trim unused workaround list entries
In reply to Tvrtko UrsulinFrom: Tvrtko Ursulin <[email protected]>
The new workaround list allocator grows the list in chunks so will end up with some unused space. Trim it when the initialization phase is done to free up a tiny bit of slab.
v2:
* Simplify with kmemdup. (Chris Wilson)
v3:
* Refactor for __size removal.
--- drivers/gpu/drm/i915/intel_workarounds.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/intel_workarounds.c +15 -1
@@ -53,8 +53,22 @@ static void wa_init_start(struct i915_wa_list *wal, const char *name) wal->name = name; } +#define WA_LIST_CHUNK (1 << 4) +static void wa_init_finish (struct i915_wa_list *wal) { +/* Trim unused entries. */ +if (!IS_ALIGNED (wal->count,WA_LIST_CHUNK )) { +struct i915_wa *list =kmemdup (wal->list, + wal->count *sizeof (*list), +GFP_KERNEL ); + +if (list) { +kfree (wal->list); + wal->list = list; + } + } +if (wal->count)DRM_DEBUG_DRIVER ("Initialized %u %s workarounds\n" , wal->wa_count, wal->name);
@@ -64,7 +78,7 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) {unsigned int addr =i915_mmio_reg_offset (wa->reg);unsigned int start =0 , end = wal->count; -const unsigned int grow =1 <<4 ; +const unsigned int grow =WA_LIST_CHUNK ;struct i915_wa *wa_;GEM_BUG_ON (!is_power_of_2 (grow));
Signature · 6 lines
2.19.1 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 7/7] drm/i915: Trim unused workaround list entries
Quoting Tvrtko Ursulin (2018-12-03 11:46:17)
Quoted 6 lines
From: Tvrtko Ursulin <[email protected]>
The new workaround list allocator grows the list in chunks so will end up with some unused space. Trim it when the initialization phase is done to free up a tiny bit of slab.
Quoted 8 lines
v2:
* Simplify with kmemdup. (Chris Wilson)v3:
* Refactor for__sizeremoval.Signed-off-by Tvrtko Ursulin <[email protected]>Reviewed-by Chris Wilson <[email protected]> # v2
I'd jiggle the if (!wal->count) return around, r-b will remain. -Chris
_______________________________________________
Intel-gfx mailing list [email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ Fi.CI.CHECKPATCH: warning for Restore workarounds after engine reset and unify their handling (rev3)
In reply to Tvrtko Ursulin== Series Details ==
== Series Details ==
Series: Restore workarounds after engine reset and unify their handling (rev3)
URL : https://patchwork.freedesktop.org/series/53313/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip ed91418aaeec drm/i915: Record GT workarounds in a list -:492: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #492: FILE: drivers/gpu/drm/i915/intel_workarounds.c:914: + wa_masked_en(wal, + _3D_CHICKEN3,
-:768: WARNING:SPACE_BEFORE_TAB: please, no space before tabs #768: FILE: drivers/gpu/drm/i915/intel_workarounds.h:20:
+^Iunsigned int ^Icount;$
total: 0 errors, 1 warnings, 1 checks, 724 lines checkedccb6a2c1f539 drm/i915: Introduce per-engine workarounds
-:10: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 59b449d5c82a ("drm/i915: Split out functions for different kinds of workarounds")' #10:59b449d5c82a ("drm/i915: Split out functions for different kinds of
total: 1 errors, 0 warnings, 0 checks, 389 lines checked8d055232d8d8 drm/i915: Verify GT workaround state after GPU inite779d6bfdfef drm/i915/selftests: Add tests for GT and engine workaround verification -:56: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating? #56:
new file mode 100644
-:61: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1 #61: FILE: drivers/gpu/drm/i915/selftests/igt_reset.c:1:
+/*
-:111: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1 #111: FILE: drivers/gpu/drm/i915/selftests/igt_reset.h:1:
+/*
total: 0 errors, 3 warnings, 0 checks, 361 lines checkedd695d6e32eb8 drm/i915: Move register white-listing to the common workaround framework ea620326b3cb drm/i915: Fuse per-context workaround handling with the common framework dc515637f6ec drm/i915: Trim unused workaround list entries
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✗ Fi.CI.SPARSE: warning for Restore workarounds after engine reset and unify their handling (rev3)
In reply to Tvrtko Ursulin== Series Details ==
== Series Details ==
Series: Restore workarounds after engine reset and unify their handling (rev3)
URL : https://patchwork.freedesktop.org/series/53313/
State : warning
== Summary ==
$ dim sparse origin/drm-tip Sparse version: v0.5.2 Commit: drm/i915: Record GT workarounds in a list -drivers/gpu/drm/i915/selftests/../i915_drv.h:3571:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3573:16: warning: expression using sizeof(void) +./include/linux/slab.h:665:13: error: undefined identifier '__builtin_mul_overflow' +./include/linux/slab.h:665:13: warning: call with no type!
Commit: drm/i915: Introduce per-engine workarounds Okay!
Commit: drm/i915: Verify GT workaround state after GPU init Okay!
Commit: drm/i915/selftests: Add tests for GT and engine workaround verification +./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)
Commit: drm/i915: Move register white-listing to the common workaround framework Okay!
Commit: drm/i915: Fuse per-context workaround handling with the common framework -drivers/gpu/drm/i915/selftests/../i915_drv.h:3573:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3558:16: warning: expression using sizeof(void)
Commit: drm/i915: Trim unused workaround list entries Okay!
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✓ Fi.CI.BAT: success for Restore workarounds after engine reset and unify their handling (rev3)
In reply to Tvrtko Ursulin== Series Details ==
== Series Details ==
Series: Restore workarounds after engine reset and unify their handling (rev3)
URL : https://patchwork.freedesktop.org/series/53313/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5239 -> Patchwork_10997
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/53313/revisions/3/mbox/
Known issues
------------
Here are the changes found in Patchwork_10997 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_create@basic-files:
- fi-bsw-n3050: PASS -> FAIL [fdo#108656]
* igt@gem_exec_reloc@basic-gtt:
- fi-skl-6770hq: PASS -> DMESG-WARN [fdo#105541]
* {igt@runner@aborted}:
- {fi-icl-y}: NOTRUN -> FAIL [fdo#108070]
#### Possible fixes ####
* igt@i915_selftest@live_execlists:
- fi-apl-guc: INCOMPLETE [fdo#103927] / [fdo#108622] -> PASS
#### Warnings ####
* igt@i915_selftest@live_contexts:
- {fi-icl-u3}: INCOMPLETE [fdo#108315] -> DMESG-FAIL [fdo#108569]
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#105541]: https://bugs.freedesktop.org/show_bug.cgi?id=105541
[fdo#108070]: https://bugs.freedesktop.org/show_bug.cgi?id=108070
[fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
[fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656
Participating hosts (47 -> 42)
------------------------------
Additional (1): fi-icl-y
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-pnv-d510
Build changes
-------------
* Linux: CI_DRM_5239 -> Patchwork_10997
CI_DRM_5239: 6b82ae50cbf9b70fb3884937a221f69261c4c41c @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10997: dc515637f6ecad20b108197a91ecbdd6046b7e31 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
dc515637f6ec drm/i915: Trim unused workaround list entriesea620326b3cb drm/i915: Fuse per-context workaround handling with the common framework d695d6e32eb8 drm/i915: Move register white-listing to the common workaround framework e779d6bfdfef drm/i915/selftests: Add tests for GT and engine workaround verification 8d055232d8d8 drm/i915: Verify GT workaround state after GPU init ccb6a2c1f539 drm/i915: Introduce per-engine workaroundsed91418aaeec drm/i915: Record GT workarounds in a list
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10997/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx