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

[Hans de Goede]

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.

Regards,

Hans


> 
>> +#endif
>> +
>>         if (ms->fd < 0) {
>>
>>             char bus_id[64];
>> @@ -472,6 +491,9 @@ drv_pre_init(ScrnInfoPtr pScrn, int flags)
>>         goto out_err_bus;
>>      }
>>
>> +    if (pEnt->location.type == BUS_PLATFORM)
>> +        ms->platform_dev = pEnt->location.id.plat;
>> +
>>      xf86SetPrimInitDone(pScrn->entityList[0]);
>>
>>      ms->hdriver = vmwgfx_hosted_detect();
>> @@ -605,7 +627,7 @@ drv_pre_init(ScrnInfoPtr pScrn, int flags)
>>      free(ms->Options);
>>    out_depth:
>>    out_drm_version:
>> -    if (!vmwgfx_is_hosted(ms->hdriver))
>> +    if (!vmwgfx_is_hosted(ms->hdriver) && !vmwgfx_use_server_fd(ms))
>>         close(ms->fd);
>>    out_no_drm:
>>      vmwgfx_hosted_destroy(ms->hdriver, ms->hosted);
>> @@ -783,8 +805,8 @@ drv_set_master(ScrnInfoPtr pScrn)
>>  {
>>      modesettingPtr ms = modesettingPTR(pScrn);
>>
>> -    if (!vmwgfx_is_hosted(ms->hdriver) && !ms->isMaster &&
>> -       drmSetMaster(ms->fd) != 0) {
>> +    if (!vmwgfx_is_hosted(ms->hdriver) && !vmwgfx_use_server_fd(ms) &&
>> +            !ms->isMaster && drmSetMaster(ms->fd) != 0) {
>>         if (errno == EINVAL) {
>>             xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
>>                        "drmSetMaster failed: 2.6.29 or newer kernel required for "
>> @@ -1184,7 +1206,8 @@ drv_leave_vt(VT_FUNC_ARGS_DECL)
>>
>>      vmwgfx_saa_drop_master(pScrn->pScreen);
>>
>> -    if (!vmwgfx_is_hosted(ms->hdriver) && drmDropMaster(ms->fd))
>> +    if (!vmwgfx_is_hosted(ms->hdriver) && !vmwgfx_use_server_fd(ms) &&
>> +            drmDropMaster(ms->fd))
>>         xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
>>                    "drmDropMaster failed: %s\n", strerror(errno));
>>      ms->isMaster = FALSE;
>> diff --git a/vmwgfx/vmwgfx_driver.h b/vmwgfx/vmwgfx_driver.h
>> index c044a81..31dfc0f 100644
>> --- a/vmwgfx/vmwgfx_driver.h
>> +++ b/vmwgfx/vmwgfx_driver.h
>> @@ -83,6 +83,7 @@ enum xorg_throttling_reason {
>>  };
>>
>>  struct vmwgfx_hosted;
>> +struct xf86_platform_device;
>>
>>  typedef struct _modesettingRec
>>  {
>> @@ -98,6 +99,7 @@ typedef struct _modesettingRec
>>      int Chipset;
>>      EntityInfoPtr pEnt;
>>      struct pci_device *PciInfo;
>> +    struct xf86_platform_device *platform_dev;
>>
>>      /* Accel */
>>      Bool accelerate_render;
>> --
>> 1.9.0
>>
>> _______________________________________________
>> xorg-devel at lists.x.org: X.Org development
>> Archives: http://lists.x.org/archives/xorg-devel
>> Info: http://lists.x.org/mailman/listinfo/xorg-devel