Re: [Qemu-devel] [PATCH] ati-vga: i2c fix

[Gerd Hoffmann]

On Thu, Mar 14, 2019 at 08:11:21PM +0100, BALATON Zoltan wrote:
> On Thu, 14 Mar 2019, Gerd Hoffmann wrote:
> > gets radeonfb going for me, on top of your i2c patches.
> > ---
> > hw/display/ati_int.h  |  1 +
> > hw/display/ati_regs.h |  1 +
> > hw/display/ati.c      | 35 +++++++++++++++++++++++++++--------
> > 3 files changed, 29 insertions(+), 8 deletions(-)
> > 
> > diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
> > index 3f4a06f1e1ed..7289db206cd2 100644
> > --- a/hw/display/ati_int.h
> > +++ b/hw/display/ati_int.h
> > @@ -38,6 +38,7 @@ typedef struct ATIVGARegs {
> >     uint32_t crtc_ext_cntl;
> >     uint32_t dac_cntl;
> >     uint32_t gpio_vga_ddc;
> > +    uint32_t gpio_dvi_ddc;
> >     uint32_t gpio_monid;
> >     uint32_t crtc_h_total_disp;
> >     uint32_t crtc_h_sync_strt_wid;
> > diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
> > index 90384c886ecb..1ec3498b731c 100644
> > --- a/hw/display/ati_regs.h
> > +++ b/hw/display/ati_regs.h
> > @@ -38,6 +38,7 @@
> > #define CRTC_EXT_CNTL                           0x0054
> > #define DAC_CNTL                                0x0058
> > #define GPIO_VGA_DDC                            0x0060
> > +#define GPIO_DVI_DDC                            0x0064
> > #define GPIO_MONID                              0x0068
> > #define I2C_CNTL_1                              0x0094
> > #define PALETTE_INDEX                           0x00b0
> > diff --git a/hw/display/ati.c b/hw/display/ati.c
> > index e2efc6f2225e..ffced39aad9c 100644
> > --- a/hw/display/ati.c
> > +++ b/hw/display/ati.c
> > @@ -272,6 +272,9 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, 
> > unsigned int size)
> >     case GPIO_VGA_DDC:
> >         val = s->regs.gpio_vga_ddc;
> >         break;
> > +    case GPIO_DVI_DDC:
> > +        val = s->regs.gpio_dvi_ddc;
> > +        break;
> >     case GPIO_MONID:
> >         val = s->regs.gpio_monid;
> >         break;
> > @@ -426,6 +429,22 @@ static inline void ati_reg_write_offs(uint32_t *reg, 
> > int offs,
> >     }
> > }
> > 
> > +static uint64_t ati_i2c(bitbang_i2c_interface *i2c,
> > +                        uint64_t data)
> > +{
> > +    bool clk = !(data & BIT(17));
> > +    bool out = !(data & BIT(16));
> > +    bool in;
> > +
> > +    bitbang_i2c_set(i2c, BITBANG_I2C_SCL, clk);
> > +    in = bitbang_i2c_set(i2c, BITBANG_I2C_SDA, out);
> > +
> > +    data &= 0xf000f;
> > +    data |= clk ? BIT(9) : 0;
> > +    data |= in  ? BIT(8) : 0;
> > +    return data;
> > +}
> > +
> > static void ati_mm_write(void *opaque, hwaddr addr,
> >                            uint64_t data, unsigned int size)
> > {
> > @@ -512,15 +531,15 @@ static void ati_mm_write(void *opaque, hwaddr addr,
> >         if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
> >             break;
> >         }
> > -        s->regs.gpio_vga_ddc = data & 0xf000f;
> > -        if (data & BIT(17)) {
> > -            s->regs.gpio_monid |= !!(data & BIT(1)) << 9;
> > -            bitbang_i2c_set(s->bbi2c, BITBANG_I2C_SCL, (data & BIT(1)) != 
> > 0);
> > -        }
> > -        if (data & BIT(16)) {
> > -            s->regs.gpio_monid |= bitbang_i2c_set(s->bbi2c, 
> > BITBANG_I2C_SDA,
> > -                                                  data & BIT(0)) << 8;
> > +#if 0
> > +        s->regs.gpio_vga_ddc = ati_i2c(s->bbi2c, data);
> > +#endif
> 
> Thanks, I'll try and merge this. What's this #if 0 line?

Avoid the monitor show up on both vga ...

> 
> Regards,
> BALATON Zoltan
> 
> > +        break;
> > +    case GPIO_DVI_DDC:
> > +        if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
> > +            break;
> >         }
> > +        s->regs.gpio_dvi_ddc = ati_i2c(s->bbi2c, data);

... and dvi.

A more correct model would probably be to create two i2c busses for
that, then hook up the ddc to one of them (possibly depending on a
config option).

cheers,
  Gerd