Re: [PATCH v2 4/5] drm/udl: Untangle .get_modes() and .detect_ctx()

[Thomas Zimmermann]

Hi

Am 10.05.24 um 14:17 schrieb Jani Nikula:

+       /*
+        * The adapter sends all-zeros if no monitor has been
+        * connected. We consider anything else a connection.
+        */
+       return memcmp(no_edid, hdr, 8) != 0;

Nitpick, this works, but you can drop the no_edid buf by using:

        return memchr_inv(hdr, 0, sizeof(hdr));

Makes sense to me. Thanks for reviewing.

Best regards
Thomas

Up to you,

Reviewed-by: Jani Nikula <jani.nik...@intel.com>

+}
+
+const struct drm_edid *udl_edid_read(struct drm_connector *connector)
+{
+       struct udl_device *udl = to_udl(connector->dev);
+
+       return drm_edid_read_custom(connector, udl_read_edid_block, udl);
+}
diff --git a/drivers/gpu/drm/udl/udl_edid.h b/drivers/gpu/drm/udl/udl_edid.h
new file mode 100644
index 0000000000000..fe15ff3752b7d
--- /dev/null
+++ b/drivers/gpu/drm/udl/udl_edid.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef UDL_EDID_H
+#define UDL_EDID_H
+
+#include <linux/types.h>
+
+struct drm_connector;
+struct drm_edid;
+struct udl_device;
+
+bool udl_probe_edid(struct udl_device *udl);
+const struct drm_edid *udl_edid_read(struct drm_connector *connector);
+
+#endif
diff --git a/drivers/gpu/drm/udl/udl_modeset.c 
b/drivers/gpu/drm/udl/udl_modeset.c
index 3df9fc38388b4..4236ce57f5945 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -25,6 +25,7 @@
  #include <drm/drm_vblank.h>

#include "udl_drv.h"

+#include "udl_edid.h"
  #include "udl_proto.h"

/*

@@ -415,97 +416,44 @@ static const struct drm_encoder_funcs udl_encoder_funcs = 
{

static int udl_connector_helper_get_modes(struct drm_connector *connector)

  {
-       struct udl_connector *udl_connector = to_udl_connector(connector);
+       const struct drm_edid *drm_edid;
+       int count;

- drm_connector_update_edid_property(connector, udl_connector->edid);

-       if (udl_connector->edid)
-               return drm_add_edid_modes(connector, udl_connector->edid);
+       drm_edid = udl_edid_read(connector);
+       drm_edid_connector_update(connector, drm_edid);
+       count = drm_edid_connector_add_modes(connector);
+       drm_edid_free(drm_edid);

- return 0;

+       return count;
  }

-static const struct drm_connector_helper_funcs udl_connector_helper_funcs = {

-       .get_modes = udl_connector_helper_get_modes,
-};
-
-static int udl_get_edid_block(void *data, u8 *buf, unsigned int block, size_t 
len)
+static int udl_connector_helper_detect_ctx(struct drm_connector *connector,
+                                          struct drm_modeset_acquire_ctx *ctx,
+                                          bool force)
  {
-       struct udl_device *udl = data;
-       struct drm_device *dev = &udl->drm;
-       struct usb_device *udev = udl_to_usb_device(udl);
-       u8 *read_buff;
-       int idx, ret;
-       size_t i;
-
-       read_buff = kmalloc(2, GFP_KERNEL);
-       if (!read_buff)
-               return -ENOMEM;
+       struct udl_device *udl = to_udl(connector->dev);

- if (!drm_dev_enter(dev, &idx)) {

-               ret = -ENODEV;
-               goto err_kfree;
-       }
-
-       for (i = 0; i < len; i++) {
-               int bval = (i + block * EDID_LENGTH) << 8;
-
-               ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
-                                     0x02, (0x80 | (0x02 << 5)), bval,
-                                     0xA1, read_buff, 2, USB_CTRL_GET_TIMEOUT);
-               if (ret < 0) {
-                       drm_err(dev, "Read EDID byte %zu failed err %x\n", i, 
ret);
-                       goto err_drm_dev_exit;
-               } else if (ret < 1) {
-                       ret = -EIO;
-                       drm_err(dev, "Read EDID byte %zu failed\n", i);
-                       goto err_drm_dev_exit;
-               }
-
-               buf[i] = read_buff[1];
-       }
+       if (udl_probe_edid(udl))
+               return connector_status_connected;

- drm_dev_exit(idx);

-       kfree(read_buff);
-
-       return 0;
-
-err_drm_dev_exit:
-       drm_dev_exit(idx);
-err_kfree:
-       kfree(read_buff);
-       return ret;
+       return connector_status_disconnected;
  }

-static enum drm_connector_status udl_connector_detect(struct drm_connector *connector, bool force)

-{
-       struct drm_device *dev = connector->dev;
-       struct udl_device *udl = to_udl(dev);
-       struct udl_connector *udl_connector = to_udl_connector(connector);
-       enum drm_connector_status status = connector_status_disconnected;
-
-       /* cleanup previous EDID */
-       kfree(udl_connector->edid);
-       udl_connector->edid = NULL;
-
-       udl_connector->edid = drm_do_get_edid(connector, udl_get_edid_block, 
udl);
-       if (udl_connector->edid)
-               status = connector_status_connected;
-
-       return status;
-}
+static const struct drm_connector_helper_funcs udl_connector_helper_funcs = {
+       .get_modes = udl_connector_helper_get_modes,
+       .detect_ctx = udl_connector_helper_detect_ctx,
+};

static void udl_connector_destroy(struct drm_connector *connector)

  {
        struct udl_connector *udl_connector = to_udl_connector(connector);

drm_connector_cleanup(connector);

-       kfree(udl_connector->edid);
        kfree(udl_connector);
  }

static const struct drm_connector_funcs udl_connector_funcs = {

        .reset = drm_atomic_helper_connector_reset,
-       .detect = udl_connector_detect,
        .fill_modes = drm_helper_probe_single_connector_modes,
        .destroy = udl_connector_destroy,
        .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)