Mir

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

Proposed by Daniel van Vugt on 2017-04-05
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 on 2017-04-06
Cemil Azizoglu (community) Abstain on 2017-04-05
Chris Halse Rogers 2017-04-05 Abstain on 2017-04-05
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.
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
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 on 2017-04-05

Again?

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)
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 on 2017-04-06

Merge latest trunk

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)
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'.

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 on 2017-04-06

Try again

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 on 2017-04-06

Try again

4142. By Daniel van Vugt on 2017-04-06

Merge latest trunk

4141. By Daniel van Vugt on 2017-04-05

Again?

4140. By Daniel van Vugt on 2017-04-05

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
1=== modified file 'src/platforms/mesa/server/kms/display.cpp'
2--- src/platforms/mesa/server/kms/display.cpp 2017-03-31 06:56:29 +0000
3+++ src/platforms/mesa/server/kms/display.cpp 2017-04-06 09:50:50 +0000
4@@ -95,34 +95,6 @@
5 return fds;
6 }
7
8-double calculate_vrefresh_hz(drmModeModeInfo const& mode)
9-{
10- if (mode.htotal == 0 || mode.vtotal == 0)
11- return 0.0;
12-
13- /* mode.clock is in KHz */
14- double hz = (mode.clock * 100000LL /
15- ((long)mode.htotal * (long)mode.vtotal)
16- ) / 100.0;
17-
18- return hz;
19-}
20-
21-char const* describe_connection_status(drmModeConnector const& connection)
22-{
23- switch (connection.connection)
24- {
25- case DRM_MODE_CONNECTED:
26- return "connected";
27- case DRM_MODE_DISCONNECTED:
28- return "disconnected";
29- case DRM_MODE_UNKNOWNCONNECTION:
30- return "UNKNOWN";
31- default:
32- return "<Unexpected connection value>";
33- }
34-}
35-
36 void log_drm_details(std::vector<std::shared_ptr<mgm::helpers::DRMHelper>> const& drm)
37 {
38 mir::log_info("DRM device details:");
39@@ -146,23 +118,6 @@
40 version->version_minor,
41 version->version_patchlevel,
42 version->date);
43-
44- mg::kms::DRMModeResources resources{device->fd};
45- for (auto const& connector : resources.connectors())
46- {
47- mir::log_info(
48- "\tOutput: %s (%s)",
49- mg::kms::connector_name(connector).c_str(),
50- describe_connection_status(*connector));
51- for (auto i = 0; i < connector->count_modes; ++i)
52- {
53- mir::log_info(
54- "\t\tMode: %i×%i@%.2f",
55- connector->modes[i].hdisplay,
56- connector->modes[i].vdisplay,
57- calculate_vrefresh_hz(connector->modes[i]));
58- }
59- }
60 }
61 }
62

Subscribers

People subscribed via source and target branches