Monitoring for VCP features

[jpetso]

I only had a quick read of the code and documentation, please correct me if I missed anything.

The current D-Bus API supports GetVcp and SetVcp, which would e.g. be sufficient for reading and setting brightness. But if another app (e.g. your control panel) sets the brightness, then my service won't know about it and will still show the wrong value to the user unless I do regular polling.

As a centralized D-Bus service, you can at least monitor changes as they come into your API. You're closing the display handle after each D-Bus call, which I figure means that other users can access ddcutil through other means, so the service might miss out on those changes. My understanding is that libddcutil also doesn't provide notifications for changes made on the display's own firmware OSD - which seems reasonable I guess but also implies polling, maybe only when the UI appears or something.

So my questions would be:

• Going forward, should the expectation be that users must query VCP values whenever a UI starts showing?
• Even if so, can we still reduce the lag in the common case by signaling property changes when detected, so the UI already has the correct value most of the time when it starts showing?

[digitaltrails]

This is somewhat new territory.

I haven't really given a lot of thought to users wanting to use multiple applications that simultaneously adjust VCP values. This is partly because it hasn't been a major source of issues.

Multiple-applications is similar to the issue where the user alters a VDUs via its onboard controls, which is the main reason my own vdu_controls application has a refresh-button. I suppose the button also covers "interference" from other applications.

Going forward, should the expectation be that users must query VCP values whenever a UI starts showing?

Because of the aforementioned onboard-control issue, I suppose it's wise if an application did refresh VCP values on showing.

I can see the service as raising signals on an setvcp, or even on a getvcp if the value doesn't match the last known value. That seems to be easy to do. However the onboard-control problem undermines its value. I suppose the service could internally poll the VCP values to keep them up to date - I have an aversion to doing too much polling - I'm not sure what side-effects it has on VDU operation.

Up until now, the model I had commonly encountered, was that a user would pick one application to manage their VDU's. That's mainly because I have only been working with users of vdu_controls. Again, up until now, doing getvcp via the ddcutil was rather sluggish, so that meant minimising queries. The new service it is relatively fast to do a getvcp, plus the service offers a get_multiple_vcp method, so it could be quite quick to do a query.

However if one steps beyond just working with brightness, there may be many values to get, so it might be premature to say that a refresh is going to be fast in all cases.

Perhaps some concrete examples to finish with:

The onboard-control issues has meant that vdu_controls does assume values may change independently. For example, when vdu_controls is stepping brightness automatically (due to sun-elevation or lux-level), as it steps toward the new brightness, it also keeps checking if the values remain as expected. If values aren't as expected it stops.

The detection of hotplug and DPMS illustrate another can of worms. Not all drivers implement DRM and not all DRM supporting drivers implement connectivity and dpms status properly. Currently libddcutil writes-off these non-DRM/incomplete-DRM cases. I wanted the service to at least offer some option for these cases: currently the service may optionally be set to poll for changes. When polling, DPMS state is actually checked by doing a getvcp. I guess polling could be prefered and brightness could be included in this poll. I'm uncertain how much polling is safe for all combinations of hardware, so I only poll every 30 seconds.

I believe ddcci-plasmoid also does some amount of polling for similar reasons.

It's all a bit shaky. One of the reasons I appreciate libddcutil/ddcutil is that it manages to mostly keep working with dodgy combinations of drivers, GPUs, and cables. It will be interesting to see if the kernel folk can eventually come up with a DDC approach which is as robust, I guess they may be able to borrow heuristics from ddcutil.

[digitaltrails]

By the time I finished my brain dump I realised I had not completely addressed your comments. You packed in a lot of good points and information.

As a centralized D-Bus service, you can at least monitor changes as they come into your API.

Yes, I think I should raise signals on changes that come through.

You're closing the display handle after each D-Bus call, which I figure means that other users can access ddcutil through other means, so the service might miss out on those changes.

Yes, although I figure most uses will only use one DDC application, their will be some also doing things such as running ddcutil on the command line or in jobs.

My understanding is that libddcutil also doesn't provide notifications for changes made on the display's own firmware OSD - which seems reasonable I guess but also implies polling, maybe only when the UI appears or something.

Yes, correct. And yes, as both of us mentioned, polling might be an option.

[digitaltrails]

Proposed signal:

    <!--
        VcpChangedValue:
        @display_number: the display number
        @edid_txt: The base-64 encoded EDID of the display.
        @vcp_code: The VCP code whose value changed.
        @vcp_new_value: The new value.
        @source_client_name: The D-Bus client-name that requested the change (eases filtering signals caused by self).

        This signal will be raised if a SetVcp method call succeeds.
    -->
    <signal name='VcpChangedValue'>
        <arg name='display_number' type='i'/>
        <arg name='edid_txt' type='s'/>
        <arg name='vcp_code' type='y'/>
        <arg name='vcp_new_value' type='q'/>
        <arg name='source_client_name' type='s'/>
    </signal>

[digitaltrails]

There is a possible need for a client_context as well as a source_client_name. The D-Bus source_client_name provides a way for the originating client to identify itself as the source of a signal, but if the client is a multi-threaded, multi-compartmentalised GUI, it lacks an unambiguous way to tell what part of itself was responsible for the signal.

This would require a version of set_vcp which passes along a context: set_vcp_with_context()

[jpetso]

Thanks! I'd say the whole API is a little more "call-style" (with functions, and a signal with a good number of parameters) than "object-style" (with a D-Bus object for each connected display, and properties with PropertiesChanged signal) so it looks notably different than e.g. KWin's org.kde.KWin.InputDevice hierarchy. But this looks very workable, and indeed the addition of a context is a nice touch that I can see turning out useful.

To me it seems ready now to attempt getting buy-in for Plasma / PowerDevil / future KWin-on-Wayland depending on your service, as opposed to KDE's current direct use of libddcutil.