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
