drm: Fix drm.h uapi header for Windows

drm: Fix drm.h uapi header for Windows

[James Park]

Attempting to submit patch in response to https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6162#note_712454

This will allow Mesa to port code to Windows more easily.

Hide BSD header and drm_handle_t behind _WIN32 check.

Change __volatile__ to volatile, which is standard.

James Park (1):
  drm: Fix drm.h uapi header for Windows

 include/uapi/drm/drm.h | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: Fix drm.h uapi header for Windows

[James Park]

This will allow Mesa to port code to Windows more easily.

Hide BSD header and drm_handle_t behind _WIN32 check.

Change __volatile__ to volatile, which is standard.
---
 include/uapi/drm/drm.h | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 808b48a..53dc3c9 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -48,10 +48,9 @@ typedef unsigned int drm_handle_t;
 #include <asm/ioctl.h>
 typedef unsigned int drm_handle_t;
 
-#else /* One of the BSDs */
+#else
 
 #include <stdint.h>
-#include <sys/ioccom.h>
 #include <sys/types.h>
 typedef int8_t   __s8;
 typedef uint8_t  __u8;
@@ -62,10 +61,16 @@ typedef uint32_t __u32;
 typedef int64_t  __s64;
 typedef uint64_t __u64;
 typedef size_t   __kernel_size_t;
+
+#ifndef _WIN32 /* One of the BSDs */
+
+#include <sys/ioccom.h>
 typedef unsigned long drm_handle_t;
 
 #endif
 
+#endif
+
 #if defined(__cplusplus)
 extern "C" {
 #endif
@@ -128,7 +133,7 @@ struct drm_tex_region {
  * other data stored in the same cache line.
  */
 struct drm_hw_lock {
-	__volatile__ unsigned int lock;		/**< lock variable */
+	volatile unsigned int lock;		/**< lock variable */
 	char padding[60];			/**< Pad to cache line */
 };
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Simon Ser]

Can you add a Signed-off-by line to your commit message? This means
you agree to the Developer Certificate of Origin [1].

[1]: https://developercertificate.org/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: Fix drm.h uapi header for Windows

[James Park]

Attempting to submit patch in response to https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6162#note_712454

This will allow Mesa to port code to Windows more easily.

Hide BSD header and drm_handle_t behind _WIN32 check.

Change __volatile__ to volatile, which is standard.

Signed-off-by: James Park <jpark37@lagfreegames.com>

James Park (1):
  drm: Fix drm.h uapi header for Windows

 include/uapi/drm/drm.h | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: Fix drm.h uapi header for Windows

[James Park]

This will allow Mesa to port code to Windows more easily.

Hide BSD header and drm_handle_t behind _WIN32 check.

Change __volatile__ to volatile, which is standard.

Signed-off-by: James Park <jpark37@lagfreegames.com>
---
 include/uapi/drm/drm.h | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 808b48a..53dc3c9 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -48,10 +48,9 @@ typedef unsigned int drm_handle_t;
 #include <asm/ioctl.h>
 typedef unsigned int drm_handle_t;
 
-#else /* One of the BSDs */
+#else
 
 #include <stdint.h>
-#include <sys/ioccom.h>
 #include <sys/types.h>
 typedef int8_t   __s8;
 typedef uint8_t  __u8;
@@ -62,10 +61,16 @@ typedef uint32_t __u32;
 typedef int64_t  __s64;
 typedef uint64_t __u64;
 typedef size_t   __kernel_size_t;
+
+#ifndef _WIN32 /* One of the BSDs */
+
+#include <sys/ioccom.h>
 typedef unsigned long drm_handle_t;
 
 #endif
 
+#endif
+
 #if defined(__cplusplus)
 extern "C" {
 #endif
@@ -128,7 +133,7 @@ struct drm_tex_region {
  * other data stored in the same cache line.
  */
 struct drm_hw_lock {
-	__volatile__ unsigned int lock;		/**< lock variable */
+	volatile unsigned int lock;		/**< lock variable */
 	char padding[60];			/**< Pad to cache line */
 };
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Michel Dänzer]

On 2020-12-01 11:01 a.m., James Park wrote:
> This will allow Mesa to port code to Windows more easily.

As discussed in 
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6162#note_712779 
, including drm.h makes no sense when building for Windows.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Daniel Vetter]

On Wed, Dec 2, 2020 at 12:43 PM Michel Dänzer <michel@daenzer.net> wrote:
>
> On 2020-12-01 11:01 a.m., James Park wrote:
> > This will allow Mesa to port code to Windows more easily.
>
> As discussed in
> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6162#note_712779
> , including drm.h makes no sense when building for Windows.

Yeah I think it'd be cleanest if we can avoid this. If not I think the
right fix would be to split out the actually needed parts from drm.h
into a new header (still included by drm.h for backwards compat
reasons) which mesa can use. Since it looks like the problematic parts
are the legacy gunk, and not the new ioctl structures. Pulling out
drm_render.h for all the render stuff and mabe drm_vblank.h for the
vblank stuff (which would fit better in drm_mode.h but mistakes were
made, oops).
-Daniel

>
> --
> Earthling Michel Dänzer               |               https://redhat.com
> Libre software enthusiast             |             Mesa and X developer
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Michel Dänzer]

On 2020-12-02 1:46 p.m., Daniel Vetter wrote:
> On Wed, Dec 2, 2020 at 12:43 PM Michel Dänzer <michel@daenzer.net> wrote:
>>
>> On 2020-12-01 11:01 a.m., James Park wrote:
>>> This will allow Mesa to port code to Windows more easily.
>>
>> As discussed in
>> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6162#note_712779
>> , including drm.h makes no sense when building for Windows.
> 
> Yeah I think it'd be cleanest if we can avoid this. If not I think the
> right fix would be to split out the actually needed parts from drm.h
> into a new header (still included by drm.h for backwards compat
> reasons) which mesa can use. Since it looks like the problematic parts
> are the legacy gunk, and not the new ioctl structures. Pulling out
> drm_render.h for all the render stuff and mabe drm_vblank.h for the
> vblank stuff (which would fit better in drm_mode.h but mistakes were
> made, oops).

If anything currently in drm.h is needed while building for Windows, it 
points to a broken abstraction somewhere in userspace. (Specifically, 
the Mesa Gallium/Vulkan winsys is supposed to abstract away platform 
details like these)


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 2120 bytes --]

I can avoid modifying drm.h by doing this to drm_fourcc.h:

#ifdef _WIN32
#include <stdint.h>
typedef uint64_t __u64;
#else
#include "drm.h"
#endif

And this to amdgpu_drm.h:

#ifdef _WIN32
#include <stdint.h>
typedef int32_t  __s32;
typedef uint32_t __u32;
typedef uint64_t __u64;
#else
#include "drm.h"
#endif

But now I'm touching two files under drm-uapi instead of one, and weirdly.

If we're trying to cut ties with the drm-uapi folder entirely, the stuff
ac_surface.c need includes the AMD_FMT_MOD stuff in drm_fourcc.h,
and AMDGPU_TILING_* under amdgpu_drm.h. Is there a better spot for these
definitions?

Thanks,
James

On Wed, Dec 2, 2020 at 10:06 AM Michel Dänzer <michel@daenzer.net> wrote:

> On 2020-12-02 1:46 p.m., Daniel Vetter wrote:
> > On Wed, Dec 2, 2020 at 12:43 PM Michel Dänzer <michel@daenzer.net>
> wrote:
> >>
> >> On 2020-12-01 11:01 a.m., James Park wrote:
> >>> This will allow Mesa to port code to Windows more easily.
> >>
> >> As discussed in
> >>
> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6162#note_712779
> >> , including drm.h makes no sense when building for Windows.
> >
> > Yeah I think it'd be cleanest if we can avoid this. If not I think the
> > right fix would be to split out the actually needed parts from drm.h
> > into a new header (still included by drm.h for backwards compat
> > reasons) which mesa can use. Since it looks like the problematic parts
> > are the legacy gunk, and not the new ioctl structures. Pulling out
> > drm_render.h for all the render stuff and mabe drm_vblank.h for the
> > vblank stuff (which would fit better in drm_mode.h but mistakes were
> > made, oops).
>
> If anything currently in drm.h is needed while building for Windows, it
> points to a broken abstraction somewhere in userspace. (Specifically,
> the Mesa Gallium/Vulkan winsys is supposed to abstract away platform
> details like these)
>
>
> --
> Earthling Michel Dänzer               |               https://redhat.com
> Libre software enthusiast             |             Mesa and X developer
>

[-- Attachment #1.2: Type: text/html, Size: 3036 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Daniel Vetter]

On Wed, Dec 2, 2020 at 8:48 PM James Park <james.park@lagfreegames.com> wrote:
>
> I can avoid modifying drm.h by doing this to drm_fourcc.h:
>
> #ifdef _WIN32
> #include <stdint.h>
> typedef uint64_t __u64;
> #else
> #include "drm.h"
> #endif
>
> And this to amdgpu_drm.h:
>
> #ifdef _WIN32
> #include <stdint.h>
> typedef int32_t  __s32;
> typedef uint32_t __u32;
> typedef uint64_t __u64;
> #else
> #include "drm.h"
> #endif
>
> But now I'm touching two files under drm-uapi instead of one, and weirdly.
>
> If we're trying to cut ties with the drm-uapi folder entirely, the stuff ac_surface.c need includes the AMD_FMT_MOD stuff in drm_fourcc.h, and AMDGPU_TILING_* under amdgpu_drm.h. Is there a better spot for these definitions?

The drm_fourcc.h maybe makes some sense (I think in some places mesa
uses these internally, and many drivers use the modifiers directly in
the main driver). But the amdgpu header should be all ioctl stuff,
which should be all entirely in the winsys only.

Also kinda disappointing that drm_fourcc.h includes drm.h and isn't
standalone, but I guess that sailed (at least for linux).
-Daniel

> Thanks,
> James
>
> On Wed, Dec 2, 2020 at 10:06 AM Michel Dänzer <michel@daenzer.net> wrote:
>>
>> On 2020-12-02 1:46 p.m., Daniel Vetter wrote:
>> > On Wed, Dec 2, 2020 at 12:43 PM Michel Dänzer <michel@daenzer.net> wrote:
>> >>
>> >> On 2020-12-01 11:01 a.m., James Park wrote:
>> >>> This will allow Mesa to port code to Windows more easily.
>> >>
>> >> As discussed in
>> >> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6162#note_712779
>> >> , including drm.h makes no sense when building for Windows.
>> >
>> > Yeah I think it'd be cleanest if we can avoid this. If not I think the
>> > right fix would be to split out the actually needed parts from drm.h
>> > into a new header (still included by drm.h for backwards compat
>> > reasons) which mesa can use. Since it looks like the problematic parts
>> > are the legacy gunk, and not the new ioctl structures. Pulling out
>> > drm_render.h for all the render stuff and mabe drm_vblank.h for the
>> > vblank stuff (which would fit better in drm_mode.h but mistakes were
>> > made, oops).
>>
>> If anything currently in drm.h is needed while building for Windows, it
>> points to a broken abstraction somewhere in userspace. (Specifically,
>> the Mesa Gallium/Vulkan winsys is supposed to abstract away platform
>> details like these)
>>
>>
>> --
>> Earthling Michel Dänzer               |               https://redhat.com
>> Libre software enthusiast             |             Mesa and X developer



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 3451 bytes --]

If the definitions in drm_fourcc.h make sense to live there, and we can't
remove drm.h from that header for backward compatibility, and the code that
I'm trying to compile on Windows needs the definitions in drm_fourcc.h,
then doesn't it make sense to adjust drm.h?

The patch that I'm proposing doesn't change very much. It might be easier
to read here:
https://github.com/jpark37/linux/commit/648e9281824ddc943c3ea6b34d6d6c154717a0a3

Thanks,
James

On Wed, Dec 2, 2020 at 2:26 PM Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Dec 2, 2020 at 8:48 PM James Park <james.park@lagfreegames.com>
> wrote:
> >
> > I can avoid modifying drm.h by doing this to drm_fourcc.h:
> >
> > #ifdef _WIN32
> > #include <stdint.h>
> > typedef uint64_t __u64;
> > #else
> > #include "drm.h"
> > #endif
> >
> > And this to amdgpu_drm.h:
> >
> > #ifdef _WIN32
> > #include <stdint.h>
> > typedef int32_t  __s32;
> > typedef uint32_t __u32;
> > typedef uint64_t __u64;
> > #else
> > #include "drm.h"
> > #endif
> >
> > But now I'm touching two files under drm-uapi instead of one, and
> weirdly.
> >
> > If we're trying to cut ties with the drm-uapi folder entirely, the stuff
> ac_surface.c need includes the AMD_FMT_MOD stuff in drm_fourcc.h, and
> AMDGPU_TILING_* under amdgpu_drm.h. Is there a better spot for these
> definitions?
>
> The drm_fourcc.h maybe makes some sense (I think in some places mesa
> uses these internally, and many drivers use the modifiers directly in
> the main driver). But the amdgpu header should be all ioctl stuff,
> which should be all entirely in the winsys only.
>
> Also kinda disappointing that drm_fourcc.h includes drm.h and isn't
> standalone, but I guess that sailed (at least for linux).
> -Daniel
>
> > Thanks,
> > James
> >
> > On Wed, Dec 2, 2020 at 10:06 AM Michel Dänzer <michel@daenzer.net>
> wrote:
> >>
> >> On 2020-12-02 1:46 p.m., Daniel Vetter wrote:
> >> > On Wed, Dec 2, 2020 at 12:43 PM Michel Dänzer <michel@daenzer.net>
> wrote:
> >> >>
> >> >> On 2020-12-01 11:01 a.m., James Park wrote:
> >> >>> This will allow Mesa to port code to Windows more easily.
> >> >>
> >> >> As discussed in
> >> >>
> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6162#note_712779
> >> >> , including drm.h makes no sense when building for Windows.
> >> >
> >> > Yeah I think it'd be cleanest if we can avoid this. If not I think the
> >> > right fix would be to split out the actually needed parts from drm.h
> >> > into a new header (still included by drm.h for backwards compat
> >> > reasons) which mesa can use. Since it looks like the problematic parts
> >> > are the legacy gunk, and not the new ioctl structures. Pulling out
> >> > drm_render.h for all the render stuff and mabe drm_vblank.h for the
> >> > vblank stuff (which would fit better in drm_mode.h but mistakes were
> >> > made, oops).
> >>
> >> If anything currently in drm.h is needed while building for Windows, it
> >> points to a broken abstraction somewhere in userspace. (Specifically,
> >> the Mesa Gallium/Vulkan winsys is supposed to abstract away platform
> >> details like these)
> >>
> >>
> >> --
> >> Earthling Michel Dänzer               |
> https://redhat.com
> >> Libre software enthusiast             |             Mesa and X developer
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>

[-- Attachment #1.2: Type: text/html, Size: 4899 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 1084 bytes --]

On Wed, 2 Dec 2020 23:25:58 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> Also kinda disappointing that drm_fourcc.h includes drm.h and isn't
> standalone, but I guess that sailed (at least for linux).

Hi,

FWIW, libweston core needs drm_fourcc.h too, even if nothing would ever
use DRM or need libdrm otherwise. A stand-alone drm_fourcc.h
replacement would make sense, although distributing it through libdrm
would still make libweston require libdrm headers at build time, even
if it doesn't need libdrm.so. Not a big deal, and I don't know if
anyone actually builds libweston without DRM-backend.

Inventing yet another pixel format enumeration just because you don't
want to depend on a specific piece of other software really sucks, so
libweston went with DRM formats as the canonical enumeration. And
Wayland protocols use it too - Wayland clients rarely have any use for
libdrm otherwise.

Maybe a new header drm_formats.h that is what drm_fourcc.h should have
been, and make drm_fourcc.h include that to be backwards API compatible?


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Ville Syrjälä]

On Wed, Dec 02, 2020 at 11:25:58PM +0100, Daniel Vetter wrote:
> On Wed, Dec 2, 2020 at 8:48 PM James Park <james.park@lagfreegames.com> wrote:
> >
> > I can avoid modifying drm.h by doing this to drm_fourcc.h:
> >
> > #ifdef _WIN32
> > #include <stdint.h>
> > typedef uint64_t __u64;
> > #else
> > #include "drm.h"
> > #endif
> >
> > And this to amdgpu_drm.h:
> >
> > #ifdef _WIN32
> > #include <stdint.h>
> > typedef int32_t  __s32;
> > typedef uint32_t __u32;
> > typedef uint64_t __u64;
> > #else
> > #include "drm.h"
> > #endif
> >
> > But now I'm touching two files under drm-uapi instead of one, and weirdly.
> >
> > If we're trying to cut ties with the drm-uapi folder entirely, the stuff ac_surface.c need includes the AMD_FMT_MOD stuff in drm_fourcc.h, and AMDGPU_TILING_* under amdgpu_drm.h. Is there a better spot for these definitions?
> 
> The drm_fourcc.h maybe makes some sense (I think in some places mesa
> uses these internally, and many drivers use the modifiers directly in
> the main driver). But the amdgpu header should be all ioctl stuff,
> which should be all entirely in the winsys only.
> 
> Also kinda disappointing that drm_fourcc.h includes drm.h and isn't
> standalone, but I guess that sailed (at least for linux).

Isn't the only thing it needs the __u32? I would think we could just
replace those with unsigned int (DRM_FORMAT_BIG_ENDIAN already assumes
int is 32bit it seems) and drop the drm.h.

Or are we're worried something already depends on getting drm.h via
just including drm_fourcc.h?

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Simon Ser]

On Thursday, December 3, 2020 1:54 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> > The drm_fourcc.h maybe makes some sense (I think in some places mesa
> > uses these internally, and many drivers use the modifiers directly in
> > the main driver). But the amdgpu header should be all ioctl stuff,
> > which should be all entirely in the winsys only.
> > Also kinda disappointing that drm_fourcc.h includes drm.h and isn't
> > standalone, but I guess that sailed (at least for linux).
>
> Isn't the only thing it needs the __u32? I would think we could just
> replace those with unsigned int (DRM_FORMAT_BIG_ENDIAN already assumes
> int is 32bit it seems) and drop the drm.h.
>
> Or are we're worried something already depends on getting drm.h via
> just including drm_fourcc.h?

Yes, some user-space might only include drm_fourcc.h and use stuff
coming from drm.h.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Michel Dänzer]

On 2020-12-02 8:47 p.m., James Park wrote:
> 
> If we're trying to cut ties with the drm-uapi folder entirely, the stuff 
> ac_surface.c need includes the AMD_FMT_MOD stuff in drm_fourcc.h, 
> and AMDGPU_TILING_* under amdgpu_drm.h. Is there a better spot for these 
> definitions?

The Mesa src/amd/ code should use platform-neutral abstractions for 
these. This wasn't deemed necessary before, because nobody was trying to 
build these drivers for non-UNIX OSes. But now you are.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Daniel Vetter]

On Thu, Dec 3, 2020 at 9:18 AM Michel Dänzer <michel@daenzer.net> wrote:
>
> On 2020-12-02 8:47 p.m., James Park wrote:
> >
> > If we're trying to cut ties with the drm-uapi folder entirely, the stuff
> > ac_surface.c need includes the AMD_FMT_MOD stuff in drm_fourcc.h,
> > and AMDGPU_TILING_* under amdgpu_drm.h. Is there a better spot for these
> > definitions?
>
> The Mesa src/amd/ code should use platform-neutral abstractions for
> these. This wasn't deemed necessary before, because nobody was trying to
> build these drivers for non-UNIX OSes. But now you are.

I think that's a bit much busy work for not much gain. drm_fourcc.h is
even included as the official source of truth of some khr extensions,
making that header stand-alone and useable cross-platform sounds like
a good idea to me. Something like the below is imo perfectly fine:

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index ca48ed0e6bc1..0a121b3efb58 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -24,7 +24,11 @@
#ifndef DRM_FOURCC_H
#define DRM_FOURCC_H

+#ifndef DRM_FOURCC_STANDALONE_
+/* include the linux uapi types here */
+#else
#include "drm.h"
+#endif

#if defined(__cplusplus)
extern "C" {


Cheers, Daniel

>
>
> --
> Earthling Michel Dänzer               |               https://redhat.com
> Libre software enthusiast             |             Mesa and X developer
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 2084 bytes --]

The trailing underscore for  DRM_FOURCC_STANDALONE_ isn't intentional,
right? Should I put all the integer types, or just the ones that are used
in that file?

Thanks,
James

On Thu, Dec 3, 2020 at 6:52 AM Daniel Vetter <daniel@ffwll.ch> wrote:

> On Thu, Dec 3, 2020 at 9:18 AM Michel Dänzer <michel@daenzer.net> wrote:
> >
> > On 2020-12-02 8:47 p.m., James Park wrote:
> > >
> > > If we're trying to cut ties with the drm-uapi folder entirely, the
> stuff
> > > ac_surface.c need includes the AMD_FMT_MOD stuff in drm_fourcc.h,
> > > and AMDGPU_TILING_* under amdgpu_drm.h. Is there a better spot for
> these
> > > definitions?
> >
> > The Mesa src/amd/ code should use platform-neutral abstractions for
> > these. This wasn't deemed necessary before, because nobody was trying to
> > build these drivers for non-UNIX OSes. But now you are.
>
> I think that's a bit much busy work for not much gain. drm_fourcc.h is
> even included as the official source of truth of some khr extensions,
> making that header stand-alone and useable cross-platform sounds like
> a good idea to me. Something like the below is imo perfectly fine:
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index ca48ed0e6bc1..0a121b3efb58 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -24,7 +24,11 @@
> #ifndef DRM_FOURCC_H
> #define DRM_FOURCC_H
>
> +#ifndef DRM_FOURCC_STANDALONE_
> +/* include the linux uapi types here */
> +#else
> #include "drm.h"
> +#endif
>
> #if defined(__cplusplus)
> extern "C" {
>
>
> Cheers, Daniel
>
> >
> >
> > --
> > Earthling Michel Dänzer               |               https://redhat.com
> > Libre software enthusiast             |             Mesa and X developer
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>

[-- Attachment #1.2: Type: text/html, Size: 3012 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Daniel Vetter]

On Thu, Dec 3, 2020 at 7:55 PM James Park <james.park@lagfreegames.com> wrote:
>
> The trailing underscore for  DRM_FOURCC_STANDALONE_ isn't intentional, right? Should I put all the integer types, or just the ones that are used in that file?

Yeah that trailing _ just slipped in. And I'd just do the types
already used. I don't think anything else than __u32 (for drm fourcc)
and __u64 (for drm modifier) is needed.
-Daniel

>
> Thanks,
> James
>
> On Thu, Dec 3, 2020 at 6:52 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>>
>> On Thu, Dec 3, 2020 at 9:18 AM Michel Dänzer <michel@daenzer.net> wrote:
>> >
>> > On 2020-12-02 8:47 p.m., James Park wrote:
>> > >
>> > > If we're trying to cut ties with the drm-uapi folder entirely, the stuff
>> > > ac_surface.c need includes the AMD_FMT_MOD stuff in drm_fourcc.h,
>> > > and AMDGPU_TILING_* under amdgpu_drm.h. Is there a better spot for these
>> > > definitions?
>> >
>> > The Mesa src/amd/ code should use platform-neutral abstractions for
>> > these. This wasn't deemed necessary before, because nobody was trying to
>> > build these drivers for non-UNIX OSes. But now you are.
>>
>> I think that's a bit much busy work for not much gain. drm_fourcc.h is
>> even included as the official source of truth of some khr extensions,
>> making that header stand-alone and useable cross-platform sounds like
>> a good idea to me. Something like the below is imo perfectly fine:
>>
>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>> index ca48ed0e6bc1..0a121b3efb58 100644
>> --- a/include/uapi/drm/drm_fourcc.h
>> +++ b/include/uapi/drm/drm_fourcc.h
>> @@ -24,7 +24,11 @@
>> #ifndef DRM_FOURCC_H
>> #define DRM_FOURCC_H
>>
>> +#ifndef DRM_FOURCC_STANDALONE_
>> +/* include the linux uapi types here */
>> +#else
>> #include "drm.h"
>> +#endif
>>
>> #if defined(__cplusplus)
>> extern "C" {
>>
>>
>> Cheers, Daniel
>>
>> >
>> >
>> > --
>> > Earthling Michel Dänzer               |               https://redhat.com
>> > Libre software enthusiast             |             Mesa and X developer
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: Allow drm_fourcc.h without including drm.h

[James Park]

Add DRM_FOURCC_STANDALONE guard to skip drm.h dependency.

This will allow Mesa to port code to Windows more easily.

Signed-off-by: James Park <jpark37@lagfreegames.com>

James Park (1):
  drm: Allow drm_fourcc.h without including drm.h

 include/uapi/drm/drm_fourcc.h | 6 ++++++
 1 file changed, 6 insertions(+)

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: Allow drm_fourcc.h without including drm.h

[James Park]

Add DRM_FOURCC_STANDALONE guard to skip drm.h dependency.

This will allow Mesa to port code to Windows more easily.

Signed-off-by: James Park <jpark37@lagfreegames.com>
---
 include/uapi/drm/drm_fourcc.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 82f3278..159a9d0 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -24,7 +24,13 @@
 #ifndef DRM_FOURCC_H
 #define DRM_FOURCC_H
 
+#ifdef DRM_FOURCC_STANDALONE
+#include <stdint.h>
+typedef uint32_t __u32;
+typedef uint64_t __u64;
+#else
 #include "drm.h"
+#endif
 
 #if defined(__cplusplus)
 extern "C" {
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Allow drm_fourcc.h without including drm.h

[Simon Ser]

On Friday, December 4, 2020 5:53 AM, James Park <jpark37@lagfreegames.com> wrote:

> +#ifdef DRM_FOURCC_STANDALONE
> +#include <stdint.h>
>
> +typedef uint32_t __u32;
> +typedef uint64_t __u64;
> +#else
> #include "drm.h"
> +#endif

C11 allows duplicate typedefs, but older versions of the standard
don't AFAIK. If this is a concern, a solution would be to guard the
typedefs.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Allow drm_fourcc.h without including drm.h

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 907 bytes --]

The typedefs might also conflict on Linux if DRM_FOURCC_STANDALONE is
enabled with whatever LInux declared __u32/__u64 as, but I think the
implication is that once DRM_FOURCC_STANDALONE has been declared, that's
kind of a promise not to include drm.h.

I'm fine with this, but I'm not married to it if someone has a problem
where they want to define DRM_FOURCC_STANDALONE, but also can't avoid
including drm.h for some reason.

On Fri, Dec 4, 2020 at 12:53 AM Simon Ser <contact@emersion.fr> wrote:

> On Friday, December 4, 2020 5:53 AM, James Park <jpark37@lagfreegames.com>
> wrote:
>
> > +#ifdef DRM_FOURCC_STANDALONE
> > +#include <stdint.h>
> >
> > +typedef uint32_t __u32;
> > +typedef uint64_t __u64;
> > +#else
> > #include "drm.h"
> > +#endif
>
> C11 allows duplicate typedefs, but older versions of the standard
> don't AFAIK. If this is a concern, a solution would be to guard the
> typedefs.
>

[-- Attachment #1.2: Type: text/html, Size: 1383 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Allow drm_fourcc.h without including drm.h

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 1343 bytes --]

I suppose I should do this to avoid fighting with <linux/types.h>

#ifdef DRM_FOURCC_STANDALONE
#if defined(__linux__)
#include <linux/types.h>
#else
#include <stdint.h>
typedef uint32_t __u32;
typedef uint64_t __u64;
#endif
#else
#include "drm.h"
#endif

I'll wait for more feedback before updating the patch though.

On Fri, Dec 4, 2020 at 1:47 AM James Park <james.park@lagfreegames.com>
wrote:

> The typedefs might also conflict on Linux if DRM_FOURCC_STANDALONE is
> enabled with whatever LInux declared __u32/__u64 as, but I think the
> implication is that once DRM_FOURCC_STANDALONE has been declared, that's
> kind of a promise not to include drm.h.
>
> I'm fine with this, but I'm not married to it if someone has a problem
> where they want to define DRM_FOURCC_STANDALONE, but also can't avoid
> including drm.h for some reason.
>
> On Fri, Dec 4, 2020 at 12:53 AM Simon Ser <contact@emersion.fr> wrote:
>
>> On Friday, December 4, 2020 5:53 AM, James Park <jpark37@lagfreegames.com>
>> wrote:
>>
>> > +#ifdef DRM_FOURCC_STANDALONE
>> > +#include <stdint.h>
>> >
>> > +typedef uint32_t __u32;
>> > +typedef uint64_t __u64;
>> > +#else
>> > #include "drm.h"
>> > +#endif
>>
>> C11 allows duplicate typedefs, but older versions of the standard
>> don't AFAIK. If this is a concern, a solution would be to guard the
>> typedefs.
>>
>

[-- Attachment #1.2: Type: text/html, Size: 2224 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Allow drm_fourcc.h without including drm.h

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1568 bytes --]

Hi James,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master drm/drm-next v5.10-rc6 next-20201204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/James-Park/drm-Allow-drm_fourcc-h-without-including-drm-h/20201204-163753
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-c002-20201204 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/444ac999e27a36307f741eb0ef60d630b0b8946a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review James-Park/drm-Allow-drm_fourcc-h-without-including-drm-h/20201204-163753
        git checkout 444ac999e27a36307f741eb0ef60d630b0b8946a
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> usr/include/drm/drm_fourcc.h:29: found __[us]{8,16,32,64} type without #include <linux/types.h>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32201 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Allow drm_fourcc.h without including drm.h

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1602 bytes --]

Hi James,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master drm/drm-next v5.10-rc6 next-20201204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/James-Park/drm-Allow-drm_fourcc-h-without-including-drm-h/20201204-163753
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-c002-20201204 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/444ac999e27a36307f741eb0ef60d630b0b8946a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review James-Park/drm-Allow-drm_fourcc-h-without-including-drm-h/20201204-163753
        git checkout 444ac999e27a36307f741eb0ef60d630b0b8946a
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> usr/include/drm/drm_fourcc.h:29: found __[us]{8,16,32,64} type without #include <linux/types.h>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32201 bytes --]

Re: [PATCH] drm: Allow drm_fourcc.h without including drm.h

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1930 bytes --]

Hi James,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.10-rc6 next-20201204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/James-Park/drm-Allow-drm_fourcc-h-without-including-drm-h/20201204-163753
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-a014-20201204 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 32c501dd88b62787d3a5ffda7aabcf4650dbe3cd)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/444ac999e27a36307f741eb0ef60d630b0b8946a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review James-Park/drm-Allow-drm_fourcc-h-without-including-drm-h/20201204-163753
        git checkout 444ac999e27a36307f741eb0ef60d630b0b8946a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> usr/include/drm/drm_fourcc.h:29: found __[us]{8,16,32,64} type without #include <linux/types.h>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45004 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Allow drm_fourcc.h without including drm.h

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1968 bytes --]

Hi James,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.10-rc6 next-20201204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/James-Park/drm-Allow-drm_fourcc-h-without-including-drm-h/20201204-163753
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-a014-20201204 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 32c501dd88b62787d3a5ffda7aabcf4650dbe3cd)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/444ac999e27a36307f741eb0ef60d630b0b8946a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review James-Park/drm-Allow-drm_fourcc-h-without-including-drm-h/20201204-163753
        git checkout 444ac999e27a36307f741eb0ef60d630b0b8946a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> usr/include/drm/drm_fourcc.h:29: found __[us]{8,16,32,64} type without #include <linux/types.h>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 45004 bytes --]

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 978 bytes --]

On Thu, 3 Dec 2020 21:45:14 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Thu, Dec 3, 2020 at 7:55 PM James Park <james.park@lagfreegames.com> wrote:
> >
> > The trailing underscore for  DRM_FOURCC_STANDALONE_ isn't
> > intentional, right? Should I put all the integer types, or just the
> > ones that are used in that file?  
> 
> Yeah that trailing _ just slipped in. And I'd just do the types
> already used. I don't think anything else than __u32 (for drm fourcc)
> and __u64 (for drm modifier) is needed.

Hi,

can that create conflicts if userspace first includes drm_fourcc.h and
then drm.h?

I would find it natural to userspace have generic headers including
drm_fourcc.h and then DRM-specific C-files including drm.h as well
(through libdrm headers). I think Weston might already do this.

The generic userspace (weston) header would obviously #define
DRM_FOURCC_STANDALONE, because it is used by non-DRM C-files as well.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Daniel Vetter]

On Fri, Dec 4, 2020 at 9:12 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>
> On Thu, 3 Dec 2020 21:45:14 +0100
> Daniel Vetter <daniel@ffwll.ch> wrote:
>
> > On Thu, Dec 3, 2020 at 7:55 PM James Park <james.park@lagfreegames.com> wrote:
> > >
> > > The trailing underscore for  DRM_FOURCC_STANDALONE_ isn't
> > > intentional, right? Should I put all the integer types, or just the
> > > ones that are used in that file?
> >
> > Yeah that trailing _ just slipped in. And I'd just do the types
> > already used. I don't think anything else than __u32 (for drm fourcc)
> > and __u64 (for drm modifier) is needed.
>
> Hi,
>
> can that create conflicts if userspace first includes drm_fourcc.h and
> then drm.h?
>
> I would find it natural to userspace have generic headers including
> drm_fourcc.h and then DRM-specific C-files including drm.h as well
> (through libdrm headers). I think Weston might already do this.
>
> The generic userspace (weston) header would obviously #define
> DRM_FOURCC_STANDALONE, because it is used by non-DRM C-files as well.

Hm yes that would break. I guess we could just include the linux types
header for this. And I guess on windows you'd need to have that from
somewhere. Or we just require that users of the standalone header pull
the right header or defines in first?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 1996 bytes --]

I could adjust the block to look like this:

#ifdef DRM_FOURCC_STANDALONE
#if defined(__linux__)
#include <linux/types.h>
#else
#include <stdint.h>
typedef uint32_t __u32;
typedef uint64_t __u64;
#endif
#else
#include "drm.h"
#endif

Alternatively, I could create a new common header to be included from both
drm.h and drm_fourcc.h, drm_base_types.h or something like that:

#ifdef DRM_FOURCC_STANDALONE
#include "drm_base_types.h"
#else
#include "drm.h"
#endif

On Fri, Dec 4, 2020 at 7:58 AM Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, Dec 4, 2020 at 9:12 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> >
> > On Thu, 3 Dec 2020 21:45:14 +0100
> > Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > > On Thu, Dec 3, 2020 at 7:55 PM James Park <james.park@lagfreegames.com>
> wrote:
> > > >
> > > > The trailing underscore for  DRM_FOURCC_STANDALONE_ isn't
> > > > intentional, right? Should I put all the integer types, or just the
> > > > ones that are used in that file?
> > >
> > > Yeah that trailing _ just slipped in. And I'd just do the types
> > > already used. I don't think anything else than __u32 (for drm fourcc)
> > > and __u64 (for drm modifier) is needed.
> >
> > Hi,
> >
> > can that create conflicts if userspace first includes drm_fourcc.h and
> > then drm.h?
> >
> > I would find it natural to userspace have generic headers including
> > drm_fourcc.h and then DRM-specific C-files including drm.h as well
> > (through libdrm headers). I think Weston might already do this.
> >
> > The generic userspace (weston) header would obviously #define
> > DRM_FOURCC_STANDALONE, because it is used by non-DRM C-files as well.
>
> Hm yes that would break. I guess we could just include the linux types
> header for this. And I guess on windows you'd need to have that from
> somewhere. Or we just require that users of the standalone header pull
> the right header or defines in first?
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>

[-- Attachment #1.2: Type: text/html, Size: 2929 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

Create drm_basic_types.h to define types previously defined by drm.h.

Use DRM_FOURCC_STANDALONE to include drm_fourcc.h, replacing drm.h
dependency with drm_basic_types.h.

This will allow Mesa to port code to Windows more easily.

Signed-off-by: James Park <jpark37@lagfreegames.com>

James Park (1):
  drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

 include/uapi/drm/drm.h             | 14 ++--------
 include/uapi/drm/drm_basic_types.h | 52 ++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/drm_fourcc.h      |  4 +++
 3 files changed, 58 insertions(+), 12 deletions(-)
 create mode 100644 include/uapi/drm/drm_basic_types.h

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

Create drm_basic_types.h to define types previously defined by drm.h.

Use DRM_FOURCC_STANDALONE to include drm_fourcc.h, replacing drm.h
dependency with drm_basic_types.h.

This will allow Mesa to port code to Windows more easily.

Signed-off-by: James Park <jpark37@lagfreegames.com>
---
 include/uapi/drm/drm.h             | 14 ++--------
 include/uapi/drm/drm_basic_types.h | 52 ++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/drm_fourcc.h      |  4 +++
 3 files changed, 58 insertions(+), 12 deletions(-)
 create mode 100644 include/uapi/drm/drm_basic_types.h

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 808b48a..a7f38fc 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -36,32 +36,22 @@
 #ifndef _DRM_H_
 #define _DRM_H_
 
+#include "drm_basic_types.h"
+
 #if defined(__KERNEL__)
 
-#include <linux/types.h>
 #include <asm/ioctl.h>
 typedef unsigned int drm_handle_t;
 
 #elif defined(__linux__)
 
-#include <linux/types.h>
 #include <asm/ioctl.h>
 typedef unsigned int drm_handle_t;
 
 #else /* One of the BSDs */
 
-#include <stdint.h>
 #include <sys/ioccom.h>
 #include <sys/types.h>
-typedef int8_t   __s8;
-typedef uint8_t  __u8;
-typedef int16_t  __s16;
-typedef uint16_t __u16;
-typedef int32_t  __s32;
-typedef uint32_t __u32;
-typedef int64_t  __s64;
-typedef uint64_t __u64;
-typedef size_t   __kernel_size_t;
 typedef unsigned long drm_handle_t;
 
 #endif
diff --git a/include/uapi/drm/drm_basic_types.h b/include/uapi/drm/drm_basic_types.h
new file mode 100644
index 0000000..b3c00bb
--- /dev/null
+++ b/include/uapi/drm/drm_basic_types.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
+ * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
+ * All rights reserved.
+ *
+ * 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
+ * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS 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.
+ */
+
+#ifndef _DRM_BASIC_TYPES_H_
+#define _DRM_BASIC_TYPES_H_
+
+#if defined(__KERNEL__)
+
+#include <linux/types.h>
+
+#elif defined(__linux__)
+
+#include <linux/types.h>
+
+#else /* One of the BSDs */
+
+#include <stdint.h>
+typedef int8_t   __s8;
+typedef uint8_t  __u8;
+typedef int16_t  __s16;
+typedef uint16_t __u16;
+typedef int32_t  __s32;
+typedef uint32_t __u32;
+typedef int64_t  __s64;
+typedef uint64_t __u64;
+typedef size_t   __kernel_size_t;
+
+#endif
+
+#endif
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 82f3278..5eb07a5 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -24,7 +24,11 @@
 #ifndef DRM_FOURCC_H
 #define DRM_FOURCC_H
 
+#ifdef DRM_FOURCC_STANDALONE
+#include "drm_basic_types.h"
+#else
 #include "drm.h"
+#endif
 
 #if defined(__cplusplus)
 extern "C" {
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[Simon Ser]

On Sunday, December 6th, 2020 at 1:39 AM, James Park <jpark37@lagfreegames.com> wrote:

> Create drm_basic_types.h to define types previously defined by drm.h.
>
> Use DRM_FOURCC_STANDALONE to include drm_fourcc.h, replacing drm.h
> dependency with drm_basic_types.h.

This approach looks better to me than the other alternatives.

> This will allow Mesa to port code to Windows more easily.
>
> Signed-off-by: James Park <jpark37@lagfreegames.com>
> ---
>  include/uapi/drm/drm.h             | 14 ++--------
>  include/uapi/drm/drm_basic_types.h | 52 ++++++++++++++++++++++++++++++++++++++
>  include/uapi/drm/drm_fourcc.h      |  4 +++
>  3 files changed, 58 insertions(+), 12 deletions(-)
>  create mode 100644 include/uapi/drm/drm_basic_types.h
>
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 808b48a..a7f38fc 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -36,32 +36,22 @@
>  #ifndef _DRM_H_
>  #define _DRM_H_
>
> +#include "drm_basic_types.h"
> +
>  #if defined(__KERNEL__)
>
> -#include <linux/types.h>
>  #include <asm/ioctl.h>
>  typedef unsigned int drm_handle_t;
>
>  #elif defined(__linux__)
>
> -#include <linux/types.h>
>  #include <asm/ioctl.h>
>  typedef unsigned int drm_handle_t;
>
>  #else /* One of the BSDs */
>
> -#include <stdint.h>
>  #include <sys/ioccom.h>
>  #include <sys/types.h>
> -typedef int8_t   __s8;
> -typedef uint8_t  __u8;
> -typedef int16_t  __s16;
> -typedef uint16_t __u16;
> -typedef int32_t  __s32;
> -typedef uint32_t __u32;
> -typedef int64_t  __s64;
> -typedef uint64_t __u64;
> -typedef size_t   __kernel_size_t;
>  typedef unsigned long drm_handle_t;
>
>  #endif
> diff --git a/include/uapi/drm/drm_basic_types.h b/include/uapi/drm/drm_basic_types.h
> new file mode 100644
> index 0000000..b3c00bb
> --- /dev/null
> +++ b/include/uapi/drm/drm_basic_types.h
> @@ -0,0 +1,52 @@
> +/*
> + * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
> + * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
> + * All rights reserved.
> + *
> + * 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
> + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS 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.
> + */
> +
> +#ifndef _DRM_BASIC_TYPES_H_
> +#define _DRM_BASIC_TYPES_H_
> +
> +#if defined(__KERNEL__)
> +
> +#include <linux/types.h>
> +
> +#elif defined(__linux__)

Nit: these two #ifs can be combined together.

> +#include <linux/types.h>
> +
> +#else /* One of the BSDs */

Maybe replace with /* Not Linux */?

> +#include <stdint.h>
> +typedef int8_t   __s8;
> +typedef uint8_t  __u8;
> +typedef int16_t  __s16;
> +typedef uint16_t __u16;
> +typedef int32_t  __s32;
> +typedef uint32_t __u32;
> +typedef int64_t  __s64;
> +typedef uint64_t __u64;
> +typedef size_t   __kernel_size_t;
> +
> +#endif
> +
> +#endif
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 82f3278..5eb07a5 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -24,7 +24,11 @@
>  #ifndef DRM_FOURCC_H
>  #define DRM_FOURCC_H
>
> +#ifdef DRM_FOURCC_STANDALONE
> +#include "drm_basic_types.h"
> +#else
>  #include "drm.h"
> +#endif
>
>  #if defined(__cplusplus)
>  extern "C" {
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 5179 bytes --]

I'd noticed the #if could be combined, but they weren't in drm,h when they
could have been, so I didn't want to depart from the existing pattern.

I think "One of the BSDs" is more informative than "Not Linux" if that
statement is still true. Given the aversion to making drm.h robust to
Windows, I don't think we want to imply compatibility that isn't there.

Thanks,
James

On Mon, Dec 7, 2020 at 1:45 AM Simon Ser <contact@emersion.fr> wrote:

> On Sunday, December 6th, 2020 at 1:39 AM, James Park <
> jpark37@lagfreegames.com> wrote:
>
> > Create drm_basic_types.h to define types previously defined by drm.h.
> >
> > Use DRM_FOURCC_STANDALONE to include drm_fourcc.h, replacing drm.h
> > dependency with drm_basic_types.h.
>
> This approach looks better to me than the other alternatives.
>
> > This will allow Mesa to port code to Windows more easily.
> >
> > Signed-off-by: James Park <jpark37@lagfreegames.com>
> > ---
> >  include/uapi/drm/drm.h             | 14 ++--------
> >  include/uapi/drm/drm_basic_types.h | 52
> ++++++++++++++++++++++++++++++++++++++
> >  include/uapi/drm/drm_fourcc.h      |  4 +++
> >  3 files changed, 58 insertions(+), 12 deletions(-)
> >  create mode 100644 include/uapi/drm/drm_basic_types.h
> >
> > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > index 808b48a..a7f38fc 100644
> > --- a/include/uapi/drm/drm.h
> > +++ b/include/uapi/drm/drm.h
> > @@ -36,32 +36,22 @@
> >  #ifndef _DRM_H_
> >  #define _DRM_H_
> >
> > +#include "drm_basic_types.h"
> > +
> >  #if defined(__KERNEL__)
> >
> > -#include <linux/types.h>
> >  #include <asm/ioctl.h>
> >  typedef unsigned int drm_handle_t;
> >
> >  #elif defined(__linux__)
> >
> > -#include <linux/types.h>
> >  #include <asm/ioctl.h>
> >  typedef unsigned int drm_handle_t;
> >
> >  #else /* One of the BSDs */
> >
> > -#include <stdint.h>
> >  #include <sys/ioccom.h>
> >  #include <sys/types.h>
> > -typedef int8_t   __s8;
> > -typedef uint8_t  __u8;
> > -typedef int16_t  __s16;
> > -typedef uint16_t __u16;
> > -typedef int32_t  __s32;
> > -typedef uint32_t __u32;
> > -typedef int64_t  __s64;
> > -typedef uint64_t __u64;
> > -typedef size_t   __kernel_size_t;
> >  typedef unsigned long drm_handle_t;
> >
> >  #endif
> > diff --git a/include/uapi/drm/drm_basic_types.h
> b/include/uapi/drm/drm_basic_types.h
> > new file mode 100644
> > index 0000000..b3c00bb
> > --- /dev/null
> > +++ b/include/uapi/drm/drm_basic_types.h
> > @@ -0,0 +1,52 @@
> > +/*
> > + * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
> > + * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
> > + * All rights reserved.
> > + *
> > + * 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
> > + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS 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.
> > + */
> > +
> > +#ifndef _DRM_BASIC_TYPES_H_
> > +#define _DRM_BASIC_TYPES_H_
> > +
> > +#if defined(__KERNEL__)
> > +
> > +#include <linux/types.h>
> > +
> > +#elif defined(__linux__)
>
> Nit: these two #ifs can be combined together.
>
> > +#include <linux/types.h>
> > +
> > +#else /* One of the BSDs */
>
> Maybe replace with /* Not Linux */?
>
> > +#include <stdint.h>
> > +typedef int8_t   __s8;
> > +typedef uint8_t  __u8;
> > +typedef int16_t  __s16;
> > +typedef uint16_t __u16;
> > +typedef int32_t  __s32;
> > +typedef uint32_t __u32;
> > +typedef int64_t  __s64;
> > +typedef uint64_t __u64;
> > +typedef size_t   __kernel_size_t;
> > +
> > +#endif
> > +
> > +#endif
> > diff --git a/include/uapi/drm/drm_fourcc.h
> b/include/uapi/drm/drm_fourcc.h
> > index 82f3278..5eb07a5 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -24,7 +24,11 @@
> >  #ifndef DRM_FOURCC_H
> >  #define DRM_FOURCC_H
> >
> > +#ifdef DRM_FOURCC_STANDALONE
> > +#include "drm_basic_types.h"
> > +#else
> >  #include "drm.h"
> > +#endif
> >
> >  #if defined(__cplusplus)
> >  extern "C" {
> > --
> > 2.7.4
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[-- Attachment #1.2: Type: text/html, Size: 6837 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[Simon Ser]

On Monday, December 7th, 2020 at 10:55 AM, James Park <james.park@lagfreegames.com> wrote:

> I'd noticed the #if could be combined, but they weren't in drm,h when
> they could have been, so I didn't want to depart from the existing
> pattern.

I see. The original code really needed the two branches and
drm_basic_types.h doesn't. But let's see what others think.

> I think "One of the BSDs" is more informative than "Not Linux" if
> that statement is still true. Given the aversion to making drm.h
> robust to Windows, I don't think we want to imply compatibility that
> isn't there.

Well, drm_basic_types.h would be included on Windows as well from
drm_fourcc.h, right?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 1052 bytes --]

The original code blocks in drm.h look identical to me. I see:

#include <linux/types.h>
#include <asm/ioctl.h>
typedef unsigned int drm_handle_t;

Good point about drm_basic_types.h. I'll change it to say "Not Linux" after
waiting a bit for more feedback.

Thanks,
James

On Mon, Dec 7, 2020 at 1:59 AM Simon Ser <contact@emersion.fr> wrote:

> On Monday, December 7th, 2020 at 10:55 AM, James Park <
> james.park@lagfreegames.com> wrote:
>
> > I'd noticed the #if could be combined, but they weren't in drm,h when
> > they could have been, so I didn't want to depart from the existing
> > pattern.
>
> I see. The original code really needed the two branches and
> drm_basic_types.h doesn't. But let's see what others think.
>
> > I think "One of the BSDs" is more informative than "Not Linux" if
> > that statement is still true. Given the aversion to making drm.h
> > robust to Windows, I don't think we want to imply compatibility that
> > isn't there.
>
> Well, drm_basic_types.h would be included on Windows as well from
> drm_fourcc.h, right?
>

[-- Attachment #1.2: Type: text/html, Size: 1636 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[Simon Ser]

On Monday, December 7th, 2020 at 11:05 AM, James Park <james.park@lagfreegames.com> wrote:

> The original code blocks in drm.h look identical to me. I see:
>
> #include <linux/types.h>
> #include <asm/ioctl.h>
> typedef unsigned int drm_handle_t;

Hmm. Actually you're completely right, it turns out it's necessary to
duplicate the branches like this to make `make headers_install` work
properly. See 00c9672606f7 ("drm: Untangle __KERNEL__ guards").
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1551 bytes --]

Hi James,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master drm/drm-next v5.10-rc7 next-20201207]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/James-Park/drm-drm_basic_types-h-DRM_FOURCC_STANDALONE/20201207-165922
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/c614dbde0c1dc422490fb22281d3e6dcc6355f66
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review James-Park/drm-drm_basic_types-h-DRM_FOURCC_STANDALONE/20201207-165922
        git checkout c614dbde0c1dc422490fb22281d3e6dcc6355f66
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> usr/include/linux/kfd_ioctl.h:37: found __[us]{8,16,32,64} type without #include <linux/types.h>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 77020 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

Hi James,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master drm/drm-next v5.10-rc7 next-20201207]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/James-Park/drm-drm_basic_types-h-DRM_FOURCC_STANDALONE/20201207-165922
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/c614dbde0c1dc422490fb22281d3e6dcc6355f66
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review James-Park/drm-drm_basic_types-h-DRM_FOURCC_STANDALONE/20201207-165922
        git checkout c614dbde0c1dc422490fb22281d3e6dcc6355f66
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> usr/include/linux/kfd_ioctl.h:37: found __[us]{8,16,32,64} type without #include <linux/types.h>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 77020 bytes --]

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

Create drm_basic_types.h to define types previously defined by drm.h.

Use DRM_FOURCC_STANDALONE to include drm_fourcc.h without drm.h.

This will allow Mesa to port code to Windows more easily.

Signed-off-by: James Park <jpark37@lagfreegames.com>

James Park (1):
  drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

 include/uapi/drm/drm.h             | 12 ++-------
 include/uapi/drm/drm_basic_types.h | 52 ++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/drm_fourcc.h      |  4 +++
 3 files changed, 58 insertions(+), 10 deletions(-)
 create mode 100644 include/uapi/drm/drm_basic_types.h

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

Create drm_basic_types.h to define types previously defined by drm.h.

Use DRM_FOURCC_STANDALONE to include drm_fourcc.h without drm.h.

This will allow Mesa to port code to Windows more easily.

Signed-off-by: James Park <jpark37@lagfreegames.com>
---
 include/uapi/drm/drm.h             | 12 ++-------
 include/uapi/drm/drm_basic_types.h | 52 ++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/drm_fourcc.h      |  4 +++
 3 files changed, 58 insertions(+), 10 deletions(-)
 create mode 100644 include/uapi/drm/drm_basic_types.h

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 808b48a..d9ba922 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -36,6 +36,8 @@
 #ifndef _DRM_H_
 #define _DRM_H_
 
+#include "drm_basic_types.h"
+
 #if defined(__KERNEL__)
 
 #include <linux/types.h>
@@ -50,18 +52,8 @@ typedef unsigned int drm_handle_t;
 
 #else /* One of the BSDs */
 
-#include <stdint.h>
 #include <sys/ioccom.h>
 #include <sys/types.h>
-typedef int8_t   __s8;
-typedef uint8_t  __u8;
-typedef int16_t  __s16;
-typedef uint16_t __u16;
-typedef int32_t  __s32;
-typedef uint32_t __u32;
-typedef int64_t  __s64;
-typedef uint64_t __u64;
-typedef size_t   __kernel_size_t;
 typedef unsigned long drm_handle_t;
 
 #endif
diff --git a/include/uapi/drm/drm_basic_types.h b/include/uapi/drm/drm_basic_types.h
new file mode 100644
index 0000000..da1f2c0
--- /dev/null
+++ b/include/uapi/drm/drm_basic_types.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
+ * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
+ * All rights reserved.
+ *
+ * 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
+ * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS 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.
+ */
+
+#ifndef _DRM_BASIC_TYPES_H_
+#define _DRM_BASIC_TYPES_H_
+
+#if defined(__KERNEL__)
+
+#include <linux/types.h>
+
+#elif defined(__linux__)
+
+#include <linux/types.h>
+
+#else /* Not Linux */
+
+#include <stdint.h>
+typedef int8_t   __s8;
+typedef uint8_t  __u8;
+typedef int16_t  __s16;
+typedef uint16_t __u16;
+typedef int32_t  __s32;
+typedef uint32_t __u32;
+typedef int64_t  __s64;
+typedef uint64_t __u64;
+typedef size_t   __kernel_size_t;
+
+#endif
+
+#endif
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 82f3278..539870f 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -24,7 +24,11 @@
 #ifndef DRM_FOURCC_H
 #define DRM_FOURCC_H
 
+#include "drm_basic_types.h"
+
+#ifndef DRM_FOURCC_STANDALONE
 #include "drm.h"
+#endif
 
 #if defined(__cplusplus)
 extern "C" {
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[Simon Ser]

May I ask what exactly fails when you drop #include <linux/types.h>
from drm.h?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 304 bytes --]

This was the message from kernel test robot.

>> usr/include/linux/kfd_ioctl.h:37: found __[us]{8,16,32,64} type without
#include <linux/types.h>

On Tue, Dec 8, 2020 at 4:31 AM Simon Ser <contact@emersion.fr> wrote:

> May I ask what exactly fails when you drop #include <linux/types.h>
> from drm.h?
>

[-- Attachment #1.2: Type: text/html, Size: 633 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[Simon Ser]

On Tuesday, December 8th, 2020 at 7:32 PM, James Park <james.park@lagfreegames.com> wrote:

> This was the message from kernel test robot.
>
> >> usr/include/linux/kfd_ioctl.h:37: found __[us]{8,16,32,64} type without #include <linux/types.h>

Interesting that the warning comes from linux/kfd_ioctl.h. I guess
keeping the linux/types.h isn't that of a big deal anyways.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[Simon Ser]

On Monday, December 7th, 2020 at 7:15 PM, James Park <jpark37@lagfreegames.com> wrote:

> Create drm_basic_types.h to define types previously defined by drm.h.
> Use DRM_FOURCC_STANDALONE to include drm_fourcc.h without drm.h.
> This will allow Mesa to port code to Windows more easily.
>
> Signed-off-by: James Park <jpark37@lagfreegames.com>

This version sounds fine to me, thanks.

Acked-by: Simon Ser <contact@emersion.fr>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

Create drm_basic_types.h to define types previously defined by drm.h.

Use DRM_FOURCC_STANDALONE to include drm_fourcc.h without drm.h.

This will allow Mesa to port code to Windows more easily.

Signed-off-by: James Park <jpark37@lagfreegames.com>
Acked-by: Simon Ser <contact@emersion.fr>

James Park (1):
  drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

 include/uapi/drm/drm.h             | 12 ++-------
 include/uapi/drm/drm_basic_types.h | 52 ++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/drm_fourcc.h      |  4 +++
 3 files changed, 58 insertions(+), 10 deletions(-)
 create mode 100644 include/uapi/drm/drm_basic_types.h

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

Create drm_basic_types.h to define types previously defined by drm.h.

Use DRM_FOURCC_STANDALONE to include drm_fourcc.h without drm.h.

This will allow Mesa to port code to Windows more easily.

Signed-off-by: James Park <jpark37@lagfreegames.com>
Acked-by: Simon Ser <contact@emersion.fr>
---
 include/uapi/drm/drm.h             | 12 ++-------
 include/uapi/drm/drm_basic_types.h | 52 ++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/drm_fourcc.h      |  4 +++
 3 files changed, 58 insertions(+), 10 deletions(-)
 create mode 100644 include/uapi/drm/drm_basic_types.h

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 808b48a..d9ba922 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -36,6 +36,8 @@
 #ifndef _DRM_H_
 #define _DRM_H_
 
+#include "drm_basic_types.h"
+
 #if defined(__KERNEL__)
 
 #include <linux/types.h>
@@ -50,18 +52,8 @@ typedef unsigned int drm_handle_t;
 
 #else /* One of the BSDs */
 
-#include <stdint.h>
 #include <sys/ioccom.h>
 #include <sys/types.h>
-typedef int8_t   __s8;
-typedef uint8_t  __u8;
-typedef int16_t  __s16;
-typedef uint16_t __u16;
-typedef int32_t  __s32;
-typedef uint32_t __u32;
-typedef int64_t  __s64;
-typedef uint64_t __u64;
-typedef size_t   __kernel_size_t;
 typedef unsigned long drm_handle_t;
 
 #endif
diff --git a/include/uapi/drm/drm_basic_types.h b/include/uapi/drm/drm_basic_types.h
new file mode 100644
index 0000000..da1f2c0
--- /dev/null
+++ b/include/uapi/drm/drm_basic_types.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
+ * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
+ * All rights reserved.
+ *
+ * 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
+ * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS 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.
+ */
+
+#ifndef _DRM_BASIC_TYPES_H_
+#define _DRM_BASIC_TYPES_H_
+
+#if defined(__KERNEL__)
+
+#include <linux/types.h>
+
+#elif defined(__linux__)
+
+#include <linux/types.h>
+
+#else /* Not Linux */
+
+#include <stdint.h>
+typedef int8_t   __s8;
+typedef uint8_t  __u8;
+typedef int16_t  __s16;
+typedef uint16_t __u16;
+typedef int32_t  __s32;
+typedef uint32_t __u32;
+typedef int64_t  __s64;
+typedef uint64_t __u64;
+typedef size_t   __kernel_size_t;
+
+#endif
+
+#endif
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 82f3278..539870f 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -24,7 +24,11 @@
 #ifndef DRM_FOURCC_H
 #define DRM_FOURCC_H
 
+#include "drm_basic_types.h"
+
+#ifndef DRM_FOURCC_STANDALONE
 #include "drm.h"
+#endif
 
 #if defined(__cplusplus)
 extern "C" {
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 4253 bytes --]

On Wed,  9 Dec 2020 03:03:10 -0800
James Park <jpark37@lagfreegames.com> wrote:

> Create drm_basic_types.h to define types previously defined by drm.h.
> 
> Use DRM_FOURCC_STANDALONE to include drm_fourcc.h without drm.h.
> 
> This will allow Mesa to port code to Windows more easily.
> 
> Signed-off-by: James Park <jpark37@lagfreegames.com>
> Acked-by: Simon Ser <contact@emersion.fr>

Looks good to me.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>

But with the caveat that I didn't actually test this.


Thanks,
pq

> ---
>  include/uapi/drm/drm.h             | 12 ++-------
>  include/uapi/drm/drm_basic_types.h | 52 ++++++++++++++++++++++++++++++++++++++
>  include/uapi/drm/drm_fourcc.h      |  4 +++
>  3 files changed, 58 insertions(+), 10 deletions(-)
>  create mode 100644 include/uapi/drm/drm_basic_types.h
> 
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 808b48a..d9ba922 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -36,6 +36,8 @@
>  #ifndef _DRM_H_
>  #define _DRM_H_
>  
> +#include "drm_basic_types.h"
> +
>  #if defined(__KERNEL__)
>  
>  #include <linux/types.h>
> @@ -50,18 +52,8 @@ typedef unsigned int drm_handle_t;
>  
>  #else /* One of the BSDs */
>  
> -#include <stdint.h>
>  #include <sys/ioccom.h>
>  #include <sys/types.h>
> -typedef int8_t   __s8;
> -typedef uint8_t  __u8;
> -typedef int16_t  __s16;
> -typedef uint16_t __u16;
> -typedef int32_t  __s32;
> -typedef uint32_t __u32;
> -typedef int64_t  __s64;
> -typedef uint64_t __u64;
> -typedef size_t   __kernel_size_t;
>  typedef unsigned long drm_handle_t;
>  
>  #endif
> diff --git a/include/uapi/drm/drm_basic_types.h b/include/uapi/drm/drm_basic_types.h
> new file mode 100644
> index 0000000..da1f2c0
> --- /dev/null
> +++ b/include/uapi/drm/drm_basic_types.h
> @@ -0,0 +1,52 @@
> +/*
> + * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
> + * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
> + * All rights reserved.
> + *
> + * 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
> + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS 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.
> + */
> +
> +#ifndef _DRM_BASIC_TYPES_H_
> +#define _DRM_BASIC_TYPES_H_
> +
> +#if defined(__KERNEL__)
> +
> +#include <linux/types.h>
> +
> +#elif defined(__linux__)
> +
> +#include <linux/types.h>
> +
> +#else /* Not Linux */
> +
> +#include <stdint.h>
> +typedef int8_t   __s8;
> +typedef uint8_t  __u8;
> +typedef int16_t  __s16;
> +typedef uint16_t __u16;
> +typedef int32_t  __s32;
> +typedef uint32_t __u32;
> +typedef int64_t  __s64;
> +typedef uint64_t __u64;
> +typedef size_t   __kernel_size_t;
> +
> +#endif
> +
> +#endif
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 82f3278..539870f 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -24,7 +24,11 @@
>  #ifndef DRM_FOURCC_H
>  #define DRM_FOURCC_H
>  
> +#include "drm_basic_types.h"
> +
> +#ifndef DRM_FOURCC_STANDALONE
>  #include "drm.h"
> +#endif
>  
>  #if defined(__cplusplus)
>  extern "C" {


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

Create drm_basic_types.h to define types previously defined by drm.h.

Use DRM_FOURCC_STANDALONE to include drm_fourcc.h without drm.h.

This will allow Mesa to port code to Windows more easily.

Signed-off-by: James Park <jpark37@lagfreegames.com>
Acked-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>

James Park (1):
  drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

 include/uapi/drm/drm.h             | 12 ++-------
 include/uapi/drm/drm_basic_types.h | 52 ++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/drm_fourcc.h      |  4 +++
 3 files changed, 58 insertions(+), 10 deletions(-)
 create mode 100644 include/uapi/drm/drm_basic_types.h

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

Create drm_basic_types.h to define types previously defined by drm.h.

Use DRM_FOURCC_STANDALONE to include drm_fourcc.h without drm.h.

This will allow Mesa to port code to Windows more easily.

Signed-off-by: James Park <jpark37@lagfreegames.com>
Acked-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>
---
 include/uapi/drm/drm.h             | 12 ++-------
 include/uapi/drm/drm_basic_types.h | 52 ++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/drm_fourcc.h      |  4 +++
 3 files changed, 58 insertions(+), 10 deletions(-)
 create mode 100644 include/uapi/drm/drm_basic_types.h

diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 808b48a..d9ba922 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -36,6 +36,8 @@
 #ifndef _DRM_H_
 #define _DRM_H_
 
+#include "drm_basic_types.h"
+
 #if defined(__KERNEL__)
 
 #include <linux/types.h>
@@ -50,18 +52,8 @@ typedef unsigned int drm_handle_t;
 
 #else /* One of the BSDs */
 
-#include <stdint.h>
 #include <sys/ioccom.h>
 #include <sys/types.h>
-typedef int8_t   __s8;
-typedef uint8_t  __u8;
-typedef int16_t  __s16;
-typedef uint16_t __u16;
-typedef int32_t  __s32;
-typedef uint32_t __u32;
-typedef int64_t  __s64;
-typedef uint64_t __u64;
-typedef size_t   __kernel_size_t;
 typedef unsigned long drm_handle_t;
 
 #endif
diff --git a/include/uapi/drm/drm_basic_types.h b/include/uapi/drm/drm_basic_types.h
new file mode 100644
index 0000000..da1f2c0
--- /dev/null
+++ b/include/uapi/drm/drm_basic_types.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
+ * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
+ * All rights reserved.
+ *
+ * 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
+ * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS 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.
+ */
+
+#ifndef _DRM_BASIC_TYPES_H_
+#define _DRM_BASIC_TYPES_H_
+
+#if defined(__KERNEL__)
+
+#include <linux/types.h>
+
+#elif defined(__linux__)
+
+#include <linux/types.h>
+
+#else /* Not Linux */
+
+#include <stdint.h>
+typedef int8_t   __s8;
+typedef uint8_t  __u8;
+typedef int16_t  __s16;
+typedef uint16_t __u16;
+typedef int32_t  __s32;
+typedef uint32_t __u32;
+typedef int64_t  __s64;
+typedef uint64_t __u64;
+typedef size_t   __kernel_size_t;
+
+#endif
+
+#endif
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 82f3278..539870f 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -24,7 +24,11 @@
 #ifndef DRM_FOURCC_H
 #define DRM_FOURCC_H
 
+#include "drm_basic_types.h"
+
+#ifndef DRM_FOURCC_STANDALONE
 #include "drm.h"
+#endif
 
 #if defined(__cplusplus)
 extern "C" {
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 4137 bytes --]

Hello,

Is there something I can do to help move this patch along?

Thanks,
James Park

On Thu, Dec 10, 2020 at 1:13 AM James Park <jpark37@lagfreegames.com> wrote:

> Create drm_basic_types.h to define types previously defined by drm.h.
>
> Use DRM_FOURCC_STANDALONE to include drm_fourcc.h without drm.h.
>
> This will allow Mesa to port code to Windows more easily.
>
> Signed-off-by: James Park <jpark37@lagfreegames.com>
> Acked-by: Simon Ser <contact@emersion.fr>
> Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> ---
>  include/uapi/drm/drm.h             | 12 ++-------
>  include/uapi/drm/drm_basic_types.h | 52
> ++++++++++++++++++++++++++++++++++++++
>  include/uapi/drm/drm_fourcc.h      |  4 +++
>  3 files changed, 58 insertions(+), 10 deletions(-)
>  create mode 100644 include/uapi/drm/drm_basic_types.h
>
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 808b48a..d9ba922 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -36,6 +36,8 @@
>  #ifndef _DRM_H_
>  #define _DRM_H_
>
> +#include "drm_basic_types.h"
> +
>  #if defined(__KERNEL__)
>
>  #include <linux/types.h>
> @@ -50,18 +52,8 @@ typedef unsigned int drm_handle_t;
>
>  #else /* One of the BSDs */
>
> -#include <stdint.h>
>  #include <sys/ioccom.h>
>  #include <sys/types.h>
> -typedef int8_t   __s8;
> -typedef uint8_t  __u8;
> -typedef int16_t  __s16;
> -typedef uint16_t __u16;
> -typedef int32_t  __s32;
> -typedef uint32_t __u32;
> -typedef int64_t  __s64;
> -typedef uint64_t __u64;
> -typedef size_t   __kernel_size_t;
>  typedef unsigned long drm_handle_t;
>
>  #endif
> diff --git a/include/uapi/drm/drm_basic_types.h
> b/include/uapi/drm/drm_basic_types.h
> new file mode 100644
> index 0000000..da1f2c0
> --- /dev/null
> +++ b/include/uapi/drm/drm_basic_types.h
> @@ -0,0 +1,52 @@
> +/*
> + * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
> + * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
> + * All rights reserved.
> + *
> + * 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
> + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS 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.
> + */
> +
> +#ifndef _DRM_BASIC_TYPES_H_
> +#define _DRM_BASIC_TYPES_H_
> +
> +#if defined(__KERNEL__)
> +
> +#include <linux/types.h>
> +
> +#elif defined(__linux__)
> +
> +#include <linux/types.h>
> +
> +#else /* Not Linux */
> +
> +#include <stdint.h>
> +typedef int8_t   __s8;
> +typedef uint8_t  __u8;
> +typedef int16_t  __s16;
> +typedef uint16_t __u16;
> +typedef int32_t  __s32;
> +typedef uint32_t __u32;
> +typedef int64_t  __s64;
> +typedef uint64_t __u64;
> +typedef size_t   __kernel_size_t;
> +
> +#endif
> +
> +#endif
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 82f3278..539870f 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -24,7 +24,11 @@
>  #ifndef DRM_FOURCC_H
>  #define DRM_FOURCC_H
>
> +#include "drm_basic_types.h"
> +
> +#ifndef DRM_FOURCC_STANDALONE
>  #include "drm.h"
> +#endif
>
>  #if defined(__cplusplus)
>  extern "C" {
> --
> 2.7.4
>
>

[-- Attachment #1.2: Type: text/html, Size: 5216 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[Emil Velikov]

Hi James,

On Tue, 2 Feb 2021 at 08:27, James Park <james.park@lagfreegames.com> wrote:
>
> Hello,
>
> Is there something I can do to help move this patch along?
>
> Thanks,
> James Park
>
> On Thu, Dec 10, 2020 at 1:13 AM James Park <jpark37@lagfreegames.com> wrote:
>>
>> Create drm_basic_types.h to define types previously defined by drm.h.
>>
>> Use DRM_FOURCC_STANDALONE to include drm_fourcc.h without drm.h.
>>
>> This will allow Mesa to port code to Windows more easily.
>>
>> Signed-off-by: James Park <jpark37@lagfreegames.com>
>> Acked-by: Simon Ser <contact@emersion.fr>
>> Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>
>> ---
>>  include/uapi/drm/drm.h             | 12 ++-------
>>  include/uapi/drm/drm_basic_types.h | 52 ++++++++++++++++++++++++++++++++++++++
>>  include/uapi/drm/drm_fourcc.h      |  4 +++
>>  3 files changed, 58 insertions(+), 10 deletions(-)
>>  create mode 100644 include/uapi/drm/drm_basic_types.h
>>
Have you considered the possibility of having the ifdef block inlined
within drm_fourcc.h?

Sure some users might need to add an drm.h include in their code. At
the same time they also need to explicitly define
DRM_FOURCC_STANDALONE, so that is fine.
We had all sorts of issues with these headers in the past, so adding
another one might end up repeating some of those yet again.

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 1847 bytes --]

I'm not sure what your suggestion is. Move which #ifdef block where?

I don't think everyone is of the opinion that adding drm.h and pulling in
unnecessary kernel structures is fine. If I'm not mistaken, the reason
people are making me jump through hoops in the first place is to avoid that.

I appreciate the feedback though.

- James

On Tue, Feb 2, 2021 at 9:28 AM Emil Velikov <emil.l.velikov@gmail.com>
wrote:

> Hi James,
>
> On Tue, 2 Feb 2021 at 08:27, James Park <james.park@lagfreegames.com>
> wrote:
> >
> > Hello,
> >
> > Is there something I can do to help move this patch along?
> >
> > Thanks,
> > James Park
> >
> > On Thu, Dec 10, 2020 at 1:13 AM James Park <jpark37@lagfreegames.com>
> wrote:
> >>
> >> Create drm_basic_types.h to define types previously defined by drm.h.
> >>
> >> Use DRM_FOURCC_STANDALONE to include drm_fourcc.h without drm.h.
> >>
> >> This will allow Mesa to port code to Windows more easily.
> >>
> >> Signed-off-by: James Park <jpark37@lagfreegames.com>
> >> Acked-by: Simon Ser <contact@emersion.fr>
> >> Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> >> ---
> >>  include/uapi/drm/drm.h             | 12 ++-------
> >>  include/uapi/drm/drm_basic_types.h | 52
> ++++++++++++++++++++++++++++++++++++++
> >>  include/uapi/drm/drm_fourcc.h      |  4 +++
> >>  3 files changed, 58 insertions(+), 10 deletions(-)
> >>  create mode 100644 include/uapi/drm/drm_basic_types.h
> >>
> Have you considered the possibility of having the ifdef block inlined
> within drm_fourcc.h?
>
> Sure some users might need to add an drm.h include in their code. At
> the same time they also need to explicitly define
> DRM_FOURCC_STANDALONE, so that is fine.
> We had all sorts of issues with these headers in the past, so adding
> another one might end up repeating some of those yet again.
>
> Thanks
> Emil
>

[-- Attachment #1.2: Type: text/html, Size: 2817 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[Emil Velikov]

Hi james,

On Tue, 2 Feb 2021 at 18:15, James Park <james.park@lagfreegames.com> wrote:
>
> I'm not sure what your suggestion is. Move which #ifdef block where?
>
Fair enough. Just sent out a patch which demonstrates what I have in mind.

Thanks
Emil

P.S. Please try to avoid top-posting and HTML emails in public discussions.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[Simon Ser]

On Tuesday, February 2nd, 2021 at 11:48 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:

> P.S. Please try to avoid top-posting and HTML emails in public discussions.

(James: as a side note, here is a guide to configure your e-mail client
to use plain-text: https://useplaintext.email/)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[Simon Ser]

On Thursday, December 10th, 2020 at 10:12 AM, James Park <jpark37@lagfreegames.com> wrote:

> +#ifndef _DRM_BASIC_TYPES_H_
> +#define _DRM_BASIC_TYPES_H_

Nit: the rest of the kernel doesn't use an underscore prefix for these
defines. Note that anything starting with an underscore followed by an
upper-case letter is a reserved identifier in C.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: drm_basic_types.h, DRM_FOURCC_STANDALONE

[Simon Ser]

On Tuesday, February 2nd, 2021 at 9:51 AM, Simon Ser <contact@emersion.fr> wrote:

> On Thursday, December 10th, 2020 at 10:12 AM, James Park jpark37@lagfreegames.com wrote:
>
> > +#ifndef _DRM_BASIC_TYPES_H
> > +#define _DRM_BASIC_TYPES_H
>
> Nit: the rest of the kernel doesn't use an underscore prefix for these
> defines. Note that anything starting with an underscore followed by an
> upper-case letter is a reserved identifier in C.

Hm, please disregard, it seems some files still use an underscore.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 2639 bytes --]

On Fri, 4 Dec 2020 11:07:41 -0800
James Park <james.park@lagfreegames.com> wrote:

> I could adjust the block to look like this:
> 
> #ifdef DRM_FOURCC_STANDALONE
> #if defined(__linux__)
> #include <linux/types.h>
> #else
> #include <stdint.h>
> typedef uint32_t __u32;
> typedef uint64_t __u64;
> #endif
> #else
> #include "drm.h"
> #endif
> 
> Alternatively, I could create a new common header to be included from both
> drm.h and drm_fourcc.h, drm_base_types.h or something like that:
> 
> #ifdef DRM_FOURCC_STANDALONE
> #include "drm_base_types.h"
> #else
> #include "drm.h"
> #endif

Hi,

my point is, any solution relying on DRM_FOURCC_STANDALONE will fail
sometimes, because there is no reason why userspace would *not* #define
DRM_FOURCC_STANDALONE. Hence, #ifdef DRM_FOURCC_STANDALONE is
completely moot, you have to make the headers work in any include
order when DRM_FOURCC_STANDALONE is defined anyway.


Thanks.
pq

> On Fri, Dec 4, 2020 at 7:58 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Fri, Dec 4, 2020 at 9:12 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:  
> > >
> > > On Thu, 3 Dec 2020 21:45:14 +0100
> > > Daniel Vetter <daniel@ffwll.ch> wrote:
> > >  
> > > > On Thu, Dec 3, 2020 at 7:55 PM James Park <james.park@lagfreegames.com>  
> > wrote:  
> > > > >
> > > > > The trailing underscore for  DRM_FOURCC_STANDALONE_ isn't
> > > > > intentional, right? Should I put all the integer types, or just the
> > > > > ones that are used in that file?  
> > > >
> > > > Yeah that trailing _ just slipped in. And I'd just do the types
> > > > already used. I don't think anything else than __u32 (for drm fourcc)
> > > > and __u64 (for drm modifier) is needed.  
> > >
> > > Hi,
> > >
> > > can that create conflicts if userspace first includes drm_fourcc.h and
> > > then drm.h?
> > >
> > > I would find it natural to userspace have generic headers including
> > > drm_fourcc.h and then DRM-specific C-files including drm.h as well
> > > (through libdrm headers). I think Weston might already do this.
> > >
> > > The generic userspace (weston) header would obviously #define
> > > DRM_FOURCC_STANDALONE, because it is used by non-DRM C-files as well.  
> >
> > Hm yes that would break. I guess we could just include the linux types
> > header for this. And I guess on windows you'd need to have that from
> > somewhere. Or we just require that users of the standalone header pull
> > the right header or defines in first?
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> >  


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 3072 bytes --]

I'm not completely sure what you're saying, but doesn't drm_base_types.h
(now drm_basic_types.h) make things robust to header order? The patch is in
another email. You can also see it here:
https://github.com/jpark37/linux/commit/0cc8ae750bfd9eab7e31c7de6aa84f24682f4f18

Thanks,
James

On Mon, Dec 7, 2020 at 12:51 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Fri, 4 Dec 2020 11:07:41 -0800
> James Park <james.park@lagfreegames.com> wrote:
>
> > I could adjust the block to look like this:
> >
> > #ifdef DRM_FOURCC_STANDALONE
> > #if defined(__linux__)
> > #include <linux/types.h>
> > #else
> > #include <stdint.h>
> > typedef uint32_t __u32;
> > typedef uint64_t __u64;
> > #endif
> > #else
> > #include "drm.h"
> > #endif
> >
> > Alternatively, I could create a new common header to be included from
> both
> > drm.h and drm_fourcc.h, drm_base_types.h or something like that:
> >
> > #ifdef DRM_FOURCC_STANDALONE
> > #include "drm_base_types.h"
> > #else
> > #include "drm.h"
> > #endif
>
> Hi,
>
> my point is, any solution relying on DRM_FOURCC_STANDALONE will fail
> sometimes, because there is no reason why userspace would *not* #define
> DRM_FOURCC_STANDALONE. Hence, #ifdef DRM_FOURCC_STANDALONE is
> completely moot, you have to make the headers work in any include
> order when DRM_FOURCC_STANDALONE is defined anyway.
>
>
> Thanks.
> pq
>
> > On Fri, Dec 4, 2020 at 7:58 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > > On Fri, Dec 4, 2020 at 9:12 AM Pekka Paalanen <ppaalanen@gmail.com>
> wrote:
> > > >
> > > > On Thu, 3 Dec 2020 21:45:14 +0100
> > > > Daniel Vetter <daniel@ffwll.ch> wrote:
> > > >
> > > > > On Thu, Dec 3, 2020 at 7:55 PM James Park <
> james.park@lagfreegames.com>
> > > wrote:
> > > > > >
> > > > > > The trailing underscore for  DRM_FOURCC_STANDALONE_ isn't
> > > > > > intentional, right? Should I put all the integer types, or just
> the
> > > > > > ones that are used in that file?
> > > > >
> > > > > Yeah that trailing _ just slipped in. And I'd just do the types
> > > > > already used. I don't think anything else than __u32 (for drm
> fourcc)
> > > > > and __u64 (for drm modifier) is needed.
> > > >
> > > > Hi,
> > > >
> > > > can that create conflicts if userspace first includes drm_fourcc.h
> and
> > > > then drm.h?
> > > >
> > > > I would find it natural to userspace have generic headers including
> > > > drm_fourcc.h and then DRM-specific C-files including drm.h as well
> > > > (through libdrm headers). I think Weston might already do this.
> > > >
> > > > The generic userspace (weston) header would obviously #define
> > > > DRM_FOURCC_STANDALONE, because it is used by non-DRM C-files as
> well.
> > >
> > > Hm yes that would break. I guess we could just include the linux types
> > > header for this. And I guess on windows you'd need to have that from
> > > somewhere. Or we just require that users of the standalone header pull
> > > the right header or defines in first?
> > > -Daniel
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> > >
>
>

[-- Attachment #1.2: Type: text/html, Size: 4628 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 4090 bytes --]

On Mon, 7 Dec 2020 01:08:58 -0800
James Park <james.park@lagfreegames.com> wrote:

> I'm not completely sure what you're saying, but doesn't drm_base_types.h
> (now drm_basic_types.h) make things robust to header order? The patch is in
> another email. You can also see it here:
> https://github.com/jpark37/linux/commit/0cc8ae750bfd9eab7e31c7de6aa84f24682f4f18

If that is robust (I don't know if it is, I haven't checked), then why
do you have #ifdef DRM_FOURCC_STANDALONE in it at all?

Like Simon said:

On Mon, 07 Dec 2020 10:02:36 +0000
Simon Ser <contact@emersion.fr> wrote:

> In my compositors I'd like to globally define DRM_FOURCC_STANDALONE
> (to make sure I don't miss any #define) but I still may include drm.h
> in the same files as well.

If any project #defines it globally, then what good does checking for
it do? Why not assume that everyone will always want what
DRM_FOURCC_STANDALONE would bring?


Thanks,
pq

> 
> Thanks,
> James
> 
> On Mon, Dec 7, 2020 at 12:51 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> 
> > On Fri, 4 Dec 2020 11:07:41 -0800
> > James Park <james.park@lagfreegames.com> wrote:
> >  
> > > I could adjust the block to look like this:
> > >
> > > #ifdef DRM_FOURCC_STANDALONE
> > > #if defined(__linux__)
> > > #include <linux/types.h>
> > > #else
> > > #include <stdint.h>
> > > typedef uint32_t __u32;
> > > typedef uint64_t __u64;
> > > #endif
> > > #else
> > > #include "drm.h"
> > > #endif
> > >
> > > Alternatively, I could create a new common header to be included from  
> > both  
> > > drm.h and drm_fourcc.h, drm_base_types.h or something like that:
> > >
> > > #ifdef DRM_FOURCC_STANDALONE
> > > #include "drm_base_types.h"
> > > #else
> > > #include "drm.h"
> > > #endif  
> >
> > Hi,
> >
> > my point is, any solution relying on DRM_FOURCC_STANDALONE will fail
> > sometimes, because there is no reason why userspace would *not* #define
> > DRM_FOURCC_STANDALONE. Hence, #ifdef DRM_FOURCC_STANDALONE is
> > completely moot, you have to make the headers work in any include
> > order when DRM_FOURCC_STANDALONE is defined anyway.
> >
> >
> > Thanks.
> > pq
> >  
> > > On Fri, Dec 4, 2020 at 7:58 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> > >  
> > > > On Fri, Dec 4, 2020 at 9:12 AM Pekka Paalanen <ppaalanen@gmail.com>  
> > wrote:  
> > > > >
> > > > > On Thu, 3 Dec 2020 21:45:14 +0100
> > > > > Daniel Vetter <daniel@ffwll.ch> wrote:
> > > > >  
> > > > > > On Thu, Dec 3, 2020 at 7:55 PM James Park <  
> > james.park@lagfreegames.com>  
> > > > wrote:  
> > > > > > >
> > > > > > > The trailing underscore for  DRM_FOURCC_STANDALONE_ isn't
> > > > > > > intentional, right? Should I put all the integer types, or just  
> > the  
> > > > > > > ones that are used in that file?  
> > > > > >
> > > > > > Yeah that trailing _ just slipped in. And I'd just do the types
> > > > > > already used. I don't think anything else than __u32 (for drm  
> > fourcc)  
> > > > > > and __u64 (for drm modifier) is needed.  
> > > > >
> > > > > Hi,
> > > > >
> > > > > can that create conflicts if userspace first includes drm_fourcc.h  
> > and  
> > > > > then drm.h?
> > > > >
> > > > > I would find it natural to userspace have generic headers including
> > > > > drm_fourcc.h and then DRM-specific C-files including drm.h as well
> > > > > (through libdrm headers). I think Weston might already do this.
> > > > >
> > > > > The generic userspace (weston) header would obviously #define
> > > > > DRM_FOURCC_STANDALONE, because it is used by non-DRM C-files as  
> > well.  
> > > >
> > > > Hm yes that would break. I guess we could just include the linux types
> > > > header for this. And I guess on windows you'd need to have that from
> > > > somewhere. Or we just require that users of the standalone header pull
> > > > the right header or defines in first?
> > > > -Daniel
> > > > --
> > > > Daniel Vetter
> > > > Software Engineer, Intel Corporation
> > > > http://blog.ffwll.ch
> > > >  
> >
> >  


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 1385 bytes --]

On Mon, 7 Dec 2020 12:35:14 +0200
Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Mon, 7 Dec 2020 01:08:58 -0800
> James Park <james.park@lagfreegames.com> wrote:
> 
> > I'm not completely sure what you're saying, but doesn't drm_base_types.h
> > (now drm_basic_types.h) make things robust to header order? The patch is in
> > another email. You can also see it here:
> > https://github.com/jpark37/linux/commit/0cc8ae750bfd9eab7e31c7de6aa84f24682f4f18  
> 
> If that is robust (I don't know if it is, I haven't checked), then why
> do you have #ifdef DRM_FOURCC_STANDALONE in it at all?
> 
> Like Simon said:
> 
> On Mon, 07 Dec 2020 10:02:36 +0000
> Simon Ser <contact@emersion.fr> wrote:
> 
> > In my compositors I'd like to globally define DRM_FOURCC_STANDALONE
> > (to make sure I don't miss any #define) but I still may include drm.h
> > in the same files as well.  
> 
> If any project #defines it globally, then what good does checking for
> it do? Why not assume that everyone will always want what
> DRM_FOURCC_STANDALONE would bring?

Sorry! Now I got it.

DRM_FOURCC_STANDALONE is a promise that the user is not relying on
drm_foucc.h to pull in drm.h. Nothing else. That's fine.

But then, the code in the header should be literally

#ifndef DRM_FOURCC_STANDALONE
#include "drm.h"
#endif

without an #else branch.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Simon Ser]

On Monday, December 7th, 2020 at 11:44 AM, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> But then, the code in the header should be literally
>
> #ifndef DRM_FOURCC_STANDALONE
> #include "drm.h"
> #endif
>
> without an #else branch.

As long as there is a #include "drm_basic_types.h" right before
(drm_fourcc.h needs __u32 and __u64), I believe this can work too
indeed.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 599 bytes --]

That would work, but that's kind of an annoying requirement. I would prefer
the header to be self-sufficient.

Thanks,
James

On Mon, Dec 7, 2020 at 2:47 AM Simon Ser <contact@emersion.fr> wrote:

> On Monday, December 7th, 2020 at 11:44 AM, Pekka Paalanen <
> ppaalanen@gmail.com> wrote:
>
> > But then, the code in the header should be literally
> >
> > #ifndef DRM_FOURCC_STANDALONE
> > #include "drm.h"
> > #endif
> >
> > without an #else branch.
>
> As long as there is a #include "drm_basic_types.h" right before
> (drm_fourcc.h needs __u32 and __u64), I believe this can work too
> indeed.
>

[-- Attachment #1.2: Type: text/html, Size: 1056 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Simon Ser]

On Monday, December 7th, 2020 at 11:49 AM, James Park <james.park@lagfreegames.com> wrote:

> That would work, but that's kind of an annoying requirement. I would
> prefer the header to be self-sufficient.

I don't want to make it more confusing than before, but here Pekka (I
think) suggests to replace this:

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 82f3278..5eb07a5 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -24,7 +24,11 @@
 #ifndef DRM_FOURCC_H
 #define DRM_FOURCC_H

+#ifdef DRM_FOURCC_STANDALONE
+#include "drm_basic_types.h"
+#else
 #include "drm.h"
+#endif

 #if defined(__cplusplus)
 extern "C" {

With this:

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 82f3278..5eb07a5 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -24,7 +24,11 @@
 #ifndef DRM_FOURCC_H
 #define DRM_FOURCC_H

+#include "drm_basic_types.h"
+
+#ifndef DRM_FOURCC_STANDALONE
 #include "drm.h"
+#endif

 #if defined(__cplusplus)
 extern "C" {

That wouldn't change whether the header is self-sufficient or not,
would it?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 1532 bytes --]

Oh, I thought you meant:

#include "drm_basic_types.h"
#include "drm_fourcc.h"

Yours should work too. I don't have a preference vs. what I already have,
so if no one says anything, I can switch over to yours.

Thanks,
James

On Mon, Dec 7, 2020 at 2:53 AM Simon Ser <contact@emersion.fr> wrote:

> On Monday, December 7th, 2020 at 11:49 AM, James Park <
> james.park@lagfreegames.com> wrote:
>
> > That would work, but that's kind of an annoying requirement. I would
> > prefer the header to be self-sufficient.
>
> I don't want to make it more confusing than before, but here Pekka (I
> think) suggests to replace this:
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 82f3278..5eb07a5 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -24,7 +24,11 @@
>  #ifndef DRM_FOURCC_H
>  #define DRM_FOURCC_H
>
> +#ifdef DRM_FOURCC_STANDALONE
> +#include "drm_basic_types.h"
> +#else
>  #include "drm.h"
> +#endif
>
>  #if defined(__cplusplus)
>  extern "C" {
>
> With this:
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 82f3278..5eb07a5 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -24,7 +24,11 @@
>  #ifndef DRM_FOURCC_H
>  #define DRM_FOURCC_H
>
> +#include "drm_basic_types.h"
> +
> +#ifndef DRM_FOURCC_STANDALONE
>  #include "drm.h"
> +#endif
>
>  #if defined(__cplusplus)
>  extern "C" {
>
> That wouldn't change whether the header is self-sufficient or not,
> would it?
>

[-- Attachment #1.2: Type: text/html, Size: 2230 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 1935 bytes --]

On Mon, 07 Dec 2020 10:53:49 +0000
Simon Ser <contact@emersion.fr> wrote:

> On Monday, December 7th, 2020 at 11:49 AM, James Park <james.park@lagfreegames.com> wrote:
> 
> > That would work, but that's kind of an annoying requirement. I would
> > prefer the header to be self-sufficient.  
> 
> I don't want to make it more confusing than before, but here Pekka (I
> think) suggests to replace this:
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 82f3278..5eb07a5 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -24,7 +24,11 @@
>  #ifndef DRM_FOURCC_H
>  #define DRM_FOURCC_H
> 
> +#ifdef DRM_FOURCC_STANDALONE
> +#include "drm_basic_types.h"
> +#else
>  #include "drm.h"
> +#endif
> 
>  #if defined(__cplusplus)
>  extern "C" {
> 
> With this:
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 82f3278..5eb07a5 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -24,7 +24,11 @@
>  #ifndef DRM_FOURCC_H
>  #define DRM_FOURCC_H
> 
> +#include "drm_basic_types.h"
> +
> +#ifndef DRM_FOURCC_STANDALONE
>  #include "drm.h"
> +#endif
> 
>  #if defined(__cplusplus)
>  extern "C" {
> 
> That wouldn't change whether the header is self-sufficient or not,
> would it?

Exactly this.

This communicates properly that DRM_FOURCC_STANDALONE only affects
whether drm.h gets pulled in or not, and there are no other effects.

This also makes testing better: when you unconditionally include
drm_basic_types.h, you are more likely to catch breakage there.

For functionality, it makes no difference. Whether userspace does

#include "drm.h"
#define DRM_FOURCC_STANDALONE
#include "drm_fourcc.h"

or

#define DRM_FOURCC_STANDALONE
#include "drm_fourcc.h"
#include "drm.h"

the result must always be good.


Thanks,
pq

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 2312 bytes --]

I updated the patch earlier today incorporating the suggestions. I also had
to bring back "#include <linux/types.h>" to drm.h because there's some
sanity check that fails, as if it doesn't scan past the first level of
#includes..

- James

On Mon, Dec 7, 2020 at 3:14 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Mon, 07 Dec 2020 10:53:49 +0000
> Simon Ser <contact@emersion.fr> wrote:
>
> > On Monday, December 7th, 2020 at 11:49 AM, James Park <
> james.park@lagfreegames.com> wrote:
> >
> > > That would work, but that's kind of an annoying requirement. I would
> > > prefer the header to be self-sufficient.
> >
> > I don't want to make it more confusing than before, but here Pekka (I
> > think) suggests to replace this:
> >
> > diff --git a/include/uapi/drm/drm_fourcc.h
> b/include/uapi/drm/drm_fourcc.h
> > index 82f3278..5eb07a5 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -24,7 +24,11 @@
> >  #ifndef DRM_FOURCC_H
> >  #define DRM_FOURCC_H
> >
> > +#ifdef DRM_FOURCC_STANDALONE
> > +#include "drm_basic_types.h"
> > +#else
> >  #include "drm.h"
> > +#endif
> >
> >  #if defined(__cplusplus)
> >  extern "C" {
> >
> > With this:
> >
> > diff --git a/include/uapi/drm/drm_fourcc.h
> b/include/uapi/drm/drm_fourcc.h
> > index 82f3278..5eb07a5 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -24,7 +24,11 @@
> >  #ifndef DRM_FOURCC_H
> >  #define DRM_FOURCC_H
> >
> > +#include "drm_basic_types.h"
> > +
> > +#ifndef DRM_FOURCC_STANDALONE
> >  #include "drm.h"
> > +#endif
> >
> >  #if defined(__cplusplus)
> >  extern "C" {
> >
> > That wouldn't change whether the header is self-sufficient or not,
> > would it?
>
> Exactly this.
>
> This communicates properly that DRM_FOURCC_STANDALONE only affects
> whether drm.h gets pulled in or not, and there are no other effects.
>
> This also makes testing better: when you unconditionally include
> drm_basic_types.h, you are more likely to catch breakage there.
>
> For functionality, it makes no difference. Whether userspace does
>
> #include "drm.h"
> #define DRM_FOURCC_STANDALONE
> #include "drm_fourcc.h"
>
> or
>
> #define DRM_FOURCC_STANDALONE
> #include "drm_fourcc.h"
> #include "drm.h"
>
> the result must always be good.
>
>
> Thanks,
> pq
>

[-- Attachment #1.2: Type: text/html, Size: 3282 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Jason Ekstrand]

I'd really like this for Mesa so we can pull drm_fourcc.h into common
WSI code.  Why has it stalled?

--Jason

On Tue, Dec 8, 2020 at 2:33 AM James Park <james.park@lagfreegames.com> wrote:
>
> I updated the patch earlier today incorporating the suggestions. I also had to bring back "#include <linux/types.h>" to drm.h because there's some sanity check that fails, as if it doesn't scan past the first level of #includes..
>
> - James
>
> On Mon, Dec 7, 2020 at 3:14 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>>
>> On Mon, 07 Dec 2020 10:53:49 +0000
>> Simon Ser <contact@emersion.fr> wrote:
>>
>> > On Monday, December 7th, 2020 at 11:49 AM, James Park <james.park@lagfreegames.com> wrote:
>> >
>> > > That would work, but that's kind of an annoying requirement. I would
>> > > prefer the header to be self-sufficient.
>> >
>> > I don't want to make it more confusing than before, but here Pekka (I
>> > think) suggests to replace this:
>> >
>> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>> > index 82f3278..5eb07a5 100644
>> > --- a/include/uapi/drm/drm_fourcc.h
>> > +++ b/include/uapi/drm/drm_fourcc.h
>> > @@ -24,7 +24,11 @@
>> >  #ifndef DRM_FOURCC_H
>> >  #define DRM_FOURCC_H
>> >
>> > +#ifdef DRM_FOURCC_STANDALONE
>> > +#include "drm_basic_types.h"
>> > +#else
>> >  #include "drm.h"
>> > +#endif
>> >
>> >  #if defined(__cplusplus)
>> >  extern "C" {
>> >
>> > With this:
>> >
>> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>> > index 82f3278..5eb07a5 100644
>> > --- a/include/uapi/drm/drm_fourcc.h
>> > +++ b/include/uapi/drm/drm_fourcc.h
>> > @@ -24,7 +24,11 @@
>> >  #ifndef DRM_FOURCC_H
>> >  #define DRM_FOURCC_H
>> >
>> > +#include "drm_basic_types.h"
>> > +
>> > +#ifndef DRM_FOURCC_STANDALONE
>> >  #include "drm.h"
>> > +#endif
>> >
>> >  #if defined(__cplusplus)
>> >  extern "C" {
>> >
>> > That wouldn't change whether the header is self-sufficient or not,
>> > would it?
>>
>> Exactly this.
>>
>> This communicates properly that DRM_FOURCC_STANDALONE only affects
>> whether drm.h gets pulled in or not, and there are no other effects.
>>
>> This also makes testing better: when you unconditionally include
>> drm_basic_types.h, you are more likely to catch breakage there.
>>
>> For functionality, it makes no difference. Whether userspace does
>>
>> #include "drm.h"
>> #define DRM_FOURCC_STANDALONE
>> #include "drm_fourcc.h"
>>
>> or
>>
>> #define DRM_FOURCC_STANDALONE
>> #include "drm_fourcc.h"
>> #include "drm.h"
>>
>> the result must always be good.
>>
>>
>> Thanks,
>> pq
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Simon Ser]

On Monday, December 7th, 2020 at 9:57 AM, James Park <james.park@lagfreegames.com> wrote:

> I could adjust the block to look like this:
>
> #ifdef DRM_FOURCC_STANDALONE
> #if defined(__linux__)
> #include <linux/types.h>
> #else
> #include <stdint.h>
> typedef uint32_t __u32;
> typedef uint64_t __u64;
> #endif
> #else
> #include "drm.h"
> #endif

This approach still breaks on BSDs when DRM_FOURCC_STANDALONE is defined and
drm.h is included afterwards.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[James Park]

[-- Attachment #1.1: Type: text/plain, Size: 969 bytes --]

Not to make too big a deal of it, but the idea was that if you went out of
your way to define DRM_FOURCC_STANDALONE in your code base, that you would
also go through the pain of removing drm.h includes elsewhere. It's too
annoying of an implication to document/communicate, so I'm happier with the
other DRM_FOURCC_STANDALONE solution that pulls the basic types into a
common header.

Thanks,
James

On Mon, Dec 7, 2020 at 1:49 AM Simon Ser <contact@emersion.fr> wrote:

> On Monday, December 7th, 2020 at 9:57 AM, James Park <
> james.park@lagfreegames.com> wrote:
>
> > I could adjust the block to look like this:
> >
> > #ifdef DRM_FOURCC_STANDALONE
> > #if defined(__linux__)
> > #include <linux/types.h>
> > #else
> > #include <stdint.h>
> > typedef uint32_t __u32;
> > typedef uint64_t __u64;
> > #endif
> > #else
> > #include "drm.h"
> > #endif
>
> This approach still breaks on BSDs when DRM_FOURCC_STANDALONE is defined
> and
> drm.h is included afterwards.
>

[-- Attachment #1.2: Type: text/html, Size: 1472 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix drm.h uapi header for Windows

[Simon Ser]

On Monday, December 7th, 2020 at 11:00 AM, James Park <james.park@lagfreegames.com> wrote:

> Not to make too big a deal of it, but the idea was that if you went
> out of your way to define DRM_FOURCC_STANDALONE in your code base,
> that you would also go through the pain of removing drm.h includes
> elsewhere. It's too annoying of an implication to
> document/communicate, so I'm happier with the other
> DRM_FOURCC_STANDALONE solution that pulls the basic types into a
> common header.

In my compositors I'd like to globally define DRM_FOURCC_STANDALONE
(to make sure I don't miss any #define) but I still may include drm.h
in the same files as well.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel