[Mesa-dev] [RFC 3/5] platform/android: Enable kms_swrast fallback

[Emil Velikov]

Hi Rob,

On 5 July 2018 at 11:07, Robert Foss <robert.foss at collabora.com> wrote:
> Add support for the ForceSoftware option, which is togglable
> on the Android platform through setting the "drm.gpu.force_software"
> property to a non-zero value.
>
> kms_swrast is also enabled as a fallback for when a driver is not
> able to be loaded for for a drm node that was opened.
>
> Signed-off-by: Robert Foss <robert.foss at collabora.com>
> ---
>  src/egl/drivers/dri2/platform_android.c | 26 +++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
> index 92b2d2b343e..bc644c25bf9 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -1193,17 +1193,21 @@ static const __DRIextension *droid_image_loader_extensions[] = {
>  };
>
>  EGLBoolean
> -droid_load_driver(_EGLDisplay *disp)
> +droid_load_driver(_EGLDisplay *disp, EGLBoolean force_software)
>  {
>     struct dri2_egl_display *dri2_dpy = disp->DriverData;
>     const char *err;
>
> -   dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd);
> +   if (force_software) {
> +      dri2_dpy->driver_name = strdup("kms_swrast");
> +   } else {
> +      dri2_dpy->driver_name = loader_get_driver_for_fd(dri2_dpy->fd);
> +   }
> +
Nit: no brackets are needed here.

>     if (dri2_dpy->driver_name == NULL)
>        return false;
>
>     dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER;
> -
>     if (!dri2_dpy->is_render_node) {
>     #ifdef HAVE_DRM_GRALLOC
>         /* Handle control nodes using __DRI_DRI2_LOADER extension and GEM names
> @@ -1359,6 +1363,7 @@ EGLBoolean
>  dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp)
>  {
>     struct dri2_egl_display *dri2_dpy;
> +   int force_software = disp->Options.ForceSoftware;
ForceSoftware is EGLBoolean and droid_load_driver() uses the same.
Yet "int" is used here and "= true" below.

>     const char *err;
>     int ret;
>
> @@ -1384,13 +1389,18 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp)
>
>     dri2_dpy->fd = droid_open_device(disp);
>     if (dri2_dpy->fd < 0) {
> -      err = "DRI2: failed to open device";
> -      goto cleanup;
> +      err = "DRI2: failed to open device, trying software device";
Ignoring the first patch for a second - the goto cleanup should stay.

>     }
>
> -   if (!droid_load_driver(disp)) {
> -      err = "DRI2: failed to load driver";
> -      goto cleanup;
> +load_driver:
> +   if (!droid_load_driver(disp, force_software)) {
> +      if (force_software) {
> +         err = "DRI2: failed to load driver";
> +         goto cleanup;
> +      } else {
> +         force_software = true;
> +         goto load_driver;
> +      }
You can trivially eliminate the goto statement here.

Thanks
Emil