Den 18.04.2019 18.40, skrev Noralf Trønnes: > > > Den 18.04.2019 14.41, skrev Maxime Ripard: >> Rotations and reflections setup are needed in some scenarios to initialise >> properly the initial framebuffer. Some drivers already had a bunch of >> quirks to deal with this, such as either a private kernel command line >> parameter (omapdss) or on the device tree (various panels). >> >> In order to accomodate this, let's create a video mode parameter to deal >> with the rotation and reflexion. >> >> Signed-off-by: Maxime Ripard <maxime.rip...@bootlin.com> >> --- >> drivers/gpu/drm/drm_client_modeset.c | 10 +++- >> drivers/gpu/drm/drm_modes.c | 110 ++++++++++++++++++++++------ >> include/drm/drm_connector.h | 9 ++- >> 3 files changed, 109 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_client_modeset.c >> b/drivers/gpu/drm/drm_client_modeset.c >> index f2869c82510c..15145d2c86d5 100644 >> --- a/drivers/gpu/drm/drm_client_modeset.c >> +++ b/drivers/gpu/drm/drm_client_modeset.c >> @@ -823,6 +823,7 @@ EXPORT_SYMBOL(drm_client_modeset_probe); >> bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int >> *rotation) >> { >> struct drm_connector *connector = modeset->connectors[0]; >> + struct drm_cmdline_mode *cmdline; >> struct drm_plane *plane = modeset->crtc->primary; >> u64 valid_mask = 0; >> unsigned int i; >> @@ -844,6 +845,15 @@ bool drm_client_panel_rotation(struct drm_mode_set >> *modeset, unsigned int *rotat >> *rotation = DRM_MODE_ROTATE_0; >> } >> >> + /** >> + * We want the rotation on the command line to overwrite >> + * whatever comes from the panel. >> + */ >> + cmdline = &connector->cmdline_mode; >> + if (cmdline->specified && >> + cmdline->rotation != DRM_MODE_ROTATE_0) > > I believe you need to drop that second check, otherwise rotate=0 will > not overwrite panel rotation. > >> + *rotation = cmdline->rotation;
I remembered that you wanted this to propagate to DRM userspace. That's not happening here. The only way I see for that to happen, is to set ->panel_orientation. And to repeat myself, imo that makes 'orientation' a better name for this video= option. Noralf. >> + >> /* >> * TODO: support 90 / 270 degree hardware rotation, >> * depending on the hardware this may require the framebuffer >> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c >> index 9613c1a28487..ac8d70b92b62 100644 >> --- a/drivers/gpu/drm/drm_modes.c >> +++ b/drivers/gpu/drm/drm_modes.c >> @@ -1531,6 +1531,71 @@ static int drm_mode_parse_cmdline_res_mode(const char >> *str, unsigned int length, >> return 0; >> } >> >> +static int drm_mode_parse_cmdline_options(char *str, size_t len, >> + struct drm_connector *connector, >> + struct drm_cmdline_mode *mode) >> +{ >> + unsigned int rotation = 0; >> + char *sep = str; >> + >> + while ((sep = strchr(sep, ','))) { >> + char *delim, *option; >> + >> + option = sep + 1; >> + delim = strchr(option, '='); >> + if (!delim) { >> + delim = strchr(option, ','); >> + >> + if (!delim) >> + delim = str + len; >> + } >> + >> + if (!strncmp(option, "rotate", delim - option)) { >> + const char *value = delim + 1; >> + unsigned int deg; >> + >> + deg = simple_strtol(value, &sep, 10); >> + >> + /* Make sure we have parsed something */ >> + if (sep == value) >> + return -EINVAL; >> + >> + switch (deg) { >> + case 0: >> + rotation |= DRM_MODE_ROTATE_0; >> + break; >> + >> + case 90: >> + rotation |= DRM_MODE_ROTATE_90; >> + break; >> + >> + case 180: >> + rotation |= DRM_MODE_ROTATE_180; >> + break; >> + >> + case 270: >> + rotation |= DRM_MODE_ROTATE_270; >> + break; >> + >> + default: >> + return -EINVAL; >> + } >> + } else if (!strncmp(option, "reflect_x", delim - option)) { >> + rotation |= DRM_MODE_REFLECT_X; >> + sep = delim; >> + } else if (!strncmp(option, "reflect_y", delim - option)) { > > I think you should drop reflect_x and _y for now since they're not > supported. People like me that only reads code and not documentation > (ahem..) will get the impression that this should work. > > Documentation/fb/modedb.txt needs to be updated with this new video= option. > > Noralf. > >> + rotation |= DRM_MODE_REFLECT_Y; >> + sep = delim; >> + } else { >> + return -EINVAL; >> + } >> + } >> + >> + mode->rotation = rotation; >> + >> + return 0; >> +} >> + >> /** >> * drm_mode_parse_command_line_for_connector - parse command line modeline >> for connector >> * @mode_option: optional per connector mode option >> @@ -1558,9 +1623,10 @@ bool drm_mode_parse_command_line_for_connector(const >> char *mode_option, >> { >> const char *name; >> bool named_mode = false, parse_extras = false; >> - unsigned int bpp_off = 0, refresh_off = 0; >> + unsigned int bpp_off = 0, refresh_off = 0, options_off = 0; >> unsigned int mode_end = 0; >> char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL; >> + char *options_ptr = NULL; >> char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL; >> int ret; >> >> @@ -1601,13 +1667,18 @@ bool drm_mode_parse_command_line_for_connector(const >> char *mode_option, >> mode->refresh_specified = true; >> } >> >> + /* Locate the start of named options */ >> + options_ptr = strchr(name, ','); >> + if (options_ptr) >> + options_off = options_ptr - name; >> + >> /* Locate the end of the name / resolution, and parse it */ >> - if (bpp_ptr && refresh_ptr) { >> - mode_end = min(bpp_off, refresh_off); >> - } else if (bpp_ptr) { >> + if (bpp_ptr) { >> mode_end = bpp_off; >> } else if (refresh_ptr) { >> mode_end = refresh_off; >> + } else if (options_ptr) { >> + mode_end = options_off; >> } else { >> mode_end = strlen(name); >> parse_extras = true; >> @@ -1649,24 +1720,23 @@ bool drm_mode_parse_command_line_for_connector(const >> char *mode_option, >> else if (refresh_ptr) >> extra_ptr = refresh_end_ptr; >> >> - if (extra_ptr) { >> - if (!named_mode) { >> - int len = strlen(name) - (extra_ptr - name); >> + if (extra_ptr && >> + extra_ptr != options_ptr) { >> + int len = strlen(name) - (extra_ptr - name); >> >> - ret = drm_mode_parse_cmdline_extra(extra_ptr, len, >> - connector, mode); >> - if (ret) >> - return false; >> - } else { >> - int remaining = strlen(name) - (extra_ptr - name); >> + ret = drm_mode_parse_cmdline_extra(extra_ptr, len, >> + connector, mode); >> + if (ret) >> + return false; >> + } >> >> - /* >> - * We still have characters to process, while >> - * we shouldn't have any >> - */ >> - if (remaining > 0) >> - return false; >> - } >> + if (options_ptr) { >> + int len = strlen(name) - (options_ptr - name); >> + >> + ret = drm_mode_parse_cmdline_options(options_ptr, len, >> + connector, mode); >> + if (ret) >> + return false; >> } >> >> return true; >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h >> index 06aa3b9cb920..6f57c1a3afff 100644 >> --- a/include/drm/drm_connector.h >> +++ b/include/drm/drm_connector.h >> @@ -908,6 +908,15 @@ struct drm_cmdline_mode { >> bool cvt; >> bool margins; >> enum drm_connector_force force; >> + >> + /** >> + * @rotation: >> + * >> + * Initial rotation of the mode setup from the command line. >> + * See DRM_MODE_ROTATE_*. Only DRM_MODE_ROTATE_0 and >> + * DRM_MODE_ROTATE_180 are supported at the moment. >> + */ >> + unsigned int rotation; >> }; >> >> /** >> > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel