Hi
Am 27.09.24 um 16:08 schrieb Jani Nikula:
On Mon, 05 Aug 2024, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
The VGA-BMC connector selects the VGA output if a display has been
attached to the physical connector. Otherwise it selects the BMC
output. In any case, the connector status is set to 'detected', so
that the userspace compositor displays to it.
Depending on the setting, the connector's display modes either come
from the VGA monitor's EDID or from an internal list of BMC-compatible
modes.
Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
---
drivers/gpu/drm/mgag200/mgag200_vga_bmc.c | 50 ++++++++++++++++++++++-
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_vga_bmc.c b/drivers/gpu/drm/mgag200/mgag200_vga_bmc.c
index b6b90632b3c6..3a958c3587ac 100644
--- a/drivers/gpu/drm/mgag200/mgag200_vga_bmc.c
+++ b/drivers/gpu/drm/mgag200/mgag200_vga_bmc.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
#include <drm/drm_atomic_helper.h>
+#include <drm/drm_edid.h>
#include <drm/drm_modeset_helper_vtables.h>
#include <drm/drm_probe_helper.h>
@@ -11,9 +12,54 @@ static const struct drm_encoder_funcs mgag200_dac_encoder_funcs = {
.destroy = drm_encoder_cleanup
};
+static int mgag200_vga_bmc_connector_helper_get_modes(struct drm_connector *connector)
+{
+ struct mga_device *mdev = to_mga_device(connector->dev);
+ const struct mgag200_device_info *minfo = mdev->info;
+ int count;
+
+ count = drm_connector_helper_get_modes(connector);
+
+ if (!count) {
+ /*
+ * There's no EDID data without a connected monitor. Set BMC-
+ * compatible modes in this case. The XGA default resolution
+ * should work well for all BMCs.
+ */
+ count = drm_add_modes_noedid(connector, minfo->max_hdisplay, minfo->max_vdisplay);
+ if (count)
+ drm_set_preferred_mode(connector, 1024, 768);
+ }
+
+ return count;
+}
+
+/*
+ * There's no monitor connected if the DDC did not return an EDID. Still
+ * return 'connected' as there's always a BMC. Incrementing the connector's
+ * epoch counter triggers an update of the related properties.
+ */
+static int mgag200_vga_bmc_connector_helper_detect_ctx(struct drm_connector *connector,
+ struct drm_modeset_acquire_ctx *ctx,
+ bool force)
+{
+ enum drm_connector_status old_status, status;
+
+ if (connector->edid_blob_ptr)
This is now the only place outside of drm_edid.c that uses edid_blob_ptr
for anything.
Seems like you're using it as a proxy for "had a display connected".
I wish it could be kept private to the EDID code.
No problem. I'd also prefer to change this to work like in ast, [1]
where that function tests for the connector's 'physical status'.
But I'd like to store the physical status in struct drm_connector and
have an optional update helper that detects the actual (logical) status,
as outlined at [2]. Can we talk about that?
Best regards
Thomas
[1]
https://gitlab.freedesktop.org/drm/kernel/-/blob/ae2c6d8b3b88c176dff92028941a4023f1b4cb91/drivers/gpu/drm/ast/ast_vga.c#L29
[2]
https://lore.kernel.org/dri-devel/1d16c1ae-2e27-4daa-b8a6-5eab179ef551@xxxxxxx/
BR,
Jani.
+ old_status = connector_status_connected;
+ else
+ old_status = connector_status_disconnected;
+
+ status = drm_connector_helper_detect_from_ddc(connector, ctx, force);
+
+ if (status != old_status)
+ ++connector->epoch_counter;
+ return connector_status_connected;
+}
+
static const struct drm_connector_helper_funcs mgag200_vga_connector_helper_funcs = {
- .get_modes = drm_connector_helper_get_modes,
- .detect_ctx = drm_connector_helper_detect_from_ddc
+ .get_modes = mgag200_vga_bmc_connector_helper_get_modes,
+ .detect_ctx = mgag200_vga_bmc_connector_helper_detect_ctx,
};
static const struct drm_connector_funcs mgag200_vga_connector_funcs = {
--
--
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)
