Repository navigation
Conversation
Reviewer's GuideBackports i915 fixes for Xe2LPD/Xe2 CDCLK and MDCLK coordination, moving MBUS joining and DBUF programming into atomic state and carefully ordered DDB/plane-update commit phases while restructuring CRTC update sequencing. Sequence diagram for coordinated Xe2 CDCLK and MBUS updatesequenceDiagram
participant Atomic as AtomicCommit
participant CDCLK as intel_set_cdclk_pre_plane_update
participant DBUF as intel_dbuf_mbus_pre_ddb_update
participant DDB as intel_mbus_dbox_update
participant HW as MBUS_and_DBUF_Registers
Atomic->>CDCLK: intel_set_cdclk_pre_plane_update(state)
CDCLK->>HW: Program CDCLK and MDCLK ratio
Atomic->>DBUF: intel_dbuf_mbus_pre_ddb_update(state)
DBUF->>HW: intel_dbuf_mbus_join_update(state)
DBUF->>DDB: intel_mbus_dbox_update(state)
DBUF->>HW: intel_dbuf_mdclk_min_tracker_update(state)
Sequence diagram for ordered MBUS unjoin during DDB updatesequenceDiagram
participant Atomic as AtomicCommit
participant DDB as intel_dbuf_mbus_post_ddb_update
participant HW as MBUS_and_DBUF_Registers
participant Pipe as ActiveCRTC
Atomic->>DDB: intel_dbuf_mbus_post_ddb_update(state)
DDB->>HW: intel_dbuf_mdclk_min_tracker_update(state)
DDB->>HW: intel_mbus_dbox_update(state)
DDB->>HW: intel_dbuf_mbus_join_update(state)
DDB->>Pipe: intel_crtc_wait_for_next_vblank(crtc)
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 |
Split intel_update_crtc() into two parts such that the first part performs all the non-vblank evasion preparatory stuff, and the second part just does the vblank evasion stuff. For now we just call these back to back so that there is no funcitonal change. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230907122541.32261-3-ville.syrjala@linux.intel.com Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> (cherry picked from commit c39c935) (cherry picked from commit 324f3a119af9f1822d50232097939c32b3f85c18) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
…sables() Copy the pipe bitmask based approach from skl_commit_modeset_enables() into intel_commit_modeset_disables(). This avoids doing so many duplicated checks in all the loops, and also let's WARN at the end if we screwed up somewhere and forgot to disable some pipe. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240305084730.19182-2-ville.syrjala@linux.intel.com Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> (cherry picked from commit 5efb03c) Signed-off-by: Rakshith M O <rakshith.m.o@intel.com> Reviewed-by: Manasi Navare <navaremanasi@chromium.org> (cherry picked from commit acb031ed613d2d77e6f23a11e5627c79dabd02fa) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Perform all the intel_pre_update_crtc() stuff for all pipes first, and only then do the intel_update_crtc() vblank evasion stuff for every pipe back to back. This should make it more likely that the plane updates from multiple pipes happen on the same frame (assuming the pipes are running in sync, eg. due to bigjoiner or port sync). Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20230907122541.32261-4-ville.syrjala@linux.intel.com Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> (cherry picked from commit c610e84) (cherry picked from commit 7b97392bda11e57c4ded41768971796956dd451a) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Follow in the footsteps of commit c610e84 ("drm/i915: Do plane/etc. updates more atomically across pipes") and do the plane disables back to back for all pipes also when we are disabling pipes. This should provide for a potentially more atomic user experience, which might be especially nice when using joiner or tiled displays. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240305084730.19182-3-ville.syrjala@linux.intel.com Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> (cherry picked from commit 4bc6745) Signed-off-by: Rakshith M O <rakshith.m.o@intel.com> Reviewed-by: Manasi Navare <navaremanasi@chromium.org> (cherry picked from commit cfd72221356477ddcb9207e082d5685ae03d233a) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Xe2LPD always selects the CDCLK PLL as source for the MDCLK. Because of that, the ratio between MDCLK and CDCLK is not be constant anymore. As such, make sure to have the current ratio available in intel_dbuf_state so that it can be used during dbuf programming. Note that we write-lock the global state instead of serializing to a hardware commit because a change in the ratio should be rather handled in the CDCLK change sequence, which will need to take care of updating the necessary registers in that case. We will implement that in upcoming changes. That said, changes in the MBus joining state should be handled by the DBUF/MBUS logic, just like it is already done, but the logic will need to know the ratio to properly update the registers. v2: - Make first sentence of commit message more intelligible. (Matt) Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240312163639.172321-6-gustavo.sousa@intel.com Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> (cherry picked from commit 9161e31) Conflicts: drivers/gpu/drm/i915/display/intel_cdclk.c (cherry picked from commit e446734708a5fa11132d024043df6db5c1549561) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Commit 394b4b7 ("drm/i915/lnl: Add CDCLK table") and commit 3d3696c ("drm/i915/lnl: Start using CDCLK through PLL") started adding support for CDCLK programming support for Xe2LPD. One final piece is missing, which is the programming necessary for changed in the ratio between MDCLK and CDCLK. Let's do that now. BSpec instructs us to update MBUS_CTL and DBUF_CTL_S* registers when the ratio between MDCLK and CDCLK changes. The updates must be done before changing the CDCLK when decreasing the frequency; or after it when increasing the frequency. Ratio-related updates to MBUS_CTL also depend on the state of MBus joining, so they are performed by either CDCLK change sequence or by changes in MBus joining. Since one might happen independently of the other, we need to make sure that both logics see the necessary state values when programming that register. MBus joining logic needs to know the MDCLK:CDCLK ratio and that's already provided via mdclk_cdclk_ratio field of struct intel_dbuf_state. For the CDCLK logic, we need to have something similar: we need to propagate the status of MBus joining to struct intel_cdclk_state. Do that by adding the field joined_mbus to struct intel_cdclk_config. (Preferably, that field would be added to intel_cdclk_state, however currently only intel_cdclk_config is passed down to the functions that do the register programming. We might revisit this decision if we find that refactoring the code to pass the whole intel_cdclk_state is worth it.) Bspec: 68864, 68868, 69090, 69482 Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240312163639.172321-7-gustavo.sousa@intel.com Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> (cherry picked from commit c834a08) (cherry picked from commit 21a6eb124db1e9e4319adfb840d513d0af066381) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
intel_mbus_dbox_update() will become static soon. Relocate it into a place that avoids having to add a forward declaration for it. Reviewed-by: Uma Shankar <uma.shankar@intel.com> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240402155016.13733-7-ville.syrjala@linux.intel.com (cherry picked from commit 359e4f4) (cherry picked from commit ad1b6ecd14a34bddbe351d9a47b5a28ad02dd4af) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Extact the stuff that writes the joining bits in MBUS_CTL into its own function. Will help with correctly sequencing the operations done during mbus programming. Reviewed-by: Uma Shankar <uma.shankar@intel.com> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240402155016.13733-8-ville.syrjala@linux.intel.com (cherry picked from commit 8d7fc26) (cherry picked from commit 36babad2404d11a89d0c1f383d5143e456de5cd8) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Extract the stuff that writes the dbuf/mbus ratio stuff into its own function. Will help with correctly sequencing the operations done during mbus programming. Reviewed-by: Uma Shankar <uma.shankar@intel.com> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240402155016.13733-9-ville.syrjala@linux.intel.com (cherry picked from commit a2e83f2) (cherry picked from commit 3c5b231591bd5295fe3e8dbd7b460f6e265bb18e) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Add some debugs so that we can actually observe what is actually happening during the mbus/dbuf programming steps. We can just shove them into fairly low level functions as none of them are called during any critical sections/etc. Reviewed-by: Uma Shankar <uma.shankar@intel.com> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240402155016.13733-10-ville.syrjala@linux.intel.com (cherry picked from commit d91254e) (cherry picked from commit 91a5706a0e577b354890a99e1d89e4dd0ca76285) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
In order to make sure we are not breaking the proper sequence
let's do updates step by step and don't change MBUS join value
during MDCLK/CDCLK programming stage.
MBUS join programming would be taken care by pre/post ddb hooks.
v2: - Reworded comment about using old mbus_join value in
intel_set_cdclk(Ville Syrjälä)
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
[v3: vsyrjala: rebase on top of cdclk changes, reword a bit more]
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240402155016.13733-11-ville.syrjala@linux.intel.com
(cherry picked from commit 48b8530)
(cherry picked from commit 7634402374da312cc9b9dd9408d8a4529bcb3794)
Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
The current cdclk/mbus programming sequence is as follows: 1. intel_set_cdclk_pre_plane_update() 2. update_mbus_pre_enable() 3. intel_set_cdclk_post_plane_update() when the actual mdclk/cdclk programming is postponed to intel_set_cdclk_post_plane_update() we must keep using the old mdclk/cdclk ratio during update_mbus_pre_enable(). This guarantees the programmed ratio matches the rest of the hardware state (mdlk/cdclk/mbus joining). v2: Extracted from the vblank synchronized mbus programming patch Cc: Gustavo Sousa <gustavo.sousa@intel.com> Reviewed-by: Uma Shankar <uma.shankar@intel.com> #v1 Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240402155016.13733-12-ville.syrjala@linux.intel.com (cherry picked from commit bea9898) (cherry picked from commit df13785f21d5d9c69492c772c4a0518f4a4d31d2) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Currently we can't change MBUS join status without doing a modeset,
because we are lacking mechanism to synchronize those with vblank.
However then this means that we can't do a fastset, if there is a need
to change MBUS join state. Fix that by implementing such change.
We already call correspondent check and update at pre_plane dbuf update,
so the only thing left is to have a non-modeset version of that.
If active pipes stay the same then fastset is possible and only MBUS
join state/ddb allocation updates would be committed.
The full mbus/cdclk sequence will look as follows:
1. disable pipes
2. increase cdclk if necessary
2.1 reprogram cdclk
2.2 update dbuf tracker value
3. enable mbus joining if necessary
3.1 update mbus_ctl
3.2 update dbuf tracker value
4. reallocate dbuf for planes on active pipes
5. disable mbus joining if necessary
5.1 update dbuf tracker value
5.2 update mbus_ctl
6. enable pipes
7. decrease cdclk if necessary
7.1 update dbuf tracker value
7.2 reprogram cdclk
And in order to keep things in sync we need:
Step 2:
- mbus_join == old
- mdclk/cdclk ratio == new
Step 3:
- mbus_join == new
- mdclk/cdclk ratio == old when cdclk is changing in step 7
- mdclk/cdclk ratio == new when cdclk is changing in step 2
Step 5:
- mbus_join == new
- mdclk/cdclk ratio == old when cdclk is changing in step 7
- mdclk/cdclk ratio == new when cdclk is changing in step 2
Step 7:
- mbus_join == new
- mdclk/cdclk ratio == new
v2: - Removed redundant parentheses(Ville Syrjälä)
- Constified new_crtc_state in intel_mbus_joined_pipe(Ville Syrjälä)
- Removed pipe_select variable(Ville Syrjälä)
[v3: vsyrjala: Correctly sequence vs. cdclk updates,
properly describe the full sequence,
shuffle code around to make the diff more legible,
streamline a few things]
[v4: vsyrjala: Move the intel_cdclk_is_decreasing_later() stuff
to a separate patch]
Cc: Gustavo Sousa <gustavo.sousa@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com> #v3
Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Co-developed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240402155016.13733-13-ville.syrjala@linux.intel.com
(cherry picked from commit 8c5dbb5)
(cherry picked from commit fea3cb41110ea4b4cf12a09d44c0a22a912d9a51)
Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
No point in throwing around u8 when we're dealing with just an integer. Use a plain old boring 'int'. Reviewed-by: Uma Shankar <uma.shankar@intel.com> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240402155016.13733-14-ville.syrjala@linux.intel.com (cherry picked from commit 96c420d) Conflicts: drivers/gpu/drm/i915/display/intel_cdclk.c (cherry picked from commit 9bfa2b4ff064a27a02a836422918fec6123b6f6e) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
if the new dbuf slices are a superset of the old dbuf slices then we don't have to do anything in intel_dbuf_post_plane_update(). Restructure the code to skip such redundant dbuf slice updates. The main benefit is slightly less confusing logs. Reviewed-by: Uma Shankar <uma.shankar@intel.com> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240402155016.13733-15-ville.syrjala@linux.intel.com (cherry picked from commit c8361cd) (cherry picked from commit ce9c02ed9666bea7c76afcae73e3de2a14e2e021) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
df8f840 to
c6b4f07
Compare
There was a problem hiding this comment.
🟡 Changes recommended
A joined-to-unjoined MBUS transition with no active pipes can look up pipe -1 and dereference a NULL CRTC.
1 open issue
What changed in this PR
Backports upstream i915 display fixes into Linux 6.6.y to coordinate Xe2 MBUS, display-buffer, watermark, and display-clock transitions.
Changes:
- Track MDCLK/CDCLK ratios and MBUS joining through hardware readout and atomic state updates.
- Reorder MBUS and DBUF programming around plane updates and vblank synchronization.
- Separate CRTC update preparation from plane arming to coordinate multi-pipe changes.
| File | Description |
|---|---|
drivers/gpu/drm/i915/display/skl_watermark.h |
Adds ratio state and MBUS transition interfaces. |
drivers/gpu/drm/i915/display/skl_watermark.c |
Implements ratio programming and staged MBUS/DBUF updates. |
drivers/gpu/drm/i915/display/skl_watermark_regs.h |
Defines MBUS translation-throttle register fields. |
drivers/gpu/drm/i915/display/intel_display.c |
Reorders CRTC, plane, and MBUS update stages. |
drivers/gpu/drm/i915/display/intel_cdclk.h |
Extends clock state and exposes transition helpers. |
drivers/gpu/drm/i915/display/intel_cdclk.c |
Coordinates ratio programming and MBUS state with clock changes. |
Review effort: Max
| struct intel_crtc *crtc; | ||
|
|
||
| drm_WARN_ON(&i915->drm, !dbuf_state->joined_mbus); | ||
| drm_WARN_ON(&i915->drm, !is_power_of_2(dbuf_state->active_pipes)); |

Summary by Sourcery
Backport upstream i915 display fixes to synchronize CDCLK, DBUF, and MBUS state changes during atomic modesets.
Bug Fixes:
Enhancements:
Chores: