[PATCH 00/10] drm/bridge: Make panel and bridge probe order consistent
Hi,
We've encountered an issue with the RaspberryPi DSI panel that prevented the
whole display driver from probing.
The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
Only register our component once a DSI device is attached"), but the basic idea
is that since the panel is probed through i2c, there's no synchronization
between its probe and the registration of the MIPI-DSI host it's attached to.
We initially moved the component framework registration to the MIPI-DSI Host
attach hook to make sure we register our component only when we have a DSI
device attached to our MIPI-DSI host, and then use lookup our DSI device in our
bind hook.
However, all the DSI bridges controlled through i2c are only registering their
associated DSI device in their bridge attach hook, meaning with our change
above, we never got that far, and therefore ended up in the same situation than
the one we were trying to fix for panels.
Since the RaspberryPi panel is the only driver in that situation, whereas it
seems like there's a consensus in bridge drivers, it makes more sense to try to
mimic the bridge pattern in the panel driver.
However, panels don't have an attach hook, and adding more panel hooks would
lead to more path to maintain in each and every driver, while the general push
is towards bridges. We also have to make sure that each and every DSI host and
device driver behaves the same in order to have expectations to rely on.
The solution I'm proposing is thus done in several steps:
- We get rid of the initial patch to make sure we support the bridge case,
and not the odd-panel one.
- Add a function that returns a bridge from a DT node, reducing the amount of
churn in each and every driver and making it a real incentive to not care
about panels in display drivers but only bridges.
- Add an attach and detach hook into the panel operations, and make it called
automatically by the DRM panel bridge.
- Convert the VC4 DSI host to this new bridge function, and the RaspberryPi
Panel to the new attach and detach hooks.
If the general approach is agreed upon, other drivers will obviously be
converted to drm_of_get_next.
Let me know what you think,
Maxime
Maxime Ripard (10):
Revert "drm/vc4: dsi: Only register our component once a DSI device is
attached"
drm/bridge: Add a function to abstract away panels
drm/bridge: Add documentation sections
drm/bridge: Document the probe issue with MIPI-DSI bridges
drm/panel: Create attach and detach callbacks
drm/bridge: panel: Call attach and detach for the panel
drm/vc4: dsi: Switch to drm_of_get_next
drm/panel: raspberrypi-touchscreen: Prevent double-free
drm/panel: raspberrypi-touchscreen: Use the attach hook
drm/panel: raspberrypi-touchscreen: Remove MIPI-DSI driver
Documentation/gpu/drm-kms-helpers.rst | 12 ++
drivers/gpu/drm/bridge/panel.c| 4 +
drivers/gpu/drm/drm_bridge.c | 134 ++-
drivers/gpu/drm/drm_of.c | 3 +
drivers/gpu/drm/drm_panel.c | 20 +++
.../drm/panel/panel-raspberrypi-touchscreen.c | 159 +-
drivers/gpu/drm/vc4/vc4_drv.c | 2 +
drivers/gpu/drm/vc4/vc4_dsi.c | 53 +++---
include/drm/drm_bridge.h | 2 +
include/drm/drm_panel.h | 6 +
10 files changed, 273 insertions(+), 122 deletions(-)
--
2.31.1
[PATCH 01/10] Revert "drm/vc4: dsi: Only register our component once a DSI device is attached"
This reverts commit 7213246a803f9b8da0677adb9ae06a3d8b806d02.
The commit 7213246a803f ("drm/vc4: dsi: Only register our component once
a DSI device is attached") aimed at preventing an endless probe loop
between the DSI host driver and its panel/bridge where both would wait
for each other to probe.
The solution implemented in that commit however relies on the fact that
MIPI-DSI device will either be a MIPI-DSI device, or would call
mipi_dsi_device_register_full() at probe time.
This assumption isn't true for bridges though where most drivers will do
so in the bridge attach hook. However, the drm_bridge_attach is usually
called in the DSI host bind hook, and thus we never get this far,
resulting in a DSI bridge that will never have its attach run, and the
DSI host that will never be bound, effectively creating the same
situation we were trying to avoid for panels.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dsi.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index a55256ed0955..6dfcbd9e234e 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -1257,12 +1257,10 @@ static ssize_t vc4_dsi_host_transfer(struct
mipi_dsi_host *host,
return ret;
}
-static const struct component_ops vc4_dsi_ops;
static int vc4_dsi_host_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device *device)
{
struct vc4_dsi *dsi = host_to_dsi(host);
- int ret;
dsi->lanes = device->lanes;
dsi->channel = device->channel;
@@ -1297,12 +1295,6 @@ static int vc4_dsi_host_attach(struct mipi_dsi_host
*host,
return 0;
}
- ret = component_add(&dsi->pdev->dev, &vc4_dsi_ops);
- if (ret) {
- mipi_dsi_host_unregister(&dsi->dsi_host);
- return ret;
- }
-
return 0;
}
@@ -1689,6 +1681,7 @@ static int vc4_dsi_dev_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct vc4_dsi *dsi;
+ int ret;
dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
if (!dsi)
@@ -1696,10 +1689,26 @@ static int vc4_dsi_dev_probe(struct platform_device
*pdev)
dev_set_drvdata(dev, dsi);
dsi->pdev = pdev;
+
+ /* Note, the initialization sequence for DSI and panels is
+* tricky. The component bind above won't get past its
+* -EPROBE_DEFER until the panel/bridge probes. The
+* panel/bridge will return -EPROBE_DEFER until it has a
+* mipi_dsi_host to register its device to. So, we register
+* the host during pdev probe time, so vc4 as a whole can then
+* -EPROBE_DEFER its component bind process until the panel
+* successfully attaches.
+*/
dsi->dsi_host.ops = &vc4_dsi_host_ops;
dsi->dsi_host.dev = dev;
mipi_dsi_host_register(&dsi->dsi_host);
+ ret = component_add(&pdev->dev, &vc4_dsi_ops);
+ if (ret) {
+ mipi_dsi_host_unregister(&dsi->dsi_host);
+ return ret;
+ }
+
return 0;
}
--
2.31.1
[PATCH 02/10] drm/bridge: Add a function to abstract away panels
Display drivers so far need to have a lot of boilerplate to first
retrieve either the panel or bridge that they are connected to using
drm_of_find_panel_or_bridge(), and then either deal with each with ad-hoc
functions or create a drm panel bridge through drm_panel_bridge_add.
In order to reduce the boilerplate and hopefully create a path of least
resistance towards using the DRM panel bridge layer, let's create the
function devm_drm_of_get_next to reduce that boilerplate.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_bridge.c | 62 +---
drivers/gpu/drm/drm_of.c | 3 ++
include/drm/drm_bridge.h | 2 ++
3 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 044acd07c153..aef8c9f4fb9f 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -24,10 +24,12 @@
#include
#include
#include
+#include
#include
#include
#include
+#include
#include "drm_crtc_internal.h"
@@ -50,10 +52,8 @@
*
* Display drivers are responsible for linking encoders with the first bridge
* in the chains. This is done by acquiring the appropriate bridge with
- * of_drm_find_bridge() or drm_of_find_panel_or_bridge(), or creating it for a
- * panel with drm_panel_bridge_add_typed() (or the managed version
- * devm_drm_panel_bridge_add_typed()). Once acquired, the bridge shall be
- * attached to the encoder with a call to drm_bridge_attach().
+ * drm_of_get_next(). Once acquired, the bridge shall be attached to the
+ * encoder with a call to drm_bridge_attach().
*
* Bridges are responsible for linking themselves with the next bridge in the
* chain, if any. This is done the same way as for encoders, with the call to
@@ -1223,6 +1223,60 @@ struct drm_bridge *of_drm_find_bridge(struct device_node
*np)
return NULL;
}
EXPORT_SYMBOL(of_drm_find_bridge);
+
+/**
+ * devm_drm_of_get_next - Return next bridge in the chain
+ * @dev: device to tie the bridge lifetime to
+ * @np: device tree node containing encoder output ports
+ * @port: port in the device tree node
+ * @endpoint: endpoint in the device tree node
+ *
+ * Given a DT node's port and endpoint number, finds the connected node
+ * and returns the associated bridge if any, or creates and returns a
+ * drm panel bridge instance if a panel is connected.
+ *
+ * Returns a pointer to the bridge if successful, or an error pointer
+ * otherwise.
+ */
+struct drm_bridge *devm_drm_of_get_next(struct device *dev,
+ struct device_node *np,
+ unsigned int port,
+ unsigned int endpoint)
+{
+ struct device_node *remote;
+ struct drm_bridge *bridge;
+ struct drm_panel *panel;
+
+ /*
+* of_graph_get_remote_node() produces a noisy error message if port
+* node isn't found and the absence of the port is a legit case here,
+* so at first we silently check whether graph presents in the
+* device-tree node.
+*/
+ if (!of_graph_is_present(np))
+ return ERR_PTR(-ENODEV);
+
+ remote = of_graph_get_remote_node(np, port, endpoint);
+ if (!remote)
+ return ERR_PTR(-ENODEV);
+
+ bridge = of_drm_find_bridge(remote);
+ if (bridge) {
+ of_node_put(remote);
+ return bridge;
+ }
+
+ panel = of_drm_find_panel(remote);
+ if (IS_ERR(panel)) {
+ of_node_put(remote);
+ return ERR_CAST(panel);
+ }
+
+ of_node_put(remote);
+
+ return devm_drm_panel_bridge_add(dev, panel);
+}
+EXPORT_SYMBOL(devm_drm_of_get_next);
#endif
MODULE_AUTHOR("Ajay Kumar ");
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 997b8827fed2..bbbdc4d17ac9 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -231,6 +231,9 @@ EXPORT_SYMBOL_GPL(drm_of_encoder_active_endpoint);
* return either the associated struct drm_panel or drm_bridge device. Either
* @panel or @bridge must not be NULL.
*
+ * This function is deprecated and should not be used in new drivers. Use
+ * drm_of_get_next() instead.
+ *
* Returns zero if successful, or one of the standard error codes if it fails.
*/
int drm_of_find_panel_or_bridge(const struct device_node *np,
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 46bdfa48c413..e16fafc6f37d 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -911,6 +911,8 @@ struct drm_bridge *devm_drm_panel_bridge_add(struct device
*dev,
struct drm_bridge *devm_drm_panel_bridge_add_typed(struct device *dev,
struct drm_panel *panel,
u32 connector_type);
+struct drm_bridge *devm_drm_of_get_next(struct dev
[PATCH 03/10] drm/bridge: Add documentation sections
The bridge documentation overview is quite packed already, and we'll add some more documentation that isn't part of an overview at all. Let's add some sections to the documentation to separare each bits. Signed-off-by: Maxime Ripard --- Documentation/gpu/drm-kms-helpers.rst | 6 ++ drivers/gpu/drm/drm_bridge.c | 14 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index 389892f36185..10f8df7aecc0 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -151,6 +151,12 @@ Overview .. kernel-doc:: drivers/gpu/drm/drm_bridge.c :doc: overview +Display Driver Integration +-- + +.. kernel-doc:: drivers/gpu/drm/drm_bridge.c + :doc: display driver integration + Bridge Operations - diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index aef8c9f4fb9f..c9a950bfdfe5 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -50,6 +50,15 @@ * Chaining multiple bridges to the output of a bridge, or the same bridge to * the output of different bridges, is not supported. * + * &drm_bridge, like &drm_panel, aren't &drm_mode_object entities like planes, + * CRTCs, encoders or connectors and hence are not visible to userspace. They + * just provide additional hooks to get the desired output at the end of the + * encoder chain. + */ + +/** + * DOC:display driver integration + * * Display drivers are responsible for linking encoders with the first bridge * in the chains. This is done by acquiring the appropriate bridge with * drm_of_get_next(). Once acquired, the bridge shall be attached to the @@ -84,11 +93,6 @@ * helper to create the &drm_connector, or implement it manually on top of the * connector-related operations exposed by the bridge (see the overview * documentation of bridge operations for more details). - * - * &drm_bridge, like &drm_panel, aren't &drm_mode_object entities like planes, - * CRTCs, encoders or connectors and hence are not visible to userspace. They - * just provide additional hooks to get the desired output at the end of the - * encoder chain. */ static DEFINE_MUTEX(bridge_lock); -- 2.31.1
[PATCH 04/10] drm/bridge: Document the probe issue with MIPI-DSI bridges
Interactions between bridges, panels, MIPI-DSI host and the component framework are not trivial and can lead to probing issues when implementing a display driver. Let's document the various cases we need too consider, and the solution to support all the cases. Signed-off-by: Maxime Ripard --- Documentation/gpu/drm-kms-helpers.rst | 6 +++ drivers/gpu/drm/drm_bridge.c | 60 +++ 2 files changed, 66 insertions(+) diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index 10f8df7aecc0..ec2f65b31930 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -157,6 +157,12 @@ Display Driver Integration .. kernel-doc:: drivers/gpu/drm/drm_bridge.c :doc: display driver integration +Special Care with MIPI-DSI bridges +-- + +.. kernel-doc:: drivers/gpu/drm/drm_bridge.c + :doc: special care dsi + Bridge Operations - diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index c9a950bfdfe5..81f8dac12367 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -95,6 +95,66 @@ * documentation of bridge operations for more details). */ +/** + * DOC: special care dsi + * + * The interaction between the bridges and other frameworks involved in + * the probing of the display driver and the bridge driver can be + * challenging. Indeed, there's multiple cases that needs to be + * considered: + * + * - The display driver doesn't use the component framework and isn't a + * MIPI-DSI host. In this case, the bridge driver will probe at some + * point and the display driver should try to probe again by returning + * EPROBE_DEFER as long as the bridge driver hasn't probed. + * + * - The display driver doesn't use the component framework, but is a + * MIPI-DSI host. The bridge device uses the MIPI-DCS commands to be + * controlled. In this case, the bridge device is a child of the + * display device and when it will probe it's assured that the display + * device (and MIPI-DSI host) is present. The display driver will be + * assured that the bridge driver is connected between the + * &mipi_dsi_host_ops.attach and &mipi_dsi_host_ops.detach operations. + * Therefore, it must run mipi_dsi_host_register() in its probe + * function, and then run drm_bridge_attach() in its + * &mipi_dsi_host_ops.attach hook. + * + * - The display driver uses the component framework and is a MIPI-DSI + * host. The bridge device uses the MIPI-DCS commands to be + * controlled. This is the same situation than above, and can run + * mipi_dsi_host_register() in either its probe or bind hooks. + * + * - The display driver uses the component framework and is a MIPI-DSI + * host. The bridge device uses a separate bus (such as I2C) to be + * controlled. In this case, there's no correlation between the probe + * of the bridge and display drivers, so care must be taken to avoid + * an endless EPROBE_DEFER loop, with each driver waiting for the + * other to probe. + * + * The ideal pattern to cover the last item (and all the others in the + * display driver case) is to split the operations like this: + * + * - In the display driver must run mipi_dsi_host_register() and + * component_add in its probe hook. It will make sure that the + * MIPI-DSI host sticks around, and that the driver's bind can be + * called. + * + * - In its probe hook, the bridge driver must not try to find its + * MIPI-DSI host or register as a MIPI-DSI device. As far as the + * framework is concerned, it must only call drm_bridge_add(). + * + * - In its bind hook, the display driver must try to find the bridge + * and return -EPROBE_DEFER if it doesn't find it. If it's there, it + * must call drm_bridge_attach(). The MIPI-DSI host is now functional. + * + * - In its &drm_bridge_funcs.attach hook, the bridge driver can now try + * to find its MIPI-DSI host and can register as a MIPI-DSI device. + * + * At this point, we're now certain that both the display driver and the + * bridge driver are functional and we can't have a deadlock-like + * situation when probing. + */ + static DEFINE_MUTEX(bridge_lock); static LIST_HEAD(bridge_list); -- 2.31.1
[PATCH 05/10] drm/panel: Create attach and detach callbacks
In order to make the probe order expectation more consistent between
bridges, let's create attach and detach hooks for the panels as well to
match what is there for bridges.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_panel.c | 20
include/drm/drm_panel.h | 6 ++
2 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index f634371c717a..23bca798a2f3 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -223,6 +223,26 @@ int drm_panel_get_modes(struct drm_panel *panel,
}
EXPORT_SYMBOL(drm_panel_get_modes);
+int drm_panel_attach(struct drm_panel *panel)
+{
+ if (!panel)
+ return -EINVAL;
+
+ if (panel->funcs && panel->funcs->attach)
+ return panel->funcs->attach(panel);
+
+ return -EOPNOTSUPP;
+}
+
+void drm_panel_detach(struct drm_panel *panel)
+{
+ if (!panel)
+ return;
+
+ if (panel->funcs && panel->funcs->detach)
+ panel->funcs->detach(panel);
+}
+
#ifdef CONFIG_OF
/**
* of_drm_find_panel - look up a panel using a device tree node
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 4602f833eb51..b9201d520754 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -68,6 +68,9 @@ enum drm_panel_orientation;
* does not need to implement the functionality to enable/disable backlight.
*/
struct drm_panel_funcs {
+ int (*attach)(struct drm_panel *panel);
+ void (*detach)(struct drm_panel *panel);
+
/**
* @prepare:
*
@@ -180,6 +183,9 @@ void drm_panel_init(struct drm_panel *panel, struct device
*dev,
void drm_panel_add(struct drm_panel *panel);
void drm_panel_remove(struct drm_panel *panel);
+int drm_panel_attach(struct drm_panel *panel);
+void drm_panel_detach(struct drm_panel *panel);
+
int drm_panel_prepare(struct drm_panel *panel);
int drm_panel_unprepare(struct drm_panel *panel);
--
2.31.1
[PATCH 06/10] drm/bridge: panel: Call attach and detach for the panel
Now that we have additional attach and detach hooks for panels, make sure that the panel bridge driver calls them when relevant. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/bridge/panel.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index c916f4b8907e..c2249f3fd357 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -60,6 +60,8 @@ static int panel_bridge_attach(struct drm_bridge *bridge, struct drm_connector *connector = &panel_bridge->connector; int ret; + drm_panel_attach(panel_bridge->panel); + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) return 0; @@ -90,6 +92,8 @@ static void panel_bridge_detach(struct drm_bridge *bridge) struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge); struct drm_connector *connector = &panel_bridge->connector; + drm_panel_detach(panel_bridge->panel); + /* * Cleanup the connector if we know it was initialized. * -- 2.31.1
[PATCH 07/10] drm/vc4: dsi: Switch to drm_of_get_next
The new drm_of_get_next removes most of the boilerplate we have to deal
with. Let's switch to it.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_drv.c | 2 ++
drivers/gpu/drm/vc4/vc4_dsi.c | 28
2 files changed, 6 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 73335feb712f..ff056ee8bc4b 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -25,7 +25,9 @@
#include
#include
#include
+#include
#include
+#include
#include
#include
#include
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 6dfcbd9e234e..f51ce8db0f4e 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -1489,7 +1489,6 @@ static int vc4_dsi_bind(struct device *dev, struct device
*master, void *data)
struct drm_device *drm = dev_get_drvdata(master);
struct vc4_dsi *dsi = dev_get_drvdata(dev);
struct vc4_dsi_encoder *vc4_dsi_encoder;
- struct drm_panel *panel;
const struct of_device_id *match;
dma_cap_mask_t dma_mask;
int ret;
@@ -1601,27 +1600,9 @@ static int vc4_dsi_bind(struct device *dev, struct
device *master, void *data)
return ret;
}
- ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
- &panel, &dsi->bridge);
- if (ret) {
- /* If the bridge or panel pointed by dev->of_node is not
-* enabled, just return 0 here so that we don't prevent the DRM
-* dev from being registered. Of course that means the DSI
-* encoder won't be exposed, but that's not a problem since
-* nothing is connected to it.
-*/
- if (ret == -ENODEV)
- return 0;
-
- return ret;
- }
-
- if (panel) {
- dsi->bridge = devm_drm_panel_bridge_add_typed(dev, panel,
-
DRM_MODE_CONNECTOR_DSI);
- if (IS_ERR(dsi->bridge))
- return PTR_ERR(dsi->bridge);
- }
+ dsi->bridge = devm_drm_of_get_next(dev, dev->of_node, 0, 0);
+ if (IS_ERR(dsi->bridge))
+ return PTR_ERR(dsi->bridge);
/* The esc clock rate is supposed to always be 100Mhz. */
ret = clk_set_rate(dsi->escape_clock, 100 * 100);
@@ -1661,8 +1642,7 @@ static void vc4_dsi_unbind(struct device *dev, struct
device *master,
{
struct vc4_dsi *dsi = dev_get_drvdata(dev);
- if (dsi->bridge)
- pm_runtime_disable(dev);
+ pm_runtime_disable(dev);
/*
* Restore the bridge_chain so the bridge detach procedure can happen
--
2.31.1
[PATCH 08/10] drm/panel: raspberrypi-touchscreen: Prevent double-free
The mipi_dsi_device allocated by mipi_dsi_device_register_full() is
already free'd on release.
Fixes: 2f733d6194bd ("drm/panel: Add support for the Raspberry Pi 7"
Touchscreen.")
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
index 6f2ce3b81f47..462faae0f446 100644
--- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
+++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
@@ -447,7 +447,6 @@ static int rpi_touchscreen_remove(struct i2c_client *i2c)
drm_panel_remove(&ts->base);
mipi_dsi_device_unregister(ts->dsi);
- kfree(ts->dsi);
return 0;
}
--
2.31.1
[PATCH 09/10] drm/panel: raspberrypi-touchscreen: Use the attach hook
Now that we have an attach hook available for panels as well, let's use
it for the RaspberryPi 7" DSI panel.
This now mimics what all the other bridges in a similar situation are
doing, and we avoid our probe order issue entirely.
Signed-off-by: Maxime Ripard
---
.../drm/panel/panel-raspberrypi-touchscreen.c | 135 ++
1 file changed, 77 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
index 462faae0f446..995c5cafb970 100644
--- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
+++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
@@ -346,7 +346,83 @@ static int rpi_touchscreen_get_modes(struct drm_panel
*panel,
return num;
}
+static int rpi_touchscreen_attach(struct drm_panel *panel)
+{
+ struct rpi_touchscreen *ts = panel_to_ts(panel);
+ struct device *dev = &ts->i2c->dev;
+ struct device_node *endpoint, *dsi_host_node;
+ struct mipi_dsi_device *dsi;
+ struct mipi_dsi_host *host;
+ int ret;
+
+ struct mipi_dsi_device_info info = {
+ .type = RPI_DSI_DRIVER_NAME,
+ .channel = 0,
+ .node = NULL,
+ };
+
+ /* Look up the DSI host. It needs to probe before we do. */
+ endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
+ if (!endpoint)
+ return -ENODEV;
+
+ dsi_host_node = of_graph_get_remote_port_parent(endpoint);
+ if (!dsi_host_node) {
+ of_node_put(endpoint);
+ return -ENODEV;
+ }
+
+ host = of_find_mipi_dsi_host_by_node(dsi_host_node);
+ of_node_put(dsi_host_node);
+ if (!host) {
+ of_node_put(endpoint);
+ return -EPROBE_DEFER;
+ }
+
+ info.node = of_graph_get_remote_port(endpoint);
+ if (!info.node) {
+ of_node_put(endpoint);
+ return -ENODEV;
+ }
+
+ of_node_put(endpoint);
+
+ dsi = mipi_dsi_device_register_full(host, &info);
+ if (IS_ERR(dsi)) {
+ dev_err(dev, "DSI device registration failed: %ld\n",
+ PTR_ERR(dsi));
+ return PTR_ERR(dsi);
+ }
+
+ ts->dsi = dsi;
+
+ dsi->mode_flags = (MIPI_DSI_MODE_VIDEO |
+ MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+ MIPI_DSI_MODE_LPM);
+ dsi->format = MIPI_DSI_FMT_RGB888;
+ dsi->lanes = 1;
+
+ ret = mipi_dsi_attach(dsi);
+ if (ret) {
+ dev_err(&dsi->dev, "failed to attach dsi to host: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void rpi_touchscreen_detach(struct drm_panel *panel)
+{
+ struct rpi_touchscreen *ts = panel_to_ts(panel);
+
+ mipi_dsi_detach(ts->dsi);
+ mipi_dsi_device_unregister(ts->dsi);
+}
+
static const struct drm_panel_funcs rpi_touchscreen_funcs = {
+ .attach = rpi_touchscreen_attach,
+ .detach = rpi_touchscreen_detach,
+
.disable = rpi_touchscreen_disable,
.unprepare = rpi_touchscreen_noop,
.prepare = rpi_touchscreen_noop,
@@ -359,14 +435,7 @@ static int rpi_touchscreen_probe(struct i2c_client *i2c,
{
struct device *dev = &i2c->dev;
struct rpi_touchscreen *ts;
- struct device_node *endpoint, *dsi_host_node;
- struct mipi_dsi_host *host;
int ver;
- struct mipi_dsi_device_info info = {
- .type = RPI_DSI_DRIVER_NAME,
- .channel = 0,
- .node = NULL,
- };
ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
if (!ts)
@@ -394,35 +463,6 @@ static int rpi_touchscreen_probe(struct i2c_client *i2c,
/* /\* Turn off at boot, so we can cleanly sequence powering on. *\/ */
/* rpi_touchscreen_i2c_write(ts, REG_POWERON, 0); */
- /* Look up the DSI host. It needs to probe before we do. */
- endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
- if (!endpoint)
- return -ENODEV;
-
- dsi_host_node = of_graph_get_remote_port_parent(endpoint);
- if (!dsi_host_node)
- goto error;
-
- host = of_find_mipi_dsi_host_by_node(dsi_host_node);
- of_node_put(dsi_host_node);
- if (!host) {
- of_node_put(endpoint);
- return -EPROBE_DEFER;
- }
-
- info.node = of_graph_get_remote_port(endpoint);
- if (!info.node)
- goto error;
-
- of_node_put(endpoint);
-
- ts->dsi = mipi_dsi_device_register_full(host, &info);
- if (IS_ERR(ts->dsi)) {
- dev_err(dev, "DSI device registration failed: %ld\n",
- PTR_ERR(ts->dsi));
- return PTR_ERR(ts->dsi);
- }
-
drm_panel_init(
[PATCH 10/10] drm/panel: raspberrypi-touchscreen: Remove MIPI-DSI driver
The driver was using a two-steps initialisation when probing with the
i2c probe first registering the MIPI-DSI device, and then when that
device was probed the driver would attach the device to its host. This
resulted in a fairly non-standard probe logic.
The previous commit changed that logic entirely though, resulting in a
completely empty MIPI-DSI device probe. Let's simplify the driver by
removing it entirely and just behave as a normal i2c driver.
Signed-off-by: Maxime Ripard
---
.../drm/panel/panel-raspberrypi-touchscreen.c | 25 +--
1 file changed, 1 insertion(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
index 995c5cafb970..09937aa26c6a 100644
--- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
+++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
@@ -483,16 +483,6 @@ static int rpi_touchscreen_remove(struct i2c_client *i2c)
return 0;
}
-static int rpi_touchscreen_dsi_probe(struct mipi_dsi_device *dsi)
-{
- return 0;
-}
-
-static struct mipi_dsi_driver rpi_touchscreen_dsi_driver = {
- .driver.name = RPI_DSI_DRIVER_NAME,
- .probe = rpi_touchscreen_dsi_probe,
-};
-
static const struct of_device_id rpi_touchscreen_of_ids[] = {
{ .compatible = "raspberrypi,7inch-touchscreen-panel" },
{ } /* sentinel */
@@ -507,20 +497,7 @@ static struct i2c_driver rpi_touchscreen_driver = {
.probe = rpi_touchscreen_probe,
.remove = rpi_touchscreen_remove,
};
-
-static int __init rpi_touchscreen_init(void)
-{
- mipi_dsi_driver_register(&rpi_touchscreen_dsi_driver);
- return i2c_add_driver(&rpi_touchscreen_driver);
-}
-module_init(rpi_touchscreen_init);
-
-static void __exit rpi_touchscreen_exit(void)
-{
- i2c_del_driver(&rpi_touchscreen_driver);
- mipi_dsi_driver_unregister(&rpi_touchscreen_dsi_driver);
-}
-module_exit(rpi_touchscreen_exit);
+module_i2c_driver(rpi_touchscreen_driver);
MODULE_AUTHOR("Eric Anholt ");
MODULE_DESCRIPTION("Raspberry Pi 7-inch touchscreen driver");
--
2.31.1
[PATCH v6] Documentation: gpu: Mention the requirements for new properties
New KMS properties come with a bunch of requirements to avoid each driver from running their own, inconsistent, set of properties, eventually leading to issues like property conflicts, inconsistencies between drivers and semantics, etc. Let's document what we expect. Cc: Alexandre Belloni Cc: Alexandre Torgue Cc: Alex Deucher Cc: Alison Wang Cc: Alyssa Rosenzweig Cc: Andrew Jeffery Cc: Andrzej Hajda Cc: Anitha Chrisanthus Cc: Benjamin Gaignard Cc: Ben Skeggs Cc: Boris Brezillon Cc: Brian Starkey Cc: Chen Feng Cc: Chen-Yu Tsai Cc: Christian Gmeiner Cc: "Christian König" Cc: Chun-Kuang Hu Cc: Edmund Dea Cc: Eric Anholt Cc: Fabio Estevam Cc: Gerd Hoffmann Cc: Haneen Mohammed Cc: Hans de Goede Cc: "Heiko Stübner" Cc: Huang Rui Cc: Hyun Kwon Cc: Inki Dae Cc: Jani Nikula Cc: Jernej Skrabec Cc: Jerome Brunet Cc: John Stultz Cc: Jonas Karlman Cc: Jonathan Hunter Cc: Joonas Lahtinen Cc: Joonyoung Shim Cc: Jyri Sarha Cc: Kevin Hilman Cc: Kieran Bingham Cc: Krzysztof Kozlowski Cc: Kyungmin Park Cc: Laurent Pinchart Cc: Linus Walleij Cc: Liviu Dudau Cc: Lucas Stach Cc: Ludovic Desroches Cc: Marek Vasut Cc: Martin Blumenstingl Cc: Matthias Brugger Cc: Maxime Coquelin Cc: Maxime Ripard Cc: Melissa Wen Cc: Neil Armstrong Cc: Nicolas Ferre Cc: "Noralf Trønnes" Cc: NXP Linux Team Cc: Oleksandr Andrushchenko Cc: Patrik Jakobsson Cc: Paul Cercueil Cc: Pekka Paalanen Cc: Pengutronix Kernel Team Cc: Philippe Cornu Cc: Philipp Zabel Cc: Qiang Yu Cc: Rob Clark Cc: Robert Foss Cc: Rob Herring Cc: Rodrigo Siqueira Cc: Rodrigo Vivi Cc: Roland Scheidegger Cc: Russell King Cc: Sam Ravnborg Cc: Sandy Huang Cc: Sascha Hauer Cc: Sean Paul Cc: Seung-Woo Kim Cc: Shawn Guo Cc: Simon Ser Cc: Stefan Agner Cc: Steven Price Cc: Sumit Semwal Cc: Thierry Reding Cc: Tian Tao Cc: Tomeu Vizoso Cc: Tomi Valkeinen Cc: VMware Graphics Cc: Xinliang Liu Cc: Xinwei Kong Cc: Yannick Fertre Cc: Zack Rusin Reviewed-by: Pekka Paalanen Reviewed-by: Daniel Vetter Signed-off-by: Maxime Ripard --- Changes from v5: - Typos fixes suggested by Pekka and Daniel - Added Pekka Reviewed-by Changes from v4: - Changes suggested by Pekka Changes from v3: - Roll back to the v2 - Add Simon and Pekka in Cc Changes from v2: - Take into account the feedback from Laurent and Lidiu to no longer force generic properties, but prefix vendor-specific properties with the vendor name Changes from v1: - Typos and wording reported by Daniel and Alex --- Documentation/gpu/drm-kms.rst | 29 + 1 file changed, 29 insertions(+) diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 87e5023e3f55..12e25119e563 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -463,6 +463,35 @@ KMS Properties This section of the documentation is primarily aimed at user-space developers. For the driver APIs, see the other sections. +Requirements + + +KMS drivers might need to add extra properties to support new features. Each +new property introduced in a driver needs to meet a few requirements, in +addition to the one mentioned above: + +* It must be standardized, documenting: + + * The full, exact, name string; + * If the property is an enum, all the valid value name strings; + * What values are accepted, and what these values mean; + * What the property does and how it can be used; + * How the property might interact with other, existing properties. + +* It must provide a generic helper in the core code to register that + property on the object it attaches to. + +* Its content must be decoded by the core and provided in the object's + associated state structure. That includes anything drivers might want + to precompute, like struct drm_clip_rect for planes. + +* Its initial state must match the behavior prior to the property + introduction. This might be a fixed value matching what the hardware + does, or it may be inherited from the state the firmware left the + system in during boot. + +* An IGT test must be submitted where reasonable. + Property Types and Blob Property Support -- 2.31.1
[PATCH 10/54] dt-bindings: display: panel-lvds: Document panel compatibles
The binding mentions that all the drivers using that driver must use a vendor-specific compatible but never enforces it, nor documents the vendor-specific compatibles. Let's make we document all of them, and that the binding will create an error if we add one that isn't. Cc: dri-devel@lists.freedesktop.org Cc: Laurent Pinchart Cc: Sam Ravnborg Cc: Thierry Reding Signed-off-by: Maxime Ripard --- .../bindings/display/panel/lvds.yaml | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml b/Documentation/devicetree/bindings/display/panel/lvds.yaml index 49460c9dceea..d1513111eb48 100644 --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml +++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml @@ -31,12 +31,18 @@ allOf: properties: compatible: -contains: - const: panel-lvds -description: - Shall contain "panel-lvds" in addition to a mandatory panel-specific - compatible string defined in individual panel bindings. The "panel-lvds" - value shall never be used on its own. +items: + - enum: + - advantech,idk-1110wr + - advantech,idk-2121wr + - auo,b101ew05 + - innolux,ee101ia-01d + - mitsubishi,aa104xd12 + - mitsubishi,aa121td01 + - sgd,gktw70sdae4se + - sharp,lq150x1lg11 + - tbs,a711-panel + - const: panel-lvds data-mapping: enum: -- 2.31.1
[PATCH 11/54] dt-bindings: display: simple-bridge: Add corpro, gm7123 compatible
The corpro,gm7123 was in use in a DT but was never properly documented, let's add it. Cc: dri-devel@lists.freedesktop.org Reviewed-by: Laurent Pinchart Signed-off-by: Maxime Ripard --- Changes from v1: - Removed the dumb-vga-dac compatible from the list --- .../devicetree/bindings/display/bridge/simple-bridge.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml index 6c7b577fd471..43cf4df9811a 100644 --- a/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml +++ b/Documentation/devicetree/bindings/display/bridge/simple-bridge.yaml @@ -22,6 +22,9 @@ properties: - ti,ths8134a - ti,ths8134b - const: ti,ths8134 + - items: + - const: corpro,gm7123 + - const: adi,adv7123 - enum: - adi,adv7123 - dumb-vga-dac -- 2.31.1
Re: [PATCH v1 1/7] drm/bridge: ps8640: Use atomic variants of drm_bridge_funcs
On Thu, Jul 22, 2021 at 08:22:40AM +0200, Sam Ravnborg wrote: > The atomic variants of enable/disable in drm_bridge_funcs are the > preferred operations - introduce these. > > The ps8640 driver used the non-atomic variants of the drm_bridge chain > functions - convert these to the atomic variants. > > Signed-off-by: Sam Ravnborg > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Robert Foss > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec Reviewed-by: Maxime Ripard Maxime
Re: [PATCH v1 2/7] drm/bridge: Drop unused drm_bridge_chain functions
On Thu, Jul 22, 2021 at 08:22:41AM +0200, Sam Ravnborg wrote:
> The drm_bridge_chain_{pre_enable,enable,disable,post_disable} has no
> users left and we have atomic variants that should be used.
> Drop them so they do not gain new users.
>
> Adjust a few comments to avoid references to the dropped functions.
>
> Signed-off-by: Sam Ravnborg
> Cc: Laurent Pinchart
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Daniel Vetter
Reviewed-by: Maxime Ripard
Maxime
Re: [PATCH v1 3/7] drm/bridge: Add drm_bridge_new_crtc_state() helper
Hi,
On Thu, Jul 22, 2021 at 08:22:42AM +0200, Sam Ravnborg wrote:
> drm_bridge_new_crtc_state() will be used by bridge drivers to provide
> easy access to the mode from the drm_bridge_funcs operations.
>
> The helper will be useful in the atomic operations of
> struct drm_bridge_funcs.
>
> Signed-off-by: Sam Ravnborg
> Suggested-by: Laurent Pinchart
> Cc: Laurent Pinchart
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/drm_atomic.c | 34 ++
> include/drm/drm_atomic.h | 3 +++
> 2 files changed, 37 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index a8bbb021684b..93d513078e9a 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1133,6 +1133,40 @@ drm_atomic_get_new_bridge_state(struct
> drm_atomic_state *state,
> }
> EXPORT_SYMBOL(drm_atomic_get_new_bridge_state);
>
> +/**
> + * drm_bridge_new_crtc_state - get crtc_state for the bridge
> + * @bridge: bridge object
> + * @old_bridge_state: state of the bridge
> + *
> + * This function returns the &struct drm_crtc_state for the given
> bridge/state,
> + * or NULL if no crtc_state could be looked up. In case no crtc_state then
> this is
> + * logged using WARN as the crtc_state is always expected to be present.
> + * This function is often used in the &struct drm_bridge_funcs operations.
> + */
> +const struct drm_crtc_state *
> +drm_bridge_new_crtc_state(struct drm_bridge *bridge,
Shouldn't we call this drm_atomic_bridge_get_new_crtc_state for consistency?
> + struct drm_bridge_state *old_bridge_state)
It seems odd to me to name it old_bridge_state, I guess it would make
more sense to pass in the new bridge state?
> +{
> + struct drm_atomic_state *state = old_bridge_state->base.state;
> + const struct drm_connector_state *conn_state;
> + const struct drm_crtc_state *crtc_state;
> + struct drm_connector *connector;
> +
> + connector = drm_atomic_get_new_connector_for_encoder(state,
> bridge->encoder);
> + if (WARN_ON(!connector))
> + return NULL;
> +
> + conn_state = drm_atomic_get_new_connector_state(state, connector);
> + if (WARN_ON(!conn_state || !conn_state->crtc))
> + return NULL;
> +
> + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
> + if (WARN_ON(!crtc_state))
> + return NULL;
> +
> + return crtc_state;
You don't even seem to use the bridge state itself, so maybe we just
need to pass the drm_atomic_state? And thus we end up with something
like drm_atomic_get_new_connector_for_encoder, so maybe we should just
call it drm_atomic_get_new_crtc_for_bridge?
Also, can we end up with a commit that affects the bridge state but not
the crtc state (like a connector property change)? In such a case
drm_atomic_get_new_crtc_state would return NULL.
I'm not sure if it's a big deal or not, but we should make it clear in
the documentation and remove the WARN_ON.
Maxime
Re: [PATCH v1 5/7] drm/mediatek: Drop chain_mode_fixup call in mode_valid()
On Thu, Jul 22, 2021 at 08:22:44AM +0200, Sam Ravnborg wrote: > The mode_valid implementation had a call to > drm_bridge_chain_mode_fixup() which would be wrong as the mode_valid is > not allowed to change anything - only to validate the mode. > > As the next bridge is often/always a connector the call had no effect > anyway. So drop it. > > From the git history I could see this call was included in the original > version of the driver so there was no help there to find out why it was > added in the first place. But a lot has changed since the initial driver > were added and is seems safe to remove the call now. > > Signed-off-by: Sam Ravnborg > Cc: Chun-Kuang Hu > Cc: Philipp Zabel > Cc: Matthias Brugger > Cc: Dafna Hirschfeld > Cc: linux-media...@lists.infradead.org > Cc: linux-arm-ker...@lists.infradead.org Reviewed-by: Maxime Ripard Maxime
Re: [PATCH v1 6/7] drm/bridge: Drop drm_bridge_chain_mode_fixup
On Thu, Jul 22, 2021 at 08:22:45AM +0200, Sam Ravnborg wrote: > There are no users left and we do not want to have this function > available. > > drm_atomic_bridge_check() is used to call the mode_fixup() operation for > the chained bridges and there is no need for drm_atomic_bridge_check(). > Drop it. > > Signed-off-by: Sam Ravnborg > Cc: Laurent Pinchart > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter Reviewed-by: Maxime Ripard Maxime
Re: [PATCH v1 7/7] drm/todo: Add bridge related todo items
On Thu, Jul 22, 2021 at 08:22:46AM +0200, Sam Ravnborg wrote: > - deprecated callbacks in drm_bridge_funcs > - move connector creation to display drivers > > Signed-off-by: Sam Ravnborg > Cc: Laurent Pinchart > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter Acked-by: Maxime Ripard Maxime
Re: [PATCH v1 4/7] drm/bridge: lontium-lt9611: Use atomic variants of drm_bridge_funcs
Hi,
On Thu, Jul 22, 2021 at 08:22:43AM +0200, Sam Ravnborg wrote:
> The atomic variants of enable/disable in drm_bridge_funcs are the
> preferred operations - introduce these.
>
> Use of mode_set is deprecated - merge the functionality with
> atomic_enable()
>
> Signed-off-by: Sam Ravnborg
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> ---
> drivers/gpu/drm/bridge/lontium-lt9611.c | 69 ++---
> 1 file changed, 27 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c
> b/drivers/gpu/drm/bridge/lontium-lt9611.c
> index 29b1ce2140ab..dfa7baefe2ab 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
> @@ -700,9 +700,17 @@ lt9611_connector_mode_valid(struct drm_connector
> *connector,
> }
>
> /* bridge funcs */
> -static void lt9611_bridge_enable(struct drm_bridge *bridge)
> +static void lt9611_bridge_atomic_enable(struct drm_bridge *bridge,
> + struct drm_bridge_state
> *old_bridge_state)
> {
> struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
> + const struct drm_display_mode *mode;
> + const struct drm_crtc_state *crtc_state;
> + struct hdmi_avi_infoframe avi_frame;
> + int ret;
> +
> + crtc_state = drm_bridge_new_crtc_state(bridge, old_bridge_state);
> + mode = &crtc_state->mode;
So, yeah, it looks like you can't make the assumption that crtc_state is
going to be valid here.
I'm not entirely clear on how bridge states are allocated, but it looks
to me that they are through drm_atomic_add_encoder_bridges, which is
called for all the affected connectors in a commit here:
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_atomic_helper.c#L744
Then, the atomic_enable call is made through
drm_atomic_bridge_chain_enable(), which is called in
drm_atomic_helper_commit_modeset_enables only if the CRTC is active and
needs a modeset.
I guess this means that we won't have a null pointer for crtc_state
there, but wouldn't that cause some issues? I can imagine a property
like the bpc count or output format where it wouldn't imply a modeset
but would definitely affect the bridges in the chain?
Maxime
Re: [PATCH 08/10] drm/panel: raspberrypi-touchscreen: Prevent double-free
On Tue, Jul 20, 2021 at 07:19:40PM +0200, Sam Ravnborg wrote:
> Hi Maxime,
> On Tue, Jul 20, 2021 at 03:45:23PM +0200, Maxime Ripard wrote:
> > The mipi_dsi_device allocated by mipi_dsi_device_register_full() is
> > already free'd on release.
> >
> > Fixes: 2f733d6194bd ("drm/panel: Add support for the Raspberry Pi 7"
> > Touchscreen.")
> > Signed-off-by: Maxime Ripard
>
> Reviewed-by: Sam Ravnborg
Thanks, I applied it to drm-misc-fixes
> I did a quick audit (as using grep mostly) to see if other panels had
> the same bug, but did not find others.
Yeah, the RaspberryPi panel seems to be the only odd DSI panel not
controlled through DCS, and the other panels don't have to allocate the
mipi-dsi device anyway.
No bridge seems to have the issue though.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 11/54] dt-bindings: display: simple-bridge: Add corpro, gm7123 compatible
On Wed, Jul 21, 2021 at 04:16:13PM +0200, Sam Ravnborg wrote: > On Wed, Jul 21, 2021 at 04:03:41PM +0200, Maxime Ripard wrote: > > The corpro,gm7123 was in use in a DT but was never properly documented, > > let's add it. > > > > Cc: dri-devel@lists.freedesktop.org > > Reviewed-by: Laurent Pinchart > > Signed-off-by: Maxime Ripard > Acked-by: Sam Ravnborg Applied to drm-misc-next, thanks! Maxime signature.asc Description: PGP signature
Re: [PATCH v6] Documentation: gpu: Mention the requirements for new properties
On Tue, Jul 20, 2021 at 04:35:44PM +0200, Maxime Ripard wrote: > New KMS properties come with a bunch of requirements to avoid each > driver from running their own, inconsistent, set of properties, > eventually leading to issues like property conflicts, inconsistencies > between drivers and semantics, etc. > > Let's document what we expect. > > Cc: Alexandre Belloni > Cc: Alexandre Torgue > Cc: Alex Deucher > Cc: Alison Wang > Cc: Alyssa Rosenzweig > Cc: Andrew Jeffery > Cc: Andrzej Hajda > Cc: Anitha Chrisanthus > Cc: Benjamin Gaignard > Cc: Ben Skeggs > Cc: Boris Brezillon > Cc: Brian Starkey > Cc: Chen Feng > Cc: Chen-Yu Tsai > Cc: Christian Gmeiner > Cc: "Christian König" > Cc: Chun-Kuang Hu > Cc: Edmund Dea > Cc: Eric Anholt > Cc: Fabio Estevam > Cc: Gerd Hoffmann > Cc: Haneen Mohammed > Cc: Hans de Goede > Cc: "Heiko Stübner" > Cc: Huang Rui > Cc: Hyun Kwon > Cc: Inki Dae > Cc: Jani Nikula > Cc: Jernej Skrabec > Cc: Jerome Brunet > Cc: John Stultz > Cc: Jonas Karlman > Cc: Jonathan Hunter > Cc: Joonas Lahtinen > Cc: Joonyoung Shim > Cc: Jyri Sarha > Cc: Kevin Hilman > Cc: Kieran Bingham > Cc: Krzysztof Kozlowski > Cc: Kyungmin Park > Cc: Laurent Pinchart > Cc: Linus Walleij > Cc: Liviu Dudau > Cc: Lucas Stach > Cc: Ludovic Desroches > Cc: Marek Vasut > Cc: Martin Blumenstingl > Cc: Matthias Brugger > Cc: Maxime Coquelin > Cc: Maxime Ripard > Cc: Melissa Wen > Cc: Neil Armstrong > Cc: Nicolas Ferre > Cc: "Noralf Trønnes" > Cc: NXP Linux Team > Cc: Oleksandr Andrushchenko > Cc: Patrik Jakobsson > Cc: Paul Cercueil > Cc: Pekka Paalanen > Cc: Pengutronix Kernel Team > Cc: Philippe Cornu > Cc: Philipp Zabel > Cc: Qiang Yu > Cc: Rob Clark > Cc: Robert Foss > Cc: Rob Herring > Cc: Rodrigo Siqueira > Cc: Rodrigo Vivi > Cc: Roland Scheidegger > Cc: Russell King > Cc: Sam Ravnborg > Cc: Sandy Huang > Cc: Sascha Hauer > Cc: Sean Paul > Cc: Seung-Woo Kim > Cc: Shawn Guo > Cc: Simon Ser > Cc: Stefan Agner > Cc: Steven Price > Cc: Sumit Semwal > Cc: Thierry Reding > Cc: Tian Tao > Cc: Tomeu Vizoso > Cc: Tomi Valkeinen > Cc: VMware Graphics > Cc: Xinliang Liu > Cc: Xinwei Kong > Cc: Yannick Fertre > Cc: Zack Rusin > Reviewed-by: Pekka Paalanen > Reviewed-by: Daniel Vetter > Signed-off-by: Maxime Ripard Applied with Dave's Ack (on IRC) Maxime signature.asc Description: PGP signature
Re: [PATCH 04/10] drm/bridge: Document the probe issue with MIPI-DSI bridges
Hi Daniel, On Wed, Jul 21, 2021 at 02:05:01PM +0200, Daniel Vetter wrote: > On Tue, Jul 20, 2021 at 03:45:19PM +0200, Maxime Ripard wrote: > > Interactions between bridges, panels, MIPI-DSI host and the component > > framework are not trivial and can lead to probing issues when > > implementing a display driver. Let's document the various cases we need > > too consider, and the solution to support all the cases. > > > > Signed-off-by: Maxime Ripard > > I still have this dream that eventually we resurrect a patch to add > device_link to bridges/panels (ideally automatically), to help with some > of the suspend/resume issues around here. > > Will this make things worse? > > I think it'd be really good to figure that out with some coding, since if > we have incompatible solution to handle probe issues vs suspend/resume > issues, we're screwed. > > Atm the duct-tape is to carefully move things around between suspend and > suspend_early hooks (and resume and resume_late) and hope it all works ... My initial idea to fix this was indeed to use device links. I gave up after a while since it doesn't look like there's a way to add a device link before either the bridge or encoder probes. Indeed the OF-Graph representation is device-specific, so it can't be generic, and if you need to probe to add that link, well, it's already too late for the probe ordering :) Maxime signature.asc Description: PGP signature
Re: [PATCH 2/2] drm/vc4: hdmi: Remove unused struct
On Wed, Jul 28, 2021 at 12:01:34PM +0100, Dave Stevenson wrote:
> On Wed, 7 Jul 2021 at 10:36, Maxime Ripard wrote:
> >
> > Commit 91e99e113929 ("drm/vc4: hdmi: Register HDMI codec") removed the
> > references to the vc4_hdmi_audio_component_drv structure, but not the
> > structure itself resulting in a warning. Remove it.
> >
> > Fixes: 91e99e113929 ("drm/vc4: hdmi: Register HDMI codec")
> > Reported-by: kernel test robot
> > Signed-off-by: Maxime Ripard
>
> Reviewed-by: Dave Stevenson
Applied, thanks!
Maxime
signature.asc
Description: PGP signature
Re: [PATCH] drm/vc4: hdmi: Add debugfs prefix
Hi, On Fri, Jul 23, 2021 at 09:24:14AM +0200, Ivan T. Ivanov wrote: > Without prefix debugfs can't properly create component > debug information tree when driver register more than > one component per device, in this case two. Fix this. > > debugfs: Directory 'fef00700.hdmi' with parent 'vc4-hdmi-0' already present! > > Signed-off-by: Ivan T. Ivanov Thanks for your patch. However, that part changed fairly significantly recently so you'll need to rebase it on top of the drm-misc-next (or linux-next) Maxime signature.asc Description: PGP signature
Re: [PATCH 04/10] drm/bridge: Document the probe issue with MIPI-DSI bridges
Hi, On Tue, Jul 27, 2021 at 11:20:54AM +0200, Daniel Vetter wrote: > On Mon, Jul 26, 2021 at 05:16:57PM +0200, Maxime Ripard wrote: > > Hi Daniel, > > > > On Wed, Jul 21, 2021 at 02:05:01PM +0200, Daniel Vetter wrote: > > > On Tue, Jul 20, 2021 at 03:45:19PM +0200, Maxime Ripard wrote: > > > > Interactions between bridges, panels, MIPI-DSI host and the component > > > > framework are not trivial and can lead to probing issues when > > > > implementing a display driver. Let's document the various cases we need > > > > too consider, and the solution to support all the cases. > > > > > > > > Signed-off-by: Maxime Ripard > > > > > > I still have this dream that eventually we resurrect a patch to add > > > device_link to bridges/panels (ideally automatically), to help with some > > > of the suspend/resume issues around here. > > > > > > Will this make things worse? > > > > > > I think it'd be really good to figure that out with some coding, since if > > > we have incompatible solution to handle probe issues vs suspend/resume > > > issues, we're screwed. > > > > > > Atm the duct-tape is to carefully move things around between suspend and > > > suspend_early hooks (and resume and resume_late) and hope it all works ... > > > > My initial idea to fix this was indeed to use device links. I gave up > > after a while since it doesn't look like there's a way to add a device > > link before either the bridge or encoder probes. > > > > Indeed the OF-Graph representation is device-specific, so it can't be > > generic, and if you need to probe to add that link, well, it's already > > too late for the probe ordering :) > > But don't we still need the device_link for suspend/resume and module > reload? All very annoying indeed anyway. I guess we would still need it for proper suspend and resume ordering (but I never really worked on that part, so I'm not sure), but it's a bit orthogonal to the issue here since those can be added after probe Maxime signature.asc Description: PGP signature
[PATCH v2 0/8] drm/bridge: Make panel and bridge probe order consistent
Hi,
We've encountered an issue with the RaspberryPi DSI panel that prevented the
whole display driver from probing.
The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
Only register our component once a DSI device is attached"), but the basic idea
is that since the panel is probed through i2c, there's no synchronization
between its probe and the registration of the MIPI-DSI host it's attached to.
We initially moved the component framework registration to the MIPI-DSI Host
attach hook to make sure we register our component only when we have a DSI
device attached to our MIPI-DSI host, and then use lookup our DSI device in our
bind hook.
However, all the DSI bridges controlled through i2c are only registering their
associated DSI device in their bridge attach hook, meaning with our change
above, we never got that far, and therefore ended up in the same situation than
the one we were trying to fix for panels.
Since the RaspberryPi panel is the only driver in that situation, whereas it
seems like there's a consensus in bridge drivers, it makes more sense to try to
mimic the bridge pattern in the panel driver.
However, panels don't have an attach hook, and adding more panel hooks would
lead to more path to maintain in each and every driver, while the general push
is towards bridges. We also have to make sure that each and every DSI host and
device driver behaves the same in order to have expectations to rely on.
The solution I'm proposing is thus done in several steps:
- We get rid of the initial patch to make sure we support the bridge case,
and not the odd-panel one.
- Add a function that returns a bridge from a DT node, reducing the amount of
churn in each and every driver and making it a real incentive to not care
about panels in display drivers but only bridges.
- Add an attach and detach hook into the panel operations, and make it called
automatically by the DRM panel bridge.
- Convert the VC4 DSI host to this new bridge function, and the RaspberryPi
Panel to the new attach and detach hooks.
If the general approach is agreed upon, other drivers will obviously be
converted to drm_of_get_next.
Let me know what you think,
Maxime
---
Changes from v1:
- Change the name of drm_of_get_next function to drm_of_get_bridge
- Mention the revert of 87154ff86bf6 and squash the two patches that were
reverting that commit
- Add some documentation
- Make drm_panel_attach and _detach succeed when no callback is there
Maxime Ripard (8):
Revert "drm/vc4: dsi: Only register our component once a DSI device is
attached"
drm/bridge: Add a function to abstract away panels
drm/bridge: Add documentation sections
drm/bridge: Document the probe issue with MIPI-DSI bridges
drm/panel: Create attach and detach callbacks
drm/vc4: dsi: Switch to drm_of_get_bridge
drm/panel: raspberrypi-touchscreen: Use the attach hook
drm/panel: raspberrypi-touchscreen: Remove MIPI-DSI driver
Documentation/gpu/drm-kms-helpers.rst | 12 ++
drivers/gpu/drm/bridge/panel.c| 6 +
drivers/gpu/drm/drm_bridge.c | 114 -
drivers/gpu/drm/drm_of.c | 3 +
drivers/gpu/drm/drm_panel.c | 47 ++
.../drm/panel/panel-raspberrypi-touchscreen.c | 158 +-
drivers/gpu/drm/vc4/vc4_drv.c | 2 +
drivers/gpu/drm/vc4/vc4_dsi.c | 53 +++---
include/drm/drm_bridge.h | 2 +
include/drm/drm_panel.h | 27 +++
10 files changed, 303 insertions(+), 121 deletions(-)
--
2.31.1
[PATCH v2 1/8] Revert "drm/vc4: dsi: Only register our component once a DSI device is attached"
This reverts commit 7213246a803f9b8da0677adb9ae06a3d8b806d02.
The commit 7213246a803f ("drm/vc4: dsi: Only register our component once
a DSI device is attached") aimed at preventing an endless probe loop
between the DSI host driver and its panel/bridge where both would wait
for each other to probe.
The solution implemented in that commit however relies on the fact that
MIPI-DSI device will either be a MIPI-DSI device, or would call
mipi_dsi_device_register_full() at probe time.
This assumption isn't true for bridges though where most drivers will do
so in the bridge attach hook. However, the drm_bridge_attach is usually
called in the DSI host bind hook, and thus we never get this far,
resulting in a DSI bridge that will never have its attach run, and the
DSI host that will never be bound, effectively creating the same
situation we were trying to avoid for panels.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_dsi.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index a55256ed0955..6dfcbd9e234e 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -1257,12 +1257,10 @@ static ssize_t vc4_dsi_host_transfer(struct
mipi_dsi_host *host,
return ret;
}
-static const struct component_ops vc4_dsi_ops;
static int vc4_dsi_host_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device *device)
{
struct vc4_dsi *dsi = host_to_dsi(host);
- int ret;
dsi->lanes = device->lanes;
dsi->channel = device->channel;
@@ -1297,12 +1295,6 @@ static int vc4_dsi_host_attach(struct mipi_dsi_host
*host,
return 0;
}
- ret = component_add(&dsi->pdev->dev, &vc4_dsi_ops);
- if (ret) {
- mipi_dsi_host_unregister(&dsi->dsi_host);
- return ret;
- }
-
return 0;
}
@@ -1689,6 +1681,7 @@ static int vc4_dsi_dev_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct vc4_dsi *dsi;
+ int ret;
dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
if (!dsi)
@@ -1696,10 +1689,26 @@ static int vc4_dsi_dev_probe(struct platform_device
*pdev)
dev_set_drvdata(dev, dsi);
dsi->pdev = pdev;
+
+ /* Note, the initialization sequence for DSI and panels is
+* tricky. The component bind above won't get past its
+* -EPROBE_DEFER until the panel/bridge probes. The
+* panel/bridge will return -EPROBE_DEFER until it has a
+* mipi_dsi_host to register its device to. So, we register
+* the host during pdev probe time, so vc4 as a whole can then
+* -EPROBE_DEFER its component bind process until the panel
+* successfully attaches.
+*/
dsi->dsi_host.ops = &vc4_dsi_host_ops;
dsi->dsi_host.dev = dev;
mipi_dsi_host_register(&dsi->dsi_host);
+ ret = component_add(&pdev->dev, &vc4_dsi_ops);
+ if (ret) {
+ mipi_dsi_host_unregister(&dsi->dsi_host);
+ return ret;
+ }
+
return 0;
}
--
2.31.1
[PATCH v2 2/8] drm/bridge: Add a function to abstract away panels
Display drivers so far need to have a lot of boilerplate to first
retrieve either the panel or bridge that they are connected to using
drm_of_find_panel_or_bridge(), and then either deal with each with ad-hoc
functions or create a drm panel bridge through drm_panel_bridge_add.
In order to reduce the boilerplate and hopefully create a path of least
resistance towards using the DRM panel bridge layer, let's create the
function devm_drm_of_get_next to reduce that boilerplate.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/drm_bridge.c | 42
drivers/gpu/drm/drm_of.c | 3 +++
include/drm/drm_bridge.h | 2 ++
3 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 044acd07c153..426ce7442c86 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include "drm_crtc_internal.h"
@@ -50,10 +51,8 @@
*
* Display drivers are responsible for linking encoders with the first bridge
* in the chains. This is done by acquiring the appropriate bridge with
- * of_drm_find_bridge() or drm_of_find_panel_or_bridge(), or creating it for a
- * panel with drm_panel_bridge_add_typed() (or the managed version
- * devm_drm_panel_bridge_add_typed()). Once acquired, the bridge shall be
- * attached to the encoder with a call to drm_bridge_attach().
+ * devm_drm_of_get_bridge(). Once acquired, the bridge shall be attached to the
+ * encoder with a call to drm_bridge_attach().
*
* Bridges are responsible for linking themselves with the next bridge in the
* chain, if any. This is done the same way as for encoders, with the call to
@@ -1223,6 +1222,41 @@ struct drm_bridge *of_drm_find_bridge(struct device_node
*np)
return NULL;
}
EXPORT_SYMBOL(of_drm_find_bridge);
+
+/**
+ * devm_drm_of_get_bridge - Return next bridge in the chain
+ * @dev: device to tie the bridge lifetime to
+ * @np: device tree node containing encoder output ports
+ * @port: port in the device tree node
+ * @endpoint: endpoint in the device tree node
+ *
+ * Given a DT node's port and endpoint number, finds the connected node
+ * and returns the associated bridge if any, or creates and returns a
+ * drm panel bridge instance if a panel is connected.
+ *
+ * Returns a pointer to the bridge if successful, or an error pointer
+ * otherwise.
+ */
+struct drm_bridge *devm_drm_of_get_bridge(struct device *dev,
+ struct device_node *np,
+ unsigned int port,
+ unsigned int endpoint)
+{
+ struct drm_bridge *bridge;
+ struct drm_panel *panel;
+ int ret;
+
+ ret = drm_of_find_panel_or_bridge(np, port, endpoint,
+ &panel, &bridge);
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (panel)
+ bridge = devm_drm_panel_bridge_add(dev, panel);
+
+ return bridge;
+}
+EXPORT_SYMBOL(devm_drm_of_get_bridge);
#endif
MODULE_AUTHOR("Ajay Kumar ");
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 997b8827fed2..37c34146eea8 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -231,6 +231,9 @@ EXPORT_SYMBOL_GPL(drm_of_encoder_active_endpoint);
* return either the associated struct drm_panel or drm_bridge device. Either
* @panel or @bridge must not be NULL.
*
+ * This function is deprecated and should not be used in new drivers. Use
+ * devm_drm_of_get_bridge() instead.
+ *
* Returns zero if successful, or one of the standard error codes if it fails.
*/
int drm_of_find_panel_or_bridge(const struct device_node *np,
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 46bdfa48c413..f70c88ca96ef 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -911,6 +911,8 @@ struct drm_bridge *devm_drm_panel_bridge_add(struct device
*dev,
struct drm_bridge *devm_drm_panel_bridge_add_typed(struct device *dev,
struct drm_panel *panel,
u32 connector_type);
+struct drm_bridge *devm_drm_of_get_bridge(struct device *dev, struct
device_node *node,
+ unsigned int port, unsigned int
endpoint);
struct drm_connector *drm_panel_bridge_connector(struct drm_bridge *bridge);
#endif
--
2.31.1
[PATCH v2 3/8] drm/bridge: Add documentation sections
The bridge documentation overview is quite packed already, and we'll add some more documentation that isn't part of an overview at all. Let's add some sections to the documentation to separate each bits. Reviewed-by: Sam Ravnborg Signed-off-by: Maxime Ripard --- Documentation/gpu/drm-kms-helpers.rst | 6 ++ drivers/gpu/drm/drm_bridge.c | 14 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index 389892f36185..10f8df7aecc0 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -151,6 +151,12 @@ Overview .. kernel-doc:: drivers/gpu/drm/drm_bridge.c :doc: overview +Display Driver Integration +-- + +.. kernel-doc:: drivers/gpu/drm/drm_bridge.c + :doc: display driver integration + Bridge Operations - diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 426ce7442c86..71d3370ce209 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -49,6 +49,15 @@ * Chaining multiple bridges to the output of a bridge, or the same bridge to * the output of different bridges, is not supported. * + * &drm_bridge, like &drm_panel, aren't &drm_mode_object entities like planes, + * CRTCs, encoders or connectors and hence are not visible to userspace. They + * just provide additional hooks to get the desired output at the end of the + * encoder chain. + */ + +/** + * DOC:display driver integration + * * Display drivers are responsible for linking encoders with the first bridge * in the chains. This is done by acquiring the appropriate bridge with * devm_drm_of_get_bridge(). Once acquired, the bridge shall be attached to the @@ -83,11 +92,6 @@ * helper to create the &drm_connector, or implement it manually on top of the * connector-related operations exposed by the bridge (see the overview * documentation of bridge operations for more details). - * - * &drm_bridge, like &drm_panel, aren't &drm_mode_object entities like planes, - * CRTCs, encoders or connectors and hence are not visible to userspace. They - * just provide additional hooks to get the desired output at the end of the - * encoder chain. */ static DEFINE_MUTEX(bridge_lock); -- 2.31.1
[PATCH v2 4/8] drm/bridge: Document the probe issue with MIPI-DSI bridges
Interactions between bridges, panels, MIPI-DSI host and the component framework are not trivial and can lead to probing issues when implementing a display driver. Let's document the various cases we need too consider, and the solution to support all the cases. Signed-off-by: Maxime Ripard --- Documentation/gpu/drm-kms-helpers.rst | 6 +++ drivers/gpu/drm/drm_bridge.c | 60 +++ 2 files changed, 66 insertions(+) diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index 10f8df7aecc0..ec2f65b31930 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -157,6 +157,12 @@ Display Driver Integration .. kernel-doc:: drivers/gpu/drm/drm_bridge.c :doc: display driver integration +Special Care with MIPI-DSI bridges +-- + +.. kernel-doc:: drivers/gpu/drm/drm_bridge.c + :doc: special care dsi + Bridge Operations - diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 71d3370ce209..fd01f1ce7976 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -94,6 +94,66 @@ * documentation of bridge operations for more details). */ +/** + * DOC: special care dsi + * + * The interaction between the bridges and other frameworks involved in + * the probing of the display driver and the bridge driver can be + * challenging. Indeed, there's multiple cases that needs to be + * considered: + * + * - The display driver doesn't use the component framework and isn't a + * MIPI-DSI host. In this case, the bridge driver will probe at some + * point and the display driver should try to probe again by returning + * EPROBE_DEFER as long as the bridge driver hasn't probed. + * + * - The display driver doesn't use the component framework, but is a + * MIPI-DSI host. The bridge device uses the MIPI-DCS commands to be + * controlled. In this case, the bridge device is a child of the + * display device and when it will probe it's assured that the display + * device (and MIPI-DSI host) is present. The display driver will be + * assured that the bridge driver is connected between the + * &mipi_dsi_host_ops.attach and &mipi_dsi_host_ops.detach operations. + * Therefore, it must run mipi_dsi_host_register() in its probe + * function, and then run drm_bridge_attach() in its + * &mipi_dsi_host_ops.attach hook. + * + * - The display driver uses the component framework and is a MIPI-DSI + * host. The bridge device uses the MIPI-DCS commands to be + * controlled. This is the same situation than above, and can run + * mipi_dsi_host_register() in either its probe or bind hooks. + * + * - The display driver uses the component framework and is a MIPI-DSI + * host. The bridge device uses a separate bus (such as I2C) to be + * controlled. In this case, there's no correlation between the probe + * of the bridge and display drivers, so care must be taken to avoid + * an endless EPROBE_DEFER loop, with each driver waiting for the + * other to probe. + * + * The ideal pattern to cover the last item (and all the others in the + * display driver case) is to split the operations like this: + * + * - In the display driver must run mipi_dsi_host_register() and + * component_add in its probe hook. It will make sure that the + * MIPI-DSI host sticks around, and that the driver's bind can be + * called. + * + * - In its probe hook, the bridge driver must not try to find its + * MIPI-DSI host or register as a MIPI-DSI device. As far as the + * framework is concerned, it must only call drm_bridge_add(). + * + * - In its bind hook, the display driver must try to find the bridge + * and return -EPROBE_DEFER if it doesn't find it. If it's there, it + * must call drm_bridge_attach(). The MIPI-DSI host is now functional. + * + * - In its &drm_bridge_funcs.attach hook, the bridge driver can now try + * to find its MIPI-DSI host and can register as a MIPI-DSI device. + * + * At this point, we're now certain that both the display driver and the + * bridge driver are functional and we can't have a deadlock-like + * situation when probing. + */ + static DEFINE_MUTEX(bridge_lock); static LIST_HEAD(bridge_list); -- 2.31.1
[PATCH v2 6/8] drm/vc4: dsi: Switch to drm_of_get_bridge
The new drm_of_get_bridge removes most of the boilerplate we have to deal
with. Let's switch to it.
Signed-off-by: Maxime Ripard
fixup! drm/vc4: dsi: Switch to drm_of_get_bridge
---
drivers/gpu/drm/vc4/vc4_drv.c | 2 ++
drivers/gpu/drm/vc4/vc4_dsi.c | 28
2 files changed, 6 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 73335feb712f..ff056ee8bc4b 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -25,7 +25,9 @@
#include
#include
#include
+#include
#include
+#include
#include
#include
#include
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 6dfcbd9e234e..3db03c95707f 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -1489,7 +1489,6 @@ static int vc4_dsi_bind(struct device *dev, struct device
*master, void *data)
struct drm_device *drm = dev_get_drvdata(master);
struct vc4_dsi *dsi = dev_get_drvdata(dev);
struct vc4_dsi_encoder *vc4_dsi_encoder;
- struct drm_panel *panel;
const struct of_device_id *match;
dma_cap_mask_t dma_mask;
int ret;
@@ -1601,27 +1600,9 @@ static int vc4_dsi_bind(struct device *dev, struct
device *master, void *data)
return ret;
}
- ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
- &panel, &dsi->bridge);
- if (ret) {
- /* If the bridge or panel pointed by dev->of_node is not
-* enabled, just return 0 here so that we don't prevent the DRM
-* dev from being registered. Of course that means the DSI
-* encoder won't be exposed, but that's not a problem since
-* nothing is connected to it.
-*/
- if (ret == -ENODEV)
- return 0;
-
- return ret;
- }
-
- if (panel) {
- dsi->bridge = devm_drm_panel_bridge_add_typed(dev, panel,
-
DRM_MODE_CONNECTOR_DSI);
- if (IS_ERR(dsi->bridge))
- return PTR_ERR(dsi->bridge);
- }
+ dsi->bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
+ if (IS_ERR(dsi->bridge))
+ return PTR_ERR(dsi->bridge);
/* The esc clock rate is supposed to always be 100Mhz. */
ret = clk_set_rate(dsi->escape_clock, 100 * 100);
@@ -1661,8 +1642,7 @@ static void vc4_dsi_unbind(struct device *dev, struct
device *master,
{
struct vc4_dsi *dsi = dev_get_drvdata(dev);
- if (dsi->bridge)
- pm_runtime_disable(dev);
+ pm_runtime_disable(dev);
/*
* Restore the bridge_chain so the bridge detach procedure can happen
--
2.31.1
[PATCH v2 5/8] drm/panel: Create attach and detach callbacks
This is a partial revert of 87154ff86bf6 ("drm: Remove unnecessary
drm_panel_attach and drm_panel_detach").
In order to make the probe order expectation more consistent between
bridges, let's create attach and detach hooks for the panels as well to
match what is there for bridges.
Most drivers have changed significantly since the reverted commit, so we
only brought back the calls in the panel bridge to lessen the risk of
regressions, and since panel bridge is what we're converging to these
days, it's not a big deal anyway.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/bridge/panel.c | 6 +
drivers/gpu/drm/drm_panel.c| 47 ++
include/drm/drm_panel.h| 27 +++
3 files changed, 80 insertions(+)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index c916f4b8907e..0b06e0dbad00 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -82,6 +82,10 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
drm_connector_attach_encoder(&panel_bridge->connector,
bridge->encoder);
+ ret = drm_panel_attach(panel_bridge->panel, &panel_bridge->connector);
+ if (ret < 0)
+ return ret;
+
return 0;
}
@@ -90,6 +94,8 @@ static void panel_bridge_detach(struct drm_bridge *bridge)
struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
struct drm_connector *connector = &panel_bridge->connector;
+ drm_panel_detach(panel_bridge->panel);
+
/*
* Cleanup the connector if we know it was initialized.
*
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index f634371c717a..10716b740685 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -93,6 +93,53 @@ void drm_panel_remove(struct drm_panel *panel)
}
EXPORT_SYMBOL(drm_panel_remove);
+/**
+ * drm_panel_attach - attach a panel to a connector
+ * @panel: DRM panel
+ * @connector: DRM connector
+ *
+ * After obtaining a pointer to a DRM panel a display driver calls this
+ * function to attach a panel to a connector.
+ *
+ * An error is returned if the panel is already attached to another connector.
+ *
+ * When unloading, the driver should detach from the panel by calling
+ * drm_panel_detach().
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector)
+{
+ if (!panel)
+ return -EINVAL;
+
+ if (panel->funcs && panel->funcs->attach)
+ return panel->funcs->attach(panel);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_panel_attach);
+
+/**
+ * drm_panel_detach - detach a panel from a connector
+ * @panel: DRM panel
+ *
+ * Detaches a panel from the connector it is attached to. If a panel is not
+ * attached to any connector this is effectively a no-op.
+ *
+ * This function should not be called by the panel device itself. It
+ * is only for the drm device that called drm_panel_attach().
+ */
+void drm_panel_detach(struct drm_panel *panel)
+{
+ if (!panel)
+ return;
+
+ if (panel->funcs && panel->funcs->detach)
+ panel->funcs->detach(panel);
+}
+EXPORT_SYMBOL(drm_panel_detach);
+
/**
* drm_panel_prepare - power on a panel
* @panel: DRM panel
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 4602f833eb51..33d139e98b19 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -68,6 +68,30 @@ enum drm_panel_orientation;
* does not need to implement the functionality to enable/disable backlight.
*/
struct drm_panel_funcs {
+ /**
+* @attach:
+*
+* This callback is invoked whenever our panel is being attached
+* to its DRM connector.
+*
+* The @attach callback is optional.
+*
+* RETURNS:
+*
+* Zero on success, error code on failure.
+*/
+ int (*attach)(struct drm_panel *panel);
+
+ /**
+* @detach:
+*
+* This callback is invoked whenever our panel is being detached
+* from its DRM connector.
+*
+* The @detach callback is optional.
+*/
+ void (*detach)(struct drm_panel *panel);
+
/**
* @prepare:
*
@@ -180,6 +204,9 @@ void drm_panel_init(struct drm_panel *panel, struct device
*dev,
void drm_panel_add(struct drm_panel *panel);
void drm_panel_remove(struct drm_panel *panel);
+int drm_panel_attach(struct drm_panel *panel, struct drm_connector *connector);
+void drm_panel_detach(struct drm_panel *panel);
+
int drm_panel_prepare(struct drm_panel *panel);
int drm_panel_unprepare(struct drm_panel *panel);
--
2.31.1
[PATCH v2 8/8] drm/panel: raspberrypi-touchscreen: Remove MIPI-DSI driver
The driver was using a two-steps initialisation when probing with the
i2c probe first registering the MIPI-DSI device, and then when that
device was probed the driver would attach the device to its host. This
resulted in a fairly non-standard probe logic.
The previous commit changed that logic entirely though, resulting in a
completely empty MIPI-DSI device probe. Let's simplify the driver by
removing it entirely and just behave as a normal i2c driver.
Signed-off-by: Maxime Ripard
---
.../drm/panel/panel-raspberrypi-touchscreen.c | 25 +--
1 file changed, 1 insertion(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
index 995c5cafb970..09937aa26c6a 100644
--- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
+++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
@@ -483,16 +483,6 @@ static int rpi_touchscreen_remove(struct i2c_client *i2c)
return 0;
}
-static int rpi_touchscreen_dsi_probe(struct mipi_dsi_device *dsi)
-{
- return 0;
-}
-
-static struct mipi_dsi_driver rpi_touchscreen_dsi_driver = {
- .driver.name = RPI_DSI_DRIVER_NAME,
- .probe = rpi_touchscreen_dsi_probe,
-};
-
static const struct of_device_id rpi_touchscreen_of_ids[] = {
{ .compatible = "raspberrypi,7inch-touchscreen-panel" },
{ } /* sentinel */
@@ -507,20 +497,7 @@ static struct i2c_driver rpi_touchscreen_driver = {
.probe = rpi_touchscreen_probe,
.remove = rpi_touchscreen_remove,
};
-
-static int __init rpi_touchscreen_init(void)
-{
- mipi_dsi_driver_register(&rpi_touchscreen_dsi_driver);
- return i2c_add_driver(&rpi_touchscreen_driver);
-}
-module_init(rpi_touchscreen_init);
-
-static void __exit rpi_touchscreen_exit(void)
-{
- i2c_del_driver(&rpi_touchscreen_driver);
- mipi_dsi_driver_unregister(&rpi_touchscreen_dsi_driver);
-}
-module_exit(rpi_touchscreen_exit);
+module_i2c_driver(rpi_touchscreen_driver);
MODULE_AUTHOR("Eric Anholt ");
MODULE_DESCRIPTION("Raspberry Pi 7-inch touchscreen driver");
--
2.31.1
[PATCH v2 7/8] drm/panel: raspberrypi-touchscreen: Use the attach hook
Now that we have an attach hook available for panels as well, let's use
it for the RaspberryPi 7" DSI panel.
This now mimics what all the other bridges in a similar situation are
doing, and we avoid our probe order issue entirely.
Signed-off-by: Maxime Ripard
---
.../drm/panel/panel-raspberrypi-touchscreen.c | 135 ++
1 file changed, 77 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
index 462faae0f446..995c5cafb970 100644
--- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
+++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
@@ -346,7 +346,83 @@ static int rpi_touchscreen_get_modes(struct drm_panel
*panel,
return num;
}
+static int rpi_touchscreen_attach(struct drm_panel *panel)
+{
+ struct rpi_touchscreen *ts = panel_to_ts(panel);
+ struct device *dev = &ts->i2c->dev;
+ struct device_node *endpoint, *dsi_host_node;
+ struct mipi_dsi_device *dsi;
+ struct mipi_dsi_host *host;
+ int ret;
+
+ struct mipi_dsi_device_info info = {
+ .type = RPI_DSI_DRIVER_NAME,
+ .channel = 0,
+ .node = NULL,
+ };
+
+ /* Look up the DSI host. It needs to probe before we do. */
+ endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
+ if (!endpoint)
+ return -ENODEV;
+
+ dsi_host_node = of_graph_get_remote_port_parent(endpoint);
+ if (!dsi_host_node) {
+ of_node_put(endpoint);
+ return -ENODEV;
+ }
+
+ host = of_find_mipi_dsi_host_by_node(dsi_host_node);
+ of_node_put(dsi_host_node);
+ if (!host) {
+ of_node_put(endpoint);
+ return -EPROBE_DEFER;
+ }
+
+ info.node = of_graph_get_remote_port(endpoint);
+ if (!info.node) {
+ of_node_put(endpoint);
+ return -ENODEV;
+ }
+
+ of_node_put(endpoint);
+
+ dsi = mipi_dsi_device_register_full(host, &info);
+ if (IS_ERR(dsi)) {
+ dev_err(dev, "DSI device registration failed: %ld\n",
+ PTR_ERR(dsi));
+ return PTR_ERR(dsi);
+ }
+
+ ts->dsi = dsi;
+
+ dsi->mode_flags = (MIPI_DSI_MODE_VIDEO |
+ MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
+ MIPI_DSI_MODE_LPM);
+ dsi->format = MIPI_DSI_FMT_RGB888;
+ dsi->lanes = 1;
+
+ ret = mipi_dsi_attach(dsi);
+ if (ret) {
+ dev_err(&dsi->dev, "failed to attach dsi to host: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void rpi_touchscreen_detach(struct drm_panel *panel)
+{
+ struct rpi_touchscreen *ts = panel_to_ts(panel);
+
+ mipi_dsi_detach(ts->dsi);
+ mipi_dsi_device_unregister(ts->dsi);
+}
+
static const struct drm_panel_funcs rpi_touchscreen_funcs = {
+ .attach = rpi_touchscreen_attach,
+ .detach = rpi_touchscreen_detach,
+
.disable = rpi_touchscreen_disable,
.unprepare = rpi_touchscreen_noop,
.prepare = rpi_touchscreen_noop,
@@ -359,14 +435,7 @@ static int rpi_touchscreen_probe(struct i2c_client *i2c,
{
struct device *dev = &i2c->dev;
struct rpi_touchscreen *ts;
- struct device_node *endpoint, *dsi_host_node;
- struct mipi_dsi_host *host;
int ver;
- struct mipi_dsi_device_info info = {
- .type = RPI_DSI_DRIVER_NAME,
- .channel = 0,
- .node = NULL,
- };
ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
if (!ts)
@@ -394,35 +463,6 @@ static int rpi_touchscreen_probe(struct i2c_client *i2c,
/* /\* Turn off at boot, so we can cleanly sequence powering on. *\/ */
/* rpi_touchscreen_i2c_write(ts, REG_POWERON, 0); */
- /* Look up the DSI host. It needs to probe before we do. */
- endpoint = of_graph_get_next_endpoint(dev->of_node, NULL);
- if (!endpoint)
- return -ENODEV;
-
- dsi_host_node = of_graph_get_remote_port_parent(endpoint);
- if (!dsi_host_node)
- goto error;
-
- host = of_find_mipi_dsi_host_by_node(dsi_host_node);
- of_node_put(dsi_host_node);
- if (!host) {
- of_node_put(endpoint);
- return -EPROBE_DEFER;
- }
-
- info.node = of_graph_get_remote_port(endpoint);
- if (!info.node)
- goto error;
-
- of_node_put(endpoint);
-
- ts->dsi = mipi_dsi_device_register_full(host, &info);
- if (IS_ERR(ts->dsi)) {
- dev_err(dev, "DSI device registration failed: %ld\n",
- PTR_ERR(ts->dsi));
- return PTR_ERR(ts->dsi);
- }
-
drm_panel_init(
Re: [PATCH 04/10] drm/bridge: Document the probe issue with MIPI-DSI bridges
Hi Jagan, On Tue, Jul 27, 2021 at 03:12:09PM +0530, Jagan Teki wrote: > On Tue, Jul 20, 2021 at 7:15 PM Maxime Ripard wrote: > > > > Interactions between bridges, panels, MIPI-DSI host and the component > > framework are not trivial and can lead to probing issues when > > implementing a display driver. Let's document the various cases we need > > too consider, and the solution to support all the cases. > > > > Signed-off-by: Maxime Ripard > > --- > > Documentation/gpu/drm-kms-helpers.rst | 6 +++ > > drivers/gpu/drm/drm_bridge.c | 60 +++ > > 2 files changed, 66 insertions(+) > > > > diff --git a/Documentation/gpu/drm-kms-helpers.rst > > b/Documentation/gpu/drm-kms-helpers.rst > > index 10f8df7aecc0..ec2f65b31930 100644 > > --- a/Documentation/gpu/drm-kms-helpers.rst > > +++ b/Documentation/gpu/drm-kms-helpers.rst > > @@ -157,6 +157,12 @@ Display Driver Integration > > .. kernel-doc:: drivers/gpu/drm/drm_bridge.c > > :doc: display driver integration > > > > +Special Care with MIPI-DSI bridges > > +-- > > + > > +.. kernel-doc:: drivers/gpu/drm/drm_bridge.c > > + :doc: special care dsi > > + > > Bridge Operations > > - > > > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > > index c9a950bfdfe5..81f8dac12367 100644 > > --- a/drivers/gpu/drm/drm_bridge.c > > +++ b/drivers/gpu/drm/drm_bridge.c > > @@ -95,6 +95,66 @@ > > * documentation of bridge operations for more details). > > */ > > > > +/** > > + * DOC: special care dsi > > + * > > + * The interaction between the bridges and other frameworks involved in > > + * the probing of the display driver and the bridge driver can be > > + * challenging. Indeed, there's multiple cases that needs to be > > + * considered: > > + * > > + * - The display driver doesn't use the component framework and isn't a > > + * MIPI-DSI host. In this case, the bridge driver will probe at some > > + * point and the display driver should try to probe again by returning > > + * EPROBE_DEFER as long as the bridge driver hasn't probed. > > + * > > + * - The display driver doesn't use the component framework, but is a > > + * MIPI-DSI host. The bridge device uses the MIPI-DCS commands to be > > + * controlled. In this case, the bridge device is a child of the > > + * display device and when it will probe it's assured that the display > > + * device (and MIPI-DSI host) is present. The display driver will be > > + * assured that the bridge driver is connected between the > > + * &mipi_dsi_host_ops.attach and &mipi_dsi_host_ops.detach operations. > > + * Therefore, it must run mipi_dsi_host_register() in its probe > > + * function, and then run drm_bridge_attach() in its > > + * &mipi_dsi_host_ops.attach hook. > > + * > > + * - The display driver uses the component framework and is a MIPI-DSI > > + * host. The bridge device uses the MIPI-DCS commands to be > > + * controlled. This is the same situation than above, and can run > > + * mipi_dsi_host_register() in either its probe or bind hooks. > > + * > > + * - The display driver uses the component framework and is a MIPI-DSI > > + * host. The bridge device uses a separate bus (such as I2C) to be > > + * controlled. In this case, there's no correlation between the probe > > + * of the bridge and display drivers, so care must be taken to avoid > > + * an endless EPROBE_DEFER loop, with each driver waiting for the > > + * other to probe. > > + * > > + * The ideal pattern to cover the last item (and all the others in the > > + * display driver case) is to split the operations like this: > > + * > > + * - In the display driver must run mipi_dsi_host_register() and > > + * component_add in its probe hook. It will make sure that the > > + * MIPI-DSI host sticks around, and that the driver's bind can be > > + * called. > > + * > > + * - In its probe hook, the bridge driver must not try to find its > > + * MIPI-DSI host or register as a MIPI-DSI device. As far as the > > + * framework is concerned, it must only call drm_bridge_add(). > > + * > > + * - In its bind hook, the display driver must try to find the bridge > > + * and return -EPROBE_DEFER if it doesn't find it. If it's there, it > > + * must call drm_bridge_attach(). The MIPI-DSI host is now functional. > >
Re: [Regression] No framebuffer console on Rpi since 5.14-rc1
Hi Stefan, On Wed, Jul 28, 2021 at 05:14:38PM +0200, Stefan Wahren wrote: > Hi, > > Am 15.07.21 um 18:35 schrieb Stefan Wahren: > > Hi guys, > > > > starting with Linux 5.14-rc1 the framebuffer console on Raspberry Pi 3/4 > > (no U-Boot, multi_v7_defconfig) isn't available anymore. The display > > shows the rainbow screen from the bootloader and than the HDMI screen > > goes black instead of kernel messages. > > > > I bisected the issue: > > > > 62fb9874f5da54fdb243003b386128037319b219 good > > e73f0f0ee7541171d89f2e2491130c7771ba58d3 bad > > e058a84bfddc42ba356a2316f2cf1141974625c9 bad > > a6eaf3850cb171c328a8b0db6d3c79286a1eba9d good > > 007b312c6f294770de01fbc0643610145012d244 good > > 18703923a66aecf6f7ded0e16d22eb412ddae72f good > > 334200bf52f0637a5ab8331c557dfcecbb9c30fa bad > > c707b73f0cfb1acc94a20389aecde65e6385349b bad > > caa18dd6dd9305d52943a6b59f410cbc960ad0a0 good > > 691cf8cd7a531dbfcc29d09a23c509a86fd9b24f bad > > 2fdcb55dfc86835e4845e3f422180b5596d23cb4 bad > > 6c3f953381e526a1623d4575660afae8b19ffa20 bad > > 5ea4dba68305d9648b9dba30036cc36d4e877bca bad > > 4a888ba03fd97d1cb0253581973533965bf348c4 good > > c5ef15ae09637fb51ae43e1d1d98329d67dd4fd6 good > > f611b1e7624ccdbd495c19e9805629e22265aa16 bad > > ff323d6d72e1e4971c8ba9e2f3cf8afc48f22383 good > > > > f611b1e7624ccdbd495c19e9805629e22265aa16 is the first bad commit > > commit f611b1e7624ccdbd495c19e9805629e22265aa16 > > Author: Kees Cook > > Date: Wed Jun 2 14:52:50 2021 -0700 > > > > drm: Avoid circular dependencies for CONFIG_FB > > > > When cleaning up other drm config dependencies, it is too easy to create > > larger problems. Instead, mark CONFIG_FB as a "depends": > > > > drivers/gpu/drm/Kconfig:74:error: recursive dependency detected! > > > > I compared the changes to the config (based on multi_v7_defconfig) with > > and without this patch and it shows a lot of changes. Is this intended? > > just one question: does the VC4 drm driver functionally depend on > CONFIG_FB / FRAMEBUFFER_CONSOLE ? No, just the fbdev emulation but it can work perfectly fine without it. > Or with other words should we re-enable CONFIG_FB like this [1] but for > Raspberry Pi related configs? > > [1] - > https://lore.kernel.org/linux-samsung-soc/20210611100204.6240-1-m.szyprow...@samsung.com/ It should work indeed Maxime signature.asc Description: PGP signature
Re: [PATCH v4 4/6] drm/sprd: add Unisoc's drm display controller driver
Hi,
Adding Jörg, Will and Robin,
On Wed, Mar 31, 2021 at 09:21:19AM +0800, Kevin Tang wrote:
> > > +static u32 check_mmu_isr(struct sprd_dpu *dpu, u32 reg_val)
> > > +{
> > > + struct dpu_context *ctx = &dpu->ctx;
> > > + u32 mmu_mask = BIT_DPU_INT_MMU_VAOR_RD |
> > > + BIT_DPU_INT_MMU_VAOR_WR |
> > > + BIT_DPU_INT_MMU_INV_RD |
> > > + BIT_DPU_INT_MMU_INV_WR;
> > > + u32 val = reg_val & mmu_mask;
> > > + int i;
> > > +
> > > + if (val) {
> > > + drm_err(dpu->drm, "--- iommu interrupt err: 0x%04x ---\n",
> > val);
> > > +
> > > + if (val & BIT_DPU_INT_MMU_INV_RD)
> > > + drm_err(dpu->drm, "iommu invalid read error, addr:
> > 0x%08x\n",
> > > + readl(ctx->base + REG_MMU_INV_ADDR_RD));
> > > + if (val & BIT_DPU_INT_MMU_INV_WR)
> > > + drm_err(dpu->drm, "iommu invalid write error,
> > addr: 0x%08x\n",
> > > + readl(ctx->base + REG_MMU_INV_ADDR_WR));
> > > + if (val & BIT_DPU_INT_MMU_VAOR_RD)
> > > + drm_err(dpu->drm, "iommu va out of range read
> > error, addr: 0x%08x\n",
> > > + readl(ctx->base + REG_MMU_VAOR_ADDR_RD));
> > > + if (val & BIT_DPU_INT_MMU_VAOR_WR)
> > > + drm_err(dpu->drm, "iommu va out of range write
> > error, addr: 0x%08x\n",
> > > + readl(ctx->base + REG_MMU_VAOR_ADDR_WR));
> >
> > Is that the IOMMU page fault interrupt? I would expect it to be in the
> > iommu driver.
>
> Our iommu driver is indeed an separate driver, and also in upstreaming,
> but iommu fault interrupts reporting by display controller, not iommu
> itself,
> if use iommu_set_fault_handler() to hook in our reporting function, there
> must be cross-module access to h/w registers.
Can you explain a bit more the design of the hardware then? Each device
connected to the IOMMU has a status register (and an interrupt) that
reports when there's a fault?
I'd like to get an ack at least from the IOMMU maintainers and
reviewers.
> > > +static void sprd_dpi_init(struct sprd_dpu *dpu)
> > > +{
> > > + struct dpu_context *ctx = &dpu->ctx;
> > > + u32 int_mask = 0;
> > > + u32 reg_val;
> > > +
> > > + if (ctx->if_type == SPRD_DPU_IF_DPI) {
> > > + /* use dpi as interface */
> > > + dpu_reg_clr(ctx, REG_DPU_CFG0, BIT_DPU_IF_EDPI);
> > > + /* disable Halt function for SPRD DSI */
> > > + dpu_reg_clr(ctx, REG_DPI_CTRL, BIT_DPU_DPI_HALT_EN);
> > > + /* select te from external pad */
> > > + dpu_reg_set(ctx, REG_DPI_CTRL,
> > BIT_DPU_EDPI_FROM_EXTERNAL_PAD);
> > > +
> > > + /* set dpi timing */
> > > + reg_val = ctx->vm.hsync_len << 0 |
> > > + ctx->vm.hback_porch << 8 |
> > > + ctx->vm.hfront_porch << 20;
> > > + writel(reg_val, ctx->base + REG_DPI_H_TIMING);
> > > +
> > > + reg_val = ctx->vm.vsync_len << 0 |
> > > + ctx->vm.vback_porch << 8 |
> > > + ctx->vm.vfront_porch << 20;
> > > + writel(reg_val, ctx->base + REG_DPI_V_TIMING);
> > > +
> > > + if (ctx->vm.vsync_len + ctx->vm.vback_porch < 32)
> > > + drm_warn(dpu->drm, "Warning: (vsync + vbp) < 32, "
> > > + "underflow risk!\n");
> >
> > I don't think a warning is appropriate here. Maybe we should just
> > outright reject any mode that uses it?
> >
> This issue has been fixed on the new soc, maybe I should remove it.
If it still requires a workaround on older SoC, you can definitely add
it but we should prevent any situation where the underflow might occur
instead of reporting it once we're there.
> > > +static enum drm_mode_status sprd_crtc_mode_valid(struct drm_crtc *crtc,
> > > + const struct drm_display_mode
> > *mode)
> > > +{
> > > + struct sprd_dpu *dpu = to_sprd_crtc(crtc);
> > > +
> > > + drm_dbg(dpu->drm, "%s() mode: "DRM_MODE_FMT"\n", __func__,
> > DRM_MODE_ARG(mode));
> > > +
> > > + if (mode->type & DRM_MODE_TYPE_PREFERRED) {
> > > + drm_display_mode_to_videomode(mode, &dpu->ctx.vm);
> >
> > You don't seem to use that anywhere else? And that's a bit fragile,
> > nothing really guarantees that it's the mode you're going to use, and
> > even then it can be adjusted.
> >
> drm_mode convert to video_mode is been use in "sprd_dpu_init" and
> "sprd_dpi_init "
> Preferred mode should be fixed mode, we generally don’t adjust it.
That's not really the assumption DRM is built upon though. The userspace
is even allowed to setup its own mode and try to configure it, and your
driver should take that into account.
I'd just drop that mode_valid hook, and retrieve the videomode if you
need it in atomi
Re: [PATCH v4 5/6] dt-bindings: display: add Unisoc's mipi dsi controller bindings
On Wed, Mar 31, 2021 at 09:49:14AM +0800, Kevin Tang wrote: > Hi Maxime, > > Maxime Ripard 于2021年3月24日周三 下午7:13写道： > > > On Mon, Feb 22, 2021 at 09:28:21PM +0800, Kevin Tang wrote: > > > From: Kevin Tang > > > > > > Adds MIPI DSI Controller > > > support for Unisoc's display subsystem. > > > > > > Cc: Orson Zhai > > > Cc: Chunyan Zhang > > > Signed-off-by: Kevin Tang > > > Reviewed-by: Rob Herring > > > --- > > > .../display/sprd/sprd,sharkl3-dsi-host.yaml | 102 ++ > > > 1 file changed, 102 insertions(+) > > > create mode 100644 > > Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > > > diff --git > > a/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > b/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > new file mode 100644 > > > index 0..d439f688f > > > --- /dev/null > > > +++ > > b/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > @@ -0,0 +1,102 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: > > http://devicetree.org/schemas/display/sprd/sprd,sharkl3-dsi-host.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Unisoc MIPI DSI Controller > > > + > > > +maintainers: > > > + - Kevin Tang > > > + > > > +properties: > > > + compatible: > > > +const: sprd,sharkl3-dsi-host > > > + > > > + reg: > > > +maxItems: 1 > > > + > > > + interrupts: > > > +maxItems: 2 > > > + > > > + clocks: > > > +minItems: 1 > > > + > > > + clock-names: > > > +items: > > > + - const: clk_src_96m > > > + > > > + power-domains: > > > +maxItems: 1 > > > + > > > + ports: > > > +type: object > > > + > > > +properties: > > > + "#address-cells": > > > +const: 1 > > > + > > > + "#size-cells": > > > +const: 0 > > > + > > > + port@0: > > > +type: object > > > +description: > > > + A port node with endpoint definitions as defined in > > > + Documentation/devicetree/bindings/media/video-interfaces.txt. > > > + That port should be the input endpoint, usually coming from > > > + the associated DPU. > > > + port@1: > > > +type: object > > > +description: > > > + A port node with endpoint definitions as defined in > > > + Documentation/devicetree/bindings/media/video-interfaces.txt. > > > + That port should be the output endpoint, usually output to > > > + the associated panel. > > > > The DSI generic binding asks that peripherals that are controlled > > through a DCS be a subnode of the MIPI-DSI bus, not through a port > > endpoint. > > > Our DSI controller don't support DCS now... I'm not sure I follow you, you mentionned in the patch 4 that you were testing for a device to be in command mode, how would that work without DCS support? Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 6/6] drm/sprd: add Unisoc's drm mipi dsi&dphy driver
On Wed, Mar 31, 2021 at 09:47:12AM +0800, Kevin Tang wrote: > > > diff --git a/drivers/gpu/drm/sprd/Makefile > > b/drivers/gpu/drm/sprd/Makefile > > > index 6c25bfa99..d49f4977b 100644 > > > --- a/drivers/gpu/drm/sprd/Makefile > > > +++ b/drivers/gpu/drm/sprd/Makefile > > > @@ -1,5 +1,8 @@ > > > # SPDX-License-Identifier: GPL-2.0 > > > > > > obj-y := sprd_drm.o \ > > > - sprd_dpu.o > > > - > > > + sprd_dpu.o \ > > > + sprd_dsi.o \ > > > + dw_dsi_ctrl.o \ > > > + dw_dsi_ctrl_ppi.o \ > > > > So it's a designware IP? There's a driver for it already that seems > > fairly similar: > > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > > > Our dw dsi controller is not a standard synopsys ip, we have updated a lot > on the basic ip version, > the entire control register is different, i have cc to drm/bridge reviewers > and maintainers. You should make it more obvious then in a comment or in the name of the driver. If it's fairly different from the original IP from Synopsys, maybe you should just drop the reference to the name? Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] drm: Generic dumb_map_offset for TTM-based drivers
On Tue, Apr 06, 2021 at 10:29:38AM +0200, Thomas Zimmermann wrote: > The implementation of drm_driver.dumb_map_offset is the same for several > TTM-based drivers. Provide a common function in GEM-TTM helpers. For the series: Acked-by: Maxime Ripard Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 08/10] drm/simpledrm: Acquire clocks from DT device node
Hi, On Thu, Mar 18, 2021 at 11:29:19AM +0100, Thomas Zimmermann wrote: > Make sure required hardware clocks are enabled while the firmware > framebuffer is in use. > > The basic code has been taken from the simplefb driver and adapted > to DRM. Clocks are released automatically via devres helpers. > > Signed-off-by: Thomas Zimmermann > Tested-by: nerdopolis Even though it's definitely simpler to review, merging the driver first and then the clocks and regulators will break bisection on the platforms that rely on them Another thing worth considering is also that both drivers will probe if they are enabled (which is pretty likely), which is not great :) I guess we should make them mutually exclusive through Kconfig Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 10/11] drm: Use state helper instead of the plane state pointer
Hi Stephen, On Tue, Mar 30, 2021 at 11:56:15AM -0700, Stephen Boyd wrote: > Quoting Maxime Ripard (2021-03-30 08:35:27) > > Hi Stephen, > > > > On Mon, Mar 29, 2021 at 06:52:01PM -0700, Stephen Boyd wrote: > > > Trimming Cc list way down, sorry if that's too much. > > > > > > Quoting Maxime Ripard (2021-02-19 04:00:30) > > > > Many drivers reference the plane->state pointer in order to get the > > > > current plane state in their atomic_update or atomic_disable hooks, > > > > which would be the new plane state in the global atomic state since > > > > _swap_state happened when those hooks are run. > > > > > > Does this mean drm_atomic_helper_swap_state()? > > > > Yep. Previous to that call in drm_atomic_helper_commit, plane->state is > > the state currently programmed in the hardware, so the old state (that's > > the case you have with atomic_check for example) > > > > Once drm_atomic_helper_swap_state has run, plane->state is now the state > > that needs to be programmed into the hardware, so the new state. > > Ok, and I suppose that is called by drm_atomic_helper_commit()? Yep :) > So presumably a modeset is causing this? I get the NULL pointer around > the time we switch from the splash screen to the login screen. I think > there's a modeset during that transition. It's very likely yeah. I really don't get how that pointer could be null though :/ Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next
Hi Dave, Daniel, Like you asked, here's this week drm-misc-next PR Maxime drm-misc-next-2021-04-09: drm-misc-next for 5.13: UAPI Changes: Cross-subsystem Changes: Core Changes: - bridge: Fix Kconfig dependency - cmdline: Refuse zero width/height mode - ttm: Ignore signaled move fences, ioremap buffer according to mem caching settins Driver Changes: - Conversions to sysfs_emit - tegra: Don't register DP AUX channels before connectors - zynqmp: Fix for an out-of-bound (but within struct padding) memset The following changes since commit 6c744983004ebc66756e582294672f8b991288d5: drm/bridge: anx7625: disable regulators when power off (2021-04-01 10:38:02 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2021-04-09 for you to fetch changes up to e8b8b0df8694e39ea6bbbdb9e2fcfa78a61e2e42: drm/panel: Convert sysfs sprintf/snprintf family to sysfs_emit (2021-04-08 20:41:38 -0400) drm-misc-next for 5.13: UAPI Changes: Cross-subsystem Changes: Core Changes: - bridge: Fix Kconfig dependency - cmdline: Refuse zero width/height mode - ttm: Ignore signaled move fences, ioremap buffer according to mem caching settins Driver Changes: - Conversions to sysfs_emit - tegra: Don't register DP AUX channels before connectors - zynqmp: Fix for an out-of-bound (but within struct padding) memset Carsten Haitzler (1): drm/komeda: Fix bit check to import to value of proper type Christian König (1): drm/sched: add missing member documentation Dafna Hirschfeld (1): drm/bridge: fix typo in Kconfig Dan Carpenter (1): drm: xlnx: zynqmp: fix a memset in zynqmp_dp_train() David Stevens (1): drm/syncobj: use newly allocated stub fences Felix Kuehling (1): drm/ttm: Ignore signaled move fences Guobin Huang (1): gma500: Use DEFINE_SPINLOCK() for spinlock Julian Braha (1): drivers: gpu: drm: bridge: fix kconfig dependency on DRM_KMS_HELPER Lyude Paul (4): drm/dp: Fixup kernel docs for struct drm_dp_aux drm/tegra: Don't register DP AUX channels before connectors drm/print: Fixup DRM_DEBUG_KMS_RATELIMITED() drm/dp_mst: Drop DRM_ERROR() on kzalloc() fail in drm_dp_mst_handle_up_req() Oak Zeng (1): drm/ttm: ioremap buffer according to TTM mem caching setting Tian Tao (2): drm/komeda: Convert sysfs sprintf/snprintf family to sysfs_emit drm/panel: Convert sysfs sprintf/snprintf family to sysfs_emit Ville Syrjälä (2): drm: Refuse to create zero width/height cmdline modes drm/vblank: Do not store a new vblank timestamp in drm_vblank_restore() Wan Jiabing (1): drm/drm_internal.h: Remove repeated struct declaration Zhang Jianhua (1): drm/bridge: lt8912b: Add header file drivers/dma-buf/dma-fence.c| 27 - drivers/gpu/drm/arm/display/include/malidp_utils.h | 3 -- drivers/gpu/drm/arm/display/komeda/komeda_dev.c| 6 +-- .../gpu/drm/arm/display/komeda/komeda_pipeline.c | 16 +--- .../drm/arm/display/komeda/komeda_pipeline_state.c | 19 ++ drivers/gpu/drm/bridge/Kconfig | 3 +- drivers/gpu/drm/bridge/lontium-lt8912b.c | 1 + drivers/gpu/drm/drm_dp_mst_topology.c | 5 +-- drivers/gpu/drm/drm_internal.h | 1 - drivers/gpu/drm/drm_modes.c| 3 ++ drivers/gpu/drm/drm_syncobj.c | 25 +--- drivers/gpu/drm/drm_vblank.c | 3 +- drivers/gpu/drm/gma500/power.c | 3 +- drivers/gpu/drm/panel/panel-tpo-td043mtea1.c | 4 +- drivers/gpu/drm/tegra/dpaux.c | 11 +++--- drivers/gpu/drm/ttm/ttm_bo.c | 3 +- drivers/gpu/drm/ttm/ttm_bo_util.c | 14 +++ drivers/gpu/drm/xlnx/zynqmp_dp.c | 2 +- include/drm/drm_dp_helper.h| 44 +++--- include/drm/drm_print.h| 20 ++ include/drm/gpu_scheduler.h| 1 + include/linux/dma-fence.h | 1 + 22 files changed, 142 insertions(+), 73 deletions(-) signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/5] drm/vc4: hdmi: Enable the scrambler
Hi Dave,
On Thu, Apr 01, 2021 at 12:30:45PM +0100, Dave Stevenson wrote:
> > Signed-off-by: Maxime Ripard
> > ---
> > drivers/gpu/drm/vc4/vc4_hdmi.c | 56 +
> > drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 ++
> > 2 files changed, 59 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > index 0924a1b9e186..530c83097b1a 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > @@ -35,6 +35,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -76,6 +77,8 @@
> > #define VC5_HDMI_VERTB_VSPO_SHIFT 16
> > #define VC5_HDMI_VERTB_VSPO_MASK VC4_MASK(29, 16)
> >
> > +#define VC5_HDMI_SCRAMBLER_CTL_ENABLE BIT(0)
> > +
> > #define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_SHIFT 8
> > #define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK
> > VC4_MASK(10, 8)
> >
> > @@ -457,6 +460,56 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
> > *encoder)
> > vc4_hdmi_set_audio_infoframe(encoder);
> > }
> >
> > +static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
> > +struct drm_display_mode *mode)
> > +{
> > + struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
> > + struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
> > + struct drm_display_info *display =
> > &vc4_hdmi->connector.display_info;
> > +
> > + if (!vc4_encoder->hdmi_monitor)
> > + return false;
> > +
> > + if (!display->hdmi.scdc.supported ||
> > + !display->hdmi.scdc.scrambling.supported)
> > + return false;
> > +
>
> I think I made this comment last time, but possibly not very clearly.
I must have missed it then, sorry :/
> Up to this point in the function is whether the display/hdmi interface
> *supports* scrambling. The bit after is whether it is *required* to be
> enabled by the mode.
Thomas made a suggestion to move the mode clock check to a helper, so
we'll end up with essentially a helper about whether we support
scrambling or not and if the mode requires it.
> At the moment, if the display/interface supports scrambling but the
> mode doesn't need it, then the scrambling setup is never touched. That
> makes a few assumptions about the default settings.
> Boot with the firmware selecting 4k60 (ie scrambling on), but
> video=HDMI-1:1920x1080 in the kernel command line and you'll have a
> mess with scrambling enabled but not signalled.
>
> I'd be happier if the display/interface says scrambling is supported
> then we always call drm_scdc_set_high_tmds_clock_ratio,
> drm_scdc_set_scrambling and set the VC5_HDMI_SCRAMBLER_CTL_ENABLE
> register bit appropriately for the mode. Feel free to disagree with me
> though.
I think part of it is due to our custom helpers never being called
currently during the boot process. Once that is fixed, the disable
helpers will be called and will disable the scrambling so we should be
good there.
This creates another issue though. That function takes the mode as the
argument and at boot time the mode pointer will be null. I think we can
work around it by assuming that we need to disable it at boot all the
time (and thus ignore the test if our pointer is null).
Would that work for you?
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/vc4: hdmi: Convert to the new clock request API
The new clock request API allows us to increase the rate of the HSM
clock to match our pixel rate requirements while decreasing it when
we're done, resulting in a better power-efficiency.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 19 ---
drivers/gpu/drm/vc4/vc4_hdmi.h | 3 +++
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 1fda574579af..244053de6150 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -473,7 +473,9 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct
drm_encoder *encoder,
HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE);
clk_disable_unprepare(vc4_hdmi->pixel_bvb_clock);
+ clk_request_done(vc4_hdmi->bvb_req);
clk_disable_unprepare(vc4_hdmi->hsm_clock);
+ clk_request_done(vc4_hdmi->hsm_req);
clk_disable_unprepare(vc4_hdmi->pixel_clock);
ret = pm_runtime_put(&vc4_hdmi->pdev->dev);
@@ -778,9 +780,9 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct
drm_encoder *encoder,
* pixel clock, but HSM ends up being the limiting factor.
*/
hsm_rate = max_t(unsigned long, 12000, (pixel_rate / 100) * 101);
- ret = clk_set_min_rate(vc4_hdmi->hsm_clock, hsm_rate);
- if (ret) {
- DRM_ERROR("Failed to set HSM clock rate: %d\n", ret);
+ vc4_hdmi->hsm_req = clk_request_start(vc4_hdmi->hsm_clock, hsm_rate);
+ if (IS_ERR(vc4_hdmi->hsm_req)) {
+ DRM_ERROR("Failed to set HSM clock rate: %ld\n",
PTR_ERR(vc4_hdmi->hsm_req));
return;
}
@@ -797,10 +799,11 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct
drm_encoder *encoder,
* FIXME: When the pixel freq is 594MHz (4k60), this needs to be setup
* at 300MHz.
*/
- ret = clk_set_min_rate(vc4_hdmi->pixel_bvb_clock,
- (hsm_rate > VC4_HSM_MID_CLOCK ? 15000 :
7500));
- if (ret) {
- DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret);
+ vc4_hdmi->bvb_req = clk_request_start(vc4_hdmi->pixel_bvb_clock,
+ (hsm_rate > VC4_HSM_MID_CLOCK ?
15000 : 7500));
+ if (IS_ERR(vc4_hdmi->bvb_req)) {
+ DRM_ERROR("Failed to set pixel bvb clock rate: %ld\n",
PTR_ERR(vc4_hdmi->bvb_req));
+ clk_request_done(vc4_hdmi->hsm_req);
clk_disable_unprepare(vc4_hdmi->hsm_clock);
clk_disable_unprepare(vc4_hdmi->pixel_clock);
return;
@@ -809,6 +812,8 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct
drm_encoder *encoder,
ret = clk_prepare_enable(vc4_hdmi->pixel_bvb_clock);
if (ret) {
DRM_ERROR("Failed to turn on pixel bvb clock: %d\n", ret);
+ clk_request_done(vc4_hdmi->bvb_req);
+ clk_request_done(vc4_hdmi->hsm_req);
clk_disable_unprepare(vc4_hdmi->hsm_clock);
clk_disable_unprepare(vc4_hdmi->pixel_clock);
return;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 3cebd1fd00fc..9ac4a2c751df 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -167,6 +167,9 @@ struct vc4_hdmi {
struct reset_control *reset;
+ struct clk_request *bvb_req;
+ struct clk_request *hsm_req;
+
struct debugfs_regset32 hdmi_regset;
struct debugfs_regset32 hd_regset;
};
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] clk: Introduce a clock request API
It's not unusual to find clocks being shared across multiple devices
that need to change the rate depending on what the device is doing at a
given time.
The SoC found on the RaspberryPi4 (BCM2711) is in such a situation
between its two HDMI controllers that share a clock that needs to be
raised depending on the output resolution of each controller.
The current clk_set_rate API doesn't really allow to support that case
since there's really no synchronisation between multiple users, it's
essentially a fire-and-forget solution.
clk_set_min_rate does allow for such a synchronisation, but has another
drawback: it doesn't allow to reduce the clock rate once the work is
over.
In our previous example, this means that if we were to raise the
resolution of one HDMI controller to the largest resolution and then
changing for a smaller one, we would still have the clock running at the
largest resolution rate resulting in a poor power-efficiency.
In order to address both issues, let's create an API that allows user to
create temporary requests to increase the rate to a minimum, before
going back to the initial rate once the request is done.
This introduces mainly two side-effects:
* There's an interaction between clk_set_rate and requests. This has
been addressed by having clk_set_rate increasing the rate if it's
greater than what the requests asked for, and in any case changing
the rate the clock will return to once all the requests are done.
* Similarly, clk_round_rate has been adjusted to take the requests
into account and return a rate that will be greater or equal to the
requested rates.
Signed-off-by: Maxime Ripard
---
drivers/clk/clk.c | 121
include/linux/clk.h | 4 ++
2 files changed, 125 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 5052541a0986..4fd91a57e6db 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -77,12 +77,14 @@ struct clk_core {
unsigned intprotect_count;
unsigned long min_rate;
unsigned long max_rate;
+ unsigned long default_request_rate;
unsigned long accuracy;
int phase;
struct clk_duty duty;
struct hlist_head children;
struct hlist_node child_node;
struct hlist_head clks;
+ struct list_headpending_requests;
unsigned intnotifier_count;
#ifdef CONFIG_DEBUG_FS
struct dentry *dentry;
@@ -105,6 +107,12 @@ struct clk {
struct hlist_node clks_node;
};
+struct clk_request {
+ struct list_head list;
+ struct clk *clk;
+ unsigned long rate;
+};
+
/*** runtime pm ***/
static int clk_pm_runtime_get(struct clk_core *core)
{
@@ -1434,10 +1442,14 @@ unsigned long clk_hw_round_rate(struct clk_hw *hw,
unsigned long rate)
{
int ret;
struct clk_rate_request req;
+ struct clk_request *clk_req;
clk_core_get_boundaries(hw->core, &req.min_rate, &req.max_rate);
req.rate = rate;
+ list_for_each_entry(clk_req, &hw->core->pending_requests, list)
+ req.min_rate = max(clk_req->rate, req.min_rate);
+
ret = clk_core_round_rate_nolock(hw->core, &req);
if (ret)
return 0;
@@ -1458,6 +1470,7 @@ EXPORT_SYMBOL_GPL(clk_hw_round_rate);
long clk_round_rate(struct clk *clk, unsigned long rate)
{
struct clk_rate_request req;
+ struct clk_request *clk_req;
int ret;
if (!clk)
@@ -1471,6 +1484,9 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
clk_core_get_boundaries(clk->core, &req.min_rate, &req.max_rate);
req.rate = rate;
+ list_for_each_entry(clk_req, &clk->core->pending_requests, list)
+ req.min_rate = max(clk_req->rate, req.min_rate);
+
ret = clk_core_round_rate_nolock(clk->core, &req);
if (clk->exclusive_count)
@@ -1938,6 +1954,7 @@ static struct clk_core *clk_calc_new_rates(struct
clk_core *core,
unsigned long new_rate;
unsigned long min_rate;
unsigned long max_rate;
+ struct clk_request *req;
int p_index = 0;
long ret;
@@ -1952,6 +1969,9 @@ static struct clk_core *clk_calc_new_rates(struct
clk_core *core,
clk_core_get_boundaries(core, &min_rate, &max_rate);
+ list_for_each_entry(req, &core->pending_requests, list)
+ min_rate = max(req->rate, min_rate);
+
/* find the closest rate and parent clk/rate */
if (clk_core_can_round(core)) {
struct clk_rate_request req;
@@ -2156,6 +2176,7 @@ static unsigned long
clk_core_req_round_rate_nolock(struct clk_core *core,
{
int ret, cnt;
struct clk_rate_req
[PATCH 0/2] clk: Implement a clock request API
Hi, This is a follow-up of the discussion here: https://lore.kernel.org/linux-clk/20210319150355.xzw7ikwdaga2dwhv@gilmour/ This implements a mechanism to raise and lower clock rates based on consumer workloads, with an example of such an implementation for the RaspberryPi4 HDMI controller. There's a couple of things worth discussing: - The name is in conflict with clk_request_rate, and even though it feels like the right name to me, we should probably avoid any confusion - The code so far implements a policy of always going for the lowest rate possible. While we don't have an use-case for something else, this should maybe be made more flexible? Let me know what you think Maxime Maxime Ripard (2): clk: Introduce a clock request API drm/vc4: hdmi: Convert to the new clock request API drivers/clk/clk.c | 121 + drivers/gpu/drm/vc4/vc4_hdmi.c | 19 -- drivers/gpu/drm/vc4/vc4_hdmi.h | 3 + include/linux/clk.h| 4 ++ 4 files changed, 140 insertions(+), 7 deletions(-) -- 2.30.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/5] drm/connector: Add helper to compare HDR metadata
All the drivers that support the HDR metadata property have a similar
function to compare the metadata from one connector state to the next,
and force a mode change if they differ.
All these functions run pretty much the same code, so let's turn it into
an helper that can be shared across those drivers.
Reviewed-by: Harry Wentland
Reviewed-by: Jernej Skrabec
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Rebased on latest drm-misc-next tag
- Added the tags
- Fix build breakage on amdgpu
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++-
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +--
drivers/gpu/drm/drm_connector.c | 28 +++
drivers/gpu/drm/i915/display/intel_atomic.c | 13 +
include/drm/drm_connector.h | 2 ++
5 files changed, 34 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 1e22ce718010..ed1972fb531d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5967,25 +5967,6 @@ static int fill_hdr_info_packet(const struct
drm_connector_state *state,
return 0;
}
-static bool
-is_hdr_metadata_different(const struct drm_connector_state *old_state,
- const struct drm_connector_state *new_state)
-{
- struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
- struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
-
- if (old_blob != new_blob) {
- if (old_blob && new_blob &&
- old_blob->length == new_blob->length)
- return memcmp(old_blob->data, new_blob->data,
- old_blob->length);
-
- return true;
- }
-
- return false;
-}
-
static int
amdgpu_dm_connector_atomic_check(struct drm_connector *conn,
struct drm_atomic_state *state)
@@ -6003,7 +5984,7 @@ amdgpu_dm_connector_atomic_check(struct drm_connector
*conn,
if (!crtc)
return 0;
- if (is_hdr_metadata_different(old_con_state, new_con_state)) {
+ if (!drm_connector_atomic_hdr_metadata_equal(old_con_state,
new_con_state)) {
struct dc_info_packet hdr_infopacket;
ret = fill_hdr_info_packet(new_con_state, &hdr_infopacket);
@@ -8357,7 +8338,7 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
dm_old_crtc_state->abm_level;
hdr_changed =
- is_hdr_metadata_different(old_con_state, new_con_state);
+ !drm_connector_atomic_hdr_metadata_equal(old_con_state,
new_con_state);
if (!scaling_changed && !abm_changed && !hdr_changed)
continue;
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index f24bbb840dbf..f871e33c2fc9 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2395,21 +2395,6 @@ static int dw_hdmi_connector_get_modes(struct
drm_connector *connector)
return ret;
}
-static bool hdr_metadata_equal(const struct drm_connector_state *old_state,
- const struct drm_connector_state *new_state)
-{
- struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
- struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
-
- if (!old_blob || !new_blob)
- return old_blob == new_blob;
-
- if (old_blob->length != new_blob->length)
- return false;
-
- return !memcmp(old_blob->data, new_blob->data, old_blob->length);
-}
-
static int dw_hdmi_connector_atomic_check(struct drm_connector *connector,
struct drm_atomic_state *state)
{
@@ -2423,7 +2408,7 @@ static int dw_hdmi_connector_atomic_check(struct
drm_connector *connector,
if (!crtc)
return 0;
- if (!hdr_metadata_equal(old_state, new_state)) {
+ if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
crtc_state = drm_atomic_get_crtc_state(state, crtc);
if (IS_ERR(crtc_state))
return PTR_ERR(crtc_state);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index a4aa2d87af35..b13021b1b128 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2171,6 +2171,34 @@ int
drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
}
EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+/**
+ * drm_connector_atomic_hdr_metadata_equal - checks if the h
[PATCH v2 4/5] drm/connector: Add a helper to attach the colorspace property
The intel driver uses the same logic to attach the Colorspace property
in multiple places and we'll need it in vc4 too. Let's move that common
code in a helper.
Signed-off-by: Maxime Ripard
---
Changes from v1:
- New patch
---
drivers/gpu/drm/drm_connector.c | 20 +++
.../gpu/drm/i915/display/intel_connector.c| 6 ++
include/drm/drm_connector.h | 1 +
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b13021b1b128..6a20b249e533 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2171,6 +2171,26 @@ int
drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
}
EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+/**
+ * drm_connector_attach_colorspace_property - attach "Colorspace" property
+ * @connector: connector to attach the property on.
+ *
+ * This is used to allow the userspace to signal the output colorspace
+ * to the driver.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_colorspace_property(struct drm_connector *connector)
+{
+ struct drm_property *prop = connector->colorspace_property;
+
+ drm_object_attach_property(&connector->base, prop,
DRM_MODE_COLORIMETRY_DEFAULT);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_colorspace_property);
+
/**
* drm_connector_atomic_hdr_metadata_equal - checks if the hdr metadata changed
* @old_state: old connector state to compare
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index d5ceb7bdc14b..9bed1ccecea0 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -282,14 +282,12 @@ void
intel_attach_hdmi_colorspace_property(struct drm_connector *connector)
{
if (!drm_mode_create_hdmi_colorspace_property(connector))
- drm_object_attach_property(&connector->base,
- connector->colorspace_property, 0);
+ drm_connector_attach_colorspace_property(connector);
}
void
intel_attach_dp_colorspace_property(struct drm_connector *connector)
{
if (!drm_mode_create_dp_colorspace_property(connector))
- drm_object_attach_property(&connector->base,
- connector->colorspace_property, 0);
+ drm_connector_attach_colorspace_property(connector);
}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 1f51d73ca715..714d1a01c065 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1671,6 +1671,7 @@ int drm_connector_attach_scaling_mode_property(struct
drm_connector *connector,
u32 scaling_mode_mask);
int drm_connector_attach_vrr_capable_property(
struct drm_connector *connector);
+int drm_connector_attach_colorspace_property(struct drm_connector *connector);
int drm_connector_attach_hdr_output_metadata_property(struct drm_connector
*connector);
bool drm_connector_atomic_hdr_metadata_equal(struct drm_connector_state
*old_state,
struct drm_connector_state
*new_state);
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/5] drm/connector: Create a helper to attach the hdr_output_metadata property
All the drivers that implement HDR output call pretty much the same
function to initialise the hdr_output_metadata property, and while the
creation of that property is in a helper, every driver uses the same
code to attach it.
Provide a helper for it as well
Reviewed-by: Harry Wentland
Reviewed-by: Jernej Skrabec
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Rebased on latest drm-misc-next tag
- Added the tags
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 +--
drivers/gpu/drm/drm_connector.c | 21 +++
drivers/gpu/drm/i915/display/intel_hdmi.c | 3 +--
include/drm/drm_connector.h | 1 +
5 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 55e39b462a5e..1e22ce718010 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7078,9 +7078,7 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
connector_type == DRM_MODE_CONNECTOR_eDP) {
- drm_object_attach_property(
- &aconnector->base.base,
- dm->ddev->mode_config.hdr_output_metadata_property, 0);
+
drm_connector_attach_hdr_output_metadata_property(&aconnector->base);
if (!aconnector->mst_port)
drm_connector_attach_vrr_capable_property(&aconnector->base);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index dda4fa9a1a08..f24bbb840dbf 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2492,8 +2492,7 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
drm_connector_attach_max_bpc_property(connector, 8, 16);
if (hdmi->version >= 0x200a && hdmi->plat_data->use_drm_infoframe)
- drm_object_attach_property(&connector->base,
-
connector->dev->mode_config.hdr_output_metadata_property, 0);
+ drm_connector_attach_hdr_output_metadata_property(connector);
drm_connector_attach_encoder(connector, hdmi->bridge.encoder);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 7631f76e7f34..a4aa2d87af35 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2150,6 +2150,27 @@ int drm_connector_attach_max_bpc_property(struct
drm_connector *connector,
}
EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
+/**
+ * drm_connector_attach_hdr_output_metadata_property - attach
"HDR_OUTPUT_METADA" property
+ * @connector: connector to attach the property on.
+ *
+ * This is used to allow the userspace to send HDR Metadata to the
+ * driver.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_hdr_output_metadata_property(struct drm_connector
*connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_property *prop =
dev->mode_config.hdr_output_metadata_property;
+
+ drm_object_attach_property(&connector->base, prop, 0);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+
/**
* drm_connector_set_vrr_capable_property - sets the variable refresh rate
* capable property for a connector
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 95919d325b0b..f2f1b025e6ba 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2965,8 +2965,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi,
struct drm_connector *c
drm_connector_attach_content_type_property(connector);
if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
- drm_object_attach_property(&connector->base,
-
connector->dev->mode_config.hdr_output_metadata_property, 0);
+ drm_connector_attach_hdr_output_metadata_property(connector);
if (!HAS_GMCH(dev_priv))
drm_connector_attach_max_bpc_property(connector, 8, 12);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 1922b278ffad..32172dab8427 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1671,6 +1671,7 @@ int drm_connector_attach_scaling_mode_property(struct
drm_connector *connector,
u32 scaling_mode_mask);
int drm_connector_attach_vrr_capable_property(
struct drm_connector *c
[PATCH v2 5/5] drm/vc4: hdmi: Signal the proper colorimetry info in the infoframe
Our driver while supporting HDR didn't send the proper colorimetry info
in the AVI infoframe.
Let's add the property needed so that the userspace can let us know what
the colorspace is supposed to be.
Signed-off-by: Maxime Ripard
---
Changes from v1:
- New patch
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index a33fa1662588..a22e17788074 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -226,7 +226,8 @@ static int vc4_hdmi_connector_atomic_check(struct
drm_connector *connector,
if (!crtc)
return 0;
- if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
+ if (old_state->colorspace != new_state->colorspace ||
+ !drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
struct drm_crtc_state *crtc_state;
crtc_state = drm_atomic_get_crtc_state(state, crtc);
@@ -316,6 +317,11 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
if (ret)
return ret;
+ ret = drm_mode_create_hdmi_colorspace_property(connector);
+ if (ret)
+ return ret;
+
+ drm_connector_attach_colorspace_property(connector);
drm_connector_attach_tv_margin_properties(connector);
drm_connector_attach_max_bpc_property(connector, 8, 12);
@@ -424,7 +430,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder
*encoder)
vc4_encoder->limited_rgb_range ?
HDMI_QUANTIZATION_RANGE_LIMITED :
HDMI_QUANTIZATION_RANGE_FULL);
-
+ drm_hdmi_avi_infoframe_colorspace(&frame.avi, cstate);
drm_hdmi_avi_infoframe_bars(&frame.avi, cstate);
vc4_hdmi_write_infoframe(encoder, &frame);
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/5] drm/vc4: Add HDR metadata property to the VC5 HDMI connectors
From: Dave Stevenson
Now that we can export deeper colour depths, add in the signalling
for HDR metadata.
Signed-off-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Rebased on latest drm-misc-next tag
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 53 ++
drivers/gpu/drm/vc4/vc4_hdmi.h | 3 ++
2 files changed, 56 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 1fda574579af..a33fa1662588 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -214,6 +214,31 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
return ret;
}
+static int vc4_hdmi_connector_atomic_check(struct drm_connector *connector,
+ struct drm_atomic_state *state)
+{
+ struct drm_connector_state *old_state =
+ drm_atomic_get_old_connector_state(state, connector);
+ struct drm_connector_state *new_state =
+ drm_atomic_get_new_connector_state(state, connector);
+ struct drm_crtc *crtc = new_state->crtc;
+
+ if (!crtc)
+ return 0;
+
+ if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
+ struct drm_crtc_state *crtc_state;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ crtc_state->mode_changed = true;
+ }
+
+ return 0;
+}
+
static void vc4_hdmi_connector_reset(struct drm_connector *connector)
{
struct vc4_hdmi_connector_state *old_state =
@@ -263,6 +288,7 @@ static const struct drm_connector_funcs
vc4_hdmi_connector_funcs = {
static const struct drm_connector_helper_funcs vc4_hdmi_connector_helper_funcs
= {
.get_modes = vc4_hdmi_connector_get_modes,
+ .atomic_check = vc4_hdmi_connector_atomic_check,
};
static int vc4_hdmi_connector_init(struct drm_device *dev,
@@ -299,6 +325,9 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
connector->interlace_allowed = 1;
connector->doublescan_allowed = 0;
+ if (vc4_hdmi->variant->supports_hdr)
+ drm_connector_attach_hdr_output_metadata_property(connector);
+
drm_connector_attach_encoder(connector, encoder);
return 0;
@@ -432,6 +461,25 @@ static void vc4_hdmi_set_audio_infoframe(struct
drm_encoder *encoder)
vc4_hdmi_write_infoframe(encoder, &frame);
}
+static void vc4_hdmi_set_hdr_infoframe(struct drm_encoder *encoder)
+{
+ struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct drm_connector *connector = &vc4_hdmi->connector;
+ struct drm_connector_state *conn_state = connector->state;
+ union hdmi_infoframe frame;
+
+ if (!vc4_hdmi->variant->supports_hdr)
+ return;
+
+ if (!conn_state->hdr_output_metadata)
+ return;
+
+ if (drm_hdmi_infoframe_set_hdr_metadata(&frame.drm, conn_state))
+ return;
+
+ vc4_hdmi_write_infoframe(encoder, &frame);
+}
+
static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
@@ -444,6 +492,8 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
*/
if (vc4_hdmi->audio.streaming)
vc4_hdmi_set_audio_infoframe(encoder);
+
+ vc4_hdmi_set_hdr_infoframe(encoder);
}
static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
@@ -2102,6 +2152,7 @@ static const struct vc4_hdmi_variant bcm2835_variant = {
.phy_rng_enable = vc4_hdmi_phy_rng_enable,
.phy_rng_disable= vc4_hdmi_phy_rng_disable,
.channel_map= vc4_hdmi_channel_map,
+ .supports_hdr = false,
};
static const struct vc4_hdmi_variant bcm2711_hdmi0_variant = {
@@ -2129,6 +2180,7 @@ static const struct vc4_hdmi_variant
bcm2711_hdmi0_variant = {
.phy_rng_enable = vc5_hdmi_phy_rng_enable,
.phy_rng_disable= vc5_hdmi_phy_rng_disable,
.channel_map= vc5_hdmi_channel_map,
+ .supports_hdr = true,
};
static const struct vc4_hdmi_variant bcm2711_hdmi1_variant = {
@@ -2156,6 +2208,7 @@ static const struct vc4_hdmi_variant
bcm2711_hdmi1_variant = {
.phy_rng_enable = vc5_hdmi_phy_rng_enable,
.phy_rng_disable= vc5_hdmi_phy_rng_disable,
.channel_map= vc5_hdmi_channel_map,
+ .supports_hdr = true,
};
static const struct of_device_id vc4_hdmi_dt_match[] = {
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 3cebd1fd00fc..060bcaefbeb5 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -99,6 +99,9 @@ struct vc4_hdm
[PATCH v3 0/9] drm/vc4: hdmi: Support the 4k @ 60Hz modes
Hi, Here is a series that enables the higher resolutions on the HDMI0 Controller found in the BCM2711 (RPi4). In order to work it needs a few adjustments to config.txt, most notably to enable the enable_hdmi_4kp60 option. The firmware also has a glitch at the moment and will not properly release the BSC controllers, which will make the EDID retrieval fail. We can work around this using the following config.txt options: disable_fw_kms_setup=1 hdmi_edid_file:0=1 hdmi_edid_filename:0=1366x768.bin hdmi_ignore_edid:0=1 hdmi_edid_file:1=1 hdmi_edid_filename:1=1366x768.bin hdmi_ignore_edid:1=1 A fix will come for the firmware eventually. Let me know what you think, Maxime --- Changes from v2: - Gathered the various tags - Added Cc stable when relevant - Split out the check to test whether the scrambler is required into an helper - Fixed a bug where the scrambler state wouldn't be tracked properly if it was enabled at boot Changes from v1: - Dropped the range accessors - Drop the mention of force_turbo - Reordered the SCRAMBLER_CTL register to match the offset - Removed duplicate HDMI_14_MAX_TMDS_CLK define - Warn about enable_hdmi_4kp60 only if there's some modes that can't be reached - Rework the BVB clock computation Maxime Ripard (9): drm/vc4: txp: Properly set the possible_crtcs mask drm/vc4: crtc: Skip the TXP drm/vc4: Rework the encoder retrieval code drm/vc4: hdmi: Prevent clock unbalance drm/vc4: hvs: Make the HVS bind first drm/vc4: hdmi: Properly compute the BVB clock rate drm/vc4: hdmi: Check and warn if we can't reach 4kp60 frequencies drm/vc4: hdmi: Enable the scrambler drm/vc4: hdmi: Raise the maximum clock rate drivers/gpu/drm/vc4/vc4_crtc.c | 28 ++- drivers/gpu/drm/vc4/vc4_drv.c | 47 ++- drivers/gpu/drm/vc4/vc4_drv.h | 10 +++ drivers/gpu/drm/vc4/vc4_hdmi.c | 122 ++-- drivers/gpu/drm/vc4/vc4_hdmi.h | 8 ++ drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 + drivers/gpu/drm/vc4/vc4_txp.c | 2 +- 7 files changed, 207 insertions(+), 13 deletions(-) -- 2.30.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/9] drm/vc4: txp: Properly set the possible_crtcs mask
The current code does a binary OR on the possible_crtcs variable of the
TXP encoder, while we want to set it to that value instead.
Cc: # v5.9+
Fixes: 39fcb2808376 ("drm/vc4: txp: Turn the TXP into a CRTC of its own")
Acked-by: Thomas Zimmermann
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_txp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index c0122d83b651..2fc7f4b5fa09 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -507,7 +507,7 @@ static int vc4_txp_bind(struct device *dev, struct device
*master, void *data)
return ret;
encoder = &txp->connector.encoder;
- encoder->possible_crtcs |= drm_crtc_mask(crtc);
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
ret = devm_request_irq(dev, irq, vc4_txp_interrupt, 0,
dev_name(dev), txp);
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 4/9] drm/vc4: hdmi: Prevent clock unbalance
Since we fixed the hooks to disable the encoder at boot, we now have an
unbalanced clk_disable call at boot since we never enabled them in the
first place.
Let's mimic the state of the hardware and enable the clocks at boot if
the controller is enabled to get the use-count right.
Cc: # v5.10+
Fixes: 09c438139b8f ("drm/vc4: hdmi: Implement finer-grained hooks")
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 1fda574579af..9c919472ae84 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1995,6 +1995,14 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
if (vc4_hdmi->variant->reset)
vc4_hdmi->variant->reset(vc4_hdmi);
+ if ((of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi0") ||
+of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi1")) &&
+ HDMI_READ(HDMI_VID_CTL) & VC4_HD_VID_CTL_ENABLE) {
+ clk_prepare_enable(vc4_hdmi->pixel_clock);
+ clk_prepare_enable(vc4_hdmi->hsm_clock);
+ clk_prepare_enable(vc4_hdmi->pixel_bvb_clock);
+ }
+
pm_runtime_enable(dev);
drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 5/9] drm/vc4: hvs: Make the HVS bind first
We'll need to have the HVS binding before the HDMI controllers so that
we can check whether the firmware allows to run in 4kp60. Reorder a bit
the component list, and document the current constraints we're aware of.
Acked-by: Dave Stevenson
Acked-by: Thomas Zimmermann
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_drv.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index cd1fb75c66a7..fef31d6eea60 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -339,12 +339,21 @@ static const struct component_master_ops vc4_drm_ops = {
.unbind = vc4_drm_unbind,
};
+/*
+ * This list determines the binding order of our components, and we have
+ * a few constraints:
+ * - The TXP driver needs to be bound before the PixelValves (CRTC)
+ * but after the HVS to set the possible_crtc field properly
+ * - The HDMI driver needs to be bound after the HVS so that we can
+ * lookup the HVS maximum core clock rate and figure out if we
+ * support 4kp60 or not.
+ */
static struct platform_driver *const component_drivers[] = {
+ &vc4_hvs_driver,
&vc4_hdmi_driver,
&vc4_vec_driver,
&vc4_dpi_driver,
&vc4_dsi_driver,
- &vc4_hvs_driver,
&vc4_txp_driver,
&vc4_crtc_driver,
&vc4_v3d_driver,
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/9] drm/vc4: Rework the encoder retrieval code
Due to a FIFO that cannot be flushed between the pixelvalve and the HDMI
controllers on BCM2711, we need to carefully disable both at boot time
if they were left enabled by the firmware.
However, at the time we're running that code, the struct drm_connector
encoder pointer isn't set yet, and thus we cannot retrieve the encoder
associated to our CRTC.
We can however make use of the fact that we have a less flexible setup
than what DRM allows where we have a 1:1 relationship between our CRTCs
and encoders (and connectors), and thus store the crtc associated to our
encoder at boot time.
We cannot do that at the time the encoders are probed though, since the
CRTCs won't be probed yet and thus we don't know at that time which CRTC
index we're going to get, so let's do this in two passes: we can first
bind all the components and then once they all are bound, we can iterate
over all the encoders to find their associated CRTC and set the pointer.
This is similar to what we're doing to set the possible_crtcs field.
Fixes: 875a4d536842 ("drm/vc4: drv: Disable the CRTC at boot time")
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_crtc.c | 25 +--
drivers/gpu/drm/vc4/vc4_drv.c | 36 ++
drivers/gpu/drm/vc4/vc4_drv.h | 10 ++
3 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index f1f2e8cbce79..2bcd7c4e0fc7 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -255,6 +255,19 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct
vc4_crtc *vc4_crtc,
PV_CONTROL_FIFO_LEVEL);
}
+static struct drm_encoder *vc4_get_connector_encoder(struct drm_connector
*connector)
+{
+ struct drm_encoder *encoder;
+
+ if (drm_WARN_ON(connector->dev, hweight32(connector->possible_encoders)
!= 1))
+ return NULL;
+
+ drm_connector_for_each_possible_encoder(connector, encoder)
+ return encoder;
+
+ return NULL;
+}
+
/*
* Returns the encoder attached to the CRTC.
*
@@ -269,9 +282,17 @@ static struct drm_encoder *vc4_get_crtc_encoder(struct
drm_crtc *crtc)
drm_connector_list_iter_begin(crtc->dev, &conn_iter);
drm_for_each_connector_iter(connector, &conn_iter) {
- if (connector->state->crtc == crtc) {
+ struct drm_encoder *encoder;
+ struct vc4_encoder *vc4_encoder;
+
+ encoder = vc4_get_connector_encoder(connector);
+ if (!encoder)
+ continue;
+
+ vc4_encoder = to_vc4_encoder(encoder);
+ if (vc4_encoder->crtc == crtc) {
drm_connector_list_iter_end(&conn_iter);
- return connector->encoder;
+ return encoder;
}
}
drm_connector_list_iter_end(&conn_iter);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 556ad0f02a0d..cd1fb75c66a7 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -199,6 +199,41 @@ static int compare_dev(struct device *dev, void *data)
return dev == data;
}
+static struct drm_crtc *vc4_drv_find_crtc(struct drm_device *drm,
+ struct drm_encoder *encoder)
+{
+ struct drm_crtc *crtc;
+
+ if (WARN_ON(hweight32(encoder->possible_crtcs) != 1))
+ return NULL;
+
+ drm_for_each_crtc(crtc, drm) {
+ if (!drm_encoder_crtc_ok(encoder, crtc))
+ continue;
+
+ return crtc;
+ }
+
+ return NULL;
+}
+
+static void vc4_drv_set_encoder_data(struct drm_device *drm)
+{
+ struct drm_encoder *encoder;
+
+ drm_for_each_encoder(encoder, drm) {
+ struct vc4_encoder *vc4_encoder;
+ struct drm_crtc *crtc;
+
+ crtc = vc4_drv_find_crtc(drm, encoder);
+ if (WARN_ON(!crtc))
+ return;
+
+ vc4_encoder = to_vc4_encoder(encoder);
+ vc4_encoder->crtc = crtc;
+ }
+}
+
static void vc4_match_add_drivers(struct device *dev,
struct component_match **match,
struct platform_driver *const *drivers,
@@ -261,6 +296,7 @@ static int vc4_drm_bind(struct device *dev)
ret = component_bind_all(dev, drm);
if (ret)
return ret;
+ vc4_drv_set_encoder_data(drm);
ret = vc4_plane_create_additional_planes(drm);
if (ret)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index a7500716cf3f..a22e49665d9c 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -438,6 +438,16 @@ enum vc4_encoder_type {
[PATCH v3 9/9] drm/vc4: hdmi: Raise the maximum clock rate
Now that we have the infrastructure in place, we can raise the maximum
pixel rate we can reach for HDMI0 on the BCM2711.
HDMI1 is left untouched since its pixelvalve has a smaller FIFO and
would need a clock faster than what we can provide to support the same
modes.
Acked-by: Thomas Zimmermann
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index bda12fea0dce..aecda5766a9c 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2212,7 +2212,7 @@ static const struct vc4_hdmi_variant
bcm2711_hdmi0_variant = {
.encoder_type = VC4_ENCODER_TYPE_HDMI0,
.debugfs_name = "hdmi0_regs",
.card_name = "vc4-hdmi-0",
- .max_pixel_clock= HDMI_14_MAX_TMDS_CLK,
+ .max_pixel_clock= 6,
.registers = vc5_hdmi_hdmi0_fields,
.num_registers = ARRAY_SIZE(vc5_hdmi_hdmi0_fields),
.phy_lane_mapping = {
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 6/9] drm/vc4: hdmi: Properly compute the BVB clock rate
The BVB clock rate computation doesn't take into account a mode clock of
594MHz that we're going to need to support 4k60.
Acked-by: Thomas Zimmermann
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 9c919472ae84..c50dc5a59b2f 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -91,7 +91,6 @@
# define VC4_HD_M_ENABLE BIT(0)
#define CEC_CLOCK_FREQ 4
-#define VC4_HSM_MID_CLOCK 149985000
#define HDMI_14_MAX_TMDS_CLK (340 * 1000 * 1000)
@@ -739,7 +738,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct
drm_encoder *encoder,
conn_state_to_vc4_hdmi_conn_state(conn_state);
struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
- unsigned long pixel_rate, hsm_rate;
+ unsigned long bvb_rate, pixel_rate, hsm_rate;
int ret;
ret = pm_runtime_get_sync(&vc4_hdmi->pdev->dev);
@@ -793,12 +792,14 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct
drm_encoder *encoder,
vc4_hdmi_cec_update_clk_div(vc4_hdmi);
- /*
-* FIXME: When the pixel freq is 594MHz (4k60), this needs to be setup
-* at 300MHz.
-*/
- ret = clk_set_min_rate(vc4_hdmi->pixel_bvb_clock,
- (hsm_rate > VC4_HSM_MID_CLOCK ? 15000 :
7500));
+ if (pixel_rate > 29700)
+ bvb_rate = 3;
+ else if (pixel_rate > 14850)
+ bvb_rate = 15000;
+ else
+ bvb_rate = 7500;
+
+ ret = clk_set_min_rate(vc4_hdmi->pixel_bvb_clock, bvb_rate);
if (ret) {
DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret);
clk_disable_unprepare(vc4_hdmi->hsm_clock);
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 7/9] drm/vc4: hdmi: Check and warn if we can't reach 4kp60 frequencies
In order to reach the frequencies needed to output at 594MHz, the
firmware needs to be configured with the appropriate parameters in the
config.txt file (enable_hdmi_4kp60 and force_turbo).
Let's detect it at bind time, warn the user if we can't, and filter out
the relevant modes.
Acked-by: Thomas Zimmermann
Reviewed-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 31 +++
drivers/gpu/drm/vc4/vc4_hdmi.h | 8
2 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index c50dc5a59b2f..01d24ce8a795 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -94,6 +94,11 @@
#define HDMI_14_MAX_TMDS_CLK (340 * 1000 * 1000)
+static bool vc4_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode)
+{
+ return (mode->clock * 1000) > HDMI_14_MAX_TMDS_CLK;
+}
+
static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
{
struct drm_info_node *node = (struct drm_info_node *)m->private;
@@ -210,6 +215,18 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
+ if (vc4_hdmi->disable_4kp60) {
+ struct drm_device *drm = connector->dev;
+ struct drm_display_mode *mode;
+
+ list_for_each_entry(mode, &connector->probed_modes, head) {
+ if (vc4_hdmi_mode_needs_scrambling(mode)) {
+ drm_warn_once(drm, "The core clock cannot reach
frequencies high enough to support 4k @ 60Hz.");
+ drm_warn_once(drm, "Please change your
config.txt file to add hdmi_enable_4kp60.");
+ }
+ }
+ }
+
return ret;
}
@@ -959,6 +976,9 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder
*encoder,
if (pixel_rate > vc4_hdmi->variant->max_pixel_clock)
return -EINVAL;
+ if (vc4_hdmi->disable_4kp60 && (pixel_rate > HDMI_14_MAX_TMDS_CLK))
+ return -EINVAL;
+
vc4_state->pixel_rate = pixel_rate;
return 0;
@@ -978,6 +998,9 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder,
if ((mode->clock * 1000) > vc4_hdmi->variant->max_pixel_clock)
return MODE_CLOCK_HIGH;
+ if (vc4_hdmi->disable_4kp60 && vc4_hdmi_mode_needs_scrambling(mode))
+ return MODE_CLOCK_HIGH;
+
return MODE_OK;
}
@@ -1993,6 +2016,14 @@ static int vc4_hdmi_bind(struct device *dev, struct
device *master, void *data)
vc4_hdmi->disable_wifi_frequencies =
of_property_read_bool(dev->of_node, "wifi-2.4ghz-coexistence");
+ if (variant->max_pixel_clock == 6) {
+ struct vc4_dev *vc4 = to_vc4_dev(drm);
+ long max_rate = clk_round_rate(vc4->hvs->core_clk, 55000);
+
+ if (max_rate < 55000)
+ vc4_hdmi->disable_4kp60 = true;
+ }
+
if (vc4_hdmi->variant->reset)
vc4_hdmi->variant->reset(vc4_hdmi);
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 3cebd1fd00fc..3cd021136402 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -154,6 +154,14 @@ struct vc4_hdmi {
*/
bool disable_wifi_frequencies;
+ /*
+* Even if HDMI0 on the RPi4 can output modes requiring a pixel
+* rate higher than 297MHz, it needs some adjustments in the
+* config.txt file to be able to do so and thus won't always be
+* available.
+*/
+ bool disable_4kp60;
+
struct cec_adapter *cec_adap;
struct cec_msg cec_rx_msg;
bool cec_tx_ok;
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 8/9] drm/vc4: hdmi: Enable the scrambler
The HDMI controller on the BCM2711 includes a scrambler in order to
reach the HDMI 2.0 modes that require it. Let's add the support for it.
Acked-by: Thomas Zimmermann
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 64 +
drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 ++
2 files changed, 67 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 01d24ce8a795..bda12fea0dce 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -76,6 +77,8 @@
#define VC5_HDMI_VERTB_VSPO_SHIFT 16
#define VC5_HDMI_VERTB_VSPO_MASK VC4_MASK(29, 16)
+#define VC5_HDMI_SCRAMBLER_CTL_ENABLE BIT(0)
+
#define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_SHIFT 8
#define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK VC4_MASK(10, 8)
@@ -462,6 +465,64 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
vc4_hdmi_set_audio_infoframe(encoder);
}
+static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
+struct drm_display_mode *mode)
+{
+ struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
+ struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct drm_display_info *display = &vc4_hdmi->connector.display_info;
+
+ if (!vc4_encoder->hdmi_monitor)
+ return false;
+
+ if (!display->hdmi.scdc.supported ||
+ !display->hdmi.scdc.scrambling.supported)
+ return false;
+
+ return true;
+}
+
+static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
+{
+ struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
+ struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+
+ if (!vc4_hdmi_supports_scrambling(encoder, mode))
+ return;
+
+ if (!vc4_hdmi_mode_needs_scrambling(mode))
+ return;
+
+ drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true);
+ drm_scdc_set_scrambling(vc4_hdmi->ddc, true);
+
+ HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) |
+ VC5_HDMI_SCRAMBLER_CTL_ENABLE);
+}
+
+static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder)
+{
+ struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct drm_crtc *crtc = encoder->crtc;
+
+ /*
+* At boot, encoder->crtc will be NULL. Since we don't know the
+* state of the scrambler and in order to avoid any
+* inconsistency, let's disable it all the time.
+*/
+ if (crtc && !vc4_hdmi_supports_scrambling(encoder, &crtc->mode))
+ return;
+
+ if (crtc && !vc4_hdmi_mode_needs_scrambling(&crtc->mode))
+ return;
+
+ HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) &
+ ~VC5_HDMI_SCRAMBLER_CTL_ENABLE);
+
+ drm_scdc_set_scrambling(vc4_hdmi->ddc, false);
+ drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, false);
+}
+
static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
struct drm_atomic_state *state)
{
@@ -474,6 +535,8 @@ static void vc4_hdmi_encoder_post_crtc_disable(struct
drm_encoder *encoder,
HDMI_WRITE(HDMI_VID_CTL,
HDMI_READ(HDMI_VID_CTL) | VC4_HD_VID_CTL_BLANKPIX);
+
+ vc4_hdmi_disable_scrambling(encoder);
}
static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder,
@@ -924,6 +987,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct
drm_encoder *encoder,
}
vc4_hdmi_recenter_fifo(vc4_hdmi);
+ vc4_hdmi_enable_scrambling(encoder);
}
static void vc4_hdmi_encoder_enable(struct drm_encoder *encoder)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
index e1b58eac766f..19d2fdc446bc 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
@@ -100,6 +100,7 @@ enum vc4_hdmi_field {
HDMI_RM_FORMAT,
HDMI_RM_OFFSET,
HDMI_SCHEDULER_CONTROL,
+ HDMI_SCRAMBLER_CTL,
HDMI_SW_RESET_CONTROL,
HDMI_TX_PHY_CHANNEL_SWAP,
HDMI_TX_PHY_CLK_DIV,
@@ -238,6 +239,7 @@ static const struct vc4_hdmi_register __maybe_unused
vc5_hdmi_hdmi0_fields[] = {
VC4_HDMI_REG(HDMI_GCP_CONFIG, 0x178),
VC4_HDMI_REG(HDMI_GCP_WORD_1, 0x17c),
VC4_HDMI_REG(HDMI_HOTPLUG, 0x1a8),
+ VC4_HDMI_REG(HDMI_SCRAMBLER_CTL, 0x1c4),
VC5_DVP_REG(HDMI_CLOCK_STOP, 0x0bc),
VC5_DVP_REG(HDMI_VEC_INTERFACE_XBAR, 0x0f0),
@@ -317,6 +319,7 @@ static const struct vc4_hdmi_register __maybe_unused
vc5_hdmi_hdmi1_fields
[PATCH v3 2/9] drm/vc4: crtc: Skip the TXP
The vc4_set_crtc_possible_masks is meant to run over all the encoders
and then set their possible_crtcs mask to their associated pixelvalve.
However, since the commit 39fcb2808376 ("drm/vc4: txp: Turn the TXP into
a CRTC of its own"), the TXP has been turned to a CRTC and encoder of
its own, and while it does indeed register an encoder, it no longer has
an associated pixelvalve. The code will thus run over the TXP encoder
and set a bogus possible_crtcs mask, overriding the one set in the TXP
bind function.
In order to fix this, let's skip any virtual encoder.
Cc: # v5.9+
Fixes: 39fcb2808376 ("drm/vc4: txp: Turn the TXP into a CRTC of its own")
Acked-by: Thomas Zimmermann
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_crtc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 269390bc586e..f1f2e8cbce79 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -1018,6 +1018,9 @@ static void vc4_set_crtc_possible_masks(struct drm_device
*drm,
struct vc4_encoder *vc4_encoder;
int i;
+ if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL)
+ continue;
+
vc4_encoder = to_vc4_encoder(encoder);
for (i = 0; i < ARRAY_SIZE(pv_data->encoder_types); i++) {
if (vc4_encoder->type == encoder_types[i]) {
--
2.30.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/12] drm/vc4: Don't set allow_fb_modifiers explicitly
On Tue, Apr 13, 2021 at 11:49:02AM +0200, Daniel Vetter wrote: > Since > > commit 890880ddfdbe256083170866e49c87618b706ac7 > Author: Paul Kocialkowski > Date: Fri Jan 4 09:56:10 2019 +0100 > > drm: Auto-set allow_fb_modifiers when given modifiers at plane init > > this is done automatically as part of plane init, if drivers set the > modifier list correctly. Which is the case here. > > Signed-off-by: Daniel Vetter > Cc: Eric Anholt > Cc: Maxime Ripard Acked-by: Maxime Ripard Thanks! Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 12/12] drm/modifiers: Enforce consistency between the cap an IN_FORMATS
On Tue, Apr 13, 2021 at 11:49:03AM +0200, Daniel Vetter wrote: > It's very confusing for userspace to have to deal with inconsistencies > here, and some drivers screwed this up a bit. Most just ommitted the > format list when they meant to say that only linear modifier is > allowed, but some also meant that only implied modifiers are > acceptable (because actually none of the planes registered supported > modifiers). > > Now that this is all done consistently across all drivers, document > the rules and enforce it in the drm core. > > Cc: Pekka Paalanen > Signed-off-by: Daniel Vetter > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter Acked-by: Maxime Ripard Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/18] drm/vc4: hdmi: Add full range RGB helper
Hi Thomas,
On Mon, Apr 12, 2021 at 11:44:05AM +0200, Thomas Zimmermann wrote:
>
>
> Am 17.03.21 um 16:43 schrieb Maxime Ripard:
> > We're going to need to tell whether we want to run with a full or
> > limited range RGB output in multiple places in the code, so let's create
> > a helper that will return whether we need with full range or not.
> >
> > Signed-off-by: Maxime Ripard
>
> Acked-by: Thomas Zimmermann
>
> although with a comments
>
> > ---
> > drivers/gpu/drm/vc4/vc4_hdmi.c | 12 ++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > index eee9751009c2..fc545072b173 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > @@ -95,6 +95,15 @@
> > #define HDMI_14_MAX_TMDS_CLK (340 * 1000 * 1000)
> > +static bool vc4_hdmi_is_full_range_rgb(struct vc4_hdmi *vc4_hdmi,
> > + const struct drm_display_mode *mode)
> > +{
> > + struct vc4_hdmi_encoder *vc4_encoder = &vc4_hdmi->encoder;
> > +
> > + return !vc4_encoder->hdmi_monitor ||
>
> Is this ever being called from non-HDMI code? If not, I'd put an
> drm_WARN_ONCE around this check.
I'm not sure we need to worry about this, it's a static function in the
HDMI controller driver so it can't be called from anywhere else
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/6] dt-bindings: display: add Unisoc's mipi dsi controller bindings
On Fri, Apr 09, 2021 at 08:23:19AM +0800, Kevin Tang wrote: > Maxime Ripard 于2021年4月7日周三 下午6:46写道： > > > On Wed, Mar 31, 2021 at 09:49:14AM +0800, Kevin Tang wrote: > > > Hi Maxime, > > > > > > Maxime Ripard 于2021年3月24日周三 下午7:13写道： > > > > > > > On Mon, Feb 22, 2021 at 09:28:21PM +0800, Kevin Tang wrote: > > > > > From: Kevin Tang > > > > > > > > > > Adds MIPI DSI Controller > > > > > support for Unisoc's display subsystem. > > > > > > > > > > Cc: Orson Zhai > > > > > Cc: Chunyan Zhang > > > > > Signed-off-by: Kevin Tang > > > > > Reviewed-by: Rob Herring > > > > > --- > > > > > .../display/sprd/sprd,sharkl3-dsi-host.yaml | 102 > > ++ > > > > > 1 file changed, 102 insertions(+) > > > > > create mode 100644 > > > > > > Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > > > > > > > diff --git > > > > > > a/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > > > b/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > > new file mode 100644 > > > > > index 0..d439f688f > > > > > --- /dev/null > > > > > +++ > > > > > > b/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > > @@ -0,0 +1,102 @@ > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > +%YAML 1.2 > > > > > +--- > > > > > +$id: > > > > http://devicetree.org/schemas/display/sprd/sprd,sharkl3-dsi-host.yaml# > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > + > > > > > +title: Unisoc MIPI DSI Controller > > > > > + > > > > > +maintainers: > > > > > + - Kevin Tang > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > +const: sprd,sharkl3-dsi-host > > > > > + > > > > > + reg: > > > > > +maxItems: 1 > > > > > + > > > > > + interrupts: > > > > > +maxItems: 2 > > > > > + > > > > > + clocks: > > > > > +minItems: 1 > > > > > + > > > > > + clock-names: > > > > > +items: > > > > > + - const: clk_src_96m > > > > > + > > > > > + power-domains: > > > > > +maxItems: 1 > > > > > + > > > > > + ports: > > > > > +type: object > > > > > + > > > > > +properties: > > > > > + "#address-cells": > > > > > +const: 1 > > > > > + > > > > > + "#size-cells": > > > > > +const: 0 > > > > > + > > > > > + port@0: > > > > > +type: object > > > > > +description: > > > > > + A port node with endpoint definitions as defined in > > > > > + > > Documentation/devicetree/bindings/media/video-interfaces.txt. > > > > > + That port should be the input endpoint, usually coming > > from > > > > > + the associated DPU. > > > > > + port@1: > > > > > +type: object > > > > > +description: > > > > > + A port node with endpoint definitions as defined in > > > > > + > > Documentation/devicetree/bindings/media/video-interfaces.txt. > > > > > + That port should be the output endpoint, usually output to > > > > > + the associated panel. > > > > > > > > The DSI generic binding asks that peripherals that are controlled > > > > through a DCS be a subnode of the MIPI-DSI bus, not through a port > > > > endpoint. > > > > > > > Our DSI controller don't support DCS now... > > > > I'm not sure I follow you, you mentionned in the patch 4 that you were > > testing for a device to be in command mode, how would that work without > > DCS support? > > > Sorry, I see DCS as DSC, pls ignore my previous comments. > > dsi input node is display controller and dsi output node is panel, > I still don't understand what it has to do with dcs? and it seems that > other vendors also like this. > > can you help provide some cases? So the device tree is a tree organized through which bus controls which device: Your DSI controller is accessed through a memory-mapped region and is thus a child node of the main bus (I guess?) and then, since the DSI panel is going to be controlled through the DSI controller and MIPI-DCS, it needs to be a child of the display controller. This is exactly what is being described here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/mipi-dsi-bus.txt#n42 The second port is thus not needed at all Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 4/6] drm/sprd: add Unisoc's drm display controller driver
Hi,
On Thu, Apr 15, 2021 at 08:18:52AM +0800, Kevin Tang wrote:
> Maxime Ripard 于2021年3月24日周三 下午7:10写道：
> > > +static struct sprd_dpu *sprd_crtc_init(struct drm_device *drm,
> > > + struct drm_plane *primary)
> > > +{
> > > + struct device_node *port;
> > > + struct sprd_dpu *dpu;
> > > +
> > > + /*
> > > + * set crtc port so that drm_of_find_possible_crtcs call works
> > > + */
> > > + port = of_parse_phandle(drm->dev->of_node, "ports", 0);
> > > + if (!port) {
> > > + drm_err(drm, "find 'ports' phandle of %s failed\n",
> > > + drm->dev->of_node->full_name);
> > > + return ERR_PTR(-EINVAL);
> > > + }
> > > + of_node_put(port);
> >
> > The YAML binding should already make sure that your binding is sane, and
> > if you still get a DT that doesn't follow it, you have a whole lot of
> > other issues than whether ports is there :)
> >
> > > + dpu = drmm_crtc_alloc_with_planes(drm, struct sprd_dpu, base,
> > > + primary, NULL,
> > > + &sprd_crtc_funcs, NULL);
> > > + if (IS_ERR(dpu)) {
> > > + drm_err(drm, "failed to init crtc.\n");
> > > + return dpu;
> > > + }
> > > +
> > > + dpu->base.port = port;
> >
> > But you're still referencing it here, while you called of_node_put on it
> > already? You should only call it once you're done with it.
>
> of_node_put should be called after done with it, this maybe indeed be a bug.
> i will fix it.
> >
> >
> > I'm not really sure why you would need drm_of_find_possible_crtcs to
> > work then if you don't follow the OF-Graph bindings.
>
> it scan all endports of encoder, if a matching crtc is found by
> OF-Graph bindings
> and then genarate the crtc mask, here is description:
> 41 /**
> 42 * drm_of_find_possible_crtcs - find the possible CRTCs for an encoder
> port
> 43 * @dev: DRM device
> 44 * @port: encoder port to scan for endpoints
> 45 *
> 46 * Scan all endpoints attached to a port, locate their attached CRTCs,
> 47 * and generate the DRM mask of CRTCs which may be attached to this
> 48 * encoder.
> 49 *
> if we don't follow the OF-Graph bindings, crtc can't attched to encoder.
Yeah, what I'm actually confused about is why you would need the
of_parse_phandle call. You usually call drm_of_find_possible_crtcs with
the encoder device node, so from your MIPI-DSI driver in your case, and
with it's device->of_node pointer and it should work perfectly fine?
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 4/6] drm/sprd: add Unisoc's drm display controller driver
Hi,
On Fri, Apr 09, 2021 at 09:35:07PM +0800, Kevin Tang wrote:
> > > > > + }
> > > > > +
> > > > > + return MODE_OK;
> > > > > +}
> > > > > +
> > > > > +static void sprd_crtc_atomic_enable(struct drm_crtc *crtc,
> > > > > +struct drm_atomic_state *state)
> > > > > +{
> > > > > + struct sprd_dpu *dpu = to_sprd_crtc(crtc);
> > > > > +
> > > > > + sprd_dpu_init(dpu);
> > > > > +
> > > > > + sprd_dpi_init(dpu);
> > > > > +
> > > > > + enable_irq(dpu->ctx.irq);
> > > >
> > > > Shouldn't this be in enable_vblank? And I would assume that you would
> > > > have the interrupts enabled all the time, but disabled in your device?
> > > >
> > > It seems better to put in enable_vblank, i will try and test it... Thks
> > >
> > > And I would assume that you would
> > > have the interrupts enabled all the time, but disabled in your device?
> > > [kevin]I don’t quite understand this, can you help me explain it in
> > > detail?
> >
> > You seem to have a register that enables and disables the interrupt in
> > that device. The way we usually deal with them in this case is just to
> > call request_irq in your bind/probe with the interrupts enabled at the
> > controller level, and mask them when needed at the device level by
> > clearing / setting that bit.
> >
> Yeah, we have display controller interrupts setting and clear register.
> But the interrupts all been enabled in bootloader(eg, lk or uboot),
> if the interrupt handler is active in the probe/bind phase by request_irq,
> but the whole display pipeline is not ready, there maybe have some problems.
It's fairly common to clear / ack the interrupts from the device before
calling request_irq precisely to avoid that issue.
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 08/10] drm/simpledrm: Acquire clocks from DT device node
Hi, On Thu, Apr 15, 2021 at 09:31:01AM +0200, Thomas Zimmermann wrote: > Am 08.04.21 um 10:13 schrieb Maxime Ripard: > > Hi, > > > > On Thu, Mar 18, 2021 at 11:29:19AM +0100, Thomas Zimmermann wrote: > > > Make sure required hardware clocks are enabled while the firmware > > > framebuffer is in use. > > > > > > The basic code has been taken from the simplefb driver and adapted > > > to DRM. Clocks are released automatically via devres helpers. > > > > > > Signed-off-by: Thomas Zimmermann > > > Tested-by: nerdopolis > > > > Even though it's definitely simpler to review, merging the driver first > > and then the clocks and regulators will break bisection on the platforms > > that rely on them > > I'd like to keep the patches separate for now, but can squash patches 6 to 8 > them into one before pushing them. OK? Yep, that works for me :) > > > > Another thing worth considering is also that both drivers will probe if > > they are enabled (which is pretty likely), which is not great :) > > > > I guess we should make them mutually exclusive through Kconfig > > We already have several drivers in fbdev and DRM that handle the same > hardware. We don't do this for any other pair, why bother now? Yeah, but simplefb/simpledrm are going to be enabled pretty much everywhere, as opposed to the other drivers that are more specialized. Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/ingenic: Don't request full modeset if property is not modified
Hi,
On Mon, Mar 29, 2021 at 06:50:46PM +0100, Paul Cercueil wrote:
> Avoid requesting a full modeset if the sharpness property is not
> modified, because then we don't actually need it.
>
> Fixes: fc1acf317b01 ("drm/ingenic: Add support for the IPU")
> Cc: # 5.8+
> Signed-off-by: Paul Cercueil
> ---
> drivers/gpu/drm/ingenic/ingenic-ipu.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c
> b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> index 3b1091e7c0cd..95b665c4a7b0 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
> @@ -640,10 +640,12 @@ ingenic_ipu_plane_atomic_set_property(struct drm_plane
> *plane,
> {
> struct ingenic_ipu *ipu = plane_to_ingenic_ipu(plane);
> struct drm_crtc_state *crtc_state;
> + bool mode_changed;
>
> if (property != ipu->sharpness_prop)
> return -EINVAL;
>
> + mode_changed = val != ipu->sharpness;
> ipu->sharpness = val;
>
> if (state->crtc) {
> @@ -651,7 +653,7 @@ ingenic_ipu_plane_atomic_set_property(struct drm_plane
> *plane,
> if (WARN_ON(!crtc_state))
> return -EINVAL;
>
> - crtc_state->mode_changed = true;
> + crtc_state->mode_changed |= mode_changed;
> }
I'd just change the condition from
if (state->crtc)
to
if (state->crtc && val != ipu->sharpness)
It's going to be easier to extend if you ever need to. Also, drivers
usually do this in atomic_check, is there a specific reason to do it in
atomic_set_property?
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vc4: remove unused function
On Thu, Apr 15, 2021 at 04:45:25PM +0800, Jiapeng Chong wrote: > Fix the following clang warning: > > drivers/gpu/drm/vc4/vc4_vec.c:201:1: warning: unused function > 'to_vc4_vec_connector' [-Wunused-function]. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong Merged, thanks! Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/6] dt-bindings: display: add Unisoc's mipi dsi controller bindings
Hi, On Mon, Apr 19, 2021 at 12:33:42AM +0800, Kevin Tang wrote: > Maxime Ripard 于2021年4月15日周四 下午4:42写道： > > > > On Fri, Apr 09, 2021 at 08:23:19AM +0800, Kevin Tang wrote: > > > Maxime Ripard 于2021年4月7日周三 下午6:46写道： > > > > > > > On Wed, Mar 31, 2021 at 09:49:14AM +0800, Kevin Tang wrote: > > > > > Hi Maxime, > > > > > > > > > > Maxime Ripard 于2021年3月24日周三 下午7:13写道： > > > > > > > > > > > On Mon, Feb 22, 2021 at 09:28:21PM +0800, Kevin Tang wrote: > > > > > > > From: Kevin Tang > > > > > > > > > > > > > > Adds MIPI DSI Controller > > > > > > > support for Unisoc's display subsystem. > > > > > > > > > > > > > > Cc: Orson Zhai > > > > > > > Cc: Chunyan Zhang > > > > > > > Signed-off-by: Kevin Tang > > > > > > > Reviewed-by: Rob Herring > > > > > > > --- > > > > > > > .../display/sprd/sprd,sharkl3-dsi-host.yaml | 102 > > > > ++ > > > > > > > 1 file changed, 102 insertions(+) > > > > > > > create mode 100644 > > > > > > > > > > Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > > > > > > > > > > > diff --git > > > > > > > > > > a/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > > > > > > > b/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > > > > new file mode 100644 > > > > > > > index 0..d439f688f > > > > > > > --- /dev/null > > > > > > > +++ > > > > > > > > > > b/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml > > > > > > > @@ -0,0 +1,102 @@ > > > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > > > > +%YAML 1.2 > > > > > > > +--- > > > > > > > +$id: > > > > > > http://devicetree.org/schemas/display/sprd/sprd,sharkl3-dsi-host.yaml# > > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > > + > > > > > > > +title: Unisoc MIPI DSI Controller > > > > > > > + > > > > > > > +maintainers: > > > > > > > + - Kevin Tang > > > > > > > + > > > > > > > +properties: > > > > > > > + compatible: > > > > > > > +const: sprd,sharkl3-dsi-host > > > > > > > + > > > > > > > + reg: > > > > > > > +maxItems: 1 > > > > > > > + > > > > > > > + interrupts: > > > > > > > +maxItems: 2 > > > > > > > + > > > > > > > + clocks: > > > > > > > +minItems: 1 > > > > > > > + > > > > > > > + clock-names: > > > > > > > +items: > > > > > > > + - const: clk_src_96m > > > > > > > + > > > > > > > + power-domains: > > > > > > > +maxItems: 1 > > > > > > > + > > > > > > > + ports: > > > > > > > +type: object > > > > > > > + > > > > > > > +properties: > > > > > > > + "#address-cells": > > > > > > > +const: 1 > > > > > > > + > > > > > > > + "#size-cells": > > > > > > > +const: 0 > > > > > > > + > > > > > > > + port@0: > > > > > > > +type: object > > > > > > > +description: > > > > > > > + A port node with endpoint definitions as defined in > > > > > > > + > > > > Documentation/devicetree/bindings/media/video-interfaces.txt. > > > > > > > + That port should be the input endpoint, usually coming > > > > from > > > > > > > + the associated DPU. > > > > > > > + port@1: > > > > &g
Re: [PATCH v4 4/6] drm/sprd: add Unisoc's drm display controller driver
On Mon, Apr 19, 2021 at 07:01:00AM +0800, Kevin Tang wrote:
> Maxime Ripard 于2021年4月15日周四 下午5:03写道：
> > On Thu, Apr 15, 2021 at 08:18:52AM +0800, Kevin Tang wrote:
> > > Maxime Ripard 于2021年3月24日周三 下午7:10写道：
> > > > > +static struct sprd_dpu *sprd_crtc_init(struct drm_device *drm,
> > > > > + struct drm_plane *primary)
> > > > > +{
> > > > > + struct device_node *port;
> > > > > + struct sprd_dpu *dpu;
> > > > > +
> > > > > + /*
> > > > > + * set crtc port so that drm_of_find_possible_crtcs call works
> > > > > + */
> > > > > + port = of_parse_phandle(drm->dev->of_node, "ports", 0);
> > > > > + if (!port) {
> > > > > + drm_err(drm, "find 'ports' phandle of %s failed\n",
> > > > > + drm->dev->of_node->full_name);
> > > > > + return ERR_PTR(-EINVAL);
> > > > > + }
> > > > > + of_node_put(port);
> > > >
> > > > The YAML binding should already make sure that your binding is sane, and
> > > > if you still get a DT that doesn't follow it, you have a whole lot of
> > > > other issues than whether ports is there :)
> > > >
> > > > > + dpu = drmm_crtc_alloc_with_planes(drm, struct sprd_dpu, base,
> > > > > + primary, NULL,
> > > > > + &sprd_crtc_funcs, NULL);
> > > > > + if (IS_ERR(dpu)) {
> > > > > + drm_err(drm, "failed to init crtc.\n");
> > > > > + return dpu;
> > > > > + }
> > > > > +
> > > > > + dpu->base.port = port;
> > > >
> > > > But you're still referencing it here, while you called of_node_put on it
> > > > already? You should only call it once you're done with it.
> > >
> > > of_node_put should be called after done with it, this maybe indeed be a
> > > bug.
> > > i will fix it.
> > > >
> > > >
> > > > I'm not really sure why you would need drm_of_find_possible_crtcs to
> > > > work then if you don't follow the OF-Graph bindings.
> > >
> > > it scan all endports of encoder, if a matching crtc is found by
> > > OF-Graph bindings
> > > and then genarate the crtc mask, here is description:
> > > 41 /**
> > > 42 * drm_of_find_possible_crtcs - find the possible CRTCs for an
> > > encoder port
> > > 43 * @dev: DRM device
> > > 44 * @port: encoder port to scan for endpoints
> > > 45 *
> > > 46 * Scan all endpoints attached to a port, locate their attached CRTCs,
> > > 47 * and generate the DRM mask of CRTCs which may be attached to this
> > > 48 * encoder.
> > > 49 *
> > > if we don't follow the OF-Graph bindings, crtc can't attched to encoder.
> >
> > Yeah, what I'm actually confused about is why you would need the
> > of_parse_phandle call. You usually call drm_of_find_possible_crtcs with
> > the encoder device node, so from your MIPI-DSI driver in your case, and
> > with it's device->of_node pointer and it should work perfectly fine?
> I still confused about use drm_of_find_possible_crtcs to bind crtc and
> encoder, the port of drm_crtc, default is null?
>
> 709 /**
> 710 * struct drm_crtc - central CRTC control structure
> 711 * @dev: parent DRM device
> 712 * @port: OF node used by drm_of_find_possible_crtcs()
> -
> 25 static uint32_t drm_crtc_port_mask(struct drm_device *dev,
> 26 struct device_node *port)
> 27 {
> 28 unsigned int index = 0;
> 29 struct drm_crtc *tmp;
> 30
> 31 drm_for_each_crtc(tmp, dev) {
> 32 if (tmp->port == port)
> 33 return 1 << index;
> 34
> 35 index++;
> 36 }
> 37
> 38 return 0;
> 39 }
>
> I did not see any place to initialize the port of drm_crtc in the drm
> framework, if not setup it.
> it looks like the port of drm_crtc is undefined.
>
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/rockchip/rockchip_drm_vop.c#L1851
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/tilcdc/tilcdc_crtc.c#L1051
Yeah, you need to initialize it for drm_of_find_possible_crtcs to work,
but my point was that you shouldn't do it through of_parse_phandle, just
put the output of_graph_get_port_by_id like you found in those drivers.
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next-fixes
Hi Dave, Daniel, Here's this week drm-misc-next-fixes PR, for the next merge window Thanks! Maxime drm-misc-next-fixes-2021-04-22: A few fixes for the next merge window, with some build fixes for anx7625 and lt8912b bridges, incorrect error handling for lt8912b and TTM, and one fix for TTM page limit accounting. The following changes since commit 9c0fed84d5750e1eea6c664e073ffa2534a17743: Merge tag 'drm-intel-next-2021-04-01' of git://anongit.freedesktop.org/drm/drm-intel into drm-next (2021-04-08 14:02:21 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2021-04-22 for you to fetch changes up to a4394b6d0a273941a75ebe86a86d6416d536ed0f: drm/ttm: Don't count pages in SG BOs against pages_limit (2021-04-21 15:35:20 +0200) A few fixes for the next merge window, with some build fixes for anx7625 and lt8912b bridges, incorrect error handling for lt8912b and TTM, and one fix for TTM page limit accounting. Adrien Grassein (1): drm/bridge: lt8912b: fix incorrect handling of of_* return values Christian König (1): drm/ttm: fix return value check Felix Kuehling (1): drm/ttm: Don't count pages in SG BOs against pages_limit Randy Dunlap (2): drm: bridge: fix ANX7625 use of mipi_dsi_() functions drm: bridge: fix LONTIUM use of mipi_dsi_() functions drivers/gpu/drm/bridge/Kconfig | 3 +++ drivers/gpu/drm/bridge/analogix/Kconfig | 1 + drivers/gpu/drm/bridge/lontium-lt8912b.c | 32 +--- drivers/gpu/drm/ttm/ttm_tt.c | 29 +++-- 4 files changed, 40 insertions(+), 25 deletions(-) signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PULL] drm-misc-next-fixes
Hi Alex, On Thu, Apr 22, 2021 at 12:40:10PM -0400, Alex Deucher wrote: > On Thu, Apr 22, 2021 at 12:33 PM Maxime Ripard wrote: > > > > Hi Dave, Daniel, > > > > Here's this week drm-misc-next-fixes PR, for the next merge window > > > > Can we also cherry-pick this patch: > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d510c88cfbb294d2b1e2d0b71576e9b79d0e2e83 > It should have really gone into drm-misc-next-fixes rather than > drm-misc-next, but I misjudged the timing. Yeah, just cherry-pick it, I'll keep sending PR during the merge window :) Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next-fixes
Hi Dave, Daniel, Here's this week drm-misc-next-fixes PR Maxime drm-misc-next-fixes-2021-04-29: Two patches in drm-misc-next-fixes this week, one to fix the error handling in TTM when a BO can't be swapped out and one to prevent a wrong dereference in efifb. The following changes since commit a4394b6d0a273941a75ebe86a86d6416d536ed0f: drm/ttm: Don't count pages in SG BOs against pages_limit (2021-04-21 15:35:20 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2021-04-29 for you to fetch changes up to 74deef03a44ae77db85dd80e7ef95777a902e0b3: efifb: Check efifb_pci_dev before using it (2021-04-26 17:33:03 -0400) Two patches in drm-misc-next-fixes this week, one to fix the error handling in TTM when a BO can't be swapped out and one to prevent a wrong dereference in efifb. Kai-Heng Feng (1): efifb: Check efifb_pci_dev before using it Shiwu Zhang (1): drm/ttm: fix error handling if no BO can be swapped out v4 drivers/gpu/drm/ttm/ttm_device.c| 2 +- drivers/gpu/drm/ttm/ttm_tt.c| 2 ++ drivers/gpu/drm/vmwgfx/ttm_memory.c | 2 +- drivers/video/fbdev/efifb.c | 6 -- 4 files changed, 8 insertions(+), 4 deletions(-) signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PULL] drm-misc-next-fixes
On Wed, Apr 28, 2021 at 04:57:26PM -0400, Alex Deucher wrote: > On Mon, Apr 26, 2021 at 3:35 AM Maxime Ripard wrote: > > > > Hi Alex, > > > > On Thu, Apr 22, 2021 at 12:40:10PM -0400, Alex Deucher wrote: > > > On Thu, Apr 22, 2021 at 12:33 PM Maxime Ripard wrote: > > > > > > > > Hi Dave, Daniel, > > > > > > > > Here's this week drm-misc-next-fixes PR, for the next merge window > > > > > > > > > > Can we also cherry-pick this patch: > > > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d510c88cfbb294d2b1e2d0b71576e9b79d0e2e83 > > > It should have really gone into drm-misc-next-fixes rather than > > > drm-misc-next, but I misjudged the timing. > > > > Yeah, just cherry-pick it, I'll keep sending PR during the merge window :) > > Thanks, I cherry-picked it yesterday. I sent the PR earlier :) Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vc4: fix argument ordering in vc4_crtc_get_margins()
On Wed, Apr 21, 2021 at 01:18:03PM +0300, Dan Carpenter wrote: > Cppcheck complains that the declaration doesn't match the function > definition. Obviously "left" should come before "right". The caller > and the function implementation are done this way, it's just the > declaration which is wrong so this doesn't affect runtime. > > Reported-by: kernel test robot > Signed-off-by: Dan Carpenter Applied, thanks! Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 0/9] drm: Support simple-framebuffer devices and firmware fbs
On Fri, Apr 16, 2021 at 11:00:39AM +0200, Thomas Zimmermann wrote: > This patchset adds support for simple-framebuffer platform devices and > a handover mechanism for native drivers to take-over control of the > hardware. > > The new driver, called simpledrm, binds to a simple-frambuffer platform > device. The kernel's boot code creates such devices for firmware-provided > framebuffers, such as EFI-GOP or VESA. Typically the BIOS, UEFI or boot > loader sets up the framebuffers. Description via device tree is also an > option. > > Simpledrm is small enough to be linked into the kernel. The driver's main > purpose is to provide graphical output during the early phases of the boot > process, before the native DRM drivers are available. Native drivers are > typically loaded from an initrd ram disk. Occationally simpledrm can also > serve as interim solution on graphics hardware without native DRM driver. > > So far distributions rely on fbdev drivers, such as efifb, vesafb or > simplefb, for early-boot graphical output. However fbdev is deprecated and > the drivers do not provide DRM interfaces for modern userspace. > > Patches 1 and 2 prepare the DRM format helpers for simpledrm. > > Patches 4 to 8 add the simpledrm driver. It's build on simple DRM helpers > and SHMEM. It supports 16-bit, 24-bit and 32-bit RGB framebuffers. During > pageflips, SHMEM buffers are copied into the framebuffer memory, similar > to cirrus or mgag200. The code in patches 7 and 8 handles clocks and > regulators. It's based on the simplefb drivers, but has been modified for > DRM. > > Patches 3 and 9 add a hand-over mechanism. Simpledrm acquires it's > framebuffer's I/O-memory range and can be hot-unplugged by a native driver. > The native driver will remove simpledrm before taking over the hardware. > The removal is integrated into existing helpers, so existing drivers use > it automatically. > > I've also been working on fastboot support (i.e., flicker-free booting). > This requires state-readout from simpledrm via generic interfaces, as > outlined in [1]. I do have some prototype code, but it will take a while > to get this ready. Simpledrm will then support it. > > I've tested simpledrm with x86 EFI and VESA framebuffers, which both work > reliably. The fbdev console and Weston work automatically. Xorg requires > manual configuration of the device. Xorgs current modesetting driver does > not work with both, platform and PCI device, for the same physical > hardware. Once configured, X11 works. I looked into X11, but couldn't see > an easy way of fixing the problem. With the push towards Wayland+Xwayland > I expect the problem to become a non-issue soon. Additional testing has > been reported at [2]. > > One cosmetical issue is that simpledrm's device file is card0 and the > native driver's device file is card1. After simpledrm has been kicked out, > only card1 is left. This does not seem to be a practical problem however. Provided that patches 4 to 8 are squashed when merged: Acked-by: Maxime Ripard Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] clk: Implement a clock request API
Hi Mike, Stephen, On Tue, Apr 13, 2021 at 12:13:18PM +0200, Maxime Ripard wrote: > Hi, > > This is a follow-up of the discussion here: > https://lore.kernel.org/linux-clk/20210319150355.xzw7ikwdaga2dwhv@gilmour/ > > This implements a mechanism to raise and lower clock rates based on consumer > workloads, with an example of such an implementation for the RaspberryPi4 HDMI > controller. > > There's a couple of things worth discussing: > > - The name is in conflict with clk_request_rate, and even though it feels > like the right name to me, we should probably avoid any confusion > > - The code so far implements a policy of always going for the lowest rate > possible. While we don't have an use-case for something else, this should > maybe be made more flexible? Ping? Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/6] dt-bindings: display: add Unisoc's drm master bindings
On Sun, Apr 25, 2021 at 08:36:02PM +0800, Kevin Tang wrote:
> From: Kevin Tang
>
> The Unisoc DRM master device is a virtual device needed to list all
> DPU devices or other display interface nodes that comprise the
> graphics subsystem
>
> Unisoc's display pipeline have several components as below
> description, multi display controllers and corresponding physical
> interfaces.
> For different display scenarios, dpu0 and dpu1 maybe binding to
> different encoder.
>
> E.g:
> dpu0 and dpu1 both binding to DSI for dual mipi-dsi display;
> dpu0 binding to DSI for primary display, and dpu1 binding to DP for
> external display;
>
> Cc: Orson Zhai
> Cc: Chunyan Zhang
> Signed-off-by: Kevin Tang
> Reviewed-by: Rob Herring
> ---
> .../display/sprd/sprd,display-subsystem.yaml | 64 +++
> 1 file changed, 64 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/sprd/sprd,display-subsystem.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/sprd/sprd,display-subsystem.yaml
> b/Documentation/devicetree/bindings/display/sprd/sprd,display-subsystem.yaml
> new file mode 100644
> index 0..3d107e943
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/sprd/sprd,display-subsystem.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/sprd/sprd,display-subsystem.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Unisoc DRM master device
> +
> +maintainers:
> + - Kevin Tang
> +
> +description: |
> + The Unisoc DRM master device is a virtual device needed to list all
> + DPU devices or other display interface nodes that comprise the
> + graphics subsystem.
> +
> + Unisoc's display pipeline have several components as below description,
> + multi display controllers and corresponding physical interfaces.
> + For different display scenarios, dpu0 and dpu1 maybe binding to different
> + encoder.
> +
> + E.g:
> + dpu0 and dpu1 both binding to DSI for dual mipi-dsi display;
> + dpu0 binding to DSI for primary display, and dpu1 binding to DP for
> external display;
> +
> + +-+
> + | |
> + |+-+ |
> + ++ | +++-+|DPHY/CPHY| | +--+
> + |+->+dpu0+--->+MIPI|DSI +--->+Combo+->+Panel0|
> + |AXI | | +++-++-+ | +--+
> + || | ^ |
> + || | | |
> + || | +---+ |
> + || | | |
> + |APB | | +--+-++---++---+ | +--+
> + |+->+dpu1+--->+DisplayPort+--->+PHY+->+Panel1|
> + || | +++---++---+ | +--+
> + ++ | |
> + +-+
> +
> +properties:
> + compatible:
> +const: sprd,display-subsystem
> +
> + ports:
> +$ref: /schemas/types.yaml#/definitions/phandle-array
> +description:
> + Should contain a list of phandles pointing to display interface port
> + of DPU devices.
> +
> +required:
> + - compatible
> + - ports
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +display-subsystem {
> +compatible = "sprd,display-subsystem";
> +ports = <&dpu_out>;
> +};
Given your diagram, I guess it should be the dpu input?
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 4/6] drm/sprd: add Unisoc's drm display controller driver
On Sun, Apr 25, 2021 at 08:36:05PM +0800, Kevin Tang wrote: > Adds DPU(Display Processor Unit) support for the Unisoc's display > subsystem. > It's support multi planes, scaler, rotation, PQ(Picture Quality) and more. > > v2: > - Use drm_xxx to replace all DRM_XXX. > - Use kzalloc to replace devm_kzalloc for sprd_dpu structure init. > > v3: > - Remove dpu_layer stuff layer and commit layers by aotmic_update > > v4: > - Use drmm_helpers to allocate crtc and planes. > - Move rotation enum definitions to crtc layer reg bitfields. > - Move allocate crtc and planes to bind function. > > v5: > - Fix the checkpatch warnings. > - Use mode_set_nofb instead of mode_valid callback. > - Follow the OF-Graph bindings, use of_graph_get_port_by_id > instead of of_parse_phandle. > - Use zpos to represent the layer position. > - Rebase to last drm misc branch. > > Cc: Orson Zhai > Cc: Chunyan Zhang > Signed-off-by: Kevin Tang > --- > drivers/gpu/drm/sprd/Kconfig| 1 + > drivers/gpu/drm/sprd/Makefile | 3 +- > drivers/gpu/drm/sprd/sprd_dpu.c | 939 > drivers/gpu/drm/sprd/sprd_dpu.h | 109 > drivers/gpu/drm/sprd/sprd_drm.c | 1 + > drivers/gpu/drm/sprd/sprd_drm.h | 2 + > 6 files changed, 1054 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/sprd/sprd_dpu.c > create mode 100644 drivers/gpu/drm/sprd/sprd_dpu.h > > diff --git a/drivers/gpu/drm/sprd/Kconfig b/drivers/gpu/drm/sprd/Kconfig > index 726c3e76d..37762c333 100644 > --- a/drivers/gpu/drm/sprd/Kconfig > +++ b/drivers/gpu/drm/sprd/Kconfig > @@ -5,6 +5,7 @@ config DRM_SPRD > select DRM_GEM_CMA_HELPER > select DRM_KMS_CMA_HELPER > select DRM_KMS_HELPER > + select VIDEOMODE_HELPERS > help > Choose this option if you have a Unisoc chipset. > If M is selected the module will be called sprd_drm. > diff --git a/drivers/gpu/drm/sprd/Makefile b/drivers/gpu/drm/sprd/Makefile > index 9850f00b8..ab12b95e6 100644 > --- a/drivers/gpu/drm/sprd/Makefile > +++ b/drivers/gpu/drm/sprd/Makefile > @@ -1,3 +1,4 @@ > # SPDX-License-Identifier: GPL-2.0 > > -obj-y := sprd_drm.o > +obj-y := sprd_drm.o \ > + sprd_dpu.o > diff --git a/drivers/gpu/drm/sprd/sprd_dpu.c b/drivers/gpu/drm/sprd/sprd_dpu.c > new file mode 100644 > index 0..e74c3dbb3 > --- /dev/null > +++ b/drivers/gpu/drm/sprd/sprd_dpu.c > @@ -0,0 +1,939 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2020 Unisoc Inc. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "sprd_drm.h" > +#include "sprd_dpu.h" > + > +/* Global control registers */ > +#define REG_DPU_CTRL 0x04 > +#define REG_DPU_CFG0 0x08 > +#define REG_PANEL_SIZE 0x20 > +#define REG_BLEND_SIZE 0x24 > +#define REG_BG_COLOR 0x2C > + > +/* Layer0 control registers */ > +#define REG_LAY_BASE_ADDR0 0x30 > +#define REG_LAY_BASE_ADDR1 0x34 > +#define REG_LAY_BASE_ADDR2 0x38 > +#define REG_LAY_CTRL 0x40 > +#define REG_LAY_SIZE 0x44 > +#define REG_LAY_PITCH0x48 > +#define REG_LAY_POS 0x4C > +#define REG_LAY_ALPHA0x50 > +#define REG_LAY_CROP_START 0x5C > + > +/* Interrupt control registers */ > +#define REG_DPU_INT_EN 0x1E0 > +#define REG_DPU_INT_CLR 0x1E4 > +#define REG_DPU_INT_STS 0x1E8 > + > +/* DPI control registers */ > +#define REG_DPI_CTRL 0x1F0 > +#define REG_DPI_H_TIMING 0x1F4 > +#define REG_DPI_V_TIMING 0x1F8 > + > +/* MMU control registers */ > +#define REG_MMU_EN 0x800 > +#define REG_MMU_VPN_RANGE0x80C > +#define REG_MMU_VAOR_ADDR_RD 0x818 > +#define REG_MMU_VAOR_ADDR_WR 0x81C > +#define REG_MMU_INV_ADDR_RD 0x820 > +#define REG_MMU_INV_ADDR_WR 0x824 > +#define REG_MMU_PPN1 0x83C > +#define REG_MMU_RANGE1 0x840 > +#define REG_MMU_PPN2 0x844 > +#define REG_MMU_RANGE2 0x848 > + > +/* Global control bits */ > +#define BIT_DPU_RUN BIT(0) > +#define BIT_DPU_STOP BIT(1) > +#define BIT_DPU_REG_UPDATE BIT(2) > +#define BIT_DPU_IF_EDPI BIT(0) > + > +/* Layer control bits */ > +#define BIT_DPU_LAY_EN BIT(0) > +#define BIT_DPU_LAY_LAYER_ALPHA (0x01 << 2) > +#define BIT_DPU_LAY_COMBO_ALPHA (0x02 << 2) > +#define BIT_DPU_LAY_FORMAT_YUV422_2PLANE (0x00 << 4) > +#define BIT_DPU_LAY_FORMAT_YUV420_2PLANE (0x01 << 4) > +#define BIT_DPU_LAY_FORMAT_YUV420_3PLANE (0x02 << 4) > +#define BIT_DPU_LAY_FORMAT_ARGB (0x03 << 4) > +#define BIT_DPU_LAY_FORM
Re: [PATCH v5 6/6] drm/sprd: add Unisoc's drm mipi dsi&dphy driver
Hi,
On Sun, Apr 25, 2021 at 08:36:07PM +0800, Kevin Tang wrote:
> Adds dsi host controller support for the Unisoc's display subsystem.
> Adds dsi phy support for the Unisoc's display subsystem.
> Only MIPI DSI Displays supported, DP/TV/HMDI will be support
> in the feature.
>
> v1:
> - Remove dphy and dsi graph binding, merge the dphy driver into the dsi.
>
> v2:
> - Use drm_xxx to replace all DRM_XXX.
> - Use kzalloc to replace devm_kzalloc for sprd_dsi structure init.
>
> v4:
> - Use drmm_helpers to allocate encoder.
> - Move allocate encoder and connector to bind function.
>
> v5:
> - Drop the dsi ip file prefix.
> - Fix the checkpatch warnings.
> - Add Signed-off-by for dsi&dphy patch.
> - Use the mode_flags of mipi_dsi_device to setup crtc DPI and EDPI
> mode.
>
> Cc: Orson Zhai
> Cc: Chunyan Zhang
> Signed-off-by: Kevin Tang
Output from checkpatch:
total: 0 errors, 3 warnings, 100 checks, 4207 lines checked
> ---
> drivers/gpu/drm/sprd/Kconfig |1 +
> drivers/gpu/drm/sprd/Makefile|6 +-
> drivers/gpu/drm/sprd/dsi_ctrl.c | 794 ++
> drivers/gpu/drm/sprd/dsi_ctrl.h | 1475 ++
> drivers/gpu/drm/sprd/dsi_ctrl_ppi.c | 157 +++
> drivers/gpu/drm/sprd/dsi_ctrl_ppi.h | 26 +
> drivers/gpu/drm/sprd/megacores_pll.c | 317 ++
> drivers/gpu/drm/sprd/megacores_pll.h | 146 +++
> drivers/gpu/drm/sprd/sprd_dpu.c | 17 +
> drivers/gpu/drm/sprd/sprd_drm.c |1 +
> drivers/gpu/drm/sprd/sprd_drm.h |1 +
> drivers/gpu/drm/sprd/sprd_dsi.c | 1124
> drivers/gpu/drm/sprd/sprd_dsi.h | 107 ++
> 13 files changed, 4171 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/sprd/dsi_ctrl.c
> create mode 100644 drivers/gpu/drm/sprd/dsi_ctrl.h
> create mode 100644 drivers/gpu/drm/sprd/dsi_ctrl_ppi.c
> create mode 100644 drivers/gpu/drm/sprd/dsi_ctrl_ppi.h
> create mode 100644 drivers/gpu/drm/sprd/megacores_pll.c
> create mode 100644 drivers/gpu/drm/sprd/megacores_pll.h
> create mode 100644 drivers/gpu/drm/sprd/sprd_dsi.c
> create mode 100644 drivers/gpu/drm/sprd/sprd_dsi.h
>
> diff --git a/drivers/gpu/drm/sprd/Kconfig b/drivers/gpu/drm/sprd/Kconfig
> index 37762c333..3edeaeca0 100644
> --- a/drivers/gpu/drm/sprd/Kconfig
> +++ b/drivers/gpu/drm/sprd/Kconfig
> @@ -5,6 +5,7 @@ config DRM_SPRD
> select DRM_GEM_CMA_HELPER
> select DRM_KMS_CMA_HELPER
> select DRM_KMS_HELPER
> + select DRM_MIPI_DSI
> select VIDEOMODE_HELPERS
> help
> Choose this option if you have a Unisoc chipset.
> diff --git a/drivers/gpu/drm/sprd/Makefile b/drivers/gpu/drm/sprd/Makefile
> index ab12b95e6..d49f4977b 100644
> --- a/drivers/gpu/drm/sprd/Makefile
> +++ b/drivers/gpu/drm/sprd/Makefile
> @@ -1,4 +1,8 @@
> # SPDX-License-Identifier: GPL-2.0
>
> obj-y := sprd_drm.o \
> - sprd_dpu.o
> + sprd_dpu.o \
> + sprd_dsi.o \
> + dw_dsi_ctrl.o \
> + dw_dsi_ctrl_ppi.o \
> + megacores_pll.o
> diff --git a/drivers/gpu/drm/sprd/dsi_ctrl.c b/drivers/gpu/drm/sprd/dsi_ctrl.c
> new file mode 100644
> index 0..7eccf9654
> --- /dev/null
> +++ b/drivers/gpu/drm/sprd/dsi_ctrl.c
> @@ -0,0 +1,794 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Unisoc Inc.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include "dsi_ctrl.h"
> +
> +/*
> + * Modify power status of DSI Host core
> + */
> +void dsi_power_enable(struct dsi_context *ctx, int enable)
> +{
> + struct dsi_reg *reg = (struct dsi_reg *)ctx->base;
> +
> + writel(enable, ®->SOFT_RESET);
> +}
> +/*
> + * Enable/disable DPI video mode
> + */
> +void dsi_video_mode(struct dsi_context *ctx)
> +{
> + struct dsi_reg *reg = (struct dsi_reg *)ctx->base;
> +
> + writel(0, ®->DSI_MODE_CFG);
> +}
> +/*
> + * Enable command mode (Generic interface)
> + */
> +void dsi_cmd_mode(struct dsi_context *ctx)
> +{
> + struct dsi_reg *reg = (struct dsi_reg *)ctx->base;
> +
> + writel(1, ®->DSI_MODE_CFG);
> +}
> +
> +bool dsi_is_cmd_mode(struct dsi_context *ctx)
> +{
> + struct dsi_reg *reg = (struct dsi_reg *)ctx->base;
> +
> + return readl(®->DSI_MODE_CFG);
> +}
> +/*
> + * Configure the read back virtual channel for the generic interface
> + */
> +void dsi_rx_vcid(struct dsi_context *ctx, u8 vc)
> +{
> + struct dsi_reg *reg = (struct dsi_reg *)ctx->base;
> + union _0x1C virtual_channel_id;
> +
> + virtual_channel_id.val = readl(®->VIRTUAL_CHANNEL_ID);
> + virtual_channel_id.bits.gen_rx_vcid = vc;
> +
> + writel(virtual_channel_id.val, ®->VIRTUAL_CHANNEL_ID);
> +}
> +/*
> + * Write the DPI video virtual channel destination
> + */
> +void dsi_video_vcid(struct dsi_context *ctx, u8 vc)
> +{
> + struct dsi_reg *reg = (struct dsi_reg *)ctx->base;
> + union _0x1C virtual_channel_id;
> +
> + virtual_channel_id.val = readl(®->VIRTUAL_CHANNEL_ID);
> + virtual_channel_id.bits.vi
[PATCH v3 1/5] drm/connector: Create a helper to attach the hdr_output_metadata property
All the drivers that implement HDR output call pretty much the same
function to initialise the hdr_output_metadata property, and while the
creation of that property is in a helper, every driver uses the same
code to attach it.
Provide a helper for it as well
Reviewed-by: Harry Wentland
Reviewed-by: Jernej Skrabec
Signed-off-by: Maxime Ripard
---
Changes from v2:
- Rebased on current drm-misc-next
- Fixed a merge conflict with i915
Changes from v1:
- Rebased on latest drm-misc-next tag
- Added the tags
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 +--
drivers/gpu/drm/drm_connector.c | 21 +++
drivers/gpu/drm/i915/display/intel_hdmi.c | 3 +--
include/drm/drm_connector.h | 1 +
5 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index a0c8c41e4e57..c8d7e7dbc05e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7498,9 +7498,7 @@ void amdgpu_dm_connector_init_helper(struct
amdgpu_display_manager *dm,
if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
connector_type == DRM_MODE_CONNECTOR_eDP) {
- drm_object_attach_property(
- &aconnector->base.base,
- dm->ddev->mode_config.hdr_output_metadata_property, 0);
+
drm_connector_attach_hdr_output_metadata_property(&aconnector->base);
if (!aconnector->mst_port)
drm_connector_attach_vrr_capable_property(&aconnector->base);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ae97513ef886..dd7f6eda2ce2 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2492,8 +2492,7 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
drm_connector_attach_max_bpc_property(connector, 8, 16);
if (hdmi->version >= 0x200a && hdmi->plat_data->use_drm_infoframe)
- drm_object_attach_property(&connector->base,
-
connector->dev->mode_config.hdr_output_metadata_property, 0);
+ drm_connector_attach_hdr_output_metadata_property(connector);
drm_connector_attach_encoder(connector, hdmi->bridge.encoder);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index eab8c0b82de2..c5e2f642acd9 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2151,6 +2151,27 @@ int drm_connector_attach_max_bpc_property(struct
drm_connector *connector,
}
EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
+/**
+ * drm_connector_attach_hdr_output_metadata_property - attach
"HDR_OUTPUT_METADA" property
+ * @connector: connector to attach the property on.
+ *
+ * This is used to allow the userspace to send HDR Metadata to the
+ * driver.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_hdr_output_metadata_property(struct drm_connector
*connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_property *prop =
dev->mode_config.hdr_output_metadata_property;
+
+ drm_object_attach_property(&connector->base, prop, 0);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+
/**
* drm_connector_set_vrr_capable_property - sets the variable refresh rate
* capable property for a connector
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 9c172dd6fb5b..3c767bcc47b1 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2459,8 +2459,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi,
struct drm_connector *c
drm_connector_attach_content_type_property(connector);
if (DISPLAY_VER(dev_priv) >= 10)
- drm_object_attach_property(&connector->base,
-
connector->dev->mode_config.hdr_output_metadata_property, 0);
+ drm_connector_attach_hdr_output_metadata_property(connector);
if (!HAS_GMCH(dev_priv))
drm_connector_attach_max_bpc_property(connector, 8, 12);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 1922b278ffad..32172dab8427 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1671,6 +1671,7 @@ int drm_connector_attach_scaling_mode_property(struct
drm_connector *connector,
u32 scaling_mode_mask
[PATCH v3 2/5] drm/connector: Add helper to compare HDR metadata
All the drivers that support the HDR metadata property have a similar
function to compare the metadata from one connector state to the next,
and force a mode change if they differ.
All these functions run pretty much the same code, so let's turn it into
an helper that can be shared across those drivers.
Reviewed-by: Harry Wentland
Reviewed-by: Jernej Skrabec
Signed-off-by: Maxime Ripard
---
Changes from v2:
- Rebased on current drm-misc-next
Changes from v1:
- Rebased on latest drm-misc-next tag
- Added the tags
- Fix build breakage on amdgpu
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++-
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +--
drivers/gpu/drm/drm_connector.c | 28 +++
drivers/gpu/drm/i915/display/intel_atomic.c | 13 +
include/drm/drm_connector.h | 2 ++
5 files changed, 34 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c8d7e7dbc05e..296704ce3768 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6275,25 +6275,6 @@ static int fill_hdr_info_packet(const struct
drm_connector_state *state,
return 0;
}
-static bool
-is_hdr_metadata_different(const struct drm_connector_state *old_state,
- const struct drm_connector_state *new_state)
-{
- struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
- struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
-
- if (old_blob != new_blob) {
- if (old_blob && new_blob &&
- old_blob->length == new_blob->length)
- return memcmp(old_blob->data, new_blob->data,
- old_blob->length);
-
- return true;
- }
-
- return false;
-}
-
static int
amdgpu_dm_connector_atomic_check(struct drm_connector *conn,
struct drm_atomic_state *state)
@@ -6311,7 +6292,7 @@ amdgpu_dm_connector_atomic_check(struct drm_connector
*conn,
if (!crtc)
return 0;
- if (is_hdr_metadata_different(old_con_state, new_con_state)) {
+ if (!drm_connector_atomic_hdr_metadata_equal(old_con_state,
new_con_state)) {
struct dc_info_packet hdr_infopacket;
ret = fill_hdr_info_packet(new_con_state, &hdr_infopacket);
@@ -8803,7 +8784,7 @@ static void amdgpu_dm_atomic_commit_tail(struct
drm_atomic_state *state)
dm_old_crtc_state->abm_level;
hdr_changed =
- is_hdr_metadata_different(old_con_state, new_con_state);
+ !drm_connector_atomic_hdr_metadata_equal(old_con_state,
new_con_state);
if (!scaling_changed && !abm_changed && !hdr_changed)
continue;
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index dd7f6eda2ce2..e7c7c9b9c646 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2395,21 +2395,6 @@ static int dw_hdmi_connector_get_modes(struct
drm_connector *connector)
return ret;
}
-static bool hdr_metadata_equal(const struct drm_connector_state *old_state,
- const struct drm_connector_state *new_state)
-{
- struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
- struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
-
- if (!old_blob || !new_blob)
- return old_blob == new_blob;
-
- if (old_blob->length != new_blob->length)
- return false;
-
- return !memcmp(old_blob->data, new_blob->data, old_blob->length);
-}
-
static int dw_hdmi_connector_atomic_check(struct drm_connector *connector,
struct drm_atomic_state *state)
{
@@ -2423,7 +2408,7 @@ static int dw_hdmi_connector_atomic_check(struct
drm_connector *connector,
if (!crtc)
return 0;
- if (!hdr_metadata_equal(old_state, new_state)) {
+ if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
crtc_state = drm_atomic_get_crtc_state(state, crtc);
if (IS_ERR(crtc_state))
return PTR_ERR(crtc_state);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index c5e2f642acd9..eed9cd05c94e 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2172,6 +2172,34 @@ int
drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
}
EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+/**
+ * drm_connec
[PATCH v3 3/5] drm/vc4: Add HDR metadata property to the VC5 HDMI connectors
From: Dave Stevenson
Now that we can export deeper colour depths, add in the signalling
for HDR metadata.
Signed-off-by: Dave Stevenson
Signed-off-by: Maxime Ripard
---
Changes from v2:
- Rebased on current drm-misc-next
Changes from v1:
- Rebased on latest drm-misc-next tag
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 53 ++
drivers/gpu/drm/vc4/vc4_hdmi.h | 3 ++
2 files changed, 56 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 1fda574579af..a33fa1662588 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -214,6 +214,31 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
return ret;
}
+static int vc4_hdmi_connector_atomic_check(struct drm_connector *connector,
+ struct drm_atomic_state *state)
+{
+ struct drm_connector_state *old_state =
+ drm_atomic_get_old_connector_state(state, connector);
+ struct drm_connector_state *new_state =
+ drm_atomic_get_new_connector_state(state, connector);
+ struct drm_crtc *crtc = new_state->crtc;
+
+ if (!crtc)
+ return 0;
+
+ if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
+ struct drm_crtc_state *crtc_state;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ crtc_state->mode_changed = true;
+ }
+
+ return 0;
+}
+
static void vc4_hdmi_connector_reset(struct drm_connector *connector)
{
struct vc4_hdmi_connector_state *old_state =
@@ -263,6 +288,7 @@ static const struct drm_connector_funcs
vc4_hdmi_connector_funcs = {
static const struct drm_connector_helper_funcs vc4_hdmi_connector_helper_funcs
= {
.get_modes = vc4_hdmi_connector_get_modes,
+ .atomic_check = vc4_hdmi_connector_atomic_check,
};
static int vc4_hdmi_connector_init(struct drm_device *dev,
@@ -299,6 +325,9 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
connector->interlace_allowed = 1;
connector->doublescan_allowed = 0;
+ if (vc4_hdmi->variant->supports_hdr)
+ drm_connector_attach_hdr_output_metadata_property(connector);
+
drm_connector_attach_encoder(connector, encoder);
return 0;
@@ -432,6 +461,25 @@ static void vc4_hdmi_set_audio_infoframe(struct
drm_encoder *encoder)
vc4_hdmi_write_infoframe(encoder, &frame);
}
+static void vc4_hdmi_set_hdr_infoframe(struct drm_encoder *encoder)
+{
+ struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct drm_connector *connector = &vc4_hdmi->connector;
+ struct drm_connector_state *conn_state = connector->state;
+ union hdmi_infoframe frame;
+
+ if (!vc4_hdmi->variant->supports_hdr)
+ return;
+
+ if (!conn_state->hdr_output_metadata)
+ return;
+
+ if (drm_hdmi_infoframe_set_hdr_metadata(&frame.drm, conn_state))
+ return;
+
+ vc4_hdmi_write_infoframe(encoder, &frame);
+}
+
static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
@@ -444,6 +492,8 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
*/
if (vc4_hdmi->audio.streaming)
vc4_hdmi_set_audio_infoframe(encoder);
+
+ vc4_hdmi_set_hdr_infoframe(encoder);
}
static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
@@ -2102,6 +2152,7 @@ static const struct vc4_hdmi_variant bcm2835_variant = {
.phy_rng_enable = vc4_hdmi_phy_rng_enable,
.phy_rng_disable= vc4_hdmi_phy_rng_disable,
.channel_map= vc4_hdmi_channel_map,
+ .supports_hdr = false,
};
static const struct vc4_hdmi_variant bcm2711_hdmi0_variant = {
@@ -2129,6 +2180,7 @@ static const struct vc4_hdmi_variant
bcm2711_hdmi0_variant = {
.phy_rng_enable = vc5_hdmi_phy_rng_enable,
.phy_rng_disable= vc5_hdmi_phy_rng_disable,
.channel_map= vc5_hdmi_channel_map,
+ .supports_hdr = true,
};
static const struct vc4_hdmi_variant bcm2711_hdmi1_variant = {
@@ -2156,6 +2208,7 @@ static const struct vc4_hdmi_variant
bcm2711_hdmi1_variant = {
.phy_rng_enable = vc5_hdmi_phy_rng_enable,
.phy_rng_disable= vc5_hdmi_phy_rng_disable,
.channel_map= vc5_hdmi_channel_map,
+ .supports_hdr = true,
};
static const struct of_device_id vc4_hdmi_dt_match[] = {
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 3cebd1fd00fc..060bcaefbeb5 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/dri
[PATCH v3 4/5] drm/connector: Add a helper to attach the colorspace property
The intel driver uses the same logic to attach the Colorspace property
in multiple places and we'll need it in vc4 too. Let's move that common
code in a helper.
Signed-off-by: Maxime Ripard
---
Changes from v2:
- Rebased on current drm-misc-next
Changes from v1:
- New patch
---
drivers/gpu/drm/drm_connector.c | 20 +++
.../gpu/drm/i915/display/intel_connector.c| 6 ++
include/drm/drm_connector.h | 1 +
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index eed9cd05c94e..733da42cfd31 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2172,6 +2172,26 @@ int
drm_connector_attach_hdr_output_metadata_property(struct drm_connector *conn
}
EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property);
+/**
+ * drm_connector_attach_colorspace_property - attach "Colorspace" property
+ * @connector: connector to attach the property on.
+ *
+ * This is used to allow the userspace to signal the output colorspace
+ * to the driver.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_colorspace_property(struct drm_connector *connector)
+{
+ struct drm_property *prop = connector->colorspace_property;
+
+ drm_object_attach_property(&connector->base, prop,
DRM_MODE_COLORIMETRY_DEFAULT);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_colorspace_property);
+
/**
* drm_connector_atomic_hdr_metadata_equal - checks if the hdr metadata changed
* @old_state: old connector state to compare
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index d5ceb7bdc14b..9bed1ccecea0 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -282,14 +282,12 @@ void
intel_attach_hdmi_colorspace_property(struct drm_connector *connector)
{
if (!drm_mode_create_hdmi_colorspace_property(connector))
- drm_object_attach_property(&connector->base,
- connector->colorspace_property, 0);
+ drm_connector_attach_colorspace_property(connector);
}
void
intel_attach_dp_colorspace_property(struct drm_connector *connector)
{
if (!drm_mode_create_dp_colorspace_property(connector))
- drm_object_attach_property(&connector->base,
- connector->colorspace_property, 0);
+ drm_connector_attach_colorspace_property(connector);
}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 1f51d73ca715..714d1a01c065 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1671,6 +1671,7 @@ int drm_connector_attach_scaling_mode_property(struct
drm_connector *connector,
u32 scaling_mode_mask);
int drm_connector_attach_vrr_capable_property(
struct drm_connector *connector);
+int drm_connector_attach_colorspace_property(struct drm_connector *connector);
int drm_connector_attach_hdr_output_metadata_property(struct drm_connector
*connector);
bool drm_connector_atomic_hdr_metadata_equal(struct drm_connector_state
*old_state,
struct drm_connector_state
*new_state);
--
2.31.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 5/5] drm/vc4: hdmi: Signal the proper colorimetry info in the infoframe
Our driver while supporting HDR didn't send the proper colorimetry info
in the AVI infoframe.
Let's add the property needed so that the userspace can let us know what
the colorspace is supposed to be.
Signed-off-by: Maxime Ripard
---
Changes from v2:
- Rebased on current drm-misc-next
Changes from v1:
- New patch
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index a33fa1662588..a22e17788074 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -226,7 +226,8 @@ static int vc4_hdmi_connector_atomic_check(struct
drm_connector *connector,
if (!crtc)
return 0;
- if (!drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
+ if (old_state->colorspace != new_state->colorspace ||
+ !drm_connector_atomic_hdr_metadata_equal(old_state, new_state)) {
struct drm_crtc_state *crtc_state;
crtc_state = drm_atomic_get_crtc_state(state, crtc);
@@ -316,6 +317,11 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
if (ret)
return ret;
+ ret = drm_mode_create_hdmi_colorspace_property(connector);
+ if (ret)
+ return ret;
+
+ drm_connector_attach_colorspace_property(connector);
drm_connector_attach_tv_margin_properties(connector);
drm_connector_attach_max_bpc_property(connector, 8, 12);
@@ -424,7 +430,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder
*encoder)
vc4_encoder->limited_rgb_range ?
HDMI_QUANTIZATION_RANGE_LIMITED :
HDMI_QUANTIZATION_RANGE_FULL);
-
+ drm_hdmi_avi_infoframe_colorspace(&frame.avi, cstate);
drm_hdmi_avi_infoframe_bars(&frame.avi, cstate);
vc4_hdmi_write_infoframe(encoder, &frame);
--
2.31.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm/dpu: Delete bonkers code
Hi,
On Fri, Apr 30, 2021 at 10:44:53AM -0700, Stephen Boyd wrote:
> Quoting Rob Clark (2021-04-30 10:17:39)
> > From: Rob Clark
> >
> > dpu_crtc_atomic_flush() was directly poking it's attached planes in a
> > code path that ended up in dpu_plane_atomic_update(), even if the plane
> > was not involved in the current atomic update. While a bit dubious,
> > this worked before because plane->state would always point to something
> > valid. But now using drm_atomic_get_new_plane_state() we could get a
> > NULL state pointer instead, leading to:
> >
> >[ 20.873273] Call trace:
> >[ 20.875740] dpu_plane_atomic_update+0x5c/0xed0
> >[ 20.880311] dpu_plane_restore+0x40/0x88
> >[ 20.884266] dpu_crtc_atomic_flush+0xf4/0x208
> >[ 20.888660] drm_atomic_helper_commit_planes+0x150/0x238
> >[ 20.894014] msm_atomic_commit_tail+0x1d4/0x7a0
> >[ 20.898579] commit_tail+0xa4/0x168
> >[ 20.902102] drm_atomic_helper_commit+0x164/0x178
> >[ 20.906841] drm_atomic_commit+0x54/0x60
> >[ 20.910798] drm_atomic_connector_commit_dpms+0x10c/0x118
> >[ 20.916236] drm_mode_obj_set_property_ioctl+0x1e4/0x440
> >[ 20.921588] drm_connector_property_set_ioctl+0x60/0x88
> >[ 20.926852] drm_ioctl_kernel+0xd0/0x120
> >[ 20.930807] drm_ioctl+0x21c/0x478
> >[ 20.934235] __arm64_sys_ioctl+0xa8/0xe0
> >[ 20.938193] invoke_syscall+0x64/0x130
> >[ 20.941977] el0_svc_common.constprop.3+0x5c/0xe0
> >[ 20.946716] do_el0_svc+0x80/0xa0
> >[ 20.950058] el0_svc+0x20/0x30
> >[ 20.953145] el0_sync_handler+0x88/0xb0
> >[ 20.957014] el0_sync+0x13c/0x140
> >
> > The reason for the codepath seems dubious, the atomic suspend/resume
> > heplers should handle the power-collapse case. If not, the CRTC's
> > atomic_check() should be adding the planes to the atomic update.
> >
> > Reported-by: Stephen Boyd
>
> Maybe better to use swb...@chromium.org for this one.
>
> > Reported-by: John Stultz
> > Fixes: 37418bf14c13 drm: Use state helper instead of the plane state pointer
>
> Should be
>
> Fixes: 37418bf14c13 ("drm: Use state helper instead of the plane state
> pointer")
>
> to match the preferred format.
>
> > Signed-off-by: Rob Clark
>
> Otherwise looks good, thanks.
Thanks for figuring this out, I've applied it with your chromium address
and the proper fixes format.
Maxime
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] clk: Implement a clock request API
Hi Stephen, On Fri, Apr 30, 2021 at 01:59:39PM -0700, Stephen Boyd wrote: > Quoting Maxime Ripard (2021-04-13 03:13:18) > > Hi, > > > > This is a follow-up of the discussion here: > > https://lore.kernel.org/linux-clk/20210319150355.xzw7ikwdaga2dwhv@gilmour/ > > > > This implements a mechanism to raise and lower clock rates based on consumer > > workloads, with an example of such an implementation for the RaspberryPi4 > > HDMI > > controller. > > > > There's a couple of things worth discussing: > > > > - The name is in conflict with clk_request_rate, and even though it feels > > like the right name to me, we should probably avoid any confusion > > > > - The code so far implements a policy of always going for the lowest rate > > possible. While we don't have an use-case for something else, this > > should > > maybe be made more flexible? > > I'm definitely confused how it is different from the > clk_set_rate_exclusive() API and associated > clk_rate_exclusive_get()/clk_rate_exclusive_put(). Can you explain > further the differences in the cover letter here? The exclusive API is meant to prevent the clock rate from changing, allowing a single user to make sure that no other user will be able to change it. What we want here is instead to allow multiple users to be able to express a set of minimum rates and then let the CCF figure out a rate for that clock that matches those constraints (so basically what clk_set_min_rate does), but then does allow for the clock to go back to its initial rate once that constraint is not needed anymore. So I guess it's more akin to clk_set_min_rate with rollback than the exclusive API? Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/7] drm: Completely remove struct drm_device.pdev
On Sun, May 02, 2021 at 12:49:46PM +0200, Thomas Zimmermann wrote: > The pdev field in struct drm_device was recently moved into the legacy > section. Remove it entirely. References are replaced with upcasts from > the structure's dev field. This change only affects legacy drivers for > userspace modesetting. Reviewed-by: Maxime Ripard Thanks! Maxime signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vc4: hdmi: make vc4_hdmi_codec_pdata static
On Fri, Jul 30, 2021 at 06:26:34PM +0800, Jiapeng Chong wrote: > This symbol is not used outside of vc4_hdmi.c, so marks it static. > > Fix the following sparse warning: > > drivers/gpu/drm/vc4/vc4_hdmi.c:1479:25: warning: symbol > 'vc4_hdmi_codec_pdata' was not declared. Should it be static? > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong Applied, thanks Maxime signature.asc Description: PGP signature
Re: [PATCH 10/54] dt-bindings: display: panel-lvds: Document panel compatibles
Hi Rob, Sam,
On Wed, Jul 21, 2021 at 08:29:47PM -0600, Rob Herring wrote:
> On Wed, Jul 21, 2021 at 04:03:40PM +0200, Maxime Ripard wrote:
> > The binding mentions that all the drivers using that driver must use a
> > vendor-specific compatible but never enforces it, nor documents the
> > vendor-specific compatibles.
> >
> > Let's make we document all of them, and that the binding will create an
> > error if we add one that isn't.
> >
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: Laurent Pinchart
> > Cc: Sam Ravnborg
> > Cc: Thierry Reding
> > Signed-off-by: Maxime Ripard
> > ---
> > .../bindings/display/panel/lvds.yaml | 18 --
> > 1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > index 49460c9dceea..d1513111eb48 100644
> > --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > @@ -31,12 +31,18 @@ allOf:
> >
> > properties:
> >compatible:
> > -contains:
> > - const: panel-lvds
> > -description:
> > - Shall contain "panel-lvds" in addition to a mandatory panel-specific
> > - compatible string defined in individual panel bindings. The
> > "panel-lvds"
> > - value shall never be used on its own.
> > +items:
> > + - enum:
> > + - advantech,idk-1110wr
>
> At least this one is documented elsewhere.
Indeed, I missed it.
> You can add 'minItems: 2' if you want to just enforce having 2 compatibles.
> Or do:
>
> items:
> - {}
> - const: panel-lvds
>
> Which also enforces the order.
It's not just about the order since a missing compatible will also raise
a warning.
Some of those panels have a binding of their own, but some probably
won't (and I can't find anything specific about the one I'm most
interested in: tbs,a711-panel)
Can we have something like:
compatible:
oneOf:
- items:
- enum:
- tbs,a711-panel
- const: panel-lvds
- items:
- {}
- const: panel-lvds
That would work for both cases I guess?
Maxime
signature.asc
Description: PGP signature
[PATCH v7 00/10] drm/vc4: hdmi: Support the 4k @ 60Hz modes
Hi, Here is a series that enables the higher resolutions on the HDMI0 Controller found in the BCM2711 (RPi4). In order to work it needs a few adjustments to config.txt, most notably to enable the enable_hdmi_4kp60 option. Let me know what you think, Maxime --- Changes from v6: - Rebased on current drm-misc-next - Removed stale clk_request pointer Changes from v5: - Fixed unused variables warning Changes from v4: - Removed the patches already applied - Added various fixes for the issues that have been discovered on the downstream tree Changes from v3: - Rework the encoder retrieval code that was broken on the RPi3 and older - Fix a scrambling enabling issue on some display Changes from v2: - Gathered the various tags - Added Cc stable when relevant - Split out the check to test whether the scrambler is required into an helper - Fixed a bug where the scrambler state wouldn't be tracked properly if it was enabled at boot Changes from v1: - Dropped the range accessors - Drop the mention of force_turbo - Reordered the SCRAMBLER_CTL register to match the offset - Removed duplicate HDMI_14_MAX_TMDS_CLK define - Warn about enable_hdmi_4kp60 only if there's some modes that can't be reached - Rework the BVB clock computation Maxime Ripard (10): drm/vc4: hdmi: Remove the DDC probing for status detection drm/vc4: hdmi: Fix HPD GPIO detection drm/vc4: Make vc4_crtc_get_encoder public drm/vc4: crtc: Add encoder to vc4_crtc_config_pv prototype drm/vc4: crtc: Rework the encoder retrieval code (again) drm/vc4: crtc: Add some logging drm/vc4: Leverage the load tracker on the BCM2711 drm/vc4: hdmi: Raise the maximum clock rate drm/vc4: hdmi: Enable the scrambler on reconnection drm/vc4: Increase the core clock based on HVS load drivers/gpu/drm/vc4/vc4_crtc.c| 60 -- drivers/gpu/drm/vc4/vc4_debugfs.c | 7 +- drivers/gpu/drm/vc4/vc4_drv.h | 8 +- drivers/gpu/drm/vc4/vc4_hdmi.c| 20 +++-- drivers/gpu/drm/vc4/vc4_kms.c | 126 +- drivers/gpu/drm/vc4/vc4_plane.c | 5 -- 6 files changed, 163 insertions(+), 63 deletions(-) -- 2.31.1
[PATCH v7 01/10] drm/vc4: hdmi: Remove the DDC probing for status detection
Commit 9d44a8d5 ("drm/vc4: Fall back to using an EDID probe in the
absence of a GPIO.") added some code to read the EDID through DDC in the
HDMI driver detect hook since the Pi3 had no HPD GPIO back then.
However, commit b1b8f45b3130 ("ARM: dts: bcm2837: Add missing GPIOs of
Expander") changed that a couple of years later.
This causes an issue though since some TV (like the LG 55C8) when it
comes out of standy will deassert the HPD line, but the EDID will
remain readable.
It causes an issues nn platforms without an HPD GPIO, like the Pi4,
where the DDC probing will be our primary mean to detect a display, and
thus we will never detect the HPD pulse. This was fine before since the
pulse was small enough that we would never detect it, and we also didn't
have anything (like the scrambler) that needed to be set up in the
display.
However, now that we have both, the display during the HPD pulse will
clear its scrambler status, and since we won't detect the
disconnect/reconnect cycle we will never enable the scrambler back.
As our main reason for that DDC probing is gone, let's just remove it.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index b7dc32a0c9bb..f9a672a641ab 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -172,8 +172,6 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
if (vc4_hdmi->hpd_gpio &&
gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
connected = true;
- } else if (drm_probe_ddc(vc4_hdmi->ddc)) {
- connected = true;
} else if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) {
connected = true;
}
--
2.31.1
[PATCH v7 03/10] drm/vc4: Make vc4_crtc_get_encoder public
We'll need that function in vc4_kms to compute the core clock rate
requirements.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_crtc.c | 8
drivers/gpu/drm/vc4/vc4_drv.h | 5 +
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 18f5009ce90e..902862a67341 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -279,10 +279,10 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct
vc4_crtc *vc4_crtc,
* allows drivers to push pixels to more than one encoder from the
* same CRTC.
*/
-static struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
- struct drm_atomic_state *state,
- struct drm_connector_state
*(*get_state)(struct drm_atomic_state *state,
-
struct drm_connector *connector))
+struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
+struct drm_atomic_state *state,
+struct drm_connector_state
*(*get_state)(struct drm_atomic_state *state,
+
struct drm_connector *connector))
{
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index ef73e0aaf726..0865f05822e0 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -517,6 +517,11 @@ vc4_crtc_to_vc4_pv_data(const struct vc4_crtc *crtc)
return container_of(data, struct vc4_pv_data, base);
}
+struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
+struct drm_atomic_state *state,
+struct drm_connector_state
*(*get_state)(struct drm_atomic_state *state,
+
struct drm_connector *connector));
+
struct vc4_crtc_state {
struct drm_crtc_state base;
/* Dlist area for this CRTC configuration. */
--
2.31.1
[PATCH v7 02/10] drm/vc4: hdmi: Fix HPD GPIO detection
Prior to commit 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod"), in the
detect hook, if we had an HPD GPIO we would only rely on it and return
whatever state it was in.
However, that commit changed that by mistake to only consider the case
where we have a GPIO and it returns a logical high, and would fall back
to the other methods otherwise.
Since we can read the EDIDs when the HPD signal is low on some displays,
we changed the detection status from disconnected to connected, and we
would ignore an HPD pulse.
Fixes: 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod")
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index f9a672a641ab..251dfecf1d4c 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -169,9 +169,9 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev));
- if (vc4_hdmi->hpd_gpio &&
- gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
- connected = true;
+ if (vc4_hdmi->hpd_gpio) {
+ if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio))
+ connected = true;
} else if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) {
connected = true;
}
--
2.31.1
