Hi Andi,
On 2026-08-28 at 18:18:35 +0200, Andi Shyti wrote:
> Hi Krzysztof,
>
> On Fri, Aug 28, 2026 at 10:03:35AM +0000, Krzysztof Karas wrote:
> > Move away from generic pr prints to device context aware drm
> > versions.
> >
> > Assisted-by: GitHub-Copilot:claude-opus-5
>
> what a ride!
>
> While reviewing everything I was thinking whether it would make
> sense to add an i915_err() like xe and gt.
The idea makes sense, the question is when to introduce this
macro. I have the v4 pretty much ready to send, apart from a few
minor issues I have to fix, but I am not against modifying it to
accomodate i915_* family of prints.
If we agreed on this, then we'd have this new macro ready when
we'd be replacing pr_* prints in other parts of the driver.
I think it would be worth to keep the series in the oven for a
few more days, especially when it is not that critical of a
change. What do you think?
>
> ...
>
> > @@ -540,7 +551,7 @@ static void spinner_kill(struct timer_list *timer)
> > struct spinner_timer *st = timer_container_of(st, timer, timer);
> >
> > igt_spinner_end(&st->spin);
> > - pr_info("%s\n", __func__);
> > + drm_info(st->spin.hws->base.dev, "killing spinner\n");
>
> considering that you kept the __func__ reference everywhere else,
> you could have also kept it here. I like that you have added a
> message, though.
>
> As you are going to resend, anyway, please fix this little nit.
Roger that.
>
> > }
>
> ...
>
> > sort(t, ARRAY_SIZE(t), sizeof(*t), wrap_ktime_compare, NULL);
> > - pr_info("%s: %zd KiB copy: %lld MiB/s\n",
> > - ce->engine->name, sz >> 10,
> > - div64_u64(mul_u32_u32(4 * sz,
> > - 1000 * 1000 * 1000),
> > - t[1] + 2 * t[2] + t[3]) >> 20);
> > + drm_info(&ce->engine->i915->drm, "%s: %zd KiB copy: %lld MiB/s\n",
>
> Not a binding comment: rq->i915 would have made it a bit shorter.
>
> > + ce->engine->name, sz >> 10,
> > + div64_u64(mul_u32_u32(4 * sz, 1000 * 1000 * 1000),
> > + t[1] + 2 * t[2] + t[3]) >> 20);
> > return 0;
> > }
>
> ...
>
> > @@ -1211,7 +1231,7 @@ live_gpu_reset_workarounds(void *arg)
> > if (!lists)
> > return -ENOMEM;
> >
> > - pr_info("Verifying after GPU reset...\n");
> > + drm_info(>->i915->drm, "Verifying after GPU reset...\n");
>
> I'm thinking whether it's better to use gt_info() when we have
> the gt reference.
Sure.
>
> Andi
>
> >
> > igt_global_reset_lock(gt);
> > wakeref = intel_runtime_pm_get(gt->uncore->rpm);
--
Best Regards,
Krzysztof
