diff mbox

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

Message ID 1408032725-6236-4-git-send-email-deathsimple@vodafone.de (mailing list archive)
State New, archived
Headers show

Commit Message

Christian König Aug. 14, 2014, 4:12 p.m. UTC
From: Christian König <christian.koenig@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@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(-)

Comments

Jerome Glisse Aug. 14, 2014, 6:43 p.m. UTC | #1
On Thu, Aug 14, 2014 at 06:12:04PM +0200, Christian König wrote:
> From: Christian König <christian.koenig@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@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

> +#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
Alex Deucher Aug. 14, 2014, 8:34 p.m. UTC | #2
On Thu, Aug 14, 2014 at 2:43 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
> On Thu, Aug 14, 2014 at 06:12:04PM +0200, Christian König wrote:
>> From: Christian König <christian.koenig@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@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.

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
Jerome Glisse Aug. 14, 2014, 9:06 p.m. UTC | #3
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@gmail.com> wrote:
> > On Thu, Aug 14, 2014 at 06:12:04PM +0200, Christian König wrote:
> >> From: Christian König <christian.koenig@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@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@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
Christian König Aug. 15, 2014, 7:46 a.m. UTC | #4
Am 14.08.2014 um 23:06 schrieb Jerome Glisse:
> 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@gmail.com> wrote:
>>> On Thu, Aug 14, 2014 at 06:12:04PM +0200, Christian König wrote:
>>>> From: Christian König <christian.koenig@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@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.

Which is what I actually wanted to do, but forgotten about it because 
all of the fence discussion.

Going to send out a separate cleanup patch soon,
Christian.

>
>> 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
diff mbox

Patch

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)
+#define RADEON_RELOC_DONT_SYNC		(1 << 4)
 
 struct drm_radeon_cs_reloc {
 	uint32_t		handle;