Mir

Merge lp:~vanvugt/mir/remove-hybrid-outputs-logging into lp:mir

Proposed by Daniel van Vugt
Status: Work in progress
Proposed branch: lp:~vanvugt/mir/remove-hybrid-outputs-logging
Merge into: lp:mir
Diff against target: 61 lines (+0/-45)
1 file modified
src/platforms/mesa/server/kms/display.cpp (+0/-45)
To merge this branch: bzr merge lp:~vanvugt/mir/remove-hybrid-outputs-logging
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve
Cemil Azizoglu (community) Abstain
Chris Halse Rogers Abstain
Review via email: mp+321943@code.launchpad.net

Commit message

Remove redundant logging of outputs introduced with hybrid support
in mesa-kms recently.

The existing logging we already do for all platforms is more detailed
and more useful. The new logging being removed here is less than useful
because it's single-platform, confused by missing output IDs, missing most
of the output details we already log elsewhere, and simultaneously floods
the server log with all supported mode details which we don't need.

If we do want that last part, it should be added to the existing
mir::report::logging::DisplayConfigurationReport
instead, where it would apply to all graphics drivers.

To post a comment you must log in.
Revision history for this message
Chris Halse Rogers (raof) wrote :

Weak disapprove.

I agree that we log similar information elsewhere in startup (and at other times), but I think this information is valuable enough to log. It shows some of the raw hardware state at startup, and associates it with the relevant device node; this is inherently driver-specific.

On a meta level, I think we log roughly an order of magnitude too little information on startup, so I place a low cost on extra once-off startup logging.

Since we don't plan to provide fake modes in our DisplayConfiguration I'd be happy enough for the supported modes information to be moved to the generic DisplayConfigurationReport.

review: Abstain
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:4140
https://mir-jenkins.ubuntu.com/job/mir-ci/3309/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/4472/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4587
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4576
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4576
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4576
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4576
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4503
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4503/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4503
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4503/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4503
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4503/artifact/output/*zip*/output.zip
    ABORTED: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4503/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4503/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4503
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4503/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4503
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4503/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3309/rebuild

review: Needs Fixing (continuous-integration)
4141. By Daniel van Vugt

Again?

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:4141
https://mir-jenkins.ubuntu.com/job/mir-ci/3313/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/4478/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4595
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4584
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4584
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4584
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4584
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4509
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4509/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4509
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4509/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4509
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4509/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4509/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4509/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4509
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4509/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4509
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4509/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3313/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

I don't think this is really hurting anything and the extra information should be useful.

review: Abstain
4142. By Daniel van Vugt

Merge latest trunk

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:4142
https://mir-jenkins.ubuntu.com/job/mir-ci/3325/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/4492/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4610
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4599
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4599
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4599
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4599
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4523
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4523/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4523
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4523/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4523
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4523/artifact/output/*zip*/output.zip
    ABORTED: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4523/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4523/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4523
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4523/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4523
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4523/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3325/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The only extra information here is a full listing of supported video modes emitted to the server log. But if you want that you can get it already by running 'mirout'.

Revision history for this message
Chris Halse Rogers (raof) wrote :

It also shows the DRM devices associated with each output, which isn't
available anywhere else.

Also mirout isn't piped into our logs, so is less useful for
bugreporting.

(I'm perfectly happy for the full mode info to be a part of
initial_configuration() logging, though)

4143. By Daniel van Vugt

Try again

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4143
https://mir-jenkins.ubuntu.com/job/mir-ci/3329/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4498
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4616
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4605
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4605
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4605
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4605
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4529
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4529/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4529
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4529/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4529
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4529/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4529
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4529/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4529
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4529/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4529
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4529/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3329/rebuild

review: Approve (continuous-integration)

Unmerged revisions

4143. By Daniel van Vugt

Try again

4142. By Daniel van Vugt

Merge latest trunk

4141. By Daniel van Vugt

Again?

4140. By Daniel van Vugt

Remove extraneous logging of outputs introduced with hybrid support
in mesa-kms recently.

The existing logging we already do for all platforms is more detailed
and more useful. The new logging being removed here is less than useful
because it's single-platform, confused by missing output IDs, missing most
of the output details we already log elsewhere, and simultaneously floods
the server log with all supported mode details which we don't need.

If we do want that last part, it should be added to the existing
mir::report::logging::DisplayConfigurationReport
instead, where it would apply to all graphics drivers.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/platforms/mesa/server/kms/display.cpp'
--- src/platforms/mesa/server/kms/display.cpp 2017-03-31 06:56:29 +0000
+++ src/platforms/mesa/server/kms/display.cpp 2017-04-06 09:50:50 +0000
@@ -95,34 +95,6 @@
95 return fds;95 return fds;
96}96}
9797
98double calculate_vrefresh_hz(drmModeModeInfo const& mode)
99{
100 if (mode.htotal == 0 || mode.vtotal == 0)
101 return 0.0;
102
103 /* mode.clock is in KHz */
104 double hz = (mode.clock * 100000LL /
105 ((long)mode.htotal * (long)mode.vtotal)
106 ) / 100.0;
107
108 return hz;
109}
110
111char const* describe_connection_status(drmModeConnector const& connection)
112{
113 switch (connection.connection)
114 {
115 case DRM_MODE_CONNECTED:
116 return "connected";
117 case DRM_MODE_DISCONNECTED:
118 return "disconnected";
119 case DRM_MODE_UNKNOWNCONNECTION:
120 return "UNKNOWN";
121 default:
122 return "<Unexpected connection value>";
123 }
124}
125
126void log_drm_details(std::vector<std::shared_ptr<mgm::helpers::DRMHelper>> const& drm)98void log_drm_details(std::vector<std::shared_ptr<mgm::helpers::DRMHelper>> const& drm)
127{99{
128 mir::log_info("DRM device details:");100 mir::log_info("DRM device details:");
@@ -146,23 +118,6 @@
146 version->version_minor,118 version->version_minor,
147 version->version_patchlevel,119 version->version_patchlevel,
148 version->date);120 version->date);
149
150 mg::kms::DRMModeResources resources{device->fd};
151 for (auto const& connector : resources.connectors())
152 {
153 mir::log_info(
154 "\tOutput: %s (%s)",
155 mg::kms::connector_name(connector).c_str(),
156 describe_connection_status(*connector));
157 for (auto i = 0; i < connector->count_modes; ++i)
158 {
159 mir::log_info(
160 "\t\tMode: %i×%i@%.2f",
161 connector->modes[i].hdisplay,
162 connector->modes[i].vdisplay,
163 calculate_vrefresh_hz(connector->modes[i]));
164 }
165 }
166 }121 }
167}122}
168123

Subscribers

People subscribed via source and target branches