[PATCH] xRandr strikes back

Thu Mar 14 06:31:24 PDT 2013

On Mon, Mar 11, 2013 at 3:47 PM, Alex Deucher <alexdeucher at gmail.com> wrote:

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

Hi Alex,

Thanks for your reply. I wanted to make a few things clearer so we're all
on the same page here.

>
> Some other things to consider, lest you end up in the same boat as X:
> 1. outputs on multiple GPUs
>

I believe weston drm backend already has multiple gpu support. I haven't
tested it though, someone can correct me if I am wrong.

> 2. one surface per output?
>

I'm pretty sure weston already does this. The default is multiple surfaces
and I don't think there's a way to have it use a single surface. Might this
help in the 'cloned' output case that weston does not currently support?

> 3. probably better to mirror the kernels display abstraction layers
> (crtc, encoder, connector) than X's (crtc, output) as it better
> matches the hardware.
>

After our IRC conversation, I think it's noteworthy to mention that you're
not saying to expose the crtc/enocoder/connector to the user but instead
handle these details properly for the drm case, internally. After reviewing
the drm backend code, it seems it does handle this information internally.
The user ends up with human readable information they can use to manipulate
output modes and surfaces including connector_name, available_outputs and
available_modes.

>
> Alex
>

- Scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130314/5f20391e/attachment-0001.html>