Re: [PATCH 4/4] nouveau/debugfs: add interface for current load

[Martin Peres <martin.peres-GANU6spQydw@public.gmane.org>]

On 26/10/15 20:17, Ilia Mirkin wrote:
> On Mon, Oct 26, 2015 at 2:13 PM, Karol Herbst <nouveau@karolherbst.de> wrote:
>> From: Karol Herbst <git@karolherbst.de>
>>
>> ---
>>   drm/nouveau/include/nvif/device.h     |  1 +
>>   drm/nouveau/include/nvkm/subdev/pmu.h | 10 ++++++++++
>>   drm/nouveau/nouveau_debugfs.c         | 23 +++++++++++++++++++++++
>>   drm/nouveau/nvkm/subdev/pmu/base.c    | 18 ++++++++++++++++++
>>   4 files changed, 52 insertions(+)
>>
>> diff --git a/drm/nouveau/include/nvif/device.h b/drm/nouveau/include/nvif/device.h
>> index 700a9b2..d289fdf 100644
>> --- a/drm/nouveau/include/nvif/device.h
>> +++ b/drm/nouveau/include/nvif/device.h
>> @@ -63,6 +63,7 @@ u64  nvif_device_time(struct nvif_device *);
>>   #define nvxx_clk(a) nvxx_device(a)->clk
>>   #define nvxx_i2c(a) nvxx_device(a)->i2c
>>   #define nvxx_therm(a) nvxx_device(a)->therm
>> +#define nvxx_pmu(a) nvxx_device(a)->pmu
>>
>>   #include <core/device.h>
>>   #include <engine/fifo.h>
>> diff --git a/drm/nouveau/include/nvkm/subdev/pmu.h b/drm/nouveau/include/nvkm/subdev/pmu.h
>> index e61923d..be3c60e 100644
>> --- a/drm/nouveau/include/nvkm/subdev/pmu.h
>> +++ b/drm/nouveau/include/nvkm/subdev/pmu.h
>> @@ -23,6 +23,13 @@ struct nvkm_pmu {
>>          } recv;
>>   };
>>
>> +struct nvkm_pmu_load_data {
>> +       u8 core;
>> +       u8 mem;
>> +       u8 video;
>> +       u8 pcie;
>> +};
>> +
>>   int nvkm_pmu_send(struct nvkm_pmu *, u32 reply[2], u32 process,
>>                    u32 message, u32 data0, u32 data1);
>>   void nvkm_pmu_pgob(struct nvkm_pmu *, bool enable);
>> @@ -48,4 +55,7 @@ void nvkm_memx_train(struct nvkm_memx *);
>>   int  nvkm_memx_train_result(struct nvkm_pmu *, u32 *, int);
>>   void nvkm_memx_block(struct nvkm_memx *);
>>   void nvkm_memx_unblock(struct nvkm_memx *);
>> +
>> +/* interface to PERF process running on PMU */
>> +int nvkm_pmu_get_perf_data(struct nvkm_pmu *, struct nvkm_pmu_load_data *);
>>   #endif
>> diff --git a/drm/nouveau/nouveau_debugfs.c b/drm/nouveau/nouveau_debugfs.c
>> index 5392e07..ec3d3d3 100644
>> --- a/drm/nouveau/nouveau_debugfs.c
>> +++ b/drm/nouveau/nouveau_debugfs.c
>> @@ -28,6 +28,8 @@
>>    *  Ben Skeggs <bskeggs@redhat.com>
>>    */
>>
>> +#include <nvkm/subdev/pmu.h>
>> +
>>   #include "nouveau_debugfs.h"
>>   #include "nouveau_drm.h"
>>
>> @@ -43,8 +45,29 @@ nouveau_debugfs_vbios_image(struct seq_file *m, void *data)
>>          return 0;
>>   }
>>
>> +static int
>> +nouveau_debugfs_current_load(struct seq_file *m, void *data)
>> +{
>> +       struct drm_info_node *node = (struct drm_info_node *) m->private;
>> +       struct nouveau_drm *drm = nouveau_drm(node->minor->dev);
>> +       struct nvkm_pmu *pmu = nvxx_pmu(&drm->device);
>> +       struct nvkm_pmu_load_data load_data;
>> +       int ret;
>> +
>> +       ret = nvkm_pmu_get_perf_data(pmu, &load_data);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       seq_printf(m, "core: %i\n", load_data.core);
>> +       seq_printf(m, "mem: %i\n", load_data.mem);
>> +       seq_printf(m, "video: %i\n", load_data.video);
>> +       seq_printf(m, "pcie: %i\n", load_data.pcie);
>> +       return 0;
>> +}
>> +
>>   static struct drm_info_list nouveau_debugfs_list[] = {
>>          { "vbios.rom", nouveau_debugfs_vbios_image, 0, NULL },
>> +       { "current_load", nouveau_debugfs_current_load, 0, NULL },
>>   };
>>   #define NOUVEAU_DEBUGFS_ENTRIES ARRAY_SIZE(nouveau_debugfs_list)
>>
>> diff --git a/drm/nouveau/nvkm/subdev/pmu/base.c b/drm/nouveau/nvkm/subdev/pmu/base.c
>> index d95eb86..ddb36e7 100644
>> --- a/drm/nouveau/nvkm/subdev/pmu/base.c
>> +++ b/drm/nouveau/nvkm/subdev/pmu/base.c
>> @@ -140,6 +140,24 @@ nvkm_pmu_recv(struct work_struct *work)
>>                    process, message, data0, data1);
>>   }
>>
>> +#define get_counter_index(v, i) (((v) & 0xff << ((i)*8)) >> ((i)*8))
> Is this the same thing as
>
> (v >> (i*8)) & 0xff
>
> ? I can't tell if you're attempting to preserve the sign, but don't
> see why you'd want to since it all just becomes a u8 anyways.

There is no sign and the values are u8 anyway. So you are right, this is 
what Karol should do.

>
>> +
>> +int
>> +nvkm_pmu_get_perf_data(struct nvkm_pmu *pmu, struct nvkm_pmu_load_data *data)
>> +{
>> +       int result[2], ret;
> Given the bit manipulations, sounds like result should be (a) unsigned
> and (b) sized.

Agreed. u32 is what he needs.

>
>> +       ret = nvkm_pmu_send(pmu, result, PROC_PERF, PERF_MSG_LOAD, 0, 0);
>> +
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       data->core = get_counter_index(result[0], 0);
>> +       data->video = get_counter_index(result[0], 1);
>> +       data->mem = get_counter_index(result[0], 2);
>> +       data->pcie = get_counter_index(result[0], 3);
>> +       return 0;
>> +}
>> +
>>   static void
>>   nvkm_pmu_intr(struct nvkm_subdev *subdev)
>>   {
>> --
>> 2.6.2
>>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nouveau
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau