[alsa-devel] [PATCH v5 0/6] ASoC: Intel: Skylake: Add a clk driver to enable ssp clks early
For certain platforms, clocks (mclk/sclk/fs) are required to be up before the stream start. Example: some codecs needs the mclk/sclk/fs to be enabled early for a successful clock synchronization. Some platforms require clock to be enabled at boot and be always ON.
By sending set_dma_control IPC (with the i2s blobs queried from NHLT), these clocks can be enabled early after the firmware is downloaded.
With this series, a virtual clock driver is created which provides interface to send the required IPCs from machine driver to enable the clocks. NHLT is parsed during probe and the clock information is populated. The pointer to blob is cached and sent along with the set_dma_control IPC structure during the clk prepare/unprepare callback. Clocks are created for a ssp if the nhlt table has endpoint configuration for that particular ssp.
Kabylake machine driver uses the clock interface to enable the clocks early as it is required by the rt5663 driver for clock synchronization.
v4 -> v5 - Remove checks for clock enable status from machine driver since taken care in the framework already
- Add check in the skl_clk_set_rate to avoid different rates when clock is enabled already
v3 -> v4 - Add missing signed-offs
v2 -> v3 - Moved the clk ops and IPCs from Skylake driver to clk driver and reordered commits accordingly
- Add the support for extended I2S blob config which supports multiple mclk dividers in NHLT
- Enable the clocks as well in DAPM PMU event instead of hw_params in machine drivers as confirmed by codec vendor
- Do not register the clk if there is no valid clock source is avail in the I2S blob
- Take care of error return in the clk driver
- Address rest of the review comments and more optimization added
- Fix the warning sound/soc/intel/skylake/skl.c:724:1-3: WARNING: PTR_ERR_OR_ZERO can be used reported by scripts/coccinelle/api/ptr_ret.cocci
- Modified DSP replies as human readable to ease the debugging
- Add firmware replies for MCLK/SCLK clocks if they are running already
v1 -> v2 - Register parent clocks with skylake device. With the patch "clk: Add support for runtime PM" soon to be merged will help DSP to stay active on call to clock enable. Reference: (https://patchwork.kernel.org/patch/9911741/)
- Fix the machine driver to enable clocks early for headphone playback path as well to fix a pop noise issue
Harsha Priya (1): ASoC: Intel: kbl: Enable mclk and ssp sclk early
Naveen M (1): ASoC: Intel: eve: Enable mclk and ssp sclk early
Sriram Periyasamy (2): ASoC: Intel: Skylake: Add ssp clock driver ASoC: Intel: Skylake: Add extended I2S config blob support in Clock driver
Subhransu S. Prusty (2): ASoC: Intel: Skylake: Make DSP replies more human readable ASoC: Intel: Skylake: Add FW reply for MCLK/SCLK IPC
sound/soc/intel/Kconfig | 3 + sound/soc/intel/boards/Kconfig | 2 + sound/soc/intel/boards/kbl_rt5663_max98927.c | 95 ++++- .../soc/intel/boards/kbl_rt5663_rt5514_max98927.c | 94 ++++ sound/soc/intel/skylake/Makefile | 5 + sound/soc/intel/skylake/skl-i2s.h | 31 ++ sound/soc/intel/skylake/skl-messages.c | 1 + sound/soc/intel/skylake/skl-nhlt.c | 41 +- sound/soc/intel/skylake/skl-ssp-clk.c | 473 +++++++++++++++++++++ sound/soc/intel/skylake/skl-ssp-clk.h | 38 ++ sound/soc/intel/skylake/skl-sst-ipc.c | 50 ++- sound/soc/intel/skylake/skl.h | 6 + 12 files changed, 815 insertions(+), 24 deletions(-) create mode 100644 sound/soc/intel/skylake/skl-ssp-clk.c
For certain platforms, it is required to start the clocks (mclk/sclk/fs) before the stream start. Example: for few chrome systems, codec needs the mclk/sclk to be enabled early for a successful clock synchronization and for few IVI platforms, clock need to be enabled at boot and should be ON always.
Add the required structures and create set_dma_control ipc to enable or disable the clock. To enable sclk without fs, mclk ipc structure is used, else sclkfs ipc structure is used.
Clock prepare/unprepare are used to enable/disable the clock as the IPC will be sent in non-atomic context. The clk set_dma_control IPC structures are populated during the set_rate callback and IPC is sent to enable the clock during prepare callback.
This patch creates virtual clock driver, which allows the machine driver to use the clock interface to send IPCs to DSP to enable/disable the clocks.
Signed-off-by: Sriram Periyasamy sriramx.periyasamy@intel.com Signed-off-by: Jaikrishna Nemallapudi jaikrishnax.nemallapudi@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com --- sound/soc/intel/Kconfig | 3 + sound/soc/intel/skylake/Makefile | 5 + sound/soc/intel/skylake/skl-messages.c | 1 + sound/soc/intel/skylake/skl-ssp-clk.c | 473 +++++++++++++++++++++++++++++++++ sound/soc/intel/skylake/skl-ssp-clk.h | 38 +++ sound/soc/intel/skylake/skl.h | 6 + 6 files changed, 526 insertions(+) create mode 100644 sound/soc/intel/skylake/skl-ssp-clk.c
diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index e18118209b75..826a30f76249 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -53,6 +53,9 @@ config SND_SST_ATOM_HIFI2_PLATFORM depends on SND_SOC_INTEL_SST_TOPLEVEL && X86 select SND_SOC_COMPRESS
+config SND_SOC_INTEL_SKYLAKE_SSP_CLK + tristate + config SND_SOC_INTEL_SKYLAKE tristate "Intel ASoC SST driver for SKL/BXT/KBL/GLK/CNL" depends on SND_SOC_INTEL_SST_TOPLEVEL && PCI && ACPI diff --git a/sound/soc/intel/skylake/Makefile b/sound/soc/intel/skylake/Makefile index 3380deb81015..9131c35ad4bb 100644 --- a/sound/soc/intel/skylake/Makefile +++ b/sound/soc/intel/skylake/Makefile @@ -13,3 +13,8 @@ snd-soc-skl-ipc-objs := skl-sst-ipc.o skl-sst-dsp.o cnl-sst-dsp.o \ skl-sst-utils.o
obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE) += snd-soc-skl-ipc.o + +#Skylake Clock device support +snd-soc-skl-ssp-clk-objs := skl-ssp-clk.o + +obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE_SSP_CLK) += snd-soc-skl-ssp-clk.o diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c index 4e63213a8d55..05de2b8bd7a9 100644 --- a/sound/soc/intel/skylake/skl-messages.c +++ b/sound/soc/intel/skylake/skl-messages.c @@ -671,6 +671,7 @@ int skl_dsp_set_dma_control(struct skl_sst *ctx, u32 *caps, kfree(dma_ctrl); return err; } +EXPORT_SYMBOL_GPL(skl_dsp_set_dma_control);
static void skl_setup_out_format(struct skl_sst *ctx, struct skl_module_cfg *mconfig, diff --git a/sound/soc/intel/skylake/skl-ssp-clk.c b/sound/soc/intel/skylake/skl-ssp-clk.c new file mode 100644 index 000000000000..385ecd560a8a --- /dev/null +++ b/sound/soc/intel/skylake/skl-ssp-clk.c @@ -0,0 +1,473 @@ +/* + * skl-ssp-clk.c - ASoC skylake ssp clock driver + * + * Copyright (C) 2017 Intel Corp + * Author: Jaikrishna Nemallapudi jaikrishnax.nemallapudi@intel.com + * Author: Subhransu S. Prusty subhransu.s.prusty@intel.com + * + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/err.h> +#include <linux/platform_device.h> +#include <linux/clk-provider.h> +#include <linux/clkdev.h> +#include "skl.h" +#include "skl-ssp-clk.h" +#include "skl-topology.h" + +#define to_skl_clk(_hw) container_of(_hw, struct skl_clk, hw) + +struct skl_clk_parent { + struct clk_hw *hw; + struct clk_lookup *lookup; +}; + +struct skl_clk { + struct clk_hw hw; + struct clk_lookup *lookup; + unsigned long rate; + struct skl_clk_pdata *pdata; + u32 id; +}; + +struct skl_clk_data { + struct skl_clk_parent parent[SKL_MAX_CLK_SRC]; + struct skl_clk *clk[SKL_MAX_CLK_CNT]; + u8 avail_clk_cnt; +}; + +static int skl_get_clk_type(u32 index) +{ + switch (index) { + case 0 ... (SKL_SCLK_OFS - 1): + return SKL_MCLK; + + case SKL_SCLK_OFS ... (SKL_SCLKFS_OFS - 1): + return SKL_SCLK; + + case SKL_SCLKFS_OFS ... (SKL_MAX_CLK_CNT - 1): + return SKL_SCLK_FS; + + default: + return -EINVAL; + } +} + +static int skl_get_vbus_id(u32 index, u8 clk_type) +{ + switch (clk_type) { + case SKL_MCLK: + return index; + + case SKL_SCLK: + return index - SKL_SCLK_OFS; + + case SKL_SCLK_FS: + return index - SKL_SCLKFS_OFS; + + default: + return -EINVAL; + } +} + +static void skl_fill_clk_ipc(struct skl_clk_rate_cfg_table *rcfg, u8 clk_type) +{ + struct nhlt_fmt_cfg *fmt_cfg; + union skl_clk_ctrl_ipc *ipc; + struct wav_fmt *wfmt; + + if (!rcfg) + return; + + ipc = &rcfg->dma_ctl_ipc; + if (clk_type == SKL_SCLK_FS) { + fmt_cfg = (struct nhlt_fmt_cfg *)rcfg->config; + wfmt = &fmt_cfg->fmt_ext.fmt; + + /* Remove TLV Header size */ + ipc->sclk_fs.hdr.size = sizeof(struct skl_dmactrl_sclkfs_cfg) - + sizeof(struct skl_tlv_hdr); + ipc->sclk_fs.sampling_frequency = wfmt->samples_per_sec; + ipc->sclk_fs.bit_depth = wfmt->bits_per_sample; + ipc->sclk_fs.valid_bit_depth = + fmt_cfg->fmt_ext.sample.valid_bits_per_sample; + ipc->sclk_fs.number_of_channels = wfmt->channels; + } else { + ipc->mclk.hdr.type = DMA_CLK_CONTROLS; + /* Remove TLV Header size */ + ipc->mclk.hdr.size = sizeof(struct skl_dmactrl_mclk_cfg) - + sizeof(struct skl_tlv_hdr); + } +} + +/* Sends dma control IPC to turn the clock ON/OFF */ +static int skl_send_clk_dma_control(struct skl *skl, + struct skl_clk_rate_cfg_table *rcfg, + u32 vbus_id, u8 clk_type, + bool enable) +{ + struct nhlt_specific_cfg *sp_cfg; + u32 i2s_config_size, node_id = 0; + struct nhlt_fmt_cfg *fmt_cfg; + union skl_clk_ctrl_ipc *ipc; + void *i2s_config = NULL; + u8 *data, size; + int ret; + + if (!rcfg) + return -EIO; + + ipc = &rcfg->dma_ctl_ipc; + fmt_cfg = (struct nhlt_fmt_cfg *)rcfg->config; + sp_cfg = &fmt_cfg->config; + + if (clk_type == SKL_SCLK_FS) { + ipc->sclk_fs.hdr.type = + enable ? DMA_TRANSMITION_START : DMA_TRANSMITION_STOP; + data = (u8 *)&ipc->sclk_fs; + size = sizeof(struct skl_dmactrl_sclkfs_cfg); + } else { + /* 1 to enable mclk, 0 to enable sclk */ + if (clk_type == SKL_SCLK) + ipc->mclk.mclk = 0; + else + ipc->mclk.mclk = 1; + + ipc->mclk.keep_running = enable; + ipc->mclk.warm_up_over = enable; + ipc->mclk.clk_stop_over = !enable; + data = (u8 *)&ipc->mclk; + size = sizeof(struct skl_dmactrl_mclk_cfg); + } + + i2s_config_size = sp_cfg->size + size; + i2s_config = kzalloc(i2s_config_size, GFP_KERNEL); + if (!i2s_config) + return -ENOMEM; + + /* copy blob */ + memcpy(i2s_config, sp_cfg->caps, sp_cfg->size); + + /* copy additional dma controls information */ + memcpy(i2s_config + sp_cfg->size, data, size); + + node_id = ((SKL_DMA_I2S_LINK_INPUT_CLASS << 8) | (vbus_id << 4)); + ret = skl_dsp_set_dma_control(skl->skl_sst, (u32 *)i2s_config, + i2s_config_size, node_id); + kfree(i2s_config); + + return ret; +} + +static struct skl_clk_rate_cfg_table *skl_get_rate_cfg( + struct skl_clk_rate_cfg_table *rcfg, + unsigned long rate) +{ + int i; + + for (i = 0; (i < SKL_MAX_CLK_RATES) && rcfg[i].rate; i++) { + if (rcfg[i].rate == rate) + return &rcfg[i]; + } + + return NULL; +} + +static int skl_clk_change_status(struct skl_clk *clkdev, + bool enable) +{ + struct skl_clk_rate_cfg_table *rcfg; + int vbus_id, clk_type; + + clk_type = skl_get_clk_type(clkdev->id); + if (clk_type < 0) + return clk_type; + + vbus_id = skl_get_vbus_id(clkdev->id, clk_type); + if (vbus_id < 0) + return vbus_id; + + rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg, + clkdev->rate); + if (!rcfg) + return -EINVAL; + + return skl_send_clk_dma_control(clkdev->pdata->pvt_data, rcfg, + vbus_id, clk_type, enable); +} + +static int skl_clk_prepare(struct clk_hw *hw) +{ + struct skl_clk *clkdev = to_skl_clk(hw); + + if (!clkdev) + return -ENODEV; + + return skl_clk_change_status(clkdev, true); +} + +static void skl_clk_unprepare(struct clk_hw *hw) +{ + struct skl_clk *clkdev = to_skl_clk(hw); + + if (!clkdev) + return; + + skl_clk_change_status(clkdev, false); +} + +static int skl_clk_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + struct skl_clk *clkdev = to_skl_clk(hw); + struct skl_clk_rate_cfg_table *rcfg; + int clk_type; + + if (!clkdev) + return -ENODEV; + + if (!rate) + return -EINVAL; + + if (__clk_is_enabled(hw->clk) && (clkdev->rate != rate)) + return -EBUSY; + + rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg, + rate); + if (!rcfg) + return -EINVAL; + + clk_type = skl_get_clk_type(clkdev->id); + if (clk_type < 0) + return clk_type; + + skl_fill_clk_ipc(rcfg, clk_type); + clkdev->rate = rate; + + return 0; +} + +static unsigned long skl_clk_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct skl_clk *clkdev = to_skl_clk(hw); + struct skl_clk_rate_cfg_table *rcfg; + int clk_type; + + if (!clkdev) + return 0; + + if (clkdev->rate) + return clkdev->rate; + + rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg, + parent_rate); + if (!rcfg) + return 0; + + clk_type = skl_get_clk_type(clkdev->id); + if (clk_type < 0) + return 0; + + skl_fill_clk_ipc(rcfg, clk_type); + clkdev->rate = rcfg->rate; + + return clkdev->rate; +} + +/* Not supported by clk driver. Implemented to satisfy clk fw */ +long skl_clk_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + return rate; +} + +/* + * prepare/unprepare are used instead of enable/disable as IPC will be sent + * in non-atomic context. + */ +static const struct clk_ops skl_clk_ops = { + .prepare = skl_clk_prepare, + .unprepare = skl_clk_unprepare, + .set_rate = skl_clk_set_rate, + .round_rate = skl_clk_round_rate, + .recalc_rate = skl_clk_recalc_rate, +}; + +static void unregister_parent_src_clk(struct skl_clk_parent *pclk, + unsigned int id) +{ + while (id--) { + clkdev_drop(pclk[id].lookup); + clk_hw_unregister_fixed_rate(pclk[id].hw); + } +} + +static void unregister_src_clk(struct skl_clk_data *dclk) +{ + u8 cnt = dclk->avail_clk_cnt; + + while (cnt--) + clkdev_drop(dclk->clk[cnt]->lookup); +} + +static int skl_register_parent_clks(struct device *dev, + struct skl_clk_parent *parent, + struct skl_clk_parent_src *pclk) +{ + int i, ret; + + for (i = 0; i < SKL_MAX_CLK_SRC; i++) { + + /* Register Parent clock */ + parent[i].hw = clk_hw_register_fixed_rate(dev, pclk[i].name, + pclk[i].parent_name, 0, pclk[i].rate); + if (IS_ERR(parent[i].hw)) { + ret = PTR_ERR(parent[i].hw); + goto err; + } + + parent[i].lookup = clkdev_hw_create(parent[i].hw, pclk[i].name, + NULL); + if (!parent[i].lookup) { + clk_hw_unregister_fixed_rate(parent[i].hw); + ret = -ENOMEM; + goto err; + } + } + + return 0; +err: + unregister_parent_src_clk(parent, i); + return ret; +} + +/* Assign fmt_config to clk_data */ +static struct skl_clk *register_skl_clk(struct device *dev, + struct skl_ssp_clk *clk, + struct skl_clk_pdata *clk_pdata, int id) +{ + struct clk_init_data init; + struct skl_clk *clkdev; + int ret; + + clkdev = devm_kzalloc(dev, sizeof(*clkdev), GFP_KERNEL); + if (!clkdev) + return ERR_PTR(-ENOMEM); + + init.name = clk->name; + init.ops = &skl_clk_ops; + init.flags = 0; + init.parent_names = &clk->parent_name; + init.num_parents = 1; + clkdev->hw.init = &init; + clkdev->pdata = clk_pdata; + + clkdev->id = id; + ret = devm_clk_hw_register(dev, &clkdev->hw); + if (ret) { + clkdev = ERR_PTR(ret); + return clkdev; + } + + clkdev->lookup = clkdev_hw_create(&clkdev->hw, init.name, NULL); + if (!clkdev->lookup) + clkdev = ERR_PTR(-ENOMEM); + + return clkdev; +} + +static int skl_clk_dev_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device *parent_dev = dev->parent; + struct skl_clk_parent_src *parent_clks; + struct skl_clk_pdata *clk_pdata; + struct skl_clk_data *data; + struct skl_ssp_clk *clks; + int ret, i; + + clk_pdata = dev_get_platdata(&pdev->dev); + parent_clks = clk_pdata->parent_clks; + clks = clk_pdata->ssp_clks; + if (!parent_clks || !clks) + return -EIO; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + /* Register Parent clock */ + ret = skl_register_parent_clks(parent_dev, data->parent, parent_clks); + if (ret < 0) + return ret; + + for (i = 0; i < clk_pdata->num_clks; i++) { + /* + * Only register valid clocks + * i.e. for which nhlt entry is present. + */ + if (clks[i].rate_cfg[0].rate == 0) + continue; + + data->clk[i] = register_skl_clk(dev, &clks[i], clk_pdata, i); + if (IS_ERR(data->clk[i])) { + ret = PTR_ERR(data->clk[i]); + goto err_unreg_skl_clk; + } + + data->avail_clk_cnt++; + } + + platform_set_drvdata(pdev, data); + + return 0; + +err_unreg_skl_clk: + unregister_src_clk(data); + unregister_parent_src_clk(data->parent, SKL_MAX_CLK_SRC); + + return ret; +} + +static int skl_clk_dev_remove(struct platform_device *pdev) +{ + struct skl_clk_data *data; + + data = platform_get_drvdata(pdev); + unregister_src_clk(data); + unregister_parent_src_clk(data->parent, SKL_MAX_CLK_SRC); + + return 0; +} + +static struct platform_driver skl_clk_driver = { + .driver = { + .name = "skl-ssp-clk", + }, + .probe = skl_clk_dev_probe, + .remove = skl_clk_dev_remove, +}; + +module_platform_driver(skl_clk_driver); + +MODULE_DESCRIPTION("Skylake clock driver"); +MODULE_AUTHOR("Jaikrishna Nemallapudi jaikrishnax.nemallapudi@intel.com"); +MODULE_AUTHOR("Subhransu S. Prusty subhransu.s.prusty@intel.com"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:skl-ssp-clk"); diff --git a/sound/soc/intel/skylake/skl-ssp-clk.h b/sound/soc/intel/skylake/skl-ssp-clk.h index c9ea84004260..d1be50f96c05 100644 --- a/sound/soc/intel/skylake/skl-ssp-clk.h +++ b/sound/soc/intel/skylake/skl-ssp-clk.h @@ -54,8 +54,46 @@ struct skl_clk_parent_src { const char *parent_name; };
+struct skl_tlv_hdr { + u32 type; + u32 size; +}; + +struct skl_dmactrl_mclk_cfg { + struct skl_tlv_hdr hdr; + /* DMA Clk TLV params */ + u32 clk_warm_up:16; + u32 mclk:1; + u32 warm_up_over:1; + u32 rsvd0:14; + u32 clk_stop_delay:16; + u32 keep_running:1; + u32 clk_stop_over:1; + u32 rsvd1:14; +}; + +struct skl_dmactrl_sclkfs_cfg { + struct skl_tlv_hdr hdr; + /* DMA SClk&FS TLV params */ + u32 sampling_frequency; + u32 bit_depth; + u32 channel_map; + u32 channel_config; + u32 interleaving_style; + u32 number_of_channels : 8; + u32 valid_bit_depth : 8; + u32 sample_type : 8; + u32 reserved : 8; +}; + +union skl_clk_ctrl_ipc { + struct skl_dmactrl_mclk_cfg mclk; + struct skl_dmactrl_sclkfs_cfg sclk_fs; +}; + struct skl_clk_rate_cfg_table { unsigned long rate; + union skl_clk_ctrl_ipc dma_ctl_ipc; void *config; };
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h index 46dda88ba139..bb73068d2a21 100644 --- a/sound/soc/intel/skylake/skl.h +++ b/sound/soc/intel/skylake/skl.h @@ -38,6 +38,10 @@ /* D0I3C Register fields */ #define AZX_REG_VS_D0I3C_CIP 0x1 /* Command in progress */ #define AZX_REG_VS_D0I3C_I3 0x4 /* D0i3 enable */ +#define SKL_MAX_DMACTRL_CFG 18 +#define DMA_CLK_CONTROLS 1 +#define DMA_TRANSMITION_START 2 +#define DMA_TRANSMITION_STOP 3
struct skl_dsp_resource { u32 max_mcps; @@ -146,6 +150,8 @@ int skl_nhlt_create_sysfs(struct skl *skl); void skl_nhlt_remove_sysfs(struct skl *skl); void skl_get_clks(struct skl *skl, struct skl_ssp_clk *ssp_clks); struct skl_clk_parent_src *skl_get_parent_clk(u8 clk_id); +int skl_dsp_set_dma_control(struct skl_sst *ctx, u32 *caps, + u32 caps_size, u32 node_id);
struct skl_module_cfg;
On 12/11, Sriram Periyasamy wrote:
+static int skl_clk_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)
+{
- struct skl_clk *clkdev = to_skl_clk(hw);
- struct skl_clk_rate_cfg_table *rcfg;
- int clk_type;
- if (!clkdev)
return -ENODEV;
These checks don't make sense. container_of() on clk_hw structures returning NULL wouldn't happen.
- if (!rate)
return -EINVAL;
- if (__clk_is_enabled(hw->clk) && (clkdev->rate != rate))
Any chance you can directly read the hardware instead of going through the framework to find out if the clk is enabled? Seems circular to do it this way.
return -EBUSY;
- rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
rate);
- if (!rcfg)
return -EINVAL;
- clk_type = skl_get_clk_type(clkdev->id);
- if (clk_type < 0)
return clk_type;
- skl_fill_clk_ipc(rcfg, clk_type);
- clkdev->rate = rate;
- return 0;
+}
+static unsigned long skl_clk_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
+{
- struct skl_clk *clkdev = to_skl_clk(hw);
- struct skl_clk_rate_cfg_table *rcfg;
- int clk_type;
- if (!clkdev)
return 0;
- if (clkdev->rate)
return clkdev->rate;
Why is the rate being cached? We should always be able to calculate the rate based on parent_rate that gets passed to this function?
- rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
parent_rate);
- if (!rcfg)
return 0;
- clk_type = skl_get_clk_type(clkdev->id);
- if (clk_type < 0)
return 0;
- skl_fill_clk_ipc(rcfg, clk_type);
- clkdev->rate = rcfg->rate;
- return clkdev->rate;
+}
+/* Not supported by clk driver. Implemented to satisfy clk fw */ +long skl_clk_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long *parent_rate)
+{
- return rate;
+}
On Wed, Dec 13, 2017 at 02:30:32PM -0800, Stephen Boyd wrote:
On 12/11, Sriram Periyasamy wrote:
+static int skl_clk_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)
+{
- struct skl_clk *clkdev = to_skl_clk(hw);
- struct skl_clk_rate_cfg_table *rcfg;
- int clk_type;
- if (!clkdev)
return -ENODEV;
These checks don't make sense. container_of() on clk_hw structures returning NULL wouldn't happen.
Sure. will remove.
- if (!rate)
return -EINVAL;
- if (__clk_is_enabled(hw->clk) && (clkdev->rate != rate))
Any chance you can directly read the hardware instead of going through the framework to find out if the clk is enabled? Seems
No. This involves sending an IPC to DSP to enable clock and interpreting the return error code. I would like to avoid doing this here in set_rate.
circular to do it this way.
return -EBUSY;
- rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
rate);
- if (!rcfg)
return -EINVAL;
- clk_type = skl_get_clk_type(clkdev->id);
- if (clk_type < 0)
return clk_type;
- skl_fill_clk_ipc(rcfg, clk_type);
- clkdev->rate = rate;
- return 0;
+}
+static unsigned long skl_clk_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
+{
- struct skl_clk *clkdev = to_skl_clk(hw);
- struct skl_clk_rate_cfg_table *rcfg;
- int clk_type;
- if (!clkdev)
return 0;
- if (clkdev->rate)
return clkdev->rate;
Why is the rate being cached? We should always be able to calculate the rate based on parent_rate that gets passed to this function?
Will check and get back.
Regards, Subhransu
- rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
parent_rate);
- if (!rcfg)
return 0;
- clk_type = skl_get_clk_type(clkdev->id);
- if (clk_type < 0)
return 0;
- skl_fill_clk_ipc(rcfg, clk_type);
- clkdev->rate = rcfg->rate;
- return clkdev->rate;
+}
+/* Not supported by clk driver. Implemented to satisfy clk fw */ +long skl_clk_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long *parent_rate)
+{
- return rate;
+}
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
--
On Mon, Dec 18, 2017 at 09:27:16AM +0530, Subhransu S. Prusty wrote:
On Wed, Dec 13, 2017 at 02:30:32PM -0800, Stephen Boyd wrote:
On 12/11, Sriram Periyasamy wrote:
+static int skl_clk_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)
+{
- struct skl_clk *clkdev = to_skl_clk(hw);
- struct skl_clk_rate_cfg_table *rcfg;
- int clk_type;
- if (!clkdev)
return -ENODEV;
These checks don't make sense. container_of() on clk_hw structures returning NULL wouldn't happen.
Sure. will remove.
- if (!rate)
return -EINVAL;
- if (__clk_is_enabled(hw->clk) && (clkdev->rate != rate))
Any chance you can directly read the hardware instead of going through the framework to find out if the clk is enabled? Seems
No. This involves sending an IPC to DSP to enable clock and interpreting the return error code. I would like to avoid doing this here in set_rate.
circular to do it this way.
return -EBUSY;
- rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
rate);
- if (!rcfg)
return -EINVAL;
- clk_type = skl_get_clk_type(clkdev->id);
- if (clk_type < 0)
return clk_type;
- skl_fill_clk_ipc(rcfg, clk_type);
- clkdev->rate = rate;
- return 0;
+}
+static unsigned long skl_clk_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
+{
- struct skl_clk *clkdev = to_skl_clk(hw);
- struct skl_clk_rate_cfg_table *rcfg;
- int clk_type;
- if (!clkdev)
return 0;
- if (clkdev->rate)
return clkdev->rate;
Why is the rate being cached? We should always be able to calculate the rate based on parent_rate that gets passed to this function?
Will check and get back.
If I understand correctly, you refer to deriving the rate from parent_rate using ratios. But since only the DSP is aware of the ratios and not the driver, the driver can't derive the rate from the parent_rate and thus cached.
Regards, Subhransu
- rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
parent_rate);
- if (!rcfg)
return 0;
- clk_type = skl_get_clk_type(clkdev->id);
- if (clk_type < 0)
return 0;
- skl_fill_clk_ipc(rcfg, clk_type);
- clkdev->rate = rcfg->rate;
- return clkdev->rate;
+}
+/* Not supported by clk driver. Implemented to satisfy clk fw */ +long skl_clk_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long *parent_rate)
+{
- return rate;
+}
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
--
--
On 12/18, Subhransu S. Prusty wrote:
On Mon, Dec 18, 2017 at 09:27:16AM +0530, Subhransu S. Prusty wrote:
On Wed, Dec 13, 2017 at 02:30:32PM -0800, Stephen Boyd wrote:
On 12/11, Sriram Periyasamy wrote:
- if (!rate)
return -EINVAL;
- if (__clk_is_enabled(hw->clk) && (clkdev->rate != rate))
Any chance you can directly read the hardware instead of going through the framework to find out if the clk is enabled? Seems
No. This involves sending an IPC to DSP to enable clock and interpreting the return error code. I would like to avoid doing this here in set_rate.
Ok. So we're checking to see if software has already enabled the clk and then checking to see if the rate the consumer is requesting is the same as the rate it previously requested? I'm still confused what's going on here. Does skl_fill_clk_ipc() change the rate of the clk? Is there any way to ask the DSP what the rate would be if we were to use some rate configuration?
circular to do it this way.
return -EBUSY;
- rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
rate);
- if (!rcfg)
return -EINVAL;
- clk_type = skl_get_clk_type(clkdev->id);
- if (clk_type < 0)
return clk_type;
- skl_fill_clk_ipc(rcfg, clk_type);
- clkdev->rate = rate;
- return 0;
+}
+static unsigned long skl_clk_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
+{
- struct skl_clk *clkdev = to_skl_clk(hw);
- struct skl_clk_rate_cfg_table *rcfg;
- int clk_type;
- if (!clkdev)
return 0;
- if (clkdev->rate)
return clkdev->rate;
Why is the rate being cached? We should always be able to calculate the rate based on parent_rate that gets passed to this function?
Will check and get back.
If I understand correctly, you refer to deriving the rate from parent_rate using ratios. But since only the DSP is aware of the ratios and not the driver, the driver can't derive the rate from the parent_rate and thus cached.
I was thinking the code would do what's below all the time.
- rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
parent_rate);
- if (!rcfg)
return 0;
- clk_type = skl_get_clk_type(clkdev->id);
- if (clk_type < 0)
return 0;
- skl_fill_clk_ipc(rcfg, clk_type);
- clkdev->rate = rcfg->rate;
- return clkdev->rate;
+}
I guess that means doing an IPC to the DSP to figure out the ratio and how that relates to the parent rate? recalc_rate() can be called many times with different things when the framework is speculating on the tree. We don't want clk providers to rely on the order of this op being called with respect to clk_set_rate().
On Mon, Dec 18, 2017 at 11:10:40AM -0800, Stephen Boyd wrote:
On 12/18, Subhransu S. Prusty wrote:
On Mon, Dec 18, 2017 at 09:27:16AM +0530, Subhransu S. Prusty wrote:
On Wed, Dec 13, 2017 at 02:30:32PM -0800, Stephen Boyd wrote:
On 12/11, Sriram Periyasamy wrote:
- if (!rate)
return -EINVAL;
- if (__clk_is_enabled(hw->clk) && (clkdev->rate != rate))
Any chance you can directly read the hardware instead of going through the framework to find out if the clk is enabled? Seems
No. This involves sending an IPC to DSP to enable clock and interpreting the return error code. I would like to avoid doing this here in set_rate.
Ok. So we're checking to see if software has already enabled the clk and then checking to see if the rate the consumer is requesting is the same as the rate it previously requested? I'm
The second check is not required, will remove it.
still confused what's going on here. Does skl_fill_clk_ipc() change the rate of the clk? Is there any way to ask the DSP what
skl_fill_clk_ipc() prepares the IPC message based on the rate request by the consumer. This IPC will be sent to DSP during a call to clock enable.
the rate would be if we were to use some rate configuration?
No. If the clock is already running, reconfiguration is not allowed. So the above check is invalid.
circular to do it this way.
return -EBUSY;
- rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
rate);
- if (!rcfg)
return -EINVAL;
- clk_type = skl_get_clk_type(clkdev->id);
- if (clk_type < 0)
return clk_type;
- skl_fill_clk_ipc(rcfg, clk_type);
- clkdev->rate = rate;
- return 0;
+}
+static unsigned long skl_clk_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
+{
- struct skl_clk *clkdev = to_skl_clk(hw);
- struct skl_clk_rate_cfg_table *rcfg;
- int clk_type;
- if (!clkdev)
return 0;
- if (clkdev->rate)
return clkdev->rate;
Why is the rate being cached? We should always be able to calculate the rate based on parent_rate that gets passed to this function?
Will check and get back.
If I understand correctly, you refer to deriving the rate from parent_rate using ratios. But since only the DSP is aware of the ratios and not the driver, the driver can't derive the rate from the parent_rate and thus cached.
I was thinking the code would do what's below all the time.
I think we interpreted incorrectly. As recalc_rate is meant to be used only when parent_rate changes, so this can be removed as the set_parent is not supported for this driver. Please let me know if I understand correctly.
Regards, Subhransu
- rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
parent_rate);
- if (!rcfg)
return 0;
- clk_type = skl_get_clk_type(clkdev->id);
- if (clk_type < 0)
return 0;
- skl_fill_clk_ipc(rcfg, clk_type);
- clkdev->rate = rcfg->rate;
- return clkdev->rate;
+}
I guess that means doing an IPC to the DSP to figure out the ratio and how that relates to the parent rate? recalc_rate() can be called many times with different things when the framework is speculating on the tree. We don't want clk providers to rely on the order of this op being called with respect to clk_set_rate().
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
--
On 12/19, Subhransu S. Prusty wrote:
On Mon, Dec 18, 2017 at 11:10:40AM -0800, Stephen Boyd wrote:
On 12/18, Subhransu S. Prusty wrote:
On Mon, Dec 18, 2017 at 09:27:16AM +0530, Subhransu S. Prusty wrote:
On Wed, Dec 13, 2017 at 02:30:32PM -0800, Stephen Boyd wrote:
On 12/11, Sriram Periyasamy wrote:
- if (!rate)
return -EINVAL;
- if (__clk_is_enabled(hw->clk) && (clkdev->rate != rate))
Any chance you can directly read the hardware instead of going through the framework to find out if the clk is enabled? Seems
No. This involves sending an IPC to DSP to enable clock and interpreting the return error code. I would like to avoid doing this here in set_rate.
Ok. So we're checking to see if software has already enabled the clk and then checking to see if the rate the consumer is requesting is the same as the rate it previously requested? I'm
The second check is not required, will remove it.
still confused what's going on here. Does skl_fill_clk_ipc() change the rate of the clk? Is there any way to ask the DSP what
skl_fill_clk_ipc() prepares the IPC message based on the rate request by the consumer. This IPC will be sent to DSP during a call to clock enable.
the rate would be if we were to use some rate configuration?
No. If the clock is already running, reconfiguration is not allowed. So the above check is invalid.
But we can't figure out if the clk is already running when we probe this driver, correct? It seems that we're relying on knowing if the clk is already running by looking at the software enable count that relates to if the clk is enabled in software by some linux consumer.
circular to do it this way.
return -EBUSY;
- rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
rate);
- if (!rcfg)
return -EINVAL;
- clk_type = skl_get_clk_type(clkdev->id);
- if (clk_type < 0)
return clk_type;
- skl_fill_clk_ipc(rcfg, clk_type);
- clkdev->rate = rate;
- return 0;
+}
+static unsigned long skl_clk_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
+{
- struct skl_clk *clkdev = to_skl_clk(hw);
- struct skl_clk_rate_cfg_table *rcfg;
- int clk_type;
- if (!clkdev)
return 0;
- if (clkdev->rate)
return clkdev->rate;
Why is the rate being cached? We should always be able to calculate the rate based on parent_rate that gets passed to this function?
Will check and get back.
If I understand correctly, you refer to deriving the rate from parent_rate using ratios. But since only the DSP is aware of the ratios and not the driver, the driver can't derive the rate from the parent_rate and thus cached.
I was thinking the code would do what's below all the time.
I think we interpreted incorrectly. As recalc_rate is meant to be used only when parent_rate changes, so this can be removed as the set_parent is not supported for this driver. Please let me know if I understand correctly.
recalc_rate() is called whenever the clk rate could change. It could be that clk_set_rate() is called directly on this clk, and then recalc_rate() would be called. Or it could be that the parent of this clk has its rate change, and then again recalc_rate() would be called on this clk. set_parent is about changing the parent of the clk, which also would cause the framework to call recalc_rate() on a clk that gets a new parent.
On Tue, Dec 19, 2017 at 11:17:27AM -0800, Stephen Boyd wrote:
On 12/19, Subhransu S. Prusty wrote:
On Mon, Dec 18, 2017 at 11:10:40AM -0800, Stephen Boyd wrote:
On 12/18, Subhransu S. Prusty wrote:
On Mon, Dec 18, 2017 at 09:27:16AM +0530, Subhransu S. Prusty wrote:
On Wed, Dec 13, 2017 at 02:30:32PM -0800, Stephen Boyd wrote:
On 12/11, Sriram Periyasamy wrote:
> + > + if (!rate) > + return -EINVAL; > + > + if (__clk_is_enabled(hw->clk) && (clkdev->rate != rate))
Any chance you can directly read the hardware instead of going through the framework to find out if the clk is enabled? Seems
No. This involves sending an IPC to DSP to enable clock and interpreting the return error code. I would like to avoid doing this here in set_rate.
Ok. So we're checking to see if software has already enabled the clk and then checking to see if the rate the consumer is requesting is the same as the rate it previously requested? I'm
The second check is not required, will remove it.
still confused what's going on here. Does skl_fill_clk_ipc() change the rate of the clk? Is there any way to ask the DSP what
skl_fill_clk_ipc() prepares the IPC message based on the rate request by the consumer. This IPC will be sent to DSP during a call to clock enable.
the rate would be if we were to use some rate configuration?
No. If the clock is already running, reconfiguration is not allowed. So the above check is invalid.
But we can't figure out if the clk is already running when we probe this driver, correct? It seems that we're relying on knowing if the clk is already running by looking at the software enable count that relates to if the clk is enabled in software by some linux consumer.
So here are the details: - clock is turned ON, when we send the IPC. At probe we don't send, so the clock will be OFF. - The clock is configured by DSP firmware and it will need an IPC to trigger that. By default power up of HW and DSP fw bootup ensures clk is OFF
circular to do it this way.
> + return -EBUSY; > + > + rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg, > + rate); > + if (!rcfg) > + return -EINVAL; > + > + clk_type = skl_get_clk_type(clkdev->id); > + if (clk_type < 0) > + return clk_type; > + > + skl_fill_clk_ipc(rcfg, clk_type); > + clkdev->rate = rate; > + > + return 0; > +} > + > +static unsigned long skl_clk_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct skl_clk *clkdev = to_skl_clk(hw); > + struct skl_clk_rate_cfg_table *rcfg; > + int clk_type; > + > + if (!clkdev) > + return 0; > + > + if (clkdev->rate) > + return clkdev->rate;
Why is the rate being cached? We should always be able to calculate the rate based on parent_rate that gets passed to this function?
Will check and get back.
If I understand correctly, you refer to deriving the rate from parent_rate using ratios. But since only the DSP is aware of the ratios and not the driver, the driver can't derive the rate from the parent_rate and thus cached.
I was thinking the code would do what's below all the time.
I think we interpreted incorrectly. As recalc_rate is meant to be used only when parent_rate changes, so this can be removed as the set_parent is not supported for this driver. Please let me know if I understand correctly.
recalc_rate() is called whenever the clk rate could change. It could be that clk_set_rate() is called directly on this clk, and then recalc_rate() would be called. Or it could be that the parent of this clk has its rate change, and then again recalc_rate() would be called on this clk. set_parent is about changing the parent of the clk, which also would cause the framework to call recalc_rate() on a clk that gets a new parent.
Thanks for the explanation.
So, we have a parent of the clk which is fixed. so change of parent is not applicable here.
For us, recalc_rate() doesn't mean much as we can only return current rate, if it is same otherwise 0. Pls do advise in this case if the behaviour needs to change, if so how?
Regards, Subhransu
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
--
On 12/20, Subhransu S. Prusty wrote:
On Tue, Dec 19, 2017 at 11:17:27AM -0800, Stephen Boyd wrote:
But we can't figure out if the clk is already running when we probe this driver, correct? It seems that we're relying on knowing if the clk is already running by looking at the software enable count that relates to if the clk is enabled in software by some linux consumer.
So here are the details:
- clock is turned ON, when we send the IPC. At probe we don't send, so the clock will be OFF.
- The clock is configured by DSP firmware and it will need an IPC to trigger that. By default power up of HW and DSP fw bootup ensures clk is OFF
Ok great.
recalc_rate() is called whenever the clk rate could change. It could be that clk_set_rate() is called directly on this clk, and then recalc_rate() would be called. Or it could be that the parent of this clk has its rate change, and then again recalc_rate() would be called on this clk. set_parent is about changing the parent of the clk, which also would cause the framework to call recalc_rate() on a clk that gets a new parent.
Thanks for the explanation.
So, we have a parent of the clk which is fixed. so change of parent is not applicable here.
Yeah, let's ignore a changing parent frequency. recalc_rate() is also called when *this* clk rate is changed. The parent rate is passed in because that's usually helpful to calculate the rate that this op is supposed to return.
For us, recalc_rate() doesn't mean much as we can only return current rate, if it is same otherwise 0. Pls do advise in this case if the behaviour needs to change, if so how?
Can the DSP tell us what the rate of the clk is? Or what the rate of the clk is configured for? What is that configuration out of boot when it's OFF? Typically, recalc_rate() can tell us what the rate of the clk is, even when its OFF, because we can read the hardware and calculate the rate of the clk given the parent frequency. We do have clk drivers out there that are like this DSP and don't tell anything about the rate and we can't even ask. In that case, we return 0 and cache the rate in the set_rate op.
Ideally, recalc_rate would always return the frequency of the clk that's been configured in the hardware on the DSP side. If that can't be done, I suppose faking it and caching the rate that the set_rate op figures out would work. Or if enabling the clk let's us know the rate we should cache it there too. But definitely don't do any sort of rate caching in recalc_rate. It should just blindly return the cached value if it can't read hardware.
On Thu, Dec 21, 2017 at 06:04:36PM -0800, Stephen Boyd wrote:
On 12/20, Subhransu S. Prusty wrote:
On Tue, Dec 19, 2017 at 11:17:27AM -0800, Stephen Boyd wrote: applicable here.
Yeah, let's ignore a changing parent frequency. recalc_rate() is also called when *this* clk rate is changed. The parent rate is passed in because that's usually helpful to calculate the rate that this op is supposed to return.
For us, recalc_rate() doesn't mean much as we can only return current rate, if it is same otherwise 0. Pls do advise in this case if the behaviour needs to change, if so how?
Can the DSP tell us what the rate of the clk is? Or what the rate of the clk is configured for? What is that configuration out of boot when it's OFF? Typically, recalc_rate() can tell us what the
No, the DSP clock is programmed using configuration parameters which are sent from driver. DSP doesn't have any mechanism to report the clock configuration back to driver. So the rate is cached.
rate of the clk is, even when its OFF, because we can read the hardware and calculate the rate of the clk given the parent frequency. We do have clk drivers out there that are like this DSP and don't tell anything about the rate and we can't even ask. In that case, we return 0 and cache the rate in the set_rate op.
Ideally, recalc_rate would always return the frequency of the clk that's been configured in the hardware on the DSP side. If that can't be done, I suppose faking it and caching the rate that the set_rate op figures out would work. Or if enabling the clk let's us know the rate we should cache it there too. But definitely
Yes, this behavior is applicable for this driver. I will remove the recalculation part and resubmit the patch by returning the cached rate already done in set_rate.
Regards, Subhransu
don't do any sort of rate caching in recalc_rate. It should just blindly return the cached value if it can't read hardware.
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
--
Extended I2S config blob supports multiple mclk dividers in NHLT blob. This patch detects whether the I2S blob is legacy or extended based on the signature value and chooses the mclk source and divider accordingly.
Signed-off-by: Sriram Periyasamy sriramx.periyasamy@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com --- sound/soc/intel/skylake/skl-i2s.h | 31 ++++++++++++++++++++++++++++ sound/soc/intel/skylake/skl-nhlt.c | 41 +++++++++++++++++++++++++++----------- 2 files changed, 60 insertions(+), 12 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-i2s.h b/sound/soc/intel/skylake/skl-i2s.h index dcf819bc688f..ad0a1bbca13c 100644 --- a/sound/soc/intel/skylake/skl-i2s.h +++ b/sound/soc/intel/skylake/skl-i2s.h @@ -27,6 +27,12 @@ #define SKL_SHIFT(x) (ffs(x) - 1) #define SKL_MCLK_DIV_RATIO_MASK GENMASK(11, 0)
+#define is_legacy_blob(x) (x.signature != 0xEE) +#define ext_to_legacy_blob(i2s_config_blob_ext) \ + ((struct skl_i2s_config_blob_legacy *) i2s_config_blob_ext) + +#define get_clk_src(mclk, mask) \ + ((mclk.mdivctrl & mask) >> SKL_SHIFT(mask)) struct skl_i2s_config { u32 ssc0; u32 ssc1; @@ -45,6 +51,24 @@ struct skl_i2s_config_mclk { u32 mdivr; };
+struct skl_i2s_config_mclk_ext { + u32 mdivctrl; + u32 mdivr_count; + u32 mdivr[0]; +} __packed; + +struct skl_i2s_config_blob_signature { + u32 minor_ver : 8; + u32 major_ver : 8; + u32 resvdz : 8; + u32 signature : 8; +} __packed; + +struct skl_i2s_config_blob_header { + struct skl_i2s_config_blob_signature sig; + u32 size; +}; + /** * struct skl_i2s_config_blob_legacy - Structure defines I2S Gateway * configuration legacy blob @@ -61,4 +85,11 @@ struct skl_i2s_config_blob_legacy { struct skl_i2s_config_mclk mclk; };
+struct skl_i2s_config_blob_ext { + u32 gtw_attr; + struct skl_i2s_config_blob_header hdr; + u32 tdm_ts_group[SKL_I2S_MAX_TIME_SLOTS]; + struct skl_i2s_config i2s_cfg; + struct skl_i2s_config_mclk_ext mclk; +} __packed; #endif /* __SOUND_SOC_SKL_I2S_H */ diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c index ca5dc2be7b68..770a9c7afca1 100644 --- a/sound/soc/intel/skylake/skl-nhlt.c +++ b/sound/soc/intel/skylake/skl-nhlt.c @@ -26,6 +26,7 @@ static guid_t osc_guid = GUID_INIT(0xA69F886E, 0x6CEB, 0x4594, 0xA4, 0x1F, 0x7B, 0x5D, 0xCE, 0x24, 0xC5, 0x53);
+ struct nhlt_acpi_table *skl_nhlt_init(struct device *dev) { acpi_handle handle; @@ -272,6 +273,7 @@ void skl_nhlt_remove_sysfs(struct skl *skl) static void skl_get_ssp_clks(struct skl *skl, struct skl_ssp_clk *ssp_clks, struct nhlt_fmt *fmt, u8 id) { + struct skl_i2s_config_blob_ext *i2s_config_ext; struct skl_i2s_config_blob_legacy *i2s_config; struct skl_clk_parent_src *parent; struct skl_ssp_clk *sclk, *sclkfs; @@ -332,12 +334,18 @@ static void skl_get_ssp_clks(struct skl *skl, struct skl_ssp_clk *ssp_clks,
/* Fill rate and parent for sclk/sclkfs */ if (!present) { - /* MCLK Divider Source Select */ - i2s_config = (struct skl_i2s_config_blob_legacy *) + i2s_config_ext = (struct skl_i2s_config_blob_ext *) fmt->fmt_config[0].config.caps; - clk_src = ((i2s_config->mclk.mdivctrl) - & SKL_MNDSS_DIV_CLK_SRC_MASK) >> - SKL_SHIFT(SKL_MNDSS_DIV_CLK_SRC_MASK); + + /* MCLK Divider Source Select */ + if (is_legacy_blob(i2s_config_ext->hdr.sig)) { + i2s_config = ext_to_legacy_blob(i2s_config_ext); + clk_src = get_clk_src(i2s_config->mclk, + SKL_MNDSS_DIV_CLK_SRC_MASK); + } else { + clk_src = get_clk_src(i2s_config_ext->mclk, + SKL_MNDSS_DIV_CLK_SRC_MASK); + }
parent = skl_get_parent_clk(clk_src);
@@ -363,6 +371,7 @@ static void skl_get_ssp_clks(struct skl *skl, struct skl_ssp_clk *ssp_clks, static void skl_get_mclk(struct skl *skl, struct skl_ssp_clk *mclk, struct nhlt_fmt *fmt, u8 id) { + struct skl_i2s_config_blob_ext *i2s_config_ext; struct skl_i2s_config_blob_legacy *i2s_config; struct nhlt_specific_cfg *fmt_cfg; struct skl_clk_parent_src *parent; @@ -370,13 +379,21 @@ static void skl_get_mclk(struct skl *skl, struct skl_ssp_clk *mclk, u8 clk_src;
fmt_cfg = &fmt->fmt_config[0].config; - i2s_config = (struct skl_i2s_config_blob_legacy *)fmt_cfg->caps; - - /* MCLK Divider Source Select */ - clk_src = ((i2s_config->mclk.mdivctrl) & SKL_MCLK_DIV_CLK_SRC_MASK) >> - SKL_SHIFT(SKL_MCLK_DIV_CLK_SRC_MASK); - - clkdiv = i2s_config->mclk.mdivr & SKL_MCLK_DIV_RATIO_MASK; + i2s_config_ext = (struct skl_i2s_config_blob_ext *)fmt_cfg->caps; + + /* MCLK Divider Source Select and divider */ + if (is_legacy_blob(i2s_config_ext->hdr.sig)) { + i2s_config = ext_to_legacy_blob(i2s_config_ext); + clk_src = get_clk_src(i2s_config->mclk, + SKL_MCLK_DIV_CLK_SRC_MASK); + clkdiv = i2s_config->mclk.mdivr & + SKL_MCLK_DIV_RATIO_MASK; + } else { + clk_src = get_clk_src(i2s_config_ext->mclk, + SKL_MCLK_DIV_CLK_SRC_MASK); + clkdiv = i2s_config_ext->mclk.mdivr[0] & + SKL_MCLK_DIV_RATIO_MASK; + }
/* bypass divider */ div_ratio = 1;
The patch
ASoC: Intel: Skylake: Add extended I2S config blob support in Clock driver
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 9afbc5ec76526d412de1c5c368524aae36eb608d Mon Sep 17 00:00:00 2001
From: Sriram Periyasamy sriramx.periyasamy@intel.com Date: Thu, 4 Jan 2018 16:55:15 +0530 Subject: [PATCH] ASoC: Intel: Skylake: Add extended I2S config blob support in Clock driver
Extended I2S config blob supports multiple mclk dividers in NHLT blob. This patch detects whether the I2S blob is legacy or extended based on the signature value and chooses the mclk source and divider accordingly.
Signed-off-by: Sriram Periyasamy sriramx.periyasamy@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Acked-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/skylake/skl-i2s.h | 31 ++++++++++++++++++++++++++++ sound/soc/intel/skylake/skl-nhlt.c | 41 +++++++++++++++++++++++++++----------- 2 files changed, 60 insertions(+), 12 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-i2s.h b/sound/soc/intel/skylake/skl-i2s.h index dcf819bc688f..ad0a1bbca13c 100644 --- a/sound/soc/intel/skylake/skl-i2s.h +++ b/sound/soc/intel/skylake/skl-i2s.h @@ -27,6 +27,12 @@ #define SKL_SHIFT(x) (ffs(x) - 1) #define SKL_MCLK_DIV_RATIO_MASK GENMASK(11, 0)
+#define is_legacy_blob(x) (x.signature != 0xEE) +#define ext_to_legacy_blob(i2s_config_blob_ext) \ + ((struct skl_i2s_config_blob_legacy *) i2s_config_blob_ext) + +#define get_clk_src(mclk, mask) \ + ((mclk.mdivctrl & mask) >> SKL_SHIFT(mask)) struct skl_i2s_config { u32 ssc0; u32 ssc1; @@ -45,6 +51,24 @@ struct skl_i2s_config_mclk { u32 mdivr; };
+struct skl_i2s_config_mclk_ext { + u32 mdivctrl; + u32 mdivr_count; + u32 mdivr[0]; +} __packed; + +struct skl_i2s_config_blob_signature { + u32 minor_ver : 8; + u32 major_ver : 8; + u32 resvdz : 8; + u32 signature : 8; +} __packed; + +struct skl_i2s_config_blob_header { + struct skl_i2s_config_blob_signature sig; + u32 size; +}; + /** * struct skl_i2s_config_blob_legacy - Structure defines I2S Gateway * configuration legacy blob @@ -61,4 +85,11 @@ struct skl_i2s_config_blob_legacy { struct skl_i2s_config_mclk mclk; };
+struct skl_i2s_config_blob_ext { + u32 gtw_attr; + struct skl_i2s_config_blob_header hdr; + u32 tdm_ts_group[SKL_I2S_MAX_TIME_SLOTS]; + struct skl_i2s_config i2s_cfg; + struct skl_i2s_config_mclk_ext mclk; +} __packed; #endif /* __SOUND_SOC_SKL_I2S_H */ diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c index 3b1d2b828c1b..b9b140275be0 100644 --- a/sound/soc/intel/skylake/skl-nhlt.c +++ b/sound/soc/intel/skylake/skl-nhlt.c @@ -28,6 +28,7 @@ static guid_t osc_guid = GUID_INIT(0xA69F886E, 0x6CEB, 0x4594, 0xA4, 0x1F, 0x7B, 0x5D, 0xCE, 0x24, 0xC5, 0x53);
+ struct nhlt_acpi_table *skl_nhlt_init(struct device *dev) { acpi_handle handle; @@ -287,6 +288,7 @@ void skl_nhlt_remove_sysfs(struct skl *skl) static void skl_get_ssp_clks(struct skl *skl, struct skl_ssp_clk *ssp_clks, struct nhlt_fmt *fmt, u8 id) { + struct skl_i2s_config_blob_ext *i2s_config_ext; struct skl_i2s_config_blob_legacy *i2s_config; struct skl_clk_parent_src *parent; struct skl_ssp_clk *sclk, *sclkfs; @@ -347,12 +349,18 @@ static void skl_get_ssp_clks(struct skl *skl, struct skl_ssp_clk *ssp_clks,
/* Fill rate and parent for sclk/sclkfs */ if (!present) { - /* MCLK Divider Source Select */ - i2s_config = (struct skl_i2s_config_blob_legacy *) + i2s_config_ext = (struct skl_i2s_config_blob_ext *) fmt->fmt_config[0].config.caps; - clk_src = ((i2s_config->mclk.mdivctrl) - & SKL_MNDSS_DIV_CLK_SRC_MASK) >> - SKL_SHIFT(SKL_MNDSS_DIV_CLK_SRC_MASK); + + /* MCLK Divider Source Select */ + if (is_legacy_blob(i2s_config_ext->hdr.sig)) { + i2s_config = ext_to_legacy_blob(i2s_config_ext); + clk_src = get_clk_src(i2s_config->mclk, + SKL_MNDSS_DIV_CLK_SRC_MASK); + } else { + clk_src = get_clk_src(i2s_config_ext->mclk, + SKL_MNDSS_DIV_CLK_SRC_MASK); + }
parent = skl_get_parent_clk(clk_src);
@@ -378,6 +386,7 @@ static void skl_get_ssp_clks(struct skl *skl, struct skl_ssp_clk *ssp_clks, static void skl_get_mclk(struct skl *skl, struct skl_ssp_clk *mclk, struct nhlt_fmt *fmt, u8 id) { + struct skl_i2s_config_blob_ext *i2s_config_ext; struct skl_i2s_config_blob_legacy *i2s_config; struct nhlt_specific_cfg *fmt_cfg; struct skl_clk_parent_src *parent; @@ -385,13 +394,21 @@ static void skl_get_mclk(struct skl *skl, struct skl_ssp_clk *mclk, u8 clk_src;
fmt_cfg = &fmt->fmt_config[0].config; - i2s_config = (struct skl_i2s_config_blob_legacy *)fmt_cfg->caps; - - /* MCLK Divider Source Select */ - clk_src = ((i2s_config->mclk.mdivctrl) & SKL_MCLK_DIV_CLK_SRC_MASK) >> - SKL_SHIFT(SKL_MCLK_DIV_CLK_SRC_MASK); - - clkdiv = i2s_config->mclk.mdivr & SKL_MCLK_DIV_RATIO_MASK; + i2s_config_ext = (struct skl_i2s_config_blob_ext *)fmt_cfg->caps; + + /* MCLK Divider Source Select and divider */ + if (is_legacy_blob(i2s_config_ext->hdr.sig)) { + i2s_config = ext_to_legacy_blob(i2s_config_ext); + clk_src = get_clk_src(i2s_config->mclk, + SKL_MCLK_DIV_CLK_SRC_MASK); + clkdiv = i2s_config->mclk.mdivr & + SKL_MCLK_DIV_RATIO_MASK; + } else { + clk_src = get_clk_src(i2s_config_ext->mclk, + SKL_MCLK_DIV_CLK_SRC_MASK); + clkdiv = i2s_config_ext->mclk.mdivr[0] & + SKL_MCLK_DIV_RATIO_MASK; + }
/* bypass divider */ div_ratio = 1;
From: Harsha Priya harshapriya.n@intel.com
rt5663 needs mclk/sclk early to synchronize its internal clocks. Enable these clocks early.
Signed-off-by: Harsha Priya harshapriya.n@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Sriram Periyasamy sriramx.periyasamy@intel.com --- sound/soc/intel/boards/Kconfig | 1 + sound/soc/intel/boards/kbl_rt5663_max98927.c | 95 +++++++++++++++++++++++++++- 2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index 449bc8baaa60..f62f2ab1481e 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -262,6 +262,7 @@ config SND_SOC_INTEL_KBL_RT5663_MAX98927_MACH select SND_SOC_MAX98927 select SND_SOC_DMIC select SND_SOC_HDAC_HDMI + select SND_SOC_INTEL_SKYLAKE_SSP_CLK help This adds support for ASoC Onboard Codec I2S machine driver. This will create an alsa sound card for RT5663 + MAX98927. diff --git a/sound/soc/intel/boards/kbl_rt5663_max98927.c b/sound/soc/intel/boards/kbl_rt5663_max98927.c index 94a34db4f8c0..bb3e8a97df4b 100644 --- a/sound/soc/intel/boards/kbl_rt5663_max98927.c +++ b/sound/soc/intel/boards/kbl_rt5663_max98927.c @@ -28,6 +28,9 @@ #include "../../codecs/rt5663.h" #include "../../codecs/hdac_hdmi.h" #include "../skylake/skl.h" +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/clkdev.h>
#define KBL_REALTEK_CODEC_DAI "rt5663-aif" #define KBL_MAXIM_CODEC_DAI "max98927-aif1" @@ -48,6 +51,8 @@ struct kbl_hdmi_pcm { struct kbl_rt5663_private { struct snd_soc_jack kabylake_headset; struct list_head hdmi_pcm_list; + struct clk *mclk; + struct clk *sclk; };
enum { @@ -69,6 +74,61 @@ static const struct snd_kcontrol_new kabylake_controls[] = { SOC_DAPM_PIN_SWITCH("Right Spk"), };
+static int platform_clock_control(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *k, int event) +{ + struct snd_soc_dapm_context *dapm = w->dapm; + struct snd_soc_card *card = dapm->card; + struct kbl_rt5663_private *priv = snd_soc_card_get_drvdata(card); + int ret = 0; + + /* + * MCLK/SCLK need to be ON early for a successful synchronization of + * codec internal clock. And the clocks are turned off during + * POST_PMD after the stream is stopped. + */ + switch (event) { + case SND_SOC_DAPM_PRE_PMU: + /* Enable MCLK */ + ret = clk_set_rate(priv->mclk, 24000000); + if (ret < 0) { + dev_err(card->dev, "Can't set rate for mclk, err: %d\n", + ret); + return ret; + } + + ret = clk_prepare_enable(priv->mclk); + if (ret < 0) { + dev_err(card->dev, "Can't enable mclk, err: %d\n", ret); + return ret; + } + + /* Enable SCLK */ + ret = clk_set_rate(priv->sclk, 3072000); + if (ret < 0) { + dev_err(card->dev, "Can't set rate for sclk, err: %d\n", + ret); + clk_disable_unprepare(priv->mclk); + return ret; + } + + ret = clk_prepare_enable(priv->sclk); + if (ret < 0) { + dev_err(card->dev, "Can't enable sclk, err: %d\n", ret); + clk_disable_unprepare(priv->mclk); + } + break; + case SND_SOC_DAPM_POST_PMD: + clk_disable_unprepare(priv->mclk); + clk_disable_unprepare(priv->sclk); + break; + default: + return 0; + } + + return 0; +} + static const struct snd_soc_dapm_widget kabylake_widgets[] = { SND_SOC_DAPM_HP("Headphone Jack", NULL), SND_SOC_DAPM_MIC("Headset Mic", NULL), @@ -78,11 +138,14 @@ static const struct snd_soc_dapm_widget kabylake_widgets[] = { SND_SOC_DAPM_SPK("HDMI1", NULL), SND_SOC_DAPM_SPK("HDMI2", NULL), SND_SOC_DAPM_SPK("HDMI3", NULL), - + SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0, + platform_clock_control, SND_SOC_DAPM_PRE_PMU | + SND_SOC_DAPM_POST_PMD), };
static const struct snd_soc_dapm_route kabylake_map[] = { /* HP jack connectors - unknown if we have jack detection */ + { "Headphone Jack", NULL, "Platform Clock" }, { "Headphone Jack", NULL, "HPOL" }, { "Headphone Jack", NULL, "HPOR" },
@@ -91,6 +154,7 @@ static const struct snd_soc_dapm_route kabylake_map[] = { { "Right Spk", NULL, "Right BE_OUT" },
/* other jacks */ + { "Headset Mic", NULL, "Platform Clock" }, { "IN1P", NULL, "Headset Mic" }, { "IN1N", NULL, "Headset Mic" }, { "DMic", NULL, "SoC DMIC" }, @@ -901,6 +965,7 @@ static int kabylake_audio_probe(struct platform_device *pdev) { struct kbl_rt5663_private *ctx; struct skl_machine_pdata *pdata; + int ret;
ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC); if (!ctx) @@ -919,6 +984,34 @@ static int kabylake_audio_probe(struct platform_device *pdev) dmic_constraints = pdata->dmic_num == 2 ? &constraints_dmic_2ch : &constraints_dmic_channels;
+ ctx->mclk = devm_clk_get(&pdev->dev, "ssp1_mclk"); + if (IS_ERR(ctx->mclk)) { + ret = PTR_ERR(ctx->mclk); + if (ret == -ENOENT) { + dev_info(&pdev->dev, + "Failed to get ssp1_sclk, defer probe\n"); + return -EPROBE_DEFER; + } + + dev_err(&pdev->dev, "Failed to get ssp1_mclk with err:%d\n", + ret); + return ret; + } + + ctx->sclk = devm_clk_get(&pdev->dev, "ssp1_sclk"); + if (IS_ERR(ctx->sclk)) { + ret = PTR_ERR(ctx->sclk); + if (ret == -ENOENT) { + dev_info(&pdev->dev, + "Failed to get ssp1_sclk, defer probe\n"); + return -EPROBE_DEFER; + } + + dev_err(&pdev->dev, "Failed to get ssp1_sclk with err:%d\n", + ret); + return ret; + } + return devm_snd_soc_register_card(&pdev->dev, kabylake_audio_card); }
The patch
ASoC: Intel: kbl: Enable mclk and ssp sclk early
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From f7f61e08fe5840ca43baa49355b40d3aede1ac97 Mon Sep 17 00:00:00 2001
From: Harsha Priya harshapriya.n@intel.com Date: Thu, 4 Jan 2018 16:55:16 +0530 Subject: [PATCH] ASoC: Intel: kbl: Enable mclk and ssp sclk early
rt5663 needs mclk/sclk early to synchronize its internal clocks. Enable these clocks early.
Signed-off-by: Harsha Priya harshapriya.n@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Sriram Periyasamy sriramx.periyasamy@intel.com Acked-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/boards/Kconfig | 1 + sound/soc/intel/boards/kbl_rt5663_max98927.c | 95 +++++++++++++++++++++++++++- 2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index d4e103615f51..fefb1ee9fec6 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -235,6 +235,7 @@ config SND_SOC_INTEL_KBL_RT5663_MAX98927_MACH select SND_SOC_MAX98927 select SND_SOC_DMIC select SND_SOC_HDAC_HDMI + select SND_SOC_INTEL_SKYLAKE_SSP_CLK help This adds support for ASoC Onboard Codec I2S machine driver. This will create an alsa sound card for RT5663 + MAX98927. diff --git a/sound/soc/intel/boards/kbl_rt5663_max98927.c b/sound/soc/intel/boards/kbl_rt5663_max98927.c index bf7014ca486f..f5df6bca3156 100644 --- a/sound/soc/intel/boards/kbl_rt5663_max98927.c +++ b/sound/soc/intel/boards/kbl_rt5663_max98927.c @@ -28,6 +28,9 @@ #include "../../codecs/rt5663.h" #include "../../codecs/hdac_hdmi.h" #include "../skylake/skl.h" +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/clkdev.h>
#define KBL_REALTEK_CODEC_DAI "rt5663-aif" #define KBL_MAXIM_CODEC_DAI "max98927-aif1" @@ -48,6 +51,8 @@ struct kbl_hdmi_pcm { struct kbl_rt5663_private { struct snd_soc_jack kabylake_headset; struct list_head hdmi_pcm_list; + struct clk *mclk; + struct clk *sclk; };
enum { @@ -69,6 +74,61 @@ static const struct snd_kcontrol_new kabylake_controls[] = { SOC_DAPM_PIN_SWITCH("Right Spk"), };
+static int platform_clock_control(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *k, int event) +{ + struct snd_soc_dapm_context *dapm = w->dapm; + struct snd_soc_card *card = dapm->card; + struct kbl_rt5663_private *priv = snd_soc_card_get_drvdata(card); + int ret = 0; + + /* + * MCLK/SCLK need to be ON early for a successful synchronization of + * codec internal clock. And the clocks are turned off during + * POST_PMD after the stream is stopped. + */ + switch (event) { + case SND_SOC_DAPM_PRE_PMU: + /* Enable MCLK */ + ret = clk_set_rate(priv->mclk, 24000000); + if (ret < 0) { + dev_err(card->dev, "Can't set rate for mclk, err: %d\n", + ret); + return ret; + } + + ret = clk_prepare_enable(priv->mclk); + if (ret < 0) { + dev_err(card->dev, "Can't enable mclk, err: %d\n", ret); + return ret; + } + + /* Enable SCLK */ + ret = clk_set_rate(priv->sclk, 3072000); + if (ret < 0) { + dev_err(card->dev, "Can't set rate for sclk, err: %d\n", + ret); + clk_disable_unprepare(priv->mclk); + return ret; + } + + ret = clk_prepare_enable(priv->sclk); + if (ret < 0) { + dev_err(card->dev, "Can't enable sclk, err: %d\n", ret); + clk_disable_unprepare(priv->mclk); + } + break; + case SND_SOC_DAPM_POST_PMD: + clk_disable_unprepare(priv->mclk); + clk_disable_unprepare(priv->sclk); + break; + default: + return 0; + } + + return 0; +} + static const struct snd_soc_dapm_widget kabylake_widgets[] = { SND_SOC_DAPM_HP("Headphone Jack", NULL), SND_SOC_DAPM_MIC("Headset Mic", NULL), @@ -78,11 +138,14 @@ static const struct snd_soc_dapm_widget kabylake_widgets[] = { SND_SOC_DAPM_SPK("HDMI1", NULL), SND_SOC_DAPM_SPK("HDMI2", NULL), SND_SOC_DAPM_SPK("HDMI3", NULL), - + SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0, + platform_clock_control, SND_SOC_DAPM_PRE_PMU | + SND_SOC_DAPM_POST_PMD), };
static const struct snd_soc_dapm_route kabylake_map[] = { /* HP jack connectors - unknown if we have jack detection */ + { "Headphone Jack", NULL, "Platform Clock" }, { "Headphone Jack", NULL, "HPOL" }, { "Headphone Jack", NULL, "HPOR" },
@@ -91,6 +154,7 @@ static const struct snd_soc_dapm_route kabylake_map[] = { { "Right Spk", NULL, "Right BE_OUT" },
/* other jacks */ + { "Headset Mic", NULL, "Platform Clock" }, { "IN1P", NULL, "Headset Mic" }, { "IN1N", NULL, "Headset Mic" }, { "DMic", NULL, "SoC DMIC" }, @@ -901,6 +965,7 @@ static int kabylake_audio_probe(struct platform_device *pdev) { struct kbl_rt5663_private *ctx; struct skl_machine_pdata *pdata; + int ret;
ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC); if (!ctx) @@ -919,6 +984,34 @@ static int kabylake_audio_probe(struct platform_device *pdev) dmic_constraints = pdata->dmic_num == 2 ? &constraints_dmic_2ch : &constraints_dmic_channels;
+ ctx->mclk = devm_clk_get(&pdev->dev, "ssp1_mclk"); + if (IS_ERR(ctx->mclk)) { + ret = PTR_ERR(ctx->mclk); + if (ret == -ENOENT) { + dev_info(&pdev->dev, + "Failed to get ssp1_sclk, defer probe\n"); + return -EPROBE_DEFER; + } + + dev_err(&pdev->dev, "Failed to get ssp1_mclk with err:%d\n", + ret); + return ret; + } + + ctx->sclk = devm_clk_get(&pdev->dev, "ssp1_sclk"); + if (IS_ERR(ctx->sclk)) { + ret = PTR_ERR(ctx->sclk); + if (ret == -ENOENT) { + dev_info(&pdev->dev, + "Failed to get ssp1_sclk, defer probe\n"); + return -EPROBE_DEFER; + } + + dev_err(&pdev->dev, "Failed to get ssp1_sclk with err:%d\n", + ret); + return ret; + } + return devm_snd_soc_register_card(&pdev->dev, kabylake_audio_card); }
From: Naveen M naveen.m@intel.com
rt5663 and rt5514 needs mclk/sclk early to synchronize its internal clocks.
Signed-off-by: Naveen M naveen.m@intel.com Signed-off-by: Harsha Priya harshapriya.n@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Sriram Periyasamy sriramx.periyasamy@intel.com --- sound/soc/intel/boards/Kconfig | 1 + .../soc/intel/boards/kbl_rt5663_rt5514_max98927.c | 94 ++++++++++++++++++++++ 2 files changed, 95 insertions(+)
diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index f62f2ab1481e..bdddab586e95 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -275,6 +275,7 @@ config SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH depends on X86_INTEL_LPSS && I2C && SPI select SND_SOC_INTEL_SST depends on SND_SOC_INTEL_SKYLAKE + select SND_SOC_INTEL_SKYLAKE_SSP_CLK select SND_SOC_RT5663 select SND_SOC_RT5514 select SND_SOC_RT5514_SPI diff --git a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c index 38512f0d1a73..f39f6e2f3db1 100644 --- a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c +++ b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c @@ -30,6 +30,9 @@ #include "../../codecs/rt5663.h" #include "../../codecs/hdac_hdmi.h" #include "../skylake/skl.h" +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/clkdev.h>
#define KBL_REALTEK_CODEC_DAI "rt5663-aif" #define KBL_REALTEK_DMIC_CODEC_DAI "rt5514-aif1" @@ -58,6 +61,8 @@ struct kbl_codec_private { struct snd_soc_jack kabylake_headset; struct list_head hdmi_pcm_list; struct snd_soc_jack kabylake_hdmi[2]; + struct clk *mclk; + struct clk *sclk; };
enum { @@ -79,6 +84,61 @@ static const struct snd_kcontrol_new kabylake_controls[] = { SOC_DAPM_PIN_SWITCH("DMIC"), };
+static int platform_clock_control(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *k, int event) +{ + struct snd_soc_dapm_context *dapm = w->dapm; + struct snd_soc_card *card = dapm->card; + struct kbl_codec_private *priv = snd_soc_card_get_drvdata(card); + int ret = 0; + + /* + * MCLK/SCLK need to be ON early for a successful synchronization of + * codec internal clock. And the clocks are turned off during + * POST_PMD after the stream is stopped. + */ + switch (event) { + case SND_SOC_DAPM_PRE_PMU: + /* Enable MCLK */ + ret = clk_set_rate(priv->mclk, 24000000); + if (ret < 0) { + dev_err(card->dev, "Can't set rate for mclk, err: %d\n", + ret); + return ret; + } + + ret = clk_prepare_enable(priv->mclk); + if (ret < 0) { + dev_err(card->dev, "Can't enable mclk, err: %d\n", ret); + return ret; + } + + /* Enable SCLK */ + ret = clk_set_rate(priv->sclk, 3072000); + if (ret < 0) { + dev_err(card->dev, "Can't set rate for sclk, err: %d\n", + ret); + clk_disable_unprepare(priv->mclk); + return ret; + } + + ret = clk_prepare_enable(priv->sclk); + if (ret < 0) { + dev_err(card->dev, "Can't enable sclk, err: %d\n", ret); + clk_disable_unprepare(priv->mclk); + } + break; + case SND_SOC_DAPM_POST_PMD: + clk_disable_unprepare(priv->mclk); + clk_disable_unprepare(priv->sclk); + break; + default: + return 0; + } + + return 0; +} + static const struct snd_soc_dapm_widget kabylake_widgets[] = { SND_SOC_DAPM_HP("Headphone Jack", NULL), SND_SOC_DAPM_MIC("Headset Mic", NULL), @@ -87,11 +147,15 @@ static const struct snd_soc_dapm_widget kabylake_widgets[] = { SND_SOC_DAPM_MIC("DMIC", NULL), SND_SOC_DAPM_SPK("HDMI1", NULL), SND_SOC_DAPM_SPK("HDMI2", NULL), + SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0, + platform_clock_control, SND_SOC_DAPM_PRE_PMU | + SND_SOC_DAPM_POST_PMD),
};
static const struct snd_soc_dapm_route kabylake_map[] = { /* Headphones */ + { "Headphone Jack", NULL, "Platform Clock" }, { "Headphone Jack", NULL, "HPOL" }, { "Headphone Jack", NULL, "HPOR" },
@@ -100,6 +164,7 @@ static const struct snd_soc_dapm_route kabylake_map[] = { { "Right Spk", NULL, "Right BE_OUT" },
/* other jacks */ + { "Headset Mic", NULL, "Platform Clock" }, { "IN1P", NULL, "Headset Mic" }, { "IN1N", NULL, "Headset Mic" },
@@ -648,6 +713,7 @@ static int kabylake_audio_probe(struct platform_device *pdev) { struct kbl_codec_private *ctx; struct skl_machine_pdata *pdata; + int ret = 0;
ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC); if (!ctx) @@ -663,6 +729,34 @@ static int kabylake_audio_probe(struct platform_device *pdev) dmic_constraints = pdata->dmic_num == 2 ? &constraints_dmic_2ch : &constraints_dmic_channels;
+ ctx->mclk = devm_clk_get(&pdev->dev, "ssp1_mclk"); + if (IS_ERR(ctx->mclk)) { + ret = PTR_ERR(ctx->mclk); + if (ret == -ENOENT) { + dev_info(&pdev->dev, + "Failed to get ssp1_mclk, defer probe\n"); + return -EPROBE_DEFER; + } + + dev_err(&pdev->dev, "Failed to get ssp1_mclk with err:%d\n", + ret); + return ret; + } + + ctx->sclk = devm_clk_get(&pdev->dev, "ssp1_sclk"); + if (IS_ERR(ctx->sclk)) { + ret = PTR_ERR(ctx->sclk); + if (ret == -ENOENT) { + dev_info(&pdev->dev, + "Failed to get ssp1_sclk, defer probe\n"); + return -EPROBE_DEFER; + } + + dev_err(&pdev->dev, "Failed to get ssp1_sclk with err:%d\n", + ret); + return ret; + } + return devm_snd_soc_register_card(&pdev->dev, &kabylake_audio_card); }
From: "Subhransu S. Prusty" subhransu.s.prusty@intel.com
Add more meaning to the IPC replies for easy debugging. Replace the switch case with a lookup table to lookup for the IPC replies and print in human readable form.
Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Sriram Periyasamy sriramx.periyasamy@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com --- sound/soc/intel/skylake/skl-sst-ipc.c | 44 ++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 11 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-sst-ipc.c b/sound/soc/intel/skylake/skl-sst-ipc.c index 5234fafb758a..8708755a8f9a 100644 --- a/sound/soc/intel/skylake/skl-sst-ipc.c +++ b/sound/soc/intel/skylake/skl-sst-ipc.c @@ -392,18 +392,43 @@ int skl_ipc_process_notification(struct sst_generic_ipc *ipc, return 0; }
-static int skl_ipc_set_reply_error_code(u32 reply) +struct skl_ipc_err_map { + const char *msg; + enum skl_ipc_glb_reply reply; + int err; +}; + +static struct skl_ipc_err_map skl_err_map[] = { + {"DSP out of memory", IPC_GLB_REPLY_OUT_OF_MEMORY, -ENOMEM}, + {"DSP busy", IPC_GLB_REPLY_BUSY, -EBUSY}, +}; + +static int skl_ipc_set_reply_error_code(struct sst_generic_ipc *ipc, u32 reply) { - switch (reply) { - case IPC_GLB_REPLY_OUT_OF_MEMORY: - return -ENOMEM; + int i;
- case IPC_GLB_REPLY_BUSY: - return -EBUSY; + for (i = 0; i < ARRAY_SIZE(skl_err_map); i++) { + if (skl_err_map[i].reply == reply) + break; + }
- default: + if (i == ARRAY_SIZE(skl_err_map)) { + dev_err(ipc->dev, "ipc FW reply: %d FW Error Code: %u\n", + reply, + ipc->dsp->fw_ops.get_fw_errcode(ipc->dsp)); return -EINVAL; } + + if (skl_err_map[i].err < 0) + dev_err(ipc->dev, "ipc FW reply: %s FW Error Code: %u\n", + skl_err_map[i].msg, + ipc->dsp->fw_ops.get_fw_errcode(ipc->dsp)); + else + dev_info(ipc->dev, "ipc FW reply: %s FW Error Code: %u\n", + skl_err_map[i].msg, + ipc->dsp->fw_ops.get_fw_errcode(ipc->dsp)); + + return skl_err_map[i].err; }
void skl_ipc_process_reply(struct sst_generic_ipc *ipc, @@ -441,10 +466,7 @@ void skl_ipc_process_reply(struct sst_generic_ipc *ipc,
} } else { - msg->errno = skl_ipc_set_reply_error_code(reply); - dev_err(ipc->dev, "ipc FW reply: reply=%d\n", reply); - dev_err(ipc->dev, "FW Error Code: %u\n", - ipc->dsp->fw_ops.get_fw_errcode(ipc->dsp)); + msg->errno = skl_ipc_set_reply_error_code(ipc, reply); switch (IPC_GLB_NOTIFY_MSG_TYPE(header.primary)) { case IPC_GLB_LOAD_MULTIPLE_MODS: case IPC_GLB_LOAD_LIBRARY:
I would also suggest to make similar changes for FW error code, since it doesn't give any clue what does FW error code indicates.
-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Sriram Periyasamy Sent: Sunday, December 10, 2017 11:46 PM To: ALSA ML alsa-devel@alsa-project.org; Mark Brown broonie@kernel.org Cc: Takashi Iwai tiwai@suse.de; Periyasamy, SriramX sriramx.periyasamy@intel.com; mturquette@baylibre.com; sboyd@codeaurora.org; Patches Audio patches.audio@intel.com; Liam Girdwood liam.r.girdwood@linux.intel.com; Koul, Vinod vinod.koul@intel.com; Prusty, Subhransu S subhransu.s.prusty@intel.com; linux-clk@vger.kernel.org Subject: [alsa-devel] [PATCH v5 5/6] ASoC: Intel: Skylake: Make DSP replies more human readable
From: "Subhransu S. Prusty" subhransu.s.prusty@intel.com
Add more meaning to the IPC replies for easy debugging. Replace the switch case with a lookup table to lookup for the IPC replies and print in human readable form.
Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Sriram Periyasamy sriramx.periyasamy@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com --- sound/soc/intel/skylake/skl-sst-ipc.c | 44 ++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 11 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-sst-ipc.c b/sound/soc/intel/skylake/skl-sst-ipc.c index 5234fafb758a..8708755a8f9a 100644 --- a/sound/soc/intel/skylake/skl-sst-ipc.c +++ b/sound/soc/intel/skylake/skl-sst-ipc.c @@ -392,18 +392,43 @@ int skl_ipc_process_notification(struct sst_generic_ipc *ipc, return 0; }
-static int skl_ipc_set_reply_error_code(u32 reply) +struct skl_ipc_err_map { + const char *msg; + enum skl_ipc_glb_reply reply; + int err; +}; + +static struct skl_ipc_err_map skl_err_map[] = { + {"DSP out of memory", IPC_GLB_REPLY_OUT_OF_MEMORY, -ENOMEM}, + {"DSP busy", IPC_GLB_REPLY_BUSY, -EBUSY}, }; + +static int skl_ipc_set_reply_error_code(struct sst_generic_ipc *ipc, +u32 reply) { - switch (reply) { - case IPC_GLB_REPLY_OUT_OF_MEMORY: - return -ENOMEM; + int i;
- case IPC_GLB_REPLY_BUSY: - return -EBUSY; + for (i = 0; i < ARRAY_SIZE(skl_err_map); i++) { + if (skl_err_map[i].reply == reply) + break; + }
- default: + if (i == ARRAY_SIZE(skl_err_map)) { + dev_err(ipc->dev, "ipc FW reply: %d FW Error Code: %u\n", + reply, + ipc->dsp->fw_ops.get_fw_errcode(ipc->dsp)); return -EINVAL; } + + if (skl_err_map[i].err < 0) + dev_err(ipc->dev, "ipc FW reply: %s FW Error Code: %u\n", + skl_err_map[i].msg, + ipc->dsp->fw_ops.get_fw_errcode(ipc->dsp)); + else + dev_info(ipc->dev, "ipc FW reply: %s FW Error Code: %u\n", + skl_err_map[i].msg, + ipc->dsp->fw_ops.get_fw_errcode(ipc->dsp)); + + return skl_err_map[i].err; }
void skl_ipc_process_reply(struct sst_generic_ipc *ipc, @@ -441,10 +466,7 @@ void skl_ipc_process_reply(struct sst_generic_ipc *ipc,
} } else { - msg->errno = skl_ipc_set_reply_error_code(reply); - dev_err(ipc->dev, "ipc FW reply: reply=%d\n", reply); - dev_err(ipc->dev, "FW Error Code: %u\n", - ipc->dsp->fw_ops.get_fw_errcode(ipc->dsp)); + msg->errno = skl_ipc_set_reply_error_code(ipc, reply); switch (IPC_GLB_NOTIFY_MSG_TYPE(header.primary)) { case IPC_GLB_LOAD_MULTIPLE_MODS: case IPC_GLB_LOAD_LIBRARY: -- 2.7.4
_______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Tue, Dec 12, 2017 at 11:57:07PM +0530, Patel, Chintan M wrote:
I would also suggest to make similar changes for FW error code, since it doesn't give any clue what does FW error code indicates.
First, Why are you top posting ? Second, why are you not wrapping your replies ?
Please never top post on mailing list and wrap your replies.
On the actual question, sure a good suggestion but not relvant in this context. Will check and see if we can do that.
From: "Subhransu S. Prusty" subhransu.s.prusty@intel.com
If mclk/sclk is already running, FW responds with IPC reply MCLK/SCLK already running. Add these to the IPC reply lookup table.
Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Sriram Periyasamy sriramx.periyasamy@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com --- sound/soc/intel/skylake/skl-sst-ipc.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-sst-ipc.c b/sound/soc/intel/skylake/skl-sst-ipc.c index 8708755a8f9a..9f3ce73593ae 100644 --- a/sound/soc/intel/skylake/skl-sst-ipc.c +++ b/sound/soc/intel/skylake/skl-sst-ipc.c @@ -249,6 +249,8 @@ enum skl_ipc_glb_reply { IPC_GLB_REPLY_INVALID_CONFIG_DATA_LEN = 121, IPC_GLB_REPLY_GATEWAY_NOT_INITIALIZED = 140, IPC_GLB_REPLY_GATEWAY_NOT_EXIST = 141, + IPC_GLB_REPLY_SCLK_ALREADY_RUNNING = 150, + IPC_GLB_REPLY_MCLK_ALREADY_RUNNING = 151,
IPC_GLB_REPLY_PPL_NOT_INITIALIZED = 160, IPC_GLB_REPLY_PPL_NOT_EXIST = 161, @@ -401,6 +403,10 @@ struct skl_ipc_err_map { static struct skl_ipc_err_map skl_err_map[] = { {"DSP out of memory", IPC_GLB_REPLY_OUT_OF_MEMORY, -ENOMEM}, {"DSP busy", IPC_GLB_REPLY_BUSY, -EBUSY}, + {"SCLK already running", IPC_GLB_REPLY_SCLK_ALREADY_RUNNING, + IPC_GLB_REPLY_SCLK_ALREADY_RUNNING}, + {"MCLK already running", IPC_GLB_REPLY_MCLK_ALREADY_RUNNING, + IPC_GLB_REPLY_MCLK_ALREADY_RUNNING}, };
static int skl_ipc_set_reply_error_code(struct sst_generic_ipc *ipc, u32 reply)
participants (6)
-
Mark Brown
-
Patel, Chintan M
-
Sriram Periyasamy
-
Stephen Boyd
-
Subhransu S. Prusty
-
Vinod Koul