Re: [Intel-gfx] [PATCH] drm/i915: Fix workarounds on Gen2-3

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 18/11/2022 17:14, Matt Roper wrote:
On Fri, Nov 18, 2022 at 11:52:49AM +0000, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

In 3653727560d0 ("drm/i915: Simplify internal helper function signature")
I broke the old platforms by not noticing engine workaround init does not
initialize the list on old platforms. Fix it by always initializing which
already does the right thing by mostly not doing anything if there aren't
any workarounds on the list.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Fixes: 3653727560d0 ("drm/i915: Simplify internal helper function signature")
Reported-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/gt/intel_workarounds.c | 5 +----
  1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 213160f29ec3..4d7a01b45e09 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2991,7 +2991,7 @@ general_render_compute_wa_init(struct intel_engine_cs *engine, struct i915_wa_li
  static void
  engine_init_workarounds(struct intel_engine_cs *engine, struct i915_wa_list *wal)
  {
-	if (I915_SELFTEST_ONLY(GRAPHICS_VER(engine->i915) < 4))
+	if (GRAPHICS_VER(engine->i915) < 4)
  		return;

Do we even need this early return at all?  As far as I can see, letting
this function run its course doesn't wind up having any effect or cause
any problems (you still wind up with an empty list).

True, it looks to me like that as well, now that you are pointing it out. Btw originally I was most perplexed by the "selftests only" annotation, but did not find time to go digging through history to figure out why was that even needed.

I left the return as is for now and pushed it to fix the breakage. Will try to revisit this at some point. Thanks for the review!

Regards,

Tvrtko


Regardless,

Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>

engine_fake_wa_init(engine, wal);
@@ -3016,9 +3016,6 @@ void intel_engine_init_workarounds(struct intel_engine_cs *engine)
  {
  	struct i915_wa_list *wal = &engine->wa_list;
- if (GRAPHICS_VER(engine->i915) < 4)
-		return;
-
  	wa_init_start(wal, engine->gt, "engine", engine->name);
  	engine_init_workarounds(engine, wal);
  	wa_init_finish(wal);
--
2.34.1





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux