diff mbox series

[5/8] fbdev: sm712fb: fix crashes during framebuffer writes by correctly mapping VRAM.

Message ID 20190316222504.27170-6-tomli@tomli.me (mailing list archive)
State New, archived
Headers show
Series fbdev: sm712fb: fix a series of lockups, crashes and gliches. | expand

Commit Message

Yifeng Li March 16, 2019, 10:25 p.m. UTC
On a Thinkpad s30 (Pentium III / i440MX, Lynx3DM), running fbtest or X
will crash the machine instantly, because the VRAM/framebuffer is not
mapped correctly.

On SM712, the framebuffer starts at the beginning of address space, but
SM720's framebuffer starts at the 1 MiB offset from the beginning. However,
sm712fb fails to take this into account, as a result, writing to the
framebuffer will destroy all the registers and kill the system immediately.
Another problem is the driver assumes 8 MiB of VRAM for SM720, but some
SM720 system, such as this IBM Thinkpad, only has 4 MiB of VRAM.

Fix this problem by removing the hardcoded VRAM size, adding a function to
query the amount of VRAM from register MCR76 on SM720, and adding proper
framebuffer offset.

Please note that the memory map may have additional problems on Big-Endian
system, which is not available for testing by myself. But I highly suspect
that the original code is also broken on Big-Endian machines for SM720, so
at least we are not making the problem worse. More, the driver also assumed
SM710/SM712 has 4 MiB of VRAM, but it has a 2 MiB version as well, and used
in earlier laptops, such as IBM Thinkpad 240X, the driver would probably
crash on them. I've never seen one of those machines and cannot fix it, but
I have documented these problems in the comments.

Signed-off-by: Yifeng Li <tomli@tomli.me>
Cc: stable@vger.kernel.org  # v4.4+
---
 drivers/video/fbdev/sm712.h   |  5 ----
 drivers/video/fbdev/sm712fb.c | 48 ++++++++++++++++++++++++++++++++---
 2 files changed, 44 insertions(+), 9 deletions(-)

Comments

Sudip Mukherjee March 24, 2019, 9:39 p.m. UTC | #1
Hi Teddy,

On Sun, Mar 17, 2019 at 06:25:01AM +0800, Yifeng Li wrote:
> On a Thinkpad s30 (Pentium III / i440MX, Lynx3DM), running fbtest or X
> will crash the machine instantly, because the VRAM/framebuffer is not
> mapped correctly.
> 
> On SM712, the framebuffer starts at the beginning of address space, but
> SM720's framebuffer starts at the 1 MiB offset from the beginning. However,
> sm712fb fails to take this into account, as a result, writing to the
> framebuffer will destroy all the registers and kill the system immediately.
> Another problem is the driver assumes 8 MiB of VRAM for SM720, but some
> SM720 system, such as this IBM Thinkpad, only has 4 MiB of VRAM.
> 
> Fix this problem by removing the hardcoded VRAM size, adding a function to
> query the amount of VRAM from register MCR76 on SM720, and adding proper
> framebuffer offset.
> 
> Please note that the memory map may have additional problems on Big-Endian
> system, which is not available for testing by myself. But I highly suspect
> that the original code is also broken on Big-Endian machines for SM720, so
> at least we are not making the problem worse. More, the driver also assumed
> SM710/SM712 has 4 MiB of VRAM, but it has a 2 MiB version as well, and used
> in earlier laptops, such as IBM Thinkpad 240X, the driver would probably
> crash on them. I've never seen one of those machines and cannot fix it, but
> I have documented these problems in the comments.

I only have access to SM712 and I am testing this series on it. Can you please
test them on SM710, SM720 and also on big-endian machine, specially the patches
from 5 to 8.

--
Regards
Sudip
Sudip Mukherjee March 31, 2019, 12:20 p.m. UTC | #2
On Sun, Mar 17, 2019 at 06:25:01AM +0800, Yifeng Li wrote:
> On a Thinkpad s30 (Pentium III / i440MX, Lynx3DM), running fbtest or X
> will crash the machine instantly, because the VRAM/framebuffer is not
> mapped correctly.
> 
> On SM712, the framebuffer starts at the beginning of address space, but
> SM720's framebuffer starts at the 1 MiB offset from the beginning. However,
> sm712fb fails to take this into account, as a result, writing to the
> framebuffer will destroy all the registers and kill the system immediately.
> Another problem is the driver assumes 8 MiB of VRAM for SM720, but some
> SM720 system, such as this IBM Thinkpad, only has 4 MiB of VRAM.
> 
> Fix this problem by removing the hardcoded VRAM size, adding a function to
> query the amount of VRAM from register MCR76 on SM720, and adding proper
> framebuffer offset.
> 
> Please note that the memory map may have additional problems on Big-Endian
> system, which is not available for testing by myself. But I highly suspect
> that the original code is also broken on Big-Endian machines for SM720, so
> at least we are not making the problem worse. More, the driver also assumed
> SM710/SM712 has 4 MiB of VRAM, but it has a 2 MiB version as well, and used
> in earlier laptops, such as IBM Thinkpad 240X, the driver would probably
> crash on them. I've never seen one of those machines and cannot fix it, but
> I have documented these problems in the comments.
> 
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> Cc: stable@vger.kernel.org  # v4.4+

Tested-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>

--
Regards
Sudip
diff mbox series

Patch

diff --git a/drivers/video/fbdev/sm712.h b/drivers/video/fbdev/sm712.h
index aad1cc4be34a..2cba1e73ed24 100644
--- a/drivers/video/fbdev/sm712.h
+++ b/drivers/video/fbdev/sm712.h
@@ -19,11 +19,6 @@ 
 #define SCREEN_Y_RES      600
 #define SCREEN_BPP        16
 
-/*Assume SM712 graphics chip has 4MB VRAM */
-#define SM712_VIDEOMEMORYSIZE	  0x00400000
-/*Assume SM722 graphics chip has 8MB VRAM */
-#define SM722_VIDEOMEMORYSIZE	  0x00800000
-
 #define dac_reg	(0x3c8)
 #define dac_val	(0x3c9)
 
diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
index e8149f0f47d5..52234c57be02 100644
--- a/drivers/video/fbdev/sm712fb.c
+++ b/drivers/video/fbdev/sm712fb.c
@@ -1329,6 +1329,11 @@  static int smtc_map_smem(struct smtcfb_info *sfb,
 {
 	sfb->fb->fix.smem_start = pci_resource_start(pdev, 0);
 
+	if (sfb->chip_id == 0x720)
+		/* on SM720, the framebuffer starts at the 1 MB offset */
+		sfb->fb->fix.smem_start += 0x00200000;
+
+	/* XXX: is it safe for SM720 on Big-Endian? */
 	if (sfb->fb->var.bits_per_pixel == 32)
 		sfb->fb->fix.smem_start += big_addr;
 
@@ -1366,12 +1371,45 @@  static inline void sm7xx_init_hw(void)
 	outb_p(0x11, 0x3c5);
 }
 
+static u_long sm7xx_vram_probe(struct smtcfb_info *sfb)
+{
+	u8 vram;
+
+	switch (sfb->chip_id) {
+	case 0x710:
+	case 0x712:
+		/*
+		 * Assume SM712 graphics chip has 4MB VRAM.
+		 *
+		 * FIXME: SM712 can have 2MB VRAM, which is used on earlier
+		 * laptops, such as IBM Thinkpad 240X. This driver would
+		 * probably crash on those machines. If anyone gets one of
+		 * those and is willing to help, run "git blame" and send me
+		 * an E-mail.
+		 */
+		return 0x00400000;
+	case 0x720:
+		outb_p(0x76, 0x3c4);
+		vram = inb_p(0x3c5) >> 6;
+
+		if (vram == 0x00)
+			return 0x00800000;  /* 8 MB */
+		else if (vram == 0x01)
+			return 0x01000000;  /* 16 MB */
+		else if (vram == 0x02)
+			return 0x00400000;  /* illegal, fallback to 4 MB */
+		else if (vram == 0x03)
+			return 0x00400000;  /* 4 MB */
+	}
+	return 0;  /* unknown hardware */
+}
+
 static int smtcfb_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *ent)
 {
 	struct smtcfb_info *sfb;
 	struct fb_info *info;
-	u_long smem_size = 0x00800000;	/* default 8MB */
+	u_long smem_size;
 	int err;
 	unsigned long mmio_base;
 
@@ -1428,12 +1466,15 @@  static int smtcfb_pci_probe(struct pci_dev *pdev,
 	mmio_base = pci_resource_start(pdev, 0);
 	pci_read_config_byte(pdev, PCI_REVISION_ID, &sfb->chip_rev_id);
 
+	smem_size = sm7xx_vram_probe(sfb);
+	dev_info(&pdev->dev, "%lu MiB of VRAM detected.\n",
+					smem_size / 1048576);
+
 	switch (sfb->chip_id) {
 	case 0x710:
 	case 0x712:
 		sfb->fb->fix.mmio_start = mmio_base + 0x00400000;
 		sfb->fb->fix.mmio_len = 0x00400000;
-		smem_size = SM712_VIDEOMEMORYSIZE;
 		sfb->lfb = ioremap(mmio_base, mmio_addr);
 		if (!sfb->lfb) {
 			dev_err(&pdev->dev,
@@ -1465,8 +1506,7 @@  static int smtcfb_pci_probe(struct pci_dev *pdev,
 	case 0x720:
 		sfb->fb->fix.mmio_start = mmio_base;
 		sfb->fb->fix.mmio_len = 0x00200000;
-		smem_size = SM722_VIDEOMEMORYSIZE;
-		sfb->dp_regs = ioremap(mmio_base, 0x00a00000);
+		sfb->dp_regs = ioremap(mmio_base, 0x00200000 + smem_size);
 		sfb->lfb = sfb->dp_regs + 0x00200000;
 		sfb->mmio = (smtc_regbaseaddress =
 		    sfb->dp_regs + 0x000c0000);