<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Jul 26, 2018 at 7:22 AM Emil Velikov <<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi David,<br>
<br>
On 18 July 2018 at 01:12, David Riley <<a href="mailto:davidriley@chromium.org" target="_blank">davidriley@chromium.org</a>> wrote:<br>
<br>
Commit message here should explain why this is needed. Is the current<br>
kms_swrast usage failing/crashing somewhere, etc.<br></blockquote><div><br></div><div>The change wasn't needed for kms_swrast, just for swrast which added an dependency on having the loader extension defined with the change <a href="https://github.com/mesa3d/mesa/commit/63c427fa71a07649d5c033a5c6184ef701348590#diff-aa8c7f5cc2f62d6a098b04df0603c87b">https://github.com/mesa3d/mesa/commit/63c427fa71a07649d5c033a5c6184ef701348590#diff-aa8c7f5cc2f62d6a098b04df0603c87b</a></div><div><br></div><div>I added one to avoid potential issues if other dependencies on a swrast loader extension are required in the future and to keep the DRM and DRMless paths be similar.  As far as I can tell it's not strictly needed at this time and I'm okay dropping it.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> Signed-off-by: David Riley <<a href="mailto:davidriley@chromium.org" target="_blank">davidriley@chromium.org</a>><br>
> ---<br>
>  src/egl/drivers/dri2/platform_surfaceless.c | 28 +++++++++++++++++----<br>
>  1 file changed, 23 insertions(+), 5 deletions(-)<br>
><br>
> diff --git a/src/egl/drivers/dri2/platform_surfaceless.c b/src/egl/drivers/dri2/platform_surfaceless.c<br>
> index a0348a5e95..f5fe7119c6 100644<br>
> --- a/src/egl/drivers/dri2/platform_surfaceless.c<br>
> +++ b/src/egl/drivers/dri2/platform_surfaceless.c<br>
> @@ -260,6 +260,13 @@ static const __DRIimageLoaderExtension image_loader_extension = {<br>
>     .flushFrontBuffer = surfaceless_flush_front_buffer,<br>
>  };<br>
><br>
> +static const __DRIswrastLoaderExtension swrast_loader_extension = {<br>
> +   .base            = { __DRI_SWRAST_LOADER, 1 },<br>
> +   .getDrawableInfo = NULL,<br>
> +   .putImage        = NULL,<br>
> +   .getImage        = NULL,<br>
> +};<br>
> +<br>
>  #define DRM_RENDER_DEV_NAME  "%s/renderD%d"<br>
><br>
>  static const __DRIextension *image_loader_extensions[] = {<br>
> @@ -269,6 +276,14 @@ static const __DRIextension *image_loader_extensions[] = {<br>
>     NULL,<br>
>  };<br>
><br>
> +static const __DRIextension *swrast_loader_extensions[] = {<br>
> +   &swrast_loader_extension.base,<br>
> +   &image_loader_extension.base,<br>
> +   &image_lookup_extension.base,<br>
> +   &use_invalidate.base,<br>
How did you compiler this list. Gut suggests that at least one of<br>
those should not be here.<br>
Doesn't this break the existing kms_swrast usage?<br>
<br></blockquote><div>This was the existing list of extensions used prior to this change with a newly added one for the swrast loader.  I ran a basic set of DEQP tests (dEQP-GLES3.functional.draw.draw_elements.*, dEQP-GLES3.info.*) </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
><br>
>        dri2_dpy->fd = fd;<br>
> -      if (dri2_load_driver_dri3(dpy))<br>
> +      if (dri2_load_driver_dri3(dpy)) {<br>
>           return true;<br>
> +      }<br>
Unnecessary change.<br></blockquote><div><br></div><div>I've fixed this for v3.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
HTH<br>
Emil<br>
</blockquote></div></div>