Bump libdrm-intel dependency library to a newer version that support softpin.

Submitted by marius vlad on July 14, 2016, 10:39 a.m.

Details

Message ID 1468492786-32226-1-git-send-email-marius.c.vlad@intel.com
State New
Headers show
Series "Bump libdrm-intel dependency library to a newer version that support softpin." ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

marius vlad July 14, 2016, 10:39 a.m.
Required by commit 2603b98ca (aubdump: Support softpin bos).

Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
CC: Kristian Høgsberg Kristensen <kristian.h.kristensen@intel.com>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/configure.ac b/configure.ac
index f05bcb0..ade9756 100644
--- a/configure.ac
+++ b/configure.ac
@@ -119,7 +119,7 @@  if test "x$GCC" = "xyes"; then
 fi
 AC_SUBST(ASSEMBLER_WARN_CFLAGS)
 
-PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
+PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
 PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
 
 case "$target_cpu" in

Comments

Hi Marius,

Just double-checking - this is an i-g-t patch, isn't it ?

On 14 July 2016 at 11:39, Marius Vlad <marius.c.vlad@intel.com> wrote:
> Required by commit 2603b98ca (aubdump: Support softpin bos).
>
> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> CC: Kristian Høgsberg Kristensen <kristian.h.kristensen@intel.com>
> ---
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index f05bcb0..ade9756 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
>  fi
>  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
>
> -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
> +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
Yes please. As you do that one can nuke most/all the "define LOCAL_"
and "struct local_" (in lib/ioctl_wrappers.h)
and replace with with the official symbols. A very nice plus imho ;-)

Side note: this will conflict with Robert Foss's work to make
libdrm_intel an optional dependency. Have you/others had the chance to
look at the series ? What can we do to get that moving/accepted ?

Thanks
Emil
On Thu, Jul 14, 2016 at 05:29:55PM +0100, Emil Velikov wrote:
> Hi Marius,
> 
> Just double-checking - this is an i-g-t patch, isn't it ?
> 
> On 14 July 2016 at 11:39, Marius Vlad <marius.c.vlad@intel.com> wrote:
> > Required by commit 2603b98ca (aubdump: Support softpin bos).
> >
> > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> > CC: Kristian Høgsberg Kristensen <kristian.h.kristensen@intel.com>
> > ---
> >  configure.ac | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index f05bcb0..ade9756 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
> >  fi
> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >
> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
> Yes please. As you do that one can nuke most/all the "define LOCAL_"
> and "struct local_" (in lib/ioctl_wrappers.h)
> and replace with with the official symbols. A very nice plus imho ;-)

Please don't. It makes running on older installations even more
cumbersome.
-Chris
On 14 July 2016 at 17:49, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, Jul 14, 2016 at 05:29:55PM +0100, Emil Velikov wrote:
>> Hi Marius,
>>
>> Just double-checking - this is an i-g-t patch, isn't it ?
>>
>> On 14 July 2016 at 11:39, Marius Vlad <marius.c.vlad@intel.com> wrote:
>> > Required by commit 2603b98ca (aubdump: Support softpin bos).
>> >
>> > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
>> > CC: Kristian Høgsberg Kristensen <kristian.h.kristensen@intel.com>
>> > ---
>> >  configure.ac | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/configure.ac b/configure.ac
>> > index f05bcb0..ade9756 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
>> >  fi
>> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
>> >
>> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
>> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
>> Yes please. As you do that one can nuke most/all the "define LOCAL_"
>> and "struct local_" (in lib/ioctl_wrappers.h)
>> and replace with with the official symbols. A very nice plus imho ;-)
>
> Please don't. It makes running on older installations even more
> cumbersome.
Slightly confused here: are you against the libdrm_intel bump, or the
removal of the local symbols ?

Admittedly sometimes distros don't bother/refuse to update libdrm
which could be an issue in the former case. Although if the package
(with all the definitions) is compulsory, how would that cause an
issue ?

Genuine question here, not trying to be smart/cheeky/etc.

Thanks
Emil
On Thu, Jul 14, 2016 at 07:54:59PM +0100, Emil Velikov wrote:
> On 14 July 2016 at 17:49, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Thu, Jul 14, 2016 at 05:29:55PM +0100, Emil Velikov wrote:
> >> Hi Marius,
> >>
> >> Just double-checking - this is an i-g-t patch, isn't it ?
> >>
> >> On 14 July 2016 at 11:39, Marius Vlad <marius.c.vlad@intel.com> wrote:
> >> > Required by commit 2603b98ca (aubdump: Support softpin bos).
> >> >
> >> > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> >> > CC: Kristian Høgsberg Kristensen <kristian.h.kristensen@intel.com>
> >> > ---
> >> >  configure.ac | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/configure.ac b/configure.ac
> >> > index f05bcb0..ade9756 100644
> >> > --- a/configure.ac
> >> > +++ b/configure.ac
> >> > @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
> >> >  fi
> >> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >> >
> >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
> >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
> >> Yes please. As you do that one can nuke most/all the "define LOCAL_"
> >> and "struct local_" (in lib/ioctl_wrappers.h)
> >> and replace with with the official symbols. A very nice plus imho ;-)
> >
> > Please don't. It makes running on older installations even more
> > cumbersome.
> Slightly confused here: are you against the libdrm_intel bump, or the
> removal of the local symbols ?

Local symbols. They save a lot of time if you can just get the test you
need compiling and not worry about dependencies. One of the basic
tenents is that we drop a new kernel into an old userspace and expect to
have not broken anything. Being lazy, for smoke testing I just build
in situ.

> Admittedly sometimes distros don't bother/refuse to update libdrm
> which could be an issue in the former case. Although if the package
> (with all the definitions) is compulsory, how would that cause an
> issue ?

The package may not even exist when testing on v.old distro images.
It is mostly a major of convenience, but since the work is already done
to be independent, removing causes more work.
-Chris
On 14 July 2016 at 21:03, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, Jul 14, 2016 at 07:54:59PM +0100, Emil Velikov wrote:
>> On 14 July 2016 at 17:49, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > On Thu, Jul 14, 2016 at 05:29:55PM +0100, Emil Velikov wrote:
>> >> Hi Marius,
>> >>
>> >> Just double-checking - this is an i-g-t patch, isn't it ?
>> >>
>> >> On 14 July 2016 at 11:39, Marius Vlad <marius.c.vlad@intel.com> wrote:
>> >> > Required by commit 2603b98ca (aubdump: Support softpin bos).
>> >> >
>> >> > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
>> >> > CC: Kristian Høgsberg Kristensen <kristian.h.kristensen@intel.com>
>> >> > ---
>> >> >  configure.ac | 2 +-
>> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/configure.ac b/configure.ac
>> >> > index f05bcb0..ade9756 100644
>> >> > --- a/configure.ac
>> >> > +++ b/configure.ac
>> >> > @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
>> >> >  fi
>> >> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
>> >> >
>> >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
>> >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
>> >> Yes please. As you do that one can nuke most/all the "define LOCAL_"
>> >> and "struct local_" (in lib/ioctl_wrappers.h)
>> >> and replace with with the official symbols. A very nice plus imho ;-)
>> >
>> > Please don't. It makes running on older installations even more
>> > cumbersome.
>> Slightly confused here: are you against the libdrm_intel bump, or the
>> removal of the local symbols ?
>
> Local symbols. They save a lot of time if you can just get the test you
> need compiling and not worry about dependencies. One of the basic
> tenents is that we drop a new kernel into an old userspace and expect to
> have not broken anything. Being lazy, for smoke testing I just build
> in situ.
>
Fully agree.

>> Admittedly sometimes distros don't bother/refuse to update libdrm
>> which could be an issue in the former case. Although if the package
>> (with all the definitions) is compulsory, how would that cause an
>> issue ?
>
> The package may not even exist when testing on v.old distro images.
> It is mostly a major of convenience, but since the work is already done
> to be independent, removing causes more work.
Seems that things are not completely independent, as illustrated by
the version bump.

Thus was wondering what's the benefit of (or why object against
nuking) the local copies. Since even if they were needed before, they
won't be after the bump.That said, it seems that it's a picky/sore
topic so I won't dig into it.

If anything I'll just send patches to be acked/nacked.
-Emil
On Thu, Jul 14, 2016 at 09:52:24PM +0100, Emil Velikov wrote:
> On 14 July 2016 at 21:03, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Thu, Jul 14, 2016 at 07:54:59PM +0100, Emil Velikov wrote:
> >> On 14 July 2016 at 17:49, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >> > On Thu, Jul 14, 2016 at 05:29:55PM +0100, Emil Velikov wrote:
> >> >> Hi Marius,
> >> >>
> >> >> Just double-checking - this is an i-g-t patch, isn't it ?
> >> >>
> >> >> On 14 July 2016 at 11:39, Marius Vlad <marius.c.vlad@intel.com> wrote:
> >> >> > Required by commit 2603b98ca (aubdump: Support softpin bos).
> >> >> >
> >> >> > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> >> >> > CC: Kristian Høgsberg Kristensen <kristian.h.kristensen@intel.com>
> >> >> > ---
> >> >> >  configure.ac | 2 +-
> >> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/configure.ac b/configure.ac
> >> >> > index f05bcb0..ade9756 100644
> >> >> > --- a/configure.ac
> >> >> > +++ b/configure.ac
> >> >> > @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
> >> >> >  fi
> >> >> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >> >> >
> >> >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
> >> >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
> >> >> Yes please. As you do that one can nuke most/all the "define LOCAL_"
> >> >> and "struct local_" (in lib/ioctl_wrappers.h)
> >> >> and replace with with the official symbols. A very nice plus imho ;-)
> >> >
> >> > Please don't. It makes running on older installations even more
> >> > cumbersome.
> >> Slightly confused here: are you against the libdrm_intel bump, or the
> >> removal of the local symbols ?
> >
> > Local symbols. They save a lot of time if you can just get the test you
> > need compiling and not worry about dependencies. One of the basic
> > tenents is that we drop a new kernel into an old userspace and expect to
> > have not broken anything. Being lazy, for smoke testing I just build
> > in situ.
> >
> Fully agree.
> 
> >> Admittedly sometimes distros don't bother/refuse to update libdrm
> >> which could be an issue in the former case. Although if the package
> >> (with all the definitions) is compulsory, how would that cause an
> >> issue ?
> >
> > The package may not even exist when testing on v.old distro images.
> > It is mostly a major of convenience, but since the work is already done
> > to be independent, removing causes more work.
> Seems that things are not completely independent, as illustrated by
> the version bump.
>
> Thus was wondering what's the benefit of (or why object against
> nuking) the local copies. Since even if they were needed before, they
> won't be after the bump.That said, it seems that it's a picky/sore
> topic so I won't dig into it.

I don't have uptodate or even recent libdrm everywhere. Getting rid of
the locals means another dependency chain to build.

Just being completely lazy and saying "if ain't broke, don't try to fix it".
-Chris
So, the issue is that i-g-t will not build on our builder system, with
the current version of librm-intel. It just happens to work because I
took the liberity of upgrading the entire system, a while ago.

No build, no newer i-g-t, no tests, no CI.

There are a few of _PINNED macros defined by Chris all over the
gem tests. Thought of doing just that, figured it would be better to
bump the library instead.

On Thu, Jul 14, 2016 at 05:29:55PM +0100, Emil Velikov wrote:
> Hi Marius,
> 
> Just double-checking - this is an i-g-t patch, isn't it ?

Sorry, my bad!

> 
> On 14 July 2016 at 11:39, Marius Vlad <marius.c.vlad@intel.com> wrote:
> > Required by commit 2603b98ca (aubdump: Support softpin bos).
> >
> > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> > CC: Kristian Høgsberg Kristensen <kristian.h.kristensen@intel.com>
> > ---
> >  configure.ac | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index f05bcb0..ade9756 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -119,7 +119,7 @@ if test "x$GCC" = "xyes"; then
> >  fi
> >  AC_SUBST(ASSEMBLER_WARN_CFLAGS)
> >
> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.64 libdrm])
> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.68 libdrm])
> Yes please. As you do that one can nuke most/all the "define LOCAL_"
> and "struct local_" (in lib/ioctl_wrappers.h)
> and replace with with the official symbols. A very nice plus imho ;-)
> 
> Side note: this will conflict with Robert Foss's work to make
> libdrm_intel an optional dependency. Have you/others had the chance to
> look at the series ? What can we do to get that moving/accepted ?

Yes I've seen them. Someone to Ack'em :-).

> 
> Thanks
> Emil
On 15 July 2016 at 16:42, Marius Vlad <marius.c.vlad@intel.com> wrote:

>> Side note: this will conflict with Robert Foss's work to make
>> libdrm_intel an optional dependency. Have you/others had the chance to
>> look at the series ? What can we do to get that moving/accepted ?
>
> Yes I've seen them. Someone to Ack'em :-).
>
Can you please elaborate who can/should Ack them - is that someone
from the IGT team ? From functionality POV they're pretty good, as
I've been through them a few times already (and have my r-b, fwiw).

Thanks
Emil
On Fri, Jul 22, 2016 at 03:21:29PM +0100, Emil Velikov wrote:
> On 15 July 2016 at 16:42, Marius Vlad <marius.c.vlad@intel.com> wrote:
> 
> >> Side note: this will conflict with Robert Foss's work to make
> >> libdrm_intel an optional dependency. Have you/others had the chance to
> >> look at the series ? What can we do to get that moving/accepted ?
> >
> > Yes I've seen them. Someone to Ack'em :-).
> >
> Can you please elaborate who can/should Ack them - is that someone
> from the IGT team ? From functionality POV they're pretty good, as
> I've been through them a few times already (and have my r-b, fwiw).

Not sure where the work from Robert Foss is right now tbh ... Do you want
commit rights to igt to move it forward? Preemptive ack from me, just ask
Daniel Stone to add you to the right groups on fd.o.
-Daniel
On 27 July 2016 at 11:23, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Jul 22, 2016 at 03:21:29PM +0100, Emil Velikov wrote:
>> On 15 July 2016 at 16:42, Marius Vlad <marius.c.vlad@intel.com> wrote:
>>
>> >> Side note: this will conflict with Robert Foss's work to make
>> >> libdrm_intel an optional dependency. Have you/others had the chance to
>> >> look at the series ? What can we do to get that moving/accepted ?
>> >
>> > Yes I've seen them. Someone to Ack'em :-).
>> >
>> Can you please elaborate who can/should Ack them - is that someone
>> from the IGT team ? From functionality POV they're pretty good, as
>> I've been through them a few times already (and have my r-b, fwiw).
>
> Not sure where the work from Robert Foss is right now tbh ... Do you want
> commit rights to igt to move it forward? Preemptive ack from me, just ask
> Daniel Stone to add you to the right groups on fd.o.
Ack will check with Daniel for next round. Getting access for
Tomeu/Rob or myself about pushing generic fixes to IGT will be great.

Meanwhile thanks for pushing v5 ;-)
-Emil