[Pixman] [PATCH 04/11] MIPS: dspr2: runtime detection extended

[Søren Sandmann]

The commit log here needs to say why and how the runtime detection was
extended. 

See below for more.

> ---
>  pixman/pixman-mips.c |   83 ++++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 63 insertions(+), 20 deletions(-)
>
> diff --git a/pixman/pixman-mips.c b/pixman/pixman-mips.c
> index 3048813..93fda99 100644
> --- a/pixman/pixman-mips.c
> +++ b/pixman/pixman-mips.c
> @@ -24,14 +24,27 @@
>  #endif
>  
>  #include "pixman-private.h"
> -
> -#if defined(USE_MIPS_DSPR2) || defined(USE_LOONGSON_MMI)
> -
>  #include <string.h>
>  #include <stdlib.h>
>  
> +#ifdef USE_MIPS_DSPR2
> +static const char *mips_dspr2_cores[] =
> +{
> +    "MIPS 74K", NULL
> +};
> +#endif
> +
> +#ifdef USE_LOONGSON_MMI
> +static const char *mips_loongson_cores[] =
> +{
> +    "Loongson", NULL
> +};
> +#endif
> +
> +#if defined(USE_MIPS_DSPR2) || defined(USE_LOONGSON_MMI)
> +
>  static pixman_bool_t
> -have_feature (const char *search_string)
> +have_feature (const char **cores, const char *search_string)
>  {
>  #if defined (__linux__) /* linux ELF */
>      /* Simple detection of MIPS features at runtime for Linux.
> @@ -40,20 +53,52 @@ have_feature (const char *search_string)
>       * facility is universally available on the MIPS architectures, so it's up
>       * to individual OSes to provide such.
>       */
> -    const char *file_name = "/proc/cpuinfo";
> -    char cpuinfo_line[256];
> +    const char *file_name = "/proc/version";
> +    char line[512];
> +    int v0 = 0, v1 = 0;
>      FILE *f = NULL;
> +    char **temp = (char **)cores;
> +
> +    if ((f = fopen (file_name, "r")) != NULL)
> +    {
> +        if (fgets (line, sizeof (line), f) != NULL)
> +            sscanf (line, "%*s%*s%d%*c%d", &v0, &v1);
> +        fclose (f);
> +    }
> +
> +    file_name = "/proc/cpuinfo";
>  
>      if ((f = fopen (file_name, "r")) == NULL)
>          return FALSE;
>  
> -    while (fgets (cpuinfo_line, sizeof (cpuinfo_line), f) != NULL)
> +    if ((v0 >= 3) && (v1 >= 7))

This looks wrong. What happens if v0 is some day increased to 4?

> +    {
> +        /* isa filed (mips32r2) is available from kernel version 3.9
> +         * ASEs implemented field (dsp, dsp2) is available from 3.7
> +         * In older kernel versions "dsp" represents both DSPr1 and DSPr2
> +         */
> +        while (fgets (line, sizeof (line), f) != NULL)
> +        {
> +            if (strstr (line, search_string) != NULL)
> +            {
> +                fclose (f);
> +                return TRUE;
> +            }
> +        }
> +        rewind(f);
> +    }
> +
> +    while (fgets (line, sizeof (line), f) != NULL)
>      {
> -        if (strstr (cpuinfo_line, search_string) != NULL)
> +        while (*temp)
>          {
> -            fclose (f);
> -            return TRUE;
> +            if (strstr (line, *temp++) != NULL)
> +            {
> +                fclose (f);
> +                return TRUE;
> +            }
>          }
> +        temp = (char **)cores;
>      }

I'm wondering if these two loops could be combined into one. I may be
missing something though.

>  
>      fclose (f);
> @@ -70,23 +115,21 @@ _pixman_mips_get_implementations (pixman_implementation_t *imp)
>  {
>  #ifdef USE_LOONGSON_MMI
>      /* I really don't know if some Loongson CPUs don't have MMI. */
> -    if (!_pixman_disabled ("loongson-mmi") && have_feature ("Loongson"))
> -	imp = _pixman_implementation_create_mmx (imp);
> +    if (!_pixman_disabled ("loongson-mmi") &&
> +        have_feature (mips_loongson_cores, "Loongson"))
> +        imp = _pixman_implementation_create_mmx (imp);
>  #endif

Braces are needed around the body of an "if" where the condition is more
than one line.

>  #ifdef USE_MIPS_DSPR2
>      if (!_pixman_disabled ("mips-dspr2"))
>      {
> -	int already_compiling_everything_for_dspr2 = 0;
> +        int already_compiling_everything_for_dspr2 = 0;
>  #if defined(__mips_dsp) && (__mips_dsp_rev >= 2)
> -	already_compiling_everything_for_dspr2 = 1;
> +        already_compiling_everything_for_dspr2 = 1;
>  #endif
> -	if (already_compiling_everything_for_dspr2 ||
> -	    /* Only currently available MIPS core that supports DSPr2 is 74K. */
> -	    have_feature ("MIPS 74K"))
> -	{
> -	    imp = _pixman_implementation_create_mips_dspr2 (imp);
> -	}
> +        if (already_compiling_everything_for_dspr2 ||
> +            have_feature (mips_dspr2_cores, "dsp2"))
> +            imp = _pixman_implementation_create_mips_dspr2 (imp);
>      }
>  #endif

Same here.


Søren