Re: [PATCH] drm/loongson: Add support for the DC in LS2K1000 SoC
Hi, On 2023/10/19 16:11, Maxime Ripard wrote: On Fri, Oct 13, 2023 at 06:28:01PM +0800, Sui Jingfeng wrote: Hi, On 2023/10/13 16:22, Icenowy Zheng wrote: 在 2023-10-12星期四的 00:26 +0800，Sui Jingfeng写道： LS2K1000 is a low end SoC (two core 1.0gHz), it don't has dedicated VRAM. It requires the framebuffer to be phisically continguous to scanout. The display controller in LS2K1000 don't has built-in GPIO hardware, the structure (register bit field) of its pixel, DC, GPU, DDR PLL are also defferent from other model. But for the display controller itself, Most of hardware features of it are same with ls7a1000. Below is a simple dts for it. Why don't you write a proper, YAML-formatted binding? This patch use only one DT property, that is the "memory-region = <&display_reserved>;". But the memory-region property is a common property, I means that everyone know how to use this property. I'm not sure the if YAML documentation is strictly required now. AFAIK it is, and even if it's not, please do it. OK, thanks a lot for the feedback. I will try to solve this problem at the next version. I'm preparing the next version. Maxime
Re: [PATCH] drm/loongson: Add support for the DC in LS2K1000 SoC
On Fri, Oct 13, 2023 at 06:28:01PM +0800, Sui Jingfeng wrote: > Hi, > > > On 2023/10/13 16:22, Icenowy Zheng wrote: > > 在 2023-10-12星期四的 00:26 +0800，Sui Jingfeng写道： > > > LS2K1000 is a low end SoC (two core 1.0gHz), it don't has dedicated > > > VRAM. > > > It requires the framebuffer to be phisically continguous to scanout. > > > The > > > display controller in LS2K1000 don't has built-in GPIO hardware, the > > > structure (register bit field) of its pixel, DC, GPU, DDR PLL are > > > also > > > defferent from other model. But for the display controller itself, > > > Most of > > > hardware features of it are same with ls7a1000. > > > > > > Below is a simple dts for it. > > Why don't you write a proper, YAML-formatted binding? > > > This patch use only one DT property, that is the "memory-region = > <&display_reserved>;". > But the memory-region property is a common property, I means that everyone > know how to > use this property. I'm not sure the if YAML documentation is strictly > required now. AFAIK it is, and even if it's not, please do it. Maxime
Re: [PATCH] drm/loongson: Add support for the DC in LS2K1000 SoC
Hi,
On 2023/10/13 16:22, Icenowy Zheng wrote:
在 2023-10-12星期四的 00:26 +0800，Sui Jingfeng写道：
LS2K1000 is a low end SoC (two core 1.0gHz), it don't has dedicated
VRAM.
It requires the framebuffer to be phisically continguous to scanout.
The
display controller in LS2K1000 don't has built-in GPIO hardware, the
structure (register bit field) of its pixel, DC, GPU, DDR PLL are
also
defferent from other model. But for the display controller itself,
Most of
hardware features of it are same with ls7a1000.
Below is a simple dts for it.
Why don't you write a proper, YAML-formatted binding?
This patch use only one DT property, that is the "memory-region =
<&display_reserved>;".
But the memory-region property is a common property, I means that everyone know
how to
use this property. I'm not sure the if YAML documentation is strictly required
now. And
the checkpatch.pl show no warnings to me.
This will help handling the correctness of device trees, and a binding
is required to allow the driver to enter.
Yeah, this may be true. But I have forget the rules and I don't know
what the status now. I remember that a reviewer told me to drop dts and
write(provide) generic code only in the past. Maybe the arch-specific
maintainers want to provide a dts. I don't know what's the status for now, I am waiting some feedback.
aliases {
i2c0 = &i2c0;
i2c1 = &i2c1;
};
reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
ranges;
display_reserved: framebuffer@3000 {
compatible = "shared-dma-pool";
reg = <0x0 0x2000 0x0 0x0800>; /* 128M */
linux,cma-default;
};
};
i2c0: i2c-gpio-0 {
compatible = "i2c-gpio";
scl-gpios = <&gpio0 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
sda-gpios = <&gpio0 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
i2c-gpio,delay-us = <5>; /* ~100 kHz */
status = "okay";
};
i2c1: i2c-gpio-1 {
compatible = "i2c-gpio";
scl-gpios = <&gpio0 33 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
sda-gpios = <&gpio0 32 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
i2c-gpio,delay-us = <5>; /* ~100 kHz */
status = "okay";
};
display-controller@6,0 {
reg = <0x3000 0x0 0x0 0x0 0x0>;
interrupt-parent = <&liointc0>;
interrupts = <28 IRQ_TYPE_LEVEL_HIGH>
memory-region = <&display_reserved>;
Is a system-wide CMA pool enough for this usage? And for a display
controller, will 128M be too much? (I assume the Vivante GPU do not
require contingous memory).
Yes, Vivante GPU do not require continuous memory. As the GPU has IOMMU
which map the scatter physical pages to continuous virtual address.
But loongson display control don't has such a luxury. The dumb framebuffer
has to be physically contiguous to scanout.
128M is not too much, as we have plenty system RAM. 64M is also OK.
status = "okay";
};
This patch is tested on ls2k1000 evaluation board.
$ dmesg | grep ":00:06.0"
loongson :00:06.0: Found LS2K1000 SoC, revision: 0
loongson :00:06.0: [drm] dc: 250MHz, ddr: 400MHz, gpu: 228MHz
loongson :00:06.0: [drm] Using of reserved mem:
800@0x2000
loongson :00:06.0: [drm] VRAM: 8192 pages ready
loongson :00:06.0: [drm] GTT: 32768 pages ready
loongson :00:06.0: [drm] display pipe-0 has a DVO
loongson :00:06.0: [drm] display pipe-1 has a DVO
loongson :00:06.0: [drm] Total 2 outputs
loongson :00:06.0: [drm] registered irq: 28
[drm] Initialized loongson 1.0.0 20220701 for :00:06.0 on minor
0
loongson :00:06.0: [drm] fb0: loongsondrmfb frame buffer device
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/loongson/Makefile | 1 +
drivers/gpu/drm/loongson/loongson_device.c | 59 +++
drivers/gpu/drm/loongson/lsdc_drv.c | 44 -
drivers/gpu/drm/loongson/lsdc_drv.h | 9 ++
drivers/gpu/drm/loongson/lsdc_gfxpll.c | 11 +-
drivers/gpu/drm/loongson/lsdc_gfxpll.h | 4 +
drivers/gpu/drm/loongson/lsdc_gfxpll_2k1000.c | 141
drivers/gpu/drm/loongson/lsdc_i2c.c | 41 +
drivers/gpu/drm/loongson/lsdc_i2c.h | 4 +
drivers/gpu/drm/loongson/lsdc_pixpll.c | 153
+-
drivers/gpu/drm/loongson/lsdc_pixpll.h | 4 +
drivers/gpu/drm/loongson/lsdc_probe.c | 27
drivers/gpu/drm/loongson/lsdc_probe.h | 2 +
drivers/gpu/drm/loongson/lsdc_regs.h | 36 +
14 files changed, 528 insertions(+), 8 deletions(-)
create mode 100644 drivers/gpu/drm/loongson/lsdc_gfxpll_2k1000.c
diff --git a/drivers/gpu/drm/loongson/Makefile
b/drivers/gpu/drm/loongson/Makefile
index 91e72bd900c1..d6e709e19fba 100644
--- a/drivers/gpu/drm/loongson/Makefile
+++ b/drivers/gpu/drm/loongson/Makefile
@@ -7,6 +7,7 @@ loongson-y := \
lsdc_drv.o \
Re: [PATCH] drm/loongson: Add support for the DC in LS2K1000 SoC
在 2023-10-12星期四的 00:26 +0800，Sui Jingfeng写道：
> LS2K1000 is a low end SoC (two core 1.0gHz), it don't has dedicated
> VRAM.
> It requires the framebuffer to be phisically continguous to scanout.
> The
> display controller in LS2K1000 don't has built-in GPIO hardware, the
> structure (register bit field) of its pixel, DC, GPU, DDR PLL are
> also
> defferent from other model. But for the display controller itself,
> Most of
> hardware features of it are same with ls7a1000.
>
> Below is a simple dts for it.
Why don't you write a proper, YAML-formatted binding?
This will help handling the correctness of device trees, and a binding
is required to allow the driver to enter.
>
> aliases {
> i2c0 = &i2c0;
> i2c1 = &i2c1;
> };
>
> reserved-memory {
> #address-cells = <2>;
> #size-cells = <2>;
> ranges;
>
> display_reserved: framebuffer@3000 {
> compatible = "shared-dma-pool";
> reg = <0x0 0x2000 0x0 0x0800>; /* 128M */
> linux,cma-default;
> };
> };
>
> i2c0: i2c-gpio-0 {
> compatible = "i2c-gpio";
> scl-gpios = <&gpio0 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> sda-gpios = <&gpio0 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> i2c-gpio,delay-us = <5>; /* ~100 kHz */
> status = "okay";
> };
>
> i2c1: i2c-gpio-1 {
> compatible = "i2c-gpio";
> scl-gpios = <&gpio0 33 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> sda-gpios = <&gpio0 32 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> i2c-gpio,delay-us = <5>; /* ~100 kHz */
> status = "okay";
> };
>
> display-controller@6,0 {
> reg = <0x3000 0x0 0x0 0x0 0x0>;
> interrupt-parent = <&liointc0>;
> interrupts = <28 IRQ_TYPE_LEVEL_HIGH>
> memory-region = <&display_reserved>;
Is a system-wide CMA pool enough for this usage? And for a display
controller, will 128M be too much? (I assume the Vivante GPU do not
require contingous memory).
> status = "okay";
> };
>
> This patch is tested on ls2k1000 evaluation board.
>
> $ dmesg | grep ":00:06.0"
>
> loongson :00:06.0: Found LS2K1000 SoC, revision: 0
> loongson :00:06.0: [drm] dc: 250MHz, ddr: 400MHz, gpu: 228MHz
> loongson :00:06.0: [drm] Using of reserved mem:
> 800@0x2000
> loongson :00:06.0: [drm] VRAM: 8192 pages ready
> loongson :00:06.0: [drm] GTT: 32768 pages ready
> loongson :00:06.0: [drm] display pipe-0 has a DVO
> loongson :00:06.0: [drm] display pipe-1 has a DVO
> loongson :00:06.0: [drm] Total 2 outputs
> loongson :00:06.0: [drm] registered irq: 28
> [drm] Initialized loongson 1.0.0 20220701 for :00:06.0 on minor
> 0
> loongson :00:06.0: [drm] fb0: loongsondrmfb frame buffer device
>
> Signed-off-by: Sui Jingfeng
> ---
> drivers/gpu/drm/loongson/Makefile | 1 +
> drivers/gpu/drm/loongson/loongson_device.c | 59 +++
> drivers/gpu/drm/loongson/lsdc_drv.c | 44 -
> drivers/gpu/drm/loongson/lsdc_drv.h | 9 ++
> drivers/gpu/drm/loongson/lsdc_gfxpll.c | 11 +-
> drivers/gpu/drm/loongson/lsdc_gfxpll.h | 4 +
> drivers/gpu/drm/loongson/lsdc_gfxpll_2k1000.c | 141
> drivers/gpu/drm/loongson/lsdc_i2c.c | 41 +
> drivers/gpu/drm/loongson/lsdc_i2c.h | 4 +
> drivers/gpu/drm/loongson/lsdc_pixpll.c | 153
> +-
> drivers/gpu/drm/loongson/lsdc_pixpll.h | 4 +
> drivers/gpu/drm/loongson/lsdc_probe.c | 27
> drivers/gpu/drm/loongson/lsdc_probe.h | 2 +
> drivers/gpu/drm/loongson/lsdc_regs.h | 36 +
> 14 files changed, 528 insertions(+), 8 deletions(-)
> create mode 100644 drivers/gpu/drm/loongson/lsdc_gfxpll_2k1000.c
>
> diff --git a/drivers/gpu/drm/loongson/Makefile
> b/drivers/gpu/drm/loongson/Makefile
> index 91e72bd900c1..d6e709e19fba 100644
> --- a/drivers/gpu/drm/loongson/Makefile
> +++ b/drivers/gpu/drm/loongson/Makefile
> @@ -7,6 +7,7 @@ loongson-y := \
> lsdc_drv.o \
> lsdc_gem.o \
> lsdc_gfxpll.o \
> + lsdc_gfxpll_2k1000.o \
> lsdc_i2c.o \
> lsdc_irq.o \
> lsdc_output_7a1000.o \
> diff --git a/drivers/gpu/drm/loongson/loongson_device.c
> b/drivers/gpu/drm/loongson/loongson_device.c
> index 9986c8a2a255..67274d9e3b12 100644
> --- a/drivers/gpu/drm/loongson/loongson_device.c
> +++ b/drivers/gpu/drm/loongson/loongson_device.c
> @@ -6,6 +6,7 @@
> #include
>
> #include "lsdc_drv.h"
> +#include "lsdc_probe.h"
>
> static const struct lsdc_kms_funcs ls7a1000_kms_funcs = {
> .create_i2c = lsdc_create_i2c_chan,
> @@ -25,6 +26,20 @@ static const struct lsdc_kms_funcs
> ls7a2000_kms_funcs = {
> .crtc_init = ls7a2000_crtc_init,
> };
>
> +/*
> + * Most of hardware features of ls2k1000 are same with ls7a1000, we
> take the
> + * ls7a1000_kms_funcs as a prototype,
[PATCH] drm/loongson: Add support for the DC in LS2K1000 SoC
LS2K1000 is a low end SoC (two core 1.0gHz), it don't has dedicated VRAM.
It requires the framebuffer to be phisically continguous to scanout. The
display controller in LS2K1000 don't has built-in GPIO hardware, the
structure (register bit field) of its pixel, DC, GPU, DDR PLL are also
defferent from other model. But for the display controller itself, Most of
hardware features of it are same with ls7a1000.
Below is a simple dts for it.
aliases {
i2c0 = &i2c0;
i2c1 = &i2c1;
};
reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
ranges;
display_reserved: framebuffer@3000 {
compatible = "shared-dma-pool";
reg = <0x0 0x2000 0x0 0x0800>; /* 128M */
linux,cma-default;
};
};
i2c0: i2c-gpio-0 {
compatible = "i2c-gpio";
scl-gpios = <&gpio0 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
sda-gpios = <&gpio0 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
i2c-gpio,delay-us = <5>;/* ~100 kHz */
status = "okay";
};
i2c1: i2c-gpio-1 {
compatible = "i2c-gpio";
scl-gpios = <&gpio0 33 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
sda-gpios = <&gpio0 32 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
i2c-gpio,delay-us = <5>;/* ~100 kHz */
status = "okay";
};
display-controller@6,0 {
reg = <0x3000 0x0 0x0 0x0 0x0>;
interrupt-parent = <&liointc0>;
interrupts = <28 IRQ_TYPE_LEVEL_HIGH>
memory-region = <&display_reserved>;
status = "okay";
};
This patch is tested on ls2k1000 evaluation board.
$ dmesg | grep ":00:06.0"
loongson :00:06.0: Found LS2K1000 SoC, revision: 0
loongson :00:06.0: [drm] dc: 250MHz, ddr: 400MHz, gpu: 228MHz
loongson :00:06.0: [drm] Using of reserved mem: 800@0x2000
loongson :00:06.0: [drm] VRAM: 8192 pages ready
loongson :00:06.0: [drm] GTT: 32768 pages ready
loongson :00:06.0: [drm] display pipe-0 has a DVO
loongson :00:06.0: [drm] display pipe-1 has a DVO
loongson :00:06.0: [drm] Total 2 outputs
loongson :00:06.0: [drm] registered irq: 28
[drm] Initialized loongson 1.0.0 20220701 for :00:06.0 on minor 0
loongson :00:06.0: [drm] fb0: loongsondrmfb frame buffer device
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/loongson/Makefile | 1 +
drivers/gpu/drm/loongson/loongson_device.c| 59 +++
drivers/gpu/drm/loongson/lsdc_drv.c | 44 -
drivers/gpu/drm/loongson/lsdc_drv.h | 9 ++
drivers/gpu/drm/loongson/lsdc_gfxpll.c| 11 +-
drivers/gpu/drm/loongson/lsdc_gfxpll.h| 4 +
drivers/gpu/drm/loongson/lsdc_gfxpll_2k1000.c | 141
drivers/gpu/drm/loongson/lsdc_i2c.c | 41 +
drivers/gpu/drm/loongson/lsdc_i2c.h | 4 +
drivers/gpu/drm/loongson/lsdc_pixpll.c| 153 +-
drivers/gpu/drm/loongson/lsdc_pixpll.h| 4 +
drivers/gpu/drm/loongson/lsdc_probe.c | 27
drivers/gpu/drm/loongson/lsdc_probe.h | 2 +
drivers/gpu/drm/loongson/lsdc_regs.h | 36 +
14 files changed, 528 insertions(+), 8 deletions(-)
create mode 100644 drivers/gpu/drm/loongson/lsdc_gfxpll_2k1000.c
diff --git a/drivers/gpu/drm/loongson/Makefile
b/drivers/gpu/drm/loongson/Makefile
index 91e72bd900c1..d6e709e19fba 100644
--- a/drivers/gpu/drm/loongson/Makefile
+++ b/drivers/gpu/drm/loongson/Makefile
@@ -7,6 +7,7 @@ loongson-y := \
lsdc_drv.o \
lsdc_gem.o \
lsdc_gfxpll.o \
+ lsdc_gfxpll_2k1000.o \
lsdc_i2c.o \
lsdc_irq.o \
lsdc_output_7a1000.o \
diff --git a/drivers/gpu/drm/loongson/loongson_device.c
b/drivers/gpu/drm/loongson/loongson_device.c
index 9986c8a2a255..67274d9e3b12 100644
--- a/drivers/gpu/drm/loongson/loongson_device.c
+++ b/drivers/gpu/drm/loongson/loongson_device.c
@@ -6,6 +6,7 @@
#include
#include "lsdc_drv.h"
+#include "lsdc_probe.h"
static const struct lsdc_kms_funcs ls7a1000_kms_funcs = {
.create_i2c = lsdc_create_i2c_chan,
@@ -25,6 +26,20 @@ static const struct lsdc_kms_funcs ls7a2000_kms_funcs = {
.crtc_init = ls7a2000_crtc_init,
};
+/*
+ * Most of hardware features of ls2k1000 are same with ls7a1000, we take the
+ * ls7a1000_kms_funcs as a prototype, copy and modify to form a variant for
+ * ls2k1000.
+ */
+static const struct lsdc_kms_funcs ls2k1000_kms_funcs = {
+ .create_i2c = ls2k1000_get_i2c,
+ .irq_handler = ls7a1000_dc_irq_handler,
+ .output_init = ls7a1000_output_init,
+ .cursor_plane_init = ls7a1000_cursor_plane_init,
+ .primary_plane_init = lsdc_primary_plane_init,
+ .crtc_init = ls7a1000_crtc_init,
+};
+
static const struct loongson_gfx_desc ls7a1000_gfx = {
.dc = {
.num_of_crtc = 2,
@@ -36,6 +51,7 @@ static const struct loongson_gfx_desc ls7a1000_gfx = {
.hw_cursor_h = 32,
