Re: [PATCH 14/21] drm/omap: Don't call HPD registration operations recursively

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, Jun 06, 2018 at 12:36:43PM +0300, Laurent Pinchart wrote:
> Instead of calling the hot-plug detection callback registration
> operations (.register_hpd_cb() and .unregister_hpd_cb()) recursively
> from the display device back to the first device that provides hot plug
> detection support, iterate over the devices manually in the DRM
> connector code. This moves the complexity to a single central location
> and simplifies the logic in omap_dss_device drivers.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxxxx>

-- Sebastian

>  drivers/gpu/drm/omapdrm/displays/connector-dvi.c   |  8 ++-
>  drivers/gpu/drm/omapdrm/displays/connector-hdmi.c  | 67 ++++++++----------
>  .../gpu/drm/omapdrm/displays/encoder-tpd12s015.c   |  3 +-
>  drivers/gpu/drm/omapdrm/omap_connector.c           | 79 ++++++++++++++--------
>  4 files changed, 88 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> index f1674b3eee50..e9353e4cd297 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> @@ -372,8 +372,12 @@ static int dvic_probe(struct platform_device *pdev)
>  	dssdev->type = OMAP_DISPLAY_TYPE_DVI;
>  	dssdev->owner = THIS_MODULE;
>  	dssdev->of_ports = BIT(0);
> -	dssdev->ops_flags = ddata->hpd_gpio || ddata->i2c_adapter
> -			  ? OMAP_DSS_DEVICE_OP_DETECT : 0;
> +
> +	if (ddata->hpd_gpio)
> +		dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT
> +				  | OMAP_DSS_DEVICE_OP_HPD;
> +	else if (ddata->i2c_adapter)
> +		dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT;
>  
>  	omapdss_display_init(dssdev);
>  	omapdss_device_register(dssdev);
> diff --git a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> index 0d22d7004c98..8eae973474dd 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> @@ -153,62 +153,53 @@ static int hdmic_register_hpd_cb(struct omap_dss_device *dssdev,
>  				 void *cb_data)
>  {
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
> -	struct omap_dss_device *src = dssdev->src;
>  
> -	if (ddata->hpd_gpio) {
> -		mutex_lock(&ddata->hpd_lock);
> -		ddata->hpd_cb = cb;
> -		ddata->hpd_cb_data = cb_data;
> -		mutex_unlock(&ddata->hpd_lock);
> -		return 0;
> -	} else if (src->ops->register_hpd_cb) {
> -		return src->ops->register_hpd_cb(src, cb, cb_data);
> -	}
> +	if (!ddata->hpd_gpio)
> +		return -ENOTSUPP;
>  
> -	return -ENOTSUPP;
> +	mutex_lock(&ddata->hpd_lock);
> +	ddata->hpd_cb = cb;
> +	ddata->hpd_cb_data = cb_data;
> +	mutex_unlock(&ddata->hpd_lock);
> +
> +	return 0;
>  }
>  
>  static void hdmic_unregister_hpd_cb(struct omap_dss_device *dssdev)
>  {
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
> -	struct omap_dss_device *src = dssdev->src;
>  
> -	if (ddata->hpd_gpio) {
> -		mutex_lock(&ddata->hpd_lock);
> -		ddata->hpd_cb = NULL;
> -		ddata->hpd_cb_data = NULL;
> -		mutex_unlock(&ddata->hpd_lock);
> -	} else if (src->ops->unregister_hpd_cb) {
> -		src->ops->unregister_hpd_cb(src);
> -	}
> +	if (!ddata->hpd_gpio)
> +		return;
> +
> +	mutex_lock(&ddata->hpd_lock);
> +	ddata->hpd_cb = NULL;
> +	ddata->hpd_cb_data = NULL;
> +	mutex_unlock(&ddata->hpd_lock);
>  }
>  
>  static void hdmic_enable_hpd(struct omap_dss_device *dssdev)
>  {
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
> -	struct omap_dss_device *src = dssdev->src;
>  
> -	if (ddata->hpd_gpio) {
> -		mutex_lock(&ddata->hpd_lock);
> -		ddata->hpd_enabled = true;
> -		mutex_unlock(&ddata->hpd_lock);
> -	} else if (src->ops->enable_hpd) {
> -		src->ops->enable_hpd(src);
> -	}
> +	if (!ddata->hpd_gpio)
> +		return;
> +
> +	mutex_lock(&ddata->hpd_lock);
> +	ddata->hpd_enabled = true;
> +	mutex_unlock(&ddata->hpd_lock);
>  }
>  
>  static void hdmic_disable_hpd(struct omap_dss_device *dssdev)
>  {
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
> -	struct omap_dss_device *src = dssdev->src;
>  
> -	if (ddata->hpd_gpio) {
> -		mutex_lock(&ddata->hpd_lock);
> -		ddata->hpd_enabled = false;
> -		mutex_unlock(&ddata->hpd_lock);
> -	} else if (src->ops->disable_hpd) {
> -		src->ops->disable_hpd(src);
> -	}
> +	if (!ddata->hpd_gpio)
> +		return;
> +
> +	mutex_lock(&ddata->hpd_lock);
> +	ddata->hpd_enabled = false;
> +	mutex_unlock(&ddata->hpd_lock);
>  }
>  
>  static int hdmic_set_hdmi_mode(struct omap_dss_device *dssdev, bool hdmi_mode)
> @@ -314,7 +305,9 @@ static int hdmic_probe(struct platform_device *pdev)
>  	dssdev->type = OMAP_DISPLAY_TYPE_HDMI;
>  	dssdev->owner = THIS_MODULE;
>  	dssdev->of_ports = BIT(0);
> -	dssdev->ops_flags = ddata->hpd_gpio ? OMAP_DSS_DEVICE_OP_DETECT : 0;
> +	dssdev->ops_flags = ddata->hpd_gpio
> +			  ? OMAP_DSS_DEVICE_OP_DETECT | OMAP_DSS_DEVICE_OP_HPD
> +			  : 0;
>  
>  	omapdss_display_init(dssdev);
>  	omapdss_device_register(dssdev);
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> index f37878ca6077..e5a25baa0364 100644
> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> @@ -289,7 +289,8 @@ static int tpd_probe(struct platform_device *pdev)
>  	dssdev->output_type = OMAP_DISPLAY_TYPE_HDMI;
>  	dssdev->owner = THIS_MODULE;
>  	dssdev->of_ports = BIT(1) | BIT(0);
> -	dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT;
> +	dssdev->ops_flags = OMAP_DSS_DEVICE_OP_DETECT
> +			  | OMAP_DSS_DEVICE_OP_HPD;
>  
>  	dssdev->next = omapdss_of_find_connected_device(pdev->dev.of_node, 1);
>  	if (IS_ERR(dssdev->next)) {
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
> index 6c02c2492d47..578b0b105755 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -57,6 +57,21 @@ bool omap_connector_get_hdmi_mode(struct drm_connector *connector)
>  	return omap_connector->hdmi_mode;
>  }
>  
> +static struct omap_dss_device *
> +omap_connector_find_device(struct drm_connector *connector,
> +			   enum omap_dss_device_ops_flag op)
> +{
> +	struct omap_connector *omap_connector = to_omap_connector(connector);
> +	struct omap_dss_device *dssdev;
> +
> +	for (dssdev = omap_connector->dssdev; dssdev; dssdev = dssdev->src) {
> +		if (dssdev->ops_flags & op)
> +			return dssdev;
> +	}
> +
> +	return NULL;
> +}
> +
>  static enum drm_connector_status omap_connector_detect(
>  		struct drm_connector *connector, bool force)
>  {
> @@ -64,10 +79,8 @@ static enum drm_connector_status omap_connector_detect(
>  	struct omap_dss_device *dssdev;
>  	enum drm_connector_status status;
>  
> -	for (dssdev = omap_connector->dssdev; dssdev; dssdev = dssdev->src) {
> -		if (dssdev->ops_flags & OMAP_DSS_DEVICE_OP_DETECT)
> -			break;
> -	}
> +	dssdev = omap_connector_find_device(connector,
> +					    OMAP_DSS_DEVICE_OP_DETECT);
>  
>  	if (dssdev) {
>  		if (dssdev->ops->detect(dssdev))
> @@ -96,18 +109,21 @@ static enum drm_connector_status omap_connector_detect(
>  static void omap_connector_destroy(struct drm_connector *connector)
>  {
>  	struct omap_connector *omap_connector = to_omap_connector(connector);
> -	struct omap_dss_device *dssdev = omap_connector->dssdev;
> +	struct omap_dss_device *dssdev;
>  
>  	DBG("%s", omap_connector->dssdev->name);
> -	if (connector->polled == DRM_CONNECTOR_POLL_HPD &&
> -	    dssdev->ops->unregister_hpd_cb) {
> +
> +	if (connector->polled == DRM_CONNECTOR_POLL_HPD) {
> +		dssdev = omap_connector_find_device(connector,
> +						    OMAP_DSS_DEVICE_OP_HPD);
>  		dssdev->ops->unregister_hpd_cb(dssdev);
>  	}
> +
>  	drm_connector_unregister(connector);
>  	drm_connector_cleanup(connector);
>  	kfree(omap_connector);
>  
> -	omapdss_device_put(dssdev);
> +	omapdss_device_put(omap_connector->dssdev);
>  }
>  
>  #define MAX_EDID  512
> @@ -258,45 +274,50 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
>  {
>  	struct drm_connector *connector = NULL;
>  	struct omap_connector *omap_connector;
> -	bool hpd_supported = false;
>  
>  	DBG("%s", dssdev->name);
>  
> -	omapdss_device_get(dssdev);
> -
>  	omap_connector = kzalloc(sizeof(*omap_connector), GFP_KERNEL);
>  	if (!omap_connector)
>  		goto fail;
>  
> -	omap_connector->dssdev = dssdev;
> +	omap_connector->dssdev = omapdss_device_get(dssdev);
>  
>  	connector = &omap_connector->base;
> +	connector->interlace_allowed = 1;
> +	connector->doublescan_allowed = 0;
>  
>  	drm_connector_init(dev, connector, &omap_connector_funcs,
>  				connector_type);
>  	drm_connector_helper_add(connector, &omap_connector_helper_funcs);
>  
> -	if (dssdev->ops->register_hpd_cb) {
> -		int ret = dssdev->ops->register_hpd_cb(dssdev,
> -						       omap_connector_hpd_cb,
> -						       omap_connector);
> -		if (!ret)
> -			hpd_supported = true;
> -		else if (ret != -ENOTSUPP)
> +	/*
> +	 * Initialize connector status handling. First try to find a device that
> +	 * supports hot-plug reporting. If it fails, fall back to a device that
> +	 * support polling. If that fails too, we don't support hot-plug
> +	 * detection at all.
> +	 */
> +	dssdev = omap_connector_find_device(connector, OMAP_DSS_DEVICE_OP_HPD);
> +	if (dssdev) {
> +		int ret;
> +
> +		ret = dssdev->ops->register_hpd_cb(dssdev,
> +						   omap_connector_hpd_cb,
> +						   omap_connector);
> +		if (ret < 0)
>  			DBG("%s: Failed to register HPD callback (%d).",
>  			    dssdev->name, ret);
> +		else
> +			connector->polled = DRM_CONNECTOR_POLL_HPD;
>  	}
>  
> -	if (hpd_supported)
> -		connector->polled = DRM_CONNECTOR_POLL_HPD;
> -	else if (dssdev->ops->detect)
> -		connector->polled = DRM_CONNECTOR_POLL_CONNECT |
> -				    DRM_CONNECTOR_POLL_DISCONNECT;
> -	else
> -		connector->polled = 0;
> -
> -	connector->interlace_allowed = 1;
> -	connector->doublescan_allowed = 0;
> +	if (!connector->polled) {
> +		dssdev = omap_connector_find_device(connector,
> +						    OMAP_DSS_DEVICE_OP_DETECT);
> +		if (dssdev)
> +			connector->polled = DRM_CONNECTOR_POLL_CONNECT |
> +					    DRM_CONNECTOR_POLL_DISCONNECT;
> +	}
>  
>  	return connector;
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux