Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/sound/sof/ext_manifest.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ enum sof_ext_man_elem_type {
SOF_EXT_MAN_ELEM_CC_VERSION = SOF_IPC_EXT_CC_INFO,
SOF_EXT_MAN_ELEM_DBG_ABI = SOF_IPC_EXT_USER_ABI_INFO,
SOF_EXT_MAN_ELEM_CONFIG_DATA = 5, /**< ABI3.17 */
SOF_EXT_MAN_ELEM_PLATFORM_CONFIG_DATA = 6,
};

/* extended manifest element header */
Expand Down
2 changes: 1 addition & 1 deletion sound/soc/sof/intel/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ snd-sof-intel-ipc-objs := intel-ipc.o
snd-sof-intel-hda-common-objs := hda.o hda-loader.o hda-stream.o hda-trace.o \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, @ranj063 and @fredoh9 -- while ICCMAX is somewhat known as a concept, in this case this is a very specific and non-obvious quirk of our DSP bootflow. I won't complain now as the same text is already merged for TGL in upstream, but the commit message is not very helpful for people outside SOF project. But yeah, I'm ok to go with this The bit about "recommended hw programming sequence" is key and it's there...

hda-dsp.o hda-ipc.o hda-ctrl.o hda-pcm.o \
hda-dai.o hda-bus.o \
apl.o cnl.o tgl.o
apl.o cnl.o tgl.o icl.o
snd-sof-intel-hda-common-$(CONFIG_SND_SOC_SOF_HDA_PROBES) += hda-compress.o

snd-sof-intel-hda-objs := hda-codec.o
Expand Down
3 changes: 3 additions & 0 deletions sound/soc/sof/intel/apl.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ const struct snd_sof_dsp_ops sof_apl_ops = {
.pre_fw_run = hda_dsp_pre_fw_run,
.post_fw_run = hda_dsp_post_fw_run,

/* parse platform specific extended manifest */
.parse_platform_ext_manifest = hda_dsp_ext_man_get_cavs_config_data,

/* dsp core power up/down */
.core_power_up = hda_dsp_enable_core,
.core_power_down = hda_dsp_core_reset_power_down,
Expand Down
19 changes: 3 additions & 16 deletions sound/soc/sof/intel/cnl.c
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,9 @@ const struct snd_sof_dsp_ops sof_cnl_ops = {
.pre_fw_run = hda_dsp_pre_fw_run,
.post_fw_run = hda_dsp_post_fw_run,

/* parse platform specific extended manifest */
.parse_platform_ext_manifest = hda_dsp_ext_man_get_cavs_config_data,

/* dsp core power up/down */
.core_power_up = hda_dsp_enable_core,
.core_power_down = hda_dsp_core_reset_power_down,
Expand Down Expand Up @@ -346,22 +349,6 @@ const struct sof_intel_dsp_desc cnl_chip_info = {
};
EXPORT_SYMBOL_NS(cnl_chip_info, SND_SOC_SOF_INTEL_HDA_COMMON);

const struct sof_intel_dsp_desc icl_chip_info = {
/* Icelake */
.cores_num = 4,
.init_core_mask = 1,
.host_managed_cores_mask = GENMASK(3, 0),
.ipc_req = CNL_DSP_REG_HIPCIDR,
.ipc_req_mask = CNL_DSP_REG_HIPCIDR_BUSY,
.ipc_ack = CNL_DSP_REG_HIPCIDA,
.ipc_ack_mask = CNL_DSP_REG_HIPCIDA_DONE,
.ipc_ctl = CNL_DSP_REG_HIPCCTL,
.rom_init_timeout = 300,
.ssp_count = ICL_SSP_COUNT,
.ssp_base_offset = CNL_SSP_BASE_OFFSET,
};
EXPORT_SYMBOL_NS(icl_chip_info, SND_SOC_SOF_INTEL_HDA_COMMON);

const struct sof_intel_dsp_desc ehl_chip_info = {
/* Elkhartlake */
.cores_num = 4,
Expand Down
35 changes: 35 additions & 0 deletions sound/soc/sof/intel/ext_manifest.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAK on the file name.
You already have sound/sof/ext_manifest.h, changed in the previous patch.

This has got to be "intel_ext_manifest.h" or something clearly identifying Intel and not the generic SOF stuff

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart it is already in intel/ directory, can it really be confused for being SOF-generic? I find it a bit confusing in general: in most cases I prefer to eliminate redundancy in file names, so, sound/soc/sof/core.c is clear, that it's the SOF core, not ASoC or even ALSA core.c. But we also have sound/soc/sof/sof-audio.[ch], sof-priv.[ch], sof-{acpi,of,pci}-dev.c which seem both inconsistent and redundant, but ok, we already have them, and I personally don't see that as important enough to merit renames. OTOH ASoC has all their .c files named as sound/soc/soc-*.c which does look redundant to me, but at least it's consistent :-) In short, personally I find group/function.c preferable over group/group-function.c and it does seem to me the prevailing variant. But maybe this is a special case...

Copy link
Collaborator

@marc-hb marc-hb Nov 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like duplication in general either but note the number of "intel" occurrences in #include "ext_manifest.h" is zero, could be a bit confusing.

Now you also have to pray that the Makefiles know what they're doing which is quite rare :-) OK, hopefully not rare in the kernel...

Many editors also show only the basename in their menus etc.

Copy link
Collaborator Author

@fredoh9 fredoh9 Nov 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like unique file name personally as I manage all the files in a project. But I already found multiple same file names in linux kernel directory. My understanding... if same file in include directory, there will be strong objection. linux kernel seems allow same file name in different source directory and doesn't like redundancy in file name. This file is in SOF source directory.
I don't have strong opinion. @plbossart what's your position? (after reading all comments after yours)

/*
* This file is provided under a dual BSD/GPLv2 license. When using or
* redistributing this file, you may do so under either license.
*
* Copyright(c) 2020 Intel Corporation. All rights reserved.
*/

/*
* Intel extended manifest is a extra place to store Intel cavs specific
* metadata about firmware, for example LPRO/HPRO configuration is
* Intel cavs specific. This part of output binary is not signed.
*/

#ifndef __INTEL_CAVS_EXT_MANIFEST_H__
#define __INTEL_CAVS_EXT_MANIFEST_H__

#include <sound/sof/ext_manifest.h>

/* EXT_MAN_ELEM_PLATFORM_CONFIG_DATA elements identificators */
enum sof_cavs_config_elem_type {
SOF_EXT_MAN_CAVS_CONFIG_EMPTY = 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the point of an EMPTY token that will be skipped anyways?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart I think the explanation here was to check against 0-initialised data, as used in switch below, but I also don't find this neither useful nor common. There are some cases in the kernel where enums skip 0 and begin with 1, but usually there's a very specific reason for that. Also the naming of LAST_ELEM which isn't in fact the last element but the one after it, usually named NUMBER_OF_ELEMENTS or similar. And using it as the size of an array in the ABI - all this I still don't like in this (and related) patch(es).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need explicit EMPTY token to skip, I think, this is better than just comparing 0.
And also this is symmetrical change include/sound/sof/ext_manifest.h

https://github.com/thesofproject/linux/blob/topic/sof-dev/include/sound/sof/ext_manifest.h#L105

	SOF_EXT_MAN_CONFIG_EMPTY		= 0,

SOF_EXT_MAN_CAVS_CONFIG_CAVS_LPRO = 1,
SOF_EXT_MAN_CAVS_CONFIG_OUTBOX_SIZE = 2,
SOF_EXT_MAN_CAVS_CONFIG_INBOX_SIZE = 3,
};

/* EXT_MAN_ELEM_PLATFORM_CONFIG_DATA elements */
struct sof_ext_man_cavs_config_data {
struct sof_ext_man_elem_header hdr;

struct sof_config_elem elems[];
} __packed;

#endif /* __INTEL_CAVS_EXT_MANIFEST_H__ */
100 changes: 100 additions & 0 deletions sound/soc/sof/intel/hda-loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <sound/hdaudio_ext.h>
#include <sound/hda_register.h>
#include <sound/sof.h>
#include "ext_manifest.h"
#include "../ops.h"
#include "hda.h"

Expand Down Expand Up @@ -470,3 +471,102 @@ int hda_dsp_post_fw_run(struct snd_sof_dev *sdev)
/* re-enable clock gating and power gating */
return hda_dsp_ctrl_clock_power_gating(sdev, true);
}

/*
* post fw run operations for ICL,
* Core 3 will be powered up and in stall when HPRO is enabled
*/
int hda_dsp_post_fw_run_icl(struct snd_sof_dev *sdev)
{
struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
int ret;

if (sdev->first_boot) {
ret = hda_sdw_startup(sdev);
if (ret < 0) {
dev_err(sdev->dev,
"error: could not startup SoundWire links\n");
return ret;
}
}

hda_sdw_int_enable(sdev, true);

/*
* The recommended HW programming sequence for ICL is to
* power up core 3 and keep it in stall if HPRO is enabled.
* Major difference between ICL and TGL, on ICL core 3 is managed by
* the host whereas on TGL it is handled by the firmware.
*/
if (!hda->clk_config_lpro) {
ret = snd_sof_dsp_core_power_up(sdev, BIT(3));
if (ret < 0) {
dev_err(sdev->dev, "error: dsp core power up failed on core 3\n");
return ret;
}

snd_sof_dsp_stall(sdev, BIT(3));
}

/* re-enable clock gating and power gating */
return hda_dsp_ctrl_clock_power_gating(sdev, true);
}

int hda_dsp_ext_man_get_cavs_config_data(struct snd_sof_dev *sdev,
const struct sof_ext_man_elem_header *hdr)
{
const struct sof_ext_man_cavs_config_data *config_data =
container_of(hdr, struct sof_ext_man_cavs_config_data, hdr);
struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
int i, elem_num;

/* calculate total number of config data elements */
elem_num = (hdr->size - sizeof(struct sof_ext_man_elem_header))
/ sizeof(struct sof_config_elem);
if (elem_num <= 0) {
dev_err(sdev->dev, "cavs config data is inconsistent: %d\n", elem_num);
return -EINVAL;
}

for (i = 0; i < elem_num; i++)
switch (config_data->elems[i].token) {
case SOF_EXT_MAN_CAVS_CONFIG_EMPTY:
/* skip empty token */
break;
case SOF_EXT_MAN_CAVS_CONFIG_CAVS_LPRO:
hda->clk_config_lpro = config_data->elems[i].value;
dev_dbg(sdev->dev, "FW clock config: %s\n",
hda->clk_config_lpro ? "LPRO" : "HPRO");
break;
case SOF_EXT_MAN_CAVS_CONFIG_OUTBOX_SIZE:
case SOF_EXT_MAN_CAVS_CONFIG_INBOX_SIZE:
/* These elements are defined but not being used yet. No warn is required */
break;
default:
dev_info(sdev->dev, "unsupported token type: %d\n",
config_data->elems[i].token);
}

return 0;
}

int hda_dsp_core_stall_icl(struct snd_sof_dev *sdev, unsigned int core_mask)
{
struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
const struct sof_intel_dsp_desc *chip = hda->desc;

/* make sure core_mask in host managed cores */
core_mask &= chip->host_managed_cores_mask;
if (!core_mask) {
dev_err(sdev->dev, "error: core_mask is not in host managed cores\n");
return -EINVAL;
}

/* stall core */
snd_sof_dsp_update_bits_unlocked(sdev, HDA_DSP_BAR,
HDA_DSP_REG_ADSPCS,
HDA_DSP_ADSPCS_CSTALL_MASK(core_mask),
HDA_DSP_ADSPCS_CSTALL_MASK(core_mask));

return 0;
}
11 changes: 11 additions & 0 deletions sound/soc/sof/intel/hda.h
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,9 @@ struct sof_intel_hda_dev {

/* sdw context allocated by SoundWire driver */
struct sdw_intel_ctx *sdw;

/* FW clock config, 0:HPRO, 1:LPRO */
bool clk_config_lpro;
};

static inline struct hdac_bus *sof_to_bus(struct snd_sof_dev *s)
Expand Down Expand Up @@ -612,11 +615,18 @@ int hda_dsp_ipc_cmd_done(struct snd_sof_dev *sdev, int dir);
*/
int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev);
int hda_dsp_cl_boot_firmware_iccmax(struct snd_sof_dev *sdev);
int hda_dsp_cl_boot_firmware_iccmax_icl(struct snd_sof_dev *sdev);
int hda_dsp_cl_boot_firmware_skl(struct snd_sof_dev *sdev);

/* pre and post fw run ops */
int hda_dsp_pre_fw_run(struct snd_sof_dev *sdev);
int hda_dsp_post_fw_run(struct snd_sof_dev *sdev);
int hda_dsp_post_fw_run_icl(struct snd_sof_dev *sdev);
int hda_dsp_core_stall_icl(struct snd_sof_dev *sdev, unsigned int core_mask);

/* parse platform specific ext manifest ops */
int hda_dsp_ext_man_get_cavs_config_data(struct snd_sof_dev *sdev,
const struct sof_ext_man_elem_header *hdr);

/*
* HDA Controller Operations.
Expand Down Expand Up @@ -733,6 +743,7 @@ extern struct snd_soc_dai_driver skl_dai[];
extern const struct snd_sof_dsp_ops sof_apl_ops;
extern const struct snd_sof_dsp_ops sof_cnl_ops;
extern const struct snd_sof_dsp_ops sof_tgl_ops;
extern const struct snd_sof_dsp_ops sof_icl_ops;

extern const struct sof_intel_dsp_desc apl_chip_info;
extern const struct sof_intel_dsp_desc cnl_chip_info;
Expand Down
145 changes: 145 additions & 0 deletions sound/soc/sof/intel/icl.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
//
// Copyright(c) 2020 Intel Corporation. All rights reserved.
//
// Author: Fred Oh <fred.oh@linux.intel.com>
//

/*
* Hardware interface for audio DSP on IceLake.
*/

#include <linux/kernel.h>
#include <linux/kconfig.h>
#include <linux/export.h>
#include <linux/bits.h>
#include "../ops.h"
#include "hda.h"
#include "hda-ipc.h"
#include "../sof-audio.h"

static const struct snd_sof_debugfs_map icl_dsp_debugfs[] = {
{"hda", HDA_DSP_HDA_BAR, 0, 0x4000, SOF_DEBUGFS_ACCESS_ALWAYS},
{"pp", HDA_DSP_PP_BAR, 0, 0x1000, SOF_DEBUGFS_ACCESS_ALWAYS},
{"dsp", HDA_DSP_BAR, 0, 0x10000, SOF_DEBUGFS_ACCESS_ALWAYS},
};

/* Icelake ops */
const struct snd_sof_dsp_ops sof_icl_ops = {
/* probe and remove */
.probe = hda_dsp_probe,
.remove = hda_dsp_remove,

/* Register IO */
.write = sof_io_write,
.read = sof_io_read,
.write64 = sof_io_write64,
.read64 = sof_io_read64,

/* Block IO */
.block_read = sof_block_read,
.block_write = sof_block_write,

/* doorbell */
.irq_thread = cnl_ipc_irq_thread,

/* ipc */
.send_msg = cnl_ipc_send_msg,
.fw_ready = sof_fw_ready,
.get_mailbox_offset = hda_dsp_ipc_get_mailbox_offset,
.get_window_offset = hda_dsp_ipc_get_window_offset,

.ipc_msg_data = hda_ipc_msg_data,
.ipc_pcm_params = hda_ipc_pcm_params,

/* machine driver */
.machine_select = hda_machine_select,
.machine_register = sof_machine_register,
.machine_unregister = sof_machine_unregister,
.set_mach_params = hda_set_mach_params,

/* debug */
.debug_map = icl_dsp_debugfs,
.debug_map_count = ARRAY_SIZE(icl_dsp_debugfs),
.dbg_dump = hda_dsp_dump,
.ipc_dump = cnl_ipc_dump,

/* stream callbacks */
.pcm_open = hda_dsp_pcm_open,
.pcm_close = hda_dsp_pcm_close,
.pcm_hw_params = hda_dsp_pcm_hw_params,
.pcm_hw_free = hda_dsp_stream_hw_free,
.pcm_trigger = hda_dsp_pcm_trigger,
.pcm_pointer = hda_dsp_pcm_pointer,

#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
/* probe callbacks */
.probe_assign = hda_probe_compr_assign,
.probe_free = hda_probe_compr_free,
.probe_set_params = hda_probe_compr_set_params,
.probe_trigger = hda_probe_compr_trigger,
.probe_pointer = hda_probe_compr_pointer,
#endif

/* firmware loading */
.load_firmware = snd_sof_load_firmware_raw,

/* pre/post fw run */
.pre_fw_run = hda_dsp_pre_fw_run,
.post_fw_run = hda_dsp_post_fw_run_icl,

/* parse platform specific extended manifest */
.parse_platform_ext_manifest = hda_dsp_ext_man_get_cavs_config_data,

/* dsp core power up/down */
.core_power_up = hda_dsp_enable_core,
.core_power_down = hda_dsp_core_reset_power_down,

/* firmware run */
.run = hda_dsp_cl_boot_firmware_iccmax,
.stall = hda_dsp_core_stall_icl,

/* trace callback */
.trace_init = hda_dsp_trace_init,
.trace_release = hda_dsp_trace_release,
.trace_trigger = hda_dsp_trace_trigger,

/* DAI drivers */
.drv = skl_dai,
.num_drv = SOF_SKL_NUM_DAIS,

/* PM */
.suspend = hda_dsp_suspend,
.resume = hda_dsp_resume,
.runtime_suspend = hda_dsp_runtime_suspend,
.runtime_resume = hda_dsp_runtime_resume,
.runtime_idle = hda_dsp_runtime_idle,
.set_hw_params_upon_resume = hda_dsp_set_hw_params_upon_resume,
.set_power_state = hda_dsp_set_power_state,

/* ALSA HW info flags */
.hw_info = SNDRV_PCM_INFO_MMAP |
SNDRV_PCM_INFO_MMAP_VALID |
SNDRV_PCM_INFO_INTERLEAVED |
SNDRV_PCM_INFO_PAUSE |
SNDRV_PCM_INFO_NO_PERIOD_WAKEUP,

.arch_ops = &sof_xtensa_arch_ops,
};
EXPORT_SYMBOL_NS(sof_icl_ops, SND_SOC_SOF_INTEL_HDA_COMMON);

const struct sof_intel_dsp_desc icl_chip_info = {
/* Icelake */
.cores_num = 4,
.init_core_mask = 1,
.host_managed_cores_mask = GENMASK(3, 0),
.ipc_req = CNL_DSP_REG_HIPCIDR,
.ipc_req_mask = CNL_DSP_REG_HIPCIDR_BUSY,
.ipc_ack = CNL_DSP_REG_HIPCIDA,
.ipc_ack_mask = CNL_DSP_REG_HIPCIDA_DONE,
.ipc_ctl = CNL_DSP_REG_HIPCCTL,
.rom_init_timeout = 300,
.ssp_count = ICL_SSP_COUNT,
.ssp_base_offset = CNL_SSP_BASE_OFFSET,
};
EXPORT_SYMBOL_NS(icl_chip_info, SND_SOC_SOF_INTEL_HDA_COMMON);
3 changes: 3 additions & 0 deletions sound/soc/sof/intel/tgl.c
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ const struct snd_sof_dsp_ops sof_tgl_ops = {
.pre_fw_run = hda_dsp_pre_fw_run,
.post_fw_run = hda_dsp_post_fw_run,

/* parse platform specific extended manifest */
.parse_platform_ext_manifest = hda_dsp_ext_man_get_cavs_config_data,

/* dsp core power up/down */
.core_power_up = hda_dsp_enable_core,
.core_power_down = hda_dsp_core_reset_power_down,
Expand Down
Loading