Hi, In trying to understand a bit how the driver works, I've stumbled upon a bit of unreachable code. In rhd_mc.c line 156ff there is an if: // line 156: if (rhdPtr->ChipSet < RHD_R600) { // ... // line 174: } else if (rhdPtr->ChipSet < RHD_R600) { // ... Obviously, the else if case can never be reached. On a side note, I believe the two cases could be merged by putting a few more register offsets into variables, thus reducing code redundancy. Attached patch takes the guess that the first if really wants to test against RHD_RS690 and implements the additional suggestion. Sebastian Redl
On Feb 28, 08 23:53:42 +0100, Sebastian Redl wrote:
In trying to understand a bit how the driver works, I've stumbled upon a bit of unreachable code. In rhd_mc.c line 156ff there is an if:
// line 156: if (rhdPtr->ChipSet < RHD_R600) { // ... // line 174: } else if (rhdPtr->ChipSet < RHD_R600) { // ...
gitk shows me this has been changed by Luc... Luc, do you know what's up here, and whether the patch is ok?
Obviously, the else if case can never be reached.
On a side note, I believe the two cases could be merged by putting a few more register offsets into variables, thus reducing code redundancy.
Attached patch takes the guess that the first if really wants to test against RHD_RS690 and implements the additional suggestion.
Sebastian Redl
diff --git a/src/rhd_mc.c b/src/rhd_mc.c index 9bc2397..d24de85 100644 --- a/src/rhd_mc.c +++ b/src/rhd_mc.c @@ -155,34 +155,29 @@ RHDMCSetup(RHDPtr rhdPtr)
if (rhdPtr->ChipSet < RHD_R600) { unsigned int reg; - - if (rhdPtr->ChipSet == RHD_RV515) - reg = RV515_MC_FB_LOCATION | MC_IND_ALL; - else - reg = R5XX_MC_FB_LOCATION | MC_IND_ALL; - - fb_location = RHDReadMC(rhdPtr, reg); - fb_size = (fb_location >> 16) - (fb_location & 0xFFFF); - fb_location_tmp = rhdPtr->FbIntAddress >> 16; - fb_location_tmp |= (fb_location_tmp + fb_size) << 16; - - RHDDebug(rhdPtr->scrnIndex, "%s: fb_location: 0x%08X " - "[fb_size: 0x%04X] -> fb_location: 0x%08X\n", - __func__, (unsigned int)fb_location, - fb_size,(unsigned int)fb_location_tmp); - RHDWriteMC(rhdPtr, reg | MC_IND_WR_EN, fb_location_tmp); - } else if (rhdPtr->ChipSet < RHD_R600) { - fb_location = RHDReadMC(rhdPtr, RS69_MCCFG_FB_LOCATION); - fb_size = (fb_location >> 16) - (fb_location & 0xFFFF); - fb_location_tmp = rhdPtr->FbIntAddress >> 16; - fb_location_tmp |= (fb_location_tmp + fb_size) << 16; - - RHDDebug(rhdPtr->scrnIndex, "%s: fb_location: 0x%08X " - "[fb_size: 0x%04X] -> fb_location: 0x%08X\n", - __func__, (unsigned int)fb_location, - fb_size,(unsigned int)fb_location_tmp); - RHDWriteMC(rhdPtr, RS69_C_IND_WR_EN | RS69_MCCFG_FB_LOCATION, - fb_location_tmp); + unsigned int en_flag; + + if(rhdPtr->ChipSet < RHD_RS690) { + if (rhdPtr->ChipSet == RHD_RV515) + reg = RV515_MC_FB_LOCATION | MC_IND_ALL; + else + reg = R5XX_MC_FB_LOCATION | MC_IND_ALL; + en_flag = MC_IND_WR_EN; + } else { + reg = RS69_MCCFG_FB_LOCATION; + en_flag = RS69_C_IND_WR_EN; + } + + fb_location = RHDReadMC(rhdPtr, reg); + fb_size = (fb_location >> 16) - (fb_location & 0xFFFF); + fb_location_tmp = rhdPtr->FbIntAddress >> 16; + fb_location_tmp |= (fb_location_tmp + fb_size) << 16; + + RHDDebug(rhdPtr->scrnIndex, "%s: fb_location: 0x%08X " + "[fb_size: 0x%04X] -> fb_location: 0x%08X\n", + __func__, (unsigned int)fb_location, + fb_size,(unsigned int)fb_location_tmp); + RHDWriteMC(rhdPtr, reg | en_flag, fb_location_tmp); } else { fb_location = RHDRegRead(rhdPtr, R6XX_MC_VM_FB_LOCATION); fb_size = (fb_location >> 16) - (fb_location & 0xFFFF);
CU
Matthias
--
Matthias Hopf
participants (2)
-
Matthias Hopf
-
Sebastian Redl