Re: simple pledge for xeyes(1)

2023-09-11 Thread Thomas Frohwein
On Fri, Sep 08, 2023 at 08:55:10AM -0300, Lucas de Sena wrote:

[...]

> Quoting from `xenocara/app/xclock/xclock.c`:
> 
> > {
> > /* force reading of XErrorDB into memory to avoid adding "rpath" to 
> >pledge below */
> > char buf[1];
> >
> >(void)XGetErrorDatabaseText(XtDisplay(toplevel), "XProtoError", "0", "", 
> >  buf, 1);
> > }
> > if (pledge("stdio", NULL) == -1)
> >err(1, "pledge");

In xclock.c, this happens right before XtAppMainLoop, and it looks like
we could do the exact same thing for xeyes... Is this better?

Index: xeyes.c
===
RCS file: /cvs/xenocara/app/xeyes/xeyes.c,v
retrieving revision 1.5
diff -u -p -r1.5 xeyes.c
--- xeyes.c 29 Aug 2021 17:50:32 -  1.5
+++ xeyes.c 12 Sep 2023 01:34:03 -
@@ -32,6 +32,10 @@ from the X Consortium.
 # include "config.h"
 #endif
 
+#ifdef HAVE_PLEDGE
+# include 
+# include 
+#endif
 #include 
 #include 
 #include 
@@ -142,6 +146,19 @@ main(int argc, char **argv)
 XtRealizeWidget (toplevel);
 (void) XSetWMProtocols (XtDisplay(toplevel), XtWindow(toplevel),
 _delete_window, 1);
+
+#ifdef HAVE_PLEDGE
+{
+   /* force reading of XErrorDB into memory to avoid adding "rpath" to
+ * pledge below */
+char buf[1];
+
+(void)XGetErrorDatabaseText(XtDisplay(toplevel), "XProtoError", "0", 
"", buf, 1);
+}
+if (pledge("stdio", NULL) == -1)
+   err(1, "pledge");
+#endif
+
 XtAppMainLoop(app_context);
 
 return 0;



simple pledge for xeyes(1)

2023-09-07 Thread Thomas Frohwein
Very basic pledge(2) for the whole program. I didn't dive too much into
the details and maybe this can be refined some more. This is kind of a
product of me trying a tool I made `abstain` [1] for usefulness of
pledge(2) execpromises and it helped quickly find that xeyes(1) can run
with a very limited set of promises. I tested all permutations of
running xeyes(1) that are listed in the man page and none of them break
with this configuration.

ok to add?

[1] https://github.com/rfht/abstain

Index: xeyes.c
===
RCS file: /cvs/xenocara/app/xeyes/xeyes.c,v
retrieving revision 1.5
diff -u -p -r1.5 xeyes.c
--- xeyes.c 29 Aug 2021 17:50:32 -  1.5
+++ xeyes.c 8 Sep 2023 03:23:51 -
@@ -38,6 +38,8 @@ from the X Consortium.
 #include "Eyes.h"
 #include 
 #include 
+#include 
+#include 
 #include "eyes.bit"
 #include "eyesmask.bit"
 
@@ -111,6 +113,8 @@ main(int argc, char **argv)
 Arg arg[2];
 Cardinal i;
 
+if(pledge("stdio rpath unix prot_exec", NULL) == -1)
+   err(1, "pledge");
 XtSetLanguageProc(NULL, (XtLanguageProc) NULL, NULL);
 
 toplevel = XtAppInitialize(_context, "XEyes",



Re: PATCH: a bit of introspection in make

2023-08-09 Thread Thomas Frohwein
On Tue, Aug 08, 2023 at 01:02:53PM +0200, Marc Espie wrote:
> Actually, as far as bsd.port.mk, it doesn't need to
> move too much stuff around thanks to make's lazyness.
> 
> Note that having a list of defined MASTER_SITES variables simplifies
> the check.
> 
> I've also added a check for the right MASTER_SITES to be defined,
> since currently we do not error out until actually using it, which
> means that fiddling around with MASTER_SITES before committing may
> often go unnoticed.
> 
> (That final part is meant to go in sooner rather than later)

this simplifies things overall a little (because var.c does more of the
heavy lifting now). checked this on limited scale with
`make show=_ALL_MASTER_SITES` working as expected. The DISTFILE
assignment to non-`:[0-9]` MASTER_SITESx needs a little more work, to
fully function, but this is a step in that direction.

ok thfr@ for this.

> Index: bsd.port.mk
> ===
> RCS file: /cvs/ports/infrastructure/mk/bsd.port.mk,v
> retrieving revision 1.1592
> diff -u -p -r1.1592 bsd.port.mk
> --- bsd.port.mk   13 Jun 2023 10:28:40 -  1.1592
> +++ bsd.port.mk   8 Aug 2023 10:59:38 -
> @@ -118,9 +118,8 @@ _ALL_VARIABLES_PER_ARCH =
>  # consumers of (dump-vars) include sqlports generation and dpb
>  # dpb doesn't need everything, those are speed optimizations
>  .if ${DPB:L:Mfetch} || ${DPB:L:Mall}
> -_ALL_VARIABLES += DISTFILES PATCHFILES SUPDISTFILES DIST_SUBDIR MASTER_SITES 
> \
> - MASTER_SITES0 MASTER_SITES1 MASTER_SITES2 MASTER_SITES3 MASTER_SITES4 \
> - MASTER_SITES5 MASTER_SITES6 MASTER_SITES7 MASTER_SITES8 MASTER_SITES9 \
> +_ALL_VARIABLES += DISTFILES PATCHFILES SUPDISTFILES DIST_SUBDIR \
> + ${_ALL_MASTER_SITES} \
>   CHECKSUM_FILE FETCH_MANUALLY MISSING_FILES PERMIT_DISTFILES
>  .endif
>  .if ${DPB:L:Mtest} || ${DPB:L:Mall}
> @@ -1280,19 +1280,15 @@ MASTER_SITES ?=
>  # sites for distfiles, add them to MASTER_SITE_BACKUP
>  
>  _warn_checksum = :
> -.if !empty(MASTER_SITES:M*[^/])
> -_warn_checksum += ;echo ">>> MASTER_SITES not ending in /: 
> ${MASTER_SITES:M*[^/]}"
> -.endif
>  
> -.for _I in 0 1 2 3 4 5 6 7 8 9
> -.  if defined(MASTER_SITES${_I})
> -.if !empty(MASTER_SITES${_I}:M*[^/])
> -_warn_checksum += ;echo ">>> MASTER_SITES${_I} not ending in /: 
> ${MASTER_SITES${_I}:M*[^/]}"
> -.endif
> +_ALL_MASTER_SITES = ${.VARIABLES:MMASTER_SITES*:NMASTER_SITES_*}
> +
> +.for _S in ${_ALL_MASTER_SITES}
> +.  if !empty(${_S}:M*[^/])
> +_warn_checksum += ;echo ">>> ${_S} not ending in /: ${${_S}:M*[^/]}"
>  .  endif
>  .endfor
>  
> -
>  EXTRACT_SUFX ?= .tar.gz
>  
>  .if !empty(GH_COMMIT)
> @@ -1322,6 +1318,9 @@ _FILES=
>  .  if !empty($v)
>  .for e in ${$v}
>  .  for f m u in ${e:C/:[0-9]$//:C/^(.*)\{.*\}(.*)$/\1\2/} 
> MASTER_SITES${e:M*\:[0-9]:C/^.*:([0-9])$/\1/} 
> ${e:C/:[0-9]$//:C/^.*\{(.*)\}(.*)$/\1\2/}
> +.if !defined($m)
> +ERRORS += "Fatal: $m is not defined but referenced by $e in $v"
> +.endif
>  .if empty(_FILES:M$f)
>  _FILES += $f
>  .  if empty(DIST_SUBDIR)



Re: PATCH: a bit of introspection in make

2023-08-09 Thread Thomas Frohwein
On Tue, Aug 08, 2023 at 12:51:43PM +0200, Marc Espie wrote:
> Here's a revised diff (reordered plus actual use of the boolean)
> plus concrete example  use for bsd.port.mk (disregarding the fact
> _ALL_VARIABLES would have to move *after* all MASTER_SITES have been defined.

I tested this on a small scale and it looks okay to me. ok thfr@ for
var.c part; see other email for the bsd.port.mk part.

> Index: var.c
> ===
> RCS file: /cvs/src/usr.bin/make/var.c,v
> retrieving revision 1.104
> diff -u -p -r1.104 var.c
> --- var.c 9 Jun 2022 13:13:14 -   1.104
> +++ var.c 8 Aug 2023 10:48:05 -
> @@ -104,6 +104,8 @@ static char   varNoError[] = "";
>  bool errorIsOkay;
>  static bool  checkEnvFirst;  /* true if environment should be searched for
>* variables before the global context */
> + /* do we need to recompute varname_list */
> +static bool  varname_list_changed = true;
>  
>  void
>  Var_setCheckEnvFirst(bool yes)
> @@ -222,6 +224,7 @@ typedef struct Var_ {
>  #define VAR_FROM_ENV 8   /* Special source: environment */
>  #define VAR_SEEN_ENV 16  /* No need to go look up environment again */
>  #define VAR_IS_SHELL 32  /* Magic behavior */
> +#define VAR_IS_NAMES 1024/* Very expensive, only defined when needed */
>  /* XXX there are also some flag values which are part of the visible API
>   * and thus defined inside var.h, don't forget to look there if you want
>   * to define some new flags !
> @@ -231,6 +234,8 @@ typedef struct Var_ {
>   char name[1];   /* the variable's name */
>  }  Var;
>  
> +/* for GNU make compatibility */
> +#define VARNAME_LIST ".VARIABLES"
>  
>  static struct ohash_info var_info = {
>   offsetof(Var, name),
> @@ -245,10 +250,11 @@ static void fill_from_env(Var *);
>  static Var *create_var(const char *, const char *);
>  static void var_set_initial_value(Var *, const char *);
>  static void var_set_value(Var *, const char *);
> -#define var_get_value(v) ((v)->flags & VAR_EXEC_LATER ? \
> - var_exec_cmd(v) : \
> - Buf_Retrieve(&((v)->val)))
> -static char *var_exec_cmd(Var *);
> +static char *var_get_value(Var *);
> +static void var_exec_cmd(Var *);
> +static void varname_list_retrieve(Var *);
> +
> +
>  static void var_append_value(Var *, const char *);
>  static void poison_check(Var *);
>  static void var_set_append(const char *, const char *, const char *, int, 
> bool);
> @@ -423,6 +429,7 @@ var_set_initial_value(Var *v, const char
>   len = strlen(val);
>   Buf_Init(&(v->val), len+1);
>   Buf_AddChars(&(v->val), len, val);
> + varname_list_changed = true;
>  }
>  
>  /* Normal version of var_set_value(), to be called after variable is fully
> @@ -440,6 +447,16 @@ var_set_value(Var *v, const char *val)
>   }
>  }
>  
> +static char *
> +var_get_value(Var *v)
> +{
> + if (v->flags & VAR_IS_NAMES)
> + varname_list_retrieve(v);
> + else if (v->flags & VAR_EXEC_LATER)
> + var_exec_cmd(v);
> + return Buf_Retrieve(&(v->val));
> +}
> +
>  /* Add to a variable, insert a separating space if the variable was already
>   * defined.
>   */
> @@ -628,6 +645,7 @@ Var_Deletei(const char *name, const char
>  
>   ohash_remove(_variables, slot);
>   delete_var(v);
> + varname_list_changed = true;
>  }
>  
>  /* Set or add a global variable, either to VAR_CMD or VAR_GLOBAL.
> @@ -687,7 +705,7 @@ Var_Appendi_with_ctxt(const char *name, 
>   var_set_append(name, ename, val, ctxt, true);
>  }
>  
> -static char *
> +static void
>  var_exec_cmd(Var *v)
>  {
>   char *arg = Buf_Retrieve(&(v->val));
> @@ -699,7 +717,30 @@ var_exec_cmd(Var *v)
>   var_set_value(v, res1);
>   free(res1);
>   v->flags &= ~VAR_EXEC_LATER;
> - return Buf_Retrieve(&(v->val));
> +}
> +
> +static void
> +varname_list_retrieve(Var *v)
> +{
> + unsigned int i;
> + void *e;
> + bool first = true;
> +
> + if (!varname_list_changed)
> + return;
> + for (e = ohash_first(_variables, ); e != NULL;
> + e = ohash_next(_variables, )) {
> + Var *v2 = e;
> + if (v2->flags & VAR_DUMMY)
> + continue;
> +
> + if (first)
> + var_set_value(v, v2->name);
> + else
> + var_append_value(v, v2->name);
> + first = false;
> + }
> + varname_list_changed = false;
>  }
>  
>  /* XXX different semantics for Var_Valuei() and Var_Definedi():
> @@ -1339,6 +1380,19 @@ set_magic_shell_variable()
>   v->flags = VAR_IS_SHELL | VAR_SEEN_ENV;
>  }
>  
> +static void
> +set_magic_name_list_variable()
> +{
> + const char *name = VARNAME_LIST;
> + const char *ename = NULL;
> + uint32_t k;
> + Var *v;
> +
> + k = ohash_interval(name, );
> + v = find_global_var_without_env(name, 

distexpand for autogenerated upstream distfile resources (was: standardize and simplify GitHub submodule handling in ports?)

2023-08-08 Thread Thomas Frohwein
On Mon, Aug 07, 2023 at 09:17:05PM +0100, Stuart Henderson wrote:

[...]

> I think maybe I'd prefer to have some variable that could be used
> *instead* of the existing GH_* variables rather than in conjunction with
> them (so they can be used for all GH archive ports, rather than have
> them a special case for multi-distfile ports). If that's the standard
> way to do things we can have a sweep of the tree converting other
> ports (or at least the ones that don't use go.port.mk ;)
> 
> It would be kind-of helpful if it could support more than just github
> too (gitlab.com, sr.ht, ..). While that could be done with different
> variables (GH_xx, GL_xx, SRHT_xx etc) they're all a similar enough
> layout to each other that making the site part of the variable itself
> rather than the name would be simpler and easier to add more sites
> (plus it covers the case where you have some port using one file from
> github and one from gitlab, etc).
> 
> Playing with syntax ideas, maybe something like this would be easy to
> use for pprts not needing a rename -
> 
> SOMEVAR+= github vim vim refs/tags/v9.0.1677
> SOMEVAR+= github vim colorschemes 22986fa2a3d2f7229efd4019fcbca411caa6afbb
> 
> or with some auto-renaming (and specifying more of the path to avoid the
> extra GH_WRKSRC which I think might not be enough in some cases anyway -
> a port may have several distfiles that need to go into different base
> dirs) -
> 
> SOMEVAR+= github fortran-lang fpm refs/tags/v0.7.0
> OTHERVAR+=github toml-f toml-f e49f5523e4ee67db6628618864504448fb8c8939 
> vendor/toml-f
> OTHERVAR+=github urbanjost M_CLI2 
> 90a1a146e19c8ad37b0469b8cbd04bc28eb67a50 vendor/M_CLI2
> 
> (no idea what to use as real names instead of SOMEVAR/OTHERVAR though!)
> 
> How does that sort of thing seem to you? (i.e. using the same basic idea as
> you have for submodules, but making it the standard for all gh distfiles)?

I ran with your suggestion and came up with a solution that I've named
distexpand. The idea is to use templates for commonly used,
automatically generated and therefore predictably named, stored, and
packaged dist files. 2 variables take different arguments/parts that
are 'expanded' with the template to working MASTER_SITESn and
DISTFILES.

The current configuration in the ports Makefile is done like this,
after putting distexpand.port.mk into /usr/ports/infrastructure/mk/:

MODULES += distexpand
DISTEXPAND += template account1 project1 id1(commithash/tag)
DISTEXPANDX += template account2 project2 id2(commithash/tag) targetdir

'template' is currently set up for github, gitlab, and sourcehut. You
can use multiple DISTEXPAND and DISTEXPANDX as needed. This will _not_
use up more MASTER_SITESn, as long as the template stays the same.

Regarding the naming, I'm definitely open to discuss other suggestions.
DISTEXPAND is what I've been able to think of that most clearly conveys
the use of the fragments that are expanded to a full address for
fetching the distfile. DISTEXPANDX - the last 'X' is meant to stand for
'extended' as this is the version that relocates the extracted files to
a target dir. I'm slightly partial to consider naming the variables
instead 'DISTEXPAND4' and 'DISTEXPAND5' which would remind the porter
of the number of components for each version.

For the templating, I used %account, %project, %id, %subdir as the
placeholders. Those are substituted later with :S. I'm open to
suggestions if there may be a more established pattern for placeholders
in strings in Makefile context.

This can replace GH_{ACCOUNT,PROJECT,TAGNAME,COMMIT}. Tags are detected
as such, and in that case a DISTNAME will be set to $project-$tag if
not otherwise set. In other scenarios, a DISTNAME or PKGNAME may need
to be set.

A couple of other things to note compared to before:
- GH_WRKSRC is gone without replacement. Its usefulness was
  questionable.
- It includes logic that finds the first MASTER_SITESn that isn't
  otherwise used, and throws an ERROR if it overruns past
  MASTER_SITES9.
- Using tags is now by just proving '0.1.0' or 'v0.11.2' or other
  non-commithash string (the heuristic checks for length to determine
  if this is a tag or a commit hash).
- It currently uses 2 longer for-loops that are almost identical, but
  one for DISTEXPAND, and the other one for DISTEXPANDX. Given the
  limitations in Makefiles, I couldn't think of a way to reuse more
  code there.

This doesn't need to be in a module, but this way it's easy to plug in
and experiment with.

I'm attaching the distexpand.port.mk, as well as the patch for using it
with neovim as an example. I've tested this with about 3 dozen ports
that use combinations of mostly github sites, but also a gitlab and a
sourcehut dist source [1].

[1] https://thfr.info/tmp/distexpand-ports.txt
Index: Makefile
===
RCS file: /cvs/ports/editors/neovim/Makefile,v
retrieving revision 1.37
diff -u -p -r1.37 Makefile
--- 

Re: PATCH: a bit of introspection in make

2023-08-07 Thread Thomas Frohwein
On Mon, Aug 07, 2023 at 04:42:54PM +0200, Marc Espie wrote:
> I think it could be occasionally useful to know which variables have
> been defined in make.
> 
> Incidentally, this DOES exist in GNU make, so I've reused the same name
> for basically the same functionality.
> 
> I haven't checked whether NetBSD/FreeBSD introduced something similar.
> 
> This is a fairly straightforward patch, introduces .VARIABLES corresponding
> to the full list of global variables (from the command line and the Makefile)
> that have been defined.
> 
> (^ says the guy who had to remember a few details from his own(!) var.c 
> implementation from a few years ago)
> 
> I just took var_get_value offline from the old macro, for readability,
> even though it's likely the compiler may still decide to inline it.
> 
> For efficiency, that list is only computed as needed, since it is
> somewhat long.
> 
> For debugging purposes, this can come in fairly handy, and I see at 
> least another application in ports.
> 
> Comments and nits welcome.
> 
> Note that the list is completely unsorted. This could be sorted through
> since we already have the code for dumping purposes, but it would be even
> more expensive (the order will be "random", as per the hash)

I can't judge all the implications of this, but can speak to that I
compiled it and that I get a very long list of variables with:

$ make show=.VARIABLES

> Index: var.c
> ===
> RCS file: /cvs/src/usr.bin/make/var.c,v
> retrieving revision 1.104
> diff -u -p -r1.104 var.c
> --- var.c 9 Jun 2022 13:13:14 -   1.104
> +++ var.c 7 Aug 2023 14:33:42 -
> @@ -104,6 +104,8 @@ static char   varNoError[] = "";
>  bool errorIsOkay;
>  static bool  checkEnvFirst;  /* true if environment should be searched for
>* variables before the global context */
> + /* do we need to recompute varname_list */
> +static bool  varname_list_changed = true;

I assume the comment /* do we need to recompute varname_list */ is not
meant to be contiguous with the 2 comment lines above (/* true if...
* variables before...). I suggest using whitespace (newline?) to make
that clearer.

I looked through the file and it seems that varname_list_changed is
never set to anything but true. Is there a bit missing, like down lower
in varname_list_retrieve()?

>  
>  void
>  Var_setCheckEnvFirst(bool yes)
> @@ -228,9 +230,12 @@ typedef struct Var_ {
>   */
>  #define POISONS (POISON_NORMAL | POISON_EMPTY | POISON_NOT_DEFINED)
>   /* Defined in var.h */
> +#define VAR_IS_NAMES 1024/* Very expensive, only defined when needed */
>   char name[1];   /* the variable's name */
>  }  Var;
>  
> +/* for GNU make compatibility */
> +#define VARNAME_LIST ".VARIABLES"
>  
>  static struct ohash_info var_info = {
>   offsetof(Var, name),
> @@ -245,10 +250,11 @@ static void fill_from_env(Var *);
>  static Var *create_var(const char *, const char *);
>  static void var_set_initial_value(Var *, const char *);
>  static void var_set_value(Var *, const char *);
> -#define var_get_value(v) ((v)->flags & VAR_EXEC_LATER ? \
> - var_exec_cmd(v) : \
> - Buf_Retrieve(&((v)->val)))
> -static char *var_exec_cmd(Var *);
> +static char *var_get_value(Var *);
> +static void var_exec_cmd(Var *);
> +static void varname_list_retrieve(Var *);
> +
> +
>  static void var_append_value(Var *, const char *);
>  static void poison_check(Var *);
>  static void var_set_append(const char *, const char *, const char *, int, 
> bool);
> @@ -423,6 +429,7 @@ var_set_initial_value(Var *v, const char
>   len = strlen(val);
>   Buf_Init(&(v->val), len+1);
>   Buf_AddChars(&(v->val), len, val);
> + varname_list_changed = true;
>  }
>  
>  /* Normal version of var_set_value(), to be called after variable is fully
> @@ -440,6 +447,16 @@ var_set_value(Var *v, const char *val)
>   }
>  }
>  
> +static char *
> +var_get_value(Var *v)
> +{
> + if (v->flags & VAR_IS_NAMES)
> + varname_list_retrieve(v);
> + else if (v->flags & VAR_EXEC_LATER)
> + var_exec_cmd(v);
> + return Buf_Retrieve(&(v->val));
> +}
> +
>  /* Add to a variable, insert a separating space if the variable was already
>   * defined.
>   */
> @@ -628,6 +645,7 @@ Var_Deletei(const char *name, const char
>  
>   ohash_remove(_variables, slot);
>   delete_var(v);
> + varname_list_changed = true;
>  }
>  
>  /* Set or add a global variable, either to VAR_CMD or VAR_GLOBAL.
> @@ -687,7 +705,7 @@ Var_Appendi_with_ctxt(const char *name, 
>   var_set_append(name, ename, val, ctxt, true);
>  }
>  
> -static char *
> +static void
>  var_exec_cmd(Var *v)
>  {
>   char *arg = Buf_Retrieve(&(v->val));
> @@ -699,7 +717,27 @@ var_exec_cmd(Var *v)
>   var_set_value(v, res1);
>   free(res1);
>   v->flags &= ~VAR_EXEC_LATER;
> - 

Re: standardize and simplify GitHub submodule handling in ports?

2023-08-07 Thread Thomas Frohwein
On Mon, Aug 07, 2023 at 06:59:15PM +0100, Stuart Henderson wrote:
[...]

> I haven't looked at other ports, but asterisk, vim and vmm-firmware do
> not use git submodules.

With vim, it's the way colorschemes are pulled in that *could* be
reworked using GH_SUBMODULES syntax. The old way continues to work, so
for any of the ports listed, there is no need to change anything.



Re: standardize and simplify GitHub submodule handling in ports?

2023-08-07 Thread Thomas Frohwein
On Sun, Aug 06, 2023 at 07:00:49PM +0200, Marc Espie wrote:

[...]

> > > I'm also wondering if keeping the main GH_* stuff in bsd.port.mk makes a 
> > > lot
> > > of sense, instead of grouping everything in github.port.mk
> > 
> > I'm for it, maybe as a second step after the module for just the
> > submodule handling is done because there would be a lot of ports churn
> > with moving the main GH_* stuff out of bsd.port.mk.
> 
> Probably not. We have a few "big" modules that don't depend on explicitly
> adding to modules, but on some variable triggering it (historic imake stuff
> or configure), having github.port.mk brought in from one of the GH_* variables
> (probably don't need to test them all) would be acceptable.

I'm sharing a new version after some testing, this time as a diff
because it turns out the handling of GH_*-related DISTNAME juggling
needs to happen before setting up the DISTFILES in the module. So this
diff now effectively moves all the GH_* bits from bsd.port.mk into
github.port.mk. The module is hooked up by defining GH_ACCOUNT or
GH_SUBMODULES.

Some rework was needed while testing to handle the different situations
for the target directory of the submodule. This ended up with a test if
the directory exists, and depending on the result either rmdir'ing the
(presumed empty) directory, or mkdir'ing the parent directory.

I tested this with the about 30 ports I could identify that use GitHub
submodules, by adjusting the Makefile to use GH_SUBMODULES. Here a few
points from what I've observed:

1. It's well possible to mix and match with the old way of defining
MASTER_SITES0 etc and DISTFILES=filename.ext:0. An example is
devel/fpm, where I kept this line:
MASTER_SITES0 =
https://github.com/fortran-lang/fpm/releases/download/v${V}/
while replacing MASTER_SITES{1,2} with the new GH_SUBMODULES way.

2. Setting GH_WRKSRC allows keeping the diff from the current Makefile
small, for example in devel/fpm:
+GH_WRKSRC =${WRKSRC}/vendor
and in editors/neovim:
+GH_WRKSRC =${STATIC_DEPS_WRKSRC}

3. It's possible to either use the commit hash or a tagname, like here
for lang/jruby:
+GH_SUBMODULES+=jnr jffi refs/tags/jffi-1.3.10 jffi

4. When using EXTRACT_ONLY, the name to use is more complicated now,
but can be identified with `$ make show=CHECKSUMFILES`. Here is what I
had to change it to with print/lilypond:
-EXTRACT_ONLY=  ${DISTNAME}.tar.gz urw-base35-fonts-${URW_V}.tar.gz
+EXTRACT_ONLY=  ${DISTNAME}.tar.gz \
+   
gh-submodules/ArtifexSoftware-urw-base35-fonts-${URW_V}.tar.gz

The full table of what I tested and the result up to if the port still
packages is here: https://thfr.info/tmp/github-submodule-ports.txt

I also tested megaglest as a port that uses GH_ACCOUNT etc without using any
submodules. Tested also with dxx-rebirth as an example of a port not
using any GH_*.

This draft hijacks MASTER_SITES8 for the modules purposes - not
MASTER_SITES9, as I noticed that is used by some modules like cargo and
go. I grep'd through the ports tree and couldn't identify any explicit
use of MASTER_SITES8.

I think this is ready for more general testing:
$ cd /usr/ports/infrastructure/mk; patch < /path/to/github-submodules.diff
Index: bsd.port.mk
===
RCS file: /cvs/ports/infrastructure/mk/bsd.port.mk,v
retrieving revision 1.1595
diff -u -p -r1.1595 bsd.port.mk
--- bsd.port.mk 8 Jul 2023 10:20:16 -   1.1595
+++ bsd.port.mk 7 Aug 2023 16:42:28 -
@@ -136,8 +136,8 @@ _ALL_VARIABLES += BROKEN COMES_WITH \
CONFIGURE_STYLE USE_LIBTOOL SEPARATE_BUILD \
SHARED_LIBS TARGETS PSEUDO_FLAVOR \
AUTOCONF_VERSION AUTOMAKE_VERSION CONFIGURE_ARGS \
-   GH_ACCOUNT GH_COMMIT GH_PROJECT GH_TAGNAME MAKEFILE_LIST \
-   USE_LLD USE_NOEXECONLY USE_WXNEEDED USE_NOBTCFI \
+   GH_ACCOUNT GH_COMMIT GH_PROJECT GH_SUBMODULES GH_TAGNAME \
+   MAKEFILE_LIST USE_LLD USE_NOEXECONLY USE_WXNEEDED USE_NOBTCFI \
COMPILER COMPILER_LANGS COMPILER_LINKS \
SUBST_VARS UPDATE_PLIST_ARGS \
PKGPATHS DEBUG_PACKAGES DEBUG_CONFIGURE_ARGS \
@@ -313,6 +313,10 @@ MODULES += ${_i}
 .  endif
 .endfor
 
+.if defined(GH_ACCOUNT) || defined(GH_SUBMODULES)
+MODULES += github
+.endif
+
 MODULES ?=
 .if !empty(MODULES) || !empty(COMPILER)
 _MODULES_DONE =
@@ -611,18 +615,6 @@ BUILD_DEPENDS += textproc/groff>=1.21
 _PKG_ARGS += -DUSE_GROFF=1
 .endif
 
-# github related variables
-GH_TAGNAME ?=
-GH_COMMIT ?=
-GH_ACCOUNT ?=
-GH_PROJECT ?=
-
-.if !empty(GH_PROJECT) && !empty(GH_TAGNAME)
-_GH_TAG_DIST = ${GH_TAGNAME:C/^(v|V|ver|[Rr]el|[Rr]elease)[-._]?([0-9])/\2/}
-DISTNAME ?=   ${GH_PROJECT}-${_GH_TAG_DIST}
-GH_DISTFILE = ${GH_PROJECT}-${_GH_TAG_DIST}${EXTRACT_SUFX}
-.endif
-
 PKGNAME ?= ${DISTNAME}
 FULLPKGNAME ?= ${PKGNAME}${FLAVOR_EXT}
 _MASTER ?=
@@ -871,11 +863,7 @@ _WRKDIRS = ${WRKOBJDIR_${PKGPATH}}/${_WR
 _WRKDIRS += ${WRKOBJDIR}/${_WRKDIR_STEM}
 _WRKDIRS += 

Re: standardize and simplify GitHub submodule handling in ports?

2023-08-05 Thread Thomas Frohwein
On Sat, Aug 05, 2023 at 11:27:24PM +0200, Marc Espie wrote:
> Some comments already. I haven't looked very closely.

> On Sat, Aug 05, 2023 at 03:12:18PM -0400, Thomas Frohwein wrote:
> > The current draft hijacks post-extract target, but it would be easy to
> > add this to _post-extract-finalize in bsd.port.mk similar to how the
> > post-extract commands from modules are handled, if this is of interest.
> 
> Please do that.

Thanks, I updated the draft. Realized that including it with
MODULES=github is easiest and then this can just use
MODGITHUB_post-extract and doesn't need any custom code in bsd.port.mk.
I had a thinko in post-extract (needs to be '||', not '&&') which is
also corrected.

> > # where submodule distfiles will be stored
> > GHSM_DIST_SUBDIR ?= gh-submodules
> 
> Please keep to the GH_* subspace.
> 
> Already, modules usually use MOD* variable names, but in the GH case, that
> would be a bit long.

I renamed GHSM_* to GH_*. I wouldn't mind using MODGH_* if that's an
option, but MODGITHUB_* would be pretty unwieldy, especially if we were
to make the existing GH_{ACCOUNT,PROJECT,TAGNAME} etc. part of this.

[...]

> Please do a single loop.  That's slightly more readable for me.

yes, done.

[...]

> Also please draft a diff for port-modules(5)

I'm attaching a diff for port-modules.5, along with the updated
github.port.mk.

> I'm also wondering if keeping the main GH_* stuff in bsd.port.mk makes a lot
> of sense, instead of grouping everything in github.port.mk

I'm for it, maybe as a second step after the module for just the
submodule handling is done because there would be a lot of ports churn
with moving the main GH_* stuff out of bsd.port.mk.
# List of static dependencies. The format is:
# account project tag_or_commit target_dir # license
# Example:
# GH_SUBMODULES +=  moonlight-stream moonlight-common-c \
#   c9426a6a71c4162e65dde8c0c71a25f1dbca46ba \
#   third-party/moonlight-common-c # GPL-v3.0+
GH_SUBMODULES ?=

# Master site for github tarballs
GH_MASTER_SITES ?=  https://github.com/

# where submodule distfiles will be stored
GH_DIST_SUBDIR ?=   gh-submodules

# where submodules will be extracted to
GH_WRKSRC ?=${WRKSRC}

# Grab submodules by default with MASTER_SITES8. (Don't use 9 to avoid collision
# with language-specific mechanisms, like devel/cargo or lang/go.)
GH_MASTER_SITESN ?= 8
MASTER_SITES${GH_MASTER_SITESN} ?=  ${GH_MASTER_SITES}

# Default GitHub distfile suffix
GH_SUFX ?=  .tar.gz

.if defined(DISTNAME)
DISTFILES ?= ${DISTNAME}${EXTRACT_SUFX}
.elif !empty(GH_ACCOUNT) && !empty(GH_PROJECT)
DISTFILES ?= ${GH_DISTFILE}
.endif

# post-extract target for moving the submodules to GH_WRKSRC
MODGITHUB_post-extract = \
@${ECHO_MSG} "moving GitHub submodules to ${GH_WRKSRC}" ; \
mkdir -p ${GH_WRKSRC} ;

.for _ghaccount _ghproject _ghtagcommit _targetdir in ${GH_SUBMODULES}
DISTFILES +=
${GH_DIST_SUBDIR}/{}${_ghaccount}/${_ghproject}/archive/${_ghtagcommit}${GH_SUFX}:${GH_MASTER_SITESN}
MODGITHUB_post-extract += \
test -d ${GH_WRKSRC}/${_targetdir} && \
rm -rf ${GH_WRKSRC}/${_targetdir} ; \
mv ${WRKDIR}/${_ghproject}-${_ghtagcommit} ${GH_WRKSRC}/${_targetdir} ;
.endfor
Index: port-modules.5
===
RCS file: /cvs/src/share/man/man5/port-modules.5,v
retrieving revision 1.264
diff -u -p -r1.264 port-modules.5
--- port-modules.5  9 May 2023 19:44:06 -   1.264
+++ port-modules.5  6 Aug 2023 01:48:22 -
@@ -744,6 +744,15 @@ contains c++, this module provides
 .Ev MODGCC4_CPPLIBDEP
 and
 .Ev MODGCC4_CPPWANTLIB .
+.It github
+Set
+.Li GH_SUBMODULES += w x y z
+for each GitHub submodule to be used in the port, specifying GitHub account, 
project, commit hash, and the target directory relative to
+.Ev GH_WRKSRC
+.Po
+defaults to
+.Ev WRKSRC
+.Pc .
 .It gnu
 This module is documented in the main
 .Xr bsd.port.mk 5


standardize and simplify GitHub submodule handling in ports?

2023-08-05 Thread Thomas Frohwein
bRocket
 
 # c++11
 COMPILER = base-clang ports-gcc
@@ -59,14 +52,8 @@ CONFIGURE_ARGS = -DCMAKE_INSTALL_PREFIX=
 NO_TEST =  Yes
 
 # remove bundled SDL
-post-extract:
+pre-configure:
rm -rf  ${WRKSRC}/lib/libsdl
-   rmdir   ${WRKSRC}/cmake/external/rpavlik-cmake-modules
-   ln -s   ${WRKDIR}/cmake-modules-${RPAVLIK_CMAKE_MOD} \
-   ${WRKSRC}/cmake/external/rpavlik-cmake-modules
-   rmdir   ${WRKSRC}/lib/libRocket
-   ln -s   ${WRKDIR}/libRocket-${LIBROCKET} \
-   ${WRKSRC}/lib/libRocket
 
 do-gen:
${SUBST_CMD} ${WRKSRC}/CMakeLists.txt
@@ -74,4 +61,5 @@ do-gen:
 post-install:
${SUBST_CMD} -c -m 755 ${FILESDIR}/fs2open ${PREFIX}/bin/fs2open
 
+.include "/usr/ports/mystuff/github.port.mk"
 .include 
Index: distinfo
===
RCS file: /cvs/ports/games/fs2open/distinfo,v
retrieving revision 1.7
diff -u -p -r1.7 distinfo
--- distinfo22 Feb 2023 06:51:24 -  1.7
+++ distinfo5 Aug 2023 19:05:53 -
@@ -1,6 +1,6 @@
-SHA256 (fs2open-23.0.0/7cef9577d6fc35057ea57f46b4986a8a28aeff50.tar.gz) = 
e2kS2gGlbuyljBP4u7B7YSCvjwRyIxZ0fcgTKo7GWso=
-SHA256 (fs2open-23.0.0/ecd648a43aff8a9f3daf064d75ca5725237d5b38.tar.gz) = 
JcjDM2xciQqxzt/90Z17tubwjVy+HASUyiAs+5h/Tfc=
-SHA256 (fs2open-23.0.0/fs2open-23.0.0.tar.gz) = 
AZok/u73pI+BcPAznFUwEisya9iPtz6ddRW/6eU4Gqg=
-SIZE (fs2open-23.0.0/7cef9577d6fc35057ea57f46b4986a8a28aeff50.tar.gz) = 267464
-SIZE (fs2open-23.0.0/ecd648a43aff8a9f3daf064d75ca5725237d5b38.tar.gz) = 2417905
-SIZE (fs2open-23.0.0/fs2open-23.0.0.tar.gz) = 12210430
+SHA256 (fs2open-23.0.0.tar.gz) = AZok/u73pI+BcPAznFUwEisya9iPtz6ddRW/6eU4Gqg=
+SHA256 
(gh-submodules/asarium/cmake-modules/archive/7cef9577d6fc35057ea57f46b4986a8a28aeff50.tar.gz)
 = e2kS2gGlbuyljBP4u7B7YSCvjwRyIxZ0fcgTKo7GWso=
+SHA256 
(gh-submodules/asarium/libRocket/archive/ecd648a43aff8a9f3daf064d75ca5725237d5b38.tar.gz)
 = JcjDM2xciQqxzt/90Z17tubwjVy+HASUyiAs+5h/Tfc=
+SIZE (fs2open-23.0.0.tar.gz) = 12210430
+SIZE 
(gh-submodules/asarium/cmake-modules/archive/7cef9577d6fc35057ea57f46b4986a8a28aeff50.tar.gz)
 = 267464
+SIZE 
(gh-submodules/asarium/libRocket/archive/ecd648a43aff8a9f3daf064d75ca5725237d5b38.tar.gz)
 = 2417905
Index: Makefile
===
RCS file: /cvs/ports/games/fna/Makefile,v
retrieving revision 1.17
diff -u -p -r1.17 Makefile
--- Makefile15 Jul 2023 23:24:35 -  1.17
+++ Makefile5 Aug 2023 19:10:00 -
@@ -7,21 +7,16 @@ HOMEPAGE =https://fna-xna.github.io/
 MAINTAINER =   Thomas Frohwein 
 
 # MS-PL, includes lzxdecoder (dual MSPL/LGPL) and Mono.Xna (MIT)
-# zlib (Vorbisfile-CS)
 PERMIT_PACKAGE =   Yes
 
-NETSTUB =  ebff244074bb3c28aeeb8cf7b383b5a029d7e28d
-VORBISFILE =   521c8532f03b3608a141b36d7c1343e816b46cb1
+GH_SUBMODULES+=FNA-XNA FNA.NetStub \
+   ebff244074bb3c28aeeb8cf7b383b5a029d7e28d \
+   ../FNA.NetStub  # Ms-PL
+GH_SUBMODULES+=flibitijibibo Vorbisfile-CS \
+   521c8532f03b3608a141b36d7c1343e816b46cb1 \
+   Vorbisfile-CS   # zlib
 
 MASTER_SITES = https://github.com/FNA-XNA/FNA/releases/download/${V}/
-MASTER_SITES0 =https://github.com/FNA-XNA/FNA.NetStub/archive/
-MASTER_SITES1 =https://github.com/flibitijibibo/Vorbisfile-CS/archive/
-
-DISTFILES =${DISTNAME}${EXTRACT_SUFX} \
-   ${NETSTUB}.tar.gz:0 \
-   ${VORBISFILE}.tar.gz:1
-
-DIST_SUBDIR =  fna-${V}
 
 EXTRACT_SUFX = .zip
 MODULES =  lang/mono
@@ -31,14 +26,9 @@ RUN_DEPENDS =audio/faudio \
 WRKDIST =  ${WRKDIR}/FNA
 NO_TEST =  Yes
 ALL_TARGET =   release
-SUBST_VARS +=  VORBISFILE
-
-post-extract:
-   ln -sf ${WRKDIR}/FNA.NetStub-${NETSTUB} ${WRKDIR}/FNA.NetStub
 
 do-gen:
cp ${FILESDIR}/FNA.Settings.props ${WRKSRC}/
-   ${SUBST_CMD} ${WRKSRC}/FNA.Settings.props
 
 # need to rm bin,obj before FNA.sln to build with FNA.Settings.props
 do-build:
@@ -63,4 +53,5 @@ do-install:
${PREFIX}/share/FNA/
${INSTALL_DATA} ${FILESDIR}/FNA.dll.config ${PREFIX}/share/FNA/
 
+.include "/usr/ports/mystuff/github.port.mk"
 .include 
Index: distinfo
===
RCS file: /cvs/ports/games/fna/distinfo,v
retrieving revision 1.15
diff -u -p -r1.15 distinfo
--- distinfo15 Jul 2023 23:24:35 -  1.15
+++ distinfo5 Aug 2023 19:10:00 -
@@ -1,6 +1,6 @@
-SHA256 (fna-23.07/521c8532f03b3608a141b36d7c1343e816b46cb1.tar.gz) = 
Cj2HaaOFBDsXtb/LPiedFdKzTb59XU9nMU7fuR9C6gc=
-SHA256 (fna-23.07/ebff244074bb3c28aeeb8cf7b383b5a029d7e28d.tar.gz) = 
34qgjqOOFTODn6zSKEs6Gt9jbdaJi9MR/7rw12tlXIA=
-SHA256 (fna-23.07/fna-2307.zip) = 6ZmbKqwz0X+RKXGvQTgqsZtLDtPvdcNURGoROTZye0s=
-SIZE (fna-23.07/521c8532f03b3608a141b36d7c1343e816b46cb1.tar.gz) = 5607
-SIZE (fna-23.07/ebff244074bb3c28aeeb8cf7b383b5a029d7e28d.tar.gz) 

Re: Diff for evaluation (WACOM tablet driver)

2023-07-04 Thread Thomas Frohwein
On Mon, Jul 03, 2023 at 11:22:45AM +0200, Marc Espie wrote:
> I hope Vladimir will find the time to complete this answer.
> 
> As far as Vlad's work goes, he did a presentation last week-end:
> https://www.lre.epita.fr/news_content/SS_summer_week_pres/Vladimir_Driver_OpenBSD.pptx
> 
> (sorry for the medium, fortunately we have libreoffice)
> 
> In the mean time, here is an updated diff.
> 
> I removed the Gaomon stuff, which if anything should be a different patch.
> 
> And I cleaned up the 20+ minor style violations I could find...
> (tabs instead of +4 spaces for continued lines, a few non-style compliant
> function declarations and/or code blocks, oh well)
> 
> plus an extra malloc.h that snuck in and is not at all needed.
> 
> And some typos in comments.
> And a C++ style comment. Oh well
> 
> I would really for some version of this to get in soonish.
> I can vouch that my tablet "works" with it (well, as good as it can work
> within the limitations of wscons not allowing it to be easily differentiated
> from the normal mouse, which is really a pain for programs like gimp)

Thanks, I built a kernel with this and no issues observed. I have a
Wacom Bamboo (CTH-470, product id 0x00de) that doesn't attach yet, but
this can be left for future work. I don't see anything glaring. hid.c
could probably use some refactoring at some point in the future because
at least 3 functions now share 99% identical code (hid_is_collection,
hid_get_collection_data, hid_get_id_of_collection).

I recommend getting an ok from someone with more track record with
dev/hid and/or dev/usb, but from my side this is ok thfr@.

> 
> dmesg for the tablet with the diff
> | uhidev1 at uhub1 port 4 configuration 1 interface 0 "Wacom Co.,Ltd. Intuos 
> S" rev 2.00/1.07 addr 6
> | uhidev1: iclass 3/0, 228 report ids
> | uwacom0 at uhidev1: 9 buttons, Z and W dir, tip, barrel
> | wsmouse5 at uwacom0 mux 0
> | uwacom1 at uhidev1: 9 buttons, Z and W dir, tip, barrel
> | wsmouse6 at uwacom1 mux 0
> | uwacom2 at uhidev1: 9 buttons, Z and W dir, tip, barrel
> | wsmouse7 at uwacom2 mux 0
> 
> as far as I understand, it appears as several mice because the stylus
> acts as totally different devices depending on the mode/end used
> (stuff that wscons completely hides from us).
> 
> Without the patch, that tablet appears as 42 different uhid devices (!)
> 
> The idea is that the parser for collections was really primitive. The
> debug stuff can show the details of various collection. There is the actual
> tablet mechanisms (which becomes one device) including scale, stylus, etc,
> and some other wacky collections (!): a debug collection that the wacom guys
> told us "oh some of our hw team needs that, but don't ever touch" and
> some other stuff we can't support yet (like battery support for some
> advanced models of stylus)
> 
> Index: dev/hid/hid.c
> ===
> RCS file: /cvs/src/sys/dev/hid/hid.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 hid.c
> --- dev/hid/hid.c 20 May 2022 05:03:45 -  1.5
> +++ dev/hid/hid.c 3 Jul 2023 09:04:50 -
> @@ -657,3 +657,51 @@ hid_is_collection(const void *desc, int 
>   hid_end_parse(hd);
>   return (0);
>  }
> +
> +struct hid_data *
> +hid_get_collection_data(const void *desc, int size, int32_t usage,
> +uint32_t collection)
> +{
> + struct hid_data *hd;
> + struct hid_item hi;
> +
> + hd = hid_start_parse(desc, size, hid_all);
> +
> + DPRINTF("%s: usage=0x%x\n", __func__, usage);
> + while (hid_get_item(hd, )) {
> + DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__,
> + hi.kind, hi.report_ID, hi.usage, usage);
> + if (hi.kind == hid_collection &&
> + hi.collection == collection && hi.usage == usage){
> + DPRINTF("%s: found\n", __func__);
> + return hd;
> + }
> + }
> + DPRINTF("%s: not found\n", __func__);
> + hid_end_parse(hd);
> + return NULL;
> +}
> +
> +int
> +hid_get_id_of_collection(const void *desc, int size, int32_t usage,
> +uint32_t collection)
> +{
> + struct hid_data *hd;
> + struct hid_item hi;
> +
> + hd = hid_start_parse(desc, size, hid_all);
> +
> + DPRINTF("%s: id=%d usage=0x%x\n", __func__, id, usage);
> + while (hid_get_item(hd, )) {
> + DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__,
> + hi.kind, hi.report_ID, hi.usage, usage);
> + if (hi.kind == hid_collection &&
> + hi.collection == collection && hi.usage == usage){
> + DPRINTF("%s: found\n", __func__);
> + return hi.report_ID;
> + }
> + }
> + DPRINTF("%s: not found\n", __func__);
> + hid_end_parse(hd);
> + return 0;
> +}
> Index: dev/hid/hid.h
> ===
> RCS file: 

Re: Intel Arc / DG2

2023-02-13 Thread Thomas Frohwein
On Mon, Feb 13, 2023 at 08:40:22PM +1100, Jonathan Gray wrote:
> On Sun, Feb 12, 2023 at 02:25:47PM -0500, Thomas Frohwein wrote:
> > On Tue, Feb 07, 2023 at 08:51:40PM +1100, Jonathan Gray wrote:
> > 
> > [...] 
> > > > ...
> > > > i915_resize_lmem_bar: stub
> > > > panic: kernel diagnostic assertion "pdev->pci->sc_bridgetag == NULL" 
> > > > failed: file "/usr/src/sys/dev/pci/drm/drm_linux.c", line 1277
> > > > ...
> > > 
> > > The vga arbiter bits need to change.  There isn't an easy way to get
> > > a softc to avoid having state shared by multiple inteldrm instances.
> > > 
> > > perhaps they can be skipped for the moment?
> > 
> > Thanks, this leads to a uvm_fault now:
> > 
> > xehp_load_dss_mask: stub
> > xehp_load_dss_mask: stub
> > intel_slicemask_from_xehp_dssmask: stub
> > intel_slicemask_from_xehp_dssmask: stub
> > i915_resize_lmem_bar: stub
> > uvm_fault(0x825181f8, 0x10, 0, 1) -> e
> > 
> > screen photo at https://thfr.info/tmp/DG2-error-20230212.jpg
> 
> unfortunately, that isn't much help
> 
> If you boot -d and do:
>   w db_panic 0
>   c
> you should be able to get a backtrace.

There is no difference if I do this, and still no backtrace.

> 
> > 
> > > 
> > > Index: sys/dev/pci/drm/drm_linux.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v
> > > retrieving revision 1.95
> > > diff -u -p -r1.95 drm_linux.c
> > > --- sys/dev/pci/drm/drm_linux.c   1 Jan 2023 01:34:34 -   1.95
> > > +++ sys/dev/pci/drm/drm_linux.c   7 Feb 2023 09:31:55 -
> > > @@ -1274,7 +1274,8 @@ vga_disable_bridge(struct pci_attach_arg
> > >  void
> > >  vga_get_uninterruptible(struct pci_dev *pdev, int rsrc)
> > >  {
> > > - KASSERT(pdev->pci->sc_bridgetag == NULL);
> > > + if (pdev->pci->sc_bridgetag != NULL)
> > > + return;
> > >   pci_enumerate_bus(pdev->pci, vga_disable_bridge, NULL);
> > >  }
> > >  
> > > @@ -1282,6 +1283,9 @@ void
> > >  vga_put(struct pci_dev *pdev, int rsrc)
> > >  {
> > >   pcireg_t bc;
> > > +
> > > + if (pdev->pci->sc_bridgetag != NULL)
> > > + return;
> > >  
> > >   if (!vga_bridge_disabled)
> > >   return;
> > > Index: sys/dev/pci/drm/i915/i915_pci.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_pci.c,v
> > > retrieving revision 1.15
> > > diff -u -p -r1.15 i915_pci.c
> > > --- sys/dev/pci/drm/i915/i915_pci.c   25 Jan 2023 01:51:59 -  
> > > 1.15
> > > +++ sys/dev/pci/drm/i915/i915_pci.c   3 Feb 2023 01:43:02 -
> > > @@ -1078,7 +1078,6 @@ static const struct intel_device_info dg
> > >   XE_LPD_FEATURES,
> > >   .__runtime.cpu_transcoder_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
> > >  BIT(TRANSCODER_C) | BIT(TRANSCODER_D),
> > > - .require_force_probe = 1,
> > >  };
> > >  
> > >  static const struct intel_device_info ats_m_info = {
> > > Index: sys/dev/pci/drm/i915/intel_memory_region.h
> > > ===
> > > RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_memory_region.h,v
> > > retrieving revision 1.3
> > > diff -u -p -r1.3 intel_memory_region.h
> > > --- sys/dev/pci/drm/i915/intel_memory_region.h1 Jan 2023 01:34:55 
> > > -   1.3
> > > +++ sys/dev/pci/drm/i915/intel_memory_region.h4 Feb 2023 00:59:23 
> > > -
> > > @@ -70,8 +70,13 @@ struct intel_memory_region {
> > >  
> > >   const struct intel_memory_region_ops *ops;
> > >  
> > > -#ifdef notyet
> > > +#ifdef __linux__
> > >   struct io_mapping iomap;
> > > +#else
> > > + struct vm_page *pgs;
> > > + struct agp_map *agph;
> > > + bus_space_handle_t bsh;
> > > + bus_space_tag_t bst;
> > >  #endif
> > >   struct resource region;
> > >  
> > > Index: sys/dev/pci/drm/i915/gem/i915_gem_lmem.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/pci/drm/i915/gem/i915_gem_lmem.c,v
> > > retrieving revision 1.4
> > > diff -u -p -r1.4 i915_ge

Re: Intel Arc / DG2

2023-02-12 Thread Thomas Frohwein
On Tue, Feb 07, 2023 at 08:51:40PM +1100, Jonathan Gray wrote:

[...] 
> > ...
> > i915_resize_lmem_bar: stub
> > panic: kernel diagnostic assertion "pdev->pci->sc_bridgetag == NULL" 
> > failed: file "/usr/src/sys/dev/pci/drm/drm_linux.c", line 1277
> > ...
> 
> The vga arbiter bits need to change.  There isn't an easy way to get
> a softc to avoid having state shared by multiple inteldrm instances.
> 
> perhaps they can be skipped for the moment?

Thanks, this leads to a uvm_fault now:

xehp_load_dss_mask: stub
xehp_load_dss_mask: stub
intel_slicemask_from_xehp_dssmask: stub
intel_slicemask_from_xehp_dssmask: stub
i915_resize_lmem_bar: stub
uvm_fault(0x825181f8, 0x10, 0, 1) -> e

screen photo at https://thfr.info/tmp/DG2-error-20230212.jpg

> 
> Index: sys/dev/pci/drm/drm_linux.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v
> retrieving revision 1.95
> diff -u -p -r1.95 drm_linux.c
> --- sys/dev/pci/drm/drm_linux.c   1 Jan 2023 01:34:34 -   1.95
> +++ sys/dev/pci/drm/drm_linux.c   7 Feb 2023 09:31:55 -
> @@ -1274,7 +1274,8 @@ vga_disable_bridge(struct pci_attach_arg
>  void
>  vga_get_uninterruptible(struct pci_dev *pdev, int rsrc)
>  {
> - KASSERT(pdev->pci->sc_bridgetag == NULL);
> + if (pdev->pci->sc_bridgetag != NULL)
> + return;
>   pci_enumerate_bus(pdev->pci, vga_disable_bridge, NULL);
>  }
>  
> @@ -1282,6 +1283,9 @@ void
>  vga_put(struct pci_dev *pdev, int rsrc)
>  {
>   pcireg_t bc;
> +
> + if (pdev->pci->sc_bridgetag != NULL)
> + return;
>  
>   if (!vga_bridge_disabled)
>   return;
> Index: sys/dev/pci/drm/i915/i915_pci.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_pci.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 i915_pci.c
> --- sys/dev/pci/drm/i915/i915_pci.c   25 Jan 2023 01:51:59 -  1.15
> +++ sys/dev/pci/drm/i915/i915_pci.c   3 Feb 2023 01:43:02 -
> @@ -1078,7 +1078,6 @@ static const struct intel_device_info dg
>   XE_LPD_FEATURES,
>   .__runtime.cpu_transcoder_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
>  BIT(TRANSCODER_C) | BIT(TRANSCODER_D),
> - .require_force_probe = 1,
>  };
>  
>  static const struct intel_device_info ats_m_info = {
> Index: sys/dev/pci/drm/i915/intel_memory_region.h
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_memory_region.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 intel_memory_region.h
> --- sys/dev/pci/drm/i915/intel_memory_region.h1 Jan 2023 01:34:55 
> -   1.3
> +++ sys/dev/pci/drm/i915/intel_memory_region.h4 Feb 2023 00:59:23 
> -
> @@ -70,8 +70,13 @@ struct intel_memory_region {
>  
>   const struct intel_memory_region_ops *ops;
>  
> -#ifdef notyet
> +#ifdef __linux__
>   struct io_mapping iomap;
> +#else
> + struct vm_page *pgs;
> + struct agp_map *agph;
> + bus_space_handle_t bsh;
> + bus_space_tag_t bst;
>  #endif
>   struct resource region;
>  
> Index: sys/dev/pci/drm/i915/gem/i915_gem_lmem.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/gem/i915_gem_lmem.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 i915_gem_lmem.c
> --- sys/dev/pci/drm/i915/gem/i915_gem_lmem.c  1 Jan 2023 01:34:56 -   
> 1.4
> +++ sys/dev/pci/drm/i915/gem/i915_gem_lmem.c  4 Feb 2023 00:58:16 -
> @@ -15,9 +15,6 @@ i915_gem_object_lmem_io_map(struct drm_i
>   unsigned long n,
>   unsigned long size)
>  {
> - STUB();
> - return NULL;
> -#ifdef notyet
>   resource_size_t offset;
>  
>   GEM_BUG_ON(!i915_gem_object_is_contiguous(obj));
> @@ -25,7 +22,11 @@ i915_gem_object_lmem_io_map(struct drm_i
>   offset = i915_gem_object_get_dma_address(obj, n);
>   offset -= obj->mm.region->region.start;
>  
> +#ifdef __linux__
>   return io_mapping_map_wc(>mm.region->iomap, offset, size);
> +#else
> + agp_map_atomic(obj->mm.region->agph, offset, >mm.region->bsh);
> + return bus_space_vaddr(obj->mm.region->bst, obj->mm.region->bsh);
>  #endif
>  }
>  
> Index: sys/dev/pci/drm/i915/gem/i915_gem_stolen.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/gem/i915_gem_stolen.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 i915_gem_stolen.c
> --- sys/dev/pci/drm/i915/gem/i915_gem_stolen.c4 Feb 2023 00:07:11 
> -   1.5
> +++ sys/dev/pci/drm/i915/gem/i915_gem_stolen.c4 Feb 2023 01:23:34 
> -
> @@ -774,18 +774,44 @@ static int init_stolen_lmem(struct intel
>   if (err)
>   return err;
>  
> - STUB();
> - return -ENOSYS;
> -#ifdef notyet
> +#ifdef __linux__
>   if (mem->io_size && 

Re: Intel Arc / DG2

2023-02-03 Thread Thomas Frohwein
On Sat, Feb 04, 2023 at 01:24:42PM +1100, Jonathan Gray wrote:

[...]

> 
> I've committed the i915_gem_stolen_lmem_setup() portion.
> 
> Another diff
> 
> likely some more iomap use will show up later
> 
> i915_gem_object_read_from_page_iomap()
> i915_gem_object_map_pfn()
> i915_gem_object_map()
> i915_ttm_io_mem_pfn()

I applied this; now the kernel panics when trying to switch the video
mode. Excerpt (handtyped; screenshot at [1]):

...
i915_resize_lmem_bar: stub
panic: kernel diagnostic assertion "pdev->pci->sc_bridgetag == NULL" failed: 
file "/usr/src/sys/dev/pci/drm/drm_linux.c", line 1277
...

[1] https://thfr.info/tmp/DG2-error-20230203.jpg

> 
> Index: sys/dev/pci/drm/i915/i915_pci.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_pci.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 i915_pci.c
> --- sys/dev/pci/drm/i915/i915_pci.c   25 Jan 2023 01:51:59 -  1.15
> +++ sys/dev/pci/drm/i915/i915_pci.c   3 Feb 2023 01:43:02 -
> @@ -1078,7 +1078,6 @@ static const struct intel_device_info dg
>   XE_LPD_FEATURES,
>   .__runtime.cpu_transcoder_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
>  BIT(TRANSCODER_C) | BIT(TRANSCODER_D),
> - .require_force_probe = 1,
>  };
>  
>  static const struct intel_device_info ats_m_info = {
> Index: sys/dev/pci/drm/i915/intel_memory_region.h
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_memory_region.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 intel_memory_region.h
> --- sys/dev/pci/drm/i915/intel_memory_region.h1 Jan 2023 01:34:55 
> -   1.3
> +++ sys/dev/pci/drm/i915/intel_memory_region.h4 Feb 2023 00:59:23 
> -
> @@ -70,8 +70,13 @@ struct intel_memory_region {
>  
>   const struct intel_memory_region_ops *ops;
>  
> -#ifdef notyet
> +#ifdef __linux__
>   struct io_mapping iomap;
> +#else
> + struct vm_page *pgs;
> + struct agp_map *agph;
> + bus_space_handle_t bsh;
> + bus_space_tag_t bst;
>  #endif
>   struct resource region;
>  
> Index: sys/dev/pci/drm/i915/gem/i915_gem_lmem.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/gem/i915_gem_lmem.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 i915_gem_lmem.c
> --- sys/dev/pci/drm/i915/gem/i915_gem_lmem.c  1 Jan 2023 01:34:56 -   
> 1.4
> +++ sys/dev/pci/drm/i915/gem/i915_gem_lmem.c  4 Feb 2023 00:58:16 -
> @@ -15,9 +15,6 @@ i915_gem_object_lmem_io_map(struct drm_i
>   unsigned long n,
>   unsigned long size)
>  {
> - STUB();
> - return NULL;
> -#ifdef notyet
>   resource_size_t offset;
>  
>   GEM_BUG_ON(!i915_gem_object_is_contiguous(obj));
> @@ -25,7 +22,11 @@ i915_gem_object_lmem_io_map(struct drm_i
>   offset = i915_gem_object_get_dma_address(obj, n);
>   offset -= obj->mm.region->region.start;
>  
> +#ifdef __linux__
>   return io_mapping_map_wc(>mm.region->iomap, offset, size);
> +#else
> + agp_map_atomic(obj->mm.region->agph, offset, >mm.region->bsh);
> + return bus_space_vaddr(obj->mm.region->bst, obj->mm.region->bsh);
>  #endif
>  }
>  
> Index: sys/dev/pci/drm/i915/gem/i915_gem_stolen.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/gem/i915_gem_stolen.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 i915_gem_stolen.c
> --- sys/dev/pci/drm/i915/gem/i915_gem_stolen.c4 Feb 2023 00:07:11 
> -   1.5
> +++ sys/dev/pci/drm/i915/gem/i915_gem_stolen.c4 Feb 2023 01:23:34 
> -
> @@ -774,18 +774,44 @@ static int init_stolen_lmem(struct intel
>   if (err)
>   return err;
>  
> - STUB();
> - return -ENOSYS;
> -#ifdef notyet
> +#ifdef __linux__
>   if (mem->io_size && !io_mapping_init_wc(>iomap,
>   mem->io_start,
>   mem->io_size)) {
>   err = -EIO;
>   goto err_cleanup;
>   }
> +#else
> + if (mem->io_size) {
> + int i;
> +
> + uvm_page_physload(atop(mem->io_start),
> + atop(mem->io_start + mem->io_size),
> + atop(mem->io_start),
> + atop(mem->io_start + mem->io_size),
> + PHYSLOAD_DEVICE);
> + /* array of vm pages that physload introduced. */
> + mem->pgs = PHYS_TO_VM_PAGE(mem->io_start);
> + KASSERT(mem->pgs != NULL);
> + /*
> +  * XXX mark all pages write combining so user mmaps get the
> +  * right bits. We really need a proper MI api for doing this,
> +  * but for now this allows us to use PAT where available.
> +  */
> + for (i = 0; i < atop(mem->io_size); i++)
> + 

Re: Intel Arc / DG2

2023-02-03 Thread Thomas Frohwein
On Fri, Feb 03, 2023 at 12:54:52PM +1100, Jonathan Gray wrote:
[...]
> > +xehp_load_dss_mask: stub
> > +xehp_load_dss_mask: stub
> > +intel_slicemask_from_xehp_dssmask: stub
> > +intel_slicemask_from_xehp_dssmask: stub
> > +i915_gem_stolen_lmem_setup: stub
> > +drm:pid0:intel_memory_regions_hw_probe *ERROR* [drm] *ERROR* Failed to 
> > setup region(-78) type=3
> > +Device initialization failed (-78)
> 
> thanks
> 
> xehp_load_dss_mask() needs:
> bitmap_from_arr32()
> 
> intel_slicemask_from_xehp_dssmask() needs:
> bitmap_intersects()
> bitmap_shift_right()
> 
> updated diff which unstubs i915_gem_stolen_lmem_setup()

thanks, this moves forward into the next stub:

-i915_gem_stolen_lmem_setup: stub
+init_stolen_lmem: stub

attaching the complete diff of the dmesg before and after your most recent diff.

> 
> Index: sys/dev/pci/drm/i915/i915_pci.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_pci.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 i915_pci.c
> --- sys/dev/pci/drm/i915/i915_pci.c   25 Jan 2023 01:51:59 -  1.15
> +++ sys/dev/pci/drm/i915/i915_pci.c   3 Feb 2023 01:43:02 -
> @@ -1078,7 +1078,6 @@ static const struct intel_device_info dg
>   XE_LPD_FEATURES,
>   .__runtime.cpu_transcoder_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
>  BIT(TRANSCODER_C) | BIT(TRANSCODER_D),
> - .require_force_probe = 1,
>  };
>  
>  static const struct intel_device_info ats_m_info = {
> Index: sys/dev/pci/drm/i915/gem/i915_gem_stolen.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/gem/i915_gem_stolen.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 i915_gem_stolen.c
> --- sys/dev/pci/drm/i915/gem/i915_gem_stolen.c1 Jan 2023 01:34:56 
> -   1.4
> +++ sys/dev/pci/drm/i915/gem/i915_gem_stolen.c3 Feb 2023 01:50:07 
> -
> @@ -813,26 +813,40 @@ struct intel_memory_region *
>  i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type,
>  u16 instance)
>  {
> - STUB();
> - return ERR_PTR(-ENOSYS);
> -#ifdef notyet
>   struct intel_uncore *uncore = >uncore;
>   struct pci_dev *pdev = i915->drm.pdev;
>   resource_size_t dsm_size, dsm_base, lmem_size;
>   struct intel_memory_region *mem;
>   resource_size_t io_start, io_size;
>   resource_size_t min_page_size;
> + pcireg_t mtype;
> + bus_addr_t lmem_start;
> + bus_size_t lmem_len;
> + int ret;
>  
>   if (WARN_ON_ONCE(instance))
>   return ERR_PTR(-ENODEV);
>  
> +#ifdef __linux__
>   if (!i915_pci_resource_valid(pdev, GEN12_LMEM_BAR))
>   return ERR_PTR(-ENXIO);
> +#else
> + mtype = pci_mapreg_type(i915->pc, i915->tag,
> + 0x10 + (4 * GEN12_LMEM_BAR));
> + ret = pci_mapreg_info(i915->pc, i915->tag,
> + 0x10 + (4 * GEN12_LMEM_BAR), mtype, _start, _len, NULL);
> + if (ret != 0)
> + return ERR_PTR(-ENXIO);
> +#endif
>  
>   /* Use DSM base address instead for stolen memory */
>   dsm_base = intel_uncore_read64(uncore, GEN12_DSMBASE);
>   if (IS_DG1(uncore->i915)) {
> +#ifdef __linux__
>   lmem_size = pci_resource_len(pdev, GEN12_LMEM_BAR);
> +#else
> + lmem_size = lmem_len;
> +#endif
>   if (WARN_ON(lmem_size < dsm_base))
>   return ERR_PTR(-ENODEV);
>   } else {
> @@ -844,6 +858,7 @@ i915_gem_stolen_lmem_setup(struct drm_i9
>   }
>  
>   dsm_size = lmem_size - dsm_base;
> +#ifdef __linux__
>   if (pci_resource_len(pdev, GEN12_LMEM_BAR) < lmem_size) {
>   io_start = 0;
>   io_size = 0;
> @@ -851,6 +866,15 @@ i915_gem_stolen_lmem_setup(struct drm_i9
>   io_start = pci_resource_start(pdev, GEN12_LMEM_BAR) + dsm_base;
>   io_size = dsm_size;
>   }
> +#else
> + if (lmem_len < lmem_size) {
> + io_start = 0;
> + io_size = 0;
> + } else {
> + io_start = lmem_start + dsm_base;
> + io_size = dsm_size;
> + }
> +#endif
>  
>   min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K :
>   I915_GTT_PAGE_SIZE_4K;
> @@ -878,7 +902,6 @@ i915_gem_stolen_lmem_setup(struct drm_i9
>   mem->private = true;
>  
>   return mem;
> -#endif
>  }
>  
>  struct intel_memory_region*
--- dmesg-20230202-postDG2patch.txt Thu Feb  2 15:49:28 2023
+++ dmesg-20230202-post-secondDG2diff.txt   Fri Feb  3 14:23:46 2023
@@ -1,7 +1,7 @@
-OpenBSD 7.2-current (GENERIC.MP) #0: Thu Feb  2 15:38:20 EST 2023
+OpenBSD 7.2-current (GENERIC.MP) #1: Fri Feb  3 14:18:10 EST 2023
 t...@thfr-hdd320g.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC.MP
 real mem = 34226315264 (32640MB)
-avail mem = 33169633280 (31633MB)
+avail mem = 33169629184 (31633MB)
 random: good seed from bootblocks
 mpath0 at 

Re: Intel Arc / DG2

2023-02-02 Thread Thomas Frohwein
On Mon, Jan 30, 2023 at 09:32:08PM +1100, Jonathan Gray wrote:
> The current generation of Intel Arc branded graphics cards are part of
> what drm and Mesa refers to as DG2.
> 
> https://ark.intel.com/content/www/us/en/ark/products/codename/226095/products-formerly-alchemist.html
> 
> In -current we now have Mesa 22.3 which has support for DG2.
> There is support in inteldrm but DG2 is not matched by default.
> 
> The following diff matches it by default, as do versions of linux
> after 6.1.
> d2eae8e98d59 ("drm/i915/dg2: Drop force_probe requirement")
> 
> I'm not sure if all the intel discrete gpu paths work.  I don't have
> hardware to test.

I have tested this with Intel Arc A750. It recognizes the device, but
fails to initialize it. diff of dmesg before vs. after your diff attached.

> 
> Index: sys/dev/pci/drm/i915/i915_pci.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_pci.c,v
> retrieving revision 1.15
> diff -u -p -U5 -r1.15 i915_pci.c
> --- sys/dev/pci/drm/i915/i915_pci.c   25 Jan 2023 01:51:59 -  1.15
> +++ sys/dev/pci/drm/i915/i915_pci.c   29 Jan 2023 23:15:09 -
> @@ -1076,11 +1076,10 @@ static const struct intel_device_info xe
>  static const struct intel_device_info dg2_info = {
>   DG2_FEATURES,
>   XE_LPD_FEATURES,
>   .__runtime.cpu_transcoder_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
>  BIT(TRANSCODER_C) | BIT(TRANSCODER_D),
> - .require_force_probe = 1,
>  };
>  
>  static const struct intel_device_info ats_m_info = {
>   DG2_FEATURES,
>   NO_DISPLAY,
> 
--- dmesg-20230202-preDG2patch.txt  Thu Feb  2 15:50:30 2023
+++ dmesg-20230202-postDG2patch.txt Thu Feb  2 15:50:46 2023
@@ -1,7 +1,7 @@
-OpenBSD 7.2-current (GENERIC.MP) #1015: Thu Feb  2 06:25:57 MST 2023
-dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
+OpenBSD 7.2-current (GENERIC.MP) #0: Thu Feb  2 15:38:20 EST 2023
+t...@thfr-hdd320g.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC.MP
 real mem = 34226315264 (32640MB)
-avail mem = 33169625088 (31633MB)
+avail mem = 33169633280 (31633MB)
 random: good seed from bootblocks
 mpath0 at root
 scsibus0 at mpath0: 256 targets
@@ -29,22 +29,22 @@
 cpu0: apic clock running at 24MHz
 cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE
 cpu1 at mainbus0: apid 2 (application processor)
-cpu1: Intel(R) Core(TM) i7-10700 CPU @ 2.90GHz, 4589.31 MHz, 06-a5-05
+cpu1: Intel(R) Core(TM) i7-10700 CPU @ 2.90GHz, 4589.32 MHz, 06-a5-05
 cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,PKU,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
 cpu1: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 256KB 64b/line 
4-way L2 cache, 16MB 64b/line 16-way L3 cache
 cpu1: smt 0, core 1, package 0
 cpu2 at mainbus0: apid 4 (application processor)
-cpu2: Intel(R) Core(TM) i7-10700 CPU @ 2.90GHz, 4589.31 MHz, 06-a5-05
+cpu2: Intel(R) Core(TM) i7-10700 CPU @ 2.90GHz, 4589.33 MHz, 06-a5-05
 cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,PKU,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
 cpu2: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 256KB 64b/line 
4-way L2 cache, 16MB 64b/line 16-way L3 cache
 cpu2: smt 0, core 2, package 0
 cpu3 at mainbus0: apid 6 (application processor)
-cpu3: Intel(R) Core(TM) i7-10700 CPU @ 2.90GHz, 4589.31 MHz, 06-a5-05
+cpu3: Intel(R) Core(TM) i7-10700 CPU @ 2.90GHz, 4589.32 MHz, 06-a5-05
 cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,PKU,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
 cpu3: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 256KB 64b/line 
4-way L2 cache, 16MB 64b/line 16-way L3 cache
 cpu3: smt 0, core 3, package 0
 cpu4 at mainbus0: apid 8 

Re: azalia module fix for 7.2-stable and some Intel 500 HDA Chips

2022-11-04 Thread Thomas Frohwein
On Fri, Nov 04, 2022 at 04:22:55PM +0100, Mark Kettenis wrote:
> > Date: Sat, 5 Nov 2022 02:06:11 +1100
> > From: Jonathan Gray 
> > 
> > On Fri, Nov 04, 2022 at 09:10:52AM -0500, John Browning wrote:
> > > Hi,
> > > I noticed I did not have sound on my new thinkpad which has a newer
> > > Intel 500 HDA chipset.
> > > 
> > > bsd$ doas pcidump -vvv 0:31:3
> > >  0:31:3: Intel 500 Series HD Audio
> > > 0x: Vendor ID: 8086, Product ID: 43c8
> > > 0x0004: Command: 0006, Status: 0010
> > > 0x0008:Class: 04 Multimedia, Subclass: 01 Audio,
> > > Interface: 00, Revision: 11
> > > 0x000c: BIST: 00, Header Type: 00, Latency Timer: 00,
> > > Cache Line Size: 00
> > > 0x0010: BAR mem 64bit addr: 0x00603d1c/0x4000
> > > 0x0018: BAR empty ()
> > > 0x001c: BAR empty ()
> > > 0x0020: BAR mem 64bit addr: 0x00603d00/0x0010
> > > 0x0028: Cardbus CIS: 
> > > 0x002c: Subsystem Vendor ID: 17aa Product ID: 22e4
> > > 0x0030: Expansion ROM Base Address: 
> > > 0x0038: 
> > > 0x003c: Interrupt Pin: 01 Line: ff Min Gnt: 00 Max Lat: 00
> > > 0x0050: Capability 0x01: Power Management
> > > State: D0
> > > 0x0080: Capability 0x09: Vendor Specific
> > > 0x0060: Capability 0x05: Message Signalled Interrupts (MSI)
> > > Enabled: yes
> > > 
> > > 
> > > Looking at src/sys/dev/pci/azalia.c I noticed making the change in the
> > > below diff corrects the issue with a new kernel build.  It seems the
> > > pci subsystem match inclusion may not be needed, but hopefully someone
> > > smarter than I can figure that out (or help me figure that out).
> > 
> > When the subclass is not hd-audio, we match with azalia_pci_devices[].
> > 
> > Which model thinkpad is this?
> 
> ok kettenis@

ok thfr@

(came up with the same diff based on a preceding discussion with John
Browning on #openbsd-gaming IRC on libera.chat; also with input from
brynet@)

> > Index: sys/dev/pci/azalia.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/azalia.c,v
> > retrieving revision 1.280
> > diff -u -p -r1.280 azalia.c
> > --- sys/dev/pci/azalia.c26 Oct 2022 20:19:08 -  1.280
> > +++ sys/dev/pci/azalia.c4 Nov 2022 14:10:59 -
> > @@ -488,6 +488,7 @@ const struct pci_matchid azalia_pci_devi
> > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_300SERIES_U_HDA },
> > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_400SERIES_CAVS },
> > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_400SERIES_LP_HDA },
> > +   { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_500SERIES_HDA },
> > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_500SERIES_LP_HDA },
> > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_600SERIES_LP_HDA },
> > { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_GLK_HDA },
> > 
> > 
> 



Re: XBox One gamecontroller support

2022-03-20 Thread Thomas Frohwein
On Sat, Mar 19, 2022 at 09:15:23PM +0100, Stefan Sperling wrote:

[...]

> > +   /* XBox One controller initialization */
> 
> Shouldn't this initialization code be under #ifndef SMALL_KERNEL,
> like the rest of the code your patch is adding to this file?
> 
> > +   if (sc->sc_flags & UHIDEV_F_XB1) {
> > +   uint8_t init_data[] = { 0x05, 0x20, 0x00, 0x01, 0x00 };
> > +   int init_data_len = sizeof(init_data);
> 
> I would use 'size_t init_data_len' instead of 'int init_data_len'.
> Largely a matter of taste, but this way everything stays unsigned.
> usbd_setup_xfer() expects an unsigned int.

[...]

> Both of the above suggestions as a diff:

[...]

I updated the diff for the controller with your diff. Below is the
complete diff for all the files involved. I tested it again with my
controller and sdl-jstest (in ports); it continues to work as intended.

ok?

Index: uhid_rdesc.h
===
RCS file: /cvs/src/sys/dev/usb/uhid_rdesc.h,v
retrieving revision 1.1
diff -u -p -r1.1 uhid_rdesc.h
--- uhid_rdesc.h25 Oct 2013 03:09:59 -  1.1
+++ uhid_rdesc.h20 Mar 2022 22:41:04 -
@@ -273,3 +273,93 @@ static const uByte uhid_xb360gp_report_d
 0x81, 0x01,/*  INPUT (Constant)*/
 0xc0,  /* END COLLECTION   */
 };
+
+static const uByte uhid_xbonegp_report_descr[] = {
+0x05, 0x01,/* USAGE PAGE (Generic Desktop) 
*/
+0x09, 0x05,/* USAGE (Gamepad)  
*/
+0xa1, 0x01,/* COLLECTION (Application) 
*/
+/* Button packet */
+0xa1, 0x00,/*  COLLECTION (Physical)   
*/
+0x85, 0x20,/*   REPORT ID (0x20)   
*/
+/* Skip unknown field and counter */
+0x09, 0x00,/*   USAGE (Undefined)  
*/
+0x75, 0x08,/*   REPORT SIZE (8)
*/
+0x95, 0x02,/*   REPORT COUNT (2)   
*/
+0x81, 0x03,/*   INPUT (Constant, Variable, 
Absolute)   */
+/* Payload size */
+0x09, 0x3b,/*   USAGE (Byte Count) 
*/
+0x95, 0x01,/*   REPORT COUNT (1)   
*/
+0x81, 0x02,/*   INPUT (Data, Variable, Absolute)   
*/
+/* 16 Buttons: 1-2=Unknown, 3=Start, 4=Back, 5-8=ABXY,
+ *9-12=D-Pad(Up,Dn,Lt,Rt), 13=LB, 14=RB, 15=LS, 16=RS
+ */
+/* Skip the first 2 as they are not used */
+0x75, 0x01,/*   REPORT SIZE (1)
*/
+0x95, 0x02,/*   REPORT COUNT (2)   
*/
+0x81, 0x01,/*   INPUT (Constant)   
*/
+/* Assign buttons Start(7), Back(8), ABXY(1-4) */
+0x15, 0x00,/*   LOGICAL MINIMUM (0)
*/
+0x25, 0x01,/*   LOGICAL MAXIMUM (1)
*/
+0x95, 0x06,/*   REPORT COUNT (6)   
*/
+0x05, 0x09,/*   USAGE PAGE (Button)
*/
+0x09, 0x08,/*   USAGE (Button 8)   
*/
+0x09, 0x07,/*   USAGE (Button 7)   
*/
+0x09, 0x01,/*   USAGE (Button 1)   
*/
+0x09, 0x02,/*   USAGE (Button 2)   
*/
+0x09, 0x03,/*   USAGE (Button 3)   
*/
+0x09, 0x04,/*   USAGE (Button 4)   
*/
+0x81, 0x02,/*   INPUT (Data, Variable, Absolute)   
*/
+/* D-Pad */
+0x05, 0x01,/*   USAGE PAGE (Generic Desktop)   
*/
+0x09, 0x01,/*   USAGE (Pointer)
*/
+0xa1, 0x00,/*   COLLECTION (Physical)  
*/
+0x75, 0x01,/*REPORT SIZE (1)   
*/
+0x15, 0x00,/*LOGICAL MINIMUM (0)   
*/
+0x25, 0x01,/*LOGICAL MAXIMUM (1)   
*/
+0x95, 0x04,/*REPORT COUNT (4)  
*/
+0x05, 0x01,/*USAGE PAGE (Generic Desktop)  
*/
+0x09, 0x90,   

Re: XBox One gamecontroller support

2022-03-17 Thread Thomas Frohwein
On Thu, Mar 17, 2022 at 10:58:21PM +0100, Solene Rapenne wrote:

[...]

> > 
> 
> ping
> 
> this diff is still applying to the kernel and allows to use a popular
> and affordable game controller

The diff was written fairly conservatively based on pre-existing code
and NetBSD's solution. It would be nice if someone with more experience
with the USB fundamentals would comment, but maybe it could just go in
in its current form.



Re: Suspend/Resume on modern Intel laptop platforms

2021-09-20 Thread Thomas Frohwein
On Thu, Sep 16, 2021 at 12:11:24AM +0200, Mark Kettenis wrote:
> > Date: Wed, 15 Sep 2021 17:29:39 +0200 (CEST)
> > From: Mark Kettenis 
> > 
> > The diff below is a preliminary diff to fix a suspend/resume issue on
> > recent Thinkpads.  This needs testing on a wider range of laptops to
> > make sure it doesn't break things.  The diff also puts some
> > information in dmesg that will help me improve things in the future.
> > 
> > So, if you have a laptop where pchgpio(4) attaches *and* supports S3
> > suspen/resume, please apply this diff, do a suspend/resume cycle and
> > send me a dmesg collected after that suspend/resume cycle.
> 
> This diff is now in snapshots, so instead of applying the diff and
> build your own kernel, you can just upgrade to the latest snapshot.

My ASUS Expertbook B9400 has pchgpio(4) attach, so here a dmesg after
a suspend and resume. I don't notice any changes - resuming after a few
minutes of sleep (zzz) or stand-by (apm -S) works, but if I leave the
laptop alone for longer, it doesn't wake anymore. None of opening lid,
pressing keys, or pressing power button brings it back then. That was
the same before and after the update to snapshot from 16 Sep.

(I have machdep.pwraction=0 in /etc/sysctl.conf, but doubt that would
be related...)

OpenBSD 7.0 (GENERIC.MP) #219: Thu Sep 16 00:17:22 MDT 2021
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 16825262080 (16045MB)
avail mem = 16299315200 (15544MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.3 @ 0x5a4b2000 (95 entries)
bios0: vendor ASUSTeK COMPUTER INC. version "B9400CEA.304" date 06/02/2021
bios0: ASUSTeK COMPUTER INC. ASUS EXPERTBOOK B9400CEA_B9450CEA
acpi0 at bios0: ACPI 6.2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP MCFG SSDT FIDT ECDT MSDM SSDT SSDT SSDT SSDT HPET APIC 
SSDT NHLT LPIT SSDT DBGP DBG2 SSDT DMAR SSDT SSDT BGRT TPM2 PTDT WSMT FPDT
acpi0: wakeup devices PEGP(S4) PEGP(S4) PEGP(S4) PEG0(S4) PEGP(S4) GLAN(S4) 
XHCI(S3) XDCI(S4) HDAS(S4) CNVW(S4) TXHC(S3) TDM0(S4) TRP0(S3) PXSX(S4) 
TRP1(S3) PXSX(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimcfg0 at acpi0
acpimcfg0: addr 0xc000, bus 0-255
acpiec0 at acpi0
acpihpet0 at acpi0: 1920 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 4689.19 MHz, 06-8c-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,AVX512F,AVX512DQ,RDSEED,ADX,SMAP,AVX512IFMA,CLFLUSHOPT,CLWB,PT,AVX512CD,SHA,AVX512BW,AVX512VL,AVX512VBMI,UMIP,PKU,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu0: 256KB 64b/line disabled L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 38MHz
cpu0: mwait min=64, max=64, C-substates=0.2.0.1.2.1.1.1, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 4689.19 MHz, 06-8c-01
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,AVX512F,AVX512DQ,RDSEED,ADX,SMAP,AVX512IFMA,CLFLUSHOPT,CLWB,PT,AVX512CD,SHA,AVX512BW,AVX512VL,AVX512VBMI,UMIP,PKU,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu1: 256KB 64b/line disabled L2 cache
cpu1: disabling user TSC (skew=-911753242)
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
cpu2: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 4090.57 MHz, 06-8c-01
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,AVX512F,AVX512DQ,RDSEED,ADX,SMAP,AVX512IFMA,CLFLUSHOPT,CLWB,PT,AVX512CD,SHA,AVX512BW,AVX512VL,AVX512VBMI,UMIP,PKU,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu2: 256KB 64b/line disabled L2 cache
cpu2: disabling user TSC (skew=-911753252)
cpu2: smt 0, core 2, package 0
cpu3 at mainbus0: apid 6 (application processor)
cpu3: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 

Re: ihidev: fix data request size

2021-08-23 Thread Thomas Frohwein
On Mon, Aug 23, 2021 at 08:57:59AM -0500, joshua stein wrote:
> On Sun, 22 Aug 2021 at 22:37:51 -0600, Thomas Frohwein wrote:
[...]
> > I thought I had the same problem on my new Asus Expertbook B9400CEA.
> > During boot, while reordering libraries and later it would print:
> > 
> > dwiic2: timed out reading remaining 16
> > 
> > This is accompanied by generalized slowness of the system - boot is
> > noticeably slow, xenodm takes about 20 seconds to get to the login
> > window, and glxgears(1) slows down visibly with any mouse movement.
> 
> That is because dwiic has to use polling while being invoked by 
> ihidev's interrupt handler or else it panics in tsleep.  It has to 
> wait longer than normal using delay(), so it blocks other kernel 
> processing.
> 
> As for why the timeouts are happening in the first place, it could 
> be a similar issue with the max report size being bogus.  Does this 
> diff change anything?

Thanks for taking a look. The only thing that seems to change is that
the message is now:

dwiic2: timed out reading remaining 4

... not 16 anymore.

dmesg from a kernel built with this patch attached.

> 
> 
> diff --git sys/dev/i2c/ihidev.c sys/dev/i2c/ihidev.c
> index 92778c679ad..0baba616329 100644
> --- sys/dev/i2c/ihidev.c
> +++ sys/dev/i2c/ihidev.c
> @@ -152,7 +152,7 @@ ihidev_attach(struct device *parent, struct device *self, 
> void *aux)
>   }
>  
>   /* find largest report size and allocate memory for input buffer */
> - sc->sc_isize = letoh16(sc->hid_desc.wMaxInputLength);
> + sc->sc_isize = 4;
>   for (repid = 0; repid < sc->sc_nrepid; repid++) {
>   repsz = hid_report_size(sc->sc_report, sc->sc_reportlen,
>   hid_input, repid);
> @@ -643,7 +643,7 @@ ihidev_intr(void *arg)
>  
>   iic_acquire_bus(sc->sc_tag, I2C_F_POLL);
>   res = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, sc->sc_addr, NULL, 0,
> - sc->sc_ibuf, letoh16(sc->hid_desc.wMaxInputLength), I2C_F_POLL);
> + sc->sc_ibuf, sc->sc_isize, I2C_F_POLL);
>   iic_release_bus(sc->sc_tag, I2C_F_POLL);
>  
>   /*
> 
OpenBSD 7.0-beta (GENERIC.MP) #1: Mon Aug 23 08:20:12 MDT 2021
t...@thfr.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 16825262080 (16045MB)
avail mem = 16299372544 (15544MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.3 @ 0x5a4b2000 (95 entries)
bios0: vendor ASUSTeK COMPUTER INC. version "B9400CEA.304" date 06/02/2021
bios0: ASUSTeK COMPUTER INC. ASUS EXPERTBOOK B9400CEA_B9450CEA
acpi0 at bios0: ACPI 6.2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP MCFG SSDT FIDT ECDT MSDM SSDT SSDT SSDT SSDT HPET APIC 
SSDT NHLT LPIT SSDT DBGP DBG2 SSDT DMAR SSDT SSDT TPM2 BGRT PTDT WSMT FPDT
acpi0: wakeup devices PEGP(S4) PEGP(S4) PEGP(S4) PEG0(S4) PEGP(S4) GLAN(S4) 
XHCI(S3) XDCI(S4) HDAS(S4) CNVW(S4) TXHC(S3) TDM0(S4) TRP0(S3) PXSX(S4) 
TRP1(S3) PXSX(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimcfg0 at acpi0
acpimcfg0: addr 0xc000, bus 0-255
acpiec0 at acpi0
acpihpet0 at acpi0: 1920 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 68555.29 MHz, 06-8c-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,AVX512F,AVX512DQ,RDSEED,ADX,SMAP,AVX512IFMA,CLFLUSHOPT,CLWB,PT,AVX512CD,SHA,AVX512BW,AVX512VL,AVX512VBMI,UMIP,PKU,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu0: 256KB 64b/line disabled L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 38MHz
cpu0: mwait min=64, max=64, C-substates=0.2.0.1.2.1.1.1, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 4689.20 MHz, 06-8c-01
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,AVX512F,AVX512DQ,RDSEED,ADX,SMAP,AVX512IFMA,CLFLUSHOPT,CLWB,PT,AVX512CD,SHA,AVX512BW,AVX512VL,AVX512VBMI,UMIP,PKU,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XG

Re: ihidev: fix data request size

2021-08-22 Thread Thomas Frohwein
On Sun, Aug 22, 2021 at 09:50:15PM -0500, joshua stein wrote:
> On a particular laptop with a touchpad behind ihidev, dwiic would 
> report a timeout every time it had to fetch touch data:
> 
> dwiic0: timed out reading remaining 2
> 
> On re-reading the i2c HID spec, the size supplied by wMaxInputLength 
> is already supposed to account for the size and report id bytes so 
> we shouldn't be adding them after the fact.  Otherwise ihidev would 
> ask for more data than can be available and, on this laptop anyway, 
> dwiic would have to wait for this transaction to timeout and fail.
> 
> This fix matches how the Linux i2c-hid driver operates.  I've tested 
> this on 3 laptops with touchpads and touchscreens and it doesn't 
> cause any regressions here while fixing the touchpad on one of them.  
> I'd appreciate tests on other laptops to make sure it doesn't break 
> anything and perhaps fixes your issue if you've also seen constant 
> dwiic timeouts.
> 

I thought I had the same problem on my new Asus Expertbook B9400CEA.
During boot, while reordering libraries and later it would print:

dwiic2: timed out reading remaining 16

This is accompanied by generalized slowness of the system - boot is
noticeably slow, xenodm takes about 20 seconds to get to the login
window, and glxgears(1) slows down visibly with any mouse movement.

Oddly enough these issues don't occur if I boot into bsd.rd before
booting regularly.

I built a kernel with your diff, but the problem persists. dmesg is
attached. If I disable the internal pointing device in the bios, the
apparent touchpad (via ihidev0 in the dmesg) is not present anymore,
but the problems still persist including the "timed out..." message.

No regression observed though.

> 
> 
> Index: sys/dev/i2c/ihidev.c
> ===
> RCS file: /cvs/src/sys/dev/i2c/ihidev.c,v
> retrieving revision 1.23
> diff -u -p -u -p -r1.23 ihidev.c
> --- sys/dev/i2c/ihidev.c  9 Jul 2020 21:01:55 -   1.23
> +++ sys/dev/i2c/ihidev.c  23 Aug 2021 02:38:21 -
> @@ -106,7 +106,6 @@ ihidev_attach(struct device *parent, str
>   struct device *dev;
>   int repid, repsz;
>   int repsizes[256];
> - int isize;
>  
>   sc->sc_tag = ia->ia_tag;
>   sc->sc_addr = ia->ia_addr;
> @@ -158,12 +157,8 @@ ihidev_attach(struct device *parent, str
>   repsz = hid_report_size(sc->sc_report, sc->sc_reportlen,
>   hid_input, repid);
>   repsizes[repid] = repsz;
> -
> - isize = repsz + 2; /* two bytes for the length */
> - isize += (sc->sc_nrepid != 1); /* one byte for the report ID */
> - if (isize > sc->sc_isize)
> - sc->sc_isize = isize;
> -
> + if (repsz > sc->sc_isize)
> + sc->sc_isize = repsz;
>   if (repsz != 0)
>   DPRINTF(("%s: repid %d size %d\n", sc->sc_dev.dv_xname,
>   repid, repsz));
> @@ -648,7 +643,7 @@ ihidev_intr(void *arg)
>  
>   iic_acquire_bus(sc->sc_tag, I2C_F_POLL);
>   res = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, sc->sc_addr, NULL, 0,
> - sc->sc_ibuf, sc->sc_isize, I2C_F_POLL);
> + sc->sc_ibuf, letoh16(sc->hid_desc.wMaxInputLength), I2C_F_POLL);
>   iic_release_bus(sc->sc_tag, I2C_F_POLL);
>  
>   /*
> 
OpenBSD 7.0-beta (GENERIC.MP) #0: Sun Aug 22 21:55:30 MDT 2021
t...@thfr.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 16825262080 (16045MB)
avail mem = 16299376640 (15544MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.3 @ 0x5a4b2000 (95 entries)
bios0: vendor ASUSTeK COMPUTER INC. version "B9400CEA.304" date 06/02/2021
bios0: ASUSTeK COMPUTER INC. ASUS EXPERTBOOK B9400CEA_B9450CEA
acpi0 at bios0: ACPI 6.2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP MCFG SSDT FIDT ECDT MSDM SSDT SSDT SSDT SSDT HPET APIC 
SSDT NHLT LPIT SSDT DBGP DBG2 SSDT DMAR SSDT SSDT TPM2 BGRT PTDT WSMT FPDT
acpi0: wakeup devices PEGP(S4) PEGP(S4) PEGP(S4) PEG0(S4) PEGP(S4) GLAN(S4) 
XHCI(S3) XDCI(S4) HDAS(S4) CNVW(S4) TXHC(S3) TDM0(S4) TRP0(S3) PXSX(S4) 
TRP1(S3) PXSX(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimcfg0 at acpi0
acpimcfg0: addr 0xc000, bus 0-255
acpiec0 at acpi0
acpihpet0 at acpi0: 1920 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz, 68549.21 MHz, 06-8c-01
cpu0: 

Re: XBox One gamecontroller support

2021-02-22 Thread Thomas Frohwein
On Fri, Jan 15, 2021 at 06:32:04AM -0700, Thomas Frohwein wrote:
> On Sat, Jan 09, 2021 at 10:50:35AM -0700, Thomas Frohwein wrote:
> > Hi,
> > 
> > This diff adds support for the XBox One gamecontroller in a similar way
> > to what we have for the (older) XBox 360 controller [1][2]. This diff
> > is based on the pertinent code in NetBSD's uhidev.c.
> > 
> > Similarities include that the device doesn't provide a report
> > descriptor, so this diff adds one to uhid_rdesc.h.
> > 
> > The biggest difference (that held this up for a while) is that the
> > controller needs an init byte sequence to "wake up."
> > 
> > The original report descriptor from NetBSD just maps the buttons and
> > axes in the order that they appear in the report. I modified the report
> > descriptor to assign the buttons/axes in the most logical order for
> > compatibility, which results in the same layout that we already have
> > for the XBox 360 controller.
> > 
> > The addition of the member sc_flags to struct uhidev_softc follows the
> > NetBSD example.
> > 
> > I have tested this with usbhidctl(1), sdl2-jstest (from sdl-jstest
> > package), and a couple of SDL2 games (Owlboy, Cryptark).
> > 
> > If you have an XBox One controller, the easiest way to test is with:
> > 
> > $ usbhidctl -f /dev/uhidX -l
> > 
> > Make sure to pick the right uhid device and that you have read
> > permissions for it.
> > 
> > Of course, this works only with the controller connected via USB, and
> > doesn't magically add wireless support.
> > 
> > If you test actual SDL2 applications, the button layout will likely
> > default to an odd configuration. I am simultaneously preparing a diff
> > for sdl2-2.0.14p0 that improves recognition and automatic mapping and
> > solves any issues with XBox One default button layout.
> 
> I have an update to this diff. 2 testers used a more recent XBox One
> controller model (model number 1708 in both cases) and ran into
> problems with the original diff. Below is a new diff that uses a longer
> 5-byte init sequence taken from Linux' xpad.c [1], compared to the
> 2-byte sequence from NetBSD's uhidev.c that I offered in the initial
> diff. This fixed the issue for the 2 testers.
> 
> My own XBox One controller is model number 1537 and still works with
> this diff.
> 
> [1] https://github.com/paroj/xpad/blob/master/xpad.c#L479

solene@ has tested this and is ok with it, but I was hoping to get an
opinion from someone with more mileage with the USB internals.

The items of note to direct anyone wanting to give a quick opinion
inlined below.

> 
> Index: uhid_rdesc.h
> ===
> RCS file: /cvs/src/sys/dev/usb/uhid_rdesc.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 uhid_rdesc.h
> --- uhid_rdesc.h  25 Oct 2013 03:09:59 -  1.1
> +++ uhid_rdesc.h  9 Jan 2021 15:11:30 -
> @@ -272,4 +272,94 @@ static const uByte uhid_xb360gp_report_d
>  0x95, 0x01,  /*  REPORT COUNT (1)*/
>  0x81, 0x01,  /*  INPUT (Constant)*/
>  0xc0,/* END COLLECTION   */
> +};
> +
> +static const uByte uhid_xbonegp_report_descr[] = {
> +0x05, 0x01,  /* USAGE PAGE (Generic Desktop) 
> */
> +0x09, 0x05,  /* USAGE (Gamepad)  
> */
> +0xa1, 0x01,  /* COLLECTION (Application) 
> */
> +/* Button packet */
> +0xa1, 0x00,  /*  COLLECTION (Physical)   
> */
> +0x85, 0x20,  /*   REPORT ID (0x20)   
> */
> +/* Skip unknown field and counter */
> +0x09, 0x00,  /*   USAGE (Undefined)  
> */
> +0x75, 0x08,  /*   REPORT SIZE (8)
> */
> +0x95, 0x02,  /*   REPORT COUNT (2)   
> */
> +0x81, 0x03,  /*   INPUT (Constant, Variable, 
> Absolute)   */
> +/* Payload size */
> +0x09, 0x3b,  /*   USAGE (Byte Count) 
> */
> +0x95, 0x01,  /*   REPORT COUNT (1)   
> */
> +0x81, 0x02,  /*   INPUT (Data, Variable, Absolute)   
> */
> +/* 16 Buttons: 1-2=Unknown, 3=Start, 4=Back, 5-8=ABXY,
> + *  9-12=D-Pad(Up,Dn,Lt,Rt), 13=LB, 14=RB, 

Re: New ujoy(4) device for USB gamecontrollers

2021-01-22 Thread Thomas Frohwein
===
RCS file: /cvs/src/share/man/man4/uhidev.4,v
retrieving revision 1.12
diff -u -p -r1.12 uhidev.4
--- share/man/man4/uhidev.4 21 Aug 2020 19:02:46 -  1.12
+++ share/man/man4/uhidev.4 22 Jan 2021 20:05:06 -
@@ -40,6 +40,7 @@
 .Cd "ucycom*  at uhidev?"
 .Cd "ugold*   at uhidev?"
 .Cd "uhid*at uhidev?"
+.Cd "ujoy*at uhidev?"
 .Cd "ukbd*at uhidev?"
 .Cd "ums* at uhidev?"
 .Cd "umstc*   at uhidev?"
@@ -73,6 +74,7 @@ only dispatches data to them based on th
 .Xr ucycom 4 ,
 .Xr ugold 4 ,
 .Xr uhid 4 ,
+.Xr ujoy 4 ,
 .Xr ukbd 4 ,
 .Xr ums 4 ,
 .Xr umstc 4 ,
Index: share/man/man4/ujoy.4
===
RCS file: share/man/man4/ujoy.4
diff -N share/man/man4/ujoy.4
--- /dev/null   1 Jan 1970 00:00:00 -
+++ share/man/man4/ujoy.4   22 Jan 2021 20:05:06 -
@@ -0,0 +1,53 @@
+.\" $OpenBSD: ujoy.4,v 1.4 2020/08/21 19:02:46 mglocker Exp $
+.\"
+.\" Copyright (c) 2020 Thomas Frohwein 
+.\" Copyright (c) 2020 Bryan Steele
+.\"
+.\" Permission to use, copy, modify, and distribute this software for any
+.\" purpose with or without fee is hereby granted, provided that the above
+.\" copyright notice and this permission notice appear in all copies.
+.\"
+.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+.\"
+.Dd $Mdocdate: December 27 2020 $
+.Dt UJOY 4
+.Os
+.Sh NAME
+.Nm ujoy
+.Nd USB joystick/gamecontroller
+.Sh SYNOPSIS
+.Cd "ujoy* at uhidev?"
+.Sh DESCRIPTION
+The
+.Nm
+driver provides support for USB joysticks and other gamecontrollers.
+They are Human Interface Devices (HID) which can be accessed via the
+.Pa /dev/ujoy/N
+interface.
+.Pp
+The driver is compatible with the
+.Xr read 2 ,
+and a subset of
+.Xr ioctl 2
+operations of the generic
+.Xr uhid 4
+device.
+.Sh FILES
+.Bl -tag -width /dev/ujoy/* -compact
+.It Pa /dev/ujoy/*
+.El
+.Sh SEE ALSO
+.Xr uhid 4 ,
+.Xr uhidev 4 ,
+.Xr usb 4
+.Sh HISTORY
+The
+.Nm
+driver first appeared in
+.Ox 6.9 .
Index: share/man/man4/usb.4
===
RCS file: /cvs/src/share/man/man4/usb.4,v
retrieving revision 1.203
diff -u -p -r1.203 usb.4
--- share/man/man4/usb.421 Aug 2020 20:38:56 -  1.203
+++ share/man/man4/usb.422 Jan 2021 20:05:06 -
@@ -255,6 +255,8 @@ TEMPer gold HID thermometer and hygromet
 Generic driver for Human Interface Devices
 .It Xr uhidev 4
 Base driver for all Human Interface Devices
+.It Xr ujoy 4
+USB joysticks/gamecontrollers
 .It Xr ukbd 4
 USB keyboards that follow the boot protocol
 .It Xr ums 4
Index: sys/arch/alpha/alpha/conf.c
===
RCS file: /cvs/src/sys/arch/alpha/alpha/conf.c,v
retrieving revision 1.88
diff -u -p -r1.88 conf.c
--- sys/arch/alpha/alpha/conf.c 6 Jul 2020 04:32:25 -   1.88
+++ sys/arch/alpha/alpha/conf.c 22 Jan 2021 20:05:06 -
@@ -113,6 +113,7 @@ cdev_decl(cy);
 #include "usb.h"
 #include "uhid.h"
 #include "fido.h"
+#include "ujoy.h"
 #include "ugen.h"
 #include "ulpt.h"
 #include "ucom.h"
@@ -207,6 +208,7 @@ struct cdevsw   cdevsw[] =
cdev_switch_init(NSWITCH,switch), /* 69: switch(4) control interface */
cdev_fido_init(NFIDO,fido), /* 70: FIDO/U2F security key */
cdev_pppx_init(NPPPX,pppac),/* 71: PPP Access Concentrator */
+   cdev_ujoy_init(NUJOY,ujoy), /* 72: USB joystick/gamecontroller */
 };
 intnchrdev = nitems(cdevsw);
 
Index: sys/arch/alpha/conf/GENERIC
===
RCS file: /cvs/src/sys/arch/alpha/conf/GENERIC,v
retrieving revision 1.266
diff -u -p -r1.266 GENERIC
--- sys/arch/alpha/conf/GENERIC 23 May 2020 06:28:29 -  1.266
+++ sys/arch/alpha/conf/GENERIC 22 Jan 2021 20:05:06 -
@@ -107,6 +107,7 @@ uslhcom* at uhidev? # Silicon Labs CP2
 ucom*  at uslhcom?
 uhid*  at uhidev?  # USB generic HID support
 fido*  at uhidev?  # FIDO/U2F security key support
+ujoy*  at uhidev?  # USB joystick/gamecontroller support
 upd*   at uhidev?  # USB Power Devices sensors
 aue*   at uhub?# ADMtek AN986 Pegasus Ethernet
 #atu*  at uhub?   

Re: XBox One gamecontroller support

2021-01-15 Thread Thomas Frohwein
On Sat, Jan 09, 2021 at 10:50:35AM -0700, Thomas Frohwein wrote:
> Hi,
> 
> This diff adds support for the XBox One gamecontroller in a similar way
> to what we have for the (older) XBox 360 controller [1][2]. This diff
> is based on the pertinent code in NetBSD's uhidev.c.
> 
> Similarities include that the device doesn't provide a report
> descriptor, so this diff adds one to uhid_rdesc.h.
> 
> The biggest difference (that held this up for a while) is that the
> controller needs an init byte sequence to "wake up."
> 
> The original report descriptor from NetBSD just maps the buttons and
> axes in the order that they appear in the report. I modified the report
> descriptor to assign the buttons/axes in the most logical order for
> compatibility, which results in the same layout that we already have
> for the XBox 360 controller.
> 
> The addition of the member sc_flags to struct uhidev_softc follows the
> NetBSD example.
> 
> I have tested this with usbhidctl(1), sdl2-jstest (from sdl-jstest
> package), and a couple of SDL2 games (Owlboy, Cryptark).
> 
> If you have an XBox One controller, the easiest way to test is with:
> 
> $ usbhidctl -f /dev/uhidX -l
> 
> Make sure to pick the right uhid device and that you have read
> permissions for it.
> 
> Of course, this works only with the controller connected via USB, and
> doesn't magically add wireless support.
> 
> If you test actual SDL2 applications, the button layout will likely
> default to an odd configuration. I am simultaneously preparing a diff
> for sdl2-2.0.14p0 that improves recognition and automatic mapping and
> solves any issues with XBox One default button layout.

I have an update to this diff. 2 testers used a more recent XBox One
controller model (model number 1708 in both cases) and ran into
problems with the original diff. Below is a new diff that uses a longer
5-byte init sequence taken from Linux' xpad.c [1], compared to the
2-byte sequence from NetBSD's uhidev.c that I offered in the initial
diff. This fixed the issue for the 2 testers.

My own XBox One controller is model number 1537 and still works with
this diff.

[1] https://github.com/paroj/xpad/blob/master/xpad.c#L479

Index: uhid_rdesc.h
===
RCS file: /cvs/src/sys/dev/usb/uhid_rdesc.h,v
retrieving revision 1.1
diff -u -p -r1.1 uhid_rdesc.h
--- uhid_rdesc.h25 Oct 2013 03:09:59 -  1.1
+++ uhid_rdesc.h9 Jan 2021 15:11:30 -
@@ -272,4 +272,94 @@ static const uByte uhid_xb360gp_report_d
 0x95, 0x01,/*  REPORT COUNT (1)*/
 0x81, 0x01,/*  INPUT (Constant)*/
 0xc0,  /* END COLLECTION   */
+};
+
+static const uByte uhid_xbonegp_report_descr[] = {
+0x05, 0x01,/* USAGE PAGE (Generic Desktop) 
*/
+0x09, 0x05,/* USAGE (Gamepad)  
*/
+0xa1, 0x01,/* COLLECTION (Application) 
*/
+/* Button packet */
+0xa1, 0x00,/*  COLLECTION (Physical)   
*/
+0x85, 0x20,/*   REPORT ID (0x20)   
*/
+/* Skip unknown field and counter */
+0x09, 0x00,/*   USAGE (Undefined)  
*/
+0x75, 0x08,/*   REPORT SIZE (8)
*/
+0x95, 0x02,/*   REPORT COUNT (2)   
*/
+0x81, 0x03,/*   INPUT (Constant, Variable, 
Absolute)   */
+/* Payload size */
+0x09, 0x3b,/*   USAGE (Byte Count) 
*/
+0x95, 0x01,/*   REPORT COUNT (1)   
*/
+0x81, 0x02,/*   INPUT (Data, Variable, Absolute)   
*/
+/* 16 Buttons: 1-2=Unknown, 3=Start, 4=Back, 5-8=ABXY,
+ *9-12=D-Pad(Up,Dn,Lt,Rt), 13=LB, 14=RB, 15=LS, 16=RS
+ */
+/* Skip the first 2 as they are not used */
+0x75, 0x01,/*   REPORT SIZE (1)
*/
+0x95, 0x02,/*   REPORT COUNT (2)   
*/
+0x81, 0x01,/*   INPUT (Constant)   
*/
+/* Assign buttons Start(7), Back(8), ABXY(1-4) */
+0x15, 0x00,/*   LOGICAL MINIMUM (0)
*/
+0x25, 0x01,/*   LOGICAL MAXIMUM (1)
*/
+0x95, 0x06,/*   REPORT COUNT (6)   
*/
+0x05, 0x09,

Re: New ujoy(4) device for USB gamecontrollers

2021-01-15 Thread Thomas Frohwein
On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote:
> On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote:
> 
> > > I have heard from others who tried the diff that the PS4 controller is
> > > causing problems with the way it attaches. I ordered one to trial-and-
> > > error this myself at home. Could you share output of lsusb -vv? Thanks
> > > for giving it a try!
> > 
> > Sure, here we go.
> > If I can find anything myself in the meantime I let you know.
> 
> So the problem doesn't seem to be in your new ujoy(4) code, but how the
> dev/hid/hid.c:hid_is_collection() function tries to cope with the PS4
> controller.

So with the hid_is_collection() problem not easy to mitigate [1],
should we table the ujoy(4) proposal for now pending a fix for the
problems with the PS4 controller? Or is this interesting enough for
some to work on moving forward despite this issue and finding a
solution for this specific (and in some ways unusual) device later?

3-4 have tested and reported to me so far. It seems so far that the
only new breakage is with the PS4 controller, and there is probably
another solution that can be found later that doesn't break other
drivers like [1]?

[1] https://marc.info/?l=openbsd-tech=161043081617336=mbox

> 
> I'm not much in to HID, but when I sync up the hid_is_collection()
> function with NetBSD, the PS4 controller attaches to ujoy(4) as
> expected, and also can be accessed by usbhidctl(1), while my other
> HID devices continue to work as before:
> 
> uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony Interactive
> Entertainment Wireless Controller" rev 2.00/1.00 addr 6
> uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, 4 ctls
> audio1 at uaudio0
> uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony Interactive
> Entertainment Wireless Controller" rev 2.00/1.00 addr 6
> uhidev6: iclass 3/0, 180 report ids
> ujoy0 at uhidev6 reportid 1: input=63, output=0, feature=0 <--
> uhid6 at uhidev6 reportid 2: input=0, output=0, feature=36
> uhid7 at uhidev6 reportid 4: input=0, output=0, feature=36
> uhid8 at uhidev6 reportid 5: input=0, output=31, feature=0
> uhid9 at uhidev6 reportid 8: input=0, output=0, feature=3
> uhid10 at uhidev6 reportid 16: input=0, output=0, feature=4
> uhid11 at uhidev6 reportid 17: input=0, output=0, feature=2
> uhid12 at uhidev6 reportid 18: input=0, output=0, feature=15
> uhid13 at uhidev6 reportid 19: input=0, output=0, feature=22
> uhid14 at uhidev6 reportid 20: input=0, output=0, feature=16
> uhid15 at uhidev6 reportid 21: input=0, output=0, feature=44
> uhid16 at uhidev6 reportid 128: input=0, output=0, feature=6
> uhid17 at uhidev6 reportid 129: input=0, output=0, feature=6
> uhid18 at uhidev6 reportid 130: input=0, output=0, feature=5
> uhid19 at uhidev6 reportid 131: input=0, output=0, feature=1
> uhid20 at uhidev6 reportid 132: input=0, output=0, feature=4
> uhid21 at uhidev6 reportid 133: input=0, output=0, feature=6
> uhid22 at uhidev6 reportid 134: input=0, output=0, feature=6
> uhid23 at uhidev6 reportid 135: input=0, output=0, feature=35
> uhid24 at uhidev6 reportid 136: input=0, output=0, feature=34
> uhid25 at uhidev6 reportid 137: input=0, output=0, feature=2
> uhid26 at uhidev6 reportid 144: input=0, output=0, feature=5
> uhid27 at uhidev6 reportid 145: input=0, output=0, feature=3
> uhid28 at uhidev6 reportid 146: input=0, output=0, feature=3
> uhid29 at uhidev6 reportid 147: input=0, output=0, feature=12
> uhid30 at uhidev6 reportid 160: input=0, output=0, feature=6
> uhid31 at uhidev6 reportid 161: input=0, output=0, feature=1
> uhid32 at uhidev6 reportid 162: input=0, output=0, feature=1
> uhid33 at uhidev6 reportid 163: input=0, output=0, feature=48
> uhid34 at uhidev6 reportid 164: input=0, output=0, feature=13
> uhid35 at uhidev6 reportid 165: input=0, output=0, feature=21
> uhid36 at uhidev6 reportid 166: input=0, output=0, feature=21
> uhid37 at uhidev6 reportid 167: input=0, output=0, feature=1
> uhid38 at uhidev6 reportid 168: input=0, output=0, feature=1
> uhid39 at uhidev6 reportid 169: input=0, output=0, feature=8
> uhid40 at uhidev6 reportid 170: input=0, output=0, feature=1
> uhid41 at uhidev6 reportid 171: input=0, output=0, feature=57
> uhid42 at uhidev6 reportid 172: input=0, output=0, feature=57
> uhid43 at uhidev6 reportid 173: input=0, output=0, feature=11
> uhid44 at uhidev6 reportid 174: input=0, output=0, feature=1
> uhid45 at uhidev6 reportid 175: input=0, output=0, feature=2
> uhid46 at uhidev6 reportid 176: input=0, output=0, feature=63
> uhid47 at uhidev6 reportid 177: input=0, output=0, feature=2
> uhid48 at uhidev6 reportid 178: input=0, output=0, feature=2
> uhid49 at uhidev6 reportid 179: input=0, output=0, feature=63
> uhid50 at uhidev6 reportid 180: input=0, output=0, feature=63
> 
>   $ usbhidctl -f /dev/ujoy/0 -l
>   Game_Pad.X=126
>   Game_Pad.Y=126
>   Game_Pad.Z=125
>   Game_Pad.Rz=129
>   Game_Pad.Hat_Switch=8
>   Game_Pad.Button_1=0

Re: New ujoy(4) device for USB gamecontrollers

2021-01-09 Thread Thomas Frohwein
On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote:

[...]

> So the problem doesn't seem to be in your new ujoy(4) code, but how the
> dev/hid/hid.c:hid_is_collection() function tries to cope with the PS4
> controller.
> 
> I'm not much in to HID, but when I sync up the hid_is_collection()
> function with NetBSD, the PS4 controller attaches to ujoy(4) as
> expected, and also can be accessed by usbhidctl(1), while my other
> HID devices continue to work as before:

Nice find.

[...]

> Once this has been fixed I would continue to do some more ujoy(4)
> testing, and check about a possible import.  We also should agree on
> who/how to fix the ports part.

While I can't speak for all 11,000 packages, I am yet to encounter a
port that uses gamecontrollers/joysticks without going through sdl or
sdl2. And for those 2 ports it's literally a 1-line diff each.

I'm not sure if looking at all the ports with 'usbhid' in WANTLIB would
be helpful. I have a list of 55 of those from sqlports, excluding
anything with 'sdl' in its name.



XBox One gamecontroller support

2021-01-09 Thread Thomas Frohwein
Hi,

This diff adds support for the XBox One gamecontroller in a similar way
to what we have for the (older) XBox 360 controller [1][2]. This diff
is based on the pertinent code in NetBSD's uhidev.c.

Similarities include that the device doesn't provide a report
descriptor, so this diff adds one to uhid_rdesc.h.

The biggest difference (that held this up for a while) is that the
controller needs an init byte sequence to "wake up."

The original report descriptor from NetBSD just maps the buttons and
axes in the order that they appear in the report. I modified the report
descriptor to assign the buttons/axes in the most logical order for
compatibility, which results in the same layout that we already have
for the XBox 360 controller.

The addition of the member sc_flags to struct uhidev_softc follows the
NetBSD example.

I have tested this with usbhidctl(1), sdl2-jstest (from sdl-jstest
package), and a couple of SDL2 games (Owlboy, Cryptark).

If you have an XBox One controller, the easiest way to test is with:

$ usbhidctl -f /dev/uhidX -l

Make sure to pick the right uhid device and that you have read
permissions for it.

Of course, this works only with the controller connected via USB, and
doesn't magically add wireless support.

If you test actual SDL2 applications, the button layout will likely
default to an odd configuration. I am simultaneously preparing a diff
for sdl2-2.0.14p0 that improves recognition and automatic mapping and
solves any issues with XBox One default button layout.

comments? ok?

[1] https://marc.info/?l=openbsd-cvs=138267062532046=mbox
[2] https://marc.info/?l=openbsd-cvs=144956819605939=mbox

Index: uhid_rdesc.h
===
RCS file: /cvs/src/sys/dev/usb/uhid_rdesc.h,v
retrieving revision 1.1
diff -u -p -r1.1 uhid_rdesc.h
--- uhid_rdesc.h25 Oct 2013 03:09:59 -  1.1
+++ uhid_rdesc.h9 Jan 2021 15:11:30 -
@@ -272,4 +272,94 @@ static const uByte uhid_xb360gp_report_d
 0x95, 0x01,/*  REPORT COUNT (1)*/
 0x81, 0x01,/*  INPUT (Constant)*/
 0xc0,  /* END COLLECTION   */
+};
+
+static const uByte uhid_xbonegp_report_descr[] = {
+0x05, 0x01,/* USAGE PAGE (Generic Desktop) 
*/
+0x09, 0x05,/* USAGE (Gamepad)  
*/
+0xa1, 0x01,/* COLLECTION (Application) 
*/
+/* Button packet */
+0xa1, 0x00,/*  COLLECTION (Physical)   
*/
+0x85, 0x20,/*   REPORT ID (0x20)   
*/
+/* Skip unknown field and counter */
+0x09, 0x00,/*   USAGE (Undefined)  
*/
+0x75, 0x08,/*   REPORT SIZE (8)
*/
+0x95, 0x02,/*   REPORT COUNT (2)   
*/
+0x81, 0x03,/*   INPUT (Constant, Variable, 
Absolute)   */
+/* Payload size */
+0x09, 0x3b,/*   USAGE (Byte Count) 
*/
+0x95, 0x01,/*   REPORT COUNT (1)   
*/
+0x81, 0x02,/*   INPUT (Data, Variable, Absolute)   
*/
+/* 16 Buttons: 1-2=Unknown, 3=Start, 4=Back, 5-8=ABXY,
+ *9-12=D-Pad(Up,Dn,Lt,Rt), 13=LB, 14=RB, 15=LS, 16=RS
+ */
+/* Skip the first 2 as they are not used */
+0x75, 0x01,/*   REPORT SIZE (1)
*/
+0x95, 0x02,/*   REPORT COUNT (2)   
*/
+0x81, 0x01,/*   INPUT (Constant)   
*/
+/* Assign buttons Start(7), Back(8), ABXY(1-4) */
+0x15, 0x00,/*   LOGICAL MINIMUM (0)
*/
+0x25, 0x01,/*   LOGICAL MAXIMUM (1)
*/
+0x95, 0x06,/*   REPORT COUNT (6)   
*/
+0x05, 0x09,/*   USAGE PAGE (Button)
*/
+0x09, 0x08,/*   USAGE (Button 8)   
*/
+0x09, 0x07,/*   USAGE (Button 7)   
*/
+0x09, 0x01,/*   USAGE (Button 1)   
*/
+0x09, 0x02,/*   USAGE (Button 2)   
*/
+0x09, 0x03,/*   USAGE (Button 3)   
*/
+0x09, 0x04,/*   USAGE (Button 4)   
*/
+0x81, 0x02,/*   INPUT (Data, Variable, 

Re: slimblade support

2021-01-09 Thread Thomas Frohwein
On Sat, Nov 21, 2020 at 08:10:03AM +0200, Timo Myyrä wrote:
> Hi,
> 
> The last attempt at adding Kensington Slimblade trackball support seems
> to have stalled:
> https://marc.info/?l=openbsd-tech=147444999319756=2
> 
> I tested the diff and it still seems apply with little fuzz and works
> with my slimblade. It would be nice to have this included so I can paste
> with mouse.
> 
> Here's cleaned up patch for reference.
> 
> timo

I've been running with this diff through several snapshot upgrades
without issues. Would be interested to hear if there are concerns about
this diff or if I can get another ok? As far as quirks go, it's not
much code and relatively simple...

> 
> 
> Index: sys/dev/usb/ums.c
> ===
> RCS file: /cvs/src/sys/dev/usb/ums.c,v
> retrieving revision 1.45
> diff -u -p -u -p -r1.45 ums.c
> --- sys/dev/usb/ums.c 23 Aug 2020 11:08:02 -  1.45
> +++ sys/dev/usb/ums.c 20 Nov 2020 20:22:11 -
> @@ -150,6 +150,8 @@ ums_attach(struct device *parent, struct
>   qflags |= HIDMS_MS_BAD_CLASS;
>   if (quirks & UQ_MS_LEADING_BYTE)
>   qflags |= HIDMS_LEADINGBYTE;
> + if (quirks & UQ_MS_VENDOR_BUTTONS)
> + qflags |= HIDMS_VENDOR_BUTTONS;
>  
>   if (hidms_setup(self, ms, qflags, uha->reportid, desc, size) != 0)
>   return;
> Index: sys/dev/usb/usb_quirks.c
> ===
> RCS file: /cvs/src/sys/dev/usb/usb_quirks.c,v
> retrieving revision 1.76
> diff -u -p -u -p -r1.76 usb_quirks.c
> --- sys/dev/usb/usb_quirks.c  5 Jan 2020 00:54:13 -   1.76
> +++ sys/dev/usb/usb_quirks.c  20 Nov 2020 20:22:11 -
> @@ -150,6 +150,9 @@ const struct usbd_quirk_entry {
>   { USB_VENDOR_MICROSOFT, USB_PRODUCT_MICROSOFT_WLNOTEBOOK2,
>   ANY, { UQ_MS_BAD_CLASS | UQ_MS_LEADING_BYTE }},
>  
> + { USB_VENDOR_KENSINGTON, USB_PRODUCT_KENSINGTON_SLIMBLADE,
> + ANY, { UQ_MS_VENDOR_BUTTONS }},
> +
>   { 0, 0, 0, { 0 } }
>  };
>  
> Index: sys/dev/usb/usb_quirks.h
> ===
> RCS file: /cvs/src/sys/dev/usb/usb_quirks.h,v
> retrieving revision 1.16
> diff -u -p -u -p -r1.16 usb_quirks.h
> --- sys/dev/usb/usb_quirks.h  19 Jul 2010 05:08:37 -  1.16
> +++ sys/dev/usb/usb_quirks.h  20 Nov 2020 20:22:11 -
> @@ -49,6 +49,8 @@ struct usbd_quirks {
>  #define UQ_MS_LEADING_BYTE   0x0001 /* mouse sends unknown leading byte 
> */
>  #define UQ_EHCI_NEEDTO_DISOWN0x0002 /* must hand device over to 
> USB 1.1
>   if attached to EHCI */
> +#define UQ_MS_VENDOR_BUTTONS 0x0004 /* mouse reports extra buttons in
> + vendor page */
>  };
>  
>  extern const struct usbd_quirks usbd_no_quirk;
> Index: sys/dev/usb/usbdevs
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs,v
> retrieving revision 1.728
> diff -u -p -u -p -r1.728 usbdevs
> --- sys/dev/usb/usbdevs   16 Nov 2020 09:49:10 -  1.728
> +++ sys/dev/usb/usbdevs   20 Nov 2020 20:22:11 -
> @@ -2491,6 +2491,7 @@ product KENSINGTON TURBOBALL0x1005  Turb
>  product KENSINGTON ORBIT_MAC 0x1009  Orbit trackball for Mac
>  product KENSINGTON BT_EDR0x105e  Bluetooth
>  product KENSINGTON VIDEOCAM_VGA  0x5002  VideoCAM VGA
> +product KENSINGTON SLIMBLADE 0x2041  Slimblade Trackball
>  
>  /* Keyspan products */
>  product KEYSPAN USA28_NF 0x0101  USA-28 serial
> Index: sys/dev/usb/usbdevs.h
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs.h,v
> retrieving revision 1.740
> diff -u -p -u -p -r1.740 usbdevs.h
> --- sys/dev/usb/usbdevs.h 16 Nov 2020 09:49:40 -  1.740
> +++ sys/dev/usb/usbdevs.h 20 Nov 2020 20:22:11 -
> @@ -2498,6 +2498,7 @@
>  #define  USB_PRODUCT_KENSINGTON_ORBIT_MAC0x1009  /* 
> Orbit trackball for Mac */
>  #define  USB_PRODUCT_KENSINGTON_BT_EDR   0x105e  /* Bluetooth */
>  #define  USB_PRODUCT_KENSINGTON_VIDEOCAM_VGA 0x5002  /* 
> VideoCAM VGA */
> +#define  USB_PRODUCT_KENSINGTON_SLIMBLADE0x2041  /* 
> Slimblade Trackball */
>  
>  /* Keyspan products */
>  #define  USB_PRODUCT_KEYSPAN_USA28_NF0x0101  /* USA-28 
> serial */
> Index: sys/dev/usb/usbdevs_data.h
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs_data.h,v
> retrieving revision 1.734
> diff -u -p -u -p -r1.734 usbdevs_data.h
> --- sys/dev/usb/usbdevs_data.h16 Nov 2020 09:49:40 -  1.734
> +++ sys/dev/usb/usbdevs_data.h20 Nov 2020 20:22:11 -
> @@ -5402,6 +5402,10 @@ const struct usb_known_product usb_known
>   "VideoCAM VGA",
>   },
>   {
> + USB_VENDOR_KENSINGTON, 

Re: New ujoy(4) device for USB gamecontrollers

2021-01-07 Thread Thomas Frohwein
On Wed, Jan 06, 2021 at 10:48:58PM +0100, Marcus Glocker wrote:

[...]

> The implementation as such looks fine to me.
> But I quickly gave the diff a spin before on amd64 using my PS
> controller, and the result is not what I would expect.
> 
> With uhid, I can access the controller on /dev/uhid6.  The attach looks
> like this:
> 
> imac /bsd: uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony
> Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6
> imac /bsd: uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec,
> 4 ctls
> imac /bsd: audio1 at uaudio0
> imac /bsd: uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony
> Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6
> imac /bsd: uhidev6: iclass 3/0, 180 report ids
> imac /bsd: uhid6 at uhidev6 reportid 1: input=63, output=0, feature=0

[...]

> imac /bsd: uhid50 at uhidev6 reportid 179: input=0, output=0, feature=63
> imac /bsd: uhid51 at uhidev6 reportid 180: input=0, output=0, feature=63
> 
> ujoy instead attached to previous uhid51, and there it is of no use
> obviously.  I still can access the controller through uhid6.  The attach
> with ujoy looks like this:
> 
> imac /bsd: uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony
> Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6
> imac /bsd: uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec,
> 4 ctls
> imac /bsd: audio1 at uaudio0
> imac /bsd: uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony
> Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6
> imac /bsd: uhidev6: iclass 3/0, 180 report ids

[...]

> imac /bsd: uhid50 at uhidev6 reportid 179: input=0, output=0, feature=63
> imac /bsd: ujoy0 at uhidev6 reportid 180: input=0, output=0, feature=63
> 
> I haven't analyzed yet what happens in the code.
> I can provide an lsusb of the controller if it's more obvious to you.

I have heard from others who tried the diff that the PS4 controller is
causing problems with the way it attaches. I ordered one to trial-and-
error this myself at home. Could you share output of lsusb -vv? Thanks
for giving it a try!



New ujoy(4) device for USB gamecontrollers

2020-12-28 Thread Thomas Frohwein
=
RCS file: /cvs/src/etc/etc.powerpc64/MAKEDEV.md,v
retrieving revision 1.6
diff -u -p -r1.6 MAKEDEV.md
--- etc/etc.powerpc64/MAKEDEV.md24 Oct 2020 21:10:41 -  1.6
+++ etc/etc.powerpc64/MAKEDEV.md28 Dec 2020 03:25:05 -
@@ -52,6 +52,7 @@ _DEV(uall)
 _DEV(ugen, 49)
 _DEV(uhid, 50)
 _DEV(fido, 51)
+_DEV(ujoy, 94)
 _DEV(ulpt, 65)
 _DEV(usb, 48)
 _TITLE(spec)
Index: etc/etc.sgi/MAKEDEV.md
===
RCS file: /cvs/src/etc/etc.sgi/MAKEDEV.md,v
retrieving revision 1.54
diff -u -p -r1.54 MAKEDEV.md
--- etc/etc.sgi/MAKEDEV.md  6 Jul 2020 06:11:27 -   1.54
+++ etc/etc.sgi/MAKEDEV.md  28 Dec 2020 03:25:05 -
@@ -69,6 +69,7 @@ _DEV(uall)
 _DEV(ugen, 63)
 _DEV(uhid, 62)
 _DEV(fido, 76)
+_DEV(ujoy, 78)
 _DEV(ulpt, 64)
 _DEV(usb, 61)
 _TITLE(spec)
Index: etc/etc.sparc64/MAKEDEV.md
===
RCS file: /cvs/src/etc/etc.sparc64/MAKEDEV.md,v
retrieving revision 1.95
diff -u -p -r1.95 MAKEDEV.md
--- etc/etc.sparc64/MAKEDEV.md  22 Jul 2020 14:04:37 -  1.95
+++ etc/etc.sparc64/MAKEDEV.md  28 Dec 2020 03:25:05 -
@@ -104,6 +104,7 @@ _DEV(uall)
 _DEV(ugen, 92)
 _DEV(uhid, 91)
 _DEV(fido, 137)
+_DEV(ujoy, 139)
 _DEV(ulpt, 93)
 _DEV(usb, 90)
 _TITLE(spec)
Index: share/man/man4/Makefile
===
RCS file: /cvs/src/share/man/man4/Makefile,v
retrieving revision 1.790
diff -u -p -r1.790 Makefile
--- share/man/man4/Makefile 6 Dec 2020 20:48:12 -   1.790
+++ share/man/man4/Makefile 28 Dec 2020 03:25:31 -
@@ -84,7 +84,7 @@ MAN=  aac.4 abcrtc.4 abl.4 ac97.4 acphy.4
ubsec.4 ucom.4 uchcom.4 ucrcom.4 ucycom.4 ukspan.4 uslhcom.4 \
udav.4 udcf.4 udl.4 udp.4 udsbr.4 \
uftdi.4 ugen.4 ugl.4 ugold.4 uguru.4 uhci.4 uhid.4 uhidev.4 uipaq.4 \
-   uk.4 ukbd.4 \
+   ujoy.4 uk.4 ukbd.4 \
ukphy.4 ulpt.4 umass.4 umb.4 umbg.4 umcs.4 umct.4 umidi.4 umodem.4 \
ums.4 umsm.4 umstc.4 umt.4 unix.4 uonerng.4 uow.4 uoaklux.4 uoakrh.4 \
uoakv.4 upd.4 upgt.4 upl.4 uplcom.4 ural.4 ure.4 url.4 urlphy.4 \
Index: share/man/man4/uhidev.4
===
RCS file: /cvs/src/share/man/man4/uhidev.4,v
retrieving revision 1.12
diff -u -p -r1.12 uhidev.4
--- share/man/man4/uhidev.4 21 Aug 2020 19:02:46 -  1.12
+++ share/man/man4/uhidev.4 28 Dec 2020 03:25:31 -
@@ -40,6 +40,7 @@
 .Cd "ucycom*  at uhidev?"
 .Cd "ugold*   at uhidev?"
 .Cd "uhid*at uhidev?"
+.Cd "ujoy*at uhidev?"
 .Cd "ukbd*at uhidev?"
 .Cd "ums* at uhidev?"
 .Cd "umstc*   at uhidev?"
@@ -73,6 +74,7 @@ only dispatches data to them based on th
 .Xr ucycom 4 ,
 .Xr ugold 4 ,
 .Xr uhid 4 ,
+.Xr ujoy 4 ,
 .Xr ukbd 4 ,
 .Xr ums 4 ,
 .Xr umstc 4 ,
Index: share/man/man4/ujoy.4
===
RCS file: share/man/man4/ujoy.4
diff -N share/man/man4/ujoy.4
--- /dev/null   1 Jan 1970 00:00:00 -
+++ share/man/man4/ujoy.4   28 Dec 2020 03:25:31 -0000
@@ -0,0 +1,53 @@
+.\" $OpenBSD: ujoy.4,v 1.4 2020/08/21 19:02:46 mglocker Exp $
+.\"
+.\" Copyright (c) 2020 Thomas Frohwein 
+.\" Copyright (c) 2020 Bryan Steele
+.\"
+.\" Permission to use, copy, modify, and distribute this software for any
+.\" purpose with or without fee is hereby granted, provided that the above
+.\" copyright notice and this permission notice appear in all copies.
+.\"
+.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+.\"
+.Dd $Mdocdate: December 27 2020 $
+.Dt UJOY 4
+.Os
+.Sh NAME
+.Nm ujoy
+.Nd USB joystick/gamecontroller
+.Sh SYNOPSIS
+.Cd "ujoy* at uhidev?"
+.Sh DESCRIPTION
+The
+.Nm
+driver provides support for USB joysticks and other gamecontrollers.
+They are Human Interface Devices (HID) which can be accessed via the
+.Pa /dev/ujoy/N
+interface.
+.Pp
+The driver is compatible with the
+.Xr read 2 ,
+and a subset of
+.Xr ioctl 2
+operations of the generic
+.Xr uhid 4
+device.
+.Sh FILES
+.Bl -tag -width /dev/ujoy/* -compact
+.It Pa /dev/ujoy/*
+.El
+.Sh SEE ALSO
+.Xr uhid 4 ,
+.Xr uhidev 4 ,
+.Xr usb 4
+.Sh HISTORY
+The
+.Nm
+driver first appeared in
+.Ox 6.9 .
Index: share/man/man4/usb.4
===
RCS fi

Re: slimblade support

2020-11-21 Thread Thomas Frohwein
Hi timo

On Sat, Nov 21, 2020 at 08:10:03AM +0200, Timo Myyrä wrote:
> Hi,
> 
> The last attempt at adding Kensington Slimblade trackball support seems
> to have stalled:
> https://marc.info/?l=openbsd-tech=147444999319756=2

Thanks for digging this up. I got a Slimblade, but have been using
Emulate3Buttons to paste because I didn't know about this diff.

> 
> I tested the diff and it still seems apply with little fuzz and works
> with my slimblade. It would be nice to have this included so I can paste
> with mouse.
> 
> Here's cleaned up patch for reference.

I built a kernel with the patch. Kensington Slimblade works, now can paste
with the button in the upper left corner. xev(1) detects all buttons.
Scrolling by "twisting" the trackball still works as usual.

The code looks ok thfr@ to me, but better wait for an additional ok from
someone with a bit more familiarity with the usb and hid parts.

> 
> timo
> 
> 
> Index: sys/dev/usb/ums.c
> ===
> RCS file: /cvs/src/sys/dev/usb/ums.c,v
> retrieving revision 1.45
> diff -u -p -u -p -r1.45 ums.c
> --- sys/dev/usb/ums.c 23 Aug 2020 11:08:02 -  1.45
> +++ sys/dev/usb/ums.c 20 Nov 2020 20:22:11 -
> @@ -150,6 +150,8 @@ ums_attach(struct device *parent, struct
>   qflags |= HIDMS_MS_BAD_CLASS;
>   if (quirks & UQ_MS_LEADING_BYTE)
>   qflags |= HIDMS_LEADINGBYTE;
> + if (quirks & UQ_MS_VENDOR_BUTTONS)
> + qflags |= HIDMS_VENDOR_BUTTONS;
>  
>   if (hidms_setup(self, ms, qflags, uha->reportid, desc, size) != 0)
>   return;
> Index: sys/dev/usb/usb_quirks.c
> ===
> RCS file: /cvs/src/sys/dev/usb/usb_quirks.c,v
> retrieving revision 1.76
> diff -u -p -u -p -r1.76 usb_quirks.c
> --- sys/dev/usb/usb_quirks.c  5 Jan 2020 00:54:13 -   1.76
> +++ sys/dev/usb/usb_quirks.c  20 Nov 2020 20:22:11 -
> @@ -150,6 +150,9 @@ const struct usbd_quirk_entry {
>   { USB_VENDOR_MICROSOFT, USB_PRODUCT_MICROSOFT_WLNOTEBOOK2,
>   ANY, { UQ_MS_BAD_CLASS | UQ_MS_LEADING_BYTE }},
>  
> + { USB_VENDOR_KENSINGTON, USB_PRODUCT_KENSINGTON_SLIMBLADE,
> + ANY, { UQ_MS_VENDOR_BUTTONS }},
> +
>   { 0, 0, 0, { 0 } }
>  };
>  
> Index: sys/dev/usb/usb_quirks.h
> ===
> RCS file: /cvs/src/sys/dev/usb/usb_quirks.h,v
> retrieving revision 1.16
> diff -u -p -u -p -r1.16 usb_quirks.h
> --- sys/dev/usb/usb_quirks.h  19 Jul 2010 05:08:37 -  1.16
> +++ sys/dev/usb/usb_quirks.h  20 Nov 2020 20:22:11 -
> @@ -49,6 +49,8 @@ struct usbd_quirks {
>  #define UQ_MS_LEADING_BYTE   0x0001 /* mouse sends unknown leading byte 
> */
>  #define UQ_EHCI_NEEDTO_DISOWN0x0002 /* must hand device over to 
> USB 1.1
>   if attached to EHCI */
> +#define UQ_MS_VENDOR_BUTTONS 0x0004 /* mouse reports extra buttons in
> + vendor page */
>  };
>  
>  extern const struct usbd_quirks usbd_no_quirk;
> Index: sys/dev/usb/usbdevs
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs,v
> retrieving revision 1.728
> diff -u -p -u -p -r1.728 usbdevs
> --- sys/dev/usb/usbdevs   16 Nov 2020 09:49:10 -  1.728
> +++ sys/dev/usb/usbdevs   20 Nov 2020 20:22:11 -
> @@ -2491,6 +2491,7 @@ product KENSINGTON TURBOBALL0x1005  Turb
>  product KENSINGTON ORBIT_MAC 0x1009  Orbit trackball for Mac
>  product KENSINGTON BT_EDR0x105e  Bluetooth
>  product KENSINGTON VIDEOCAM_VGA  0x5002  VideoCAM VGA
> +product KENSINGTON SLIMBLADE 0x2041  Slimblade Trackball
>  
>  /* Keyspan products */
>  product KEYSPAN USA28_NF 0x0101  USA-28 serial
> Index: sys/dev/usb/usbdevs.h
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs.h,v
> retrieving revision 1.740
> diff -u -p -u -p -r1.740 usbdevs.h
> --- sys/dev/usb/usbdevs.h 16 Nov 2020 09:49:40 -  1.740
> +++ sys/dev/usb/usbdevs.h 20 Nov 2020 20:22:11 -
> @@ -2498,6 +2498,7 @@
>  #define  USB_PRODUCT_KENSINGTON_ORBIT_MAC0x1009  /* 
> Orbit trackball for Mac */
>  #define  USB_PRODUCT_KENSINGTON_BT_EDR   0x105e  /* Bluetooth */
>  #define  USB_PRODUCT_KENSINGTON_VIDEOCAM_VGA 0x5002  /* 
> VideoCAM VGA */
> +#define  USB_PRODUCT_KENSINGTON_SLIMBLADE0x2041  /* 
> Slimblade Trackball */
>  
>  /* Keyspan products */
>  #define  USB_PRODUCT_KEYSPAN_USA28_NF0x0101  /* USA-28 
> serial */
> Index: sys/dev/usb/usbdevs_data.h
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs_data.h,v
> retrieving revision 1.734
> diff -u -p -u -p -r1.734 usbdevs_data.h
> --- sys/dev/usb/usbdevs_data.h16 Nov 2020 09:49:40 -   

Re: RFC: kern.video.record

2020-09-18 Thread Thomas Frohwein
On Sun, Sep 13, 2020 at 09:23:36AM +0100, Laurence Tratt wrote:
> Since I recently opened my big fat mouth and suggested that
> "kern.video.record" (analogous to kern.audio.record) might be a good idea, I

I support this suggestion. I think the idea behind it is based on the
same concerns that led to "kern.audio.record" (though admittedly I
wasn't privy to the conversations leading up to it).

> decided to put together a quick prototype (heavily based on the
> kern.audio.record code). This at least roughly works for me but raises some
> questions such as:
> 
>   * Is uvideo the only driver that can capture video? [I imagine not, but I
> don't really know.]
> 
>   * I've taken the same approach as kern.audio.record which is to keep on
> handing out data even when kern.video.record=0 *but* it's harder to work
> out what data we should hand out. At the moment we just pass on whatever
> happened to be in the buffer beforehand (which is clearly not a good
> idea in the long term, but proves the point for now). For uncompressed
> video, handing over (say) an entirely black image is probably easy; for
> compressed video we'd have to think about whether we also want to
> manipulate frame headers etc. It would probably be easier to simply
> intercept the "opening video" event but that would mean that if
> something is already streaming video, setting kern.video.record=0 would
> allow it to keep recording.

I built a kernel and sysctl with this and can confirm it works with my
Logitech C310. Of note, in its current form, running video(1) while
kern.video.record is 0 leads to a window showing mostly green and pink colors.

> 
> Comments welcome, including "this is a terrible idea full stop"!

Again, I'm in favor of this idea.

It might be worth considering combining kern.audio.record and
kern.video.record, as they serve the same purpose (just different
modalities). I doubt that granular controls with separate switches will
be very useful at this level; maybe rather leave this differentiation
to the application?

An added benefit would be that there would be less largely duplicated
code; and probably better maintainability.

> 
> 
> Laurie
> 
> 
> 
> Index: sbin/sysctl/sysctl.c
> ===
> RCS file: /cvs/src/sbin/sysctl/sysctl.c,v
> retrieving revision 1.252
> diff -u -p -r1.252 sysctl.c
> --- sbin/sysctl/sysctl.c  15 Jul 2020 07:13:56 -  1.252
> +++ sbin/sysctl/sysctl.c  13 Sep 2020 08:08:57 -
> @@ -130,6 +130,7 @@ struct ctlname machdepname[] = CTL_MACHD
>  #endif
>  struct ctlname ddbname[] = CTL_DDB_NAMES;
>  struct ctlname audioname[] = CTL_KERN_AUDIO_NAMES;
> +struct ctlname videoname[] = CTL_KERN_VIDEO_NAMES;
>  struct ctlname witnessname[] = CTL_KERN_WITNESS_NAMES;
>  char names[BUFSIZ];
>  int lastused;
> @@ -219,6 +220,7 @@ void print_sensor(struct sensor *);
>  int sysctl_chipset(char *, char **, int *, int, int *);
>  #endif
>  int sysctl_audio(char *, char **, int *, int, int *);
> +int sysctl_video(char *, char **, int *, int, int *);
>  int sysctl_witness(char *, char **, int *, int, int *);
>  void vfsinit(void);
>  
> @@ -517,6 +519,11 @@ parse(char *string, int flags)
>   if (len < 0)
>   return;
>   break;
> + case KERN_VIDEO:
> + len = sysctl_video(string, , mib, flags, );
> + if (len < 0)
> + return;
> + break;
>   case KERN_WITNESS:
>   len = sysctl_witness(string, , mib, flags, );
>   if (len < 0)
> @@ -1766,6 +1773,7 @@ struct list shmlist = { shmname, KERN_SH
>  struct list watchdoglist = { watchdogname, KERN_WATCHDOG_MAXID };
>  struct list tclist = { tcname, KERN_TIMECOUNTER_MAXID };
>  struct list audiolist = { audioname, KERN_AUDIO_MAXID };
> +struct list videolist = { audioname, KERN_VIDEO_MAXID };
>  struct list witnesslist = { witnessname, KERN_WITNESS_MAXID };
>  
>  /*
> @@ -2813,6 +2821,25 @@ sysctl_audio(char *string, char **bufpp,
>   return (-1);
>   mib[2] = indx;
>   *typep = audiolist.list[indx].ctl_type;
> + return (3);
> +}
> +
> +/*
> + * Handle video support
> + */
> +int
> +sysctl_video(char *string, char **bufpp, int mib[], int flags, int *typep)
> +{
> + int indx;
> +
> + if (*bufpp == NULL) {
> + listall(string, );
> + return (-1);
> + }
> + if ((indx = findname(string, "third", bufpp, )) == -1)
> + return (-1);
> + mib[2] = indx;
> + *typep = videolist.list[indx].ctl_type;
>   return (3);
>  }
>  
> Index: sys/dev/usb/uvideo.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
> retrieving revision 1.209
> diff -u -p -r1.209 uvideo.c
> --- sys/dev/usb/uvideo.c  31 Jul 2020 

Re: amdgpu (and possible radeondrm) fix

2020-01-22 Thread Thomas Frohwein
On Tue, Jan 21, 2020, at 7:10 PM, Mark Kettenis wrote:
> The attached diff fixes amdgpu(4) and might very well fix radeondrm(4) 
> as well.  
[...]

For the record, this fixes running piglit with radeon on my HD7570 since this
was committed. It used to lock up on the test spec/arb_sync/sync_api, but
now piglit completes. I haven't done further investigations how that may be
related, but though it may be good to know that this means progress with
the piglit testing suite.
-- 
  
tfrohw...@fastmail.com

PGP Public Key: https://pgp.mit.edu/pks/lookup?op=get=0xE1A22D58D20C6D22



Re: 64-bit dma for drm(4) on amd64

2019-06-06 Thread Thomas Frohwein
On Thu, Jun 06, 2019 at 11:41:07PM +0200, Mark Kettenis wrote:
[...]
> It seems to work fine on my Intel Broadwell laptop.  I haven't tested
> this on radeon(4) yet.  So further testing, especially on systems with
> 4GB of memory or more is necessary.
> 
> Please test.

Tested on a desktop with 8GB RAM and amdgpu enabled (GPU is a Vega 64,
but that may not be relevant). I'm attaching dmesg below. Tested for
about 2 hours with some Chrome, some Firefox, mono... No regression
noticed. The situations where the display freezes were encountered as
before building the kernel with this patch, that is with Chrome after a
while with Google Maps (one of the most reliable triggers). Also with
`piglit run all results/all` after 39 tests (dmafence on wait channel
again).

If there's more that would help testing or reporting, I'm happy to.

> 
> 
> Index: arch/amd64/amd64/bus_dma.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/bus_dma.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 bus_dma.c
> --- arch/amd64/amd64/bus_dma.c14 Oct 2017 04:44:43 -  1.50
> +++ arch/amd64/amd64/bus_dma.c6 Jun 2019 21:19:21 -
> @@ -319,7 +319,8 @@ _bus_dmamap_load_raw(bus_dma_tag_t t, bu
>   if (plen < sgsize)
>   sgsize = plen;
>  
> - if (paddr > dma_constraint.ucr_high)
> + if (paddr > dma_constraint.ucr_high &&
> + (map->_dm_flags & BUS_DMA_64BIT) == 0)
>   panic("Non dma-reachable buffer at paddr 
> %#lx(raw)",
>   paddr);
>  
> @@ -583,7 +584,8 @@ _bus_dmamap_load_buffer(bus_dma_tag_t t,
>*/
>   pmap_extract(pmap, vaddr, (paddr_t *));
>  
> - if (curaddr > dma_constraint.ucr_high)
> + if (curaddr > dma_constraint.ucr_high &&
> + (map->_dm_flags & BUS_DMA_64BIT) == 0)
>   panic("Non dma-reachable buffer at curaddr %#lx(raw)",
>   curaddr);
>  
> Index: dev/pci/drm/drm_linux.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v
> retrieving revision 1.37
> diff -u -p -r1.37 drm_linux.c
> --- dev/pci/drm/drm_linux.c   4 Jun 2019 12:08:22 -   1.37
> +++ dev/pci/drm/drm_linux.c   6 Jun 2019 21:19:21 -
> @@ -293,16 +293,19 @@ struct vm_page *
>  alloc_pages(unsigned int gfp_mask, unsigned int order)
>  {
>   int flags = (gfp_mask & M_NOWAIT) ? UVM_PLA_NOWAIT : UVM_PLA_WAITOK;
> + struct uvm_constraint_range *constraint = _constraint;
>   struct pglist mlist;
>  
>   if (gfp_mask & M_CANFAIL)
>   flags |= UVM_PLA_FAILOK;
>   if (gfp_mask & M_ZERO)
>   flags |= UVM_PLA_ZERO;
> + if (gfp_mask & __GFP_DMA32)
> + constraint = _constraint;
>  
>   TAILQ_INIT();
> - if (uvm_pglistalloc(PAGE_SIZE << order, dma_constraint.ucr_low,
> - dma_constraint.ucr_high, PAGE_SIZE, 0, , 1, flags))
> + if (uvm_pglistalloc(PAGE_SIZE << order, constraint->ucr_low,
> + constraint->ucr_high, PAGE_SIZE, 0, , 1, flags))
>   return NULL;
>   return TAILQ_FIRST();
>  }
> Index: dev/pci/drm/include/linux/gfp.h
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/include/linux/gfp.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 gfp.h
> --- dev/pci/drm/include/linux/gfp.h   14 Apr 2019 10:14:53 -  1.1
> +++ dev/pci/drm/include/linux/gfp.h   6 Jun 2019 21:19:21 -
> @@ -7,24 +7,25 @@
>  #include 
>  #include 
>  
> -#define GFP_ATOMIC   M_NOWAIT
> -#define GFP_NOWAIT   M_NOWAIT
> -#define GFP_KERNEL   (M_WAITOK | M_CANFAIL)
> -#define GFP_USER (M_WAITOK | M_CANFAIL)
> -#define GFP_TEMPORARY(M_WAITOK | M_CANFAIL)
> -#define GFP_HIGHUSER 0
> -#define GFP_DMA320
> -#define __GFP_NOWARN 0
> -#define __GFP_NORETRY0
> -#define __GFP_ZERO   M_ZERO
> +#define __GFP_ZERO   M_ZERO
> +#define __GFP_DMA32  0x8000
> +#define __GFP_NOWARN 0
> +#define __GFP_NORETRY0
>  #define __GFP_RETRY_MAYFAIL  0
>  #define __GFP_MOVABLE0
>  #define __GFP_COMP   0
> -#define GFP_TRANSHUGE_LIGHT  0
>  #define __GFP_KSWAPD_RECLAIM 0
>  #define __GFP_HIGHMEM0
>  #define __GFP_RECLAIMABLE0
> -#define __GFP_DMA32  0
> +
> +#define GFP_ATOMIC   M_NOWAIT
> +#define GFP_NOWAIT   M_NOWAIT
> +#define GFP_KERNEL   (M_WAITOK | M_CANFAIL)
> +#define GFP_USER (M_WAITOK | M_CANFAIL)
> +#define GFP_TEMPORARY(M_WAITOK | M_CANFAIL)
> +#define GFP_HIGHUSER 0
> +#define GFP_DMA32__GFP_DMA32
> +#define GFP_TRANSHUGE_LIGHT  0
>  
>  static inline bool
>  gfpflags_allow_blocking(const unsigned int flags)
> Index: 

disable seemingly unsupported MSI for AMD 17h/Raven Ridge HD Audio in azalia.c

2018-07-08 Thread Thomas Frohwein
Hi,

It appears that HD Audio from AMD's generation Ryzen can't handle MSI.
This leads to the bug that I reported here:

https://marc.info/?l=openbsd-bugs=151648196215922=2

Disabling MSI resolves the problem on my current system which is a Raven
Ridge APU. I don't have the Summit Ridge hardware anymore to test that
it is also resolved there, but the line is included in the diff
(PCI_PRODUCT_AMD_AMD64_17_HDA). It seems likely that this diff will also
fix HD Audio on Summit Ridge. However, testing would be welcome by
anyone who has a first-gen Ryzen.

I was slightly confused by the fact that so far it seems I've been the
only one who reported this. Even searching online for such an issue in
other OS didn't yield anything. However, the issue was there between 2
different Ryzen CPUs, 3 different motherboards, and at least 2 separate
OpenBSD -current installations. And it was never there on any
Intel-based setup, with otherwise same hardware and OpenBSD install.
While there have been several reports of people using Ryzen with
OpenBSD, they may not have used audio (that's my explanation for this
at the moment).

This diff was collaborative work with brynet@.

ok?

Index: azalia.c
===
RCS file: /cvs/src/sys/dev/pci/azalia.c,v
retrieving revision 1.244
diff -u -p -r1.244 azalia.c
--- azalia.c22 Apr 2018 10:02:13 -  1.244
+++ azalia.c7 Jul 2018 18:26:20 -
@@ -517,6 +517,15 @@ azalia_pci_attach(struct device *parent,
azalia_pci_write(sc->pc, sc->tag, ICH_PCI_MMC, reg);
}
 
+   /* disable MSI for AMD Summit Ridge/Raven Ridge HD Audio */
+   if (PCI_VENDOR(sc->pciid) == PCI_VENDOR_AMD) {
+   switch (PCI_PRODUCT(sc->pciid)) {
+   case PCI_PRODUCT_AMD_AMD64_17_HDA:
+   case PCI_PRODUCT_AMD_RAVENRIDGE_HDA:
+   pa->pa_flags &= ~PCI_FLAGS_MSI_ENABLED;
+   }
+   }
+
/* interrupt */
if (pci_intr_map_msi(pa, ) && pci_intr_map(pa, )) {
printf(": can't map interrupt\n");



pcidevs for AMD 17h and Raven Ridge APUs (Ryzen)

2018-07-07 Thread Thomas Frohwein
Hi,

This is a diff to add a bunch of AMD Summit Ridge (17h) and Raven Ridge
PCI devices. It's in preparation of an upcoming diff for these that I've
worked on in collaboration with brynet@.

The link to http://www.pcidatabase.com/ in the comments for pcidevs
isn't retrievable anymore. I have found device ids here instead:

https://pci-ids.ucw.cz/read/PC/1022

This includes all that I could find in my dmesg on Raven Ridge Ryzen 3
2200G, as well as dmesg of the Summit Ridge Ryzen 5 1400 that I owned
previously (see
https://marc.info/?l=openbsd-bugs=151648196215922=2).

ok?

Index: pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.1853
diff -u -p -r1.1853 pcidevs
--- pcidevs 6 Jul 2018 01:37:00 -   1.1853
+++ pcidevs 7 Jul 2018 18:28:47 -
@@ -730,7 +730,22 @@ product AMD AMD64_15_3X_PCIE_1 0x1424  AM
 product AMD AMD64_15_3X_PCIE_2 0x1425  AMD64 15h PCIE
 product AMD AMD64_15_3X_PCIE_3 0x1426  AMD64 15h PCIE
 product AMD AMD64_16_PCIE  0x1439  AMD64 16h PCIE
+product AMD AMD64_17_RC0x1450  AMD64 17h Root Complex
+product AMD AMD64_17_IOMMU 0x1451  AMD64 17h IOMMU
+product AMD AMD64_17_PCIE_10x1452  AMD64 17h PCIE
+product AMD AMD64_17_PCIE_20x1453  AMD64 17h PCIE
+product AMD AMD64_17_PCIE_30x1454  AMD64 17h PCIE
 product AMD CCPV5A 0x1456  Cryptographic Co-processor v5a
+product AMD AMD64_17_HDA   0x1457  AMD64 17h HD Audio
+product AMD AMD64_17_XHCI  0x145c  AMD64 17h USB xHCI
+product AMD AMD64_17_DF_1  0x1460  AMD64 17h Data Fabric
+product AMD AMD64_17_DF_2  0x1461  AMD64 17h Data Fabric
+product AMD AMD64_17_DF_3  0x1462  AMD64 17h Data Fabric
+product AMD AMD64_17_DF_4  0x1463  AMD64 17h Data Fabric
+product AMD AMD64_17_DF_5  0x1464  AMD64 17h Data Fabric
+product AMD AMD64_17_DF_6  0x1465  AMD64 17h Data Fabric
+product AMD AMD64_17_DF_7  0x1466  AMD64 17h Data Fabric
+product AMD AMD64_17_DF_8  0x1467  AMD64 17h Data Fabric
 product AMD CCPV5B 0x1468  Cryptographic Co-processor v5b
 product AMD AMD64_14_HB0x1510  AMD64 14h Host
 product AMD AMD64_14_PCIE_10x1512  AMD64 14h PCIE
@@ -766,6 +781,7 @@ product AMD AMD64_16_3X_DRAM0x1582  AMD6
 product AMD AMD64_16_3X_MISC   0x1583  AMD64 16h Misc Cfg
 product AMD AMD64_16_3X_CPU_PM 0x1584  AMD64 16h CPU Power
 product AMD AMD64_16_3X_MISC_2 0x1585  AMD64 16h Misc Cfg
+product AMD RAVENRIDGE_HDA 0x15e3  Raven Ridge HD Audio
 product AMD AMD64_15_0x_LINK   0x1600  AMD64 15/0xh Link Cfg
 product AMD AMD64_15_0x_ADDR   0x1601  AMD64 15/0xh Address Map
 product AMD AMD64_15_0x_DRAM   0x1602  AMD64 15/0xh DRAM Cfg
@@ -809,6 +825,9 @@ product AMD HUDSON2_PCIE_1  0x43a0  Hudson
 product AMD HUDSON2_PCIE_2 0x43a1  Hudson-2 PCIE
 product AMD HUDSON2_PCIE_3 0x43a2  Hudson-2 PCIE
 product AMD HUDSON2_PCIE_4 0x43a3  Hudson-2 PCIE
+product AMD 300SERIES_PCIE 0x43b4  300 Series PCIe Port
+product AMD 300SERIES_SATA 0x43b7  300 Series SATA
+product AMD 300SERIES_XHCI 0x43bb  300 Series xHCI
 /* http://www.amd.com/products/cpg/athlon/techdocs/pdf/21910.pdf */
 product AMD SC751_SC   0x7006  751 System
 product AMD SC751_PPB  0x7007  751



mono: partial progress and possible issue with TLS/pthread_getspecific(3)

2018-02-24 Thread Thomas Frohwein
In December, I found the mono port in a state where it only built with the
(deprecated) mcs compiler, and it would catch a SIGABRT predominantly when
exiting applications (including any call to the newer csc/roslyn compiler).

I've since made what seems like partial progress with mono's functionality and
reliability, but hit a dead end. It appears to be related to the thread local
storage. Here's a brief summary, hoping for some additional input from those
with more knowledge about these matters:

- I found that the SIGABRT was consistently associated with a SIGTTIN signal
  that mono sets in mono-threads-posix-signals.c as the abort signal for
  threads. Setting it to SIGUSR1 resolved this and allows for applications
  to exit without aborting. (I found some reference that Linux uses SIGUSR1/2,
  and on platforms with SIGRTMIN, the first one available from there is chosen)

- After this, trying to build with csc/roslyn still fails - it deadlocks when
  building the first dll (mscorlib). Attaching it to gdb shows ~16-20 parallel
  threads with no clear pattern (to me) that would lead anywhere.

- However, setting build option '--with-cooperative-gc=yes' allows the build
  process to run further. This is also what upstream is planning to make the
  default (https://github.com/mono/mono/issues/6921).
  Now it builds several dozen if not more dll's with csc/roslyn, but eventually
  segfaults. This occurs at different times and with different dll's each time.
  I've tried to troubleshoot it, but this seems to be beyond my capabilities.
  Every time, the backtrace of the core file looks like this:

[...]
Core was generated by `mono-sgen'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  mono_get_lmf () at mini-runtime.c:741
741 return jit_tls->lmf;
(gdb) where
#0  mono_get_lmf () at mini-runtime.c:741
#1  0x1f5d55296988 in mono_thread_state_init (ctx=0x1f5f82f08070) at 
mini-exceptions.c:3127
#2  0x1f5d554d9b19 in 
mono_threads_enter_gc_safe_region_unbalanced_with_info (info=0x1f5f82f08000, 
stackdata=) at mono-threads-coop.c:245
#3  0x1f5d554d69b3 in mono_thread_info_suspend_lock_with_info 
(info=0x1f5f82f08000) at mono-threads.c:1093
#4  0x1f5d55479a79 in acquire_gc_locks () at sgen-stw.c:87
#5  sgen_client_stop_world (generation=0) at sgen-stw.c:111
#6  0x1f5d5548869e in sgen_stop_world (generation=0) at sgen-gc.c:3790
#7  0x1f5d5545d5a5 in mono_gc_clear_domain (domain=0x1f5fc211d400) at 
sgen-mono.c:865
#8  0x1f5d55362a88 in mono_domain_free (domain=0x1f5fc211d400, 
force=) at domain.c:1088
#9  0x1f5d55209556 in mini_cleanup (domain=0x1f5fc211d400) at 
mini-runtime.c:4445
#10 0x1f5d5525c5eb in mono_main (argc=26, argv=) at 
driver.c:2374
#11 0x1f5d552016f0 in mono_main_with_options (argc=2, argv=0x7f7d4e78) 
at ./main.c:47
#12 main (argc=26, argv=) at ./main.c:344
(gdb) info locals
jit_tls = 0x1f5f8df48000
(gdb) p *jit_tls
Cannot access memory at address 0x1f5f8df48000
(gdb)

I've checked with a printf and mono_get_lmf gets called countless times with no
apparent errors/bugs before the segfault. I tried running the build command
that failed in gdb, but that lead to very inconsistent results. Sometimes
the program would complete normally, sometimes catch SIGSEGV (in a different
place), sometimes catch SIGABRT elsewhere.

In mini-runtime.c, jit_tls is obtained via mono_tls_get_jit_tls which in the
end calls pthread_getspecific(3) (in mono-tls.h). It is unclear to me why it
would call an unaccessible memory address. The key is created by
pthread_key_create(3), also in mono-tls.h.

It appears that finding out why mono_get_lmf segfaults might be a way to
restore mono to working condition. It otherwise has been running mostly stable
with these changes (with SIGUSR1 and cooperative gc via MONO_ENABLE_GC env var),
including hour-long application test sessions and building C# projects with
xbuild. Not sure if this behavior is indicative of a race condition, or an edge
case that is regularly triggered.

Any advice on how to get to the bottom of this?