diff mbox series

[v3,2/2] drm/ast: report connection status on Display Port.

Message ID 20230712083733.223275-2-jfalempe@redhat.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/2] drm/ast: Add BMC virtual connector | expand

Commit Message

Jocelyn Falempe July 12, 2023, 8:35 a.m. UTC 
Aspeed always report the display port as "connected", because it
doesn't set a .detect callback.
Fix this by providing the proper detect callback for astdp and dp501.

This also fixes the following regression:
Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
EDID on DP")
The default resolution is now 640x480 when no monitor is connected. But
Aspeed graphics is mostly used in servers, where no monitor is attached.
This also affects the remote BMC resolution to 640x480, which is
inconvenient, and breaks the anaconda installer.

v2: Add .detect callback to the dp/dp501 connector (Jani Nikula)
v3: Use .detect_ctx callback, and refactors (Thomas Zimmermann)
    Add a BMC virtual connector

Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID on DP")
Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/ast/ast_dp.c    | 11 ++++++++++
 drivers/gpu/drm/ast/ast_dp501.c | 37 ++++++++++++++++++++++-----------
 drivers/gpu/drm/ast/ast_drv.h   |  2 ++
 drivers/gpu/drm/ast/ast_mode.c  | 27 ++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 12 deletions(-)

Comments

Thomas Zimmermann July 12, 2023, 10:23 a.m. UTC | #1 
Hi

Am 12.07.23 um 10:35 schrieb Jocelyn Falempe:
> Aspeed always report the display port as "connected", because it
> doesn't set a .detect callback.
> Fix this by providing the proper detect callback for astdp and dp501.
> 
> This also fixes the following regression:
> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
> EDID on DP")
> The default resolution is now 640x480 when no monitor is connected. But
> Aspeed graphics is mostly used in servers, where no monitor is attached.
> This also affects the remote BMC resolution to 640x480, which is
> inconvenient, and breaks the anaconda installer.
> 
> v2: Add .detect callback to the dp/dp501 connector (Jani Nikula)
> v3: Use .detect_ctx callback, and refactors (Thomas Zimmermann)
>      Add a BMC virtual connector
> 
> Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID on DP")
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>

Reviewed-by: Thomas Zimmermann <tzimemrmann@suse.de>

with a minor comment below.

> ---
>   drivers/gpu/drm/ast/ast_dp.c    | 11 ++++++++++
>   drivers/gpu/drm/ast/ast_dp501.c | 37 ++++++++++++++++++++++-----------
>   drivers/gpu/drm/ast/ast_drv.h   |  2 ++
>   drivers/gpu/drm/ast/ast_mode.c  | 27 ++++++++++++++++++++++++
>   4 files changed, 65 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
> index 6dc1a09504e1..bf78f3d4aa3f 100644
> --- a/drivers/gpu/drm/ast/ast_dp.c
> +++ b/drivers/gpu/drm/ast/ast_dp.c
> @@ -7,6 +7,17 @@
>   #include <drm/drm_print.h>
>   #include "ast_drv.h"
>   
> +bool ast_astdp_is_connected(struct ast_device *ast)
> +{
> +	if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, ASTDP_MCU_FW_EXECUTING))
> +		return false;
> +	if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDC, ASTDP_LINK_SUCCESS))
> +		return false;
> +	if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDF, ASTDP_HPD))
> +		return false;
> +	return true;
> +}
> +
>   int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
>   {
>   	struct ast_device *ast = to_ast_device(dev);
> diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
> index a5d285850ffb..f10d53b0c94f 100644
> --- a/drivers/gpu/drm/ast/ast_dp501.c
> +++ b/drivers/gpu/drm/ast/ast_dp501.c
> @@ -272,11 +272,9 @@ static bool ast_launch_m68k(struct drm_device *dev)
>   	return true;
>   }
>   
> -bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
> +bool ast_dp501_is_connected(struct ast_device *ast)
>   {
> -	struct ast_device *ast = to_ast_device(dev);
> -	u32 i, boot_address, offset, data;
> -	u32 *pEDIDidx;
> +	u32 boot_address, offset, data;
>   
>   	if (ast->config_mode == ast_use_p2a) {
>   		boot_address = get_fw_base(ast);
> @@ -292,14 +290,6 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>   		data = ast_mindwm(ast, boot_address + offset);
>   		if (!(data & AST_DP501_PNP_CONNECTED))
>   			return false;
> -
> -		/* Read EDID */
> -		offset = AST_DP501_EDID_DATA;
> -		for (i = 0; i < 128; i += 4) {
> -			data = ast_mindwm(ast, boot_address + offset + i);
> -			pEDIDidx = (u32 *)(ediddata + i);
> -			*pEDIDidx = data;
> -		}
>   	} else {
>   		if (!ast->dp501_fw_buf)
>   			return false;
> @@ -319,7 +309,30 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>   		data = readl(ast->dp501_fw_buf + offset);
>   		if (!(data & AST_DP501_PNP_CONNECTED))
>   			return false;
> +	}
> +	return true;
> +}
> +
> +bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
> +{
> +	struct ast_device *ast = to_ast_device(dev);
> +	u32 i, boot_address, offset, data;
> +	u32 *pEDIDidx;
> +
> +	if (!ast_dp501_is_connected(ast))
> +		return false;
> +
> +	if (ast->config_mode == ast_use_p2a) {
> +		boot_address = get_fw_base(ast);
>   
> +		/* Read EDID */
> +		offset = AST_DP501_EDID_DATA;
> +		for (i = 0; i < 128; i += 4) {
> +			data = ast_mindwm(ast, boot_address + offset + i);
> +			pEDIDidx = (u32 *)(ediddata + i);
> +			*pEDIDidx = data;
> +		}
> +	} else {
>   		/* Read EDID */
>   		offset = AST_DP501_EDID_DATA;
>   		for (i = 0; i < 128; i += 4) {
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index c9659e72002f..848a9f1403e8 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -514,6 +514,7 @@ void ast_patch_ahb_2500(struct ast_device *ast);
>   /* ast dp501 */
>   void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
>   bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size);
> +bool ast_dp501_is_connected(struct ast_device *ast);
>   bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata);
>   u8 ast_get_dp501_max_clk(struct drm_device *dev);
>   void ast_init_3rdtx(struct drm_device *dev);
> @@ -522,6 +523,7 @@ void ast_init_3rdtx(struct drm_device *dev);
>   struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
>   
>   /* aspeed DP */
> +bool ast_astdp_is_connected(struct ast_device *ast);
>   int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata);
>   void ast_dp_launch(struct drm_device *dev);
>   void ast_dp_power_on_off(struct drm_device *dev, bool no);
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 8896b22eb5cf..8d019a785fb4 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -1582,8 +1582,22 @@ static int ast_dp501_connector_helper_get_modes(struct drm_connector *connector)
>   	return 0;
>   }
>   
> +static int ast_dp501_connector_helper_detect_ctx(
> +	struct drm_connector *connector,
> +	struct drm_modeset_acquire_ctx *ctx,
> +	bool force)

If this fits within the 100-char limit prt line, it would be preferable 
to move the first argument to first line.

> +{
> +	struct ast_device *ast = to_ast_device(connector->dev);
> +
> +	if (ast_dp501_is_connected(ast))
> +		return connector_status_connected;
> +	return connector_status_disconnected;
> +}
> +
> +
>   static const struct drm_connector_helper_funcs ast_dp501_connector_helper_funcs = {
>   	.get_modes = ast_dp501_connector_helper_get_modes,
> +	.detect_ctx = ast_dp501_connector_helper_detect_ctx,
>   };
>   
>   static const struct drm_connector_funcs ast_dp501_connector_funcs = {
> @@ -1680,8 +1694,21 @@ static int ast_astdp_connector_helper_get_modes(struct drm_connector *connector)
>   	return 0;
>   }
>   
> +static int ast_astdp_connector_helper_detect_ctx(
> +	struct drm_connector *connector,
> +	struct drm_modeset_acquire_ctx *ctx,
> +	bool force)

Same here.

Best regards
Thomas

> +{
> +	struct ast_device *ast = to_ast_device(connector->dev);
> +
> +	if (ast_astdp_is_connected(ast))
> +		return connector_status_connected;
> +	return connector_status_disconnected;
> +}
> +
>   static const struct drm_connector_helper_funcs ast_astdp_connector_helper_funcs = {
>   	.get_modes = ast_astdp_connector_helper_get_modes,
> +	.detect_ctx = ast_astdp_connector_helper_detect_ctx,
>   };
>   
>   static const struct drm_connector_funcs ast_astdp_connector_funcs = {
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index 6dc1a09504e1..bf78f3d4aa3f 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -7,6 +7,17 @@ 
 #include <drm/drm_print.h>
 #include "ast_drv.h"
 
+bool ast_astdp_is_connected(struct ast_device *ast)
+{
+	if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, ASTDP_MCU_FW_EXECUTING))
+		return false;
+	if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDC, ASTDP_LINK_SUCCESS))
+		return false;
+	if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDF, ASTDP_HPD))
+		return false;
+	return true;
+}
+
 int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
 {
 	struct ast_device *ast = to_ast_device(dev);
diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index a5d285850ffb..f10d53b0c94f 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -272,11 +272,9 @@  static bool ast_launch_m68k(struct drm_device *dev)
 	return true;
 }
 
-bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
+bool ast_dp501_is_connected(struct ast_device *ast)
 {
-	struct ast_device *ast = to_ast_device(dev);
-	u32 i, boot_address, offset, data;
-	u32 *pEDIDidx;
+	u32 boot_address, offset, data;
 
 	if (ast->config_mode == ast_use_p2a) {
 		boot_address = get_fw_base(ast);
@@ -292,14 +290,6 @@  bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
 		data = ast_mindwm(ast, boot_address + offset);
 		if (!(data & AST_DP501_PNP_CONNECTED))
 			return false;
-
-		/* Read EDID */
-		offset = AST_DP501_EDID_DATA;
-		for (i = 0; i < 128; i += 4) {
-			data = ast_mindwm(ast, boot_address + offset + i);
-			pEDIDidx = (u32 *)(ediddata + i);
-			*pEDIDidx = data;
-		}
 	} else {
 		if (!ast->dp501_fw_buf)
 			return false;
@@ -319,7 +309,30 @@  bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
 		data = readl(ast->dp501_fw_buf + offset);
 		if (!(data & AST_DP501_PNP_CONNECTED))
 			return false;
+	}
+	return true;
+}
+
+bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
+{
+	struct ast_device *ast = to_ast_device(dev);
+	u32 i, boot_address, offset, data;
+	u32 *pEDIDidx;
+
+	if (!ast_dp501_is_connected(ast))
+		return false;
+
+	if (ast->config_mode == ast_use_p2a) {
+		boot_address = get_fw_base(ast);
 
+		/* Read EDID */
+		offset = AST_DP501_EDID_DATA;
+		for (i = 0; i < 128; i += 4) {
+			data = ast_mindwm(ast, boot_address + offset + i);
+			pEDIDidx = (u32 *)(ediddata + i);
+			*pEDIDidx = data;
+		}
+	} else {
 		/* Read EDID */
 		offset = AST_DP501_EDID_DATA;
 		for (i = 0; i < 128; i += 4) {
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index c9659e72002f..848a9f1403e8 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -514,6 +514,7 @@  void ast_patch_ahb_2500(struct ast_device *ast);
 /* ast dp501 */
 void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
 bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size);
+bool ast_dp501_is_connected(struct ast_device *ast);
 bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata);
 u8 ast_get_dp501_max_clk(struct drm_device *dev);
 void ast_init_3rdtx(struct drm_device *dev);
@@ -522,6 +523,7 @@  void ast_init_3rdtx(struct drm_device *dev);
 struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
 
 /* aspeed DP */
+bool ast_astdp_is_connected(struct ast_device *ast);
 int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata);
 void ast_dp_launch(struct drm_device *dev);
 void ast_dp_power_on_off(struct drm_device *dev, bool no);
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 8896b22eb5cf..8d019a785fb4 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1582,8 +1582,22 @@  static int ast_dp501_connector_helper_get_modes(struct drm_connector *connector)
 	return 0;
 }
 
+static int ast_dp501_connector_helper_detect_ctx(
+	struct drm_connector *connector,
+	struct drm_modeset_acquire_ctx *ctx,
+	bool force)
+{
+	struct ast_device *ast = to_ast_device(connector->dev);
+
+	if (ast_dp501_is_connected(ast))
+		return connector_status_connected;
+	return connector_status_disconnected;
+}
+
+
 static const struct drm_connector_helper_funcs ast_dp501_connector_helper_funcs = {
 	.get_modes = ast_dp501_connector_helper_get_modes,
+	.detect_ctx = ast_dp501_connector_helper_detect_ctx,
 };
 
 static const struct drm_connector_funcs ast_dp501_connector_funcs = {
@@ -1680,8 +1694,21 @@  static int ast_astdp_connector_helper_get_modes(struct drm_connector *connector)
 	return 0;
 }
 
+static int ast_astdp_connector_helper_detect_ctx(
+	struct drm_connector *connector,
+	struct drm_modeset_acquire_ctx *ctx,
+	bool force)
+{
+	struct ast_device *ast = to_ast_device(connector->dev);
+
+	if (ast_astdp_is_connected(ast))
+		return connector_status_connected;
+	return connector_status_disconnected;
+}
+
 static const struct drm_connector_helper_funcs ast_astdp_connector_helper_funcs = {
 	.get_modes = ast_astdp_connector_helper_get_modes,
+	.detect_ctx = ast_astdp_connector_helper_detect_ctx,
 };
 
 static const struct drm_connector_funcs ast_astdp_connector_funcs = {