From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerome Glisse Subject: Re: AMD GPU new API for new module Date: Thu, 9 Oct 2014 04:02:57 -0400 Message-ID: <20141009080256.GB4576@gmail.com> References: <20141008160026.GA7643@gmail.com> <54363116.2050900@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-qg0-f44.google.com (mail-qg0-f44.google.com [209.85.192.44]) by gabe.freedesktop.org (Postfix) with ESMTP id E0D2B6E2BA for ; Thu, 9 Oct 2014 01:03:01 -0700 (PDT) Received: by mail-qg0-f44.google.com with SMTP id j5so907726qga.31 for ; Thu, 09 Oct 2014 01:03:01 -0700 (PDT) Content-Disposition: inline In-Reply-To: <54363116.2050900@amd.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Oded Gabbay Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Thu, Oct 09, 2014 at 09:54:14AM +0300, Oded Gabbay wrote: > Hi Jerome, > = > Do you think your proposed change should also be applied to amdkfd's > IOCTLs ? It might make sense it really depends on the lifespan you expect for amdkfd, do you expect that you will need substential API evolution during that timespan or do you foresee that the currently propose API is good enough. The thing i am proposing here is really to address the issue of IOCTL needing fixes because new generation of GPU requires new informations or do not care about old and now meaningless informations. The radeon ioctl sets is a testimony to the fact that we can not predict what kind of broken API we expose and have to leave with. My proposal is really about allowing to evolve each ioctl in what i believe to be the simplest and cleanest way to do it while still preserving backward compatibility ie i expect that old version of an ioctl will continue to live in there own functions untouch and unlove but still working. While new development can move to updated ioctl. Other solution would be to just keep adding new ioctl but i fear we might run out of ioctl number. Cheers, J=E9r=F4me > = > Oded > = > On 08/10/14 19:00, Jerome Glisse wrote: > > Hi, > > = > > So if i do not start the discussion now it might be already too late. G= iven > > plan to converge open source driver and closed source driver to use a s= ingle > > common kernel driver and that this would be a new kernel driver. This i= s an > > opportunity to fix some of the radeon design issues (at least things th= at i > > would have done differently if only i could get some gas for my DeLorea= n). > > = > > Among the thing that i will not do is the chunk stuff associated with cs > > ioctl. I find it ugly, if my memory serve me well i was trying to be fu= ture > > proof and allow the cs ioctl to be extended. While this original aim ha= ve > > been somewhat successfully, i think it was the wrong day to do it. > > = > > My lastest (and what i still believe to be a good idea until proven wro= ng), > > is to change the way we do ioctl and use a little trick. This idea was = also > > spark by the continuous additions we do to info ioctl which is getting = ugly. > > = > > So idea is simple, each ioctl would use some struct like : > > = > > struct radeon_ioctl { > > u32 version; > > u32 size; > > }; > > = > > The version field is the key here, think of it as an index into an arra= y of > > ioctl dispatch functions. So something like : > > = > > struct radeon_ioctls { > > int (*iotcl)[MAX_IOCTL_NUM](void *data, ...); > > }; > > = > > struct radeon_ioctls rdispatch_ioctls[N]; > > = > > And now all ioctl go through this single entry point : > > = > > int radeon_ioctl_stub(int ioctl, void *data, ...) > > { > > struct radeon_ioctl *rio =3D data; > > = > > return rdispatch_ioctls[rio->version][ioctl](data, ...); > > } > > = > > So this is rough idea but the point is that we can do proper ioctl vers= ioning > > and have separate functions for each new versions and avoid ioctl cruft= , or > > at least this is the theory. > > = > > The two things this gave us, is feedback from userspace as we the versi= on the > > kernel will know which version of userspace it is dealing with. The oth= ers one > > is that it does allow you to introduce a completely new API either for = new > > generation of hardware or just as an evolution. And small bonus is that= it > > does allow to slowly phase out API that we consider broken (ioctl per i= octl). > > = > > So this is the main design change that i would do. I should probably ro= ught > > up something that goes deeper for the cs ioctl. > > = > > Cheers, > > J=E9r=F4me > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > =