dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/ast: Disconnect BMC if physical connector is connected
@ 2023-11-16 13:02 Thomas Zimmermann
  2023-11-16 13:43 ` Jocelyn Falempe
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Zimmermann @ 2023-11-16 13:02 UTC (permalink / raw)
  To: jfalempe, airlied, maarten.lankhorst, mripard, daniel
  Cc: stable, Thomas Zimmermann, dri-devel

Many user-space compositors fail with mode setting if a CRTC has
more than one connected connector. This is the case with the BMC
on Aspeed systems. Work around this problem by setting the BMC's
connector status to disconnected when the physical connector has
a display attached. This way compositors will only see one connected
connector at a time; either the physical one or the BMC.

Suggested-by: Jocelyn Falempe <jfalempe@redhat.com>
Fixes: e329cb53b45d ("drm/ast: Add BMC virtual connector")
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: <stable@vger.kernel.org> # v6.6+
---
 drivers/gpu/drm/ast/ast_drv.h  | 13 ++++++-
 drivers/gpu/drm/ast/ast_mode.c | 62 ++++++++++++++++++++++++++++++----
 2 files changed, 67 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 2aee32344f4a2..772f3b049c169 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -174,6 +174,17 @@ to_ast_sil164_connector(struct drm_connector *connector)
 	return container_of(connector, struct ast_sil164_connector, base);
 }
 
+struct ast_bmc_connector {
+	struct drm_connector base;
+	struct drm_connector *physical_connector;
+};
+
+static inline struct ast_bmc_connector *
+to_ast_bmc_connector(struct drm_connector *connector)
+{
+	return container_of(connector, struct ast_bmc_connector, base);
+}
+
 /*
  * Device
  */
@@ -218,7 +229,7 @@ struct ast_device {
 		} astdp;
 		struct {
 			struct drm_encoder encoder;
-			struct drm_connector connector;
+			struct ast_bmc_connector bmc_connector;
 		} bmc;
 	} output;
 
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index cb96149842851..c20534d0ef7c8 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1767,6 +1767,30 @@ static const struct drm_encoder_funcs ast_bmc_encoder_funcs = {
 	.destroy = drm_encoder_cleanup,
 };
 
+static int ast_bmc_connector_helper_detect_ctx(struct drm_connector *connector,
+					       struct drm_modeset_acquire_ctx *ctx,
+					       bool force)
+{
+	struct ast_bmc_connector *bmc_connector = to_ast_bmc_connector(connector);
+	struct drm_connector *physical_connector = bmc_connector->physical_connector;
+
+	/*
+	 * Most user-space compositors cannot handle more than one connected
+	 * connector per CRTC. Hence, we only mark the BMC as connected if the
+	 * physical connector is disconnected. If the physical connector's status
+	 * is connected or unknown, the BMC remains disconnected. This has no
+	 * effect on the output of the BMC.
+	 *
+	 * FIXME: Remove this logic once user-space compositors can handle more
+	 *        than one connector per CRTC. The BMC should always be connected.
+	 */
+
+	if (physical_connector && physical_connector->status == connector_status_disconnected)
+		return connector_status_connected;
+
+	return connector_status_disconnected;
+}
+
 static int ast_bmc_connector_helper_get_modes(struct drm_connector *connector)
 {
 	return drm_add_modes_noedid(connector, 4096, 4096);
@@ -1774,6 +1798,7 @@ static int ast_bmc_connector_helper_get_modes(struct drm_connector *connector)
 
 static const struct drm_connector_helper_funcs ast_bmc_connector_helper_funcs = {
 	.get_modes = ast_bmc_connector_helper_get_modes,
+	.detect_ctx = ast_bmc_connector_helper_detect_ctx,
 };
 
 static const struct drm_connector_funcs ast_bmc_connector_funcs = {
@@ -1784,12 +1809,33 @@ static const struct drm_connector_funcs ast_bmc_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
-static int ast_bmc_output_init(struct ast_device *ast)
+static int ast_bmc_connector_init(struct drm_device *dev,
+				  struct ast_bmc_connector *bmc_connector,
+				  struct drm_connector *physical_connector)
+{
+	struct drm_connector *connector = &bmc_connector->base;
+	int ret;
+
+	ret = drm_connector_init(dev, connector, &ast_bmc_connector_funcs,
+				 DRM_MODE_CONNECTOR_VIRTUAL);
+	if (ret)
+		return ret;
+
+	drm_connector_helper_add(connector, &ast_bmc_connector_helper_funcs);
+
+	bmc_connector->physical_connector = physical_connector;
+
+	return 0;
+}
+
+static int ast_bmc_output_init(struct ast_device *ast,
+			       struct drm_connector *physical_connector)
 {
 	struct drm_device *dev = &ast->base;
 	struct drm_crtc *crtc = &ast->crtc;
 	struct drm_encoder *encoder = &ast->output.bmc.encoder;
-	struct drm_connector *connector = &ast->output.bmc.connector;
+	struct ast_bmc_connector *bmc_connector = &ast->output.bmc.bmc_connector;
+	struct drm_connector *connector = &bmc_connector->base;
 	int ret;
 
 	ret = drm_encoder_init(dev, encoder,
@@ -1799,13 +1845,10 @@ static int ast_bmc_output_init(struct ast_device *ast)
 		return ret;
 	encoder->possible_crtcs = drm_crtc_mask(crtc);
 
-	ret = drm_connector_init(dev, connector, &ast_bmc_connector_funcs,
-				 DRM_MODE_CONNECTOR_VIRTUAL);
+	ret = ast_bmc_connector_init(dev, bmc_connector, physical_connector);
 	if (ret)
 		return ret;
 
-	drm_connector_helper_add(connector, &ast_bmc_connector_helper_funcs);
-
 	ret = drm_connector_attach_encoder(connector, encoder);
 	if (ret)
 		return ret;
@@ -1864,6 +1907,7 @@ static const struct drm_mode_config_funcs ast_mode_config_funcs = {
 int ast_mode_config_init(struct ast_device *ast)
 {
 	struct drm_device *dev = &ast->base;
+	struct drm_connector *physical_connector = NULL;
 	int ret;
 
 	ret = drmm_mode_config_init(dev);
@@ -1904,23 +1948,27 @@ int ast_mode_config_init(struct ast_device *ast)
 		ret = ast_vga_output_init(ast);
 		if (ret)
 			return ret;
+		physical_connector = &ast->output.vga.vga_connector.base;
 	}
 	if (ast->tx_chip_types & AST_TX_SIL164_BIT) {
 		ret = ast_sil164_output_init(ast);
 		if (ret)
 			return ret;
+		physical_connector = &ast->output.sil164.sil164_connector.base;
 	}
 	if (ast->tx_chip_types & AST_TX_DP501_BIT) {
 		ret = ast_dp501_output_init(ast);
 		if (ret)
 			return ret;
+		physical_connector = &ast->output.dp501.connector;
 	}
 	if (ast->tx_chip_types & AST_TX_ASTDP_BIT) {
 		ret = ast_astdp_output_init(ast);
 		if (ret)
 			return ret;
+		physical_connector = &ast->output.astdp.connector;
 	}
-	ret = ast_bmc_output_init(ast);
+	ret = ast_bmc_output_init(ast, physical_connector);
 	if (ret)
 		return ret;
 
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/ast: Disconnect BMC if physical connector is connected
  2023-11-16 13:02 [PATCH] drm/ast: Disconnect BMC if physical connector is connected Thomas Zimmermann
@ 2023-11-16 13:43 ` Jocelyn Falempe
  2023-11-21  8:55   ` Thomas Zimmermann
  0 siblings, 1 reply; 3+ messages in thread
From: Jocelyn Falempe @ 2023-11-16 13:43 UTC (permalink / raw)
  To: Thomas Zimmermann, airlied, maarten.lankhorst, mripard, daniel
  Cc: stable, dri-devel

On 16/11/2023 14:02, Thomas Zimmermann wrote:
> Many user-space compositors fail with mode setting if a CRTC has
> more than one connected connector. This is the case with the BMC
> on Aspeed systems. Work around this problem by setting the BMC's
> connector status to disconnected when the physical connector has
> a display attached. This way compositors will only see one connected
> connector at a time; either the physical one or the BMC.


Thanks for the patch.
I can't test it because I don't have physical access to a machine with 
aspeed GPU, but it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

-- 

Jocelyn


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/ast: Disconnect BMC if physical connector is connected
  2023-11-16 13:43 ` Jocelyn Falempe
@ 2023-11-21  8:55   ` Thomas Zimmermann
  0 siblings, 0 replies; 3+ messages in thread
From: Thomas Zimmermann @ 2023-11-21  8:55 UTC (permalink / raw)
  To: Jocelyn Falempe, airlied, maarten.lankhorst, mripard, daniel
  Cc: dri-devel, stable


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

Hi

Am 16.11.23 um 14:43 schrieb Jocelyn Falempe:
> On 16/11/2023 14:02, Thomas Zimmermann wrote:
>> Many user-space compositors fail with mode setting if a CRTC has
>> more than one connected connector. This is the case with the BMC
>> on Aspeed systems. Work around this problem by setting the BMC's
>> connector status to disconnected when the physical connector has
>> a display attached. This way compositors will only see one connected
>> connector at a time; either the physical one or the BMC.
> 
> 
> Thanks for the patch.
> I can't test it because I don't have physical access to a machine with 
> aspeed GPU, but it looks good to me.
> 
> Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
> 

Thanks. The patch is now in drm-misc-fixes.

Best regards
Thomas

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-11-21  8:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-16 13:02 [PATCH] drm/ast: Disconnect BMC if physical connector is connected Thomas Zimmermann
2023-11-16 13:43 ` Jocelyn Falempe
2023-11-21  8:55   ` Thomas Zimmermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).