Repository navigation
fix CPU max freq for 4K H.265 encode on qcs8300 and qcs9100 for Config 1 - #328
Conversation
Qualcomm AI ReviewClick to expand Code ReviewReviewed Commits: a41e6ae
Signed-off-by: Varun Singhal varusing@qti.qualcomm.com Pull Request OverviewThis PR updates CPU frequency scaling configurations for camera encode operations across two target platforms (qcs8300 and qcs9100). The changes increase the maximum frequency limits for different CPU clusters to improve camera encoding performance. Files Changed Summary
Key Changes
Critical Issues
[FUNCTIONALITY] Potential Configuration Inconsistency - Medium SeverityThe updated frequency values for the default camera encode signal (SigId: 0x0003) now match the maximum performance values that were already defined in the same configuration block. This creates a potential issue:
The comment on line 97 (qcs8300) and line 96 (qcs9100) states "Default encode - 30fps", suggesting this should be a baseline configuration. However, setting maximum frequency limits to such high values for a "default" scenario may:
Recommendation: Consider whether these high frequency values are truly appropriate for the "default" 30fps encode scenario, or if they should be reserved for higher performance requirements (e.g., higher frame rates, multiple streams, or higher resolutions). If these values are intentional for default operation, update the comments to reflect the performance expectations more accurately. Fixed Code Snippet (qcs8300 example): # camera encode
# High-performance encode - 30fps (optimized for quality/latency)
- SigId: "0x0003"
Category: "0x03"
Name: "URM_SIG_CAMERA_ENCODE"
Enable: true
TargetsEnabled: ["qcs8300", "qcs8275", "sa7255p"]
Permissions: ["third_party", "system"]
Timeout: -1
Resources:
- {ResCode: "RES_CGRP_RUN_CORES", Values: [2, 0,1,2,3]}
- {ResCode: "RES_CGRP_RUN_CORES", Values: [3, 4,5,6,7]}
- {ResCode: "RES_CGRP_REL_CPU_WEIGHT", Values: [3, 90]}
- {ResCode: "RES_CGRP_HIGH_MEM", Values: [3, 1048576]}
- {ResCode: "RES_CGRP_RUN_CORES", Values: [4, 0,1,2,3,4,5,6,7]}
- {ResCode: "RES_CGRP_REL_CPU_WEIGHT", Values: [4, 150]}
- {ResCode: "RES_CGRP_CPU_LATENCY", Values: [4, -20]}
- {ResCode: "RES_CGRP_LOW_MEM", Values: [4, 507256]}
- {ResCode: "RES_CGRP_MIN_MEM", Values: [4, 116631]}
- {ResCode: "RES_SCALE_MAX_FREQ", ResInfo: "CLUSTER_LITTLE_ALL_CORES", Values: [1881600]}
- {ResCode: "RES_SCALE_MAX_FREQ", ResInfo: "CLUSTER_BIG_ALL_CORES", Values: [2054400]}
- {ResCode: "RES_SCALE_MAX_FREQ", ResInfo: "CLUSTER_PLUS_ALL_CORES", Values: [2208000]}[MAINTAINABILITY] Frequency Value Duplication - Low SeverityIn the qcs8300 configuration file, the frequency values for While this is currently just a configuration update and may be intentional for this specific signal, consider whether there's an opportunity to:
This is a low-severity observation as the configuration appears syntactically correct and the values are appropriate for the hardware specifications mentioned in the comments (lines 164-166 for qcs8300 show the CPU cluster capabilities). |
| - {ResCode: "RES_CGRP_CPU_LATENCY", Values: [4, -20]} | ||
| - {ResCode: "RES_CGRP_LOW_MEM", Values: [4, 507256]} | ||
| - {ResCode: "RES_CGRP_MIN_MEM", Values: [4, 116631]} | ||
| - {ResCode: "RES_SCALE_MAX_FREQ", ResInfo: "CLUSTER_LITTLE_ALL_CORES", Values: [1200000]} | ||
| - {ResCode: "RES_SCALE_MAX_FREQ", ResInfo: "CLUSTER_BIG_ALL_CORES", Values: [1200000]} | ||
| - {ResCode: "RES_SCALE_MAX_FREQ", ResInfo: "CLUSTER_PLUS_ALL_CORES", Values: [1200000]} | ||
| - {ResCode: "RES_SCALE_MAX_FREQ", ResInfo: "CLUSTER_LITTLE_ALL_CORES", Values: [1881600]} | ||
| - {ResCode: "RES_SCALE_MAX_FREQ", ResInfo: "CLUSTER_BIG_ALL_CORES", Values: [2054400]} | ||
| - {ResCode: "RES_SCALE_MAX_FREQ", ResInfo: "CLUSTER_PLUS_ALL_CORES", Values: [2208000]} | ||
|
|
||
| # camera encode multi-stream | ||
| # encode (0-12 streams) |
There was a problem hiding this comment.
[MAINTAINABILITY] Frequency Value Duplication - Low Severity
In the qcs8300 configuration file, the frequency values for URM_SIG_CAMERA_ENCODE (lines 115-117) are now identical to the values that appear to be defined elsewhere in the same signal configuration. This duplication could lead to maintenance issues if these values need to be updated in the future, as they would need to be changed in multiple locations.
While this is currently just a configuration update and may be intentional for this specific signal, consider whether there's an opportunity to:
- Use a reference mechanism if the configuration system supports it
- Document why these specific values are used for the default encode scenario
- Ensure consistency across similar signal configurations
This is a low-severity observation as the configuration appears syntactically correct and the values are appropriate for the hardware specifications mentioned in the comments (lines 164-166 for qcs8300 show the CPU cluster capabilities).
|
without this getting fps drop | running camera usecase for config 1 but with this change getting good fps: |
CRs-Fixed: 4489411 Signed-off-by: Varun Singhal <varusing@qti.qualcomm.com>
a41e6ae to
4d35b01
Compare
Sarbojit Ganguly (sarbojit-g)
left a comment
There was a problem hiding this comment.
Fixed the issue on config 1 with correct codes.
CRs-Fixed: 4489411