qxl: fix primary surface handling

Submitted by Gerd Hoffmann on Sept. 11, 2017, 9:39 a.m.

Details

Message ID 20170911093950.22401-1-kraxel@redhat.com
State New
Headers show
Series "qxl: fix primary surface handling" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Gerd Hoffmann Sept. 11, 2017, 9:39 a.m.
The atomic conversion of the qxl driver didn't got the primary surface
handling completely right.  It works in the common simple cases, but
fails for example when changing the display resolution using xrandr or
in multihead setups.

The rules are simple:  There is one primary surface.  Before defining a
new one you have to destroy the old one.

This patch makes qxl_primary_atomic_update() destroy the primary surface
before defining a new one.  It fixes is_primary flag updates.  It adds
is_primary checks so we don't try to update the primary surface in case
it already has the state we want it being in.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_display.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 14c5613b43..e1dd05423e 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -509,23 +509,25 @@  static void qxl_primary_atomic_update(struct drm_plane *plane,
 	    .y2 = qfb->base.height
 	};
 
-	if (!old_state->fb) {
-		qxl_io_log(qdev,
-			   "create primary fb: %dx%d,%d,%d\n",
-			   bo->surf.width, bo->surf.height,
-			   bo->surf.stride, bo->surf.format);
-
-		qxl_io_create_primary(qdev, 0, bo);
-		bo->is_primary = true;
-		return;
-
-	} else {
+	if (old_state->fb) {
 		qfb_old = to_qxl_framebuffer(old_state->fb);
 		bo_old = gem_to_qxl_bo(qfb_old->obj);
+	} else {
+		bo_old = NULL;
+	}
+
+	if (bo == bo_old)
+		return;
+
+	if (bo_old && bo_old->is_primary) {
+		qxl_io_destroy_primary(qdev);
 		bo_old->is_primary = false;
 	}
 
-	bo->is_primary = true;
+	if (!bo->is_primary) {
+		qxl_io_create_primary(qdev, 0, bo);
+		bo->is_primary = true;
+	}
 	qxl_draw_dirty_fb(qdev, qfb, bo, 0, 0, &norect, 1, 1);
 }
 
@@ -534,13 +536,15 @@  static void qxl_primary_atomic_disable(struct drm_plane *plane,
 {
 	struct qxl_device *qdev = plane->dev->dev_private;
 
-	if (old_state->fb)
-	{	struct qxl_framebuffer *qfb =
+	if (old_state->fb) {
+		struct qxl_framebuffer *qfb =
 			to_qxl_framebuffer(old_state->fb);
 		struct qxl_bo *bo = gem_to_qxl_bo(qfb->obj);
 
-		qxl_io_destroy_primary(qdev);
-		bo->is_primary = false;
+		if (bo->is_primary) {
+			qxl_io_destroy_primary(qdev);
+			bo->is_primary = false;
+		}
 	}
 }
 

Comments

[CC-ing Gabriel]

Hi Gerd,

On 11 September 2017 at 10:39, Gerd Hoffmann <kraxel@redhat.com> wrote:
> The atomic conversion of the qxl driver didn't got the primary surface
> handling completely right.  It works in the common simple cases, but
> fails for example when changing the display resolution using xrandr or
> in multihead setups.
>
> The rules are simple:  There is one primary surface.  Before defining a
> new one you have to destroy the old one.
>
> This patch makes qxl_primary_atomic_update() destroy the primary surface
> before defining a new one.  It fixes is_primary flag updates.  It adds
> is_primary checks so we don't try to update the primary surface in case
> it already has the state we want it being in.
>
Please include a fixes tag alongside the bug reports (if it addresses
both that is). Adding cross-references is very beneficial.

Fixes: 3538e80a869b ("drm: qxl: Atomic phase 1: Implement mode_set_nofb")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102338
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196777


-Emil
Gerd Hoffmann <kraxel@redhat.com> writes:

> The atomic conversion of the qxl driver didn't got the primary surface
> handling completely right.  It works in the common simple cases, but
> fails for example when changing the display resolution using xrandr or
> in multihead setups.
>
> The rules are simple:  There is one primary surface.  Before defining a
> new one you have to destroy the old one.
>
> This patch makes qxl_primary_atomic_update() destroy the primary surface
> before defining a new one.  It fixes is_primary flag updates.  It adds
> is_primary checks so we don't try to update the primary surface in case
> it already has the state we want it being in.
>

Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>