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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> Sent: Tuesday, September 24, 2024 11:27 PM
> To: Mark Brown <broonie@xxxxxxxxxx>; Jack Yu <jack.yu@xxxxxxxxxxx>
> Cc: lgirdwood@xxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; lars@xxxxxxxxxx;
> Flove(HsinFu) <flove@xxxxxxxxxxx>; Oder Chiou <oder_chiou@xxxxxxxxxxx>;
> Shuming [范書銘] <shumingf@xxxxxxxxxxx>; Derek [方德義]
> <derek.fang@xxxxxxxxxxx>; Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx>;
> Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v2] ASoC: rt721-sdca: Add RT721 SDCA driver
> 
> 
> External mail.
> 
> 
> 
> >>> 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.
> >
> >>>> +     /* 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.
> 
> The problem is that those blind writes are supposed to be sku-specific, so it's
> not great to encode a behavior in a generic codec driver.
> 
> It's my understanding that the Windows driver will rely on blind writes, it'd
> seem natural to use the same initialization parameters, no?
> 
> > 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?).
> 
> Yes, the code is already available in a prototype, see the initial code
> here:
> https://github.com/thesofproject/linux/pull/5010/commits/3e4ff84242dbb777
> 4bbed785371d27c3afc10a96
> 

Yes, I understand there is already a prototype in github, but it hasn't been merged into upstream 
and the customer requested Realtek to upstream the first version which already been verified. 
We'll sure follow this to do the blind write on later codec driver once this PR merged into upstream.


> The goal was to add a sound/soc/sdca library to parse ACPI function
> information, extract initialization tables, deal with interrupts, create controls,
> etc.
> 
> That said, it's probably best if Bard and/or Charles take over since I won't be
> able to work on this short-term.
> 
> One of the big opens is how we deal with regmap. In theory each SDCA
> function is independent from others, but in practice they all share a common
> control channel and interrupt mechanism.
> 
> I initially thought we could have one regmap for each function, but Charles had
> a different idea that we should handle regmap at the device level. Both options
> have merits and issues, we didn't really reach a conclusion on this.
> 
> One of the other opens is also related to regmap, we now have two types of
> regmap for SoundWire (sdw- and sdw-mbq), but there are parts of ASoC where
> components are assumed to have a single regmap.





[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux