diff mbox series

ALSA: hda: Add new CM9825 driver

Message ID 20240830084521.15706-1-antivirus621@gmail.com (mailing list archive)
State New
Headers show
Series ALSA: hda: Add new CM9825 driver | expand

Commit Message

Leo Tsai Aug. 30, 2024, 8:45 a.m. UTC
From: Leo Tsai <leo.tsai@cmedia.com.tw>

Add new CM9825 driver with standard and NCR model.

Signed-off-by: Leo Tsai <leo.tsai@cmedia.com.tw>
---
 sound/pci/hda/patch_cmedia.c | 646 ++++++++++++++++++++++++++++++++++-
 1 file changed, 643 insertions(+), 3 deletions(-)

Comments

Takashi Iwai Aug. 30, 2024, 10:20 a.m. UTC | #1
On Fri, 30 Aug 2024 10:45:21 +0200,
Leo Tsai wrote:
> 
> From: Leo Tsai <leo.tsai@cmedia.com.tw>
> 
> Add new CM9825 driver with standard and NCR model.
> 
> Signed-off-by: Leo Tsai <leo.tsai@cmedia.com.tw>

Thanks for the patch.

But first of all, the patch description is too short and lack of info
in comparison with the code change you've added.  If it were a simple
quirk, it's OK, but this isn't such a case.  Ideally speaking, the
code should be understandable enough without looking at the too much
details in the datasheet.

That is, please give more information in both the patch description
and the comment in the code.

And, if there is some external reference, put the information such as
the URL of the spec sheet.

Also, what is NCR model?
i guess it's a certain hardware vendor or such, but it has to be
clarified.  And, the patch should be better split to two patches, one
implementing the standard behavior and another NCR-specific quirk.


In anyway, go for the more details:

> +static char CM9825_Standard_Drv_Ver[15] = { "0.240723.0" };
> +static char CM9825_NCR_Drv_Ver[15] = { "1.240805.0" };
> +
> +module_param_string(CM9825_Standard_Drv_Ver, CM9825_Standard_Drv_Ver,
> +		    sizeof(CM9825_Standard_Drv_Ver), 0444);
> +module_param_string(CM9825_NCR_Drv_Ver, CM9825_NCR_Drv_Ver,
> +		    sizeof(CM9825_NCR_Drv_Ver), 0444);

Don't put such stuff.  Those are purely downstream info, and have
nothing to do with the upstream stuff.

>  struct cmi_spec {
>  	struct hda_gen_spec gen;
> +	const struct hda_verb *chip_D0_verbs;
> +	const struct hda_verb *chip_D3_verbs;
> +	const struct hda_verb *chip_playback_start_verbs;
> +	const struct hda_verb *chip_playback_stop_verbs;
> +	const struct hda_verb *chip_HP_Present_verbs;
> +	const struct hda_verb *chip_HP_Remove_verbs;

The variable, function and field names are usually in lower letters.

> +	struct hda_codec *codec;
> +	struct delayed_work unsol_hp_work;
> +	int quirk;
> +	unsigned int playback_started:1;
> +};
> +
> +static const struct hda_verb cm9825_NCR_TpCon_verbs[] = {

Ditto.

> +	{0x01, 0x720, 0xF0},
> +	{0x01, 0x721, 0x88},
> +	{0x01, 0x722, 0x43},
> +	{0x01, 0x723, 0x10},
> +	{0x34, 0x70C, 0x02},
> +	{0x36, 0x71E, 0x11},
(snip)

Please try to use AC_VERB_* for the parameters.
I see many of the verbs are undefined ones, i.e. vendor-specific, and
they can be better defined locally.  It makes much more
understandable, and it can help debugging.

For example, the verb 0x71e is AC_VERB_SET_CONFIG_DEFAULT_BYTES_2.
But you change only this.  Why?  The intention can be commented.

> +static const struct hda_verb cm9825_std_playback_start_verbs[] = {
> +	{}
> +};
> +
> +static const struct hda_verb cm9825_std_playback_stop_verbs[] = {
> +	{}
> +};

Those can be set NULL instead?

> +
> +/*
> + * CM9825 quirks table
> + */
> +enum {
> +	QUIRK_NONE,
> +	QUIRK_CM9825_STANDARD,
> +	QUIRK_CM9825_NCR,
> +};
> +
> +static const struct snd_pci_quirk cm9825_quirks[] = {
> +	SND_PCI_QUIRK(0x13F6, 0x9825, "Cmedia Standard", QUIRK_CM9825_STANDARD),
> +	{}
> +};

Do you really need this quirk entry?  Why not just taking the STANDARD
as default?

> +static hda_nid_t cmi_get_hp_pin(struct cmi_spec *spec)
> +{
> +	if (spec->gen.autocfg.hp_pins[0]) {
> +		codec_dbg(spec->codec, "hp_pin 0x%X\n",
> +			  spec->gen.autocfg.hp_pins[0]);
> +		return spec->gen.autocfg.hp_pins[0];
> +	}
> +	return 0;
> +}

Simply refer to spec->gen.autocfg.hp_pins[0] in the caller side
instead.  There is little merit of factoring out as a function.

> +static void cm9825_unsol_hp_delayed(struct work_struct *work)
> +{
> +	struct cmi_spec *spec =
> +	    container_of(to_delayed_work(work), struct cmi_spec, unsol_hp_work);
> +	struct hda_jack_tbl *jack;
> +	hda_nid_t hp_pin = cmi_get_hp_pin(spec);
> +	bool hp_jack_plugin = false;
> +	int err = 0;
> +
> +	hp_jack_plugin = snd_hda_jack_detect(spec->codec, hp_pin);
> +
> +	codec_dbg(spec->codec, "hp_jack_plugin %d, hp_pin 0x%X\n",
> +		  (int)hp_jack_plugin, hp_pin);
> +
> +	if (!hp_jack_plugin) {
> +		err = snd_hda_codec_write(spec->codec, 0x42, 0, 0x707, 0x40);

Again, try to use the AC_VERB_*.

> +static void CM9825_init_hook(struct hda_codec *codec)
> +{
> +	struct cmi_spec *spec = codec->spec;
> +
> +	codec_dbg(spec->codec, "Start\n");
> +
> +	snd_hda_sequence_write(codec, spec->chip_D0_verbs);
> +}

You don't have to make an extra function but just add the init verbs
via snd_hda_add_verbs() at the probe time.

> +static void cm9825_playback_pcm_hook(struct hda_pcm_stream *hinfo,
> +				     struct hda_codec *codec,
> +				     struct snd_pcm_substream *substream,
> +				     int action)
> +{
> +	struct cmi_spec *spec = codec->spec;
> +
> +	codec_dbg(codec, "start, action %d\n", action);
> +
> +	switch (action) {
> +	case HDA_GEN_PCM_ACT_PREPARE:
> +		spec->playback_started = 1;

This flag is never referred.  Can be dropped?

> +static int CM9825_init(struct hda_codec *codec)

Again, use lower letters.

> +#ifdef CONFIG_PM

This ifdef is no longer needed for the latest upstream code.

> +static int cm9825_suspend(struct hda_codec *codec)
> +{
> +	struct cmi_spec *spec = codec->spec;
> +
> +	codec_dbg(codec, "Start\n");

Better to be a more understandable debug print :)

> +static void cm9825_detect_quirk(struct hda_codec *codec)
> +{
> +	struct cmi_spec *spec = codec->spec;
> +
> +	switch (codec->core.subsystem_id) {
> +	case 0x104316E2:
> +		spec->quirk = QUIRK_CM9825_STANDARD;
> +		break;
> +	case 0x104388F0:
> +		spec->quirk = QUIRK_CM9825_NCR;
> +		break;
> +	default:
> +		spec->quirk = QUIRK_CM9825_STANDARD;
> +		break;
> +	}

Those can be simply put in the quirk table instead.
The codec SSID is also checked at the quirk table lookup.


> +static u32 get_amp_max_value(struct hda_codec *codec, hda_nid_t nid, int dir,
> +			     unsigned int ofs)
> +{
> +	u32 caps = query_amp_caps(codec, nid, dir);
> +	/* get num steps */
> +	caps = (caps & AC_AMPCAP_NUM_STEPS) >> AC_AMPCAP_NUM_STEPS_SHIFT;
> +	if (ofs < caps)
> +		caps -= ofs;
> +	return caps;
> +}
> +
> +static inline int
> +update_amp_value(struct hda_codec *codec, hda_nid_t nid,
> +		 int ch, int dir, int idx, unsigned int ofs, unsigned int val)
> +{
> +	unsigned int maxval;
> +
> +	if (val > 0)
> +		val += ofs;
> +	/* ofs = 0: raw max value */
> +	maxval = get_amp_max_value(codec, nid, dir, 0);
> +	if (val > maxval)
> +		val = maxval;
> +	return snd_hda_codec_amp_update(codec, nid, ch, dir, idx,
> +					HDA_AMP_VOLMASK, val);
> +}
> +
> +static int cm9825_ncr_spk_vol_put(struct snd_kcontrol *kcontrol,
> +				  struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> +	hda_nid_t nid = get_amp_nid(kcontrol);
> +	int chs = get_amp_channels(kcontrol);
> +	int dir = get_amp_direction(kcontrol);
> +	int idx = get_amp_index(kcontrol);
> +	unsigned int ofs = get_amp_offset(kcontrol);
> +	long *valp = ucontrol->value.integer.value;
> +	int change = 0;
> +
> +	codec_dbg(codec, "nid 0x%X, chs %d, dir %d, *valp %ld\n",
> +		  nid, chs, dir, *valp);
> +
> +	if (chs & 1) {
> +		change = update_amp_value(codec, nid, 0, dir, idx, ofs, *valp);
> +		update_amp_value(codec, 0x38, 0, dir, idx, ofs, *valp);

What this mirroring to the node 0x38 at all?  This has to be clarified.

> +static int patch_cm9825(struct hda_codec *codec)
> +{
> +	/* Detect codec quirk */
> +	quirk = snd_pci_quirk_lookup(codec->bus->pci, cm9825_quirks);
> +	if (quirk)
> +		spec->quirk = quirk->value;
> +	else
> +		spec->quirk = QUIRK_CM9825_STANDARD;
> +
> +	if (spec->quirk == QUIRK_CM9825_STANDARD)
> +		cm9825_detect_quirk(codec);
> +
> +	codec_dbg(spec->codec, "spec->quirk %d\n", spec->quirk);

So this quirk lookup is weird.  Just put all needed entries in the
quirk table and do a single lookup.

> +	codec->patch_ops = cmi_auto_patch_ops;
> +	codec->patch_ops.init = CM9825_init;
> +#ifdef CONFIG_PM

The ifdef is superfluous.

> +	if (spec->quirk == (int)QUIRK_CM9825_STANDARD) {
> +		INIT_DELAYED_WORK(&spec->unsol_hp_work,
> +				  cm9825_unsol_hp_delayed);

Better to initialize the work unconditionally.
Then you don't have to differentiate at suspend or free, and you can
call cancel_delayed_work_sync() no matter whether it's used or not.

> @@ -91,8 +729,8 @@ static int patch_cmi8888(struct hda_codec *codec)
>  	if (get_defcfg_device(snd_hda_codec_get_pincfg(codec, 0x10)) ==
>  	    AC_JACK_HP_OUT) {
>  		static const struct snd_kcontrol_new amp_kctl =
> -			HDA_CODEC_VOLUME("Headphone Amp Playback Volume",
> -					 0x10, 0, HDA_OUTPUT);
> +		    HDA_CODEC_VOLUME("Headphone Amp Playback Volume",
> +				     0x10, 0, HDA_OUTPUT);

This is a superfluous change?


thanks,

Takashi
kernel test robot Aug. 31, 2024, 3:40 p.m. UTC | #2
Hi Leo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tiwai-sound/for-next]
[also build test WARNING on tiwai-sound/for-linus linus/master v6.11-rc5 next-20240830]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Leo-Tsai/ALSA-hda-Add-new-CM9825-driver/20240830-164709
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
patch link:    https://lore.kernel.org/r/20240830084521.15706-1-antivirus621%40gmail.com
patch subject: [PATCH] ALSA: hda: Add new CM9825 driver
config: i386-buildonly-randconfig-002-20240831 (https://download.01.org/0day-ci/archive/20240831/202408312351.eV8p77QU-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240831/202408312351.eV8p77QU-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408312351.eV8p77QU-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> sound/pci/hda/patch_cmedia.c:572:12: warning: variable 'spec' is uninitialized when used here [-Wuninitialized]
     572 |         codec_dbg(spec->codec, "Start\n");
         |                   ^~~~
   sound/pci/hda/hda_local.h:735:24: note: expanded from macro 'codec_dbg'
     735 |         dev_dbg(hda_codec_dev(codec), fmt, ##args)
         |                               ^~~~~
   include/sound/hda_codec.h:289:32: note: expanded from macro 'hda_codec_dev'
     289 | #define hda_codec_dev(_dev)     (&(_dev)->core.dev)
         |                                    ^~~~
   include/linux/dev_printk.h:165:18: note: expanded from macro 'dev_dbg'
     165 |         dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                         ^~~
   note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/dynamic_debug.h:250:59: note: expanded from macro '_dynamic_func_call'
     250 |         _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
         |                                                                  ^~~~~~~~~~~
   include/linux/dynamic_debug.h:248:65: note: expanded from macro '_dynamic_func_call_cls'
     248 |         __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
         |                                                                        ^~~~~~~~~~~
   include/linux/dynamic_debug.h:224:15: note: expanded from macro '__dynamic_func_call_cls'
     224 |                 func(&id, ##__VA_ARGS__);                       \
         |                             ^~~~~~~~~~~
   sound/pci/hda/patch_cmedia.c:567:23: note: initialize the variable 'spec' to silence this warning
     567 |         struct cmi_spec *spec;
         |                              ^
         |                               = NULL
   1 warning generated.


vim +/spec +572 sound/pci/hda/patch_cmedia.c

   564	
   565	static int patch_cm9825(struct hda_codec *codec)
   566	{
   567		struct cmi_spec *spec;
   568		struct auto_pin_cfg *cfg;
   569		int err, i;
   570		const struct snd_pci_quirk *quirk;
   571	
 > 572		codec_dbg(spec->codec, "Start\n");
   573	
   574		spec = kzalloc(sizeof(*spec), GFP_KERNEL);
   575		if (spec == NULL)
   576			return -ENOMEM;
   577	
   578		codec->spec = spec;
   579		spec->codec = codec;
   580	
   581		/* Detect codec quirk */
   582		quirk = snd_pci_quirk_lookup(codec->bus->pci, cm9825_quirks);
   583		if (quirk)
   584			spec->quirk = quirk->value;
   585		else
   586			spec->quirk = QUIRK_CM9825_STANDARD;
   587	
   588		if (spec->quirk == QUIRK_CM9825_STANDARD)
   589			cm9825_detect_quirk(codec);
   590	
   591		codec_dbg(spec->codec, "spec->quirk %d\n", spec->quirk);
   592	
   593		codec->patch_ops = cmi_auto_patch_ops;
   594		codec->patch_ops.init = CM9825_init;
   595	#ifdef CONFIG_PM
   596		codec->patch_ops.suspend = cm9825_suspend;
   597		codec->patch_ops.resume = cm9825_resume;
   598		codec->patch_ops.check_power_status = snd_hda_gen_check_power_status;
   599	#endif
   600		spec->gen.init_hook = CM9825_init_hook;
   601		/* add hooks */
   602		spec->gen.pcm_playback_hook = cm9825_playback_pcm_hook;
   603	
   604		cfg = &spec->gen.autocfg;
   605		snd_hda_gen_spec_init(&spec->gen);
   606	
   607		if (spec->quirk == (int)QUIRK_CM9825_STANDARD) {
   608			snd_hda_codec_set_name(codec, "CM9825 Standard");
   609			spec->chip_D0_verbs = cm9825_D0_verbs;
   610			spec->chip_D3_verbs = cm9825_D3_verbs;
   611			spec->chip_HP_Present_verbs = cm9825_HP_Present_verbs;
   612			spec->chip_HP_Remove_verbs = cm9825_HP_Remove_verbs;
   613			spec->chip_playback_start_verbs =
   614			    cm9825_std_playback_start_verbs;
   615			spec->chip_playback_stop_verbs = cm9825_std_playback_stop_verbs;
   616		} else if (spec->quirk == (int)QUIRK_CM9825_NCR) {
   617	#ifdef CONFIG_PM
   618			codec->patch_ops.suspend = cm9825_NCR_suspend;
   619			codec->patch_ops.resume = cm9825_NCR_resume;
   620			codec->patch_ops.check_power_status =
   621			    snd_hda_gen_check_power_status;
   622	#endif
   623			snd_hda_codec_set_name(codec, "CM9825 NCR");
   624			snd_hda_sequence_write(codec, cm9825_NCR_TpCon_verbs);
   625			spec->chip_D0_verbs = cm9825_NCR_D0_verbs;
   626			spec->chip_D3_verbs = cm9825_NCR_D3_verbs;
   627			spec->chip_playback_start_verbs =
   628			    cm9825_NCR_playback_start_verbs;
   629			spec->chip_playback_stop_verbs = cm9825_NCR_playback_stop_verbs;
   630	
   631			for (i = 0; i < ARRAY_SIZE(cm9825_ncr_mixer); i++) {
   632				err = snd_hda_add_new_ctls(codec, &cm9825_ncr_mixer[i]);
   633				if (err < 0) {
   634					codec_info(codec, "add new ctls fail: %d\n",
   635						   err);
   636					goto error;
   637				}
   638			}
   639		} else {
   640			snd_hda_codec_set_name(codec, "CM9825 Standard");
   641			spec->chip_D0_verbs = cm9825_D0_verbs;
   642			spec->chip_D3_verbs = cm9825_D3_verbs;
   643			spec->chip_HP_Present_verbs = cm9825_HP_Present_verbs;
   644			spec->chip_HP_Remove_verbs = cm9825_HP_Remove_verbs;
   645			spec->chip_playback_start_verbs =
   646			    cm9825_std_playback_start_verbs;
   647			spec->chip_playback_stop_verbs = cm9825_std_playback_stop_verbs;
   648		}
   649	
   650		err = snd_hda_parse_pin_defcfg(codec, cfg, NULL, 0);
   651		if (err < 0)
   652			goto error;
   653		err = snd_hda_gen_parse_auto_config(codec, cfg);
   654		if (err < 0)
   655			goto error;
   656	
   657		if (spec->quirk == (int)QUIRK_CM9825_STANDARD) {
   658			INIT_DELAYED_WORK(&spec->unsol_hp_work,
   659					  cm9825_unsol_hp_delayed);
   660			cm9825_setup_unsol(codec);
   661		}
   662	
   663		return 0;
   664	
   665	 error:
   666		snd_hda_gen_free(codec);
   667	
   668		codec_info(codec, "Enter err %d\n", err);
   669	
   670		return err;
   671	}
   672
diff mbox series

Patch

diff --git a/sound/pci/hda/patch_cmedia.c b/sound/pci/hda/patch_cmedia.c
index 2ddd33f8dd6c..c25bd0e74879 100644
--- a/sound/pci/hda/patch_cmedia.c
+++ b/sound/pci/hda/patch_cmedia.c
@@ -17,8 +17,538 @@ 
 #include "hda_jack.h"
 #include "hda_generic.h"
 
+static char CM9825_Standard_Drv_Ver[15] = { "0.240723.0" };
+static char CM9825_NCR_Drv_Ver[15] = { "1.240805.0" };
+
+module_param_string(CM9825_Standard_Drv_Ver, CM9825_Standard_Drv_Ver,
+		    sizeof(CM9825_Standard_Drv_Ver), 0444);
+module_param_string(CM9825_NCR_Drv_Ver, CM9825_NCR_Drv_Ver,
+		    sizeof(CM9825_NCR_Drv_Ver), 0444);
+
 struct cmi_spec {
 	struct hda_gen_spec gen;
+	const struct hda_verb *chip_D0_verbs;
+	const struct hda_verb *chip_D3_verbs;
+	const struct hda_verb *chip_playback_start_verbs;
+	const struct hda_verb *chip_playback_stop_verbs;
+	const struct hda_verb *chip_HP_Present_verbs;
+	const struct hda_verb *chip_HP_Remove_verbs;
+	struct hda_codec *codec;
+	struct delayed_work unsol_hp_work;
+	int quirk;
+	unsigned int playback_started:1;
+};
+
+static const struct hda_verb cm9825_NCR_TpCon_verbs[] = {
+	{0x01, 0x720, 0xF0},
+	{0x01, 0x721, 0x88},
+	{0x01, 0x722, 0x43},
+	{0x01, 0x723, 0x10},
+	{0x34, 0x70C, 0x02},
+	{0x36, 0x71E, 0x11},
+	{0x43, 0x7A2, 0x8C},
+	{0x43, 0x7A9, 0x76},
+	{0x43, 0x7AA, 0x60},
+	{0x43, 0x7AD, 0xC0},
+	{0x43, 0x7B0, 0xF7},
+	{0x43, 0x7B1, 0xE1},
+	{0x43, 0x7B2, 0xCD},
+	{0x43, 0x7AA, 0xE0},
+	{0x43, 0x7A9, 0xF6},
+	{0x3C, 0x3A0, 0x2D},
+	{0x3C, 0x390, 0x2D},
+	{0x46, 0x7EF, 0x64},
+	{0x46, 0x78A, 0x40},
+	{0x46, 0x78B, 0x10},
+	{0x46, 0x78C, 0x06},
+	{0x46, 0x7B8, 0x2D},
+	{0x46, 0x7B9, 0x2D},
+	{0x46, 0x7BA, 0x83},
+	{}
+};
+
+static const struct hda_verb cm9825_NCR_D3_verbs[] = {
+	/* chip sleep verbs */
+	{0x43, 0x7A9, 0x62},
+	{0x43, 0x7A0, 0x01},
+	{0x43, 0x7A1, 0xC2},
+	{0x43, 0x7A2, 0x00},
+	{0x43, 0x7A3, 0x02},
+	{0x43, 0x7A8, 0x50},
+	{0x43, 0x7A4, 0x00},
+	{0x43, 0x7AC, 0x04},
+	{0x43, 0x7B0, 0xF6},
+	{0x43, 0x7B2, 0xCD},
+	{}
+};
+
+static const struct hda_verb cm9825_NCR_D0_verbs[] = {
+	/* chip init verbs */
+	{0x34, 0x70C, 0x02},
+	{0x43, 0x7B6, 0x30},
+	{0x43, 0x7A0, 0x00},
+	{0x43, 0x7A2, 0x00},
+	{0x43, 0x7A3, 0x02},
+	{0x43, 0x7A4, 0x00},
+	{0x43, 0x7A8, 0x56},
+	{0x43, 0x7A9, 0x62},
+	{0x43, 0x7AA, 0x00},
+	{0x43, 0x7AC, 0x0C},
+	{0x43, 0x7AD, 0xC4},
+	{0x43, 0x7B0, 0xF4},
+	{0x43, 0x7B2, 0xCD},
+	{0x43, 0x7B1, 0x61},
+	{0x43, 0x7B3, 0x33},
+	{0x43, 0x7B4, 0x07},
+	{0x43, 0x7B5, 0x26},
+	{0x3C, 0x3A0, 0x2D},
+	{0x3C, 0x390, 0x2D},
+	{0x43, 0x781, 0x40},
+	{0x43, 0x785, 0x40},
+	{0x3C, 0x341, 0x80},
+	{0x01, 0x720, 0xF0},
+	{0x01, 0x721, 0x88},
+	{0x01, 0x715, 0x01},
+	{0x01, 0x717, 0x01},
+	{0x3C, 0x340, 0x00},
+	{}
+};
+
+static const struct hda_verb cm9825_std_playback_start_verbs[] = {
+	{}
+};
+
+static const struct hda_verb cm9825_std_playback_stop_verbs[] = {
+	{}
+};
+
+static const struct hda_verb cm9825_NCR_playback_start_verbs[] = {
+	{0x43, 0x7A3, 0xAE},
+	{0x43, 0x7A9, 0xF2},
+	{0x43, 0x7AD, 0xC4},
+	{0x43, 0x7AA, 0xE0},
+	{}
+};
+
+static const struct hda_verb cm9825_NCR_playback_stop_verbs[] = {
+	{0x43, 0x7AD, 0xC0},
+	{0x43, 0x7A9, 0x62},
+	{0x43, 0x7AD, 0x80},
+	{0x43, 0x7AA, 0x00},
+	{0x43, 0x7A3, 0x02},
+	{}
+};
+
+static const struct hda_verb cm9825_D3_verbs[] = {
+	/* chip sleep verbs */
+	{0x43, 0x7A9, 0x62},
+	{0x43, 0x7A0, 0x01},
+	{0x43, 0x7A1, 0xC2},
+	{0x43, 0x7A2, 0x00},
+	{0x43, 0x7A3, 0x02},
+	{0x43, 0x7A8, 0x50},
+	{0x43, 0x7A4, 0x00},
+	{0x43, 0x7AC, 0x04},
+	{0x43, 0x7B0, 0xF6},
+	{0x43, 0x7B2, 0xCD},
+	{}
+};
+
+static const struct hda_verb cm9825_D0_verbs[] = {
+	/* chip init verbs */
+	{0x34, 0x70C, 0x02},
+	{0x43, 0x7B6, 0x30},
+	{0x43, 0x7A0, 0x00},
+	{0x43, 0x7A2, 0x00},
+	{0x43, 0x7A3, 0x02},
+	{0x43, 0x7A4, 0x00},
+	{0x43, 0x7A8, 0x56},
+	{0x43, 0x7A9, 0x62},
+	{0x43, 0x7AA, 0x00},
+	{0x43, 0x7AC, 0x0C},
+	{0x43, 0x7AD, 0x80},
+	{0x43, 0x7B0, 0xF4},
+	{0x43, 0x7B2, 0xCD},
+	{0x43, 0x7B1, 0x61},
+	{0x43, 0x7B3, 0x33},
+	{0x43, 0x7B4, 0x07},
+	{0x43, 0x7B5, 0x26},
+	{0x3C, 0x3A0, 0x2D},
+	{0x3C, 0x390, 0x2D},
+	{0x43, 0x781, 0x40},
+	{0x43, 0x785, 0x40},
+	{}
+};
+
+static const struct hda_verb cm9825_HP_Present_verbs[] = {
+	{0x42, 0x707, 0x00},
+	{0x43, 0x7A2, 0x88},
+	{0x43, 0x7A3, 0xAA},
+	{0x43, 0x7A4, 0x10},
+	{0x43, 0x7A9, 0xF2},
+	{0x43, 0x7AA, 0x00},
+	{0x43, 0x7AD, 0xC4},
+	{}
+};
+
+static const struct hda_verb cm9825_HP_Remove_verbs[] = {
+	{0x43, 0x7A2, 0x00},
+	{0x43, 0x7A3, 0x56},
+	{0x43, 0x7A4, 0x00},
+	{0x43, 0x7A9, 0x62},
+	{0x43, 0x7AA, 0xE0},
+	{0x43, 0x7AD, 0x80},
+	{0x42, 0x707, 0x40},
+	{}
+};
+
+/*
+ * CM9825 quirks table
+ */
+enum {
+	QUIRK_NONE,
+	QUIRK_CM9825_STANDARD,
+	QUIRK_CM9825_NCR,
+};
+
+static const struct snd_pci_quirk cm9825_quirks[] = {
+	SND_PCI_QUIRK(0x13F6, 0x9825, "Cmedia Standard", QUIRK_CM9825_STANDARD),
+	{}
+};
+
+static hda_nid_t cmi_get_hp_pin(struct cmi_spec *spec)
+{
+	if (spec->gen.autocfg.hp_pins[0]) {
+		codec_dbg(spec->codec, "hp_pin 0x%X\n",
+			  spec->gen.autocfg.hp_pins[0]);
+		return spec->gen.autocfg.hp_pins[0];
+	}
+	return 0;
+}
+
+static void cm9825_unsol_hp_delayed(struct work_struct *work)
+{
+	struct cmi_spec *spec =
+	    container_of(to_delayed_work(work), struct cmi_spec, unsol_hp_work);
+	struct hda_jack_tbl *jack;
+	hda_nid_t hp_pin = cmi_get_hp_pin(spec);
+	bool hp_jack_plugin = false;
+	int err = 0;
+
+	hp_jack_plugin = snd_hda_jack_detect(spec->codec, hp_pin);
+
+	codec_dbg(spec->codec, "hp_jack_plugin %d, hp_pin 0x%X\n",
+		  (int)hp_jack_plugin, hp_pin);
+
+	if (!hp_jack_plugin) {
+		err = snd_hda_codec_write(spec->codec, 0x42, 0, 0x707, 0x40);
+		if (err)
+			codec_dbg(spec->codec, "codec_write err %d\n", err);
+
+		snd_hda_sequence_write(spec->codec, spec->chip_HP_Remove_verbs);
+	} else {
+		snd_hda_sequence_write(spec->codec,
+				       spec->chip_HP_Present_verbs);
+	}
+
+	jack = snd_hda_jack_tbl_get(spec->codec, hp_pin);
+	if (jack) {
+		jack->block_report = 0;
+		snd_hda_jack_report_sync(spec->codec);
+	}
+}
+
+static void hp_callback(struct hda_codec *codec, struct hda_jack_callback *cb)
+{
+	struct cmi_spec *spec = codec->spec;
+	struct hda_jack_tbl *tbl;
+
+	/* Delay enabling the HP amp, to let the mic-detection
+	 * state machine run.
+	 */
+
+	codec_dbg(spec->codec, "cb->nid 0x%X\n", cb->nid);
+
+	tbl = snd_hda_jack_tbl_get(codec, cb->nid);
+	if (tbl)
+		tbl->block_report = 1;
+	schedule_delayed_work(&spec->unsol_hp_work, msecs_to_jiffies(200));
+}
+
+static void cm9825_setup_unsol(struct hda_codec *codec)
+{
+	struct cmi_spec *spec = codec->spec;
+
+	hda_nid_t hp_pin = cmi_get_hp_pin(spec);
+
+	snd_hda_jack_detect_enable_callback(codec, hp_pin, hp_callback);
+}
+
+static void CM9825_init_hook(struct hda_codec *codec)
+{
+	struct cmi_spec *spec = codec->spec;
+
+	codec_dbg(spec->codec, "Start\n");
+
+	snd_hda_sequence_write(codec, spec->chip_D0_verbs);
+}
+
+static void cm9825_playback_pcm_hook(struct hda_pcm_stream *hinfo,
+				     struct hda_codec *codec,
+				     struct snd_pcm_substream *substream,
+				     int action)
+{
+	struct cmi_spec *spec = codec->spec;
+
+	codec_dbg(codec, "start, action %d\n", action);
+
+	switch (action) {
+	case HDA_GEN_PCM_ACT_PREPARE:
+		spec->playback_started = 1;
+		snd_hda_sequence_write(codec, spec->chip_playback_start_verbs);
+		break;
+	case HDA_GEN_PCM_ACT_CLEANUP:
+		spec->playback_started = 0;
+		snd_hda_sequence_write(codec, spec->chip_playback_stop_verbs);
+		break;
+	default:
+		return;
+	}
+}
+
+#define cmi_is_s3_resume(codec) \
+	((codec)->core.dev.power.power_state.event == PM_EVENT_RESUME)
+#define cmi_is_s4_resume(codec) \
+	((codec)->core.dev.power.power_state.event == PM_EVENT_RESTORE)
+
+static int CM9825_init(struct hda_codec *codec)
+{
+	codec_dbg(codec, "Start\n");
+
+	snd_hda_gen_init(codec);
+	snd_hda_apply_fixup(codec, HDA_FIXUP_ACT_INIT);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int cm9825_suspend(struct hda_codec *codec)
+{
+	struct cmi_spec *spec = codec->spec;
+
+	codec_dbg(codec, "Start\n");
+
+	cancel_delayed_work_sync(&spec->unsol_hp_work);
+
+	snd_hda_sequence_write(codec, spec->chip_D3_verbs);
+
+	return 0;
+}
+
+static int cm9825_NCR_suspend(struct hda_codec *codec)
+{
+	struct cmi_spec *spec = codec->spec;
+
+	codec_dbg(codec, "Start\n");
+
+	snd_hda_sequence_write(codec, spec->chip_D3_verbs);
+
+	return 0;
+}
+
+static int cm9825_resume(struct hda_codec *codec)
+{
+	struct cmi_spec *spec = codec->spec;
+	hda_nid_t hp_pin = 0;
+	bool hp_jack_plugin = false;
+	int err;
+
+	codec_dbg(spec->codec, "Start\n");
+
+	err = snd_hda_codec_write(spec->codec, 0x42, 0, 0x707, 0x00);
+	if (err)
+		codec_dbg(codec, "codec_write err %d\n", err);
+
+	/* hibernation resume needs the full chip initialization */
+	if (cmi_is_s4_resume(codec))
+		codec_dbg(spec->codec, "resume from S4\n");
+
+	msleep(150);
+
+	codec->patch_ops.init(codec);
+
+	hp_pin = cmi_get_hp_pin(spec);
+	hp_jack_plugin = snd_hda_jack_detect(spec->codec, hp_pin);
+
+	codec_dbg(spec->codec, "hp_jack_plugin %d, hp_pin 0x%X\n",
+		  (int)hp_jack_plugin, hp_pin);
+
+	if (!hp_jack_plugin) {
+		err = snd_hda_codec_write(spec->codec, 0x42, 0, 0x707, 0x40);
+
+		if (err)
+			codec_dbg(codec, "codec_write err %d\n", err);
+
+		snd_hda_sequence_write(codec, cm9825_HP_Remove_verbs);
+	}
+
+	snd_hda_regmap_sync(codec);
+	hda_call_check_power_status(codec, 0x01);
+
+	return 0;
+}
+
+static int cm9825_NCR_resume(struct hda_codec *codec)
+{
+	struct cmi_spec *spec = codec->spec;
+
+	codec_dbg(spec->codec, "Start\n");
+
+	/* hibernation resume needs the full chip initialization */
+	if (cmi_is_s4_resume(codec))
+		codec_dbg(spec->codec, "resume from S4\n");
+
+	codec->patch_ops.init(codec);
+
+	snd_hda_regmap_sync(codec);
+	hda_call_check_power_status(codec, 0x01);
+
+	return 0;
+}
+#endif
+
+static void cm9825_detect_quirk(struct hda_codec *codec)
+{
+	struct cmi_spec *spec = codec->spec;
+
+	switch (codec->core.subsystem_id) {
+	case 0x104316E2:
+		spec->quirk = QUIRK_CM9825_STANDARD;
+		break;
+	case 0x104388F0:
+		spec->quirk = QUIRK_CM9825_NCR;
+		break;
+	default:
+		spec->quirk = QUIRK_CM9825_STANDARD;
+		break;
+	}
+}
+
+static u32 get_amp_max_value(struct hda_codec *codec, hda_nid_t nid, int dir,
+			     unsigned int ofs)
+{
+	u32 caps = query_amp_caps(codec, nid, dir);
+	/* get num steps */
+	caps = (caps & AC_AMPCAP_NUM_STEPS) >> AC_AMPCAP_NUM_STEPS_SHIFT;
+	if (ofs < caps)
+		caps -= ofs;
+	return caps;
+}
+
+static inline int
+update_amp_value(struct hda_codec *codec, hda_nid_t nid,
+		 int ch, int dir, int idx, unsigned int ofs, unsigned int val)
+{
+	unsigned int maxval;
+
+	if (val > 0)
+		val += ofs;
+	/* ofs = 0: raw max value */
+	maxval = get_amp_max_value(codec, nid, dir, 0);
+	if (val > maxval)
+		val = maxval;
+	return snd_hda_codec_amp_update(codec, nid, ch, dir, idx,
+					HDA_AMP_VOLMASK, val);
+}
+
+static int cm9825_ncr_spk_vol_put(struct snd_kcontrol *kcontrol,
+				  struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
+	hda_nid_t nid = get_amp_nid(kcontrol);
+	int chs = get_amp_channels(kcontrol);
+	int dir = get_amp_direction(kcontrol);
+	int idx = get_amp_index(kcontrol);
+	unsigned int ofs = get_amp_offset(kcontrol);
+	long *valp = ucontrol->value.integer.value;
+	int change = 0;
+
+	codec_dbg(codec, "nid 0x%X, chs %d, dir %d, *valp %ld\n",
+		  nid, chs, dir, *valp);
+
+	if (chs & 1) {
+		change = update_amp_value(codec, nid, 0, dir, idx, ofs, *valp);
+		update_amp_value(codec, 0x38, 0, dir, idx, ofs, *valp);
+		valp++;
+	}
+	if (chs & 2) {
+		change |= update_amp_value(codec, nid, 1, dir, idx, ofs, *valp);
+		update_amp_value(codec, 0x38, 1, dir, idx, ofs, *valp);
+	}
+	return change;
+}
+
+static int cm9825_ncr_switch_put(struct snd_kcontrol *kcontrol,
+				 struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
+	hda_nid_t nid = get_amp_nid(kcontrol);
+	int chs = get_amp_channels(kcontrol);
+	int dir = get_amp_direction(kcontrol);
+	int idx = get_amp_index(kcontrol);
+	long *valp = ucontrol->value.integer.value;
+	int change = 0;
+
+	codec_dbg(codec, "nid 0x%X, chs %d, dir %d, *valp %ld\n",
+		  nid, chs, dir, *valp);
+
+	if (chs & 1) {
+		change = snd_hda_codec_amp_update(codec, nid, 0, dir, idx,
+						  HDA_AMP_MUTE,
+						  *valp ? 0 : HDA_AMP_MUTE);
+		snd_hda_codec_amp_update(codec, 0x38, 0, dir, idx,
+					 HDA_AMP_MUTE,
+					 *valp ? 0 : HDA_AMP_MUTE);
+		valp++;
+	}
+	if (chs & 2) {
+		change |= snd_hda_codec_amp_update(codec, nid, 1, dir, idx,
+						   HDA_AMP_MUTE,
+						   *valp ? 0 : HDA_AMP_MUTE);
+		snd_hda_codec_amp_update(codec, 0x38, 1, dir, idx,
+					 HDA_AMP_MUTE,
+					 *valp ? 0 : HDA_AMP_MUTE);
+	}
+	hda_call_check_power_status(codec, nid);
+	return change;
+}
+
+#define CM9825_NCR_CODEC_VOL(xname, nid, channel, dir) \
+	{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
+	  .name = xname, \
+	  .subdevice = HDA_SUBDEV_AMP_FLAG, \
+	  .access = SNDRV_CTL_ELEM_ACCESS_READWRITE | \
+			SNDRV_CTL_ELEM_ACCESS_TLV_READ | \
+			SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK, \
+	  .info = snd_hda_mixer_amp_volume_info, \
+	  .get = snd_hda_mixer_amp_volume_get, \
+	  .put = cm9825_ncr_spk_vol_put, \
+	  .tlv = { .c = snd_hda_mixer_amp_tlv }, \
+	  .private_value = HDA_COMPOSE_AMP_VAL(nid, channel, 0, dir) }
+
+#define CM9825_NCR_CODEC_MUTE(xname, nid, channel, dir) \
+	{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
+	  .name = xname, \
+	  .subdevice = HDA_SUBDEV_AMP_FLAG, \
+	  .info = snd_hda_mixer_amp_switch_info, \
+	  .get = snd_hda_mixer_amp_switch_get, \
+	  .put = cm9825_ncr_switch_put, \
+	  .private_value = HDA_COMPOSE_AMP_VAL(nid, channel, 0, dir) }
+
+static const struct snd_kcontrol_new cm9825_ncr_mixer[] = {
+	CM9825_NCR_CODEC_VOL("Master Playback Volume", 0x3C, 3, HDA_OUTPUT),
+	CM9825_NCR_CODEC_MUTE("Master Playback Switch", 0x3C, 3, HDA_OUTPUT),
+	{}
 };
 
 /*
@@ -32,6 +562,114 @@  static const struct hda_codec_ops cmi_auto_patch_ops = {
 	.unsol_event = snd_hda_jack_unsol_event,
 };
 
+static int patch_cm9825(struct hda_codec *codec)
+{
+	struct cmi_spec *spec;
+	struct auto_pin_cfg *cfg;
+	int err, i;
+	const struct snd_pci_quirk *quirk;
+
+	codec_dbg(spec->codec, "Start\n");
+
+	spec = kzalloc(sizeof(*spec), GFP_KERNEL);
+	if (spec == NULL)
+		return -ENOMEM;
+
+	codec->spec = spec;
+	spec->codec = codec;
+
+	/* Detect codec quirk */
+	quirk = snd_pci_quirk_lookup(codec->bus->pci, cm9825_quirks);
+	if (quirk)
+		spec->quirk = quirk->value;
+	else
+		spec->quirk = QUIRK_CM9825_STANDARD;
+
+	if (spec->quirk == QUIRK_CM9825_STANDARD)
+		cm9825_detect_quirk(codec);
+
+	codec_dbg(spec->codec, "spec->quirk %d\n", spec->quirk);
+
+	codec->patch_ops = cmi_auto_patch_ops;
+	codec->patch_ops.init = CM9825_init;
+#ifdef CONFIG_PM
+	codec->patch_ops.suspend = cm9825_suspend;
+	codec->patch_ops.resume = cm9825_resume;
+	codec->patch_ops.check_power_status = snd_hda_gen_check_power_status;
+#endif
+	spec->gen.init_hook = CM9825_init_hook;
+	/* add hooks */
+	spec->gen.pcm_playback_hook = cm9825_playback_pcm_hook;
+
+	cfg = &spec->gen.autocfg;
+	snd_hda_gen_spec_init(&spec->gen);
+
+	if (spec->quirk == (int)QUIRK_CM9825_STANDARD) {
+		snd_hda_codec_set_name(codec, "CM9825 Standard");
+		spec->chip_D0_verbs = cm9825_D0_verbs;
+		spec->chip_D3_verbs = cm9825_D3_verbs;
+		spec->chip_HP_Present_verbs = cm9825_HP_Present_verbs;
+		spec->chip_HP_Remove_verbs = cm9825_HP_Remove_verbs;
+		spec->chip_playback_start_verbs =
+		    cm9825_std_playback_start_verbs;
+		spec->chip_playback_stop_verbs = cm9825_std_playback_stop_verbs;
+	} else if (spec->quirk == (int)QUIRK_CM9825_NCR) {
+#ifdef CONFIG_PM
+		codec->patch_ops.suspend = cm9825_NCR_suspend;
+		codec->patch_ops.resume = cm9825_NCR_resume;
+		codec->patch_ops.check_power_status =
+		    snd_hda_gen_check_power_status;
+#endif
+		snd_hda_codec_set_name(codec, "CM9825 NCR");
+		snd_hda_sequence_write(codec, cm9825_NCR_TpCon_verbs);
+		spec->chip_D0_verbs = cm9825_NCR_D0_verbs;
+		spec->chip_D3_verbs = cm9825_NCR_D3_verbs;
+		spec->chip_playback_start_verbs =
+		    cm9825_NCR_playback_start_verbs;
+		spec->chip_playback_stop_verbs = cm9825_NCR_playback_stop_verbs;
+
+		for (i = 0; i < ARRAY_SIZE(cm9825_ncr_mixer); i++) {
+			err = snd_hda_add_new_ctls(codec, &cm9825_ncr_mixer[i]);
+			if (err < 0) {
+				codec_info(codec, "add new ctls fail: %d\n",
+					   err);
+				goto error;
+			}
+		}
+	} else {
+		snd_hda_codec_set_name(codec, "CM9825 Standard");
+		spec->chip_D0_verbs = cm9825_D0_verbs;
+		spec->chip_D3_verbs = cm9825_D3_verbs;
+		spec->chip_HP_Present_verbs = cm9825_HP_Present_verbs;
+		spec->chip_HP_Remove_verbs = cm9825_HP_Remove_verbs;
+		spec->chip_playback_start_verbs =
+		    cm9825_std_playback_start_verbs;
+		spec->chip_playback_stop_verbs = cm9825_std_playback_stop_verbs;
+	}
+
+	err = snd_hda_parse_pin_defcfg(codec, cfg, NULL, 0);
+	if (err < 0)
+		goto error;
+	err = snd_hda_gen_parse_auto_config(codec, cfg);
+	if (err < 0)
+		goto error;
+
+	if (spec->quirk == (int)QUIRK_CM9825_STANDARD) {
+		INIT_DELAYED_WORK(&spec->unsol_hp_work,
+				  cm9825_unsol_hp_delayed);
+		cm9825_setup_unsol(codec);
+	}
+
+	return 0;
+
+ error:
+	snd_hda_gen_free(codec);
+
+	codec_info(codec, "Enter err %d\n", err);
+
+	return err;
+}
+
 static int patch_cmi9880(struct hda_codec *codec)
 {
 	struct cmi_spec *spec;
@@ -91,8 +729,8 @@  static int patch_cmi8888(struct hda_codec *codec)
 	if (get_defcfg_device(snd_hda_codec_get_pincfg(codec, 0x10)) ==
 	    AC_JACK_HP_OUT) {
 		static const struct snd_kcontrol_new amp_kctl =
-			HDA_CODEC_VOLUME("Headphone Amp Playback Volume",
-					 0x10, 0, HDA_OUTPUT);
+		    HDA_CODEC_VOLUME("Headphone Amp Playback Volume",
+				     0x10, 0, HDA_OUTPUT);
 		if (!snd_hda_gen_add_kctl(&spec->gen, NULL, &amp_kctl)) {
 			err = -ENOMEM;
 			goto error;
@@ -113,8 +751,10 @@  static const struct hda_device_id snd_hda_id_cmedia[] = {
 	HDA_CODEC_ENTRY(0x13f68888, "CMI8888", patch_cmi8888),
 	HDA_CODEC_ENTRY(0x13f69880, "CMI9880", patch_cmi9880),
 	HDA_CODEC_ENTRY(0x434d4980, "CMI9880", patch_cmi9880),
-	{} /* terminator */
+	HDA_CODEC_ENTRY(0x13f69825, "CM9825", patch_cm9825),
+	{}			/* terminator */
 };
+
 MODULE_DEVICE_TABLE(hdaudio, snd_hda_id_cmedia);
 
 MODULE_LICENSE("GPL");