Re: [PATCH v2] qxl: introduce hardware revision 5

[Maxim Levitsky <mlevitsk@redhat.com>]

On Thu, 2020-02-06 at 08:43 +0100, Gerd Hoffmann wrote:
> The only difference to hardware revision 4 is that the device doesn't
> switch to VGA mode in case someone happens to touch a VGA register,
> which should make things more robust in configurations with multiple
> vga devices.
> 
> Swtiching back to VGA mode happens on reset, either full machine
> reset or qxl device reset (QXL_IO_RESET ioport command).
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/display/qxl.h  | 2 +-
>  hw/core/machine.c | 2 ++
>  hw/display/qxl.c  | 7 ++++++-
>  3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/display/qxl.h b/hw/display/qxl.h
> index 80eb0d267269..707631a1f573 100644
> --- a/hw/display/qxl.h
> +++ b/hw/display/qxl.h
> @@ -144,7 +144,7 @@ typedef struct PCIQXLDevice {
>          }                                                               \
>      } while (0)
>  
> -#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V12
> +#define QXL_DEFAULT_REVISION (QXL_REVISION_STABLE_V12 + 1)
Minor nitpick: Maybe add a #define instead of +1?

>  
>  /* qxl.c */
>  void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 3e288bfceb7f..13a3b2c9c425 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -34,6 +34,8 @@ GlobalProperty hw_compat_4_2[] = {
>      { "vhost-blk-device", "seg_max_adjust", "off"},
>      { "usb-host", "suppress-remote-wake", "off" },
>      { "usb-redir", "suppress-remote-wake", "off" },
> +    { "qxl", "revision", "4" },
> +    { "qxl-vga", "revision", "4" },
>  };
>  const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2);
>  
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index c33b1915a52c..64884da70857 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -1309,7 +1309,8 @@ static void qxl_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
>      PCIQXLDevice *qxl = container_of(vga, PCIQXLDevice, vga);
>  
>      trace_qxl_io_write_vga(qxl->id, qxl_mode_to_string(qxl->mode), addr, val);
> -    if (qxl->mode != QXL_MODE_VGA) {
> +    if (qxl->mode != QXL_MODE_VGA &&
> +        qxl->revision <= QXL_REVISION_STABLE_V12) {
>          qxl_destroy_primary(qxl, QXL_SYNC);
>          qxl_soft_reset(qxl);
>      }
> @@ -2121,6 +2122,10 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error **errp)
>          pci_device_rev = QXL_REVISION_STABLE_V12;
>          io_size = pow2ceil(QXL_IO_RANGE_SIZE);
>          break;
> +    case 5: /* qxl-5 */
> +        pci_device_rev = QXL_REVISION_STABLE_V12 + 1;
> +        io_size = pow2ceil(QXL_IO_RANGE_SIZE);
> +        break;
>      default:
>          error_setg(errp, "Invalid revision %d for qxl device (max %d)",
>                     qxl->revision, QXL_DEFAULT_REVISION);

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Best regards,
	Maxim Levitsky