[Qemu-devel] [PATCH] ati-vga: Implement DDC and EDID info from monitor

[Qemu-devel] [PATCH] ati-vga: Implement DDC and EDID info from monitor

[BALATON Zoltan]

This adds DDC support to ati-vga and connects i2c-ddc to provide EDID
info that is read by guests to find available screen modes. Not sure
if this is 100% correct yet but at least MorphOS is happy with it and
starts in a high resolution mode instead of 640x480 (although its
splash screen is still not correct). Linux needs support from VESA
vgabios, it seems to be missing INT10 0x4F15 function (see
https://gitlab.freedesktop.org/xorg/xserver/blob/master/hw/xfree86/vbe/vbe.c)
without which no DDC is available that also prevents loading the
accelerated X driver.

Besides, this depends on bitbang_i2c.h which is now in hw/i2c so if
including it from there is not desirable that may need to be moved
somewhere.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/Kconfig   |  2 ++
 hw/display/ati.c     | 24 ++++++++++++++++++++++--
 hw/display/ati_int.h |  3 +++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 86c1d544c5..f8d65802a9 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -112,3 +112,5 @@ config ATI_VGA
     default y if PCI_DEVICES
     depends on PCI
     select VGA
+    select BITBANG_I2C
+    select DDC
diff --git a/hw/display/ati.c b/hw/display/ati.c
index 8322f52aff..260cd803d8 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -24,6 +24,8 @@
 #include "qapi/error.h"
 #include "hw/hw.h"
 #include "ui/console.h"
+#include "hw/i2c/i2c-ddc.h"
+#include "../i2c/bitbang_i2c.h"
 #include "trace.h"
 
 #define ATI_DEBUG_HW_CURSOR 0
@@ -267,7 +269,9 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
     case DAC_CNTL:
         val = s->regs.dac_cntl;
         break;
-/*    case GPIO_MONID: FIXME hook up DDC I2C here */
+    case GPIO_MONID:
+        val = s->regs.gpio_monid;
+        break;
     case PALETTE_INDEX:
         /* FIXME unaligned access */
         val = vga_ioport_read(&s->vga, VGA_PEL_IR) << 16;
@@ -501,7 +505,17 @@ static void ati_mm_write(void *opaque, hwaddr addr,
         s->regs.dac_cntl = data & 0xffffe3ff;
         s->vga.dac_8bit = !!(data & DAC_8BIT_EN);
         break;
-/*    case GPIO_MONID: FIXME hook up DDC I2C here */
+    case GPIO_MONID:
+        s->regs.gpio_monid = data & 0x0f0f000f;
+        if (data & BIT(2) << 24) {
+            s->regs.gpio_monid |= !!(data & BIT(2)) << 10;
+            bitbang_i2c_set(s->bbi2c, BITBANG_I2C_SCL, (data & BIT(2)) != 0);
+        }
+        if (data & BIT(1) << 24) {
+            s->regs.gpio_monid |= bitbang_i2c_set(s->bbi2c, BITBANG_I2C_SDA,
+                                                  (data & BIT(1)) != 0) << 9;
+        }
+        break;
     case PALETTE_INDEX ... PALETTE_INDEX + 3:
         if (size == 4) {
             vga_ioport_write(&s->vga, VGA_PEL_IR, (data >> 16) & 0xff);
@@ -792,6 +806,12 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp)
         vga->cursor_draw_line = ati_cursor_draw_line;
     }
 
+    /* ddc, edid */
+    I2CBus *i2cbus = i2c_init_bus(DEVICE(s), "ati-vga.ddc");
+    s->bbi2c = bitbang_i2c_init(i2cbus);
+    I2CSlave *i2cddc = I2C_SLAVE(qdev_create(BUS(i2cbus), TYPE_I2CDDC));
+    i2c_set_slave_address(i2cddc, 0x50);
+
     /* mmio register space */
     memory_region_init_io(&s->mm, OBJECT(s), &ati_mm_ops, s,
                           "ati.mmregs", 0x4000);
diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index a6f3e20e63..8df00efd93 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -11,6 +11,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/pci/pci.h"
+#include "hw/i2c/i2c.h"
 #include "vga_int.h"
 
 /*#define DEBUG_ATI*/
@@ -36,6 +37,7 @@ typedef struct ATIVGARegs {
     uint32_t crtc_gen_cntl;
     uint32_t crtc_ext_cntl;
     uint32_t dac_cntl;
+    uint32_t gpio_monid;
     uint32_t crtc_h_total_disp;
     uint32_t crtc_h_sync_strt_wid;
     uint32_t crtc_v_total_disp;
@@ -84,6 +86,7 @@ typedef struct ATIVGAState {
     uint16_t cursor_size;
     uint32_t cursor_offset;
     QEMUCursor *cursor;
+    bitbang_i2c_interface *bbi2c;
     MemoryRegion io;
     MemoryRegion mm;
     ATIVGARegs regs;
-- 
2.13.7

Re: [Qemu-devel] [PATCH] ati-vga: Implement DDC and EDID info from monitor

[Gerd Hoffmann]

On Sun, Mar 10, 2019 at 12:22:17AM +0100, BALATON Zoltan wrote:
> This adds DDC support to ati-vga and connects i2c-ddc to provide EDID
> info that is read by guests to find available screen modes. Not sure
> if this is 100% correct yet but at least MorphOS is happy with it and
> starts in a high resolution mode instead of 640x480 (although its
> splash screen is still not correct).

> Linux needs support from VESA
> vgabios, it seems to be missing INT10 0x4F15 function (see
> https://gitlab.freedesktop.org/xorg/xserver/blob/master/hw/xfree86/vbe/vbe.c)
> without which no DDC is available that also prevents loading the
> accelerated X driver.

radeonfb doesn't pick up the edid either.
There seem to be four i2c busses though:

void radeon_create_i2c_busses(struct radeonfb_info *rinfo)
{
	rinfo->i2c[0].rinfo	= rinfo;
	rinfo->i2c[0].ddc_reg	= GPIO_MONID;
#ifndef CONFIG_PPC
	rinfo->i2c[0].adapter.class = I2C_CLASS_HWMON;
#endif
	radeon_setup_i2c_bus(&rinfo->i2c[0], "monid");

	rinfo->i2c[1].rinfo	= rinfo;
	rinfo->i2c[1].ddc_reg	= GPIO_DVI_DDC;
	radeon_setup_i2c_bus(&rinfo->i2c[1], "dvi");

	rinfo->i2c[2].rinfo	= rinfo;
	rinfo->i2c[2].ddc_reg	= GPIO_VGA_DDC;
	radeon_setup_i2c_bus(&rinfo->i2c[2], "vga");

	rinfo->i2c[3].rinfo	= rinfo;
	rinfo->i2c[3].ddc_reg	= GPIO_CRT2_DDC;
	radeon_setup_i2c_bus(&rinfo->i2c[3], "crt2");
}

aty128fb has no i2c support.

> Besides, this depends on bitbang_i2c.h which is now in hw/i2c so if
> including it from there is not desirable that may need to be moved
> somewhere.

include/hw/i2c/ looks like a sensible location to me.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] ati-vga: Implement DDC and EDID info from monitor

[BALATON Zoltan]

On Mon, 11 Mar 2019, Gerd Hoffmann wrote:
> On Sun, Mar 10, 2019 at 12:22:17AM +0100, BALATON Zoltan wrote:
>> This adds DDC support to ati-vga and connects i2c-ddc to provide EDID
>> info that is read by guests to find available screen modes. Not sure
>> if this is 100% correct yet but at least MorphOS is happy with it and
>> starts in a high resolution mode instead of 640x480 (although its
>> splash screen is still not correct).
>
>> Linux needs support from VESA
>> vgabios, it seems to be missing INT10 0x4F15 function (see
>> https://gitlab.freedesktop.org/xorg/xserver/blob/master/hw/xfree86/vbe/vbe.c)
>> without which no DDC is available that also prevents loading the
>> accelerated X driver.
>
> radeonfb doesn't pick up the edid either.
> There seem to be four i2c busses though:
>
> void radeon_create_i2c_busses(struct radeonfb_info *rinfo)
> {
> 	rinfo->i2c[0].rinfo	= rinfo;
> 	rinfo->i2c[0].ddc_reg	= GPIO_MONID;
> #ifndef CONFIG_PPC
> 	rinfo->i2c[0].adapter.class = I2C_CLASS_HWMON;
> #endif
> 	radeon_setup_i2c_bus(&rinfo->i2c[0], "monid");
>
> 	rinfo->i2c[1].rinfo	= rinfo;
> 	rinfo->i2c[1].ddc_reg	= GPIO_DVI_DDC;
> 	radeon_setup_i2c_bus(&rinfo->i2c[1], "dvi");
>
> 	rinfo->i2c[2].rinfo	= rinfo;
> 	rinfo->i2c[2].ddc_reg	= GPIO_VGA_DDC;
> 	radeon_setup_i2c_bus(&rinfo->i2c[2], "vga");
>
> 	rinfo->i2c[3].rinfo	= rinfo;
> 	rinfo->i2c[3].ddc_reg	= GPIO_CRT2_DDC;
> 	radeon_setup_i2c_bus(&rinfo->i2c[3], "crt2");
> }
>
> aty128fb has no i2c support.

On rage128p I think there's only GPIO_MONID where there are 4 pins that 
can have 2 i2c busses. I think they are DDC for CRT and flat-panel ports 
but not sure about that. So what it's missing is i2c for hw monitoring 
which the GPIO_MONID may be used for on radeon according to the above.

The r128 xorg driver complains about missing support in VESA VBE as quoted 
above and unloads itself. I think it may work if that support is 
implemented in vgabios which I hope you can look at as I have no 
experience with vgabios.

For radeon the DDC may need to be added on the GPIO_VGA_PORT as well based 
on the above code excerpt but I haven't checked that.

>> Besides, this depends on bitbang_i2c.h which is now in hw/i2c so if
>> including it from there is not desirable that may need to be moved
>> somewhere.
>
> include/hw/i2c/ looks like a sensible location to me.

The i2c_bitbang.h could be merged with include/i2c/i2c.h where a line 
from it was already moved before or it could be moved to include/hw/i2c 
and revert previous change and adjust its users again. I'm inclined to 
just merge it with i2c.h but not sure who is to decide about this. The 
bitbang i2c does not seem to have a maintainer.

Regards,
BALATON Zoltan