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

[David Riley]

On Thu, Jul 26, 2018 at 7:22 AM Emil Velikov <emil.l.velikov at gmail.com>
wrote:

> Hi David,
>
> On 18 July 2018 at 01:12, David Riley <davidriley at chromium.org> 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 <davidriley at chromium.org>
> > ---
> >  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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180726/fc16858e/attachment-0001.html>