| Message ID | 20180217140051.22731-2-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] hw/i2c-ddc: Do not fail writes | expand |
Hi Linus, On 02/17/2018 11:00 AM, Linus Walleij wrote: > This adds support for emulating the Silicon Image SII9022 DVI/HDMI > bridge. It's not very clever right now, it just acknowledges > the switch into DDC I2C mode and back. Combining this with the > existing DDC I2C emulation gives the right behavior on the Versatile > Express emulation passing through the QEMU EDID to the emulated > platform. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > hw/display/Makefile.objs | 1 + > hw/display/sii9022.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 186 insertions(+) > create mode 100644 hw/display/sii9022.c > > diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs > index d3a4cb396eb9..3c7c75b94da5 100644 > --- a/hw/display/Makefile.objs > +++ b/hw/display/Makefile.objs > @@ -3,6 +3,7 @@ common-obj-$(CONFIG_VGA_CIRRUS) += cirrus_vga.o > common-obj-$(CONFIG_G364FB) += g364fb.o > common-obj-$(CONFIG_JAZZ_LED) += jazz_led.o > common-obj-$(CONFIG_PL110) += pl110.o > +common-obj-$(CONFIG_SII9022) += sii9022.o > common-obj-$(CONFIG_SSD0303) += ssd0303.o > common-obj-$(CONFIG_SSD0323) += ssd0323.o > common-obj-$(CONFIG_XEN) += xenfb.o > diff --git a/hw/display/sii9022.c b/hw/display/sii9022.c > new file mode 100644 > index 000000000000..d6f3cdc04293 > --- /dev/null > +++ b/hw/display/sii9022.c > @@ -0,0 +1,185 @@ > +/* > + * Silicon Image SiI9022 > + * > + * This is a pretty hollow emulation: all we do is acknowledge that we > + * exist (chip ID) and confirm that we get switched over into DDC mode > + * so the emulated host can proceed to read out EDID data. All subsequent > + * set-up of connectors etc will be acknowledged and ignored. > + * > + * Copyright (c) 2018 Linus Walleij > + * > + * This code is licensed under the GNU GPL v2. > + * > + * Contributions after 2012-01-13 are licensed under the terms of the > + * GNU GPL, version 2 or (at your option) any later version. > + */ > + > +#include "qemu/osdep.h" > +#include "qemu-common.h" > +#include "hw/i2c/i2c.h" > + > +#define DEBUG_SII9022 0 > + > +#define DPRINTF(fmt, ...) \ > + do { \ > + if (DEBUG_SII9022) { \ > + printf("sii9022: " fmt, ## __VA_ARGS__); \ > + } \ > + } while (0) Can you replace DPRINTF() by trace events? Except that, patch looks fine. Regards, Phil. > + > +#define SII9022_SYS_CTRL_DATA 0x1a > +#define SII9022_SYS_CTRL_PWR_DWN 0x10 > +#define SII9022_SYS_CTRL_AV_MUTE 0x08 > +#define SII9022_SYS_CTRL_DDC_BUS_REQ 0x04 > +#define SII9022_SYS_CTRL_DDC_BUS_GRTD 0x02 > +#define SII9022_SYS_CTRL_OUTPUT_MODE 0x01 > +#define SII9022_SYS_CTRL_OUTPUT_HDMI 1 > +#define SII9022_SYS_CTRL_OUTPUT_DVI 0 > +#define SII9022_REG_CHIPID 0x1b > +#define SII9022_INT_ENABLE 0x3c > +#define SII9022_INT_STATUS 0x3d > +#define SII9022_INT_STATUS_HOTPLUG 0x01; > +#define SII9022_INT_STATUS_PLUGGED 0x04; > + > +#define TYPE_SII9022 "sii9022" > +#define SII9022(obj) OBJECT_CHECK(sii9022_state, (obj), TYPE_SII9022) > + > +typedef struct sii9022_state { > + I2CSlave parent_obj; > + uint8_t ptr; > + bool addr_byte; > + bool ddc_req; > + bool ddc_skip_finish; > + bool ddc; > +} sii9022_state; > + > +static const VMStateDescription vmstate_sii9022 = { > + .name = "sii9022", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_I2C_SLAVE(parent_obj, sii9022_state), > + VMSTATE_UINT8(ptr, sii9022_state), > + VMSTATE_BOOL(addr_byte, sii9022_state), > + VMSTATE_BOOL(ddc_req, sii9022_state), > + VMSTATE_BOOL(ddc_skip_finish, sii9022_state), > + VMSTATE_BOOL(ddc, sii9022_state), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static int sii9022_event(I2CSlave *i2c, enum i2c_event event) > +{ > + sii9022_state *s = SII9022(i2c); > + > + switch (event) { > + case I2C_START_SEND: > + s->addr_byte = true; > + break; > + case I2C_START_RECV: > + break; > + case I2C_FINISH: > + break; > + case I2C_NACK: > + break; > + } > + > + return 0; > +} > + > +static int sii9022_rx(I2CSlave *i2c) > +{ > + sii9022_state *s = SII9022(i2c); > + uint8_t res = 0x00; > + > + switch (s->ptr) { > + case SII9022_SYS_CTRL_DATA: > + if (s->ddc_req) { > + /* Acknowledge DDC bus request */ > + res = SII9022_SYS_CTRL_DDC_BUS_GRTD | SII9022_SYS_CTRL_DDC_BUS_REQ; > + } > + break; > + case SII9022_REG_CHIPID: > + res = 0xb0; > + break; > + case SII9022_INT_STATUS: > + /* Something is cold-plugged in, no interrupts */ > + res = SII9022_INT_STATUS_PLUGGED; > + break; > + default: > + break; > + } > + DPRINTF("%02x read from %02x\n", res, s->ptr); > + s->ptr++; > + > + return res; > +} > + > +static int sii9022_tx(I2CSlave *i2c, uint8_t data) > +{ > + sii9022_state *s = SII9022(i2c); > + > + if (s->addr_byte) { > + s->ptr = data; > + s->addr_byte = false; > + return 0; > + } > + > + switch (s->ptr) { > + case SII9022_SYS_CTRL_DATA: > + if (data & SII9022_SYS_CTRL_DDC_BUS_REQ) { > + s->ddc_req = true; > + if (data & SII9022_SYS_CTRL_DDC_BUS_GRTD) { > + s->ddc = true; > + /* Skip this finish since we just switched to DDC */ > + s->ddc_skip_finish = true; > + DPRINTF("switched to DDC mode\n"); > + } > + } else { > + s->ddc_req = false; > + s->ddc = false; > + } > + break; > + default: > + break; > + } > + > + DPRINTF("%02x written to %02x\n", data, s->ptr); > + s->ptr++; > + > + return 0; > +} > + > +static void sii9022_reset(DeviceState *dev) > +{ > + sii9022_state *s = SII9022(dev); > + > + s->ptr = 0; > + s->addr_byte = false; > +} > + > +static void sii9022_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass); > + > + k->event = sii9022_event; > + k->recv = sii9022_rx; > + k->send = sii9022_tx; > + dc->reset = sii9022_reset; > + dc->vmsd = &vmstate_sii9022; > +} > + > +static const TypeInfo sii9022_info = { > + .name = TYPE_SII9022, > + .parent = TYPE_I2C_SLAVE, > + .instance_size = sizeof(sii9022_state), > + .class_init = sii9022_class_init, > +}; > + > +static void sii9022_register_types(void) > +{ > + type_register_static(&sii9022_info); > +} > + > +type_init(sii9022_register_types) >
On 17 February 2018 at 14:00, Linus Walleij <linus.walleij@linaro.org> wrote: > This adds support for emulating the Silicon Image SII9022 DVI/HDMI > bridge. It's not very clever right now, it just acknowledges > the switch into DDC I2C mode and back. Combining this with the > existing DDC I2C emulation gives the right behavior on the Versatile > Express emulation passing through the QEMU EDID to the emulated > platform. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > hw/display/Makefile.objs | 1 + > hw/display/sii9022.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 186 insertions(+) > create mode 100644 hw/display/sii9022.c > > diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs > index d3a4cb396eb9..3c7c75b94da5 100644 > --- a/hw/display/Makefile.objs > +++ b/hw/display/Makefile.objs > @@ -3,6 +3,7 @@ common-obj-$(CONFIG_VGA_CIRRUS) += cirrus_vga.o > common-obj-$(CONFIG_G364FB) += g364fb.o > common-obj-$(CONFIG_JAZZ_LED) += jazz_led.o > common-obj-$(CONFIG_PL110) += pl110.o > +common-obj-$(CONFIG_SII9022) += sii9022.o > common-obj-$(CONFIG_SSD0303) += ssd0303.o > common-obj-$(CONFIG_SSD0323) += ssd0323.o > common-obj-$(CONFIG_XEN) += xenfb.o > diff --git a/hw/display/sii9022.c b/hw/display/sii9022.c > new file mode 100644 > index 000000000000..d6f3cdc04293 > --- /dev/null > +++ b/hw/display/sii9022.c > @@ -0,0 +1,185 @@ > +/* > + * Silicon Image SiI9022 > + * > + * This is a pretty hollow emulation: all we do is acknowledge that we > + * exist (chip ID) and confirm that we get switched over into DDC mode > + * so the emulated host can proceed to read out EDID data. All subsequent > + * set-up of connectors etc will be acknowledged and ignored. > + * > + * Copyright (c) 2018 Linus Walleij Sanity check: really copyright you and not Linaro ? > + * > + * This code is licensed under the GNU GPL v2. > + * > + * Contributions after 2012-01-13 are licensed under the terms of the > + * GNU GPL, version 2 or (at your option) any later version. If this is copyright 2018 then what parts of it are not covered by the "contributions after 2012-01-13" condition? Could we just have this be GPL-v2-or-later, which is what most new code is ? > +static void sii9022_reset(DeviceState *dev) > +{ > + sii9022_state *s = SII9022(dev); > + > + s->ptr = 0; > + s->addr_byte = false; I think we should reset ddc/ddc_skip_finish/ddc_req here too. I suspect the state machine logic means we can't get to a place where their values are used once we reset ptr and addr_byte, but it's easier to reason about if we just reset all the device's state. I agree with Philippe that we should use trace events here rather than the DPRINTF macro. Otherwise the code looks fine. thanks -- PMM
On Sat, Feb 17, 2018 at 7:32 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > [Me] >> +#define DEBUG_SII9022 0 >> + >> +#define DPRINTF(fmt, ...) \ >> + do { \ >> + if (DEBUG_SII9022) { \ >> + printf("sii9022: " fmt, ## __VA_ARGS__); \ >> + } \ >> + } while (0) > > Can you replace DPRINTF() by trace events? Absolutely but which ones? I do not feel senior enough to also invent new trace events for displays or I2C devices... Yours, Linus Walleij
On 27 February 2018 at 07:41, Linus Walleij <linus.walleij@linaro.org> wrote: > On Sat, Feb 17, 2018 at 7:32 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > >> [Me] >>> +#define DEBUG_SII9022 0 >>> + >>> +#define DPRINTF(fmt, ...) \ >>> + do { \ >>> + if (DEBUG_SII9022) { \ >>> + printf("sii9022: " fmt, ## __VA_ARGS__); \ >>> + } \ >>> + } while (0) >> >> Can you replace DPRINTF() by trace events? > > Absolutely but which ones? > > I do not feel senior enough to also invent new trace events > for displays or I2C devices... Just put a trace event where you've put DPRINTF debug statements. thanks -- PMM
On Tue, Feb 27, 2018 at 11:09 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 27 February 2018 at 07:41, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Sat, Feb 17, 2018 at 7:32 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> >>> [Me] >>>> +#define DEBUG_SII9022 0 >>>> + >>>> +#define DPRINTF(fmt, ...) \ >>>> + do { \ >>>> + if (DEBUG_SII9022) { \ >>>> + printf("sii9022: " fmt, ## __VA_ARGS__); \ >>>> + } \ >>>> + } while (0) >>> >>> Can you replace DPRINTF() by trace events? >> >> Absolutely but which ones? >> >> I do not feel senior enough to also invent new trace events >> for displays or I2C devices... > > Just put a trace event where you've put DPRINTF debug statements. Yeah, hm the question might be silly or something but I don't know how to do that. I guess I should include "trace.h". #include "trace.h" says it is autogenerated from tracetool. Is there some documentation I should be reading or a good example commit I can study to get the hang of it? Yours, Linus Walleij
On 27 February 2018 at 10:21, Linus Walleij <linus.walleij@linaro.org> wrote: > On Tue, Feb 27, 2018 at 11:09 AM, Peter Maydell > <peter.maydell@linaro.org> wrote: >> On 27 February 2018 at 07:41, Linus Walleij <linus.walleij@linaro.org> wrote: >>> On Sat, Feb 17, 2018 at 7:32 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >>> >>>> [Me] >>>>> +#define DEBUG_SII9022 0 >>>>> + >>>>> +#define DPRINTF(fmt, ...) \ >>>>> + do { \ >>>>> + if (DEBUG_SII9022) { \ >>>>> + printf("sii9022: " fmt, ## __VA_ARGS__); \ >>>>> + } \ >>>>> + } while (0) >>>> >>>> Can you replace DPRINTF() by trace events? >>> >>> Absolutely but which ones? >>> >>> I do not feel senior enough to also invent new trace events >>> for displays or I2C devices... >> >> Just put a trace event where you've put DPRINTF debug statements. > > Yeah, hm the question might be silly or something but I don't > know how to do that. docs/devel/tracing.txt describes them, but basically: * include "trace.h" * define trace events with a line for each in the trace-events file for the subdirectory (basically a function prototype-ish thing followed by a printf-style format string) * call them like normal function calls with a trace_ prefix commit 1b640aa9292bc00beb441e97d862ba322a7ba18d is a recent one which converted some DRINTFs in hw/sd/sd.c to trace events. thanks -- PMM
On 27 February 2018 at 10:24, Peter Maydell <peter.maydell@linaro.org> wrote: > docs/devel/tracing.txt describes them, but basically: > * include "trace.h" > * define trace events with a line for each in the trace-events > file for the subdirectory (basically a function prototype-ish > thing followed by a printf-style format string) > * call them like normal function calls with a trace_ prefix > > commit 1b640aa9292bc00beb441e97d862ba322a7ba18d is a recent one > which converted some DRINTFs in hw/sd/sd.c to trace events. ...and you can test them most conveniently with the QEMU -d argument: "-d trace:sii9022*" will enable tracing to stderr of all trace events with names that begin 'sii9022'. thanks -- PMM
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs index d3a4cb396eb9..3c7c75b94da5 100644 --- a/hw/display/Makefile.objs +++ b/hw/display/Makefile.objs @@ -3,6 +3,7 @@ common-obj-$(CONFIG_VGA_CIRRUS) += cirrus_vga.o common-obj-$(CONFIG_G364FB) += g364fb.o common-obj-$(CONFIG_JAZZ_LED) += jazz_led.o common-obj-$(CONFIG_PL110) += pl110.o +common-obj-$(CONFIG_SII9022) += sii9022.o common-obj-$(CONFIG_SSD0303) += ssd0303.o common-obj-$(CONFIG_SSD0323) += ssd0323.o common-obj-$(CONFIG_XEN) += xenfb.o diff --git a/hw/display/sii9022.c b/hw/display/sii9022.c new file mode 100644 index 000000000000..d6f3cdc04293 --- /dev/null +++ b/hw/display/sii9022.c @@ -0,0 +1,185 @@ +/* + * Silicon Image SiI9022 + * + * This is a pretty hollow emulation: all we do is acknowledge that we + * exist (chip ID) and confirm that we get switched over into DDC mode + * so the emulated host can proceed to read out EDID data. All subsequent + * set-up of connectors etc will be acknowledged and ignored. + * + * Copyright (c) 2018 Linus Walleij + * + * This code is licensed under the GNU GPL v2. + * + * Contributions after 2012-01-13 are licensed under the terms of the + * GNU GPL, version 2 or (at your option) any later version. + */ + +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "hw/i2c/i2c.h" + +#define DEBUG_SII9022 0 + +#define DPRINTF(fmt, ...) \ + do { \ + if (DEBUG_SII9022) { \ + printf("sii9022: " fmt, ## __VA_ARGS__); \ + } \ + } while (0) + +#define SII9022_SYS_CTRL_DATA 0x1a +#define SII9022_SYS_CTRL_PWR_DWN 0x10 +#define SII9022_SYS_CTRL_AV_MUTE 0x08 +#define SII9022_SYS_CTRL_DDC_BUS_REQ 0x04 +#define SII9022_SYS_CTRL_DDC_BUS_GRTD 0x02 +#define SII9022_SYS_CTRL_OUTPUT_MODE 0x01 +#define SII9022_SYS_CTRL_OUTPUT_HDMI 1 +#define SII9022_SYS_CTRL_OUTPUT_DVI 0 +#define SII9022_REG_CHIPID 0x1b +#define SII9022_INT_ENABLE 0x3c +#define SII9022_INT_STATUS 0x3d +#define SII9022_INT_STATUS_HOTPLUG 0x01; +#define SII9022_INT_STATUS_PLUGGED 0x04; + +#define TYPE_SII9022 "sii9022" +#define SII9022(obj) OBJECT_CHECK(sii9022_state, (obj), TYPE_SII9022) + +typedef struct sii9022_state { + I2CSlave parent_obj; + uint8_t ptr; + bool addr_byte; + bool ddc_req; + bool ddc_skip_finish; + bool ddc; +} sii9022_state; + +static const VMStateDescription vmstate_sii9022 = { + .name = "sii9022", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_I2C_SLAVE(parent_obj, sii9022_state), + VMSTATE_UINT8(ptr, sii9022_state), + VMSTATE_BOOL(addr_byte, sii9022_state), + VMSTATE_BOOL(ddc_req, sii9022_state), + VMSTATE_BOOL(ddc_skip_finish, sii9022_state), + VMSTATE_BOOL(ddc, sii9022_state), + VMSTATE_END_OF_LIST() + } +}; + +static int sii9022_event(I2CSlave *i2c, enum i2c_event event) +{ + sii9022_state *s = SII9022(i2c); + + switch (event) { + case I2C_START_SEND: + s->addr_byte = true; + break; + case I2C_START_RECV: + break; + case I2C_FINISH: + break; + case I2C_NACK: + break; + } + + return 0; +} + +static int sii9022_rx(I2CSlave *i2c) +{ + sii9022_state *s = SII9022(i2c); + uint8_t res = 0x00; + + switch (s->ptr) { + case SII9022_SYS_CTRL_DATA: + if (s->ddc_req) { + /* Acknowledge DDC bus request */ + res = SII9022_SYS_CTRL_DDC_BUS_GRTD | SII9022_SYS_CTRL_DDC_BUS_REQ; + } + break; + case SII9022_REG_CHIPID: + res = 0xb0; + break; + case SII9022_INT_STATUS: + /* Something is cold-plugged in, no interrupts */ + res = SII9022_INT_STATUS_PLUGGED; + break; + default: + break; + } + DPRINTF("%02x read from %02x\n", res, s->ptr); + s->ptr++; + + return res; +} + +static int sii9022_tx(I2CSlave *i2c, uint8_t data) +{ + sii9022_state *s = SII9022(i2c); + + if (s->addr_byte) { + s->ptr = data; + s->addr_byte = false; + return 0; + } + + switch (s->ptr) { + case SII9022_SYS_CTRL_DATA: + if (data & SII9022_SYS_CTRL_DDC_BUS_REQ) { + s->ddc_req = true; + if (data & SII9022_SYS_CTRL_DDC_BUS_GRTD) { + s->ddc = true; + /* Skip this finish since we just switched to DDC */ + s->ddc_skip_finish = true; + DPRINTF("switched to DDC mode\n"); + } + } else { + s->ddc_req = false; + s->ddc = false; + } + break; + default: + break; + } + + DPRINTF("%02x written to %02x\n", data, s->ptr); + s->ptr++; + + return 0; +} + +static void sii9022_reset(DeviceState *dev) +{ + sii9022_state *s = SII9022(dev); + + s->ptr = 0; + s->addr_byte = false; +} + +static void sii9022_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + I2CSlaveClass *k = I2C_SLAVE_CLASS(klass); + + k->event = sii9022_event; + k->recv = sii9022_rx; + k->send = sii9022_tx; + dc->reset = sii9022_reset; + dc->vmsd = &vmstate_sii9022; +} + +static const TypeInfo sii9022_info = { + .name = TYPE_SII9022, + .parent = TYPE_I2C_SLAVE, + .instance_size = sizeof(sii9022_state), + .class_init = sii9022_class_init, +}; + +static void sii9022_register_types(void) +{ + type_register_static(&sii9022_info); +} + +type_init(sii9022_register_types)
This adds support for emulating the Silicon Image SII9022 DVI/HDMI bridge. It's not very clever right now, it just acknowledges the switch into DDC I2C mode and back. Combining this with the existing DDC I2C emulation gives the right behavior on the Versatile Express emulation passing through the QEMU EDID to the emulated platform. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- hw/display/Makefile.objs | 1 + hw/display/sii9022.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100644 hw/display/sii9022.c -- 2.14.3