[PATCH V2 1/8] wesston: Add weston randr protocol
Pekka Paalanen
ppaalanen at gmail.com
Mon Mar 31 00:13:15 PDT 2014
On Mon, 31 Mar 2014 02:48:09 +0000
"Wang, Quanxian" <quanxian.wang at intel.com> wrote:
>
>
> >-----Original Message-----
> >From: Pekka Paalanen [mailto:ppaalanen at gmail.com]
> >Sent: Friday, March 28, 2014 11:00 PM
> >To: Wang, Quanxian
> >Cc: wayland-devel at lists.freedesktop.org
> >Subject: Re: [PATCH V2 1/8] wesston: Add weston randr protocol
> >
> >On Mon, 24 Mar 2014 19:39:13 +0800
> >Quanxian Wang <quanxian.wang at intel.com> wrote:
> >
> >> Weston protocol wrandr will provide interface to
> >> 1) Mode set of output (scale, transform, mode)
> >> 2) Position of output (currently support leftof, rightof)
> >> 3) New a custom mode
> >> 4) Delete mode
> >>
> >> This protocol is not expose public. It is only for QA testing and
> >> Admin configuration currently.
> >>
> >> Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
> >> ---
> >> protocol/Makefile.am | 1 +
> >> protocol/randr.xml | 228
> >+++++++++++++++++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 229 insertions(+)
> >> create mode 100644 protocol/randr.xml
> >>
> >> diff --git a/protocol/Makefile.am b/protocol/Makefile.am index
> >> 5e331a7..df2e070 100644
> >> --- a/protocol/Makefile.am
> >> +++ b/protocol/Makefile.am
> >> @@ -5,6 +5,7 @@ protocol_sources = \
> >> text.xml \
> >> input-method.xml \
> >> workspaces.xml \
> >> + randr.xml \
> >> text-cursor-position.xml \
> >> wayland-test.xml \
> >> xdg-shell.xml \
> >> diff --git a/protocol/randr.xml b/protocol/randr.xml new file mode
> >> 100644 index 0000000..07f83a4
> >> --- /dev/null
> >> +++ b/protocol/randr.xml
> >> @@ -0,0 +1,228 @@
> >> +<?xml version="1.0" encoding="UTF-8"?> <protocol name="randr">
> >> +
> >> + <copyright>
> >> + Copyright (c) 2014 Quanxian Wang
> >> + Copyright (c) 2014 Intel Corporation
> >> +
> >> + 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="weston_randr" version="1">
> >> + <description summary="randr">
> >> + The global interface exposing randr capabilities.
> >> + As a weston_randr, that provides the interfaces for apps to more
> >operations
> >> + on output.
> >> +
> >> + The aim of weston_randr is to get modes list, choose preferred mode,
> >> + layout the output including position, rotate, and en/disable.
> >> + The idea is from xrandr protocoal of xserver. It is very convenient for
> >> + weston/wayland user to operates on mode setting of output.
> >> + </description>
> >> +
> >> + <request name="destroy" type="destructor">
> >> + <description summary="unbind from the weston_randr interface">
> >> + Informs the server that the client will not be using this
> >> + protocol object anymore. This does not affect any other
> >> + objects, weston_randr objects included.
> >> + </description>
> >> + </request>
> >> +
> >> + <request name="start">
> >> + <description summary="randr request callback">
> >> + It is request notification when the next output randr commit
> >> + is coming and is useful for notifying client the result of
> >> + operations on the output. The randr request will take effect
> >> + on the next weston_randr.commit. The notification will only be
> >> + posted for one randr request unless requested again.
> >> + </description>
> >> + <arg name="output" type="object" interface="wl_output"/>
> >> + <arg name="callback" type="new_id"
> >> +interface="wrandr_callback"/>
> >
> >Ok, the reply object is created as the first thing. What happens if you create
> >multiple of them without sending commit in between? They all return the same
> >result?
> >
> >Why do you have an output argument here?
> >Are you explicitly forbidding the very useful case, where one could gather changes
> >to multiple outputs into the same batch to be committed?
> >
> >I think that would be a misfeature. I don't see any reason to not allow changing
> >multiple outputs in the same batch. Quite the contrary, when atomic modesetting
> >becomes available in the kernel, we will be able to use it only if we allow changing
> >multiple outputs at the same time in the protocol.
> [Wang, Quanxian] Currently I delete randr_start interface, and will return callback in randr_commit.
> We support multiple outputs now. But the return results will base on every output and will send result to user who are interested with that one by one.
> With output argument in callback, It will tell user what operation on that and what results is for every operations.
>
> For example: 'run this complex randr command', it will return the a little detailed information that what is be done? What is error? What is not done? It will be helpful for client to check their operations.
> weston-wrandr --output HDMI3 --transform 1 --scale 2 --mode 2 --rightof VGA1 --newmode='1000,300,100,120,150,400,350,370,399,+hsync -vsync csync +csync -csync'
> output:HDMI3
> TRANSFORM:SUCCESS!
> MODE:SUCCESS!
> SCALE:SUCCESS!
> MOVE:Same as current, no change!
> NEWMODE:SUCCESS!
> >
> >It does not matter if the kernel or platform do not support atomic mode setting
> >over multiple outputs, the protocol can and should still have it.
> >
> >> + </request>
> >> +
> >> + <enum name="mode">
> >> + <description summary="mode information">
> >> + These flags describe properties of an output mode.
> >> + They are used in the flags bitfield of the mode event.
> >> + Here we take the last 28-32th bit as additional flags
> >> + which is different with original output mode.
> >
> >Sorry, how does this work? What mode event?
> >
> >If you mean wl_output.mode event, I'm not sure you can extend the flags there
> >from here. wl_output would need to specify, that flags not defined there should
> >be ignored by the client.
> >
> >> + </description>
> >> + <entry name="custom" value="0x1000"
> >> + summary="indicates this is the custom mode"/>
> >
> >Only one custom mode ever possible? Per output?
> [Wang, Quanxian] right.
> >
> >> + <entry name="del" value="0x2000"
> >> + summary="indicates the mode will be deleted"/>
> >
> >How do you know, that a client can handle this flag properly?
> >I mean, that the client does not just ignore an unknown flag and add a mode
> >instead of deleting one.
> [Wang, Quanxian] If we want to add delete action, client should know that and update their process that some modes are deleted. Otherwise it will be in-consistent with compositor.
> Custom flag will tell user, this mode is custom by client instead of from edid or hardware information. Why do I add here? because I am thinking if we should control the delete action, client could only delete the custom mode instead of the modes from hardware or EDID. Currently I will not add the flags in wl_output, because this weston randr will be only for QA and developers, admin to use. Weston randr should be a right place to be put in for special case.
>
> >
> >> + </enum>
> >> +
> >> + <request name="set_mode">
> >> + <description summary="set the mode of output">
> >> + Set the mode of output.
> >> + </description>
> >> + <arg name="output" type="object" interface="wl_output"
> >> + summary="the output object"/>
> >> + <arg name="width" type="int" summary="width of the mode in
> >hardware units"/>
> >> + <arg name="height" type="int" summary="height of the mode in
> >> +hardware units"/>
> >
> >I've come to realize there are two different use cases for "modes":
> >
> >- the usual definition, the video mode for a monitor, which includes
> > all timing details either implicitly or explicitly
> >
> >- the VNC/RDP case, where there is not so much a "mode" but only the
> > size of an output, although in these cases it is better controlled
> > from the remote.
> >
> >For the latter case, I only now realized that it would be silly to have to create a new
> >custom mode for every single output size you want to set, when the output really
> >is just a window on another system. Or is it?
> >
> >I guess there is some ground for having two different paths.
> >
> >OTOH it makes me wonder if a VNC/RDP-backed compositor should ever even
> >expose weston_randr. It quite clearly does not fit the intended use case.
> [Wang, Quanxian] I will expose two mode setting method. One is for mode timing with detailed information.
> Another is the width, height, refresh parameter(first matched, first set).
> >
> >> + <arg name="refresh" type="int" summary="vertical refresh rate in
> >mHz"/>
> >> + </request>
> >> +
> >> + <request name="set_transform">
> >> + <description summary="set the transform of output">
> >> + Set the transform of output, the valuable value should
> >> + be 0-7. This means 0, 90, 180, 270, FLIP-0, FLIP-90,
> >> + FLIP-180, FLIP-270. The values are referred from
> >> + the wl_output specification.
> >> + </description>
> >> + <arg name="output" type="object" interface="wl_output"
> >> + summary="the output object"/>
> >> + <arg name="transform" type="uint"
> >> + summary="the value should be between 0-7"/>
> >> + </request>
> >> +
> >> + <request name="set_scale">
> >> + <description summary="set the scale of output">
> >> + Set the scale of output.
> >> + </description>
> >> + <arg name="output" type="object" interface="wl_output"
> >> + summary="the output object"/>
> >> + <arg name="scale" type="int"
> >> + summary="Scale the output"/>
> >> + </request>
> >> +
> >> + <enum name="move">
> >> + <description summary="move target output to the position of source
> >output">
> >> + The purpose is mainly to allow clients move target output to
> >> + the position of source output.
> >> + </description>
> >> + <entry name="rightof" value="0"/>
> >> + <entry name="leftof" value="1"/>
> >
> >Still missing above/below at least.
> [Wang, Quanxian] Yes. Compositor doesn't support above or below. If we add it, do we add null function for that? When they are ready, we could put detailed content there?
> >
> >> + </enum>
> >> +
> >> + <request name="move">
> >> + <description summary="move target output">
> >> + Move the target output to be leftof/rightof source output.
> >> + </description>
> >> + <arg name="target_output" type="object" interface="wl_output"
> >> + summary="the target output object"/>
> >> + <arg name="source_output" type="object" interface="wl_output"
> >> + summary="the source output object"/>
> >> + <arg name="move" type="int"/>
> >> + </request>
> >> +
> >> + <request name="del_mode">
> >> + <description summary="set the mode of output">
> >> + Set the mode of output.
> >
> >I think you forgot to document this.
> [Wang, Quanxian] Sorry, it is changed now in my private branch. In next version, it will be updated. Sorry about that.
> >
> >> + </description>
> >> + <arg name="output" type="object" interface="wl_output"
> >> + summary="the output object"/>
> >> + <arg name="width" type="int" summary="width of the mode in
> >hardware units"/>
> >> + <arg name="height" type="int" summary="height of the mode in
> >hardware units"/>
> >> + <arg name="refresh" type="int" summary="vertical refresh rate in
> >mHz"/>
> >> + </request>
> >> +
> >> + <request name="new_mode">
> >> + <description summary="new custom mode">
> >> + Set the mode of output.
> >> + </description>
> >> + <arg name="output" type="object" interface="wl_output"
> >> + summary="the output object"/>
> >> + <arg name="fclock" type="int"
> >> + summary="Pixel clock in hardware units Mhz"/>
> >
> >int MHz? hardware units? That does not sound too precise, maybe it should be
> >kHz? That would still go up to 2 THz.
> [Wang, Quanxian] Generally user will use mHz, but in mode timing from hardware, it uses kHz(for example, timing from edid), so we will provide the interface with user custom.
> Before setting and showing them, we need to transfer them. (done in code for that)
> >
> >> + <arg name="hdisplay" type="int"
> >> + summary="The number of pixel for one horticontal line in units
> >Hz"/>
> >> + <arg name="hsync_start" type="int"
> >> + summary="Start of horizontal sync signal in units Hz"/>
> >> + <arg name="hsync_end" type="int"
> >> + summary="End of horizontal sync signal in units Hz"/>
> >> + <arg name="htotal" type="int"
> >> + summary="The whole cycle of a horizontal line in units Hz"/>
> >> + <arg name="vdisplay" type="int"
> >> + summary="The number of pixel for one vertical line in units Hz"/>
> >> + <arg name="vsync_start" type="int"
> >> + summary="Start of vertical sync signal in units Hz"/>
> >> + <arg name="vsync_end" type="int"
> >> + summary="End of vertical sync signal in units Hz"/>
> >> + <arg name="vtotal" type="int"
> >> + summary="The whole cycle of a vorizontal line in units
> >> + Hz"/>
> >
> >Careful with the units there.
> [Wang, Quanxian] I will recheck that.
> >
> >> + <arg name="hsync" type="string"
> >> + summary="the polarity of the horizontal sync pulses"/>
> >> + <arg name="vsync" type="string"
> >> + summary="the polarity of the vertical sync pulses"/>
> >
> >The strings need to be documented.
> >
> >Are you assuming there can be only one custom mode for an output?
> >And if the current mode happens to be custom, is there a way e.g. to read it back,
> >so that if you try another mode, you can return back to it?
> [Wang, Quanxian] Currently it is changed to flags. The flags will contain vsync, hsync, csync, and others. If client input the char parameters, we will translate them into flags and then talk with weston randr.
> >
> >> + </request>
> >> +
> >> + <request name="commit">
> >> + <description summary="multiple operations on one output">
> >> + Commit all the previous output changes from the client.
> >> + Before the commit, the chagne request will be stored in wrandr interface.
> >> + Once randr interface commit, it will update the change and clear up
> >> + the requests from this client.
> >> + </description>
> >> + <arg name="output" type="object" interface="wl_output"
> >> + summary="the output object"/>
> >
> >Here, the same comments apply as to the "start" request. We should not be
> >restricted to just one output, hence the output argument is not needed here.
> [Wang, Quanxian] It has been deleted. But callback done event will have.
> >
> >You could also just remove "start" request completely, and create the reply object
> >here. If we even want an explicit reply object at all. I'm starting to lean on Jasper's
> >side here after all.
> [Wang, Quanxian] yes, done.
> >
> >> + </request>
> >> + </interface>
> >> +
> >> + <interface name="wrandr_callback" version="1">
> >> + <description summary="weston randr callback object">
> >> + Clients can handle the 'done' event to get notified when
> >> + the related request is done.
> >> + </description>
> >> +
> >> + <request name="destroy" type="destructor">
> >> + <description summary="unbind from the interface">
> >> + Informs the server that the client will not be using this
> >> + protocol object anymore. This does not affect any other
> >> + objects, wrandr_callback objects included.
> >
> >Copy-pasta doc.
> [Wang, Quanxian] Reuse the words and make a little change. If you think it is not right here, I will write them again using my words. Anyway, I like official words accepted by developers, it will have less syntax error.:)
> >
> >> + </description>
> >> + </request>
> >> +
> >> + <enum name="action">
> >> + <description summary="flags">
> >> + These flags show the status of every action excuted by randr.
> >> + </description>
> >> + <entry name="mode" value="0"/>
> >> + <entry name="move" value="1"/>
> >> + <entry name="transform" value="2"/>
> >> + <entry name="scale" value="3"/>
> >> + <entry name="newmode" value="4"/>
> >> + <entry name="delmode" value="5"/>
> >> + </enum>
> >> +
> >> + <enum name="result">
> >> + <description summary="action result">
> >> + Action result
> >> + </description>
> >> + <entry name="SUCCESS" value="1"/>
> >> + <entry name="FAIL" value="2"/>
> >> + <entry name="NOACT" value="3"/>
> >> + </enum>
> >> +
> >> + <event name="done">
> >> + <description summary="action done event">
> >> + Notify the randr client that mode setting has been done.
> >> + Flag shows the action executed. every bit stands for one action.
> >> + Result shows the final status for every action, every 2
> >> + bit will show that.
> >> + </description>
> >> + <arg name="flags" type="uint" summary="flag action"/>
> >> + <arg name="result" type="uint" summary="flag result"/>
> >
> >The probably only interesting failure case is the "mode" case. Move, transform,
> >and scale can only come from illegal values in the requests, right? For newmode
> >and delmode I'm not sure they do here, just for illegal values?
> >
> >If illegal values can be known before-hand to be illegal, you could just do a fatal
> >protocol error.
> >
> >Extending this to cover multiple outputs does not seem to work too well, either.
> >For now, I would be satisfied with a simple "succeeded"
> >or "failed" reply. The failure reply might carry a string with an explanation for the
> >user. It is very hard to design what kind of failures would need to be
> >communicated and what the compositor could even detect, so using a free-form
> >string seems the best. Presumably the only thing the client would do with the
> >failure information is to show it to the user.
> [Wang, Quanxian] No, it is for all operations. Flag will contain all the actions you have done from weston-randr. The result will contain all the results of actions you have done.
> Still the same example:
>
> Command: (transform, scale, set mode, move, newmode, ...)
> weston-wrandr --output HDMI3 --transform 1 --scale 2 --mode 2 --rightof VGA1 --newmode='1000,300,100,120,150,400,350,370,399,+hsync -vsync csync +csync -csync'
>
> Results: (it will return the result in one commit, done event is received by user, user could parse the result based on the weston randr protocol)
> output:HDMI3
> TRANSFORM:SUCCESS!
> MODE:SUCCESS!
> SCALE:SUCCESS!
> MOVE:Same as current, no change!
> NEWMODE:SUCCESS!
Well, I guess that could be interesting somehow, but I think even for
the kernel modesetting interfaces, it is not yet clear what should be
communicated on a failure. At least I haven't seen an explicit
discussion about that, although it could also be already in the patches
for all I know. You should prepare for the case, where the platform/OS
APIs do not tell the server in sufficient detail about what failed, and
write guidelines on what should be reported in that case to the Wayland
client. Or at least keep that in mind when developing this. That is the
reason why I suggested just a free-form human readable string as the
"why" argument.
What happens if only some part of the operations fail? Do you then
abort/revert everything, or do you let the succeeded changes stay?
FWIW, I think it should be all or nothing. If even one bit fails, then
the server should revert to the original state as if the requests never
happened, and communicate the failure to the client.
> >
> >> + </event>
> >> + </interface>
> >> +</protocol>
> >
> >I'm still personally not quite sure in which direction to develop this protocol with
> >the custom modes thing.
> [Wang, Quanxian]
> Custom mode, my understanding is that it should be the same format as mode from EDID or hardware information. For example new mode, client should input the right timing parameters to help compositor create a custom mode.
> We have defined that in weston randr protocol. Client should clearly know that what every field of timing means and then custom the mode. After that, the new mode should act as the mode gotten from hardware.
> The mode format will be standard for xserver or others display server. If the format provided by client is not right, we will tell them it is invalid.
>
> For example, the following parameters list. It stands for "clock(mHz), hdisplay, hsync_start, hsync_end, htotal, vdisplay, vsync_start, vsync_end, vtotal, flags string".
> newmode='105,300,100,120,150,400,350,370,399,+hsync -vsync csync +csync -csync'
>
You are right for a real modeline for a real monitor case.
I meant more like the two very different uses cases: arbitrary size on
"virtual monitor" vs. a real modeline on a real monitor. Do we need to
cater for both? Can they both be sufficiently supported by the real
modeline on a real monitor approach?
I would hope to forget the virtual case here.
Thanks,
pq
More information about the wayland-devel
mailing list