[PATCH v2] drm/amdgpu: expose amdgpu_sensors on pre-powerplay chips

[Samuel Pitoiset]

On 02/14/2017 05:41 PM, Tom St Denis wrote:
> On 14/02/17 10:08 AM, Samuel Pitoiset wrote:
>> Totally untested but as long as read_sensor() has been recently
>> implemented for dpm based boards, amdgpu_sensors can now be
>> exposed.
>>
>> v2: - make sure read_sensor is not NULL on dpm chips
>>     - keep sanity check for powerplay chips
>>
>> Cc: Tom St Denis <tom.stdenis at amd.com>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 6f021e70f15f..80821b436aeb 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -3204,6 +3204,9 @@ static ssize_t amdgpu_debugfs_sensor_read(struct
>> file *f, char __user *buf,
>>      valuesize = sizeof(values);
>>      if (adev->powerplay.pp_funcs &&
>> adev->powerplay.pp_funcs->read_sensor)
>>          r =
>> adev->powerplay.pp_funcs->read_sensor(adev->powerplay.pp_handle, idx,
>> &values[0], &valuesize);
>> +    else if (adev->pm.funcs && adev->pm.funcs->read_sensor)
>> +        r = adev->pm.funcs->read_sensor(adev, idx, &values[0],
>> +                        &valuesize);
>>      else
>>          return -EINVAL;
>>
>>
>
> Sorry NAK again, even with dpm=0 those function pointers are set and you
> end up inside the dpm code trying to parse data structures that aren't
> initialized.

No worries. I thought that check was enough. Anyway, writing code 
without the hardware should be avoided. :)

Can you try the thing suggested by Alex? Because I will need to fix up 
the DRM ioctl codepath as well.

>
> For instance, this oops on my Kaveri when trying to read SCLK.
>
> [  729.616382] BUG: unable to handle kernel NULL pointer dereference at
> 00000000000000c4
> [  729.616467] IP: [<ffffffffc0e62d40>] kv_dpm_read_sensor+0xa0/0xd0
> [amdgpu]
> [  729.616619] PGD 232c58067
> [  729.616644] PUD 232c4f067
> [  729.616670] PMD 0
>
> [  729.616697] Oops: 0000 [#1] SMP
> [  729.616726] Modules linked in: fuse amdkfd amd_iommu_v2 amdgpu
> i2c_algo_bit ttm drm_kms_helper drm xt_CHECKSUM iptable_mangle
> ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat
> nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack tun bridge
> ebtable_filter ebtables ip6table_filter ip6_tables rpcsec_gss_krb5 nfsv4
> dns_resolver nfs fscache bnep vfat fat edac_mce_amd arc4 edac_core
> kvm_amd iwlmvm snd_hda_codec_realtek mac80211 kvm snd_hda_codec_generic
> snd_hda_codec_hdmi snd_hda_intel snd_hda_codec snd_hda_core btusb btrtl
> btbcm btintel iwlwifi bluetooth snd_hwdep cfg80211 snd_seq
> snd_seq_device snd_pcm irqbypass joydev snd_timer snd crct10dif_pclmul
> crc32_pclmul pcspkr ghash_clmulni_intel rfkill fam15h_power sp5100_tco
> k10temp soundcore acpi_cpufreq tpm_tis i2c_piix4 tpm_tis_core
> [  729.617499]  shpchp video tpm nfsd auth_rpcgss nfs_acl lockd grace
> sunrpc ata_generic pata_acpi 8021q crc32c_intel garp stp llc serio_raw
> mrp pata_atiixp r8169 mii fjes
> [  729.617672] CPU: 0 PID: 1417 Comm: umr Not tainted 4.9.0+ #4
> [  729.617720] Hardware name: Gigabyte Technology Co., Ltd. To be filled
> by O.E.M./F2A88XN-WIFI, BIOS F5 04/22/2015
> [  729.617802] task: ffff8e43ef37d880 task.stack: ffff9d7501594000
> [  729.617852] RIP: 0010:[<ffffffffc0e62d40>]  [<ffffffffc0e62d40>]
> kv_dpm_read_sensor+0xa0/0xd0 [amdgpu]
> [  729.618000] RSP: 0018:ffff9d7501597d60  EFLAGS: 00010246
> [  729.618046] RAX: 0000000000000000 RBX: ffff9d7501597d98 RCX:
> 0000000000000000
> [  729.618104] RDX: 0000000000000000 RSI: 0000000000000286 RDI:
> 0000000000000286
> [  729.618162] RBP: ffff9d7501597d80 R08: 0000000000000000 R09:
> 0000000000000000
> [  729.618221] R10: 0000000000000000 R11: ffff8e43ef37d880 R12:
> ffff9d7501597d98
> [  729.618285] R13: 0000000000000000 R14: ffff9d7501597d94 R15:
> 0000000000000000
> [  729.618344] FS:  00007f6525bfd700(0000) GS:ffff8e43fec00000(0000)
> knlGS:0000000000000000
> [  729.618411] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  729.618458] CR2: 00000000000000c4 CR3: 00000002261f6000 CR4:
> 00000000000406f0
> [  729.618515] Stack:
> [  729.618535]  0000000000000004 00007ffe1e85ca00 ffff8e43f3162100
> 0000000000000004
> [  729.618607]  ffff9d7501597df8 ffffffffc0e2c637 000000401e85ca00
> ffff9d7501597f18
> [  729.618676]  ffff9d7501597df8 ffffffffc0e2dc7d 0000000000005403
> 0000000000000000
> [  729.618747] Call Trace:
> [  729.618826]  [<ffffffffc0e2c637>]
> amdgpu_debugfs_sensor_read+0xf7/0x120 [amdgpu]
> [  729.618938]  [<ffffffffc0e2dc7d>] ?
> amdgpu_debugfs_regs_read+0x17d/0x220 [amdgpu]
> [  729.619002]  [<ffffffff8f3433e4>] full_proxy_read+0x54/0x90
> [  729.619049]  [<ffffffff8f24be67>] __vfs_read+0x37/0x150
> [  729.619095]  [<ffffffff8f35e67b>] ? security_file_permission+0x9b/0xc0
> [  729.621382]  [<ffffffff8f24cfe6>] vfs_read+0x96/0x130
> [  729.622997]  [<ffffffff8f24e4d5>] SyS_read+0x55/0xc0
> [  729.624540]  [<ffffffff8f8010b7>] entry_SYSCALL_64_fastpath+0x1a/0xa9
> [  729.626139] Code: 24 31 c0 41 c7 06 04 00 00 00 41 5c 41 5d 41 5e 41
> 5f 5d c3 c1 e8 10 83 e0 1f 83 f8 07 77 ad 48 8d 14 c5 00 00 00 00 5b 48
> 29 c2 <41> 8b 84 95 c4 00 00 00 0f c8 41 89 04 24 41 c7 06 04 00 00 00
>