[Sound-open-firmware] CNL/APL alignment

[Pierre-Louis Bossart]

On 02/21/2018 05:18 AM, Jie, Yang wrote:
>> -----Original Message-----
>> From: sound-open-firmware-bounces at alsa-project.org [mailto:sound-open-
>> firmware-bounces at alsa-project.org] On Behalf Of Pierre-Louis Bossart
>> Sent: Wednesday, February 21, 2018 3:34 AM
>> To: sound-open-firmware at alsa-project.org
>> Subject: [Sound-open-firmware] CNL/APL alignment
>>
>> Looking at the code and latest patches I see quite a few differences
>> between APL and CNL that makes no sense to me.
>>
>> 1. why would use a workqueue based on SSP (APL) and CPU (CNL)?
> Currently the workqueue is using wall clock on both APL and CNL, this is
> 19.2MHz on APL and 24MHz on CNL, so I think we should use SSP(or
> another new created type?) on both APL and CNL.

Nope, it's CLK_CPU for CNL, see code below:

CNL:
static struct work_queue_timesource platform_generic_queue = {
     .timer     = {
         .id = TIMER3, /* external timer */
         .irq = IRQ_EXT_TSTAMP0_LVL2(0),
     },
     .clk        = CLK_CPU,
     .notifier    = NOTIFIER_ID_CPU_FREQ,
     .timer_set    = platform_timer_set,
     .timer_clear    = platform_timer_clear,
     .timer_get    = platform_timer_get,
};

APL:
static struct work_queue_timesource platform_generic_queue = {
     .timer     = {
         .id = TIMER3, /* external timer, XTAL 19.2M */
         .irq = IRQ_EXT_TSTAMP0_LVL2(0),
     },
     .clk        = CLK_SSP,
     .notifier    = NOTIFIER_ID_SSP_FREQ,
     .timer_set    = platform_timer_set,
     .timer_clear    = platform_timer_clear,
     .timer_get    = platform_timer_get,
};


>
>> 2. why would we initialize the SSP to 25MHz for CNL? It's clearly not a
>> supported frequency, 25MHz can only be reached on BYT and CHT. On a
>> similar note not sure why APL makes a reference to 24 MHz, it should be
>> 24576000.
> On APL, it is now set to 19.2MHz at platform_init, I had plan to enable this
> (without ECS set) before CNY but failed to get it work.
>
> For CNL, I agree it should be 24MHz, let's change it.
ok

>
>> 3. why would we use DMAC class 1/2 on APL and class 6/7 on CNL?
> Actually this doesn't impact the usage of DMAC for us during my testing,
> but right, we should make it consistent for APL and CNL.
>
> Liam, any comment on this?
>
>>
>> Conversely I always thought there were differences in the HDaudio DMA
>> channel count between APL and CNL, so can we really share common
>> definitions in src/drivers/hda-dma.c ?
> Yes we can, the difference(channel count, irq num, dmac list, ...) is listed
> in  platform/dma.c dma array.

for APL, the code shows
host in: 7 channels
host out: 6 channels
link in: 8 channels
link out: 8 channels

for CNL, the last patch from Tomasz says:
host in: 9 channels
host out: 7 channels
link in: 9 channels
link out: 7 channels

with HDA_DMA_MAX_CHANS modified to 9.

But if src/drivers/hda-dma.c, we do this:
     /* init channel status */
     for (i = 0; i < HDA_DMA_MAX_CHANS; i++)
         hda_pdata->chan[i].status = COMP_STATE_INIT;

So that means we declare some DMA channels as initialized, even though 
they might not actually exist, and we are not making any distinctions 
between in and out channels, which is somewhat odd.

>
> Thanks,
> ~Keyon
>
>> Thanks
>> -Pierre
>> _______________________________________________
>> Sound-open-firmware mailing list
>> Sound-open-firmware at alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware