[PATCH 18/21] drm/omap: Don't call EDID read operation recursively
Sebastian Reichel
sre at kernel.org
Mon Jun 11 22:47:09 UTC 2018
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 at ideasonboard.com>
> ---
Reviewed-by: Sebastian Reichel <sebastian.reichel at collabora.co.uk>
-- 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20180612/174804b4/attachment.sig>
More information about the dri-devel
mailing list