diff mbox series

[3/3] ati-vga: Silence some noisy traces

Message ID 489ce252f9d5f902f7d240ff9895e77bb335f1a9.1565907489.git.balaton@eik.bme.hu
State New
Headers show
Series ati-vga fixes for MacOS driver | expand

Commit Message

BALATON Zoltan Aug. 15, 2019, 10:18 p.m. UTC
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 <balaton@eik.bme.hu>
---
 hw/display/ati.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Gerd Hoffmann Aug. 21, 2019, 9:13 a.m. UTC | #1
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 <balaton@eik.bme.hu>
> ---
>  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.

cheers,
  Gerd
BALATON Zoltan Aug. 21, 2019, 11:22 a.m. UTC | #2
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 <balaton@eik.bme.hu>
>> ---
>>  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
diff mbox series

Patch

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);
     }
     return val;
@@ -511,7 +518,14 @@  static void ati_mm_write(void *opaque, hwaddr addr,
 {
     ATIVGAState *s = opaque;
 
-    if (addr < CUR_OFFSET || addr > CUR_CLR1 || ATI_DEBUG_HW_CURSOR) {
+    if ((((addr < CUR_OFFSET || addr > CUR_CLR1 + 3) &&
+          addr != CRTC_GEN_CNTL + 2) || (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 != 0x1714 && addr > MM_DATA + 3) {
         trace_ati_mm_write(size, addr, ati_reg_name(addr & ~3ULL), data);
     }
     switch (addr) {