[PATCH 00/16] More drm master and SET_UNIQUE cleanups

[PATCH 00/16] More drm master and SET_UNIQUE cleanups

[Daniel Vetter]

Hi all,

This resends the remaining bits of my drm master cleanup. The simple bugfix for
patch 1 for the leak that Chris spotted resulted in cascading rebase conflicts.
On top of that:
- SET_UNIQUE cleanup, including big documentation section to explain all the
  lessons learned.
- drm master and authentication cleanup plus again documentation, motivated by
  irc discussions with Chris and Emil.

Feedback, testing and review highly welcome, as usual.

Cheers, Daniel

Daniel Vetter (16):
  drm: Only do the hw.lock cleanup in master_relase for !MODESET
  drm: Move authmagic cleanup into drm_master_release
  drm: Protect authmagic with master_mutex
  drm: Mark authmagic ioctls as unlocked
  drm: Mark set/drop master ioctl as unlocked.
  drm: Move master pointer from drm_minor to drm_device
  drm: Clean up drm_crtc.h
  drm: Use dev->name as fallback for dev->unique
  drm/vgem: Stop calling drm_drv_set_unique
  drm: Don't call drm_dev_set_unique from platform drivers
  drm: Nuke SET_UNIQUE ioctl
  drm: Lobotomize set_busid nonsense for !pci drivers
  drm: Refactor drop/set master code a bit
  drm: Extract drm_is_current_master
  drm: Clear up master tracking booleans
  drm: document drm_auth.c

 Documentation/DocBook/gpu.tmpl                  |  10 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c         |   1 -
 drivers/gpu/drm/armada/armada_drv.c             |   1 -
 drivers/gpu/drm/drm_auth.c                      | 181 +++++++++++++----------
 drivers/gpu/drm/drm_bufs.c                      |   8 +-
 drivers/gpu/drm/drm_crtc.c                      |  10 +-
 drivers/gpu/drm/drm_crtc_internal.h             |  86 ++++++++++-
 drivers/gpu/drm/drm_drv.c                       |  46 +++---
 drivers/gpu/drm/drm_fb_helper.c                 |   2 +-
 drivers/gpu/drm/drm_fops.c                      |   6 +-
 drivers/gpu/drm/drm_info.c                      |  12 +-
 drivers/gpu/drm/drm_internal.h                  |   3 -
 drivers/gpu/drm/drm_ioctl.c                     | 125 ++++++++--------
 drivers/gpu/drm/drm_lock.c                      |   4 +-
 drivers/gpu/drm/drm_pci.c                       |  51 -------
 drivers/gpu/drm/drm_platform.c                  |  18 ---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c           |   1 -
 drivers/gpu/drm/exynos/exynos_drm_drv.c         |   1 -
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |   1 -
 drivers/gpu/drm/i915/i915_drv.h                 |   1 +
 drivers/gpu/drm/i915/i915_gem_execbuffer.c      |   4 +-
 drivers/gpu/drm/imx/imx-drm-core.c              |   1 -
 drivers/gpu/drm/mediatek/mtk_drm_drv.c          |   2 -
 drivers/gpu/drm/msm/msm_drv.c                   |   1 -
 drivers/gpu/drm/nouveau/nouveau_drm.c           |   1 -
 drivers/gpu/drm/omapdrm/omap_drv.c              |   2 -
 drivers/gpu/drm/rcar-du/rcar_du_drv.c           |   2 -
 drivers/gpu/drm/shmobile/shmob_drm_drv.c        |   1 -
 drivers/gpu/drm/sis/sis_mm.c                    |   2 +-
 drivers/gpu/drm/sun4i/sun4i_drv.c               |   4 -
 drivers/gpu/drm/tilcdc/tilcdc_drv.c             |   1 -
 drivers/gpu/drm/vgem/vgem_drv.c                 |   2 -
 drivers/gpu/drm/via/via_mm.c                    |   2 +-
 drivers/gpu/drm/virtio/virtgpu_drm_bus.c        |  10 --
 drivers/gpu/drm/virtio/virtgpu_drv.c            |   1 -
 drivers/gpu/drm/virtio/virtgpu_drv.h            |   1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c             |   5 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h             |   1 +
 include/drm/drmP.h                              |  55 ++-----
 include/drm/drm_auth.h                          |  59 ++++++++
 include/drm/drm_crtc.h                          | 188 +++++++-----------------
 include/drm/drm_legacy.h                        |   2 +
 42 files changed, 445 insertions(+), 470 deletions(-)
 create mode 100644 include/drm/drm_auth.h

-- 
2.8.1

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

[PATCH 01/16] drm: Only do the hw.lock cleanup in master_relase for !MODESET

[Daniel Vetter]

Another place gone where modern drivers could have hit
dev->struct_mutex.

To avoid too deeply nesting control flow rework it a bit.

v2: Review from Chris:
- remove spurious newline.
- fix file_priv->master like for the !file_priv->is_master case.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_auth.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index e015a7edb154..54ad64a6d052 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -246,11 +246,13 @@ int drm_master_open(struct drm_file *file_priv)
 void drm_master_release(struct drm_file *file_priv)
 {
 	struct drm_device *dev = file_priv->minor->dev;
+	struct drm_master *master = file_priv->master;
 
 	mutex_lock(&dev->master_mutex);
-	if (file_priv->is_master) {
-		struct drm_master *master = file_priv->master;
+	if (!file_priv->is_master)
+		goto out;
 
+	if (!drm_core_check_feature(dev, DRIVER_MODESET)) {
 		/*
 		 * Since the master is disappearing, so is the
 		 * possibility to lock.
@@ -264,15 +266,15 @@ void drm_master_release(struct drm_file *file_priv)
 			wake_up_interruptible_all(&master->lock.lock_queue);
 		}
 		mutex_unlock(&dev->struct_mutex);
-
-		if (file_priv->minor->master == file_priv->master) {
-			/* drop the reference held my the minor */
-			if (dev->driver->master_drop)
-				dev->driver->master_drop(dev, file_priv, true);
-			drm_master_put(&file_priv->minor->master);
-		}
 	}
 
+	if (file_priv->minor->master == file_priv->master) {
+		/* drop the reference held my the minor */
+		if (dev->driver->master_drop)
+			dev->driver->master_drop(dev, file_priv, true);
+		drm_master_put(&file_priv->minor->master);
+	}
+out:
 	/* drop the master reference held by the file priv */
 	if (file_priv->master)
 		drm_master_put(&file_priv->master);
-- 
2.8.1

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

[PATCH 02/16] drm: Move authmagic cleanup into drm_master_release

[Daniel Vetter]

It's related, and soon authmagic will also use the master_mutex.

There is an ever-so-slightly semantic change here:
- authmagic will only be cleaned up for primary_client drm_minors. But
  it's impossible to create authmagic on render/control nodes, so this
  is fine.
- The cleanup is moved down a bit in the release processing. Doesn't
  matter at all since authmagic is purely internal logic used by the
  core ioctl access checks, and when we're in a file's release
  callback no one can do ioctls any more.

v2: Rebased.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_auth.c | 5 +++++
 drivers/gpu/drm/drm_fops.c | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 54ad64a6d052..24f0f2dc1cce 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -248,6 +248,11 @@ void drm_master_release(struct drm_file *file_priv)
 	struct drm_device *dev = file_priv->minor->dev;
 	struct drm_master *master = file_priv->master;
 
+	mutex_lock(&dev->struct_mutex);
+	if (file_priv->magic)
+		idr_remove(&file_priv->master->magic_map, file_priv->magic);
+	mutex_unlock(&dev->struct_mutex);
+
 	mutex_lock(&dev->master_mutex);
 	if (!file_priv->is_master)
 		goto out;
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index f3b2677de882..f6dfdfcd018b 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -376,11 +376,6 @@ int drm_release(struct inode *inode, struct file *filp)
 	list_del(&file_priv->lhead);
 	mutex_unlock(&dev->filelist_mutex);
 
-	mutex_lock(&dev->struct_mutex);
-	if (file_priv->magic)
-		idr_remove(&file_priv->master->magic_map, file_priv->magic);
-	mutex_unlock(&dev->struct_mutex);
-
 	if (dev->driver->preclose)
 		dev->driver->preclose(dev, file_priv);
 
-- 
2.8.1

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

[PATCH 03/16] drm: Protect authmagic with master_mutex

[Daniel Vetter]

Simplifies cleanup, and there's no reason drivers should ever care
about authmagic at all - it's all handled in the core.

And with that, Ladies and Gentlemen, it's time to pop the champagen
and celebrate: dev->struct_mutex is now officially gone from modern
drivers, and if a driver is using gem_free_object_unlocked and doesn't
do anything else silly it's positively impossible to ever touch
dev->struct_mutex at runtime, anywhere.

Well except for the mutex_init on driver load ;-)

v2: Rebased.

Cc: Chris Wilson <chris@chris-wilson.co.uk> (v1)
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_auth.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 24f0f2dc1cce..d858e69d337b 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -49,7 +49,7 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv)
 	struct drm_auth *auth = data;
 	int ret = 0;
 
-	mutex_lock(&dev->struct_mutex);
+	mutex_lock(&dev->master_mutex);
 	if (!file_priv->magic) {
 		ret = idr_alloc(&file_priv->master->magic_map, file_priv,
 				1, 0, GFP_KERNEL);
@@ -57,7 +57,7 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv)
 			file_priv->magic = ret;
 	}
 	auth->magic = file_priv->magic;
-	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&dev->master_mutex);
 
 	DRM_DEBUG("%u\n", auth->magic);
 
@@ -82,13 +82,13 @@ int drm_authmagic(struct drm_device *dev, void *data,
 
 	DRM_DEBUG("%u\n", auth->magic);
 
-	mutex_lock(&dev->struct_mutex);
+	mutex_lock(&dev->master_mutex);
 	file = idr_find(&file_priv->master->magic_map, auth->magic);
 	if (file) {
 		file->authenticated = 1;
 		idr_replace(&file_priv->master->magic_map, NULL, auth->magic);
 	}
-	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&dev->master_mutex);
 
 	return file ? 0 : -EINVAL;
 }
@@ -117,7 +117,7 @@ static struct drm_master *drm_master_create(struct drm_device *dev)
  * @dev: The associated device.
  * @fpriv: File private identifying the client.
  *
- * This function must be called with dev::struct_mutex held.
+ * This function must be called with dev::master_mutex held.
  * Returns negative error code on failure. Zero on success.
  */
 static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
@@ -248,12 +248,10 @@ void drm_master_release(struct drm_file *file_priv)
 	struct drm_device *dev = file_priv->minor->dev;
 	struct drm_master *master = file_priv->master;
 
-	mutex_lock(&dev->struct_mutex);
+	mutex_lock(&dev->master_mutex);
 	if (file_priv->magic)
 		idr_remove(&file_priv->master->magic_map, file_priv->magic);
-	mutex_unlock(&dev->struct_mutex);
 
-	mutex_lock(&dev->master_mutex);
 	if (!file_priv->is_master)
 		goto out;
 
-- 
2.8.1

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

[PATCH 04/16] drm: Mark authmagic ioctls as unlocked

[Daniel Vetter]

All protected by dev->master_mutex. And there's no driver callbacks,
which means no need to sync with old dri1 horror show drivers at all.
Hence safe to drop the drm legacy BKL from these paths.

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

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index b7a39771c152..e4ccb5c3fdea 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -504,7 +504,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_VERSION, drm_version,
 		      DRM_UNLOCKED|DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_UNIQUE, drm_getunique, 0),
-	DRM_IOCTL_DEF(DRM_IOCTL_GET_MAGIC, drm_getmagic, 0),
+	DRM_IOCTL_DEF(DRM_IOCTL_GET_MAGIC, drm_getmagic, DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_IRQ_BUSID, drm_irq_by_busid, DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_legacy_getmap_ioctl, DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, DRM_UNLOCKED),
@@ -516,7 +516,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_setunique, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_BLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_UNBLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
-	DRM_IOCTL_DEF(DRM_IOCTL_AUTH_MAGIC, drm_authmagic, DRM_AUTH|DRM_MASTER),
+	DRM_IOCTL_DEF(DRM_IOCTL_AUTH_MAGIC, drm_authmagic, DRM_AUTH|DRM_UNLOCKED|DRM_MASTER),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_ADD_MAP, drm_legacy_addmap_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_RM_MAP, drm_legacy_rmmap_ioctl, DRM_AUTH),
-- 
2.8.1

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

[PATCH 05/16] drm: Mark set/drop master ioctl as unlocked.

[Daniel Vetter]

Again this is neatly protected by the dev->master_mutex now. There is
a driver callback both for set and drop, but it's only used by vmwgfx.
And vmwgfx has it's own solid locking for shared resources (besides
dev->master_mutex), hence is all safe. Let's drop another place where
the drm legacy bkl is used.

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

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index e4ccb5c3fdea..11eda9050215 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -524,8 +524,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_legacy_setsareactx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_legacy_getsareactx, DRM_AUTH),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_SET_MASTER, drm_setmaster_ioctl, DRM_ROOT_ONLY),
-	DRM_IOCTL_DEF(DRM_IOCTL_DROP_MASTER, drm_dropmaster_ioctl, DRM_ROOT_ONLY),
+	DRM_IOCTL_DEF(DRM_IOCTL_SET_MASTER, drm_setmaster_ioctl, DRM_UNLOCKED|DRM_ROOT_ONLY),
+	DRM_IOCTL_DEF(DRM_IOCTL_DROP_MASTER, drm_dropmaster_ioctl, DRM_UNLOCKED|DRM_ROOT_ONLY),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_ADD_CTX, drm_legacy_addctx, DRM_AUTH|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_RM_CTX, drm_legacy_rmctx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
-- 
2.8.1

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

[PATCH 06/16] drm: Move master pointer from drm_minor to drm_device

[Daniel Vetter]

There can only be one current master, and it's for the overall device.
Render/control minors don't support master-based auth at all.

This simplifies the master logic a lot, at least in my eyes: All these
additional pointer chases are just confusing.

While doing the conversion I spotted some locking fail:
- drm_lock/drm_auth check dev->master without holding the
  master_mutex. This is fallout from

  commit c996fd0b956450563454e7ccc97a82ca31f9d043
  Author: Thomas Hellstrom <thellstrom@vmware.com>
  Date:   Tue Feb 25 19:57:44 2014 +0100

      drm: Protect the master management with a drm_device::master_mutex v3

  but I honestly don't care one bit about those old legacy drivers
  using this.

- debugfs name info should just grab master_mutex.

- And the fbdev helper looked at it to figure out whether someone is
  using KMS. We just need a consistent value, so READ_ONCE. Aside: We
  should probably check if anyone has opened a control node too, but I
  guess current userspace doesn't really do that yet.

v2: Balance locking, reported by Julia.

Cc: Julia Lawall <julia.lawall@lip6.fr>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_auth.c      | 26 +++++++++++++-------------
 drivers/gpu/drm/drm_bufs.c      |  8 ++++----
 drivers/gpu/drm/drm_fb_helper.c |  2 +-
 drivers/gpu/drm/drm_info.c      | 10 ++++++++--
 drivers/gpu/drm/drm_lock.c      |  2 +-
 drivers/gpu/drm/sis/sis_mm.c    |  2 +-
 drivers/gpu/drm/via/via_mm.c    |  2 +-
 include/drm/drmP.h              |  9 +++++----
 8 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index d858e69d337b..6bba6b9e9dcc 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -128,13 +128,13 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
 	lockdep_assert_held_once(&dev->master_mutex);
 
 	/* create a new master */
-	fpriv->minor->master = drm_master_create(fpriv->minor->dev);
-	if (!fpriv->minor->master)
+	dev->master = drm_master_create(dev);
+	if (!dev->master)
 		return -ENOMEM;
 
 	/* take another reference for the copy in the local file priv */
 	old_master = fpriv->master;
-	fpriv->master = drm_master_get(fpriv->minor->master);
+	fpriv->master = drm_master_get(dev->master);
 
 	if (dev->driver->master_create) {
 		ret = dev->driver->master_create(dev, fpriv->master);
@@ -157,7 +157,7 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
 
 out_err:
 	/* drop both references and restore old master on failure */
-	drm_master_put(&fpriv->minor->master);
+	drm_master_put(&dev->master);
 	drm_master_put(&fpriv->master);
 	fpriv->master = old_master;
 
@@ -173,7 +173,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
 	if (file_priv->is_master)
 		goto out_unlock;
 
-	if (file_priv->minor->master) {
+	if (dev->master) {
 		ret = -EINVAL;
 		goto out_unlock;
 	}
@@ -188,13 +188,13 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
 		goto out_unlock;
 	}
 
-	file_priv->minor->master = drm_master_get(file_priv->master);
+	dev->master = drm_master_get(file_priv->master);
 	file_priv->is_master = 1;
 	if (dev->driver->master_set) {
 		ret = dev->driver->master_set(dev, file_priv, false);
 		if (unlikely(ret != 0)) {
 			file_priv->is_master = 0;
-			drm_master_put(&file_priv->minor->master);
+			drm_master_put(&dev->master);
 		}
 	}
 
@@ -212,13 +212,13 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
 	if (!file_priv->is_master)
 		goto out_unlock;
 
-	if (!file_priv->minor->master)
+	if (!dev->master)
 		goto out_unlock;
 
 	ret = 0;
 	if (dev->driver->master_drop)
 		dev->driver->master_drop(dev, file_priv, false);
-	drm_master_put(&file_priv->minor->master);
+	drm_master_put(&dev->master);
 	file_priv->is_master = 0;
 
 out_unlock:
@@ -234,10 +234,10 @@ int drm_master_open(struct drm_file *file_priv)
 	/* if there is no current master make this fd it, but do not create
 	 * any master object for render clients */
 	mutex_lock(&dev->master_mutex);
-	if (!file_priv->minor->master)
+	if (!dev->master)
 		ret = drm_new_set_master(dev, file_priv);
 	else
-		file_priv->master = drm_master_get(file_priv->minor->master);
+		file_priv->master = drm_master_get(dev->master);
 	mutex_unlock(&dev->master_mutex);
 
 	return ret;
@@ -271,11 +271,11 @@ void drm_master_release(struct drm_file *file_priv)
 		mutex_unlock(&dev->struct_mutex);
 	}
 
-	if (file_priv->minor->master == file_priv->master) {
+	if (dev->master == file_priv->master) {
 		/* drop the reference held my the minor */
 		if (dev->driver->master_drop)
 			dev->driver->master_drop(dev, file_priv, true);
-		drm_master_put(&file_priv->minor->master);
+		drm_master_put(&dev->master);
 	}
 out:
 	/* drop the master reference held by the file priv */
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 9b34158c0f77..c3a12cd8bd0d 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -51,7 +51,7 @@ static struct drm_map_list *drm_find_matching_map(struct drm_device *dev,
 		 */
 		if (!entry->map ||
 		    map->type != entry->map->type ||
-		    entry->master != dev->primary->master)
+		    entry->master != dev->master)
 			continue;
 		switch (map->type) {
 		case _DRM_SHM:
@@ -245,12 +245,12 @@ static int drm_addmap_core(struct drm_device * dev, resource_size_t offset,
 		map->offset = (unsigned long)map->handle;
 		if (map->flags & _DRM_CONTAINS_LOCK) {
 			/* Prevent a 2nd X Server from creating a 2nd lock */
-			if (dev->primary->master->lock.hw_lock != NULL) {
+			if (dev->master->lock.hw_lock != NULL) {
 				vfree(map->handle);
 				kfree(map);
 				return -EBUSY;
 			}
-			dev->sigdata.lock = dev->primary->master->lock.hw_lock = map->handle;	/* Pointer to lock */
+			dev->sigdata.lock = dev->master->lock.hw_lock = map->handle;	/* Pointer to lock */
 		}
 		break;
 	case _DRM_AGP: {
@@ -356,7 +356,7 @@ static int drm_addmap_core(struct drm_device * dev, resource_size_t offset,
 	mutex_unlock(&dev->struct_mutex);
 
 	if (!(map->flags & _DRM_DRIVER))
-		list->master = dev->primary->master;
+		list->master = dev->master;
 	*maplist = list;
 	return 0;
 }
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0bac5246e5a7..0a06f9120b5a 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -464,7 +464,7 @@ static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
 
 	/* Sometimes user space wants everything disabled, so don't steal the
 	 * display if there's a master. */
-	if (dev->primary->master)
+	if (READ_ONCE(dev->master))
 		return false;
 
 	drm_for_each_crtc(crtc, dev) {
diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
index 0090d5987801..24c80dd13837 100644
--- a/drivers/gpu/drm/drm_info.c
+++ b/drivers/gpu/drm/drm_info.c
@@ -50,9 +50,12 @@ int drm_name_info(struct seq_file *m, void *data)
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
 	struct drm_minor *minor = node->minor;
 	struct drm_device *dev = minor->dev;
-	struct drm_master *master = minor->master;
+	struct drm_master *master;
+
+	mutex_lock(&dev->master_mutex);
+	master = dev->master;
 	if (!master)
-		return 0;
+		goto out_unlock;
 
 	if (master->unique) {
 		seq_printf(m, "%s %s %s\n",
@@ -62,6 +65,9 @@ int drm_name_info(struct seq_file *m, void *data)
 		seq_printf(m, "%s %s\n",
 			   dev->driver->name, dev_name(dev->dev));
 	}
+out_unlock:
+	mutex_unlock(&dev->master_mutex);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
index 0fb8f9e73486..ae0a4d39deec 100644
--- a/drivers/gpu/drm/drm_lock.c
+++ b/drivers/gpu/drm/drm_lock.c
@@ -334,7 +334,7 @@ void drm_legacy_lock_release(struct drm_device *dev, struct file *filp)
 	struct drm_file *file_priv = filp->private_data;
 
 	/* if the master has gone away we can't do anything with the lock */
-	if (!file_priv->minor->master)
+	if (!dev->master)
 		return;
 
 	if (drm_legacy_i_have_hw_lock(dev, file_priv)) {
diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c
index 93ad8a5704d1..03defda77766 100644
--- a/drivers/gpu/drm/sis/sis_mm.c
+++ b/drivers/gpu/drm/sis/sis_mm.c
@@ -316,7 +316,7 @@ void sis_reclaim_buffers_locked(struct drm_device *dev,
 	struct sis_file_private *file_priv = file->driver_priv;
 	struct sis_memblock *entry, *next;
 
-	if (!(file->minor->master && file->master->lock.hw_lock))
+	if (!(dev->master && file->master->lock.hw_lock))
 		return;
 
 	drm_legacy_idlelock_take(&file->master->lock);
diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c
index 4f20742e7788..a04ef1c992d9 100644
--- a/drivers/gpu/drm/via/via_mm.c
+++ b/drivers/gpu/drm/via/via_mm.c
@@ -208,7 +208,7 @@ void via_reclaim_buffers_locked(struct drm_device *dev,
 	struct via_file_private *file_priv = file->driver_priv;
 	struct via_memblock *entry, *next;
 
-	if (!(file->minor->master && file->master->lock.hw_lock))
+	if (!(dev->master && file->master->lock.hw_lock))
 		return;
 
 	drm_legacy_idlelock_take(&file->master->lock);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index e8788d7b29dc..62e0e70cb5a2 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -336,7 +336,7 @@ struct drm_file {
 	void *driver_priv;
 
 	struct drm_master *master; /* master this node is currently associated with
-				      N.B. not always minor->master */
+				      N.B. not always dev->master */
 	/**
 	 * fbs - List of framebuffers associated with this file.
 	 *
@@ -708,9 +708,6 @@ struct drm_minor {
 
 	struct list_head debugfs_list;
 	struct mutex debugfs_lock; /* Protects debugfs_list. */
-
-	/* currently active master for this node. Protected by master_mutex */
-	struct drm_master *master;
 };
 
 
@@ -759,6 +756,10 @@ struct drm_device {
 	struct drm_minor *control;		/**< Control node */
 	struct drm_minor *primary;		/**< Primary node */
 	struct drm_minor *render;		/**< Render node */
+
+	/* currently active master for this device. Protected by master_mutex */
+	struct drm_master *master;
+
 	atomic_t unplugged;			/**< Flag whether dev is dead */
 	struct inode *anon_inode;		/**< inode for private address-space */
 	char *unique;				/**< unique name of the device */
-- 
2.8.1

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

[PATCH 07/16] drm: Clean up drm_crtc.h

[Daniel Vetter]

- Group declarations for separate files (drm_bridge.c, drm_edid.c)
- Move declarations only used within drm.ko to drm_crtc_internal.h
- drm_property_type_valid to drm_crtc.c, its only callsite

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_crtc.c          |   7 ++
 drivers/gpu/drm/drm_crtc_internal.h |  86 ++++++++++++++++-
 drivers/gpu/drm/drm_drv.c           |   1 +
 drivers/gpu/drm/drm_fops.c          |   1 +
 include/drm/drm_crtc.h              | 188 +++++++++++-------------------------
 5 files changed, 150 insertions(+), 133 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 4ec35f9e6de5..4ff818ad34d7 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3656,6 +3656,13 @@ void drm_fb_release(struct drm_file *priv)
 	}
 }
 
+static bool drm_property_type_valid(struct drm_property *property)
+{
+	if (property->flags & DRM_MODE_PROP_EXTENDED_TYPE)
+		return !(property->flags & DRM_MODE_PROP_LEGACY_TYPE);
+	return !!(property->flags & DRM_MODE_PROP_LEGACY_TYPE);
+}
+
 /**
  * drm_property_create - create a new property type
  * @dev: drm device
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index a78c138282ea..8186c0e05c42 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -31,14 +31,98 @@
  * and are not exported to drivers.
  */
 
+
+/* drm_crtc.c */
+void drm_connector_ida_init(void);
+void drm_connector_ida_destroy(void);
 int drm_mode_object_get(struct drm_device *dev,
 			struct drm_mode_object *obj, uint32_t obj_type);
 void drm_mode_object_unregister(struct drm_device *dev,
 				struct drm_mode_object *object);
+bool drm_property_change_valid_get(struct drm_property *property,
+				   uint64_t value,
+				   struct drm_mode_object **ref);
+void drm_property_change_valid_put(struct drm_property *property,
+				   struct drm_mode_object *ref);
+
+int drm_plane_check_pixel_format(const struct drm_plane *plane,
+				 u32 format);
+int drm_crtc_check_viewport(const struct drm_crtc *crtc,
+			    int x, int y,
+			    const struct drm_display_mode *mode,
+			    const struct drm_framebuffer *fb);
+
+void drm_fb_release(struct drm_file *file_priv);
+void drm_property_destroy_user_blobs(struct drm_device *dev,
+				     struct drm_file *file_priv);
+
+/* dumb buffer support IOCTLs */
+int drm_mode_create_dumb_ioctl(struct drm_device *dev,
+			       void *data, struct drm_file *file_priv);
+int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
+			     void *data, struct drm_file *file_priv);
+int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
+				void *data, struct drm_file *file_priv);
+
+/* framebuffer IOCTLs */
+extern int drm_mode_addfb(struct drm_device *dev,
+			  void *data, struct drm_file *file_priv);
+extern int drm_mode_addfb2(struct drm_device *dev,
+			   void *data, struct drm_file *file_priv);
+int drm_mode_rmfb(struct drm_device *dev,
+			 void *data, struct drm_file *file_priv);
+int drm_mode_getfb(struct drm_device *dev,
+		   void *data, struct drm_file *file_priv);
+int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
+			   void *data, struct drm_file *file_priv);
+
+/* IOCTLs */
+int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
+				      struct drm_file *file_priv);
+int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
+				    struct drm_file *file_priv);
+
+int drm_mode_getresources(struct drm_device *dev,
+			  void *data, struct drm_file *file_priv);
+int drm_mode_getplane_res(struct drm_device *dev, void *data,
+			  struct drm_file *file_priv);
+int drm_mode_getcrtc(struct drm_device *dev,
+		     void *data, struct drm_file *file_priv);
+int drm_mode_getconnector(struct drm_device *dev,
+			  void *data, struct drm_file *file_priv);
+int drm_mode_setcrtc(struct drm_device *dev,
+		     void *data, struct drm_file *file_priv);
+int drm_mode_getplane(struct drm_device *dev,
+		      void *data, struct drm_file *file_priv);
+int drm_mode_setplane(struct drm_device *dev,
+		      void *data, struct drm_file *file_priv);
+int drm_mode_cursor_ioctl(struct drm_device *dev,
+			  void *data, struct drm_file *file_priv);
+int drm_mode_cursor2_ioctl(struct drm_device *dev,
+			   void *data, struct drm_file *file_priv);
+int drm_mode_getproperty_ioctl(struct drm_device *dev,
+			       void *data, struct drm_file *file_priv);
+int drm_mode_getblob_ioctl(struct drm_device *dev,
+			   void *data, struct drm_file *file_priv);
+int drm_mode_createblob_ioctl(struct drm_device *dev,
+			      void *data, struct drm_file *file_priv);
+int drm_mode_destroyblob_ioctl(struct drm_device *dev,
+			       void *data, struct drm_file *file_priv);
+int drm_mode_connector_property_set_ioctl(struct drm_device *dev,
+					  void *data, struct drm_file *file_priv);
+int drm_mode_getencoder(struct drm_device *dev,
+			void *data, struct drm_file *file_priv);
+int drm_mode_gamma_get_ioctl(struct drm_device *dev,
+			     void *data, struct drm_file *file_priv);
+int drm_mode_gamma_set_ioctl(struct drm_device *dev,
+			     void *data, struct drm_file *file_priv);
+
+int drm_mode_page_flip_ioctl(struct drm_device *dev,
+			     void *data, struct drm_file *file_priv);
 
 /* drm_atomic.c */
 int drm_atomic_get_property(struct drm_mode_object *obj,
-			   struct drm_property *property, uint64_t *val);
+			    struct drm_property *property, uint64_t *val);
 int drm_mode_atomic_ioctl(struct drm_device *dev,
 			  void *data, struct drm_file *file_priv);
 
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 8e67b4f4573e..10afa2539181 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -36,6 +36,7 @@
 #include <drm/drm_core.h>
 #include "drm_legacy.h"
 #include "drm_internal.h"
+#include "drm_crtc_internal.h"
 
 /*
  * drm_debug: Enable debug output.
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index f6dfdfcd018b..323c238fcac7 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -40,6 +40,7 @@
 #include <linux/module.h>
 #include "drm_legacy.h"
 #include "drm_internal.h"
+#include "drm_crtc_internal.h"
 
 /* from BKL pushdown */
 DEFINE_MUTEX(drm_global_mutex);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 914baa8c161d..c1177eb534b9 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -44,6 +44,7 @@ struct drm_file;
 struct drm_clip_rect;
 struct device_node;
 struct fence;
+struct edid;
 
 struct drm_mode_object {
 	uint32_t id;
@@ -2467,12 +2468,10 @@ static inline uint32_t drm_crtc_mask(struct drm_crtc *crtc)
 	return 1 << drm_crtc_index(crtc);
 }
 
-extern void drm_connector_ida_init(void);
-extern void drm_connector_ida_destroy(void);
-extern int drm_connector_init(struct drm_device *dev,
-			      struct drm_connector *connector,
-			      const struct drm_connector_funcs *funcs,
-			      int connector_type);
+int drm_connector_init(struct drm_device *dev,
+		       struct drm_connector *connector,
+		       const struct drm_connector_funcs *funcs,
+		       int connector_type);
 int drm_connector_register(struct drm_connector *connector);
 void drm_connector_unregister(struct drm_connector *connector);
 
@@ -2486,22 +2485,6 @@ static inline unsigned drm_connector_index(struct drm_connector *connector)
 extern int drm_connector_register_all(struct drm_device *dev);
 extern void drm_connector_unregister_all(struct drm_device *dev);
 
-extern int drm_bridge_add(struct drm_bridge *bridge);
-extern void drm_bridge_remove(struct drm_bridge *bridge);
-extern struct drm_bridge *of_drm_find_bridge(struct device_node *np);
-extern int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge);
-
-bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
-			const struct drm_display_mode *mode,
-			struct drm_display_mode *adjusted_mode);
-void drm_bridge_disable(struct drm_bridge *bridge);
-void drm_bridge_post_disable(struct drm_bridge *bridge);
-void drm_bridge_mode_set(struct drm_bridge *bridge,
-			struct drm_display_mode *mode,
-			struct drm_display_mode *adjusted_mode);
-void drm_bridge_pre_enable(struct drm_bridge *bridge);
-void drm_bridge_enable(struct drm_bridge *bridge);
-
 extern __printf(5, 6)
 int drm_encoder_init(struct drm_device *dev,
 		     struct drm_encoder *encoder,
@@ -2563,14 +2546,8 @@ static inline unsigned int drm_plane_index(struct drm_plane *plane)
 }
 extern struct drm_plane * drm_plane_from_index(struct drm_device *dev, int idx);
 extern void drm_plane_force_disable(struct drm_plane *plane);
-extern int drm_plane_check_pixel_format(const struct drm_plane *plane,
-					u32 format);
 extern void drm_crtc_get_hv_timing(const struct drm_display_mode *mode,
 				   int *hdisplay, int *vdisplay);
-extern int drm_crtc_check_viewport(const struct drm_crtc *crtc,
-				   int x, int y,
-				   const struct drm_display_mode *mode,
-				   const struct drm_framebuffer *fb);
 
 extern void drm_encoder_cleanup(struct drm_encoder *encoder);
 
@@ -2581,16 +2558,6 @@ extern const char *drm_get_dvi_i_subconnector_name(int val);
 extern const char *drm_get_dvi_i_select_name(int val);
 extern const char *drm_get_tv_subconnector_name(int val);
 extern const char *drm_get_tv_select_name(int val);
-extern void drm_fb_release(struct drm_file *file_priv);
-extern void drm_property_destroy_user_blobs(struct drm_device *dev,
-                                            struct drm_file *file_priv);
-extern bool drm_probe_ddc(struct i2c_adapter *adapter);
-extern struct edid *drm_get_edid(struct drm_connector *connector,
-				 struct i2c_adapter *adapter);
-extern struct edid *drm_get_edid_switcheroo(struct drm_connector *connector,
-					    struct i2c_adapter *adapter);
-extern struct edid *drm_edid_duplicate(const struct edid *edid);
-extern int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid);
 extern void drm_mode_config_init(struct drm_device *dev);
 extern void drm_mode_config_reset(struct drm_device *dev);
 extern void drm_mode_config_cleanup(struct drm_device *dev);
@@ -2614,13 +2581,6 @@ static inline bool drm_property_type_is(struct drm_property *property,
 	return property->flags & type;
 }
 
-static inline bool drm_property_type_valid(struct drm_property *property)
-{
-	if (property->flags & DRM_MODE_PROP_EXTENDED_TYPE)
-		return !(property->flags & DRM_MODE_PROP_LEGACY_TYPE);
-	return !!(property->flags & DRM_MODE_PROP_LEGACY_TYPE);
-}
-
 extern int drm_object_property_set_value(struct drm_mode_object *obj,
 					 struct drm_property *property,
 					 uint64_t val);
@@ -2678,86 +2638,15 @@ extern int drm_mode_create_scaling_mode_property(struct drm_device *dev);
 extern int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
 extern int drm_mode_create_dirty_info_property(struct drm_device *dev);
 extern int drm_mode_create_suggested_offset_properties(struct drm_device *dev);
-extern bool drm_property_change_valid_get(struct drm_property *property,
-					 uint64_t value, struct drm_mode_object **ref);
-extern void drm_property_change_valid_put(struct drm_property *property,
-		struct drm_mode_object *ref);
 
 extern int drm_mode_connector_attach_encoder(struct drm_connector *connector,
 					     struct drm_encoder *encoder);
 extern int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
 					 int gamma_size);
-extern struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
-		uint32_t id, uint32_t type);
-void drm_mode_object_reference(struct drm_mode_object *obj);
-void drm_mode_object_unreference(struct drm_mode_object *obj);
 
-/* IOCTLs */
-extern int drm_mode_getresources(struct drm_device *dev,
-				 void *data, struct drm_file *file_priv);
-extern int drm_mode_getplane_res(struct drm_device *dev, void *data,
-				   struct drm_file *file_priv);
-extern int drm_mode_getcrtc(struct drm_device *dev,
-			    void *data, struct drm_file *file_priv);
-extern int drm_mode_getconnector(struct drm_device *dev,
-			      void *data, struct drm_file *file_priv);
 extern int drm_mode_set_config_internal(struct drm_mode_set *set);
-extern int drm_mode_setcrtc(struct drm_device *dev,
-			    void *data, struct drm_file *file_priv);
-extern int drm_mode_getplane(struct drm_device *dev,
-			       void *data, struct drm_file *file_priv);
-extern int drm_mode_setplane(struct drm_device *dev,
-			       void *data, struct drm_file *file_priv);
-extern int drm_mode_cursor_ioctl(struct drm_device *dev,
-				void *data, struct drm_file *file_priv);
-extern int drm_mode_cursor2_ioctl(struct drm_device *dev,
-				void *data, struct drm_file *file_priv);
-extern int drm_mode_addfb(struct drm_device *dev,
-			  void *data, struct drm_file *file_priv);
-extern int drm_mode_addfb2(struct drm_device *dev,
-			   void *data, struct drm_file *file_priv);
+
 extern uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
-extern int drm_mode_rmfb(struct drm_device *dev,
-			 void *data, struct drm_file *file_priv);
-extern int drm_mode_getfb(struct drm_device *dev,
-			  void *data, struct drm_file *file_priv);
-extern int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
-				  void *data, struct drm_file *file_priv);
-
-extern int drm_mode_getproperty_ioctl(struct drm_device *dev,
-				      void *data, struct drm_file *file_priv);
-extern int drm_mode_getblob_ioctl(struct drm_device *dev,
-				  void *data, struct drm_file *file_priv);
-extern int drm_mode_createblob_ioctl(struct drm_device *dev,
-				     void *data, struct drm_file *file_priv);
-extern int drm_mode_destroyblob_ioctl(struct drm_device *dev,
-				      void *data, struct drm_file *file_priv);
-extern int drm_mode_connector_property_set_ioctl(struct drm_device *dev,
-					      void *data, struct drm_file *file_priv);
-extern int drm_mode_getencoder(struct drm_device *dev,
-			       void *data, struct drm_file *file_priv);
-extern int drm_mode_gamma_get_ioctl(struct drm_device *dev,
-				    void *data, struct drm_file *file_priv);
-extern int drm_mode_gamma_set_ioctl(struct drm_device *dev,
-				    void *data, struct drm_file *file_priv);
-extern u8 drm_match_cea_mode(const struct drm_display_mode *to_match);
-extern enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
-extern bool drm_detect_hdmi_monitor(struct edid *edid);
-extern bool drm_detect_monitor_audio(struct edid *edid);
-extern bool drm_rgb_quant_range_selectable(struct edid *edid);
-extern int drm_mode_page_flip_ioctl(struct drm_device *dev,
-				    void *data, struct drm_file *file_priv);
-extern int drm_add_modes_noedid(struct drm_connector *connector,
-				int hdisplay, int vdisplay);
-extern void drm_set_preferred_mode(struct drm_connector *connector,
-				   int hpref, int vpref);
-
-extern int drm_edid_header_is_valid(const u8 *raw_edid);
-extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
-				 bool *edid_corrupt);
-extern bool drm_edid_is_valid(struct edid *edid);
-extern void drm_edid_get_monitor_name(struct edid *edid, char *name,
-				      int buflen);
 
 extern struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
 							 char topology[8]);
@@ -2765,25 +2654,10 @@ extern struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev,
 					       char topology[8]);
 extern void drm_mode_put_tile_group(struct drm_device *dev,
 				   struct drm_tile_group *tg);
-struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
-					   int hsize, int vsize, int fresh,
-					   bool rb);
 
-extern int drm_mode_create_dumb_ioctl(struct drm_device *dev,
-				      void *data, struct drm_file *file_priv);
-extern int drm_mode_mmap_dumb_ioctl(struct drm_device *dev,
-				    void *data, struct drm_file *file_priv);
-extern int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
-				      void *data, struct drm_file *file_priv);
-extern int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
-					     struct drm_file *file_priv);
-extern int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
-					   struct drm_file *file_priv);
 extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
 				       struct drm_property *property,
 				       uint64_t value);
-extern int drm_mode_atomic_ioctl(struct drm_device *dev,
-				 void *data, struct drm_file *file_priv);
 
 extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,
 							      unsigned int supported_rotations);
@@ -2794,6 +2668,10 @@ extern void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 				       bool has_ctm,
 				       uint gamma_lut_size);
 /* Helpers */
+struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
+					     uint32_t id, uint32_t type);
+void drm_mode_object_reference(struct drm_mode_object *obj);
+void drm_mode_object_unreference(struct drm_mode_object *obj);
 
 static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
 		uint32_t id)
@@ -2959,4 +2837,50 @@ assert_drm_connector_list_read_locked(struct drm_mode_config *mode_config)
 	     &fb->head != (&(dev)->mode_config.fb_list);			\
 	     fb = list_next_entry(fb, head))
 
+/* drm_edid.c */
+bool drm_probe_ddc(struct i2c_adapter *adapter);
+struct edid *drm_get_edid(struct drm_connector *connector,
+			  struct i2c_adapter *adapter);
+struct edid *drm_get_edid_switcheroo(struct drm_connector *connector,
+				     struct i2c_adapter *adapter);
+struct edid *drm_edid_duplicate(const struct edid *edid);
+int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid);
+
+u8 drm_match_cea_mode(const struct drm_display_mode *to_match);
+enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
+bool drm_detect_hdmi_monitor(struct edid *edid);
+bool drm_detect_monitor_audio(struct edid *edid);
+bool drm_rgb_quant_range_selectable(struct edid *edid);
+int drm_add_modes_noedid(struct drm_connector *connector,
+			 int hdisplay, int vdisplay);
+void drm_set_preferred_mode(struct drm_connector *connector,
+			    int hpref, int vpref);
+
+int drm_edid_header_is_valid(const u8 *raw_edid);
+bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
+			  bool *edid_corrupt);
+bool drm_edid_is_valid(struct edid *edid);
+void drm_edid_get_monitor_name(struct edid *edid, char *name,
+			       int buflen);
+struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
+					   int hsize, int vsize, int fresh,
+					   bool rb);
+
+/* drm_bridge.c */
+extern int drm_bridge_add(struct drm_bridge *bridge);
+extern void drm_bridge_remove(struct drm_bridge *bridge);
+extern struct drm_bridge *of_drm_find_bridge(struct device_node *np);
+extern int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge);
+
+bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
+			const struct drm_display_mode *mode,
+			struct drm_display_mode *adjusted_mode);
+void drm_bridge_disable(struct drm_bridge *bridge);
+void drm_bridge_post_disable(struct drm_bridge *bridge);
+void drm_bridge_mode_set(struct drm_bridge *bridge,
+			struct drm_display_mode *mode,
+			struct drm_display_mode *adjusted_mode);
+void drm_bridge_pre_enable(struct drm_bridge *bridge);
+void drm_bridge_enable(struct drm_bridge *bridge);
+
 #endif /* __DRM_CRTC_H__ */
-- 
2.8.1

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

[PATCH 08/16] drm: Use dev->name as fallback for dev->unique

[Daniel Vetter]

Lots of arm drivers get this wrong and for most arm boards this is the
right thing actually. And anyway with most loaders you want to chase
sysfs links anyway to figure out which dri device you want.

This will fix dmesg noise for rockchip and sti.

Also add a fallback to driver->name for entirely virtual drivers like
vgem.

v2: Rebase on top of

commit e112e593b215c394c0303dbf0534db0928e87967
Author: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
Date:   Fri Dec 11 11:20:28 2015 +0100

    drm: use dev_name as default unique name in drm_dev_alloc()

and simplify a bit. Plus add a comment.

Cc: Ilia Mirkin <imirkin@alum.mit.edu>
Reported-by: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c   | 10 +++++-----
 drivers/gpu/drm/drm_ioctl.c |  8 +-------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 10afa2539181..ecba2511ef5a 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -523,11 +523,11 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver,
 		}
 	}
 
-	if (parent) {
-		ret = drm_dev_set_unique(dev, dev_name(parent));
-		if (ret)
-			goto err_setunique;
-	}
+	/* Use the parent device name as DRM device unique identifier, but fall
+	 * back to the driver name for virtual devices like vgem. */
+	ret = drm_dev_set_unique(dev, parent ? dev_name(parent) : driver->name);
+	if (ret)
+		goto err_setunique;
 
 	return dev;
 
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 11eda9050215..83892b6b1e0d 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -134,13 +134,7 @@ static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv)
 			drm_unset_busid(dev, master);
 			return ret;
 		}
-	} else {
-		if (WARN(dev->unique == NULL,
-			 "No drm_driver.set_busid() implementation provided by "
-			 "%ps. Use drm_dev_set_unique() to set the unique "
-			 "name explicitly.", dev->driver))
-			return -EINVAL;
-
+	} else if (dev->unique) {
 		master->unique = kstrdup(dev->unique, GFP_KERNEL);
 		if (master->unique)
 			master->unique_len = strlen(dev->unique);
-- 
2.8.1

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

[PATCH 09/16] drm/vgem: Stop calling drm_drv_set_unique

[Daniel Vetter]

With the previous patch this is now redudant, the core always
sets a reasonable dev->unique string.

Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/vgem/vgem_drv.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 1b4cc8b27080..35ea5d02a827 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -260,8 +260,6 @@ static int __init vgem_init(void)
 		goto out;
 	}
 
-	drm_dev_set_unique(vgem_device, "vgem");
-
 	ret  = drm_dev_register(vgem_device, 0);
 
 	if (ret)
-- 
2.8.1

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

[PATCH 10/16] drm: Don't call drm_dev_set_unique from platform drivers

[Daniel Vetter]

Since

commit e112e593b215c394c0303dbf0534db0928e87967
Author: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
Date:   Fri Dec 11 11:20:28 2015 +0100

    drm: use dev_name as default unique name in drm_dev_alloc()

we're using a reasonable default which should work for everyone. Only
mtk, rcar-du and sun4i are affected, and as kms-only drivers without
any rendering support no one should ever care about the unique name

v2: Rebase on top of mediatek.

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c              | 35 +++++++++++-----------------------
 drivers/gpu/drm/mediatek/mtk_drm_drv.c |  2 --
 drivers/gpu/drm/rcar-du/rcar_du_drv.c  |  2 --
 drivers/gpu/drm/sun4i/sun4i_drv.c      |  4 ----
 include/drm/drmP.h                     |  1 -
 5 files changed, 11 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index ecba2511ef5a..25f10586a74d 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -298,10 +298,9 @@ void drm_minor_release(struct drm_minor *minor)
  * callbacks implemented by the driver. The driver then needs to initialize all
  * the various subsystems for the drm device like memory management, vblank
  * handling, modesetting support and intial output configuration plus obviously
- * initialize all the corresponding hardware bits. An important part of this is
- * also calling drm_dev_set_unique() to set the userspace-visible unique name of
- * this device instance. Finally when everything is up and running and ready for
- * userspace the device instance can be published using drm_dev_register().
+ * initialize all the corresponding hardware bits. Finally when everything is up
+ * and running and ready for userspace the device instance can be published
+ * using drm_dev_register().
  *
  * There is also deprecated support for initalizing device instances using
  * bus-specific helpers and the ->load() callback. But due to
@@ -323,6 +322,14 @@ void drm_minor_release(struct drm_minor *minor)
  * dev_priv field of &drm_device.
  */
 
+static int drm_dev_set_unique(struct drm_device *dev, const char *name)
+{
+	kfree(dev->unique);
+	dev->unique = kstrdup(name, GFP_KERNEL);
+
+	return dev->unique ? 0 : -ENOMEM;
+}
+
 /**
  * drm_put_dev - Unregister and release a DRM device
  * @dev: DRM device
@@ -697,26 +704,6 @@ void drm_dev_unregister(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_dev_unregister);
 
-/**
- * drm_dev_set_unique - Set the unique name of a DRM device
- * @dev: device of which to set the unique name
- * @name: unique name
- *
- * Sets the unique name of a DRM device using the specified string. Drivers
- * can use this at driver probe time if the unique name of the devices they
- * drive is static.
- *
- * Return: 0 on success or a negative error code on failure.
- */
-int drm_dev_set_unique(struct drm_device *dev, const char *name)
-{
-	kfree(dev->unique);
-	dev->unique = kstrdup(name, GFP_KERNEL);
-
-	return dev->unique ? 0 : -ENOMEM;
-}
-EXPORT_SYMBOL(drm_dev_set_unique);
-
 /*
  * DRM Core
  * The DRM core module initializes all global DRM objects and makes them
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index c33bf98c5d5e..04e901a80234 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -280,8 +280,6 @@ static int mtk_drm_bind(struct device *dev)
 	if (!drm)
 		return -ENOMEM;
 
-	drm_dev_set_unique(drm, dev_name(dev));
-
 	drm->dev_private = private;
 	private->drm = drm;
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 48ec4b6e8b26..8784208d8eed 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -320,8 +320,6 @@ static int rcar_du_probe(struct platform_device *pdev)
 	if (!ddev)
 		return -ENOMEM;
 
-	drm_dev_set_unique(ddev, dev_name(&pdev->dev));
-
 	rcdu->ddev = ddev;
 	ddev->dev_private = rcdu;
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 68e9d85085fb..5c4b4ad17ad3 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -135,10 +135,6 @@ static int sun4i_drv_bind(struct device *dev)
 	if (!drm)
 		return -ENOMEM;
 
-	ret = drm_dev_set_unique(drm, dev_name(drm->dev));
-	if (ret)
-		goto free_drm;
-
 	drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
 	if (!drv) {
 		ret = -ENOMEM;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 62e0e70cb5a2..bd40d7ee5ec5 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1073,7 +1073,6 @@ void drm_dev_ref(struct drm_device *dev);
 void drm_dev_unref(struct drm_device *dev);
 int drm_dev_register(struct drm_device *dev, unsigned long flags);
 void drm_dev_unregister(struct drm_device *dev);
-int drm_dev_set_unique(struct drm_device *dev, const char *name);
 
 struct drm_minor *drm_minor_acquire(unsigned int minor_id);
 void drm_minor_release(struct drm_minor *minor);
-- 
2.8.1

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

[PATCH 11/16] drm: Nuke SET_UNIQUE ioctl

[Daniel Vetter]

Ever since

commit 2e1868b560315a8b20d688e646c489a5ad93eeae
Author: Eric Anholt <anholt@freebsd.org>
Date:   Wed Jun 16 09:25:21 2004 +0000

    DRI trunk-20040613 import

the X server supports drm 1.1, and doesn't even call this ioctl any
more. When reviewing this note that for hilarity both the
kernel-internal functiosn (set_busid) and the libdrm wrapper
(drmSetBusid) have names not matching this ioctl (SET_UNIQUE).

Cc: Emil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_internal.h |  3 ---
 drivers/gpu/drm/drm_ioctl.c    | 47 +-------------------------------------
 drivers/gpu/drm/drm_pci.c      | 51 ------------------------------------------
 3 files changed, 1 insertion(+), 100 deletions(-)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 38401d406532..b86dc9b921a5 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -29,9 +29,6 @@ extern struct mutex drm_global_mutex;
 void drm_lastclose(struct drm_device *dev);
 
 /* drm_pci.c */
-int drm_pci_set_unique(struct drm_device *dev,
-		       struct drm_master *master,
-		       struct drm_unique *u);
 int drm_irq_by_busid(struct drm_device *dev, void *data,
 		     struct drm_file *file_priv);
 
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 83892b6b1e0d..a57a5ea3ebf2 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -75,51 +75,6 @@ drm_unset_busid(struct drm_device *dev,
 	master->unique_len = 0;
 }
 
-/*
- * Set the bus id.
- *
- * \param inode device inode.
- * \param file_priv DRM file private.
- * \param cmd command.
- * \param arg user argument, pointing to a drm_unique structure.
- * \return zero on success or a negative number on failure.
- *
- * Copies the bus id from userspace into drm_device::unique, and verifies that
- * it matches the device this DRM is attached to (EINVAL otherwise).  Deprecated
- * in interface version 1.1 and will return EBUSY when setversion has requested
- * version 1.1 or greater. Also note that KMS is all version 1.1 and later and
- * UMS was only ever supported on pci devices.
- */
-static int drm_setunique(struct drm_device *dev, void *data,
-		  struct drm_file *file_priv)
-{
-	struct drm_unique *u = data;
-	struct drm_master *master = file_priv->master;
-	int ret;
-
-	if (master->unique_len || master->unique)
-		return -EBUSY;
-
-	if (!u->unique_len || u->unique_len > 1024)
-		return -EINVAL;
-
-	if (drm_core_check_feature(dev, DRIVER_MODESET))
-		return 0;
-
-	if (WARN_ON(!dev->pdev))
-		return -EINVAL;
-
-	ret = drm_pci_set_unique(dev, master, u);
-	if (ret)
-		goto err;
-
-	return 0;
-
-err:
-	drm_unset_busid(dev, master);
-	return ret;
-}
-
 static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv)
 {
 	struct drm_master *master = file_priv->master;
@@ -507,7 +462,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_CLIENT_CAP, drm_setclientcap, 0),
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_setunique, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
+	DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_invalid_op, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_BLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_UNBLOCK, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF(DRM_IOCTL_AUTH_MAGIC, drm_authmagic, DRM_AUTH|DRM_UNLOCKED|DRM_MASTER),
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 29d5a548d07a..b2f8f1062d5f 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -144,50 +144,6 @@ int drm_pci_set_busid(struct drm_device *dev, struct drm_master *master)
 }
 EXPORT_SYMBOL(drm_pci_set_busid);
 
-int drm_pci_set_unique(struct drm_device *dev,
-		       struct drm_master *master,
-		       struct drm_unique *u)
-{
-	int domain, bus, slot, func, ret;
-
-	master->unique_len = u->unique_len;
-	master->unique = kmalloc(master->unique_len + 1, GFP_KERNEL);
-	if (!master->unique) {
-		ret = -ENOMEM;
-		goto err;
-	}
-
-	if (copy_from_user(master->unique, u->unique, master->unique_len)) {
-		ret = -EFAULT;
-		goto err;
-	}
-
-	master->unique[master->unique_len] = '\0';
-
-	/* Return error if the busid submitted doesn't match the device's actual
-	 * busid.
-	 */
-	ret = sscanf(master->unique, "PCI:%d:%d:%d", &bus, &slot, &func);
-	if (ret != 3) {
-		ret = -EINVAL;
-		goto err;
-	}
-
-	domain = bus >> 8;
-	bus &= 0xff;
-
-	if ((domain != drm_get_pci_domain(dev)) ||
-	    (bus != dev->pdev->bus->number) ||
-	    (slot != PCI_SLOT(dev->pdev->devfn)) ||
-	    (func != PCI_FUNC(dev->pdev->devfn))) {
-		ret = -EINVAL;
-		goto err;
-	}
-	return 0;
-err:
-	return ret;
-}
-
 static int drm_pci_irq_by_busid(struct drm_device *dev, struct drm_irq_busid *p)
 {
 	if ((p->busnum >> 8) != drm_get_pci_domain(dev) ||
@@ -444,13 +400,6 @@ int drm_irq_by_busid(struct drm_device *dev, void *data,
 {
 	return -EINVAL;
 }
-
-int drm_pci_set_unique(struct drm_device *dev,
-		       struct drm_master *master,
-		       struct drm_unique *u)
-{
-	return -EINVAL;
-}
 #endif
 
 EXPORT_SYMBOL(drm_pci_init);
-- 
2.8.1

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

[PATCH 12/16] drm: Lobotomize set_busid nonsense for !pci drivers

[Daniel Vetter]

We already have a fallback in place to fill out the unique from
dev->unique, which is set to something reasonable in drm_dev_alloc.

Which means we only need to have a special set_busid for pci devices,
to be able to care the backwards compat code for drm 1.1 around, which
libdrm still needs.

While developing and testing this patch things blew up in really
interesting ways, and the code is rather confusing in naming things
between the kernel code, ioctl #defines and libdrm. For the next brave
dragon slayer, document all this madness properly in the userspace
interface section of gpu.tmpl.

v2: Make drm_dev_set_unique static and update kerneldoc.

v3: Entire rewrite, plus document what's going on for posterity in the
gpu docbook uapi section.

Cc: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Cc: Emil Velikov <emil.l.velikov@gmail.com>
Tested-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> (virt_gpu)
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/DocBook/gpu.tmpl                  |  4 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c         |  1 -
 drivers/gpu/drm/armada/armada_drv.c             |  1 -
 drivers/gpu/drm/drm_ioctl.c                     | 58 +++++++++++++++++++++++++
 drivers/gpu/drm/drm_platform.c                  | 18 --------
 drivers/gpu/drm/etnaviv/etnaviv_drv.c           |  1 -
 drivers/gpu/drm/exynos/exynos_drm_drv.c         |  1 -
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  1 -
 drivers/gpu/drm/imx/imx-drm-core.c              |  1 -
 drivers/gpu/drm/msm/msm_drv.c                   |  1 -
 drivers/gpu/drm/nouveau/nouveau_drm.c           |  1 -
 drivers/gpu/drm/omapdrm/omap_drv.c              |  2 -
 drivers/gpu/drm/shmobile/shmob_drm_drv.c        |  1 -
 drivers/gpu/drm/tilcdc/tilcdc_drv.c             |  1 -
 drivers/gpu/drm/virtio/virtgpu_drm_bus.c        | 10 -----
 drivers/gpu/drm/virtio/virtgpu_drv.c            |  1 -
 drivers/gpu/drm/virtio/virtgpu_drv.h            |  1 -
 include/drm/drmP.h                              |  1 -
 18 files changed, 62 insertions(+), 43 deletions(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index a836a8bcd289..94c6bdee8080 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -3099,6 +3099,10 @@ int num_ioctls;</synopsis>
   <!-- External: render nodes -->
 
     <sect1>
+      <title>libdrm Device Lookup</title>
+!Pdrivers/gpu/drm/drm_vma_manager.c getunique and setversion story
+    </sect1>
+    <sect1>
       <title>Render nodes</title>
       <para>
         DRM core provides multiple character-devices for user-space to use.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index f888c015f76c..4ea1f1ac87f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -522,7 +522,6 @@ static struct drm_driver kms_driver = {
 	.preclose = amdgpu_driver_preclose_kms,
 	.postclose = amdgpu_driver_postclose_kms,
 	.lastclose = amdgpu_driver_lastclose_kms,
-	.set_busid = drm_pci_set_busid,
 	.unload = amdgpu_driver_unload_kms,
 	.get_vblank_counter = amdgpu_get_vblank_counter_kms,
 	.enable_vblank = amdgpu_enable_vblank_kms,
diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index cb21c0b6374a..f5ebdd681445 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -189,7 +189,6 @@ static struct drm_driver armada_drm_driver = {
 	.load			= armada_drm_load,
 	.lastclose		= armada_drm_lastclose,
 	.unload			= armada_drm_unload,
-	.set_busid		= drm_platform_set_busid,
 	.get_vblank_counter	= drm_vblank_no_hw_counter,
 	.enable_vblank		= armada_drm_enable_vblank,
 	.disable_vblank		= armada_drm_disable_vblank,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index a57a5ea3ebf2..bb230fce4b38 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -37,6 +37,64 @@
 #include <linux/pci.h>
 #include <linux/export.h>
 
+/**
+ * DOC: getunique and setversion story
+ *
+ * BEWARE THE DRAGONS! MIND THE TRAPDOORS!
+ *
+ * In and attempt to warn anyone else who's trying to figure out what's going
+ * on here, I'll try to summarize the story. First things first, let's clear up
+ * the names, because the kernel internals, libdrm and the ioctls are all named
+ * differently:
+ *
+ *  - GET_UNIQUE ioctl, implemented by drm_getunique is wrapped up in libdrm
+ *    through the drmGetBusid function.
+ *  - The libdrm drmSetBusid function is backed by the SET_UNIQUE ioctl. All
+ *    that code is nerved in the kernel with drm_invalid_op().
+ *  - The internal set_busid kernel functions and driver callbacks are
+ *    exclusively use by the SET_VERSION ioctl, because only drm 1.0 (which is
+ *    nerved) allowed userspace to set the busid through the above ioctl.
+ *  - Other ioctls and functions involved are named consistently.
+ *
+ * For anyone wondering what's the difference between drm 1.1 and 1.4: Correctly
+ * handling pci domains in the busid on ppc. Doing this correctly was only
+ * implemented in libdrm in 2010, hence can't be nerved yet. No one knows what's
+ * special with drm 1.2 and 1.3.
+ *
+ * Now the actual horror story of how device lookup in drm works. At large,
+ * there's 2 different ways, either by busid, or by device driver name.
+ *
+ * Opening by busid is fairly simple:
+ *
+ * 1. First call SET_VERSION to make sure pci domains are handled properly. As a
+ *    side-effect this fills out the unique name in the master structure.
+ * 2. Call GET_UNIQUE to read out the unique name from the master structure,
+ *    which matches the busid thanks to step 1. If it doesn't, proceed to try
+ *    the next device node.
+ *
+ * Opening by name is slightly different:
+ *
+ * 1. Directly call VERSION to get the version and to match against the driver
+ *    name returned by that ioctl. Note that SET_VERSION is not called, which
+ *    means the the unique name for the master node just opening is _not_ filled
+ *    out. This despite that with current drm device nodes are always bound to
+ *    one device, and can't be runtime assigned like with drm 1.0.
+ * 2. Match driver name. If it mismatches, proceed to the next device node.
+ * 3. Call GET_UNIQUE, and check whether the unique name has length zero (by
+ *    checking that the first byte in the string is 0). If that's not the case
+ *    libdrm skips and proceeds to the next device node. Probably this is just
+ *    copypasta from drm 1.0 times where a set unique name meant that the driver
+ *    was in use already, but that's just conjecture.
+ *
+ * Long story short: To keep the open by name logic working, GET_UNIQUE must
+ * _not_ return a unique string when SET_VERSION hasn't been called yet,
+ * otherwise libdrm breaks. Even when that unique string can't ever change, and
+ * is totally irrelevant for actually opening the device because runtime
+ * assignable device instances were only support in drm 1.0, which is long dead.
+ * But the libdrm code in drmOpenByName somehow survived, hence this can't be
+ * broken.
+ */
+
 static int drm_version(struct drm_device *dev, void *data,
 		       struct drm_file *file_priv);
 
diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
index 644169e1a029..2c819ef90090 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -68,24 +68,6 @@ err_free:
 	return ret;
 }
 
-int drm_platform_set_busid(struct drm_device *dev, struct drm_master *master)
-{
-	int id;
-
-	id = dev->platformdev->id;
-	if (id < 0)
-		id = 0;
-
-	master->unique = kasprintf(GFP_KERNEL, "platform:%s:%02d",
-						dev->platformdev->name, id);
-	if (!master->unique)
-		return -ENOMEM;
-
-	master->unique_len = strlen(master->unique);
-	return 0;
-}
-EXPORT_SYMBOL(drm_platform_set_busid);
-
 /**
  * drm_platform_init - Register a platform device with the DRM subsystem
  * @driver: DRM device driver
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 3d4f56df8359..340d390306d8 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -496,7 +496,6 @@ static struct drm_driver etnaviv_drm_driver = {
 				DRIVER_RENDER,
 	.open               = etnaviv_open,
 	.preclose           = etnaviv_preclose,
-	.set_busid          = drm_platform_set_busid,
 	.gem_free_object_unlocked = etnaviv_gem_free_object,
 	.gem_vm_ops         = &vm_ops,
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 4a679fb9bb02..13d28d4229e2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -407,7 +407,6 @@ static struct drm_driver exynos_drm_driver = {
 	.preclose		= exynos_drm_preclose,
 	.lastclose		= exynos_drm_lastclose,
 	.postclose		= exynos_drm_postclose,
-	.set_busid		= drm_platform_set_busid,
 	.get_vblank_counter	= drm_vblank_no_hw_counter,
 	.enable_vblank		= exynos_drm_crtc_enable_vblank,
 	.disable_vblank		= exynos_drm_crtc_disable_vblank,
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index 193657259ee9..418a87cc6ac9 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -171,7 +171,6 @@ static struct drm_driver kirin_drm_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
 				  DRIVER_ATOMIC | DRIVER_HAVE_IRQ,
 	.fops			= &kirin_drm_fops,
-	.set_busid		= drm_platform_set_busid,
 
 	.gem_free_object_unlocked = drm_gem_cma_free_object,
 	.gem_vm_ops		= &drm_gem_cma_vm_ops,
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 82656654fb21..7746418a4c08 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -407,7 +407,6 @@ static struct drm_driver imx_drm_driver = {
 	.load			= imx_drm_driver_load,
 	.unload			= imx_drm_driver_unload,
 	.lastclose		= imx_drm_driver_lastclose,
-	.set_busid		= drm_platform_set_busid,
 	.gem_free_object_unlocked = drm_gem_cma_free_object,
 	.gem_vm_ops		= &drm_gem_cma_vm_ops,
 	.dumb_create		= drm_gem_cma_dumb_create,
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 9c654092ef78..798822bfb5c3 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -730,7 +730,6 @@ static struct drm_driver msm_driver = {
 	.open               = msm_open,
 	.preclose           = msm_preclose,
 	.lastclose          = msm_lastclose,
-	.set_busid          = drm_platform_set_busid,
 	.irq_handler        = msm_irq,
 	.irq_preinstall     = msm_irq_preinstall,
 	.irq_postinstall    = msm_irq_postinstall,
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index c00ff6e786a3..295e7621cc68 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1070,7 +1070,6 @@ nouveau_drm_init(void)
 	driver_pci = driver_stub;
 	driver_pci.set_busid = drm_pci_set_busid;
 	driver_platform = driver_stub;
-	driver_platform.set_busid = drm_platform_set_busid;
 
 	nouveau_display_options();
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 6b97011154bf..0e6d5a8ff5fd 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -801,8 +801,6 @@ static struct drm_driver omap_drm_driver = {
 	.unload = dev_unload,
 	.open = dev_open,
 	.lastclose = dev_lastclose,
-	.set_busid = drm_platform_set_busid,
-	.get_vblank_counter = drm_vblank_no_hw_counter,
 	.enable_vblank = omap_irq_enable_vblank,
 	.disable_vblank = omap_irq_disable_vblank,
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
index ee79264b5b6a..f0492603ea88 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -259,7 +259,6 @@ static struct drm_driver shmob_drm_driver = {
 				| DRIVER_PRIME,
 	.load			= shmob_drm_load,
 	.unload			= shmob_drm_unload,
-	.set_busid		= drm_platform_set_busid,
 	.irq_handler		= shmob_drm_irq,
 	.get_vblank_counter	= drm_vblank_no_hw_counter,
 	.enable_vblank		= shmob_drm_enable_vblank,
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 308e197908fc..d27809372d54 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -541,7 +541,6 @@ static struct drm_driver tilcdc_driver = {
 	.load               = tilcdc_load,
 	.unload             = tilcdc_unload,
 	.lastclose          = tilcdc_lastclose,
-	.set_busid          = drm_platform_set_busid,
 	.irq_handler        = tilcdc_irq,
 	.irq_preinstall     = tilcdc_irq_preinstall,
 	.irq_postinstall    = tilcdc_irq_postinstall,
diff --git a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
index 88a39165edd5..7f0e93f87a55 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
@@ -27,16 +27,6 @@
 
 #include "virtgpu_drv.h"
 
-int drm_virtio_set_busid(struct drm_device *dev, struct drm_master *master)
-{
-	struct pci_dev *pdev = dev->pdev;
-
-	if (pdev) {
-		return drm_pci_set_busid(dev, master);
-	}
-	return 0;
-}
-
 static void virtio_pci_kick_out_firmware_fb(struct pci_dev *pci_dev)
 {
 	struct apertures_struct *ap;
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 5820b7020ae5..c13f70cfc461 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -117,7 +117,6 @@ static const struct file_operations virtio_gpu_driver_fops = {
 
 static struct drm_driver driver = {
 	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_RENDER | DRIVER_ATOMIC,
-	.set_busid = drm_virtio_set_busid,
 	.load = virtio_gpu_driver_load,
 	.unload = virtio_gpu_driver_unload,
 	.open = virtio_gpu_driver_open,
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index acf556a35cb2..b18ef3111f0c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -49,7 +49,6 @@
 #define DRIVER_PATCHLEVEL 1
 
 /* virtgpu_drm_bus.c */
-int drm_virtio_set_busid(struct drm_device *dev, struct drm_master *master);
 int drm_virtio_init(struct drm_driver *driver, struct virtio_device *vdev);
 
 struct virtio_gpu_object {
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index bd40d7ee5ec5..fc86a933309e 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1125,7 +1125,6 @@ extern int drm_pcie_get_max_link_width(struct drm_device *dev, u32 *mlw);
 
 /* platform section */
 extern int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device);
-extern int drm_platform_set_busid(struct drm_device *d, struct drm_master *m);
 
 /* returns true if currently okay to sleep */
 static __inline__ bool drm_can_sleep(void)
-- 
2.8.1

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

[PATCH 13/16] drm: Refactor drop/set master code a bit

[Daniel Vetter]

File open/set_maseter ioctl and file close/drop_master ioctl share the
same master handling code. Extract it.

Note that vmwgfx's master_set callback needs to know whether the
master is a new one or has been used already, so thread this through.
On the close/drop side a similar parameter existed, but wasnt used.
Drop it to simplify the flow.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_auth.c          | 70 ++++++++++++++++++++-----------------
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c |  3 +-
 include/drm/drmP.h                  |  3 +-
 3 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 6bba6b9e9dcc..698b82032185 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -110,6 +110,24 @@ static struct drm_master *drm_master_create(struct drm_device *dev)
 	return master;
 }
 
+static int drm_set_master(struct drm_device *dev, struct drm_file *fpriv,
+			  bool new_master)
+{
+	int ret = 0;
+
+	dev->master = drm_master_get(fpriv->master);
+	fpriv->is_master = 1;
+	if (dev->driver->master_set) {
+		ret = dev->driver->master_set(dev, fpriv, new_master);
+		if (unlikely(ret != 0)) {
+			fpriv->is_master = 0;
+			drm_master_put(&dev->master);
+		}
+	}
+
+	return ret;
+}
+
 /*
  * drm_new_set_master - Allocate a new master object and become master for the
  * associated master realm.
@@ -134,24 +152,21 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
 
 	/* take another reference for the copy in the local file priv */
 	old_master = fpriv->master;
-	fpriv->master = drm_master_get(dev->master);
+	fpriv->master = drm_master_create(dev);
+	if (!fpriv->master)
+		return -ENOMEM;
 
 	if (dev->driver->master_create) {
 		ret = dev->driver->master_create(dev, fpriv->master);
 		if (ret)
 			goto out_err;
 	}
-	if (dev->driver->master_set) {
-		ret = dev->driver->master_set(dev, fpriv, true);
-		if (ret)
-			goto out_err;
-	}
-
-	fpriv->is_master = 1;
 	fpriv->allowed_master = 1;
 	fpriv->authenticated = 1;
-	if (old_master)
-		drm_master_put(&old_master);
+
+	ret = drm_set_master(dev, fpriv, true);
+	if (ret)
+		goto out_err;
 
 	return 0;
 
@@ -188,21 +203,21 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
 		goto out_unlock;
 	}
 
-	dev->master = drm_master_get(file_priv->master);
-	file_priv->is_master = 1;
-	if (dev->driver->master_set) {
-		ret = dev->driver->master_set(dev, file_priv, false);
-		if (unlikely(ret != 0)) {
-			file_priv->is_master = 0;
-			drm_master_put(&dev->master);
-		}
-	}
-
+	ret = drm_set_master(dev, file_priv, false);
 out_unlock:
 	mutex_unlock(&dev->master_mutex);
 	return ret;
 }
 
+void drm_drop_master(struct drm_device *dev,
+		     struct drm_file *fpriv)
+{
+	if (dev->driver->master_drop)
+		dev->driver->master_drop(dev, fpriv);
+	drm_master_put(&dev->master);
+	fpriv->is_master = 0;
+}
+
 int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
 			 struct drm_file *file_priv)
 {
@@ -216,11 +231,7 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
 		goto out_unlock;
 
 	ret = 0;
-	if (dev->driver->master_drop)
-		dev->driver->master_drop(dev, file_priv, false);
-	drm_master_put(&dev->master);
-	file_priv->is_master = 0;
-
+	drm_drop_master(dev, file_priv);
 out_unlock:
 	mutex_unlock(&dev->master_mutex);
 	return ret;
@@ -271,17 +282,12 @@ void drm_master_release(struct drm_file *file_priv)
 		mutex_unlock(&dev->struct_mutex);
 	}
 
-	if (dev->master == file_priv->master) {
-		/* drop the reference held my the minor */
-		if (dev->driver->master_drop)
-			dev->driver->master_drop(dev, file_priv, true);
-		drm_master_put(&dev->master);
-	}
+	if (dev->master == file_priv->master)
+		drm_drop_master(dev, file_priv);
 out:
 	/* drop the master reference held by the file priv */
 	if (file_priv->master)
 		drm_master_put(&file_priv->master);
-	file_priv->is_master = 0;
 	mutex_unlock(&dev->master_mutex);
 }
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 9fcd8200d485..35eedc9d44fa 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1228,8 +1228,7 @@ static int vmw_master_set(struct drm_device *dev,
 }
 
 static void vmw_master_drop(struct drm_device *dev,
-			    struct drm_file *file_priv,
-			    bool from_release)
+			    struct drm_file *file_priv)
 {
 	struct vmw_private *dev_priv = vmw_priv(dev);
 	struct vmw_fpriv *vmw_fp = vmw_fpriv(file_priv);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fc86a933309e..24bdad38f444 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -573,8 +573,7 @@ struct drm_driver {
 
 	int (*master_set)(struct drm_device *dev, struct drm_file *file_priv,
 			  bool from_open);
-	void (*master_drop)(struct drm_device *dev, struct drm_file *file_priv,
-			    bool from_release);
+	void (*master_drop)(struct drm_device *dev, struct drm_file *file_priv);
 
 	int (*debugfs_init)(struct drm_minor *minor);
 	void (*debugfs_cleanup)(struct drm_minor *minor);
-- 
2.8.1

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

[PATCH 14/16] drm: Extract drm_is_current_master

[Daniel Vetter]

Just rolling out a bit of abstraction to be able to clean
up the master logic in the next step.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_auth.c                 | 12 +++++++++---
 drivers/gpu/drm/drm_crtc.c                 |  2 +-
 drivers/gpu/drm/drm_info.c                 |  2 +-
 drivers/gpu/drm/drm_ioctl.c                |  3 ++-
 drivers/gpu/drm/drm_lock.c                 |  2 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c        |  2 +-
 include/drm/drmP.h                         | 15 ++++++++-------
 8 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 698b82032185..4c241e53cbf3 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -185,7 +185,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
 	int ret = 0;
 
 	mutex_lock(&dev->master_mutex);
-	if (file_priv->is_master)
+	if (drm_is_current_master(file_priv))
 		goto out_unlock;
 
 	if (dev->master) {
@@ -224,7 +224,7 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
 	int ret = -EINVAL;
 
 	mutex_lock(&dev->master_mutex);
-	if (!file_priv->is_master)
+	if (!drm_is_current_master(file_priv))
 		goto out_unlock;
 
 	if (!dev->master)
@@ -263,7 +263,7 @@ void drm_master_release(struct drm_file *file_priv)
 	if (file_priv->magic)
 		idr_remove(&file_priv->master->magic_map, file_priv->magic);
 
-	if (!file_priv->is_master)
+	if (!drm_is_current_master(file_priv))
 		goto out;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET)) {
@@ -291,6 +291,12 @@ out:
 	mutex_unlock(&dev->master_mutex);
 }
 
+bool drm_is_current_master(struct drm_file *fpriv)
+{
+	return fpriv->is_master;
+}
+EXPORT_SYMBOL(drm_is_current_master);
+
 struct drm_master *drm_master_get(struct drm_master *master)
 {
 	kref_get(&master->refcount);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 4ff818ad34d7..81083f98d155 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3499,7 +3499,7 @@ int drm_mode_getfb(struct drm_device *dev,
 	r->bpp = fb->bits_per_pixel;
 	r->pitch = fb->pitches[0];
 	if (fb->funcs->create_handle) {
-		if (file_priv->is_master || capable(CAP_SYS_ADMIN) ||
+		if (drm_is_current_master(file_priv) || capable(CAP_SYS_ADMIN) ||
 		    drm_is_control_client(file_priv)) {
 			ret = fb->funcs->create_handle(fb, file_priv,
 						       &r->handle);
diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
index 24c80dd13837..ba9607397a00 100644
--- a/drivers/gpu/drm/drm_info.c
+++ b/drivers/gpu/drm/drm_info.c
@@ -102,7 +102,7 @@ int drm_clients_info(struct seq_file *m, void *data)
 			   task ? task->comm : "<unknown>",
 			   pid_vnr(priv->pid),
 			   priv->minor->index,
-			   priv->is_master ? 'y' : 'n',
+			   drm_is_current_master(priv) ? 'y' : 'n',
 			   priv->authenticated ? 'y' : 'n',
 			   from_kuid_munged(seq_user_ns(m), priv->uid),
 			   priv->magic);
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index bb230fce4b38..a0c1d172954d 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -480,7 +480,8 @@ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
 		return -EACCES;
 
 	/* MASTER is only for master or control clients */
-	if (unlikely((flags & DRM_MASTER) && !file_priv->is_master &&
+	if (unlikely((flags & DRM_MASTER) && 
+		     !drm_is_current_master(file_priv) &&
 		     !drm_is_control_client(file_priv)))
 		return -EACCES;
 
diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
index ae0a4d39deec..48ac0ebbd663 100644
--- a/drivers/gpu/drm/drm_lock.c
+++ b/drivers/gpu/drm/drm_lock.c
@@ -219,7 +219,7 @@ int drm_legacy_lock(struct drm_device *dev, void *data,
 	/* don't set the block all signals on the master process for now 
 	 * really probably not the correct answer but lets us debug xkb
  	 * xserver for now */
-	if (!file_priv->is_master) {
+	if (!drm_is_current_master(file_priv)) {
 		dev->sigdata.context = lock->context;
 		dev->sigdata.lock = master->lock.hw_lock;
 	}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 8097698b9622..7941f1fe9cd2 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1446,7 +1446,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 
 	dispatch_flags = 0;
 	if (args->flags & I915_EXEC_SECURE) {
-		if (!file->is_master || !capable(CAP_SYS_ADMIN))
+		if (!drm_is_current_master(file) || !capable(CAP_SYS_ADMIN))
 		    return -EPERM;
 
 		dispatch_flags |= I915_DISPATCH_SECURE;
@@ -1553,7 +1553,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 							     batch_obj,
 							     args->batch_start_offset,
 							     args->batch_len,
-							     file->is_master);
+							     drm_is_current_master(file));
 		if (IS_ERR(parsed_batch_obj)) {
 			ret = PTR_ERR(parsed_batch_obj);
 			goto err;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 35eedc9d44fa..60646644bef3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1049,7 +1049,7 @@ static struct vmw_master *vmw_master_check(struct drm_device *dev,
 	if (unlikely(ret != 0))
 		return ERR_PTR(-ERESTARTSYS);
 
-	if (file_priv->is_master) {
+	if (drm_is_current_master(file_priv)) {
 		mutex_unlock(&dev->master_mutex);
 		return NULL;
 	}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 24bdad38f444..e0599cf24e1e 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1010,14 +1010,15 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc
 	return &crtc->dev->vblank[drm_crtc_index(crtc)].queue;
 }
 
-				/* Stub support (drm_stub.h) */
-extern struct drm_master *drm_master_get(struct drm_master *master);
-extern void drm_master_put(struct drm_master **master);
-
-extern void drm_put_dev(struct drm_device *dev);
-extern void drm_unplug_dev(struct drm_device *dev);
+/* drm_auth.c */
+struct drm_master *drm_master_get(struct drm_master *master);
+void drm_master_put(struct drm_master **master);
+bool drm_is_current_master(struct drm_file *fpriv);
+
+/* drm_drv.c */
+void drm_put_dev(struct drm_device *dev);
+void drm_unplug_dev(struct drm_device *dev);
 extern unsigned int drm_debug;
-extern bool drm_atomic;
 
 				/* Debugfs support */
 #if defined(CONFIG_DEBUG_FS)
-- 
2.8.1

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

[PATCH 15/16] drm: Clear up master tracking booleans

[Daniel Vetter]

- is_master can be removed, we can compute this by checking allowed_master
  (which really just tracks whether a master struct has been allocated
  for this fpriv in either open or set_master), and whether the fpriv is
  the current master on the device.

- that frees up is_master as a good replacement name for allowed_master.
  With that it's clear that it tracks whether the fpriv is a master (with
  possibly clients attached to it and authenticated against it), and that
  one of those fprivs with is_master set is the current master.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_auth.c | 9 +++------
 include/drm/drmP.h         | 6 ++----
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 4c241e53cbf3..b4dfa8ab20d7 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -116,11 +116,9 @@ static int drm_set_master(struct drm_device *dev, struct drm_file *fpriv,
 	int ret = 0;
 
 	dev->master = drm_master_get(fpriv->master);
-	fpriv->is_master = 1;
 	if (dev->driver->master_set) {
 		ret = dev->driver->master_set(dev, fpriv, new_master);
 		if (unlikely(ret != 0)) {
-			fpriv->is_master = 0;
 			drm_master_put(&dev->master);
 		}
 	}
@@ -161,7 +159,7 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
 		if (ret)
 			goto out_err;
 	}
-	fpriv->allowed_master = 1;
+	fpriv->is_master = 1;
 	fpriv->authenticated = 1;
 
 	ret = drm_set_master(dev, fpriv, true);
@@ -198,7 +196,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
 		goto out_unlock;
 	}
 
-	if (!file_priv->allowed_master) {
+	if (!file_priv->is_master) {
 		ret = drm_new_set_master(dev, file_priv);
 		goto out_unlock;
 	}
@@ -215,7 +213,6 @@ void drm_drop_master(struct drm_device *dev,
 	if (dev->driver->master_drop)
 		dev->driver->master_drop(dev, fpriv);
 	drm_master_put(&dev->master);
-	fpriv->is_master = 0;
 }
 
 int drm_dropmaster_ioctl(struct drm_device *dev, void *data,
@@ -293,7 +290,7 @@ out:
 
 bool drm_is_current_master(struct drm_file *fpriv)
 {
-	return fpriv->is_master;
+	return fpriv->is_master && fpriv->master == fpriv->minor->dev->master;
 }
 EXPORT_SYMBOL(drm_is_current_master);
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index e0599cf24e1e..761b20332321 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -303,8 +303,6 @@ struct drm_prime_file_private {
 /** File private data */
 struct drm_file {
 	unsigned authenticated :1;
-	/* Whether we're master for a minor. Protected by master_mutex */
-	unsigned is_master :1;
 	/* true when the client has asked us to expose stereo 3D mode flags */
 	unsigned stereo_allowed :1;
 	/*
@@ -315,10 +313,10 @@ struct drm_file {
 	/* true if client understands atomic properties */
 	unsigned atomic:1;
 	/*
-	 * This client is allowed to gain master privileges for @master.
+	 * This client is the create the master.
 	 * Protected by struct drm_device::master_mutex.
 	 */
-	unsigned allowed_master:1;
+	unsigned is_master:1;
 
 	struct pid *pid;
 	kuid_t uid;
-- 
2.8.1

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

[PATCH 16/16] drm: document drm_auth.c

[Daniel Vetter]

Also extract drm_auth.h for nicer grouping.

v2: Nuke the other comments since they don't really explain a lot, and
within the drm core we generally only document functions exported to
drivers: The main audience for these docs are driver writers.

v3: Limit the exposure of drm_master internals by only including
drm_auth.h where it is neede (Chris).

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/DocBook/gpu.tmpl      |  6 ++++
 drivers/gpu/drm/drm_auth.c          | 69 +++++++++++++++++++++----------------
 drivers/gpu/drm/drm_crtc.c          |  1 +
 drivers/gpu/drm/drm_ioctl.c         |  1 +
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h |  1 +
 include/drm/drmP.h                  | 30 +---------------
 include/drm/drm_auth.h              | 59 +++++++++++++++++++++++++++++++
 include/drm/drm_legacy.h            |  2 ++
 9 files changed, 112 insertions(+), 58 deletions(-)
 create mode 100644 include/drm/drm_auth.h

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 94c6bdee8080..b7f6316b7bee 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -3103,6 +3103,12 @@ int num_ioctls;</synopsis>
 !Pdrivers/gpu/drm/drm_vma_manager.c getunique and setversion story
     </sect1>
     <sect1>
+      <title>Primary Nodes, DRM Master and Authentication</title>
+!Pdrivers/gpu/drm_auth.c master and authentication
+!Edrivers/gpu/drm_auth.c
+!Einclude/drm/drm_auth.h
+    </sect1>
+    <sect1>
       <title>Render nodes</title>
       <para>
         DRM core provides multiple character-devices for user-space to use.
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index b4dfa8ab20d7..3774b9964dbe 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -32,18 +32,27 @@
 #include "drm_internal.h"
 #include "drm_legacy.h"
 
-/**
- * drm_getmagic - Get unique magic of a client
- * @dev: DRM device to operate on
- * @data: ioctl data containing the drm_auth object
- * @file_priv: DRM file that performs the operation
+/** DOC: master and authentication
+ *
+ * struct &drm_master is used to track groups of clients with open
+ * primary/legacy device nodes. For every struct &drm_file which at least once
+ * successfully became the device master (either through the SET_MASTER IOCTL,
+ * or implicitly through opening the primary device node when no one else is the
+ * current master that time) there exists one &drm_master. This is noted in the
+ * is_master member of &drm_master. All other clients have just a pointer to the
+ * &drm_master they are associated with.
  *
- * This looks up the unique magic of the passed client and returns it. If the
- * client did not have a magic assigned, yet, a new one is registered. The magic
- * is stored in the passed drm_auth object.
+ * In addition only one &drm_master can be the current master for a &drm_device.
+ * It can be switched through the DROP_MASTER and SET_MASTER IOCTL, or
+ * implicitly through closing/openeing the primary device node. See also
+ * drm_is_current_master().
  *
- * Returns: 0 on success, negative error code on failure.
+ * Clients can authenticate against the current master (if it matches their own)
+ * using the GETMAGIC and AUTHMAGIC IOCTLs. Together with exchanging masters,
+ * this allows controlled access to the device for an entire group of mutually
+ * trusted clients.
  */
+
 int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv)
 {
 	struct drm_auth *auth = data;
@@ -64,16 +73,6 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv)
 	return ret < 0 ? ret : 0;
 }
 
-/**
- * drm_authmagic - Authenticate client with a magic
- * @dev: DRM device to operate on
- * @data: ioctl data containing the drm_auth object
- * @file_priv: DRM file that performs the operation
- *
- * This looks up a DRM client by the passed magic and authenticates it.
- *
- * Returns: 0 on success, negative error code on failure.
- */
 int drm_authmagic(struct drm_device *dev, void *data,
 		  struct drm_file *file_priv)
 {
@@ -126,16 +125,6 @@ static int drm_set_master(struct drm_device *dev, struct drm_file *fpriv,
 	return ret;
 }
 
-/*
- * drm_new_set_master - Allocate a new master object and become master for the
- * associated master realm.
- *
- * @dev: The associated device.
- * @fpriv: File private identifying the client.
- *
- * This function must be called with dev::master_mutex held.
- * Returns negative error code on failure. Zero on success.
- */
 static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
 {
 	struct drm_master *old_master;
@@ -288,12 +277,28 @@ out:
 	mutex_unlock(&dev->master_mutex);
 }
 
+/**
+ * drm_is_current_master - checks whether this master is the current one
+ * @fpriv: DRM file private
+ *
+ * Checks whether @fpriv is a master and that it is the current master on its
+ * device. This decides whether a client is allowed to run DRM_MASTER IOCTLs.
+ *
+ * Most of the modern IOCTL which require DRM_MASTER are for kernel modesetting
+ * - the current master is assumed to own the non-shareable display hardware.
+ */
 bool drm_is_current_master(struct drm_file *fpriv)
 {
 	return fpriv->is_master && fpriv->master == fpriv->minor->dev->master;
 }
 EXPORT_SYMBOL(drm_is_current_master);
 
+/**
+ * drm_master_get - reference a master pointer
+ * @master: struct &drm_master
+ *
+ * Increments the reference count of @master.
+ */
 struct drm_master *drm_master_get(struct drm_master *master)
 {
 	kref_get(&master->refcount);
@@ -316,6 +321,12 @@ static void drm_master_destroy(struct kref *kref)
 	kfree(master);
 }
 
+/**
+ * drm_master_put - unreference and clear a master pointer
+ * @master: pointer to a pointer of struct &drm_master
+ *
+ * This decrements the &drm_master behind @master and sets it to NULL.
+ */
 void drm_master_put(struct drm_master **master)
 {
 	kref_put(&(*master)->refcount, drm_master_destroy);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 81083f98d155..871af372662d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -39,6 +39,7 @@
 #include <drm/drm_fourcc.h>
 #include <drm/drm_modeset_lock.h>
 #include <drm/drm_atomic.h>
+#include <drm/drm_auth.h>
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index a0c1d172954d..88796a383e40 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -30,6 +30,7 @@
 
 #include <drm/drmP.h>
 #include <drm/drm_core.h>
+#include <drm/drm_auth.h>
 #include "drm_legacy.h"
 #include "drm_internal.h"
 #include "drm_crtc_internal.h"
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9fa9698fe247..0f8632c93e95 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -47,6 +47,7 @@
 #include <drm/intel-gtt.h>
 #include <drm/drm_legacy.h> /* for struct drm_dma_handle */
 #include <drm/drm_gem.h>
+#include <drm/drm_auth.h>
 
 #include "i915_params.h"
 #include "i915_reg.h"
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 1980e2a28265..9a90f824814e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -32,6 +32,7 @@
 #include <drm/drmP.h>
 #include <drm/vmwgfx_drm.h>
 #include <drm/drm_hashtab.h>
+#include <drm/drm_auth.h>
 #include <linux/suspend.h>
 #include <drm/ttm/ttm_bo_driver.h>
 #include <drm/ttm/ttm_object.h>
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 761b20332321..d22ba6bf2299 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -86,6 +86,7 @@ struct drm_local_map;
 struct drm_device_dma;
 struct drm_dma_handle;
 struct drm_gem_object;
+struct drm_master;
 
 struct device_node;
 struct videomode;
@@ -373,30 +374,6 @@ struct drm_lock_data {
 	int idle_has_lock;
 };
 
-/**
- * struct drm_master - drm master structure
- *
- * @refcount: Refcount for this master object.
- * @dev: Link back to the DRM device
- * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex.
- * @unique_len: Length of unique field. Protected by drm_global_mutex.
- * @magic_map: Map of used authentication tokens. Protected by struct_mutex.
- * @lock: DRI lock information.
- * @driver_priv: Pointer to driver-private information.
- *
- * Note that master structures are only relevant for the legacy/primary device
- * nodes, hence there can only be one per device, not one per drm_minor.
- */
-struct drm_master {
-	struct kref refcount;
-	struct drm_device *dev;
-	char *unique;
-	int unique_len;
-	struct idr magic_map;
-	struct drm_lock_data lock;
-	void *driver_priv;
-};
-
 /* Flags and return codes for get_vblank_timestamp() driver function. */
 #define DRM_CALLED_FROM_VBLIRQ 1
 #define DRM_VBLANKTIME_SCANOUTPOS_METHOD (1 << 0)
@@ -1008,11 +985,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc
 	return &crtc->dev->vblank[drm_crtc_index(crtc)].queue;
 }
 
-/* drm_auth.c */
-struct drm_master *drm_master_get(struct drm_master *master);
-void drm_master_put(struct drm_master **master);
-bool drm_is_current_master(struct drm_file *fpriv);
-
 /* drm_drv.c */
 void drm_put_dev(struct drm_device *dev);
 void drm_unplug_dev(struct drm_device *dev);
diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
new file mode 100644
index 000000000000..610223b0481b
--- /dev/null
+++ b/include/drm/drm_auth.h
@@ -0,0 +1,59 @@
+/*
+ * Internal Header for the Direct Rendering Manager
+ *
+ * Copyright 2016 Intel Corporation
+ *
+ * Author: Daniel Vetter <daniel.vetter@ffwll.ch>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef _DRM_AUTH_H_
+#define _DRM_AUTH_H_
+
+/**
+ * struct drm_master - drm master structure
+ *
+ * @refcount: Refcount for this master object.
+ * @dev: Link back to the DRM device
+ * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex.
+ * @unique_len: Length of unique field. Protected by drm_global_mutex.
+ * @magic_map: Map of used authentication tokens. Protected by struct_mutex.
+ * @lock: DRI lock information.
+ * @driver_priv: Pointer to driver-private information.
+ *
+ * Note that master structures are only relevant for the legacy/primary device
+ * nodes, hence there can only be one per device, not one per drm_minor.
+ */
+struct drm_master {
+	struct kref refcount;
+	struct drm_device *dev;
+	char *unique;
+	int unique_len;
+	struct idr magic_map;
+	struct drm_lock_data lock;
+	void *driver_priv;
+};
+
+struct drm_master *drm_master_get(struct drm_master *master);
+void drm_master_put(struct drm_master **master);
+bool drm_is_current_master(struct drm_file *fpriv);
+
+#endif
diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
index a5ef2c7e40f8..cf0e7d89bcdf 100644
--- a/include/drm/drm_legacy.h
+++ b/include/drm/drm_legacy.h
@@ -1,6 +1,8 @@
 #ifndef __DRM_DRM_LEGACY_H__
 #define __DRM_DRM_LEGACY_H__
 
+#include <drm/drm_auth.h>
+
 /*
  * Legacy driver interfaces for the Direct Rendering Manager
  *
-- 
2.8.1

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

Re: [PATCH 06/16] drm: Move master pointer from drm_minor to drm_device

[Chris Wilson]

On Fri, Jun 17, 2016 at 09:33:24AM +0200, Daniel Vetter wrote:
> There can only be one current master, and it's for the overall device.
> Render/control minors don't support master-based auth at all.
> 
> This simplifies the master logic a lot, at least in my eyes: All these
> additional pointer chases are just confusing.
> 
> While doing the conversion I spotted some locking fail:
> - drm_lock/drm_auth check dev->master without holding the
>   master_mutex. This is fallout from
> 
>   commit c996fd0b956450563454e7ccc97a82ca31f9d043
>   Author: Thomas Hellstrom <thellstrom@vmware.com>
>   Date:   Tue Feb 25 19:57:44 2014 +0100
> 
>       drm: Protect the master management with a drm_device::master_mutex v3
> 
>   but I honestly don't care one bit about those old legacy drivers
>   using this.
> 
> - debugfs name info should just grab master_mutex.
> 
> - And the fbdev helper looked at it to figure out whether someone is
>   using KMS. We just need a consistent value, so READ_ONCE. Aside: We
>   should probably check if anyone has opened a control node too, but I
>   guess current userspace doesn't really do that yet.
> 
> v2: Balance locking, reported by Julia.
> 
> Cc: Julia Lawall <julia.lawall@lip6.fr>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Ok, I understand now why drm_new_set_master appears backwards to me. It
really is creating a new master that fpriv inherits (just like fpriv
inherits the existing master otherwise).

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-chris

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

Re: [Intel-gfx] [PATCH 07/16] drm: Clean up drm_crtc.h

[Chris Wilson]

On Fri, Jun 17, 2016 at 09:33:25AM +0200, Daniel Vetter wrote:
> - Group declarations for separate files (drm_bridge.c, drm_edid.c)
> - Move declarations only used within drm.ko to drm_crtc_internal.h
> - drm_property_type_valid to drm_crtc.c, its only callsite
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-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

Re: [PATCH 13/16] drm: Refactor drop/set master code a bit

[Chris Wilson]

On Fri, Jun 17, 2016 at 09:33:31AM +0200, Daniel Vetter wrote:
> File open/set_maseter ioctl and file close/drop_master ioctl share the
> same master handling code. Extract it.
> 
> Note that vmwgfx's master_set callback needs to know whether the
> master is a new one or has been used already, so thread this through.
> On the close/drop side a similar parameter existed, but wasnt used.
> Drop it to simplify the flow.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

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

Re: [PATCH 14/16] drm: Extract drm_is_current_master

[Chris Wilson]

On Fri, Jun 17, 2016 at 09:33:32AM +0200, Daniel Vetter wrote:
> Just rolling out a bit of abstraction to be able to clean
> up the master logic in the next step.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> ---
> diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
> index 24c80dd13837..ba9607397a00 100644
> --- a/drivers/gpu/drm/drm_info.c
> +++ b/drivers/gpu/drm/drm_info.c
> @@ -102,7 +102,7 @@ int drm_clients_info(struct seq_file *m, void *data)
>  			   task ? task->comm : "<unknown>",
>  			   pid_vnr(priv->pid),
>  			   priv->minor->index,
> -			   priv->is_master ? 'y' : 'n',
> +			   drm_is_current_master(priv) ? 'y' : 'n',
>  			   priv->authenticated ? 'y' : 'n',

Aside: looks like another case for yesno()
-Chris

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

Re: [PATCH 15/16] drm: Clear up master tracking booleans

[Chris Wilson]

On Fri, Jun 17, 2016 at 09:33:33AM +0200, Daniel Vetter wrote:
> - is_master can be removed, we can compute this by checking allowed_master
>   (which really just tracks whether a master struct has been allocated
>   for this fpriv in either open or set_master), and whether the fpriv is
>   the current master on the device.
> 
> - that frees up is_master as a good replacement name for allowed_master.
>   With that it's clear that it tracks whether the fpriv is a master (with
>   possibly clients attached to it and authenticated against it), and that
>   one of those fprivs with is_master set is the current master.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

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

Re: [PATCH 16/16] drm: document drm_auth.c

[Chris Wilson]

On Fri, Jun 17, 2016 at 09:33:34AM +0200, Daniel Vetter wrote:
> Also extract drm_auth.h for nicer grouping.
> 
> v2: Nuke the other comments since they don't really explain a lot, and
> within the drm core we generally only document functions exported to
> drivers: The main audience for these docs are driver writers.
> 
> v3: Limit the exposure of drm_master internals by only including
> drm_auth.h where it is neede (Chris).
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9fa9698fe247..0f8632c93e95 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -47,6 +47,7 @@
>  #include <drm/intel-gtt.h>
>  #include <drm/drm_legacy.h> /* for struct drm_dma_handle */
>  #include <drm/drm_gem.h>
> +#include <drm/drm_auth.h>

I think we should be able to wean this further.
-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

✗ Ro.CI.BAT: failure for More drm master and SET_UNIQUE cleanups

[Patchwork]

== Series Details ==

Series: More drm master and SET_UNIQUE cleanups
URL   : https://patchwork.freedesktop.org/series/8808/
State : failure

== Summary ==

Applying: drm: Only do the hw.lock cleanup in master_relase for !MODESET
Applying: drm: Move authmagic cleanup into drm_master_release
Applying: drm: Protect authmagic with master_mutex
Applying: drm: Mark authmagic ioctls as unlocked
Applying: drm: Mark set/drop master ioctl as unlocked.
Applying: drm: Move master pointer from drm_minor to drm_device
Applying: drm: Clean up drm_crtc.h
Applying: drm: Use dev->name as fallback for dev->unique
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/drm_drv.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/drm_drv.c
Applying: drm/vgem: Stop calling drm_drv_set_unique
Applying: drm: Don't call drm_dev_set_unique from platform drivers
Applying: drm: Nuke SET_UNIQUE ioctl
Applying: drm: Lobotomize set_busid nonsense for !pci drivers
Applying: drm: Refactor drop/set master code a bit
Applying: drm: Extract drm_is_current_master
fatal: sha1 information is lacking or useless (include/drm/drmP.h).
error: could not build fake ancestor
Patch failed at 0014 drm: Extract drm_is_current_master
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

Re: [PATCH 10/16] drm: Don't call drm_dev_set_unique from platform drivers

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Friday 17 Jun 2016 09:33:28 Daniel Vetter wrote:
> Since
> 
> commit e112e593b215c394c0303dbf0534db0928e87967
> Author: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> Date:   Fri Dec 11 11:20:28 2015 +0100
> 
>     drm: use dev_name as default unique name in drm_dev_alloc()
> 
> we're using a reasonable default which should work for everyone. Only
> mtk, rcar-du and sun4i are affected, and as kms-only drivers without
> any rendering support no one should ever care about the unique name
> 
> v2: Rebase on top of mediatek.
> 
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Emil Velikov <emil.l.velikov@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/drm_drv.c              | 35 +++++++++---------------------
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c |  2 --
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c  |  2 --
>  drivers/gpu/drm/sun4i/sun4i_drv.c      |  4 ----
>  include/drm/drmP.h                     |  1 -
>  5 files changed, 11 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index ecba2511ef5a..25f10586a74d 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -298,10 +298,9 @@ void drm_minor_release(struct drm_minor *minor)
>   * callbacks implemented by the driver. The driver then needs to initialize
> all * the various subsystems for the drm device like memory management,
> vblank * handling, modesetting support and intial output configuration plus
> obviously - * initialize all the corresponding hardware bits. An important
> part of this is - * also calling drm_dev_set_unique() to set the
> userspace-visible unique name of - * this device instance. Finally when
> everything is up and running and ready for - * userspace the device
> instance can be published using drm_dev_register(). + * initialize all the
> corresponding hardware bits. Finally when everything is up + * and running
> and ready for userspace the device instance can be published + * using
> drm_dev_register().
>   *
>   * There is also deprecated support for initalizing device instances using
>   * bus-specific helpers and the ->load() callback. But due to
> @@ -323,6 +322,14 @@ void drm_minor_release(struct drm_minor *minor)
>   * dev_priv field of &drm_device.
>   */
> 
> +static int drm_dev_set_unique(struct drm_device *dev, const char *name)
> +{
> +	kfree(dev->unique);
> +	dev->unique = kstrdup(name, GFP_KERNEL);
> +
> +	return dev->unique ? 0 : -ENOMEM;
> +}
> +
>  /**
>   * drm_put_dev - Unregister and release a DRM device
>   * @dev: DRM device
> @@ -697,26 +704,6 @@ void drm_dev_unregister(struct drm_device *dev)
>  }
>  EXPORT_SYMBOL(drm_dev_unregister);
> 
> -/**
> - * drm_dev_set_unique - Set the unique name of a DRM device
> - * @dev: device of which to set the unique name
> - * @name: unique name
> - *
> - * Sets the unique name of a DRM device using the specified string. Drivers
> - * can use this at driver probe time if the unique name of the devices
> they - * drive is static.
> - *
> - * Return: 0 on success or a negative error code on failure.
> - */
> -int drm_dev_set_unique(struct drm_device *dev, const char *name)
> -{
> -	kfree(dev->unique);
> -	dev->unique = kstrdup(name, GFP_KERNEL);
> -
> -	return dev->unique ? 0 : -ENOMEM;
> -}
> -EXPORT_SYMBOL(drm_dev_set_unique);
> -
>  /*
>   * DRM Core
>   * The DRM core module initializes all global DRM objects and makes them
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index c33bf98c5d5e..04e901a80234
> 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -280,8 +280,6 @@ static int mtk_drm_bind(struct device *dev)
>  	if (!drm)
>  		return -ENOMEM;
> 
> -	drm_dev_set_unique(drm, dev_name(dev));
> -
>  	drm->dev_private = private;
>  	private->drm = drm;
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 48ec4b6e8b26..8784208d8eed
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -320,8 +320,6 @@ static int rcar_du_probe(struct platform_device *pdev)
>  	if (!ddev)
>  		return -ENOMEM;
> 
> -	drm_dev_set_unique(ddev, dev_name(&pdev->dev));
> -
>  	rcdu->ddev = ddev;
>  	ddev->dev_private = rcdu;
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c
> b/drivers/gpu/drm/sun4i/sun4i_drv.c index 68e9d85085fb..5c4b4ad17ad3 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -135,10 +135,6 @@ static int sun4i_drv_bind(struct device *dev)
>  	if (!drm)
>  		return -ENOMEM;
> 
> -	ret = drm_dev_set_unique(drm, dev_name(drm->dev));
> -	if (ret)
> -		goto free_drm;
> -
>  	drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
>  	if (!drv) {
>  		ret = -ENOMEM;
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 62e0e70cb5a2..bd40d7ee5ec5 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1073,7 +1073,6 @@ void drm_dev_ref(struct drm_device *dev);
>  void drm_dev_unref(struct drm_device *dev);
>  int drm_dev_register(struct drm_device *dev, unsigned long flags);
>  void drm_dev_unregister(struct drm_device *dev);
> -int drm_dev_set_unique(struct drm_device *dev, const char *name);
> 
>  struct drm_minor *drm_minor_acquire(unsigned int minor_id);
>  void drm_minor_release(struct drm_minor *minor);

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 08/16] drm: Use dev->name as fallback for dev->unique

[Emil Velikov]

Hi Daniel,

On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Lots of arm drivers get this wrong and for most arm boards this is the
> right thing actually. And anyway with most loaders you want to chase
> sysfs links anyway to figure out which dri device you want.
>
> This will fix dmesg noise for rockchip and sti.
>
> Also add a fallback to driver->name for entirely virtual drivers like
> vgem.
>
> v2: Rebase on top of
>
> commit e112e593b215c394c0303dbf0534db0928e87967
> Author: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> Date:   Fri Dec 11 11:20:28 2015 +0100
>
>     drm: use dev_name as default unique name in drm_dev_alloc()
>
> and simplify a bit. Plus add a comment.
>
> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> Reported-by: Ilia Mirkin <imirkin@alum.mit.edu>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_drv.c   | 10 +++++-----
>  drivers/gpu/drm/drm_ioctl.c |  8 +-------
>  2 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 10afa2539181..ecba2511ef5a 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -523,11 +523,11 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver,
>                 }
>         }
>
> -       if (parent) {
> -               ret = drm_dev_set_unique(dev, dev_name(parent));
> -               if (ret)
> -                       goto err_setunique;
> -       }
> +       /* Use the parent device name as DRM device unique identifier, but fall
> +        * back to the driver name for virtual devices like vgem. */
> +       ret = drm_dev_set_unique(dev, parent ? dev_name(parent) : driver->name);
> +       if (ret)
> +               goto err_setunique;
>
>         return dev;
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 11eda9050215..83892b6b1e0d 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -134,13 +134,7 @@ static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv)
>                         drm_unset_busid(dev, master);
>                         return ret;
>                 }
> -       } else {
> -               if (WARN(dev->unique == NULL,
> -                        "No drm_driver.set_busid() implementation provided by "
> -                        "%ps. Use drm_dev_set_unique() to set the unique "
> -                        "name explicitly.", dev->driver))
> -                       return -EINVAL;
> -
> +       } else if (dev->unique) {
With the drm_drv.c hunk in place this will always evaluate to true,
correct ? Hmmm strictly speaking it could be NULL since vgem/platform
devices call drm_dev_set_unique() and only sun4i checks if the
function has failed.

Is it worth dropping the check here or after (alongside) patch 10 ?

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

Re: [PATCH 11/16] drm: Nuke SET_UNIQUE ioctl

[Emil Velikov]

On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Ever since
>
> commit 2e1868b560315a8b20d688e646c489a5ad93eeae
> Author: Eric Anholt <anholt@freebsd.org>
> Date:   Wed Jun 16 09:25:21 2004 +0000
>
>     DRI trunk-20040613 import
>
> the X server supports drm 1.1, and doesn't even call this ioctl any
> more. When reviewing this note that for hilarity both the
s/and doesn't even call this ioctl any more/thus doesn't call libdrm's
drmSetBusid - the sole user of the said API ioctl./

> kernel-internal functiosn (set_busid) and the libdrm wrapper
s/functiosn/functions/

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

Re: [PATCH 13/16] drm: Refactor drop/set master code a bit

[Emil Velikov]

Hi Daniel,

It doesn't look quite right I'm afraid. This causes a leak plus
there's a small style issue. See below for details.

On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:


> @@ -134,24 +152,21 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
>
>         /* take another reference for the copy in the local file priv */
>         old_master = fpriv->master;
> -       fpriv->master = drm_master_get(dev->master);
> +       fpriv->master = drm_master_create(dev);
> +       if (!fpriv->master)
> +               return -ENOMEM;
>
Now dev->master != fpriv->master

>         if (dev->driver->master_create) {
>                 ret = dev->driver->master_create(dev, fpriv->master);
>                 if (ret)
>                         goto out_err;
>         }
> -       if (dev->driver->master_set) {
> -               ret = dev->driver->master_set(dev, fpriv, true);
> -               if (ret)
> -                       goto out_err;
> -       }
> -
> -       fpriv->is_master = 1;
>         fpriv->allowed_master = 1;
>         fpriv->authenticated = 1;
> -       if (old_master)
> -               drm_master_put(&old_master);
> +
> +       ret = drm_set_master(dev, fpriv, true);

> +static int drm_set_master(struct drm_device *dev, struct drm_file *fpriv,
> +                         bool new_master)
> +{
> +       int ret = 0;
> +
> +       dev->master = drm_master_get(fpriv->master);
drm_master_get() is defined as follows
      kref_get(&master->refcount);
      return master;

Since dev->master != fpriv->master, we'll leak the former.


> +void drm_drop_master(struct drm_device *dev,
> +                    struct drm_file *fpriv)
Function is used only locally thus it should be static.

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

Re: [PATCH 15/16] drm: Clear up master tracking booleans

[Emil Velikov]

On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> @@ -315,10 +313,10 @@ struct drm_file {
>         /* true if client understands atomic properties */
>         unsigned atomic:1;
>         /*
> -        * This client is allowed to gain master privileges for @master.
> +        * This client is the create the master.
You've missed a word here.

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

Re: [PATCH 16/16] drm: document drm_auth.c

[Emil Velikov]

On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Also extract drm_auth.h for nicer grouping.
>
> v2: Nuke the other comments since they don't really explain a lot, and
> within the drm core we generally only document functions exported to
> drivers: The main audience for these docs are driver writers.
>
> v3: Limit the exposure of drm_master internals by only including
> drm_auth.h where it is neede (Chris).
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  Documentation/DocBook/gpu.tmpl      |  6 ++++
>  drivers/gpu/drm/drm_auth.c          | 69 +++++++++++++++++++++----------------
>  drivers/gpu/drm/drm_crtc.c          |  1 +
>  drivers/gpu/drm/drm_ioctl.c         |  1 +
>  drivers/gpu/drm/i915/i915_drv.h     |  1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h |  1 +
>  include/drm/drmP.h                  | 30 +---------------
>  include/drm/drm_auth.h              | 59 +++++++++++++++++++++++++++++++
>  include/drm/drm_legacy.h            |  2 ++
>  9 files changed, 112 insertions(+), 58 deletions(-)
>  create mode 100644 include/drm/drm_auth.h
>
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index 94c6bdee8080..b7f6316b7bee 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -3103,6 +3103,12 @@ int num_ioctls;</synopsis>
>  !Pdrivers/gpu/drm/drm_vma_manager.c getunique and setversion story
>      </sect1>
>      <sect1>
> +      <title>Primary Nodes, DRM Master and Authentication</title>
> +!Pdrivers/gpu/drm_auth.c master and authentication
> +!Edrivers/gpu/drm_auth.c
> +!Einclude/drm/drm_auth.h
> +    </sect1>
> +    <sect1>
>        <title>Render nodes</title>
>        <para>
>          DRM core provides multiple character-devices for user-space to use.
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index b4dfa8ab20d7..3774b9964dbe 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -32,18 +32,27 @@
>  #include "drm_internal.h"
>  #include "drm_legacy.h"
>
> -/**
> - * drm_getmagic - Get unique magic of a client
> - * @dev: DRM device to operate on
> - * @data: ioctl data containing the drm_auth object
> - * @file_priv: DRM file that performs the operation
> +/** DOC: master and authentication
> + *
> + * struct &drm_master is used to track groups of clients with open
> + * primary/legacy device nodes. For every struct &drm_file which at least once
s/which at least/which has had at least/

> + * successfully became the device master (either through the SET_MASTER IOCTL,
> + * or implicitly through opening the primary device node when no one else is the
> + * current master that time) there exists one &drm_master. This is noted in the/
> + * is_master member of &drm_master. All other clients have just a pointer to the
s/member of &drm_master/member of &drm_file/

> + * &drm_master they are associated with.
>   *
> - * This looks up the unique magic of the passed client and returns it. If the
> - * client did not have a magic assigned, yet, a new one is registered. The magic
> - * is stored in the passed drm_auth object.
> + * In addition only one &drm_master can be the current master for a &drm_device.
> + * It can be switched through the DROP_MASTER and SET_MASTER IOCTL, or
> + * implicitly through closing/openeing the primary device node. See also
> + * drm_is_current_master().
>   *
> - * Returns: 0 on success, negative error code on failure.
> + * Clients can authenticate against the current master (if it matches their own)
> + * using the GETMAGIC and AUTHMAGIC IOCTLs. Together with exchanging masters,
> + * this allows controlled access to the device for an entire group of mutually
> + * trusted clients.
>   */
> +
>  int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv)
>  {
>         struct drm_auth *auth = data;
> @@ -64,16 +73,6 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv)
>         return ret < 0 ? ret : 0;
>  }
>
> -/**
> - * drm_authmagic - Authenticate client with a magic
> - * @dev: DRM device to operate on
> - * @data: ioctl data containing the drm_auth object
> - * @file_priv: DRM file that performs the operation
> - *
> - * This looks up a DRM client by the passed magic and authenticates it.
> - *
> - * Returns: 0 on success, negative error code on failure.
> - */
Why is this and drm_getmagic()'s documetation going away ? Kernel doc
isn't restricted to EXPORTED_SYMBOL(s) only, is it ?

>  int drm_authmagic(struct drm_device *dev, void *data,
>                   struct drm_file *file_priv)
>  {
> @@ -126,16 +125,6 @@ static int drm_set_master(struct drm_device *dev, struct drm_file *fpriv,
>         return ret;
>  }
>
> -/*
> - * drm_new_set_master - Allocate a new master object and become master for the
> - * associated master realm.
> - *
> - * @dev: The associated device.
> - * @fpriv: File private identifying the client.
> - *
> - * This function must be called with dev::master_mutex held.
> - * Returns negative error code on failure. Zero on success.
> - */
>  static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
>  {
>         struct drm_master *old_master;
> @@ -288,12 +277,28 @@ out:
>         mutex_unlock(&dev->master_mutex);
>  }
>
> +/**
> + * drm_is_current_master - checks whether this master is the current one
s/this master is the current one/@fpriv is the current master/ perhaps ?

> + * @fpriv: DRM file private
> + *
> + * Checks whether @fpriv is a master and that it is the current master on its
s/a master and that it is the current/the current/

> + * device. This decides whether a client is allowed to run DRM_MASTER IOCTLs.
> + *
> + * Most of the modern IOCTL which require DRM_MASTER are for kernel modesetting
> + * - the current master is assumed to own the non-shareable display hardware.
> + */
>  bool drm_is_current_master(struct drm_file *fpriv)
>  {
>         return fpriv->is_master && fpriv->master == fpriv->minor->dev->master;
>  }
>  EXPORT_SYMBOL(drm_is_current_master);
>
> +/**
> + * drm_master_get - reference a master pointer
> + * @master: struct &drm_master
> + *
> + * Increments the reference count of @master.
Bikeshed: s/@master./@master and returns a pointer to @master./

> + */
>  struct drm_master *drm_master_get(struct drm_master *master)
>  {
>         kref_get(&master->refcount);
> @@ -316,6 +321,12 @@ static void drm_master_destroy(struct kref *kref)
>         kfree(master);
>  }
>
> +/**
> + * drm_master_put - unreference and clear a master pointer
> + * @master: pointer to a pointer of struct &drm_master
> + *
> + * This decrements the &drm_master behind @master and sets it to NULL.
> + */
>  void drm_master_put(struct drm_master **master)
>  {
>         kref_put(&(*master)->refcount, drm_master_destroy);
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 81083f98d155..871af372662d 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -39,6 +39,7 @@
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_modeset_lock.h>
>  #include <drm/drm_atomic.h>
> +#include <drm/drm_auth.h>
>
>  #include "drm_crtc_internal.h"
>  #include "drm_internal.h"
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index a0c1d172954d..88796a383e40 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -30,6 +30,7 @@
>
>  #include <drm/drmP.h>
>  #include <drm/drm_core.h>
> +#include <drm/drm_auth.h>
>  #include "drm_legacy.h"
>  #include "drm_internal.h"
>  #include "drm_crtc_internal.h"
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9fa9698fe247..0f8632c93e95 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -47,6 +47,7 @@
>  #include <drm/intel-gtt.h>
>  #include <drm/drm_legacy.h> /* for struct drm_dma_handle */
>  #include <drm/drm_gem.h>
> +#include <drm/drm_auth.h>
>
>  #include "i915_params.h"
>  #include "i915_reg.h"
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 1980e2a28265..9a90f824814e 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -32,6 +32,7 @@
>  #include <drm/drmP.h>
>  #include <drm/vmwgfx_drm.h>
>  #include <drm/drm_hashtab.h>
> +#include <drm/drm_auth.h>
>  #include <linux/suspend.h>
>  #include <drm/ttm/ttm_bo_driver.h>
>  #include <drm/ttm/ttm_object.h>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 761b20332321..d22ba6bf2299 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -86,6 +86,7 @@ struct drm_local_map;
>  struct drm_device_dma;
>  struct drm_dma_handle;
>  struct drm_gem_object;
> +struct drm_master;
>
>  struct device_node;
>  struct videomode;
> @@ -373,30 +374,6 @@ struct drm_lock_data {
>         int idle_has_lock;
>  };
>
> -/**
> - * struct drm_master - drm master structure
> - *
> - * @refcount: Refcount for this master object.
> - * @dev: Link back to the DRM device
> - * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex.
> - * @unique_len: Length of unique field. Protected by drm_global_mutex.
> - * @magic_map: Map of used authentication tokens. Protected by struct_mutex.
> - * @lock: DRI lock information.
> - * @driver_priv: Pointer to driver-private information.
> - *
> - * Note that master structures are only relevant for the legacy/primary device
> - * nodes, hence there can only be one per device, not one per drm_minor.
> - */
> -struct drm_master {
> -       struct kref refcount;
> -       struct drm_device *dev;
> -       char *unique;
> -       int unique_len;
> -       struct idr magic_map;
> -       struct drm_lock_data lock;
> -       void *driver_priv;
> -};
> -
>  /* Flags and return codes for get_vblank_timestamp() driver function. */
>  #define DRM_CALLED_FROM_VBLIRQ 1
>  #define DRM_VBLANKTIME_SCANOUTPOS_METHOD (1 << 0)
> @@ -1008,11 +985,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc
>         return &crtc->dev->vblank[drm_crtc_index(crtc)].queue;
>  }
>
> -/* drm_auth.c */
> -struct drm_master *drm_master_get(struct drm_master *master);
> -void drm_master_put(struct drm_master **master);
> -bool drm_is_current_master(struct drm_file *fpriv);
> -
>  /* drm_drv.c */
>  void drm_put_dev(struct drm_device *dev);
>  void drm_unplug_dev(struct drm_device *dev);
> diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
> new file mode 100644
> index 000000000000..610223b0481b
> --- /dev/null
> +++ b/include/drm/drm_auth.h
> @@ -0,0 +1,59 @@
> +/*
> + * Internal Header for the Direct Rendering Manager
> + *
> + * Copyright 2016 Intel Corporation
> + *
> + * Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#ifndef _DRM_AUTH_H_
> +#define _DRM_AUTH_H_
> +
> +/**
> + * struct drm_master - drm master structure
> + *
> + * @refcount: Refcount for this master object.
> + * @dev: Link back to the DRM device
> + * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex.
> + * @unique_len: Length of unique field. Protected by drm_global_mutex.
> + * @magic_map: Map of used authentication tokens. Protected by struct_mutex.
> + * @lock: DRI lock information.
> + * @driver_priv: Pointer to driver-private information.
> + *
> + * Note that master structures are only relevant for the legacy/primary device
> + * nodes, hence there can only be one per device, not one per drm_minor.
> + */
> +struct drm_master {
> +       struct kref refcount;
> +       struct drm_device *dev;
> +       char *unique;
> +       int unique_len;
> +       struct idr magic_map;
> +       struct drm_lock_data lock;
> +       void *driver_priv;
> +};
> +
> +struct drm_master *drm_master_get(struct drm_master *master);
> +void drm_master_put(struct drm_master **master);
> +bool drm_is_current_master(struct drm_file *fpriv);
> +
> +#endif
> diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
> index a5ef2c7e40f8..cf0e7d89bcdf 100644
> --- a/include/drm/drm_legacy.h
> +++ b/include/drm/drm_legacy.h
> @@ -1,6 +1,8 @@
>  #ifndef __DRM_DRM_LEGACY_H__
>  #define __DRM_DRM_LEGACY_H__
>
> +#include <drm/drm_auth.h>
> +
>  /*
>   * Legacy driver interfaces for the Direct Rendering Manager
>   *
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 13/16] drm: Refactor drop/set master code a bit

[Emil Velikov]

On 18 June 2016 at 00:00, Emil Velikov <emil.l.velikov@gmail.com> wrote:

>> @@ -134,24 +152,21 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
>>
>>         /* take another reference for the copy in the local file priv */
>>         old_master = fpriv->master;
>> -       fpriv->master = drm_master_get(dev->master);
>> +       fpriv->master = drm_master_create(dev);
To avoid the leak just drop the fpriv->minor->dev->master =
drm_master_create() a few lines above (+ the associated comment/error
handling drm_master_put(fpriv->minor->dev->master))

>> +       if (!fpriv->master)
>> +               return -ENOMEM;
>>
... and restore the old_master before the return "fpriv->master = old_master;"

With that and the other minor comments the series is
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>

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

Re: [PATCH 10/16] drm: Don't call drm_dev_set_unique from platform drivers

[Maxime Ripard]

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

On Fri, Jun 17, 2016 at 02:12:09PM +0300, Laurent Pinchart wrote:
> Hi Daniel,
> 
> Thank you for the patch.
> 
> On Friday 17 Jun 2016 09:33:28 Daniel Vetter wrote:
> > Since
> > 
> > commit e112e593b215c394c0303dbf0534db0928e87967
> > Author: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> > Date:   Fri Dec 11 11:20:28 2015 +0100
> > 
> >     drm: use dev_name as default unique name in drm_dev_alloc()
> > 
> > we're using a reasonable default which should work for everyone. Only
> > mtk, rcar-du and sun4i are affected, and as kms-only drivers without
> > any rendering support no one should ever care about the unique name
> > 
> > v2: Rebase on top of mediatek.
> > 
> > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Emil Velikov <emil.l.velikov@gmail.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 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 09/16] drm/vgem: Stop calling drm_drv_set_unique

[Chris Wilson]

On Fri, Jun 17, 2016 at 09:33:27AM +0200, Daniel Vetter wrote:
> With the previous patch this is now redudant, the core always
> sets a reasonable dev->unique string.
> 
> Cc: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Will this fix:

[ 4442.886507] ==================================================================
[ 4442.886854] BUG: KASAN: null-ptr-deref on address 0000000000000050
[ 4442.887116] Read of size 8 by task cat/1376
[ 4442.887369] CPU: 1 PID: 1376 Comm: cat Not tainted 4.7.0-rc4+ #356
[ 4442.887692] Hardware name:                  /NUC5CPYB, BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[ 4442.888169]  0000000000000000 ffff88022f057a50 ffffffff8145ebab ffff88022f057ae0
[ 4442.889531]  ffff880234672900 ffff88022f057ad0 ffffffff812509f8 ffff880234672900
[ 4442.890551]  ffffffff8124c214 0000000000000292 ffff88022f057ab0 ffffffff81114554
[ 4442.891561] Call Trace:
[ 4442.891832]  [<ffffffff8145ebab>] dump_stack+0x68/0x9d
[ 4442.892119]  [<ffffffff812509f8>] kasan_report_error+0x438/0x530
[ 4442.892416]  [<ffffffff8124c214>] ? __slab_alloc.constprop.66+0x44/0x70
[ 4442.892710]  [<ffffffff81114554>] ? __lock_is_held+0x84/0xc0
[ 4442.893003]  [<ffffffff81250ee9>] kasan_report+0x39/0x3b
[ 4442.893290]  [<ffffffff8124c300>] ? __kmalloc+0xc0/0x2b0
[ 4442.893578]  [<ffffffff815e2963>] ? drm_name_info+0xf3/0x150
[ 4442.893864]  [<ffffffff8124fa3e>] __asan_load8+0x5e/0x70
[ 4442.894148]  [<ffffffff815e2963>] drm_name_info+0xf3/0x150
[ 4442.894436]  [<ffffffff81294085>] seq_read+0x1f5/0x820
[ 4442.894727]  [<ffffffff81293e90>] ? seq_hlist_next_percpu+0x120/0x120
[ 4442.895019]  [<ffffffff811f2630>] ? warn_alloc_failed+0x1e0/0x1e0
[ 4442.895314]  [<ffffffff813d7ec5>] ? full_proxy_read+0x5/0xf0
[ 4442.895604]  [<ffffffff813d7f70>] full_proxy_read+0xb0/0xf0
[ 4442.895892]  [<ffffffff813d7ec5>] ? full_proxy_read+0x5/0xf0
[ 4442.896182]  [<ffffffff81254ad7>] __vfs_read+0xd7/0x320
[ 4442.896469]  [<ffffffff81254a00>] ? do_loop_readv_writev+0x120/0x120
[ 4442.896760]  [<ffffffff811185c0>] ? debug_check_no_locks_freed+0x1a0/0x1a0
[ 4442.897063]  [<ffffffff81226c60>] ? copy_page_range+0xc20/0xc20
[ 4442.897352]  [<ffffffff811368aa>] ? debug_lockdep_rcu_enabled.part.4+0x1a/0x30
[ 4442.897694]  [<ffffffff811368f5>] ? debug_lockdep_rcu_enabled+0x35/0x40
[ 4442.897987]  [<ffffffff81256735>] ? rw_verify_area+0x65/0x140
[ 4442.898276]  [<ffffffff812568cc>] vfs_read+0xbc/0x170
[ 4442.898564]  [<ffffffff812586bb>] SyS_read+0xab/0x130
[ 4442.898850]  [<ffffffff81258610>] ? vfs_copy_file_range+0x2f0/0x2f0
[ 4442.899139]  [<ffffffff81118072>] ? trace_hardirqs_on_caller+0x182/0x280
[ 4442.899433]  [<ffffffff8100179a>] ? trace_hardirqs_on_thunk+0x1a/0x1c
[ 4442.899728]  [<ffffffff8181b165>] entry_SYSCALL_64_fastpath+0x18/0xa8
[ 4442.900018]  [<ffffffff81113b20>] ? trace_hardirqs_off_caller+0xc0/0x110
[ 4442.900301] ==================================================================
[ 4442.900603] Disabling lock debugging due to kernel taint
[ 4442.901031] BUG: unable to handle kernel NULL pointer dereference at 0000000000000050
[ 4442.906576] IP: [<ffffffff815e2963>] drm_name_info+0xf3/0x150
[ 4442.906877] PGD 23472f067 PUD 2350f8067 PMD 0 
[ 4442.907418] Oops: 0000 [#1] SMP KASAN
[ 4442.907592] Modules linked in: vgem i915 intel_gtt
[ 4442.908279] CPU: 1 PID: 1376 Comm: cat Tainted: G    B           4.7.0-rc4+ #356
[ 4442.908500] Hardware name:                  /NUC5CPYB, BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[ 4442.908732] task: ffff880234672900 ti: ffff88022f050000 task.ti: ffff88022f050000
[ 4442.908952] RIP: 0010:[<ffffffff815e2963>]  [<ffffffff815e2963>] drm_name_info+0xf3/0x150
[ 4442.909310] RSP: 0018:ffff88022f057b28  EFLAGS: 00010282
[ 4442.909492] RAX: ffff880234672900 RBX: 0000000000000000 RCX: ffffffff81117f06
[ 4442.909680] RDX: 0000000000000004 RSI: 0000000000000003 RDI: ffffffff82181b20
[ 4442.909868] RBP: ffff88022f057b50 R08: 0000000000000003 R09: 0000000000000000
[ 4442.910054] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8802346c55f0
[ 4442.910240] R13: ffff880235a6a000 R14: 0000000000000000 R15: ffff880231f1c7e0
[ 4442.910428] FS:  00007f9349817700(0000) GS:ffff880237700000(0000) knlGS:0000000000000000
[ 4442.910652] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 4442.910834] CR2: 0000000000000050 CR3: 00000002350f6000 CR4: 00000000001006e0
[ 4442.911016] Stack:
[ 4442.911185]  ffff880235a6a000 0000000000000001 ffff880235a6a0c0 0000000000000000
[ 4442.911906]  ffff880231f1c7e0 ffff88022f057ca0 ffffffff81294085 ffff880234673028
[ 4442.912627]  ffff880234672fd8 00007f93497f5000 ffff880235a6a030 ffff88022f057ee0
[ 4442.913349] Call Trace:
[ 4442.913534]  [<ffffffff81294085>] seq_read+0x1f5/0x820
[ 4442.913735]  [<ffffffff81293e90>] ? seq_hlist_next_percpu+0x120/0x120
[ 4442.919906]  [<ffffffff811f2630>] ? warn_alloc_failed+0x1e0/0x1e0
[ 4442.920111]  [<ffffffff813d7ec5>] ? full_proxy_read+0x5/0xf0
[ 4442.920314]  [<ffffffff813d7f70>] full_proxy_read+0xb0/0xf0
[ 4442.920514]  [<ffffffff813d7ec5>] ? full_proxy_read+0x5/0xf0
[ 4442.920715]  [<ffffffff81254ad7>] __vfs_read+0xd7/0x320
[ 4442.920916]  [<ffffffff81254a00>] ? do_loop_readv_writev+0x120/0x120
[ 4442.921119]  [<ffffffff811185c0>] ? debug_check_no_locks_freed+0x1a0/0x1a0
[ 4442.921322]  [<ffffffff81226c60>] ? copy_page_range+0xc20/0xc20
[ 4442.921522]  [<ffffffff811368aa>] ? debug_lockdep_rcu_enabled.part.4+0x1a/0x30
[ 4442.921757]  [<ffffffff811368f5>] ? debug_lockdep_rcu_enabled+0x35/0x40
[ 4442.921961]  [<ffffffff81256735>] ? rw_verify_area+0x65/0x140
[ 4442.922162]  [<ffffffff812568cc>] vfs_read+0xbc/0x170
[ 4442.922360]  [<ffffffff812586bb>] SyS_read+0xab/0x130
[ 4442.922558]  [<ffffffff81258610>] ? vfs_copy_file_range+0x2f0/0x2f0
[ 4442.922758]  [<ffffffff81118072>] ? trace_hardirqs_on_caller+0x182/0x280
[ 4442.922962]  [<ffffffff8100179a>] ? trace_hardirqs_on_thunk+0x1a/0x1c
[ 4442.923163]  [<ffffffff8181b165>] entry_SYSCALL_64_fastpath+0x18/0xa8
[ 4442.923365]  [<ffffffff81113b20>] ? trace_hardirqs_off_caller+0xc0/0x110
[ 4442.923561] Code: 5c 41 5d 41 5e 41 5f 5d c3 48 8d 7b 10 e8 96 d0 c6 ff 4c 8b 7b 10 eb ad e8 8b d0 c6 ff 49 8b 5c 24 18 48 8d 7b 50 e8 7d d0 c6 ff <4c> 8b 73 50 4d 85 f6 74 41 49 8d 7c 24 20 e8 6a d0 c6 ff 49 8b 
[ 4442.937003] RIP  [<ffffffff815e2963>] drm_name_info+0xf3/0x150
[ 4442.937306]  RSP <ffff88022f057b28>
[ 4442.937476] CR2: 0000000000000050
[ 4442.941304] ---[ end trace 7b3b90baf4ed1a85 ]---

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

Re: [PATCH 10/16] drm: Don't call drm_dev_set_unique from platform drivers

[Philipp Zabel]

Am Freitag, den 17.06.2016, 09:33 +0200 schrieb Daniel Vetter:
> Since
> 
> commit e112e593b215c394c0303dbf0534db0928e87967
> Author: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> Date:   Fri Dec 11 11:20:28 2015 +0100
> 
>     drm: use dev_name as default unique name in drm_dev_alloc()
> 
> we're using a reasonable default which should work for everyone. Only
> mtk, rcar-du and sun4i are affected, and as kms-only drivers without
> any rendering support no one should ever care about the unique name
> 
> v2: Rebase on top of mediatek.
> 
> Cc: Philipp Zabel <p.zabel@pengutronix.de>

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

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

Re: [PATCH 09/16] drm/vgem: Stop calling drm_drv_set_unique

[Daniel Vetter]

On Mon, Jun 20, 2016 at 12:42:55PM +0100, Chris Wilson wrote:
> On Fri, Jun 17, 2016 at 09:33:27AM +0200, Daniel Vetter wrote:
> > With the previous patch this is now redudant, the core always
> > sets a reasonable dev->unique string.
> > 
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> Will this fix:

Oh the hilarity. No, this will unfortunately not fix this. And the bug has
been there since forever, since if you use the drmOpenByName (which
doesn't call SET_VERSION which hence might result with master->unique
still NULL).

I think the right fix for this would be to insert another

	else if (dev->unique)

case in drm_name_info. I'll try to type that one.
-Daniel

> 
> [ 4442.886507] ==================================================================
> [ 4442.886854] BUG: KASAN: null-ptr-deref on address 0000000000000050
> [ 4442.887116] Read of size 8 by task cat/1376
> [ 4442.887369] CPU: 1 PID: 1376 Comm: cat Not tainted 4.7.0-rc4+ #356
> [ 4442.887692] Hardware name:                  /NUC5CPYB, BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> [ 4442.888169]  0000000000000000 ffff88022f057a50 ffffffff8145ebab ffff88022f057ae0
> [ 4442.889531]  ffff880234672900 ffff88022f057ad0 ffffffff812509f8 ffff880234672900
> [ 4442.890551]  ffffffff8124c214 0000000000000292 ffff88022f057ab0 ffffffff81114554
> [ 4442.891561] Call Trace:
> [ 4442.891832]  [<ffffffff8145ebab>] dump_stack+0x68/0x9d
> [ 4442.892119]  [<ffffffff812509f8>] kasan_report_error+0x438/0x530
> [ 4442.892416]  [<ffffffff8124c214>] ? __slab_alloc.constprop.66+0x44/0x70
> [ 4442.892710]  [<ffffffff81114554>] ? __lock_is_held+0x84/0xc0
> [ 4442.893003]  [<ffffffff81250ee9>] kasan_report+0x39/0x3b
> [ 4442.893290]  [<ffffffff8124c300>] ? __kmalloc+0xc0/0x2b0
> [ 4442.893578]  [<ffffffff815e2963>] ? drm_name_info+0xf3/0x150
> [ 4442.893864]  [<ffffffff8124fa3e>] __asan_load8+0x5e/0x70
> [ 4442.894148]  [<ffffffff815e2963>] drm_name_info+0xf3/0x150
> [ 4442.894436]  [<ffffffff81294085>] seq_read+0x1f5/0x820
> [ 4442.894727]  [<ffffffff81293e90>] ? seq_hlist_next_percpu+0x120/0x120
> [ 4442.895019]  [<ffffffff811f2630>] ? warn_alloc_failed+0x1e0/0x1e0
> [ 4442.895314]  [<ffffffff813d7ec5>] ? full_proxy_read+0x5/0xf0
> [ 4442.895604]  [<ffffffff813d7f70>] full_proxy_read+0xb0/0xf0
> [ 4442.895892]  [<ffffffff813d7ec5>] ? full_proxy_read+0x5/0xf0
> [ 4442.896182]  [<ffffffff81254ad7>] __vfs_read+0xd7/0x320
> [ 4442.896469]  [<ffffffff81254a00>] ? do_loop_readv_writev+0x120/0x120
> [ 4442.896760]  [<ffffffff811185c0>] ? debug_check_no_locks_freed+0x1a0/0x1a0
> [ 4442.897063]  [<ffffffff81226c60>] ? copy_page_range+0xc20/0xc20
> [ 4442.897352]  [<ffffffff811368aa>] ? debug_lockdep_rcu_enabled.part.4+0x1a/0x30
> [ 4442.897694]  [<ffffffff811368f5>] ? debug_lockdep_rcu_enabled+0x35/0x40
> [ 4442.897987]  [<ffffffff81256735>] ? rw_verify_area+0x65/0x140
> [ 4442.898276]  [<ffffffff812568cc>] vfs_read+0xbc/0x170
> [ 4442.898564]  [<ffffffff812586bb>] SyS_read+0xab/0x130
> [ 4442.898850]  [<ffffffff81258610>] ? vfs_copy_file_range+0x2f0/0x2f0
> [ 4442.899139]  [<ffffffff81118072>] ? trace_hardirqs_on_caller+0x182/0x280
> [ 4442.899433]  [<ffffffff8100179a>] ? trace_hardirqs_on_thunk+0x1a/0x1c
> [ 4442.899728]  [<ffffffff8181b165>] entry_SYSCALL_64_fastpath+0x18/0xa8
> [ 4442.900018]  [<ffffffff81113b20>] ? trace_hardirqs_off_caller+0xc0/0x110
> [ 4442.900301] ==================================================================
> [ 4442.900603] Disabling lock debugging due to kernel taint
> [ 4442.901031] BUG: unable to handle kernel NULL pointer dereference at 0000000000000050
> [ 4442.906576] IP: [<ffffffff815e2963>] drm_name_info+0xf3/0x150
> [ 4442.906877] PGD 23472f067 PUD 2350f8067 PMD 0 
> [ 4442.907418] Oops: 0000 [#1] SMP KASAN
> [ 4442.907592] Modules linked in: vgem i915 intel_gtt
> [ 4442.908279] CPU: 1 PID: 1376 Comm: cat Tainted: G    B           4.7.0-rc4+ #356
> [ 4442.908500] Hardware name:                  /NUC5CPYB, BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> [ 4442.908732] task: ffff880234672900 ti: ffff88022f050000 task.ti: ffff88022f050000
> [ 4442.908952] RIP: 0010:[<ffffffff815e2963>]  [<ffffffff815e2963>] drm_name_info+0xf3/0x150
> [ 4442.909310] RSP: 0018:ffff88022f057b28  EFLAGS: 00010282
> [ 4442.909492] RAX: ffff880234672900 RBX: 0000000000000000 RCX: ffffffff81117f06
> [ 4442.909680] RDX: 0000000000000004 RSI: 0000000000000003 RDI: ffffffff82181b20
> [ 4442.909868] RBP: ffff88022f057b50 R08: 0000000000000003 R09: 0000000000000000
> [ 4442.910054] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8802346c55f0
> [ 4442.910240] R13: ffff880235a6a000 R14: 0000000000000000 R15: ffff880231f1c7e0
> [ 4442.910428] FS:  00007f9349817700(0000) GS:ffff880237700000(0000) knlGS:0000000000000000
> [ 4442.910652] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 4442.910834] CR2: 0000000000000050 CR3: 00000002350f6000 CR4: 00000000001006e0
> [ 4442.911016] Stack:
> [ 4442.911185]  ffff880235a6a000 0000000000000001 ffff880235a6a0c0 0000000000000000
> [ 4442.911906]  ffff880231f1c7e0 ffff88022f057ca0 ffffffff81294085 ffff880234673028
> [ 4442.912627]  ffff880234672fd8 00007f93497f5000 ffff880235a6a030 ffff88022f057ee0
> [ 4442.913349] Call Trace:
> [ 4442.913534]  [<ffffffff81294085>] seq_read+0x1f5/0x820
> [ 4442.913735]  [<ffffffff81293e90>] ? seq_hlist_next_percpu+0x120/0x120
> [ 4442.919906]  [<ffffffff811f2630>] ? warn_alloc_failed+0x1e0/0x1e0
> [ 4442.920111]  [<ffffffff813d7ec5>] ? full_proxy_read+0x5/0xf0
> [ 4442.920314]  [<ffffffff813d7f70>] full_proxy_read+0xb0/0xf0
> [ 4442.920514]  [<ffffffff813d7ec5>] ? full_proxy_read+0x5/0xf0
> [ 4442.920715]  [<ffffffff81254ad7>] __vfs_read+0xd7/0x320
> [ 4442.920916]  [<ffffffff81254a00>] ? do_loop_readv_writev+0x120/0x120
> [ 4442.921119]  [<ffffffff811185c0>] ? debug_check_no_locks_freed+0x1a0/0x1a0
> [ 4442.921322]  [<ffffffff81226c60>] ? copy_page_range+0xc20/0xc20
> [ 4442.921522]  [<ffffffff811368aa>] ? debug_lockdep_rcu_enabled.part.4+0x1a/0x30
> [ 4442.921757]  [<ffffffff811368f5>] ? debug_lockdep_rcu_enabled+0x35/0x40
> [ 4442.921961]  [<ffffffff81256735>] ? rw_verify_area+0x65/0x140
> [ 4442.922162]  [<ffffffff812568cc>] vfs_read+0xbc/0x170
> [ 4442.922360]  [<ffffffff812586bb>] SyS_read+0xab/0x130
> [ 4442.922558]  [<ffffffff81258610>] ? vfs_copy_file_range+0x2f0/0x2f0
> [ 4442.922758]  [<ffffffff81118072>] ? trace_hardirqs_on_caller+0x182/0x280
> [ 4442.922962]  [<ffffffff8100179a>] ? trace_hardirqs_on_thunk+0x1a/0x1c
> [ 4442.923163]  [<ffffffff8181b165>] entry_SYSCALL_64_fastpath+0x18/0xa8
> [ 4442.923365]  [<ffffffff81113b20>] ? trace_hardirqs_off_caller+0xc0/0x110
> [ 4442.923561] Code: 5c 41 5d 41 5e 41 5f 5d c3 48 8d 7b 10 e8 96 d0 c6 ff 4c 8b 7b 10 eb ad e8 8b d0 c6 ff 49 8b 5c 24 18 48 8d 7b 50 e8 7d d0 c6 ff <4c> 8b 73 50 4d 85 f6 74 41 49 8d 7c 24 20 e8 6a d0 c6 ff 49 8b 
> [ 4442.937003] RIP  [<ffffffff815e2963>] drm_name_info+0xf3/0x150
> [ 4442.937306]  RSP <ffff88022f057b28>
> [ 4442.937476] CR2: 0000000000000050
> [ 4442.941304] ---[ end trace 7b3b90baf4ed1a85 ]---
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
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

Re: [PATCH 12/16] drm: Lobotomize set_busid nonsense for !pci drivers

[Emil Velikov]

On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -522,7 +522,6 @@ static struct drm_driver kms_driver = {
>         .preclose = amdgpu_driver_preclose_kms,
>         .postclose = amdgpu_driver_postclose_kms,
>         .lastclose = amdgpu_driver_lastclose_kms,
> -       .set_busid = drm_pci_set_busid,
This hunk shouldn't be here.

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

Re: [Intel-gfx] [PATCH 07/16] drm: Clean up drm_crtc.h

[Daniel Vetter]

On Fri, Jun 17, 2016 at 08:53:11AM +0100, Chris Wilson wrote:
> On Fri, Jun 17, 2016 at 09:33:25AM +0200, Daniel Vetter wrote:
> > - Group declarations for separate files (drm_bridge.c, drm_edid.c)
> > - Move declarations only used within drm.ko to drm_crtc_internal.h
> > - drm_property_type_valid to drm_crtc.c, its only callsite
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Merged up to this patch to drm-misc, thanks for the review.
-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

Re: [PATCH 13/16] drm: Refactor drop/set master code a bit

[Daniel Vetter]

On Sat, Jun 18, 2016 at 12:47:21AM +0100, Emil Velikov wrote:
> On 18 June 2016 at 00:00, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> 
> >> @@ -134,24 +152,21 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
> >>
> >>         /* take another reference for the copy in the local file priv */
> >>         old_master = fpriv->master;
> >> -       fpriv->master = drm_master_get(dev->master);
> >> +       fpriv->master = drm_master_create(dev);
> To avoid the leak just drop the fpriv->minor->dev->master =
> drm_master_create() a few lines above (+ the associated comment/error
> handling drm_master_put(fpriv->minor->dev->master))
> 
> >> +       if (!fpriv->master)
> >> +               return -ENOMEM;
> >>
> ... and restore the old_master before the return "fpriv->master = old_master;"
> 
> With that and the other minor comments the series is
> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>

This one is a real mess, I've redone it but don't feel like I can just add
your r-b. I'll resend again.
-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

Re: [PATCH 16/16] drm: document drm_auth.c

[Daniel Vetter]

On Sat, Jun 18, 2016 at 12:46:38AM +0100, Emil Velikov wrote:
> On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > @@ -64,16 +73,6 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv)
> >         return ret < 0 ? ret : 0;
> >  }
> >
> > -/**
> > - * drm_authmagic - Authenticate client with a magic
> > - * @dev: DRM device to operate on
> > - * @data: ioctl data containing the drm_auth object
> > - * @file_priv: DRM file that performs the operation
> > - *
> > - * This looks up a DRM client by the passed magic and authenticates it.
> > - *
> > - * Returns: 0 on success, negative error code on failure.
> > - */
> Why is this and drm_getmagic()'s documetation going away ? Kernel doc
> isn't restricted to EXPORTED_SYMBOL(s) only, is it ?

No, but imo the documentation for the drm core&helpers should be aimed at
driver writers. And driver writers can only use EXPORT_SYMBOL stuff (or
from headers), which is why I think it's good to kick out everything else
to avoid too much clutter. It's already a daunting thing to get started
with a new drm driver.

Of course we can keep the comments as normal comments, but for these here
I didn't see the value.

Also note that this is just for the drm core/helpers. In the i915 driver
documentation we instead try to document non-static stuff (since that's
exposed to other parts), but just as a rough guideline. Since often our
source file split doesn't make that much sense, or is too monolithic.

In both cases the goal is to give a useful guideline to users of a piece
of code (calling it or otherwise using), not developers changing the
implementation details themselves.
-Daniel

> 
> >  int drm_authmagic(struct drm_device *dev, void *data,
> >                   struct drm_file *file_priv)
> >  {
> > @@ -126,16 +125,6 @@ static int drm_set_master(struct drm_device *dev, struct drm_file *fpriv,
> >         return ret;
> >  }
> >
> > -/*
> > - * drm_new_set_master - Allocate a new master object and become master for the
> > - * associated master realm.
> > - *
> > - * @dev: The associated device.
> > - * @fpriv: File private identifying the client.
> > - *
> > - * This function must be called with dev::master_mutex held.
> > - * Returns negative error code on failure. Zero on success.
> > - */
> >  static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv)
> >  {
> >         struct drm_master *old_master;
> > @@ -288,12 +277,28 @@ out:
> >         mutex_unlock(&dev->master_mutex);
> >  }
> >
> > +/**
> > + * drm_is_current_master - checks whether this master is the current one
> s/this master is the current one/@fpriv is the current master/ perhaps ?
> 
> > + * @fpriv: DRM file private
> > + *
> > + * Checks whether @fpriv is a master and that it is the current master on its
> s/a master and that it is the current/the current/
> 
> > + * device. This decides whether a client is allowed to run DRM_MASTER IOCTLs.
> > + *
> > + * Most of the modern IOCTL which require DRM_MASTER are for kernel modesetting
> > + * - the current master is assumed to own the non-shareable display hardware.
> > + */
> >  bool drm_is_current_master(struct drm_file *fpriv)
> >  {
> >         return fpriv->is_master && fpriv->master == fpriv->minor->dev->master;
> >  }
> >  EXPORT_SYMBOL(drm_is_current_master);
> >
> > +/**
> > + * drm_master_get - reference a master pointer
> > + * @master: struct &drm_master
> > + *
> > + * Increments the reference count of @master.
> Bikeshed: s/@master./@master and returns a pointer to @master./
> 
> > + */
> >  struct drm_master *drm_master_get(struct drm_master *master)
> >  {
> >         kref_get(&master->refcount);
> > @@ -316,6 +321,12 @@ static void drm_master_destroy(struct kref *kref)
> >         kfree(master);
> >  }
> >
> > +/**
> > + * drm_master_put - unreference and clear a master pointer
> > + * @master: pointer to a pointer of struct &drm_master
> > + *
> > + * This decrements the &drm_master behind @master and sets it to NULL.
> > + */
> >  void drm_master_put(struct drm_master **master)
> >  {
> >         kref_put(&(*master)->refcount, drm_master_destroy);
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 81083f98d155..871af372662d 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -39,6 +39,7 @@
> >  #include <drm/drm_fourcc.h>
> >  #include <drm/drm_modeset_lock.h>
> >  #include <drm/drm_atomic.h>
> > +#include <drm/drm_auth.h>
> >
> >  #include "drm_crtc_internal.h"
> >  #include "drm_internal.h"
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index a0c1d172954d..88796a383e40 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -30,6 +30,7 @@
> >
> >  #include <drm/drmP.h>
> >  #include <drm/drm_core.h>
> > +#include <drm/drm_auth.h>
> >  #include "drm_legacy.h"
> >  #include "drm_internal.h"
> >  #include "drm_crtc_internal.h"
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 9fa9698fe247..0f8632c93e95 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -47,6 +47,7 @@
> >  #include <drm/intel-gtt.h>
> >  #include <drm/drm_legacy.h> /* for struct drm_dma_handle */
> >  #include <drm/drm_gem.h>
> > +#include <drm/drm_auth.h>
> >
> >  #include "i915_params.h"
> >  #include "i915_reg.h"
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> > index 1980e2a28265..9a90f824814e 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> > @@ -32,6 +32,7 @@
> >  #include <drm/drmP.h>
> >  #include <drm/vmwgfx_drm.h>
> >  #include <drm/drm_hashtab.h>
> > +#include <drm/drm_auth.h>
> >  #include <linux/suspend.h>
> >  #include <drm/ttm/ttm_bo_driver.h>
> >  #include <drm/ttm/ttm_object.h>
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 761b20332321..d22ba6bf2299 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -86,6 +86,7 @@ struct drm_local_map;
> >  struct drm_device_dma;
> >  struct drm_dma_handle;
> >  struct drm_gem_object;
> > +struct drm_master;
> >
> >  struct device_node;
> >  struct videomode;
> > @@ -373,30 +374,6 @@ struct drm_lock_data {
> >         int idle_has_lock;
> >  };
> >
> > -/**
> > - * struct drm_master - drm master structure
> > - *
> > - * @refcount: Refcount for this master object.
> > - * @dev: Link back to the DRM device
> > - * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex.
> > - * @unique_len: Length of unique field. Protected by drm_global_mutex.
> > - * @magic_map: Map of used authentication tokens. Protected by struct_mutex.
> > - * @lock: DRI lock information.
> > - * @driver_priv: Pointer to driver-private information.
> > - *
> > - * Note that master structures are only relevant for the legacy/primary device
> > - * nodes, hence there can only be one per device, not one per drm_minor.
> > - */
> > -struct drm_master {
> > -       struct kref refcount;
> > -       struct drm_device *dev;
> > -       char *unique;
> > -       int unique_len;
> > -       struct idr magic_map;
> > -       struct drm_lock_data lock;
> > -       void *driver_priv;
> > -};
> > -
> >  /* Flags and return codes for get_vblank_timestamp() driver function. */
> >  #define DRM_CALLED_FROM_VBLIRQ 1
> >  #define DRM_VBLANKTIME_SCANOUTPOS_METHOD (1 << 0)
> > @@ -1008,11 +985,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc
> >         return &crtc->dev->vblank[drm_crtc_index(crtc)].queue;
> >  }
> >
> > -/* drm_auth.c */
> > -struct drm_master *drm_master_get(struct drm_master *master);
> > -void drm_master_put(struct drm_master **master);
> > -bool drm_is_current_master(struct drm_file *fpriv);
> > -
> >  /* drm_drv.c */
> >  void drm_put_dev(struct drm_device *dev);
> >  void drm_unplug_dev(struct drm_device *dev);
> > diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
> > new file mode 100644
> > index 000000000000..610223b0481b
> > --- /dev/null
> > +++ b/include/drm/drm_auth.h
> > @@ -0,0 +1,59 @@
> > +/*
> > + * Internal Header for the Direct Rendering Manager
> > + *
> > + * Copyright 2016 Intel Corporation
> > + *
> > + * Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > + * OTHER DEALINGS IN THE SOFTWARE.
> > + */
> > +
> > +#ifndef _DRM_AUTH_H_
> > +#define _DRM_AUTH_H_
> > +
> > +/**
> > + * struct drm_master - drm master structure
> > + *
> > + * @refcount: Refcount for this master object.
> > + * @dev: Link back to the DRM device
> > + * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex.
> > + * @unique_len: Length of unique field. Protected by drm_global_mutex.
> > + * @magic_map: Map of used authentication tokens. Protected by struct_mutex.
> > + * @lock: DRI lock information.
> > + * @driver_priv: Pointer to driver-private information.
> > + *
> > + * Note that master structures are only relevant for the legacy/primary device
> > + * nodes, hence there can only be one per device, not one per drm_minor.
> > + */
> > +struct drm_master {
> > +       struct kref refcount;
> > +       struct drm_device *dev;
> > +       char *unique;
> > +       int unique_len;
> > +       struct idr magic_map;
> > +       struct drm_lock_data lock;
> > +       void *driver_priv;
> > +};
> > +
> > +struct drm_master *drm_master_get(struct drm_master *master);
> > +void drm_master_put(struct drm_master **master);
> > +bool drm_is_current_master(struct drm_file *fpriv);
> > +
> > +#endif
> > diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
> > index a5ef2c7e40f8..cf0e7d89bcdf 100644
> > --- a/include/drm/drm_legacy.h
> > +++ b/include/drm/drm_legacy.h
> > @@ -1,6 +1,8 @@
> >  #ifndef __DRM_DRM_LEGACY_H__
> >  #define __DRM_DRM_LEGACY_H__
> >
> > +#include <drm/drm_auth.h>
> > +
> >  /*
> >   * Legacy driver interfaces for the Direct Rendering Manager
> >   *
> > --
> > 2.8.1
> >
> > _______________________________________________
> > 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

Re: [PATCH 16/16] drm: document drm_auth.c

[Emil Velikov]

On 20 June 2016 at 22:17, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Sat, Jun 18, 2016 at 12:46:38AM +0100, Emil Velikov wrote:
>> On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> > @@ -64,16 +73,6 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv)
>> >         return ret < 0 ? ret : 0;
>> >  }
>> >
>> > -/**
>> > - * drm_authmagic - Authenticate client with a magic
>> > - * @dev: DRM device to operate on
>> > - * @data: ioctl data containing the drm_auth object
>> > - * @file_priv: DRM file that performs the operation
>> > - *
>> > - * This looks up a DRM client by the passed magic and authenticates it.
>> > - *
>> > - * Returns: 0 on success, negative error code on failure.
>> > - */
>> Why is this and drm_getmagic()'s documetation going away ? Kernel doc
>> isn't restricted to EXPORTED_SYMBOL(s) only, is it ?
>
> No, but imo the documentation for the drm core&helpers should be aimed at
> driver writers. And driver writers can only use EXPORT_SYMBOL stuff (or
> from headers), which is why I think it's good to kick out everything else
> to avoid too much clutter. It's already a daunting thing to get started
> with a new drm driver.
>
> Of course we can keep the comments as normal comments, but for these here
> I didn't see the value.
>
> Also note that this is just for the drm core/helpers. In the i915 driver
> documentation we instead try to document non-static stuff (since that's
> exposed to other parts), but just as a rough guideline. Since often our
> source file split doesn't make that much sense, or is too monolithic.
>
> In both cases the goal is to give a useful guideline to users of a piece
> of code (calling it or otherwise using), not developers changing the
> implementation details themselves.

Indeed... Having this (and similar internal/implementation details) as
the kernel doc will bring volume about something that most devs cannot
and should not care about, and is likely confuse them. If at all, one
could keep them as normal comments. Don't feel to strongly about them,
just wanted to educate myself a bit on the topic (dos and don'ts)

Thanks a million !
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel