* [PATCH 1/2] drm/i915: make field unsigned
@ 2018-08-28 17:41 Lucas De Marchi
2018-08-28 17:41 ` [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct Lucas De Marchi
` (7 more replies)
0 siblings, 8 replies; 17+ messages in thread
From: Lucas De Marchi @ 2018-08-28 17:41 UTC (permalink / raw)
To: intel-gfx
subvendor and subdevice are unsigned, so fix their initialization in
INTEL_VGA_DEVICE.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
include/drm/i915_pciids.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index fd965ffbb92e..754ce4b10129 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -37,7 +37,7 @@
*/
#define INTEL_VGA_DEVICE(id, info) { \
0x8086, id, \
- ~0, ~0, \
+ ~0u, ~0u, \
0x030000, 0xff0000, \
(unsigned long) info }
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct
2018-08-28 17:41 [PATCH 1/2] drm/i915: make field unsigned Lucas De Marchi
@ 2018-08-28 17:41 ` Lucas De Marchi
2018-08-28 18:05 ` Chris Wilson
2018-08-28 18:06 ` Ville Syrjälä
2018-08-28 18:15 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: make field unsigned Patchwork
` (6 subsequent siblings)
7 siblings, 2 replies; 17+ messages in thread
From: Lucas De Marchi @ 2018-08-28 17:41 UTC (permalink / raw)
To: intel-gfx
Document it like a real struct for ease of copy and paste, remove
comment of C99 compatibility and document that in some cases the first 2
fields can be u16.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
include/drm/i915_pciids.h | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 754ce4b10129..0c2cc43f916c 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -26,14 +26,16 @@
#define _I915_PCIIDS_H
/*
- * A pci_device_id struct {
- * __u32 vendor, device;
- * __u32 subvendor, subdevice;
- * __u32 class, class_mask;
- * kernel_ulong_t driver_data;
+ * These macros can be used with a struct declared like this:
+ *
+ * struct pci_device_id {
+ * __u32 vendor, device;
+ * __u32 subvendor, subdevice;
+ * __u32 class, class_mask;
+ * kernel_ulong_t driver_data;
* };
- * Don't use C99 here because "class" is reserved and we want to
- * give userspace flexibility.
+ *
+ * First two fields may be __u16 if PCI_DEVICE_ANY is not used
*/
#define INTEL_VGA_DEVICE(id, info) { \
0x8086, id, \
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct
2018-08-28 17:41 ` [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct Lucas De Marchi
@ 2018-08-28 18:05 ` Chris Wilson
2018-08-28 19:30 ` Lucas De Marchi
2018-08-28 18:06 ` Ville Syrjälä
1 sibling, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2018-08-28 18:05 UTC (permalink / raw)
To: Lucas De Marchi, intel-gfx
Quoting Lucas De Marchi (2018-08-28 18:41:46)
> Document it like a real struct for ease of copy and paste, remove
> comment of C99 compatibility and document that in some cases the first 2
I do recall that we couldn't use either C99 or class due to userspace
compatibility... The essence is that we need a reminder that we can't
assume the relaxed nature of kcc here.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct
2018-08-28 17:41 ` [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct Lucas De Marchi
2018-08-28 18:05 ` Chris Wilson
@ 2018-08-28 18:06 ` Ville Syrjälä
2018-08-28 19:34 ` Lucas De Marchi
2018-09-05 20:19 ` [PATCH v2] " Lucas De Marchi
1 sibling, 2 replies; 17+ messages in thread
From: Ville Syrjälä @ 2018-08-28 18:06 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
On Tue, Aug 28, 2018 at 10:41:46AM -0700, Lucas De Marchi wrote:
> Document it like a real struct for ease of copy and paste, remove
> comment of C99 compatibility and document that in some cases the first 2
> fields can be u16.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> include/drm/i915_pciids.h | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index 754ce4b10129..0c2cc43f916c 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -26,14 +26,16 @@
> #define _I915_PCIIDS_H
>
> /*
> - * A pci_device_id struct {
> - * __u32 vendor, device;
> - * __u32 subvendor, subdevice;
> - * __u32 class, class_mask;
> - * kernel_ulong_t driver_data;
> + * These macros can be used with a struct declared like this:
> + *
> + * struct pci_device_id {
> + * __u32 vendor, device;
> + * __u32 subvendor, subdevice;
> + * __u32 class, class_mask;
> + * kernel_ulong_t driver_data;
> * };
> - * Don't use C99 here because "class" is reserved and we want to
> - * give userspace flexibility.
> + *
> + * First two fields may be __u16 if PCI_DEVICE_ANY is not used
PCI_DEVICE_ANY undefined?
Also you can surely use u16 just fine as long as you're careful when
comparing with ~0?
> */
> #define INTEL_VGA_DEVICE(id, info) { \
> 0x8086, id, \
> --
> 2.17.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: make field unsigned
2018-08-28 17:41 [PATCH 1/2] drm/i915: make field unsigned Lucas De Marchi
2018-08-28 17:41 ` [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct Lucas De Marchi
@ 2018-08-28 18:15 ` Patchwork
2018-08-28 18:34 ` ✓ Fi.CI.BAT: success " Patchwork
` (5 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-08-28 18:15 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: make field unsigned
URL : https://patchwork.freedesktop.org/series/48818/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
3def105ca25a drm/i915: make field unsigned
586500c7bf00 drm/i915: reword documentation of possible pci_device_id struct
-:30: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#30: FILE: include/drm/i915_pciids.h:32:
+ * ^I__u32 vendor, device;$
-:31: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#31: FILE: include/drm/i915_pciids.h:33:
+ * ^I__u32 subvendor, subdevice;$
-:32: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#32: FILE: include/drm/i915_pciids.h:34:
+ * ^I__u32 class, class_mask;$
-:33: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#33: FILE: include/drm/i915_pciids.h:35:
+ * ^Ikernel_ulong_t driver_data;$
total: 0 errors, 4 warnings, 0 checks, 23 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: make field unsigned
2018-08-28 17:41 [PATCH 1/2] drm/i915: make field unsigned Lucas De Marchi
2018-08-28 17:41 ` [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct Lucas De Marchi
2018-08-28 18:15 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: make field unsigned Patchwork
@ 2018-08-28 18:34 ` Patchwork
2018-08-28 22:33 ` ✓ Fi.CI.IGT: " Patchwork
` (4 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-08-28 18:34 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: make field unsigned
URL : https://patchwork.freedesktop.org/series/48818/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4718 -> Patchwork_10036 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/48818/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_10036 that come from known issues:
=== IGT changes ===
==== Issues hit ====
{igt@amdgpu/amd_basic@userptr}:
{fi-kbl-8809g}: PASS -> INCOMPLETE (fdo#107402)
igt@drv_selftest@live_hangcheck:
fi-skl-6600u: PASS -> DMESG-FAIL (fdo#106560, fdo#107174)
igt@kms_frontbuffer_tracking@basic:
{fi-byt-clapper}: PASS -> FAIL (fdo#103167)
igt@prime_vgem@basic-fence-flip:
fi-ilk-650: PASS -> FAIL (fdo#104008)
==== Possible fixes ====
igt@drv_selftest@live_hangcheck:
{fi-cfl-8109u}: DMESG-FAIL (fdo#106560) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-snb-2520m: INCOMPLETE (fdo#103713) -> PASS
{igt@kms_psr@primary_page_flip}:
fi-cnl-psr: FAIL (fdo#107336) -> PASS
{igt@pm_rpm@module-reload}:
fi-cnl-psr: WARN (fdo#107602, fdo#107708) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402
fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602
fdo#107708 https://bugs.freedesktop.org/show_bug.cgi?id=107708
== Participating hosts (54 -> 49) ==
Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4718 -> Patchwork_10036
CI_DRM_4718: c7398fd19cef9b11c79af7292109507b6be075c4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4611: b966dd93a30f41581fe1dbf9bc1c4a29b552ca05 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10036: 586500c7bf00e9295701c4df871a67a2a76f6cae @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
586500c7bf00 drm/i915: reword documentation of possible pci_device_id struct
3def105ca25a drm/i915: make field unsigned
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10036/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct
2018-08-28 18:05 ` Chris Wilson
@ 2018-08-28 19:30 ` Lucas De Marchi
2018-09-04 13:24 ` Jani Nikula
0 siblings, 1 reply; 17+ messages in thread
From: Lucas De Marchi @ 2018-08-28 19:30 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Tue, Aug 28, 2018 at 07:05:46PM +0100, Chris Wilson wrote:
> Quoting Lucas De Marchi (2018-08-28 18:41:46)
> > Document it like a real struct for ease of copy and paste, remove
> > comment of C99 compatibility and document that in some cases the first 2
>
> I do recall that we couldn't use either C99 or class due to userspace
you can't actually use a c++ compiler.
For C it works with any of -std=c99, gnu99, c11, gnu11, c17, gnu17.
Tested with both gcc and clang. I've never heard of class being a
reserved keyword and section 6.4.5 of said standard doesn't list it
neither.
Here the struct definition is in a *comment*... i.e. the user will copy
and paste somewhere else and probably change __u16 to uint16_t in
userspace. If he's building with g++, he can name the field to something
else.
If it was something we were defining in this header than I would agree
with you... to retain compatibility with c++, not c99.
Lucas De Marchi
> compatibility... The essence is that we need a reminder that we can't
> assume the relaxed nature of kcc here.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct
2018-08-28 18:06 ` Ville Syrjälä
@ 2018-08-28 19:34 ` Lucas De Marchi
2018-09-05 20:19 ` [PATCH v2] " Lucas De Marchi
1 sibling, 0 replies; 17+ messages in thread
From: Lucas De Marchi @ 2018-08-28 19:34 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Tue, Aug 28, 2018 at 09:06:15PM +0300, Ville Syrjälä wrote:
> On Tue, Aug 28, 2018 at 10:41:46AM -0700, Lucas De Marchi wrote:
> > Document it like a real struct for ease of copy and paste, remove
> > comment of C99 compatibility and document that in some cases the first 2
> > fields can be u16.
> >
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > ---
> > include/drm/i915_pciids.h | 16 +++++++++-------
> > 1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> > index 754ce4b10129..0c2cc43f916c 100644
> > --- a/include/drm/i915_pciids.h
> > +++ b/include/drm/i915_pciids.h
> > @@ -26,14 +26,16 @@
> > #define _I915_PCIIDS_H
> >
> > /*
> > - * A pci_device_id struct {
> > - * __u32 vendor, device;
> > - * __u32 subvendor, subdevice;
> > - * __u32 class, class_mask;
> > - * kernel_ulong_t driver_data;
> > + * These macros can be used with a struct declared like this:
> > + *
> > + * struct pci_device_id {
> > + * __u32 vendor, device;
> > + * __u32 subvendor, subdevice;
> > + * __u32 class, class_mask;
> > + * kernel_ulong_t driver_data;
> > * };
> > - * Don't use C99 here because "class" is reserved and we want to
> > - * give userspace flexibility.
> > + *
> > + * First two fields may be __u16 if PCI_DEVICE_ANY is not used
>
> PCI_DEVICE_ANY undefined?
this should be PCI_ANY_ID, but now I'm not sure if I should even mention
it since it's defined somewhere that's private to kernel.
>
> Also you can surely use u16 just fine as long as you're careful when
> comparing with ~0?
nops, since ~0u is unsigned int (~0 being int before patch 1), it will
overflow and cause a warning due to -Woverflow. This is what happens in
igt if I do what you said:
../intel/i915_pciids.h:40:2: warning: conversion from ‘unsigned int’ to ‘short unsigned int’ changes value from ‘4294967295’ to ‘65535’ [-Woverflow]
~0u, ~0u, \
^
Lucas De Marchi
>
> > */
> > #define INTEL_VGA_DEVICE(id, info) { \
> > 0x8086, id, \
> > --
> > 2.17.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: make field unsigned
2018-08-28 17:41 [PATCH 1/2] drm/i915: make field unsigned Lucas De Marchi
` (2 preceding siblings ...)
2018-08-28 18:34 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-08-28 22:33 ` Patchwork
2018-09-05 21:05 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: make field unsigned (rev2) Patchwork
` (3 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-08-28 22:33 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: make field unsigned
URL : https://patchwork.freedesktop.org/series/48818/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4718_full -> Patchwork_10036_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10036_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10036_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_10036_full:
=== IGT changes ===
==== Warnings ====
igt@kms_chv_cursor_fail@pipe-a-128x128-right-edge:
shard-snb: PASS -> SKIP +1
== Known issues ==
Here are the changes found in Patchwork_10036_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_cursor_crc@cursor-64x64-suspend:
shard-kbl: PASS -> INCOMPLETE (fdo#107556, fdo#103665)
igt@kms_flip@wf_vblank-ts-check-interruptible:
shard-kbl: PASS -> DMESG-WARN (fdo#103558, fdo#105602) +17
igt@kms_setmode@basic:
shard-kbl: PASS -> DMESG-FAIL (fdo#103558, fdo#105602, fdo#99912)
igt@perf@blocking:
shard-hsw: PASS -> FAIL (fdo#102252)
==== Possible fixes ====
igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
shard-glk: FAIL (fdo#105454, fdo#106509) -> PASS
igt@kms_draw_crc@draw-method-xrgb8888-blt-ytiled:
shard-glk: FAIL (fdo#107589) -> PASS
igt@kms_flip@2x-flip-vs-expired-vblank:
shard-glk: FAIL (fdo#105363) -> PASS
igt@kms_pipe_crc_basic@read-crc-pipe-b:
shard-glk: DMESG-WARN (fdo#105763) -> PASS +1
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
fdo#107589 https://bugs.freedesktop.org/show_bug.cgi?id=107589
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4718 -> Patchwork_10036
CI_DRM_4718: c7398fd19cef9b11c79af7292109507b6be075c4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4611: b966dd93a30f41581fe1dbf9bc1c4a29b552ca05 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10036: 586500c7bf00e9295701c4df871a67a2a76f6cae @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10036/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct
2018-08-28 19:30 ` Lucas De Marchi
@ 2018-09-04 13:24 ` Jani Nikula
0 siblings, 0 replies; 17+ messages in thread
From: Jani Nikula @ 2018-09-04 13:24 UTC (permalink / raw)
To: Lucas De Marchi, Chris Wilson; +Cc: intel-gfx
On Tue, 28 Aug 2018, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Tue, Aug 28, 2018 at 07:05:46PM +0100, Chris Wilson wrote:
>> Quoting Lucas De Marchi (2018-08-28 18:41:46)
>> > Document it like a real struct for ease of copy and paste, remove
>> > comment of C99 compatibility and document that in some cases the first 2
>>
>> I do recall that we couldn't use either C99 or class due to userspace
>
> you can't actually use a c++ compiler.
>
> For C it works with any of -std=c99, gnu99, c11, gnu11, c17, gnu17.
> Tested with both gcc and clang. I've never heard of class being a
> reserved keyword and section 6.4.5 of said standard doesn't list it
> neither.
>
> Here the struct definition is in a *comment*... i.e. the user will copy
> and paste somewhere else and probably change __u16 to uint16_t in
> userspace. If he's building with g++, he can name the field to something
> else.
>
> If it was something we were defining in this header than I would agree
> with you... to retain compatibility with c++, not c99.
I always thought the comment told you not to use designated initializers
(introduced in C99), which would both impose a minimum C version
requirement as well as bring .class = foo in the code, which requires
more than just renaming the field with C++.
BR,
Jani.
>
> Lucas De Marchi
>
>> compatibility... The essence is that we need a reminder that we can't
>> assume the relaxed nature of kcc here.
>> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] drm/i915: reword documentation of possible pci_device_id struct
2018-08-28 18:06 ` Ville Syrjälä
2018-08-28 19:34 ` Lucas De Marchi
@ 2018-09-05 20:19 ` Lucas De Marchi
2018-09-15 0:10 ` Lucas De Marchi
1 sibling, 1 reply; 17+ messages in thread
From: Lucas De Marchi @ 2018-09-05 20:19 UTC (permalink / raw)
To: intel-gfx
Document it like a real struct for ease of copy and paste, remove
comment of C99 compatibility and document that in some cases the first 2
fields can be u16.
v2: - remove mention to (non-existent) PCI_DEVICE_ANY and better explain
use of __u16 in first 2 fields
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
include/drm/i915_pciids.h | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 754ce4b10129..0f15d7b2e8ea 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -26,14 +26,17 @@
#define _I915_PCIIDS_H
/*
- * A pci_device_id struct {
- * __u32 vendor, device;
- * __u32 subvendor, subdevice;
- * __u32 class, class_mask;
- * kernel_ulong_t driver_data;
+ * These macros can be used with a struct declared like this:
+ *
+ * struct pci_device_id {
+ * __u32 vendor, device;
+ * __u32 subvendor, subdevice;
+ * __u32 class, class_mask;
+ * kernel_ulong_t driver_data;
* };
- * Don't use C99 here because "class" is reserved and we want to
- * give userspace flexibility.
+ *
+ * This matches the struct used in the kernel. First two fields may be
+ * changed to __u16 if using this header in a userspace program.
*/
#define INTEL_VGA_DEVICE(id, info) { \
0x8086, id, \
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 17+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: make field unsigned (rev2)
2018-08-28 17:41 [PATCH 1/2] drm/i915: make field unsigned Lucas De Marchi
` (3 preceding siblings ...)
2018-08-28 22:33 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-09-05 21:05 ` Patchwork
2018-09-05 21:38 ` Lucas De Marchi
2018-09-05 21:23 ` ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Patchwork @ 2018-09-05 21:05 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: make field unsigned (rev2)
URL : https://patchwork.freedesktop.org/series/48818/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
c39485603dc4 drm/i915: make field unsigned
6ee99a76cca9 drm/i915: reword documentation of possible pci_device_id struct
-:33: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#33: FILE: include/drm/i915_pciids.h:32:
+ * ^I__u32 vendor, device;$
-:34: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#34: FILE: include/drm/i915_pciids.h:33:
+ * ^I__u32 subvendor, subdevice;$
-:35: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#35: FILE: include/drm/i915_pciids.h:34:
+ * ^I__u32 class, class_mask;$
-:36: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#36: FILE: include/drm/i915_pciids.h:35:
+ * ^Ikernel_ulong_t driver_data;$
total: 0 errors, 4 warnings, 0 checks, 24 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: make field unsigned (rev2)
2018-08-28 17:41 [PATCH 1/2] drm/i915: make field unsigned Lucas De Marchi
` (4 preceding siblings ...)
2018-09-05 21:05 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: make field unsigned (rev2) Patchwork
@ 2018-09-05 21:23 ` Patchwork
2018-09-06 6:04 ` ✓ Fi.CI.IGT: " Patchwork
2018-10-31 18:12 ` [PATCH 1/2] drm/i915: make field unsigned Lucas De Marchi
7 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-09-05 21:23 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: make field unsigned (rev2)
URL : https://patchwork.freedesktop.org/series/48818/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4775 -> Patchwork_10103 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/48818/revisions/2/mbox/
== Known issues ==
Here are the changes found in Patchwork_10103 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@debugfs_test@read_all_entries:
fi-kbl-7560u: PASS -> INCOMPLETE (fdo#103665)
igt@kms_flip@basic-flip-vs-wf_vblank:
fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927)
==== Possible fixes ====
igt@drv_selftest@live_coherency:
fi-gdg-551: DMESG-FAIL (fdo#107164) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-byt-clapper: FAIL (fdo#103191, fdo#107362) -> PASS
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
== Participating hosts (54 -> 49) ==
Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4775 -> Patchwork_10103
CI_DRM_4775: 1a2bb6c061217718b972b3f4a74b96b61cf19d0c @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4630: 86686c6e2f7c6f0944bced11550e06d20bc6957f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10103: 6ee99a76cca960284fc550b0d138a353f723f2e3 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
6ee99a76cca9 drm/i915: reword documentation of possible pci_device_id struct
c39485603dc4 drm/i915: make field unsigned
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10103/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: make field unsigned (rev2)
2018-09-05 21:05 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: make field unsigned (rev2) Patchwork
@ 2018-09-05 21:38 ` Lucas De Marchi
0 siblings, 0 replies; 17+ messages in thread
From: Lucas De Marchi @ 2018-09-05 21:38 UTC (permalink / raw)
To: intel-gfx
On Wed, Sep 05, 2018 at 09:05:40PM +0000, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [1/2] drm/i915: make field unsigned (rev2)
> URL : https://patchwork.freedesktop.org/series/48818/
> State : warning
>
> == Summary ==
>
> $ dim checkpatch origin/drm-tip
> c39485603dc4 drm/i915: make field unsigned
> 6ee99a76cca9 drm/i915: reword documentation of possible pci_device_id struct
> -:33: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
> #33: FILE: include/drm/i915_pciids.h:32:
> + * ^I__u32 vendor, device;$
>
> -:34: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
> #34: FILE: include/drm/i915_pciids.h:33:
> + * ^I__u32 subvendor, subdevice;$
>
> -:35: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
> #35: FILE: include/drm/i915_pciids.h:34:
> + * ^I__u32 class, class_mask;$
>
> -:36: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
> #36: FILE: include/drm/i915_pciids.h:35:
> + * ^Ikernel_ulong_t driver_data;$
Those are in a comment and the spaces are actually correct since it's
part of the multline comment style.
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: make field unsigned (rev2)
2018-08-28 17:41 [PATCH 1/2] drm/i915: make field unsigned Lucas De Marchi
` (5 preceding siblings ...)
2018-09-05 21:23 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-09-06 6:04 ` Patchwork
2018-10-31 18:12 ` [PATCH 1/2] drm/i915: make field unsigned Lucas De Marchi
7 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-09-06 6:04 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: make field unsigned (rev2)
URL : https://patchwork.freedesktop.org/series/48818/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4775_full -> Patchwork_10103_full =
== Summary - SUCCESS ==
No regressions found.
== Known issues ==
Here are the changes found in Patchwork_10103_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-glk: PASS -> FAIL (fdo#105363, fdo#102887)
==== Possible fixes ====
igt@gem_exec_big:
shard-hsw: INCOMPLETE (fdo#103540) -> PASS
igt@kms_flip@modeset-vs-vblank-race-interruptible:
shard-glk: INCOMPLETE (fdo#103359, k.org#198133) -> PASS
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4775 -> Patchwork_10103
CI_DRM_4775: 1a2bb6c061217718b972b3f4a74b96b61cf19d0c @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4630: 86686c6e2f7c6f0944bced11550e06d20bc6957f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_10103: 6ee99a76cca960284fc550b0d138a353f723f2e3 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10103/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] drm/i915: reword documentation of possible pci_device_id struct
2018-09-05 20:19 ` [PATCH v2] " Lucas De Marchi
@ 2018-09-15 0:10 ` Lucas De Marchi
0 siblings, 0 replies; 17+ messages in thread
From: Lucas De Marchi @ 2018-09-15 0:10 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
On Wed, Sep 5, 2018 at 1:21 PM Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>
> Document it like a real struct for ease of copy and paste, remove
> comment of C99 compatibility and document that in some cases the first 2
> fields can be u16.
>
> v2: - remove mention to (non-existent) PCI_DEVICE_ANY and better explain
> use of __u16 in first 2 fields
ping review?
thanks
Lucas De Marchi
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> include/drm/i915_pciids.h | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index 754ce4b10129..0f15d7b2e8ea 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -26,14 +26,17 @@
> #define _I915_PCIIDS_H
>
> /*
> - * A pci_device_id struct {
> - * __u32 vendor, device;
> - * __u32 subvendor, subdevice;
> - * __u32 class, class_mask;
> - * kernel_ulong_t driver_data;
> + * These macros can be used with a struct declared like this:
> + *
> + * struct pci_device_id {
> + * __u32 vendor, device;
> + * __u32 subvendor, subdevice;
> + * __u32 class, class_mask;
> + * kernel_ulong_t driver_data;
> * };
> - * Don't use C99 here because "class" is reserved and we want to
> - * give userspace flexibility.
> + *
> + * This matches the struct used in the kernel. First two fields may be
> + * changed to __u16 if using this header in a userspace program.
> */
> #define INTEL_VGA_DEVICE(id, info) { \
> 0x8086, id, \
> --
> 2.17.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] drm/i915: make field unsigned
2018-08-28 17:41 [PATCH 1/2] drm/i915: make field unsigned Lucas De Marchi
` (6 preceding siblings ...)
2018-09-06 6:04 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-10-31 18:12 ` Lucas De Marchi
7 siblings, 0 replies; 17+ messages in thread
From: Lucas De Marchi @ 2018-10-31 18:12 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
+Chris
On Tue, Aug 28, 2018 at 10:41 AM Lucas De Marchi
<lucas.demarchi@intel.com> wrote:
>
> subvendor and subdevice are unsigned, so fix their initialization in
> INTEL_VGA_DEVICE.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> include/drm/i915_pciids.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index fd965ffbb92e..754ce4b10129 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -37,7 +37,7 @@
> */
> #define INTEL_VGA_DEVICE(id, info) { \
> 0x8086, id, \
> - ~0, ~0, \
> + ~0u, ~0u, \
This came from your suggestion in
https://patchwork.freedesktop.org/patch/245943/
"And then while you are there, add the missing 'u' to ~0u"
Should I resend this simple patch alone (since the second got nacked)?
thanks
Lucas De Marchi
> 0x030000, 0xff0000, \
> (unsigned long) info }
>
> --
> 2.17.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-10-31 18:12 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-28 17:41 [PATCH 1/2] drm/i915: make field unsigned Lucas De Marchi
2018-08-28 17:41 ` [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct Lucas De Marchi
2018-08-28 18:05 ` Chris Wilson
2018-08-28 19:30 ` Lucas De Marchi
2018-09-04 13:24 ` Jani Nikula
2018-08-28 18:06 ` Ville Syrjälä
2018-08-28 19:34 ` Lucas De Marchi
2018-09-05 20:19 ` [PATCH v2] " Lucas De Marchi
2018-09-15 0:10 ` Lucas De Marchi
2018-08-28 18:15 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: make field unsigned Patchwork
2018-08-28 18:34 ` ✓ Fi.CI.BAT: success " Patchwork
2018-08-28 22:33 ` ✓ Fi.CI.IGT: " Patchwork
2018-09-05 21:05 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: make field unsigned (rev2) Patchwork
2018-09-05 21:38 ` Lucas De Marchi
2018-09-05 21:23 ` ✓ Fi.CI.BAT: success " Patchwork
2018-09-06 6:04 ` ✓ Fi.CI.IGT: " Patchwork
2018-10-31 18:12 ` [PATCH 1/2] drm/i915: make field unsigned Lucas De Marchi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.