On Monday 14 January 2002 9:57 am, Sottek, Matthew J wrote:
> Why do you think that isn't a virtual address? agp_alloc_page
> gets a page and returns page_addresss(page) which is the kernel
> virtual address for the page, right?
agp_alloc_page (agp_generic_alloc_page in the i810 case) indeed returns a
kernel virtual address, but the next line I quoted:
new->memory[0] =
agp_bridge.mask_memory(
virt_to_phys((void *) new->memory[0]),
type);
converts it to a physical address, then intel_i810_mask_memory() ORs in
the GATT "valid" bit. The next line I quoted:
new->physical = virt_to_phys((void *) new->memory[0]);
takes that masked physical address and applies virt_to_phys to it, which
just seems wrong. Like I said, I don't have an i810, and I haven't
exhaustively analyzed it to see, for instance, whether new->physical is
even used anywhere, but it still looks wrong to me.
intel_i830_alloc_by_type() contains very similar code that makes more
sense to me:
if (type == AGP_PHYS_MEMORY) {
unsigned long physical;
...
nw->memory[0] = agp_bridge.agp_alloc_page();
physical = nw->memory[0];
...
nw->memory[0] = agp_bridge.mask_memory(virt_to_phys((void
*) nw->memory[0]),type);
...
nw->physical = virt_to_phys((void *) physical);
Although the temporary "physical" is mis-named, since it actually contains
a kernel virtual address :-)
Bjorn
> -----Original Message-----
> From: Bjorn Helgaas [mailto:[EMAIL PROTECTED]]
> Sent: Friday, January 11, 2002 10:31 AM
> To: [EMAIL PROTECTED]
> Subject: [Dri-devel] i810 agpgart curiosity
>
>
> The following code in agpgart_be.c looks bogus to me:
>
> static agp_memory *intel_i810_alloc_by_type(size_t pg_count, int type)
> ...
> if(type == AGP_PHYS_MEMORY) {
> ...
> new->memory[0] = agp_bridge.agp_alloc_page();
> ...
> new->memory[0] =
> agp_bridge.mask_memory(
> virt_to_phys((void *)
> new->memory[0]), type);
> ...
> new->physical = virt_to_phys((void *) new->memory[0]);
>
> I don't have an i810, so I can't verify anything, but it looks like it
> puts garbage in new->physical. At least, it's calling virt_to_phys() on
> something that is definitely NOT a virtual address!
>
> Any i810 gurus care to comment?
--
Bjorn Helgaas - [EMAIL PROTECTED]
Linux Systems Operation R&D
Hewlett-Packard
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
