From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 5/8] drm/i915/context: context validation for execbuffer2 Date: Mon, 14 Feb 2011 15:10:49 -0800 Message-ID: <20110214231049.GA25157@snipes.kumite> References: <1296687620-27019-1-git-send-email-bwidawsk@gmail.com> <1296687620-27019-6-git-send-email-bwidawsk@gmail.com> <20110214202122.GA3569@viiv.ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-yx0-f177.google.com (mail-yx0-f177.google.com [209.85.213.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 67C8A9E74C for ; Mon, 14 Feb 2011 15:09:25 -0800 (PST) Received: by yxd30 with SMTP id 30so2261785yxd.36 for ; Mon, 14 Feb 2011 15:09:24 -0800 (PST) Content-Disposition: inline In-Reply-To: <20110214202122.GA3569@viiv.ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Feb 14, 2011 at 09:21:22PM +0100, Daniel Vetter wrote: > On Wed, Feb 02, 2011 at 03:00:17PM -0800, Ben Widawsky wrote: > > Adds the support for actually doing something with buffer validation for > > the context when submitted via execbuffer2. When a set of context flags > > are submitted in the execbuffer call, the code is able to handle the > > commands. It will also go through the existing buffers associated with > > the context and make sure they are still present. The big thing missing > > is if the client wants to change the domains of buffers which have > > already been associated. In this case, the client must disassociate and > > then re-associate with the proper domains. > > Hm, this remark got me thinking (because currently with read/write domains > per reloc domain changes on the fly are super-simple): Why not drop the > whole associate/disassociate idea and require userspace to always submit a > full list of bos still used by this context (and their required > offsets)? > The code has changed quite a bit since these patches: http://cgit.freedesktop.org/~bwidawsk/drm-intel/ I have no problem with this. The complex part of the code has become tracking when I can retire the context's backing object. As I see it, your proposal is essentially an implicit associate (if it has an offset), with no disassociate. In terms of the driver code to handle it, it would just read through the flags instead of slots. I had, and still have very little clue as to what clients of the interface would like. I'm very scared of digging into mesa. A fear which I am trying to get over presently. So if people think this is at least as good, or better - then great! I think what I'll do is fix the couple of outstanding bugs I'm aware of, get a couple of better unit tests, do some kind of code review on the result of that, start tinkering with mesa - and then decide which one seems better. Unless others chime in and say they like one over the other... Your argument is pretty convincing. It seems the only advantage of slots is you can save on the number of flags submitted per execbuffer call, which is probably negligible since multiple people have said there are relatively few buffers which we actually need to be tied to a context anyway. > expecting to get this right on the first try is probably wishful > thinking. This is my opinion as well, but I'm not sure everyone agrees. > > Yours, Daniel Thanks Daniel. Ben