diff mbox series

[tiL4.19-AD,2/2] drm/tilcdc: Remove obsolete crtc_mode_valid() hack

Message ID 7254b3a3bcd489fb44f6b57fd8bab325c4fbfad7.1551352503.git.jsarha@ti.com (mailing list archive)
State New, archived
Headers show
Series drm/tilcdc: a cleanup and a fix | expand

Commit Message

Jyri Sarha Feb. 28, 2019, 11:18 a.m. UTC 
Earlier there were no mode_valid() helper for crtc and tilcdc had a
hack to over come this limitation. But now the mode_valid() helper is
there (has been since v4.13), so it is about time to get rid of that
hack.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c     | 28 +++-----
 drivers/gpu/drm/tilcdc/tilcdc_drv.c      |  1 -
 drivers/gpu/drm/tilcdc/tilcdc_drv.h      |  2 -
 drivers/gpu/drm/tilcdc/tilcdc_external.c | 88 +++---------------------
 drivers/gpu/drm/tilcdc/tilcdc_external.h |  1 -
 drivers/gpu/drm/tilcdc/tilcdc_panel.c    |  9 ---
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c   |  9 ---
 7 files changed, 19 insertions(+), 119 deletions(-)

Comments

Laurent Pinchart March 5, 2019, 7:21 p.m. UTC | #1 
Hi Jyri,

Thank you for the patch.

On Thu, Feb 28, 2019 at 01:18:51PM +0200, Jyri Sarha wrote:
> Earlier there were no mode_valid() helper for crtc and tilcdc had a
> hack to over come this limitation. But now the mode_valid() helper is
> there (has been since v4.13), so it is about time to get rid of that
> hack.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c     | 28 +++-----
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c      |  1 -
>  drivers/gpu/drm/tilcdc/tilcdc_drv.h      |  2 -
>  drivers/gpu/drm/tilcdc/tilcdc_external.c | 88 +++---------------------
>  drivers/gpu/drm/tilcdc/tilcdc_external.h |  1 -
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c    |  9 ---
>  drivers/gpu/drm/tilcdc/tilcdc_tfp410.c   |  9 ---
>  7 files changed, 19 insertions(+), 119 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index a8ae064f52bb..f9600f2ad660 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -659,9 +659,6 @@ static bool tilcdc_crtc_mode_fixup(struct drm_crtc *crtc,
>  static int tilcdc_crtc_atomic_check(struct drm_crtc *crtc,
>  				    struct drm_crtc_state *state)
>  {
> -	struct drm_display_mode *mode = &state->mode;
> -	int ret;
> -
>  	/* If we are not active we don't care */
>  	if (!state->active)
>  		return 0;
> @@ -673,12 +670,6 @@ static int tilcdc_crtc_atomic_check(struct drm_crtc *crtc,
>  		return -EINVAL;
>  	}
>  
> -	ret = tilcdc_crtc_mode_valid(crtc, mode);
> -	if (ret) {
> -		dev_dbg(crtc->dev->dev, "Mode \"%s\" not valid", mode->name);
> -		return -EINVAL;
> -	}
> -
>  	return 0;
>  }
>  
> @@ -730,13 +721,6 @@ static const struct drm_crtc_funcs tilcdc_crtc_funcs = {
>  	.disable_vblank	= tilcdc_crtc_disable_vblank,
>  };
>  
> -static const struct drm_crtc_helper_funcs tilcdc_crtc_helper_funcs = {
> -		.mode_fixup     = tilcdc_crtc_mode_fixup,
> -		.atomic_check	= tilcdc_crtc_atomic_check,
> -		.atomic_enable	= tilcdc_crtc_atomic_enable,
> -		.atomic_disable	= tilcdc_crtc_atomic_disable,
> -};
> -
>  int tilcdc_crtc_max_width(struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
> @@ -751,7 +735,9 @@ int tilcdc_crtc_max_width(struct drm_crtc *crtc)
>  	return max_width;
>  }
>  
> -int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode)
> +static enum drm_mode_status
> +tilcdc_crtc_mode_valid(struct drm_crtc *crtc,
> +		       const struct drm_display_mode *mode)
>  {
>  	struct tilcdc_drm_private *priv = crtc->dev->dev_private;
>  	unsigned int bandwidth;
> @@ -839,6 +825,14 @@ int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode)
>  	return MODE_OK;
>  }
>  
> +static const struct drm_crtc_helper_funcs tilcdc_crtc_helper_funcs = {
> +		.mode_valid	= tilcdc_crtc_mode_valid,
> +		.mode_fixup	= tilcdc_crtc_mode_fixup,
> +		.atomic_check	= tilcdc_crtc_atomic_check,
> +		.atomic_enable	= tilcdc_crtc_atomic_enable,
> +		.atomic_disable	= tilcdc_crtc_atomic_disable,

While at it, could you fix the indentation ?

This patch looks good to me overall, nice cleanup.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Is it paving the way to removal of the custom tftp410 module ? :-)

> +};
> +
>  void tilcdc_crtc_set_panel_info(struct drm_crtc *crtc,
>  		const struct tilcdc_panel_info *info)
>  {
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index 3dac08b24140..36fd05b145a4 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -192,7 +192,6 @@ static void tilcdc_fini(struct drm_device *dev)
>  	drm_kms_helper_poll_fini(dev);
>  	drm_irq_uninstall(dev);
>  	drm_mode_config_cleanup(dev);
> -	tilcdc_remove_external_device(dev);
>  
>  #ifdef CONFIG_CPU_FREQ
>  	if (priv->freq_transition.notifier_call)
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> index 62cea5ff5558..3298f39d320e 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> @@ -86,7 +86,6 @@ struct tilcdc_drm_private {
>  
>  	struct drm_encoder *external_encoder;
>  	struct drm_connector *external_connector;
> -	const struct drm_connector_helper_funcs *connector_funcs;
>  
>  	bool is_registered;
>  	bool is_componentized;
> @@ -168,7 +167,6 @@ void tilcdc_crtc_set_panel_info(struct drm_crtc *crtc,
>  		const struct tilcdc_panel_info *info);
>  void tilcdc_crtc_set_simulate_vesa_sync(struct drm_crtc *crtc,
>  					bool simulate_vesa_sync);
> -int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode);
>  int tilcdc_crtc_max_width(struct drm_crtc *crtc);
>  void tilcdc_crtc_shutdown(struct drm_crtc *crtc);
>  int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> index b4eaf9bc87f8..54adf05f44e6 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> @@ -40,64 +40,6 @@ static const struct tilcdc_panel_info panel_info_default = {
>  		.raster_order           = 0,
>  };
>  
> -static int tilcdc_external_mode_valid(struct drm_connector *connector,
> -				      struct drm_display_mode *mode)
> -{
> -	struct tilcdc_drm_private *priv = connector->dev->dev_private;
> -	int ret;
> -
> -	ret = tilcdc_crtc_mode_valid(priv->crtc, mode);
> -	if (ret != MODE_OK)
> -		return ret;
> -
> -	BUG_ON(priv->external_connector != connector);
> -	BUG_ON(!priv->connector_funcs);
> -
> -	/* If the connector has its own mode_valid call it. */
> -	if (!IS_ERR(priv->connector_funcs) &&
> -	    priv->connector_funcs->mode_valid)
> -		return priv->connector_funcs->mode_valid(connector, mode);
> -
> -	return MODE_OK;
> -}
> -
> -static int tilcdc_add_external_connector(struct drm_device *dev,
> -					 struct drm_connector *connector)
> -{
> -	struct tilcdc_drm_private *priv = dev->dev_private;
> -	struct drm_connector_helper_funcs *connector_funcs;
> -
> -	/* There should never be more than one connector */
> -	if (WARN_ON(priv->external_connector))
> -		return -EINVAL;
> -
> -	priv->external_connector = connector;
> -	connector_funcs = devm_kzalloc(dev->dev, sizeof(*connector_funcs),
> -				       GFP_KERNEL);
> -	if (!connector_funcs)
> -		return -ENOMEM;
> -
> -	/* connector->helper_private contains always struct
> -	 * connector_helper_funcs pointer. For tilcdc crtc to have a
> -	 * say if a specific mode is Ok, we need to install our own
> -	 * helper functions. In our helper functions we copy
> -	 * everything else but use our own mode_valid() (above).
> -	 */
> -	if (connector->helper_private) {
> -		priv->connector_funcs =	connector->helper_private;
> -		*connector_funcs = *priv->connector_funcs;
> -	} else {
> -		priv->connector_funcs = ERR_PTR(-ENOENT);
> -	}
> -	connector_funcs->mode_valid = tilcdc_external_mode_valid;
> -	drm_connector_helper_add(connector, connector_funcs);
> -
> -	dev_dbg(dev->dev, "External connector '%s' connected\n",
> -		connector->name);
> -
> -	return 0;
> -}
> -
>  static
>  struct drm_connector *tilcdc_encoder_find_connector(struct drm_device *ddev,
>  						    struct drm_encoder *encoder)
> @@ -118,7 +60,6 @@ struct drm_connector *tilcdc_encoder_find_connector(struct drm_device *ddev,
>  int tilcdc_add_component_encoder(struct drm_device *ddev)
>  {
>  	struct tilcdc_drm_private *priv = ddev->dev_private;
> -	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
>  
>  	list_for_each_entry(encoder, &ddev->mode_config.encoder_list, head)
> @@ -130,28 +71,17 @@ int tilcdc_add_component_encoder(struct drm_device *ddev)
>  		return -ENODEV;
>  	}
>  
> -	connector = tilcdc_encoder_find_connector(ddev, encoder);
> +	priv->external_connector
> +		= tilcdc_encoder_find_connector(ddev, encoder);
>  
> -	if (!connector)
> +	if (!priv->external_connector)
>  		return -ENODEV;
>  
>  	/* Only tda998x is supported at the moment. */
>  	tilcdc_crtc_set_simulate_vesa_sync(priv->crtc, true);
>  	tilcdc_crtc_set_panel_info(priv->crtc, &panel_info_tda998x);
>  
> -	return tilcdc_add_external_connector(ddev, connector);
> -}
> -
> -void tilcdc_remove_external_device(struct drm_device *dev)
> -{
> -	struct tilcdc_drm_private *priv = dev->dev_private;
> -
> -	/* Restore the original helper functions, if any. */
> -	if (IS_ERR(priv->connector_funcs))
> -		drm_connector_helper_add(priv->external_connector, NULL);
> -	else if (priv->connector_funcs)
> -		drm_connector_helper_add(priv->external_connector,
> -					 priv->connector_funcs);
> +	return 0;
>  }
>  
>  static const struct drm_encoder_funcs tilcdc_external_encoder_funcs = {
> @@ -162,7 +92,6 @@ static
>  int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
>  {
>  	struct tilcdc_drm_private *priv = ddev->dev_private;
> -	struct drm_connector *connector;
>  	int ret;
>  
>  	priv->external_encoder->possible_crtcs = BIT(0);
> @@ -175,13 +104,12 @@ int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
>  
>  	tilcdc_crtc_set_panel_info(priv->crtc, &panel_info_default);
>  
> -	connector = tilcdc_encoder_find_connector(ddev, priv->external_encoder);
> -	if (!connector)
> +	priv->external_connector =
> +		tilcdc_encoder_find_connector(ddev, priv->external_encoder);
> +	if (!priv->external_connector)
>  		return -ENODEV;
>  
> -	ret = tilcdc_add_external_connector(ddev, connector);
> -
> -	return ret;
> +	return 0;
>  }
>  
>  int tilcdc_attach_external_device(struct drm_device *ddev)
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.h b/drivers/gpu/drm/tilcdc/tilcdc_external.h
> index 763d18f006c7..a28b9df68c8f 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_external.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.h
> @@ -19,7 +19,6 @@
>  #define __TILCDC_EXTERNAL_H__
>  
>  int tilcdc_add_component_encoder(struct drm_device *dev);
> -void tilcdc_remove_external_device(struct drm_device *dev);
>  int tilcdc_get_external_components(struct device *dev,
>  				   struct component_match **match);
>  int tilcdc_attach_external_device(struct drm_device *ddev);
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> index a1acab39d87f..7c26f81fb3f0 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> @@ -170,14 +170,6 @@ static int panel_connector_get_modes(struct drm_connector *connector)
>  	return i;
>  }
>  
> -static int panel_connector_mode_valid(struct drm_connector *connector,
> -		  struct drm_display_mode *mode)
> -{
> -	struct tilcdc_drm_private *priv = connector->dev->dev_private;
> -	/* our only constraints are what the crtc can generate: */
> -	return tilcdc_crtc_mode_valid(priv->crtc, mode);
> -}
> -
>  static struct drm_encoder *panel_connector_best_encoder(
>  		struct drm_connector *connector)
>  {
> @@ -195,7 +187,6 @@ static const struct drm_connector_funcs panel_connector_funcs = {
>  
>  static const struct drm_connector_helper_funcs panel_connector_helper_funcs = {
>  	.get_modes          = panel_connector_get_modes,
> -	.mode_valid         = panel_connector_mode_valid,
>  	.best_encoder       = panel_connector_best_encoder,
>  };
>  
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> index daebf1aa6b0a..54c6d825b1a0 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> @@ -183,14 +183,6 @@ static int tfp410_connector_get_modes(struct drm_connector *connector)
>  	return ret;
>  }
>  
> -static int tfp410_connector_mode_valid(struct drm_connector *connector,
> -		  struct drm_display_mode *mode)
> -{
> -	struct tilcdc_drm_private *priv = connector->dev->dev_private;
> -	/* our only constraints are what the crtc can generate: */
> -	return tilcdc_crtc_mode_valid(priv->crtc, mode);
> -}
> -
>  static struct drm_encoder *tfp410_connector_best_encoder(
>  		struct drm_connector *connector)
>  {
> @@ -209,7 +201,6 @@ static const struct drm_connector_funcs tfp410_connector_funcs = {
>  
>  static const struct drm_connector_helper_funcs tfp410_connector_helper_funcs = {
>  	.get_modes          = tfp410_connector_get_modes,
> -	.mode_valid         = tfp410_connector_mode_valid,
>  	.best_encoder       = tfp410_connector_best_encoder,
>  };
>
Jyri Sarha March 13, 2019, 5:30 p.m. UTC | #2 
On 05/03/2019 21:21, Laurent Pinchart wrote:
> Hi Jyri,
> 
> Thank you for the patch.
> 
> On Thu, Feb 28, 2019 at 01:18:51PM +0200, Jyri Sarha wrote:
>> Earlier there were no mode_valid() helper for crtc and tilcdc had a
>> hack to over come this limitation. But now the mode_valid() helper is
>> there (has been since v4.13), so it is about time to get rid of that
>> hack.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
...
>> +static const struct drm_crtc_helper_funcs tilcdc_crtc_helper_funcs = {
>> +		.mode_valid	= tilcdc_crtc_mode_valid,
>> +		.mode_fixup	= tilcdc_crtc_mode_fixup,
>> +		.atomic_check	= tilcdc_crtc_atomic_check,
>> +		.atomic_enable	= tilcdc_crtc_atomic_enable,
>> +		.atomic_disable	= tilcdc_crtc_atomic_disable,
> 
> While at it, could you fix the indentation ?
> 
> This patch looks good to me overall, nice cleanup.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Is it paving the way to removal of the custom tftp410 module ? :-)
> 

Not really. There is really no simple way to get rid of the if we want
to keep the DTB backward compatibility. There has not been any other
reason to keep it for some time. Tilcdc supports brides and there is a
bridge driver for tfp410.

BR,
Jyri
diff mbox series

Patch

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index a8ae064f52bb..f9600f2ad660 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -659,9 +659,6 @@  static bool tilcdc_crtc_mode_fixup(struct drm_crtc *crtc,
 static int tilcdc_crtc_atomic_check(struct drm_crtc *crtc,
 				    struct drm_crtc_state *state)
 {
-	struct drm_display_mode *mode = &state->mode;
-	int ret;
-
 	/* If we are not active we don't care */
 	if (!state->active)
 		return 0;
@@ -673,12 +670,6 @@  static int tilcdc_crtc_atomic_check(struct drm_crtc *crtc,
 		return -EINVAL;
 	}
 
-	ret = tilcdc_crtc_mode_valid(crtc, mode);
-	if (ret) {
-		dev_dbg(crtc->dev->dev, "Mode \"%s\" not valid", mode->name);
-		return -EINVAL;
-	}
-
 	return 0;
 }
 
@@ -730,13 +721,6 @@  static const struct drm_crtc_funcs tilcdc_crtc_funcs = {
 	.disable_vblank	= tilcdc_crtc_disable_vblank,
 };
 
-static const struct drm_crtc_helper_funcs tilcdc_crtc_helper_funcs = {
-		.mode_fixup     = tilcdc_crtc_mode_fixup,
-		.atomic_check	= tilcdc_crtc_atomic_check,
-		.atomic_enable	= tilcdc_crtc_atomic_enable,
-		.atomic_disable	= tilcdc_crtc_atomic_disable,
-};
-
 int tilcdc_crtc_max_width(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -751,7 +735,9 @@  int tilcdc_crtc_max_width(struct drm_crtc *crtc)
 	return max_width;
 }
 
-int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode)
+static enum drm_mode_status
+tilcdc_crtc_mode_valid(struct drm_crtc *crtc,
+		       const struct drm_display_mode *mode)
 {
 	struct tilcdc_drm_private *priv = crtc->dev->dev_private;
 	unsigned int bandwidth;
@@ -839,6 +825,14 @@  int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode)
 	return MODE_OK;
 }
 
+static const struct drm_crtc_helper_funcs tilcdc_crtc_helper_funcs = {
+		.mode_valid	= tilcdc_crtc_mode_valid,
+		.mode_fixup	= tilcdc_crtc_mode_fixup,
+		.atomic_check	= tilcdc_crtc_atomic_check,
+		.atomic_enable	= tilcdc_crtc_atomic_enable,
+		.atomic_disable	= tilcdc_crtc_atomic_disable,
+};
+
 void tilcdc_crtc_set_panel_info(struct drm_crtc *crtc,
 		const struct tilcdc_panel_info *info)
 {
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 3dac08b24140..36fd05b145a4 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -192,7 +192,6 @@  static void tilcdc_fini(struct drm_device *dev)
 	drm_kms_helper_poll_fini(dev);
 	drm_irq_uninstall(dev);
 	drm_mode_config_cleanup(dev);
-	tilcdc_remove_external_device(dev);
 
 #ifdef CONFIG_CPU_FREQ
 	if (priv->freq_transition.notifier_call)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 62cea5ff5558..3298f39d320e 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -86,7 +86,6 @@  struct tilcdc_drm_private {
 
 	struct drm_encoder *external_encoder;
 	struct drm_connector *external_connector;
-	const struct drm_connector_helper_funcs *connector_funcs;
 
 	bool is_registered;
 	bool is_componentized;
@@ -168,7 +167,6 @@  void tilcdc_crtc_set_panel_info(struct drm_crtc *crtc,
 		const struct tilcdc_panel_info *info);
 void tilcdc_crtc_set_simulate_vesa_sync(struct drm_crtc *crtc,
 					bool simulate_vesa_sync);
-int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode);
 int tilcdc_crtc_max_width(struct drm_crtc *crtc);
 void tilcdc_crtc_shutdown(struct drm_crtc *crtc);
 int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
index b4eaf9bc87f8..54adf05f44e6 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
@@ -40,64 +40,6 @@  static const struct tilcdc_panel_info panel_info_default = {
 		.raster_order           = 0,
 };
 
-static int tilcdc_external_mode_valid(struct drm_connector *connector,
-				      struct drm_display_mode *mode)
-{
-	struct tilcdc_drm_private *priv = connector->dev->dev_private;
-	int ret;
-
-	ret = tilcdc_crtc_mode_valid(priv->crtc, mode);
-	if (ret != MODE_OK)
-		return ret;
-
-	BUG_ON(priv->external_connector != connector);
-	BUG_ON(!priv->connector_funcs);
-
-	/* If the connector has its own mode_valid call it. */
-	if (!IS_ERR(priv->connector_funcs) &&
-	    priv->connector_funcs->mode_valid)
-		return priv->connector_funcs->mode_valid(connector, mode);
-
-	return MODE_OK;
-}
-
-static int tilcdc_add_external_connector(struct drm_device *dev,
-					 struct drm_connector *connector)
-{
-	struct tilcdc_drm_private *priv = dev->dev_private;
-	struct drm_connector_helper_funcs *connector_funcs;
-
-	/* There should never be more than one connector */
-	if (WARN_ON(priv->external_connector))
-		return -EINVAL;
-
-	priv->external_connector = connector;
-	connector_funcs = devm_kzalloc(dev->dev, sizeof(*connector_funcs),
-				       GFP_KERNEL);
-	if (!connector_funcs)
-		return -ENOMEM;
-
-	/* connector->helper_private contains always struct
-	 * connector_helper_funcs pointer. For tilcdc crtc to have a
-	 * say if a specific mode is Ok, we need to install our own
-	 * helper functions. In our helper functions we copy
-	 * everything else but use our own mode_valid() (above).
-	 */
-	if (connector->helper_private) {
-		priv->connector_funcs =	connector->helper_private;
-		*connector_funcs = *priv->connector_funcs;
-	} else {
-		priv->connector_funcs = ERR_PTR(-ENOENT);
-	}
-	connector_funcs->mode_valid = tilcdc_external_mode_valid;
-	drm_connector_helper_add(connector, connector_funcs);
-
-	dev_dbg(dev->dev, "External connector '%s' connected\n",
-		connector->name);
-
-	return 0;
-}
-
 static
 struct drm_connector *tilcdc_encoder_find_connector(struct drm_device *ddev,
 						    struct drm_encoder *encoder)
@@ -118,7 +60,6 @@  struct drm_connector *tilcdc_encoder_find_connector(struct drm_device *ddev,
 int tilcdc_add_component_encoder(struct drm_device *ddev)
 {
 	struct tilcdc_drm_private *priv = ddev->dev_private;
-	struct drm_connector *connector;
 	struct drm_encoder *encoder;
 
 	list_for_each_entry(encoder, &ddev->mode_config.encoder_list, head)
@@ -130,28 +71,17 @@  int tilcdc_add_component_encoder(struct drm_device *ddev)
 		return -ENODEV;
 	}
 
-	connector = tilcdc_encoder_find_connector(ddev, encoder);
+	priv->external_connector
+		= tilcdc_encoder_find_connector(ddev, encoder);
 
-	if (!connector)
+	if (!priv->external_connector)
 		return -ENODEV;
 
 	/* Only tda998x is supported at the moment. */
 	tilcdc_crtc_set_simulate_vesa_sync(priv->crtc, true);
 	tilcdc_crtc_set_panel_info(priv->crtc, &panel_info_tda998x);
 
-	return tilcdc_add_external_connector(ddev, connector);
-}
-
-void tilcdc_remove_external_device(struct drm_device *dev)
-{
-	struct tilcdc_drm_private *priv = dev->dev_private;
-
-	/* Restore the original helper functions, if any. */
-	if (IS_ERR(priv->connector_funcs))
-		drm_connector_helper_add(priv->external_connector, NULL);
-	else if (priv->connector_funcs)
-		drm_connector_helper_add(priv->external_connector,
-					 priv->connector_funcs);
+	return 0;
 }
 
 static const struct drm_encoder_funcs tilcdc_external_encoder_funcs = {
@@ -162,7 +92,6 @@  static
 int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
 {
 	struct tilcdc_drm_private *priv = ddev->dev_private;
-	struct drm_connector *connector;
 	int ret;
 
 	priv->external_encoder->possible_crtcs = BIT(0);
@@ -175,13 +104,12 @@  int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
 
 	tilcdc_crtc_set_panel_info(priv->crtc, &panel_info_default);
 
-	connector = tilcdc_encoder_find_connector(ddev, priv->external_encoder);
-	if (!connector)
+	priv->external_connector =
+		tilcdc_encoder_find_connector(ddev, priv->external_encoder);
+	if (!priv->external_connector)
 		return -ENODEV;
 
-	ret = tilcdc_add_external_connector(ddev, connector);
-
-	return ret;
+	return 0;
 }
 
 int tilcdc_attach_external_device(struct drm_device *ddev)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.h b/drivers/gpu/drm/tilcdc/tilcdc_external.h
index 763d18f006c7..a28b9df68c8f 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_external.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_external.h
@@ -19,7 +19,6 @@ 
 #define __TILCDC_EXTERNAL_H__
 
 int tilcdc_add_component_encoder(struct drm_device *dev);
-void tilcdc_remove_external_device(struct drm_device *dev);
 int tilcdc_get_external_components(struct device *dev,
 				   struct component_match **match);
 int tilcdc_attach_external_device(struct drm_device *ddev);
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index a1acab39d87f..7c26f81fb3f0 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -170,14 +170,6 @@  static int panel_connector_get_modes(struct drm_connector *connector)
 	return i;
 }
 
-static int panel_connector_mode_valid(struct drm_connector *connector,
-		  struct drm_display_mode *mode)
-{
-	struct tilcdc_drm_private *priv = connector->dev->dev_private;
-	/* our only constraints are what the crtc can generate: */
-	return tilcdc_crtc_mode_valid(priv->crtc, mode);
-}
-
 static struct drm_encoder *panel_connector_best_encoder(
 		struct drm_connector *connector)
 {
@@ -195,7 +187,6 @@  static const struct drm_connector_funcs panel_connector_funcs = {
 
 static const struct drm_connector_helper_funcs panel_connector_helper_funcs = {
 	.get_modes          = panel_connector_get_modes,
-	.mode_valid         = panel_connector_mode_valid,
 	.best_encoder       = panel_connector_best_encoder,
 };
 
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
index daebf1aa6b0a..54c6d825b1a0 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
@@ -183,14 +183,6 @@  static int tfp410_connector_get_modes(struct drm_connector *connector)
 	return ret;
 }
 
-static int tfp410_connector_mode_valid(struct drm_connector *connector,
-		  struct drm_display_mode *mode)
-{
-	struct tilcdc_drm_private *priv = connector->dev->dev_private;
-	/* our only constraints are what the crtc can generate: */
-	return tilcdc_crtc_mode_valid(priv->crtc, mode);
-}
-
 static struct drm_encoder *tfp410_connector_best_encoder(
 		struct drm_connector *connector)
 {
@@ -209,7 +201,6 @@  static const struct drm_connector_funcs tfp410_connector_funcs = {
 
 static const struct drm_connector_helper_funcs tfp410_connector_helper_funcs = {
 	.get_modes          = tfp410_connector_get_modes,
-	.mode_valid         = tfp410_connector_mode_valid,
 	.best_encoder       = tfp410_connector_best_encoder,
 };