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

[Robert Foss]

Hey Tomasz,

On 05/07/18 16:26, Tomasz Figa wrote:
> Hi Rob,
> 
> On Thu, Jul 5, 2018 at 7:07 PM 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(-)
> 
> Thanks for the patch. Please see my comments inline.
> 
>>
>> 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);
>> +   }
>> +
>>      if (dri2_dpy->driver_name == NULL)
>>         return false;
>>
>>      dri2_dpy->is_render_node = drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER;
>> -
> 
> Unrelated whitespace change.

Ack.

> 
>>      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;
> 
> EGLBoolean

Ack.

> 
>>      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";
>>      }
>>
>> -   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;
>> +      }
> 
> I believe we don't need this retry here, because if we fail here once,
> _eglMatchDriver() would retry us with ForceSoftware = EGL_TRUE [1].
> 
> [1] https://cgit.freedesktop.org/mesa/mesa/tree/src/egl/main/egldriver.c#n80

Oh nice. I didn't realize that the fallback was implemented somewhere else already.

> 
> Best regards,
> Tomasz
>