[9/9] drm/amdgpu:properly fix some JumpTable issues

Submitted by Liu, Monk on Sept. 28, 2016, 8:36 a.m.

Details

Message ID 1475051780-21634-9-git-send-email-Monk.Liu@amd.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Liu, Monk Sept. 28, 2016, 8:36 a.m.
we found some MEC ucode leads to IB test fail or even
ring test fail if Jump Table of it is not start in
FW bo with page aligned address, fixed by always make
JT address page aligned.

we don't need to patch JT2 for MEC2, because for VI,
MEC2 is a copy of MEC1, thus when converting fw_type
for MEC_JT2 we just return MEC1,hw can use the same
JT for both MEC1 & MEC2.

above two change fixed some ring/ib test failure issue
for some version of MEC ucode.

Change-Id: Ie3b3c4c5722fdf68f64547cdfbf9c0d3274a2a15
Signed-off-by: Frank Min <Frank.Min@amd.com>
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c   | 21 ++++++++++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 32 +++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
index cb1ade1..7278898 100755
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
@@ -685,11 +685,14 @@  static uint32_t fw_type_convert(struct cgs_device *cgs_device, uint32_t fw_type)
 		result = AMDGPU_UCODE_ID_CP_MEC1;
 		break;
 	case CGS_UCODE_ID_CP_MEC_JT2:
-		if (adev->asic_type == CHIP_TONGA || adev->asic_type == CHIP_POLARIS11
-		  || adev->asic_type == CHIP_POLARIS10)
-			result = AMDGPU_UCODE_ID_CP_MEC2;
-		else
+		/* for VI. JT2 should be the same as JT1, because:
+			1, MEC2 and MEC1 use exactly same FW.
+			2, JT2 is not pached but JT1 is.
+		*/
+		if (adev->asic_type >= CHIP_TOPAZ)
 			result = AMDGPU_UCODE_ID_CP_MEC1;
+		else
+			result = AMDGPU_UCODE_ID_CP_MEC2;
 		break;
 	case CGS_UCODE_ID_RLC_G:
 		result = AMDGPU_UCODE_ID_RLC_G;
@@ -779,12 +782,18 @@  static int amdgpu_cgs_get_firmware_info(struct cgs_device *cgs_device,
 
 		if ((type == CGS_UCODE_ID_CP_MEC_JT1) ||
 		    (type == CGS_UCODE_ID_CP_MEC_JT2)) {
-			gpu_addr += le32_to_cpu(header->jt_offset) << 2;
+			gpu_addr += ALIGN(le32_to_cpu(header->header.ucode_size_bytes), PAGE_SIZE);
 			data_size = le32_to_cpu(header->jt_size) << 2;
 		}
-		info->mc_addr = gpu_addr;
+
+		info->kptr = ucode->kaddr;
 		info->image_size = data_size;
+		info->mc_addr = gpu_addr;
 		info->version = (uint16_t)le32_to_cpu(header->header.ucode_version);
+
+		if (CGS_UCODE_ID_CP_MEC == type)
+			info->image_size = (header->jt_offset) << 2;
+
 		info->fw_version = amdgpu_get_firmware_version(cgs_device, type);
 		info->feature_version = (uint16_t)le32_to_cpu(header->ucode_feature_version);
 	} else {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index 06baac9..e2ea2c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -239,6 +239,31 @@  static int amdgpu_ucode_init_single_fw(struct amdgpu_firmware_info *ucode,
 	return 0;
 }
 
+static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode,
+				uint64_t mc_addr, void *kptr)
+{
+	const struct gfx_firmware_header_v1_0 *header = NULL;
+	const struct common_firmware_header *comm_hdr = NULL;
+	uint8_t* src_addr = NULL;
+	uint8_t* dst_addr = NULL;
+
+	if (NULL == ucode->fw)
+		return 0;
+
+	comm_hdr = (const struct common_firmware_header *)ucode->fw->data;
+	header = (const struct gfx_firmware_header_v1_0 *)ucode->fw->data;
+	dst_addr = ucode->kaddr +
+			   ALIGN(le32_to_cpu(comm_hdr->ucode_size_bytes),
+			   PAGE_SIZE);
+	src_addr = (uint8_t *)ucode->fw->data +
+			   le32_to_cpu(comm_hdr->ucode_array_offset_bytes) +
+			   (le32_to_cpu(header->jt_offset) * 4);
+	memcpy(dst_addr, src_addr, le32_to_cpu(header->jt_size) * 4);
+
+	return 0;
+}
+
+
 int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
 {
 	struct amdgpu_bo **bo = &adev->firmware.fw_buf;
@@ -285,6 +310,13 @@  int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
 			header = (const struct common_firmware_header *)ucode->fw->data;
 			amdgpu_ucode_init_single_fw(ucode, fw_mc_addr + fw_offset,
 						    fw_buf_ptr + fw_offset);
+			if (i == AMDGPU_UCODE_ID_CP_MEC1) {
+				const struct gfx_firmware_header_v1_0 *cp_hdr;
+				cp_hdr = (const struct gfx_firmware_header_v1_0 *)ucode->fw->data;
+				amdgpu_ucode_patch_jt(ucode, fw_mc_addr + fw_offset,
+						    fw_buf_ptr + fw_offset);
+				fw_offset += ALIGN(le32_to_cpu(cp_hdr->jt_size) << 2, PAGE_SIZE);
+			}
 			fw_offset += ALIGN(le32_to_cpu(header->ucode_size_bytes), PAGE_SIZE);
 		}
 	}

Comments

On 09/28/2016 06:36 PM, Monk Liu wrote:
> we found some MEC ucode leads to IB test fail or even
> ring test fail if Jump Table of it is not start in
> FW bo with page aligned address, fixed by always make
> JT address page aligned.
> 
> we don't need to patch JT2 for MEC2, because for VI,
> MEC2 is a copy of MEC1, thus when converting fw_type
> for MEC_JT2 we just return MEC1,hw can use the same
> JT for both MEC1 & MEC2.
> 
> above two change fixed some ring/ib test failure issue
> for some version of MEC ucode.
> 
> Change-Id: Ie3b3c4c5722fdf68f64547cdfbf9c0d3274a2a15
> Signed-off-by: Frank Min <Frank.Min@amd.com>
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c   | 21 ++++++++++++++------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 32 +++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> index cb1ade1..7278898 100755
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> @@ -685,11 +685,14 @@ static uint32_t fw_type_convert(struct cgs_device *cgs_device, uint32_t fw_type)
>  		result = AMDGPU_UCODE_ID_CP_MEC1;
>  		break;
>  	case CGS_UCODE_ID_CP_MEC_JT2:
> -		if (adev->asic_type == CHIP_TONGA || adev->asic_type == CHIP_POLARIS11
> -		  || adev->asic_type == CHIP_POLARIS10)
> -			result = AMDGPU_UCODE_ID_CP_MEC2;
> -		else
> +		/* for VI. JT2 should be the same as JT1, because:
> +			1, MEC2 and MEC1 use exactly same FW.
> +			2, JT2 is not pached but JT1 is.
> +		*/
> +		if (adev->asic_type >= CHIP_TOPAZ)
>  			result = AMDGPU_UCODE_ID_CP_MEC1;
> +		else
> +			result = AMDGPU_UCODE_ID_CP_MEC2;
>  		break;
>  	case CGS_UCODE_ID_RLC_G:
>  		result = AMDGPU_UCODE_ID_RLC_G;
> @@ -779,12 +782,18 @@ static int amdgpu_cgs_get_firmware_info(struct cgs_device *cgs_device,
>  
>  		if ((type == CGS_UCODE_ID_CP_MEC_JT1) ||
>  		    (type == CGS_UCODE_ID_CP_MEC_JT2)) {
> -			gpu_addr += le32_to_cpu(header->jt_offset) << 2;
> +			gpu_addr += ALIGN(le32_to_cpu(header->header.ucode_size_bytes), PAGE_SIZE);
>  			data_size = le32_to_cpu(header->jt_size) << 2;
>  		}
> -		info->mc_addr = gpu_addr;
> +
> +		info->kptr = ucode->kaddr;
>  		info->image_size = data_size;
> +		info->mc_addr = gpu_addr;
>  		info->version = (uint16_t)le32_to_cpu(header->header.ucode_version);
> +
> +		if (CGS_UCODE_ID_CP_MEC == type)
> +			info->image_size = (header->jt_offset) << 2;
> +
>  		info->fw_version = amdgpu_get_firmware_version(cgs_device, type);
>  		info->feature_version = (uint16_t)le32_to_cpu(header->ucode_feature_version);
>  	} else {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index 06baac9..e2ea2c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -239,6 +239,31 @@ static int amdgpu_ucode_init_single_fw(struct amdgpu_firmware_info *ucode,
>  	return 0;
>  }
>  
> +static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode,
> +				uint64_t mc_addr, void *kptr)
> +{
> +	const struct gfx_firmware_header_v1_0 *header = NULL;
> +	const struct common_firmware_header *comm_hdr = NULL;
> +	uint8_t* src_addr = NULL;
> +	uint8_t* dst_addr = NULL;
> +
> +	if (NULL == ucode->fw)
Can be simplified to just:
+	if (!ucode->fw)

> +		return 0;
Do you really want to return 0 here? In fact, at the moment the return
value isn't used nor is it currently useful. Maybe just drop it.

> +
> +	comm_hdr = (const struct common_firmware_header *)ucode->fw->data;
> +	header = (const struct gfx_firmware_header_v1_0 *)ucode->fw->data;
> +	dst_addr = ucode->kaddr +
> +			   ALIGN(le32_to_cpu(comm_hdr->ucode_size_bytes),
> +			   PAGE_SIZE);
> +	src_addr = (uint8_t *)ucode->fw->data +
> +			   le32_to_cpu(comm_hdr->ucode_array_offset_bytes) +
> +			   (le32_to_cpu(header->jt_offset) * 4);
> +	memcpy(dst_addr, src_addr, le32_to_cpu(header->jt_size) * 4);
> +
> +	return 0;
> +}
> +
> +
>  int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
>  {
>  	struct amdgpu_bo **bo = &adev->firmware.fw_buf;
> @@ -285,6 +310,13 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
>  			header = (const struct common_firmware_header *)ucode->fw->data;
>  			amdgpu_ucode_init_single_fw(ucode, fw_mc_addr + fw_offset,
>  						    fw_buf_ptr + fw_offset);
> +			if (i == AMDGPU_UCODE_ID_CP_MEC1) {
> +				const struct gfx_firmware_header_v1_0 *cp_hdr;
> +				cp_hdr = (const struct gfx_firmware_header_v1_0 *)ucode->fw->data;
> +				amdgpu_ucode_patch_jt(ucode, fw_mc_addr + fw_offset,
> +						    fw_buf_ptr + fw_offset);
> +				fw_offset += ALIGN(le32_to_cpu(cp_hdr->jt_size) << 2, PAGE_SIZE);
> +			}
>  			fw_offset += ALIGN(le32_to_cpu(header->ucode_size_bytes), PAGE_SIZE);
>  		}
>  	}
>