mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Chung, ChiaHsuan (Tom)" <chiahsuan.chung@amd.com>
To: Hugo Sales <hugo@hsal.es>, amd-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	harry.wentland@amd.com, sunpeng.li@amd.com, siqueira@igalia.com,
	alexander.deucher@amd.com
Subject: Re: [PATCH 1/2] drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS
Date: Sat, 24 Oct 2026 02:26:26 +0800	[thread overview]
Message-ID: <74c73b6f-a6b3-4652-8da1-b5c46ae58809@amd.com> (raw)
In-Reply-To: <20261014223041.171317-2-hugo@hsal.es>


On 10/15/2026 6:30 AM, Hugo Sales wrote:
> Currently, create_stream_for_sink() takes the native timing for a scaled
> mode from the preferred mode in connector->modes. After fbdev emulation
> probes with size limits below the native resolution, as it does when
> video= sets a smaller mode, that mode is no longer in the list
>
> If that probe is the last one before a suspend, for example after an
> HDMI hotplug, the resume finds no preferred mode, and the panel is
> driven with the scaled mode's timing instead of its own
>
> amdgpu_dm_get_native_mode() copies the preferred mode to the encoder on
> every probe, before the size limits apply. Fall back to that copy when
> connector->modes has no preferred mode. Limit this to eDP and LVDS,
> where amdgpu turns scaling on by itself. On other connectors mode_valid
> can also leave the preferred mode out, and then the first valid mode is
> the right fallback
>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5983
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Hugo Sales <hugo@hsal.es>

Hi Hugo,

Thanks for the patch and the detailed analysis.
This no longer applies to the current amd-staging-drm-next. It
conflicts with commit 3299935e65c0 ("drm/amd/display: Use separate
iterator for preferred mode lookup"), which landed after the tree you
based this on. That commit removed native_mode_found from
create_stream_for_sink() and now checks preferred_mode directly:

     if (!preferred_mode)
         preferred_mode = list_first_entry_or_null(...);

Could you rebase the series onto the latest amd-staging-drm-next and
send a v2? The change should be mechanical: test !preferred_mode
instead of !native_mode_found, and drop the native_mode_found = true
assignment. The logic stays the same. Patch 2/2 still applies cleanly.
Also, the dm_test_fs_caps_hdmi_vsdb and dm_test_fs_caps_pcon_whitelist
failures you mentioned in the cover letter were fixed by commit
5ac0b4dd2a1f ("drm/amd/display: Fix FreeSync caps tests after common
EDID VSDB switch"). With the series rebased that way, the whole
amdgpu_dm_connector KUnit suite passes on our side, including your new
tests.


Thanks,
Tom

> ---
>   .../display/amdgpu_dm/amdgpu_dm_connector.c   | 28 +++++++++++++++++++
>   1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> index 428a5f937..a1dc3e287 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> @@ -257,6 +257,8 @@ STATIC_IFN_KUNIT void update_subconnector_property(struct amdgpu_dm_connector *a
>   EXPORT_IF_KUNIT(update_subconnector_property);
>
>   STATIC_IFN_KUNIT int amdgpu_dm_connector_get_modes(struct drm_connector *connector);
> +STATIC_IFN_KUNIT struct drm_encoder *amdgpu_dm_connector_to_encoder(
> +       struct drm_connector *connector);
>
>   STATIC_IFN_KUNIT void amdgpu_dm_fbc_init(struct drm_connector *connector)
>   {
> @@ -1470,6 +1472,7 @@ create_stream_for_sink(struct drm_connector *connector,
>          struct dc_stream_state *stream = NULL;
>          struct drm_display_mode mode;
>          struct drm_display_mode saved_mode;
> +       struct drm_display_mode native_mode;
>          struct drm_display_mode *freesync_mode = NULL;
>          bool native_mode_found = false;
>          bool recalculate_timing = false;
> @@ -1529,6 +1532,31 @@ create_stream_for_sink(struct drm_connector *connector,
>                          break;
>                  }
>          }
> +
> +       /*
> +        * After a probe with size limits smaller than the preferred mode,
> +        * connector->modes no longer has that mode, and the first mode left
> +        * would become the stream timing. fbdev emulation probes with limits
> +        * that video= can set below the native size. The encoder has a copy of
> +        * the preferred mode from amdgpu_dm_get_native_mode(), made on every
> +        * probe before the limits apply, so use that copy. Only do this for eDP
> +        * and LVDS panels, which take only their native timing. On other
> +        * connectors mode_valid can also reject the preferred mode, and then
> +        * the first valid mode is the right one
> +        */
> +       if (!native_mode_found && aconnector && !aconnector->mst_root && aconnector->drm_edid &&
> +           (connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
> +            connector->connector_type == DRM_MODE_CONNECTOR_LVDS)) {
> +               struct drm_encoder *encoder = amdgpu_dm_connector_to_encoder(connector);
> +
> +               if (encoder && to_amdgpu_encoder(encoder)->native_mode.clock) {
> +                       drm_mode_copy(&native_mode, &to_amdgpu_encoder(encoder)->native_mode);
> +                       drm_mode_set_crtcinfo(&native_mode, CRTC_INTERLACE_HALVE_V);
> +                       preferred_mode = &native_mode;
> +                       native_mode_found = true;
> +               }
> +       }
> +
>          if (!native_mode_found)
>                  preferred_mode = list_first_entry_or_null(
>                                  &connector->modes,
> --
> 2.55.0
>

  reply	other threads:[~2026-10-23 18:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-14 22:30 [PATCH 0/2] drm/amd/display: Keep eDP native timing after resume with video= Hugo Sales
2026-10-14 22:30 ` [PATCH 1/2] drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS Hugo Sales
2026-10-23 18:26   ` Chung, ChiaHsuan (Tom) [this message]
2026-10-24 16:33     ` [PATCH v2 0/2] drm/amd/display: Keep eDP native timing after resume with video= Hugo Sales
2026-10-24 16:33       ` [PATCH v2 1/2] drm/amd/display: Fall back to the native mode, not the first, on eDP/LVDS Hugo Sales
2026-10-24 16:33       ` [PATCH v2 2/2] drm/amd/display: Add KUnit tests for the eDP/LVDS native mode fallback Hugo Sales
2026-10-14 22:30 ` [PATCH " Hugo Sales

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=74c73b6f-a6b3-4652-8da1-b5c46ae58809@amd.com \
    --to=chiahsuan.chung@amd.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=harry.wentland@amd.com \
    --cc=hugo@hsal.es \
    --cc=linux-kernel@vger.kernel.org \
    --cc=siqueira@igalia.com \
    --cc=sunpeng.li@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®