On 8/22/2022 10:53 AM, Dmitry Baryshkov wrote:
On 15/07/2022 00:54, Abhinav Kumar wrote:
On 7/12/2022 6:22 AM, Dmitry Baryshkov wrote:
Currently the DSI driver has two separate paths: one if the next device
in a chain is a bridge and another one if the panel is connected
directly to the DSI host. Simplify the code path by using panel-bridge
driver (already selected in Kconfig) and dropping support for
handling the panel directly.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
I'm not sending this as a separate patchset (I'd like to sort out mdp5
first), but more of a preview of changes related to
msm_dsi_manager_ext_bridge_init().
---
drivers/gpu/drm/msm/dsi/dsi.c | 35 +---
drivers/gpu/drm/msm/dsi/dsi.h | 16 +-
drivers/gpu/drm/msm/dsi/dsi_host.c | 25 ---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 283 +++-----------------------
4 files changed, 36 insertions(+), 323 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c
b/drivers/gpu/drm/msm/dsi/dsi.c
index 1625328fa430..4edb9167e600 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -6,14 +6,6 @@
#include "dsi.h"
#include "dsi_cfg.h"
-struct drm_encoder *msm_dsi_get_encoder(struct msm_dsi *msm_dsi)
-{
- if (!msm_dsi || !msm_dsi_device_connected(msm_dsi))
- return NULL;
-
- return msm_dsi->encoder;
-}
-
bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi)
{
unsigned long host_flags =
msm_dsi_host_get_mode_flags(msm_dsi->host);
@@ -220,7 +212,6 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi,
struct drm_device *dev,
struct drm_encoder *encoder)
{
struct msm_drm_private *priv;
- struct drm_bridge *ext_bridge;
int ret;
if (WARN_ON(!encoder) || WARN_ON(!msm_dsi) || WARN_ON(!dev))
@@ -254,26 +245,10 @@ int msm_dsi_modeset_init(struct msm_dsi
*msm_dsi, struct drm_device *dev,
goto fail;
}
- /*
- * check if the dsi encoder output is connected to a panel or an
- * external bridge. We create a connector only if we're
connected to a
- * drm_panel device. When we're connected to an external bridge, we
- * assume that the drm_bridge driver will create the connector
itself.
- */
- ext_bridge = msm_dsi_host_get_bridge(msm_dsi->host);
-
- if (ext_bridge)
- msm_dsi->connector =
- msm_dsi_manager_ext_bridge_init(msm_dsi->id);
- else
- msm_dsi->connector =
- msm_dsi_manager_connector_init(msm_dsi->id);
-
- if (IS_ERR(msm_dsi->connector)) {
- ret = PTR_ERR(msm_dsi->connector);
+ ret = msm_dsi_manager_ext_bridge_init(msm_dsi->id);
+ if (ret) {
DRM_DEV_ERROR(dev->dev,
"failed to create dsi connector: %d\n", ret);
- msm_dsi->connector = NULL;
goto fail;
}
@@ -287,12 +262,6 @@ int msm_dsi_modeset_init(struct msm_dsi
*msm_dsi, struct drm_device *dev,
msm_dsi->bridge = NULL;
}
- /* don't destroy connector if we didn't make it */
- if (msm_dsi->connector && !msm_dsi->external_bridge)
- msm_dsi->connector->funcs->destroy(msm_dsi->connector);
-
- msm_dsi->connector = NULL;
From what i can see all the usages of msm_dsi->connector are removed
after this change. So can we drop that?
The connector field is dropped from the msm_dsi struct. If you are
asking about the msm_dsi_modeset_init(), we can not drop it since we
require the DRM device with GEM being initialized in order to allocate
DSI DMA buffer. We can think about moving DMA buffer allocation towards
the usage point, however this is definitely a separate commit.
[skipped]
Yes, got it.
*msm_dsi_manager_ext_bridge_init(u8 id)
ret = drm_bridge_attach(encoder, ext_bridge, int_bridge,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret == -EINVAL) {
- struct drm_connector *connector;
- struct list_head *connector_list;
-
- /* link the internal dsi bridge to the external bridge */
- drm_bridge_attach(encoder, ext_bridge, int_bridge, 0);
-
/*
- * we need the drm_connector created by the external bridge
- * driver (or someone else) to feed it to our driver's
- * priv->connector[] list, mainly for msm_fbdev_init()
+ * link the internal dsi bridge to the external bridge,
+ * connector is created by the next bridge.
*/
- connector_list = &dev->mode_config.connector_list;
+ ret = drm_bridge_attach(encoder, ext_bridge, int_bridge, 0);
+ if (ret < 0)
+ return ret;
+ } else {
+ struct drm_connector *connector;
- list_for_each_entry(connector, connector_list, head) {
- if (drm_connector_has_possible_encoder(connector, encoder))
- return connector;
+ /* We are in charge of the connector, create one now. */
+ connector = drm_bridge_connector_init(dev, encoder);
+ if (IS_ERR(connector)) {
+ DRM_ERROR("Unable to create bridge connector\n");
+ return PTR_ERR(connector);
}
Ok, I understood now. We create the connector using
drm_bridge_connector_init() only when the brige doesnt create one
already.
In both cases since now we are leaving the hpd handling to the next
bridge, like I was suggesting, the dsi_hpd_worker() etc can be dropped
now. Because anyway without setting the DRM_CONNECTOR_POLL_HPD, event
will not be sent to usermode.
I've submitted https://patchwork.freedesktop.org/series/107564/ as a
separate change.
Thanks for pushing this change, I have R-bed that too.
For this one, now I can
Reviewed-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
- return ERR_PTR(-ENODEV);
- }
-
- connector = drm_bridge_connector_init(dev, encoder);
- if (IS_ERR(connector)) {
- DRM_ERROR("Unable to create bridge connector\n");
- return ERR_CAST(connector);
+ ret = drm_connector_attach_encoder(connector, encoder);
+ if (ret < 0)
+ return ret;
}
- drm_connector_attach_encoder(connector, encoder);
+ /* The pipeline is ready, ping encoders if necessary */
+ msm_dsi_manager_set_split_display(id);
- return connector;
+ return 0;
}
void msm_dsi_manager_bridge_destroy(struct drm_bridge *bridge)
