Re: [PATCH v2 5/5] accel/amdxdna: Provide real-time NPU power estimate via AMD PMF

[Shyam Sundar S K <Shyam-sundar.S-k@amd.com>]

On 11/19/2025 18:14, Ilpo Järvinen wrote:
> On Tue, 18 Nov 2025, Shyam Sundar S K wrote:
>> On 11/18/2025 21:31, Ilpo Järvinen wrote:
>>> On Tue, 18 Nov 2025, Ilpo Järvinen wrote:
>>>
>>>> On Fri, 14 Nov 2025, Mario Limonciello wrote:
>>>>> On 11/13/2025 1:33 AM, Shyam Sundar S K wrote:
>>>>>> On 11/12/2025 23:33, Mario Limonciello wrote:
>>>>>>> On 11/11/25 12:37 AM, Shyam Sundar S K wrote:
>>>>>>>> From: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
>>>>>>>>
>>>>>>>> Add aie2_smu_get_power_estimate() to obtain NPU power readings from the
>>>>>>>> AMD PMF driver. This xdna interface enables user space to reflect
>>>>>>>> current
>>>>>>>> workload power consumption in real time.
>>>>>>>
>>>>>>> But.. it doesn't.  It just adds a new function that could potentially
>>>>>>> do this job.  The actual gluing it to userspace in some way to use
>>>>>>> this function will be another patch.
>>>>>>
>>>>>> For now, yes.. that's right. XDNA team will add more support on it
>>>>>> that goes via the accel tree. But this patch is meant to have a
>>>>>> minimal change on the xdna side so that there is a  consumer of the
>>>>>> pmf symbol that is getting exported to the kernel.
>>>>>>
>>>>>> So, there is more support coming on the way. If you think, its worth
>>>>>> to rephrase the commit?
>>>>>
>>>>> Yes; that other support isn't happening this kernel cycle.  So the commit
>>>>> message should be accurate to what it's actually doing (laying ground work).
>>>>
>>>> To me it looks this and patch 4 should be submitted with the actual user 
>>>> instead of trying to justify things with far in the future changes. I've 
>>>> no problem with creating immutable branch if necessary when multiple trees 
>>>> are involved.
>>>
>>> In case it wasn't obvious, I'm fine with taking patches 1-3 in earlier.
>>>
>>
>> The context for this series is that the XDNA driver needs
>> power-related information, which is only available from the PMF
>> driver. We’re providing that information via an exported symbol.
>> Patch 4 exports the data and patch 5 begins consuming it. The goal is
>> to establish the channel through which power-related information can flow.
> 
> It doesn't consume anything, it just adds function without apparent 
> purpose (and this will remain so until the other work is submitted).
> 
>> Creating an immutable branch is a reasonable idea, but in this case
>> all further PMF-side work stops after this patch. The subsequent
>> activity is primarily on the XDNA side — for example, passing the
>> power data into the XRT environment via IOCTLs. If both trees had
>> active development or large, simultaneous changes, an immutable branch
>> would make sense, but given the current situation it doesn’t seem
>> worth the added complexity.
>>
>> The XDNA changes are distributed and will be implemented gradually;
>> they depend on this exported power information from PMF. So my
>> proposal is for PMF to export the information now with minimal changes
>> on the XDNA side, and land that upstream. The XDNA team can then build
>> on top of the exported interface, and their changes can be submitted
>> via the misc-drm tree.
>>
>> Ilpo, does this approach work for you?
> 
> Lets just wait with these two patches until have real consumer changes 
> ready to be submitted, these patches can appear at the head of such a 
> series.
> 

OK, I get your point. Thank you. Will have an internal discussion on
this and come back on what could be a best approach for this.

For now, I have dropped the patch 4 and 5 and sent out the rest for
review and they can be applied on mainline with 6fb7433c7b8e
("platform/x86/amd/pmf: Use ring buffer to store custom BIOS input
values") as the git tip.

Thanks,
Shyam