<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>On 4/4/2025 5:13 PM, Jeff Hugo wrote:</p>
<blockquote type="cite"
cite="mid:464ccbb4-3c72-4ecf-a0cf-35d2ad9d04a4@oss.qualcomm.com">On
4/1/2025 9:59 AM, Maciej Falkowski wrote:
<br>
<blockquote type="cite">From: Andrzej Kacprowski
<a class="moz-txt-link-rfc2396E" href="mailto:Andrzej.Kacprowski@intel.com"><Andrzej.Kacprowski@intel.com></a>
<br>
<br>
Add sysfs files that show maximum and current
<br>
frequency of the NPU's data processing unit.
<br>
New sysfs entries:
<br>
- npu_max_frequency_mhz
<br>
</blockquote>
<br>
Don't you have an ioctl to get this, which is fixed in patch 1 of
the series? Why duplicate that with a sysfs?<br>
</blockquote>
<p>We added this to keep it consistent with sysfs entry for current
frequency.<br>
This is more for a convenience but ioctl is still required as
user-mode driver requires this.<br>
Do you think it is justified having this consideration?</p>
<blockquote type="cite"
cite="mid:464ccbb4-3c72-4ecf-a0cf-35d2ad9d04a4@oss.qualcomm.com">
<br>
<blockquote type="cite">- npu_current_frequency_mhz
<br>
</blockquote>
<br>
Do you have userspace code that consumes these?<br>
</blockquote>
<p>We do have a support for two device monitors in a userspace -
resources[1] and btop[2].<br>
I develop myself a btop support and have already frequency
included, I will also include it in resources.</p>
<p>[1]: <a
href="https://github.com/nokyan/resources/blob/main/src/utils/npu/intel.rs"
rel="noreferrer noopener"
title="https://github.com/nokyan/resources/blob/main/src/utils/npu/intel.rs"
target="_blank">https://github.com/nokyan/resources/blob/main/src/utils/npu/intel.rs<br>
</a>[2]: <a href="https://github.com/m-falkowski/btop"
rel="noreferrer noopener"
title="https://github.com/m-falkowski/btop" target="_blank"
class="moz-txt-link-freetext">https://github.com/m-falkowski/btop</a></p>
<blockquote type="cite"
cite="mid:464ccbb4-3c72-4ecf-a0cf-35d2ad9d04a4@oss.qualcomm.com">
<br>
<blockquote type="cite">
<br>
Signed-off-by: Andrzej Kacprowski
<a class="moz-txt-link-rfc2396E" href="mailto:Andrzej.Kacprowski@intel.com"><Andrzej.Kacprowski@intel.com></a>
<br>
Signed-off-by: Maciej Falkowski
<a class="moz-txt-link-rfc2396E" href="mailto:maciej.falkowski@linux.intel.com"><maciej.falkowski@linux.intel.com></a>
<br>
---
<br>
drivers/accel/ivpu/ivpu_hw.h | 5 ++++
<br>
drivers/accel/ivpu/ivpu_hw_btrs.c | 8 +++++
<br>
drivers/accel/ivpu/ivpu_hw_btrs.h | 1 +
<br>
drivers/accel/ivpu/ivpu_sysfs.c | 49
++++++++++++++++++++++++++++++-
<br>
</blockquote>
<br>
Missing uapi documentation.
<br>
<br>
Also, should the sysfs maintainers be included in the review?
<br>
<br>
</blockquote>
Thank you for point this out. I included recipients using
get_maintainers script.<br>
<br>
Best regards,<br>
Maciej<br>
<br>
</body>
</html>