| Message ID | 20140905171959.GO5216@hansolo.jdub.homelinux.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Hi Josh On 05/09/14 18:19, Josh Boyer wrote: > The userspace drm.h include doesn't prefix the drm directory. This can lead > to compile failures as /usr/include/drm/ isn't in the standard gcc include > paths. Fix it to be <drm/drm.h>, which matches the rest of the driver drm > header files that get installed into /usr/include/drm. > Is this an actual issue or a hypothetical one ? Afaict no-one is using the kernel drm headers, but instead the ones from libdrm are in place. linux-headers does not even ship /usr/include/drm on my Archlinux box. Additionally most (all?) vmwgfx components (mesa, ddx) use a local version of the header, which albeit not ideal should not cause issues. Or perhaps I'm missing something ? To the VMware guys, Any objections if we update the libdrm header and drop the mesa/ddx copies ? Cheers, Emil P.S. I'm against the patch in any way :) > Red Hat Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1138759 > > Fixes: 1d7a5cbf8f74e > Reported-by: Jeffrey Bastian <jbastian@redhat.com> > Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> > --- > include/uapi/drm/vmwgfx_drm.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/uapi/drm/vmwgfx_drm.h b/include/uapi/drm/vmwgfx_drm.h > index 4fc66f6b12ce..c472bedbe38e 100644 > --- a/include/uapi/drm/vmwgfx_drm.h > +++ b/include/uapi/drm/vmwgfx_drm.h > @@ -29,7 +29,7 @@ > #define __VMWGFX_DRM_H__ > > #ifndef __KERNEL__ > -#include <drm.h> > +#include <drm/drm.h> > #endif > > #define DRM_VMW_MAX_SURFACE_FACES 6 >
On Mon, Sep 8, 2014 at 8:01 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: > Hi Josh > > On 05/09/14 18:19, Josh Boyer wrote: >> The userspace drm.h include doesn't prefix the drm directory. This can lead >> to compile failures as /usr/include/drm/ isn't in the standard gcc include >> paths. Fix it to be <drm/drm.h>, which matches the rest of the driver drm >> header files that get installed into /usr/include/drm. >> > Is this an actual issue or a hypothetical one ? Afaict no-one is using the > kernel drm headers, but instead the ones from libdrm are in place. > linux-headers does not even ship /usr/include/drm on my Archlinux box. It's shipped in kernel-headers in Fedora and I'm guessing someone hit it at one point, but I don't know what the actual initial problem was. > Additionally most (all?) vmwgfx components (mesa, ddx) use a local version of > the header, which albeit not ideal should not cause issues. > > Or perhaps I'm missing something ? > > > To the VMware guys, > > Any objections if we update the libdrm header and drop the mesa/ddx copies ? > > Cheers, > Emil > > P.S. I'm against the patch in any way :) Was that meant to say "I'm not against the patch in any way" ? josh
Hi! On 09/08/2014 02:01 PM, Emil Velikov wrote: > Hi Josh > > On 05/09/14 18:19, Josh Boyer wrote: >> The userspace drm.h include doesn't prefix the drm directory. This can lead >> to compile failures as /usr/include/drm/ isn't in the standard gcc include >> paths. Fix it to be <drm/drm.h>, which matches the rest of the driver drm >> header files that get installed into /usr/include/drm. >> > Is this an actual issue or a hypothetical one ? Afaict no-one is using the > kernel drm headers, but instead the ones from libdrm are in place. > linux-headers does not even ship /usr/include/drm on my Archlinux box. > > Additionally most (all?) vmwgfx components (mesa, ddx) use a local version of > the header, which albeit not ideal should not cause issues. > > Or perhaps I'm missing something ? > > > To the VMware guys, > > Any objections if we update the libdrm header and drop the mesa/ddx copies ? > > Cheers, > Emil Hi! vmwgfx libdrm is pretty obsolete and AFAIK not used by anyone. As such, it's unnecessary to release a new version of libdrm each time the vmwgfx header is updated, and I'd like to avoid that dependency. Better to keep the local copies in the gallium winsys and the DDX. Since the ioctl interface is backwards compatible, it doesn't really matter if the headers are slightly out of sync. /Thomas > > P.S. I'm against the patch in any way :) > >> Red Hat Bugzilla: https://urldefense.proofpoint.com/v1/url?u=https://bugzilla.redhat.com/show_bug.cgi?id%3D1138759&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=l5Ago9ekmVFZ3c4M6eauqrJWGwjf6fTb%2BP3CxbBFkVM%3D%0A&m=YR9QcTDSKJ%2BRxDHr%2BlEBv%2Fo37iPucyP5QKdoQnUPjcU%3D%0A&s=c8f9a09bb72428883d57b5ba757295a82326cfc0cac094c56c7754836054ae13 >> >> Fixes: 1d7a5cbf8f74e >> Reported-by: Jeffrey Bastian <jbastian@redhat.com> >> Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> >> --- >> include/uapi/drm/vmwgfx_drm.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/uapi/drm/vmwgfx_drm.h b/include/uapi/drm/vmwgfx_drm.h >> index 4fc66f6b12ce..c472bedbe38e 100644 >> --- a/include/uapi/drm/vmwgfx_drm.h >> +++ b/include/uapi/drm/vmwgfx_drm.h >> @@ -29,7 +29,7 @@ >> #define __VMWGFX_DRM_H__ >> >> #ifndef __KERNEL__ >> -#include <drm.h> >> +#include <drm/drm.h> >> #endif >> >> #define DRM_VMW_MAX_SURFACE_FACES 6 >>
On 08/09/14 13:14, Josh Boyer wrote: > On Mon, Sep 8, 2014 at 8:01 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote: [snip] >> Any objections if we update the libdrm header and drop the mesa/ddx copies ? >> >> Cheers, >> Emil >> >> P.S. I'm against the patch in any way :) > > Was that meant to say "I'm not against the patch in any way" ? > Indeed. Quite a silly typo on my behalf. -Emil > josh >
On 08/09/14 13:15, Thomas Hellstrom wrote: > Hi! > > On 09/08/2014 02:01 PM, Emil Velikov wrote: [snip] >> >> To the VMware guys, >> >> Any objections if we update the libdrm header and drop the mesa/ddx copies ? >> >> Cheers, >> Emil > Hi! > > vmwgfx libdrm is pretty obsolete and AFAIK not used by anyone. > As such, it's unnecessary to release a new version of libdrm each time > the vmwgfx header is updated, and I'd like to avoid that dependency. > Better to keep the local copies in the gallium winsys and the DDX. Since > the ioctl interface is backwards compatible, it doesn't really matter if > the headers are slightly out of sync. > Afaict libdrm releases are dirt cheap, and should not cause any issues. Yet it's your code so you'll be the judge of it all. I'm just a de-duplication fanatic :) -Emil
On Fri, Sep 5, 2014 at 1:19 PM, Josh Boyer <jwboyer@fedoraproject.org> wrote: > The userspace drm.h include doesn't prefix the drm directory. This can lead > to compile failures as /usr/include/drm/ isn't in the standard gcc include > paths. Fix it to be <drm/drm.h>, which matches the rest of the driver drm > header files that get installed into /usr/include/drm. > > Red Hat Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1138759 > > Fixes: 1d7a5cbf8f74e > Reported-by: Jeffrey Bastian <jbastian@redhat.com> > Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> Ping? Did this get queued anywhere? josh > --- > include/uapi/drm/vmwgfx_drm.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/uapi/drm/vmwgfx_drm.h b/include/uapi/drm/vmwgfx_drm.h > index 4fc66f6b12ce..c472bedbe38e 100644 > --- a/include/uapi/drm/vmwgfx_drm.h > +++ b/include/uapi/drm/vmwgfx_drm.h > @@ -29,7 +29,7 @@ > #define __VMWGFX_DRM_H__ > > #ifndef __KERNEL__ > -#include <drm.h> > +#include <drm/drm.h> > #endif > > #define DRM_VMW_MAX_SURFACE_FACES 6 > -- > 1.9.3 > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
On 16 September 2014 23:43, Josh Boyer <jwboyer@fedoraproject.org> wrote: > On Fri, Sep 5, 2014 at 1:19 PM, Josh Boyer <jwboyer@fedoraproject.org> wrote: >> The userspace drm.h include doesn't prefix the drm directory. This can lead >> to compile failures as /usr/include/drm/ isn't in the standard gcc include >> paths. Fix it to be <drm/drm.h>, which matches the rest of the driver drm >> header files that get installed into /usr/include/drm. >> >> Red Hat Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1138759 >> >> Fixes: 1d7a5cbf8f74e >> Reported-by: Jeffrey Bastian <jbastian@redhat.com> >> Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> > > Ping? Did this get queued anywhere? Yes its in drm-next. Dave.
On Tue, Sep 16, 2014 at 09:43:50 -0400, Josh Boyer wrote: > On Fri, Sep 5, 2014 at 1:19 PM, Josh Boyer <jwboyer@fedoraproject.org> wrote: > > The userspace drm.h include doesn't prefix the drm directory. This can lead > > to compile failures as /usr/include/drm/ isn't in the standard gcc include > > paths. Fix it to be <drm/drm.h>, which matches the rest of the driver drm > > header files that get installed into /usr/include/drm. > > > > Red Hat Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1138759 > > > > Fixes: 1d7a5cbf8f74e > > Reported-by: Jeffrey Bastian <jbastian@redhat.com> > > Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> > > Ping? Did this get queued anywhere? > Any reason this doesn't just #include "drm.h"? That should work whether the drm headers are installed in /usr/include/drm/ or /usr/include/libdrm/... Cheers, Julien, who doesn't have a /usr/include/drm/ either
diff --git a/include/uapi/drm/vmwgfx_drm.h b/include/uapi/drm/vmwgfx_drm.h index 4fc66f6b12ce..c472bedbe38e 100644 --- a/include/uapi/drm/vmwgfx_drm.h +++ b/include/uapi/drm/vmwgfx_drm.h @@ -29,7 +29,7 @@ #define __VMWGFX_DRM_H__ #ifndef __KERNEL__ -#include <drm.h> +#include <drm/drm.h> #endif #define DRM_VMW_MAX_SURFACE_FACES 6
The userspace drm.h include doesn't prefix the drm directory. This can lead to compile failures as /usr/include/drm/ isn't in the standard gcc include paths. Fix it to be <drm/drm.h>, which matches the rest of the driver drm header files that get installed into /usr/include/drm. Red Hat Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1138759 Fixes: 1d7a5cbf8f74e Reported-by: Jeffrey Bastian <jbastian@redhat.com> Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org> --- include/uapi/drm/vmwgfx_drm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)