[PATCH 1/1] drm/amdgpu: export gfx capability by gpu info

Zhang, Jerry Jerry.Zhang at amd.com
Fri Feb 17 05:02:53 UTC 2017


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
> 



More information about the amd-gfx mailing list