[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freedesktop-xorg
Subject:    Re: Radeon MC setup cleanup
From:       Maciej Cencora <m.cencora () gmail ! com>
Date:       2008-02-29 21:20:28
Message-ID: 47C8771C.3000704 () gmail ! com
[Download RAW message or body]

Jerome Glisse pisze:
> On Fri, 29 Feb 2008 17:31:46 +0100
> Maciej Cencora <m.cencora@gmail.com> wrote:
>
>   
>> Hi all,
>>
>> I'm trying to understand and cleanup MC setup in Radeon DDX driver. 
>> Currently it's overcomplicated and sometimes unreliable.
>>
>> That's how I understand how MC should be setup. Please check if I 
>> understand it correctly.
>>
>> When DRI is disabled, fb/agp locations should not change during whole 
>> session (even when doing VT switch) so there is no need to save/restore 
>> them.
>>
>> Below I describe the cases when DRI is enabled.
>> -- User starts the machine --
>> Initial reg values are set by BIOS.
>>
>> -- User starts X --
>> 1. Store regs in info->SavedReg.
>> 2. Init DRI (it sets new values to fb/agp location regs).
>> 3. Save new fb/agp location values to info->ModeReg.
>>
>> -- User calls LeaveVT --
>> Restore regs from info->SavedReg and do the rest of LeaveVT.
>>
>> -- User backs to X (EnterVT) --
>> Restore regs from info->ModeReg and do the rest of EnterVT.
>>
>> -- User closes X -- (that case is similar to LeaveVT)
>> Restore regs from info->SavedReg and do the rest of LeaveVT.
>>
>>
>> Regards,
>> Maciej Cencora
>>     
>
> What you are outlining is already done see RADEONRestore
> in radeon_driver.c. Or do i misunderstand the problem you
> are trying to solve ?
>
> Note that when X close we call LeaveVT (at least this what
> my memory told me :))
>
> Cheers,
> Jerome Glisse <glisse@freedesktop.org>
>
>   
You misunderstand. I attach the proposed patch. It works for me.

What I do in the patch is:
- use info->ModeReg and info->SavedReg only
- remove info->mc_fb/agp_location, remove RADEONInitMemMapRegisters
- only two places to restore regs for entering and leaving vt (removed
from atombios_crtc_mode_set and legacy_crtc_mode_set)
- set RADEONRestoreMemMapRegisters static
- remove RADEONRestoreMemMapRegisters call in RADEONAdjustMemMapRegisters

There's only one thing to do. Restore/Save memmap regs only when dri is
enabled.

Regards,
Maciej Cencora


["my2" (text/plain)]

diff --git a/src/atombios_crtc.c b/src/atombios_crtc.c
index bc2df18..f7d9c37 100644
--- a/src/atombios_crtc.c
+++ b/src/atombios_crtc.c
@@ -330,9 +330,6 @@ atombios_crtc_mode_set(xf86CrtcPtr crtc,
     ErrorF("Mode %dx%d - %d %d %d\n", adjusted_mode->CrtcHDisplay, adjusted_mode->CrtcVDisplay,
 	   adjusted_mode->CrtcHTotal, adjusted_mode->CrtcVTotal, adjusted_mode->Flags);
 
-    RADEONInitMemMapRegisters(pScrn, info->ModeReg, info);
-    RADEONRestoreMemMapRegisters(pScrn, info->ModeReg);
-
     if (IS_AVIVO_VARIANT) {
 	radeon_crtc->fb_width = mode->CrtcHDisplay;
 	radeon_crtc->fb_height = pScrn->virtualY;
diff --git a/src/legacy_crtc.c b/src/legacy_crtc.c
index 06ad60c..52e49cc 100644
--- a/src/legacy_crtc.c
+++ b/src/legacy_crtc.c
@@ -1693,8 +1693,6 @@ legacy_crtc_mode_set(xf86CrtcPtr crtc, DisplayModePtr mode,
     }
 
 
-    ErrorF("init memmap\n");
-    RADEONInitMemMapRegisters(pScrn, info->ModeReg, info);
     ErrorF("init common\n");
     RADEONInitCommonRegisters(info->ModeReg, info);
 
@@ -1748,8 +1746,6 @@ legacy_crtc_mode_set(xf86CrtcPtr crtc, DisplayModePtr mode,
 	}
     }
 
-    ErrorF("restore memmap\n");
-    RADEONRestoreMemMapRegisters(pScrn, info->ModeReg);
     ErrorF("restore common\n");
     RADEONRestoreCommonRegisters(pScrn, info->ModeReg);
 
diff --git a/src/radeon.h b/src/radeon.h
index 9c83a4f..b00d1e2 100644
--- a/src/radeon.h
+++ b/src/radeon.h
@@ -355,9 +355,6 @@ typedef struct {
     unsigned long     BIOSAddr;         /* BIOS physical address             */
     CARD32            fbLocation;
     CARD32            gartLocation;
-    CARD32            mc_fb_location;
-    CARD32            mc_agp_location;
-    CARD32            mc_agp_location_hi;
 
     void              *MMIO;            /* Map of MMIO region                */
     void              *FB;              /* Map of frame buffer               */
@@ -817,8 +814,6 @@ extern Bool        RADEONGetTVInfoFromBIOS (xf86OutputPtr output);
 extern Bool        RADEONGetDAC2InfoFromBIOS (xf86OutputPtr output);
 extern Bool        RADEONGetHardCodedEDIDFromBIOS (xf86OutputPtr output);
 
-extern void        RADEONRestoreMemMapRegisters(ScrnInfoPtr pScrn,
-						RADEONSavePtr restore);
 extern void        RADEONRestoreCommonRegisters(ScrnInfoPtr pScrn,
 						RADEONSavePtr restore);
 extern void        RADEONRestoreCrtcRegisters(ScrnInfoPtr pScrn,
@@ -840,9 +835,6 @@ extern void        RADEONRestoreCrtc2Registers(ScrnInfoPtr pScrn,
 extern void        RADEONRestorePLL2Registers(ScrnInfoPtr pScrn,
 					      RADEONSavePtr restore);
 
-extern void        RADEONInitMemMapRegisters(ScrnInfoPtr pScrn,
-					     RADEONSavePtr save,
-					     RADEONInfoPtr info);
 extern void        RADEONInitDispBandwidth(ScrnInfoPtr pScrn);
 extern Bool        RADEONI2cInit(ScrnInfoPtr pScrn);
 extern Bool        RADEONSetupConnectors(ScrnInfoPtr pScrn);
diff --git a/src/radeon_atombios.c b/src/radeon_atombios.c
diff --git a/src/radeon_driver.c b/src/radeon_driver.c
index c38e39b..18755ef 100644
--- a/src/radeon_driver.c
+++ b/src/radeon_driver.c
@@ -1207,30 +1207,16 @@ static Bool RADEONPreInitWeight(ScrnInfoPtr pScrn)
     return TRUE;
 }
 
-void RADEONInitMemMapRegisters(ScrnInfoPtr pScrn, RADEONSavePtr save,
-				      RADEONInfoPtr info)
-{
-    save->mc_fb_location = info->mc_fb_location;
-    save->mc_agp_location = info->mc_agp_location;
-
-    if (IS_AVIVO_VARIANT) {
-	save->mc_agp_location_hi = info->mc_agp_location_hi;
-    } else {
-	save->display_base_addr = info->fbLocation;
-	save->display2_base_addr = info->fbLocation;
-	save->ov0_base_addr = info->fbLocation;
-    }
-}
-
 static void RADEONInitMemoryMap(ScrnInfoPtr pScrn)
 {
     RADEONInfoPtr  info   = RADEONPTR(pScrn);
     unsigned char *RADEONMMIO = info->MMIO;
+    RADEONSavePtr  modeReg = info->ModeReg;
     CARD32 mem_size;
     CARD32 aper_size;
 
-    radeon_read_mc_fb_agp_location(pScrn, LOC_FB | LOC_AGP, &info->mc_fb_location,
-				   &info->mc_agp_location, &info->mc_agp_location_hi);
+    radeon_read_mc_fb_agp_location(pScrn, LOC_FB | LOC_AGP, &modeReg->mc_fb_location,
+				   &modeReg->mc_agp_location, &modeReg->mc_agp_location_hi);
 
     /* We shouldn't use info->videoRam here which might have been clipped
      * but the real video RAM instead
@@ -1261,13 +1247,13 @@ static void RADEONInitMemoryMap(ScrnInfoPtr pScrn)
 
     if (info->ChipFamily != CHIP_FAMILY_RS690) {
 	if (info->IsIGP)
-	    info->mc_fb_location = INREG(RADEON_NB_TOM);
+	    modeReg->mc_fb_location = INREG(RADEON_NB_TOM);
 	else
 #ifdef XF86DRI
 	/* Old DRI has restrictions on the memory map */
 	if ( info->directRenderingEnabled &&
 	     info->pKernelDRMVersion->version_minor < 10 )
-	    info->mc_fb_location = (mem_size - 1) & 0xffff0000U;
+	    modeReg->mc_fb_location = (mem_size - 1) & 0xffff0000U;
 	else
 #endif
 	{
@@ -1295,19 +1281,19 @@ static void RADEONInitMemoryMap(ScrnInfoPtr pScrn)
 		    aper0_base &= ~(mem_size - 1);
 
 	    if (info->ChipFamily >= CHIP_FAMILY_R600) {
-		info->mc_fb_location = (aper0_base >> 24) |
+		modeReg->mc_fb_location = (aper0_base >> 24) |
 		    (((aper0_base + mem_size - 1) & 0xff000000U) >> 8);
-		ErrorF("mc fb loc is %08x\n", (unsigned int)info->mc_fb_location);
+		ErrorF("mc fb loc is %08x\n", (unsigned int)modeReg->mc_fb_location);
 	    } else {
-		info->mc_fb_location = (aper0_base >> 16) |
+		modeReg->mc_fb_location = (aper0_base >> 16) |
 		    ((aper0_base + mem_size - 1) & 0xffff0000U);
 	    }
 	}
     }
     if (info->ChipFamily >= CHIP_FAMILY_R600) {
-	info->fbLocation = (info->mc_fb_location & 0xffff) << 24;
+	info->fbLocation = (modeReg->mc_fb_location & 0xffff) << 24;
     } else {
-   	info->fbLocation = (info->mc_fb_location & 0xffff) << 16;
+   	info->fbLocation = (modeReg->mc_fb_location & 0xffff) << 16;
     }
     /* Just disable the damn AGP apertures for now, it may be
      * re-enabled later by the DRM
@@ -1315,22 +1301,22 @@ static void RADEONInitMemoryMap(ScrnInfoPtr pScrn)
 
     if (IS_AVIVO_VARIANT) {
 	if (info->ChipFamily >= CHIP_FAMILY_R600) {
-	    OUTREG(R600_HDP_NONSURFACE_BASE, (info->mc_fb_location << 16) & 0xff0000);
+	    OUTREG(R600_HDP_NONSURFACE_BASE, (modeReg->mc_fb_location << 16) & 0xff0000);
 	} else {
-	    OUTREG(AVIVO_HDP_FB_LOCATION, info->mc_fb_location);
+	    OUTREG(AVIVO_HDP_FB_LOCATION, modeReg->mc_fb_location);
 	}
-    	info->mc_agp_location = 0x003f0000;
+    	modeReg->mc_agp_location = 0x003f0000;
     } else
-    	info->mc_agp_location = 0xffffffc0;
+    	modeReg->mc_agp_location = 0xffffffc0;
     xf86DrvMsg(pScrn->scrnIndex, X_INFO,
 	       "RADEONInitMemoryMap() : \n");
     xf86DrvMsg(pScrn->scrnIndex, X_INFO,
 	       "  mem_size         : 0x%08x\n", (unsigned)mem_size);
     xf86DrvMsg(pScrn->scrnIndex, X_INFO,
-	       "  MC_FB_LOCATION   : 0x%08x\n", (unsigned)info->mc_fb_location);
+	       "  MC_FB_LOCATION   : 0x%08x\n", (unsigned)modeReg->mc_fb_location);
     xf86DrvMsg(pScrn->scrnIndex, X_INFO,
 	       "  MC_AGP_LOCATION  : 0x%08x\n",
-	       (unsigned)info->mc_agp_location);
+	       (unsigned)modeReg->mc_agp_location);
 }
 
 static void RADEONGetVRamType(ScrnInfoPtr pScrn)
@@ -3537,7 +3523,7 @@ Bool RADEONScreenInit(int scrnIndex, ScreenPtr pScreen,
 }
 
 /* Write memory mapping registers */
-void RADEONRestoreMemMapRegisters(ScrnInfoPtr pScrn,
+static void RADEONRestoreMemMapRegisters(ScrnInfoPtr pScrn,
 					 RADEONSavePtr restore)
 {
     RADEONInfoPtr  info       = RADEONPTR(pScrn);
@@ -3759,6 +3745,7 @@ void RADEONRestoreMemMapRegisters(ScrnInfoPtr pScrn,
 static void RADEONAdjustMemMapRegisters(ScrnInfoPtr pScrn, RADEONSavePtr save)
 {
     RADEONInfoPtr  info   = RADEONPTR(pScrn);
+    RADEONSavePtr modeReg = info->ModeReg;
     CARD32 fb, agp, agp_hi;
     int changed;
 
@@ -3767,8 +3754,8 @@ static void RADEONAdjustMemMapRegisters(ScrnInfoPtr pScrn, RADEONSavePtr save)
 
     radeon_read_mc_fb_agp_location(pScrn, LOC_FB | LOC_AGP, &fb, &agp, &agp_hi);
     
-    if (fb != info->mc_fb_location || agp != info->mc_agp_location ||
-	agp_hi || info->mc_agp_location_hi)
+    if (fb != modeReg->mc_fb_location || agp != modeReg->mc_agp_location ||
+	agp_hi || modeReg->mc_agp_location_hi)
 	changed = 1;
 
     if (changed) {
@@ -3776,22 +3763,21 @@ static void RADEONAdjustMemMapRegisters(ScrnInfoPtr pScrn, RADEONSavePtr save)
 		   "DRI init changed memory map, adjusting ...\n");
 	xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
 		   "  MC_FB_LOCATION  was: 0x%08lx is: 0x%08lx\n",
-		   (long unsigned int)info->mc_fb_location, (long unsigned int)fb);
+		   (long unsigned int)modeReg->mc_fb_location, (long unsigned int)fb);
 	xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
 		   "  MC_AGP_LOCATION was: 0x%08lx is: 0x%08lx\n",
-		   (long unsigned int)info->mc_agp_location, (long unsigned int)agp);
-	info->mc_fb_location = fb;
-	info->mc_agp_location = agp;
+		   (long unsigned int)modeReg->mc_agp_location, (long unsigned int)agp);
+	modeReg->mc_fb_location = fb;
+	modeReg->mc_agp_location = agp;
 	if (info->ChipFamily >= CHIP_FAMILY_R600)
-	    info->fbLocation = (info->mc_fb_location & 0xffff) << 24;
+	    info->fbLocation = (modeReg->mc_fb_location & 0xffff) << 24;
 	else
-	    info->fbLocation = (info->mc_fb_location & 0xffff) << 16;
+	    info->fbLocation = (modeReg->mc_fb_location & 0xffff) << 16;
 
 	info->dst_pitch_offset =
 	    (((pScrn->displayWidth * info->CurrentLayout.pixel_bytes / 64)
 	      << 22) | ((info->fbLocation + pScrn->fbOffset) >> 10));
-	RADEONInitMemMapRegisters(pScrn, save, info);
-	RADEONRestoreMemMapRegisters(pScrn, save);
+
     }
 
 #ifdef USE_EXA
@@ -4968,6 +4954,8 @@ Bool RADEONEnterVT(int scrnIndex, int flags)
     if (IS_R300_VARIANT || IS_RV100_VARIANT)
 	RADEONForceSomeClocks(pScrn);
 
+    RADEONRestoreMemMapRegisters(pScrn, info->ModeReg);
+    
     pScrn->vtSema = TRUE;
     for (i = 0; i < xf86_config->num_crtc; i++) {
 	xf86CrtcPtr	crtc = xf86_config->crtc[i];
@@ -4983,7 +4971,6 @@ Bool RADEONEnterVT(int scrnIndex, int flags)
 	if (!xf86CrtcSetMode (crtc, &crtc->desiredMode, crtc->desiredRotation,
 			      crtc->desiredX, crtc->desiredY))
 	    return FALSE;
-
     }
 
     RADEONRestoreSurfaces(pScrn, info->ModeReg);
@@ -4998,8 +4985,6 @@ Bool RADEONEnterVT(int scrnIndex, int flags)
 	/* get the DRI back into shape after resume */
 	RADEONDRISetVBlankInterrupt (pScrn, TRUE);
 	RADEONDRIResume(pScrn->pScreen);
-	RADEONAdjustMemMapRegisters(pScrn, info->ModeReg);
-
     }
 #endif
     /* this will get XVideo going again, but only if XVideo was initialised
diff --git a/src/radeon_probe.h b/src/radeon_probe.h



_______________________________________________
xorg mailing list
xorg@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xorg

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic