[PATCH 3/4] drm/radeon: add DONT_SYNC flag to CS relocs

Jerome Glisse j.glisse at gmail.com
Thu Aug 14 14:06:27 PDT 2014


On Thu, Aug 14, 2014 at 04:34:29PM -0400, Alex Deucher wrote:
> On Thu, Aug 14, 2014 at 2:43 PM, Jerome Glisse <j.glisse at gmail.com> wrote:
> > On Thu, Aug 14, 2014 at 06:12:04PM +0200, Christian König wrote:
> >> From: Christian König <christian.koenig at amd.com>
> >>
> >> This allows userspace to explicitly don't sync submission to
> >> different rings as long as all uses stay in the same client.
> >>
> >> Signed-off-by: Christian König <christian.koenig at amd.com>
> >> ---
> >>  drivers/gpu/drm/radeon/radeon.h     |  3 +++
> >>  drivers/gpu/drm/radeon/radeon_cs.c  | 23 ++++++++++++++++++++++-
> >>  drivers/gpu/drm/radeon/radeon_gem.c |  1 +
> >>  drivers/gpu/drm/radeon/radeon_ttm.c |  3 +++
> >>  include/uapi/drm/radeon_drm.h       |  2 ++
> >>  5 files changed, 31 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> >> index c0f7773..740a0b2 100644
> >> --- a/drivers/gpu/drm/radeon/radeon.h
> >> +++ b/drivers/gpu/drm/radeon/radeon.h
> >> @@ -478,6 +478,8 @@ struct radeon_bo {
> >>       u32                             tiling_flags;
> >>       u32                             pitch;
> >>       int                             surface_reg;
> >> +     struct drm_file                 *last_user;
> >> +     struct radeon_fence             *last_sync;
> >>       struct radeon_fence             *written;
> >>       /* list of all virtual address to which this bo
> >>        * is associated to
> >> @@ -1018,6 +1020,7 @@ struct radeon_cs_reloc {
> >>       unsigned                        allowed_domains;
> >>       uint32_t                        tiling_flags;
> >>       uint32_t                        handle;
> >> +     uint32_t                        flags;
> >>       bool                            written;
> >>  };
> >>
> >> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
> >> index 3aa7e48..11e4789 100644
> >> --- a/drivers/gpu/drm/radeon/radeon_cs.c
> >> +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> >> @@ -166,6 +166,7 @@ static int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
> >>
> >>               p->relocs[i].tv.bo = &p->relocs[i].robj->tbo;
> >>               p->relocs[i].handle = r->handle;
> >> +             p->relocs[i].flags = r->flags;
> >>               p->relocs[i].written = !!r->write_domain;
> >>
> >>               radeon_cs_buckets_add(&buckets, &p->relocs[i].tv.head,
> >> @@ -236,6 +237,14 @@ static void radeon_cs_sync_rings(struct radeon_cs_parser *p)
> >>               if (!bo)
> >>                       continue;
> >>
> >> +             /* always sync to the last operation
> >> +                the clients doesn't know about */
> >> +             radeon_semaphore_sync_to(p->ib.presync, bo->last_sync);
> >> +
> >> +             if (bo->last_user == p->filp &&
> >> +                 reloc->flags & RADEON_RELOC_DONT_SYNC)
> >> +                     continue;
> >> +
> >>               fence = bo->tbo.sync_obj;
> >>
> >>               if (bo->written && radeon_fence_signaled(bo->written))
> >> @@ -421,7 +430,19 @@ static void radeon_cs_parser_fini(struct radeon_cs_parser *parser, int error, bo
> >>                       struct radeon_cs_reloc *reloc = &parser->relocs[i];
> >>                       struct radeon_bo *bo = reloc->robj;
> >>
> >> -                     if (!bo || !reloc->written)
> >> +                     if (!bo)
> >> +                             continue;
> >> +
> >> +                     /* if the client changes remember that and always serialize
> >> +                        all operations from different clients */
> >> +                     if (bo->last_user != parser->filp && bo->tbo.sync_obj) {
> >> +                             struct radeon_fence *fence = bo->tbo.sync_obj;
> >> +                             radeon_fence_unref(&bo->last_sync);
> >> +                             bo->last_sync = radeon_fence_ref(fence);
> >> +                     }
> >> +                     bo->last_user = parser->filp;
> >> +
> >> +                     if (!reloc->written)
> >>                               continue;
> >>
> >>                       radeon_fence_unref(&bo->written);
> >> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> >> index bfd7e1b..c73dbc1 100644
> >> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> >> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> >> @@ -259,6 +259,7 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data,
> >>               r = radeon_gem_handle_lockup(rdev, r);
> >>               return r;
> >>       }
> >> +     gem_to_radeon_bo(gobj)->last_user = filp;
> >>       r = drm_gem_handle_create(filp, gobj, &handle);
> >>       /* drop reference from allocate - handle holds it now */
> >>       drm_gem_object_unreference_unlocked(gobj);
> >> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> >> index 76be612..a4f964f 100644
> >> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> >> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> >> @@ -273,6 +273,9 @@ static int radeon_move_blit(struct ttm_buffer_object *bo,
> >>                       &fence);
> >>
> >>       if (!r) {
> >> +             rbo->last_user = NULL;
> >> +             radeon_fence_unref(&rbo->last_sync);
> >> +             rbo->last_sync = radeon_fence_ref(fence);
> >>               radeon_fence_unref(&rbo->written);
> >>               rbo->written = radeon_fence_ref(fence);
> >>       }
> >> diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h
> >> index 509b2d7..5bd3f68 100644
> >> --- a/include/uapi/drm/radeon_drm.h
> >> +++ b/include/uapi/drm/radeon_drm.h
> >> @@ -944,6 +944,8 @@ struct drm_radeon_cs_chunk {
> >>  };
> >>
> >>  /* drm_radeon_cs_reloc.flags */
> >> +#define RADEON_RELOC_PRIO_MASK               (0xf << 0)
> >
> > RADEON_RELOC_PRIO_MASK not use anywhere, not explain by anything, so new API
> > with no justification, i would say NAK
> 
> The first 4 bits are used in:
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c9b76548899cde2e729e3bca015d7e78ec5baad7
> and that interface is already used in mesa.
> 
> This just makes it clear that those bits are already used when adding
> additional flags.

Well would be good to introduce the flag and use it as part of
separate patch.

> 
> Alex
> 
> >
> >> +#define RADEON_RELOC_DONT_SYNC               (1 << 4)
> >>
> >>  struct drm_radeon_cs_reloc {
> >>       uint32_t                handle;
> >> --
> >> 1.9.1
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel


More information about the dri-devel mailing list