diff mbox

[RESEND,V5,08/12] drm/bridge: ptn3460: Support bridge chaining

Message ID 1405629839-12086-9-git-send-email-ajaykumar.rs@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ajay Kumar July 17, 2014, 8:43 p.m. UTC
Modify the driver to invoke callbacks for the next bridge
in the bridge chain.
Also, remove the drm_connector implementation from ptn3460,
since the same is implemented using panel_binder.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
---
 drivers/gpu/drm/bridge/ptn3460.c        |  137 +++++--------------------------
 drivers/gpu/drm/exynos/exynos_dp_core.c |   16 ++--
 include/drm/bridge/ptn3460.h            |   15 ++--
 3 files changed, 39 insertions(+), 129 deletions(-)

Comments

Inki Dae July 21, 2014, 7:55 a.m. UTC | #1
On 2014? 07? 18? 05:43, Ajay Kumar wrote:
> Modify the driver to invoke callbacks for the next bridge
> in the bridge chain.
> Also, remove the drm_connector implementation from ptn3460,
> since the same is implemented using panel_binder.
> 
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> ---
>  drivers/gpu/drm/bridge/ptn3460.c        |  137 +++++--------------------------
>  drivers/gpu/drm/exynos/exynos_dp_core.c |   16 ++--
>  include/drm/bridge/ptn3460.h            |   15 ++--
>  3 files changed, 39 insertions(+), 129 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c
> index d466696..5fe16c6 100644
> --- a/drivers/gpu/drm/bridge/ptn3460.c
> +++ b/drivers/gpu/drm/bridge/ptn3460.c
> @@ -34,37 +34,15 @@
>  #define PTN3460_EDID_SRAM_LOAD_ADDR		0x85
>  
>  struct ptn3460_bridge {
> -	struct drm_connector connector;
>  	struct i2c_client *client;
>  	struct drm_encoder *encoder;
>  	struct drm_bridge *bridge;
> -	struct edid *edid;
>  	int gpio_pd_n;
>  	int gpio_rst_n;
>  	u32 edid_emulation;
>  	bool enabled;
>  };
>  
> -static int ptn3460_read_bytes(struct ptn3460_bridge *ptn_bridge, char addr,
> -		u8 *buf, int len)
> -{
> -	int ret;
> -
> -	ret = i2c_master_send(ptn_bridge->client, &addr, 1);
> -	if (ret <= 0) {
> -		DRM_ERROR("Failed to send i2c command, ret=%d\n", ret);
> -		return ret;
> -	}
> -
> -	ret = i2c_master_recv(ptn_bridge->client, buf, len);
> -	if (ret <= 0) {
> -		DRM_ERROR("Failed to recv i2c data, ret=%d\n", ret);
> -		return ret;
> -	}
> -
> -	return 0;
> -}
> -
>  static int ptn3460_write_byte(struct ptn3460_bridge *ptn_bridge, char addr,
>  		char val)
>  {
> @@ -126,6 +104,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
>  		gpio_set_value(ptn_bridge->gpio_rst_n, 1);
>  	}
>  
> +	drm_next_bridge_pre_enable(bridge);
> +
>  	/*
>  	 * 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 +122,7 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
>  
>  static void ptn3460_enable(struct drm_bridge *bridge)
>  {
> +	drm_next_bridge_enable(bridge);
>  }
>  
>  static void ptn3460_disable(struct drm_bridge *bridge)
> @@ -153,6 +134,8 @@ static void ptn3460_disable(struct drm_bridge *bridge)
>  
>  	ptn_bridge->enabled = false;
>  
> +	drm_next_bridge_disable(bridge);
> +
>  	if (gpio_is_valid(ptn_bridge->gpio_rst_n))
>  		gpio_set_value(ptn_bridge->gpio_rst_n, 1);
>  
> @@ -162,6 +145,7 @@ static void ptn3460_disable(struct drm_bridge *bridge)
>  
>  static void ptn3460_post_disable(struct drm_bridge *bridge)
>  {
> +	drm_next_bridge_post_disable(bridge);
>  }
>  
>  void ptn3460_bridge_destroy(struct drm_bridge *bridge)
> @@ -173,6 +157,9 @@ void ptn3460_bridge_destroy(struct drm_bridge *bridge)
>  		gpio_free(ptn_bridge->gpio_pd_n);
>  	if (gpio_is_valid(ptn_bridge->gpio_rst_n))
>  		gpio_free(ptn_bridge->gpio_rst_n);
> +
> +	drm_next_bridge_destroy(bridge);
> +
>  	/* Nothing else to free, we've got devm allocated memory */
>  }
>  
> @@ -184,81 +171,10 @@ struct drm_bridge_funcs ptn3460_bridge_funcs = {
>  	.destroy = ptn3460_bridge_destroy,
>  };
>  
> -int ptn3460_get_modes(struct drm_connector *connector)
> -{
> -	struct ptn3460_bridge *ptn_bridge;
> -	u8 *edid;
> -	int ret, num_modes;
> -	bool power_off;
> -
> -	ptn_bridge = container_of(connector, struct ptn3460_bridge, connector);
> -
> -	if (ptn_bridge->edid)
> -		return drm_add_edid_modes(connector, ptn_bridge->edid);
> -
> -	power_off = !ptn_bridge->enabled;
> -	ptn3460_pre_enable(ptn_bridge->bridge);
> -
> -	edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
> -	if (!edid) {
> -		DRM_ERROR("Failed to allocate edid\n");
> -		return 0;
> -	}
> -
> -	ret = ptn3460_read_bytes(ptn_bridge, PTN3460_EDID_ADDR, edid,
> -			EDID_LENGTH);
> -	if (ret) {
> -		kfree(edid);
> -		num_modes = 0;
> -		goto out;
> -	}
> -
> -	ptn_bridge->edid = (struct edid *)edid;
> -	drm_mode_connector_update_edid_property(connector, ptn_bridge->edid);
> -
> -	num_modes = drm_add_edid_modes(connector, ptn_bridge->edid);
> -
> -out:
> -	if (power_off)
> -		ptn3460_disable(ptn_bridge->bridge);
> -
> -	return num_modes;
> -}
> -
> -struct drm_encoder *ptn3460_best_encoder(struct drm_connector *connector)
> -{
> -	struct ptn3460_bridge *ptn_bridge;
> -
> -	ptn_bridge = container_of(connector, struct ptn3460_bridge, connector);
> -
> -	return ptn_bridge->encoder;
> -}
> -
> -struct drm_connector_helper_funcs ptn3460_connector_helper_funcs = {
> -	.get_modes = ptn3460_get_modes,
> -	.best_encoder = ptn3460_best_encoder,
> -};
> -
> -enum drm_connector_status ptn3460_detect(struct drm_connector *connector,
> -		bool force)
> -{
> -	return connector_status_connected;
> -}
> -
> -void ptn3460_connector_destroy(struct drm_connector *connector)
> -{
> -	drm_connector_cleanup(connector);
> -}
> -
> -struct drm_connector_funcs ptn3460_connector_funcs = {
> -	.dpms = drm_helper_connector_dpms,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.detect = ptn3460_detect,
> -	.destroy = ptn3460_connector_destroy,
> -};
> -
> -int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
> -		struct i2c_client *client, struct device_node *node)
> +struct drm_bridge *ptn3460_init(struct drm_device *dev,
> +				struct drm_encoder *encoder,
> +				struct i2c_client *client,
> +				struct device_node *node)
>  {
>  	int ret;
>  	struct drm_bridge *bridge;
> @@ -267,13 +183,13 @@ int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
>  	bridge = devm_kzalloc(dev->dev, sizeof(*bridge), GFP_KERNEL);
>  	if (!bridge) {
>  		DRM_ERROR("Failed to allocate drm bridge\n");
> -		return -ENOMEM;
> +		return NULL;

I think you could handle error case correctly. Please return
ERR_PTR(-ENOMEM) instead of NULL, and below codes also.

Thanks,
Inki Dae

>  	}
>  
>  	ptn_bridge = devm_kzalloc(dev->dev, sizeof(*ptn_bridge), GFP_KERNEL);
>  	if (!ptn_bridge) {
>  		DRM_ERROR("Failed to allocate ptn bridge\n");
> -		return -ENOMEM;
> +		return NULL;
>  	}
>  
>  	ptn_bridge->client = client;
> @@ -285,7 +201,7 @@ int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
>  				GPIOF_OUT_INIT_HIGH, "PTN3460_PD_N");
>  		if (ret) {
>  			DRM_ERROR("Request powerdown-gpio failed (%d)\n", ret);
> -			return ret;
> +			return NULL;
>  		}
>  	}
>  
> @@ -300,7 +216,7 @@ int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
>  		if (ret) {
>  			DRM_ERROR("Request reset-gpio failed (%d)\n", ret);
>  			gpio_free(ptn_bridge->gpio_pd_n);
> -			return ret;
> +			return NULL;
>  		}
>  	}
>  
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding July 21, 2014, 8:22 a.m. UTC | #2
On Fri, Jul 18, 2014 at 02:13:54AM +0530, Ajay Kumar wrote:
[...]
> Also, remove the drm_connector implementation from ptn3460,
> since the same is implemented using panel_binder.

I think that's a step backwards. In fact I think the panel-bridge binder
driver shouldn't be needed at all. At least not for now. We have a very
limited number of bridge drivers, so it shouldn't hurt at this stage to
implement the connector instantiation within each driver. Once we have
more bridge drivers, and therefore a better understanding of what they
need, we can always add something like the panel-binder (though I think
it should be library code, similar to the drm_encoder_helper_add() API,
rather than this kind of self-contained object).

Thierry
Ajay kumar July 21, 2014, 11:58 a.m. UTC | #3
Hi Thierry,

On Mon, Jul 21, 2014 at 1:52 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Fri, Jul 18, 2014 at 02:13:54AM +0530, Ajay Kumar wrote:
> [...]
>> Also, remove the drm_connector implementation from ptn3460,
>> since the same is implemented using panel_binder.
>
> I think that's a step backwards. In fact I think the panel-bridge binder
> driver shouldn't be needed at all. At least not for now. We have a very
> limited number of bridge drivers, so it shouldn't hurt at this stage to
> implement the connector instantiation within each driver. Once we have
> more bridge drivers, and therefore a better understanding of what they
> need, we can always add something like the panel-binder (though I think
> it should be library code, similar to the drm_encoder_helper_add() API,
> rather than this kind of self-contained object).
panel_binder was needed to wrap around panel as a bridge, and this was in turn
needed because people wanted to represent a bridge+panel combo as a chain
of bridges.
So, if we choose to drop panel_binder, we choose to drop the idea of chaining
the bridges, and end up calling drm_panel functions directly from
individual bridges.
And, the bridge will implement the connector functions as you suggested.
I am okay with this, if Daniel/Rob don't have an issue with this.

Ajay
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding July 21, 2014, 12:40 p.m. UTC | #4
On Mon, Jul 21, 2014 at 05:28:13PM +0530, Ajay kumar wrote:
> Hi Thierry,
> 
> On Mon, Jul 21, 2014 at 1:52 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Fri, Jul 18, 2014 at 02:13:54AM +0530, Ajay Kumar wrote:
> > [...]
> >> Also, remove the drm_connector implementation from ptn3460,
> >> since the same is implemented using panel_binder.
> >
> > I think that's a step backwards. In fact I think the panel-bridge binder
> > driver shouldn't be needed at all. At least not for now. We have a very
> > limited number of bridge drivers, so it shouldn't hurt at this stage to
> > implement the connector instantiation within each driver. Once we have
> > more bridge drivers, and therefore a better understanding of what they
> > need, we can always add something like the panel-binder (though I think
> > it should be library code, similar to the drm_encoder_helper_add() API,
> > rather than this kind of self-contained object).
> panel_binder was needed to wrap around panel as a bridge, and this was in turn
> needed because people wanted to represent a bridge+panel combo as a chain
> of bridges.
> So, if we choose to drop panel_binder, we choose to drop the idea of chaining
> the bridges, and end up calling drm_panel functions directly from
> individual bridges.
> And, the bridge will implement the connector functions as you suggested.
> I am okay with this, if Daniel/Rob don't have an issue with this.

I think bridge chaining and panel handling are separate issues. That's
why I think we shouldn't mix them like this. From earlier discussion[0]
the conclusion was that the final element in the chain should implement
a connector (with the appropriate type). Often that last element would
be an encoder (when there are no bridges). Sometimes the last element
would be a bridge. It's logical for that same element to also implement
the panel integration since it's closely tied to the connector.

Thierry

[0]: http://lists.freedesktop.org/archives/dri-devel/2014-May/059685.html
Ajay kumar July 22, 2014, 6:21 a.m. UTC | #5
On Mon, Jul 21, 2014 at 6:10 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Mon, Jul 21, 2014 at 05:28:13PM +0530, Ajay kumar wrote:
>> Hi Thierry,
>>
>> On Mon, Jul 21, 2014 at 1:52 PM, Thierry Reding
>> <thierry.reding@gmail.com> wrote:
>> > On Fri, Jul 18, 2014 at 02:13:54AM +0530, Ajay Kumar wrote:
>> > [...]
>> >> Also, remove the drm_connector implementation from ptn3460,
>> >> since the same is implemented using panel_binder.
>> >
>> > I think that's a step backwards. In fact I think the panel-bridge binder
>> > driver shouldn't be needed at all. At least not for now. We have a very
>> > limited number of bridge drivers, so it shouldn't hurt at this stage to
>> > implement the connector instantiation within each driver. Once we have
>> > more bridge drivers, and therefore a better understanding of what they
>> > need, we can always add something like the panel-binder (though I think
>> > it should be library code, similar to the drm_encoder_helper_add() API,
>> > rather than this kind of self-contained object).
>> panel_binder was needed to wrap around panel as a bridge, and this was in turn
>> needed because people wanted to represent a bridge+panel combo as a chain
>> of bridges.
>> So, if we choose to drop panel_binder, we choose to drop the idea of chaining
>> the bridges, and end up calling drm_panel functions directly from
>> individual bridges.
>> And, the bridge will implement the connector functions as you suggested.
>> I am okay with this, if Daniel/Rob don't have an issue with this.
>
> I think bridge chaining and panel handling are separate issues. That's
> why I think we shouldn't mix them like this. From earlier discussion[0]
> the conclusion was that the final element in the chain should implement
> a connector (with the appropriate type). Often that last element would
> be an encoder (when there are no bridges). Sometimes the last element
> would be a bridge. It's logical for that same element to also implement
> the panel integration since it's closely tied to the connector.
>
> Thierry
>
> [0]: http://lists.freedesktop.org/archives/dri-devel/2014-May/059685.html
Going with Thierry's opinion, if the bridge is allowed to do panel integration,
there is no need for a bridge_chain. I mean a bridge_chain doesn't exist in
my case at all. I have just one bridge which integrates a panel.
Is it really necessary to keep next_bridge pointer and other helpers?

Ajay
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpu/drm/bridge/ptn3460.c b/drivers/gpu/drm/bridge/ptn3460.c
index d466696..5fe16c6 100644
--- a/drivers/gpu/drm/bridge/ptn3460.c
+++ b/drivers/gpu/drm/bridge/ptn3460.c
@@ -34,37 +34,15 @@ 
 #define PTN3460_EDID_SRAM_LOAD_ADDR		0x85
 
 struct ptn3460_bridge {
-	struct drm_connector connector;
 	struct i2c_client *client;
 	struct drm_encoder *encoder;
 	struct drm_bridge *bridge;
-	struct edid *edid;
 	int gpio_pd_n;
 	int gpio_rst_n;
 	u32 edid_emulation;
 	bool enabled;
 };
 
-static int ptn3460_read_bytes(struct ptn3460_bridge *ptn_bridge, char addr,
-		u8 *buf, int len)
-{
-	int ret;
-
-	ret = i2c_master_send(ptn_bridge->client, &addr, 1);
-	if (ret <= 0) {
-		DRM_ERROR("Failed to send i2c command, ret=%d\n", ret);
-		return ret;
-	}
-
-	ret = i2c_master_recv(ptn_bridge->client, buf, len);
-	if (ret <= 0) {
-		DRM_ERROR("Failed to recv i2c data, ret=%d\n", ret);
-		return ret;
-	}
-
-	return 0;
-}
-
 static int ptn3460_write_byte(struct ptn3460_bridge *ptn_bridge, char addr,
 		char val)
 {
@@ -126,6 +104,8 @@  static void ptn3460_pre_enable(struct drm_bridge *bridge)
 		gpio_set_value(ptn_bridge->gpio_rst_n, 1);
 	}
 
+	drm_next_bridge_pre_enable(bridge);
+
 	/*
 	 * 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 +122,7 @@  static void ptn3460_pre_enable(struct drm_bridge *bridge)
 
 static void ptn3460_enable(struct drm_bridge *bridge)
 {
+	drm_next_bridge_enable(bridge);
 }
 
 static void ptn3460_disable(struct drm_bridge *bridge)
@@ -153,6 +134,8 @@  static void ptn3460_disable(struct drm_bridge *bridge)
 
 	ptn_bridge->enabled = false;
 
+	drm_next_bridge_disable(bridge);
+
 	if (gpio_is_valid(ptn_bridge->gpio_rst_n))
 		gpio_set_value(ptn_bridge->gpio_rst_n, 1);
 
@@ -162,6 +145,7 @@  static void ptn3460_disable(struct drm_bridge *bridge)
 
 static void ptn3460_post_disable(struct drm_bridge *bridge)
 {
+	drm_next_bridge_post_disable(bridge);
 }
 
 void ptn3460_bridge_destroy(struct drm_bridge *bridge)
@@ -173,6 +157,9 @@  void ptn3460_bridge_destroy(struct drm_bridge *bridge)
 		gpio_free(ptn_bridge->gpio_pd_n);
 	if (gpio_is_valid(ptn_bridge->gpio_rst_n))
 		gpio_free(ptn_bridge->gpio_rst_n);
+
+	drm_next_bridge_destroy(bridge);
+
 	/* Nothing else to free, we've got devm allocated memory */
 }
 
@@ -184,81 +171,10 @@  struct drm_bridge_funcs ptn3460_bridge_funcs = {
 	.destroy = ptn3460_bridge_destroy,
 };
 
-int ptn3460_get_modes(struct drm_connector *connector)
-{
-	struct ptn3460_bridge *ptn_bridge;
-	u8 *edid;
-	int ret, num_modes;
-	bool power_off;
-
-	ptn_bridge = container_of(connector, struct ptn3460_bridge, connector);
-
-	if (ptn_bridge->edid)
-		return drm_add_edid_modes(connector, ptn_bridge->edid);
-
-	power_off = !ptn_bridge->enabled;
-	ptn3460_pre_enable(ptn_bridge->bridge);
-
-	edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
-	if (!edid) {
-		DRM_ERROR("Failed to allocate edid\n");
-		return 0;
-	}
-
-	ret = ptn3460_read_bytes(ptn_bridge, PTN3460_EDID_ADDR, edid,
-			EDID_LENGTH);
-	if (ret) {
-		kfree(edid);
-		num_modes = 0;
-		goto out;
-	}
-
-	ptn_bridge->edid = (struct edid *)edid;
-	drm_mode_connector_update_edid_property(connector, ptn_bridge->edid);
-
-	num_modes = drm_add_edid_modes(connector, ptn_bridge->edid);
-
-out:
-	if (power_off)
-		ptn3460_disable(ptn_bridge->bridge);
-
-	return num_modes;
-}
-
-struct drm_encoder *ptn3460_best_encoder(struct drm_connector *connector)
-{
-	struct ptn3460_bridge *ptn_bridge;
-
-	ptn_bridge = container_of(connector, struct ptn3460_bridge, connector);
-
-	return ptn_bridge->encoder;
-}
-
-struct drm_connector_helper_funcs ptn3460_connector_helper_funcs = {
-	.get_modes = ptn3460_get_modes,
-	.best_encoder = ptn3460_best_encoder,
-};
-
-enum drm_connector_status ptn3460_detect(struct drm_connector *connector,
-		bool force)
-{
-	return connector_status_connected;
-}
-
-void ptn3460_connector_destroy(struct drm_connector *connector)
-{
-	drm_connector_cleanup(connector);
-}
-
-struct drm_connector_funcs ptn3460_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.detect = ptn3460_detect,
-	.destroy = ptn3460_connector_destroy,
-};
-
-int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
-		struct i2c_client *client, struct device_node *node)
+struct drm_bridge *ptn3460_init(struct drm_device *dev,
+				struct drm_encoder *encoder,
+				struct i2c_client *client,
+				struct device_node *node)
 {
 	int ret;
 	struct drm_bridge *bridge;
@@ -267,13 +183,13 @@  int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
 	bridge = devm_kzalloc(dev->dev, sizeof(*bridge), GFP_KERNEL);
 	if (!bridge) {
 		DRM_ERROR("Failed to allocate drm bridge\n");
-		return -ENOMEM;
+		return NULL;
 	}
 
 	ptn_bridge = devm_kzalloc(dev->dev, sizeof(*ptn_bridge), GFP_KERNEL);
 	if (!ptn_bridge) {
 		DRM_ERROR("Failed to allocate ptn bridge\n");
-		return -ENOMEM;
+		return NULL;
 	}
 
 	ptn_bridge->client = client;
@@ -285,7 +201,7 @@  int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
 				GPIOF_OUT_INIT_HIGH, "PTN3460_PD_N");
 		if (ret) {
 			DRM_ERROR("Request powerdown-gpio failed (%d)\n", ret);
-			return ret;
+			return NULL;
 		}
 	}
 
@@ -300,7 +216,7 @@  int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
 		if (ret) {
 			DRM_ERROR("Request reset-gpio failed (%d)\n", ret);
 			gpio_free(ptn_bridge->gpio_pd_n);
-			return ret;
+			return NULL;
 		}
 	}
 
@@ -318,26 +234,17 @@  int ptn3460_init(struct drm_device *dev, struct drm_encoder *encoder,
 	}
 
 	bridge->driver_private = ptn_bridge;
-	encoder->bridge = bridge;
+	if (!encoder->bridge)
+		/* First entry in the bridge chain */
+		encoder->bridge = bridge;
 
-	ret = drm_connector_init(dev, &ptn_bridge->connector,
-			&ptn3460_connector_funcs, DRM_MODE_CONNECTOR_LVDS);
-	if (ret) {
-		DRM_ERROR("Failed to initialize connector with drm\n");
-		goto err;
-	}
-	drm_connector_helper_add(&ptn_bridge->connector,
-			&ptn3460_connector_helper_funcs);
-	drm_connector_register(&ptn_bridge->connector);
-	drm_mode_connector_attach_encoder(&ptn_bridge->connector, encoder);
-
-	return 0;
+	return bridge;
 
 err:
 	if (gpio_is_valid(ptn_bridge->gpio_pd_n))
 		gpio_free(ptn_bridge->gpio_pd_n);
 	if (gpio_is_valid(ptn_bridge->gpio_rst_n))
 		gpio_free(ptn_bridge->gpio_rst_n);
-	return ret;
+	return NULL;
 }
 EXPORT_SYMBOL(ptn3460_init);
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index b3d0d9b..9d31296 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -988,19 +988,19 @@  static bool find_bridge(const char *compat, struct bridge_init *bridge)
 	return true;
 }
 
-/* returns the number of bridges attached */
-static int exynos_drm_attach_lcd_bridge(struct drm_device *dev,
+static int exynos_drm_attach_lcd_bridge(struct exynos_dp_device *dp,
 		struct drm_encoder *encoder)
 {
 	struct bridge_init bridge;
-	int ret;
+	struct drm_bridge *bridge_chain = NULL;
+	bool connector_created = false;
 
 	if (find_bridge("nxp,ptn3460", &bridge)) {
-		ret = ptn3460_init(dev, encoder, bridge.client, bridge.node);
-		if (!ret)
-			return 1;
+		bridge_chain = ptn3460_init(dp->drm_dev, encoder, bridge.client,
+								bridge.node);
 	}
-	return 0;
+
+	return connector_created;
 }
 
 static int exynos_dp_create_connector(struct exynos_drm_display *display,
@@ -1013,7 +1013,7 @@  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);
+	ret = exynos_drm_attach_lcd_bridge(dp, encoder);
 	if (ret)
 		return 0;
 
diff --git a/include/drm/bridge/ptn3460.h b/include/drm/bridge/ptn3460.h
index ff62344..f612b9b 100644
--- a/include/drm/bridge/ptn3460.h
+++ b/include/drm/bridge/ptn3460.h
@@ -21,15 +21,18 @@  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 drm_bridge *ptn3460_init(struct drm_device *dev,
+				struct drm_encoder *encoder,
+				struct i2c_client *client,
+				struct device_node *node);
 #else
 
-static inline int ptn3460_init(struct drm_device *dev,
-		struct drm_encoder *encoder, struct i2c_client *client,
-		struct device_node *node)
+static inline struct drm_bridge *ptn3460_init(struct drm_device *dev,
+						struct drm_encoder *encoder,
+						struct i2c_client *client,
+						struct device_node *node)
 {
-	return 0;
+	return NULL;
 }
 
 #endif