> These mangled physical addresses are used by DRM, for example in 
> DRM(vm_nopage), where those extra chipset-specific bits have 
> to be masked out to obtain the actual physical address.

And your change simplifies a lot. I see and would subscribe to it.

> I propose a change along the lines of the following patch, 
> which makes the 
> elements of memory[] plain physical addresses and keeps all the 
> chipset-specific bits encapsulated inside the AGP/GART module.  The 
> general idea is to move the "masking" (chipset-specific 
> address mangling) 
> from the memory allocation path to the GATT insertion path.

Its a one time initalisation that only needs a small bit more
of memory for the data, while in for the handling of the data
the code gets much simpler, easier to understand and a faster.

That nicer data structures enforce a change to the better in code.

> I'm not a DRM guru, so I'm sure there are subtleties I'm 
> missing, and I 
> know there are some interface versioning issues to work out, but I'd 
> appreciate any feedback on the approach and advice on how to 
> proceed.  The 
> attached patch is against 2.4.16 with the IA64 patch applied.

I havent looked at any IA64 patches yet, but i think if considered
as just another sort of chipset it doesnt hurt if made availaibel.
At least its easier to maintain when all versions are put aside.

So far as i have seen your patch, i only feel a bit unsure on what
problems that PAGE_MASK_INTERFACE changes might raise. It is part
of the exported symbol/function agp_copy_info(), even if only of
use between front and backend, anybody could have made use of it
by attaching to it. If no one ever tries to access this member
then its okay, else something is broken up until the box "oopses".

If really changed, then the agpgart module version number should 
receive incrementation. Keeping this structure member for the 
current stable kernel series would be the prefered solution for me.
And it might provide an easier migration path if some coding,
i.e. experimental patches for new chipsets, still rely on it.
But the member should be marked as "dont use - will vanish".

My expirience:
Some other day i suggested that agp_enable() should return
a status value for indicating that operation did succed, 
(thus incrementing agpgart version to 1.00) but even this 
pretty compatible change (at least for ix86) didnt make 
it into any stable kernel series.

OT:
Does someone know about Jeff Hartmann? Is he well and up again?

Regards Alex.

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

Reply via email to