Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP

From: Thomas Zimmermann <tzimmermann@suse.de>
To: Jocelyn Falempe <jfalempe@redhat.com>,
	airlied@redhat.com, kuohsiang_chou@aspeedtech.com,
	jammy_huang@aspeedtech.com
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
Date: Tue, 4 Jul 2023 16:19:25 +0200	[thread overview]
Message-ID: <2422ef03-a309-2789-04ec-a8c4559f5024@suse.de> (raw)
In-Reply-To: <20230623094639.434293-1-jfalempe@redhat.com>

[-- Attachment #1.1: Type: text/plain, Size: 3773 bytes --]

Hi Jocelyn

Am 23.06.23 um 11:46 schrieb Jocelyn Falempe:
> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
>   EDID on DP")
> The default resolution is now 640x480 when no monitor is connected.
> But Aspeed graphics is mostly used in servers, where no monitor
> is attached. This also affects the "remote" resolution to 640x480, which is
> inconvenient, and breaks the anaconda installer.

By "remote resolution", you mean the display mode that the BMC uses?

> So when no EDID is present, set 1024x768 as preferred resolution.
> 
> Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID on DP")

This commit says that 640x480 is the designated failsafe mode if no EDID 
is available. Therefore, I think we should not override it 
unconditionally. The ast driver is no special in that case.

But I see why you're doing this change. I think any solution should be 
implemented in drm_helper_probe_single_connector_modes().

But before we solve this in the kernel, is it possible to delegate this 
to userspace? If no EDID has been given, userspace could try a 
non-failsafe display mode.

Best regards
Thomas

> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> ---
>   drivers/gpu/drm/ast/ast_mode.c | 26 ++++++++++++++++++++++++--
>   1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 36374828f6c8..8f7b7cc021c7 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -1589,9 +1589,31 @@ static const struct drm_connector_helper_funcs ast_dp501_connector_helper_funcs
>   	.get_modes = ast_dp501_connector_helper_get_modes,
>   };
>   
> +static int ast_dp_probe_single_connector_modes(struct drm_connector *connector,
> +					       uint32_t maxX, uint32_t maxY)
> +{
> +	int ret;
> +	struct drm_display_mode *mode;
> +
> +	ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
> +	/*
> +	 * When no monitor are detected, DP now default to 640x480
> +	 * As aspeed is mostly used in remote server, and DP monitors are
> +	 * rarely attached, it's better to default to 1024x768
> +	 */
> +	if (!connector->edid_blob_ptr) {
> +		list_for_each_entry(mode, &connector->modes, head) {
> +			if (mode->hdisplay == 1024 && mode->vdisplay == 768)
> +				mode->type |= DRM_MODE_TYPE_PREFERRED;
> +			drm_mode_sort(&connector->modes);
> +		}
> +	}
> +	return ret;
> +}
> +
>   static const struct drm_connector_funcs ast_dp501_connector_funcs = {
>   	.reset = drm_atomic_helper_connector_reset,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.fill_modes = ast_dp_probe_single_connector_modes,
>   	.destroy = drm_connector_cleanup,
>   	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> @@ -1678,7 +1700,7 @@ static const struct drm_connector_helper_funcs ast_astdp_connector_helper_funcs
>   
>   static const struct drm_connector_funcs ast_astdp_connector_funcs = {
>   	.reset = drm_atomic_helper_connector_reset,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.fill_modes = ast_dp_probe_single_connector_modes,
>   	.destroy = drm_connector_cleanup,
>   	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> 
> base-commit: 0adec22702d497385dbdc52abb165f379a00efba

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]