[PATCH 03/13] drm/amdgpu/UAPI: add new CS chunk for GFX shadow buffers

[Alex Deucher]

On Thu, Apr 13, 2023 at 11:54 AM Christian König
<ckoenig.leichtzumerken at gmail.com> wrote:
>
> Am 13.04.23 um 14:26 schrieb Alex Deucher:
> > On Thu, Apr 13, 2023 at 7:32 AM Christian König
> > <ckoenig.leichtzumerken at gmail.com> wrote:
> >> Ok, then we have a problem.
> >>
> >> Alex what do you think?
> > If you program it to 0, FW skips the GDS backup I think so UMD's can
> > decide whether they want to use it or not, depending on whether they
> > use GDS.
>
> Yeah, but when Mesa isn't using it we have a hard way justifying to
> upstream that because it isn't tested at all.

Well, the interface would still get used, it's just that mesa would
likely only ever pass 0 for the virtual address.  It's just a
passthrough to the packet.  If we discover we need it at some point,
it would be nice to not have to add a new interface to add it.

Alex

>
> Christian.
>
> >
> > Alex
> >
> >
> >> Christian.
> >>
> >> Am 13.04.23 um 11:21 schrieb Marek Olšák:
> >>
> >> That's not why it was removed. It was removed because userspace doesn't use GDS memory and gds_va is always going to be 0.
> >>
> >> Firmware shouldn't use it because using it would increase preemption latency.
> >>
> >> Marek
> >>
> >> On Sun, Apr 9, 2023, 11:21 Christian König <ckoenig.leichtzumerken at gmail.com> wrote:
> >>> We removed the GDS information because they were unnecessary. The GDS size was already part of the device info before we added the shadow info.
> >>>
> >>> But as far as I know the firmware needs valid VAs for all three buffers or won't work correctly.
> >>>
> >>> Christian.
> >>>
> >>> Am 06.04.23 um 17:01 schrieb Marek Olšák:
> >>>
> >>> There is no GDS shadowing info in the device info uapi, so userspace can't create any GDS buffer and thus can't have any GDS va. It's a uapi issue, not what firmware wants to do.
> >>>
> >>> Marek
> >>>
> >>> On Thu, Apr 6, 2023 at 6:31 AM Christian König <ckoenig.leichtzumerken at gmail.com> wrote:
> >>>> That's what I thought as well, but Mitch/Hans insisted on that.
> >>>>
> >>>> We should probably double check internally.
> >>>>
> >>>> Christian.
> >>>>
> >>>> Am 06.04.23 um 11:43 schrieb Marek Olšák:
> >>>>
> >>>> GDS memory isn't used on gfx11. Only GDS OA is used.
> >>>>
> >>>> Marek
> >>>>
> >>>> On Thu, Apr 6, 2023 at 5:09 AM Christian König <christian.koenig at amd.com> wrote:
> >>>>> Why that?
> >>>>>
> >>>>> This is the save buffer for GDS, not the old style GDS BOs.
> >>>>>
> >>>>> Christian.
> >>>>>
> >>>>> Am 06.04.23 um 09:36 schrieb Marek Olšák:
> >>>>>
> >>>>> gds_va is unnecessary.
> >>>>>
> >>>>> Marek
> >>>>>
> >>>>> On Thu, Mar 30, 2023 at 3:18 PM Alex Deucher <alexander.deucher at amd.com> wrote:
> >>>>>> For GFX11, the UMD needs to allocate some shadow buffers
> >>>>>> to be used for preemption.  The UMD allocates the buffers
> >>>>>> and passes the GPU virtual address to the kernel since the
> >>>>>> kernel will program the packet that specified these
> >>>>>> addresses as part of its IB submission frame.
> >>>>>>
> >>>>>> v2: UMD passes shadow init to tell kernel when to initialize
> >>>>>>      the shadow
> >>>>>>
> >>>>>> Reviewed-by: Christian König <christian.koenig at amd.com>
> >>>>>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> >>>>>> ---
> >>>>>>   include/uapi/drm/amdgpu_drm.h | 10 ++++++++++
> >>>>>>   1 file changed, 10 insertions(+)
> >>>>>>
> >>>>>> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> >>>>>> index b6eb90df5d05..3d9474af6566 100644
> >>>>>> --- a/include/uapi/drm/amdgpu_drm.h
> >>>>>> +++ b/include/uapi/drm/amdgpu_drm.h
> >>>>>> @@ -592,6 +592,7 @@ struct drm_amdgpu_gem_va {
> >>>>>>   #define AMDGPU_CHUNK_ID_SCHEDULED_DEPENDENCIES 0x07
> >>>>>>   #define AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT    0x08
> >>>>>>   #define AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL  0x09
> >>>>>> +#define AMDGPU_CHUNK_ID_CP_GFX_SHADOW   0x0a
> >>>>>>
> >>>>>>   struct drm_amdgpu_cs_chunk {
> >>>>>>          __u32           chunk_id;
> >>>>>> @@ -708,6 +709,15 @@ struct drm_amdgpu_cs_chunk_data {
> >>>>>>          };
> >>>>>>   };
> >>>>>>
> >>>>>> +#define AMDGPU_CS_CHUNK_CP_GFX_SHADOW_FLAGS_INIT_SHADOW         0x1
> >>>>>> +
> >>>>>> +struct drm_amdgpu_cs_chunk_cp_gfx_shadow {
> >>>>>> +       __u64 shadow_va;
> >>>>>> +       __u64 csa_va;
> >>>>>> +       __u64 gds_va;
> >>>>>> +       __u64 flags;
> >>>>>> +};
> >>>>>> +
> >>>>>>   /*
> >>>>>>    *  Query h/w info: Flag that this is integrated (a.h.a. fusion) GPU
> >>>>>>    *
> >>>>>> --
> >>>>>> 2.39.2
> >>>>>>
>