<div dir="ltr"><div>I'm OK with that.</div><div> </div><div>Marek </div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Apr 13, 2023 at 12:56 PM Alex Deucher <<a href="mailto:alexdeucher@gmail.com">alexdeucher@gmail.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, Apr 13, 2023 at 11:54 AM Christian König <<a href="mailto:ckoenig.leichtzumerken@gmail.com" target="_blank">ckoenig.leichtzumerken@gmail.com</a>> wrote: > > Am 13.04.23 um 14:26 schrieb Alex Deucher: > > On Thu, Apr 13, 2023 at 7:32 AM Christian König > > <<a href="mailto:ckoenig.leichtzumerken@gmail.com" target="_blank">ckoenig.leichtzumerken@gmail.com</a>> 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 <<a href="mailto:ckoenig.leichtzumerken@gmail.com" target="_blank">ckoenig.leichtzumerken@gmail.com</a>> 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 <<a href="mailto:ckoenig.leichtzumerken@gmail.com" target="_blank">ckoenig.leichtzumerken@gmail.com</a>> 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 <<a href="mailto:christian.koenig@amd.com" target="_blank">christian.koenig@amd.com</a>> 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 <<a href="mailto:alexander.deucher@amd.com" target="_blank">alexander.deucher@amd.com</a>> 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 <<a href="mailto:christian.koenig@amd.com" target="_blank">christian.koenig@amd.com</a>> > >>>>>> Signed-off-by: Alex Deucher <<a href="mailto:alexander.deucher@amd.com" target="_blank">alexander.deucher@amd.com</a>> > >>>>>> --- > >>>>>> 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 > >>>>>> > </blockquote></div>