Re: [PATCH v5 2/5] firmware: tegra: bpmp: Add ACPI support

[Thierry Reding <thierry.reding@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4095 bytes --]

On Fri, Nov 06, 2026 at 09:25:56AM +0000, Sudeep Holla wrote:
> On Thu, Nov 05, 2026 at 02:07:48PM +0100, Thierry Reding wrote:
> > On Thu, Nov 05, 2026 at 09:50:26AM +0000, Sudeep Holla wrote:
> > > On Tue, Sep 29, 2026 at 09:59:12AM +0000, Aniruddha Rao wrote:
> > > > Add required changes in the Tegra BPMP driver to make it compatible with
> > > > ACPI-based platforms.
> > > > 
> > > > On ACPI systems, IPC is handled through an AML method instead of the
> > > > kernel mailbox and IVC frameworks.
> > > > 
> > > > Clock, reset and power-domain providers are not controlled by the Linux
> > > > BPMP driver on ACPI-based systems, so do not initialize them there.
> > > > 
> > > 
> > > The above 2 statements are very clear and says this driver is not much
> > > useful on ACPI systems which is good and aligns with the general
> > > expectations.
> > > 
> > > > Keep transport initialization in the SoC-specific init operation. Use an
> > > > optional init_providers operation for DT child and provider registration
> > > > after BPMP communication has been established and the firmware tag has
> > > > been read. ACPI platforms leave this operation unset.
> > > > 
> > > 
> > > "ACPI platforms leave this operation unset" - I don't fully understand
> > > what this means ? If it means we need to do something on ACPI systems,
> > > why can't it be done by some boot firmware instead ?
> > 
> > I think it just means that the "init_providers" operation isn't needed
> > on ACPI systems because we don't expose clocks, resets or power domains
> > in ACPI (we do this only on DT so that we can have cross-references
> > between device tree nodes). I don't think anything needs to be done in
> > the ACPI case, so leaving this operation unset is exactly that.
> > 
> 
> Thanks for the confirmation.
> 
> > > > DT transports register an MRQ_PING handler for requests initiated by
> > > > BPMP firmware. This request direction was used only by a diagnostic
> > > > command in older firmware and is not provided by firmware on ACPI-based
> > > > systems. The ACPI transport therefore does not register an incoming
> > > > handler; its init operation only sends the outgoing MRQ_PING request used
> > > > to verify communication with firmware.
> > > > 
> > > 
> > > OK, I am more confused as you state more things are not needed on ACPI
> > > systems. Why do we need to add any support for ACPI systems then? Can you
> > > make it clear as what is needed on ACPI systems and why it needs to be
> > > done by OSPM and not any boot firmware if it is just one time initialisation.
> > 
> > My understanding of this is that the goal was to completely hide BPMP on
> > ACPI systems and do all the clock, reset and power domain handling via
> > standard ACPI operations. MBWT is a feature that's needed on ACPI
> > systems and that can't be represented as a standard ACPI operation, as I
> > understand it, hence the addition of ACPI support to BPMP.
> > 
> 
> OK.
> 
> > So some of this patch is stripping out things that don't make sense on
> > ACPI (the ACPI support for BPMP is very minimal) just to get enough
> > support in place to expose the MBWT caps via sysfs.
> > 
> 
> I am bit concerned as MBWT is something to do with memory bandwidth
> throttling and resctrl provides user ABI for such things, this sysfs
> may be duplicating part of it.
> 
> > The sysfs knobs exposed in patches 4 and 5 exist because the bandwidth
> > caps may need to be adjusted dynamically at runtime. It is indeed a bit
> > absurd to bootstrap a restricted version of BPMP like this for just this
> > one feature, but I don't see any alternatives.
> > 
> 
> Is MPAM/resctrl path explored and you have valid reasons to not use that
> and create your own sysfs ?

My understanding is that only monitoring is implemented for MPAM with no
way of actually controlling bandwidth through it.

I've started an internal discussion to evaluate whether we can avoid
custom sysfs attributes in favour of exposing them through resctrl.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]