Hi David,

Would you have time to document the master_set operation in Documentation/DocBook/drm.tmpl ? :-)

On Sunday 07 October 2012 19:53:26 David Herrmann wrote:

We link every DRM "file_priv" to a "drm_master" structure. Currently, the drmSetMaster() call returns 0 when there is _any_ active master associated with the "drm_master" structure of the calling "file_priv". This means, that after drmSetMaster() we are not guaranteed to be DRM-Master and might not be able to perform mode-setting.

A way to reproduce this is by starting weston with the DRM backend from within an X-console (eg., xterm). Because the xserver's "drm_master" is currently active, weston is assigned to the same master but is inactive because its VT is inactive and the xserver is still active. But when "fake-activating" weston, it calls drmSetMaster(). With current behavior this returns "0/success" and weston thinks that it is DRM-Master, even though it is not (as the xserver is still DRM-Master). Expected behavior would be drmSetMaster() to return -EINVAL, because the xserver is still DRM-Master. This patch changes exactly that.

The only way this bogus behavior would be useful is for clients to check whether their associated "drm_master" is currently the active DRM-Master. But this logic fails if no DRM-Master is currently active at all. Because then the client itself would become DRM-Master (if it is root) and this makes this whole thing useles.

Also note that the second "if-condition": file_priv->minor->master != file_priv->master is always true and can be skipped.

Signed-off-by: David Herrmann dh.herrmann@googlemail.com

Note: Note that this only removes the "if-clause". The code that performs the setmaster() is actually left unchanged but makes the patch look scarier than it really is.

drivers/gpu/drm/drm_stub.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index c236fd2..581e61d 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -221,20 +221,20 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data, if (!file_priv->master) return -EINVAL;

• if (!file_priv->minor->master &&

•    file_priv->minor->master != file_priv->master) {
  

• mutex_lock(&dev->struct_mutex);
  

• file_priv->minor->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);
  

• 	}
  

• if (file_priv->minor->master)

• return -EINVAL;
  

• mutex_lock(&dev->struct_mutex);
• file_priv->minor->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);
  

  }

• mutex_unlock(&dev->struct_mutex);
  

  }

• mutex_unlock(&dev->struct_mutex);

  return 0;

}