[Nouveau] [PATCH 2/2] Add Option "DRI" to allow selection of maximum DRI level. (v2)

[Emil Velikov]

On 29 July 2015 at 13:39, Mario Kleiner <mario.kleiner.de at gmail.com> wrote:
> Allow user to select the maximum level of DRI implementation
> to use, DRI2 or DRI3.
>
> exa accel supports both DRI2 and, if the kernel supports
> rendernodes, also DRI3. However, DRI3 still seems to have
> some bugs on current implementations, and additionally it
> doesn't work well at all for X-Servers older than 1.16.3
> due to X-Server bugs. Therefore we default to DRI2 on exa,
> but allow the user to enable DRI3 with this new option.
>
> nouveau's glamor accel backend currently doesn't work under
> DRI2 at all, so we continue to use DRI3 whenever it is
> supported and ignore this new option for now.
>
> Also add a bit more output about status of Present and
> DRI3 to aid debugging.
>
> Note: This was originally meant to be a boolean parameter,
>       to just select between DRI3 on and off, but changed
>       here to a DRI level to make it consistent with the
>       same option in the released Intel-ddx.
>
> v2: Use fixed up Bool return type of nouveau_present_init().
>
> Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: Emil Velikov <emil.l.velikov at gmail.com>
> Cc: Martin Peres <martin.peres at free.fr>
> Cc: Ben Skeggs <bskeggs at redhat.com>
> ---
>  man/nouveau.man      |  6 ++++++
>  src/nouveau_dri2.c   | 11 ++++++++++-
>  src/nouveau_glamor.c |  2 +-
>  src/nv_const.h       |  2 ++
>  src/nv_driver.c      | 30 ++++++++++++++++++++++++++++--
>  src/nv_type.h        |  1 +
>  6 files changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/man/nouveau.man b/man/nouveau.man
> index 129bb7f..3d5a428 100644
> --- a/man/nouveau.man
> +++ b/man/nouveau.man
> @@ -125,6 +125,12 @@ that relies on correct presentation timing behaviour as defined in that
>  specification.
>  .br
>  Default: 1.
> +.TP
> +.BI "Option \*qDRI\*q \*q" integer \*q
> +Define the maximum level of DRI to enable. Valid values are 2 or 3.
> +exa acceleration will honor the maximum level if it is supported.
> +Under glamor acceleration DRI3 is always enabled if supported,
> +as glamor currently does not support DRI2. Default: 2 on exa, 3 on glamor.
>  .SH "SEE ALSO"
>  __xservername__(__appmansuffix__), __xconfigfile__(__filemansuffix__), Xserver(__appmansuffix__), X(__miscmansuffix__)
>  .SH AUTHORS
> diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
> index ce6f53e..81ee9be 100644
> --- a/src/nouveau_dri2.c
> +++ b/src/nouveau_dri2.c
> @@ -1134,7 +1134,16 @@ nouveau_dri3_screen_init(ScreenPtr screen)
>         if (buf && stat(buf, &render) == 0 &&
>             master.st_mode == render.st_mode) {
>                 pNv->render_node = buf;
> -               return dri3_screen_init(screen, &nouveau_dri3_screen_info);
> +               if (dri3_screen_init(screen, &nouveau_dri3_screen_info)) {
> +                       xf86DrvMsg(pScrn->scrnIndex, X_INFO,
> +                                  "DRI3 on EXA enabled\n");
> +                       return TRUE;
> +               }
> +               else {
> +                       xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
> +                                  "DRI3 on EXA initialization failed\n");
> +                       return FALSE;
> +               }
>         } else
>                 free(buf);
>  #endif
> diff --git a/src/nouveau_glamor.c b/src/nouveau_glamor.c
> index b8bca17..a8e9206 100644
> --- a/src/nouveau_glamor.c
> +++ b/src/nouveau_glamor.c
> @@ -240,7 +240,7 @@ nouveau_glamor_init(ScreenPtr screen)
>         screen->SharePixmapBacking = nouveau_glamor_share_pixmap_backing;
>         screen->SetSharedPixmapBacking = nouveau_glamor_set_shared_pixmap_backing;
>
> -       xf86DrvMsg(scrn->scrnIndex, X_INFO, "[GLAMOR] initialised\n");
> +       xf86DrvMsg(scrn->scrnIndex, X_INFO, "[GLAMOR] initialised with DRI3\n");
>         pNv->Flush = nouveau_glamor_flush;
>         return TRUE;
>  }
> diff --git a/src/nv_const.h b/src/nv_const.h
> index f1b4e9b..3f18d23 100644
> --- a/src/nv_const.h
> +++ b/src/nv_const.h
> @@ -18,6 +18,7 @@ typedef enum {
>      OPTION_SWAP_LIMIT,
>      OPTION_ASYNC_COPY,
>      OPTION_ACCELMETHOD,
> +    OPTION_DRI,
>  } NVOpts;
>
>
> @@ -34,6 +35,7 @@ static const OptionInfoRec NVOptions[] = {
>      { OPTION_SWAP_LIMIT,       "SwapLimit",    OPTV_INTEGER,   {0}, FALSE },
>      { OPTION_ASYNC_COPY,       "AsyncUTSDFS",  OPTV_BOOLEAN,   {0}, FALSE },
>      { OPTION_ACCELMETHOD,      "AccelMethod",  OPTV_STRING,    {0}, FALSE },
> +    { OPTION_DRI,              "DRI",          OPTV_INTEGER,   {0}, FALSE },
>      { -1,                       NULL,           OPTV_NONE,      {0}, FALSE }
>  };
>
> diff --git a/src/nv_driver.c b/src/nv_driver.c
> index 4218e4f..514a8bc 100644
> --- a/src/nv_driver.c
> +++ b/src/nv_driver.c
> @@ -1095,6 +1095,25 @@ NVPreInit(ScrnInfoPtr pScrn, int flags)
>         pNv->ce_enabled =
>                 xf86ReturnOptValBool(pNv->Options, OPTION_ASYNC_COPY, FALSE);
>
> +       /* Define maximum allowed level of DRI implementation to use.
> +        * We default to DRI2 on EXA for now, as DRI3 still has some
> +        * problems. However, the max_dri_level can be only honored
> +        * by EXA, as GLAMOR only supports DRI3 at the moment.
> +        */
Looks great afaict. On the bikeshed side: one could add a note to
update the manpage as these change, but I doubt that'll be useful any
time soon.

-Emil