in 64-bit Linux, we found radeon driver will print out absurdly-large in the log messege II) RADEON(0): PLL parameters: rf=2700 rd=12 min=20000 max=47996259547800; xclk=20000 this problem is because print format mismatch data type. pll->max_pll_freq is defined as a 32-bit unsigned integer -- type CARD32. To change the print format from ‘%ld’to "%u" will solve this problem. we have noticed that git checkin 'd7230939f523610c57f92bdfc72966bdbc6f1070: 64 bit warning fixes' has attepmted to adjust printf format to match CARD32, but it changes pll->max_pll_freq print format from '%ld' to '%d', we have concerns that '%d' will still have problem to handle larger numbers. so we suggest do an adjust to change '%d' to '%u'.
Created attachment 12987 [details] [review] patch against git main-branch
Pushed, thanks. (The clock limit would have to be extremely high for it to matter though :)
right, when pixel clock reaches to no less than 2Ghz, will this problem happen :)
Actually I think it's 2 Terahertz even. :)
Well, I believe this bug isn't fixed yet. I know this is just a log message, but it still cost me some time to figure out what was at fault when debugging driver/VBIOS issues. What good is a log message if it can't be trusted? I think this statement is (unfortunately) going to need to be #ifdef'ed between AMD64 and i386 compiles. This is because CARD32 is defined differently in AMD64 and i386... ########## Take a look at X11/Xmd.h #ifdef LONG64 typedef unsigned long CARD64; typedef unsigned int CARD32; #else typedef unsigned long CARD32; #endif ########## Also in X11/Xmd.h typedef unsigned short CARD16; ########## Oddly, this would mean that CARD16 is really 32 bits... ########## Why bother change CARD32 between platforms and keep CARD16? ########## I don't know, but I'm not going to attempt to open that can of worms... ########## Then take a look at radeon.h typedef struct { CARD16 reference_freq; CARD16 reference_div; CARD32 min_pll_freq; CARD32 max_pll_freq; CARD16 xclk; } RADEONPLLRec, *RADEONPLLPtr; Looking at the above, I would guess that the code for the debug message should look like this: #ifdef LONG64 xf86DrvMsg (pScrn->scrnIndex, X_INFO, "PLL parameters: rf=%u rd=%u min=%u max=%u; xclk=%u\n", pll->reference_freq, pll->reference_div, pll->min_pll_freq, pll->max_pll_freq, pll->xclk); #else xf86DrvMsg (pScrn->scrnIndex, X_INFO, "PLL parameters: rf=%u rd=%u min=%lu max=%lu; xclk=%u\n", pll->reference_freq, pll->reference_div, pll->min_pll_freq, pll->max_pll_freq, pll->xclk); #endif
Can't you just explictly cast arguments to unsigned long and use %lu? That will avoid ugly ifdefs.
(In reply to comment #5) > ########## Also in X11/Xmd.h > typedef unsigned short CARD16; > ########## Oddly, this would mean that CARD16 is really 32 bits... On what platform that Xorg supports is a short anything but 16-bits?
(In reply to comment #6) > Can't you just explictly cast arguments to unsigned long and use %lu? That will > avoid ugly ifdefs. > I hadn't thought of this; it sounds even better to me. This is exactly why I'm not submitting any patches. :-)
(In reply to comment #7) > (In reply to comment #5) > > ########## Also in X11/Xmd.h > > typedef unsigned short CARD16; > > ########## Oddly, this would mean that CARD16 is really 32 bits... > > On what platform that Xorg supports is a short anything but 16-bits? > My understanding is that there is no distinction between "short int" and "int" in C. "short" is an optional disambiguation.
(In reply to comment #9) > My understanding is that there is no distinction between "short int" and "int" > in C. "short" is an optional disambiguation. > I apologize, I just checked the C99 standard. There /is/ a distinction between "short int" and "int."
The referenced commit basically addressed comment #5: As CARD32 values can never exceed 32 bits, using %u (or %d, ignoring the MSB) and casts to unsigned works correctly on 32 and 64 bit platforms. The issue reported here has been fixed, so resolving again. Feel free to file another bug if there have since been regressions in this regard or if you feel strongly about fixing the remaining cases of %d.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.