RE: [PATCH v2] drm/ast: Fix soft lockup

[Jammy Huang]

Hi Thomas,

Thank you.

Regards,
Jammy Huang

> 
> Hi,
> 
> I've added a Fixes tag and pushed to patch into drm-misc-fixes.
> 
> Best regards
> Thomas
> 
> Am 03.04.24 um 11:02 schrieb Jammy Huang:
> > There is a while-loop in ast_dp_set_on_off() that could lead to
> > infinite-loop. This is because the register, VGACRI-Dx, checked in
> > this API is a scratch register actually controlled by a MCU, named
> > DPMCU, in BMC.
> >
> > These scratch registers are protected by scu-lock. If suc-lock is not
> > off, DPMCU can not update these registers and then host will have soft
> > lockup due to never updated status.
> >
> > DPMCU is used to control DP and relative registers to handshake with
> > host's VGA driver. Even the most time-consuming task, DP's link
> > training, is less than 100ms. 200ms should be enough.
> >
> > Signed-off-by: Jammy Huang 
> > Reviewed-by: Jocelyn Falempe 
> > ---
> >   drivers/gpu/drm/ast/ast_dp.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/ast/ast_dp.c
> > b/drivers/gpu/drm/ast/ast_dp.c index ebb6d8ebd44e..1e9259416980 100644
> > --- a/drivers/gpu/drm/ast/ast_dp.c
> > +++ b/drivers/gpu/drm/ast/ast_dp.c
> > @@ -180,6 +180,7 @@ void ast_dp_set_on_off(struct drm_device *dev, bool
> on)
> >   {
> > struct ast_device *ast = to_ast_device(dev);
> > u8 video_on_off = on;
> > +   u32 i = 0;
> >
> > // Video On/Off
> > ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE3, (u8)
> > ~AST_DP_VIDEO_ENABLE, on); @@ -192,6 +193,8 @@ void
> ast_dp_set_on_off(struct drm_device *dev, bool on)
> > ASTDP_MIRROR_VIDEO_ENABLE) !=
> video_on_off) {
> > // wait 1 ms
> > mdelay(1);
> > +   if (++i > 200)
> > +   break;
> > }
> > }
> >   }
> >
> > base-commit: b0546776ad3f332e215cebc0b063ba4351971cca
> 
> --
> --
> 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)

[PATCH v2] drm/ast: Fix soft lockup

[Jammy Huang]

There is a while-loop in ast_dp_set_on_off() that could lead to
infinite-loop. This is because the register, VGACRI-Dx, checked in
this API is a scratch register actually controlled by a MCU, named
DPMCU, in BMC.

These scratch registers are protected by scu-lock. If suc-lock is not
off, DPMCU can not update these registers and then host will have soft
lockup due to never updated status.

DPMCU is used to control DP and relative registers to handshake with
host's VGA driver. Even the most time-consuming task, DP's link
training, is less than 100ms. 200ms should be enough.

Signed-off-by: Jammy Huang 
Reviewed-by: Jocelyn Falempe 
---
 drivers/gpu/drm/ast/ast_dp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index ebb6d8ebd44e..1e9259416980 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -180,6 +180,7 @@ void ast_dp_set_on_off(struct drm_device *dev, bool on)
 {
struct ast_device *ast = to_ast_device(dev);
u8 video_on_off = on;
+   u32 i = 0;
 
// Video On/Off
ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE3, (u8) 
~AST_DP_VIDEO_ENABLE, on);
@@ -192,6 +193,8 @@ void ast_dp_set_on_off(struct drm_device *dev, bool on)
ASTDP_MIRROR_VIDEO_ENABLE) != 
video_on_off) {
// wait 1 ms
mdelay(1);
+   if (++i > 200)
+   break;
}
}
 }

base-commit: b0546776ad3f332e215cebc0b063ba4351971cca
-- 
2.25.1

[PATCH] drm/ast: Fix soft lockup

[Jammy Huang]

Avoid infinite-loop in ast_dp_set_on_off().

Signed-off-by: Jammy Huang 
---
 drivers/gpu/drm/ast/ast_dp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index ebb6d8ebd44e..1e9259416980 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -180,6 +180,7 @@ void ast_dp_set_on_off(struct drm_device *dev, bool on)
 {
struct ast_device *ast = to_ast_device(dev);
u8 video_on_off = on;
+   u32 i = 0;
 
// Video On/Off
ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE3, (u8) 
~AST_DP_VIDEO_ENABLE, on);
@@ -192,6 +193,8 @@ void ast_dp_set_on_off(struct drm_device *dev, bool on)
ASTDP_MIRROR_VIDEO_ENABLE) != 
video_on_off) {
// wait 1 ms
mdelay(1);
+   if (++i > 200)
+   break;
}
}
 }

base-commit: b0546776ad3f332e215cebc0b063ba4351971cca
-- 
2.25.1

Re: [PATCH v5 2/2] drm/ast: report connection status on Display Port.

[Jammy Huang]

Hi Jocelyn,

On 2023/7/13 下午 04:17, Jocelyn Falempe wrote:

On 13/07/2023 09:00, Jammy Huang wrote:

Hi Jocelyn,

Thanks for your work.

On 2023/7/13 下午 02:40, Jocelyn Falempe wrote:

Aspeed always report the display port as "connected", because it
doesn't set a .detect_ctx callback.
Fix this by providing the proper detect callback for astdp and dp501.

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

is inconvenient, and breaks the anaconda installer.

v2: Add .detect callback to the dp/dp501 connector (Jani Nikula)
v3: Use .detect_ctx callback, and refactors (Thomas Zimmermann)
 Add a BMC virtual connector
v4: Better indent detect_ctx() functions (Thomas Zimmermann)
v5: Enable polling of the dp and dp501 connector status
 (Thomas Zimmermann)

Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no 
EDID on DP")

Signed-off-by: Jocelyn Falempe 
Reviewed-by: Thomas Zimmermann 
---
  drivers/gpu/drm/ast/ast_dp.c    | 11 ++
  drivers/gpu/drm/ast/ast_dp501.c | 37 
++---

  drivers/gpu/drm/ast/ast_drv.h   |  2 ++
  drivers/gpu/drm/ast/ast_mode.c  | 31 +--
  4 files changed, 67 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp.c 
b/drivers/gpu/drm/ast/ast_dp.c

index 6dc1a09504e1..bf78f3d4aa3f 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -7,6 +7,17 @@
  #include 
  #include "ast_drv.h"
+bool ast_astdp_is_connected(struct ast_device *ast)
+{
+    if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
ASTDP_MCU_FW_EXECUTING))

+    return false;
+    if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDC, 
ASTDP_LINK_SUCCESS))

+    return false;
+    if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDF, 
ASTDP_HPD))

+    return false;
+    return true;
+}


* ASPDP_LINK_SUCCESS is true, when DP link training is finished. The 
DP link quality is good


enough to deliver video data.

* ASTDP_HPD is true, when there is DP sink pull high HPD.

Thus, ASTDP_HPD is the prerequisite of ASTDP_LINK_SUCCESS. I would 
suggest to remove


the check for ASTDP_LINK_SUCCESS here. ASTDP_HPD is good enough for 
connected status.


If you want to check all these three status, please change the order, 
FW_EXECUTING -> HPD ->


LINK_SUCCESS.


Thanks for the detailed explanations.
I looked at other drivers to see if HPD is good enough for "connected" 
status, but I didn't find a clear answer.

There is also a drm_link_status, but that looks to be mostly unused.
https://elixir.bootlin.com/linux/latest/source/include/drm/drm_connector.h#L331 



So I think I will follow your suggestion, and remove the check for 
ASTDP_LINK_SUCCESS.



For the BMC connector patch, you know if there is a register setting I 
can check to see if a BMC is present or not ?


I think you can use VGA CRD0[7] whose definition is as below:

DRAM Initial Selection (see note 1)
0: VBIOS Initial the DRAM
1: SOC Firmware Initial the DRAM

If CRD0[7] is 1, there is BMC.



Best regards,


--
Best Regards
Jammy

Re: [PATCH v5 2/2] drm/ast: report connection status on Display Port.

[Jammy Huang]

Hi Jocelyn,

Thanks for your work.

On 2023/7/13 下午 02:40, Jocelyn Falempe wrote:

Aspeed always report the display port as "connected", because it
doesn't set a .detect_ctx callback.
Fix this by providing the proper detect callback for astdp and dp501.

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

v2: Add .detect callback to the dp/dp501 connector (Jani Nikula)
v3: Use .detect_ctx callback, and refactors (Thomas Zimmermann)
 Add a BMC virtual connector
v4: Better indent detect_ctx() functions (Thomas Zimmermann)
v5: Enable polling of the dp and dp501 connector status
 (Thomas Zimmermann)

Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID on DP")
Signed-off-by: Jocelyn Falempe 
Reviewed-by: Thomas Zimmermann 
---
  drivers/gpu/drm/ast/ast_dp.c| 11 ++
  drivers/gpu/drm/ast/ast_dp501.c | 37 ++---
  drivers/gpu/drm/ast/ast_drv.h   |  2 ++
  drivers/gpu/drm/ast/ast_mode.c  | 31 +--
  4 files changed, 67 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index 6dc1a09504e1..bf78f3d4aa3f 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -7,6 +7,17 @@
  #include 
  #include "ast_drv.h"
  
+bool ast_astdp_is_connected(struct ast_device *ast)

+{
+   if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
ASTDP_MCU_FW_EXECUTING))
+   return false;
+   if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDC, 
ASTDP_LINK_SUCCESS))
+   return false;
+   if (!ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDF, ASTDP_HPD))
+   return false;
+   return true;
+}


* ASPDP_LINK_SUCCESS is true, when DP link training is finished. The DP 
link quality is good


enough to deliver video data.

* ASTDP_HPD is true, when there is DP sink pull high HPD.

Thus, ASTDP_HPD is the prerequisite of ASTDP_LINK_SUCCESS. I would 
suggest to remove


the check for ASTDP_LINK_SUCCESS here. ASTDP_HPD is good enough for 
connected status.


If you want to check all these three status, please change the order, 
FW_EXECUTING -> HPD ->


LINK_SUCCESS.


+
  int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
  {
struct ast_device *ast = to_ast_device(dev);
diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index a5d285850ffb..f10d53b0c94f 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -272,11 +272,9 @@ static bool ast_launch_m68k(struct drm_device *dev)
return true;
  }
  
-bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)

+bool ast_dp501_is_connected(struct ast_device *ast)
  {
-   struct ast_device *ast = to_ast_device(dev);
-   u32 i, boot_address, offset, data;
-   u32 *pEDIDidx;
+   u32 boot_address, offset, data;
  
  	if (ast->config_mode == ast_use_p2a) {

boot_address = get_fw_base(ast);
@@ -292,14 +290,6 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 
*ediddata)
data = ast_mindwm(ast, boot_address + offset);
if (!(data & AST_DP501_PNP_CONNECTED))
return false;
-
-   /* Read EDID */
-   offset = AST_DP501_EDID_DATA;
-   for (i = 0; i < 128; i += 4) {
-   data = ast_mindwm(ast, boot_address + offset + i);
-   pEDIDidx = (u32 *)(ediddata + i);
-   *pEDIDidx = data;
-   }
} else {
if (!ast->dp501_fw_buf)
return false;
@@ -319,7 +309,30 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 
*ediddata)
data = readl(ast->dp501_fw_buf + offset);
if (!(data & AST_DP501_PNP_CONNECTED))
return false;
+   }
+   return true;
+}
+
+bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
+{
+   struct ast_device *ast = to_ast_device(dev);
+   u32 i, boot_address, offset, data;
+   u32 *pEDIDidx;
+
+   if (!ast_dp501_is_connected(ast))
+   return false;
+
+   if (ast->config_mode == ast_use_p2a) {
+   boot_address = get_fw_base(ast);
  
+		/* Read EDID */

+   offset = AST_DP501_EDID_DATA;
+   for (i = 0; i < 128; i += 4) {
+   data = ast_mindwm(ast, boot_address + offset + i);
+   pEDIDidx = (u32 *)(ediddata + i);
+   *pEDIDidx = data;
+   }
+   } else {
/* Read EDID */
offset = 

Re: [PATCH v2 00/14] drm/ast: Refactor the device-detection code

[Jammy Huang]

Hi,

LGTM. Thanks

Reviewed-by: Jammy Huang 

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

[PATCH v3] drm/ast: Fix modeset failed on DisplayPort

[Jammy Huang]

If we switch display and update cursor together, it could lead to
modeset failed because of concurrent access to IO registers.

Add lock protection in DP's edid access to avoid this problem.

Reviewed-by: Thomas Zimmermann 
Signed-off-by: Jammy Huang 
---
 v3 changes:
  - Update label name after mutex added.
 v2 changes:
  - Fix build error since new struct ast_device is used.
---
 drivers/gpu/drm/ast/ast_mode.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 36374828f6c8..b3c670af6ef2 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1647,6 +1647,8 @@ static int ast_dp501_output_init(struct ast_device *ast)
 static int ast_astdp_connector_helper_get_modes(struct drm_connector 
*connector)
 {
void *edid;
+   struct drm_device *dev = connector->dev;
+   struct ast_device *ast = to_ast_device(dev);
 
int succ;
int count;
@@ -1655,9 +1657,17 @@ static int ast_astdp_connector_helper_get_modes(struct 
drm_connector *connector)
if (!edid)
goto err_drm_connector_update_edid_property;
 
+   /*
+* Protect access to I/O registers from concurrent modesetting
+* by acquiring the I/O-register lock.
+*/
+   mutex_lock(>ioregs_lock);
+
succ = ast_astdp_read_edid(connector->dev, edid);
if (succ < 0)
-   goto err_kfree;
+   goto err_mutex_unlock;
+
+   mutex_unlock(>ioregs_lock);
 
drm_connector_update_edid_property(connector, edid);
count = drm_add_edid_modes(connector, edid);
@@ -1665,7 +1675,8 @@ static int ast_astdp_connector_helper_get_modes(struct 
drm_connector *connector)
 
return count;
 
-err_kfree:
+err_mutex_unlock:
+   mutex_unlock(>ioregs_lock);
kfree(edid);
 err_drm_connector_update_edid_property:
drm_connector_update_edid_property(connector, NULL);

base-commit: e338142b39cf40155054f95daa28d210d2ee1b2d
-- 
2.25.1

[PATCH v3] drm/ast: Fix long time waiting on s3/s4 resume

[Jammy Huang]

In resume, DP's launch function, ast_dp_launch, could wait at most 30
seconds before timeout to check if DP is enabled. It could lead to 'DPM
device timeout' and trigger unrecoverable kernel panic.

To avoid this problem, we check if DP enable or not at driver probe only.

Reported-and-tested-by: Wendy Wang 
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217278
Acked-by: Thomas Zimmermann 
Signed-off-by: Jammy Huang 
---
 v3 changes:
  - Update comments
 v2 changes:
  - Fix build error.
---
 drivers/gpu/drm/ast/ast_dp.c   | 55 +++---
 drivers/gpu/drm/ast/ast_drv.h  |  5 +---
 drivers/gpu/drm/ast/ast_main.c | 11 +--
 drivers/gpu/drm/ast/ast_post.c |  3 +-
 4 files changed, 29 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index fbb070f63e36..6dc1a09504e1 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -119,53 +119,32 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 
*ediddata)
 /*
  * Launch Aspeed DP
  */
-void ast_dp_launch(struct drm_device *dev, u8 bPower)
+void ast_dp_launch(struct drm_device *dev)
 {
-   u32 i = 0, j = 0, WaitCount = 1;
-   u8 bDPTX = 0;
+   u32 i = 0;
u8 bDPExecute = 1;
-
struct ast_device *ast = to_ast_device(dev);
-   // S3 come back, need more time to wait BMC ready.
-   if (bPower)
-   WaitCount = 300;
-
-
-   // Wait total count by different condition.
-   for (j = 0; j < WaitCount; j++) {
-   bDPTX = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
TX_TYPE_MASK);
-
-   if (bDPTX)
-   break;
 
+   // Wait one second then timeout.
+   while (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
ASTDP_MCU_FW_EXECUTING) !=
+   ASTDP_MCU_FW_EXECUTING) {
+   i++;
+   // wait 100 ms
msleep(100);
-   }
 
-   // 0xE : ASTDP with DPMCU FW handling
-   if (bDPTX == ASTDP_DPMCU_TX) {
-   // Wait one second then timeout.
-   i = 0;
-
-   while (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
COPROCESSOR_LAUNCH) !=
-   COPROCESSOR_LAUNCH) {
-   i++;
-   // wait 100 ms
-   msleep(100);
-
-   if (i >= 10) {
-   // DP would not be ready.
-   bDPExecute = 0;
-   break;
-   }
+   if (i >= 10) {
+   // DP would not be ready.
+   bDPExecute = 0;
+   break;
}
+   }
 
-   if (bDPExecute)
-   ast->tx_chip_types |= BIT(AST_TX_ASTDP);
+   if (!bDPExecute)
+   drm_err(dev, "Wait DPMCU executing timeout\n");
 
-   ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5,
-   (u8) 
~ASTDP_HOST_EDID_READ_DONE_MASK,
-   
ASTDP_HOST_EDID_READ_DONE);
-   }
+   ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5,
+  (u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
+  ASTDP_HOST_EDID_READ_DONE);
 }
 
 
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index a501169cddad..5498a6676f2e 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -350,9 +350,6 @@ int ast_mode_config_init(struct ast_device *ast);
 #define AST_DP501_LINKRATE 0xf014
 #define AST_DP501_EDID_DATA0xf020
 
-/* Define for Soc scratched reg */
-#define COPROCESSOR_LAUNCH BIT(5)
-
 /*
  * Display Transmitter Type:
  */
@@ -480,7 +477,7 @@ struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
 
 /* aspeed DP */
 int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata);
-void ast_dp_launch(struct drm_device *dev, u8 bPower);
+void ast_dp_launch(struct drm_device *dev);
 void ast_dp_power_on_off(struct drm_device *dev, bool no);
 void ast_dp_set_on_off(struct drm_device *dev, bool no);
 void ast_dp_set_mode(struct drm_crtc *crtc, struct ast_vbios_mode_info 
*vbios_mode);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f32ce29edba7..1f35438f614a 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -254,8 +254,13 @@ static int ast_detect_chip(struct drm_device *dev, bool 
*need_post)
case 0x0c:
ast->tx_chip_types = AST_TX_DP501_BIT;
}
-   } else if (ast->chip == AST2600)
-   ast_dp_launch(>base, 0);
+   } else if (ast->chip == AST2600) {
+   if (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
TX_TYPE_MASK) ==

Re: [PATCH v2] drm/ast: Fix long time waiting on s3/s4 resume

[Jammy Huang]

Hi Thomas,

Thanks for your review.

On 2023/5/24 下午 06:41, Thomas Zimmermann wrote:

Hi,

sorry that this took so long.

Am 24.05.23 um 04:34 schrieb Jammy Huang:

Hi Thomas,

Could you help review this patch?

This is an issue leading to kernel panic found by Intel. Wendy has 
confirmed issue resolved by this patch.


On 2023/4/14 下午 03:42, Jammy Huang wrote:

In resume, DP's launch function, ast_dp_launch, could wait at most 30
seconds before timeout to check if DP is enabled.

To avoid this problem, we only check if DP enable or not at driver 
probe.




You should say what the problem is. Has the DP always been disabled? 
Is the DP only disabled after resume? Or is it a firmware bug?


The problem is that It could lead to 'DPM device timeout' and trigger 
unrecoverable kernel panic. I will describe

the problem more clearly.



If you have the name/email of "wendy.wang", you should probably 
mention her in a Reported-by tag here.



Link: https://bugzilla.kernel.org/show_bug.cgi?id=217278


This should be

Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217278


Signed-off-by: Jammy Huang 

OK, thanks for corrections.


With these changes considered, feel free to add

Acked-by: Thomas Zimmermann 

But I cannot test the patch or even verify the bugfix.

I do have comments below that you might want to consider as well.


---
  v2 changes:
   - Fix build error.
---
  drivers/gpu/drm/ast/ast_dp.c   | 55 
+++---

  drivers/gpu/drm/ast/ast_drv.h  |  2 +-
  drivers/gpu/drm/ast/ast_main.c | 11 +--
  drivers/gpu/drm/ast/ast_post.c |  3 +-
  4 files changed, 29 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp.c 
b/drivers/gpu/drm/ast/ast_dp.c

index 56483860306b..eee2f264c880 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -119,53 +119,32 @@ int ast_astdp_read_edid(struct drm_device 
*dev, u8 *ediddata)

  /*
   * Launch Aspeed DP
   */
-void ast_dp_launch(struct drm_device *dev, u8 bPower)
+void ast_dp_launch(struct drm_device *dev)
  {
-    u32 i = 0, j = 0, WaitCount = 1;
-    u8 bDPTX = 0;
+    u32 i = 0;
  u8 bDPExecute = 1;
-
  struct ast_private *ast = to_ast_private(dev);
-    // S3 come back, need more time to wait BMC ready.
-    if (bPower)
-    WaitCount = 300;
-
-
-    // Wait total count by different condition.
-    for (j = 0; j < WaitCount; j++) {
-    bDPTX = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
TX_TYPE_MASK);

-
-    if (bDPTX)
-    break;
+    // Wait one second then timeout.
+    while (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
COPROCESSOR_LAUNCH) !=

+    COPROCESSOR_LAUNCH) {
+    i++;
+    // wait 100 ms
  msleep(100);
-    }
-    // 0xE : ASTDP with DPMCU FW handling
-    if (bDPTX == ASTDP_DPMCU_TX) {
-    // Wait one second then timeout.
-    i = 0;
-
-    while (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
COPROCESSOR_LAUNCH) !=

-    COPROCESSOR_LAUNCH) {
-    i++;
-    // wait 100 ms
-    msleep(100);
-
-    if (i >= 10) {
-    // DP would not be ready.
-    bDPExecute = 0;
-    break;
-    }
+    if (i >= 10) {
+    // DP would not be ready.
+    bDPExecute = 0;
+    break;
  }
+    }
-    if (bDPExecute)
-    ast->tx_chip_types |= BIT(AST_TX_ASTDP);
+    if (!bDPExecute)
+    drm_err(dev, "Wait DPMCU executing timeout\n");


If waiting fails, should the function return an error? The caller 
could then disable the DP functionality.


The check for COPROCESSOR_LAUNCH is actually MCU running status on BMC. 
For some cases, the execution
of MCU on BMC could be halted, which leads to failure on check for this 
status. But DP is workable after BMC
release the execution of MCU. Thus, we do not want to disable DP 
functionality here.



-    ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5,
-    (u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
-    ASTDP_HOST_EDID_READ_DONE);
-    }
+    ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5,
+   (u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
+   ASTDP_HOST_EDID_READ_DONE);
  }
diff --git a/drivers/gpu/drm/ast/ast_drv.h 
b/drivers/gpu/drm/ast/ast_drv.h

index d51b81fea9c8..15e86394be4f 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -498,7 +498,7 @@ struct ast_i2c_chan *ast_i2c_create(struct 
drm_device *dev);

  /* aspeed DP */
  int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata);
-void ast_dp_launch(struct drm_device *dev, u8 bPower);
+void ast_dp_launch(struct drm_device *dev);
  void ast_dp_power_on_off(struct drm_device *dev, bool no);
  void ast_dp_set_on_off(struct drm_device *dev, bool no);
  void ast_dp_set_mode(struct drm_crtc *crtc, struct 
ast_vbios_mode_info *vbios_mod

Re: [PATCH] drm/ast: Fix modeset failed on DisplayPort

[Jammy Huang]

Hi Thomas,

Thanks, I will modify the patch accordingly.

On 2023/5/24 下午 06:47, Thomas Zimmermann wrote:

Hi,

this patch also fell through the cracks. Apologies.

Am 24.05.23 um 04:29 schrieb Jammy Huang:

Hi Thomas,

Do you have other suggestion for this patch??


The main issue was that struct ast_private is now called struct 
ast_device.  So the current patch cannot be applied.




Please kindly advise.

On 2023/4/25 下午 03:39, Jammy Huang wrote:

Hi Thomas,

I think DP501 is OK. It doesn't use ioregs in ast_dp501_read_edid().

On 2023/4/25 下午 03:27, Thomas Zimmermann wrote:

Hi

Am 25.04.23 um 09:03 schrieb Jammy Huang:

If we switch display and update cursor together, it could lead to
modeset failed because of concurrent access to IO registers.

Add lock protection in DP's edid access to avoid this problem.


Thanks for the patch. I thought I fixed this issue already, but 
that apparently only happened for SIL164 and VGA.


What about ast_dp501_connector_helper_get_modes()? Does it require 
the locking as well?




Signed-off-by: Jammy Huang 
---
  drivers/gpu/drm/ast/ast_mode.c | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/ast/ast_mode.c 
b/drivers/gpu/drm/ast/ast_mode.c

index 984ec590a7e7..fe5f1fd61361 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1635,6 +1635,8 @@ static int ast_dp501_output_init(struct 
ast_private *ast)
  static int ast_astdp_connector_helper_get_modes(struct 
drm_connector *connector)

  {
  void *edid;
+    struct drm_device *dev = connector->dev;
+    struct ast_private *ast = to_ast_private(dev);


We've meanwhile renamed ast_private to ast_device. Could you please 
provide an updated patch for the drm-misc-next tree?


Best regards
Thomas


    int succ;
  int count;
@@ -1643,10 +1645,18 @@ static int 
ast_astdp_connector_helper_get_modes(struct drm_connector *connector)

  if (!edid)
  goto err_drm_connector_update_edid_property;
  +    /*
+ * Protect access to I/O registers from concurrent modesetting
+ * by acquiring the I/O-register lock.
+ */
+    mutex_lock(>ioregs_lock);
+
  succ = ast_astdp_read_edid(connector->dev, edid);
  if (succ < 0)
  goto err_kfree;
  +    mutex_unlock(>ioregs_lock);
+
  drm_connector_update_edid_property(connector, edid);
  count = drm_add_edid_modes(connector, edid);
  kfree(edid);
@@ -1654,6 +1664,7 @@ static int 
ast_astdp_connector_helper_get_modes(struct drm_connector *connector)

  return count;
    err_kfree:


Here's a minor issue that the goto label should now be called 
err_mutex_unlock.


Best regards
Thomas


+ mutex_unlock(>ioregs_lock);
  kfree(edid);
  err_drm_connector_update_edid_property:
  drm_connector_update_edid_property(connector, NULL);

base-commit: 61d325dcbc05d8fef88110d35ef7776f3ac3f68b





--
Best Regards
Jammy

[PATCH v2] drm/ast: Fix modeset failed on DisplayPort

[Jammy Huang]

If we switch display and update cursor together, it could lead to
modeset failed because of concurrent access to IO registers.

Add lock protection in DP's edid access to avoid this problem.

Signed-off-by: Jammy Huang 
---
 v2 changes:
  - Fix build error since new struct ast_device is used.
---
 drivers/gpu/drm/ast/ast_mode.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 36374828f6c8..9fcbf540d6fc 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1647,6 +1647,8 @@ static int ast_dp501_output_init(struct ast_device *ast)
 static int ast_astdp_connector_helper_get_modes(struct drm_connector 
*connector)
 {
void *edid;
+   struct drm_device *dev = connector->dev;
+   struct ast_device *ast = to_ast_device(dev);
 
int succ;
int count;
@@ -1655,10 +1657,18 @@ static int ast_astdp_connector_helper_get_modes(struct 
drm_connector *connector)
if (!edid)
goto err_drm_connector_update_edid_property;
 
+   /*
+* Protect access to I/O registers from concurrent modesetting
+* by acquiring the I/O-register lock.
+*/
+   mutex_lock(>ioregs_lock);
+
succ = ast_astdp_read_edid(connector->dev, edid);
if (succ < 0)
goto err_kfree;
 
+   mutex_unlock(>ioregs_lock);
+
drm_connector_update_edid_property(connector, edid);
count = drm_add_edid_modes(connector, edid);
kfree(edid);
@@ -1666,6 +1676,7 @@ static int ast_astdp_connector_helper_get_modes(struct 
drm_connector *connector)
return count;
 
 err_kfree:
+   mutex_unlock(>ioregs_lock);
kfree(edid);
 err_drm_connector_update_edid_property:
drm_connector_update_edid_property(connector, NULL);

base-commit: 933174ae28ba72ab8de5b35cb7c98fc211235096
-- 
2.25.1

Re: [PATCH v2] drm/ast: Fix long time waiting on s3/s4 resume

[Jammy Huang]

Hi Thomas,

Could you help review this patch?

This is an issue leading to kernel panic found by Intel. Wendy has 
confirmed issue resolved by this patch.


On 2023/4/14 下午 03:42, Jammy Huang wrote:

In resume, DP's launch function, ast_dp_launch, could wait at most 30
seconds before timeout to check if DP is enabled.

To avoid this problem, we only check if DP enable or not at driver probe.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217278
Signed-off-by: Jammy Huang 
---
  v2 changes:
   - Fix build error.
---
  drivers/gpu/drm/ast/ast_dp.c   | 55 +++---
  drivers/gpu/drm/ast/ast_drv.h  |  2 +-
  drivers/gpu/drm/ast/ast_main.c | 11 +--
  drivers/gpu/drm/ast/ast_post.c |  3 +-
  4 files changed, 29 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index 56483860306b..eee2f264c880 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -119,53 +119,32 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 
*ediddata)
  /*
   * Launch Aspeed DP
   */
-void ast_dp_launch(struct drm_device *dev, u8 bPower)
+void ast_dp_launch(struct drm_device *dev)
  {
-   u32 i = 0, j = 0, WaitCount = 1;
-   u8 bDPTX = 0;
+   u32 i = 0;
u8 bDPExecute = 1;
-
struct ast_private *ast = to_ast_private(dev);
-   // S3 come back, need more time to wait BMC ready.
-   if (bPower)
-   WaitCount = 300;
-
-
-   // Wait total count by different condition.
-   for (j = 0; j < WaitCount; j++) {
-   bDPTX = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
TX_TYPE_MASK);
-
-   if (bDPTX)
-   break;
  
+	// Wait one second then timeout.

+   while (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
COPROCESSOR_LAUNCH) !=
+   COPROCESSOR_LAUNCH) {
+   i++;
+   // wait 100 ms
msleep(100);
-   }
  
-	// 0xE : ASTDP with DPMCU FW handling

-   if (bDPTX == ASTDP_DPMCU_TX) {
-   // Wait one second then timeout.
-   i = 0;
-
-   while (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
COPROCESSOR_LAUNCH) !=
-   COPROCESSOR_LAUNCH) {
-   i++;
-   // wait 100 ms
-   msleep(100);
-
-   if (i >= 10) {
-   // DP would not be ready.
-   bDPExecute = 0;
-   break;
-   }
+   if (i >= 10) {
+   // DP would not be ready.
+   bDPExecute = 0;
+   break;
}
+   }
  
-		if (bDPExecute)

-   ast->tx_chip_types |= BIT(AST_TX_ASTDP);
+   if (!bDPExecute)
+   drm_err(dev, "Wait DPMCU executing timeout\n");
  
-		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5,

-   (u8) 
~ASTDP_HOST_EDID_READ_DONE_MASK,
-   
ASTDP_HOST_EDID_READ_DONE);
-   }
+   ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5,
+  (u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
+  ASTDP_HOST_EDID_READ_DONE);
  }
  
  
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h

index d51b81fea9c8..15e86394be4f 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -498,7 +498,7 @@ struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
  
  /* aspeed DP */

  int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata);
-void ast_dp_launch(struct drm_device *dev, u8 bPower);
+void ast_dp_launch(struct drm_device *dev);
  void ast_dp_power_on_off(struct drm_device *dev, bool no);
  void ast_dp_set_on_off(struct drm_device *dev, bool no);
  void ast_dp_set_mode(struct drm_crtc *crtc, struct ast_vbios_mode_info 
*vbios_mode);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f83ce77127cb..8ecddf20113f 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -254,8 +254,13 @@ static int ast_detect_chip(struct drm_device *dev, bool 
*need_post)
case 0x0c:
ast->tx_chip_types = AST_TX_DP501_BIT;
}
-   } else if (ast->chip == AST2600)
-   ast_dp_launch(>base, 0);
+   } else if (ast->chip == AST2600) {
+   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;
+   ast_dp_launch(>base);
+   }
+   }
  
  	/* Print stuff for diagnostic purposes */

if (ast->tx_chip_types & AST_TX_NONE_BIT)
@@ -26

Re: [PATCH] drm/ast: Fix modeset failed on DisplayPort

[Jammy Huang]

Hi Thomas,

Do you have other suggestion for this patch??

Please kindly advise.

On 2023/4/25 下午 03:39, Jammy Huang wrote:

Hi Thomas,

I think DP501 is OK. It doesn't use ioregs in ast_dp501_read_edid().

On 2023/4/25 下午 03:27, Thomas Zimmermann wrote:

Hi

Am 25.04.23 um 09:03 schrieb Jammy Huang:

If we switch display and update cursor together, it could lead to
modeset failed because of concurrent access to IO registers.

Add lock protection in DP's edid access to avoid this problem.


Thanks for the patch. I thought I fixed this issue already, but that 
apparently only happened for SIL164 and VGA.


What about ast_dp501_connector_helper_get_modes()? Does it require 
the locking as well?




Signed-off-by: Jammy Huang 
---
  drivers/gpu/drm/ast/ast_mode.c | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/ast/ast_mode.c 
b/drivers/gpu/drm/ast/ast_mode.c

index 984ec590a7e7..fe5f1fd61361 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1635,6 +1635,8 @@ static int ast_dp501_output_init(struct 
ast_private *ast)
  static int ast_astdp_connector_helper_get_modes(struct 
drm_connector *connector)

  {
  void *edid;
+    struct drm_device *dev = connector->dev;
+    struct ast_private *ast = to_ast_private(dev);


We've meanwhile renamed ast_private to ast_device. Could you please 
provide an updated patch for the drm-misc-next tree?


Best regards
Thomas


    int succ;
  int count;
@@ -1643,10 +1645,18 @@ static int 
ast_astdp_connector_helper_get_modes(struct drm_connector *connector)

  if (!edid)
  goto err_drm_connector_update_edid_property;
  +    /*
+ * Protect access to I/O registers from concurrent modesetting
+ * by acquiring the I/O-register lock.
+ */
+    mutex_lock(>ioregs_lock);
+
  succ = ast_astdp_read_edid(connector->dev, edid);
  if (succ < 0)
  goto err_kfree;
  +    mutex_unlock(>ioregs_lock);
+
  drm_connector_update_edid_property(connector, edid);
  count = drm_add_edid_modes(connector, edid);
  kfree(edid);
@@ -1654,6 +1664,7 @@ static int 
ast_astdp_connector_helper_get_modes(struct drm_connector *connector)

  return count;
    err_kfree:
+    mutex_unlock(>ioregs_lock);
  kfree(edid);
  err_drm_connector_update_edid_property:
  drm_connector_update_edid_property(connector, NULL);

base-commit: 61d325dcbc05d8fef88110d35ef7776f3ac3f68b



--
Best Regards
Jammy

Re: [PATCH] drm/ast: Fix modeset failed on DisplayPort

[Jammy Huang]

Hi Thomas,

I think DP501 is OK. It doesn't use ioregs in ast_dp501_read_edid().

On 2023/4/25 下午 03:27, Thomas Zimmermann wrote:

Hi

Am 25.04.23 um 09:03 schrieb Jammy Huang:

If we switch display and update cursor together, it could lead to
modeset failed because of concurrent access to IO registers.

Add lock protection in DP's edid access to avoid this problem.


Thanks for the patch. I thought I fixed this issue already, but that 
apparently only happened for SIL164 and VGA.


What about ast_dp501_connector_helper_get_modes()? Does it require the 
locking as well?




Signed-off-by: Jammy Huang 
---
  drivers/gpu/drm/ast/ast_mode.c | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/ast/ast_mode.c 
b/drivers/gpu/drm/ast/ast_mode.c

index 984ec590a7e7..fe5f1fd61361 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1635,6 +1635,8 @@ static int ast_dp501_output_init(struct 
ast_private *ast)
  static int ast_astdp_connector_helper_get_modes(struct 
drm_connector *connector)

  {
  void *edid;
+    struct drm_device *dev = connector->dev;
+    struct ast_private *ast = to_ast_private(dev);


We've meanwhile renamed ast_private to ast_device. Could you please 
provide an updated patch for the drm-misc-next tree?


Best regards
Thomas


    int succ;
  int count;
@@ -1643,10 +1645,18 @@ static int 
ast_astdp_connector_helper_get_modes(struct drm_connector *connector)

  if (!edid)
  goto err_drm_connector_update_edid_property;
  +    /*
+ * Protect access to I/O registers from concurrent modesetting
+ * by acquiring the I/O-register lock.
+ */
+    mutex_lock(>ioregs_lock);
+
  succ = ast_astdp_read_edid(connector->dev, edid);
  if (succ < 0)
  goto err_kfree;
  +    mutex_unlock(>ioregs_lock);
+
  drm_connector_update_edid_property(connector, edid);
  count = drm_add_edid_modes(connector, edid);
  kfree(edid);
@@ -1654,6 +1664,7 @@ static int 
ast_astdp_connector_helper_get_modes(struct drm_connector *connector)

  return count;
    err_kfree:
+    mutex_unlock(>ioregs_lock);
  kfree(edid);
  err_drm_connector_update_edid_property:
  drm_connector_update_edid_property(connector, NULL);

base-commit: 61d325dcbc05d8fef88110d35ef7776f3ac3f68b



--
Best Regards
Jammy

[PATCH] drm/ast: Fix modeset failed on DisplayPort

[Jammy Huang]

If we switch display and update cursor together, it could lead to
modeset failed because of concurrent access to IO registers.

Add lock protection in DP's edid access to avoid this problem.

Signed-off-by: Jammy Huang 
---
 drivers/gpu/drm/ast/ast_mode.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 984ec590a7e7..fe5f1fd61361 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1635,6 +1635,8 @@ static int ast_dp501_output_init(struct ast_private *ast)
 static int ast_astdp_connector_helper_get_modes(struct drm_connector 
*connector)
 {
void *edid;
+   struct drm_device *dev = connector->dev;
+   struct ast_private *ast = to_ast_private(dev);
 
int succ;
int count;
@@ -1643,10 +1645,18 @@ static int ast_astdp_connector_helper_get_modes(struct 
drm_connector *connector)
if (!edid)
goto err_drm_connector_update_edid_property;
 
+   /*
+* Protect access to I/O registers from concurrent modesetting
+* by acquiring the I/O-register lock.
+*/
+   mutex_lock(>ioregs_lock);
+
succ = ast_astdp_read_edid(connector->dev, edid);
if (succ < 0)
goto err_kfree;
 
+   mutex_unlock(>ioregs_lock);
+
drm_connector_update_edid_property(connector, edid);
count = drm_add_edid_modes(connector, edid);
kfree(edid);
@@ -1654,6 +1664,7 @@ static int ast_astdp_connector_helper_get_modes(struct 
drm_connector *connector)
return count;
 
 err_kfree:
+   mutex_unlock(>ioregs_lock);
kfree(edid);
 err_drm_connector_update_edid_property:
drm_connector_update_edid_property(connector, NULL);

base-commit: 61d325dcbc05d8fef88110d35ef7776f3ac3f68b
-- 
2.25.1

[PATCH v3] drm/ast: Fix ARM compatibility

[Jammy Huang]

ARM architecture only has 'memory', so all devices are accessed by
MMIO if possible.

Signed-off-by: Jammy Huang 
Reviewed-by: Thomas Zimmermann 
---
 v2 changes:
  - Use MMIO after AST2500 which enable MMIO by default.
 v3 changes:
  - Correct comments
---
 drivers/gpu/drm/ast/ast_main.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f83ce77127cb..a6d0ee4da2b8 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -425,11 +425,12 @@ struct ast_private *ast_device_create(const struct 
drm_driver *drv,
return ERR_PTR(-EIO);
 
/*
-* If we don't have IO space at all, use MMIO now and
-* assume the chip has MMIO enabled by default (rev 0x20
-* and higher).
+* After AST2500, MMIO is enabled by default, and it should be adopted
+* to be compatible with Arm.
 */
-   if (!(pci_resource_flags(pdev, 2) & IORESOURCE_IO)) {
+   if (pdev->revision >= 0x40) {
+   ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
+   } else if (!(pci_resource_flags(pdev, 2) & IORESOURCE_IO)) {
drm_info(dev, "platform has no IO space, trying MMIO\n");
ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
}

base-commit: e62252bc55b6d4eddc6c2bdbf95a448180d6a08d
-- 
2.25.1

Re: [PATCH v2] drm/ast: Fix ARM compatibility

[Jammy Huang]

Hi Thomas,

Thanks for help. I will update a new patch per your comments

On 2023/4/20 下午 04:57, Thomas Zimmermann wrote:

Hi

Am 20.04.23 um 10:09 schrieb Jammy Huang:

ARM architecture only has 'memory', so all devices are accessed by
MMIO if possible.

Signed-off-by: Jammy Huang 
---
  v2 changes:
   - Use MMIO after AST2500 which enable MMIO by default.
---
  drivers/gpu/drm/ast/ast_main.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c 
b/drivers/gpu/drm/ast/ast_main.c

index f83ce77127cb..d384e810fa4d 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -425,11 +425,12 @@ struct ast_private *ast_device_create(const 
struct drm_driver *drv,

  return ERR_PTR(-EIO);
    /*
- * If we don't have IO space at all, use MMIO now and
- * assume the chip has MMIO enabled by default (rev 0x20
- * and higher).
+ * After AST2500, MMIO is enabled by default, and it should be 
adapted


By 'adapted', you mean 'adopted'?

Apart from that:

Reviewed-by: Thomas Zimmermann 

I also ran this patch on an AST2100, which still works as before.

Best regards
Thomas


+ * to be compatible with Arm.
   */
-    if (!(pci_resource_flags(pdev, 2) & IORESOURCE_IO)) {
+    if (pdev->revision >= 0x40) {
+    ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
+    } else if (!(pci_resource_flags(pdev, 2) & IORESOURCE_IO)) {
  drm_info(dev, "platform has no IO space, trying MMIO\n");
  ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
  }

base-commit: e62252bc55b6d4eddc6c2bdbf95a448180d6a08d



--
Best Regards
Jammy

[PATCH v2] drm/ast: Fix ARM compatibility

[Jammy Huang]

ARM architecture only has 'memory', so all devices are accessed by
MMIO if possible.

Signed-off-by: Jammy Huang 
---
 v2 changes:
  - Use MMIO after AST2500 which enable MMIO by default.
---
 drivers/gpu/drm/ast/ast_main.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f83ce77127cb..d384e810fa4d 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -425,11 +425,12 @@ struct ast_private *ast_device_create(const struct 
drm_driver *drv,
return ERR_PTR(-EIO);
 
/*
-* If we don't have IO space at all, use MMIO now and
-* assume the chip has MMIO enabled by default (rev 0x20
-* and higher).
+* After AST2500, MMIO is enabled by default, and it should be adapted
+* to be compatible with Arm.
 */
-   if (!(pci_resource_flags(pdev, 2) & IORESOURCE_IO)) {
+   if (pdev->revision >= 0x40) {
+   ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
+   } else if (!(pci_resource_flags(pdev, 2) & IORESOURCE_IO)) {
drm_info(dev, "platform has no IO space, trying MMIO\n");
ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
}

base-commit: e62252bc55b6d4eddc6c2bdbf95a448180d6a08d
-- 
2.25.1

Re: [PATCH] drm/ast: Fix ARM compatibility

[Jammy Huang]

Hi Thomas,

Thanks for you reminder. The comment you mentioned is added in 2014 for 
AST2400 rev 0x20, which means MMIO is not enable by default before that 
revision. I will send another patch to handle it.



On 2023/4/18 下午 03:24, Thomas Zimmermann wrote:

Hi

Am 18.04.23 um 03:23 schrieb Jammy Huang:

Hi Thomas,

The Intel(x86) CPUs have a separate address space for "IO", but the 
ARM architecture only has "memory", so all IO devices are accessed as 
if they were memory. Which means ARM does not support isolated IO. 
Here is a related discussion on ARM's forum.


https://community.arm.com/support-forums/f/architectures-and-processors-forum/52046/how-to-read-write-an-i-o-port-in-aarch64 



Thus, we want to adapt MMIO only after this patch.


What I mean is that there's a comment that says "assume the chip has 
MMIO enabled by default (rev 0x20 and higher)". We also support revs 
before 0x20. What happens to them?


Best regards
Thomas



On 2023/4/17 下午 07:51, Thomas Zimmermann wrote:

Hi

Am 07.04.23 um 04:09 schrieb Jammy Huang:

Hi Thomas,

Could you help review this patch??

We met some problem on nvidia's ARM platfrom and need this patch to 
fix it.


On 2023/3/2 上午 10:19, Jammy Huang wrote:
ARM architecture only has 'memory', so all devices are accessed by 
MMIO.


Signed-off-by: Jammy Huang 
---
  drivers/gpu/drm/ast/ast_main.c | 17 +
  1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c 
b/drivers/gpu/drm/ast/ast_main.c

index 794ffd4a29c5..f86d01e9f024 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -424,22 +424,7 @@ struct ast_device *ast_device_create(const 
struct drm_driver *drv,

  if (!ast->regs)
  return ERR_PTR(-EIO);
-    /*
- * If we don't have IO space at all, use MMIO now and
- * assume the chip has MMIO enabled by default (rev 0x20
- * and higher).
- */
-    if (!(pci_resource_flags(pdev, 2) & IORESOURCE_IO)) {
-    drm_info(dev, "platform has no IO space, trying MMIO\n");
-    ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
-    }
-
-    /* "map" IO regs if the above hasn't done so already */
-    if (!ast->ioregs) {
-    ast->ioregs = pcim_iomap(pdev, 2, 0);


What happens on systems that use this branch?

Best regards
Thomas


-    if (!ast->ioregs)
-    return ERR_PTR(-EIO);
-    }
+    ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
  ast_detect_chip(dev, _post);

base-commit: 254986e324add8a30d0019c6da59f81adc8b565f





--
Best Regards
Jammy




--
Best Regards
Jammy

Re: [PATCH] drm/ast: Fix ARM compatibility

[Jammy Huang]

Hi Thomas,

The Intel(x86) CPUs have a separate address space for "IO", but the ARM 
architecture only has "memory", so all IO devices are accessed as if 
they were memory. Which means ARM does not support isolated IO. Here is 
a related discussion on ARM's forum.


https://community.arm.com/support-forums/f/architectures-and-processors-forum/52046/how-to-read-write-an-i-o-port-in-aarch64

Thus, we want to adapt MMIO only after this patch.

On 2023/4/17 下午 07:51, Thomas Zimmermann wrote:

Hi

Am 07.04.23 um 04:09 schrieb Jammy Huang:

Hi Thomas,

Could you help review this patch??

We met some problem on nvidia's ARM platfrom and need this patch to 
fix it.


On 2023/3/2 上午 10:19, Jammy Huang wrote:
ARM architecture only has 'memory', so all devices are accessed by 
MMIO.


Signed-off-by: Jammy Huang 
---
  drivers/gpu/drm/ast/ast_main.c | 17 +
  1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c 
b/drivers/gpu/drm/ast/ast_main.c

index 794ffd4a29c5..f86d01e9f024 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -424,22 +424,7 @@ struct ast_device *ast_device_create(const 
struct drm_driver *drv,

  if (!ast->regs)
  return ERR_PTR(-EIO);
-    /*
- * If we don't have IO space at all, use MMIO now and
- * assume the chip has MMIO enabled by default (rev 0x20
- * and higher).
- */
-    if (!(pci_resource_flags(pdev, 2) & IORESOURCE_IO)) {
-    drm_info(dev, "platform has no IO space, trying MMIO\n");
-    ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
-    }
-
-    /* "map" IO regs if the above hasn't done so already */
-    if (!ast->ioregs) {
-    ast->ioregs = pcim_iomap(pdev, 2, 0);


What happens on systems that use this branch?

Best regards
Thomas


-    if (!ast->ioregs)
-    return ERR_PTR(-EIO);
-    }
+    ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
  ast_detect_chip(dev, _post);

base-commit: 254986e324add8a30d0019c6da59f81adc8b565f





--
Best Regards
Jammy

Re: [PATCH 01/11] drm/ast: Use drm_aperture_remove_conflicting_pci_framebuffers

[Jammy Huang]

Hi Thomas,

The Intel(x86) CPUs have a separate address space for "IO", but the ARM 
architecture only has "memory", so all IO devices are accessed as if 
they were memory. Which means ARM does not support isolated IO. Here is 
a related discussion on ARM's forum.


https://community.arm.com/support-forums/f/architectures-and-processors-forum/52046/how-to-read-write-an-i-o-port-in-aarch64

Thus, we adapt MMIO only after this patch.


On 2023/4/4 下午 10:45, Thomas Zimmermann wrote:

Hi,

FYI I have merged patches 1, 6 and 7 of this patchset. They look fine 
and are worthwhile fixes on their own.


Best regards
Thomas

Am 11.01.23 um 16:41 schrieb Daniel Vetter:

It's just open coded and matches.

Note that Thomas said that his version apparently failed for some
reason, but hey maybe we should try again.

Signed-off-by: Daniel Vetter 
Cc: Dave Airlie 
Cc: Thomas Zimmermann 
Cc: Javier Martinez Canillas 
Cc: Helge Deller 
Cc: linux-fb...@vger.kernel.org
---
  drivers/gpu/drm/ast/ast_drv.c | 16 +---
  1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.c 
b/drivers/gpu/drm/ast/ast_drv.c

index 420fc75c240e..3ac24a780f50 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -90,27 +90,13 @@ static const struct pci_device_id ast_pciidlist[] 
= {

    MODULE_DEVICE_TABLE(pci, ast_pciidlist);
  -static int ast_remove_conflicting_framebuffers(struct pci_dev *pdev)
-{
-    bool primary = false;
-    resource_size_t base, size;
-
-    base = pci_resource_start(pdev, 0);
-    size = pci_resource_len(pdev, 0);
-#ifdef CONFIG_X86
-    primary = pdev->resource[PCI_ROM_RESOURCE].flags & 
IORESOURCE_ROM_SHADOW;

-#endif
-
-    return drm_aperture_remove_conflicting_framebuffers(base, size, 
primary, _driver);

-}
-
  static int ast_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)

  {
  struct ast_private *ast;
  struct drm_device *dev;
  int ret;
  -    ret = ast_remove_conflicting_framebuffers(pdev);
+    ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, 
_driver);

  if (ret)
  return ret;



--
Best Regards
Jammy

[PATCH v2] drm/ast: Fix long time waiting on s3/s4 resume

[Jammy Huang]

In resume, DP's launch function, ast_dp_launch, could wait at most 30
seconds before timeout to check if DP is enabled.

To avoid this problem, we only check if DP enable or not at driver probe.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217278
Signed-off-by: Jammy Huang 
---
 v2 changes:
  - Fix build error.
---
 drivers/gpu/drm/ast/ast_dp.c   | 55 +++---
 drivers/gpu/drm/ast/ast_drv.h  |  2 +-
 drivers/gpu/drm/ast/ast_main.c | 11 +--
 drivers/gpu/drm/ast/ast_post.c |  3 +-
 4 files changed, 29 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index 56483860306b..eee2f264c880 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -119,53 +119,32 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 
*ediddata)
 /*
  * Launch Aspeed DP
  */
-void ast_dp_launch(struct drm_device *dev, u8 bPower)
+void ast_dp_launch(struct drm_device *dev)
 {
-   u32 i = 0, j = 0, WaitCount = 1;
-   u8 bDPTX = 0;
+   u32 i = 0;
u8 bDPExecute = 1;
-
struct ast_private *ast = to_ast_private(dev);
-   // S3 come back, need more time to wait BMC ready.
-   if (bPower)
-   WaitCount = 300;
-
-
-   // Wait total count by different condition.
-   for (j = 0; j < WaitCount; j++) {
-   bDPTX = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
TX_TYPE_MASK);
-
-   if (bDPTX)
-   break;
 
+   // Wait one second then timeout.
+   while (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
COPROCESSOR_LAUNCH) !=
+   COPROCESSOR_LAUNCH) {
+   i++;
+   // wait 100 ms
msleep(100);
-   }
 
-   // 0xE : ASTDP with DPMCU FW handling
-   if (bDPTX == ASTDP_DPMCU_TX) {
-   // Wait one second then timeout.
-   i = 0;
-
-   while (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
COPROCESSOR_LAUNCH) !=
-   COPROCESSOR_LAUNCH) {
-   i++;
-   // wait 100 ms
-   msleep(100);
-
-   if (i >= 10) {
-   // DP would not be ready.
-   bDPExecute = 0;
-   break;
-   }
+   if (i >= 10) {
+   // DP would not be ready.
+   bDPExecute = 0;
+   break;
}
+   }
 
-   if (bDPExecute)
-   ast->tx_chip_types |= BIT(AST_TX_ASTDP);
+   if (!bDPExecute)
+   drm_err(dev, "Wait DPMCU executing timeout\n");
 
-   ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5,
-   (u8) 
~ASTDP_HOST_EDID_READ_DONE_MASK,
-   
ASTDP_HOST_EDID_READ_DONE);
-   }
+   ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5,
+  (u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
+  ASTDP_HOST_EDID_READ_DONE);
 }
 
 
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index d51b81fea9c8..15e86394be4f 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -498,7 +498,7 @@ struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
 
 /* aspeed DP */
 int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata);
-void ast_dp_launch(struct drm_device *dev, u8 bPower);
+void ast_dp_launch(struct drm_device *dev);
 void ast_dp_power_on_off(struct drm_device *dev, bool no);
 void ast_dp_set_on_off(struct drm_device *dev, bool no);
 void ast_dp_set_mode(struct drm_crtc *crtc, struct ast_vbios_mode_info 
*vbios_mode);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f83ce77127cb..8ecddf20113f 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -254,8 +254,13 @@ static int ast_detect_chip(struct drm_device *dev, bool 
*need_post)
case 0x0c:
ast->tx_chip_types = AST_TX_DP501_BIT;
}
-   } else if (ast->chip == AST2600)
-   ast_dp_launch(>base, 0);
+   } else if (ast->chip == AST2600) {
+   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;
+   ast_dp_launch(>base);
+   }
+   }
 
/* Print stuff for diagnostic purposes */
if (ast->tx_chip_types & AST_TX_NONE_BIT)
@@ -264,6 +269,8 @@ static int ast_detect_chip(struct drm_device *dev, bool 
*need_post)
drm_info(dev, "Using Sil164 TMDS transmitter\n");
if (ast->tx_chip_types &a

[PATCH] drm/ast: Fix long time waiting on s3/s4 resume

[Jammy Huang]

In resume, DP's launch function, ast_dp_launch, could wait at most 30
seconds before timeout to check if DP is enabled.

To avoid this problem, we only check if DP enable or not at driver probe.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217278
Signed-off-by: Jammy Huang 
---
 drivers/gpu/drm/ast/ast_dp.c   | 53 ++
 drivers/gpu/drm/ast/ast_drv.h  |  2 +-
 drivers/gpu/drm/ast/ast_main.c | 11 +--
 drivers/gpu/drm/ast/ast_post.c |  3 +-
 4 files changed, 28 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
index 56483860306b..3a4218102631 100644
--- a/drivers/gpu/drm/ast/ast_dp.c
+++ b/drivers/gpu/drm/ast/ast_dp.c
@@ -121,51 +121,30 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 
*ediddata)
  */
 void ast_dp_launch(struct drm_device *dev, u8 bPower)
 {
-   u32 i = 0, j = 0, WaitCount = 1;
-   u8 bDPTX = 0;
+   u32 i = 0;
u8 bDPExecute = 1;
-
struct ast_private *ast = to_ast_private(dev);
-   // S3 come back, need more time to wait BMC ready.
-   if (bPower)
-   WaitCount = 300;
-
-
-   // Wait total count by different condition.
-   for (j = 0; j < WaitCount; j++) {
-   bDPTX = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
TX_TYPE_MASK);
-
-   if (bDPTX)
-   break;
 
+   // Wait one second then timeout.
+   while (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
COPROCESSOR_LAUNCH) !=
+   COPROCESSOR_LAUNCH) {
+   i++;
+   // wait 100 ms
msleep(100);
-   }
 
-   // 0xE : ASTDP with DPMCU FW handling
-   if (bDPTX == ASTDP_DPMCU_TX) {
-   // Wait one second then timeout.
-   i = 0;
-
-   while (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, 
COPROCESSOR_LAUNCH) !=
-   COPROCESSOR_LAUNCH) {
-   i++;
-   // wait 100 ms
-   msleep(100);
-
-   if (i >= 10) {
-   // DP would not be ready.
-   bDPExecute = 0;
-   break;
-   }
+   if (i >= 10) {
+   // DP would not be ready.
+   bDPExecute = 0;
+   break;
}
+   }
 
-   if (bDPExecute)
-   ast->tx_chip_types |= BIT(AST_TX_ASTDP);
+   if (!bDPExecute)
+   drm_err(dev, "Wait DPMCU executing timeout\n");
 
-   ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5,
-   (u8) 
~ASTDP_HOST_EDID_READ_DONE_MASK,
-   
ASTDP_HOST_EDID_READ_DONE);
-   }
+   ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5,
+  (u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
+  ASTDP_HOST_EDID_READ_DONE);
 }
 
 
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index d51b81fea9c8..15e86394be4f 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -498,7 +498,7 @@ struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
 
 /* aspeed DP */
 int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata);
-void ast_dp_launch(struct drm_device *dev, u8 bPower);
+void ast_dp_launch(struct drm_device *dev);
 void ast_dp_power_on_off(struct drm_device *dev, bool no);
 void ast_dp_set_on_off(struct drm_device *dev, bool no);
 void ast_dp_set_mode(struct drm_crtc *crtc, struct ast_vbios_mode_info 
*vbios_mode);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f83ce77127cb..8ecddf20113f 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -254,8 +254,13 @@ static int ast_detect_chip(struct drm_device *dev, bool 
*need_post)
case 0x0c:
ast->tx_chip_types = AST_TX_DP501_BIT;
}
-   } else if (ast->chip == AST2600)
-   ast_dp_launch(>base, 0);
+   } else if (ast->chip == AST2600) {
+   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;
+   ast_dp_launch(>base);
+   }
+   }
 
/* Print stuff for diagnostic purposes */
if (ast->tx_chip_types & AST_TX_NONE_BIT)
@@ -264,6 +269,8 @@ static int ast_detect_chip(struct drm_device *dev, bool 
*need_post)
drm_info(dev, "Using Sil164 TMDS transmitter\n");
if (ast->tx_chip_types & AST_TX_DP501_BIT)
drm_info(dev, "Using DP501 DisplayPort transmitter\n");
+

Re: [PATCH] drm/ast: Fix ARM compatibility

[Jammy Huang]

Hi Thomas,

Could you help review this patch??

We met some problem on nvidia's ARM platfrom and need this patch to fix it.

On 2023/3/2 上午 10:19, Jammy Huang wrote:

ARM architecture only has 'memory', so all devices are accessed by MMIO.

Signed-off-by: Jammy Huang 
---
  drivers/gpu/drm/ast/ast_main.c | 17 +
  1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 794ffd4a29c5..f86d01e9f024 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -424,22 +424,7 @@ struct ast_device *ast_device_create(const struct 
drm_driver *drv,
if (!ast->regs)
return ERR_PTR(-EIO);
  
-	/*

-* If we don't have IO space at all, use MMIO now and
-* assume the chip has MMIO enabled by default (rev 0x20
-* and higher).
-*/
-   if (!(pci_resource_flags(pdev, 2) & IORESOURCE_IO)) {
-   drm_info(dev, "platform has no IO space, trying MMIO\n");
-   ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
-   }
-
-   /* "map" IO regs if the above hasn't done so already */
-   if (!ast->ioregs) {
-   ast->ioregs = pcim_iomap(pdev, 2, 0);
-   if (!ast->ioregs)
-   return ERR_PTR(-EIO);
-   }
+   ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
  
  	ast_detect_chip(dev, _post);
  


base-commit: 254986e324add8a30d0019c6da59f81adc8b565f


--
Best Regards
Jammy

[PATCH] drm/ast: Fix ARM compatibility

[Jammy Huang]

ARM architecture only has 'memory', so all devices are accessed by MMIO.

Signed-off-by: Jammy Huang 
---
 drivers/gpu/drm/ast/ast_main.c | 17 +
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 794ffd4a29c5..f86d01e9f024 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -424,22 +424,7 @@ struct ast_device *ast_device_create(const struct 
drm_driver *drv,
if (!ast->regs)
return ERR_PTR(-EIO);
 
-   /*
-* If we don't have IO space at all, use MMIO now and
-* assume the chip has MMIO enabled by default (rev 0x20
-* and higher).
-*/
-   if (!(pci_resource_flags(pdev, 2) & IORESOURCE_IO)) {
-   drm_info(dev, "platform has no IO space, trying MMIO\n");
-   ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
-   }
-
-   /* "map" IO regs if the above hasn't done so already */
-   if (!ast->ioregs) {
-   ast->ioregs = pcim_iomap(pdev, 2, 0);
-   if (!ast->ioregs)
-   return ERR_PTR(-EIO);
-   }
+   ast->ioregs = ast->regs + AST_IO_MM_OFFSET;
 
ast_detect_chip(dev, _post);
 

base-commit: 254986e324add8a30d0019c6da59f81adc8b565f
-- 
2.25.1

Re: [PATCH v3] drm/ast: Fix start address computation

[Jammy Huang]

On 2023/2/9 下午 05:55, Thomas Zimmermann wrote:



Am 09.02.23 um 10:44 schrieb Jocelyn Falempe:

During the driver conversion to shmem, the start address for the
scanout buffer was set to the base PCI address.
In most cases it works because only the lower 24bits are used, and
due to alignment it was almost always 0.
But on some unlucky hardware, it's not the case, and some unitilized


'uninitialized'


memory is displayed on the BMC.
With shmem, the primary plane is always at offset 0 in GPU memory.

  * v2: rewrite the patch to set the offset to 0. (Thomas Zimmermann)
  * v3: move the change to plane_init() and also fix the cursor plane.
    (Jammy Huang)

Tested on a sr645 affected by this bug.

Fixes: f2fa5a99ca81 ("drm/ast: Convert ast to SHMEM")
Signed-off-by: Jocelyn Falempe 


Reviewed-by: Thomas Zimmermann 


Reviewed-by: Jammy Huang 




---
  drivers/gpu/drm/ast/ast_mode.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c 
b/drivers/gpu/drm/ast/ast_mode.c

index c7443317c747..66a4a41c3fe9 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -714,7 +714,7 @@ static int ast_primary_plane_init(struct 
ast_private *ast)

  struct ast_plane *ast_primary_plane = >primary_plane;
  struct drm_plane *primary_plane = _primary_plane->base;
  void __iomem *vaddr = ast->vram;
-    u64 offset = ast->vram_base;
+    u64 offset = 0; /* with shmem, the primary plane is always at 
offset 0 */
  unsigned long cursor_size = roundup(AST_HWC_SIZE + 
AST_HWC_SIGNATURE_SIZE, PAGE_SIZE);

  unsigned long size = ast->vram_fb_available - cursor_size;
  int ret;
@@ -972,7 +972,7 @@ static int ast_cursor_plane_init(struct 
ast_private *ast)

  return -ENOMEM;
    vaddr = ast->vram + ast->vram_fb_available - size;
-    offset = ast->vram_base + ast->vram_fb_available - size;
+    offset = ast->vram_fb_available - size;
    ret = ast_plane_init(dev, ast_cursor_plane, vaddr, offset, size,
   0x01, _cursor_plane_funcs,



--
Best Regards
Jammy

Re: [PATCH v2] drm/ast: Fix start address computation

[Jammy Huang]

Hello,

The offset given to ast_set_start_address_crt1() should be offset in 
vram. It should 0 now as your patch.


I think it is better to correct it in ast_primary_plane_init() and 
ast_cursor_plane_init() as below.


--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -714,7 +714,7 @@ static int ast_primary_plane_init(struct ast_private 
*ast)

    struct ast_plane *ast_primary_plane = >primary_plane;
    struct drm_plane *primary_plane = _primary_plane->base;
    void __iomem *vaddr = ast->vram;
-   u64 offset = ast->vram_base;
+   u64 offset = 0;
    unsigned long cursor_size = roundup(AST_HWC_SIZE + 
AST_HWC_SIGNATURE_SIZE, PAGE_SIZE);

    unsigned long size = ast->vram_fb_available - cursor_size;
    int ret;
@@ -972,7 +972,7 @@ static int ast_cursor_plane_init(struct ast_private 
*ast)

    return -ENOMEM;

    vaddr = ast->vram + ast->vram_fb_available - size;
-   offset = ast->vram_base + ast->vram_fb_available - size;
+   offset = st->vram_fb_available - size;

On 2023/2/9 下午 05:12, Jocelyn Falempe wrote:

During the driver conversion to shmem, the start address for the
scanout buffer was set to the base PCI address.
In most cases it works because only the lower 24bits are used, and
due to alignment it was almost always 0.
But on some unlucky hardware, it's not the case, and some unitilized
memory is displayed on the BMC.
With shmem, the primary plane is always at offset 0 in GPU memory.

Tested on a sr645 affected by this bug.

Fixes: f2fa5a99ca81 ("drm/ast: Convert ast to SHMEM")
Signed-off-by: Jocelyn Falempe 
---
  drivers/gpu/drm/ast/ast_mode.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index c7443317c747..54deb29bfeb3 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -681,7 +681,8 @@ static void ast_primary_plane_helper_atomic_update(struct 
drm_plane *plane,
if (!old_fb || old_fb->pitches[0] != fb->pitches[0])
ast_set_offset_reg(ast, fb);
if (!old_fb) {
-   ast_set_start_address_crt1(ast, (u32)ast_plane->offset);
+   /* with shmem, the primary plane is always at offset 0 */
+   ast_set_start_address_crt1(ast, 0);
ast_set_index_reg_mask(ast, AST_IO_SEQ_PORT, 0x1, 0xdf, 0x00);
}
  }


--
Best Regards
Jammy

Re: [PATCH] drm/ast: Fix no display at WayLand after power-off

[Jammy Huang]

Hi Thomas,


OK. Thanks for the reminder.


On 2022/12/9 下午 05:45, Thomas Zimmermann wrote:

Hi

Am 09.12.22 um 10:11 schrieb Jammy Huang:

With WayLand, there is error log when display waken up from power-off:
gnome-shell: Failed to post KMS update: CRTC property (GAMMA_LUT) not 
found

gnome-shell: Page flip discarded: CRTC property (GAMMA_LUT) not found

To fix the issue, enable GAMMA_LUT property on CRTC.


This code has long been replaced by commit ce7fcf700386 ("drm/ast: Add 
Atomic gamma lut support for aspeed"), which adds proper color 
management to ast.


Please only submit patches for the latest driver in drm-misc-next.

Best regards
Thomas



Signed-off-by: Jammy Huang 
---
  drivers/gpu/drm/ast/ast_drv.h  | 2 ++
  drivers/gpu/drm/ast/ast_mode.c | 5 +++--
  2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h 
b/drivers/gpu/drm/ast/ast_drv.h

index 2e44b971c3a6..fd9af1cf0563 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -86,6 +86,8 @@ enum ast_tx_chip {
  #define AST_DRAM_4Gx16   7
  #define AST_DRAM_8Gx16   8
  +#define MAX_COLOR_LUT_ENTRIES 256
+
  /*
   * Cursor plane
   */
diff --git a/drivers/gpu/drm/ast/ast_mode.c 
b/drivers/gpu/drm/ast/ast_mode.c

index 1bc0220e6783..87bd9697bb44 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -74,7 +74,7 @@ static void ast_crtc_load_lut(struct ast_private 
*ast, struct drm_crtc *crtc)

  g = r + crtc->gamma_size;
  b = g + crtc->gamma_size;
  -    for (i = 0; i < 256; i++)
+    for (i = 0; i < MAX_COLOR_LUT_ENTRIES; i++)
  ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ 
>> 8);

  }
  @@ -1323,7 +1323,8 @@ static int ast_crtc_init(struct drm_device *dev)
  if (ret)
  return ret;
  -    drm_mode_crtc_set_gamma_size(crtc, 256);
+    drm_crtc_enable_color_mgmt(crtc, 0, false, MAX_COLOR_LUT_ENTRIES);
+    drm_mode_crtc_set_gamma_size(crtc, MAX_COLOR_LUT_ENTRIES);
  drm_crtc_helper_add(crtc, _crtc_helper_funcs);
    return 0;

base-commit: 8ed710da2873c2aeb3bb805864a699affaf1d03b



--
Best Regards
Jammy

[PATCH] drm/ast: Fix no display at WayLand after power-off

[Jammy Huang]

With WayLand, there is error log when display waken up from power-off:
gnome-shell: Failed to post KMS update: CRTC property (GAMMA_LUT) not found
gnome-shell: Page flip discarded: CRTC property (GAMMA_LUT) not found

To fix the issue, enable GAMMA_LUT property on CRTC.

Signed-off-by: Jammy Huang 
---
 drivers/gpu/drm/ast/ast_drv.h  | 2 ++
 drivers/gpu/drm/ast/ast_mode.c | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 2e44b971c3a6..fd9af1cf0563 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -86,6 +86,8 @@ enum ast_tx_chip {
 #define AST_DRAM_4Gx16   7
 #define AST_DRAM_8Gx16   8
 
+#define MAX_COLOR_LUT_ENTRIES 256
+
 /*
  * Cursor plane
  */
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 1bc0220e6783..87bd9697bb44 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -74,7 +74,7 @@ static void ast_crtc_load_lut(struct ast_private *ast, struct 
drm_crtc *crtc)
g = r + crtc->gamma_size;
b = g + crtc->gamma_size;
 
-   for (i = 0; i < 256; i++)
+   for (i = 0; i < MAX_COLOR_LUT_ENTRIES; i++)
ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8);
 }
 
@@ -1323,7 +1323,8 @@ static int ast_crtc_init(struct drm_device *dev)
if (ret)
return ret;
 
-   drm_mode_crtc_set_gamma_size(crtc, 256);
+   drm_crtc_enable_color_mgmt(crtc, 0, false, MAX_COLOR_LUT_ENTRIES);
+   drm_mode_crtc_set_gamma_size(crtc, MAX_COLOR_LUT_ENTRIES);
drm_crtc_helper_add(crtc, _crtc_helper_funcs);
 
return 0;

base-commit: 8ed710da2873c2aeb3bb805864a699affaf1d03b
-- 
2.25.1

[PATCH] drm/hisilicon/kirin: Fix typo in kirin_drm_drv.c

[Jammy Huang]

'config_max_height' should be used for 'max_height'.

Signed-off-by: Jammy Huang 
---
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c 
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index 2af51df6dca7..b23ad03157e1 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -165,7 +165,7 @@ static int kirin_drm_kms_init(struct drm_device *dev,
dev->mode_config.min_width = 0;
dev->mode_config.min_height = 0;
dev->mode_config.max_width = driver_data->config_max_width;
-   dev->mode_config.max_height = driver_data->config_max_width;
+   dev->mode_config.max_height = driver_data->config_max_height;
dev->mode_config.funcs = driver_data->mode_config_funcs;
 
/* display controller init */
-- 
2.25.1

Re: [PATCH] drm/ast: Fix video broken in some resolutions on ast2600

[Jammy Huang]

Hi Thomas,

On 2022/9/20 下午 04:43, Thomas Zimmermann wrote:

Hi

Am 16.09.22 um 11:17 schrieb Jammy Huang:

Some cases are not handled well for ast2600.

Signed-off-by: Jammy Huang 
---
  drivers/gpu/drm/ast/ast_mode.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c 
b/drivers/gpu/drm/ast/ast_mode.c

index d327dcbaf032..be9bbc888ac3 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -314,7 +314,7 @@ static void ast_set_crtc_reg(struct ast_private 
*ast,
  u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD = 0, 
jregAE = 0;

  u16 temp, precache = 0;
  -    if ((ast->chip == AST2500) &&
+    if ((ast->chip == AST2500 || ast->chip == AST2600) &&
  (vbios_mode->enh_table->flags & AST2500PreCatchCRT))
  precache = 40;
  @@ -355,6 +355,12 @@ static void ast_set_crtc_reg(struct 
ast_private *ast,

  ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xAC, 0x00, jregAC);
  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);


Applied to drm-misc-next, but what is an 'octave pixel'?


It means to make the pixel number a multiple of 8.




Best regards
Thomas


+    if ((ast->chip == AST2600) && (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);

+
  /* vert timings */
  temp = (mode->crtc_vtotal) - 2;
  if (temp & 0x100)
@@ -432,7 +438,7 @@ static void ast_set_dclk_reg(struct ast_private 
*ast,

  {
  const struct ast_vbios_dclk_info *clk_info;
  -    if (ast->chip == AST2500)
+    if ((ast->chip == AST2500) || (ast->chip == AST2600))
  clk_info = 
_table_ast2500[vbios_mode->enh_table->dclk_index];

  else
  clk_info = _table[vbios_mode->enh_table->dclk_index];



--
Best Regards
Jammy

[PATCH] drm/ast: Fix video broken in some resolutions on ast2600

[Jammy Huang]

Some cases are not handled well for ast2600.

Signed-off-by: Jammy Huang 
---
 drivers/gpu/drm/ast/ast_mode.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index d327dcbaf032..be9bbc888ac3 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -314,7 +314,7 @@ static void ast_set_crtc_reg(struct ast_private *ast,
u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD = 0, jregAE = 
0;
u16 temp, precache = 0;
 
-   if ((ast->chip == AST2500) &&
+   if ((ast->chip == AST2500 || ast->chip == AST2600) &&
(vbios_mode->enh_table->flags & AST2500PreCatchCRT))
precache = 40;
 
@@ -355,6 +355,12 @@ static void ast_set_crtc_reg(struct ast_private *ast,
ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xAC, 0x00, jregAC);
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))
+   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);
+
/* vert timings */
temp = (mode->crtc_vtotal) - 2;
if (temp & 0x100)
@@ -432,7 +438,7 @@ static void ast_set_dclk_reg(struct ast_private *ast,
 {
const struct ast_vbios_dclk_info *clk_info;
 
-   if (ast->chip == AST2500)
+   if ((ast->chip == AST2500) || (ast->chip == AST2600))
clk_info = 
_table_ast2500[vbios_mode->enh_table->dclk_index];
else
clk_info = _table[vbios_mode->enh_table->dclk_index];
-- 
2.25.1

[PATCH v2] drm/ast: Add resolution support for 1152x864@75

[Jammy Huang]

Add 1152x864 into support list.

Signed-off-by: Jammy Huang 
---
v2 changes:
 - add check in mode_valid
---
 drivers/gpu/drm/ast/ast_mode.c   | 9 +
 drivers/gpu/drm/ast/ast_tables.h | 7 +++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 214b10178454..d327dcbaf032 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -114,6 +114,9 @@ static bool ast_get_vbios_mode_info(const struct 
drm_format_info *format,
case 1024:
vbios_mode->enh_table = _1024x768[refresh_rate_index];
break;
+   case 1152:
+   vbios_mode->enh_table = _1152x864[refresh_rate_index];
+   break;
case 1280:
if (mode->crtc_vdisplay == 800)
vbios_mode->enh_table = 
_1280x800[refresh_rate_index];
@@ -1058,6 +1061,8 @@ ast_crtc_helper_mode_valid(struct drm_crtc *crtc, const 
struct drm_display_mode
return MODE_OK;
if ((mode->hdisplay == 1600) && (mode->vdisplay == 900))
return MODE_OK;
+   if ((mode->hdisplay == 1152) && (mode->vdisplay == 864))
+   return MODE_OK;
 
if ((ast->chip == AST2100) || (ast->chip == AST2200) ||
(ast->chip == AST2300) || (ast->chip == AST2400) ||
@@ -1090,6 +1095,10 @@ ast_crtc_helper_mode_valid(struct drm_crtc *crtc, const 
struct drm_display_mode
if (mode->vdisplay == 768)
status = MODE_OK;
break;
+   case 1152:
+   if (mode->vdisplay == 864)
+   status = MODE_OK;
+   break;
case 1280:
if (mode->vdisplay == 1024)
status = MODE_OK;
diff --git a/drivers/gpu/drm/ast/ast_tables.h b/drivers/gpu/drm/ast/ast_tables.h
index dbe1cc620f6e..0378c9bc079b 100644
--- a/drivers/gpu/drm/ast/ast_tables.h
+++ b/drivers/gpu/drm/ast/ast_tables.h
@@ -272,6 +272,13 @@ static const struct ast_vbios_enhtable res_1600x1200[] = {
 (SyncPP | Charx8Dot), 0xFF, 1, 0x33 },
 };
 
+static const struct ast_vbios_enhtable res_1152x864[] = {
+   {1600, 1152, 64, 128,  900,  864, 1, 3, VCLK108,/* 75Hz */
+(SyncPP | Charx8Dot | NewModeInfo), 75, 1, 0x3B },
+   {1600, 1152, 64, 128,  900,  864, 1, 3, VCLK108,/* end */
+(SyncPP | Charx8Dot | NewModeInfo), 0xFF, 1, 0x3B },
+};
+
 /* 16:9 */
 static const struct ast_vbios_enhtable res_1360x768[] = {
{1792, 1360, 64, 112, 795, 768, 3, 6, VCLK85_5, /* 60Hz */
-- 
2.25.1

[PATCH] drm/ast: Add resolution support for 1152x864@75

[Jammy Huang]

Add 1152x864 into support list.

Signed-off-by: Jammy Huang 
---
 drivers/gpu/drm/ast/ast_mode.c   | 3 +++
 drivers/gpu/drm/ast/ast_tables.h | 7 +++
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 214b10178454..0b2c5fe8dc2c 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -114,6 +114,9 @@ static bool ast_get_vbios_mode_info(const struct 
drm_format_info *format,
case 1024:
vbios_mode->enh_table = _1024x768[refresh_rate_index];
break;
+   case 1152:
+   vbios_mode->enh_table = _1152x864[refresh_rate_index];
+   break;
case 1280:
if (mode->crtc_vdisplay == 800)
vbios_mode->enh_table = 
_1280x800[refresh_rate_index];
diff --git a/drivers/gpu/drm/ast/ast_tables.h b/drivers/gpu/drm/ast/ast_tables.h
index dbe1cc620f6e..0378c9bc079b 100644
--- a/drivers/gpu/drm/ast/ast_tables.h
+++ b/drivers/gpu/drm/ast/ast_tables.h
@@ -272,6 +272,13 @@ static const struct ast_vbios_enhtable res_1600x1200[] = {
 (SyncPP | Charx8Dot), 0xFF, 1, 0x33 },
 };
 
+static const struct ast_vbios_enhtable res_1152x864[] = {
+   {1600, 1152, 64, 128,  900,  864, 1, 3, VCLK108,/* 75Hz */
+(SyncPP | Charx8Dot | NewModeInfo), 75, 1, 0x3B },
+   {1600, 1152, 64, 128,  900,  864, 1, 3, VCLK108,/* end */
+(SyncPP | Charx8Dot | NewModeInfo), 0xFF, 1, 0x3B },
+};
+
 /* 16:9 */
 static const struct ast_vbios_enhtable res_1360x768[] = {
{1792, 1360, 64, 112, 795, 768, 3, 6, VCLK85_5, /* 60Hz */
-- 
2.25.1

[PATCH] drm/hisilicon/kirin: Fix typo in kirin_drm_drv.c

[Jammy Huang]

'config_max_height' should be used for 'max_height'.

Signed-off-by: Jammy Huang 
---
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c 
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index 2af51df6dca7..b23ad03157e1 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -165,7 +165,7 @@ static int kirin_drm_kms_init(struct drm_device *dev,
dev->mode_config.min_width = 0;
dev->mode_config.min_height = 0;
dev->mode_config.max_width = driver_data->config_max_width;
-   dev->mode_config.max_height = driver_data->config_max_width;
+   dev->mode_config.max_height = driver_data->config_max_height;
dev->mode_config.funcs = driver_data->mode_config_funcs;
 
/* display controller init */
-- 
2.25.1