| Message ID | 1525407594-25644-1-git-send-email-conmanx360@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Sorry for sending these twice, I made a formatting mistake in the first series, and they would not apply properly. Hopefully these do not show up as spam because of this. I went through and fixed them all individually and re-committed them, but kept the same commit messages. I still have a lot to learn. Thanks, Connor On Fri, May 4, 2018 at 12:19 AM, Connor McAdams <conmanx360@gmail.com> wrote: > This patch adds PCI quirk ID's for the Sound Blaster Z and Recon3Di. > Only the currently tested ID's have been added. > > This patch also adds the ability to load alternative firmwares for each > card, the firmwares can be obtained from within the Windows driver. > The Recon3Di uses "ctefx-r3di.bin" and the Sound Blaster Z uses > "ctefx-sbz.bin". If the alternative firmware for the given quirk is not > found, the original ctefx.bin will be used. This has been confirmed to > work for both the R3Di and the SBZ. > > This patch also makes the character array *dirstr a const. > > Signed-off-by: Connor McAdams <conmanx360@gmail.com> > --- > sound/pci/hda/patch_ca0132.c | 61 +++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 55 insertions(+), 6 deletions(-) > > diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c > index 768ea86..8346100 100644 > --- a/sound/pci/hda/patch_ca0132.c > +++ b/sound/pci/hda/patch_ca0132.c > @@ -72,12 +72,16 @@ > #define SCP_GET 1 > > #define EFX_FILE "ctefx.bin" > +#define SBZ_EFX_FILE "ctefx-sbz.bin" > +#define R3DI_EFX_FILE "ctefx-r3di.bin" > > #ifdef CONFIG_SND_HDA_CODEC_CA0132_DSP > MODULE_FIRMWARE(EFX_FILE); > +MODULE_FIRMWARE(SBZ_EFX_FILE); > +MODULE_FIRMWARE(R3DI_EFX_FILE); > #endif > > -static char *dirstr[2] = { "Playback", "Capture" }; > +static const char *dirstr[2] = { "Playback", "Capture" }; > > enum { > SPEAKER_OUT, > @@ -734,6 +738,7 @@ struct ca0132_spec { > unsigned int scp_resp_header; > unsigned int scp_resp_data[4]; > unsigned int scp_resp_count; > + bool alt_firmware_present; > > /* mixer and effects related */ > unsigned char dmic_ctl; > @@ -762,6 +767,8 @@ struct ca0132_spec { > enum { > QUIRK_NONE, > QUIRK_ALIENWARE, > + QUIRK_SBZ, > + QUIRK_R3DI, > }; > > static const struct hda_pintbl alienware_pincfgs[] = { > @@ -782,6 +789,10 @@ static const struct snd_pci_quirk ca0132_quirks[] = { > SND_PCI_QUIRK(0x1028, 0x0685, "Alienware 15 2015", QUIRK_ALIENWARE), > SND_PCI_QUIRK(0x1028, 0x0688, "Alienware 17 2015", QUIRK_ALIENWARE), > SND_PCI_QUIRK(0x1028, 0x0708, "Alienware 15 R2 2016", QUIRK_ALIENWARE), > + SND_PCI_QUIRK(0x1102, 0x0010, "Sound Blaster Z", QUIRK_SBZ), > + SND_PCI_QUIRK(0x1102, 0x0023, "Sound Blaster Z", QUIRK_SBZ), > + SND_PCI_QUIRK(0x1458, 0xA016, "Recon3Di", QUIRK_R3DI), > + SND_PCI_QUIRK(0x1458, 0xA036, "Recon3Di", QUIRK_R3DI), > {} > }; > > @@ -3207,7 +3218,7 @@ static int ca0132_select_out(struct hda_codec *codec) > pin_ctl & ~PIN_HP); > /* enable speaker node */ > pin_ctl = snd_hda_codec_read(codec, spec->out_pins[0], 0, > - AC_VERB_GET_PIN_WIDGET_CONTROL, 0); > + AC_VERB_GET_PIN_WIDGET_CONTROL, 0); > snd_hda_set_pin_ctl(codec, spec->out_pins[0], > pin_ctl | PIN_OUT); > } else { > @@ -4370,11 +4381,49 @@ static void ca0132_set_dsp_msr(struct hda_codec *codec, bool is96k) > static bool ca0132_download_dsp_images(struct hda_codec *codec) > { > bool dsp_loaded = false; > + struct ca0132_spec *spec = codec->spec; > const struct dsp_image_seg *dsp_os_image; > const struct firmware *fw_entry; > - > - if (request_firmware(&fw_entry, EFX_FILE, codec->card->dev) != 0) > - return false; > + /* > + * Alternate firmwares for different variants. The Recon3Di apparently > + * can use the default firmware, but I'll leave the option in case > + * it needs it again. > + */ > + switch (spec->quirk) { > + case QUIRK_SBZ: > + if (request_firmware(&fw_entry, SBZ_EFX_FILE, > + codec->card->dev) != 0) { > + codec_dbg(codec, "SBZ alt firmware not detected. "); > + spec->alt_firmware_present = false; > + } else { > + codec_dbg(codec, "Sound Blaster Z firmware selected."); > + spec->alt_firmware_present = true; > + } > + break; > + case QUIRK_R3DI: > + if (request_firmware(&fw_entry, R3DI_EFX_FILE, > + codec->card->dev) != 0) { > + codec_dbg(codec, "Recon3Di alt firmware not detected."); > + spec->alt_firmware_present = false; > + } else { > + codec_dbg(codec, "Recon3Di firmware selected."); > + spec->alt_firmware_present = true; > + } > + break; > + default: > + spec->alt_firmware_present = false; > + break; > + } > + /* > + * Use default ctefx.bin if no alt firmware is detected, or if none > + * exists for your particular codec. > + */ > + if (!spec->alt_firmware_present) { > + codec_dbg(codec, "Default firmware selected."); > + if (request_firmware(&fw_entry, EFX_FILE, > + codec->card->dev) != 0) > + return false; > + } > > dsp_os_image = (struct dsp_image_seg *)(fw_entry->data); > if (dspload_image(codec, dsp_os_image, 0, 0, true, 0)) { > @@ -4476,7 +4525,7 @@ static struct hda_verb ca0132_base_exit_verbs[] = { > {} > }; > > -/* Other verbs tables. Sends after DSP download. */ > +/* Other verbs tables. Sends after DSP download. */ > static struct hda_verb ca0132_init_verbs0[] = { > /* chip init verbs */ > {0x15, 0x70D, 0xF0}, > -- > 2.7.4 >
Hi, On May 04 2018 13:30, Connor McAdams wrote: > Sorry for sending these twice, I made a formatting mistake in the > first series, and they would not apply properly. Hopefully these do > not show up as spam because of this. > > I went through and fixed them all individually and re-committed them, > but kept the same commit messages. I still have a lot to learn. For this case, please use '--subject-prefix' option for git-format-patch. Like: $ git format-patch --subject-prefix='PATCH v2' Additionally, it's preferable to add 'ALSA: hda/ca0132: ' prefix for each patch. Like: 'ALSA: hda/ca0132: R3Di and SBZ quirk entires + alt firmware loading' Furthermore, when posting several patches as a one series, it's preferable to post cover letter as well. Like: $ git format-patch --cover-letter 0000-cover-letter.patch ... Of course, it's better to write enough information in the cover letter. For its content, please refer to cover letters by the other developers in alsa-devel archive. http://mailman.alsa-project.org/pipermail/alsa-devel/ Thanks Takashi Sakamoto
On Fri, 04 May 2018 06:19:44 +0200, Connor McAdams wrote: > > This patch adds PCI quirk ID's for the Sound Blaster Z and Recon3Di. > Only the currently tested ID's have been added. > > This patch also adds the ability to load alternative firmwares for each > card, the firmwares can be obtained from within the Windows driver. > The Recon3Di uses "ctefx-r3di.bin" and the Sound Blaster Z uses > "ctefx-sbz.bin". If the alternative firmware for the given quirk is not > found, the original ctefx.bin will be used. This has been confirmed to > work for both the R3Di and the SBZ. > > This patch also makes the character array *dirstr a const. > > Signed-off-by: Connor McAdams <conmanx360@gmail.com> I reply here as the patch series lacks of a cover letter, as already Sakamoto-san mentioned. I like the patches in general. It's a great effort for the long-time PITA. I already replied one minor thing I stumbled on. But the whole changes are specific to ca0132, so there is no big danger by the patch series itself per se. However, one thing to be fixed is the style you took in the series, "put some changes commented out for the next patch". I guess you made it in that way for ease of splitting patches. But of course it's messy when you follow the patch history as a reader. So please clean up them, move the new code to the new patch, and resubmit as v3. And, please give also a proper subject prefix and a cover letter, too :) Thanks! Takashi
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index 768ea86..8346100 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -72,12 +72,16 @@ #define SCP_GET 1 #define EFX_FILE "ctefx.bin" +#define SBZ_EFX_FILE "ctefx-sbz.bin" +#define R3DI_EFX_FILE "ctefx-r3di.bin" #ifdef CONFIG_SND_HDA_CODEC_CA0132_DSP MODULE_FIRMWARE(EFX_FILE); +MODULE_FIRMWARE(SBZ_EFX_FILE); +MODULE_FIRMWARE(R3DI_EFX_FILE); #endif -static char *dirstr[2] = { "Playback", "Capture" }; +static const char *dirstr[2] = { "Playback", "Capture" }; enum { SPEAKER_OUT, @@ -734,6 +738,7 @@ struct ca0132_spec { unsigned int scp_resp_header; unsigned int scp_resp_data[4]; unsigned int scp_resp_count; + bool alt_firmware_present; /* mixer and effects related */ unsigned char dmic_ctl; @@ -762,6 +767,8 @@ struct ca0132_spec { enum { QUIRK_NONE, QUIRK_ALIENWARE, + QUIRK_SBZ, + QUIRK_R3DI, }; static const struct hda_pintbl alienware_pincfgs[] = { @@ -782,6 +789,10 @@ static const struct snd_pci_quirk ca0132_quirks[] = { SND_PCI_QUIRK(0x1028, 0x0685, "Alienware 15 2015", QUIRK_ALIENWARE), SND_PCI_QUIRK(0x1028, 0x0688, "Alienware 17 2015", QUIRK_ALIENWARE), SND_PCI_QUIRK(0x1028, 0x0708, "Alienware 15 R2 2016", QUIRK_ALIENWARE), + SND_PCI_QUIRK(0x1102, 0x0010, "Sound Blaster Z", QUIRK_SBZ), + SND_PCI_QUIRK(0x1102, 0x0023, "Sound Blaster Z", QUIRK_SBZ), + SND_PCI_QUIRK(0x1458, 0xA016, "Recon3Di", QUIRK_R3DI), + SND_PCI_QUIRK(0x1458, 0xA036, "Recon3Di", QUIRK_R3DI), {} }; @@ -3207,7 +3218,7 @@ static int ca0132_select_out(struct hda_codec *codec) pin_ctl & ~PIN_HP); /* enable speaker node */ pin_ctl = snd_hda_codec_read(codec, spec->out_pins[0], 0, - AC_VERB_GET_PIN_WIDGET_CONTROL, 0); + AC_VERB_GET_PIN_WIDGET_CONTROL, 0); snd_hda_set_pin_ctl(codec, spec->out_pins[0], pin_ctl | PIN_OUT); } else { @@ -4370,11 +4381,49 @@ static void ca0132_set_dsp_msr(struct hda_codec *codec, bool is96k) static bool ca0132_download_dsp_images(struct hda_codec *codec) { bool dsp_loaded = false; + struct ca0132_spec *spec = codec->spec; const struct dsp_image_seg *dsp_os_image; const struct firmware *fw_entry; - - if (request_firmware(&fw_entry, EFX_FILE, codec->card->dev) != 0) - return false; + /* + * Alternate firmwares for different variants. The Recon3Di apparently + * can use the default firmware, but I'll leave the option in case + * it needs it again. + */ + switch (spec->quirk) { + case QUIRK_SBZ: + if (request_firmware(&fw_entry, SBZ_EFX_FILE, + codec->card->dev) != 0) { + codec_dbg(codec, "SBZ alt firmware not detected. "); + spec->alt_firmware_present = false; + } else { + codec_dbg(codec, "Sound Blaster Z firmware selected."); + spec->alt_firmware_present = true; + } + break; + case QUIRK_R3DI: + if (request_firmware(&fw_entry, R3DI_EFX_FILE, + codec->card->dev) != 0) { + codec_dbg(codec, "Recon3Di alt firmware not detected."); + spec->alt_firmware_present = false; + } else { + codec_dbg(codec, "Recon3Di firmware selected."); + spec->alt_firmware_present = true; + } + break; + default: + spec->alt_firmware_present = false; + break; + } + /* + * Use default ctefx.bin if no alt firmware is detected, or if none + * exists for your particular codec. + */ + if (!spec->alt_firmware_present) { + codec_dbg(codec, "Default firmware selected."); + if (request_firmware(&fw_entry, EFX_FILE, + codec->card->dev) != 0) + return false; + } dsp_os_image = (struct dsp_image_seg *)(fw_entry->data); if (dspload_image(codec, dsp_os_image, 0, 0, true, 0)) { @@ -4476,7 +4525,7 @@ static struct hda_verb ca0132_base_exit_verbs[] = { {} }; -/* Other verbs tables. Sends after DSP download. */ +/* Other verbs tables. Sends after DSP download. */ static struct hda_verb ca0132_init_verbs0[] = { /* chip init verbs */ {0x15, 0x70D, 0xF0},
This patch adds PCI quirk ID's for the Sound Blaster Z and Recon3Di. Only the currently tested ID's have been added. This patch also adds the ability to load alternative firmwares for each card, the firmwares can be obtained from within the Windows driver. The Recon3Di uses "ctefx-r3di.bin" and the Sound Blaster Z uses "ctefx-sbz.bin". If the alternative firmware for the given quirk is not found, the original ctefx.bin will be used. This has been confirmed to work for both the R3Di and the SBZ. This patch also makes the character array *dirstr a const. Signed-off-by: Connor McAdams <conmanx360@gmail.com> --- sound/pci/hda/patch_ca0132.c | 61 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 6 deletions(-)