[PATCH 2/2] drm/amdgpu: support polaris10/11/12 new cp firmwares

Christian König deathsimple at vodafone.de
Mon Aug 21 11:25:05 UTC 2017


Am 21.08.2017 um 11:35 schrieb Quan, Evan:
> Hi Christian,
>
> On the 1st run, it goes for amdgpu/%s_mec2_2.bin. Then it will goes for amdgpu/%s_mec2.bin if failed.
> So, i did not see any problem with it.

Ah, now I see. The naming of the firmware files is a bit confusing, but 
should indeed work.

> Actually i talked with firmware guys. There is no change for the firmware loading way.
> The new firmwares depends on a critical feature(unhalt MEC) which we lost before. We added that feature.
> But considering the possible use case: old kernel + new firmwares, we decide to give the new firmwares other names.

Makes sense. In this case the patch is Acked-by: Christian König 
<christian.koenig at amd.com>.

Regards,
Christian.

>
> Regards,
> Evan
>> -----Original Message-----
>> From: Christian König [mailto:deathsimple at vodafone.de]
>> Sent: Monday, August 21, 2017 4:18 PM
>> To: Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
>> Subject: Re: [PATCH 2/2] drm/amdgpu: support polaris10/11/12 new cp firmwares
>>
>> Am 21.08.2017 um 03:39 schrieb Evan Quan:
>>> Newer versions of the CP firmware require changes in how the driver
>>> initializes the hw block.
>>> Change the firmware name for new firmware to maintain compatibility with
>>> older kernels.
>>>
>>> Change-Id: I32e3382768a2d9ff1e2978bcadb3bd44afb3db01
>>> Signed-off-by: Evan Quan <evan.quan at amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 65 +++++++++++++++++++++++++++++-
>> -----
>>>    1 file changed, 55 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> index 38f70a1..6f2901a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> @@ -918,8 +918,17 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device *adev)
>>>    		BUG();
>>>    	}
>>>
>>> -	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_pfp.bin", chip_name);
>>> -	err = request_firmware(&adev->gfx.pfp_fw, fw_name, adev->dev);
>>> +	if (adev->asic_type >= CHIP_POLARIS10 && adev->asic_type <= CHIP_POLARIS12)
>> {
>>> +		snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_pfp_2.bin", chip_name);
>>> +		err = request_firmware(&adev->gfx.pfp_fw, fw_name, adev->dev);
>>> +		if (err == -ENOENT) {
>>> +			snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_pfp.bin",
>> chip_name);
>>> +			err = request_firmware(&adev->gfx.pfp_fw, fw_name, adev->dev);
>>> +		}
>>> +	} else {
>>> +		snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_pfp.bin", chip_name);
>>> +		err = request_firmware(&adev->gfx.pfp_fw, fw_name, adev->dev);
>>> +	}
>>>    	if (err)
>>>    		goto out;
>>>    	err = amdgpu_ucode_validate(adev->gfx.pfp_fw);
>>> @@ -929,8 +938,17 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device *adev)
>>>    	adev->gfx.pfp_fw_version = le32_to_cpu(cp_hdr->header.ucode_version);
>>>    	adev->gfx.pfp_feature_version = le32_to_cpu(cp_hdr->ucode_feature_version);
>>>
>>> -	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_me.bin", chip_name);
>>> -	err = request_firmware(&adev->gfx.me_fw, fw_name, adev->dev);
>>> +	if (adev->asic_type >= CHIP_POLARIS10 && adev->asic_type <= CHIP_POLARIS12)
>> {
>>> +		snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_me_2.bin", chip_name);
>>> +		err = request_firmware(&adev->gfx.me_fw, fw_name, adev->dev);
>>> +		if (err == -ENOENT) {
>>> +			snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_me.bin",
>> chip_name);
>>> +			err = request_firmware(&adev->gfx.me_fw, fw_name, adev->dev);
>>> +		}
>>> +	} else {
>>> +		snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_me.bin", chip_name);
>>> +		err = request_firmware(&adev->gfx.me_fw, fw_name, adev->dev);
>>> +	}
>>>    	if (err)
>>>    		goto out;
>>>    	err = amdgpu_ucode_validate(adev->gfx.me_fw);
>>> @@ -941,8 +959,17 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device *adev)
>>>
>>>    	adev->gfx.me_feature_version = le32_to_cpu(cp_hdr->ucode_feature_version);
>>>
>>> -	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ce.bin", chip_name);
>>> -	err = request_firmware(&adev->gfx.ce_fw, fw_name, adev->dev);
>>> +	if (adev->asic_type >= CHIP_POLARIS10 && adev->asic_type <= CHIP_POLARIS12)
>> {
>>> +		snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ce_2.bin", chip_name);
>>> +		err = request_firmware(&adev->gfx.ce_fw, fw_name, adev->dev);
>>> +		if (err == -ENOENT) {
>>> +			snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ce.bin",
>> chip_name);
>>> +			err = request_firmware(&adev->gfx.ce_fw, fw_name, adev->dev);
>>> +		}
>>> +	} else {
>>> +		snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ce.bin", chip_name);
>>> +		err = request_firmware(&adev->gfx.ce_fw, fw_name, adev->dev);
>>> +	}
>>>    	if (err)
>>>    		goto out;
>>>    	err = amdgpu_ucode_validate(adev->gfx.ce_fw);
>>> @@ -1012,8 +1039,17 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device
>> *adev)
>>>    	for (i = 0 ; i < (rlc_hdr->reg_list_size_bytes >> 2); i++)
>>>    		adev->gfx.rlc.register_restore[i] = le32_to_cpu(tmp[i]);
>>>
>>> -	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mec.bin", chip_name);
>>> -	err = request_firmware(&adev->gfx.mec_fw, fw_name, adev->dev);
>>> +	if (adev->asic_type >= CHIP_POLARIS10 && adev->asic_type <= CHIP_POLARIS12)
>> {
>>> +		snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mec_2.bin", chip_name);
>>> +		err = request_firmware(&adev->gfx.mec_fw, fw_name, adev->dev);
>>> +		if (err == -ENOENT) {
>>> +			snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mec.bin",
>> chip_name);
>>> +			err = request_firmware(&adev->gfx.mec_fw, fw_name, adev->dev);
>>> +		}
>>> +	} else {
>>> +		snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mec.bin", chip_name);
>>> +		err = request_firmware(&adev->gfx.mec_fw, fw_name, adev->dev);
>>> +	}
>>>    	if (err)
>>>    		goto out;
>>>    	err = amdgpu_ucode_validate(adev->gfx.mec_fw);
>>> @@ -1025,8 +1061,17 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device
>> *adev)
>>>    	if ((adev->asic_type != CHIP_STONEY) &&
>>>    	    (adev->asic_type != CHIP_TOPAZ)) {
>>> -		snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mec2.bin", chip_name);
>>> -		err = request_firmware(&adev->gfx.mec2_fw, fw_name, adev->dev);
>>> +		if (adev->asic_type >= CHIP_POLARIS10 && adev->asic_type <=
>> CHIP_POLARIS12) {
>>> +			snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mec2_2.bin",
>> chip_name);
>>> +			err = request_firmware(&adev->gfx.mec2_fw, fw_name, adev->dev);
>>> +			if (err == -ENOENT) {
>>> +				snprintf(fw_name, sizeof(fw_name),
>> "amdgpu/%s_mec2.bin", chip_name);
>>> +				err = request_firmware(&adev->gfx.mec2_fw, fw_name,
>> adev->dev);
>>
>> That chunk above looks incorrect to me. You just query
>> "amdgpu/%s_mec2.bin" twice.
>>
>> Apart from that do we really need that? Couldn't we just push back on
>> the firmware guys to start the MEC after loading like in older versions?
>>
>> Regards,
>> Christian.
>>
>>> +			}
>>> +		} else {
>>> +			snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_mec2.bin",
>> chip_name);
>>> +			err = request_firmware(&adev->gfx.mec2_fw, fw_name, adev->dev);
>>> +		}
>>>    		if (!err) {
>>>    			err = amdgpu_ucode_validate(adev->gfx.mec2_fw);
>>>    			if (err)




More information about the amd-gfx mailing list