[PATCH] xRandr strikes back

[Alex Deucher]

On Mon, Mar 11, 2013 at 5:39 PM, Scott Moreau <oreaus at gmail.com> wrote:
>
>
> On Mon, Mar 11, 2013 at 12:59 PM, Pekka Paalanen <ppaalanen at gmail.com>
> wrote:
>>
>> On Sun, 10 Mar 2013 15:53:30 +0100
>> Hardening <rdp.effort at gmail.com> wrote:
>>
>> > This patch adds a wlrandr extension. It is useful to test
>> > mode switching. The patch provides the weston-switch-mode
>> > utility that can be use quite the same way as xrandr to
>> > change graphical modes. For now only the DRM backend supports
>> > mode switching, but other may follow.
>> > ---
>> >  clients/Makefile.am   |   10 ++
>> >  clients/switch-mode.c |  300
>> > +++++++++++++++++++++++++++++++++++++++++++++++++
>> >  protocol/Makefile.am  |    3 +-
>> >  protocol/wlrandr.xml  |   62 ++++++++++
>> >  src/Makefile.am       |    5 +
>> >  src/compositor.c      |    1 +
>> >  src/compositor.h      |    3 +
>> >  src/wlrandr.c         |  105 +++++++++++++++++
>> >  8 files changed, 488 insertions(+), 1 deletion(-)
>> >  create mode 100644 clients/switch-mode.c
>> >  create mode 100644 protocol/wlrandr.xml
>> >  create mode 100644 src/wlrandr.c
>> ...
>> > diff --git a/protocol/wlrandr.xml b/protocol/wlrandr.xml
>> > new file mode 100644
>> > index 0000000..192ba24
>> > --- /dev/null
>> > +++ b/protocol/wlrandr.xml
>> > @@ -0,0 +1,62 @@
>> > +<protocol name="wlrandr">
>> > +  <copyright>
>> > +    Copyright © 2013 Hardening, rdp.effort at gmail.com
>> > +
>> > +    Permission to use, copy, modify, distribute, and sell this
>> > +    software and its documentation for any purpose is hereby granted
>> > +    without fee, provided that the above copyright notice appear in
>> > +    all copies and that both that copyright notice and this permission
>> > +    notice appear in supporting documentation, and that the name of
>> > +    the copyright holders not be used in advertising or publicity
>> > +    pertaining to distribution of the software without specific,
>> > +    written prior permission.  The copyright holders make no
>> > +    representations about the suitability of this software for any
>> > +    purpose.  It is provided "as is" without express or implied
>> > +    warranty.
>> > +
>> > +    THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
>> > +    SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
>> > +    FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
>> > +    SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>> > +    WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
>> > +    AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
>> > +    ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
>> > +    THIS SOFTWARE.
>> > +  </copyright>
>> > +
>> > +
>> > +  <interface name="wlrandr" version="1">
>> > +    <description summary="xRandr for weston">
>> > +    This is a port of the xRandr extension for weston
>> > +    </description>
>> > +
>> > +    <request name="switch_mode">
>> > +      <description summary="switch mode">
>> > +        Asks weston to switch graphical mode for the given output.
>> > +      </description>
>> > +
>> > +      <arg name="output" type="object" interface="wl_output"/>
>> > +      <arg name="width" type="int"/>
>> > +      <arg name="height" type="int"/>
>> > +      <arg name="refresh" type="int"/>
>> > +    </request>
>> > +
>> > +    <enum name="switch_mode_result">
>> > +      <description summary="switch mode result">
>> > +        The result of a switch_mode request
>> > +      </description>
>> > +
>> > +     <entry name="ok" value="0"
>> > +             summary="the mode switch completed successfully"/>
>> > +     <entry name="failed" value="1"
>> > +             summary="something failed during mode switch (internal
>> > error)"/>
>> > +     <entry name="not_available" value="2"
>> > +             summary="the requested mode was not available"/>
>> > +    </enum>
>> > +
>> > +    <event name="done">
>> > +     <arg name='result' type='uint'/>
>> > +    </event>
>> > +  </interface>
>> > +
>> > +</protocol>
>>
>> Let's assume you have 3 outputs, and you change the modes on all of
>> them in a bunch. How do you know which 'done' event corresponds to
>> which 'switch_mode' request?
>>
>> I think if you want this kind of a one-shot reply for a request,
>> the request should create a new protocol object. That new object
>> then would have a single event and no requests defined, and it will
>> selfdestruct when the event leaves the server.
>>
>> wl_display.sync and wl_surface.frame are examples of such requests,
>> and I think you could reuse wl_callback as the reply object type,
>> as it carries an integer payload.
>>
>> That way you let the protocol object management take care of which
>> reply was for which request.
>>
>> Mode switching can take time, and it might even be best to be
>> performed with all outputs at once, instead of one output at a time
>> sequentially. So, maybe you should also think whether the protocol
>> itself should also have a concept of atomic mode setting for many
>> outputs in one go, so that a server does not have to guess how many
>> outputs you might want to change at a time.
>
>
> Indeed, multiple outputs introduces complexities. I think we should be able
> to follow most of the default xrandr assumptions here. These are a few
> random thoughts I have with that idea in mind:
>
> 1) A second output should be added to the right (or left) of the primary by
> default. (Is there a concept of a primary output in weston? Maybe the one at
> 0,0?)
> 2) There is no concept of --same-as with weston currently. (i.e. clone mode)
> 3) Outputs are generally ordered in a vertical fashion by default.
> 4) For multiple outputs, all of the outputs' positions should be
> recalculated even if only one of the output changes mode. (Perhaps it could
> be intelligent enough to detect when the positions do not need to be reset)

Some other things to consider, lest you end up in the same boat as X:
1. outputs on multiple GPUs
2. one surface per output?
3. probably better to mirror the kernels display abstraction layers
(crtc, encoder, connector) than X's (crtc, output) as it better
matches the hardware.

Alex