freebsd stable - Feb 2014 - Radeon driver in stable/9: changes in VM, atomic.h and iicbus

Hello!

I'm working on the merge of the Radeon KMS driver to stable/9, to help
with a future merge of vt(9) and the activation of WITH_NEW_XORG by
default in this branch.

Here's a first version of the merge:
http://people.freebsd.org/~dumbbell/radeonkms/radeon-stable9.a.patch

However, the current driver relies on changes to other parts of the kernel:

== VM =
In my first attempt, I merged the following revisions, which add
vm_page_alloc_contig(), used by TTM:
  - 226824
  - 226848
  - 226891
  - 226928
  - 227012
  - 227072
  - 227127
  - 227568

Here's a shorter patch containing only the VM changes:
http://people.freebsd.org/~dumbbell/radeonkms/radeon-stable9-vm.a.patch

I found the following API/ABI breakage:
   o  kmem_alloc_contig()'s and vm_phys_alloc_contig()'s
"boundary"
      argument, going from unsigned long to vm_paddr_t.
   o  vm_page_alloc_init() becoming a static function.
   o  vm_phys_bootstrap_alloc() removed.

I don't know how to proceed with this. Should TTM use another function
than vm_page_alloc_contig() to avoid the MFC? Or should the merge be
modified so that "boundary" argument keeps its unsigned long type,
vm_page_alloc_init() remains public and vm_phys_bootstrap_alloc() is not
removed?

== atomic.h =
I merged the following revisions, which add new atomic operations for
both amd64 and i386:
  - 254610
  - 254611
  - 254617
  - 254620

The merge was modified to not break the API/ABI. For instance, in the
original commits, some operations were transformed from a real function
to a macro using one of the new operation.

So here's shorter patch with only the changes to atomic.h:
http://people.freebsd.org/~dumbbell/radeonkms/radeon-stable9-atomic.a.patch

Therefore, it only adds new code used by the Radeon driver, and I think
it's safe. Opinions on that?

== iicbus =
Revision 232365 was merged to add optional pre/post transfer methods to
iicbb, used by the Radeon driver.

Again, here's a patch limited to iicbb changes:
http://people.freebsd.org/~dumbbell/radeonkms/radeon-stable9-iic.a.patch

I think this merge is safe too, but I'm not confident enough :) Any
thoughts?

-- 
Jean-S?bastien P?dron

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL:
<http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20140217/047676ac/attachment.sig>

On Monday, February 17, 2014 8:44:05 am Jean-S?bastien P?dron
wrote:
> Hello!
> 
> I'm working on the merge of the Radeon KMS driver to stable/9, to help
> with a future merge of vt(9) and the activation of WITH_NEW_XORG by
> default in this branch.
> 
> Here's a first version of the merge:
> http://people.freebsd.org/~dumbbell/radeonkms/radeon-stable9.a.patch
> 
> However, the current driver relies on changes to other parts of the kernel:
> 
> == VM => 
> In my first attempt, I merged the following revisions, which add
> vm_page_alloc_contig(), used by TTM:
>   - 226824
>   - 226848
>   - 226891
>   - 226928
>   - 227012
>   - 227072
>   - 227127
>   - 227568
> 
> Here's a shorter patch containing only the VM changes:
> http://people.freebsd.org/~dumbbell/radeonkms/radeon-stable9-vm.a.patch
> 
> I found the following API/ABI breakage:
>    o  kmem_alloc_contig()'s and vm_phys_alloc_contig()'s
"boundary"
>       argument, going from unsigned long to vm_paddr_t.
>    o  vm_page_alloc_init() becoming a static function.
>    o  vm_phys_bootstrap_alloc() removed.
> 
> I don't know how to proceed with this. Should TTM use another function
> than vm_page_alloc_contig() to avoid the MFC? Or should the merge be
> modified so that "boundary" argument keeps its unsigned long
type,
> vm_page_alloc_init() remains public and vm_phys_bootstrap_alloc() is not
> removed?
You can leave the boundary as unsigned long.  The other two functions are not
public APIs, so I think they are fine to change.
> == atomic.h => 
> I merged the following revisions, which add new atomic operations for
> both amd64 and i386:
>   - 254610
>   - 254611
>   - 254617
>   - 254620
> 
> The merge was modified to not break the API/ABI. For instance, in the
> original commits, some operations were transformed from a real function
> to a macro using one of the new operation.
> 
> So here's shorter patch with only the changes to atomic.h:
> http://people.freebsd.org/~dumbbell/radeonkms/radeon-stable9-atomic.a.patch
> 
> Therefore, it only adds new code used by the Radeon driver, and I think
> it's safe. Opinions on that?
Looks fine.
> == iicbus => 
> Revision 232365 was merged to add optional pre/post transfer methods to
> iicbb, used by the Radeon driver.
> 
> Again, here's a patch limited to iicbb changes:
> http://people.freebsd.org/~dumbbell/radeonkms/radeon-stable9-iic.a.patch
> 
> I think this merge is safe too, but I'm not confident enough :) Any
> thoughts?
This looks fine.

-- 
John Baldwin