diff mbox series

drm/ast: add multiple connectors support

Message ID 20240711090102.352213-1-oushixiong1025@163.com (mailing list archive)
State New, archived
Headers show
Series drm/ast: add multiple connectors support | expand

Commit Message

oushixiong July 11, 2024, 9:01 a.m. UTC
From: Shixiong Ou <oushixiong@kylinos.cn>

[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@kylinos.cn>
---
 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(-)

Comments

Thomas Zimmermann July 11, 2024, 2:36 p.m. UTC | #1
Hi

Am 11.07.24 um 11:01 schrieb oushixiong1025@163.com:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> [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@kylinos.cn>
> ---
>   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 July 29, 2024, 12:29 p.m. UTC | #2
Hi

Am 11.07.24 um 11:01 schrieb oushixiong1025@163.com:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> [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.

I have another question about this patch. Is there a physical connector 
for each type (VGA, DP) on your board? Or just one of them?

Best regards
Thomas

>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> ---
>   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;
>   };
>   
>   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 July 29, 2024, 12:34 p.m. UTC | #3
Hi

Am 11.07.24 um 11:01 schrieb oushixiong1025@163.com:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> [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.

And another question: does the patch series at

   https://patchwork.freedesktop.org/series/136198/

fix the problem?

Best regards
Thomas

>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> ---
>   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;
>   };
>   
>   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;
>
oushixiong July 30, 2024, 1:20 a.m. UTC | #4
Just a VGA connector on the board.

Best regards
Shixiong

在 2024/7/29 20:29, Thomas Zimmermann 写道:
> Hi
>
> Am 11.07.24 um 11:01 schrieb oushixiong1025@163.com:
>> From: Shixiong Ou <oushixiong@kylinos.cn>
>>
>> [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.
>
> I have another question about this patch. Is there a physical 
> connector for each type (VGA, DP) on your board? Or just one of them?
>
> Best regards
> Thomas
>
>>
>> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
>> ---
>>   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;
>>   };
>>     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;
>
oushixiong July 30, 2024, 1:25 a.m. UTC | #5
Yes, I have  tested these patches, and these patches fix the problem.

Best regards
Shixiong

在 2024/7/29 20:34, Thomas Zimmermann 写道:
> Hi
>
> Am 11.07.24 um 11:01 schrieb oushixiong1025@163.com:
>> From: Shixiong Ou <oushixiong@kylinos.cn>
>>
>> [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.
>
> And another question: does the patch series at
>
>   https://patchwork.freedesktop.org/series/136198/
>
> fix the problem?
>
> Best regards
> Thomas
>
>>
>> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
>> ---
>>   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;
>>   };
>>     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 July 30, 2024, 6:42 a.m. UTC | #6
Hi

Am 30.07.24 um 03:25 schrieb oushixiong:
> Yes, I have  tested these patches, and these patches fix the problem.

Thanks a lot. Patch 2 is the one that fixes the issue. I'll add your 
Tested-by tag to it.

Best regards
Thomas

>
> Best regards
> Shixiong
>
> 在 2024/7/29 20:34, Thomas Zimmermann 写道:
>> Hi
>>
>> Am 11.07.24 um 11:01 schrieb oushixiong1025@163.com:
>>> From: Shixiong Ou <oushixiong@kylinos.cn>
>>>
>>> [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.
>>
>> And another question: does the patch series at
>>
>>   https://patchwork.freedesktop.org/series/136198/
>>
>> fix the problem?
>>
>> Best regards
>> Thomas
>>
>>>
>>> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
>>> ---
>>>   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;
>>>   };
>>>     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;
>>
diff mbox series

Patch

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;
 };
 
 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;