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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 21 May 2024, Maíra Canal <mcanal@igalia.com> wrote:
> Hi Jani,
>
> On 5/21/24 08:07, Jani Nikula wrote:
>> On Mon, 20 May 2024, Maíra Canal <mcanal@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.

Another report [1].


BR,
Jani.

[1] https://lore.kernel.org/all/20240529122955.4cc16889@canb.auug.org.au



-- 
Jani Nikula, Intel