Skip to content

ipc: add DAI_HW_FREE IPC command #4262

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
brentlu wants to merge 3 commits into from
Closed

ipc: add DAI_HW_FREE IPC command #4262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c133a7
dai: add dai_hw_free routine
bardliao May 20, 2021
0debc3c
ipc: add DAI_HW_FREE IPC command
brentlu May 28, 2021
5a60205
ssp: add hw_params and hw_free dai ops
bardliao May 20, 2021
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
17 changes: 17 additions & 0 deletions src/drivers/intel/ssp/ssp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,21 @@ static int ssp_get_hw_params(struct dai *dai,
return 0;
}

/* SSP hw params */
static int ssp_hw_params(struct dai *dai,
struct sof_ipc_stream_params *params)
{
dai_info(dai, "ssp_hw_params()");

return 0;
}

/* SSP hw free */
static void ssp_hw_free(struct dai *dai)
{
dai_info(dai, "ssp_hw_free()");
}

/* start the SSP for either playback or capture */
static void ssp_start(struct dai *dai, int direction)
{
Expand Down Expand Up @@ -949,6 +964,8 @@ const struct dai_driver ssp_driver = {
.get_hw_params = ssp_get_hw_params,
.get_handshake = ssp_get_handshake,
.get_fifo = ssp_get_fifo,
.hw_params = ssp_hw_params,
.hw_free = ssp_hw_free,
.probe = ssp_probe,
.remove = ssp_remove,
},
Expand Down
7 changes: 7 additions & 0 deletions src/include/ipc/dai.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,11 @@ struct sof_ipc_dai_config {
};
} __attribute__((packed, aligned(4)));

/* DAI hardware free */
struct sof_ipc_dai_hw_free {
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 */
} __attribute__((packed, aligned(4)));

#endif /* __IPC_DAI_H__ */
1 change: 1 addition & 0 deletions src/include/ipc/header.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@
*/
#define SOF_IPC_DAI_CONFIG SOF_CMD_TYPE(0x001)
#define SOF_IPC_DAI_LOOPBACK SOF_CMD_TYPE(0x002)
#define SOF_IPC_DAI_HW_FREE SOF_CMD_TYPE(0x004)

/** @} */

Expand Down
7 changes: 7 additions & 0 deletions src/include/sof/lib/dai.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ struct dai_ops {
int (*get_hw_params)(struct dai *dai,
struct sof_ipc_stream_params *params, int dir);
int (*hw_params)(struct dai *dai, struct sof_ipc_stream_params *params);
void (*hw_free)(struct dai *dai);
int (*get_handshake)(struct dai *dai, int direction, int stream_id);
int (*get_fifo)(struct dai *dai, int direction, int stream_id);
int (*probe)(struct dai *dai);
Expand Down Expand Up @@ -423,6 +424,12 @@ static inline int dai_hw_params(struct dai *dai,
return ret;
}

static inline void dai_hw_free(struct dai *dai)
{
if (dai->drv->ops.hw_free)
dai->drv->ops.hw_free(dai);
}

/**
* \brief Get Digital Audio interface DMA Handshake
*/
Expand Down
30 changes: 30 additions & 0 deletions src/ipc/handler-ipc3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,33 @@ static int ipc_msg_dai_config(uint32_t header)
return ipc_comp_dai_config(ipc, &config_dai, ipc->comp_data);
}

static int ipc_msg_dai_hw_free(uint32_t header)
{
struct ipc *ipc = ipc_get();
struct sof_ipc_dai_hw_free hw_free;
struct dai *dai;

/* copy message with ABI safe method */
IPC_COPY_CMD(hw_free, ipc->comp_data);

tr_info(&ipc_tr, "ipc: dai %d.%d -> hw_free", hw_free.type,
hw_free.dai_index);

/* get DAI */
dai = dai_get(hw_free.type, hw_free.dai_index, 0 /* existing only */);
if (!dai) {
tr_err(&ipc_tr, "ipc: dai %d,%d not found", hw_free.type,
hw_free.dai_index);
return -ENODEV;
}

/* DAI hardware free */
dai_hw_free(dai);
dai_put(dai); /* free ref immediately */

return 0;
}

static int ipc_glb_dai_message(uint32_t header)
{
uint32_t cmd = iCS(header);
Expand All @@ -569,6 +596,9 @@ static int ipc_glb_dai_message(uint32_t header)
return ipc_msg_dai_config(header);
case SOF_IPC_DAI_LOOPBACK:
//return ipc_comp_set_value(header, COMP_CMD_LOOPBACK);
return -EINVAL;
case SOF_IPC_DAI_HW_FREE:
return ipc_msg_dai_hw_free(header);
Copy link
Contributor

@keyonjie keyonjie May 31, 2021

Choose a reason for hiding this comment

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

We already have SOF_IPC_STREAM_PCM_FREE which is sent at ALSA PCM .hw_free(), why we need a new IPC, just call dai_hw_free() in dai_reset() as @bardliao do in PR #4219 is good enough?

Copy link
Contributor Author

@brentlu brentlu May 31, 2021

Choose a reason for hiding this comment

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

For two reasons, first the PCM_FREE is 'usually' sent in trigger stop now. Second, we need to send something in BE DAI's hw_free instead of FE DAI or the SOF will still stop the bclk too early.

DROP:
sof's trigger()
=> send STREAM_TRIG_STOP
=> send STREAM_PCM_FREE
HW_FREE:
sof's hw_free() (FE)
=> too early, codec could do nothing
codec's mute_stream()
=> codec could handle it's PLL here
sof's hw_free() (BE)
=> send DAI_HW_FREE to SOF to stop bclk
codec's hw_free()

Copy link
Contributor Author

@brentlu brentlu May 31, 2021

Choose a reason for hiding this comment

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

BTW, this PR is meant to be merged to #4219 if agreed. To fix the issue found by codec vendor when validating #4219.

Copy link
Contributor

@keyonjie keyonjie Jun 1, 2021

Choose a reason for hiding this comment

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

got the issue now @brentlu
IMO we can still use the existed _IPC_STREAM_PCM_FREE but change the point of sending it, e.g. send it later, at the point where we want the clocks to be freed.

Copy link
Contributor Author

@brentlu brentlu Jun 1, 2021

Choose a reason for hiding this comment

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

It seems weird for me to move it from FE to BE DAI Link since this command deals with PCM pipe...

default:
tr_err(&ipc_tr, "ipc: unknown DAI cmd 0x%x", cmd);
return -EINVAL;
Expand Down