Rob,
Signed-off-by: Rob Clark
drivers/gpu/drm/bridge/ptn3460.c | 39 +++++++++++++++++++++++++++++++++------ include/drm/bridge/ptn3460.h | 6 ++++-- 2 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c index b171901..e3e6b46 100644 --- a/drivers/gpu/drm/bridge/ptn3460.c +++ b/drivers/gpu/drm/bridge/ptn3460.c @@ -26,6 +26,7 @@ #include "drm_crtc_helper.h"
#include "bridge/ptn3460.h" +#include "drm_bridge_util.h"
#define PTN3460_EDID_ADDR 0x0 #define PTN3460_EDID_EMULATION_ADDR 0x84 @@ -112,7 +113,6 @@ static int ptn3460_select_edid(struct ptn3460_bridge *ptn_bridge) static void ptn3460_pre_enable(struct drm_bridge *bridge) { struct ptn3460_bridge *ptn_bridge = bridge->driver_private;
- int ret;
if (ptn_bridge->enabled) return; @@ -132,6 +132,15 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
- time specified in the chip's datasheet to make sure we're really up.
*/ msleep(90); +}
+static void ptn3460_enable(struct drm_bridge *bridge) +{
- struct ptn3460_bridge *ptn_bridge = bridge->driver_private;
- int ret;
- if (ptn_bridge->enabled)
- return;
No need to move it into enable. We should keep this entirely inside pre_enable itself.
ret = ptn3460_select_edid(ptn_bridge); if (ret) @@ -140,10 +149,6 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) ptn_bridge->enabled = true; }
-static void ptn3460_enable(struct drm_bridge *bridge) -{ -}
static void ptn3460_disable(struct drm_bridge *bridge) { struct ptn3460_bridge *ptn_bridge = bridge->driver_private; @@ -265,7 +270,8 @@ struct drm_connector_funcs ptn3460_connector_funcs = { };
int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
- struct i2c_client *client, struct device_node *node)
- struct i2c_client *client, struct device_node *node,
- struct drm_panel *panel)
{ int ret; struct drm_bridge *bridge; @@ -324,6 +330,27 @@ int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, goto err; }
- if (panel) {
- struct drm_bridge *cbridge, *pbridge;
- pbridge = drm_panel_bridge_init(panel);
- if (IS_ERR(pbridge)) {
- ret = PTR_ERR(pbridge);
- goto err;
- }
- /* panel sequence is after ptn4360 bridge bridge in
- enable path, before in disable path:
- */
- cbridge = drm_composite_bridge_init(bridge, pbridge);
- if (IS_ERR(cbridge)) {
- ret = PTR_ERR(cbridge);
- goto err;
- }
- bridge = cbridge;
- }
We cannot have this here. Because, at this point the drm_panel probe would not have happened and the drm_panel would not have got added into the panel_list itself!
Instead, the panel discovery should be moved into "detect" callback of the ptn3460 connector. I tried this, and it causes one more problem. We cannot make a call to drm_panel_bridge_init/drm_composite_bridge_init sicne both of them call "drm_bridge_init", and drm_bridge_init tries to acquire dev->mode_config.mutex via drm_modeset_lock_all(dev), and the same lock is already held by drm core!
I could somehow test your patchset on a board with ptn3460 by commenting the locking part inside drm_bridge_init, and it works only with all these hacks.
bridge->driver_private = ptn_bridge; encoder->bridge = bridge;
diff --git a/include/drm/bridge/ptn3460.h b/include/drm/bridge/ptn3460.h index ff62344..9a557da 100644 --- a/include/drm/bridge/ptn3460.h +++ b/include/drm/bridge/ptn3460.h @@ -16,18 +16,20 @@
struct drm_device; struct drm_encoder; +struct drm_panel; struct i2c_client; struct device_node;
#if defined(CONFIG_DRM_PTN3460) || defined(CONFIG_DRM_PTN3460_MODULE)
int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
- struct i2c_client *client, struct device_node *node);
- struct i2c_client *client, struct device_node *node,
- struct drm_panel *panel);
#else
static inline int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, struct i2c_client *client,
- struct device_node *node)
- struct device_node *node, struct drm_panel *panel)
{ return 0; } -- 1.9.0