public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Also check view->type for a normal GGTT view
@ 2018-02-20 10:50 Chris Wilson
  2018-02-20 10:50 ` [PATCH 2/2] drm/i915: Move the policy for placement of the GGTT vma into the caller Chris Wilson
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Chris Wilson @ 2018-02-20 10:50 UTC (permalink / raw)
  To: intel-gfx

We cannot simply use !view as shorthand for all normal GGTT views as a
few callers will always populate a i915_ggtt_view struct and set the
type to NORMAL instead. So check for (!view || view->type == NORMAL)
inside i915_gem_object_ggtt_pin().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 631a2db2bb6e..1854a69bc354 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4282,7 +4282,8 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
 
 	lockdep_assert_held(&obj->base.dev->struct_mutex);
 
-	if (!view && flags & PIN_MAPPABLE) {
+	if ((!view || view->type == I915_GGTT_VIEW_NORMAL) &&
+	    flags & PIN_MAPPABLE) {
 		/* If the required space is larger than the available
 		 * aperture, we will not able to find a slot for the
 		 * object and unbinding the object now will be in
-- 
2.16.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] drm/i915: Move the policy for placement of the GGTT vma into the caller
  2018-02-20 10:50 [PATCH 1/2] drm/i915: Also check view->type for a normal GGTT view Chris Wilson
@ 2018-02-20 10:50 ` Chris Wilson
  2018-02-20 11:39   ` Ville Syrjälä
  2018-02-20 11:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Also check view->type for a normal GGTT view Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2018-02-20 10:50 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ville

Currently we make the unilateral decision inside
i915_gem_object_pin_to_display() where the VMA should resided (inside
the fence and mappable region or above?). This is not our decision to
make as it impacts on how the display engine can use the resulting
scanout object, and it would rather instruct us where to place the VMA so
that it can enable the features it wants. As such, make the pin flags an
argument to i915_gem_object_pin_to_display() and control them from
intel_pin_and_fence_fb_obj()

Whilst taking control of the mapping for ourselves, start tracking how
we use it to avoid trying to free a fence we never claimed:

<3>[  227.151869] GEM_BUG_ON(vma->fence->pin_count <= 0)
<4>[  227.152064] ------------[ cut here ]------------
<2>[  227.152068] kernel BUG at drivers/gpu/drm/i915/i915_vma.h:391!
<4>[  227.152084] invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
<0>[  227.152092] Dumping ftrace buffer:
<0>[  227.152099]    (ftrace buffer empty)
<4>[  227.152102] Modules linked in: i915 snd_hda_codec_analog snd_hda_codec_generic coretemp snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm lpc_ich e1000e mei_me mei prime_numbers
<4>[  227.152131] CPU: 1 PID: 1587 Comm: kworker/u16:49 Tainted: G     U           4.16.0-rc1-gbab67b2f6177-kasan_7+ #1
<4>[  227.152134] Hardware name: Dell Inc. OptiPlex 755                 /0PU052, BIOS A08 02/19/2008
<4>[  227.152236] Workqueue: events_unbound intel_atomic_commit_work [i915]
<4>[  227.152292] RIP: 0010:intel_unpin_fb_vma+0x23a/0x2a0 [i915]
<4>[  227.152295] RSP: 0018:ffff88005aad7b68 EFLAGS: 00010286
<4>[  227.152300] RAX: 0000000000000026 RBX: ffff88005c359580 RCX: 0000000000000000
<4>[  227.152304] RDX: 0000000000000026 RSI: ffffffff8707d840 RDI: ffffed000b55af63
<4>[  227.152307] RBP: ffff880056817e58 R08: 0000000000000001 R09: 0000000000000000
<4>[  227.152311] R10: ffff88005aad7b88 R11: 0000000000000000 R12: ffff8800568184d0
<4>[  227.152314] R13: ffff880065b5ab08 R14: 0000000000000000 R15: dffffc0000000000
<4>[  227.152318] FS:  0000000000000000(0000) GS:ffff88006ac40000(0000) knlGS:0000000000000000
<4>[  227.152322] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[  227.152325] CR2: 00007f5fb25550a8 CR3: 0000000068c78000 CR4: 00000000000006e0
<4>[  227.152328] Call Trace:
<4>[  227.152385]  intel_cleanup_plane_fb+0x6b/0xd0 [i915]
<4>[  227.152395]  drm_atomic_helper_cleanup_planes+0x166/0x280
<4>[  227.152452]  intel_atomic_commit_tail+0x159d/0x3380 [i915]
<4>[  227.152463]  ? process_one_work+0x66e/0x1460
<4>[  227.152516]  ? skl_update_crtcs+0x9c0/0x9c0 [i915]
<4>[  227.152523]  ? lock_acquire+0x13d/0x390
<4>[  227.152527]  ? lock_acquire+0x13d/0x390
<4>[  227.152534]  process_one_work+0x71a/0x1460
<4>[  227.152540]  ? __schedule+0x815/0x1e20
<4>[  227.152547]  ? pwq_dec_nr_in_flight+0x2b0/0x2b0
<4>[  227.152553]  ? _raw_spin_lock_irq+0xa/0x40
<4>[  227.152559]  worker_thread+0xdf/0xf60
<4>[  227.152569]  ? process_one_work+0x1460/0x1460
<4>[  227.152573]  kthread+0x2cf/0x3c0
<4>[  227.152578]  ? _kthread_create_on_node+0xa0/0xa0
<4>[  227.152583]  ret_from_fork+0x3a/0x50
<4>[  227.152591] Code: c6 00 11 86 c0 48 c7 c7 e0 bd 85 c0 e8 60 e7 a9 c4 0f ff e9 1f fe ff ff 48 c7 c6 40 10 86 c0 48 c7 c7 e0 ca 85 c0 e8 2b 95 bd c4 <0f> 0b 48 89 ef e8 4c 44 e8 c4 e9 ef fd ff ff e8 42 44 e8 c4 e9
<1>[  227.152720] RIP: intel_unpin_fb_vma+0x23a/0x2a0 [i915] RSP: ffff88005aad7b68

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com
---
 drivers/gpu/drm/i915/i915_drv.h           |  3 ++-
 drivers/gpu/drm/i915/i915_gem.c           | 26 ++++++------------
 drivers/gpu/drm/i915/intel_atomic_plane.c |  1 +
 drivers/gpu/drm/i915/intel_display.c      | 45 ++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h          |  9 +++++--
 drivers/gpu/drm/i915/intel_fbdev.c        | 10 ++++---
 drivers/gpu/drm/i915/intel_overlay.c      |  3 ++-
 7 files changed, 62 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 76bfe909168c..20575b3ee406 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3413,7 +3413,8 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write);
 struct i915_vma * __must_check
 i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 				     u32 alignment,
-				     const struct i915_ggtt_view *view);
+				     const struct i915_ggtt_view *view,
+				     unsigned int flags);
 void i915_gem_object_unpin_from_display_plane(struct i915_vma *vma);
 int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
 				int align);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1854a69bc354..bdc2069d5433 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4078,7 +4078,8 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,
 struct i915_vma *
 i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 				     u32 alignment,
-				     const struct i915_ggtt_view *view)
+				     const struct i915_ggtt_view *view,
+				     unsigned int flags)
 {
 	struct i915_vma *vma;
 	int ret;
@@ -4115,25 +4116,14 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 	 * try to preserve the existing ABI).
 	 */
 	vma = ERR_PTR(-ENOSPC);
-	if (!view || view->type == I915_GGTT_VIEW_NORMAL)
+	if ((flags & PIN_MAPPABLE) == 0 &&
+	    (!view || view->type == I915_GGTT_VIEW_NORMAL))
 		vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment,
-					       PIN_MAPPABLE | PIN_NONBLOCK);
-	if (IS_ERR(vma)) {
-		struct drm_i915_private *i915 = to_i915(obj->base.dev);
-		unsigned int flags;
-
-		/* Valleyview is definitely limited to scanning out the first
-		 * 512MiB. Lets presume this behaviour was inherited from the
-		 * g4x display engine and that all earlier gen are similarly
-		 * limited. Testing suggests that it is a little more
-		 * complicated than this. For example, Cherryview appears quite
-		 * happy to scanout from anywhere within its global aperture.
-		 */
-		flags = 0;
-		if (HAS_GMCH_DISPLAY(i915))
-			flags = PIN_MAPPABLE;
+					       flags |
+					       PIN_MAPPABLE |
+					       PIN_NONBLOCK);
+	if (IS_ERR(vma))
 		vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment, flags);
-	}
 	if (IS_ERR(vma))
 		goto err_unpin_global;
 
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 0ee32275994a..7481ce85746b 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -85,6 +85,7 @@ intel_plane_duplicate_state(struct drm_plane *plane)
 	__drm_atomic_helper_plane_duplicate_state(plane, state);
 
 	intel_state->vma = NULL;
+	intel_state->flags = 0;
 
 	return state;
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 280c04326f64..8075e1990157 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2068,7 +2068,9 @@ static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
 }
 
 struct i915_vma *
-intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
+intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
+			   unsigned int rotation,
+			   unsigned long *out_flags)
 {
 	struct drm_device *dev = fb->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -2076,6 +2078,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
 	struct i915_ggtt_view view;
 	struct i915_vma *vma;
 	u32 alignment;
+	unsigned int pinctl;
 
 	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
 
@@ -2102,7 +2105,20 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
 
 	atomic_inc(&dev_priv->gpu_error.pending_fb_pin);
 
-	vma = i915_gem_object_pin_to_display_plane(obj, alignment, &view);
+	pinctl = 0;
+
+	/* Valleyview is definitely limited to scanning out the first
+	 * 512MiB. Lets presume this behaviour was inherited from the
+	 * g4x display engine and that all earlier gen are similarly
+	 * limited. Testing suggests that it is a little more
+	 * complicated than this. For example, Cherryview appears quite
+	 * happy to scanout from anywhere within its global aperture.
+	 */
+	if (HAS_GMCH_DISPLAY(dev_priv))
+		pinctl |= PIN_MAPPABLE;
+
+	vma = i915_gem_object_pin_to_display_plane(obj,
+						   alignment, &view, pinctl);
 	if (IS_ERR(vma))
 		goto err;
 
@@ -2123,7 +2139,8 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
 		 * something and try to run the system in a "less than optimal"
 		 * mode that matches the user configuration.
 		 */
-		i915_vma_pin_fence(vma);
+		if (i915_vma_pin_fence(vma) == 0)
+			*out_flags |= PLANE_HAS_FENCE;
 	}
 
 	i915_vma_get(vma);
@@ -2134,11 +2151,12 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
 	return vma;
 }
 
-void intel_unpin_fb_vma(struct i915_vma *vma)
+void intel_unpin_fb_vma(struct i915_vma *vma, unsigned long flags)
 {
 	lockdep_assert_held(&vma->vm->i915->drm.struct_mutex);
 
-	i915_vma_unpin_fence(vma);
+	if (flags & PLANE_HAS_FENCE)
+		i915_vma_unpin_fence(vma);
 	i915_gem_object_unpin_from_display_plane(vma);
 	i915_vma_put(vma);
 }
@@ -2808,7 +2826,9 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 valid_fb:
 	mutex_lock(&dev->struct_mutex);
 	intel_state->vma =
-		intel_pin_and_fence_fb_obj(fb, primary->state->rotation);
+		intel_pin_and_fence_fb_obj(fb,
+					   primary->state->rotation,
+					   &intel_state->flags);
 	mutex_unlock(&dev->struct_mutex);
 	if (IS_ERR(intel_state->vma)) {
 		DRM_ERROR("failed to pin boot fb on pipe %d: %li\n",
@@ -12713,7 +12733,9 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 	} else {
 		struct i915_vma *vma;
 
-		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
+		vma = intel_pin_and_fence_fb_obj(fb,
+						 new_state->rotation,
+						 &to_intel_plane_state(new_state)->flags);
 		if (!IS_ERR(vma))
 			to_intel_plane_state(new_state)->vma = vma;
 		else
@@ -12768,7 +12790,7 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 	vma = fetch_and_zero(&to_intel_plane_state(old_state)->vma);
 	if (vma) {
 		mutex_lock(&plane->dev->struct_mutex);
-		intel_unpin_fb_vma(vma);
+		intel_unpin_fb_vma(vma, to_intel_plane_state(old_state)->flags);
 		mutex_unlock(&plane->dev->struct_mutex);
 	}
 }
@@ -13129,7 +13151,9 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 			goto out_unlock;
 		}
 	} else {
-		vma = intel_pin_and_fence_fb_obj(fb, new_plane_state->rotation);
+		vma = intel_pin_and_fence_fb_obj(fb,
+						 new_plane_state->rotation,
+						 &to_intel_plane_state(new_plane_state)->flags);
 		if (IS_ERR(vma)) {
 			DRM_DEBUG_KMS("failed to pin object\n");
 
@@ -13160,7 +13184,8 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 
 	old_vma = fetch_and_zero(&to_intel_plane_state(old_plane_state)->vma);
 	if (old_vma)
-		intel_unpin_fb_vma(old_vma);
+		intel_unpin_fb_vma(old_vma,
+				   to_intel_plane_state(old_plane_state)->flags);
 
 out_unlock:
 	mutex_unlock(&dev_priv->drm.struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 898064e8bea7..50874f4035cf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -204,6 +204,7 @@ struct intel_fbdev {
 	struct drm_fb_helper helper;
 	struct intel_framebuffer *fb;
 	struct i915_vma *vma;
+	unsigned long vma_flags;
 	async_cookie_t cookie;
 	int preferred_bpp;
 };
@@ -490,6 +491,8 @@ struct intel_atomic_state {
 struct intel_plane_state {
 	struct drm_plane_state base;
 	struct i915_vma *vma;
+	unsigned long flags;
+#define PLANE_HAS_FENCE BIT(0)
 
 	struct {
 		u32 offset;
@@ -1503,8 +1506,10 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 				    struct intel_load_detect_pipe *old,
 				    struct drm_modeset_acquire_ctx *ctx);
 struct i915_vma *
-intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
-void intel_unpin_fb_vma(struct i915_vma *vma);
+intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
+			   unsigned int rotation,
+			   unsigned long *out_flags);
+void intel_unpin_fb_vma(struct i915_vma *vma, unsigned long flags);
 struct drm_framebuffer *
 intel_framebuffer_create(struct drm_i915_gem_object *obj,
 			 struct drm_mode_fb_cmd2 *mode_cmd);
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index da48af11eb6b..3d8ce3a62743 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -177,6 +177,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	struct fb_info *info;
 	struct drm_framebuffer *fb;
 	struct i915_vma *vma;
+	unsigned long flags = 0;
 	bool prealloc = false;
 	void __iomem *vaddr;
 	int ret;
@@ -211,7 +212,9 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	 * This also validates that any existing fb inherited from the
 	 * BIOS is suitable for own access.
 	 */
-	vma = intel_pin_and_fence_fb_obj(&ifbdev->fb->base, DRM_MODE_ROTATE_0);
+	vma = intel_pin_and_fence_fb_obj(&ifbdev->fb->base,
+					 DRM_MODE_ROTATE_0,
+					 &flags);
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
 		goto out_unlock;
@@ -268,6 +271,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	DRM_DEBUG_KMS("allocated %dx%d fb: 0x%08x\n",
 		      fb->width, fb->height, i915_ggtt_offset(vma));
 	ifbdev->vma = vma;
+	ifbdev->vma_flags = flags;
 
 	intel_runtime_pm_put(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
@@ -275,7 +279,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	return 0;
 
 out_unpin:
-	intel_unpin_fb_vma(vma);
+	intel_unpin_fb_vma(vma, flags);
 out_unlock:
 	intel_runtime_pm_put(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
@@ -513,7 +517,7 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
 
 	if (ifbdev->vma) {
 		mutex_lock(&ifbdev->helper.dev->struct_mutex);
-		intel_unpin_fb_vma(ifbdev->vma);
+		intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags);
 		mutex_unlock(&ifbdev->helper.dev->struct_mutex);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 41e9465d44a8..89f568e739ee 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -801,7 +801,8 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 
 	atomic_inc(&dev_priv->gpu_error.pending_fb_pin);
 
-	vma = i915_gem_object_pin_to_display_plane(new_bo, 0, NULL);
+	vma = i915_gem_object_pin_to_display_plane(new_bo,
+						   0, NULL, PIN_MAPPABLE);
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
 		goto out_pin_section;
-- 
2.16.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drm/i915: Move the policy for placement of the GGTT vma into the caller
  2018-02-20 10:50 ` [PATCH 2/2] drm/i915: Move the policy for placement of the GGTT vma into the caller Chris Wilson
@ 2018-02-20 11:39   ` Ville Syrjälä
  2018-02-20 11:45     ` Chris Wilson
  0 siblings, 1 reply; 8+ messages in thread
From: Ville Syrjälä @ 2018-02-20 11:39 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, Ville

On Tue, Feb 20, 2018 at 10:50:11AM +0000, Chris Wilson wrote:
> Currently we make the unilateral decision inside
> i915_gem_object_pin_to_display() where the VMA should resided (inside
> the fence and mappable region or above?). This is not our decision to
> make as it impacts on how the display engine can use the resulting
> scanout object, and it would rather instruct us where to place the VMA so
> that it can enable the features it wants. As such, make the pin flags an
> argument to i915_gem_object_pin_to_display() and control them from
> intel_pin_and_fence_fb_obj()
> 
> Whilst taking control of the mapping for ourselves, start tracking how
> we use it to avoid trying to free a fence we never claimed:
> 
> <3>[  227.151869] GEM_BUG_ON(vma->fence->pin_count <= 0)
> <4>[  227.152064] ------------[ cut here ]------------
> <2>[  227.152068] kernel BUG at drivers/gpu/drm/i915/i915_vma.h:391!
> <4>[  227.152084] invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI
> <0>[  227.152092] Dumping ftrace buffer:
> <0>[  227.152099]    (ftrace buffer empty)
> <4>[  227.152102] Modules linked in: i915 snd_hda_codec_analog snd_hda_codec_generic coretemp snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm lpc_ich e1000e mei_me mei prime_numbers
> <4>[  227.152131] CPU: 1 PID: 1587 Comm: kworker/u16:49 Tainted: G     U           4.16.0-rc1-gbab67b2f6177-kasan_7+ #1
> <4>[  227.152134] Hardware name: Dell Inc. OptiPlex 755                 /0PU052, BIOS A08 02/19/2008
> <4>[  227.152236] Workqueue: events_unbound intel_atomic_commit_work [i915]
> <4>[  227.152292] RIP: 0010:intel_unpin_fb_vma+0x23a/0x2a0 [i915]
> <4>[  227.152295] RSP: 0018:ffff88005aad7b68 EFLAGS: 00010286
> <4>[  227.152300] RAX: 0000000000000026 RBX: ffff88005c359580 RCX: 0000000000000000
> <4>[  227.152304] RDX: 0000000000000026 RSI: ffffffff8707d840 RDI: ffffed000b55af63
> <4>[  227.152307] RBP: ffff880056817e58 R08: 0000000000000001 R09: 0000000000000000
> <4>[  227.152311] R10: ffff88005aad7b88 R11: 0000000000000000 R12: ffff8800568184d0
> <4>[  227.152314] R13: ffff880065b5ab08 R14: 0000000000000000 R15: dffffc0000000000
> <4>[  227.152318] FS:  0000000000000000(0000) GS:ffff88006ac40000(0000) knlGS:0000000000000000
> <4>[  227.152322] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> <4>[  227.152325] CR2: 00007f5fb25550a8 CR3: 0000000068c78000 CR4: 00000000000006e0
> <4>[  227.152328] Call Trace:
> <4>[  227.152385]  intel_cleanup_plane_fb+0x6b/0xd0 [i915]
> <4>[  227.152395]  drm_atomic_helper_cleanup_planes+0x166/0x280
> <4>[  227.152452]  intel_atomic_commit_tail+0x159d/0x3380 [i915]
> <4>[  227.152463]  ? process_one_work+0x66e/0x1460
> <4>[  227.152516]  ? skl_update_crtcs+0x9c0/0x9c0 [i915]
> <4>[  227.152523]  ? lock_acquire+0x13d/0x390
> <4>[  227.152527]  ? lock_acquire+0x13d/0x390
> <4>[  227.152534]  process_one_work+0x71a/0x1460
> <4>[  227.152540]  ? __schedule+0x815/0x1e20
> <4>[  227.152547]  ? pwq_dec_nr_in_flight+0x2b0/0x2b0
> <4>[  227.152553]  ? _raw_spin_lock_irq+0xa/0x40
> <4>[  227.152559]  worker_thread+0xdf/0xf60
> <4>[  227.152569]  ? process_one_work+0x1460/0x1460
> <4>[  227.152573]  kthread+0x2cf/0x3c0
> <4>[  227.152578]  ? _kthread_create_on_node+0xa0/0xa0
> <4>[  227.152583]  ret_from_fork+0x3a/0x50
> <4>[  227.152591] Code: c6 00 11 86 c0 48 c7 c7 e0 bd 85 c0 e8 60 e7 a9 c4 0f ff e9 1f fe ff ff 48 c7 c6 40 10 86 c0 48 c7 c7 e0 ca 85 c0 e8 2b 95 bd c4 <0f> 0b 48 89 ef e8 4c 44 e8 c4 e9 ef fd ff ff e8 42 44 e8 c4 e9
> <1>[  227.152720] RIP: intel_unpin_fb_vma+0x23a/0x2a0 [i915] RSP: ffff88005aad7b68
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com
> ---
>  drivers/gpu/drm/i915/i915_drv.h           |  3 ++-
>  drivers/gpu/drm/i915/i915_gem.c           | 26 ++++++------------
>  drivers/gpu/drm/i915/intel_atomic_plane.c |  1 +
>  drivers/gpu/drm/i915/intel_display.c      | 45 ++++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_drv.h          |  9 +++++--
>  drivers/gpu/drm/i915/intel_fbdev.c        | 10 ++++---
>  drivers/gpu/drm/i915/intel_overlay.c      |  3 ++-
>  7 files changed, 62 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 76bfe909168c..20575b3ee406 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3413,7 +3413,8 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write);
>  struct i915_vma * __must_check
>  i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  				     u32 alignment,
> -				     const struct i915_ggtt_view *view);
> +				     const struct i915_ggtt_view *view,
> +				     unsigned int flags);
>  void i915_gem_object_unpin_from_display_plane(struct i915_vma *vma);
>  int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
>  				int align);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1854a69bc354..bdc2069d5433 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4078,7 +4078,8 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,
>  struct i915_vma *
>  i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  				     u32 alignment,
> -				     const struct i915_ggtt_view *view)
> +				     const struct i915_ggtt_view *view,
> +				     unsigned int flags)
>  {
>  	struct i915_vma *vma;
>  	int ret;
> @@ -4115,25 +4116,14 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  	 * try to preserve the existing ABI).
>  	 */
>  	vma = ERR_PTR(-ENOSPC);
> -	if (!view || view->type == I915_GGTT_VIEW_NORMAL)
> +	if ((flags & PIN_MAPPABLE) == 0 &&
> +	    (!view || view->type == I915_GGTT_VIEW_NORMAL))
>  		vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment,
> -					       PIN_MAPPABLE | PIN_NONBLOCK);
> -	if (IS_ERR(vma)) {
> -		struct drm_i915_private *i915 = to_i915(obj->base.dev);
> -		unsigned int flags;
> -
> -		/* Valleyview is definitely limited to scanning out the first
> -		 * 512MiB. Lets presume this behaviour was inherited from the
> -		 * g4x display engine and that all earlier gen are similarly
> -		 * limited. Testing suggests that it is a little more
> -		 * complicated than this. For example, Cherryview appears quite
> -		 * happy to scanout from anywhere within its global aperture.
> -		 */
> -		flags = 0;
> -		if (HAS_GMCH_DISPLAY(i915))
> -			flags = PIN_MAPPABLE;
> +					       flags |
> +					       PIN_MAPPABLE |
> +					       PIN_NONBLOCK);
> +	if (IS_ERR(vma))
>  		vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment, flags);
> -	}
>  	if (IS_ERR(vma))
>  		goto err_unpin_global;
>  
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 0ee32275994a..7481ce85746b 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -85,6 +85,7 @@ intel_plane_duplicate_state(struct drm_plane *plane)
>  	__drm_atomic_helper_plane_duplicate_state(plane, state);
>  
>  	intel_state->vma = NULL;
> +	intel_state->flags = 0;
>  
>  	return state;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 280c04326f64..8075e1990157 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2068,7 +2068,9 @@ static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
>  }
>  
>  struct i915_vma *
> -intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
> +intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
> +			   unsigned int rotation,
> +			   unsigned long *out_flags)
>  {
>  	struct drm_device *dev = fb->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -2076,6 +2078,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
>  	struct i915_ggtt_view view;
>  	struct i915_vma *vma;
>  	u32 alignment;
> +	unsigned int pinctl;
>  
>  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>  
> @@ -2102,7 +2105,20 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
>  
>  	atomic_inc(&dev_priv->gpu_error.pending_fb_pin);
>  
> -	vma = i915_gem_object_pin_to_display_plane(obj, alignment, &view);
> +	pinctl = 0;
> +
> +	/* Valleyview is definitely limited to scanning out the first
> +	 * 512MiB. Lets presume this behaviour was inherited from the
> +	 * g4x display engine and that all earlier gen are similarly
> +	 * limited. Testing suggests that it is a little more
> +	 * complicated than this. For example, Cherryview appears quite
> +	 * happy to scanout from anywhere within its global aperture.
> +	 */
> +	if (HAS_GMCH_DISPLAY(dev_priv))
> +		pinctl |= PIN_MAPPABLE;
> +
> +	vma = i915_gem_object_pin_to_display_plane(obj,
> +						   alignment, &view, pinctl);
>  	if (IS_ERR(vma))
>  		goto err;
>  
> @@ -2123,7 +2139,8 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
>  		 * something and try to run the system in a "less than optimal"
>  		 * mode that matches the user configuration.
>  		 */
> -		i915_vma_pin_fence(vma);
> +		if (i915_vma_pin_fence(vma) == 0)
> +			*out_flags |= PLANE_HAS_FENCE;
>  	}
>  
>  	i915_vma_get(vma);
> @@ -2134,11 +2151,12 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
>  	return vma;
>  }
>  
> -void intel_unpin_fb_vma(struct i915_vma *vma)
> +void intel_unpin_fb_vma(struct i915_vma *vma, unsigned long flags)
>  {
>  	lockdep_assert_held(&vma->vm->i915->drm.struct_mutex);
>  
> -	i915_vma_unpin_fence(vma);
> +	if (flags & PLANE_HAS_FENCE)
> +		i915_vma_unpin_fence(vma);
>  	i915_gem_object_unpin_from_display_plane(vma);
>  	i915_vma_put(vma);
>  }
> @@ -2808,7 +2826,9 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  valid_fb:
>  	mutex_lock(&dev->struct_mutex);
>  	intel_state->vma =
> -		intel_pin_and_fence_fb_obj(fb, primary->state->rotation);
> +		intel_pin_and_fence_fb_obj(fb,
> +					   primary->state->rotation,
> +					   &intel_state->flags);
>  	mutex_unlock(&dev->struct_mutex);
>  	if (IS_ERR(intel_state->vma)) {
>  		DRM_ERROR("failed to pin boot fb on pipe %d: %li\n",
> @@ -12713,7 +12733,9 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>  	} else {
>  		struct i915_vma *vma;
>  
> -		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
> +		vma = intel_pin_and_fence_fb_obj(fb,
> +						 new_state->rotation,
> +						 &to_intel_plane_state(new_state)->flags);
>  		if (!IS_ERR(vma))
>  			to_intel_plane_state(new_state)->vma = vma;
>  		else
> @@ -12768,7 +12790,7 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
>  	vma = fetch_and_zero(&to_intel_plane_state(old_state)->vma);
>  	if (vma) {
>  		mutex_lock(&plane->dev->struct_mutex);
> -		intel_unpin_fb_vma(vma);
> +		intel_unpin_fb_vma(vma, to_intel_plane_state(old_state)->flags);
>  		mutex_unlock(&plane->dev->struct_mutex);
>  	}
>  }
> @@ -13129,7 +13151,9 @@ intel_legacy_cursor_update(struct drm_plane *plane,
>  			goto out_unlock;
>  		}
>  	} else {
> -		vma = intel_pin_and_fence_fb_obj(fb, new_plane_state->rotation);
> +		vma = intel_pin_and_fence_fb_obj(fb,
> +						 new_plane_state->rotation,
> +						 &to_intel_plane_state(new_plane_state)->flags);
>  		if (IS_ERR(vma)) {
>  			DRM_DEBUG_KMS("failed to pin object\n");
>  
> @@ -13160,7 +13184,8 @@ intel_legacy_cursor_update(struct drm_plane *plane,
>  
>  	old_vma = fetch_and_zero(&to_intel_plane_state(old_plane_state)->vma);
>  	if (old_vma)
> -		intel_unpin_fb_vma(old_vma);
> +		intel_unpin_fb_vma(old_vma,
> +				   to_intel_plane_state(old_plane_state)->flags);
>  
>  out_unlock:
>  	mutex_unlock(&dev_priv->drm.struct_mutex);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 898064e8bea7..50874f4035cf 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -204,6 +204,7 @@ struct intel_fbdev {
>  	struct drm_fb_helper helper;
>  	struct intel_framebuffer *fb;
>  	struct i915_vma *vma;
> +	unsigned long vma_flags;
>  	async_cookie_t cookie;
>  	int preferred_bpp;
>  };
> @@ -490,6 +491,8 @@ struct intel_atomic_state {
>  struct intel_plane_state {
>  	struct drm_plane_state base;
>  	struct i915_vma *vma;
> +	unsigned long flags;

long seems potentially wasteful when we have just the one flag.
Although maybe the next thing gets aligned to 64bits anyway.

Anyways, patch looks reasonable.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

fbc should probably start looking at the new flag instead of
using the racy vma->fence checks it has now.

> +#define PLANE_HAS_FENCE BIT(0)
>  
>  	struct {
>  		u32 offset;
> @@ -1503,8 +1506,10 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
>  				    struct intel_load_detect_pipe *old,
>  				    struct drm_modeset_acquire_ctx *ctx);
>  struct i915_vma *
> -intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
> -void intel_unpin_fb_vma(struct i915_vma *vma);
> +intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
> +			   unsigned int rotation,
> +			   unsigned long *out_flags);
> +void intel_unpin_fb_vma(struct i915_vma *vma, unsigned long flags);
>  struct drm_framebuffer *
>  intel_framebuffer_create(struct drm_i915_gem_object *obj,
>  			 struct drm_mode_fb_cmd2 *mode_cmd);
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index da48af11eb6b..3d8ce3a62743 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -177,6 +177,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
>  	struct fb_info *info;
>  	struct drm_framebuffer *fb;
>  	struct i915_vma *vma;
> +	unsigned long flags = 0;
>  	bool prealloc = false;
>  	void __iomem *vaddr;
>  	int ret;
> @@ -211,7 +212,9 @@ static int intelfb_create(struct drm_fb_helper *helper,
>  	 * This also validates that any existing fb inherited from the
>  	 * BIOS is suitable for own access.
>  	 */
> -	vma = intel_pin_and_fence_fb_obj(&ifbdev->fb->base, DRM_MODE_ROTATE_0);
> +	vma = intel_pin_and_fence_fb_obj(&ifbdev->fb->base,
> +					 DRM_MODE_ROTATE_0,
> +					 &flags);
>  	if (IS_ERR(vma)) {
>  		ret = PTR_ERR(vma);
>  		goto out_unlock;
> @@ -268,6 +271,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
>  	DRM_DEBUG_KMS("allocated %dx%d fb: 0x%08x\n",
>  		      fb->width, fb->height, i915_ggtt_offset(vma));
>  	ifbdev->vma = vma;
> +	ifbdev->vma_flags = flags;
>  
>  	intel_runtime_pm_put(dev_priv);
>  	mutex_unlock(&dev->struct_mutex);
> @@ -275,7 +279,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
>  	return 0;
>  
>  out_unpin:
> -	intel_unpin_fb_vma(vma);
> +	intel_unpin_fb_vma(vma, flags);
>  out_unlock:
>  	intel_runtime_pm_put(dev_priv);
>  	mutex_unlock(&dev->struct_mutex);
> @@ -513,7 +517,7 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
>  
>  	if (ifbdev->vma) {
>  		mutex_lock(&ifbdev->helper.dev->struct_mutex);
> -		intel_unpin_fb_vma(ifbdev->vma);
> +		intel_unpin_fb_vma(ifbdev->vma, ifbdev->vma_flags);
>  		mutex_unlock(&ifbdev->helper.dev->struct_mutex);
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index 41e9465d44a8..89f568e739ee 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -801,7 +801,8 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
>  
>  	atomic_inc(&dev_priv->gpu_error.pending_fb_pin);
>  
> -	vma = i915_gem_object_pin_to_display_plane(new_bo, 0, NULL);
> +	vma = i915_gem_object_pin_to_display_plane(new_bo,
> +						   0, NULL, PIN_MAPPABLE);
>  	if (IS_ERR(vma)) {
>  		ret = PTR_ERR(vma);
>  		goto out_pin_section;
> -- 
> 2.16.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] drm/i915: Move the policy for placement of the GGTT vma into the caller
  2018-02-20 11:39   ` Ville Syrjälä
@ 2018-02-20 11:45     ` Chris Wilson
  0 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2018-02-20 11:45 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Ville

Quoting Ville Syrjälä (2018-02-20 11:39:13)
> On Tue, Feb 20, 2018 at 10:50:11AM +0000, Chris Wilson wrote:
> > @@ -490,6 +491,8 @@ struct intel_atomic_state {
> >  struct intel_plane_state {
> >       struct drm_plane_state base;
> >       struct i915_vma *vma;
> > +     unsigned long flags;
> 
> long seems potentially wasteful when we have just the one flag.
> Although maybe the next thing gets aligned to 64bits anyway.

Yeah, wasteful until you look at the holes. :) I tend to find I run out
of flag space all too quick so erred on the side of plenty of bits. Plus
it stops all the arguments about BIT() being UL but the type just U.

> Anyways, patch looks reasonable.
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> fbc should probably start looking at the new flag instead of
> using the racy vma->fence checks it has now.

Right, I was looking at how we could pass requirements like we want fbc
so try harder to give us a fence (with the usual cop-out of if we are
triple buffering 4k screens, we run out of mappable very quickly).
Hooking up to the plane->flags & HAS_FENCE is a good idea.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Also check view->type for a normal GGTT view
  2018-02-20 10:50 [PATCH 1/2] drm/i915: Also check view->type for a normal GGTT view Chris Wilson
  2018-02-20 10:50 ` [PATCH 2/2] drm/i915: Move the policy for placement of the GGTT vma into the caller Chris Wilson
@ 2018-02-20 11:47 ` Patchwork
  2018-02-20 11:53 ` [PATCH 1/2] " Ville Syrjälä
  2018-02-20 12:03 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
  3 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-02-20 11:47 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Also check view->type for a normal GGTT view
URL   : https://patchwork.freedesktop.org/series/38579/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
c5a4e635c47b drm/i915: Also check view->type for a normal GGTT view
8d590e83edd5 drm/i915: Move the policy for placement of the GGTT vma into the caller
-:28: WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#28: 
<4>[  227.152102] Modules linked in: i915 snd_hda_codec_analog snd_hda_codec_generic coretemp snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm lpc_ich e1000e mei_me mei prime_numbers

-:64: ERROR: Unrecognized email address: 'Ville Syrjälä <ville.syrjala@linux.intel.com'
#64: 
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com

-:227: WARNING: line over 80 characters
#227: FILE: drivers/gpu/drm/i915/intel_display.c:12738:
+						 &to_intel_plane_state(new_state)->flags);

-:247: WARNING: line over 80 characters
#247: FILE: drivers/gpu/drm/i915/intel_display.c:13156:
+						 &to_intel_plane_state(new_plane_state)->flags);

-:257: WARNING: line over 80 characters
#257: FILE: drivers/gpu/drm/i915/intel_display.c:13188:
+				   to_intel_plane_state(old_plane_state)->flags);

total: 1 errors, 4 warnings, 0 checks, 240 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] drm/i915: Also check view->type for a normal GGTT view
  2018-02-20 10:50 [PATCH 1/2] drm/i915: Also check view->type for a normal GGTT view Chris Wilson
  2018-02-20 10:50 ` [PATCH 2/2] drm/i915: Move the policy for placement of the GGTT vma into the caller Chris Wilson
  2018-02-20 11:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Also check view->type for a normal GGTT view Patchwork
@ 2018-02-20 11:53 ` Ville Syrjälä
  2018-02-20 12:03 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
  3 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2018-02-20 11:53 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Tue, Feb 20, 2018 at 10:50:10AM +0000, Chris Wilson wrote:
> We cannot simply use !view as shorthand for all normal GGTT views as a
> few callers will always populate a i915_ggtt_view struct and set the
> type to NORMAL instead. So check for (!view || view->type == NORMAL)
> inside i915_gem_object_ggtt_pin().
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 631a2db2bb6e..1854a69bc354 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4282,7 +4282,8 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
>  
>  	lockdep_assert_held(&obj->base.dev->struct_mutex);
>  
> -	if (!view && flags & PIN_MAPPABLE) {
> +	if ((!view || view->type == I915_GGTT_VIEW_NORMAL) &&
> +	    flags & PIN_MAPPABLE) {

So looks like this is just an optimization to quickly reject clearly
impossible things. So I assume the only proble was the optimization
not working in all cases.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  		/* If the required space is larger than the available
>  		 * aperture, we will not able to find a slot for the
>  		 * object and unbinding the object now will be in
> -- 
> 2.16.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Also check view->type for a normal GGTT view
  2018-02-20 10:50 [PATCH 1/2] drm/i915: Also check view->type for a normal GGTT view Chris Wilson
                   ` (2 preceding siblings ...)
  2018-02-20 11:53 ` [PATCH 1/2] " Ville Syrjälä
@ 2018-02-20 12:03 ` Patchwork
  2018-02-20 12:08   ` Chris Wilson
  3 siblings, 1 reply; 8+ messages in thread
From: Patchwork @ 2018-02-20 12:03 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Also check view->type for a normal GGTT view
URL   : https://patchwork.freedesktop.org/series/38579/
State : warning

== Summary ==

Series 38579v1 series starting with [1/2] drm/i915: Also check view->type for a normal GGTT view
https://patchwork.freedesktop.org/api/1.0/series/38579/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                pass       -> DMESG-WARN (fi-cfl-s2)
Test pm_backlight:
        Subgroup basic-brightness:
                pass       -> DMESG-WARN (fi-cfl-s2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (fi-cfl-s2)
        Subgroup basic-rte:
                pass       -> DMESG-WARN (fi-cfl-s2)
Test prime_vgem:
        Subgroup basic-fence-flip:
                pass       -> DMESG-WARN (fi-cfl-s2)
Test drv_module_reload:
        Subgroup basic-reload:
                pass       -> DMESG-WARN (fi-cfl-s2)
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (fi-cfl-s2)
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (fi-cfl-s2)

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:420s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:428s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:377s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:482s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:284s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:481s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:482s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:464s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:452s
fi-cfl-s2        total:288  pass:254  dwarn:8   dfail:0   fail:0   skip:26  time:574s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:409s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:282s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:511s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:385s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:407s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:449s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:414s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:450s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:489s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:447s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:494s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:583s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:426s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:499s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:513s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:491s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:487s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:409s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:427s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:523s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:393s

2c22e35947dba3af04655046412457c83401aa63 drm-tip: 2018y-02m-20d-11h-11m-23s UTC integration manifest
8d590e83edd5 drm/i915: Move the policy for placement of the GGTT vma into the caller
c5a4e635c47b drm/i915: Also check view->type for a normal GGTT view

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8078/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Also check view->type for a normal GGTT view
  2018-02-20 12:03 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
@ 2018-02-20 12:08   ` Chris Wilson
  0 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2018-02-20 12:08 UTC (permalink / raw)
  To: Patchwork; +Cc: intel-gfx

Quoting Patchwork (2018-02-20 12:03:36)
> == Series Details ==
> 
> Series: series starting with [1/2] drm/i915: Also check view->type for a normal GGTT view
> URL   : https://patchwork.freedesktop.org/series/38579/
> State : warning
> 
> == Summary ==
> 
> Series 38579v1 series starting with [1/2] drm/i915: Also check view->type for a normal GGTT view
> https://patchwork.freedesktop.org/api/1.0/series/38579/revisions/1/mbox/
> 
> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-b:
>                 incomplete -> PASS       (fi-snb-2520m) fdo#103713
> Test kms_setmode:
>         Subgroup basic-clone-single-crtc:
>                 pass       -> DMESG-WARN (fi-cfl-s2)
> Test pm_backlight:
>         Subgroup basic-brightness:
>                 pass       -> DMESG-WARN (fi-cfl-s2)
> Test pm_rpm:
>         Subgroup basic-pci-d3-state:
>                 pass       -> DMESG-WARN (fi-cfl-s2)
>         Subgroup basic-rte:
>                 pass       -> DMESG-WARN (fi-cfl-s2)
> Test prime_vgem:
>         Subgroup basic-fence-flip:
>                 pass       -> DMESG-WARN (fi-cfl-s2)
> Test drv_module_reload:
>         Subgroup basic-reload:
>                 pass       -> DMESG-WARN (fi-cfl-s2)
>         Subgroup basic-no-display:
>                 pass       -> DMESG-WARN (fi-cfl-s2)
>         Subgroup basic-reload-inject:
>                 pass       -> DMESG-WARN (fi-cfl-s2)

Looks to be only LPSCON woes.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-02-20 12:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-20 10:50 [PATCH 1/2] drm/i915: Also check view->type for a normal GGTT view Chris Wilson
2018-02-20 10:50 ` [PATCH 2/2] drm/i915: Move the policy for placement of the GGTT vma into the caller Chris Wilson
2018-02-20 11:39   ` Ville Syrjälä
2018-02-20 11:45     ` Chris Wilson
2018-02-20 11:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Also check view->type for a normal GGTT view Patchwork
2018-02-20 11:53 ` [PATCH 1/2] " Ville Syrjälä
2018-02-20 12:03 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
2018-02-20 12:08   ` Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox