diff mbox series

[v2,3/3] drm/loongson: Add dummy gpu driver as a subcomponent

Message ID 20240526195826.109008-4-sui.jingfeng@linux.dev (mailing list archive)
State New, archived
Headers show
Series drm/loongson: Introduce component framework support | expand

Commit Message

Sui Jingfeng May 26, 2024, 7:58 p.m. UTC
Loongson Graphics are PCIe multi-functional devices, the GPU device and
the display controller are two distinct devices. Drivers of them should
loose coupling, but still be able to works togather to provide a unified
service to userspace.

Add a dummy driver for the GPU, it functional as a subcomponent as well.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/loongson/Makefile            |  3 +
 drivers/gpu/drm/loongson/loong_gpu_pci_drv.c | 90 ++++++++++++++++++++
 drivers/gpu/drm/loongson/loong_gpu_pci_drv.h | 27 ++++++
 drivers/gpu/drm/loongson/loongson_module.c   |  9 ++
 drivers/gpu/drm/loongson/loongson_module.h   |  7 ++
 drivers/gpu/drm/loongson/lsdc_drv.c          | 12 ++-
 drivers/gpu/drm/loongson/lsdc_drv.h          |  8 +-
 7 files changed, 149 insertions(+), 7 deletions(-)
 create mode 100644 drivers/gpu/drm/loongson/loong_gpu_pci_drv.c
 create mode 100644 drivers/gpu/drm/loongson/loong_gpu_pci_drv.h

Comments

Markus Elfring May 27, 2024, 9:45 a.m. UTC | #1
> loose coupling, but still be able to works togather to provide a unified

  use loose?          should?          work together?            an?


…
> Add a dummy driver for the GPU, it functional as a subcomponent as well.

                                     is?


Please improve your change descriptions considerably.


…
> +++ b/drivers/gpu/drm/loongson/loongson_module.c
> @@ -29,8 +29,15 @@ static int __init loongson_module_init(void)
>  	if (ret)
>  		return ret;
>
> +	ret = pci_register_driver(&loong_gpu_pci_driver);
> +	if (ret) {
> +		platform_driver_unregister(&lsdc_output_port_platform_driver);
> +		return ret;
> +	}
> +
>  	ret = pci_register_driver(&lsdc_pci_driver);
>  	if (ret) {
> +		pci_unregister_driver(&loong_gpu_pci_driver);
>  		platform_driver_unregister(&lsdc_output_port_platform_driver);
>  		return ret;
>  	}

How do you think about to use another goto chain for common exception handling?
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources

Would you become interested in the application of scope-based resource management here?


> @@ -43,6 +50,8 @@ static void __exit loongson_module_exit(void)
>  {
>  	pci_unregister_driver(&lsdc_pci_driver);
>
> +	pci_unregister_driver(&loong_gpu_pci_driver);
> +
>  	platform_driver_unregister(&lsdc_output_port_platform_driver);
>  }

I suggest to avoid blank lines for this function implementation.

Regards,
Markus
diff mbox series

Patch

diff --git a/drivers/gpu/drm/loongson/Makefile b/drivers/gpu/drm/loongson/Makefile
index e15cb9bff378..4f4c1c42bbba 100644
--- a/drivers/gpu/drm/loongson/Makefile
+++ b/drivers/gpu/drm/loongson/Makefile
@@ -17,6 +17,9 @@  loongson-y := \
 	lsdc_probe.o \
 	lsdc_ttm.o
 
+loongson-y += \
+	loong_gpu_pci_drv.o
+
 loongson-y += loongson_device.o \
 	      loongson_module.o
 
diff --git a/drivers/gpu/drm/loongson/loong_gpu_pci_drv.c b/drivers/gpu/drm/loongson/loong_gpu_pci_drv.c
new file mode 100644
index 000000000000..4ae6a5807d1d
--- /dev/null
+++ b/drivers/gpu/drm/loongson/loong_gpu_pci_drv.c
@@ -0,0 +1,90 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/component.h>
+#include <linux/pci.h>
+
+#include <drm/drm_device.h>
+#include <drm/drm_print.h>
+
+#include "loongson_module.h"
+#include "loong_gpu_pci_drv.h"
+
+static int loong_gpu_bind(struct device *dev, struct device *master, void *data)
+{
+	struct drm_device *drm = data;
+	struct loong_gpu_device *gpu;
+	u32 hw_info;
+	u8 host_id;
+	u8 revision;
+
+	gpu = devm_kzalloc(dev, sizeof(*gpu), GFP_KERNEL);
+	if (!gpu)
+		return -ENOMEM;
+
+	gpu->reg_base = pcim_iomap(to_pci_dev(dev), 0, 0);
+	if (!gpu->reg_base)
+		return -ENOMEM;
+
+	hw_info = loong_rreg32(gpu, 0x8C);
+
+	gpu->ver_major = (hw_info >> 8) * 0x0F;
+	gpu->ver_minor = (hw_info & 0xF0) >> 4;
+	revision = hw_info & 0x0F;
+	host_id = (hw_info >> 16) & 0xFF;
+
+	drm_info(drm, "Found LoongGPU: LG%x%x0, revision: %x, Host: %s\n",
+		 gpu->ver_major, gpu->ver_minor, revision,
+		 host_id ? "LS2K2000" : "LS7A2000");
+
+	dev_set_drvdata(dev, gpu);
+
+	return 0;
+}
+
+static void loong_gpu_unbind(struct device *dev, struct device *master, void *data)
+{
+	struct loong_gpu_device *gpu = dev_get_drvdata(dev);
+
+	if (gpu) {
+		pcim_iounmap(to_pci_dev(dev), gpu->reg_base);
+		devm_kfree(dev, gpu);
+	}
+}
+
+static const struct component_ops loong_gpu_component_ops = {
+	.bind = loong_gpu_bind,
+	.unbind = loong_gpu_unbind,
+};
+
+static int loong_gpu_pci_probe(struct pci_dev *pdev,
+			       const struct pci_device_id *ent)
+{
+	int ret;
+
+	ret = pcim_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	pci_set_master(pdev);
+
+	return component_add(&pdev->dev, &loong_gpu_component_ops);
+}
+
+static void loong_gpu_pci_remove(struct pci_dev *pdev)
+{
+	component_del(&pdev->dev, &loong_gpu_component_ops);
+}
+
+static const struct pci_device_id loong_gpu_pci_id_list[] = {
+	{PCI_VDEVICE(LOONGSON, 0x7a25), CHIP_LS7A2000},
+	{ },
+};
+
+struct pci_driver loong_gpu_pci_driver = {
+	.name = "loong",
+	.id_table = loong_gpu_pci_id_list,
+	.probe = loong_gpu_pci_probe,
+	.remove = loong_gpu_pci_remove,
+};
+
+MODULE_DEVICE_TABLE(pci, loong_gpu_pci_id_list);
diff --git a/drivers/gpu/drm/loongson/loong_gpu_pci_drv.h b/drivers/gpu/drm/loongson/loong_gpu_pci_drv.h
new file mode 100644
index 000000000000..f620820ab263
--- /dev/null
+++ b/drivers/gpu/drm/loongson/loong_gpu_pci_drv.h
@@ -0,0 +1,27 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef __LOONG_GPU_PCI_DRV_H__
+#define __LOONG_GPU_PCI_DRV_H__
+
+#include <linux/pci.h>
+
+struct loong_gpu_device {
+	struct pci_dev *pdev;
+	void __iomem *reg_base;
+
+	u32 ver_major;
+	u32 ver_minor;
+	u32 revision;
+};
+
+static inline u32 loong_rreg32(struct loong_gpu_device *ldev, u32 offset)
+{
+	return readl(ldev->reg_base + offset);
+}
+
+static inline void loong_wreg32(struct loong_gpu_device *ldev, u32 offset, u32 val)
+{
+	writel(val, ldev->reg_base + offset);
+}
+
+#endif
diff --git a/drivers/gpu/drm/loongson/loongson_module.c b/drivers/gpu/drm/loongson/loongson_module.c
index 037fa7ffe9c9..d4c0d5cec856 100644
--- a/drivers/gpu/drm/loongson/loongson_module.c
+++ b/drivers/gpu/drm/loongson/loongson_module.c
@@ -29,8 +29,15 @@  static int __init loongson_module_init(void)
 	if (ret)
 		return ret;
 
+	ret = pci_register_driver(&loong_gpu_pci_driver);
+	if (ret) {
+		platform_driver_unregister(&lsdc_output_port_platform_driver);
+		return ret;
+	}
+
 	ret = pci_register_driver(&lsdc_pci_driver);
 	if (ret) {
+		pci_unregister_driver(&loong_gpu_pci_driver);
 		platform_driver_unregister(&lsdc_output_port_platform_driver);
 		return ret;
 	}
@@ -43,6 +50,8 @@  static void __exit loongson_module_exit(void)
 {
 	pci_unregister_driver(&lsdc_pci_driver);
 
+	pci_unregister_driver(&loong_gpu_pci_driver);
+
 	platform_driver_unregister(&lsdc_output_port_platform_driver);
 }
 module_exit(loongson_module_exit);
diff --git a/drivers/gpu/drm/loongson/loongson_module.h b/drivers/gpu/drm/loongson/loongson_module.h
index 8dc71b98f5cc..ac4ff8ea50ca 100644
--- a/drivers/gpu/drm/loongson/loongson_module.h
+++ b/drivers/gpu/drm/loongson/loongson_module.h
@@ -6,8 +6,15 @@ 
 #ifndef __LOONGSON_MODULE_H__
 #define __LOONGSON_MODULE_H__
 
+enum loongson_chip_id {
+	CHIP_LS7A1000 = 0,
+	CHIP_LS7A2000 = 1,
+	CHIP_LS_LAST,
+};
+
 extern int loongson_vblank;
 extern struct pci_driver lsdc_pci_driver;
+extern struct pci_driver loong_gpu_pci_driver;
 extern struct platform_driver lsdc_output_port_platform_driver;
 
 #endif
diff --git a/drivers/gpu/drm/loongson/lsdc_drv.c b/drivers/gpu/drm/loongson/lsdc_drv.c
index 02429c95bd1a..ab258de6a264 100644
--- a/drivers/gpu/drm/loongson/lsdc_drv.c
+++ b/drivers/gpu/drm/loongson/lsdc_drv.c
@@ -154,9 +154,10 @@  static int lsdc_get_dedicated_vram(struct lsdc_device *ldev,
 	base = pci_resource_start(pdev_gpu, 2);
 	size = pci_resource_len(pdev_gpu, 2);
 
+	pci_dev_put(pdev_gpu);
+
 	ldev->vram_base = base;
 	ldev->vram_size = size;
-	ldev->gpu = pdev_gpu;
 
 	drm_info(ddev, "Dedicated vram start: 0x%llx, size: %uMiB\n",
 		 (u64)base, (u32)(size >> 20));
@@ -281,6 +282,7 @@  static const struct component_master_ops loongson_drm_master_ops = {
 
 static int lsdc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
+	struct pci_dev *gpu = NULL;
 	struct component_match *matches = NULL;
 	const struct lsdc_desc *descp;
 	struct lsdc_device *ldev;
@@ -339,6 +341,14 @@  static int lsdc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 				    &ldev->child[i]->dev);
 	}
 
+	gpu = pci_get_device(PCI_VENDOR_ID_LOONGSON, 0x7a25, NULL);
+	if (gpu) {
+		component_match_add(&pdev->dev, &matches,
+				    component_compare_dev,
+				    &gpu->dev);
+		pci_dev_put(gpu);
+	}
+
 	ret = component_master_add_with_match(&pdev->dev,
 					      &loongson_drm_master_ops,
 					      matches);
diff --git a/drivers/gpu/drm/loongson/lsdc_drv.h b/drivers/gpu/drm/loongson/lsdc_drv.h
index 267fcba74572..770c7819caa2 100644
--- a/drivers/gpu/drm/loongson/lsdc_drv.h
+++ b/drivers/gpu/drm/loongson/lsdc_drv.h
@@ -16,6 +16,8 @@ 
 #include <drm/drm_plane.h>
 #include <drm/ttm/ttm_device.h>
 
+#include "loongson_module.h"
+
 #include "lsdc_i2c.h"
 #include "lsdc_irq.h"
 #include "lsdc_gfxpll.h"
@@ -38,12 +40,6 @@ 
  * display pipe 1 = crtc1 + dvo1 + encoder1 + connectro1 + cursor1 + primary1
  */
 
-enum loongson_chip_id {
-	CHIP_LS7A1000 = 0,
-	CHIP_LS7A2000 = 1,
-	CHIP_LS_LAST,
-};
-
 const struct lsdc_desc *
 lsdc_device_probe(struct pci_dev *pdev, enum loongson_chip_id chip);