Skip to content
Closed
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
5 changes: 5 additions & 0 deletions include/sound/sof.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <sound/soc.h>
#include <sound/soc-acpi.h>

struct sof_dai_clk;
struct snd_sof_dsp_ops;

/*
Expand Down Expand Up @@ -52,6 +53,10 @@ struct snd_sof_pdata {
struct platform_device *pdev_mach;
const struct snd_soc_acpi_mach *machine;

/* dai clocks */
struct sof_dai_clk *dai_clks;
int num_clks;

void *hw_pdata;
};

Expand Down
10 changes: 10 additions & 0 deletions include/sound/sof/dai-intel.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,16 @@
/* DMIC max. four controllers for eight microphone channels */
#define SOF_DAI_INTEL_DMIC_NUM_CTRL 4

/* SSP clocks to enable/disable
*
* Macros for clk_id field in sof_ipc_dai_ssp_clkctrl struct.
*/

/* mclk */
#define SOF_DAI_INTEL_CLKCTRL_MCLK 1
/* bclk */
#define SOF_DAI_INTEL_CLKCTRL_BCLK 2

/* SSP Configuration Request - SOF_IPC_DAI_SSP_CONFIG */
struct sof_ipc_dai_ssp_params {
struct sof_ipc_hdr hdr;
Expand Down
10 changes: 10 additions & 0 deletions include/sound/sof/dai.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,14 @@ struct sof_ipc_dai_config {
};
} __packed;

/* general purpose DAI clock control */
struct sof_ipc_dai_clkctrl {
struct sof_ipc_cmd_hdr hdr;
uint32_t type; /**< DAI type - enum sof_ipc_dai_type */
uint32_t dai_index; /**< index of this type dai */

uint32_t clk_en; /**< zero to disable the clock */
uint32_t clk_id; /**< clock id to operate */

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

how sure are we that this is all we'll ever need? Can it be that at some point we'll also need to send a rate down? Or maybe even the duty cycle (ok, maybe not this one)?

} __packed;

#endif
1 change: 1 addition & 0 deletions include/sound/sof/header.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
/* DAI messages */
#define SOF_IPC_DAI_CONFIG SOF_CMD_TYPE(0x001)
#define SOF_IPC_DAI_LOOPBACK SOF_CMD_TYPE(0x002)
#define SOF_IPC_DAI_CLKCTRL SOF_CMD_TYPE(0x003)

/* stream */
#define SOF_IPC_STREAM_PCM_PARAMS SOF_CMD_TYPE(0x001)
Expand Down
2 changes: 1 addition & 1 deletion sound/soc/sof/Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)

snd-sof-objs := core.o ops.o loader.o ipc.o pcm.o pm.o debug.o topology.o\
control.o trace.o utils.o sof-audio.o
control.o trace.o utils.o sof-audio.o sof-dai-clk.o
snd-sof-$(CONFIG_SND_SOC_SOF_DEBUG_PROBES) += probe.o compress.o

snd-sof-pci-objs := sof-pci-dev.o
Expand Down
2 changes: 2 additions & 0 deletions sound/soc/sof/ipc.c
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ static void ipc_log_header(struct device *dev, u8 *text, u32 cmd)
str2 = "CONFIG"; break;
case SOF_IPC_DAI_LOOPBACK:
str2 = "LOOPBACK"; break;
case SOF_IPC_DAI_CLKCTRL:
str2 = "CLKCTRL"; break;
default:
str2 = "unknown type"; break;
}
Expand Down
11 changes: 11 additions & 0 deletions sound/soc/sof/pcm.c
Original file line number Diff line number Diff line change
Expand Up @@ -862,11 +862,22 @@ static int sof_pcm_probe(struct snd_soc_component *component)
return ret;
}

/* Register DAI clock control */
ret = snd_sof_dai_clks_probe(component);
if (ret) {
dev_err(component->dev, "error: failed to register clock control %d\n",
ret);
return ret;
}

return ret;
}

static void sof_pcm_remove(struct snd_soc_component *component)
{
/* remove DAI clock control */
snd_sof_dai_clks_remove(component);

/* remove topology */
snd_soc_tplg_component_remove(component);
}
Expand Down
153 changes: 153 additions & 0 deletions sound/soc/sof/sof-dai-clk.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
//
// 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) 2021 Intel Corporation. All rights reserved.
//
// Author: Brent Lu <brent.lu@intel.com>
//
// DAI clock layer to support explicit clock control in machine driver.
//

#include <linux/clk-provider.h>
#include <linux/clkdev.h>
#include <sound/sof/dai.h>
#include "sof-priv.h"
#include "sof-dai-clk.h"

struct sof_clk {
struct clk_hw hw;
struct snd_sof_dev *sdev;
struct sof_dai_clk *dai_clk;
};

static int sof_clk_ctrl(struct snd_sof_dev *sdev, struct sof_dai_clk *dai_clk, int enable)
{
struct sof_ipc_dai_clkctrl clkctrl;
struct sof_ipc_reply reply;
int ret;

memset(&clkctrl, 0, sizeof(clkctrl));

/* set IPC DAI clock control */
clkctrl.hdr.size = sizeof(clkctrl);
clkctrl.hdr.cmd = SOF_IPC_GLB_DAI_MSG | SOF_IPC_DAI_CLKCTRL;

clkctrl.type = dai_clk->type;
clkctrl.dai_index = dai_clk->dai_index;

clkctrl.clk_en = enable ? 1 : 0;
clkctrl.clk_id = dai_clk->clk_id;

dev_dbg(sdev->dev, "clkctrl: SSP%d clk_en %d clk_id %d\n",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is this clock SSP-only? If it is, it should be clear at the file / function level, if it isn't, it shouldn't be hard-coded here

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It might be better to be human readable:
dev_dbg(sdev->dev, "clkctrl: %s (clkid: %d) - %s\n", dai_clk->name, dai_clk->clk_id, enable ? "Enable" : "Disable");

clkctrl.dai_index, clkctrl.clk_en, clkctrl.clk_id);

/* send IPC to the DSP */
ret = sof_ipc_tx_message(sdev->ipc, clkctrl.hdr.cmd, &clkctrl,
sizeof(clkctrl), &reply, sizeof(reply));

if (ret < 0) {
dev_err(sdev->dev,
"failed to control DAI clock for type %d index %d\n",
dai_clk->type, dai_clk->dai_index);
return ret;
}

return 0;
}

static int sof_clk_prepare(struct clk_hw *hw)
{
struct sof_clk *clkdev = container_of(hw, struct sof_clk, hw);
struct snd_sof_dev *sdev = clkdev->sdev;

return sof_clk_ctrl(sdev, clkdev->dai_clk, 1);
}

static void sof_clk_unprepare(struct clk_hw *hw)
{
struct sof_clk *clkdev = container_of(hw, struct sof_clk, hw);
struct snd_sof_dev *sdev = clkdev->sdev;

sof_clk_ctrl(sdev, clkdev->dai_clk, 0);
}

static const struct clk_ops sof_clk_ops = {
.prepare = sof_clk_prepare,
.unprepare = sof_clk_unprepare,
};

int snd_sof_dai_clks_probe(struct snd_soc_component *component)
{
struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component);
struct snd_sof_pdata *pdata = sdev->pdata;
struct device *dev = component->dev;
int i, ret;

sdev->num_clks = pdata->num_clks;

if (sdev->num_clks > SOF_DAI_CLKS)
sdev->num_clks = SOF_DAI_CLKS;

for (i = 0; i < sdev->num_clks; ++i) {
struct clk_init_data init = {};
struct clk_lookup *dai_clk_lookup;
struct sof_clk *clkdev;

clkdev = devm_kzalloc(dev, sizeof(*clkdev), GFP_KERNEL);
if (!clkdev) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this operations should be platform specific. In IMX case we just have the clocks defined in the device tree file (dts) and then just get a reference to the clock here.

Also, with the IMX we enable and configure IP clocks (not only DAI clocks) on the Host side.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@dbaluta, you are right, but it should also depend on firmware version. Do we have the clock support enabled in the firmware?
At the end we will have SOF as a clock provider as well so platforms can move audio related clock implementations into the firmware and manage it via IPC.

@dbaluta dbaluta May 24, 2021 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@ujfalusi for IMX there is little to zero support for IPs clocks in the firmware. 2 out of 3 IMX supported platforms have a dedicated M4 core that is running a firmware which handles the clocks.
I like the idea of making the SOF (DSP) as a clock provider. It will make sense for IMX future platforms.

ret = -ENOMEM;
goto err;
}

init.name = pdata->dai_clks[i].name;
init.ops = &sof_clk_ops;
init.flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_GATE;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The problem here is I don't know the correct flag for the virtual clock. Need expert to comment the flag.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

not an expert in the clock API, but how about 0? :-) So far this clock doesn't even change rate, so why cannot it be cached?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

RATE_GATE might be valid for SCLK they tend to like being stopped in order to be changed, but are we ever going to allow that?
if not then should we use fixed-clock for SOF clocks?

init.parent_names = NULL;
init.num_parents = 0;

clkdev->hw.init = &init;
clkdev->sdev = sdev;
clkdev->dai_clk = &pdata->dai_clks[i];

ret = devm_clk_hw_register(dev, &clkdev->hw);
if (ret) {
dev_warn(dev, "failed to register %s: %d\n",
init.name, ret);
goto err;
}

dai_clk_lookup = clkdev_hw_create(&clkdev->hw, init.name,
NULL);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

NULL would comfortably fit onto the previous line

if (!dai_clk_lookup) {
dev_warn(dev, "failed to create %s: %d\n",
init.name, ret);
ret = -ENOMEM;
goto err;
}

sdev->dai_clks_lookup[i] = dai_clk_lookup;
}

return 0;

err:
do {
if (sdev->dai_clks_lookup[i])
clkdev_drop(sdev->dai_clks_lookup[i]);
} while (i-- > 0);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you can simplify this: from your above code whenever on iteration i you encounter an error and jump to err you know, that all sdev->dai_clks_lookup[0..i-1] have been created successfully, right? So, you could just

while (i-- > 0)
	clkdev_drop(sdev->dai_clks_lookup[i]);

or similar


return ret;
}

void snd_sof_dai_clks_remove(struct snd_soc_component *component)
{
struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component);
int i;

for (i = sdev->num_clks - 1; i >= 0; --i) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

just curious - any specific reason to run this in reverse order?

if (sdev->dai_clks_lookup[i])
clkdev_drop(sdev->dai_clks_lookup[i]);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same here: I assume, this is only called if .probe() above was successful. In that case you know, that all sdev->dai_clks_lookup[0..sdev->num_clks-1] elements have been created successfully

}
19 changes: 19 additions & 0 deletions sound/soc/sof/sof-dai-clk.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
/*
* 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) 2021 Intel Corporation. All rights reserved.
*/

#ifndef __SOUND_SOC_SOF_DAI_CLK_H
#define __SOUND_SOC_SOF_DAI_CLK_H

struct sof_dai_clk {
const char *name;
int type;
int dai_index;
int clk_id;
};

#endif
38 changes: 38 additions & 0 deletions sound/soc/sof/sof-pci-dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,14 @@
#include <sound/soc-acpi.h>
#include <sound/soc-acpi-intel-match.h>
#include <sound/sof.h>
#include <sound/sof/dai.h>
#include "sof-dai-clk.h"
#include "ops.h"
#include "sof-pci-dev.h"

/* platform specific devices */
#include "intel/shim.h"

static char *fw_path;
module_param(fw_path, charp, 0444);
MODULE_PARM_DESC(fw_path, "alternate path for SOF firmware.");
Expand Down Expand Up @@ -110,11 +115,40 @@ static void sof_pci_probe_complete(struct device *dev)
pm_runtime_put_noidle(dev);
}

static struct sof_dai_clk sof_dai_clks[] = {
{.name = "ssp0_mclk", .type = SOF_DAI_INTEL_SSP, .dai_index = 0,
.clk_id = SOF_DAI_INTEL_CLKCTRL_MCLK},
{.name = "ssp0_sclk", .type = SOF_DAI_INTEL_SSP, .dai_index = 0,
.clk_id = SOF_DAI_INTEL_CLKCTRL_BCLK},
{.name = "ssp1_mclk", .type = SOF_DAI_INTEL_SSP, .dai_index = 1,
.clk_id = SOF_DAI_INTEL_CLKCTRL_MCLK},
{.name = "ssp1_sclk", .type = SOF_DAI_INTEL_SSP, .dai_index = 1,
.clk_id = SOF_DAI_INTEL_CLKCTRL_BCLK},
{.name = "ssp2_mclk", .type = SOF_DAI_INTEL_SSP, .dai_index = 2,
.clk_id = SOF_DAI_INTEL_CLKCTRL_MCLK},
{.name = "ssp2_sclk", .type = SOF_DAI_INTEL_SSP, .dai_index = 2,
.clk_id = SOF_DAI_INTEL_CLKCTRL_BCLK},
{.name = "ssp3_mclk", .type = SOF_DAI_INTEL_SSP, .dai_index = 3,
.clk_id = SOF_DAI_INTEL_CLKCTRL_MCLK},
{.name = "ssp3_sclk", .type = SOF_DAI_INTEL_SSP, .dai_index = 3,
.clk_id = SOF_DAI_INTEL_CLKCTRL_BCLK},
{.name = "ssp4_mclk", .type = SOF_DAI_INTEL_SSP, .dai_index = 4,
.clk_id = SOF_DAI_INTEL_CLKCTRL_MCLK},
{.name = "ssp4_sclk", .type = SOF_DAI_INTEL_SSP, .dai_index = 4,
.clk_id = SOF_DAI_INTEL_CLKCTRL_BCLK},
{.name = "ssp5_mclk", .type = SOF_DAI_INTEL_SSP, .dai_index = 5,
.clk_id = SOF_DAI_INTEL_CLKCTRL_MCLK},
{.name = "ssp5_sclk", .type = SOF_DAI_INTEL_SSP, .dai_index = 5,
.clk_id = SOF_DAI_INTEL_CLKCTRL_BCLK},
};

int sof_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id)
{
struct device *dev = &pci->dev;
const struct sof_dev_desc *desc =
(const struct sof_dev_desc *)pci_id->driver_data;
const struct sof_intel_dsp_desc *chip =
(const struct sof_intel_dsp_desc *)desc->chip_info;
struct snd_sof_pdata *sof_pdata;
int ret;

Expand Down Expand Up @@ -177,6 +211,10 @@ int sof_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id)
sof_pdata->tplg_filename_prefix =
sof_pdata->desc->default_tplg_path;

/* 2 clocks per SSP port: mclk and bclk */
sof_pdata->dai_clks = sof_dai_clks;
sof_pdata->num_clks = 2 * chip->ssp_count;

dmi_check_system(sof_tplg_table);
if (sof_override_tplg_name)
sof_pdata->tplg_filename = sof_override_tplg_name;
Expand Down
9 changes: 9 additions & 0 deletions sound/soc/sof/sof-priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ extern int sof_core_debug;
#define SOF_IPC_DSP_REPLY 0
#define SOF_IPC_HOST_REPLY 1

#define SOF_DAI_CLKS 12

/* convenience constructor for DAI driver streams */
#define SOF_DAI_STREAM(sname, scmin, scmax, srates, sfmt) \
{.stream_name = sname, .channels_min = scmin, .channels_max = scmax, \
Expand Down Expand Up @@ -457,6 +459,10 @@ struct snd_sof_dev {
bool dtrace_error;
bool dtrace_draining;

/* DAI clock control */
struct clk_lookup *dai_clks_lookup[SOF_DAI_CLKS];
int num_clks;

bool msi_enabled;

void *private; /* core does not touch this */
Expand Down Expand Up @@ -486,6 +492,9 @@ int snd_sof_create_page_table(struct device *dev,
struct snd_dma_buffer *dmab,
unsigned char *page_table, size_t size);

int snd_sof_dai_clks_probe(struct snd_soc_component *component);
void snd_sof_dai_clks_remove(struct snd_soc_component *component);

/*
* Firmware loading.
*/
Expand Down