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@xxxxxxxxxxxxxx>
> ---
> 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;
> }
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
