loongarch.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/4] LoongArch: Add Loongson-2K0500 BMC support
@ 2024-12-30  9:31 Binbin Zhou
  2024-12-30  9:31 ` [PATCH v1 1/4] mfd: ls2kbmc: Introduce Loongson-2K BMC MFD Core driver Binbin Zhou
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Binbin Zhou @ 2024-12-30  9:31 UTC (permalink / raw)
  To: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter
  Cc: Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Binbin Zhou

Hi all:

This patch set introduces the Loongson-2K0500 BMC.

It is a PCIe device present on servers similar to the Loongson-3C6000.
And it is a multifunctional device (MFD), such as display as a sub-function
of it.

For IPMI, according to the existing design, we use software simulation to
implement the KCS interface registers: Stauts/Command/Data_Out/Data_In.

Also since both host side and BMC side read and write kcs status, we use
fifo pointer to ensure data consistency.

For the display, based on simpledrm, the resolution is read from a fixed
position in the BMC since the hardware does not support auto-detection
of the resolution. Of course, we will try to support multiple
resolutions later, through a vbios-like approach.

Especially, for the BMC reset function, since the display will be
disconnected when BMC reset, we made a special treatment of re-push.

Based on this, I will present it in four patches:
patch-1: BMC device PCI resource allocation.
patch-2: IPMI implementation
patch-3: display, based on simpledrm
patch-4: BMC reboot support

Thanks.

Binbin Zhou (4):
  mfd: ls2kbmc: Introduce Loongson-2K BMC MFD Core driver
  ipmi: Add Loongson-2K BMC support
  drm/ls2kbmc: Add support for Loongson-2K BMC display
  drm/ls2kbmc: Add Loongson-2K BMC reset function support

 drivers/char/ipmi/Makefile       |   1 +
 drivers/char/ipmi/ipmi_si.h      |   8 +
 drivers/char/ipmi/ipmi_si_intf.c |   3 +
 drivers/char/ipmi/ipmi_si_ls2k.c | 250 +++++++++
 drivers/gpu/drm/tiny/Kconfig     |  18 +
 drivers/gpu/drm/tiny/Makefile    |   1 +
 drivers/gpu/drm/tiny/ls2kbmc.c   | 918 +++++++++++++++++++++++++++++++
 drivers/mfd/Kconfig              |  15 +
 drivers/mfd/Makefile             |   2 +
 drivers/mfd/ls2kbmc-mfd.c        | 145 +++++
 10 files changed, 1361 insertions(+)
 create mode 100644 drivers/char/ipmi/ipmi_si_ls2k.c
 create mode 100644 drivers/gpu/drm/tiny/ls2kbmc.c
 create mode 100644 drivers/mfd/ls2kbmc-mfd.c

-- 
2.43.5


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v1 1/4] mfd: ls2kbmc: Introduce Loongson-2K BMC MFD Core driver
  2024-12-30  9:31 [PATCH v1 0/4] LoongArch: Add Loongson-2K0500 BMC support Binbin Zhou
@ 2024-12-30  9:31 ` Binbin Zhou
  2024-12-30  9:31 ` [PATCH v1 2/4] ipmi: Add Loongson-2K BMC support Binbin Zhou
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Binbin Zhou @ 2024-12-30  9:31 UTC (permalink / raw)
  To: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter
  Cc: Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Binbin Zhou, Chong Qiao

The Loongson-2K Board Management Controller provides an PCIe
interface to the host to access the feature implemented in the BMC.

The BMC is assembled on a server similar to the server machine with
Loongson-3C6000 CPUs. It supports multiple sub-devices like DRM.

Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/mfd/Kconfig       |  15 ++++
 drivers/mfd/Makefile      |   2 +
 drivers/mfd/ls2kbmc-mfd.c | 145 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 162 insertions(+)
 create mode 100644 drivers/mfd/ls2kbmc-mfd.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index ae23b317a64e..97d9d52b7e8a 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2414,5 +2414,20 @@ config MFD_RSMU_SPI
 	  Additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_LS2K_BMC
+	tristate "Loongson-2K Board Management Controller Support"
+	depends on LOONGARCH
+	select MFD_CORE
+	help
+	  Say yes here to add support for the Loongson-2K BMC
+	  which is a Board Management Controller connected to the PCIe bus.
+	  The device supports multiple sub-devices like DRM.
+	  This driver provides common support for accessing the devices;
+	  additional drivers must be enabled in order to use the
+	  functionality of the BMC device.
+
+	  This driver can also be built as a module. If so the module
+	  will be called ls2kbmc-mfd.
+
 endmenu
 endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e057d6d6faef..3faec9638303 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -290,3 +290,5 @@ obj-$(CONFIG_MFD_ATC260X_I2C)	+= atc260x-i2c.o
 
 obj-$(CONFIG_MFD_RSMU_I2C)	+= rsmu_i2c.o rsmu_core.o
 obj-$(CONFIG_MFD_RSMU_SPI)	+= rsmu_spi.o rsmu_core.o
+
+obj-$(CONFIG_MFD_LS2K_BMC)	+= ls2kbmc-mfd.o
diff --git a/drivers/mfd/ls2kbmc-mfd.c b/drivers/mfd/ls2kbmc-mfd.c
new file mode 100644
index 000000000000..2912112c41c8
--- /dev/null
+++ b/drivers/mfd/ls2kbmc-mfd.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Loongson-2K Board Management Controller (BMC) MFD Core Driver.
+ *
+ * Copyright (C) 2024 Loongson Technology Corporation Limited.
+ *
+ * Originally written by Chong Qiao <qiaochong@loongson.cn>
+ * Rewritten for mainline by Binbin Zhou <zhoubinbin@loongson.cn>
+ */
+
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/pci_ids.h>
+#include <linux/platform_device.h>
+
+static struct resource ls2k_display_resources[] = {
+	{
+		.name	= "ls2kbmc-simplebuf-res",
+		.start	= SZ_16M + SZ_2M,
+		.end	= SZ_16M + SZ_2M + SZ_4M - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct resource ls2k_ipmi0_resources[] = {
+	{
+		.name	= "ipmi-res0",
+		.start	= SZ_16M + 0x00f00000,
+		.end	= SZ_16M + 0x00f00000 + 0x1c - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct resource ls2k_ipmi1_resources[] = {
+	{
+		.name	= "ipmi-res1",
+		.start	= SZ_16M + 0x00f00000 + 0x1c,
+		.end	= SZ_16M + 0x00f00000 + 0x1c * 2 - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct resource ls2k_ipmi2_resources[] = {
+	{
+		.name	= "ipmi-res2",
+		.start	= SZ_16M + 0x00f00000 + 0x1c * 2,
+		.end	= SZ_16M + 0x00f00000 + 0x1c * 3 - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct resource ls2k_ipmi3_resources[] = {
+	{
+		.name	= "ipmi-res3",
+		.start	= SZ_16M + 0x00f00000 + 0x1c * 3,
+		.end	= SZ_16M + 0x00f00000 + 0x1c * 4 - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct resource ls2k_ipmi4_resources[] = {
+	{
+		.name	= "ipmi-res4",
+		.start	= SZ_16M + 0x00f00000 + 0x1c * 4,
+		.end	= SZ_16M + 0x00f00000 + 0x1c * 5 - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct mfd_cell ls2k_bmc_cells[] = {
+	{
+		.name = "ls2kbmc-framebuffer",
+		.num_resources = ARRAY_SIZE(ls2k_display_resources),
+		.resources = ls2k_display_resources,
+	},
+	{
+		.name = "ls2k-ipmi-si",
+		.num_resources = ARRAY_SIZE(ls2k_ipmi0_resources),
+		.resources = ls2k_ipmi0_resources,
+	},
+	{
+		.name = "ls2k-ipmi-si",
+		.num_resources = ARRAY_SIZE(ls2k_ipmi1_resources),
+		.resources = ls2k_ipmi1_resources,
+	},
+	{
+		.name = "ls2k-ipmi-si",
+		.num_resources = ARRAY_SIZE(ls2k_ipmi2_resources),
+		.resources = ls2k_ipmi2_resources,
+	},
+	{
+		.name = "ls2k-ipmi-si",
+		.num_resources = ARRAY_SIZE(ls2k_ipmi3_resources),
+		.resources = ls2k_ipmi3_resources,
+	},
+	{
+		.name = "ls2k-ipmi-si",
+		.num_resources = ARRAY_SIZE(ls2k_ipmi4_resources),
+		.resources = ls2k_ipmi4_resources,
+	},
+};
+
+static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	int ret = 0;
+
+	ret = pci_enable_device(dev);
+	if (ret)
+		return ret;
+
+	ls2k_bmc_cells[0].platform_data = &dev;
+	ls2k_bmc_cells[0].pdata_size = sizeof(dev);
+
+	return devm_mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
+				    ls2k_bmc_cells, ARRAY_SIZE(ls2k_bmc_cells),
+				    &dev->resource[0], 0, NULL);
+}
+
+static void ls2k_bmc_remove(struct pci_dev *dev)
+{
+	pci_disable_device(dev);
+}
+
+static struct pci_device_id ls2k_bmc_devices[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x1a05) },
+	{ }
+};
+MODULE_DEVICE_TABLE(pci, ls2k_bmc_devices);
+
+static struct pci_driver ls2k_bmc_driver = {
+	.name = "ls2k-bmc",
+	.id_table = ls2k_bmc_devices,
+	.probe = ls2k_bmc_probe,
+	.remove = ls2k_bmc_remove,
+};
+
+module_pci_driver(ls2k_bmc_driver);
+
+MODULE_DESCRIPTION("Loongson-2K BMC driver");
+MODULE_AUTHOR("Loongson Technology Corporation Limited");
+MODULE_LICENSE("GPL");
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v1 2/4] ipmi: Add Loongson-2K BMC support
  2024-12-30  9:31 [PATCH v1 0/4] LoongArch: Add Loongson-2K0500 BMC support Binbin Zhou
  2024-12-30  9:31 ` [PATCH v1 1/4] mfd: ls2kbmc: Introduce Loongson-2K BMC MFD Core driver Binbin Zhou
@ 2024-12-30  9:31 ` Binbin Zhou
  2024-12-31 12:37   ` kernel test robot
  2025-01-03  4:29   ` kernel test robot
  2024-12-30  9:31 ` [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display Binbin Zhou
  2024-12-30  9:31 ` [PATCH v1 4/4] drm/ls2kbmc: Add Loongson-2K BMC reset function support Binbin Zhou
  3 siblings, 2 replies; 19+ messages in thread
From: Binbin Zhou @ 2024-12-30  9:31 UTC (permalink / raw)
  To: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter
  Cc: Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Binbin Zhou, Chong Qiao

This patch adds Loongson-2K BMC IPMI support.

According to the existing design, we use software simulation to
implement the KCS interface registers: Stauts/Command/Data_Out/Data_In.

Also since both host side and BMC side read and write kcs status, I use
fifo pointer to ensure data consistency.

Therefore I made the whole IPMI driver independent.

Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/char/ipmi/Makefile       |   1 +
 drivers/char/ipmi/ipmi_si.h      |   8 +
 drivers/char/ipmi/ipmi_si_intf.c |   3 +
 drivers/char/ipmi/ipmi_si_ls2k.c | 250 +++++++++++++++++++++++++++++++
 4 files changed, 262 insertions(+)
 create mode 100644 drivers/char/ipmi/ipmi_si_ls2k.c

diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
index e0944547c9d0..614be45863b4 100644
--- a/drivers/char/ipmi/Makefile
+++ b/drivers/char/ipmi/Makefile
@@ -7,6 +7,7 @@ ipmi_si-y := ipmi_si_intf.o ipmi_kcs_sm.o ipmi_smic_sm.o ipmi_bt_sm.o \
 	ipmi_si_hotmod.o ipmi_si_hardcode.o ipmi_si_platform.o \
 	ipmi_si_mem_io.o
 ipmi_si-$(CONFIG_HAS_IOPORT) += ipmi_si_port_io.o
+ipmi_si-$(CONFIG_LOONGARCH) += ipmi_si_ls2k.o
 ipmi_si-$(CONFIG_PCI) += ipmi_si_pci.o
 ipmi_si-$(CONFIG_PARISC) += ipmi_si_parisc.o
 
diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h
index a7ead2a4c753..0a4af352a42c 100644
--- a/drivers/char/ipmi/ipmi_si.h
+++ b/drivers/char/ipmi/ipmi_si.h
@@ -101,6 +101,14 @@ static inline void ipmi_si_parisc_init(void) { }
 static inline void ipmi_si_parisc_shutdown(void) { }
 #endif
 
+#ifdef CONFIG_LOONGARCH
+void ipmi_si_ls2k_init(void);
+void ipmi_si_ls2k_shutdown(void);
+#else
+static inline void ipmi_si_ls2k_init(void) { }
+static inline void ipmi_si_ls2k_shutdown(void) { }
+#endif
+
 int ipmi_si_port_setup(struct si_sm_io *io);
 int ipmi_si_mem_setup(struct si_sm_io *io);
 
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index eea23a3b966e..7227bc61be79 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -2108,6 +2108,7 @@ static int __init init_ipmi_si(void)
 	ipmi_si_pci_init();
 
 	ipmi_si_parisc_init();
+	ipmi_si_ls2k_init();
 
 	/* We prefer devices with interrupts, but in the case of a machine
 	   with multiple BMCs we assume that there will be several instances
@@ -2292,6 +2293,8 @@ static void cleanup_ipmi_si(void)
 
 	ipmi_si_platform_shutdown();
 
+	ipmi_si_ls2k_shutdown();
+
 	mutex_lock(&smi_infos_lock);
 	list_for_each_entry_safe(e, tmp_e, &smi_infos, link)
 		cleanup_one_si(e);
diff --git a/drivers/char/ipmi/ipmi_si_ls2k.c b/drivers/char/ipmi/ipmi_si_ls2k.c
new file mode 100644
index 000000000000..cb31bb989fca
--- /dev/null
+++ b/drivers/char/ipmi/ipmi_si_ls2k.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Loongson-2K BMC IPMI
+ *
+ * Copyright (C) 2024 Loongson Technology Corporation Limited.
+ *
+ * Originally written by Chong Qiao <qiaochong@loongson.cn>
+ * Rewritten for mainline by Binbin Zhou <zhoubinbin@loongson.cn>
+ */
+
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/types.h>
+
+#include "ipmi_si.h"
+
+#define LS2K_KCS_STS_OBF	BIT(0)
+#define LS2K_KCS_STS_IBF	BIT(1)
+#define LS2K_KCS_STS_SMS_ATN	BIT(2)
+#define LS2K_KCS_STS_CMD	BIT(3)
+
+#define LS2K_KCS_DATA_MASK	(LS2K_KCS_STS_OBF | LS2K_KCS_STS_IBF | LS2K_KCS_STS_CMD)
+
+/* Read and write fifo pointers for data consistency. */
+struct ls2k_fifo_flag {
+	u8 ibfh;
+	u8 ibft;
+	u8 obfh;
+	u8 obft;
+};
+
+struct ls2k_kcs_reg {
+	u8 status;
+	u8 data_out;
+	s16 data_in;
+	s16 cmd;
+};
+
+struct ls2k_kcs_data {
+	struct ls2k_fifo_flag fifo;
+	struct ls2k_kcs_reg reg;
+	u8 cmd_data;
+	u8 version;
+	u32 write_req;
+	u32 write_ack;
+	u32 reserved[2];
+};
+
+static void ls2k_set_obf(struct ls2k_kcs_data *ik, u8 sts)
+{
+	ik->reg.status = (ik->reg.status & ~LS2K_KCS_STS_OBF) | (sts & BIT(0));
+}
+
+static void ls2k_set_ibf(struct ls2k_kcs_data *ik, u8 sts)
+{
+	ik->reg.status = (ik->reg.status & ~LS2K_KCS_STS_IBF) | ((sts & BIT(0)) << 1);
+}
+
+static u8 ls2k_get_ibf(struct ls2k_kcs_data *ik)
+{
+	return (ik->reg.status >> 1) & BIT(0);
+}
+
+static unsigned char intf_sim_inb_v0(struct ls2k_kcs_data *ik,
+				     unsigned int offset)
+{
+	u32 inb = 0;
+
+	switch (offset & BIT(0)) {
+	case 0:
+		inb = ik->reg.data_out;
+		ls2k_set_obf(ik, 0);
+		break;
+	case 1:
+		inb = ik->reg.status;
+		break;
+	}
+
+	return inb;
+}
+
+static unsigned char intf_sim_inb_v1(struct ls2k_kcs_data *ik,
+				     unsigned int offset)
+{
+	u32 inb = 0;
+	int cmd;
+	bool obf, ibf;
+
+	obf = ik->fifo.obfh != ik->fifo.obft;
+	ibf = ik->fifo.ibfh != ik->fifo.ibft;
+	cmd = ik->cmd_data;
+
+	switch (offset & BIT(0)) {
+	case 0:
+		inb = ik->reg.data_out;
+		ik->fifo.obft = ik->fifo.obfh;
+		break;
+	case 1:
+		inb = ik->reg.status & ~LS2K_KCS_DATA_MASK;
+		inb |= obf | (ibf << 1) | (cmd << 3);
+		break;
+	}
+
+	return inb;
+}
+
+static unsigned char ls2k_mem_inb(const struct si_sm_io *io,
+				  unsigned int offset)
+{
+	struct ls2k_kcs_data *ik = io->addr;
+	int inb = 0;
+
+	if (ik->version == 0)
+		inb = intf_sim_inb_v0(ik, offset);
+	else if (ik->version == 1)
+		inb = intf_sim_inb_v1(ik, offset);
+
+	return inb;
+}
+
+static void intf_sim_outb_v0(struct ls2k_kcs_data *ik, unsigned int offset,
+			     unsigned char val)
+{
+	if (ls2k_get_ibf(ik))
+		return;
+
+	switch (offset & BIT(0)) {
+	case 0:
+		ik->reg.data_in = val;
+		ik->reg.status &= ~LS2K_KCS_STS_CMD;
+		break;
+
+	case 1:
+		ik->reg.cmd = val;
+		ik->reg.status |= LS2K_KCS_STS_CMD;
+		break;
+	}
+
+	ls2k_set_ibf(ik, 1);
+	ik->write_req++;
+}
+
+static void intf_sim_outb_v1(struct ls2k_kcs_data *ik, unsigned int offset,
+			     unsigned char val)
+{
+	if (ik->fifo.ibfh != ik->fifo.ibft)
+		return;
+
+	switch (offset & BIT(0)) {
+	case 0:
+		ik->reg.data_in = val;
+		ik->cmd_data = 0;
+		break;
+
+	case 1:
+		ik->reg.cmd = val;
+		ik->cmd_data = 1;
+		break;
+	}
+
+	ik->fifo.ibfh = !ik->fifo.ibft;
+	ik->write_req++;
+}
+
+static void ls2k_mem_outb(const struct si_sm_io *io, unsigned int offset,
+			  unsigned char val)
+{
+	struct ls2k_kcs_data *ik = io->addr;
+
+	if (ik->version == 0)
+		intf_sim_outb_v0(ik, offset, val);
+	else if (ik->version == 1)
+		intf_sim_outb_v1(ik, offset, val);
+}
+
+static void ls2k_mem_cleanup(struct si_sm_io *io)
+{
+	if (io->addr)
+		iounmap(io->addr);
+}
+
+static int ipmi_ls2k_sim_setup(struct si_sm_io *io)
+{
+	io->addr = ioremap(io->addr_data, io->regspacing);
+	if (!io->addr)
+		return -EIO;
+
+	io->inputb = ls2k_mem_inb;
+	io->outputb = ls2k_mem_outb;
+	io->io_cleanup = ls2k_mem_cleanup;
+
+	return 0;
+}
+
+static int ipmi_ls2k_probe(struct platform_device *pdev)
+{
+	struct si_sm_io io;
+
+	dev_info(&pdev->dev, "probing via ls2k platform");
+	memset(&io, 0, sizeof(io));
+
+	io.addr_source	= SI_PLATFORM;
+	io.si_type	= SI_KCS;
+	io.addr_space	= IPMI_MEM_ADDR_SPACE;
+	io.io_setup	= ipmi_ls2k_sim_setup;
+	io.addr_data	= pdev->resource[0].start;
+	io.regspacing	= pdev->resource[0].end - pdev->resource[0].start + 1;
+	io.regsize	= DEFAULT_REGSIZE;
+	io.regshift	= 0;
+	io.dev		= &pdev->dev;
+	io.irq		= 0;
+	if (io.irq)
+		io.irq_setup = ipmi_std_irq_setup;
+
+	dev_info(&pdev->dev, "%pR regsize %d spacing %d irq %d\n",
+		 &pdev->resource[0], io.regsize, io.regspacing, io.irq);
+
+	return ipmi_si_add_smi(&io);
+}
+
+static void ipmi_ls2k_remove(struct platform_device *pdev)
+{
+	ipmi_si_remove_by_dev(&pdev->dev);
+}
+
+struct platform_driver ipmi_ls2k_platform_driver = {
+	.driver = {
+		.name = "ls2k-ipmi-si",
+	},
+	.probe	= ipmi_ls2k_probe,
+	.remove	= ipmi_ls2k_remove,
+};
+
+static bool platform_registered;
+void ipmi_si_ls2k_init(void)
+{
+	int rv;
+
+	rv = platform_driver_register(&ipmi_ls2k_platform_driver);
+	if (rv)
+		pr_err("Unable to register driver: %d\n", rv);
+	else
+		platform_registered = true;
+}
+
+void ipmi_si_ls2k_shutdown(void)
+{
+	if (platform_registered)
+		platform_driver_unregister(&ipmi_ls2k_platform_driver);
+}
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display
  2024-12-30  9:31 [PATCH v1 0/4] LoongArch: Add Loongson-2K0500 BMC support Binbin Zhou
  2024-12-30  9:31 ` [PATCH v1 1/4] mfd: ls2kbmc: Introduce Loongson-2K BMC MFD Core driver Binbin Zhou
  2024-12-30  9:31 ` [PATCH v1 2/4] ipmi: Add Loongson-2K BMC support Binbin Zhou
@ 2024-12-30  9:31 ` Binbin Zhou
  2025-01-02  9:07   ` Thomas Zimmermann
  2025-01-15  8:48   ` Thomas Zimmermann
  2024-12-30  9:31 ` [PATCH v1 4/4] drm/ls2kbmc: Add Loongson-2K BMC reset function support Binbin Zhou
  3 siblings, 2 replies; 19+ messages in thread
From: Binbin Zhou @ 2024-12-30  9:31 UTC (permalink / raw)
  To: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter
  Cc: Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Binbin Zhou, Chong Qiao

Adds a driver for the Loongson-2K BMC display as a sub-function of
the BMC device.

Display-related scan output buffers, sizes, and display formats are
provided through the Loongson-2K BMC MFD driver.

Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/gpu/drm/tiny/Kconfig   |  18 +
 drivers/gpu/drm/tiny/Makefile  |   1 +
 drivers/gpu/drm/tiny/ls2kbmc.c | 636 +++++++++++++++++++++++++++++++++
 3 files changed, 655 insertions(+)
 create mode 100644 drivers/gpu/drm/tiny/ls2kbmc.c

diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index 94cbdb1337c0..5412f639a964 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -171,6 +171,24 @@ config TINYDRM_ILI9486
 
 	  If M is selected the module will be called ili9486.
 
+config TINYDRM_LS2KBMC
+	tristate "DRM support for Loongson-2K BMC display"
+	depends on DRM && MMU
+	depends on MFD_LS2K_BMC
+	select APERTURE_HELPERS
+	select DRM_CLIENT_SELECTION
+	select DRM_GEM_SHMEM_HELPER
+	select DRM_KMS_HELPER
+	help
+	  DRM driver for the Loongson-2K BMC display.
+
+	  This driver assumes that the display hardware has been initialized
+	  by the Loongson-2K BMC. Since the Loongson-2K BMC does not support
+	  resolution detection now, the scan buffer, size and display format
+	  are fixed and provided by the BMC.
+
+	  If M is selected the module will be called ls2kbmc.
+
 config TINYDRM_MI0283QT
 	tristate "DRM support for MI0283QT"
 	depends on DRM && SPI
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 4aaf56f8707d..fa4e1646db77 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_TINYDRM_ILI9163)		+= ili9163.o
 obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
 obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
 obj-$(CONFIG_TINYDRM_ILI9486)		+= ili9486.o
+obj-$(CONFIG_TINYDRM_LS2KBMC)		+= ls2kbmc.o
 obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
 obj-$(CONFIG_TINYDRM_REPAPER)		+= repaper.o
 obj-$(CONFIG_TINYDRM_SHARP_MEMORY)	+= sharp-memory.o
diff --git a/drivers/gpu/drm/tiny/ls2kbmc.c b/drivers/gpu/drm/tiny/ls2kbmc.c
new file mode 100644
index 000000000000..909d6c687193
--- /dev/null
+++ b/drivers/gpu/drm/tiny/ls2kbmc.c
@@ -0,0 +1,636 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * DRM driver for Loongson-2K BMC display
+ *
+ * Copyright (C) 2024 Loongson Technology Corporation Limited.
+ *
+ * Based on simpledrm
+ */
+
+#include <linux/aperture.h>
+#include <linux/minmax.h>
+#include <linux/pci.h>
+#include <linux/platform_data/simplefb.h>
+#include <linux/platform_device.h>
+
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_client_setup.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_damage_helper.h>
+#include <drm/drm_device.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fbdev_shmem.h>
+#include <drm/drm_format_helper.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_panic.h>
+#include <drm/drm_probe_helper.h>
+
+struct ls2kbmc_pdata {
+	struct pci_dev *pdev;
+	struct simplefb_platform_data pd;
+};
+
+/*
+ * Helpers for simplefb_platform_data
+ */
+
+static int
+simplefb_get_validated_int(struct drm_device *dev, const char *name,
+			   u32 value)
+{
+	if (value > INT_MAX) {
+		drm_err(dev, "simplefb: invalid framebuffer %s of %u\n",
+			name, value);
+		return -EINVAL;
+	}
+	return (int)value;
+}
+
+static int
+simplefb_get_validated_int0(struct drm_device *dev, const char *name,
+			    u32 value)
+{
+	if (!value) {
+		drm_err(dev, "simplefb: invalid framebuffer %s of %u\n",
+			name, value);
+		return -EINVAL;
+	}
+	return simplefb_get_validated_int(dev, name, value);
+}
+
+static const struct drm_format_info *
+simplefb_get_validated_format(struct drm_device *dev, const char *format_name)
+{
+	static const struct simplefb_format formats[] = SIMPLEFB_FORMATS;
+	const struct simplefb_format *fmt = formats;
+	const struct simplefb_format *end = fmt + ARRAY_SIZE(formats);
+	const struct drm_format_info *info;
+
+	if (!format_name) {
+		drm_err(dev, "simplefb: missing framebuffer format\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	while (fmt < end) {
+		if (!strcmp(format_name, fmt->name)) {
+			info = drm_format_info(fmt->fourcc);
+			if (!info)
+				return ERR_PTR(-EINVAL);
+			return info;
+		}
+		++fmt;
+	}
+
+	drm_err(dev, "simplefb: unknown framebuffer format %s\n",
+		format_name);
+
+	return ERR_PTR(-EINVAL);
+}
+
+static int
+simplefb_get_width_pd(struct drm_device *dev,
+		      const struct simplefb_platform_data *pd)
+{
+	return simplefb_get_validated_int0(dev, "width", pd->width);
+}
+
+static int
+simplefb_get_height_pd(struct drm_device *dev,
+		       const struct simplefb_platform_data *pd)
+{
+	return simplefb_get_validated_int0(dev, "height", pd->height);
+}
+
+static int
+simplefb_get_stride_pd(struct drm_device *dev,
+		       const struct simplefb_platform_data *pd)
+{
+	return simplefb_get_validated_int(dev, "stride", pd->stride);
+}
+
+static const struct drm_format_info *
+simplefb_get_format_pd(struct drm_device *dev,
+		       const struct simplefb_platform_data *pd)
+{
+	return simplefb_get_validated_format(dev, pd->format);
+}
+
+/*
+ * ls2kbmc Framebuffer device
+ */
+
+struct ls2kbmc_device {
+	struct drm_device dev;
+
+	/* simplefb settings */
+	struct drm_display_mode mode;
+	const struct drm_format_info *format;
+	unsigned int pitch;
+
+	/* memory management */
+	struct iosys_map screen_base;
+
+	/* modesetting */
+	u32 formats[8];
+	struct drm_plane primary_plane;
+	struct drm_crtc crtc;
+	struct drm_encoder encoder;
+	struct drm_connector connector;
+};
+
+static struct ls2kbmc_device *ls2kbmc_device_of_dev(struct drm_device *dev)
+{
+	return container_of(dev, struct ls2kbmc_device, dev);
+}
+
+/*
+ * Modesetting
+ */
+
+static const u64 ls2kbmc_primary_plane_format_modifiers[] = {
+	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_INVALID
+};
+
+static int ls2kbmc_primary_plane_helper_atomic_check(struct drm_plane *plane,
+						     struct drm_atomic_state *state)
+{
+	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+	struct drm_shadow_plane_state *new_shadow_plane_state =
+		to_drm_shadow_plane_state(new_plane_state);
+	struct drm_framebuffer *new_fb = new_plane_state->fb;
+	struct drm_crtc *new_crtc = new_plane_state->crtc;
+	struct drm_crtc_state *new_crtc_state = NULL;
+	struct drm_device *dev = plane->dev;
+	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(dev);
+	int ret;
+
+	if (new_crtc)
+		new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
+
+	ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state,
+						  DRM_PLANE_NO_SCALING,
+						  DRM_PLANE_NO_SCALING,
+						  false, false);
+	if (ret)
+		return ret;
+	else if (!new_plane_state->visible)
+		return 0;
+
+	if (new_fb->format != sdev->format) {
+		void *buf;
+
+		/* format conversion necessary; reserve buffer */
+		buf = drm_format_conv_state_reserve(&new_shadow_plane_state->fmtcnv_state,
+						    sdev->pitch, GFP_KERNEL);
+		if (!buf)
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void ls2kbmc_primary_plane_helper_atomic_update(struct drm_plane *plane,
+						       struct drm_atomic_state *state)
+{
+	struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
+	struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
+	struct drm_framebuffer *fb = plane_state->fb;
+	struct drm_device *dev = plane->dev;
+	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(dev);
+	struct drm_atomic_helper_damage_iter iter;
+	struct drm_rect damage;
+	int ret, idx;
+
+	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+	if (ret)
+		return;
+
+	if (!drm_dev_enter(dev, &idx))
+		goto out_drm_gem_fb_end_cpu_access;
+
+	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
+	drm_atomic_for_each_plane_damage(&iter, &damage) {
+		struct drm_rect dst_clip = plane_state->dst;
+		struct iosys_map dst = sdev->screen_base;
+
+		if (!drm_rect_intersect(&dst_clip, &damage))
+			continue;
+
+		iosys_map_incr(&dst, drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip));
+		drm_fb_blit(&dst, &sdev->pitch, sdev->format->format, shadow_plane_state->data,
+			    fb, &damage, &shadow_plane_state->fmtcnv_state);
+	}
+
+	drm_dev_exit(idx);
+out_drm_gem_fb_end_cpu_access:
+	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+}
+
+static void ls2kbmc_primary_plane_helper_atomic_disable(struct drm_plane *plane,
+							struct drm_atomic_state *state)
+{
+	struct drm_device *dev = plane->dev;
+	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(dev);
+	int idx;
+
+	if (!drm_dev_enter(dev, &idx))
+		return;
+
+	/* Clear screen to black if disabled */
+	iosys_map_memset(&sdev->screen_base, 0, 0, sdev->pitch * sdev->mode.vdisplay);
+
+	drm_dev_exit(idx);
+}
+
+static int ls2kbmc_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane,
+							   struct drm_scanout_buffer *sb)
+{
+	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(plane->dev);
+
+	sb->width = sdev->mode.hdisplay;
+	sb->height = sdev->mode.vdisplay;
+	sb->format = sdev->format;
+	sb->pitch[0] = sdev->pitch;
+	sb->map[0] = sdev->screen_base;
+
+	return 0;
+}
+
+static const struct drm_plane_helper_funcs ls2kbmc_primary_plane_helper_funcs = {
+	DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+	.atomic_check = ls2kbmc_primary_plane_helper_atomic_check,
+	.atomic_update = ls2kbmc_primary_plane_helper_atomic_update,
+	.atomic_disable = ls2kbmc_primary_plane_helper_atomic_disable,
+	.get_scanout_buffer = ls2kbmc_primary_plane_helper_get_scanout_buffer,
+};
+
+static const struct drm_plane_funcs ls2kbmc_primary_plane_funcs = {
+	.update_plane = drm_atomic_helper_update_plane,
+	.disable_plane = drm_atomic_helper_disable_plane,
+	.destroy = drm_plane_cleanup,
+	DRM_GEM_SHADOW_PLANE_FUNCS,
+};
+
+static enum drm_mode_status ls2kbmc_crtc_helper_mode_valid(struct drm_crtc *crtc,
+							   const struct drm_display_mode *mode)
+{
+	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(crtc->dev);
+
+	return drm_crtc_helper_mode_valid_fixed(crtc, mode, &sdev->mode);
+}
+
+/*
+ * The CRTC is always enabled. Screen updates are performed by
+ * the primary plane's atomic_update function. Disabling clears
+ * the screen in the primary plane's atomic_disable function.
+ */
+static const struct drm_crtc_helper_funcs ls2kbmc_crtc_helper_funcs = {
+	.mode_valid = ls2kbmc_crtc_helper_mode_valid,
+	.atomic_check = drm_crtc_helper_atomic_check,
+};
+
+static const struct drm_crtc_funcs ls2kbmc_crtc_funcs = {
+	.reset = drm_atomic_helper_crtc_reset,
+	.destroy = drm_crtc_cleanup,
+	.set_config = drm_atomic_helper_set_config,
+	.page_flip = drm_atomic_helper_page_flip,
+	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+};
+
+static const struct drm_encoder_funcs ls2kbmc_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+static int ls2kbmc_connector_helper_get_modes(struct drm_connector *connector)
+{
+	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(connector->dev);
+
+	return drm_connector_helper_get_modes_fixed(connector, &sdev->mode);
+}
+
+static const struct drm_connector_helper_funcs ls2kbmc_connector_helper_funcs = {
+	.get_modes = ls2kbmc_connector_helper_get_modes,
+};
+
+static const struct drm_connector_funcs ls2kbmc_connector_funcs = {
+	.reset = drm_atomic_helper_connector_reset,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static const struct drm_mode_config_funcs ls2kbmc_mode_config_funcs = {
+	.fb_create = drm_gem_fb_create_with_dirty,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+/*
+ * Init / Cleanup
+ */
+
+static struct drm_display_mode ls2kbmc_mode(unsigned int width, unsigned int height,
+					    unsigned int width_mm, unsigned int height_mm)
+{
+	const struct drm_display_mode mode = {
+		DRM_MODE_INIT(60, width, height, width_mm, height_mm)
+	};
+
+	return mode;
+}
+
+/*
+ * DRM driver
+ */
+
+DEFINE_DRM_GEM_FOPS(ls2kbmc_fops);
+
+static struct drm_driver ls2kbmc_driver = {
+	DRM_GEM_SHMEM_DRIVER_OPS,
+	DRM_FBDEV_SHMEM_DRIVER_OPS,
+	.name			= "simpledrm",
+	.desc			= "DRM driver for Loongson-2K BMC",
+	.date			= "20241211",
+	.major			= 1,
+	.minor			= 0,
+	.driver_features	= DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
+	.fops			= &ls2kbmc_fops,
+};
+
+/*
+ * Currently the Loongson-2K0500 BMC hardware does not have an i2c interface to
+ * adapt to the resolution.
+ * We set the resolution by presetting "video=1280x1024-16@2M" to the bmc memory.
+ */
+static int ls2kbmc_get_video_mode(struct pci_dev *pdev, struct simplefb_platform_data *pd)
+{
+	char *mode;
+	int depth, ret;
+
+	/* The pci mem bar last 16M is used to store the string. */
+	mode = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0) + SZ_16M, SZ_16M);
+	if (!mode)
+		return -ENOMEM;
+
+	/*env at last 16M's beginning, first env is video */
+	if (!strncmp(mode, "video=", 6))
+		mode = mode + 6;
+
+	ret = kstrtoint(strsep(&mode, "x"), 10, &pd->width);
+	if (ret)
+		return ret;
+
+	ret = kstrtoint(strsep(&mode, "-"), 10, &pd->height);
+	if (ret)
+		return ret;
+
+	ret = kstrtoint(strsep(&mode, "@"), 10, &depth);
+	if (ret)
+		return ret;
+
+	pd->stride = pd->width * depth / 8;
+	pd->format = depth == 32 ? "a8r8g8b8" : "r5g6b5";
+
+	return 0;
+}
+
+static struct ls2kbmc_device *ls2kbmc_device_create(struct drm_driver *drv,
+						    struct platform_device *pdev,
+						    struct ls2kbmc_pdata *priv)
+{
+	struct pci_dev *ppdev = priv->pdev;
+	struct simplefb_platform_data *pd = &priv->pd;
+	struct ls2kbmc_device *sdev;
+	struct drm_device *dev;
+	int width, height, stride;
+	int width_mm = 0, height_mm = 0;
+	const struct drm_format_info *format;
+	struct resource *res, *mem = NULL;
+	struct drm_plane *primary_plane;
+	struct drm_crtc *crtc;
+	struct drm_encoder *encoder;
+	struct drm_connector *connector;
+	unsigned long max_width, max_height;
+	void __iomem *screen_base;
+	size_t nformats;
+	int ret;
+
+	sdev = devm_drm_dev_alloc(&pdev->dev, drv, struct ls2kbmc_device, dev);
+	if (IS_ERR(sdev))
+		return ERR_CAST(sdev);
+	dev = &sdev->dev;
+	platform_set_drvdata(pdev, sdev);
+
+	ret = ls2kbmc_get_video_mode(ppdev, pd);
+	if (ret) {
+		drm_err(dev, "no simplefb configuration found\n");
+		return ERR_PTR(ret);
+	}
+
+	width = simplefb_get_width_pd(dev, pd);
+	if (width < 0)
+		return ERR_PTR(width);
+
+	height = simplefb_get_height_pd(dev, pd);
+	if (height < 0)
+		return ERR_PTR(height);
+
+	stride = simplefb_get_stride_pd(dev, pd);
+	if (stride < 0)
+		return ERR_PTR(stride);
+
+	if (!stride) {
+		stride = drm_format_info_min_pitch(format, 0, width);
+		if (drm_WARN_ON(dev, !stride))
+			return ERR_PTR(-EINVAL);
+	}
+
+	format = simplefb_get_format_pd(dev, pd);
+	if (IS_ERR(format))
+		return ERR_CAST(format);
+
+	/*
+	 * Assume a monitor resolution of 96 dpi if physical dimensions
+	 * are not specified to get a somewhat reasonable screen size.
+	 */
+	if (!width_mm)
+		width_mm = DRM_MODE_RES_MM(width, 96ul);
+	if (!height_mm)
+		height_mm = DRM_MODE_RES_MM(height, 96ul);
+
+	sdev->mode = ls2kbmc_mode(width, height, width_mm, height_mm);
+	sdev->format = format;
+	sdev->pitch = stride;
+
+	drm_dbg(dev, "display mode={" DRM_MODE_FMT "}\n", DRM_MODE_ARG(&sdev->mode));
+	drm_dbg(dev, "framebuffer format=%p4cc, size=%dx%d, stride=%d byte\n",
+		&format->format, width, height, stride);
+
+	/*
+	 * Memory management
+	 */
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return ERR_PTR(-EINVAL);
+
+	ret = aperture_remove_conflicting_pci_devices(ppdev, ls2kbmc_driver.name);
+	if (ret) {
+		drm_err(dev, "could not acquire memory range %pr: %d\n", res, ret);
+		return ERR_PTR(ret);
+	}
+
+	drm_dbg(dev, "using I/O memory framebuffer at %pr\n", res);
+
+	mem = devm_request_mem_region(&ppdev->dev, res->start, resource_size(res),
+				      drv->name);
+	if (!mem) {
+		/*
+		 * We cannot make this fatal. Sometimes this comes from magic
+		 * spaces our resource handlers simply don't know about. Use
+		 * the I/O-memory resource as-is and try to map that instead.
+		 */
+		drm_warn(dev, "could not acquire memory region %pr\n", res);
+		mem = res;
+	}
+
+	screen_base = devm_ioremap_wc(&ppdev->dev, mem->start, resource_size(mem));
+	if (!screen_base)
+		return ERR_PTR(-ENOMEM);
+
+	iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
+
+	/*
+	 * Modesetting
+	 */
+
+	ret = drmm_mode_config_init(dev);
+	if (ret)
+		return ERR_PTR(ret);
+
+	max_width = max_t(unsigned long, width, DRM_SHADOW_PLANE_MAX_WIDTH);
+	max_height = max_t(unsigned long, height, DRM_SHADOW_PLANE_MAX_HEIGHT);
+
+	dev->mode_config.min_width = width;
+	dev->mode_config.max_width = max_width;
+	dev->mode_config.min_height = height;
+	dev->mode_config.max_height = max_height;
+	dev->mode_config.preferred_depth = format->depth;
+	dev->mode_config.funcs = &ls2kbmc_mode_config_funcs;
+
+	/* Primary plane */
+
+	nformats = drm_fb_build_fourcc_list(dev, &format->format, 1,
+					    sdev->formats, ARRAY_SIZE(sdev->formats));
+
+	primary_plane = &sdev->primary_plane;
+	ret = drm_universal_plane_init(dev, primary_plane, 0, &ls2kbmc_primary_plane_funcs,
+				       sdev->formats, nformats,
+				       ls2kbmc_primary_plane_format_modifiers,
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret)
+		return ERR_PTR(ret);
+	drm_plane_helper_add(primary_plane, &ls2kbmc_primary_plane_helper_funcs);
+	drm_plane_enable_fb_damage_clips(primary_plane);
+
+	/* CRTC */
+
+	crtc = &sdev->crtc;
+	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
+					&ls2kbmc_crtc_funcs, NULL);
+	if (ret)
+		return ERR_PTR(ret);
+	drm_crtc_helper_add(crtc, &ls2kbmc_crtc_helper_funcs);
+
+	/* Encoder */
+
+	encoder = &sdev->encoder;
+	ret = drm_encoder_init(dev, encoder, &ls2kbmc_encoder_funcs,
+			       DRM_MODE_ENCODER_NONE, NULL);
+	if (ret)
+		return ERR_PTR(ret);
+	encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+	/* Connector */
+
+	connector = &sdev->connector;
+	ret = drm_connector_init(dev, connector, &ls2kbmc_connector_funcs,
+				 DRM_MODE_CONNECTOR_Unknown);
+	if (ret)
+		return ERR_PTR(ret);
+	drm_connector_helper_add(connector, &ls2kbmc_connector_helper_funcs);
+	drm_connector_set_panel_orientation_with_quirk(connector,
+						       DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
+						       width, height);
+
+	ret = drm_connector_attach_encoder(connector, encoder);
+	if (ret)
+		return ERR_PTR(ret);
+
+	drm_mode_config_reset(dev);
+
+	return sdev;
+}
+
+/*
+ * Platform driver
+ */
+
+static int ls2kbmc_probe(struct platform_device *pdev)
+{
+	struct ls2kbmc_device *sdev;
+	struct ls2kbmc_pdata *priv;
+	struct drm_device *dev;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (IS_ERR(priv))
+		return -ENOMEM;
+
+	priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
+
+	sdev = ls2kbmc_device_create(&ls2kbmc_driver, pdev, priv);
+	if (IS_ERR(sdev))
+		return PTR_ERR(sdev);
+	dev = &sdev->dev;
+
+	ret = drm_dev_register(dev, 0);
+	if (ret)
+		return ret;
+
+	drm_client_setup(dev, sdev->format);
+
+	return 0;
+}
+
+static void ls2kbmc_remove(struct platform_device *pdev)
+{
+	struct ls2kbmc_device *sdev = platform_get_drvdata(pdev);
+	struct drm_device *dev = &sdev->dev;
+
+	drm_dev_unplug(dev);
+}
+
+static struct platform_driver ls2kbmc_platform_driver = {
+	.driver = {
+		.name = "ls2kbmc-framebuffer",
+	},
+	.probe = ls2kbmc_probe,
+	.remove = ls2kbmc_remove,
+};
+
+module_platform_driver(ls2kbmc_platform_driver);
+
+MODULE_DESCRIPTION("DRM driver for Loongson-2K BMC");
+MODULE_LICENSE("GPL");
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v1 4/4] drm/ls2kbmc: Add Loongson-2K BMC reset function support
  2024-12-30  9:31 [PATCH v1 0/4] LoongArch: Add Loongson-2K0500 BMC support Binbin Zhou
                   ` (2 preceding siblings ...)
  2024-12-30  9:31 ` [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display Binbin Zhou
@ 2024-12-30  9:31 ` Binbin Zhou
  2025-01-15  8:57   ` Thomas Zimmermann
  3 siblings, 1 reply; 19+ messages in thread
From: Binbin Zhou @ 2024-12-30  9:31 UTC (permalink / raw)
  To: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter
  Cc: Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Binbin Zhou, Chong Qiao

Since the display is a sub-function of the Loongson-2K BMC, when the
BMC reset, the entire BMC PCIe is disconnected, including the display
which is interrupted.

Not only do you need to save/restore the relevant PCIe registers
throughout the reset process, but you also need to re-push the display
to the monitor at the end.

Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/gpu/drm/tiny/ls2kbmc.c | 284 ++++++++++++++++++++++++++++++++-
 1 file changed, 283 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tiny/ls2kbmc.c b/drivers/gpu/drm/tiny/ls2kbmc.c
index 909d6c687193..4b440f20cb4d 100644
--- a/drivers/gpu/drm/tiny/ls2kbmc.c
+++ b/drivers/gpu/drm/tiny/ls2kbmc.c
@@ -8,10 +8,12 @@
  */
 
 #include <linux/aperture.h>
+#include <linux/delay.h>
 #include <linux/minmax.h>
 #include <linux/pci.h>
 #include <linux/platform_data/simplefb.h>
 #include <linux/platform_device.h>
+#include <linux/stop_machine.h>
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_state_helper.h>
@@ -32,9 +34,27 @@
 #include <drm/drm_panic.h>
 #include <drm/drm_probe_helper.h>
 
+#define BMC_RESET_DELAY	(60 * HZ)
+#define BMC_RESET_WAIT	10000
+
+static const u32 index[] = { 0x4, 0x10, 0x14, 0x18, 0x1c, 0x20, 0x24,
+			     0x30, 0x3c, 0x54, 0x58, 0x78, 0x7c, 0x80 };
+static const u32 cindex[] = { 0x4, 0x10, 0x3c };
+
+struct ls2kbmc_pci_data {
+	u32 d80c;
+	u32 d71c;
+	u32 data[14];
+	u32 cdata[3];
+};
+
 struct ls2kbmc_pdata {
 	struct pci_dev *pdev;
+	struct drm_device *ddev;
+	struct work_struct bmc_work;
+	unsigned long reset_time;
 	struct simplefb_platform_data pd;
+	struct ls2kbmc_pci_data pci_data;
 };
 
 /*
@@ -583,6 +603,265 @@ static struct ls2kbmc_device *ls2kbmc_device_create(struct drm_driver *drv,
 	return sdev;
 }
 
+static bool ls2kbmc_bar0_addr_is_set(struct pci_dev *ppdev)
+{
+	u32 addr;
+
+	pci_read_config_dword(ppdev, PCI_BASE_ADDRESS_0, &addr);
+	addr &= PCI_BASE_ADDRESS_MEM_MASK;
+
+	return addr ? true : false;
+}
+
+static void ls2kbmc_save_pci_data(struct ls2kbmc_pdata *priv)
+{
+	struct pci_dev *pdev = priv->pdev;
+	struct pci_dev *parent = pdev->bus->self;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(index); i++)
+		pci_read_config_dword(parent, index[i], &priv->pci_data.data[i]);
+
+	for (i = 0; i < ARRAY_SIZE(cindex); i++)
+		pci_read_config_dword(pdev, cindex[i], &priv->pci_data.cdata[i]);
+
+	pci_read_config_dword(parent, 0x80c, &priv->pci_data.d80c);
+	priv->pci_data.d80c = (priv->pci_data.d80c & ~(3 << 17)) | (1 << 17);
+
+	pci_read_config_dword(parent, 0x71c, &priv->pci_data.d71c);
+	priv->pci_data.d71c |= 1 << 26;
+}
+
+static bool ls2kbmc_check_pcie_connected(struct pci_dev *parent, struct drm_device *dev)
+{
+	void __iomem *mmio;
+	int sts, timeout = 10000;
+
+	mmio = pci_iomap(parent, 0, 0x100);
+	if (!mmio)
+		return false;
+
+	writel(readl(mmio) | 0x8, mmio);
+	while (timeout) {
+		sts = readl(mmio + 0xc);
+		if ((sts & 0x11) == 0x11)
+			break;
+		mdelay(1);
+		timeout--;
+	}
+
+	pci_iounmap(parent, mmio);
+
+	if (!timeout) {
+		drm_err(dev, "pcie train failed status=0x%x\n", sts);
+		return false;
+	}
+
+	return true;
+}
+
+static int ls2kbmc_recove_pci_data(void *data)
+{
+	struct ls2kbmc_pdata *priv = data;
+	struct pci_dev *pdev = priv->pdev;
+	struct drm_device *dev = priv->ddev;
+	struct pci_dev *parent = pdev->bus->self;
+	u32 i, timeout, retry = 0;
+	bool ready;
+
+	pci_write_config_dword(parent, PCI_BASE_ADDRESS_2, 0);
+	pci_write_config_dword(parent, PCI_BASE_ADDRESS_3, 0);
+	pci_write_config_dword(parent, PCI_BASE_ADDRESS_4, 0);
+
+	timeout = 10000;
+	while (timeout) {
+		ready = ls2kbmc_bar0_addr_is_set(parent);
+		if (!ready)
+			break;
+		mdelay(1);
+		timeout--;
+	};
+
+	if (!timeout)
+		drm_warn(dev, "bar not clear 0\n");
+
+retrain:
+	for (i = 0; i < ARRAY_SIZE(index); i++)
+		pci_write_config_dword(parent, index[i], priv->pci_data.data[i]);
+
+	pci_write_config_dword(parent, 0x80c, priv->pci_data.d80c);
+	pci_write_config_dword(parent, 0x71c, priv->pci_data.d71c);
+
+	/* Check if the pcie is connected */
+	ready = ls2kbmc_check_pcie_connected(parent, dev);
+	if (!ready)
+		return ready;
+
+	for (i = 0; i < ARRAY_SIZE(cindex); i++)
+		pci_write_config_dword(pdev, cindex[i], priv->pci_data.cdata[i]);
+
+	drm_info(dev, "pcie recovered done\n");
+
+	if (!retry) {
+		/*wait u-boot ddr config */
+		mdelay(BMC_RESET_WAIT);
+		ready = ls2kbmc_bar0_addr_is_set(parent);
+		if (!ready) {
+			retry = 1;
+			goto retrain;
+		}
+	}
+
+	return 0;
+}
+
+static int ls2kbmc_push_drm_mode(struct drm_device *dev)
+{
+	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(dev);
+	struct drm_crtc *crtc = &sdev->crtc;
+	struct drm_plane *plane = crtc->primary;
+	struct drm_connector *connector = &sdev->connector;
+	struct drm_framebuffer *fb = NULL;
+	struct drm_mode_set set;
+	struct drm_modeset_acquire_ctx ctx;
+	int ret;
+
+	mutex_lock(&dev->mode_config.mutex);
+	connector->funcs->fill_modes(connector, 4096, 4096);
+	mutex_unlock(&dev->mode_config.mutex);
+
+	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx,
+				   DRM_MODESET_ACQUIRE_INTERRUPTIBLE, ret);
+
+	if (plane->state)
+		fb = plane->state->fb;
+	else
+		fb = plane->fb;
+
+	if (!fb) {
+		drm_dbg(dev, "CRTC doesn't have current FB\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	drm_framebuffer_get(fb);
+
+	set.crtc = crtc;
+	set.x = 0;
+	set.y = 0;
+	set.mode = &sdev->mode;
+	set.connectors = &connector;
+	set.num_connectors = 1;
+	set.fb = fb;
+
+	ret = crtc->funcs->set_config(&set, &ctx);
+
+out:
+	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
+
+	return ret;
+}
+
+static void ls2kbmc_events_fn(struct work_struct *work)
+{
+	struct ls2kbmc_pdata *priv = container_of(work, struct ls2kbmc_pdata, bmc_work);
+
+	/*
+	 * The pcie is lost when the BMC resets,
+	 * at which point access to the pcie from other CPUs
+	 * is suspended to prevent a crash.
+	 */
+	stop_machine(ls2kbmc_recove_pci_data, priv, NULL);
+
+	drm_info(priv->ddev, "redraw console\n");
+
+	/* We need to re-push the display due to previous pcie loss. */
+	ls2kbmc_push_drm_mode(priv->ddev);
+}
+
+static irqreturn_t ls2kbmc_interrupt(int irq, void *arg)
+{
+	struct ls2kbmc_pdata *priv = arg;
+
+	if (system_state != SYSTEM_RUNNING)
+		return IRQ_HANDLED;
+
+	/* skip interrupt in BMC_RESET_DELAY */
+	if (time_after(jiffies, priv->reset_time + BMC_RESET_DELAY))
+		schedule_work(&priv->bmc_work);
+
+	priv->reset_time = jiffies;
+
+	return IRQ_HANDLED;
+}
+
+#define BMC_RESET_GPIO			14
+#define LOONGSON_GPIO_REG_BASE		0x1fe00500
+#define LOONGSON_GPIO_REG_SIZE		0x18
+#define LOONGSON_GPIO_OEN		0x0
+#define LOONGSON_GPIO_FUNC		0x4
+#define LOONGSON_GPIO_INTPOL		0x10
+#define LOONGSON_GPIO_INTEN		0x14
+
+/* The gpio interrupt is a watchdog interrupt that is triggered when the BMC resets. */
+static int ls2kbmc_gpio_reset_handler(struct ls2kbmc_pdata *priv)
+{
+	int irq, ret = 0;
+	int gsi = 16 + (BMC_RESET_GPIO & 7);
+	void __iomem *gpio_base;
+
+	/* Since Loongson-3A hardware does not support GPIO interrupt cascade,
+	 * chip->gpio_to_irq() cannot be implemented,
+	 * here acpi_register_gsi() is used to get gpio irq.
+	 */
+	irq = acpi_register_gsi(NULL, gsi, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW);
+	if (irq < 0)
+		return irq;
+
+	gpio_base = ioremap(LOONGSON_GPIO_REG_BASE, LOONGSON_GPIO_REG_SIZE);
+	if (!gpio_base) {
+		acpi_unregister_gsi(gsi);
+		return PTR_ERR(gpio_base);
+	}
+
+	writel(readl(gpio_base + LOONGSON_GPIO_OEN) | BIT(BMC_RESET_GPIO),
+	       gpio_base + LOONGSON_GPIO_OEN);
+	writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(BMC_RESET_GPIO),
+	       gpio_base + LOONGSON_GPIO_FUNC);
+	writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(BMC_RESET_GPIO),
+	       gpio_base + LOONGSON_GPIO_INTPOL);
+	writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(BMC_RESET_GPIO),
+	       gpio_base + LOONGSON_GPIO_INTEN);
+
+	ret = request_irq(irq, ls2kbmc_interrupt, IRQF_SHARED | IRQF_TRIGGER_FALLING,
+			  "ls2kbmc gpio", priv);
+
+	acpi_unregister_gsi(gsi);
+	iounmap(gpio_base);
+
+	return ret;
+}
+
+static int ls2kbmc_pdata_initial(struct platform_device *pdev, struct ls2kbmc_pdata *priv)
+{
+	int ret;
+
+	priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
+
+	ls2kbmc_save_pci_data(priv);
+
+	INIT_WORK(&priv->bmc_work, ls2kbmc_events_fn);
+
+	ret = request_irq(priv->pdev->irq, ls2kbmc_interrupt,
+			  IRQF_SHARED | IRQF_TRIGGER_RISING, "ls2kbmc pcie", priv);
+	if (ret) {
+		pr_err("request_irq(%d) failed\n", priv->pdev->irq);
+		return ret;
+	}
+
+	return ls2kbmc_gpio_reset_handler(priv);
+}
+
 /*
  * Platform driver
  */
@@ -598,12 +877,15 @@ static int ls2kbmc_probe(struct platform_device *pdev)
 	if (IS_ERR(priv))
 		return -ENOMEM;
 
-	priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
+	ret = ls2kbmc_pdata_initial(pdev, priv);
+	if (ret)
+		return ret;
 
 	sdev = ls2kbmc_device_create(&ls2kbmc_driver, pdev, priv);
 	if (IS_ERR(sdev))
 		return PTR_ERR(sdev);
 	dev = &sdev->dev;
+	priv->ddev = &sdev->dev;
 
 	ret = drm_dev_register(dev, 0);
 	if (ret)
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 2/4] ipmi: Add Loongson-2K BMC support
  2024-12-30  9:31 ` [PATCH v1 2/4] ipmi: Add Loongson-2K BMC support Binbin Zhou
@ 2024-12-31 12:37   ` kernel test robot
  2025-01-03  4:29   ` kernel test robot
  1 sibling, 0 replies; 19+ messages in thread
From: kernel test robot @ 2024-12-31 12:37 UTC (permalink / raw)
  To: Binbin Zhou, Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter
  Cc: oe-kbuild-all, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi Binbin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.13-rc5]
[cannot apply to cminyard-ipmi/for-next lee-mfd/for-mfd-next lee-mfd/for-mfd-fixes next-20241220]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Binbin-Zhou/mfd-ls2kbmc-Introduce-Loongson-2K-BMC-MFD-Core-driver/20241230-174205
base:   linus/master
patch link:    https://lore.kernel.org/r/a4cfdeed1a91a7a12c7ebe56bed2ba94991c4065.1735550269.git.zhoubinbin%40loongson.cn
patch subject: [PATCH v1 2/4] ipmi: Add Loongson-2K BMC support
config: loongarch-randconfig-r123-20241231 (https://download.01.org/0day-ci/archive/20241231/202412311950.Ij7v1vKv-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20241231/202412311950.Ij7v1vKv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412311950.Ij7v1vKv-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/char/ipmi/ipmi_si_ls2k.c:110:38: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct ls2k_kcs_data *ik @@     got void [noderef] __iomem *const addr @@
   drivers/char/ipmi/ipmi_si_ls2k.c:110:38: sparse:     expected struct ls2k_kcs_data *ik
   drivers/char/ipmi/ipmi_si_ls2k.c:110:38: sparse:     got void [noderef] __iomem *const addr
   drivers/char/ipmi/ipmi_si_ls2k.c:168:38: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct ls2k_kcs_data *ik @@     got void [noderef] __iomem *const addr @@
   drivers/char/ipmi/ipmi_si_ls2k.c:168:38: sparse:     expected struct ls2k_kcs_data *ik
   drivers/char/ipmi/ipmi_si_ls2k.c:168:38: sparse:     got void [noderef] __iomem *const addr
>> drivers/char/ipmi/ipmi_si_ls2k.c:226:24: sparse: sparse: symbol 'ipmi_ls2k_platform_driver' was not declared. Should it be static?

vim +110 drivers/char/ipmi/ipmi_si_ls2k.c

   106	
   107	static unsigned char ls2k_mem_inb(const struct si_sm_io *io,
   108					  unsigned int offset)
   109	{
 > 110		struct ls2k_kcs_data *ik = io->addr;
   111		int inb = 0;
   112	
   113		if (ik->version == 0)
   114			inb = intf_sim_inb_v0(ik, offset);
   115		else if (ik->version == 1)
   116			inb = intf_sim_inb_v1(ik, offset);
   117	
   118		return inb;
   119	}
   120	
   121	static void intf_sim_outb_v0(struct ls2k_kcs_data *ik, unsigned int offset,
   122				     unsigned char val)
   123	{
   124		if (ls2k_get_ibf(ik))
   125			return;
   126	
   127		switch (offset & BIT(0)) {
   128		case 0:
   129			ik->reg.data_in = val;
   130			ik->reg.status &= ~LS2K_KCS_STS_CMD;
   131			break;
   132	
   133		case 1:
   134			ik->reg.cmd = val;
   135			ik->reg.status |= LS2K_KCS_STS_CMD;
   136			break;
   137		}
   138	
   139		ls2k_set_ibf(ik, 1);
   140		ik->write_req++;
   141	}
   142	
   143	static void intf_sim_outb_v1(struct ls2k_kcs_data *ik, unsigned int offset,
   144				     unsigned char val)
   145	{
   146		if (ik->fifo.ibfh != ik->fifo.ibft)
   147			return;
   148	
   149		switch (offset & BIT(0)) {
   150		case 0:
   151			ik->reg.data_in = val;
   152			ik->cmd_data = 0;
   153			break;
   154	
   155		case 1:
   156			ik->reg.cmd = val;
   157			ik->cmd_data = 1;
   158			break;
   159		}
   160	
   161		ik->fifo.ibfh = !ik->fifo.ibft;
   162		ik->write_req++;
   163	}
   164	
   165	static void ls2k_mem_outb(const struct si_sm_io *io, unsigned int offset,
   166				  unsigned char val)
   167	{
   168		struct ls2k_kcs_data *ik = io->addr;
   169	
   170		if (ik->version == 0)
   171			intf_sim_outb_v0(ik, offset, val);
   172		else if (ik->version == 1)
   173			intf_sim_outb_v1(ik, offset, val);
   174	}
   175	
   176	static void ls2k_mem_cleanup(struct si_sm_io *io)
   177	{
   178		if (io->addr)
   179			iounmap(io->addr);
   180	}
   181	
   182	static int ipmi_ls2k_sim_setup(struct si_sm_io *io)
   183	{
   184		io->addr = ioremap(io->addr_data, io->regspacing);
   185		if (!io->addr)
   186			return -EIO;
   187	
   188		io->inputb = ls2k_mem_inb;
   189		io->outputb = ls2k_mem_outb;
   190		io->io_cleanup = ls2k_mem_cleanup;
   191	
   192		return 0;
   193	}
   194	
   195	static int ipmi_ls2k_probe(struct platform_device *pdev)
   196	{
   197		struct si_sm_io io;
   198	
   199		dev_info(&pdev->dev, "probing via ls2k platform");
   200		memset(&io, 0, sizeof(io));
   201	
   202		io.addr_source	= SI_PLATFORM;
   203		io.si_type	= SI_KCS;
   204		io.addr_space	= IPMI_MEM_ADDR_SPACE;
   205		io.io_setup	= ipmi_ls2k_sim_setup;
   206		io.addr_data	= pdev->resource[0].start;
   207		io.regspacing	= pdev->resource[0].end - pdev->resource[0].start + 1;
   208		io.regsize	= DEFAULT_REGSIZE;
   209		io.regshift	= 0;
   210		io.dev		= &pdev->dev;
   211		io.irq		= 0;
   212		if (io.irq)
   213			io.irq_setup = ipmi_std_irq_setup;
   214	
   215		dev_info(&pdev->dev, "%pR regsize %d spacing %d irq %d\n",
   216			 &pdev->resource[0], io.regsize, io.regspacing, io.irq);
   217	
   218		return ipmi_si_add_smi(&io);
   219	}
   220	
   221	static void ipmi_ls2k_remove(struct platform_device *pdev)
   222	{
   223		ipmi_si_remove_by_dev(&pdev->dev);
   224	}
   225	
 > 226	struct platform_driver ipmi_ls2k_platform_driver = {
   227		.driver = {
   228			.name = "ls2k-ipmi-si",
   229		},
   230		.probe	= ipmi_ls2k_probe,
   231		.remove	= ipmi_ls2k_remove,
   232	};
   233	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display
  2024-12-30  9:31 ` [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display Binbin Zhou
@ 2025-01-02  9:07   ` Thomas Zimmermann
  2025-01-02 12:55     ` Binbin Zhou
  2025-01-15  8:48   ` Thomas Zimmermann
  1 sibling, 1 reply; 19+ messages in thread
From: Thomas Zimmermann @ 2025-01-02  9:07 UTC (permalink / raw)
  To: Binbin Zhou, Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter
  Cc: Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi


Am 30.12.24 um 10:31 schrieb Binbin Zhou:
[...]
> +
> +static struct platform_driver ls2kbmc_platform_driver = {
> +	.driver = {
> +		.name = "ls2kbmc-framebuffer",

The driver is mostly a copy of simpledrm. Why don't you use 
"simple-framebuffer" for your device name? You could use simpledrm 
directly then.

Best regards
Thomas

> +	},
> +	.probe = ls2kbmc_probe,
> +	.remove = ls2kbmc_remove,
> +};
> +
> +module_platform_driver(ls2kbmc_platform_driver);
> +
> +MODULE_DESCRIPTION("DRM driver for Loongson-2K BMC");
> +MODULE_LICENSE("GPL");

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display
  2025-01-02  9:07   ` Thomas Zimmermann
@ 2025-01-02 12:55     ` Binbin Zhou
  2025-01-02 13:32       ` Thomas Zimmermann
  0 siblings, 1 reply; 19+ messages in thread
From: Binbin Zhou @ 2025-01-02 12:55 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
	Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi Thomas:

Thanks for your reply.

On Thu, Jan 2, 2025 at 5:07 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
>
> Am 30.12.24 um 10:31 schrieb Binbin Zhou:
> [...]
> > +
> > +static struct platform_driver ls2kbmc_platform_driver = {
> > +     .driver = {
> > +             .name = "ls2kbmc-framebuffer",
>
> The driver is mostly a copy of simpledrm. Why don't you use
> "simple-framebuffer" for your device name? You could use simpledrm
> directly then.

Ah, indeed, the driver is based on simpledrm.

Initially, I also tried to use simpledrm directly, but it will fail in
drm memory acquire.
Because although we register the driver in platform form, its memory
belongs to pci space and we can see the corresponding pci probe and
resource allocation in Patch-1.
Therefore, we need to use aperture_remove_conflicting_pci_devices().

Also, since we are using BMC display, the display will be disconnected
when BMC reset, at this time we need to push the display data (crtc,
connector, etc.) manually as shown in Patch-4.

Probably it's not the most suitable way to implement it.

>
> Best regards
> Thomas
>
> > +     },
> > +     .probe = ls2kbmc_probe,
> > +     .remove = ls2kbmc_remove,
> > +};
> > +
> > +module_platform_driver(ls2kbmc_platform_driver);
> > +
> > +MODULE_DESCRIPTION("DRM driver for Loongson-2K BMC");
> > +MODULE_LICENSE("GPL");
>
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
>


-- 
Thanks.
Binbin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display
  2025-01-02 12:55     ` Binbin Zhou
@ 2025-01-02 13:32       ` Thomas Zimmermann
  2025-01-06  1:56         ` Binbin Zhou
  2025-01-06  7:03         ` Binbin Zhou
  0 siblings, 2 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2025-01-02 13:32 UTC (permalink / raw)
  To: Binbin Zhou
  Cc: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
	Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi


Am 02.01.25 um 13:55 schrieb Binbin Zhou:
> Hi Thomas:
>
> Thanks for your reply.
>
> On Thu, Jan 2, 2025 at 5:07 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Hi
>>
>>
>> Am 30.12.24 um 10:31 schrieb Binbin Zhou:
>> [...]
>>> +
>>> +static struct platform_driver ls2kbmc_platform_driver = {
>>> +     .driver = {
>>> +             .name = "ls2kbmc-framebuffer",
>> The driver is mostly a copy of simpledrm. Why don't you use
>> "simple-framebuffer" for your device name? You could use simpledrm
>> directly then.
> Ah, indeed, the driver is based on simpledrm.
>
> Initially, I also tried to use simpledrm directly, but it will fail in
> drm memory acquire.

Could you point to the exact call that fails within simpledrm?

> Because although we register the driver in platform form, its memory
> belongs to pci space and we can see the corresponding pci probe and
> resource allocation in Patch-1.

I don't understand. Graphics memory is often located on the PCI bus. 
What is so special about this one?

> Therefore, we need to use aperture_remove_conflicting_pci_devices().

So there is already a device that represents the graphics card? That's 
what you'd remove here? If you only add that MFD device, who owns the 
framebuffer? If it's the PCI device from patch 1 ("ls2k-bmc"), why does 
aperture_remove_conflicting_pci_devices() not remove that device? I'm 
somewhat confused, because the logic in your driver mostly looks like it 
binds to a pre-configured framebuffer, but some of the code doesn't. 
Best regards Thomas

>
> Also, since we are using BMC display, the display will be disconnected
> when BMC reset, at this time we need to push the display data (crtc,
> connector, etc.) manually as shown in Patch-4.
>
> Probably it's not the most suitable way to implement it.
>
>> Best regards
>> Thomas
>>
>>> +     },
>>> +     .probe = ls2kbmc_probe,
>>> +     .remove = ls2kbmc_remove,
>>> +};
>>> +
>>> +module_platform_driver(ls2kbmc_platform_driver);
>>> +
>>> +MODULE_DESCRIPTION("DRM driver for Loongson-2K BMC");
>>> +MODULE_LICENSE("GPL");
>> --
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>>
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 2/4] ipmi: Add Loongson-2K BMC support
  2024-12-30  9:31 ` [PATCH v1 2/4] ipmi: Add Loongson-2K BMC support Binbin Zhou
  2024-12-31 12:37   ` kernel test robot
@ 2025-01-03  4:29   ` kernel test robot
  1 sibling, 0 replies; 19+ messages in thread
From: kernel test robot @ 2025-01-03  4:29 UTC (permalink / raw)
  To: Binbin Zhou, Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter
  Cc: oe-kbuild-all, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi Binbin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.13-rc5]
[cannot apply to cminyard-ipmi/for-next lee-mfd/for-mfd-next lee-mfd/for-mfd-fixes next-20241220]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Binbin-Zhou/mfd-ls2kbmc-Introduce-Loongson-2K-BMC-MFD-Core-driver/20241230-174205
base:   linus/master
patch link:    https://lore.kernel.org/r/a4cfdeed1a91a7a12c7ebe56bed2ba94991c4065.1735550269.git.zhoubinbin%40loongson.cn
patch subject: [PATCH v1 2/4] ipmi: Add Loongson-2K BMC support
config: loongarch-randconfig-r123-20241231 (https://download.01.org/0day-ci/archive/20250103/202501031235.JEXCZscQ-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250103/202501031235.JEXCZscQ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501031235.JEXCZscQ-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/char/ipmi/ipmi_si_ls2k.c:110:38: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct ls2k_kcs_data *ik @@     got void [noderef] __iomem *const addr @@
   drivers/char/ipmi/ipmi_si_ls2k.c:110:38: sparse:     expected struct ls2k_kcs_data *ik
   drivers/char/ipmi/ipmi_si_ls2k.c:110:38: sparse:     got void [noderef] __iomem *const addr
   drivers/char/ipmi/ipmi_si_ls2k.c:168:38: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct ls2k_kcs_data *ik @@     got void [noderef] __iomem *const addr @@
   drivers/char/ipmi/ipmi_si_ls2k.c:168:38: sparse:     expected struct ls2k_kcs_data *ik
   drivers/char/ipmi/ipmi_si_ls2k.c:168:38: sparse:     got void [noderef] __iomem *const addr
>> drivers/char/ipmi/ipmi_si_ls2k.c:226:24: sparse: sparse: symbol 'ipmi_ls2k_platform_driver' was not declared. Should it be static?

vim +110 drivers/char/ipmi/ipmi_si_ls2k.c

   106	
   107	static unsigned char ls2k_mem_inb(const struct si_sm_io *io,
   108					  unsigned int offset)
   109	{
 > 110		struct ls2k_kcs_data *ik = io->addr;
   111		int inb = 0;
   112	
   113		if (ik->version == 0)
   114			inb = intf_sim_inb_v0(ik, offset);
   115		else if (ik->version == 1)
   116			inb = intf_sim_inb_v1(ik, offset);
   117	
   118		return inb;
   119	}
   120	
   121	static void intf_sim_outb_v0(struct ls2k_kcs_data *ik, unsigned int offset,
   122				     unsigned char val)
   123	{
   124		if (ls2k_get_ibf(ik))
   125			return;
   126	
   127		switch (offset & BIT(0)) {
   128		case 0:
   129			ik->reg.data_in = val;
   130			ik->reg.status &= ~LS2K_KCS_STS_CMD;
   131			break;
   132	
   133		case 1:
   134			ik->reg.cmd = val;
   135			ik->reg.status |= LS2K_KCS_STS_CMD;
   136			break;
   137		}
   138	
   139		ls2k_set_ibf(ik, 1);
   140		ik->write_req++;
   141	}
   142	
   143	static void intf_sim_outb_v1(struct ls2k_kcs_data *ik, unsigned int offset,
   144				     unsigned char val)
   145	{
   146		if (ik->fifo.ibfh != ik->fifo.ibft)
   147			return;
   148	
   149		switch (offset & BIT(0)) {
   150		case 0:
   151			ik->reg.data_in = val;
   152			ik->cmd_data = 0;
   153			break;
   154	
   155		case 1:
   156			ik->reg.cmd = val;
   157			ik->cmd_data = 1;
   158			break;
   159		}
   160	
   161		ik->fifo.ibfh = !ik->fifo.ibft;
   162		ik->write_req++;
   163	}
   164	
   165	static void ls2k_mem_outb(const struct si_sm_io *io, unsigned int offset,
   166				  unsigned char val)
   167	{
   168		struct ls2k_kcs_data *ik = io->addr;
   169	
   170		if (ik->version == 0)
   171			intf_sim_outb_v0(ik, offset, val);
   172		else if (ik->version == 1)
   173			intf_sim_outb_v1(ik, offset, val);
   174	}
   175	
   176	static void ls2k_mem_cleanup(struct si_sm_io *io)
   177	{
   178		if (io->addr)
   179			iounmap(io->addr);
   180	}
   181	
   182	static int ipmi_ls2k_sim_setup(struct si_sm_io *io)
   183	{
   184		io->addr = ioremap(io->addr_data, io->regspacing);
   185		if (!io->addr)
   186			return -EIO;
   187	
   188		io->inputb = ls2k_mem_inb;
   189		io->outputb = ls2k_mem_outb;
   190		io->io_cleanup = ls2k_mem_cleanup;
   191	
   192		return 0;
   193	}
   194	
   195	static int ipmi_ls2k_probe(struct platform_device *pdev)
   196	{
   197		struct si_sm_io io;
   198	
   199		dev_info(&pdev->dev, "probing via ls2k platform");
   200		memset(&io, 0, sizeof(io));
   201	
   202		io.addr_source	= SI_PLATFORM;
   203		io.si_type	= SI_KCS;
   204		io.addr_space	= IPMI_MEM_ADDR_SPACE;
   205		io.io_setup	= ipmi_ls2k_sim_setup;
   206		io.addr_data	= pdev->resource[0].start;
   207		io.regspacing	= pdev->resource[0].end - pdev->resource[0].start + 1;
   208		io.regsize	= DEFAULT_REGSIZE;
   209		io.regshift	= 0;
   210		io.dev		= &pdev->dev;
   211		io.irq		= 0;
   212		if (io.irq)
   213			io.irq_setup = ipmi_std_irq_setup;
   214	
   215		dev_info(&pdev->dev, "%pR regsize %d spacing %d irq %d\n",
   216			 &pdev->resource[0], io.regsize, io.regspacing, io.irq);
   217	
   218		return ipmi_si_add_smi(&io);
   219	}
   220	
   221	static void ipmi_ls2k_remove(struct platform_device *pdev)
   222	{
   223		ipmi_si_remove_by_dev(&pdev->dev);
   224	}
   225	
 > 226	struct platform_driver ipmi_ls2k_platform_driver = {
   227		.driver = {
   228			.name = "ls2k-ipmi-si",
   229		},
   230		.probe	= ipmi_ls2k_probe,
   231		.remove	= ipmi_ls2k_remove,
   232	};
   233	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display
  2025-01-02 13:32       ` Thomas Zimmermann
@ 2025-01-06  1:56         ` Binbin Zhou
  2025-01-06  7:03         ` Binbin Zhou
  1 sibling, 0 replies; 19+ messages in thread
From: Binbin Zhou @ 2025-01-06  1:56 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
	Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi Thomas:


On Thu, Jan 2, 2025 at 9:32 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
>
> Am 02.01.25 um 13:55 schrieb Binbin Zhou:
> > Hi Thomas:
> >
> > Thanks for your reply.
> >
> > On Thu, Jan 2, 2025 at 5:07 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >> Hi
> >>
> >>
> >> Am 30.12.24 um 10:31 schrieb Binbin Zhou:
> >> [...]
> >>> +
> >>> +static struct platform_driver ls2kbmc_platform_driver = {
> >>> +     .driver = {
> >>> +             .name = "ls2kbmc-framebuffer",
> >> The driver is mostly a copy of simpledrm. Why don't you use
> >> "simple-framebuffer" for your device name? You could use simpledrm
> >> directly then.
> > Ah, indeed, the driver is based on simpledrm.
> >
> > Initially, I also tried to use simpledrm directly, but it will fail in
> > drm memory acquire.
>
> Could you point to the exact call that fails within simpledrm?

[    8.289823] simple-framebuffer simple-framebuffer.0: [drm] *ERROR*
could not acquire memory range [mem 0xe0031200000-0xe00315fffff flags
0x200]: -16
[    8.312681] simple-framebuffer simple-framebuffer.0: probe with
driver simple-framebuffer failed with error -16
>
> > Because although we register the driver in platform form, its memory
> > belongs to pci space and we can see the corresponding pci probe and
> > resource allocation in Patch-1.
>
> I don't understand. Graphics memory is often located on the PCI bus.
> What is so special about this one?
>
> > Therefore, we need to use aperture_remove_conflicting_pci_devices().
>
> So there is already a device that represents the graphics card? That's
> what you'd remove here? If you only add that MFD device, who owns the
> framebuffer? If it's the PCI device from patch 1 ("ls2k-bmc"), why does
> aperture_remove_conflicting_pci_devices() not remove that device? I'm
> somewhat confused, because the logic in your driver mostly looks like it
> binds to a pre-configured framebuffer, but some of the code doesn't.
> Best regards Thomas
>
> >
> > Also, since we are using BMC display, the display will be disconnected
> > when BMC reset, at this time we need to push the display data (crtc,
> > connector, etc.) manually as shown in Patch-4.
> >
> > Probably it's not the most suitable way to implement it.
> >
> >> Best regards
> >> Thomas
> >>
> >>> +     },
> >>> +     .probe = ls2kbmc_probe,
> >>> +     .remove = ls2kbmc_remove,
> >>> +};
> >>> +
> >>> +module_platform_driver(ls2kbmc_platform_driver);
> >>> +
> >>> +MODULE_DESCRIPTION("DRM driver for Loongson-2K BMC");
> >>> +MODULE_LICENSE("GPL");
> >> --
> >> --
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Software Solutions Germany GmbH
> >> Frankenstrasse 146, 90461 Nuernberg, Germany
> >> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> >> HRB 36809 (AG Nuernberg)
> >>
> >
>
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
>


-- 
Thanks.
Binbin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display
  2025-01-02 13:32       ` Thomas Zimmermann
  2025-01-06  1:56         ` Binbin Zhou
@ 2025-01-06  7:03         ` Binbin Zhou
  2025-01-06 14:10           ` Thomas Zimmermann
  1 sibling, 1 reply; 19+ messages in thread
From: Binbin Zhou @ 2025-01-06  7:03 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
	Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi Thomas:

The last reply email was incomplete, sorry for the incomplete reply
due to my mistake.

On Thu, Jan 2, 2025 at 9:32 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
>
> Am 02.01.25 um 13:55 schrieb Binbin Zhou:
> > Hi Thomas:
> >
> > Thanks for your reply.
> >
> > On Thu, Jan 2, 2025 at 5:07 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >> Hi
> >>
> >>
> >> Am 30.12.24 um 10:31 schrieb Binbin Zhou:
> >> [...]
> >>> +
> >>> +static struct platform_driver ls2kbmc_platform_driver = {
> >>> +     .driver = {
> >>> +             .name = "ls2kbmc-framebuffer",
> >> The driver is mostly a copy of simpledrm. Why don't you use
> >> "simple-framebuffer" for your device name? You could use simpledrm
> >> directly then.
> > Ah, indeed, the driver is based on simpledrm.
> >
> > Initially, I also tried to use simpledrm directly, but it will fail in
> > drm memory acquire.
>
> Could you point to the exact call that fails within simpledrm?

If we use simpledrm directly, the following error occurs:

[    8.289823] simple-framebuffer simple-framebuffer.0: [drm] *ERROR*
could not acquire memory range [mem 0xe0031200000-0xe00315fffff flags
0x200]: -16
[    8.312681] simple-framebuffer simple-framebuffer.0: probe with
driver simple-framebuffer failed with error -16

The reason for the failure: overlapping resources.

https://elixir.bootlin.com/linux/v6.12.6/source/drivers/video/aperture.c#L175
>
> > Because although we register the driver in platform form, its memory
> > belongs to pci space and we can see the corresponding pci probe and
> > resource allocation in Patch-1.
>
> I don't understand. Graphics memory is often located on the PCI bus.
> What is so special about this one?
>
> > Therefore, we need to use aperture_remove_conflicting_pci_devices().
>
> So there is already a device that represents the graphics card? That's
> what you'd remove here? If you only add that MFD device, who owns the
> framebuffer? If it's the PCI device from patch 1 ("ls2k-bmc"), why does
> aperture_remove_conflicting_pci_devices() not remove that device? I'm
> somewhat confused, because the logic in your driver mostly looks like it
> binds to a pre-configured framebuffer, but some of the code doesn't.

Perhaps the use of aperture_remove_conflicting_pci_devices() is wrong,
as there is only one display device for the LS2K BMC and there will be
no phase conflict.

When I tried to use that API before, it was partly due to the error
above, and partly because I referenced that other display drivers via
pci_driver.probe() would have it, just in case I used it, which was
probably the wrong choice.

The resources for pci bar0 are as follows:
BAR0: e0030000000/SZ_32M

0x0              0x600000  0xf00001c    16M            32M
+----+--------------+--------+-----------+---+-----------------+
| 2M | simpldrm |           | IPMI      |     | video env     |
+-----------------------------------------------------------------+

The mfd driver registers the ls2kbmc-framebuffer and ls2k-ipmi-si
devices according to the resource allocation shown above. At the same
time, the ls2kbmc drm is bound to the pre-configured “simpldrm”
resource in the above figure, which is passed through the
ls2kbmc-framebuffer driver. In addition, the resolution is read from
“video env” for the time being, and the resolution adaption is planned
to be added later.

> Best regards Thomas
>
> >
> > Also, since we are using BMC display, the display will be disconnected
> > when BMC reset, at this time we need to push the display data (crtc,
> > connector, etc.) manually as shown in Patch-4.
> >
> > Probably it's not the most suitable way to implement it.
> >
> >> Best regards
> >> Thomas
> >>
> >>> +     },
> >>> +     .probe = ls2kbmc_probe,
> >>> +     .remove = ls2kbmc_remove,
> >>> +};
> >>> +
> >>> +module_platform_driver(ls2kbmc_platform_driver);
> >>> +
> >>> +MODULE_DESCRIPTION("DRM driver for Loongson-2K BMC");
> >>> +MODULE_LICENSE("GPL");
> >> --
> >> --
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Software Solutions Germany GmbH
> >> Frankenstrasse 146, 90461 Nuernberg, Germany
> >> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> >> HRB 36809 (AG Nuernberg)
> >>
> >
>
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
>


--
Thanks.
Binbin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display
  2025-01-06  7:03         ` Binbin Zhou
@ 2025-01-06 14:10           ` Thomas Zimmermann
  2025-01-09 12:56             ` Binbin Zhou
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Zimmermann @ 2025-01-06 14:10 UTC (permalink / raw)
  To: Binbin Zhou
  Cc: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
	Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi,

Thanks for the info.


Am 06.01.25 um 08:03 schrieb Binbin Zhou:
[...]
>> Could you point to the exact call that fails within simpledrm?
> If we use simpledrm directly, the following error occurs:
>
> [    8.289823] simple-framebuffer simple-framebuffer.0: [drm] *ERROR*
> could not acquire memory range [mem 0xe0031200000-0xe00315fffff flags
> 0x200]: -16
> [    8.312681] simple-framebuffer simple-framebuffer.0: probe with
> driver simple-framebuffer failed with error -16
>
> The reason for the failure: overlapping resources.
>
> https://elixir.bootlin.com/linux/v6.12.6/source/drivers/video/aperture.c#L175

This error means that there's already an instance of simpledrm bound to 
the BMC framebuffer. So you already have a working display and some 
graphics under Linux without the new driver, right?

If so, why do you need a new driver that does exactly the same as simpledrm?

Best regards
Thomas

>>> Because although we register the driver in platform form, its memory
>>> belongs to pci space and we can see the corresponding pci probe and
>>> resource allocation in Patch-1.
>> I don't understand. Graphics memory is often located on the PCI bus.
>> What is so special about this one?
>>
>>> Therefore, we need to use aperture_remove_conflicting_pci_devices().
>> So there is already a device that represents the graphics card? That's
>> what you'd remove here? If you only add that MFD device, who owns the
>> framebuffer? If it's the PCI device from patch 1 ("ls2k-bmc"), why does
>> aperture_remove_conflicting_pci_devices() not remove that device? I'm
>> somewhat confused, because the logic in your driver mostly looks like it
>> binds to a pre-configured framebuffer, but some of the code doesn't.
> Perhaps the use of aperture_remove_conflicting_pci_devices() is wrong,
> as there is only one display device for the LS2K BMC and there will be
> no phase conflict.
>
> When I tried to use that API before, it was partly due to the error
> above, and partly because I referenced that other display drivers via
> pci_driver.probe() would have it, just in case I used it, which was
> probably the wrong choice.
>
> The resources for pci bar0 are as follows:
> BAR0: e0030000000/SZ_32M
>
> 0x0              0x600000  0xf00001c    16M            32M
> +----+--------------+--------+-----------+---+-----------------+
> | 2M | simpldrm |           | IPMI      |     | video env     |
> +-----------------------------------------------------------------+
>
> The mfd driver registers the ls2kbmc-framebuffer and ls2k-ipmi-si
> devices according to the resource allocation shown above. At the same
> time, the ls2kbmc drm is bound to the pre-configured “simpldrm”
> resource in the above figure, which is passed through the
> ls2kbmc-framebuffer driver. In addition, the resolution is read from
> “video env” for the time being, and the resolution adaption is planned
> to be added later.
>
>> Best regards Thomas
>>
>>> Also, since we are using BMC display, the display will be disconnected
>>> when BMC reset, at this time we need to push the display data (crtc,
>>> connector, etc.) manually as shown in Patch-4.
>>>
>>> Probably it's not the most suitable way to implement it.
>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>>> +     },
>>>>> +     .probe = ls2kbmc_probe,
>>>>> +     .remove = ls2kbmc_remove,
>>>>> +};
>>>>> +
>>>>> +module_platform_driver(ls2kbmc_platform_driver);
>>>>> +
>>>>> +MODULE_DESCRIPTION("DRM driver for Loongson-2K BMC");
>>>>> +MODULE_LICENSE("GPL");
>>>> --
>>>> --
>>>> Thomas Zimmermann
>>>> Graphics Driver Developer
>>>> SUSE Software Solutions Germany GmbH
>>>> Frankenstrasse 146, 90461 Nuernberg, Germany
>>>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>>>> HRB 36809 (AG Nuernberg)
>>>>
>> --
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>>
>
> --
> Thanks.
> Binbin

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display
  2025-01-06 14:10           ` Thomas Zimmermann
@ 2025-01-09 12:56             ` Binbin Zhou
  0 siblings, 0 replies; 19+ messages in thread
From: Binbin Zhou @ 2025-01-09 12:56 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
	Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi Thomas:

Sorry for the late reply.

On Mon, Jan 6, 2025 at 10:10 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi,
>
> Thanks for the info.
>
>
> Am 06.01.25 um 08:03 schrieb Binbin Zhou:
> [...]
> >> Could you point to the exact call that fails within simpledrm?
> > If we use simpledrm directly, the following error occurs:
> >
> > [    8.289823] simple-framebuffer simple-framebuffer.0: [drm] *ERROR*
> > could not acquire memory range [mem 0xe0031200000-0xe00315fffff flags
> > 0x200]: -16
> > [    8.312681] simple-framebuffer simple-framebuffer.0: probe with
> > driver simple-framebuffer failed with error -16
> >
> > The reason for the failure: overlapping resources.
> >
> > https://elixir.bootlin.com/linux/v6.12.6/source/drivers/video/aperture.c#L175
>
> This error means that there's already an instance of simpledrm bound to
> the BMC framebuffer. So you already have a working display and some
> graphics under Linux without the new driver, right?

Yes, I checked again and found that the **efifb** binds to the BMC framebuffer.

>
> If so, why do you need a new driver that does exactly the same as simpledrm?

Regarding the new driver, we implemented the BMC display based on
simpledrm. But first we need to fix the initialization failure above,
and more importantly, we need to implement the BMC reset function [1].

Specifically, when the BMC reset, it will cause the pcie controller to
disconnect and the display will be disconnected with it. After that,
we need to restore the pcie bar data, as well as re-push the display
information (ls2kbmc_push_drm_mode()).

Based on this, I would like to rewrite a new display driver.

[1]: patch(4/4)
https://lore.kernel.org/all/b0ac8b81fbb8955ed8ada27aba423cff45aad9f6.1735550269.git.zhoubinbin@loongson.cn/
>
> Best regards
> Thomas
>
> >>> Because although we register the driver in platform form, its memory
> >>> belongs to pci space and we can see the corresponding pci probe and
> >>> resource allocation in Patch-1.
> >> I don't understand. Graphics memory is often located on the PCI bus.
> >> What is so special about this one?
> >>
> >>> Therefore, we need to use aperture_remove_conflicting_pci_devices().
> >> So there is already a device that represents the graphics card? That's
> >> what you'd remove here? If you only add that MFD device, who owns the
> >> framebuffer? If it's the PCI device from patch 1 ("ls2k-bmc"), why does
> >> aperture_remove_conflicting_pci_devices() not remove that device? I'm
> >> somewhat confused, because the logic in your driver mostly looks like it
> >> binds to a pre-configured framebuffer, but some of the code doesn't.
> > Perhaps the use of aperture_remove_conflicting_pci_devices() is wrong,
> > as there is only one display device for the LS2K BMC and there will be
> > no phase conflict.
> >
> > When I tried to use that API before, it was partly due to the error
> > above, and partly because I referenced that other display drivers via
> > pci_driver.probe() would have it, just in case I used it, which was
> > probably the wrong choice.
> >
> > The resources for pci bar0 are as follows:
> > BAR0: e0030000000/SZ_32M
> >
> > 0x0              0x600000  0xf00001c    16M            32M
> > +----+--------------+--------+-----------+---+-----------------+
> > | 2M | simpldrm |           | IPMI      |     | video env     |
> > +-----------------------------------------------------------------+
> >
> > The mfd driver registers the ls2kbmc-framebuffer and ls2k-ipmi-si
> > devices according to the resource allocation shown above. At the same
> > time, the ls2kbmc drm is bound to the pre-configured “simpldrm”
> > resource in the above figure, which is passed through the
> > ls2kbmc-framebuffer driver. In addition, the resolution is read from
> > “video env” for the time being, and the resolution adaption is planned
> > to be added later.
> >
> >> Best regards Thomas
> >>
> >>> Also, since we are using BMC display, the display will be disconnected
> >>> when BMC reset, at this time we need to push the display data (crtc,
> >>> connector, etc.) manually as shown in Patch-4.
> >>>
> >>> Probably it's not the most suitable way to implement it.
> >>>
> >>>> Best regards
> >>>> Thomas
> >>>>
> >>>>> +     },
> >>>>> +     .probe = ls2kbmc_probe,
> >>>>> +     .remove = ls2kbmc_remove,
> >>>>> +};
> >>>>> +
> >>>>> +module_platform_driver(ls2kbmc_platform_driver);
> >>>>> +
> >>>>> +MODULE_DESCRIPTION("DRM driver for Loongson-2K BMC");
> >>>>> +MODULE_LICENSE("GPL");
> >>>> --
> >>>> --
> >>>> Thomas Zimmermann
> >>>> Graphics Driver Developer
> >>>> SUSE Software Solutions Germany GmbH
> >>>> Frankenstrasse 146, 90461 Nuernberg, Germany
> >>>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> >>>> HRB 36809 (AG Nuernberg)
> >>>>
> >> --
> >> --
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Software Solutions Germany GmbH
> >> Frankenstrasse 146, 90461 Nuernberg, Germany
> >> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> >> HRB 36809 (AG Nuernberg)
> >>
> >
> > --
> > Thanks.
> > Binbin
>
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
>


-- 
Thanks.
Binbin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display
  2024-12-30  9:31 ` [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display Binbin Zhou
  2025-01-02  9:07   ` Thomas Zimmermann
@ 2025-01-15  8:48   ` Thomas Zimmermann
  1 sibling, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2025-01-15  8:48 UTC (permalink / raw)
  To: Binbin Zhou, Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter
  Cc: Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi,

after the discussion, let me give you a preliminary review.


Am 30.12.24 um 10:31 schrieb Binbin Zhou:
> Adds a driver for the Loongson-2K BMC display as a sub-function of
> the BMC device.
>
> Display-related scan output buffers, sizes, and display formats are
> provided through the Loongson-2K BMC MFD driver.
>
> Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>   drivers/gpu/drm/tiny/Kconfig   |  18 +
>   drivers/gpu/drm/tiny/Makefile  |   1 +
>   drivers/gpu/drm/tiny/ls2kbmc.c | 636 +++++++++++++++++++++++++++++++++
>   3 files changed, 655 insertions(+)
>   create mode 100644 drivers/gpu/drm/tiny/ls2kbmc.c
>
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index 94cbdb1337c0..5412f639a964 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -171,6 +171,24 @@ config TINYDRM_ILI9486
>   
>   	  If M is selected the module will be called ili9486.
>   
> +config TINYDRM_LS2KBMC

DRM_LS2KBMC. The TINY prefix is no longer in use.

> +	tristate "DRM support for Loongson-2K BMC display"
> +	depends on DRM && MMU
> +	depends on MFD_LS2K_BMC
> +	select APERTURE_HELPERS
> +	select DRM_CLIENT_SELECTION
> +	select DRM_GEM_SHMEM_HELPER
> +	select DRM_KMS_HELPER
> +	help
> +	  DRM driver for the Loongson-2K BMC display.
> +
> +	  This driver assumes that the display hardware has been initialized
> +	  by the Loongson-2K BMC. Since the Loongson-2K BMC does not support
> +	  resolution detection now, the scan buffer, size and display format
> +	  are fixed and provided by the BMC.
> +
> +	  If M is selected the module will be called ls2kbmc.
> +
>   config TINYDRM_MI0283QT
>   	tristate "DRM support for MI0283QT"
>   	depends on DRM && SPI
> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> index 4aaf56f8707d..fa4e1646db77 100644
> --- a/drivers/gpu/drm/tiny/Makefile
> +++ b/drivers/gpu/drm/tiny/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_TINYDRM_ILI9163)		+= ili9163.o
>   obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
>   obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
>   obj-$(CONFIG_TINYDRM_ILI9486)		+= ili9486.o
> +obj-$(CONFIG_TINYDRM_LS2KBMC)		+= ls2kbmc.o
>   obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
>   obj-$(CONFIG_TINYDRM_REPAPER)		+= repaper.o
>   obj-$(CONFIG_TINYDRM_SHARP_MEMORY)	+= sharp-memory.o
> diff --git a/drivers/gpu/drm/tiny/ls2kbmc.c b/drivers/gpu/drm/tiny/ls2kbmc.c
> new file mode 100644
> index 000000000000..909d6c687193
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/ls2kbmc.c
> @@ -0,0 +1,636 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * DRM driver for Loongson-2K BMC display
> + *
> + * Copyright (C) 2024 Loongson Technology Corporation Limited.
> + *
> + * Based on simpledrm
> + */
> +
> +#include <linux/aperture.h>
> +#include <linux/minmax.h>
> +#include <linux/pci.h>
> +#include <linux/platform_data/simplefb.h>

Please don't use these data structures. Create a new one for your driver 
instead. Let''s call it 'struct ls2kbmc_framebuffer' for now.

> +#include <linux/platform_device.h>
> +
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_state_helper.h>
> +#include <drm/drm_client_setup.h>
> +#include <drm/drm_connector.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_damage_helper.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_fbdev_shmem.h>
> +#include <drm/drm_format_helper.h>
> +#include <drm/drm_framebuffer.h>
> +#include <drm/drm_gem_atomic_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_gem_shmem_helper.h>
> +#include <drm/drm_managed.h>
> +#include <drm/drm_modeset_helper_vtables.h>
> +#include <drm/drm_panic.h>
> +#include <drm/drm_probe_helper.h>
> +
> +struct ls2kbmc_pdata {
> +	struct pci_dev *pdev;
> +	struct simplefb_platform_data pd;
> +};
> +
> +/*
> + * Helpers for simplefb_platform_data
> + */
> +
> +static int
> +simplefb_get_validated_int(struct drm_device *dev, const char *name,
> +			   u32 value)
> +{
> +	if (value > INT_MAX) {
> +		drm_err(dev, "simplefb: invalid framebuffer %s of %u\n",
> +			name, value);
> +		return -EINVAL;
> +	}
> +	return (int)value;
> +}
> +
> +static int
> +simplefb_get_validated_int0(struct drm_device *dev, const char *name,
> +			    u32 value)
> +{
> +	if (!value) {
> +		drm_err(dev, "simplefb: invalid framebuffer %s of %u\n",
> +			name, value);
> +		return -EINVAL;
> +	}
> +	return simplefb_get_validated_int(dev, name, value);
> +}
> +
> +static const struct drm_format_info *
> +simplefb_get_validated_format(struct drm_device *dev, const char *format_name)
> +{
> +	static const struct simplefb_format formats[] = SIMPLEFB_FORMATS;
> +	const struct simplefb_format *fmt = formats;
> +	const struct simplefb_format *end = fmt + ARRAY_SIZE(formats);
> +	const struct drm_format_info *info;
> +
> +	if (!format_name) {
> +		drm_err(dev, "simplefb: missing framebuffer format\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	while (fmt < end) {
> +		if (!strcmp(format_name, fmt->name)) {
> +			info = drm_format_info(fmt->fourcc);
> +			if (!info)
> +				return ERR_PTR(-EINVAL);
> +			return info;
> +		}
> +		++fmt;
> +	}
> +
> +	drm_err(dev, "simplefb: unknown framebuffer format %s\n",
> +		format_name);
> +
> +	return ERR_PTR(-EINVAL);
> +}
> +
> +static int
> +simplefb_get_width_pd(struct drm_device *dev,
> +		      const struct simplefb_platform_data *pd)
> +{
> +	return simplefb_get_validated_int0(dev, "width", pd->width);
> +}
> +
> +static int
> +simplefb_get_height_pd(struct drm_device *dev,
> +		       const struct simplefb_platform_data *pd)
> +{
> +	return simplefb_get_validated_int0(dev, "height", pd->height);
> +}
> +
> +static int
> +simplefb_get_stride_pd(struct drm_device *dev,
> +		       const struct simplefb_platform_data *pd)
> +{
> +	return simplefb_get_validated_int(dev, "stride", pd->stride);
> +}
> +
> +static const struct drm_format_info *
> +simplefb_get_format_pd(struct drm_device *dev,
> +		       const struct simplefb_platform_data *pd)
> +{
> +	return simplefb_get_validated_format(dev, pd->format);
> +}

The simplefb_ prefixes have to be replaced and the helpers have to 
operate on the new custom data structures.

> +
> +/*
> + * ls2kbmc Framebuffer device
> + */
> +
> +struct ls2kbmc_device {
> +	struct drm_device dev;
> +

> +	/* simplefb settings */
> +	struct drm_display_mode mode;
> +	const struct drm_format_info *format;
> +	unsigned int pitch;

You should be able to store a pointer to the instance of struct 
ls2kbmc_framebuffer here.

> +
> +	/* memory management */
> +	struct iosys_map screen_base;
> +
> +	/* modesetting */
> +	u32 formats[8];
> +	struct drm_plane primary_plane;
> +	struct drm_crtc crtc;
> +	struct drm_encoder encoder;
> +	struct drm_connector connector;
> +};
> +
> +static struct ls2kbmc_device *ls2kbmc_device_of_dev(struct drm_device *dev)
> +{
> +	return container_of(dev, struct ls2kbmc_device, dev);
> +}
> +
> +/*
> + * Modesetting
> + */
> +
> +static const u64 ls2kbmc_primary_plane_format_modifiers[] = {
> +	DRM_FORMAT_MOD_LINEAR,
> +	DRM_FORMAT_MOD_INVALID
> +};
> +
> +static int ls2kbmc_primary_plane_helper_atomic_check(struct drm_plane *plane,
> +						     struct drm_atomic_state *state)
> +{
> +	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
> +	struct drm_shadow_plane_state *new_shadow_plane_state =
> +		to_drm_shadow_plane_state(new_plane_state);
> +	struct drm_framebuffer *new_fb = new_plane_state->fb;
> +	struct drm_crtc *new_crtc = new_plane_state->crtc;
> +	struct drm_crtc_state *new_crtc_state = NULL;
> +	struct drm_device *dev = plane->dev;
> +	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(dev);
> +	int ret;
> +
> +	if (new_crtc)
> +		new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
> +
> +	ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state,
> +						  DRM_PLANE_NO_SCALING,
> +						  DRM_PLANE_NO_SCALING,
> +						  false, false);
> +	if (ret)
> +		return ret;
> +	else if (!new_plane_state->visible)
> +		return 0;
> +
> +	if (new_fb->format != sdev->format) {
> +		void *buf;
> +
> +		/* format conversion necessary; reserve buffer */
> +		buf = drm_format_conv_state_reserve(&new_shadow_plane_state->fmtcnv_state,
> +						    sdev->pitch, GFP_KERNEL);
> +		if (!buf)
> +			return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
> +static void ls2kbmc_primary_plane_helper_atomic_update(struct drm_plane *plane,
> +						       struct drm_atomic_state *state)
> +{
> +	struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
> +	struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
> +	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
> +	struct drm_framebuffer *fb = plane_state->fb;
> +	struct drm_device *dev = plane->dev;
> +	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(dev);
> +	struct drm_atomic_helper_damage_iter iter;
> +	struct drm_rect damage;
> +	int ret, idx;
> +
> +	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
> +	if (ret)
> +		return;
> +
> +	if (!drm_dev_enter(dev, &idx))
> +		goto out_drm_gem_fb_end_cpu_access;
> +
> +	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
> +	drm_atomic_for_each_plane_damage(&iter, &damage) {
> +		struct drm_rect dst_clip = plane_state->dst;
> +		struct iosys_map dst = sdev->screen_base;
> +
> +		if (!drm_rect_intersect(&dst_clip, &damage))
> +			continue;
> +
> +		iosys_map_incr(&dst, drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip));
> +		drm_fb_blit(&dst, &sdev->pitch, sdev->format->format, shadow_plane_state->data,
> +			    fb, &damage, &shadow_plane_state->fmtcnv_state);
> +	}
> +
> +	drm_dev_exit(idx);
> +out_drm_gem_fb_end_cpu_access:
> +	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
> +}
> +
> +static void ls2kbmc_primary_plane_helper_atomic_disable(struct drm_plane *plane,
> +							struct drm_atomic_state *state)
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(dev);
> +	int idx;
> +
> +	if (!drm_dev_enter(dev, &idx))
> +		return;
> +
> +	/* Clear screen to black if disabled */
> +	iosys_map_memset(&sdev->screen_base, 0, 0, sdev->pitch * sdev->mode.vdisplay);
> +
> +	drm_dev_exit(idx);
> +}
> +
> +static int ls2kbmc_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane,
> +							   struct drm_scanout_buffer *sb)
> +{
> +	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(plane->dev);
> +
> +	sb->width = sdev->mode.hdisplay;
> +	sb->height = sdev->mode.vdisplay;
> +	sb->format = sdev->format;
> +	sb->pitch[0] = sdev->pitch;
> +	sb->map[0] = sdev->screen_base;
> +
> +	return 0;
> +}
> +
> +static const struct drm_plane_helper_funcs ls2kbmc_primary_plane_helper_funcs = {
> +	DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
> +	.atomic_check = ls2kbmc_primary_plane_helper_atomic_check,
> +	.atomic_update = ls2kbmc_primary_plane_helper_atomic_update,
> +	.atomic_disable = ls2kbmc_primary_plane_helper_atomic_disable,
> +	.get_scanout_buffer = ls2kbmc_primary_plane_helper_get_scanout_buffer,
> +};
> +
> +static const struct drm_plane_funcs ls2kbmc_primary_plane_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = drm_plane_cleanup,
> +	DRM_GEM_SHADOW_PLANE_FUNCS,
> +};
> +
> +static enum drm_mode_status ls2kbmc_crtc_helper_mode_valid(struct drm_crtc *crtc,
> +							   const struct drm_display_mode *mode)
> +{
> +	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(crtc->dev);
> +
> +	return drm_crtc_helper_mode_valid_fixed(crtc, mode, &sdev->mode);
> +}
> +
> +/*
> + * The CRTC is always enabled. Screen updates are performed by
> + * the primary plane's atomic_update function. Disabling clears
> + * the screen in the primary plane's atomic_disable function.
> + */
> +static const struct drm_crtc_helper_funcs ls2kbmc_crtc_helper_funcs = {
> +	.mode_valid = ls2kbmc_crtc_helper_mode_valid,
> +	.atomic_check = drm_crtc_helper_atomic_check,
> +};
> +
> +static const struct drm_crtc_funcs ls2kbmc_crtc_funcs = {
> +	.reset = drm_atomic_helper_crtc_reset,
> +	.destroy = drm_crtc_cleanup,
> +	.set_config = drm_atomic_helper_set_config,
> +	.page_flip = drm_atomic_helper_page_flip,
> +	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> +};
> +
> +static const struct drm_encoder_funcs ls2kbmc_encoder_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
> +static int ls2kbmc_connector_helper_get_modes(struct drm_connector *connector)
> +{
> +	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(connector->dev);
> +
> +	return drm_connector_helper_get_modes_fixed(connector, &sdev->mode);
> +}
> +
> +static const struct drm_connector_helper_funcs ls2kbmc_connector_helper_funcs = {
> +	.get_modes = ls2kbmc_connector_helper_get_modes,
> +};
> +
> +static const struct drm_connector_funcs ls2kbmc_connector_funcs = {
> +	.reset = drm_atomic_helper_connector_reset,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = drm_connector_cleanup,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static const struct drm_mode_config_funcs ls2kbmc_mode_config_funcs = {
> +	.fb_create = drm_gem_fb_create_with_dirty,
> +	.atomic_check = drm_atomic_helper_check,
> +	.atomic_commit = drm_atomic_helper_commit,
> +};
> +
> +/*
> + * Init / Cleanup
> + */
> +
> +static struct drm_display_mode ls2kbmc_mode(unsigned int width, unsigned int height,
> +					    unsigned int width_mm, unsigned int height_mm)
> +{
> +	const struct drm_display_mode mode = {
> +		DRM_MODE_INIT(60, width, height, width_mm, height_mm)
> +	};
> +
> +	return mode;
> +}
> +
> +/*
> + * DRM driver
> + */
> +
> +DEFINE_DRM_GEM_FOPS(ls2kbmc_fops);
> +
> +static struct drm_driver ls2kbmc_driver = {
> +	DRM_GEM_SHMEM_DRIVER_OPS,
> +	DRM_FBDEV_SHMEM_DRIVER_OPS,
> +	.name			= "simpledrm",

You must not call it simpledrm. 'ls2kbmc' should be fine.

> +	.desc			= "DRM driver for Loongson-2K BMC",

> +	.date			= "20241211",

We recently removed the driver date from all drivers. So please drop it 
here as well.

> +	.major			= 1,
> +	.minor			= 0,
> +	.driver_features	= DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
> +	.fops			= &ls2kbmc_fops,
> +};
> +
> +/*
> + * Currently the Loongson-2K0500 BMC hardware does not have an i2c interface to
> + * adapt to the resolution.
> + * We set the resolution by presetting "video=1280x1024-16@2M" to the bmc memory.
> + */
> +static int ls2kbmc_get_video_mode(struct pci_dev *pdev, struct simplefb_platform_data *pd)

That entire function should be in the BMC core driver. The core driver 
should retrieve the framebuffer parameters, set up an instance of struct 
l2kbmc_framebuffer and forward it as device data.

> +{
> +	char *mode;
> +	int depth, ret;
> +
> +	/* The pci mem bar last 16M is used to store the string. */
> +	mode = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0) + SZ_16M, SZ_16M);
> +	if (!mode)
> +		return -ENOMEM;
> +
> +	/*env at last 16M's beginning, first env is video */
> +	if (!strncmp(mode, "video=", 6))

NEVER use video=. It is not for DRM drivers. (fbdev code gets this 
entirely wrong.)

Instead use module_param() to define a module parameter in the core BMC 
driver. Something like

   module_param("framebuffer", charp, 0400)

Your firmware can then do something like

ls2kbmc-mfd.framebuffer=<width>x<height>-<res>@<Hz> on the kernel 
command line. The core driver parses the string and creates the struct 
ls2kbmc_framebuffer from it. I have some doubts about the whole idea of 
using a framebuffer string in the first place. But at least it is self 
contained.
> +		mode = mode + 6;
> +
> +	ret = kstrtoint(strsep(&mode, "x"), 10, &pd->width);
> +	if (ret)
> +		return ret;
> +
> +	ret = kstrtoint(strsep(&mode, "-"), 10, &pd->height);
> +	if (ret)
> +		return ret;
> +
> +	ret = kstrtoint(strsep(&mode, "@"), 10, &depth);
> +	if (ret)
> +		return ret;
> +
> +	pd->stride = pd->width * depth / 8;
> +	pd->format = depth == 32 ? "a8r8g8b8" : "r5g6b5";
> +
> +	return 0;
> +}
> +
> +static struct ls2kbmc_device *ls2kbmc_device_create(struct drm_driver *drv,
> +						    struct platform_device *pdev,
> +						    struct ls2kbmc_pdata *priv)
> +{

> +	struct pci_dev *ppdev = priv->pdev;

No need to store the pci_dev explicitly. The PCI device is already the 
parent of the platform device pdev. You can do

   struct pci_dev *pparent;

   if (!dev_is_pci(pdev->dev.parent))
     return -ENODEV;

   pparent = to_pci_dev(pdev->dev.parent);

That will free up the device-data slot for storing the framebuffer info.

> +	struct simplefb_platform_data *pd = &priv->pd;
> +	struct ls2kbmc_device *sdev;
> +	struct drm_device *dev;
> +	int width, height, stride;
> +	int width_mm = 0, height_mm = 0;
> +	const struct drm_format_info *format;
> +	struct resource *res, *mem = NULL;
> +	struct drm_plane *primary_plane;
> +	struct drm_crtc *crtc;
> +	struct drm_encoder *encoder;
> +	struct drm_connector *connector;
> +	unsigned long max_width, max_height;
> +	void __iomem *screen_base;
> +	size_t nformats;
> +	int ret;
> +
> +	sdev = devm_drm_dev_alloc(&pdev->dev, drv, struct ls2kbmc_device, dev);
> +	if (IS_ERR(sdev))
> +		return ERR_CAST(sdev);
> +	dev = &sdev->dev;
> +	platform_set_drvdata(pdev, sdev);
> +

> +	ret = ls2kbmc_get_video_mode(ppdev, pd);
> +	if (ret) {
> +		drm_err(dev, "no simplefb configuration found\n");
> +		return ERR_PTR(ret);
> +	}

This will be in the BMC core driver and you can simply do

   pd = dev_get_platdata(pdev->dev)

Best regards
Thomas

> +
> +	width = simplefb_get_width_pd(dev, pd);
> +	if (width < 0)
> +		return ERR_PTR(width);
> +
> +	height = simplefb_get_height_pd(dev, pd);
> +	if (height < 0)
> +		return ERR_PTR(height);
> +
> +	stride = simplefb_get_stride_pd(dev, pd);
> +	if (stride < 0)
> +		return ERR_PTR(stride);
> +
> +	if (!stride) {
> +		stride = drm_format_info_min_pitch(format, 0, width);
> +		if (drm_WARN_ON(dev, !stride))
> +			return ERR_PTR(-EINVAL);
> +	}
> +
> +	format = simplefb_get_format_pd(dev, pd);
> +	if (IS_ERR(format))
> +		return ERR_CAST(format);
> +
> +	/*
> +	 * Assume a monitor resolution of 96 dpi if physical dimensions
> +	 * are not specified to get a somewhat reasonable screen size.
> +	 */
> +	if (!width_mm)
> +		width_mm = DRM_MODE_RES_MM(width, 96ul);
> +	if (!height_mm)
> +		height_mm = DRM_MODE_RES_MM(height, 96ul);
> +
> +	sdev->mode = ls2kbmc_mode(width, height, width_mm, height_mm);
> +	sdev->format = format;
> +	sdev->pitch = stride;
> +
> +	drm_dbg(dev, "display mode={" DRM_MODE_FMT "}\n", DRM_MODE_ARG(&sdev->mode));
> +	drm_dbg(dev, "framebuffer format=%p4cc, size=%dx%d, stride=%d byte\n",
> +		&format->format, width, height, stride);
> +
> +	/*
> +	 * Memory management
> +	 */
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return ERR_PTR(-EINVAL);
> +
> +	ret = aperture_remove_conflicting_pci_devices(ppdev, ls2kbmc_driver.name);
> +	if (ret) {
> +		drm_err(dev, "could not acquire memory range %pr: %d\n", res, ret);
> +		return ERR_PTR(ret);
> +	}
> +
> +	drm_dbg(dev, "using I/O memory framebuffer at %pr\n", res);
> +
> +	mem = devm_request_mem_region(&ppdev->dev, res->start, resource_size(res),
> +				      drv->name);
> +	if (!mem) {
> +		/*
> +		 * We cannot make this fatal. Sometimes this comes from magic
> +		 * spaces our resource handlers simply don't know about. Use
> +		 * the I/O-memory resource as-is and try to map that instead.
> +		 */
> +		drm_warn(dev, "could not acquire memory region %pr\n", res);
> +		mem = res;
> +	}
> +
> +	screen_base = devm_ioremap_wc(&ppdev->dev, mem->start, resource_size(mem));
> +	if (!screen_base)
> +		return ERR_PTR(-ENOMEM);
> +
> +	iosys_map_set_vaddr_iomem(&sdev->screen_base, screen_base);
> +
> +	/*
> +	 * Modesetting
> +	 */
> +
> +	ret = drmm_mode_config_init(dev);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	max_width = max_t(unsigned long, width, DRM_SHADOW_PLANE_MAX_WIDTH);
> +	max_height = max_t(unsigned long, height, DRM_SHADOW_PLANE_MAX_HEIGHT);
> +
> +	dev->mode_config.min_width = width;
> +	dev->mode_config.max_width = max_width;
> +	dev->mode_config.min_height = height;
> +	dev->mode_config.max_height = max_height;
> +	dev->mode_config.preferred_depth = format->depth;
> +	dev->mode_config.funcs = &ls2kbmc_mode_config_funcs;
> +
> +	/* Primary plane */
> +
> +	nformats = drm_fb_build_fourcc_list(dev, &format->format, 1,
> +					    sdev->formats, ARRAY_SIZE(sdev->formats));
> +
> +	primary_plane = &sdev->primary_plane;
> +	ret = drm_universal_plane_init(dev, primary_plane, 0, &ls2kbmc_primary_plane_funcs,
> +				       sdev->formats, nformats,
> +				       ls2kbmc_primary_plane_format_modifiers,
> +				       DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (ret)
> +		return ERR_PTR(ret);
> +	drm_plane_helper_add(primary_plane, &ls2kbmc_primary_plane_helper_funcs);
> +	drm_plane_enable_fb_damage_clips(primary_plane);
> +
> +	/* CRTC */
> +
> +	crtc = &sdev->crtc;
> +	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
> +					&ls2kbmc_crtc_funcs, NULL);
> +	if (ret)
> +		return ERR_PTR(ret);
> +	drm_crtc_helper_add(crtc, &ls2kbmc_crtc_helper_funcs);
> +
> +	/* Encoder */
> +
> +	encoder = &sdev->encoder;
> +	ret = drm_encoder_init(dev, encoder, &ls2kbmc_encoder_funcs,
> +			       DRM_MODE_ENCODER_NONE, NULL);
> +	if (ret)
> +		return ERR_PTR(ret);
> +	encoder->possible_crtcs = drm_crtc_mask(crtc);
> +
> +	/* Connector */
> +
> +	connector = &sdev->connector;
> +	ret = drm_connector_init(dev, connector, &ls2kbmc_connector_funcs,
> +				 DRM_MODE_CONNECTOR_Unknown);
> +	if (ret)
> +		return ERR_PTR(ret);
> +	drm_connector_helper_add(connector, &ls2kbmc_connector_helper_funcs);
> +	drm_connector_set_panel_orientation_with_quirk(connector,
> +						       DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
> +						       width, height);
> +
> +	ret = drm_connector_attach_encoder(connector, encoder);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	drm_mode_config_reset(dev);
> +
> +	return sdev;
> +}
> +
> +/*
> + * Platform driver
> + */
> +
> +static int ls2kbmc_probe(struct platform_device *pdev)
> +{
> +	struct ls2kbmc_device *sdev;
> +	struct ls2kbmc_pdata *priv;
> +	struct drm_device *dev;
> +	int ret;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (IS_ERR(priv))
> +		return -ENOMEM;
> +
> +	priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
> +
> +	sdev = ls2kbmc_device_create(&ls2kbmc_driver, pdev, priv);
> +	if (IS_ERR(sdev))
> +		return PTR_ERR(sdev);
> +	dev = &sdev->dev;
> +
> +	ret = drm_dev_register(dev, 0);
> +	if (ret)
> +		return ret;
> +
> +	drm_client_setup(dev, sdev->format);
> +
> +	return 0;
> +}
> +
> +static void ls2kbmc_remove(struct platform_device *pdev)
> +{
> +	struct ls2kbmc_device *sdev = platform_get_drvdata(pdev);
> +	struct drm_device *dev = &sdev->dev;
> +
> +	drm_dev_unplug(dev);
> +}
> +
> +static struct platform_driver ls2kbmc_platform_driver = {
> +	.driver = {
> +		.name = "ls2kbmc-framebuffer",
> +	},
> +	.probe = ls2kbmc_probe,
> +	.remove = ls2kbmc_remove,
> +};
> +
> +module_platform_driver(ls2kbmc_platform_driver);
> +
> +MODULE_DESCRIPTION("DRM driver for Loongson-2K BMC");
> +MODULE_LICENSE("GPL");

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 4/4] drm/ls2kbmc: Add Loongson-2K BMC reset function support
  2024-12-30  9:31 ` [PATCH v1 4/4] drm/ls2kbmc: Add Loongson-2K BMC reset function support Binbin Zhou
@ 2025-01-15  8:57   ` Thomas Zimmermann
  2025-02-11 11:27     ` Binbin Zhou
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Zimmermann @ 2025-01-15  8:57 UTC (permalink / raw)
  To: Binbin Zhou, Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter
  Cc: Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi


Am 30.12.24 um 10:31 schrieb Binbin Zhou:
> Since the display is a sub-function of the Loongson-2K BMC, when the
> BMC reset, the entire BMC PCIe is disconnected, including the display
> which is interrupted.

To me, that's a strong indicator to set up the entire thing from scratch.

>
> Not only do you need to save/restore the relevant PCIe registers
> throughout the reset process, but you also need to re-push the display
> to the monitor at the end.
>
> Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>   drivers/gpu/drm/tiny/ls2kbmc.c | 284 ++++++++++++++++++++++++++++++++-
>   1 file changed, 283 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tiny/ls2kbmc.c b/drivers/gpu/drm/tiny/ls2kbmc.c
> index 909d6c687193..4b440f20cb4d 100644
> --- a/drivers/gpu/drm/tiny/ls2kbmc.c
> +++ b/drivers/gpu/drm/tiny/ls2kbmc.c

Move all the reset detection into the BMC core driver. When you see a 
reset, remove the display's platform device via 
platform_device_unregister(). This will release the device  and the DRM 
driver on top. We do this for simpledrm/efifb/etc. Hence user-space code 
is able to deal with it. Then set up a new platform device when the new 
framebuffer is available. Your DRM driver will bind to it and user-space 
code will continue with the new DRM device.

Best regards
Thomas

> @@ -8,10 +8,12 @@
>    */
>   
>   #include <linux/aperture.h>
> +#include <linux/delay.h>
>   #include <linux/minmax.h>
>   #include <linux/pci.h>
>   #include <linux/platform_data/simplefb.h>
>   #include <linux/platform_device.h>
> +#include <linux/stop_machine.h>
>   
>   #include <drm/drm_atomic.h>
>   #include <drm/drm_atomic_state_helper.h>
> @@ -32,9 +34,27 @@
>   #include <drm/drm_panic.h>
>   #include <drm/drm_probe_helper.h>
>   
> +#define BMC_RESET_DELAY	(60 * HZ)
> +#define BMC_RESET_WAIT	10000
> +
> +static const u32 index[] = { 0x4, 0x10, 0x14, 0x18, 0x1c, 0x20, 0x24,
> +			     0x30, 0x3c, 0x54, 0x58, 0x78, 0x7c, 0x80 };
> +static const u32 cindex[] = { 0x4, 0x10, 0x3c };
> +
> +struct ls2kbmc_pci_data {
> +	u32 d80c;
> +	u32 d71c;
> +	u32 data[14];
> +	u32 cdata[3];
> +};
> +
>   struct ls2kbmc_pdata {
>   	struct pci_dev *pdev;
> +	struct drm_device *ddev;
> +	struct work_struct bmc_work;
> +	unsigned long reset_time;
>   	struct simplefb_platform_data pd;
> +	struct ls2kbmc_pci_data pci_data;
>   };
>   
>   /*
> @@ -583,6 +603,265 @@ static struct ls2kbmc_device *ls2kbmc_device_create(struct drm_driver *drv,
>   	return sdev;
>   }
>   
> +static bool ls2kbmc_bar0_addr_is_set(struct pci_dev *ppdev)
> +{
> +	u32 addr;
> +
> +	pci_read_config_dword(ppdev, PCI_BASE_ADDRESS_0, &addr);
> +	addr &= PCI_BASE_ADDRESS_MEM_MASK;
> +
> +	return addr ? true : false;
> +}
> +
> +static void ls2kbmc_save_pci_data(struct ls2kbmc_pdata *priv)
> +{
> +	struct pci_dev *pdev = priv->pdev;
> +	struct pci_dev *parent = pdev->bus->self;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(index); i++)
> +		pci_read_config_dword(parent, index[i], &priv->pci_data.data[i]);
> +
> +	for (i = 0; i < ARRAY_SIZE(cindex); i++)
> +		pci_read_config_dword(pdev, cindex[i], &priv->pci_data.cdata[i]);
> +
> +	pci_read_config_dword(parent, 0x80c, &priv->pci_data.d80c);
> +	priv->pci_data.d80c = (priv->pci_data.d80c & ~(3 << 17)) | (1 << 17);
> +
> +	pci_read_config_dword(parent, 0x71c, &priv->pci_data.d71c);
> +	priv->pci_data.d71c |= 1 << 26;
> +}
> +
> +static bool ls2kbmc_check_pcie_connected(struct pci_dev *parent, struct drm_device *dev)
> +{
> +	void __iomem *mmio;
> +	int sts, timeout = 10000;
> +
> +	mmio = pci_iomap(parent, 0, 0x100);
> +	if (!mmio)
> +		return false;
> +
> +	writel(readl(mmio) | 0x8, mmio);
> +	while (timeout) {
> +		sts = readl(mmio + 0xc);
> +		if ((sts & 0x11) == 0x11)
> +			break;
> +		mdelay(1);
> +		timeout--;
> +	}
> +
> +	pci_iounmap(parent, mmio);
> +
> +	if (!timeout) {
> +		drm_err(dev, "pcie train failed status=0x%x\n", sts);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static int ls2kbmc_recove_pci_data(void *data)
> +{
> +	struct ls2kbmc_pdata *priv = data;
> +	struct pci_dev *pdev = priv->pdev;
> +	struct drm_device *dev = priv->ddev;
> +	struct pci_dev *parent = pdev->bus->self;
> +	u32 i, timeout, retry = 0;
> +	bool ready;
> +
> +	pci_write_config_dword(parent, PCI_BASE_ADDRESS_2, 0);
> +	pci_write_config_dword(parent, PCI_BASE_ADDRESS_3, 0);
> +	pci_write_config_dword(parent, PCI_BASE_ADDRESS_4, 0);
> +
> +	timeout = 10000;
> +	while (timeout) {
> +		ready = ls2kbmc_bar0_addr_is_set(parent);
> +		if (!ready)
> +			break;
> +		mdelay(1);
> +		timeout--;
> +	};
> +
> +	if (!timeout)
> +		drm_warn(dev, "bar not clear 0\n");
> +
> +retrain:
> +	for (i = 0; i < ARRAY_SIZE(index); i++)
> +		pci_write_config_dword(parent, index[i], priv->pci_data.data[i]);
> +
> +	pci_write_config_dword(parent, 0x80c, priv->pci_data.d80c);
> +	pci_write_config_dword(parent, 0x71c, priv->pci_data.d71c);
> +
> +	/* Check if the pcie is connected */
> +	ready = ls2kbmc_check_pcie_connected(parent, dev);
> +	if (!ready)
> +		return ready;
> +
> +	for (i = 0; i < ARRAY_SIZE(cindex); i++)
> +		pci_write_config_dword(pdev, cindex[i], priv->pci_data.cdata[i]);
> +
> +	drm_info(dev, "pcie recovered done\n");
> +
> +	if (!retry) {
> +		/*wait u-boot ddr config */
> +		mdelay(BMC_RESET_WAIT);
> +		ready = ls2kbmc_bar0_addr_is_set(parent);
> +		if (!ready) {
> +			retry = 1;
> +			goto retrain;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int ls2kbmc_push_drm_mode(struct drm_device *dev)
> +{
> +	struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(dev);
> +	struct drm_crtc *crtc = &sdev->crtc;
> +	struct drm_plane *plane = crtc->primary;
> +	struct drm_connector *connector = &sdev->connector;
> +	struct drm_framebuffer *fb = NULL;
> +	struct drm_mode_set set;
> +	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
> +
> +	mutex_lock(&dev->mode_config.mutex);
> +	connector->funcs->fill_modes(connector, 4096, 4096);
> +	mutex_unlock(&dev->mode_config.mutex);
> +
> +	DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx,
> +				   DRM_MODESET_ACQUIRE_INTERRUPTIBLE, ret);
> +
> +	if (plane->state)
> +		fb = plane->state->fb;
> +	else
> +		fb = plane->fb;
> +
> +	if (!fb) {
> +		drm_dbg(dev, "CRTC doesn't have current FB\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	drm_framebuffer_get(fb);
> +
> +	set.crtc = crtc;
> +	set.x = 0;
> +	set.y = 0;
> +	set.mode = &sdev->mode;
> +	set.connectors = &connector;
> +	set.num_connectors = 1;
> +	set.fb = fb;
> +
> +	ret = crtc->funcs->set_config(&set, &ctx);
> +
> +out:
> +	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> +
> +	return ret;
> +}
> +
> +static void ls2kbmc_events_fn(struct work_struct *work)
> +{
> +	struct ls2kbmc_pdata *priv = container_of(work, struct ls2kbmc_pdata, bmc_work);
> +
> +	/*
> +	 * The pcie is lost when the BMC resets,
> +	 * at which point access to the pcie from other CPUs
> +	 * is suspended to prevent a crash.
> +	 */
> +	stop_machine(ls2kbmc_recove_pci_data, priv, NULL);
> +
> +	drm_info(priv->ddev, "redraw console\n");
> +
> +	/* We need to re-push the display due to previous pcie loss. */
> +	ls2kbmc_push_drm_mode(priv->ddev);
> +}
> +
> +static irqreturn_t ls2kbmc_interrupt(int irq, void *arg)
> +{
> +	struct ls2kbmc_pdata *priv = arg;
> +
> +	if (system_state != SYSTEM_RUNNING)
> +		return IRQ_HANDLED;
> +
> +	/* skip interrupt in BMC_RESET_DELAY */
> +	if (time_after(jiffies, priv->reset_time + BMC_RESET_DELAY))
> +		schedule_work(&priv->bmc_work);
> +
> +	priv->reset_time = jiffies;
> +
> +	return IRQ_HANDLED;
> +}
> +
> +#define BMC_RESET_GPIO			14
> +#define LOONGSON_GPIO_REG_BASE		0x1fe00500
> +#define LOONGSON_GPIO_REG_SIZE		0x18
> +#define LOONGSON_GPIO_OEN		0x0
> +#define LOONGSON_GPIO_FUNC		0x4
> +#define LOONGSON_GPIO_INTPOL		0x10
> +#define LOONGSON_GPIO_INTEN		0x14
> +
> +/* The gpio interrupt is a watchdog interrupt that is triggered when the BMC resets. */
> +static int ls2kbmc_gpio_reset_handler(struct ls2kbmc_pdata *priv)
> +{
> +	int irq, ret = 0;
> +	int gsi = 16 + (BMC_RESET_GPIO & 7);
> +	void __iomem *gpio_base;
> +
> +	/* Since Loongson-3A hardware does not support GPIO interrupt cascade,
> +	 * chip->gpio_to_irq() cannot be implemented,
> +	 * here acpi_register_gsi() is used to get gpio irq.
> +	 */
> +	irq = acpi_register_gsi(NULL, gsi, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW);
> +	if (irq < 0)
> +		return irq;
> +
> +	gpio_base = ioremap(LOONGSON_GPIO_REG_BASE, LOONGSON_GPIO_REG_SIZE);
> +	if (!gpio_base) {
> +		acpi_unregister_gsi(gsi);
> +		return PTR_ERR(gpio_base);
> +	}
> +
> +	writel(readl(gpio_base + LOONGSON_GPIO_OEN) | BIT(BMC_RESET_GPIO),
> +	       gpio_base + LOONGSON_GPIO_OEN);
> +	writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(BMC_RESET_GPIO),
> +	       gpio_base + LOONGSON_GPIO_FUNC);
> +	writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(BMC_RESET_GPIO),
> +	       gpio_base + LOONGSON_GPIO_INTPOL);
> +	writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(BMC_RESET_GPIO),
> +	       gpio_base + LOONGSON_GPIO_INTEN);
> +
> +	ret = request_irq(irq, ls2kbmc_interrupt, IRQF_SHARED | IRQF_TRIGGER_FALLING,
> +			  "ls2kbmc gpio", priv);
> +
> +	acpi_unregister_gsi(gsi);
> +	iounmap(gpio_base);
> +
> +	return ret;
> +}
> +
> +static int ls2kbmc_pdata_initial(struct platform_device *pdev, struct ls2kbmc_pdata *priv)
> +{
> +	int ret;
> +
> +	priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
> +
> +	ls2kbmc_save_pci_data(priv);
> +
> +	INIT_WORK(&priv->bmc_work, ls2kbmc_events_fn);
> +
> +	ret = request_irq(priv->pdev->irq, ls2kbmc_interrupt,
> +			  IRQF_SHARED | IRQF_TRIGGER_RISING, "ls2kbmc pcie", priv);
> +	if (ret) {
> +		pr_err("request_irq(%d) failed\n", priv->pdev->irq);
> +		return ret;
> +	}
> +
> +	return ls2kbmc_gpio_reset_handler(priv);
> +}
> +
>   /*
>    * Platform driver
>    */
> @@ -598,12 +877,15 @@ static int ls2kbmc_probe(struct platform_device *pdev)
>   	if (IS_ERR(priv))
>   		return -ENOMEM;
>   
> -	priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
> +	ret = ls2kbmc_pdata_initial(pdev, priv);
> +	if (ret)
> +		return ret;
>   
>   	sdev = ls2kbmc_device_create(&ls2kbmc_driver, pdev, priv);
>   	if (IS_ERR(sdev))
>   		return PTR_ERR(sdev);
>   	dev = &sdev->dev;
> +	priv->ddev = &sdev->dev;
>   
>   	ret = drm_dev_register(dev, 0);
>   	if (ret)

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 4/4] drm/ls2kbmc: Add Loongson-2K BMC reset function support
  2025-01-15  8:57   ` Thomas Zimmermann
@ 2025-02-11 11:27     ` Binbin Zhou
  2025-03-25 12:39       ` Binbin Zhou
  2025-03-31  7:53       ` Thomas Zimmermann
  0 siblings, 2 replies; 19+ messages in thread
From: Binbin Zhou @ 2025-02-11 11:27 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
	Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi Thomas:

Sorry for my late reply and thanks for your advice.

On Wed, Jan 15, 2025 at 2:57 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
>
> Am 30.12.24 um 10:31 schrieb Binbin Zhou:
> > Since the display is a sub-function of the Loongson-2K BMC, when the
> > BMC reset, the entire BMC PCIe is disconnected, including the display
> > which is interrupted.
>
> To me, that's a strong indicator to set up the entire thing from scratch.
>
> >
> > Not only do you need to save/restore the relevant PCIe registers
> > throughout the reset process, but you also need to re-push the display
> > to the monitor at the end.
> >
> > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> >   drivers/gpu/drm/tiny/ls2kbmc.c | 284 ++++++++++++++++++++++++++++++++-
> >   1 file changed, 283 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/tiny/ls2kbmc.c b/drivers/gpu/drm/tiny/ls2kbmc.c
> > index 909d6c687193..4b440f20cb4d 100644
> > --- a/drivers/gpu/drm/tiny/ls2kbmc.c
> > +++ b/drivers/gpu/drm/tiny/ls2kbmc.c
>
> Move all the reset detection into the BMC core driver. When you see a
> reset, remove the display's platform device via
> platform_device_unregister(). This will release the device  and the DRM
> driver on top. We do this for simpledrm/efifb/etc. Hence user-space code
> is able to deal with it. Then set up a new platform device when the new
> framebuffer is available. Your DRM driver will bind to it and user-space
> code will continue with the new DRM device.

I tried to refactor the bmc restart part according to your scheme. I'm
not quite sure if the experimental results are exactly right.

Key part:

New solution:
1. platform_device_unregister(simpledrm)
2. wait and detect if the BMC reboot is complete;
3. platform_device_register_resndata(simpledrm)

Original solution:
1. wait and detect if the BMC reboot is complete;
2. ls2kbmc_push_drm_mode() pushes display data such as crtc.

When the BMC reboot is completed, the display in the new solution will
lose all the information of the previous desktop and redisplay the
system login interface, while the original solution will keep the
desktop information.

Is this normal for our new solution, or is there something wrong with
my implementation?

>
> Best regards
> Thomas
>
> > @@ -8,10 +8,12 @@
> >    */
> >
> >   #include <linux/aperture.h>
> > +#include <linux/delay.h>
> >   #include <linux/minmax.h>
> >   #include <linux/pci.h>
> >   #include <linux/platform_data/simplefb.h>
> >   #include <linux/platform_device.h>
> > +#include <linux/stop_machine.h>
> >
> >   #include <drm/drm_atomic.h>
> >   #include <drm/drm_atomic_state_helper.h>
> > @@ -32,9 +34,27 @@
> >   #include <drm/drm_panic.h>
> >   #include <drm/drm_probe_helper.h>
> >
> > +#define BMC_RESET_DELAY      (60 * HZ)
> > +#define BMC_RESET_WAIT       10000
> > +
> > +static const u32 index[] = { 0x4, 0x10, 0x14, 0x18, 0x1c, 0x20, 0x24,
> > +                          0x30, 0x3c, 0x54, 0x58, 0x78, 0x7c, 0x80 };
> > +static const u32 cindex[] = { 0x4, 0x10, 0x3c };
> > +
> > +struct ls2kbmc_pci_data {
> > +     u32 d80c;
> > +     u32 d71c;
> > +     u32 data[14];
> > +     u32 cdata[3];
> > +};
> > +
> >   struct ls2kbmc_pdata {
> >       struct pci_dev *pdev;
> > +     struct drm_device *ddev;
> > +     struct work_struct bmc_work;
> > +     unsigned long reset_time;
> >       struct simplefb_platform_data pd;
> > +     struct ls2kbmc_pci_data pci_data;
> >   };
> >
> >   /*
> > @@ -583,6 +603,265 @@ static struct ls2kbmc_device *ls2kbmc_device_create(struct drm_driver *drv,
> >       return sdev;
> >   }
> >
> > +static bool ls2kbmc_bar0_addr_is_set(struct pci_dev *ppdev)
> > +{
> > +     u32 addr;
> > +
> > +     pci_read_config_dword(ppdev, PCI_BASE_ADDRESS_0, &addr);
> > +     addr &= PCI_BASE_ADDRESS_MEM_MASK;
> > +
> > +     return addr ? true : false;
> > +}
> > +
> > +static void ls2kbmc_save_pci_data(struct ls2kbmc_pdata *priv)
> > +{
> > +     struct pci_dev *pdev = priv->pdev;
> > +     struct pci_dev *parent = pdev->bus->self;
> > +     int i;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(index); i++)
> > +             pci_read_config_dword(parent, index[i], &priv->pci_data.data[i]);
> > +
> > +     for (i = 0; i < ARRAY_SIZE(cindex); i++)
> > +             pci_read_config_dword(pdev, cindex[i], &priv->pci_data.cdata[i]);
> > +
> > +     pci_read_config_dword(parent, 0x80c, &priv->pci_data.d80c);
> > +     priv->pci_data.d80c = (priv->pci_data.d80c & ~(3 << 17)) | (1 << 17);
> > +
> > +     pci_read_config_dword(parent, 0x71c, &priv->pci_data.d71c);
> > +     priv->pci_data.d71c |= 1 << 26;
> > +}
> > +
> > +static bool ls2kbmc_check_pcie_connected(struct pci_dev *parent, struct drm_device *dev)
> > +{
> > +     void __iomem *mmio;
> > +     int sts, timeout = 10000;
> > +
> > +     mmio = pci_iomap(parent, 0, 0x100);
> > +     if (!mmio)
> > +             return false;
> > +
> > +     writel(readl(mmio) | 0x8, mmio);
> > +     while (timeout) {
> > +             sts = readl(mmio + 0xc);
> > +             if ((sts & 0x11) == 0x11)
> > +                     break;
> > +             mdelay(1);
> > +             timeout--;
> > +     }
> > +
> > +     pci_iounmap(parent, mmio);
> > +
> > +     if (!timeout) {
> > +             drm_err(dev, "pcie train failed status=0x%x\n", sts);
> > +             return false;
> > +     }
> > +
> > +     return true;
> > +}
> > +
> > +static int ls2kbmc_recove_pci_data(void *data)
> > +{
> > +     struct ls2kbmc_pdata *priv = data;
> > +     struct pci_dev *pdev = priv->pdev;
> > +     struct drm_device *dev = priv->ddev;
> > +     struct pci_dev *parent = pdev->bus->self;
> > +     u32 i, timeout, retry = 0;
> > +     bool ready;
> > +
> > +     pci_write_config_dword(parent, PCI_BASE_ADDRESS_2, 0);
> > +     pci_write_config_dword(parent, PCI_BASE_ADDRESS_3, 0);
> > +     pci_write_config_dword(parent, PCI_BASE_ADDRESS_4, 0);
> > +
> > +     timeout = 10000;
> > +     while (timeout) {
> > +             ready = ls2kbmc_bar0_addr_is_set(parent);
> > +             if (!ready)
> > +                     break;
> > +             mdelay(1);
> > +             timeout--;
> > +     };
> > +
> > +     if (!timeout)
> > +             drm_warn(dev, "bar not clear 0\n");
> > +
> > +retrain:
> > +     for (i = 0; i < ARRAY_SIZE(index); i++)
> > +             pci_write_config_dword(parent, index[i], priv->pci_data.data[i]);
> > +
> > +     pci_write_config_dword(parent, 0x80c, priv->pci_data.d80c);
> > +     pci_write_config_dword(parent, 0x71c, priv->pci_data.d71c);
> > +
> > +     /* Check if the pcie is connected */
> > +     ready = ls2kbmc_check_pcie_connected(parent, dev);
> > +     if (!ready)
> > +             return ready;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(cindex); i++)
> > +             pci_write_config_dword(pdev, cindex[i], priv->pci_data.cdata[i]);
> > +
> > +     drm_info(dev, "pcie recovered done\n");
> > +
> > +     if (!retry) {
> > +             /*wait u-boot ddr config */
> > +             mdelay(BMC_RESET_WAIT);
> > +             ready = ls2kbmc_bar0_addr_is_set(parent);
> > +             if (!ready) {
> > +                     retry = 1;
> > +                     goto retrain;
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int ls2kbmc_push_drm_mode(struct drm_device *dev)
> > +{
> > +     struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(dev);
> > +     struct drm_crtc *crtc = &sdev->crtc;
> > +     struct drm_plane *plane = crtc->primary;
> > +     struct drm_connector *connector = &sdev->connector;
> > +     struct drm_framebuffer *fb = NULL;
> > +     struct drm_mode_set set;
> > +     struct drm_modeset_acquire_ctx ctx;
> > +     int ret;
> > +
> > +     mutex_lock(&dev->mode_config.mutex);
> > +     connector->funcs->fill_modes(connector, 4096, 4096);
> > +     mutex_unlock(&dev->mode_config.mutex);
> > +
> > +     DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx,
> > +                                DRM_MODESET_ACQUIRE_INTERRUPTIBLE, ret);
> > +
> > +     if (plane->state)
> > +             fb = plane->state->fb;
> > +     else
> > +             fb = plane->fb;
> > +
> > +     if (!fb) {
> > +             drm_dbg(dev, "CRTC doesn't have current FB\n");
> > +             ret = -EINVAL;
> > +             goto out;
> > +     }
> > +
> > +     drm_framebuffer_get(fb);
> > +
> > +     set.crtc = crtc;
> > +     set.x = 0;
> > +     set.y = 0;
> > +     set.mode = &sdev->mode;
> > +     set.connectors = &connector;
> > +     set.num_connectors = 1;
> > +     set.fb = fb;
> > +
> > +     ret = crtc->funcs->set_config(&set, &ctx);
> > +
> > +out:
> > +     DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> > +
> > +     return ret;
> > +}
> > +
> > +static void ls2kbmc_events_fn(struct work_struct *work)
> > +{
> > +     struct ls2kbmc_pdata *priv = container_of(work, struct ls2kbmc_pdata, bmc_work);
> > +
> > +     /*
> > +      * The pcie is lost when the BMC resets,
> > +      * at which point access to the pcie from other CPUs
> > +      * is suspended to prevent a crash.
> > +      */
> > +     stop_machine(ls2kbmc_recove_pci_data, priv, NULL);
> > +
> > +     drm_info(priv->ddev, "redraw console\n");
> > +
> > +     /* We need to re-push the display due to previous pcie loss. */
> > +     ls2kbmc_push_drm_mode(priv->ddev);
> > +}
> > +
> > +static irqreturn_t ls2kbmc_interrupt(int irq, void *arg)
> > +{
> > +     struct ls2kbmc_pdata *priv = arg;
> > +
> > +     if (system_state != SYSTEM_RUNNING)
> > +             return IRQ_HANDLED;
> > +
> > +     /* skip interrupt in BMC_RESET_DELAY */
> > +     if (time_after(jiffies, priv->reset_time + BMC_RESET_DELAY))
> > +             schedule_work(&priv->bmc_work);
> > +
> > +     priv->reset_time = jiffies;
> > +
> > +     return IRQ_HANDLED;
> > +}
> > +
> > +#define BMC_RESET_GPIO                       14
> > +#define LOONGSON_GPIO_REG_BASE               0x1fe00500
> > +#define LOONGSON_GPIO_REG_SIZE               0x18
> > +#define LOONGSON_GPIO_OEN            0x0
> > +#define LOONGSON_GPIO_FUNC           0x4
> > +#define LOONGSON_GPIO_INTPOL         0x10
> > +#define LOONGSON_GPIO_INTEN          0x14
> > +
> > +/* The gpio interrupt is a watchdog interrupt that is triggered when the BMC resets. */
> > +static int ls2kbmc_gpio_reset_handler(struct ls2kbmc_pdata *priv)
> > +{
> > +     int irq, ret = 0;
> > +     int gsi = 16 + (BMC_RESET_GPIO & 7);
> > +     void __iomem *gpio_base;
> > +
> > +     /* Since Loongson-3A hardware does not support GPIO interrupt cascade,
> > +      * chip->gpio_to_irq() cannot be implemented,
> > +      * here acpi_register_gsi() is used to get gpio irq.
> > +      */
> > +     irq = acpi_register_gsi(NULL, gsi, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW);
> > +     if (irq < 0)
> > +             return irq;
> > +
> > +     gpio_base = ioremap(LOONGSON_GPIO_REG_BASE, LOONGSON_GPIO_REG_SIZE);
> > +     if (!gpio_base) {
> > +             acpi_unregister_gsi(gsi);
> > +             return PTR_ERR(gpio_base);
> > +     }
> > +
> > +     writel(readl(gpio_base + LOONGSON_GPIO_OEN) | BIT(BMC_RESET_GPIO),
> > +            gpio_base + LOONGSON_GPIO_OEN);
> > +     writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(BMC_RESET_GPIO),
> > +            gpio_base + LOONGSON_GPIO_FUNC);
> > +     writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(BMC_RESET_GPIO),
> > +            gpio_base + LOONGSON_GPIO_INTPOL);
> > +     writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(BMC_RESET_GPIO),
> > +            gpio_base + LOONGSON_GPIO_INTEN);
> > +
> > +     ret = request_irq(irq, ls2kbmc_interrupt, IRQF_SHARED | IRQF_TRIGGER_FALLING,
> > +                       "ls2kbmc gpio", priv);
> > +
> > +     acpi_unregister_gsi(gsi);
> > +     iounmap(gpio_base);
> > +
> > +     return ret;
> > +}
> > +
> > +static int ls2kbmc_pdata_initial(struct platform_device *pdev, struct ls2kbmc_pdata *priv)
> > +{
> > +     int ret;
> > +
> > +     priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
> > +
> > +     ls2kbmc_save_pci_data(priv);
> > +
> > +     INIT_WORK(&priv->bmc_work, ls2kbmc_events_fn);
> > +
> > +     ret = request_irq(priv->pdev->irq, ls2kbmc_interrupt,
> > +                       IRQF_SHARED | IRQF_TRIGGER_RISING, "ls2kbmc pcie", priv);
> > +     if (ret) {
> > +             pr_err("request_irq(%d) failed\n", priv->pdev->irq);
> > +             return ret;
> > +     }
> > +
> > +     return ls2kbmc_gpio_reset_handler(priv);
> > +}
> > +
> >   /*
> >    * Platform driver
> >    */
> > @@ -598,12 +877,15 @@ static int ls2kbmc_probe(struct platform_device *pdev)
> >       if (IS_ERR(priv))
> >               return -ENOMEM;
> >
> > -     priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
> > +     ret = ls2kbmc_pdata_initial(pdev, priv);
> > +     if (ret)
> > +             return ret;
> >
> >       sdev = ls2kbmc_device_create(&ls2kbmc_driver, pdev, priv);
> >       if (IS_ERR(sdev))
> >               return PTR_ERR(sdev);
> >       dev = &sdev->dev;
> > +     priv->ddev = &sdev->dev;
> >
> >       ret = drm_dev_register(dev, 0);
> >       if (ret)
>
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
>


--
Thanks.
Binbin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 4/4] drm/ls2kbmc: Add Loongson-2K BMC reset function support
  2025-02-11 11:27     ` Binbin Zhou
@ 2025-03-25 12:39       ` Binbin Zhou
  2025-03-31  7:53       ` Thomas Zimmermann
  1 sibling, 0 replies; 19+ messages in thread
From: Binbin Zhou @ 2025-03-25 12:39 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
	Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

On Tue, Feb 11, 2025 at 7:27 PM Binbin Zhou <zhoubb.aaron@gmail.com> wrote:
>
> Hi Thomas:
>
> Sorry for my late reply and thanks for your advice.
>
> On Wed, Jan 15, 2025 at 2:57 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> > Hi
> >
> >
> > Am 30.12.24 um 10:31 schrieb Binbin Zhou:
> > > Since the display is a sub-function of the Loongson-2K BMC, when the
> > > BMC reset, the entire BMC PCIe is disconnected, including the display
> > > which is interrupted.
> >
> > To me, that's a strong indicator to set up the entire thing from scratch.
> >
> > >
> > > Not only do you need to save/restore the relevant PCIe registers
> > > throughout the reset process, but you also need to re-push the display
> > > to the monitor at the end.
> > >
> > > Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
> > > Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > ---
> > >   drivers/gpu/drm/tiny/ls2kbmc.c | 284 ++++++++++++++++++++++++++++++++-
> > >   1 file changed, 283 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/tiny/ls2kbmc.c b/drivers/gpu/drm/tiny/ls2kbmc.c
> > > index 909d6c687193..4b440f20cb4d 100644
> > > --- a/drivers/gpu/drm/tiny/ls2kbmc.c
> > > +++ b/drivers/gpu/drm/tiny/ls2kbmc.c
> >
> > Move all the reset detection into the BMC core driver. When you see a
> > reset, remove the display's platform device via
> > platform_device_unregister(). This will release the device  and the DRM
> > driver on top. We do this for simpledrm/efifb/etc. Hence user-space code
> > is able to deal with it. Then set up a new platform device when the new
> > framebuffer is available. Your DRM driver will bind to it and user-space
> > code will continue with the new DRM device.
>
> I tried to refactor the bmc restart part according to your scheme. I'm
> not quite sure if the experimental results are exactly right.
>
> Key part:
>
> New solution:
> 1. platform_device_unregister(simpledrm)
> 2. wait and detect if the BMC reboot is complete;
> 3. platform_device_register_resndata(simpledrm)
>
> Original solution:
> 1. wait and detect if the BMC reboot is complete;
> 2. ls2kbmc_push_drm_mode() pushes display data such as crtc.
>
> When the BMC reboot is completed, the display in the new solution will
> lose all the information of the previous desktop and redisplay the
> system login interface, while the original solution will keep the
> desktop information.
>
> Is this normal for our new solution, or is there something wrong with
> my implementation?

Hi Thomas:

Sorry to bother you and my last email[1] may have been lost.
Some time ago, I modified the code and tested it based on your
previous comments, but the results were a bit different than expected.

I'm not sure if the code changes are wrong, so it would be great if
you could take the time to help me out:

[1]: https://lore.kernel.org/all/CAMpQs4JXKu4GDD79Mdkq9vASSDE_5SQsjsg4htfaZ5yGm3=k5g@mail.gmail.com/
>
> >
> > Best regards
> > Thomas
> >
> > > @@ -8,10 +8,12 @@
> > >    */
> > >
> > >   #include <linux/aperture.h>
> > > +#include <linux/delay.h>
> > >   #include <linux/minmax.h>
> > >   #include <linux/pci.h>
> > >   #include <linux/platform_data/simplefb.h>
> > >   #include <linux/platform_device.h>
> > > +#include <linux/stop_machine.h>
> > >
> > >   #include <drm/drm_atomic.h>
> > >   #include <drm/drm_atomic_state_helper.h>
> > > @@ -32,9 +34,27 @@
> > >   #include <drm/drm_panic.h>
> > >   #include <drm/drm_probe_helper.h>
> > >
> > > +#define BMC_RESET_DELAY      (60 * HZ)
> > > +#define BMC_RESET_WAIT       10000
> > > +
> > > +static const u32 index[] = { 0x4, 0x10, 0x14, 0x18, 0x1c, 0x20, 0x24,
> > > +                          0x30, 0x3c, 0x54, 0x58, 0x78, 0x7c, 0x80 };
> > > +static const u32 cindex[] = { 0x4, 0x10, 0x3c };
> > > +
> > > +struct ls2kbmc_pci_data {
> > > +     u32 d80c;
> > > +     u32 d71c;
> > > +     u32 data[14];
> > > +     u32 cdata[3];
> > > +};
> > > +
> > >   struct ls2kbmc_pdata {
> > >       struct pci_dev *pdev;
> > > +     struct drm_device *ddev;
> > > +     struct work_struct bmc_work;
> > > +     unsigned long reset_time;
> > >       struct simplefb_platform_data pd;
> > > +     struct ls2kbmc_pci_data pci_data;
> > >   };
> > >
> > >   /*
> > > @@ -583,6 +603,265 @@ static struct ls2kbmc_device *ls2kbmc_device_create(struct drm_driver *drv,
> > >       return sdev;
> > >   }
> > >
> > > +static bool ls2kbmc_bar0_addr_is_set(struct pci_dev *ppdev)
> > > +{
> > > +     u32 addr;
> > > +
> > > +     pci_read_config_dword(ppdev, PCI_BASE_ADDRESS_0, &addr);
> > > +     addr &= PCI_BASE_ADDRESS_MEM_MASK;
> > > +
> > > +     return addr ? true : false;
> > > +}
> > > +
> > > +static void ls2kbmc_save_pci_data(struct ls2kbmc_pdata *priv)
> > > +{
> > > +     struct pci_dev *pdev = priv->pdev;
> > > +     struct pci_dev *parent = pdev->bus->self;
> > > +     int i;
> > > +
> > > +     for (i = 0; i < ARRAY_SIZE(index); i++)
> > > +             pci_read_config_dword(parent, index[i], &priv->pci_data.data[i]);
> > > +
> > > +     for (i = 0; i < ARRAY_SIZE(cindex); i++)
> > > +             pci_read_config_dword(pdev, cindex[i], &priv->pci_data.cdata[i]);
> > > +
> > > +     pci_read_config_dword(parent, 0x80c, &priv->pci_data.d80c);
> > > +     priv->pci_data.d80c = (priv->pci_data.d80c & ~(3 << 17)) | (1 << 17);
> > > +
> > > +     pci_read_config_dword(parent, 0x71c, &priv->pci_data.d71c);
> > > +     priv->pci_data.d71c |= 1 << 26;
> > > +}
> > > +
> > > +static bool ls2kbmc_check_pcie_connected(struct pci_dev *parent, struct drm_device *dev)
> > > +{
> > > +     void __iomem *mmio;
> > > +     int sts, timeout = 10000;
> > > +
> > > +     mmio = pci_iomap(parent, 0, 0x100);
> > > +     if (!mmio)
> > > +             return false;
> > > +
> > > +     writel(readl(mmio) | 0x8, mmio);
> > > +     while (timeout) {
> > > +             sts = readl(mmio + 0xc);
> > > +             if ((sts & 0x11) == 0x11)
> > > +                     break;
> > > +             mdelay(1);
> > > +             timeout--;
> > > +     }
> > > +
> > > +     pci_iounmap(parent, mmio);
> > > +
> > > +     if (!timeout) {
> > > +             drm_err(dev, "pcie train failed status=0x%x\n", sts);
> > > +             return false;
> > > +     }
> > > +
> > > +     return true;
> > > +}
> > > +
> > > +static int ls2kbmc_recove_pci_data(void *data)
> > > +{
> > > +     struct ls2kbmc_pdata *priv = data;
> > > +     struct pci_dev *pdev = priv->pdev;
> > > +     struct drm_device *dev = priv->ddev;
> > > +     struct pci_dev *parent = pdev->bus->self;
> > > +     u32 i, timeout, retry = 0;
> > > +     bool ready;
> > > +
> > > +     pci_write_config_dword(parent, PCI_BASE_ADDRESS_2, 0);
> > > +     pci_write_config_dword(parent, PCI_BASE_ADDRESS_3, 0);
> > > +     pci_write_config_dword(parent, PCI_BASE_ADDRESS_4, 0);
> > > +
> > > +     timeout = 10000;
> > > +     while (timeout) {
> > > +             ready = ls2kbmc_bar0_addr_is_set(parent);
> > > +             if (!ready)
> > > +                     break;
> > > +             mdelay(1);
> > > +             timeout--;
> > > +     };
> > > +
> > > +     if (!timeout)
> > > +             drm_warn(dev, "bar not clear 0\n");
> > > +
> > > +retrain:
> > > +     for (i = 0; i < ARRAY_SIZE(index); i++)
> > > +             pci_write_config_dword(parent, index[i], priv->pci_data.data[i]);
> > > +
> > > +     pci_write_config_dword(parent, 0x80c, priv->pci_data.d80c);
> > > +     pci_write_config_dword(parent, 0x71c, priv->pci_data.d71c);
> > > +
> > > +     /* Check if the pcie is connected */
> > > +     ready = ls2kbmc_check_pcie_connected(parent, dev);
> > > +     if (!ready)
> > > +             return ready;
> > > +
> > > +     for (i = 0; i < ARRAY_SIZE(cindex); i++)
> > > +             pci_write_config_dword(pdev, cindex[i], priv->pci_data.cdata[i]);
> > > +
> > > +     drm_info(dev, "pcie recovered done\n");
> > > +
> > > +     if (!retry) {
> > > +             /*wait u-boot ddr config */
> > > +             mdelay(BMC_RESET_WAIT);
> > > +             ready = ls2kbmc_bar0_addr_is_set(parent);
> > > +             if (!ready) {
> > > +                     retry = 1;
> > > +                     goto retrain;
> > > +             }
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int ls2kbmc_push_drm_mode(struct drm_device *dev)
> > > +{
> > > +     struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(dev);
> > > +     struct drm_crtc *crtc = &sdev->crtc;
> > > +     struct drm_plane *plane = crtc->primary;
> > > +     struct drm_connector *connector = &sdev->connector;
> > > +     struct drm_framebuffer *fb = NULL;
> > > +     struct drm_mode_set set;
> > > +     struct drm_modeset_acquire_ctx ctx;
> > > +     int ret;
> > > +
> > > +     mutex_lock(&dev->mode_config.mutex);
> > > +     connector->funcs->fill_modes(connector, 4096, 4096);
> > > +     mutex_unlock(&dev->mode_config.mutex);
> > > +
> > > +     DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx,
> > > +                                DRM_MODESET_ACQUIRE_INTERRUPTIBLE, ret);
> > > +
> > > +     if (plane->state)
> > > +             fb = plane->state->fb;
> > > +     else
> > > +             fb = plane->fb;
> > > +
> > > +     if (!fb) {
> > > +             drm_dbg(dev, "CRTC doesn't have current FB\n");
> > > +             ret = -EINVAL;
> > > +             goto out;
> > > +     }
> > > +
> > > +     drm_framebuffer_get(fb);
> > > +
> > > +     set.crtc = crtc;
> > > +     set.x = 0;
> > > +     set.y = 0;
> > > +     set.mode = &sdev->mode;
> > > +     set.connectors = &connector;
> > > +     set.num_connectors = 1;
> > > +     set.fb = fb;
> > > +
> > > +     ret = crtc->funcs->set_config(&set, &ctx);
> > > +
> > > +out:
> > > +     DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static void ls2kbmc_events_fn(struct work_struct *work)
> > > +{
> > > +     struct ls2kbmc_pdata *priv = container_of(work, struct ls2kbmc_pdata, bmc_work);
> > > +
> > > +     /*
> > > +      * The pcie is lost when the BMC resets,
> > > +      * at which point access to the pcie from other CPUs
> > > +      * is suspended to prevent a crash.
> > > +      */
> > > +     stop_machine(ls2kbmc_recove_pci_data, priv, NULL);
> > > +
> > > +     drm_info(priv->ddev, "redraw console\n");
> > > +
> > > +     /* We need to re-push the display due to previous pcie loss. */
> > > +     ls2kbmc_push_drm_mode(priv->ddev);
> > > +}
> > > +
> > > +static irqreturn_t ls2kbmc_interrupt(int irq, void *arg)
> > > +{
> > > +     struct ls2kbmc_pdata *priv = arg;
> > > +
> > > +     if (system_state != SYSTEM_RUNNING)
> > > +             return IRQ_HANDLED;
> > > +
> > > +     /* skip interrupt in BMC_RESET_DELAY */
> > > +     if (time_after(jiffies, priv->reset_time + BMC_RESET_DELAY))
> > > +             schedule_work(&priv->bmc_work);
> > > +
> > > +     priv->reset_time = jiffies;
> > > +
> > > +     return IRQ_HANDLED;
> > > +}
> > > +
> > > +#define BMC_RESET_GPIO                       14
> > > +#define LOONGSON_GPIO_REG_BASE               0x1fe00500
> > > +#define LOONGSON_GPIO_REG_SIZE               0x18
> > > +#define LOONGSON_GPIO_OEN            0x0
> > > +#define LOONGSON_GPIO_FUNC           0x4
> > > +#define LOONGSON_GPIO_INTPOL         0x10
> > > +#define LOONGSON_GPIO_INTEN          0x14
> > > +
> > > +/* The gpio interrupt is a watchdog interrupt that is triggered when the BMC resets. */
> > > +static int ls2kbmc_gpio_reset_handler(struct ls2kbmc_pdata *priv)
> > > +{
> > > +     int irq, ret = 0;
> > > +     int gsi = 16 + (BMC_RESET_GPIO & 7);
> > > +     void __iomem *gpio_base;
> > > +
> > > +     /* Since Loongson-3A hardware does not support GPIO interrupt cascade,
> > > +      * chip->gpio_to_irq() cannot be implemented,
> > > +      * here acpi_register_gsi() is used to get gpio irq.
> > > +      */
> > > +     irq = acpi_register_gsi(NULL, gsi, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW);
> > > +     if (irq < 0)
> > > +             return irq;
> > > +
> > > +     gpio_base = ioremap(LOONGSON_GPIO_REG_BASE, LOONGSON_GPIO_REG_SIZE);
> > > +     if (!gpio_base) {
> > > +             acpi_unregister_gsi(gsi);
> > > +             return PTR_ERR(gpio_base);
> > > +     }
> > > +
> > > +     writel(readl(gpio_base + LOONGSON_GPIO_OEN) | BIT(BMC_RESET_GPIO),
> > > +            gpio_base + LOONGSON_GPIO_OEN);
> > > +     writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(BMC_RESET_GPIO),
> > > +            gpio_base + LOONGSON_GPIO_FUNC);
> > > +     writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(BMC_RESET_GPIO),
> > > +            gpio_base + LOONGSON_GPIO_INTPOL);
> > > +     writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(BMC_RESET_GPIO),
> > > +            gpio_base + LOONGSON_GPIO_INTEN);
> > > +
> > > +     ret = request_irq(irq, ls2kbmc_interrupt, IRQF_SHARED | IRQF_TRIGGER_FALLING,
> > > +                       "ls2kbmc gpio", priv);
> > > +
> > > +     acpi_unregister_gsi(gsi);
> > > +     iounmap(gpio_base);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static int ls2kbmc_pdata_initial(struct platform_device *pdev, struct ls2kbmc_pdata *priv)
> > > +{
> > > +     int ret;
> > > +
> > > +     priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
> > > +
> > > +     ls2kbmc_save_pci_data(priv);
> > > +
> > > +     INIT_WORK(&priv->bmc_work, ls2kbmc_events_fn);
> > > +
> > > +     ret = request_irq(priv->pdev->irq, ls2kbmc_interrupt,
> > > +                       IRQF_SHARED | IRQF_TRIGGER_RISING, "ls2kbmc pcie", priv);
> > > +     if (ret) {
> > > +             pr_err("request_irq(%d) failed\n", priv->pdev->irq);
> > > +             return ret;
> > > +     }
> > > +
> > > +     return ls2kbmc_gpio_reset_handler(priv);
> > > +}
> > > +
> > >   /*
> > >    * Platform driver
> > >    */
> > > @@ -598,12 +877,15 @@ static int ls2kbmc_probe(struct platform_device *pdev)
> > >       if (IS_ERR(priv))
> > >               return -ENOMEM;
> > >
> > > -     priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
> > > +     ret = ls2kbmc_pdata_initial(pdev, priv);
> > > +     if (ret)
> > > +             return ret;
> > >
> > >       sdev = ls2kbmc_device_create(&ls2kbmc_driver, pdev, priv);
> > >       if (IS_ERR(sdev))
> > >               return PTR_ERR(sdev);
> > >       dev = &sdev->dev;
> > > +     priv->ddev = &sdev->dev;
> > >
> > >       ret = drm_dev_register(dev, 0);
> > >       if (ret)
> >
> > --
> > --
> > Thomas Zimmermann
> > Graphics Driver Developer
> > SUSE Software Solutions Germany GmbH
> > Frankenstrasse 146, 90461 Nuernberg, Germany
> > GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> > HRB 36809 (AG Nuernberg)
> >
>
>
> --
> Thanks.
> Binbin



-- 
Thanks.
Binbin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 4/4] drm/ls2kbmc: Add Loongson-2K BMC reset function support
  2025-02-11 11:27     ` Binbin Zhou
  2025-03-25 12:39       ` Binbin Zhou
@ 2025-03-31  7:53       ` Thomas Zimmermann
  1 sibling, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2025-03-31  7:53 UTC (permalink / raw)
  To: Binbin Zhou
  Cc: Binbin Zhou, Huacai Chen, Lee Jones, Corey Minyard,
	Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
	Huacai Chen, linux-kernel, openipmi-developer, dri-devel,
	Xuerui Wang, loongarch, Chong Qiao

Hi

Am 11.02.25 um 12:27 schrieb Binbin Zhou:
> Hi Thomas:
>
> Sorry for my late reply and thanks for your advice.

Apologies, I missed your email.


>
> On Wed, Jan 15, 2025 at 2:57 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Hi
>>
>>
>> Am 30.12.24 um 10:31 schrieb Binbin Zhou:
>>> Since the display is a sub-function of the Loongson-2K BMC, when the
>>> BMC reset, the entire BMC PCIe is disconnected, including the display
>>> which is interrupted.
>> To me, that's a strong indicator to set up the entire thing from scratch.
>>
>>> Not only do you need to save/restore the relevant PCIe registers
>>> throughout the reset process, but you also need to re-push the display
>>> to the monitor at the end.
>>>
>>> Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
>>> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
>>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
>>> ---
>>>    drivers/gpu/drm/tiny/ls2kbmc.c | 284 ++++++++++++++++++++++++++++++++-
>>>    1 file changed, 283 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/tiny/ls2kbmc.c b/drivers/gpu/drm/tiny/ls2kbmc.c
>>> index 909d6c687193..4b440f20cb4d 100644
>>> --- a/drivers/gpu/drm/tiny/ls2kbmc.c
>>> +++ b/drivers/gpu/drm/tiny/ls2kbmc.c
>> Move all the reset detection into the BMC core driver. When you see a
>> reset, remove the display's platform device via
>> platform_device_unregister(). This will release the device  and the DRM
>> driver on top. We do this for simpledrm/efifb/etc. Hence user-space code
>> is able to deal with it. Then set up a new platform device when the new
>> framebuffer is available. Your DRM driver will bind to it and user-space
>> code will continue with the new DRM device.
> I tried to refactor the bmc restart part according to your scheme. I'm
> not quite sure if the experimental results are exactly right.
>
> Key part:
>
> New solution:
> 1. platform_device_unregister(simpledrm)
> 2. wait and detect if the BMC reboot is complete;
> 3. platform_device_register_resndata(simpledrm)

You are doing this in the MFD driver, right?

Apologies again, I find it very confusing which component does what.

If the MFD driver created the initial graphics device, it should also 
remove it on BMC resets. And then establish a new graphics device when 
the reset has been complete. It shouldn't matter if the driver is 
simpledrm or a custom driver for your hardware.


>
> Original solution:
> 1. wait and detect if the BMC reboot is complete;
> 2. ls2kbmc_push_drm_mode() pushes display data such as crtc.
>
> When the BMC reboot is completed, the display in the new solution will
> lose all the information of the previous desktop and redisplay the
> system login interface, while the original solution will keep the
> desktop information.
>
> Is this normal for our new solution, or is there something wrong with
> my implementation?

The desktop should mostly remain as-is. Maybe flicker, but not go back 
to the login screen. Is there any error in the log files of the desktop 
or compositor?

Best regards
Thomas

>
>> Best regards
>> Thomas
>>
>>> @@ -8,10 +8,12 @@
>>>     */
>>>
>>>    #include <linux/aperture.h>
>>> +#include <linux/delay.h>
>>>    #include <linux/minmax.h>
>>>    #include <linux/pci.h>
>>>    #include <linux/platform_data/simplefb.h>
>>>    #include <linux/platform_device.h>
>>> +#include <linux/stop_machine.h>
>>>
>>>    #include <drm/drm_atomic.h>
>>>    #include <drm/drm_atomic_state_helper.h>
>>> @@ -32,9 +34,27 @@
>>>    #include <drm/drm_panic.h>
>>>    #include <drm/drm_probe_helper.h>
>>>
>>> +#define BMC_RESET_DELAY      (60 * HZ)
>>> +#define BMC_RESET_WAIT       10000
>>> +
>>> +static const u32 index[] = { 0x4, 0x10, 0x14, 0x18, 0x1c, 0x20, 0x24,
>>> +                          0x30, 0x3c, 0x54, 0x58, 0x78, 0x7c, 0x80 };
>>> +static const u32 cindex[] = { 0x4, 0x10, 0x3c };
>>> +
>>> +struct ls2kbmc_pci_data {
>>> +     u32 d80c;
>>> +     u32 d71c;
>>> +     u32 data[14];
>>> +     u32 cdata[3];
>>> +};
>>> +
>>>    struct ls2kbmc_pdata {
>>>        struct pci_dev *pdev;
>>> +     struct drm_device *ddev;
>>> +     struct work_struct bmc_work;
>>> +     unsigned long reset_time;
>>>        struct simplefb_platform_data pd;
>>> +     struct ls2kbmc_pci_data pci_data;
>>>    };
>>>
>>>    /*
>>> @@ -583,6 +603,265 @@ static struct ls2kbmc_device *ls2kbmc_device_create(struct drm_driver *drv,
>>>        return sdev;
>>>    }
>>>
>>> +static bool ls2kbmc_bar0_addr_is_set(struct pci_dev *ppdev)
>>> +{
>>> +     u32 addr;
>>> +
>>> +     pci_read_config_dword(ppdev, PCI_BASE_ADDRESS_0, &addr);
>>> +     addr &= PCI_BASE_ADDRESS_MEM_MASK;
>>> +
>>> +     return addr ? true : false;
>>> +}
>>> +
>>> +static void ls2kbmc_save_pci_data(struct ls2kbmc_pdata *priv)
>>> +{
>>> +     struct pci_dev *pdev = priv->pdev;
>>> +     struct pci_dev *parent = pdev->bus->self;
>>> +     int i;
>>> +
>>> +     for (i = 0; i < ARRAY_SIZE(index); i++)
>>> +             pci_read_config_dword(parent, index[i], &priv->pci_data.data[i]);
>>> +
>>> +     for (i = 0; i < ARRAY_SIZE(cindex); i++)
>>> +             pci_read_config_dword(pdev, cindex[i], &priv->pci_data.cdata[i]);
>>> +
>>> +     pci_read_config_dword(parent, 0x80c, &priv->pci_data.d80c);
>>> +     priv->pci_data.d80c = (priv->pci_data.d80c & ~(3 << 17)) | (1 << 17);
>>> +
>>> +     pci_read_config_dword(parent, 0x71c, &priv->pci_data.d71c);
>>> +     priv->pci_data.d71c |= 1 << 26;
>>> +}
>>> +
>>> +static bool ls2kbmc_check_pcie_connected(struct pci_dev *parent, struct drm_device *dev)
>>> +{
>>> +     void __iomem *mmio;
>>> +     int sts, timeout = 10000;
>>> +
>>> +     mmio = pci_iomap(parent, 0, 0x100);
>>> +     if (!mmio)
>>> +             return false;
>>> +
>>> +     writel(readl(mmio) | 0x8, mmio);
>>> +     while (timeout) {
>>> +             sts = readl(mmio + 0xc);
>>> +             if ((sts & 0x11) == 0x11)
>>> +                     break;
>>> +             mdelay(1);
>>> +             timeout--;
>>> +     }
>>> +
>>> +     pci_iounmap(parent, mmio);
>>> +
>>> +     if (!timeout) {
>>> +             drm_err(dev, "pcie train failed status=0x%x\n", sts);
>>> +             return false;
>>> +     }
>>> +
>>> +     return true;
>>> +}
>>> +
>>> +static int ls2kbmc_recove_pci_data(void *data)
>>> +{
>>> +     struct ls2kbmc_pdata *priv = data;
>>> +     struct pci_dev *pdev = priv->pdev;
>>> +     struct drm_device *dev = priv->ddev;
>>> +     struct pci_dev *parent = pdev->bus->self;
>>> +     u32 i, timeout, retry = 0;
>>> +     bool ready;
>>> +
>>> +     pci_write_config_dword(parent, PCI_BASE_ADDRESS_2, 0);
>>> +     pci_write_config_dword(parent, PCI_BASE_ADDRESS_3, 0);
>>> +     pci_write_config_dword(parent, PCI_BASE_ADDRESS_4, 0);
>>> +
>>> +     timeout = 10000;
>>> +     while (timeout) {
>>> +             ready = ls2kbmc_bar0_addr_is_set(parent);
>>> +             if (!ready)
>>> +                     break;
>>> +             mdelay(1);
>>> +             timeout--;
>>> +     };
>>> +
>>> +     if (!timeout)
>>> +             drm_warn(dev, "bar not clear 0\n");
>>> +
>>> +retrain:
>>> +     for (i = 0; i < ARRAY_SIZE(index); i++)
>>> +             pci_write_config_dword(parent, index[i], priv->pci_data.data[i]);
>>> +
>>> +     pci_write_config_dword(parent, 0x80c, priv->pci_data.d80c);
>>> +     pci_write_config_dword(parent, 0x71c, priv->pci_data.d71c);
>>> +
>>> +     /* Check if the pcie is connected */
>>> +     ready = ls2kbmc_check_pcie_connected(parent, dev);
>>> +     if (!ready)
>>> +             return ready;
>>> +
>>> +     for (i = 0; i < ARRAY_SIZE(cindex); i++)
>>> +             pci_write_config_dword(pdev, cindex[i], priv->pci_data.cdata[i]);
>>> +
>>> +     drm_info(dev, "pcie recovered done\n");
>>> +
>>> +     if (!retry) {
>>> +             /*wait u-boot ddr config */
>>> +             mdelay(BMC_RESET_WAIT);
>>> +             ready = ls2kbmc_bar0_addr_is_set(parent);
>>> +             if (!ready) {
>>> +                     retry = 1;
>>> +                     goto retrain;
>>> +             }
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int ls2kbmc_push_drm_mode(struct drm_device *dev)
>>> +{
>>> +     struct ls2kbmc_device *sdev = ls2kbmc_device_of_dev(dev);
>>> +     struct drm_crtc *crtc = &sdev->crtc;
>>> +     struct drm_plane *plane = crtc->primary;
>>> +     struct drm_connector *connector = &sdev->connector;
>>> +     struct drm_framebuffer *fb = NULL;
>>> +     struct drm_mode_set set;
>>> +     struct drm_modeset_acquire_ctx ctx;
>>> +     int ret;
>>> +
>>> +     mutex_lock(&dev->mode_config.mutex);
>>> +     connector->funcs->fill_modes(connector, 4096, 4096);
>>> +     mutex_unlock(&dev->mode_config.mutex);
>>> +
>>> +     DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx,
>>> +                                DRM_MODESET_ACQUIRE_INTERRUPTIBLE, ret);
>>> +
>>> +     if (plane->state)
>>> +             fb = plane->state->fb;
>>> +     else
>>> +             fb = plane->fb;
>>> +
>>> +     if (!fb) {
>>> +             drm_dbg(dev, "CRTC doesn't have current FB\n");
>>> +             ret = -EINVAL;
>>> +             goto out;
>>> +     }
>>> +
>>> +     drm_framebuffer_get(fb);
>>> +
>>> +     set.crtc = crtc;
>>> +     set.x = 0;
>>> +     set.y = 0;
>>> +     set.mode = &sdev->mode;
>>> +     set.connectors = &connector;
>>> +     set.num_connectors = 1;
>>> +     set.fb = fb;
>>> +
>>> +     ret = crtc->funcs->set_config(&set, &ctx);
>>> +
>>> +out:
>>> +     DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +static void ls2kbmc_events_fn(struct work_struct *work)
>>> +{
>>> +     struct ls2kbmc_pdata *priv = container_of(work, struct ls2kbmc_pdata, bmc_work);
>>> +
>>> +     /*
>>> +      * The pcie is lost when the BMC resets,
>>> +      * at which point access to the pcie from other CPUs
>>> +      * is suspended to prevent a crash.
>>> +      */
>>> +     stop_machine(ls2kbmc_recove_pci_data, priv, NULL);
>>> +
>>> +     drm_info(priv->ddev, "redraw console\n");
>>> +
>>> +     /* We need to re-push the display due to previous pcie loss. */
>>> +     ls2kbmc_push_drm_mode(priv->ddev);
>>> +}
>>> +
>>> +static irqreturn_t ls2kbmc_interrupt(int irq, void *arg)
>>> +{
>>> +     struct ls2kbmc_pdata *priv = arg;
>>> +
>>> +     if (system_state != SYSTEM_RUNNING)
>>> +             return IRQ_HANDLED;
>>> +
>>> +     /* skip interrupt in BMC_RESET_DELAY */
>>> +     if (time_after(jiffies, priv->reset_time + BMC_RESET_DELAY))
>>> +             schedule_work(&priv->bmc_work);
>>> +
>>> +     priv->reset_time = jiffies;
>>> +
>>> +     return IRQ_HANDLED;
>>> +}
>>> +
>>> +#define BMC_RESET_GPIO                       14
>>> +#define LOONGSON_GPIO_REG_BASE               0x1fe00500
>>> +#define LOONGSON_GPIO_REG_SIZE               0x18
>>> +#define LOONGSON_GPIO_OEN            0x0
>>> +#define LOONGSON_GPIO_FUNC           0x4
>>> +#define LOONGSON_GPIO_INTPOL         0x10
>>> +#define LOONGSON_GPIO_INTEN          0x14
>>> +
>>> +/* The gpio interrupt is a watchdog interrupt that is triggered when the BMC resets. */
>>> +static int ls2kbmc_gpio_reset_handler(struct ls2kbmc_pdata *priv)
>>> +{
>>> +     int irq, ret = 0;
>>> +     int gsi = 16 + (BMC_RESET_GPIO & 7);
>>> +     void __iomem *gpio_base;
>>> +
>>> +     /* Since Loongson-3A hardware does not support GPIO interrupt cascade,
>>> +      * chip->gpio_to_irq() cannot be implemented,
>>> +      * here acpi_register_gsi() is used to get gpio irq.
>>> +      */
>>> +     irq = acpi_register_gsi(NULL, gsi, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW);
>>> +     if (irq < 0)
>>> +             return irq;
>>> +
>>> +     gpio_base = ioremap(LOONGSON_GPIO_REG_BASE, LOONGSON_GPIO_REG_SIZE);
>>> +     if (!gpio_base) {
>>> +             acpi_unregister_gsi(gsi);
>>> +             return PTR_ERR(gpio_base);
>>> +     }
>>> +
>>> +     writel(readl(gpio_base + LOONGSON_GPIO_OEN) | BIT(BMC_RESET_GPIO),
>>> +            gpio_base + LOONGSON_GPIO_OEN);
>>> +     writel(readl(gpio_base + LOONGSON_GPIO_FUNC) & ~BIT(BMC_RESET_GPIO),
>>> +            gpio_base + LOONGSON_GPIO_FUNC);
>>> +     writel(readl(gpio_base + LOONGSON_GPIO_INTPOL) & ~BIT(BMC_RESET_GPIO),
>>> +            gpio_base + LOONGSON_GPIO_INTPOL);
>>> +     writel(readl(gpio_base + LOONGSON_GPIO_INTEN) | BIT(BMC_RESET_GPIO),
>>> +            gpio_base + LOONGSON_GPIO_INTEN);
>>> +
>>> +     ret = request_irq(irq, ls2kbmc_interrupt, IRQF_SHARED | IRQF_TRIGGER_FALLING,
>>> +                       "ls2kbmc gpio", priv);
>>> +
>>> +     acpi_unregister_gsi(gsi);
>>> +     iounmap(gpio_base);
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +static int ls2kbmc_pdata_initial(struct platform_device *pdev, struct ls2kbmc_pdata *priv)
>>> +{
>>> +     int ret;
>>> +
>>> +     priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
>>> +
>>> +     ls2kbmc_save_pci_data(priv);
>>> +
>>> +     INIT_WORK(&priv->bmc_work, ls2kbmc_events_fn);
>>> +
>>> +     ret = request_irq(priv->pdev->irq, ls2kbmc_interrupt,
>>> +                       IRQF_SHARED | IRQF_TRIGGER_RISING, "ls2kbmc pcie", priv);
>>> +     if (ret) {
>>> +             pr_err("request_irq(%d) failed\n", priv->pdev->irq);
>>> +             return ret;
>>> +     }
>>> +
>>> +     return ls2kbmc_gpio_reset_handler(priv);
>>> +}
>>> +
>>>    /*
>>>     * Platform driver
>>>     */
>>> @@ -598,12 +877,15 @@ static int ls2kbmc_probe(struct platform_device *pdev)
>>>        if (IS_ERR(priv))
>>>                return -ENOMEM;
>>>
>>> -     priv->pdev = *(struct pci_dev **)dev_get_platdata(&pdev->dev);
>>> +     ret = ls2kbmc_pdata_initial(pdev, priv);
>>> +     if (ret)
>>> +             return ret;
>>>
>>>        sdev = ls2kbmc_device_create(&ls2kbmc_driver, pdev, priv);
>>>        if (IS_ERR(sdev))
>>>                return PTR_ERR(sdev);
>>>        dev = &sdev->dev;
>>> +     priv->ddev = &sdev->dev;
>>>
>>>        ret = drm_dev_register(dev, 0);
>>>        if (ret)
>> --
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>>
>
> --
> Thanks.
> Binbin

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2025-03-31  7:53 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-30  9:31 [PATCH v1 0/4] LoongArch: Add Loongson-2K0500 BMC support Binbin Zhou
2024-12-30  9:31 ` [PATCH v1 1/4] mfd: ls2kbmc: Introduce Loongson-2K BMC MFD Core driver Binbin Zhou
2024-12-30  9:31 ` [PATCH v1 2/4] ipmi: Add Loongson-2K BMC support Binbin Zhou
2024-12-31 12:37   ` kernel test robot
2025-01-03  4:29   ` kernel test robot
2024-12-30  9:31 ` [PATCH v1 3/4] drm/ls2kbmc: Add support for Loongson-2K BMC display Binbin Zhou
2025-01-02  9:07   ` Thomas Zimmermann
2025-01-02 12:55     ` Binbin Zhou
2025-01-02 13:32       ` Thomas Zimmermann
2025-01-06  1:56         ` Binbin Zhou
2025-01-06  7:03         ` Binbin Zhou
2025-01-06 14:10           ` Thomas Zimmermann
2025-01-09 12:56             ` Binbin Zhou
2025-01-15  8:48   ` Thomas Zimmermann
2024-12-30  9:31 ` [PATCH v1 4/4] drm/ls2kbmc: Add Loongson-2K BMC reset function support Binbin Zhou
2025-01-15  8:57   ` Thomas Zimmermann
2025-02-11 11:27     ` Binbin Zhou
2025-03-25 12:39       ` Binbin Zhou
2025-03-31  7:53       ` Thomas Zimmermann

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