Re: [PATCH v6 2/3] drm/tyr: add GPU reset infrastructure
Daniel Almeida <[email protected]>
| Newsgroups | org.kernel.vger.rust-for-linux,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
> Why do you think it is useful to print "Starting GPU reset." and "GPU reset > completed." with dev_info()? How does this help users or help with debugging? > > In general, if drivers work properly they should remain silent. A successful > reset means it worked properly. What you rather want to print, likely with > dev_warn() or dev_err(), is what caused the GPU reset to be performed in the > first place. I agree, no argument from me there. The problem is, when the GPU is resetting, things are far from "working". In fact, things went so bad somewhere that it brought the device down for everybody, and the system is taking steps to (try to) bring it up again. As a user, I very much appreciated this when games went down, it made me think that the problem was important enough to report, and also likely explained why, in that particular hardware, things didn't really work 100% well afterwards, which prompted me to reboot the machine. Two outcomes could have come from that, a) filing a bug on the game itself, and b) telling the kernel guys that their reset code wasn't really working 100% well either. I do agree that "Starting GPU reset" is frivolous, but at least a message saying that a reset took place, either sucessfully or not, is better than silence. In fact, isn't this exactly what AMDGPU does? [2]. [2]: https://elixir.bootlin.com/linux/v7.2/source/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c#L5094 > On 21 Aug 2026, at 14:01, Danilo Krummrich <[email protected]> wrote: > > On Fri Aug 21, 2026 at 6:39 PM CEST, Daniel Almeida wrote: >> My point is that resetting the GPU should not be a “debug” trace, but rather >> something that should hit dmesg unconditionally. Of all things that are >> printed on a driver, a “hey the GPU is being reset because something crashed” >> is something that should always show up IMHO. > > Please see my reply in [1]. If something crashed, please feel free to add a > dev_warn() or dev_err() print about the exact error condition. But please do not > add dev_info() prints about things that were successful and just bloat dmesg. > > In this specific case you want the caller of ResetHandle::schedule() to print > about *why* the reset work was scheduled. You also already have a dev_err() > print when the GPU reset failed. So, at this point you're good already. I guess that also works, so long as it says that the GPU is resetting? i.e.: it’s not only a fault or something, but also a message saying “this fault” (or whatever it was) is causing a reset. > > Additional prints about when exactly the reset work starts and when it completes > is stuff for dev_dbg(). > > [1] https://lore.kernel.org/all/[email protected]/