* [PATCH 1/2] drm/amdgpu: Pass IP instance/hwid as parameters
@ 2025-01-28 6:31 Lijo Lazar
2025-01-28 6:31 ` [PATCH 2/2] drm/amdgpu: Use version to figure out harvest info Lijo Lazar
0 siblings, 1 reply; 4+ messages in thread
From: Lijo Lazar @ 2025-01-28 6:31 UTC (permalink / raw)
To: amd-gfx; +Cc: Hawking.Zhang, Alexander.Deucher, Asad.Kamal, Le.Ma
Use IP instance number and hwid as function args for validation checks.
Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 42 ++++++++++++-------
1 file changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index fbe1e23526f0..d34b97a081d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -587,16 +587,19 @@ void amdgpu_discovery_fini(struct amdgpu_device *adev)
adev->mman.discovery_bin = NULL;
}
-static int amdgpu_discovery_validate_ip(const struct ip_v4 *ip)
+static int amdgpu_discovery_validate_ip(struct amdgpu_device *adev,
+ uint8_t instance, uint16_t hw_id)
{
- if (ip->instance_number >= HWIP_MAX_INSTANCE) {
- DRM_ERROR("Unexpected instance_number (%d) from ip discovery blob\n",
- ip->instance_number);
+ if (instance >= HWIP_MAX_INSTANCE) {
+ dev_err(adev->dev,
+ "Unexpected instance_number (%d) from ip discovery blob\n",
+ instance);
return -EINVAL;
}
- if (le16_to_cpu(ip->hw_id) >= HW_ID_MAX) {
- DRM_ERROR("Unexpected hw_id (%d) from ip discovery blob\n",
- le16_to_cpu(ip->hw_id));
+ if (hw_id >= HW_ID_MAX) {
+ dev_err(adev->dev,
+ "Unexpected hw_id (%d) from ip discovery blob\n",
+ hw_id);
return -EINVAL;
}
@@ -611,6 +614,8 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
struct die_header *dhdr;
struct ip_v4 *ip;
uint16_t die_offset, ip_offset, num_dies, num_ips;
+ uint16_t hw_id;
+ uint8_t inst;
int i, j;
bhdr = (struct binary_header *)adev->mman.discovery_bin;
@@ -627,15 +632,16 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
for (j = 0; j < num_ips; j++) {
ip = (struct ip_v4 *)(adev->mman.discovery_bin + ip_offset);
-
- if (amdgpu_discovery_validate_ip(ip))
+ inst = ip->instance_number;
+ hw_id = le16_to_cpu(ip->hw_id);
+ if (amdgpu_discovery_validate_ip(adev, inst, hw_id))
goto next_ip;
if (le16_to_cpu(ip->variant) == 1) {
- switch (le16_to_cpu(ip->hw_id)) {
+ switch (hw_id) {
case VCN_HWID:
(*vcn_harvest_count)++;
- if (ip->instance_number == 0) {
+ if (inst == 0) {
adev->vcn.harvest_config |= AMDGPU_VCN_HARVEST_VCN0;
adev->vcn.inst_mask &=
~AMDGPU_VCN_HARVEST_VCN0;
@@ -1019,6 +1025,8 @@ static int amdgpu_discovery_sysfs_ips(struct amdgpu_device *adev,
bool reg_base_64)
{
int ii, jj, kk, res;
+ uint16_t hw_id;
+ uint8_t inst;
DRM_DEBUG("num_ips:%d", num_ips);
@@ -1034,8 +1042,10 @@ static int amdgpu_discovery_sysfs_ips(struct amdgpu_device *adev,
struct ip_hw_instance *ip_hw_instance;
ip = (struct ip_v4 *)(adev->mman.discovery_bin + ip_offset);
- if (amdgpu_discovery_validate_ip(ip) ||
- le16_to_cpu(ip->hw_id) != ii)
+ inst = ip->instance_number;
+ hw_id = le16_to_cpu(ip->hw_id);
+ if (amdgpu_discovery_validate_ip(adev, inst, hw_id) ||
+ hw_id != ii)
goto next_ip;
DRM_DEBUG("match:%d @ ip_offset:%zu", ii, ip_offset);
@@ -1282,6 +1292,8 @@ static int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev)
uint16_t ip_offset;
uint16_t num_dies;
uint16_t num_ips;
+ uint16_t hw_id;
+ uint8_t inst;
int hw_ip;
int i, j, k;
int r;
@@ -1321,7 +1333,9 @@ static int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev)
for (j = 0; j < num_ips; j++) {
ip = (struct ip_v4 *)(adev->mman.discovery_bin + ip_offset);
- if (amdgpu_discovery_validate_ip(ip))
+ inst = ip->instance_number;
+ hw_id = le16_to_cpu(ip->hw_id);
+ if (amdgpu_discovery_validate_ip(adev, inst, hw_id))
goto next_ip;
num_base_address = ip->num_base_address;
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] drm/amdgpu: Use version to figure out harvest info
2025-01-28 6:31 [PATCH 1/2] drm/amdgpu: Pass IP instance/hwid as parameters Lijo Lazar
@ 2025-01-28 6:31 ` Lijo Lazar
2025-01-28 15:31 ` Alex Deucher
0 siblings, 1 reply; 4+ messages in thread
From: Lijo Lazar @ 2025-01-28 6:31 UTC (permalink / raw)
To: amd-gfx; +Cc: Hawking.Zhang, Alexander.Deucher, Asad.Kamal, Le.Ma
IP tables with version <=2 may use harvest bit. For version 3 and above,
harvest bit is not applicable, instead uses harvest table. Fix the
logic accordingly.
Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 27 +++++++++++--------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index d34b97a081d8..e3afdf933dc8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -612,7 +612,7 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
struct binary_header *bhdr;
struct ip_discovery_header *ihdr;
struct die_header *dhdr;
- struct ip_v4 *ip;
+ struct ip *ip;
uint16_t die_offset, ip_offset, num_dies, num_ips;
uint16_t hw_id;
uint8_t inst;
@@ -631,13 +631,14 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
ip_offset = die_offset + sizeof(*dhdr);
for (j = 0; j < num_ips; j++) {
- ip = (struct ip_v4 *)(adev->mman.discovery_bin + ip_offset);
- inst = ip->instance_number;
+ ip = (struct ip *)(adev->mman.discovery_bin +
+ ip_offset);
+ inst = ip->number_instance;
hw_id = le16_to_cpu(ip->hw_id);
if (amdgpu_discovery_validate_ip(adev, inst, hw_id))
goto next_ip;
- if (le16_to_cpu(ip->variant) == 1) {
+ if (le16_to_cpu(ip->harvest) == 1) {
switch (hw_id) {
case VCN_HWID:
(*vcn_harvest_count)++;
@@ -663,10 +664,8 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
}
}
next_ip:
- if (ihdr->base_addr_64_bit)
- ip_offset += struct_size(ip, base_address_64, ip->num_base_address);
- else
- ip_offset += struct_size(ip, base_address, ip->num_base_address);
+ ip_offset += struct_size(ip, base_address,
+ ip->num_base_address);
}
}
}
@@ -1474,18 +1473,24 @@ static int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev)
static void amdgpu_discovery_harvest_ip(struct amdgpu_device *adev)
{
+ struct ip_discovery_header *ihdr;
+ struct binary_header *bhdr;
int vcn_harvest_count = 0;
int umc_harvest_count = 0;
+ uint16_t offset, ihdr_ver;
+ bhdr = (struct binary_header *)adev->mman.discovery_bin;
+ offset = le16_to_cpu(bhdr->table_list[IP_DISCOVERY].offset);
+ ihdr = (struct ip_discovery_header *)(adev->mman.discovery_bin +
+ offset);
+ ihdr_ver = le16_to_cpu(ihdr->version);
/*
* Harvest table does not fit Navi1x and legacy GPUs,
* so read harvest bit per IP data structure to set
* harvest configuration.
*/
if (amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(10, 2, 0) &&
- amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(9, 4, 3) &&
- amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(9, 4, 4) &&
- amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(9, 5, 0)) {
+ ihdr_ver <= 2) {
if ((adev->pdev->device == 0x731E &&
(adev->pdev->revision == 0xC6 ||
adev->pdev->revision == 0xC7)) ||
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: Use version to figure out harvest info
2025-01-28 6:31 ` [PATCH 2/2] drm/amdgpu: Use version to figure out harvest info Lijo Lazar
@ 2025-01-28 15:31 ` Alex Deucher
2025-01-29 3:53 ` Lazar, Lijo
0 siblings, 1 reply; 4+ messages in thread
From: Alex Deucher @ 2025-01-28 15:31 UTC (permalink / raw)
To: Lijo Lazar; +Cc: amd-gfx, Hawking.Zhang, Alexander.Deucher, Asad.Kamal, Le.Ma
On Tue, Jan 28, 2025 at 1:42 AM Lijo Lazar <lijo.lazar@amd.com> wrote:
>
> IP tables with version <=2 may use harvest bit. For version 3 and above,
> harvest bit is not applicable, instead uses harvest table. Fix the
> logic accordingly.
>
> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 27 +++++++++++--------
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index d34b97a081d8..e3afdf933dc8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -612,7 +612,7 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
> struct binary_header *bhdr;
> struct ip_discovery_header *ihdr;
> struct die_header *dhdr;
> - struct ip_v4 *ip;
> + struct ip *ip;
> uint16_t die_offset, ip_offset, num_dies, num_ips;
> uint16_t hw_id;
> uint8_t inst;
> @@ -631,13 +631,14 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
> ip_offset = die_offset + sizeof(*dhdr);
>
> for (j = 0; j < num_ips; j++) {
> - ip = (struct ip_v4 *)(adev->mman.discovery_bin + ip_offset);
> - inst = ip->instance_number;
> + ip = (struct ip *)(adev->mman.discovery_bin +
> + ip_offset);
> + inst = ip->number_instance;
> hw_id = le16_to_cpu(ip->hw_id);
> if (amdgpu_discovery_validate_ip(adev, inst, hw_id))
> goto next_ip;
>
> - if (le16_to_cpu(ip->variant) == 1) {
> + if (le16_to_cpu(ip->harvest) == 1) {
ip->harvest is a uint8_t so it doesn't need byte swapping. Other than
that, the series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> switch (hw_id) {
> case VCN_HWID:
> (*vcn_harvest_count)++;
> @@ -663,10 +664,8 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
> }
> }
> next_ip:
> - if (ihdr->base_addr_64_bit)
> - ip_offset += struct_size(ip, base_address_64, ip->num_base_address);
> - else
> - ip_offset += struct_size(ip, base_address, ip->num_base_address);
> + ip_offset += struct_size(ip, base_address,
> + ip->num_base_address);
> }
> }
> }
> @@ -1474,18 +1473,24 @@ static int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev)
>
> static void amdgpu_discovery_harvest_ip(struct amdgpu_device *adev)
> {
> + struct ip_discovery_header *ihdr;
> + struct binary_header *bhdr;
> int vcn_harvest_count = 0;
> int umc_harvest_count = 0;
> + uint16_t offset, ihdr_ver;
>
> + bhdr = (struct binary_header *)adev->mman.discovery_bin;
> + offset = le16_to_cpu(bhdr->table_list[IP_DISCOVERY].offset);
> + ihdr = (struct ip_discovery_header *)(adev->mman.discovery_bin +
> + offset);
> + ihdr_ver = le16_to_cpu(ihdr->version);
> /*
> * Harvest table does not fit Navi1x and legacy GPUs,
> * so read harvest bit per IP data structure to set
> * harvest configuration.
> */
> if (amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(10, 2, 0) &&
> - amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(9, 4, 3) &&
> - amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(9, 4, 4) &&
> - amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(9, 5, 0)) {
> + ihdr_ver <= 2) {
> if ((adev->pdev->device == 0x731E &&
> (adev->pdev->revision == 0xC6 ||
> adev->pdev->revision == 0xC7)) ||
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: Use version to figure out harvest info
2025-01-28 15:31 ` Alex Deucher
@ 2025-01-29 3:53 ` Lazar, Lijo
0 siblings, 0 replies; 4+ messages in thread
From: Lazar, Lijo @ 2025-01-29 3:53 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx, Hawking.Zhang, Alexander.Deucher, Asad.Kamal, Le.Ma
On 1/28/2025 9:01 PM, Alex Deucher wrote:
> On Tue, Jan 28, 2025 at 1:42 AM Lijo Lazar <lijo.lazar@amd.com> wrote:
>>
>> IP tables with version <=2 may use harvest bit. For version 3 and above,
>> harvest bit is not applicable, instead uses harvest table. Fix the
>> logic accordingly.
>>
>> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 27 +++++++++++--------
>> 1 file changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> index d34b97a081d8..e3afdf933dc8 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> @@ -612,7 +612,7 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
>> struct binary_header *bhdr;
>> struct ip_discovery_header *ihdr;
>> struct die_header *dhdr;
>> - struct ip_v4 *ip;
>> + struct ip *ip;
>> uint16_t die_offset, ip_offset, num_dies, num_ips;
>> uint16_t hw_id;
>> uint8_t inst;
>> @@ -631,13 +631,14 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
>> ip_offset = die_offset + sizeof(*dhdr);
>>
>> for (j = 0; j < num_ips; j++) {
>> - ip = (struct ip_v4 *)(adev->mman.discovery_bin + ip_offset);
>> - inst = ip->instance_number;
>> + ip = (struct ip *)(adev->mman.discovery_bin +
>> + ip_offset);
>> + inst = ip->number_instance;
>> hw_id = le16_to_cpu(ip->hw_id);
>> if (amdgpu_discovery_validate_ip(adev, inst, hw_id))
>> goto next_ip;
>>
>> - if (le16_to_cpu(ip->variant) == 1) {
>> + if (le16_to_cpu(ip->harvest) == 1) {
>
> ip->harvest is a uint8_t so it doesn't need byte swapping. Other than
> that, the series is:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Thanks for the catch. Will make this change before commit.
Thanks,
Lijo
>
>
>> switch (hw_id) {
>> case VCN_HWID:
>> (*vcn_harvest_count)++;
>> @@ -663,10 +664,8 @@ static void amdgpu_discovery_read_harvest_bit_per_ip(struct amdgpu_device *adev,
>> }
>> }
>> next_ip:
>> - if (ihdr->base_addr_64_bit)
>> - ip_offset += struct_size(ip, base_address_64, ip->num_base_address);
>> - else
>> - ip_offset += struct_size(ip, base_address, ip->num_base_address);
>> + ip_offset += struct_size(ip, base_address,
>> + ip->num_base_address);
>> }
>> }
>> }
>> @@ -1474,18 +1473,24 @@ static int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev)
>>
>> static void amdgpu_discovery_harvest_ip(struct amdgpu_device *adev)
>> {
>> + struct ip_discovery_header *ihdr;
>> + struct binary_header *bhdr;
>> int vcn_harvest_count = 0;
>> int umc_harvest_count = 0;
>> + uint16_t offset, ihdr_ver;
>>
>> + bhdr = (struct binary_header *)adev->mman.discovery_bin;
>> + offset = le16_to_cpu(bhdr->table_list[IP_DISCOVERY].offset);
>> + ihdr = (struct ip_discovery_header *)(adev->mman.discovery_bin +
>> + offset);
>> + ihdr_ver = le16_to_cpu(ihdr->version);
>> /*
>> * Harvest table does not fit Navi1x and legacy GPUs,
>> * so read harvest bit per IP data structure to set
>> * harvest configuration.
>> */
>> if (amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(10, 2, 0) &&
>> - amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(9, 4, 3) &&
>> - amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(9, 4, 4) &&
>> - amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(9, 5, 0)) {
>> + ihdr_ver <= 2) {
>> if ((adev->pdev->device == 0x731E &&
>> (adev->pdev->revision == 0xC6 ||
>> adev->pdev->revision == 0xC7)) ||
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-01-29 3:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-28 6:31 [PATCH 1/2] drm/amdgpu: Pass IP instance/hwid as parameters Lijo Lazar
2025-01-28 6:31 ` [PATCH 2/2] drm/amdgpu: Use version to figure out harvest info Lijo Lazar
2025-01-28 15:31 ` Alex Deucher
2025-01-29 3:53 ` Lazar, Lijo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox