Re: [PATCH 18/21] drm/omap: Don't call EDID read operation recursively

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

 



Hi,

On Wed, Jun 06, 2018 at 12:36:47PM +0300, Laurent Pinchart wrote:
> Instead of calling the EDID read operation (.read_edid()) recursively
> from the display device back to the first device that provides EDID read
> 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   |  15 +--
>  drivers/gpu/drm/omapdrm/displays/connector-hdmi.c  |  11 ---
>  .../gpu/drm/omapdrm/displays/encoder-tpd12s015.c   |  13 ---
>  drivers/gpu/drm/omapdrm/dss/hdmi4.c                |   1 +
>  drivers/gpu/drm/omapdrm/dss/hdmi5.c                |   1 +
>  drivers/gpu/drm/omapdrm/omap_connector.c           | 101 ++++++++++++---------
>  6 files changed, 65 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> index 6be260ff6458..eae4108330f1 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-dvi.c
> @@ -166,12 +166,6 @@ static int dvic_read_edid(struct omap_dss_device *dssdev,
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	int r, l, bytes_read;
>  
> -	if (ddata->hpd_gpio && !gpiod_get_value_cansleep(ddata->hpd_gpio))
> -		return -ENODEV;
> -
> -	if (!ddata->i2c_adapter)
> -		return -ENODEV;
> -
>  	l = min(EDID_LENGTH, len);
>  	r = dvic_ddc_read(ddata->i2c_adapter, edid, l, 0);
>  	if (r)
> @@ -341,10 +335,11 @@ static int dvic_probe(struct platform_device *pdev)
>  	dssdev->of_ports = BIT(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;
> +		dssdev->ops_flags |= OMAP_DSS_DEVICE_OP_DETECT
> +				  |  OMAP_DSS_DEVICE_OP_HPD;
> +	if (ddata->i2c_adapter)
> +		dssdev->ops_flags |= OMAP_DSS_DEVICE_OP_DETECT
> +				  |  OMAP_DSS_DEVICE_OP_EDID;
>  
>  	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 6f2364afb14a..16dc22edcb8e 100644
> --- a/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/connector-hdmi.c
> @@ -15,8 +15,6 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> -#include <drm/drm_edid.h>
> -
>  #include "../dss/omapdss.h"
>  
>  static const struct videomode hdmic_default_vm = {
> @@ -126,14 +124,6 @@ static int hdmic_check_timings(struct omap_dss_device *dssdev,
>  	return src->ops->check_timings(src, vm);
>  }
>  
> -static int hdmic_read_edid(struct omap_dss_device *dssdev,
> -		u8 *edid, int len)
> -{
> -	struct omap_dss_device *src = dssdev->src;
> -
> -	return src->ops->read_edid(src, edid, len);
> -}
> -
>  static bool hdmic_detect(struct omap_dss_device *dssdev)
>  {
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
> @@ -190,7 +180,6 @@ static const struct omap_dss_device_ops hdmic_ops = {
>  	.get_timings		= hdmic_get_timings,
>  	.check_timings		= hdmic_check_timings,
>  
> -	.read_edid		= hdmic_read_edid,
>  	.detect			= hdmic_detect,
>  	.register_hpd_cb	= hdmic_register_hpd_cb,
>  	.unregister_hpd_cb	= hdmic_unregister_hpd_cb,
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> index 2772af84531a..dbbf9683bd51 100644
> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> @@ -115,18 +115,6 @@ static int tpd_check_timings(struct omap_dss_device *dssdev,
>  	return src->ops->check_timings(src, vm);
>  }
>  
> -static int tpd_read_edid(struct omap_dss_device *dssdev,
> -		u8 *edid, int len)
> -{
> -	struct panel_drv_data *ddata = to_panel_data(dssdev);
> -	struct omap_dss_device *src = dssdev->src;
> -
> -	if (!gpiod_get_value_cansleep(ddata->hpd_gpio))
> -		return -ENODEV;
> -
> -	return src->ops->read_edid(src, edid, len);
> -}
> -
>  static bool tpd_detect(struct omap_dss_device *dssdev)
>  {
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
> @@ -180,7 +168,6 @@ static const struct omap_dss_device_ops tpd_ops = {
>  	.disable		= tpd_disable,
>  	.check_timings		= tpd_check_timings,
>  	.set_timings		= tpd_set_timings,
> -	.read_edid		= tpd_read_edid,
>  	.detect			= tpd_detect,
>  	.register_hpd_cb	= tpd_register_hpd_cb,
>  	.unregister_hpd_cb	= tpd_unregister_hpd_cb,
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> index bebce93fed3e..c92564300446 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> @@ -711,6 +711,7 @@ static int hdmi4_init_output(struct omap_hdmi *hdmi)
>  	out->ops = &hdmi_ops;
>  	out->owner = THIS_MODULE;
>  	out->of_ports = BIT(0);
> +	out->ops_flags = OMAP_DSS_DEVICE_OP_EDID;
>  
>  	out->next = omapdss_of_find_connected_device(out->dev->of_node, 0);
>  	if (IS_ERR(out->next)) {
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> index 7c07e0208107..2aaa8ee61662 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> @@ -703,6 +703,7 @@ static int hdmi5_init_output(struct omap_hdmi *hdmi)
>  	out->ops = &hdmi_ops;
>  	out->owner = THIS_MODULE;
>  	out->of_ports = BIT(0);
> +	out->ops_flags = OMAP_DSS_DEVICE_OP_EDID;
>  
>  	out->next = omapdss_of_find_connected_device(out->dev->of_node, 0);
>  	if (IS_ERR(out->next)) {
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
> index 1b3f85a02c85..a1d379816732 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -170,65 +170,80 @@ static void omap_connector_destroy(struct drm_connector *connector)
>  
>  #define MAX_EDID  512
>  
> +static int omap_connector_get_modes_edid(struct drm_connector *connector,
> +					 struct omap_dss_device *dssdev)
> +{
> +	struct omap_connector *omap_connector = to_omap_connector(connector);
> +	enum drm_connector_status status;
> +	void *edid;
> +	int n;
> +
> +	status = omap_connector_detect(connector, false);
> +	if (status != connector_status_connected)
> +		goto no_edid;
> +
> +	edid = kzalloc(MAX_EDID, GFP_KERNEL);
> +	if (!edid)
> +		goto no_edid;
> +
> +	if (dssdev->ops->read_edid(dssdev, edid, MAX_EDID) <= 0 ||
> +	    !drm_edid_is_valid(edid)) {
> +		kfree(edid);
> +		goto no_edid;
> +	}
> +
> +	drm_mode_connector_update_edid_property(connector, edid);
> +	n = drm_add_edid_modes(connector, edid);
> +
> +	omap_connector->hdmi_mode = drm_detect_hdmi_monitor(edid);
> +
> +	kfree(edid);
> +	return n;
> +
> +no_edid:
> +	drm_mode_connector_update_edid_property(connector, NULL);
> +	return 0;
> +}
> +
>  static int omap_connector_get_modes(struct drm_connector *connector)
>  {
>  	struct omap_connector *omap_connector = to_omap_connector(connector);
> -	struct omap_dss_device *dssdev = omap_connector->dssdev;
> -	struct drm_device *dev = connector->dev;
> -	int n = 0;
> +	struct omap_dss_device *dssdev;
> +	struct drm_display_mode *mode;
> +	struct videomode vm = {0};
>  
>  	DBG("%s", omap_connector->dssdev->name);
>  
> -	/* if display exposes EDID, then we parse that in the normal way to
> -	 * build table of supported modes.. otherwise (ie. fixed resolution
> +	/*
> +	 * If display exposes EDID, then we parse that in the normal way to
> +	 * build table of supported modes. Otherwise (ie. fixed resolution
>  	 * LCD panels) we just return a single mode corresponding to the
> -	 * currently configured timings:
> +	 * currently configured timings.
>  	 */
> -	if (dssdev->ops->read_edid) {
> -		void *edid = kzalloc(MAX_EDID, GFP_KERNEL);
> -
> -		if (!edid)
> -			return 0;
> -
> -		if ((dssdev->ops->read_edid(dssdev, edid, MAX_EDID) > 0) &&
> -				drm_edid_is_valid(edid)) {
> -			drm_mode_connector_update_edid_property(
> -					connector, edid);
> -			n = drm_add_edid_modes(connector, edid);
> -
> -			omap_connector->hdmi_mode =
> -				drm_detect_hdmi_monitor(edid);
> -		} else {
> -			drm_mode_connector_update_edid_property(
> -					connector, NULL);
> -		}
> -
> -		kfree(edid);
> -	} else {
> -		struct drm_display_mode *mode = drm_mode_create(dev);
> -		struct videomode vm = {0};
> +	dssdev = omap_connector_find_device(connector,
> +					    OMAP_DSS_DEVICE_OP_EDID);
> +	if (dssdev)
> +		return omap_connector_get_modes_edid(connector, dssdev);
>  
> -		if (!mode)
> -			return 0;
> +	mode = drm_mode_create(connector->dev);
> +	if (!mode)
> +		return 0;
>  
> -		dssdev->ops->get_timings(dssdev, &vm);
> +	dssdev = omap_connector->dssdev;
> +	dssdev->ops->get_timings(dssdev, &vm);
>  
> -		drm_display_mode_from_videomode(&vm, mode);
> +	drm_display_mode_from_videomode(&vm, mode);
>  
> -		mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> -		drm_mode_set_name(mode);
> -		drm_mode_probed_add(connector, mode);
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	drm_mode_set_name(mode);
> +	drm_mode_probed_add(connector, mode);
>  
> -		if (dssdev->driver && dssdev->driver->get_size) {
> -			dssdev->driver->get_size(dssdev,
> +	if (dssdev->driver && dssdev->driver->get_size)
> +		dssdev->driver->get_size(dssdev,
>  					 &connector->display_info.width_mm,
>  					 &connector->display_info.height_mm);
> -		}
>  
> -		n = 1;
> -	}
> -
> -	return n;
> +	return 1;
>  }
>  
>  static int omap_connector_mode_valid(struct drm_connector *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