* [PATCH v2 0/7] implement 2D acceleration, minor cleanups, doc updates.
@ 2019-03-22 5:17 ` Yifeng Li
0 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
v2:
- Also implement 2D acceleration for SM720.
- Remove writel_relaxed() and an explicit memory barrier, on MIPS, PowerPC and
x86, relaxed writes do not have additional performance, and complicates the
code.
- Document additional issues in the driver.
NOTE:
- This patchset should be applied after "[PATCH 0/8] fbdev: sm712fb: fix a
series of lockups, crashes and gliches" (*), as it contains important
fixes for issues in the original driver.
- This patchset has been tested by the author for SM712 and SM720, on MIPS
and x86 without problem.
(*) https://marc.info/?l=linux-fbdev&m=155277512210375&w=2
Previously, in staging/sm7xxfb (now fbdev/sm712fb), 2D acceleration
was implemented, but after its submission, a critical bug that causes
total system hang was discovered, as a stopgap measure, 2D ops was
completele removed in commit 3af805735a25 ("staging: sm7xx: remove the
buggy 2D acceleration support") and never implemented again.
It created a massive usability problem - on YeeLoong 8089, a notable
MIPS platform which uses SM712 - even scrolling a single line of text
on the console required an unaccelerated screen redraw, running "dmesg"
typically takes 8-11 seconds, and absurdly, printf(), became a significant
performance bottleneck that slows down GCC and "make", make the computer
largely unusable.
So I decided to take a look. Most of the my actual development was done
in 2014 in a personal out-of-tree driver, I did not mainline it because
2D acceleration was not working properly in 24-bit color. I discovered
the solution in early 2019 and now it's ready to be mainlined.
This commit reimplements the 2D acceleration for sm712fb. Unlike the
original implementation, which was messy and unnecessarily complicated
by calling a 2D acceleration wrapper file with many unneeded functions,
this is a minimum and (relatively) clean implementation. My tests have
shown that running "dmesg" only takes 0.9 seconds, a performance boost
of 950%. System hangs did not occur in my tests.
Currently, 2D acceleration is only supported on little-endian CPUs, it's
disabled on Big Endian systems as a safety measure, since I code for myself
without any monetary or hardware support from any company or OEMs, I don't
have the hardware and it's completely untested. I should be also to
purchase a Big Endian test platform and add proper support soon.
Also, thanks to Miodrag Vallat and other OpenBSD developers, this work
would be impossible without their code, that served as a reference
implementation for me.
Finally, during the development and testing of 2D acceleration, many
identified existing issues in driver in general have been documented.
Yifeng Li (7):
fbdev: sm712fb: use type "u8" for 8-bit I/O.
fbdev: sm712fb: add 2D-related I/O headers and functions.
fbdev: sm712fb: support 2D acceleration on SM712 w/ Little-Endian CPU.
fbdev: sm712fb: add 32-bit color modes, drops some other modes.
Documentation: fb: sm712fb: add information mainly about 2D.
fbdev: sm712fb: Kconfig: add information about docs.
MAINTAINERS: sm712fb: list myself as one maintainer.
Documentation/fb/sm712fb.txt | 129 +++++++-
MAINTAINERS | 1 +
drivers/video/fbdev/Kconfig | 4 +
drivers/video/fbdev/sm712.h | 109 ++++++-
drivers/video/fbdev/sm712fb.c | 551 +++++++++++++++++++++++++---------
5 files changed, 642 insertions(+), 152 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 0/7] implement 2D acceleration, minor cleanups, doc updates.
@ 2019-03-22 5:17 ` Yifeng Li
0 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
v2:
- Also implement 2D acceleration for SM720.
- Remove writel_relaxed() and an explicit memory barrier, on MIPS, PowerPC and
x86, relaxed writes do not have additional performance, and complicates the
code.
- Document additional issues in the driver.
NOTE:
- This patchset should be applied after "[PATCH 0/8] fbdev: sm712fb: fix a
series of lockups, crashes and gliches" (*), as it contains important
fixes for issues in the original driver.
- This patchset has been tested by the author for SM712 and SM720, on MIPS
and x86 without problem.
(*) https://marc.info/?l=linux-fbdev&m\x155277512210375&w=2
Previously, in staging/sm7xxfb (now fbdev/sm712fb), 2D acceleration
was implemented, but after its submission, a critical bug that causes
total system hang was discovered, as a stopgap measure, 2D ops was
completele removed in commit 3af805735a25 ("staging: sm7xx: remove the
buggy 2D acceleration support") and never implemented again.
It created a massive usability problem - on YeeLoong 8089, a notable
MIPS platform which uses SM712 - even scrolling a single line of text
on the console required an unaccelerated screen redraw, running "dmesg"
typically takes 8-11 seconds, and absurdly, printf(), became a significant
performance bottleneck that slows down GCC and "make", make the computer
largely unusable.
So I decided to take a look. Most of the my actual development was done
in 2014 in a personal out-of-tree driver, I did not mainline it because
2D acceleration was not working properly in 24-bit color. I discovered
the solution in early 2019 and now it's ready to be mainlined.
This commit reimplements the 2D acceleration for sm712fb. Unlike the
original implementation, which was messy and unnecessarily complicated
by calling a 2D acceleration wrapper file with many unneeded functions,
this is a minimum and (relatively) clean implementation. My tests have
shown that running "dmesg" only takes 0.9 seconds, a performance boost
of 950%. System hangs did not occur in my tests.
Currently, 2D acceleration is only supported on little-endian CPUs, it's
disabled on Big Endian systems as a safety measure, since I code for myself
without any monetary or hardware support from any company or OEMs, I don't
have the hardware and it's completely untested. I should be also to
purchase a Big Endian test platform and add proper support soon.
Also, thanks to Miodrag Vallat and other OpenBSD developers, this work
would be impossible without their code, that served as a reference
implementation for me.
Finally, during the development and testing of 2D acceleration, many
identified existing issues in driver in general have been documented.
Yifeng Li (7):
fbdev: sm712fb: use type "u8" for 8-bit I/O.
fbdev: sm712fb: add 2D-related I/O headers and functions.
fbdev: sm712fb: support 2D acceleration on SM712 w/ Little-Endian CPU.
fbdev: sm712fb: add 32-bit color modes, drops some other modes.
Documentation: fb: sm712fb: add information mainly about 2D.
fbdev: sm712fb: Kconfig: add information about docs.
MAINTAINERS: sm712fb: list myself as one maintainer.
Documentation/fb/sm712fb.txt | 129 +++++++-
MAINTAINERS | 1 +
drivers/video/fbdev/Kconfig | 4 +
drivers/video/fbdev/sm712.h | 109 ++++++-
drivers/video/fbdev/sm712fb.c | 551 +++++++++++++++++++++++++---------
5 files changed, 642 insertions(+), 152 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 1/7] fbdev: sm712fb: use type "u8" for 8-bit I/O.
2019-03-22 5:17 ` Yifeng Li
@ 2019-03-22 5:17 ` Yifeng Li
-1 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
This commit converts "unsigned int" and "int" in I/O wrappers
to "u8". It improves readability since it's consistent with
the prototypes of readb() and writeb(). More importantly, it
reduces readers' confusion, since the upcoming 2D acceleration
code will use a different wordsize.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
drivers/video/fbdev/sm712.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/sm712.h b/drivers/video/fbdev/sm712.h
index bb144f038267..89e446db2ac7 100644
--- a/drivers/video/fbdev/sm712.h
+++ b/drivers/video/fbdev/sm712.h
@@ -59,19 +59,19 @@ extern void __iomem *smtc_regbaseaddress;
#define SIZE_CR30_CR4D (0x4D - 0x30 + 1)
#define SIZE_CR90_CRA7 (0xA7 - 0x90 + 1)
-static inline void smtc_crtcw(int reg, int val)
+static inline void smtc_crtcw(u8 reg, u8 val)
{
smtc_mmiowb(reg, 0x3d4);
smtc_mmiowb(val, 0x3d5);
}
-static inline void smtc_grphw(int reg, int val)
+static inline void smtc_grphw(u8 reg, u8 val)
{
smtc_mmiowb(reg, 0x3ce);
smtc_mmiowb(val, 0x3cf);
}
-static inline void smtc_attrw(int reg, int val)
+static inline void smtc_attrw(u8 reg, u8 val)
{
smtc_mmiorb(0x3da);
smtc_mmiowb(reg, 0x3c0);
@@ -79,13 +79,13 @@ static inline void smtc_attrw(int reg, int val)
smtc_mmiowb(val, 0x3c0);
}
-static inline void smtc_seqw(int reg, int val)
+static inline void smtc_seqw(u8 reg, u8 val)
{
smtc_mmiowb(reg, 0x3c4);
smtc_mmiowb(val, 0x3c5);
}
-static inline unsigned int smtc_seqr(int reg)
+static inline u8 smtc_seqr(u8 reg)
{
smtc_mmiowb(reg, 0x3c4);
return smtc_mmiorb(0x3c5);
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 1/7] fbdev: sm712fb: use type "u8" for 8-bit I/O.
@ 2019-03-22 5:17 ` Yifeng Li
0 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
This commit converts "unsigned int" and "int" in I/O wrappers
to "u8". It improves readability since it's consistent with
the prototypes of readb() and writeb(). More importantly, it
reduces readers' confusion, since the upcoming 2D acceleration
code will use a different wordsize.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
drivers/video/fbdev/sm712.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/sm712.h b/drivers/video/fbdev/sm712.h
index bb144f038267..89e446db2ac7 100644
--- a/drivers/video/fbdev/sm712.h
+++ b/drivers/video/fbdev/sm712.h
@@ -59,19 +59,19 @@ extern void __iomem *smtc_regbaseaddress;
#define SIZE_CR30_CR4D (0x4D - 0x30 + 1)
#define SIZE_CR90_CRA7 (0xA7 - 0x90 + 1)
-static inline void smtc_crtcw(int reg, int val)
+static inline void smtc_crtcw(u8 reg, u8 val)
{
smtc_mmiowb(reg, 0x3d4);
smtc_mmiowb(val, 0x3d5);
}
-static inline void smtc_grphw(int reg, int val)
+static inline void smtc_grphw(u8 reg, u8 val)
{
smtc_mmiowb(reg, 0x3ce);
smtc_mmiowb(val, 0x3cf);
}
-static inline void smtc_attrw(int reg, int val)
+static inline void smtc_attrw(u8 reg, u8 val)
{
smtc_mmiorb(0x3da);
smtc_mmiowb(reg, 0x3c0);
@@ -79,13 +79,13 @@ static inline void smtc_attrw(int reg, int val)
smtc_mmiowb(val, 0x3c0);
}
-static inline void smtc_seqw(int reg, int val)
+static inline void smtc_seqw(u8 reg, u8 val)
{
smtc_mmiowb(reg, 0x3c4);
smtc_mmiowb(val, 0x3c5);
}
-static inline unsigned int smtc_seqr(int reg)
+static inline u8 smtc_seqr(u8 reg)
{
smtc_mmiowb(reg, 0x3c4);
return smtc_mmiorb(0x3c5);
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 2/7] fbdev: sm712fb: add 2D-related I/O headers and functions.
2019-03-22 5:17 ` Yifeng Li
@ 2019-03-22 5:17 ` Yifeng Li
-1 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
This commit adds I/O macros and functions related to 2D opeartions.
A hunk of hardware register definitions are taken verbatim from
OpenBSD.
In addition, a utility function pad_to_dword() is added to help
padding data for the 2D engine. It accepts 3, 2, or 1 byte(s) of
data, and pads it to a 32-bit word suitable for 2D Drawing Engine.
Yes, we can set info->pixmap.scan_align/buf_align = 4 and forget
about padding, but it's incompatible with cfb_imageblit() w/
depth == 1. In case we need to fallback (e.g. debugging), it would
be inconvenient, so we pad it manually.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
drivers/video/fbdev/sm712.h | 96 +++++++++++++++++++++++++++++++++++++
1 file changed, 96 insertions(+)
diff --git a/drivers/video/fbdev/sm712.h b/drivers/video/fbdev/sm712.h
index 89e446db2ac7..4892fd485f08 100644
--- a/drivers/video/fbdev/sm712.h
+++ b/drivers/video/fbdev/sm712.h
@@ -91,6 +91,102 @@ static inline u8 smtc_seqr(u8 reg)
return smtc_mmiorb(0x3c5);
}
+/*
+ * DPR (2D drawing engine)
+ */
+#define DPR_COORDS(x, y) (((x) << 16) | (y))
+
+#define SCR_DE_STATUS 0x16
+#define SCR_DE_STATUS_MASK 0x18
+#define SCR_DE_ENGINE_IDLE 0x10
+
+#define DPR_BASE 0x00408000
+#define DPR_SRC_COORDS 0x00
+#define DPR_DST_COORDS 0x04
+#define DPR_SPAN_COORDS 0x08
+#define DPR_DE_CTRL 0x0c
+#define DPR_PITCH 0x10
+#define DPR_FG_COLOR 0x14
+#define DPR_BG_COLOR 0x18
+#define DPR_STRETCH 0x1c
+#define DPR_DE_FORMAT_SELECT 0x1e
+#define DPR_COLOR_COMPARE 0x20
+#define DPR_COLOR_COMPARE_MASK 0x24
+#define DPR_BYTE_BIT_MASK 0x28
+#define DPR_CROP_TOPLEFT_COORDS 0x2c
+#define DPR_CROP_BOTRIGHT_COORDS 0x30
+#define DPR_MONO_PATTERN_LO32 0x34
+#define DPR_MONO_PATTERN_HI32 0x38
+#define DPR_SRC_WINDOW 0x3c
+#define DPR_SRC_BASE 0x40
+#define DPR_DST_BASE 0x44
+
+#define DE_CTRL_START 0x80000000
+#define DE_CTRL_RTOL 0x08000000
+#define DE_CTRL_COMMAND_MASK 0x001f0000
+#define DE_CTRL_COMMAND_SHIFT 16
+#define DE_CTRL_COMMAND_BITBLT 0x00
+#define DE_CTRL_COMMAND_SOLIDFILL 0x01
+#define DE_CTRL_COMMAND_HOSTWRITE 0x08
+#define DE_CTRL_ROP2_SELECT 0x00008000
+#define DE_CTRL_ROP2_SRC_IS_PATTERN 0x00004000
+#define DE_CTRL_ROP2_SHIFT 0
+#define DE_CTRL_ROP2_COPY 0x0c
+#define DE_CTRL_HOST_SHIFT 22
+#define DE_CTRL_HOST_SRC_IS_MONO 0x01
+#define DE_CTRL_FORMAT_XY 0x00
+#define DE_CTRL_FORMAT_24BIT 0x30
+
+/*
+ * 32-bit I/O for 2D opeartions.
+ */
+extern void __iomem *smtc_dprbaseaddress; /* DPR, 2D control registers */
+
+static inline u8 smtc_dprr(u8 reg)
+{
+ return readl(smtc_dprbaseaddress + reg);
+}
+
+static inline void smtc_dprw(u8 reg, u32 val)
+{
+ writel(val, smtc_dprbaseaddress + reg);
+}
+
+static inline void smtc_dprw_16(u8 reg, u16 val)
+{
+ writew(val, smtc_dprbaseaddress + reg);
+}
+
+static inline u32 pad_to_dword(const u8 *bytes, int length)
+{
+ u32 dword = 0;
+
+ switch (length) {
+#ifdef __BIG_ENDIAN
+ case 3:
+ dword |= bytes[2] << 8;
+ /* fallthrough */
+ case 2:
+ dword |= bytes[1] << 16;
+ /* fallthrough */
+ case 1:
+ dword |= bytes[0] << 24;
+ break;
+#else
+ case 3:
+ dword |= bytes[2] << 16;
+ /* fallthrough */
+ case 2:
+ dword |= bytes[1] << 8;
+ /* fallthrough */
+ case 1:
+ dword |= bytes[0];
+ break;
+#endif
+ }
+ return dword;
+}
+
/* The next structure holds all information relevant for a specific video mode.
*/
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 2/7] fbdev: sm712fb: add 2D-related I/O headers and functions.
@ 2019-03-22 5:17 ` Yifeng Li
0 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
This commit adds I/O macros and functions related to 2D opeartions.
A hunk of hardware register definitions are taken verbatim from
OpenBSD.
In addition, a utility function pad_to_dword() is added to help
padding data for the 2D engine. It accepts 3, 2, or 1 byte(s) of
data, and pads it to a 32-bit word suitable for 2D Drawing Engine.
Yes, we can set info->pixmap.scan_align/buf_align = 4 and forget
about padding, but it's incompatible with cfb_imageblit() w/
depth = 1. In case we need to fallback (e.g. debugging), it would
be inconvenient, so we pad it manually.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
drivers/video/fbdev/sm712.h | 96 +++++++++++++++++++++++++++++++++++++
1 file changed, 96 insertions(+)
diff --git a/drivers/video/fbdev/sm712.h b/drivers/video/fbdev/sm712.h
index 89e446db2ac7..4892fd485f08 100644
--- a/drivers/video/fbdev/sm712.h
+++ b/drivers/video/fbdev/sm712.h
@@ -91,6 +91,102 @@ static inline u8 smtc_seqr(u8 reg)
return smtc_mmiorb(0x3c5);
}
+/*
+ * DPR (2D drawing engine)
+ */
+#define DPR_COORDS(x, y) (((x) << 16) | (y))
+
+#define SCR_DE_STATUS 0x16
+#define SCR_DE_STATUS_MASK 0x18
+#define SCR_DE_ENGINE_IDLE 0x10
+
+#define DPR_BASE 0x00408000
+#define DPR_SRC_COORDS 0x00
+#define DPR_DST_COORDS 0x04
+#define DPR_SPAN_COORDS 0x08
+#define DPR_DE_CTRL 0x0c
+#define DPR_PITCH 0x10
+#define DPR_FG_COLOR 0x14
+#define DPR_BG_COLOR 0x18
+#define DPR_STRETCH 0x1c
+#define DPR_DE_FORMAT_SELECT 0x1e
+#define DPR_COLOR_COMPARE 0x20
+#define DPR_COLOR_COMPARE_MASK 0x24
+#define DPR_BYTE_BIT_MASK 0x28
+#define DPR_CROP_TOPLEFT_COORDS 0x2c
+#define DPR_CROP_BOTRIGHT_COORDS 0x30
+#define DPR_MONO_PATTERN_LO32 0x34
+#define DPR_MONO_PATTERN_HI32 0x38
+#define DPR_SRC_WINDOW 0x3c
+#define DPR_SRC_BASE 0x40
+#define DPR_DST_BASE 0x44
+
+#define DE_CTRL_START 0x80000000
+#define DE_CTRL_RTOL 0x08000000
+#define DE_CTRL_COMMAND_MASK 0x001f0000
+#define DE_CTRL_COMMAND_SHIFT 16
+#define DE_CTRL_COMMAND_BITBLT 0x00
+#define DE_CTRL_COMMAND_SOLIDFILL 0x01
+#define DE_CTRL_COMMAND_HOSTWRITE 0x08
+#define DE_CTRL_ROP2_SELECT 0x00008000
+#define DE_CTRL_ROP2_SRC_IS_PATTERN 0x00004000
+#define DE_CTRL_ROP2_SHIFT 0
+#define DE_CTRL_ROP2_COPY 0x0c
+#define DE_CTRL_HOST_SHIFT 22
+#define DE_CTRL_HOST_SRC_IS_MONO 0x01
+#define DE_CTRL_FORMAT_XY 0x00
+#define DE_CTRL_FORMAT_24BIT 0x30
+
+/*
+ * 32-bit I/O for 2D opeartions.
+ */
+extern void __iomem *smtc_dprbaseaddress; /* DPR, 2D control registers */
+
+static inline u8 smtc_dprr(u8 reg)
+{
+ return readl(smtc_dprbaseaddress + reg);
+}
+
+static inline void smtc_dprw(u8 reg, u32 val)
+{
+ writel(val, smtc_dprbaseaddress + reg);
+}
+
+static inline void smtc_dprw_16(u8 reg, u16 val)
+{
+ writew(val, smtc_dprbaseaddress + reg);
+}
+
+static inline u32 pad_to_dword(const u8 *bytes, int length)
+{
+ u32 dword = 0;
+
+ switch (length) {
+#ifdef __BIG_ENDIAN
+ case 3:
+ dword |= bytes[2] << 8;
+ /* fallthrough */
+ case 2:
+ dword |= bytes[1] << 16;
+ /* fallthrough */
+ case 1:
+ dword |= bytes[0] << 24;
+ break;
+#else
+ case 3:
+ dword |= bytes[2] << 16;
+ /* fallthrough */
+ case 2:
+ dword |= bytes[1] << 8;
+ /* fallthrough */
+ case 1:
+ dword |= bytes[0];
+ break;
+#endif
+ }
+ return dword;
+}
+
/* The next structure holds all information relevant for a specific video mode.
*/
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 3/7] fbdev: sm712fb: support 2D acceleration on SM712 w/ Little-Endian CPU.
2019-03-22 5:17 ` Yifeng Li
@ 2019-03-22 5:17 ` Yifeng Li
-1 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
Previously, in staging/sm7xxfb (now fbdev/sm712fb), 2D acceleration
was implemented, but after its submission, a critical bug that causes
total system hang was discovered, as a stopgap measure, 2D ops was
completele removed in commit 3af805735a25 ("staging: sm7xx: remove the
buggy 2D acceleration support") and never implemented again.
It created a massive usability problem - on YeeLoong 8089, a notable
MIPS platform which uses SM712 - even scrolling a single line of text
on the console required an unaccelerated screen redraw, running "dmesg"
typically takes 8-11 seconds, and absurdly, printf(), became a significant
performance bottleneck that slows down GCC and "make", make the computer
largely unusable.
So I decided to take a look. Most of the my actual development was done
in 2014 in a personal out-of-tree driver, I did not mainline it because
2D acceleration was not working properly in 24-bit color. I discovered
the solution in early 2019 and now it's ready to be mainlined.
This commit reimplements the 2D acceleration for sm712fb. Unlike the
original implementation, which was messy and unnecessarily complicated
by calling a 2D acceleration wrapper file with many unneeded functions,
this is a minimum and (relatively) clean implementation. My tests have
shown that running "dmesg" only takes 0.9 seconds, a performance boost
of 950%. System hangs did not occur in my tests.
Currently, 2D acceleration is only supported on little-endian CPUs, it's
disabled on Big Endian systems as a safety measure, since I code for myself
without any monetary or hardware support from any company or OEMs, I don't
have the hardware and it's completely untested. I should be also to
purchase a Big Endian test platform and add proper support soon.
Finally, thanks to Miodrag Vallat and other OpenBSD developers, this
work would be impossible without their code, that served as a reference
implementation for me.
As a historical note, though I'm not extremely sure, but I believe the
source of the original system hang was a type punning.
unsigned char ajRemain[4];
SMTC_write2Ddataport(0, *(unsigned long *)ajRemain);
Type punning is undefined behavior in C, and here it caused an unaligned
memory access, which is illegal on MIPS, thus crashing the computer.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
drivers/video/fbdev/sm712.h | 3 +
drivers/video/fbdev/sm712fb.c | 393 +++++++++++++++++++++++++++++++++-
2 files changed, 389 insertions(+), 7 deletions(-)
diff --git a/drivers/video/fbdev/sm712.h b/drivers/video/fbdev/sm712.h
index 4892fd485f08..ad63676d3d4f 100644
--- a/drivers/video/fbdev/sm712.h
+++ b/drivers/video/fbdev/sm712.h
@@ -135,7 +135,10 @@ static inline u8 smtc_seqr(u8 reg)
#define DE_CTRL_HOST_SHIFT 22
#define DE_CTRL_HOST_SRC_IS_MONO 0x01
#define DE_CTRL_FORMAT_XY 0x00
+#define DE_CTRL_FORMAT_8BIT 0x00
+#define DE_CTRL_FORMAT_16BIT 0x10
#define DE_CTRL_FORMAT_24BIT 0x30
+#define DE_CTRL_FORMAT_32BIT 0x20
/*
* 32-bit I/O for 2D opeartions.
diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
index fef5b076589c..75d60ea63883 100644
--- a/drivers/video/fbdev/sm712fb.c
+++ b/drivers/video/fbdev/sm712fb.c
@@ -47,6 +47,7 @@
#include <linux/module.h>
#include <linux/console.h>
#include <linux/screen_info.h>
+#include <linux/delay.h>
#include <linux/pm.h>
@@ -62,11 +63,14 @@ struct smtcfb_info {
u8 chip_rev_id;
void __iomem *lfb; /* linear frame buffer */
+ void __iomem *dp_port; /* drawing processor data port */
void __iomem *dp_regs; /* drawing processor control regs */
void __iomem *vp_regs; /* video processor control regs */
void __iomem *cp_regs; /* capture processor control regs */
void __iomem *mmio; /* memory map IO port */
+ bool accel; /* whether to actually use drawing processor */
+
u_int width;
u_int height;
u_int hz;
@@ -75,6 +79,7 @@ struct smtcfb_info {
};
void __iomem *smtc_regbaseaddress; /* Memory Map IO starting address */
+void __iomem *smtc_dprbaseaddress; /* DPR, 2D control registers */
static const struct fb_var_screeninfo smtcfb_var = {
.xres = 1024,
@@ -848,14 +853,21 @@ static const struct modeinit vgamode[] = {
},
};
-static struct screen_info smtc_scr_info;
+/* prototypes of two cross-referenced functions */
+static void smtcfb_reset_accel(void);
+static int smtcfb_init_accel(struct smtcfb_info *fb);
+
+static struct screen_info smtc_scr_info;
static char *mode_option;
+static bool accel = true; /* can be ignored if not supported */
+static bool accel_status_reported;
-/* process command line options, get vga parameter */
+/* process command line options, get vga and accel parameter */
static void __init sm7xx_vga_setup(char *options)
{
int i;
+ char *this_opt;
if (!options || !*options)
return;
@@ -872,9 +884,20 @@ static void __init sm7xx_vga_setup(char *options)
smtc_scr_info.lfb_height =
vesa_mode_table[i].lfb_height;
smtc_scr_info.lfb_depth = vesa_mode_table[i].lfb_depth;
- return;
+ break;
}
}
+
+ while ((this_opt = strsep(&options, ",")) != NULL) {
+ if (!*this_opt)
+ continue;
+
+ if (!strcmp(this_opt, "accel:0"))
+ accel = false;
+ else if (!strcmp(this_opt, "accel:1"))
+ accel = true;
+ }
+ accel_status_reported = false;
}
static void sm712_setpalette(int regno, unsigned int red, unsigned int green,
@@ -1361,7 +1384,42 @@ static void smtcfb_setmode(struct smtcfb_info *sfb)
sfb->width = sfb->fb->var.xres;
sfb->height = sfb->fb->var.yres;
sfb->hz = 60;
+
+ /*
+ * We reset the 2D engine twice, once before the modesetting, once
+ * after the modesetting (mandatory), since users may chance the
+ * mode on-the-fly. Just be safe.
+ */
+ smtcfb_reset_accel();
+
smtc_set_timing(sfb);
+
+ /*
+ * Currently, 2D acceleration is only supported on SM712 with
+ * little-endian CPUs, it's disabled on Big Endian systems and SM720
+ * chips as a safety measure. Since I don't have monetary or hardware
+ * support from any company or OEMs, I don't have the hardware and
+ * it's completely untested. I should be also to purchase a Big Endian
+ * test platform and add proper support soon. I still have to spend
+ * 200 USD+ to purchase this piece of 1998's hardware, yikes! If you
+ * have a Big-Endian platform with SM7xx available for testing, please
+ * send an E-mail to Tom, thanks!
+ */
+#ifdef __BIG_ENDIAN
+ sfb->accel = false;
+ if (accel)
+ dev_info(&sfb->pdev->dev,
+ "2D acceleration is unsupported on Big Endian.\n");
+#endif
+ if (!accel) {
+ sfb->accel = false;
+ dev_info(&sfb->pdev->dev,
+ "2D acceleration is disabled by the user.\n");
+ }
+
+ /* reset 2D engine after a modesetting is mandatory */
+ smtcfb_reset_accel();
+ smtcfb_init_accel(sfb);
}
static int smtc_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
@@ -1401,6 +1459,316 @@ static struct fb_ops smtcfb_ops = {
.fb_write = smtcfb_write,
};
+static int smtcfb_wait(struct smtcfb_info *fb)
+{
+ int i;
+ u8 reg;
+
+ smtc_dprr(DPR_DE_CTRL);
+ for (i = 0; i < 10000; i++) {
+ reg = smtc_seqr(SCR_DE_STATUS);
+ if ((reg & SCR_DE_STATUS_MASK) == SCR_DE_ENGINE_IDLE)
+ return 0;
+ udelay(1);
+ }
+ dev_err(&fb->pdev->dev, "2D engine hang detected!\n");
+ return -EBUSY;
+}
+
+static void
+smtcfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
+{
+ u32 width = rect->width, height = rect->height;
+ u32 dx = rect->dx, dy = rect->dy;
+ u32 color;
+
+ struct smtcfb_info *sfb = info->par;
+
+ if (unlikely(info->state != FBINFO_STATE_RUNNING))
+ return;
+
+ if (unlikely(rect->rop != ROP_COPY)) {
+ /*
+ * It must be ROP_XOR. It's only used to combine a hardware
+ * cursor with the screen, and should never occur. Included
+ * for completeness. If one wants to implement hardware cursor
+ * (you don't, hardware only has RGB332 cursor), ROP2_XOR
+ * should be implemented here.
+ */
+ cfb_fillrect(info, rect);
+ return;
+ }
+
+ if ((rect->dx >= info->var.xres_virtual) ||
+ (rect->dy >= info->var.yres_virtual))
+ return;
+
+ if (info->fix.visual == FB_VISUAL_TRUECOLOR ||
+ info->fix.visual == FB_VISUAL_DIRECTCOLOR)
+ color = ((u32 *) (info->pseudo_palette))[rect->color];
+ else
+ color = rect->color;
+
+ if (sfb->fb->var.bits_per_pixel == 24) {
+ /*
+ * In 24-bit mode, all x, y coordinates and widths (but not
+ * height) must be multipiled by three.
+ */
+ dx *= 3;
+ dy *= 3;
+ width *= 3;
+
+ /*
+ * In 24-bit color mode, SOLIDFILL will sometimes put random
+ * color stripes of garbage on the screen, it seems to be a
+ * hardware bug. Alternatively, we initialize MONO_PATTERN_LOW
+ * & HIGH with 0xffffffff (all ones, and we have already set
+ * that in smtcfb_init_accel). Since the color of this mono
+ * pattern is controlled by DPR_FG_COLOR, BITBLTing it with
+ * ROP_COPY is effectively a rectfill().
+ */
+ smtc_dprw(DPR_FG_COLOR, color);
+ smtc_dprw(DPR_DST_COORDS, DPR_COORDS(dx, dy));
+ smtc_dprw(DPR_SPAN_COORDS, DPR_COORDS(width, height));
+ smtc_dprw(DPR_DE_CTRL, DE_CTRL_START | DE_CTRL_ROP2_SELECT |
+ DE_CTRL_ROP2_SRC_IS_PATTERN |
+ (DE_CTRL_COMMAND_BITBLT <<
+ DE_CTRL_COMMAND_SHIFT) |
+ (DE_CTRL_ROP2_COPY <<
+ DE_CTRL_ROP2_SHIFT));
+ } else {
+ smtc_dprw(DPR_FG_COLOR, color);
+ smtc_dprw(DPR_DST_COORDS, DPR_COORDS(dx, dy));
+ smtc_dprw(DPR_SPAN_COORDS, DPR_COORDS(width, height));
+ smtc_dprw(DPR_DE_CTRL, DE_CTRL_START | DE_CTRL_ROP2_SELECT |
+ (DE_CTRL_COMMAND_SOLIDFILL <<
+ DE_CTRL_COMMAND_SHIFT) |
+ (DE_CTRL_ROP2_COPY <<
+ DE_CTRL_ROP2_SHIFT));
+ }
+ smtcfb_wait(sfb);
+}
+
+static void
+smtcfb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
+{
+ u32 sx = area->sx, sy = area->sy;
+ u32 dx = area->dx, dy = area->dy;
+ u32 height = area->height, width = area->width;
+ u32 direction;
+
+ struct smtcfb_info *sfb = info->par;
+
+ if (unlikely(info->state != FBINFO_STATE_RUNNING))
+ return;
+ if ((sx >= info->var.xres_virtual) || (sy >= info->var.yres_virtual))
+ return;
+
+ if (sy < dy || (sy == dy && sx <= dx)) {
+ sx += width - 1;
+ dx += width - 1;
+ sy += height - 1;
+ dy += height - 1;
+ direction = DE_CTRL_RTOL;
+ } else {
+ direction = 0;
+ }
+
+ if (sfb->fb->var.bits_per_pixel == 24) {
+ sx *= 3;
+ sy *= 3;
+ dx *= 3;
+ dy *= 3;
+ width *= 3;
+ if (direction == DE_CTRL_RTOL) {
+ /*
+ * some hardware shenanigan from the original git
+ * commit, that is never clearly mentioned in the
+ * official datasheet. Not sure whether it even
+ * works correctly.
+ */
+ sx += 2;
+ dx += 2;
+ }
+ }
+
+ smtc_dprw(DPR_SRC_COORDS, DPR_COORDS(sx, sy));
+ smtc_dprw(DPR_DST_COORDS, DPR_COORDS(dx, dy));
+ smtc_dprw(DPR_SPAN_COORDS, DPR_COORDS(width, height));
+ smtc_dprw(DPR_DE_CTRL,
+ DE_CTRL_START | DE_CTRL_ROP2_SELECT | direction |
+ (DE_CTRL_COMMAND_BITBLT << DE_CTRL_COMMAND_SHIFT) |
+ (DE_CTRL_ROP2_COPY << DE_CTRL_ROP2_SHIFT));
+ smtcfb_wait(sfb);
+}
+
+static void
+smtcfb_imageblit(struct fb_info *info, const struct fb_image *image)
+{
+ u32 dx = image->dx, dy = image->dy;
+ u32 width = image->width, height = image->height;
+ u32 fg_color, bg_color;
+
+ u32 total_bytes, total_dwords, leftovers;
+ u32 i;
+ u32 idx = 0;
+ u32 scanline = image->width >> 3;
+
+ struct smtcfb_info *sfb = info->par;
+
+ if (unlikely(info->state != FBINFO_STATE_RUNNING))
+ return;
+ if ((image->dx >= info->var.xres_virtual) ||
+ (image->dy >= info->var.yres_virtual))
+ return;
+
+ if (unlikely(image->depth != 1)) {
+ /* unsupported depth, fallback to draw Tux */
+ cfb_imageblit(info, image);
+ return;
+ }
+
+ if (info->fix.visual == FB_VISUAL_TRUECOLOR ||
+ info->fix.visual == FB_VISUAL_DIRECTCOLOR) {
+ fg_color = ((u32 *) (info->pseudo_palette))[image->fg_color];
+ bg_color = ((u32 *) (info->pseudo_palette))[image->bg_color];
+ } else {
+ fg_color = image->fg_color;
+ bg_color = image->bg_color;
+ }
+
+ /* total bytes we need to write */
+ total_bytes = (width + 7) / 8;
+ total_dwords = (total_bytes & ~3) / 4;
+ leftovers = total_bytes & 3;
+
+ if (sfb->fb->var.bits_per_pixel == 24) {
+ dx *= 3;
+ dy *= 3;
+ width *= 3;
+ }
+ smtc_dprw(DPR_SRC_COORDS, 0);
+ smtc_dprw(DPR_DST_COORDS, DPR_COORDS(dx, dy));
+ smtc_dprw(DPR_SPAN_COORDS, DPR_COORDS(width, height));
+ smtc_dprw(DPR_FG_COLOR, fg_color);
+ smtc_dprw(DPR_BG_COLOR, bg_color);
+ smtc_dprw(DPR_DE_CTRL, DE_CTRL_START | DE_CTRL_ROP2_SELECT |
+ (DE_CTRL_COMMAND_HOSTWRITE << DE_CTRL_COMMAND_SHIFT) |
+ (DE_CTRL_HOST_SRC_IS_MONO << DE_CTRL_HOST_SHIFT) |
+ (DE_CTRL_ROP2_COPY << DE_CTRL_ROP2_SHIFT));
+
+ for (i = 0; i < height; i++) {
+ iowrite32_rep(sfb->dp_port, &image->data[idx], total_dwords);
+ if (leftovers) {
+ /*
+ * We can set info->pixmap.scan_align/buf_align = 4
+ * for automatic padding. But it would be sometimes
+ * incompatible with cfb_*(), especially imageblit()
+ * when depth = 1. In case we need to fallback (e.g.
+ * debugging), it would be inconvenient, so we pad it
+ * manually.
+ */
+ iowrite32(
+ pad_to_dword(
+ &image->data[idx + total_dwords * 4],
+ leftovers),
+ sfb->dp_port);
+ }
+ idx += scanline;
+ }
+ smtcfb_wait(sfb);
+}
+
+static void smtcfb_reset_accel(void)
+{
+ u8 reg;
+
+ /* enable Zoom Video Port, 2D Drawing Engine and Video Processor */
+ smtc_seqw(0x21, smtc_seqr(0x21) & 0xf8);
+
+ /* abort pending 2D Drawing Engine operations */
+ reg = smtc_seqr(0x15);
+ smtc_seqw(0x15, reg | 0x30);
+ smtc_seqw(0x15, reg);
+}
+
+/*
+ * Function smtcfb_reset_accel(); should be called before calling
+ * this function
+ */
+static int smtcfb_init_accel(struct smtcfb_info *fb)
+{
+
+ if (accel && !fb->accel) {
+ fb->fb->flags |= FBINFO_HWACCEL_NONE;
+ return 0;
+ } else if (!accel && !fb->accel) {
+ fb->fb->flags |= FBINFO_HWACCEL_DISABLED;
+ return 0;
+ }
+
+ if (smtcfb_wait(fb) != 0) {
+ fb->fb->flags |= FBINFO_HWACCEL_NONE;
+ dev_err(&fb->pdev->dev,
+ "2D acceleration initialization failed!\n");
+ fb->accel = false;
+ return -1;
+ }
+
+ smtc_dprw(DPR_CROP_TOPLEFT_COORDS, DPR_COORDS(0, 0));
+
+ /* same width for DPR_PITCH and DPR_SRC_WINDOW */
+ smtc_dprw(DPR_PITCH, DPR_COORDS(fb->fb->var.xres, fb->fb->var.xres));
+ smtc_dprw(DPR_SRC_WINDOW,
+ DPR_COORDS(fb->fb->var.xres, fb->fb->var.xres));
+
+ switch (fb->fb->var.bits_per_pixel) {
+ case 8:
+ smtc_dprw_16(DPR_DE_FORMAT_SELECT,
+ DE_CTRL_FORMAT_XY | DE_CTRL_FORMAT_8BIT);
+ break;
+ case 16:
+ smtc_dprw_16(DPR_DE_FORMAT_SELECT,
+ DE_CTRL_FORMAT_XY | DE_CTRL_FORMAT_16BIT);
+ break;
+ case 24:
+ smtc_dprw_16(DPR_DE_FORMAT_SELECT,
+ DE_CTRL_FORMAT_XY | DE_CTRL_FORMAT_24BIT);
+ smtc_dprw(DPR_PITCH,
+ DPR_COORDS(fb->fb->var.xres * 3,
+ fb->fb->var.xres * 3));
+ break;
+ case 32:
+ smtc_dprw_16(DPR_DE_FORMAT_SELECT,
+ DE_CTRL_FORMAT_XY | DE_CTRL_FORMAT_32BIT);
+ break;
+ }
+
+ smtc_dprw(DPR_BYTE_BIT_MASK, 0xffffffff);
+ smtc_dprw(DPR_COLOR_COMPARE_MASK, 0);
+ smtc_dprw(DPR_COLOR_COMPARE, 0);
+ smtc_dprw(DPR_SRC_BASE, 0);
+ smtc_dprw(DPR_DST_BASE, 0);
+ smtc_dprw(DPR_MONO_PATTERN_LO32, 0xffffffff);
+ smtc_dprw(DPR_MONO_PATTERN_HI32, 0xffffffff);
+ smtc_dprr(DPR_DST_BASE);
+
+ smtcfb_ops.fb_copyarea = smtcfb_copyarea;
+ smtcfb_ops.fb_fillrect = smtcfb_fillrect;
+ smtcfb_ops.fb_imageblit = smtcfb_imageblit;
+ fb->fb->flags |= FBINFO_HWACCEL_COPYAREA |
+ FBINFO_HWACCEL_FILLRECT |
+ FBINFO_HWACCEL_IMAGEBLIT |
+ FBINFO_READS_FAST;
+
+ /* don't spam the kernel log after each modesetting */
+ if (!accel_status_reported)
+ dev_info(&fb->pdev->dev, "2D acceleration is enabled.\n");
+ accel_status_reported = true;
+
+ return 0;
+}
+
/*
* Unmap in the memory mapped IO registers
*/
@@ -1599,10 +1967,14 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
goto failed_fb;
}
- sfb->mmio = (smtc_regbaseaddress =
- sfb->lfb + 0x00700000);
+ sfb->mmio = sfb->lfb + 0x00700000;
+ sfb->dp_port = sfb->lfb + 0x00400000;
sfb->dp_regs = sfb->lfb + 0x00408000;
sfb->vp_regs = sfb->lfb + 0x0040c000;
+
+ smtc_regbaseaddress = sfb->mmio;
+ smtc_dprbaseaddress = sfb->dp_regs;
+ sfb->accel = accel;
if (sfb->fb->var.bits_per_pixel == 32) {
sfb->lfb += big_addr;
dev_info(&pdev->dev, "sfb->lfb=%p\n", sfb->lfb);
@@ -1623,9 +1995,13 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
sfb->fb->fix.mmio_len = 0x00200000;
sfb->dp_regs = ioremap(mmio_base, 0x00200000 + smem_size);
sfb->lfb = sfb->dp_regs + 0x00200000;
- sfb->mmio = (smtc_regbaseaddress =
- sfb->dp_regs + 0x000c0000);
+ sfb->mmio = sfb->dp_regs + 0x000c0000;
sfb->vp_regs = sfb->dp_regs + 0x800;
+ sfb->dp_port = sfb->dp_regs + 0x00006000;
+
+ smtc_regbaseaddress = sfb->mmio;
+ smtc_dprbaseaddress = sfb->dp_regs;
+ sfb->accel = accel;
smtc_seqw(0x62, 0xff);
smtc_seqw(0x6a, 0x0d);
@@ -1807,6 +2183,9 @@ static void __exit sm712fb_exit(void)
module_exit(sm712fb_exit);
+module_param(accel, bool, 0444);
+MODULE_PARM_DESC(accel, "Use Acceleration (2D Drawing) Engine (default = 1)");
+
MODULE_AUTHOR("Siliconmotion ");
MODULE_DESCRIPTION("Framebuffer driver for SMI Graphic Cards");
MODULE_LICENSE("GPL");
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 3/7] fbdev: sm712fb: support 2D acceleration on SM712 w/ Little-Endian CPU.
@ 2019-03-22 5:17 ` Yifeng Li
0 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
Previously, in staging/sm7xxfb (now fbdev/sm712fb), 2D acceleration
was implemented, but after its submission, a critical bug that causes
total system hang was discovered, as a stopgap measure, 2D ops was
completele removed in commit 3af805735a25 ("staging: sm7xx: remove the
buggy 2D acceleration support") and never implemented again.
It created a massive usability problem - on YeeLoong 8089, a notable
MIPS platform which uses SM712 - even scrolling a single line of text
on the console required an unaccelerated screen redraw, running "dmesg"
typically takes 8-11 seconds, and absurdly, printf(), became a significant
performance bottleneck that slows down GCC and "make", make the computer
largely unusable.
So I decided to take a look. Most of the my actual development was done
in 2014 in a personal out-of-tree driver, I did not mainline it because
2D acceleration was not working properly in 24-bit color. I discovered
the solution in early 2019 and now it's ready to be mainlined.
This commit reimplements the 2D acceleration for sm712fb. Unlike the
original implementation, which was messy and unnecessarily complicated
by calling a 2D acceleration wrapper file with many unneeded functions,
this is a minimum and (relatively) clean implementation. My tests have
shown that running "dmesg" only takes 0.9 seconds, a performance boost
of 950%. System hangs did not occur in my tests.
Currently, 2D acceleration is only supported on little-endian CPUs, it's
disabled on Big Endian systems as a safety measure, since I code for myself
without any monetary or hardware support from any company or OEMs, I don't
have the hardware and it's completely untested. I should be also to
purchase a Big Endian test platform and add proper support soon.
Finally, thanks to Miodrag Vallat and other OpenBSD developers, this
work would be impossible without their code, that served as a reference
implementation for me.
As a historical note, though I'm not extremely sure, but I believe the
source of the original system hang was a type punning.
unsigned char ajRemain[4];
SMTC_write2Ddataport(0, *(unsigned long *)ajRemain);
Type punning is undefined behavior in C, and here it caused an unaligned
memory access, which is illegal on MIPS, thus crashing the computer.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
drivers/video/fbdev/sm712.h | 3 +
drivers/video/fbdev/sm712fb.c | 393 +++++++++++++++++++++++++++++++++-
2 files changed, 389 insertions(+), 7 deletions(-)
diff --git a/drivers/video/fbdev/sm712.h b/drivers/video/fbdev/sm712.h
index 4892fd485f08..ad63676d3d4f 100644
--- a/drivers/video/fbdev/sm712.h
+++ b/drivers/video/fbdev/sm712.h
@@ -135,7 +135,10 @@ static inline u8 smtc_seqr(u8 reg)
#define DE_CTRL_HOST_SHIFT 22
#define DE_CTRL_HOST_SRC_IS_MONO 0x01
#define DE_CTRL_FORMAT_XY 0x00
+#define DE_CTRL_FORMAT_8BIT 0x00
+#define DE_CTRL_FORMAT_16BIT 0x10
#define DE_CTRL_FORMAT_24BIT 0x30
+#define DE_CTRL_FORMAT_32BIT 0x20
/*
* 32-bit I/O for 2D opeartions.
diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
index fef5b076589c..75d60ea63883 100644
--- a/drivers/video/fbdev/sm712fb.c
+++ b/drivers/video/fbdev/sm712fb.c
@@ -47,6 +47,7 @@
#include <linux/module.h>
#include <linux/console.h>
#include <linux/screen_info.h>
+#include <linux/delay.h>
#include <linux/pm.h>
@@ -62,11 +63,14 @@ struct smtcfb_info {
u8 chip_rev_id;
void __iomem *lfb; /* linear frame buffer */
+ void __iomem *dp_port; /* drawing processor data port */
void __iomem *dp_regs; /* drawing processor control regs */
void __iomem *vp_regs; /* video processor control regs */
void __iomem *cp_regs; /* capture processor control regs */
void __iomem *mmio; /* memory map IO port */
+ bool accel; /* whether to actually use drawing processor */
+
u_int width;
u_int height;
u_int hz;
@@ -75,6 +79,7 @@ struct smtcfb_info {
};
void __iomem *smtc_regbaseaddress; /* Memory Map IO starting address */
+void __iomem *smtc_dprbaseaddress; /* DPR, 2D control registers */
static const struct fb_var_screeninfo smtcfb_var = {
.xres = 1024,
@@ -848,14 +853,21 @@ static const struct modeinit vgamode[] = {
},
};
-static struct screen_info smtc_scr_info;
+/* prototypes of two cross-referenced functions */
+static void smtcfb_reset_accel(void);
+static int smtcfb_init_accel(struct smtcfb_info *fb);
+
+static struct screen_info smtc_scr_info;
static char *mode_option;
+static bool accel = true; /* can be ignored if not supported */
+static bool accel_status_reported;
-/* process command line options, get vga parameter */
+/* process command line options, get vga and accel parameter */
static void __init sm7xx_vga_setup(char *options)
{
int i;
+ char *this_opt;
if (!options || !*options)
return;
@@ -872,9 +884,20 @@ static void __init sm7xx_vga_setup(char *options)
smtc_scr_info.lfb_height vesa_mode_table[i].lfb_height;
smtc_scr_info.lfb_depth = vesa_mode_table[i].lfb_depth;
- return;
+ break;
}
}
+
+ while ((this_opt = strsep(&options, ",")) != NULL) {
+ if (!*this_opt)
+ continue;
+
+ if (!strcmp(this_opt, "accel:0"))
+ accel = false;
+ else if (!strcmp(this_opt, "accel:1"))
+ accel = true;
+ }
+ accel_status_reported = false;
}
static void sm712_setpalette(int regno, unsigned int red, unsigned int green,
@@ -1361,7 +1384,42 @@ static void smtcfb_setmode(struct smtcfb_info *sfb)
sfb->width = sfb->fb->var.xres;
sfb->height = sfb->fb->var.yres;
sfb->hz = 60;
+
+ /*
+ * We reset the 2D engine twice, once before the modesetting, once
+ * after the modesetting (mandatory), since users may chance the
+ * mode on-the-fly. Just be safe.
+ */
+ smtcfb_reset_accel();
+
smtc_set_timing(sfb);
+
+ /*
+ * Currently, 2D acceleration is only supported on SM712 with
+ * little-endian CPUs, it's disabled on Big Endian systems and SM720
+ * chips as a safety measure. Since I don't have monetary or hardware
+ * support from any company or OEMs, I don't have the hardware and
+ * it's completely untested. I should be also to purchase a Big Endian
+ * test platform and add proper support soon. I still have to spend
+ * 200 USD+ to purchase this piece of 1998's hardware, yikes! If you
+ * have a Big-Endian platform with SM7xx available for testing, please
+ * send an E-mail to Tom, thanks!
+ */
+#ifdef __BIG_ENDIAN
+ sfb->accel = false;
+ if (accel)
+ dev_info(&sfb->pdev->dev,
+ "2D acceleration is unsupported on Big Endian.\n");
+#endif
+ if (!accel) {
+ sfb->accel = false;
+ dev_info(&sfb->pdev->dev,
+ "2D acceleration is disabled by the user.\n");
+ }
+
+ /* reset 2D engine after a modesetting is mandatory */
+ smtcfb_reset_accel();
+ smtcfb_init_accel(sfb);
}
static int smtc_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
@@ -1401,6 +1459,316 @@ static struct fb_ops smtcfb_ops = {
.fb_write = smtcfb_write,
};
+static int smtcfb_wait(struct smtcfb_info *fb)
+{
+ int i;
+ u8 reg;
+
+ smtc_dprr(DPR_DE_CTRL);
+ for (i = 0; i < 10000; i++) {
+ reg = smtc_seqr(SCR_DE_STATUS);
+ if ((reg & SCR_DE_STATUS_MASK) = SCR_DE_ENGINE_IDLE)
+ return 0;
+ udelay(1);
+ }
+ dev_err(&fb->pdev->dev, "2D engine hang detected!\n");
+ return -EBUSY;
+}
+
+static void
+smtcfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
+{
+ u32 width = rect->width, height = rect->height;
+ u32 dx = rect->dx, dy = rect->dy;
+ u32 color;
+
+ struct smtcfb_info *sfb = info->par;
+
+ if (unlikely(info->state != FBINFO_STATE_RUNNING))
+ return;
+
+ if (unlikely(rect->rop != ROP_COPY)) {
+ /*
+ * It must be ROP_XOR. It's only used to combine a hardware
+ * cursor with the screen, and should never occur. Included
+ * for completeness. If one wants to implement hardware cursor
+ * (you don't, hardware only has RGB332 cursor), ROP2_XOR
+ * should be implemented here.
+ */
+ cfb_fillrect(info, rect);
+ return;
+ }
+
+ if ((rect->dx >= info->var.xres_virtual) ||
+ (rect->dy >= info->var.yres_virtual))
+ return;
+
+ if (info->fix.visual = FB_VISUAL_TRUECOLOR ||
+ info->fix.visual = FB_VISUAL_DIRECTCOLOR)
+ color = ((u32 *) (info->pseudo_palette))[rect->color];
+ else
+ color = rect->color;
+
+ if (sfb->fb->var.bits_per_pixel = 24) {
+ /*
+ * In 24-bit mode, all x, y coordinates and widths (but not
+ * height) must be multipiled by three.
+ */
+ dx *= 3;
+ dy *= 3;
+ width *= 3;
+
+ /*
+ * In 24-bit color mode, SOLIDFILL will sometimes put random
+ * color stripes of garbage on the screen, it seems to be a
+ * hardware bug. Alternatively, we initialize MONO_PATTERN_LOW
+ * & HIGH with 0xffffffff (all ones, and we have already set
+ * that in smtcfb_init_accel). Since the color of this mono
+ * pattern is controlled by DPR_FG_COLOR, BITBLTing it with
+ * ROP_COPY is effectively a rectfill().
+ */
+ smtc_dprw(DPR_FG_COLOR, color);
+ smtc_dprw(DPR_DST_COORDS, DPR_COORDS(dx, dy));
+ smtc_dprw(DPR_SPAN_COORDS, DPR_COORDS(width, height));
+ smtc_dprw(DPR_DE_CTRL, DE_CTRL_START | DE_CTRL_ROP2_SELECT |
+ DE_CTRL_ROP2_SRC_IS_PATTERN |
+ (DE_CTRL_COMMAND_BITBLT <<
+ DE_CTRL_COMMAND_SHIFT) |
+ (DE_CTRL_ROP2_COPY <<
+ DE_CTRL_ROP2_SHIFT));
+ } else {
+ smtc_dprw(DPR_FG_COLOR, color);
+ smtc_dprw(DPR_DST_COORDS, DPR_COORDS(dx, dy));
+ smtc_dprw(DPR_SPAN_COORDS, DPR_COORDS(width, height));
+ smtc_dprw(DPR_DE_CTRL, DE_CTRL_START | DE_CTRL_ROP2_SELECT |
+ (DE_CTRL_COMMAND_SOLIDFILL <<
+ DE_CTRL_COMMAND_SHIFT) |
+ (DE_CTRL_ROP2_COPY <<
+ DE_CTRL_ROP2_SHIFT));
+ }
+ smtcfb_wait(sfb);
+}
+
+static void
+smtcfb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
+{
+ u32 sx = area->sx, sy = area->sy;
+ u32 dx = area->dx, dy = area->dy;
+ u32 height = area->height, width = area->width;
+ u32 direction;
+
+ struct smtcfb_info *sfb = info->par;
+
+ if (unlikely(info->state != FBINFO_STATE_RUNNING))
+ return;
+ if ((sx >= info->var.xres_virtual) || (sy >= info->var.yres_virtual))
+ return;
+
+ if (sy < dy || (sy = dy && sx <= dx)) {
+ sx += width - 1;
+ dx += width - 1;
+ sy += height - 1;
+ dy += height - 1;
+ direction = DE_CTRL_RTOL;
+ } else {
+ direction = 0;
+ }
+
+ if (sfb->fb->var.bits_per_pixel = 24) {
+ sx *= 3;
+ sy *= 3;
+ dx *= 3;
+ dy *= 3;
+ width *= 3;
+ if (direction = DE_CTRL_RTOL) {
+ /*
+ * some hardware shenanigan from the original git
+ * commit, that is never clearly mentioned in the
+ * official datasheet. Not sure whether it even
+ * works correctly.
+ */
+ sx += 2;
+ dx += 2;
+ }
+ }
+
+ smtc_dprw(DPR_SRC_COORDS, DPR_COORDS(sx, sy));
+ smtc_dprw(DPR_DST_COORDS, DPR_COORDS(dx, dy));
+ smtc_dprw(DPR_SPAN_COORDS, DPR_COORDS(width, height));
+ smtc_dprw(DPR_DE_CTRL,
+ DE_CTRL_START | DE_CTRL_ROP2_SELECT | direction |
+ (DE_CTRL_COMMAND_BITBLT << DE_CTRL_COMMAND_SHIFT) |
+ (DE_CTRL_ROP2_COPY << DE_CTRL_ROP2_SHIFT));
+ smtcfb_wait(sfb);
+}
+
+static void
+smtcfb_imageblit(struct fb_info *info, const struct fb_image *image)
+{
+ u32 dx = image->dx, dy = image->dy;
+ u32 width = image->width, height = image->height;
+ u32 fg_color, bg_color;
+
+ u32 total_bytes, total_dwords, leftovers;
+ u32 i;
+ u32 idx = 0;
+ u32 scanline = image->width >> 3;
+
+ struct smtcfb_info *sfb = info->par;
+
+ if (unlikely(info->state != FBINFO_STATE_RUNNING))
+ return;
+ if ((image->dx >= info->var.xres_virtual) ||
+ (image->dy >= info->var.yres_virtual))
+ return;
+
+ if (unlikely(image->depth != 1)) {
+ /* unsupported depth, fallback to draw Tux */
+ cfb_imageblit(info, image);
+ return;
+ }
+
+ if (info->fix.visual = FB_VISUAL_TRUECOLOR ||
+ info->fix.visual = FB_VISUAL_DIRECTCOLOR) {
+ fg_color = ((u32 *) (info->pseudo_palette))[image->fg_color];
+ bg_color = ((u32 *) (info->pseudo_palette))[image->bg_color];
+ } else {
+ fg_color = image->fg_color;
+ bg_color = image->bg_color;
+ }
+
+ /* total bytes we need to write */
+ total_bytes = (width + 7) / 8;
+ total_dwords = (total_bytes & ~3) / 4;
+ leftovers = total_bytes & 3;
+
+ if (sfb->fb->var.bits_per_pixel = 24) {
+ dx *= 3;
+ dy *= 3;
+ width *= 3;
+ }
+ smtc_dprw(DPR_SRC_COORDS, 0);
+ smtc_dprw(DPR_DST_COORDS, DPR_COORDS(dx, dy));
+ smtc_dprw(DPR_SPAN_COORDS, DPR_COORDS(width, height));
+ smtc_dprw(DPR_FG_COLOR, fg_color);
+ smtc_dprw(DPR_BG_COLOR, bg_color);
+ smtc_dprw(DPR_DE_CTRL, DE_CTRL_START | DE_CTRL_ROP2_SELECT |
+ (DE_CTRL_COMMAND_HOSTWRITE << DE_CTRL_COMMAND_SHIFT) |
+ (DE_CTRL_HOST_SRC_IS_MONO << DE_CTRL_HOST_SHIFT) |
+ (DE_CTRL_ROP2_COPY << DE_CTRL_ROP2_SHIFT));
+
+ for (i = 0; i < height; i++) {
+ iowrite32_rep(sfb->dp_port, &image->data[idx], total_dwords);
+ if (leftovers) {
+ /*
+ * We can set info->pixmap.scan_align/buf_align = 4
+ * for automatic padding. But it would be sometimes
+ * incompatible with cfb_*(), especially imageblit()
+ * when depth = 1. In case we need to fallback (e.g.
+ * debugging), it would be inconvenient, so we pad it
+ * manually.
+ */
+ iowrite32(
+ pad_to_dword(
+ &image->data[idx + total_dwords * 4],
+ leftovers),
+ sfb->dp_port);
+ }
+ idx += scanline;
+ }
+ smtcfb_wait(sfb);
+}
+
+static void smtcfb_reset_accel(void)
+{
+ u8 reg;
+
+ /* enable Zoom Video Port, 2D Drawing Engine and Video Processor */
+ smtc_seqw(0x21, smtc_seqr(0x21) & 0xf8);
+
+ /* abort pending 2D Drawing Engine operations */
+ reg = smtc_seqr(0x15);
+ smtc_seqw(0x15, reg | 0x30);
+ smtc_seqw(0x15, reg);
+}
+
+/*
+ * Function smtcfb_reset_accel(); should be called before calling
+ * this function
+ */
+static int smtcfb_init_accel(struct smtcfb_info *fb)
+{
+
+ if (accel && !fb->accel) {
+ fb->fb->flags |= FBINFO_HWACCEL_NONE;
+ return 0;
+ } else if (!accel && !fb->accel) {
+ fb->fb->flags |= FBINFO_HWACCEL_DISABLED;
+ return 0;
+ }
+
+ if (smtcfb_wait(fb) != 0) {
+ fb->fb->flags |= FBINFO_HWACCEL_NONE;
+ dev_err(&fb->pdev->dev,
+ "2D acceleration initialization failed!\n");
+ fb->accel = false;
+ return -1;
+ }
+
+ smtc_dprw(DPR_CROP_TOPLEFT_COORDS, DPR_COORDS(0, 0));
+
+ /* same width for DPR_PITCH and DPR_SRC_WINDOW */
+ smtc_dprw(DPR_PITCH, DPR_COORDS(fb->fb->var.xres, fb->fb->var.xres));
+ smtc_dprw(DPR_SRC_WINDOW,
+ DPR_COORDS(fb->fb->var.xres, fb->fb->var.xres));
+
+ switch (fb->fb->var.bits_per_pixel) {
+ case 8:
+ smtc_dprw_16(DPR_DE_FORMAT_SELECT,
+ DE_CTRL_FORMAT_XY | DE_CTRL_FORMAT_8BIT);
+ break;
+ case 16:
+ smtc_dprw_16(DPR_DE_FORMAT_SELECT,
+ DE_CTRL_FORMAT_XY | DE_CTRL_FORMAT_16BIT);
+ break;
+ case 24:
+ smtc_dprw_16(DPR_DE_FORMAT_SELECT,
+ DE_CTRL_FORMAT_XY | DE_CTRL_FORMAT_24BIT);
+ smtc_dprw(DPR_PITCH,
+ DPR_COORDS(fb->fb->var.xres * 3,
+ fb->fb->var.xres * 3));
+ break;
+ case 32:
+ smtc_dprw_16(DPR_DE_FORMAT_SELECT,
+ DE_CTRL_FORMAT_XY | DE_CTRL_FORMAT_32BIT);
+ break;
+ }
+
+ smtc_dprw(DPR_BYTE_BIT_MASK, 0xffffffff);
+ smtc_dprw(DPR_COLOR_COMPARE_MASK, 0);
+ smtc_dprw(DPR_COLOR_COMPARE, 0);
+ smtc_dprw(DPR_SRC_BASE, 0);
+ smtc_dprw(DPR_DST_BASE, 0);
+ smtc_dprw(DPR_MONO_PATTERN_LO32, 0xffffffff);
+ smtc_dprw(DPR_MONO_PATTERN_HI32, 0xffffffff);
+ smtc_dprr(DPR_DST_BASE);
+
+ smtcfb_ops.fb_copyarea = smtcfb_copyarea;
+ smtcfb_ops.fb_fillrect = smtcfb_fillrect;
+ smtcfb_ops.fb_imageblit = smtcfb_imageblit;
+ fb->fb->flags |= FBINFO_HWACCEL_COPYAREA |
+ FBINFO_HWACCEL_FILLRECT |
+ FBINFO_HWACCEL_IMAGEBLIT |
+ FBINFO_READS_FAST;
+
+ /* don't spam the kernel log after each modesetting */
+ if (!accel_status_reported)
+ dev_info(&fb->pdev->dev, "2D acceleration is enabled.\n");
+ accel_status_reported = true;
+
+ return 0;
+}
+
/*
* Unmap in the memory mapped IO registers
*/
@@ -1599,10 +1967,14 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
goto failed_fb;
}
- sfb->mmio = (smtc_regbaseaddress - sfb->lfb + 0x00700000);
+ sfb->mmio = sfb->lfb + 0x00700000;
+ sfb->dp_port = sfb->lfb + 0x00400000;
sfb->dp_regs = sfb->lfb + 0x00408000;
sfb->vp_regs = sfb->lfb + 0x0040c000;
+
+ smtc_regbaseaddress = sfb->mmio;
+ smtc_dprbaseaddress = sfb->dp_regs;
+ sfb->accel = accel;
if (sfb->fb->var.bits_per_pixel = 32) {
sfb->lfb += big_addr;
dev_info(&pdev->dev, "sfb->lfb=%p\n", sfb->lfb);
@@ -1623,9 +1995,13 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
sfb->fb->fix.mmio_len = 0x00200000;
sfb->dp_regs = ioremap(mmio_base, 0x00200000 + smem_size);
sfb->lfb = sfb->dp_regs + 0x00200000;
- sfb->mmio = (smtc_regbaseaddress - sfb->dp_regs + 0x000c0000);
+ sfb->mmio = sfb->dp_regs + 0x000c0000;
sfb->vp_regs = sfb->dp_regs + 0x800;
+ sfb->dp_port = sfb->dp_regs + 0x00006000;
+
+ smtc_regbaseaddress = sfb->mmio;
+ smtc_dprbaseaddress = sfb->dp_regs;
+ sfb->accel = accel;
smtc_seqw(0x62, 0xff);
smtc_seqw(0x6a, 0x0d);
@@ -1807,6 +2183,9 @@ static void __exit sm712fb_exit(void)
module_exit(sm712fb_exit);
+module_param(accel, bool, 0444);
+MODULE_PARM_DESC(accel, "Use Acceleration (2D Drawing) Engine (default = 1)");
+
MODULE_AUTHOR("Siliconmotion ");
MODULE_DESCRIPTION("Framebuffer driver for SMI Graphic Cards");
MODULE_LICENSE("GPL");
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 4/7] fbdev: sm712fb: add 32-bit color modes, drops some other modes.
2019-03-22 5:17 ` Yifeng Li
@ 2019-03-22 5:17 ` Yifeng Li
-1 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
The modesetting in sm712fb is an ugly hack. First, all the registers
are programmed by hardcoded register arrays, which makes it difficult
to support different variations of color depths, refresh rates, CRT/LCD
panel, etc of the same resolution. Second, it means the standard
fb_find_mode() cannot be used and a confusing non-standard "vga="
parameter is needed. Third, there's only minimum differences between
some modes, yet around 70 lines of code and 100 registers are needed to
be indepentently specified for each mode. Fourth, the register between
some modes are inconsistent: the register configuration of different
color depths in 640 x 480 modes are identical, but for 800 x 600 modes
it's completely different. Also, some modes can drive the LCD panel
properly yet some other modes will only show a white screen of death on
the LCD panel. Fifth, some modes, such as 32-bit color modes, are
supported but never listed, and some modes are listed, such as 1280x1024
modes, but never supported by the register configuration arrays. And
some modes are partially implemented but neither listed nor supported,
i.e. the 8-bit color modes.
Fixing these problems requires a complete rewrite of modesetting code,
which is well-beyond my motivation. This commit only perform a minimum
cleanup:
1. Remove the 320 x 240 register settings, since there are never listed
and never actually being used. This resolution is not even supported by
vesafb, so it's safe to assume that no one is using such hardware. Future
developers who needs them can simply recover them from the git history.
2. Remove 1280x1024 modes and 8-bit color modes. They are listed but never
supported by the register array. So it doesn't work in the beginning, and
only gives a black screen.
3. Add 32-bit color modes. They are supported but never listed, and are
useful to some users.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
drivers/video/fbdev/sm712fb.c | 158 +++++++---------------------------
1 file changed, 31 insertions(+), 127 deletions(-)
diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
index 75d60ea63883..5e887653ef5d 100644
--- a/drivers/video/fbdev/sm712fb.c
+++ b/drivers/video/fbdev/sm712fb.c
@@ -118,25 +118,48 @@ struct vesa_mode {
};
static const struct vesa_mode vesa_mode_table[] = {
- {"0x301", 640, 480, 8},
- {"0x303", 800, 600, 8},
- {"0x305", 1024, 768, 8},
- {"0x307", 1280, 1024, 8},
-
{"0x311", 640, 480, 16},
{"0x314", 800, 600, 16},
{"0x317", 1024, 768, 16},
- {"0x31A", 1280, 1024, 16},
{"0x312", 640, 480, 24},
{"0x315", 800, 600, 24},
{"0x318", 1024, 768, 24},
- {"0x31B", 1280, 1024, 24},
+
+ {"0x329", 640, 480, 32},
+ {"0x32e", 800, 600, 32},
+ {"0x338", 1024, 768, 32},
};
/**********************************************************************
SM712 Mode table.
- **********************************************************************/
+
+ The modesetting in sm712fb is an ugly hack. First, all the registers
+ are programmed by hardcoded register arrays, which makes it difficult
+ to support different variations of color depths, refresh rates, CRT/LCD
+ panel, etc of the same resolution. Second, it means the standard
+ fb_find_mode() cannot be used and a confusing non-standard "vga="
+ parameter is needed. Third, there's only minimum differences between
+ some modes, yet around 70 lines of code and 100 registers are needed to
+ be indepentently specified for each mode. Fourth, the register between
+ some modes are inconsistent: the register configuration of different
+ color depths in 640 x 480 modes are identical, but for 800 x 600 modes
+ it's completely different. Also, some modes can drive the LCD panel
+ properly yet some other modes will only show a white screen of death on
+ the LCD panel. Fifth, there is a specific hack for Lemote Loongson 8089D
+ laptop, the 1024x768, 16-bit color mode was modified to drive its LCD panel
+ and changed to 1024x600, but the original mode was not preserved, so
+ 1024x768 16-bit color mode is completely unsupported. And previously,
+ some modes are listed, such as 1280x1024 modes, but never supported by
+ the register configuration arrays, so they are now removed. And some modes
+ are partially implemented but neither listed nor supported, i.e. the 8-bit
+ color modes, so they have been removed from vesa_mode_table, too.
+
+ I'm not the original author of the code, fixing these problems requires a
+ complete rewrite of modesetting code, which is well-beyond my motivation.
+
+ See Documentation/fb/sm712fb.txt for more information.
+**********************************************************************/
static const struct modeinit vgamode[] = {
{
/* mode#0: 640 x 480 16Bpp 60Hz */
@@ -732,125 +755,6 @@ static const struct modeinit vgamode[] = {
0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x15, 0x03,
},
},
- { /* mode#6: 320 x 240 16Bpp 60Hz */
- 320, 240, 16, 60,
- /* Init_MISC */
- 0xEB,
- { /* Init_SR0_SR4 */
- 0x03, 0x01, 0x0F, 0x03, 0x0E,
- },
- { /* Init_SR10_SR24 */
- 0xF3, 0xB6, 0xC0, 0xDD, 0x00, 0x0E, 0x17, 0x2C,
- 0x99, 0x02, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0xC4, 0x32, 0x02, 0x01, 0x01,
- },
- { /* Init_SR30_SR75 */
- 0x38, 0x03, 0x20, 0x09, 0xC0, 0x3A, 0x3A, 0x3A,
- 0x3A, 0x3A, 0x3A, 0x3A, 0x00, 0x00, 0x03, 0xFF,
- 0x00, 0xFC, 0x00, 0x00, 0x20, 0x18, 0x00, 0xFC,
- 0x20, 0x0C, 0x44, 0x20, 0x00, 0x00, 0x00, 0x3A,
- 0x06, 0x68, 0xA7, 0x7F, 0x83, 0x24, 0xFF, 0x03,
- 0x00, 0x60, 0x59, 0x3A, 0x3A, 0x00, 0x00, 0x3A,
- 0x01, 0x80, 0x7E, 0x1A, 0x1A, 0x00, 0x00, 0x00,
- 0x50, 0x03, 0x74, 0x14, 0x08, 0x43, 0x08, 0x43,
- 0x04, 0x45, 0x30, 0x30, 0x40, 0x20,
- },
- { /* Init_SR80_SR93 */
- 0xFF, 0x07, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0x3A,
- 0xF7, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0x3A, 0x3A,
- 0x00, 0x00, 0x00, 0x00,
- },
- { /* Init_SRA0_SRAF */
- 0x00, 0xFB, 0x9F, 0x01, 0x00, 0xED, 0xED, 0xED,
- 0x7B, 0xFB, 0xFF, 0xFF, 0x97, 0xEF, 0xBF, 0xDF,
- },
- { /* Init_GR00_GR08 */
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x05, 0x0F,
- 0xFF,
- },
- { /* Init_AR00_AR14 */
- 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
- 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
- 0x41, 0x00, 0x0F, 0x00, 0x00,
- },
- { /* Init_CR00_CR18 */
- 0xA3, 0x7F, 0x7F, 0x00, 0x85, 0x16, 0x24, 0xF5,
- 0x00, 0x60, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x03, 0x09, 0xFF, 0x80, 0x40, 0xFF, 0x00, 0xE3,
- 0xFF,
- },
- { /* Init_CR30_CR4D */
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, 0x02, 0x20,
- 0x00, 0x00, 0x30, 0x40, 0x00, 0xFF, 0xBF, 0xFF,
- 0x2E, 0x27, 0x00, 0x2b, 0x0c, 0x0F, 0xEF, 0x00,
- 0xFe, 0x0f, 0x01, 0xC0, 0x27, 0xEF,
- },
- { /* Init_CR90_CRA7 */
- 0x55, 0xD9, 0x5D, 0xE1, 0x86, 0x1B, 0x8E, 0x26,
- 0xDA, 0x8D, 0xDE, 0x94, 0x00, 0x00, 0x18, 0x00,
- 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x15, 0x03,
- },
- },
-
- { /* mode#8: 320 x 240 32Bpp 60Hz */
- 320, 240, 32, 60,
- /* Init_MISC */
- 0xEB,
- { /* Init_SR0_SR4 */
- 0x03, 0x01, 0x0F, 0x03, 0x0E,
- },
- { /* Init_SR10_SR24 */
- 0xF3, 0xB6, 0xC0, 0xDD, 0x00, 0x0E, 0x17, 0x2C,
- 0x99, 0x02, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0xC4, 0x32, 0x02, 0x01, 0x01,
- },
- { /* Init_SR30_SR75 */
- 0x38, 0x03, 0x20, 0x09, 0xC0, 0x3A, 0x3A, 0x3A,
- 0x3A, 0x3A, 0x3A, 0x3A, 0x00, 0x00, 0x03, 0xFF,
- 0x00, 0xFC, 0x00, 0x00, 0x20, 0x18, 0x00, 0xFC,
- 0x20, 0x0C, 0x44, 0x20, 0x00, 0x00, 0x00, 0x3A,
- 0x06, 0x68, 0xA7, 0x7F, 0x83, 0x24, 0xFF, 0x03,
- 0x00, 0x60, 0x59, 0x3A, 0x3A, 0x00, 0x00, 0x3A,
- 0x01, 0x80, 0x7E, 0x1A, 0x1A, 0x00, 0x00, 0x00,
- 0x50, 0x03, 0x74, 0x14, 0x08, 0x43, 0x08, 0x43,
- 0x04, 0x45, 0x30, 0x30, 0x40, 0x20,
- },
- { /* Init_SR80_SR93 */
- 0xFF, 0x07, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0x3A,
- 0xF7, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0x3A, 0x3A,
- 0x00, 0x00, 0x00, 0x00,
- },
- { /* Init_SRA0_SRAF */
- 0x00, 0xFB, 0x9F, 0x01, 0x00, 0xED, 0xED, 0xED,
- 0x7B, 0xFB, 0xFF, 0xFF, 0x97, 0xEF, 0xBF, 0xDF,
- },
- { /* Init_GR00_GR08 */
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x05, 0x0F,
- 0xFF,
- },
- { /* Init_AR00_AR14 */
- 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
- 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
- 0x41, 0x00, 0x0F, 0x00, 0x00,
- },
- { /* Init_CR00_CR18 */
- 0xA3, 0x7F, 0x7F, 0x00, 0x85, 0x16, 0x24, 0xF5,
- 0x00, 0x60, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x03, 0x09, 0xFF, 0x80, 0x40, 0xFF, 0x00, 0xE3,
- 0xFF,
- },
- { /* Init_CR30_CR4D */
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, 0x02, 0x20,
- 0x00, 0x00, 0x30, 0x40, 0x00, 0xFF, 0xBF, 0xFF,
- 0x2E, 0x27, 0x00, 0x2b, 0x0c, 0x0F, 0xEF, 0x00,
- 0xFe, 0x0f, 0x01, 0xC0, 0x27, 0xEF,
- },
- { /* Init_CR90_CRA7 */
- 0x55, 0xD9, 0x5D, 0xE1, 0x86, 0x1B, 0x8E, 0x26,
- 0xDA, 0x8D, 0xDE, 0x94, 0x00, 0x00, 0x18, 0x00,
- 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x15, 0x03,
- },
- },
};
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 4/7] fbdev: sm712fb: add 32-bit color modes, drops some other modes.
@ 2019-03-22 5:17 ` Yifeng Li
0 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
The modesetting in sm712fb is an ugly hack. First, all the registers
are programmed by hardcoded register arrays, which makes it difficult
to support different variations of color depths, refresh rates, CRT/LCD
panel, etc of the same resolution. Second, it means the standard
fb_find_mode() cannot be used and a confusing non-standard "vga="
parameter is needed. Third, there's only minimum differences between
some modes, yet around 70 lines of code and 100 registers are needed to
be indepentently specified for each mode. Fourth, the register between
some modes are inconsistent: the register configuration of different
color depths in 640 x 480 modes are identical, but for 800 x 600 modes
it's completely different. Also, some modes can drive the LCD panel
properly yet some other modes will only show a white screen of death on
the LCD panel. Fifth, some modes, such as 32-bit color modes, are
supported but never listed, and some modes are listed, such as 1280x1024
modes, but never supported by the register configuration arrays. And
some modes are partially implemented but neither listed nor supported,
i.e. the 8-bit color modes.
Fixing these problems requires a complete rewrite of modesetting code,
which is well-beyond my motivation. This commit only perform a minimum
cleanup:
1. Remove the 320 x 240 register settings, since there are never listed
and never actually being used. This resolution is not even supported by
vesafb, so it's safe to assume that no one is using such hardware. Future
developers who needs them can simply recover them from the git history.
2. Remove 1280x1024 modes and 8-bit color modes. They are listed but never
supported by the register array. So it doesn't work in the beginning, and
only gives a black screen.
3. Add 32-bit color modes. They are supported but never listed, and are
useful to some users.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
drivers/video/fbdev/sm712fb.c | 158 +++++++---------------------------
1 file changed, 31 insertions(+), 127 deletions(-)
diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
index 75d60ea63883..5e887653ef5d 100644
--- a/drivers/video/fbdev/sm712fb.c
+++ b/drivers/video/fbdev/sm712fb.c
@@ -118,25 +118,48 @@ struct vesa_mode {
};
static const struct vesa_mode vesa_mode_table[] = {
- {"0x301", 640, 480, 8},
- {"0x303", 800, 600, 8},
- {"0x305", 1024, 768, 8},
- {"0x307", 1280, 1024, 8},
-
{"0x311", 640, 480, 16},
{"0x314", 800, 600, 16},
{"0x317", 1024, 768, 16},
- {"0x31A", 1280, 1024, 16},
{"0x312", 640, 480, 24},
{"0x315", 800, 600, 24},
{"0x318", 1024, 768, 24},
- {"0x31B", 1280, 1024, 24},
+
+ {"0x329", 640, 480, 32},
+ {"0x32e", 800, 600, 32},
+ {"0x338", 1024, 768, 32},
};
/**********************************************************************
SM712 Mode table.
- **********************************************************************/
+
+ The modesetting in sm712fb is an ugly hack. First, all the registers
+ are programmed by hardcoded register arrays, which makes it difficult
+ to support different variations of color depths, refresh rates, CRT/LCD
+ panel, etc of the same resolution. Second, it means the standard
+ fb_find_mode() cannot be used and a confusing non-standard "vga="
+ parameter is needed. Third, there's only minimum differences between
+ some modes, yet around 70 lines of code and 100 registers are needed to
+ be indepentently specified for each mode. Fourth, the register between
+ some modes are inconsistent: the register configuration of different
+ color depths in 640 x 480 modes are identical, but for 800 x 600 modes
+ it's completely different. Also, some modes can drive the LCD panel
+ properly yet some other modes will only show a white screen of death on
+ the LCD panel. Fifth, there is a specific hack for Lemote Loongson 8089D
+ laptop, the 1024x768, 16-bit color mode was modified to drive its LCD panel
+ and changed to 1024x600, but the original mode was not preserved, so
+ 1024x768 16-bit color mode is completely unsupported. And previously,
+ some modes are listed, such as 1280x1024 modes, but never supported by
+ the register configuration arrays, so they are now removed. And some modes
+ are partially implemented but neither listed nor supported, i.e. the 8-bit
+ color modes, so they have been removed from vesa_mode_table, too.
+
+ I'm not the original author of the code, fixing these problems requires a
+ complete rewrite of modesetting code, which is well-beyond my motivation.
+
+ See Documentation/fb/sm712fb.txt for more information.
+**********************************************************************/
static const struct modeinit vgamode[] = {
{
/* mode#0: 640 x 480 16Bpp 60Hz */
@@ -732,125 +755,6 @@ static const struct modeinit vgamode[] = {
0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x15, 0x03,
},
},
- { /* mode#6: 320 x 240 16Bpp 60Hz */
- 320, 240, 16, 60,
- /* Init_MISC */
- 0xEB,
- { /* Init_SR0_SR4 */
- 0x03, 0x01, 0x0F, 0x03, 0x0E,
- },
- { /* Init_SR10_SR24 */
- 0xF3, 0xB6, 0xC0, 0xDD, 0x00, 0x0E, 0x17, 0x2C,
- 0x99, 0x02, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0xC4, 0x32, 0x02, 0x01, 0x01,
- },
- { /* Init_SR30_SR75 */
- 0x38, 0x03, 0x20, 0x09, 0xC0, 0x3A, 0x3A, 0x3A,
- 0x3A, 0x3A, 0x3A, 0x3A, 0x00, 0x00, 0x03, 0xFF,
- 0x00, 0xFC, 0x00, 0x00, 0x20, 0x18, 0x00, 0xFC,
- 0x20, 0x0C, 0x44, 0x20, 0x00, 0x00, 0x00, 0x3A,
- 0x06, 0x68, 0xA7, 0x7F, 0x83, 0x24, 0xFF, 0x03,
- 0x00, 0x60, 0x59, 0x3A, 0x3A, 0x00, 0x00, 0x3A,
- 0x01, 0x80, 0x7E, 0x1A, 0x1A, 0x00, 0x00, 0x00,
- 0x50, 0x03, 0x74, 0x14, 0x08, 0x43, 0x08, 0x43,
- 0x04, 0x45, 0x30, 0x30, 0x40, 0x20,
- },
- { /* Init_SR80_SR93 */
- 0xFF, 0x07, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0x3A,
- 0xF7, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0x3A, 0x3A,
- 0x00, 0x00, 0x00, 0x00,
- },
- { /* Init_SRA0_SRAF */
- 0x00, 0xFB, 0x9F, 0x01, 0x00, 0xED, 0xED, 0xED,
- 0x7B, 0xFB, 0xFF, 0xFF, 0x97, 0xEF, 0xBF, 0xDF,
- },
- { /* Init_GR00_GR08 */
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x05, 0x0F,
- 0xFF,
- },
- { /* Init_AR00_AR14 */
- 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
- 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
- 0x41, 0x00, 0x0F, 0x00, 0x00,
- },
- { /* Init_CR00_CR18 */
- 0xA3, 0x7F, 0x7F, 0x00, 0x85, 0x16, 0x24, 0xF5,
- 0x00, 0x60, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x03, 0x09, 0xFF, 0x80, 0x40, 0xFF, 0x00, 0xE3,
- 0xFF,
- },
- { /* Init_CR30_CR4D */
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, 0x02, 0x20,
- 0x00, 0x00, 0x30, 0x40, 0x00, 0xFF, 0xBF, 0xFF,
- 0x2E, 0x27, 0x00, 0x2b, 0x0c, 0x0F, 0xEF, 0x00,
- 0xFe, 0x0f, 0x01, 0xC0, 0x27, 0xEF,
- },
- { /* Init_CR90_CRA7 */
- 0x55, 0xD9, 0x5D, 0xE1, 0x86, 0x1B, 0x8E, 0x26,
- 0xDA, 0x8D, 0xDE, 0x94, 0x00, 0x00, 0x18, 0x00,
- 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x15, 0x03,
- },
- },
-
- { /* mode#8: 320 x 240 32Bpp 60Hz */
- 320, 240, 32, 60,
- /* Init_MISC */
- 0xEB,
- { /* Init_SR0_SR4 */
- 0x03, 0x01, 0x0F, 0x03, 0x0E,
- },
- { /* Init_SR10_SR24 */
- 0xF3, 0xB6, 0xC0, 0xDD, 0x00, 0x0E, 0x17, 0x2C,
- 0x99, 0x02, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0xC4, 0x32, 0x02, 0x01, 0x01,
- },
- { /* Init_SR30_SR75 */
- 0x38, 0x03, 0x20, 0x09, 0xC0, 0x3A, 0x3A, 0x3A,
- 0x3A, 0x3A, 0x3A, 0x3A, 0x00, 0x00, 0x03, 0xFF,
- 0x00, 0xFC, 0x00, 0x00, 0x20, 0x18, 0x00, 0xFC,
- 0x20, 0x0C, 0x44, 0x20, 0x00, 0x00, 0x00, 0x3A,
- 0x06, 0x68, 0xA7, 0x7F, 0x83, 0x24, 0xFF, 0x03,
- 0x00, 0x60, 0x59, 0x3A, 0x3A, 0x00, 0x00, 0x3A,
- 0x01, 0x80, 0x7E, 0x1A, 0x1A, 0x00, 0x00, 0x00,
- 0x50, 0x03, 0x74, 0x14, 0x08, 0x43, 0x08, 0x43,
- 0x04, 0x45, 0x30, 0x30, 0x40, 0x20,
- },
- { /* Init_SR80_SR93 */
- 0xFF, 0x07, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0x3A,
- 0xF7, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0x3A, 0x3A,
- 0x00, 0x00, 0x00, 0x00,
- },
- { /* Init_SRA0_SRAF */
- 0x00, 0xFB, 0x9F, 0x01, 0x00, 0xED, 0xED, 0xED,
- 0x7B, 0xFB, 0xFF, 0xFF, 0x97, 0xEF, 0xBF, 0xDF,
- },
- { /* Init_GR00_GR08 */
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x05, 0x0F,
- 0xFF,
- },
- { /* Init_AR00_AR14 */
- 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
- 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
- 0x41, 0x00, 0x0F, 0x00, 0x00,
- },
- { /* Init_CR00_CR18 */
- 0xA3, 0x7F, 0x7F, 0x00, 0x85, 0x16, 0x24, 0xF5,
- 0x00, 0x60, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x03, 0x09, 0xFF, 0x80, 0x40, 0xFF, 0x00, 0xE3,
- 0xFF,
- },
- { /* Init_CR30_CR4D */
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, 0x02, 0x20,
- 0x00, 0x00, 0x30, 0x40, 0x00, 0xFF, 0xBF, 0xFF,
- 0x2E, 0x27, 0x00, 0x2b, 0x0c, 0x0F, 0xEF, 0x00,
- 0xFe, 0x0f, 0x01, 0xC0, 0x27, 0xEF,
- },
- { /* Init_CR90_CRA7 */
- 0x55, 0xD9, 0x5D, 0xE1, 0x86, 0x1B, 0x8E, 0x26,
- 0xDA, 0x8D, 0xDE, 0x94, 0x00, 0x00, 0x18, 0x00,
- 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x15, 0x03,
- },
- },
};
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 5/7] Documentation: fb: sm712fb: add information mainly about 2D.
2019-03-22 5:17 ` Yifeng Li
@ 2019-03-22 5:17 ` Yifeng Li
-1 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
This commits add information about 32-bit color, 2D acceleration,
as well as adding additional, general information about the hardware
and many existing problems of the sm712fb driver.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
Documentation/fb/sm712fb.txt | 123 +++++++++++++++++++++++++++++++----
1 file changed, 110 insertions(+), 13 deletions(-)
diff --git a/Documentation/fb/sm712fb.txt b/Documentation/fb/sm712fb.txt
index c388442edf51..906b48aa40e4 100644
--- a/Documentation/fb/sm712fb.txt
+++ b/Documentation/fb/sm712fb.txt
@@ -1,31 +1,128 @@
What is sm712fb?
=================
-This is a graphics framebuffer driver for Silicon Motion SM712 based processors.
+"sm712fb" is a graphics framebuffer driver for Silicon Motion SM710 (LynxEM),
+SM712 (LynxEM+), and SM720 (Lynx3DM, Lynx3DM+, aka. LynxEM4+) series of
+video controllers. This series of video controller is a legacy from ~1998,
+and was used on many classic, "prehistoric" laptops from 1998-2004, such as
+IBM Thinkpad S30 and 240X. It was also used on some servers, industrial
+computers, x86 and non-x86 embedded devices where only basic graphics was
+needed.
+
+Notably, Lemote YeeLoong 8089, a MIPS laptop based on the Chinese Loongson
+2F MIPS processor, is also using this chip because of hardware constraints,
+and at a time, somewhat popular in the free software community due the
+fact that it was the first laptop powered exclusively by free software,
+and it was also an inexpensive platform for non-x86 hobbyists to explore.
How to use it?
==============
-Switching modes is done using the video=sm712fb:... boot parameter.
-
-If you want, for example, enable a resolution of 1280x1024x24bpp you should
-pass to the kernel this command line: "video=sm712fb:0x31B".
+You should not compile-in vesafb, since SM7xx can be used in a VGA
+compatible mode, resulting conflicts with this driver. In addition,
+the VGA compatible mode was never tested by the maintainers.
-You should not compile-in vesafb.
+Currently, the driver supports 3 modes: 640x480, 800x600, 1024x768,
+at 16, 24 or 32-bit depth. Switching modes is done using the
+`video=sm712fb:0x___` boot parameter. If you want, for example,
+enable a resolution of 1280x1024x24bpp, you should pass to the kernel
+this command line: "video=sm712fb:0x31B".
-Currently supported video modes are:
+Please consult the following table for the hexadecimal codes of
+different modes.
[Graphic modes]
-bpp | 640x480 800x600 1024x768 1280x1024
-----+--------------------------------------------
- 8 | 0x301 0x303 0x305 0x307
- 16 | 0x311 0x314 0x317 0x31A
- 24 | 0x312 0x315 0x318 0x31B
+bpp | 640x480 800x600 1024x768
+----+---------------------------
+ 16 | 0x311 0x314 0x317
+ 24 | 0x312 0x315 0x318
+ 32 | 0x329 0x32E 0x338
+
+32-bit is really 8:8:8:8, but the final 8-bit number is an "empty"
+alpha channel, it's otherwise equal to 24-bit color. However, they
+could still be useful. For example, "fbterm" supports 32-bit mode
+but not 24-bit mode.
+
+Notes about Modesetting
+========================
+
+The modesetting code in sm712fb has major problems.
+
+* Switching to 8-bit color mode will result in a black screen, so
+they are removed from the list of supported graphic modes. But they
+can still be switched to on-the-fly, don't do that then!
+
+* Only a refresh rate of 60 Hz is supported.
+
+* 1024x768 with 16-bit color is not really supported, because the
+registers have been hacked by the original developer to adapt
+the 1024x600 screen on Lemote YeeLoong 8089.
+
+* If you are using a Lemote YeeLoong 8089, please remember that only
+the 1024x768 modes are guaranteed to drive the LCD panel properly.
+Other modes are meant to drive a CRT, and may drive the LCD incorrectly
+and result in a white screen with random garbage. External VGA output is
+unaffected.
+
+Due to the way registers are hardcoded, it's impossible to fix them
+without a major code rewrite. If you've been hit by these problems badly
+and really need to get them fixed, please contact the driver maintainers.
+
+2D acceleration
+==============
+
+Without 2D acceleration, the framebuffer suffers from extremely low performance,
+even scrolling a single line of text on the console required an unaccelerated
+screen redraw. Thus, 2D acceleration is enable by default. However, currently
+it's only supported on SM710/712 with Little-Endian CPUs. Big-endian and
+SM720 devices are currently not supported.
+
+2D acceleration can be controlled using the `video=sm712fb:accel:1` parameter.
+The default option, "1" activate 2D acceleration. If you have problems, you can
+set "0" to disable it. Different options can be separated by a comma, for
+example, "video=sm712fb:0x31B,accel:0" set the resolution to 1280x1024x24bpp
+while disabling the 2D acceleration.
+
+Although it has been extensively tested by the maintainer, 2D acceleration
+in 24-bit color mode may still have minor issues. If you've encounter any
+screen glitches in 24-bit mode in Linux framebuffer the framebuffer, don't
+hurry disabling it, you should try switching to 32-bit mode first, normally
+it should fix the problem. If you can reliably reproduce the screen glitches,
+please report your method to the maintainers.
Missing Features
================
(alias TODO list)
- * 2D acceleratrion
+The following features are not implemented in this driver,
+
+ * 2D acceleration on SM720 and Big-Endian CPUs.
+ * More VGA modes.
* dual-head support
+ * hardware cursor support
+
+The first feature is planned to be implemented soon, but the maintainer
+does not receive any monetary or hardware support from any company or OEMs,
+and he has to purchase a test platform personally. The 1998's hardware
+still costs 200 USD+, so don't expected an ETA. If you have a Big-Endian
+platform and willing to help testing, please contact the maintainer, thanks!
+
+Other VGA modes, dual-head, or hardware cursor support should be possible to
+implement, but parts of the code must be rewritten, and there's little demand
+for them on this legacy (retro?) platform, so there's no plan to implement them.
+If you have a genuine need for them, please contact the maintainers.
+
+Maintainers
+================
+
+This driver is maintained by
+
+ * Tom Li <tomli@tomli.me>
+ * Sudip Mukherjee <sudipm.mukherjee@gmail.com>
+ * Teddy Wang <teddy.wang@siliconmotion.com>
+
+Tom Li was the last contributor of this driver who implemented 2D acceleration,
+and is the main author of this documentation, please send any bug reports or
+requests to Tom, but don't forget to CC other maintainers as well to make everyone
+be informed.
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 5/7] Documentation: fb: sm712fb: add information mainly about 2D.
@ 2019-03-22 5:17 ` Yifeng Li
0 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
This commits add information about 32-bit color, 2D acceleration,
as well as adding additional, general information about the hardware
and many existing problems of the sm712fb driver.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
Documentation/fb/sm712fb.txt | 123 +++++++++++++++++++++++++++++++----
1 file changed, 110 insertions(+), 13 deletions(-)
diff --git a/Documentation/fb/sm712fb.txt b/Documentation/fb/sm712fb.txt
index c388442edf51..906b48aa40e4 100644
--- a/Documentation/fb/sm712fb.txt
+++ b/Documentation/fb/sm712fb.txt
@@ -1,31 +1,128 @@
What is sm712fb?
========
-This is a graphics framebuffer driver for Silicon Motion SM712 based processors.
+"sm712fb" is a graphics framebuffer driver for Silicon Motion SM710 (LynxEM),
+SM712 (LynxEM+), and SM720 (Lynx3DM, Lynx3DM+, aka. LynxEM4+) series of
+video controllers. This series of video controller is a legacy from ~1998,
+and was used on many classic, "prehistoric" laptops from 1998-2004, such as
+IBM Thinkpad S30 and 240X. It was also used on some servers, industrial
+computers, x86 and non-x86 embedded devices where only basic graphics was
+needed.
+
+Notably, Lemote YeeLoong 8089, a MIPS laptop based on the Chinese Loongson
+2F MIPS processor, is also using this chip because of hardware constraints,
+and at a time, somewhat popular in the free software community due the
+fact that it was the first laptop powered exclusively by free software,
+and it was also an inexpensive platform for non-x86 hobbyists to explore.
How to use it?
=======
-Switching modes is done using the video=sm712fb:... boot parameter.
-
-If you want, for example, enable a resolution of 1280x1024x24bpp you should
-pass to the kernel this command line: "video=sm712fb:0x31B".
+You should not compile-in vesafb, since SM7xx can be used in a VGA
+compatible mode, resulting conflicts with this driver. In addition,
+the VGA compatible mode was never tested by the maintainers.
-You should not compile-in vesafb.
+Currently, the driver supports 3 modes: 640x480, 800x600, 1024x768,
+at 16, 24 or 32-bit depth. Switching modes is done using the
+`video=sm712fb:0x___` boot parameter. If you want, for example,
+enable a resolution of 1280x1024x24bpp, you should pass to the kernel
+this command line: "video=sm712fb:0x31B".
-Currently supported video modes are:
+Please consult the following table for the hexadecimal codes of
+different modes.
[Graphic modes]
-bpp | 640x480 800x600 1024x768 1280x1024
-----+--------------------------------------------
- 8 | 0x301 0x303 0x305 0x307
- 16 | 0x311 0x314 0x317 0x31A
- 24 | 0x312 0x315 0x318 0x31B
+bpp | 640x480 800x600 1024x768
+----+---------------------------
+ 16 | 0x311 0x314 0x317
+ 24 | 0x312 0x315 0x318
+ 32 | 0x329 0x32E 0x338
+
+32-bit is really 8:8:8:8, but the final 8-bit number is an "empty"
+alpha channel, it's otherwise equal to 24-bit color. However, they
+could still be useful. For example, "fbterm" supports 32-bit mode
+but not 24-bit mode.
+
+Notes about Modesetting
+============
+
+The modesetting code in sm712fb has major problems.
+
+* Switching to 8-bit color mode will result in a black screen, so
+they are removed from the list of supported graphic modes. But they
+can still be switched to on-the-fly, don't do that then!
+
+* Only a refresh rate of 60 Hz is supported.
+
+* 1024x768 with 16-bit color is not really supported, because the
+registers have been hacked by the original developer to adapt
+the 1024x600 screen on Lemote YeeLoong 8089.
+
+* If you are using a Lemote YeeLoong 8089, please remember that only
+the 1024x768 modes are guaranteed to drive the LCD panel properly.
+Other modes are meant to drive a CRT, and may drive the LCD incorrectly
+and result in a white screen with random garbage. External VGA output is
+unaffected.
+
+Due to the way registers are hardcoded, it's impossible to fix them
+without a major code rewrite. If you've been hit by these problems badly
+and really need to get them fixed, please contact the driver maintainers.
+
+2D acceleration
+=======
+
+Without 2D acceleration, the framebuffer suffers from extremely low performance,
+even scrolling a single line of text on the console required an unaccelerated
+screen redraw. Thus, 2D acceleration is enable by default. However, currently
+it's only supported on SM710/712 with Little-Endian CPUs. Big-endian and
+SM720 devices are currently not supported.
+
+2D acceleration can be controlled using the `video=sm712fb:accel:1` parameter.
+The default option, "1" activate 2D acceleration. If you have problems, you can
+set "0" to disable it. Different options can be separated by a comma, for
+example, "video=sm712fb:0x31B,accel:0" set the resolution to 1280x1024x24bpp
+while disabling the 2D acceleration.
+
+Although it has been extensively tested by the maintainer, 2D acceleration
+in 24-bit color mode may still have minor issues. If you've encounter any
+screen glitches in 24-bit mode in Linux framebuffer the framebuffer, don't
+hurry disabling it, you should try switching to 32-bit mode first, normally
+it should fix the problem. If you can reliably reproduce the screen glitches,
+please report your method to the maintainers.
Missing Features
========
(alias TODO list)
- * 2D acceleratrion
+The following features are not implemented in this driver,
+
+ * 2D acceleration on SM720 and Big-Endian CPUs.
+ * More VGA modes.
* dual-head support
+ * hardware cursor support
+
+The first feature is planned to be implemented soon, but the maintainer
+does not receive any monetary or hardware support from any company or OEMs,
+and he has to purchase a test platform personally. The 1998's hardware
+still costs 200 USD+, so don't expected an ETA. If you have a Big-Endian
+platform and willing to help testing, please contact the maintainer, thanks!
+
+Other VGA modes, dual-head, or hardware cursor support should be possible to
+implement, but parts of the code must be rewritten, and there's little demand
+for them on this legacy (retro?) platform, so there's no plan to implement them.
+If you have a genuine need for them, please contact the maintainers.
+
+Maintainers
+========
+
+This driver is maintained by
+
+ * Tom Li <tomli@tomli.me>
+ * Sudip Mukherjee <sudipm.mukherjee@gmail.com>
+ * Teddy Wang <teddy.wang@siliconmotion.com>
+
+Tom Li was the last contributor of this driver who implemented 2D acceleration,
+and is the main author of this documentation, please send any bug reports or
+requests to Tom, but don't forget to CC other maintainers as well to make everyone
+be informed.
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 6/7] fbdev: sm712fb: Kconfig: add information about docs.
2019-03-22 5:17 ` Yifeng Li
@ 2019-03-22 5:17 ` Yifeng Li
-1 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
This commits update the Kconfig description of sm712fb,
and inform the user about the existence of documentation.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
Documentation/fb/sm712fb.txt | 44 ++++++++++++++++++++----------------
drivers/video/fbdev/Kconfig | 4 ++++
2 files changed, 29 insertions(+), 19 deletions(-)
diff --git a/Documentation/fb/sm712fb.txt b/Documentation/fb/sm712fb.txt
index 906b48aa40e4..c9a8fe059ac7 100644
--- a/Documentation/fb/sm712fb.txt
+++ b/Documentation/fb/sm712fb.txt
@@ -18,9 +18,10 @@ and it was also an inexpensive platform for non-x86 hobbyists to explore.
How to use it?
==============
-You should not compile-in vesafb, since SM7xx can be used in a VGA
-compatible mode, resulting conflicts with this driver. In addition,
-the VGA compatible mode was never tested by the maintainers.
+SM7xx can be used in a VGA compatible mode and a native framebuffer mode,
+this driver is a native driver. By default, it has no problem taking over
+the VGA driver automatically, but you should not attempt to use both at
+the same time.
Currently, the driver supports 3 modes: 640x480, 800x600, 1024x768,
at 16, 24 or 32-bit depth. Switching modes is done using the
@@ -44,30 +45,35 @@ alpha channel, it's otherwise equal to 24-bit color. However, they
could still be useful. For example, "fbterm" supports 32-bit mode
but not 24-bit mode.
-Notes about Modesetting
+WARNING
========================
The modesetting code in sm712fb has major problems.
-* Switching to 8-bit color mode will result in a black screen, so
-they are removed from the list of supported graphic modes. But they
-can still be switched to on-the-fly, don't do that then!
+* If you are using a laptop, using a non-native resolution (for example,
+using 640x480 on a 1024x768 screen) may garbled your LCD display, showing
+a white screen.
-* Only a refresh rate of 60 Hz is supported.
+* The driver assumes a 1024x768 LCD by default, if you have a rare 800x600,
+or 640x480 screen (e.g. Thinkpad 240X), it may garble your LCD by default.
+
+* The driver will crash/hang on systems with less than 2 MiB of VRAM,
+(e.g. Thinkpad 240X), it cannot be fixed because the maintainers don't
+have the hardware for testing.
-* 1024x768 with 16-bit color is not really supported, because the
-registers have been hacked by the original developer to adapt
-the 1024x600 screen on Lemote YeeLoong 8089.
+* Switching to 8-bit color mode will result in a black screen, so they are
+removed from the list of supported graphic modes. But they can still be
+switched to on-the-fly with fbset, don't do that then!
-* If you are using a Lemote YeeLoong 8089, please remember that only
-the 1024x768 modes are guaranteed to drive the LCD panel properly.
-Other modes are meant to drive a CRT, and may drive the LCD incorrectly
-and result in a white screen with random garbage. External VGA output is
-unaffected.
+* It is not possible to use different resolutions for LCD and VGA out.
+
+* Only a refresh rate of 60 Hz is supported.
Due to the way registers are hardcoded, it's impossible to fix them
-without a major code rewrite. If you've been hit by these problems badly
-and really need to get them fixed, please contact the driver maintainers.
+without a major code rewrite.
+
+If you've been hit by these problems badly and really need to get them
+fixed, please contact the driver maintainers.
2D acceleration
==============
@@ -97,7 +103,7 @@ Missing Features
The following features are not implemented in this driver,
- * 2D acceleration on SM720 and Big-Endian CPUs.
+ * 2D acceleration on Big-Endian CPUs.
* More VGA modes.
* dual-head support
* hardware cursor support
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index ae7712c9687a..4f3c0075352a 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2302,6 +2302,10 @@ config FB_SM712
called sm712fb. If you want to compile it as a module, say M
here and read <file:Documentation/kbuild/modules.txt>.
+ Warning: this driver has many known problems and limitations,
+ please make sure you've checked the documentation, at
+ <file:Documentation/fb/sm712fb.txt>.
+
source "drivers/video/fbdev/omap/Kconfig"
source "drivers/video/fbdev/omap2/Kconfig"
source "drivers/video/fbdev/mmp/Kconfig"
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 6/7] fbdev: sm712fb: Kconfig: add information about docs.
@ 2019-03-22 5:17 ` Yifeng Li
0 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
This commits update the Kconfig description of sm712fb,
and inform the user about the existence of documentation.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
Documentation/fb/sm712fb.txt | 44 ++++++++++++++++++++----------------
drivers/video/fbdev/Kconfig | 4 ++++
2 files changed, 29 insertions(+), 19 deletions(-)
diff --git a/Documentation/fb/sm712fb.txt b/Documentation/fb/sm712fb.txt
index 906b48aa40e4..c9a8fe059ac7 100644
--- a/Documentation/fb/sm712fb.txt
+++ b/Documentation/fb/sm712fb.txt
@@ -18,9 +18,10 @@ and it was also an inexpensive platform for non-x86 hobbyists to explore.
How to use it?
=======
-You should not compile-in vesafb, since SM7xx can be used in a VGA
-compatible mode, resulting conflicts with this driver. In addition,
-the VGA compatible mode was never tested by the maintainers.
+SM7xx can be used in a VGA compatible mode and a native framebuffer mode,
+this driver is a native driver. By default, it has no problem taking over
+the VGA driver automatically, but you should not attempt to use both at
+the same time.
Currently, the driver supports 3 modes: 640x480, 800x600, 1024x768,
at 16, 24 or 32-bit depth. Switching modes is done using the
@@ -44,30 +45,35 @@ alpha channel, it's otherwise equal to 24-bit color. However, they
could still be useful. For example, "fbterm" supports 32-bit mode
but not 24-bit mode.
-Notes about Modesetting
+WARNING
============
The modesetting code in sm712fb has major problems.
-* Switching to 8-bit color mode will result in a black screen, so
-they are removed from the list of supported graphic modes. But they
-can still be switched to on-the-fly, don't do that then!
+* If you are using a laptop, using a non-native resolution (for example,
+using 640x480 on a 1024x768 screen) may garbled your LCD display, showing
+a white screen.
-* Only a refresh rate of 60 Hz is supported.
+* The driver assumes a 1024x768 LCD by default, if you have a rare 800x600,
+or 640x480 screen (e.g. Thinkpad 240X), it may garble your LCD by default.
+
+* The driver will crash/hang on systems with less than 2 MiB of VRAM,
+(e.g. Thinkpad 240X), it cannot be fixed because the maintainers don't
+have the hardware for testing.
-* 1024x768 with 16-bit color is not really supported, because the
-registers have been hacked by the original developer to adapt
-the 1024x600 screen on Lemote YeeLoong 8089.
+* Switching to 8-bit color mode will result in a black screen, so they are
+removed from the list of supported graphic modes. But they can still be
+switched to on-the-fly with fbset, don't do that then!
-* If you are using a Lemote YeeLoong 8089, please remember that only
-the 1024x768 modes are guaranteed to drive the LCD panel properly.
-Other modes are meant to drive a CRT, and may drive the LCD incorrectly
-and result in a white screen with random garbage. External VGA output is
-unaffected.
+* It is not possible to use different resolutions for LCD and VGA out.
+
+* Only a refresh rate of 60 Hz is supported.
Due to the way registers are hardcoded, it's impossible to fix them
-without a major code rewrite. If you've been hit by these problems badly
-and really need to get them fixed, please contact the driver maintainers.
+without a major code rewrite.
+
+If you've been hit by these problems badly and really need to get them
+fixed, please contact the driver maintainers.
2D acceleration
=======
@@ -97,7 +103,7 @@ Missing Features
The following features are not implemented in this driver,
- * 2D acceleration on SM720 and Big-Endian CPUs.
+ * 2D acceleration on Big-Endian CPUs.
* More VGA modes.
* dual-head support
* hardware cursor support
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index ae7712c9687a..4f3c0075352a 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2302,6 +2302,10 @@ config FB_SM712
called sm712fb. If you want to compile it as a module, say M
here and read <file:Documentation/kbuild/modules.txt>.
+ Warning: this driver has many known problems and limitations,
+ please make sure you've checked the documentation, at
+ <file:Documentation/fb/sm712fb.txt>.
+
source "drivers/video/fbdev/omap/Kconfig"
source "drivers/video/fbdev/omap2/Kconfig"
source "drivers/video/fbdev/mmp/Kconfig"
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
2019-03-22 5:17 ` Yifeng Li
@ 2019-03-22 5:17 ` Yifeng Li
-1 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
I have working on the sm712fb driver for a while and have some familiarity
with this hardware, I'll be helping working on and testing problems of this
driver, so add myself to the MAINTAINERS file.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index dce5c099f43c..3f6686f0f020 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13871,6 +13871,7 @@ F: drivers/input/touchscreen/silead.c
F: drivers/platform/x86/touchscreen_dmi.c
SILICON MOTION SM712 FRAME BUFFER DRIVER
+M: Tom Li <tomli@tomli.me>
M: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
M: Teddy Wang <teddy.wang@siliconmotion.com>
M: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
@ 2019-03-22 5:17 ` Yifeng Li
0 siblings, 0 replies; 60+ messages in thread
From: Yifeng Li @ 2019-03-22 5:17 UTC (permalink / raw)
To: Sudip Mukherjee, Teddy Wang, linux-kernel
Cc: Bartlomiej Zolnierkiewicz, linux-fbdev, dri-devel, Yifeng Li
I have working on the sm712fb driver for a while and have some familiarity
with this hardware, I'll be helping working on and testing problems of this
driver, so add myself to the MAINTAINERS file.
Signed-off-by: Yifeng Li <tomli@tomli.me>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index dce5c099f43c..3f6686f0f020 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13871,6 +13871,7 @@ F: drivers/input/touchscreen/silead.c
F: drivers/platform/x86/touchscreen_dmi.c
SILICON MOTION SM712 FRAME BUFFER DRIVER
+M: Tom Li <tomli@tomli.me>
M: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
M: Teddy Wang <teddy.wang@siliconmotion.com>
M: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
--
2.20.1
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/7] fbdev: sm712fb: use type "u8" for 8-bit I/O.
2019-03-22 5:17 ` Yifeng Li
(?)
@ 2019-03-31 17:16 ` Sudip Mukherjee
-1 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 17:16 UTC (permalink / raw)
To: Yifeng Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Fri, Mar 22, 2019 at 01:17:53PM +0800, Yifeng Li wrote:
> This commit converts "unsigned int" and "int" in I/O wrappers
> to "u8". It improves readability since it's consistent with
> the prototypes of readb() and writeb(). More importantly, it
> reduces readers' confusion, since the upcoming 2D acceleration
> code will use a different wordsize.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
Tested-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/7] fbdev: sm712fb: use type "u8" for 8-bit I/O.
@ 2019-03-31 17:16 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 17:16 UTC (permalink / raw)
To: Yifeng Li
Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Teddy Wang, dri-devel,
linux-kernel
On Fri, Mar 22, 2019 at 01:17:53PM +0800, Yifeng Li wrote:
> This commit converts "unsigned int" and "int" in I/O wrappers
> to "u8". It improves readability since it's consistent with
> the prototypes of readb() and writeb(). More importantly, it
> reduces readers' confusion, since the upcoming 2D acceleration
> code will use a different wordsize.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
Tested-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/7] fbdev: sm712fb: use type "u8" for 8-bit I/O.
@ 2019-03-31 17:16 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 17:16 UTC (permalink / raw)
To: Yifeng Li
Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Teddy Wang, dri-devel,
linux-kernel
On Fri, Mar 22, 2019 at 01:17:53PM +0800, Yifeng Li wrote:
> This commit converts "unsigned int" and "int" in I/O wrappers
> to "u8". It improves readability since it's consistent with
> the prototypes of readb() and writeb(). More importantly, it
> reduces readers' confusion, since the upcoming 2D acceleration
> code will use a different wordsize.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
Tested-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
--
Regards
Sudip
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 2/7] fbdev: sm712fb: add 2D-related I/O headers and functions.
2019-03-22 5:17 ` Yifeng Li
@ 2019-03-31 17:25 ` Sudip Mukherjee
-1 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 17:25 UTC (permalink / raw)
To: Yifeng Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Fri, Mar 22, 2019 at 01:17:54PM +0800, Yifeng Li wrote:
> This commit adds I/O macros and functions related to 2D opeartions.
> A hunk of hardware register definitions are taken verbatim from
> OpenBSD.
>
> In addition, a utility function pad_to_dword() is added to help
> padding data for the 2D engine. It accepts 3, 2, or 1 byte(s) of
> data, and pads it to a 32-bit word suitable for 2D Drawing Engine.
>
> Yes, we can set info->pixmap.scan_align/buf_align = 4 and forget
> about padding, but it's incompatible with cfb_imageblit() w/
> depth == 1. In case we need to fallback (e.g. debugging), it would
> be inconvenient, so we pad it manually.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> ---
> drivers/video/fbdev/sm712.h | 96 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 96 insertions(+)
>
> diff --git a/drivers/video/fbdev/sm712.h b/drivers/video/fbdev/sm712.h
> index 89e446db2ac7..4892fd485f08 100644
> --- a/drivers/video/fbdev/sm712.h
> +++ b/drivers/video/fbdev/sm712.h
> @@ -91,6 +91,102 @@ static inline u8 smtc_seqr(u8 reg)
> return smtc_mmiorb(0x3c5);
> }
>
> +/*
> + * DPR (2D drawing engine)
> + */
> +#define DPR_COORDS(x, y) (((x) << 16) | (y))
> +
<snip>
> +#define DPR_SRC_WINDOW 0x3c
> +#define DPR_SRC_BASE 0x40
> +#define DPR_DST_BASE 0x44
> +
> +#define DE_CTRL_START 0x80000000
> +#define DE_CTRL_RTOL 0x08000000
> +#define DE_CTRL_COMMAND_MASK 0x001f0000
> +#define DE_CTRL_COMMAND_SHIFT 16
> +#define DE_CTRL_COMMAND_BITBLT 0x00
> +#define DE_CTRL_COMMAND_SOLIDFILL 0x01
> +#define DE_CTRL_COMMAND_HOSTWRITE 0x08
> +#define DE_CTRL_ROP2_SELECT 0x00008000
> +#define DE_CTRL_ROP2_SRC_IS_PATTERN 0x00004000
> +#define DE_CTRL_ROP2_SHIFT 0
> +#define DE_CTRL_ROP2_COPY 0x0c
> +#define DE_CTRL_HOST_SHIFT 22
> +#define DE_CTRL_HOST_SRC_IS_MONO 0x01
> +#define DE_CTRL_FORMAT_XY 0x00
> +#define DE_CTRL_FORMAT_24BIT 0x30
Please fix the alignment. Some of them are right aligned and some are left.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 2/7] fbdev: sm712fb: add 2D-related I/O headers and functions.
@ 2019-03-31 17:25 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 17:25 UTC (permalink / raw)
To: Yifeng Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Fri, Mar 22, 2019 at 01:17:54PM +0800, Yifeng Li wrote:
> This commit adds I/O macros and functions related to 2D opeartions.
> A hunk of hardware register definitions are taken verbatim from
> OpenBSD.
>
> In addition, a utility function pad_to_dword() is added to help
> padding data for the 2D engine. It accepts 3, 2, or 1 byte(s) of
> data, and pads it to a 32-bit word suitable for 2D Drawing Engine.
>
> Yes, we can set info->pixmap.scan_align/buf_align = 4 and forget
> about padding, but it's incompatible with cfb_imageblit() w/
> depth = 1. In case we need to fallback (e.g. debugging), it would
> be inconvenient, so we pad it manually.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> ---
> drivers/video/fbdev/sm712.h | 96 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 96 insertions(+)
>
> diff --git a/drivers/video/fbdev/sm712.h b/drivers/video/fbdev/sm712.h
> index 89e446db2ac7..4892fd485f08 100644
> --- a/drivers/video/fbdev/sm712.h
> +++ b/drivers/video/fbdev/sm712.h
> @@ -91,6 +91,102 @@ static inline u8 smtc_seqr(u8 reg)
> return smtc_mmiorb(0x3c5);
> }
>
> +/*
> + * DPR (2D drawing engine)
> + */
> +#define DPR_COORDS(x, y) (((x) << 16) | (y))
> +
<snip>
> +#define DPR_SRC_WINDOW 0x3c
> +#define DPR_SRC_BASE 0x40
> +#define DPR_DST_BASE 0x44
> +
> +#define DE_CTRL_START 0x80000000
> +#define DE_CTRL_RTOL 0x08000000
> +#define DE_CTRL_COMMAND_MASK 0x001f0000
> +#define DE_CTRL_COMMAND_SHIFT 16
> +#define DE_CTRL_COMMAND_BITBLT 0x00
> +#define DE_CTRL_COMMAND_SOLIDFILL 0x01
> +#define DE_CTRL_COMMAND_HOSTWRITE 0x08
> +#define DE_CTRL_ROP2_SELECT 0x00008000
> +#define DE_CTRL_ROP2_SRC_IS_PATTERN 0x00004000
> +#define DE_CTRL_ROP2_SHIFT 0
> +#define DE_CTRL_ROP2_COPY 0x0c
> +#define DE_CTRL_HOST_SHIFT 22
> +#define DE_CTRL_HOST_SRC_IS_MONO 0x01
> +#define DE_CTRL_FORMAT_XY 0x00
> +#define DE_CTRL_FORMAT_24BIT 0x30
Please fix the alignment. Some of them are right aligned and some are left.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 3/7] fbdev: sm712fb: support 2D acceleration on SM712 w/ Little-Endian CPU.
2019-03-22 5:17 ` Yifeng Li
@ 2019-03-31 18:09 ` Sudip Mukherjee
-1 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 18:09 UTC (permalink / raw)
To: Yifeng Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Fri, Mar 22, 2019 at 01:17:55PM +0800, Yifeng Li wrote:
> Previously, in staging/sm7xxfb (now fbdev/sm712fb), 2D acceleration
> was implemented, but after its submission, a critical bug that causes
> total system hang was discovered, as a stopgap measure, 2D ops was
> completele removed in commit 3af805735a25 ("staging: sm7xx: remove the
> buggy 2D acceleration support") and never implemented again.
>
> It created a massive usability problem - on YeeLoong 8089, a notable
> MIPS platform which uses SM712 - even scrolling a single line of text
> on the console required an unaccelerated screen redraw, running "dmesg"
> typically takes 8-11 seconds, and absurdly, printf(), became a significant
> performance bottleneck that slows down GCC and "make", make the computer
> largely unusable.
>
> So I decided to take a look. Most of the my actual development was done
> in 2014 in a personal out-of-tree driver, I did not mainline it because
> 2D acceleration was not working properly in 24-bit color. I discovered
> the solution in early 2019 and now it's ready to be mainlined.
>
> This commit reimplements the 2D acceleration for sm712fb. Unlike the
> original implementation, which was messy and unnecessarily complicated
> by calling a 2D acceleration wrapper file with many unneeded functions,
> this is a minimum and (relatively) clean implementation. My tests have
> shown that running "dmesg" only takes 0.9 seconds, a performance boost
> of 950%. System hangs did not occur in my tests.
I didnot notice any performace improvement in my system. Infact, I have
never seen the performance problem that you mentioned. But, it will be
good to have the 2D feature back again.
>
<snip>
> + */
> + smtcfb_reset_accel();
> +
> smtc_set_timing(sfb);
> +
> + /*
> + * Currently, 2D acceleration is only supported on SM712 with
> + * little-endian CPUs, it's disabled on Big Endian systems and SM720
> + * chips as a safety measure. Since I don't have monetary or hardware
> + * support from any company or OEMs, I don't have the hardware and
> + * it's completely untested. I should be also to purchase a Big Endian
> + * test platform and add proper support soon. I still have to spend
> + * 200 USD+ to purchase this piece of 1998's hardware, yikes! If you
> + * have a Big-Endian platform with SM7xx available for testing, please
> + * send an E-mail to Tom, thanks!
> + */
comments in the code are usually used to increase the readability, and
I dont think adding this comment adds to the readibility. I also spent
personal money to get these hardwares but that was never added to any
commit message or comment. Please remove this comment.
> +#ifdef __BIG_ENDIAN
> + sfb->accel = false;
> + if (accel)
> + dev_info(&sfb->pdev->dev,
> + "2D acceleration is unsupported on Big Endian.\n");
> +#endif
> + if (!accel) {
> + sfb->accel = false;
> + dev_info(&sfb->pdev->dev,
> + "2D acceleration is disabled by the user.\n");
> + }
> +
> + /* reset 2D engine after a modesetting is mandatory */
> + smtcfb_reset_accel();
If it is a big endian, acceleration is disabled, but you are still
calling smtcfb_reset_accel() which will "enable Zoom Video Port,
2D Drawing Engine and Video Processor". Will that not create any problem
in a big endian system?
> + smtcfb_init_accel(sfb);
> }
>
> static int smtc_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
> @@ -1401,6 +1459,316 @@ static struct fb_ops smtcfb_ops = {
> .fb_write = smtcfb_write,
> };
>
> +static int smtcfb_wait(struct smtcfb_info *fb)
> +{
> + int i;
> + u8 reg;
> +
> + smtc_dprr(DPR_DE_CTRL);
> + for (i = 0; i < 10000; i++) {
> + reg = smtc_seqr(SCR_DE_STATUS);
> + if ((reg & SCR_DE_STATUS_MASK) == SCR_DE_ENGINE_IDLE)
> + return 0;
> + udelay(1);
> + }
> + dev_err(&fb->pdev->dev, "2D engine hang detected!\n");
> + return -EBUSY;
> +}
> +
> +static void
> +smtcfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
> +{
> + u32 width = rect->width, height = rect->height;
> + u32 dx = rect->dx, dy = rect->dy;
> + u32 color;
> +
> + struct smtcfb_info *sfb = info->par;
> +
> + if (unlikely(info->state != FBINFO_STATE_RUNNING))
> + return;
Did you measure the performance difference with and without "unlikely"?
Quoting GregKH from https://lkml.org/lkml/2018/11/7/36
"don't do stuff like this unless you can actually measure the
difference".
> +
<snip>
> + * & HIGH with 0xffffffff (all ones, and we have already set
> + * that in smtcfb_init_accel). Since the color of this mono
> + * pattern is controlled by DPR_FG_COLOR, BITBLTing it with
> + * ROP_COPY is effectively a rectfill().
> + */
> + smtc_dprw(DPR_FG_COLOR, color);
> + smtc_dprw(DPR_DST_COORDS, DPR_COORDS(dx, dy));
I will need some more time to verify all these and other registers with
the datasheet and test again.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 3/7] fbdev: sm712fb: support 2D acceleration on SM712 w/ Little-Endian CPU.
@ 2019-03-31 18:09 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 18:09 UTC (permalink / raw)
To: Yifeng Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Fri, Mar 22, 2019 at 01:17:55PM +0800, Yifeng Li wrote:
> Previously, in staging/sm7xxfb (now fbdev/sm712fb), 2D acceleration
> was implemented, but after its submission, a critical bug that causes
> total system hang was discovered, as a stopgap measure, 2D ops was
> completele removed in commit 3af805735a25 ("staging: sm7xx: remove the
> buggy 2D acceleration support") and never implemented again.
>
> It created a massive usability problem - on YeeLoong 8089, a notable
> MIPS platform which uses SM712 - even scrolling a single line of text
> on the console required an unaccelerated screen redraw, running "dmesg"
> typically takes 8-11 seconds, and absurdly, printf(), became a significant
> performance bottleneck that slows down GCC and "make", make the computer
> largely unusable.
>
> So I decided to take a look. Most of the my actual development was done
> in 2014 in a personal out-of-tree driver, I did not mainline it because
> 2D acceleration was not working properly in 24-bit color. I discovered
> the solution in early 2019 and now it's ready to be mainlined.
>
> This commit reimplements the 2D acceleration for sm712fb. Unlike the
> original implementation, which was messy and unnecessarily complicated
> by calling a 2D acceleration wrapper file with many unneeded functions,
> this is a minimum and (relatively) clean implementation. My tests have
> shown that running "dmesg" only takes 0.9 seconds, a performance boost
> of 950%. System hangs did not occur in my tests.
I didnot notice any performace improvement in my system. Infact, I have
never seen the performance problem that you mentioned. But, it will be
good to have the 2D feature back again.
>
<snip>
> + */
> + smtcfb_reset_accel();
> +
> smtc_set_timing(sfb);
> +
> + /*
> + * Currently, 2D acceleration is only supported on SM712 with
> + * little-endian CPUs, it's disabled on Big Endian systems and SM720
> + * chips as a safety measure. Since I don't have monetary or hardware
> + * support from any company or OEMs, I don't have the hardware and
> + * it's completely untested. I should be also to purchase a Big Endian
> + * test platform and add proper support soon. I still have to spend
> + * 200 USD+ to purchase this piece of 1998's hardware, yikes! If you
> + * have a Big-Endian platform with SM7xx available for testing, please
> + * send an E-mail to Tom, thanks!
> + */
comments in the code are usually used to increase the readability, and
I dont think adding this comment adds to the readibility. I also spent
personal money to get these hardwares but that was never added to any
commit message or comment. Please remove this comment.
> +#ifdef __BIG_ENDIAN
> + sfb->accel = false;
> + if (accel)
> + dev_info(&sfb->pdev->dev,
> + "2D acceleration is unsupported on Big Endian.\n");
> +#endif
> + if (!accel) {
> + sfb->accel = false;
> + dev_info(&sfb->pdev->dev,
> + "2D acceleration is disabled by the user.\n");
> + }
> +
> + /* reset 2D engine after a modesetting is mandatory */
> + smtcfb_reset_accel();
If it is a big endian, acceleration is disabled, but you are still
calling smtcfb_reset_accel() which will "enable Zoom Video Port,
2D Drawing Engine and Video Processor". Will that not create any problem
in a big endian system?
> + smtcfb_init_accel(sfb);
> }
>
> static int smtc_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
> @@ -1401,6 +1459,316 @@ static struct fb_ops smtcfb_ops = {
> .fb_write = smtcfb_write,
> };
>
> +static int smtcfb_wait(struct smtcfb_info *fb)
> +{
> + int i;
> + u8 reg;
> +
> + smtc_dprr(DPR_DE_CTRL);
> + for (i = 0; i < 10000; i++) {
> + reg = smtc_seqr(SCR_DE_STATUS);
> + if ((reg & SCR_DE_STATUS_MASK) = SCR_DE_ENGINE_IDLE)
> + return 0;
> + udelay(1);
> + }
> + dev_err(&fb->pdev->dev, "2D engine hang detected!\n");
> + return -EBUSY;
> +}
> +
> +static void
> +smtcfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
> +{
> + u32 width = rect->width, height = rect->height;
> + u32 dx = rect->dx, dy = rect->dy;
> + u32 color;
> +
> + struct smtcfb_info *sfb = info->par;
> +
> + if (unlikely(info->state != FBINFO_STATE_RUNNING))
> + return;
Did you measure the performance difference with and without "unlikely"?
Quoting GregKH from https://lkml.org/lkml/2018/11/7/36
"don't do stuff like this unless you can actually measure the
difference".
> +
<snip>
> + * & HIGH with 0xffffffff (all ones, and we have already set
> + * that in smtcfb_init_accel). Since the color of this mono
> + * pattern is controlled by DPR_FG_COLOR, BITBLTing it with
> + * ROP_COPY is effectively a rectfill().
> + */
> + smtc_dprw(DPR_FG_COLOR, color);
> + smtc_dprw(DPR_DST_COORDS, DPR_COORDS(dx, dy));
I will need some more time to verify all these and other registers with
the datasheet and test again.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 4/7] fbdev: sm712fb: add 32-bit color modes, drops some other modes.
2019-03-22 5:17 ` Yifeng Li
@ 2019-03-31 18:33 ` Sudip Mukherjee
-1 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 18:33 UTC (permalink / raw)
To: Yifeng Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Fri, Mar 22, 2019 at 01:17:56PM +0800, Yifeng Li wrote:
> The modesetting in sm712fb is an ugly hack. First, all the registers
> are programmed by hardcoded register arrays, which makes it difficult
> to support different variations of color depths, refresh rates, CRT/LCD
> panel, etc of the same resolution. Second, it means the standard
> fb_find_mode() cannot be used and a confusing non-standard "vga="
> parameter is needed. Third, there's only minimum differences between
> some modes, yet around 70 lines of code and 100 registers are needed to
> be indepentently specified for each mode. Fourth, the register between
> some modes are inconsistent: the register configuration of different
> color depths in 640 x 480 modes are identical, but for 800 x 600 modes
> it's completely different. Also, some modes can drive the LCD panel
> properly yet some other modes will only show a white screen of death on
> the LCD panel. Fifth, some modes, such as 32-bit color modes, are
> supported but never listed, and some modes are listed, such as 1280x1024
> modes, but never supported by the register configuration arrays. And
> some modes are partially implemented but neither listed nor supported,
> i.e. the 8-bit color modes.
>
> Fixing these problems requires a complete rewrite of modesetting code,
> which is well-beyond my motivation. This commit only perform a minimum
> cleanup:
>
> 1. Remove the 320 x 240 register settings, since there are never listed
> and never actually being used. This resolution is not even supported by
> vesafb, so it's safe to assume that no one is using such hardware. Future
> developers who needs them can simply recover them from the git history.
Why are you removing existing functionality from the driver? These are
supported but were never listed so could not be used. I think you can
just add these to vesa_mode_table[] and they can be used. I have an old
CRT in India which supports 320x240 mode and can test there when I am
there next.
>
> 2. Remove 1280x1024 modes and 8-bit color modes. They are listed but never
> supported by the register array. So it doesn't work in the beginning, and
> only gives a black screen.
>
> 3. Add 32-bit color modes. They are supported but never listed, and are
> useful to some users.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> ---
> drivers/video/fbdev/sm712fb.c | 158 +++++++---------------------------
> 1 file changed, 31 insertions(+), 127 deletions(-)
>
> diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
> index 75d60ea63883..5e887653ef5d 100644
> --- a/drivers/video/fbdev/sm712fb.c
> +++ b/drivers/video/fbdev/sm712fb.c
> @@ -118,25 +118,48 @@ struct vesa_mode {
> };
>
> static const struct vesa_mode vesa_mode_table[] = {
> - {"0x301", 640, 480, 8},
> - {"0x303", 800, 600, 8},
> - {"0x305", 1024, 768, 8},
> - {"0x307", 1280, 1024, 8},
> -
> {"0x311", 640, 480, 16},
> {"0x314", 800, 600, 16},
> {"0x317", 1024, 768, 16},
> - {"0x31A", 1280, 1024, 16},
>
> {"0x312", 640, 480, 24},
> {"0x315", 800, 600, 24},
> {"0x318", 1024, 768, 24},
> - {"0x31B", 1280, 1024, 24},
> +
> + {"0x329", 640, 480, 32},
> + {"0x32e", 800, 600, 32},
> + {"0x338", 1024, 768, 32},
> };
>
> /**********************************************************************
> SM712 Mode table.
> - **********************************************************************/
> +
> + The modesetting in sm712fb is an ugly hack. First, all the registers
> + are programmed by hardcoded register arrays, which makes it difficult
> + to support different variations of color depths, refresh rates, CRT/LCD
> + panel, etc of the same resolution. Second, it means the standard
> + fb_find_mode() cannot be used and a confusing non-standard "vga="
> + parameter is needed. Third, there's only minimum differences between
> + some modes, yet around 70 lines of code and 100 registers are needed to
> + be indepentently specified for each mode. Fourth, the register between
> + some modes are inconsistent: the register configuration of different
> + color depths in 640 x 480 modes are identical, but for 800 x 600 modes
> + it's completely different. Also, some modes can drive the LCD panel
> + properly yet some other modes will only show a white screen of death on
> + the LCD panel. Fifth, there is a specific hack for Lemote Loongson 8089D
> + laptop, the 1024x768, 16-bit color mode was modified to drive its LCD panel
> + and changed to 1024x600, but the original mode was not preserved, so
> + 1024x768 16-bit color mode is completely unsupported. And previously,
> + some modes are listed, such as 1280x1024 modes, but never supported by
> + the register configuration arrays, so they are now removed. And some modes
> + are partially implemented but neither listed nor supported, i.e. the 8-bit
> + color modes, so they have been removed from vesa_mode_table, too.
I think this comment sounds more like a commit message instead of a
code comment. Does this improve readability?
> +
> + I'm not the original author of the code, fixing these problems requires a
> + complete rewrite of modesetting code, which is well-beyond my motivation.
> +
> + See Documentation/fb/sm712fb.txt for more information.
> +**********************************************************************/
Is this needed? Many of the commits of the driver are done by people who
are not the original author.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 4/7] fbdev: sm712fb: add 32-bit color modes, drops some other modes.
@ 2019-03-31 18:33 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 18:33 UTC (permalink / raw)
To: Yifeng Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Fri, Mar 22, 2019 at 01:17:56PM +0800, Yifeng Li wrote:
> The modesetting in sm712fb is an ugly hack. First, all the registers
> are programmed by hardcoded register arrays, which makes it difficult
> to support different variations of color depths, refresh rates, CRT/LCD
> panel, etc of the same resolution. Second, it means the standard
> fb_find_mode() cannot be used and a confusing non-standard "vga="
> parameter is needed. Third, there's only minimum differences between
> some modes, yet around 70 lines of code and 100 registers are needed to
> be indepentently specified for each mode. Fourth, the register between
> some modes are inconsistent: the register configuration of different
> color depths in 640 x 480 modes are identical, but for 800 x 600 modes
> it's completely different. Also, some modes can drive the LCD panel
> properly yet some other modes will only show a white screen of death on
> the LCD panel. Fifth, some modes, such as 32-bit color modes, are
> supported but never listed, and some modes are listed, such as 1280x1024
> modes, but never supported by the register configuration arrays. And
> some modes are partially implemented but neither listed nor supported,
> i.e. the 8-bit color modes.
>
> Fixing these problems requires a complete rewrite of modesetting code,
> which is well-beyond my motivation. This commit only perform a minimum
> cleanup:
>
> 1. Remove the 320 x 240 register settings, since there are never listed
> and never actually being used. This resolution is not even supported by
> vesafb, so it's safe to assume that no one is using such hardware. Future
> developers who needs them can simply recover them from the git history.
Why are you removing existing functionality from the driver? These are
supported but were never listed so could not be used. I think you can
just add these to vesa_mode_table[] and they can be used. I have an old
CRT in India which supports 320x240 mode and can test there when I am
there next.
>
> 2. Remove 1280x1024 modes and 8-bit color modes. They are listed but never
> supported by the register array. So it doesn't work in the beginning, and
> only gives a black screen.
>
> 3. Add 32-bit color modes. They are supported but never listed, and are
> useful to some users.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> ---
> drivers/video/fbdev/sm712fb.c | 158 +++++++---------------------------
> 1 file changed, 31 insertions(+), 127 deletions(-)
>
> diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
> index 75d60ea63883..5e887653ef5d 100644
> --- a/drivers/video/fbdev/sm712fb.c
> +++ b/drivers/video/fbdev/sm712fb.c
> @@ -118,25 +118,48 @@ struct vesa_mode {
> };
>
> static const struct vesa_mode vesa_mode_table[] = {
> - {"0x301", 640, 480, 8},
> - {"0x303", 800, 600, 8},
> - {"0x305", 1024, 768, 8},
> - {"0x307", 1280, 1024, 8},
> -
> {"0x311", 640, 480, 16},
> {"0x314", 800, 600, 16},
> {"0x317", 1024, 768, 16},
> - {"0x31A", 1280, 1024, 16},
>
> {"0x312", 640, 480, 24},
> {"0x315", 800, 600, 24},
> {"0x318", 1024, 768, 24},
> - {"0x31B", 1280, 1024, 24},
> +
> + {"0x329", 640, 480, 32},
> + {"0x32e", 800, 600, 32},
> + {"0x338", 1024, 768, 32},
> };
>
> /**********************************************************************
> SM712 Mode table.
> - **********************************************************************/
> +
> + The modesetting in sm712fb is an ugly hack. First, all the registers
> + are programmed by hardcoded register arrays, which makes it difficult
> + to support different variations of color depths, refresh rates, CRT/LCD
> + panel, etc of the same resolution. Second, it means the standard
> + fb_find_mode() cannot be used and a confusing non-standard "vga="
> + parameter is needed. Third, there's only minimum differences between
> + some modes, yet around 70 lines of code and 100 registers are needed to
> + be indepentently specified for each mode. Fourth, the register between
> + some modes are inconsistent: the register configuration of different
> + color depths in 640 x 480 modes are identical, but for 800 x 600 modes
> + it's completely different. Also, some modes can drive the LCD panel
> + properly yet some other modes will only show a white screen of death on
> + the LCD panel. Fifth, there is a specific hack for Lemote Loongson 8089D
> + laptop, the 1024x768, 16-bit color mode was modified to drive its LCD panel
> + and changed to 1024x600, but the original mode was not preserved, so
> + 1024x768 16-bit color mode is completely unsupported. And previously,
> + some modes are listed, such as 1280x1024 modes, but never supported by
> + the register configuration arrays, so they are now removed. And some modes
> + are partially implemented but neither listed nor supported, i.e. the 8-bit
> + color modes, so they have been removed from vesa_mode_table, too.
I think this comment sounds more like a commit message instead of a
code comment. Does this improve readability?
> +
> + I'm not the original author of the code, fixing these problems requires a
> + complete rewrite of modesetting code, which is well-beyond my motivation.
> +
> + See Documentation/fb/sm712fb.txt for more information.
> +**********************************************************************/
Is this needed? Many of the commits of the driver are done by people who
are not the original author.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 5/7] Documentation: fb: sm712fb: add information mainly about 2D.
2019-03-22 5:17 ` Yifeng Li
(?)
@ 2019-03-31 18:54 ` Sudip Mukherjee
-1 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 18:54 UTC (permalink / raw)
To: Yifeng Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Fri, Mar 22, 2019 at 01:17:57PM +0800, Yifeng Li wrote:
> This commits add information about 32-bit color, 2D acceleration,
> as well as adding additional, general information about the hardware
> and many existing problems of the sm712fb driver.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> ---
> Documentation/fb/sm712fb.txt | 123 +++++++++++++++++++++++++++++++----
> 1 file changed, 110 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/fb/sm712fb.txt b/Documentation/fb/sm712fb.txt
> index c388442edf51..906b48aa40e4 100644
> --- a/Documentation/fb/sm712fb.txt
> +++ b/Documentation/fb/sm712fb.txt
> @@ -1,31 +1,128 @@
> What is sm712fb?
> =================
>
> -This is a graphics framebuffer driver for Silicon Motion SM712 based processors.
> +"sm712fb" is a graphics framebuffer driver for Silicon Motion SM710 (LynxEM),
> +SM712 (LynxEM+), and SM720 (Lynx3DM, Lynx3DM+, aka. LynxEM4+) series of
> +video controllers. This series of video controller is a legacy from ~1998,
> +and was used on many classic, "prehistoric" laptops from 1998-2004, such as
> +IBM Thinkpad S30 and 240X. It was also used on some servers, industrial
> +computers, x86 and non-x86 embedded devices where only basic graphics was
> +needed.
I think this is wrong. Loongson 3A Notebook was released around 2011-2012
and had SM712.
> +
> +Notably, Lemote YeeLoong 8089, a MIPS laptop based on the Chinese Loongson
<snip>
> + * More VGA modes.
> * dual-head support
> + * hardware cursor support
> +
> +The first feature is planned to be implemented soon, but the maintainer
> +does not receive any monetary or hardware support from any company or OEMs,
> +and he has to purchase a test platform personally. The 1998's hardware
> +still costs 200 USD+, so don't expected an ETA. If you have a Big-Endian
> +platform and willing to help testing, please contact the maintainer, thanks!
I am not sure why will you want to mention about monetary or hardware
support. Maintainers are supposed to work voluntarily.
> +
> +Other VGA modes, dual-head, or hardware cursor support should be possible to
> +implement, but parts of the code must be rewritten, and there's little demand
> +for them on this legacy (retro?) platform, so there's no plan to implement them.
> +If you have a genuine need for them, please contact the maintainers.
If there is any need for new features then I think the plan should be to
make a drm driver.
> +
> +Maintainers
> +================
> +
> +This driver is maintained by
> +
> + * Tom Li <tomli@tomli.me>
??
I didn't know this. MAINTAINERS file doesnot say so.
> + * Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> + * Teddy Wang <teddy.wang@siliconmotion.com>
> +
> +Tom Li was the last contributor of this driver who implemented 2D acceleration,
> +and is the main author of this documentation, please send any bug reports or
> +requests to Tom, but don't forget to CC other maintainers as well to make everyone
> +be informed.
There is a MAINTAINERS file to list the maintainers. There is no need to
add that in documentation.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 5/7] Documentation: fb: sm712fb: add information mainly about 2D.
@ 2019-03-31 18:54 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 18:54 UTC (permalink / raw)
To: Yifeng Li
Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Teddy Wang, dri-devel,
linux-kernel
On Fri, Mar 22, 2019 at 01:17:57PM +0800, Yifeng Li wrote:
> This commits add information about 32-bit color, 2D acceleration,
> as well as adding additional, general information about the hardware
> and many existing problems of the sm712fb driver.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> ---
> Documentation/fb/sm712fb.txt | 123 +++++++++++++++++++++++++++++++----
> 1 file changed, 110 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/fb/sm712fb.txt b/Documentation/fb/sm712fb.txt
> index c388442edf51..906b48aa40e4 100644
> --- a/Documentation/fb/sm712fb.txt
> +++ b/Documentation/fb/sm712fb.txt
> @@ -1,31 +1,128 @@
> What is sm712fb?
> ========>
> -This is a graphics framebuffer driver for Silicon Motion SM712 based processors.
> +"sm712fb" is a graphics framebuffer driver for Silicon Motion SM710 (LynxEM),
> +SM712 (LynxEM+), and SM720 (Lynx3DM, Lynx3DM+, aka. LynxEM4+) series of
> +video controllers. This series of video controller is a legacy from ~1998,
> +and was used on many classic, "prehistoric" laptops from 1998-2004, such as
> +IBM Thinkpad S30 and 240X. It was also used on some servers, industrial
> +computers, x86 and non-x86 embedded devices where only basic graphics was
> +needed.
I think this is wrong. Loongson 3A Notebook was released around 2011-2012
and had SM712.
> +
> +Notably, Lemote YeeLoong 8089, a MIPS laptop based on the Chinese Loongson
<snip>
> + * More VGA modes.
> * dual-head support
> + * hardware cursor support
> +
> +The first feature is planned to be implemented soon, but the maintainer
> +does not receive any monetary or hardware support from any company or OEMs,
> +and he has to purchase a test platform personally. The 1998's hardware
> +still costs 200 USD+, so don't expected an ETA. If you have a Big-Endian
> +platform and willing to help testing, please contact the maintainer, thanks!
I am not sure why will you want to mention about monetary or hardware
support. Maintainers are supposed to work voluntarily.
> +
> +Other VGA modes, dual-head, or hardware cursor support should be possible to
> +implement, but parts of the code must be rewritten, and there's little demand
> +for them on this legacy (retro?) platform, so there's no plan to implement them.
> +If you have a genuine need for them, please contact the maintainers.
If there is any need for new features then I think the plan should be to
make a drm driver.
> +
> +Maintainers
> +========
> +
> +This driver is maintained by
> +
> + * Tom Li <tomli@tomli.me>
??
I didn't know this. MAINTAINERS file doesnot say so.
> + * Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> + * Teddy Wang <teddy.wang@siliconmotion.com>
> +
> +Tom Li was the last contributor of this driver who implemented 2D acceleration,
> +and is the main author of this documentation, please send any bug reports or
> +requests to Tom, but don't forget to CC other maintainers as well to make everyone
> +be informed.
There is a MAINTAINERS file to list the maintainers. There is no need to
add that in documentation.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 5/7] Documentation: fb: sm712fb: add information mainly about 2D.
@ 2019-03-31 18:54 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 18:54 UTC (permalink / raw)
To: Yifeng Li
Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Teddy Wang, dri-devel,
linux-kernel
On Fri, Mar 22, 2019 at 01:17:57PM +0800, Yifeng Li wrote:
> This commits add information about 32-bit color, 2D acceleration,
> as well as adding additional, general information about the hardware
> and many existing problems of the sm712fb driver.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> ---
> Documentation/fb/sm712fb.txt | 123 +++++++++++++++++++++++++++++++----
> 1 file changed, 110 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/fb/sm712fb.txt b/Documentation/fb/sm712fb.txt
> index c388442edf51..906b48aa40e4 100644
> --- a/Documentation/fb/sm712fb.txt
> +++ b/Documentation/fb/sm712fb.txt
> @@ -1,31 +1,128 @@
> What is sm712fb?
> =================
>
> -This is a graphics framebuffer driver for Silicon Motion SM712 based processors.
> +"sm712fb" is a graphics framebuffer driver for Silicon Motion SM710 (LynxEM),
> +SM712 (LynxEM+), and SM720 (Lynx3DM, Lynx3DM+, aka. LynxEM4+) series of
> +video controllers. This series of video controller is a legacy from ~1998,
> +and was used on many classic, "prehistoric" laptops from 1998-2004, such as
> +IBM Thinkpad S30 and 240X. It was also used on some servers, industrial
> +computers, x86 and non-x86 embedded devices where only basic graphics was
> +needed.
I think this is wrong. Loongson 3A Notebook was released around 2011-2012
and had SM712.
> +
> +Notably, Lemote YeeLoong 8089, a MIPS laptop based on the Chinese Loongson
<snip>
> + * More VGA modes.
> * dual-head support
> + * hardware cursor support
> +
> +The first feature is planned to be implemented soon, but the maintainer
> +does not receive any monetary or hardware support from any company or OEMs,
> +and he has to purchase a test platform personally. The 1998's hardware
> +still costs 200 USD+, so don't expected an ETA. If you have a Big-Endian
> +platform and willing to help testing, please contact the maintainer, thanks!
I am not sure why will you want to mention about monetary or hardware
support. Maintainers are supposed to work voluntarily.
> +
> +Other VGA modes, dual-head, or hardware cursor support should be possible to
> +implement, but parts of the code must be rewritten, and there's little demand
> +for them on this legacy (retro?) platform, so there's no plan to implement them.
> +If you have a genuine need for them, please contact the maintainers.
If there is any need for new features then I think the plan should be to
make a drm driver.
> +
> +Maintainers
> +================
> +
> +This driver is maintained by
> +
> + * Tom Li <tomli@tomli.me>
??
I didn't know this. MAINTAINERS file doesnot say so.
> + * Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> + * Teddy Wang <teddy.wang@siliconmotion.com>
> +
> +Tom Li was the last contributor of this driver who implemented 2D acceleration,
> +and is the main author of this documentation, please send any bug reports or
> +requests to Tom, but don't forget to CC other maintainers as well to make everyone
> +be informed.
There is a MAINTAINERS file to list the maintainers. There is no need to
add that in documentation.
--
Regards
Sudip
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 6/7] fbdev: sm712fb: Kconfig: add information about docs.
2019-03-22 5:17 ` Yifeng Li
@ 2019-03-31 19:01 ` Sudip Mukherjee
-1 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 19:01 UTC (permalink / raw)
To: Yifeng Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Fri, Mar 22, 2019 at 01:17:58PM +0800, Yifeng Li wrote:
> This commits update the Kconfig description of sm712fb,
> and inform the user about the existence of documentation.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> ---
> Documentation/fb/sm712fb.txt | 44 ++++++++++++++++++++----------------
> drivers/video/fbdev/Kconfig | 4 ++++
> 2 files changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/fb/sm712fb.txt b/Documentation/fb/sm712fb.txt
> index 906b48aa40e4..c9a8fe059ac7 100644
> --- a/Documentation/fb/sm712fb.txt
> +++ b/Documentation/fb/sm712fb.txt
> @@ -18,9 +18,10 @@ and it was also an inexpensive platform for non-x86 hobbyists to explore.
> How to use it?
> ==============
>
> -You should not compile-in vesafb, since SM7xx can be used in a VGA
> -compatible mode, resulting conflicts with this driver. In addition,
> -the VGA compatible mode was never tested by the maintainers.
> +SM7xx can be used in a VGA compatible mode and a native framebuffer mode,
> +this driver is a native driver. By default, it has no problem taking over
> +the VGA driver automatically, but you should not attempt to use both at
> +the same time.
You added this in the documentation with just the previous patch.
>
<snip>
>
> 2D acceleration
> ==============
> @@ -97,7 +103,7 @@ Missing Features
>
> The following features are not implemented in this driver,
>
> - * 2D acceleration on SM720 and Big-Endian CPUs.
> + * 2D acceleration on Big-Endian CPUs.
> * More VGA modes.
> * dual-head support
> * hardware cursor support
You might want to squash the two patches for documentation instead of
having a separate patch to fix what you added in a previous patch.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 6/7] fbdev: sm712fb: Kconfig: add information about docs.
@ 2019-03-31 19:01 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 19:01 UTC (permalink / raw)
To: Yifeng Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Fri, Mar 22, 2019 at 01:17:58PM +0800, Yifeng Li wrote:
> This commits update the Kconfig description of sm712fb,
> and inform the user about the existence of documentation.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> ---
> Documentation/fb/sm712fb.txt | 44 ++++++++++++++++++++----------------
> drivers/video/fbdev/Kconfig | 4 ++++
> 2 files changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/fb/sm712fb.txt b/Documentation/fb/sm712fb.txt
> index 906b48aa40e4..c9a8fe059ac7 100644
> --- a/Documentation/fb/sm712fb.txt
> +++ b/Documentation/fb/sm712fb.txt
> @@ -18,9 +18,10 @@ and it was also an inexpensive platform for non-x86 hobbyists to explore.
> How to use it?
> =======
>
> -You should not compile-in vesafb, since SM7xx can be used in a VGA
> -compatible mode, resulting conflicts with this driver. In addition,
> -the VGA compatible mode was never tested by the maintainers.
> +SM7xx can be used in a VGA compatible mode and a native framebuffer mode,
> +this driver is a native driver. By default, it has no problem taking over
> +the VGA driver automatically, but you should not attempt to use both at
> +the same time.
You added this in the documentation with just the previous patch.
>
<snip>
>
> 2D acceleration
> =======
> @@ -97,7 +103,7 @@ Missing Features
>
> The following features are not implemented in this driver,
>
> - * 2D acceleration on SM720 and Big-Endian CPUs.
> + * 2D acceleration on Big-Endian CPUs.
> * More VGA modes.
> * dual-head support
> * hardware cursor support
You might want to squash the two patches for documentation instead of
having a separate patch to fix what you added in a previous patch.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
2019-03-22 5:17 ` Yifeng Li
(?)
@ 2019-03-31 19:08 ` Sudip Mukherjee
-1 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 19:08 UTC (permalink / raw)
To: Yifeng Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Fri, Mar 22, 2019 at 01:17:59PM +0800, Yifeng Li wrote:
> I have working on the sm712fb driver for a while and have some familiarity
> with this hardware, I'll be helping working on and testing problems of this
> driver, so add myself to the MAINTAINERS file.
Technically I donot have any problem with this, you seem to know more
about SM712 than I know. But Teddy Wang is also an existing maintainer
and I think there should be an ack from him before this is accepted.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dce5c099f43c..3f6686f0f020 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13871,6 +13871,7 @@ F: drivers/input/touchscreen/silead.c
> F: drivers/platform/x86/touchscreen_dmi.c
>
> SILICON MOTION SM712 FRAME BUFFER DRIVER
> +M: Tom Li <tomli@tomli.me>
Sorry, I am confused. Is your name "Tom Li"? Then why is your
Signed-off-by: and From: name different?
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
@ 2019-03-31 19:08 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 19:08 UTC (permalink / raw)
To: Yifeng Li
Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Teddy Wang, dri-devel,
linux-kernel
On Fri, Mar 22, 2019 at 01:17:59PM +0800, Yifeng Li wrote:
> I have working on the sm712fb driver for a while and have some familiarity
> with this hardware, I'll be helping working on and testing problems of this
> driver, so add myself to the MAINTAINERS file.
Technically I donot have any problem with this, you seem to know more
about SM712 than I know. But Teddy Wang is also an existing maintainer
and I think there should be an ack from him before this is accepted.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dce5c099f43c..3f6686f0f020 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13871,6 +13871,7 @@ F: drivers/input/touchscreen/silead.c
> F: drivers/platform/x86/touchscreen_dmi.c
>
> SILICON MOTION SM712 FRAME BUFFER DRIVER
> +M: Tom Li <tomli@tomli.me>
Sorry, I am confused. Is your name "Tom Li"? Then why is your
Signed-off-by: and From: name different?
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
@ 2019-03-31 19:08 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-03-31 19:08 UTC (permalink / raw)
To: Yifeng Li
Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Teddy Wang, dri-devel,
linux-kernel
On Fri, Mar 22, 2019 at 01:17:59PM +0800, Yifeng Li wrote:
> I have working on the sm712fb driver for a while and have some familiarity
> with this hardware, I'll be helping working on and testing problems of this
> driver, so add myself to the MAINTAINERS file.
Technically I donot have any problem with this, you seem to know more
about SM712 than I know. But Teddy Wang is also an existing maintainer
and I think there should be an ack from him before this is accepted.
>
> Signed-off-by: Yifeng Li <tomli@tomli.me>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dce5c099f43c..3f6686f0f020 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13871,6 +13871,7 @@ F: drivers/input/touchscreen/silead.c
> F: drivers/platform/x86/touchscreen_dmi.c
>
> SILICON MOTION SM712 FRAME BUFFER DRIVER
> +M: Tom Li <tomli@tomli.me>
Sorry, I am confused. Is your name "Tom Li"? Then why is your
Signed-off-by: and From: name different?
--
Regards
Sudip
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 2/7] fbdev: sm712fb: add 2D-related I/O headers and functions.
2019-03-31 17:25 ` Sudip Mukherjee
@ 2019-04-01 16:04 ` Tom Li
-1 siblings, 0 replies; 60+ messages in thread
From: Tom Li @ 2019-04-01 16:04 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Sun, Mar 31, 2019 at 06:25:58PM +0100, Sudip Mukherjee wrote:
> On Fri, Mar 22, 2019 at 01:17:54PM +0800, Yifeng Li wrote:
> > +#define DE_CTRL_COMMAND_SHIFT 16
> > +#define DE_CTRL_COMMAND_BITBLT 0x00
> > +#define DE_CTRL_COMMAND_SOLIDFILL 0x01
> > +#define DE_CTRL_COMMAND_HOSTWRITE 0x08
> > +#define DE_CTRL_ROP2_SELECT 0x00008000
> > +#define DE_CTRL_ROP2_SRC_IS_PATTERN 0x00004000
> > +#define DE_CTRL_ROP2_SHIFT 0
> > +#define DE_CTRL_ROP2_COPY 0x0c
> > +#define DE_CTRL_HOST_SHIFT 22
> > +#define DE_CTRL_HOST_SRC_IS_MONO 0x01
> > +#define DE_CTRL_FORMAT_XY 0x00
> > +#define DE_CTRL_FORMAT_24BIT 0x30
>
> Please fix the alignment. Some of them are right aligned and some are left.
Noted. It will be fixed in PATCH v3.
Thanks,
Tom Li
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 2/7] fbdev: sm712fb: add 2D-related I/O headers and functions.
@ 2019-04-01 16:04 ` Tom Li
0 siblings, 0 replies; 60+ messages in thread
From: Tom Li @ 2019-04-01 16:04 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Sun, Mar 31, 2019 at 06:25:58PM +0100, Sudip Mukherjee wrote:
> On Fri, Mar 22, 2019 at 01:17:54PM +0800, Yifeng Li wrote:
> > +#define DE_CTRL_COMMAND_SHIFT 16
> > +#define DE_CTRL_COMMAND_BITBLT 0x00
> > +#define DE_CTRL_COMMAND_SOLIDFILL 0x01
> > +#define DE_CTRL_COMMAND_HOSTWRITE 0x08
> > +#define DE_CTRL_ROP2_SELECT 0x00008000
> > +#define DE_CTRL_ROP2_SRC_IS_PATTERN 0x00004000
> > +#define DE_CTRL_ROP2_SHIFT 0
> > +#define DE_CTRL_ROP2_COPY 0x0c
> > +#define DE_CTRL_HOST_SHIFT 22
> > +#define DE_CTRL_HOST_SRC_IS_MONO 0x01
> > +#define DE_CTRL_FORMAT_XY 0x00
> > +#define DE_CTRL_FORMAT_24BIT 0x30
>
> Please fix the alignment. Some of them are right aligned and some are left.
Noted. It will be fixed in PATCH v3.
Thanks,
Tom Li
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 3/7] fbdev: sm712fb: support 2D acceleration on SM712 w/ Little-Endian CPU.
2019-03-31 18:09 ` Sudip Mukherjee
@ 2019-04-01 16:26 ` Tom Li
-1 siblings, 0 replies; 60+ messages in thread
From: Tom Li @ 2019-04-01 16:26 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Sun, Mar 31, 2019 at 07:09:47PM +0100, Sudip Mukherjee wrote:
> On Fri, Mar 22, 2019 at 01:17:55PM +0800, Yifeng Li wrote:
> I didnot notice any performace improvement in my system. Infact, I have
> never seen the performance problem that you mentioned. But, it will be
> good to have the 2D feature back again.
Are you using sm712fb under the Linux tty console? If you are using X,
Qt or DirectFB or something similar, these 2D code won't have any effect
at all, because they are only used by the Linux VT console internally.
Please try testing it under a Linux tty.
Another possibility is that you are using a high-performance CPU with
high clock/bus frequencies, which writing to the framebuffer directly
can break-even/outperform the 2D acceleration. But I'm not sure about
it. Please let me know more about your setup.
> If it is a big endian, acceleration is disabled, but you are still
> calling smtcfb_reset_accel() which will "enable Zoom Video Port,
> 2D Drawing Engine and Video Processor". Will that not create any problem
> in a big endian system?
Personally, I don't think there's an issue, because smtc_seqw() only does
8-bit I/O, and is known to be safe under Big-Endian.
But I agree, calling this function should be avoided since it's not tested.
> > + if (unlikely(info->state != FBINFO_STATE_RUNNING))
> > + return;
>
> Did you measure the performance difference with and without "unlikely"?
> Quoting GregKH from https://lkml.org/lkml/2018/11/7/36
> "don't do stuff like this unless you can actually measure the
> difference".
In the old days, these two macros are used liberally, everywhere. I learned
about them by reading drivers code in fbdev/. Thanks for informing me the
new policy regarding the use of them. I'll remove them.
> <snip>
> > + * & HIGH with 0xffffffff (all ones, and we have already set
> > + * that in smtcfb_init_accel). Since the color of this mono
> > + * pattern is controlled by DPR_FG_COLOR, BITBLTing it with
> > + * ROP_COPY is effectively a rectfill().
> > + */
> > + smtc_dprw(DPR_FG_COLOR, color);
> > + smtc_dprw(DPR_DST_COORDS, DPR_COORDS(dx, dy));
>
> I will need some more time to verify all these and other registers with
> the datasheet and test again.
No problem.
But again, the 2D code is only used by Linux VT code internally, if you are
not using VT console, the behavior cannot be tested properly. Are you testing
it under the Linux tty console? And does your dmesg log says, "2D acceleration
is enabled"?
Running the "fbtest" testsuit is also a good way to "smoke-test" the fbdev
code.
Thanks,
Tom Li
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 3/7] fbdev: sm712fb: support 2D acceleration on SM712 w/ Little-Endian CPU.
@ 2019-04-01 16:26 ` Tom Li
0 siblings, 0 replies; 60+ messages in thread
From: Tom Li @ 2019-04-01 16:26 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Sun, Mar 31, 2019 at 07:09:47PM +0100, Sudip Mukherjee wrote:
> On Fri, Mar 22, 2019 at 01:17:55PM +0800, Yifeng Li wrote:
> I didnot notice any performace improvement in my system. Infact, I have
> never seen the performance problem that you mentioned. But, it will be
> good to have the 2D feature back again.
Are you using sm712fb under the Linux tty console? If you are using X,
Qt or DirectFB or something similar, these 2D code won't have any effect
at all, because they are only used by the Linux VT console internally.
Please try testing it under a Linux tty.
Another possibility is that you are using a high-performance CPU with
high clock/bus frequencies, which writing to the framebuffer directly
can break-even/outperform the 2D acceleration. But I'm not sure about
it. Please let me know more about your setup.
> If it is a big endian, acceleration is disabled, but you are still
> calling smtcfb_reset_accel() which will "enable Zoom Video Port,
> 2D Drawing Engine and Video Processor". Will that not create any problem
> in a big endian system?
Personally, I don't think there's an issue, because smtc_seqw() only does
8-bit I/O, and is known to be safe under Big-Endian.
But I agree, calling this function should be avoided since it's not tested.
> > + if (unlikely(info->state != FBINFO_STATE_RUNNING))
> > + return;
>
> Did you measure the performance difference with and without "unlikely"?
> Quoting GregKH from https://lkml.org/lkml/2018/11/7/36
> "don't do stuff like this unless you can actually measure the
> difference".
In the old days, these two macros are used liberally, everywhere. I learned
about them by reading drivers code in fbdev/. Thanks for informing me the
new policy regarding the use of them. I'll remove them.
> <snip>
> > + * & HIGH with 0xffffffff (all ones, and we have already set
> > + * that in smtcfb_init_accel). Since the color of this mono
> > + * pattern is controlled by DPR_FG_COLOR, BITBLTing it with
> > + * ROP_COPY is effectively a rectfill().
> > + */
> > + smtc_dprw(DPR_FG_COLOR, color);
> > + smtc_dprw(DPR_DST_COORDS, DPR_COORDS(dx, dy));
>
> I will need some more time to verify all these and other registers with
> the datasheet and test again.
No problem.
But again, the 2D code is only used by Linux VT code internally, if you are
not using VT console, the behavior cannot be tested properly. Are you testing
it under the Linux tty console? And does your dmesg log says, "2D acceleration
is enabled"?
Running the "fbtest" testsuit is also a good way to "smoke-test" the fbdev
code.
Thanks,
Tom Li
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 4/7] fbdev: sm712fb: add 32-bit color modes, drops some other modes.
2019-03-31 18:33 ` Sudip Mukherjee
@ 2019-04-01 16:41 ` Tom Li
-1 siblings, 0 replies; 60+ messages in thread
From: Tom Li @ 2019-04-01 16:41 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Sun, Mar 31, 2019 at 07:33:33PM +0100, Sudip Mukherjee wrote:
> Why are you removing existing functionality from the driver? These are
> supported but were never listed so could not be used. I think you can
> just add these to vesa_mode_table[] and they can be used.
If there are some "functionalities" in a system, but they are never used,
even worse, they are programmed in a way that they cannot be used by any
user no matter what, meanwhile not a single user had filed a bug report
in the entire lifecycle of the program, then I'd call them "dead code",
that serves no useful purposes, rather than "functionalities". I think
most kernel developers can agree on this.
> I have an old CRT in India which supports 320x240 mode and can test there
> when I am there next.
Well... If someone (e.g. you) do see a need of this feature, then fixing
them (instead of removing them) becomes a reasonable choice.
Coincidentally, I've also purchased a video converter a few days ago. Please
allow some time for me to testing it, so I can see if they're working. If so,
I'll add them to the vesa_mode_table[] in PATCH v3.
> > /**********************************************************************
> > SM712 Mode table.
> > - **********************************************************************/
> > +
> > + The modesetting in sm712fb is an ugly hack. First, all the registers
> > + are programmed by hardcoded register arrays, which makes it difficult
> > + to support different variations of color depths, refresh rates, CRT/LCD
> > + panel, etc of the same resolution. Second, it means the standard
> > + fb_find_mode() cannot be used and a confusing non-standard "vga="
> > + parameter is needed. Third, there's only minimum differences between
> > + some modes, yet around 70 lines of code and 100 registers are needed to
> > + be indepentently specified for each mode. Fourth, the register between
> > + some modes are inconsistent: the register configuration of different
> > + color depths in 640 x 480 modes are identical, but for 800 x 600 modes
> > + it's completely different. Also, some modes can drive the LCD panel
> > + properly yet some other modes will only show a white screen of death on
> > + the LCD panel. Fifth, there is a specific hack for Lemote Loongson 8089D
> > + laptop, the 1024x768, 16-bit color mode was modified to drive its LCD panel
> > + and changed to 1024x600, but the original mode was not preserved, so
> > + 1024x768 16-bit color mode is completely unsupported. And previously,
> > + some modes are listed, such as 1280x1024 modes, but never supported by
> > + the register configuration arrays, so they are now removed. And some modes
> > + are partially implemented but neither listed nor supported, i.e. the 8-bit
> > + color modes, so they have been removed from vesa_mode_table, too.
>
> I think this comment sounds more like a commit message instead of a
> code comment. Does this improve readability?
Will remove, thanks.
>
> > +
> > + I'm not the original author of the code, fixing these problems requires a
> > + complete rewrite of modesetting code, which is well-beyond my motivation.
> > +
> > + See Documentation/fb/sm712fb.txt for more information.
> > +**********************************************************************/
>
> Is this needed? Many of the commits of the driver are done by people who
> are not the original author.
I'll reword it.
Thanks,
Tom Li
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 4/7] fbdev: sm712fb: add 32-bit color modes, drops some other modes.
@ 2019-04-01 16:41 ` Tom Li
0 siblings, 0 replies; 60+ messages in thread
From: Tom Li @ 2019-04-01 16:41 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Sun, Mar 31, 2019 at 07:33:33PM +0100, Sudip Mukherjee wrote:
> Why are you removing existing functionality from the driver? These are
> supported but were never listed so could not be used. I think you can
> just add these to vesa_mode_table[] and they can be used.
If there are some "functionalities" in a system, but they are never used,
even worse, they are programmed in a way that they cannot be used by any
user no matter what, meanwhile not a single user had filed a bug report
in the entire lifecycle of the program, then I'd call them "dead code",
that serves no useful purposes, rather than "functionalities". I think
most kernel developers can agree on this.
> I have an old CRT in India which supports 320x240 mode and can test there
> when I am there next.
Well... If someone (e.g. you) do see a need of this feature, then fixing
them (instead of removing them) becomes a reasonable choice.
Coincidentally, I've also purchased a video converter a few days ago. Please
allow some time for me to testing it, so I can see if they're working. If so,
I'll add them to the vesa_mode_table[] in PATCH v3.
> > /**********************************************************************
> > SM712 Mode table.
> > - **********************************************************************/
> > +
> > + The modesetting in sm712fb is an ugly hack. First, all the registers
> > + are programmed by hardcoded register arrays, which makes it difficult
> > + to support different variations of color depths, refresh rates, CRT/LCD
> > + panel, etc of the same resolution. Second, it means the standard
> > + fb_find_mode() cannot be used and a confusing non-standard "vga="
> > + parameter is needed. Third, there's only minimum differences between
> > + some modes, yet around 70 lines of code and 100 registers are needed to
> > + be indepentently specified for each mode. Fourth, the register between
> > + some modes are inconsistent: the register configuration of different
> > + color depths in 640 x 480 modes are identical, but for 800 x 600 modes
> > + it's completely different. Also, some modes can drive the LCD panel
> > + properly yet some other modes will only show a white screen of death on
> > + the LCD panel. Fifth, there is a specific hack for Lemote Loongson 8089D
> > + laptop, the 1024x768, 16-bit color mode was modified to drive its LCD panel
> > + and changed to 1024x600, but the original mode was not preserved, so
> > + 1024x768 16-bit color mode is completely unsupported. And previously,
> > + some modes are listed, such as 1280x1024 modes, but never supported by
> > + the register configuration arrays, so they are now removed. And some modes
> > + are partially implemented but neither listed nor supported, i.e. the 8-bit
> > + color modes, so they have been removed from vesa_mode_table, too.
>
> I think this comment sounds more like a commit message instead of a
> code comment. Does this improve readability?
Will remove, thanks.
>
> > +
> > + I'm not the original author of the code, fixing these problems requires a
> > + complete rewrite of modesetting code, which is well-beyond my motivation.
> > +
> > + See Documentation/fb/sm712fb.txt for more information.
> > +**********************************************************************/
>
> Is this needed? Many of the commits of the driver are done by people who
> are not the original author.
I'll reword it.
Thanks,
Tom Li
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 5/7] Documentation: fb: sm712fb: add information mainly about 2D.
2019-03-31 18:54 ` Sudip Mukherjee
@ 2019-04-01 17:30 ` Tom Li
-1 siblings, 0 replies; 60+ messages in thread
From: Tom Li @ 2019-04-01 17:30 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Sun, Mar 31, 2019 at 07:54:28PM +0100, Sudip Mukherjee wrote:
> On Fri, Mar 22, 2019 at 01:17:57PM +0800, Yifeng Li wrote:
> > +video controllers. This series of video controller is a legacy from ~1998,
> > +and was used on many classic, "prehistoric" laptops from 1998-2004, such as
> > +IBM Thinkpad S30 and 240X. It was also used on some servers, industrial
> > +computers, x86 and non-x86 embedded devices where only basic graphics was
> > +needed.
>
> I think this is wrong. Loongson 3A Notebook was released around 2011-2012
> and had SM712.
"This series of video controller is a legacy from ~1998 and was used on many
classic, prehistoric laptops from 1998-2004" is an objective fact. Even if
they have been used on newer hardware, it doesn't automatically make the original
statement false.
But I agree that the description gives incomplete information, I think this
paragraph should be reworded for clarity.
I would change the description to,
> "sm712fb" is a graphics framebuffer driver for Silicon Motion SM710 (LynxEM),
SM712 (LynxEM+), and SM720 (Lynx3DM, Lynx3DM+, aka. LynxEM4+) series of video
controllers.
> This series of video controllers is a legacy product from ~1998, they are
designed to be primarily used on low-power mobile systems running Windows 95/
98/NT/2000, some examples are HP OmniBook XE2 (2000), Panasonic TOUGHBook 28
(2002), FLORA 210W NL3 (2003), Sony Vaio VGN-U50 (2004) OQO Model 01 (2004).
> After 2004, they continued to be used on some non-x86 systems, including
PowerPC and MIPS. It also saw applications on embedded devices, servers,
industrial computers, embedded devices, where low-power operation and/or only
basic graphics was needed.
> Notably, Lemote YeeLoong 8089, a MIPS laptop based on the Chinese Loongson
[...]
I think it would be enough.
BTW, most Loongson 3A notebooks don't use SM712. I don't know that there are
Loongson 3A notebooks that are still using SM712 graphics chip, do you have one?
Could you tell me its model number?
> > +The first feature is planned to be implemented soon, but the maintainer
> > +does not receive any monetary or hardware support from any company or OEMs,
> > +and he has to purchase a test platform personally. The 1998's hardware
> > +still costs 200 USD+, so don't expected an ETA. If you have a Big-Endian
> > +platform and willing to help testing, please contact the maintainer, thanks!
>
> I am not sure why will you want to mention about monetary or hardware
> support. Maintainers are supposed to work voluntarily.
I agree, I will reword it.
> > +Other VGA modes, dual-head, or hardware cursor support should be possible to
> > +implement, but parts of the code must be rewritten, and there's little demand
> > +for them on this legacy (retro?) platform, so there's no plan to implement them.
> > +If you have a genuine need for them, please contact the maintainers.
>
> If there is any need for new features then I think the plan should be to
> make a drm driver.
That's the plan. I will reword.
> There is a MAINTAINERS file to list the maintainers. There is no need to
> add that in documentation.
I see.
Thanks,
Tom Li
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 5/7] Documentation: fb: sm712fb: add information mainly about 2D.
@ 2019-04-01 17:30 ` Tom Li
0 siblings, 0 replies; 60+ messages in thread
From: Tom Li @ 2019-04-01 17:30 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Sun, Mar 31, 2019 at 07:54:28PM +0100, Sudip Mukherjee wrote:
> On Fri, Mar 22, 2019 at 01:17:57PM +0800, Yifeng Li wrote:
> > +video controllers. This series of video controller is a legacy from ~1998,
> > +and was used on many classic, "prehistoric" laptops from 1998-2004, such as
> > +IBM Thinkpad S30 and 240X. It was also used on some servers, industrial
> > +computers, x86 and non-x86 embedded devices where only basic graphics was
> > +needed.
>
> I think this is wrong. Loongson 3A Notebook was released around 2011-2012
> and had SM712.
"This series of video controller is a legacy from ~1998 and was used on many
classic, prehistoric laptops from 1998-2004" is an objective fact. Even if
they have been used on newer hardware, it doesn't automatically make the original
statement false.
But I agree that the description gives incomplete information, I think this
paragraph should be reworded for clarity.
I would change the description to,
> "sm712fb" is a graphics framebuffer driver for Silicon Motion SM710 (LynxEM),
SM712 (LynxEM+), and SM720 (Lynx3DM, Lynx3DM+, aka. LynxEM4+) series of video
controllers.
> This series of video controllers is a legacy product from ~1998, they are
designed to be primarily used on low-power mobile systems running Windows 95/
98/NT/2000, some examples are HP OmniBook XE2 (2000), Panasonic TOUGHBook 28
(2002), FLORA 210W NL3 (2003), Sony Vaio VGN-U50 (2004) OQO Model 01 (2004).
> After 2004, they continued to be used on some non-x86 systems, including
PowerPC and MIPS. It also saw applications on embedded devices, servers,
industrial computers, embedded devices, where low-power operation and/or only
basic graphics was needed.
> Notably, Lemote YeeLoong 8089, a MIPS laptop based on the Chinese Loongson
[...]
I think it would be enough.
BTW, most Loongson 3A notebooks don't use SM712. I don't know that there are
Loongson 3A notebooks that are still using SM712 graphics chip, do you have one?
Could you tell me its model number?
> > +The first feature is planned to be implemented soon, but the maintainer
> > +does not receive any monetary or hardware support from any company or OEMs,
> > +and he has to purchase a test platform personally. The 1998's hardware
> > +still costs 200 USD+, so don't expected an ETA. If you have a Big-Endian
> > +platform and willing to help testing, please contact the maintainer, thanks!
>
> I am not sure why will you want to mention about monetary or hardware
> support. Maintainers are supposed to work voluntarily.
I agree, I will reword it.
> > +Other VGA modes, dual-head, or hardware cursor support should be possible to
> > +implement, but parts of the code must be rewritten, and there's little demand
> > +for them on this legacy (retro?) platform, so there's no plan to implement them.
> > +If you have a genuine need for them, please contact the maintainers.
>
> If there is any need for new features then I think the plan should be to
> make a drm driver.
That's the plan. I will reword.
> There is a MAINTAINERS file to list the maintainers. There is no need to
> add that in documentation.
I see.
Thanks,
Tom Li
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 6/7] fbdev: sm712fb: Kconfig: add information about docs.
2019-03-31 19:01 ` Sudip Mukherjee
@ 2019-04-01 17:33 ` Tom Li
-1 siblings, 0 replies; 60+ messages in thread
From: Tom Li @ 2019-04-01 17:33 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Sun, Mar 31, 2019 at 08:01:18PM +0100, Sudip Mukherjee wrote:
> On Fri, Mar 22, 2019 at 01:17:58PM +0800, Yifeng Li wrote:
> > @@ -18,9 +18,10 @@ and it was also an inexpensive platform for non-x86 hobbyists to explore.
> > How to use it?
> > ==============
> >
> > -You should not compile-in vesafb, since SM7xx can be used in a VGA
> > -compatible mode, resulting conflicts with this driver. In addition,
> > -the VGA compatible mode was never tested by the maintainers.
> > +SM7xx can be used in a VGA compatible mode and a native framebuffer mode,
> > +this driver is a native driver. By default, it has no problem taking over
> > +the VGA driver automatically, but you should not attempt to use both at
> > +the same time.
>
> You added this in the documentation with just the previous patch.
Oops, this mistake was introduced during "git rebase", it should have been
squashed into the previous patch, not this one. Thanks for spotting it.
> > - * 2D acceleration on SM720 and Big-Endian CPUs.
> > + * 2D acceleration on Big-Endian CPUs.
> > * More VGA modes.
> > * dual-head support
> > * hardware cursor support
>
> You might want to squash the two patches for documentation instead of
> having a separate patch to fix what you added in a previous patch.
Same as above, it will be fixed in PATCH v3.
Thanks,
Tom Li
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 6/7] fbdev: sm712fb: Kconfig: add information about docs.
@ 2019-04-01 17:33 ` Tom Li
0 siblings, 0 replies; 60+ messages in thread
From: Tom Li @ 2019-04-01 17:33 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Sun, Mar 31, 2019 at 08:01:18PM +0100, Sudip Mukherjee wrote:
> On Fri, Mar 22, 2019 at 01:17:58PM +0800, Yifeng Li wrote:
> > @@ -18,9 +18,10 @@ and it was also an inexpensive platform for non-x86 hobbyists to explore.
> > How to use it?
> > =======
> >
> > -You should not compile-in vesafb, since SM7xx can be used in a VGA
> > -compatible mode, resulting conflicts with this driver. In addition,
> > -the VGA compatible mode was never tested by the maintainers.
> > +SM7xx can be used in a VGA compatible mode and a native framebuffer mode,
> > +this driver is a native driver. By default, it has no problem taking over
> > +the VGA driver automatically, but you should not attempt to use both at
> > +the same time.
>
> You added this in the documentation with just the previous patch.
Oops, this mistake was introduced during "git rebase", it should have been
squashed into the previous patch, not this one. Thanks for spotting it.
> > - * 2D acceleration on SM720 and Big-Endian CPUs.
> > + * 2D acceleration on Big-Endian CPUs.
> > * More VGA modes.
> > * dual-head support
> > * hardware cursor support
>
> You might want to squash the two patches for documentation instead of
> having a separate patch to fix what you added in a previous patch.
Same as above, it will be fixed in PATCH v3.
Thanks,
Tom Li
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
2019-03-31 19:08 ` Sudip Mukherjee
@ 2019-04-01 17:41 ` Tom Li
-1 siblings, 0 replies; 60+ messages in thread
From: Tom Li @ 2019-04-01 17:41 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Sun, Mar 31, 2019 at 08:08:01PM +0100, Sudip Mukherjee wrote:
> Technically I donot have any problem with this, you seem to know more
> about SM712 than I know. But Teddy Wang is also an existing maintainer
> and I think there should be an ack from him before this is accepted.
Okay, I'll write a personal mail to Teddy. I think it could be a separate
patch to allow more time for communication, but the problem is that, if
the MAINTAINERS and the new changes are not merged at the same time, users
who have problems may unable to see my name and E-mail address for reporting
problems.
I think I can send PATCH v3 early for your review, meanwhile I can write
a personal E-mail to Teddy.
> On Fri, Mar 22, 2019 at 01:17:59PM +0800, Yifeng Li wrote:
> > SILICON MOTION SM712 FRAME BUFFER DRIVER
> > +M: Tom Li <tomli@tomli.me>
>
> Sorry, I am confused. Is your name "Tom Li"? Then why is your
> Signed-off-by: and From: name different?
Sorry for the confusion.
"Tom Li" is my common name. I use this name both in offline and online
activities, and I've used it to contribute various patches to free and
open source projects and mailing list discussions as well. But the
"Signed-off-by" name is the legal name. Linux Kernel's "Developer's
Certificate of Origin" requires the use of legal name in sign-offs.
I think I should write my name as `Yifeng "Tom" Li` in MAINTAINERS to
avoid the confusion.
Thanks for your code review, I've noted your suggestions and I'll
send PATCH v3 soon.
Thanks,
Tom Li
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
@ 2019-04-01 17:41 ` Tom Li
0 siblings, 0 replies; 60+ messages in thread
From: Tom Li @ 2019-04-01 17:41 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, linux-fbdev,
dri-devel
On Sun, Mar 31, 2019 at 08:08:01PM +0100, Sudip Mukherjee wrote:
> Technically I donot have any problem with this, you seem to know more
> about SM712 than I know. But Teddy Wang is also an existing maintainer
> and I think there should be an ack from him before this is accepted.
Okay, I'll write a personal mail to Teddy. I think it could be a separate
patch to allow more time for communication, but the problem is that, if
the MAINTAINERS and the new changes are not merged at the same time, users
who have problems may unable to see my name and E-mail address for reporting
problems.
I think I can send PATCH v3 early for your review, meanwhile I can write
a personal E-mail to Teddy.
> On Fri, Mar 22, 2019 at 01:17:59PM +0800, Yifeng Li wrote:
> > SILICON MOTION SM712 FRAME BUFFER DRIVER
> > +M: Tom Li <tomli@tomli.me>
>
> Sorry, I am confused. Is your name "Tom Li"? Then why is your
> Signed-off-by: and From: name different?
Sorry for the confusion.
"Tom Li" is my common name. I use this name both in offline and online
activities, and I've used it to contribute various patches to free and
open source projects and mailing list discussions as well. But the
"Signed-off-by" name is the legal name. Linux Kernel's "Developer's
Certificate of Origin" requires the use of legal name in sign-offs.
I think I should write my name as `Yifeng "Tom" Li` in MAINTAINERS to
avoid the confusion.
Thanks for your code review, I've noted your suggestions and I'll
send PATCH v3 soon.
Thanks,
Tom Li
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 3/7] fbdev: sm712fb: support 2D acceleration on SM712 w/ Little-Endian CPU.
2019-04-01 16:26 ` Tom Li
@ 2019-04-02 20:53 ` Sudip Mukherjee
-1 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-02 20:53 UTC (permalink / raw)
To: Tom Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, LFBDEV, dri-devel
On Mon, Apr 1, 2019 at 5:26 PM Tom Li <tomli@tomli.me> wrote:
>
> On Sun, Mar 31, 2019 at 07:09:47PM +0100, Sudip Mukherjee wrote:
> > On Fri, Mar 22, 2019 at 01:17:55PM +0800, Yifeng Li wrote:
> > I didnot notice any performace improvement in my system. Infact, I have
> > never seen the performance problem that you mentioned. But, it will be
> > good to have the 2D feature back again.
>
<snip>
> But again, the 2D code is only used by Linux VT code internally, if you are
> not using VT console, the behavior cannot be tested properly. Are you testing
> it under the Linux tty console? And does your dmesg log says, "2D acceleration
> is enabled"?
>
> Running the "fbtest" testsuit is also a good way to "smoke-test" the fbdev
> code.
Yes, I do run fbtest as part of my testing.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 3/7] fbdev: sm712fb: support 2D acceleration on SM712 w/ Little-Endian CPU.
@ 2019-04-02 20:53 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-02 20:53 UTC (permalink / raw)
To: Tom Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, LFBDEV, dri-devel
On Mon, Apr 1, 2019 at 5:26 PM Tom Li <tomli@tomli.me> wrote:
>
> On Sun, Mar 31, 2019 at 07:09:47PM +0100, Sudip Mukherjee wrote:
> > On Fri, Mar 22, 2019 at 01:17:55PM +0800, Yifeng Li wrote:
> > I didnot notice any performace improvement in my system. Infact, I have
> > never seen the performance problem that you mentioned. But, it will be
> > good to have the 2D feature back again.
>
<snip>
> But again, the 2D code is only used by Linux VT code internally, if you are
> not using VT console, the behavior cannot be tested properly. Are you testing
> it under the Linux tty console? And does your dmesg log says, "2D acceleration
> is enabled"?
>
> Running the "fbtest" testsuit is also a good way to "smoke-test" the fbdev
> code.
Yes, I do run fbtest as part of my testing.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 4/7] fbdev: sm712fb: add 32-bit color modes, drops some other modes.
2019-04-01 16:41 ` Tom Li
@ 2019-04-02 20:59 ` Sudip Mukherjee
-1 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-02 20:59 UTC (permalink / raw)
To: Tom Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, LFBDEV, dri-devel
On Mon, Apr 1, 2019 at 5:42 PM Tom Li <tomli@tomli.me> wrote:
>
> On Sun, Mar 31, 2019 at 07:33:33PM +0100, Sudip Mukherjee wrote:
> > Why are you removing existing functionality from the driver? These are
> > supported but were never listed so could not be used. I think you can
> > just add these to vesa_mode_table[] and they can be used.
>
> If there are some "functionalities" in a system, but they are never used,
> even worse, they are programmed in a way that they cannot be used by any
> user no matter what, meanwhile not a single user had filed a bug report
> in the entire lifecycle of the program, then I'd call them "dead code",
> that serves no useful purposes, rather than "functionalities". I think
> most kernel developers can agree on this.
I think I will call that as a bug, a bug which did not export the
configuration and so it was not used. But, now that we know of the bug
we should fix the bug instead of removing the configuration.
>
> > I have an old CRT in India which supports 320x240 mode and can test there
> > when I am there next.
>
> Well... If someone (e.g. you) do see a need of this feature, then fixing
> them (instead of removing them) becomes a reasonable choice.
>
> Coincidentally, I've also purchased a video converter a few days ago. Please
> allow some time for me to testing it, so I can see if they're working. If so,
> I'll add them to the vesa_mode_table[] in PATCH v3.
Thanks.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 4/7] fbdev: sm712fb: add 32-bit color modes, drops some other modes.
@ 2019-04-02 20:59 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-02 20:59 UTC (permalink / raw)
To: Tom Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, LFBDEV, dri-devel
On Mon, Apr 1, 2019 at 5:42 PM Tom Li <tomli@tomli.me> wrote:
>
> On Sun, Mar 31, 2019 at 07:33:33PM +0100, Sudip Mukherjee wrote:
> > Why are you removing existing functionality from the driver? These are
> > supported but were never listed so could not be used. I think you can
> > just add these to vesa_mode_table[] and they can be used.
>
> If there are some "functionalities" in a system, but they are never used,
> even worse, they are programmed in a way that they cannot be used by any
> user no matter what, meanwhile not a single user had filed a bug report
> in the entire lifecycle of the program, then I'd call them "dead code",
> that serves no useful purposes, rather than "functionalities". I think
> most kernel developers can agree on this.
I think I will call that as a bug, a bug which did not export the
configuration and so it was not used. But, now that we know of the bug
we should fix the bug instead of removing the configuration.
>
> > I have an old CRT in India which supports 320x240 mode and can test there
> > when I am there next.
>
> Well... If someone (e.g. you) do see a need of this feature, then fixing
> them (instead of removing them) becomes a reasonable choice.
>
> Coincidentally, I've also purchased a video converter a few days ago. Please
> allow some time for me to testing it, so I can see if they're working. If so,
> I'll add them to the vesa_mode_table[] in PATCH v3.
Thanks.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 5/7] Documentation: fb: sm712fb: add information mainly about 2D.
2019-04-01 17:30 ` Tom Li
(?)
@ 2019-04-02 21:03 ` Sudip Mukherjee
-1 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-02 21:03 UTC (permalink / raw)
To: Tom Li
Cc: Teddy Wang, linux-kernel, Bartlomiej Zolnierkiewicz, LFBDEV, dri-devel
On Mon, Apr 1, 2019 at 6:30 PM Tom Li <tomli@tomli.me> wrote:
>
> On Sun, Mar 31, 2019 at 07:54:28PM +0100, Sudip Mukherjee wrote:
> > On Fri, Mar 22, 2019 at 01:17:57PM +0800, Yifeng Li wrote:
> > > +video controllers. This series of video controller is a legacy from ~1998,
> > > +and was used on many classic, "prehistoric" laptops from 1998-2004, such as
> > > +IBM Thinkpad S30 and 240X. It was also used on some servers, industrial
> > > +computers, x86 and non-x86 embedded devices where only basic graphics was
> > > +needed.
<snip>
>
> BTW, most Loongson 3A notebooks don't use SM712. I don't know that there are
> Loongson 3A notebooks that are still using SM712 graphics chip, do you have one?
> Could you tell me its model number?
No, I donot have them. I only have SM712 based PCI card which I
purchased from Silicon Motion.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 5/7] Documentation: fb: sm712fb: add information mainly about 2D.
@ 2019-04-02 21:03 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-02 21:03 UTC (permalink / raw)
To: Tom Li
Cc: LFBDEV, Bartlomiej Zolnierkiewicz, Teddy Wang, dri-devel, linux-kernel
On Mon, Apr 1, 2019 at 6:30 PM Tom Li <tomli@tomli.me> wrote:
>
> On Sun, Mar 31, 2019 at 07:54:28PM +0100, Sudip Mukherjee wrote:
> > On Fri, Mar 22, 2019 at 01:17:57PM +0800, Yifeng Li wrote:
> > > +video controllers. This series of video controller is a legacy from ~1998,
> > > +and was used on many classic, "prehistoric" laptops from 1998-2004, such as
> > > +IBM Thinkpad S30 and 240X. It was also used on some servers, industrial
> > > +computers, x86 and non-x86 embedded devices where only basic graphics was
> > > +needed.
<snip>
>
> BTW, most Loongson 3A notebooks don't use SM712. I don't know that there are
> Loongson 3A notebooks that are still using SM712 graphics chip, do you have one?
> Could you tell me its model number?
No, I donot have them. I only have SM712 based PCI card which I
purchased from Silicon Motion.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 5/7] Documentation: fb: sm712fb: add information mainly about 2D.
@ 2019-04-02 21:03 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-02 21:03 UTC (permalink / raw)
To: Tom Li
Cc: LFBDEV, Bartlomiej Zolnierkiewicz, Teddy Wang, dri-devel, linux-kernel
On Mon, Apr 1, 2019 at 6:30 PM Tom Li <tomli@tomli.me> wrote:
>
> On Sun, Mar 31, 2019 at 07:54:28PM +0100, Sudip Mukherjee wrote:
> > On Fri, Mar 22, 2019 at 01:17:57PM +0800, Yifeng Li wrote:
> > > +video controllers. This series of video controller is a legacy from ~1998,
> > > +and was used on many classic, "prehistoric" laptops from 1998-2004, such as
> > > +IBM Thinkpad S30 and 240X. It was also used on some servers, industrial
> > > +computers, x86 and non-x86 embedded devices where only basic graphics was
> > > +needed.
<snip>
>
> BTW, most Loongson 3A notebooks don't use SM712. I don't know that there are
> Loongson 3A notebooks that are still using SM712 graphics chip, do you have one?
> Could you tell me its model number?
No, I donot have them. I only have SM712 based PCI card which I
purchased from Silicon Motion.
--
Regards
Sudip
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
2019-04-01 17:41 ` Tom Li
(?)
@ 2019-04-02 21:09 ` Sudip Mukherjee
-1 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-02 21:09 UTC (permalink / raw)
To: Tom Li, Bartlomiej Zolnierkiewicz
Cc: Teddy Wang, linux-kernel, LFBDEV, dri-devel
On Mon, Apr 1, 2019 at 6:41 PM Tom Li <tomli@tomli.me> wrote:
>
> On Sun, Mar 31, 2019 at 08:08:01PM +0100, Sudip Mukherjee wrote:
> > Technically I donot have any problem with this, you seem to know more
> > about SM712 than I know. But Teddy Wang is also an existing maintainer
> > and I think there should be an ack from him before this is accepted.
>
> Okay, I'll write a personal mail to Teddy. I think it could be a separate
> patch to allow more time for communication, but the problem is that, if
> the MAINTAINERS and the new changes are not merged at the same time, users
> who have problems may unable to see my name and E-mail address for reporting
> problems.
git will not forget that you have done the changes. :)
So anyone who has a problem only needs to do a "git blame" to see who
has done it and the user will get your name and email to contact you.
If that is the only reason you think your name is added as a
maintainer then I guess that is not a valid concern and then in that
case I am not seeing any need to add your name in maintainer.
Bartlomiej can you advise please.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
@ 2019-04-02 21:09 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-02 21:09 UTC (permalink / raw)
To: Tom Li, Bartlomiej Zolnierkiewicz
Cc: LFBDEV, Teddy Wang, dri-devel, linux-kernel
On Mon, Apr 1, 2019 at 6:41 PM Tom Li <tomli@tomli.me> wrote:
>
> On Sun, Mar 31, 2019 at 08:08:01PM +0100, Sudip Mukherjee wrote:
> > Technically I donot have any problem with this, you seem to know more
> > about SM712 than I know. But Teddy Wang is also an existing maintainer
> > and I think there should be an ack from him before this is accepted.
>
> Okay, I'll write a personal mail to Teddy. I think it could be a separate
> patch to allow more time for communication, but the problem is that, if
> the MAINTAINERS and the new changes are not merged at the same time, users
> who have problems may unable to see my name and E-mail address for reporting
> problems.
git will not forget that you have done the changes. :)
So anyone who has a problem only needs to do a "git blame" to see who
has done it and the user will get your name and email to contact you.
If that is the only reason you think your name is added as a
maintainer then I guess that is not a valid concern and then in that
case I am not seeing any need to add your name in maintainer.
Bartlomiej can you advise please.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
@ 2019-04-02 21:09 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-02 21:09 UTC (permalink / raw)
To: Tom Li, Bartlomiej Zolnierkiewicz
Cc: LFBDEV, Teddy Wang, dri-devel, linux-kernel
On Mon, Apr 1, 2019 at 6:41 PM Tom Li <tomli@tomli.me> wrote:
>
> On Sun, Mar 31, 2019 at 08:08:01PM +0100, Sudip Mukherjee wrote:
> > Technically I donot have any problem with this, you seem to know more
> > about SM712 than I know. But Teddy Wang is also an existing maintainer
> > and I think there should be an ack from him before this is accepted.
>
> Okay, I'll write a personal mail to Teddy. I think it could be a separate
> patch to allow more time for communication, but the problem is that, if
> the MAINTAINERS and the new changes are not merged at the same time, users
> who have problems may unable to see my name and E-mail address for reporting
> problems.
git will not forget that you have done the changes. :)
So anyone who has a problem only needs to do a "git blame" to see who
has done it and the user will get your name and email to contact you.
If that is the only reason you think your name is added as a
maintainer then I guess that is not a valid concern and then in that
case I am not seeing any need to add your name in maintainer.
Bartlomiej can you advise please.
--
Regards
Sudip
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
2019-04-02 21:09 ` Sudip Mukherjee
@ 2019-04-03 13:53 ` Bartlomiej Zolnierkiewicz
-1 siblings, 0 replies; 60+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2019-04-03 13:53 UTC (permalink / raw)
To: Sudip Mukherjee, Tom Li; +Cc: Teddy Wang, linux-kernel, LFBDEV, dri-devel
On 04/02/2019 11:09 PM, Sudip Mukherjee wrote:
> On Mon, Apr 1, 2019 at 6:41 PM Tom Li <tomli@tomli.me> wrote:
>>
>> On Sun, Mar 31, 2019 at 08:08:01PM +0100, Sudip Mukherjee wrote:
>>> Technically I donot have any problem with this, you seem to know more
>>> about SM712 than I know. But Teddy Wang is also an existing maintainer
>>> and I think there should be an ack from him before this is accepted.
>>
>> Okay, I'll write a personal mail to Teddy. I think it could be a separate
>> patch to allow more time for communication, but the problem is that, if
The first version of the patchset was posted on 2nd February and
Teddy was on Cc:. Unless there is an explicit NACK (with valid
rationale) from him lets assume that he is fine with having another
co-maintainer.
>> the MAINTAINERS and the new changes are not merged at the same time, users
>> who have problems may unable to see my name and E-mail address for reporting
>> problems.
>
> git will not forget that you have done the changes. :)
> So anyone who has a problem only needs to do a "git blame" to see who
> has done it and the user will get your name and email to contact you.
> If that is the only reason you think your name is added as a
The original patch description suggests that there is more than
that:
"I have working on the sm712fb driver for a while and have some
familiarity with this hardware, I'll be helping working on and
testing problems of this driver, so add myself to the MAINTAINERS
file."
> maintainer then I guess that is not a valid concern and then in that
> case I am not seeing any need to add your name in maintainer.
>
> Bartlomiej can you advise please.
2D acceleration seems to be an important contribution to the driver
(functionality wise and code wise -> it grows the driver source code
by ~25%) and it shows that Yifeng knows the driver well.
He has also access to SM720 (you have SM712 only) to verify potential
issues with the future driver changes (I assume that he don't mind
getting Cc:ed on all driver related patches, not that there should be
much of them).
Thus I think that it would be beneficial to add him as the driver
co-maintainer but I would prefer to keep the current ordering in
the MAINTAINERS file, IOW something like this:
Index: b/MAINTAINERS
===================================================================
--- a/MAINTAINERS 2019-04-01 13:12:10.607271933 +0200
+++ b/MAINTAINERS 2019-04-03 15:39:54.404646279 +0200
@@ -14143,6 +14143,7 @@ SILICON MOTION SM712 FRAME BUFFER DRIVER
M: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
M: Teddy Wang <teddy.wang@siliconmotion.com>
M: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
+M: Yifeng Li <tomli@tomli.me>
L: linux-fbdev@vger.kernel.org
S: Maintained
F: drivers/video/fbdev/sm712*
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
@ 2019-04-03 13:53 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 60+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2019-04-03 13:53 UTC (permalink / raw)
To: Sudip Mukherjee, Tom Li; +Cc: Teddy Wang, linux-kernel, LFBDEV, dri-devel
On 04/02/2019 11:09 PM, Sudip Mukherjee wrote:
> On Mon, Apr 1, 2019 at 6:41 PM Tom Li <tomli@tomli.me> wrote:
>>
>> On Sun, Mar 31, 2019 at 08:08:01PM +0100, Sudip Mukherjee wrote:
>>> Technically I donot have any problem with this, you seem to know more
>>> about SM712 than I know. But Teddy Wang is also an existing maintainer
>>> and I think there should be an ack from him before this is accepted.
>>
>> Okay, I'll write a personal mail to Teddy. I think it could be a separate
>> patch to allow more time for communication, but the problem is that, if
The first version of the patchset was posted on 2nd February and
Teddy was on Cc:. Unless there is an explicit NACK (with valid
rationale) from him lets assume that he is fine with having another
co-maintainer.
>> the MAINTAINERS and the new changes are not merged at the same time, users
>> who have problems may unable to see my name and E-mail address for reporting
>> problems.
>
> git will not forget that you have done the changes. :)
> So anyone who has a problem only needs to do a "git blame" to see who
> has done it and the user will get your name and email to contact you.
> If that is the only reason you think your name is added as a
The original patch description suggests that there is more than
that:
"I have working on the sm712fb driver for a while and have some
familiarity with this hardware, I'll be helping working on and
testing problems of this driver, so add myself to the MAINTAINERS
file."
> maintainer then I guess that is not a valid concern and then in that
> case I am not seeing any need to add your name in maintainer.
>
> Bartlomiej can you advise please.
2D acceleration seems to be an important contribution to the driver
(functionality wise and code wise -> it grows the driver source code
by ~25%) and it shows that Yifeng knows the driver well.
He has also access to SM720 (you have SM712 only) to verify potential
issues with the future driver changes (I assume that he don't mind
getting Cc:ed on all driver related patches, not that there should be
much of them).
Thus I think that it would be beneficial to add him as the driver
co-maintainer but I would prefer to keep the current ordering in
the MAINTAINERS file, IOW something like this:
Index: b/MAINTAINERS
=================================--- a/MAINTAINERS 2019-04-01 13:12:10.607271933 +0200
+++ b/MAINTAINERS 2019-04-03 15:39:54.404646279 +0200
@@ -14143,6 +14143,7 @@ SILICON MOTION SM712 FRAME BUFFER DRIVER
M: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
M: Teddy Wang <teddy.wang@siliconmotion.com>
M: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
+M: Yifeng Li <tomli@tomli.me>
L: linux-fbdev@vger.kernel.org
S: Maintained
F: drivers/video/fbdev/sm712*
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
2019-04-03 13:53 ` Bartlomiej Zolnierkiewicz
(?)
@ 2019-04-05 22:11 ` Sudip Mukherjee
-1 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-05 22:11 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Tom Li, Teddy Wang, linux-kernel, LFBDEV, dri-devel
On Wed, Apr 3, 2019 at 2:53 PM Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
>
>
> On 04/02/2019 11:09 PM, Sudip Mukherjee wrote:
> > On Mon, Apr 1, 2019 at 6:41 PM Tom Li <tomli@tomli.me> wrote:
> >>
> >> On Sun, Mar 31, 2019 at 08:08:01PM +0100, Sudip Mukherjee wrote:
> >>> Technically I donot have any problem with this, you seem to know more
> >>> about SM712 than I know. But Teddy Wang is also an existing maintainer
> >>> and I think there should be an ack from him before this is accepted.
> >>
> >> Okay, I'll write a personal mail to Teddy. I think it could be a separate
> >> patch to allow more time for communication, but the problem is that, if
>
> The first version of the patchset was posted on 2nd February and
> Teddy was on Cc:. Unless there is an explicit NACK (with valid
> rationale) from him lets assume that he is fine with having another
> co-maintainer.
>
> >> the MAINTAINERS and the new changes are not merged at the same time, users
> >> who have problems may unable to see my name and E-mail address for reporting
> >> problems.
> >
> > git will not forget that you have done the changes. :)
> > So anyone who has a problem only needs to do a "git blame" to see who
> > has done it and the user will get your name and email to contact you.
> > If that is the only reason you think your name is added as a
>
> The original patch description suggests that there is more than
> that:
>
> "I have working on the sm712fb driver for a while and have some
> familiarity with this hardware, I'll be helping working on and
> testing problems of this driver, so add myself to the MAINTAINERS
> file."
>
> > maintainer then I guess that is not a valid concern and then in that
> > case I am not seeing any need to add your name in maintainer.
> >
> > Bartlomiej can you advise please.
>
> 2D acceleration seems to be an important contribution to the driver
> (functionality wise and code wise -> it grows the driver source code
> by ~25%) and it shows that Yifeng knows the driver well.
>
> He has also access to SM720 (you have SM712 only) to verify potential
> issues with the future driver changes (I assume that he don't mind
> getting Cc:ed on all driver related patches, not that there should be
> much of them).
Agreed with all the points.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
@ 2019-04-05 22:11 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-05 22:11 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: dri-devel, LFBDEV, Teddy Wang, Tom Li, linux-kernel
On Wed, Apr 3, 2019 at 2:53 PM Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
>
>
> On 04/02/2019 11:09 PM, Sudip Mukherjee wrote:
> > On Mon, Apr 1, 2019 at 6:41 PM Tom Li <tomli@tomli.me> wrote:
> >>
> >> On Sun, Mar 31, 2019 at 08:08:01PM +0100, Sudip Mukherjee wrote:
> >>> Technically I donot have any problem with this, you seem to know more
> >>> about SM712 than I know. But Teddy Wang is also an existing maintainer
> >>> and I think there should be an ack from him before this is accepted.
> >>
> >> Okay, I'll write a personal mail to Teddy. I think it could be a separate
> >> patch to allow more time for communication, but the problem is that, if
>
> The first version of the patchset was posted on 2nd February and
> Teddy was on Cc:. Unless there is an explicit NACK (with valid
> rationale) from him lets assume that he is fine with having another
> co-maintainer.
>
> >> the MAINTAINERS and the new changes are not merged at the same time, users
> >> who have problems may unable to see my name and E-mail address for reporting
> >> problems.
> >
> > git will not forget that you have done the changes. :)
> > So anyone who has a problem only needs to do a "git blame" to see who
> > has done it and the user will get your name and email to contact you.
> > If that is the only reason you think your name is added as a
>
> The original patch description suggests that there is more than
> that:
>
> "I have working on the sm712fb driver for a while and have some
> familiarity with this hardware, I'll be helping working on and
> testing problems of this driver, so add myself to the MAINTAINERS
> file."
>
> > maintainer then I guess that is not a valid concern and then in that
> > case I am not seeing any need to add your name in maintainer.
> >
> > Bartlomiej can you advise please.
>
> 2D acceleration seems to be an important contribution to the driver
> (functionality wise and code wise -> it grows the driver source code
> by ~25%) and it shows that Yifeng knows the driver well.
>
> He has also access to SM720 (you have SM712 only) to verify potential
> issues with the future driver changes (I assume that he don't mind
> getting Cc:ed on all driver related patches, not that there should be
> much of them).
Agreed with all the points.
--
Regards
Sudip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer.
@ 2019-04-05 22:11 ` Sudip Mukherjee
0 siblings, 0 replies; 60+ messages in thread
From: Sudip Mukherjee @ 2019-04-05 22:11 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: dri-devel, LFBDEV, Teddy Wang, Tom Li, linux-kernel
On Wed, Apr 3, 2019 at 2:53 PM Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
>
>
> On 04/02/2019 11:09 PM, Sudip Mukherjee wrote:
> > On Mon, Apr 1, 2019 at 6:41 PM Tom Li <tomli@tomli.me> wrote:
> >>
> >> On Sun, Mar 31, 2019 at 08:08:01PM +0100, Sudip Mukherjee wrote:
> >>> Technically I donot have any problem with this, you seem to know more
> >>> about SM712 than I know. But Teddy Wang is also an existing maintainer
> >>> and I think there should be an ack from him before this is accepted.
> >>
> >> Okay, I'll write a personal mail to Teddy. I think it could be a separate
> >> patch to allow more time for communication, but the problem is that, if
>
> The first version of the patchset was posted on 2nd February and
> Teddy was on Cc:. Unless there is an explicit NACK (with valid
> rationale) from him lets assume that he is fine with having another
> co-maintainer.
>
> >> the MAINTAINERS and the new changes are not merged at the same time, users
> >> who have problems may unable to see my name and E-mail address for reporting
> >> problems.
> >
> > git will not forget that you have done the changes. :)
> > So anyone who has a problem only needs to do a "git blame" to see who
> > has done it and the user will get your name and email to contact you.
> > If that is the only reason you think your name is added as a
>
> The original patch description suggests that there is more than
> that:
>
> "I have working on the sm712fb driver for a while and have some
> familiarity with this hardware, I'll be helping working on and
> testing problems of this driver, so add myself to the MAINTAINERS
> file."
>
> > maintainer then I guess that is not a valid concern and then in that
> > case I am not seeing any need to add your name in maintainer.
> >
> > Bartlomiej can you advise please.
>
> 2D acceleration seems to be an important contribution to the driver
> (functionality wise and code wise -> it grows the driver source code
> by ~25%) and it shows that Yifeng knows the driver well.
>
> He has also access to SM720 (you have SM712 only) to verify potential
> issues with the future driver changes (I assume that he don't mind
> getting Cc:ed on all driver related patches, not that there should be
> much of them).
Agreed with all the points.
--
Regards
Sudip
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 60+ messages in thread
end of thread, other threads:[~2019-04-05 22:12 UTC | newest]
Thread overview: 60+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-22 5:17 [PATCH v2 0/7] implement 2D acceleration, minor cleanups, doc updates Yifeng Li
2019-03-22 5:17 ` Yifeng Li
2019-03-22 5:17 ` [PATCH v2 1/7] fbdev: sm712fb: use type "u8" for 8-bit I/O Yifeng Li
2019-03-22 5:17 ` Yifeng Li
2019-03-31 17:16 ` Sudip Mukherjee
2019-03-31 17:16 ` Sudip Mukherjee
2019-03-31 17:16 ` Sudip Mukherjee
2019-03-22 5:17 ` [PATCH v2 2/7] fbdev: sm712fb: add 2D-related I/O headers and functions Yifeng Li
2019-03-22 5:17 ` Yifeng Li
2019-03-31 17:25 ` Sudip Mukherjee
2019-03-31 17:25 ` Sudip Mukherjee
2019-04-01 16:04 ` Tom Li
2019-04-01 16:04 ` Tom Li
2019-03-22 5:17 ` [PATCH v2 3/7] fbdev: sm712fb: support 2D acceleration on SM712 w/ Little-Endian CPU Yifeng Li
2019-03-22 5:17 ` Yifeng Li
2019-03-31 18:09 ` Sudip Mukherjee
2019-03-31 18:09 ` Sudip Mukherjee
2019-04-01 16:26 ` Tom Li
2019-04-01 16:26 ` Tom Li
2019-04-02 20:53 ` Sudip Mukherjee
2019-04-02 20:53 ` Sudip Mukherjee
2019-03-22 5:17 ` [PATCH v2 4/7] fbdev: sm712fb: add 32-bit color modes, drops some other modes Yifeng Li
2019-03-22 5:17 ` Yifeng Li
2019-03-31 18:33 ` Sudip Mukherjee
2019-03-31 18:33 ` Sudip Mukherjee
2019-04-01 16:41 ` Tom Li
2019-04-01 16:41 ` Tom Li
2019-04-02 20:59 ` Sudip Mukherjee
2019-04-02 20:59 ` Sudip Mukherjee
2019-03-22 5:17 ` [PATCH v2 5/7] Documentation: fb: sm712fb: add information mainly about 2D Yifeng Li
2019-03-22 5:17 ` Yifeng Li
2019-03-31 18:54 ` Sudip Mukherjee
2019-03-31 18:54 ` Sudip Mukherjee
2019-03-31 18:54 ` Sudip Mukherjee
2019-04-01 17:30 ` Tom Li
2019-04-01 17:30 ` Tom Li
2019-04-02 21:03 ` Sudip Mukherjee
2019-04-02 21:03 ` Sudip Mukherjee
2019-04-02 21:03 ` Sudip Mukherjee
2019-03-22 5:17 ` [PATCH v2 6/7] fbdev: sm712fb: Kconfig: add information about docs Yifeng Li
2019-03-22 5:17 ` Yifeng Li
2019-03-31 19:01 ` Sudip Mukherjee
2019-03-31 19:01 ` Sudip Mukherjee
2019-04-01 17:33 ` Tom Li
2019-04-01 17:33 ` Tom Li
2019-03-22 5:17 ` [PATCH v2 7/7] MAINTAINERS: sm712fb: list myself as one maintainer Yifeng Li
2019-03-22 5:17 ` Yifeng Li
2019-03-31 19:08 ` Sudip Mukherjee
2019-03-31 19:08 ` Sudip Mukherjee
2019-03-31 19:08 ` Sudip Mukherjee
2019-04-01 17:41 ` Tom Li
2019-04-01 17:41 ` Tom Li
2019-04-02 21:09 ` Sudip Mukherjee
2019-04-02 21:09 ` Sudip Mukherjee
2019-04-02 21:09 ` Sudip Mukherjee
2019-04-03 13:53 ` Bartlomiej Zolnierkiewicz
2019-04-03 13:53 ` Bartlomiej Zolnierkiewicz
2019-04-05 22:11 ` Sudip Mukherjee
2019-04-05 22:11 ` Sudip Mukherjee
2019-04-05 22:11 ` Sudip Mukherjee
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.