[rfc] Radeon AGP support patches

[Tijl Coosemans]

On Wed, 29 Oct 2014 19:03:29 +0200 Konstantin Belousov <kostikbel at gmail.com> wrote:
> On Mon, Oct 27, 2014 at 10:10:58PM +0100, Tijl Coosemans wrote:
>> On Mon, 27 Oct 2014 18:27:53 +0200 Konstantin Belousov <kostikbel at gmail.com> wrote:
>>> On Mon, Oct 27, 2014 at 05:00:55PM +0100, Tijl Coosemans wrote:
>>>> In ttm_agp_bind the ttm->pages array is already populated.  These are
>>>> the pages that need to be put into the GTT.  The patch modifies struct
>>>> agp_memory in sys/dev/agp such that ttm->pages can be passed via
>>>> agp_bind_memory.  Maybe it would be better to add two new functions to
>>>> sys/dev/agp/agp.c: agp_bind_pages and agp_unbind_pages.  These would
>>>> take a vm_page_t array as argument and bind/unbind the pages directly
>>>> in the GTT.  There's no need for ttm_agp_bind to call agp_alloc_memory
>>>> then and struct agp_memory would not be involved at all.  Does that
>>>> sound better?
>>> 
>>> Yes, this approach is much better IMO.  Having discriminated storage
>>> for the bound pages is too ugly;
>> 
>> New patch 1 & 3 attached.
>> 
>>> was the whole code audited for correctness after the change ?
>> 
>> I'm fairly confident these patches are all that's needed yes.  I made
>> a first implementation on Sunday afternoon.  It got to the point that
>> X showed a mouse pointer and background colour and then it crashed.
>> It took the rest of the week to figure out why (NULL dereference in
>> ttm_bo_vm_fault) and how to solve it (mark aperture range fictitious).
>> It's hard to debug something without a screen.  I read the code front
>> to back and back to front in that time and compared it with the old DRM
>> code and with the Linux DRM code.  That's where patch 2 & 4 come from.
> Looks good, commit. See trivial notes below.

Thanks.  Just to be sure, did you review 2 & 4 as well?