[PATCH v2] glamor: check max native ALU instructions

[Eric Anholt]

Olivier Fourdan <ofourdan at redhat.com> writes:

> When using glamor (either in Xephyr or Xwayland) on hardware with too
> low instructions limit, glamor fallbacks to sw due to large shaders.
>
> This makes glamor unbearably slow on such hardware.
>
> Check reported value for GL_MAX_PROGRAM_NATIVE_ALU_INSTRUCTIONS_ARB
> and fail in glamor_init() if the limit is lower than 128.
>
> Can be overridden with the environment variable
> GLAMOR_MIN_ALU_INSTRUCTIONS for testing and debugging purpose.
>
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=88316
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>

I like the solution!  Just a few little nitpicks.

> ---
> v2: Check for GL version < 30 
>
>  glamor/glamor.c      | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  glamor/glamor_priv.h |  2 ++
>  2 files changed, 49 insertions(+)
>
> diff --git a/glamor/glamor.c b/glamor/glamor.c
> index 017266a..8270914 100644
> --- a/glamor/glamor.c
> +++ b/glamor/glamor.c
> @@ -305,6 +305,50 @@ glamor_create_screen_resources(ScreenPtr screen)
>      return ret;
>  }
>  
> +static int
> +glamor_get_min_instructions(void)
> +{
> +    char *min_instructions_string;
> +    int min_instructions;
> +
> +    min_instructions_string = getenv("GLAMOR_MIN_ALU_INSTRUCTIONS");
> +    if (min_instructions_string
> +        && sscanf(min_instructions_string, "%d", &min_instructions) == 1)
> +        return min_instructions;
> +
> +    return GLAMOR_MIN_ALU_INSTRUCTIONS;
> +}

I'd rather we didn't ship with a getenv in the server.  If you want to
hack on glamor on low-instruction-count GPUs, just edit the code.

> +static Bool
> +glamor_check_limits(int gl_version)
> +{
> +    GLint max_native_alu_instructions;
> +    int min_instructions;
> +
> +    /* Avoid using glamor if the reported instructions limit is too low,
> +     * as this would cause glamor to fallback on sw due to large shaders
> +     * which ends up being unbearably slow.
> +     */
> +    min_instructions = glamor_get_min_instructions();
> +    if (gl_version < 30 && min_instructions > 0) {
> +        if (!epoxy_has_gl_extension("GL_ARB_fragment_program")) {
> +            ErrorF("GL_ARB_fragment_program required\n");
> +            return FALSE;
> +        }

These ErrorFs should be LogMessage, like in the caller.

> +
> +        glGetProgramivARB(GL_FRAGMENT_PROGRAM_ARB,
> +                          GL_MAX_PROGRAM_NATIVE_ALU_INSTRUCTIONS_ARB,
> +                          &max_native_alu_instructions);
> +        if (max_native_alu_instructions < min_instructions) {
> +            ErrorF("Require at least %d instructions (%d reported)\n",
> +                   min_instructions, max_native_alu_instructions);

"glamor requires at least..."

> +            return FALSE;
> +        }
> +    }
> +
> +    return TRUE;
> +}

This function is poorly named, since other limit checks are in the
caller.  Maybe just call it glamor_check_instruction_count()?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20150204/a6e3c211/attachment.sig>