intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).