Re: [PATCH 1/2] drm/amdgpu: make mcbp a per device setting

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

Am 16.06.23 um 23:10 schrieb Alex Deucher:
> So we can selectively enable it on certain devices.  No
> intended functional change.
>
> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>

Acked-by: Christian König <christian.koenig@xxxxxxx> for the series.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 19 +++++++++++++++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h    |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c   |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   |  3 ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c     |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c     |  2 +-
>   7 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index f39db4a2c2cf..78c6265fe79b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2551,7 +2551,7 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
>   			adev->ip_blocks[i].status.hw = true;
>   
>   			/* right after GMC hw init, we create CSA */
> -			if (amdgpu_mcbp) {
> +			if (adev->gfx.mcbp) {
>   				r = amdgpu_allocate_static_csa(adev, &adev->virt.csa_obj,
>   							       AMDGPU_GEM_DOMAIN_VRAM |
>   							       AMDGPU_GEM_DOMAIN_GTT,
> @@ -3672,6 +3672,18 @@ static const struct attribute *amdgpu_dev_attributes[] = {
>   	NULL
>   };
>   
> +static void amdgpu_device_set_mcbp(struct amdgpu_device *adev)
> +{
> +	if (amdgpu_mcbp == 1)
> +		adev->gfx.mcbp = true;
> +
> +	if (amdgpu_sriov_vf(adev))
> +		adev->gfx.mcbp = true;
> +
> +	if (adev->gfx.mcbp)
> +		DRM_INFO("MCBP is enabled\n");
> +}
> +
>   /**
>    * amdgpu_device_init - initialize the driver
>    *
> @@ -3823,9 +3835,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   	DRM_INFO("register mmio base: 0x%08X\n", (uint32_t)adev->rmmio_base);
>   	DRM_INFO("register mmio size: %u\n", (unsigned)adev->rmmio_size);
>   
> -	if (amdgpu_mcbp)
> -		DRM_INFO("MCBP is enabled\n");
> -
>   	/*
>   	 * Reset domain needs to be present early, before XGMI hive discovered
>   	 * (if any) and intitialized to use reset sem and in_gpu reset flag
> @@ -3851,6 +3860,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   	if (r)
>   		return r;
>   
> +	amdgpu_device_set_mcbp(adev);
> +
>   	/* Get rid of things like offb */
>   	r = drm_aperture_remove_conflicting_pci_framebuffers(adev->pdev, &amdgpu_kms_driver);
>   	if (r)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index ce0f7a8ad4b8..a4ff515ce896 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -434,6 +434,7 @@ struct amdgpu_gfx {
>   	uint16_t 			xcc_mask;
>   	uint32_t			num_xcc_per_xcp;
>   	struct mutex			partition_mutex;
> +	bool				mcbp; /* mid command buffer preemption */
>   };
>   
>   struct amdgpu_gfx_ras_reg_entry {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index e3531aa3c8bd..cca5a495611f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -805,7 +805,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>   		dev_info->ids_flags = 0;
>   		if (adev->flags & AMD_IS_APU)
>   			dev_info->ids_flags |= AMDGPU_IDS_FLAGS_FUSION;
> -		if (amdgpu_mcbp)
> +		if (adev->gfx.mcbp)
>   			dev_info->ids_flags |= AMDGPU_IDS_FLAGS_PREEMPTION;
>   		if (amdgpu_is_tmz(adev))
>   			dev_info->ids_flags |= AMDGPU_IDS_FLAGS_TMZ;
> @@ -1247,7 +1247,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
>   		goto error_vm;
>   	}
>   
> -	if (amdgpu_mcbp) {
> +	if (adev->gfx.mcbp) {
>   		uint64_t csa_addr = amdgpu_csa_vaddr(adev) & AMDGPU_GMC_HOLE_MASK;
>   
>   		r = amdgpu_map_static_csa(adev, &fpriv->vm, adev->virt.csa_obj,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> index 78ec3420ef85..dacf281d2b21 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> @@ -72,7 +72,7 @@ uint64_t amdgpu_sdma_get_csa_mc_addr(struct amdgpu_ring *ring,
>   	int r;
>   
>   	/* don't enable OS preemption on SDMA under SRIOV */
> -	if (amdgpu_sriov_vf(adev) || vmid == 0 || !amdgpu_mcbp)
> +	if (amdgpu_sriov_vf(adev) || vmid == 0 || !adev->gfx.mcbp)
>   		return 0;
>   
>   	if (ring->is_mes_queue) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 25b4d7f0bd35..41aa853a07d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -66,9 +66,6 @@ void amdgpu_virt_init_setting(struct amdgpu_device *adev)
>   	adev->cg_flags = 0;
>   	adev->pg_flags = 0;
>   
> -	/* enable mcbp for sriov */
> -	amdgpu_mcbp = 1;
> -
>   	/* Reduce kcq number to 2 to reduce latency */
>   	if (amdgpu_num_kcq == -1)
>   		amdgpu_num_kcq = 2;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index be984f8c71c7..44af8022b89f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -8307,7 +8307,7 @@ static void gfx_v10_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>   
>   	control |= ib->length_dw | (vmid << 24);
>   
> -	if (amdgpu_mcbp && (ib->flags & AMDGPU_IB_FLAG_PREEMPT)) {
> +	if (ring->adev->gfx.mcbp && (ib->flags & AMDGPU_IB_FLAG_PREEMPT)) {
>   		control |= INDIRECT_BUFFER_PRE_ENB(1);
>   
>   		if (flags & AMDGPU_IB_PREEMPTED)
> @@ -8482,7 +8482,7 @@ static void gfx_v10_0_ring_emit_cntxcntl(struct amdgpu_ring *ring,
>   {
>   	uint32_t dw2 = 0;
>   
> -	if (amdgpu_mcbp)
> +	if (ring->adev->gfx.mcbp)
>   		gfx_v10_0_ring_emit_ce_meta(ring,
>   				    (!amdgpu_sriov_vf(ring->adev) && flags & AMDGPU_IB_PREEMPTED) ? true : false);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> index 690e121d9dda..3a7af59e83ca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> @@ -5311,7 +5311,7 @@ static void gfx_v11_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>   
>   	control |= ib->length_dw | (vmid << 24);
>   
> -	if (amdgpu_mcbp && (ib->flags & AMDGPU_IB_FLAG_PREEMPT)) {
> +	if (ring->adev->gfx.mcbp && (ib->flags & AMDGPU_IB_FLAG_PREEMPT)) {
>   		control |= INDIRECT_BUFFER_PRE_ENB(1);
>   
>   		if (flags & AMDGPU_IB_PREEMPTED)