Emil,
> -----Original Message-----
> From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
> Sent: Thursday, August 3, 2017 4:06 PM
> To: Marathe, Yogesh <yogesh.mara...@intel.com>
> Cc: ML mesa-dev <mesa-dev@lists.freedesktop.org>; Wu, Zhongmin
> <zhongmin...@intel.com>
> Subject: Re: [Mesa-dev] [PATCH v5 2/2] i965: Queue the buffer with a sync
> fence
> for Android OS
>
> On 2 August 2017 at 20:01, <yogesh.mara...@intel.com> wrote:
> > From: Zhongmin Wu <zhongmin...@intel.com>
> >
> > Before we queued the buffer with a invalid fence (-1), it causes
> > benchmarks such as flatland to fail. This patch enables explicit sync
> > feature on android.
> >
> > Now we get the out fence during the flushing buffer and then pass it
> > to SurfaceFlinger in eglSwapbuffer function.
> >
> > v2: a) Also implement the fence in cancelBuffer.
> > b) The last sync fence is stored in drawable object
> > rather than brw context.
> > c) format clear.
> >
> > v3: a) Save the last fence fd in DRI Context object.
> > b) Return the last fence if the batch buffer is empty and
> > nothing to be flushed when _intel_batchbuffer_flush_fence
> > c) Add the new interface in vbtl to set the retrieve fence
> >
> > v3.1 a) close fd in the new vbtl interface on none Android platform
> >
> > v4: a) The last fence is saved in brw context.
> > b) The retrieve fd is for all the platform but not just Android
> > c) Add a uniform dri2 interface to initialize the surface.
> >
> > v4.1: a) make some changes of variable name.
> > b) the patch is broken into two patches.
> >
> > v4.2: a) Add a deinit interface for surface to clear the out fence
> >
> > v5: a) Add enable_out_fence to init, platform sets it true or
> > false
> > b) Change get fd to update fd and check for fence
> > c) Commit description updated
> >
> Seems like most suggestions in last version did not get addressed.
> Please comment if you disagree (it can be that we've misread/misremember
> something) before posting another version.
>
> To reiterate:
> - add blank line between variable declarations and code
> - use more consistent function names
> - comment above queueBuffer needs fixing
> - version check (2+) the fence extension, calling .create_fence_fd() only
> when
> .get_capabilities() advertises __DRI_FENCE_CAP_NATIVE_FD
> - don't introduce unused variables (in make_current)
> - the create fd for the old display surface (in make_current) seems bogus
>
I think the patch has gone through headline changes, so it's bit difficult to
track back. I tried
to address Rafael's comments (latest before v5) assuming others were taken care
of, after
4 versions. Doesn’t seem to be the case. No problem, thanks for re-iterating.
I'll work on
fixing these and discuss as required.
> Also, please change the commit summary message to reflect reality.
> I'm thinking of the following, although you can come with something better.
>
> "egl: allow creation of per surface out fence
>
> Add plumbing to allow creation of per display surface out fence.
>
> Currently enabled only on Android, since the system expects a valid fd in
> ANativeWindow::{queue,cancel}Buffer.
> Without it tools such as flatland will fail, since they cannot get the
> timestamp as
> we currently pass an fd of -1."
>
Ok. This sounds good to me, will also check commit message while pushing next
version.
> Thanks
> Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
