-
Notifications
You must be signed in to change notification settings - Fork 108
Switch fastrpc recipe to GitHub version #843
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Current fastrpc recipe is fetching sources from codelinaro fastrpc project which has very limited feature supported. New fastrpc project is available on github which supports more advanced features along with the ones supported on codelinaro version. The additional features supported are audio PD daemons, DSP capabilities, mem_map and mem_unmap requests. Signed-off-by: Ekansh Gupta <[email protected]>
Systemd services automatic start is disabled. Enable the daemons might be used from DSP root PD. Signed-off-by: Ekansh Gupta <[email protected]>
Few of the fastrpc header files are used by developers for their projects. Copy these headers from source directory to target directory to ensure that they are available for developers to include in their own projects. Signed-off-by: Ekansh Gupta <[email protected]>
Test jobs for commit 9637e0c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Please follow the standard format for commit messages. None of the commits uses
recipes-foo
in the commit subject. - This version of FastRPC has had issues with the older platforms. Has it been tested on db410c? db820c?
SUMMARY = "Qualcomm FastRPC applications and library" | ||
SECTION = "devel" | ||
|
||
LICENSE = "BSD-3-Clause" | ||
LIC_FILES_CHKSUM = "file://src/fastrpc_apps_user.c;beginline=1;endline=29;md5=f94f3a7beba14ae2f59f817e9634f891" | ||
LIC_FILES_CHKSUM = "file://src/fastrpc_apps_user.c;beginline=1;endline=2;md5=a5b0aa365758f6917baf7a2d81f5d29e" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking two lines? Please use LICENSE.txt as a LIC_FILE here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack
install -d ${D}${includedir}/ | ||
for header in ${HEADER_FILES}; do | ||
install -m 0644 ${S}/inc/${header} ${D}${includedir}/ | ||
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be done by make install.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, can't we instead fix upstream to have all headers installed via make install? We shouldn't be tracking the headers we want via HEADER_FILES, this is something that the individual project should be able to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And please, install only the headers that make an actual API for the library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll test the change and update this, thanks.
@@ -53,6 +63,7 @@ FILES:${PN} += " \ | |||
${libdir}/libcdsprpc.so \ | |||
${libdir}/libsdsprpc.so \ | |||
" | |||
FILES:${PN}-dev:append = "${includedir}/*.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really necessary? If so, why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving headers installation to make install. This will be removed in the next patch
file://adsprpcd.service \ | ||
file://cdsprpcd.service \ | ||
file://sdsprpcd.service \ | ||
file://usr-lib-rfsa.service \ | ||
file://mount-dsp.sh \ | ||
" | ||
|
||
PV = "0.0+" | ||
PV = "0.1+" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, do we need this recipe to be tracking an unreleased version? Otherwise I would prefer if we could instead rename the recipe based on a released tag (latest is v0.1.4, so fastrpc_0.1.4.bb). If we need to track a newer revision, can't we work just creating another tag upstream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 regarding the use of version and the recipe file name reflecting the version.
@quic-ekangupt if the intent is to be able to use the tip of fastrpc for purpose of develpment against the tip you can use devupstream.
@@ -29,7 +29,7 @@ SYSTEMD_PACKAGES = "${PN} ${PN}-systemd" | |||
SYSTEMD_SERVICE:${PN} = "usr-lib-rfsa.service" | |||
|
|||
SYSTEMD_SERVICE:${PN}-systemd = "adsprpcd.service cdsprpcd.service sdsprpcd.service" | |||
SYSTEMD_AUTO_ENABLE:${PN}-systemd = "disable" | |||
SYSTEMD_AUTO_ENABLE:${PN}-systemd = "enable" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't enable the default here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I'll remove this
|
Please fix your quotations, it's impossible to understand, where is the quoted text and where is your answer. No, 'please open issues' is not a way to go. Please verify that it actually works, before submitting it. Those devices must be a part of your test / CI setup. |
SRC_URI = "\ | ||
git://git.codelinaro.org/linaro/qcomlt/fastrpc.git;branch=automake;protocol=https \ | ||
file://0001-apps_std_fopen_with_env-account-for-domain-kinds-whe.patch \ | ||
git://github.com/quic/fastrpc.git;branch=main;protocol=https \ | ||
file://adsprpcd.service \ | ||
file://cdsprpcd.service \ | ||
file://sdsprpcd.service \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't these services be part of fastrpc.git itself and a makefile can install these? That way fastrpc project can be easily extended to other distributions as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try this out. Thanks.
The FastRPC library is an open-source project, which means there are numerous potential use cases that we may not be able to test or verify on all targets. This is precisely why we rely on our users to test the library in their specific use cases/scenarios and report any issues they encounter. While we have thoroughly tested all targets and use cases that we support (will be documented as part of Release notes), we welcome and encourage the community to identify and report any issues they find. Moreover, we are happy to accept and integrate any fixes provided by the users. Therefore, there is no need for our team to test every possible target or use case available in public. |
Then could you please provide a reasonable way for e.g. me to test FastRPC on those targets. In other words, a way to build the sample app to be executed on the target without going through the trobles of QPM, SDK, etc. I am using Linux+arm64 laptop and I'm not sure how well this configuration is supported. Please provide a comprehensive testsuite that can be used to test kernel uAPI (in the manner of IGT). There is none available, as far as I remember. We have discussed all of this previously, it has been pointed out that this kind of things is usually a requirement for DMA-BUF drivers. Could you possibly share your CI details? How are you testing the kernel + userspace combos on the targets? Is it only a manual testing? Can we integrate simple FastRPC tests into the meta-qcom CI? Into Linaro's QC LT kernel CI? I have asked to stop playing NIH and to consider working with hexagonrpc / libssc as a clean implementation where it is easy to implement features, it is easy to implement testsuit, etc. I don't think anything changed FastRPC has been a gray zone for multiple reasons. One of them was the lack of activity. Everybody expected that the project is in the 'odd-fixes' state and didn't expect anything. You have opened this can and started contributing to both kernel and userspace parts. So I'd say now we are expected to be a good player. |
Hexagon SDK is a mandatory tool for Hexagon development and is publicly available for Windows and Linux. Support for ARM64 is planned and will be available soon.
FastRPC does not have CI within GitHub; instead, it is managed through separate Jenkins automation. A few FastRPC tests can be integrated into meta-qcom to help prevent potential issues.
There's no bias here. HexagonRPC delivers a clean implementation, primarily focused for sensor-specific use cases. Given that focus, the test suite implementation likely doesn't require additional tools. However, when adapted for NSP use cases, it will need all the tools required by fastRPC. Both projects serve distinct purposes, so mutual respect should be maintained as they each contribute in their own way.
FastRPC has been gaining momentum lately, with active contributions to both kernel and user space components. Our goal is to be a dependable presence in this domain. As part of Qualcomm's effort to foster community collaboration, the FastRPC user space library has been open-sourced. Over the past year, we've rolled out numerous features and bug fixes, reaffirming our commitment to long-term support. So, I’d say we’re holding our own quite well! |
So... How can I run the tests? I defeinitely do not want to have an SDK on the build server. Nor can I run QPM on our CI runners. See below for the additional details.
Yes, please. At the very least it would be nice to have a set of userspace apps that I can run on the target and verify that ADSP, CDSP and SDSP accept workloads.
Is anybody from Qualcomm contributing to that project? What is our alternative for implementing sensors access? Also with linux-msm/hexagonrpc#10 in place, the HexagonRPC will get FastRPC support. It would be nice if somebody from your team reviews that.
As I wrote, it was a gray zone. The kernel driver was merged before we established all the rules for DMA-BUF drivers. If it was submitted now, you'd have been asked to provide a completely open-source toolchain & userspace in order for people to understand and test how the driver works. All accelerator drivers provide a reference open toolchain in addition to the fully-optimized closed source one. Likewise, for the kernel driver there was a discussion which ended with Simona's "Yeah, if fastrpc just keeps growing the story will completely different." I support the idea of FastRPC driver and userspace components evolving, but if you want to evolve them, you have to follow the rules. TL;DR. Without a testing suite that one can execute on the target devices I consider merging this to be risky. There were already backwards compatibility issues both on the driver and on the userspace side. We have a known working snapshot, switching to a newer source sounds risky. |
You can find a README in the test folder that will provide you with the information.
The FastRPC library is constructed using a distinct infrastructure provided by the Hexagon SDK, specifically utilizing the stub and skel method. Many market-available applications like AI Hub by Qualcomm are developed based on this infrastructure. Hexagon RPC is designed to eliminate the need for the Hexagon SDK, which raises the point of how to support these existing market-available applications on Hexagon RPC.
Let's address the driver discussions separately, as the FastRPC user space will remain consistent for users regardless of the driver. We are currently evaluating the accell framework, and during the migration phase, it is crucial to maintain support for fastRPC to ensure uninterrupted development. Have you checked this one? https://github.com/[quic/toolchain_for_hexagon](https://github.com/quic/toolchain_for_hexagon) |
I've tried this on my X13s. Running the tests with older implementation fails because it can not find
Note, I'm running the fastrpc_test with
I'd say, that utilizing SDK or not is an orthogonal question. We should be able to run apps no matter how they were built.
I think we are on the same page here. As I wrote, my main concern is to prevent breaking existing devices.
Broken link. I will take a look, but this seems to be a Hexagon compiler. Does it comrise the full toolchain? Can I use it with the headers and libs from SDK to build an app? |
Same issue as qualcomm/fastrpc#145, @quic-ekangupt is already looking into this. falling back to adsp is a legacy code, qualcomm/fastrpc#164 for the fix.
Let's agree to disagree. Should we concentrate on refining the fastRPC library, which already offers a complete solution, or should we implement extensive changes to Hexagon RPC to support all existing applications? I believe the first option is more practical.
My bad, try this. quic/toolchain_for_hexagon. This package includes the LLVM toolchain required for Hexagon, with limited featues. Yes, this toolchain can be utilized to compile the sources for DSP. |
So.... the test doesn't work :-( Also, the tests depend on the particular version of the fastrpc, so I can not run them to compare legacy fastrpc vs new fastrpc behaviour.
I see two distinct issues: the libadsprpcd library and the daemons. I think the daemons should be able to be used interchangeably. Currently FastRPC ecosystem doesn't provide open-source support for onboard sensors, while HexagonRPC clearly does it. |
Also, for the sake of mentioning it here. @laklimov reported an issue on SM8250: qualcomm/fastrpc#146. Current FastRPC recipe has been tested on SM2850, so it is a regression. |
The tests were validated on the targets supported by meta-qcom. We will certainly enable them for other targets as well.
Hexagon RPC clearly lacks many features that FastRPC supports, so is it sufficient to converge to FastRPC? What if onboard sensors support is added to fastRPC? I think there is no need to replicate efforts in this manner. |
This issue will be resolved in the upcoming release. Additionally, we will implement CI to prevent any regressions/failures. |
I run a test on the RB3 (not Gen2) board, qualcomm/fastrpc#167 . So... No, it is not ready for the switch. I am converting this to a draft until RB3 / RB5 boards work without any significant issues. |
Could you please post somewhere a list of machines, kernel versions (from linus or linus-next), FastRPC commit ids and test results? The meta-qcom layer supports a lot of machines going back to APQ8064 generation, I doubt that you've validated all of them (e.g. because not all of them actually support FastRPC).
What should be our proposal if the users want to have both sensors and DSP offloading? |
@quic-ekangupt , pls help out here.
I have observed that the audience for sensors differs from that of NSP offloading. If necessary, I would certainly recommend using both tools. |
[I'm sorry, I edited your comment by mistake, it should be now reverted back to the state it was]
I'm thinking from the distro point of view. Can both daemons be used at the same time? |
The Sensors PD on DSP supports connections from multiple HLOS processes. Although only one default daemon can be active at a time, we can plan to enable this functionality using hexagonRPC. Before proceeding, I suggest renaming the project to sensorsRPC to make it clearer for everyone. what do you think? |
No, I think current name is good enough. The daemon is not limited to the Sensors and once linux-msm/hexagonrpc#10 lands, it should be once step closer to be able to provide support for other workloads. |
To reiterate, I have no objections to using hexagonRPC for sensor use cases, as this was the primary goal of the project, and it should remain focused on sensors. Attempting to duplicate the functionality of an existing open-source software like fastRPC with hexagonRPC would be pointless. If this is not clear, there is no further need for discussion. |
@quic-ekangupt @quic-bkumar One of the key thing to migrate from codelinaro to this fastrpc version is by making sure it works on all the upstream platforms that are already tested with codelinaro fastrpc. AFAIU, current state of the library is that it is either not tested on those platforms or it does not work on all of those platforms. Do you have the test coverage results for these platforms, starting from DB410c, DB820c, R1, RB2, RB3, RB5, X13s, QCS405, sm8250, sm8450, sm8550, sm8650, sm8750? |
Thanks, but I'd not agree here. What should we be proposing to the customers, which need both sensors and DSP offloading? Yes, we had such questions beforehand. |
TBH, fastrpc library is supposed to cater for Sensors, audio offloading and compute, if you are saying that gihub version of fastrpc library is not providing the same sensor level functionality as hexagonRPC, then we have a problem. I have not tried it my self, but if its broken, then this is another thing that should be fixed before we migrate. I dont see a point in having fastrpc lib for sensor and another one for compute.. |
The FastRPC library supports sensors, audio, and NSP, with no differences regarding sensors. |
Our current focus is on the platforms enabled on meta-qcom. We have identified an issue with the RB3 platform, for which we have created an issue and are in the process of fixing it. @quic-ekangupt will be adding the test details to the fastRPC project. We will expand it to include all the platforms mentioned above. |
meta-qcom supports almost all mentioned platforms (well, maybe except QCS405 and SM8750). |
I'm able to test meta-qcom on qcs6490 with the steps mentioned in README. I was trying to do the same for RB2 board. Here, I don't see any dedicated machine conf for qrb4210, so I'm assuming that I need to go with the generic "qcom-armv8a" machine. With this, I see that the prog_firehose_ddr.elf, rawprogram and patch files are not generated which is flashed as per the doc(https://github.com/qualcomm-linux/meta-qcom?tab=readme-ov-file#flash-images). Are the steps to boot the board(RB2) with qcom-armv8a any different which is not yet documented? |
|
Thanks! Able to boot-up RB2 board with this. I have 1 more question, to transfer any artifacts, I generally use scp. However, I am unable to see the IP address even after connecting via Ethernet (the same observation with RB3Gen2 also). Is there an alternative method to push the files? Or should all the files, including test binaries and libraries, be included in the package itself? |
First of all, could you please post outout of Second, OE has a built-in mechanism for package tests, it is called |
root@qcom-armv8a:~# ip addr First, I flashed linaro rescue image and then flashed boot img and rootfs from my local meta-qcom(+ fastrpc switch changes). Thanks for the info on 'ptest'. I'll look into this. |
So, obviously, there is no ethernet card. RB2 board has USB ethernet onboard, so you have to disconnect USB-C switch and turn the USB part of the DIP-switch to get Ethernet. |
Thanks, I'll try this. |
Switched the fastrpc recipe from the CodeLinaro version to the GitHub version. This update includes:
The GitHub version of fastrpc has been tested on multiple platforms, confirming its reliability and robustness.