Hi Jocelyn
Am 24.01.25 um 09:45 schrieb Jocelyn Falempe:
The physical_status of ast_dp is not reliable, as it reports as
connected even when no monitor is connected.
This status comes from VGACRDF, which is undocumented unfortunately. So
IDK the exact semantics. Do you know if the other case can also happen
where a connected monitor is not reported?
This makes the default
BMC resolution to be 640x480 for remote access.
So consider that if there is no edid, no monitor is connected, and
add the BMC 1024x768 default resolution.
I've debugged this regression on ast_dp, but as dp501 is similar, I
fixed both in this patch.
This regression was likely introduced by commit 2281475168d2
("drm/ast: astdp: Perform link training during atomic_enable")
But I fixed it in the BMC get_modes handling.
Signed-off-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>
Fixes: bbad0090b9f4 ("drm/ast: astdp: Transparently handle BMC support")
---
drivers/gpu/drm/ast/ast_dp.c | 14 +++++++-------
drivers/gpu/drm/ast/ast_dp501.c | 14 +++++++-------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index 0e282b7b167c..6c8ea95a2230 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -361,19 +361,19 @@ static const struct drm_encoder_helper_funcs ast_astdp_encoder_helper_funcs = {
static int ast_astdp_connector_helper_get_modes(struct drm_connector *connector)
I don't think this is the right place to fix the problem. The field
physical_status should always contain the correct physical status. So
the fix should go into ast_dp_connector_helper_detect_ctx(). There's [1]
something like
if (ast_dp_status_is_connected(ast))
status = connected
and that's where it should read the EDID without updating the
connector's EDID property. Example code:
if (ast_dp_status_is_connected(ast)) {
edid = drm_edid_read_custom(/* like in get_modes */)
if (drm_edid_valid(edid))
status = connected
drm_edid_free(edid)
}
The EDID test could also go into _is_connected() directly. A comment
about false positives from VGACRDF might make sense as well.
[1]
https://elixir.bootlin.com/linux/v6.12.6/source/drivers/gpu/drm/ast/ast_dp.c#L397
{
struct ast_connector *ast_connector = to_ast_connector(connector);
+ struct ast_device *ast = to_ast_device(connector->dev);
+ const struct drm_edid *drm_edid = NULL;
int count;
- if (ast_connector->physical_status == connector_status_connected) {
- struct ast_device *ast = to_ast_device(connector->dev);
- const struct drm_edid *drm_edid;
-
+ if (ast_connector->physical_status == connector_status_connected)
drm_edid = drm_edid_read_custom(connector, ast_astdp_read_edid_block, ast);
- drm_edid_connector_update(connector, drm_edid);
+
+ drm_edid_connector_update(connector, drm_edid);
+
+ if (drm_edid) {
count = drm_edid_connector_add_modes(connector);
drm_edid_free(drm_edid);
} else {
- drm_edid_connector_update(connector, NULL);
-
/*
* There's no EDID data without a connected monitor. Set BMC-
* compatible modes in this case. The XGA default resolution
diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 9e19d8c17730..c92db65e3f20 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
I'd rather leave this out. The detection works differently for DP501.
Best regards
Thomas
@@ -504,19 +504,19 @@ static const struct drm_encoder_helper_funcs ast_dp501_encoder_helper_funcs = {
static int ast_dp501_connector_helper_get_modes(struct drm_connector *connector)
{
struct ast_connector *ast_connector = to_ast_connector(connector);
+ struct ast_device *ast = to_ast_device(connector->dev);
+ const struct drm_edid *drm_edid = NULL;
int count;
- if (ast_connector->physical_status == connector_status_connected) {
- struct ast_device *ast = to_ast_device(connector->dev);
- const struct drm_edid *drm_edid;
-
+ if (ast_connector->physical_status == connector_status_connected)
drm_edid = drm_edid_read_custom(connector, ast_dp512_read_edid_block, ast);
- drm_edid_connector_update(connector, drm_edid);
+
+ drm_edid_connector_update(connector, drm_edid);
+
+ if (drm_edid) {
count = drm_edid_connector_add_modes(connector);
drm_edid_free(drm_edid);
} else {
- drm_edid_connector_update(connector, NULL);
-
/*
* There's no EDID data without a connected monitor. Set BMC-
* compatible modes in this case. The XGA default resolution
base-commit: 798047e63ac970f105c49c22e6d44df901c486b2
--
--
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)
