Re: [PATCH libdrm] libdrm/amdgpu: add interface for kernel semaphores

[Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Hi Dave,

Barring the other discussions, allow me to put a couple of trivial suggestions:

Please re-wrap the long lines to follow existing code style.

On 14 March 2017 at 00:50, Dave Airlie <airlied@gmail.com> wrote:

> @@ -882,6 +894,12 @@ int amdgpu_cs_submit(amdgpu_context_handle context,
>                      struct amdgpu_cs_request *ibs_request,
>                      uint32_t number_of_requests);
>
> +int amdgpu_cs_submit_sem(amdgpu_context_handle context,
> +                        uint64_t flags,
> +                        struct amdgpu_cs_request *ibs_request,
> +                        struct amdgpu_cs_request_sem *ibs_sem,
> +                        uint32_t number_of_requests);
> +
>  /**
>   *  Query status of Command Buffer Submission
>   *
> @@ -1255,4 +1273,14 @@ int amdgpu_cs_destroy_semaphore(amdgpu_semaphore_handle sem);
>  */
>  const char *amdgpu_get_marketing_name(amdgpu_device_handle dev);
>
> +int amdgpu_cs_create_sem(amdgpu_device_handle dev,
> +                        amdgpu_sem_handle *sem);
> +int amdgpu_cs_export_sem(amdgpu_device_handle dev,
> +                         amdgpu_sem_handle sem,
> +                        int *shared_handle);
> +int amdgpu_cs_import_sem(amdgpu_device_handle dev,
> +                         int shared_handle,
> +                        amdgpu_sem_handle *sem);
> +int amdgpu_cs_destroy_sem(amdgpu_device_handle dev,
> +                         amdgpu_sem_handle sem);
The new symbols should be added to the amdgpu-symbol-check test.
If in doubt - run `make -C amdgpu check'

> --- a/include/drm/amdgpu_drm.h
> +++ b/include/drm/amdgpu_drm.h
Please sync this as PATCH 1/2 via "make headers_install" + cp + git
commit -asm "....Generated using make headers_install.\nGenerated from
$tree/branch commit $sha."

There's a handful of other changes that are missing/should be merged.

> @@ -50,6 +50,7 @@ extern "C" {

> +struct drm_amdgpu_cs_chunk_sem {
> +       uint32_t handle;
> +};
> +
Seems unused in the UAPI header - might what to add a note ?
Also sizeof(struct drm_amdgpu_cs_chunk_sem) is not multiple of 64bit -
worth mentioning that it's safe and/or why ?

Thanks
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx