RE: [PATCH v2] ASoC: rt721-sdca: Add RT721 SDCA driver

[Jack Yu <jack.yu@realtek.com>]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Tuesday, September 24, 2024 8:11 PM
> To: Jack Yu <jack.yu@realtek.com>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>;
> lgirdwood@gmail.com; alsa-devel@alsa-project.org; 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
> 
> On Tue, Sep 24, 2024 at 11:54:25AM +0000, Jack Yu wrote:
> 
> > > 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.
> 
> > RT721 has 3 functions just like RT722, but it's still a different
> > codec and from internal discussion, we think it's better to separate the driver
> for future code management.
> 
> As I mentioned with the events it's possible there's some room for factoring
> out some of the code for the common bits that are shared between multiple
> devices.  Look at how Cirrus' Arizona drivers for example, each chip has
> specific support in a separate driver but there's a lot of shared code.
> 

Ok, we'll do the first version of common code for Realtek soundwire codec driver.

> > > > +     /* 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?
> 
> > It seems there is no api or function to support blind write from ACPI
> > from latest upstream code, and we think it's easier for us to manage the
> different function's blind write.
> 
> It's always possible for you to add shared code for things like parsing ACPI
> tables (any references to the spec for blind writes here?).
> 
> >
> > >
> > > > +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?
> > >
> > Same reply as above one.
> >
> > > > +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.
> > I'll remove this in next patch.