Re: AMD GPU new API for new module

[Jerome Glisse <j.glisse@gmail.com>]

On Thu, Oct 09, 2014 at 03:32:26AM -0400, Rob Clark wrote:
> On Wed, Oct 8, 2014 at 12:00 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
> >
> > So idea is simple, each ioctl would use some struct like :
> >
> > struct radeon_ioctl {
> >         u32     version;
> >         u32     size;
> > };
> 
> 
> fwiw, drm_ioctl() will do the right thing (zero-pad) for growing
> ioctls these days..

It's more about userspace knowing about ioctl XY and having a way
to override/ask for special version of each ioctl. So if we screw
ioctl XY, we can add a new version to XY and we can try to work
around if userspace still request the old version. It is mostly
about trying to keep the code clean and avoiding things like the
chunk stuff of the cs ioctl.

For instance if you cs ioctl with the following struct as ioctl
parameter:

struct drm_radeon_cs_ioctl_version_00 {
   u64  *cs;
   u32  size;
   u32  flags;
};

And now you have a new generation of hardware or are just adding
a new feature :

struct drm_radeon_cs_ioctl_version_01 {
   u64  *cs;
   u32  size;
   u32  flags;
   u32  newfeature fields;
};

Of course you can argue that you could use the size of user space
paremeter to do this simple example. But with versioning you can
also move fields around, remove fields, ... basicly it is just more
flexible with small overhead of having one more indirection but this
will be lost into the ioctl cost anyway (i highly doubt it would
turns as a bottleneck).

Cheers,
Jérôme

> 
> BR,
> -R