Re: [PATCH] drm/amdgpu: Fix uvd firmware version information for vega20

2018-06-14 Thread Alex Deucher 
On Thu, Jun 14, 2018 at 12:36 PM, James Zhu  wrote:
> +Peter
>
>
>
> On 2018-06-14 11:51 AM, Leo Liu wrote:
>>
>>
>>
>> On 06/14/2018 11:29 AM, Alex Deucher wrote:
>>>
>>> The uvd version information was not set correctly for vega20.
>>> Rearrange the logic to set it correctly and fix the warnings
>>> as a result.
>>>
>>> Fixes: 7b3c773f405 (drm/amdgpu/vg20:support new UVD FW version naming
>>> convention)
>>> Signed-off-by: Alex Deucher 
>>> ---
>>>
>>> I'm not sure if this is exactly how we want to expose the new firmware
>>> versioning
>>> to userspace, but it's better than random stack data.
>>
>> The encode team like to have the origin number which they put to FW, back
>> to user space for chip beyond RV and Vega20.
>>
>>>
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 43
>>> +++--
>>>   1 file changed, 25 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>> index 630e273c16ce..dde0c4383014 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>> @@ -127,7 +127,7 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
>>>   unsigned long bo_size;
>>>   const char *fw_name;
>>>   const struct common_firmware_header *hdr;
>>> -unsigned version_major, version_minor, family_id;
>>> +unsigned family_id;
>>>   int i, j, r;
>>> INIT_DELAYED_WORK(>uvd.inst->idle_work,
>>> amdgpu_uvd_idle_work_handler);
>>> @@ -210,10 +210,31 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
>>>   family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
>>> if (adev->asic_type < CHIP_VEGA20) {
>>> +unsigned version_major, version_minor;
>>> +
>>>   version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
>>>   version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
>>>   DRM_INFO("Found UVD firmware Version: %hu.%hu Family ID:
>>> %hu\n",
>>>   version_major, version_minor, family_id);
>>> +
>>> +/*
>>> + * Limit the number of UVD handles depending on microcode major
>>> + * and minor versions. The firmware version which has 40 UVD
>>> + * instances support is 1.80. So all subsequent versions should
>>> + * also have the same support.
>>> + */
>>> +if ((version_major > 0x01) ||
>>> +((version_major == 0x01) && (version_minor >= 0x50)))
>>> +adev->uvd.max_handles = AMDGPU_MAX_UVD_HANDLES;
>>> +
>>> +adev->uvd.fw_version = ((version_major << 24) | (version_minor
>>> << 16) |
>>> +(family_id << 8));
>>> +
>>> +if ((adev->asic_type == CHIP_POLARIS10 ||
>>> + adev->asic_type == CHIP_POLARIS11) &&
>>> +(adev->uvd.fw_version < FW_1_66_16))
>>> +DRM_ERROR("POLARIS10/11 UVD firmware version %hu.%hu is too
>>> old.\n",
>>> +  version_major, version_minor);
>>>   } else {
>>>   unsigned int enc_major, enc_minor, dec_minor;
>>>   @@ -222,26 +243,12 @@ int amdgpu_uvd_sw_init(struct amdgpu_device
>>> *adev)
>>>   enc_major = (le32_to_cpu(hdr->ucode_version) >> 30) & 0x3;
>>>   DRM_INFO("Found UVD firmware ENC: %hu.%hu DEC: .%hu Family ID:
>>> %hu\n",
>>>   enc_major, enc_minor, dec_minor, family_id);
>>> -}
>>>   -/*
>>> - * Limit the number of UVD handles depending on microcode major
>>> - * and minor versions. The firmware version which has 40 UVD
>>> - * instances support is 1.80. So all subsequent versions should
>>> - * also have the same support.
>>> - */
>>> -if (adev->asic_type >= CHIP_VEGA20 || (version_major > 0x01) ||
>>> -((version_major == 0x01) && (version_minor >= 0x50)))
>>>   adev->uvd.max_handles = AMDGPU_MAX_UVD_HANDLES;
>>>   -adev->uvd.fw_version = ((version_major << 24) | (version_minor <<
>>> 16) |
>>> -(family_id << 8));
>>> -
>>> -if ((adev->asic_type == CHIP_POLARIS10 ||
>>> - adev->asic_type == CHIP_POLARIS11) &&
>>> -(adev->uvd.fw_version < FW_1_66_16))
>>> -DRM_ERROR("POLARIS10/11 UVD firmware version %hu.%hu is too
>>> old.\n",
>>> -  version_major, version_minor);
>>> +adev->uvd.fw_version = ((enc_major << 24) | (enc_minor << 16) |
>>> +(dec_minor << 8) | (family_id << 0));
>>
>> It should be:
>>
>> adev->uvd.fw_version = hdr->ucode_version;
>
> Hi Peter, Does user space need the whole ucode_version or just fields with
> enc/dec/familyID? James

We also export this info via debugfs for debugging to query what
firmware versions are loaded on each engine on the GPU.

Alex

>>
>>
>> With that fixed the patch is:
>>
>> Reviewed-by: Leo Liu 
>>
>>
>> Regards,
>> Leo
>>
>>
>>> +}
>>> bo_size = AMDGPU_UVD_STACK_SIZE + AMDGPU_UVD_HEAP_SIZE
>>> +  AMDGPU_UVD_SESSION_SIZE *

Re: [PATCH] drm/amdgpu: Fix uvd firmware version information for vega20

2018-06-14 Thread James Zhu 

+Peter


On 2018-06-14 11:51 AM, Leo Liu wrote:



On 06/14/2018 11:29 AM, Alex Deucher wrote:

The uvd version information was not set correctly for vega20.
Rearrange the logic to set it correctly and fix the warnings
as a result.

Fixes: 7b3c773f405 (drm/amdgpu/vg20:support new UVD FW version naming 
convention)

Signed-off-by: Alex Deucher 
---

I'm not sure if this is exactly how we want to expose the new 
firmware versioning

to userspace, but it's better than random stack data.
The encode team like to have the origin number which they put to FW, 
back to user space for chip beyond RV and Vega20.




  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 43 
+++--

  1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c

index 630e273c16ce..dde0c4383014 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -127,7 +127,7 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
  unsigned long bo_size;
  const char *fw_name;
  const struct common_firmware_header *hdr;
-    unsigned version_major, version_minor, family_id;
+    unsigned family_id;
  int i, j, r;
    INIT_DELAYED_WORK(>uvd.inst->idle_work, 
amdgpu_uvd_idle_work_handler);

@@ -210,10 +210,31 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
  family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
    if (adev->asic_type < CHIP_VEGA20) {
+    unsigned version_major, version_minor;
+
  version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 
0xff;

  version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
  DRM_INFO("Found UVD firmware Version: %hu.%hu Family ID: 
%hu\n",

  version_major, version_minor, family_id);
+
+    /*
+ * Limit the number of UVD handles depending on microcode major
+ * and minor versions. The firmware version which has 40 UVD
+ * instances support is 1.80. So all subsequent versions should
+ * also have the same support.
+ */
+    if ((version_major > 0x01) ||
+    ((version_major == 0x01) && (version_minor >= 0x50)))
+    adev->uvd.max_handles = AMDGPU_MAX_UVD_HANDLES;
+
+    adev->uvd.fw_version = ((version_major << 24) | 
(version_minor << 16) |

+    (family_id << 8));
+
+    if ((adev->asic_type == CHIP_POLARIS10 ||
+ adev->asic_type == CHIP_POLARIS11) &&
+    (adev->uvd.fw_version < FW_1_66_16))
+    DRM_ERROR("POLARIS10/11 UVD firmware version %hu.%hu is 
too old.\n",

+  version_major, version_minor);
  } else {
  unsigned int enc_major, enc_minor, dec_minor;
  @@ -222,26 +243,12 @@ int amdgpu_uvd_sw_init(struct amdgpu_device 
*adev)

  enc_major = (le32_to_cpu(hdr->ucode_version) >> 30) & 0x3;
  DRM_INFO("Found UVD firmware ENC: %hu.%hu DEC: .%hu Family 
ID: %hu\n",

  enc_major, enc_minor, dec_minor, family_id);
-    }
  -    /*
- * Limit the number of UVD handles depending on microcode major
- * and minor versions. The firmware version which has 40 UVD
- * instances support is 1.80. So all subsequent versions should
- * also have the same support.
- */
-    if (adev->asic_type >= CHIP_VEGA20 || (version_major > 0x01) ||
-    ((version_major == 0x01) && (version_minor >= 0x50)))
  adev->uvd.max_handles = AMDGPU_MAX_UVD_HANDLES;
  -    adev->uvd.fw_version = ((version_major << 24) | (version_minor 
<< 16) |

-    (family_id << 8));
-
-    if ((adev->asic_type == CHIP_POLARIS10 ||
- adev->asic_type == CHIP_POLARIS11) &&
-    (adev->uvd.fw_version < FW_1_66_16))
-    DRM_ERROR("POLARIS10/11 UVD firmware version %hu.%hu is too 
old.\n",

-  version_major, version_minor);
+    adev->uvd.fw_version = ((enc_major << 24) | (enc_minor << 16) |
+    (dec_minor << 8) | (family_id << 0));

It should be:

adev->uvd.fw_version = hdr->ucode_version;
Hi Peter, Does user space need the whole ucode_version or just fields 
with enc/dec/familyID? James


With that fixed the patch is:

Reviewed-by: Leo Liu 


Regards,
Leo



+    }
    bo_size = AMDGPU_UVD_STACK_SIZE + AMDGPU_UVD_HEAP_SIZE
    +  AMDGPU_UVD_SESSION_SIZE * adev->uvd.max_handles;


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


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

Re: [PATCH] drm/amdgpu: Fix uvd firmware version information for vega20

2018-06-14 Thread Leo Liu 



On 06/14/2018 11:29 AM, Alex Deucher wrote:

The uvd version information was not set correctly for vega20.
Rearrange the logic to set it correctly and fix the warnings
as a result.

Fixes: 7b3c773f405 (drm/amdgpu/vg20:support new UVD FW version naming 
convention)
Signed-off-by: Alex Deucher 
---

I'm not sure if this is exactly how we want to expose the new firmware 
versioning
to userspace, but it's better than random stack data.
The encode team like to have the origin number which they put to FW, 
back to user space for chip beyond RV and Vega20.




  drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 43 +++--
  1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index 630e273c16ce..dde0c4383014 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -127,7 +127,7 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
unsigned long bo_size;
const char *fw_name;
const struct common_firmware_header *hdr;
-   unsigned version_major, version_minor, family_id;
+   unsigned family_id;
int i, j, r;
  
  	INIT_DELAYED_WORK(>uvd.inst->idle_work, amdgpu_uvd_idle_work_handler);

@@ -210,10 +210,31 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
  
  	if (adev->asic_type < CHIP_VEGA20) {

+   unsigned version_major, version_minor;
+
version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
DRM_INFO("Found UVD firmware Version: %hu.%hu Family ID: %hu\n",
version_major, version_minor, family_id);
+
+   /*
+* Limit the number of UVD handles depending on microcode major
+* and minor versions. The firmware version which has 40 UVD
+* instances support is 1.80. So all subsequent versions should
+* also have the same support.
+*/
+   if ((version_major > 0x01) ||
+   ((version_major == 0x01) && (version_minor >= 0x50)))
+   adev->uvd.max_handles = AMDGPU_MAX_UVD_HANDLES;
+
+   adev->uvd.fw_version = ((version_major << 24) | (version_minor 
<< 16) |
+   (family_id << 8));
+
+   if ((adev->asic_type == CHIP_POLARIS10 ||
+adev->asic_type == CHIP_POLARIS11) &&
+   (adev->uvd.fw_version < FW_1_66_16))
+   DRM_ERROR("POLARIS10/11 UVD firmware version %hu.%hu is too 
old.\n",
+ version_major, version_minor);
} else {
unsigned int enc_major, enc_minor, dec_minor;
  
@@ -222,26 +243,12 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)

enc_major = (le32_to_cpu(hdr->ucode_version) >> 30) & 0x3;
DRM_INFO("Found UVD firmware ENC: %hu.%hu DEC: .%hu Family ID: 
%hu\n",
enc_major, enc_minor, dec_minor, family_id);
-   }
  
-	/*

-* Limit the number of UVD handles depending on microcode major
-* and minor versions. The firmware version which has 40 UVD
-* instances support is 1.80. So all subsequent versions should
-* also have the same support.
-*/
-   if (adev->asic_type >= CHIP_VEGA20 || (version_major > 0x01) ||
-   ((version_major == 0x01) && (version_minor >= 0x50)))
adev->uvd.max_handles = AMDGPU_MAX_UVD_HANDLES;
  
-	adev->uvd.fw_version = ((version_major << 24) | (version_minor << 16) |

-   (family_id << 8));
-
-   if ((adev->asic_type == CHIP_POLARIS10 ||
-adev->asic_type == CHIP_POLARIS11) &&
-   (adev->uvd.fw_version < FW_1_66_16))
-   DRM_ERROR("POLARIS10/11 UVD firmware version %hu.%hu is too 
old.\n",
- version_major, version_minor);
+   adev->uvd.fw_version = ((enc_major << 24) | (enc_minor << 16) |
+   (dec_minor << 8) | (family_id << 0));

It should be:

adev->uvd.fw_version = hdr->ucode_version;

With that fixed the patch is:

Reviewed-by: Leo Liu 


Regards,
Leo



+   }
  
  	bo_size = AMDGPU_UVD_STACK_SIZE + AMDGPU_UVD_HEAP_SIZE

  +  AMDGPU_UVD_SESSION_SIZE * adev->uvd.max_handles;


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