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

[Edward O'Callaghan <funfunctor-dczkZgxz+BNUPWh3PAxdjQ@public.gmane.org>]

[-- Attachment #1.1.1: Type: text/plain, Size: 5032 bytes --]



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-5C7GfCeVMHo@public.gmane.org>
> Signed-off-by: Monk Liu <Monk.Liu-5C7GfCeVMHo@public.gmane.org>
> ---
>  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);
>  		}
>  	}
> 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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