Re: Advertising supported Dmabuf modifiers of DRM device

2018-07-26 Thread Daniel Stone
Hi Emre,

On Thu, 26 Jul 2018 at 14:54, Ucan, Emre (ADITG/ESB)
 wrote:
> We are currently querying supported modifiers only from gpu drivers via 
> EGL_EXT_image_dma_buf_modifiers extension.
> But we should also query and advertise modifiers which are supported by DRM 
> device. We are already getting this information via IN_FORMATS of a drm plane.
>
> IMO, it is not enough to just query and advertise every supported format and 
> modifiers combination to applications.
> Applications can easily choose a format, which is only supported by GPU 
> device and not DRM device. In the end, this would force us to use gpu for 
> composition.
> We should mark modifiers of DRM device as preferred ones. This would require 
> modifying linux-dmabuf protocol.

I agree; I've had the same thought. I think we want to do something
more nuanced than this though.

Say for example the display controller only supports linear formats.
Doing a clear intersection would mean all clients rendering to linear
at a performance loss, even if they are not candidates for display
scanout. What we would want is a more dynamic hint from the
compositor: to tell clients to prefer scanout-compatible formats where
they are useful, but else to just pick the best GPU format.

I think the best thing to do would be to make a new dmabuf interface
which takes a surface as a creation parameter, where the compositor
would update the per-surface interface with preferred modifiers. Since
we don't know preference order between modifiers, rather than
enforcing any order, the best thing to do would be to break it into
tranches: send one set of modifiers which are both GPU + scanout
compatible and ask the client to preferentially allocate from that
set, then send a second GPU-only set which the client may fall back to
if it cannot render to the first set.

I don't have plans to work on that right now - I'd happily review any
patches or have a more detailed discussion though! - but as part of
the debug work I sent out last week, I am planning to expose the
current scanout status of surfaces to the core compositor. For
example, 'is on a plane now', vs. 'can never be on a plane because of
compositor configuration (transform/alpha/etc)' vs. 'could maybe be on
a plane but is not allowed (couldn't find suitable plane, kernel
rejected, etc)' vs. 'could be on a plane but client needs to change
(modifiers, transform, etc)'. That should be quite useful for this
work as well.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [RFC libinput] dox: switch to sphinx for the user-visible documentation

2018-07-26 Thread Daniel Stone
Hi,

On Thu, 26 Jul 2018 at 02:53, Peter Hutterer  wrote:
> On Tue, Jul 24, 2018 at 03:42:50PM +1000, Peter Hutterer wrote:
> > Sending this out as patch even though what really matters is the
> > output. Which is... here, tadaaa!
> >
> > https://people.freedesktop.org/~whot/libinput-rtd/
>
> fwiw, I've played around with it a bit and updated the URL above. It now has
> a proper hierarchy, more doxygen tags are being parsed.
>
> There are still a few parsing issues that need to be fixed manually. And the
> doc needs a general do-over to split it better into user docs vs developer
> docs. And of course doxygen itself needs to be separated, generated, and
> link to the sphinx docs. But otherwise this looks quite nice IMO.

Oh, that is really nice. I like that it actually has a navigable
hierarchy now. That was always my biggest issue with Doxygen. Given
that the kernel also uses Sphinx for the DRM/KMS documentation in
particular, it's probably good for us to be using it as well.

If it counts for anything at all:
Acked-by: Daniel Stone 

Cheers,
Daniel

PS: You might want to disable syntax highlighting on the MIT license;
also 'generated by from git commit' at the very bottom.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [Mesa-dev] [PATCH v3] dri3: For 1.2, use root window instead of pixmap drawable

2018-07-26 Thread Daniel Stone
On Thu, 26 Jul 2018 at 08:46, Olivier Fourdan  wrote:
> get_supported_modifiers() and pixmap_from_buffers() requests both
> expect a window as drawable, passing a pixmap will fail as the Xserver
> will fail to match the given drawable to a window.
>
> That leads to dri3_alloc_render_buffer() to return NULL and breaks
> rendering when using GLX_DOUBLEBUFFER on pixmaps.
>
> Query the root window of the pixmap on first init, and use the root
> window instead of the pixmap drawable for get_supported_modifiers()
> and pixmap_from_buffers().

Looks good to me, thanks Olivier!

Reviewed-by: Daniel Stone 

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [RFC weston] clients: Don't attach a buffer if mouse cursor surface is unchanged

2018-07-26 Thread Daniel Stone
Hi Jasper,

On Thu, 26 Jul 2018 at 03:53, Jasper St. Pierre  wrote:
> From IRC conversations with krh a long time ago, this is indeed intentional 
> and the cursor surface should "lose its role" in modern parlance.
>
> The original intention was to prevent glitching of the cursor surface. e.g. 
> If the left side of the surface has a resize left cursor, you leave, and 
> hover over the right, you don't want to see the resize left cursor for a 
> split second before the resize right cursor appears.
>
> The original implementation of Weston respected this and would only change 
> the cursor on set_cursor calls and would not even remember a per-client 
> cursor surface. This behavior has probably been lost in numerous reactors by 
> now.

Right, it's not about having the cursor surface stick to the normal
surface, but about having the buffer stick to the cursor surface. The
former definitely isn't controversial, and EFL is re-associating the
cursor surface with the normal surface. What's happening is that they
need to attach/damage/commit to that surface to get the content.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [RFC weston] clients: Don't attach a buffer if mouse cursor surface is unchanged

2018-07-24 Thread Daniel Stone
Hey,

On Tue, 24 Jul 2018 at 22:45, Derek Foreman
 wrote:
> On 2018-07-22 05:55 AM, Daniel Stone wrote:
> > I take it the problem is that the client sets a particular surface as
> > the pointer surface, loses focus, sets the same surface as the pointer
> > surface on re-enter after not changing the content, and then the
> > content is never shown?
>
> That's my understanding of what I'm seeing, yes.
>
> Note that it only happens when the cursor can be placed in the cursor
> plane (ie: it's wayland_shm).
>
> The old cursor continues to be shown - if I move into an EFL client from
> the desktop, the desktop cursor arrow is sometimes unchanged.
>
> I do get a surface enter for my pointer surface, though.
>
> I'm reasonably confident the first time I saw this I would get no cursor
> at all on re-enter, but now I get the existing cursor.  Though sometimes
> it updates to my client cursor, despite no damage or new buffer attach
> on the surface.

To be fair, this has changed a fair bit with atomic, so I'm not
surprised it's different in master. But I am a bit disappointed it's
not perfect yet. ;)

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [RFC libinput] dox: switch to sphinx for the user-visible documentation

2018-07-24 Thread Daniel Stone
Hey,

On Tue, 24 Jul 2018 at 06:43, Peter Hutterer  wrote:
> Sending this out as patch even though what really matters is the
> output. Which is... here, tadaaa!
>
> https://people.freedesktop.org/~whot/libinput-rtd/
>
> Basically the motivation here is to make the user-visible documentation less
> awful, especially because these days, 90% of the doc needs are by end users,
> not the developers. The API itself is just fine in doxygen, but for prose
> doxygen's "Related Pages" features is not perfect.
>
> So I'm wondering if using RTD-style documentation is a better option here.
> This patch is a more-or-less 1:1 conversion of the hand-written
> documentation to use sphinx with the RTD theme. For a good chuckle, look at
> the awk/sed script. (that script would go away anway after the one-time
> conversion to an .rst source format).

Makes sense, and this does look really good, but ... did you remember
to run the script? The output at the URL above is full of @ref, @dot,
etc. The rest looks good though. :) I'm all for something like Sphinx
- not because I've ever used it or know any of its pros and cons, but
it seems like a more stable tooling which is also more widely used.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [RFC weston] clients: Don't attach a buffer if mouse cursor surface is unchanged

2018-07-22 Thread Daniel Stone
Hey Derek,

On Thu, 22 Feb 2018 at 22:16, Derek Foreman  wrote:
> Keep track of what cusor image buffer is attached to the cursor
> surface and avoid re-attaching it if we don't have to.
>
> This isn't just an obviously pointless optimization, it turns all
> of toy toolkit into a test case for handling this properly.
>
> Signed-off-by: Derek Foreman 
> ---
>
> Continuing my streak of posting unpopular patches, this patch breaks
> weston, so I'm not pushing for inclusion, but I think we need to
> resolve why it breaks, and fix either weston or wayland documentation
> to reflect expected behaviour.
>
> I think this can be attributed to a weston bug, and we should be able to
> expect that the compositor will be able to redisplay the surface without
> needing to attach a new buffer, and that if the compositor has released
> the buffer then it has a kept copy somewhere...
>
> Any other opinions?

Hmm, I'm really not sure what to think about this.

I take it the problem is that the client sets a particular surface as
the pointer surface, loses focus, sets the same surface as the pointer
surface on re-enter after not changing the content, and then the
content is never shown?

If so, the two solutions would be:
  - the surface content should be preserved across leave/enter; Weston
is buggy for not showing the content on re-enter
  - mouse leave and your surface no longer being shown as a pointer
surface causes the surface to lose that role; Wayland protocol spec
should explicitly state this

I have a very slight lean towards the latter because it more closely
matches my recollection of what the XDG roles do. But that
recollection might be totally wrong, who knows.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] shared/config-parser: Allow spaces/tabs at the end for

2018-07-22 Thread Daniel Stone
Hi Marius,
On Wed, 21 Feb 2018 at 14:38, Marius Vlad  wrote:
> @@ -456,6 +457,10 @@ weston_config_parse(const char *name)
> section = config_add_section(config, [1]);
> continue;
> default:
> +   start_line = stop_line = line;
> +   while (stop_line && *stop_line != ' ' && *stop_line 
> != '\t')

Should this be *stop_line? The pointer itself will never be NULL, so
it seems like this might read a long way out of bounds.

> @@ -474,7 +479,9 @@ weston_config_parse(const char *name)
> p[i - 1] = '\0';
> i--;
> }
> -   section_add_entry(section, line, p);
> +
> +   start_line[stop_line - start_line] = '\0';

I think you could remove the start_line variable and continue to just
use 'line' here, if you simplified this to *stop_line = '\0';

This should also have a few tests in config-parser-test.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] xwayland/selection: do not remove NULL property_source

2018-07-22 Thread Daniel Stone
Hi Greg,

On Tue, 10 Apr 2018 at 18:01, Greg V  wrote:
> Happened mostly with neovim's xclip usage.

Thanks, I've merged this, updated to include more cases where it could be NULL:
   67546bed0..3ea5437db  3ea5437dbd07d9a94aebbb651d8f8eeacc8765bd -> master

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [prefix=PATCH weston] gl-renderer.c: Pass visual ID for choosing egl configs for pbuffer

2018-07-22 Thread Daniel Stone
Hi Madhurkiran,

On Wed, 21 Mar 2018 at 22:45, Madhurkiran Harikrishnan
 wrote:
> +   if (pbuffer_config != gr->egl_config &&
> +   !gr->has_configless_context) {
> +   weston_log("attempted to use a different EGL config for an "
> +  "output but EGL_KHR_no_config_context or "
> +  "EGL_MESA_configless_context is not supported\n");
> +   return -1;
> +   }

If we must ensure that the config is exactly identical (not having
configless_context), then we should just use gr->egl_config directly,
but ensure that it has EGL_WINDOW_BIT and EGL_PBUFFER_BIT set when it
is selected for the main output surface. Could you please rework the
patch so it does this? The rest seems fine to me though.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v2] ivi-shell: use install paths in example config

2018-07-22 Thread Daniel Stone
Hi all,

On Wed, 27 Jun 2018 at 13:50, Emil Velikov  wrote:
> On 27 June 2018 at 12:12, Emil Velikov  wrote:
> > On 27 June 2018 at 12:01, Michael Tretter  wrote:
> >> On Fri, 25 May 2018 08:46:16 +0200, Michael Tretter wrote:
> >>> On Thu, 24 May 2018 17:08:47 +0200, Emre Ucan wrote:
> >>> > The example weston.ini file uses source and build
> >>> > directory paths. Therefore, it is only useful when
> >>> > used on the same system that is used to build Weston.
> >>> >
> >>> > We can use install paths instead of build/source paths
> >>> > to fix this problem.
> >>> >
> >>> > v2 changes:
> >>> > - use $(westondatadir) instead of $(datadir)
> >>> >
> >>> > Reported-by: Michael Tretter 
> >>> > Signed-off-by: Emre Ucan 
> >>>
> >>> Reviewed-by: Michael Tretter 
> >>
> >> Some variables that this patch touches have already been removed, but I
> >> think it would still make change to rename 'abs_top_builddir' and
> >> 'abs_top_srcdir'. Is there anything preventing this patch from being
> >> applied?
> >
> > Is this still required with the recently landed builddir changes by 
> > Dan/Pekka?
> > I'll give it a look after lunch and can offer some feedback, after
> > lunch. Gut feeling atm is that this breaks make check.
>
> My gut feeling and coffee deprived brain were miles off.
> Patch is spot on and is
> Reviewed-by: Emil Velikov 

Thanks for the patch and all the review; I've added mine and pushed:
   cf4113c62..67546bed0  67546bed04a464d18508265976f2a4b673db565c -> master

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] ivi-shell: listen compositor wake_signal

2018-07-22 Thread Daniel Stone
Hi Emre,

On Tue, 5 Jun 2018 at 09:30, Emre Ucan  wrote:
> If compositor wakes up from sleep state, we have
> to trigger repaint for all outputs.

Seems trivially correct, merged with review:
   280b73014..cf4113c62  cf4113c629751adcbb9904087932e12b5621219b -> master

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 0/2] Fix autotools doc build with Meson

2018-07-22 Thread Daniel Stone
Hi,
The horribly ugly second patch fixes the autotools doc build, which
started failing after adding Meson support. The presence of
$(srcdir)/doc/doxygen/xml, created to hold meson.build, caused autotools
to believe it no longer needed to create $(builddir)/doc/doxygen/xml,
then immediately fail when it tried to generate files into a directory
which doesn't exist.

We work around this by explicitly naming the absolute path of the build
directory in the target for mkdir, so autotools will no longer try to
consult VPATH for it.

The first patch simply removes the orphan pre-generated PNG files, which
were not referenced anywhere in the build, and did not get distributed
into tarballs.

Cheers,
Daniel


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 1/2] spec: Delete old unused directory

2018-07-22 Thread Daniel Stone
The protocol spec used to live here, but it's now part of the regular
doc build. The PNG files are created as part of the doc build. Delete
the pre-generated versions.

Signed-off-by: Daniel Stone 
---
 spec/.gitignore   |   3 ---
 spec/wayland-architecture.png | Bin 29162 -> 0 bytes
 spec/x-architecture.png   | Bin 37306 -> 0 bytes
 3 files changed, 3 deletions(-)
 delete mode 100644 spec/.gitignore
 delete mode 100644 spec/wayland-architecture.png
 delete mode 100644 spec/x-architecture.png

diff --git a/spec/.gitignore b/spec/.gitignore
deleted file mode 100644
index 4962683d..
--- a/spec/.gitignore
+++ /dev/null
@@ -1,3 +0,0 @@
-main.aux
-main.log
-main.pdf
diff --git a/spec/wayland-architecture.png b/spec/wayland-architecture.png
deleted file mode 100644
index 
4f92e0fb93825a68095b75d499bac00e968419e5..
GIT binary patch
literal 0
HcmV?d1

literal 29162
zcma%iWmJ@3)GrOfPy#Y^cZ1SMN(+c|BOu*f14DyIhe(`Iq@$oHB{eii$H36<
z0zBY}RwC~L
z@DImVNyC5u_zNSjPXN9@@KQJR#lU#z_wR$b^qMIMcu3>-%*5}d=WD+}TOS9Ez`#I3
zXAf6jJ6kUYK~EpY`~z893=C!r4HZR$pn|=^z;_f=7xBwPT8F*HFQluOd09({$_R{9
znoGEO(S~H#v)2i@d?@R6K-66?IWqypNJ76+;et3HE#z&~9e3a2u77gLsho}9g>4D_qd;6Dt;chpO#|}YVs4_DU6s1S(`L17Os+MI
zt144^1Ort@);fRXe-ed7$q+n^Hb=+SpS<-@s?$Mw)9DA;#
z$48tyS57@|MLLuZR_gVlrXn4vsQLv!t6oIODXur}9(=rBhhki2l}q8!N1t(Ao>L
zOU6U#G-is2=E;=pDGAXoankrSXHNzkh3!A$2*-g?_C3TRDyljYLds7tA-G?MzA#6#
zB!bnuATmp;-~qO7v_lnP!bsq&9*`(FBSE`_Zzn_*cx9dG-s8|I`G^Kont`XytZbOt
z>+5qAhfK7c_Q8XU;xAUfMo`N{q7da;qP$mwJ8Q5$t7QRrsH?#8kBAenT^
z)(k#d7@l%R1?I%yJc06^*Wh7X5Us5+C5GU+8=)}!gIHNbLTO;}(JW&;SOrX;Y@pJY
z1c~FXqA0WKj?DV38wgwELrcPQ5f(b*8ZReJa16fZje-Qmj(?ZRMldJ9u
zWuM}t`5b7SW-xr`EV@i{B$@{`4N*1T{GqeSLlm
z$%QP9ivj5};xwR@)h_g+C@9A>5+O6dpg6diy5W_|MQG-+@u(q6u@P}9LaO{W$-_Kf
z0x|uCq8wTAXN(JCIaV)^?Y9V8Z+B-HH_JaM|=
zpb^oPlTW@$%M|4$roDsYu*K_VzY#f68AGe?Z&`9W1R>8RZ8nu8!yb23Jul*OV7~Bq
zd6(v6h!{~r>*tYvgd-(vmRSy6QzUPC%@`>lQJntB-cV%S=5e!_enZeh(aruh?E{O(
zgY?{N9!~((Q2)uqgT5Hy`yr*pG{fQI9qo94{cgA7$;@D+vx}QIGWZ9)3OjV`#Je
z{YQhy<1_wMv}0kI#}`icP4*FD^~YeNofi6-<8pyGE@u-+wFQAB9RAW10
zgAbx6Hso8-3l`sx#J(Q`8~p0>nw!((hYN0`epG(_sG^wAzXqp0{QMO<5c_czW}#VZ
zK=1rHJPt?f`q3d?Fee-6x&4a?xh=Bo9hWe0h*)DU+yg)GS2NT+jc|4R1n
zSzo)S;013QH3KhcIJ3O$hGu{Kg<`B|KjH%wUM*?N`wPgTINFvmONcnPfT|;TdXm_(
z>EWY#av3v;nA3S|WNon~(2a!yi$kQv)Oo#mhP8-8)O_??I`M?4?W!Vra?
zUwqmfsaVeqUWdxGpC=W|i-mW!Fvww1R27`QXZwJWAE1;8nS4&7Qf?-^+E7rmx+VXm
zVor4QZny-#^_n66sqUAn{iA-l(!}N6OnZyHpfsU~L!F~dn<@kGtx#<1N8b0SCqa8`
zG`vR})u|4kzeDEOGjbcG9Va}qp~Ba@0)Mq;MCtEL4^ab$`vObB-0M%ScaKJVwlAW>
zilBp@zjQhvRS6mLRA1rqfnViNAD4-I+-7i!u0g80h`T$A@|EMhqiz4Cna#$tB79LI
zxq!Hf!#{?}Xez%lPKYOjV=4bWs7UPR{I_wRrxuc!JbUJ%`+!pFNo_;PIjAlO{
zUrv(bGsDoi@+5q!4i_eP=ak3SLDD^5u1M3~G2dtCuh~(26A$*K1NU=PAGSei+bNTenAswbPMM}+s|N=X#h;zer7tl!_L&-PL4^gT-v`1?h!x71Mi68cV($SR-v
zm357zMNkM*`U=LrwDjv1dG@nELZ+K%Nh@v=chPok}
zQ*?U%MJC8#N>E|r=$>yAu+x(%9$GU(_h}pSM|M0boQQUJo-#fcD7lK~tf#3^`
z+%ZP%iRIOBm@C~ep|M-c1SEzITg5VTi%!aF=E)fcX|xdH;JM^yPr)^Z{X6{fj|l2c
zhiJI}4XQD@@Ukz$%3RXRM0PrNc#9Giul7~0>oaz8gaL%C;M!|^%?k~ur86}0nl)_k`V`NaKU`_y%W7@R8^O>QTXrsu30X^up!cJxKxbQ}=mPzz=^|
zP$pmn-o>w@T=G!@@Ie7Rw+<4IQO653{GX`z#GdeqryN)8CLve?GhO4Rp)
zmwxaMu_ocAJq?c5py%f^Ta8)XVwLtSrA^CRChfcn-p=ZqTQ(zBQbvtr<`a$^oIw#S
zvJsBz$zI~)##*KReTcLvMJ-KneIbCFAvEE)wt%au`6_MUBn$wMK
zzOp`g1ZA^DHqfAak%1iXCa4{qzpK3*x(>Z136|UKnAsgooxvBK6Gjt*Pouvn
zZH+L-OB-Utth!?8Q(a974ic8XKKH3Jl?p2KkN%xUoh>J$D83(apAy#>!=axO+%;b-
zu6#_SLhz)hd}~D}?(%RLj>75A9XC0Ferd>ac$HP2Y30v>-!bTPfwI;%Fv7Ymhz
z*)3!Sv>{PoMLy!Gb+L0%4LNVk23<*tP}
zKWZT4SE7KN2r_tQeli;c@7yULEd@dmpnNjOt?otUbx)}L!r7}M+zsXn5A@d2^vp^-
z{*QBZ$>yA7m~?28-81(I(wQFN2w~9JKfw%K@Z40|F=}(o_*9_p6`kH|6a=R^czaH`9FH9YPFX0Ps1M*Hn|-~uV~
zj1zjX6qS4}^$G`?E86oILmHHWih6!ErcYX_LQrcMsmqwfz!)r2!qp@FqR3c~%X{kt
zk=QCbztKl|9BNyT%+)Z5Wge(ih^j@fI<(USrE8NsM{6jF8a8rksXS`Pa1gO$f-dQX
ztVOSOSpsKW;sC$y`>QEl`(dY-eLT;C`*73Wqc&6NzkH5>Pq!~$Wg|0EC
zdv^jJ=UJ_T_E|xugGk_n*wF6yS3>!)-;qTRxsh*$R_2%2{{$RTo9oo!KHpVbZfHRa
zM`<tSDaJXgeR=Wz!WuVbb`O++FX>;pVg}-1#^v2y)a&@eCyk%jvdn
zM&46%yy;(NtJv*xYSm_Eki#?NTMthKjlG@x<$WCZIKXM*s@<#XP;i;Khz;ICx`CkQ
zxo4oVJ)*8LIa+=}ex!#Dia{(z`@pc>1>WF|wAOv+qf^X!FX<(+b5+~(hGz9Si;ZK?
zTLS#b0aBT}QFZc(gZ%~ar)l1`K?KFW0zSu*+Lv(~4dAc7;LJNx%zzZ@r$4qK;z{%H
zyl0sW$LK@T42N<)VBDs#}5>;uNXw)Y`piRWvln%KXrWI=cT^TSmiopB;7<;&*rpWR5ar<0D2r0>Tv-
zG{@p_GU?dJ!$>>e5@iDz+m4wbGA+XNh6@a_HTQ)6v~_yyzSMT|xF6na?crcuHc%WE
zJh#G?T4IT&>@6F_E)qlPHwaIez#fZq=-;i2>{x4N1Zv@vOcQ3>gLikqYvH=Z_~K8R
zXpdT#6(exxx>R;vwp^w8OUGQCzMl~v9?!Zga`e41!+bhXTEDk!{Su|y<)Ka!fF+O#EvBEU@SGq(!4~I5^a<_MHC8upKBSA1Oi=ATIAx21QOc1gYBH=}@SAyqp
z!4y?nBrx97O2t;Q)e(n{iIv+Ra(`9*m7!u6;C6tFQ7Ss`!)`>R7F158ol|oGCGIwl
zV#*Itc?AEE7Bx-$36^7I(lS7!pYl{Dy^mPiGz
zv?y12*zx(GOBK7YRrdrIAD@X8VE;23gmAmCPG7xB
zf3tA+-WZJ`zW<{kRzaw)Cvk`vx2OM1AY8~kOLQ~D50(%M
zO+c0B9dN*;ayTrlm7%XijwHRXcCH*Knk#L{uVy4HU6B+>3Q>dL5}8%_P5kI
z

[PATCH wayland v3 2/4] Add Meson build to Wayland

2018-07-21 Thread Daniel Stone
From: Emmanuele Bassi 

Meson is a next generation build system, simpler than Autotools and,
more importantly, faster and more portable. While the latter
consideration is of lesser importance for Wayland, being easier to
understand and faster are pretty much key reasons to switch.

This is mostly a mechanical port of Wayland to the Meson build system.

The goal is to maintain feature parity of the Meson build with the
Autotools build, until such time when we can drop the latter.

[daniels: Changed to bump version, use GitLab issues URL, remove header
  checks not used in any code, remove pre-pkg-config Expat
  support, added missing include paths to wayland-egl and
  cpp-compile-test, added GitLab CI.]

Reviewed-by: Daniel Stone 
---
 .gitlab-ci.yml |  24 +-
 cursor/meson.build |  30 +++
 doc/meson.build|   5 ++
 egl/meson.build|  45 ++
 meson.build|  94 +
 meson_options.txt  |  20 +
 src/meson.build| 203 +
 tests/meson.build  | 180 
 8 files changed, 599 insertions(+), 2 deletions(-)
 create mode 100644 cursor/meson.build
 create mode 100644 doc/meson.build
 create mode 100644 egl/meson.build
 create mode 100644 meson.build
 create mode 100644 meson_options.txt
 create mode 100644 src/meson.build
 create mode 100644 tests/meson.build

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 24896659..1d0018b1 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -9,10 +9,11 @@ before_script:
   - echo '#!/bin/sh' > /usr/sbin/policy-rc.d
   - echo 'exit 101' >> /usr/sbin/policy-rc.d
   - chmod +x /usr/sbin/policy-rc.d
+  - echo 'deb http://deb.debian.org/debian stretch-backports main' >> 
/etc/apt/sources.list
   - apt-get update
-  - apt-get -y --no-install-recommends install build-essential automake 
autoconf libtool pkg-config libexpat1-dev libffi-dev libxml2-dev doxygen 
graphviz xmlto xsltproc docbook-xsl
+  - apt-get -y --no-install-recommends install build-essential automake 
autoconf libtool pkg-config libexpat1-dev libffi-dev libxml2-dev doxygen 
graphviz xmlto xsltproc docbook-xsl meson/stretch-backports
 
-build-native:
+build-native-autotools:
   stage: build
   script:
   - export BUILD_ID="wayland-$CI_JOB_NAME_$CI_COMMIT_SHA-$CI_JOB_ID"
@@ -34,3 +35,22 @@ build-native:
 - build-*/wayland*/_build/sub/*.log
 - build-*/*.log
 - prefix-*
+
+build-native-meson:
+  stage: build
+  script:
+  - export BUILD_ID="wayland-$CI_JOB_NAME_$CI_COMMIT_SHA-$CI_JOB_ID"
+  - export PREFIX="$(pwd)/prefix-$BUILD_ID"
+  - export BUILDDIR="$(pwd)/build-$BUILD_ID"
+  - export MAKEFLAGS="-j4"
+  - mkdir "$BUILDDIR" "$PREFIX"
+  - cd "$BUILDDIR"
+  - meson --prefix="$PREFIX" -Dicon_directory=/usr/share/X11/icons ..
+  - ninja -k0 test
+  - ninja clean
+  artifacts:
+name: wayland-meson-$CI_COMMIT_SHA-$CI_JOB_ID
+when: always
+paths:
+- build-meson/meson-logs
+- prefix-*
diff --git a/cursor/meson.build b/cursor/meson.build
new file mode 100644
index ..c8ed1aab
--- /dev/null
+++ b/cursor/meson.build
@@ -0,0 +1,30 @@
+icondir = get_option('icon_directory')
+if icondir == ''
+  icondir = join_paths(get_option('prefix'), get_option('datadir'), 'icons')
+endif
+
+wayland_cursor = library(
+  'wayland-cursor',
+  sources: [
+'wayland-cursor.c',
+'os-compatibility.c',
+'xcursor.c',
+  ],
+  version: '0.0.0',
+  dependencies: [ wayland_client_dep ],
+  c_args: [
+'-DICONDIR="@0@"'.format(icondir),
+  ],
+  install: true,
+)
+
+install_headers('wayland-cursor.h')
+
+pkgconfig.generate(
+  name: 'Wayland Cursor',
+  description: 'Wayland cursor helper library',
+  version: meson.project_version(),
+  libraries: wayland_cursor,
+  filebase: 'wayland-cursor',
+  install_dir: join_paths(get_option('prefix'), get_option('libdir'), 
'pkgconfig'),
+)
diff --git a/doc/meson.build b/doc/meson.build
new file mode 100644
index ..8c743fe9
--- /dev/null
+++ b/doc/meson.build
@@ -0,0 +1,5 @@
+xsltproc = find_program('xsltproc', required: false)
+doxygen = find_program('doxygen')
+xmlto = find_program('xmlto')
+dot = find_program('dot')
+
diff --git a/egl/meson.build b/egl/meson.build
new file mode 100644
index ..8d488c36
--- /dev/null
+++ b/egl/meson.build
@@ -0,0 +1,45 @@
+wayland_egl = library(
+  'wayland-egl',
+  sources: [
+'wayland-egl.c',
+get_variable('wayland_client_protocol_h'),
+  ],
+  include_directories: src_inc,
+  version: '1.0.0',
+  install: true,
+)
+
+executable('wayland-egl-abi-check', 'wayland-egl-abi-check.c')
+
+nm_path = find_program('nm').path()
+
+test(
+  'wayland-egl symbols check',
+  find_program('wayland-egl-symbols-check'),
+  env: [
+'WAYLAND_EGL_LIB=@0@'.format(wayland_egl.full_path()),
+'NM=@0@'.format(nm_path),
+  ],
+)
+
+install_headers([
+ 

[PATCH wayland v3 1/4] Support running tests from different build directories

2018-07-21 Thread Daniel Stone
From: Emmanuele Bassi 

The tests that run exec-fd-leak-checker expect the binary to be located
in the current directory. This is not always the case; for instance, the
binaries could be built under `tests`, but be invoked under the
top-level build directory.

We can use an environment variable to control what's the location of the
test binaries, and fall back to the current directory if the variable is
unset.

Reviewed-by: Daniel Stone 
---
 tests/test-helpers.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tests/test-helpers.c b/tests/test-helpers.c
index b2189d8e..8ad332b1 100644
--- a/tests/test-helpers.c
+++ b/tests/test-helpers.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -67,11 +68,19 @@ count_open_fds(void)
 void
 exec_fd_leak_check(int nr_expected_fds)
 {
-   const char *exe = "./exec-fd-leak-checker";
+   const char *exe = "exec-fd-leak-checker";
char number[16] = { 0 };
+const char *test_build_dir = getenv("TEST_BUILD_DIR");
+char exe_path[256] = { 0 };
+
+if (test_build_dir == NULL || test_build_dir[0] == 0) {
+test_build_dir = ".";
+}
+
+snprintf(exe_path, sizeof exe_path - 1, "%s/%s", test_build_dir, exe);
 
snprintf(number, sizeof number - 1, "%d", nr_expected_fds);
-   execl(exe, exe, number, (char *)NULL);
+   execl(exe_path, exe, number, (char *)NULL);
assert(0 && "execing fd leak checker failed");
 }
 
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland v3 0/4] Meson build

2018-07-21 Thread Daniel Stone
Hi,
This is an updated version of Emmanuele's series. Notably it fixes up
the Doxygen man build, so we now correctly build and install all the man
pages.

I think this should be good to merge at this point. It is past the alpha
freeze, however getting it in a release would be a great way to get
testing from both downstreams and users, who wouldn't necessarily be
testing git. As it's additive, it shouldn't regress anything for anyone,
and the autotools build still exists and works.

Cheers,
Daniel


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland v3 4/4] meson: Generate man pages

2018-07-21 Thread Daniel Stone
From: Emmanuele Bassi 

There are ancillary man pages not built by Doxygen that we need to
generate an install.
---
 doc/man/meson.build | 64 +
 doc/meson.build |  1 +
 2 files changed, 65 insertions(+)
 create mode 100644 doc/man/meson.build

diff --git a/doc/man/meson.build b/doc/man/meson.build
new file mode 100644
index ..211dc203
--- /dev/null
+++ b/doc/man/meson.build
@@ -0,0 +1,64 @@
+xsltproc = find_program('xsltproc', required: false)
+
+manpage_xsl = 
'http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl'
+man_pages = []
+
+if xsltproc.found()
+  cmd = run_command([xsltproc, '--nonet', manpage_xsl])
+  have_manpage_xsl = cmd.returncode() == 0
+else
+  warning('No docbook stylesheet found for generating man pages')
+  have_manpage_xsl = false
+endif
+
+if have_manpage_xsl
+  # man section, [ [ xml_file, name, alias ] ]
+  man_pages += [
+[
+  '3', [
+[ 'wl_display_connect.xml', 'wl_display_connect', 
'wl_display_connect_to_fd' ],
+  ],
+]
+  ]
+endif
+
+xsltproc_opts = [
+  '--nonet',
+  '--stringparam', 'man.authors.section.enabled', '0',
+  '--stringparam', 'man.copyright.section.enabled', '0',
+  '--stringparam', 'funcsynopsis.style', 'ansi',
+  '--stringparam', 'man.output.quietly', '1',
+]
+
+foreach section: man_pages
+  section_number = section[0]
+  pages = section.get(1, [])
+
+  foreach page: pages
+xml_input = page[0]
+name = page[1]
+alias = page.get(2, '')
+
+man_output = name + '.' + section_number
+if alias != ''
+  alias_output = alias + '.' + section_number
+else
+  alias_output = []
+endif
+
+man_page = custom_target(name + '-man',
+  command: [
+xsltproc,
+xsltproc_opts,
+'-o', '@OUTPUT0@',
+manpage_xsl,
+'@INPUT@',
+  ],
+  input: xml_input,
+  output: [ man_output, alias_output ],
+  install: true,
+  install_dir: join_paths(get_option('prefix'), get_option('mandir'), 
'man' + section_number),
+  build_by_default: true,
+)
+  endforeach
+endforeach
diff --git a/doc/meson.build b/doc/meson.build
index f05c57b3..a1928800 100644
--- a/doc/meson.build
+++ b/doc/meson.build
@@ -1 +1,2 @@
 subdir('doxygen')
+subdir('man')
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v3 weston 1/5] compositor-drm: Implement support for GAMMA_LUT drm property

2018-07-21 Thread Daniel Stone
Hi Harsha,
Some small comments here, but more substantial comments in another patch.

On Thu, 28 Jun 2018 at 14:28,  wrote:
> +   gamma_prop_id = output->props_crtc[WDRM_CRTC_GAMMA_LUT].prop_id;
> +   gamma_size_prop_id = output->props_crtc[WDRM_CRTC_GAMMA_LUT_SIZE].
> +   prop_id;
> +
> +   if (gamma_prop_id && gamma_size_prop_id) {
> +   lut = malloc(sizeof(struct drm_color_lut) * size);
> +   if (!lut) {
> +   weston_log("failed to allocate memory for gamma 
> lut\n");
> +   goto out;
> +   }
> +
> +   for (loop = 0; loop < size; loop++) {
> +   lut[loop].red = r[loop];
> +   lut[loop].green = g[loop];
> +   lut[loop].blue = b[loop];
> +   }

Since this is the new interface, it would be nice to switch Weston's
internals to use the same structure internally; this would mean a copy
when using the old drmModeSetGamma interface, and no copies when using
the property interface.

> @@ -4758,15 +4809,35 @@ drm_output_init_gamma_size(struct drm_output *output)
> +   props_crtc = >props_crtc[0];

This should just be output->props_crtc, but per below, can be
completely eliminated.

> +   gamma_prop_id = props_crtc[WDRM_CRTC_GAMMA_LUT].prop_id;
> +   gamma_size_prop_id = props_crtc[WDRM_CRTC_GAMMA_LUT_SIZE].prop_id;
> +
> +   if (gamma_prop_id && gamma_size_prop_id) {
> +   props  = drmModeObjectGetProperties(backend->drm.fd,
> +  output->crtc_id,
> +  DRM_MODE_OBJECT_CRTC);
> +   if (props) {
> +
> +   output->base.gamma_size = drm_property_get_value(
> +  _crtc[WDRM_CRTC_GAMMA_LUT_SIZE],
> +  props,
> +  WDRM_CRTC__COUNT);

The last argument for this function is a default value. You could
simply rewrite this to unconditionally get the CRTC properties and
pass the gamma size from drmModeCrtc as the default, which avoids both
branches and all the temporary variables.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 1/2] build: Remove execinfo.h check

2018-07-21 Thread Daniel Stone
The check for the execinfo.h header is only advisory; the build will not
fail if it is not present, and set HAVE_EXECINFO_H if it is. The check
was added in commit 5cfdbef3d299 ("build: Allow disabling building of
wayland libraries") with no obvious use or reasoning.

Remove the no-op check.

Signed-off-by: Daniel Stone 
---
 configure.ac | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 81cf4077..8c2fb822 100644
--- a/configure.ac
+++ b/configure.ac
@@ -109,7 +109,6 @@ if test "x$enable_libraries" = "xyes"; then
AC_CHECK_DECL(CLOCK_MONOTONIC,[],
  [AC_MSG_ERROR("CLOCK_MONOTONIC is needed to compile 
wayland libraries")],
  [[#include ]])
-   AC_CHECK_HEADERS([execinfo.h])
 fi
 
 PKG_CHECK_MODULES(EXPAT, [expat], [],
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 2/2] build: Remove support for non-pkg-config Expat

2018-07-21 Thread Daniel Stone
The Expat XML library has shipped a pkg-config file for long enough to
be in Debian's oldstable (Jessie, April 2015) and Ubuntu's oldest
supported LTS (Trusty, 14.04). The pkg-config file was added in Expat
upstream's commit 352cfc8f59a7, in September 2007.

Drop build support for versions of Expat which do not ship a
pkg-config file.

Signed-off-by: Daniel Stone 
---
 configure.ac | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index 8c2fb822..0022dcda 100644
--- a/configure.ac
+++ b/configure.ac
@@ -111,16 +111,7 @@ if test "x$enable_libraries" = "xyes"; then
  [[#include ]])
 fi
 
-PKG_CHECK_MODULES(EXPAT, [expat], [],
-   [AC_CHECK_HEADERS(expat.h, [],
-   [AC_MSG_ERROR([Can't find expat.h. Please install expat.])])
-SAVE_LIBS="$LIBS"
-AC_SEARCH_LIBS(XML_ParserCreate, expat, [],
-   [AC_MSG_ERROR([Can't find expat library. Please install 
expat.])])
-EXPAT_LIBS="$LIBS"
-LIBS="$SAVE_LIBS"
-AC_SUBST(EXPAT_LIBS)
-   ])
+PKG_CHECK_MODULES(EXPAT, [expat])
 
 AM_CONDITIONAL([DTD_VALIDATION], [test "x$enable_dtd_validation" = "xyes"])
 if test "x$enable_dtd_validation" = "xyes"; then
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 0/2] Build system detritus removal

2018-07-21 Thread Daniel Stone
Hi,
Whilst going over Emmanuele's Meson series, I noticed a couple of
artifacts in the autotools build which could probably disappear; one
no-op header check, and one check for incredibly ancient Expat versions.

Cheers,
Daniel


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v5 13/14] compositor-drm: Calculate atomic-commit flags earlier

2018-07-20 Thread Daniel Stone
Shift up our calculation of the flags we use for atomic commits. We will
later use this to differentiate between test-only and full commits when
printing debug information inside drm_output_state_apply_atomic.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index e27671437..653d13e0c 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -2499,12 +2499,24 @@ drm_pending_state_apply_atomic(struct drm_pending_state 
*pending_state,
struct drm_output_state *output_state, *tmp;
struct drm_plane *plane;
drmModeAtomicReq *req = drmModeAtomicAlloc();
-   uint32_t flags = 0;
+   uint32_t flags;
int ret = 0;
 
if (!req)
return -1;
 
+   switch (mode) {
+   case DRM_STATE_APPLY_SYNC:
+   flags = 0;
+   break;
+   case DRM_STATE_APPLY_ASYNC:
+   flags = DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
+   break;
+   case DRM_STATE_TEST_ONLY:
+   flags = DRM_MODE_ATOMIC_TEST_ONLY;
+   break;
+   }
+
if (b->state_invalid) {
struct weston_head *head_base;
struct drm_head *head;
@@ -2597,17 +2609,6 @@ drm_pending_state_apply_atomic(struct drm_pending_state 
*pending_state,
goto out;
}
 
-   switch (mode) {
-   case DRM_STATE_APPLY_SYNC:
-   break;
-   case DRM_STATE_APPLY_ASYNC:
-   flags |= DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
-   break;
-   case DRM_STATE_TEST_ONLY:
-   flags |= DRM_MODE_ATOMIC_TEST_ONLY;
-   break;
-   }
-
ret = drmModeAtomicCommit(b->drm.fd, req, flags, b);
 
/* Test commits do not take ownership of the state; return
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v5 07/14] xwm: dump_property() to use FILE internally

2018-07-20 Thread Daniel Stone
From: Pekka Paalanen 

Write the output of dump_property() out in one log call. When multiple
processes (weston and Xwayland) are writing to the same file, this will
keep the property dump uninterrupted by Xwayland debug prints.

This is also preparation for more development in the same direction.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Maniraj Devadoss 
Reviewed-by: Pekka Paalanen 
Reviewed-by: Daniel Stone 
---
 xwayland/window-manager.c | 58 ---
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 2b3defb70..3bf323a42 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -410,20 +410,14 @@ dump_cardinal_array_elem(FILE *fp, unsigned format,
 }
 
 static void
-dump_cardinal_array(xcb_get_property_reply_t *reply)
+dump_cardinal_array(FILE *fp, xcb_get_property_reply_t *reply)
 {
unsigned i = 0;
-   FILE *fp;
void *arr;
char *str = NULL;
-   size_t size = 0;
 
assert(reply->type == XCB_ATOM_CARDINAL);
 
-   fp = open_memstream(, );
-   if (!fp)
-   return;
-
arr = xcb_get_property_value(reply);
 
fprintf(fp, "[");
@@ -432,10 +426,6 @@ dump_cardinal_array(xcb_get_property_reply_t *reply)
 arr, reply->value_len, i);
fprintf(fp, "]");
 
-   if (fclose(fp) != 0)
-   return;
-
-   wm_log_continue("%s\n", str);
free(str);
 }
 
@@ -449,22 +439,29 @@ dump_property(struct weston_wm *wm,
xcb_window_t *window_value;
int width, len;
uint32_t i;
+   FILE *fp;
+   char *logstr;
+   size_t logsize;
 
-   width = wm_log_continue("%s: ", get_atom_name(wm->conn, property));
-   if (reply == NULL) {
-   wm_log_continue("(no reply)\n");
+   fp = open_memstream(, );
+   if (!fp)
return;
+
+   width = fprintf(fp, "%s: ", get_atom_name(wm->conn, property));
+   if (reply == NULL) {
+   fprintf(fp, "(no reply)\n");
+   goto out;
}
 
-   width += wm_log_continue("%s/%d, length %d (value_len %d): ",
-get_atom_name(wm->conn, reply->type),
-reply->format,
-xcb_get_property_value_length(reply),
-reply->value_len);
+   width += fprintf(fp, "%s/%d, length %d (value_len %d): ",
+get_atom_name(wm->conn, reply->type),
+reply->format,
+xcb_get_property_value_length(reply),
+reply->value_len);
 
if (reply->type == wm->atom.incr) {
incr_value = xcb_get_property_value(reply);
-   wm_log_continue("%d\n", *incr_value);
+   fprintf(fp, "%d\n", *incr_value);
} else if (reply->type == wm->atom.utf8_string ||
   reply->type == wm->atom.string) {
text_value = xcb_get_property_value(reply);
@@ -472,29 +469,34 @@ dump_property(struct weston_wm *wm,
len = 40;
else
len = reply->value_len;
-   wm_log_continue("\"%.*s\"\n", len, text_value);
+   fprintf(fp, "\"%.*s\"\n", len, text_value);
} else if (reply->type == XCB_ATOM_ATOM) {
atom_value = xcb_get_property_value(reply);
for (i = 0; i < reply->value_len; i++) {
name = get_atom_name(wm->conn, atom_value[i]);
if (width + strlen(name) + 2 > 78) {
-   wm_log_continue("\n");
+   fprintf(fp, "\n");
width = 4;
} else if (i > 0) {
-   width +=  wm_log_continue(", ");
+   width +=  fprintf(fp, ", ");
}
 
-   width +=  wm_log_continue("%s", name);
+   width +=  fprintf(fp, "%s", name);
}
-   wm_log_continue("\n");
+   fprintf(fp, "\n");
} else if (reply->type == XCB_ATOM_CARDINAL) {
-   dump_cardinal_array(reply);
+   dump_cardinal_array(fp, reply);
} else if (reply->type == XCB_ATOM_WINDOW && reply->format == 32) {
window_value = xcb_get_property_value(reply);
-   wm_log_continue("win %u\n", *window_value);
+   fprintf(fp, "win %u\n&

[PATCH weston v5 11/14] compositor: Add weston_layer_mask_is_infinite

2018-07-20 Thread Daniel Stone
As a counterpart to weston_layer_set_mask_infinite(), returning if the
mask is the same as what is set.

Signed-off-by: Daniel Stone 
---
 libweston/compositor.c | 9 +
 libweston/compositor.h | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/libweston/compositor.c b/libweston/compositor.c
index 8768f67a0..0c147d4a6 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -2746,6 +2746,15 @@ weston_layer_set_mask_infinite(struct weston_layer 
*layer)
 UINT32_MAX, UINT32_MAX);
 }
 
+WL_EXPORT bool
+weston_layer_mask_is_infinite(struct weston_layer *layer)
+{
+   return layer->mask.x1 == INT32_MIN &&
+  layer->mask.y1 == INT32_MIN &&
+  layer->mask.x2 == INT32_MIN + UINT32_MAX &&
+  layer->mask.y2 == INT32_MIN + UINT32_MAX;
+}
+
 WL_EXPORT void
 weston_output_schedule_repaint(struct weston_output *output)
 {
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 83448b70f..67a835713 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1686,6 +1686,9 @@ weston_layer_set_mask(struct weston_layer *layer, int x, 
int y, int width, int h
 void
 weston_layer_set_mask_infinite(struct weston_layer *layer);
 
+bool
+weston_layer_mask_is_infinite(struct weston_layer *layer);
+
 void
 weston_plane_init(struct weston_plane *plane,
struct weston_compositor *ec,
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v5 04/14] compositor: add option to enable weston_debug

2018-07-20 Thread Daniel Stone
From: Pekka Paalanen 

Let users enable the compositor debug protocol on the compositor command
line. This allows weston-debug tool to work.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Maniraj Devadoss 
Reviewed-by: Pekka Paalanen 
---
 compositor/main.c |  7 +++
 man/weston.man| 11 +++
 2 files changed, 18 insertions(+)

diff --git a/compositor/main.c b/compositor/main.c
index b5b4fc594..2f34e1115 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -60,6 +60,7 @@
 #include "compositor-x11.h"
 #include "compositor-wayland.h"
 #include "windowed-output-api.h"
+#include "weston-debug.h"
 
 #define WINDOW_TITLE "Weston Compositor"
 
@@ -508,6 +509,7 @@ usage(int error_code)
"  -c, --config=FILE\tConfig file to load, defaults to 
weston.ini\n"
"  --no-config\t\tDo not read weston.ini\n"
"  --wait-for-debugger\tRaise SIGSTOP on start-up\n"
+   "  --debug\t\tEnable debug extension\n"
"  -h, --help\t\tThis help message\n\n");
 
 #if defined(BUILD_DRM_COMPOSITOR)
@@ -2375,6 +2377,7 @@ int main(int argc, char *argv[])
char *socket_name = NULL;
int32_t version = 0;
int32_t noconfig = 0;
+   int32_t debug_protocol = 0;
int32_t numlock_on;
char *config_file = NULL;
struct weston_config *config = NULL;
@@ -2399,6 +2402,7 @@ int main(int argc, char *argv[])
{ WESTON_OPTION_BOOLEAN, "no-config", 0,  },
{ WESTON_OPTION_STRING, "config", 'c', _file },
{ WESTON_OPTION_BOOLEAN, "wait-for-debugger", 0, 
_for_debugger },
+   { WESTON_OPTION_BOOLEAN, "debug", 0, _protocol },
};
 
wl_list_init(_list);
@@ -2486,6 +2490,9 @@ int main(int argc, char *argv[])
}
segv_compositor = wet.compositor;
 
+   if (debug_protocol)
+   weston_compositor_enable_debug_protocol(wet.compositor);
+
if (weston_compositor_init_config(wet.compositor, config) < 0)
goto out;
 
diff --git a/man/weston.man b/man/weston.man
index 596041dff..8a0e4a6f8 100644
--- a/man/weston.man
+++ b/man/weston.man
@@ -133,6 +133,17 @@ If also
 .B --no-config
 is given, no configuration file will be read.
 .TP
+.BR \-\-debug
+Enable debug protocol extension
+.I weston_debug_v1
+which any client can use to receive debugging messages from the compositor.
+
+.B WARNING:
+This is risky for two reasons. First, a client may cause a denial-of-service
+blocking the compositor by providing an unsuitable file descriptor, and
+second, the debug messages may expose sensitive information. This option
+should not be used in production.
+.TP
 .BR \-\-version
 Print the program version.
 .TP
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v5 06/14] compositor: offer logs via weston-debug

2018-07-20 Thread Daniel Stone
From: Pekka Paalanen 

This registers a new weston-debug scope "log" through which one can get
live log output interspersed with possible other debugging prints.

Signed-off-by: Pekka Paalanen 

pass the log_scope to weston_debug_scope_timestamp API to append
the scope name to the timestamp

Signed-off-by: Maniraj Devadoss 
Reviewed-by: Pekka Paalanen 
Reviewed-by: Daniel Stone 
---
 compositor/main.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/compositor/main.c b/compositor/main.c
index 2f34e1115..eaf4cf381 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -119,6 +119,7 @@ struct wet_compositor {
 };
 
 static FILE *weston_logfile = NULL;
+static struct weston_debug_scope *log_scope;
 
 static int cached_tm_mday = -1;
 
@@ -149,9 +150,16 @@ static int weston_log_timestamp(void)
 static void
 custom_handler(const char *fmt, va_list arg)
 {
+   char timestr[128];
+
weston_log_timestamp();
fprintf(weston_logfile, "libwayland: ");
vfprintf(weston_logfile, fmt, arg);
+
+   weston_debug_scope_printf(log_scope, "%s libwayland: ",
+   weston_debug_scope_timestamp(log_scope,
+   timestr, sizeof timestr));
+   weston_debug_scope_vprintf(log_scope, fmt, arg);
 }
 
 static void
@@ -183,6 +191,14 @@ static int
 vlog(const char *fmt, va_list ap)
 {
int l;
+   char timestr[128];
+
+   if (weston_debug_scope_is_enabled(log_scope)) {
+   weston_debug_scope_printf(log_scope, "%s ",
+   weston_debug_scope_timestamp(log_scope,
+   timestr, sizeof timestr));
+   weston_debug_scope_vprintf(log_scope, fmt, ap);
+   }
 
l = weston_log_timestamp();
l += vfprintf(weston_logfile, fmt, ap);
@@ -193,6 +209,8 @@ vlog(const char *fmt, va_list ap)
 static int
 vlog_continue(const char *fmt, va_list argp)
 {
+   weston_debug_scope_vprintf(log_scope, fmt, argp);
+
return vfprintf(weston_logfile, fmt, argp);
 }
 
@@ -2490,6 +2508,9 @@ int main(int argc, char *argv[])
}
segv_compositor = wet.compositor;
 
+   log_scope = weston_compositor_add_debug_scope(wet.compositor, "log",
+   "Weston and Wayland log\n", NULL, NULL);
+
if (debug_protocol)
weston_compositor_enable_debug_protocol(wet.compositor);
 
@@ -2602,6 +2623,7 @@ out:
/* free(NULL) is valid, and it won't be NULL if it's used */
free(wet.parsed_options);
 
+   weston_debug_scope_destroy(log_scope);
weston_compositor_destroy(wet.compositor);
 
 out_signals:
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v5 14/14] compositor-drm: Add drm-backend log debug scope

2018-07-20 Thread Daniel Stone
Add a 'drm-debug' scope which prints verbose information about the DRM
backend's repaint cycle, including the decision tree on how views are
assigned (or not) to planes.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 233 -
 1 file changed, 206 insertions(+), 27 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 653d13e0c..2cadf036c 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -51,6 +51,7 @@
 
 #include "compositor.h"
 #include "compositor-drm.h"
+#include "weston-debug.h"
 #include "shared/helpers.h"
 #include "shared/timespec-util.h"
 #include "gl-renderer.h"
@@ -73,6 +74,9 @@
 #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
 #endif
 
+#define drm_debug(b, ...) \
+   weston_debug_scope_printf((b)->debug, __VA_ARGS__)
+
 #define MAX_CLONED_CONNECTORS 4
 
 /**
@@ -302,6 +306,8 @@ struct drm_backend {
bool shutting_down;
 
bool aspect_ratio_supported;
+
+   struct weston_debug_scope *debug;
 };
 
 struct drm_mode {
@@ -2358,6 +2364,9 @@ crtc_add_prop(drmModeAtomicReq *req, struct drm_output 
*output,
 
ret = drmModeAtomicAddProperty(req, output->crtc_id, info->prop_id,
   val);
+   drm_debug(output->backend, "\t\t\t[CRTC:%d] %d (%s) -> %lld (0x%llx)\n",
+ output->crtc_id, info->prop_id, info->name,
+ (unsigned long long) val, (unsigned long long) val);
return (ret <= 0) ? -1 : 0;
 }
 
@@ -2373,6 +2382,9 @@ connector_add_prop(drmModeAtomicReq *req, struct drm_head 
*head,
 
ret = drmModeAtomicAddProperty(req, head->connector_id,
   info->prop_id, val);
+   drm_debug(head->backend, "\t\t\t[CONN:%d] %d (%s) -> %lld (0x%llx)\n",
+ head->connector_id, info->prop_id, info->name,
+ (unsigned long long) val, (unsigned long long) val);
return (ret <= 0) ? -1 : 0;
 }
 
@@ -2388,6 +2400,9 @@ plane_add_prop(drmModeAtomicReq *req, struct drm_plane 
*plane,
 
ret = drmModeAtomicAddProperty(req, plane->plane_id, info->prop_id,
   val);
+   drm_debug(plane->backend, "\t\t\t[PLANE:%d] %d (%s) -> %lld (0x%llx)\n",
+ plane->plane_id, info->prop_id, info->name,
+ (unsigned long long) val, (unsigned long long) val);
return (ret <= 0) ? -1 : 0;
 }
 
@@ -2406,6 +2421,9 @@ drm_mode_ensure_blob(struct drm_backend *backend, struct 
drm_mode *mode)
if (ret != 0)
weston_log("failed to create mode property blob: %m\n");
 
+   drm_debug(backend, "\t\t\t[atomic] created new mode blob %ld for %s",
+ (unsigned long) mode->blob_id, mode->mode_info.name);
+
return ret;
 }
 
@@ -2415,17 +2433,23 @@ drm_output_apply_state_atomic(struct drm_output_state 
*state,
  uint32_t *flags)
 {
struct drm_output *output = state->output;
-   struct drm_backend *backend = to_drm_backend(output->base.compositor);
+   struct drm_backend *b = to_drm_backend(output->base.compositor);
struct drm_plane_state *plane_state;
struct drm_mode *current_mode = to_drm_mode(output->base.current_mode);
struct drm_head *head;
int ret = 0;
 
-   if (state->dpms != output->state_cur->dpms)
+   drm_debug(b, "\t\t[atomic] %s output %d (%s) state\n",
+ (*flags & DRM_MODE_ATOMIC_TEST_ONLY) ? "testing" : "applying",
+ output->base.id, output->base.name);
+
+   if (state->dpms != output->state_cur->dpms) {
+   drm_debug(b, "\t\t\t[atomic] DPMS state differs, modeset OK\n");
*flags |= DRM_MODE_ATOMIC_ALLOW_MODESET;
+   }
 
if (state->dpms == WESTON_DPMS_ON) {
-   ret = drm_mode_ensure_blob(backend, current_mode);
+   ret = drm_mode_ensure_blob(b, current_mode);
if (ret != 0)
return ret;
 
@@ -2523,6 +2547,9 @@ drm_pending_state_apply_atomic(struct drm_pending_state 
*pending_state,
uint32_t *unused;
int err;
 
+   drm_debug(b, "\t\t[atomic] previous state invalid; "
+"starting with fresh state\n");
+
/* If we need to reset all our state (e.g. because we've
 * just started, or just been VT-switched in), explicitly
 * disable all the CRTCs and connectors we aren't using. */
@@ -2535,9 +2562,14 @@ drm_pending_state_apply_atomic(struct drm_pending_state 
*pending_state,
 
head = t

[PATCH weston v5 05/14] clients: add weston-debug

2018-07-20 Thread Daniel Stone
From: Pekka Paalanen 

A tool for accessing the zcompositor_debug_v1 interface features.

Installed along weston-info, because it should be potentially useful for
people running libweston-based compositors.

Signed-off-by: Pekka Paalanen 

Added a man page for weston-debug client

Signed-off-by: Maniraj Devadoss 
[Pekka: fixed 'missing braces aroudn initializer' warning]

Add --list and --all arguments, using interface advertisement.

Signed-off-by: Daniel Stone 
---
 Makefile.am|  16 +-
 clients/weston-debug.c | 453 +
 man/weston-debug.man   |  46 +
 3 files changed, 513 insertions(+), 2 deletions(-)
 create mode 100644 clients/weston-debug.c
 create mode 100644 man/weston-debug.man

diff --git a/Makefile.am b/Makefile.am
index c2d9048b3..04381e0f7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -531,7 +531,7 @@ spring_tool_SOURCES =   \
 
 if BUILD_CLIENTS
 
-bin_PROGRAMS += weston-terminal weston-info
+bin_PROGRAMS += weston-terminal weston-info weston-debug
 
 libexec_PROGRAMS +=\
weston-desktop-shell\
@@ -862,6 +862,15 @@ nodist_weston_info_SOURCES =   
\
 weston_info_LDADD = $(WESTON_INFO_LIBS) libshared.la
 weston_info_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
 
+weston_debug_SOURCES = \
+   clients/weston-debug.c  \
+   shared/helpers.h
+nodist_weston_debug_SOURCES =  \
+   protocol/weston-debug-protocol.c\
+   protocol/weston-debug-client-protocol.h
+weston_debug_LDADD = $(WESTON_INFO_LIBS) libshared.la
+weston_debug_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
+
 weston_desktop_shell_SOURCES = \
clients/desktop-shell.c \
shared/helpers.h
@@ -898,6 +907,8 @@ BUILT_SOURCES +=\
protocol/weston-screenshooter-client-protocol.h \
protocol/weston-touch-calibration-protocol.c\
protocol/weston-touch-calibration-client-protocol.h \
+   protocol/weston-debug-protocol.c\
+   protocol/weston-debug-client-protocol.h \
protocol/text-cursor-position-client-protocol.h \
protocol/text-cursor-position-protocol.c\
protocol/text-input-unstable-v1-protocol.c  \
@@ -1595,7 +1606,7 @@ surface_screenshot_la_SOURCES = tests/surface-screenshot.c
 # Documentation
 #
 
-man_MANS = weston.1 weston.ini.5
+man_MANS = weston.1 weston.ini.5 weston-debug.1
 
 if ENABLE_DRM_COMPOSITOR
 man_MANS += weston-drm.7
@@ -1623,6 +1634,7 @@ SUFFIXES = .1 .5 .7 .man
 EXTRA_DIST +=  \
doc/calibration-helper.bash \
man/weston.man  \
+   man/weston-debug.man\
man/weston-drm.man  \
man/weston-rdp.man  \
man/weston.ini.man
diff --git a/clients/weston-debug.c b/clients/weston-debug.c
new file mode 100644
index 0..59dacd269
--- /dev/null
+++ b/clients/weston-debug.c
@@ -0,0 +1,453 @@
+/*
+ * Copyright © 2017 Pekka Paalanen 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "config.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "shared/helpers.h"
+#include "shared/zalloc.h"
+#include "weston-debug-client-protocol.h"
+
+struct debug_app {
+   struct {
+   bool help;
+   bool list;
+   bool bind_al

[PATCH weston v5 12/14] compositor: Add scene-graph debug scope

2018-07-20 Thread Daniel Stone
Add a 'scene-graph' debug scope which will dump out the current set of
outputs, layers, and views and as much information as possible about how
they are rendered and composited.

Signed-off-by: Daniel Stone 
---
 libweston/compositor.c | 219 +
 libweston/compositor.h |   4 +
 2 files changed, 223 insertions(+)

diff --git a/libweston/compositor.c b/libweston/compositor.c
index 0c147d4a6..dea91d173 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -55,6 +55,8 @@
 #include "timeline.h"
 
 #include "compositor.h"
+#include "weston-debug.h"
+#include "linux-dmabuf.h"
 #include "viewporter-server-protocol.h"
 #include "presentation-time-server-protocol.h"
 #include "shared/helpers.h"
@@ -6306,6 +6308,216 @@ timeline_key_binding_handler(struct weston_keyboard 
*keyboard,
weston_timeline_open(compositor);
 }
 
+static const char *
+output_repaint_status_text(struct weston_output *output)
+{
+   switch (output->repaint_status) {
+   case REPAINT_NOT_SCHEDULED:
+   return "no repaint";
+   case REPAINT_BEGIN_FROM_IDLE:
+   return "start_repaint_loop scheduled";
+   case REPAINT_SCHEDULED:
+   return "repaint scheduled";
+   case REPAINT_AWAITING_COMPLETION:
+   return "awaiting completion";
+   }
+
+   assert(!"output_repaint_status_text missing enum");
+   return NULL;
+}
+
+static void
+debug_scene_view_print_buffer(FILE *fp, struct weston_view *view)
+{
+   struct weston_buffer *buffer = view->surface->buffer_ref.buffer;
+   struct wl_shm_buffer *shm;
+   struct linux_dmabuf_buffer *dmabuf;
+
+   if (!buffer) {
+   fprintf(fp, "\t\tsolid-colour surface\n");
+   return;
+   }
+
+   shm = wl_shm_buffer_get(buffer->resource);
+   if (shm) {
+   fprintf(fp, "\t\tSHM buffer\n");
+   fprintf(fp, "\t\t\tformat: 0x%lx\n",
+   (unsigned long) wl_shm_buffer_get_format(shm));
+   return;
+   }
+
+   dmabuf = linux_dmabuf_buffer_get(buffer->resource);
+   if (dmabuf) {
+   fprintf(fp, "\t\tdmabuf buffer\n");
+   fprintf(fp, "\t\t\tformat: 0x%lx\n",
+   (unsigned long) dmabuf->attributes.format);
+   fprintf(fp, "\t\t\tmodifier: 0x%llx\n",
+   (unsigned long long) dmabuf->attributes.modifier[0]);
+   return;
+   }
+
+   fprintf(fp, "\t\tEGL buffer");
+}
+
+static void
+debug_scene_view_print(FILE *fp, struct weston_view *view, int view_idx)
+{
+   struct weston_compositor *ec = view->surface->compositor;
+   struct weston_output *output;
+   pixman_box32_t *box;
+   uint32_t surface_id = 0;
+   pid_t pid = 0;
+
+   if (view->surface->resource) {
+   struct wl_resource *resource = view->surface->resource;
+   wl_client_get_credentials(wl_resource_get_client(resource),
+ , NULL, NULL);
+   surface_id = wl_resource_get_id(view->surface->resource);
+   }
+
+   fprintf(fp, "\tView %d (role %s, PID %d, surface ID %u, %p):\n",
+   view_idx, view->surface->role_name, pid, surface_id, view);
+
+   box = pixman_region32_extents(>transform.boundingbox);
+   fprintf(fp, "\t\tposition: (%d, %d) -> (%d, %d)\n",
+   box->x1, box->y1, box->x2, box->y2);
+   box = pixman_region32_extents(>transform.opaque);
+
+   if (pixman_region32_equal(>transform.opaque,
+ >transform.boundingbox)) {
+   fprintf(fp, "\t\t[fully opaque]\n");
+   } else if (!pixman_region32_not_empty(>transform.opaque)) {
+   fprintf(fp, "\t\t[not opaque]\n");
+   } else {
+   fprintf(fp, "\t\t[opaque: (%d, %d) -> (%d, %d)]\n",
+   box->x1, box->y1, box->x2, box->y2);
+   }
+
+   if (view->alpha < 1.0)
+   fprintf(fp, "\t\talpha: %f\n", view->alpha);
+
+   if (view->output_mask != 0) {
+   fprintf(fp, "\t\toutputs: ");
+   wl_list_for_each(output, >output_list, link) {
+   if (!(view->output_mask & (1 << output->id)))
+   continue;
+   fprintf(fp, "%s%d (%s)%s",
+   (view->output_mask & ((1 << output->id) - 1)) ? 
", " : "",
+   output->id, output->name,
+  

[PATCH weston v5 08/14] xwm: move FILE to the callers of dump_property()

2018-07-20 Thread Daniel Stone
From: Pekka Paalanen 

This is preparation for using the weston-debug infrastructure for
WM_DEBUG. dump_property() may be called from different debugging
contexts and often needs to be prefixed with more information.

An alternative to this patch would be to pass in the weston_debug_scope
as an argument to dump_property(), but then all callers would need to be
converted to weston-debug infra in a single commit.

Therefore require the callers to provide the FILE* to print to.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Maniraj Devadoss 
Reviewed-by: Pekka Paalanen 
Reviewed-by: Daniel Stone 
---
 xwayland/selection.c  | 39 +--
 xwayland/window-manager.c | 67 ---
 xwayland/xwayland.h   |  3 +-
 3 files changed, 65 insertions(+), 44 deletions(-)

diff --git a/xwayland/selection.c b/xwayland/selection.c
index a75557044..85c320c29 100644
--- a/xwayland/selection.c
+++ b/xwayland/selection.c
@@ -34,6 +34,12 @@
 #include "xwayland.h"
 #include "shared/helpers.h"
 
+#ifdef WM_DEBUG
+#define wm_log(...) weston_log(__VA_ARGS__)
+#else
+#define wm_log(...) do {} while (0)
+#endif
+
 static int
 writable_callback(int fd, uint32_t mask, void *data)
 {
@@ -102,6 +108,9 @@ weston_wm_get_incr_chunk(struct weston_wm *wm)
 {
xcb_get_property_cookie_t cookie;
xcb_get_property_reply_t *reply;
+   FILE *fp;
+   char *logstr;
+   size_t logsize;
 
cookie = xcb_get_property(wm->conn,
  0, /* delete */
@@ -115,7 +124,13 @@ weston_wm_get_incr_chunk(struct weston_wm *wm)
if (reply == NULL)
return;
 
-   dump_property(wm, wm->atom.wl_selection, reply);
+   fp = open_memstream(, );
+   if (fp) {
+   dump_property(fp, wm, wm->atom.wl_selection, reply);
+   if (fclose(fp) == 0)
+   wm_log("%s", logstr);
+   free(logstr);
+   }
 
if (xcb_get_property_value_length(reply) > 0) {
/* reply's ownership is transferred to wm, which is responsible
@@ -178,6 +193,9 @@ weston_wm_get_selection_targets(struct weston_wm *wm)
xcb_atom_t *value;
char **p;
uint32_t i;
+   FILE *fp;
+   char *logstr;
+   size_t logsize;
 
cookie = xcb_get_property(wm->conn,
  1, /* delete */
@@ -191,7 +209,13 @@ weston_wm_get_selection_targets(struct weston_wm *wm)
if (reply == NULL)
return;
 
-   dump_property(wm, wm->atom.wl_selection, reply);
+   fp = open_memstream(, );
+   if (fp) {
+   dump_property(fp, wm, wm->atom.wl_selection, reply);
+   if (fclose(fp) == 0)
+   wm_log("%s", logstr);
+   free(logstr);
+   }
 
if (reply->type != XCB_ATOM_ATOM) {
free(reply);
@@ -232,6 +256,9 @@ weston_wm_get_selection_data(struct weston_wm *wm)
 {
xcb_get_property_cookie_t cookie;
xcb_get_property_reply_t *reply;
+   FILE *fp;
+   char *logstr;
+   size_t logsize;
 
cookie = xcb_get_property(wm->conn,
  1, /* delete */
@@ -243,7 +270,13 @@ weston_wm_get_selection_data(struct weston_wm *wm)
 
reply = xcb_get_property_reply(wm->conn, cookie, NULL);
 
-   dump_property(wm, wm->atom.wl_selection, reply);
+   fp = open_memstream(, );
+   if (fp) {
+   dump_property(fp, wm, wm->atom.wl_selection, reply);
+   if (fclose(fp) == 0)
+   wm_log("%s", logstr);
+   free(logstr);
+   }
 
if (reply == NULL) {
return;
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 3bf323a42..4a26f6e7f 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -210,23 +210,6 @@ wm_log(const char *fmt, ...)
 #endif
 }
 
-static int __attribute__ ((format (printf, 1, 2)))
-wm_log_continue(const char *fmt, ...)
-{
-#ifdef WM_DEBUG
-   int l;
-   va_list argp;
-
-   va_start(argp, fmt);
-   l = weston_vlog_continue(fmt, argp);
-   va_end(argp);
-
-   return l;
-#else
-   return 0;
-#endif
-}
-
 static void
 weston_output_weak_ref_init(struct weston_output_weak_ref *ref)
 {
@@ -430,7 +413,7 @@ dump_cardinal_array(FILE *fp, xcb_get_property_reply_t 
*reply)
 }
 
 void
-dump_property(struct weston_wm *wm,
+dump_property(FILE *fp, struct weston_wm *wm,
  xcb_atom_t property, xcb_get_property_reply_t *reply)
 {
int32_t *incr_value;
@@ -439,18 +422,11 @@ dump_property(struct weston_wm *wm,
xcb_window_t *window_value;
int width, len;
uint32_t i;
-   FILE *fp;
-   char *logstr;
-   size_t logsize;
-
-   fp = open_memstream(, );
-   if (!fp)
-   return;
 
wid

[PATCH weston v5 09/14] xwm: convert WM_DEBUG into a weston-debug scope

2018-07-20 Thread Daniel Stone
From: Pekka Paalanen 

Instead of a compile time choice, offer the XWM debugging messages
through the weston-debug protocol and tool on demand. Users will not
need to recompile weston to get XWM debugging, and it won't flood the
weston log.

The debug scope needs to be initialized in launcher.c for it be
available from start, before the first X11 client tries to connect and
initializes XWM.

Signed-off-by: Pekka Paalanen 

pass the wm_debug scope to weston_debug_scope_printf API to append
the scopename to the timestr

Signed-off-by: Maniraj Devadoss 
Reviewed-by: Pekka Paalanen 
Reviewed-by: Daniel Stone 
---
 xwayland/launcher.c   |   7 ++
 xwayland/window-manager.c | 166 --
 xwayland/xwayland.h   |   3 +
 3 files changed, 99 insertions(+), 77 deletions(-)

diff --git a/xwayland/launcher.c b/xwayland/launcher.c
index 0ecdb205e..c5b993851 100644
--- a/xwayland/launcher.c
+++ b/xwayland/launcher.c
@@ -229,6 +229,8 @@ weston_xserver_destroy(struct wl_listener *l, void *data)
if (wxs->loop)
weston_xserver_shutdown(wxs);
 
+   weston_debug_scope_destroy(wxs->wm_debug);
+
free(wxs);
 }
 
@@ -391,5 +393,10 @@ weston_module_init(struct weston_compositor *compositor)
wxs->destroy_listener.notify = weston_xserver_destroy;
wl_signal_add(>destroy_signal, >destroy_listener);
 
+   wxs->wm_debug = weston_compositor_add_debug_scope(wxs->compositor,
+   "xwm-wm-x11",
+   "XWM's window management X11 events\n",
+   NULL, NULL);
+
return 0;
 }
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 4a26f6e7f..ccdae57fd 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -193,23 +193,27 @@ static void
 xserver_map_shell_surface(struct weston_wm_window *window,
  struct weston_surface *surface);
 
-static int __attribute__ ((format (printf, 1, 2)))
-wm_log(const char *fmt, ...)
+static bool
+wm_debug_is_enabled(struct weston_wm *wm)
 {
-#ifdef WM_DEBUG
-   int l;
-   va_list argp;
+   return weston_debug_scope_is_enabled(wm->server->wm_debug);
+}
 
-   va_start(argp, fmt);
-   l = weston_vlog(fmt, argp);
-   va_end(argp);
+static void __attribute__ ((format (printf, 2, 3)))
+wm_printf(struct weston_wm *wm, const char *fmt, ...)
+{
+   va_list ap;
+   char timestr[128];
 
-   return l;
-#else
-   return 0;
-#endif
-}
+   if (wm_debug_is_enabled(wm))
+   weston_debug_scope_printf(wm->server->wm_debug, "%s ",
+   
weston_debug_scope_timestamp(wm->server->wm_debug,
+   timestr, sizeof timestr));
 
+   va_start(ap, fmt);
+   weston_debug_scope_vprintf(wm->server->wm_debug, fmt, ap);
+   va_end(ap);
+}
 static void
 weston_output_weak_ref_init(struct weston_output_weak_ref *ref)
 {
@@ -717,10 +721,10 @@ weston_wm_handle_configure_request(struct weston_wm *wm, 
xcb_generic_event_t *ev
uint32_t mask, values[16];
int x, y, width, height, i = 0;
 
-   wm_log("XCB_CONFIGURE_REQUEST (window %d) %d,%d @ %dx%d\n",
-  configure_request->window,
-  configure_request->x, configure_request->y,
-  configure_request->width, configure_request->height);
+   wm_printf(wm, "XCB_CONFIGURE_REQUEST (window %d) %d,%d @ %dx%d\n",
+ configure_request->window,
+ configure_request->x, configure_request->y,
+ configure_request->width, configure_request->height);
 
if (!wm_lookup_window(wm, configure_request->window, ))
return;
@@ -786,11 +790,11 @@ weston_wm_handle_configure_notify(struct weston_wm *wm, 
xcb_generic_event_t *eve
wm->server->compositor->xwayland_interface;
struct weston_wm_window *window;
 
-   wm_log("XCB_CONFIGURE_NOTIFY (window %d) %d,%d @ %dx%d%s\n",
-  configure_notify->window,
-  configure_notify->x, configure_notify->y,
-  configure_notify->width, configure_notify->height,
-  configure_notify->override_redirect ? ", override" : "");
+   wm_printf(wm, "XCB_CONFIGURE_NOTIFY (window %d) %d,%d @ %dx%d%s\n",
+ configure_notify->window,
+ configure_notify->x, configure_notify->y,
+ configure_notify->width, configure_notify->height,
+ configure_notify->override_redirect ? ", override" : "");
 
if (!wm_lookup_window(wm, configure_notify->window, ))
return;
@@ -839,7 +843,7 @@ weston_wm_create_surface(struct wl_listener *listener, void 
*data)
if (wl_resou

[PATCH weston v5 03/14] libweston: add weston_debug API and implementation

2018-07-20 Thread Daniel Stone
From: Pekka Paalanen 

weston_debug is both a libweston API for relaying debugging messages,
and the compositor-debug wayland protocol implementation for accessing those
debug messages from a Wayland client.

weston_debug_compositor_{create,destroy}() are private API, hence not
exported.

Signed-off-by: Pekka Paalanen 

append the debug scope name along with the timestamp in
weston_debug_scope_timestamp API

Signed-off-by: Maniraj Devadoss 
Reviewed-by: Pekka Paalanen 

Add explicit advertisement of debug scope names.

Signed-off-by: Daniel Stone 
---
 Makefile.am  |   2 +
 libweston/compositor.c   |   6 +
 libweston/compositor.h   |   9 +
 libweston/weston-debug.c | 723 +++
 libweston/weston-debug.h | 107 ++
 5 files changed, 847 insertions(+)
 create mode 100644 libweston/weston-debug.c
 create mode 100644 libweston/weston-debug.h

diff --git a/Makefile.am b/Makefile.am
index 66eb365c5..c2d9048b3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -98,6 +98,8 @@ libweston_@LIBWESTON_MAJOR@_la_SOURCES =  
\
libweston/linux-dmabuf.h\
libweston/pixel-formats.c   \
libweston/pixel-formats.h   \
+   libweston/weston-debug.c\
+   libweston/weston-debug.h\
shared/helpers.h\
shared/matrix.c \
shared/matrix.h \
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 9deb7817f..8768f67a0 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -6361,6 +6361,9 @@ weston_compositor_create(struct wl_display *display, void 
*user_data)
  ec, bind_presentation))
goto fail;
 
+   if (weston_debug_compositor_create(ec) < 0)
+   goto fail;
+
if (weston_input_init(ec) != 0)
goto fail;
 
@@ -6699,9 +6702,12 @@ weston_compositor_destroy(struct weston_compositor 
*compositor)
 
weston_plugin_api_destroy_list(compositor);
 
+
if (compositor->heads_changed_source)
wl_event_source_remove(compositor->heads_changed_source);
 
+   weston_debug_compositor_destroy(compositor);
+
free(compositor);
 }
 
diff --git a/libweston/compositor.h b/libweston/compositor.h
index fd0ff7b5b..83448b70f 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1049,6 +1049,7 @@ struct weston_touch_calibrator;
 
 struct weston_desktop_xwayland;
 struct weston_desktop_xwayland_interface;
+struct weston_debug_compositor;
 
 struct weston_compositor {
struct wl_signal destroy_signal;
@@ -1161,6 +1162,8 @@ struct weston_compositor {
weston_touch_calibration_save_func touch_calibration_save;
struct weston_layer calibrator_layer;
struct weston_touch_calibrator *touch_calibrator;
+
+   struct weston_debug_compositor *weston_debug;
 };
 
 struct weston_buffer {
@@ -2315,6 +2318,12 @@ int
 weston_compositor_enable_touch_calibrator(struct weston_compositor *compositor,
weston_touch_calibration_save_func save);
 
+int
+weston_debug_compositor_create(struct weston_compositor *compositor);
+
+void
+weston_debug_compositor_destroy(struct weston_compositor *compositor);
+
 #ifdef  __cplusplus
 }
 #endif
diff --git a/libweston/weston-debug.c b/libweston/weston-debug.c
new file mode 100644
index 0..039247f14
--- /dev/null
+++ b/libweston/weston-debug.c
@@ -0,0 +1,723 @@
+/*
+ * Copyright © 2017 Pekka Paalanen 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "config.h"
+
+#include "weston-debug.h"
+#include "helpers.h"
+#include "compositor.h"
+
+#inclu

[PATCH weston v5 00/14] weston-debug API and tool

2018-07-20 Thread Daniel Stone
Hi,
This patch series implements the weston-debug interface and tool, as
previously worked on by Pekka, Maniraj and Emre.

The xwm changes are unchanged from the previous series, and additionally
reviewed by myself.

The core protocol has seen the addition of a registry-like advertising
interface: this allows the tool to bind to all scopes if requested, or
to list them in a slightly more friendly format.

On top of this, I have added a 'scene-graph' debug interface which
prints some information about the outputs, as well as the current trees
of layers and views. This could likely be improved to be more
informative about, e.g., subsurfaces. It also doesn't print much for SHM
buffers in particular, as these are often not retained after upload.

Rather than carry around a 'drm debug' patch in my tree forever, I have
implemented most of the information from that patch as a new
'drm-backend' scope, which prints as much information as possible about
the repaint cycle, including why it has (or hasn't!) assigned views to
planes.

I would like to push this further, including enhanced surface and buffer
debug as above, but these require pushing more information back from the
renderer and backend into the core. Similarly, I would like to add a
command-line argument which will automatically log the output of named
scopes into the core Weston log, for debugging issues with startup in
particular.

One additional feature I think would be very useful is implementing a
ring buffer for all debug scopes, recording all output even if the scope
is not active. A 'something bad happened' debug keybinding could then be
added which would dump all the stored output, allowing us to find out
more about issues which happened in the past, without incurring the
runtime overhead of dumping huge files to disk.

Given the above two paragraphs are a little more invasive and
controversial, I thought it would be best to send these out now, and
then see where we can take it from there.

Cheers,
Daniel


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v5 01/14] compositor-drm: Add backend pointer to drm_output

2018-07-20 Thread Daniel Stone
Add this for convenience, so it's easier to access when we add the DRM
backend debug scope.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 704ac32c7..e27671437 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -463,6 +463,7 @@ struct drm_head {
 
 struct drm_output {
struct weston_output base;
+   struct drm_backend *backend;
 
uint32_t crtc_id; /* object ID to pass to DRM functions */
int pipe; /* index of CRTC in resource array / bitmasks */
@@ -6101,6 +6102,8 @@ drm_output_create(struct weston_compositor *compositor, 
const char *name)
if (output == NULL)
return NULL;
 
+   output->backend = b;
+
weston_output_init(>base, compositor, name);
 
output->base.enable = drm_output_enable;
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH] compositor-drm: Don't test render-only atomic configuration

2018-07-20 Thread Daniel Stone
In the RENDERER_ONLY state proposal mode, we don't actually have a
viable configuration to test, because we won't get a renderer buffer
until after assign_planes - where we're called from - has completed.

This can result in us trying to test a configuration with the CRTC and
connectors active, but no planes active, which the kernel can
legitimately fail.

If we're working in renderer-only mode, just return the state we have
without trying to test it first, and let the kernel fill it in later.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 98e6ff839..704ac32c7 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3488,6 +3488,11 @@ drm_output_propose_state(struct weston_output 
*output_base,
pixman_region32_fini(_region);
pixman_region32_fini(_region);
 
+   /* In renderer-only mode, we can't test the state as we don't have a
+* renderer buffer yet. */
+   if (mode == DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY)
+   return state;
+
/* Check to see if this state will actually work. */
ret = drm_pending_state_test(state->pending_state);
if (ret != 0)
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: DRM page-flip with damage for weston

2018-07-20 Thread Daniel Stone
Hi Deepak,

On Fri, 20 Jul 2018 at 12:21, Deepak Singh Rawat  wrote:
> In brief the damage is in frame-buffer coordinate of attached fb to the plane.
> Unlike plane src coordinates, damage clips are not in 16.16 fixed point. 
> Damage
> during page flip is helpful for some drivers like vmwgfx where each 
> framebuffer
> change needs to be transmitted over network, usb, etc.
>
> Now that I have some code ready and got it working for vmwgfx driver, the
> next step is to change weston to send damage during page flip. With my current
> understanding of weston I think the damage received during
> weston_output.repaint() is exactly what I am looking for ? Does this damage
> region during weston_output.repaint() is in frame-buffer coordinate ?

The damage region received during output repaint is in Weston's global
co-ordinate space. To shift to CRTC co-ordinates, you need to
translate the damage region by (-output->x, -output->y). When we are
using the renderer, there is no scaling, so CRTC co-ordinates and
framebuffer co-ordinates are guaranteed to be equal. This only
accounts for the primary plane; damage to views on other planes is
considered separately.

Also, if you are not already working from git master, I recommend you
do so, as the DRM backend has changed hugely since 4.0.0.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH] compositor-drm: Remove unnecessary libdrm defines

2018-07-20 Thread Daniel Stone
The backend begins with a series of #defines of libdrm tokens, in case
the libdrm we build against is too old.

Commit efdebbc4e82b ("configure.ac: bump libdrm requirement to 2.4.68")
did what it said on the box; since we now depend on a relatively modern
libdrm, we can get rid of most of our compatibility defines.

DRM_CAP_TIMESTAMP_MONOTONIC was added in libdrm 2.4.47 (f8f1f6e37ae2).
DRM_CLIENT_CAP_UNIVERSAL_PLANES was added in libdrm 2.4.55
(8fc62ca8ac01).
DRM_CAP_CURSOR_WIDTH and HEIGHT were added in libdrm 2.4.68
(cc9a53f076d4).

Remove these four fallback definitions.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 16 
 1 file changed, 16 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index da9cf5e4f..e3e127a2d 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -91,26 +91,10 @@
 #include "linux-dmabuf.h"
 #include "linux-dmabuf-unstable-v1-server-protocol.h"
 
-#ifndef DRM_CAP_TIMESTAMP_MONOTONIC
-#define DRM_CAP_TIMESTAMP_MONOTONIC 0x6
-#endif
-
-#ifndef DRM_CLIENT_CAP_UNIVERSAL_PLANES
-#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2
-#endif
-
 #ifndef DRM_CLIENT_CAP_ASPECT_RATIO
 #define DRM_CLIENT_CAP_ASPECT_RATIO4
 #endif
 
-#ifndef DRM_CAP_CURSOR_WIDTH
-#define DRM_CAP_CURSOR_WIDTH 0x8
-#endif
-
-#ifndef DRM_CAP_CURSOR_HEIGHT
-#define DRM_CAP_CURSOR_HEIGHT 0x9
-#endif
-
 #ifndef GBM_BO_USE_CURSOR
 #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
 #endif
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [RFC wayland] wayland-server: Finally remove deprecated struct wl_buffer definition

2018-07-14 Thread Daniel Stone
Hey Derek,
On Fri, 16 Feb 2018 at 16:55, Derek Foreman  wrote:
> commit d94a8722cb29d8b897672be66ff3c9ff79eab6fe
> warned this was coming, back in 2013.
>
> I've seen libraries that have wayland client and server using functions
> in the same file.  Since struct wl_buffer still exists as an opaque
> entity in client code, the vestigial deprecated wl_buffer from the
> server include will generate warnings when not building with
> WL_HIDE_DEPRECATED.

I wonder if it's now too late to land this for the next release. Seems
good to me though, and certainly people have had enough notice ...

For landing whenever (I'll happily take your judgement), this is:
Reviewed-by: Daniel Stone 

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 1/4] Add CONTRIBUTING.md document

2018-07-14 Thread Daniel Stone
From: Pekka Paalanen 

Taken from Pekka's wayland/wayland@630c25f4c160 and follow-ups, use
Wayland's CONTRIBUTING document as a basis for Weston.

Signed-off-by: Daniel Stone 
---
 CONTRIBUTING.md | 343 
 1 file changed, 343 insertions(+)
 create mode 100644 CONTRIBUTING.md

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
new file mode 100644
index 0..4273d99d4
--- /dev/null
+++ b/CONTRIBUTING.md
@@ -0,0 +1,343 @@
+Contributing to Wayland
+===
+
+Sending patches
+---
+
+Patches should be sent to **wayland-devel@lists.freedesktop.org**, using
+`git send-email`. See [git documentation] for help.
+
+The first line of a commit message should contain a prefix indicating
+what part is affected by the patch followed by one sentence that
+describes the change. For examples:
+
+protocol: Support scaled outputs and surfaces
+
+and
+
+doc: generate server documentation from XML too
+
+If in doubt what prefix to use, look at other commits that change the
+same file(s) as the patch being sent.
+
+The body of the commit message should describe what the patch changes
+and why, and also note any particular side effects. This shouldn't be
+empty on most of the cases. It shouldn't take a lot of effort to write
+a commit message for an obvious change, so an empty commit message
+body is only acceptable if the questions "What?" and "Why?" are already
+answered on the one-line summary.
+
+The lines of the commit message should have at most 76 characters, to
+cope with the way git log presents them.
+
+See [notes on commit messages] for a recommended reading on writing commit
+messages.
+
+Your patches should also include a Signed-off-by line with your name and
+email address.  If you're not the patch's original author, you should
+also gather S-o-b's by them (and/or whomever gave the patch to you.) The
+significance of this is that it certifies that you created the patch,
+that it was created under an appropriate open source license, or
+provided to you under those terms.  This lets us indicate a chain of
+responsibility for the copyright status of the code.
+
+We won't reject patches that lack S-o-b, but it is strongly recommended.
+
+When you re-send patches, revised or not, it would be very good to document the
+changes compared to the previous revision in the commit message and/or the
+cover letter. If you have already received Reviewed-by or Acked-by tags, you
+should evaluate whether they still apply and include them in the respective
+commit messages. Otherwise the tags may be lost, reviewers miss the credit they
+deserve, and the patches may cause redundant review effort.
+
+
+Tracking patches and following up
+-
+
+[Wayland Patchwork](http://patchwork.freedesktop.org/project/wayland/list/) is
+used for tracking patches to Wayland and Weston. Xwayland patches are tracked
+with the [Xorg project](https://patchwork.freedesktop.org/project/Xorg/list/)
+instead. Libinput patches, even though they use the same mailing list as
+Wayland, are not tracked in the Wayland Patchwork.
+
+The following applies only to Wayland and Weston.
+
+If a patch is not found in Patchwork, there is a high possibility for it to be
+forgotten. Patches attached to bug reports or not arriving to the mailing list
+because of e.g. subscription issues will not be in Patchwork because Patchwork
+only collects patches sent to the list.
+
+When you send a revised version of a patch, it would be very nice to mark your
+old patch as superseded (or rejected, if that is applicable). You can change
+the status of your own patches by registering to Patchwork - ownership is
+identified by email address you use to register. Updating your patch status
+appropriately will help maintainer work.
+
+The following patch states are found in Patchwork:
+
+- **New**:
+Patches under discussion or not yet processed.
+
+- **Under review**:
+Mostly unused state.
+
+- **Accepted**:
+The patch is merged in the master branch upstream, as is or slightly
+modified.
+
+- **Rejected**:
+The idea or approach is rejected and cannot be fixed by revising
+the patch.
+
+- **RFC**:
+Request for comments, not meant to be merged as is.
+
+- **Not applicable**:
+The email was not actually a patch, or the patch is not for Wayland or
+Weston. Libinput patches are usually automatically ignored by Wayland
+Patchwork, but if they get through, they will be marked as Not
+applicable.
+
+- **Changes requested**:
+Reviewers determined that changes to the patch are needed. The
+submitter is expected to send a revised version. (You should
+not wait for your patch to be set to this state before revising,
+though.)
+
+- **Awaiting upstream**:
+Mostly unused as the patch is waiting for upstream actions but
+is not shown in the default list, which means it is easy to
+overloo

[PATCH weston 0/4] Doc updates, GitLab MRs

2018-07-14 Thread Daniel Stone
Hi,
The first two patches simply copy Pekka's CONTRIBUTING.md work, which
just landed in Wayland, into Weston.

The third patch attempts to use the power of the English language,
augmented with Markdown, to explain to people what Weston is in 2018,
and how they might go about using it. The previous documentation was
never quite clear on any of those things.

The final patch is a slightly larger update. Though people had
reservations about using MRs for Wayland and wayland-protocols in the
discussion about migrating to GitLab, the opinion from Weston developers
was unanimously in favour of doing so.

I suggest we begin using MRs for Weston either shortly before 5.0 (as we
enter beta?), or immediately afterwards. Doing so allows us to track
review comments across multiple revisions of patches, lets us run CI on
all the branches before we push, and offers much more foolproof tracking
of active patchsets than Patchwork.

Cheers,
Daniel


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 3/3 v4] simple-dmabuf-drm: use GBM generic calls

2018-07-13 Thread Daniel Stone
Hi Emilio,

On Thu, 12 Jul 2018 at 12:46, Emilio Pozuelo Monfort  wrote:
> @@ -343,17 +143,16 @@ fill_content(struct buffer *my_buf, uint64_t modifier)
> }
> else if (modifier == DRM_FORMAT_MOD_LINEAR) {
> uint8_t *pix8;
> -   /* first plane: Y (2/3 of the buffer)   */
> -   for (y = 0; y < my_buf->height * 2/3; y++) {
> +   /* first plane: Y */
> +   for (y = 0; y < my_buf->height; y++) {
> pix8 = my_buf->mmap + y * my_buf->stride;
> for (x = 0; x < my_buf->width; x++)
> -   *pix8++ = x % 0xff;
> +   *pix8++ = x % 256;
> }
> -   /* second plane (CbCr) is half the size of Y
> -  plane (last 1/3 of the buffer) */
> -   for (y = my_buf->height * 2/3; y < my_buf->height; 
> y++) {
> -   pix8 = my_buf->mmap + y * my_buf->stride;
> -   for (x = 0; x < my_buf->width; x+=2) {
> +   /* second plane (CbCr) is half the size of Y */
> +   for (y = 0; y < my_buf->height; y++) {
> +   pix8 = my_buf->mmap2 + y * my_buf->stride2;
> +   for (x = 0; x < my_buf->width; x++) {

This should only allocate and fill half the width/height. I'll squash
that in when I push.

> -   drmVersionPtr version = drmGetVersion(buf->drm_fd);
> +   close(buf->dmabuf_fd);
> +   close(buf->dmabuf_fd2);

As there was no initialisation to -1, this can close stdout instead.

> +static struct gbm_bo *
> +create_bo(struct buffer *buffer, int format, const uint64_t *modifiers, 
> const unsigned int count)
> +{
> +   struct gbm_bo *bo;
> +
> +   if (!modifiers)
> +   bo = gbm_bo_create(buffer->dev, buffer->width, 
> buffer->height, format, 0 /* usage */);
> +   else
> +   bo = gbm_bo_create_with_modifiers(buffer->dev, buffer->width, 
> buffer->height, GBM_FORMAT_GR88,
> + modifiers, count);

I've fixed the hardcoded format. These are also some seriously long
lines ... we don't stick strictly to 80 in this file, but these are
something like 110.

> +   if (buffer->bo2) gbm_bo_unmap(buffer->bo2, buffer->mmap2);
> +   buffer->dmabuf_fd = gbm_bo_get_fd(buffer->bo);
> +   if (buffer->bo2) buffer->dmabuf_fd2 = gbm_bo_get_fd(buffer->bo2);

Please also put conditionals and statements on separate lines. There
are also a fair few if ((foo = thing) == NULL) tests, which is
something we avoid.

> @@ -391,7 +391,7 @@ AC_ARG_ENABLE(simple-dmabuf-drm-client,
>   [do not build the simple dmabuf drm client]),,
>enable_simple_dmabuf_drm_client="auto")
>  if ! test "x$enable_simple_dmabuf_drm_client" = "xno"; then
> -  PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm], 
> [have_simple_dmabuf_libs=yes],
> +  PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm gbm], 
> [have_simple_dmabuf_libs=yes],

This is missing a version check for the modifiers API.

I was going to fix these up and push with review, but whilst this
works for NV12, it breaks RGB for me on Intel:
[1495104.378]  -> wl_compositor@4.create_surface(new id wl_surface@3)
[1495104.389]  -> zxdg_shell_v6@6.get_xdg_surface(new id
zxdg_surface_v6@7, wl_surface@3)
[1495104.404]  -> zxdg_surface_v6@7.get_toplevel(new id zxdg_toplevel_v6@8)
[1495104.414]  -> zxdg_toplevel_v6@8.set_title("simple-dmabuf")
[1495104.422]  -> wl_surface@3.commit()
Missing separate debuginfo for /lib64/libstdc++.so.6
Try: dnf --enablerepo='*debug*' install
/usr/lib/debug/.build-id/a0/41a97a2ee53859845097dc52b0f8cc39738ecb.debug
[New Thread 0x74829700 (LWP 20761)]
weston-simple-dmabuf-drm:
../src/mesa/drivers/dri/i965/brw_bufmgr.c:658: brw_bo_unreference:
Assertion `p_atomic_read(>refcount) > 0' failed.

Thread 1 "weston-simple-d" received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50   return ret;
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x75db2561 in __GI_abort () at abort.c:79
#2  0x75db2431 in __assert_fail_base (fmt=0x75f151a8
"%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x756a3500
"p_atomic_read(>refcount) > 0", file=0x756a32d8
"../src/mesa/drivers/dri/i965/brw_bufmgr.c", line=658,
function=0x756a3930 <__PRETTY_FUNCTION__.46376>
"brw_bo_unreference") at assert.c:92
#3  0x75dc0692 in __GI___assert_fail
(assertion=assertion@entry=0x756a3500
"p_atomic_read(>refcount) > 0", file=file@entry=0x756a32d8
"../src/mesa/drivers/dri/i965/brw_bufmgr.c", line=line@entry=658,

Re: [PATCH weston v4] simple-dmabuf-drm: nv12: properly fill Y plane

2018-07-13 Thread Daniel Stone
Hi Emilio,

On Thu, 12 Jul 2018 at 12:46, Emilio Pozuelo Monfort  wrote:
> @@ -147,7 +147,7 @@ fill_content(struct buffer *my_buf, uint64_t modifier)
> for (y = 0; y < my_buf->height; y++) {
> pix8 = my_buf->mmap + y * my_buf->stride;
> for (x = 0; x < my_buf->width; x++)
> -   *pix8++ = x % 0xff;
> +   *pix8++ = x % 256;

This somehow ended up also squashed into the GBM rewrite. I've pulled
it out of that patch.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 1/3 v4] simple-dmabuf-drm: require zwp_linux_dmabuf_v1 v3

2018-07-13 Thread Daniel Stone
Hi Emilio,

On Thu, 12 Jul 2018 at 12:47, Emilio Pozuelo Monfort  wrote:
> @@ -821,16 +820,8 @@ registry_handle_global(void *data, struct wl_registry 
> *registry,
> d->fshell = wl_registry_bind(registry,
>  id, 
> _fullscreen_shell_v1_interface, 1);
> } else if (strcmp(interface, "zwp_linux_dmabuf_v1") == 0) {
> -   int ver;
> -   if (d->req_dmabuf_modifiers)
> -   ver = 3;
> -   else if (d->req_dmabuf_immediate)
> -   ver = 2;
> -   else
> -   ver = 1;
> d->dmabuf = wl_registry_bind(registry,
> -id, 
> _linux_dmabuf_v1_interface,
> -ver);
> +id, 
> _linux_dmabuf_v1_interface, 3);

This is missing a version check now, which I'll squash in when pushing.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 1/4] contributing: how to read the review rules

2018-07-13 Thread Daniel Stone
Hi,

On Tue, 3 Jul 2018 at 11:32, Pekka Paalanen  wrote:
> This is to avoid fighting around the letter of the guidelines. This is
> not a protocol spec.

Seems entirely sensible to me; thanks for writing this up!

Series is:
Reviewed-by: Daniel Stone 

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] protocol: allow to send a zero physical output size

2018-07-13 Thread Daniel Stone
Hi,

On Tue, 3 Jul 2018 at 12:27, Simon Ser  wrote:
> Physical size doesn't always make sense for all outputs. In case
> it's not available or not relevant, allow compositors to send zero.
> ---
> In practice this doesn't seem to break any client toolkit. We've been
> doing that for a long time in wlroots.

Seems legitimate to me. I'd like to hear if anyone else touching
output has thoughts, but this is:
Acked-by: Daniel Stone 

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH] compositor-drm: Remove addfb warning for user buffers

2018-07-12 Thread Daniel Stone
THe KMS AddFB call can fail for any reason at all: format/modifier not
suitable, stride not aligned, allocation not contiguous, etc. If this
happens with Weston's own buffers, the result is bad - no composition
output.

Failing AddFB from user-supplied buffers though, is not an error. The
user can't necessarily allocate suitable buffers, nor does it have to.
Don't spam the log with warnings when we fail on user buffers.

Signed-off-by: Daniel Stone 
Reported-by: Pekka Paalanen 
---
 libweston/compositor-drm.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 95b379740..f57be6265 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1184,10 +1184,8 @@ drm_fb_get_from_dmabuf(struct linux_dmabuf_buffer 
*dmabuf,
goto err_free;
}
 
-   if (drm_fb_addfb(fb) != 0) {
-   weston_log("failed to create kms fb: %m\n");
+   if (drm_fb_addfb(fb) != 0)
goto err_free;
-   }
 
return fb;
 
@@ -1258,7 +1256,8 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend 
*backend,
}
 
if (drm_fb_addfb(fb) != 0) {
-   weston_log("failed to create kms fb: %m\n");
+   if (type == BUFFER_GBM_SURFACE)
+   weston_log("failed to create kms fb: %m\n");
goto err_free;
}
 
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v21.1 2/4] compositor-drm: Allow scanout plane to be occluded by overlay

2018-07-11 Thread Daniel Stone
Hi Pekka,

On Wed, 11 Jul 2018 at 14:26, Pekka Paalanen  wrote:
> On Wed, 11 Jul 2018 14:10:55 +0100 Daniel Stone  wrote:
> > a0f8276fe814 ("compositor-drm: Disallow overlapping overlay planes") was
> > a little too pessimistic in rejecting occluded views. Whilst it
> > correctly prevented overlay planes from occluding each other, it also
> > prevented overlay planes from occluding the scanout plane.
> >
> > This is undesirable: the primary/scanout plane is specified to stack
> > strictly below all overlay planes, so there is no need to reject a plane
> > from consideration for scanout due to being occluded by an overlay
> > plane.
> >
> > Shift the check downwards so it only applies to overlay rather than
> > scanout planes.
>
> Reviewed-by: Pekka Paalanen 

Thankyou so much! The series is all landed now; Weston is fully atomic. :o

Enjoy your holiday - and to everyone else, enjoy a spam-free mailing list.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v21 2/4] compositor-drm: Allow scanout plane to be occluded by overlay

2018-07-11 Thread Daniel Stone
Hi,

On Wed, 11 Jul 2018 at 14:07, Pekka Paalanen  wrote:
> On Wed, 11 Jul 2018 13:16:18 +0100 Daniel Stone  wrote:
> > @@ -3426,17 +3426,6 @@ drm_output_propose_state(struct weston_output 
> > *output_base,
> >   if (pixman_region32_not_empty(_overlap))
> >   force_renderer = true;
> >
> > - /* We do not control the stacking order of overlay planes;
> > -  * the scanout plane is strictly stacked bottom and the cursor
> > -  * plane top, but the ordering of overlay planes with respect
> > -  * to each other is undefined. Make sure we do not have two
> > -  * planes overlapping each other. */
> > - pixman_region32_intersect(_overlap, _region,
> > -   _view);
> > - if (pixman_region32_not_empty(_overlap))
> > - force_renderer = true;
> > - pixman_region32_fini(_overlap);
> > -
> >   /* The cursor plane is 'special' in the sense that we can 
> > still
> >* place it in the legacy API, and we gate that with a 
> > separate
> >* cursors_are_broken flag. */
>
> Does this not introduce a failure mode that we get wrong z-order if an
> overlay was supposed to occlude a view that's otherwise eligible for
> the cursor plane?

It certainly does, which I seem to have noticed at the exact same time
as you. v21.1 replaces this with a separate 'view is partly occluded
by plane' bool: we don't attempt to place a cursor (cursor plane must
be above) or overlay (relative stacking order undefined) plane if this
bool is set, but we do let scanout go on.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v21.1 2/4] compositor-drm: Allow scanout plane to be occluded by overlay

2018-07-11 Thread Daniel Stone
a0f8276fe814 ("compositor-drm: Disallow overlapping overlay planes") was
a little too pessimistic in rejecting occluded views. Whilst it
correctly prevented overlay planes from occluding each other, it also
prevented overlay planes from occluding the scanout plane.

This is undesirable: the primary/scanout plane is specified to stack
strictly below all overlay planes, so there is no need to reject a plane
from consideration for scanout due to being occluded by an overlay
plane.

Shift the check downwards so it only applies to overlay rather than
scanout planes.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 3e70265c3..a99ac8eae 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3392,7 +3392,8 @@ drm_output_propose_state(struct weston_output 
*output_base,
struct drm_plane_state *ps = NULL;
bool force_renderer = false;
pixman_region32_t clipped_view;
-   bool occluded = false;
+   bool totally_occluded = false;
+   bool overlay_occluded = false;
 
/* If this view doesn't touch our output at all, there's no
 * reason to do anything with it. */
@@ -3416,8 +3417,8 @@ drm_output_propose_state(struct weston_output 
*output_base,
pixman_region32_init(_overlap);
pixman_region32_subtract(_overlap, _view,
 _region);
-   occluded = !pixman_region32_not_empty(_overlap);
-   if (occluded) {
+   totally_occluded = !pixman_region32_not_empty(_overlap);
+   if (totally_occluded) {
pixman_region32_fini(_overlap);
pixman_region32_fini(_view);
continue;
@@ -3439,13 +3440,13 @@ drm_output_propose_state(struct weston_output 
*output_base,
pixman_region32_intersect(_overlap, _region,
  _view);
if (pixman_region32_not_empty(_overlap))
-   force_renderer = true;
+   overlay_occluded = true;
pixman_region32_fini(_overlap);
 
/* The cursor plane is 'special' in the sense that we can still
 * place it in the legacy API, and we gate that with a separate
 * cursors_are_broken flag. */
-   if (!force_renderer && !b->cursors_are_broken)
+   if (!force_renderer && !overlay_occluded && 
!b->cursors_are_broken)
ps = drm_output_prepare_cursor_view(state, ev);
 
/* If sprites are disabled or the view is not fully opaque, we
@@ -3462,7 +3463,8 @@ drm_output_propose_state(struct weston_output 
*output_base,
 * scanout plane. */
if (!ps && !force_renderer && !renderer_ok)
ps = drm_output_prepare_scanout_view(state, ev, mode);
-   if (!ps && !force_renderer)
+
+   if (!ps && !overlay_occluded && !force_renderer)
ps = drm_output_prepare_overlay_view(state, ev, mode);
 
if (ps) {
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v21 1/4] compositor-drm: Incrementally test plane states in mixed mode

2018-07-11 Thread Daniel Stone
In the plane-only mode, we try to place every view on a hardware plane,
and fail if we can't do this. This requires a full walk of the scene
graph to come up with a complete configuration in order to be able to
test.

In mixed mode, we know at least some visible views will fail to be
promoted to planes and must be composited via the renderer. In order to
still use some planes where possible, we use atomic modesetting's
test-only mode to incrementally test configurations.

We know that the renderer output will always be visible, and because it
is the renderer, that it will be occupying the scanout plane underneath
everything else. The actual renderer buffer doesn't materialise until
after assign_planes, because it cannot know what to render until then.

However, in order to test whether a configuration is valid, we need the
renderer buffer in the scanout plane. For testing, we fake this by
temporarily stealing the old buffer - if it seems sufficiently
compatible - and placing it in the state we construct. This is used to
test whether or not a renderer buffer will work with the addition of
overlay planes.

Doing this incremental testing will allow us to enable plane usage for
atomic by default, since we know ahead of time that our chosen plane
configuration will work.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 118 +++--
 1 file changed, 86 insertions(+), 32 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 486fb71d9..d192eec16 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1949,7 +1949,8 @@ enum drm_output_propose_state_mode {
 
 static struct drm_plane_state *
 drm_output_prepare_scanout_view(struct drm_output_state *output_state,
-   struct weston_view *ev)
+   struct weston_view *ev,
+   enum drm_output_propose_state_mode mode)
 {
struct drm_output *output = output_state->output;
struct drm_backend *b = to_drm_backend(output->base.compositor);
@@ -1959,6 +1960,7 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
pixman_box32_t *extents;
 
assert(!b->sprites_are_broken);
+   assert(mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
 
/* Check the view spans exactly the output size, calculated in the
 * logical co-ordinate space. */
@@ -1983,15 +1985,13 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
}
 
state = drm_output_state_get_plane(output_state, scanout_plane);
-   if (state->fb) {
-   /* If there is already a framebuffer on the scanout plane,
-* a client view has already been placed on the scanout
-* view. In that case, do not free or put back the state,
-* but just leave it in place and quietly exit. */
-   drm_fb_unref(fb);
-   return NULL;
-   }
 
+   /* The only way we can already have a buffer in the scanout plane is
+* if we are in mixed mode, or if a client buffer has already been
+* placed into scanout. The former case will never call into here,
+* and in the latter case, the view must have been marked as occluded,
+* meaning we should never have ended up here. */
+   assert(!state->fb);
state->fb = fb;
state->ev = ev;
state->output = output;
@@ -2007,6 +2007,8 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
 
+   /* In plane-only mode, we don't need to test the state now, as we
+* will only test it once at the end. */
return state;
 
 err:
@@ -3049,7 +3051,8 @@ atomic_flip_handler(int fd, unsigned int frame, unsigned 
int sec,
 
 static struct drm_plane_state *
 drm_output_prepare_overlay_view(struct drm_output_state *output_state,
-   struct weston_view *ev)
+   struct weston_view *ev,
+   enum drm_output_propose_state_mode mode)
 {
struct drm_output *output = output_state->output;
struct weston_compositor *ec = output->base.compositor;
@@ -3058,6 +3061,7 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
struct drm_plane_state *state = NULL;
struct drm_fb *fb;
unsigned int i;
+   int ret;
 
assert(!b->sprites_are_broken);
 
@@ -3098,31 +3102,37 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
continue;
}
 
-   break;
-   }
+   state->ev = ev;
+   state->output = output;
+   drm_plane_state_coords_for_view(state, ev);
+   if (state->src_w 

[PATCH v21 0/4] DRM overlay plane testing and enablement

2018-07-11 Thread Daniel Stone
Hi,
After a long discussion on IRC, patch 1 has been extensively rewritten.
Firstly, the handling of temporarily placing the old renderer
framebuffer has been localised to drm_output_propose_state, reducing the
damage. Secondly, the viciously ugly state_old handling in
prepare_scanout_view has been ripped out completely. We can only have a
renderer framebuffer to preserve if we are in mixed mode, and we can
only be in mixed mode if we've been unable to use a client framebuffer
for scanout. Hence, there is no need for the complicated handling.

Patch 2 is new, noticed after doing some of my own testing.

Patches 3 and 4 are still unchanged, and still pretty trivial.

Cheers,
Daniel


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v21 4/4] compositor-drm: Enable planes for atomic

2018-07-11 Thread Daniel Stone
Now that we can sensibly test proposed plane configurations with atomic,
sprites are not broken.

Signed-off-by: Daniel Stone 
Reviewed-by: Pekka Paalanen 
---
 libweston/compositor-drm.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 9b769b59d..67aed7015 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3828,6 +3828,17 @@ init_kms_caps(struct drm_backend *b)
weston_log("DRM: %s atomic modesetting\n",
   b->atomic_modeset ? "supports" : "does not support");
 
+   /*
+* KMS support for hardware planes cannot properly synchronize
+* without nuclear page flip. Without nuclear/atomic, hw plane
+* and cursor plane updates would either tear or cause extra
+* waits for vblanks which means dropping the compositor framerate
+* to a fraction. For cursors, it's not so bad, so they are
+* enabled.
+*/
+   if (!b->atomic_modeset)
+   b->sprites_are_broken = 1;
+
ret = drmSetClientCap(b->drm.fd, DRM_CLIENT_CAP_ASPECT_RATIO, 1);
b->aspect_ratio_supported = (ret == 0);
weston_log("DRM: %s picture aspect ratio\n",
@@ -6708,17 +6719,6 @@ drm_backend_create(struct weston_compositor *compositor,
b->drm.fd = -1;
wl_array_init(>unused_crtcs);
 
-   /*
-* KMS support for hardware planes cannot properly synchronize
-* without nuclear page flip. Without nuclear/atomic, hw plane
-* and cursor plane updates would either tear or cause extra
-* waits for vblanks which means dropping the compositor framerate
-* to a fraction. For cursors, it's not so bad, so they are
-* enabled.
-*
-* These can be enabled again when nuclear/atomic support lands.
-*/
-   b->sprites_are_broken = 1;
b->compositor = compositor;
b->use_pixman = config->use_pixman;
b->pageflip_timeout = config->pageflip_timeout;
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v21 2/4] compositor-drm: Allow scanout plane to be occluded by overlay

2018-07-11 Thread Daniel Stone
a0f8276fe814 ("compositor-drm: Disallow overlapping overlay planes") was
a little too pessimistic in rejecting occluded views. Whilst it
correctly prevented overlay planes from occluding each other, it also
prevented overlay planes from occluding the scanout plane.

This is undesirable: the primary/scanout plane is specified to stack
strictly below all overlay planes, so there is no need to reject a plane
from consideration for scanout due to being occluded by an overlay
plane.

Shift the check downwards so it only applies to overlay rather than
scanout planes.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index d192eec16..3d1847790 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3426,17 +3426,6 @@ drm_output_propose_state(struct weston_output 
*output_base,
if (pixman_region32_not_empty(_overlap))
force_renderer = true;
 
-   /* We do not control the stacking order of overlay planes;
-* the scanout plane is strictly stacked bottom and the cursor
-* plane top, but the ordering of overlay planes with respect
-* to each other is undefined. Make sure we do not have two
-* planes overlapping each other. */
-   pixman_region32_intersect(_overlap, _region,
- _view);
-   if (pixman_region32_not_empty(_overlap))
-   force_renderer = true;
-   pixman_region32_fini(_overlap);
-
/* The cursor plane is 'special' in the sense that we can still
 * place it in the legacy API, and we gate that with a separate
 * cursors_are_broken flag. */
@@ -3457,6 +3446,18 @@ drm_output_propose_state(struct weston_output 
*output_base,
 * scanout plane. */
if (!ps && !force_renderer && !renderer_ok)
ps = drm_output_prepare_scanout_view(state, ev, mode);
+
+   /* We do not control the stacking order of overlay planes;
+* the scanout plane is strictly stacked bottom and the cursor
+* plane top, but the ordering of overlay planes with respect
+* to each other is undefined. Make sure we do not have two
+* planes overlapping each other. */
+   pixman_region32_intersect(_overlap, _region,
+ _view);
+   if (pixman_region32_not_empty(_overlap))
+   force_renderer = true;
+   pixman_region32_fini(_overlap);
+
if (!ps && !force_renderer)
ps = drm_output_prepare_overlay_view(state, ev, mode);
 
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 3/4] simple-dmabuf-drm: use GBM generic calls

2018-07-11 Thread Daniel Stone
Hi Emilio,

On Wed, 11 Jul 2018 at 12:53, Emilio Pozuelo Monfort  wrote:
> As for NV12 support: I tried to make gbm support that in
> backends/dri/gbm_dri.c by adding a mapping to gbm_dri_visuals_table
> from GBM_FORMAT_NV12 to __DRI_IMAGE_FOURCC_NV12 or DRM_FORMAT_NV12,
> but that didn't work. I'm not sure what we can do here. Does anybody
> who know the stack better have any pointers?

There is no native NV12 support, apart form some special-case handling
when using them as textures. For NV12, you would want to create two
separate BOs - one with GBM_FORMAT_R8 (luma/Y), and one with
GBM_FORMAT_RG88 (chroma/UV), and fill those like you would fill the
two planes of a native NV12 surface. They can then be combined and
sent to the compositor as two planes of a single NV12 image.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v20 05/10] compositor-drm: Add test-only mode to state application

2018-07-11 Thread Daniel Stone
Hi,

On Wed, 11 Jul 2018 at 12:43, Pekka Paalanen  wrote:
> On Wed, 11 Jul 2018 11:41:29 +0100 Daniel Stone  wrote:
> > @@ -2610,9 +2612,18 @@ drm_pending_state_apply_atomic(struct 
> > drm_pending_state *pending_state,
> >   case DRM_STATE_APPLY_ASYNC:
> >   flags |= DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
> >   break;
> > + case DRM_STATE_TEST_ONLY:
> > + flags |= DRM_MODE_ATOMIC_TEST_ONLY;
> > + break;
> >   }
> >
> >   ret = drmModeAtomicCommit(b->drm.fd, req, flags, b);
> > +
> > + if (mode == DRM_STATE_TEST_ONLY) {
> > + drmModeAtomicFree(req);
> > + return ret;
>
> In this case this function did not take ownership of pending_state,
> right? That's different from all earlier behaviours. I'm thinking of
> comments, if anything more would need updating. The below is probably
> good already.

Right you are, but a comment definitely seems worthwhile. (And yes,
deleting the old comment was certainly a mistake ...)

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v20 09/10] compositor-drm: Relax plane restrictions for atomic

2018-07-11 Thread Daniel Stone
Since we now incrementally test atomic state as we build it, we can
loosen restrictions on what we can do with planes, and let the kernel
tell us whether or not it's OK.

Signed-off-by: Daniel Stone 
Reviewed-by: Pekka Paalanen 
---
 libweston/compositor-drm.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index d664a8df5..284385d10 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1980,7 +1980,7 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
return NULL;
 
/* Can't change formats with just a pageflip */
-   if (fb->format->format != output->gbm_format) {
+   if (!b->atomic_modeset && fb->format->format != output->gbm_format) {
drm_fb_unref(fb);
return NULL;
}
@@ -2010,15 +2010,18 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
if (!drm_plane_state_coords_for_view(state, ev))
goto err;
 
-   /* The legacy API does not let us perform cropping or scaling. */
-   if (state->src_x != 0 || state->src_y != 0 ||
-   state->src_w != state->dest_w << 16 ||
-   state->src_h != state->dest_h << 16 ||
-   state->dest_x != 0 || state->dest_y != 0 ||
+   if (state->dest_x != 0 || state->dest_y != 0 ||
state->dest_w != (unsigned) output->base.current_mode->width ||
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
 
+   /* The legacy API does not let us perform cropping or scaling. */
+   if (!b->atomic_modeset &&
+   (state->src_x != 0 || state->src_y != 0 ||
+state->src_w != state->dest_w << 16 ||
+state->src_h != state->dest_h << 16))
+   goto err;
+
/* In plane-only mode, we don't need to test the state now, as we
 * will only test it once at the end. */
if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) {
@@ -3126,8 +3129,9 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
state->ev = ev;
state->output = output;
drm_plane_state_coords_for_view(state, ev);
-   if (state->src_w != state->dest_w << 16 ||
-   state->src_h != state->dest_h << 16) {
+   if (!b->atomic_modeset &&
+   (state->src_w != state->dest_w << 16 ||
+state->src_h != state->dest_h << 16)) {
drm_plane_state_put_back(state);
continue;
}
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v20 08/10] compositor-drm: Incrementally test plane states in mixed mode

2018-07-11 Thread Daniel Stone
In the plane-only mode, we try to place every view on a hardware plane,
and fail if we can't do this. This requires a full walk of the scene
graph to come up with a complete configuration in order to be able to
test.

In mixed mode, we know at least some visible views will fail to be
promoted to planes and must be composited via the renderer. In order to
still use some planes where possible, we use atomic modesetting's
test-only mode to incrementally test configurations.

We know that the renderer output will always be visible, and because it
is the renderer, that it will be occupying the scanout plane underneath
everything else. The actual renderer buffer doesn't materialise until
after assign_planes, because it cannot know what to render until then.

However, in order to test whether a configuration is valid, we need the
renderer buffer in the scanout plane. For testing, we fake this by
temporarily stealing the old buffer - if it seems sufficiently
compatible - and placing it in the state we construct. This is used to
test whether or not a renderer buffer will work with the addition of
overlay planes.

Doing this incremental testing will allow us to enable plane usage for
atomic by default, since we know ahead of time that our chosen plane
configuration will work.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 133 -
 1 file changed, 102 insertions(+), 31 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index d4555da78..d664a8df5 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1949,14 +1949,17 @@ enum drm_output_propose_state_mode {
 
 static struct drm_plane_state *
 drm_output_prepare_scanout_view(struct drm_output_state *output_state,
-   struct weston_view *ev)
+   struct weston_view *ev,
+   enum drm_output_propose_state_mode mode)
 {
struct drm_output *output = output_state->output;
struct drm_backend *b = to_drm_backend(output->base.compositor);
struct drm_plane *scanout_plane = output->scanout_plane;
struct drm_plane_state *state;
+   struct drm_plane_state *state_old = NULL;
struct drm_fb *fb;
pixman_box32_t *extents;
+   int ret;
 
assert(!b->sprites_are_broken);
 
@@ -1983,11 +1986,20 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
}
 
state = drm_output_state_get_plane(output_state, scanout_plane);
-   if (state->fb) {
-   /* If there is already a framebuffer on the scanout plane,
-* a client view has already been placed on the scanout
-* view. In that case, do not free or put back the state,
-* but just leave it in place and quietly exit. */
+
+   /* Check if we've already placed a buffer on this plane; if there's a
+* buffer there but it comes from GBM, then it's the result of
+* drm_output_propose_state placing it here for testing purposes.
+* If we already have another client buffer, then we can't use this
+* plane. */
+   if (state->fb &&
+   (state->fb->type == BUFFER_GBM_SURFACE ||
+state->fb->type == BUFFER_PIXMAN_DUMB)) {
+   state_old = calloc(1, sizeof(*state_old));
+   memcpy(state_old, state, sizeof(*state_old));
+   state_old->output_state = NULL;
+   wl_list_init(_old->link);
+   } else if (state->fb) {
drm_fb_unref(fb);
return NULL;
}
@@ -2007,10 +2019,26 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
 
+   /* In plane-only mode, we don't need to test the state now, as we
+* will only test it once at the end. */
+   if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) {
+   drm_plane_state_free(state_old, false);
+   return state;
+   }
+
+   ret = drm_pending_state_test(output_state->pending_state);
+   if (ret != 0)
+   goto err;
+
+   drm_plane_state_free(state_old, false);
return state;
 
 err:
-   drm_plane_state_put_back(state);
+   drm_plane_state_free(state, false);
+   if (state_old) {
+   wl_list_insert(_state->plane_list, _old->link);
+   state_old->output_state = output_state;
+   }
return NULL;
 }
 
@@ -2078,7 +2106,9 @@ drm_output_render(struct drm_output_state *state, 
pixman_region32_t *damage)
 * want to render. */
scanout_state = drm_output_state_get_plane(state,
   output->scanout_plane);
-   if (scanout_state->fb)
+   if (scanout_state->fb &&
+

[PATCH v20 04/10] compositor-drm: Return plane state from plane preparation

2018-07-11 Thread Daniel Stone
Return a pointer to the plane state, rather than returning its
underlying weston_plane.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 63 +++---
 1 file changed, 31 insertions(+), 32 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 56e7e5d1f..4bf18581b 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1944,7 +1944,7 @@ enum drm_output_propose_state_mode {
DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer & 
cursor */
 };
 
-static struct weston_plane *
+static struct drm_plane_state *
 drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
 {
@@ -2004,7 +2004,7 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
 
-   return _plane->base;
+   return state;
 
 err:
drm_plane_state_put_back(state);
@@ -2170,7 +2170,6 @@ drm_output_apply_state_legacy(struct drm_output_state 
*state)
struct drm_property_info *dpms_prop;
struct drm_plane_state *scanout_state;
struct drm_plane_state *ps;
-   struct drm_plane *p;
struct drm_mode *mode;
struct drm_head *head;
uint32_t connectors[MAX_CLONED_CONNECTORS];
@@ -2197,7 +2196,7 @@ drm_output_apply_state_legacy(struct drm_output_state 
*state)
 
if (state->dpms != WESTON_DPMS_ON) {
wl_list_for_each(ps, >plane_list, link) {
-   p = ps->plane;
+   struct drm_plane *p = ps->plane;
assert(ps->fb == NULL);
assert(ps->output == NULL);
 
@@ -2287,8 +2286,8 @@ drm_output_apply_state_legacy(struct drm_output_state 
*state)
.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT,
.request.sequence = 1,
};
+   struct drm_plane *p = ps->plane;
 
-   p = ps->plane;
if (p->type != WDRM_PLANE_TYPE_OVERLAY)
continue;
 
@@ -3000,7 +2999,7 @@ atomic_flip_handler(int fd, unsigned int frame, unsigned 
int sec,
 }
 #endif
 
-static struct weston_plane *
+static struct drm_plane_state *
 drm_output_prepare_overlay_view(struct drm_output_state *output_state,
struct weston_view *ev)
 {
@@ -3071,7 +3070,7 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
state->src_h != state->dest_h << 16)
goto err;
 
-   return >base;
+   return state;
 
 err:
drm_plane_state_put_back(state);
@@ -3115,7 +3114,7 @@ cursor_bo_update(struct drm_plane_state *plane_state, 
struct weston_view *ev)
weston_log("failed update cursor: %m\n");
 }
 
-static struct weston_plane *
+static struct drm_plane_state *
 drm_output_prepare_cursor_view(struct drm_output_state *output_state,
   struct weston_view *ev)
 {
@@ -3201,7 +3200,7 @@ drm_output_prepare_cursor_view(struct drm_output_state 
*output_state,
plane_state->dest_w = b->cursor_width;
plane_state->dest_h = b->cursor_height;
 
-   return >base;
+   return plane_state;
 
 err:
drm_plane_state_put_back(plane_state);
@@ -3271,7 +3270,6 @@ drm_output_propose_state(struct weston_output 
*output_base,
struct drm_output_state *state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
-   struct weston_plane *primary = _base->compositor->primary_plane;
bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
 
assert(!output->state_last);
@@ -3296,7 +3294,8 @@ drm_output_propose_state(struct weston_output 
*output_base,
pixman_region32_init(_region);
 
wl_list_for_each(ev, _base->compositor->view_list, link) {
-   struct weston_plane *next_plane = NULL;
+   struct drm_plane_state *ps = NULL;
+   bool force_renderer = false;
pixman_region32_t clipped_view;
bool occluded = false;
 
@@ -3308,7 +3307,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
/* We only assign planes to views which are exclusively present
 * on our output. */
if (ev->output_mask != (1u << output->base.id))
-   next_plane = primary;
+   force_renderer = true;
 
/* Ignore views we know to be totally occluded. */
pixman_region32_init(_view);
@@ -3332,7 +3331,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
pixman_region32_intersect(_overlap, 

[PATCH v20 10/10] compositor-drm: Enable planes for atomic

2018-07-11 Thread Daniel Stone
Now that we can sensibly test proposed plane configurations with atomic,
sprites are not broken.

Signed-off-by: Daniel Stone 
Reviewed-by: Pekka Paalanen 
---
 libweston/compositor-drm.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 284385d10..a6a349689 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3840,6 +3840,17 @@ init_kms_caps(struct drm_backend *b)
weston_log("DRM: %s atomic modesetting\n",
   b->atomic_modeset ? "supports" : "does not support");
 
+   /*
+* KMS support for hardware planes cannot properly synchronize
+* without nuclear page flip. Without nuclear/atomic, hw plane
+* and cursor plane updates would either tear or cause extra
+* waits for vblanks which means dropping the compositor framerate
+* to a fraction. For cursors, it's not so bad, so they are
+* enabled.
+*/
+   if (!b->atomic_modeset)
+   b->sprites_are_broken = 1;
+
ret = drmSetClientCap(b->drm.fd, DRM_CLIENT_CAP_ASPECT_RATIO, 1);
b->aspect_ratio_supported = (ret == 0);
weston_log("DRM: %s picture aspect ratio\n",
@@ -6720,17 +6731,6 @@ drm_backend_create(struct weston_compositor *compositor,
b->drm.fd = -1;
wl_array_init(>unused_crtcs);
 
-   /*
-* KMS support for hardware planes cannot properly synchronize
-* without nuclear page flip. Without nuclear/atomic, hw plane
-* and cursor plane updates would either tear or cause extra
-* waits for vblanks which means dropping the compositor framerate
-* to a fraction. For cursors, it's not so bad, so they are
-* enabled.
-*
-* These can be enabled again when nuclear/atomic support lands.
-*/
-   b->sprites_are_broken = 1;
b->compositor = compositor;
b->use_pixman = config->use_pixman;
b->pageflip_timeout = config->pageflip_timeout;
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v20 03/10] compositor-drm: Add modes to drm_output_propose_state

2018-07-11 Thread Daniel Stone
Add support for multiple modes to drm_output_propose_state. Currently we
intend to operate in three modes: planes-only (no renderer buffer,
client buffers in planes only), mixed-mode (promote client buffers to
planes where possible, falling back to the renderer where not), and
renderer-only (no plane usage at all).

We want to use the first (planes-only) mode where possible: it can avoid
us having to allocate buffers for the renderer, and it also gives us the
best chance of the optimal configuration, with no composition. In this
mode, we walk the scene looking at all views, trying to put them in
planes, and failing as soon as we find a view we cannot place in a
plane.

In the second mode, rather than failing, we assign those views which
cannot be on a plane to the renderer, and allow the renderer to
composite them.

In the third mode, planes are not usable, so everything but the cursor
goes to the renderer. We will use this when we cannot use the planes-only
mode (because some views cannot be placed in planes), but also cannot
use the 'mixed' mode because we have no renderer buffer yet. Since we
walk the scene graph from top to bottom, using atomic modesetting we
will determine if planes can be promoted in mixed mode by placing a
renderer buffer at the bottom of the scene, placing a cursor buffer if
applicable, then testing if we can add overlay planes to this mode.

Without a buffer from the renderer, we cannot do these tests, so we push
everything through the renderer and then switch to mixed mode on the
next repaint.

This patch implements the mixed and renderer-only modes (previously
differentiated only by the sprites_are_broken flag), with the
planes-only mode being left for a later patch.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 70 --
 1 file changed, 45 insertions(+), 25 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index d045778aa..56e7e5d1f 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1939,6 +1939,11 @@ drm_output_assign_state(struct drm_output_state *state,
}
 }
 
+enum drm_output_propose_state_mode {
+   DRM_OUTPUT_PROPOSE_STATE_MIXED, /**< mix renderer & planes */
+   DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer & 
cursor */
+};
+
 static struct weston_plane *
 drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
@@ -3121,10 +3126,9 @@ drm_output_prepare_cursor_view(struct drm_output_state 
*output_state,
struct wl_shm_buffer *shmbuf;
bool needs_update = false;
 
-   if (!plane)
-   return NULL;
+   assert(!b->cursors_are_broken);
 
-   if (b->cursors_are_broken)
+   if (!plane)
return NULL;
 
if (!plane->state_cur->complete)
@@ -3259,7 +3263,8 @@ err:
 
 static struct drm_output_state *
 drm_output_propose_state(struct weston_output *output_base,
-struct drm_pending_state *pending_state)
+struct drm_pending_state *pending_state,
+enum drm_output_propose_state_mode mode)
 {
struct drm_output *output = to_drm_output(output_base);
struct drm_backend *b = to_drm_backend(output->base.compositor);
@@ -3267,6 +3272,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
struct weston_plane *primary = _base->compositor->primary_plane;
+   bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
 
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
@@ -3339,13 +3345,16 @@ drm_output_propose_state(struct weston_output 
*output_base,
next_plane = primary;
pixman_region32_fini(_overlap);
 
-   if (next_plane == NULL)
+   /* The cursor plane is 'special' in the sense that we can still
+* place it in the legacy API, and we gate that with a separate
+* cursors_are_broken flag. */
+   if (next_plane == NULL && !b->cursors_are_broken)
next_plane = drm_output_prepare_cursor_view(state, ev);
 
if (next_plane == NULL && !drm_view_is_opaque(ev))
next_plane = primary;
 
-   if (next_plane == NULL && b->sprites_are_broken)
+   if (next_plane == NULL && !planes_ok)
next_plane = primary;
 
if (next_plane == NULL)
@@ -3354,24 +3363,27 @@ drm_output_propose_state(struct weston_output 
*output_base,
if (next_plane == NULL)
next_plane = drm_output_prepare_overlay_view(state, ev);
 
-

[PATCH v20 05/10] compositor-drm: Add test-only mode to state application

2018-07-11 Thread Daniel Stone
The atomic API can allow us to test state before we apply it, to see if
it will be valid. Use this when we construct a plane configuration, to
see if it has a chance of ever working. If not, we can fail
assign_planes early.

This will be used in later patches to incrementally build state by
proposing and testing potential configurations one at a time.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 56 +++---
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 4bf18581b..65079eac0 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -260,6 +260,7 @@ enum drm_output_state_duplicate_mode {
 enum drm_state_apply_mode {
DRM_STATE_APPLY_SYNC, /**< state fully processed */
DRM_STATE_APPLY_ASYNC, /**< state pending event delivery */
+   DRM_STATE_TEST_ONLY, /**< test if the state can be applied */
 };
 
 struct drm_backend {
@@ -1822,6 +1823,7 @@ drm_pending_state_get_output(struct drm_pending_state 
*pending_state,
 }
 
 static int drm_pending_state_apply_sync(struct drm_pending_state *state);
+static int drm_pending_state_test(struct drm_pending_state *state);
 
 /**
  * Mark a drm_output_state (the output's last state) as complete. This handles
@@ -2610,9 +2612,18 @@ drm_pending_state_apply_atomic(struct drm_pending_state 
*pending_state,
case DRM_STATE_APPLY_ASYNC:
flags |= DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
break;
+   case DRM_STATE_TEST_ONLY:
+   flags |= DRM_MODE_ATOMIC_TEST_ONLY;
+   break;
}
 
ret = drmModeAtomicCommit(b->drm.fd, req, flags, b);
+
+   if (mode == DRM_STATE_TEST_ONLY) {
+   drmModeAtomicFree(req);
+   return ret;
+   }
+
if (ret != 0) {
weston_log("atomic: couldn't commit new state: %m\n");
goto out;
@@ -2634,12 +2645,39 @@ out:
 #endif
 
 /**
- * Applies all of a pending_state asynchronously: the primary entry point for
- * applying KMS state to a device. Updates the state for all outputs in the
- * pending_state, as well as disabling any unclaimed outputs.
+ * Tests a pending state, to see if the kernel will accept the update as
+ * constructed.
  *
- * Unconditionally takes ownership of pending_state, and clears state_invalid.
+ * Using atomic modesetting, the kernel performs the same checks as it would
+ * on a real commit, returning success or failure without actually modifying
+ * the running state. It does not return -EBUSY if there are pending updates
+ * in flight, so states may be tested at any point, however this means a
+ * state which passed testing may fail on a real commit if the timing is not
+ * respected (e.g. committing before the previous commit has completed).
+ *
+ * Without atomic modesetting, we have no way to check, so we optimistically
+ * claim it will work.
+ *
+ * Unlike drm_pending_state_apply() and drm_pending_state_apply_sync(), this
+ * function does _not_ take ownership of pending_state, nor does it clear
+ * state_invalid.
  */
+static int
+drm_pending_state_test(struct drm_pending_state *pending_state)
+{
+#ifdef HAVE_DRM_ATOMIC
+   struct drm_backend *b = pending_state->backend;
+
+   if (b->atomic_modeset)
+   return drm_pending_state_apply_atomic(pending_state,
+ DRM_STATE_TEST_ONLY);
+#endif
+
+   /* We have no way to test state before application on the legacy
+* modesetting API, so just claim it succeeded. */
+   return 0;
+}
+
 static int
 drm_pending_state_apply(struct drm_pending_state *pending_state)
 {
@@ -3271,6 +3309,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
+   int ret;
 
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
@@ -3388,7 +3427,16 @@ drm_output_propose_state(struct weston_output 
*output_base,
pixman_region32_fini(_region);
pixman_region32_fini(_region);
 
+   /* Check to see if this state will actually work. */
+   ret = drm_pending_state_test(state->pending_state);
+   if (ret != 0)
+   goto err;
+
return state;
+
+err:
+   drm_output_state_free(state);
+   return NULL;
 }
 
 static void
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v20 01/10] compositor-drm: Disallow overlapping overlay planes

2018-07-11 Thread Daniel Stone
The scanout plane strictly stacks under all overlay planes, and the
cursor plane above. However, the stacking of overlay planes with respect
to each other is undefined.

We can control the stacking order of overlay planes with the zpos
property, though this significantly complicates plane assignment. In the
meantime, simply disallow assigning a view to an overlay, when it
overlaps another view which is already on an overlay. This ensures
stacking order is irrelevant, since the planes never intersect each
other.

Signed-off-by: Daniel Stone 
Reviewed-by: Pekka Paalanen 
---
 libweston/compositor-drm.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 51b2482d9..2305f708f 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3324,6 +3324,16 @@ drm_output_propose_state(struct weston_output 
*output_base,
  _view);
if (pixman_region32_not_empty(_overlap))
next_plane = primary;
+
+   /* We do not control the stacking order of overlay planes;
+* the scanout plane is strictly stacked bottom and the cursor
+* plane top, but the ordering of overlay planes with respect
+* to each other is undefined. Make sure we do not have two
+* planes overlapping each other. */
+   pixman_region32_intersect(_overlap, _region,
+ _view);
+   if (pixman_region32_not_empty(_overlap))
+   next_plane = primary;
pixman_region32_fini(_overlap);
 
if (next_plane == NULL)
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v20 07/10] compositor-drm: Add planes-only mode to state proposal

2018-07-11 Thread Daniel Stone
Add a new mode, which attempts to construct a scene exclusively using
planes. This is a building block for incrementally testing and
constructing state: in the plane-only mode, we test the state exactly
once, when we have constructed a full set of planes and want to know if
it works or not.

When using the renderer, we need to incrementally test views one by one
to see if they will work on planes, falling back to the renderer if not.
This test is different, since the scanout plane will be occupied by the
renderer's buffer. Testing using the renderer or client buffers may have
completely different characteristics, so we need two passes: first,
constructing a state with only planes and testing if that succeeds,
falling back later to a mixed renderer/plane mode which tests
incrementally.

This implements the first mode, and preferentially attempts to use it.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 26 --
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index c7ca6ef79..d4555da78 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1944,6 +1944,7 @@ drm_output_assign_state(struct drm_output_state *state,
 enum drm_output_propose_state_mode {
DRM_OUTPUT_PROPOSE_STATE_MIXED, /**< mix renderer & planes */
DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer & 
cursor */
+   DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY, /**< no renderer use, only planes 
*/
 };
 
 static struct drm_plane_state *
@@ -3309,6 +3310,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
+   bool renderer_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
int ret;
 
assert(!output->state_last);
@@ -3400,6 +3402,11 @@ drm_output_propose_state(struct weston_output 
*output_base,
if (!ps && !drm_view_is_opaque(ev))
force_renderer = true;
 
+   if (force_renderer && !renderer_ok) {
+   pixman_region32_fini(_view);
+   goto err_region;
+   }
+
if (!ps && !force_renderer)
ps = drm_output_prepare_scanout_view(state, ev);
if (!ps && !force_renderer)
@@ -3422,6 +3429,14 @@ drm_output_propose_state(struct weston_output 
*output_base,
continue;
}
 
+   /* We have been assigned to the primary (renderer) plane:
+* check if this is OK, and add ourselves to the renderer
+* region if so. */
+   if (!renderer_ok) {
+   pixman_region32_fini(_view);
+   goto err_region;
+   }
+
pixman_region32_union(_region,
  _region,
  _view);
@@ -3437,6 +3452,9 @@ drm_output_propose_state(struct weston_output 
*output_base,
 
return state;
 
+err_region:
+   pixman_region32_fini(_region);
+   pixman_region32_fini(_region);
 err:
drm_output_state_free(state);
return NULL;
@@ -3453,9 +3471,13 @@ drm_assign_planes(struct weston_output *output_base, 
void *repaint_data)
struct weston_view *ev;
struct weston_plane *primary = _base->compositor->primary_plane;
 
-   if (!b->sprites_are_broken)
+   if (!b->sprites_are_broken) {
state = drm_output_propose_state(output_base, pending_state,
-
DRM_OUTPUT_PROPOSE_STATE_MIXED);
+
DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
+   if (!state)
+   state = drm_output_propose_state(output_base, 
pending_state,
+
DRM_OUTPUT_PROPOSE_STATE_MIXED);
+   }
 
if (!state)
state = drm_output_propose_state(output_base, pending_state,
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v20 02/10] compositor-drm: Use sprites_are_broken for scanout plane

2018-07-11 Thread Daniel Stone
When the sprites_are_broken variable is set, do not attempt to promote
client surfaces to the scanout plane.

We are currently assuming that every client buffer will be compatible
with the scanout plane, but that is not the case, particularly with more
exotic tiled/compressed buffers. Once we promote the client buffer to
scanout, there is no going back: if the repaint fails, we do not mark
this as failed and go back to repaint through composition.

This permanently removes the ability for scanout bypass when using the
non-atomic path. Future patches lift the restriction when using atomic
modesetting, as we can actually test and ensure that the view is
compatible with scanout.

Signed-off-by: Daniel Stone 
Reviewed-by: Pekka Paalanen 
Reported-by: Pekka Paalanen 
---
 libweston/compositor-drm.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 2305f708f..d045778aa 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1944,11 +1944,14 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
struct weston_view *ev)
 {
struct drm_output *output = output_state->output;
+   struct drm_backend *b = to_drm_backend(output->base.compositor);
struct drm_plane *scanout_plane = output->scanout_plane;
struct drm_plane_state *state;
struct drm_fb *fb;
pixman_box32_t *extents;
 
+   assert(!b->sprites_are_broken);
+
/* Check the view spans exactly the output size, calculated in the
 * logical co-ordinate space. */
extents = pixman_region32_extents(>transform.boundingbox);
@@ -3004,8 +3007,7 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
struct drm_fb *fb;
unsigned int i;
 
-   if (b->sprites_are_broken)
-   return NULL;
+   assert(!b->sprites_are_broken);
 
fb = drm_fb_get_from_view(output_state, ev);
if (!fb)
@@ -3260,6 +3262,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
 struct drm_pending_state *pending_state)
 {
struct drm_output *output = to_drm_output(output_base);
+   struct drm_backend *b = to_drm_backend(output->base.compositor);
struct drm_output_state *state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
@@ -3342,6 +3345,9 @@ drm_output_propose_state(struct weston_output 
*output_base,
if (next_plane == NULL && !drm_view_is_opaque(ev))
next_plane = primary;
 
+   if (next_plane == NULL && b->sprites_are_broken)
+   next_plane = primary;
+
if (next_plane == NULL)
next_plane = drm_output_prepare_scanout_view(state, ev);
 
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v20 00/10] DRM backend planes

2018-07-11 Thread Daniel Stone
Hi,
Another revision after Pekka's review. There is a memory leak fix in
testing atomic commits, as well as cosmetic changes inside
drm_output_propose_state.

The largest change is that if we enter drm_output_propose_state in
renderer-only mode, we no longer return early. This matches the
documentation promise of RENDERER_ONLY, which is that we will still
attempt placement on cursor planes.

Cheers,
Daniel


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v20 06/10] compositor-drm: Never lift solid surfaces to planes

2018-07-11 Thread Daniel Stone
This will never work, so don't even try to do it.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 65079eac0..c7ca6ef79 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3348,6 +3348,9 @@ drm_output_propose_state(struct weston_output 
*output_base,
if (ev->output_mask != (1u << output->base.id))
force_renderer = true;
 
+   if (!ev->surface->buffer_ref.buffer)
+   force_renderer = true;
+
/* Ignore views we know to be totally occluded. */
pixman_region32_init(_view);
pixman_region32_intersect(_view,
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v19 05/10] compositor-drm: Add test-only mode to state application

2018-07-11 Thread Daniel Stone
Hi,
On Wed, 11 Jul 2018 at 11:20, Pekka Paalanen  wrote:
> On Tue, 10 Jul 2018 18:58:44 +0100 Daniel Stone  wrote:
> > @@ -2610,9 +2612,16 @@ drm_pending_state_apply_atomic(struct 
> > drm_pending_state *pending_state,
> >   case DRM_STATE_APPLY_ASYNC:
> >   flags |= DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
> >   break;
> > + case DRM_STATE_TEST_ONLY:
> > + flags |= DRM_MODE_ATOMIC_TEST_ONLY;
> > + break;
> >   }
> >
> >   ret = drmModeAtomicCommit(b->drm.fd, req, flags, b);
> > +
> > + if (mode == DRM_STATE_TEST_ONLY)
> > + return ret;
>
> This leaks at least 'req'.

Right, I found that before re-sending for v20 and fixed locally.

> > @@ -2640,6 +2649,22 @@ out:
> >   *
> >   * Unconditionally takes ownership of pending_state, and clears 
> > state_invalid.
> >   */
>
> The documentation above is misplaced now. drm_pending_state_test()
> needs new docs, particularly about what it is supposed to do with
> pending_state.

Done.

> An idea for the future: maybe 'req' could be cached with the
> pending_state if it passed the test? To avoid having to build the same
> working 'req' twice every frame.

I did think about it but decided it probably wasn't particularly
valuable. drmModeAtomicReq is just a dumb container:
drmModeAtomicAddProperty adds (obj,prop,value) to a list, increments
the serial, and returns. The 'expensive' work is done in
drmModeAtomicCommit, which collates that list into a set of final
values before submission to the kernel. This is done so that an atomic
request can have a list of properties which potentially overwrite each
other, but can then be 'rewound' with drmModeAtomicSetCursor in order
to do non-destructive testing. It also complicates our tracking
somewhat, since we'd need to track the dirty status of a pending_state
everywhere we modified its child states.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v19 03/10] compositor-drm: Add modes to drm_output_propose_state

2018-07-11 Thread Daniel Stone
Hi Pekka,
On Wed, 11 Jul 2018 at 10:21, Pekka Paalanen  wrote:
> On Tue, 10 Jul 2018 18:58:42 +0100 Daniel Stone  wrote:
> > @@ -3339,13 +3347,16 @@ drm_output_propose_state(struct weston_output 
> > *output_base,
> >   next_plane = primary;
> >   pixman_region32_fini(_overlap);
> >
> > - if (next_plane == NULL)
> > + /* The cursor plane is 'special' in the sense that we can 
> > still
> > +  * place it in the legacy API, and we gate that with a 
> > separate
> > +  * cursors_are_broken flag. */
> > + if (next_plane == NULL && !b->cursors_are_broken)
> >   next_plane = drm_output_prepare_cursor_view(state, 
> > ev);
> >
> >   if (next_plane == NULL && !drm_view_is_opaque(ev))
> >   next_plane = primary;
> >
> > - if (next_plane == NULL && b->sprites_are_broken)
> > + if (next_plane == NULL && !planes_ok)
>
> planes_ok is guaranteed to be true.

Thinking about it, adding an early return from 'planes_ok' is an
unwanted behaviour change, since it also disables cursor usage. With
the other three fixed, and the planes_ok early-return removed, would
your R-b still stand?

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v19 05/10] compositor-drm: Add test-only mode to state application

2018-07-11 Thread Daniel Stone
On Tue, 10 Jul 2018 at 18:59, Daniel Stone  wrote:
> @@ -3388,7 +3414,18 @@ drm_output_propose_state(struct weston_output 
> *output_base,
> pixman_region32_fini(_region);
> pixman_region32_fini(_region);
>
> +   /* Check to see if this state will actually work. */
> +   ret = drm_pending_state_test(state->pending_state);
> +   if (ret != 0)
> +   goto err;
> +
> return state;
> +
> +err:
> +   pixman_region32_fini(_region);
> +   pixman_region32_fini(_region);
> +   drm_output_state_free(state);
> +   return NULL;
>  }

Looking at this fresh, this err jump should jump to freeing the state
and returning NULL, not finishing the regions. The region free is only
needed by 'Add planes-only mode to state proposal', which can bail out
inside the per-view loop.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] simple-dmabuf-drm: use GBM generic calls

2018-07-11 Thread Daniel Stone
Hi,
On Wed, 11 Jul 2018 at 09:16, Pekka Paalanen  wrote:
> On Tue, 10 Jul 2018 17:53:15 +0200 Emilio Pozuelo Monfort  
> wrote:
> > No need to write libdrm driver specific code for each supported
> > driver, we can just let GBM call the right one for us now.
> >
> > Signed-off-by: Emilio Pozuelo Monfort 
> > ---
> >
> > Hi,
> >
> > This simplifies the code a lot, using gbm_bo as Emil suggested. Some 
> > problems
> > I still see:
> >
> > - NV12 doesn't work, it seems that 
> > backends/dri/gbm_dri.c:gbm_format_to_dri_format()
> >   doesn't support it.
>
> I think NV12 and other less common formats, should someone add them in
> this program, should not be lost. That may be part of the reason GBM
> wasn't used: the need to test YUV formats, and non-GPU-renderable
> formats in general.

Honestly, most of the reason was because I wrote the original client
in about 15 minutes on a very short bus trip. You can probably see
that in comments like 'XXX: would be nice to draw something that
changes here'. So I don't think you can really read too much into the
original code.

Other great reasons include:
  - we wanted to explicitly get modifiers allocated, and this was
before the GBM modifiers interface existed
  - at the time, I was doing bring-up of a GBM implementation which
wasn't usable by generic clients (privileged allocation only)
  - YUV format support

I'm pretty ambivalent about it though. V4L2 and Vivid might well cover
the YUV case well enough, and even if not, GBM should still be able to
allocate R/RG buffers.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v19 08/10] compositor-drm: Incrementally test plane states in mixed mode

2018-07-10 Thread Daniel Stone
In the plane-only mode, we try to place every view on a hardware plane,
and fail if we can't do this. This requires a full walk of the scene
graph to come up with a complete configuration in order to be able to
test.

In mixed mode, we know at least some visible views will fail to be
promoted to planes and must be composited via the renderer. In order to
still use some planes where possible, we use atomic modesetting's
test-only mode to incrementally test configurations.

We know that the renderer output will always be visible, and because it
is the renderer, that it will be occupying the scanout plane underneath
everything else. The actual renderer buffer doesn't materialise until
after assign_planes, because it cannot know what to render until then.

However, in order to test whether a configuration is valid, we need the
renderer buffer in the scanout plane. For testing, we fake this by
temporarily stealing the old buffer - if it seems sufficiently
compatible - and placing it in the state we construct. This is used to
test whether or not a renderer buffer will work with the addition of
overlay planes.

Doing this incremental testing will allow us to enable plane usage for
atomic by default, since we know ahead of time that our chosen plane
configuration will work.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 133 -
 1 file changed, 102 insertions(+), 31 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 3a627dd34..ab3eea6cf 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1949,14 +1949,17 @@ enum drm_output_propose_state_mode {
 
 static struct drm_plane_state *
 drm_output_prepare_scanout_view(struct drm_output_state *output_state,
-   struct weston_view *ev)
+   struct weston_view *ev,
+   enum drm_output_propose_state_mode mode)
 {
struct drm_output *output = output_state->output;
struct drm_backend *b = to_drm_backend(output->base.compositor);
struct drm_plane *scanout_plane = output->scanout_plane;
struct drm_plane_state *state;
+   struct drm_plane_state *state_old = NULL;
struct drm_fb *fb;
pixman_box32_t *extents;
+   int ret;
 
assert(!b->sprites_are_broken);
 
@@ -1983,11 +1986,20 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
}
 
state = drm_output_state_get_plane(output_state, scanout_plane);
-   if (state->fb) {
-   /* If there is already a framebuffer on the scanout plane,
-* a client view has already been placed on the scanout
-* view. In that case, do not free or put back the state,
-* but just leave it in place and quietly exit. */
+
+   /* Check if we've already placed a buffer on this plane; if there's a
+* buffer there but it comes from GBM, then it's the result of
+* drm_output_propose_state placing it here for testing purposes.
+* If we already have another client buffer, then we can't use this
+* plane. */
+   if (state->fb &&
+   (state->fb->type == BUFFER_GBM_SURFACE ||
+state->fb->type == BUFFER_PIXMAN_DUMB)) {
+   state_old = calloc(1, sizeof(*state_old));
+   memcpy(state_old, state, sizeof(*state_old));
+   state_old->output_state = NULL;
+   wl_list_init(_old->link);
+   } else if (state->fb) {
drm_fb_unref(fb);
return NULL;
}
@@ -2007,10 +2019,26 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
 
+   /* In plane-only mode, we don't need to test the state now, as we
+* will only test it once at the end. */
+   if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) {
+   drm_plane_state_free(state_old, false);
+   return state;
+   }
+
+   ret = drm_pending_state_test(output_state->pending_state);
+   if (ret != 0)
+   goto err;
+
+   drm_plane_state_free(state_old, false);
return state;
 
 err:
-   drm_plane_state_put_back(state);
+   drm_plane_state_free(state, false);
+   if (state_old) {
+   wl_list_insert(_state->plane_list, _old->link);
+   state_old->output_state = output_state;
+   }
return NULL;
 }
 
@@ -2078,7 +2106,9 @@ drm_output_render(struct drm_output_state *state, 
pixman_region32_t *damage)
 * want to render. */
scanout_state = drm_output_state_get_plane(state,
   output->scanout_plane);
-   if (scanout_state->fb)
+   if (scanout_state->fb &&
+

[PATCH v19 07/10] compositor-drm: Add planes-only mode to state proposal

2018-07-10 Thread Daniel Stone
Add a new mode, which attempts to construct a scene exclusively using
planes. This is a building block for incrementally testing and
constructing state: in the plane-only mode, we test the state exactly
once, when we have constructed a full set of planes and want to know if
it works or not.

When using the renderer, we need to incrementally test views one by one
to see if they will work on planes, falling back to the renderer if not.
This test is different, since the scanout plane will be occupied by the
renderer's buffer. Testing using the renderer or client buffers may have
completely different characteristics, so we need two passes: first,
constructing a state with only planes and testing if that succeeds,
falling back later to a mixed renderer/plane mode which tests
incrementally.

This implements the first mode, and preferentially attempts to use it.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 282d6d67f..3a627dd34 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1944,6 +1944,7 @@ drm_output_assign_state(struct drm_output_state *state,
 enum drm_output_propose_state_mode {
DRM_OUTPUT_PROPOSE_STATE_MIXED, /**< mix renderer & planes */
DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer & 
cursor */
+   DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY, /**< no renderer use, only planes 
*/
 };
 
 static struct drm_plane_state *
@@ -3296,6 +3297,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
+   bool renderer_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
int ret;
 
assert(!output->state_last);
@@ -3387,6 +3389,11 @@ drm_output_propose_state(struct weston_output 
*output_base,
if (!drm_view_is_opaque(ev))
force_renderer = true;
 
+   if (force_renderer && !renderer_ok) {
+   pixman_region32_fini(_view);
+   goto err;
+   }
+
if (!force_renderer && !ps)
ps = drm_output_prepare_scanout_view(state, ev);
if (!force_renderer && !ps)
@@ -3409,6 +3416,14 @@ drm_output_propose_state(struct weston_output 
*output_base,
continue;
}
 
+   /* We have been assigned to the primary (renderer) plane:
+* check if this is OK, and add ourselves to the renderer
+* region if so. */
+   if (!renderer_ok) {
+   pixman_region32_fini(_view);
+   goto err;
+   }
+
pixman_region32_union(_region,
  _region,
  _view);
@@ -3441,16 +3456,21 @@ drm_assign_planes(struct weston_output *output_base, 
void *repaint_data)
struct drm_plane_state *plane_state;
struct weston_view *ev;
struct weston_plane *primary = _base->compositor->primary_plane;
-   enum drm_output_propose_state_mode mode;
 
-   if (!b->sprites_are_broken)
+   if (!b->sprites_are_broken) {
state = drm_output_propose_state(output_base, pending_state,
-
DRM_OUTPUT_PROPOSE_STATE_MIXED);
+
DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
+   if (!state)
+   state = drm_output_propose_state(output_base, 
pending_state,
+
DRM_OUTPUT_PROPOSE_STATE_MIXED);
+   }
 
if (!state)
state = drm_output_propose_state(output_base, pending_state,
 
DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
 
+   assert(state);
+
wl_list_for_each(ev, _base->compositor->view_list, link) {
struct drm_plane *target_plane = NULL;
 
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v19 10/10] compositor-drm: Enable planes for atomic

2018-07-10 Thread Daniel Stone
Now that we can sensibly test proposed plane configurations with atomic,
sprites are not broken.

Signed-off-by: Daniel Stone 
Reviewed-by: Pekka Paalanen 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index a364fb5df..7fe14ae5e 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3826,6 +3826,17 @@ init_kms_caps(struct drm_backend *b)
weston_log("DRM: %s atomic modesetting\n",
   b->atomic_modeset ? "supports" : "does not support");
 
+   /*
+* KMS support for hardware planes cannot properly synchronize
+* without nuclear page flip. Without nuclear/atomic, hw plane
+* and cursor plane updates would either tear or cause extra
+* waits for vblanks which means dropping the compositor framerate
+* to a fraction. For cursors, it's not so bad, so they are
+* enabled.
+*/
+   if (!b->atomic_modeset)
+   b->sprites_are_broken = 1;
+
ret = drmSetClientCap(b->drm.fd, DRM_CLIENT_CAP_ASPECT_RATIO, 1);
b->aspect_ratio_supported = (ret == 0);
weston_log("DRM: %s picture aspect ratio\n",
@@ -6706,17 +6717,6 @@ drm_backend_create(struct weston_compositor *compositor,
b->drm.fd = -1;
wl_array_init(>unused_crtcs);
 
-   /*
-* KMS support for hardware planes cannot properly synchronize
-* without nuclear page flip. Without nuclear/atomic, hw plane
-* and cursor plane updates would either tear or cause extra
-* waits for vblanks which means dropping the compositor framerate
-* to a fraction. For cursors, it's not so bad, so they are
-* enabled.
-*
-* These can be enabled again when nuclear/atomic support lands.
-*/
-   b->sprites_are_broken = 1;
b->compositor = compositor;
b->use_pixman = config->use_pixman;
b->pageflip_timeout = config->pageflip_timeout;
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v19 01/10] compositor-drm: Disallow overlapping overlay planes

2018-07-10 Thread Daniel Stone
The scanout plane strictly stacks under all overlay planes, and the
cursor plane above. However, the stacking of overlay planes with respect
to each other is undefined.

We can control the stacking order of overlay planes with the zpos
property, though this significantly complicates plane assignment. In the
meantime, simply disallow assigning a view to an overlay, when it
overlaps another view which is already on an overlay. This ensures
stacking order is irrelevant, since the planes never intersect each
other.

Signed-off-by: Daniel Stone 
Reviewed-by: Pekka Paalanen 
---
 libweston/compositor-drm.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 51b2482d9..2305f708f 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3324,6 +3324,16 @@ drm_output_propose_state(struct weston_output 
*output_base,
  _view);
if (pixman_region32_not_empty(_overlap))
next_plane = primary;
+
+   /* We do not control the stacking order of overlay planes;
+* the scanout plane is strictly stacked bottom and the cursor
+* plane top, but the ordering of overlay planes with respect
+* to each other is undefined. Make sure we do not have two
+* planes overlapping each other. */
+   pixman_region32_intersect(_overlap, _region,
+ _view);
+   if (pixman_region32_not_empty(_overlap))
+   next_plane = primary;
pixman_region32_fini(_overlap);
 
if (next_plane == NULL)
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v19 06/10] compositor-drm: Never lift solid surfaces to planes

2018-07-10 Thread Daniel Stone
This will never work, so don't even try to do it.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index f7a364837..282d6d67f 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3337,6 +3337,9 @@ drm_output_propose_state(struct weston_output 
*output_base,
if (ev->output_mask != (1u << output->base.id))
force_renderer = true;
 
+   if (!ev->surface->buffer_ref.buffer)
+   force_renderer = true;
+
/* Ignore views we know to be totally occluded. */
pixman_region32_init(_view);
pixman_region32_intersect(_view,
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v19 05/10] compositor-drm: Add test-only mode to state application

2018-07-10 Thread Daniel Stone
The atomic API can allow us to test state before we apply it, to see if
it will be valid. Use this when we construct a plane configuration, to
see if it has a chance of ever working. If not, we can fail
assign_planes early.

This will be used in later patches to incrementally build state by
proposing and testing potential configurations one at a time.

Signed-off-by: Daniel Stone 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 37 +
 1 file changed, 37 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 28a580cd2..f7a364837 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -260,6 +260,7 @@ enum drm_output_state_duplicate_mode {
 enum drm_state_apply_mode {
DRM_STATE_APPLY_SYNC, /**< state fully processed */
DRM_STATE_APPLY_ASYNC, /**< state pending event delivery */
+   DRM_STATE_TEST_ONLY, /**< test if the state can be applied */
 };
 
 struct drm_backend {
@@ -1822,6 +1823,7 @@ drm_pending_state_get_output(struct drm_pending_state 
*pending_state,
 }
 
 static int drm_pending_state_apply_sync(struct drm_pending_state *state);
+static int drm_pending_state_test(struct drm_pending_state *state);
 
 /**
  * Mark a drm_output_state (the output's last state) as complete. This handles
@@ -2610,9 +2612,16 @@ drm_pending_state_apply_atomic(struct drm_pending_state 
*pending_state,
case DRM_STATE_APPLY_ASYNC:
flags |= DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
break;
+   case DRM_STATE_TEST_ONLY:
+   flags |= DRM_MODE_ATOMIC_TEST_ONLY;
+   break;
}
 
ret = drmModeAtomicCommit(b->drm.fd, req, flags, b);
+
+   if (mode == DRM_STATE_TEST_ONLY)
+   return ret;
+
if (ret != 0) {
weston_log("atomic: couldn't commit new state: %m\n");
goto out;
@@ -2640,6 +2649,22 @@ out:
  *
  * Unconditionally takes ownership of pending_state, and clears state_invalid.
  */
+static int
+drm_pending_state_test(struct drm_pending_state *pending_state)
+{
+#ifdef HAVE_DRM_ATOMIC
+   struct drm_backend *b = pending_state->backend;
+
+   if (b->atomic_modeset)
+   return drm_pending_state_apply_atomic(pending_state,
+ DRM_STATE_TEST_ONLY);
+#endif
+
+   /* We have no way to test state before application on the legacy
+* modesetting API, so just claim it succeeded. */
+   return 0;
+}
+
 static int
 drm_pending_state_apply(struct drm_pending_state *pending_state)
 {
@@ -3271,6 +3296,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
+   int ret;
 
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
@@ -3388,7 +3414,18 @@ drm_output_propose_state(struct weston_output 
*output_base,
pixman_region32_fini(_region);
pixman_region32_fini(_region);
 
+   /* Check to see if this state will actually work. */
+   ret = drm_pending_state_test(state->pending_state);
+   if (ret != 0)
+   goto err;
+
return state;
+
+err:
+   pixman_region32_fini(_region);
+   pixman_region32_fini(_region);
+   drm_output_state_free(state);
+   return NULL;
 }
 
 static void
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v19 09/10] compositor-drm: Relax plane restrictions for atomic

2018-07-10 Thread Daniel Stone
Since we now incrementally test atomic state as we build it, we can
loosen restrictions on what we can do with planes, and let the kernel
tell us whether or not it's OK.

Signed-off-by: Daniel Stone 
Reviewed-by: Pekka Paalanen 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index ab3eea6cf..a364fb5df 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1980,7 +1980,7 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
return NULL;
 
/* Can't change formats with just a pageflip */
-   if (fb->format->format != output->gbm_format) {
+   if (!b->atomic_modeset && fb->format->format != output->gbm_format) {
drm_fb_unref(fb);
return NULL;
}
@@ -2010,15 +2010,18 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
if (!drm_plane_state_coords_for_view(state, ev))
goto err;
 
-   /* The legacy API does not let us perform cropping or scaling. */
-   if (state->src_x != 0 || state->src_y != 0 ||
-   state->src_w != state->dest_w << 16 ||
-   state->src_h != state->dest_h << 16 ||
-   state->dest_x != 0 || state->dest_y != 0 ||
+   if (state->dest_x != 0 || state->dest_y != 0 ||
state->dest_w != (unsigned) output->base.current_mode->width ||
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
 
+   /* The legacy API does not let us perform cropping or scaling. */
+   if (!b->atomic_modeset &&
+   (state->src_x != 0 || state->src_y != 0 ||
+state->src_w != state->dest_w << 16 ||
+state->src_h != state->dest_h << 16))
+   goto err;
+
/* In plane-only mode, we don't need to test the state now, as we
 * will only test it once at the end. */
if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) {
@@ -3113,8 +3116,9 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
state->ev = ev;
state->output = output;
drm_plane_state_coords_for_view(state, ev);
-   if (state->src_w != state->dest_w << 16 ||
-   state->src_h != state->dest_h << 16) {
+   if (!b->atomic_modeset &&
+   (state->src_w != state->dest_w << 16 ||
+state->src_h != state->dest_h << 16)) {
drm_plane_state_put_back(state);
continue;
}
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v19 02/10] compositor-drm: Use sprites_are_broken for scanout plane

2018-07-10 Thread Daniel Stone
When the sprites_are_broken variable is set, do not attempt to promote
client surfaces to the scanout plane.

We are currently assuming that every client buffer will be compatible
with the scanout plane, but that is not the case, particularly with more
exotic tiled/compressed buffers. Once we promote the client buffer to
scanout, there is no going back: if the repaint fails, we do not mark
this as failed and go back to repaint through composition.

This permanently removes the ability for scanout bypass when using the
non-atomic path. Future patches lift the restriction when using atomic
modesetting, as we can actually test and ensure that the view is
compatible with scanout.

Signed-off-by: Daniel Stone 
Reported-by: Pekka Paalanen 
---
 libweston/compositor-drm.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 2305f708f..d045778aa 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1944,11 +1944,14 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
struct weston_view *ev)
 {
struct drm_output *output = output_state->output;
+   struct drm_backend *b = to_drm_backend(output->base.compositor);
struct drm_plane *scanout_plane = output->scanout_plane;
struct drm_plane_state *state;
struct drm_fb *fb;
pixman_box32_t *extents;
 
+   assert(!b->sprites_are_broken);
+
/* Check the view spans exactly the output size, calculated in the
 * logical co-ordinate space. */
extents = pixman_region32_extents(>transform.boundingbox);
@@ -3004,8 +3007,7 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
struct drm_fb *fb;
unsigned int i;
 
-   if (b->sprites_are_broken)
-   return NULL;
+   assert(!b->sprites_are_broken);
 
fb = drm_fb_get_from_view(output_state, ev);
if (!fb)
@@ -3260,6 +3262,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
 struct drm_pending_state *pending_state)
 {
struct drm_output *output = to_drm_output(output_base);
+   struct drm_backend *b = to_drm_backend(output->base.compositor);
struct drm_output_state *state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
@@ -3342,6 +3345,9 @@ drm_output_propose_state(struct weston_output 
*output_base,
if (next_plane == NULL && !drm_view_is_opaque(ev))
next_plane = primary;
 
+   if (next_plane == NULL && b->sprites_are_broken)
+   next_plane = primary;
+
if (next_plane == NULL)
next_plane = drm_output_prepare_scanout_view(state, ev);
 
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v19 10/10] DRM backend planes

2018-07-10 Thread Daniel Stone
Hi,
I believe this should resolve all the issues Pekka pointed out in the
(too hurried) v18. It's also been split up into a few different patches
with much more detailed commit messages, so should be much easier to
reason about and review.

Cheers,
Daniel


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v19 03/10] compositor-drm: Add modes to drm_output_propose_state

2018-07-10 Thread Daniel Stone
Add support for multiple modes to drm_output_propose_state. Currently we
intend to operate in three modes: planes-only (no renderer buffer,
client buffers in planes only), mixed-mode (promote client buffers to
planes where possible, falling back to the renderer where not), and
renderer-only (no plane usage at all).

We want to use the first (planes-only) mode where possible: it can avoid
us having to allocate buffers for the renderer, and it also gives us the
best chance of the optimal configuration, with no composition. In this
mode, we walk the scene looking at all views, trying to put them in
planes, and failing as soon as we find a view we cannot place in a
plane.

In the second mode, rather than failing, we assign those views which
cannot be on a plane to the renderer, and allow the renderer to
composite them.

In the third mode, planes are not usable, so everything but the cursor
goes to the renderer. We will use this when we cannot use the planes-only
mode (because some views cannot be placed in planes), but also cannot
use the 'mixed' mode because we have no renderer buffer yet. Since we
walk the scene graph from top to bottom, using atomic modesetting we
will determine if planes can be promoted in mixed mode by placing a
renderer buffer at the bottom of the scene, placing a cursor buffer if
applicable, then testing if we can add overlay planes to this mode.

Without a buffer from the renderer, we cannot do these tests, so we push
everything through the renderer and then switch to mixed mode on the
next repaint.

Signed-off-by: Daniel Stone 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 69 +-
 1 file changed, 45 insertions(+), 24 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index d045778aa..d4898bc46 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1939,6 +1939,11 @@ drm_output_assign_state(struct drm_output_state *state,
}
 }
 
+enum drm_output_propose_state_mode {
+   DRM_OUTPUT_PROPOSE_STATE_MIXED, /**< mix renderer & planes */
+   DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer & 
cursor */
+};
+
 static struct weston_plane *
 drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
@@ -3121,10 +3126,9 @@ drm_output_prepare_cursor_view(struct drm_output_state 
*output_state,
struct wl_shm_buffer *shmbuf;
bool needs_update = false;
 
-   if (!plane)
-   return NULL;
+   assert(!b->cursors_are_broken);
 
-   if (b->cursors_are_broken)
+   if (!plane)
return NULL;
 
if (!plane->state_cur->complete)
@@ -3259,7 +3263,8 @@ err:
 
 static struct drm_output_state *
 drm_output_propose_state(struct weston_output *output_base,
-struct drm_pending_state *pending_state)
+struct drm_pending_state *pending_state,
+enum drm_output_propose_state_mode mode)
 {
struct drm_output *output = to_drm_output(output_base);
struct drm_backend *b = to_drm_backend(output->base.compositor);
@@ -3267,11 +3272,14 @@ drm_output_propose_state(struct weston_output 
*output_base,
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
struct weston_plane *primary = _base->compositor->primary_plane;
+   bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
 
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
   pending_state,
   DRM_OUTPUT_STATE_CLEAR_PLANES);
+   if (!planes_ok)
+   return state;
 
/*
 * Find a surface for each sprite in the output using some heuristics:
@@ -3339,13 +3347,16 @@ drm_output_propose_state(struct weston_output 
*output_base,
next_plane = primary;
pixman_region32_fini(_overlap);
 
-   if (next_plane == NULL)
+   /* The cursor plane is 'special' in the sense that we can still
+* place it in the legacy API, and we gate that with a separate
+* cursors_are_broken flag. */
+   if (next_plane == NULL && !b->cursors_are_broken)
next_plane = drm_output_prepare_cursor_view(state, ev);
 
if (next_plane == NULL && !drm_view_is_opaque(ev))
next_plane = primary;
 
-   if (next_plane == NULL && b->sprites_are_broken)
+   if (next_plane == NULL && !planes_ok)
next_plane = primary;
 
if (next_plane == NULL)
@@ -3354,24 +3365,27 @@ drm_output_propose_state(struct weston_output 
*output_b

[PATCH v19 04/10] compositor-drm: Return plane state from plane preparation

2018-07-10 Thread Daniel Stone
Return a pointer to the plane state, rather than returning its
underlying weston_plane.

Signed-off-by: Daniel Stone 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 53 ++
 1 file changed, 25 insertions(+), 28 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index d4898bc46..28a580cd2 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1944,7 +1944,7 @@ enum drm_output_propose_state_mode {
DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer & 
cursor */
 };
 
-static struct weston_plane *
+static struct drm_plane_state *
 drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
 {
@@ -2004,7 +2004,7 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
 
-   return _plane->base;
+   return state;
 
 err:
drm_plane_state_put_back(state);
@@ -2170,7 +2170,6 @@ drm_output_apply_state_legacy(struct drm_output_state 
*state)
struct drm_property_info *dpms_prop;
struct drm_plane_state *scanout_state;
struct drm_plane_state *ps;
-   struct drm_plane *p;
struct drm_mode *mode;
struct drm_head *head;
uint32_t connectors[MAX_CLONED_CONNECTORS];
@@ -2197,7 +2196,7 @@ drm_output_apply_state_legacy(struct drm_output_state 
*state)
 
if (state->dpms != WESTON_DPMS_ON) {
wl_list_for_each(ps, >plane_list, link) {
-   p = ps->plane;
+   struct drm_plane *p = ps->plane;
assert(ps->fb == NULL);
assert(ps->output == NULL);
 
@@ -2287,8 +2286,8 @@ drm_output_apply_state_legacy(struct drm_output_state 
*state)
.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT,
.request.sequence = 1,
};
+   struct drm_plane *p = ps->plane;
 
-   p = ps->plane;
if (p->type != WDRM_PLANE_TYPE_OVERLAY)
continue;
 
@@ -3000,7 +2999,7 @@ atomic_flip_handler(int fd, unsigned int frame, unsigned 
int sec,
 }
 #endif
 
-static struct weston_plane *
+static struct drm_plane_state *
 drm_output_prepare_overlay_view(struct drm_output_state *output_state,
struct weston_view *ev)
 {
@@ -3071,7 +3070,7 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
state->src_h != state->dest_h << 16)
goto err;
 
-   return >base;
+   return state;
 
 err:
drm_plane_state_put_back(state);
@@ -3115,7 +3114,7 @@ cursor_bo_update(struct drm_plane_state *plane_state, 
struct weston_view *ev)
weston_log("failed update cursor: %m\n");
 }
 
-static struct weston_plane *
+static struct drm_plane_state *
 drm_output_prepare_cursor_view(struct drm_output_state *output_state,
   struct weston_view *ev)
 {
@@ -3201,7 +3200,7 @@ drm_output_prepare_cursor_view(struct drm_output_state 
*output_state,
plane_state->dest_w = b->cursor_width;
plane_state->dest_h = b->cursor_height;
 
-   return >base;
+   return plane_state;
 
 err:
drm_plane_state_put_back(plane_state);
@@ -3271,7 +3270,6 @@ drm_output_propose_state(struct weston_output 
*output_base,
struct drm_output_state *state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
-   struct weston_plane *primary = _base->compositor->primary_plane;
bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
 
assert(!output->state_last);
@@ -3298,7 +3296,8 @@ drm_output_propose_state(struct weston_output 
*output_base,
pixman_region32_init(_region);
 
wl_list_for_each(ev, _base->compositor->view_list, link) {
-   struct weston_plane *next_plane = NULL;
+   struct drm_plane_state *ps = NULL;
+   bool force_renderer = false;
pixman_region32_t clipped_view;
bool occluded = false;
 
@@ -3310,7 +3309,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
/* We only assign planes to views which are exclusively present
 * on our output. */
if (ev->output_mask != (1u << output->base.id))
-   next_plane = primary;
+   force_renderer = true;
 
/* Ignore views we know to be totally occluded. */
pixman_region32_init(_view);
@@ -3334,7 +,7 @@ drm_output_propose_state(struct weston_output 
*output_b

[PATCH v18 4/4] compositor-drm: Return plane state from plane preparation

2018-07-10 Thread Daniel Stone
Return a pointer to the plane state, rather than indirecting via a
weston_plane.

Signed-off-by: Daniel Stone 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 58 --
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 81fcd2412..43dc9856d 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1945,7 +1945,7 @@ enum drm_output_propose_state_mode {
DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer & 
cursor */
 };
 
-static struct weston_plane *
+static struct drm_plane_state *
 drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
 {
@@ -2005,7 +2005,7 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
 
-   return _plane->base;
+   return state;
 
 err:
drm_plane_state_put_back(state);
@@ -2171,7 +2171,6 @@ drm_output_apply_state_legacy(struct drm_output_state 
*state)
struct drm_property_info *dpms_prop;
struct drm_plane_state *scanout_state;
struct drm_plane_state *ps;
-   struct drm_plane *p;
struct drm_mode *mode;
struct drm_head *head;
uint32_t connectors[MAX_CLONED_CONNECTORS];
@@ -2198,7 +2197,7 @@ drm_output_apply_state_legacy(struct drm_output_state 
*state)
 
if (state->dpms != WESTON_DPMS_ON) {
wl_list_for_each(ps, >plane_list, link) {
-   p = ps->plane;
+   struct drm_plane *p = ps->plane;
assert(ps->fb == NULL);
assert(ps->output == NULL);
 
@@ -2288,8 +2287,8 @@ drm_output_apply_state_legacy(struct drm_output_state 
*state)
.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT,
.request.sequence = 1,
};
+   struct drm_plane *p = ps->plane;
 
-   p = ps->plane;
if (p->type != WDRM_PLANE_TYPE_OVERLAY)
continue;
 
@@ -3001,7 +3000,7 @@ atomic_flip_handler(int fd, unsigned int frame, unsigned 
int sec,
 }
 #endif
 
-static struct weston_plane *
+static struct drm_plane_state *
 drm_output_prepare_overlay_view(struct drm_output_state *output_state,
struct weston_view *ev)
 {
@@ -3072,7 +3071,7 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
state->src_h != state->dest_h << 16)
goto err;
 
-   return >base;
+   return state;
 
 err:
drm_plane_state_put_back(state);
@@ -3116,7 +3115,7 @@ cursor_bo_update(struct drm_plane_state *plane_state, 
struct weston_view *ev)
weston_log("failed update cursor: %m\n");
 }
 
-static struct weston_plane *
+static struct drm_plane_state *
 drm_output_prepare_cursor_view(struct drm_output_state *output_state,
   struct weston_view *ev)
 {
@@ -3202,7 +3201,7 @@ drm_output_prepare_cursor_view(struct drm_output_state 
*output_state,
plane_state->dest_w = b->cursor_width;
plane_state->dest_h = b->cursor_height;
 
-   return >base;
+   return plane_state;
 
 err:
drm_plane_state_put_back(plane_state);
@@ -3272,7 +3271,6 @@ drm_output_propose_state(struct weston_output 
*output_base,
struct drm_output_state *state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
-   struct weston_plane *primary = _base->compositor->primary_plane;
bool renderer_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
 
@@ -3298,7 +3296,8 @@ drm_output_propose_state(struct weston_output 
*output_base,
pixman_region32_init(_region);
 
wl_list_for_each(ev, _base->compositor->view_list, link) {
-   struct weston_plane *next_plane = NULL;
+   struct drm_plane_state *ps = NULL;
+   bool force_renderer = false;
pixman_region32_t clipped_view;
bool occluded = false;
 
@@ -3310,7 +3309,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
/* We only assign planes to views which are exclusively present
 * on our output. */
if (ev->output_mask != (1u << output->base.id))
-   next_plane = primary;
+   force_renderer = true;
 
/* Ignore views we know to be totally occluded. */
pixman_region32_init(_view);
@@ -3334,7 +,7 @@ drm_output_propose_state(struct weston_output 
*output_base

[PATCH v18 1/4] compositor-drm: Disallow overlapping overlay planes

2018-07-10 Thread Daniel Stone
The scanout plane strictly stacks under all overlay planes, and the
cursor plane above. However, the stacking of overlay planes with respect
to each other is undefined.

We can control the stacking order of overlay planes with the zpos
property, though this significantly complicates plane assignment. In the
meantime, simply disallow assigning a view to an overlay, when it
overlaps another view which is already on an overlay. This ensures
stacking order is irrelevant, since the planes never intersect each
other.

Signed-off-by: Daniel Stone 
Reported-by: Pekka Paalanen 
---
 libweston/compositor-drm.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 51b2482d9..2305f708f 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3324,6 +3324,16 @@ drm_output_propose_state(struct weston_output 
*output_base,
  _view);
if (pixman_region32_not_empty(_overlap))
next_plane = primary;
+
+   /* We do not control the stacking order of overlay planes;
+* the scanout plane is strictly stacked bottom and the cursor
+* plane top, but the ordering of overlay planes with respect
+* to each other is undefined. Make sure we do not have two
+* planes overlapping each other. */
+   pixman_region32_intersect(_overlap, _region,
+ _view);
+   if (pixman_region32_not_empty(_overlap))
+   next_plane = primary;
pixman_region32_fini(_overlap);
 
if (next_plane == NULL)
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v18 0/4] DRM atomic preparation

2018-07-10 Thread Daniel Stone
Hi,
A couple of changes after Pekka's review of the second two patches,
including two preparatory patches pulled out.

Cheers,
Daniel


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v18 2/4] compositor-drm: Use sprites_are_broken for scanout plane

2018-07-10 Thread Daniel Stone
When the sprites_are_broken variable is set, do not attempt to promote
client surfaces to the scanout plane.

We are currently assuming that every client buffer will be compatible
with the scanout plane, but that is not the case, particularly with more
exotic tiled/compressed buffers. Once we promote the client buffer to
scanout, there is no going back: if the repaint fails, we do not mark
this as failed and go back to repaint through composition.

This removes the ability for scanout bypass when using the non-atomic
path, however future patches lift the restriction when using atomic
modesetting, as we can actually test and ensure that the view is
compatible with scanout.

Signed-off-by: Daniel Stone 
Reported-by: Pekka Paalanen 
---
 libweston/compositor-drm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 2305f708f..4a48e2d02 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3342,6 +3342,9 @@ drm_output_propose_state(struct weston_output 
*output_base,
if (next_plane == NULL && !drm_view_is_opaque(ev))
next_plane = primary;
 
+   if (next_plane == NULL && !planes_ok)
+   next_plane = primary;
+
if (next_plane == NULL)
next_plane = drm_output_prepare_scanout_view(state, ev);
 
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v18 3/4] compositor-drm: Add modes to drm_output_propose_state

2018-07-10 Thread Daniel Stone
Add support for multiple modes to drm_output_propose_state. Currently we
intend to operate in three modes: planes-only (no renderer buffer,
client buffers in planes only), mixed-mode (promote client buffers to
planes where possible, falling back to the renderer where not), and
renderer-only (no plane usage at all).

We want to use the first (planes-only) mode where possible: it can avoid
us having to allocate buffers for the renderer, and it also gives us the
best chance of the optimal configuration, with no composition. In this
mode, we walk the scene looking at all views, trying to put them in
planes, and failing as soon as we find a view we cannot place in a
plane.

In the second mode, rather than failing, we assign those views which
cannot be on a plane to the renderer, and allow the renderer to
composite them.

In the third mode, planes are not usable, so everything but the cursor
goes to the renderer. We will use this when we cannot use the planes-only
mode (because some views cannot be placed in planes), but also cannot
use the 'mixed' mode because we have no renderer buffer yet. Since we
walk the scene graph from top to bottom, using atomic modesetting we
will determine if planes can be promoted in mixed mode by placing a
renderer buffer at the bottom of the scene, placing a cursor buffer if
applicable, then testing if we can add overlay planes to this mode.

Without a buffer from the renderer, we cannot do these tests, so we push
everything through the renderer and then switch to mixed mode on the
next repaint.

Signed-off-by: Daniel Stone 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 86 +++---
 1 file changed, 61 insertions(+), 25 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 4a48e2d02..81fcd2412 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1939,16 +1939,25 @@ drm_output_assign_state(struct drm_output_state *state,
}
 }
 
+enum drm_output_propose_state_mode {
+   DRM_OUTPUT_PROPOSE_STATE_MIXED, /**< mix renderer & planes */
+   DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY, /**< only assign to planes */
+   DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY, /**< only assign to renderer & 
cursor */
+};
+
 static struct weston_plane *
 drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
 {
struct drm_output *output = output_state->output;
+   struct drm_backend *b = to_drm_backend(output->base.compositor);
struct drm_plane *scanout_plane = output->scanout_plane;
struct drm_plane_state *state;
struct drm_fb *fb;
pixman_box32_t *extents;
 
+   assert(!b->sprites_are_broken);
+
/* Check the view spans exactly the output size, calculated in the
 * logical co-ordinate space. */
extents = pixman_region32_extents(>transform.boundingbox);
@@ -3004,8 +3013,7 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
struct drm_fb *fb;
unsigned int i;
 
-   if (b->sprites_are_broken)
-   return NULL;
+   assert(!b->sprites_are_broken);
 
fb = drm_fb_get_from_view(output_state, ev);
if (!fb)
@@ -3119,10 +3127,9 @@ drm_output_prepare_cursor_view(struct drm_output_state 
*output_state,
struct wl_shm_buffer *shmbuf;
bool needs_update = false;
 
-   if (!plane)
-   return NULL;
+   assert(!b->cursors_are_broken);
 
-   if (b->cursors_are_broken)
+   if (!plane)
return NULL;
 
if (!plane->state_cur->complete)
@@ -3257,13 +3264,17 @@ err:
 
 static struct drm_output_state *
 drm_output_propose_state(struct weston_output *output_base,
-struct drm_pending_state *pending_state)
+struct drm_pending_state *pending_state,
+enum drm_output_propose_state_mode mode)
 {
struct drm_output *output = to_drm_output(output_base);
+   struct drm_backend *b = to_drm_backend(output_base->compositor);
struct drm_output_state *state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
struct weston_plane *primary = _base->compositor->primary_plane;
+   bool renderer_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
+   bool planes_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_RENDERER_ONLY);
 
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
@@ -3336,7 +3347,10 @@ drm_output_propose_state(struct weston_output 
*output_base,
next_plane = primary;
pixman_region32_fini(_overlap);
 
-   if (next_plane == NULL)
+   /* The cursor plane is 'special' in the sense that we can still
+ 

Re: [PATCH v17 09/14] compositor-drm: Ignore occluded views

2018-07-10 Thread Daniel Stone
Hi,
On Tue, 10 Jul 2018 at 10:06, Pekka Paalanen  wrote:
> On Mon,  9 Jul 2018 14:23:15 +0100 Daniel Stone  wrote:
> > @@ -3302,6 +3319,9 @@ drm_output_propose_state(struct weston_output 
> > *output_base,
> >   if (next_plane == NULL)
> >   next_plane = drm_output_prepare_cursor_view(state, 
> > ev);
> >
> > + if (next_plane == NULL && !drm_view_is_opaque(ev))
> > + next_plane = primary;

As you noted here, we only promote fully opaque views.

> However, I noticed something else. drm_output_preprare_overlay_view()
> may work also for non-opaque views. Therefore we need to handle
> overlays as non-opaque or know if they actually are opaque.
>
> Why do we even toggle opaqueness by the plane assignment? Why not
> simply directly add the opaque region to occluded_region regardless of
> the assigned plane?
>
> weston_view::transform.opaque is there for that purpose.
>
> drm_view_is_opaque() requires the whole view to be opaque, but we don't
> actually need that for occluded_region. We should be able to do with
> the proper opaque region in global coordinates, so that an opaque
> sub-region of a view on a plane would be counted as occlusion.

At the moment the whole area must be opaque in order to be promoted to
a view. There is another, later, stage where we can begin to promote
non-opaque views, however that gets tricky. We need to make sure that
DRM's plane blending is in line with what we expect (it might be, but
I'm not immediately sure), and we also need to begin to take care of
stacking order. At the moment, scanout/primary strictly stacks at the
bottom (furthest from eye) and cursor at the top (closest to eye),
with all the overlay planes in between.

We currently ensure the stacking order is correct by only promoting
views which do not overlap each other to overlay planes. If we started
loosening that, so planes could overlap each other (opaque or not), we
need to be setting the 'zpos' property of planes, or picking the plane
in the correct zpos. This got rather difficult rather fast, so I
decided it was probably best not to roll it into the series. Fabien
and Benji from STM had an old patchset which did implement this, which
should be checked and resurrected once this all lands.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v14 38/41] compositor-drm: Relax plane restrictions for atomic

2018-07-10 Thread Daniel Stone
Hi,

On Mon, 29 Jan 2018 at 13:59, Pekka Paalanen  wrote:
> On Wed, 20 Dec 2017 12:26:55 +0000 Daniel Stone  wrote:
> > @@ -1818,12 +1819,10 @@ drm_output_prepare_scanout_view(struct 
> > drm_output_state *output_state,
> >   drm_plane_state_coords_for_view(state, ev);
> >
> >   /* The legacy API does not let us perform cropping or scaling. */
> > - if (state->src_x != 0 || state->src_y != 0 ||
> > - state->src_w != state->dest_w << 16 ||
> > - state->src_h != state->dest_h << 16 ||
> > - state->dest_x != 0 || state->dest_y != 0 ||
> > - state->dest_w != (unsigned) output->base.current_mode->width ||
> > - state->dest_h != (unsigned) output->base.current_mode->height)
> > + if (!b->atomic_modeset &&
> > + (state->src_x != 0 || state->src_y != 0 ||
> > +  state->src_w != state->dest_w << 16 ||
> > +  state->src_h != state->dest_h << 16))
>
> Where did the mode size checks go?

These are retained in v17.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v14 35/41] compositor-drm: Add modes to drm_output_propose_state

2018-07-10 Thread Daniel Stone
Hi,
On Mon, 29 Jan 2018 at 10:55, Pekka Paalanen  wrote:
> On Mon, 29 Jan 2018 09:17:49 +0000 Daniel Stone  wrote:
> > On 26 January 2018 at 14:04, Pekka Paalanen  wrote:
> > > On Wed, 20 Dec 2017 12:26:52 +
> > > Daniel Stone  wrote:
> > >> +enum drm_output_propose_state_mode {
> > >> + DRM_OUTPUT_PROPOSE_STATE_MIXED, /**< mix renderer & planes */
> > >> + DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY, /**< only assign to planes */
> > >> +};
> > >
> > > what is the reason to have a planes-only mode? I saw the patch
> > > "compositor-drm: Add test-only mode to state application" will first
> > > attempt a planes-only mode and then fall back to mixed mode. Why does
> > > the planes-only mode not fall out of mixed mode naturally when it runs
> > > out of views to show? This would be good to explain in the commit
> > > message to justify the modes.
> >
> > This could again go into a comment or documentation I suppose, but I'm
> > not sure where I'd put it without adding so much clutter to the code
> > as to hide it. Ideas?
>
> Commit message would be fine by me, you can be as verbose as you want
> without a worry.

This got missed.

> > We have two options: either punt when we don't have valid scanout
> > state, or do what we do here. Which is, speculatively build a state
> > composed purely of planes with no intermediate checks, and test once
> > at the end. If it works, then we use it. If not, we build a
> > renderer-only (i.e. renderer + cursor) state and just use that. In the
> > next repaint, we will use the normal 'mixed' mode.
> >
> > (All of the above described is the intention. If these patches differ
> > from that description, then the patches are wrong.)
>
> Ok, this is something that never occurred to me when reading this
> patch. This is going to a great length to avoid allocating a
> framebuffer for the renderer unless absolutely necessary.

Not just avoiding allocating, but avoiding putting up one frame where
everything has gone through the GPU, followed by another where
everything's on the overlay, for no real reason. That could cause
flickers when playing media, due to different colourspace conversions
etc. But yes, we do bend over backwards to use planes where we can.

> A quick check on "compositor-drm: Add test-only mode to state
> application" revealed code:
>
> drm_assign_planes(struct weston_output *output_base, void *repaint_data)
> {
> ...
> state = drm_output_propose_state(output_base, pending_state,
>  
> DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
> if (!state)
> state = drm_output_propose_state(output_base, pending_state,
>  
> DRM_OUTPUT_PROPOSE_STATE_MIXED);
>
> from which I made wrong conclusions on what the two modes do. An
> important bit that I missed was the hunk added to
> drm_output_propose_state() that fundamentally changes how 'planes_ok'
> gets set.
>
> Another reason I was confused was that STATE_PLANES_ONLY is attempted
> on every frame. Should that not be skipped if we have an existing
> primary fb to test with? Then the renderer-not-needed case might be
> slightly more obvious to fall out from STATE_MIXED.

It could still be useful. For instance, if the hardware has one
detiling unit shared between all planes, with the GPU rendering to
tiled buffers and the media decoder linear. The client has given us
one GPU buffer placed on top of one fullscreen media buffer. In this
case, if we test compositor rendering buffer (tiled) + client GPU
buffer (tiled), we will fail the test and not promote the client GPU
buffer to an overlay. There's no guarantee that whatever you want to
put on fullscreen is compatible with the renderer output.

> Maybe it would be more clear if the hunk that sets up the real
> planes_ok logic in "compositor-drm: Add test-only mode to state
> application" would be moved into this patch?

Sure, I think that could work.

> Or thinking these patches from a higher level...
>
> Essentially there are three modes: renderer-only, mixed, and
> planes-only. In this patch series, the renderer-only mode is a hidden
> mode under the mixed mode, and it's not obvious why the planes-only
> mode exists. Mixed mode is a bit special, because it can natually fall
> into renderer-only or planes-only setups.
>
> What would you think of exposing all three modes explicitly, and having
> the logic to choose one mode and the fallback to renderer-only mode in
> drm_assing_planes()?
>
> You could have a helper like
> bool drm_output_has_testable_scano

Re: [PATCH v14 40/41] compositor-drm: Support plane IN_FORMATS

2018-07-10 Thread Daniel Stone
Hi Pekka,

On Mon, 29 Jan 2018 at 15:01, Pekka Paalanen  wrote:
> On Wed, 20 Dec 2017 12:26:57 +0000 Daniel Stone  wrote:
> > @@ -212,6 +212,9 @@ if test x$enable_drm_compositor = xyes; then
> >PKG_CHECK_MODULES(DRM_COMPOSITOR_ATOMIC, [libdrm >= 2.4.78],
> >   [AC_DEFINE([HAVE_DRM_ATOMIC], 1, [libdrm supports atomic 
> > API])],
> >   [AC_MSG_WARN([libdrm does not support atomic modesetting, 
> > will omit that capability])])
> > +  PKG_CHECK_MODULES(DRM_COMPOSITOR_FORMATS_BLOB, [libdrm >= 2.4.83],
> > + [AC_DEFINE([HAVE_DRM_FORMATS_BLOB], 1, [libdrm supports 
> > modifier advertisement])],
> > + [AC_MSG_WARN([libdrm does not support modifier 
> > advertisement])])
> >PKG_CHECK_MODULES(DRM_COMPOSITOR_GBM, [gbm >= 17.2],
> >   [AC_DEFINE([HAVE_GBM_FD_IMPORT], 1, [gbm supports import 
> > with modifiers])],
> >   [AC_MSG_WARN([GBM does not support dmabuf import, will 
> > omit that capability])])
>
> Hi,
>
> I wonder, we are getting more and more of these "libdrm has ..."
> feature checks. How about merging some to produce fewer build types?
> Just a general idea, not specifically for this patch.

Assuming enough distros support the new bits, I'd be open to having
two levels of libdrm support: ancient or supports-everything.
Annoyingly the gbm check is separate, but I suppose we could connect,
e.g., both of libdrm >= 2.4.83 + gbm >= 17.2 to modifiers, rather than
having modifiers half-supported when only one of them is sufficiently
new.

> > +#ifdef HAVE_DRM_FORMATS_BLOB
> > +static inline uint32_t *
> > +formats_ptr(struct drm_format_modifier_blob *blob)
> > +{
> > + return (uint32_t *)(((char *)blob) + blob->formats_offset);
> > +}
> > +
> > +static inline struct drm_format_modifier *
> > +modifiers_ptr(struct drm_format_modifier_blob *blob)
> > +{
> > + return (struct drm_format_modifier *)(((char *)blob) + 
> > blob->modifiers_offset);
> > +}
>
> These two functions are unlikely to be used anywhere else than
> populate_format_modifiers(), so they might as well be in the same #ifdef
> block. If even functions at all. A long line.

Both done.

> > @@ -3635,6 +3667,61 @@ init_pixman(struct drm_backend *b)
> >   return pixman_renderer_init(b->compositor);
> >  }
> >
> > +/**
> > + * Populates the formats array, and the modifiers of each format for a 
> > drm_plane.
> > + */
> > +#ifdef HAVE_DRM_FORMATS_BLOB
> > +static bool
> > +populate_format_modifiers(struct drm_plane *plane, const drmModePlane 
> > *kplane,
> > +   uint32_t blob_id)
>
> I think this should be:
>
> static int
> drm_plane_populate_format_modifiers(...
>
> I believe we tend to use int for success/failure returns, where
> negative is a failure. Boolean return values are more used for
> functions that return a truth value, like drm_device_is_kms().

Both done.

> > + unsigned i, j;
> > + drmModePropertyBlobRes *blob;
> > + struct drm_format_modifier_blob *fmt_mod_blob;
> > + uint32_t *blob_formats;
> > + struct drm_format_modifier *blob_modifiers;
> > +
> > + blob = drmModeGetPropertyBlob(plane->backend->drm.fd, blob_id);
> > + if (!blob)
> > + return false;
> > +
> > + fmt_mod_blob = blob->data;
>
> Should we not check that fmt_mod_blob.version == 1?

No. Future versions can add new bits to the end of the blob, but they
can't break compatibility - doing so would require a new property
name. Usually DRM uses size to do this, but as we have two
variable-length arrays whose size can't be trivially determined by the
client, we use version to indicate any possible future extensions to
the structure, and the offset fields to point to the VLAs.

The current version is 1; I guess we could check for version 0 and
bail out if we see it, but no kernel has ever had it and it seems a
bit pathological. Future versions will still be parseable as the
original, so those are fine.

> > + blob_formats = formats_ptr(fmt_mod_blob);
> > + blob_modifiers = modifiers_ptr(fmt_mod_blob);
> > +
> > + assert(plane->count_formats == fmt_mod_blob->count_formats);
> > +
> > + for (i = 0; i < fmt_mod_blob->count_formats; i++) {
> > + uint32_t count_modifiers = 0;
> > + uint64_t *modifiers = NULL;
> > +
> > + for (j = 0; j < fmt_mod_blob->count_modifiers; j++) {
> > + struct drm_format_modifier *mod = _modifiers[j];
>

Re: [PATCH v14 34/41] compositor-drm: Ignore occluded views

2018-07-09 Thread Daniel Stone
Hi Pekka,

On Fri, 26 Jan 2018 at 12:45, Pekka Paalanen  wrote:
> On Wed, 20 Dec 2017 12:26:51 +0000 Daniel Stone  wrote:
> > @@ -3116,9 +3136,16 @@ drm_output_propose_state(struct weston_output 
> > *output_base,
> >   if (next_plane == primary)
> >   pixman_region32_union(_region,
> > _region,
> > -   >transform.boundingbox);
> > +   _view);
> > + else if (output->cursor_plane &&
> > +  next_plane != >cursor_plane->base)
>
> Should this not be:
>
> if (!output->cursor_plane || next_plane != 
> >cursor_plane->base)
>
> so that lack of a cursor plane goes straight to occluded?

Very right. I've force-pushed this fix (and the resultant changes when
rebasing 'Add modes to drm_output_propose_state' as well as 'Return
plane state from plane preparation') to the v17 branch on Collabora
GitLab, but happy to re-send if that makes life easier.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v17.1 01/14] helpers: Move static_assert definition to shared

2018-07-09 Thread Daniel Stone
Collect the fallback definitions of static_assert() from desktop-shell
and the test shell, and move them to helpers.h. This allows code
throughout the tree to use static_assert() for build-time assertions,
where it is supported by the compiler.

As GCC goes out of its way to only add static_assert() when C11 has been
explicitly requested - which we don't do - make sure to use the more
widely available _Static_assert() if that is provided.

This will be used in future patches to ensure two array lengths don't go
out of sync.

Signed-off-by: Daniel Stone 
---
 desktop-shell/shell.c |  4 
 shared/helpers.h  | 33 +++
 tests/weston-test-desktop-shell.c |  4 
 3 files changed, 33 insertions(+), 8 deletions(-)

On Mon, 9 Jul 2018 at 14:54, Pekka Paalanen  wrote:
> I'm fairly paranoid of copying GPL code. The implementation sure is
> trivial, but what exactly did you take from the kernel?
> The documentation bit looks already copyrightable.

They probably have better documentation. :) I wrote that documentation
myself, so it's just the macro body which was taken from the kernel. My
understanding is that this isn't copyrightable.

> Did you know of the existing uses of static_assert in Weston?
> Oh, but there are no uses left, all we have left are the fallback empty
> definitions... two of them. >_<

But that's a good point. How about the following patch instead, which
uses static_assert() where possible? It turns out static_assert() was
actually mostly not defined, but I tried this one with:
static_assert(1 != 0, "must not trigger");
static_assert(1 == 0, "must trigger");

... and that worked.


diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 8b7a23ade..ea3c45354 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -47,10 +47,6 @@
 #define DEFAULT_NUM_WORKSPACES 1
 #define DEFAULT_WORKSPACE_CHANGE_ANIMATION_LENGTH 200
 
-#ifndef static_assert
-#define static_assert(cond, msg)
-#endif
-
 struct focus_state {
struct desktop_shell *shell;
struct weston_seat *seat;
diff --git a/shared/helpers.h b/shared/helpers.h
index 46f745d1b..9e1de947e 100644
--- a/shared/helpers.h
+++ b/shared/helpers.h
@@ -100,6 +100,39 @@ extern "C" {
(type *)( (char *)__mptr - offsetof(type,member) );})
 #endif
 
+/**
+ * Build-time static assertion support
+ *
+ * A build-time equivalent to assert(), will generate a compilation error
+ * if the supplied condition does not evaluate true.
+ *
+ * The following example demonstrates use of static_assert to ensure that
+ * arrays which are supposed to mirror each other have a consistent
+ * size.
+ *
+ * This is only a fallback definition; support must be provided by the
+ * compiler itself.
+ *
+ * @code
+ * int small[4];
+ * long expanded[4];
+ *
+ * static_assert(ARRAY_LENGTH(small) != ARRAY_LENGTH(expanded));
+ * for (i = 0; i < ARRAY_LENGTH(small); i++)
+ * expanded[i] = small[4];
+ * @endcode
+ *
+ * @param condition Expression to check for truth
+ * @param msg Message to print on failure
+ */
+#ifndef static_assert
+# ifdef _Static_assert
+#  define static_assert(cond, msg) _Static_assert(cond, msg)
+# else
+#  define static_assert(cond, msg)
+# endif
+#endif
+
 #ifdef  __cplusplus
 }
 #endif
diff --git a/tests/weston-test-desktop-shell.c 
b/tests/weston-test-desktop-shell.c
index de8442512..c780316d9 100644
--- a/tests/weston-test-desktop-shell.c
+++ b/tests/weston-test-desktop-shell.c
@@ -41,10 +41,6 @@
 #include "shared/helpers.h"
 #include "libweston-desktop/libweston-desktop.h"
 
-#ifndef static_assert
-#define static_assert(cond, msg)
-#endif
-
 struct desktest_shell {
struct wl_listener compositor_destroy_listener;
struct weston_desktop *desktop;
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v17 07/14] compositor-drm: Split drm_assign_planes in two

2018-07-09 Thread Daniel Stone
Move drm_assign_planes into two functions: one which proposes a plane
configuration, and another which applies that state to the Weston
internal structures. This will be used to try multiple configurations
and see which is supported.

Signed-off-by: Daniel Stone 
Reviewed-by: Pekka Paalanen 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 155 +++--
 1 file changed, 97 insertions(+), 58 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index acaf2639e..2eb3d4073 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -407,6 +407,8 @@ struct drm_plane_state {
 
struct drm_fb *fb;
 
+   struct weston_view *ev; /**< maintained for drm_assign_planes only */
+
int32_t src_x, src_y;
uint32_t src_w, src_h;
int32_t dest_x, dest_y;
@@ -1974,6 +1976,7 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
}
 
state->fb = fb;
+   state->ev = ev;
state->output = output;
if (!drm_plane_state_coords_for_view(state, ev))
goto err;
@@ -3045,6 +3048,7 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
}
 
state->fb = fb;
+   state->ev = ev;
state->output = output;
 
if (!drm_plane_state_coords_for_view(state, ev))
@@ -3172,6 +3176,7 @@ drm_output_prepare_cursor_view(struct drm_output_state 
*output_state,
}
 
output->cursor_view = ev;
+   plane_state->ev = ev;
 
plane_state->fb =
drm_fb_ref(output->gbm_cursor_fb[output->current_cursor]);
@@ -3248,40 +3253,15 @@ err:
drmModeSetCursor(b->drm.fd, output->crtc_id, 0, 0, 0);
 }
 
-/*
- * Get the aspect-ratio from drmModeModeInfo mode flags.
- *
- * @param drm_mode_flags- flags from drmModeModeInfo structure.
- * @returns aspect-ratio as encoded in enum 'weston_mode_aspect_ratio'.
- */
-static enum weston_mode_aspect_ratio
-drm_to_weston_mode_aspect_ratio(uint32_t drm_mode_flags)
-{
-   return (drm_mode_flags & DRM_MODE_FLAG_PIC_AR_MASK) >>
-   DRM_MODE_FLAG_PIC_AR_BITS_POS;
-}
-
-static const char *
-aspect_ratio_to_string(enum weston_mode_aspect_ratio ratio)
-{
-   if (ratio < 0 || ratio >= ARRAY_LENGTH(aspect_ratio_as_string) ||
-   !aspect_ratio_as_string[ratio])
-   return " (unknown aspect ratio)";
-
-   return aspect_ratio_as_string[ratio];
-}
-
-static void
-drm_assign_planes(struct weston_output *output_base, void *repaint_data)
+static struct drm_output_state *
+drm_output_propose_state(struct weston_output *output_base,
+struct drm_pending_state *pending_state)
 {
-   struct drm_backend *b = to_drm_backend(output_base->compositor);
-   struct drm_pending_state *pending_state = repaint_data;
struct drm_output *output = to_drm_output(output_base);
struct drm_output_state *state;
-   struct drm_plane_state *plane_state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region;
-   struct weston_plane *primary, *next_plane;
+   struct weston_plane *primary = _base->compositor->primary_plane;
 
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
@@ -3302,35 +3282,21 @@ drm_assign_planes(struct weston_output *output_base, 
void *repaint_data)
 * as we do for flipping full screen surfaces.
 */
pixman_region32_init(_region);
-   primary = _base->compositor->primary_plane;
 
wl_list_for_each(ev, _base->compositor->view_list, link) {
-   struct weston_surface *es = ev->surface;
-
-   /* Test whether this buffer can ever go into a plane:
-* non-shm, or small enough to be a cursor.
-*
-* Also, keep a reference when using the pixman renderer.
-* That makes it possible to do a seamless switch to the GL
-* renderer and since the pixman renderer keeps a reference
-* to the buffer anyway, there is no side effects.
-*/
-   if (b->use_pixman ||
-   (es->buffer_ref.buffer &&
-   (!wl_shm_buffer_get(es->buffer_ref.buffer->resource) ||
-(ev->surface->width <= b->cursor_width &&
- ev->surface->height <= b->cursor_height
-   es->keep_buffer = true;
-   else
-   es->keep_buffer = false;
+   struct weston_plane *next_plane = NULL;
 
+   /* Since we process views from top to bottom, we know that if
+* the view intersects the calculated renderer region, it must
+* be part of, 

[PATCH v17 09/14] compositor-drm: Ignore occluded views

2018-07-09 Thread Daniel Stone
When trying to assign planes, keep track of the areas which are
already occluded, and ignore views which are completely occluded. This
allows us to build a state using planes only, when there are occluded
views which cannot go into a plane behind views which can.

Signed-off-by: Daniel Stone 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 37 -
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index ca885f96e..b4a65209a 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3252,7 +3252,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
struct drm_output *output = to_drm_output(output_base);
struct drm_output_state *state;
struct weston_view *ev;
-   pixman_region32_t surface_overlap, renderer_region;
+   pixman_region32_t surface_overlap, renderer_region, occluded_region;
struct weston_plane *primary = _base->compositor->primary_plane;
 
assert(!output->state_last);
@@ -3274,9 +3274,12 @@ drm_output_propose_state(struct weston_output 
*output_base,
 * as we do for flipping full screen surfaces.
 */
pixman_region32_init(_region);
+   pixman_region32_init(_region);
 
wl_list_for_each(ev, _base->compositor->view_list, link) {
struct weston_plane *next_plane = NULL;
+   pixman_region32_t clipped_view;
+   bool occluded = false;
 
/* If this view doesn't touch our output at all, there's no
 * reason to do anything with it. */
@@ -3288,13 +3291,27 @@ drm_output_propose_state(struct weston_output 
*output_base,
if (ev->output_mask != (1u << output->base.id))
next_plane = primary;
 
+   /* Ignore views we know to be totally occluded. */
+   pixman_region32_init(_view);
+   pixman_region32_intersect(_view,
+ >transform.boundingbox,
+ >base.region);
+
+   pixman_region32_init(_overlap);
+   pixman_region32_subtract(_overlap, _view,
+_region);
+   occluded = !pixman_region32_not_empty(_overlap);
+   if (occluded) {
+   pixman_region32_fini(_overlap);
+   pixman_region32_fini(_view);
+   continue;
+   }
+
/* Since we process views from top to bottom, we know that if
 * the view intersects the calculated renderer region, it must
 * be part of, or occluded by, it, and cannot go on a plane. */
-   pixman_region32_init(_overlap);
pixman_region32_intersect(_overlap, _region,
- >transform.boundingbox);
-
+ _view);
if (pixman_region32_not_empty(_overlap))
next_plane = primary;
pixman_region32_fini(_overlap);
@@ -3302,6 +3319,9 @@ drm_output_propose_state(struct weston_output 
*output_base,
if (next_plane == NULL)
next_plane = drm_output_prepare_cursor_view(state, ev);
 
+   if (next_plane == NULL && !drm_view_is_opaque(ev))
+   next_plane = primary;
+
if (next_plane == NULL)
next_plane = drm_output_prepare_scanout_view(state, ev);
 
@@ -3314,9 +3334,16 @@ drm_output_propose_state(struct weston_output 
*output_base,
if (next_plane == primary)
pixman_region32_union(_region,
  _region,
- >transform.boundingbox);
+ _view);
+   else if (output->cursor_plane &&
+next_plane != >cursor_plane->base)
+   pixman_region32_union(_region,
+ _region,
+ _view);
+   pixman_region32_fini(_view);
}
pixman_region32_fini(_region);
+   pixman_region32_fini(_region);
 
return state;
 }
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v17 12/14] compositor-drm: Add test-only mode to state application

2018-07-09 Thread Daniel Stone
The atomic API can allow us to test state before we apply it, to see if
it will be valid. Add support for this, which we will later use in
assign_planes to ensure our plane configuration is valid.

Signed-off-by: Daniel Stone 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 176 -
 1 file changed, 136 insertions(+), 40 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index ec0f9e7eb..0a3f524f5 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -260,6 +260,7 @@ enum drm_output_state_duplicate_mode {
 enum drm_state_apply_mode {
DRM_STATE_APPLY_SYNC, /**< state fully processed */
DRM_STATE_APPLY_ASYNC, /**< state pending event delivery */
+   DRM_STATE_TEST_ONLY, /**< test if the state can be applied */
 };
 
 struct drm_backend {
@@ -1812,6 +1813,7 @@ drm_pending_state_get_output(struct drm_pending_state 
*pending_state,
 }
 
 static int drm_pending_state_apply_sync(struct drm_pending_state *state);
+static int drm_pending_state_test(struct drm_pending_state *state);
 
 /**
  * Mark a drm_output_state (the output's last state) as complete. This handles
@@ -1936,13 +1938,16 @@ enum drm_output_propose_state_mode {
 
 static struct drm_plane_state *
 drm_output_prepare_scanout_view(struct drm_output_state *output_state,
-   struct weston_view *ev)
+   struct weston_view *ev,
+   enum drm_output_propose_state_mode mode)
 {
struct drm_output *output = output_state->output;
struct drm_plane *scanout_plane = output->scanout_plane;
struct drm_plane_state *state;
+   struct drm_plane_state *state_old = NULL;
struct drm_fb *fb;
pixman_box32_t *extents;
+   int ret;
 
/* Check the view spans exactly the output size, calculated in the
 * logical co-ordinate space. */
@@ -1967,11 +1972,18 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
}
 
state = drm_output_state_get_plane(output_state, scanout_plane);
-   if (state->fb) {
-   /* If there is already a framebuffer on the scanout plane,
-* a client view has already been placed on the scanout
-* view. In that case, do not free or put back the state,
-* but just leave it in place and quietly exit. */
+
+   /* Check if we've already placed a buffer on this plane; if there's a
+* buffer there but it comes from GBM, then it's the result of
+* drm_output_propose_state placing it here for testing purposes. */
+   if (state->fb &&
+   (state->fb->type == BUFFER_GBM_SURFACE ||
+state->fb->type == BUFFER_PIXMAN_DUMB)) {
+   state_old = calloc(1, sizeof(*state_old));
+   memcpy(state_old, state, sizeof(*state_old));
+   state_old->output_state = NULL;
+   wl_list_init(_old->link);
+   } else if (state->fb) {
drm_fb_unref(fb);
return NULL;
}
@@ -1991,10 +2003,24 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
 
+   if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) {
+   drm_plane_state_free(state_old, false);
+   return state;
+   }
+
+   ret = drm_pending_state_test(output_state->pending_state);
+   if (ret != 0)
+   goto err;
+
+   drm_plane_state_free(state_old, false);
return state;
 
 err:
-   drm_plane_state_put_back(state);
+   drm_plane_state_free(state, false);
+   if (state_old) {
+   wl_list_insert(_state->plane_list, _old->link);
+   state_old->output_state = output_state;
+   }
return NULL;
 }
 
@@ -2062,7 +2088,9 @@ drm_output_render(struct drm_output_state *state, 
pixman_region32_t *damage)
 * want to render. */
scanout_state = drm_output_state_get_plane(state,
   output->scanout_plane);
-   if (scanout_state->fb)
+   if (scanout_state->fb &&
+   scanout_state->fb->type != BUFFER_GBM_SURFACE &&
+   scanout_state->fb->type != BUFFER_PIXMAN_DUMB)
return;
 
if (!pixman_region32_not_empty(damage) &&
@@ -2085,6 +2113,7 @@ drm_output_render(struct drm_output_state *state, 
pixman_region32_t *damage)
return;
}
 
+   drm_fb_unref(scanout_state->fb);
scanout_state->fb = fb;
scanout_state->output = output;
 
@@ -2597,9 +2626,16 @@ drm_pending_state_apply_atomic(struct drm_pending_state 
*pending_state,
case DRM_STATE_APPLY_ASYNC:

[PATCH v17 14/14] compositor-drm: Enable planes for atomic

2018-07-09 Thread Daniel Stone
Now that we can sensibly test proposed plane configurations with atomic,
sprites are not broken.

Signed-off-by: Daniel Stone 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index b97872aa1..f0f40385d 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3772,6 +3772,17 @@ init_kms_caps(struct drm_backend *b)
weston_log("DRM: %s atomic modesetting\n",
   b->atomic_modeset ? "supports" : "does not support");
 
+   /*
+* KMS support for hardware planes cannot properly synchronize
+* without nuclear page flip. Without nuclear/atomic, hw plane
+* and cursor plane updates would either tear or cause extra
+* waits for vblanks which means dropping the compositor framerate
+* to a fraction. For cursors, it's not so bad, so they are
+* enabled.
+*/
+   if (!b->atomic_modeset)
+   b->sprites_are_broken = 1;
+
ret = drmSetClientCap(b->drm.fd, DRM_CLIENT_CAP_ASPECT_RATIO, 1);
b->aspect_ratio_supported = (ret == 0);
weston_log("DRM: %s picture aspect ratio\n",
@@ -6652,17 +6663,6 @@ drm_backend_create(struct weston_compositor *compositor,
b->drm.fd = -1;
wl_array_init(>unused_crtcs);
 
-   /*
-* KMS support for hardware planes cannot properly synchronize
-* without nuclear page flip. Without nuclear/atomic, hw plane
-* and cursor plane updates would either tear or cause extra
-* waits for vblanks which means dropping the compositor framerate
-* to a fraction. For cursors, it's not so bad, so they are
-* enabled.
-*
-* These can be enabled again when nuclear/atomic support lands.
-*/
-   b->sprites_are_broken = 1;
b->compositor = compositor;
b->use_pixman = config->use_pixman;
b->pageflip_timeout = config->pageflip_timeout;
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v17 11/14] compositor-drm: Return plane state from plane preparation

2018-07-09 Thread Daniel Stone
Return a pointer to the plane state, rather than indirecting via a
weston_plane.

Signed-off-by: Daniel Stone 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 71 +-
 1 file changed, 39 insertions(+), 32 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index c91428d96..ec0f9e7eb 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1934,7 +1934,7 @@ enum drm_output_propose_state_mode {
DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY, /**< only assign to planes */
 };
 
-static struct weston_plane *
+static struct drm_plane_state *
 drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
 {
@@ -1991,7 +1991,7 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
state->dest_h != (unsigned) output->base.current_mode->height)
goto err;
 
-   return _plane->base;
+   return state;
 
 err:
drm_plane_state_put_back(state);
@@ -2157,7 +2157,6 @@ drm_output_apply_state_legacy(struct drm_output_state 
*state)
struct drm_property_info *dpms_prop;
struct drm_plane_state *scanout_state;
struct drm_plane_state *ps;
-   struct drm_plane *p;
struct drm_mode *mode;
struct drm_head *head;
uint32_t connectors[MAX_CLONED_CONNECTORS];
@@ -2184,7 +2183,7 @@ drm_output_apply_state_legacy(struct drm_output_state 
*state)
 
if (state->dpms != WESTON_DPMS_ON) {
wl_list_for_each(ps, >plane_list, link) {
-   p = ps->plane;
+   struct drm_plane *p = ps->plane;
assert(ps->fb == NULL);
assert(ps->output == NULL);
 
@@ -2274,8 +2273,8 @@ drm_output_apply_state_legacy(struct drm_output_state 
*state)
.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT,
.request.sequence = 1,
};
+   struct drm_plane *p = ps->plane;
 
-   p = ps->plane;
if (p->type != WDRM_PLANE_TYPE_OVERLAY)
continue;
 
@@ -2987,7 +2986,7 @@ atomic_flip_handler(int fd, unsigned int frame, unsigned 
int sec,
 }
 #endif
 
-static struct weston_plane *
+static struct drm_plane_state *
 drm_output_prepare_overlay_view(struct drm_output_state *output_state,
struct weston_view *ev)
 {
@@ -3059,7 +3058,7 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
state->src_h != state->dest_h << 16)
goto err;
 
-   return >base;
+   return state;
 
 err:
drm_plane_state_put_back(state);
@@ -3103,7 +3102,7 @@ cursor_bo_update(struct drm_plane_state *plane_state, 
struct weston_view *ev)
weston_log("failed update cursor: %m\n");
 }
 
-static struct weston_plane *
+static struct drm_plane_state *
 drm_output_prepare_cursor_view(struct drm_output_state *output_state,
   struct weston_view *ev)
 {
@@ -3190,7 +3189,7 @@ drm_output_prepare_cursor_view(struct drm_output_state 
*output_state,
plane_state->dest_w = b->cursor_width;
plane_state->dest_h = b->cursor_height;
 
-   return >base;
+   return plane_state;
 
 err:
drm_plane_state_put_back(plane_state);
@@ -3260,7 +3259,6 @@ drm_output_propose_state(struct weston_output 
*output_base,
struct drm_output_state *state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
-   struct weston_plane *primary = _base->compositor->primary_plane;
bool renderer_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
bool planes_ok = !b->sprites_are_broken;
 
@@ -3286,7 +3284,8 @@ drm_output_propose_state(struct weston_output 
*output_base,
pixman_region32_init(_region);
 
wl_list_for_each(ev, _base->compositor->view_list, link) {
-   struct weston_plane *next_plane = NULL;
+   struct drm_plane_state *ps = NULL;
+   bool force_renderer = false;
pixman_region32_t clipped_view;
bool occluded = false;
 
@@ -3298,7 +3297,7 @@ drm_output_propose_state(struct weston_output 
*output_base,
/* We only assign planes to views which are exclusively present
 * on our output. */
if (ev->output_mask != (1u << output->base.id))
-   next_plane = primary;
+   force_renderer = true;
 
/* Ignore views we know to be totally occluded. */
pixman_region32_init(_view);
@@ -3322,40 +3321,48 @@ drm_output_propose_state(struct weston_output 
*output_base,
pixman_region32_intersect(_over

[PATCH v17 10/14] compositor-drm: Add modes to drm_output_propose_state

2018-07-09 Thread Daniel Stone
Add support for multiple modes: toggling whether or not the renderer
and/or planes are acceptable. This will be used to implement a smarter
plane-placement heuristic when we have support for testing output
states.

Signed-off-by: Daniel Stone 
Tested-by: Emre Ucan 
---
 libweston/compositor-drm.c | 39 ++
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index b4a65209a..c91428d96 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1929,6 +1929,11 @@ drm_output_assign_state(struct drm_output_state *state,
}
 }
 
+enum drm_output_propose_state_mode {
+   DRM_OUTPUT_PROPOSE_STATE_MIXED, /**< mix renderer & planes */
+   DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY, /**< only assign to planes */
+};
+
 static struct weston_plane *
 drm_output_prepare_scanout_view(struct drm_output_state *output_state,
struct weston_view *ev)
@@ -3247,13 +3252,17 @@ err:
 
 static struct drm_output_state *
 drm_output_propose_state(struct weston_output *output_base,
-struct drm_pending_state *pending_state)
+struct drm_pending_state *pending_state,
+enum drm_output_propose_state_mode mode)
 {
struct drm_output *output = to_drm_output(output_base);
+   struct drm_backend *b = to_drm_backend(output_base->compositor);
struct drm_output_state *state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region, occluded_region;
struct weston_plane *primary = _base->compositor->primary_plane;
+   bool renderer_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
+   bool planes_ok = !b->sprites_are_broken;
 
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
@@ -3316,36 +3325,49 @@ drm_output_propose_state(struct weston_output 
*output_base,
next_plane = primary;
pixman_region32_fini(_overlap);
 
-   if (next_plane == NULL)
+   /* The cursor plane is 'special' in the sense that we can still
+* place it in the legacy API, and we gate that with a separate
+* cursors_are_broken flag. */
+   if (next_plane == NULL && !b->cursors_are_broken)
next_plane = drm_output_prepare_cursor_view(state, ev);
 
if (next_plane == NULL && !drm_view_is_opaque(ev))
next_plane = primary;
 
+   if (next_plane == NULL && !planes_ok)
+   next_plane = primary;
+
if (next_plane == NULL)
next_plane = drm_output_prepare_scanout_view(state, ev);
 
if (next_plane == NULL)
next_plane = drm_output_prepare_overlay_view(state, ev);
 
-   if (next_plane == NULL)
-   next_plane = primary;
+   if (!next_plane || next_plane == primary) {
+   if (!renderer_ok)
+   goto err;
 
-   if (next_plane == primary)
pixman_region32_union(_region,
  _region,
  _view);
-   else if (output->cursor_plane &&
-next_plane != >cursor_plane->base)
+   } else if (output->cursor_plane &&
+  next_plane != >cursor_plane->base) {
pixman_region32_union(_region,
  _region,
  _view);
+   }
pixman_region32_fini(_view);
}
pixman_region32_fini(_region);
pixman_region32_fini(_region);
 
return state;
+
+err:
+   pixman_region32_fini(_region);
+   pixman_region32_fini(_region);
+   drm_output_state_free(state);
+   return NULL;
 }
 
 static void
@@ -3359,7 +3381,8 @@ drm_assign_planes(struct weston_output *output_base, void 
*repaint_data)
struct weston_view *ev;
struct weston_plane *primary = _base->compositor->primary_plane;
 
-   state = drm_output_propose_state(output_base, pending_state);
+   state = drm_output_propose_state(output_base, pending_state,
+DRM_OUTPUT_PROPOSE_STATE_MIXED);
 
wl_list_for_each(ev, _base->compositor->view_list, link) {
struct drm_plane *target_plane = NULL;
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v17 03/14] compositor-drm: Don't set fb->size for non-dumb buffers

2018-07-09 Thread Daniel Stone
When creating a drm_fb from client (wl_buffer/dmabuf), gbm_surface, or
client buffers, set fb->size to 0. The size member is only used for dumb
buffers, where we mmap the whole buffer, and need the size recorded to
later pass to munmap.

Determining the full size of multi-planar buffers is difficult, as
auxiliary planes are not guaranteed to have a (height*stride)
allocation, e.g. if they are subsampled or if they do not contain pixel
data at all but, e.g., compression information. Single-plane tiled
buffers also often pad the buffer allocation to a multiple of tile
height, making our existing calculation incorrect.

Though it does no harm to record incorrect information, it also does
no good as we never use it; remove it in order to avoid any confusion.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 7753dfba6..29fa98da6 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1063,7 +1063,7 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend 
*backend,
fb->handles[0] = gbm_bo_get_handle(bo).u32;
fb->format = pixel_format_get_info(gbm_bo_get_format(bo));
fb->modifier = DRM_FORMAT_MOD_INVALID;
-   fb->size = fb->strides[0] * fb->height;
+   fb->size = 0;
fb->fd = backend->drm.fd;
 
if (!fb->format) {
-- 
2.17.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


<    3   4   5   6   7   8   9   10   11   12   >