[Mesa-dev] [PATCH 1/5] i965: Query and store GPU properties from kernel

[Matt Turner]

On Mon, Mar 7, 2016 at 5:39 PM, Ben Widawsky
<benjamin.widawsky at intel.com> wrote:
> Certain products are not uniquely identifiable based on device id alone. The
> kernel exports an interface to help deal with this. This patch merely introduces
> the consumer of the interface and makes sure nothing breaks.
>
> It is also possible to use these values for programming GPGPU mode, and I plan
> to do that as well.
>
> The interface was introduced in libdrm 2.4.60, which is already required, so it
> should all be fine.
>
> Signed-off-by: Ben Widawsky <benjamin.widawsky at intel.com>
> ---
>  src/mesa/drivers/dri/i965/intel_screen.c | 21 +++++++++++++++++++++
>  src/mesa/drivers/dri/i965/intel_screen.h | 12 +++++++++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index ee7c1d7..343b497 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1082,6 +1082,7 @@ static bool
>  intel_init_bufmgr(struct intel_screen *intelScreen)
>  {
>     __DRIscreen *spriv = intelScreen->driScrnPriv;
> +   bool devid_override = getenv("INTEL_DEVID_OVERRIDE") != NULL;
>
>     intelScreen->no_hw = getenv("INTEL_NO_HW") != NULL;
>
> @@ -1099,6 +1100,26 @@ intel_init_bufmgr(struct intel_screen *intelScreen)
>        return false;
>     }
>
> +   intelScreen->subslice_total = -1;
> +   intelScreen->eu_total = -1;
> +
> +   /* Everything below this is for real hardware only */
> +   if (intelScreen->no_hw || devid_override)
> +      return true;
> +
> +   intel_get_param(spriv, I915_PARAM_SUBSLICE_TOTAL,
> +                   &intelScreen->subslice_total);
> +   intel_get_param(spriv, I915_PARAM_EU_TOTAL, &intelScreen->eu_total);
> +
> +   /* Without this information, we cannot get the right Braswell brandstrings,
> +    * and we have to use conservative numbers for GPGPU on many platforms, but
> +    * otherwise, things will just work.
> +    */
> +   if (intelScreen->subslice_total == -1 ||
> +       intelScreen->eu_total == -1)

I think this condition will fit on one line.

> +      _mesa_warning(NULL,
> +                    "Kernel 4.1 required to properly query GPU properties.\n");
> +
>     return true;
>  }
>
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h
> index 3a5f22c..695ed50 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.h
> +++ b/src/mesa/drivers/dri/i965/intel_screen.h
> @@ -81,7 +81,17 @@ struct intel_screen
>      * I915_PARAM_CMD_PARSER_VERSION parameter
>      */
>     int cmd_parser_version;
> - };
> +
> +   /**
> +    * Best effort attempt to get system information. Needed for GPGPU, and brand
> +    * strings (sigh)

The comment doesn't really describe the fields. Maybe

/**
 * Number of subslices reported by the I915_PARAM_SUBSLICE_TOTAL parameter
 */
int subslice_total;

/**
 * Number of EUs reported by the I915_PARAM_EU_TOTAL parameter
 */
int eu_total;

(Might have to linewrap the comments, not sure)

> +    * I915_PARAM_SUBSLICE_TOTAL, and I915_PARAM_EU_TOTAL
> +    */
> +   struct {

Do these need to be together in a struct?

> +      int subslice_total;
> +      int eu_total;
> +   };
> +};