[RFC 0/1] Add support for server managed fds

[RFC 0/1] Add support for server managed fds

[Hans de Goede]

Hi All,

Here is a patch to add support for server managed fds to xf86-video-intel
this is RFC only atm since not all related server patches have landed yet.

With the server patches + this patch it is possible to run Xorg without
root-rights on systemd-logind using systems.

Regards,

Hans

[PATCH] intel: Add support for server managed fds

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 src/intel_device.c | 19 ++++++-------------
 src/intel_module.c |  4 ++++
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/intel_device.c b/src/intel_device.c
index d0c8092..b19884c 100644
--- a/src/intel_device.c
+++ b/src/intel_device.c
@@ -240,19 +240,10 @@ static char *get_path(struct xf86_platform_device *dev)
 #endif
 
 
-#if defined(ODEV_ATTRIB_FD) && 0
+#if defined(ODEV_ATTRIB_FD)
 static int get_fd(struct xf86_platform_device *dev)
 {
-	const char *str;
-
-	if (dev == NULL)
-		return -1;
-
-	str = xf86_get_platform_device_attrib(dev, ODEV_ATTRIB_FD);
-	if (str == NULL)
-		return -1;
-
-	return atoi(str);
+	return xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_FD, -1);
 }
 
 #else
@@ -270,7 +261,7 @@ int intel_open_device(int entity_num,
 {
 	struct intel_device *dev;
 	char *local_path;
-	int fd;
+	int fd, init_master_count = 0;
 
 	if (intel_device_key == -1)
 		intel_device_key = xf86AllocateEntityPrivateIndex();
@@ -286,6 +277,8 @@ int intel_open_device(int entity_num,
 	fd = get_fd(platform);
 	if (fd == -1)
 		fd = __intel_open_device(pci, &local_path);
+	else
+		init_master_count = 1; /* Server fd is already master */
 	if (fd == -1)
 		goto err_path;
 
@@ -298,7 +291,7 @@ int intel_open_device(int entity_num,
 
 	dev->fd = fd;
 	dev->open_count = 0;
-	dev->master_count = 0;
+	dev->master_count = init_master_count;
 	dev->master_node = local_path;
 	dev->render_node = find_render_node(fd);
 	if (dev->render_node == NULL)
diff --git a/src/intel_module.c b/src/intel_module.c
index 51de62a..02062d1 100644
--- a/src/intel_module.c
+++ b/src/intel_module.c
@@ -419,6 +419,10 @@ static Bool intel_driver_func(ScrnInfoPtr pScrn,
 #endif
 
 		return TRUE;
+#if XORG_VERSION_CURRENT > XORG_VERSION_NUMERIC(1,15,99,0,0)
+	case SUPPORTS_SERVER_FDS:
+		return TRUE;	
+#endif
 	default:
 		/* Unknown or deprecated function */
 		return FALSE;
-- 
1.9.0

Re: [PATCH] intel: Add support for server managed fds

[Chris Wilson]

On Fri, Mar 07, 2014 at 02:13:38PM +0100, Hans de Goede wrote:
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  src/intel_device.c | 19 ++++++-------------
>  src/intel_module.c |  4 ++++
>  2 files changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/src/intel_device.c b/src/intel_device.c
> index d0c8092..b19884c 100644
> --- a/src/intel_device.c
> +++ b/src/intel_device.c
> @@ -240,19 +240,10 @@ static char *get_path(struct xf86_platform_device *dev)
>  #endif
>  
>  
> -#if defined(ODEV_ATTRIB_FD) && 0
> +#if defined(ODEV_ATTRIB_FD)
>  static int get_fd(struct xf86_platform_device *dev)
>  {
> -	const char *str;
> -
> -	if (dev == NULL)
> -		return -1;
> -
> -	str = xf86_get_platform_device_attrib(dev, ODEV_ATTRIB_FD);
> -	if (str == NULL)
> -		return -1;
> -
> -	return atoi(str);
> +	return xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_FD, -1);
>  }
>  
>  #else
> @@ -270,7 +261,7 @@ int intel_open_device(int entity_num,
>  {
>  	struct intel_device *dev;
>  	char *local_path;
> -	int fd;
> +	int fd, init_master_count = 0;
>  
>  	if (intel_device_key == -1)
>  		intel_device_key = xf86AllocateEntityPrivateIndex();
> @@ -286,6 +277,8 @@ int intel_open_device(int entity_num,
>  	fd = get_fd(platform);
>  	if (fd == -1)
>  		fd = __intel_open_device(pci, &local_path);
> +	else
> +		init_master_count = 1; /* Server fd is already master */

If we set master_count to non-zero, we won't ever call drmDropMaster
(since our get/put will be unbalanced). Does the server manage
DRM_MASTER across VT switches for this fd?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] intel: Add support for server managed fds

[Hans de Goede]

Hi,

On 03/07/2014 02:18 PM, Chris Wilson wrote:
> On Fri, Mar 07, 2014 at 02:13:38PM +0100, Hans de Goede wrote:
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  src/intel_device.c | 19 ++++++-------------
>>  src/intel_module.c |  4 ++++
>>  2 files changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/intel_device.c b/src/intel_device.c
>> index d0c8092..b19884c 100644
>> --- a/src/intel_device.c
>> +++ b/src/intel_device.c
>> @@ -240,19 +240,10 @@ static char *get_path(struct xf86_platform_device *dev)
>>  #endif
>>  
>>  
>> -#if defined(ODEV_ATTRIB_FD) && 0
>> +#if defined(ODEV_ATTRIB_FD)
>>  static int get_fd(struct xf86_platform_device *dev)
>>  {
>> -	const char *str;
>> -
>> -	if (dev == NULL)
>> -		return -1;
>> -
>> -	str = xf86_get_platform_device_attrib(dev, ODEV_ATTRIB_FD);
>> -	if (str == NULL)
>> -		return -1;
>> -
>> -	return atoi(str);
>> +	return xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_FD, -1);
>>  }
>>  
>>  #else
>> @@ -270,7 +261,7 @@ int intel_open_device(int entity_num,
>>  {
>>  	struct intel_device *dev;
>>  	char *local_path;
>> -	int fd;
>> +	int fd, init_master_count = 0;
>>  
>>  	if (intel_device_key == -1)
>>  		intel_device_key = xf86AllocateEntityPrivateIndex();
>> @@ -286,6 +277,8 @@ int intel_open_device(int entity_num,
>>  	fd = get_fd(platform);
>>  	if (fd == -1)
>>  		fd = __intel_open_device(pci, &local_path);
>> +	else
>> +		init_master_count = 1; /* Server fd is already master */
> 
> If we set master_count to non-zero, we won't ever call drmDropMaster
> (since our get/put will be unbalanced). Does the server manage
> DRM_MASTER across VT switches for this fd?

Yes (*), so the never calling drmDropMaster is the intention.

Regards,

Hans

*) Well actually this is done by systemd-logind since it requires root
rights, but from the driver POV the server takes care of it. If we IE
get server managed fds on one of the BSD-s, it will no longer be done
by systemd-logind of course, but all that driver really needs to know
is that the server takes care of this one way or the other.

Re: [PATCH] intel: Add support for server managed fds

[Chris Wilson]

On Fri, Mar 07, 2014 at 07:18:29PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 03/07/2014 02:18 PM, Chris Wilson wrote:
> > If we set master_count to non-zero, we won't ever call drmDropMaster
> > (since our get/put will be unbalanced). Does the server manage
> > DRM_MASTER across VT switches for this fd?
> 
> Yes (*), so the never calling drmDropMaster is the intention.

Ok. Looks good, please ping when SUPPORTS_SERVER_FD lands upstream.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] intel: Add support for server managed fds

[Chris Wilson]

On Fri, Mar 07, 2014 at 08:05:19PM +0000, Chris Wilson wrote:
> On Fri, Mar 07, 2014 at 07:18:29PM +0100, Hans de Goede wrote:
> > Hi,
> > 
> > On 03/07/2014 02:18 PM, Chris Wilson wrote:
> > > If we set master_count to non-zero, we won't ever call drmDropMaster
> > > (since our get/put will be unbalanced). Does the server manage
> > > DRM_MASTER across VT switches for this fd?
> > 
> > Yes (*), so the never calling drmDropMaster is the intention.
> 
> Ok. Looks good, please ping when SUPPORTS_SERVER_FD lands upstream.

Bumped the dependency of SUPPORTS_SERVER_FD to 1.15.99.902 and pushed.
Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre