| Message ID | 1383063198-10526-32-git-send-email-seanpaul@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sean, On Tuesday 29 of October 2013 12:13:17 Sean Paul wrote: > This patch moves the lvds bridge discovery and connector pre-emption > code from exynos common code into the dp driver (since the bridge is > only applicable for dp). > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- > > Changes in v3: > - Added to the patchset > > drivers/gpu/drm/exynos/exynos_dp_core.c | 41 ++++++++++++++++++++++++++++++++ > drivers/gpu/drm/exynos/exynos_drm_core.c | 41 -------------------------------- > 2 files changed, 41 insertions(+), 41 deletions(-) > Well, DRM bridge infrastructure is useful for more than just DP. Currently there are several platforms that could use it with DSI as well, e.g. Trats, Trats2, Arndale. With a simple abstraction worth of one or at most two days of work, we could get something that would cover much more than just a single Chromebook. Best regards, Tomasz
Hi Tomasz, Thanks for review, 2013/11/30 Tomasz Figa <t.figa@samsung.com>: > Hi Sean, > > On Tuesday 29 of October 2013 12:13:17 Sean Paul wrote: >> This patch moves the lvds bridge discovery and connector pre-emption >> code from exynos common code into the dp driver (since the bridge is >> only applicable for dp). >> >> Signed-off-by: Sean Paul <seanpaul@chromium.org> >> --- >> >> Changes in v3: >> - Added to the patchset >> >> drivers/gpu/drm/exynos/exynos_dp_core.c | 41 ++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/exynos/exynos_drm_core.c | 41 -------------------------------- >> 2 files changed, 41 insertions(+), 41 deletions(-) >> > > Well, DRM bridge infrastructure is useful for more than just DP. Currently > there are several platforms that could use it with DSI as well, e.g. In case that some board with DSI bus needs this LVDS bridge, the bridge type would be MIPI-DSI to LVDS Display bridge. So I think moving this lvds stuff into eDP driver is reasonable becasue the normal DSI bus cannot use this LVDS bridge, and this bridge (eDP to LVDS) would be for eDP bus. BTW, Sean, why not implementing this lvds codes to eDP driver without previous related patch? that seems unnecessary work. > Trats, Trats2, Arndale. With a simple abstraction worth of one or at most > two days of work, we could get something that would cover much more than > just a single Chromebook. I also thought a better way is to use simple abstraction layer. But the abstraction layer I thought was the case that lvds binding is done in this lvds driver itself so drm connector should call some bridge related functions of the abstraction layer. But it seems that other maintainers don't agree to this way. :( Anyway, we can say the use of lvds bridge is dependent on board but actually, the use of lvds bridge would be more dependent on bus device or lcd panel attached to the board. So my opinion is to bind dt in the bus or lcd panel drivers that need lvds bridge. If dt binding of lvds bridge cannot be done itself, Sean's way is reasonable to me except the unnecessary patch. Thanks, Inki Dae > > Best regards, > Tomasz > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Inki, On Saturday 30 of November 2013 14:18:08 Inki Dae wrote: > Hi Tomasz, > > Thanks for review, You're welcome. > > 2013/11/30 Tomasz Figa <t.figa@samsung.com>: > > Hi Sean, > > > > On Tuesday 29 of October 2013 12:13:17 Sean Paul wrote: > >> This patch moves the lvds bridge discovery and connector pre-emption > >> code from exynos common code into the dp driver (since the bridge is > >> only applicable for dp). > >> > >> Signed-off-by: Sean Paul <seanpaul@chromium.org> > >> --- > >> > >> Changes in v3: > >> - Added to the patchset > >> > >> drivers/gpu/drm/exynos/exynos_dp_core.c | 41 ++++++++++++++++++++++++++++++++ > >> drivers/gpu/drm/exynos/exynos_drm_core.c | 41 -------------------------------- > >> 2 files changed, 41 insertions(+), 41 deletions(-) > >> > > > > Well, DRM bridge infrastructure is useful for more than just DP. Currently > > there are several platforms that could use it with DSI as well, e.g. > > In case that some board with DSI bus needs this LVDS bridge, the > bridge type would be MIPI-DSI to LVDS Display bridge. So I think > moving this lvds stuff into eDP driver is reasonable becasue the > normal DSI bus cannot use this LVDS bridge, and this bridge (eDP to > LVDS) would be for eDP bus. BTW, Sean, why not implementing this lvds > codes to eDP driver without previous related patch? that seems > unnecessary work. I don't mean this particular bridge chip, but rather the bridge binding infrastructure. Anyway, having such hacks just in the DP driver is better than in Exynos DRM core and since we don't use DP on our platforms it doesn't bother me that much. So, if you really insist, I'm okay with this, just place all the bridge related code in this driver already, as Inki suggests. > > > Trats, Trats2, Arndale. With a simple abstraction worth of one or at most > > two days of work, we could get something that would cover much more than > > just a single Chromebook. > > I also thought a better way is to use simple abstraction layer. But > the abstraction layer I thought was the case that lvds binding is done > in this lvds driver itself so drm connector should call some bridge > related functions of the abstraction layer. But it seems that other > maintainers don't agree to this way. :( > > Anyway, we can say the use of lvds bridge is dependent on board but > actually, the use of lvds bridge would be more dependent on bus device > or lcd panel attached to the board. So my opinion is to bind dt in the > bus or lcd panel drivers that need lvds bridge. If dt binding of lvds > bridge cannot be done itself, Sean's way is reasonable to me except > the unnecessary patch. Well, I don't see why DT bindings couldn't be made for this. We already have bindings for video-interfaces (the so called v4l2 bindings), bindings for the LVDS bridge itself would be mostly device-specific, although we even already have bindings for display timings. Anyway, if even DRM people don't care, why should I? Just do what you want as long as it doesn't break other hardware. Best regards, Tomasz
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index 139ea15..32ba791 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -24,6 +24,7 @@ #include <drm/drmP.h> #include <drm/drm_crtc.h> #include <drm/drm_crtc_helper.h> +#include <drm/bridge/ptn3460.h> #include "exynos_drm_drv.h" #include "exynos_dp_core.h" @@ -31,6 +32,11 @@ #define ctx_from_connector(c) container_of(c, struct exynos_dp_device, \ connector) +struct bridge_init { + struct i2c_client *client; + struct device_node *node; +}; + static int exynos_dp_init_dp(struct exynos_dp_device *dp) { exynos_dp_reset(dp); @@ -972,6 +978,35 @@ static int exynos_dp_initialize(struct exynos_drm_display *display, return 0; } +static bool find_bridge(const char *compat, struct bridge_init *bridge) +{ + bridge->client = NULL; + bridge->node = of_find_compatible_node(NULL, NULL, compat); + if (!bridge->node) + return false; + + bridge->client = of_find_i2c_device_by_node(bridge->node); + if (!bridge->client) + return false; + + return true; +} + +/* returns the number of bridges attached */ +static int exynos_drm_attach_lcd_bridge(struct drm_device *dev, + struct drm_encoder *encoder) +{ + struct bridge_init bridge; + int ret; + + if (find_bridge("nxp,ptn3460", &bridge)) { + ret = ptn3460_init(dev, encoder, bridge.client, bridge.node); + if (!ret) + return 1; + } + return 0; +} + static int exynos_dp_create_connector(struct exynos_drm_display *display, struct drm_encoder *encoder) { @@ -980,6 +1015,12 @@ static int exynos_dp_create_connector(struct exynos_drm_display *display, int ret; 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) + return 0; + connector->polled = DRM_CONNECTOR_POLL_HPD; ret = drm_connector_init(dp->drm_dev, connector, diff --git a/drivers/gpu/drm/exynos/exynos_drm_core.c b/drivers/gpu/drm/exynos/exynos_drm_core.c index 1a60f5a..2446352 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_core.c +++ b/drivers/gpu/drm/exynos/exynos_drm_core.c @@ -14,7 +14,6 @@ #include <linux/i2c.h> #include <drm/drmP.h> -#include <drm/bridge/ptn3460.h> #include "exynos_drm_drv.h" #include "exynos_drm_crtc.h" #include "exynos_drm_encoder.h" @@ -25,40 +24,6 @@ static LIST_HEAD(exynos_drm_subdrv_list); static LIST_HEAD(exynos_drm_manager_list); static LIST_HEAD(exynos_drm_display_list); -struct bridge_init { - struct i2c_client *client; - struct device_node *node; -}; - -static bool find_bridge(const char *compat, struct bridge_init *bridge) -{ - bridge->client = NULL; - bridge->node = of_find_compatible_node(NULL, NULL, compat); - if (!bridge->node) - return false; - - bridge->client = of_find_i2c_device_by_node(bridge->node); - if (!bridge->client) - return false; - - return true; -} - -/* returns the number of bridges attached */ -static int exynos_drm_attach_lcd_bridge(struct drm_device *dev, - struct drm_encoder *encoder) -{ - struct bridge_init bridge; - int ret; - - if (find_bridge("nxp,ptn3460", &bridge)) { - ret = ptn3460_init(dev, encoder, bridge.client, bridge.node); - if (!ret) - return 1; - } - return 0; -} - static int exynos_drm_create_enc_conn(struct drm_device *dev, struct exynos_drm_display *display) { @@ -81,12 +46,6 @@ static int exynos_drm_create_enc_conn(struct drm_device *dev, } display->encoder = encoder; - if (display->type == EXYNOS_DISPLAY_TYPE_LCD) { - ret = exynos_drm_attach_lcd_bridge(dev, encoder); - if (ret) - return 0; - } - if (display->ops->create_connector) return display->ops->create_connector(display, encoder);
This patch moves the lvds bridge discovery and connector pre-emption code from exynos common code into the dp driver (since the bridge is only applicable for dp). Signed-off-by: Sean Paul <seanpaul@chromium.org> --- Changes in v3: - Added to the patchset drivers/gpu/drm/exynos/exynos_dp_core.c | 41 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/exynos/exynos_drm_core.c | 41 -------------------------------- 2 files changed, 41 insertions(+), 41 deletions(-)