The INFO ioctl was designed to allow increasing the sizes of all info structures. GB_ADDR_CONFIG isn't that special to justify a separate query. Marek On Wed, Jun 19, 2024 at 5:31 AM Christian König <christian.koenig@xxxxxxx> wrote: > > I would try to avoid that. > > Putting everything into amdgpu_info_device was a mistake only done > because people assumed that IOCTLs on Linux are to expensive to query > all information separately. > > We should rather have distinct IOCTLs for each value because that is way > more flexible and we won't find later that we have to deprecate fields > and work around issues because of legacy hw. > > Regards, > Christian. > > Am 19.06.24 um 02:34 schrieb Marek Olšák: > > I would put this into drm_amdgpu_info_device. That structure can grow in size. > > > > Marek > > > > On Tue, Jun 18, 2024 at 11:30 AM Pierre-Eric Pelloux-Prayer > > <pierre-eric.pelloux-prayer@xxxxxxx> wrote: > >> libdrm_amdgpu uses AMDGPU_INFO_READ_MMR_REG to fill the dev->info.gb_addr_cfg > >> value. > >> Since this value is already known by the kernel, this commit implements a new > >> query to return it. > >> > >> The libdrm MR to use this query is: > >> https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/368 > >> > >> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@xxxxxxx> > >> --- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 5 +++++ > >> include/uapi/drm/amdgpu_drm.h | 2 ++ > >> 3 files changed, 9 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > >> index f51f121d804e..403add7f05af 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > >> @@ -117,9 +117,10 @@ > >> * - 3.56.0 - Update IB start address and size alignment for decode and encode > >> * - 3.57.0 - Compute tunneling on GFX10+ > >> * - 3.58.0 - Add AMDGPU_IDS_FLAGS_MODE_PF, AMDGPU_IDS_FLAGS_MODE_VF & AMDGPU_IDS_FLAGS_MODE_PT > >> + * - 3.59.0 - Add AMDGPU_INFO_GB_ADDR_CONFIG support > >> */ > >> #define KMS_DRIVER_MAJOR 3 > >> -#define KMS_DRIVER_MINOR 58 > >> +#define KMS_DRIVER_MINOR 59 > >> #define KMS_DRIVER_PATCHLEVEL 0 > >> > >> /* > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > >> index b32ff6e1baaf..dbb05d51682b 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > >> @@ -1256,6 +1256,10 @@ static int amdgpu_info(struct drm_device *dev, void *data, struct drm_file *filp > >> return copy_to_user(out, &gpuvm_fault, > >> min((size_t)size, sizeof(gpuvm_fault))) ? -EFAULT : 0; > >> } > >> + case AMDGPU_INFO_GB_ADDR_CONFIG: { > >> + ui32 = adev->gfx.config.gb_addr_config; > >> + return copy_to_user(out, &ui32, min(size, 4u)) ? -EFAULT : 0; > >> + } > >> default: > >> DRM_DEBUG_KMS("Invalid request %d\n", info->query); > >> return -EINVAL; > >> @@ -1310,6 +1314,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) > >> case AMDGPU_INFO_VIDEO_CAPS: > >> case AMDGPU_INFO_MAX_IBS: > >> case AMDGPU_INFO_GPUVM_FAULT: > >> + case AMDGPU_INFO_GB_ADDR_CONFIG: > >> need_runtime_pm = false; > >> break; > >> > >> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h > >> index 3e488b0119eb..680492cd73d8 100644 > >> --- a/include/uapi/drm/amdgpu_drm.h > >> +++ b/include/uapi/drm/amdgpu_drm.h > >> @@ -933,6 +933,8 @@ struct drm_amdgpu_cs_chunk_cp_gfx_shadow { > >> #define AMDGPU_INFO_MAX_IBS 0x22 > >> /* query last page fault info */ > >> #define AMDGPU_INFO_GPUVM_FAULT 0x23 > >> +/* Query GB_ADDR_CONFIG */ > >> +#define AMDGPU_INFO_GB_ADDR_CONFIG 0x24 > >> > >> #define AMDGPU_INFO_MMR_SE_INDEX_SHIFT 0 > >> #define AMDGPU_INFO_MMR_SE_INDEX_MASK 0xff > >> -- > >> 2.40.1 > >> >
