>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.

Yup, you are right. It is a physical address, but it isn't correct.
The physical address has the PTE_VALID bit set. This doesn't break
anything since it is page aligned and the PTE_VALID is bit 1... so
we are just off by 1. I just verified it by printing out the result.

The physical address IS used. It is the only way to do hardware cursor
on the Intel platforms. Luckily they further align the address before
using it so the off by one doesn't break anything.

You are correct, it is certainly wrong. The i830 way looks like the
right way to go... except you can drop incorrect "physical" by doing it
like this...

        if (type == AGP_PHYS_MEMORY) {
                unsigned long physical;
                ...
                nw->memory[0] = agp_bridge.agp_alloc_page();
                nw->physical = virt_to_phys((void *)nw->memory[0]);
                ...
                nw->memory[0] = agp_bridge.mask_memory(
                        nw->physical,type);


BTW: has anyone else noticed that 820_cleanup isn't used in 2.4.17?
Is it supposed to be used, or is the generic cleanup ok? It either
needs to get dropped or get used, right now the compile dumps a
warning.

-Matt

_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to