[PATCH 0/5] Assorted reviewed patches for CI run

[PATCH 0/5] Assorted reviewed patches for CI run

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

As the subject says, just to trigger a CI run for some assorted
patches which are all already R-Bed.

Tvrtko Ursulin (5):
  drm/i915: Use appropriate spinlock flavour
  drm/i915: GEM operations need to be done under the big lock
  drm/i915: Fix struct mutex vs. RPS lock inversion
  drm/i915/guc: Do not wait for firmware load atomically
  drm/i915/ilk: Move register read under spinlock

 drivers/gpu/drm/i915/i915_gem.c         |  6 ++----
 drivers/gpu/drm/i915/i915_gem_stolen.c  |  2 ++
 drivers/gpu/drm/i915/intel_display.c    | 12 ++++++------
 drivers/gpu/drm/i915/intel_guc_loader.c |  6 +++---
 drivers/gpu/drm/i915/intel_pm.c         | 15 ++++++++-------
 5 files changed, 21 insertions(+), 20 deletions(-)

-- 
1.9.1

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

[PATCH 1/5] drm/i915: Use appropriate spinlock flavour

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

We know this never runs from interrupt context so
don't need to use the flags variant.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index de57e7f0be0f..ef53bb314639 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2970,11 +2970,9 @@ i915_gem_retire_requests(struct drm_device *dev)
 		i915_gem_retire_requests_ring(ring);
 		idle &= list_empty(&ring->request_list);
 		if (i915.enable_execlists) {
-			unsigned long flags;
-
-			spin_lock_irqsave(&ring->execlist_lock, flags);
+			spin_lock_irq(&ring->execlist_lock);
 			idle &= list_empty(&ring->execlist_queue);
-			spin_unlock_irqrestore(&ring->execlist_lock, flags);
+			spin_unlock_irq(&ring->execlist_lock);
 
 			intel_execlists_retire_requests(ring);
 		}
-- 
1.9.1

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

[PATCH 2/5] drm/i915: GEM operations need to be done under the big lock

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

VMA creation and GEM list management need the big lock.

v2:

Mutex unlock ended on the wrong path somehow. (0-day, Julia Lawall)

Not to mention drm_gem_object_unreference was there in existing
code with no mutex held.

v3:

Some callers of i915_gem_object_create_stolen_for_preallocated
already hold the lock so move the mutex into the other caller
as well.

v4:

Changed to lockdep_assert_held. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 2 ++
 drivers/gpu/drm/i915/intel_display.c   | 8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index ba1a00d815d3..feec0f80d8ef 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -638,6 +638,8 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
 	if (!drm_mm_initialized(&dev_priv->mm.stolen))
 		return NULL;
 
+	lockdep_assert_held(&dev->struct_mutex);
+
 	DRM_DEBUG_KMS("creating preallocated stolen object: stolen_offset=%x, gtt_offset=%x, size=%x\n",
 			stolen_offset, gtt_offset, size);
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 836bbdc239b6..57e465ed088b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2551,12 +2551,16 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
 	if (size_aligned * 2 > dev_priv->gtt.stolen_usable_size)
 		return false;
 
+	mutex_lock(&dev->struct_mutex);
+
 	obj = i915_gem_object_create_stolen_for_preallocated(dev,
 							     base_aligned,
 							     base_aligned,
 							     size_aligned);
-	if (!obj)
+	if (!obj) {
+		mutex_unlock(&dev->struct_mutex);
 		return false;
+	}
 
 	obj->tiling_mode = plane_config->tiling;
 	if (obj->tiling_mode == I915_TILING_X)
@@ -2569,12 +2573,12 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
 	mode_cmd.modifier[0] = fb->modifier[0];
 	mode_cmd.flags = DRM_MODE_FB_MODIFIERS;
 
-	mutex_lock(&dev->struct_mutex);
 	if (intel_framebuffer_init(dev, to_intel_framebuffer(fb),
 				   &mode_cmd, obj)) {
 		DRM_DEBUG_KMS("intel fb init failed\n");
 		goto out_unref_obj;
 	}
+
 	mutex_unlock(&dev->struct_mutex);
 
 	DRM_DEBUG_KMS("initial plane fb obj %p\n", obj);
-- 
1.9.1

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

[PATCH 3/5] drm/i915: Fix struct mutex vs. RPS lock inversion

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

RPS lock must be taken before the struct_mutex to avoid
locking inversion. So stop grabbing it for the whole
powersave initialization and instead only take it during
the sections which need it.

Also, struct_mutex is not needed any more since dedicated
RPS lock was added in:

   commit 4fc688ce79772496503d22263d61b071a8fb596e
   Author: Jesse Barnes <jbarnes@virtuousgeek.org>
   Date:   Fri Nov 2 11:14:01 2012 -0700

       drm/i915: protect RPS/RC6 related accesses (including PCU) with a new mutex

Based on prototype patch by Chris Wilson and a subsequent
mailing list discussion involving Ville, Imre, Chris and
Daniel.

v2: More details in the commit.

v3: Use drm_gem_object_unreference_unlocked. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ----
 drivers/gpu/drm/i915/intel_pm.c      | 9 ++++-----
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 57e465ed088b..a0f3e399a2d9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15976,9 +15976,7 @@ void intel_modeset_gem_init(struct drm_device *dev)
 	struct drm_i915_gem_object *obj;
 	int ret;
 
-	mutex_lock(&dev->struct_mutex);
 	intel_init_gt_powersave(dev);
-	mutex_unlock(&dev->struct_mutex);
 
 	intel_modeset_init_hw(dev);
 
@@ -16058,9 +16056,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	intel_cleanup_overlay(dev);
 
-	mutex_lock(&dev->struct_mutex);
 	intel_cleanup_gt_powersave(dev);
-	mutex_unlock(&dev->struct_mutex);
 
 	intel_teardown_gmbus(dev);
 }
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 379eabe093cb..8e869f183404 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5229,8 +5229,6 @@ static void cherryview_setup_pctx(struct drm_device *dev)
 	u32 pcbr;
 	int pctx_size = 32*1024;
 
-	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
-
 	pcbr = I915_READ(VLV_PCBR);
 	if ((pcbr >> VLV_PCBR_ADDR_SHIFT) == 0) {
 		DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
@@ -5252,7 +5250,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
 	u32 pcbr;
 	int pctx_size = 24*1024;
 
-	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+	mutex_lock(&dev->struct_mutex);
 
 	pcbr = I915_READ(VLV_PCBR);
 	if (pcbr) {
@@ -5280,7 +5278,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
 	pctx = i915_gem_object_create_stolen(dev, pctx_size);
 	if (!pctx) {
 		DRM_DEBUG("not enough stolen space for PCTX, disabling\n");
-		return;
+		goto out;
 	}
 
 	pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start;
@@ -5289,6 +5287,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
 out:
 	DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
 	dev_priv->vlv_pctx = pctx;
+	mutex_unlock(&dev->struct_mutex);
 }
 
 static void valleyview_cleanup_pctx(struct drm_device *dev)
@@ -5298,7 +5297,7 @@ static void valleyview_cleanup_pctx(struct drm_device *dev)
 	if (WARN_ON(!dev_priv->vlv_pctx))
 		return;
 
-	drm_gem_object_unreference(&dev_priv->vlv_pctx->base);
+	drm_gem_object_unreference_unlocked(&dev_priv->vlv_pctx->base);
 	dev_priv->vlv_pctx = NULL;
 }
 
-- 
1.9.1

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

[PATCH 4/5] drm/i915/guc: Do not wait for firmware load atomically

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

It does not look like this code needs to wait atomically?

Higher in the call chain it calls the GEM API and I do
not see that the section is under any spin locks or such.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Alex Dai <yu.dai@intel.com>
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 3accd914490f..82a3c03fbc0e 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -199,7 +199,7 @@ static void set_guc_init_params(struct drm_i915_private *dev_priv)
  * the value matches either of two values representing completion
  * of the GuC boot process.
  *
- * This is used for polling the GuC status in a wait_for_atomic()
+ * This is used for polling the GuC status in a wait_for()
  * loop below.
  */
 static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
@@ -259,14 +259,14 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv)
 	I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
 
 	/*
-	 * Spin-wait for the DMA to complete & the GuC to start up.
+	 * Wait for the DMA to complete & the GuC to start up.
 	 * NB: Docs recommend not using the interrupt for completion.
 	 * Measurements indicate this should take no more than 20ms, so a
 	 * timeout here indicates that the GuC has failed and is unusable.
 	 * (Higher levels of the driver will attempt to fall back to
 	 * execlist mode if this happens.)
 	 */
-	ret = wait_for_atomic(guc_ucode_response(dev_priv, &status), 100);
+	ret = wait_for(guc_ucode_response(dev_priv, &status), 100);
 
 	DRM_DEBUG_DRIVER("DMA status 0x%x, GuC status 0x%x\n",
 			I915_READ(DMA_CTRL), status);
-- 
1.9.1

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

[PATCH 5/5] drm/i915/ilk: Move register read under spinlock

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Code does read-modify-write but the read was outside the lock.

It is fine since the caller holds struct mutex, but if we
correct this we open up the opportunity for decreasing the
mutex duration time since the call to ironlake_enable_drps
does not need it any longer since it is covered by the
mchdev_lock lock.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_pm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8e869f183404..b63cdb23b222 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4116,11 +4116,13 @@ bool ironlake_set_drps(struct drm_device *dev, u8 val)
 static void ironlake_enable_drps(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 rgvmodectl = I915_READ(MEMMODECTL);
+	u32 rgvmodectl;
 	u8 fmax, fmin, fstart, vstart;
 
 	spin_lock_irq(&mchdev_lock);
 
+	rgvmodectl = I915_READ(MEMMODECTL);
+
 	/* Enable temp reporting */
 	I915_WRITE16(PMMISC, I915_READ(PMMISC) | MCPPCE_EN);
 	I915_WRITE16(TSC1, I915_READ(TSC1) | TSE);
@@ -6240,8 +6242,8 @@ void intel_enable_gt_powersave(struct drm_device *dev)
 		return;
 
 	if (IS_IRONLAKE_M(dev)) {
-		mutex_lock(&dev->struct_mutex);
 		ironlake_enable_drps(dev);
+		mutex_lock(&dev->struct_mutex);
 		intel_init_emon(dev);
 		mutex_unlock(&dev->struct_mutex);
 	} else if (INTEL_INFO(dev)->gen >= 6) {
-- 
1.9.1

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

✗ Fi.CI.BAT: warning for Assorted reviewed patches for CI run

[Patchwork]

== Summary ==

Series 3277v1 Assorted reviewed patches for CI run
http://patchwork.freedesktop.org/api/1.0/series/3277/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (skl-i5k-2)
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (skl-i5k-2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                fail       -> PASS       (bdw-nuci7)
        Subgroup basic-rte:
                pass       -> DMESG-WARN (byt-nuc) UNSTABLE

bdw-nuci7        total:162  pass:152  dwarn:0   dfail:0   fail:0   skip:10 
bdw-ultra        total:165  pass:152  dwarn:0   dfail:0   fail:0   skip:13 
bsw-nuc-2        total:165  pass:134  dwarn:2   dfail:0   fail:0   skip:29 
byt-nuc          total:165  pass:140  dwarn:1   dfail:0   fail:0   skip:24 
hsw-brixbox      total:165  pass:151  dwarn:0   dfail:0   fail:0   skip:14 
hsw-gt2          total:165  pass:154  dwarn:0   dfail:0   fail:1   skip:10 
ilk-hp8440p      total:165  pass:116  dwarn:0   dfail:0   fail:1   skip:48 
ivb-t430s        total:165  pass:150  dwarn:0   dfail:0   fail:1   skip:14 
skl-i5k-2        total:165  pass:149  dwarn:1   dfail:0   fail:0   skip:15 
snb-dellxps      total:165  pass:142  dwarn:0   dfail:0   fail:1   skip:22 
snb-x220t        total:165  pass:142  dwarn:0   dfail:0   fail:2   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1394/

f2110d8eac120416f8f5669f2aa561d9ab330a77 drm-intel-nightly: 2016y-02m-15d-09h-53m-04s UTC integration manifest
b569392e655ae61e36db9c2855e3c5444e80f681 drm/i915/ilk: Move register read under spinlock
5ebea17ad6e3ead0efef6ad4e6dcdddc16209c9d drm/i915/guc: Do not wait for firmware load atomically
8bfc1509b39863c3cbb5755d065e241d27d96c9d drm/i915: Fix struct mutex vs. RPS lock inversion
9daa18b7c90411745d5e3783a595080fed6d7ef7 drm/i915: GEM operations need to be done under the big lock
7725bdeaf3de05cb3db4cd04198aaabb594054ca drm/i915: Use appropriate spinlock flavour

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

Re: ✗ Fi.CI.BAT: warning for Assorted reviewed patches for CI run

[Tvrtko Ursulin]

On 15/02/16 14:53, Patchwork wrote:
> == Summary ==
>
> Series 3277v1 Assorted reviewed patches for CI run
> http://patchwork.freedesktop.org/api/1.0/series/3277/revisions/1/mbox/
>
> Test kms_pipe_crc_basic:
>          Subgroup suspend-read-crc-pipe-a:
>                  pass       -> DMESG-WARN (skl-i5k-2)

Known unrelated: https://bugs.freedesktop.org/show_bug.cgi?id=93294

>          Subgroup suspend-read-crc-pipe-b:
>                  dmesg-warn -> PASS       (skl-i5k-2)
> Test pm_rpm:
>          Subgroup basic-pci-d3-state:
>                  fail       -> PASS       (bdw-nuci7)
>          Subgroup basic-rte:
>                  pass       -> DMESG-WARN (byt-nuc) UNSTABLE

Known unrelated: https://bugs.freedesktop.org/show_bug.cgi?id=93121

>
> bdw-nuci7        total:162  pass:152  dwarn:0   dfail:0   fail:0   skip:10
> bdw-ultra        total:165  pass:152  dwarn:0   dfail:0   fail:0   skip:13
> bsw-nuc-2        total:165  pass:134  dwarn:2   dfail:0   fail:0   skip:29
> byt-nuc          total:165  pass:140  dwarn:1   dfail:0   fail:0   skip:24
> hsw-brixbox      total:165  pass:151  dwarn:0   dfail:0   fail:0   skip:14
> hsw-gt2          total:165  pass:154  dwarn:0   dfail:0   fail:1   skip:10
> ilk-hp8440p      total:165  pass:116  dwarn:0   dfail:0   fail:1   skip:48
> ivb-t430s        total:165  pass:150  dwarn:0   dfail:0   fail:1   skip:14
> skl-i5k-2        total:165  pass:149  dwarn:1   dfail:0   fail:0   skip:15
> snb-dellxps      total:165  pass:142  dwarn:0   dfail:0   fail:1   skip:22
> snb-x220t        total:165  pass:142  dwarn:0   dfail:0   fail:2   skip:21
>
> Results at /archive/results/CI_IGT_test/Patchwork_1394/
>
> f2110d8eac120416f8f5669f2aa561d9ab330a77 drm-intel-nightly: 2016y-02m-15d-09h-53m-04s UTC integration manifest
> b569392e655ae61e36db9c2855e3c5444e80f681 drm/i915/ilk: Move register read under spinlock
> 5ebea17ad6e3ead0efef6ad4e6dcdddc16209c9d drm/i915/guc: Do not wait for firmware load atomically
> 8bfc1509b39863c3cbb5755d065e241d27d96c9d drm/i915: Fix struct mutex vs. RPS lock inversion
> 9daa18b7c90411745d5e3783a595080fed6d7ef7 drm/i915: GEM operations need to be done under the big lock
> 7725bdeaf3de05cb3db4cd04198aaabb594054ca drm/i915: Use appropriate spinlock flavour

So will merge this bunch.

Regards,

Tvrtko

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