GNOME Bugzilla – Bug 727023
Better error when additional output cannot be enabled
Last modified: 2021-06-09 16:10:34 UTC
If I plug in a second external monitor to my docking station (one builtin display and two externals), then the third output shows up as "off" in the Display panel. Which is fine because my hardware can only handle 2 outputs. However, I can still click the third output and try to enable it from the setup dialog. This does not do anything apart from sending me back to the panel. I think we should somehow communicate to the user that enabling this output is not expected to work. Maybe by not showing the setup dialog, or by keeping the "Apply" insensitive in the dialog? This is on a Lenovo x220 with Sandy Bridge graphics, but I am told that you can reproduce using a Lenovo T420 too.
For the Sandy Bridge case, the HW only has two display pipes, and we have that information in user space (there are only two CRTCs). gnome-control-center already checks that the configuration is applicable before making the button sensitive, so it is supposed to work. This could be a bug in gnome-control-center (maybe it lost the check?), gnome-desktop (maybe it's mistakenly cloning two monitors?) or mutter (maybe we expose one CRTC too many on the bus?). OTOH, the T420 has an Ivybridge, which has 3 display pipes and 2 clock generators, so it can technically drive three monitors, if two of them use a link of the same type (ie, both HDMI or both DP) and the same mode (size and refresh). Unfortunately, the current DRM API is too limited to expose this information, which is buried deep in the i915 code in the kernel, so there is nothing we can do in mutter or gnome-control-center to proactively inform the user.
(In reply to comment #1) > gnome-control-center > already checks that the configuration is applicable before making the button > sensitive, so it is supposed to work. > This could be a bug in gnome-control-center (maybe it lost the check?), I doubt that it checked in the recent past. The bug was pointed out by the RHEL7 QA folks, which is GNOME 3.8 and had the older UI, and I can reproduce using GNOME 3.11.x.
(In reply to comment #2) > (In reply to comment #1) > > gnome-control-center > > already checks that the configuration is applicable before making the button > > sensitive, so it is supposed to work. > > This could be a bug in gnome-control-center (maybe it lost the check?), > > I doubt that it checked in the recent past. The bug was pointed out by the > RHEL7 QA folks, which is GNOME 3.8 and had the older UI, and I can reproduce > using GNOME 3.11.x. I'm quite sure the old UI used to check, but yeah, the new one doesn't, so let's fix this one.
Created attachment 272874 [details] [review] display: make the Apply button unsensitive for invalid configuration If the configuration is not applicable, due to HW constraints we know about, make the button insensitive, to avoid an error dialog later on.
Review of attachment 272874 [details] [review]: Did you test this? It does not work for me. When I plug in the 3rd output and click on it, the Apply button stays insensitive. If I turn off the 2nd output in order to turn on the 3rd, then the Apply button does not come back. At that point I don't have a way to get back either the 2nd or 3rd output.
Created attachment 272877 [details] [review] display: Remove redundant statement and variable definition
Created attachment 272885 [details] [review] display: Silence -Wmaybe-uninitialized
Review of attachment 272874 [details] [review]: What is happening here is that when GnomeRR tries to check whether a CRTC can be assigned to the inactive output (see crtc_assignment_assign), the rotation of the OutputInfo is GNOME_RR_ROTATION_NEXT. I am not familiar with the semantics of the GnomeRR API, but it looks to me that this is meant to be an invalid value, and none of the CRTCs claim to support it. Hence gnome_rr_config_applicable fails. One way to fix this could be to ensure that OutputInfo->priv->rotation is initialized with GNOME_RR_ROTATION_0 during construction. The other could be to do that in gnome_rr_config_load_current.
Created attachment 273000 [details] [review] gnome-rr: Initialize GnomeRROutputInfo:rotation to GNOME_RR_ROTATION_0 I decided to do it during contruction because that looked like the more sensible place to do it. With this patch and the one from Giovanni, things work as expected.
(In reply to comment #8) > Review of attachment 272874 [details] [review]: > > What is happening here is that when GnomeRR tries to check whether a CRTC can > be assigned to the inactive output (see crtc_assignment_assign), the rotation > of the OutputInfo is GNOME_RR_ROTATION_NEXT. I am not familiar with the > semantics of the GnomeRR API, but it looks to me that this is meant to be an > invalid value, and none of the CRTCs claim to support it. Hence > gnome_rr_config_applicable fails. This makes no sense: if the output is inactive, we should not assign it to a CRTC (and indeed we don't, see real_assign_crtcs), so it doesn't matter what rotation it has.
(In reply to comment #10) > (In reply to comment #8) > > Review of attachment 272874 [details] [review] [details]: > > > > What is happening here is that when GnomeRR tries to check whether a CRTC can > > be assigned to the inactive output (see crtc_assignment_assign), the rotation > > of the OutputInfo is GNOME_RR_ROTATION_NEXT. I am not familiar with the > > semantics of the GnomeRR API, but it looks to me that this is meant to be an > > invalid value, and none of the CRTCs claim to support it. Hence > > gnome_rr_config_applicable fails. > > This makes no sense: if the output is inactive, we should not assign it to a > CRTC (and indeed we don't, see real_assign_crtcs), so it doesn't matter what > rotation it has. TLDR: setup_listbox_row_activated in cc-display-panel.c Every time you click on one of "primary", "secondary", "mirror", "turn off" in the GtkListBox the current GnomeRROutputInfo is activated depending whether "turn off" was chosen or not. So when gnome_rr_config_applicable is called after that (as in your first patch), it can check whether this output can actually be activated or not. Otherwise, as you noted, real_assign_crtcs will skip it and return TRUE instead of actually checking for applicability.
Review of attachment 273000 [details] [review]: Right, this makes sense.
Comment on attachment 273000 [details] [review] gnome-rr: Initialize GnomeRROutputInfo:rotation to GNOME_RR_ROTATION_0 Thanks for the review, Giovanni.
Review of attachment 272877 [details] [review]: ++ Not redudant though, it's unused. Redundant means you're doing it somewhere else.
Review of attachment 272885 [details] [review]: Meh. I wish we didn't have to reindent all of that, but...
*** Bug 728618 has been marked as a duplicate of this bug. ***
Review of attachment 272874 [details] [review]: It should be OK to push this now that the accompanying gnome-rr patch has gone in.
I'll reopen this with the associated ui-review.
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new bug report at https://gitlab.gnome.org/GNOME/gnome-control-center/-/issues/ Thank you for your understanding and your help.