Re: Advertising supported Dmabuf modifiers of DRM device
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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