Hi Christian, Thanks for specifying it. Please check my update in another mail if getting the correct info as you mentioned. > Just make sure that the default value is zero, this way you keep backward > compatible with older kernel interfaces. I have checked all gfx from 6 to 8. By default they are 1. Just 2 APUs(gfxv8) are 0. Regards, Jerry (Junwei Zhang) Linux Base Graphics SRDC Software Development _____________________________________ > -----Original Message----- > From: Christian König [mailto:deathsimple at vodafone.de] > Sent: Thursday, February 16, 2017 17:37 > To: Zhang, Jerry; amd-gfx at lists.freedesktop.org > Subject: Re: [PATCH 1/1] drm/amdgpu: export gfx capability by gpu info > > Am 16.02.2017 um 10:24 schrieb Zhang, Jerry: > >> -----Original Message----- > >> From: Christian König [mailto:deathsimple at vodafone.de] > >> Sent: Thursday, February 16, 2017 17:12 > >> To: Zhang, Jerry; amd-gfx at lists.freedesktop.org > >> Subject: Re: [PATCH 1/1] drm/amdgpu: export gfx capability by gpu > >> info > >> > >> Am 16.02.2017 um 03:53 schrieb Junwei Zhang: > >>> Change-Id: Ibf3e4dbb7deb83271adabc275c9b7a0e0652541a > >>> Signed-off-by: Junwei Zhang <Jerry.Zhang at amd.com> > >> Complete NAK on this. The drm_amdgpu_capability structure is specific > >> to the pro driver. > > Thanks to reminder it. > > I neglect it. > > > >> The Vulkan stack on the other hand is supposed to work on the open > >> stack as well. > >> > >> So please but that into the drm_amdgpu_info_device structure and the > >> new field in the amdgpu_gca_config structure (you might as well want > >> to rename the amdgpu_gca_config structure). > > So you mean add the feature or config structure in amdgpu_device directly > from now on. > > Right? > > No, just forget about the config or feature structure. Add gfx features to > amdgpu_gca_config. > > > Could you explain the meaning about gca? > > I just think that the naming isn't the best, something like amdgpu_gfx_config > would probably be better. > > > > > BTW, > > 1) in the future we may add more feature/config in the same structure, but > may not for gfx. > > Stop, that is really bad design. The features/config should be kept on a per IP > basis. > > So gfx config/features should only be grouped together with gfx features. Don't > mix features from different block inside the kernel module. > > The IOCTL interface is a different story, here we have the > drm_amdgpu_info_device structure for information about the whole GPU as > well as individual structures about each IP block (like firmware versions etc...). > > For the current case adding that to drm_amdgpu_info_device sounds like the > right approach to me. > > > 2) Shall we add a version to control these features? UMD could know the > features status according to the version. > > No, the size of the drm_amdgpu_info_device structure is already version > controlled. > > Just make sure that the default value is zero, this way you keep backward > compatible with older kernel interfaces. > > Regards, > Christian. > > > > >> Regards, > >> Christian. > >> > >>> --- > >>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 6 ++++++ > >>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 ++ > >>> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 6 ++++++ > >>> drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 6 ++++++ > >>> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 14 ++++++++++++++ > >>> include/uapi/drm/amdgpu_drm.h | 1 + > >>> 6 files changed, 35 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > >>> index 1ad3f08..cdc2b2a 100644 > >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > >>> @@ -869,6 +869,10 @@ struct amdgpu_gfx_funcs { > >>> void (*read_wave_sgprs)(struct amdgpu_device *adev, uint32_t > >>> simd, > >> uint32_t wave, uint32_t start, uint32_t size, uint32_t *dst); > >>> }; > >>> > >>> +struct amdgpu_gfx_cap { > >>> + uint32_t gc_double_offchip_lds_buf; }; > >>> + > >>> struct amdgpu_gfx { > >>> struct mutex gpu_clock_mutex; > >>> struct amdgpu_gca_config config; > >>> @@ -911,6 +915,8 @@ struct amdgpu_gfx { > >>> /* reset mask */ > >>> uint32_t grbm_soft_reset; > >>> uint32_t srbm_soft_reset; > >>> + > >>> + struct amdgpu_gfx_cap cap; > >>> }; > >>> > >>> int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm > >>> *vm, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > >>> index 7f59608..e7aa382 100644 > >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > >>> @@ -618,6 +618,8 @@ static int amdgpu_info_ioctl(struct drm_device > >>> *dev, > >> void *data, struct drm_file > >>> cap.flag |= AMDGPU_CAPABILITY_DIRECT_GMA_FLAG; > >>> cap.direct_gma_size = amdgpu_direct_gma_size; > >>> } > >>> + cap.gc_double_offchip_lds_buf = > >>> + adev->gfx.cap.gc_double_offchip_lds_buf; > >>> return copy_to_user(out, &cap, > >>> min((size_t)size, sizeof(cap))) ? -EFAULT : 0; > >>> } > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c > >>> b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c > >>> index ce75d46..a1e221b 100644 > >>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c > >>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c > >>> @@ -1534,6 +1534,11 @@ static void gfx_v6_0_setup_spi(struct > >> amdgpu_device *adev, > >>> mutex_unlock(&adev->grbm_idx_mutex); > >>> } > >>> > >>> +static void gfx_v6_0_cap_init(struct amdgpu_device *adev) { > >>> + adev->gfx.cap.gc_double_offchip_lds_buf = 1; } > >>> + > >>> static void gfx_v6_0_gpu_init(struct amdgpu_device *adev) > >>> { > >>> u32 gb_addr_config = 0; > >>> @@ -1692,6 +1697,7 @@ static void gfx_v6_0_gpu_init(struct > >>> amdgpu_device > >> *adev) > >>> adev->gfx.config.max_cu_per_sh); > >>> > >>> gfx_v6_0_get_cu_info(adev); > >>> + gfx_v6_0_cap_init(adev); > >>> > >>> WREG32(mmCP_QUEUE_THRESHOLDS, ((0x16 << > >> CP_QUEUE_THRESHOLDS__ROQ_IB1_START__SHIFT) | > >>> (0x2b << > >> CP_QUEUE_THRESHOLDS__ROQ_IB2_START__SHIFT))); > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > >>> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > >>> index aaf66fe..e1e97ae 100644 > >>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > >>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c > >>> @@ -1876,6 +1876,11 @@ static void gmc_v7_0_init_compute_vmid(struct > >> amdgpu_device *adev) > >>> mutex_unlock(&adev->srbm_mutex); > >>> } > >>> > >>> +static void gfx_v7_0_cap_init(struct amdgpu_device *adev) { > >>> + adev->gfx.cap.gc_double_offchip_lds_buf = 1; } > >>> + > >>> /** > >>> * gfx_v7_0_gpu_init - setup the 3D engine > >>> * > >>> @@ -1900,6 +1905,7 @@ static void gfx_v7_0_gpu_init(struct > >>> amdgpu_device *adev) > >>> > >>> gfx_v7_0_setup_rb(adev); > >>> gfx_v7_0_get_cu_info(adev); > >>> + gfx_v7_0_cap_init(adev); > >>> > >>> /* set HW defaults for 3D engine */ > >>> WREG32(mmCP_MEQ_THRESHOLDS, > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > >>> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > >>> index ce05e38..934fc71 100644 > >>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > >>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > >>> @@ -3826,6 +3826,19 @@ static void gfx_v8_0_init_compute_vmid(struct > >> amdgpu_device *adev) > >>> mutex_unlock(&adev->srbm_mutex); > >>> } > >>> > >>> +static void gfx_v8_0_cap_init(struct amdgpu_device *adev) { > >>> + switch (adev->asic_type) { > >>> + default: > >>> + adev->gfx.cap.gc_double_offchip_lds_buf = 1; > >>> + break; > >>> + case CHIP_CARRIZO: > >>> + case CHIP_STONEY: > >>> + adev->gfx.cap.gc_double_offchip_lds_buf = 0; > >>> + break; > >>> + } > >>> +} > >>> + > >>> static void gfx_v8_0_gpu_init(struct amdgpu_device *adev) > >>> { > >>> u32 tmp, sh_static_mem_cfg; > >>> @@ -3839,6 +3852,7 @@ static void gfx_v8_0_gpu_init(struct > >>> amdgpu_device > >> *adev) > >>> gfx_v8_0_tiling_mode_table_init(adev); > >>> gfx_v8_0_setup_rb(adev); > >>> gfx_v8_0_get_cu_info(adev); > >>> + gfx_v8_0_cap_init(adev); > >>> > >>> /* XXX SH_MEM regs */ > >>> /* where to put LDS, scratch, GPUVM in FSA64 space */ diff --git > >>> a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h > >>> index 04daab3..7064fdc 100644 > >>> --- a/include/uapi/drm/amdgpu_drm.h > >>> +++ b/include/uapi/drm/amdgpu_drm.h > >>> @@ -853,6 +853,7 @@ struct drm_amdgpu_virtual_range { > >>> struct drm_amdgpu_capability { > >>> __u32 flag; > >>> __u32 direct_gma_size; > >>> + __u32 gc_double_offchip_lds_buf; > >>> }; > >>> > >>> /* > > _______________________________________________ > > amd-gfx mailing list > > amd-gfx at lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx >
