| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20220204161245.v2.1.I3ab26b7f197cc56c874246a43e57913e9c2c1028@changeid>
Date: Fri, 4 Feb 2022 16:13:40 -0800
From: Douglas Anderson <dianders@...omium.org>
To: dri-devel@...ts.freedesktop.org
Cc: Daniel Vetter <daniel@...ll.ch>,
Javier Martinez Canillas <javierm@...hat.com>,
robert.foss@...aro.org, lschyi@...omium.org,
Sam Ravnborg <sam@...nborg.org>, jjsu@...omium.org,
Douglas Anderson <dianders@...omium.org>,
Andrzej Hajda <andrzej.hajda@...el.com>,
David Airlie <airlied@...ux.ie>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Jonas Karlman <jonas@...boo.se>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Neil Armstrong <narmstrong@...libre.com>,
linux-kernel@...r.kernel.org
Subject: [PATCH v2 1/3] drm/bridge: ti-sn65dsi86: Use drm_bridge_connector
The ti-sn65dsi86 driver shouldn't hand-roll its own bridge
connector. It should use the normal drm_bridge_connector. Let's switch
to do that, removing all of the custom code.
NOTE: this still _doesn't_ implement DRM_BRIDGE_ATTACH_NO_CONNECTOR
support for ti-sn65dsi86 and that would still be a useful thing to do
in the future. It was attempted in the past [1] but put on the back
burner. However, unless we instantly change ti-sn65dsi86 fully from
not supporting DRM_BRIDGE_ATTACH_NO_CONNECTOR at all to _only_
supporting DRM_BRIDGE_ATTACH_NO_CONNECTOR then we'll still need a bit
of time when we support both. This is a better way to support the old
way where the driver hand rolls things itself.
A new notes about the implementation here:
* When using the drm_bridge_connector the connector should be created
after all the bridges, so we change the ordering a bit.
* I'm reasonably certain that we don't need to do anything to "free"
the new drm_bridge_connector. If drm_bridge_connector_init() returns
success then we know drm_connector_init() was called with the
`drm_bridge_connector_funcs`. The `drm_bridge_connector_funcs` has a
.destroy() that does all the cleanup. drm_connector_init() calls
__drm_mode_object_add() with a drm_connector_free() that will call
the .destroy().
* I'm also reasonably certain that I don't need to "undo" the
drm_bridge_attach() if drm_bridge_connector_init() fails. The
"detach" function is private and other similar code doesn't try to
undo the drm_bridge_attach() in error cases. There's also a comment
indicating the lack of balance at the top of drm_bridge_attach().
[1] https://lore.kernel.org/r/20210920225801.227211-4-robdclark@gmail.com
Signed-off-by: Douglas Anderson <dianders@...omium.org>
---
Changes in v2:
- ("ti-sn65dsi86: Use drm_bridge_connector") new for v2.
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 72 ++++++---------------------
1 file changed, 14 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index ba136a188be7..38616aab12ac 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -26,6 +26,7 @@
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
#include <drm/dp/drm_dp_aux_bus.h>
#include <drm/dp/drm_dp_helper.h>
#include <drm/drm_mipi_dsi.h>
@@ -174,7 +175,7 @@ struct ti_sn65dsi86 {
struct regmap *regmap;
struct drm_dp_aux aux;
struct drm_bridge bridge;
- struct drm_connector connector;
+ struct drm_connector *connector;
struct device_node *host_node;
struct mipi_dsi_device *dsi;
struct clk *refclk;
@@ -646,54 +647,6 @@ static struct auxiliary_driver ti_sn_aux_driver = {
.id_table = ti_sn_aux_id_table,
};
-/* -----------------------------------------------------------------------------
- * DRM Connector Operations
- */
-
-static struct ti_sn65dsi86 *
-connector_to_ti_sn65dsi86(struct drm_connector *connector)
-{
- return container_of(connector, struct ti_sn65dsi86, connector);
-}
-
-static int ti_sn_bridge_connector_get_modes(struct drm_connector *connector)
-{
- struct ti_sn65dsi86 *pdata = connector_to_ti_sn65dsi86(connector);
-
- return drm_bridge_get_modes(pdata->next_bridge, connector);
-}
-
-static struct drm_connector_helper_funcs ti_sn_bridge_connector_helper_funcs = {
- .get_modes = ti_sn_bridge_connector_get_modes,
-};
-
-static const struct drm_connector_funcs ti_sn_bridge_connector_funcs = {
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = drm_connector_cleanup,
- .reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-static int ti_sn_bridge_connector_init(struct ti_sn65dsi86 *pdata)
-{
- int ret;
-
- ret = drm_connector_init(pdata->bridge.dev, &pdata->connector,
- &ti_sn_bridge_connector_funcs,
- DRM_MODE_CONNECTOR_eDP);
- if (ret) {
- DRM_ERROR("Failed to initialize connector with drm\n");
- return ret;
- }
-
- drm_connector_helper_add(&pdata->connector,
- &ti_sn_bridge_connector_helper_funcs);
- drm_connector_attach_encoder(&pdata->connector, pdata->bridge.encoder);
-
- return 0;
-}
-
/*------------------------------------------------------------------------------
* DRM Bridge
*/
@@ -757,10 +710,6 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge,
return ret;
}
- ret = ti_sn_bridge_connector_init(pdata);
- if (ret < 0)
- goto err_conn_init;
-
/* We never want the next bridge to *also* create a connector: */
flags |= DRM_BRIDGE_ATTACH_NO_CONNECTOR;
@@ -768,13 +717,20 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge,
ret = drm_bridge_attach(bridge->encoder, pdata->next_bridge,
&pdata->bridge, flags);
if (ret < 0)
- goto err_dsi_host;
+ goto err_initted_aux;
+
+ pdata->connector = drm_bridge_connector_init(pdata->bridge.dev,
+ pdata->bridge.encoder);
+ if (IS_ERR(pdata->connector)) {
+ ret = PTR_ERR(pdata->connector);
+ goto err_initted_aux;
+ }
+
+ drm_connector_attach_encoder(pdata->connector, pdata->bridge.encoder);
return 0;
-err_dsi_host:
- drm_connector_cleanup(&pdata->connector);
-err_conn_init:
+err_initted_aux:
drm_dp_aux_unregister(&pdata->aux);
return ret;
}
@@ -824,7 +780,7 @@ static void ti_sn_bridge_set_dsi_rate(struct ti_sn65dsi86 *pdata)
static unsigned int ti_sn_bridge_get_bpp(struct ti_sn65dsi86 *pdata)
{
- if (pdata->connector.display_info.bpc <= 6)
+ if (pdata->connector->display_info.bpc <= 6)
return 18;
else
return 24;
--
2.35.0.263.gb82422642f-goog
Powered by blists - more mailing lists