[PATCH] drm/amdgpu: sort probed modes before adding common modes

[PATCH] drm/amdgpu: sort probed modes before adding common modes

[Mohan Marimuthu, Yogesh]

[Why]
There are monitors which can have more than one preferred mode
set. There are chances in these monitors that if common modes are
added in function amdgpu_dm_connector_add_common_modes(), these
common modes can be calculated with different preferred mode than
the one used in function decide_crtc_timing_for_drm_display_mode().
The preferred mode can be different because after common modes
are added, the mode list is sorted and this changes the order of
preferred modes in the list. The first mode in the list with
preferred flag set is selected as preferred mode. Due to this the
preferred mode selected varies.
If same preferred mode is not selected in common mode calculation
and crtc timing, then during mode set instead of setting preferred
timing, common mode timing will be applied which can cause "out of
range" message in the monitor with monitor blanking out.

[How]
Sort the modes before adding common modes. The same sorting function
is called during common mode addition and deciding crtc timing.

Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohanmarimuthu@amd.com>
Change-Id: I48036a476ad621de022e2fda5e1861e72e7e8c30
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 4a1755bce..418e3956a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4677,6 +4677,15 @@ static void amdgpu_dm_connector_ddc_get_modes(struct drm_connector *connector,
 		amdgpu_dm_connector->num_modes =
 				drm_add_edid_modes(connector, edid);
 
+		/* sorting the probed modes before calling function
+		 * amdgpu_dm_get_native_mode() since EDID can have
+		 * more than one preferred mode. The modes that are
+		 * later in the probed mode list could be of higher
+		 * and preferred resolution. For example, 3840x2160
+		 * resolution in base EDID preferred timing and 4096x2160
+		 * preferred resolution in DID extension block later.
+		 */
+		drm_mode_sort(&connector->probed_modes);
 		amdgpu_dm_get_native_mode(connector);
 	} else {
 		amdgpu_dm_connector->num_modes = 0;
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: sort probed modes before adding common modes

[Kazlauskas, Nicholas]

On 5/23/19 10:03 AM, Mohan Marimuthu, Yogesh wrote:
> 
> [Why]
> There are monitors which can have more than one preferred mode
> set. There are chances in these monitors that if common modes are
> added in function amdgpu_dm_connector_add_common_modes(), these
> common modes can be calculated with different preferred mode than
> the one used in function decide_crtc_timing_for_drm_display_mode().
> The preferred mode can be different because after common modes
> are added, the mode list is sorted and this changes the order of
> preferred modes in the list. The first mode in the list with
> preferred flag set is selected as preferred mode. Due to this the
> preferred mode selected varies.
> If same preferred mode is not selected in common mode calculation
> and crtc timing, then during mode set instead of setting preferred
> timing, common mode timing will be applied which can cause "out of
> range" message in the monitor with monitor blanking out.
> 
> [How]
> Sort the modes before adding common modes. The same sorting function
> is called during common mode addition and deciding crtc timing.
> 
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohanmarimuthu@amd.com>
> Change-Id: I48036a476ad621de022e2fda5e1861e72e7e8c30

With the patch title fixed to start with "drm/amd/display:", this is:

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

So the sorting normally seems to happen at the end of 
drm_helper_probe_single_connector_modes, or after both get_modes and 
fill_modes have been called.

So sorting the list in advance ahead of DRM shouldn't hurt here.

This should help the order at least match the order we query for the 
preferred mode later on when creating the sink for the stream, but it 
still is just masking the overall problem of not checking the right mode 
for deciding when to scale or not.

We'll probably need to either add a private flag to the common modes to 
indicate that they should be scaled or using check the scaling against 
what we thought was the native mode, ie. the one from the encoder.

Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 4a1755bce..418e3956a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4677,6 +4677,15 @@ static void amdgpu_dm_connector_ddc_get_modes(struct drm_connector *connector,
>                  amdgpu_dm_connector->num_modes =
>                                  drm_add_edid_modes(connector, edid);
> 
> +               /* sorting the probed modes before calling function
> +                * amdgpu_dm_get_native_mode() since EDID can have
> +                * more than one preferred mode. The modes that are
> +                * later in the probed mode list could be of higher
> +                * and preferred resolution. For example, 3840x2160
> +                * resolution in base EDID preferred timing and 4096x2160
> +                * preferred resolution in DID extension block later.
> +                */
> +               drm_mode_sort(&connector->probed_modes);
>                  amdgpu_dm_get_native_mode(connector);
>          } else {
>                  amdgpu_dm_connector->num_modes = 0;
> --
> 2.17.1
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx