From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerome Glisse Subject: Re: [PATCH 3/4] drm/radeon: add DONT_SYNC flag to CS relocs Date: Thu, 14 Aug 2014 17:06:27 -0400 Message-ID: <20140814210626.GJ2000@gmail.com> References: <1408032725-6236-1-git-send-email-deathsimple@vodafone.de> <1408032725-6236-4-git-send-email-deathsimple@vodafone.de> <20140814184333.GG2000@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-qg0-f51.google.com (mail-qg0-f51.google.com [209.85.192.51]) by gabe.freedesktop.org (Postfix) with ESMTP id 8460F6E752 for ; Thu, 14 Aug 2014 14:06:31 -0700 (PDT) Received: by mail-qg0-f51.google.com with SMTP id a108so1513232qge.38 for ; Thu, 14 Aug 2014 14:06:30 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Alex Deucher Cc: Maling list - DRI developers List-Id: dri-devel@lists.freedesktop.org On Thu, Aug 14, 2014 at 04:34:29PM -0400, Alex Deucher wrote: > On Thu, Aug 14, 2014 at 2:43 PM, Jerome Glisse wrote: > > On Thu, Aug 14, 2014 at 06:12:04PM +0200, Christian K=F6nig wrote: > >> From: Christian K=F6nig > >> > >> 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=F6nig > >> --- > >> 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/rade= on/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_c= s_parser *p) > >> > >> p->relocs[i].tv.bo =3D &p->relocs[i].robj->tbo; > >> p->relocs[i].handle =3D r->handle; > >> + p->relocs[i].flags =3D r->flags; > >> p->relocs[i].written =3D !!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 =3D=3D p->filp && > >> + reloc->flags & RADEON_RELOC_DONT_SYNC) > >> + continue; > >> + > >> fence =3D bo->tbo.sync_obj; > >> > >> if (bo->written && radeon_fence_signaled(bo->written)) > >> @@ -421,7 +430,19 @@ static void radeon_cs_parser_fini(struct radeon_c= s_parser *parser, int error, bo > >> struct radeon_cs_reloc *reloc =3D &parser->reloc= s[i]; > >> struct radeon_bo *bo =3D reloc->robj; > >> > >> - if (!bo || !reloc->written) > >> + if (!bo) > >> + continue; > >> + > >> + /* if the client changes remember that and alway= s serialize > >> + all operations from different clients */ > >> + if (bo->last_user !=3D parser->filp && bo->tbo.s= ync_obj) { > >> + struct radeon_fence *fence =3D bo->tbo.s= ync_obj; > >> + radeon_fence_unref(&bo->last_sync); > >> + bo->last_sync =3D radeon_fence_ref(fence= ); > >> + } > >> + bo->last_user =3D 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/rad= eon/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 =3D radeon_gem_handle_lockup(rdev, r); > >> return r; > >> } > >> + gem_to_radeon_bo(gobj)->last_user =3D filp; > >> r =3D 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/rad= eon/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_obje= ct *bo, > >> &fence); > >> > >> if (!r) { > >> + rbo->last_user =3D NULL; > >> + radeon_fence_unref(&rbo->last_sync); > >> + rbo->last_sync =3D radeon_fence_ref(fence); > >> radeon_fence_unref(&rbo->written); > >> rbo->written =3D radeon_fence_ref(fence); > >> } > >> diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_d= rm.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 ne= w 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= =3Dc9b76548899cde2e729e3bca015d7e78ec5baad7 > 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@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel