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

[BALATON Zoltan]

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?

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);
>         break;
>     case GPIO_MONID:
>         if (s->dev_id != PCI_DEVICE_ID_ATI_RAGE128_PF) {