Skip to content

Add --drm-vout-display Parameter #474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,9 @@ OPTIONS:
--dummy-display-size "width,height" The width & height of the dummy display
in pixels.

--display <device> The display to use.\n\
HDMI-A-1, HDMI-A-2, DSI-1, DSI-2.\n\

-h, --help Show this help and exit.

EXAMPLES:
Expand Down
63 changes: 62 additions & 1 deletion src/flutter-pi.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ OPTIONS:\n\
without a display attached.\n\
--dummy-display-size \"width,height\" The width & height of the dummy display\n\
in pixels.\n\
--display <device> The display to use.\n\
HDMI-A-1, HDMI-A-2, DSI-1, DSI-2.\n\
\n\
-h, --help Show this help and exit.\n\
\n\
Expand Down Expand Up @@ -262,6 +264,8 @@ struct flutterpi {
bool session_active;

char *desired_videomode;

char *drm_vout_display;
};

struct device_id_and_fd {
Expand Down Expand Up @@ -1853,6 +1857,16 @@ static bool parse_vec2i(const char *str, struct vec2i *out) {
return true;
}

bool is_valid_drm_display(const char *display) {
const char *valid_displays[] = { "HDMI-A-1", "HDMI-A-2", "DSI-1", "DSI-2" };
for (size_t i = 0; i < sizeof(valid_displays) / sizeof(valid_displays[0]); i++) {
if (strcmp(display, valid_displays[i]) == 0) {
return true;
}
}
return false;
}

bool flutterpi_parse_cmdline_args(int argc, char **argv, struct flutterpi_cmdline_args *result_out) {
bool finished_parsing_options;
int runtime_mode_int = FLUTTER_RUNTIME_MODE_DEBUG;
Expand All @@ -1876,6 +1890,7 @@ bool flutterpi_parse_cmdline_args(int argc, char **argv, struct flutterpi_cmdlin
{ "videomode", required_argument, NULL, 'v' },
{ "dummy-display", no_argument, &dummy_display_int, 1 },
{ "dummy-display-size", required_argument, NULL, 's' },
{ "display", required_argument, NULL, 'i' },
{ 0, 0, 0, 0 },
};

Expand Down Expand Up @@ -1997,6 +2012,17 @@ bool flutterpi_parse_cmdline_args(int argc, char **argv, struct flutterpi_cmdlin

break;

case 'i': // --display
result_out->drm_vout_display = strdup(optarg);
if (result_out->drm_vout_display == NULL) {
return false;
}
if (!is_valid_drm_display(result_out->drm_vout_display)) {
LOG_ERROR("Invalid display specified: %s. Valid options are HDMI-A-1, HDMI-A-2, DSI-1, DSI-2.\n", optarg);
return false;
}
break;

case 'h': printf("%s", usage); return false;

case '?':
Expand Down Expand Up @@ -2099,6 +2125,25 @@ static void on_drmdev_close(int fd, void *fd_metadata, void *userdata) {

static const struct drmdev_interface drmdev_interface = { .open = on_drmdev_open, .close = on_drmdev_close };

bool parse_drm_vout_display(const char *display, int *type_out, int *type_id_out) {
if (strcmp(display, "HDMI-A-1") == 0) {
*type_out = DRM_MODE_CONNECTOR_HDMIA;
*type_id_out = 1;
} else if (strcmp(display, "HDMI-A-2") == 0) {
*type_out = DRM_MODE_CONNECTOR_HDMIA;
*type_id_out = 2;
} else if (strcmp(display, "DSI-1") == 0) {
*type_out = DRM_MODE_CONNECTOR_DSI;
*type_id_out = 1;
} else if (strcmp(display, "DSI-2") == 0) {
*type_out = DRM_MODE_CONNECTOR_DSI;
*type_id_out = 2;
} else {
return false;
}
return true;
}

static struct drmdev *find_drmdev(struct libseat *libseat) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static struct drmdev *find_drmdev(struct libseat *libseat) {
static struct drmdev *find_drmdev(struct libseat *libseat, const char *connector_name) {

I think passing via argument here is better, generally I don't want to add new code that uses the flutterpi global variable. (Especially since find_drmdev is called from the initialization of that same flutterpi instance, so it's very easy to make mistakes and e.g. use something before it's initialized)

struct drm_connector *connector;
struct drmdev *drmdev;
Expand Down Expand Up @@ -2137,7 +2182,21 @@ static struct drmdev *find_drmdev(struct libseat *libseat) {

for_each_connector_in_drmdev(drmdev, connector) {
if (connector->variable_state.connection_state == kConnected_DrmConnectionState) {
goto found_connected_connector;
if (flutterpi->drm_vout_display != NULL) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (flutterpi->drm_vout_display != NULL) {
if (connector_name != NULL) {

// We only want to use the display that was specified on the command line.
int expected_type, expected_type_id;
if (!parse_drm_vout_display(flutterpi->drm_vout_display, &expected_type, &expected_type_id)) {
continue;
}
Comment on lines +2187 to +2190
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be a bit counter-intuitive but what could also be done here is converting the current connector to a string and comparing that to the passed in name:

char *connector_type_name = drmModeGetConnectorTypeName(connector->type);
if (connector_type_name == null) continue;

char *connector_name = NULL;
int ok = asprintf(&connector_name, "%s-%d", connector_name, connector->type_id);
if (ok == -1) continue;

if (strcmp(connector_name) == desired_connector_name)) {
  // found connector
}

Has the benefit that it's compatible to all the connectors: https://github.com/torvalds/linux/blob/master/include/uapi/drm/drm_mode.h#L403-L423 and not just the ones present on the Pi, and it's also future-proof since we don't need to change the code to recognize new connector types, drmModeGetConnectorTypeName (which is from libdrm, not flutter-pi) will handle that for us. But that's your choice, I think this is good enough for now. :)


if (connector->type == expected_type && connector->type_id == expected_type_id) {
goto found_connected_connector;
} else {
continue;
}
} else {
goto found_connected_connector;
}
}
}
LOG_ERROR("Device \"%s\" doesn't have a display connected. Skipping.\n", device->nodes[DRM_NODE_PRIMARY]);
Expand Down Expand Up @@ -2335,6 +2394,8 @@ struct flutterpi *flutterpi_new_from_args(int argc, char **argv) {
goto fail_free_fpi;
}

fpi->drm_vout_display = cmd_args.drm_vout_display ? strdup(cmd_args.drm_vout_display) : NULL;

#ifndef HAVE_VULKAN
if (cmd_args.use_vulkan == true) {
LOG_ERROR("ERROR: --vulkan was specified, but flutter-pi was built without vulkan support.\n");
Expand Down
2 changes: 2 additions & 0 deletions src/flutter-pi.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ struct flutterpi_cmdline_args {

bool dummy_display;
struct vec2i dummy_display_size;

char *drm_vout_display;
};

int flutterpi_fill_view_properties(bool has_orientation, enum device_orientation orientation, bool has_rotation, int rotation);
Expand Down