On Mon, Mar 7, 2016 at 5:39 PM, Ben Widawsky
<benjamin.widaw...@intel.com> wrote:
> "Braswell" is a Cherryview based *thing*. It unfortunately requires extra
> information to determine its marketing name. Unlike all previous products, and
> hopefully all future ones, there is no unique 1:1 mapping of PCI device ID to
> brand string.
>
> I put up a fight about adding any complexity to our GL renderer string code
> for
> a very long time. However, a wise man made a comment to me that I couldn't
> argue
> with: if a user installs Windows on their hardware, the brand string should be
> the same as what we display in Linux. The Windows driver apparently does this
> check, so we should too.
>
> Note that I did manage to find a good use for this info anyway in the computer
> shader thread counts.
>
> Cc: Kaveh Nasri <kaveh.na...@intel.com>
> Signed-off-by: Ben Widawsky <benjamin.widaw...@intel.com>
> ---
> include/pci_ids/i965_pci_ids.h | 4 ++--
> src/mesa/drivers/dri/i965/brw_context.c | 33
> +++++++++++++++++++++++++++++---
> src/mesa/drivers/dri/i965/brw_context.h | 3 ++-
> src/mesa/drivers/dri/i965/intel_screen.c | 2 +-
> 4 files changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/include/pci_ids/i965_pci_ids.h b/include/pci_ids/i965_pci_ids.h
> index bdfbefe..d783e39 100644
> --- a/include/pci_ids/i965_pci_ids.h
> +++ b/include/pci_ids/i965_pci_ids.h
> @@ -156,8 +156,8 @@ CHIPSET(0x5932, kbl_gt4, "Intel(R) Kabylake GT4")
> CHIPSET(0x593A, kbl_gt4, "Intel(R) Kabylake GT4")
> CHIPSET(0x593B, kbl_gt4, "Intel(R) Kabylake GT4")
> CHIPSET(0x593D, kbl_gt4, "Intel(R) Kabylake GT4")
> -CHIPSET(0x22B0, chv, "Intel(R) HD Graphics (Cherryview)")
> -CHIPSET(0x22B1, chv, "Intel(R) HD Graphics (Cherryview)")
> +CHIPSET(0x22B0, chv, "Intel(R) HD Graphics (Cherrytrail)")
> +CHIPSET(0x22B1, chv, "Intel(R) HD Graphics XXX (Braswell)") /* Overriden
> in brw_get_renderer_string */
Typo: Overridden
> CHIPSET(0x22B2, chv, "Intel(R) HD Graphics (Cherryview)")
> CHIPSET(0x22B3, chv, "Intel(R) HD Graphics (Cherryview)")
> CHIPSET(0x0A84, bxt, "Intel(R) HD Graphics (Broxton)")
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index df0f6bb..f57184f 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -77,13 +77,27 @@
>
> const char *const brw_vendor_string = "Intel Open Source Technology Center";
>
> +static const char *
> +get_bsw_model(const struct intel_screen *intelScreen)
> +{
> + switch (intelScreen->eu_total) {
> + case 16:
> + return "405";
> + case 12:
> + return "400";
> + default:
I think this is safe to just mark unreachable(), right?
> + return " ";
> + }
> +}
> +
> const char *
> -brw_get_renderer_string(unsigned deviceID)
> +brw_get_renderer_string(const struct intel_screen *intelScreen)
> {
> const char *chipset;
> static char buffer[128];
Not your fault, but driGetRendererString() into this static buffer
isn't thread-safe. I ran into a similar problem in EGL with
shader-db's run.c last year.
> + char *bsw = NULL;
Thought the initialization wasn't necessary at first, but indeed it is
if you want to unconditionally call free().
>
> - switch (deviceID) {
> + switch (intelScreen->deviceID) {
> #undef CHIPSET
> #define CHIPSET(id, symbol, str) case id: chipset = str; break;
> #include "pci_ids/i965_pci_ids.h"
> @@ -92,7 +106,20 @@ brw_get_renderer_string(unsigned deviceID)
> break;
> }
>
> + /* Braswell branding is funny, so we have to fix it up here */
> + if (intelScreen->deviceID == 0x22B1) {
> + char *needle;
> +
> + bsw = strdup(chipset);
> + needle = strstr(bsw, "XXX");
Could declare char *needle here and initialize on one line if you wanted.
> + if (needle) {
> + strncpy(needle, get_bsw_model(intelScreen), strlen("XXX"));
Don't actually need (or want) any of the features of strncpy. Should
just use memcpy.
> + chipset = bsw;
> + }
> + }
> +
> (void) driGetRendererString(buffer, chipset, 0);
> + free(bsw);
> return buffer;
> }
>
> @@ -107,7 +134,7 @@ intel_get_string(struct gl_context * ctx, GLenum name)
>
> case GL_RENDERER:
> return
> - (GLubyte *) brw_get_renderer_string(brw->intelScreen->deviceID);
> + (GLubyte *) brw_get_renderer_string(brw->intelScreen);
>
With the couple things fixed,
Reviewed-by: Matt Turner <matts...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
