[PATCH 1/3] drm/ttm: Make ttm_bo_mem_compat available

[PATCH 1/3] drm/ttm: Make ttm_bo_mem_compat available

[syeh]

From: Sinclair Yeh <syeh@vmware.com>

There are cases where it is desired to see if a proposed placement
is compatible with a buffer object before calling ttm_bo_validate().

Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Cc: <stable@vger.kernel.org>
---
This is the first of a 3-patch series to fix a black screen
issue observed on Ubuntu 16.04 server.
---
 drivers/gpu/drm/ttm/ttm_bo.c |  7 ++++---
 include/drm/ttm/ttm_bo_api.h | 14 ++++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 39386f5..a71cf98 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1034,9 +1034,9 @@ out_unlock:
 	return ret;
 }
 
-static bool ttm_bo_mem_compat(struct ttm_placement *placement,
-			      struct ttm_mem_reg *mem,
-			      uint32_t *new_flags)
+bool ttm_bo_mem_compat(struct ttm_placement *placement,
+		       struct ttm_mem_reg *mem,
+		       uint32_t *new_flags)
 {
 	int i;
 
@@ -1068,6 +1068,7 @@ static bool ttm_bo_mem_compat(struct ttm_placement *placement,
 
 	return false;
 }
+EXPORT_SYMBOL(ttm_bo_mem_compat);
 
 int ttm_bo_validate(struct ttm_buffer_object *bo,
 			struct ttm_placement *placement,
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index c801d90..4cecb0b 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -316,6 +316,20 @@ ttm_bo_reference(struct ttm_buffer_object *bo)
  */
 extern int ttm_bo_wait(struct ttm_buffer_object *bo,
 		       bool interruptible, bool no_wait);
+
+/**
+ * ttm_bo_mem_compat - Check if proposed placement is compatible with a bo
+ *
+ * @placement:  Return immediately if buffer is busy.
+ * @mem:  The struct ttm_mem_reg indicating the region where the bo resides
+ * @new_flags: Describes compatible placement found
+ *
+ * Returns true if the placement is compatible
+ */
+extern bool ttm_bo_mem_compat(struct ttm_placement *placement,
+			      struct ttm_mem_reg *mem,
+			      uint32_t *new_flags);
+
 /**
  * ttm_bo_validate
  *
-- 
2.8.2

[PATCH 1/3] drm/ttm: Make ttm_bo_mem_compat available

[syeh]

From: Sinclair Yeh <syeh@vmware.com>

There are cases where it is desired to see if a proposed placement
is compatible with a buffer object before calling ttm_bo_validate().

Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Cc: <stable@vger.kernel.org>
---
This is the first of a 3-patch series to fix a black screen
issue observed on Ubuntu 16.04 server.
---
 drivers/gpu/drm/ttm/ttm_bo.c |  7 ++++---
 include/drm/ttm/ttm_bo_api.h | 14 ++++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 39386f5..a71cf98 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1034,9 +1034,9 @@ out_unlock:
 	return ret;
 }
 
-static bool ttm_bo_mem_compat(struct ttm_placement *placement,
-			      struct ttm_mem_reg *mem,
-			      uint32_t *new_flags)
+bool ttm_bo_mem_compat(struct ttm_placement *placement,
+		       struct ttm_mem_reg *mem,
+		       uint32_t *new_flags)
 {
 	int i;
 
@@ -1068,6 +1068,7 @@ static bool ttm_bo_mem_compat(struct ttm_placement *placement,
 
 	return false;
 }
+EXPORT_SYMBOL(ttm_bo_mem_compat);
 
 int ttm_bo_validate(struct ttm_buffer_object *bo,
 			struct ttm_placement *placement,
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index c801d90..4cecb0b 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -316,6 +316,20 @@ ttm_bo_reference(struct ttm_buffer_object *bo)
  */
 extern int ttm_bo_wait(struct ttm_buffer_object *bo,
 		       bool interruptible, bool no_wait);
+
+/**
+ * ttm_bo_mem_compat - Check if proposed placement is compatible with a bo
+ *
+ * @placement:  Return immediately if buffer is busy.
+ * @mem:  The struct ttm_mem_reg indicating the region where the bo resides
+ * @new_flags: Describes compatible placement found
+ *
+ * Returns true if the placement is compatible
+ */
+extern bool ttm_bo_mem_compat(struct ttm_placement *placement,
+			      struct ttm_mem_reg *mem,
+			      uint32_t *new_flags);
+
 /**
  * ttm_bo_validate
  *
-- 
2.8.2

[PATCH 2/3] drm/vmwgfx: Check pin count before attempting to move a buffer

[syeh]

From: Sinclair Yeh <syeh@vmware.com>

In certain scenarios, e.g. when fbdev is enabled, we can get into
a situation where a vmw_framebuffer_pin() is called on a buffer
that is already pinned.

When this happens, ttm_bo_validate() will unintentially remove the
TTM_PL_FLAG_NO_EVICT flag, thus unpinning it, and leaving no way
to actually pin the buffer again.

To prevent this, if a buffer is already pinned, then instead of
calling ttm_bo_validate(), just make sure the proposed placement is
compatible with the existing placement.

Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Cc: <stable@vger.kernel.org>
---
This is the 2nd patch in a 3-patch series to fix a console black
screen issue on Ubuntu 16.04 server.  This fixes a BUG_ON()
condition where a pinned buffer gets accidentally put onto the
LRU list.
---
 drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c
index 9b078a4..0cd8890 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c
@@ -49,6 +49,7 @@ int vmw_dmabuf_pin_in_placement(struct vmw_private *dev_priv,
 {
 	struct ttm_buffer_object *bo = &buf->base;
 	int ret;
+	uint32_t new_flags;
 
 	ret = ttm_write_lock(&dev_priv->reservation_sem, interruptible);
 	if (unlikely(ret != 0))
@@ -60,7 +61,12 @@ int vmw_dmabuf_pin_in_placement(struct vmw_private *dev_priv,
 	if (unlikely(ret != 0))
 		goto err;
 
-	ret = ttm_bo_validate(bo, placement, interruptible, false);
+	if (buf->pin_count > 0)
+		ret = ttm_bo_mem_compat(placement, &bo->mem,
+					&new_flags) == true ? 0 : -EINVAL;
+	else
+		ret = ttm_bo_validate(bo, placement, interruptible, false);
+
 	if (!ret)
 		vmw_bo_pin_reserved(buf, true);
 
@@ -91,6 +97,7 @@ int vmw_dmabuf_pin_in_vram_or_gmr(struct vmw_private *dev_priv,
 {
 	struct ttm_buffer_object *bo = &buf->base;
 	int ret;
+	uint32_t new_flags;
 
 	ret = ttm_write_lock(&dev_priv->reservation_sem, interruptible);
 	if (unlikely(ret != 0))
@@ -102,6 +109,12 @@ int vmw_dmabuf_pin_in_vram_or_gmr(struct vmw_private *dev_priv,
 	if (unlikely(ret != 0))
 		goto err;
 
+	if (buf->pin_count > 0) {
+		ret = ttm_bo_mem_compat(&vmw_vram_gmr_placement, &bo->mem,
+					&new_flags) == true ? 0 : -EINVAL;
+		goto out_unreserve;
+	}
+
 	ret = ttm_bo_validate(bo, &vmw_vram_gmr_placement, interruptible,
 			      false);
 	if (likely(ret == 0) || ret == -ERESTARTSYS)
@@ -161,6 +174,7 @@ int vmw_dmabuf_pin_in_start_of_vram(struct vmw_private *dev_priv,
 	struct ttm_placement placement;
 	struct ttm_place place;
 	int ret = 0;
+	uint32_t new_flags;
 
 	place = vmw_vram_placement.placement[0];
 	place.lpfn = bo->num_pages;
@@ -185,10 +199,15 @@ int vmw_dmabuf_pin_in_start_of_vram(struct vmw_private *dev_priv,
 	 */
 	if (bo->mem.mem_type == TTM_PL_VRAM &&
 	    bo->mem.start < bo->num_pages &&
-	    bo->mem.start > 0)
+	    bo->mem.start > 0 &&
+	    buf->pin_count == 0)
 		(void) ttm_bo_validate(bo, &vmw_sys_placement, false, false);
 
-	ret = ttm_bo_validate(bo, &placement, interruptible, false);
+	if (buf->pin_count > 0)
+		ret = ttm_bo_mem_compat(&placement, &bo->mem,
+					&new_flags) == true ? 0 : -EINVAL;
+	else
+		ret = ttm_bo_validate(bo, &placement, interruptible, false);
 
 	/* For some reason we didn't end up at the start of vram */
 	WARN_ON(ret == 0 && bo->offset != 0);
-- 
2.8.2

[PATCH 2/3] drm/vmwgfx: Check pin count before attempting to move a buffer

[syeh]

From: Sinclair Yeh <syeh@vmware.com>

In certain scenarios, e.g. when fbdev is enabled, we can get into
a situation where a vmw_framebuffer_pin() is called on a buffer
that is already pinned.

When this happens, ttm_bo_validate() will unintentially remove the
TTM_PL_FLAG_NO_EVICT flag, thus unpinning it, and leaving no way
to actually pin the buffer again.

To prevent this, if a buffer is already pinned, then instead of
calling ttm_bo_validate(), just make sure the proposed placement is
compatible with the existing placement.

Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Cc: <stable@vger.kernel.org>
---
This is the 2nd patch in a 3-patch series to fix a console black
screen issue on Ubuntu 16.04 server.  This fixes a BUG_ON()
condition where a pinned buffer gets accidentally put onto the
LRU list.
---
 drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c
index 9b078a4..0cd8890 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c
@@ -49,6 +49,7 @@ int vmw_dmabuf_pin_in_placement(struct vmw_private *dev_priv,
 {
 	struct ttm_buffer_object *bo = &buf->base;
 	int ret;
+	uint32_t new_flags;
 
 	ret = ttm_write_lock(&dev_priv->reservation_sem, interruptible);
 	if (unlikely(ret != 0))
@@ -60,7 +61,12 @@ int vmw_dmabuf_pin_in_placement(struct vmw_private *dev_priv,
 	if (unlikely(ret != 0))
 		goto err;
 
-	ret = ttm_bo_validate(bo, placement, interruptible, false);
+	if (buf->pin_count > 0)
+		ret = ttm_bo_mem_compat(placement, &bo->mem,
+					&new_flags) == true ? 0 : -EINVAL;
+	else
+		ret = ttm_bo_validate(bo, placement, interruptible, false);
+
 	if (!ret)
 		vmw_bo_pin_reserved(buf, true);
 
@@ -91,6 +97,7 @@ int vmw_dmabuf_pin_in_vram_or_gmr(struct vmw_private *dev_priv,
 {
 	struct ttm_buffer_object *bo = &buf->base;
 	int ret;
+	uint32_t new_flags;
 
 	ret = ttm_write_lock(&dev_priv->reservation_sem, interruptible);
 	if (unlikely(ret != 0))
@@ -102,6 +109,12 @@ int vmw_dmabuf_pin_in_vram_or_gmr(struct vmw_private *dev_priv,
 	if (unlikely(ret != 0))
 		goto err;
 
+	if (buf->pin_count > 0) {
+		ret = ttm_bo_mem_compat(&vmw_vram_gmr_placement, &bo->mem,
+					&new_flags) == true ? 0 : -EINVAL;
+		goto out_unreserve;
+	}
+
 	ret = ttm_bo_validate(bo, &vmw_vram_gmr_placement, interruptible,
 			      false);
 	if (likely(ret == 0) || ret == -ERESTARTSYS)
@@ -161,6 +174,7 @@ int vmw_dmabuf_pin_in_start_of_vram(struct vmw_private *dev_priv,
 	struct ttm_placement placement;
 	struct ttm_place place;
 	int ret = 0;
+	uint32_t new_flags;
 
 	place = vmw_vram_placement.placement[0];
 	place.lpfn = bo->num_pages;
@@ -185,10 +199,15 @@ int vmw_dmabuf_pin_in_start_of_vram(struct vmw_private *dev_priv,
 	 */
 	if (bo->mem.mem_type == TTM_PL_VRAM &&
 	    bo->mem.start < bo->num_pages &&
-	    bo->mem.start > 0)
+	    bo->mem.start > 0 &&
+	    buf->pin_count == 0)
 		(void) ttm_bo_validate(bo, &vmw_sys_placement, false, false);
 
-	ret = ttm_bo_validate(bo, &placement, interruptible, false);
+	if (buf->pin_count > 0)
+		ret = ttm_bo_mem_compat(&placement, &bo->mem,
+					&new_flags) == true ? 0 : -EINVAL;
+	else
+		ret = ttm_bo_validate(bo, &placement, interruptible, false);
 
 	/* For some reason we didn't end up at the start of vram */
 	WARN_ON(ret == 0 && bo->offset != 0);
-- 
2.8.2

[PATCH 3/3] drm/vmwgfx: Delay pinning fbdev framebuffer until after mode set

[syeh]

From: Sinclair Yeh <syeh@vmware.com>

For the Screen Object display unit, we need to reserve a
guest-invisible region equal to the size of the framebuffer for
the host.  This region can only be reserved in VRAM, whereas
the guest-visible framebuffer can be reserved in either VRAM or
GMR.

As such priority should be given to the guest-invisible
region otherwise in a limited VRAM situation, we can fail to
allocate this region.

This patch makes it so that vmw_sou_backing_alloc() is called
before the framebuffer is pinned.

Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Cc: <stable@vger.kernel.org>
---
This is the last patch of a 3-patch series to fix console black
screen issue on Ubuntu 16.04 server
---
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 47 ++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index 679a4cb..66eaa30 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -517,28 +517,6 @@ static int vmw_fb_kms_framebuffer(struct fb_info *info)
 
 	par->set_fb = &vfb->base;
 
-	if (!par->bo_ptr) {
-		/*
-		 * Pin before mapping. Since we don't know in what placement
-		 * to pin, call into KMS to do it for us.
-		 */
-		ret = vfb->pin(vfb);
-		if (ret) {
-			DRM_ERROR("Could not pin the fbdev framebuffer.\n");
-			return ret;
-		}
-
-		ret = ttm_bo_kmap(&par->vmw_bo->base, 0,
-				  par->vmw_bo->base.num_pages, &par->map);
-		if (ret) {
-			vfb->unpin(vfb);
-			DRM_ERROR("Could not map the fbdev framebuffer.\n");
-			return ret;
-		}
-
-		par->bo_ptr = ttm_kmap_obj_virtual(&par->map, &par->bo_iowrite);
-	}
-
 	return 0;
 }
 
@@ -601,6 +579,31 @@ static int vmw_fb_set_par(struct fb_info *info)
 	if (ret)
 		goto out_unlock;
 
+	if (!par->bo_ptr) {
+		struct vmw_framebuffer *vfb = vmw_framebuffer_to_vfb(set.fb);
+
+		/*
+		 * Pin before mapping. Since we don't know in what placement
+		 * to pin, call into KMS to do it for us.
+		 */
+		ret = vfb->pin(vfb);
+		if (ret) {
+			DRM_ERROR("Could not pin the fbdev framebuffer.\n");
+			return ret;
+		}
+
+		ret = ttm_bo_kmap(&par->vmw_bo->base, 0,
+				  par->vmw_bo->base.num_pages, &par->map);
+		if (ret) {
+			vfb->unpin(vfb);
+			DRM_ERROR("Could not map the fbdev framebuffer.\n");
+			return ret;
+		}
+
+		par->bo_ptr = ttm_kmap_obj_virtual(&par->map, &par->bo_iowrite);
+	}
+
+
 	vmw_fb_dirty_mark(par, par->fb_x, par->fb_y,
 			  par->set_fb->width, par->set_fb->height);
 
-- 
2.8.2

[PATCH 3/3] drm/vmwgfx: Delay pinning fbdev framebuffer until after mode set

[syeh]

From: Sinclair Yeh <syeh@vmware.com>

For the Screen Object display unit, we need to reserve a
guest-invisible region equal to the size of the framebuffer for
the host.  This region can only be reserved in VRAM, whereas
the guest-visible framebuffer can be reserved in either VRAM or
GMR.

As such priority should be given to the guest-invisible
region otherwise in a limited VRAM situation, we can fail to
allocate this region.

This patch makes it so that vmw_sou_backing_alloc() is called
before the framebuffer is pinned.

Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Cc: <stable@vger.kernel.org>
---
This is the last patch of a 3-patch series to fix console black
screen issue on Ubuntu 16.04 server
---
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 47 ++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index 679a4cb..66eaa30 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -517,28 +517,6 @@ static int vmw_fb_kms_framebuffer(struct fb_info *info)
 
 	par->set_fb = &vfb->base;
 
-	if (!par->bo_ptr) {
-		/*
-		 * Pin before mapping. Since we don't know in what placement
-		 * to pin, call into KMS to do it for us.
-		 */
-		ret = vfb->pin(vfb);
-		if (ret) {
-			DRM_ERROR("Could not pin the fbdev framebuffer.\n");
-			return ret;
-		}
-
-		ret = ttm_bo_kmap(&par->vmw_bo->base, 0,
-				  par->vmw_bo->base.num_pages, &par->map);
-		if (ret) {
-			vfb->unpin(vfb);
-			DRM_ERROR("Could not map the fbdev framebuffer.\n");
-			return ret;
-		}
-
-		par->bo_ptr = ttm_kmap_obj_virtual(&par->map, &par->bo_iowrite);
-	}
-
 	return 0;
 }
 
@@ -601,6 +579,31 @@ static int vmw_fb_set_par(struct fb_info *info)
 	if (ret)
 		goto out_unlock;
 
+	if (!par->bo_ptr) {
+		struct vmw_framebuffer *vfb = vmw_framebuffer_to_vfb(set.fb);
+
+		/*
+		 * Pin before mapping. Since we don't know in what placement
+		 * to pin, call into KMS to do it for us.
+		 */
+		ret = vfb->pin(vfb);
+		if (ret) {
+			DRM_ERROR("Could not pin the fbdev framebuffer.\n");
+			return ret;
+		}
+
+		ret = ttm_bo_kmap(&par->vmw_bo->base, 0,
+				  par->vmw_bo->base.num_pages, &par->map);
+		if (ret) {
+			vfb->unpin(vfb);
+			DRM_ERROR("Could not map the fbdev framebuffer.\n");
+			return ret;
+		}
+
+		par->bo_ptr = ttm_kmap_obj_virtual(&par->map, &par->bo_iowrite);
+	}
+
+
 	vmw_fb_dirty_mark(par, par->fb_x, par->fb_y,
 			  par->set_fb->width, par->set_fb->height);
 
-- 
2.8.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/vmwgfx: Delay pinning fbdev framebuffer until after mode set

[Emil Velikov]

Hi Sinclair,

On 1 July 2016 at 19:24,  <syeh@vmware.com> wrote:
> From: Sinclair Yeh <syeh@vmware.com>
>
> For the Screen Object display unit, we need to reserve a
> guest-invisible region equal to the size of the framebuffer for
> the host.  This region can only be reserved in VRAM, whereas
> the guest-visible framebuffer can be reserved in either VRAM or
> GMR.
>
> As such priority should be given to the guest-invisible
> region otherwise in a limited VRAM situation, we can fail to
> allocate this region.
>
> This patch makes it so that vmw_sou_backing_alloc() is called
> before the framebuffer is pinned.
>
> Signed-off-by: Sinclair Yeh <syeh@vmware.com>
> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: <stable@vger.kernel.org>
> ---
> This is the last patch of a 3-patch series to fix console black
> screen issue on Ubuntu 16.04 server
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 47 ++++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> index 679a4cb..66eaa30 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> @@ -517,28 +517,6 @@ static int vmw_fb_kms_framebuffer(struct fb_info *info)
>
>         par->set_fb = &vfb->base;
>
> -       if (!par->bo_ptr) {
> -               /*
> -                * Pin before mapping. Since we don't know in what placement
> -                * to pin, call into KMS to do it for us.
> -                */
> -               ret = vfb->pin(vfb);
> -               if (ret) {
> -                       DRM_ERROR("Could not pin the fbdev framebuffer.\n");
> -                       return ret;
> -               }
> -
> -               ret = ttm_bo_kmap(&par->vmw_bo->base, 0,
> -                                 par->vmw_bo->base.num_pages, &par->map);
> -               if (ret) {
> -                       vfb->unpin(vfb);
> -                       DRM_ERROR("Could not map the fbdev framebuffer.\n");
> -                       return ret;
> -               }
> -
> -               par->bo_ptr = ttm_kmap_obj_virtual(&par->map, &par->bo_iowrite);
> -       }
> -
>         return 0;
>  }
>
> @@ -601,6 +579,31 @@ static int vmw_fb_set_par(struct fb_info *info)
>         if (ret)
>                 goto out_unlock;
>
> +       if (!par->bo_ptr) {
> +               struct vmw_framebuffer *vfb = vmw_framebuffer_to_vfb(set.fb);
> +
> +               /*
> +                * Pin before mapping. Since we don't know in what placement
> +                * to pin, call into KMS to do it for us.
> +                */
> +               ret = vfb->pin(vfb);
> +               if (ret) {
> +                       DRM_ERROR("Could not pin the fbdev framebuffer.\n");
> +                       return ret;
> +               }
> +
> +               ret = ttm_bo_kmap(&par->vmw_bo->base, 0,
> +                                 par->vmw_bo->base.num_pages, &par->map);
> +               if (ret) {
> +                       vfb->unpin(vfb);
> +                       DRM_ERROR("Could not map the fbdev framebuffer.\n");
> +                       return ret;
> +               }
> +
> +               par->bo_ptr = ttm_kmap_obj_virtual(&par->map, &par->bo_iowrite);
> +       }
> +
> +
Seems like you forgot to update the error paths. Thankfully we
shouldn't be reaching those too often, if at all.

-Emil

Re: [PATCH 3/3] drm/vmwgfx: Delay pinning fbdev framebuffer until after mode set

[Sinclair Yeh]

Argh, you're right.  I'll send out a patch.  Thanks for catching this!

Sinclair

On Mon, Jul 04, 2016 at 04:51:52PM +0100, Emil Velikov wrote:
> Hi Sinclair,
> 
> On 1 July 2016 at 19:24,  <syeh@vmware.com> wrote:
> > From: Sinclair Yeh <syeh@vmware.com>
> >
> > For the Screen Object display unit, we need to reserve a
> > guest-invisible region equal to the size of the framebuffer for
> > the host.  This region can only be reserved in VRAM, whereas
> > the guest-visible framebuffer can be reserved in either VRAM or
> > GMR.
> >
> > As such priority should be given to the guest-invisible
> > region otherwise in a limited VRAM situation, we can fail to
> > allocate this region.
> >
> > This patch makes it so that vmw_sou_backing_alloc() is called
> > before the framebuffer is pinned.
> >
> > Signed-off-by: Sinclair Yeh <syeh@vmware.com>
> > Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> > This is the last patch of a 3-patch series to fix console black
> > screen issue on Ubuntu 16.04 server
> > ---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 47 ++++++++++++++++++++------------------
> >  1 file changed, 25 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> > index 679a4cb..66eaa30 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> > @@ -517,28 +517,6 @@ static int vmw_fb_kms_framebuffer(struct fb_info *info)
> >
> >         par->set_fb = &vfb->base;
> >
> > -       if (!par->bo_ptr) {
> > -               /*
> > -                * Pin before mapping. Since we don't know in what placement
> > -                * to pin, call into KMS to do it for us.
> > -                */
> > -               ret = vfb->pin(vfb);
> > -               if (ret) {
> > -                       DRM_ERROR("Could not pin the fbdev framebuffer.\n");
> > -                       return ret;
> > -               }
> > -
> > -               ret = ttm_bo_kmap(&par->vmw_bo->base, 0,
> > -                                 par->vmw_bo->base.num_pages, &par->map);
> > -               if (ret) {
> > -                       vfb->unpin(vfb);
> > -                       DRM_ERROR("Could not map the fbdev framebuffer.\n");
> > -                       return ret;
> > -               }
> > -
> > -               par->bo_ptr = ttm_kmap_obj_virtual(&par->map, &par->bo_iowrite);
> > -       }
> > -
> >         return 0;
> >  }
> >
> > @@ -601,6 +579,31 @@ static int vmw_fb_set_par(struct fb_info *info)
> >         if (ret)
> >                 goto out_unlock;
> >
> > +       if (!par->bo_ptr) {
> > +               struct vmw_framebuffer *vfb = vmw_framebuffer_to_vfb(set.fb);
> > +
> > +               /*
> > +                * Pin before mapping. Since we don't know in what placement
> > +                * to pin, call into KMS to do it for us.
> > +                */
> > +               ret = vfb->pin(vfb);
> > +               if (ret) {
> > +                       DRM_ERROR("Could not pin the fbdev framebuffer.\n");
> > +                       return ret;
> > +               }
> > +
> > +               ret = ttm_bo_kmap(&par->vmw_bo->base, 0,
> > +                                 par->vmw_bo->base.num_pages, &par->map);
> > +               if (ret) {
> > +                       vfb->unpin(vfb);
> > +                       DRM_ERROR("Could not map the fbdev framebuffer.\n");
> > +                       return ret;
> > +               }
> > +
> > +               par->bo_ptr = ttm_kmap_obj_virtual(&par->map, &par->bo_iowrite);
> > +       }
> > +
> > +
> Seems like you forgot to update the error paths. Thankfully we
> shouldn't be reaching those too often, if at all.
> 
> -Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel