[PATCH 1/2] drm/amdgpu: Clean up gmc_v9_0_gart_enable

[PATCH 1/2] drm/amdgpu: Clean up gmc_v9_0_gart_enable

[Zeng, Oak]

Many logic in this function are HDP set up,
not gart set up. Moved those logic to gmc_v9_0_hw_init.
No functional change.

Change-Id: Ib00cc1ffd1e486e77571796dce53aa7506c0c55f
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 82 +++++++++++++++++------------------
 1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 4b11f7e..c7e07f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1135,13 +1135,7 @@ static void gmc_v9_0_init_golden_registers(struct amdgpu_device *adev)
  */
 static int gmc_v9_0_gart_enable(struct amdgpu_device *adev)
 {
-	int r, i;
-	bool value;
-	u32 tmp;
-
-	amdgpu_device_program_register_sequence(adev,
-						golden_settings_vega10_hdp,
-						ARRAY_SIZE(golden_settings_vega10_hdp));
+	int r;
 
 	if (adev->gart.bo == NULL) {
 		dev_err(adev->dev, "No VRAM object for PCIE GART.\n");
@@ -1151,15 +1145,6 @@ static int gmc_v9_0_gart_enable(struct amdgpu_device *adev)
 	if (r)
 		return r;
 
-	switch (adev->asic_type) {
-	case CHIP_RAVEN:
-		/* TODO for renoir */
-		mmhub_v1_0_update_power_gating(adev, true);
-		break;
-	default:
-		break;
-	}
-
 	r = gfxhub_v1_0_gart_enable(adev);
 	if (r)
 		return r;
@@ -1171,6 +1156,46 @@ static int gmc_v9_0_gart_enable(struct amdgpu_device *adev)
 	if (r)
 		return r;
 
+	DRM_INFO("PCIE GART of %uM enabled (table at 0x%016llX).\n",
+		 (unsigned)(adev->gmc.gart_size >> 20),
+		 (unsigned long long)amdgpu_bo_gpu_offset(adev->gart.bo));
+	adev->gart.ready = true;
+	return 0;
+}
+
+static int gmc_v9_0_hw_init(void *handle)
+{
+	int r, i;
+	bool value;
+	u32 tmp;
+	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+	/* The sequence of these two function calls matters.*/
+	gmc_v9_0_init_golden_registers(adev);
+
+	if (adev->mode_info.num_crtc) {
+		if (adev->asic_type != CHIP_ARCTURUS) {
+			/* Lockout access through VGA aperture*/
+			WREG32_FIELD15(DCE, 0, VGA_HDP_CONTROL, VGA_MEMORY_DISABLE, 1);
+
+			/* disable VGA render */
+			WREG32_FIELD15(DCE, 0, VGA_RENDER_CONTROL, VGA_VSTATUS_CNTL, 0);
+		}
+	}
+
+	amdgpu_device_program_register_sequence(adev,
+						golden_settings_vega10_hdp,
+						ARRAY_SIZE(golden_settings_vega10_hdp));
+
+	switch (adev->asic_type) {
+	case CHIP_RAVEN:
+		/* TODO for renoir */
+		mmhub_v1_0_update_power_gating(adev, true);
+		break;
+	default:
+		break;
+	}
+
 	WREG32_FIELD15(HDP, 0, HDP_MISC_CNTL, FLUSH_INVALIDATE_CACHE, 1);
 
 	tmp = RREG32_SOC15(HDP, 0, mmHDP_HOST_PATH_CNTL);
@@ -1199,31 +1224,6 @@ static int gmc_v9_0_gart_enable(struct amdgpu_device *adev)
 	if (adev->umc.funcs && adev->umc.funcs->init_registers)
 		adev->umc.funcs->init_registers(adev);
 
-	DRM_INFO("PCIE GART of %uM enabled (table at 0x%016llX).\n",
-		 (unsigned)(adev->gmc.gart_size >> 20),
-		 (unsigned long long)amdgpu_bo_gpu_offset(adev->gart.bo));
-	adev->gart.ready = true;
-	return 0;
-}
-
-static int gmc_v9_0_hw_init(void *handle)
-{
-	int r;
-	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-
-	/* The sequence of these two function calls matters.*/
-	gmc_v9_0_init_golden_registers(adev);
-
-	if (adev->mode_info.num_crtc) {
-		if (adev->asic_type != CHIP_ARCTURUS) {
-			/* Lockout access through VGA aperture*/
-			WREG32_FIELD15(DCE, 0, VGA_HDP_CONTROL, VGA_MEMORY_DISABLE, 1);
-
-			/* disable VGA render */
-			WREG32_FIELD15(DCE, 0, VGA_RENDER_CONTROL, VGA_VSTATUS_CNTL, 0);
-		}
-	}
-
 	r = gmc_v9_0_gart_enable(adev);
 
 	return r;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amdgpu: Enable gfx cache probing on HDP write for arcturus

[Zeng, Oak]

This allows gfx cache to be probed and invalidated (for none-dirty cache lines)
on a HDP write (from either another GPU or CPU). This should work only for the
memory mapped as RW memory type newly added for arcturus, to achieve some cache
coherence b/t multiple memory clients.

Change-Id: I5c9a6a25d88cd75c71c88822123e0d4c067aa3f8
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index c7e07f1..6e45ebb 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1192,6 +1192,9 @@ static int gmc_v9_0_hw_init(void *handle)
 		/* TODO for renoir */
 		mmhub_v1_0_update_power_gating(adev, true);
 		break;
+	case CHIP_ARCTURUS:
+		WREG32_FIELD15(HDP, 0, HDP_MMHUB_CNTL, HDP_MMHUB_GCC, 1);
+		break;
 	default:
 		break;
 	}
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Clean up gmc_v9_0_gart_enable

[Christian König]

Am 07.10.19 um 22:34 schrieb Zeng, Oak:
> Many logic in this function are HDP set up,
> not gart set up. Moved those logic to gmc_v9_0_hw_init.
> No functional change.
>
> Change-Id: Ib00cc1ffd1e486e77571796dce53aa7506c0c55f
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>

One minor note on the coding style below, apart from that the patch is 
Acked-by: Christian König <christian.koenig@amd.com>.

> ---
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 82 +++++++++++++++++------------------
>   1 file changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 4b11f7e..c7e07f1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1135,13 +1135,7 @@ static void gmc_v9_0_init_golden_registers(struct amdgpu_device *adev)
>    */
>   static int gmc_v9_0_gart_enable(struct amdgpu_device *adev)
>   {
> -	int r, i;
> -	bool value;
> -	u32 tmp;
> -
> -	amdgpu_device_program_register_sequence(adev,
> -						golden_settings_vega10_hdp,
> -						ARRAY_SIZE(golden_settings_vega10_hdp));
> +	int r;
>   
>   	if (adev->gart.bo == NULL) {
>   		dev_err(adev->dev, "No VRAM object for PCIE GART.\n");
> @@ -1151,15 +1145,6 @@ static int gmc_v9_0_gart_enable(struct amdgpu_device *adev)
>   	if (r)
>   		return r;
>   
> -	switch (adev->asic_type) {
> -	case CHIP_RAVEN:
> -		/* TODO for renoir */
> -		mmhub_v1_0_update_power_gating(adev, true);
> -		break;
> -	default:
> -		break;
> -	}
> -
>   	r = gfxhub_v1_0_gart_enable(adev);
>   	if (r)
>   		return r;
> @@ -1171,6 +1156,46 @@ static int gmc_v9_0_gart_enable(struct amdgpu_device *adev)
>   	if (r)
>   		return r;
>   
> +	DRM_INFO("PCIE GART of %uM enabled (table at 0x%016llX).\n",
> +		 (unsigned)(adev->gmc.gart_size >> 20),
> +		 (unsigned long long)amdgpu_bo_gpu_offset(adev->gart.bo));
> +	adev->gart.ready = true;
> +	return 0;
> +}
> +
> +static int gmc_v9_0_hw_init(void *handle)
> +{
> +	int r, i;
> +	bool value;
> +	u32 tmp;
> +	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

We usually try to have variables like r and i declared last and long 
structure defines first.

Regards,
Christian.

> +
> +	/* The sequence of these two function calls matters.*/
> +	gmc_v9_0_init_golden_registers(adev);
> +
> +	if (adev->mode_info.num_crtc) {
> +		if (adev->asic_type != CHIP_ARCTURUS) {
> +			/* Lockout access through VGA aperture*/
> +			WREG32_FIELD15(DCE, 0, VGA_HDP_CONTROL, VGA_MEMORY_DISABLE, 1);
> +
> +			/* disable VGA render */
> +			WREG32_FIELD15(DCE, 0, VGA_RENDER_CONTROL, VGA_VSTATUS_CNTL, 0);
> +		}
> +	}
> +
> +	amdgpu_device_program_register_sequence(adev,
> +						golden_settings_vega10_hdp,
> +						ARRAY_SIZE(golden_settings_vega10_hdp));
> +
> +	switch (adev->asic_type) {
> +	case CHIP_RAVEN:
> +		/* TODO for renoir */
> +		mmhub_v1_0_update_power_gating(adev, true);
> +		break;
> +	default:
> +		break;
> +	}
> +
>   	WREG32_FIELD15(HDP, 0, HDP_MISC_CNTL, FLUSH_INVALIDATE_CACHE, 1);
>   
>   	tmp = RREG32_SOC15(HDP, 0, mmHDP_HOST_PATH_CNTL);
> @@ -1199,31 +1224,6 @@ static int gmc_v9_0_gart_enable(struct amdgpu_device *adev)
>   	if (adev->umc.funcs && adev->umc.funcs->init_registers)
>   		adev->umc.funcs->init_registers(adev);
>   
> -	DRM_INFO("PCIE GART of %uM enabled (table at 0x%016llX).\n",
> -		 (unsigned)(adev->gmc.gart_size >> 20),
> -		 (unsigned long long)amdgpu_bo_gpu_offset(adev->gart.bo));
> -	adev->gart.ready = true;
> -	return 0;
> -}
> -
> -static int gmc_v9_0_hw_init(void *handle)
> -{
> -	int r;
> -	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> -
> -	/* The sequence of these two function calls matters.*/
> -	gmc_v9_0_init_golden_registers(adev);
> -
> -	if (adev->mode_info.num_crtc) {
> -		if (adev->asic_type != CHIP_ARCTURUS) {
> -			/* Lockout access through VGA aperture*/
> -			WREG32_FIELD15(DCE, 0, VGA_HDP_CONTROL, VGA_MEMORY_DISABLE, 1);
> -
> -			/* disable VGA render */
> -			WREG32_FIELD15(DCE, 0, VGA_RENDER_CONTROL, VGA_VSTATUS_CNTL, 0);
> -		}
> -	}
> -
>   	r = gmc_v9_0_gart_enable(adev);
>   
>   	return r;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: Enable gfx cache probing on HDP write for arcturus

[Christian König]

Am 07.10.19 um 22:34 schrieb Zeng, Oak:
> This allows gfx cache to be probed and invalidated (for none-dirty cache lines)
> on a HDP write (from either another GPU or CPU). This should work only for the
> memory mapped as RW memory type newly added for arcturus, to achieve some cache
> coherence b/t multiple memory clients.
>
> Change-Id: I5c9a6a25d88cd75c71c88822123e0d4c067aa3f8
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index c7e07f1..6e45ebb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1192,6 +1192,9 @@ static int gmc_v9_0_hw_init(void *handle)
>   		/* TODO for renoir */
>   		mmhub_v1_0_update_power_gating(adev, true);
>   		break;
> +	case CHIP_ARCTURUS:
> +		WREG32_FIELD15(HDP, 0, HDP_MMHUB_CNTL, HDP_MMHUB_GCC, 1);
> +		break;
>   	default:
>   		break;
>   	}

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx