Introduce component framework to bind child and sibling devices, for better
modularity and offload the deferral probe issue to submodule if it need to
attach exterinal module someday. Also for better reflect the hardware
layout.
Hardware units that come with PCIe are all ready to drive, but there are
some board specific modules will return -EPROBE_DEFER to us. We need all
submodules ready to use before we can register the drm device to userspace.
The idea is to device the exterinal module dependent part and exterinal
module independent part. For example, the display controller and the
GPIO-I2C just belong to exterinal module independent part. While the
outputs are just belong to exterinal module dependent part.
We abstract the output ports as child devices, the output ports may
consists of encoder phy and level shifter. Well, the GPU are standalone
siblings relative to the DC. Those units are relatively separated
hardware units from display controller itself.
By design, the display controller PCI(e) is selected as master, gpio-i2c
go with master. Manually created virtual subdevice functional as agents
for the master, it could return the -EPROBE_DEFER back to the drvier core.
This allows the master don't have to tear down everything, thereore
majority setups work can be preserved. The potential cyclic dependency
problem can be solved then.
v1 -> v2:
* Squash patch 0002 and patch 0003 into one
* Fill type and improve commit message
Sui Jingfeng (3):
drm/loongson: Add a helper for creating child devices
drm/loongson: Introduce component framework support
drm/loongson: Add dummy gpu driver as a subcomponent
drivers/gpu/drm/loongson/Makefile | 4 +
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_device.c | 42 ++++
drivers/gpu/drm/loongson/loongson_module.c | 26 ++-
drivers/gpu/drm/loongson/loongson_module.h | 8 +
drivers/gpu/drm/loongson/lsdc_drv.c | 217 +++++++++++-------
drivers/gpu/drm/loongson/lsdc_drv.h | 45 +---
drivers/gpu/drm/loongson/lsdc_i2c.c | 5 +-
drivers/gpu/drm/loongson/lsdc_i2c.h | 3 -
drivers/gpu/drm/loongson/lsdc_output.c | 176 ++++++++++++++
drivers/gpu/drm/loongson/lsdc_output.h | 38 ++-
drivers/gpu/drm/loongson/lsdc_output_7a1000.c | 3 +-
drivers/gpu/drm/loongson/lsdc_output_7a2000.c | 17 +-
14 files changed, 564 insertions(+), 137 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
create mode 100644 drivers/gpu/drm/loongson/lsdc_output.c
--
2.34.1
In some display subsystems, the functionality of a PCIe device may too
complex to be managed by a single driver. A split of the functionality
into child devices can helpful to achieve better modularity.
Another benefit is that we could migrate the dependency on exterinal
modules to a submodule level with the helper created. For example, it's
not uncommon that some exterinal module will return -EPROBE_DEFER to our
driver during probe time. KMS driver has to tear down everything when it
receives -EPROBE_DEFER, the problem is that it's completely not necessary
and rising cyclic dependency problems if not process correctly.
Add the loongson_create_platform_device() function, which allows the KMS
driver to create sub-devices for it. The manually created decice acts as
agents for the principal part, migrate the potential issue to submodule.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/loongson/loongson_device.c | 42 ++++++++++++++++++++++
drivers/gpu/drm/loongson/lsdc_drv.h | 6 ++++
2 files changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/loongson/loongson_device.c b/drivers/gpu/drm/loongson/loongson_device.c
index 9986c8a2a255..b268549d643e 100644
--- a/drivers/gpu/drm/loongson/loongson_device.c
+++ b/drivers/gpu/drm/loongson/loongson_device.c
@@ -4,6 +4,7 @@
*/
#include <linux/pci.h>
+#include <linux/platform_device.h>
#include "lsdc_drv.h"
@@ -100,3 +101,44 @@ lsdc_device_probe(struct pci_dev *pdev, enum loongson_chip_id chip_id)
{
return __chip_id_desc_table[chip_id];
}
+
+int loongson_create_platform_device(struct device *parent,
+ const char *name, int id,
+ struct resource *pres,
+ void *data,
+ struct platform_device **ppdev)
+{
+ struct platform_device *pdev;
+ int ret;
+
+ pdev = platform_device_alloc(name, id);
+ if (!pdev)
+ return -ENOMEM;
+
+ pdev->dev.parent = parent;
+
+ if (pres) {
+ ret = platform_device_add_resources(pdev, pres, 1);
+ if (ret) {
+ platform_device_put(pdev);
+ return ret;
+ }
+ }
+
+ if (data) {
+ void *pdata = kmalloc(sizeof(void *), GFP_KERNEL);
+
+ *(void **)pdata = data;
+ pdev->dev.platform_data = pdata;
+ }
+
+ ret = platform_device_add(pdev);
+ if (ret) {
+ platform_device_put(pdev);
+ return ret;
+ }
+
+ *ppdev = pdev;
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/loongson/lsdc_drv.h b/drivers/gpu/drm/loongson/lsdc_drv.h
index fbf2d760ef27..a2c6b496a69f 100644
--- a/drivers/gpu/drm/loongson/lsdc_drv.h
+++ b/drivers/gpu/drm/loongson/lsdc_drv.h
@@ -47,6 +47,12 @@ enum loongson_chip_id {
const struct lsdc_desc *
lsdc_device_probe(struct pci_dev *pdev, enum loongson_chip_id chip);
+int loongson_create_platform_device(struct device *parent,
+ const char *name, int id,
+ struct resource *pres,
+ void *data,
+ struct platform_device **ppdev);
+
struct lsdc_kms_funcs;
/* DC specific */
--
2.34.1
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 <[email protected]>
---
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
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);
--
2.34.1
Hardware units come with PCIe are actually all ready to be driven, but
there has some board specific modules could return '-EPROBE_DEFER'.
However, the driver needs all of the subcompoments ready to use before
it can register the drm service to userspace.
Introduce the component framework to tackle such problems, move DRM
device related code into loongson_drm_master_bind() function. Move
output related things into subdriver. Display controller and GPIO-I2C
goes with the PCIe master, sinch they has no dependency on exterinal
modules.
While the outputs drivers, such as encoders and conectors, may has some
dependency on exterinal modules. Those hardware units are relatively
independent hardware IPs from the CRTC. Hence, offload them to submodules.
This design allows subdriver return '-EPROBE_DEFER' to the driver core
if it need to do so, the master drvier won't bind until all submodules
are ready.
Signed-off-by: Sui Jingfeng <[email protected]>
---
drivers/gpu/drm/loongson/Makefile | 1 +
drivers/gpu/drm/loongson/loongson_module.c | 17 +-
drivers/gpu/drm/loongson/loongson_module.h | 1 +
drivers/gpu/drm/loongson/lsdc_drv.c | 205 +++++++++++-------
drivers/gpu/drm/loongson/lsdc_drv.h | 31 +--
drivers/gpu/drm/loongson/lsdc_i2c.c | 5 +-
drivers/gpu/drm/loongson/lsdc_i2c.h | 3 -
drivers/gpu/drm/loongson/lsdc_output.c | 176 +++++++++++++++
drivers/gpu/drm/loongson/lsdc_output.h | 38 +++-
drivers/gpu/drm/loongson/lsdc_output_7a1000.c | 3 +-
drivers/gpu/drm/loongson/lsdc_output_7a2000.c | 17 +-
11 files changed, 367 insertions(+), 130 deletions(-)
create mode 100644 drivers/gpu/drm/loongson/lsdc_output.c
diff --git a/drivers/gpu/drm/loongson/Makefile b/drivers/gpu/drm/loongson/Makefile
index 91e72bd900c1..e15cb9bff378 100644
--- a/drivers/gpu/drm/loongson/Makefile
+++ b/drivers/gpu/drm/loongson/Makefile
@@ -9,6 +9,7 @@ loongson-y := \
lsdc_gfxpll.o \
lsdc_i2c.o \
lsdc_irq.o \
+ lsdc_output.o \
lsdc_output_7a1000.o \
lsdc_output_7a2000.o \
lsdc_plane.o \
diff --git a/drivers/gpu/drm/loongson/loongson_module.c b/drivers/gpu/drm/loongson/loongson_module.c
index d2a51bd395f6..037fa7ffe9c9 100644
--- a/drivers/gpu/drm/loongson/loongson_module.c
+++ b/drivers/gpu/drm/loongson/loongson_module.c
@@ -4,6 +4,7 @@
*/
#include <linux/pci.h>
+#include <linux/platform_device.h>
#include <video/nomodeset.h>
@@ -19,15 +20,29 @@ module_param_named(vblank, loongson_vblank, int, 0400);
static int __init loongson_module_init(void)
{
+ int ret;
+
if (!loongson_modeset || video_firmware_drivers_only())
return -ENODEV;
- return pci_register_driver(&lsdc_pci_driver);
+ ret = platform_driver_register(&lsdc_output_port_platform_driver);
+ if (ret)
+ return ret;
+
+ ret = pci_register_driver(&lsdc_pci_driver);
+ if (ret) {
+ platform_driver_unregister(&lsdc_output_port_platform_driver);
+ return ret;
+ }
+
+ return 0;
}
module_init(loongson_module_init);
static void __exit loongson_module_exit(void)
{
pci_unregister_driver(&lsdc_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 931c17521bf0..8dc71b98f5cc 100644
--- a/drivers/gpu/drm/loongson/loongson_module.h
+++ b/drivers/gpu/drm/loongson/loongson_module.h
@@ -8,5 +8,6 @@
extern int loongson_vblank;
extern struct pci_driver lsdc_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 adc7344d2f80..02429c95bd1a 100644
--- a/drivers/gpu/drm/loongson/lsdc_drv.c
+++ b/drivers/gpu/drm/loongson/lsdc_drv.c
@@ -3,7 +3,9 @@
* Copyright (C) 2023 Loongson Technology Corporation Limited
*/
+#include <linux/component.h>
#include <linux/pci.h>
+#include <linux/platform_device.h>
#include <linux/vgaarb.h>
#include <drm/drm_aperture.h>
@@ -67,31 +69,6 @@ static int lsdc_modeset_init(struct lsdc_device *ldev,
unsigned int i;
int ret;
- for (i = 0; i < num_crtc; i++) {
- dispipe = &ldev->dispipe[i];
-
- /* We need an index before crtc is initialized */
- dispipe->index = i;
-
- ret = funcs->create_i2c(ddev, dispipe, i);
- if (ret)
- return ret;
- }
-
- for (i = 0; i < num_crtc; i++) {
- struct i2c_adapter *ddc = NULL;
-
- dispipe = &ldev->dispipe[i];
- if (dispipe->li2c)
- ddc = &dispipe->li2c->adapter;
-
- ret = funcs->output_init(ddev, dispipe, ddc, i);
- if (ret)
- return ret;
-
- ldev->num_output++;
- }
-
for (i = 0; i < num_crtc; i++) {
dispipe = &ldev->dispipe[i];
@@ -187,30 +164,17 @@ static int lsdc_get_dedicated_vram(struct lsdc_device *ldev,
return (size > SZ_1M) ? 0 : -ENODEV;
}
-static struct lsdc_device *
-lsdc_create_device(struct pci_dev *pdev,
- const struct lsdc_desc *descp,
- const struct drm_driver *driver)
+static int lsdc_device_init(struct lsdc_device *ldev,
+ const struct lsdc_desc *descp,
+ const struct drm_driver *driver)
{
- struct lsdc_device *ldev;
- struct drm_device *ddev;
+ struct drm_device *ddev = &ldev->base;
int ret;
- ldev = devm_drm_dev_alloc(&pdev->dev, driver, struct lsdc_device, base);
- if (IS_ERR(ldev))
- return ldev;
-
- ldev->dc = pdev;
- ldev->descp = descp;
-
- ddev = &ldev->base;
-
- loongson_gfxpll_create(ddev, &ldev->gfxpll);
-
- ret = lsdc_get_dedicated_vram(ldev, pdev, descp);
+ ret = lsdc_get_dedicated_vram(ldev, ldev->dc, ldev->descp);
if (ret) {
drm_err(ddev, "Init VRAM failed: %d\n", ret);
- return ERR_PTR(ret);
+ return ret;
}
ret = drm_aperture_remove_conflicting_framebuffers(ldev->vram_base,
@@ -218,51 +182,110 @@ lsdc_create_device(struct pci_dev *pdev,
driver);
if (ret) {
drm_err(ddev, "Remove firmware framebuffers failed: %d\n", ret);
- return ERR_PTR(ret);
+ return ret;
}
ret = lsdc_ttm_init(ldev);
if (ret) {
drm_err(ddev, "Memory manager init failed: %d\n", ret);
- return ERR_PTR(ret);
+ return ret;
}
lsdc_gem_init(ddev);
/* Bar 0 of the DC device contains the MMIO register's base address */
- ldev->reg_base = pcim_iomap(pdev, 0, 0);
+ ldev->reg_base = pcim_iomap(ldev->dc, 0, 0);
if (!ldev->reg_base)
- return ERR_PTR(-ENODEV);
+ return -ENODEV;
spin_lock_init(&ldev->reglock);
+ return 0;
+}
+
+/* For multiple GPU driver instance co-exixt in the system */
+
+static unsigned int lsdc_vga_set_decode(struct pci_dev *pdev, bool state)
+{
+ return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
+}
+
+static int loongson_drm_master_bind(struct device *dev)
+{
+ struct lsdc_device *ldev = dev_get_drvdata(dev);
+ const struct lsdc_desc *descp = ldev->descp;
+ struct drm_device *ddev = &ldev->base;
+ int ret;
+
+ if (loongson_vblank) {
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ ret = drm_vblank_init(ddev, descp->num_of_crtc);
+ if (ret)
+ return ret;
+
+ ret = devm_request_irq(dev, pdev->irq,
+ descp->funcs->irq_handler,
+ IRQF_SHARED,
+ dev_name(dev), ddev);
+ if (ret) {
+ drm_err(ddev, "Failed to register interrupt: %d\n", ret);
+ return ret;
+ }
+
+ drm_info(ddev, "registered irq: %u\n", pdev->irq);
+ }
+
ret = lsdc_mode_config_init(ddev, descp);
if (ret)
- return ERR_PTR(ret);
+ return ret;
+
+ ret = component_bind_all(dev, ddev);
+ if (ret) {
+ dev_err(dev, "master bind all failed: %d\n", ret);
+ return ret;
+ }
ret = lsdc_modeset_init(ldev, descp->num_of_crtc, descp->funcs,
loongson_vblank);
if (ret)
- return ERR_PTR(ret);
+ return ret;
drm_mode_config_reset(ddev);
- return ldev;
-}
+ drm_kms_helper_poll_init(ddev);
-/* For multiple GPU driver instance co-exixt in the system */
+ ret = drm_dev_register(ddev, 0);
+ if (ret)
+ return ret;
-static unsigned int lsdc_vga_set_decode(struct pci_dev *pdev, bool state)
+ drm_fbdev_ttm_setup(ddev, 32);
+
+ return 0;
+}
+
+static void loongson_drm_master_unbind(struct device *dev)
{
- return VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
+ struct lsdc_device *ldev = dev_get_drvdata(dev);
+ struct drm_device *ddev = &ldev->base;
+
+ component_unbind_all(dev, ddev);
+
+ return;
}
+static const struct component_master_ops loongson_drm_master_ops = {
+ .bind = loongson_drm_master_bind,
+ .unbind = loongson_drm_master_unbind,
+};
+
static int lsdc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
+ struct component_match *matches = NULL;
const struct lsdc_desc *descp;
- struct drm_device *ddev;
struct lsdc_device *ldev;
int ret;
+ int i;
descp = lsdc_device_probe(pdev, ent->driver_data);
if (IS_ERR_OR_NULL(descp))
@@ -281,55 +304,77 @@ static int lsdc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
dev_info(&pdev->dev, "Found %s, revision: %u",
to_loongson_gfx(descp)->model, pdev->revision);
- ldev = lsdc_create_device(pdev, descp, &lsdc_drm_driver);
+ ldev = devm_drm_dev_alloc(&pdev->dev, &lsdc_drm_driver,
+ struct lsdc_device, base);
if (IS_ERR(ldev))
return PTR_ERR(ldev);
- ddev = &ldev->base;
+ pci_set_drvdata(pdev, ldev);
- pci_set_drvdata(pdev, ddev);
+ ldev->dc = pdev;
+ ldev->descp = descp;
- vga_client_register(pdev, lsdc_vga_set_decode);
+ loongson_gfxpll_create(&ldev->base, &ldev->gfxpll);
- drm_kms_helper_poll_init(ddev);
+ ret = lsdc_device_init(ldev, descp, &lsdc_drm_driver);
+ if (ret)
+ return ret;
- if (loongson_vblank) {
- ret = drm_vblank_init(ddev, descp->num_of_crtc);
+ for (i = 0; i < descp->num_of_crtc; ++i) {
+ ret = descp->funcs->create_i2c(&ldev->base, i);
if (ret)
return ret;
+ }
- ret = devm_request_irq(&pdev->dev, pdev->irq,
- descp->funcs->irq_handler,
- IRQF_SHARED,
- dev_name(&pdev->dev), ddev);
- if (ret) {
- drm_err(ddev, "Failed to register interrupt: %d\n", ret);
+ for (i = 0; i < descp->num_of_crtc; ++i) {
+ ret = lsdc_output_preinit(&pdev->dev, descp, i,
+ &ldev->child[i]);
+ if (ret)
return ret;
- }
+ }
- drm_info(ddev, "registered irq: %u\n", pdev->irq);
+ for (i = 0; i < descp->num_of_crtc; ++i) {
+ component_match_add(&pdev->dev, &matches,
+ component_compare_dev,
+ &ldev->child[i]->dev);
}
- ret = drm_dev_register(ddev, 0);
- if (ret)
- return ret;
+ ret = component_master_add_with_match(&pdev->dev,
+ &loongson_drm_master_ops,
+ matches);
- drm_fbdev_ttm_setup(ddev, 32);
+ dev_info(&pdev->dev, "loongson add component: %u\n", ret);
+
+ vga_client_register(pdev, lsdc_vga_set_decode);
return 0;
}
static void lsdc_pci_remove(struct pci_dev *pdev)
{
- struct drm_device *ddev = pci_get_drvdata(pdev);
+ struct lsdc_device *ldev = pci_get_drvdata(pdev);
+ struct drm_device *ddev = &ldev->base;
+ unsigned int i;
drm_dev_unregister(ddev);
drm_atomic_helper_shutdown(ddev);
+
+ component_master_del(&pdev->dev, &loongson_drm_master_ops);
+
+ for (i = 0; i < ldev->descp->num_of_crtc; ++i) {
+ if (ldev->child[i]) {
+ platform_device_unregister(ldev->child[i]);
+ ldev->child[i] = NULL;
+ }
+ }
}
static void lsdc_pci_shutdown(struct pci_dev *pdev)
{
- drm_atomic_helper_shutdown(pci_get_drvdata(pdev));
+ struct lsdc_device *ldev = pci_get_drvdata(pdev);
+ struct drm_device *ddev = &ldev->base;
+
+ drm_atomic_helper_shutdown(ddev);
}
static int lsdc_drm_freeze(struct drm_device *ddev)
@@ -383,7 +428,8 @@ static int lsdc_drm_freeze(struct drm_device *ddev)
static int lsdc_drm_resume(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
- struct drm_device *ddev = pci_get_drvdata(pdev);
+ struct lsdc_device *ldev = pci_get_drvdata(pdev);
+ struct drm_device *ddev = &ldev->base;
return drm_mode_config_helper_resume(ddev);
}
@@ -391,7 +437,8 @@ static int lsdc_drm_resume(struct device *dev)
static int lsdc_pm_freeze(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
- struct drm_device *ddev = pci_get_drvdata(pdev);
+ struct lsdc_device *ldev = pci_get_drvdata(pdev);
+ struct drm_device *ddev = &ldev->base;
return lsdc_drm_freeze(ddev);
}
diff --git a/drivers/gpu/drm/loongson/lsdc_drv.h b/drivers/gpu/drm/loongson/lsdc_drv.h
index a2c6b496a69f..267fcba74572 100644
--- a/drivers/gpu/drm/loongson/lsdc_drv.h
+++ b/drivers/gpu/drm/loongson/lsdc_drv.h
@@ -173,47 +173,22 @@ struct lsdc_cursor {
struct lsdc_device *ldev;
};
-struct lsdc_output {
- struct drm_encoder encoder;
- struct drm_connector connector;
-};
-
-static inline struct lsdc_output *
-connector_to_lsdc_output(struct drm_connector *connector)
-{
- return container_of(connector, struct lsdc_output, connector);
-}
-
-static inline struct lsdc_output *
-encoder_to_lsdc_output(struct drm_encoder *encoder)
-{
- return container_of(encoder, struct lsdc_output, encoder);
-}
-
struct lsdc_display_pipe {
struct lsdc_crtc crtc;
struct lsdc_primary primary;
struct lsdc_cursor cursor;
- struct lsdc_output output;
- struct lsdc_i2c *li2c;
+ struct lsdc_output *output;
unsigned int index;
};
-static inline struct lsdc_display_pipe *
-output_to_display_pipe(struct lsdc_output *output)
-{
- return container_of(output, struct lsdc_display_pipe, output);
-}
-
struct lsdc_kms_funcs {
irqreturn_t (*irq_handler)(int irq, void *arg);
int (*create_i2c)(struct drm_device *ddev,
- struct lsdc_display_pipe *dispipe,
unsigned int index);
int (*output_init)(struct drm_device *ddev,
- struct lsdc_display_pipe *dispipe,
+ struct lsdc_output *output,
struct i2c_adapter *ddc,
unsigned int index);
@@ -290,6 +265,8 @@ struct lsdc_device {
resource_size_t gtt_size;
struct lsdc_display_pipe dispipe[LSDC_NUM_CRTC];
+ struct platform_device *child[LSDC_NUM_CRTC];
+ struct i2c_adapter *i2c[LSDC_NUM_CRTC];
struct lsdc_gem gem;
diff --git a/drivers/gpu/drm/loongson/lsdc_i2c.c b/drivers/gpu/drm/loongson/lsdc_i2c.c
index ce90c25536d2..f10416b856af 100644
--- a/drivers/gpu/drm/loongson/lsdc_i2c.c
+++ b/drivers/gpu/drm/loongson/lsdc_i2c.c
@@ -115,7 +115,6 @@ static void lsdc_destroy_i2c(struct drm_device *ddev, void *data)
* @index: output channel index, 0 for PIPE0, 1 for PIPE1
*/
int lsdc_create_i2c_chan(struct drm_device *ddev,
- struct lsdc_display_pipe *dispipe,
unsigned int index)
{
struct lsdc_device *ldev = to_lsdc(ddev);
@@ -127,8 +126,6 @@ int lsdc_create_i2c_chan(struct drm_device *ddev,
if (!li2c)
return -ENOMEM;
- dispipe->li2c = li2c;
-
if (index == 0) {
li2c->sda = 0x01; /* pin 0 */
li2c->scl = 0x02; /* pin 1 */
@@ -171,6 +168,8 @@ int lsdc_create_i2c_chan(struct drm_device *ddev,
if (ret)
return ret;
+ ldev->i2c[index] = adapter;
+
drm_info(ddev, "%s(sda pin mask=%u, scl pin mask=%u) created\n",
adapter->name, li2c->sda, li2c->scl);
diff --git a/drivers/gpu/drm/loongson/lsdc_i2c.h b/drivers/gpu/drm/loongson/lsdc_i2c.h
index 88cd1a1817a5..17636c09f7e7 100644
--- a/drivers/gpu/drm/loongson/lsdc_i2c.h
+++ b/drivers/gpu/drm/loongson/lsdc_i2c.h
@@ -20,10 +20,7 @@ struct lsdc_i2c {
u8 scl;
};
-struct lsdc_display_pipe;
-
int lsdc_create_i2c_chan(struct drm_device *ddev,
- struct lsdc_display_pipe *dispipe,
unsigned int index);
#endif
diff --git a/drivers/gpu/drm/loongson/lsdc_output.c b/drivers/gpu/drm/loongson/lsdc_output.c
new file mode 100644
index 000000000000..d447534e1150
--- /dev/null
+++ b/drivers/gpu/drm/loongson/lsdc_output.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/delay.h>
+#include <linux/component.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include "lsdc_drv.h"
+#include "lsdc_output.h"
+
+static struct lsdc_output_desc ls7a1000_output_port_desc[2] = {
+ {
+ .pipe = 0,
+ .type = "DVO-0",
+ },
+ {
+ .pipe = 1,
+ .type = "DVO-1",
+ },
+};
+
+static struct lsdc_output_desc ls7a2000_output_port_desc[2] = {
+ {
+ .pipe = 0,
+ .type = "HDMI-or-VGA-0",
+ },
+ {
+ .pipe = 1,
+ .type = "HDMI-1",
+ },
+};
+
+int lsdc_output_preinit(struct device *parent,
+ const struct lsdc_desc *descp,
+ unsigned int index,
+ struct platform_device **ppdev)
+{
+ struct lsdc_output_desc *output_port_info;
+ int ret;
+
+ switch (to_loongson_gfx(descp)->chip_id) {
+ case CHIP_LS7A1000:
+ output_port_info = &ls7a1000_output_port_desc[index];
+ break;
+ case CHIP_LS7A2000:
+ output_port_info = &ls7a2000_output_port_desc[index];
+ break;
+ default:
+ output_port_info = NULL;
+ break;
+ };
+
+ ret = loongson_create_platform_device(parent,
+ "lsdc-output-port",
+ index,
+ NULL,
+ (void *)output_port_info,
+ ppdev);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+/*
+ * @dev: pointer to the port@0, port@1, ..., port@n of the dispplay controller
+ * @master: pointer to the component master
+ * @data: pointer to the drm device control structure
+ */
+static int lsdc_output_port_bind(struct device *dev,
+ struct device *master,
+ void *data)
+{
+ struct drm_device *drm = data;
+ struct lsdc_device *ldev = to_lsdc(drm);
+ const struct lsdc_kms_funcs *kms_funcs = ldev->descp->funcs;
+ struct lsdc_output *output = dev_get_drvdata(dev);
+ struct lsdc_display_pipe *dispipe;
+ unsigned int pipe;
+ int ret;
+
+ if (!output->descp)
+ return -ENODEV;
+
+ pipe = output->descp->pipe;
+ dispipe = &ldev->dispipe[pipe];
+
+ ret = kms_funcs->output_init(drm, output, ldev->i2c[pipe], pipe);
+ if (ret)
+ return ret;
+
+ dispipe->output = output;
+
+ ldev->num_output++;
+
+ drm_info(drm, "Output port-%d bound, type: %s\n",
+ pipe, output->descp->type);
+
+ return 0;
+}
+
+static void lsdc_output_port_unbind(struct device *dev,
+ struct device *master,
+ void *data)
+{
+ struct drm_device *drm = data;
+ struct lsdc_device *ldev = to_lsdc(drm);
+ struct lsdc_output *output = dev_get_drvdata(dev);
+ unsigned int pipe;
+
+ pipe = output->descp->pipe;
+ ldev->dispipe[pipe].output = NULL;
+}
+
+static const struct component_ops lsdc_output_port_component_ops = {
+ .bind = lsdc_output_port_bind,
+ .unbind = lsdc_output_port_unbind,
+};
+
+static int lsdc_output_port_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct lsdc_output *output;
+ struct lsdc_output_desc *descp;
+ int ret;
+
+ descp = *(void **)dev->platform_data;
+ if (!descp) {
+ dev_err(dev, "No platform specific data for output port\n");
+ return -ENODEV;
+ }
+
+ output = devm_kzalloc(dev, sizeof(*output), GFP_KERNEL);
+ if (!output)
+ return -ENOMEM;
+
+ output->dev = dev;
+ output->descp = descp;
+
+ ret = component_add(dev, &lsdc_output_port_component_ops);
+ if (ret) {
+ devm_kfree(dev, output);
+ dev_err(dev, "failed to register component: %d\n", ret);
+ return ret;
+ }
+
+ dev_set_drvdata(dev, output);
+
+ return 0;
+}
+
+static void lsdc_output_port_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct lsdc_output *output;
+
+ component_del(dev, &lsdc_output_port_component_ops);
+
+ output = dev_get_drvdata(dev);
+ if (output)
+ devm_kfree(dev, output);
+}
+
+static const struct platform_device_id lsdc_output_port_ids[] = {
+ { .name = "lsdc-output-port" },
+ { },
+};
+
+struct platform_driver lsdc_output_port_platform_driver = {
+ .driver = {
+ .name = "lsdc-output-port",
+ },
+ .probe = lsdc_output_port_probe,
+ .remove_new = lsdc_output_port_remove,
+ .id_table = lsdc_output_port_ids,
+};
diff --git a/drivers/gpu/drm/loongson/lsdc_output.h b/drivers/gpu/drm/loongson/lsdc_output.h
index 097789051a1d..684a5b19bc70 100644
--- a/drivers/gpu/drm/loongson/lsdc_output.h
+++ b/drivers/gpu/drm/loongson/lsdc_output.h
@@ -6,16 +6,48 @@
#ifndef __LSDC_OUTPUT_H__
#define __LSDC_OUTPUT_H__
-#include "lsdc_drv.h"
+#include <drm/drm_encoder.h>
+#include <drm/drm_connector.h>
+
+struct lsdc_desc;
+
+struct lsdc_output_desc {
+ u32 pipe;
+ const char type[32];
+};
+
+struct lsdc_output {
+ struct device *dev;
+ struct drm_encoder encoder;
+ struct drm_connector connector;
+ struct lsdc_output_desc *descp;
+};
+
+static inline struct lsdc_output *
+connector_to_lsdc_output(struct drm_connector *connector)
+{
+ return container_of(connector, struct lsdc_output, connector);
+}
+
+static inline struct lsdc_output *
+encoder_to_lsdc_output(struct drm_encoder *encoder)
+{
+ return container_of(encoder, struct lsdc_output, encoder);
+}
int ls7a1000_output_init(struct drm_device *ddev,
- struct lsdc_display_pipe *dispipe,
+ struct lsdc_output *output,
struct i2c_adapter *ddc,
unsigned int index);
int ls7a2000_output_init(struct drm_device *ldev,
- struct lsdc_display_pipe *dispipe,
+ struct lsdc_output *output,
struct i2c_adapter *ddc,
unsigned int index);
+int lsdc_output_preinit(struct device *parent,
+ const struct lsdc_desc *descp,
+ unsigned int index,
+ struct platform_device **ppdev);
+
#endif
diff --git a/drivers/gpu/drm/loongson/lsdc_output_7a1000.c b/drivers/gpu/drm/loongson/lsdc_output_7a1000.c
index 6fc8dd1c7d9a..f331c198e3d4 100644
--- a/drivers/gpu/drm/loongson/lsdc_output_7a1000.c
+++ b/drivers/gpu/drm/loongson/lsdc_output_7a1000.c
@@ -140,11 +140,10 @@ static const struct drm_encoder_funcs ls7a1000_encoder_funcs[2] = {
};
int ls7a1000_output_init(struct drm_device *ddev,
- struct lsdc_display_pipe *dispipe,
+ struct lsdc_output *output,
struct i2c_adapter *ddc,
unsigned int index)
{
- struct lsdc_output *output = &dispipe->output;
struct drm_encoder *encoder = &output->encoder;
struct drm_connector *connector = &output->connector;
int ret;
diff --git a/drivers/gpu/drm/loongson/lsdc_output_7a2000.c b/drivers/gpu/drm/loongson/lsdc_output_7a2000.c
index ce3dabec887e..2a51ad214394 100644
--- a/drivers/gpu/drm/loongson/lsdc_output_7a2000.c
+++ b/drivers/gpu/drm/loongson/lsdc_output_7a2000.c
@@ -284,8 +284,7 @@ static int ls7a2000_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
struct drm_display_mode *mode)
{
struct lsdc_output *output = encoder_to_lsdc_output(encoder);
- struct lsdc_display_pipe *dispipe = output_to_display_pipe(output);
- unsigned int index = dispipe->index;
+ unsigned int index = output->descp->pipe;
struct drm_device *ddev = encoder->dev;
struct lsdc_device *ldev = to_lsdc(ddev);
struct hdmi_avi_infoframe infoframe;
@@ -335,8 +334,7 @@ static void ls7a2000_hdmi_atomic_disable(struct drm_encoder *encoder,
struct drm_atomic_state *state)
{
struct lsdc_output *output = encoder_to_lsdc_output(encoder);
- struct lsdc_display_pipe *dispipe = output_to_display_pipe(output);
- unsigned int index = dispipe->index;
+ unsigned int index = output->descp->pipe;
struct drm_device *ddev = encoder->dev;
struct lsdc_device *ldev = to_lsdc(ddev);
u32 val;
@@ -360,8 +358,7 @@ static void ls7a2000_hdmi_atomic_enable(struct drm_encoder *encoder,
struct drm_device *ddev = encoder->dev;
struct lsdc_device *ldev = to_lsdc(ddev);
struct lsdc_output *output = encoder_to_lsdc_output(encoder);
- struct lsdc_display_pipe *dispipe = output_to_display_pipe(output);
- unsigned int index = dispipe->index;
+ unsigned int index = output->descp->pipe;
u32 val;
/* datasheet say it should larger than 48 */
@@ -482,8 +479,7 @@ static void ls7a2000_hdmi_atomic_mode_set(struct drm_encoder *encoder,
struct drm_connector_state *conn_state)
{
struct lsdc_output *output = encoder_to_lsdc_output(encoder);
- struct lsdc_display_pipe *dispipe = output_to_display_pipe(output);
- unsigned int index = dispipe->index;
+ unsigned int index = output->descp->pipe;
struct drm_device *ddev = encoder->dev;
struct lsdc_device *ldev = to_lsdc(ddev);
struct drm_display_mode *mode = &crtc_state->mode;
@@ -512,11 +508,10 @@ static const struct drm_encoder_helper_funcs ls7a2000_encoder_helper_funcs = {
* writing hdmi register do no harms.
*/
int ls7a2000_output_init(struct drm_device *ddev,
- struct lsdc_display_pipe *dispipe,
+ struct lsdc_output *output,
struct i2c_adapter *ddc,
unsigned int pipe)
{
- struct lsdc_output *output = &dispipe->output;
struct drm_encoder *encoder = &output->encoder;
struct drm_connector *connector = &output->connector;
int ret;
@@ -536,8 +531,6 @@ int ls7a2000_output_init(struct drm_device *ddev,
if (ret)
return ret;
- drm_info(ddev, "display pipe-%u has HDMI %s\n", pipe, pipe ? "" : "and/or VGA");
-
drm_connector_helper_add(connector, &ls7a2000_connector_helpers);
drm_connector_attach_encoder(connector, encoder);
--
2.34.1
On Mon, May 27, 2024 at 03:58:24AM +0800, Sui Jingfeng wrote:
> In some display subsystems, the functionality of a PCIe device may too
> complex to be managed by a single driver. A split of the functionality
> into child devices can helpful to achieve better modularity.
>
> Another benefit is that we could migrate the dependency on exterinal
> modules to a submodule level with the helper created. For example, it's
> not uncommon that some exterinal module will return -EPROBE_DEFER to our
> driver during probe time. KMS driver has to tear down everything when it
> receives -EPROBE_DEFER, the problem is that it's completely not necessary
> and rising cyclic dependency problems if not process correctly.
>
> Add the loongson_create_platform_device() function, which allows the KMS
> driver to create sub-devices for it. The manually created decice acts as
> agents for the principal part, migrate the potential issue to submodule.
>
> Signed-off-by: Sui Jingfeng <[email protected]>
> ---
> drivers/gpu/drm/loongson/loongson_device.c | 42 ++++++++++++++++++++++
> drivers/gpu/drm/loongson/lsdc_drv.h | 6 ++++
> 2 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/loongson/loongson_device.c b/drivers/gpu/drm/loongson/loongson_device.c
> index 9986c8a2a255..b268549d643e 100644
> --- a/drivers/gpu/drm/loongson/loongson_device.c
> +++ b/drivers/gpu/drm/loongson/loongson_device.c
> @@ -4,6 +4,7 @@
> */
>
> #include <linux/pci.h>
> +#include <linux/platform_device.h>
>
> #include "lsdc_drv.h"
>
> @@ -100,3 +101,44 @@ lsdc_device_probe(struct pci_dev *pdev, enum loongson_chip_id chip_id)
> {
> return __chip_id_desc_table[chip_id];
> }
> +
> +int loongson_create_platform_device(struct device *parent,
> + const char *name, int id,
> + struct resource *pres,
> + void *data,
> + struct platform_device **ppdev)
> +{
> + struct platform_device *pdev;
> + int ret;
> +
> + pdev = platform_device_alloc(name, id);
> + if (!pdev)
> + return -ENOMEM;
> +
> + pdev->dev.parent = parent;
> +
> + if (pres) {
> + ret = platform_device_add_resources(pdev, pres, 1);
> + if (ret) {
> + platform_device_put(pdev);
> + return ret;
> + }
> + }
> +
> + if (data) {
> + void *pdata = kmalloc(sizeof(void *), GFP_KERNEL);
> +
> + *(void **)pdata = data;
> + pdev->dev.platform_data = pdata;
> + }
> +
> + ret = platform_device_add(pdev);
> + if (ret) {
> + platform_device_put(pdev);
> + return ret;
> + }
Please use platform_device_register_resndata().
> +
> + *ppdev = pdev;
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/loongson/lsdc_drv.h b/drivers/gpu/drm/loongson/lsdc_drv.h
> index fbf2d760ef27..a2c6b496a69f 100644
> --- a/drivers/gpu/drm/loongson/lsdc_drv.h
> +++ b/drivers/gpu/drm/loongson/lsdc_drv.h
> @@ -47,6 +47,12 @@ enum loongson_chip_id {
> const struct lsdc_desc *
> lsdc_device_probe(struct pci_dev *pdev, enum loongson_chip_id chip);
>
> +int loongson_create_platform_device(struct device *parent,
> + const char *name, int id,
> + struct resource *pres,
> + void *data,
> + struct platform_device **ppdev);
> +
> struct lsdc_kms_funcs;
>
> /* DC specific */
> --
> 2.34.1
>
--
With best wishes
Dmitry
> Introduce component framework to bind child and sibling devices, for better
> modularity and offload the deferral probe issue to submodule if it need to
needs?
> attach exterinal module someday. Also for better reflect the hardware
external? Reflect the hardware layout better?
> layout.
…
> consists of encoder phy and level shifter. Well, the GPU are standalone
is stand-alone?
…
> for the master, it could return the -EPROBE_DEFER back to the drvier core.
driver?
> This allows the master don't have to tear down everything, thereore
therefore?
Less typos would be nicer also for such a cover letter.
Regards,
Markus
> In some display subsystems, the functionality of a PCIe device may too
might be?
…
> into child devices can helpful to …
be?
> Another benefit is that we could migrate the dependency on exterinal
external?
…
> and rising cyclic dependency problems if not process correctly.
processed?
…
> driver to create sub-devices for it. The manually created decice acts as
device?
> agents for the principal part, migrate the potential issue to submodule.
an agent?
Please improve your change descriptions considerably.
…
> +++ b/drivers/gpu/drm/loongson/loongson_device.c
…
> @@ -100,3 +101,44 @@ lsdc_device_probe(struct pci_dev *pdev, enum loongson_chip_id chip_id)
> {
> return __chip_id_desc_table[chip_id];
> }
> +
> +int loongson_create_platform_device(struct device *parent,
> + const char *name, int id,
> + struct resource *pres,
> + void *data,
> + struct platform_device **ppdev)
> +{
…
> + ret = platform_device_add_resources(pdev, pres, 1);
> + if (ret) {
> + platform_device_put(pdev);
> + return ret;
> + }
…
> + ret = platform_device_add(pdev);
> + if (ret) {
> + platform_device_put(pdev);
> + return ret;
> + }
…
Please use a 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
Regards,
Markus
…
> However, the driver needs all of the subcompoments ready to use before
subcomponents?
…
> goes with the PCIe master, sinch they has no dependency on exterinal
since they have no dependencies on external?
> modules.
>
> While the outputs drivers, such as encoders and conectors, may has some
output? connectors, might have?
…
> if it need to do so, the master drvier won't bind until all submodules
…
Please improve your change descriptions considerably.
Regards,
Markus
…
> 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