Closed
Bug 600280
Opened 14 years ago
Closed 14 years ago
[gfxInfo] Driver version and date are empty or null for some graphic cards under Windows 2000/XP
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b11
People
(Reporter: scoobidiver, Assigned: tete009+bugzilla)
References
(Blocks 1 open bug)
Details
Attachments
(5 files, 3 obsolete files)
(deleted),
image/jpeg
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
tete009+bugzilla
:
review+
joe
:
approval2.0+
|
Details | Diff | Splinter Review |
Build : Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100928 Firefox/4.0b7pre
Driver version and date are empty in the graphic section of "about:support" page.
Comment 1•14 years ago
|
||
Do they show up with gfxbot?
Reporter | ||
Comment 2•14 years ago
|
||
Comment 3•14 years ago
|
||
Ftr, "Adapter RAM : Unknown" is bug 591787.
Assignee | ||
Comment 4•14 years ago
|
||
FYI, gfxInfo's mDeviceID gets the following string on my system:
PCI\VEN_1002&DEV_4150&SUBSYS_0200174B&REV_00
Assignee | ||
Comment 5•14 years ago
|
||
Assignee | ||
Comment 6•14 years ago
|
||
I tried creating a patch by reference to the following document:
"Adding a PnP Device to a Running System"
http://msdn.microsoft.com/en-us/library/ff540535%28v=VS.85%29.aspx
approach:
1. Get "Driver" value from HKLM\System\CurrentControlSet\Enum\<enumerator>\<deviceID>\<instanceID>.
2. Open HKLM\System\CurrentControlSet\Control\Class\<DriverValue> and get DriverVersion and DriverDate.
Worrisome point is Microsoft has listed the registry's Enum branch for debugging purposes only.
When using SetupAPI, it seems that we can avoid the direct access of Enum key, but I think Windows CE 6 may not include SetupAPI. :-(
Assignee | ||
Comment 7•14 years ago
|
||
(In reply to comment #6)
I think this approach needs the instance ID of device which displays primary desktop, but I haven't found a way to get it...
> approach:
> 1. Get "Driver" value from
> HKLM\System\CurrentControlSet\Enum\<enumerator>\<deviceID>\<instanceID>.
Reporter | ||
Updated•14 years ago
|
Summary: [gfxInfo] Driver version and date are empty for some graphic cards under Windows 2000/XP → [gfxInfo] Driver version and date are empty or null for some graphic cards under Windows 2000/XP
Comment 8•14 years ago
|
||
(In reply to comment #6)
> Worrisome point is Microsoft has listed the registry's Enum branch for
> debugging purposes only.
> When using SetupAPI, it seems that we can avoid the direct access of Enum key,
> but I think Windows CE 6 may not include SetupAPI. :-(
Do we support Windows CE at all? Does it matter for the present purpose of checking desktop graphics cards drivers?
Assignee | ||
Comment 9•14 years ago
|
||
(In reply to comment #8)
> Do we support Windows CE at all? Does it matter for the present purpose of
> checking desktop graphics cards drivers?
I don't own Windows CE and fixing this bug is beyond my knowledge...
Comment 10•14 years ago
|
||
I wouldn't worry about supporting Windows CE at all. If needed we can always #ifdef stuff out.
Comment 11•14 years ago
|
||
So, this bug is really important and we should review Tetsuro's patch. Will do ASAP...
Updated•14 years ago
|
Attachment #480478 -
Flags: review?(bjacob)
Assignee | ||
Updated•14 years ago
|
Attachment #480478 -
Flags: review?(bjacob)
Assignee | ||
Comment 12•14 years ago
|
||
My patch has a bug. I think, it is necessary to insert the following line before calling RegQueryValueExW:
dwcbData = sizeof(value);
Comment 13•14 years ago
|
||
Indeed, I had to do something similar to fix another bug recently (bug 621393)
Will apply this fix to your patch.
Let me keep the r? flag, it ensures I dont forget to review.
Assignee | ||
Comment 14•14 years ago
|
||
When unused device's informations are left in the registry's Enum branch, my current approach may get a wrong information.
So I submit a new patch using Setup API and obsolete my current patch.
Assignee | ||
Comment 15•14 years ago
|
||
Attachment #480478 -
Attachment is obsolete: true
Assignee | ||
Updated•14 years ago
|
Attachment #505046 -
Flags: review?(bjacob)
Comment 16•14 years ago
|
||
Comment on attachment 505046 [details] [diff] [review]
use Setup API rv1.0
The patch looks good!
Attachment #505046 -
Flags: review?(bjacob) → review+
Comment 17•14 years ago
|
||
Comment on attachment 505046 [details] [diff] [review]
use Setup API rv1.0
Handing this over to Jeff who has more comments.
Attachment #505046 -
Flags: review+ → review?(jmuizelaar)
Comment 18•14 years ago
|
||
Comment on attachment 505046 [details] [diff] [review]
use Setup API rv1.0
for our sanity, please check all 4 function pointers using this if condition (w && x && y && z):
>+ if (setupGetClassDevs) {
>+ if (devinfo != INVALID_HANDLE_VALUE) {
>+ HKEY key;
>+ LONG result;
>+ WCHAR value[255];
>+ DWORD dwcbData;
>+ SP_DEVINFO_DATA devinfoData;
>+ DWORD memberIndex = 0;
>+
>+ devinfoData.cbSize = sizeof(devinfoData);
reference mark A
>+ while (setupEnumDeviceInfo(devinfo, memberIndex++, &devinfoData)) {
>+ if (setupGetDeviceRegistryProperty(devinfo,
please align arguments to first argument, i.e. ^ here:
>+ &devinfoData,
>+ SPDRP_DRIVER,
>+ NULL,
>+ (PBYTE)value,
>+ sizeof(value),
>+ NULL)) {
this should be split into an NS_NAMED_LITERAL_STRING (at reference mark A above) and an nsAutoString here.
>+ nsAutoString driverKey(L"System\\CurrentControlSet\\Control\\Class\\");
Attachment #505046 -
Flags: review-
Comment 19•14 years ago
|
||
Comment on attachment 505046 [details] [diff] [review]
use Setup API rv1.0
A couple of quick comments (I haven't looked into too much detail yet):
- Please drop the WINCE stuff.
- It would be nice if the SetupAPI calls were commented more. It's not that easy to follow what they're doing.
- Does this cause us to load setupapi.dll when wouldn't otherwise? and will it hurt startup performance?
Attachment #505046 -
Flags: review?(jmuizelaar) → review-
Reporter | ||
Updated•14 years ago
|
Blocks: about:support++
Assignee | ||
Comment 20•14 years ago
|
||
* Checking all 4 function pointers of SetupAPI.
* Aligned arguments to first argument.
* Dropped the WINCE stuff, except the part of "#include <setupapi.h>".
* Added comments about SetupAPI calls.
I measured elapsed time from LoadLibraryW to FreeLibrary, using QueryPerformanceCounter.
Tested under x86 Windows 7 (CPU: Athlon X2 5050e, HDD: IC35L090AVV207-0, Mem: 2GB):
cold startup:
2.89452 ms
2.82560 ms
2.84988 ms
2.82544 ms
2.84692 ms
hot startup:
1.56996 ms
1.55084 ms
1.55232 ms
1.52980 ms
2.81968 ms
I got almost the same results under Windows 2000 SP4.
Attachment #505046 -
Attachment is obsolete: true
Attachment #505416 -
Flags: review?(jmuizelaar)
Comment 21•14 years ago
|
||
Comment on attachment 505416 [details] [diff] [review]
use SetupAPI rv1.1
>+#ifndef WINCE
>+#include <setupapi.h>
>+#endif
one WINCE left.
>+ /* create a device information set composed of the current display device */
>+ HDEVINFO devinfo = setupGetClassDevs(NULL,
>+ mDeviceID.BeginReading(),
this should use PromiseFlatString(mDeviceID).get()
>+ NULL,
>+ DIGCF_PRESENT | DIGCF_PROFILE | DIGCF_ALLCLASSES);
>+
Other than that, this looks good. Thanks a lot for getting the timing information!
Attachment #505416 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 22•14 years ago
|
||
Attachment #505416 -
Attachment is obsolete: true
Attachment #505753 -
Flags: review+
Assignee | ||
Comment 23•14 years ago
|
||
Does this patch need superreview?
If necessary, who should I request superreview to?
I'm afraid I'm not used to patch-review-checkin process...
Comment 24•14 years ago
|
||
No need for superreview, as far as I know.
Comment 25•14 years ago
|
||
Comment on attachment 505753 [details] [diff] [review]
use SetupAPI rv1.2
Nope, it just needs approval which I recommend.
Attachment #505753 -
Flags: approval2.0?
Updated•14 years ago
|
Attachment #505753 -
Flags: approval2.0? → approval2.0+
Comment 26•14 years ago
|
||
I think this should wait until beta10 is cut to minimize the risk of it breaking things right before the freeze.
Comment 27•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Assignee: nobody → tete009+bugzilla
Target Milestone: --- → mozilla2.0b11
You need to log in
before you can comment on or make changes to this bug.
Description
•