Re: [RFC Patch 1/5] hw/display: Allwinner A10 HDMI controller emulation

[Philippe Mathieu-Daudé]

Hi Strahinja,

On 5/9/23 22:14, Strahinja Jankovic wrote:
> This patch adds basic Allwinner A10 HDMI controller support.
> Emulated HDMI component will always show that a display is connected and
> provide default EDID info.
>
> Signed-off-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
> ---
>   hw/arm/allwinner-a10.c                  |   7 +
>   hw/display/allwinner-a10-hdmi.c         | 214 ++++++++++++++++++++++++
>   hw/display/meson.build                  |   2 +
>   hw/display/trace-events                 |   4 +
>   include/hw/arm/allwinner-a10.h          |   2 +
>   include/hw/display/allwinner-a10-hdmi.h |  69 ++++++++
>   6 files changed, 298 insertions(+)
>   create mode 100644 hw/display/allwinner-a10-hdmi.c
>   create mode 100644 include/hw/display/allwinner-a10-hdmi.h


> diff --git a/hw/display/allwinner-a10-hdmi.c b/hw/display/allwinner-a10-hdmi.c
> new file mode 100644
> index 0000000000..0f046e3cc7
> --- /dev/null
> +++ b/hw/display/allwinner-a10-hdmi.c


> +#define REG_INDEX(offset)    (offset / sizeof(uint32_t))
> +
> +static uint64_t allwinner_a10_hdmi_read(void *opaque, hwaddr offset,
> +                                       unsigned size)
> +{
> +    AwA10HdmiState *s = AW_A10_HDMI(opaque);
> +    const uint32_t idx = REG_INDEX(offset);
> +    uint32_t val = s->regs[idx];
> +
> +    switch (offset) {
> +    case REG_HPD:
> +        val = FIELD_HPD_HOTPLUG_DET_HIGH;
> +        break;


> +}
> +
> +static void allwinner_a10_hdmi_write(void *opaque, hwaddr offset,
> +                                   uint64_t val, unsigned size)
> +{
> +    AwA10HdmiState *s = AW_A10_HDMI(opaque);
> +    const uint32_t idx = REG_INDEX(offset);
> +
> +    switch (offset) {
> +    case REG_DDC_CTRL:
> +        if (val & FIELD_DDC_CTRL_SW_RST) {
> +            val &= ~FIELD_DDC_CTRL_SW_RST;
> +        }


> +    s->regs[idx] = (uint32_t) val;
> +}
> +
> +static const MemoryRegionOps allwinner_a10_hdmi_ops = {
> +    .read = allwinner_a10_hdmi_read,
> +    .write = allwinner_a10_hdmi_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 4,
> +    },
> +    .impl.min_access_size = 1,

Per REG_INDEX() you have .impl.min/max = 4.

Otherwise your patch LGTM :)

> +};