[PATCH xf86-video-vmware 2/2] Add support for server managed fds

[Jakob Bornecrantz]

On Wed, May 14, 2014 at 11:18 AM, Hans de Goede <hdegoede at redhat.com> wrote:
> Hi,
>
> Thanks for the reviews!
>
> On 05/14/2014 11:07 AM, Jakob Bornecrantz wrote:
>> On Wed, May 7, 2014 at 3:23 PM, Hans de Goede <hdegoede at redhat.com> wrote:
>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>> ---
>>>  src/vmware_bootstrap.c |  5 +++++
>>>  vmwgfx/vmwgfx_driver.c | 31 +++++++++++++++++++++++++++----
>>>  vmwgfx/vmwgfx_driver.h |  2 ++
>>>  3 files changed, 34 insertions(+), 4 deletions(-)
>>
>> This patch has, if one thing is fixed as detailed below, my
>> Reviewed-by: Jakob Bornecrantz <jakob at vmware.com>
>>
>>>
>>> diff --git a/src/vmware_bootstrap.c b/src/vmware_bootstrap.c
>>> index b30cf2b..1635876 100644
>>> --- a/src/vmware_bootstrap.c
>>> +++ b/src/vmware_bootstrap.c
>>> @@ -30,6 +30,7 @@
>>>  #endif
>>>
>>>  #include "xf86.h"
>>> +#include <xorgVersion.h>
>>>  #include "compiler.h"
>>>  #include "xf86Pci.h"           /* pci */
>>>  #include "vm_device_version.h"
>>> @@ -496,6 +497,10 @@ VMWareDriverFunc(ScrnInfoPtr pScrn,
>>>                               pScrn->yDpi / 2) / pScrn->yDpi;
>>>        }
>>>        return TRUE;
>>> +#if XORG_VERSION_CURRENT >= XORG_VERSION_NUMERIC(1,15,99,902,0)
>>> +   case SUPPORTS_SERVER_FDS:
>>> +      return TRUE;
>>> +#endif
>>>     default:
>>>        return FALSE;
>>>     }
>>> diff --git a/vmwgfx/vmwgfx_driver.c b/vmwgfx/vmwgfx_driver.c
>>> index 7e5dc4e..03103a8 100644
>>> --- a/vmwgfx/vmwgfx_driver.c
>>> +++ b/vmwgfx/vmwgfx_driver.c
>>> @@ -54,6 +54,10 @@
>>>
>>>  #include <pciaccess.h>
>>>
>>> +#ifdef XSERVER_PLATFORM_BUS
>>> +#include "xf86platformBus.h"
>>> +#endif
>>> +
>>>  #include "vmwgfx_driver.h"
>>>
>>>  #include <saa.h>
>>> @@ -243,6 +247,15 @@ static const xf86CrtcConfigFuncsRec crtc_config_funcs = {
>>>      .resize = drv_crtc_resize
>>>  };
>>>
>>> +static Bool vmwgfx_use_server_fd(modesettingPtr ms)
>>> +{
>>> +#ifdef XF86_PDEV_SERVER_FD
>>> +    return ms->platform_dev && (ms->platform_dev->flags & XF86_PDEV_SERVER_FD);
>>> +#else
>>> +    return FALSE;
>>> +#endif
>>> +}
>>> +
>>>  static Bool
>>>  drv_init_drm(ScrnInfoPtr pScrn)
>>>  {
>>> @@ -253,6 +266,12 @@ drv_init_drm(ScrnInfoPtr pScrn)
>>>
>>>         ms->fd = vmwgfx_hosted_drm_fd(ms->hdriver, ms->hosted, ms->PciInfo);
>>>
>>> +#ifdef ODEV_ATTRIB_FD
>>> +       if (ms->fd < 0 && vmwgfx_use_server_fd(ms))
>>> +           ms->fd = xf86_get_platform_device_int_attrib(ms->platform_dev,
>>> +                                                        ODEV_ATTRIB_FD, -1);
>>
>> If this fails you probably want to error, since the rest of the DRM set/drop
>> master code also looks at vmwgfx_use_server_fd. Then again I don't know
>> if other drivers does it like this, and will try all methods when the platform
>> bus is used.
>
> The other drivers are like this since the server guarantees that the call
> will succeed if vmwgfx_use_server_fd(ms) succeeds that ODEV_ATTRIB_FD will
> be set and won't be -1.
>
> But if you want I can easily add error handling by adding an "else" above
> the #endif, then the normal open will only get skipped if vmwgfx_use_server_fd(ms)
> is true, and the error handling path below the normal open path will trigger
> if xf86_get_platform_device_int_attrib somehow failed.
>
> Let me know how you want to proceed with this and then I'll add the "else"
> (or not), your Reviewed-by and push these.

Thanks for the explanation, go ahead and push the changes as is.

Cheers, Jakob.