Re: [PATCH] drm/ast: add multiple connectors support

[Thomas Zimmermann <tzimmermann@xxxxxxx>]

Hi

Am 11.07.24 um 11:01 schrieb oushixiong1025@xxxxxxx:
> From: Shixiong Ou <oushixiong@xxxxxxxxxx>
>
> [WHY]
> The AST2600 tx_chip_types will be detected as AST_TX_DP, but some BMC
> boards that use AST2600 use the VGA interface instead of the DP interface.
> In this case, it will use Virtual connector as the DP is disconnected.
>
> [HOW]
> Allows multiple physical connectors to exist at the same time.
>
> Signed-off-by: Shixiong Ou <oushixiong@xxxxxxxxxx>
> ---
>   drivers/gpu/drm/ast/ast_drv.h  |  6 ++++-
>   drivers/gpu/drm/ast/ast_main.c |  8 +++----
>   drivers/gpu/drm/ast/ast_mode.c | 40 ++++++++++++++++++++--------------
>   3 files changed, 33 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index ba3d86973995..e326124b3fec 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -150,9 +150,13 @@ static inline struct ast_plane *to_ast_plane(struct drm_plane *plane)
>    * BMC
>    */
>   
> +#define MAX_CONNECTORS 2
> +
>   struct ast_bmc_connector {
>   	struct drm_connector base;
> -	struct drm_connector *physical_connector;
> +
> +	struct drm_connector *physical_connectors[MAX_CONNECTORS];
> +	int count;

It won't work like that. Due to userspace limitations, only one 
connector can be reported as connected as at a time. So we have to build 
a chain of connectors. I had this on my TODO list anyway, so I can also 
prioritize.

Best regards
Thomas

>   };
>   
>   static inline struct ast_bmc_connector *
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 0637abb70361..428529749ae6 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -85,7 +85,7 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool need_post)
>   	if (!need_post) {
>   		jreg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xa3, 0xff);
>   		if (jreg & 0x80)
> -			ast->tx_chip_types = AST_TX_SIL164_BIT;
> +			ast->tx_chip_types |= AST_TX_SIL164_BIT;
>   	}
>   
>   	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast)) {
> @@ -97,7 +97,7 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool need_post)
>   		jreg = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd1, 0xff);
>   		switch (jreg) {
>   		case 0x04:
> -			ast->tx_chip_types = AST_TX_SIL164_BIT;
> +			ast->tx_chip_types |= AST_TX_SIL164_BIT;
>   			break;
>   		case 0x08:
>   			ast->dp501_fw_addr = drmm_kzalloc(dev, 32*1024, GFP_KERNEL);
> @@ -110,12 +110,12 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool need_post)
>   			}
>   			fallthrough;
>   		case 0x0c:
> -			ast->tx_chip_types = AST_TX_DP501_BIT;
> +			ast->tx_chip_types |= AST_TX_DP501_BIT;
>   		}
>   	} else if (IS_AST_GEN7(ast)) {
>   		if (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD1, TX_TYPE_MASK) ==
>   		    ASTDP_DPMCU_TX) {
> -			ast->tx_chip_types = AST_TX_ASTDP_BIT;
> +			ast->tx_chip_types |= AST_TX_ASTDP_BIT;
>   			ast_dp_launch(&ast->base);
>   		}
>   	}
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 6695af70768f..31a49d32e506 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -1717,7 +1717,8 @@ static int ast_bmc_connector_helper_detect_ctx(struct drm_connector *connector,
>   					       bool force)
>   {
>   	struct ast_bmc_connector *bmc_connector = to_ast_bmc_connector(connector);
> -	struct drm_connector *physical_connector = bmc_connector->physical_connector;
> +	struct drm_connector *physical_connector;
> +	int i, count = bmc_connector->count;
>   
>   	/*
>   	 * Most user-space compositors cannot handle more than one connected
> @@ -1730,10 +1731,13 @@ static int ast_bmc_connector_helper_detect_ctx(struct drm_connector *connector,
>   	 *        than one connector per CRTC. The BMC should always be connected.
>   	 */
>   
> -	if (physical_connector && physical_connector->status == connector_status_disconnected)
> -		return connector_status_connected;
> +	for (i = 0; i < count; i++) {
> +		physical_connector = bmc_connector->physical_connectors[i];
> +		if (physical_connector && physical_connector->status == connector_status_connected)
> +			return connector_status_disconnected;
> +	}
>   
> -	return connector_status_disconnected;
> +	return connector_status_connected;
>   }
>   
>   static int ast_bmc_connector_helper_get_modes(struct drm_connector *connector)
> @@ -1756,10 +1760,11 @@ static const struct drm_connector_funcs ast_bmc_connector_funcs = {
>   
>   static int ast_bmc_connector_init(struct drm_device *dev,
>   				  struct ast_bmc_connector *bmc_connector,
> -				  struct drm_connector *physical_connector)
> +				  struct drm_connector **physical_connector,
> +				  int count)
>   {
>   	struct drm_connector *connector = &bmc_connector->base;
> -	int ret;
> +	int i, ret;
>   
>   	ret = drm_connector_init(dev, connector, &ast_bmc_connector_funcs,
>   				 DRM_MODE_CONNECTOR_VIRTUAL);
> @@ -1768,13 +1773,16 @@ static int ast_bmc_connector_init(struct drm_device *dev,
>   
>   	drm_connector_helper_add(connector, &ast_bmc_connector_helper_funcs);
>   
> -	bmc_connector->physical_connector = physical_connector;
> +	for (i = 0; i < count; i++)
> +		bmc_connector->physical_connectors[i] = physical_connector[i];
> +	bmc_connector->count = count;
>   
>   	return 0;
>   }
>   
>   static int ast_bmc_output_init(struct ast_device *ast,
> -			       struct drm_connector *physical_connector)
> +			       struct drm_connector **physical_connector,
> +			       int count)
>   {
>   	struct drm_device *dev = &ast->base;
>   	struct drm_crtc *crtc = &ast->crtc;
> @@ -1790,7 +1798,7 @@ static int ast_bmc_output_init(struct ast_device *ast,
>   		return ret;
>   	encoder->possible_crtcs = drm_crtc_mask(crtc);
>   
> -	ret = ast_bmc_connector_init(dev, bmc_connector, physical_connector);
> +	ret = ast_bmc_connector_init(dev, bmc_connector, physical_connector, count);
>   	if (ret)
>   		return ret;
>   
> @@ -1852,8 +1860,8 @@ 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;
> +	struct drm_connector *physical_connector[MAX_CONNECTORS] = {NULL};
> +	int count, ret;
>   
>   	ret = drmm_mutex_init(dev, &ast->modeset_lock);
>   	if (ret)
> @@ -1897,27 +1905,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.connector;
> +		physical_connector[count++] = &ast->output.vga.connector;
>   	}
>   	if (ast->tx_chip_types & AST_TX_SIL164_BIT) {
>   		ret = ast_sil164_output_init(ast);
>   		if (ret)
>   			return ret;
> -		physical_connector = &ast->output.sil164.connector;
> +		physical_connector[count++] = &ast->output.sil164.connector;
>   	}
>   	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;
> +		physical_connector[count++] = &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;
> +		physical_connector[count++] = &ast->output.astdp.connector;
>   	}
> -	ret = ast_bmc_output_init(ast, physical_connector);
> +	ret = ast_bmc_output_init(ast, physical_connector, count);
>   	if (ret)
>   		return ret;
>   

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