[PATCH] drm/v3d: Add DRM_IOCTL_V3D_PERFMON_SET_GLOBAL

Christian Gmeiner christian.gmeiner at gmail.com
Thu Oct 31 20:57:13 UTC 2024


Hi Maíra,

>
> I have one major issue with this approach: I don't think we should
> introduce a `global_perfmon` in `struct v3d_perfmon_info`. `struct
> v3d_perfmon_info` was created to store information about the counters,
> such as total number of perfcnts supported and the description of the
> counters.
>

Ah okay.. got the idea of global_perfmon.

>
> I believe you should use `active_perfmon` in your implementation and
> don't create `global_perfmon`. This is going to make the code less
> tricky to understand and it's going to make sure that the hardware inner
> working is transparent in software.
>
>
> Only one perfmon can be active in a given moment of time, therefore,
> let's use `active_perfmon` to represent it.
>

Relying solely on active_perfmon makes the code hard to follow. I need at
least a flag to indicate whether we are in global perfmon mode.

> I couple more things came to my attention. First, I don't think we need
> to limit the creation of other perfmons. We can create perfmons and
> don't use it for a while. We only need to make sure that the user can't
> attach perfmons to jobs, when the global perfmon is enabled.
>
> For sure, if we go through this strategy, there is no need to have a
> count of all the perfmons that V3D has.
>

That is a fantastic idea.

> I would prefer to treat the global perfmon as a state. Ideally, we would
> enable and disable this state through the IOCTL.
>
> One last thing is: don't forget to stop the perfmons when you don't use
> it anymore :)
>

I've sent v2 of this patch and hope I've addressed all your comments.

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy


More information about the dri-devel mailing list