On Wednesday 13 February 2002 11:26 pm, Jeff Hartmann wrote: > Just a few quick comments, it looks like you removed the need for the > num_of_masks variable inside the agp_bridge structure. I'd prefer to > remove this entirely if all places where it is referenced are removed. > There are advantages to doing the masking beforehand, however at the > moment inserting and removing agp memory is not very important to the > DRI. This might not be the case forever though. Perhaps allowing the > chipset to decide the behavior would be the best solution.
Thanks for catching the lingering num_of_masks. Allowing the chipset to decide sounds reasonable, but given that (a) there's no performance issue for current DRI usage, and (b) a simple mask can't handle the general case anyway (i.e,. 460GX), it doesn't seem worth the complexity to go down the per-chipset path yet. Attached is an updated patch against 2.5.5-pre1. -- Bjorn Helgaas - [EMAIL PROTECTED] Linux Systems Operation R&D Hewlett-Packard Export plain physical addresses in agp_memory.memory[]. Previously memory[] contained GATT entries and agpgart exported a page_mask. DRM converts GATT entries to physical addresses thusly: phys_addr = memory[i] & page_mask; but 460GX requires a shift as well as a mask, so the ia64 patch has a chipset-specific wart like this: #if defined(__ia64__) phys_addr = (memory[i] & page_mask) << 12 #else phys_addr = memory[i] & page_mask; #endif This patch changes agpgart so memory[] contains plain physical addresses, page_mask = ~0UL so the old DRM code continues to work and no ia64-specific wart is needed, and agpgart's GATT-insertion functions do the physical->GATT conversion themselves. diff -u -ur linux-2.5.5-pre1/drivers/char/agp/agp.h build/linux-2.5.5-pre1/drivers/char/agp/agp.h --- linux-2.5.5-pre1/drivers/char/agp/agp.h Thu Feb 14 13:02:32 2002 +++ build/linux-2.5.5-pre1/drivers/char/agp/agp.h Thu Feb 14 13:04:08 2002 @@ -99,7 +99,6 @@ int needs_scratch_page; int aperture_size_idx; int num_aperture_sizes; - int num_of_masks; int capndx; int cant_use_aperture; diff -u -ur linux-2.5.5-pre1/drivers/char/agp/agpgart_be.c build/linux-2.5.5-pre1/drivers/char/agp/agpgart_be.c --- linux-2.5.5-pre1/drivers/char/agp/agpgart_be.c Thu Feb 14 13:02:32 2002 +++ build/linux-2.5.5-pre1/drivers/char/agp/agpgart_be.c Thu Feb 14 13:04:08 +2002 @@ -207,7 +207,6 @@ } if (curr->page_count != 0) { for (i = 0; i < curr->page_count; i++) { - curr->memory[i] &= ~(0x00000fff); agp_bridge.agp_destroy_page((unsigned long) phys_to_virt(curr->memory[i])); } @@ -260,10 +259,7 @@ agp_free_memory(new); return NULL; } - new->memory[i] = - agp_bridge.mask_memory( - virt_to_phys((void *) new->memory[i]), - type); + new->memory[i] = virt_to_phys((void *) new->memory[i]); new->page_count++; } @@ -307,9 +303,6 @@ void agp_copy_info(agp_kern_info * info) { - unsigned long page_mask = 0; - int i; - memset(info, 0, sizeof(agp_kern_info)); if (agp_bridge.type == NOT_SUPPORTED) { info->chipset = agp_bridge.type; @@ -325,11 +318,7 @@ info->max_memory = agp_bridge.max_memory_agp; info->current_memory = atomic_read(&agp_bridge.current_memory_agp); info->cant_use_aperture = agp_bridge.cant_use_aperture; - - for(i = 0; i < agp_bridge.num_of_masks; i++) - page_mask |= agp_bridge.mask_memory(page_mask, i); - - info->page_mask = ~page_mask; + info->page_mask = ~0UL; } /* End - Routine to copy over information structure */ @@ -756,7 +745,8 @@ mem->is_flushed = TRUE; } for (i = 0, j = pg_start; i < mem->page_count; i++, j++) { - agp_bridge.gatt_table[j] = mem->memory[i]; + agp_bridge.gatt_table[j] = + agp_bridge.mask_memory(mem->memory[i], mem->type); } agp_bridge.tlb_flush(mem); @@ -993,7 +983,8 @@ CACHE_FLUSH(); for (i = 0, j = pg_start; i < mem->page_count; i++, j++) { OUTREG32(intel_i810_private.registers, - I810_PTE_BASE + (j * 4), mem->memory[i]); + I810_PTE_BASE + (j * 4), + agp_bridge.mask_memory(mem->memory[i], mem->type)); } CACHE_FLUSH(); @@ -1059,10 +1050,7 @@ agp_free_memory(new); return NULL; } - new->memory[0] = - agp_bridge.mask_memory( - virt_to_phys((void *) new->memory[0]), - type); + new->memory[0] = virt_to_phys((void *) new->memory[0]); new->page_count = 1; new->num_scratch_pages = 1; new->type = AGP_PHYS_MEMORY; @@ -1096,7 +1084,6 @@ intel_i810_private.i810_dev = i810_dev; agp_bridge.masks = intel_i810_masks; - agp_bridge.num_of_masks = 2; agp_bridge.aperture_sizes = (void *) intel_i810_sizes; agp_bridge.size_type = FIXED_APER_SIZE; agp_bridge.num_aperture_sizes = 2; @@ -1298,7 +1285,8 @@ CACHE_FLUSH(); for (i = 0, j = pg_start; i < mem->page_count; i++, j++) - OUTREG32(intel_i830_private.registers,I810_PTE_BASE + (j * 4),mem->memory[i]); + OUTREG32(intel_i830_private.registers,I810_PTE_BASE + (j * 4), + agp_bridge.mask_memory(mem->memory[i], mem->type)); CACHE_FLUSH(); @@ -1359,7 +1347,7 @@ return(NULL); } - nw->memory[0] = agp_bridge.mask_memory(virt_to_phys((void *) nw->memory[0]),type); + nw->memory[0] = virt_to_phys((void *) nw->memory[0]); nw->page_count = 1; nw->num_scratch_pages = 1; nw->type = AGP_PHYS_MEMORY; @@ -1375,7 +1363,6 @@ intel_i830_private.i830_dev = i830_dev; agp_bridge.masks = intel_i810_masks; - agp_bridge.num_of_masks = 3; agp_bridge.aperture_sizes = (void *) intel_i830_sizes; agp_bridge.size_type = FIXED_APER_SIZE; agp_bridge.num_aperture_sizes = 2; @@ -1794,7 +1781,6 @@ static int __init intel_generic_setup (struct pci_dev *pdev) { agp_bridge.masks = intel_generic_masks; - agp_bridge.num_of_masks = 1; agp_bridge.aperture_sizes = (void *) intel_generic_sizes; agp_bridge.size_type = U16_APER_SIZE; agp_bridge.num_aperture_sizes = 7; @@ -1828,7 +1814,6 @@ static int __init intel_820_setup (struct pci_dev *pdev) { agp_bridge.masks = intel_generic_masks; - agp_bridge.num_of_masks = 1; agp_bridge.aperture_sizes = (void *) intel_8xx_sizes; agp_bridge.size_type = U8_APER_SIZE; agp_bridge.num_aperture_sizes = 7; @@ -1861,7 +1846,6 @@ static int __init intel_830mp_setup (struct pci_dev *pdev) { agp_bridge.masks = intel_generic_masks; - agp_bridge.num_of_masks = 1; agp_bridge.aperture_sizes = (void *) intel_830mp_sizes; agp_bridge.size_type = U8_APER_SIZE; agp_bridge.num_aperture_sizes = 4; @@ -1894,7 +1878,6 @@ static int __init intel_840_setup (struct pci_dev *pdev) { agp_bridge.masks = intel_generic_masks; - agp_bridge.num_of_masks = 1; agp_bridge.aperture_sizes = (void *) intel_8xx_sizes; agp_bridge.size_type = U8_APER_SIZE; agp_bridge.num_aperture_sizes = 7; @@ -1927,7 +1910,6 @@ static int __init intel_845_setup (struct pci_dev *pdev) { agp_bridge.masks = intel_generic_masks; - agp_bridge.num_of_masks = 1; agp_bridge.aperture_sizes = (void *) intel_8xx_sizes; agp_bridge.size_type = U8_APER_SIZE; agp_bridge.num_aperture_sizes = 7; @@ -1960,7 +1942,6 @@ static int __init intel_850_setup (struct pci_dev *pdev) { agp_bridge.masks = intel_generic_masks; - agp_bridge.num_of_masks = 1; agp_bridge.aperture_sizes = (void *) intel_8xx_sizes; agp_bridge.size_type = U8_APER_SIZE; agp_bridge.num_aperture_sizes = 7; @@ -1993,7 +1974,6 @@ static int __init intel_860_setup (struct pci_dev *pdev) { agp_bridge.masks = intel_generic_masks; - agp_bridge.num_of_masks = 1; agp_bridge.aperture_sizes = (void *) intel_8xx_sizes; agp_bridge.size_type = U8_APER_SIZE; agp_bridge.num_aperture_sizes = 7; @@ -2113,7 +2093,6 @@ static int __init via_generic_setup (struct pci_dev *pdev) { agp_bridge.masks = via_generic_masks; - agp_bridge.num_of_masks = 1; agp_bridge.aperture_sizes = (void *) via_generic_sizes; agp_bridge.size_type = U8_APER_SIZE; agp_bridge.num_aperture_sizes = 7; @@ -2227,7 +2206,6 @@ static int __init sis_generic_setup (struct pci_dev *pdev) { agp_bridge.masks = sis_generic_masks; - agp_bridge.num_of_masks = 1; agp_bridge.aperture_sizes = (void *) sis_generic_sizes; agp_bridge.size_type = U8_APER_SIZE; agp_bridge.num_aperture_sizes = 7; @@ -2559,7 +2537,8 @@ for (i = 0, j = pg_start; i < mem->page_count; i++, j++) { addr = (j * PAGE_SIZE) + agp_bridge.gart_bus_addr; cur_gatt = GET_GATT(addr); - cur_gatt[GET_GATT_OFF(addr)] = mem->memory[i]; + cur_gatt[GET_GATT_OFF(addr)] = + agp_bridge.mask_memory(mem->memory[i], mem->type); } agp_bridge.tlb_flush(mem); return 0; @@ -2605,7 +2584,6 @@ static int __init amd_irongate_setup (struct pci_dev *pdev) { agp_bridge.masks = amd_irongate_masks; - agp_bridge.num_of_masks = 1; agp_bridge.aperture_sizes = (void *) amd_irongate_sizes; agp_bridge.size_type = LVL2_APER_SIZE; agp_bridge.num_aperture_sizes = 7; @@ -2853,7 +2831,6 @@ static int __init ali_generic_setup (struct pci_dev *pdev) { agp_bridge.masks = ali_generic_masks; - agp_bridge.num_of_masks = 1; agp_bridge.aperture_sizes = (void *) ali_generic_sizes; agp_bridge.size_type = U32_APER_SIZE; agp_bridge.num_aperture_sizes = 7; @@ -3261,7 +3238,8 @@ for (i = 0, j = pg_start; i < mem->page_count; i++, j++) { addr = (j * PAGE_SIZE) + agp_bridge.gart_bus_addr; cur_gatt = SVRWRKS_GET_GATT(addr); - cur_gatt[GET_GATT_OFF(addr)] = mem->memory[i]; + cur_gatt[GET_GATT_OFF(addr)] = + agp_bridge.mask_memory(mem->memory[i], mem->type); } agp_bridge.tlb_flush(mem); return 0; @@ -3451,7 +3429,6 @@ serverworks_private.svrwrks_dev = pdev; agp_bridge.masks = serverworks_masks; - agp_bridge.num_of_masks = 1; agp_bridge.aperture_sizes = (void *) serverworks_sizes; agp_bridge.size_type = LVL2_APER_SIZE; agp_bridge.num_aperture_sizes = 7; _______________________________________________ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel