Hi,

On 04/22/2013 02:57 PM, Richard Hughes wrote:
At the moment we're only extracting interesting strings. We have to be quite
careful parsing the EDID data, as vendors like to do insane things.

You should add some information of why this patch is necessary to the commit message. It seems you added that to the cover letter, but it should be in the commit message so we can find that later without going through the list archives.

The original EDID parsing code was written by me for gnome-color-manager.
---
  src/compositor-drm.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++
  src/compositor.h     |   2 +-
  2 files changed, 140 insertions(+), 1 deletion(-)

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index da1ba79..1909712 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c

[...]

@@ -1499,7 +1603,10 @@ create_output_for_connector(struct drm_compositor *ec,
        drmModeEncoder *encoder;
        drmModeModeInfo crtc_mode;
        drmModeCrtc *crtc;
+       drmModePropertyPtr property;
+       drmModePropertyBlobPtr edid_blob = NULL;
        int i;
+       int rc;
        char name[32];
        const char *type_name;

@@ -1517,6 +1624,7 @@ create_output_for_connector(struct drm_compositor *ec,
        output->base.subpixel = drm_subpixel_to_wayland(connector->subpixel);
        output->base.make = "unknown";
        output->base.model = "unknown";
+       output->base.serial = "unknown";
        wl_list_init(&output->base.mode_list);

        if (connector->connector_type < ARRAY_LENGTH(connector_type_names))
@@ -1642,6 +1750,37 @@ create_output_for_connector(struct drm_compositor *ec,

        wl_list_insert(ec->base.output_list.prev, &output->base.link);

+       /* find and parse the EDID blob */
+       for (i = 0; i < connector->count_props && !edid_blob; i++) {
+               property = drmModeGetProperty(ec->drm.fd, connector->props[i]);
+               if (!property)
+                       continue;
+               if ((property->flags & DRM_MODE_PROP_BLOB) &&
+                   !strcmp(property->name, "EDID")) {
+                       edid_blob = drmModeGetPropertyBlob(ec->drm.fd,
+                                                          
connector->prop_values[i]);
+               }
+               drmModeFreeProperty(property);
+       }
+       if (edid_blob) {
+               rc = edid_parse(&output->edid,
+                               edid_blob->data,
+                               edid_blob->length);
+               if (!rc) {
+                       weston_log("EDID data '%s', '%s', '%s'\n",
+                                  output->edid.pnp_id,
+                                  output->edid.monitor_name,
+                                  output->edid.serial_number);
+                       if (output->edid.pnp_id[0] != '\0')
+                               output->base.make = output->edid.pnp_id;
+                       if (output->edid.monitor_name[0] != '\0')
+                               output->base.model = output->edid.monitor_name;
+                       if (output->edid.serial_number[0] != '\0')
+                               output->base.serial = 
output->edid.serial_number;
+               }
+               drmModeFreePropertyBlob(edid_blob);
+       }
+

Could you split this whole hunk into a separate function, create_output_for_connector() is already awfully long.

        output->base.origin = output->base.current;
        output->base.start_repaint_loop = drm_output_start_repaint_loop;
        output->base.repaint = drm_output_repaint;
diff --git a/src/compositor.h b/src/compositor.h
index 1e999a6..fa6162c 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -178,7 +178,7 @@ struct weston_output {
        uint32_t frame_time;
        int disable_planes;

-       char *make, *model;
+       char *make, *model, *serial;

I think it would be better to call this field 'serial_number'. We normally use 'serial' for numbers obtained from wl_display_next_serial(), and that may lead to unnecessary confusion.


Cheers,
Ander

        uint32_t subpixel;
        uint32_t transform;
        


_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to