From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Jack Yu <jack.yu@realtek.com>,
"broonie@kernel.org" <broonie@kernel.org>,
"lgirdwood@gmail.com" <lgirdwood@gmail.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
"lars@metafoo.de" <lars@metafoo.de>,
"Flove(HsinFu)" <flove@realtek.com>,
"Oder Chiou" <oder_chiou@realtek.com>,
"Shuming [范書銘]" <shumingf@realtek.com>,
"Derek [方德義]" <derek.fang@realtek.com>,
"Bard Liao" <yung-chuan.liao@linux.intel.com>
Subject: Re: [PATCH v2] ASoC: rt721-sdca: Add RT721 SDCA driver
Date: Tue, 24 Sep 2024 11:36:00 +0200 [thread overview]
Message-ID: <65339ec1-ee9d-40cb-acd2-b6cfa0445c7e@linux.intel.com> (raw)
In-Reply-To: <1538ca2e1df042a7b771cc387b438710@realtek.com>
On 9/24/24 11:03, Jack Yu wrote:
> This is the initial codec driver for rt721-sdca.
I wouldn't hurt to provide a short description. 722 has 3 functions,
what about 721?
> + case SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT721_SDCA_ENT_USER_FU05, RT721_SDCA_CTL_FU_VOLUME,
> + CH_L):
> + case SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT721_SDCA_ENT_USER_FU05, RT721_SDCA_CTL_FU_VOLUME,
> + CH_R):
> + case SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT721_SDCA_ENT_USER_FU0F, RT721_SDCA_CTL_FU_VOLUME,
> + CH_L):
> + case SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT721_SDCA_ENT_USER_FU0F, RT721_SDCA_CTL_FU_VOLUME,
> + CH_R):
> + case SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT721_SDCA_ENT_PLATFORM_FU44,
> + RT721_SDCA_CTL_FU_CH_GAIN, CH_L):
> + case SDW_SDCA_CTL(FUNC_NUM_JACK_CODEC, RT721_SDCA_ENT_PLATFORM_FU44,
> + RT721_SDCA_CTL_FU_CH_GAIN, CH_R):
> + case SDW_SDCA_CTL(FUNC_NUM_MIC_ARRAY, RT721_SDCA_ENT_USER_FU1E, RT721_SDCA_CTL_FU_VOLUME,
> + CH_01):
> + case SDW_SDCA_CTL(FUNC_NUM_MIC_ARRAY, RT721_SDCA_ENT_USER_FU1E, RT721_SDCA_CTL_FU_VOLUME,
> + CH_02):
> + case SDW_SDCA_CTL(FUNC_NUM_MIC_ARRAY, RT721_SDCA_ENT_USER_FU1E, RT721_SDCA_CTL_FU_VOLUME,
> + CH_03):
> + case SDW_SDCA_CTL(FUNC_NUM_MIC_ARRAY, RT721_SDCA_ENT_USER_FU1E, RT721_SDCA_CTL_FU_VOLUME,
> + CH_04):
> + case SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_USER_FU06, RT721_SDCA_CTL_FU_VOLUME, CH_L):
> + case SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_USER_FU06, RT721_SDCA_CTL_FU_VOLUME, CH_R):
> + return true;
That tells us it has 3 functions (jack, mic, amp), so what's different
to RT722? could we have a single driver for both parts? A lot of this
driver seems copy-pasted-renamed.
> +static int rt721_sdca_read_prop(struct sdw_slave *slave)
> +{
> + struct sdw_slave_prop *prop = &slave->prop;
> + int nval;
> + int i, j;
> + u32 bit;
> + unsigned long addr;
> + struct sdw_dpn_prop *dpn;
> +
> + sdw_slave_read_prop(slave);
I thought we agreed to use a helper to read only the number of lanes
from platform firmware?
Bard, did you share this already?
> + prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY;
> + prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY;
> +
> + prop->paging_support = true;
> +
> + /*
> + * port = 1 for headphone playback
> + * port = 2 for headset-mic capture
> + * port = 3 for speaker playback
> + * port = 6 for digital-mic capture
> + */
> + prop->source_ports = BIT(6) | BIT(2); /* BITMAP: 01000100 */
> + prop->sink_ports = BIT(3) | BIT(1); /* BITMAP: 00001010 */
> +
> + nval = hweight32(prop->source_ports);
> + prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
> + sizeof(*prop->src_dpn_prop), GFP_KERNEL);
> + if (!prop->src_dpn_prop)
> + return -ENOMEM;
> +
> + i = 0;
> + dpn = prop->src_dpn_prop;
> + addr = prop->source_ports;
> + for_each_set_bit(bit, &addr, 32) {
> + dpn[i].num = bit;
> + dpn[i].type = SDW_DPN_FULL;
> + dpn[i].simple_ch_prep_sm = true;
> + dpn[i].ch_prep_timeout = 10;
> + i++;
> + }
> +
> + /* do this again for sink now */
> + nval = hweight32(prop->sink_ports);
> + prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
> + sizeof(*prop->sink_dpn_prop), GFP_KERNEL);
> + if (!prop->sink_dpn_prop)
> + return -ENOMEM;
> +
> + j = 0;
> + dpn = prop->sink_dpn_prop;
> + addr = prop->sink_ports;
> + for_each_set_bit(bit, &addr, 32) {
> + dpn[j].num = bit;
> + dpn[j].type = SDW_DPN_FULL;
> + dpn[j].simple_ch_prep_sm = true;
> + dpn[j].ch_prep_timeout = 10;
> + j++;
> + }
> +
> + /* set the timeout values */
> + prop->clk_stop_timeout = 200;
> +
> + /* wake-up event */
> + prop->wake_capable = 1;
> +
> + /* Three data lanes are supported by rt721-sdca codec */
> + prop->lane_control_support = true;
this doesn't seem needed if we already read information on lane support.
> +static void rt721_sdca_dmic_preset(struct rt721_sdca_priv *rt721)
> +{
> + /* Power down group1/2/3_mic_pdb */
> + rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,
> + RT721_MISC_POWER_CTL31, 0x8000);
> + /* VREF_HV_EN_AUTO_FAST */
> + rt721_sdca_index_write(rt721, RT721_ANA_POW_PART,
> + RT721_VREF1_HV_CTRL1, 0xe000);
> + /* Power up group1/2/3_mic_pdb */
> + rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,
> + RT721_MISC_POWER_CTL31, 0x8007);
> + /* Set AD07/08 power entity floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_ENT_FLOAT_CTL9, 0x2a2a);
> + /* Set AD10 power entity floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_ENT_FLOAT_CTL10, 0x2a00);
> + /* Set DMIC1/DMIC2 power entity floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_ENT_FLOAT_CTL6, 0x2a2a);
> + /* Set DMIC1/DMIC2 IT entity floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_ENT_FLOAT_CTL5, 0x2626);
> + /* Set AD10 FU entity floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_ENT_FLOAT_CTL8, 0x1e00);
> + /* Set DMIC1/DMIC2 FU input gain floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_ENT_FLOAT_CTL7, 0x1515);
> + /* Set DMIC2 FU input gain channel floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_CH_FLOAT_CTL3, 0x0304);
> + /* Set AD10 FU channel floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_CH_FLOAT_CTL4, 0x0304);
> + /* vf71f_r12_07_06 and vf71f_r13_07_06 = 2’b00 */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_HDA_LEGACY_CTL1, 0x0000);
> + /* Enable vf707_r12_05/vf707_r13_05 */
> + regmap_write(rt721->regmap,
> + SDW_SDCA_CTL(FUNC_NUM_MIC_ARRAY, RT721_SDCA_ENT_IT26,
> + RT721_SDCA_CTL_VENDOR_DEF, 0), 0x01);
> + /* Set usd_flag_sel, usd_in_sel */
> + regmap_write(rt721->mbq_regmap, 0x5910009, 0x2e01);
> + /* Set RC calibration */
> + rt721_sdca_index_write(rt721, RT721_RC_CALIB_CTRL,
> + RT721_RC_CALIB_CTRL0, 0x0b00);
> + rt721_sdca_index_write(rt721, RT721_RC_CALIB_CTRL,
> + RT721_RC_CALIB_CTRL0, 0x0b40);
> + /* Fine tune PDE2A latency */
> + regmap_write(rt721->regmap, 0x2f5c, 0x25);
> +}
Humm, isn't all this supposed to be handled with blind writes? It seems
odd to hard-code all this, no?
> +static void rt721_sdca_amp_preset(struct rt721_sdca_priv *rt721)
> +{;
> + /* Power down group1/2/3_mic_pdb */
> + rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,
> + RT721_MISC_POWER_CTL31, 0x8000);
> + /* VREF_HV_EN_AUTO_FAST */
> + rt721_sdca_index_write(rt721, RT721_ANA_POW_PART,
> + RT721_VREF1_HV_CTRL1, 0xe000);
> + /* Power up group1/2/3_mic_pdb */
> + rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,
> + RT721_MISC_POWER_CTL31, 0x8007);
> + /* Reset dc_cal_top */
> + regmap_write(rt721->mbq_regmap, 0x5810000, 0x6420);
> + /* Turn back to normal dc_cal_top */
> + regmap_write(rt721->mbq_regmap, 0x5810000, 0x6421);
> + /* W1C Trigger Calibration */
> + regmap_write(rt721->mbq_regmap, 0x5810000, 0xe421);
> + /* DAC04 FU entity floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_CH_FLOAT_CTL6, 0x5561);
> + /* Set EAPD high */
> + rt721_sdca_index_write(rt721, RT721_VENDOR_REG,
> + RT721_GPIO_PAD_CTRL5, 0x8003);
> + /* Enable vf707_r14 */
> + regmap_write(rt721->regmap,
> + SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_OT23,
> + RT721_SDCA_CTL_VENDOR_DEF, 0), 0x04);
> + /* FU 23 SPK mute control - L */
> + regmap_write(rt721->regmap,
> + SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_PDE23,
> + RT721_SDCA_CTL_FU_MUTE, CH_01), 0x00);
> + /* FU 23 SPK mute control - R */
> + regmap_write(rt721->regmap,
> + SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_PDE23,
> + RT721_SDCA_CTL_FU_MUTE, CH_02), 0x00);
> + /* FU 55 DAC04 mute control - L */
> + regmap_write(rt721->regmap,
> + SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_FU55,
> + RT721_SDCA_CTL_FU_MUTE, CH_01), 0x00);
> + /* FU 55 DAC04 mute control - R */
> + regmap_write(rt721->regmap,
> + SDW_SDCA_CTL(FUNC_NUM_AMP, RT721_SDCA_ENT_FU55,
> + RT721_SDCA_CTL_FU_MUTE, CH_02), 0x00);
> +}
> +
> +static void rt721_sdca_jack_preset(struct rt721_sdca_priv *rt721)
> +{
> + /* Power down group1/2/3_mic_pdb */
> + rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,
> + RT721_MISC_POWER_CTL31, 0x8000);
> + /* VREF_HV_EN_AUTO_FAST */
> + rt721_sdca_index_write(rt721, RT721_ANA_POW_PART,
> + RT721_VREF1_HV_CTRL1, 0xe000);
> + /* Power up group1/2/3_mic_pdb */
> + rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,
> + RT721_MISC_POWER_CTL31, 0x8007);
> + /* GE0 mode related control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_GE_REL_CTRL1, 0x8011);
> + /* Button A, B, C, D bypass mode */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_UMP_HID_CTRL3, 0xcf00);
> + /* HID1 slot enable */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_UMP_HID_CTRL4, 0x000f);
> + /* Report ID for HID1 */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_UMP_HID_CTRL1, 0x1100);
> + /* OSC/OOC for slot 2, 3 */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_UMP_HID_CTRL5, 0x0c12);
> + /* Set JD de-bounce clock control */
> + rt721_sdca_index_write(rt721, RT721_JD_CTRL,
> + RT721_JD_1PIN_GAT_CTRL2, 0xc002);
> + /* RC calibration -1 */
> + rt721_sdca_index_write(rt721, RT721_RC_CALIB_CTRL,
> + RT721_RC_CALIB_CTRL0, 0x0b00);
> + /* RC calibration -2 */
> + rt721_sdca_index_write(rt721, RT721_RC_CALIB_CTRL,
> + RT721_RC_CALIB_CTRL0, 0x0b40);
> + /* pow_clk_12p288mhz_dre03 change to register mode */
> + rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,
> + RT721_UAJ_TOP_TCON14, 0x3333);
> + /* Tune calibration timing control */
> + regmap_write(rt721->mbq_regmap, 0x5810035, 0x0036);
> + /* calibration HP amp output select control from Efuse */
> + regmap_write(rt721->mbq_regmap, 0x5810030, 0xee00);
> + /* FSM related control */
> + rt721_sdca_index_write(rt721, RT721_CAP_PORT_CTRL,
> + RT721_HP_AMP_2CH_CAL1, 0x0140);
> + /* HP calibration related control */
> + regmap_write(rt721->mbq_regmap, 0x5810000, 0x0021);
> + /* W1C HP calibration*/
> + regmap_write(rt721->mbq_regmap, 0x5810000, 0x8021);
> + /* reg_sel_cin_hp_0010/0011 */
> + rt721_sdca_index_write(rt721, RT721_CAP_PORT_CTRL,
> + RT721_HP_AMP_2CH_CAL18, 0x5522);
> + regmap_write(rt721->mbq_regmap, 0x5b10007, 0x2000);
> + /* sel_sensing_lr_hp */
> + regmap_write(rt721->mbq_regmap, 0x5B10017, 0x1b0f);
> + /* Release HP-JD */
> + rt721_sdca_index_write(rt721, RT721_CBJ_CTRL,
> + RT721_CBJ_A0_GAT_CTRL1, 0x2a02);
> + /* en_osw gating auto done bit */
> + rt721_sdca_index_write(rt721, RT721_CAP_PORT_CTRL,
> + RT721_HP_AMP_2CH_CAL4, 0xa105);
> + /* pow_clk_en_sw_amp_detect_sel to register mode */
> + rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,
> + RT721_UAJ_TOP_TCON14, 0x3b33);
> + /* cp_sw_hp to auto mode */
> + regmap_write(rt721->mbq_regmap, 0x310400, 0x3023);
> + /* pow_clk_en_sw_amp_detect power up */
> + rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,
> + RT721_UAJ_TOP_TCON14, 0x3f33);
> + rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,
> + RT721_UAJ_TOP_TCON13, 0x6048);
> + /* switch size detect threshold */
> + regmap_write(rt721->mbq_regmap, 0x310401, 0x3000);
> + regmap_write(rt721->mbq_regmap, 0x310402, 0x1b00);
> + /* en_hp_amp_detect auto mode */
> + regmap_write(rt721->mbq_regmap, 0x310300, 0x000f);
> + /* amp detect threshold */
> + regmap_write(rt721->mbq_regmap, 0x310301, 0x3000);
> + regmap_write(rt721->mbq_regmap, 0x310302, 0x1b00);
> + /* gating_sdw_link_rst_n_1_cbj_reg */
> + rt721_sdca_index_write(rt721, RT721_VENDOR_ANA_CTL,
> + RT721_UAJ_TOP_TCON17, 0x0008);
> + /* CKXEN_SDAC chopper function */
> + rt721_sdca_index_write(rt721, RT721_DAC_CTRL,
> + RT721_DAC_2CH_CTRL3, 0x55ff);
> + /* CKXSEL_SDAC chopper frequency */
> + rt721_sdca_index_write(rt721, RT721_DAC_CTRL,
> + RT721_DAC_2CH_CTRL4, 0xcc00);
> + /* Bias current for SDAC */
> + rt721_sdca_index_write(rt721, RT721_ANA_POW_PART,
> + RT721_MBIAS_LV_CTRL2, 0x6677);
> + /* VREF2 level selection */
> + rt721_sdca_index_write(rt721, RT721_ANA_POW_PART,
> + RT721_VREF2_LV_CTRL1, 0x7600);
> + /* ADC09/MIC2 power entity floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_ENT_FLOAT_CTL2, 0x1234);
> + /* LINE2 power entity floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_ENT_FLOAT_CTL3, 0x3512);
> + /* DAC03/HP power entity floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_ENT_FLOAT_CTL1, 0x4040);
> + /* ADC27 power entity floating control */
> + rt721_sdca_index_write(rt721, RT721_HDA_SDCA_FLOAT,
> + RT721_ENT_FLOAT_CTL4, 0x1201);
> + /* Fine tune PDE40 latency */
> + regmap_write(rt721->regmap, 0x2f58, 0x07);
> +}
same here, shouldn't this come from ACPI/blind write tables?
> +enum rt721_sdca_jd_src {
> + RT721_JD_NULL,
> + RT721_JD1,
> + RT721_JD2,
> +};
is this supported in SDCA? I can't recall seeing this for other drivers.
next prev parent reply other threads:[~2024-09-24 9:37 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-24 9:03 [PATCH v2] ASoC: rt721-sdca: Add RT721 SDCA driver Jack Yu
2024-09-24 9:36 ` Pierre-Louis Bossart [this message]
2024-09-24 11:54 ` Jack Yu
2024-09-24 12:11 ` Mark Brown
2024-09-24 15:26 ` Pierre-Louis Bossart
2024-09-25 4:03 ` Jack Yu
2024-09-25 4:07 ` Jack Yu
2024-09-24 14:03 ` Liao, Bard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=65339ec1-ee9d-40cb-acd2-b6cfa0445c7e@linux.intel.com \
--to=pierre-louis.bossart@linux.intel.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=derek.fang@realtek.com \
--cc=flove@realtek.com \
--cc=jack.yu@realtek.com \
--cc=lars@metafoo.de \
--cc=lgirdwood@gmail.com \
--cc=oder_chiou@realtek.com \
--cc=shumingf@realtek.com \
--cc=yung-chuan.liao@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.