On Fri, 23 Jun 2023, Jocelyn Falempe <jfalempe@xxxxxxxxxx> wrote:
> 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.
> So when no EDID is present, set 1024x768 as preferred resolution.
This conflates "monitor connected" and "EDID present", which are not
necessarily the same thing.
The fallback in drm_helper_probe_single_connector_modes() is for no
modes, but connector status is connected or unknown.
You could add a connector ->detect callback that returns disconnected
when there's no display, and the problem should go away. If there's no
->detect callback, it'll default to connected.
> Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID on DP")
> Signed-off-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>
> ---
> 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) {
Please try not to use connector->edid_blob_ptr for anything in
drivers. The logic is complicated enough as it is, with the firmware and
override EDIDs and everything, and makes future refactoring of EDID
handling harder.
BR,
Jani.
> + 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
--
Jani Nikula, Intel Open Source Graphics Center
