Re: [PATCH] drm/ast: Remove TX-chip bitmask

Jocelyn Falempe <jfalempe@xxxxxxxxxx> · Thu, 5 Sep 2024 11:19:12 +0200

On 03/09/2024 15:25, Thomas Zimmermann wrote:
The CRTC has only one output attached to it. Store the output's type
of TX chip in a single field and remove the related bitmask.

Turn the type-less output field in struct ast_device into a union, as
only one of its fields will be used at a time.

Thanks for this simplification.

Reviewed-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>

Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
---
  drivers/gpu/drm/ast/ast_dp501.c |  2 +-
  drivers/gpu/drm/ast/ast_drv.h   | 10 +++-------
  drivers/gpu/drm/ast/ast_main.c  | 27 +++++++++++++--------------
  drivers/gpu/drm/ast/ast_mode.c  | 26 +++++++++++---------------
  drivers/gpu/drm/ast/ast_post.c  |  4 ++--
  5 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index e4c636f45082..99bf44eec7b3 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -466,7 +466,7 @@ void ast_init_3rdtx(struct drm_device *dev)
  			ast_init_dvo(dev);
  			break;
  		default:
-			if (ast->tx_chip_types & BIT(AST_TX_SIL164))
+			if (ast->tx_chip == AST_TX_SIL164)
  				ast_init_dvo(dev);
  			else
  				ast_init_analog(dev);
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 91fe07cf7b07..b6ca14a3b967 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -91,11 +91,6 @@ enum ast_tx_chip {
  	AST_TX_ASTDP,
  };
  
-#define AST_TX_NONE_BIT		BIT(AST_TX_NONE)
-#define AST_TX_SIL164_BIT	BIT(AST_TX_SIL164)
-#define AST_TX_DP501_BIT	BIT(AST_TX_DP501)
-#define AST_TX_ASTDP_BIT	BIT(AST_TX_ASTDP)
-
  enum ast_config_mode {
  	ast_use_p2a,
  	ast_use_dt,
@@ -187,10 +182,12 @@ struct ast_device {
  
  	struct mutex modeset_lock; /* Protects access to modeset I/O registers in ioregs */
  
+	enum ast_tx_chip tx_chip;
+
  	struct ast_plane primary_plane;
  	struct ast_plane cursor_plane;
  	struct drm_crtc crtc;
-	struct {
+	union {
  		struct {
  			struct drm_encoder encoder;
  			struct ast_connector connector;
@@ -211,7 +208,6 @@ struct ast_device {
  
  	bool support_wide_screen;
  
-	unsigned long tx_chip_types;		/* bitfield of enum ast_chip_type */
  	u8 *dp501_fw_addr;
  	const struct firmware *dp501_fw;	/* dp501 fw */
  };
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index d836f2a4f9f3..d7d503e78e25 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -68,11 +68,18 @@ static void ast_detect_widescreen(struct ast_device *ast)
  
  static void ast_detect_tx_chip(struct ast_device *ast, bool need_post)
  {
+	static const char * const info_str[] = {
+		"analog VGA",
+		"Sil164 TMDS transmitter",
+		"DP501 DisplayPort transmitter",
+		"ASPEED DisplayPort transmitter",
+	};
+
  	struct drm_device *dev = &ast->base;
  	u8 jreg;
  
  	/* Check 3rd Tx option (digital output afaik) */
-	ast->tx_chip_types |= AST_TX_NONE_BIT;
+	ast->tx_chip = AST_TX_NONE;
  
  	/*
  	 * VGACRA3 Enhanced Color Mode Register, check if DVO is already
@@ -85,7 +92,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 = AST_TX_SIL164;
  	}
  
  	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast)) {
@@ -97,7 +104,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 = AST_TX_SIL164;
  			break;
  		case 0x08:
  			ast->dp501_fw_addr = drmm_kzalloc(dev, 32*1024, GFP_KERNEL);
@@ -110,7 +117,7 @@ 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 = AST_TX_DP501;
  		}
  	} else if (IS_AST_GEN7(ast)) {
  		if (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD1, TX_TYPE_MASK) ==
@@ -118,19 +125,11 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool need_post)
  			int ret = ast_dp_launch(ast);
  
  			if (!ret)
-				ast->tx_chip_types = AST_TX_ASTDP_BIT;
+				ast->tx_chip = AST_TX_ASTDP;
  		}
  	}
  
-	/* Print stuff for diagnostic purposes */
-	if (ast->tx_chip_types & AST_TX_NONE_BIT)
-		drm_info(dev, "Using analog VGA\n");
-	if (ast->tx_chip_types & AST_TX_SIL164_BIT)
-		drm_info(dev, "Using Sil164 TMDS transmitter\n");
-	if (ast->tx_chip_types & AST_TX_DP501_BIT)
-		drm_info(dev, "Using DP501 DisplayPort transmitter\n");
-	if (ast->tx_chip_types & AST_TX_ASTDP_BIT)
-		drm_info(dev, "Using ASPEED DisplayPort transmitter\n");
+	drm_info(dev, "Using %s\n", info_str[ast->tx_chip]);
  }
  
  static int ast_get_dram_info(struct drm_device *dev)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index ed496fb32bf3..784e8ac0e668 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1398,26 +1398,22 @@ int ast_mode_config_init(struct ast_device *ast)
  
  	ast_crtc_init(dev);
  
-	if (ast->tx_chip_types & AST_TX_NONE_BIT) {
+	switch (ast->tx_chip) {
+	case AST_TX_NONE:
  		ret = ast_vga_output_init(ast);
-		if (ret)
-			return ret;
-	}
-	if (ast->tx_chip_types & AST_TX_SIL164_BIT) {
+		break;
+	case AST_TX_SIL164:
  		ret = ast_sil164_output_init(ast);
-		if (ret)
-			return ret;
-	}
-	if (ast->tx_chip_types & AST_TX_DP501_BIT) {
+		break;
+	case AST_TX_DP501:
  		ret = ast_dp501_output_init(ast);
-		if (ret)
-			return ret;
-	}
-	if (ast->tx_chip_types & AST_TX_ASTDP_BIT) {
+		break;
+	case AST_TX_ASTDP:
  		ret = ast_astdp_output_init(ast);
-		if (ret)
-			return ret;
+		break;
  	}
+	if (ret)
+		return ret;
  
  	drm_mode_config_reset(dev);
  
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 65755798ab94..902bf8114b6e 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -350,7 +350,7 @@ void ast_post_gpu(struct drm_device *dev)
  	ast_set_def_ext_reg(dev);
  
  	if (IS_AST_GEN7(ast)) {
-		if (ast->tx_chip_types & AST_TX_ASTDP_BIT)
+		if (ast->tx_chip == AST_TX_ASTDP)
  			ast_dp_launch(ast);
  	} else if (ast->config_mode == ast_use_p2a) {
  		if (IS_AST_GEN6(ast))
@@ -362,7 +362,7 @@ void ast_post_gpu(struct drm_device *dev)
  
  		ast_init_3rdtx(dev);
  	} else {
-		if (ast->tx_chip_types & AST_TX_SIL164_BIT)
+		if (ast->tx_chip == AST_TX_SIL164)
  			ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xa3, 0xcf, 0x80);	/* Enable DVO */
  	}
  }







