Re: [Qemu-devel] [PATCH 3/3] ati-vga: Silence some noisy traces

[BALATON Zoltan]

On Wed, 21 Aug 2019, Gerd Hoffmann wrote:
> On Fri, Aug 16, 2019 at 12:18:09AM +0200, BALATON Zoltan wrote:
> > Some registers are accessed very frequently so exclude these from
> > traces to avoid flooding output with a lot of trace logs when traces
> > are enabled thus helping debugging.
> >
> > Signed-off-by: BALATON Zoltan <address@hidden>
> > ---
> >  hw/display/ati.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/display/ati.c b/hw/display/ati.c
> > index 5e2c4ba4aa..36d2a75f71 100644
> > --- a/hw/display/ati.c
> > +++ b/hw/display/ati.c
> > @@ -489,7 +489,14 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, 
> > unsigned int size)
> >      default:
> >          break;
> >      }
> > -    if (addr < CUR_OFFSET || addr > CUR_CLR1 || ATI_DEBUG_HW_CURSOR) {
> > +    if ((addr < CUR_OFFSET || addr > CUR_CLR1 + 3 || (ATI_DEBUG_HW_CURSOR &&
> > +        (addr >= CUR_OFFSET && addr <= CUR_CLR1 + 3))) &&
> > +        (addr < GEN_INT_CNTL || addr > GEN_INT_STATUS + 3) &&
> > +        (addr < GPIO_MONID || addr > GPIO_MONID + 3) &&
> > +        (addr < AMCGPIO_MASK_MIR || addr > AMCGPIO_EN_MIR + 3) &&
> > +        (addr < 0x908 || addr > 0x90f) && (addr < 0xc4c || addr > 0xc53) &&
> > +        addr != RBBM_STATUS && addr != 0x1714 &&
> > +        addr != 0x7b8 && addr > MM_DATA + 3) {
> >          trace_ati_mm_read(size, addr, ati_reg_name(addr & ~3ULL), val);
>
> I'd suggest to split the trace_ati_mm_read tracepoint, so this can be
> tweaked at runtime without patching the source code.
>
> One tracepoint per register is probably a bit over the top.  Grouping
> registers by function (i2c, crtc, irq, cursor, ...) looks useful to me.

Thanks for the suggestion but I don't have time for that now. Maybe I'll 
ckean it up later in a follow up patch but not sure when. Until then you 
can either drop this one (but that would make tracing useless due to 
flooding log) or take it as it is (as this is just debug logging noe and 
maybe I should have use DPRINTF here as well).

Regards,
BALATON Zoltan