[PATCH] drm/amdgpu: Add "harvest" to IP discovery sysfs

[Luben Tuikov]

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 at amd.com>
>>>> Signed-off-by: Luben Tuikov <luben.tuikov at 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