Re: [Mesa-dev] [PATCH v2 1/2] egl/surfaceless: Define DRI_SWRastLoader extension when using swrast.
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.
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.
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