Hi, On Tue, Mar 11, 2025 at 10:33:14AM +0100, Mikolaj Wasiak wrote: > This test was designed to isolate a bug in tigerlake and dg2 hardware. > The bug was found and fixed in newer generations. > Since we won't support any new hardware with this driver, the test > should now be turned off in the CI to not pollute it with random failures > on previous hardware. > > Fixes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13697 > Please don't make it as tag. Fixes has a specific meaning that means that the patch is fixing a bug introduced by a specific commit and it needs to be backported to stable kernels. You can use a discursive form, saying, e.g. that the issue has been reported here[*]. I can fix it, no need to resend. > Signed-off-by: Mikolaj Wasiak <mikolaj.wasiak@xxxxxxxxx> > Reviewed-by: Chris Wilson <chris.p.wilson@xxxxxxxxxxxxxxx> Welcome back Chris! > --- > v1 -> v2: > - Disabled the test instead of removing it, > - Changed name from 'remove' to 'disable' > > v2 -> v3: > - Fix vocabulary > - Disable this test only on related hardware > > drivers/gpu/drm/i915/gt/selftest_lrc.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c > index 22e750108c5f..c928b4723c38 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c > +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c > @@ -7,6 +7,7 @@ > > #include "gem/i915_gem_internal.h" > > +#include "i915_drv.h" > #include "i915_selftest.h" > #include "intel_engine_heartbeat.h" > #include "intel_engine_pm.h" > @@ -858,6 +859,16 @@ static int live_lrc_timestamp(void *arg) > U32_MAX, > }; > > + /* > + * This test was designed to isolate a hardware bug. > + * The bug was found and fixed in future generations but > + * now the test pollutes our CI on previous generation. > + * > + * https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13697 Please, no links in comments. If you want to refer to something specific in the link you can summarize it rather than adding the link. Is there anything you want to add from the link? Otherwise I am going to remove it. > + */ > + if (GRAPHICS_VER(gt->i915) == 12) > + return 0; yes, much cleaner than the first version. With the changes above: Reviewed-by: Andi Shyti <andi.shyti@xxxxxxxxxx> Thanks, Andi > + > /* > * We want to verify that the timestamp is saved and restore across > * context switches and is monotonic. > -- > 2.48.1 >