dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/14] drm/ast: Refactor the device-detection code
@ 2023-06-21 12:53 Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 01/14] drm/ast: Fix DRAM init on AST2200 Thomas Zimmermann
                   ` (14 more replies)
  0 siblings, 15 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

Ast's code for detecting the device type and features is convoluted.
It mixes up several state fields, chip types and sub-models. Rework
the driver into something more understandable.

Patches 1 fixes a long-standing bug. The affected code has never
worked correctly.

Patches 2 to 8 make various changes to the init code, or remove dead
and duplicated code paths.

Patch 9 introduces chip generations. Until now, ast used the value
of enum ast_chip to represent a certain set of related modes, and
also used the enum to represent individal models. This makes the
driver code hard to understand in certain places. The patch encodes
a chip generation in each model enum and converts the driver to use
it.

Patches 10 to 12 replace duplicated model checks with the correct
enum value. Detection of wide-screen functionality and the transmitter
chip can then be moved into individual functions in patch 13.

Patch 14 merges the detection of the silicon revision and the chip
model into a single function. Both need to be done in the same place
and affect each other.

Tested on AST1100 and AST2300.

v2:
	* use standard 16-bit PCI access (Jingfeng)
	* various cleanups

Thomas Zimmermann (14):
  drm/ast: Fix DRAM init on AST2200
  drm/ast: Remove vga2_clone field
  drm/ast: Implement register helpers in ast_drv.h
  drm/ast: Remove dead else branch in POST code
  drm/ast: Remove device POSTing and config from chip detection
  drm/ast: Set PCI config before accessing I/O registers
  drm/ast: Enable and unlock device access early during init
  drm/ast: Set up release action right after enabling MMIO
  drm/ast: Distinguish among chip generations
  drm/ast: Detect AST 1300 model
  drm/ast: Detect AST 1400 model
  drm/ast: Detect AST 2510 model
  drm/ast: Move widescreen and tx-chip detection into separate helpers
  drm/ast: Merge config and chip detection

 drivers/gpu/drm/ast/ast_dp501.c |   6 +-
 drivers/gpu/drm/ast/ast_drv.h   |  94 +++++++---
 drivers/gpu/drm/ast/ast_main.c  | 319 +++++++++++++++++++-------------
 drivers/gpu/drm/ast/ast_mm.c    |   2 -
 drivers/gpu/drm/ast/ast_mode.c  |  35 ++--
 drivers/gpu/drm/ast/ast_post.c  |  74 ++------
 6 files changed, 290 insertions(+), 240 deletions(-)


base-commit: 32e260cd0d16cee6f33e747679f168d63ea54bf6
prerequisite-patch-id: 0aa359f6144c4015c140c8a6750be19099c676fb
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
prerequisite-patch-id: cbc453ee02fae02af22fbfdce56ab732c7a88c36
prerequisite-patch-id: d3145eae4b35a1290199af6ff6cd5abfebc82033
prerequisite-patch-id: 242b6bc45675f1f1a62572542d75c89d4864f15a
-- 
2.41.0


^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH v2 01/14] drm/ast: Fix DRAM init on AST2200
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 02/14] drm/ast: Remove vga2_clone field Thomas Zimmermann
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: stable, Thomas Zimmermann, dri-devel

Fix the test for the AST2200 in the DRAM initialization. The value
in ast->chip has to be compared against an enum constant instead of
a numerical value.

This bug got introduced when the driver was first imported into the
kernel.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 312fec1405dd ("drm: Initial KMS driver for AST (ASpeed Technologies) 2000 series (v2)")
Cc: Dave Airlie <airlied@redhat.com>
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v3.5+
Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_post.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index a005aec18a020..0262aaafdb1c5 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -291,7 +291,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
 				;
 			} while (ast_read32(ast, 0x10100) != 0xa8);
 		} else {/* AST2100/1100 */
-			if (ast->chip == AST2100 || ast->chip == 2200)
+			if (ast->chip == AST2100 || ast->chip == AST2200)
 				dram_reg_info = ast2100_dram_table_data;
 			else
 				dram_reg_info = ast1100_dram_table_data;
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 02/14] drm/ast: Remove vga2_clone field
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 01/14] drm/ast: Fix DRAM init on AST2200 Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 03/14] drm/ast: Implement register helpers in ast_drv.h Thomas Zimmermann
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

Remove the unused field vga2_clone from struct ast_device. No
functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_drv.h  | 1 -
 drivers/gpu/drm/ast/ast_main.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 5498a6676f2e8..fc4760a67596f 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -166,7 +166,6 @@ struct ast_device {
 	void __iomem *dp501_fw_buf;
 
 	enum ast_chip chip;
-	bool vga2_clone;
 	uint32_t dram_bus_width;
 	uint32_t dram_type;
 	uint32_t mclk;
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 1f35438f614a7..da33cfc6366ec 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -179,7 +179,6 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
 			drm_info(dev, "AST 2100 detected\n");
 			break;
 		}
-		ast->vga2_clone = false;
 	} else {
 		ast->chip = AST2000;
 		drm_info(dev, "AST 2000 detected\n");
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 03/14] drm/ast: Implement register helpers in ast_drv.h
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 01/14] drm/ast: Fix DRAM init on AST2200 Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 02/14] drm/ast: Remove vga2_clone field Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 04/14] drm/ast: Remove dead else branch in POST code Thomas Zimmermann
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

There are already a number of register I/O functions in ast_drv.h.
For consistency, move the remaining functions there as well. No
functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_drv.h  | 34 ++++++++++++++++++++++++----------
 drivers/gpu/drm/ast/ast_main.c | 28 ----------------------------
 2 files changed, 24 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index fc4760a67596f..0141705beaee9 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -257,22 +257,36 @@ static inline void ast_io_write8(struct ast_device *ast, u32 reg, u8 val)
 	iowrite8(val, ast->ioregs + reg);
 }
 
-static inline void ast_set_index_reg(struct ast_device *ast,
-				     uint32_t base, uint8_t index,
-				     uint8_t val)
+static inline u8 ast_get_index_reg(struct ast_device *ast, u32 base, u8 index)
+{
+	ast_io_write8(ast, base, index);
+	++base;
+	return ast_io_read8(ast, base);
+}
+
+static inline u8 ast_get_index_reg_mask(struct ast_device *ast, u32 base, u8 index,
+					u8 preserve_mask)
+{
+	u8 val = ast_get_index_reg(ast, base, index);
+
+	return val & preserve_mask;
+}
+
+static inline void ast_set_index_reg(struct ast_device *ast, u32 base, u8 index, u8 val)
 {
 	ast_io_write8(ast, base, index);
 	++base;
 	ast_io_write8(ast, base, val);
 }
 
-void ast_set_index_reg_mask(struct ast_device *ast,
-			    uint32_t base, uint8_t index,
-			    uint8_t mask, uint8_t val);
-uint8_t ast_get_index_reg(struct ast_device *ast,
-			  uint32_t base, uint8_t index);
-uint8_t ast_get_index_reg_mask(struct ast_device *ast,
-			       uint32_t base, uint8_t index, uint8_t mask);
+static inline void ast_set_index_reg_mask(struct ast_device *ast, u32 base, u8 index,
+					  u8 preserve_mask, u8 val)
+{
+	u8 tmp = ast_get_index_reg_mask(ast, base, index, preserve_mask);
+
+	tmp |= val;
+	ast_set_index_reg(ast, base, index, tmp);
+}
 
 static inline void ast_open_key(struct ast_device *ast)
 {
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index da33cfc6366ec..862fdf02f6100 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -35,34 +35,6 @@
 
 #include "ast_drv.h"
 
-void ast_set_index_reg_mask(struct ast_device *ast,
-			    uint32_t base, uint8_t index,
-			    uint8_t mask, uint8_t val)
-{
-	u8 tmp;
-	ast_io_write8(ast, base, index);
-	tmp = (ast_io_read8(ast, base + 1) & mask) | val;
-	ast_set_index_reg(ast, base, index, tmp);
-}
-
-uint8_t ast_get_index_reg(struct ast_device *ast,
-			  uint32_t base, uint8_t index)
-{
-	uint8_t ret;
-	ast_io_write8(ast, base, index);
-	ret = ast_io_read8(ast, base + 1);
-	return ret;
-}
-
-uint8_t ast_get_index_reg_mask(struct ast_device *ast,
-			       uint32_t base, uint8_t index, uint8_t mask)
-{
-	uint8_t ret;
-	ast_io_write8(ast, base, index);
-	ret = ast_io_read8(ast, base + 1) & mask;
-	return ret;
-}
-
 static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 {
 	struct device_node *np = dev->dev->of_node;
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 04/14] drm/ast: Remove dead else branch in POST code
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2023-06-21 12:53 ` [PATCH v2 03/14] drm/ast: Implement register helpers in ast_drv.h Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 05/14] drm/ast: Remove device POSTing and config from chip detection Thomas Zimmermann
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

According to the chip detection in ast_detect_chip(), AST2300 and
later always have a PCI revision of 0x20 or higher. Therefore the
code in ast_set_def_ext_reg() can not use the else branch when
selecing the EXT register values. Remove the dead branch and the
related values.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_post.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 0262aaafdb1c5..aa3f2cb00f82c 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -64,14 +64,12 @@ bool ast_is_vga_enabled(struct drm_device *dev)
 }
 
 static const u8 extreginfo[] = { 0x0f, 0x04, 0x1c, 0xff };
-static const u8 extreginfo_ast2300a0[] = { 0x0f, 0x04, 0x1c, 0xff };
 static const u8 extreginfo_ast2300[] = { 0x0f, 0x04, 0x1f, 0xff };
 
 static void
 ast_set_def_ext_reg(struct drm_device *dev)
 {
 	struct ast_device *ast = to_ast_device(dev);
-	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	u8 i, index, reg;
 	const u8 *ext_reg_info;
 
@@ -79,13 +77,9 @@ ast_set_def_ext_reg(struct drm_device *dev)
 	for (i = 0x81; i <= 0x9f; i++)
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
 
-	if (ast->chip == AST2300 || ast->chip == AST2400 ||
-	    ast->chip == AST2500) {
-		if (pdev->revision >= 0x20)
-			ext_reg_info = extreginfo_ast2300;
-		else
-			ext_reg_info = extreginfo_ast2300a0;
-	} else
+	if (ast->chip == AST2300 || ast->chip == AST2400 || ast->chip == AST2500)
+		ext_reg_info = extreginfo_ast2300;
+	else
 		ext_reg_info = extreginfo;
 
 	index = 0xa0;
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 05/14] drm/ast: Remove device POSTing and config from chip detection
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
                   ` (3 preceding siblings ...)
  2023-06-21 12:53 ` [PATCH v2 04/14] drm/ast: Remove dead else branch in POST code Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 06/14] drm/ast: Set PCI config before accessing I/O registers Thomas Zimmermann
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

There's way too much going on in ast_detect_chip(). Move the POST
and config code from the top of the function into the caller. No
functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_main.c | 52 ++++++++++++++++------------------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 862fdf02f6100..c6987e0446618 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -44,7 +44,6 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 
 	/* Defaults */
 	ast->config_mode = ast_use_defaults;
-	*scu_rev = 0xffffffff;
 
 	/* Check if we have device-tree properties */
 	if (np && !of_property_read_u32(np, "aspeed,scu-revision-id",
@@ -92,32 +91,11 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 	drm_info(dev, "P2A bridge disabled, using default configuration\n");
 }
 
-static int ast_detect_chip(struct drm_device *dev, bool *need_post)
+static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 {
 	struct ast_device *ast = to_ast_device(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
-	uint32_t jreg, scu_rev;
-
-	/*
-	 * If VGA isn't enabled, we need to enable now or subsequent
-	 * access to the scratch registers will fail. We also inform
-	 * our caller that it needs to POST the chip
-	 * (Assumption: VGA not enabled -> need to POST)
-	 */
-	if (!ast_is_vga_enabled(dev)) {
-		ast_enable_vga(dev);
-		drm_info(dev, "VGA not enabled on entry, requesting chip POST\n");
-		*need_post = true;
-	} else
-		*need_post = false;
-
-
-	/* Enable extended register access */
-	ast_open_key(ast);
-	ast_enable_mmio(dev);
-
-	/* Find out whether P2A works or whether to use device-tree */
-	ast_detect_config_mode(dev, &scu_rev);
+	uint32_t jreg;
 
 	/* Identify chipset */
 	if (pdev->revision >= 0x50) {
@@ -195,7 +173,7 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
 	 * is at power-on reset, otherwise we'll incorrectly "detect" a
 	 * SIL164 when there is none.
 	 */
-	if (!*need_post) {
+	if (!need_post) {
 		jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xa3, 0xff);
 		if (jreg & 0x80)
 			ast->tx_chip_types = AST_TX_SIL164_BIT;
@@ -384,8 +362,9 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
 {
 	struct drm_device *dev;
 	struct ast_device *ast;
-	bool need_post;
+	bool need_post = false;
 	int ret = 0;
+	u32 scu_rev = 0xffffffff;
 
 	ast = devm_drm_dev_alloc(&pdev->dev, drv, struct ast_device, base);
 	if (IS_ERR(ast))
@@ -420,7 +399,26 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
 			return ERR_PTR(-EIO);
 	}
 
-	ast_detect_chip(dev, &need_post);
+	if (!ast_is_vga_enabled(dev)) {
+		drm_info(dev, "VGA not enabled on entry, requesting chip POST\n");
+		need_post = true;
+	}
+
+	/*
+	 * If VGA isn't enabled, we need to enable now or subsequent
+	 * access to the scratch registers will fail.
+	 */
+	if (need_post)
+		ast_enable_vga(dev);
+
+	/* Enable extended register access */
+	ast_open_key(ast);
+	ast_enable_mmio(dev);
+
+	/* Find out whether P2A works or whether to use device-tree */
+	ast_detect_config_mode(dev, &scu_rev);
+
+	ast_detect_chip(dev, need_post, scu_rev);
 
 	ret = ast_get_dram_info(dev);
 	if (ret)
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 06/14] drm/ast: Set PCI config before accessing I/O registers
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
                   ` (4 preceding siblings ...)
  2023-06-21 12:53 ` [PATCH v2 05/14] drm/ast: Remove device POSTing and config from chip detection Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-21 13:34   ` Sui Jingfeng
  2023-06-22 15:42   ` Sui Jingfeng
  2023-06-21 12:53 ` [PATCH v2 07/14] drm/ast: Enable and unlock device access early during init Thomas Zimmermann
                   ` (8 subsequent siblings)
  14 siblings, 2 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

Access to I/O registers is required to detect and set up the
device. Enable the rsp PCI config bits before. While at it,
convert the magic number to macro constants.

Enabling the PCI config bits was done after trying to detect
the device. It was probably too late at this point.

v2:
	* use standard 16-bit PCI r/w access (Jingfeng)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_drv.h  |  1 -
 drivers/gpu/drm/ast/ast_main.c | 21 +++++++++++++++++++++
 drivers/gpu/drm/ast/ast_post.c |  6 ------
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 0141705beaee9..630105feec18a 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -52,7 +52,6 @@
 #define PCI_CHIP_AST2000 0x2000
 #define PCI_CHIP_AST2100 0x2010
 
-
 enum ast_chip {
 	AST2000,
 	AST2100,
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index c6987e0446618..01f938c2da28f 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -35,6 +35,23 @@
 
 #include "ast_drv.h"
 
+static int ast_init_pci_config(struct pci_dev *pdev)
+{
+	int err;
+	u16 pcis04;
+
+	err = pci_read_config_word(pdev, PCI_COMMAND, &pcis04);
+	if (err)
+		goto out;
+
+	pcis04 |= PCI_COMMAND_MEMORY | PCI_COMMAND_IO;
+
+	err = pci_write_config_word(pdev, PCI_COMMAND, pcis04);
+
+out:
+	return pcibios_err_to_errno(err);
+}
+
 static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 {
 	struct device_node *np = dev->dev->of_node;
@@ -399,6 +416,10 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
 			return ERR_PTR(-EIO);
 	}
 
+	ret = ast_init_pci_config(pdev);
+	if (ret)
+		return ERR_PTR(ret);
+
 	if (!ast_is_vga_enabled(dev)) {
 		drm_info(dev, "VGA not enabled on entry, requesting chip POST\n");
 		need_post = true;
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index aa3f2cb00f82c..2da5bdb4bac45 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -361,12 +361,6 @@ static void ast_init_dram_reg(struct drm_device *dev)
 void ast_post_gpu(struct drm_device *dev)
 {
 	struct ast_device *ast = to_ast_device(dev);
-	struct pci_dev *pdev = to_pci_dev(dev->dev);
-	u32 reg;
-
-	pci_read_config_dword(pdev, 0x04, &reg);
-	reg |= 0x3;
-	pci_write_config_dword(pdev, 0x04, reg);
 
 	ast_enable_vga(dev);
 	ast_open_key(ast);
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 07/14] drm/ast: Enable and unlock device access early during init
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
                   ` (5 preceding siblings ...)
  2023-06-21 12:53 ` [PATCH v2 06/14] drm/ast: Set PCI config before accessing I/O registers Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 08/14] drm/ast: Set up release action right after enabling MMIO Thomas Zimmermann
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

POST and memory management contains code to enable access to the
device's memory spaces. This is too late. Consolidate this code at
the beginning of the device initialization.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_drv.h  |  8 --------
 drivers/gpu/drm/ast/ast_main.c | 30 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/ast/ast_mm.c   |  2 --
 drivers/gpu/drm/ast/ast_post.c | 29 -----------------------------
 4 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 630105feec18a..31fead32b19cc 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -287,11 +287,6 @@ static inline void ast_set_index_reg_mask(struct ast_device *ast, u32 base, u8 i
 	ast_set_index_reg(ast, base, index, tmp);
 }
 
-static inline void ast_open_key(struct ast_device *ast)
-{
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x80, 0xA8);
-}
-
 #define AST_VIDMEM_SIZE_8M    0x00800000
 #define AST_VIDMEM_SIZE_16M   0x01000000
 #define AST_VIDMEM_SIZE_32M   0x02000000
@@ -470,9 +465,6 @@ int ast_mode_config_init(struct ast_device *ast);
 int ast_mm_init(struct ast_device *ast);
 
 /* ast post */
-void ast_enable_vga(struct drm_device *dev);
-void ast_enable_mmio(struct drm_device *dev);
-bool ast_is_vga_enabled(struct drm_device *dev);
 void ast_post_gpu(struct drm_device *dev);
 u32 ast_mindwm(struct ast_device *ast, u32 r);
 void ast_moutdwm(struct ast_device *ast, u32 r, u32 v);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 01f938c2da28f..031ff4ed1920f 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -52,6 +52,36 @@ static int ast_init_pci_config(struct pci_dev *pdev)
 	return pcibios_err_to_errno(err);
 }
 
+static bool ast_is_vga_enabled(struct drm_device *dev)
+{
+	struct ast_device *ast = to_ast_device(dev);
+	u8 ch;
+
+	ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
+
+	return !!(ch & 0x01);
+}
+
+static void ast_enable_vga(struct drm_device *dev)
+{
+	struct ast_device *ast = to_ast_device(dev);
+
+	ast_io_write8(ast, AST_IO_VGA_ENABLE_PORT, 0x01);
+	ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, 0x01);
+}
+
+static void ast_enable_mmio(struct drm_device *dev)
+{
+	struct ast_device *ast = to_ast_device(dev);
+
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
+}
+
+static void ast_open_key(struct ast_device *ast)
+{
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x80, 0xA8);
+}
+
 static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 {
 	struct device_node *np = dev->dev->of_node;
diff --git a/drivers/gpu/drm/ast/ast_mm.c b/drivers/gpu/drm/ast/ast_mm.c
index e16af60deef90..bc174bd933b97 100644
--- a/drivers/gpu/drm/ast/ast_mm.c
+++ b/drivers/gpu/drm/ast/ast_mm.c
@@ -38,8 +38,6 @@ static u32 ast_get_vram_size(struct ast_device *ast)
 	u8 jreg;
 	u32 vram_size;
 
-	ast_open_key(ast);
-
 	vram_size = AST_VIDMEM_DEFAULT_SIZE;
 	jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xaa, 0xff);
 	switch (jreg & 3) {
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 2da5bdb4bac45..b765eeb55e5f1 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -37,32 +37,6 @@
 static void ast_post_chip_2300(struct drm_device *dev);
 static void ast_post_chip_2500(struct drm_device *dev);
 
-void ast_enable_vga(struct drm_device *dev)
-{
-	struct ast_device *ast = to_ast_device(dev);
-
-	ast_io_write8(ast, AST_IO_VGA_ENABLE_PORT, 0x01);
-	ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, 0x01);
-}
-
-void ast_enable_mmio(struct drm_device *dev)
-{
-	struct ast_device *ast = to_ast_device(dev);
-
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
-}
-
-
-bool ast_is_vga_enabled(struct drm_device *dev)
-{
-	struct ast_device *ast = to_ast_device(dev);
-	u8 ch;
-
-	ch = ast_io_read8(ast, AST_IO_VGA_ENABLE_PORT);
-
-	return !!(ch & 0x01);
-}
-
 static const u8 extreginfo[] = { 0x0f, 0x04, 0x1c, 0xff };
 static const u8 extreginfo_ast2300[] = { 0x0f, 0x04, 0x1f, 0xff };
 
@@ -362,9 +336,6 @@ void ast_post_gpu(struct drm_device *dev)
 {
 	struct ast_device *ast = to_ast_device(dev);
 
-	ast_enable_vga(dev);
-	ast_open_key(ast);
-	ast_enable_mmio(dev);
 	ast_set_def_ext_reg(dev);
 
 	if (ast->chip == AST2600) {
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 08/14] drm/ast: Set up release action right after enabling MMIO
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
                   ` (6 preceding siblings ...)
  2023-06-21 12:53 ` [PATCH v2 07/14] drm/ast: Enable and unlock device access early during init Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 09/14] drm/ast: Distinguish among chip generations Thomas Zimmermann
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

Ast sets up a managed release of the MMIO access flags. Move this
code next to the MMIO access code, so that it runs if other errors
occur during the device initialization.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Sui Jingfeng <suijingfeng@loongson.cn> # AST2400
Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_main.c | 38 +++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 031ff4ed1920f..d2f8396ec0a02 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -70,11 +70,25 @@ static void ast_enable_vga(struct drm_device *dev)
 	ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, 0x01);
 }
 
-static void ast_enable_mmio(struct drm_device *dev)
+/*
+ * Run this function as part of the HW device cleanup; not
+ * when the DRM device gets released.
+ */
+static void ast_enable_mmio_release(void *data)
 {
-	struct ast_device *ast = to_ast_device(dev);
+	struct ast_device *ast = data;
+
+	/* enable standard VGA decode */
+	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
+}
+
+static int ast_enable_mmio(struct ast_device *ast)
+{
+	struct drm_device *dev = &ast->base;
 
 	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
+
+	return devm_add_action_or_reset(dev->dev, ast_enable_mmio_release, ast);
 }
 
 static void ast_open_key(struct ast_device *ast)
@@ -391,18 +405,6 @@ static int ast_get_dram_info(struct drm_device *dev)
 	return 0;
 }
 
-/*
- * Run this function as part of the HW device cleanup; not
- * when the DRM device gets released.
- */
-static void ast_device_release(void *data)
-{
-	struct ast_device *ast = data;
-
-	/* enable standard VGA decode */
-	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
-}
-
 struct ast_device *ast_device_create(const struct drm_driver *drv,
 				     struct pci_dev *pdev,
 				     unsigned long flags)
@@ -464,7 +466,9 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
 
 	/* Enable extended register access */
 	ast_open_key(ast);
-	ast_enable_mmio(dev);
+	ret = ast_enable_mmio(ast);
+	if (ret)
+		return ERR_PTR(ret);
 
 	/* Find out whether P2A works or whether to use device-tree */
 	ast_detect_config_mode(dev, &scu_rev);
@@ -497,9 +501,5 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
 	if (ret)
 		return ERR_PTR(ret);
 
-	ret = devm_add_action_or_reset(dev->dev, ast_device_release, ast);
-	if (ret)
-		return ERR_PTR(ret);
-
 	return ast;
 }
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 09/14] drm/ast: Distinguish among chip generations
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
                   ` (7 preceding siblings ...)
  2023-06-21 12:53 ` [PATCH v2 08/14] drm/ast: Set up release action right after enabling MMIO Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-22 15:10   ` [v2,09/14] " Sui Jingfeng
  2023-06-21 12:53 ` [PATCH v2 10/14] drm/ast: Detect AST 1300 model Thomas Zimmermann
                   ` (5 subsequent siblings)
  14 siblings, 1 reply; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

ASpeed distinguishes among various generations of the AST graphics
chipset with various models. [1] The most-recent model AST 2600 is
of the 7th generation, the AST 2500 is of the 6th generation, and so
on.

The ast driver simply picks one of the models as representative for
the whole generation. In several places, individual models of the
same generation need to be handled differently, which then requires
additional code for detecting the model.

Introduce different generations of the Aspeed chipset. In the source
code, refer to the generation instead of the representation model where
possible. The few places that require per-model handling are now clearly
marked.

In the enum ast_chip, we arrange each model's value such that it
encodes the generation. This allows for an easy test. The actual values
are ordered, but not of interest to the driver.

v2:
	* use __ast_gen_is_eq() (Jingfeng)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Link: https://web.archive.org/web/20141007093258/http://www.aspeedtech.com/products.php?fPath=20 # 1
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_dp501.c |  6 ++--
 drivers/gpu/drm/ast/ast_drv.h   | 56 +++++++++++++++++++++++++++------
 drivers/gpu/drm/ast/ast_main.c  | 22 ++++++-------
 drivers/gpu/drm/ast/ast_mode.c  | 35 ++++++++++-----------
 drivers/gpu/drm/ast/ast_post.c  | 27 +++++++---------
 5 files changed, 89 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 1bc35a992369d..a5d285850ffb1 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -350,7 +350,7 @@ static bool ast_init_dvo(struct drm_device *dev)
 		data |= 0x00000500;
 		ast_write32(ast, 0x12008, data);
 
-		if (ast->chip == AST2300) {
+		if (IS_AST_GEN4(ast)) {
 			data = ast_read32(ast, 0x12084);
 			/* multi-pins for DVO single-edge */
 			data |= 0xfffe0000;
@@ -366,7 +366,7 @@ static bool ast_init_dvo(struct drm_device *dev)
 			data &= 0xffffffcf;
 			data |= 0x00000020;
 			ast_write32(ast, 0x12090, data);
-		} else { /* AST2400 */
+		} else { /* AST GEN5+ */
 			data = ast_read32(ast, 0x12088);
 			/* multi-pins for DVO single-edge */
 			data |= 0x30000000;
@@ -437,7 +437,7 @@ void ast_init_3rdtx(struct drm_device *dev)
 	struct ast_device *ast = to_ast_device(dev);
 	u8 jreg;
 
-	if (ast->chip == AST2300 || ast->chip == AST2400) {
+	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast)) {
 		jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
 		switch (jreg & 0x0e) {
 		case 0x04:
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 31fead32b19cc..803d72a60506c 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -52,18 +52,38 @@
 #define PCI_CHIP_AST2000 0x2000
 #define PCI_CHIP_AST2100 0x2010
 
+#define __AST_CHIP(__gen, __index)	((__gen) << 16 | (__index))
+
 enum ast_chip {
-	AST2000,
-	AST2100,
-	AST1100,
-	AST2200,
-	AST2150,
-	AST2300,
-	AST2400,
-	AST2500,
-	AST2600,
+	/* 1st gen */
+	AST1000 = __AST_CHIP(1, 0), // unused
+	AST2000 = __AST_CHIP(1, 1),
+	/* 2nd gen */
+	AST1100 = __AST_CHIP(2, 0),
+	AST2100 = __AST_CHIP(2, 1),
+	AST2050 = __AST_CHIP(2, 2), // unused
+	/* 3rd gen */
+	AST2200 = __AST_CHIP(3, 0),
+	AST2150 = __AST_CHIP(3, 1),
+	/* 4th gen */
+	AST2300 = __AST_CHIP(4, 0),
+	AST1300 = __AST_CHIP(4, 1), // unused
+	AST1050 = __AST_CHIP(4, 2), // unused
+	/* 5th gen */
+	AST2400 = __AST_CHIP(5, 0),
+	AST1400 = __AST_CHIP(5, 1), // unused
+	AST1250 = __AST_CHIP(5, 2), // unused
+	/* 6th gen */
+	AST2500 = __AST_CHIP(6, 0),
+	AST2510 = __AST_CHIP(6, 1), // unused
+	AST2520 = __AST_CHIP(6, 2), // unused
+	/* 7th gen */
+	AST2600 = __AST_CHIP(7, 0),
+	AST2620 = __AST_CHIP(7, 1), // unused
 };
 
+#define __AST_CHIP_GEN(__chip)	(((unsigned long)(__chip)) >> 16)
+
 enum ast_tx_chip {
 	AST_TX_NONE,
 	AST_TX_SIL164,
@@ -217,6 +237,24 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
 				     struct pci_dev *pdev,
 				     unsigned long flags);
 
+static inline unsigned long __ast_gen(struct ast_device *ast)
+{
+	return __AST_CHIP_GEN(ast->chip);
+}
+#define AST_GEN(__ast)	__ast_gen(__ast)
+
+static inline bool __ast_gen_is_eq(struct ast_device *ast, unsigned long gen)
+{
+	return __ast_gen(ast) == gen;
+}
+#define IS_AST_GEN1(__ast)	__ast_gen_is_eq(__ast, 1)
+#define IS_AST_GEN2(__ast)	__ast_gen_is_eq(__ast, 2)
+#define IS_AST_GEN3(__ast)	__ast_gen_is_eq(__ast, 3)
+#define IS_AST_GEN4(__ast)	__ast_gen_is_eq(__ast, 4)
+#define IS_AST_GEN5(__ast)	__ast_gen_is_eq(__ast, 5)
+#define IS_AST_GEN6(__ast)	__ast_gen_is_eq(__ast, 6)
+#define IS_AST_GEN7(__ast)	__ast_gen_is_eq(__ast, 7)
+
 #define AST_IO_AR_PORT_WRITE		(0x40)
 #define AST_IO_MISC_PORT_WRITE		(0x42)
 #define AST_IO_VGA_ENABLE_PORT		(0x43)
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index d2f8396ec0a02..8fc412fe296c2 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -127,7 +127,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
 	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
 	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
-		/* Patch AST2500 */
+		/* Patch GEN6 */
 		if (((pdev->revision & 0xF0) == 0x40)
 			&& ((jregd0 & AST_VRAM_INIT_STATUS_MASK) == 0))
 			ast_patch_ahb_2500(ast);
@@ -196,8 +196,8 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 	}
 
 	/* Check if we support wide screen */
-	switch (ast->chip) {
-	case AST2000:
+	switch (AST_GEN(ast)) {
+	case 1:
 		ast->support_wide_screen = false;
 		break;
 	default:
@@ -217,7 +217,7 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 			if (ast->chip == AST2500 &&
 			    scu_rev == 0x100)           /* ast2510 */
 				ast->support_wide_screen = true;
-			if (ast->chip == AST2600)		/* ast2600 */
+			if (IS_AST_GEN7(ast))
 				ast->support_wide_screen = true;
 		}
 		break;
@@ -240,9 +240,9 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 			ast->tx_chip_types = AST_TX_SIL164_BIT;
 	}
 
-	if ((ast->chip == AST2300) || (ast->chip == AST2400) || (ast->chip == AST2500)) {
+	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast)) {
 		/*
-		 * On AST2300 and 2400, look the configuration set by the SoC in
+		 * On AST GEN4+, look the configuration set by the SoC in
 		 * the SOC scratch register #1 bits 11:8 (interestingly marked
 		 * as "reserved" in the spec)
 		 */
@@ -264,7 +264,7 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 		case 0x0c:
 			ast->tx_chip_types = AST_TX_DP501_BIT;
 		}
-	} else if (ast->chip == AST2600) {
+	} else if (IS_AST_GEN7(ast)) {
 		if (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, TX_TYPE_MASK) ==
 		    ASTDP_DPMCU_TX) {
 			ast->tx_chip_types = AST_TX_ASTDP_BIT;
@@ -296,7 +296,7 @@ static int ast_get_dram_info(struct drm_device *dev)
 	case ast_use_dt:
 		/*
 		 * If some properties are missing, use reasonable
-		 * defaults for AST2400
+		 * defaults for GEN5
 		 */
 		if (of_property_read_u32(np, "aspeed,mcr-configuration",
 					 &mcr_cfg))
@@ -319,7 +319,7 @@ static int ast_get_dram_info(struct drm_device *dev)
 	default:
 		ast->dram_bus_width = 16;
 		ast->dram_type = AST_DRAM_1Gx16;
-		if (ast->chip == AST2500)
+		if (IS_AST_GEN6(ast))
 			ast->mclk = 800;
 		else
 			ast->mclk = 396;
@@ -331,7 +331,7 @@ static int ast_get_dram_info(struct drm_device *dev)
 	else
 		ast->dram_bus_width = 32;
 
-	if (ast->chip == AST2500) {
+	if (IS_AST_GEN6(ast)) {
 		switch (mcr_cfg & 0x03) {
 		case 0:
 			ast->dram_type = AST_DRAM_1Gx16;
@@ -347,7 +347,7 @@ static int ast_get_dram_info(struct drm_device *dev)
 			ast->dram_type = AST_DRAM_8Gx16;
 			break;
 		}
-	} else if (ast->chip == AST2300 || ast->chip == AST2400) {
+	} else if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast)) {
 		switch (mcr_cfg & 0x03) {
 		case 0:
 			ast->dram_type = AST_DRAM_512Mx16;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index b3c670af6ef2b..f711d592da52b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -342,7 +342,7 @@ static void ast_set_crtc_reg(struct ast_device *ast,
 	u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD = 0, jregAE = 0;
 	u16 temp, precache = 0;
 
-	if ((ast->chip == AST2500 || ast->chip == AST2600) &&
+	if ((IS_AST_GEN6(ast) || IS_AST_GEN7(ast)) &&
 	    (vbios_mode->enh_table->flags & AST2500PreCatchCRT))
 		precache = 40;
 
@@ -384,7 +384,7 @@ static void ast_set_crtc_reg(struct ast_device *ast,
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xAD, 0x00, jregAD);
 
 	// Workaround for HSync Time non octave pixels (1920x1080@60Hz HSync 44 pixels);
-	if ((ast->chip == AST2600) && (mode->crtc_vdisplay == 1080))
+	if (IS_AST_GEN7(ast) && (mode->crtc_vdisplay == 1080))
 		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xFC, 0xFD, 0x02);
 	else
 		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xFC, 0xFD, 0x00);
@@ -466,7 +466,7 @@ static void ast_set_dclk_reg(struct ast_device *ast,
 {
 	const struct ast_vbios_dclk_info *clk_info;
 
-	if ((ast->chip == AST2500) || (ast->chip == AST2600))
+	if (IS_AST_GEN6(ast) || IS_AST_GEN7(ast))
 		clk_info = &dclk_table_ast2500[vbios_mode->enh_table->dclk_index];
 	else
 		clk_info = &dclk_table[vbios_mode->enh_table->dclk_index];
@@ -510,17 +510,13 @@ static void ast_set_color_reg(struct ast_device *ast,
 static void ast_set_crtthd_reg(struct ast_device *ast)
 {
 	/* Set Threshold */
-	if (ast->chip == AST2600) {
+	if (IS_AST_GEN7(ast)) {
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0xe0);
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0xa0);
-	} else if (ast->chip == AST2300 || ast->chip == AST2400 ||
-	    ast->chip == AST2500) {
+	} else if (IS_AST_GEN6(ast) || IS_AST_GEN5(ast) || IS_AST_GEN4(ast)) {
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0x78);
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0x60);
-	} else if (ast->chip == AST2100 ||
-		   ast->chip == AST1100 ||
-		   ast->chip == AST2200 ||
-		   ast->chip == AST2150) {
+	} else if (IS_AST_GEN3(ast) || IS_AST_GEN2(ast)) {
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0x3f);
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0x2f);
 	} else {
@@ -1082,9 +1078,10 @@ ast_crtc_helper_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode
 		if ((mode->hdisplay == 1152) && (mode->vdisplay == 864))
 			return MODE_OK;
 
-		if ((ast->chip == AST2100) || (ast->chip == AST2200) ||
-		    (ast->chip == AST2300) || (ast->chip == AST2400) ||
-		    (ast->chip == AST2500) || (ast->chip == AST2600)) {
+		if ((ast->chip == AST2100) || // GEN2, but not AST1100 (?)
+		    (ast->chip == AST2200) || // GEN3, but not AST2150 (?)
+		    IS_AST_GEN4(ast) || IS_AST_GEN5(ast) ||
+		    IS_AST_GEN6(ast) || IS_AST_GEN7(ast)) {
 			if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080))
 				return MODE_OK;
 
@@ -1800,12 +1797,12 @@ int ast_mode_config_init(struct ast_device *ast)
 	dev->mode_config.min_height = 0;
 	dev->mode_config.preferred_depth = 24;
 
-	if (ast->chip == AST2100 ||
-	    ast->chip == AST2200 ||
-	    ast->chip == AST2300 ||
-	    ast->chip == AST2400 ||
-	    ast->chip == AST2500 ||
-	    ast->chip == AST2600) {
+	if (ast->chip == AST2100 || // GEN2, but not AST1100 (?)
+	    ast->chip == AST2200 || // GEN3, but not AST2150 (?)
+	    IS_AST_GEN7(ast) ||
+	    IS_AST_GEN6(ast) ||
+	    IS_AST_GEN5(ast) ||
+	    IS_AST_GEN4(ast)) {
 		dev->mode_config.max_width = 1920;
 		dev->mode_config.max_height = 2048;
 	} else {
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index b765eeb55e5f1..13e15173f2c5b 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -51,7 +51,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
 	for (i = 0x81; i <= 0x9f; i++)
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
 
-	if (ast->chip == AST2300 || ast->chip == AST2400 || ast->chip == AST2500)
+	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast))
 		ext_reg_info = extreginfo_ast2300;
 	else
 		ext_reg_info = extreginfo;
@@ -72,8 +72,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
 
 	/* Enable RAMDAC for A1 */
 	reg = 0x04;
-	if (ast->chip == AST2300 || ast->chip == AST2400 ||
-	    ast->chip == AST2500)
+	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast))
 		reg |= 0x20;
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0xff, reg);
 }
@@ -249,7 +248,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
 	j = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
 
 	if ((j & 0x80) == 0) { /* VGA only */
-		if (ast->chip == AST2000) {
+		if (IS_AST_GEN1(ast)) {
 			dram_reg_info = ast2000_dram_table_data;
 			ast_write32(ast, 0xf004, 0x1e6e0000);
 			ast_write32(ast, 0xf000, 0x1);
@@ -258,7 +257,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
 			do {
 				;
 			} while (ast_read32(ast, 0x10100) != 0xa8);
-		} else {/* AST2100/1100 */
+		} else { /* GEN2/GEN3 */
 			if (ast->chip == AST2100 || ast->chip == AST2200)
 				dram_reg_info = ast2100_dram_table_data;
 			else
@@ -281,7 +280,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
 			if (dram_reg_info->index == 0xff00) {/* delay fn */
 				for (i = 0; i < 15; i++)
 					udelay(dram_reg_info->data);
-			} else if (dram_reg_info->index == 0x4 && ast->chip != AST2000) {
+			} else if (dram_reg_info->index == 0x4 && !IS_AST_GEN1(ast)) {
 				data = dram_reg_info->data;
 				if (ast->dram_type == AST_DRAM_1Gx16)
 					data = 0x00000d89;
@@ -307,15 +306,13 @@ static void ast_init_dram_reg(struct drm_device *dev)
 				cbrdlli_ast2150(ast, 32); /* 32 bits */
 		}
 
-		switch (ast->chip) {
-		case AST2000:
+		switch (AST_GEN(ast)) {
+		case 1:
 			temp = ast_read32(ast, 0x10140);
 			ast_write32(ast, 0x10140, temp | 0x40);
 			break;
-		case AST1100:
-		case AST2100:
-		case AST2200:
-		case AST2150:
+		case 2:
+		case 3:
 			temp = ast_read32(ast, 0x1200c);
 			ast_write32(ast, 0x1200c, temp & 0xfffffffd);
 			temp = ast_read32(ast, 0x12040);
@@ -338,13 +335,13 @@ void ast_post_gpu(struct drm_device *dev)
 
 	ast_set_def_ext_reg(dev);
 
-	if (ast->chip == AST2600) {
+	if (IS_AST_GEN7(ast)) {
 		if (ast->tx_chip_types & AST_TX_ASTDP_BIT)
 			ast_dp_launch(dev);
 	} else if (ast->config_mode == ast_use_p2a) {
-		if (ast->chip == AST2500)
+		if (IS_AST_GEN6(ast))
 			ast_post_chip_2500(dev);
-		else if (ast->chip == AST2300 || ast->chip == AST2400)
+		else if (IS_AST_GEN5(ast) || IS_AST_GEN4(ast))
 			ast_post_chip_2300(dev);
 		else
 			ast_init_dram_reg(dev);
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 10/14] drm/ast: Detect AST 1300 model
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
                   ` (8 preceding siblings ...)
  2023-06-21 12:53 ` [PATCH v2 09/14] drm/ast: Distinguish among chip generations Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 11/14] drm/ast: Detect AST 1400 model Thomas Zimmermann
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

Detect the 4th-generation AST 1300. Allows to simplify the code
for widescreen support.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_drv.h  |  2 +-
 drivers/gpu/drm/ast/ast_main.c | 15 +++++++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 803d72a60506c..8199834f8fbe0 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -67,7 +67,7 @@ enum ast_chip {
 	AST2150 = __AST_CHIP(3, 1),
 	/* 4th gen */
 	AST2300 = __AST_CHIP(4, 0),
-	AST1300 = __AST_CHIP(4, 1), // unused
+	AST1300 = __AST_CHIP(4, 1),
 	AST1050 = __AST_CHIP(4, 2), // unused
 	/* 5th gen */
 	AST2400 = __AST_CHIP(5, 0),
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 8fc412fe296c2..7513924a5437b 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -169,8 +169,16 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 		ast->chip = AST2400;
 		drm_info(dev, "AST 2400 detected\n");
 	} else if (pdev->revision >= 0x20) {
-		ast->chip = AST2300;
-		drm_info(dev, "AST 2300 detected\n");
+		switch (scu_rev & 0x300) {
+		case 0x0000:
+			ast->chip = AST1300;
+			drm_info(dev, "AST 1300 detected\n");
+			break;
+		default:
+			ast->chip = AST2300;
+			drm_info(dev, "AST 2300 detected\n");
+			break;
+		}
 	} else if (pdev->revision >= 0x10) {
 		switch (scu_rev & 0x0300) {
 		case 0x0200:
@@ -208,8 +216,7 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 			ast->support_wide_screen = true;
 		else {
 			ast->support_wide_screen = false;
-			if (ast->chip == AST2300 &&
-			    (scu_rev & 0x300) == 0x0) /* ast1300 */
+			if (ast->chip == AST1300)
 				ast->support_wide_screen = true;
 			if (ast->chip == AST2400 &&
 			    (scu_rev & 0x300) == 0x100) /* ast1400 */
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 11/14] drm/ast: Detect AST 1400 model
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
                   ` (9 preceding siblings ...)
  2023-06-21 12:53 ` [PATCH v2 10/14] drm/ast: Detect AST 1300 model Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 12/14] drm/ast: Detect AST 2510 model Thomas Zimmermann
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

Detect the 5th-generation AST 1400. Allows to simplify the code
for widescreen support.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_drv.h  |  2 +-
 drivers/gpu/drm/ast/ast_main.c | 14 ++++++++++----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 8199834f8fbe0..876ebbd3b60e0 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -71,7 +71,7 @@ enum ast_chip {
 	AST1050 = __AST_CHIP(4, 2), // unused
 	/* 5th gen */
 	AST2400 = __AST_CHIP(5, 0),
-	AST1400 = __AST_CHIP(5, 1), // unused
+	AST1400 = __AST_CHIP(5, 1),
 	AST1250 = __AST_CHIP(5, 2), // unused
 	/* 6th gen */
 	AST2500 = __AST_CHIP(6, 0),
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 7513924a5437b..cbfe93c7929d4 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -166,8 +166,15 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 		ast->chip = AST2500;
 		drm_info(dev, "AST 2500 detected\n");
 	} else if (pdev->revision >= 0x30) {
-		ast->chip = AST2400;
-		drm_info(dev, "AST 2400 detected\n");
+		switch (scu_rev & 0x300) {
+		case 0x0100:
+			ast->chip = AST1400;
+			drm_info(dev, "AST 1400 detected\n");
+			break;
+		default:
+			ast->chip = AST2400;
+			drm_info(dev, "AST 2400 detected\n");
+		}
 	} else if (pdev->revision >= 0x20) {
 		switch (scu_rev & 0x300) {
 		case 0x0000:
@@ -218,8 +225,7 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 			ast->support_wide_screen = false;
 			if (ast->chip == AST1300)
 				ast->support_wide_screen = true;
-			if (ast->chip == AST2400 &&
-			    (scu_rev & 0x300) == 0x100) /* ast1400 */
+			if (ast->chip == AST1400)
 				ast->support_wide_screen = true;
 			if (ast->chip == AST2500 &&
 			    scu_rev == 0x100)           /* ast2510 */
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 12/14] drm/ast: Detect AST 2510 model
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
                   ` (10 preceding siblings ...)
  2023-06-21 12:53 ` [PATCH v2 11/14] drm/ast: Detect AST 1400 model Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 13/14] drm/ast: Move widescreen and tx-chip detection into separate helpers Thomas Zimmermann
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

Detect the 6th-generation AST 2510. Allows to simplify the code
for widescreen support.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_drv.h  |  2 +-
 drivers/gpu/drm/ast/ast_main.c | 14 ++++++++++----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 876ebbd3b60e0..3f6e0c984523a 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -75,7 +75,7 @@ enum ast_chip {
 	AST1250 = __AST_CHIP(5, 2), // unused
 	/* 6th gen */
 	AST2500 = __AST_CHIP(6, 0),
-	AST2510 = __AST_CHIP(6, 1), // unused
+	AST2510 = __AST_CHIP(6, 1),
 	AST2520 = __AST_CHIP(6, 2), // unused
 	/* 7th gen */
 	AST2600 = __AST_CHIP(7, 0),
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index cbfe93c7929d4..f2f8a054c52cf 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -163,8 +163,15 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 		ast->chip = AST2600;
 		drm_info(dev, "AST 2600 detected\n");
 	} else if (pdev->revision >= 0x40) {
-		ast->chip = AST2500;
-		drm_info(dev, "AST 2500 detected\n");
+		switch (scu_rev & 0x300) {
+		case 0x0100:
+			ast->chip = AST2510;
+			drm_info(dev, "AST 2510 detected\n");
+			break;
+		default:
+			ast->chip = AST2500;
+			drm_info(dev, "AST 2500 detected\n");
+		}
 	} else if (pdev->revision >= 0x30) {
 		switch (scu_rev & 0x300) {
 		case 0x0100:
@@ -227,8 +234,7 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 				ast->support_wide_screen = true;
 			if (ast->chip == AST1400)
 				ast->support_wide_screen = true;
-			if (ast->chip == AST2500 &&
-			    scu_rev == 0x100)           /* ast2510 */
+			if (ast->chip == AST2510)
 				ast->support_wide_screen = true;
 			if (IS_AST_GEN7(ast))
 				ast->support_wide_screen = true;
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 13/14] drm/ast: Move widescreen and tx-chip detection into separate helpers
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
                   ` (11 preceding siblings ...)
  2023-06-21 12:53 ` [PATCH v2 12/14] drm/ast: Detect AST 2510 model Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-21 12:53 ` [PATCH v2 14/14] drm/ast: Merge config and chip detection Thomas Zimmermann
  2023-06-28  6:03 ` [PATCH v2 00/14] drm/ast: Refactor the device-detection code Jammy Huang
  14 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

Split ast_detect_chip() into three functions and call them one by
one. The new functions detect the transmitter chip and widescreen
support. This will allow for further refactoring.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
Tested-by: Sui Jingfeng <suijingfeng@loongson.cn> # AST2400
---
 drivers/gpu/drm/ast/ast_main.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f2f8a054c52cf..7ade96f1f37f9 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -156,7 +156,6 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 {
 	struct ast_device *ast = to_ast_device(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
-	uint32_t jreg;
 
 	/* Identify chipset */
 	if (pdev->revision >= 0x50) {
@@ -217,6 +216,13 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 		drm_info(dev, "AST 2000 detected\n");
 	}
 
+	return 0;
+}
+
+static void ast_detect_widescreen(struct ast_device *ast)
+{
+	u8 jreg;
+
 	/* Check if we support wide screen */
 	switch (AST_GEN(ast)) {
 	case 1:
@@ -241,6 +247,12 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 		}
 		break;
 	}
+}
+
+static void ast_detect_tx_chip(struct ast_device *ast, bool need_post)
+{
+	struct drm_device *dev = &ast->base;
+	u8 jreg;
 
 	/* Check 3rd Tx option (digital output afaik) */
 	ast->tx_chip_types |= AST_TX_NONE_BIT;
@@ -300,8 +312,6 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
 		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");
-
-	return 0;
 }
 
 static int ast_get_dram_info(struct drm_device *dev)
@@ -493,6 +503,8 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
 	ast_detect_config_mode(dev, &scu_rev);
 
 	ast_detect_chip(dev, need_post, scu_rev);
+	ast_detect_widescreen(ast);
+	ast_detect_tx_chip(ast, need_post);
 
 	ret = ast_get_dram_info(dev);
 	if (ret)
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH v2 14/14] drm/ast: Merge config and chip detection
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
                   ` (12 preceding siblings ...)
  2023-06-21 12:53 ` [PATCH v2 13/14] drm/ast: Move widescreen and tx-chip detection into separate helpers Thomas Zimmermann
@ 2023-06-21 12:53 ` Thomas Zimmermann
  2023-06-22 15:21   ` [v2,14/14] " Sui Jingfeng
  2023-06-28  6:03 ` [PATCH v2 00/14] drm/ast: Refactor the device-detection code Jammy Huang
  14 siblings, 1 reply; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-21 12:53 UTC (permalink / raw)
  To: airlied, jfalempe, daniel, jammy_huang, suijingfeng
  Cc: Thomas Zimmermann, dri-devel

Detection of the configuration mode and the chipset model are
linked to each other. One uses values from the other; namely the
PCI device revision and the SCU revision. Merge this code into
a single function.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
---
 drivers/gpu/drm/ast/ast_main.c | 108 +++++++++++++++++----------------
 1 file changed, 57 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 7ade96f1f37f9..8bfbdfd86d77d 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -96,68 +96,75 @@ static void ast_open_key(struct ast_device *ast)
 	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x80, 0xA8);
 }
 
-static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
+static int ast_device_config_init(struct ast_device *ast)
 {
-	struct device_node *np = dev->dev->of_node;
-	struct ast_device *ast = to_ast_device(dev);
+	struct drm_device *dev = &ast->base;
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
-	uint32_t data, jregd0, jregd1;
+	struct device_node *np = dev->dev->of_node;
+	uint32_t scu_rev = 0xffffffff;
+	u32 data;
+	u8 jregd0, jregd1;
+
+	/*
+	 * Find configuration mode and read SCU revision
+	 */
 
-	/* Defaults */
 	ast->config_mode = ast_use_defaults;
 
 	/* Check if we have device-tree properties */
-	if (np && !of_property_read_u32(np, "aspeed,scu-revision-id",
-					scu_rev)) {
+	if (np && !of_property_read_u32(np, "aspeed,scu-revision-id", &data)) {
 		/* We do, disable P2A access */
 		ast->config_mode = ast_use_dt;
-		drm_info(dev, "Using device-tree for configuration\n");
-		return;
-	}
+		scu_rev = data;
+	} else if (pdev->device == PCI_CHIP_AST2000) { // Not all families have a P2A bridge
+		/*
+		 * The BMC will set SCU 0x40 D[12] to 1 if the P2 bridge
+		 * is disabled. We force using P2A if VGA only mode bit
+		 * is set D[7]
+		 */
+		jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
+		jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
+		if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
+
+			/*
+			 * We have a P2A bridge and it is enabled.
+			 */
+
+			/* Patch AST2500/AST2510 */
+			if ((pdev->revision & 0xf0) == 0x40) {
+				if (!(jregd0 & AST_VRAM_INIT_STATUS_MASK))
+					ast_patch_ahb_2500(ast);
+			}
 
-	/* Not all families have a P2A bridge */
-	if (pdev->device != PCI_CHIP_AST2000)
-		return;
+			/* Double check that it's actually working */
+			data = ast_read32(ast, 0xf004);
+			if ((data != 0xffffffff) && (data != 0x00)) {
+				ast->config_mode = ast_use_p2a;
 
-	/*
-	 * The BMC will set SCU 0x40 D[12] to 1 if the P2 bridge
-	 * is disabled. We force using P2A if VGA only mode bit
-	 * is set D[7]
-	 */
-	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
-	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
-	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
-		/* Patch GEN6 */
-		if (((pdev->revision & 0xF0) == 0x40)
-			&& ((jregd0 & AST_VRAM_INIT_STATUS_MASK) == 0))
-			ast_patch_ahb_2500(ast);
-
-		/* Double check it's actually working */
-		data = ast_read32(ast, 0xf004);
-		if ((data != 0xFFFFFFFF) && (data != 0x00)) {
-			/* P2A works, grab silicon revision */
-			ast->config_mode = ast_use_p2a;
-
-			drm_info(dev, "Using P2A bridge for configuration\n");
-
-			/* Read SCU7c (silicon revision register) */
-			ast_write32(ast, 0xf004, 0x1e6e0000);
-			ast_write32(ast, 0xf000, 0x1);
-			*scu_rev = ast_read32(ast, 0x1207c);
-			return;
+				/* Read SCU7c (silicon revision register) */
+				ast_write32(ast, 0xf004, 0x1e6e0000);
+				ast_write32(ast, 0xf000, 0x1);
+				scu_rev = ast_read32(ast, 0x1207c);
+			}
 		}
 	}
 
-	/* We have a P2A bridge but it's disabled */
-	drm_info(dev, "P2A bridge disabled, using default configuration\n");
-}
+	switch (ast->config_mode) {
+	case ast_use_defaults:
+		drm_info(dev, "Using default configuration\n");
+		break;
+	case ast_use_dt:
+		drm_info(dev, "Using device-tree for configuration\n");
+		break;
+	case ast_use_p2a:
+		drm_info(dev, "Using P2A bridge for configuration\n");
+		break;
+	}
 
-static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
-{
-	struct ast_device *ast = to_ast_device(dev);
-	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	/*
+	 * Identify chipset
+	 */
 
-	/* Identify chipset */
 	if (pdev->revision >= 0x50) {
 		ast->chip = AST2600;
 		drm_info(dev, "AST 2600 detected\n");
@@ -442,7 +449,6 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
 	struct ast_device *ast;
 	bool need_post = false;
 	int ret = 0;
-	u32 scu_rev = 0xffffffff;
 
 	ast = devm_drm_dev_alloc(&pdev->dev, drv, struct ast_device, base);
 	if (IS_ERR(ast))
@@ -499,10 +505,10 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
 	if (ret)
 		return ERR_PTR(ret);
 
-	/* Find out whether P2A works or whether to use device-tree */
-	ast_detect_config_mode(dev, &scu_rev);
+	ret = ast_device_config_init(ast);
+	if (ret)
+		return ERR_PTR(ret);
 
-	ast_detect_chip(dev, need_post, scu_rev);
 	ast_detect_widescreen(ast);
 	ast_detect_tx_chip(ast, need_post);
 
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* Re: [PATCH v2 06/14] drm/ast: Set PCI config before accessing I/O registers
  2023-06-21 12:53 ` [PATCH v2 06/14] drm/ast: Set PCI config before accessing I/O registers Thomas Zimmermann
@ 2023-06-21 13:34   ` Sui Jingfeng
  2023-06-22 15:42   ` Sui Jingfeng
  1 sibling, 0 replies; 22+ messages in thread
From: Sui Jingfeng @ 2023-06-21 13:34 UTC (permalink / raw)
  To: Thomas Zimmermann, airlied, jfalempe, daniel, jammy_huang; +Cc: dri-devel

Hi,

LGTM

On 2023/6/21 20:53, Thomas Zimmermann wrote:
> Access to I/O registers is required to detect and set up the
> device. Enable the rsp PCI config bits before. While at it,
> convert the magic number to macro constants.
>
> Enabling the PCI config bits was done after trying to detect
> the device. It was probably too late at this point.
>
> v2:
> 	* use standard 16-bit PCI r/w access (Jingfeng)
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
> Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>
> ---
>   drivers/gpu/drm/ast/ast_drv.h  |  1 -
>   drivers/gpu/drm/ast/ast_main.c | 21 +++++++++++++++++++++
>   drivers/gpu/drm/ast/ast_post.c |  6 ------
>   3 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 0141705beaee9..630105feec18a 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -52,7 +52,6 @@
>   #define PCI_CHIP_AST2000 0x2000
>   #define PCI_CHIP_AST2100 0x2010
>   
> -
>   enum ast_chip {
>   	AST2000,
>   	AST2100,
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index c6987e0446618..01f938c2da28f 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -35,6 +35,23 @@
>   
>   #include "ast_drv.h"
>   
> +static int ast_init_pci_config(struct pci_dev *pdev)
> +{
> +	int err;
> +	u16 pcis04;
> +
> +	err = pci_read_config_word(pdev, PCI_COMMAND, &pcis04);
> +	if (err)
> +		goto out;
> +
> +	pcis04 |= PCI_COMMAND_MEMORY | PCI_COMMAND_IO;
> +
> +	err = pci_write_config_word(pdev, PCI_COMMAND, pcis04);
> +
> +out:
> +	return pcibios_err_to_errno(err);
> +}
> +
>   static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>   {
>   	struct device_node *np = dev->dev->of_node;
> @@ -399,6 +416,10 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
>   			return ERR_PTR(-EIO);
>   	}
>   
> +	ret = ast_init_pci_config(pdev);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
>   	if (!ast_is_vga_enabled(dev)) {
>   		drm_info(dev, "VGA not enabled on entry, requesting chip POST\n");
>   		need_post = true;
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index aa3f2cb00f82c..2da5bdb4bac45 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -361,12 +361,6 @@ static void ast_init_dram_reg(struct drm_device *dev)
>   void ast_post_gpu(struct drm_device *dev)
>   {
>   	struct ast_device *ast = to_ast_device(dev);
> -	struct pci_dev *pdev = to_pci_dev(dev->dev);
> -	u32 reg;
> -
> -	pci_read_config_dword(pdev, 0x04, &reg);
> -	reg |= 0x3;
> -	pci_write_config_dword(pdev, 0x04, reg);
>   
>   	ast_enable_vga(dev);
>   	ast_open_key(ast);

-- 
Jingfeng


^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [v2,09/14] drm/ast: Distinguish among chip generations
  2023-06-21 12:53 ` [PATCH v2 09/14] drm/ast: Distinguish among chip generations Thomas Zimmermann
@ 2023-06-22 15:10   ` Sui Jingfeng
  0 siblings, 0 replies; 22+ messages in thread
From: Sui Jingfeng @ 2023-06-22 15:10 UTC (permalink / raw)
  To: Thomas Zimmermann, airlied, jfalempe, daniel, jammy_huang; +Cc: dri-devel

Hi,

On 2023/6/21 20:53, Thomas Zimmermann wrote:
> ASpeed distinguishes among various generations of the AST graphics
> chipset with various models. [1] The most-recent model AST 2600 is
> of the 7th generation, the AST 2500 is of the 6th generation, and so
> on.
>
> The ast driver simply picks one of the models as representative for
> the whole generation. In several places, individual models of the
> same generation need to be handled differently, which then requires
> additional code for detecting the model.
>
> Introduce different generations of the Aspeed chipset. In the source
> code, refer to the generation instead of the representation model where
> possible. The few places that require per-model handling are now clearly
> marked.
>
> In the enum ast_chip, we arrange each model's value such that it
> encodes the generation. This allows for an easy test. The actual values
> are ordered, but not of interest to the driver.
>
> v2:
> 	* use __ast_gen_is_eq() (Jingfeng)
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Link: https://web.archive.org/web/20141007093258/http://www.aspeedtech.com/products.php?fPath=20 # 1
> Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>

> Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
> ---
>   drivers/gpu/drm/ast/ast_dp501.c |  6 ++--
>   drivers/gpu/drm/ast/ast_drv.h   | 56 +++++++++++++++++++++++++++------
>   drivers/gpu/drm/ast/ast_main.c  | 22 ++++++-------
>   drivers/gpu/drm/ast/ast_mode.c  | 35 ++++++++++-----------
>   drivers/gpu/drm/ast/ast_post.c  | 27 +++++++---------
>   5 files changed, 89 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
> index 1bc35a992369d..a5d285850ffb1 100644
> --- a/drivers/gpu/drm/ast/ast_dp501.c
> +++ b/drivers/gpu/drm/ast/ast_dp501.c
> @@ -350,7 +350,7 @@ static bool ast_init_dvo(struct drm_device *dev)
>   		data |= 0x00000500;
>   		ast_write32(ast, 0x12008, data);
>   
> -		if (ast->chip == AST2300) {
> +		if (IS_AST_GEN4(ast)) {
>   			data = ast_read32(ast, 0x12084);
>   			/* multi-pins for DVO single-edge */
>   			data |= 0xfffe0000;
> @@ -366,7 +366,7 @@ static bool ast_init_dvo(struct drm_device *dev)
>   			data &= 0xffffffcf;
>   			data |= 0x00000020;
>   			ast_write32(ast, 0x12090, data);
> -		} else { /* AST2400 */
> +		} else { /* AST GEN5+ */
>   			data = ast_read32(ast, 0x12088);
>   			/* multi-pins for DVO single-edge */
>   			data |= 0x30000000;
> @@ -437,7 +437,7 @@ void ast_init_3rdtx(struct drm_device *dev)
>   	struct ast_device *ast = to_ast_device(dev);
>   	u8 jreg;
>   
> -	if (ast->chip == AST2300 || ast->chip == AST2400) {
> +	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast)) {
>   		jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
>   		switch (jreg & 0x0e) {
>   		case 0x04:
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 31fead32b19cc..803d72a60506c 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -52,18 +52,38 @@
>   #define PCI_CHIP_AST2000 0x2000
>   #define PCI_CHIP_AST2100 0x2010
>   
> +#define __AST_CHIP(__gen, __index)	((__gen) << 16 | (__index))
> +
>   enum ast_chip {
> -	AST2000,
> -	AST2100,
> -	AST1100,
> -	AST2200,
> -	AST2150,
> -	AST2300,
> -	AST2400,
> -	AST2500,
> -	AST2600,
> +	/* 1st gen */
> +	AST1000 = __AST_CHIP(1, 0), // unused
> +	AST2000 = __AST_CHIP(1, 1),
> +	/* 2nd gen */
> +	AST1100 = __AST_CHIP(2, 0),
> +	AST2100 = __AST_CHIP(2, 1),
> +	AST2050 = __AST_CHIP(2, 2), // unused
> +	/* 3rd gen */
> +	AST2200 = __AST_CHIP(3, 0),
> +	AST2150 = __AST_CHIP(3, 1),
> +	/* 4th gen */
> +	AST2300 = __AST_CHIP(4, 0),
> +	AST1300 = __AST_CHIP(4, 1), // unused
> +	AST1050 = __AST_CHIP(4, 2), // unused
> +	/* 5th gen */
> +	AST2400 = __AST_CHIP(5, 0),
> +	AST1400 = __AST_CHIP(5, 1), // unused
> +	AST1250 = __AST_CHIP(5, 2), // unused
> +	/* 6th gen */
> +	AST2500 = __AST_CHIP(6, 0),
> +	AST2510 = __AST_CHIP(6, 1), // unused
> +	AST2520 = __AST_CHIP(6, 2), // unused
> +	/* 7th gen */
> +	AST2600 = __AST_CHIP(7, 0),
> +	AST2620 = __AST_CHIP(7, 1), // unused
>   };
>   
> +#define __AST_CHIP_GEN(__chip)	(((unsigned long)(__chip)) >> 16)
> +
>   enum ast_tx_chip {
>   	AST_TX_NONE,
>   	AST_TX_SIL164,
> @@ -217,6 +237,24 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
>   				     struct pci_dev *pdev,
>   				     unsigned long flags);
>   
> +static inline unsigned long __ast_gen(struct ast_device *ast)
> +{
> +	return __AST_CHIP_GEN(ast->chip);
> +}
> +#define AST_GEN(__ast)	__ast_gen(__ast)
> +
> +static inline bool __ast_gen_is_eq(struct ast_device *ast, unsigned long gen)
> +{
> +	return __ast_gen(ast) == gen;
> +}
> +#define IS_AST_GEN1(__ast)	__ast_gen_is_eq(__ast, 1)
> +#define IS_AST_GEN2(__ast)	__ast_gen_is_eq(__ast, 2)
> +#define IS_AST_GEN3(__ast)	__ast_gen_is_eq(__ast, 3)
> +#define IS_AST_GEN4(__ast)	__ast_gen_is_eq(__ast, 4)
> +#define IS_AST_GEN5(__ast)	__ast_gen_is_eq(__ast, 5)
> +#define IS_AST_GEN6(__ast)	__ast_gen_is_eq(__ast, 6)
> +#define IS_AST_GEN7(__ast)	__ast_gen_is_eq(__ast, 7)
> +
>   #define AST_IO_AR_PORT_WRITE		(0x40)
>   #define AST_IO_MISC_PORT_WRITE		(0x42)
>   #define AST_IO_VGA_ENABLE_PORT		(0x43)
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index d2f8396ec0a02..8fc412fe296c2 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -127,7 +127,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>   	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
>   	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
>   	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
> -		/* Patch AST2500 */
> +		/* Patch GEN6 */
>   		if (((pdev->revision & 0xF0) == 0x40)
>   			&& ((jregd0 & AST_VRAM_INIT_STATUS_MASK) == 0))
>   			ast_patch_ahb_2500(ast);
> @@ -196,8 +196,8 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
>   	}
>   
>   	/* Check if we support wide screen */
> -	switch (ast->chip) {
> -	case AST2000:
> +	switch (AST_GEN(ast)) {
> +	case 1:
>   		ast->support_wide_screen = false;
>   		break;
>   	default:
> @@ -217,7 +217,7 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
>   			if (ast->chip == AST2500 &&
>   			    scu_rev == 0x100)           /* ast2510 */
>   				ast->support_wide_screen = true;
> -			if (ast->chip == AST2600)		/* ast2600 */
> +			if (IS_AST_GEN7(ast))
>   				ast->support_wide_screen = true;
>   		}
>   		break;
> @@ -240,9 +240,9 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
>   			ast->tx_chip_types = AST_TX_SIL164_BIT;
>   	}
>   
> -	if ((ast->chip == AST2300) || (ast->chip == AST2400) || (ast->chip == AST2500)) {
> +	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast)) {
>   		/*
> -		 * On AST2300 and 2400, look the configuration set by the SoC in
> +		 * On AST GEN4+, look the configuration set by the SoC in
>   		 * the SOC scratch register #1 bits 11:8 (interestingly marked
>   		 * as "reserved" in the spec)
>   		 */
> @@ -264,7 +264,7 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
>   		case 0x0c:
>   			ast->tx_chip_types = AST_TX_DP501_BIT;
>   		}
> -	} else if (ast->chip == AST2600) {
> +	} else if (IS_AST_GEN7(ast)) {
>   		if (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, TX_TYPE_MASK) ==
>   		    ASTDP_DPMCU_TX) {
>   			ast->tx_chip_types = AST_TX_ASTDP_BIT;
> @@ -296,7 +296,7 @@ static int ast_get_dram_info(struct drm_device *dev)
>   	case ast_use_dt:
>   		/*
>   		 * If some properties are missing, use reasonable
> -		 * defaults for AST2400
> +		 * defaults for GEN5
>   		 */
>   		if (of_property_read_u32(np, "aspeed,mcr-configuration",
>   					 &mcr_cfg))
> @@ -319,7 +319,7 @@ static int ast_get_dram_info(struct drm_device *dev)
>   	default:
>   		ast->dram_bus_width = 16;
>   		ast->dram_type = AST_DRAM_1Gx16;
> -		if (ast->chip == AST2500)
> +		if (IS_AST_GEN6(ast))
>   			ast->mclk = 800;
>   		else
>   			ast->mclk = 396;
> @@ -331,7 +331,7 @@ static int ast_get_dram_info(struct drm_device *dev)
>   	else
>   		ast->dram_bus_width = 32;
>   
> -	if (ast->chip == AST2500) {
> +	if (IS_AST_GEN6(ast)) {
>   		switch (mcr_cfg & 0x03) {
>   		case 0:
>   			ast->dram_type = AST_DRAM_1Gx16;
> @@ -347,7 +347,7 @@ static int ast_get_dram_info(struct drm_device *dev)
>   			ast->dram_type = AST_DRAM_8Gx16;
>   			break;
>   		}
> -	} else if (ast->chip == AST2300 || ast->chip == AST2400) {
> +	} else if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast)) {
>   		switch (mcr_cfg & 0x03) {
>   		case 0:
>   			ast->dram_type = AST_DRAM_512Mx16;
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index b3c670af6ef2b..f711d592da52b 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -342,7 +342,7 @@ static void ast_set_crtc_reg(struct ast_device *ast,
>   	u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD = 0, jregAE = 0;
>   	u16 temp, precache = 0;
>   
> -	if ((ast->chip == AST2500 || ast->chip == AST2600) &&
> +	if ((IS_AST_GEN6(ast) || IS_AST_GEN7(ast)) &&
>   	    (vbios_mode->enh_table->flags & AST2500PreCatchCRT))
>   		precache = 40;
>   
> @@ -384,7 +384,7 @@ static void ast_set_crtc_reg(struct ast_device *ast,
>   	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xAD, 0x00, jregAD);
>   
>   	// Workaround for HSync Time non octave pixels (1920x1080@60Hz HSync 44 pixels);
> -	if ((ast->chip == AST2600) && (mode->crtc_vdisplay == 1080))
> +	if (IS_AST_GEN7(ast) && (mode->crtc_vdisplay == 1080))
>   		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xFC, 0xFD, 0x02);
>   	else
>   		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xFC, 0xFD, 0x00);
> @@ -466,7 +466,7 @@ static void ast_set_dclk_reg(struct ast_device *ast,
>   {
>   	const struct ast_vbios_dclk_info *clk_info;
>   
> -	if ((ast->chip == AST2500) || (ast->chip == AST2600))
> +	if (IS_AST_GEN6(ast) || IS_AST_GEN7(ast))
>   		clk_info = &dclk_table_ast2500[vbios_mode->enh_table->dclk_index];
>   	else
>   		clk_info = &dclk_table[vbios_mode->enh_table->dclk_index];
> @@ -510,17 +510,13 @@ static void ast_set_color_reg(struct ast_device *ast,
>   static void ast_set_crtthd_reg(struct ast_device *ast)
>   {
>   	/* Set Threshold */
> -	if (ast->chip == AST2600) {
> +	if (IS_AST_GEN7(ast)) {
>   		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0xe0);
>   		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0xa0);
> -	} else if (ast->chip == AST2300 || ast->chip == AST2400 ||
> -	    ast->chip == AST2500) {
> +	} else if (IS_AST_GEN6(ast) || IS_AST_GEN5(ast) || IS_AST_GEN4(ast)) {
>   		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0x78);
>   		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0x60);
> -	} else if (ast->chip == AST2100 ||
> -		   ast->chip == AST1100 ||
> -		   ast->chip == AST2200 ||
> -		   ast->chip == AST2150) {
> +	} else if (IS_AST_GEN3(ast) || IS_AST_GEN2(ast)) {
>   		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0x3f);
>   		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0x2f);
>   	} else {
> @@ -1082,9 +1078,10 @@ ast_crtc_helper_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode
>   		if ((mode->hdisplay == 1152) && (mode->vdisplay == 864))
>   			return MODE_OK;
>   
> -		if ((ast->chip == AST2100) || (ast->chip == AST2200) ||
> -		    (ast->chip == AST2300) || (ast->chip == AST2400) ||
> -		    (ast->chip == AST2500) || (ast->chip == AST2600)) {
> +		if ((ast->chip == AST2100) || // GEN2, but not AST1100 (?)
> +		    (ast->chip == AST2200) || // GEN3, but not AST2150 (?)
> +		    IS_AST_GEN4(ast) || IS_AST_GEN5(ast) ||
> +		    IS_AST_GEN6(ast) || IS_AST_GEN7(ast)) {
>   			if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080))
>   				return MODE_OK;
>   
> @@ -1800,12 +1797,12 @@ int ast_mode_config_init(struct ast_device *ast)
>   	dev->mode_config.min_height = 0;
>   	dev->mode_config.preferred_depth = 24;
>   
> -	if (ast->chip == AST2100 ||
> -	    ast->chip == AST2200 ||
> -	    ast->chip == AST2300 ||
> -	    ast->chip == AST2400 ||
> -	    ast->chip == AST2500 ||
> -	    ast->chip == AST2600) {
> +	if (ast->chip == AST2100 || // GEN2, but not AST1100 (?)
> +	    ast->chip == AST2200 || // GEN3, but not AST2150 (?)
> +	    IS_AST_GEN7(ast) ||
> +	    IS_AST_GEN6(ast) ||
> +	    IS_AST_GEN5(ast) ||
> +	    IS_AST_GEN4(ast)) {
>   		dev->mode_config.max_width = 1920;
>   		dev->mode_config.max_height = 2048;
>   	} else {
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index b765eeb55e5f1..13e15173f2c5b 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -51,7 +51,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
>   	for (i = 0x81; i <= 0x9f; i++)
>   		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
>   
> -	if (ast->chip == AST2300 || ast->chip == AST2400 || ast->chip == AST2500)
> +	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast))
>   		ext_reg_info = extreginfo_ast2300;
>   	else
>   		ext_reg_info = extreginfo;
> @@ -72,8 +72,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
>   
>   	/* Enable RAMDAC for A1 */
>   	reg = 0x04;
> -	if (ast->chip == AST2300 || ast->chip == AST2400 ||
> -	    ast->chip == AST2500)
> +	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast))
>   		reg |= 0x20;
>   	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0xff, reg);
>   }
> @@ -249,7 +248,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
>   	j = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
>   
>   	if ((j & 0x80) == 0) { /* VGA only */
> -		if (ast->chip == AST2000) {
> +		if (IS_AST_GEN1(ast)) {
>   			dram_reg_info = ast2000_dram_table_data;
>   			ast_write32(ast, 0xf004, 0x1e6e0000);
>   			ast_write32(ast, 0xf000, 0x1);
> @@ -258,7 +257,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
>   			do {
>   				;
>   			} while (ast_read32(ast, 0x10100) != 0xa8);
> -		} else {/* AST2100/1100 */
> +		} else { /* GEN2/GEN3 */
>   			if (ast->chip == AST2100 || ast->chip == AST2200)
>   				dram_reg_info = ast2100_dram_table_data;
>   			else
> @@ -281,7 +280,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
>   			if (dram_reg_info->index == 0xff00) {/* delay fn */
>   				for (i = 0; i < 15; i++)
>   					udelay(dram_reg_info->data);
> -			} else if (dram_reg_info->index == 0x4 && ast->chip != AST2000) {
> +			} else if (dram_reg_info->index == 0x4 && !IS_AST_GEN1(ast)) {
>   				data = dram_reg_info->data;
>   				if (ast->dram_type == AST_DRAM_1Gx16)
>   					data = 0x00000d89;
> @@ -307,15 +306,13 @@ static void ast_init_dram_reg(struct drm_device *dev)
>   				cbrdlli_ast2150(ast, 32); /* 32 bits */
>   		}
>   
> -		switch (ast->chip) {
> -		case AST2000:
> +		switch (AST_GEN(ast)) {
> +		case 1:
>   			temp = ast_read32(ast, 0x10140);
>   			ast_write32(ast, 0x10140, temp | 0x40);
>   			break;
> -		case AST1100:
> -		case AST2100:
> -		case AST2200:
> -		case AST2150:
> +		case 2:
> +		case 3:
>   			temp = ast_read32(ast, 0x1200c);
>   			ast_write32(ast, 0x1200c, temp & 0xfffffffd);
>   			temp = ast_read32(ast, 0x12040);
> @@ -338,13 +335,13 @@ void ast_post_gpu(struct drm_device *dev)
>   
>   	ast_set_def_ext_reg(dev);
>   
> -	if (ast->chip == AST2600) {
> +	if (IS_AST_GEN7(ast)) {
>   		if (ast->tx_chip_types & AST_TX_ASTDP_BIT)
>   			ast_dp_launch(dev);
>   	} else if (ast->config_mode == ast_use_p2a) {
> -		if (ast->chip == AST2500)
> +		if (IS_AST_GEN6(ast))
>   			ast_post_chip_2500(dev);
> -		else if (ast->chip == AST2300 || ast->chip == AST2400)
> +		else if (IS_AST_GEN5(ast) || IS_AST_GEN4(ast))
>   			ast_post_chip_2300(dev);
>   		else
>   			ast_init_dram_reg(dev);

-- 
Jingfeng


^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [v2,14/14] drm/ast: Merge config and chip detection
  2023-06-21 12:53 ` [PATCH v2 14/14] drm/ast: Merge config and chip detection Thomas Zimmermann
@ 2023-06-22 15:21   ` Sui Jingfeng
  0 siblings, 0 replies; 22+ messages in thread
From: Sui Jingfeng @ 2023-06-22 15:21 UTC (permalink / raw)
  To: Thomas Zimmermann, airlied, jfalempe, daniel, jammy_huang; +Cc: dri-devel

Hi,

On 2023/6/21 20:53, Thomas Zimmermann wrote:
> Detection of the configuration mode and the chipset model are
> linked to each other. One uses values from the other; namely the
> PCI device revision and the SCU revision. Merge this code into
> a single function.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
> Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600

Reviewed-by: Sui Jingfeng <suijingfeng@loongson.cn>

> ---
>   drivers/gpu/drm/ast/ast_main.c | 108 +++++++++++++++++----------------
>   1 file changed, 57 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 7ade96f1f37f9..8bfbdfd86d77d 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -96,68 +96,75 @@ static void ast_open_key(struct ast_device *ast)
>   	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x80, 0xA8);
>   }
>   
> -static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
> +static int ast_device_config_init(struct ast_device *ast)
>   {
> -	struct device_node *np = dev->dev->of_node;
> -	struct ast_device *ast = to_ast_device(dev);
> +	struct drm_device *dev = &ast->base;
>   	struct pci_dev *pdev = to_pci_dev(dev->dev);
> -	uint32_t data, jregd0, jregd1;
> +	struct device_node *np = dev->dev->of_node;
> +	uint32_t scu_rev = 0xffffffff;
> +	u32 data;
> +	u8 jregd0, jregd1;
> +
> +	/*
> +	 * Find configuration mode and read SCU revision
> +	 */
>   
> -	/* Defaults */
>   	ast->config_mode = ast_use_defaults;
>   
>   	/* Check if we have device-tree properties */
> -	if (np && !of_property_read_u32(np, "aspeed,scu-revision-id",
> -					scu_rev)) {
> +	if (np && !of_property_read_u32(np, "aspeed,scu-revision-id", &data)) {
>   		/* We do, disable P2A access */
>   		ast->config_mode = ast_use_dt;
> -		drm_info(dev, "Using device-tree for configuration\n");
> -		return;
> -	}
> +		scu_rev = data;
> +	} else if (pdev->device == PCI_CHIP_AST2000) { // Not all families have a P2A bridge
> +		/*
> +		 * The BMC will set SCU 0x40 D[12] to 1 if the P2 bridge
> +		 * is disabled. We force using P2A if VGA only mode bit
> +		 * is set D[7]
> +		 */
> +		jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
> +		jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
> +		if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
> +
> +			/*
> +			 * We have a P2A bridge and it is enabled.
> +			 */
> +
> +			/* Patch AST2500/AST2510 */
> +			if ((pdev->revision & 0xf0) == 0x40) {
> +				if (!(jregd0 & AST_VRAM_INIT_STATUS_MASK))
> +					ast_patch_ahb_2500(ast);
> +			}
>   
> -	/* Not all families have a P2A bridge */
> -	if (pdev->device != PCI_CHIP_AST2000)
> -		return;
> +			/* Double check that it's actually working */
> +			data = ast_read32(ast, 0xf004);
> +			if ((data != 0xffffffff) && (data != 0x00)) {
> +				ast->config_mode = ast_use_p2a;
>   
> -	/*
> -	 * The BMC will set SCU 0x40 D[12] to 1 if the P2 bridge
> -	 * is disabled. We force using P2A if VGA only mode bit
> -	 * is set D[7]
> -	 */
> -	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
> -	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
> -	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
> -		/* Patch GEN6 */
> -		if (((pdev->revision & 0xF0) == 0x40)
> -			&& ((jregd0 & AST_VRAM_INIT_STATUS_MASK) == 0))
> -			ast_patch_ahb_2500(ast);
> -
> -		/* Double check it's actually working */
> -		data = ast_read32(ast, 0xf004);
> -		if ((data != 0xFFFFFFFF) && (data != 0x00)) {
> -			/* P2A works, grab silicon revision */
> -			ast->config_mode = ast_use_p2a;
> -
> -			drm_info(dev, "Using P2A bridge for configuration\n");
> -
> -			/* Read SCU7c (silicon revision register) */
> -			ast_write32(ast, 0xf004, 0x1e6e0000);
> -			ast_write32(ast, 0xf000, 0x1);
> -			*scu_rev = ast_read32(ast, 0x1207c);
> -			return;
> +				/* Read SCU7c (silicon revision register) */
> +				ast_write32(ast, 0xf004, 0x1e6e0000);
> +				ast_write32(ast, 0xf000, 0x1);
> +				scu_rev = ast_read32(ast, 0x1207c);
> +			}
>   		}
>   	}
>   
> -	/* We have a P2A bridge but it's disabled */
> -	drm_info(dev, "P2A bridge disabled, using default configuration\n");
> -}
> +	switch (ast->config_mode) {
> +	case ast_use_defaults:
> +		drm_info(dev, "Using default configuration\n");
> +		break;
> +	case ast_use_dt:
> +		drm_info(dev, "Using device-tree for configuration\n");
> +		break;
> +	case ast_use_p2a:
> +		drm_info(dev, "Using P2A bridge for configuration\n");
> +		break;
> +	}
>   
> -static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
> -{
> -	struct ast_device *ast = to_ast_device(dev);
> -	struct pci_dev *pdev = to_pci_dev(dev->dev);
> +	/*
> +	 * Identify chipset
> +	 */
>   
> -	/* Identify chipset */
>   	if (pdev->revision >= 0x50) {
>   		ast->chip = AST2600;
>   		drm_info(dev, "AST 2600 detected\n");
> @@ -442,7 +449,6 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
>   	struct ast_device *ast;
>   	bool need_post = false;
>   	int ret = 0;
> -	u32 scu_rev = 0xffffffff;
>   
>   	ast = devm_drm_dev_alloc(&pdev->dev, drv, struct ast_device, base);
>   	if (IS_ERR(ast))
> @@ -499,10 +505,10 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
>   	if (ret)
>   		return ERR_PTR(ret);
>   
> -	/* Find out whether P2A works or whether to use device-tree */
> -	ast_detect_config_mode(dev, &scu_rev);
> +	ret = ast_device_config_init(ast);
> +	if (ret)
> +		return ERR_PTR(ret);
>   
> -	ast_detect_chip(dev, need_post, scu_rev);
>   	ast_detect_widescreen(ast);
>   	ast_detect_tx_chip(ast, need_post);
>   

-- 
Jingfeng


^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH v2 06/14] drm/ast: Set PCI config before accessing I/O registers
  2023-06-21 12:53 ` [PATCH v2 06/14] drm/ast: Set PCI config before accessing I/O registers Thomas Zimmermann
  2023-06-21 13:34   ` Sui Jingfeng
@ 2023-06-22 15:42   ` Sui Jingfeng
  2023-06-23  9:02     ` Thomas Zimmermann
  1 sibling, 1 reply; 22+ messages in thread
From: Sui Jingfeng @ 2023-06-22 15:42 UTC (permalink / raw)
  To: Thomas Zimmermann, airlied, jfalempe, daniel, jammy_huang; +Cc: dri-devel

Hi,


I know something about this patch:


On 2023/6/21 20:53, Thomas Zimmermann wrote:
> Access to I/O registers is required to detect and set up the
> device. Enable the rsp PCI config bits before. While at it,
> convert the magic number to macro constants.
>
> Enabling the PCI config bits was done after trying to detect
> the device. It was probably too late at this point.
>
> v2:
> 	* use standard 16-bit PCI r/w access (Jingfeng)
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
> Tested-by: Jocelyn Falempe <jfalempe@redhat.com> # AST2600
> ---
>   drivers/gpu/drm/ast/ast_drv.h  |  1 -
>   drivers/gpu/drm/ast/ast_main.c | 21 +++++++++++++++++++++
>   drivers/gpu/drm/ast/ast_post.c |  6 ------
>   3 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 0141705beaee9..630105feec18a 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -52,7 +52,6 @@
>   #define PCI_CHIP_AST2000 0x2000
>   #define PCI_CHIP_AST2100 0x2010
>   
> -
>   enum ast_chip {
>   	AST2000,
>   	AST2100,
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index c6987e0446618..01f938c2da28f 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -35,6 +35,23 @@
>   
>   #include "ast_drv.h"
>   
> +static int ast_init_pci_config(struct pci_dev *pdev)
> +{
> +	int err;
> +	u16 pcis04;
> +
> +	err = pci_read_config_word(pdev, PCI_COMMAND, &pcis04);
> +	if (err)
> +		goto out;
> +
> +	pcis04 |= PCI_COMMAND_MEMORY | PCI_COMMAND_IO;
> +
> +	err = pci_write_config_word(pdev, PCI_COMMAND, pcis04);
> +
> +out:
> +	return pcibios_err_to_errno(err);
> +}
> +


This function itself is good enough.


>   static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>   {
>   	struct device_node *np = dev->dev->of_node;
> @@ -399,6 +416,10 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
>   			return ERR_PTR(-EIO);
>   	}
>   
> +	ret = ast_init_pci_config(pdev);
> +	if (ret)
> +		return ERR_PTR(ret);
> +


Is the calling to ast_init_pci_config() absolute necessary ?

I'm asking this question because

I think this function is needed to be run only when the chip need POST.

On normal case, when you call pcim_enable_device() function,

the pci_enable_device() function will do such thing for you.


```

int pci_enable_device(struct pci_dev *dev)
{
     return pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO);
}

```

>   	if (!ast_is_vga_enabled(dev)) {
>   		drm_info(dev, "VGA not enabled on entry, requesting chip POST\n");
>   		need_post = true;
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index aa3f2cb00f82c..2da5bdb4bac45 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -361,12 +361,6 @@ static void ast_init_dram_reg(struct drm_device *dev)
>   void ast_post_gpu(struct drm_device *dev)
>   {
>   	struct ast_device *ast = to_ast_device(dev);
> -	struct pci_dev *pdev = to_pci_dev(dev->dev);
> -	u32 reg;
> -
> -	pci_read_config_dword(pdev, 0x04, &reg);
> -	reg |= 0x3;
> -	pci_write_config_dword(pdev, 0x04, reg);

You have changed the semantics after this patch is applied .

Yes, it still works.

But would you like to explain something about this?

We want to learn more knowledge from your patch.

>   
>   	ast_enable_vga(dev);
>   	ast_open_key(ast);

-- 
Jingfeng


^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH v2 06/14] drm/ast: Set PCI config before accessing I/O registers
  2023-06-22 15:42   ` Sui Jingfeng
@ 2023-06-23  9:02     ` Thomas Zimmermann
  2023-06-23  9:20       ` Sui Jingfeng
  0 siblings, 1 reply; 22+ messages in thread
From: Thomas Zimmermann @ 2023-06-23  9:02 UTC (permalink / raw)
  To: Sui Jingfeng, airlied, jfalempe, daniel, jammy_huang; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2332 bytes --]

Hi

Am 22.06.23 um 17:42 schrieb Sui Jingfeng:
[...]
>> +    ret = ast_init_pci_config(pdev);
>> +    if (ret)
>> +        return ERR_PTR(ret);
>> +
> 
> 
> Is the calling to ast_init_pci_config() absolute necessary ?
> 
> I'm asking this question because
> 
> I think this function is needed to be run only when the chip need POST.
> 
> On normal case, when you call pcim_enable_device() function,
> 
> the pci_enable_device() function will do such thing for you.

You're right. We could probably remove this code from the driver. I 
don't want to do this in a patchset that is about refactoring. Maybe in 
a later patch.

> 
> 
> ```
> 
> int pci_enable_device(struct pci_dev *dev)
> {
>      return pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO);
> }
> 
> ```
> 
>>       if (!ast_is_vga_enabled(dev)) {
>>           drm_info(dev, "VGA not enabled on entry, requesting chip 
>> POST\n");
>>           need_post = true;
>> diff --git a/drivers/gpu/drm/ast/ast_post.c 
>> b/drivers/gpu/drm/ast/ast_post.c
>> index aa3f2cb00f82c..2da5bdb4bac45 100644
>> --- a/drivers/gpu/drm/ast/ast_post.c
>> +++ b/drivers/gpu/drm/ast/ast_post.c
>> @@ -361,12 +361,6 @@ static void ast_init_dram_reg(struct drm_device 
>> *dev)
>>   void ast_post_gpu(struct drm_device *dev)
>>   {
>>       struct ast_device *ast = to_ast_device(dev);
>> -    struct pci_dev *pdev = to_pci_dev(dev->dev);
>> -    u32 reg;
>> -
>> -    pci_read_config_dword(pdev, 0x04, &reg);
>> -    reg |= 0x3;
>> -    pci_write_config_dword(pdev, 0x04, reg);
> 
> You have changed the semantics after this patch is applied .
> 
> Yes, it still works.
> 
> But would you like to explain something about this?

I'm looking at the function, but I don't see the change in semantics. I 
only moved the PCI calls and added error detection. What do you mean?

Best regards
Thomas

> 
> We want to learn more knowledge from your patch.
> 
>>       ast_enable_vga(dev);
>>       ast_open_key(ast);
> 

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH v2 06/14] drm/ast: Set PCI config before accessing I/O registers
  2023-06-23  9:02     ` Thomas Zimmermann
@ 2023-06-23  9:20       ` Sui Jingfeng
  0 siblings, 0 replies; 22+ messages in thread
From: Sui Jingfeng @ 2023-06-23  9:20 UTC (permalink / raw)
  To: Thomas Zimmermann, airlied, jfalempe, daniel, jammy_huang; +Cc: dri-devel

Hi,

On 2023/6/23 17:02, Thomas Zimmermann wrote:
> Hi
>
> Am 22.06.23 um 17:42 schrieb Sui Jingfeng:
> [...]
>>> +    ret = ast_init_pci_config(pdev);
>>> +    if (ret)
>>> +        return ERR_PTR(ret);
>>> +
>>
>>
>> Is the calling to ast_init_pci_config() absolute necessary ?
>>
>> I'm asking this question because
>>
>> I think this function is needed to be run only when the chip need POST.
>>
>> On normal case, when you call pcim_enable_device() function,
>>
>> the pci_enable_device() function will do such thing for you.
>
> You're right. We could probably remove this code from the driver. I 
> don't want to do this in a patchset that is about refactoring. Maybe 
> in a later patch.
OK, this sound fine then.
>
>>
>>
>> ```
>>
>> int pci_enable_device(struct pci_dev *dev)
>> {
>>      return pci_enable_device_flags(dev, IORESOURCE_MEM | 
>> IORESOURCE_IO);
>> }
>>
>> ```
>>
>>>       if (!ast_is_vga_enabled(dev)) {
>>>           drm_info(dev, "VGA not enabled on entry, requesting chip 
>>> POST\n");
>>>           need_post = true;
>>> diff --git a/drivers/gpu/drm/ast/ast_post.c 
>>> b/drivers/gpu/drm/ast/ast_post.c
>>> index aa3f2cb00f82c..2da5bdb4bac45 100644
>>> --- a/drivers/gpu/drm/ast/ast_post.c
>>> +++ b/drivers/gpu/drm/ast/ast_post.c
>>> @@ -361,12 +361,6 @@ static void ast_init_dram_reg(struct drm_device 
>>> *dev)
>>>   void ast_post_gpu(struct drm_device *dev)
>>>   {
>>>       struct ast_device *ast = to_ast_device(dev);
>>> -    struct pci_dev *pdev = to_pci_dev(dev->dev);
>>> -    u32 reg;
>>> -
>>> -    pci_read_config_dword(pdev, 0x04, &reg);
>>> -    reg |= 0x3;
>>> -    pci_write_config_dword(pdev, 0x04, reg);
>>
>> You have changed the semantics after this patch is applied .
>>
>> Yes, it still works.
>>
>> But would you like to explain something about this?
>
> I'm looking at the function, but I don't see the change in semantics. 
> I only moved the PCI calls and added error detection. What do you mean?
>
Sorry, I'm using the wrong word. :-)

I means that this function is only needed to be call in ast_post_gpu() 
function.

In fact, on my platform the ast_post_gpu() don't get called even.

I suspect ast_post_gpu() will get called only when the firmware does not 
initial the card.

OK,  You can take another look at this function in the future.

I'm also feel OK about this series.

> Best regards
> Thomas
>
>>
>> We want to learn more knowledge from your patch.
>>
>>>       ast_enable_vga(dev);
>>>       ast_open_key(ast);
>>
>
-- 
Jingfeng


^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH v2 00/14] drm/ast: Refactor the device-detection code
  2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
                   ` (13 preceding siblings ...)
  2023-06-21 12:53 ` [PATCH v2 14/14] drm/ast: Merge config and chip detection Thomas Zimmermann
@ 2023-06-28  6:03 ` Jammy Huang
  14 siblings, 0 replies; 22+ messages in thread
From: Jammy Huang @ 2023-06-28  6:03 UTC (permalink / raw)
  To: Thomas Zimmermann, airlied, jfalempe, daniel, suijingfeng; +Cc: dri-devel

Hi,

LGTM. Thanks

Reviewed-by: Jammy Huang <jammy_huang@aspeedtech.com>

On 2023/6/21 下午 08:53, Thomas Zimmermann wrote:
> Ast's code for detecting the device type and features is convoluted.
> It mixes up several state fields, chip types and sub-models. Rework
> the driver into something more understandable.
>
> Patches 1 fixes a long-standing bug. The affected code has never
> worked correctly.
>
> Patches 2 to 8 make various changes to the init code, or remove dead
> and duplicated code paths.
>
> Patch 9 introduces chip generations. Until now, ast used the value
> of enum ast_chip to represent a certain set of related modes, and
> also used the enum to represent individal models. This makes the
> driver code hard to understand in certain places. The patch encodes
> a chip generation in each model enum and converts the driver to use
> it.
>
> Patches 10 to 12 replace duplicated model checks with the correct
> enum value. Detection of wide-screen functionality and the transmitter
> chip can then be moved into individual functions in patch 13.
>
> Patch 14 merges the detection of the silicon revision and the chip
> model into a single function. Both need to be done in the same place
> and affect each other.
>
> Tested on AST1100 and AST2300.
>
> v2:
> 	* use standard 16-bit PCI access (Jingfeng)
> 	* various cleanups
>
> Thomas Zimmermann (14):
>    drm/ast: Fix DRAM init on AST2200
>    drm/ast: Remove vga2_clone field
>    drm/ast: Implement register helpers in ast_drv.h
>    drm/ast: Remove dead else branch in POST code
>    drm/ast: Remove device POSTing and config from chip detection
>    drm/ast: Set PCI config before accessing I/O registers
>    drm/ast: Enable and unlock device access early during init
>    drm/ast: Set up release action right after enabling MMIO
>    drm/ast: Distinguish among chip generations
>    drm/ast: Detect AST 1300 model
>    drm/ast: Detect AST 1400 model
>    drm/ast: Detect AST 2510 model
>    drm/ast: Move widescreen and tx-chip detection into separate helpers
>    drm/ast: Merge config and chip detection
>
>   drivers/gpu/drm/ast/ast_dp501.c |   6 +-
>   drivers/gpu/drm/ast/ast_drv.h   |  94 +++++++---
>   drivers/gpu/drm/ast/ast_main.c  | 319 +++++++++++++++++++-------------
>   drivers/gpu/drm/ast/ast_mm.c    |   2 -
>   drivers/gpu/drm/ast/ast_mode.c  |  35 ++--
>   drivers/gpu/drm/ast/ast_post.c  |  74 ++------
>   6 files changed, 290 insertions(+), 240 deletions(-)
>
>
> base-commit: 32e260cd0d16cee6f33e747679f168d63ea54bf6
> prerequisite-patch-id: 0aa359f6144c4015c140c8a6750be19099c676fb
> prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
> prerequisite-patch-id: cbc453ee02fae02af22fbfdce56ab732c7a88c36
> prerequisite-patch-id: d3145eae4b35a1290199af6ff6cd5abfebc82033
> prerequisite-patch-id: 242b6bc45675f1f1a62572542d75c89d4864f15a

-- 
Best Regards
Jammy


^ permalink raw reply	[flat|nested] 22+ messages in thread
end of thread, other threads:[~2023-06-28  6:03 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-21 12:53 [PATCH v2 00/14] drm/ast: Refactor the device-detection code Thomas Zimmermann
2023-06-21 12:53 ` [PATCH v2 01/14] drm/ast: Fix DRAM init on AST2200 Thomas Zimmermann
2023-06-21 12:53 ` [PATCH v2 02/14] drm/ast: Remove vga2_clone field Thomas Zimmermann
2023-06-21 12:53 ` [PATCH v2 03/14] drm/ast: Implement register helpers in ast_drv.h Thomas Zimmermann
2023-06-21 12:53 ` [PATCH v2 04/14] drm/ast: Remove dead else branch in POST code Thomas Zimmermann
2023-06-21 12:53 ` [PATCH v2 05/14] drm/ast: Remove device POSTing and config from chip detection Thomas Zimmermann
2023-06-21 12:53 ` [PATCH v2 06/14] drm/ast: Set PCI config before accessing I/O registers Thomas Zimmermann
2023-06-21 13:34   ` Sui Jingfeng
2023-06-22 15:42   ` Sui Jingfeng
2023-06-23  9:02     ` Thomas Zimmermann
2023-06-23  9:20       ` Sui Jingfeng
2023-06-21 12:53 ` [PATCH v2 07/14] drm/ast: Enable and unlock device access early during init Thomas Zimmermann
2023-06-21 12:53 ` [PATCH v2 08/14] drm/ast: Set up release action right after enabling MMIO Thomas Zimmermann
2023-06-21 12:53 ` [PATCH v2 09/14] drm/ast: Distinguish among chip generations Thomas Zimmermann
2023-06-22 15:10   ` [v2,09/14] " Sui Jingfeng
2023-06-21 12:53 ` [PATCH v2 10/14] drm/ast: Detect AST 1300 model Thomas Zimmermann
2023-06-21 12:53 ` [PATCH v2 11/14] drm/ast: Detect AST 1400 model Thomas Zimmermann
2023-06-21 12:53 ` [PATCH v2 12/14] drm/ast: Detect AST 2510 model Thomas Zimmermann
2023-06-21 12:53 ` [PATCH v2 13/14] drm/ast: Move widescreen and tx-chip detection into separate helpers Thomas Zimmermann
2023-06-21 12:53 ` [PATCH v2 14/14] drm/ast: Merge config and chip detection Thomas Zimmermann
2023-06-22 15:21   ` [v2,14/14] " Sui Jingfeng
2023-06-28  6:03 ` [PATCH v2 00/14] drm/ast: Refactor the device-detection code Jammy Huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).