[Mesa-dev] [PATCH 1/2] winsys/radeon: Add a work around for drmGetVersion() when using render nodes

[David Herrmann]

Hi Tom

On Tue, Oct 29, 2013 at 9:00 PM, Tom Stellard <tom at stellard.net> wrote:
> From: Tom Stellard <thomas.stellard at amd.com>
>
> DRM_IOCTL_VERSION is not currently a legal ioctl for render nodes, so we
> need to hard code the drm version when drmGetVersion() returns NULL.
> ---
>  src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 36 ++++++++++++++++-------
>  1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> index f8aeb96..5b5191a 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> @@ -212,23 +212,37 @@ static boolean do_winsys_init(struct radeon_drm_winsys *ws)
>
>      /* Get DRM version. */
>      version = drmGetVersion(ws->fd);
> -    if (version->version_major != 2 ||
> -        version->version_minor < 3) {
> +
> +    if (!version) {

"version" might be NULL on OOM, too (which we didn't check in the
previous code..). I don't really like this fallback. drmGetVersion()
will be allowed on all render-nodes which are not experimental (which
don't need drm.rnodes=1). It seems tedious to me to support this
fallback-code just for one experimental kernel-revision.

But that's just my opinion. The code looks fine so far.

Thanks
David

> +#ifdef USE_RENDER_NODES
> +        /* Calling drmGetVersion was not allowed in the earliest
> +         * implementation of render nodes.  To work around this, we will
> +         * hard code to the oldest version with render node support.
> +         */
> +        ws->info.drm_major = 2; // version->version_major;
> +        ws->info.drm_minor = 32; //version->version_minor;
> +        ws->info.drm_patchlevel = 0; // version->version_patchlevel;
> +#else
> +        return false;
> +#endif
> +    } else {
> +        ws->info.drm_major = version->version_major;
> +        ws->info.drm_minor = version->version_minor;
> +        ws->info.drm_patchlevel = version->version_patchlevel;
> +        drmFreeVersion(version);
> +    }
> +
> +    if (ws->info.drm_major != 2 ||
> +        ws->info.drm_minor < 3) {
>          fprintf(stderr, "%s: DRM version is %d.%d.%d but this driver is "
>                  "only compatible with 2.3.x (kernel 2.6.34) or later.\n",
>                  __FUNCTION__,
> -                version->version_major,
> -                version->version_minor,
> -                version->version_patchlevel);
> -        drmFreeVersion(version);
> +                ws->info.drm_major,
> +                ws->info.drm_minor,
> +                ws->info.drm_patchlevel);
>          return FALSE;
>      }
>
> -    ws->info.drm_major = version->version_major;
> -    ws->info.drm_minor = version->version_minor;
> -    ws->info.drm_patchlevel = version->version_patchlevel;
> -    drmFreeVersion(version);
> -
>      /* Get PCI ID. */
>      if (!radeon_get_drm_value(ws->fd, RADEON_INFO_DEVICE_ID, "PCI ID",
>                                &ws->info.pci_id))
> --
> 1.8.1.5
>