Re: [PATCH 2/7] drm: Add physical status to connector

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 11.10.24 um 10:51 schrieb Jani Nikula:
On Fri, 11 Oct 2024, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
Track the connector's physical status in addition to its logical
status. The latter is directly derived from the former and for most
connectors both values are in sync.

Server chips with BMC, such as Aspeed, Matrox and HiSilicon, often
provide virtual outputs for remote management. Without a connected
display, fbcon or userspace compositors disabek the output and stop
displaying to the BMC.
Please don't assume people know what "BMC" means.

Apologies. I'll include that information in any follow-up and the kernel docs.

FTR it's the baseboard management controller.

 https://en.wikipedia.org/wiki/Intelligent_Platform_Management_Interface#Baseboard_management_controller

Best regards
Thomas


Connectors have therefore to remain in connected status, even if the
display has been physically disconnected. Tracking both physical and
logical state in separate fields will enable that. The physical status
is transparent to drivers and clients, but changes update the epoch
counter. This generates a hotplug events for clients. Clients will then
pick up changes to resolutions supported, if any.

The ast driver already contains code to track the physical status. This
commit generalizes the logic for use with other drivers. Candidates are
mgag200 and hibmc.

This commit adds the physical status and makes the regular, logical
status a copy of it. A later change will add the flag for BMC support.

Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
---
  drivers/gpu/drm/drm_connector.c    |  1 +
  drivers/gpu/drm/drm_probe_helper.c | 13 ++++++++-----
  include/drm/drm_connector.h        |  7 +++++++
  3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index fc35f47e2849..901d73416f98 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -282,6 +282,7 @@ static int __drm_connector_init(struct drm_device *dev,
  	connector->edid_blob_ptr = NULL;
  	connector->epoch_counter = 0;
  	connector->tile_blob_ptr = NULL;
+	connector->physical_status = connector_status_unknown;
  	connector->status = connector_status_unknown;
  	connector->display_info.panel_orientation =
  		DRM_MODE_PANEL_ORIENTATION_UNKNOWN;
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 62a2e5bcb315..df44be128e72 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -373,7 +373,7 @@ drm_helper_probe_detect_ctx(struct drm_connector *connector, bool force)
  	if (WARN_ON(ret < 0))
  		ret = connector_status_unknown;
- if (ret != connector->status)
+	if (ret != connector->physical_status)
  		connector->epoch_counter += 1;
drm_modeset_drop_locks(&ctx);
@@ -409,7 +409,7 @@ drm_helper_probe_detect(struct drm_connector *connector,
ret = detect_connector_status(connector, ctx, force); - if (ret != connector->status)
+	if (ret != connector->physical_status)
  		connector->epoch_counter += 1;
return ret;
@@ -588,9 +588,11 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
  	if (connector->force) {
  		if (connector->force == DRM_FORCE_ON ||
  		    connector->force == DRM_FORCE_ON_DIGITAL)
-			connector->status = connector_status_connected;
+			connector->physical_status = connector_status_connected;
  		else
-			connector->status = connector_status_disconnected;
+			connector->physical_status = connector_status_disconnected;
+		connector->status = connector->physical_status;
+
  		if (connector->funcs->force)
  			connector->funcs->force(connector);
  	} else {
@@ -602,7 +604,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
  		} else if (WARN(ret < 0, "Invalid return value %i for connector detection\n", ret))
  			ret = connector_status_unknown;
- connector->status = ret;
+		connector->physical_status = ret;
+		connector->status = connector->physical_status;
  	}
/*
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index e3fa43291f44..37e951f04ae8 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1817,6 +1817,13 @@ struct drm_connector {
  	 */
  	struct list_head modes;
+ /**
+	 * @physical_status:
+	 * One of the drm_connector_status enums (connected, not, or unknown).
+	 * Protected by &drm_mode_config.mutex.
+	 */
I don't think that's anywhere near enough documentation. It's just
copy-paste from status. The values aren't important, the difference
between status and physical_status is.

And I think we need to have both status and physical_status
documentation explain what they mean, when they change, who can change
them, etc. And crucially, tell folks not to mess with physical_status
except in the narrow use case.

Side note, this probably indicates a few places where drivers are
messing with connector status in a way they shouldn't:

	git grep "connector->status = " -- drivers/gpu/drm

BR,
Jani.


+	enum drm_connector_status physical_status;
+
  	/**
  	 * @status:
  	 * One of the drm_connector_status enums (connected, not, or unknown).

--
--
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)




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux