mbox series

[0/2] FastRPC reserved memory assignment for SDM845 SLPI

Message ID 20230325134410.21092-1-me@dylanvanassche.be
Headers show
Series FastRPC reserved memory assignment for SDM845 SLPI | expand

Message

Dylan Van Assche March 25, 2023, 1:44 p.m. UTC
* About *

The Qualcomm SDM845 SoC has a separate SLPI (Sensor Low Power Island)
DSP for sensors connected to the SoC which is responsible for exposing
sensors to userspace, power saving, and other features. 
While sensors are connected to GPIOs of the SoC, they cannot be used
because the hypervisor blocks direct access to the sensors, thus the 
DSP must be used to access any sensor on this SoC. The SLPI DSP uses a
GLink edge (dsps) to communicate with the host and has a FastRPC interface
to load files from the host filesystem such as sensor configuration files.
The FastRPC interface does not use regular FastRPC Compute Banks
but instead uses an allocated CMA region through which communication happens.

* Changes *

This patchseries add support to the FastRPC for assigning a coherent memory
region to a DSP via the hypervisor with the correct permissions.
This is necessary to support the SLPI found in the Qualcomm SDM845 SoC which
does not have dedicated FastRPC Compute Banks, in contrast to newer SoCs,
but uses a memory region instead.

* Related patches *

1. Remoteproc changes to support the SLPI DSP in SDM845:
https://lore.kernel.org/linux-remoteproc/20230325132117.19733-1-me@dylanvanassche.be/
2. DTS changes will be submitted after this serie.

This serie does not depend on any serie, but all of them are necessary
to enable the feature in the end.

Kind regards,
Dylan Van Assche

Dylan Van Assche (2):
  dt-bindings: misc: qcom,fastrpc: add qcom,assign-all-memory property
  misc: fastrpc: support complete DMA pool access to the DSP

 .../bindings/misc/qcom,fastrpc.yaml           |  6 ++++++
 drivers/misc/fastrpc.c                        | 19 ++++++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

Comments

Dylan Van Assche March 27, 2023, 4:05 p.m. UTC | #1
Hi Dmitry & Krzysztof,

On Mon, 2023-03-27 at 16:36 +0200, Krzysztof Kozlowski wrote:
> On 27/03/2023 16:26, Dylan Van Assche wrote:
> > > Bindings are not for driver behavior.
> > > 
> > > > Downstream does guard
> > > > this with a property 'restrict-access' as well, see [1] for a
> > > > random
> > > > SDM845 downstream kernel. On SDM845, this property is not
> > > > present,
> > > > thus
> > > > the IF block runs. On SDM670, this property is present, then
> > > > the IF
> > > > block is skipped. That's why I opt for this property to have
> > > > this
> > > > behaviour conditionally. I'm not sure how to explain it better
> > > > though.
> > > 
> > > Still you described driver... Please come with something more
> > > hardware
> > > related.
> > 
> > So just updating the description is enough then?
> > 
> > As this is all reverse engineered, I have no access to the
> > documentation of FastRPC, so best effort:
> > 
> > """
> > Mark allocated memory region accessible to remote processor.
> > This memory region is used by remote processor to communicate
> > when no dedicated Fastrpc context bank hardware is available 
> > for remote processor.
> 
> This description does not explain here anything. The memory region is
> already accessible without this property.
> 
> You described the desired Linux feature or behavior, not the actual
> hardware. The bindings are about the latter, so instead you need to
> rephrase the property and its description to match actual hardware
> capabilities/features/configuration etc.
> 
> Remember that any arguments to downstream are not really good
> arguments.
> Their design choices and bindings are usually totally not acceptable.
> They simply embed whatever driver needs in DT - policies, system
> configuration, driver behavior...
> 
> Also, Dmitry made here good point.
> 
> 

I agree, downstream is not doing great on being upstreamable.
Thanks Dmitry, your explanation makes it pretty clear what I should
figure out. This helps a lot! As far as I know, this assignment is only
skipped when the sensors are not on the SLPI but on the ADSP e.g.
SDM670, thus mid range SoCs. So reading these comments, this looks more
like 'driver behavior' which should not end up in bindings as mentioned
above. As I now understand the problem with this property, I will
rework it for v2 and drop it. This is only done for the SLPI so by
guarding it with a domain ID check we should be able to avoid the
property in the bindings.

Thanks for the feedback & patience! I really learned a lot!

Kind regards,
Dylan Van Assche

> 
> Best regards,
> Krzysztof
>