[Deepin-Kernel-SIG] [linux 6.6-y] [Upstream] drm/amd/display: Only allow dig mapping to pwrseq in new asic #1457
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mainline inclusion from mainline-v6.8-rc6 category: bugfix [Why] The old asic only have 1 pwrseq hw. We don't need to map the diginst to pwrseq inst in old asic. [How] 1. Only mapping dig to pwrseq for new asic. 2. Move mapping function into dcn specific panel control component Cc: Stable <stable@vger.kernel.org> # v6.6+ Cc: Mario Limonciello <mario.limonciello@amd.com> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3122 Reviewed-by: Anthony Koo <anthony.koo@amd.com> Acked-by: Rodrigo Siqueira <rodrigo.siqueira@amd.com> Tested-by: Daniel Wheeler <daniel.wheeler@amd.com> Signed-off-by: Lewis Huang <lewis.huang@amd.com> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> (cherry picked from commit 4e73826) Signed-off-by: Wentao Guan <guanwentao@uniontech.com> Conflicts: drivers/gpu/drm/amd/display/dc/link/link_factory.c
Reviewer's GuideRefactors eDP panel power sequence handling so that DIG engine to power sequence mapping is only applied for newer DCN ASICs within panel control components, while legacy DCE/DCN301 panels use a fixed pwrseq instance. Sequence diagram for panel_cntl creation with ASIC-specific pwrseq mappingsequenceDiagram
participant LinkFactory
participant ResPoolFuncs
participant PanelCntl as panel_cntl
participant Dcn31PanelCntl as dcn31_panel_cntl
participant DcePanelCntl as dce_panel_cntl
participant Dcn301PanelCntl as dcn301_panel_cntl
LinkFactory->>LinkFactory: construct_phy(link, dc_ctx,...)
Note over LinkFactory: panel_cntl_init_data.ctx = dc_ctx
Note over LinkFactory: panel_cntl_init_data.inst = dc_edp_id_count
Note over LinkFactory: panel_cntl_init_data.eng_id = link.eng_id
LinkFactory->>ResPoolFuncs: panel_cntl_create(&panel_cntl_init_data)
alt DCN31 new_asic
ResPoolFuncs->>Dcn31PanelCntl: dcn31_panel_cntl_construct(dcn31_panel_cntl, init_data)
Dcn31PanelCntl->>Dcn31PanelCntl: map eng_id to pwrseq_inst
Dcn31PanelCntl->>PanelCntl: base.pwrseq_inst = pwrseq_inst
else DCE legacy_asic
ResPoolFuncs->>DcePanelCntl: dce_panel_cntl_construct(dce_panel_cntl, init_data)
DcePanelCntl->>PanelCntl: base.pwrseq_inst = 0
else DCN301 legacy_asic
ResPoolFuncs->>Dcn301PanelCntl: dcn301_panel_cntl_construct(dcn301_panel_cntl, init_data)
Dcn301PanelCntl->>PanelCntl: base.pwrseq_inst = 0
end
ResPoolFuncs-->>LinkFactory: panel_cntl pointer
Class diagram for updated panel_cntl initialization and ASIC-specific pwrseq mappingclassDiagram
class panel_cntl_init_data {
+dc_context* ctx
+uint32_t inst
+uint32_t eng_id
}
class panel_cntl {
+panel_cntl_funcs* funcs
+dc_context* ctx
+uint32_t inst
+uint32_t pwrseq_inst
}
class dcn31_panel_cntl {
+panel_cntl base
+void dcn31_panel_cntl_construct(dcn31_panel_cntl* dcn31_panel_cntl, panel_cntl_init_data* init_data)
}
class dce_panel_cntl {
+panel_cntl base
+void dce_panel_cntl_construct(dce_panel_cntl* dce_panel_cntl, panel_cntl_init_data* init_data)
}
class dcn301_panel_cntl {
+panel_cntl base
+void dcn301_panel_cntl_construct(dcn301_panel_cntl* dcn301_panel_cntl, panel_cntl_init_data* init_data)
}
dcn31_panel_cntl --|> panel_cntl
dce_panel_cntl --|> panel_cntl
dcn301_panel_cntl --|> panel_cntl
panel_cntl_init_data --> dcn31_panel_cntl : used by
panel_cntl_init_data --> dce_panel_cntl : used by
panel_cntl_init_data --> dcn301_panel_cntl : used by
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![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 DIGA/DIGB-to-power-sequence mapping logic in dcn31_panel_cntl_construct() duplicates the old helper you removed from link_factory; consider centralizing this mapping in a DCN-specific helper to avoid future divergence if the mapping needs to change.
- In the DCE and DCN301 panel constructors, the hard-coded
pwrseq_inst = 0for legacy ASICs is a bit implicit; a named constant or brief comment explaining why only instance 0 is valid there would make the intent clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new DIGA/DIGB-to-power-sequence mapping logic in dcn31_panel_cntl_construct() duplicates the old helper you removed from link_factory; consider centralizing this mapping in a DCN-specific helper to avoid future divergence if the mapping needs to change.
- In the DCE and DCN301 panel constructors, the hard-coded `pwrseq_inst = 0` for legacy ASICs is a bit implicit; a named constant or brief comment explaining why only instance 0 is valid there would make the intent clearer.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where old ASICs with only one power sequence hardware instance were incorrectly attempting to map DIG engine IDs to power sequence instances. The fix moves the mapping logic from common code to DCN31-specific panel control initialization, allowing old ASICs to simply use a fixed power sequence instance of 0.
Changes:
- Removed the
translate_dig_inst_to_pwrseq_inst()function from common link factory code - Changed the panel control initialization API to pass
eng_idinstead of pre-calculatedpwrseq_inst - Implemented DIG-to-pwrseq mapping in DCN31+ panel control constructors for new ASICs with multiple power sequences
- Configured old ASICs (DCE, DCN301) to use a fixed
pwrseq_instvalue of 0
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| drivers/gpu/drm/amd/display/dc/link/link_factory.c | Removed the translate_dig_inst_to_pwrseq_inst() function and changed to pass eng_id instead of pwrseq_inst to panel control initialization |
| drivers/gpu/drm/amd/display/dc/inc/hw/panel_cntl.h | Changed panel_cntl_init_data structure to use eng_id field instead of pwrseq_inst |
| drivers/gpu/drm/amd/display/dc/dcn31/dcn31_panel_cntl.c | Added DIG-to-pwrseq mapping logic for new ASICs with multiple power sequence hardware instances |
| drivers/gpu/drm/amd/display/dc/dcn301/dcn301_panel_cntl.c | Configured to use fixed pwrseq_inst = 0 for old ASIC with single power sequence |
| drivers/gpu/drm/amd/display/dc/dce/dce_panel_cntl.c | Configured to use fixed pwrseq_inst = 0 for old ASIC with single power sequence |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
mainline inclusion
from mainline-v6.8-rc6
category: bugfix
[Why]
The old asic only have 1 pwrseq hw.
We don't need to map the diginst to pwrseq inst in old asic.
[How]
Cc: Stable stable@vger.kernel.org # v6.6+
Cc: Mario Limonciello mario.limonciello@amd.com
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3122
Reviewed-by: Anthony Koo anthony.koo@amd.com
Acked-by: Rodrigo Siqueira rodrigo.siqueira@amd.com
Tested-by: Daniel Wheeler daniel.wheeler@amd.com
Signed-off-by: Lewis Huang lewis.huang@amd.com
Signed-off-by: Alex Deucher alexander.deucher@amd.com
(cherry picked from commit 4e73826)
Signed-off-by: Wentao Guan guanwentao@uniontech.com
Conflicts:
drivers/gpu/drm/amd/display/dc/link/link_factory.c
Summary by Sourcery
Scope panel power sequence handling to new DCN ASICs by moving DIG-to-power-sequence mapping into DCN-specific panel control code and simplifying legacy ASICs to use a fixed instance.
Bug Fixes:
Enhancements: