[1/2] drm/amdgpu: Remove the sriov checking and add firmware checking

Submitted by Deng, Emily on Aug. 16, 2018, 1:05 p.m.

Details

Message ID 1534424701-6723-1-git-send-email-Emily.Deng@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

Deng, Emily Aug. 16, 2018, 1:05 p.m.
Unify bare metal and sriov, and add firmware checking for
reg write and reg wait unify command.

Signed-off-by: Emily Deng <Emily.Deng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |  2 ++
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 47 ++++++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 53e9e2a..f172e92 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -274,6 +274,8 @@  struct amdgpu_gfx {
 	uint32_t			rlc_srls_feature_version;
 	uint32_t			mec_feature_version;
 	uint32_t			mec2_feature_version;
+	bool				mec_fw_write_wait;
+	bool				me_fw_write_wait;
 	struct amdgpu_ring		gfx_ring[AMDGPU_MAX_GFX_RINGS];
 	unsigned			num_gfx_rings;
 	struct amdgpu_ring		compute_ring[AMDGPU_MAX_COMPUTE_RINGS];
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 76d979e..76e8973 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -482,6 +482,47 @@  static void gfx_v9_0_init_rlc_ext_microcode(struct amdgpu_device *adev)
 			le32_to_cpu(rlc_hdr->reg_list_format_direct_reg_list_length);
 }
 
+static void gfx_v9_0_check_fw_write_wait(struct amdgpu_device *adev)
+{
+	adev->gfx.me_fw_write_wait = false;
+	adev->gfx.mec_fw_write_wait = false;
+
+	switch (adev->asic_type) {
+	case CHIP_VEGA10:
+		if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev->gfx.me_feature_version >= 42) &&
+			(adev->gfx.pfp_fw_version >=  0x000000b1) && (adev->gfx.pfp_feature_version >= 42))
+				adev->gfx.me_fw_write_wait = true;
+
+		if ((adev->gfx.mec_fw_version >=  0x00000193) && (adev->gfx.mec_feature_version >= 42))
+				adev->gfx.mec_fw_write_wait = true;
+		break;
+	case CHIP_VEGA12:
+		if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev->gfx.me_feature_version >= 44) &&
+			(adev->gfx.pfp_fw_version >=  0x000000b2) && (adev->gfx.pfp_feature_version >= 44))
+				adev->gfx.me_fw_write_wait = true;
+
+		if ((adev->gfx.mec_fw_version >=  0x00000196) && (adev->gfx.mec_feature_version >= 44))
+				adev->gfx.mec_fw_write_wait = true;
+		break;
+	case CHIP_VEGA20:
+		if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev->gfx.me_feature_version >= 44) &&
+			(adev->gfx.pfp_fw_version >=  0x000000b2) && (adev->gfx.pfp_feature_version >= 44))
+				adev->gfx.me_fw_write_wait = true;
+
+		if ((adev->gfx.mec_fw_version >=  0x00000197) && (adev->gfx.mec_feature_version >= 44))
+				adev->gfx.mec_fw_write_wait = true;
+		break;
+	case CHIP_RAVEN:
+		if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev->gfx.me_feature_version >= 42) &&
+			(adev->gfx.pfp_fw_version >=  0x000000b1) && (adev->gfx.pfp_feature_version >= 42))
+				adev->gfx.me_fw_write_wait = true;
+
+		if ((adev->gfx.mec_fw_version >=  0x00000192) && (adev->gfx.mec_feature_version >= 42))
+				adev->gfx.mec_fw_write_wait = true;
+		break;
+	}
+}
+
 static int gfx_v9_0_init_microcode(struct amdgpu_device *adev)
 {
 	const char *chip_name;
@@ -716,6 +757,7 @@  static int gfx_v9_0_init_microcode(struct amdgpu_device *adev)
 	}
 
 out:
+	gfx_v9_0_check_fw_write_wait(adev);
 	if (err) {
 		dev_err(adev->dev,
 			"gfx9: Failed to load firmware \"%s\"\n",
@@ -4348,8 +4390,11 @@  static void gfx_v9_0_ring_emit_reg_write_reg_wait(struct amdgpu_ring *ring,
 						  uint32_t ref, uint32_t mask)
 {
 	int usepfp = (ring->funcs->type == AMDGPU_RING_TYPE_GFX);
+	struct amdgpu_device *adev = ring->adev;
+	bool fw_version_ok = (ring->funcs->type == AMDGPU_RING_TYPE_GFX) ?
+		adev->gfx.me_fw_write_wait : adev->gfx.mec_fw_write_wait;
 
-	if (amdgpu_sriov_vf(ring->adev))
+	if (fw_version_ok)
 		gfx_v9_0_wait_reg_mem(ring, usepfp, 0, 1, reg0, reg1,
 				      ref, mask, 0x20);
 	else

Comments

Am 16.08.2018 um 15:05 schrieb Emily Deng:
> Unify bare metal and sriov, and add firmware checking for
> reg write and reg wait unify command.
>
> Signed-off-by: Emily Deng <Emily.Deng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |  2 ++
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 47 ++++++++++++++++++++++++++++++++-
>   2 files changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 53e9e2a..f172e92 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -274,6 +274,8 @@ struct amdgpu_gfx {
>   	uint32_t			rlc_srls_feature_version;
>   	uint32_t			mec_feature_version;
>   	uint32_t			mec2_feature_version;
> +	bool				mec_fw_write_wait;
> +	bool				me_fw_write_wait;
>   	struct amdgpu_ring		gfx_ring[AMDGPU_MAX_GFX_RINGS];
>   	unsigned			num_gfx_rings;
>   	struct amdgpu_ring		compute_ring[AMDGPU_MAX_COMPUTE_RINGS];
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 76d979e..76e8973 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -482,6 +482,47 @@ static void gfx_v9_0_init_rlc_ext_microcode(struct amdgpu_device *adev)
>   			le32_to_cpu(rlc_hdr->reg_list_format_direct_reg_list_length);
>   }
>   
> +static void gfx_v9_0_check_fw_write_wait(struct amdgpu_device *adev)
> +{
> +	adev->gfx.me_fw_write_wait = false;
> +	adev->gfx.mec_fw_write_wait = false;
> +
> +	switch (adev->asic_type) {
> +	case CHIP_VEGA10:
> +		if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev->gfx.me_feature_version >= 42) &&
> +			(adev->gfx.pfp_fw_version >=  0x000000b1) && (adev->gfx.pfp_feature_version >= 42))
> +				adev->gfx.me_fw_write_wait = true;

Well that looks like the correct solution to me. Only a nit pick left to 
fix, the coding style looks a bit off.

For example the "if" should be indented so that "(adev.." in both lines 
is on the same column:

if ((adev.... ) &&
     (adev....

And the "adev->gfx.me_fw_write_wait = true;" is to far to the right.

With that fixed the patch is Acked-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.

> +
> +		if ((adev->gfx.mec_fw_version >=  0x00000193) && (adev->gfx.mec_feature_version >= 42))
> +				adev->gfx.mec_fw_write_wait = true;
> +		break;
> +	case CHIP_VEGA12:
> +		if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev->gfx.me_feature_version >= 44) &&
> +			(adev->gfx.pfp_fw_version >=  0x000000b2) && (adev->gfx.pfp_feature_version >= 44))
> +				adev->gfx.me_fw_write_wait = true;
> +
> +		if ((adev->gfx.mec_fw_version >=  0x00000196) && (adev->gfx.mec_feature_version >= 44))
> +				adev->gfx.mec_fw_write_wait = true;
> +		break;
> +	case CHIP_VEGA20:
> +		if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev->gfx.me_feature_version >= 44) &&
> +			(adev->gfx.pfp_fw_version >=  0x000000b2) && (adev->gfx.pfp_feature_version >= 44))
> +				adev->gfx.me_fw_write_wait = true;
> +
> +		if ((adev->gfx.mec_fw_version >=  0x00000197) && (adev->gfx.mec_feature_version >= 44))
> +				adev->gfx.mec_fw_write_wait = true;
> +		break;
> +	case CHIP_RAVEN:
> +		if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev->gfx.me_feature_version >= 42) &&
> +			(adev->gfx.pfp_fw_version >=  0x000000b1) && (adev->gfx.pfp_feature_version >= 42))
> +				adev->gfx.me_fw_write_wait = true;
> +
> +		if ((adev->gfx.mec_fw_version >=  0x00000192) && (adev->gfx.mec_feature_version >= 42))
> +				adev->gfx.mec_fw_write_wait = true;
> +		break;
> +	}
> +}
> +
>   static int gfx_v9_0_init_microcode(struct amdgpu_device *adev)
>   {
>   	const char *chip_name;
> @@ -716,6 +757,7 @@ static int gfx_v9_0_init_microcode(struct amdgpu_device *adev)
>   	}
>   
>   out:
> +	gfx_v9_0_check_fw_write_wait(adev);
>   	if (err) {
>   		dev_err(adev->dev,
>   			"gfx9: Failed to load firmware \"%s\"\n",
> @@ -4348,8 +4390,11 @@ static void gfx_v9_0_ring_emit_reg_write_reg_wait(struct amdgpu_ring *ring,
>   						  uint32_t ref, uint32_t mask)
>   {
>   	int usepfp = (ring->funcs->type == AMDGPU_RING_TYPE_GFX);
> +	struct amdgpu_device *adev = ring->adev;
> +	bool fw_version_ok = (ring->funcs->type == AMDGPU_RING_TYPE_GFX) ?
> +		adev->gfx.me_fw_write_wait : adev->gfx.mec_fw_write_wait;
>   
> -	if (amdgpu_sriov_vf(ring->adev))
> +	if (fw_version_ok)
>   		gfx_v9_0_wait_reg_mem(ring, usepfp, 0, 1, reg0, reg1,
>   				      ref, mask, 0x20);
>   	else
>-----Original Message-----

>From: Christian König <ckoenig.leichtzumerken@gmail.com>

>Sent: Thursday, August 16, 2018 9:17 PM

>To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@lists.freedesktop.org

>Subject: Re: [PATCH 1/2] drm/amdgpu: Remove the sriov checking and add

>firmware checking

>

>Am 16.08.2018 um 15:05 schrieb Emily Deng:

>> Unify bare metal and sriov, and add firmware checking for reg write

>> and reg wait unify command.

>>

>> Signed-off-by: Emily Deng <Emily.Deng@amd.com>

>> ---

>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |  2 ++

>>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 47

>++++++++++++++++++++++++++++++++-

>>   2 files changed, 48 insertions(+), 1 deletion(-)

>>

>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h

>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h

>> index 53e9e2a..f172e92 100644

>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h

>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h

>> @@ -274,6 +274,8 @@ struct amdgpu_gfx {

>>   	uint32_t			rlc_srls_feature_version;

>>   	uint32_t			mec_feature_version;

>>   	uint32_t			mec2_feature_version;

>> +	bool				mec_fw_write_wait;

>> +	bool				me_fw_write_wait;

>>   	struct amdgpu_ring		gfx_ring[AMDGPU_MAX_GFX_RINGS];

>>   	unsigned			num_gfx_rings;

>>   	struct amdgpu_ring

>	compute_ring[AMDGPU_MAX_COMPUTE_RINGS];

>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c

>> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c

>> index 76d979e..76e8973 100644

>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c

>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c

>> @@ -482,6 +482,47 @@ static void gfx_v9_0_init_rlc_ext_microcode(struct

>amdgpu_device *adev)

>>   			le32_to_cpu(rlc_hdr-

>>reg_list_format_direct_reg_list_length);

>>   }

>>

>> +static void gfx_v9_0_check_fw_write_wait(struct amdgpu_device *adev)

>> +{

>> +	adev->gfx.me_fw_write_wait = false;

>> +	adev->gfx.mec_fw_write_wait = false;

>> +

>> +	switch (adev->asic_type) {

>> +	case CHIP_VEGA10:

>> +		if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev-

>>gfx.me_feature_version >= 42) &&

>> +			(adev->gfx.pfp_fw_version >=  0x000000b1) && (adev-

>>gfx.pfp_feature_version >= 42))

>> +				adev->gfx.me_fw_write_wait = true;

>

>Well that looks like the correct solution to me. Only a nit pick left to fix, the

>coding style looks a bit off.

>

>For example the "if" should be indented so that "(adev.." in both lines is on the

>same column:

>

>if ((adev.... ) &&

>     (adev....

>

>And the "adev->gfx.me_fw_write_wait = true;" is to far to the right.

>

Thanks, will modify as your suggestion.
>With that fixed the patch is Acked-by: Christian König

><christian.koenig@amd.com>.

>

>Regards,

>Christian.

>

>> +

>> +		if ((adev->gfx.mec_fw_version >=  0x00000193) && (adev-

>>gfx.mec_feature_version >= 42))

>> +				adev->gfx.mec_fw_write_wait = true;

>> +		break;

>> +	case CHIP_VEGA12:

>> +		if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev-

>>gfx.me_feature_version >= 44) &&

>> +			(adev->gfx.pfp_fw_version >=  0x000000b2) && (adev-

>>gfx.pfp_feature_version >= 44))

>> +				adev->gfx.me_fw_write_wait = true;

>> +

>> +		if ((adev->gfx.mec_fw_version >=  0x00000196) && (adev-

>>gfx.mec_feature_version >= 44))

>> +				adev->gfx.mec_fw_write_wait = true;

>> +		break;

>> +	case CHIP_VEGA20:

>> +		if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev-

>>gfx.me_feature_version >= 44) &&

>> +			(adev->gfx.pfp_fw_version >=  0x000000b2) && (adev-

>>gfx.pfp_feature_version >= 44))

>> +				adev->gfx.me_fw_write_wait = true;

>> +

>> +		if ((adev->gfx.mec_fw_version >=  0x00000197) && (adev-

>>gfx.mec_feature_version >= 44))

>> +				adev->gfx.mec_fw_write_wait = true;

>> +		break;

>> +	case CHIP_RAVEN:

>> +		if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev-

>>gfx.me_feature_version >= 42) &&

>> +			(adev->gfx.pfp_fw_version >=  0x000000b1) && (adev-

>>gfx.pfp_feature_version >= 42))

>> +				adev->gfx.me_fw_write_wait = true;

>> +

>> +		if ((adev->gfx.mec_fw_version >=  0x00000192) && (adev-

>>gfx.mec_feature_version >= 42))

>> +				adev->gfx.mec_fw_write_wait = true;

>> +		break;

>> +	}

>> +}

>> +

>>   static int gfx_v9_0_init_microcode(struct amdgpu_device *adev)

>>   {

>>   	const char *chip_name;

>> @@ -716,6 +757,7 @@ static int gfx_v9_0_init_microcode(struct

>amdgpu_device *adev)

>>   	}

>>

>>   out:

>> +	gfx_v9_0_check_fw_write_wait(adev);

>>   	if (err) {

>>   		dev_err(adev->dev,

>>   			"gfx9: Failed to load firmware \"%s\"\n", @@ -4348,8

>+4390,11 @@

>> static void gfx_v9_0_ring_emit_reg_write_reg_wait(struct amdgpu_ring *ring,

>>   						  uint32_t ref, uint32_t mask)

>>   {

>>   	int usepfp = (ring->funcs->type == AMDGPU_RING_TYPE_GFX);

>> +	struct amdgpu_device *adev = ring->adev;

>> +	bool fw_version_ok = (ring->funcs->type == AMDGPU_RING_TYPE_GFX) ?

>> +		adev->gfx.me_fw_write_wait : adev->gfx.mec_fw_write_wait;

>>

>> -	if (amdgpu_sriov_vf(ring->adev))

>> +	if (fw_version_ok)

>>   		gfx_v9_0_wait_reg_mem(ring, usepfp, 0, 1, reg0, reg1,

>>   				      ref, mask, 0x20);

>>   	else