Re: [Mesa-dev] [PATCH v2 1/2] egl/surfaceless: Define DRI_SWRastLoader extension when using swrast.

2018-07-26 Thread David Riley
On Thu, Jul 26, 2018 at 7:22 AM Emil Velikov 
wrote:

> Hi David,
>
> On 18 July 2018 at 01:12, David Riley  wrote:
>
> Commit message here should explain why this is needed. Is the current
> kms_swrast usage failing/crashing somewhere, etc.
>

The change wasn't needed for kms_swrast, just for swrast which added an
dependency on having the loader extension defined with the change
https://github.com/mesa3d/mesa/commit/63c427fa71a07649d5c033a5c6184ef701348590#diff-aa8c7f5cc2f62d6a098b04df0603c87b

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.


>
> > Signed-off-by: David Riley 
> > ---
> >  src/egl/drivers/dri2/platform_surfaceless.c | 28 +
> >  1 file changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/platform_surfaceless.c
> b/src/egl/drivers/dri2/platform_surfaceless.c
> > index a0348a5e95..f5fe7119c6 100644
> > --- a/src/egl/drivers/dri2/platform_surfaceless.c
> > +++ b/src/egl/drivers/dri2/platform_surfaceless.c
> > @@ -260,6 +260,13 @@ static const __DRIimageLoaderExtension
> image_loader_extension = {
> > .flushFrontBuffer = surfaceless_flush_front_buffer,
> >  };
> >
> > +static const __DRIswrastLoaderExtension swrast_loader_extension = {
> > +   .base= { __DRI_SWRAST_LOADER, 1 },
> > +   .getDrawableInfo = NULL,
> > +   .putImage= NULL,
> > +   .getImage= NULL,
> > +};
> > +
> >  #define DRM_RENDER_DEV_NAME  "%s/renderD%d"
> >
> >  static const __DRIextension *image_loader_extensions[] = {
> > @@ -269,6 +276,14 @@ static const __DRIextension
> *image_loader_extensions[] = {
> > NULL,
> >  };
> >
> > +static const __DRIextension *swrast_loader_extensions[] = {
> > +   &swrast_loader_extension.base,
> > +   &image_loader_extension.base,
> > +   &image_lookup_extension.base,
> > +   &use_invalidate.base,
> How did you compiler this list. Gut suggests that at least one of
> those should not be here.
> Doesn't this break the existing kms_swrast usage?
>
> 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.*)


>
> >
> >dri2_dpy->fd = fd;
> > -  if (dri2_load_driver_dri3(dpy))
> > +  if (dri2_load_driver_dri3(dpy)) {
> >   return true;
> > +  }
> Unnecessary change.
>

I've fixed this for v3.


>
> HTH
> Emil
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v2 1/2] egl/surfaceless: Define DRI_SWRastLoader extension when using swrast.

2018-07-26 Thread Emil Velikov
Hi David,

On 18 July 2018 at 01:12, David Riley  wrote:

Commit message here should explain why this is needed. Is the current
kms_swrast usage failing/crashing somewhere, etc.

> Signed-off-by: David Riley 
> ---
>  src/egl/drivers/dri2/platform_surfaceless.c | 28 +
>  1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_surfaceless.c 
> b/src/egl/drivers/dri2/platform_surfaceless.c
> index a0348a5e95..f5fe7119c6 100644
> --- a/src/egl/drivers/dri2/platform_surfaceless.c
> +++ b/src/egl/drivers/dri2/platform_surfaceless.c
> @@ -260,6 +260,13 @@ static const __DRIimageLoaderExtension 
> image_loader_extension = {
> .flushFrontBuffer = surfaceless_flush_front_buffer,
>  };
>
> +static const __DRIswrastLoaderExtension swrast_loader_extension = {
> +   .base= { __DRI_SWRAST_LOADER, 1 },
> +   .getDrawableInfo = NULL,
> +   .putImage= NULL,
> +   .getImage= NULL,
> +};
> +
>  #define DRM_RENDER_DEV_NAME  "%s/renderD%d"
>
>  static const __DRIextension *image_loader_extensions[] = {
> @@ -269,6 +276,14 @@ static const __DRIextension *image_loader_extensions[] = 
> {
> NULL,
>  };
>
> +static const __DRIextension *swrast_loader_extensions[] = {
> +   &swrast_loader_extension.base,
> +   &image_loader_extension.base,
> +   &image_lookup_extension.base,
> +   &use_invalidate.base,
How did you compiler this list. Gut suggests that at least one of
those should not be here.
Doesn't this break the existing kms_swrast usage?


>
>dri2_dpy->fd = fd;
> -  if (dri2_load_driver_dri3(dpy))
> +  if (dri2_load_driver_dri3(dpy)) {
>   return true;
> +  }
Unnecessary change.

HTH
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH v2 1/2] egl/surfaceless: Define DRI_SWRastLoader extension when using swrast.

2018-07-17 Thread David Riley
Signed-off-by: David Riley 
---
 src/egl/drivers/dri2/platform_surfaceless.c | 28 +
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_surfaceless.c 
b/src/egl/drivers/dri2/platform_surfaceless.c
index a0348a5e95..f5fe7119c6 100644
--- a/src/egl/drivers/dri2/platform_surfaceless.c
+++ b/src/egl/drivers/dri2/platform_surfaceless.c
@@ -260,6 +260,13 @@ static const __DRIimageLoaderExtension 
image_loader_extension = {
.flushFrontBuffer = surfaceless_flush_front_buffer,
 };
 
+static const __DRIswrastLoaderExtension swrast_loader_extension = {
+   .base= { __DRI_SWRAST_LOADER, 1 },
+   .getDrawableInfo = NULL,
+   .putImage= NULL,
+   .getImage= NULL,
+};
+
 #define DRM_RENDER_DEV_NAME  "%s/renderD%d"
 
 static const __DRIextension *image_loader_extensions[] = {
@@ -269,6 +276,14 @@ static const __DRIextension *image_loader_extensions[] = {
NULL,
 };
 
+static const __DRIextension *swrast_loader_extensions[] = {
+   &swrast_loader_extension.base,
+   &image_loader_extension.base,
+   &image_lookup_extension.base,
+   &use_invalidate.base,
+   NULL,
+};
+
 static bool
 surfaceless_probe_device(_EGLDisplay *dpy, bool swrast)
 {
@@ -288,23 +303,28 @@ surfaceless_probe_device(_EGLDisplay *dpy, bool swrast)
   if (fd < 0)
  continue;
 
-  if (swrast)
+  if (swrast) {
  dri2_dpy->driver_name = strdup("kms_swrast");
-  else
+ dri2_dpy->loader_extensions = swrast_loader_extensions;
+  } else {
  dri2_dpy->driver_name = loader_get_driver_for_fd(fd);
+ dri2_dpy->loader_extensions = image_loader_extensions;
+  }
   if (!dri2_dpy->driver_name) {
  close(fd);
  continue;
   }
 
   dri2_dpy->fd = fd;
-  if (dri2_load_driver_dri3(dpy))
+  if (dri2_load_driver_dri3(dpy)) {
  return true;
+  }
 
   close(fd);
   dri2_dpy->fd = -1;
   free(dri2_dpy->driver_name);
   dri2_dpy->driver_name = NULL;
+  dri2_dpy->loader_extensions = NULL;
}
 
return false;
@@ -338,8 +358,6 @@ dri2_initialize_surfaceless(_EGLDriver *drv, _EGLDisplay 
*disp)
   goto cleanup;
}
 
-   dri2_dpy->loader_extensions = image_loader_extensions;
-
if (!dri2_create_screen(disp)) {
   err = "DRI2: failed to create screen";
   goto cleanup;
-- 
2.18.0.203.gfac676dfb9-goog

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev