[PATCH 00/15] acquire ctx wire-up, part 2

[PATCH 00/15] acquire ctx wire-up, part 2

[Daniel Vetter]

Hi all,

Partially this is a resend of the patches now unblocked by the vmwgfx atomic
conversion just merged. I could entirely drop the vmwgfx patch since it's all
fixed now.

Then a bit of follow-up, plus converting the gamma_set/get ioctls. fbdev
emulation and the property paths are still infested by drm_modeset_lock_all, but
I think at least for fbdev we now have a semi-clear path with Thierry's series.
Properties are still unclear to me, because it's a rather layered maze with lots
of different callsites.

As always, comments and review highly welcome.

Cheers, Daniel

Daniel Vetter (15):
  drm: Make drm_modeset_lock_crtc internal
  drm: Remove drm_modeset_(un)lock_crtc
  drm: Remove drm_modeset_legacy_acquire_ctx and crtc->acquire_ctx
  drm/atomic-helper: remove modeset_lock_all from helper_resume
  drm: drop modeset_lock_all from drm_state_info
  drm: Drop modeset_lock_all from the getproperty ioctl
  drm: Only take crtc lock in get_gamma ioctl
  drm/i915: Nuke intel_atomic_legacy_gamma_set
  drm/msm: Nerf zpos property
  drm/fb-helper: Give up on kgdb for atomic drivers
  drm: Add explicit acquire ctx handling around ->gamma_set
  drm: Add acquire ctx to ->gamma_set hook
  drm/atomic-helper: Remove legacy backoff hack from gamma_set
  drm: extract legacy framebuffer remove
  drm/fb-helper: Extract _legacy kms functions

 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |   3 +-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |   3 +-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |   3 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |   3 +-
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c  |   3 +-
 drivers/gpu/drm/ast/ast_mode.c            |   3 +-
 drivers/gpu/drm/cirrus/cirrus_mode.c      |   3 +-
 drivers/gpu/drm/drm_atomic.c              | 162 +++++++-----------------------
 drivers/gpu/drm/drm_atomic_helper.c       |  35 +++----
 drivers/gpu/drm/drm_color_mgmt.c          |  51 +++++-----
 drivers/gpu/drm/drm_crtc_internal.h       |   1 -
 drivers/gpu/drm/drm_fb_helper.c           |  81 +++++++++------
 drivers/gpu/drm/drm_framebuffer.c         | 137 +++++++++++++++++++++----
 drivers/gpu/drm/drm_modeset_lock.c        | 102 -------------------
 drivers/gpu/drm/drm_plane.c               |  49 +++++----
 drivers/gpu/drm/drm_property.c            |  72 ++++++-------
 drivers/gpu/drm/gma500/gma_display.c      |   3 +-
 drivers/gpu/drm/gma500/gma_display.h      |   3 +-
 drivers/gpu/drm/i915/intel_display.c      |  60 +----------
 drivers/gpu/drm/i915/intel_pipe_crc.c     |   2 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c    |   3 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c |   2 -
 drivers/gpu/drm/nouveau/dispnv04/crtc.c   |   3 +-
 drivers/gpu/drm/nouveau/nv50_display.c    |  13 +--
 drivers/gpu/drm/radeon/radeon_display.c   |   3 +-
 drivers/gpu/drm/vc4/vc4_crtc.c            |   3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c       |   3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h       |   3 +-
 include/drm/drm_atomic_helper.h           |   3 +-
 include/drm/drm_crtc.h                    |  12 +--
 include/drm/drm_modeset_lock.h            |   5 -
 31 files changed, 344 insertions(+), 488 deletions(-)

-- 
2.11.0

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

[PATCH 01/15] drm: Make drm_modeset_lock_crtc internal

[Daniel Vetter]

This is only for legacy paths that need to grab the crtc/plane lock
combo. If you want to lock a crtc, just use drm_modeset_lock().

Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_crtc_internal.h |  3 +++
 drivers/gpu/drm/drm_modeset_lock.c  | 14 --------------
 include/drm/drm_modeset_lock.h      |  2 --
 3 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 8c04275cf226..de1047530e07 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -61,6 +61,9 @@ int drm_mode_getresources(struct drm_device *dev,
 			  void *data, struct drm_file *file_priv);
 
 
+/* drm_modeset_lock.c */
+void drm_modeset_lock_crtc(struct drm_crtc *crtc,
+			   struct drm_plane *plane);
 /* drm_dumb_buffers.c */
 /* IOCTLs */
 int drm_mode_create_dumb_ioctl(struct drm_device *dev,
diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index bf60f2645e55..c94eff9d7544 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -148,19 +148,6 @@ void drm_modeset_unlock_all(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_modeset_unlock_all);
 
-/**
- * drm_modeset_lock_crtc - lock crtc with hidden acquire ctx for a plane update
- * @crtc: DRM CRTC
- * @plane: DRM plane to be updated on @crtc
- *
- * This function locks the given crtc and plane (which should be either the
- * primary or cursor plane) using a hidden acquire context. This is necessary so
- * that drivers internally using the atomic interfaces can grab further locks
- * with the lock acquire context.
- *
- * Note that @plane can be NULL, e.g. when the cursor support hasn't yet been
- * converted to universal planes yet.
- */
 void drm_modeset_lock_crtc(struct drm_crtc *crtc,
 			   struct drm_plane *plane)
 {
@@ -205,7 +192,6 @@ void drm_modeset_lock_crtc(struct drm_crtc *crtc,
 		goto retry;
 	}
 }
-EXPORT_SYMBOL(drm_modeset_lock_crtc);
 
 /**
  * drm_modeset_legacy_acquire_ctx - find acquire ctx for legacy ioctls
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index 96d39fbd12ca..88d35bfc9cd8 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -121,8 +121,6 @@ struct drm_plane;
 
 void drm_modeset_lock_all(struct drm_device *dev);
 void drm_modeset_unlock_all(struct drm_device *dev);
-void drm_modeset_lock_crtc(struct drm_crtc *crtc,
-			   struct drm_plane *plane);
 void drm_modeset_unlock_crtc(struct drm_crtc *crtc);
 void drm_warn_on_modeset_not_all_locked(struct drm_device *dev);
 struct drm_modeset_acquire_ctx *
-- 
2.11.0

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

Re: [PATCH 01/15] drm: Make drm_modeset_lock_crtc internal

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2054 bytes --]

Hi Daniel,

[auto build test ERROR on next-20170330]
[also build test ERROR on v4.11-rc5]
[cannot apply to drm/drm-next drm-intel/for-linux-next robclark/msm-next v4.9-rc8 v4.9-rc7 v4.9-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Daniel-Vetter/acquire-ctx-wire-up-part-2/20170404-053514
config: i386-randconfig-x010-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/vmwgfx/vmwgfx_kms.c: In function 'vmw_du_crtc_cursor_set2':
>> drivers/gpu//drm/vmwgfx/vmwgfx_kms.c:228:2: error: implicit declaration of function 'drm_modeset_lock_crtc' [-Werror=implicit-function-declaration]
     drm_modeset_lock_crtc(crtc, crtc->cursor);
     ^~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/drm_modeset_lock_crtc +228 drivers/gpu//drm/vmwgfx/vmwgfx_kms.c

8fbf9d92 Thomas Hellstrom  2015-11-26  222  		du->core_hotspot_x = hot_x;
8fbf9d92 Thomas Hellstrom  2015-11-26  223  		du->core_hotspot_y = hot_y;
8fbf9d92 Thomas Hellstrom  2015-11-26  224  	}
fb1d9738 Jakob Bornecrantz 2009-12-10  225  
bfb89928 Daniel Vetter     2012-12-02  226  out:
bfb89928 Daniel Vetter     2012-12-02  227  	drm_modeset_unlock_all(dev_priv->dev);
4d02e2de Daniel Vetter     2014-11-11 @228  	drm_modeset_lock_crtc(crtc, crtc->cursor);
bfb89928 Daniel Vetter     2012-12-02  229  
bfb89928 Daniel Vetter     2012-12-02  230  	return ret;
fb1d9738 Jakob Bornecrantz 2009-12-10  231  }

:::::: The code at line 228 was first introduced by commit
:::::: 4d02e2de0e80a786452e70d7f3a20a50641e6620 drm: Per-plane locking

:::::: TO: Daniel Vetter <daniel.vetter@ffwll.ch>
:::::: CC: Dave Airlie <airlied@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27264 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

[PATCH 02/15] drm: Remove drm_modeset_(un)lock_crtc

[Daniel Vetter]

The last user, the cursor ioctl, can just open-code this too. We
simply have to move the acquire ctx dance from the universal function
up into the top-level ioctl handler.

Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_crtc_internal.h |  3 --
 drivers/gpu/drm/drm_modeset_lock.c  | 67 -------------------------------------
 drivers/gpu/drm/drm_plane.c         | 49 +++++++++++++--------------
 include/drm/drm_modeset_lock.h      |  1 -
 4 files changed, 24 insertions(+), 96 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index de1047530e07..8c04275cf226 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -61,9 +61,6 @@ int drm_mode_getresources(struct drm_device *dev,
 			  void *data, struct drm_file *file_priv);
 
 
-/* drm_modeset_lock.c */
-void drm_modeset_lock_crtc(struct drm_crtc *crtc,
-			   struct drm_plane *plane);
 /* drm_dumb_buffers.c */
 /* IOCTLs */
 int drm_mode_create_dumb_ioctl(struct drm_device *dev,
diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index c94eff9d7544..c3ca6b859236 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -148,51 +148,6 @@ void drm_modeset_unlock_all(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_modeset_unlock_all);
 
-void drm_modeset_lock_crtc(struct drm_crtc *crtc,
-			   struct drm_plane *plane)
-{
-	struct drm_modeset_acquire_ctx *ctx;
-	int ret;
-
-	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
-	if (WARN_ON(!ctx))
-		return;
-
-	drm_modeset_acquire_init(ctx, 0);
-
-retry:
-	ret = drm_modeset_lock(&crtc->mutex, ctx);
-	if (ret)
-		goto fail;
-
-	if (plane) {
-		ret = drm_modeset_lock(&plane->mutex, ctx);
-		if (ret)
-			goto fail;
-
-		if (plane->crtc) {
-			ret = drm_modeset_lock(&plane->crtc->mutex, ctx);
-			if (ret)
-				goto fail;
-		}
-	}
-
-	WARN_ON(crtc->acquire_ctx);
-
-	/* now we hold the locks, so now that it is safe, stash the
-	 * ctx for drm_modeset_unlock_crtc():
-	 */
-	crtc->acquire_ctx = ctx;
-
-	return;
-
-fail:
-	if (ret == -EDEADLK) {
-		drm_modeset_backoff(ctx);
-		goto retry;
-	}
-}
-
 /**
  * drm_modeset_legacy_acquire_ctx - find acquire ctx for legacy ioctls
  * @crtc: drm crtc
@@ -215,28 +170,6 @@ drm_modeset_legacy_acquire_ctx(struct drm_crtc *crtc)
 EXPORT_SYMBOL(drm_modeset_legacy_acquire_ctx);
 
 /**
- * drm_modeset_unlock_crtc - drop crtc lock
- * @crtc: drm crtc
- *
- * This drops the crtc lock acquire with drm_modeset_lock_crtc() and all other
- * locks acquired through the hidden context.
- */
-void drm_modeset_unlock_crtc(struct drm_crtc *crtc)
-{
-	struct drm_modeset_acquire_ctx *ctx = crtc->acquire_ctx;
-
-	if (WARN_ON(!ctx))
-		return;
-
-	crtc->acquire_ctx = NULL;
-	drm_modeset_drop_locks(ctx);
-	drm_modeset_acquire_fini(ctx);
-
-	kfree(ctx);
-}
-EXPORT_SYMBOL(drm_modeset_unlock_crtc);
-
-/**
  * drm_warn_on_modeset_not_all_locked - check that all modeset locks are locked
  * @dev: device
  *
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index bc71aa2b7872..838ca742a28b 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -620,7 +620,8 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
 
 static int drm_mode_cursor_universal(struct drm_crtc *crtc,
 				     struct drm_mode_cursor2 *req,
-				     struct drm_file *file_priv)
+				     struct drm_file *file_priv,
+				     struct drm_modeset_acquire_ctx *ctx)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_framebuffer *fb = NULL;
@@ -634,21 +635,11 @@ static int drm_mode_cursor_universal(struct drm_crtc *crtc,
 	int32_t crtc_x, crtc_y;
 	uint32_t crtc_w = 0, crtc_h = 0;
 	uint32_t src_w = 0, src_h = 0;
-	struct drm_modeset_acquire_ctx ctx;
 	int ret = 0;
 
 	BUG_ON(!crtc->cursor);
 	WARN_ON(crtc->cursor->crtc != crtc && crtc->cursor->crtc != NULL);
 
-	drm_modeset_acquire_init(&ctx, 0);
-retry:
-	ret = drm_modeset_lock(&crtc->mutex, &ctx);
-	if (ret)
-		goto fail;
-	ret = drm_modeset_lock(&crtc->cursor->mutex, &ctx);
-	if (ret)
-		goto fail;
-
 	/*
 	 * Obtain fb we'll be using (either new or existing) and take an extra
 	 * reference to it if fb != null.  setplane will take care of dropping
@@ -693,7 +684,7 @@ static int drm_mode_cursor_universal(struct drm_crtc *crtc,
 	 */
 	ret = __setplane_internal(crtc->cursor, crtc, fb,
 				crtc_x, crtc_y, crtc_w, crtc_h,
-				0, 0, src_w, src_h, &ctx);
+				0, 0, src_w, src_h, ctx);
 
 	/* Update successful; save new cursor position, if necessary */
 	if (ret == 0 && req->flags & DRM_MODE_CURSOR_MOVE) {
@@ -701,15 +692,6 @@ static int drm_mode_cursor_universal(struct drm_crtc *crtc,
 		crtc->cursor_y = req->y;
 	}
 
-fail:
-	if (ret == -EDEADLK) {
-		drm_modeset_backoff(&ctx);
-		goto retry;
-	}
-
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
-
 	return ret;
 }
 
@@ -718,6 +700,7 @@ static int drm_mode_cursor_common(struct drm_device *dev,
 				  struct drm_file *file_priv)
 {
 	struct drm_crtc *crtc;
+	struct drm_modeset_acquire_ctx ctx;
 	int ret = 0;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
@@ -732,14 +715,24 @@ static int drm_mode_cursor_common(struct drm_device *dev,
 		return -ENOENT;
 	}
 
+	drm_modeset_acquire_init(&ctx, 0);
+retry:
+	ret = drm_modeset_lock(&crtc->mutex, &ctx);
+	if (ret)
+		goto out;
+	ret = drm_modeset_lock(&crtc->cursor->mutex, &ctx);
+	if (ret)
+		goto out;
+
 	/*
 	 * If this crtc has a universal cursor plane, call that plane's update
 	 * handler rather than using legacy cursor handlers.
 	 */
-	if (crtc->cursor)
-		return drm_mode_cursor_universal(crtc, req, file_priv);
+	if (crtc->cursor) {
+		ret = drm_mode_cursor_universal(crtc, req, file_priv, &ctx);
+		goto out;
+	}
 
-	drm_modeset_lock_crtc(crtc, crtc->cursor);
 	if (req->flags & DRM_MODE_CURSOR_BO) {
 		if (!crtc->funcs->cursor_set && !crtc->funcs->cursor_set2) {
 			ret = -ENXIO;
@@ -763,7 +756,13 @@ static int drm_mode_cursor_common(struct drm_device *dev,
 		}
 	}
 out:
-	drm_modeset_unlock_crtc(crtc);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
 
 	return ret;
 
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index 88d35bfc9cd8..2bb065bf0593 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -121,7 +121,6 @@ struct drm_plane;
 
 void drm_modeset_lock_all(struct drm_device *dev);
 void drm_modeset_unlock_all(struct drm_device *dev);
-void drm_modeset_unlock_crtc(struct drm_crtc *crtc);
 void drm_warn_on_modeset_not_all_locked(struct drm_device *dev);
 struct drm_modeset_acquire_ctx *
 drm_modeset_legacy_acquire_ctx(struct drm_crtc *crtc);
-- 
2.11.0

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

Re: [PATCH 02/15] drm: Remove drm_modeset_(un)lock_crtc

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2536 bytes --]

Hi Daniel,

[auto build test ERROR on next-20170330]
[cannot apply to drm/drm-next drm-intel/for-linux-next robclark/msm-next v4.9-rc8 v4.9-rc7 v4.9-rc6 v4.11-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Daniel-Vetter/acquire-ctx-wire-up-part-2/20170404-053514
config: i386-randconfig-x010-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c: In function 'vmw_du_crtc_cursor_set2':
>> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:158:2: error: implicit declaration of function 'drm_modeset_unlock_crtc' [-Werror=implicit-function-declaration]
     drm_modeset_unlock_crtc(crtc);
     ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:228:2: error: implicit declaration of function 'drm_modeset_lock_crtc' [-Werror=implicit-function-declaration]
     drm_modeset_lock_crtc(crtc, crtc->cursor);
     ^~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/drm_modeset_unlock_crtc +158 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c

bfb89928 Daniel Vetter    2012-12-02  152  	 * FIXME: Unclear whether there's any global state touched by the
bfb89928 Daniel Vetter    2012-12-02  153  	 * cursor_set function, especially vmw_cursor_update_position looks
bfb89928 Daniel Vetter    2012-12-02  154  	 * suspicious. For now take the easy route and reacquire all locks. We
bfb89928 Daniel Vetter    2012-12-02  155  	 * can do this since the caller in the drm core doesn't check anything
bfb89928 Daniel Vetter    2012-12-02  156  	 * which is protected by any looks.
bfb89928 Daniel Vetter    2012-12-02  157  	 */
21e88620 Rob Clark        2014-10-30 @158  	drm_modeset_unlock_crtc(crtc);
bfb89928 Daniel Vetter    2012-12-02  159  	drm_modeset_lock_all(dev_priv->dev);
8fbf9d92 Thomas Hellstrom 2015-11-26  160  	hotspot_x = hot_x + du->hotspot_x;
8fbf9d92 Thomas Hellstrom 2015-11-26  161  	hotspot_y = hot_y + du->hotspot_y;

:::::: The code at line 158 was first introduced by commit
:::::: 21e88620aa21b48d4f62d29275e3e2944a5ea2b5 drm/vmwgfx: fix lock breakage

:::::: TO: Rob Clark <robdclark@gmail.com>
:::::: CC: Thomas Hellstrom <thellstrom@vmware.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27264 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

Re: [Intel-gfx] [PATCH 02/15] drm: Remove drm_modeset_(un)lock_crtc

[Daniel Vetter]

On Tue, Apr 4, 2017 at 12:13 AM, kbuild test robot <lkp@intel.com> wrote:
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

It should compile just fine on latest linux-next (if there is one)
where this code in vmwgfx is already removed. Well you just need the
latest drm-next from Dave Airlie.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/15] drm: Remove drm_modeset_(un)lock_crtc

[Mike Lothian]

[-- Attachment #1.1: Type: text/plain, Size: 912 bytes --]

Hi

X no longer starts for me and I've bisected it back to this commit

During bisect I ended up on commits where my laptop would lockup during boot

Regards

Mike

On Tue, 4 Apr 2017 at 06:39 Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> On Tue, Apr 4, 2017 at 12:13 AM, kbuild test robot <lkp@intel.com> wrote:
> > [if your patch is applied to the wrong git tree, please drop us a note
> to help improve the system]
>
> It should compile just fine on latest linux-next (if there is one)
> where this code in vmwgfx is already removed. Well you just need the
> latest drm-next from Dave Airlie.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 <+41%2079%20365%2057%2048> - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[-- Attachment #1.2: Type: text/html, Size: 1654 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [Intel-gfx] [PATCH 02/15] drm: Remove drm_modeset_(un)lock_crtc

[Daniel Vetter]

On Fri, Apr 14, 2017 at 6:20 PM, Mike Lothian <mike@fireburn.co.uk> wrote:
> Hi
>
> X no longer starts for me and I've bisected it back to this commit
>
> During bisect I ended up on commits where my laptop would lockup during boot

The bugfix the hangs is in linux-next, but hasn't landed in drm-next yet:

commit 8cb68c83ab99a474ae847602f0c514d0fe17cc82
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Mon Apr 10 13:54:45 2017 +0200

    drm: Fix get_property logic fumble

The bugfix for the issue you bisected is already fixed in

commit 2e0641631f233b5af09f0bfeaa6220d10cad75e7
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Fri Apr 7 18:48:17 2017 +0200

    drm: Only take cursor locks when the cursor plane exists

If you still have troubles with those patches applied, then I need to dig more.
-Daniel

>
> Regards
>
> Mike
>
> On Tue, 4 Apr 2017 at 06:39 Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>
>> On Tue, Apr 4, 2017 at 12:13 AM, kbuild test robot <lkp@intel.com> wrote:
>> > [if your patch is applied to the wrong git tree, please drop us a note
>> > to help improve the system]
>>
>> It should compile just fine on latest linux-next (if there is one)
>> where this code in vmwgfx is already removed. Well you just need the
>> latest drm-next from Dave Airlie.
>> -Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/15] drm: Remove drm_modeset_legacy_acquire_ctx and crtc->acquire_ctx

[Daniel Vetter]

With all the callers of drm_modeset_lock_crtc gone, and all the places
it was formerly used properly wiring the acquire ctx through, we can
remove this.

The only hidden context magic we still have is now the global one.

Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_atomic.c          | 14 --------------
 drivers/gpu/drm/drm_atomic_helper.c   |  2 +-
 drivers/gpu/drm/drm_modeset_lock.c    | 21 ---------------------
 drivers/gpu/drm/i915/intel_display.c  |  4 ++--
 drivers/gpu/drm/i915/intel_pipe_crc.c |  2 +-
 include/drm/drm_crtc.h                |  9 ---------
 include/drm/drm_modeset_lock.h        |  2 --
 7 files changed, 4 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 9b892af7811a..345310213820 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1516,19 +1516,9 @@ EXPORT_SYMBOL(drm_atomic_add_affected_planes);
 void drm_atomic_legacy_backoff(struct drm_atomic_state *state)
 {
 	struct drm_device *dev = state->dev;
-	unsigned crtc_mask = 0;
-	struct drm_crtc *crtc;
 	int ret;
 	bool global = false;
 
-	drm_for_each_crtc(crtc, dev) {
-		if (crtc->acquire_ctx != state->acquire_ctx)
-			continue;
-
-		crtc_mask |= drm_crtc_mask(crtc);
-		crtc->acquire_ctx = NULL;
-	}
-
 	if (WARN_ON(dev->mode_config.acquire_ctx == state->acquire_ctx)) {
 		global = true;
 
@@ -1542,10 +1532,6 @@ void drm_atomic_legacy_backoff(struct drm_atomic_state *state)
 	if (ret)
 		goto retry;
 
-	drm_for_each_crtc(crtc, dev)
-		if (drm_crtc_mask(crtc) & crtc_mask)
-			crtc->acquire_ctx = state->acquire_ctx;
-
 	if (global)
 		dev->mode_config.acquire_ctx = state->acquire_ctx;
 }
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index f2d62620e5f8..8999da789bb0 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2976,7 +2976,7 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector,
 	if (!state)
 		return -ENOMEM;
 
-	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+	state->acquire_ctx = crtc->dev->mode_config.acquire_ctx;
 retry:
 	crtc_state = drm_atomic_get_crtc_state(state, crtc);
 	if (IS_ERR(crtc_state)) {
diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index c3ca6b859236..64ef09a6cccb 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -149,27 +149,6 @@ void drm_modeset_unlock_all(struct drm_device *dev)
 EXPORT_SYMBOL(drm_modeset_unlock_all);
 
 /**
- * drm_modeset_legacy_acquire_ctx - find acquire ctx for legacy ioctls
- * @crtc: drm crtc
- *
- * Legacy ioctl operations like cursor updates or page flips only have per-crtc
- * locking, and store the acquire ctx in the corresponding crtc. All other
- * legacy operations take all locks and use a global acquire context. This
- * function grabs the right one.
- */
-struct drm_modeset_acquire_ctx *
-drm_modeset_legacy_acquire_ctx(struct drm_crtc *crtc)
-{
-	if (crtc->acquire_ctx)
-		return crtc->acquire_ctx;
-
-	WARN_ON(!crtc->dev->mode_config.acquire_ctx);
-
-	return crtc->dev->mode_config.acquire_ctx;
-}
-EXPORT_SYMBOL(drm_modeset_legacy_acquire_ctx);
-
-/**
  * drm_warn_on_modeset_not_all_locked - check that all modeset locks are locked
  * @dev: device
  *
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 81baa5a9780c..ba6687e31cbd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10727,7 +10727,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		state = drm_atomic_state_alloc(dev);
 		if (!state)
 			return -ENOMEM;
-		state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+		state->acquire_ctx = dev->mode_config.acquire_ctx;
 
 retry:
 		plane_state = drm_atomic_get_plane_state(state, primary);
@@ -13090,7 +13090,7 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 		return;
 	}
 
-	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+	state->acquire_ctx = crtc->dev->mode_config.acquire_ctx;
 
 retry:
 	crtc_state = drm_atomic_get_crtc_state(state, crtc);
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
index 9fd9c70baeed..206ee4f0150e 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -522,7 +522,7 @@ static void hsw_trans_edp_pipe_A_crc_wa(struct drm_i915_private *dev_priv,
 		goto unlock;
 	}
 
-	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(&crtc->base);
+	state->acquire_ctx = crtc->base.dev->mode_config.acquire_ctx;
 	pipe_config = intel_atomic_get_crtc_state(state, crtc);
 	if (IS_ERR(pipe_config)) {
 		ret = PTR_ERR(pipe_config);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 2be2192b1373..ede60d67976f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -782,15 +782,6 @@ struct drm_crtc {
 	 */
 	spinlock_t commit_lock;
 
-	/**
-	 * @acquire_ctx:
-	 *
-	 * Per-CRTC implicit acquire context used by atomic drivers for legacy
-	 * IOCTLs, so that atomic drivers can get at the locking acquire
-	 * context.
-	 */
-	struct drm_modeset_acquire_ctx *acquire_ctx;
-
 #ifdef CONFIG_DEBUG_FS
 	/**
 	 * @debugfs_entry:
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index 2bb065bf0593..4b27c2bb955c 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -122,8 +122,6 @@ struct drm_plane;
 void drm_modeset_lock_all(struct drm_device *dev);
 void drm_modeset_unlock_all(struct drm_device *dev);
 void drm_warn_on_modeset_not_all_locked(struct drm_device *dev);
-struct drm_modeset_acquire_ctx *
-drm_modeset_legacy_acquire_ctx(struct drm_crtc *crtc);
 
 int drm_modeset_lock_all_ctx(struct drm_device *dev,
 			     struct drm_modeset_acquire_ctx *ctx);
-- 
2.11.0

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

[PATCH 04/15] drm/atomic-helper: remove modeset_lock_all from helper_resume

[Daniel Vetter]

Atomic code rely shouldn't rely on the magic hidden acquire context.

v2: Remove unused config local var (gcc).

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 8999da789bb0..978dd8f49476 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2623,14 +2623,22 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_duplicated_state);
 int drm_atomic_helper_resume(struct drm_device *dev,
 			     struct drm_atomic_state *state)
 {
-	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_modeset_acquire_ctx ctx;
 	int err;
 
 	drm_mode_config_reset(dev);
 
-	drm_modeset_lock_all(dev);
-	err = drm_atomic_helper_commit_duplicated_state(state, config->acquire_ctx);
-	drm_modeset_unlock_all(dev);
+	drm_modeset_acquire_init(&ctx, 0);
+	while (1) {
+		err = drm_atomic_helper_commit_duplicated_state(state, &ctx);
+		if (err != -EDEADLK)
+			break;
+
+		drm_modeset_backoff(&ctx);
+	}
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
 
 	return err;
 }
-- 
2.11.0

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

Re: [PATCH 04/15] drm/atomic-helper: remove modeset_lock_all from helper_resume

[Alex Deucher]

On Mon, Apr 3, 2017 at 4:32 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Atomic code rely shouldn't rely on the magic hidden acquire context.

Repeated rely in commit message.  I'm assuming you mean:
"Atomic code shouldn't rely on the magic hidden acquire context."

>
> v2: Remove unused config local var (gcc).
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 8999da789bb0..978dd8f49476 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2623,14 +2623,22 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_duplicated_state);
>  int drm_atomic_helper_resume(struct drm_device *dev,
>                              struct drm_atomic_state *state)
>  {
> -       struct drm_mode_config *config = &dev->mode_config;
> +       struct drm_modeset_acquire_ctx ctx;
>         int err;
>
>         drm_mode_config_reset(dev);
>
> -       drm_modeset_lock_all(dev);
> -       err = drm_atomic_helper_commit_duplicated_state(state, config->acquire_ctx);
> -       drm_modeset_unlock_all(dev);
> +       drm_modeset_acquire_init(&ctx, 0);
> +       while (1) {
> +               err = drm_atomic_helper_commit_duplicated_state(state, &ctx);
> +               if (err != -EDEADLK)
> +                       break;
> +
> +               drm_modeset_backoff(&ctx);
> +       }
> +
> +       drm_modeset_drop_locks(&ctx);
> +       drm_modeset_acquire_fini(&ctx);
>
>         return err;
>  }
> --
> 2.11.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/15] drm: drop modeset_lock_all from drm_state_info

[Daniel Vetter]

If we push the locks down we don't have to take them all at the same
time.

Aside: Making dump_info fully safe should be fairly simple, if we
protect the ->state pointers with rcu. Simply putting a
synchronize_rcu() into the drm_atomic_state free function should be
all that's roughly needed. Well except we shouldn't block in there, so
better to put that into a work_struct. But I've not set out to fix
that little issue.

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_atomic.c | 60 ++++++++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 345310213820..9afb14371ce0 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1676,22 +1676,8 @@ static void drm_atomic_print_state(const struct drm_atomic_state *state)
 		drm_atomic_connector_print_state(&p, connector_state);
 }
 
-/**
- * drm_state_dump - dump entire device atomic state
- * @dev: the drm device
- * @p: where to print the state to
- *
- * Just for debugging.  Drivers might want an option to dump state
- * to dmesg in case of error irq's.  (Hint, you probably want to
- * ratelimit this!)
- *
- * The caller must drm_modeset_lock_all(), or if this is called
- * from error irq handler, it should not be enabled by default.
- * (Ie. if you are debugging errors you might not care that this
- * is racey.  But calling this without all modeset locks held is
- * not inherently safe.)
- */
-void drm_state_dump(struct drm_device *dev, struct drm_printer *p)
+static void __drm_state_dump(struct drm_device *dev, struct drm_printer *p,
+			     bool take_locks)
 {
 	struct drm_mode_config *config = &dev->mode_config;
 	struct drm_plane *plane;
@@ -1702,17 +1688,51 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p)
 	if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
 		return;
 
-	list_for_each_entry(plane, &config->plane_list, head)
+	list_for_each_entry(plane, &config->plane_list, head) {
+		if (take_locks)
+			drm_modeset_lock(&plane->mutex, NULL);
 		drm_atomic_plane_print_state(p, plane->state);
+		if (take_locks)
+			drm_modeset_unlock(&plane->mutex);
+	}
 
-	list_for_each_entry(crtc, &config->crtc_list, head)
+	list_for_each_entry(crtc, &config->crtc_list, head) {
+		if (take_locks)
+			drm_modeset_lock(&crtc->mutex, NULL);
 		drm_atomic_crtc_print_state(p, crtc->state);
+		if (take_locks)
+			drm_modeset_unlock(&crtc->mutex);
+	}
 
 	drm_connector_list_iter_begin(dev, &conn_iter);
+	if (take_locks)
+		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	drm_for_each_connector_iter(connector, &conn_iter)
 		drm_atomic_connector_print_state(p, connector->state);
+	if (take_locks)
+		drm_modeset_unlock(&dev->mode_config.connection_mutex);
 	drm_connector_list_iter_end(&conn_iter);
 }
+
+/**
+ * drm_state_dump - dump entire device atomic state
+ * @dev: the drm device
+ * @p: where to print the state to
+ *
+ * Just for debugging.  Drivers might want an option to dump state
+ * to dmesg in case of error irq's.  (Hint, you probably want to
+ * ratelimit this!)
+ *
+ * The caller must drm_modeset_lock_all(), or if this is called
+ * from error irq handler, it should not be enabled by default.
+ * (Ie. if you are debugging errors you might not care that this
+ * is racey.  But calling this without all modeset locks held is
+ * not inherently safe.)
+ */
+void drm_state_dump(struct drm_device *dev, struct drm_printer *p)
+{
+	__drm_state_dump(dev, p, false);
+}
 EXPORT_SYMBOL(drm_state_dump);
 
 #ifdef CONFIG_DEBUG_FS
@@ -1722,9 +1742,7 @@ static int drm_state_info(struct seq_file *m, void *data)
 	struct drm_device *dev = node->minor->dev;
 	struct drm_printer p = drm_seq_file_printer(m);
 
-	drm_modeset_lock_all(dev);
-	drm_state_dump(dev, &p);
-	drm_modeset_unlock_all(dev);
+	__drm_state_dump(dev, &p, true);
 
 	return 0;
 }
-- 
2.11.0

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

[PATCH 06/15] drm: Drop modeset_lock_all from the getproperty ioctl

[Daniel Vetter]

Properties, i.e. the struct drm_property specifying the type and value
range of a property, not the instantiation on a given object, are
invariant over the lifetime of a driver.

Hence no locking at all is needed, we can just remove it.

While at it give the function some love and simplify it, to get it
under the 80 char limit:
- Straighten the loops to reduce the nesting.
- use u64_to_user_ptr casting helper
- use put_user for fixed u64 copies.

Note there's a small behavioural change in that we now copy parts of
the values to userspace if the arrays are a bit too small. Since
userspace will immediately retry anyway, this doesn't matter.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_property.c | 72 +++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index b17959c3e099..3feef0659940 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -442,8 +442,7 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
 	struct drm_property *property;
 	int enum_count = 0;
 	int value_count = 0;
-	int ret = 0, i;
-	int copied;
+	int i, copied;
 	struct drm_property_enum *prop_enum;
 	struct drm_mode_property_enum __user *enum_ptr;
 	uint64_t __user *values_ptr;
@@ -451,55 +450,43 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
 
-	drm_modeset_lock_all(dev);
 	property = drm_property_find(dev, out_resp->prop_id);
-	if (!property) {
-		ret = -ENOENT;
-		goto done;
-	}
-
-	if (drm_property_type_is(property, DRM_MODE_PROP_ENUM) ||
-			drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
-		list_for_each_entry(prop_enum, &property->enum_list, head)
-			enum_count++;
-	}
-
-	value_count = property->num_values;
+	if (!property)
+		return -ENOENT;
 
 	strncpy(out_resp->name, property->name, DRM_PROP_NAME_LEN);
 	out_resp->name[DRM_PROP_NAME_LEN-1] = 0;
 	out_resp->flags = property->flags;
 
-	if ((out_resp->count_values >= value_count) && value_count) {
-		values_ptr = (uint64_t __user *)(unsigned long)out_resp->values_ptr;
-		for (i = 0; i < value_count; i++) {
-			if (copy_to_user(values_ptr + i, &property->values[i], sizeof(uint64_t))) {
-				ret = -EFAULT;
-				goto done;
-			}
+	value_count = property->num_values;
+	values_ptr = u64_to_user_ptr(out_resp->values_ptr);
+
+	for (i = 0; i < value_count; i++) {
+		if (i < out_resp->count_values &&
+		    put_user(property->values[i], values_ptr + i)) {
+			return -EFAULT;
 		}
 	}
 	out_resp->count_values = value_count;
 
+	copied = 0;
+	enum_ptr = u64_to_user_ptr(out_resp->enum_blob_ptr);
+
 	if (drm_property_type_is(property, DRM_MODE_PROP_ENUM) ||
-			drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
-		if ((out_resp->count_enum_blobs >= enum_count) && enum_count) {
-			copied = 0;
-			enum_ptr = (struct drm_mode_property_enum __user *)(unsigned long)out_resp->enum_blob_ptr;
-			list_for_each_entry(prop_enum, &property->enum_list, head) {
-
-				if (copy_to_user(&enum_ptr[copied].value, &prop_enum->value, sizeof(uint64_t))) {
-					ret = -EFAULT;
-					goto done;
-				}
-
-				if (copy_to_user(&enum_ptr[copied].name,
-						 &prop_enum->name, DRM_PROP_NAME_LEN)) {
-					ret = -EFAULT;
-					goto done;
-				}
-				copied++;
-			}
+	    drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
+		list_for_each_entry(prop_enum, &property->enum_list, head) {
+			enum_count++;
+			if (out_resp->count_enum_blobs <= enum_count)
+				continue;
+
+			if (copy_to_user(&enum_ptr[copied].value,
+					 &prop_enum->value, sizeof(uint64_t)))
+				return -EFAULT;
+
+			if (copy_to_user(&enum_ptr[copied].name,
+					 &prop_enum->name, DRM_PROP_NAME_LEN))
+				return -EFAULT;
+			copied++;
 		}
 		out_resp->count_enum_blobs = enum_count;
 	}
@@ -514,9 +501,8 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
 	 */
 	if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
 		out_resp->count_enum_blobs = 0;
-done:
-	drm_modeset_unlock_all(dev);
-	return ret;
+
+	return 0;
 }
 
 static void drm_property_free_blob(struct kref *kref)
-- 
2.11.0

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

Re: [PATCH 06/15] drm: Drop modeset_lock_all from the getproperty ioctl

[Alex Deucher]

On Mon, Apr 3, 2017 at 4:32 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Properties, i.e. the struct drm_property specifying the type and value
> range of a property, not the instantiation on a given object, are
> invariant over the lifetime of a driver.
>
> Hence no locking at all is needed, we can just remove it.
>
> While at it give the function some love and simplify it, to get it
> under the 80 char limit:
> - Straighten the loops to reduce the nesting.
> - use u64_to_user_ptr casting helper
> - use put_user for fixed u64 copies.
>
> Note there's a small behavioural change in that we now copy parts of
> the values to userspace if the arrays are a bit too small. Since
> userspace will immediately retry anyway, this doesn't matter.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

This causes a segfault in our ddx:
https://bugs.freedesktop.org/show_bug.cgi?id=100673

Alex


> ---
>  drivers/gpu/drm/drm_property.c | 72 +++++++++++++++++-------------------------
>  1 file changed, 29 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index b17959c3e099..3feef0659940 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -442,8 +442,7 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
>         struct drm_property *property;
>         int enum_count = 0;
>         int value_count = 0;
> -       int ret = 0, i;
> -       int copied;
> +       int i, copied;
>         struct drm_property_enum *prop_enum;
>         struct drm_mode_property_enum __user *enum_ptr;
>         uint64_t __user *values_ptr;
> @@ -451,55 +450,43 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
>         if (!drm_core_check_feature(dev, DRIVER_MODESET))
>                 return -EINVAL;
>
> -       drm_modeset_lock_all(dev);
>         property = drm_property_find(dev, out_resp->prop_id);
> -       if (!property) {
> -               ret = -ENOENT;
> -               goto done;
> -       }
> -
> -       if (drm_property_type_is(property, DRM_MODE_PROP_ENUM) ||
> -                       drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
> -               list_for_each_entry(prop_enum, &property->enum_list, head)
> -                       enum_count++;
> -       }
> -
> -       value_count = property->num_values;
> +       if (!property)
> +               return -ENOENT;
>
>         strncpy(out_resp->name, property->name, DRM_PROP_NAME_LEN);
>         out_resp->name[DRM_PROP_NAME_LEN-1] = 0;
>         out_resp->flags = property->flags;
>
> -       if ((out_resp->count_values >= value_count) && value_count) {
> -               values_ptr = (uint64_t __user *)(unsigned long)out_resp->values_ptr;
> -               for (i = 0; i < value_count; i++) {
> -                       if (copy_to_user(values_ptr + i, &property->values[i], sizeof(uint64_t))) {
> -                               ret = -EFAULT;
> -                               goto done;
> -                       }
> +       value_count = property->num_values;
> +       values_ptr = u64_to_user_ptr(out_resp->values_ptr);
> +
> +       for (i = 0; i < value_count; i++) {
> +               if (i < out_resp->count_values &&
> +                   put_user(property->values[i], values_ptr + i)) {
> +                       return -EFAULT;
>                 }
>         }
>         out_resp->count_values = value_count;
>
> +       copied = 0;
> +       enum_ptr = u64_to_user_ptr(out_resp->enum_blob_ptr);
> +
>         if (drm_property_type_is(property, DRM_MODE_PROP_ENUM) ||
> -                       drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
> -               if ((out_resp->count_enum_blobs >= enum_count) && enum_count) {
> -                       copied = 0;
> -                       enum_ptr = (struct drm_mode_property_enum __user *)(unsigned long)out_resp->enum_blob_ptr;
> -                       list_for_each_entry(prop_enum, &property->enum_list, head) {
> -
> -                               if (copy_to_user(&enum_ptr[copied].value, &prop_enum->value, sizeof(uint64_t))) {
> -                                       ret = -EFAULT;
> -                                       goto done;
> -                               }
> -
> -                               if (copy_to_user(&enum_ptr[copied].name,
> -                                                &prop_enum->name, DRM_PROP_NAME_LEN)) {
> -                                       ret = -EFAULT;
> -                                       goto done;
> -                               }
> -                               copied++;
> -                       }
> +           drm_property_type_is(property, DRM_MODE_PROP_BITMASK)) {
> +               list_for_each_entry(prop_enum, &property->enum_list, head) {
> +                       enum_count++;
> +                       if (out_resp->count_enum_blobs <= enum_count)
> +                               continue;
> +
> +                       if (copy_to_user(&enum_ptr[copied].value,
> +                                        &prop_enum->value, sizeof(uint64_t)))
> +                               return -EFAULT;
> +
> +                       if (copy_to_user(&enum_ptr[copied].name,
> +                                        &prop_enum->name, DRM_PROP_NAME_LEN))
> +                               return -EFAULT;
> +                       copied++;
>                 }
>                 out_resp->count_enum_blobs = enum_count;
>         }
> @@ -514,9 +501,8 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
>          */
>         if (drm_property_type_is(property, DRM_MODE_PROP_BLOB))
>                 out_resp->count_enum_blobs = 0;
> -done:
> -       drm_modeset_unlock_all(dev);
> -       return ret;
> +
> +       return 0;
>  }
>
>  static void drm_property_free_blob(struct kref *kref)
> --
> 2.11.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 06/15] drm: Drop modeset_lock_all from the getproperty ioctl

[Chris Wilson]

On Thu, Apr 13, 2017 at 04:03:16PM -0400, Alex Deucher wrote:
> On Mon, Apr 3, 2017 at 4:32 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Properties, i.e. the struct drm_property specifying the type and value
> > range of a property, not the instantiation on a given object, are
> > invariant over the lifetime of a driver.
> >
> > Hence no locking at all is needed, we can just remove it.
> >
> > While at it give the function some love and simplify it, to get it
> > under the 80 char limit:
> > - Straighten the loops to reduce the nesting.
> > - use u64_to_user_ptr casting helper
> > - use put_user for fixed u64 copies.
> >
> > Note there's a small behavioural change in that we now copy parts of
> > the values to userspace if the arrays are a bit too small. Since
> > userspace will immediately retry anyway, this doesn't matter.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> This causes a segfault in our ddx:
> https://bugs.freedesktop.org/show_bug.cgi?id=100673

Should be fixed by:

commit 8cb68c83ab99a474ae847602f0c514d0fe17cc82
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Mon Apr 10 13:54:45 2017 +0200

    drm: Fix get_property logic fumble
    
    Yet again I've proven that I can't negate conditions :(
    
    Testcase: igt/kms_properties/get_property-sanity
    Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
    Reviewed-by: Sean Paul <seanpaul@chromium.org>
    Fixes: eb8eb02ed850 ("drm: Drop modeset_lock_all from the getproperty ioctl")

Does that help?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/15] drm: Only take crtc lock in get_gamma ioctl

[Daniel Vetter]

We don't call into drivers at all here, this is enough. Also, we can
reduce the critical section a bit to simplify the code.
crtc->gamma_size is set up once at driver load and then invariant, so
also doesn't need any protection.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_color_mgmt.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index cc23b9a505c0..a32be59a72d1 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -295,19 +295,15 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev,
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
 
-	drm_modeset_lock_all(dev);
 	crtc = drm_crtc_find(dev, crtc_lut->crtc_id);
-	if (!crtc) {
-		ret = -ENOENT;
-		goto out;
-	}
+	if (!crtc)
+		return -ENOENT;
 
 	/* memcpy into gamma store */
-	if (crtc_lut->gamma_size != crtc->gamma_size) {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (crtc_lut->gamma_size != crtc->gamma_size)
+		return -EINVAL;
 
+	drm_modeset_lock(&crtc->mutex, NULL);
 	size = crtc_lut->gamma_size * (sizeof(uint16_t));
 	r_base = crtc->gamma_store;
 	if (copy_to_user((void __user *)(unsigned long)crtc_lut->red, r_base, size)) {
@@ -327,6 +323,6 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev,
 		goto out;
 	}
 out:
-	drm_modeset_unlock_all(dev);
+	drm_modeset_unlock(&crtc->mutex);
 	return ret;
 }
-- 
2.11.0

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

[PATCH 08/15] drm/i915: Nuke intel_atomic_legacy_gamma_set

[Daniel Vetter]

We do set DRIVER_ATOMIC now.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 44 +-----------------------------------
 1 file changed, 1 insertion(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ba6687e31cbd..779ab46200c2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13113,50 +13113,8 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 	drm_atomic_state_put(state);
 }
 
-/*
- * FIXME: Remove this once i915 is fully DRIVER_ATOMIC by calling
- *        drm_atomic_helper_legacy_gamma_set() directly.
- */
-static int intel_atomic_legacy_gamma_set(struct drm_crtc *crtc,
-					 u16 *red, u16 *green, u16 *blue,
-					 uint32_t size)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_mode_config *config = &dev->mode_config;
-	struct drm_crtc_state *state;
-	int ret;
-
-	ret = drm_atomic_helper_legacy_gamma_set(crtc, red, green, blue, size);
-	if (ret)
-		return ret;
-
-	/*
-	 * Make sure we update the legacy properties so this works when
-	 * atomic is not enabled.
-	 */
-
-	state = crtc->state;
-
-	drm_object_property_set_value(&crtc->base,
-				      config->degamma_lut_property,
-				      (state->degamma_lut) ?
-				      state->degamma_lut->base.id : 0);
-
-	drm_object_property_set_value(&crtc->base,
-				      config->ctm_property,
-				      (state->ctm) ?
-				      state->ctm->base.id : 0);
-
-	drm_object_property_set_value(&crtc->base,
-				      config->gamma_lut_property,
-				      (state->gamma_lut) ?
-				      state->gamma_lut->base.id : 0);
-
-	return 0;
-}
-
 static const struct drm_crtc_funcs intel_crtc_funcs = {
-	.gamma_set = intel_atomic_legacy_gamma_set,
+	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 	.set_config = drm_atomic_helper_set_config,
 	.set_property = drm_atomic_helper_crtc_set_property,
 	.destroy = intel_crtc_destroy,
-- 
2.11.0

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

Re: [PATCH 08/15] drm/i915: Nuke intel_atomic_legacy_gamma_set

[Maarten Lankhorst]

Op 03-04-17 om 10:32 schreef Daniel Vetter:
> We do set DRIVER_ATOMIC now.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 44 +-----------------------------------
>  1 file changed, 1 insertion(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ba6687e31cbd..779ab46200c2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13113,50 +13113,8 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
>  	drm_atomic_state_put(state);
>  }
>  
> -/*
> - * FIXME: Remove this once i915 is fully DRIVER_ATOMIC by calling
> - *        drm_atomic_helper_legacy_gamma_set() directly.
> - */
> -static int intel_atomic_legacy_gamma_set(struct drm_crtc *crtc,
> -					 u16 *red, u16 *green, u16 *blue,
> -					 uint32_t size)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_mode_config *config = &dev->mode_config;
> -	struct drm_crtc_state *state;
> -	int ret;
> -
> -	ret = drm_atomic_helper_legacy_gamma_set(crtc, red, green, blue, size);
> -	if (ret)
> -		return ret;
> -
> -	/*
> -	 * Make sure we update the legacy properties so this works when
> -	 * atomic is not enabled.
> -	 */
> -
> -	state = crtc->state;
> -
> -	drm_object_property_set_value(&crtc->base,
> -				      config->degamma_lut_property,
> -				      (state->degamma_lut) ?
> -				      state->degamma_lut->base.id : 0);
> -
> -	drm_object_property_set_value(&crtc->base,
> -				      config->ctm_property,
> -				      (state->ctm) ?
> -				      state->ctm->base.id : 0);
> -
> -	drm_object_property_set_value(&crtc->base,
> -				      config->gamma_lut_property,
> -				      (state->gamma_lut) ?
> -				      state->gamma_lut->base.id : 0);
> -
> -	return 0;
> -}
> -
>  static const struct drm_crtc_funcs intel_crtc_funcs = {
> -	.gamma_set = intel_atomic_legacy_gamma_set,
> +	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  	.set_config = drm_atomic_helper_set_config,
>  	.set_property = drm_atomic_helper_crtc_set_property,
>  	.destroy = intel_crtc_destroy,

I guess the comment is out of date with drm_object_property_get_value now checking for
drm_drv_uses_atomic_modeset instead.

All patches in this series look good, I've seen nothing to comment on. :)

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

Re: [PATCH 08/15] drm/i915: Nuke intel_atomic_legacy_gamma_set

[Daniel Vetter]

On Tue, Apr 04, 2017 at 12:44:26PM +0200, Maarten Lankhorst wrote:
> Op 03-04-17 om 10:32 schreef Daniel Vetter:
> > We do set DRIVER_ATOMIC now.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 44 +-----------------------------------
> >  1 file changed, 1 insertion(+), 43 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ba6687e31cbd..779ab46200c2 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13113,50 +13113,8 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
> >  	drm_atomic_state_put(state);
> >  }
> >  
> > -/*
> > - * FIXME: Remove this once i915 is fully DRIVER_ATOMIC by calling
> > - *        drm_atomic_helper_legacy_gamma_set() directly.
> > - */
> > -static int intel_atomic_legacy_gamma_set(struct drm_crtc *crtc,
> > -					 u16 *red, u16 *green, u16 *blue,
> > -					 uint32_t size)
> > -{
> > -	struct drm_device *dev = crtc->dev;
> > -	struct drm_mode_config *config = &dev->mode_config;
> > -	struct drm_crtc_state *state;
> > -	int ret;
> > -
> > -	ret = drm_atomic_helper_legacy_gamma_set(crtc, red, green, blue, size);
> > -	if (ret)
> > -		return ret;
> > -
> > -	/*
> > -	 * Make sure we update the legacy properties so this works when
> > -	 * atomic is not enabled.
> > -	 */
> > -
> > -	state = crtc->state;
> > -
> > -	drm_object_property_set_value(&crtc->base,
> > -				      config->degamma_lut_property,
> > -				      (state->degamma_lut) ?
> > -				      state->degamma_lut->base.id : 0);
> > -
> > -	drm_object_property_set_value(&crtc->base,
> > -				      config->ctm_property,
> > -				      (state->ctm) ?
> > -				      state->ctm->base.id : 0);
> > -
> > -	drm_object_property_set_value(&crtc->base,
> > -				      config->gamma_lut_property,
> > -				      (state->gamma_lut) ?
> > -				      state->gamma_lut->base.id : 0);
> > -
> > -	return 0;
> > -}
> > -
> >  static const struct drm_crtc_funcs intel_crtc_funcs = {
> > -	.gamma_set = intel_atomic_legacy_gamma_set,
> > +	.gamma_set = drm_atomic_helper_legacy_gamma_set,
> >  	.set_config = drm_atomic_helper_set_config,
> >  	.set_property = drm_atomic_helper_crtc_set_property,
> >  	.destroy = intel_crtc_destroy,
> 
> I guess the comment is out of date with drm_object_property_get_value now checking for
> drm_drv_uses_atomic_modeset instead.

Good point, I added a note about that to the commit message before
applying.
> 
> All patches in this series look good, I've seen nothing to comment on. :)
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/15] drm/msm: Nerf zpos property

[Daniel Vetter]

It's not wired up, and if it is, it should be moved over to the new
fancy standardized zpos property exposed through
drm_plane_create_zpos_property().

Cc: Rob Clark <robdclark@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 60a5451ae0b9..9229c6e201a2 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -108,8 +108,6 @@ static void mdp5_plane_install_properties(struct drm_plane *plane,
 				create_enum, name##_prop_enum_list, \
 				ARRAY_SIZE(name##_prop_enum_list))
 
-	INSTALL_RANGE_PROPERTY(zpos, ZPOS, 1, 255, 1);
-
 	mdp5_plane_install_rotation_property(dev, plane);
 
 #undef INSTALL_RANGE_PROPERTY
-- 
2.11.0

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

Re: [PATCH 09/15] drm/msm: Nerf zpos property

[Daniel Vetter]

On Mon, Apr 03, 2017 at 10:32:58AM +0200, Daniel Vetter wrote:
> It's not wired up, and if it is, it should be moved over to the new
> fancy standardized zpos property exposed through
> drm_plane_create_zpos_property().
> 
> Cc: Rob Clark <robdclark@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Discussed this with Rob Clark on irc, I was wrong, this is all properly
wired up. No idea why I failed at parsing git grep output ... This should
be moved over to drm_plane_create_zpos_property(), but this patch here is
bogus. I'll drop it.
-Daniel

> ---
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> index 60a5451ae0b9..9229c6e201a2 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
> @@ -108,8 +108,6 @@ static void mdp5_plane_install_properties(struct drm_plane *plane,
>  				create_enum, name##_prop_enum_list, \
>  				ARRAY_SIZE(name##_prop_enum_list))
>  
> -	INSTALL_RANGE_PROPERTY(zpos, ZPOS, 1, 255, 1);
> -
>  	mdp5_plane_install_rotation_property(dev, plane);
>  
>  #undef INSTALL_RANGE_PROPERTY
> -- 
> 2.11.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/15] drm/fb-helper: Give up on kgdb for atomic drivers

[Daniel Vetter]

It just doesn't work. It probably stopped working way, way before that
(e.g. i915 grabbed random mutexes all over in modeset code at least
since gen6), but with atomic and all the ww_mutex stuff it's indeed
hopeless.

Remove ->mode_set_base_atomic from the 2 atomic drivers (i915 and
nouveau) that still had one (both had dummy implementations already
anyway), and shunt atomic drivers in the helpers debug_enter/leave
functions.

I'll leave the code in for radeon and amdgpu, but I think as soon as
amdgpu is atomic we should think about just ripping it out. Only
having it around for radeon and pre-nv50 is rather pointless. This
would also allow us to nuke all that code from fbdev.

Funny part is that _all_ kms drivers set this hook, despite that no
one else provides the required ->mode_set_base_atomic implementation.

The reason I'm jumping on this is that I want to wire up a full
acquire ctx for the benefit of atomic drivers, everywhere. And the
debug_enter/leave implementations call ->gamma_set. And there's just
no way ever we can create an acquire_ctx in the nmi context of kgdb.

Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c        |  6 ++++++
 drivers/gpu/drm/i915/intel_display.c   | 12 ------------
 drivers/gpu/drm/nouveau/nv50_display.c | 10 ----------
 3 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 673a47445d61..9147abb774e8 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -281,6 +281,9 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
 			if (funcs->mode_set_base_atomic == NULL)
 				continue;
 
+			if (drm_drv_uses_atomic_modeset(mode_set->crtc->dev))
+				continue;
+
 			drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
 			funcs->mode_set_base_atomic(mode_set->crtc,
 						    mode_set->fb,
@@ -338,6 +341,9 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
 		if (funcs->mode_set_base_atomic == NULL)
 			continue;
 
+		if (drm_drv_uses_atomic_modeset(crtc->dev))
+			continue;
+
 		drm_fb_helper_restore_lut_atomic(mode_set->crtc);
 		funcs->mode_set_base_atomic(mode_set->crtc, fb, crtc->x,
 					    crtc->y, LEAVE_ATOMIC_MODE_SET);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 779ab46200c2..2bc9f2f609a9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3412,17 +3412,6 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
-/* Assume fb object is pinned & idle & fenced and just update base pointers */
-static int
-intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
-			   int x, int y, enum mode_set_atomic state)
-{
-	/* Support for kgdboc is disabled, this needs a major rework. */
-	DRM_ERROR("legacy panic handler not supported any more.\n");
-
-	return -ENODEV;
-}
-
 static void intel_complete_page_flips(struct drm_i915_private *dev_priv)
 {
 	struct intel_crtc *crtc;
@@ -11017,7 +11006,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 }
 
 static const struct drm_crtc_helper_funcs intel_helper_funcs = {
-	.mode_set_base_atomic = intel_pipe_set_base_atomic,
 	.atomic_begin = intel_begin_crtc_commit,
 	.atomic_flush = intel_finish_crtc_commit,
 	.atomic_check = intel_crtc_atomic_check,
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 418872b493a3..3d381d5c82ce 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -2210,18 +2210,8 @@ nv50_head_lut_load(struct drm_crtc *crtc)
 	}
 }
 
-static int
-nv50_head_mode_set_base_atomic(struct drm_crtc *crtc,
-			       struct drm_framebuffer *fb, int x, int y,
-			       enum mode_set_atomic state)
-{
-	WARN_ON(1);
-	return 0;
-}
-
 static const struct drm_crtc_helper_funcs
 nv50_head_help = {
-	.mode_set_base_atomic = nv50_head_mode_set_base_atomic,
 	.load_lut = nv50_head_lut_load,
 	.atomic_check = nv50_head_atomic_check,
 };
-- 
2.11.0

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

[PATCH 11/15] drm: Add explicit acquire ctx handling around ->gamma_set

[Daniel Vetter]

Just the groundwork to prepare for adding the acquire cxt parameter to
the ->gamma_set hook. Again we need a temporary hack to fill out
mode_config.acquire_ctx until the atomic helpers are switched over.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_color_mgmt.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index a32be59a72d1..e1b4084c3d16 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -218,28 +218,29 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 	struct drm_crtc *crtc;
 	void *r_base, *g_base, *b_base;
 	int size;
+	struct drm_modeset_acquire_ctx ctx;
 	int ret = 0;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
 
-	drm_modeset_lock_all(dev);
 	crtc = drm_crtc_find(dev, crtc_lut->crtc_id);
-	if (!crtc) {
-		ret = -ENOENT;
-		goto out;
-	}
+	if (!crtc)
+		return -ENOENT;
 
-	if (crtc->funcs->gamma_set == NULL) {
-		ret = -ENOSYS;
-		goto out;
-	}
+	if (crtc->funcs->gamma_set == NULL)
+		return -ENOSYS;
 
 	/* memcpy into gamma store */
-	if (crtc_lut->gamma_size != crtc->gamma_size) {
-		ret = -EINVAL;
+	if (crtc_lut->gamma_size != crtc->gamma_size)
+		return -EINVAL;
+
+	drm_modeset_acquire_init(&ctx, 0);
+	dev->mode_config.acquire_ctx = &ctx;
+retry:
+	ret = drm_modeset_lock_all_ctx(dev, &ctx);
+	if (ret)
 		goto out;
-	}
 
 	size = crtc_lut->gamma_size * (sizeof(uint16_t));
 	r_base = crtc->gamma_store;
@@ -263,7 +264,13 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 	ret = crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, crtc->gamma_size);
 
 out:
-	drm_modeset_unlock_all(dev);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
 	return ret;
 
 }
-- 
2.11.0

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

[PATCH 12/15] drm: Add acquire ctx to ->gamma_set hook

[Daniel Vetter]

Atomic helpers really want this instead of the hacked-up legacy
backoff trick, which unfortunately prevents drivers from using their
own private drm_modeset_locks.

Aside: There's a few atomic drivers (nv50, vc4, soon vmwgfx) which
don't yet use the new atomic color mgmt/gamma table stuff. Would be
nice if they could switch over and just hook up
drm_atomic_helper_legacy_gamma_set() instead.

Cc: Dave Airlie <airlied@redhat.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Sinclair Yeh <syeh@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c   | 3 ++-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c   | 3 ++-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c    | 3 ++-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c    | 3 ++-
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 ++-
 drivers/gpu/drm/ast/ast_mode.c           | 3 ++-
 drivers/gpu/drm/cirrus/cirrus_mode.c     | 3 ++-
 drivers/gpu/drm/drm_atomic_helper.c      | 4 +++-
 drivers/gpu/drm/drm_color_mgmt.c         | 3 ++-
 drivers/gpu/drm/drm_fb_helper.c          | 3 ++-
 drivers/gpu/drm/gma500/gma_display.c     | 3 ++-
 drivers/gpu/drm/gma500/gma_display.h     | 3 ++-
 drivers/gpu/drm/mgag200/mgag200_mode.c   | 3 ++-
 drivers/gpu/drm/nouveau/dispnv04/crtc.c  | 3 ++-
 drivers/gpu/drm/nouveau/nv50_display.c   | 3 ++-
 drivers/gpu/drm/radeon/radeon_display.c  | 3 ++-
 drivers/gpu/drm/vc4/vc4_crtc.c           | 3 ++-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c      | 3 ++-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h      | 3 ++-
 include/drm/drm_atomic_helper.h          | 3 ++-
 include/drm/drm_crtc.h                   | 3 ++-
 21 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index f525ae4e0576..daf003dd2351 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2631,7 +2631,8 @@ static void dce_v10_0_cursor_reset(struct drm_crtc *crtc)
 }
 
 static int dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				    u16 *blue, uint32_t size)
+				    u16 *blue, uint32_t size,
+				    struct drm_modeset_acquire_ctx *ctx)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 3eac27f24d94..3a7296724457 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -2651,7 +2651,8 @@ static void dce_v11_0_cursor_reset(struct drm_crtc *crtc)
 }
 
 static int dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				    u16 *blue, uint32_t size)
+				    u16 *blue, uint32_t size,
+				    struct drm_modeset_acquire_ctx *ctx)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index 838cf1a778f2..8ccada5d6f39 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -1998,7 +1998,8 @@ static void dce_v6_0_cursor_reset(struct drm_crtc *crtc)
 }
 
 static int dce_v6_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				   u16 *blue, uint32_t size)
+				   u16 *blue, uint32_t size,
+				   struct drm_modeset_acquire_ctx *ctx)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 1b0717b11efe..6943f2641c90 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -2482,7 +2482,8 @@ static void dce_v8_0_cursor_reset(struct drm_crtc *crtc)
 }
 
 static int dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				   u16 *blue, uint32_t size)
+				   u16 *blue, uint32_t size,
+				   struct drm_modeset_acquire_ctx *ctx)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index 5c51f9a97811..81a24b6b4846 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -165,7 +165,8 @@ static void dce_virtual_bandwidth_update(struct amdgpu_device *adev)
 }
 
 static int dce_virtual_crtc_gamma_set(struct drm_crtc *crtc, u16 *red,
-				      u16 *green, u16 *blue, uint32_t size)
+				      u16 *green, u16 *blue, uint32_t size,
+				      struct drm_modeset_acquire_ctx *ctx)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 47b78e52691c..aaef0a652f10 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -645,7 +645,8 @@ static void ast_crtc_reset(struct drm_crtc *crtc)
 }
 
 static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, uint32_t size)
+			      u16 *blue, uint32_t size,
+			      struct drm_modeset_acquire_ctx *ctx)
 {
 	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c
index ed43ab10ac99..53f6f0f84206 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -327,7 +327,8 @@ static void cirrus_crtc_commit(struct drm_crtc *crtc)
  * but it's a requirement that we provide the function
  */
 static int cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				 u16 *blue, uint32_t size)
+				 u16 *blue, uint32_t size,
+				 struct drm_modeset_acquire_ctx *ctx)
 {
 	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 978dd8f49476..d5915317e7d3 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3480,6 +3480,7 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
  * @green: green correction table
  * @blue: green correction table
  * @size: size of the tables
+ * @ctx: lock acquire context
  *
  * Implements support for legacy gamma correction table for drivers
  * that support color management through the DEGAMMA_LUT/GAMMA_LUT
@@ -3487,7 +3488,8 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
  */
 int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 				       u16 *red, u16 *green, u16 *blue,
-				       uint32_t size)
+				       uint32_t size,
+				       struct drm_modeset_acquire_ctx *ctx)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_mode_config *config = &dev->mode_config;
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index e1b4084c3d16..b81dcb1d4cb3 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -261,7 +261,8 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 		goto out;
 	}
 
-	ret = crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, crtc->gamma_size);
+	ret = crtc->funcs->gamma_set(crtc, r_base, g_base, b_base,
+				     crtc->gamma_size, &ctx);
 
 out:
 	if (ret == -EDEADLK) {
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 9147abb774e8..6dc5381e1c45 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -256,7 +256,8 @@ static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
 	g_base = r_base + crtc->gamma_size;
 	b_base = g_base + crtc->gamma_size;
 
-	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, crtc->gamma_size);
+	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base,
+			       crtc->gamma_size, NULL);
 }
 
 /**
diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
index 93ff46535c04..e7fd356acf2e 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -177,7 +177,8 @@ void gma_crtc_load_lut(struct drm_crtc *crtc)
 }
 
 int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue,
-		       u32 size)
+		       u32 size,
+		       struct drm_modeset_acquire_ctx *ctx)
 {
 	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/gma500/gma_display.h b/drivers/gpu/drm/gma500/gma_display.h
index 166e608923db..239c374b6169 100644
--- a/drivers/gpu/drm/gma500/gma_display.h
+++ b/drivers/gpu/drm/gma500/gma_display.h
@@ -73,7 +73,8 @@ extern int gma_crtc_cursor_set(struct drm_crtc *crtc,
 extern int gma_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
 extern void gma_crtc_load_lut(struct drm_crtc *crtc);
 extern int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, u32 size);
+			      u16 *blue, u32 size,
+			      struct drm_modeset_acquire_ctx *ctx);
 extern void gma_crtc_dpms(struct drm_crtc *crtc, int mode);
 extern void gma_crtc_prepare(struct drm_crtc *crtc);
 extern void gma_crtc_commit(struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index f2e9b2bc18a5..adb411a078e8 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1393,7 +1393,8 @@ static void mga_crtc_commit(struct drm_crtc *crtc)
  * but it's a requirement that we provide the function
  */
 static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-			      u16 *blue, uint32_t size)
+			      u16 *blue, uint32_t size,
+			      struct drm_modeset_acquire_ctx *ctx)
 {
 	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 43ab560de7f9..4b4b0b496262 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -788,7 +788,8 @@ nv_crtc_disable(struct drm_crtc *crtc)
 
 static int
 nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
-		  uint32_t size)
+		  uint32_t size,
+		  struct drm_modeset_acquire_ctx *ctx)
 {
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 3d381d5c82ce..bf504788bce6 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -2218,7 +2218,8 @@ nv50_head_help = {
 
 static int
 nv50_head_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
-		    uint32_t size)
+		    uint32_t size,
+		    struct drm_modeset_acquire_ctx *ctx)
 {
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
 	u32 i;
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 146297a702ab..981385eb5389 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -232,7 +232,8 @@ void radeon_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
 }
 
 static int radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				 u16 *blue, uint32_t size)
+				 u16 *blue, uint32_t size,
+				 struct drm_modeset_acquire_ctx *ctx)
 {
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
 	int i;
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 04c390a487ba..1b4dbe9e1c6d 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -300,7 +300,8 @@ vc4_crtc_lut_load(struct drm_crtc *crtc)
 
 static int
 vc4_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
-		   uint32_t size)
+		   uint32_t size,
+		   struct drm_modeset_acquire_ctx *ctx)
 {
 	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
 	u32 i;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 6078654d033b..ef9f3a2a4030 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -2026,7 +2026,8 @@ static int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,
 
 int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
 			  u16 *r, u16 *g, u16 *b,
-			  uint32_t size)
+			  uint32_t size,
+			  struct drm_modeset_acquire_ctx *ctx)
 {
 	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
 	int i;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 0c226b2adea5..13f2f1d2818a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -254,7 +254,8 @@ void vmw_du_crtc_save(struct drm_crtc *crtc);
 void vmw_du_crtc_restore(struct drm_crtc *crtc);
 int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
 			   u16 *r, u16 *g, u16 *b,
-			   uint32_t size);
+			   uint32_t size,
+			   struct drm_modeset_acquire_ctx *ctx);
 int vmw_du_crtc_cursor_set2(struct drm_crtc *crtc, struct drm_file *file_priv,
 			    uint32_t handle, uint32_t width, uint32_t height,
 			    int32_t hot_x, int32_t hot_y);
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index fd395dc050ee..f0a8678ae98e 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -176,7 +176,8 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
 					  struct drm_connector_state *state);
 int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 				       u16 *red, u16 *green, u16 *blue,
-				       uint32_t size);
+				       uint32_t size,
+				       struct drm_modeset_acquire_ctx *ctx);
 
 /**
  * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index ede60d67976f..a8176a836e25 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -322,7 +322,8 @@ struct drm_crtc_funcs {
 	 * hooks.
 	 */
 	int (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
-			 uint32_t size);
+			 uint32_t size,
+			 struct drm_modeset_acquire_ctx *ctx);
 
 	/**
 	 * @destroy:
-- 
2.11.0

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

Re: [PATCH 12/15] drm: Add acquire ctx to ->gamma_set hook

[Sinclair Yeh]

vmwgfx part:  Reviewed-by: Sinclair Yeh <syeh@vmware.com>

On Mon, Apr 03, 2017 at 10:33:01AM +0200, Daniel Vetter wrote:
> Atomic helpers really want this instead of the hacked-up legacy
> backoff trick, which unfortunately prevents drivers from using their
> own private drm_modeset_locks.
> 
> Aside: There's a few atomic drivers (nv50, vc4, soon vmwgfx) which
> don't yet use the new atomic color mgmt/gamma table stuff. Would be
> nice if they could switch over and just hook up
> drm_atomic_helper_legacy_gamma_set() instead.
> 
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: Eric Anholt <eric@anholt.net>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c   | 3 ++-
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c   | 3 ++-
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c    | 3 ++-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c    | 3 ++-
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 ++-
>  drivers/gpu/drm/ast/ast_mode.c           | 3 ++-
>  drivers/gpu/drm/cirrus/cirrus_mode.c     | 3 ++-
>  drivers/gpu/drm/drm_atomic_helper.c      | 4 +++-
>  drivers/gpu/drm/drm_color_mgmt.c         | 3 ++-
>  drivers/gpu/drm/drm_fb_helper.c          | 3 ++-
>  drivers/gpu/drm/gma500/gma_display.c     | 3 ++-
>  drivers/gpu/drm/gma500/gma_display.h     | 3 ++-
>  drivers/gpu/drm/mgag200/mgag200_mode.c   | 3 ++-
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c  | 3 ++-
>  drivers/gpu/drm/nouveau/nv50_display.c   | 3 ++-
>  drivers/gpu/drm/radeon/radeon_display.c  | 3 ++-
>  drivers/gpu/drm/vc4/vc4_crtc.c           | 3 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c      | 3 ++-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h      | 3 ++-
>  include/drm/drm_atomic_helper.h          | 3 ++-
>  include/drm/drm_crtc.h                   | 3 ++-
>  21 files changed, 43 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index f525ae4e0576..daf003dd2351 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2631,7 +2631,8 @@ static void dce_v10_0_cursor_reset(struct drm_crtc *crtc)
>  }
>  
>  static int dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				    u16 *blue, uint32_t size)
> +				    u16 *blue, uint32_t size,
> +				    struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>  	int i;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index 3eac27f24d94..3a7296724457 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -2651,7 +2651,8 @@ static void dce_v11_0_cursor_reset(struct drm_crtc *crtc)
>  }
>  
>  static int dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				    u16 *blue, uint32_t size)
> +				    u16 *blue, uint32_t size,
> +				    struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>  	int i;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 838cf1a778f2..8ccada5d6f39 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -1998,7 +1998,8 @@ static void dce_v6_0_cursor_reset(struct drm_crtc *crtc)
>  }
>  
>  static int dce_v6_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				   u16 *blue, uint32_t size)
> +				   u16 *blue, uint32_t size,
> +				   struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>  	int i;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index 1b0717b11efe..6943f2641c90 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -2482,7 +2482,8 @@ static void dce_v8_0_cursor_reset(struct drm_crtc *crtc)
>  }
>  
>  static int dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				   u16 *blue, uint32_t size)
> +				   u16 *blue, uint32_t size,
> +				   struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>  	int i;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> index 5c51f9a97811..81a24b6b4846 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> @@ -165,7 +165,8 @@ static void dce_virtual_bandwidth_update(struct amdgpu_device *adev)
>  }
>  
>  static int dce_virtual_crtc_gamma_set(struct drm_crtc *crtc, u16 *red,
> -				      u16 *green, u16 *blue, uint32_t size)
> +				      u16 *green, u16 *blue, uint32_t size,
> +				      struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
>  	int i;
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 47b78e52691c..aaef0a652f10 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -645,7 +645,8 @@ static void ast_crtc_reset(struct drm_crtc *crtc)
>  }
>  
>  static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -			      u16 *blue, uint32_t size)
> +			      u16 *blue, uint32_t size,
> +			      struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
>  	int i;
> diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c
> index ed43ab10ac99..53f6f0f84206 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_mode.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
> @@ -327,7 +327,8 @@ static void cirrus_crtc_commit(struct drm_crtc *crtc)
>   * but it's a requirement that we provide the function
>   */
>  static int cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				 u16 *blue, uint32_t size)
> +				 u16 *blue, uint32_t size,
> +				 struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
>  	int i;
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 978dd8f49476..d5915317e7d3 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3480,6 +3480,7 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
>   * @green: green correction table
>   * @blue: green correction table
>   * @size: size of the tables
> + * @ctx: lock acquire context
>   *
>   * Implements support for legacy gamma correction table for drivers
>   * that support color management through the DEGAMMA_LUT/GAMMA_LUT
> @@ -3487,7 +3488,8 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
>   */
>  int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
>  				       u16 *red, u16 *green, u16 *blue,
> -				       uint32_t size)
> +				       uint32_t size,
> +				       struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_mode_config *config = &dev->mode_config;
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index e1b4084c3d16..b81dcb1d4cb3 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -261,7 +261,8 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
>  		goto out;
>  	}
>  
> -	ret = crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, crtc->gamma_size);
> +	ret = crtc->funcs->gamma_set(crtc, r_base, g_base, b_base,
> +				     crtc->gamma_size, &ctx);
>  
>  out:
>  	if (ret == -EDEADLK) {
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 9147abb774e8..6dc5381e1c45 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -256,7 +256,8 @@ static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
>  	g_base = r_base + crtc->gamma_size;
>  	b_base = g_base + crtc->gamma_size;
>  
> -	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, crtc->gamma_size);
> +	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base,
> +			       crtc->gamma_size, NULL);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
> index 93ff46535c04..e7fd356acf2e 100644
> --- a/drivers/gpu/drm/gma500/gma_display.c
> +++ b/drivers/gpu/drm/gma500/gma_display.c
> @@ -177,7 +177,8 @@ void gma_crtc_load_lut(struct drm_crtc *crtc)
>  }
>  
>  int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue,
> -		       u32 size)
> +		       u32 size,
> +		       struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
>  	int i;
> diff --git a/drivers/gpu/drm/gma500/gma_display.h b/drivers/gpu/drm/gma500/gma_display.h
> index 166e608923db..239c374b6169 100644
> --- a/drivers/gpu/drm/gma500/gma_display.h
> +++ b/drivers/gpu/drm/gma500/gma_display.h
> @@ -73,7 +73,8 @@ extern int gma_crtc_cursor_set(struct drm_crtc *crtc,
>  extern int gma_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
>  extern void gma_crtc_load_lut(struct drm_crtc *crtc);
>  extern int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -			      u16 *blue, u32 size);
> +			      u16 *blue, u32 size,
> +			      struct drm_modeset_acquire_ctx *ctx);
>  extern void gma_crtc_dpms(struct drm_crtc *crtc, int mode);
>  extern void gma_crtc_prepare(struct drm_crtc *crtc);
>  extern void gma_crtc_commit(struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index f2e9b2bc18a5..adb411a078e8 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1393,7 +1393,8 @@ static void mga_crtc_commit(struct drm_crtc *crtc)
>   * but it's a requirement that we provide the function
>   */
>  static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -			      u16 *blue, uint32_t size)
> +			      u16 *blue, uint32_t size,
> +			      struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
>  	int i;
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index 43ab560de7f9..4b4b0b496262 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -788,7 +788,8 @@ nv_crtc_disable(struct drm_crtc *crtc)
>  
>  static int
>  nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> -		  uint32_t size)
> +		  uint32_t size,
> +		  struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
>  	int i;
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index 3d381d5c82ce..bf504788bce6 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -2218,7 +2218,8 @@ nv50_head_help = {
>  
>  static int
>  nv50_head_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> -		    uint32_t size)
> +		    uint32_t size,
> +		    struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
>  	u32 i;
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
> index 146297a702ab..981385eb5389 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -232,7 +232,8 @@ void radeon_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
>  }
>  
>  static int radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				 u16 *blue, uint32_t size)
> +				 u16 *blue, uint32_t size,
> +				 struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
>  	int i;
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 04c390a487ba..1b4dbe9e1c6d 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -300,7 +300,8 @@ vc4_crtc_lut_load(struct drm_crtc *crtc)
>  
>  static int
>  vc4_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> -		   uint32_t size)
> +		   uint32_t size,
> +		   struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
>  	u32 i;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 6078654d033b..ef9f3a2a4030 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -2026,7 +2026,8 @@ static int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,
>  
>  int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
>  			  u16 *r, u16 *g, u16 *b,
> -			  uint32_t size)
> +			  uint32_t size,
> +			  struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
>  	int i;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> index 0c226b2adea5..13f2f1d2818a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> @@ -254,7 +254,8 @@ void vmw_du_crtc_save(struct drm_crtc *crtc);
>  void vmw_du_crtc_restore(struct drm_crtc *crtc);
>  int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
>  			   u16 *r, u16 *g, u16 *b,
> -			   uint32_t size);
> +			   uint32_t size,
> +			   struct drm_modeset_acquire_ctx *ctx);
>  int vmw_du_crtc_cursor_set2(struct drm_crtc *crtc, struct drm_file *file_priv,
>  			    uint32_t handle, uint32_t width, uint32_t height,
>  			    int32_t hot_x, int32_t hot_y);
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index fd395dc050ee..f0a8678ae98e 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -176,7 +176,8 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
>  					  struct drm_connector_state *state);
>  int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
>  				       u16 *red, u16 *green, u16 *blue,
> -				       uint32_t size);
> +				       uint32_t size,
> +				       struct drm_modeset_acquire_ctx *ctx);
>  
>  /**
>   * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index ede60d67976f..a8176a836e25 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -322,7 +322,8 @@ struct drm_crtc_funcs {
>  	 * hooks.
>  	 */
>  	int (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> -			 uint32_t size);
> +			 uint32_t size,
> +			 struct drm_modeset_acquire_ctx *ctx);
>  
>  	/**
>  	 * @destroy:
> -- 
> 2.11.0
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 12/15] drm: Add acquire ctx to ->gamma_set hook

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 563 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> Atomic helpers really want this instead of the hacked-up legacy
> backoff trick, which unfortunately prevents drivers from using their
> own private drm_modeset_locks.
>
> Aside: There's a few atomic drivers (nv50, vc4, soon vmwgfx) which
> don't yet use the new atomic color mgmt/gamma table stuff. Would be
> nice if they could switch over and just hook up
> drm_atomic_helper_legacy_gamma_set() instead.

For notes like this, it would be helpful to have a pointer to a driver
doing it cleanly the current way.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 12/15] drm: Add acquire ctx to ->gamma_set hook

[Daniel Vetter]

On Thu, Apr 6, 2017 at 6:51 PM, Eric Anholt <eric@anholt.net> wrote:
> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
>> Atomic helpers really want this instead of the hacked-up legacy
>> backoff trick, which unfortunately prevents drivers from using their
>> own private drm_modeset_locks.
>>
>> Aside: There's a few atomic drivers (nv50, vc4, soon vmwgfx) which
>> don't yet use the new atomic color mgmt/gamma table stuff. Would be
>> nice if they could switch over and just hook up
>> drm_atomic_helper_legacy_gamma_set() instead.
>
> For notes like this, it would be helpful to have a pointer to a driver
> doing it cleanly the current way.

Yeah, good point. And the kernel-doc for legacy_gamma_set is also not
all that awesome. So we currently have i915, mediatek and omapdrm
using this, and for the new atomic gamma stuff we do have fairly
reasonable docs (just a bit harder to find that I thought they'd be):
https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#color-management-properties

I'll try and type up a patch to sprinkle a few more hints around and
connect the pieces.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 13/15] drm/atomic-helper: Remove legacy backoff hack from gamma_set

[Daniel Vetter]

Another one knocked down.

With this we can also remove the temporary hack in the gamma_set
ioctl.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 13 ++-----------
 drivers/gpu/drm/drm_color_mgmt.c    |  1 -
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index d5915317e7d3..8de6cea733f4 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3520,8 +3520,7 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 		blob_data[i].blue = blue[i];
 	}
 
-	state->acquire_ctx = crtc->dev->mode_config.acquire_ctx;
-retry:
+	state->acquire_ctx = ctx;
 	crtc_state = drm_atomic_get_crtc_state(state, crtc);
 	if (IS_ERR(crtc_state)) {
 		ret = PTR_ERR(crtc_state);
@@ -3545,18 +3544,10 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 		goto fail;
 
 	ret = drm_atomic_commit(state);
-fail:
-	if (ret == -EDEADLK)
-		goto backoff;
 
+fail:
 	drm_atomic_state_put(state);
 	drm_property_blob_put(blob);
 	return ret;
-
-backoff:
-	drm_atomic_state_clear(state);
-	drm_atomic_legacy_backoff(state);
-
-	goto retry;
 }
 EXPORT_SYMBOL(drm_atomic_helper_legacy_gamma_set);
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index b81dcb1d4cb3..533f3a3e6877 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -236,7 +236,6 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 		return -EINVAL;
 
 	drm_modeset_acquire_init(&ctx, 0);
-	dev->mode_config.acquire_ctx = &ctx;
 retry:
 	ret = drm_modeset_lock_all_ctx(dev, &ctx);
 	if (ret)
-- 
2.11.0

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

[PATCH 14/15] drm: extract legacy framebuffer remove

[Daniel Vetter]

I got confused every time I audited what that lock_all is doing in
there until realizing it's for legacy kms only. Make that a notch more
obvious by having 2 entirely different paths.

While at it also move the atomic version of this into
drm_framebuffer.c, there's no reason it needs to be in drm_atomic.c.
That way it becomes a simple static function.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_atomic.c        |  88 -----------------------
 drivers/gpu/drm/drm_crtc_internal.h |   1 -
 drivers/gpu/drm/drm_framebuffer.c   | 137 ++++++++++++++++++++++++++++++------
 3 files changed, 115 insertions(+), 111 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 9afb14371ce0..f32506a7c1d6 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -2081,94 +2081,6 @@ static void complete_crtc_signaling(struct drm_device *dev,
 	kfree(fence_state);
 }
 
-int drm_atomic_remove_fb(struct drm_framebuffer *fb)
-{
-	struct drm_modeset_acquire_ctx ctx;
-	struct drm_device *dev = fb->dev;
-	struct drm_atomic_state *state;
-	struct drm_plane *plane;
-	struct drm_connector *conn;
-	struct drm_connector_state *conn_state;
-	int i, ret = 0;
-	unsigned plane_mask;
-
-	state = drm_atomic_state_alloc(dev);
-	if (!state)
-		return -ENOMEM;
-
-	drm_modeset_acquire_init(&ctx, 0);
-	state->acquire_ctx = &ctx;
-
-retry:
-	plane_mask = 0;
-	ret = drm_modeset_lock_all_ctx(dev, &ctx);
-	if (ret)
-		goto unlock;
-
-	drm_for_each_plane(plane, dev) {
-		struct drm_plane_state *plane_state;
-
-		if (plane->state->fb != fb)
-			continue;
-
-		plane_state = drm_atomic_get_plane_state(state, plane);
-		if (IS_ERR(plane_state)) {
-			ret = PTR_ERR(plane_state);
-			goto unlock;
-		}
-
-		if (plane_state->crtc->primary == plane) {
-			struct drm_crtc_state *crtc_state;
-
-			crtc_state = drm_atomic_get_existing_crtc_state(state, plane_state->crtc);
-
-			ret = drm_atomic_add_affected_connectors(state, plane_state->crtc);
-			if (ret)
-				goto unlock;
-
-			crtc_state->active = false;
-			ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL);
-			if (ret)
-				goto unlock;
-		}
-
-		drm_atomic_set_fb_for_plane(plane_state, NULL);
-		ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
-		if (ret)
-			goto unlock;
-
-		plane_mask |= BIT(drm_plane_index(plane));
-
-		plane->old_fb = plane->fb;
-	}
-
-	for_each_connector_in_state(state, conn, conn_state, i) {
-		ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
-
-		if (ret)
-			goto unlock;
-	}
-
-	if (plane_mask)
-		ret = drm_atomic_commit(state);
-
-unlock:
-	if (plane_mask)
-		drm_atomic_clean_old_fb(dev, plane_mask, ret);
-
-	if (ret == -EDEADLK) {
-		drm_modeset_backoff(&ctx);
-		goto retry;
-	}
-
-	drm_atomic_state_put(state);
-
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
-
-	return ret;
-}
-
 int drm_mode_atomic_ioctl(struct drm_device *dev,
 			  void *data, struct drm_file *file_priv)
 {
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 8c04275cf226..d077c5490041 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -182,7 +182,6 @@ int drm_atomic_get_property(struct drm_mode_object *obj,
 			    struct drm_property *property, uint64_t *val);
 int drm_mode_atomic_ioctl(struct drm_device *dev,
 			  void *data, struct drm_file *file_priv);
-int drm_atomic_remove_fb(struct drm_framebuffer *fb);
 
 
 /* drm_plane.c */
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index e8f9c13a0afd..fc8ef42203ec 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -24,6 +24,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_auth.h>
 #include <drm/drm_framebuffer.h>
+#include <drm/drm_atomic.h>
 
 #include "drm_crtc_internal.h"
 
@@ -755,6 +756,117 @@ void drm_framebuffer_cleanup(struct drm_framebuffer *fb)
 }
 EXPORT_SYMBOL(drm_framebuffer_cleanup);
 
+static int atomic_remove_fb(struct drm_framebuffer *fb)
+{
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_device *dev = fb->dev;
+	struct drm_atomic_state *state;
+	struct drm_plane *plane;
+	struct drm_connector *conn;
+	struct drm_connector_state *conn_state;
+	int i, ret = 0;
+	unsigned plane_mask;
+
+	state = drm_atomic_state_alloc(dev);
+	if (!state)
+		return -ENOMEM;
+
+	drm_modeset_acquire_init(&ctx, 0);
+	state->acquire_ctx = &ctx;
+
+retry:
+	plane_mask = 0;
+	ret = drm_modeset_lock_all_ctx(dev, &ctx);
+	if (ret)
+		goto unlock;
+
+	drm_for_each_plane(plane, dev) {
+		struct drm_plane_state *plane_state;
+
+		if (plane->state->fb != fb)
+			continue;
+
+		plane_state = drm_atomic_get_plane_state(state, plane);
+		if (IS_ERR(plane_state)) {
+			ret = PTR_ERR(plane_state);
+			goto unlock;
+		}
+
+		if (plane_state->crtc->primary == plane) {
+			struct drm_crtc_state *crtc_state;
+
+			crtc_state = drm_atomic_get_existing_crtc_state(state, plane_state->crtc);
+
+			ret = drm_atomic_add_affected_connectors(state, plane_state->crtc);
+			if (ret)
+				goto unlock;
+
+			crtc_state->active = false;
+			ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL);
+			if (ret)
+				goto unlock;
+		}
+
+		drm_atomic_set_fb_for_plane(plane_state, NULL);
+		ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
+		if (ret)
+			goto unlock;
+
+		plane_mask |= BIT(drm_plane_index(plane));
+
+		plane->old_fb = plane->fb;
+	}
+
+	for_each_connector_in_state(state, conn, conn_state, i) {
+		ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
+
+		if (ret)
+			goto unlock;
+	}
+
+	if (plane_mask)
+		ret = drm_atomic_commit(state);
+
+unlock:
+	if (plane_mask)
+		drm_atomic_clean_old_fb(dev, plane_mask, ret);
+
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+
+	drm_atomic_state_put(state);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	return ret;
+}
+
+static void legacy_remove_fb(struct drm_framebuffer *fb)
+{
+	struct drm_device *dev = fb->dev;
+	struct drm_crtc *crtc;
+	struct drm_plane *plane;
+
+	drm_modeset_lock_all(dev);
+	/* remove from any CRTC */
+	drm_for_each_crtc(crtc, dev) {
+		if (crtc->primary->fb == fb) {
+			/* should turn off the crtc */
+			if (drm_crtc_force_disable(crtc))
+				DRM_ERROR("failed to reset crtc %p when fb was deleted\n", crtc);
+		}
+	}
+
+	drm_for_each_plane(plane, dev) {
+		if (plane->fb == fb)
+			drm_plane_force_disable(plane);
+	}
+	drm_modeset_unlock_all(dev);
+}
+
 /**
  * drm_framebuffer_remove - remove and unreference a framebuffer object
  * @fb: framebuffer to remove
@@ -770,8 +882,6 @@ EXPORT_SYMBOL(drm_framebuffer_cleanup);
 void drm_framebuffer_remove(struct drm_framebuffer *fb)
 {
 	struct drm_device *dev;
-	struct drm_crtc *crtc;
-	struct drm_plane *plane;
 
 	if (!fb)
 		return;
@@ -797,29 +907,12 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)
 	 */
 	if (drm_framebuffer_read_refcount(fb) > 1) {
 		if (drm_drv_uses_atomic_modeset(dev)) {
-			int ret = drm_atomic_remove_fb(fb);
+			int ret = atomic_remove_fb(fb);
 			WARN(ret, "atomic remove_fb failed with %i\n", ret);
-			goto out;
-		}
-
-		drm_modeset_lock_all(dev);
-		/* remove from any CRTC */
-		drm_for_each_crtc(crtc, dev) {
-			if (crtc->primary->fb == fb) {
-				/* should turn off the crtc */
-				if (drm_crtc_force_disable(crtc))
-					DRM_ERROR("failed to reset crtc %p when fb was deleted\n", crtc);
-			}
-		}
-
-		drm_for_each_plane(plane, dev) {
-			if (plane->fb == fb)
-				drm_plane_force_disable(plane);
-		}
-		drm_modeset_unlock_all(dev);
+		} else
+			legacy_remove_fb(fb);
 	}
 
-out:
 	drm_framebuffer_put(fb);
 }
 EXPORT_SYMBOL(drm_framebuffer_remove);
-- 
2.11.0

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

[PATCH 15/15] drm/fb-helper: Extract _legacy kms functions

[Daniel Vetter]

The goal is to push all the kms locking down into these separate
_atomic and _legacy functions, so that we can correctly pass the
acquire ctx into all atomic drivers. Instead of playing games with
hidden ctx in mode_config.acquire_ctx. All the fbdev state will be
protected by a new fbdev private lock that Thierry is working on.

This here is just prep by creating a clean split between atomic and
legacy paths, which also simplifies the control flow a bit.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 72 +++++++++++++++++++++++++----------------
 1 file changed, 44 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 6dc5381e1c45..a0ea3241c651 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -418,17 +418,12 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper)
 	goto retry;
 }
 
-static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
+static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
 {
 	struct drm_device *dev = fb_helper->dev;
 	struct drm_plane *plane;
 	int i;
 
-	drm_warn_on_modeset_not_all_locked(dev);
-
-	if (drm_drv_uses_atomic_modeset(dev))
-		return restore_fbdev_mode_atomic(fb_helper);
-
 	drm_for_each_plane(plane, dev) {
 		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
 			drm_plane_force_disable(plane);
@@ -462,6 +457,18 @@ static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
 	return 0;
 }
 
+static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
+{
+	struct drm_device *dev = fb_helper->dev;
+
+	drm_warn_on_modeset_not_all_locked(dev);
+
+	if (drm_drv_uses_atomic_modeset(dev))
+		return restore_fbdev_mode_atomic(fb_helper);
+	else
+		return restore_fbdev_mode_legacy(fb_helper);
+}
+
 /**
  * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
  * @fb_helper: fbcon to restore
@@ -1513,34 +1520,14 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
 	goto retry;
 }
 
-/**
- * drm_fb_helper_pan_display - implementation for &fb_ops.fb_pan_display
- * @var: updated screen information
- * @info: fbdev registered by the helper
- */
-int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
+static int pan_display_legacy(struct fb_var_screeninfo *var,
 			      struct fb_info *info)
 {
 	struct drm_fb_helper *fb_helper = info->par;
-	struct drm_device *dev = fb_helper->dev;
 	struct drm_mode_set *modeset;
 	int ret = 0;
 	int i;
 
-	if (oops_in_progress)
-		return -EBUSY;
-
-	drm_modeset_lock_all(dev);
-	if (!drm_fb_helper_is_bound(fb_helper)) {
-		drm_modeset_unlock_all(dev);
-		return -EBUSY;
-	}
-
-	if (drm_drv_uses_atomic_modeset(dev)) {
-		ret = pan_display_atomic(var, info);
-		goto unlock;
-	}
-
 	for (i = 0; i < fb_helper->crtc_count; i++) {
 		modeset = &fb_helper->crtc_info[i].mode_set;
 
@@ -1555,8 +1542,37 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
 			}
 		}
 	}
-unlock:
+
+	return ret;
+}
+
+/**
+ * drm_fb_helper_pan_display - implementation for &fb_ops.fb_pan_display
+ * @var: updated screen information
+ * @info: fbdev registered by the helper
+ */
+int drm_fb_helper_pan_display(struct fb_var_screeninfo *var,
+			      struct fb_info *info)
+{
+	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_device *dev = fb_helper->dev;
+	int ret;
+
+	if (oops_in_progress)
+		return -EBUSY;
+
+	drm_modeset_lock_all(dev);
+	if (!drm_fb_helper_is_bound(fb_helper)) {
+		drm_modeset_unlock_all(dev);
+		return -EBUSY;
+	}
+
+	if (drm_drv_uses_atomic_modeset(dev))
+		ret = pan_display_atomic(var, info);
+	else
+		ret = pan_display_legacy(var, info);
 	drm_modeset_unlock_all(dev);
+
 	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_pan_display);
-- 
2.11.0

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

✓ Fi.CI.BAT: success for acquire ctx wire-up, part 2

[Patchwork]

== Series Details ==

Series: acquire ctx wire-up, part 2
URL   : https://patchwork.freedesktop.org/series/22354/
State : success

== Summary ==

Series 22354v1 acquire ctx wire-up, part 2
https://patchwork.freedesktop.org/api/1.0/series/22354/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007

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

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 429s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 423s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 574s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 513s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 552s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 485s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 483s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 407s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 409s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 419s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 492s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 473s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 453s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 569s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 449s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 568s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 457s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 494s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 434s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 524s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time: 400s

7eae9908d29d1e42ab9fdb19a7c04eb05dd23b16 drm-tip: 2017y-04m-03d-07h-59m-44s UTC integration manifest
285584b drm/fb-helper: Extract _legacy kms functions
7d9c945 drm: extract legacy framebuffer remove
af8e639 drm/atomic-helper: Remove legacy backoff hack from gamma_set
ab35545 drm: Add acquire ctx to ->gamma_set hook
46b4289 drm: Add explicit acquire ctx handling around ->gamma_set
7d1bb0f drm/fb-helper: Give up on kgdb for atomic drivers
e7dd123 drm/msm: Nerf zpos property
162c8da drm/i915: Nuke intel_atomic_legacy_gamma_set
0c3a80a drm: Only take crtc lock in get_gamma ioctl
9563470 drm: Drop modeset_lock_all from the getproperty ioctl
1cb7c34 drm: drop modeset_lock_all from drm_state_info
281bcd1 drm/atomic-helper: remove modeset_lock_all from helper_resume
9bc6756 drm: Remove drm_modeset_legacy_acquire_ctx and crtc->acquire_ctx
8ee3151 drm: Remove drm_modeset_(un)lock_crtc
b3c2439 drm: Make drm_modeset_lock_crtc internal

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4381/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/15] acquire ctx wire-up, part 2

[Alex Deucher]

On Mon, Apr 3, 2017 at 4:32 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Hi all,
>
> Partially this is a resend of the patches now unblocked by the vmwgfx atomic
> conversion just merged. I could entirely drop the vmwgfx patch since it's all
> fixed now.
>
> Then a bit of follow-up, plus converting the gamma_set/get ioctls. fbdev
> emulation and the property paths are still infested by drm_modeset_lock_all, but
> I think at least for fbdev we now have a semi-clear path with Thierry's series.
> Properties are still unclear to me, because it's a rather layered maze with lots
> of different callsites.
>
> As always, comments and review highly welcome.

Patches 1-3, 5-8, 10-15:
Acked-by: Alex Deucher <alexander.deucher@amd.com>

Patch 4:
typo in the commit message.  With that fixed:
Acked-by: Alex Deucher <alexander.deucher@amd.com>


>
> Cheers, Daniel
>
> Daniel Vetter (15):
>   drm: Make drm_modeset_lock_crtc internal
>   drm: Remove drm_modeset_(un)lock_crtc
>   drm: Remove drm_modeset_legacy_acquire_ctx and crtc->acquire_ctx
>   drm/atomic-helper: remove modeset_lock_all from helper_resume
>   drm: drop modeset_lock_all from drm_state_info
>   drm: Drop modeset_lock_all from the getproperty ioctl
>   drm: Only take crtc lock in get_gamma ioctl
>   drm/i915: Nuke intel_atomic_legacy_gamma_set
>   drm/msm: Nerf zpos property
>   drm/fb-helper: Give up on kgdb for atomic drivers
>   drm: Add explicit acquire ctx handling around ->gamma_set
>   drm: Add acquire ctx to ->gamma_set hook
>   drm/atomic-helper: Remove legacy backoff hack from gamma_set
>   drm: extract legacy framebuffer remove
>   drm/fb-helper: Extract _legacy kms functions
>
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |   3 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |   3 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |   3 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |   3 +-
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c  |   3 +-
>  drivers/gpu/drm/ast/ast_mode.c            |   3 +-
>  drivers/gpu/drm/cirrus/cirrus_mode.c      |   3 +-
>  drivers/gpu/drm/drm_atomic.c              | 162 +++++++-----------------------
>  drivers/gpu/drm/drm_atomic_helper.c       |  35 +++----
>  drivers/gpu/drm/drm_color_mgmt.c          |  51 +++++-----
>  drivers/gpu/drm/drm_crtc_internal.h       |   1 -
>  drivers/gpu/drm/drm_fb_helper.c           |  81 +++++++++------
>  drivers/gpu/drm/drm_framebuffer.c         | 137 +++++++++++++++++++++----
>  drivers/gpu/drm/drm_modeset_lock.c        | 102 -------------------
>  drivers/gpu/drm/drm_plane.c               |  49 +++++----
>  drivers/gpu/drm/drm_property.c            |  72 ++++++-------
>  drivers/gpu/drm/gma500/gma_display.c      |   3 +-
>  drivers/gpu/drm/gma500/gma_display.h      |   3 +-
>  drivers/gpu/drm/i915/intel_display.c      |  60 +----------
>  drivers/gpu/drm/i915/intel_pipe_crc.c     |   2 +-
>  drivers/gpu/drm/mgag200/mgag200_mode.c    |   3 +-
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c |   2 -
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c   |   3 +-
>  drivers/gpu/drm/nouveau/nv50_display.c    |  13 +--
>  drivers/gpu/drm/radeon/radeon_display.c   |   3 +-
>  drivers/gpu/drm/vc4/vc4_crtc.c            |   3 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c       |   3 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h       |   3 +-
>  include/drm/drm_atomic_helper.h           |   3 +-
>  include/drm/drm_crtc.h                    |  12 +--
>  include/drm/drm_modeset_lock.h            |   5 -
>  31 files changed, 344 insertions(+), 488 deletions(-)
>
> --
> 2.11.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/15] acquire ctx wire-up, part 2

[Daniel Vetter]

On Wed, Apr 05, 2017 at 03:45:41PM -0400, Alex Deucher wrote:
> On Mon, Apr 3, 2017 at 4:32 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Hi all,
> >
> > Partially this is a resend of the patches now unblocked by the vmwgfx atomic
> > conversion just merged. I could entirely drop the vmwgfx patch since it's all
> > fixed now.
> >
> > Then a bit of follow-up, plus converting the gamma_set/get ioctls. fbdev
> > emulation and the property paths are still infested by drm_modeset_lock_all, but
> > I think at least for fbdev we now have a semi-clear path with Thierry's series.
> > Properties are still unclear to me, because it's a rather layered maze with lots
> > of different callsites.
> >
> > As always, comments and review highly welcome.
> 
> Patches 1-3, 5-8, 10-15:
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
> 
> Patch 4:
> typo in the commit message.  With that fixed:
> Acked-by: Alex Deucher <alexander.deucher@amd.com>

Thanks for going through, unfortunately I applied 1-8 already. Pulled in
10-15 now too.
-Daniel

> 
> 
> >
> > Cheers, Daniel
> >
> > Daniel Vetter (15):
> >   drm: Make drm_modeset_lock_crtc internal
> >   drm: Remove drm_modeset_(un)lock_crtc
> >   drm: Remove drm_modeset_legacy_acquire_ctx and crtc->acquire_ctx
> >   drm/atomic-helper: remove modeset_lock_all from helper_resume
> >   drm: drop modeset_lock_all from drm_state_info
> >   drm: Drop modeset_lock_all from the getproperty ioctl
> >   drm: Only take crtc lock in get_gamma ioctl
> >   drm/i915: Nuke intel_atomic_legacy_gamma_set
> >   drm/msm: Nerf zpos property
> >   drm/fb-helper: Give up on kgdb for atomic drivers
> >   drm: Add explicit acquire ctx handling around ->gamma_set
> >   drm: Add acquire ctx to ->gamma_set hook
> >   drm/atomic-helper: Remove legacy backoff hack from gamma_set
> >   drm: extract legacy framebuffer remove
> >   drm/fb-helper: Extract _legacy kms functions
> >
> >  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |   3 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |   3 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |   3 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |   3 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_virtual.c  |   3 +-
> >  drivers/gpu/drm/ast/ast_mode.c            |   3 +-
> >  drivers/gpu/drm/cirrus/cirrus_mode.c      |   3 +-
> >  drivers/gpu/drm/drm_atomic.c              | 162 +++++++-----------------------
> >  drivers/gpu/drm/drm_atomic_helper.c       |  35 +++----
> >  drivers/gpu/drm/drm_color_mgmt.c          |  51 +++++-----
> >  drivers/gpu/drm/drm_crtc_internal.h       |   1 -
> >  drivers/gpu/drm/drm_fb_helper.c           |  81 +++++++++------
> >  drivers/gpu/drm/drm_framebuffer.c         | 137 +++++++++++++++++++++----
> >  drivers/gpu/drm/drm_modeset_lock.c        | 102 -------------------
> >  drivers/gpu/drm/drm_plane.c               |  49 +++++----
> >  drivers/gpu/drm/drm_property.c            |  72 ++++++-------
> >  drivers/gpu/drm/gma500/gma_display.c      |   3 +-
> >  drivers/gpu/drm/gma500/gma_display.h      |   3 +-
> >  drivers/gpu/drm/i915/intel_display.c      |  60 +----------
> >  drivers/gpu/drm/i915/intel_pipe_crc.c     |   2 +-
> >  drivers/gpu/drm/mgag200/mgag200_mode.c    |   3 +-
> >  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c |   2 -
> >  drivers/gpu/drm/nouveau/dispnv04/crtc.c   |   3 +-
> >  drivers/gpu/drm/nouveau/nv50_display.c    |  13 +--
> >  drivers/gpu/drm/radeon/radeon_display.c   |   3 +-
> >  drivers/gpu/drm/vc4/vc4_crtc.c            |   3 +-
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c       |   3 +-
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h       |   3 +-
> >  include/drm/drm_atomic_helper.h           |   3 +-
> >  include/drm/drm_crtc.h                    |  12 +--
> >  include/drm/drm_modeset_lock.h            |   5 -
> >  31 files changed, 344 insertions(+), 488 deletions(-)
> >
> > --
> > 2.11.0
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx