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

[Christian König]

Me as well.

Christian.

Am 13.04.23 um 20:23 schrieb Marek Olšák:
> I'm OK with that.
>
> Marek
>
> On Thu, Apr 13, 2023 at 12:56 PM Alex Deucher <alexdeucher at gmail.com> 
> wrote:
>
>     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
>     > >>>>>>
>     >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20230413/7b8e3c94/attachment.htm>