* [PATCH 1/4] edid: use physical dimensions if available
2020-08-17 12:00 [PATCH 0/4] WIP: add physical display dimensions to spice/virtio-gpu marcandre.lureau
@ 2020-08-17 12:00 ` marcandre.lureau
2020-08-17 12:21 ` Gerd Hoffmann
2020-08-17 12:00 ` [PATCH 2/4] ui: add getter for UIInfo marcandre.lureau
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: marcandre.lureau @ 2020-08-17 12:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, Gerd Hoffmann, Michael S. Tsirkin
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Add width_mm/height_mm to qemu_edid_info, and use it if it is
set (non-zero) to generate the EDID.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/display/edid-generate.c | 21 +++++++++++++--------
include/hw/display/edid.h | 2 ++
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index e58472fde5..14cfb94447 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -205,12 +205,8 @@ static void edid_desc_dummy(uint8_t *desc)
static void edid_desc_timing(uint8_t *desc,
uint32_t xres, uint32_t yres,
- uint32_t dpi)
+ uint32_t xmm, uint32_t ymm)
{
- /* physical display size */
- uint32_t xmm = xres * dpi / 254;
- uint32_t ymm = yres * dpi / 254;
-
/* pull some realistic looking timings out of thin air */
uint32_t xfront = xres * 25 / 100;
uint32_t xsync = xres * 3 / 100;
@@ -296,6 +292,7 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
uint32_t desc = 54;
uint8_t *xtra3 = NULL;
uint8_t *dta = NULL;
+ uint32_t width_mm, height_mm;
/* =============== set defaults =============== */
@@ -314,6 +311,13 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
if (!info->prefy) {
info->prefy = 768;
}
+ if (info->width_mm && info->height_mm) {
+ width_mm = info->width_mm;
+ height_mm = info->height_mm;
+ } else {
+ width_mm = info->prefx * info->dpi / 254;
+ height_mm = info->prefy * info->dpi / 254;
+ }
/* =============== extensions =============== */
@@ -360,8 +364,8 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
edid[20] = 0xa5;
/* screen size: undefined */
- edid[21] = info->prefx * 254 / 100 / info->dpi;
- edid[22] = info->prefy * 254 / 100 / info->dpi;
+ edid[21] = width_mm / 10;
+ edid[22] = height_mm / 10;
/* display gamma: 2.2 */
edid[23] = 220 - 100;
@@ -387,7 +391,8 @@ void qemu_edid_generate(uint8_t *edid, size_t size,
/* =============== descriptor blocks =============== */
- edid_desc_timing(edid + desc, info->prefx, info->prefy, info->dpi);
+ edid_desc_timing(edid + desc, info->prefx, info->prefy,
+ width_mm, height_mm);
desc += 18;
edid_desc_ranges(edid + desc);
diff --git a/include/hw/display/edid.h b/include/hw/display/edid.h
index 5b1de57f24..f98b49a944 100644
--- a/include/hw/display/edid.h
+++ b/include/hw/display/edid.h
@@ -6,6 +6,8 @@ typedef struct qemu_edid_info {
const char *name;
const char *serial;
uint32_t dpi;
+ uint16_t width_mm;
+ uint16_t height_mm;
uint32_t prefx;
uint32_t prefy;
uint32_t maxx;
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] edid: use physical dimensions if available
2020-08-17 12:00 ` [PATCH 1/4] edid: use physical dimensions if available marcandre.lureau
@ 2020-08-17 12:21 ` Gerd Hoffmann
2020-08-17 12:57 ` Marc-André Lureau
0 siblings, 1 reply; 8+ messages in thread
From: Gerd Hoffmann @ 2020-08-17 12:21 UTC (permalink / raw)
To: marcandre.lureau; +Cc: qemu-devel, Michael S. Tsirkin
On Mon, Aug 17, 2020 at 04:00:53PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Add width_mm/height_mm to qemu_edid_info, and use it if it is
> set (non-zero) to generate the EDID.
Any specific reason why you switch from dpi to xmm/ymm?
take care,
Gerd
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] edid: use physical dimensions if available
2020-08-17 12:21 ` Gerd Hoffmann
@ 2020-08-17 12:57 ` Marc-André Lureau
2020-08-18 6:25 ` Gerd Hoffmann
0 siblings, 1 reply; 8+ messages in thread
From: Marc-André Lureau @ 2020-08-17 12:57 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, Michael S. Tsirkin
Hi
On Mon, Aug 17, 2020 at 4:21 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Mon, Aug 17, 2020 at 04:00:53PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Add width_mm/height_mm to qemu_edid_info, and use it if it is
> > set (non-zero) to generate the EDID.
>
> Any specific reason why you switch from dpi to xmm/ymm?
Not really, but there is no DPI information from Gtk. I also find it
difficult to reason with DPI, dimensions are simpler to check about
correctness imho (I take the ruler from my desk for example ;). And
also DPI is a space density, without horizontal and vertical
distinction.
So by giving width/height in mm we actually have something more
correct and easier to debug imho. No?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] edid: use physical dimensions if available
2020-08-17 12:57 ` Marc-André Lureau
@ 2020-08-18 6:25 ` Gerd Hoffmann
0 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2020-08-18 6:25 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, Michael S. Tsirkin
On Mon, Aug 17, 2020 at 04:57:55PM +0400, Marc-André Lureau wrote:
> Hi
>
> On Mon, Aug 17, 2020 at 4:21 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Mon, Aug 17, 2020 at 04:00:53PM +0400, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Add width_mm/height_mm to qemu_edid_info, and use it if it is
> > > set (non-zero) to generate the EDID.
> >
> > Any specific reason why you switch from dpi to xmm/ymm?
>
> Not really, but there is no DPI information from Gtk. I also find it
> difficult to reason with DPI, dimensions are simpler to check about
> correctness imho (I take the ruler from my desk for example ;). And
> also DPI is a space density, without horizontal and vertical
> distinction.
Typically computer displays have square pixels, so that shouldn't be a
problem. For manually configuration it is easier if you have to deal
with one value only not two.
> So by giving width/height in mm we actually have something more
> correct and easier to debug imho. No?
I dislike having both with/height and dpi in struct qemu_edid_info.
Suggestion: Drop dpi struct member (should be easy, I think it isn't
wired anywhere yet). Add two little qemu_edid_* helpers to convert
from/to dpi. If only one of xmm/ymm is given go calculate the other
automatically (assuming square pixels). If none is given use 100 dpi
(like the current code does).
take care,
Gerd
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/4] ui: add getter for UIInfo
2020-08-17 12:00 [PATCH 0/4] WIP: add physical display dimensions to spice/virtio-gpu marcandre.lureau
2020-08-17 12:00 ` [PATCH 1/4] edid: use physical dimensions if available marcandre.lureau
@ 2020-08-17 12:00 ` marcandre.lureau
2020-08-17 12:00 ` [PATCH 3/4] spice: get monitors physical dimension marcandre.lureau
2020-08-17 12:00 ` [PATCH 4/4] virtio-gpu: set physical dimensions for EDID marcandre.lureau
3 siblings, 0 replies; 8+ messages in thread
From: marcandre.lureau @ 2020-08-17 12:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, Gerd Hoffmann, Michael S. Tsirkin
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The following patch is going to introduce extra fields / details to
UIInfo. Add a getter and keep the current values, instead of memset(0)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
ui/console.c | 7 +++++++
ui/spice-display.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/ui/console.c b/ui/console.c
index 0579be792f..2a0d191d4e 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1513,6 +1513,13 @@ bool dpy_ui_info_supported(QemuConsole *con)
return con->hw_ops->ui_info != NULL;
}
+const QemuUIInfo *dpy_get_ui_info(const QemuConsole *con)
+{
+ assert(con != NULL);
+
+ return &con->ui_info;
+}
+
int dpy_set_ui_info(QemuConsole *con, QemuUIInfo *info)
{
assert(con != NULL);
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 19632fdf6c..625d9232b9 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -672,7 +672,7 @@ static int interface_client_monitors_config(QXLInstance *sin,
return 1;
}
- memset(&info, 0, sizeof(info));
+ info = *dpy_get_ui_info(ssd->dcl.con);
if (mc->num_of_monitors == 1) {
/*
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] spice: get monitors physical dimension
2020-08-17 12:00 [PATCH 0/4] WIP: add physical display dimensions to spice/virtio-gpu marcandre.lureau
2020-08-17 12:00 ` [PATCH 1/4] edid: use physical dimensions if available marcandre.lureau
2020-08-17 12:00 ` [PATCH 2/4] ui: add getter for UIInfo marcandre.lureau
@ 2020-08-17 12:00 ` marcandre.lureau
2020-08-17 12:00 ` [PATCH 4/4] virtio-gpu: set physical dimensions for EDID marcandre.lureau
3 siblings, 0 replies; 8+ messages in thread
From: marcandre.lureau @ 2020-08-17 12:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, Gerd Hoffmann, Michael S. Tsirkin
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Note that for consistency, we use the same logic as MonitorsConfig to
figure out the associated monitor. However, I can't find traces of the
discussion/patches about the "new spice-server" behaviour: it still uses
the multiple-configurations path in git master.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/ui/console.h | 4 ++++
ui/spice-display.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/include/ui/console.h b/include/ui/console.h
index f35b4fc082..c334b92e70 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -133,6 +133,9 @@ typedef struct DisplaySurface {
} DisplaySurface;
typedef struct QemuUIInfo {
+ /* physical dimension */
+ uint16_t width_mm;
+ uint16_t height_mm;
/* geometry */
int xoff;
int yoff;
@@ -278,6 +281,7 @@ void update_displaychangelistener(DisplayChangeListener *dcl,
void unregister_displaychangelistener(DisplayChangeListener *dcl);
bool dpy_ui_info_supported(QemuConsole *con);
+const QemuUIInfo *dpy_get_ui_info(const QemuConsole *con);
int dpy_set_ui_info(QemuConsole *con, QemuUIInfo *info);
void dpy_gfx_update(QemuConsole *con, int x, int y, int w, int h);
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 625d9232b9..93d4e1c0a4 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -657,6 +657,53 @@ static void interface_set_client_capabilities(QXLInstance *sin,
/* nothing to do */
}
+#if SPICE_INTERFACE_QXL_MAJOR >= 3 && SPICE_INTERFACE_QXL_MINOR >= 4
+static int interface_client_monitors_mm(QXLInstance *sin, VDAgentMonitorsMM *mm)
+{
+ SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
+ QemuUIInfo info;
+ int head;
+
+ if (!dpy_ui_info_supported(ssd->dcl.con)) {
+ return 0; /* == not supported by guest */
+ }
+
+ if (!mm) {
+ return 1;
+ }
+
+ info = *dpy_get_ui_info(ssd->dcl.con);
+ /* Note: this code doesn't handle Spice multi-head support, where multiple
+ * monitor configuration for a single QXL device means multiple head. */
+ if (mm->num_of_monitors == 1) {
+ /*
+ * New spice-server version which filters the list of monitors
+ * to only include those that belong to our display channel.
+ *
+ * single-head configuration (where filtering doesn't matter)
+ * takes this code path too.
+ */
+ info.width_mm = mm->monitors[0].width;
+ info.height_mm = mm->monitors[0].height;
+ } else {
+ /*
+ * Old spice-server which gives us all monitors, so we have to
+ * figure ourself which entry we need. Array index is the
+ * channel_id, which is the qemu console index, see
+ * qemu_spice_add_display_interface().
+ */
+ head = qemu_console_get_index(ssd->dcl.con);
+ if (mm->num_of_monitors > head) {
+ info.width_mm = mm->monitors[head].width;
+ info.height_mm = mm->monitors[head].height;
+ }
+ }
+
+ dpy_set_ui_info(ssd->dcl.con, &info);
+ return 1;
+}
+#endif
+
static int interface_client_monitors_config(QXLInstance *sin,
VDAgentMonitorsConfig *mc)
{
@@ -674,6 +721,8 @@ static int interface_client_monitors_config(QXLInstance *sin,
info = *dpy_get_ui_info(ssd->dcl.con);
+ /* Note: this code doesn't handle Spice multi-head support, where multiple
+ * monitor configuration for a single QXL device means multiple head. */
if (mc->num_of_monitors == 1) {
/*
* New spice-server version which filters the list of monitors
@@ -728,6 +777,9 @@ static const QXLInterface dpy_interface = {
.update_area_complete = interface_update_area_complete,
.set_client_capabilities = interface_set_client_capabilities,
.client_monitors_config = interface_client_monitors_config,
+#if SPICE_INTERFACE_QXL_MAJOR >= 3 && SPICE_INTERFACE_QXL_MINOR >= 4
+ .client_monitors_mm = interface_client_monitors_mm,
+#endif
};
static void display_update(DisplayChangeListener *dcl,
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] virtio-gpu: set physical dimensions for EDID
2020-08-17 12:00 [PATCH 0/4] WIP: add physical display dimensions to spice/virtio-gpu marcandre.lureau
` (2 preceding siblings ...)
2020-08-17 12:00 ` [PATCH 3/4] spice: get monitors physical dimension marcandre.lureau
@ 2020-08-17 12:00 ` marcandre.lureau
3 siblings, 0 replies; 8+ messages in thread
From: marcandre.lureau @ 2020-08-17 12:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, Gerd Hoffmann, Michael S. Tsirkin
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/display/virtio-gpu-base.c | 2 ++
hw/display/virtio-gpu.c | 2 ++
include/hw/virtio/virtio-gpu.h | 1 +
3 files changed, 5 insertions(+)
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 7961308606..2fb8beb7da 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -82,6 +82,8 @@ static int virtio_gpu_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info)
g->req_state[idx].y = info->yoff;
g->req_state[idx].width = info->width;
g->req_state[idx].height = info->height;
+ g->req_state[idx].width_mm = info->width_mm;
+ g->req_state[idx].height_mm = info->height_mm;
if (info->width && info->height) {
g->enabled_output_bitmask |= (1 << idx);
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 5f0dd7c150..28c390078a 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -212,6 +212,8 @@ virtio_gpu_generate_edid(VirtIOGPU *g, int scanout,
{
VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
qemu_edid_info info = {
+ .width_mm = b->req_state[scanout].width_mm,
+ .height_mm = b->req_state[scanout].height_mm,
.prefx = b->req_state[scanout].width,
.prefy = b->req_state[scanout].height,
};
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 6dd57f2025..3d1adc563c 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -65,6 +65,7 @@ struct virtio_gpu_scanout {
};
struct virtio_gpu_requested_state {
+ uint16_t width_mm, height_mm;
uint32_t width, height;
int x, y;
};
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread