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

[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/05948ded/attachment-0001.htm>