* [Qemu-devel] [PATCH 3/3] ati-vga: Silence some noisy traces
2019-08-15 22:18 [Qemu-devel] [PATCH 0/3] ati-vga fixes for MacOS driver BALATON Zoltan
2019-08-15 22:18 ` [Qemu-devel] [PATCH 2/3] ati-vga: Support unaligned access to hardware cursor registers BALATON Zoltan
@ 2019-08-15 22:18 ` BALATON Zoltan
2019-08-21 9:13 ` Gerd Hoffmann
2019-08-15 22:18 ` [Qemu-devel] [PATCH 1/3] ati-vga: Implement dummy VBlank IRQ BALATON Zoltan
2 siblings, 1 reply; 9+ messages in thread
From: BALATON Zoltan @ 2019-08-15 22:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
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);
}
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) {
--
2.13.7
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH 0/3] ati-vga fixes for MacOS driver
@ 2019-08-15 22:18 BALATON Zoltan
2019-08-15 22:18 ` [Qemu-devel] [PATCH 2/3] ati-vga: Support unaligned access to hardware cursor registers BALATON Zoltan
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: BALATON Zoltan @ 2019-08-15 22:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Hello,
These are some fixes to get MacOS driver closer to working. Patch 1
adds a simple VBlank interrupt (this could be refined later as MacOS
seems to poll it frequently enough to get 100% CPU usage when
enabled). Patch 2 fixes problems with mouse pointer color and movement
due to byte and word access to HW cursor regs and Patch 3 removes some
annoying trace messages that are frequent enough to flood the log when
traces are enabled.
With these fixes MacOS shows desktop and the mouse pointer can be
moved around but it does not seem to fully boot yet as nothing can be
clicked so it may still miss something somewhere. (Also to get to this
point one needs to run an FCode ROM which needs patches to OpenBIOS
currently.)
Regards,
BALATON Zoltan
BALATON Zoltan (3):
ati-vga: Implement dummy VBlank IRQ
ati-vga: Support unaligned access to hardware cursor registers
ati-vga: Silence some noisy traces
hw/display/ati.c | 147 +++++++++++++++++++++++++++++++++++++++-----------
hw/display/ati_dbg.c | 1 +
hw/display/ati_int.h | 4 ++
hw/display/ati_regs.h | 6 +++
4 files changed, 128 insertions(+), 30 deletions(-)
--
2.13.7
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH 1/3] ati-vga: Implement dummy VBlank IRQ
2019-08-15 22:18 [Qemu-devel] [PATCH 0/3] ati-vga fixes for MacOS driver BALATON Zoltan
2019-08-15 22:18 ` [Qemu-devel] [PATCH 2/3] ati-vga: Support unaligned access to hardware cursor registers BALATON Zoltan
2019-08-15 22:18 ` [Qemu-devel] [PATCH 3/3] ati-vga: Silence some noisy traces BALATON Zoltan
@ 2019-08-15 22:18 ` BALATON Zoltan
2019-08-21 9:14 ` Gerd Hoffmann
2 siblings, 1 reply; 9+ messages in thread
From: BALATON Zoltan @ 2019-08-15 22:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
The MacOS driver exits if the card does not have an interrupt. If we
set PCI_INTERRUPT_PIN to 1 then it enables VBlank interrupts and it
boots but the mouse pointer cannot be moved. This patch implements a
dummy VBlank interrupt triggered by a 60 Hz timer. With this the
pointer now moves but MacOS still hangs somewhere before completely
finishing boot.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
hw/display/ati_dbg.c | 1 +
hw/display/ati_int.h | 4 ++++
hw/display/ati_regs.h | 6 ++++++
4 files changed, 55 insertions(+)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index a365e2455d..87ad18664d 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -243,6 +243,21 @@ static uint64_t ati_i2c(bitbang_i2c_interface *i2c, uint64_t data, int base)
return data;
}
+static void ati_vga_update_irq(ATIVGAState *s)
+{
+ pci_set_irq(&s->dev, !!(s->regs.gen_int_status & s->regs.gen_int_cntl));
+}
+
+static void ati_vga_vblank_irq(void *opaque)
+{
+ ATIVGAState *s = opaque;
+
+ timer_mod(&s->vblank_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
+ NANOSECONDS_PER_SECOND / 60);
+ s->regs.gen_int_status |= CRTC_VBLANK_INT;
+ ati_vga_update_irq(s);
+}
+
static inline uint64_t ati_reg_read_offs(uint32_t reg, int offs,
unsigned int size)
{
@@ -283,6 +298,12 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
addr - (BIOS_0_SCRATCH + i * 4), size);
break;
}
+ case GEN_INT_CNTL:
+ val = s->regs.gen_int_cntl;
+ break;
+ case GEN_INT_STATUS:
+ val = s->regs.gen_int_status;
+ break;
case CRTC_GEN_CNTL ... CRTC_GEN_CNTL + 3:
val = ati_reg_read_offs(s->regs.crtc_gen_cntl,
addr - CRTC_GEN_CNTL, size);
@@ -512,6 +533,21 @@ static void ati_mm_write(void *opaque, hwaddr addr,
addr - (BIOS_0_SCRATCH + i * 4), data, size);
break;
}
+ case GEN_INT_CNTL:
+ s->regs.gen_int_cntl = data;
+ if (data & CRTC_VBLANK_INT) {
+ ati_vga_vblank_irq(s);
+ } else {
+ timer_del(&s->vblank_timer);
+ ati_vga_update_irq(s);
+ }
+ break;
+ case GEN_INT_STATUS:
+ data &= (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF ?
+ 0x000f040fUL : 0xfc080effUL);
+ s->regs.gen_int_status &= ~data;
+ ati_vga_update_irq(s);
+ break;
case CRTC_GEN_CNTL ... CRTC_GEN_CNTL + 3:
{
uint32_t val = s->regs.crtc_gen_cntl;
@@ -902,12 +938,19 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp)
pci_register_bar(dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &vga->vram);
pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
pci_register_bar(dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mm);
+
+ /* most interrupts are not yet emulated but MacOS needs at least VBlank */
+ dev->config[PCI_INTERRUPT_PIN] = 1;
+ timer_init_ns(&s->vblank_timer, QEMU_CLOCK_VIRTUAL, ati_vga_vblank_irq, s);
}
static void ati_vga_reset(DeviceState *dev)
{
ATIVGAState *s = ATI_VGA(dev);
+ timer_del(&s->vblank_timer);
+ ati_vga_update_irq(s);
+
/* reset vga */
vga_common_reset(&s->vga);
s->mode = VGA_MODE;
@@ -917,6 +960,7 @@ static void ati_vga_exit(PCIDevice *dev)
{
ATIVGAState *s = ATI_VGA(dev);
+ timer_del(&s->vblank_timer);
graphic_console_close(s->vga.con);
}
diff --git a/hw/display/ati_dbg.c b/hw/display/ati_dbg.c
index 7e59c41ac2..0ebbd36f14 100644
--- a/hw/display/ati_dbg.c
+++ b/hw/display/ati_dbg.c
@@ -16,6 +16,7 @@ static struct ati_regdesc ati_reg_names[] = {
{"BUS_CNTL", 0x0030},
{"BUS_CNTL1", 0x0034},
{"GEN_INT_CNTL", 0x0040},
+ {"GEN_INT_STATUS", 0x0044},
{"CRTC_GEN_CNTL", 0x0050},
{"CRTC_EXT_CNTL", 0x0054},
{"DAC_CNTL", 0x0058},
diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index 5b4d3be1e6..2a16708e4f 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -9,6 +9,7 @@
#ifndef ATI_INT_H
#define ATI_INT_H
+#include "qemu/timer.h"
#include "hw/pci/pci.h"
#include "hw/i2c/bitbang_i2c.h"
#include "vga_int.h"
@@ -33,6 +34,8 @@
typedef struct ATIVGARegs {
uint32_t mm_index;
uint32_t bios_scratch[8];
+ uint32_t gen_int_cntl;
+ uint32_t gen_int_status;
uint32_t crtc_gen_cntl;
uint32_t crtc_ext_cntl;
uint32_t dac_cntl;
@@ -89,6 +92,7 @@ typedef struct ATIVGAState {
uint16_t cursor_size;
uint32_t cursor_offset;
QEMUCursor *cursor;
+ QEMUTimer vblank_timer;
bitbang_i2c_interface bbi2c;
MemoryRegion io;
MemoryRegion mm;
diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
index 02046e97c2..ebd37ee30d 100644
--- a/hw/display/ati_regs.h
+++ b/hw/display/ati_regs.h
@@ -34,6 +34,7 @@
#define BUS_CNTL 0x0030
#define BUS_CNTL1 0x0034
#define GEN_INT_CNTL 0x0040
+#define GEN_INT_STATUS 0x0044
#define CRTC_GEN_CNTL 0x0050
#define CRTC_EXT_CNTL 0x0054
#define DAC_CNTL 0x0058
@@ -316,6 +317,11 @@
#define XPLL_FB_DIV_MASK 0x0000FF00
#define X_MPLL_REF_DIV_MASK 0x000000FF
+/* GEN_INT_CNTL) */
+#define CRTC_VBLANK_INT 0x00000001
+#define CRTC_VLINE_INT 0x00000002
+#define CRTC_VSYNC_INT 0x00000004
+
/* Config control values (CONFIG_CNTL) */
#define APER_0_ENDIAN 0x00000003
#define APER_1_ENDIAN 0x0000000c
--
2.13.7
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH 2/3] ati-vga: Support unaligned access to hardware cursor registers
2019-08-15 22:18 [Qemu-devel] [PATCH 0/3] ati-vga fixes for MacOS driver BALATON Zoltan
@ 2019-08-15 22:18 ` BALATON Zoltan
2019-08-21 9:08 ` Gerd Hoffmann
2019-08-15 22:18 ` [Qemu-devel] [PATCH 3/3] ati-vga: Silence some noisy traces BALATON Zoltan
2019-08-15 22:18 ` [Qemu-devel] [PATCH 1/3] ati-vga: Implement dummy VBlank IRQ BALATON Zoltan
2 siblings, 1 reply; 9+ messages in thread
From: BALATON Zoltan @ 2019-08-15 22:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
This fixes horizontal mouse movement and pointer color with MacOS that
writes these registers with access size less than 4 so previously only
the last portion of access was effective overwriting previous partial
writes.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati.c | 87 +++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 58 insertions(+), 29 deletions(-)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index 87ad18664d..5e2c4ba4aa 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -385,22 +385,28 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
case 0xf00 ... 0xfff:
val = pci_default_read_config(&s->dev, addr - 0xf00, size);
break;
- case CUR_OFFSET:
- val = s->regs.cur_offset;
- break;
- case CUR_HORZ_VERT_POSN:
- val = s->regs.cur_hv_pos;
- val |= s->regs.cur_offset & BIT(31);
+ case CUR_OFFSET ... CUR_OFFSET + 3:
+ val = ati_reg_read_offs(s->regs.cur_offset, addr - CUR_OFFSET, size);
+ break;
+ case CUR_HORZ_VERT_POSN ... CUR_HORZ_VERT_POSN + 3:
+ val = ati_reg_read_offs(s->regs.cur_hv_pos,
+ addr - CUR_HORZ_VERT_POSN, size);
+ if (addr + size > CUR_HORZ_VERT_POSN + 3) {
+ val |= (s->regs.cur_offset & BIT(31)) >> (4 - size);
+ }
break;
- case CUR_HORZ_VERT_OFF:
- val = s->regs.cur_hv_offs;
- val |= s->regs.cur_offset & BIT(31);
+ case CUR_HORZ_VERT_OFF ... CUR_HORZ_VERT_OFF + 3:
+ val = ati_reg_read_offs(s->regs.cur_hv_offs,
+ addr - CUR_HORZ_VERT_OFF, size);
+ if (addr + size > CUR_HORZ_VERT_OFF + 3) {
+ val |= (s->regs.cur_offset & BIT(31)) >> (4 - size);
+ }
break;
- case CUR_CLR0:
- val = s->regs.cur_color0;
+ case CUR_CLR0 ... CUR_CLR0 + 3:
+ val = ati_reg_read_offs(s->regs.cur_color0, addr - CUR_CLR0, size);
break;
- case CUR_CLR1:
- val = s->regs.cur_color1;
+ case CUR_CLR1 ... CUR_CLR1 + 3:
+ val = ati_reg_read_offs(s->regs.cur_color1, addr - CUR_CLR1, size);
break;
case DST_OFFSET:
val = s->regs.dst_offset;
@@ -672,48 +678,71 @@ static void ati_mm_write(void *opaque, hwaddr addr,
case 0xf00 ... 0xfff:
/* read-only copy of PCI config space so ignore writes */
break;
- case CUR_OFFSET:
- if (s->regs.cur_offset != (data & 0x87fffff0)) {
- s->regs.cur_offset = data & 0x87fffff0;
+ case CUR_OFFSET ... CUR_OFFSET + 3:
+ {
+ uint32_t t = s->regs.cur_offset;
+
+ ati_reg_write_offs(&t, addr - CUR_OFFSET, data, size);
+ t &= 0x87fffff0;
+ if (s->regs.cur_offset != t) {
+ s->regs.cur_offset = t;
ati_cursor_define(s);
}
break;
- case CUR_HORZ_VERT_POSN:
- s->regs.cur_hv_pos = data & 0x3fff0fff;
- if (data & BIT(31)) {
- s->regs.cur_offset |= data & BIT(31);
+ }
+ case CUR_HORZ_VERT_POSN ... CUR_HORZ_VERT_POSN + 3:
+ {
+ uint32_t t = s->regs.cur_hv_pos | (s->regs.cur_offset & BIT(31));
+
+ ati_reg_write_offs(&t, addr - CUR_HORZ_VERT_POSN, data, size);
+ s->regs.cur_hv_pos = t & 0x3fff0fff;
+ if (t & BIT(31)) {
+ s->regs.cur_offset |= t & BIT(31);
} else if (s->regs.cur_offset & BIT(31)) {
s->regs.cur_offset &= ~BIT(31);
ati_cursor_define(s);
}
if (!s->cursor_guest_mode &&
- (s->regs.crtc_gen_cntl & CRTC2_CUR_EN) && !(data & BIT(31))) {
+ (s->regs.crtc_gen_cntl & CRTC2_CUR_EN) && !(t & BIT(31))) {
dpy_mouse_set(s->vga.con, s->regs.cur_hv_pos >> 16,
s->regs.cur_hv_pos & 0xffff, 1);
}
break;
+ }
case CUR_HORZ_VERT_OFF:
- s->regs.cur_hv_offs = data & 0x3f003f;
- if (data & BIT(31)) {
- s->regs.cur_offset |= data & BIT(31);
+ {
+ uint32_t t = s->regs.cur_hv_offs | (s->regs.cur_offset & BIT(31));
+
+ ati_reg_write_offs(&t, addr - CUR_HORZ_VERT_OFF, data, size);
+ s->regs.cur_hv_offs = t & 0x3f003f;
+ if (t & BIT(31)) {
+ s->regs.cur_offset |= t & BIT(31);
} else if (s->regs.cur_offset & BIT(31)) {
s->regs.cur_offset &= ~BIT(31);
ati_cursor_define(s);
}
break;
- case CUR_CLR0:
- if (s->regs.cur_color0 != (data & 0xffffff)) {
- s->regs.cur_color0 = data & 0xffffff;
+ }
+ case CUR_CLR0 ... CUR_CLR0 + 3:
+ {
+ uint32_t t = s->regs.cur_color0;
+
+ ati_reg_write_offs(&t, addr - CUR_CLR0, data, size);
+ t &= 0xffffff;
+ if (s->regs.cur_color0 != t) {
+ s->regs.cur_color0 = t;
ati_cursor_define(s);
}
break;
- case CUR_CLR1:
+ }
+ case CUR_CLR1 ... CUR_CLR1 + 3:
/*
* Update cursor unconditionally here because some clients set up
* other registers before actually writing cursor data to memory at
* offset so we would miss cursor change unless always updating here
*/
- s->regs.cur_color1 = data & 0xffffff;
+ ati_reg_write_offs(&s->regs.cur_color1, addr - CUR_CLR1, data, size);
+ s->regs.cur_color1 &= 0xffffff;
ati_cursor_define(s);
break;
case DST_OFFSET:
--
2.13.7
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] ati-vga: Support unaligned access to hardware cursor registers
2019-08-15 22:18 ` [Qemu-devel] [PATCH 2/3] ati-vga: Support unaligned access to hardware cursor registers BALATON Zoltan
@ 2019-08-21 9:08 ` Gerd Hoffmann
2019-08-21 11:18 ` BALATON Zoltan
0 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2019-08-21 9:08 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: qemu-devel
Hi,
> @@ -672,48 +678,71 @@ static void ati_mm_write(void *opaque, hwaddr addr,
> case 0xf00 ... 0xfff:
> /* read-only copy of PCI config space so ignore writes */
> break;
> - case CUR_OFFSET:
> - if (s->regs.cur_offset != (data & 0x87fffff0)) {
> - s->regs.cur_offset = data & 0x87fffff0;
> + case CUR_OFFSET ... CUR_OFFSET + 3:
> + {
> + uint32_t t = s->regs.cur_offset;
> +
> + ati_reg_write_offs(&t, addr - CUR_OFFSET, data, size);
> + t &= 0x87fffff0;
> + if (s->regs.cur_offset != t) {
> + s->regs.cur_offset = t;
Repeated pattern. I'd suggest to add a "wmask" parameter to
ati_reg_write_offs. Maybe also make it return true/false depending
on whenever the value did change or not.
cheers,
Gerd
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] ati-vga: Silence some noisy traces
2019-08-15 22:18 ` [Qemu-devel] [PATCH 3/3] ati-vga: Silence some noisy traces BALATON Zoltan
@ 2019-08-21 9:13 ` Gerd Hoffmann
2019-08-21 11:22 ` BALATON Zoltan
0 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2019-08-21 9:13 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] ati-vga: Implement dummy VBlank IRQ
2019-08-15 22:18 ` [Qemu-devel] [PATCH 1/3] ati-vga: Implement dummy VBlank IRQ BALATON Zoltan
@ 2019-08-21 9:14 ` Gerd Hoffmann
0 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2019-08-21 9:14 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: qemu-devel
On Fri, Aug 16, 2019 at 12:18:09AM +0200, BALATON Zoltan wrote:
> The MacOS driver exits if the card does not have an interrupt. If we
> set PCI_INTERRUPT_PIN to 1 then it enables VBlank interrupts and it
> boots but the mouse pointer cannot be moved. This patch implements a
> dummy VBlank interrupt triggered by a 60 Hz timer. With this the
> pointer now moves but MacOS still hangs somewhere before completely
> finishing boot.
Queued patch.
thanks,
Gerd
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] ati-vga: Support unaligned access to hardware cursor registers
2019-08-21 9:08 ` Gerd Hoffmann
@ 2019-08-21 11:18 ` BALATON Zoltan
0 siblings, 0 replies; 9+ messages in thread
From: BALATON Zoltan @ 2019-08-21 11:18 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
Hello,
On Wed, 21 Aug 2019, Gerd Hoffmann wrote:
>> @@ -672,48 +678,71 @@ static void ati_mm_write(void *opaque, hwaddr addr,
>> case 0xf00 ... 0xfff:
>> /* read-only copy of PCI config space so ignore writes */
>> break;
>> - case CUR_OFFSET:
>> - if (s->regs.cur_offset != (data & 0x87fffff0)) {
>> - s->regs.cur_offset = data & 0x87fffff0;
>> + case CUR_OFFSET ... CUR_OFFSET + 3:
>> + {
>> + uint32_t t = s->regs.cur_offset;
>> +
>> + ati_reg_write_offs(&t, addr - CUR_OFFSET, data, size);
>> + t &= 0x87fffff0;
>> + if (s->regs.cur_offset != t) {
>> + s->regs.cur_offset = t;
>
> Repeated pattern. I'd suggest to add a "wmask" parameter to
> ati_reg_write_offs. Maybe also make it return true/false depending
> on whenever the value did change or not.
This is a pattern in these HW cursor related regs but other callers of
write_offs don't do that (currently there are one more of the CUR_* regs
vs. others 5 to 4 but there may be other uses later as several regs
support less than 32 bit access). It would also break symmetry between
read_offs and write_offs so I think I'd leave this off write_offs for now
unless new callers in the future will also need wmask. (It you insist I
could make it a macro for CUR_* regs but not sure that would improve it
much.)
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] ati-vga: Silence some noisy traces
2019-08-21 9:13 ` Gerd Hoffmann
@ 2019-08-21 11:22 ` BALATON Zoltan
0 siblings, 0 replies; 9+ messages in thread
From: BALATON Zoltan @ 2019-08-21 11:22 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-08-21 11:42 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-15 22:18 [Qemu-devel] [PATCH 0/3] ati-vga fixes for MacOS driver BALATON Zoltan
2019-08-15 22:18 ` [Qemu-devel] [PATCH 2/3] ati-vga: Support unaligned access to hardware cursor registers BALATON Zoltan
2019-08-21 9:08 ` Gerd Hoffmann
2019-08-21 11:18 ` BALATON Zoltan
2019-08-15 22:18 ` [Qemu-devel] [PATCH 3/3] ati-vga: Silence some noisy traces BALATON Zoltan
2019-08-21 9:13 ` Gerd Hoffmann
2019-08-21 11:22 ` BALATON Zoltan
2019-08-15 22:18 ` [Qemu-devel] [PATCH 1/3] ati-vga: Implement dummy VBlank IRQ BALATON Zoltan
2019-08-21 9:14 ` Gerd Hoffmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).