Date: Thu, 30 Mar 2017 18:40:33 +0200 From: Matthew Rezny <rezny@freebsd.org> To: Johannes M Dieterich <jmd@freebsd.org> Cc: Jan Beich <jbeich@freebsd.org>, svn-ports-head@freebsd.org, svn-ports-all@freebsd.org, ports-committers@freebsd.org, owner-ports-committers@freebsd.org, swills@freebsd.org, kwm@freebsd.org Subject: Re: svn commit: r437215 - in head/graphics: gbm libEGL libGL libglapi Message-ID: <3165067.gcRftSb7iV@workstation.reztek> In-Reply-To: <425ed90f9f572884f734898c9997e499@freebsd.org> References: <201703291657.v2TGvrpM076369@repo.freebsd.org> <19694803.doR80QHWTi@workstation.reztek> <425ed90f9f572884f734898c9997e499@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday 30 March 2017 11:39:11 Johannes M Dieterich wrote:
> Dear Matthew,
> CC: Jan, Koop, Steve
>
> I will unify your two mails below.
>
> On 2017-03-30 01:26, Matthew Rezny wrote:
> > On Thursday 30 March 2017 05:36:03 Jan Beich wrote:
> >> Matthew Rezny <rezny@freebsd.org> writes:
> >> > On Wednesday 29 March 2017 19:51:55 Jan Beich wrote:
> >> >> Matthew Rezny <rezny@FreeBSD.org> writes:
> >> >> > - @${REINPLACE_CMD} -e 's|x86_64|amd64|' \
> >> >> > + @${REINPLACE_CMD} -e 's|x86_64|amd64|' -e 's|\\S\*//|[:space:]*
> >
> > //|'
> >
> >> >> > \
> >> >>
> >> >> [:space:] is invalid character class thus treated as a list of
> >> >> characters.
> >> >> \S corresponds to [^[:space:]], while \s to [[:space:]].
> >> >>
> >> >> $ man pcrepattern | col -b | fgrep -m1 \\S
> >> >>
> >> >> \S any character that is not a white space character
> >> >>
> >> >> This may break build given -march, etc. are no longer stripped.
> >> >
> >> > I wish that information had been presented when I said "I guess it
> >> > should
> >> > have been [:space:] instead of [:graph:] in the replacement." after you
> >> > stated [:graph:] was plain incorrect, although it is what had been
> >> > previously suggested to me and did seem to be working.
> >>
> >> I didn't focus on pointing out every mistake with the existing hack
> >> because it was soon going away. Now that devel/libclc depends on
> >> llvm40
> >> the original motivation to hold out on 17.* (bug 217016) before 2017Q2
> >> has been weakened.
> >
> > Pointing out something is wrong without giving the correct solution is
> > not
> > helpful. The upstream change in the 17.1-dev branch was not directly
> > applicable to 13.0.x, so the the 'hack' would remain unless I was going
> > to
> > change to change the configure.ac and patch-configure myself, which is
> > certainly
> > more work that to edit the post-patch and it would have been the same
> > changes
> > in either place lacking clearer input.
> >
> > Nobody said anything to me about committing an update to libclc at this
> > moment. I do not believe that has tested in combination with the rest
> > of the
> > graphics stack at the current versions in ports, the mix of LLVM
> > versions
> > almost certainly will be a problem, and it's a day before the quarterly
> > branch. WTF? I've been holding off Mesa 17.x and LLVM4.0 for after that
> > branch
> > while attempting to get Xorg 1.19 ready in time for that. The latter
> > won't
> > happen because it took over a week to even start an exp-run on the
> > bsd.xorg.mk
> > changes. I just explained the reason for holding of an update of libclc
> > yesterday in a PR that proposed a more recent snapshot for the
> > transition to
> > dependence on llvm40. I had not even gotten everything onto llvm39 yet;
> > pocl
> > 0.14 should be compatible past 3.8, but it is not yet released and I
> > had
> > difficulties with rc1 as they switched to cmake so the build system
> > needs to be
> > redone. While I'm sure Mesa 17.0.x will be ok with llvm40 after
> > appropriate
> > patching (I had to add several patches for clover in Mesa 13), I cannot
> > say
> > the same for all the OpenCL ports, i.e. beignet which was only recently
> > made
> > compatible past llvm37 with the 1.3.0 update that allowed it to use
> > llvm39.
> > So I expect r438268 to be reverted, or someone else to handle all the
> > fallout
> > this causes on the quarterly branch during Q2. I realize multiple
> > people want
> > to help, but we need some coordination or else we are just wasting each
> > other's time. Sorry to be brunt, but that was an ill timed commit.
>
> This patch was committed by me, after https://reviews.freebsd.org/D9394
> got accepted by swills, reviewed by kwm (with x11 hat on) in the
> beginning of February. No objections were raised since then. I did not
> commit it earlier to wait for stable llvm40 to hit the tree. I again
> checked with swills yesterday before committing.
>
I was not aware of the review until the commit. I do not know why nobody
bothered to tell me other than there is serious lack of communication.
> Hence, I will back this out if either kwm or swills tell me to.
> Steve/Koop: opinions? I will also personally say that I believe we
> should only support one version of llvm (the latest stable) across Mesa
> ports if possible, which is another reason why my patch to
> graphics/libGL bumps to LLVM4.0. I do not see any added value despite
> more maintenance work in supporting older versions of LLVM for Mesa. The
> PR you referenced was also explicitly looking for support of a newer
> LLVM, not an older one.
>
I do not advocate sticking to older LLVM for long, and just made it easier for
people who want to try moving ahead. I merely want to wait until after the
quarterly branch to switch to llvm40 because I expect some fallout. Also, I do
want to be sure all the OpenCL ports can use that version. I had wanted to get
Xorg 1.19 before the quarterly branch, then after the branch do the rest, but
it'll all have to wait a bit to not have a broken quarterly.
> Concerning your changes to graphics/libGL: I appreciate you wanting to
> help and improve things. The Mesa ports are in dire need of that. I
> however would also appreciate if you would acknowledge the work other
> people (Mark Johnston, Johannes Lundberg, Kip Macy, Koop Mast, Pete
> Wright, and myself) have put into this over the last 6+ months. For some
> selected time line:
>
I appreciate Koop's past work and Macy's efforts throughout the past year. I
actually don't know what the rest of you have been up to since there is little
discussion. kwm and dumbbell are no longer active with x11 as far as I know
and rarely active on IRC. Macy used to be active on IRC but not lately, and
I'm not sure I've seen you on the xorg channel but I don't know what nick you
might use. As far as I know, I am the only one attending to the graphics ports
in FreeBSD for the past quarter. Macy mentioned trying to get more people
involved a few times, but since nobody else ever showed up on IRC, I had no
idea who might be up to what, at least until I saw your commits.
> * Johannes Lundberg enabled wayland and dri3 on the public
> FreeBSDDesktop github in last autumn
> * Kip and I changed to using libudev-devd there in December
> * I updated to Mesa 17.0 rc in January there
> * I bumped to llvm40 in the end of January there as that was needed to
> support Polaris and Carrizo (better)
> * before that part goes amiss: compilation with the same llvm as is
> linked against is also needed for amdgpu, we had crashes otherwise
Work in external repos doesn't help until it is merged. Nothing was pointed
out to me as merge ready. I know there is plenty of work going on there, but
as far as I knew, the focus is on the kernel modules and any changes to
userland/ports were just temporary support until the kernel parts will be
merged. Nobody said there was anything mergeable from the ports and I haven't
time to pick through it at to figure out what is ready. I have looked at some
of it, what I've stumbled on really, and not much looks like it was done with
an intent for clean merge so it requires more work to use on stock FreeBSD.
> * throughout this time kwm (with x11 hat on) was involved in these
> updates
Which time frame are you talking about? I haven't seen kwm active in x11
affairs in at least 4 months. I had opened the mass of PRs for Xorg 1.18 when I
started my 1.19 work in order to try to kick him, or anyone else into
committing, and the result was I got a bit to commit those bapt hadn't already
gotten around to. Since then I've been working mostly alone, not that I want
it that way.
> * I uploaded the first version of the Mesa 17.0 patch to reviews on Feb
> 7 (which you were apparently aware of)
It was pointed out to me when I was committing Mesa 13, at which time it was
definitely premature, so something to come back to but after some number of
times without seeing any change I assumed it dead, whoops. I think it was
still an RC, or it was 17.0.0 but llvm40 was at rc1 was time I looked.
> * I've since updated the patch multiple times for minor releases and
> address comments by mat and kwm (with x11 hat on)
> * again, no comments/objections were raised there by anybody else
>
I suppose I should have raised concerns earlier, but it looked too raw (too
much stuff specific to drm-next) for me to think it anywhere close to being
committed, or to think to search for something like an update to libclc. I was
also kinda hoping you'd join #freebsd-xorg at some point.
> The first Mesa 17 patch in PR217016 dates from March 6, a month after my
> first Mesa 17 patch on reviews. Apparently you were aware of (at least)
> the last two items on the list above. So yes, coordination is important,
> which is also why we have an x11 hat (Koop). As you can see from the
> above, Koop was involved in all of this from the onset.
>
I find it odd that Koop is barely responsive on IRC but active on some reviews.
Whatever, just part of the coordination and communication issues. Last time I
talked to kwm at any length he said he didn't really have time for FreeBSD, so
I counted him out since then, only bothering to ask the occasional question
without reply.
> > I had not noticed that your review was updated recently as it sat idle
> > for
> > quite a while after the update to 13.0.x landed in ports. I'm curious
> > about
> > the comment regarding a need for libudev. As far as I know, Mesa
> > dropped the
> > dependence on udev in v13 and relies upon libdrm for all the direct
> > hardware
> > interaction. Initially, I had some hope to use libudev-devd as a single
> > translation layer, but after reading through the libdrm code it became
> > obvious
> > that would not be the case as the conditional udev code in libdrm is
> > withing
> > Linux-specific code paths so no help to us. Thus, I implemented support
> > for our
> > platform directly in libdrm in order to drop dependence on libdevq,
> > which was
> > needed for no other purpose. Could you explain why Mesa 17 would need
> > libudev-
> > devd for AMDGPU?
>
> We used libudev for loader related functionality in Mesa. I am happy if
> we do not need it anymore, at the time Kip and I just wanted to get rid
> of libdevq -- a FreeBSD-ism. libdrm did not support the linuxkpi amdgpu
> and Mesa would not load radeonsi properly.
>
Is libdrm missing some functionality I should implement, or was it just a
matter that the version with libdevq hacked in was only partially working? I
have tried to address the shortcomings I knew of in libdrm and cater to the
differences I was aware of in drm-next, so it would be good to know if libdrm
sans libdevq works for what you need. I've only had test reports from users of
drm-next that have replaced libdrm and removed libdevq, but may still have a
modified version of Mesa. I do not know the extent of changes in the the
external ports collection.
> Concerning the libdrm udev: as you may have seen from the FreeBSDDesktop
> github, I tried to do that before Christmas w/o success.
>
I have not trawled through GitHub nor was I planning to. The work there is
focused on newer hardware than I own and I have enough else to keep me
occupied supporting the hardware I do own, which includes stuff I must revive
support for after it feel by the wayside over the last several years. If there
is stuff worth looking at, please point it out to me.
> >> > To be sure there is no misunderstanding now, would you consider this
> >> > correct? @${REINPLACE_CMD} -e 's|x86_64|amd64|' -e
> >> > 's|\\S\*//|[^[:space:]]* //|' \
> >>
> >> Not quite, adding extra space after [] is unnecessary.
> >>
> >> -e 's|\\S\*|[^[:space:]]*|' \
> >
> > It is interesting how many variations appear to produce the same
> > result. You
> > mention the space is not needed, so 's|\\S\*//|[^[:space:]]*//|' but
> > then the
> > example has the trailing // removed. Any reason why?
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3165067.gcRftSb7iV>