diff mbox series

[04/10] drm/ast: astdp: Replace ast_dp_set_on_off()

Message ID 20240911115347.899148-5-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm/ast: Various cleanups | expand

Commit Message

Thomas Zimmermann Sept. 11, 2024, 11:51 a.m. UTC
Replace ast_dp_set_on_off() with ast_dp_set_enable(). The helper's
new name reflects the performed operation. If enabling fails, the
new helper prints a warning. The code that waits for the programmed
effect to take place is now located in __ast_dp_wait_enable().

Also align the register constants with the rest of the code.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_dp.c  | 49 +++++++++++++++++++++++------------
 drivers/gpu/drm/ast/ast_reg.h | 13 ++--------
 2 files changed, 35 insertions(+), 27 deletions(-)

Comments

Jocelyn Falempe Sept. 12, 2024, 1:38 p.m. UTC | #1
On 11/09/2024 13:51, Thomas Zimmermann wrote:
> Replace ast_dp_set_on_off() with ast_dp_set_enable(). The helper's
> new name reflects the performed operation. If enabling fails, the
> new helper prints a warning. The code that waits for the programmed
> effect to take place is now located in __ast_dp_wait_enable().
> 
> Also align the register constants with the rest of the code.
> 

Thanks, it looks good to me.

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

> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/ast/ast_dp.c  | 49 +++++++++++++++++++++++------------
>   drivers/gpu/drm/ast/ast_reg.h | 13 ++--------
>   2 files changed, 35 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
> index d4362807d777..0e282b7b167c 100644
> --- a/drivers/gpu/drm/ast/ast_dp.c
> +++ b/drivers/gpu/drm/ast/ast_dp.c
> @@ -186,22 +186,39 @@ static void ast_dp_link_training(struct ast_device *ast)
>   	drm_err(dev, "Link training failed\n");
>   }
>   
> -static void ast_dp_set_on_off(struct ast_device *ast, bool on)
> +static bool __ast_dp_wait_enable(struct ast_device *ast, bool enabled)
>   {
> -	u8 video_on_off = on;
> -	u32 i = 0;
> -
> -	// Video On/Off
> -	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE3, (u8) ~AST_DP_VIDEO_ENABLE, on);
> -
> -	video_on_off <<= 4;
> -	while (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDF,
> -						ASTDP_MIRROR_VIDEO_ENABLE) != video_on_off) {
> -		// wait 1 ms
> -		mdelay(1);
> -		if (++i > 200)
> -			break;
> +	u8 vgacrdf_test = 0x00;
> +	u8 vgacrdf;
> +	unsigned int i;
> +
> +	if (enabled)
> +		vgacrdf_test |= AST_IO_VGACRDF_DP_VIDEO_ENABLE;
> +
> +	for (i = 0; i < 200; ++i) {
> +		if (i)
> +			mdelay(1);
> +		vgacrdf = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xdf,
> +						 AST_IO_VGACRDF_DP_VIDEO_ENABLE);
> +		if (vgacrdf == vgacrdf_test)
> +			return true;
>   	}
> +
> +	return false;
> +}
> +
> +static void ast_dp_set_enable(struct ast_device *ast, bool enabled)
> +{
> +	struct drm_device *dev = &ast->base;
> +	u8 vgacre3 = 0x00;
> +
> +	if (enabled)
> +		vgacre3 |= AST_IO_VGACRE3_DP_VIDEO_ENABLE;
> +
> +	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xe3, (u8)~AST_IO_VGACRE3_DP_VIDEO_ENABLE,
> +			       vgacre3);
> +
> +	drm_WARN_ON(dev, !__ast_dp_wait_enable(ast, enabled));
>   }
>   
>   static void ast_dp_set_mode(struct drm_crtc *crtc, struct ast_vbios_mode_info *vbios_mode)
> @@ -318,7 +335,7 @@ static void ast_astdp_encoder_helper_atomic_enable(struct drm_encoder *encoder,
>   		ast_dp_link_training(ast);
>   
>   		ast_wait_for_vretrace(ast);
> -		ast_dp_set_on_off(ast, 1);
> +		ast_dp_set_enable(ast, true);
>   	}
>   }
>   
> @@ -327,7 +344,7 @@ static void ast_astdp_encoder_helper_atomic_disable(struct drm_encoder *encoder,
>   {
>   	struct ast_device *ast = to_ast_device(encoder->dev);
>   
> -	ast_dp_set_on_off(ast, 0);
> +	ast_dp_set_enable(ast, false);
>   	ast_dp_set_phy_sleep(ast, true);
>   }
>   
> diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
> index d7a22cea8271..6a1f756650ab 100644
> --- a/drivers/gpu/drm/ast/ast_reg.h
> +++ b/drivers/gpu/drm/ast/ast_reg.h
> @@ -41,6 +41,8 @@
>   #define AST_IO_VGACRD7_EDID_VALID_FLAG	BIT(0)
>   #define AST_IO_VGACRDC_LINK_SUCCESS	BIT(0)
>   #define AST_IO_VGACRDF_HPD		BIT(0)
> +#define AST_IO_VGACRDF_DP_VIDEO_ENABLE	BIT(4) /* mirrors AST_IO_VGACRE3_DP_VIDEO_ENABLE */
> +#define AST_IO_VGACRE3_DP_VIDEO_ENABLE	BIT(0)
>   #define AST_IO_VGACRE3_DP_PHY_SLEEP	BIT(4)
>   #define AST_IO_VGACRE5_EDID_READ_DONE	BIT(0)
>   
> @@ -69,17 +71,6 @@
>    * AST DisplayPort
>    */
>   
> -/* Define for Soc scratched reg used on ASTDP */
> -#define AST_DP_VIDEO_ENABLE		BIT(0)
> -
> -/*
> - * CRDF[b4]: Mirror of AST_DP_VIDEO_ENABLE
> - * Precondition:	A. ~AST_DP_PHY_SLEEP  &&
> - *			B. DP_HPD &&
> - *			C. DP_LINK_SUCCESS
> - */
> -#define ASTDP_MIRROR_VIDEO_ENABLE	BIT(4)
> -
>   /*
>    * ASTDP setmode registers:
>    * CRE0[7:0]: MISC0 ((0x00: 18-bpp) or (0x20: 24-bpp)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index d4362807d777..0e282b7b167c 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -186,22 +186,39 @@  static void ast_dp_link_training(struct ast_device *ast)
 	drm_err(dev, "Link training failed\n");
 }
 
-static void ast_dp_set_on_off(struct ast_device *ast, bool on)
+static bool __ast_dp_wait_enable(struct ast_device *ast, bool enabled)
 {
-	u8 video_on_off = on;
-	u32 i = 0;
-
-	// Video On/Off
-	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE3, (u8) ~AST_DP_VIDEO_ENABLE, on);
-
-	video_on_off <<= 4;
-	while (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDF,
-						ASTDP_MIRROR_VIDEO_ENABLE) != video_on_off) {
-		// wait 1 ms
-		mdelay(1);
-		if (++i > 200)
-			break;
+	u8 vgacrdf_test = 0x00;
+	u8 vgacrdf;
+	unsigned int i;
+
+	if (enabled)
+		vgacrdf_test |= AST_IO_VGACRDF_DP_VIDEO_ENABLE;
+
+	for (i = 0; i < 200; ++i) {
+		if (i)
+			mdelay(1);
+		vgacrdf = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xdf,
+						 AST_IO_VGACRDF_DP_VIDEO_ENABLE);
+		if (vgacrdf == vgacrdf_test)
+			return true;
 	}
+
+	return false;
+}
+
+static void ast_dp_set_enable(struct ast_device *ast, bool enabled)
+{
+	struct drm_device *dev = &ast->base;
+	u8 vgacre3 = 0x00;
+
+	if (enabled)
+		vgacre3 |= AST_IO_VGACRE3_DP_VIDEO_ENABLE;
+
+	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xe3, (u8)~AST_IO_VGACRE3_DP_VIDEO_ENABLE,
+			       vgacre3);
+
+	drm_WARN_ON(dev, !__ast_dp_wait_enable(ast, enabled));
 }
 
 static void ast_dp_set_mode(struct drm_crtc *crtc, struct ast_vbios_mode_info *vbios_mode)
@@ -318,7 +335,7 @@  static void ast_astdp_encoder_helper_atomic_enable(struct drm_encoder *encoder,
 		ast_dp_link_training(ast);
 
 		ast_wait_for_vretrace(ast);
-		ast_dp_set_on_off(ast, 1);
+		ast_dp_set_enable(ast, true);
 	}
 }
 
@@ -327,7 +344,7 @@  static void ast_astdp_encoder_helper_atomic_disable(struct drm_encoder *encoder,
 {
 	struct ast_device *ast = to_ast_device(encoder->dev);
 
-	ast_dp_set_on_off(ast, 0);
+	ast_dp_set_enable(ast, false);
 	ast_dp_set_phy_sleep(ast, true);
 }
 
diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
index d7a22cea8271..6a1f756650ab 100644
--- a/drivers/gpu/drm/ast/ast_reg.h
+++ b/drivers/gpu/drm/ast/ast_reg.h
@@ -41,6 +41,8 @@ 
 #define AST_IO_VGACRD7_EDID_VALID_FLAG	BIT(0)
 #define AST_IO_VGACRDC_LINK_SUCCESS	BIT(0)
 #define AST_IO_VGACRDF_HPD		BIT(0)
+#define AST_IO_VGACRDF_DP_VIDEO_ENABLE	BIT(4) /* mirrors AST_IO_VGACRE3_DP_VIDEO_ENABLE */
+#define AST_IO_VGACRE3_DP_VIDEO_ENABLE	BIT(0)
 #define AST_IO_VGACRE3_DP_PHY_SLEEP	BIT(4)
 #define AST_IO_VGACRE5_EDID_READ_DONE	BIT(0)
 
@@ -69,17 +71,6 @@ 
  * AST DisplayPort
  */
 
-/* Define for Soc scratched reg used on ASTDP */
-#define AST_DP_VIDEO_ENABLE		BIT(0)
-
-/*
- * CRDF[b4]: Mirror of AST_DP_VIDEO_ENABLE
- * Precondition:	A. ~AST_DP_PHY_SLEEP  &&
- *			B. DP_HPD &&
- *			C. DP_LINK_SUCCESS
- */
-#define ASTDP_MIRROR_VIDEO_ENABLE	BIT(4)
-
 /*
  * ASTDP setmode registers:
  * CRE0[7:0]: MISC0 ((0x00: 18-bpp) or (0x20: 24-bpp)