I'm OK with that. Marek
On Thu, Apr 13, 2023 at 12:56 PM Alex Deucher <alexdeuc...@gmail.com> wrote: > On Thu, Apr 13, 2023 at 11:54 AM Christian König > <ckoenig.leichtzumer...@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.leichtzumer...@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.leichtzumer...@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.leichtzumer...@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.koe...@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.deuc...@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.koe...@amd.com> > > >>>>>> Signed-off-by: Alex Deucher <alexander.deuc...@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 > > >>>>>> > > >