* RE: [PATCH] drm/amdgpu: Add "harvest" to IP discovery sysfs
2022-02-14 23:21 ` Luben Tuikov
@ 2022-02-15 8:58 ` Chen, Guchun
2022-02-15 14:15 ` Luben Tuikov
2022-02-15 14:25 ` Christian König
2022-02-15 16:21 ` Christian König
2 siblings, 1 reply; 10+ messages in thread
From: Chen, Guchun @ 2022-02-15 8:58 UTC (permalink / raw)
To: Tuikov, Luben, amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander, Tuikov, Luben
[Public]
Hi Luben,
I suggest holding on this pls. Harvest bit per IP data structure from VBIOS is not consistently correct. Exposing it to use via sysfs may confuse users.
Regards,
Guchun
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Luben Tuikov
Sent: Tuesday, February 15, 2022 7:22 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Tuikov, Luben <Luben.Tuikov@amd.com>
Subject: [PATCH] drm/amdgpu: Add "harvest" to IP discovery sysfs
Add the "harvest" field to the IP attributes in the IP discovery sysfs visualization, as this field is present in the binary data.
Cc: Alex Deucher <Alexander.Deucher@amd.com>
Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index c8dbdb78988ce0..0496d369504641 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -393,6 +393,7 @@ struct ip_hw_instance {
int hw_id;
u8 num_instance;
u8 major, minor, revision;
+ u8 harvest;
int num_base_addresses;
u32 base_addr[];
@@ -440,6 +441,11 @@ static ssize_t revision_show(struct ip_hw_instance *ip_hw_instance, char *buf)
return sysfs_emit(buf, "%d\n", ip_hw_instance->revision); }
+static ssize_t harvest_show(struct ip_hw_instance *ip_hw_instance, char
+*buf) {
+ return sysfs_emit(buf, "0x%01X\n", ip_hw_instance->harvest); }
+
static ssize_t num_base_addresses_show(struct ip_hw_instance *ip_hw_instance, char *buf) {
return sysfs_emit(buf, "%d\n", ip_hw_instance->num_base_addresses);
@@ -471,6 +477,7 @@ static struct ip_hw_instance_attr ip_hw_attr[] = {
__ATTR_RO(major),
__ATTR_RO(minor),
__ATTR_RO(revision),
+ __ATTR_RO(harvest),
__ATTR_RO(num_base_addresses),
__ATTR_RO(base_addr),
};
@@ -708,6 +715,7 @@ static int amdgpu_discovery_sysfs_ips(struct amdgpu_device *adev,
ip_hw_instance->major = ip->major;
ip_hw_instance->minor = ip->minor;
ip_hw_instance->revision = ip->revision;
+ ip_hw_instance->harvest = ip->harvest;
ip_hw_instance->num_base_addresses = ip->num_base_address;
for (kk = 0; kk < ip_hw_instance->num_base_addresses; kk++)
base-commit: d8604f1d237a145db48bae4ea60b85a5875df307
--
2.35.1.102.g2b9c120970
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] drm/amdgpu: Add "harvest" to IP discovery sysfs
2022-02-15 8:58 ` Chen, Guchun
@ 2022-02-15 14:15 ` Luben Tuikov
0 siblings, 0 replies; 10+ messages in thread
From: Luben Tuikov @ 2022-02-15 14:15 UTC (permalink / raw)
To: Chen, Guchun, amd-gfx@lists.freedesktop.org; +Cc: Deucher, Alexander
At the moment it's 0 across all IPs.
I doubt there are any consumers of the harvest data at the moment.
Since this will be fixed in VBIOS eventually, I suggest we add it now for completeness and it'll be there when this is fixed in VBIOS.
Regards,
Luben
On 2022-02-15 03:58, Chen, Guchun wrote:
> [Public]
>
> Hi Luben,
>
> I suggest holding on this pls. Harvest bit per IP data structure from VBIOS is not consistently correct. Exposing it to use via sysfs may confuse users.
>
> Regards,
> Guchun
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Luben Tuikov
> Sent: Tuesday, February 15, 2022 7:22 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Tuikov, Luben <Luben.Tuikov@amd.com>
> Subject: [PATCH] drm/amdgpu: Add "harvest" to IP discovery sysfs
>
> Add the "harvest" field to the IP attributes in the IP discovery sysfs visualization, as this field is present in the binary data.
>
> Cc: Alex Deucher <Alexander.Deucher@amd.com>
> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index c8dbdb78988ce0..0496d369504641 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -393,6 +393,7 @@ struct ip_hw_instance {
> int hw_id;
> u8 num_instance;
> u8 major, minor, revision;
> + u8 harvest;
>
> int num_base_addresses;
> u32 base_addr[];
> @@ -440,6 +441,11 @@ static ssize_t revision_show(struct ip_hw_instance *ip_hw_instance, char *buf)
> return sysfs_emit(buf, "%d\n", ip_hw_instance->revision); }
>
> +static ssize_t harvest_show(struct ip_hw_instance *ip_hw_instance, char
> +*buf) {
> + return sysfs_emit(buf, "0x%01X\n", ip_hw_instance->harvest); }
> +
> static ssize_t num_base_addresses_show(struct ip_hw_instance *ip_hw_instance, char *buf) {
> return sysfs_emit(buf, "%d\n", ip_hw_instance->num_base_addresses);
> @@ -471,6 +477,7 @@ static struct ip_hw_instance_attr ip_hw_attr[] = {
> __ATTR_RO(major),
> __ATTR_RO(minor),
> __ATTR_RO(revision),
> + __ATTR_RO(harvest),
> __ATTR_RO(num_base_addresses),
> __ATTR_RO(base_addr),
> };
> @@ -708,6 +715,7 @@ static int amdgpu_discovery_sysfs_ips(struct amdgpu_device *adev,
> ip_hw_instance->major = ip->major;
> ip_hw_instance->minor = ip->minor;
> ip_hw_instance->revision = ip->revision;
> + ip_hw_instance->harvest = ip->harvest;
> ip_hw_instance->num_base_addresses = ip->num_base_address;
>
> for (kk = 0; kk < ip_hw_instance->num_base_addresses; kk++)
>
> base-commit: d8604f1d237a145db48bae4ea60b85a5875df307
> --
> 2.35.1.102.g2b9c120970
Regards,
--
Luben
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: Add "harvest" to IP discovery sysfs
2022-02-14 23:21 ` Luben Tuikov
2022-02-15 8:58 ` Chen, Guchun
@ 2022-02-15 14:25 ` Christian König
2022-02-15 14:33 ` Luben Tuikov
2022-02-15 16:21 ` Christian König
2 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2022-02-15 14:25 UTC (permalink / raw)
To: Luben Tuikov, amd-gfx; +Cc: Alex Deucher
Am 15.02.22 um 00:21 schrieb Luben Tuikov:
> Add the "harvest" field to the IP attributes in
> the IP discovery sysfs visualization, as this
> field is present in the binary data.
>
> Cc: Alex Deucher <Alexander.Deucher@amd.com>
> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index c8dbdb78988ce0..0496d369504641 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -393,6 +393,7 @@ struct ip_hw_instance {
> int hw_id;
> u8 num_instance;
> u8 major, minor, revision;
> + u8 harvest;
Should we maybe use bool here instead?
Apart from that looks good to me.
Regards,
Christian.
>
> int num_base_addresses;
> u32 base_addr[];
> @@ -440,6 +441,11 @@ static ssize_t revision_show(struct ip_hw_instance *ip_hw_instance, char *buf)
> return sysfs_emit(buf, "%d\n", ip_hw_instance->revision);
> }
>
> +static ssize_t harvest_show(struct ip_hw_instance *ip_hw_instance, char *buf)
> +{
> + return sysfs_emit(buf, "0x%01X\n", ip_hw_instance->harvest);
> +}
> +
> static ssize_t num_base_addresses_show(struct ip_hw_instance *ip_hw_instance, char *buf)
> {
> return sysfs_emit(buf, "%d\n", ip_hw_instance->num_base_addresses);
> @@ -471,6 +477,7 @@ static struct ip_hw_instance_attr ip_hw_attr[] = {
> __ATTR_RO(major),
> __ATTR_RO(minor),
> __ATTR_RO(revision),
> + __ATTR_RO(harvest),
> __ATTR_RO(num_base_addresses),
> __ATTR_RO(base_addr),
> };
> @@ -708,6 +715,7 @@ static int amdgpu_discovery_sysfs_ips(struct amdgpu_device *adev,
> ip_hw_instance->major = ip->major;
> ip_hw_instance->minor = ip->minor;
> ip_hw_instance->revision = ip->revision;
> + ip_hw_instance->harvest = ip->harvest;
> ip_hw_instance->num_base_addresses = ip->num_base_address;
>
> for (kk = 0; kk < ip_hw_instance->num_base_addresses; kk++)
>
> base-commit: d8604f1d237a145db48bae4ea60b85a5875df307
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] drm/amdgpu: Add "harvest" to IP discovery sysfs
2022-02-15 14:25 ` Christian König
@ 2022-02-15 14:33 ` Luben Tuikov
2022-02-15 14:37 ` Christian König
0 siblings, 1 reply; 10+ messages in thread
From: Luben Tuikov @ 2022-02-15 14:33 UTC (permalink / raw)
To: Christian König, amd-gfx; +Cc: Alex Deucher
On 2022-02-15 09:25, Christian König wrote:
> Am 15.02.22 um 00:21 schrieb Luben Tuikov:
>> Add the "harvest" field to the IP attributes in
>> the IP discovery sysfs visualization, as this
>> field is present in the binary data.
>>
>> Cc: Alex Deucher <Alexander.Deucher@amd.com>
>> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> index c8dbdb78988ce0..0496d369504641 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> @@ -393,6 +393,7 @@ struct ip_hw_instance {
>> int hw_id;
>> u8 num_instance;
>> u8 major, minor, revision;
>> + u8 harvest;
>
> Should we maybe use bool here instead?
>
> Apart from that looks good to me.
Thanks Christian.
I don't mind using bool here.
I saw the field in the binary data is 4 bits and represented that.
Is the field actually bool in the IP binary data?
I can change the patch and resubmit.
Regards,
Luben
>
> Regards,
> Christian.
>
>>
>> int num_base_addresses;
>> u32 base_addr[];
>> @@ -440,6 +441,11 @@ static ssize_t revision_show(struct ip_hw_instance *ip_hw_instance, char *buf)
>> return sysfs_emit(buf, "%d\n", ip_hw_instance->revision);
>> }
>>
>> +static ssize_t harvest_show(struct ip_hw_instance *ip_hw_instance, char *buf)
>> +{
>> + return sysfs_emit(buf, "0x%01X\n", ip_hw_instance->harvest);
>> +}
>> +
>> static ssize_t num_base_addresses_show(struct ip_hw_instance *ip_hw_instance, char *buf)
>> {
>> return sysfs_emit(buf, "%d\n", ip_hw_instance->num_base_addresses);
>> @@ -471,6 +477,7 @@ static struct ip_hw_instance_attr ip_hw_attr[] = {
>> __ATTR_RO(major),
>> __ATTR_RO(minor),
>> __ATTR_RO(revision),
>> + __ATTR_RO(harvest),
>> __ATTR_RO(num_base_addresses),
>> __ATTR_RO(base_addr),
>> };
>> @@ -708,6 +715,7 @@ static int amdgpu_discovery_sysfs_ips(struct amdgpu_device *adev,
>> ip_hw_instance->major = ip->major;
>> ip_hw_instance->minor = ip->minor;
>> ip_hw_instance->revision = ip->revision;
>> + ip_hw_instance->harvest = ip->harvest;
>> ip_hw_instance->num_base_addresses = ip->num_base_address;
>>
>> for (kk = 0; kk < ip_hw_instance->num_base_addresses; kk++)
>>
>> base-commit: d8604f1d237a145db48bae4ea60b85a5875df307
>
Regards,
--
Luben
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] drm/amdgpu: Add "harvest" to IP discovery sysfs
2022-02-15 14:33 ` Luben Tuikov
@ 2022-02-15 14:37 ` Christian König
2022-02-15 14:39 ` Luben Tuikov
0 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2022-02-15 14:37 UTC (permalink / raw)
To: Luben Tuikov, amd-gfx; +Cc: Alex Deucher
Am 15.02.22 um 15:33 schrieb Luben Tuikov:
> On 2022-02-15 09:25, Christian König wrote:
>> Am 15.02.22 um 00:21 schrieb Luben Tuikov:
>>> Add the "harvest" field to the IP attributes in
>>> the IP discovery sysfs visualization, as this
>>> field is present in the binary data.
>>>
>>> Cc: Alex Deucher <Alexander.Deucher@amd.com>
>>> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>>> index c8dbdb78988ce0..0496d369504641 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>>> @@ -393,6 +393,7 @@ struct ip_hw_instance {
>>> int hw_id;
>>> u8 num_instance;
>>> u8 major, minor, revision;
>>> + u8 harvest;
>> Should we maybe use bool here instead?
>>
>> Apart from that looks good to me.
> Thanks Christian.
>
> I don't mind using bool here.
>
> I saw the field in the binary data is 4 bits and represented that.
>
> Is the field actually bool in the IP binary data?
I'm not sure either.
I would have expected it to be a single bit flag in the binary, but
4bits sounds like it serves some more purpose.
Probably best to stick to u8 for now in this case.
Regards,
Christian.
> I can change the patch and resubmit.
>
> Regards,
> Luben
>
>> Regards,
>> Christian.
>>
>>>
>>> int num_base_addresses;
>>> u32 base_addr[];
>>> @@ -440,6 +441,11 @@ static ssize_t revision_show(struct ip_hw_instance *ip_hw_instance, char *buf)
>>> return sysfs_emit(buf, "%d\n", ip_hw_instance->revision);
>>> }
>>>
>>> +static ssize_t harvest_show(struct ip_hw_instance *ip_hw_instance, char *buf)
>>> +{
>>> + return sysfs_emit(buf, "0x%01X\n", ip_hw_instance->harvest);
>>> +}
>>> +
>>> static ssize_t num_base_addresses_show(struct ip_hw_instance *ip_hw_instance, char *buf)
>>> {
>>> return sysfs_emit(buf, "%d\n", ip_hw_instance->num_base_addresses);
>>> @@ -471,6 +477,7 @@ static struct ip_hw_instance_attr ip_hw_attr[] = {
>>> __ATTR_RO(major),
>>> __ATTR_RO(minor),
>>> __ATTR_RO(revision),
>>> + __ATTR_RO(harvest),
>>> __ATTR_RO(num_base_addresses),
>>> __ATTR_RO(base_addr),
>>> };
>>> @@ -708,6 +715,7 @@ static int amdgpu_discovery_sysfs_ips(struct amdgpu_device *adev,
>>> ip_hw_instance->major = ip->major;
>>> ip_hw_instance->minor = ip->minor;
>>> ip_hw_instance->revision = ip->revision;
>>> + ip_hw_instance->harvest = ip->harvest;
>>> ip_hw_instance->num_base_addresses = ip->num_base_address;
>>>
>>> for (kk = 0; kk < ip_hw_instance->num_base_addresses; kk++)
>>>
>>> base-commit: d8604f1d237a145db48bae4ea60b85a5875df307
> Regards,
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] drm/amdgpu: Add "harvest" to IP discovery sysfs
2022-02-15 14:37 ` Christian König
@ 2022-02-15 14:39 ` Luben Tuikov
0 siblings, 0 replies; 10+ messages in thread
From: Luben Tuikov @ 2022-02-15 14:39 UTC (permalink / raw)
To: Christian König, amd-gfx; +Cc: Alex Deucher
On 2022-02-15 09:37, Christian König wrote:
>
>
> Am 15.02.22 um 15:33 schrieb Luben Tuikov:
>> On 2022-02-15 09:25, Christian König wrote:
>>> Am 15.02.22 um 00:21 schrieb Luben Tuikov:
>>>> Add the "harvest" field to the IP attributes in
>>>> the IP discovery sysfs visualization, as this
>>>> field is present in the binary data.
>>>>
>>>> Cc: Alex Deucher <Alexander.Deucher@amd.com>
>>>> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>>>> index c8dbdb78988ce0..0496d369504641 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>>>> @@ -393,6 +393,7 @@ struct ip_hw_instance {
>>>> int hw_id;
>>>> u8 num_instance;
>>>> u8 major, minor, revision;
>>>> + u8 harvest;
>>> Should we maybe use bool here instead?
>>>
>>> Apart from that looks good to me.
>> Thanks Christian.
>>
>> I don't mind using bool here.
>>
>> I saw the field in the binary data is 4 bits and represented that.
>>
>> Is the field actually bool in the IP binary data?
>
> I'm not sure either.
>
> I would have expected it to be a single bit flag in the binary, but
> 4bits sounds like it serves some more purpose.
>
> Probably best to stick to u8 for now in this case.
Okay. Can I get a R-B then?
Regards,
Luben
>
> Regards,
> Christian.
>
>> I can change the patch and resubmit.
>>
>> Regards,
>> Luben
>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> int num_base_addresses;
>>>> u32 base_addr[];
>>>> @@ -440,6 +441,11 @@ static ssize_t revision_show(struct ip_hw_instance *ip_hw_instance, char *buf)
>>>> return sysfs_emit(buf, "%d\n", ip_hw_instance->revision);
>>>> }
>>>>
>>>> +static ssize_t harvest_show(struct ip_hw_instance *ip_hw_instance, char *buf)
>>>> +{
>>>> + return sysfs_emit(buf, "0x%01X\n", ip_hw_instance->harvest);
>>>> +}
>>>> +
>>>> static ssize_t num_base_addresses_show(struct ip_hw_instance *ip_hw_instance, char *buf)
>>>> {
>>>> return sysfs_emit(buf, "%d\n", ip_hw_instance->num_base_addresses);
>>>> @@ -471,6 +477,7 @@ static struct ip_hw_instance_attr ip_hw_attr[] = {
>>>> __ATTR_RO(major),
>>>> __ATTR_RO(minor),
>>>> __ATTR_RO(revision),
>>>> + __ATTR_RO(harvest),
>>>> __ATTR_RO(num_base_addresses),
>>>> __ATTR_RO(base_addr),
>>>> };
>>>> @@ -708,6 +715,7 @@ static int amdgpu_discovery_sysfs_ips(struct amdgpu_device *adev,
>>>> ip_hw_instance->major = ip->major;
>>>> ip_hw_instance->minor = ip->minor;
>>>> ip_hw_instance->revision = ip->revision;
>>>> + ip_hw_instance->harvest = ip->harvest;
>>>> ip_hw_instance->num_base_addresses = ip->num_base_address;
>>>>
>>>> for (kk = 0; kk < ip_hw_instance->num_base_addresses; kk++)
>>>>
>>>> base-commit: d8604f1d237a145db48bae4ea60b85a5875df307
>> Regards,
>
Regards,
--
Luben
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: Add "harvest" to IP discovery sysfs
2022-02-14 23:21 ` Luben Tuikov
2022-02-15 8:58 ` Chen, Guchun
2022-02-15 14:25 ` Christian König
@ 2022-02-15 16:21 ` Christian König
2022-02-15 17:59 ` Alex Deucher
2 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2022-02-15 16:21 UTC (permalink / raw)
To: Luben Tuikov, amd-gfx; +Cc: Alex Deucher
Am 15.02.22 um 00:21 schrieb Luben Tuikov:
> Add the "harvest" field to the IP attributes in
> the IP discovery sysfs visualization, as this
> field is present in the binary data.
>
> Cc: Alex Deucher <Alexander.Deucher@amd.com>
> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index c8dbdb78988ce0..0496d369504641 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -393,6 +393,7 @@ struct ip_hw_instance {
> int hw_id;
> u8 num_instance;
> u8 major, minor, revision;
> + u8 harvest;
>
> int num_base_addresses;
> u32 base_addr[];
> @@ -440,6 +441,11 @@ static ssize_t revision_show(struct ip_hw_instance *ip_hw_instance, char *buf)
> return sysfs_emit(buf, "%d\n", ip_hw_instance->revision);
> }
>
> +static ssize_t harvest_show(struct ip_hw_instance *ip_hw_instance, char *buf)
> +{
> + return sysfs_emit(buf, "0x%01X\n", ip_hw_instance->harvest);
> +}
> +
Maybe add a comment regarding Guchun's concern. With that done feel free
to add a Reviewed-by: Christian König <christian.koenig@amd.com>
Regards,
Christian.
> static ssize_t num_base_addresses_show(struct ip_hw_instance *ip_hw_instance, char *buf)
> {
> return sysfs_emit(buf, "%d\n", ip_hw_instance->num_base_addresses);
> @@ -471,6 +477,7 @@ static struct ip_hw_instance_attr ip_hw_attr[] = {
> __ATTR_RO(major),
> __ATTR_RO(minor),
> __ATTR_RO(revision),
> + __ATTR_RO(harvest),
> __ATTR_RO(num_base_addresses),
> __ATTR_RO(base_addr),
> };
> @@ -708,6 +715,7 @@ static int amdgpu_discovery_sysfs_ips(struct amdgpu_device *adev,
> ip_hw_instance->major = ip->major;
> ip_hw_instance->minor = ip->minor;
> ip_hw_instance->revision = ip->revision;
> + ip_hw_instance->harvest = ip->harvest;
> ip_hw_instance->num_base_addresses = ip->num_base_address;
>
> for (kk = 0; kk < ip_hw_instance->num_base_addresses; kk++)
>
> base-commit: d8604f1d237a145db48bae4ea60b85a5875df307
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] drm/amdgpu: Add "harvest" to IP discovery sysfs
2022-02-15 16:21 ` Christian König
@ 2022-02-15 17:59 ` Alex Deucher
0 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2022-02-15 17:59 UTC (permalink / raw)
To: Christian König; +Cc: Alex Deucher, Luben Tuikov, amd-gfx list
yeah, I think it's better to keep the additional data there. While
there are some faulty vbioses out there, I think the goal is to have
it correct in the long term, so I think it is fine to just expose what
is in the ip discovery table as is for now. Maybe add a comment in
the code as Christian suggested. With that:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
On Tue, Feb 15, 2022 at 11:21 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 15.02.22 um 00:21 schrieb Luben Tuikov:
> > Add the "harvest" field to the IP attributes in
> > the IP discovery sysfs visualization, as this
> > field is present in the binary data.
> >
> > Cc: Alex Deucher <Alexander.Deucher@amd.com>
> > Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > index c8dbdb78988ce0..0496d369504641 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > @@ -393,6 +393,7 @@ struct ip_hw_instance {
> > int hw_id;
> > u8 num_instance;
> > u8 major, minor, revision;
> > + u8 harvest;
> >
> > int num_base_addresses;
> > u32 base_addr[];
> > @@ -440,6 +441,11 @@ static ssize_t revision_show(struct ip_hw_instance *ip_hw_instance, char *buf)
> > return sysfs_emit(buf, "%d\n", ip_hw_instance->revision);
> > }
> >
> > +static ssize_t harvest_show(struct ip_hw_instance *ip_hw_instance, char *buf)
> > +{
> > + return sysfs_emit(buf, "0x%01X\n", ip_hw_instance->harvest);
> > +}
> > +
>
> Maybe add a comment regarding Guchun's concern. With that done feel free
> to add a Reviewed-by: Christian König <christian.koenig@amd.com>
>
> Regards,
> Christian.
>
> > static ssize_t num_base_addresses_show(struct ip_hw_instance *ip_hw_instance, char *buf)
> > {
> > return sysfs_emit(buf, "%d\n", ip_hw_instance->num_base_addresses);
> > @@ -471,6 +477,7 @@ static struct ip_hw_instance_attr ip_hw_attr[] = {
> > __ATTR_RO(major),
> > __ATTR_RO(minor),
> > __ATTR_RO(revision),
> > + __ATTR_RO(harvest),
> > __ATTR_RO(num_base_addresses),
> > __ATTR_RO(base_addr),
> > };
> > @@ -708,6 +715,7 @@ static int amdgpu_discovery_sysfs_ips(struct amdgpu_device *adev,
> > ip_hw_instance->major = ip->major;
> > ip_hw_instance->minor = ip->minor;
> > ip_hw_instance->revision = ip->revision;
> > + ip_hw_instance->harvest = ip->harvest;
> > ip_hw_instance->num_base_addresses = ip->num_base_address;
> >
> > for (kk = 0; kk < ip_hw_instance->num_base_addresses; kk++)
> >
> > base-commit: d8604f1d237a145db48bae4ea60b85a5875df307
>
^ permalink raw reply [flat|nested] 10+ messages in thread