[PATCH v2 0/6] drm/v3d: Improve Performance Counters handling

[Maíra Canal]

Hi Jani,

On 5/21/24 08:07, Jani Nikula wrote:
> On Mon, 20 May 2024, Maíra Canal <mcanal at igalia.com> wrote:
>> On 5/12/24 19:23, Maíra Canal wrote:>
>>> Maíra Canal (6):
>>>     drm/v3d: Add Performance Counters descriptions for V3D 4.2 and 7.1
>>>     drm/v3d: Different V3D versions can have different number of perfcnt
>>>     drm/v3d: Create a new V3D parameter for the maximum number of perfcnt
>>>     drm/v3d: Create new IOCTL to expose performance counters information
>>>     drm/v3d: Use V3D_MAX_COUNTERS instead of V3D_PERFCNT_NUM
>>>     drm/v3d: Deprecate the use of the Performance Counters enum >
>>>    drivers/gpu/drm/v3d/v3d_drv.c                 |  11 +
>>>    drivers/gpu/drm/v3d/v3d_drv.h                 |  14 +-
>>>    drivers/gpu/drm/v3d/v3d_perfmon.c             |  36 ++-
>>>    .../gpu/drm/v3d/v3d_performance_counters.h    | 208 ++++++++++++++++++
>>>    drivers/gpu/drm/v3d/v3d_sched.c               |   2 +-
>>>    include/uapi/drm/v3d_drm.h                    |  48 ++++
>>>    6 files changed, 316 insertions(+), 3 deletions(-)
>>>    create mode 100644 drivers/gpu/drm/v3d/v3d_performance_counters.h
>>>
>>
>> Applied to drm-misc/drm-misc-next!
> 
> What compiler do you use? I'm hitting the same as kernel test robot [1]
> with arm-linux-gnueabihf-gcc 12.2.0.

I use clang version 17.0.6.

> 
> In general, I don't think it's a great idea to put arrays in headers,
> and then include it everywhere via v3d_drv.h. You're not just relying on
> the compiler to optimize it away in compilation units where its not
> referenced (likely to happen), but also for the linker to deduplicate
> rodata (possible, but I'm not sure that it will happen).
> 
> I think you need to move the arrays to a .c file, and then either a) add
> interfaces to access the arrays, or b) declare the arrays and make them
> global. For the latter you also need to figure out how to expose the
> size.

I'll write a patch to fix it. Sorry for the disturbance, I didn't notice
it with clang.

Best Regards,
- Maíra

> 
> BR,
> Jani.
> 
> 
> [1] https://lore.kernel.org/r/202405211137.hueFkLKG-lkp@intel.com
> 
>