* [PATCH drm/qxl v4 0/7] qxl: Various fixes
@ 2016-11-08 9:12 Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 1/7] qxl: Mark some internal functions as static Christophe Fergeau
` (6 more replies)
0 siblings, 7 replies; 9+ messages in thread
From: Christophe Fergeau @ 2016-11-08 9:12 UTC (permalink / raw)
To: spice-devel; +Cc: airlied, Gerd Hoffmann, dri-devel
Hey,
Here is what should hopefully be the final version of this series, only change
since v3 is that I made sure to keep "PATCH" in the subject line, and I added
missing Acked-by/Signed-of-by annotations to the logs.
Christophe
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH drm/qxl v4 1/7] qxl: Mark some internal functions as static
2016-11-08 9:12 [PATCH drm/qxl v4 0/7] qxl: Various fixes Christophe Fergeau
@ 2016-11-08 9:12 ` Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 2/7] qxl: Remove unused prototype Christophe Fergeau
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Christophe Fergeau @ 2016-11-08 9:12 UTC (permalink / raw)
To: spice-devel; +Cc: airlied, Gerd Hoffmann, dri-devel
They are not used outside of their respective source file
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
---
drivers/gpu/drm/qxl/qxl_cmd.c | 2 +-
drivers/gpu/drm/qxl/qxl_display.c | 4 ++--
drivers/gpu/drm/qxl/qxl_drv.h | 3 ---
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 04270f5..74fc936 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -578,7 +578,7 @@ int qxl_hw_surface_dealloc(struct qxl_device *qdev,
return 0;
}
-int qxl_update_surface(struct qxl_device *qdev, struct qxl_bo *surf)
+static int qxl_update_surface(struct qxl_device *qdev, struct qxl_bo *surf)
{
struct qxl_rect rect;
int ret;
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 3aef127..8cf5177 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -36,7 +36,7 @@ static bool qxl_head_enabled(struct qxl_head *head)
return head->width && head->height;
}
-void qxl_alloc_client_monitors_config(struct qxl_device *qdev, unsigned count)
+static void qxl_alloc_client_monitors_config(struct qxl_device *qdev, unsigned count)
{
if (qdev->client_monitors_config &&
count > qdev->client_monitors_config->count) {
@@ -559,7 +559,7 @@ static bool qxl_crtc_mode_fixup(struct drm_crtc *crtc,
return true;
}
-void
+static void
qxl_send_monitors_config(struct qxl_device *qdev)
{
int i;
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 8e633ca..a3c1131 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -394,13 +394,11 @@ qxl_framebuffer_init(struct drm_device *dev,
struct drm_gem_object *obj,
const struct drm_framebuffer_funcs *funcs);
void qxl_display_read_client_monitors_config(struct qxl_device *qdev);
-void qxl_send_monitors_config(struct qxl_device *qdev);
int qxl_create_monitors_object(struct qxl_device *qdev);
int qxl_destroy_monitors_object(struct qxl_device *qdev);
/* used by qxl_debugfs only */
void qxl_crtc_set_from_monitors_config(struct qxl_device *qdev);
-void qxl_alloc_client_monitors_config(struct qxl_device *qdev, unsigned count);
/* qxl_gem.c */
int qxl_gem_init(struct qxl_device *qdev);
@@ -573,6 +571,5 @@ int qxl_bo_check_id(struct qxl_device *qdev, struct qxl_bo *bo);
struct qxl_drv_surface *
qxl_surface_lookup(struct drm_device *dev, int surface_id);
void qxl_surface_evict(struct qxl_device *qdev, struct qxl_bo *surf, bool freeing);
-int qxl_update_surface(struct qxl_device *qdev, struct qxl_bo *surf);
#endif
--
2.9.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH drm/qxl v4 2/7] qxl: Remove unused prototype
2016-11-08 9:12 [PATCH drm/qxl v4 0/7] qxl: Various fixes Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 1/7] qxl: Mark some internal functions as static Christophe Fergeau
@ 2016-11-08 9:12 ` Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 3/7] qxl: Add missing '\n' to qxl_io_log() call Christophe Fergeau
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Christophe Fergeau @ 2016-11-08 9:12 UTC (permalink / raw)
To: spice-devel; +Cc: airlied, Gerd Hoffmann, dri-devel
qxl_crtc_set_from_monitors_config() is defined in qxl_drv.h but never
implemented.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
---
drivers/gpu/drm/qxl/qxl_drv.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index a3c1131..5a4720a 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -397,9 +397,6 @@ void qxl_display_read_client_monitors_config(struct qxl_device *qdev);
int qxl_create_monitors_object(struct qxl_device *qdev);
int qxl_destroy_monitors_object(struct qxl_device *qdev);
-/* used by qxl_debugfs only */
-void qxl_crtc_set_from_monitors_config(struct qxl_device *qdev);
-
/* qxl_gem.c */
int qxl_gem_init(struct qxl_device *qdev);
void qxl_gem_fini(struct qxl_device *qdev);
--
2.9.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH drm/qxl v4 3/7] qxl: Add missing '\n' to qxl_io_log() call
2016-11-08 9:12 [PATCH drm/qxl v4 0/7] qxl: Various fixes Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 1/7] qxl: Mark some internal functions as static Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 2/7] qxl: Remove unused prototype Christophe Fergeau
@ 2016-11-08 9:12 ` Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 4/7] qxl: Call qxl_gem_{init, fini} Christophe Fergeau
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Christophe Fergeau @ 2016-11-08 9:12 UTC (permalink / raw)
To: spice-devel; +Cc: airlied, Gerd Hoffmann, dri-devel
The message has to be terminated by a newline as it's not going to get
added automatically.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
---
drivers/gpu/drm/qxl/qxl_fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
index 28c1423..969c7c1 100644
--- a/drivers/gpu/drm/qxl/qxl_fb.c
+++ b/drivers/gpu/drm/qxl/qxl_fb.c
@@ -198,7 +198,7 @@ static int qxlfb_framebuffer_dirty(struct drm_framebuffer *fb,
/*
* we are using a shadow draw buffer, at qdev->surface0_shadow
*/
- qxl_io_log(qdev, "dirty x[%d, %d], y[%d, %d]", clips->x1, clips->x2,
+ qxl_io_log(qdev, "dirty x[%d, %d], y[%d, %d]\n", clips->x1, clips->x2,
clips->y1, clips->y2);
image->dx = clips->x1;
image->dy = clips->y1;
--
2.9.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH drm/qxl v4 4/7] qxl: Call qxl_gem_{init, fini}
2016-11-08 9:12 [PATCH drm/qxl v4 0/7] qxl: Various fixes Christophe Fergeau
` (2 preceding siblings ...)
2016-11-08 9:12 ` [PATCH drm/qxl v4 3/7] qxl: Add missing '\n' to qxl_io_log() call Christophe Fergeau
@ 2016-11-08 9:12 ` Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 5/7] qxl: Remove qxl_bo_init() return value Christophe Fergeau
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Christophe Fergeau @ 2016-11-08 9:12 UTC (permalink / raw)
To: spice-devel; +Cc: airlied, Gerd Hoffmann, dri-devel
qdev->gem.objects was initialized directly in qxl_device_init() rather
than going through qxl_gem_init(), and qxl_gem_fini() was never called.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
---
drivers/gpu/drm/qxl/qxl_kms.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index e642242..af685f1 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -131,7 +131,7 @@ static int qxl_device_init(struct qxl_device *qdev,
mutex_init(&qdev->update_area_mutex);
mutex_init(&qdev->release_mutex);
mutex_init(&qdev->surf_evict_mutex);
- INIT_LIST_HEAD(&qdev->gem.objects);
+ qxl_gem_init(qdev);
qdev->rom_base = pci_resource_start(pdev, 2);
qdev->rom_size = pci_resource_len(pdev, 2);
@@ -273,6 +273,7 @@ static void qxl_device_fini(struct qxl_device *qdev)
qxl_ring_free(qdev->command_ring);
qxl_ring_free(qdev->cursor_ring);
qxl_ring_free(qdev->release_ring);
+ qxl_gem_fini(qdev);
qxl_bo_fini(qdev);
io_mapping_free(qdev->surface_mapping);
io_mapping_free(qdev->vram_mapping);
--
2.9.3
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH drm/qxl v4 5/7] qxl: Remove qxl_bo_init() return value
2016-11-08 9:12 [PATCH drm/qxl v4 0/7] qxl: Various fixes Christophe Fergeau
` (3 preceding siblings ...)
2016-11-08 9:12 ` [PATCH drm/qxl v4 4/7] qxl: Call qxl_gem_{init, fini} Christophe Fergeau
@ 2016-11-08 9:12 ` Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 6/7] qxl: Don't notify userspace when monitors config is unchanged Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 7/7] qxl: Allow resolution which are not multiple of 8 Christophe Fergeau
6 siblings, 0 replies; 9+ messages in thread
From: Christophe Fergeau @ 2016-11-08 9:12 UTC (permalink / raw)
To: spice-devel; +Cc: airlied, Gerd Hoffmann, dri-devel
It's always returning 0, and it's always ignored.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
---
drivers/gpu/drm/qxl/qxl_drv.h | 2 +-
drivers/gpu/drm/qxl/qxl_gem.c | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 5a4720a..feac7e6 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -398,7 +398,7 @@ int qxl_create_monitors_object(struct qxl_device *qdev);
int qxl_destroy_monitors_object(struct qxl_device *qdev);
/* qxl_gem.c */
-int qxl_gem_init(struct qxl_device *qdev);
+void qxl_gem_init(struct qxl_device *qdev);
void qxl_gem_fini(struct qxl_device *qdev);
int qxl_gem_object_create(struct qxl_device *qdev, int size,
int alignment, int initial_domain,
diff --git a/drivers/gpu/drm/qxl/qxl_gem.c b/drivers/gpu/drm/qxl/qxl_gem.c
index d9746e9..3f185c4 100644
--- a/drivers/gpu/drm/qxl/qxl_gem.c
+++ b/drivers/gpu/drm/qxl/qxl_gem.c
@@ -111,10 +111,9 @@ void qxl_gem_object_close(struct drm_gem_object *obj,
{
}
-int qxl_gem_init(struct qxl_device *qdev)
+void qxl_gem_init(struct qxl_device *qdev)
{
INIT_LIST_HEAD(&qdev->gem.objects);
- return 0;
}
void qxl_gem_fini(struct qxl_device *qdev)
--
2.9.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH drm/qxl v4 6/7] qxl: Don't notify userspace when monitors config is unchanged
2016-11-08 9:12 [PATCH drm/qxl v4 0/7] qxl: Various fixes Christophe Fergeau
` (4 preceding siblings ...)
2016-11-08 9:12 ` [PATCH drm/qxl v4 5/7] qxl: Remove qxl_bo_init() return value Christophe Fergeau
@ 2016-11-08 9:12 ` Christophe Fergeau
2016-11-08 9:44 ` Frediano Ziglio
2016-11-08 9:12 ` [PATCH drm/qxl v4 7/7] qxl: Allow resolution which are not multiple of 8 Christophe Fergeau
6 siblings, 1 reply; 9+ messages in thread
From: Christophe Fergeau @ 2016-11-08 9:12 UTC (permalink / raw)
To: spice-devel; +Cc: airlied, Gerd Hoffmann, dri-devel
When the QXL driver receives a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt,
we currently always notify userspace that there was some hotplug event.
However, gnome-shell/mutter is reacting to this event by attempting a
resolution change, which it does by issueing drmModeRmFB, drmModeAddFB,
and then drmModeSetCrtc. This has the side-effect of causing
qxl_crtc_mode_set() to tell the QXL virtual hardware that a primary
surface was destroyed and created. After going through QEMU and then the
remote SPICE client, a new identical monitors config message will be
sent, resulting in a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt to
be emitted, and the same scenario occurring again.
As destroying/creating the primary surface causes a visible screen
flicker, this makes the guest hard to use (
https://bugzilla.redhat.com/show_bug.cgi?id=1266484 ).
This commit checks if the screen configuration we received is the same
one as the current one, and does not notify userspace about it if that's
the case.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
---
drivers/gpu/drm/qxl/qxl_display.c | 62 ++++++++++++++++++++++++++++++++-------
1 file changed, 52 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 8cf5177..518333c 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -57,11 +57,18 @@ static void qxl_alloc_client_monitors_config(struct qxl_device *qdev, unsigned c
qdev->client_monitors_config->count = count;
}
+enum {
+ MONITORS_CONFIG_MODIFIED,
+ MONITORS_CONFIG_UNCHANGED,
+ MONITORS_CONFIG_BAD_CRC,
+};
+
static int qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
{
int i;
int num_monitors;
uint32_t crc;
+ int status = MONITORS_CONFIG_UNCHANGED;
num_monitors = qdev->rom->client_monitors_config.count;
crc = crc32(0, (const uint8_t *)&qdev->rom->client_monitors_config,
@@ -70,7 +77,7 @@ static int qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
qxl_io_log(qdev, "crc mismatch: have %X (%zd) != %X\n", crc,
sizeof(qdev->rom->client_monitors_config),
qdev->rom->client_monitors_config_crc);
- return 1;
+ return MONITORS_CONFIG_BAD_CRC;
}
if (num_monitors > qdev->monitors_config->max_allowed) {
DRM_DEBUG_KMS("client monitors list will be truncated: %d < %d\n",
@@ -79,6 +86,10 @@ static int qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
} else {
num_monitors = qdev->rom->client_monitors_config.count;
}
+ if (qdev->client_monitors_config
+ && (num_monitors != qdev->client_monitors_config->count)) {
+ status = MONITORS_CONFIG_MODIFIED;
+ }
qxl_alloc_client_monitors_config(qdev, num_monitors);
/* we copy max from the client but it isn't used */
qdev->client_monitors_config->max_allowed =
@@ -88,17 +99,39 @@ static int qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
&qdev->rom->client_monitors_config.heads[i];
struct qxl_head *client_head =
&qdev->client_monitors_config->heads[i];
- client_head->x = c_rect->left;
- client_head->y = c_rect->top;
- client_head->width = c_rect->right - c_rect->left;
- client_head->height = c_rect->bottom - c_rect->top;
- client_head->surface_id = 0;
- client_head->id = i;
- client_head->flags = 0;
+ if (client_head->x != c_rect->left) {
+ client_head->x = c_rect->left;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->y != c_rect->top) {
+ client_head->y = c_rect->top;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->width != c_rect->right - c_rect->left) {
+ client_head->width = c_rect->right - c_rect->left;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->height != c_rect->bottom - c_rect->top) {
+ client_head->height = c_rect->bottom - c_rect->top;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->surface_id != 0) {
+ client_head->surface_id = 0;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->id != i) {
+ client_head->id = i;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
+ if (client_head->flags != 0) {
+ client_head->flags = 0;
+ status = MONITORS_CONFIG_MODIFIED;
+ }
DRM_DEBUG_KMS("read %dx%d+%d+%d\n", client_head->width, client_head->height,
client_head->x, client_head->y);
}
- return 0;
+
+ return status;
}
static void qxl_update_offset_props(struct qxl_device *qdev)
@@ -124,9 +157,18 @@ void qxl_display_read_client_monitors_config(struct qxl_device *qdev)
{
struct drm_device *dev = qdev->ddev;
- while (qxl_display_copy_rom_client_monitors_config(qdev)) {
+ int status;
+
+ status = qxl_display_copy_rom_client_monitors_config(qdev);
+ while (status == MONITORS_CONFIG_BAD_CRC) {
qxl_io_log(qdev, "failed crc check for client_monitors_config,"
" retrying\n");
+ status = qxl_display_copy_rom_client_monitors_config(qdev);
+ }
+ if (status == MONITORS_CONFIG_UNCHANGED) {
+ qxl_io_log(qdev, "config unchanged\n");
+ DRM_DEBUG("ignoring unchanged client monitors config");
+ return;
}
drm_modeset_lock_all(dev);
--
2.9.3
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH drm/qxl v4 7/7] qxl: Allow resolution which are not multiple of 8
2016-11-08 9:12 [PATCH drm/qxl v4 0/7] qxl: Various fixes Christophe Fergeau
` (5 preceding siblings ...)
2016-11-08 9:12 ` [PATCH drm/qxl v4 6/7] qxl: Don't notify userspace when monitors config is unchanged Christophe Fergeau
@ 2016-11-08 9:12 ` Christophe Fergeau
6 siblings, 0 replies; 9+ messages in thread
From: Christophe Fergeau @ 2016-11-08 9:12 UTC (permalink / raw)
To: spice-devel; +Cc: airlied, Gerd Hoffmann, dri-devel
The use of drm_cvt_mode() in qxl_add_monitors_config_modes() means that
the resolutions we are going to present to user-space are going to be
rounded down to a multiple of 8. In the QXL arbitrary resolution case,
this is not useful.
This commit forces the actual width/height that was requested by the
client in the drm_display_mode structure rather than keeping the
rounded version.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
---
drivers/gpu/drm/qxl/qxl_display.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 518333c..fa99481 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -199,6 +199,9 @@ static int qxl_add_monitors_config_modes(struct drm_connector *connector,
mode = drm_cvt_mode(dev, head->width, head->height, 60, false, false,
false);
mode->type |= DRM_MODE_TYPE_PREFERRED;
+ mode->hdisplay = head->width;
+ mode->vdisplay = head->height;
+ drm_mode_set_name(mode);
*pwidth = head->width;
*pheight = head->height;
drm_mode_probed_add(connector, mode);
--
2.9.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH drm/qxl v4 6/7] qxl: Don't notify userspace when monitors config is unchanged
2016-11-08 9:12 ` [PATCH drm/qxl v4 6/7] qxl: Don't notify userspace when monitors config is unchanged Christophe Fergeau
@ 2016-11-08 9:44 ` Frediano Ziglio
0 siblings, 0 replies; 9+ messages in thread
From: Frediano Ziglio @ 2016-11-08 9:44 UTC (permalink / raw)
To: Christophe Fergeau; +Cc: spice-devel, Gerd Hoffmann, dri-devel, airlied
> When the QXL driver receives a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG
> interrupt,
> we currently always notify userspace that there was some hotplug event.
>
> However, gnome-shell/mutter is reacting to this event by attempting a
> resolution change, which it does by issueing drmModeRmFB, drmModeAddFB,
> and then drmModeSetCrtc. This has the side-effect of causing
> qxl_crtc_mode_set() to tell the QXL virtual hardware that a primary
> surface was destroyed and created. After going through QEMU and then the
> remote SPICE client, a new identical monitors config message will be
> sent, resulting in a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt to
> be emitted, and the same scenario occurring again.
>
> As destroying/creating the primary surface causes a visible screen
> flicker, this makes the guest hard to use (
> https://bugzilla.redhat.com/show_bug.cgi?id=1266484 ).
>
> This commit checks if the screen configuration we received is the same
> one as the current one, and does not notify userspace about it if that's
> the case.
>
> Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
> ---
> drivers/gpu/drm/qxl/qxl_display.c | 62
> ++++++++++++++++++++++++++++++++-------
> 1 file changed, 52 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c
> b/drivers/gpu/drm/qxl/qxl_display.c
> index 8cf5177..518333c 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -57,11 +57,18 @@ static void qxl_alloc_client_monitors_config(struct
> qxl_device *qdev, unsigned c
> qdev->client_monitors_config->count = count;
> }
>
> +enum {
> + MONITORS_CONFIG_MODIFIED,
> + MONITORS_CONFIG_UNCHANGED,
> + MONITORS_CONFIG_BAD_CRC,
> +};
> +
> static int qxl_display_copy_rom_client_monitors_config(struct qxl_device
> *qdev)
> {
> int i;
> int num_monitors;
> uint32_t crc;
> + int status = MONITORS_CONFIG_UNCHANGED;
>
> num_monitors = qdev->rom->client_monitors_config.count;
> crc = crc32(0, (const uint8_t *)&qdev->rom->client_monitors_config,
> @@ -70,7 +77,7 @@ static int
> qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
> qxl_io_log(qdev, "crc mismatch: have %X (%zd) != %X\n", crc,
> sizeof(qdev->rom->client_monitors_config),
> qdev->rom->client_monitors_config_crc);
> - return 1;
> + return MONITORS_CONFIG_BAD_CRC;
> }
> if (num_monitors > qdev->monitors_config->max_allowed) {
> DRM_DEBUG_KMS("client monitors list will be truncated: %d < %d\n",
> @@ -79,6 +86,10 @@ static int
> qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
> } else {
> num_monitors = qdev->rom->client_monitors_config.count;
> }
> + if (qdev->client_monitors_config
> + && (num_monitors != qdev->client_monitors_config->count)) {
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> qxl_alloc_client_monitors_config(qdev, num_monitors);
> /* we copy max from the client but it isn't used */
> qdev->client_monitors_config->max_allowed =
> @@ -88,17 +99,39 @@ static int
> qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev)
> &qdev->rom->client_monitors_config.heads[i];
> struct qxl_head *client_head =
> &qdev->client_monitors_config->heads[i];
> - client_head->x = c_rect->left;
> - client_head->y = c_rect->top;
> - client_head->width = c_rect->right - c_rect->left;
> - client_head->height = c_rect->bottom - c_rect->top;
> - client_head->surface_id = 0;
> - client_head->id = i;
> - client_head->flags = 0;
> + if (client_head->x != c_rect->left) {
> + client_head->x = c_rect->left;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> + if (client_head->y != c_rect->top) {
> + client_head->y = c_rect->top;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> + if (client_head->width != c_rect->right - c_rect->left) {
> + client_head->width = c_rect->right - c_rect->left;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> + if (client_head->height != c_rect->bottom - c_rect->top) {
> + client_head->height = c_rect->bottom - c_rect->top;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> + if (client_head->surface_id != 0) {
> + client_head->surface_id = 0;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> + if (client_head->id != i) {
> + client_head->id = i;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> + if (client_head->flags != 0) {
> + client_head->flags = 0;
> + status = MONITORS_CONFIG_MODIFIED;
> + }
> DRM_DEBUG_KMS("read %dx%d+%d+%d\n", client_head->width,
> client_head->height,
> client_head->x, client_head->y);
> }
> - return 0;
> +
> + return status;
> }
>
> static void qxl_update_offset_props(struct qxl_device *qdev)
> @@ -124,9 +157,18 @@ void qxl_display_read_client_monitors_config(struct
> qxl_device *qdev)
> {
>
> struct drm_device *dev = qdev->ddev;
> - while (qxl_display_copy_rom_client_monitors_config(qdev)) {
> + int status;
> +
> + status = qxl_display_copy_rom_client_monitors_config(qdev);
> + while (status == MONITORS_CONFIG_BAD_CRC) {
> qxl_io_log(qdev, "failed crc check for client_monitors_config,"
> " retrying\n");
> + status = qxl_display_copy_rom_client_monitors_config(qdev);
> + }
> + if (status == MONITORS_CONFIG_UNCHANGED) {
> + qxl_io_log(qdev, "config unchanged\n");
> + DRM_DEBUG("ignoring unchanged client monitors config");
> + return;
> }
>
> drm_modeset_lock_all(dev);
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-11-08 9:44 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-08 9:12 [PATCH drm/qxl v4 0/7] qxl: Various fixes Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 1/7] qxl: Mark some internal functions as static Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 2/7] qxl: Remove unused prototype Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 3/7] qxl: Add missing '\n' to qxl_io_log() call Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 4/7] qxl: Call qxl_gem_{init, fini} Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 5/7] qxl: Remove qxl_bo_init() return value Christophe Fergeau
2016-11-08 9:12 ` [PATCH drm/qxl v4 6/7] qxl: Don't notify userspace when monitors config is unchanged Christophe Fergeau
2016-11-08 9:44 ` Frediano Ziglio
2016-11-08 9:12 ` [PATCH drm/qxl v4 7/7] qxl: Allow resolution which are not multiple of 8 Christophe Fergeau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox