Rob, On Tue, Apr 22, 2014 at 5:04 PM, Rob Clark <robdclark@xxxxxxxxx> wrote: > So what about, rather than adding drm_panel support to each bridge > individually, we introduce a drm_panel_bridge (with a form of > chaining).. ie: > > struct drm_panel_bridge { > struct drm_bridge base; > struct drm_panel *panel; > struct drm_bridge *bridge; /* optional */ > }; > > static void drm_panel_bridge_enable(struct drm_bridge *bridge) > { > struct drm_panel_bridge *pb = to_panel_bridge(bridge); > if (pb->bridge) > pb->bridge->funcs->enable(pb->bridge); > pb->panel->funcs->enable(pb->panel); > } > We cannot call them like this from crtc helpers in the order you mentioned, since each individual bridge chip expects the panel controls at different places. I mean, sometimes panel controls needs to be done before bridge controls, > ... and so on. > > If you don't need a real bridge, just create one of these w/ > pb->bridge==NULL, otherwise create it as a wrapper for your real > bridge. > > BR, > -R > > On Mon, Apr 21, 2014 at 6:39 PM, Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx> wrote: >> attach ptn3460 connector to drm_panel and support drm_panel routines, >> if a valid drm_panel object is passed to ptn3460_init. >> >> Signed-off-by: Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx> >> --- >> Changes since V1: >> Address few coding style comments from Jingoo Han >> >> drivers/gpu/drm/bridge/Kconfig | 1 + >> drivers/gpu/drm/bridge/ptn3460.c | 20 +++++++++++++++++++- >> drivers/gpu/drm/exynos/exynos_dp_core.c | 16 ++++++++++++---- >> include/drm/bridge/ptn3460.h | 6 ++++-- >> 4 files changed, 36 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig >> index 884923f..3bc6845 100644 >> --- a/drivers/gpu/drm/bridge/Kconfig >> +++ b/drivers/gpu/drm/bridge/Kconfig >> @@ -2,4 +2,5 @@ config DRM_PTN3460 >> tristate "PTN3460 DP/LVDS bridge" >> depends on DRM >> select DRM_KMS_HELPER >> + select DRM_PANEL >> ---help--- >> diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c >> index f1d2afc..3920202 100644 >> --- a/drivers/gpu/drm/bridge/ptn3460.c >> +++ b/drivers/gpu/drm/bridge/ptn3460.c >> @@ -19,6 +19,7 @@ >> #include <linux/i2c.h> >> #include <linux/gpio.h> >> #include <linux/delay.h> >> +#include <drm/drm_panel.h> >> >> #include "drmP.h" >> #include "drm_edid.h" >> @@ -38,6 +39,7 @@ struct ptn3460_bridge { >> struct i2c_client *client; >> struct drm_encoder *encoder; >> struct drm_bridge *bridge; >> + struct drm_panel *panel; >> struct edid *edid; >> int gpio_pd_n; >> int gpio_rst_n; >> @@ -126,6 +128,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) >> gpio_set_value(ptn_bridge->gpio_rst_n, 1); >> } >> >> + drm_panel_pre_enable(ptn_bridge->panel); >> + >> /* >> * There's a bug in the PTN chip where it falsely asserts hotplug before >> * it is fully functional. We're forced to wait for the maximum start up >> @@ -142,6 +146,10 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) >> >> static void ptn3460_enable(struct drm_bridge *bridge) >> { >> + struct ptn3460_bridge *ptn_bridge = bridge->driver_private; >> + >> + if (ptn_bridge->enabled) >> + drm_panel_enable(ptn_bridge->panel); >> } >> >> static void ptn3460_disable(struct drm_bridge *bridge) >> @@ -153,6 +161,9 @@ static void ptn3460_disable(struct drm_bridge *bridge) >> >> ptn_bridge->enabled = false; >> >> + drm_panel_disable(ptn_bridge->panel); >> + drm_panel_post_disable(ptn_bridge->panel); >> + >> if (gpio_is_valid(ptn_bridge->gpio_rst_n)) >> gpio_set_value(ptn_bridge->gpio_rst_n, 1); >> >> @@ -198,6 +209,7 @@ int ptn3460_get_modes(struct drm_connector *connector) >> >> power_off = !ptn_bridge->enabled; >> ptn3460_pre_enable(ptn_bridge->bridge); >> + ptn3460_enable(ptn_bridge->bridge); >> >> edid = kmalloc(EDID_LENGTH, GFP_KERNEL); >> if (!edid) { >> @@ -265,7 +277,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 +337,11 @@ int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder, >> goto err; >> } >> >> + if (panel) { >> + ptn_bridge->panel = panel; >> + drm_panel_attach(ptn_bridge->panel, &ptn_bridge->connector); >> + } >> + >> bridge->driver_private = ptn_bridge; >> encoder->bridge = bridge; >> ptn_bridge->connector.polled = DRM_CONNECTOR_POLL_HPD; >> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c >> index dbc5ccc..4853f31 100644 >> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c >> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c >> @@ -989,13 +989,14 @@ static bool find_bridge(const char *compat, struct bridge_init *bridge) >> >> /* returns the number of bridges attached */ >> static int exynos_drm_attach_lcd_bridge(struct drm_device *dev, >> - struct drm_encoder *encoder) >> + struct drm_encoder *encoder, struct drm_panel *panel) >> { >> struct bridge_init bridge; >> int ret; >> >> if (find_bridge("nxp,ptn3460", &bridge)) { >> - ret = ptn3460_init(dev, encoder, bridge.client, bridge.node); >> + ret = ptn3460_init(dev, encoder, bridge.client, bridge.node, >> + panel); >> if (!ret) >> return 1; >> } >> @@ -1012,9 +1013,16 @@ static int exynos_dp_create_connector(struct exynos_drm_display *display, >> dp->encoder = encoder; >> >> /* Pre-empt DP connector creation if there's a bridge */ >> - ret = exynos_drm_attach_lcd_bridge(dp->drm_dev, encoder); >> - if (ret) >> + ret = exynos_drm_attach_lcd_bridge(dp->drm_dev, encoder, dp->drm_panel); >> + if (ret) { >> + /* >> + * Also set "dp->drm_panel = NULL" so that we don't end up >> + * controlling panel power both in exynos_dp and bridge >> + * DPMS routines. >> + */ >> + dp->drm_panel = NULL; >> return 0; >> + } >> >> connector->polled = DRM_CONNECTOR_POLL_HPD; >> >> diff --git a/include/drm/bridge/ptn3460.h b/include/drm/bridge/ptn3460.h >> index ff62344..570cebb 100644 >> --- a/include/drm/bridge/ptn3460.h >> +++ b/include/drm/bridge/ptn3460.h >> @@ -18,16 +18,18 @@ struct drm_device; >> struct drm_encoder; >> struct i2c_client; >> struct device_node; >> +struct drm_panel; >> >> #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.7.9.5 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/dri-devel -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html