fix s3 problems and npu driver build failure #1415
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors the Cix NPU and VPU driver build rules to align with in-tree kernel Kbuild conventions, fixes include path resolution, simplifies the NPU driver Makefile to a standard obj-y style, and removes an unnecessary GPT timer reset on resume in the SKY1 clocksource driver. Sequence diagram for SKY1 GPT timer resume path without resetsequenceDiagram
participant KernelPM as Kernel_PM
participant ClockEvents as Clockevents_Core
participant Sky1Drv as sky1_tick_resume
participant GPT as SKY1_GPT_Hardware
KernelPM ->> ClockEvents: resume_timers()
ClockEvents ->> Sky1Drv: sky1_tick_resume(ced)
Sky1Drv ->> Sky1Drv: to_sky1_timer(ced)
Sky1Drv ->> GPT: sky1_gpt_setup_tctl(sky1tm)
Sky1Drv ->> GPT: sky1_shutdown(ced)
GPT -->> ClockEvents: timer configured and stopped
ClockEvents -->> KernelPM: resume complete
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Hi @Ghostor8. Thanks for your PR. I'm waiting for a deepin-community member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new NPU Makefile hardcodes the Zhouyi v3 and SKY1 variants and drops the previous BUILD_* configurability (architectures, platforms, devfreq, Android flags); consider reintroducing this selection via Kconfig options or CONFIG_* flags so different variants/platforms remain buildable.
- You now set include paths for the NPU driver in both the top-level Makefile (ccflags-y) and armchina-npu/Makefile (subdir-ccflags-y); consider consolidating these to a single mechanism to avoid redundant or conflicting include settings.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new NPU Makefile hardcodes the Zhouyi v3 and SKY1 variants and drops the previous BUILD_* configurability (architectures, platforms, devfreq, Android flags); consider reintroducing this selection via Kconfig options or CONFIG_* flags so different variants/platforms remain buildable.
- You now set include paths for the NPU driver in both the top-level Makefile (ccflags-y) and armchina-npu/Makefile (subdir-ccflags-y); consider consolidating these to a single mechanism to avoid redundant or conflicting include settings.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: opsiff The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
fix the build failure by adjusting the Makefile and Kbuild entries for the CIX NPU and VPU drivers, ensuring that all required objects are properly included and the build dependencies are consistent Signed-off-by: "Dylan.Wu" <[email protected]>
remove the reset_control_reset() call from the tick resume path to simplify the resume flow and avoid unnecessary hardware resets. Signed-off-by: Dylan.Wu<[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR addresses build failures and S3 suspend-to-RAM resume issues in the Cix SoC drivers by converting them to standard in-tree kernel build conventions.
- Fixes NPU and VPU driver build failures by updating include paths to use srctree-based patterns
- Converts NPU driver from out-of-tree module build to in-tree kernel build configuration
- Prevents S3 resume failure by removing unnecessary timer reset on resume
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| drivers/soc/cix/vpu/driver/Kbuild | Updated include paths to use srctree-based pattern for in-tree builds |
| drivers/soc/cix/npu/driver/armchina-npu/Makefile | Updated include paths to use srctree-based pattern for consistency |
| drivers/soc/cix/npu/driver/Makefile | Converted from out-of-tree to in-tree build, replacing complex conditional logic with simplified object file list |
| drivers/clocksource/timer-sky1-gpt.c | Removed unnecessary timer reset on resume that was causing S3 issues |
Comments suppressed due to low confidence (1)
drivers/soc/cix/npu/driver/armchina-npu/Makefile:11
- The aipu-objs variable defined here is unused since there's no corresponding obj- line to create the module. Additionally, this hard-coded list conflicts with the Kconfig-based conditional compilation used in the included Makefiles (lines 13-16), which define armchina_npu-y based on CONFIG options. This definition should either be removed or aligned with the parent Makefile's cix-aipu-y naming.
aipu-objs := aipu.o aipu_common.o aipu_io.o aipu_irq.o \
aipu_job_manager.o aipu_mm.o aipu_dma_buf.o aipu_priv.o \
aipu_tcb.o zhouyi/zhouyi.o zhouyi/v3.o zhouyi/v3_priv.o sky1/sky1.o
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ccflags-y += -I$(src)/armchina-npu/include \ | ||
| -I$(src)/armchina-npu/zhouyi \ | ||
| -I$(src)/armchina-npu/sky1 \ | ||
| -I$(src) \ | ||
| -I$(src)/armchina-npu |
There was a problem hiding this comment.
The include paths should use the srctree-based pattern like the other driver Makefiles in this PR. Using
| ccflags-y += -I$(src)/armchina-npu/include \ | |
| -I$(src)/armchina-npu/zhouyi \ | |
| -I$(src)/armchina-npu/sky1 \ | |
| -I$(src) \ | |
| -I$(src)/armchina-npu | |
| ccflags-y += -I$(srctree)/$(src)/armchina-npu/include \ | |
| -I$(srctree)/$(src)/armchina-npu/zhouyi \ | |
| -I$(srctree)/$(src)/armchina-npu/sky1 \ | |
| -I$(srctree)/$(src) \ | |
| -I$(srctree)/$(src)/armchina-npu |
| armchina-npu/aipu_tcb.o armchina-npu/zhouyi/zhouyi.o armchina-npu/zhouyi/v3.o armchina-npu/zhouyi/v3_priv.o armchina-npu/sky1/sky1.o | ||
|
|
There was a problem hiding this comment.
The object files are hard-coded to only include v3.o and v3_priv.o from the zhouyi subdirectory and sky1.o from the sky1 subdirectory. However, the Kconfig file defines multiple architecture options (V1, V2, V3, V3_1) and multiple SoC options (default, R329, SKY1). The armchina-npu subdirectory Makefiles use CONFIG_ARMCHINA_NPU_ARCH_* and CONFIG_SKY1 to conditionally include the correct object files. This hard-coded list should either use the same Kconfig-based conditional compilation pattern, or the Kconfig options should be removed if only V3 and SKY1 are supported.
| armchina-npu/aipu_tcb.o armchina-npu/zhouyi/zhouyi.o armchina-npu/zhouyi/v3.o armchina-npu/zhouyi/v3_priv.o armchina-npu/sky1/sky1.o | |
| armchina-npu/aipu_tcb.o armchina-npu/zhouyi/zhouyi.o | |
| cix-aipu-$(CONFIG_ARMCHINA_NPU_ARCH_V3) += armchina-npu/zhouyi/v3.o armchina-npu/zhouyi/v3_priv.o | |
| cix-aipu-$(CONFIG_SKY1) += armchina-npu/sky1/sky1.o |
3 participants
Summary by Sourcery
Align Cix SoC multimedia drivers and timer behavior with in-tree kernel expectations to fix build and resume issues.
Bug Fixes:
Enhancements: