Re: simple pledge for xeyes(1)
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)
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
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
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?)
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
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?
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?
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?
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?
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
=== 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
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
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
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
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
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
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
= 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
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
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
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
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
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)
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)
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?