[Mesa-dev] [PATCH] gallium/sw/kms: Fix multiple imports from PRIME FDs

[Emil Velikov]

On 15 July 2016 at 08:27, Tomasz Figa <tfiga at chromium.org> wrote:
> When a buffer with a GEM handle already existing in our context is
> (re-)imported from a PRIME FD, the resulting GEM handle is exactly the
> same as the original one. Since the GEM handles are not reference
> counted, we need to detect duplicate imports and reference count our
> internal buffer structs ourselves.
>
> Signed-off-by: Tomasz Figa <tfiga at chromium.org>
> ---
>  src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c | 30 +++++++++++++++++------
>  1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c b/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
> index 21ac0d7..c4f56cb 100644
> --- a/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
> +++ b/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
> @@ -211,7 +211,9 @@ kms_sw_displaytarget_map(struct sw_winsys *ws,
>  }
>
>  static struct kms_sw_displaytarget *
> -kms_sw_displaytarget_add_from_prime(struct kms_sw_winsys *kms_sw, int fd)
> +kms_sw_displaytarget_add_from_prime(struct kms_sw_winsys *kms_sw, int fd,
> +                                    unsigned width, unsigned height,
> +                                    unsigned stride)
>  {
>     uint32_t handle = -1;
>     struct kms_sw_displaytarget * kms_sw_dt;
> @@ -222,6 +224,17 @@ kms_sw_displaytarget_add_from_prime(struct kms_sw_winsys *kms_sw, int fd)
>     if (ret)
>        return NULL;
>
> +   LIST_FOR_EACH_ENTRY(kms_sw_dt, &kms_sw->bo_list, link) {
> +      if (kms_sw_dt->handle == handle) {
> +         kms_sw_dt->ref_count++;
> +
> +         DEBUG_PRINT("KMS-DEBUG: imported buffer %u (size %u)\n",
> +                     kms_sw_dt->handle, kms_sw_dt->size);
> +
> +         return kms_sw_dt;
> +      }
> +   }
> +
>     kms_sw_dt = CALLOC_STRUCT(kms_sw_displaytarget);
>     if (!kms_sw_dt)
>        return NULL;
> @@ -229,6 +242,9 @@ kms_sw_displaytarget_add_from_prime(struct kms_sw_winsys *kms_sw, int fd)
>     kms_sw_dt->ref_count = 1;
>     kms_sw_dt->handle = handle;
>     kms_sw_dt->size = lseek(fd, 0, SEEK_END);
> +   kms_sw_dt->width = width;
> +   kms_sw_dt->height = height;
> +   kms_sw_dt->stride = stride;
>
>     if (kms_sw_dt->size == (off_t)-1) {
>        FREE(kms_sw_dt);
> @@ -274,14 +290,12 @@ kms_sw_displaytarget_from_handle(struct sw_winsys *ws,
>
>     switch(whandle->type) {
>     case DRM_API_HANDLE_TYPE_FD:
> -      kms_sw_dt = kms_sw_displaytarget_add_from_prime(kms_sw, whandle->handle);
> -      if (kms_sw_dt) {
> -         kms_sw_dt->ref_count++;
> -         kms_sw_dt->width = templ->width0;
> -         kms_sw_dt->height = templ->height0;
> -         kms_sw_dt->stride = whandle->stride;
> +      kms_sw_dt = kms_sw_displaytarget_add_from_prime(kms_sw, whandle->handle,
> +                                                      templ->width0,
> +                                                      templ->height0,
> +                                                      whandle->stride);
> +      if (kms_sw_dt)
>           *stride = kms_sw_dt->stride;
> -      }
So there's actually a couple of things here:
 - kms_sw_displaytarget_add_from_prime now takes a width, height,
stride. not a bugfix but very nice imho.
 - DRM_API_HANDLE_TYPE_FD were ref_counted twice upon creation - once
in kms_sw_displaytarget_add_from_prime, and a second time immediatelly
after.
 - one should have checked the bo_list prior to importing the BO as prime FD.

Can we split the former (2/2) and get the other two (1/2) with CC:
mesa-stable... ?
Ideally we would have just a single
"LIST_FOR_EACH_ENTRY(...&kms_sw->bo_list...)" block in
kms_sw_displaytarget_from_handle and none in
kms_sw_displaytarget_add_from_prime.

Thanks
Emil