diff mbox series

[3/3] arm/vexpress: Add proper display connector emulation

Message ID 20180217140051.22731-3-linus.walleij@linaro.org
State Superseded
Headers show
Series [1/3] hw/i2c-ddc: Do not fail writes | expand

Commit Message

Linus Walleij Feb. 17, 2018, 2 p.m. UTC 
This adds the SiI9022 and EDID I2C devices to the ARM Versatile
Express machine, and selects the two I2C devices necessary in the
arm-softmmy.mak configuration so everything will build smoothly.

I am implementing proper handling of the graphics in the Linux
kernel and adding proper emulation of SiI9022 and EDID makes the
driver probe as nicely as before, retrieveing the resolutions
supported by the "QEMU monitor" and overall just working nice.

The assignment of the SiI9022 at address 0x39 and the EDID
DDC I2C at address 0x50 is not strictly correct: the DDC I2C
is there all the time but in the actual component it only
appears once activated inside the SiI9022, so ideally it should
be added and removed to the bus by the SiI9022. However for this
purpose it works fine to just have it around.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 default-configs/arm-softmmu.mak | 2 ++
 hw/arm/vexpress.c               | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

-- 
2.14.3

Comments

Philippe Mathieu-Daudé Feb. 17, 2018, 6:28 p.m. UTC | #1 
Hi Linus,

On 02/17/2018 11:00 AM, Linus Walleij wrote:
> This adds the SiI9022 and EDID I2C devices to the ARM Versatile

> Express machine, and selects the two I2C devices necessary in the

> arm-softmmy.mak configuration so everything will build smoothly.

> 

> I am implementing proper handling of the graphics in the Linux

> kernel and adding proper emulation of SiI9022 and EDID makes the

> driver probe as nicely as before, retrieveing the resolutions

> supported by the "QEMU monitor" and overall just working nice.

> 

> The assignment of the SiI9022 at address 0x39 and the EDID

> DDC I2C at address 0x50 is not strictly correct: the DDC I2C

> is there all the time but in the actual component it only

> appears once activated inside the SiI9022, so ideally it should

> be added and removed to the bus by the SiI9022. However for this

> purpose it works fine to just have it around.


This seems easier to just do it now rather than postpone :)

In your patch #2:

static void sii9022_realize(DeviceState *dev, Error **errp)
{
    I2CBus *bus;

    bus = I2C_BUS(qdev_get_parent_bus(dev));
    i2c_create_slave(bus, TYPE_I2CDDC, 0x50);
}

static void sii9022_class_init(ObjectClass *klass, void *data)
{
    DeviceClass *dc = DEVICE_CLASS(klass);

...
    dc->realize = sii9022_realize;
}

> 

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

> ---

>  default-configs/arm-softmmu.mak | 2 ++

>  hw/arm/vexpress.c               | 7 ++++++-

>  2 files changed, 8 insertions(+), 1 deletion(-)

> 

> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak

> index ca34cf446242..54f855d07206 100644

> --- a/default-configs/arm-softmmu.mak

> +++ b/default-configs/arm-softmmu.mak

> @@ -21,6 +21,8 @@ CONFIG_STELLARIS_INPUT=y

>  CONFIG_STELLARIS_ENET=y

>  CONFIG_SSD0303=y

>  CONFIG_SSD0323=y

> +CONFIG_DDC=y

> +CONFIG_SII9022=y

>  CONFIG_ADS7846=y

>  CONFIG_MAX111X=y

>  CONFIG_SSI=y

> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c

> index dc5928ae1ab5..d6c912c97684 100644

> --- a/hw/arm/vexpress.c

> +++ b/hw/arm/vexpress.c

> @@ -29,6 +29,7 @@

>  #include "hw/arm/arm.h"

>  #include "hw/arm/primecell.h"

>  #include "hw/devices.h"

> +#include "hw/i2c/i2c.h"

>  #include "net/net.h"

>  #include "sysemu/sysemu.h"

>  #include "hw/boards.h"

> @@ -537,6 +538,7 @@ static void vexpress_common_init(MachineState *machine)

>      uint32_t sys_id;

>      DriveInfo *dinfo;

>      pflash_t *pflash0;

> +    I2CBus *i2c;

>      ram_addr_t vram_size, sram_size;

>      MemoryRegion *sysmem = get_system_memory();

>      MemoryRegion *vram = g_new(MemoryRegion, 1);

> @@ -628,7 +630,10 @@ static void vexpress_common_init(MachineState *machine)

>      sysbus_create_simple("sp804", map[VE_TIMER01], pic[2]);

>      sysbus_create_simple("sp804", map[VE_TIMER23], pic[3]);

>  

> -    /* VE_SERIALDVI: not modelled */

> +    dev = sysbus_create_simple("versatile_i2c", map[VE_SERIALDVI], NULL);

> +    i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");

> +    i2c_create_slave(i2c, "sii9022", 0x39);

> +    i2c_create_slave(i2c, "i2c-ddc", 0x50);

>  

>      sysbus_create_simple("pl031", map[VE_RTC], pic[4]); /* RTC */

>  

>
Linus Walleij Feb. 19, 2018, 2:10 p.m. UTC | #2 
On Sat, Feb 17, 2018 at 7:28 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 02/17/2018 11:00 AM, Linus Walleij wrote:


>> The assignment of the SiI9022 at address 0x39 and the EDID

>> DDC I2C at address 0x50 is not strictly correct: the DDC I2C

>> is there all the time but in the actual component it only

>> appears once activated inside the SiI9022, so ideally it should

>> be added and removed to the bus by the SiI9022. However for this

>> purpose it works fine to just have it around.

>

> This seems easier to just do it now rather than postpone :)

>

> In your patch #2:

>

> static void sii9022_realize(DeviceState *dev, Error **errp)

> {

>     I2CBus *bus;

>

>     bus = I2C_BUS(qdev_get_parent_bus(dev));

>     i2c_create_slave(bus, TYPE_I2CDDC, 0x50);

> }


I tried an approach like this but the I2C_BUS() macro was not
publicly visible in any header file and trying to pry it out just
brought along a lot of refactoring in the i2c core.c file that
scared me away...

Has someone else fixed it already?

Yours,
Linus Walleij
Corey Minyard Feb. 19, 2018, 3:16 p.m. UTC | #3 
On 02/19/2018 08:10 AM, Linus Walleij wrote:
> On Sat, Feb 17, 2018 at 7:28 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

>> On 02/17/2018 11:00 AM, Linus Walleij wrote:

>>> The assignment of the SiI9022 at address 0x39 and the EDID

>>> DDC I2C at address 0x50 is not strictly correct: the DDC I2C

>>> is there all the time but in the actual component it only

>>> appears once activated inside the SiI9022, so ideally it should

>>> be added and removed to the bus by the SiI9022. However for this

>>> purpose it works fine to just have it around.

>> This seems easier to just do it now rather than postpone :)

>>

>> In your patch #2:

>>

>> static void sii9022_realize(DeviceState *dev, Error **errp)

>> {

>>      I2CBus *bus;

>>

>>      bus = I2C_BUS(qdev_get_parent_bus(dev));

>>      i2c_create_slave(bus, TYPE_I2CDDC, 0x50);

>> }

> I tried an approach like this but the I2C_BUS() macro was not

> publicly visible in any header file and trying to pry it out just

> brought along a lot of refactoring in the i2c core.c file that

> scared me away...

>

> Has someone else fixed it already?


I have a patch that does this.  Well, it's a small cleanup patch and another
patch, but it's really no that bad.  I'll send it.

-corey

>

> Yours,

> Linus Walleij

>
diff mbox series

Patch

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index ca34cf446242..54f855d07206 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -21,6 +21,8 @@  CONFIG_STELLARIS_INPUT=y
 CONFIG_STELLARIS_ENET=y
 CONFIG_SSD0303=y
 CONFIG_SSD0323=y
+CONFIG_DDC=y
+CONFIG_SII9022=y
 CONFIG_ADS7846=y
 CONFIG_MAX111X=y
 CONFIG_SSI=y
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index dc5928ae1ab5..d6c912c97684 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -29,6 +29,7 @@ 
 #include "hw/arm/arm.h"
 #include "hw/arm/primecell.h"
 #include "hw/devices.h"
+#include "hw/i2c/i2c.h"
 #include "net/net.h"
 #include "sysemu/sysemu.h"
 #include "hw/boards.h"
@@ -537,6 +538,7 @@  static void vexpress_common_init(MachineState *machine)
     uint32_t sys_id;
     DriveInfo *dinfo;
     pflash_t *pflash0;
+    I2CBus *i2c;
     ram_addr_t vram_size, sram_size;
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *vram = g_new(MemoryRegion, 1);
@@ -628,7 +630,10 @@  static void vexpress_common_init(MachineState *machine)
     sysbus_create_simple("sp804", map[VE_TIMER01], pic[2]);
     sysbus_create_simple("sp804", map[VE_TIMER23], pic[3]);
 
-    /* VE_SERIALDVI: not modelled */
+    dev = sysbus_create_simple("versatile_i2c", map[VE_SERIALDVI], NULL);
+    i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");
+    i2c_create_slave(i2c, "sii9022", 0x39);
+    i2c_create_slave(i2c, "i2c-ddc", 0x50);
 
     sysbus_create_simple("pl031", map[VE_RTC], pic[4]); /* RTC */