From 0a6ddf4dc2f3af38c45b8371268f6acabc965c3d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 15 Jan 2019 09:48:30 +0100 Subject: [PATCH 01/16] ply-array: Add ply_array_contains_uint32_element function Add a new ply_array_contains_uint32_element which checks if the queried ply-array contains an element with the passed in uint32_t value. Signed-off-by: Hans de Goede --- src/libply/ply-array.c | 18 ++++++++++++++++++ src/libply/ply-array.h | 3 +++ 2 files changed, 21 insertions(+) diff --git a/src/libply/ply-array.c b/src/libply/ply-array.c index 7d239534..26532cf7 100644 --- a/src/libply/ply-array.c +++ b/src/libply/ply-array.c @@ -169,4 +169,22 @@ ply_array_steal_uint32_elements (ply_array_t *array) return data; } +bool +ply_array_contains_uint32_element (ply_array_t *array, const uint32_t element) +{ + uint32_t const *elements; + int i, size; + + assert (array->element_type == PLY_ARRAY_ELEMENT_TYPE_UINT32); + + elements = (uint32_t const *) ply_buffer_get_bytes (array->buffer); + size = (ply_buffer_get_size (array->buffer) / sizeof(const uint32_t)) - 1; + + for (i = 0; i < size; i++) + if (elements[i] == element) + return true; + + return false; +} + /* vim: set ts=4 sw=4 expandtab autoindent cindent cino={.5s,(0: */ diff --git a/src/libply/ply-array.h b/src/libply/ply-array.h index 777485cc..3d8e9c47 100644 --- a/src/libply/ply-array.h +++ b/src/libply/ply-array.h @@ -23,6 +23,7 @@ #define PLY_ARRAY_H #include +#include typedef struct _ply_array ply_array_t; typedef enum _ply_array_element_type ply_array_element_type_t; @@ -46,6 +47,8 @@ uint32_t const *ply_array_get_uint32_elements (ply_array_t *array); void **ply_array_steal_pointer_elements (ply_array_t *array); uint32_t *ply_array_steal_uint32_elements (ply_array_t *array); +bool ply_array_contains_uint32_element (ply_array_t *array, + const uint32_t element); #endif #endif /* PLY_ARRAY_H */ From 88f1b1dc0e7c51d7065403783ab1057b0f3ec31d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 16 Jan 2019 09:21:59 +0100 Subject: [PATCH 02/16] ply-renderer: Add ply_renderer_handle_change_event function This function can be called to notify the renderer of udev change-events. This is a preparation patch for adding support for hotplugging monitors while plymouth is running. Signed-off-by: Hans de Goede --- src/libply-splash-core/ply-renderer-plugin.h | 1 + src/libply-splash-core/ply-renderer.c | 9 +++++++++ src/libply-splash-core/ply-renderer.h | 2 ++ 3 files changed, 12 insertions(+) diff --git a/src/libply-splash-core/ply-renderer-plugin.h b/src/libply-splash-core/ply-renderer-plugin.h index db18d195..6b6ff25a 100644 --- a/src/libply-splash-core/ply-renderer-plugin.h +++ b/src/libply-splash-core/ply-renderer-plugin.h @@ -43,6 +43,7 @@ typedef struct bool (*open_device)(ply_renderer_backend_t *backend); void (*close_device)(ply_renderer_backend_t *backend); bool (*query_device)(ply_renderer_backend_t *backend); + bool (*handle_change_event)(ply_renderer_backend_t *backend); bool (*map_to_device)(ply_renderer_backend_t *backend); void (*unmap_from_device)(ply_renderer_backend_t *backend); void (*activate)(ply_renderer_backend_t *backend); diff --git a/src/libply-splash-core/ply-renderer.c b/src/libply-splash-core/ply-renderer.c index 5e83627a..e0a0784d 100644 --- a/src/libply-splash-core/ply-renderer.c +++ b/src/libply-splash-core/ply-renderer.c @@ -297,6 +297,15 @@ ply_renderer_close (ply_renderer_t *renderer) renderer->is_active = false; } +bool +ply_renderer_handle_change_event (ply_renderer_t *renderer) +{ + if (renderer->plugin_interface->handle_change_event) + return renderer->plugin_interface->handle_change_event (renderer->backend); + + return false; +} + void ply_renderer_activate (ply_renderer_t *renderer) { diff --git a/src/libply-splash-core/ply-renderer.h b/src/libply-splash-core/ply-renderer.h index c0e0ed59..820cdb63 100644 --- a/src/libply-splash-core/ply-renderer.h +++ b/src/libply-splash-core/ply-renderer.h @@ -55,6 +55,8 @@ ply_renderer_t *ply_renderer_new (ply_renderer_type_t renderer_type, void ply_renderer_free (ply_renderer_t *renderer); bool ply_renderer_open (ply_renderer_t *renderer); void ply_renderer_close (ply_renderer_t *renderer); +/* Returns true when the heads have changed as a result of the change event */ +bool ply_renderer_handle_change_event (ply_renderer_t *renderer); void ply_renderer_activate (ply_renderer_t *renderer); void ply_renderer_deactivate (ply_renderer_t *renderer); bool ply_renderer_is_active (ply_renderer_t *renderer); From 80acc33ab37b73feb6092d477f5198e4f57dd649 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 14 Jan 2019 17:36:22 +0100 Subject: [PATCH 03/16] drm: Refactor ply_renderer_head_add_connector and ply_renderer_head_new Both these function take a bunch of info coming from the ply_output_t struct and with upcoming changes they are going to be using even more ply_output_t fields. Instead of passing all these fields one by one, simply directly pass a pointer to ply_output_t. Signed-off-by: Hans de Goede --- src/plugins/renderers/drm/plugin.c | 56 ++++++++++++++---------------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c index 20543eaa..afc8d65e 100644 --- a/src/plugins/renderers/drm/plugin.c +++ b/src/plugins/renderers/drm/plugin.c @@ -472,33 +472,29 @@ ply_renderer_connector_get_rotation_and_tiled (ply_renderer_backend_t *back static bool ply_renderer_head_add_connector (ply_renderer_head_t *head, - drmModeConnector *connector, - drmModeModeInfo *mode) + ply_output_t *output) { - if (mode->hdisplay != head->area.width || mode->vdisplay != head->area.height) { + if (output->mode->hdisplay != head->area.width || output->mode->vdisplay != head->area.height) { ply_trace ("Tried to add connector with resolution %dx%d to %dx%d head", - (int) mode->hdisplay, (int) mode->vdisplay, + (int) output->mode->hdisplay, (int) output->mode->vdisplay, (int) head->area.width, (int) head->area.height); return false; } else { ply_trace ("Adding connector with id %d to %dx%d head", - (int) connector->connector_id, + (int) output->connector->connector_id, (int) head->area.width, (int) head->area.height); } - ply_array_add_uint32_element (head->connector_ids, connector->connector_id); + ply_array_add_uint32_element (head->connector_ids, output->connector->connector_id); return true; } static ply_renderer_head_t * ply_renderer_head_new (ply_renderer_backend_t *backend, - drmModeConnector *connector, - drmModeModeInfo *mode, - uint32_t controller_id, + ply_output_t *output, uint32_t console_buffer_id, - int gamma_size, - ply_pixel_buffer_rotation_t rotation) + int gamma_size) { ply_renderer_head_t *head; int i, step; @@ -507,16 +503,16 @@ ply_renderer_head_new (ply_renderer_backend_t *backend, head->backend = backend; head->connector_ids = ply_array_new (PLY_ARRAY_ELEMENT_TYPE_UINT32); - head->controller_id = controller_id; + head->controller_id = output->controller_id; head->console_buffer_id = console_buffer_id; - head->connector0 = connector; - head->connector0_mode = mode; + head->connector0 = output->connector; + head->connector0_mode = output->mode; head->area.x = 0; head->area.y = 0; - head->area.width = mode->hdisplay; - head->area.height = mode->vdisplay; + head->area.width = output->mode->hdisplay; + head->area.height = output->mode->vdisplay; if (gamma_size) { head->gamma_size = gamma_size; @@ -530,26 +526,26 @@ ply_renderer_head_new (ply_renderer_backend_t *backend, } } - ply_renderer_head_add_connector (head, connector, mode); + ply_renderer_head_add_connector (head, output); assert (ply_array_get_size (head->connector_ids) > 0); - head->pixel_buffer = ply_pixel_buffer_new_with_device_rotation (head->area.width, head->area.height, rotation); + head->pixel_buffer = ply_pixel_buffer_new_with_device_rotation (head->area.width, head->area.height, output->rotation); ply_pixel_buffer_set_device_scale (head->pixel_buffer, ply_get_device_scale (head->area.width, head->area.height, - connector->mmWidth, - connector->mmHeight)); + output->connector->mmWidth, + output->connector->mmHeight)); ply_trace ("Creating %ldx%ld renderer head", head->area.width, head->area.height); ply_pixel_buffer_fill_with_color (head->pixel_buffer, NULL, 0.0, 0.0, 0.0, 1.0); - if (connector->connector_type == DRM_MODE_CONNECTOR_LVDS || - connector->connector_type == DRM_MODE_CONNECTOR_eDP || - connector->connector_type == DRM_MODE_CONNECTOR_DSI) { - backend->panel_width = mode->hdisplay; - backend->panel_height = mode->vdisplay; - backend->panel_rotation = rotation; + if (output->connector->connector_type == DRM_MODE_CONNECTOR_LVDS || + output->connector->connector_type == DRM_MODE_CONNECTOR_eDP || + output->connector->connector_type == DRM_MODE_CONNECTOR_DSI) { + backend->panel_width = output->mode->hdisplay; + backend->panel_height = output->mode->vdisplay; + backend->panel_rotation = output->rotation; backend->panel_scale = ply_pixel_buffer_get_device_scale (head->pixel_buffer); } @@ -1323,9 +1319,9 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) (void *) (intptr_t) controller_id); if (head == NULL) { - head = ply_renderer_head_new (backend, connector, outputs[i].mode, - controller_id, console_buffer_id, - gamma_size, outputs[i].rotation); + head = ply_renderer_head_new (backend, &outputs[i], + console_buffer_id, + gamma_size); ply_list_append_data (backend->heads, head); @@ -1333,7 +1329,7 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) (void *) (intptr_t) controller_id, head); } else { - if (!ply_renderer_head_add_connector (head, connector, outputs[i].mode)) + if (!ply_renderer_head_add_connector (head, &outputs[i])) ply_trace ("couldn't connect monitor to existing head"); drmModeFreeConnector (connector); From 9b5d4df9f92909aa4c53d98f02c9343eb4ce97c7 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 15 Jan 2019 07:31:32 +0100 Subject: [PATCH 04/16] drm: Stop keeing a drmModeConnector instance around Before this commit we were storing a pointer to the drmModeConnector in struct _ply_renderer_head, solely so that we can free it when destroying the head. This was necessary because we also stored a pointer to the mode we picked, which comes from insided the drmModeConnector. The drmModeModeInfo struct has no pointers, so we can simply store a copy of it instead of a pointer, which removes the need to keep the drmModeConnector around after probing the connectors. Signed-off-by: Hans de Goede --- src/plugins/renderers/drm/plugin.c | 41 ++++++++++++++---------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c index afc8d65e..348f880c 100644 --- a/src/plugins/renderers/drm/plugin.c +++ b/src/plugins/renderers/drm/plugin.c @@ -77,8 +77,7 @@ struct _ply_renderer_head unsigned long row_stride; ply_array_t *connector_ids; - drmModeConnector *connector0; - drmModeModeInfo *connector0_mode; + drmModeModeInfo connector0_mode; uint32_t controller_id; uint32_t console_buffer_id; @@ -118,7 +117,7 @@ typedef struct typedef struct { drmModeConnector *connector; - drmModeModeInfo *mode; + drmModeModeInfo mode; uint32_t controller_id; uint32_t possible_controllers; ply_pixel_buffer_rotation_t rotation; @@ -474,9 +473,9 @@ static bool ply_renderer_head_add_connector (ply_renderer_head_t *head, ply_output_t *output) { - if (output->mode->hdisplay != head->area.width || output->mode->vdisplay != head->area.height) { + if (output->mode.hdisplay != head->area.width || output->mode.vdisplay != head->area.height) { ply_trace ("Tried to add connector with resolution %dx%d to %dx%d head", - (int) output->mode->hdisplay, (int) output->mode->vdisplay, + (int) output->mode.hdisplay, (int) output->mode.vdisplay, (int) head->area.width, (int) head->area.height); return false; } else { @@ -505,14 +504,12 @@ ply_renderer_head_new (ply_renderer_backend_t *backend, head->connector_ids = ply_array_new (PLY_ARRAY_ELEMENT_TYPE_UINT32); head->controller_id = output->controller_id; head->console_buffer_id = console_buffer_id; - - head->connector0 = output->connector; head->connector0_mode = output->mode; head->area.x = 0; head->area.y = 0; - head->area.width = output->mode->hdisplay; - head->area.height = output->mode->vdisplay; + head->area.width = output->mode.hdisplay; + head->area.height = output->mode.vdisplay; if (gamma_size) { head->gamma_size = gamma_size; @@ -543,8 +540,8 @@ ply_renderer_head_new (ply_renderer_backend_t *backend, if (output->connector->connector_type == DRM_MODE_CONNECTOR_LVDS || output->connector->connector_type == DRM_MODE_CONNECTOR_eDP || output->connector->connector_type == DRM_MODE_CONNECTOR_DSI) { - backend->panel_width = output->mode->hdisplay; - backend->panel_height = output->mode->vdisplay; + backend->panel_width = output->mode.hdisplay; + backend->panel_height = output->mode.vdisplay; backend->panel_rotation = output->rotation; backend->panel_scale = ply_pixel_buffer_get_device_scale (head->pixel_buffer); } @@ -558,7 +555,6 @@ ply_renderer_head_free (ply_renderer_head_t *head) ply_trace ("freeing %ldx%ld renderer head", head->area.width, head->area.height); ply_pixel_buffer_free (head->pixel_buffer); - drmModeFreeConnector (head->connector0); ply_array_free (head->connector_ids); free (head->gamma); free (head); @@ -648,7 +644,7 @@ ply_renderer_head_set_scan_out_buffer (ply_renderer_backend_t *backend, ply_renderer_head_t *head, uint32_t buffer_id) { - drmModeModeInfo *mode = head->connector0_mode; + drmModeModeInfo *mode = &head->connector0_mode; uint32_t *connector_ids; int number_of_connectors; @@ -1211,6 +1207,7 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) */ found = 0; for (i = 0; i < backend->resources->count_connectors; i++) { + drmModeModeInfo *mode = NULL; drmModeConnector *connector; connector = drmModeGetConnector (backend->device_fd, @@ -1234,17 +1231,18 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) ply_renderer_connector_get_rotation_and_tiled (backend, &outputs[found]); if (!outputs[found].tiled && backend->use_preferred_mode) - outputs[found].mode = get_preferred_mode (connector); + mode = get_preferred_mode (connector); - if (!outputs[found].mode && outputs[found].controller_id) - outputs[found].mode = get_active_mode (backend, &outputs[found]); + if (!mode && outputs[found].controller_id) + mode = get_active_mode (backend, &outputs[found]); /* If we couldn't find the current active mode, fall back to the first available. */ - if (!outputs[found].mode) { + if (!mode) { ply_trace ("falling back to first available mode"); - outputs[found].mode = &connector->modes[0]; + mode = &connector->modes[0]; } + outputs[found].mode = *mode; found++; } @@ -1262,8 +1260,8 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) continue; if (outputs[i].controller_id == outputs[j].controller_id && - (outputs[i].mode->hdisplay != outputs[j].mode->hdisplay || - outputs[i].mode->vdisplay != outputs[j].mode->vdisplay)) { + (outputs[i].mode.hdisplay != outputs[j].mode.hdisplay || + outputs[i].mode.vdisplay != outputs[j].mode.vdisplay)) { ply_trace ("connector %u uses same controller as %u and modes differ, unlinking controller", outputs[j].connector->connector_id, outputs[i].connector->connector_id); @@ -1331,9 +1329,8 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) } else { if (!ply_renderer_head_add_connector (head, &outputs[i])) ply_trace ("couldn't connect monitor to existing head"); - - drmModeFreeConnector (connector); } + drmModeFreeConnector (connector); } ply_hashtable_free (heads_by_controller_id); From b3f1c7683d70b653ebde1d41342f6b549d5c2f1f Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 15 Jan 2019 08:31:20 +0100 Subject: [PATCH 05/16] drm: Stop storing a pointer to drmModeConnector in ply_output_t This is a preparation patch for hotplug support, for hotplug support we want to keep the ply_output_t for connectors around, this change decouples the lifetime of the drmModeConnector from the ply_output_t lifetime. Signed-off-by: Hans de Goede --- src/plugins/renderers/drm/plugin.c | 66 +++++++++++++++--------------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c index 348f880c..07de083f 100644 --- a/src/plugins/renderers/drm/plugin.c +++ b/src/plugins/renderers/drm/plugin.c @@ -116,10 +116,12 @@ typedef struct typedef struct { - drmModeConnector *connector; drmModeModeInfo mode; + uint32_t connector_id; + uint32_t connector_type; uint32_t controller_id; uint32_t possible_controllers; + int device_scale; ply_pixel_buffer_rotation_t rotation; bool tiled; } ply_output_t; @@ -444,6 +446,7 @@ connector_orientation_prop_to_rotation (drmModePropertyPtr prop, static void ply_renderer_connector_get_rotation_and_tiled (ply_renderer_backend_t *backend, + drmModeConnector *connector, ply_output_t *output) { drmModePropertyPtr prop; @@ -452,14 +455,14 @@ ply_renderer_connector_get_rotation_and_tiled (ply_renderer_backend_t *back output->rotation = PLY_PIXEL_BUFFER_ROTATE_UPRIGHT; output->tiled = false; - for (i = 0; i < output->connector->count_props; i++) { - prop = drmModeGetProperty (backend->device_fd, output->connector->props[i]); + for (i = 0; i < connector->count_props; i++) { + prop = drmModeGetProperty (backend->device_fd, connector->props[i]); if (!prop) continue; if ((prop->flags & DRM_MODE_PROP_ENUM) && strcmp (prop->name, "panel orientation") == 0) - output->rotation = connector_orientation_prop_to_rotation (prop, output->connector->prop_values[i]); + output->rotation = connector_orientation_prop_to_rotation (prop, connector->prop_values[i]); if ((prop->flags & DRM_MODE_PROP_BLOB) && strcmp (prop->name, "TILE") == 0) @@ -480,11 +483,11 @@ ply_renderer_head_add_connector (ply_renderer_head_t *head, return false; } else { ply_trace ("Adding connector with id %d to %dx%d head", - (int) output->connector->connector_id, + (int) output->connector_id, (int) head->area.width, (int) head->area.height); } - ply_array_add_uint32_element (head->connector_ids, output->connector->connector_id); + ply_array_add_uint32_element (head->connector_ids, output->connector_id); return true; } @@ -527,23 +530,19 @@ ply_renderer_head_new (ply_renderer_backend_t *backend, assert (ply_array_get_size (head->connector_ids) > 0); head->pixel_buffer = ply_pixel_buffer_new_with_device_rotation (head->area.width, head->area.height, output->rotation); - ply_pixel_buffer_set_device_scale (head->pixel_buffer, - ply_get_device_scale (head->area.width, - head->area.height, - output->connector->mmWidth, - output->connector->mmHeight)); + ply_pixel_buffer_set_device_scale (head->pixel_buffer, output->device_scale); ply_trace ("Creating %ldx%ld renderer head", head->area.width, head->area.height); ply_pixel_buffer_fill_with_color (head->pixel_buffer, NULL, 0.0, 0.0, 0.0, 1.0); - if (output->connector->connector_type == DRM_MODE_CONNECTOR_LVDS || - output->connector->connector_type == DRM_MODE_CONNECTOR_eDP || - output->connector->connector_type == DRM_MODE_CONNECTOR_DSI) { + if (output->connector_type == DRM_MODE_CONNECTOR_LVDS || + output->connector_type == DRM_MODE_CONNECTOR_eDP || + output->connector_type == DRM_MODE_CONNECTOR_DSI) { backend->panel_width = output->mode.hdisplay; backend->panel_height = output->mode.vdisplay; backend->panel_rotation = output->rotation; - backend->panel_scale = ply_pixel_buffer_get_device_scale (head->pixel_buffer); + backend->panel_scale = output->device_scale; } return head; @@ -973,6 +972,7 @@ close_device (ply_renderer_backend_t *backend) static void output_get_controller_info (ply_renderer_backend_t *backend, + drmModeConnector *connector, ply_output_t *output) { int i; @@ -982,16 +982,16 @@ output_get_controller_info (ply_renderer_backend_t *backend, output->possible_controllers = 0xffffffff; - for (i = 0; i < output->connector->count_encoders; i++) { + for (i = 0; i < connector->count_encoders; i++) { encoder = drmModeGetEncoder (backend->device_fd, - output->connector->encoders[i]); + connector->encoders[i]); if (encoder == NULL) continue; - if (encoder->encoder_id == output->connector->encoder_id && encoder->crtc_id) { + if (encoder->encoder_id == connector->encoder_id && encoder->crtc_id) { ply_trace ("Found already lit monitor on connector %u using controller %u", - output->connector->connector_id, encoder->crtc_id); + connector->connector_id, encoder->crtc_id); output->controller_id = encoder->crtc_id; } @@ -1000,7 +1000,7 @@ output_get_controller_info (ply_renderer_backend_t *backend, */ output->possible_controllers &= encoder->possible_crtcs; ply_trace ("connector %u encoder %u possible controllers 0x%08x/0x%08x", - output->connector->connector_id, encoder->encoder_id, + connector->connector_id, encoder->encoder_id, encoder->possible_crtcs, output->possible_controllers); drmModeFreeEncoder (encoder); } @@ -1063,6 +1063,7 @@ get_preferred_mode (drmModeConnector *connector) static drmModeModeInfo * get_active_mode (ply_renderer_backend_t *backend, + drmModeConnector *connector, ply_output_t *output) { drmModeCrtc *controller; @@ -1077,7 +1078,7 @@ get_active_mode (ply_renderer_backend_t *backend, ply_trace ("Looking for connector mode index of active mode %dx%d", controller->mode.hdisplay, controller->mode.vdisplay); - mode = find_matching_connector_mode (backend, output->connector, &controller->mode); + mode = find_matching_connector_mode (backend, connector, &controller->mode); drmModeFreeCrtc (controller); @@ -1225,16 +1226,14 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) continue; } - outputs[found].connector = connector; - - output_get_controller_info (backend, &outputs[found]); - ply_renderer_connector_get_rotation_and_tiled (backend, &outputs[found]); + output_get_controller_info (backend, connector, &outputs[found]); + ply_renderer_connector_get_rotation_and_tiled (backend, connector, &outputs[found]); if (!outputs[found].tiled && backend->use_preferred_mode) mode = get_preferred_mode (connector); if (!mode && outputs[found].controller_id) - mode = get_active_mode (backend, &outputs[found]); + mode = get_active_mode (backend, connector, &outputs[found]); /* If we couldn't find the current active mode, fall back to the first available. */ @@ -1243,6 +1242,10 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) mode = &connector->modes[0]; } outputs[found].mode = *mode; + outputs[found].device_scale = ply_get_device_scale (mode->hdisplay, mode->vdisplay, + connector->mmWidth, connector->mmHeight); + outputs[found].connector_type = connector->connector_type; + drmModeFreeConnector (connector); found++; } @@ -1263,8 +1266,7 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) (outputs[i].mode.hdisplay != outputs[j].mode.hdisplay || outputs[i].mode.vdisplay != outputs[j].mode.vdisplay)) { ply_trace ("connector %u uses same controller as %u and modes differ, unlinking controller", - outputs[j].connector->connector_id, - outputs[i].connector->connector_id); + outputs[j].connector_id, outputs[i].connector_id); outputs[j].controller_id = 0; } } @@ -1289,13 +1291,12 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) } for (i = 0; i < outputs_len; i++) ply_trace ("Using controller %u for connector %u", - outputs[i].controller_id, outputs[i].connector->connector_id); + outputs[i].controller_id, outputs[i].connector_id); /* Step 4: * Create heads for all valid outputs */ for (i = 0; i < outputs_len; i++) { - drmModeConnector *connector = outputs[i].connector; drmModeCrtc *controller; ply_renderer_head_t *head; uint32_t controller_id; @@ -1303,10 +1304,8 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) int gamma_size; controller = drmModeGetCrtc (backend->device_fd, outputs[i].controller_id); - if (!controller) { - drmModeFreeConnector (connector); + if (!controller) continue; - } controller_id = controller->crtc_id; console_buffer_id = controller->buffer_id; @@ -1330,7 +1329,6 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) if (!ply_renderer_head_add_connector (head, &outputs[i])) ply_trace ("couldn't connect monitor to existing head"); } - drmModeFreeConnector (connector); } ply_hashtable_free (heads_by_controller_id); From 4ba6d4f4a238b506dc65e4e6516384a504a414af Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 15 Jan 2019 08:34:58 +0100 Subject: [PATCH 06/16] drm: Add get_output_info helper function Add a new get_output_info helper function, which fill a ply_output_t with all info related to the connecter based on a connector-id. Signed-off-by: Hans de Goede --- src/plugins/renderers/drm/plugin.c | 86 ++++++++++++++++-------------- 1 file changed, 46 insertions(+), 40 deletions(-) diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c index 07de083f..da56fe48 100644 --- a/src/plugins/renderers/drm/plugin.c +++ b/src/plugins/renderers/drm/plugin.c @@ -124,6 +124,7 @@ typedef struct int device_scale; ply_pixel_buffer_rotation_t rotation; bool tiled; + bool connected; } ply_output_t; struct _ply_renderer_backend @@ -1085,6 +1086,48 @@ get_active_mode (ply_renderer_backend_t *backend, return mode; } +static void +get_output_info (ply_renderer_backend_t *backend, + uint32_t connector_id, + ply_output_t *output) +{ + drmModeModeInfo *mode = NULL; + drmModeConnector *connector; + + memset (output, 0, sizeof(*output)); + output->connector_id = connector_id; + + connector = drmModeGetConnector (backend->device_fd, connector_id); + if (connector == NULL) + return; + + if (connector->connection != DRM_MODE_CONNECTED || + connector->count_modes <= 0) + goto out; + + output_get_controller_info (backend, connector, output); + ply_renderer_connector_get_rotation_and_tiled (backend, connector, output); + + if (!output->tiled && backend->use_preferred_mode) + mode = get_preferred_mode (connector); + + if (!mode && output->controller_id) + mode = get_active_mode (backend, connector, output); + + /* If we couldn't find the current active mode, fall back to the first available. */ + if (!mode) { + ply_trace ("falling back to first available mode"); + mode = &connector->modes[0]; + } + output->mode = *mode; + output->device_scale = ply_get_device_scale (mode->hdisplay, mode->vdisplay, + connector->mmWidth, connector->mmHeight); + output->connector_type = connector->connector_type; + output->connected = true; +out: + drmModeFreeConnector (connector); +} + /* Some controllers can only drive some outputs, we want to find a combination * where all (connected) outputs get a controller. To do this setup_outputs * picks which output to assign a controller for first (trying all outputs), so @@ -1208,46 +1251,9 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) */ found = 0; for (i = 0; i < backend->resources->count_connectors; i++) { - drmModeModeInfo *mode = NULL; - drmModeConnector *connector; - - connector = drmModeGetConnector (backend->device_fd, - backend->resources->connectors[i]); - if (connector == NULL) - continue; - - if (connector->connection != DRM_MODE_CONNECTED) { - drmModeFreeConnector (connector); - continue; - } - - if (connector->count_modes <= 0) { - drmModeFreeConnector (connector); - continue; - } - - output_get_controller_info (backend, connector, &outputs[found]); - ply_renderer_connector_get_rotation_and_tiled (backend, connector, &outputs[found]); - - if (!outputs[found].tiled && backend->use_preferred_mode) - mode = get_preferred_mode (connector); - - if (!mode && outputs[found].controller_id) - mode = get_active_mode (backend, connector, &outputs[found]); - - /* If we couldn't find the current active mode, fall back to the first available. - */ - if (!mode) { - ply_trace ("falling back to first available mode"); - mode = &connector->modes[0]; - } - outputs[found].mode = *mode; - outputs[found].device_scale = ply_get_device_scale (mode->hdisplay, mode->vdisplay, - connector->mmWidth, connector->mmHeight); - outputs[found].connector_type = connector->connector_type; - drmModeFreeConnector (connector); - - found++; + get_output_info (backend, backend->resources->connectors[i], &outputs[found]); + if (outputs[found].connected) + found++; } outputs_len = found; /* outputs now contains found valid entries */ From d68fd78ae17c2574976b78b5f31afd58b965abe5 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 15 Jan 2019 08:58:45 +0100 Subject: [PATCH 07/16] drm: Store and keep all the outputs in the backend Put all outputs in the outputs array instead of just the connected ones and store the outputs array and the controller_id-to-head hashtable in the backend object instead of temporarily allocating them during enumeration. Also add new heads to the heads list and to the controller_id-to-head hashtable in ply_renderer_head_new where this really belongs. This allows nicely balancing these 2 with removing the head from the list and hash_table in the ply_renderer_head_remove function which is added in a follow-up commit. This is a preparation patch for adding support for hotplugging monitors while plymouth is running. Signed-off-by: Hans de Goede --- src/plugins/renderers/drm/plugin.c | 55 ++++++++++++++++-------------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c index da56fe48..a66cd675 100644 --- a/src/plugins/renderers/drm/plugin.c +++ b/src/plugins/renderers/drm/plugin.c @@ -138,10 +138,14 @@ struct _ply_renderer_backend ply_renderer_input_source_t input_source; ply_list_t *heads; - ply_hashtable_t *heads_by_connector_id; + ply_hashtable_t *heads_by_controller_id; ply_hashtable_t *output_buffers; + ply_output_t *outputs; + int outputs_len; + int connected_count; + int32_t dither_red; int32_t dither_green; int32_t dither_blue; @@ -546,6 +550,10 @@ ply_renderer_head_new (ply_renderer_backend_t *backend, backend->panel_scale = output->device_scale; } + ply_list_append_data (backend->heads, head); + ply_hashtable_insert (backend->heads_by_controller_id, + (void *) (intptr_t) output->controller_id, + head); return head; } @@ -808,6 +816,7 @@ create_backend (const char *device_name, backend->requires_explicit_flushing = true; backend->output_buffers = ply_hashtable_new (ply_hashtable_direct_hash, ply_hashtable_direct_compare); + backend->heads_by_controller_id = ply_hashtable_new (NULL, NULL); backend->use_preferred_mode = should_use_preferred_mode (); return backend; @@ -827,9 +836,11 @@ destroy_backend (ply_renderer_backend_t *backend) free (backend->device_name); ply_hashtable_free (backend->output_buffers); + ply_hashtable_free (backend->heads_by_controller_id); drmModeFreeResources (backend->resources); + free (backend->outputs); free (backend); } @@ -1194,9 +1205,9 @@ setup_outputs (ply_renderer_backend_t *backend, best_count = count_setup_controllers (outputs, outputs_len); best_outputs = outputs; - for (i = 0; i < outputs_len && best_count < outputs_len; i++) { - /* Already assigned? */ - if (outputs[i].controller_id) + for (i = 0; i < outputs_len && best_count < backend->connected_count; i++) { + /* Not connected or already assigned? */ + if (!outputs[i].connected || outputs[i].controller_id) continue; /* Assign controller for connector i */ @@ -1238,24 +1249,21 @@ setup_outputs (ply_renderer_backend_t *backend, static bool create_heads_for_active_connectors (ply_renderer_backend_t *backend) { - ply_hashtable_t *heads_by_controller_id; + int i, j, number_of_setup_outputs, outputs_len; ply_output_t *outputs; - int i, j, found, number_of_setup_outputs, outputs_len; - - heads_by_controller_id = ply_hashtable_new (NULL, NULL); outputs = calloc (backend->resources->count_connectors, sizeof(*outputs)); + outputs_len = backend->resources->count_connectors; /* Step 1: * Build a list of connected outputs and get pre-configured controllers. */ - found = 0; - for (i = 0; i < backend->resources->count_connectors; i++) { - get_output_info (backend, backend->resources->connectors[i], &outputs[found]); - if (outputs[found].connected) - found++; + backend->connected_count = 0; + for (i = 0; i < outputs_len; i++) { + get_output_info (backend, backend->resources->connectors[i], &outputs[i]); + if (outputs[i].connected) + backend->connected_count++; } - outputs_len = found; /* outputs now contains found valid entries */ /* Step 2: * Drop controllers for clones for which we've picked different modes. @@ -1282,13 +1290,13 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) * Assign controllers to outputs without a controller */ number_of_setup_outputs = count_setup_controllers (outputs, outputs_len); - if (number_of_setup_outputs != outputs_len) { + if (number_of_setup_outputs != backend->connected_count) { /* First try, try to assign controllers to outputs without one */ ply_trace ("Some outputs don't have controllers, picking controllers"); outputs = setup_outputs (backend, outputs, outputs_len); number_of_setup_outputs = count_setup_controllers (outputs, outputs_len); } - if (number_of_setup_outputs != outputs_len) { + if (number_of_setup_outputs != backend->connected_count) { /* Second try, re-assing controller for all outputs */ ply_trace ("Some outputs still don't have controllers, re-assigning controllers for all outputs"); for (i = 0; i < outputs_len; i++) @@ -1309,6 +1317,9 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) uint32_t console_buffer_id; int gamma_size; + if (!outputs[i].controller_id) + continue; + controller = drmModeGetCrtc (backend->device_fd, outputs[i].controller_id); if (!controller) continue; @@ -1318,27 +1329,21 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) gamma_size = controller->gamma_size; drmModeFreeCrtc (controller); - head = ply_hashtable_lookup (heads_by_controller_id, + head = ply_hashtable_lookup (backend->heads_by_controller_id, (void *) (intptr_t) controller_id); if (head == NULL) { head = ply_renderer_head_new (backend, &outputs[i], console_buffer_id, gamma_size); - - ply_list_append_data (backend->heads, head); - - ply_hashtable_insert (heads_by_controller_id, - (void *) (intptr_t) controller_id, - head); } else { if (!ply_renderer_head_add_connector (head, &outputs[i])) ply_trace ("couldn't connect monitor to existing head"); } } - ply_hashtable_free (heads_by_controller_id); - free (outputs); + backend->outputs_len = outputs_len; + backend->outputs = outputs; return ply_list_get_length (backend->heads) > 0; } From 7cb2eb14892a3468a2e249709e3f09b7b852d36e Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 15 Jan 2019 09:21:05 +0100 Subject: [PATCH 08/16] drm: Limit backend->resources lifetime to within query_device We do not need / use backend->resources anywhere outside of the query_device function and with the upcoming hotplug support we need to get a fresh set of resources on change events, so limit the resources lifetime to query_device. Signed-off-by: Hans de Goede --- src/plugins/renderers/drm/plugin.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c index a66cd675..4e3c41a1 100644 --- a/src/plugins/renderers/drm/plugin.c +++ b/src/plugins/renderers/drm/plugin.c @@ -838,8 +838,6 @@ destroy_backend (ply_renderer_backend_t *backend) ply_hashtable_free (backend->output_buffers); ply_hashtable_free (backend->heads_by_controller_id); - drmModeFreeResources (backend->resources); - free (backend->outputs); free (backend); } @@ -1385,6 +1383,8 @@ has_32bpp_support (ply_renderer_backend_t *backend) static bool query_device (ply_renderer_backend_t *backend) { + bool ret = true; + assert (backend != NULL); assert (backend->device_fd >= 0); @@ -1397,15 +1397,16 @@ query_device (ply_renderer_backend_t *backend) if (!create_heads_for_active_connectors (backend)) { ply_trace ("Could not initialize heads"); - return false; - } - - if (!has_32bpp_support (backend)) { + ret = false; + } else if (!has_32bpp_support (backend)) { ply_trace ("Device doesn't support 32bpp framebuffer"); - return false; + ret = false; } - return true; + drmModeFreeResources (backend->resources); + backend->resources = NULL; + + return ret; } static bool From cd0479675e0c853896a860913917300818b8f69d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 15 Jan 2019 09:54:09 +0100 Subject: [PATCH 09/16] drm: Allow calling ply_renderer_head_add_connector with existing connector_id Allow calling ply_renderer_head_add_connector with an existing connector_id and ignore this call. This allows calling create_heads_for_active_connectors multiple times, only creating/adding heads for new connectors. This is a preparation patch for adding support for hotplugging monitors while plymouth is running. Signed-off-by: Hans de Goede --- src/plugins/renderers/drm/plugin.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c index 4e3c41a1..99984590 100644 --- a/src/plugins/renderers/drm/plugin.c +++ b/src/plugins/renderers/drm/plugin.c @@ -486,12 +486,15 @@ ply_renderer_head_add_connector (ply_renderer_head_t *head, (int) output->mode.hdisplay, (int) output->mode.vdisplay, (int) head->area.width, (int) head->area.height); return false; - } else { - ply_trace ("Adding connector with id %d to %dx%d head", - (int) output->connector_id, - (int) head->area.width, (int) head->area.height); } + if (ply_array_contains_uint32_element (head->connector_ids, output->connector_id)) { + ply_trace ("Head already contains connector with id %d", output->connector_id); + return false; + } + + ply_trace ("Adding connector with id %d to %dx%d head", + (int) output->connector_id, (int) head->area.width, (int) head->area.height); ply_array_add_uint32_element (head->connector_ids, output->connector_id); return true; From 7774bd5e5132ace8060f9edd91395eaa42535246 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 15 Jan 2019 10:43:45 +0100 Subject: [PATCH 10/16] drm: Allow calling create_heads_for_active_connectors multiple times To support hotplugging monitors while plymouth is running, we must rebuild our outputs list and create and/or remove heads as necessary on every change event. This commit adds support for removing single outputs (rather then tearing down the whole backend) and adds a new first step to create_heads_for_active_connectors which goes over our view of the outputs before the change and removes any changed outputs from the heads they belong to, destroying the head if the last output/connector is removed. On the first call backend->output_len is 0, so this new first step is a no-op. On subsequent calls we can simply build the list as we do on the first call, changed outputs will already be removed by the new first step and for unchanged outputs we end up in ply_renderer_head_add_connector which will ignore the already added connector. Note this drops the "couldn't connect monitor to existing head" message, this is confusing when create_heads_for_active_connectors is called more then once and is unnecessary as ply_renderer_head_add_connector already logs a message on both failure exit paths. Signed-off-by: Hans de Goede --- src/plugins/renderers/drm/plugin.c | 111 ++++++++++++++++++++++++++--- 1 file changed, 101 insertions(+), 10 deletions(-) diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c index 99984590..bb6ab77d 100644 --- a/src/plugins/renderers/drm/plugin.c +++ b/src/plugins/renderers/drm/plugin.c @@ -734,6 +734,47 @@ ply_renderer_head_unmap (ply_renderer_backend_t *backend, head->scan_out_buffer_id = 0; } +static void +ply_renderer_head_remove (ply_renderer_backend_t *backend, + ply_renderer_head_t *head) +{ + if (head->scan_out_buffer_id) + ply_renderer_head_unmap (backend, head); + + ply_hashtable_remove (backend->heads_by_controller_id, + (void *) (intptr_t) head->controller_id); + ply_list_remove_data (backend->heads, head); + ply_renderer_head_free (head); +} + +static void +ply_renderer_head_remove_connector (ply_renderer_backend_t *backend, + ply_renderer_head_t *head, + uint32_t connector_id) +{ + int i, size = ply_array_get_size (head->connector_ids); + uint32_t *connector_ids; + + if (!ply_array_contains_uint32_element (head->connector_ids, connector_id)) { + ply_trace ("Head does not contain connector %u, cannot remove", connector_id); + return; + } + + if (size == 1) { + ply_renderer_head_remove (backend, head); + return; + } + + /* Empty the array and re-add all connectors except the one being removed */ + connector_ids = ply_array_steal_uint32_elements (head->connector_ids); + for (i = 0; i < size; i++) { + if (connector_ids[i] != connector_id) + ply_array_add_uint32_element (head->connector_ids, + connector_ids[i]); + } + free (connector_ids); +} + static void flush_area (const char *src, unsigned long src_row_stride, @@ -1247,18 +1288,65 @@ setup_outputs (ply_renderer_backend_t *backend, return (ply_output_t *)best_outputs; } +static void +remove_output (ply_renderer_backend_t *backend, ply_output_t *output) +{ + ply_renderer_head_t *head; + + head = ply_hashtable_lookup (backend->heads_by_controller_id, + (void *) (intptr_t) output->controller_id); + if (head == NULL) { + ply_trace ("Could not find head for connector %u, controller %u, cannot remove", + output->connector_id, output->controller_id); + return; + } + + ply_renderer_head_remove_connector (backend, head, output->connector_id); +} + +/* Update our outputs array to match the hardware state and + * create and/or remove heads as necessary. + * Returns true if any heads were modified. + */ static bool create_heads_for_active_connectors (ply_renderer_backend_t *backend) { int i, j, number_of_setup_outputs, outputs_len; - ply_output_t *outputs; + ply_output_t output, *outputs; + bool changed = false; + + /* Step 1: + * Remove existing outputs from heads if they have changed. + */ + ply_trace ("Checking currently connected outputs for changes"); + for (i = 0; i < backend->outputs_len; i++) { + if (!backend->outputs[i].controller_id) + continue; + + get_output_info (backend, backend->outputs[i].connector_id, &output); + + if (memcmp(&backend->outputs[i], &output, sizeof(ply_output_t))) { + ply_trace ("Output for connector %u changed, removing", + backend->outputs[i].connector_id); + remove_output (backend, &backend->outputs[i]); + changed = true; + } + } + + /* Step 2: + * Now that we've removed changed connectors from the heads, we can + * simply rebuild the outputs array from scratch. For any unchanged + * outputs for which we already have a head, we will end up in + * ply_renderer_head_add_connector which will ignore the already + * added connector. + */ + ply_trace ("(Re)enumerating all outputs"); + free (backend->outputs); + backend->outputs = NULL; outputs = calloc (backend->resources->count_connectors, sizeof(*outputs)); outputs_len = backend->resources->count_connectors; - /* Step 1: - * Build a list of connected outputs and get pre-configured controllers. - */ backend->connected_count = 0; for (i = 0; i < outputs_len; i++) { get_output_info (backend, backend->resources->connectors[i], &outputs[i]); @@ -1266,7 +1354,7 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) backend->connected_count++; } - /* Step 2: + /* Step 3: * Drop controllers for clones for which we've picked different modes. */ for (i = 0; i < outputs_len; i++) { @@ -1287,7 +1375,7 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) } } - /* Step 3: + /* Step 4: * Assign controllers to outputs without a controller */ number_of_setup_outputs = count_setup_controllers (outputs, outputs_len); @@ -1308,7 +1396,7 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) ply_trace ("Using controller %u for connector %u", outputs[i].controller_id, outputs[i].connector_id); - /* Step 4: + /* Step 5: * Create heads for all valid outputs */ for (i = 0; i < outputs_len; i++) { @@ -1337,16 +1425,19 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) head = ply_renderer_head_new (backend, &outputs[i], console_buffer_id, gamma_size); + changed = true; } else { - if (!ply_renderer_head_add_connector (head, &outputs[i])) - ply_trace ("couldn't connect monitor to existing head"); + if (ply_renderer_head_add_connector (head, &outputs[i])) + changed = true; } } backend->outputs_len = outputs_len; backend->outputs = outputs; - return ply_list_get_length (backend->heads) > 0; + ply_trace ("outputs %schanged\n", changed ? "" : "un"); + + return changed; } static bool From 475a51d0576d0a488023f870c92f39eb8b0a21c1 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 16 Jan 2019 14:50:34 +0100 Subject: [PATCH 11/16] drm: Ensure heads are mapped before flushing them The drm plugin's map_to_device function will return true if mapping of any of the heads has succeeded, potentially leaving some heads unmapped. This causes the "assert (buffer != NULL)" in begin_flush to trigger when flushing the heads as head->scan_out_buffer_id is 0. It seems that even though this is a pre-existing problem we sofar have not hit this, likely because ply_renderer_head_map in pratice never fails. However with the new monitor hotplug support, a head may be added after map_to_device is called, triggering the assert. This commit fixes both the theoretical pre-existing problem and the actual problem triggered by hotplug support by ensuring that the head is mapped before flushing it. Signed-off-by: Hans de Goede --- src/plugins/renderers/drm/plugin.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c index bb6ab77d..0963a84a 100644 --- a/src/plugins/renderers/drm/plugin.c +++ b/src/plugins/renderers/drm/plugin.c @@ -715,11 +715,6 @@ ply_renderer_head_map (ply_renderer_backend_t *backend, return false; } - /* FIXME: Maybe we should blit the fbcon contents instead of the (blank) - * shadow buffer? - */ - ply_renderer_head_redraw (backend, head); - return true; } @@ -1518,8 +1513,13 @@ map_to_device (ply_renderer_backend_t *backend) head = (ply_renderer_head_t *) ply_list_node_get_data (node); next_node = ply_list_get_next_node (backend->heads, node); - if (ply_renderer_head_map (backend, head)) + if (ply_renderer_head_map (backend, head)) { + /* FIXME: Maybe we should blit the fbcon contents instead of the (blank) + * shadow buffer? + */ + ply_renderer_head_redraw (backend, head); head_mapped = true; + } node = next_node; } @@ -1605,6 +1605,12 @@ flush_head (ply_renderer_backend_t *backend, updated_region = ply_pixel_buffer_get_updated_areas (pixel_buffer); areas_to_flush = ply_region_get_sorted_rectangle_list (updated_region); + /* A hotplugged head may not be mapped yet, map it now. */ + if (!head->scan_out_buffer_id) { + if (!ply_renderer_head_map (backend, head)) + return; + } + map_address = begin_flush (backend, head->scan_out_buffer_id); node = ply_list_get_first_node (areas_to_flush); From 15ebdd6d5b0e98d7511815e2dfbe82d2ec86f493 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 16 Jan 2019 09:41:42 +0100 Subject: [PATCH 12/16] drm: Implement handle_change_event Now that we can call create_heads_for_active_connectors multiple times we can implement handle_change_event. Signed-off-by: Hans de Goede --- src/plugins/renderers/drm/plugin.c | 32 ++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c index 0963a84a..b190737e 100644 --- a/src/plugins/renderers/drm/plugin.c +++ b/src/plugins/renderers/drm/plugin.c @@ -1304,7 +1304,7 @@ remove_output (ply_renderer_backend_t *backend, ply_output_t *output) * Returns true if any heads were modified. */ static bool -create_heads_for_active_connectors (ply_renderer_backend_t *backend) +create_heads_for_active_connectors (ply_renderer_backend_t *backend, bool change) { int i, j, number_of_setup_outputs, outputs_len; ply_output_t output, *outputs; @@ -1380,8 +1380,12 @@ create_heads_for_active_connectors (ply_renderer_backend_t *backend) outputs = setup_outputs (backend, outputs, outputs_len); number_of_setup_outputs = count_setup_controllers (outputs, outputs_len); } - if (number_of_setup_outputs != backend->connected_count) { - /* Second try, re-assing controller for all outputs */ + /* Try again if necessary, re-assing controllers for all outputs. + * Note this is skipped when processing change events, as we don't + * want to mess with the controller assignment of already lit monitors + * in that case. + */ + if (!change && number_of_setup_outputs != backend->connected_count) { ply_trace ("Some outputs still don't have controllers, re-assigning controllers for all outputs"); for (i = 0; i < outputs_len; i++) outputs[i].controller_id = 0; @@ -1484,7 +1488,7 @@ query_device (ply_renderer_backend_t *backend) return false; } - if (!create_heads_for_active_connectors (backend)) { + if (!create_heads_for_active_connectors (backend, false)) { ply_trace ("Could not initialize heads"); ret = false; } else if (!has_32bpp_support (backend)) { @@ -1498,6 +1502,25 @@ query_device (ply_renderer_backend_t *backend) return ret; } +static bool +handle_change_event (ply_renderer_backend_t *backend) +{ + bool ret = true; + + backend->resources = drmModeGetResources (backend->device_fd); + if (backend->resources == NULL) { + ply_trace ("Could not get card resources for change event"); + return false; + } + + ret = create_heads_for_active_connectors (backend, true); + + drmModeFreeResources (backend->resources); + backend->resources = NULL; + + return ret; +} + static bool map_to_device (ply_renderer_backend_t *backend) { @@ -1776,6 +1799,7 @@ ply_renderer_backend_get_interface (void) .open_device = open_device, .close_device = close_device, .query_device = query_device, + .handle_change_event = handle_change_event, .map_to_device = map_to_device, .unmap_from_device = unmap_from_device, .activate = activate, From c54870fc6663b010b2a3eb4a44df5dd41dd7011c Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 17 Jan 2019 11:41:46 +0100 Subject: [PATCH 13/16] drm: Reset mode on display-port connected outputs with a bad link-status With Display-Port links, esp. with DP MST links we may need to reset the mode if the kernel decides to retrain the link. If the kernel has retrained the link, the list of available modes may have changed. If it changed and the mode we picked is no longer available because of this, we treat this as an unplug + replug. Since we may want to set another mode, the kernel does not automatically restore the previous mode. So in case the mode did not change we need to do an explicit mode-set. This commits adds support for this, by: 1) Adding a scan_out_buffer_needs_reset member to ply_renderer_head 2) Storing the link-status when going over the connector properties 3) Checking the link-status when adding a connector to a head and setting the scan_out_buffer_needs_reset flag when the link-status is bad This commit also makes ply_renderer_head_map set scan_out_buffer_needs_reset, avoiding an unnecessary round-trip to the kernel in the first reset_scan_out_buffer_if_needed call. Signed-off-by: Hans de Goede --- src/plugins/renderers/drm/plugin.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c index b190737e..cb228f60 100644 --- a/src/plugins/renderers/drm/plugin.c +++ b/src/plugins/renderers/drm/plugin.c @@ -82,6 +82,7 @@ struct _ply_renderer_head uint32_t controller_id; uint32_t console_buffer_id; uint32_t scan_out_buffer_id; + bool scan_out_buffer_needs_reset; int gamma_size; uint16_t *gamma; @@ -122,6 +123,7 @@ typedef struct uint32_t controller_id; uint32_t possible_controllers; int device_scale; + int link_status; ply_pixel_buffer_rotation_t rotation; bool tiled; bool connected; @@ -473,6 +475,12 @@ ply_renderer_connector_get_rotation_and_tiled (ply_renderer_backend_t *back strcmp (prop->name, "TILE") == 0) output->tiled = true; + if ((prop->flags & DRM_MODE_PROP_ENUM) && + strcmp (prop->name, "link-status") == 0) { + output->link_status = connector->prop_values[i]; + ply_trace ("link-status %d", output->link_status); + } + drmModeFreeProperty (prop); } } @@ -481,6 +489,9 @@ static bool ply_renderer_head_add_connector (ply_renderer_head_t *head, ply_output_t *output) { + if (output->link_status == DRM_MODE_LINK_STATUS_BAD) + head->scan_out_buffer_needs_reset = true; + if (output->mode.hdisplay != head->area.width || output->mode.vdisplay != head->area.height) { ply_trace ("Tried to add connector with resolution %dx%d to %dx%d head", (int) output->mode.hdisplay, (int) output->mode.vdisplay, @@ -715,6 +726,7 @@ ply_renderer_head_map (ply_renderer_backend_t *backend, return false; } + head->scan_out_buffer_needs_reset = true; return true; } @@ -1589,6 +1601,13 @@ reset_scan_out_buffer_if_needed (ply_renderer_backend_t *backend, if (!ply_terminal_is_active (backend->terminal)) return false; + if (head->scan_out_buffer_needs_reset) { + ply_renderer_head_set_scan_out_buffer (backend, head, + head->scan_out_buffer_id); + head->scan_out_buffer_needs_reset = false; + return true; + } + controller = drmModeGetCrtc (backend->device_fd, head->controller_id); if (controller == NULL) From ccdb1d1fe1f99523d570b4e37cc14401c3891b9a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 21 Jan 2019 15:41:37 +0100 Subject: [PATCH 14/16] drm: Stop limiting preferred-mode picking to UEFI systems When the code to pick the preferred-mode for outputs was first added, it was limited to UEFI systems, since it was necessary there. It was not enabled everywhere right away because there were some worries it might cause regressions. We've been shipping this for a while now and no regressions have been reported, moreover with the new hotplug support we really want to pick the preferred-mode rather then falling back to the first mode in the list. Therefor this commits removes the check for UEFI systems from should_use_preferred_mode(). Signed-off-by: Hans de Goede --- src/plugins/renderers/drm/plugin.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c index cb228f60..0151f473 100644 --- a/src/plugins/renderers/drm/plugin.c +++ b/src/plugins/renderers/drm/plugin.c @@ -172,19 +172,9 @@ static bool reset_scan_out_buffer_if_needed (ply_renderer_backend_t *backend, static void flush_head (ply_renderer_backend_t *backend, ply_renderer_head_t *head); -static bool efi_enabled (void) -{ - return ply_directory_exists ("/sys/firmware/efi/efivars"); -} - /* A small helper to determine if we should try to keep the current mode - * or pick the best mode ourselves, we keep the current mode if: - * 1. The user specified a specific mode using video= on the commandline - * 2. The code to pick the best mode was added because with flicker-free boot - * we can no longer rely on the kernel's fbcon code setting things up. - * We should be able to do a better job then fbcon regardless, but for - * now lets only use the new code on flicker-free systems until it is - * more mature, this means only using it on UEFI systems. + * or pick the best mode ourselves, we keep the current mode only if the + * user specified a specific mode using video= on the commandline. */ static bool should_use_preferred_mode (void) @@ -194,9 +184,6 @@ should_use_preferred_mode (void) if (ply_kernel_command_line_get_string_after_prefix ("video=")) use_preferred_mode = false; - if (!efi_enabled ()) - use_preferred_mode = false; - ply_trace ("should_use_preferred_mode: %d", use_preferred_mode); return use_preferred_mode; From f9e376797a91ad5fbc1f8e8e4aea778f4f22397c Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 17 Jan 2019 14:52:41 +0100 Subject: [PATCH 15/16] ply-device-manager: Consume all events in one go Drm devices generate a bunch of add and change events when the kms driver loads, consume these all in one go. Signed-off-by: Hans de Goede --- src/libply-splash-core/ply-device-manager.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libply-splash-core/ply-device-manager.c b/src/libply-splash-core/ply-device-manager.c index 8ceee10d..028bf4ad 100644 --- a/src/libply-splash-core/ply-device-manager.c +++ b/src/libply-splash-core/ply-device-manager.c @@ -371,7 +371,7 @@ create_devices_for_subsystem (ply_device_manager_t *manager, return found_device; } -static void +static bool on_udev_event (ply_device_manager_t *manager) { struct udev_device *device; @@ -379,14 +379,14 @@ on_udev_event (ply_device_manager_t *manager) device = udev_monitor_receive_device (manager->udev_monitor); if (device == NULL) - return; + return false; action = udev_device_get_action (device); ply_trace ("got %s event for device %s", action, udev_device_get_sysname (device)); if (action == NULL) - return; + return false; if (strcmp (action, "add") == 0) { const char *subsystem; @@ -406,6 +406,14 @@ on_udev_event (ply_device_manager_t *manager) } udev_device_unref (device); + return true; +} + +static void +on_udev_event_loop (ply_device_manager_t *manager) +{ + /* Call on_udev_event until all events are consumed */ + while (on_udev_event (manager)) {} } static void @@ -435,7 +443,7 @@ watch_for_udev_events (ply_device_manager_t *manager) fd, PLY_EVENT_LOOP_FD_STATUS_HAS_DATA, (ply_event_handler_t) - on_udev_event, + on_udev_event_loop, NULL, manager); } From e54f6a47318d82b4e4df024855bd76d888b89675 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 14 Jan 2019 11:47:21 +0100 Subject: [PATCH 16/16] ply-device-manager: Handle change events for monitor hotplugging Not only handle add but also change events for drm-subsys devices, change events are generated when the hardware detect a new monitor has been plugged in. This is esp. important with modern DisplayPort MST docking stations where discovery / enumeration can take so long that the connected displays are not enumerated by the kernel yet when the drm plugin first calls drmModeGetResources(). Causing the monitors on these docks to sometimes not show plymouth during boot (based on various timing parameters). Note that if during the add event drm-renderer could not be bound, this commit tries to re-bind the DRM renderer on change events in case a monitor got plugged into a GPU which did not have anything connected before. This often happens with the second GPU in a laptop with a hybrid GPU setup. BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1652279 Signed-off-by: Hans de Goede --- src/libply-splash-core/ply-device-manager.c | 40 +++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/src/libply-splash-core/ply-device-manager.c b/src/libply-splash-core/ply-device-manager.c index 028bf4ad..e2a9eaee 100644 --- a/src/libply-splash-core/ply-device-manager.c +++ b/src/libply-splash-core/ply-device-manager.c @@ -51,6 +51,9 @@ static bool create_devices_for_terminal_and_renderer_type (ply_device_manager_t const char *device_path, ply_terminal_t *terminal, ply_renderer_type_t renderer_type); +static void create_pixel_displays_for_renderer (ply_device_manager_t *manager, + ply_renderer_t *renderer); + struct _ply_device_manager { ply_device_manager_flags_t flags; @@ -371,6 +374,39 @@ create_devices_for_subsystem (ply_device_manager_t *manager, return found_device; } +static void +on_drm_udev_add_or_change (ply_device_manager_t *manager, + const char *action, + struct udev_device *device) +{ + const char *device_path = udev_device_get_devnode (device); + ply_renderer_t *renderer; + bool changed; + + if (device_path == NULL) + return; + + renderer = ply_hashtable_lookup (manager->renderers, (void *) device_path); + if (renderer == NULL) { + /* We also try to create the renderer again on change events, + * renderer creation fails when no outputs are connected and + * this may have changed. + */ + create_devices_for_udev_device (manager, device); + return; + } + + /* Renderer exists, bail if this is not a change event */ + if (strcmp (action, "change")) + return; + + changed = ply_renderer_handle_change_event (renderer); + if (changed) { + free_displays_for_renderer (manager, renderer); + create_pixel_displays_for_renderer (manager, renderer); + } +} + static bool on_udev_event (ply_device_manager_t *manager) { @@ -388,7 +424,7 @@ on_udev_event (ply_device_manager_t *manager) if (action == NULL) return false; - if (strcmp (action, "add") == 0) { + if (strcmp (action, "add") == 0 || strcmp (action, "change") == 0) { const char *subsystem; subsystem = udev_device_get_subsystem (device); @@ -397,7 +433,7 @@ on_udev_event (ply_device_manager_t *manager) if (manager->local_console_managed && manager->local_console_is_text) ply_trace ("ignoring since we're already using text splash for local console"); else - create_devices_for_udev_device (manager, device); + on_drm_udev_add_or_change (manager, action, device); } else { ply_trace ("ignoring since we only handle subsystem %s devices after timeout", subsystem); }