| Message ID | 1306922798-29344-1-git-send-email-florian@mickler.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 6/1/11 6:06 AM, Florian Mickler wrote: > Recently the kernel started reporting my outputs in a different ordering due to > > commit cb0953d734 > (drm/i915: Initialize LVDS and eDP outputs before anything else) > > Which made X choose a "wrong" resolution for my VGA display. Since they are > aligned horizontally, I wish them to be aligned in vertical > Resolution only. > > Before this patch, the sum of squared distances would force my VGA display > (1680x1050 native resolution) to 1280x1024 (non-native) due to my internal > display beeing considered first and 1400x1050 as native resolution. > > This was not an issue the other way around (VGA beeing first) because 1400x1050 > is nearest to 1680x1050 anyway. > > This patch changes the heuristic to only align resolution vertically if the > displays are horizontally aligned, or vice versa. > > Signed-off-by: Florian Mickler<florian@mickler.org> > --- > > Ok, Adam... seems I lost the staring contest... :) > What about something like this? At this point, given the near-unity overlap of "RANDRful drivers" and "usable KMS support", I think I'd prefer something more like: http://pkgs.fedoraproject.org/gitweb/?p=xorg-x11-server.git;a=blob_plain;f=xserver-1.6.99-right-of.patch;h=a0c9e7f64b98a091f64faaf5a8a432e2122e25f9;hb=HEAD particularly once per-crtc pixmaps land. - ajax
On Wed, 01 Jun 2011 15:30:03 -0400 Adam Jackson <ajax@redhat.com> wrote: > On 6/1/11 6:06 AM, Florian Mickler wrote: > > Recently the kernel started reporting my outputs in a different ordering due to > > > > commit cb0953d734 > > (drm/i915: Initialize LVDS and eDP outputs before anything else) > > > > Which made X choose a "wrong" resolution for my VGA display. Since they are > > aligned horizontally, I wish them to be aligned in vertical > > Resolution only. > > > > Before this patch, the sum of squared distances would force my VGA display > > (1680x1050 native resolution) to 1280x1024 (non-native) due to my internal > > display beeing considered first and 1400x1050 as native resolution. > > > > This was not an issue the other way around (VGA beeing first) because 1400x1050 > > is nearest to 1680x1050 anyway. > > > > This patch changes the heuristic to only align resolution vertically if the > > displays are horizontally aligned, or vice versa. > > > > Signed-off-by: Florian Mickler<florian@mickler.org> > > --- > > > > Ok, Adam... seems I lost the staring contest... :) > > What about something like this? > > At this point, given the near-unity overlap of "RANDRful drivers" and > "usable KMS support", I think I'd prefer something more like: > > http://pkgs.fedoraproject.org/gitweb/?p=xorg-x11-server.git;a=blob_plain;f=xserver-1.6.99-right-of.patch;h=a0c9e7f64b98a091f64faaf5a8a432e2122e25f9;hb=HEAD > > particularly once per-crtc pixmaps land. > > - ajax I think using horizontal spanning as a default is a good idea. Also bringing all outputs up in their preferred mode could be the right move. [That commit in question wouldn't help my case, since I have a "Right Of" in the xorg.conf and thus it would use a different code-path (right now).] When there is no preferred mode, xf86ClosestMode (probably enhanced to respect horizontal or vertical relation in some form) would still be needed... choosing of mode and positioning of outputs relative to each other should probably be decoupled... Regards, Flo
On 6/1/11 4:44 PM, Florian Mickler wrote: > I think using horizontal spanning as a default is a good idea. > > Also bringing all outputs up in their preferred mode could be the right > move. > > [That commit in question wouldn't help my case, since I have a "Right Of" in the > xorg.conf and thus it would use a different code-path (right now).] Oh, sure, I see what you're saying. > When there is no preferred mode, xf86ClosestMode (probably enhanced to > respect horizontal or vertical relation in some form) would still be > needed... choosing of mode and positioning of outputs > relative to each other should probably be decoupled... Probably. But in your case, why would you not just pick the preferred mode for all outputs, rather than contorting to try to pick modes with the same height? - ajax
On Wed, 01 Jun 2011 16:48:30 -0400 Adam Jackson <ajax@redhat.com> wrote: > On 6/1/11 4:44 PM, Florian Mickler wrote: > > I think using horizontal spanning as a default is a good idea. > > > > Also bringing all outputs up in their preferred mode could be the right > > move. > > > > [That commit in question wouldn't help my case, since I have a "Right Of" in the > > xorg.conf and thus it would use a different code-path (right now).] > > Oh, sure, I see what you're saying. > > > When there is no preferred mode, xf86ClosestMode (probably enhanced to > > respect horizontal or vertical relation in some form) would still be > > needed... choosing of mode and positioning of outputs > > relative to each other should probably be decoupled... > > Probably. But in your case, why would you not just pick the preferred > mode for all outputs, rather than contorting to try to pick modes with > the same height? > > - ajax Well... because the endeffect is the same in my case. :) But yes, if there is a preferred mode, that should be used. Is there a preferred mode in all cases? Regards, Flo
On 6/1/11 5:06 PM, Florian Mickler wrote: > Well... because the endeffect is the same in my case. :) > > But yes, if there is a preferred mode, that should be used. > > Is there a preferred mode in all cases? For any display with a discrete number of pixels (LCDs, etc), almost always. CRTs, most of the time, although it's not always the mode the user wants. - ajax
diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c index 4e3f6bf..96c6c62 100644 --- a/hw/xfree86/modes/xf86Crtc.c +++ b/hw/xfree86/modes/xf86Crtc.c @@ -872,15 +872,92 @@ xf86DefaultMode (xf86OutputPtr output, int width, int height) return target_mode; } +static Bool +xf86GetOptRelation(xf86OutputPtr output, OutputOpts *relation, char **relative_name) +{ + int r; + static const OutputOpts relations[] = { + OPTION_BELOW, OPTION_RIGHT_OF, OPTION_ABOVE, OPTION_LEFT_OF + }; + + *relation = 0; + *relative_name = NULL; + + for (r = 0; r < 4; r++) + { + *relation = relations[r]; + *relative_name = xf86GetOptValString (output->options, + *relation); + if (*relative_name) + break; + } + return (*relative_name != NULL); +} + +static Bool +xf86OutputsRelatedHorizontal(xf86OutputPtr out1, xf86OutputPtr out2) +{ + OutputOpts relation; + char *other_name; + Bool ret = FALSE; + + if (xf86GetOptRelation(out1, &relation, &other_name) && + strcmp(other_name, out2->name) == 0) { + /* + * out1 related to out2 + */ + ret = (relation == OPTION_RIGHT_OF || + relation == OPTION_LEFT_OF); + } + if (!ret && xf86GetOptRelation(out2, &relation, &other_name) && + strcmp(other_name, out1->name) == 0) { + /* + * out2 related to out1 + */ + ret = (relation == OPTION_RIGHT_OF || + relation == OPTION_LEFT_OF); + } + + return ret; + +} + +static Bool +xf86OutputsRelatedVertical(xf86OutputPtr out1, xf86OutputPtr out2) +{ + OutputOpts relation; + char *other_name; + Bool ret = FALSE; + + if (xf86GetOptRelation(out1, &relation, &other_name) && + strcmp(other_name, out2->name) == 0) { + /* + * out1 related to out2 + */ + ret = (relation == OPTION_ABOVE || + relation == OPTION_BELOW); + } + if (!ret && xf86GetOptRelation(out2, &relation, &other_name) && + strcmp(other_name, out1->name) == 0) { + /* + * out2 related to out1 + */ + ret = (relation == OPTION_ABOVE || + relation == OPTION_BELOW); + } + + return ret; +} + static DisplayModePtr -xf86ClosestMode (xf86OutputPtr output, +xf86ClosestMode (xf86OutputPtr output, xf86OutputPtr match_output, DisplayModePtr match, Rotation match_rotation, int width, int height) { DisplayModePtr target_mode = NULL; DisplayModePtr mode; int target_diff = 0; - + /* * Pick a mode closest to the specified mode */ @@ -892,15 +969,26 @@ xf86ClosestMode (xf86OutputPtr output, if (xf86ModeWidth (mode, output->initial_rotation) > width || xf86ModeHeight (mode, output->initial_rotation) > height) continue; - + /* exact matches are preferred */ if (output->initial_rotation == match_rotation && xf86ModesEqual (mode, match)) return mode; - + dx = xf86ModeWidth (match, match_rotation) - xf86ModeWidth (mode, output->initial_rotation); dy = xf86ModeHeight (match, match_rotation) - xf86ModeHeight (mode, output->initial_rotation); - diff = dx * dx + dy * dy; + + /* + * If we are aligning screens horizontally or vertically use + * only one dimension for matching + */ + if (xf86OutputsRelatedHorizontal(output, match_output)) + diff = dy * dy; + else if (xf86OutputsRelatedVertical(output, match_output)) + diff = dx * dx; + else + diff = dx * dx + dy * dy; + xf86DrvMsg (0, X_INFO, "XF86ClosestMode: Output %s, Mode %s [ dx = %i, dy = %i, diff = %i ]\n", output->name, mode->name, dx, dy, diff); @@ -1122,10 +1210,6 @@ xf86UserConfiguredOutputs(ScrnInfoPtr scrn, DisplayModePtr *modes) char *position; char *relative_name; OutputOpts relation; - int r; - static const OutputOpts relations[] = { - OPTION_BELOW, OPTION_RIGHT_OF, OPTION_ABOVE, OPTION_LEFT_OF - }; position = xf86GetOptValString (output->options, OPTION_POSITION); @@ -1137,15 +1221,7 @@ xf86UserConfiguredOutputs(ScrnInfoPtr scrn, DisplayModePtr *modes) } relation = 0; relative_name = NULL; - for (r = 0; r < 4; r++) - { - relation = relations[r]; - relative_name = xf86GetOptValString (output->options, - relation); - if (relative_name) - break; - } - if (relative_name) { + if (xf86GetOptRelation(output, &relation, &relative_name)) { user_conf = TRUE; xf86DrvMsg (scrn->scrnIndex, X_INFO, "Output %s has user_conf due to relation: %s\n", @@ -2245,7 +2321,8 @@ xf86TargetFallback(ScrnInfoPtr scrn, xf86CrtcConfigPtr config, /* Fill in other output modes */ for (o = -1; nextEnabledOutput(config, enabled, &o); ) { if (!modes[o]) - modes[o] = xf86ClosestMode(config->output[o], target_mode, + modes[o] = xf86ClosestMode(config->output[o], + config->output[config->compat_output], target_mode, target_rotation, width, height); }
Recently the kernel started reporting my outputs in a different ordering due to commit cb0953d734 (drm/i915: Initialize LVDS and eDP outputs before anything else) Which made X choose a "wrong" resolution for my VGA display. Since they are aligned horizontally, I wish them to be aligned in vertical Resolution only. Before this patch, the sum of squared distances would force my VGA display (1680x1050 native resolution) to 1280x1024 (non-native) due to my internal display beeing considered first and 1400x1050 as native resolution. This was not an issue the other way around (VGA beeing first) because 1400x1050 is nearest to 1680x1050 anyway. This patch changes the heuristic to only align resolution vertically if the displays are horizontally aligned, or vice versa. Signed-off-by: Florian Mickler <florian@mickler.org> --- Ok, Adam... seems I lost the staring contest... :) What about something like this? hw/xfree86/modes/xf86Crtc.c | 115 ++++++++++++++++++++++++++++++++++++------- 1 files changed, 96 insertions(+), 19 deletions(-)