[PATCH 5/6] drm/amdgpu: add timeline support in amdgpu CS

[Nicolai Hähnle]

Hey Chunming,

On 20.09.2018 13:03, Chunming Zhou wrote:
> @@ -1113,48 +1117,91 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,
>   }
>   
>   static int amdgpu_cs_process_syncobj_in_dep(struct amdgpu_cs_parser *p,
> -					    struct amdgpu_cs_chunk *chunk)
> +					    struct amdgpu_cs_chunk *chunk,
> +					    bool timeline)
>   {
>   	unsigned num_deps;
>   	int i, r;
> -	struct drm_amdgpu_cs_chunk_sem *deps;
>   
> -	deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata;
> -	num_deps = chunk->length_dw * 4 /
> -		sizeof(struct drm_amdgpu_cs_chunk_sem);
> +	if (!timeline) {
> +		struct drm_amdgpu_cs_chunk_sem *deps;
>   
> -	for (i = 0; i < num_deps; ++i) {
> -		r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle);
> +		deps = (struct drm_amdgpu_cs_chunk_sem *)chunk->kdata;
> +		num_deps = chunk->length_dw * 4 /
> +			sizeof(struct drm_amdgpu_cs_chunk_sem);
> +		for (i = 0; i < num_deps; ++i) {
> +			r = amdgpu_syncobj_lookup_and_add_to_sync(p, deps[i].handle,
> +								  0, 0);
>   		if (r)
>   			return r;

The indentation looks wrong.


> +		}
> +	} else {
> +		struct drm_amdgpu_cs_chunk_syncobj *syncobj_deps;
> +
> +		syncobj_deps = (struct drm_amdgpu_cs_chunk_syncobj *)chunk->kdata;
> +		num_deps = chunk->length_dw * 4 /
> +			sizeof(struct drm_amdgpu_cs_chunk_syncobj);
> +		for (i = 0; i < num_deps; ++i) {
> +			r = amdgpu_syncobj_lookup_and_add_to_sync(p, syncobj_deps[i].handle,
> +								  syncobj_deps[i].point,
> +								  syncobj_deps[i].flags);
> +		if (r)
> +			return r;

Here as well.

So I'm wondering a bit about this uapi. Specifically, what happens if 
you try to use timeline syncobjs here as dependencies _without_ 
DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT?

My understanding is, it'll just return -EINVAL without any indication as 
to which syncobj actually failed. What's the caller supposed to do then?

Cheers,
Nicolai
-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.