[Bug 21515] VENDOR_WINE vs VENDOR_ATI with xf86-video-ati

[wine-bugs at winehq.org]

http://bugs.winehq.org/show_bug.cgi?id=21515





--- Comment #72 from Stefan Dösinger <stefandoesinger at gmx.at>  2010-02-11 04:54:23 ---
The patches look pretty good, just a few comments:

Patch 2:
HW_VENDOR_MESA isn't needed any longer, just use HW_VENDOR_WINE if the real HW
vendor isn't properly detected.

Patch 3:
>+    /* if (match_apple(gl_info, gl_renderer, gl_vendor, card_vendor, device)) return FALSE;
>+    if (strstr(gl_renderer, "DRI")) return FALSE;  Filter out Mesa DRI drivers. */
     return TRUE;
Yes, shouldn't be necessary, just remove both lines. I'd also invert the
gl_vendor if check
if(GL_VENDOR == GL_VENDOR_ATI) return TRUE;
else return FALSE;

Patch 4:
>+    for (i = 0; i < (sizeof(vendor_card_select_table) / sizeof(*vendor_card_select_table)); ++i)
>+    {
>+        if ((vendor_card_select_table[i].gl_vendor != *gl_vendor)
>+            || (vendor_card_select_table[i].card_vendor != *card_vendor))
>+		 continue;
>+        TRACE_(d3d_caps)("Applying card_selector \"%s\".\n", vendor_card_select_table[i].description);
>+        return vendor_card_select_table[i].select_card(gl_info, gl_renderer, vidmem);
>     }
>+
>+
>+    /* Default to generic Nvidia hardware based on the supported OpenGL extensions. The choice
I recommend to check the select_card result, to allow the card detection code
to return an unknown card(e.g. 0x0000) if the detection fails for some reason.
Then the loop can abort and use the generic nvidia card selection code below.

>                 /* Geforce6/7 lowend */
>+                /* If it's GL_VENDOR_APPLE, then it could also be an ATI card, so allow it to fall through */
>                 *vidmem = 64; /* */
>                 return CARD_NVIDIA_GEFORCE_6200; /* Geforce 6100/6150/6200/7300/7400/7500 */
Why would we end up in select_card_nvidia_binary on OSX with an ATI card?

Patch 5:
> +enum wined3d_pci_device (select_card_intel_mesa)(const struct wined3d_gl_info *gl_info, const char *gl_renderer,
> +            unsigned int *vidmem )
> +{
> +            FIXME_(d3d_caps)("Card selection not handled for Mesa Intel driver\n");
> +            if (WINE_D3D9_CAPABLE(gl_info)) return CARD_NVIDIA_GEFORCEFX_5600;
This will lead to a PCI vendor 0x8086, pci device 0x0312 which doesn't exist.
Some other nvidia card if interpreted as an Intel device might be a SATA Raid
controller *gg*

That taps into the suggestion concerning patch 4 - allow the detection
functions to fail, and add a FIXME to the generic nvidia guessing code,
something like FIXME("Implement a card ID detection function for GL vendor %s,
HW vendor %s\n", debug_gl_vendor(gl_vendor), debug_hw_vendor(hw_vendor));

General:
I don't know how you generated the patches, but the patch format(filename, lack
of author info etc) is unknown to me. I recommend to use git-format-patch, it
allows you to add a description of what the patch does into the patch, and adds
your name and Email address. There are some higher level tools like stacked git
that can do this as well.

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.