All of lore.kernel.org
 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 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.