Re: after update to r368166: no sound recording

From: Matthias Apitz <guru_at_unixarea.de>
Date: Mon, 14 Dec 2020 10:16:21 +0100
I did a step by step down grading with 'svn up -r..... hdaa.c hdaa.h'
(only these two files), starting from r368166 down to the following revisions:

r368166: no recording from pcm1

r358333: no recording from pcm1

r350078: no recording from pcm1

r337043: recording is fine

I've cc'ed now the commiters of the r358333 and r350078. kaktus_at_ and sbruno_at_
please check the issue https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=251727
and this mail thread in current_at_

Thanks

	mattias

El día domingo, diciembre 13, 2020 a las 08:06:02p. m. +0100, Matthias Apitz escribió:

> El día viernes, diciembre 11, 2020 a las 08:06:27a. m. +0100, Matthias Apitz escribió:
> 
> > FreeBSD c720-r368166 13.0-CURRENT FreeBSD 13.0-CURRENT #1 r342378:368166M: Fri Dec 11 07:46:32 CET 2020     guru_at_c720-r368166:/usr/obj/usr/src/amd64.amd64/sys/GENERIC  amd64
> > 
> > and all is fine again as it was before. Someone with more knowledge should
> > have a look into a 'svn diff -r342378:368166 sys/dev/sound/pci/hda' and
> > see which of the changes might break the things. 
> > 
> 
> I did an analyzing of the files changed in sys/dev/sound/pci/hda between
> r342378 (2018-11-02) and r368166 (2020-11-30)
> 
> I copied over all files from hda/* to a new dir hda.r368166/* and
> updated (i.e. reverted) hda/* to r342378; only the 7 files below (of 10)
> have changed between r342378 and r368166:
> 
> The kernel built with hda of r342378 works fine;
> 
> The work plan was now to change one file after the other to r368166
> and see if the kernel still works fine, i.e. which of the 7 file(s)
> is causing the regression...
> 
> i.e. I did: 
> 
> # svn up -r342378 sys/dev/sound/pci/hda
> # cp sys/dev/sound/pci/hda.r368166/hdac.c sys/dev/sound/pci/hda/
> # touch sys/dev/sound/pci/hda/hdac.c 
> # make buildkernel -DNO_CLEAN
> # make installkernel
> ...
> 
>  v----------------------------an 'x' means: was copied from hda.r368166;
> [x] Index: hda/hdac.c
> [x] Index: hda/hdac.h
> 
> recording with the kernel still works
> 
> [x] Index: hda/hdaa_patches.c     
> 
> recording with the kernel still works
> 
> [x] Index: hda/hdacc.c
> [x] Index: hda/hda_reg.h          only cleanups of empty lines
> 
> recording with the kernel still works
> 
> [x] Index: hda/hdaa.h
> [x] Index: hda/hdaa.c             
> 
> recording with the kernel stoped working
> 
> Which means the few changes in hda/hdaa.c are causing the regression:
> 
> # svn diff -r342378 hdaa.c
> Index: hdaa.c
> ===================================================================
> --- hdaa.c	(revisión: 342378)
> +++ hdaa.c	(copia de trabajo)
> _at__at_ -52,7 +52,6 _at__at_
>  #define hdaa_lock(devinfo)	snd_mtxlock((devinfo)->lock)
>  #define hdaa_unlock(devinfo)	snd_mtxunlock((devinfo)->lock)
>  #define hdaa_lockassert(devinfo) snd_mtxassert((devinfo)->lock)
> -#define hdaa_lockowned(devinfo)	mtx_owned((devinfo)->lock)
>  
>  static const struct {
>  	const char *key;
> _at__at_ -1129,7 +1128,6 _at__at_
>  	    ((step - offset) * (size + 1)) / 4);
>  }
>  
> -
>  static int
>  hdaa_sysctl_caps(SYSCTL_HANDLER_ARGS)
>  {
> _at__at_ -5034,11 +5032,13 _at__at_
>  		pincap = w->wclass.pin.cap;
>  
>  		/* Disable everything. */
> -		w->wclass.pin.ctrl &= ~(
> -		    HDA_CMD_SET_PIN_WIDGET_CTRL_HPHN_ENABLE |
> -		    HDA_CMD_SET_PIN_WIDGET_CTRL_OUT_ENABLE |
> -		    HDA_CMD_SET_PIN_WIDGET_CTRL_IN_ENABLE |
> -		    HDA_CMD_SET_PIN_WIDGET_CTRL_VREF_ENABLE_MASK);
> +		if (devinfo->init_clear) {
> +			w->wclass.pin.ctrl &= ~(
> +		    	HDA_CMD_SET_PIN_WIDGET_CTRL_HPHN_ENABLE |
> +		    	HDA_CMD_SET_PIN_WIDGET_CTRL_OUT_ENABLE |
> +		    	HDA_CMD_SET_PIN_WIDGET_CTRL_IN_ENABLE |
> +		    	HDA_CMD_SET_PIN_WIDGET_CTRL_VREF_ENABLE_MASK);
> +		}
>  
>  		if (w->enable == 0) {
>  			/* Pin is unused so left it disabled. */
> _at__at_ -6669,8 +6669,12 _at__at_
>  	    devinfo, 0, hdaa_sysctl_gpo_config, "A", "GPO configuration");
>  	SYSCTL_ADD_PROC(device_get_sysctl_ctx(dev),
>  	    SYSCTL_CHILDREN(device_get_sysctl_tree(dev)), OID_AUTO,
> -	    "reconfig", CTLTYPE_INT | CTLFLAG_RW,
> +	    "reconfig", CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_NEEDGIANT,
>  	    dev, 0, hdaa_sysctl_reconfig, "I", "Reprocess configuration");
> +	SYSCTL_ADD_INT(device_get_sysctl_ctx(dev),
> +	    SYSCTL_CHILDREN(device_get_sysctl_tree(dev)), OID_AUTO,
> +	    "init_clear", CTLFLAG_RW,
> +	    &devinfo->init_clear, 1,"Clear initial pin widget configuration");
>  	bus_generic_attach(dev);
>  	return (0);
>  }
> 
> # svn diff -r342378 hdaa.h
> Index: hdaa.h
> ===================================================================
> --- hdaa.h	(revisión: 342378)
> +++ hdaa.h	(copia de trabajo)
> _at__at_ -214,6 +214,7 _at__at_
>  	struct hdaa_chan	*chans;
>  	struct callout		poll_jack;
>  	int			poll_ival;
> +	uint32_t		init_clear;
>  };
>  
>  #define HDAA_CHN_RUNNING	0x00000001
> 
> 
> If one looks into a svn log of the file hdaa.c it seems that one of the two changes 
> r358333 or r350078 must have caused the problem:
> 
> # svn log hdaa.c
> 
> ------------------------------------------------------------------------
> r365085 | mjg | 2020-09-01 23:27:34 +0200 (mar. 01 de sept. de 2020) | 2 líneas
> 
> sound: clean up empty lines in .c and .h files
> 
> ------------------------------------------------------------------------
> r360076 | emaste | 2020-04-18 20:25:30 +0200 (sáb. 18 de abr. de 2020) | 4 líneas
> 
> hda: remove hda*_lockowned macros
> 
> These are not used anywhere.
> 
> ------------------------------------------------------------------------
> r358333 | kaktus | 2020-02-26 15:26:36 +0100 (mié. 26 de feb. de 2020) | 16 líneas
> 
> Mark more nodes as CTLFLAG_MPSAFE or CTLFLAG_NEEDGIANT (17 of many)
> 
> r357614 added CTLFLAG_NEEDGIANT to make it easier to find nodes that are
> still not MPSAFE (or already are but aren’t properly marked).
> Use it in preparation for a general review of all nodes.
> 
> This is non-functional change that adds annotations to SYSCTL_NODE and
> SYSCTL_PROC nodes using one of the soon-to-be-required flags.
> 
> Mark all obvious cases as MPSAFE.  All entries that haven't been marked
> as MPSAFE before are by default marked as NEEDGIANT
> 
> Approved by:	kib (mentor, blanket)
> Commented by:	kib, gallatin, melifaro
> Differential Revision:	https://reviews.freebsd.org/D23718
> 
> ------------------------------------------------------------------------
> r350078 | sbruno | 2019-07-17 06:13:46 +0200 (mié. 17 de jul. de 2019) | 8 líneas
> 
> I add the ability to accept the default pin widget configuration to help
> with various laptops using hdaa(4) sound devices.  We don't seem to know
> the "correct" configurations for these devices and the defaults are far
> superiour, e.g. they work if you don't nuke the default configs.
> 
> PR:	200526
> Differential Revision:	https://reviews.freebsd.org/D17772
> 
> ------------------------------------------------------------------------
> r337043 | jhibbits | 2018-08-01 16:50:41 +0200 (mié. 01 de ago. de 2018) | 13 líneas
> 
> snd_hda: Synchronize DMA buffers for the control path
> 
> Make sure both sides of the DMA buffer memory accesses for the CORB and RIRB
> (control buffers) in snd_hda (device and CPU) can see coherent memory.  This
> is needed on weakly ordered architectures including PowerPC and ARM.  Patch
> originally by mmel, with small changes.
> 
> This does not cover the data path of snd_hda.  We don't have sync operations
> for in-progress DMA buffers, to sync ranges of a map.
> 
> Reviewed By: mmel
> Differential Revision: https://reviews.freebsd.org/D16517
> 
> ...
> 

-- 
Matthias Apitz, ✉ guru_at_unixarea.de, http://www.unixarea.de/ +49-176-38902045
Public GnuPG key: http://www.unixarea.de/key.pub
Received on Mon Dec 14 2020 - 08:16:29 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:41:26 UTC