<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Am 29.01.19 um 14:32 schrieb Marek Olšák:<br>
</div>
<blockquote type="cite" cite="mid:CAAxE2A6F+Wexxe_fY22P7iWk1DwKbP6DOmxyoUV164YQFTvdaA@mail.gmail.com">
<div dir="auto">
<div><br>
<br>
<div class="gmail_quote">
<div dir="ltr">On Tue, Jan 29, 2019, 3:01 AM Christian König <<a href="mailto:ckoenig.leichtzumerken@gmail.com" moz-do-not-send="true">ckoenig.leichtzumerken@gmail.com</a> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">
Am 28.01.19 um 22:52 schrieb Marek Olšák:<br>
> From: Marek Olšák <<a href="mailto:marek.olsak@amd.com" target="_blank" rel="noreferrer" moz-do-not-send="true">marek.olsak@amd.com</a>><br>
><br>
> Normal syncobjs signal when an IB finishes. Start syncobjs signal when<br>
> an IB starts.<br>
<br>
That approach has quite a number of problems (for example you can't <br>
allocate memory at this point).<br>
<br>
Better add a flag that we should only sync on scheduling for a <br>
dependency/syncobj instead.<br>
</blockquote>
</div>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">I don't understand. Can you give me an example of the interface and how the implementation would look?</div>
</div>
</blockquote>
<br>
For example we add a new chunk type AMDGPU_CHUNK_ID_SCHEDULED which is handled the same way as AMDGPU_CHUNK_ID_DEPENDENCIES.<br>
<br>
Then in amdgpu_cs_process_fence_dep() we check if its a AMDGPU_CHUNK_ID_SCHEDULED and if yes extract the scheduled fence from the fence we got from amdgpu_ctx_get_fence().<br>
<br>
Christian.<br>
<br>
<blockquote type="cite" cite="mid:CAAxE2A6F+Wexxe_fY22P7iWk1DwKbP6DOmxyoUV164YQFTvdaA@mail.gmail.com">
<div dir="auto">
<div dir="auto"><br>
</div>
<div dir="auto">Thanks,</div>
<div dir="auto">Marek</div>
<div dir="auto"><br>
</div>
<div dir="auto">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Christian.<br>
<br>
><br>
> Signed-off-by: Marek Olšák <<a href="mailto:marek.olsak@amd.com" target="_blank" rel="noreferrer" moz-do-not-send="true">marek.olsak@amd.com</a>><br>
> ---<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 18 ++++++++++++++++++<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 ++-<br>
>   include/uapi/drm/amdgpu_drm.h           | 13 ++++++++++++-<br>
>   4 files changed, 33 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h<br>
> index d67f8b1dfe80..8e2f7e558bc9 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h<br>
> @@ -453,20 +453,21 @@ struct amdgpu_cs_parser {<br>
>       struct dma_fence                *fence;<br>
>       uint64_t                        bytes_moved_threshold;<br>
>       uint64_t                        bytes_moved_vis_threshold;<br>
>       uint64_t                        bytes_moved;<br>
>       uint64_t                        bytes_moved_vis;<br>
>       struct amdgpu_bo_list_entry     *evictable;<br>
>   <br>
>       /* user fence */<br>
>       struct amdgpu_bo_list_entry     uf_entry;<br>
>   <br>
> +     bool                            get_start_syncobj;<br>
>       unsigned num_post_dep_syncobjs;<br>
>       struct drm_syncobj **post_dep_syncobjs;<br>
>   };<br>
>   <br>
>   static inline u32 amdgpu_get_ib_value(struct amdgpu_cs_parser *p,<br>
>                                     uint32_t ib_idx, int idx)<br>
>   {<br>
>       return p->job->ibs[ib_idx].ptr[idx];<br>
>   }<br>
>   <br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c<br>
> index 1c49b8266d69..917f3818c61c 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c<br>
> @@ -1022,20 +1022,23 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,<br>
>               r = amdgpu_ctx_get_entity(parser->ctx, chunk_ib->ip_type,<br>
>                                         chunk_ib->ip_instance, chunk_ib->ring,<br>
>                                         &entity);<br>
>               if (r)<br>
>                       return r;<br>
>   <br>
>               if (chunk_ib->flags & AMDGPU_IB_FLAG_PREAMBLE)<br>
>                       parser->job->preamble_status |=<br>
>                               AMDGPU_PREAMBLE_IB_PRESENT;<br>
>   <br>
> +             if (chunk_ib->flags & AMDGPU_IB_FLAG_GET_START_SYNCOBJ)<br>
> +                     parser->get_start_syncobj = true;<br>
> +<br>
>               if (parser->entity && parser->entity != entity)<br>
>                       return -EINVAL;<br>
>   <br>
>               parser->entity = entity;<br>
>   <br>
>               ring = to_amdgpu_ring(entity->rq->sched);<br>
>               r =  amdgpu_ib_get(adev, vm, ring->funcs->parse_cs ?<br>
>                                  chunk_ib->ib_bytes : 0, ib);<br>
>               if (r) {<br>
>                       DRM_ERROR("Failed to get ib !\n");<br>
> @@ -1227,20 +1230,35 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,<br>
>       amdgpu_mn_lock(p->mn);<br>
>       amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {<br>
>               struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e-><a href="http://tv.bo" rel="noreferrer noreferrer" target="_blank" moz-do-not-send="true">tv.bo</a>);<br>
>   <br>
>               if (amdgpu_ttm_tt_userptr_needs_pages(bo->tbo.ttm)) {<br>
>                       r = -ERESTARTSYS;<br>
>                       goto error_abort;<br>
>               }<br>
>       }<br>
>   <br>
> +     if (p->get_start_syncobj) {<br>
> +             struct drm_syncobj *syncobj;<br>
> +<br>
> +             r = drm_syncobj_create(&syncobj, 0,<br>
> +                                    &job->base.s_fence->scheduled);<br>
> +             if (r)<br>
> +                     goto error_abort;<br>
> +<br>
> +             r = drm_syncobj_get_handle(p->filp, syncobj,<br>
> +                                        &cs->out.start_syncobj);<br>
> +             if (r)<br>
> +                     goto error_abort;<br>
> +             drm_syncobj_put(syncobj);<br>
> +     }<br>
> +<br>
>       job->owner = p->filp;<br>
>       p->fence = dma_fence_get(&job->base.s_fence->finished);<br>
>   <br>
>       amdgpu_ctx_add_fence(p->ctx, entity, p->fence, &seq);<br>
>       amdgpu_cs_post_dependencies(p);<br>
>   <br>
>       if ((job->preamble_status & AMDGPU_PREAMBLE_IB_PRESENT) &&<br>
>           !p->ctx->preamble_presented) {<br>
>               job->preamble_status |= AMDGPU_PREAMBLE_IB_PRESENT_FIRST;<br>
>               p->ctx->preamble_presented = true;<br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c<br>
> index c806f984bcc5..a230a30722d4 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c<br>
> @@ -64,23 +64,24 @@<br>
>    * - 3.18.0 - Export gpu always on cu bitmap<br>
>    * - 3.19.0 - Add support for UVD MJPEG decode<br>
>    * - 3.20.0 - Add support for local BOs<br>
>    * - 3.21.0 - Add DRM_AMDGPU_FENCE_TO_HANDLE ioctl<br>
>    * - 3.22.0 - Add DRM_AMDGPU_SCHED ioctl<br>
>    * - 3.23.0 - Add query for VRAM lost counter<br>
>    * - 3.24.0 - Add high priority compute support for gfx9<br>
>    * - 3.25.0 - Add support for sensor query info (stable pstate sclk/mclk).<br>
>    * - 3.26.0 - GFX9: Process AMDGPU_IB_FLAG_TC_WB_NOT_INVALIDATE.<br>
>    * - 3.27.0 - Add new chunk to to AMDGPU_CS to enable BO_LIST creation.<br>
> + * - 3.28.0 - AMDGPU_IB_FLAG_GET_START_SYNCOBJ<br>
>    */<br>
>   #define KMS_DRIVER_MAJOR    3<br>
> -#define KMS_DRIVER_MINOR     27<br>
> +#define KMS_DRIVER_MINOR     28<br>
>   #define KMS_DRIVER_PATCHLEVEL       0<br>
>   <br>
>   int amdgpu_vram_limit = 0;<br>
>   int amdgpu_vis_vram_limit = 0;<br>
>   int amdgpu_gart_size = -1; /* auto */<br>
>   int amdgpu_gtt_size = -1; /* auto */<br>
>   int amdgpu_moverate = -1; /* auto */<br>
>   int amdgpu_benchmarking = 0;<br>
>   int amdgpu_testing = 0;<br>
>   int amdgpu_audio = -1;<br>
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h<br>
> index 662d379ea624..d0e0c99cea32 100644<br>
> --- a/include/uapi/drm/amdgpu_drm.h<br>
> +++ b/include/uapi/drm/amdgpu_drm.h<br>
> @@ -538,21 +538,23 @@ struct drm_amdgpu_cs_in {<br>
>       __u32           ctx_id;<br>
>       /**  Handle of resource list associated with CS */<br>
>       __u32           bo_list_handle;<br>
>       __u32           num_chunks;<br>
>       __u32           _pad;<br>
>       /** this points to __u64 * which point to cs chunks */<br>
>       __u64           chunks;<br>
>   };<br>
>   <br>
>   struct drm_amdgpu_cs_out {<br>
> -     __u64 handle;<br>
> +     __u64 handle; /* sequence number */<br>
> +     __u32 start_syncobj; /* signalled when IB execution begins */<br>
> +     __u32 _pad;<br>
>   };<br>
>   <br>
>   union drm_amdgpu_cs {<br>
>       struct drm_amdgpu_cs_in in;<br>
>       struct drm_amdgpu_cs_out out;<br>
>   };<br>
>   <br>
>   /* Specify flags to be used for IB */<br>
>   <br>
>   /* This IB should be submitted to CE */<br>
> @@ -566,20 +568,29 @@ union drm_amdgpu_cs {<br>
>   <br>
>   /* The IB fence should do the L2 writeback but not invalidate any shader<br>
>    * caches (L2/vL1/sL1/I$). */<br>
>   #define AMDGPU_IB_FLAG_TC_WB_NOT_INVALIDATE (1 << 3)<br>
>   <br>
>   /* Set GDS_COMPUTE_MAX_WAVE_ID = DEFAULT before PACKET3_INDIRECT_BUFFER.<br>
>    * This will reset wave ID counters for the IB.<br>
>    */<br>
>   #define AMDGPU_IB_FLAG_RESET_GDS_MAX_WAVE_ID (1 << 4)<br>
>   <br>
> +/* The CS ioctl will return a syncobj representing when all IBs begin<br>
> + * execution. If set, this applies to all IBs. The returned syncobj can be<br>
> + * used as an IB dependency for other IBs.<br>
> + *<br>
> + * This is used for GPU deadlock prevention when userspace uses mid-IB fences<br>
> + * to wait for mid-IB work on other rings.<br>
> + */<br>
> +#define AMDGPU_IB_FLAG_GET_START_SYNCOBJ (1 << 5)<br>
> +<br>
>   struct drm_amdgpu_cs_chunk_ib {<br>
>       __u32 _pad;<br>
>       /** AMDGPU_IB_FLAG_* */<br>
>       __u32 flags;<br>
>       /** Virtual address to begin IB execution */<br>
>       __u64 va_start;<br>
>       /** Size of submission */<br>
>       __u32 ib_bytes;<br>
>       /** HW IP to submit to */<br>
>       __u32 ip_type;<br>
<br>
</blockquote>
</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>