Re: [PATCH 1/9] drm/xe: Remove double new line in devcoredump

["Summers, Stuart" <stuart.summers@intel.com>]

On Mon, 2024-01-22 at 15:20 -0500, Rodrigo Vivi wrote:
> On Mon, Jan 22, 2024 at 06:40:14PM +0000, Souza, Jose wrote:
> > On Mon, 2024-01-22 at 18:28 +0000, Summers, Stuart wrote:
> > > On Mon, 2024-01-22 at 09:04 -0800, José Roberto de Souza wrote:
> > > > Right now devcoredump has a new line between '**** GuC CT ****'
> > > > and
> > > > 'H2G CTB (all sizes in DW):' while other sections don't have.
> > > > 
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Maarten Lankhorst <dev@lankhorst.se>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/xe/xe_guc_ct.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c
> > > > b/drivers/gpu/drm/xe/xe_guc_ct.c
> > > > index ee5d99456aebc..0c07355690948 100644
> > > > --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> > > > +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> > > > @@ -1366,7 +1366,7 @@ void xe_guc_ct_snapshot_print(struct
> > > > xe_guc_ct_snapshot *snapshot,
> > > >                 return;
> > > >  
> > > >         if (snapshot->ct_enabled) {
> > > > -               drm_puts(p, "\nH2G CTB (all sizes in DW):\n");
> > > > +               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");
> > > 
> > > Why not here too?
> > > 
> > > Also just visually IMO it is nice to have the extra spacing here.
> > > I
> > > don't have a strong opinion though.
> > 
> > This sections outputs like below what looks fine to me:
> > 
> > **** GuC CT ****
> > H2G CTB (all sizes in DW):
> >         size: 1024
> >         resv_space: 0
> >         head: 0
> >         tail: 424
> >         space: 599
> >         broken: 0
> >         head (memory): 424
> >         tail (memory): 424
> >         status (memory): 0x0
> > 
> > G2H CTB (all sizes in DW):
> >         size: 4096
> >         resv_space: 1024
> >         head: 137
> >         tail: 0
> >         space: 3071
> >         broken: 0
> >         head (memory): 137
> >         tail (memory): 137
> >         status (memory): 0x0
> >         g2h outstanding: 0
> > 
> > GuC ID: 2
> >         Name: rcs2
> >         Class: 0
> >         Logical mask: 0x1
> >         Width: 1
> >         Ref: 2
> >         Timeout: 5000 (ms)
> >         Timeslice: 1000 (us)
> >         Preempt timeout: 640000 (us)
> >         HW Context Desc: 0x00435000
> >         LRC Head: (memory) 416
> >         LRC Tail: (internal) 552, (memory) 552
> >         Start seqno: (memory) -124
> >         Seqno: (memory) -125
> >         Schedule State: 0x3
> >         Flags: 0x8
> 
> makes sense to me as well. But without the extra line it would
> also work I guess.
> up to you:
> 

Yeah no real problem here either:
Reviewed-by: Stuart Summers <stuart.summers@intel.com>

> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> 
> > 
> > > 
> > > Thanks,
> > > Stuart
> > > 
> > > > @@ -1375,7 +1375,7 @@ void xe_guc_ct_snapshot_print(struct
> > > > xe_guc_ct_snapshot *snapshot,
> > > >                 drm_printf(p, "\tg2h outstanding: %d\n",
> > > >                            snapshot->g2h_outstanding);
> > > >         } else {
> > > > -               drm_puts(p, "\nCT disabled\n");
> > > > +               drm_puts(p, "CT disabled\n");
> > > >         }
> > > >  }
> > > >  
> > > 
> >