Re: drm/mgag200: Fix VBLANK interrupt handling
Hi, On 2024/7/31 15:09, Thomas Zimmermann wrote: Fix support for VBLANK interrupts on G200ER, G200EV and G200SE, which use a slightly different implementation than the others. The original commits forgot to update the custom helpers when adding interrupt handling for VBLANK events. Signed-off-by: Thomas Zimmermann LGTM, Acked-by: sui.jingf...@linux.dev -- Best regards, Sui
Re: [PATCH v5 2/2] drm/loongson: Add dummy gpu driver as a subcomponent
Hi, On 2024/7/29 15:15, Markus Elfring wrote: … the driver is loaded, drm/loongson driver still need to wait all of needs to wait on …? … design. Therefore, add a dummy driver for the GPU, … Is there a need to reconsider the categorisation and usability descriptions another bit for such a software component? For honoring the framework and testing the framework purpose. The GPU can possibly be the master of the all sub-components, this definitely need some extra consideration and negotiation with the DC and the fake master device. Depend on configuration and user space expectation. The problem should be solved earlier. Regards, Markus -- Best regards, Sui
Re: [PATCH v5 1/2] drm/loongson: Introduce component framework support
Hi, On 2024/7/29 14:40, Markus Elfring wrote: … +++ b/drivers/gpu/drm/loongson/loongson_drv.h @@ -0,0 +1,108 @@ … +#ifndef __LOONGSON_DRV_H__ +#define __LOONGSON_DRV_H__ … I suggest to omit leading underscores from such identifiers. https://wiki.sei.cmu.edu/confluence/display/c/DCL37-C.+Do+not+declare+or+define+a+reserved+identifier I suggest add this rules to the checkpatch.pl script, It's more safe to follow *after* your suggestion is accepted. After all, the checkpatch.pl is de-facto standard. checkpatch.pl is happy about my patch for now. Regards, Markus -- Best regards, Sui
Re: [PATCH v5 1/2] drm/loongson: Introduce component framework support
Hi,
On 2024/7/29 15:37, Markus Elfring wrote:
…
+++ b/drivers/gpu/drm/loongson/loongson_drv.c
@@ -0,0 +1,298 @@
…
+static int loongson_drm_driver_probe(struct platform_device *pdev)
+{
…
+ dev_info(>dev, "probed\n");
…
+}
…
Do you find such information really relevant?
This helps to see the probe order of all drivers,
this also help us to know the name of the fake master device.
Regards,
Markus
--
Best regards,
Sui
[PATCH v5 2/2] drm/loongson: Add dummy gpu driver as a subcomponent
Loongson Graphics are PCIe multifunctional devices, the GPU and the
display controller are two distinct devices. Despite hardware units
that come along with PCIe are already ready to be driven by the time
the driver is loaded, drm/loongson driver still need to wait all of
its dependencies ready before it can register DRM service to user space.
The newly introduced component framework allow us to use loose coupling
design. Therefore, add a dummy driver for the GPU, it is functional as
a subcomponent as well.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/loongson/Makefile | 3 +
drivers/gpu/drm/loongson/loonggpu_pci_drv.c | 163
drivers/gpu/drm/loongson/loonggpu_pci_drv.h | 35 +
drivers/gpu/drm/loongson/loongson_drv.c | 1 +
drivers/gpu/drm/loongson/loongson_module.c | 4 +
drivers/gpu/drm/loongson/loongson_module.h | 1 +
6 files changed, 207 insertions(+)
create mode 100644 drivers/gpu/drm/loongson/loonggpu_pci_drv.c
create mode 100644 drivers/gpu/drm/loongson/loonggpu_pci_drv.h
diff --git a/drivers/gpu/drm/loongson/Makefile
b/drivers/gpu/drm/loongson/Makefile
index 90c6239bd77d..cdc60ec975e4 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 += \
+ loonggpu_pci_drv.o
+
loongson-y += loongson_device.o \
loongson_drv.o \
loongson_module.o
diff --git a/drivers/gpu/drm/loongson/loonggpu_pci_drv.c
b/drivers/gpu/drm/loongson/loonggpu_pci_drv.c
new file mode 100644
index ..52785a130b83
--- /dev/null
+++ b/drivers/gpu/drm/loongson/loonggpu_pci_drv.c
@@ -0,0 +1,163 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Authors:
+ * Sui Jingfeng
+ */
+
+#include
+#include
+
+#include
+#include
+
+#include "loongson_drv.h"
+#include "loongson_module.h"
+#include "loonggpu_pci_drv.h"
+
+static int loonggpu_get_version(struct loonggpu_device *gpu)
+{
+ u32 hw_info = loong_rreg32(gpu, 0x8C);
+ u8 host_id;
+ u8 revision;
+
+ /* LoongGPU hardware info */
+ 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(gpu->drm, "LoongGPU(TM): LG%x%x0, revision: %x, Host: %s\n",
+gpu->ver_major, gpu->ver_minor, revision,
+host_id ? "LS2K2000" : "LS7A2000");
+
+ return 0;
+}
+
+static irqreturn_t loonggpu_irq_handler(int irq, void *arg)
+{
+ struct loonggpu_device *gpu = arg;
+
+ drm_dbg(gpu->drm, "LoongGPU(TM) interrupted\n");
+
+ return IRQ_HANDLED;
+}
+
+static int loonggpu_pci_component_bind(struct device *dev,
+ struct device *master,
+ void *data)
+{
+ struct loonggpu_device *gpu = dev_get_drvdata(dev);
+ struct drm_device *drm = data;
+ struct loongson_drm *ldrm = to_loongson_drm(drm);
+ struct pci_dev *pdev = to_pci_dev(dev);
+ int ret;
+
+ gpu->drm = drm;
+ ldrm->loonggpu = gpu;
+
+ loonggpu_get_version(gpu);
+
+ ret = devm_request_irq(dev,
+ pdev->irq,
+ loonggpu_irq_handler,
+ IRQF_SHARED,
+ dev_name(dev),
+ gpu);
+ if (ret)
+ return ret;
+
+ drm_info(gpu->drm, "LoongGPU(TM) irq: %d\n", pdev->irq);
+
+ return 0;
+}
+
+static void loonggpu_pci_component_unbind(struct device *dev,
+ struct device *master,
+ void *data)
+{
+ dev_dbg(dev, "LoongGPU(TM) unbind\n");
+}
+
+static const struct component_ops loonggpu_pci_component_ops = {
+ .bind = loonggpu_pci_component_bind,
+ .unbind = loonggpu_pci_component_unbind,
+};
+
+static int loonggpu_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
+{
+ struct loonggpu_device *gpu;
+ int ret;
+
+ ret = pcim_enable_device(pdev);
+ if (ret)
+ return ret;
+
+ pci_set_master(pdev);
+
+ gpu = devm_kzalloc(>dev, sizeof(*gpu), GFP_KERNEL);
+ if (!gpu)
+ return -ENOMEM;
+
+ gpu->pdev = pdev;
+
+ gpu->reg_base = pcim_iomap(pdev, 0, 0);
+ if (!gpu->reg_base)
+ return -ENOMEM;
+
+ pci_set_drvdata(pdev, gpu);
+
+ dev_info(>dev, "LoongGPU(TM) PCI driver probed\n");
+
+ return component_add(>dev, _pci_component_ops);
+}
+
+static void loonggpu_pci_remove(struct pci_dev *pdev)
+{
+ component_del(>dev, _pci
[PATCH v5 1/2] drm/loongson: Introduce component framework support
In some display subsystems, the functionality of a PCIe device might be
too complex to manage with a single driver. A split of the functionality
into child devices can help to achieve better modularity. For example,
KMS drivers who have dependencies on external modules will suffer from
the deferral probe problem. The problem is that the DRM driver has to
tear down everything when it receives '-EPROBE_DEFER', even though the
independent part of the driver is not related to this error.
The idea is to migrate (offload) the dependency to submodules, creating
submodule by creating platform devices manually during driver load time.
Submodules are functional as agents, which will be responsible for
attaching needed external modules for the main body of the whole KMS
driver. Submodules are also responsible for returning '-EPROBE_DEFER'
to the driver core if it needs to do so.
Make LSDC PCI driver as a subcomponent and creating a platform device as
a DRM proxy, which will attach the common DRM routines to our hardware
after all submodules bound. The proxy is a fake master which is working
at the foreground.
Move DRM-related codes into the loongson_drm_master_bind(), move output
related things into the submodule, because the output hardware units are
relatively standalone IPs.
Initialize the GPIO-I2Cs in the driver probe() of the LSDC PCIe device,
tie its lifetime to the LSDC.
Assign GFX PLL, TTM memory manager part, and power management part
to the DRM proxy, as those entities do not belong to the LSDC itself.
They are common resources.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/loongson/Makefile | 2 +
drivers/gpu/drm/loongson/loongson_device.c| 30 ++
drivers/gpu/drm/loongson/loongson_drv.c | 298 +++
drivers/gpu/drm/loongson/loongson_drv.h | 108 ++
drivers/gpu/drm/loongson/loongson_module.c| 80 +++-
drivers/gpu/drm/loongson/loongson_module.h| 31 ++
drivers/gpu/drm/loongson/lsdc_benchmark.c | 12 +-
drivers/gpu/drm/loongson/lsdc_benchmark.h | 2 +-
drivers/gpu/drm/loongson/lsdc_crtc.c | 4 +-
drivers/gpu/drm/loongson/lsdc_debugfs.c | 41 +--
drivers/gpu/drm/loongson/lsdc_drv.c | 346 +-
drivers/gpu/drm/loongson/lsdc_drv.h | 89 +
drivers/gpu/drm/loongson/lsdc_gem.c | 42 ++-
drivers/gpu/drm/loongson/lsdc_gem.h | 13 +
drivers/gpu/drm/loongson/lsdc_gfxpll.c| 33 +-
drivers/gpu/drm/loongson/lsdc_gfxpll.h| 3 +-
drivers/gpu/drm/loongson/lsdc_i2c.c | 55 ++-
drivers/gpu/drm/loongson/lsdc_i2c.h | 21 +-
drivers/gpu/drm/loongson/lsdc_output.c| 183 +
drivers/gpu/drm/loongson/lsdc_output.h| 33 +-
drivers/gpu/drm/loongson/lsdc_output_7a1000.c | 6 +-
drivers/gpu/drm/loongson/lsdc_output_7a2000.c | 20 +-
drivers/gpu/drm/loongson/lsdc_pixpll.c| 4 +-
drivers/gpu/drm/loongson/lsdc_plane.c | 4 +-
drivers/gpu/drm/loongson/lsdc_ttm.c | 70 ++--
drivers/gpu/drm/loongson/lsdc_ttm.h | 4 +-
26 files changed, 1026 insertions(+), 508 deletions(-)
create mode 100644 drivers/gpu/drm/loongson/loongson_drv.c
create mode 100644 drivers/gpu/drm/loongson/loongson_drv.h
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..90c6239bd77d 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 \
@@ -17,6 +18,7 @@ loongson-y := \
lsdc_ttm.o
loongson-y += loongson_device.o \
+ loongson_drv.o \
loongson_module.o
obj-$(CONFIG_DRM_LOONGSON) += loongson.o
diff --git a/drivers/gpu/drm/loongson/loongson_device.c
b/drivers/gpu/drm/loongson/loongson_device.c
index 9986c8a2a255..51de71a9b692 100644
--- a/drivers/gpu/drm/loongson/loongson_device.c
+++ b/drivers/gpu/drm/loongson/loongson_device.c
@@ -4,6 +4,7 @@
*/
#include
+#include
#include "lsdc_drv.h"
@@ -100,3 +101,32 @@ lsdc_device_probe(struct pci_dev *pdev, enum
loongson_chip_id chip_id)
{
return __chip_id_desc_table[chip_id];
}
+
+static void loongson_device_postfini(void *data)
+{
+ struct platform_device *proxy = data;
+
+ platform_device_unregister(proxy);
+}
+
+int loongson_device_preinit(struct device *parent)
+{
+ struct platform_device *proxy;
+ int ret;
+
+ proxy = platform_device_register_resndata(parent,
+ "loongson.drm.proxy",
+ PLATFORM_DEVID_NONE,
+ NULL, 0,
+
[PATCH v5 0/2] drm/loongson: Introduce component framework support
125MB/s Copy bo of 8100KiB 60 times from GTT to VRAM in 104ms: 4673MB/s Copy bo of 8100KiB 60 times from VRAM to GTT in 13480ms: 36MB/s Also run IGT kms_flip and fbdev tests, no obvious problems found. Sui Jingfeng (2): drm/loongson: Introduce component framework support drm/loongson: Add dummy gpu driver as a subcomponent drivers/gpu/drm/loongson/Makefile | 5 + drivers/gpu/drm/loongson/loonggpu_pci_drv.c | 163 + drivers/gpu/drm/loongson/loonggpu_pci_drv.h | 35 ++ drivers/gpu/drm/loongson/loongson_device.c| 30 ++ drivers/gpu/drm/loongson/loongson_drv.c | 299 +++ drivers/gpu/drm/loongson/loongson_drv.h | 108 ++ drivers/gpu/drm/loongson/loongson_module.c| 84 - drivers/gpu/drm/loongson/loongson_module.h| 32 ++ drivers/gpu/drm/loongson/lsdc_benchmark.c | 12 +- drivers/gpu/drm/loongson/lsdc_benchmark.h | 2 +- drivers/gpu/drm/loongson/lsdc_crtc.c | 4 +- drivers/gpu/drm/loongson/lsdc_debugfs.c | 41 +-- drivers/gpu/drm/loongson/lsdc_drv.c | 346 +- drivers/gpu/drm/loongson/lsdc_drv.h | 89 + drivers/gpu/drm/loongson/lsdc_gem.c | 42 ++- drivers/gpu/drm/loongson/lsdc_gem.h | 13 + drivers/gpu/drm/loongson/lsdc_gfxpll.c| 33 +- drivers/gpu/drm/loongson/lsdc_gfxpll.h| 3 +- drivers/gpu/drm/loongson/lsdc_i2c.c | 55 ++- drivers/gpu/drm/loongson/lsdc_i2c.h | 21 +- drivers/gpu/drm/loongson/lsdc_output.c| 183 + drivers/gpu/drm/loongson/lsdc_output.h| 33 +- drivers/gpu/drm/loongson/lsdc_output_7a1000.c | 6 +- drivers/gpu/drm/loongson/lsdc_output_7a2000.c | 20 +- drivers/gpu/drm/loongson/lsdc_pixpll.c| 4 +- drivers/gpu/drm/loongson/lsdc_plane.c | 4 +- drivers/gpu/drm/loongson/lsdc_ttm.c | 70 ++-- drivers/gpu/drm/loongson/lsdc_ttm.h | 4 +- 28 files changed, 1233 insertions(+), 508 deletions(-) create mode 100644 drivers/gpu/drm/loongson/loonggpu_pci_drv.c create mode 100644 drivers/gpu/drm/loongson/loonggpu_pci_drv.h create mode 100644 drivers/gpu/drm/loongson/loongson_drv.c create mode 100644 drivers/gpu/drm/loongson/loongson_drv.h create mode 100644 drivers/gpu/drm/loongson/lsdc_output.c -- 2.43.0
Re: [PATCH v3 00/19] Add Freescale i.MX8qxp Display Controller support
Hi,
On 7/28/24 04:28, Dmitry Baryshkov wrote:
On Sun, Jul 28, 2024 at 03:10:21AM GMT, Sui Jingfeng wrote:
Hi,
On 7/28/24 00:39, Dmitry Baryshkov wrote:
Hi,
This patch series aims to add Freescale i.MX8qxp Display Controller support.
The controller is comprised of three main components that include a blit
engine for 2D graphics accelerations, display controller for display output
processing, as well as a command sequencer.
Previous patch series attempts to do that can be found at:
https://patchwork.freedesktop.org/series/84524/
This series addresses Maxime's comments on the previous one:
a. Split the display controller into multiple internal devices.
1) List display engine, pixel engine, interrupt controller and more as the
controller's child devices.
2) List display engine and pixel engine's processing units as their child
devices.
b. Add minimal feature support.
Only support two display pipelines with primary planes with XR24 fb,
backed by two fetchunits. No fetchunit dynamic allocation logic(to be done
when necessary).
c. Use drm_dev_{enter, exit}().
Since this series changes a lot comparing to the previous one, I choose to
send it with a new patch series, not a new version.
I'm sorry, I have started reviewing v2 without noticing that there is a
v3 already.
Let me summarize my comments:
- You are using OF aliases. Are they documented and acked by DT
maintainers?
- I generally feel that the use of so many small devices to declare
functional blocks is an abuse of the DT. Please consider creating
_small_ units from the driver code directly rather than going throught
the components.
Well, I really don't think so. I don't agree.
I have checked the DTSpec[1] before type, the spec isn't define how
many is considered to be "many", and the spec isn't define to what
extent is think to be "small" as well.
Yeah. However _usually_ we are not defining DT devices for sub-device
components.
I guess, this depended on their hardware (i.MX8qxp) layout, reflecting
exactly what their hardware's layout is perfectly valid. It also depend
on if specific part of those sub-device will be re-visioned or not. If
only a small part of the whole is re-versioned in the future, we can
still re-using this same driver with slightly modify(update) the DTS.
The point is to controll the granularity and forward compatibility.
So at least such decisions ought to be described and
explained in the cover letter.
Agree, but I see 08/19 patch has a beautiful schematic. I have learned
a lot when reading it. I can't see any abuse of the DT through this
bulk series anyway.
Comments below are not revelant to Ying's patch series itself.
/**/
By the way, the last time that I have ever seen and feel abuse of the
DT is the aux-bridge.c[1] and aux-hpd-bridge.c[2]. I strongly feel that
those two *small* programs are abuses to the DT and possibily abuse to
the auxiliary bus framework.
1) It's so *small* that it don't even have a hardware entity (physical
device) to corresponding with. As far as I can see, all hardware
units in this patch series are bigger than yours. Because your HPD
bridge is basically a "virtual wire".
An non-physical-exist wire hold reference to several device node,
this is the most awful abuse to the DT I have ever seen. In other
words, despite you want to solve some software problems, but then,
you could put a device not in the DTS, and let the 'OF' system
create a device for you. Just like what this series do.
2) I feel your HPD fake bridge driver abuse to the philosophy of
auxiliary bus [3]. The document of auxiliary bus tell us that
"These individual devices split from the core cannot live on
the platform bus as they are not physical devices that are
controlled by DT/ACPI"
Which is nearly equivalent to say that devices that are controlled
by DT/ACPI have better ways to enforce the control. When using
auxiliary bus, we *generally* should not messed with DT. See
golden examples[4][5]. At least, their code are able to run on
X86, while the code you write just can't.
[0] https://patchwork.freedesktop.org/patch/60/?series=135786=3
[1]
https://elixir.bootlin.com/linux/v6.10.2/source/drivers/gpu/drm/bridge/aux-bridge.c
[2]
https://elixir.bootlin.com/linux/v6.10.2/source/drivers/gpu/drm/bridge/aux-hpd-bridge.c
[3] https://www.kernel.org/doc/html/latest/driver-api/auxiliary_bus.html
[4] https://patchwork.freedesktop.org/series/136431/
[5] https://patchwork.freedesktop.org/series/134837/
Best regards
Sui
[1]
https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.4
--
Best regards
Sui
Re: [PATCH v3 00/19] Add Freescale i.MX8qxp Display Controller support
Hi,
On 7/28/24 00:39, Dmitry Baryshkov wrote:
Hi,
This patch series aims to add Freescale i.MX8qxp Display Controller support.
The controller is comprised of three main components that include a blit
engine for 2D graphics accelerations, display controller for display output
processing, as well as a command sequencer.
Previous patch series attempts to do that can be found at:
https://patchwork.freedesktop.org/series/84524/
This series addresses Maxime's comments on the previous one:
a. Split the display controller into multiple internal devices.
1) List display engine, pixel engine, interrupt controller and more as the
controller's child devices.
2) List display engine and pixel engine's processing units as their child
devices.
b. Add minimal feature support.
Only support two display pipelines with primary planes with XR24 fb,
backed by two fetchunits. No fetchunit dynamic allocation logic(to be done
when necessary).
c. Use drm_dev_{enter, exit}().
Since this series changes a lot comparing to the previous one, I choose to
send it with a new patch series, not a new version.
I'm sorry, I have started reviewing v2 without noticing that there is a
v3 already.
Let me summarize my comments:
- You are using OF aliases. Are they documented and acked by DT
maintainers?
- I generally feel that the use of so many small devices to declare
functional blocks is an abuse of the DT. Please consider creating
_small_ units from the driver code directly rather than going throught
the components.
Well, I really don't think so. I don't agree.
I have checked the DTSpec[1] before type, the spec isn't define how
many is considered to be "many", and the spec isn't define to what
extent is think to be "small" as well.
[1]
https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.4
--
Best regards
Sui
Re: [PATCH v4] drm/loongson: Introduce component framework support
Hi, On 7/24/24 15:30, Markus Elfring wrote: In some display subsystems, the functionality of a PCIe device may too might be? … of the dirver is loaded, … driver? … its dependencies ready before it can register the service to userspace. user space? … submodule by creating platform devices manually during driverload time. driver load? … device as a DRM proxy, which will attach the common drm routines to our DRM? … While at it, also do some cleanups. I find such information suspicious. Is there any need to offer adjustments as separate update steps? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10#n81 Thanks a lot for review, I will try to fix those problems in the future. Regards, Markus
[PATCH v4 1/1] drm/loongson: Introduce component framework support
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 help to achieve better modularity. For example, KMS drivers who have dependencies on external modules will suffer from the deferral probe problem. The problem is that the DRM driver has to tear down everything when it receives '-EPROBE_DEFER', even though the independent part of the driver is not related to this error. Loongson Graphics are PCIe multifunctional devices, the GPU and the display controller are two distinct devices. Despite all hardware units that come along with PCIe are actually ready to be driven by the time of the dirver is loaded, drm/loongson driver still need to wait all of its dependencies ready before it can register the service to userspace. The idea is to migrate (offload) the dependency to submodules, creating submodule by creating platform devices manually during driverload time. Submodules are functional as agents, which will be responsible for attaching needed external modules for the main body of the whole KMS driver. Submodules are also responsible for returning '-EPROBE_DEFER' to the driver core if it needs to do so. Add a dummy GPU driver as a subcomponent, make LSDC PCI driver as a subcomponent as well. Introduce the component framework to bind those scatter software and/or hardware entities into one. Creating a platform device as a DRM proxy, which will attach the common drm routines to our hardware after all submodules bound. The proxy is the explicit executant working at the foreground, while the LSDC PCIe device is the master. Move DRM-related codes into the loongson_drm_master_bind(), move output related things into the submodule, because the output hardware units are relatively standalone IPs and the encoder/connector/transcoder may depend on external modules. While the display controller itself and GPIO-I2Cs have no dependency on external modules, therefore go with the LSDC PCIe device. Also assign GFX PLL, TTM memory manager part, and power management part to loongson drm proxy, as they do not belong to the LSDC itself. While at it, also do some cleanups. Signed-off-by: Sui Jingfeng --- drivers/gpu/drm/loongson/Makefile | 6 + drivers/gpu/drm/loongson/loonggpu_pci_drv.c | 163 drivers/gpu/drm/loongson/loonggpu_pci_drv.h | 35 ++ drivers/gpu/drm/loongson/loongson_device.c| 30 ++ drivers/gpu/drm/loongson/loongson_drv.c | 293 + drivers/gpu/drm/loongson/loongson_drv.h | 90 drivers/gpu/drm/loongson/loongson_module.c| 84 +++- drivers/gpu/drm/loongson/loongson_module.h| 32 ++ drivers/gpu/drm/loongson/lsdc_benchmark.c | 12 +- drivers/gpu/drm/loongson/lsdc_benchmark.h | 2 +- drivers/gpu/drm/loongson/lsdc_component.c | 159 +++ drivers/gpu/drm/loongson/lsdc_crtc.c | 4 +- drivers/gpu/drm/loongson/lsdc_debugfs.c | 44 +- drivers/gpu/drm/loongson/lsdc_drv.c | 394 ++ drivers/gpu/drm/loongson/lsdc_drv.h | 83 +--- drivers/gpu/drm/loongson/lsdc_gem.c | 44 +- drivers/gpu/drm/loongson/lsdc_gem.h | 19 +- drivers/gpu/drm/loongson/lsdc_gfxpll.c| 29 +- drivers/gpu/drm/loongson/lsdc_gfxpll.h| 3 +- drivers/gpu/drm/loongson/lsdc_i2c.c | 43 +- drivers/gpu/drm/loongson/lsdc_i2c.h | 12 +- drivers/gpu/drm/loongson/lsdc_output.c| 183 drivers/gpu/drm/loongson/lsdc_output.h| 36 +- drivers/gpu/drm/loongson/lsdc_output_7a1000.c | 14 +- drivers/gpu/drm/loongson/lsdc_output_7a2000.c | 20 +- drivers/gpu/drm/loongson/lsdc_plane.c | 4 +- drivers/gpu/drm/loongson/lsdc_ttm.c | 84 ++-- drivers/gpu/drm/loongson/lsdc_ttm.h | 4 +- 28 files changed, 1318 insertions(+), 608 deletions(-) create mode 100644 drivers/gpu/drm/loongson/loonggpu_pci_drv.c create mode 100644 drivers/gpu/drm/loongson/loonggpu_pci_drv.h create mode 100644 drivers/gpu/drm/loongson/loongson_drv.c create mode 100644 drivers/gpu/drm/loongson/loongson_drv.h create mode 100644 drivers/gpu/drm/loongson/lsdc_component.c 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..1c496713baa9 100644 --- a/drivers/gpu/drm/loongson/Makefile +++ b/drivers/gpu/drm/loongson/Makefile @@ -2,6 +2,7 @@ loongson-y := \ lsdc_benchmark.o \ + lsdc_component.o \ lsdc_crtc.o \ lsdc_debugfs.o \ lsdc_drv.o \ @@ -9,6 +10,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 \ @@ -16,7 +18,11 @@ loongson-y := \ lsdc_probe.o \ lsdc_ttm.o +loongson-y += \ + loonggpu_pci_drv.o + loongson-y
[PATCH v4 0/1] drm/loongson: Introduce component framework support
a :00:06.1]# cat benchmark Copy bo of 8100KiB 60 times from GTT to GTT in 48ms: 10125MB/s Copy bo of 8100KiB 60 times from GTT to VRAM in 104ms: 4673MB/s Copy bo of 8100KiB 60 times from VRAM to GTT in 13480ms: 36MB/s Also run IGT kms_flip and fbdev tests, no obvious problem found. Sui Jingfeng (1): drm/loongson: Introduce component framework support drivers/gpu/drm/loongson/Makefile | 6 + drivers/gpu/drm/loongson/loonggpu_pci_drv.c | 163 drivers/gpu/drm/loongson/loonggpu_pci_drv.h | 35 ++ drivers/gpu/drm/loongson/loongson_device.c| 30 ++ drivers/gpu/drm/loongson/loongson_drv.c | 293 + drivers/gpu/drm/loongson/loongson_drv.h | 90 drivers/gpu/drm/loongson/loongson_module.c| 84 +++- drivers/gpu/drm/loongson/loongson_module.h| 32 ++ drivers/gpu/drm/loongson/lsdc_benchmark.c | 12 +- drivers/gpu/drm/loongson/lsdc_benchmark.h | 2 +- drivers/gpu/drm/loongson/lsdc_component.c | 159 +++ drivers/gpu/drm/loongson/lsdc_crtc.c | 4 +- drivers/gpu/drm/loongson/lsdc_debugfs.c | 44 +- drivers/gpu/drm/loongson/lsdc_drv.c | 394 ++ drivers/gpu/drm/loongson/lsdc_drv.h | 83 +--- drivers/gpu/drm/loongson/lsdc_gem.c | 44 +- drivers/gpu/drm/loongson/lsdc_gem.h | 19 +- drivers/gpu/drm/loongson/lsdc_gfxpll.c| 29 +- drivers/gpu/drm/loongson/lsdc_gfxpll.h| 3 +- drivers/gpu/drm/loongson/lsdc_i2c.c | 43 +- drivers/gpu/drm/loongson/lsdc_i2c.h | 12 +- drivers/gpu/drm/loongson/lsdc_output.c| 183 drivers/gpu/drm/loongson/lsdc_output.h| 36 +- drivers/gpu/drm/loongson/lsdc_output_7a1000.c | 14 +- drivers/gpu/drm/loongson/lsdc_output_7a2000.c | 20 +- drivers/gpu/drm/loongson/lsdc_plane.c | 4 +- drivers/gpu/drm/loongson/lsdc_ttm.c | 84 ++-- drivers/gpu/drm/loongson/lsdc_ttm.h | 4 +- 28 files changed, 1318 insertions(+), 608 deletions(-) create mode 100644 drivers/gpu/drm/loongson/loonggpu_pci_drv.c create mode 100644 drivers/gpu/drm/loongson/loonggpu_pci_drv.h create mode 100644 drivers/gpu/drm/loongson/loongson_drv.c create mode 100644 drivers/gpu/drm/loongson/loongson_drv.h create mode 100644 drivers/gpu/drm/loongson/lsdc_component.c create mode 100644 drivers/gpu/drm/loongson/lsdc_output.c -- 2.43.0
[PATCH v3 0/1]drm/loongson: Introduce component framework support
nt_ops [loongson]) [ 10.891851] loongson :00:06.1: [drm] Total 2 outputs [ 10.910159] loongson :00:06.1: [drm] VRAM: 4096 pages ready [ 10.916131] loongson :00:06.1: [drm] GTT: 32768 pages ready [ 10.922391] [drm] Initialized loongson 1.0.0 for :00:06.1 on minor 0 [ 379.378990] Console: switching to colour frame buffer device 128x37 [ 379.385261] loongson :00:06.1: [drm] fb0: loongsondrmfb frame buffer device Sui Jingfeng (1): drm/loongson: Introduce component framework support drivers/gpu/drm/loongson/Makefile | 5 + drivers/gpu/drm/loongson/loonggpu_pci_drv.c | 156 drivers/gpu/drm/loongson/loonggpu_pci_drv.h | 33 +++ drivers/gpu/drm/loongson/loongson_device.c| 1 + drivers/gpu/drm/loongson/loongson_drv.c | 157 drivers/gpu/drm/loongson/loongson_module.c| 38 ++- drivers/gpu/drm/loongson/loongson_module.h| 17 ++ drivers/gpu/drm/loongson/lsdc_drv.c | 235 -- drivers/gpu/drm/loongson/lsdc_drv.h | 56 ++--- drivers/gpu/drm/loongson/lsdc_i2c.c | 43 ++-- drivers/gpu/drm/loongson/lsdc_i2c.h | 11 +- drivers/gpu/drm/loongson/lsdc_irq.c | 21 +- drivers/gpu/drm/loongson/lsdc_output.c| 208 drivers/gpu/drm/loongson/lsdc_output.h| 36 ++- drivers/gpu/drm/loongson/lsdc_output_7a1000.c | 7 +- drivers/gpu/drm/loongson/lsdc_output_7a2000.c | 17 +- drivers/gpu/drm/loongson/lsdc_ttm.c | 4 +- drivers/gpu/drm/loongson/lsdc_ttm.h | 2 +- 18 files changed, 832 insertions(+), 215 deletions(-) create mode 100644 drivers/gpu/drm/loongson/loonggpu_pci_drv.c create mode 100644 drivers/gpu/drm/loongson/loonggpu_pci_drv.h create mode 100644 drivers/gpu/drm/loongson/loongson_drv.c create mode 100644 drivers/gpu/drm/loongson/lsdc_output.c -- 2.43.0
[PATCH v3 1/1] drm/loongson: Introduce component framework support
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 help to achieve better modularity. For example,
KMS drivers who has a dependency on external modules will suffer from
the deferral probe problem of those modules. The problem is that the
DRM driver has to tear down everything when it receives '-EPROBE_DEFER',
even though the independent part of the entire driver is not very closely
related.
For Loongson Graphics, the GPU and the display controller are two distinct
PCIe devices. Despite hardware units that come along with PCIe are actually
all ready to be driven, DRM driver still need to wait all of its dependency
ready before it can register the service to userspace.
The idea is to migrate(offload) the dependency to submodule, creating
submodule by creating platform device manually. Such submodules are
functional as agents, which will be responsible for attaching needed
external modules for the main body of entire KMS drive. It's also for
better modularity, as output hardware units are relatively standalone
IPs.
Add dummy GPU driver as a subcomponent, use the component framework to
bind those scatter software/hardware entities into one. Move DRM-related
codes into the loongson_drm_master_bind() function. Move output-related
things into the submodule, since the encoder/connector/ transcoder are
sometimes depend on external module. The display controller and GPIO-I2Cs
go with the PCIe master, as they have no dependency on external modules.
Make lsdc PCI driver as a subcomponent as well, creating a platform device
as proxy. The proxy will responsible attach the common drm routines to our
hardware, which keep the whole drm driver balanced (and fair) enough for
both the DC and GPU.
The lsdc PCI device is the haidden(implicit) parent, while the proxy is
the explicit master, which deal with drm related affairs for its parent.
The master won't bound until all submodules are ready, submodules are
responsible to return '-EPROBE_DEFER' back to the driver core if it needs
to do so.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/loongson/Makefile | 5 +
drivers/gpu/drm/loongson/loonggpu_pci_drv.c | 156
drivers/gpu/drm/loongson/loonggpu_pci_drv.h | 33 +++
drivers/gpu/drm/loongson/loongson_device.c| 1 +
drivers/gpu/drm/loongson/loongson_drv.c | 157
drivers/gpu/drm/loongson/loongson_module.c| 38 ++-
drivers/gpu/drm/loongson/loongson_module.h| 17 ++
drivers/gpu/drm/loongson/lsdc_drv.c | 235 --
drivers/gpu/drm/loongson/lsdc_drv.h | 56 ++---
drivers/gpu/drm/loongson/lsdc_i2c.c | 43 ++--
drivers/gpu/drm/loongson/lsdc_i2c.h | 11 +-
drivers/gpu/drm/loongson/lsdc_irq.c | 21 +-
drivers/gpu/drm/loongson/lsdc_output.c| 208
drivers/gpu/drm/loongson/lsdc_output.h| 36 ++-
drivers/gpu/drm/loongson/lsdc_output_7a1000.c | 7 +-
drivers/gpu/drm/loongson/lsdc_output_7a2000.c | 17 +-
drivers/gpu/drm/loongson/lsdc_ttm.c | 4 +-
drivers/gpu/drm/loongson/lsdc_ttm.h | 2 +-
18 files changed, 832 insertions(+), 215 deletions(-)
create mode 100644 drivers/gpu/drm/loongson/loonggpu_pci_drv.c
create mode 100644 drivers/gpu/drm/loongson/loonggpu_pci_drv.h
create mode 100644 drivers/gpu/drm/loongson/loongson_drv.c
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..cdc60ec975e4 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 \
@@ -16,7 +17,11 @@ loongson-y := \
lsdc_probe.o \
lsdc_ttm.o
+loongson-y += \
+ loonggpu_pci_drv.o
+
loongson-y += loongson_device.o \
+ loongson_drv.o \
loongson_module.o
obj-$(CONFIG_DRM_LOONGSON) += loongson.o
diff --git a/drivers/gpu/drm/loongson/loonggpu_pci_drv.c
b/drivers/gpu/drm/loongson/loonggpu_pci_drv.c
new file mode 100644
index ..a34c213171e0
--- /dev/null
+++ b/drivers/gpu/drm/loongson/loonggpu_pci_drv.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Authors:
+ * Sui Jingfeng
+ */
+
+#include
+#include
+
+#include
+#include
+
+#include "loongson_module.h"
+#include "loonggpu_pci_drv.h"
+
+static int loonggpu_get_version(struct loonggpu_device *gpu)
+{
+ u32 hw_info = loong_rreg32(gpu, 0x8C);
+ u8 host_id;
+ u8 revision;
+
+ /* LoongGPU hardware info */
+ gpu->ver_major = (hw_info >> 8) & 0x0F;
+ gpu->ver_minor = (hw_info & 0xF0) >> 4;
+ revision = hw
Re: [etnaviv-next v14 0/8] drm/etnaviv: Add driver wrapper for vivante GPUs attached on PCI(e) device
Hi, On 6/25/24 11:18, Icenowy Zheng wrote: 在 2024-05-20星期一的 00:53 +0800，Sui Jingfeng写道： drm/etnaviv use the component framework to bind multiple GPU cores to a virtual master, the virtual master is manually create during driver load time. This works well for various SoCs, yet there are some PCIe card has the vivante GPU cores integrated. The driver lacks the support for PCIe devices currently. Adds PCIe driver wrapper on the top of what drm/etnaviv already has, the component framework is still being used to bind subdevices, even though there is only one GPU core. But the process is going to be reversed, we create virtual platform device for each of the vivante GPU IP core shipped by the PCIe master. The PCIe master is real, bind all the virtual child to the master with component framework. v6: * Fix build issue on system without CONFIG_PCI enabled v7: * Add a separate patch for the platform driver rearrangement (Bjorn) * Switch to runtime check if the GPU is dma coherent or not (Lucas) * Add ETNAVIV_PARAM_GPU_COHERENT to allow userspace to query (Lucas) * Remove etnaviv_gpu.no_clk member (Lucas) * Fix Various typos and coding style fixed (Bjorn) v8: * Fix typos and remove unnecessary header included (Bjorn). * Add a dedicated function to create the virtual master platform device. v9: * Use PCI_VDEVICE() macro (Bjorn) * Add trivial stubs for the PCI driver (Bjorn) * Remove a redundant dev_err() usage (Bjorn) * Clean up etnaviv_pdev_probe() with etnaviv_of_first_available_node() v10: * Add one more cleanup patch * Resolve the conflict with a patch from Rob * Make the dummy PCI stub inlined * Print only if the platform is dma-coherrent V11: * Drop unnecessary changes (Lucas) * Tweak according to other reviews of v10. V12: * Create a virtual platform device for the subcomponent GPU cores * Bind all subordinate GPU cores to the real PCI master via component. V13: * Drop the non-component code path, always use the component framework to bind subcomponent GPU core. Even though there is only one core. * Defer the irq handler register. * Rebase and improve the commit message V14: * Rebase onto etnaviv-next and improve commit message. Tested with JD9230P GPU and LingJiu GP102 GPU. BTW how should VRAM and displayed related parts be handled on these dGPUs? Emm, I can only say I have no ideas. Thanks for Christian's tested-by, but I'm a bit worry about if etnaviv folks really like(or need) this. In the past, we started to contribute before we know the policy/framework very well. I have to managed to make things work before knowing the full picture. We developing drivers in a rather rapid way and rather wild. Sometime, we do it just for fun. As the card don't has a usable driver, we want do something and we do have already learned the framework and knowledge. But now as we know a bit more, I actually don't intend to brings concerns to other people. So only first 6 patch (or only part of them) are requested to be merged, patch 0007 or patch 0008 can just leave it there to be reviewed a bit longer if something is unsure. Its totally up to etnaviv folks, I don't mind. Thanks for the nice project. -- Best regards Sui
Re: [PATCH v3] software node: Implement device_get_match_data fwnode callback
Hi, On 6/23/24 03:29, Dmitry Torokhov wrote: In case of non-OF match (which includes the case where you use software nodes) the match data is coming from matching spi_device_id entry in the driver. We don't care about much how it is probed now, rather, after the driver probed by a non-OF way, how does the additional devices properties can be get? Say: 1) "device_property_read_u32(dev, "rotation", );" and 2) "!device_property_read_string(dev, "pervasive,thermal-zone", _zone))" For those spi/i2c/platform devices, what we argues are that those drivers really should just depend on "OF" before we have a reliable fwnode API backend to redirect to. They are working fine without such restriction now, You still *NOT* answer where the additional devices properties[1][2] can be acquire. [1] device_property_read_u32(dev, "rotation", ) [2] device_property_read_string(dev, "pervasive,thermal-zone", _zone)) so I see absolutely no reason imposing this restriction. The reason is rigorous. You are acclaiming that works by hardcode or by ignoring the flaws is fine, then all driver are working fine by *your* standard. Your personal standard has nothing to do with this patch. Where the additional device_property_read_() calls redirect to? What if users want to invoke more device_property_read_() function? They are being directed to first the primary FW node instance, which may be either OF, ACPI, or SW node, and then, if property is not present there, to the secondary FW node, which can be either again. What I'm asking is, on the non-OF and no-ACPI cases, where's those device_property_read_xxx() calls can be directed to? At no point ->device_get_match_data() callback in involved in this process. The patch is written for people who need it, not for people who don't. It will be involved if the device is associated with software node. Its for fwnode API user to get a consistent experience, that is to get a matching data without introduce extra/duplicated match mechanism. The patch is focus on fixing the undefined behavior, is discussing the correct way to consolidate the fwnode API. Its not going to discuss how does the those *old" and/or how does those non-fwnode systems works. Its NOT discussing how does the driver itself can be probed, a driver can be probed multiple way and is another question. Being probed and extract matching data can two different thing and is perfectly valid. Your problem is that you are not fully understand what other people does before you rush into the discussion. You are putting restrictions onto other people, while leaving the problem itself there unsolved. Its not a place to express your personal value or you personal status, such as, you are "ready" or "not ready" for something. Or persuading somebody should get used to what or teaching people to talks with a whatever tone like a God. None of those junk words are technical, I can not see constructive ideas. Thanks.
Re: [PATCH v3] software node: Implement device_get_match_data fwnode callback
Hi,
On 6/22/24 03:58, Dmitry Torokhov wrote:
Hi Sui,
On Sun, Apr 28, 2024 at 04:36:50AM +0800, Sui Jingfeng wrote:
Because the software node backend of the fwnode API framework lacks an
implementation for the .device_get_match_data function callback. This
makes it difficult to use(and/or test) a few drivers that originates
from DT world on the non-DT platform.
Implement the .device_get_match_data fwnode callback, which helps to keep
the three backends of the fwnode API aligned as much as possible. This is
also a fundamental step to make a few drivers OF-independent truely
possible.
Device drivers or platform setup codes are expected to provide a software
node string property, named as "compatible". At this moment, the value of
this string property is being used to match against the compatible entries
in the of_device_id table. It can be extended in the future though.
I am sorry, but this is not really correct.
I fine if the maintainers of fwnode API want to reject this, but got
rejected is not really equals to "not correct".
Software nodes are used to
augment missing or incomplete parameters, but are never primary objects
in the matching process. Sometimes "compatible" property is used with
software nodes, but it does not participate in the matching process. > There
are several ways for various buses to match a device and a driver,
but none of them operate on software nodes.
It's not participate in the matching process in the *past*, but what
we present is something *new*. I fine if you adhere to *old* and/or
*subsystem-dependent* approach, but there really no need to persuade
other people to follow your "old" idea.
Consider for example how
devices on SPI bus are matched (see
drivers/spi/spi.c::spi_match_device()):
This only make the driver be able to probed in a non-DT way, but
it doesn't tell how does the *additional device properties* can
be get. This is the key point.
1. OF/device tree based match. It *requires* the device to have
dev->of_node which is coming from a DTB. It does not work on software
nodes. In case of match the match data should come from of_device_id
entry.
2. ACPI-based match. The match is done based either on OF-compatible
data (which includes "compatible" property) in _DSD (if driver supports
OF-based matching), or based on HID/CID data. In the latter case the
match data is coming from acpi_device_id entry.
3. Name-based match, typically used for board-instantiated devices. In
this case match is done by comparing device name under which it was
instantiated against names listed in the drivers id_table. The match
data is coming from spi_device_id entry.
The statements here sound right, but it's useless. Because the problems
isn't solved yet, nor does you words point out a practical approach.
Similar matching processes are implemented for i2c and platform buses,
as well as others.
Your patch is effectively hijacks the #3 matching process and
substitutes the bus-specific match data (from
spi_device_id/i2c_device_id/etc) with OF data. This is not expected and
Please stop *contaminating* other people's patch, if you have better
idea you can posting it. My patch open a new door, and there do have
programmer in requesting(need) this in the past.
while we may want this in a long term (so we can eventually remove these
bus-specific device ids and only have ACPI/OF ones) I do not think we
are ready for this yet. At the very least this needs to be very clearly
documented.
This is your *personal* wants, if you want to remove something,
just do it. Keep quiet if you are not ready. Exposing your concerns
doesn't help to solve any problems.
Or, if you want it to be clear, you can contribute to Documentation/
gpu/todo.rst. Other peoples help you to become clear there, thanks.
Please note that we are talking about the completeness of the fwnode
APIs, what's you say above has nothing to do the device fwnode APIs.
Hence, is not revelant to my patch, again is out of scope.
Fixes: ffb42e64561e ("drm/tiny/repaper: Make driver OF-independent")
Fixes: 5703d6ae9573 ("drm/tiny/st7735r: Make driver OF-independent")
As other people mentioned this patch does not fix the aforementioned
commits because they are not broken.
You still not really understand what other people does, I'm not saying
it broken. I'm talking about the completeness.
In case of non-OF match (which
includes the case where you use software nodes) the match data is coming
from matching spi_device_id entry in the driver.
We don't care about much how it is probed now, rather, after the driver
probed by a non-OF way, how does the additional devices properties
can be get?
Say:
1) "device_property_read_u32(dev, "rotation", );" and
2) "!device_property_read_string(dev, "pervasive,thermal-zone",
_zone))"
For those spi/i2c/platform devices, what we argues are that
Re: [PATCH v8 3/3] drm/mediatek: Implement OF graphs support for display paths
Hi, On 6/18/24 18:17, AngeloGioacchino Del Regno wrote: It is impossible to add each and every possible DDP path combination for each and every possible combination of SoC and board: right now, this driver hardcodes configuration for 10 SoCs and this is going to grow larger and larger, and with new hacks like the introduction of mtk_drm_route which is anyway not enough for all final routes as the DSI cannot be connected to MERGE if it's not a dual-DSI, or enabling DSC preventively doesn't work if the display doesn't support it, or others. Since practically all display IPs in MediaTek SoCs support being interconnected with different instances of other, or the same, IPs or with different IPs and in different combinations, the final DDP pipeline is effectively a board specific configuration. Implement OF graphs support to the mediatek-drm drivers, allowing to stop hardcoding the paths, and preventing this driver to get a huge amount of arrays for each board and SoC combination, also paving the way to share the same mtk_mmsys_driver_data between multiple SoCs, making it more straightforward to add support for new chips. Reviewed-by: Alexandre Mergnat Tested-by: Alexandre Mergnat Signed-off-by: AngeloGioacchino Del Regno Acked-by: Sui Jingfeng
Re: [v4,1/2] drm/bridge: sii902x: Fix mode_valid hook
Hi, Jayesh
On 5/31/24 21:33, Sam Ravnborg wrote:
Hi Jayesh,
+
static const struct drm_bridge_funcs sii902x_bridge_funcs = {
.attach = sii902x_bridge_attach,
.mode_set = sii902x_bridge_mode_set,
@@ -516,6 +529,7 @@ static const struct drm_bridge_funcs sii902x_bridge_funcs =
{
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
.atomic_get_input_bus_fmts = sii902x_bridge_atomic_get_input_bus_fmts,
.atomic_check = sii902x_bridge_atomic_check,
+ .mode_valid = sii902x_bridge_mode_valid,
As you have the possibility to test the driver, it would be nice with a
follow-up patch that replaces the use of enable() / disable() with the
atomic counterparts.
enable() / disable() are deprecated, so it is nice to reduce their use.
I agree with Sam.
Please using atomic uniformally with a follow-up patch, the mixed
using of atomic API and non atomic API is a little bit confusing IMO.
Sam
Re: [v4,1/2] drm/bridge: sii902x: Fix mode_valid hook
Hi,
On 5/30/24 17:29, Jayesh Choudhary wrote:
Currently, mode_valid hook returns all mode as valid and it is
defined only in drm_connector_helper_funcs. With the introduction of
'DRM_BRIDGE_ATTACH_NO_CONNECTOR', connector is not initialized in
bridge_attach call for cases when the encoder has this flag enabled.
So move the mode_valid hook to drm_bridge_funcs with proper clock
checks for maximum and minimum pixel clock supported by the bridge.
Signed-off-by: Jayesh Choudhary
Acked-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/sii902x.c | 32 +++-
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 2fbeda9025bf..6a6055a4ccf9 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -163,6 +163,14 @@
#define SII902X_AUDIO_PORT_INDEX 3
+/*
+ * The maximum resolution supported by the HDMI bridge is 1080p@60Hz
+ * and 1920x1200 requiring a pixel clock of 165MHz and the minimum
+ * resolution supported is 480p@60Hz requiring a pixel clock of 25MHz
+ */
+#define SII902X_MIN_PIXEL_CLOCK_KHZ25000
+#define SII902X_MAX_PIXEL_CLOCK_KHZ165000
+
This bridge can drive 2560x1080@75Hz monitor(LG 34BL650), the pixel
clock can up to 181250 kHz. I remember that I have tested the native
mode with LS2K1000 SoC, and it do works in practice. And there are
also has 320x240 panels, maybe it's also usuable with this HDMI
transmitter
Well, the datasheet mentioned that it supports up to 165 MHz
dual-edge and single-edge modes. So I'm not against your patch,
just mention it to let you know.
struct sii902x {
struct i2c_client *i2c;
struct regmap *regmap;
@@ -310,17 +318,8 @@ static int sii902x_get_modes(struct drm_connector
*connector)
return num;
}
-static enum drm_mode_status sii902x_mode_valid(struct drm_connector *connector,
- struct drm_display_mode *mode)
-{
- /* TODO: check mode */
-
- return MODE_OK;
-}
-
static const struct drm_connector_helper_funcs sii902x_connector_helper_funcs
= {
.get_modes = sii902x_get_modes,
- .mode_valid = sii902x_mode_valid,
};
static void sii902x_bridge_disable(struct drm_bridge *bridge)
@@ -504,6 +503,20 @@ static int sii902x_bridge_atomic_check(struct drm_bridge
*bridge,
return 0;
}
+static enum drm_mode_status
+sii902x_bridge_mode_valid(struct drm_bridge *bridge,
+ const struct drm_display_info *info,
+ const struct drm_display_mode *mode)
+{
+ if (mode->clock < SII902X_MIN_PIXEL_CLOCK_KHZ)
+ return MODE_CLOCK_LOW;
+
+ if (mode->clock > SII902X_MAX_PIXEL_CLOCK_KHZ)
+ return MODE_CLOCK_HIGH;
+
+ return MODE_OK;
+}
+
static const struct drm_bridge_funcs sii902x_bridge_funcs = {
.attach = sii902x_bridge_attach,
.mode_set = sii902x_bridge_mode_set,
@@ -516,6 +529,7 @@ static const struct drm_bridge_funcs sii902x_bridge_funcs =
{
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
.atomic_get_input_bus_fmts = sii902x_bridge_atomic_get_input_bus_fmts,
.atomic_check = sii902x_bridge_atomic_check,
+ .mode_valid = sii902x_bridge_mode_valid,
};
static int sii902x_mute(struct sii902x *sii902x, bool mute)
--
Best regards
Sui
Re: [PATCH v4 0/2] Add mode_valid and atomic_check hooks for sii902x bridge
Hi, On 5/30/24 17:29, Jayesh Choudhary wrote: Move the mode_valid hook to drm_bridge_funcs structure to take care of the case when the encoder attaches the bridge chain with the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag in which case, the connector is not initialized in the bridge's attach call and mode_valid is not called. Good catch, as modes being supported is actually a capability of the bridge itself. The move make the driver reflect the hardware more.
Re: MAINTAINERS: drm: Drop sam as panel reviewer
Hi, On 5/31/24 05:14, Sam Ravnborg wrote: Drop myself as reviewer of panel patches, to reflect the reality. We lost one kindness reviewer for drivers of panel, unhappy! Not sure if it is proper to give you a NAK here. :( Best regards, Sui Signed-off-by: Sam Ravnborg Cc: Neil Armstrong --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index ac285040578e..38978699bef5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7567,7 +7567,6 @@ F:include/drm/gpu_scheduler.h DRM PANEL DRIVERS M:Neil Armstrong R:Jessica Zhang -R: Sam Ravnborg L:dri-devel@lists.freedesktop.org S:Maintained T:git https://gitlab.freedesktop.org/drm/misc/kernel.git
Re: [resend,v2] drm: renesas: shmobile: Call drm_helper_force_disable_all() at shutdown time
Hi, On 2024/5/29 20:16, Geert Uytterhoeven wrote: From: Douglas Anderson Based on grepping through the source code, this driver appears to be missing a call to drm_atomic_helper_shutdown() at system shutdown time. This is important because drm_helper_force_disable_all() will cause panels to get disabled cleanly which may be important for their power sequencing. Future changes will remove any custom powering off in individual panel drivers so the DRM drivers need to start getting this right. The fact that we should call drm_atomic_helper_shutdown() in the case of OS shutdown comes straight out of the kernel doc "driver instance overview" in drm_drv.c. True, looks safer. Suggested-by: Maxime Ripard Signed-off-by: Douglas Anderson Link: https://lore.kernel.org/r/20230901164111.RFT.15.Iaf638a1d4c8b3c307a6192efabb4cbb06b195f15@changeid [geert: s/drm_helper_force_disable_all/drm_atomic_helper_shutdown/] [geert: shmob_drm_remove() already calls drm_atomic_helper_shutdown] Signed-off-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart Reviewed-by: Sui Jingfeng Best regards, Sui
Re: drm: renesas: rcar-du: Add drm_panic support for non-vsp
Hi,
On 5/27/24 21:35, Geert Uytterhoeven wrote:
Add support for the drm_panic module for DU variants not using the
VSP-compositor, to display a message on the screen when a kernel panic
occurs.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Jocelyn Falempe
After all concerns resolved:
Acked-by: Sui Jingfeng
---
Tested on Koelsch (R-Car M2-W).
Support for DU variants using the VSP-compositor is more convoluted,
and left to the DU experts.
---
drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
b/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
index e445fac8e0b46c21..c546ab0805d656f6 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_plane.c
@@ -680,6 +680,12 @@ static const struct drm_plane_helper_funcs
rcar_du_plane_helper_funcs = {
.atomic_update = rcar_du_plane_atomic_update,
};
+static const struct drm_plane_helper_funcs rcar_du_primary_plane_helper_funcs = {
+ .atomic_check = rcar_du_plane_atomic_check,
+ .atomic_update = rcar_du_plane_atomic_update,
+ .get_scanout_buffer = drm_fb_dma_get_scanout_buffer,
+};
+
static struct drm_plane_state *
rcar_du_plane_atomic_duplicate_state(struct drm_plane *plane)
{
@@ -812,8 +818,12 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
if (ret < 0)
return ret;
- drm_plane_helper_add(>plane,
-_du_plane_helper_funcs);
+ if (type == DRM_PLANE_TYPE_PRIMARY)
+ drm_plane_helper_add(>plane,
+
_du_primary_plane_helper_funcs);
+ else
+ drm_plane_helper_add(>plane,
+_du_plane_helper_funcs);
Maybe we could do some untangle, but this is not a strong requirement.
Thanks.
Best regards
Sui
drm_plane_create_alpha_property(>plane);
Re: drm: renesas: shmobile: Add drm_panic support
Hi,
On 5/27/24 21:34, Geert Uytterhoeven wrote:
Add support for the drm_panic module, which displays a message on
the screen when a kernel panic occurs.
Signed-off-by: Geert Uytterhoeven
Reviewed-by: Jocelyn Falempe
Acked-by: Sui Jingfeng
---
Tested on Armadillo-800-EVA.
---
drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
index 07ad17d24294d5e6..9d166ab2af8bd231 100644
--- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
+++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
@@ -273,6 +273,13 @@ static const struct drm_plane_helper_funcs
shmob_drm_plane_helper_funcs = {
.atomic_disable = shmob_drm_plane_atomic_disable,
};
+static const struct drm_plane_helper_funcs shmob_drm_primary_plane_helper_funcs = {
+ .atomic_check = shmob_drm_plane_atomic_check,
+ .atomic_update = shmob_drm_plane_atomic_update,
+ .atomic_disable = shmob_drm_plane_atomic_disable,
+ .get_scanout_buffer = drm_fb_dma_get_scanout_buffer,
+};
+
static const struct drm_plane_funcs shmob_drm_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
@@ -310,7 +317,12 @@ struct drm_plane *shmob_drm_plane_create(struct
shmob_drm_device *sdev,
Maybe a shmob_drm_plane_create_primary_plane() plus a
shmob_drm_plane_create_overlay().
I remember Thomas told this way or something similiar, call untangle.
splane->index = index;
- drm_plane_helper_add(>base, _drm_plane_helper_funcs);
+ if (type == DRM_PLANE_TYPE_PRIMARY)
+ drm_plane_helper_add(>base,
+_drm_primary_plane_helper_funcs);
+ else
+ drm_plane_helper_add(>base,
+_drm_plane_helper_funcs);
return >base;
}
Anyway, it looks good to me.
Best regards
Sui
Re: drm/tests: Add a missing Kconfig select
Hi,
On 5/29/24 17:19, Thomas Hellström wrote:
Fix the following warning:
WARNING: unmet direct dependencies detected for DRM_DISPLAY_HDMI_STATE_HELPER
Depends on [n]: HAS_IOMEM [=y] && DRM [=y] && DRM_DISPLAY_HELPER [=y] &&
DRM_DISPLAY_HDMI_HELPER [=n]
Selected by [y]:
- DRM_KUNIT_TEST [=y] && HAS_IOMEM [=y] && DRM [=y] && KUNIT [=y] && MMU [=y]
Signed-off-by: Thomas Hellström
Fixes: 54cb39e2293b ("drm/connector: hdmi: Create an HDMI sub-state")
Cc: Maxime Ripard
Cc: dri-devel@lists.freedesktop.org
Acked-by: Sui Jingfeng
Re: [v15,06/29] drm/tests: Add output bpc tests
Hi, On 5/27/24 21:57, Maxime Ripard wrote: Now that we're tracking the output bpc count in the connector state, let's add a few tests to make sure it works as expected. Reviewed-by: Dave Stevenson Reviewed-by: Dmitry Baryshkov Signed-off-by: Maxime Ripard Tested-by: Sui Jingfeng
Re: [v15,06/29] drm/tests: Add output bpc tests
atomic_helper_connector_hdmi_reset",
+ .test_cases = drm_atomic_helper_connector_hdmi_reset_tests,
+};
+
+kunit_test_suites(
+ _atomic_helper_connector_hdmi_check_test_suite,
+ _atomic_helper_connector_hdmi_reset_test_suite,
+);
+
+MODULE_AUTHOR("Maxime Ripard ");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/tests/drm_kunit_edid.h
b/drivers/gpu/drm/tests/drm_kunit_edid.h
new file mode 100644
index ..0366dd29c820
--- /dev/null
+++ b/drivers/gpu/drm/tests/drm_kunit_edid.h
@@ -0,0 +1,106 @@
+#ifndef DRM_KUNIT_EDID_H_
+#define DRM_KUNIT_EDID_H_
+
+/*
+ * edid-decode (hex):
+ *
+ * 00 ff ff ff ff ff ff 00 31 d8 2a 00 00 00 00 00
+ * 00 21 01 03 81 a0 5a 78 02 00 00 00 00 00 00 00
+ * 00 00 00 20 00 00 01 01 01 01 01 01 01 01 01 01
+ * 01 01 01 01 01 01 02 3a 80 18 71 38 2d 40 58 2c
+ * 45 00 40 84 63 00 00 1e 00 00 00 fc 00 54 65 73
+ * 74 20 45 44 49 44 0a 20 20 20 00 00 00 fd 00 32
+ * 46 1e 46 0f 00 0a 20 20 20 20 20 20 00 00 00 10
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 92
+ *
+ * 02 03 1b 81 e3 05 00 20 41 10 e2 00 4a 6d 03 0c
+ * 00 12 34 00 28 20 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ * 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 d0
+ *
+ *
+ *
+ * Block 0, Base EDID:
+ * EDID Structure Version & Revision: 1.3
+ * Vendor & Product Identification:
+ * Manufacturer: LNX
+ * Model: 42
+ * Made in: 2023
+ * Basic Display Parameters & Features:
+ * Digital display
+ * DFP 1.x compatible TMDS
+ * Maximum image size: 160 cm x 90 cm
+ * Gamma: 2.20
+ * Monochrome or grayscale display
+ * First detailed timing is the preferred timing
+ * Color Characteristics:
+ * Red : 0., 0.
+ * Green: 0., 0.
+ * Blue : 0., 0.
+ * White: 0., 0.
+ * Established Timings I & II:
+ * DMT 0x04: 640x48059.940476 Hz 4:3 31.469 kHz 25.175000
MHz
+ * Standard Timings: none
+ * Detailed Timing Descriptors:
+ * DTD 1: 1920x1080 60.00 Hz 16:9 67.500 kHz148.50 MHz
(1600 mm x 900 mm)
+ * Hfront 88 Hsync 44 Hback 148 Hpol P
+ * Vfront4 Vsync 5 Vback 36 Vpol P
+ * Display Product Name: 'Test EDID'
+ * Display Range Limits:
+ * Monitor ranges (GTF): 50-70 Hz V, 30-70 kHz H, max dotclock 150 MHz
+ * Dummy Descriptor:
+ * Extension blocks: 1
+ * Checksum: 0x92
+ *
+ *
+ *
+ * Block 1, CTA-861 Extension Block:
+ * Revision: 3
+ * Underscans IT Video Formats by default
+ * Native detailed modes: 1
+ * Colorimetry Data Block:
+ * sRGB
+ * Video Data Block:
+ * VIC 16: 1920x1080 60.00 Hz 16:9 67.500 kHz148.50
MHz
+ * Video Capability Data Block:
+ * YCbCr quantization: No Data
+ * RGB quantization: Selectable (via AVI Q)
+ * PT scan behavior: No Data
+ * IT scan behavior: Always Underscanned
+ * CE scan behavior: Always Underscanned
+ * Vendor-Specific Data Block (HDMI), OUI 00-0C-03:
+ * Source physical address: 1.2.3.4
+ * Maximum TMDS clock: 200 MHz
+ * Extended HDMI video details:
+ * Checksum: 0xd0 Unused space in Extension Block: 100 bytes
+ */
+static const unsigned char test_edid_hdmi_1080p_rgb_max_200mhz[] = {
+ 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x31, 0xd8, 0x2a, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x03, 0x81, 0xa0, 0x5a, 0x78,
+ 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20,
+ 0x00, 0x00, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01,
+ 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x02, 0x3a, 0x80, 0x18, 0x71, 0x38,
+ 0x2d, 0x40, 0x58, 0x2c, 0x45, 0x00, 0x40, 0x84, 0x63, 0x00, 0x00, 0x1e,
+ 0x00, 0x00, 0x00, 0xfc, 0x00, 0x54, 0x65, 0x73, 0x74, 0x20, 0x45, 0x44,
+ 0x49, 0x44, 0x0a, 0x20, 0x20, 0x20, 0x00, 0x00, 0x00, 0xfd, 0x00, 0x32,
+ 0x46, 0x00, 0x00, 0xc4, 0x00, 0x0a, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+ 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x41, 0x02, 0x03, 0x1b, 0x81,
+ 0xe3, 0x05, 0x00, 0x20, 0x41, 0x10, 0xe2, 0x00, 0x4a, 0x6d, 0x03, 0x0c,
+ 0x00, 0x12, 0x34, 0x00, 0x28, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0xd0
+};
+
+#endif // DRM_KUNIT_EDID_H_
--
Best regards
Sui Jingfeng
Re: [PATCH v6 02/10] drm/bridge: Set firmware node of drm_bridge instances automatically
Hi,
On 5/27/24 07:33, kernel test robot wrote:
Hi Sui,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-exynos/exynos-drm-next]
[also build test ERROR on linus/master v6.10-rc1 next-20240523]
[cannot apply to shawnguo/for-next]
[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/Sui-Jingfeng/drm-bridge-Allow-using-fwnode-APIs-to-get-the-next-bridge/20240527-042402
base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
exynos-drm-next
patch link:
https://lore.kernel.org/r/20240526202115.129049-3-sui.jingfeng%40linux.dev
patch subject: [PATCH v6 02/10] drm/bridge: Set firmware node of drm_bridge
instances automatically
config: arm-defconfig
(https://download.01.org/0day-ci/archive/20240527/202405270622.vdmbp9fr-...@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project
f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240527/202405270622.vdmbp9fr-...@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
| Closes:
https://lore.kernel.org/oe-kbuild-all/202405270622.vdmbp9fr-...@intel.com/
All errors (new ones prefixed by >>):
drivers/gpu/drm/omapdrm/dss/hdmi5.c:487:49: error: expected identifier
drm_bridge_add(>bridge, >pdev->dev.);
^
1 error generated.
vim +487 drivers/gpu/drm/omapdrm/dss/hdmi5.c
480
481 static void hdmi5_bridge_init(struct omap_hdmi *hdmi)
482 {
483 hdmi->bridge.funcs = _bridge_funcs;
484 hdmi->bridge.ops = DRM_BRIDGE_OP_EDID;
485 hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
486
> 487 drm_bridge_add(>bridge, >pdev->dev.);
488 }
489
Sorry, my bad. I have do compile test on ARM64 before posting.
checkpatch.pl report a style problem, then I manually modify this
patch, accidentally add the tail '.' character. Will be fixed
at the next version.
--
Best regards
Sui
Re: [PATCH v6 02/10] drm/bridge: Set firmware node of drm_bridge instances automatically
Hi,
On 5/27/24 05:19, Dmitry Baryshkov wrote:
On Mon, May 27, 2024 at 04:21:07AM +0800, Sui Jingfeng wrote:
Normally, the drm_bridge::of_node won't be used by bridge driver instances
themselves. Rather, it is mainly used by other modules to find associated
drm bridge drvier. Therefore, adding a drm bridge to the global bridge list
and setting 'of_node' field of a drm bridge share the same goal. Both are
for finding purpose, therefore better to group them to one function.
Update the drm_bridge_add() interface and implementation to achieve such
goal atomically, new implementation will fetch the device node from the
backing device of the drm bridge driver automatically. For the majority
cases, which is one device backing one drm bridge driver, this model works
well. Drivers still can set it manually by passing NULL if this model
doesn't fit.
While at it, Add a 'struct device *' pointer to the drm_bridge structure.
As it already being passed in by both of drm_bridge_add() and
devm_drm_bridge_add(). A lot of driver instances has already added it into
their derived structure, promote it into drm_bridge core helps to reduce
a batch of boilerplates.
Signed-off-by: Sui Jingfeng
---
[trimmed]
@@ -231,7 +243,7 @@ static void drm_bridge_remove_void(void *bridge)
*/
int devm_drm_bridge_add(struct device *dev, struct drm_bridge *bridge)
{
- drm_bridge_add(bridge);
+ drm_bridge_add(bridge, dev);
return devm_add_action_or_reset(dev, drm_bridge_remove_void, bridge);
This breaks aux-hpd-bridge, which gets of_node as an external pointer
rather than dev->of_node.
Yes, you are right. I forget to modify that driver.
My bad, will be fixed at the next version.
--
Best regards
Sui
[PATCH v6 10/10] drm/bridge: ch7033: Switch to use fwnode based APIs
Use the freshly created helper to replace the use of DT-dependent APIs,
also print error log if the fwnode graph is not complete which is benefit
to debug.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/chrontel-ch7033.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/chrontel-ch7033.c
b/drivers/gpu/drm/bridge/chrontel-ch7033.c
index c6374440af7f..35dd2e6ba6c0 100644
--- a/drivers/gpu/drm/bridge/chrontel-ch7033.c
+++ b/drivers/gpu/drm/bridge/chrontel-ch7033.c
@@ -531,6 +531,7 @@ static const struct regmap_config ch7033_regmap_config = {
static int ch7033_probe(struct i2c_client *client)
{
struct device *dev = >dev;
+ struct fwnode_handle *fwnode = dev_fwnode(dev);
struct ch7033_priv *priv;
unsigned int val;
int ret;
@@ -541,10 +542,15 @@ static int ch7033_probe(struct i2c_client *client)
dev_set_drvdata(dev, priv);
- ret = drm_of_find_panel_or_bridge(dev->of_node, 1, -1, NULL,
- >next_bridge);
- if (ret)
+ priv->next_bridge = drm_bridge_find_next_bridge_by_fwnode(fwnode, 1);
+ if (IS_ERR(priv->next_bridge)) {
+ ret = PTR_ERR(priv->next_bridge);
+ dev_err(dev, "Error in founding the next bridge: %d\n", ret);
return ret;
+ } else if (!priv->next_bridge) {
+ dev_dbg(dev, "Next bridge not found, deferring probe\n");
+ return -EPROBE_DEFER;
+ }
priv->regmap = devm_regmap_init_i2c(client, _regmap_config);
if (IS_ERR(priv->regmap)) {
--
2.34.1
[PATCH v6 08/10] drm/bridge: tfp410: Use fwnode APIs to acquire device properties
Make this driver less DT-dependent by calling the newly created helpers,
also switch to use fwnode APIs to acquire additional device properties.
No functional changes for DT-based systems.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/ti-tfp410.c | 39 +++---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c
b/drivers/gpu/drm/bridge/ti-tfp410.c
index 04a341133488..a1fae5e9dafd 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -261,8 +261,9 @@ static const struct drm_bridge_timings
tfp410_default_timings = {
static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
{
+ struct fwnode_handle *fwnode = dev_fwnode(dvi->dev);
struct drm_bridge_timings *timings = >timings;
- struct device_node *ep;
+ struct fwnode_handle *ep;
u32 pclk_sample = 0;
u32 bus_width = 24;
u32 deskew = 0;
@@ -283,14 +284,14 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool
i2c)
* and EDGE pins. They are specified in DT through endpoint properties
* and vendor-specific properties.
*/
- ep = of_graph_get_endpoint_by_regs(dvi->dev->of_node, 0, 0);
+ ep = fwnode_graph_get_endpoint_by_id(fwnode, 0, 0, 0);
if (!ep)
return -EINVAL;
/* Get the sampling edge from the endpoint. */
- of_property_read_u32(ep, "pclk-sample", _sample);
- of_property_read_u32(ep, "bus-width", _width);
- of_node_put(ep);
+ fwnode_property_read_u32(ep, "pclk-sample", _sample);
+ fwnode_property_read_u32(ep, "bus-width", _width);
+ fwnode_handle_put(ep);
timings->input_bus_flags = DRM_BUS_FLAG_DE_HIGH;
@@ -319,7 +320,7 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool
i2c)
}
/* Get the setup and hold time from vendor-specific properties. */
- of_property_read_u32(dvi->dev->of_node, "ti,deskew", );
+ fwnode_property_read_u32(fwnode, "ti,deskew", );
if (deskew > 7)
return -EINVAL;
@@ -331,12 +332,12 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool
i2c)
static int tfp410_init(struct device *dev, bool i2c)
{
- struct device_node *node;
+ struct fwnode_handle *fwnode = dev_fwnode(dev);
struct tfp410 *dvi;
int ret;
- if (!dev->of_node) {
- dev_err(dev, "device-tree data is missing\n");
+ if (!fwnode) {
+ dev_err(dev, "firmware data is missing\n");
return -ENXIO;
}
@@ -356,15 +357,15 @@ static int tfp410_init(struct device *dev, bool i2c)
return ret;
/* Get the next bridge, connected to port@1. */
- node = of_graph_get_remote_node(dev->of_node, 1, -1);
- if (!node)
- return -ENODEV;
-
- dvi->next_bridge = of_drm_find_bridge(node);
- of_node_put(node);
-
- if (!dvi->next_bridge)
+ dvi->next_bridge = drm_bridge_find_next_bridge_by_fwnode(fwnode, 1);
+ if (IS_ERR(dvi->next_bridge)) {
+ ret = PTR_ERR(dvi->next_bridge);
+ dev_err(dev, "Error in founding the next bridge: %d\n", ret);
+ return ret;
+ } else if (!dvi->next_bridge) {
+ dev_dbg(dev, "Next bridge not found, deferring probe\n");
return -EPROBE_DEFER;
+ }
/* Get the powerdown GPIO. */
dvi->powerdown = devm_gpiod_get_optional(dev, "powerdown",
@@ -416,10 +417,10 @@ static struct platform_driver tfp410_platform_driver = {
/* There is currently no i2c functionality. */
static int tfp410_i2c_probe(struct i2c_client *client)
{
+ struct fwnode_handle *fwnode = dev_fwnode(>dev);
int reg;
- if (!client->dev.of_node ||
- of_property_read_u32(client->dev.of_node, "reg", )) {
+ if (!fwnode || fwnode_property_read_u32(fwnode, "reg", )) {
dev_err(>dev,
"Can't get i2c reg property from device-tree\n");
return -ENXIO;
--
2.34.1
[PATCH v6 09/10] drm/bridge: sii9234: Use fwnode APIs to abstract DT dependent API away
Switch to use the freshly created drm_bridge_set_node() helper, no
functional changes. The reason behind of this introduction is that
the name 'of_node' itself has a smell of DT dependent, and it is a
internal memeber, when there has helper function, we should use the
revelant helper and avoid directly referencing and/or dereferencing
it.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/sii9234.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
index 7d2bbc31bac9..d930c093abb3 100644
--- a/drivers/gpu/drm/bridge/sii9234.c
+++ b/drivers/gpu/drm/bridge/sii9234.c
@@ -817,10 +817,11 @@ static int sii9234_init_resources(struct sii9234 *ctx,
struct i2c_client *client)
{
struct i2c_adapter *adapter = client->adapter;
+ struct fwnode_handle *fwnode = dev_fwnode(ctx->dev);
int ret;
- if (!ctx->dev->of_node) {
- dev_err(ctx->dev, "not DT device\n");
+ if (!fwnode) {
+ dev_err(ctx->dev, "firmware data is missing\n");
return -ENODEV;
}
--
2.34.1
[PATCH v6 07/10] drm-bridge: it66121: Use fwnode APIs to acquire device properties
Make this driver less DT-dependent by calling the newly created helpers,
also switch to use fwnode APIs to acquire additional device properties.
A side benifit is that boilerplates get reduced, no functional changes
for DT-based systems.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/ite-it66121.c | 55 +---
1 file changed, 34 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c
b/drivers/gpu/drm/bridge/ite-it66121.c
index 8e4fc082bb8c..3fa650f7fec9 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -15,7 +15,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -1480,7 +1479,7 @@ static int it66121_audio_codec_init(struct it66121_ctx
*ctx, struct device *dev)
dev_dbg(dev, "%s\n", __func__);
- if (!of_property_read_bool(dev->of_node, "#sound-dai-cells")) {
+ if (!fwnode_property_present(dev_fwnode(dev), "#sound-dai-cells")) {
dev_info(dev, "No \"#sound-dai-cells\", no audio\n");
return 0;
}
@@ -1503,13 +1502,36 @@ static const char * const it66121_supplies[] = {
"vcn33", "vcn18", "vrf12"
};
+static int it66121_read_bus_width(struct fwnode_handle *fwnode, u32 *bus_width)
+{
+ struct fwnode_handle *endpoint;
+ u32 val;
+ int ret;
+
+ endpoint = fwnode_graph_get_endpoint_by_id(fwnode, 0, 0, 0);
+ if (!endpoint)
+ return -EINVAL;
+
+ ret = fwnode_property_read_u32(endpoint, "bus-width", );
+ fwnode_handle_put(endpoint);
+ if (ret)
+ return ret;
+
+ if (val != 12 && val != 24)
+ return -EINVAL;
+
+ *bus_width = val;
+
+ return 0;
+}
+
static int it66121_probe(struct i2c_client *client)
{
u32 revision_id, vendor_ids[2] = { 0 }, device_ids[2] = { 0 };
- struct device_node *ep;
int ret;
struct it66121_ctx *ctx;
struct device *dev = >dev;
+ struct fwnode_handle *fwnode = dev_fwnode(dev);
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
dev_err(dev, "I2C check functionality failed.\n");
@@ -1520,29 +1542,20 @@ static int it66121_probe(struct i2c_client *client)
if (!ctx)
return -ENOMEM;
- ep = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
- if (!ep)
- return -EINVAL;
-
ctx->dev = dev;
ctx->client = client;
ctx->info = i2c_get_match_data(client);
- of_property_read_u32(ep, "bus-width", >bus_width);
- of_node_put(ep);
-
- if (ctx->bus_width != 12 && ctx->bus_width != 24)
- return -EINVAL;
-
- ep = of_graph_get_remote_node(dev->of_node, 1, -1);
- if (!ep) {
- dev_err(ctx->dev, "The endpoint is unconnected\n");
- return -EINVAL;
- }
+ ret = it66121_read_bus_width(fwnode, >bus_width);
+ if (ret)
+ return ret;
- ctx->next_bridge = of_drm_find_bridge(ep);
- of_node_put(ep);
- if (!ctx->next_bridge) {
+ ctx->next_bridge = drm_bridge_find_next_bridge_by_fwnode(fwnode, 1);
+ if (IS_ERR(ctx->next_bridge)) {
+ ret = PTR_ERR(ctx->next_bridge);
+ dev_err(dev, "Error in founding the next bridge: %d\n", ret);
+ return ret;
+ } else if (!ctx->next_bridge) {
dev_dbg(ctx->dev, "Next bridge not found, deferring probe\n");
return -EPROBE_DEFER;
}
--
2.34.1
[PATCH v6 06/10] drm/bridge: sii902x: Switch to use fwnode APIs to acquire device properties
Make this driver less DT-dependent by calling the freshly created helpers,
also switch to use fwnode APIs to acquire additional device properties.
One side benifit is that boilerplates get reduced, no functional changes
for DT-based systems.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/sii902x.c | 43 +++-
1 file changed, 15 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index f4808838717a..975511d623a4 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -827,20 +827,17 @@ static int sii902x_audio_codec_init(struct sii902x
*sii902x,
.spdif = 0,
.max_i2s_channels = 0,
};
+ struct fwnode_handle *fwnode = dev_fwnode(dev);
u8 lanes[4];
int num_lanes, i;
- if (!of_property_read_bool(dev->of_node, "#sound-dai-cells")) {
+ if (!fwnode_property_present(fwnode, "#sound-dai-cells")) {
dev_dbg(dev, "%s: No \"#sound-dai-cells\", no audio\n",
__func__);
return 0;
}
- num_lanes = of_property_read_variable_u8_array(dev->of_node,
- "sil,i2s-data-lanes",
- lanes, 1,
- ARRAY_SIZE(lanes));
-
+ num_lanes = fwnode_property_count_u8(fwnode, "sil,i2s-data-lanes");
if (num_lanes == -EINVAL) {
dev_dbg(dev,
"%s: No \"sil,i2s-data-lanes\", use default <0>\n",
@@ -852,7 +849,11 @@ static int sii902x_audio_codec_init(struct sii902x
*sii902x,
"%s: Error gettin \"sil,i2s-data-lanes\": %d\n",
__func__, num_lanes);
return num_lanes;
+ } else {
+ fwnode_property_read_u8_array(fwnode, "sil,i2s-data-lanes",
+ lanes, num_lanes);
}
+
codec_data.max_i2s_channels = 2 * num_lanes;
for (i = 0; i < num_lanes; i++)
@@ -1117,7 +1118,6 @@ static int sii902x_init(struct sii902x *sii902x)
static int sii902x_probe(struct i2c_client *client)
{
struct device *dev = >dev;
- struct device_node *endpoint;
struct sii902x *sii902x;
static const char * const supplies[] = {"iovcc", "cvcc12"};
int ret;
@@ -1146,27 +1146,14 @@ static int sii902x_probe(struct i2c_client *client)
return PTR_ERR(sii902x->reset_gpio);
}
- endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
- if (endpoint) {
- struct device_node *remote =
of_graph_get_remote_port_parent(endpoint);
-
- of_node_put(endpoint);
- if (!remote) {
- dev_err(dev, "Endpoint in port@1 unconnected\n");
- return -ENODEV;
- }
-
- if (!of_device_is_available(remote)) {
- dev_err(dev, "port@1 remote device is disabled\n");
- of_node_put(remote);
- return -ENODEV;
- }
-
- sii902x->next_bridge = of_drm_find_bridge(remote);
- of_node_put(remote);
- if (!sii902x->next_bridge)
- return dev_err_probe(dev, -EPROBE_DEFER,
-"Failed to find remote bridge\n");
+ sii902x->next_bridge =
drm_bridge_find_next_bridge_by_fwnode(dev_fwnode(dev), 1);
+ if (!sii902x->next_bridge) {
+ return dev_err_probe(dev, -EPROBE_DEFER,
+"Failed to find the next bridge\n");
+ } else if (IS_ERR(sii902x->next_bridge)) {
+ ret = PTR_ERR(sii902x->next_bridge);
+ dev_err(dev, "Error on find the next bridge: %d\n", ret);
+ return ret;
}
mutex_init(>mutex);
--
2.34.1
[PATCH v6 05/10] drm/bridge: display-connector: Use fwnode APIs to acquire device properties
Switch to use the fwnode APIs, which is a fundamental step to make this
driver OF-independent possible. No functional changes for DT-based systems.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/display-connector.c | 23 +++---
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/bridge/display-connector.c
b/drivers/gpu/drm/bridge/display-connector.c
index fb1e97d385fe..7436e7cd53fc 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -9,7 +9,6 @@
#include
#include
#include
-#include
#include
#include
@@ -204,6 +203,7 @@ static int display_connector_get_supply(struct
platform_device *pdev,
static int display_connector_probe(struct platform_device *pdev)
{
+ struct fwnode_handle *fwnode = dev_fwnode(>dev);
struct display_connector *conn;
unsigned int type;
const char *label = NULL;
@@ -215,15 +215,15 @@ static int display_connector_probe(struct platform_device
*pdev)
platform_set_drvdata(pdev, conn);
- type = (uintptr_t)of_device_get_match_data(>dev);
+ type = (uintptr_t)device_get_match_data(>dev);
/* Get the exact connector type. */
switch (type) {
case DRM_MODE_CONNECTOR_DVII: {
bool analog, digital;
- analog = of_property_read_bool(pdev->dev.of_node, "analog");
- digital = of_property_read_bool(pdev->dev.of_node, "digital");
+ analog = fwnode_property_present(fwnode, "analog");
+ digital = fwnode_property_present(fwnode, "digital");
if (analog && !digital) {
conn->bridge.type = DRM_MODE_CONNECTOR_DVIA;
} else if (!analog && digital) {
@@ -240,8 +240,7 @@ static int display_connector_probe(struct platform_device
*pdev)
case DRM_MODE_CONNECTOR_HDMIA: {
const char *hdmi_type;
- ret = of_property_read_string(pdev->dev.of_node, "type",
- _type);
+ ret = fwnode_property_read_string(fwnode, "type", _type);
if (ret < 0) {
dev_err(>dev, "HDMI connector with no type\n");
return -EINVAL;
@@ -271,7 +270,7 @@ static int display_connector_probe(struct platform_device
*pdev)
conn->bridge.interlace_allowed = true;
/* Get the optional connector label. */
- of_property_read_string(pdev->dev.of_node, "label", );
+ fwnode_property_read_string(fwnode, "label", );
/*
* Get the HPD GPIO for DVI, HDMI and DP connectors. If the GPIO can
provide
@@ -309,12 +308,12 @@ static int display_connector_probe(struct platform_device
*pdev)
if (type == DRM_MODE_CONNECTOR_DVII ||
type == DRM_MODE_CONNECTOR_HDMIA ||
type == DRM_MODE_CONNECTOR_VGA) {
- struct device_node *phandle;
+ struct fwnode_handle *phandle;
- phandle = of_parse_phandle(pdev->dev.of_node, "ddc-i2c-bus", 0);
- if (phandle) {
- conn->bridge.ddc = of_get_i2c_adapter_by_node(phandle);
- of_node_put(phandle);
+ phandle = fwnode_find_reference(fwnode, "ddc-i2c-bus", 0);
+ if (!IS_ERR(phandle)) {
+ conn->bridge.ddc = i2c_get_adapter_by_fwnode(phandle);
+ fwnode_handle_put(phandle);
if (!conn->bridge.ddc)
return -EPROBE_DEFER;
} else {
--
2.34.1
[PATCH v6 02/10] drm/bridge: Set firmware node of drm_bridge instances automatically
Normally, the drm_bridge::of_node won't be used by bridge driver instances themselves. Rather, it is mainly used by other modules to find associated drm bridge drvier. Therefore, adding a drm bridge to the global bridge list and setting 'of_node' field of a drm bridge share the same goal. Both are for finding purpose, therefore better to group them to one function. Update the drm_bridge_add() interface and implementation to achieve such goal atomically, new implementation will fetch the device node from the backing device of the drm bridge driver automatically. For the majority cases, which is one device backing one drm bridge driver, this model works well. Drivers still can set it manually by passing NULL if this model doesn't fit. While at it, Add a 'struct device *' pointer to the drm_bridge structure. As it already being passed in by both of drm_bridge_add() and devm_drm_bridge_add(). A lot of driver instances has already added it into their derived structure, promote it into drm_bridge core helps to reduce a batch of boilerplates. Signed-off-by: Sui Jingfeng --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 +-- .../gpu/drm/bridge/analogix/analogix-anx6345.c | 4 +--- .../gpu/drm/bridge/analogix/analogix-anx78xx.c | 4 +--- drivers/gpu/drm/bridge/analogix/anx7625.c| 3 +-- drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 3 +-- .../gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 3 +-- drivers/gpu/drm/bridge/chipone-icn6211.c | 5 ++--- drivers/gpu/drm/bridge/chrontel-ch7033.c | 3 +-- drivers/gpu/drm/bridge/cros-ec-anx7688.c | 4 +--- drivers/gpu/drm/bridge/display-connector.c | 3 +-- drivers/gpu/drm/bridge/fsl-ldb.c | 3 +-- drivers/gpu/drm/bridge/imx/imx-ldb-helper.c | 3 ++- drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 3 +-- .../gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c | 3 ++- drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c | 3 +-- drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c | 3 +-- drivers/gpu/drm/bridge/ite-it6505.c | 3 +-- drivers/gpu/drm/bridge/ite-it66121.c | 3 +-- drivers/gpu/drm/bridge/lontium-lt8912b.c | 3 +-- drivers/gpu/drm/bridge/lontium-lt9211.c | 3 +-- drivers/gpu/drm/bridge/lontium-lt9611.c | 3 +-- drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 3 +-- drivers/gpu/drm/bridge/lvds-codec.c | 3 +-- .../drm/bridge/megachips-stdp-ge-b850v3-fw.c | 3 +-- drivers/gpu/drm/bridge/microchip-lvds.c | 3 +-- drivers/gpu/drm/bridge/nwl-dsi.c | 3 +-- drivers/gpu/drm/bridge/nxp-ptn3460.c | 3 +-- drivers/gpu/drm/bridge/panel.c | 3 +-- drivers/gpu/drm/bridge/parade-ps8622.c | 3 +-- drivers/gpu/drm/bridge/parade-ps8640.c | 1 - drivers/gpu/drm/bridge/samsung-dsim.c| 3 +-- drivers/gpu/drm/bridge/sii902x.c | 3 +-- drivers/gpu/drm/bridge/sii9234.c | 3 +-- drivers/gpu/drm/bridge/sil-sii8620.c | 3 +-- drivers/gpu/drm/bridge/simple-bridge.c | 3 +-- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 3 +-- drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c| 3 +-- drivers/gpu/drm/bridge/tc358762.c| 3 +-- drivers/gpu/drm/bridge/tc358764.c| 3 +-- drivers/gpu/drm/bridge/tc358767.c| 3 +-- drivers/gpu/drm/bridge/tc358768.c| 3 +-- drivers/gpu/drm/bridge/tc358775.c| 3 +-- drivers/gpu/drm/bridge/thc63lvd1024.c| 3 +-- drivers/gpu/drm/bridge/ti-dlpc3433.c | 3 +-- drivers/gpu/drm/bridge/ti-sn65dsi83.c| 3 +-- drivers/gpu/drm/bridge/ti-sn65dsi86.c| 3 +-- drivers/gpu/drm/bridge/ti-tfp410.c | 3 +-- drivers/gpu/drm/bridge/ti-tpd12s015.c| 3 +-- drivers/gpu/drm/drm_bridge.c | 16 ++-- drivers/gpu/drm/exynos/exynos_drm_mic.c | 3 +-- drivers/gpu/drm/i2c/tda998x_drv.c| 5 + drivers/gpu/drm/mcde/mcde_dsi.c | 3 +-- drivers/gpu/drm/mediatek/mtk_dsi.c | 3 +-- drivers/gpu/drm/mediatek/mtk_hdmi.c | 3 +-- drivers/gpu/drm/meson/meson_encoder_cvbs.c | 3 +-- drivers/gpu/drm/meson/meson_encoder_dsi.c| 3 +-- drivers/gpu/drm/meson/meson_encoder_hdmi.c | 3 +-- drivers/gpu/drm/omapdrm/dss/dpi.c| 3 +-- drivers/gpu/drm/omapdrm/dss/dsi.c| 3 +-- drivers/gpu/drm/omapdrm/dss/hdmi4.c | 3 +-- drivers/gpu/drm/omapdrm/dss/hdmi5.c | 3 +-- drivers/gpu/drm/omapdrm/dss/sdi.c| 3 +-- drivers/gpu/drm/omapdrm/dss/venc.c | 3 +-- drivers/gpu/drm/renesas/rcar-du/rcar_lvds.c | 3 +-- drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c | 3 +-- drivers/gpu/drm/renesas/rcar-du/rzg2l_mipi_dsi.c | 3
[PATCH v6 04/10] drm/bridge: simple-bridge: Use fwnode APIs to acquire device properties
Make this driver less DT-dependent by calling the newly created helpers,
also switch to use fwnode APIs to acquire additional device properties.
A side benifit is that boilerplates get reduced, no functional changes
for DT-based systems.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/simple-bridge.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/bridge/simple-bridge.c
b/drivers/gpu/drm/bridge/simple-bridge.c
index fd7de9b6f80e..e7348ee6daf7 100644
--- a/drivers/gpu/drm/bridge/simple-bridge.c
+++ b/drivers/gpu/drm/bridge/simple-bridge.c
@@ -8,8 +8,6 @@
#include
#include
-#include
-#include
#include
#include
@@ -164,33 +162,32 @@ static const struct drm_bridge_funcs
simple_bridge_bridge_funcs = {
static int simple_bridge_probe(struct platform_device *pdev)
{
+ struct fwnode_handle *fwnode = dev_fwnode(>dev);
struct simple_bridge *sbridge;
- struct device_node *remote;
+ int ret;
sbridge = devm_kzalloc(>dev, sizeof(*sbridge), GFP_KERNEL);
if (!sbridge)
return -ENOMEM;
platform_set_drvdata(pdev, sbridge);
- sbridge->info = of_device_get_match_data(>dev);
+ sbridge->info = device_get_match_data(>dev);
/* Get the next bridge in the pipeline. */
- remote = of_graph_get_remote_node(pdev->dev.of_node, 1, -1);
- if (!remote)
- return -EINVAL;
-
- sbridge->next_bridge = of_drm_find_bridge(remote);
- of_node_put(remote);
-
+ sbridge->next_bridge = drm_bridge_find_next_bridge_by_fwnode(fwnode, 1);
if (!sbridge->next_bridge) {
dev_dbg(>dev, "Next bridge not found, deferring probe\n");
return -EPROBE_DEFER;
+ } else if (IS_ERR(sbridge->next_bridge)) {
+ ret = PTR_ERR(sbridge->next_bridge);
+ dev_err(>dev, "Error on finding the next bridge: %d\n",
ret);
+ return ret;
}
/* Get the regulator and GPIO resources. */
sbridge->vdd = devm_regulator_get_optional(>dev, "vdd");
if (IS_ERR(sbridge->vdd)) {
- int ret = PTR_ERR(sbridge->vdd);
+ ret = PTR_ERR(sbridge->vdd);
if (ret == -EPROBE_DEFER)
return -EPROBE_DEFER;
sbridge->vdd = NULL;
--
2.34.1
[PATCH v6 03/10] drm/bridge: Implement of_drm_find_bridge() on the top of drm_bridge_find_by_fwnode()
Before applying this patch, people may worry about the OF and non-OF API
will have a risk to diverge. Eliminate the risk by reimplement the
of_drm_find_bridge() on the top of drm_bridge_find_by_fwnode(). As for now
the fundamental searching method is unique.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/drm_bridge.c | 29 -
include/drm/drm_bridge.h | 14 +-
2 files changed, 5 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 7759ca066db4..4c5584922d3c 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1346,35 +1346,6 @@ void drm_bridge_hpd_notify(struct drm_bridge *bridge,
}
EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify);
-#ifdef CONFIG_OF
-/**
- * of_drm_find_bridge - find the bridge corresponding to the device node in
- * the global bridge list
- *
- * @np: device node
- *
- * RETURNS:
- * drm_bridge control struct on success, NULL on failure
- */
-struct drm_bridge *of_drm_find_bridge(struct device_node *np)
-{
- struct drm_bridge *bridge;
-
- mutex_lock(_lock);
-
- list_for_each_entry(bridge, _list, list) {
- if (bridge->of_node == np) {
- mutex_unlock(_lock);
- return bridge;
- }
- }
-
- mutex_unlock(_lock);
- return NULL;
-}
-EXPORT_SYMBOL(of_drm_find_bridge);
-#endif
-
/**
* drm_bridge_find_by_fwnode - Find the bridge corresponding to the fwnode
*
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 7b592cf30340..8d743dfe782c 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -791,21 +791,17 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct
drm_bridge *bridge,
struct drm_bridge *previous,
enum drm_bridge_attach_flags flags);
-#ifdef CONFIG_OF
-struct drm_bridge *of_drm_find_bridge(struct device_node *np);
-#else
-static inline struct drm_bridge *of_drm_find_bridge(struct device_node *np)
-{
- return NULL;
-}
-#endif
-
struct drm_bridge *
drm_bridge_find_by_fwnode(struct fwnode_handle *fwnode);
struct drm_bridge *
drm_bridge_find_next_bridge_by_fwnode(struct fwnode_handle *fwnode, u32 port);
+static inline struct drm_bridge *of_drm_find_bridge(struct device_node *np)
+{
+ return drm_bridge_find_by_fwnode(of_fwnode_handle(np));
+}
+
/**
* drm_bridge_get_next_bridge() - Get the next bridge in the chain
* @bridge: bridge object
--
2.34.1
[PATCH v6 01/10] drm/bridge: Allow using fwnode APIs to get the next bridge
The various display bridge drivers rely on 'OF' infrastructures to
works very well, yet there are some platforms and/or devices lack of
'OF' support. Such as virtual display drivers, USB display apapters
and ACPI based systems etc.
Add fwnode based helpers to fill the niche, this allows part of display
bridge drivers to work across systems. As the fwnode APIs has wider
coverage than DT counterpart, and fwnode graphs are compatible with
OF graphs. So the newly created helpers can be used on all systems
in theory, assumed that there has valid OF/fwnode graphs established.
Note, the involved drm bridge instance should also has the fwnode
assigned, so that the user of it could find it via the fwnode handle.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/drm_bridge.c | 74
include/drm/drm_bridge.h | 11 +-
2 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 584d109330ab..cef5bc88ee60 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1363,6 +1363,80 @@ struct drm_bridge *of_drm_find_bridge(struct device_node
*np)
EXPORT_SYMBOL(of_drm_find_bridge);
#endif
+/**
+ * drm_bridge_find_by_fwnode - Find the bridge corresponding to the fwnode
+ *
+ * @fwnode: fwnode for which to find the matching drm_bridge
+ *
+ * This function looks up a drm_bridge in the global bridge list, based on
+ * its associated fwnode. Drivers who want to use this function should has
+ * fwnode handle assigned to the fwnode member of the struct drm_bridge
+ * instance.
+ *
+ * Returns:
+ * * A reference to the requested drm_bridge object on success, or
+ * * %NULL otherwise (object does not exist or the driver of requested
+ *bridge not probed yet).
+ */
+struct drm_bridge *drm_bridge_find_by_fwnode(struct fwnode_handle *fwnode)
+{
+ struct drm_bridge *ret = NULL;
+ struct drm_bridge *bridge;
+
+ if (!fwnode)
+ return NULL;
+
+ mutex_lock(_lock);
+
+ list_for_each_entry(bridge, _list, list) {
+ if (bridge->fwnode == fwnode) {
+ ret = bridge;
+ break;
+ }
+ }
+
+ mutex_unlock(_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(drm_bridge_find_by_fwnode);
+
+/**
+ * drm_bridge_find_next_bridge_by_fwnode - Find the next bridge by fwnode
+ * @fwnode: fwnode pointer to the current device.
+ * @port: identifier of the port node of the next bridge is connected.
+ *
+ * This function find the next bridge of the current node the fwnode
+ * pointed to, assumed that the fwnode graph has been well established.
+ *
+ * Returns:
+ * * A reference to the requested drm_bridge object on success, or
+ * * -%ENODEV if the fwnode graph or OF graph is not complete, or
+ * * %NULL if object does not exist or the next bridge is not ready yet.
+ */
+struct drm_bridge *
+drm_bridge_find_next_bridge_by_fwnode(struct fwnode_handle *fwnode, u32 port)
+{
+ struct drm_bridge *bridge;
+ struct fwnode_handle *ep;
+ struct fwnode_handle *remote;
+
+ ep = fwnode_graph_get_endpoint_by_id(fwnode, port, 0, 0);
+ if (!ep)
+ return ERR_PTR(-ENODEV);
+
+ remote = fwnode_graph_get_remote_port_parent(ep);
+ fwnode_handle_put(ep);
+ if (!remote)
+ return ERR_PTR(-ENODEV);
+
+ bridge = drm_bridge_find_by_fwnode(remote);
+ fwnode_handle_put(remote);
+
+ return bridge;
+}
+EXPORT_SYMBOL_GPL(drm_bridge_find_next_bridge_by_fwnode);
+
MODULE_AUTHOR("Ajay Kumar ");
MODULE_DESCRIPTION("DRM bridge infrastructure");
MODULE_LICENSE("GPL and additional rights");
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4baca0d9107b..725d6dddaf36 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -26,14 +26,13 @@
#include
#include
#include
+#include
#include
#include
#include
#include
-struct device_node;
-
struct drm_bridge;
struct drm_bridge_timings;
struct drm_connector;
@@ -721,6 +720,8 @@ struct drm_bridge {
struct list_head chain_node;
/** @of_node: device node pointer to the bridge */
struct device_node *of_node;
+ /** @fwnode: fwnode pointer to the bridge */
+ struct fwnode_handle *fwnode;
/** @list: to keep track of all added bridges */
struct list_head list;
/**
@@ -797,6 +798,12 @@ static inline struct drm_bridge *of_drm_find_bridge(struct
device_node *np)
}
#endif
+struct drm_bridge *
+drm_bridge_find_by_fwnode(struct fwnode_handle *fwnode);
+
+struct drm_bridge *
+drm_bridge_find_next_bridge_by_fwnode(struct fwnode_handle *fwnode, u32 port);
+
/**
* drm_bridge_get_next_bridge() - Get the next bridge in the chain
* @bridge: bridge object
--
2.34.1
[PATCH v6 00/10] drm/bridge: Allow using fwnode API to get the next bridge
Currently, the various display bridge drivers rely on OF infrastructures to works very well, yet there are platforms and/or devices absence of 'OF' support. Such as virtual display drivers, USB display apapters and ACPI based systems etc. Add fwnode based helpers to fill the niche, this allows part of the display bridge drivers to work across systems. As the fwnode API has wider coverage than DT counterpart and the fwnode graphs are compatible with the OF graph, so the provided helpers can be used on all systems in theory. Assumed that fwnode graphs are well established on the system. Tested on TI BeaglePlay board. v1 -> v2: * Modify it66121 to switch togather * Drop the 'side-by-side' implement * Add drm_bridge_find_next_bridge_by_fwnode() helper * Add drm_bridge_set_node() helper v2 -> v3: * Read kernel-doc and improve function comments (Dmitry) * Drop the 'port' argument of it66121_read_bus_width() (Dmitry) * drm-bridge: sii902x get nuked v3 -> v4: * drm-bridge: tfp410 get nuked * Add platform module alias * Rebase and improve commit message and function comments v4 -> v5: * Modify sii9234, ch7033 and ANX7688 * Trivial fixes v5 -> v6: * Implement the same thing with no boilerplate introduced * Add 'struct device *' field to the drm_bridge structure * Re-implement of_drm_find_bridge() with drm_bridge_find_by_fwnode() Sui Jingfeng (10): drm/bridge: Allow using fwnode APIs to get the next bridge drm/bridge: Set firmware node of drm_bridge instances automatically drm/bridge: Implement of_drm_find_bridge() on the top of drm_bridge_find_by_fwnode() drm/bridge: simple-bridge: Use fwnode APIs to acquire device properties drm/bridge: display-connector: Use fwnode APIs to acquire device properties drm/bridge: sii902x: Switch to use fwnode APIs to acquire device properties drm-bridge: it66121: Use fwnode APIs to acquire device properties drm/bridge: tfp410: Use fwnode APIs to acquire device properties drm/bridge: sii9234: Use fwnode APIs to abstract DT dependent API away drm/bridge: ch7033: Switch to use fwnode based APIs drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 +- .../drm/bridge/analogix/analogix-anx6345.c| 4 +- .../drm/bridge/analogix/analogix-anx78xx.c| 4 +- drivers/gpu/drm/bridge/analogix/anx7625.c | 3 +- .../gpu/drm/bridge/cadence/cdns-dsi-core.c| 3 +- .../drm/bridge/cadence/cdns-mhdp8546-core.c | 3 +- drivers/gpu/drm/bridge/chipone-icn6211.c | 5 +- drivers/gpu/drm/bridge/chrontel-ch7033.c | 15 ++-- drivers/gpu/drm/bridge/cros-ec-anx7688.c | 4 +- drivers/gpu/drm/bridge/display-connector.c| 26 +++--- drivers/gpu/drm/bridge/fsl-ldb.c | 3 +- drivers/gpu/drm/bridge/imx/imx-ldb-helper.c | 3 +- drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 3 +- .../drm/bridge/imx/imx8qxp-pixel-combiner.c | 3 +- .../gpu/drm/bridge/imx/imx8qxp-pixel-link.c | 3 +- drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c | 3 +- drivers/gpu/drm/bridge/ite-it6505.c | 3 +- drivers/gpu/drm/bridge/ite-it66121.c | 58 - drivers/gpu/drm/bridge/lontium-lt8912b.c | 3 +- drivers/gpu/drm/bridge/lontium-lt9211.c | 3 +- drivers/gpu/drm/bridge/lontium-lt9611.c | 3 +- drivers/gpu/drm/bridge/lontium-lt9611uxc.c| 3 +- drivers/gpu/drm/bridge/lvds-codec.c | 3 +- .../bridge/megachips-stdp-ge-b850v3-fw.c | 3 +- drivers/gpu/drm/bridge/microchip-lvds.c | 3 +- drivers/gpu/drm/bridge/nwl-dsi.c | 3 +- drivers/gpu/drm/bridge/nxp-ptn3460.c | 3 +- drivers/gpu/drm/bridge/panel.c| 3 +- drivers/gpu/drm/bridge/parade-ps8622.c| 3 +- drivers/gpu/drm/bridge/parade-ps8640.c| 1 - drivers/gpu/drm/bridge/samsung-dsim.c | 3 +- drivers/gpu/drm/bridge/sii902x.c | 46 -- drivers/gpu/drm/bridge/sii9234.c | 8 +- drivers/gpu/drm/bridge/sil-sii8620.c | 3 +- drivers/gpu/drm/bridge/simple-bridge.c| 24 +++-- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 +- drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 3 +- drivers/gpu/drm/bridge/tc358762.c | 3 +- drivers/gpu/drm/bridge/tc358764.c | 3 +- drivers/gpu/drm/bridge/tc358767.c | 3 +- drivers/gpu/drm/bridge/tc358768.c | 3 +- drivers/gpu/drm/bridge/tc358775.c | 3 +- drivers/gpu/drm/bridge/thc63lvd1024.c | 3 +- drivers/gpu/drm/bridge/ti-dlpc3433.c | 3 +- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 3 +- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +- drivers/gpu/drm/bridge/ti-tfp410.c| 42 - drivers/gpu/drm/bridge/ti-tpd12s015.c | 3 +- drivers/gpu/drm/drm_bridge.c | 87 +++
[PATCH v2 3/3] drm/loongson: Add dummy gpu driver as a subcomponent
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
---
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 ..4ae6a5807d1d
--- /dev/null
+++ b/drivers/gpu/drm/loongson/loong_gpu_pci_drv.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include
+#include
+
+#include
+#include
+
+#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(>dev, _gpu_component_ops);
+}
+
+static void loong_gpu_pci_remove(struct pci_dev *pdev)
+{
+ component_del(>dev, _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 ..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
+
+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/l
[PATCH v2 2/3] drm/loongson: Introduce component framework support
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
---
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
+#include
#include
@@ -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(_pci_driver);
+ ret = platform_driver_register(_output_port_platform_driver);
+ if (ret)
+ return ret;
+
+ ret = pci_register_driver(_pci_driver);
+ if (ret) {
+ platform_driver_unregister(_output_port_platform_driver);
+ return ret;
+ }
+
+ return 0;
}
module_init(loongson_module_init);
static void __exit loongson_module_exit(void)
{
pci_unregister_driver(_pci_driver);
+
+ platform_driver_unregister(_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
#include
+#include
#include
#include
@@ -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 = >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 = >dispipe[i];
- if (dispipe->li2c)
- ddc = >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 = >dispipe[i];
@@ -187,30 +164,17 @@ st
[PATCH v2 1/3] drm/loongson: Add a helper for creating child devices
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
---
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
+#include
#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
[PATCH v2 0/3] drm/loongson: Introduce component framework support
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
Re: [v14,06/28] drm/tests: Add output bpc tests
Hi, Maxime
I love you patch, yet it generates warnning calltrace. Despite it's
just a warning but it can overwhelm when we run kunit tests. Hence,
I suggest switch to the drm_atomic_connector_get_property() function.
Logs are pasted as below for easier to ready.
[ cut here ]
WARNING: CPU: 3 PID: 1264 at drivers/gpu/drm/drm_mode_object.c:354
drm_object_property_get_value+0x2c/0x34
Modules linked in: drm_connector_test drm_display_helper
drm_kunit_helpers kunit rfkill ip_set nf_tables nfnetlink vfat fat uas
usb_storage kvm efi_pstore pstore spi_loongson_pci spi_loongson_core
fuse efivarfs [last unloaded: drm_connector_test]
CPU: 3 PID: 1264 Comm: kunit_try_catch Tainted: G N
6.9.0+ #443
Hardware name: Loongson
Loongson-3A6000-HV-7A2000-XA61200/Loongson-3A6000-HV-7A2000-XA61200,
BIOS Loongson-UDK2018-V4.0.05636-stable202311 12/
pc 93469fec ra 8225afdc tp 90011fc54000 sp
90011fc57d80
a0 90010aa84658 a1 900104432a00 a2 90011fc57d98 a3
90011fc57d98
a4 900104432a4c a5 93f14e98 a6 0008 a7
fffe
t0 0010 t1 90010aa84000 t2 t3
c0c0c0c0
t4 c0c0c0c0 t5 0220 t6 0001 t7
00107203
t8 00107303 u0 0008 s9 9001000ebe60 s0
90010aa84000
s1 9001470679c8 s2 900104432a00 s3 82284000 s4
90010aa84658
s5 90010aa84618 s6 1000 s7 0001 s8
ra: 8225afdc drm_test_connector_hdmi_init_bpc_8+0xcc/0x2d0
[drm_connector_test]
ERA: 93469fec drm_object_property_get_value+0x2c/0x34
CRMD: 00b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE)
PRMD: 0004 (PPLV0 +PIE -PWE)
EUEN: (-FPE -SXE -ASXE -BTE)
ECFG: 00071c1c (LIE=2-4,10-12 VS=7)
ESTAT: 000c [BRK] (IS= ECode=12 EsubCode=0)
PRID: 0014d000 (Loongson-64bit, Loongson-3A6000-HV)
CPU: 3 PID: 1264 Comm: kunit_try_catch Tainted: G N
6.9.0+ #443
Hardware name: Loongson
Loongson-3A6000-HV-7A2000-XA61200/Loongson-3A6000-HV-7A2000-XA61200,
BIOS Loongson-UDK2018-V4.0.05636-stable202311 12/
Stack : 94065000 92ac339c
90011fc54000
90011fc579f0 90011fc579f8
90011fc57b38
90011fc57b30 90011fc57b30 90011fc57940
0001
0001 90011fc579f8 18e7bf3ffb6e59df
900100328a00
0001 0003 0434
4c206e6f73676e6f
6f4c203a656d616e 000d0ad3 0704c000
9001000ebe60
93ee6ab0
94065000
90010aa84618 1000
0001
92ac33b4
7dd80078
00b0 0004
00071c1c
...
Call Trace:
[<92ac33b4>] show_stack+0x5c/0x180
[<93b1ed2c>] dump_stack_lvl+0x70/0xa0
[<93b01fd8>] __warn+0x84/0xc8
[<93ad282c>] report_bug+0x19c/0x204
[<93b1fe00>] do_bp+0x264/0x2b4
[<>] 0x0
[<93469fec>] drm_object_property_get_value+0x2c/0x34
[] drm_test_connector_hdmi_init_bpc_8+0xcc/0x2d0
[drm_connector_test]
[] kunit_try_run_case+0x7c/0x18c [kunit]
[] kunit_generic_run_threadfn_adapter+0x1c/0x28 [kunit]
[<92b06238>] kthread+0x124/0x130
[<92ac1248>] ret_from_kernel_thread+0xc/0xa4
---[ end trace ]---
[ cut here ]
On 5/21/24 18:13, Maxime Ripard wrote:
Now that we're tracking the output bpc count in the connector state,
let's add a few tests to make sure it works as expected.
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/Kconfig| 1 +
drivers/gpu/drm/tests/Makefile | 1 +
drivers/gpu/drm/tests/drm_connector_test.c | 140 +++
drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 438 +
drivers/gpu/drm/tests/drm_kunit_edid.h | 106 +
5 files changed, 686 insertions(+)
[...]
+
+/*
+ * Test that the registration of a connector with a maximum bpc count of
+ * 8 succeeds, registers the max bpc property, but doesn't register the
+ * HDR output metadata one.
+ */
+static void drm_test_connector_hdmi_init_bpc_8(struct kunit *test)
+{
+ struct drm_connector_init_priv *priv = test->priv;
+ struct drm_connector *connector = >connector;
+ struct drm_property *prop;
+ uint64_t val;
+ int ret;
+
+ ret = drmm_connector_hdmi_init(>drm, connector,
+ _funcs,
+ DRM_MODE_CONNECTOR_HDMIA,
+ >ddc,
+
Re: [v5,3/3] drm/mediatek: Implement OF graphs support for display paths
Hi,
On 5/21/24 15:57, AngeloGioacchino Del Regno wrote:
+static int mtk_drm_of_ddp_path_build(struct device *dev, struct device_node
*node,
+struct mtk_mmsys_driver_data *data)
+{
+ struct device_node *ep_node;
+ struct of_endpoint of_ep;
+ bool output_present[MAX_CRTC] = { false };
+ int ret;
+
+ for_each_endpoint_of_node(node, ep_node) {
+ ret = of_graph_parse_endpoint(ep_node, _ep);
+ of_node_put(ep_node);
There is going to *double* decline the reference counter, as the
__of_get_next_child() will decrease the reference counter for us
on the next iteration.
+ if (ret) {
+ dev_err_probe(dev, ret, "Cannot parse endpoint\n");
+ break;
+ }
Move the 'of_node_put(ep_node)' into brace '{}' here, if we really cares
about the reference count.
+
+ if (of_ep.id >= MAX_CRTC) {
ditto ?
+ ret = dev_err_probe(dev, -EINVAL,
+ "Invalid endpoint%u number\n",
of_ep.port);
+ break;
+ }
+
+ output_present[of_ep.id] = true;
+ }
+
+ if (ret)
+ return ret;
+
+ if (output_present[CRTC_MAIN]) {
+ ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_MAIN,
+ >main_path,
>main_len);
+ if (ret)
+ return ret;
+ }
+
+ if (output_present[CRTC_EXT]) {
+ ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_EXT,
+ >ext_path,
>ext_len);
+ if (ret)
+ return ret;
+ }
+
+ if (output_present[CRTC_THIRD]) {
+ ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_THIRD,
+ >third_path,
>third_len);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
--
Best regards
Sui Jingfeng
Re: [v5,3/3] drm/mediatek: Implement OF graphs support for display paths
Hi,
On 5/21/24 15:57, AngeloGioacchino Del Regno wrote:
+static int mtk_drm_of_get_ddp_comp_type(struct device_node *node, enum
mtk_ddp_comp_type *ctype)
+{
+ const struct of_device_id *of_id = of_match_node(mtk_ddp_comp_dt_ids,
node);
+
+ if (!of_id)
+ return -EINVAL;
+
+ *ctype = (enum mtk_ddp_comp_type)((uintptr_t)of_id->data);
+
+ return 0;
+}
+
+static int mtk_drm_of_get_ddp_ep_cid(struct device_node *node,
+int output_port, enum mtk_crtc_path
crtc_path,
+struct device_node **next, unsigned int
*cid)
+{
+ struct device_node *ep_dev_node, *ep_out;
+ enum mtk_ddp_comp_type comp_type;
+ int ret;
+
+ ep_out = of_graph_get_endpoint_by_regs(node, output_port, crtc_path);
+ if (!ep_out)
+ return -ENOENT;
+
+ ep_dev_node = of_graph_get_remote_port_parent(ep_out);
below here, 'ep_out' will not be used anymore.
of_node_put(ep_out);
Maybe we should call it under a error handling tag?
But this is trivial problem.
+ if (!ep_dev_node)
+ return -EINVAL;
+
+ /*
+* Pass the next node pointer regardless of failures in the later code
+* so that if this function is called in a loop it will walk through all
+* of the subsequent endpoints anyway.
+*/
+ *next = ep_dev_node;
+
+ if (!of_device_is_available(ep_dev_node))
+ return -ENODEV;
+
+ ret = mtk_drm_of_get_ddp_comp_type(ep_dev_node, _type);
+ if (ret) {
+ if (mtk_ovl_adaptor_is_comp_present(ep_dev_node)) {
+ *cid = (unsigned int)DDP_COMPONENT_DRM_OVL_ADAPTOR;
+ return 0;
+ }
+ return ret;
+ }
+
+ ret = mtk_ddp_comp_get_id(ep_dev_node, comp_type);
+ if (ret < 0)
+ return ret;
+
+ /* All ok! Pass the Component ID to the caller. */
+ *cid = (unsigned int)ret;
+
+ return 0;
+}
+
--
Best regards
Sui Jingfeng
Re: [v5,3/3] drm/mediatek: Implement OF graphs support for display paths
Hi, On 5/22/24 19:48, Sui Jingfeng wrote: if the not bridge is not ready 'not' -> 'next'
Re: [v5,3/3] drm/mediatek: Implement OF graphs support for display paths
Hi,
Looks good to me in overall!
On 5/21/24 15:57, AngeloGioacchino Del Regno wrote:
It is impossible to add each and every possible DDP path combination
for each and every possible combination of SoC and board: right now,
this driver hardcodes configuration for 10 SoCs and this is going to
grow larger and larger, and with new hacks like the introduction of
mtk_drm_route which is anyway not enough for all final routes as the
DSI cannot be connected to MERGE if it's not a dual-DSI, or enabling
DSC preventively doesn't work if the display doesn't support it, or
others.
Since practically all display IPs in MediaTek SoCs support being
interconnected with different instances of other, or the same, IPs
or with different IPs and in different combinations, the final DDP
pipeline is effectively a board specific configuration.
Implement OF graphs support to the mediatek-drm drivers, allowing to
stop hardcoding the paths, and preventing this driver to get a huge
amount of arrays for each board and SoC combination, also paving the
way to share the same mtk_mmsys_driver_data between multiple SoCs,
making it more straightforward to add support for new chips.
Reviewed-by: Alexandre Mergnat
Tested-by: Alexandre Mergnat
Signed-off-by: AngeloGioacchino Del Regno
---
drivers/gpu/drm/mediatek/mtk_disp_drv.h | 1 +
.../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 40 ++-
drivers/gpu/drm/mediatek/mtk_dpi.c| 16 +-
drivers/gpu/drm/mediatek/mtk_drm_drv.c| 282 --
drivers/gpu/drm/mediatek/mtk_drm_drv.h| 2 +-
drivers/gpu/drm/mediatek/mtk_dsi.c| 10 +-
6 files changed, 313 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
index 082ac18fe04a..94843974851f 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -108,6 +108,7 @@ size_t mtk_ovl_get_num_formats(struct device *dev);
void mtk_ovl_adaptor_add_comp(struct device *dev, struct mtk_mutex *mutex);
void mtk_ovl_adaptor_remove_comp(struct device *dev, struct mtk_mutex *mutex);
+bool mtk_ovl_adaptor_is_comp_present(struct device_node *node);
void mtk_ovl_adaptor_connect(struct device *dev, struct device *mmsys_dev,
unsigned int next);
void mtk_ovl_adaptor_disconnect(struct device *dev, struct device *mmsys_dev,
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
index 02dd7dcdfedb..400519d1ca1f 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
@@ -491,6 +491,38 @@ static int compare_of(struct device *dev, void *data)
return dev->of_node == data;
}
+static int ovl_adaptor_of_get_ddp_comp_type(struct device_node *node,
+ enum mtk_ovl_adaptor_comp_type
*ctype)
+{
+ const struct of_device_id *of_id =
of_match_node(mtk_ovl_adaptor_comp_dt_ids, node);
+
+ if (!of_id)
+ return -EINVAL;
+
+ *ctype = (enum mtk_ovl_adaptor_comp_type)((uintptr_t)of_id->data);
+
+ return 0;
+}
+
+bool mtk_ovl_adaptor_is_comp_present(struct device_node *node)
+{
+ enum mtk_ovl_adaptor_comp_type type;
+ int ret;
+
+ ret = ovl_adaptor_of_get_ddp_comp_type(node, );
+ if (ret)
+ return false;
+
+ if (type >= OVL_ADAPTOR_TYPE_NUM)
+ return false;
+
+ /*
+* ETHDR and Padding are used exclusively in OVL Adaptor: if this
+* component is not one of those, it's likely not an OVL Adaptor path.
+*/
+ return type == OVL_ADAPTOR_TYPE_ETHDR || type ==
OVL_ADAPTOR_TYPE_PADDING;
+}
+
static int ovl_adaptor_comp_init(struct device *dev, struct component_match
**match)
{
struct mtk_disp_ovl_adaptor *priv = dev_get_drvdata(dev);
@@ -500,12 +532,11 @@ static int ovl_adaptor_comp_init(struct device *dev,
struct component_match **ma
parent = dev->parent->parent->of_node->parent;
for_each_child_of_node(parent, node) {
- const struct of_device_id *of_id;
enum mtk_ovl_adaptor_comp_type type;
- int id;
+ int id, ret;
- of_id = of_match_node(mtk_ovl_adaptor_comp_dt_ids, node);
- if (!of_id)
+ ret = ovl_adaptor_of_get_ddp_comp_type(node, );
+ if (ret)
continue;
if (!of_device_is_available(node)) {
@@ -514,7 +545,6 @@ static int ovl_adaptor_comp_init(struct device *dev, struct
component_match **ma
continue;
}
- type = (enum mtk_ovl_adaptor_comp_type)(uintptr_t)of_id->data;
id = ovl_adaptor_comp_get_id(dev, node, type);
if (id < 0) {
dev_warn(dev, "Skipping unknown component %pOF\n",
diff --git
Re: [PATCH] drm/bridge: adv7511: Exit interrupt handling when necessary
Hi,
On 5/20/24 19:13, Dmitry Baryshkov wrote:
On Mon, 20 May 2024 at 14:11, Sui Jingfeng wrote:
Hi,
On 5/20/24 06:11, Dmitry Baryshkov wrote:
On Thu, May 16, 2024 at 06:10:06PM +0800, Liu Ying wrote:
Commit f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
fails to consider the case where adv7511->i2c_main->irq is zero, i.e.,
no interrupt requested at all.
Without interrupt, adv7511_wait_for_edid() could return -EIO sometimes,
because it polls adv7511->edid_read flag by calling adv7511_irq_process()
a few times, but adv7511_irq_process() happens to refuse to handle
interrupt by returning -ENODATA. Hence, EDID retrieval fails randomly.
Fix the issue by checking adv7511->i2c_main->irq before exiting interrupt
handling from adv7511_irq_process().
Fixes: f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
Signed-off-by: Liu Ying
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 6089b0bb9321..2074fa3c1b7b 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -479,7 +479,8 @@ static int adv7511_irq_process(struct adv7511 *adv7511,
bool process_hpd)
return ret;
/* If there is no IRQ to handle, exit indicating no IRQ data */
-if (!(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
+if (adv7511->i2c_main->irq &&
+!(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
!(irq1 & ADV7511_INT1_DDC_ERROR))
return -ENODATA;
I think it might be better to handle -ENODATA in adv7511_wait_for_edid()
instead. WDYT?
I think this is may deserve another patch.
My point is that the IRQ handler is fine to remove -ENODATA here,
[...]
there is no pending IRQ that can be handled.
But there may has other things need to do in the adv7511_irq_process()
function.
So instead of continuing
the execution when we know that IRQ bits are not set,
Even when IRQ bits are not set, it just means that there is no HPD
and no EDID ready-to-read signal. HDMI CEC interrupts still need
to process.
it's better to
ignore -ENODATA in the calling code and go on with msleep().
So, It's confusing to ignore the -ENODATA here.
--
Best regards
Sui
Re: [PATCH] drm/bridge: adv7511: Exit interrupt handling when necessary
Hi,
On 5/20/24 06:11, Dmitry Baryshkov wrote:
On Thu, May 16, 2024 at 06:10:06PM +0800, Liu Ying wrote:
Commit f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
fails to consider the case where adv7511->i2c_main->irq is zero, i.e.,
no interrupt requested at all.
Without interrupt, adv7511_wait_for_edid() could return -EIO sometimes,
because it polls adv7511->edid_read flag by calling adv7511_irq_process()
a few times, but adv7511_irq_process() happens to refuse to handle
interrupt by returning -ENODATA. Hence, EDID retrieval fails randomly.
Fix the issue by checking adv7511->i2c_main->irq before exiting interrupt
handling from adv7511_irq_process().
Fixes: f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
Signed-off-by: Liu Ying
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 6089b0bb9321..2074fa3c1b7b 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -479,7 +479,8 @@ static int adv7511_irq_process(struct adv7511 *adv7511,
bool process_hpd)
return ret;
/* If there is no IRQ to handle, exit indicating no IRQ data */
- if (!(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
+ if (adv7511->i2c_main->irq &&
+ !(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
!(irq1 & ADV7511_INT1_DDC_ERROR))
return -ENODATA;
I think it might be better to handle -ENODATA in adv7511_wait_for_edid()
instead. WDYT?
I think this is may deserve another patch.
--
Best regards
Sui
[etnaviv-next v14 8/8] drm/etnaviv: Add support for vivante GPU cores attached via PCIe device
Previouly, the component framework is being used to bind multiple platform
GPU devices to a virtual master. The virtual master is manually created by
the driver, and is also a platform device. This is fine and works well for
various SoCs, yet there some hardware venders integrate Vivante GPU cores
into PCIe card and the driver lacks the support for PCIe devices.
Create virtual platform devices as a representation for each GPU IP core,
the manually created platform devices are functional as subcomponent, and
all of them are child of the PCIe master device. The master is real for
PCIe devices, as the PCIe device has already been created by the time the
etnaviv.ko is loaded. Hence, bind all of the virtual child to the real
master, this design reflects the hardware layout perfectly and is
extensible.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/Kconfig | 9 ++
drivers/gpu/drm/etnaviv/Makefile | 2 +
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 12 +-
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 2 +
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 75 --
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 4 +
drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c | 161 ++
drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h | 44 ++
8 files changed, 293 insertions(+), 16 deletions(-)
create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c
create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h
diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
index faa7fc68b009..7cb44f72d512 100644
--- a/drivers/gpu/drm/etnaviv/Kconfig
+++ b/drivers/gpu/drm/etnaviv/Kconfig
@@ -15,6 +15,15 @@ config DRM_ETNAVIV
help
DRM driver for Vivante GPUs.
+config DRM_ETNAVIV_PCI_DRIVER
+ bool "enable ETNAVIV PCI driver support"
+ depends on DRM_ETNAVIV
+ depends on PCI
+ default n
+ help
+ Compile in support for Vivante GPUs attached via PCIe card.
+ Say Y if you have such hardwares.
+
config DRM_ETNAVIV_THERMAL
bool "enable ETNAVIV thermal throttling"
depends on DRM_ETNAVIV
diff --git a/drivers/gpu/drm/etnaviv/Makefile b/drivers/gpu/drm/etnaviv/Makefile
index 46e5ffad69a6..6829e1ebf2db 100644
--- a/drivers/gpu/drm/etnaviv/Makefile
+++ b/drivers/gpu/drm/etnaviv/Makefile
@@ -16,4 +16,6 @@ etnaviv-y := \
etnaviv_perfmon.o \
etnaviv_sched.o
+etnaviv-$(CONFIG_DRM_ETNAVIV_PCI_DRIVER) += etnaviv_pci_drv.o
+
obj-$(CONFIG_DRM_ETNAVIV) += etnaviv.o
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index dc3556aad134..90ee60b00c24 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -24,6 +24,7 @@
#include "etnaviv_gpu.h"
#include "etnaviv_gem.h"
#include "etnaviv_mmu.h"
+#include "etnaviv_pci_drv.h"
#include "etnaviv_perfmon.h"
/*
@@ -568,6 +569,10 @@ static int etnaviv_bind(struct device *dev)
if (ret < 0)
goto out_free_priv;
+ ret = etnaviv_register_irq_handler(dev, priv);
+ if (ret)
+ goto out_unbind;
+
load_gpu(drm);
ret = drm_dev_register(drm, 0);
@@ -596,7 +601,7 @@ static void etnaviv_unbind(struct device *dev)
etnaviv_private_fini(priv);
}
-static const struct component_master_ops etnaviv_master_ops = {
+const struct component_master_ops etnaviv_master_ops = {
.bind = etnaviv_bind,
.unbind = etnaviv_unbind,
};
@@ -740,6 +745,10 @@ static int __init etnaviv_init(void)
if (ret != 0)
goto unregister_gpu_driver;
+ ret = etnaviv_register_pci_driver();
+ if (ret)
+ goto unregister_platform_driver;
+
/*
* If the DT contains at least one available GPU device, instantiate
* the DRM platform device.
@@ -769,6 +778,7 @@ module_init(etnaviv_init);
static void __exit etnaviv_exit(void)
{
etnaviv_destroy_platform_device(_drm);
+ etnaviv_unregister_pci_driver();
platform_driver_unregister(_platform_driver);
platform_driver_unregister(_gpu_driver);
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 4612843ff9f6..6db26d384cbe 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -27,6 +27,8 @@ struct etnaviv_gem_object;
struct etnaviv_gem_submit;
struct etnaviv_iommu_global;
+extern const struct component_master_ops etnaviv_master_ops;
+
#define ETNAVIV_SOFTPIN_START_ADDRESS SZ_4M /* must be >= SUBALLOC_SIZE */
struct etnaviv_file_private {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 3a14e187388a..2b5955693fbb 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -10,6 +10,7 @@
#include
#in
[etnaviv-next v14 7/8] drm/etnaviv: Allow creating subdevices and pass platform specific data
Because some hardware are too complex to be managed by a monolithic driver,
a split of the functionality into child devices can helps to achieve better
modularity.
We will use this function to create subdevice as a repensentation of a
single hardware ip block, so that the same modular approach that works
for ARM-SoC can also works for PCIe cards.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 33 +++
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 9
2 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 863faac2ea19..dc3556aad134 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -670,16 +670,36 @@ static struct platform_driver etnaviv_platform_driver = {
},
};
-static int etnaviv_create_platform_device(const char *name,
- struct platform_device **ppdev)
+int etnaviv_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, PLATFORM_DEVID_NONE);
+ 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);
@@ -691,7 +711,7 @@ static int etnaviv_create_platform_device(const char *name,
return 0;
}
-static void etnaviv_destroy_platform_device(struct platform_device **ppdev)
+void etnaviv_destroy_platform_device(struct platform_device **ppdev)
{
struct platform_device *pdev = *ppdev;
@@ -728,7 +748,10 @@ static int __init etnaviv_init(void)
if (np) {
of_node_put(np);
- ret = etnaviv_create_platform_device("etnaviv", _drm);
+ ret = etnaviv_create_platform_device(NULL, "etnaviv",
+PLATFORM_DEVID_NONE,
+NULL, NULL,
+_drm);
if (ret)
goto unregister_platform_driver;
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 4b59fdb457b7..4612843ff9f6 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -98,6 +99,14 @@ bool etnaviv_cmd_validate_one(struct etnaviv_gpu *gpu,
u32 *stream, unsigned int size,
struct drm_etnaviv_gem_submit_reloc *relocs, unsigned int reloc_size);
+int etnaviv_create_platform_device(struct device *parent,
+ const char *name, int id,
+ struct resource *pres,
+ void *data,
+ struct platform_device **ppdev);
+
+void etnaviv_destroy_platform_device(struct platform_device **ppdev);
+
#ifdef CONFIG_DEBUG_FS
void etnaviv_gem_describe_objects(struct etnaviv_drm_private *priv,
struct seq_file *m);
--
2.34.1
[etnaviv-next v14 6/8] drm/etnaviv: Replace the '>dev' with 'dev'
In the etnaviv_pdev_probe(), etnaviv_gpu_platform_probe() function, the
value of '>dev' has been cached to the 'dev' local auto variable.
But part of callers use 'dev' as argument, while the rest use '>dev'.
To keep it consistent, use 'dev' uniformly.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 10 +-
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 16
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 986fd68b489a..863faac2ea19 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -614,7 +614,7 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
if (!of_device_is_available(core_node))
continue;
- drm_of_component_match_add(>dev, ,
+ drm_of_component_match_add(dev, ,
component_compare_of,
core_node);
}
} else {
@@ -637,9 +637,9 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
* bit to make sure we are allocating the command buffers and
* TLBs in the lower 4 GiB address space.
*/
- if (dma_set_mask(>dev, DMA_BIT_MASK(40)) ||
- dma_set_coherent_mask(>dev, DMA_BIT_MASK(32))) {
- dev_dbg(>dev, "No suitable DMA available\n");
+ if (dma_set_mask(dev, DMA_BIT_MASK(40)) ||
+ dma_set_coherent_mask(dev, DMA_BIT_MASK(32))) {
+ dev_dbg(dev, "No suitable DMA available\n");
return -ENODEV;
}
@@ -650,7 +650,7 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
*/
first_node = etnaviv_of_first_available_node();
if (first_node) {
- of_dma_configure(>dev, first_node, true);
+ of_dma_configure(dev, first_node, true);
of_node_put(first_node);
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index aa15682f94db..3a14e187388a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1891,7 +1891,7 @@ static int etnaviv_gpu_platform_probe(struct
platform_device *pdev)
if (!gpu)
return -ENOMEM;
- gpu->dev = >dev;
+ gpu->dev = dev;
mutex_init(>lock);
mutex_init(>sched_lock);
@@ -1905,8 +1905,8 @@ static int etnaviv_gpu_platform_probe(struct
platform_device *pdev)
if (gpu->irq < 0)
return gpu->irq;
- err = devm_request_irq(>dev, gpu->irq, irq_handler, 0,
- dev_name(gpu->dev), gpu);
+ err = devm_request_irq(dev, gpu->irq, irq_handler, 0,
+ dev_name(dev), gpu);
if (err) {
dev_err(dev, "failed to request IRQ%u: %d\n", gpu->irq, err);
return err;
@@ -1925,13 +1925,13 @@ static int etnaviv_gpu_platform_probe(struct
platform_device *pdev)
* autosuspend delay is rather arbitary: no measurements have
* yet been performed to determine an appropriate value.
*/
- pm_runtime_use_autosuspend(gpu->dev);
- pm_runtime_set_autosuspend_delay(gpu->dev, 200);
- pm_runtime_enable(gpu->dev);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_set_autosuspend_delay(dev, 200);
+ pm_runtime_enable(dev);
- err = component_add(>dev, _ops);
+ err = component_add(dev, _ops);
if (err < 0) {
- dev_err(>dev, "failed to register component: %d\n", err);
+ dev_err(dev, "failed to register component: %d\n", err);
return err;
}
--
2.34.1
[etnaviv-next v14 5/8] drm/etnaviv: Add support for cached coherent caching mode
Many modern CPUs and/or platforms choose to define their peripheral devices
as cached coherent by default, to be specific, the PCH is capable of
snooping CPU's cache. When hit the peripheral devices will access data
directly from CPU's cache. This means that device drivers do not need to
maintain the coherency issue between a processor and peripheral I/O for
the cached buffers. Hence, it dosen't need us to sync manually on the
software side, which is useful to avoid some overheads, especially for
userspace, but userspace is not known yet.
Probe the hardware maintained cached coherent support of the host platform
with the dev_is_dma_coherent() function, and store the result in struct
etnaviv_drm_private. As this is a platform implementation-defined hardware
feature and again is meant to be shared by all GPU cores. And expose it
via etnaviv parameter mechanism to let userspace know.
Please note that write-combine mapping out of scope of the discussion
and therefore is not being addressed.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++
drivers/gpu/drm/etnaviv/etnaviv_drv.h | 9 +
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 4
include/uapi/drm/etnaviv_drm.h| 1 +
4 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index e3eb31ba9a2b..986fd68b489a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -5,6 +5,7 @@
#include
#include
+#include
#include
#include
#include
@@ -57,6 +58,8 @@ static int etnaviv_private_init(struct device *dev,
return -ENOMEM;
}
+ priv->cached_coherent = dev_is_dma_coherent(dev);
+
return 0;
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 1f9b50b5a6aa..4b59fdb457b7 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -46,6 +46,15 @@ struct etnaviv_drm_private {
struct xarray active_contexts;
u32 next_context_id;
+ /*
+* If true, the cached mapping is consistent for all CPU cores and
+* peripheral bus masters in the system. It means that both of the
+* CPU and GPU will see the same data if the buffer being accessed
+* is cached. And the coherency is guaranteed by the host platform
+* specific hardwares.
+*/
+ bool cached_coherent;
+
/* list of GEM objects: */
struct mutex gem_lock;
struct list_head gem_list;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 02d7efdc82c0..aa15682f94db 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -164,6 +164,10 @@ int etnaviv_gpu_get_param(struct etnaviv_gpu *gpu, u32
param, u64 *value)
*value = gpu->identity.eco_id;
break;
+ case ETNAVIV_PARAM_CACHED_COHERENT:
+ *value = priv->cached_coherent;
+ break;
+
default:
DBG("%s: invalid param: %u", dev_name(gpu->dev), param);
return -EINVAL;
diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
index af024d90453d..61eaa8cd0f5e 100644
--- a/include/uapi/drm/etnaviv_drm.h
+++ b/include/uapi/drm/etnaviv_drm.h
@@ -77,6 +77,7 @@ struct drm_etnaviv_timespec {
#define ETNAVIV_PARAM_GPU_PRODUCT_ID0x1c
#define ETNAVIV_PARAM_GPU_CUSTOMER_ID 0x1d
#define ETNAVIV_PARAM_GPU_ECO_ID0x1e
+#define ETNAVIV_PARAM_CACHED_COHERENT 0x1f
#define ETNA_MAX_PIPES 4
--
2.34.1
[etnaviv-next v14 4/8] drm/etnaviv: Fix wrong cache property being used for vmap()
In the etnaviv_gem_vmap_impl() function, the driver vmap whatever buffers
with Write-Combine page property. This is unreasonable, as some platforms
are cached coherent. And cached buffers should be mapped with cached page
property.
Fixes: a0a5ab3e99b8 ("drm/etnaviv: call correct function when trying to vmap a
DMABUF")
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index aa95a5e98374..eed98bb9e446 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -342,6 +342,7 @@ void *etnaviv_gem_vmap(struct drm_gem_object *obj)
static void *etnaviv_gem_vmap_impl(struct etnaviv_gem_object *obj)
{
struct page **pages;
+ pgprot_t prot;
lockdep_assert_held(>lock);
@@ -349,8 +350,19 @@ static void *etnaviv_gem_vmap_impl(struct
etnaviv_gem_object *obj)
if (IS_ERR(pages))
return NULL;
- return vmap(pages, obj->base.size >> PAGE_SHIFT,
- VM_MAP, pgprot_writecombine(PAGE_KERNEL));
+ switch (obj->flags) {
+ case ETNA_BO_CACHED:
+ prot = PAGE_KERNEL;
+ break;
+ case ETNA_BO_UNCACHED:
+ prot = pgprot_noncached(PAGE_KERNEL);
+ break;
+ case ETNA_BO_WC:
+ default:
+ prot = pgprot_writecombine(PAGE_KERNEL);
+ }
+
+ return vmap(pages, obj->base.size >> PAGE_SHIFT, VM_MAP, prot);
}
static inline enum dma_data_direction etnaviv_op_to_dma_dir(u32 op)
--
2.34.1
[etnaviv-next v14 3/8] drm/etnaviv: Embed struct drm_device into struct etnaviv_drm_private
Both the instance of struct drm_device and the instance of struct
etnaviv_drm_private are intended to be shared by all GPU cores, both have
only one instance created across drm/etnaviv driver. After embedded in,
the whole structure can be allocated with devm_drm_dev_alloc(). And the
DRM device created is automatically put on driver detach, so we don't need
to call drm_dev_put() explicitly on driver leave. It's also eliminate the
need to use the .dev_private member, which is deprecated according to the
drm document. We can also use container_of() to retrieve pointer for the
containing structure.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c| 65
drivers/gpu/drm/etnaviv/etnaviv_drv.h| 7 +++
drivers/gpu/drm/etnaviv/etnaviv_gem.c| 6 +-
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 6 +-
drivers/gpu/drm/etnaviv/etnaviv_mmu.c| 4 +-
6 files changed, 40 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 22c78bc944c4..e3eb31ba9a2b 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -41,14 +41,9 @@ static struct device_node
*etnaviv_of_first_available_node(void)
return NULL;
}
-static struct etnaviv_drm_private *etnaviv_alloc_private(struct device *dev)
+static int etnaviv_private_init(struct device *dev,
+ struct etnaviv_drm_private *priv)
{
- struct etnaviv_drm_private *priv;
-
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return ERR_PTR(-ENOMEM);
-
xa_init_flags(>active_contexts, XA_FLAGS_ALLOC);
mutex_init(>gem_lock);
@@ -58,15 +53,14 @@ static struct etnaviv_drm_private
*etnaviv_alloc_private(struct device *dev)
priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(dev);
if (IS_ERR(priv->cmdbuf_suballoc)) {
- kfree(priv);
dev_err(dev, "Failed to create cmdbuf suballocator\n");
- return ERR_PTR(-ENOMEM);
+ return -ENOMEM;
}
- return priv;
+ return 0;
}
-static void etnaviv_free_private(struct etnaviv_drm_private *priv)
+static void etnaviv_private_fini(struct etnaviv_drm_private *priv)
{
if (!priv)
return;
@@ -76,13 +70,11 @@ static void etnaviv_free_private(struct etnaviv_drm_private
*priv)
etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
xa_destroy(>active_contexts);
-
- kfree(priv);
}
static void load_gpu(struct drm_device *dev)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
unsigned int i;
for (i = 0; i < ETNA_MAX_PIPES; i++) {
@@ -100,7 +92,7 @@ static void load_gpu(struct drm_device *dev)
static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct etnaviv_file_private *ctx;
int ret, i;
@@ -143,7 +135,7 @@ static int etnaviv_open(struct drm_device *dev, struct
drm_file *file)
static void etnaviv_postclose(struct drm_device *dev, struct drm_file *file)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct etnaviv_file_private *ctx = file->driver_priv;
unsigned int i;
@@ -168,7 +160,7 @@ static void etnaviv_postclose(struct drm_device *dev,
struct drm_file *file)
#ifdef CONFIG_DEBUG_FS
static int etnaviv_gem_show(struct drm_device *dev, struct seq_file *m)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
etnaviv_gem_describe_objects(priv, m);
@@ -262,7 +254,7 @@ static int show_each_gpu(struct seq_file *m, void *arg)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct etnaviv_gpu *gpu;
int (*show)(struct etnaviv_gpu *gpu, struct seq_file *m) =
node->info_ent->data;
@@ -305,7 +297,7 @@ static void etnaviv_debugfs_init(struct drm_minor *minor)
static int etnaviv_ioctl_get_param(struct drm_device *dev, void *data,
struct drm_file *file)
{
- struct etnaviv_drm_private *priv = dev->dev_private;
+ struct etnaviv_drm_private *priv = to_etnaviv_priv(dev);
struct drm_etnaviv_param *args = data;
struct etnaviv_gpu *gpu;
@@ -398,7 +390,7 @@ static int etnaviv_ioctl_wait_fence(s
[etnaviv-next v14 2/8] drm/etnaviv: Add constructor and destructor for the etnaviv_drm_private structure
Because there are a lot of data members in the struct etnaviv_drm_private,
which are intended to be shared by all GPU cores. It can be lengthy and
daunting on error handling, the 'gem_lock' of struct etnaviv_drm_private
just be forgeten to destroy on driver leave.
Switch to use the dedicated helpers introduced, etnaviv_bind() and
etnaviv_unbind() gets simplified. Another potential benefit is that
we could put the struct drm_device into struct etnaviv_drm_private
in the future, which made them share the same life time.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 72 +--
1 file changed, 46 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 6500f3999c5f..22c78bc944c4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -41,6 +41,45 @@ static struct device_node
*etnaviv_of_first_available_node(void)
return NULL;
}
+static struct etnaviv_drm_private *etnaviv_alloc_private(struct device *dev)
+{
+ struct etnaviv_drm_private *priv;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return ERR_PTR(-ENOMEM);
+
+ xa_init_flags(>active_contexts, XA_FLAGS_ALLOC);
+
+ mutex_init(>gem_lock);
+ INIT_LIST_HEAD(>gem_list);
+ priv->num_gpus = 0;
+ priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
+
+ priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(dev);
+ if (IS_ERR(priv->cmdbuf_suballoc)) {
+ kfree(priv);
+ dev_err(dev, "Failed to create cmdbuf suballocator\n");
+ return ERR_PTR(-ENOMEM);
+ }
+
+ return priv;
+}
+
+static void etnaviv_free_private(struct etnaviv_drm_private *priv)
+{
+ if (!priv)
+ return;
+
+ mutex_destroy(>gem_lock);
+
+ etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
+
+ xa_destroy(>active_contexts);
+
+ kfree(priv);
+}
+
static void load_gpu(struct drm_device *dev)
{
struct etnaviv_drm_private *priv = dev->dev_private;
@@ -521,35 +560,21 @@ static int etnaviv_bind(struct device *dev)
if (IS_ERR(drm))
return PTR_ERR(drm);
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv) {
- dev_err(dev, "failed to allocate private data\n");
- ret = -ENOMEM;
+ priv = etnaviv_alloc_private(dev);
+ if (IS_ERR(priv)) {
+ ret = PTR_ERR(priv);
goto out_put;
}
+
drm->dev_private = priv;
dma_set_max_seg_size(dev, SZ_2G);
- xa_init_flags(>active_contexts, XA_FLAGS_ALLOC);
-
- mutex_init(>gem_lock);
- INIT_LIST_HEAD(>gem_list);
- priv->num_gpus = 0;
- priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
-
- priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(drm->dev);
- if (IS_ERR(priv->cmdbuf_suballoc)) {
- dev_err(drm->dev, "Failed to create cmdbuf suballocator\n");
- ret = PTR_ERR(priv->cmdbuf_suballoc);
- goto out_free_priv;
- }
-
dev_set_drvdata(dev, drm);
ret = component_bind_all(dev, drm);
if (ret < 0)
- goto out_destroy_suballoc;
+ goto out_free_priv;
load_gpu(drm);
@@ -561,10 +586,8 @@ static int etnaviv_bind(struct device *dev)
out_unbind:
component_unbind_all(dev, drm);
-out_destroy_suballoc:
- etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
out_free_priv:
- kfree(priv);
+ etnaviv_free_private(priv);
out_put:
drm_dev_put(drm);
@@ -580,12 +603,9 @@ static void etnaviv_unbind(struct device *dev)
component_unbind_all(dev, drm);
- etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
-
- xa_destroy(>active_contexts);
+ etnaviv_free_private(priv);
drm->dev_private = NULL;
- kfree(priv);
drm_dev_put(drm);
}
--
2.34.1
[etnaviv-next v14 1/8] drm/etnaviv: Add a dedicated helper function to get various clocks
Because the current implementation is DT-based, this only works when the
host platform has the DT support. The problem is that some host platforms
does not provide DT-based clocks drivers, as a result, the driver rage
quit.
PLL hardwares are typically provided by the host platform, which is part
of the entire clock tree. The PLL hardware provide clock pulse to the GPU
core, but it's not belong to the GPU corei itself. PLL registers can be
manipulated directly by the device driver. Hence, it may need dedicated
clock driver.
Add a the etnaviv_gpu_clk_get() function to group similar code blocks,
which make it easier to call this function on the platform where it works.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 53 ---
1 file changed, 32 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index d84d73c197fc..e0c36f564fa6 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1605,6 +1605,35 @@ static irqreturn_t irq_handler(int irq, void *data)
return ret;
}
+static int etnaviv_gpu_clk_get(struct etnaviv_gpu *gpu)
+{
+ struct device *dev = gpu->dev;
+
+ gpu->clk_reg = devm_clk_get_optional(dev, "reg");
+ DBG("clk_reg: %p", gpu->clk_reg);
+ if (IS_ERR(gpu->clk_reg))
+ return PTR_ERR(gpu->clk_reg);
+
+ gpu->clk_bus = devm_clk_get_optional(dev, "bus");
+ DBG("clk_bus: %p", gpu->clk_bus);
+ if (IS_ERR(gpu->clk_bus))
+ return PTR_ERR(gpu->clk_bus);
+
+ gpu->clk_core = devm_clk_get(dev, "core");
+ DBG("clk_core: %p", gpu->clk_core);
+ if (IS_ERR(gpu->clk_core))
+ return PTR_ERR(gpu->clk_core);
+ gpu->base_rate_core = clk_get_rate(gpu->clk_core);
+
+ gpu->clk_shader = devm_clk_get_optional(dev, "shader");
+ DBG("clk_shader: %p", gpu->clk_shader);
+ if (IS_ERR(gpu->clk_shader))
+ return PTR_ERR(gpu->clk_shader);
+ gpu->base_rate_shader = clk_get_rate(gpu->clk_shader);
+
+ return 0;
+}
+
static int etnaviv_gpu_clk_enable(struct etnaviv_gpu *gpu)
{
int ret;
@@ -1880,27 +1909,9 @@ static int etnaviv_gpu_platform_probe(struct
platform_device *pdev)
}
/* Get Clocks: */
- gpu->clk_reg = devm_clk_get_optional(>dev, "reg");
- DBG("clk_reg: %p", gpu->clk_reg);
- if (IS_ERR(gpu->clk_reg))
- return PTR_ERR(gpu->clk_reg);
-
- gpu->clk_bus = devm_clk_get_optional(>dev, "bus");
- DBG("clk_bus: %p", gpu->clk_bus);
- if (IS_ERR(gpu->clk_bus))
- return PTR_ERR(gpu->clk_bus);
-
- gpu->clk_core = devm_clk_get(>dev, "core");
- DBG("clk_core: %p", gpu->clk_core);
- if (IS_ERR(gpu->clk_core))
- return PTR_ERR(gpu->clk_core);
- gpu->base_rate_core = clk_get_rate(gpu->clk_core);
-
- gpu->clk_shader = devm_clk_get_optional(>dev, "shader");
- DBG("clk_shader: %p", gpu->clk_shader);
- if (IS_ERR(gpu->clk_shader))
- return PTR_ERR(gpu->clk_shader);
- gpu->base_rate_shader = clk_get_rate(gpu->clk_shader);
+ err = etnaviv_gpu_clk_get(gpu);
+ if (err)
+ return err;
/* TODO: figure out max mapped size */
dev_set_drvdata(dev, gpu);
--
2.34.1
[etnaviv-next v14 0/8] drm/etnaviv: Add driver wrapper for vivante GPUs attached on PCI(e) device
drm/etnaviv use the component framework to bind multiple GPU cores to a virtual master, the virtual master is manually create during driver load time. This works well for various SoCs, yet there are some PCIe card has the vivante GPU cores integrated. The driver lacks the support for PCIe devices currently. Adds PCIe driver wrapper on the top of what drm/etnaviv already has, the component framework is still being used to bind subdevices, even though there is only one GPU core. But the process is going to be reversed, we create virtual platform device for each of the vivante GPU IP core shipped by the PCIe master. The PCIe master is real, bind all the virtual child to the master with component framework. v6: * Fix build issue on system without CONFIG_PCI enabled v7: * Add a separate patch for the platform driver rearrangement (Bjorn) * Switch to runtime check if the GPU is dma coherent or not (Lucas) * Add ETNAVIV_PARAM_GPU_COHERENT to allow userspace to query (Lucas) * Remove etnaviv_gpu.no_clk member (Lucas) * Fix Various typos and coding style fixed (Bjorn) v8: * Fix typos and remove unnecessary header included (Bjorn). * Add a dedicated function to create the virtual master platform device. v9: * Use PCI_VDEVICE() macro (Bjorn) * Add trivial stubs for the PCI driver (Bjorn) * Remove a redundant dev_err() usage (Bjorn) * Clean up etnaviv_pdev_probe() with etnaviv_of_first_available_node() v10: * Add one more cleanup patch * Resolve the conflict with a patch from Rob * Make the dummy PCI stub inlined * Print only if the platform is dma-coherrent V11: * Drop unnecessary changes (Lucas) * Tweak according to other reviews of v10. V12: * Create a virtual platform device for the subcomponent GPU cores * Bind all subordinate GPU cores to the real PCI master via component. V13: * Drop the non-component code path, always use the component framework to bind subcomponent GPU core. Even though there is only one core. * Defer the irq handler register. * Rebase and improve the commit message V14: * Rebase onto etnaviv-next and improve commit message. Tested with JD9230P GPU and LingJiu GP102 GPU. Sui Jingfeng (8): drm/etnaviv: Add a dedicated helper function to get various clocks drm/etnaviv: Add constructor and destructor for the etnaviv_drm_private structure drm/etnaviv: Embed struct drm_device into struct etnaviv_drm_private drm/etnaviv: Fix wrong cache property being used for vmap() drm/etnaviv: Add support for cached coherent caching mode drm/etnaviv: Replace the '>dev' with 'dev' drm/etnaviv: Allow creating subdevices and pass platform specific data drm/etnaviv: Add support for vivante GPU cores attached via PCIe device drivers/gpu/drm/etnaviv/Kconfig | 8 + drivers/gpu/drm/etnaviv/Makefile | 2 + drivers/gpu/drm/etnaviv/etnaviv_drv.c| 159 ++-- drivers/gpu/drm/etnaviv/etnaviv_drv.h| 27 +++ drivers/gpu/drm/etnaviv/etnaviv_gem.c| 22 ++- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 144 +- drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 4 + drivers/gpu/drm/etnaviv/etnaviv_mmu.c| 4 +- drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c| 187 +++ drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h| 18 ++ include/uapi/drm/etnaviv_drm.h | 1 + 12 files changed, 468 insertions(+), 110 deletions(-) create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_pci_drv.h base-commit: 52272bfff15ee70c7bd5be9368f175948fb8ecfd -- 2.34.1
Re: [RFT,v2,26/48] drm/panel: simple: Stop tracking prepared/enabled
Hi,
On 2024/5/4 05:33, Douglas Anderson wrote:
As talked about in commit d2aacaf07395 ("drm/panel: Check for already
prepared/enabled in drm_panel"), we want to remove needless code from
panel drivers that was storing and double-checking the
prepared/enabled state. Even if someone was relying on the
double-check before, that double-check is now in the core and not
needed in individual drivers.
I think you are right, as I see drm_panel already has embed
the 'prepared' and 'enabled' as members, remove them from
the derived structure could probably save memory footprint.
Reducing boilerplate is also a side benefit.
Signed-off-by: Douglas Anderson
Acked-by: Sui Jingfeng
--
Best regards,
Sui
Re: [PATCH 3/3] drm/loongson: Refactor lsdc device initialize and the output port
Hi, On 5/16/24 14:26, Markus Elfring wrote: … fullfill the implement under the new framework. fulfil the implementation? Please improve your change descriptions another bit. OK, despite has a few typos. but the quality of the patch itself can be guaranteed. The first version is mainly for preview purpose. I'll fix the problem you mentioned at the next version, thanks for reviewing. I have tested this series before posting on ls3a6000+ls7a2000 PC: $ dmesg | grep loongson: [4.125926] loongson :00:06.1: Found LS7A2000 bridge chipset, revision: 2 [4.133035] loongson :00:06.1: [drm] dc: 400MHz, gmc: 1200MHz, gpu: 480MHz [4.140215] loongson :00:06.1: [drm] Dedicated vram start: 0xe003000, size: 256MiB [4.148538] loongson :00:06.1: [drm] VRAM: 16384 pages ready [4.154506] loongson :00:06.1: [drm] GTT: 32768 pages ready [4.160410] loongson :00:06.1: [drm] lsdc-i2c0(sda pin mask=1, scl pin mask=2) created [4.168638] loongson :00:06.1: [drm] lsdc-i2c1(sda pin mask=4, scl pin mask=8) created [4.176938] loongson :00:06.1: [drm] registered irq: 54 [4.189404] loongson :00:06.1: [drm] Output port-0 bound, type: HDMI-or-VGA-0 [4.196839] loongson :00:06.1: bound lsdc-output-port.0 (ops lsdc_output_port_component_ops) [4.211629] loongson :00:06.1: [drm] Output port-1 bound, type: HDMI-1 [4.218459] loongson :00:06.1: bound lsdc-output-port.1 (ops lsdc_output_port_component_ops) [4.227203] loongson :00:06.1: [drm] Total 2 outputs [4.272617] [drm] Initialized loongson 1.0.0 for :00:06.1 on minor 0 [4.341588] loongson :00:06.1: [drm] fb0: loongsondrmfb frame buffer device [4.348867] loongson :00:06.1: loongson add component: 0 Regards, Markus -- Best regards Sui
Re: drm: have config DRM_WERROR depend on !WERROR
Hi,
On 5/16/24 16:33, Jani Nikula wrote:
If WERROR is already enabled, there's no point in enabling DRM_WERROR or
asking users about it.
Reported-by: Linus Torvalds
Closes:
https://lore.kernel.org/r/CAHk-=whxT8D_0j=bjtrvj-O=veojn6gw8gk4j2v+biduntz...@mail.gmail.com
Fixes: f89632a9e5fa ("drm: Add CONFIG_DRM_WERROR")
Signed-off-by: Jani Nikula
Wow, you successfully get Linus's attention, haha.
---
drivers/gpu/drm/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 026444eeb5c6..d0aa277fc3bf 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -450,6 +450,7 @@ config DRM_PRIVACY_SCREEN
config DRM_WERROR
bool "Compile the drm subsystem with warnings as errors"
depends on DRM && EXPERT
+ depends on !WERROR
default n
help
A kernel build should not cause any compiler warnings, and this
--
Best regards
Sui
Re: [PATCH] drm/bridge: adv7511: Exit interrupt handling when necessary
Hi,
On 5/16/24 18:10, Liu Ying wrote:
Commit f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
fails to consider the case where adv7511->i2c_main->irq is zero, i.e.,
no interrupt requested at all.
Without interrupt, adv7511_wait_for_edid() could return -EIO sometimes,
because it polls adv7511->edid_read flag by calling adv7511_irq_process()
a few times, but adv7511_irq_process() happens to refuse to handle
interrupt by returning -ENODATA. Hence, EDID retrieval fails randomly.
Fix the issue by checking adv7511->i2c_main->irq before exiting interrupt
handling from adv7511_irq_process().
Fixes: f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
Signed-off-by: Liu Ying
Acked-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 6089b0bb9321..2074fa3c1b7b 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -479,7 +479,8 @@ static int adv7511_irq_process(struct adv7511 *adv7511,
bool process_hpd)
return ret;
/* If there is no IRQ to handle, exit indicating no IRQ data */
- if (!(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
+ if (adv7511->i2c_main->irq &&
+ !(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
!(irq1 & ADV7511_INT1_DDC_ERROR))
return -ENODATA;
--
Best regards
Sui
Re: [PATCH 0/2] drm/bridge: Add 'struct device *' field to the drm_bridge structure
On 5/16/24 18:40, Sui Jingfeng wrote: use 'to_i2c_client(bridge->dev)' to retrieve the pointer to_i2c_client(bridge->kdev). Besides, this also means that we don't need to add the fwnode pointer into struct drm_bridge as member. Relief the conflicts with other reviewers if the work of switching to fwnode is still needed. As for majorities cases (1 to 1), of_node and fwnode can be retrieved with 'struct device *' easily. The aux-bridge.c and aux-hdp-bridge.c can also be converted too easily. of_node, fwnode, swnode and device properties are all belong to the backing device structure itself. It can be more natural to use device_proterty_read_xxx() APIs after init time, Which in turn avoid the need to acquire and duplicate all properties another time in the driver private structure. We could do the programming around the 'struct device *.', remove a batch of boilerplate.
Re: [PATCH] drm/bridge: adv7511: Exit interrupt handling when necessary
Hi,
On 5/16/24 18:10, Liu Ying wrote:
Commit f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
fails to consider the case where adv7511->i2c_main->irq is zero, i.e.,
no interrupt requested at all.
Without interrupt, adv7511_wait_for_edid() could return -EIO sometimes,
because it polls adv7511->edid_read flag by calling adv7511_irq_process()
a few times, but adv7511_irq_process() happens to refuse to handle
interrupt by returning -ENODATA. Hence, EDID retrieval fails randomly.
Fix the issue by checking adv7511->i2c_main->irq before exiting interrupt
handling from adv7511_irq_process().
Fixes: f3d9683346d6 ("drm/bridge: adv7511: Allow IRQ to share GPIO pins")
Signed-off-by: Liu Ying
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 6089b0bb9321..2074fa3c1b7b 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -479,7 +479,8 @@ static int adv7511_irq_process(struct adv7511 *adv7511,
bool process_hpd)
return ret;
/* If there is no IRQ to handle, exit indicating no IRQ data */
- if (!(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
+ if (adv7511->i2c_main->irq &&
Maybe you could use 'if (process_hpd)' here.
+ !(irq0 & (ADV7511_INT0_HPD | ADV7511_INT0_EDID_READY)) &&
!(irq1 & ADV7511_INT1_DDC_ERROR))
return -ENODATA;
--
Best regards
Sui
Re: [PATCH 0/2] drm/bridge: Add 'struct device *' field to the drm_bridge structure
Hi, On 5/16/24 16:25, Maxime Ripard wrote: On Wed, May 15, 2024 at 11:19:58PM +0800, Sui Jingfeng wrote: Hi, On 5/15/24 22:58, Maxime Ripard wrote: On Wed, May 15, 2024 at 10:53:00PM +0800, Sui Jingfeng wrote: On 5/15/24 22:30, Maxime Ripard wrote: On Wed, May 15, 2024 at 12:53:33AM +0800, Sui Jingfeng wrote: On 2024/5/15 00:22, Maxime Ripard wrote: Hi, On Tue, May 14, 2024 at 11:40:43PM +0800, Sui Jingfeng wrote: Because a lot of implementations has already added it into their drived class, promote it into drm_bridge core may benifits a lot. drm bridge is a driver, it should know the underlying hardware entity. Is there some actual benefits, or is it theoretical at this point? I think, DRM bridge drivers could remove the 'struct device *dev' member from their derived structure. Rely on the drm bridge core when they need the 'struct device *' pointer. Sure, but why do we need to do so? The other thread you had with Jani points out that it turns out that things are more complicated than "every bridge driver has a struct device anyway", it creates inconsistency in the API (bridges would have a struct device, but not other entities), and it looks like there's no use for it anyway. None of these things are deal-breaker by themselves, but if there's only downsides and no upside, it's not clear to me why we should do it at all. It can reduce boilerplate. You're still using a conditional here. It's for safety reason, prevent NULL pointer dereference. drm bridge can be seen as either a software entity or a device driver. It's fine to pass NULL if specific KMS drivers intend to see drm bridge as a pure software entity, and for internal use only. Both use cases are valid. Sorry, I don't follow you. We can't NULL dereference a pointer that doesn't exist. Please state why we should merge this series: what does it fix or improve, aside from the potential gain of making bridges declare one less pointer in their private structure. We could reduce more. Bridge driver instances also don't have to embed 'struct i2c_client *'. We could use 'to_i2c_client(bridge->dev)' to retrieve the pointer, where needed. Maxime -- Best regards Sui
Re: [PATCH 3/3] drm/loongson: Refactor lsdc device initialize and the output port
Hi, On 5/16/24 14:26, Markus Elfring wrote: … fullfill the implement under the new framework. fulfil the implementation? Please improve your change descriptions another bit. I'll accept you suggestions, with pleasure. Thanks. Regards, Markus -- Best regards Sui
Re: [PATCH 1/3] drm/loongson: Add helpers for creating subdevice
Hi, On 5/16/24 04:30, Markus Elfring wrote: In some display subsystems, the functionality of a PCI(e) device may too … of the functionality into child devices can helps to achieve better modularity, eaiser for understand and maintain. Add the loongson_create_platform_device() function to pove the way … Please avoid typos in such a change description. I was too hurry, sorry, my bad. Will be fixed at the next version. Regards, Markus -- Best regards Sui
Re: [07/11] drm/loongson/7a2000: convert to struct drm_edid
Hi, Jani I love your patch, thanks. On 2024/5/14 20:55, Jani Nikula wrote: Prefer the struct drm_edid based functions for reading the EDID and updating the connector. Signed-off-by: Jani Nikula --- Reviewed-by: Sui Jingfeng -- Best regards, Sui
Re: [06/11] drm/loongson/7a1000: convert to struct drm_edid
Hi, Jani I love your patch, thanks. On 2024/5/14 20:55, Jani Nikula wrote: Prefer the struct drm_edid based functions for reading the EDID and updating the connector. Signed-off-by: Jani Nikula --- Reviewed-by: Sui Jingfeng -- Best regards, Sui
Re: drm/bridge: adv7511: Attach next bridge without creating connector
Hi,
On 5/14/24 23:12, Laurent Pinchart wrote:
Hello,
On Tue, May 14, 2024 at 12:26:15AM +0800, Sui Jingfeng wrote:
On 5/13/24 16:02, Liu Ying wrote:
The connector is created by either this ADV7511 bridge driver or
any DRM device driver/previous bridge driver, so this ADV7511
bridge driver should not let the next bridge driver create connector.
If the next bridge is a HDMI connector, the next bridge driver
would fail to attach bridge from display_connector_attach() without
the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
In theory we could have another HDMI-to-something bridge connected to
the ADV7511 output, and that bridge could create a connector. However,
before commit 14b3cdbd0e5b the adv7511 driver didn't try to attach to
the next bridge, so it's clear that no platform support in mainline had
such a setup. It should be safe to set DRM_BRIDGE_ATTACH_NO_CONNECTOR
unconditionally here.
But what if there is a drm bridge prior to adv7511 but after the KMS
engine? Even though we are still safe if it doesn't create connector
by obeying modern rule.
Reviewed-by: Laurent Pinchart
Add that flag to drm_bridge_attach() function call in
adv7511_bridge_attach() to fix the issue.
This fixes the issue where the HDMI connector bridge fails to attach
to the previous ADV7535 bridge on i.MX8MP EVK platform:
[2.216442] [drm:drm_bridge_attach] *ERROR* failed to attach bridge
/hdmi-connector to encoder None-37: -22
[2.220675] mmc1: SDHCI controller on 30b5.mmc [30b5.mmc] using ADMA
[2.226262] [drm:drm_bridge_attach] *ERROR* failed to attach bridge
/soc@0/bus@3080/i2c@30a3/hdmi@3d to encoder None-37: -22
[2.245204] [drm:drm_bridge_attach] *ERROR* failed to attach bridge
/soc@0/bus@32c0/dsi@32e6 to encoder None-37: -22
[2.256445] imx-lcdif 32e8.display-controller: error -EINVAL: Failed to
attach bridge for endpoint0
[2.265850] imx-lcdif 32e8.display-controller: error -EINVAL: Cannot
connect bridge
[2.274009] imx-lcdif 32e8.display-controller: probe with driver
imx-lcdif failed with error -22
Fixes: 14b3cdbd0e5b ("drm/bridge: adv7511: make it honour next bridge in DT")
Signed-off-by: Liu Ying
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index dd21b81bd28f..66ccb61e2a66 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -953,7 +953,8 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
int ret = 0;
if (adv->next_bridge) {
- ret = drm_bridge_attach(bridge->encoder, adv->next_bridge,
bridge, flags);
+ ret = drm_bridge_attach(bridge->encoder, adv->next_bridge,
bridge,
+ flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR);
As a side note, I think, maybe you could do better in the future.
If we know that the KMS display driver side has the HDMI connector
already created for us, we should pass DRM_BRIDGE_ATTACH_NO_CONNECTOR
from the root KMS driver side. Which is to forbidden all potential
drm bridge drivers to create a connector in the middle.
That's the recommended way for new drivers. Using the
drm_bridge_connector helper handles all this for you.
The KMS display driver side could parse the DT to know if there is
a hdmi connector, or merely just hdmi connector device node, or
something else.
No, that would violate the basic principle of not peeking into the DT of
devices you know nothing about. The display engine driver can't walk the
pipeline in DT and expect to understand all the DT nodes on the path,
and what their properties mean.
The (next) bridge at the remote port is not necessary a display bridge.
Or it is a bridge from the perspective of hardware viewpoint, but under
controlled by a more complex foreign driver which generic drm bridge
driver has no authority to attach.
What KMS drivers should do is to use the drm_bridge_connector helper.
Calling drm_bridge_connector_init() will create a connector for a chain
of bridges. The KMS driver should then attach to the first bridge with
DRM_BRIDGE_ATTACH_NO_CONNECTOR, unconditionally.
OK, thanks for teaching us the modern way to use drm_bridge_connector
helper, also thanks Ying for providing the patch.
However, other maintainer and/or reviewer's opinion are of cause
more valuable. I send a A-b because I thought the bug is urgency
and it's probably more important to solve this bug first. And
maybe you can Cc: if you like.
if (ret)
return ret;
}
--
Best regards
Sui
Re: [PATCH 0/2] drm/bridge: Add 'struct device *' field to the drm_bridge structure
Hi, On 5/15/24 22:58, Maxime Ripard wrote: On Wed, May 15, 2024 at 10:53:00PM +0800, Sui Jingfeng wrote: On 5/15/24 22:30, Maxime Ripard wrote: On Wed, May 15, 2024 at 12:53:33AM +0800, Sui Jingfeng wrote: On 2024/5/15 00:22, Maxime Ripard wrote: Hi, On Tue, May 14, 2024 at 11:40:43PM +0800, Sui Jingfeng wrote: Because a lot of implementations has already added it into their drived class, promote it into drm_bridge core may benifits a lot. drm bridge is a driver, it should know the underlying hardware entity. Is there some actual benefits, or is it theoretical at this point? I think, DRM bridge drivers could remove the 'struct device *dev' member from their derived structure. Rely on the drm bridge core when they need the 'struct device *' pointer. Sure, but why do we need to do so? The other thread you had with Jani points out that it turns out that things are more complicated than "every bridge driver has a struct device anyway", it creates inconsistency in the API (bridges would have a struct device, but not other entities), and it looks like there's no use for it anyway. None of these things are deal-breaker by themselves, but if there's only downsides and no upside, it's not clear to me why we should do it at all. It can reduce boilerplate. You're still using a conditional here. It's for safety reason, prevent NULL pointer dereference. drm bridge can be seen as either a software entity or a device driver. It's fine to pass NULL if specific KMS drivers intend to see drm bridge as a pure software entity, and for internal use only. Both use cases are valid. Maxime -- Best regards Sui
Re: [PATCH 0/2] drm/bridge: Add 'struct device *' field to the drm_bridge structure
Hi, On 5/15/24 22:30, Maxime Ripard wrote: On Wed, May 15, 2024 at 12:53:33AM +0800, Sui Jingfeng wrote: Hi, On 2024/5/15 00:22, Maxime Ripard wrote: Hi, On Tue, May 14, 2024 at 11:40:43PM +0800, Sui Jingfeng wrote: Because a lot of implementations has already added it into their drived class, promote it into drm_bridge core may benifits a lot. drm bridge is a driver, it should know the underlying hardware entity. Is there some actual benefits, or is it theoretical at this point? I think, DRM bridge drivers could remove the 'struct device *dev' member from their derived structure. Rely on the drm bridge core when they need the 'struct device *' pointer. Sure, but why do we need to do so? The other thread you had with Jani points out that it turns out that things are more complicated than "every bridge driver has a struct device anyway", it creates inconsistency in the API (bridges would have a struct device, but not other entities), and it looks like there's no use for it anyway. None of these things are deal-breaker by themselves, but if there's only downsides and no upside, it's not clear to me why we should do it at all. It can reduce boilerplate. Maxime -- Best regards Sui
Re: [PATCH 1/2] drm/bridge: Support finding bridge with struct device
Hi,
On 5/15/24 18:28, Jani Nikula wrote:
On Wed, 15 May 2024, Sui Jingfeng wrote:
Hi,
On 5/15/24 17:39, Jani Nikula wrote:
On Tue, 14 May 2024, Sui Jingfeng wrote:
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 584d109330ab..1928d9d0dd3c 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -213,6 +213,23 @@ void drm_bridge_add(struct drm_bridge *bridge)
}
EXPORT_SYMBOL(drm_bridge_add);
+/**
+ * drm_bridge_add_with_dev - add the given bridge to the global bridge list
+ *
+ * @bridge: bridge control structure
+ * @dev: pointer to the kernel device that this bridge is backed.
+ */
+void drm_bridge_add_with_dev(struct drm_bridge *bridge, struct device *dev)
+{
+ if (dev) {
+ bridge->kdev = dev;
+ bridge->of_node = dev->of_node;
+ }
+
+ drm_bridge_add(bridge);
+}
+EXPORT_SYMBOL_GPL(drm_bridge_add_with_dev);
I don't actually have an opinion on whether the dev parameter is useful
or not.
But please don't add a drm_bridge_add_with_dev() and then convert more
than half the drm_bridge_add() users to that. Please just add a struct
device *dev parameter to drm_bridge_add(), and pass NULL if it's not
relevant.
To be honest, previously, I'm just do it exactly same as the way you
told me here. But I'm exhausted and finally give up.
Because this is again need me to modify *all* callers of
drm_bridge_add(), not only those bridges in drm/bridge/, but also
bridge instances in various KMS drivers.
However, their some exceptions just don't fit!
For example, the imx/imx8qxp-pixel-combiner.c just don't fit our
simple model. Our helper function assume that one device backing
one drm_bridge instance (1 to 1). Yet, that driver backing two or
more bridges with one platform device (1 to 2, 1 to 3, ..., ).
Hence, the imx/imx8qxp-pixel-combiner.c just can't use
drm_bridge_add_with_dev().
The aux_hpd_bridge.c is also bad, it store the of_node of struct device
at the .platform_data member of the struct device.
Like I said, "pass NULL if it's not relevant."
OK, good idea.
"_add_with_dev" is a terrible function name.
What if you need to add another parameter later? Add _add_with_foo and
_add_with_dev_and_foo variants?
Yes, you are right, I'll back give another try then.
BR,
Jani.
--
Best regards
Sui
Re: [PATCH 1/2] drm/bridge: Support finding bridge with struct device
Hi,
On 5/15/24 17:39, Jani Nikula wrote:
On Tue, 14 May 2024, Sui Jingfeng wrote:
The pointer of 'struct device' can also be used as a key to search drm
bridge instance from the global bridge list, traditionally, fwnode and
'OF' based APIs requires the system has decent fwnode/OF Graph support.
While the drm_find_bridge_by_dev() function introduced in this series
don't has such a restriction. It only require you has a pointer to the
backing device. Hence, it may suitable for some small and/or limited
display subsystems.
Also add the drm_bridge_add_with_dev() as a helper, which automatically
set the .of_node field of drm_bridge instances if you call it. But it
suitable for simple bridge drivers which one device backing one drm_bridge
instance.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/drm_bridge.c | 39
include/drm/drm_bridge.h | 5 +
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 584d109330ab..1928d9d0dd3c 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -213,6 +213,23 @@ void drm_bridge_add(struct drm_bridge *bridge)
}
EXPORT_SYMBOL(drm_bridge_add);
+/**
+ * drm_bridge_add_with_dev - add the given bridge to the global bridge list
+ *
+ * @bridge: bridge control structure
+ * @dev: pointer to the kernel device that this bridge is backed.
+ */
+void drm_bridge_add_with_dev(struct drm_bridge *bridge, struct device *dev)
+{
+ if (dev) {
+ bridge->kdev = dev;
+ bridge->of_node = dev->of_node;
+ }
+
+ drm_bridge_add(bridge);
+}
+EXPORT_SYMBOL_GPL(drm_bridge_add_with_dev);
I don't actually have an opinion on whether the dev parameter is useful
or not.
But please don't add a drm_bridge_add_with_dev() and then convert more
than half the drm_bridge_add() users to that. Please just add a struct
device *dev parameter to drm_bridge_add(), and pass NULL if it's not
relevant.
To be honest, previously, I'm just do it exactly same as the way you
told me here. But I'm exhausted and finally give up.
Because this is again need me to modify *all* callers of
drm_bridge_add(), not only those bridges in drm/bridge/, but also
bridge instances in various KMS drivers.
However, their some exceptions just don't fit!
For example, the imx/imx8qxp-pixel-combiner.c just don't fit our
simple model. Our helper function assume that one device backing
one drm_bridge instance (1 to 1). Yet, that driver backing two or
more bridges with one platform device (1 to 2, 1 to 3, ..., ).
Hence, the imx/imx8qxp-pixel-combiner.c just can't use
drm_bridge_add_with_dev().
The aux_hpd_bridge.c is also bad, it store the of_node of struct device
at the .platform_data member of the struct device.
BR,
Jani.
+
static void drm_bridge_remove_void(void *bridge)
{
drm_bridge_remove(bridge);
@@ -1334,6 +1351,27 @@ void drm_bridge_hpd_notify(struct drm_bridge *bridge,
}
EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify);
+struct drm_bridge *drm_find_bridge_by_dev(struct device *kdev)
+{
+ struct drm_bridge *bridge;
+
+ if (!kdev)
+ return NULL;
+
+ mutex_lock(_lock);
+
+ list_for_each_entry(bridge, _list, list) {
+ if (bridge->kdev == kdev) {
+ mutex_unlock(_lock);
+ return bridge;
+ }
+ }
+
+ mutex_unlock(_lock);
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(drm_find_bridge_by_dev);
+
#ifdef CONFIG_OF
/**
* of_drm_find_bridge - find the bridge corresponding to the device node in
@@ -1361,6 +1399,7 @@ struct drm_bridge *of_drm_find_bridge(struct device_node
*np)
return NULL;
}
EXPORT_SYMBOL(of_drm_find_bridge);
+
#endif
MODULE_AUTHOR("Ajay Kumar ");
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4baca0d9107b..70d8393bbd9c 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -715,6 +715,8 @@ struct drm_bridge {
struct drm_private_obj base;
/** @dev: DRM device this bridge belongs to */
struct drm_device *dev;
+ /** @kdev: pointer to the kernel device backing this bridge */
+ struct device *kdev;
/** @encoder: encoder to which this bridge is connected */
struct drm_encoder *encoder;
/** @chain_node: used to form a bridge chain */
@@ -782,12 +784,15 @@ drm_priv_to_bridge(struct drm_private_obj *priv)
}
void drm_bridge_add(struct drm_bridge *bridge);
+void drm_bridge_add_with_dev(struct drm_bridge *bridge, struct device *dev);
int devm_drm_bridge_add(struct device *dev, struct drm_bridge *bridge);
void drm_bridge_remove(struct drm_bridge *bridge);
int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
struct drm_bridge *previous,
enum drm_bridge_attach_flags flags);
Re: [PATCH 0/2] drm/bridge: Add 'struct device *' field to the drm_bridge structure
Hi, On 2024/5/15 00:22, Maxime Ripard wrote: Hi, On Tue, May 14, 2024 at 11:40:43PM +0800, Sui Jingfeng wrote: Because a lot of implementations has already added it into their drived class, promote it into drm_bridge core may benifits a lot. drm bridge is a driver, it should know the underlying hardware entity. Is there some actual benefits, or is it theoretical at this point? I think, DRM bridge drivers could remove the 'struct device *dev' member from their derived structure. Rely on the drm bridge core when they need the 'struct device *' pointer. Maxime -- Best regards, Sui
[PATCH 1/2] drm/bridge: Support finding bridge with struct device
The pointer of 'struct device' can also be used as a key to search drm
bridge instance from the global bridge list, traditionally, fwnode and
'OF' based APIs requires the system has decent fwnode/OF Graph support.
While the drm_find_bridge_by_dev() function introduced in this series
don't has such a restriction. It only require you has a pointer to the
backing device. Hence, it may suitable for some small and/or limited
display subsystems.
Also add the drm_bridge_add_with_dev() as a helper, which automatically
set the .of_node field of drm_bridge instances if you call it. But it
suitable for simple bridge drivers which one device backing one drm_bridge
instance.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/drm_bridge.c | 39
include/drm/drm_bridge.h | 5 +
2 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 584d109330ab..1928d9d0dd3c 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -213,6 +213,23 @@ void drm_bridge_add(struct drm_bridge *bridge)
}
EXPORT_SYMBOL(drm_bridge_add);
+/**
+ * drm_bridge_add_with_dev - add the given bridge to the global bridge list
+ *
+ * @bridge: bridge control structure
+ * @dev: pointer to the kernel device that this bridge is backed.
+ */
+void drm_bridge_add_with_dev(struct drm_bridge *bridge, struct device *dev)
+{
+ if (dev) {
+ bridge->kdev = dev;
+ bridge->of_node = dev->of_node;
+ }
+
+ drm_bridge_add(bridge);
+}
+EXPORT_SYMBOL_GPL(drm_bridge_add_with_dev);
+
static void drm_bridge_remove_void(void *bridge)
{
drm_bridge_remove(bridge);
@@ -1334,6 +1351,27 @@ void drm_bridge_hpd_notify(struct drm_bridge *bridge,
}
EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify);
+struct drm_bridge *drm_find_bridge_by_dev(struct device *kdev)
+{
+ struct drm_bridge *bridge;
+
+ if (!kdev)
+ return NULL;
+
+ mutex_lock(_lock);
+
+ list_for_each_entry(bridge, _list, list) {
+ if (bridge->kdev == kdev) {
+ mutex_unlock(_lock);
+ return bridge;
+ }
+ }
+
+ mutex_unlock(_lock);
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(drm_find_bridge_by_dev);
+
#ifdef CONFIG_OF
/**
* of_drm_find_bridge - find the bridge corresponding to the device node in
@@ -1361,6 +1399,7 @@ struct drm_bridge *of_drm_find_bridge(struct device_node
*np)
return NULL;
}
EXPORT_SYMBOL(of_drm_find_bridge);
+
#endif
MODULE_AUTHOR("Ajay Kumar ");
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4baca0d9107b..70d8393bbd9c 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -715,6 +715,8 @@ struct drm_bridge {
struct drm_private_obj base;
/** @dev: DRM device this bridge belongs to */
struct drm_device *dev;
+ /** @kdev: pointer to the kernel device backing this bridge */
+ struct device *kdev;
/** @encoder: encoder to which this bridge is connected */
struct drm_encoder *encoder;
/** @chain_node: used to form a bridge chain */
@@ -782,12 +784,15 @@ drm_priv_to_bridge(struct drm_private_obj *priv)
}
void drm_bridge_add(struct drm_bridge *bridge);
+void drm_bridge_add_with_dev(struct drm_bridge *bridge, struct device *dev);
int devm_drm_bridge_add(struct device *dev, struct drm_bridge *bridge);
void drm_bridge_remove(struct drm_bridge *bridge);
int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
struct drm_bridge *previous,
enum drm_bridge_attach_flags flags);
+struct drm_bridge *drm_find_bridge_by_dev(struct device *kdev);
+
#ifdef CONFIG_OF
struct drm_bridge *of_drm_find_bridge(struct device_node *np);
#else
--
2.43.0
[PATCH 2/2] drm/bridge: Switch to use drm_bridge_add_with_dev()
Normally, the drm_bridge::of_node member won't be used by drm bridge driver
instances, as display driver instances will use the device node fetched
from its backing device directly. The drm_bridge::of_node is mainly for
finding the drm bridge instance associated. Hence, display bridge drivers
don't have to set it manually for the canonical cases.
Let's reduce the boilerplates by using drm_bridge_add_with_dev().
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 +--
drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 4 +---
drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 4 +---
drivers/gpu/drm/bridge/analogix/anx7625.c| 3 +--
drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 3 +--
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 3 +--
drivers/gpu/drm/bridge/chipone-icn6211.c | 5 ++---
drivers/gpu/drm/bridge/chrontel-ch7033.c | 3 +--
drivers/gpu/drm/bridge/cros-ec-anx7688.c | 4 +---
drivers/gpu/drm/bridge/display-connector.c | 3 +--
drivers/gpu/drm/bridge/fsl-ldb.c | 3 +--
drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 3 +--
drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c | 3 +--
drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c | 3 +--
drivers/gpu/drm/bridge/ite-it6505.c | 3 +--
drivers/gpu/drm/bridge/ite-it66121.c | 3 +--
drivers/gpu/drm/bridge/lontium-lt8912b.c | 3 +--
drivers/gpu/drm/bridge/lontium-lt9211.c | 3 +--
drivers/gpu/drm/bridge/lontium-lt9611.c | 3 +--
drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 3 +--
drivers/gpu/drm/bridge/lvds-codec.c | 3 +--
drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 3 +--
drivers/gpu/drm/bridge/microchip-lvds.c | 3 +--
drivers/gpu/drm/bridge/nwl-dsi.c | 3 +--
drivers/gpu/drm/bridge/nxp-ptn3460.c | 3 +--
drivers/gpu/drm/bridge/panel.c | 3 +--
drivers/gpu/drm/bridge/parade-ps8622.c | 3 +--
drivers/gpu/drm/bridge/parade-ps8640.c | 1 -
drivers/gpu/drm/bridge/samsung-dsim.c| 3 +--
drivers/gpu/drm/bridge/sii902x.c | 3 +--
drivers/gpu/drm/bridge/sii9234.c | 3 +--
drivers/gpu/drm/bridge/sil-sii8620.c | 3 +--
drivers/gpu/drm/bridge/simple-bridge.c | 3 +--
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 3 +--
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c| 3 +--
drivers/gpu/drm/bridge/tc358762.c| 3 +--
drivers/gpu/drm/bridge/tc358764.c| 3 +--
drivers/gpu/drm/bridge/tc358767.c| 3 +--
drivers/gpu/drm/bridge/tc358768.c| 3 +--
drivers/gpu/drm/bridge/tc358775.c| 3 +--
drivers/gpu/drm/bridge/thc63lvd1024.c| 3 +--
drivers/gpu/drm/bridge/ti-dlpc3433.c | 3 +--
drivers/gpu/drm/bridge/ti-sn65dsi83.c| 3 +--
drivers/gpu/drm/bridge/ti-sn65dsi86.c| 3 +--
drivers/gpu/drm/bridge/ti-tfp410.c | 3 +--
drivers/gpu/drm/bridge/ti-tpd12s015.c| 3 +--
drivers/gpu/drm/i2c/tda998x_drv.c| 5 +
47 files changed, 47 insertions(+), 99 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 6089b0bb9321..8e4a95584ad8 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -1318,10 +1318,9 @@ static int adv7511_probe(struct i2c_client *i2c)
if (adv7511->i2c_main->irq)
adv7511->bridge.ops |= DRM_BRIDGE_OP_HPD;
- adv7511->bridge.of_node = dev->of_node;
adv7511->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
- drm_bridge_add(>bridge);
+ drm_bridge_add_with_dev(>bridge, dev);
adv7511_audio_init(dev, adv7511);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
index b754947e3e00..f4f3298404d2 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
@@ -697,8 +697,6 @@ static int anx6345_i2c_probe(struct i2c_client *client)
mutex_init(>lock);
- anx6345->bridge.of_node = client->dev.of_node;
-
anx6345->client = client;
i2c_set_clientdata(client, anx6345);
@@ -766,7 +764,7 @@ static int anx6345_i2c_probe(struct i2c_client *client)
anx6345_poweron(anx6345);
if (anx6345_get_chip_id(anx6345)) {
a
[PATCH 0/2] drm/bridge: Add 'struct device *' field to the drm_bridge structure
Because a lot of implementations has already added it into their drived class, promote it into drm_bridge core may benifits a lot. drm bridge is a driver, it should know the underlying hardware entity. Sui Jingfeng (2): drm/bridge: Support finding bridge with struct device drm/bridge: Switch to use drm_bridge_add_with_dev() drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 +- .../drm/bridge/analogix/analogix-anx6345.c| 4 +- .../drm/bridge/analogix/analogix-anx78xx.c| 4 +- drivers/gpu/drm/bridge/analogix/anx7625.c | 3 +- .../gpu/drm/bridge/cadence/cdns-dsi-core.c| 3 +- .../drm/bridge/cadence/cdns-mhdp8546-core.c | 3 +- drivers/gpu/drm/bridge/chipone-icn6211.c | 5 +-- drivers/gpu/drm/bridge/chrontel-ch7033.c | 3 +- drivers/gpu/drm/bridge/cros-ec-anx7688.c | 4 +- drivers/gpu/drm/bridge/display-connector.c| 3 +- drivers/gpu/drm/bridge/fsl-ldb.c | 3 +- drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 3 +- .../gpu/drm/bridge/imx/imx8qxp-pixel-link.c | 3 +- drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c | 3 +- drivers/gpu/drm/bridge/ite-it6505.c | 3 +- drivers/gpu/drm/bridge/ite-it66121.c | 3 +- drivers/gpu/drm/bridge/lontium-lt8912b.c | 3 +- drivers/gpu/drm/bridge/lontium-lt9211.c | 3 +- drivers/gpu/drm/bridge/lontium-lt9611.c | 3 +- drivers/gpu/drm/bridge/lontium-lt9611uxc.c| 3 +- drivers/gpu/drm/bridge/lvds-codec.c | 3 +- .../bridge/megachips-stdp-ge-b850v3-fw.c | 3 +- drivers/gpu/drm/bridge/microchip-lvds.c | 3 +- drivers/gpu/drm/bridge/nwl-dsi.c | 3 +- drivers/gpu/drm/bridge/nxp-ptn3460.c | 3 +- drivers/gpu/drm/bridge/panel.c| 3 +- drivers/gpu/drm/bridge/parade-ps8622.c| 3 +- drivers/gpu/drm/bridge/parade-ps8640.c| 1 - drivers/gpu/drm/bridge/samsung-dsim.c | 3 +- drivers/gpu/drm/bridge/sii902x.c | 3 +- drivers/gpu/drm/bridge/sii9234.c | 3 +- drivers/gpu/drm/bridge/sil-sii8620.c | 3 +- drivers/gpu/drm/bridge/simple-bridge.c| 3 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 +- drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 3 +- drivers/gpu/drm/bridge/tc358762.c | 3 +- drivers/gpu/drm/bridge/tc358764.c | 3 +- drivers/gpu/drm/bridge/tc358767.c | 3 +- drivers/gpu/drm/bridge/tc358768.c | 3 +- drivers/gpu/drm/bridge/tc358775.c | 3 +- drivers/gpu/drm/bridge/thc63lvd1024.c | 3 +- drivers/gpu/drm/bridge/ti-dlpc3433.c | 3 +- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 3 +- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +- drivers/gpu/drm/bridge/ti-tfp410.c| 3 +- drivers/gpu/drm/bridge/ti-tpd12s015.c | 3 +- drivers/gpu/drm/drm_bridge.c | 39 +++ drivers/gpu/drm/i2c/tda998x_drv.c | 5 +-- include/drm/drm_bridge.h | 5 +++ 49 files changed, 91 insertions(+), 99 deletions(-) -- 2.43.0
Re: drm/bridge: adv7511: Attach next bridge without creating connector
Hi,
On 5/13/24 16:02, Liu Ying wrote:
The connector is created by either this ADV7511 bridge driver or
any DRM device driver/previous bridge driver, so this ADV7511
bridge driver should not let the next bridge driver create connector.
If the next bridge is a HDMI connector, the next bridge driver
would fail to attach bridge from display_connector_attach() without
the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
Add that flag to drm_bridge_attach() function call in
adv7511_bridge_attach() to fix the issue.
This fixes the issue where the HDMI connector bridge fails to attach
to the previous ADV7535 bridge on i.MX8MP EVK platform:
[2.216442] [drm:drm_bridge_attach] *ERROR* failed to attach bridge
/hdmi-connector to encoder None-37: -22
[2.220675] mmc1: SDHCI controller on 30b5.mmc [30b5.mmc] using ADMA
[2.226262] [drm:drm_bridge_attach] *ERROR* failed to attach bridge
/soc@0/bus@3080/i2c@30a3/hdmi@3d to encoder None-37: -22
[2.245204] [drm:drm_bridge_attach] *ERROR* failed to attach bridge
/soc@0/bus@32c0/dsi@32e6 to encoder None-37: -22
[2.256445] imx-lcdif 32e8.display-controller: error -EINVAL: Failed to
attach bridge for endpoint0
[2.265850] imx-lcdif 32e8.display-controller: error -EINVAL: Cannot
connect bridge
[2.274009] imx-lcdif 32e8.display-controller: probe with driver
imx-lcdif failed with error -22
Fixes: 14b3cdbd0e5b ("drm/bridge: adv7511: make it honour next bridge in DT")
Signed-off-by: Liu Ying
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index dd21b81bd28f..66ccb61e2a66 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -953,7 +953,8 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
int ret = 0;
if (adv->next_bridge) {
- ret = drm_bridge_attach(bridge->encoder, adv->next_bridge,
bridge, flags);
+ ret = drm_bridge_attach(bridge->encoder, adv->next_bridge,
bridge,
+ flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR);
As a side note, I think, maybe you could do better in the future.
If we know that the KMS display driver side has the HDMI connector
already created for us, we should pass DRM_BRIDGE_ATTACH_NO_CONNECTOR
from the root KMS driver side. Which is to forbidden all potential
drm bridge drivers to create a connector in the middle.
The KMS display driver side could parse the DT to know if there is
a hdmi connector, or merely just hdmi connector device node, or
something else.
However, other maintainer and/or reviewer's opinion are of cause
more valuable. I send a A-b because I thought the bug is urgency
and it's probably more important to solve this bug first. And
maybe you can Cc: if you like.
Thanks.
if (ret)
return ret;
}
--
Best regards
Sui Jingfeng
Re: drm/bridge: adv7511: Attach next bridge without creating connector
Hi,
On 5/13/24 16:02, Liu Ying wrote:
The connector is created by either this ADV7511 bridge driver or
any DRM device driver/previous bridge driver, so this ADV7511
bridge driver should not let the next bridge driver create connector.
If the next bridge is a HDMI connector, the next bridge driver
would fail to attach bridge from display_connector_attach() without
the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
Add that flag to drm_bridge_attach() function call in
adv7511_bridge_attach() to fix the issue.
This fixes the issue where the HDMI connector bridge fails to attach
to the previous ADV7535 bridge on i.MX8MP EVK platform:
[2.216442] [drm:drm_bridge_attach] *ERROR* failed to attach bridge
/hdmi-connector to encoder None-37: -22
[2.220675] mmc1: SDHCI controller on 30b5.mmc [30b5.mmc] using ADMA
[2.226262] [drm:drm_bridge_attach] *ERROR* failed to attach bridge
/soc@0/bus@3080/i2c@30a3/hdmi@3d to encoder None-37: -22
[2.245204] [drm:drm_bridge_attach] *ERROR* failed to attach bridge
/soc@0/bus@32c0/dsi@32e6 to encoder None-37: -22
[2.256445] imx-lcdif 32e8.display-controller: error -EINVAL: Failed to
attach bridge for endpoint0
[2.265850] imx-lcdif 32e8.display-controller: error -EINVAL: Cannot
connect bridge
[2.274009] imx-lcdif 32e8.display-controller: probe with driver
imx-lcdif failed with error -22
Indeed, looks safer finally.
Fixes: 14b3cdbd0e5b ("drm/bridge: adv7511: make it honour next bridge in DT")
Signed-off-by: Liu Ying
Acked-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index dd21b81bd28f..66ccb61e2a66 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -953,7 +953,8 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge,
int ret = 0;
if (adv->next_bridge) {
- ret = drm_bridge_attach(bridge->encoder, adv->next_bridge,
bridge, flags);
+ ret = drm_bridge_attach(bridge->encoder, adv->next_bridge,
bridge,
+ flags | DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret)
return ret;
}
--
Best regards
Sui Jingfeng
[PATCH v2 12/12] drm/bridge: analogix: Remove redundant checks on existence of bridge->encoder
The checks on the existence of bridge->encoder in the implementation of
drm_bridge_funcs::attach() is not necessary, as it has already been checked
in the drm_bridge_attach() function call by previous bridge or KMS driver.
The drm_bridge_attach() will quit with a negative error code returned if
it fails for some reasons, hence, it is guaranteed that the .encoder member
of the drm_bridge instance is not NULL when various bridge attach functions
are called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 5 -
drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 5 -
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 -
drivers/gpu/drm/bridge/analogix/anx7625.c | 10 --
4 files changed, 25 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
index c9e35731e6a1..cfe43d2ca3be 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
@@ -528,11 +528,6 @@ static int anx6345_bridge_attach(struct drm_bridge *bridge,
return -EINVAL;
}
- if (!bridge->encoder) {
- DRM_ERROR("Parent encoder object not found");
- return -ENODEV;
- }
-
/* Register aux channel */
anx6345->aux.name = "DP-AUX";
anx6345->aux.dev = >client->dev;
diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
index 5748a8581af4..58875dde496f 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
@@ -897,11 +897,6 @@ static int anx78xx_bridge_attach(struct drm_bridge *bridge,
return -EINVAL;
}
- if (!bridge->encoder) {
- DRM_ERROR("Parent encoder object not found");
- return -ENODEV;
- }
-
/* Register aux channel */
anx78xx->aux.name = "DP-AUX";
anx78xx->aux.dev = >client->dev;
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index df9370e0ff23..7b841232321f 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1228,11 +1228,6 @@ static int analogix_dp_bridge_attach(struct drm_bridge
*bridge,
return -EINVAL;
}
- if (!bridge->encoder) {
- DRM_ERROR("Parent encoder object not found");
- return -ENODEV;
- }
-
if (!dp->plat_data->skip_connector) {
connector = >connector;
connector->polled = DRM_CONNECTOR_POLL_HPD;
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 59e9ad349969..3d09efa4199c 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2193,11 +2193,6 @@ static int anx7625_bridge_attach(struct drm_bridge
*bridge,
if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
return -EINVAL;
- if (!bridge->encoder) {
- DRM_DEV_ERROR(dev, "Parent encoder object not found");
- return -ENODEV;
- }
-
ctx->aux.drm_dev = bridge->dev;
err = drm_dp_aux_register(>aux);
if (err) {
@@ -2435,11 +2430,6 @@ static void anx7625_bridge_atomic_enable(struct
drm_bridge *bridge,
dev_dbg(dev, "drm atomic enable\n");
- if (!bridge->encoder) {
- dev_err(dev, "Parent encoder object not found");
- return;
- }
-
connector = drm_atomic_get_new_connector_for_encoder(state->base.state,
bridge->encoder);
if (!connector)
--
2.43.0
[PATCH v2 11/12] drm/bridge: imx: Remove redundant checks on existence of bridge->encoder
The checks on the existence of bridge->encoder in the implementation of
drm_bridge_funcs::attach() is not necessary, as it has already been checked
in the drm_bridge_attach() function call by previous bridge or KMS driver.
The drm_bridge_attach() will quit with a negative error code returned if
it fails for some reasons, hence, it is guaranteed that the .encoder member
of the drm_bridge instance is not NULL when various i.MX specific bridge
attach functions are called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/imx/imx-ldb-helper.c | 5 -
drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c | 5 -
drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c | 5 -
drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c| 5 -
4 files changed, 20 deletions(-)
diff --git a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
index 6967325cd8ee..9b5bebbe357d 100644
--- a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
+++ b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
@@ -116,11 +116,6 @@ int ldb_bridge_attach_helper(struct drm_bridge *bridge,
return -EINVAL;
}
- if (!bridge->encoder) {
- DRM_DEV_ERROR(ldb->dev, "missing encoder\n");
- return -ENODEV;
- }
-
return drm_bridge_attach(bridge->encoder,
ldb_ch->next_bridge, bridge,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c
b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c
index d0868a6ac6c9..e6dbbdc87ce2 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c
@@ -119,11 +119,6 @@ static int imx8qxp_pc_bridge_attach(struct drm_bridge
*bridge,
return -EINVAL;
}
- if (!bridge->encoder) {
- DRM_DEV_ERROR(pc->dev, "missing encoder\n");
- return -ENODEV;
- }
-
return drm_bridge_attach(bridge->encoder,
ch->next_bridge, bridge,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c
b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c
index ed8b7a4e0e11..1d11cc1df43c 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c
@@ -138,11 +138,6 @@ static int imx8qxp_pixel_link_bridge_attach(struct
drm_bridge *bridge,
return -EINVAL;
}
- if (!bridge->encoder) {
- DRM_DEV_ERROR(pl->dev, "missing encoder\n");
- return -ENODEV;
- }
-
return drm_bridge_attach(bridge->encoder,
pl->next_bridge, bridge,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
index 4a886cb808ca..fb7cf4369bb8 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
@@ -58,11 +58,6 @@ static int imx8qxp_pxl2dpi_bridge_attach(struct drm_bridge
*bridge,
return -EINVAL;
}
- if (!bridge->encoder) {
- DRM_DEV_ERROR(p2d->dev, "missing encoder\n");
- return -ENODEV;
- }
-
return drm_bridge_attach(bridge->encoder,
p2d->next_bridge, bridge,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
--
2.43.0
[PATCH v2 10/12] drm/bridge: lt9611uxc: Remove a redundant check on existence of bridge->encoder
In the lt9611uxc_connector_init() function, the check on the existence
of bridge->encoder is not necessary, as it has already been checked in
the drm_bridge_attach() function. And the check on the drm bridge core
happens before check in the implementation. Hence, it is guaranteed that
the .encoder member of the struct drm_bridge is not NULL when
lt9611uxc_connector_init() function get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
index ab702471f3ab..f864c033ba81 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c
@@ -337,11 +337,6 @@ static int lt9611uxc_connector_init(struct drm_bridge
*bridge, struct lt9611uxc
{
int ret;
- if (!bridge->encoder) {
- DRM_ERROR("Parent encoder object not found");
- return -ENODEV;
- }
-
lt9611uxc->connector.polled = DRM_CONNECTOR_POLL_HPD;
drm_connector_helper_add(>connector,
--
2.43.0
[PATCH v2 09/12] drm/bridge: synopsys: dw-mipi-dsi: Remove a redundant check on existence of bridge->encoder
In the dw_mipi_dsi_bridge_attach() function, the check on the existence
of bridge->encoder is not necessary, as it has already been checked in
the drm_bridge_attach() function invocked by previous bridge or KMS driver.
The previous drm_bridge_attach() will quit with a negative error code
returned if it fails for some reasons, hence, it is guaranteed that the
.encoder member of the struct drm_bridge is not NULL when
dw_mipi_dsi_bridge_attach() function gets called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 824fb3c65742..c4e9d96933dc 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -1071,11 +1071,6 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge
*bridge,
{
struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
- if (!bridge->encoder) {
- DRM_ERROR("Parent encoder object not found\n");
- return -ENODEV;
- }
-
/* Set the encoder type as caller does not know it */
bridge->encoder->encoder_type = DRM_MODE_ENCODER_DSI;
--
2.43.0
[PATCH v2 08/12] drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: Remove a redundant check on existence of bridge->encoder
In the ge_b850v3_lvds_create_connector function, the check on the existence
of bridge->encoder is not necessary, as it has already been checked in the
drm_bridge_attach() function called by upstream bridge or driver. Hence, it
is guaranteed that the .encoder member of the drm_bridge instance is not
NULL when cdns_mhdp_connector_init() function get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
index 4480523244e4..37f1acf5c0f8 100644
--- a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
+++ b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c
@@ -165,11 +165,6 @@ static int ge_b850v3_lvds_create_connector(struct
drm_bridge *bridge)
struct drm_connector *connector = _b850v3_lvds_ptr->connector;
int ret;
- if (!bridge->encoder) {
- DRM_ERROR("Parent encoder object not found");
- return -ENODEV;
- }
-
connector->polled = DRM_CONNECTOR_POLL_HPD;
drm_connector_helper_add(connector,
--
2.43.0
[PATCH v2 07/12] drm/bridge: cdns-mhdp8546: Remove a redundant check on existence of bridge->encoder
In the cdns_mhdp_connector_init() function, the check on the existence
of bridge->encoder is not necessary, as it has already been checked in
the drm_bridge_attach() function. As the cdns_mhdp_connector_init() is
only called by cdns_mhdp_attach(), it is guaranteed that the .encoder
member of the struct drm_bridge is not NULL when cdns_mhdp_attach() gets
called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index 8a91ef0ae065..dee640ab1d3a 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -1697,11 +1697,6 @@ static int cdns_mhdp_connector_init(struct
cdns_mhdp_device *mhdp)
struct drm_bridge *bridge = >bridge;
int ret;
- if (!bridge->encoder) {
- dev_err(mhdp->dev, "Parent encoder object not found");
- return -ENODEV;
- }
-
conn->polled = DRM_CONNECTOR_POLL_HPD;
ret = drm_connector_init(bridge->dev, conn, _mhdp_conn_funcs,
--
2.43.0
[PATCH v2 06/12] drm/bridge: adv7511: Remove a redundant check on existence of bridge->encoder
In the adv7511_connector_init() function, the check on the existence of
bridge->encoder is not necessary. As it has already been checked in the
drm_bridge_attach() which happens prior to the adv7511_bridge_attach()
get called. Also note that the adv7511_connector_init() is only called by
adv7511_bridge_attach(). Hence, it is guaranteed that the .encoder member
of the drm_bridge instance is not NULL when adv7511_connector_init() get
called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index dd21b81bd28f..6089b0bb9321 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -877,11 +877,6 @@ static int adv7511_connector_init(struct adv7511 *adv)
struct drm_bridge *bridge = >bridge;
int ret;
- if (!bridge->encoder) {
- DRM_ERROR("Parent encoder object not found");
- return -ENODEV;
- }
-
if (adv->i2c_main->irq)
adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
else
--
2.43.0
[PATCH v2 05/12] drm/bridge: it6505: Remove a redundant check on existence of bridge->encoder
In it6505_bridge_attach(), the check on the existence of 'bridge->encoder'
is not necessary, as it has already been checked in the drm_bridge_attach()
which happens prior to it6505_bridge_attach() get called. Note that the
it6505_bridge_attach() will only be called by .attach() of the previous
bridge or KMS driver. The previous drm_bridge_attach() will quit with a
negative error code returned if it fails for some reasons. Hence, it is
guaranteed that the .encoder member of the drm_bridge instance is not NULL
when it6505_bridge_attach() function get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/ite-it6505.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
b/drivers/gpu/drm/bridge/ite-it6505.c
index 3f68c82888c2..469157341f3a 100644
--- a/drivers/gpu/drm/bridge/ite-it6505.c
+++ b/drivers/gpu/drm/bridge/ite-it6505.c
@@ -2882,11 +2882,6 @@ static int it6505_bridge_attach(struct drm_bridge
*bridge,
return -EINVAL;
}
- if (!bridge->encoder) {
- dev_err(dev, "Parent encoder object not found");
- return -ENODEV;
- }
-
/* Register aux channel */
it6505->aux.drm_dev = bridge->dev;
--
2.43.0
[PATCH v2 04/12] drm/bridge: panel: Remove a redundant check on existence of bridge->encoder
Because the existence of 'bridge->encoder' has already been checked before
the panel_bridge_attach() function get called, and the drm_bridge_attach()
will quit with a negative error code returned if it fails for some reasons.
Hence, it is guaranteed that the .encoder member of the drm_bridge instance
is not NULL when panel_bridge_attach() function get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/panel.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index 32506524d9a2..56c40b516a8f 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -67,11 +67,6 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
return 0;
- if (!bridge->encoder) {
- DRM_ERROR("Missing encoder\n");
- return -ENODEV;
- }
-
drm_connector_helper_add(connector,
_bridge_connector_helper_funcs);
--
2.43.0
[PATCH v2 03/12] drm/bridge: nxp-ptn3460: Remove a redundant check on existence of bridge->encoder
Because the existence of 'bridge->encoder' has already been checked before
the ptn3460_bridge_attach() function get called, and drm_bridge_attach()
will quit with a negative error code returned if it fails for some reasons.
Hence, it is guaranteed that the .encoder member of the drm_bridge instance
is not NULL when the ptn3460_bridge_attach() function get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/nxp-ptn3460.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c
b/drivers/gpu/drm/bridge/nxp-ptn3460.c
index ed93fd4c3265..e77aab965fcf 100644
--- a/drivers/gpu/drm/bridge/nxp-ptn3460.c
+++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c
@@ -229,11 +229,6 @@ static int ptn3460_bridge_attach(struct drm_bridge *bridge,
if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
return 0;
- if (!bridge->encoder) {
- DRM_ERROR("Parent encoder object not found");
- return -ENODEV;
- }
-
ptn_bridge->connector.polled = DRM_CONNECTOR_POLL_HPD;
ret = drm_connector_init(bridge->dev, _bridge->connector,
_connector_funcs, DRM_MODE_CONNECTOR_LVDS);
--
2.43.0
[PATCH v2 01/12] drm/bridge: simple-bridge: Remove a redundant check on existence of bridge->encoder
Because the existence of 'bridge->encoder' has already been checked before
the simple_bridge_attach() function get called, and drm_bridge_attach()
will quit with a negative error code returned if it fails for some reasons.
Hence, it is guaranteed that the .encoder member of the drm_bridge instance
is not NULL when the simple_bridge_attach() get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/simple-bridge.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/simple-bridge.c
b/drivers/gpu/drm/bridge/simple-bridge.c
index 5813a2c4fc5e..2ca89f313cd1 100644
--- a/drivers/gpu/drm/bridge/simple-bridge.c
+++ b/drivers/gpu/drm/bridge/simple-bridge.c
@@ -116,11 +116,6 @@ static int simple_bridge_attach(struct drm_bridge *bridge,
if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
return 0;
- if (!bridge->encoder) {
- DRM_ERROR("Missing encoder\n");
- return -ENODEV;
- }
-
drm_connector_helper_add(>connector,
_bridge_con_helper_funcs);
ret = drm_connector_init_with_ddc(bridge->dev, >connector,
--
2.43.0
[PATCH v2 02/12] drm/bridge: tfp410: Remove a redundant check on existence of bridge->encoder
Because the existence of bridge->encoder has already been checked before
the simple_bridge_attach() function get called, And drm_bridge_attach()
will quit with a negative error code returned if it fails for some reasons.
Hence, it is guaranteed that the .encoder member of the drm_bridge instance
is not NULL when the tfp410_attach() function get called.
Remove the redundant checking codes "if (!bridge->encoder) { ... }".
Reviewed-by: Laurent Pinchart
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/bridge/ti-tfp410.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c
b/drivers/gpu/drm/bridge/ti-tfp410.c
index c7bef5c23927..b1b1e4d5a24a 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -133,11 +133,6 @@ static int tfp410_attach(struct drm_bridge *bridge,
if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
return 0;
- if (!bridge->encoder) {
- dev_err(dvi->dev, "Missing encoder\n");
- return -ENODEV;
- }
-
if (dvi->next_bridge->ops & DRM_BRIDGE_OP_DETECT)
dvi->connector.polled = DRM_CONNECTOR_POLL_HPD;
else
--
2.43.0
[PATCH v2 00/12] Remove redundant checks on existence of 'bridge->encoder'
The checks on the existence of bridge->encoder in the implementation of drm_bridge_funcs::attach() is not necessary, as it has already been checked in the drm_bridge_attach() function call by previous bridge or KMS driver. The drm_bridge_attach() will quit with a negative error code returned if it fails for some reasons, hence, it is guaranteed that the .encoder member of the drm_bridge instance is not NULL when various bridge attach functions are called. V1 -> V2: * Gather all similar patches to form a series (Laurent) * Fix various spell error (Laurent) * Correct commit message for bridges of i.MX (Ying) Sui Jingfeng (12): drm/bridge: simple-bridge: Remove a redundant check on existence of bridge->encoder drm/bridge: tfp410: Remove a redundant check on existence of bridge->encoder drm/bridge: nxp-ptn3460: Remove a redundant check on existence of bridge->encoder drm/bridge: panel: Remove a redundant check on existence of bridge->encoder drm/bridge: it6505: Remove a redundant check on existence of bridge->encoder drm/bridge: adv7511: Remove a redundant check on existence of bridge->encoder drm/bridge: cdns-mhdp8546: Remove a redundant check on existence of bridge->encoder drm/bridge: megachips-stdp-ge-b850v3-fw: Remove a redundant check on existence of bridge->encoder drm/bridge: synopsys: dw-mipi-dsi: Remove a redundant check on existence of bridge->encoder drm/bridge: lt9611uxc: Remove a redundant check on existence of bridge->encoder drm/bridge: imx: Remove redundant checks on existence of bridge->encoder drm/bridge: analogix: Remove redundant checks on existence of bridge->encoder drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 5 - drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 5 - drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 5 - drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 - drivers/gpu/drm/bridge/analogix/anx7625.c | 10 -- drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c| 5 - drivers/gpu/drm/bridge/imx/imx-ldb-helper.c| 5 - drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c| 5 - drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c| 5 - drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c | 5 - drivers/gpu/drm/bridge/ite-it6505.c| 5 - drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 5 - .../gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 5 - drivers/gpu/drm/bridge/nxp-ptn3460.c | 5 - drivers/gpu/drm/bridge/panel.c | 5 - drivers/gpu/drm/bridge/simple-bridge.c | 5 - drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 5 - drivers/gpu/drm/bridge/ti-tfp410.c | 5 - 18 files changed, 95 deletions(-) -- 2.43.0
Re: [PATCH] drm/bridge: imx: Remove redundant checks on existence of bridge->encoder
Hi, On 5/13/24 13:56, Liu Ying wrote: On 5/11/24 23:08, Sui Jingfeng wrote: The check on the existence of bridge->encoder on the implementation layer of drm bridge driver is not necessary, as it has already been done in the drm_bridge_attach() function. It is guaranteed that the .encoder member of the drm_bridge instance is not NULL when various imx_xxx_bridge_attach() function gets called. Nit: ldb_bridge_attach_helper() doesn't follow the fashion of imx_xxx_bridge_attach(), not even the other bridge attach functions touched by this patch do. Right, my bad. But personally, I think rename it as ldb_bridge_attach() should enough to express the meaning. :-) Maybe, reword as "when various i.MX specific bridge attach functions are called." OK, fine. I will correct the commit message at the next version, so happy being reviewed. Thanks. Regards, Liu Ying
Re: [PATCH] drm/bridge: Remove a small useless code snippet
Hi,
On 2024/5/13 05:09, Laurent Pinchart wrote:
Hi Sui,
Thank you for the patch.
On Sat, May 11, 2024 at 08:42:38PM +0800, Sui Jingfeng wrote:
Because the check on the non-existence (encoder == NULL) has already been
done in the implementation of drm_bridge_attach() function, and
drm_bridge_attach() is called earlier. The driver won't get to check point
even if drm_bridge_attach() fails for some reasons, as it will clear the
bridge->encoder to NULL and return a negective error code.
s/negective/negative/
Sorry, my bad.
Therefore, there is no need to check another again. Remove the redundant
codes at the later.
Signed-off-by: Sui Jingfeng
Reviewed-by: Laurent Pinchart
If you end up sending a second version of this patch, please include all
similar patches you have sent at the same time in a patch series,
instead of sending them separately.
I will send a second version to correct spell error and collect and fold them
into a series.
By the way, thanks a lot for you time review my patch and thanks a lot for the
generous.
So happy be reviewed and its my pleasure to have opportunities to talk with
you. Thanks a lot!
---
drivers/gpu/drm/bridge/simple-bridge.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/simple-bridge.c
b/drivers/gpu/drm/bridge/simple-bridge.c
index 28376d0ecd09..3caa918ac2e0 100644
--- a/drivers/gpu/drm/bridge/simple-bridge.c
+++ b/drivers/gpu/drm/bridge/simple-bridge.c
@@ -116,11 +116,6 @@ static int simple_bridge_attach(struct drm_bridge *bridge,
if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
return 0;
- if (!bridge->encoder) {
- DRM_ERROR("Missing encoder\n");
- return -ENODEV;
- }
-
drm_connector_helper_add(>connector,
_bridge_con_helper_funcs);
ret = drm_connector_init_with_ddc(bridge->dev, >connector,
--
Best regards,
Sui
[PATCH 3/3] drm/loongson: Refactor lsdc device initialize and the output port
Move drm related device initialization into loongson_drm_master_bind(),
As we need to wait all other submodules ready before we could register
the drm device to userspace. Move output related things into subdriver,
fullfill the implement under the new framework.
Signed-off-by: Sui Jingfeng
---
drivers/gpu/drm/loongson/lsdc_drv.c | 187 +-
drivers/gpu/drm/loongson/lsdc_drv.h | 22 +--
drivers/gpu/drm/loongson/lsdc_output.c| 41
drivers/gpu/drm/loongson/lsdc_output.h| 16 +-
drivers/gpu/drm/loongson/lsdc_output_7a1000.c | 3 +-
drivers/gpu/drm/loongson/lsdc_output_7a2000.c | 15 +-
6 files changed, 152 insertions(+), 132 deletions(-)
diff --git a/drivers/gpu/drm/loongson/lsdc_drv.c
b/drivers/gpu/drm/loongson/lsdc_drv.c
index 45c30e3d178f..796da5c3c2ee 100644
--- a/drivers/gpu/drm/loongson/lsdc_drv.c
+++ b/drivers/gpu/drm/loongson/lsdc_drv.c
@@ -69,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 = >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 = >dispipe[i];
- if (dispipe->li2c)
- ddc = >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 = >dispipe[i];
@@ -189,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 = >base;
int ret;
- ldev = devm_drm_dev_alloc(>dev, driver, struct lsdc_device, base);
- if (IS_ERR(ldev))
- return ldev;
-
- ldev->dc = pdev;
- ldev->descp = descp;
-
- ddev = >base;
-
- loongson_gfxpll_create(ddev, >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,
@@ -220,36 +182,25 @@ 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(>reglock);
- ret = lsdc_mode_config_init(ddev, descp);
- if (ret)
- return ERR_PTR(ret);
-
- ret = lsdc_modeset_init(ldev, descp->num_of_crtc, descp->funcs,
- loongson_vblank);
- if (ret)
- return ERR_PTR(ret);
-
- drm_mode_config_reset(ddev);
-
- return ldev;
+ return 0;
}
/* For multiple GPU driver instance co-exixt in the system */
@@ -261,20 +212,64 @@ static unsigned int lsdc_vga_set_decode(struct pci_dev
*pdev, bool state)
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 = >base;
int ret;
- ret = component_bind_all(dev, NULL);
+ if (loongson_vblank) {
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ ret = drm_vblank_init(ddev, descp->n
[PATCH 2/3] drm/loongson: Introduce component framework support
Introduce the component framework to bind childs and siblings, for better
modularity and paper over the deferral probe problems if it need to attach
exterinal module someday. Hardware units come with PCI(e) are actually all
ready to drive, but there has some board specific modules will return
-EPROBE_DEFER. We need all other submodules ready to attach before we can
register the drm device to userspace.
The idea is to devide the exterinal module dependent part and exterinal
module independent part clearly, for example, the display controller and
the builtin GPIO-I2C just belong to exterinal module independent part.
While the output is belong to exterinal module dependent part.
Also for better reflecting the hardware, we intend to abstract the output
ports as child devices. The output ports may consists of encoder phy and
level shift, while the GPU and VPU are standalone siblings. As those units
are relative separate hardware units from display controller itself.
By design, the display controller PCI(e) is selected as the component
master, gpio-i2c go with master. The manually created virtual child device
are functional as agents for the master, it could return the -EPROBE_DEFER
back to the component core. This allows the master don't have to tear down
everything, the majority setups work can be preserved. The potential cyclic
dependency problem can be solved with such framework.
Signed-off-by: Sui Jingfeng
---
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| 59 +
drivers/gpu/drm/loongson/lsdc_drv.h| 6 +-
drivers/gpu/drm/loongson/lsdc_output.c | 142 +
drivers/gpu/drm/loongson/lsdc_output.h | 22 +++-
7 files changed, 241 insertions(+), 7 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
+#include
#include
@@ -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(_pci_driver);
+ ret = platform_driver_register(_output_port_platform_driver);
+ if (ret)
+ return ret;
+
+ ret = pci_register_driver(_pci_driver);
+ if (ret) {
+ platform_driver_unregister(_output_port_platform_driver);
+ return ret;
+ }
+
+ return 0;
}
module_init(loongson_module_init);
static void __exit loongson_module_exit(void)
{
pci_unregister_driver(_pci_driver);
+
+ platform_driver_unregister(_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..45c30e3d178f 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
#include
+#include
#include
#include
@@ -257,12 +259,39 @@ 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)
+{
+ int ret;
+
+ ret = component_bind_all(dev, NULL);
+ if (ret) {
+ dev_err(dev, "master bind all failed: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void loongson_drm_master_unbind(struct device *dev)
+{
+ component_unbind_all(dev, NULL);
+
+ return;
+}
+
+const struct component_master_ops loongson_drm_master_ops = {
+ .bind = loongson_drm_master_bind,
+ .unbind = loongson_drm_master_unbind,
+};
+
