Hi Gustavo,
thank you for the review.
On Wed, 2017-03-08 at 11:37 -0300, Gustavo Padovan wrote:
[...]
> > @@ -385,6 +396,25 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev,
> > void *data,
> > goto err_submit_objects;
> > }
> >
> > + if (args->flags & ETNA_SUBMIT_FENCE_FD_IN) {
> > + in_fence = sync_file_get_fence(args->fence_fd);
> > + if (!in_fence) {
> > + ret = -EINVAL;
> > + goto err_submit_objects;
> > + }
> > +
> > + /* TODO if we get an array-fence due to userspace merging
> > + * multiple fences, we need a way to determine if all the
> > + * backing fences are from our own context..
> > + */
>
> All GPU drivers seem to have the same need on fence_array, so I think we
> can create a fence array helper to inspect if it has a specific context
> or not.
Do you have a pointer where I could read up on how this should be done?
> > +
> > + if (in_fence->context != gpu->fence_context) {
> > + ret = dma_fence_wait(in_fence, true);
> > + if (ret)
> > + goto err_submit_objects;
>
> sync_file_get_fence() hold a fence ref, we need to release it here
> always and not only in case of error.
We do that already. err_submit_objects is an unfortunate name for patch
review, but the out label at the end of the function falls right through
to it.
> > + }
> > + }
> > +
> > ret = submit_fence_sync(submit);
> > if (ret)
> > goto err_submit_objects;
> > @@ -419,6 +449,8 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev,
> > void *data,
> > flush_workqueue(priv->wq);
> >
> > err_submit_objects:
> > + if (in_fence)
> > + dma_fence_put(in_fence);
We pass through here in the non-error case, too.
[...]
> > @@ -154,6 +154,10 @@ struct drm_etnaviv_gem_submit_bo {
> > * one or more cmdstream buffers. This allows for conditional execution
> > * (context-restore), and IB buffers needed for per tile/bin draw cmds.
> > */
> > +#define ETNA_SUBMIT_NO_IMPLICIT 0x0001
> > +#define ETNA_SUBMIT_FENCE_FD_IN 0x0002
>
> ETNA_SUBMIT_NO_IMPLICIT and ETNA_SUBMIT_FENCE_FD_IN are the same, when
> you send and fence_fd to the kernel you are requesting on explicit sync
> thus I think the ETNA_SUBMIT_NO_IMPLICIT can be dropped and
> ETNA_SUBMIT_FENCE_FD_IN would be the one to look at.
I just followed Rob's lead. I'm not sure if there may be a reason to
submit an in fence still keep implicit fencing enabled at the same time,
or to allow a submit without any fencing whatsoever. Maybe for testing
purposes?
I'm happy to drop the NO_IMPLICIT flag if there is no need for it.
regards
Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
