Skip to content

Conversation

@opsiff
Copy link
Member

@opsiff opsiff commented Jun 16, 2025

This reverts commit cfb2d41 since it causes regressions on certain configs. Revert until the issue can be isolated and debugged.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4238

Acked-by: Alex Deucher alexander.deucher@amd.com

Cc: stable@vger.kernel.org
(cherry picked from commit 1b824ee)

Summary by Sourcery

Revert the recent liberalized vmin/vmax update for Freesync to prevent regression issues

Bug Fixes:

  • Revert commit cfb2d41 that altered vmin/vmax update conditions for Freesync
  • Restore original conditional checks for VRR, PSR, and replay when adjusting vmin/vmax

@sourcery-ai
Copy link

sourcery-ai bot commented Jun 16, 2025

Reviewer's Guide

This PR reverts the more liberal vmin/vmax update logic introduced for freesync in the high IRQ handler by removing the extra replay and PSR checks and restoring the original unconditional adjustment when freesync is active variable.

Sequence diagram for reverted vmin/vmax update logic in Freesync IRQ handler

sequenceDiagram
    participant IRQ_Handler as dm_crtc_high_irq
    participant Stream as dm_irq_params.stream
    participant FreesyncModule as freesync_module
    participant DC as dc

    IRQ_Handler->>Stream: Check vrr_params.supported && freesync_config.state == VRR_STATE_ACTIVE_VARIABLE
    alt Condition true
        IRQ_Handler->>FreesyncModule: mod_freesync_handle_v_update()
        IRQ_Handler->>DC: dc_stream_adjust_vmin_vmax()
    end
Loading

File-Level Changes

Change Details Files
Reverted liberal vmin/vmax adjustment conditions in dm_crtc_high_irq
  • Removed replay_en, psr_en and fs_active_var_en boolean declarations
  • Dropped the nested if that gated dc_stream_adjust_vmin_vmax on replay/PSR flags
  • Restored a single unconditional call to dc_stream_adjust_vmin_vmax when freesync is active
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

commit 1b824ee upstream.

This reverts commit cfb2d41 since it
causes regressions on certain configs. Revert until the issue can be
isolated and debugged.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4238
Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 3f369af6750583728c8766874ba8a383a16434d1)
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from opsiff. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@opsiff opsiff force-pushed the linux-6.6.y-2025-06-17-revert branch from a01a321 to cc3554d Compare June 16, 2025 16:20
@opsiff opsiff merged commit cd249f4 into deepin-community:linux-6.6.y Jun 16, 2025
6 of 7 checks passed
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 代码简化

    • 在修改后的代码中,去掉了对replay_enpsr_en的检查,直接调用了dc_stream_adjust_vmin_vmax函数。这可能是由于replay_enpsr_en的检查逻辑已经被包含在VRR_STATE_ACTIVE_VARIABLE状态中,或者这些检查不再必要。
  2. 注释清理

    • 删除了原有的注释,这些注释可能已经过时或者不再准确。建议保留或者更新注释,以便其他开发者理解代码逻辑。
  3. 代码可读性

    • 修改后的代码更加简洁,但是可能需要确保相关的逻辑变更已经经过充分测试,以避免引入新的问题。
  4. 错误处理

    • 检查mod_freesync_handle_v_updatedc_stream_adjust_vmin_vmax函数调用是否有可能失败,并确保在失败时进行适当的错误处理。
  5. 锁的粒度

    • 确保在spin_lock_irqsavespin_unlock_irqrestore之间的代码执行时间尽可能短,以减少对系统性能的影响。
  6. 代码风格

    • 确保代码风格与项目中的其他代码保持一致,包括缩进、空格使用等。
  7. 测试

    • 在进行此类代码修改时,建议添加单元测试或集成测试,以确保修改不会引入新的错误。

总体来说,代码的简化可能提高了可读性,但是需要确保这种简化的正确性和必要性。同时,建议进行充分的测试和代码审查,以确保代码的质量和稳定性。

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @opsiff - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR reverts the recent change that broadened vmin/vmax updates for Freesync, restoring the original conditional logic to prevent regressions on certain configurations.

  • Removed replay and PSR checks in the vmin/vmax update path
  • Require both VRR support and active-variable Freesync state for updates
  • Simplified the if condition and call to dc_stream_adjust_vmin_vmax
Comments suppressed due to low confidence (2)

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:615

  • [nitpick] Indentation of this line is deeper than the other condition lines; consider aligning all parts of the multi-line if condition uniformly for readability.
		    VRR_STATE_ACTIVE_VARIABLE) {

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:620

  • [nitpick] Split arguments across two lines; consider placing each argument on its own line with consistent indentation or keep the call on one line if it fits within line length limits.
		dc_stream_adjust_vmin_vmax(adev->dm.dc, acrtc->dm_irq_params.stream,

opsiff added a commit to opsiff/UOS-kernel that referenced this pull request Dec 1, 2025
…OUGH to default

deepin inclusion
category: other

Removed CONFIG_IOMMU_DEFAULT_PASSTHROUGH=y from defconfig to
enforce strict DMA isolation by default.
This change aligns ARM64 desktop kernel configuration with other arch.

The config also affect cix in link [1].

Link: deepin-community#1335
Fixes: 7821b9fb89ca ("add deepin-community#880 config")
Fixes: ce41a38 ("arm64: Add deepin_arm64_desktop_defconfig")
Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
opsiff added a commit to opsiff/UOS-kernel that referenced this pull request Dec 1, 2025
…OUGH to default

deepin inclusion
category: other

Removed CONFIG_IOMMU_DEFAULT_PASSTHROUGH=y from defconfig to
enforce strict DMA isolation by default.
This change aligns ARM64 desktop kernel configuration with other arch.

The config also affect cix in link [1].

Note that may bring some affect in some phytium FT2000 or Kunpeng 920 device.

Link: deepin-community#1335
Fixes: 7821b9fb89ca ("add deepin-community#880 config")
Fixes: ce41a38 ("arm64: Add deepin_arm64_desktop_defconfig")
Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
opsiff added a commit to opsiff/UOS-kernel that referenced this pull request Dec 1, 2025
…OUGH to default

deepin inclusion
category: other

Removed CONFIG_IOMMU_DEFAULT_PASSTHROUGH=y from defconfig to
enforce strict DMA isolation by default.
This change aligns ARM64 desktop kernel configuration with other arch.

The config also affect cix in link [1].

Note that may bring some affect in some phytium FT2000 or Kunpeng 920 device.

Link: deepin-community#1335
Fixes: 7821b9fb89ca ("add deepin-community#880 config")
Fixes: ce41a38 ("arm64: Add deepin_arm64_desktop_defconfig")
Reported-by: Dylan.Wu" <Dylan.Wu@cixtech.com>
Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants