[PATCH v5 1/8] drm/xe/guc: Remove spurious line feed in debug print
Souza, Jose
jose.souza at intel.com
Wed Jul 31 20:14:23 UTC 2024
On Wed, 2024-07-31 at 12:56 -0700, John Harrison wrote:
> On 7/30/2024 02:14, Michal Wajdeczko wrote:
> >
> > On 30.07.2024 01:17, John.C.Harrison at Intel.com wrote:
> > > From: John Harrison <John.C.Harrison at Intel.com>
> > >
> > > Including line feeds at the start of a debug print messes up the
> > > output when sent to dmesg. The break actually appears between all the
> > > usefu
> > typo
> >
> > > prefix information and the actual string being printed. In this
> > > case, each block of data has a very clear start line and an extra
> > > delimeter is really not necessary. So don't do it.
> > >
> > > Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> > > Reviewed-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > there was some discussion about merging this one without a conclusion
> >
> > [1] https://patchwork.freedesktop.org/patch/601018/?series=135447&rev=1
> The last comment was for Mesa people to shout if it would be a problem
> and no-one shouted, so...
>
> However, I would strongly argue that devcoredump exact layout and
> content cannot be considered UABI because it is going to change as the
> driver changes. Some of the information being printed is internal driver
> state. Driver internals can never be UABI. If there are userland tools
> parsing the dump then those tools have to be able to adapt to changing
> core dump formats. There is also the argument that we are still in
> force-probe so there is no fixed UABI yet anyway. So now is the time to
> get the formatting as good as possible before officially going live.
I don't think KMD can freely break fundamental UMD tools, during my time in KMD team it was not even accepted to break the behavior of a sysfs only
use by IGT display tests.
Like I said in the previous version, I agree with the change if you add one line breaker in the end of guc_ctb_snapshot_print(), with that: Reviewed-
by: José Roberto de Souza <jose.souza at intel.com>
having a break line between sub-sections is good for readability:
**** Xe Device Coredump ****
kernel: 6.9.0-rc6-zeh-xe+
module: xe
Snapshot time: 1715877420.647211377
Uptime: 70684.605982665
PCI ID: 0x9a49
PCI revision: 0x01
GT id: 0
Type: main
IP ver: 0.0.0
CS reference clock: 19200000
**** GuC CT ****
H2G CTB (all sizes in DW):
size: 1024
resv_space: 0
head: 978
tail: 599
space: 378
broken: 0
head (memory): 599
tail (memory): 599
status (memory): 0x0
G2H CTB (all sizes in DW):
size: 4096
resv_space: 1024
head: 626
tail: 0
space: 3071
broken: 0
head (memory): 626
tail (memory): 626
status (memory): 0x0
g2h outstanding: 0
GuC ID: 9
Name: rcs9
Class: 0
Logical mask: 0x1
Width: 1
Ref: 4
Timeout: 0 (ms)
Timeslice: 1000 (us)
Preempt timeout: 640000 (us)
HW Context Desc: 0x01480000
LRC Head: (memory) 280
LRC Tail: (internal) 552, (memory) 552
Start seqno: (memory) -125
Seqno: (memory) -126
[HWSP].length: 0x1000
>
> John.
>
>
> >
> > > ---
> > > drivers/gpu/drm/xe/xe_guc_ct.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> > > index beeeb120d1fc..422c3f5c87d8 100644
> > > --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> > > +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> > > @@ -1515,7 +1515,7 @@ void xe_guc_ct_snapshot_print(struct xe_guc_ct_snapshot *snapshot,
> > > drm_puts(p, "H2G CTB (all sizes in DW):\n");
> > > guc_ctb_snapshot_print(&snapshot->h2g, p);
> > >
> > > - drm_puts(p, "\nG2H CTB (all sizes in DW):\n");
> > > + drm_puts(p, "G2H CTB (all sizes in DW):\n");
> > > guc_ctb_snapshot_print(&snapshot->g2h, p);
> > >
> > > drm_printf(p, "\tg2h outstanding: %d\n",
>
More information about the Intel-xe
mailing list