On 2019-08-23 21:21, Pierre-Louis Bossart wrote:
On 8/22/19 2:04 PM, Cezary Rojewski wrote:
With all fields deprecated, yet another struct can be removed. Let's do so and don't forget about any functions or fields tied to its existence.
SKL/KBL and all others have different firmware loading mechanisms, so now how is the difference handled?
skl_dsp_fw_ops::load_fw / load_library and friends. skl-sst.c holds control over CL_DMA (which is implemented in separated _cldma files within /skylake) loading mechanism while struct that is being removed here - skl_dsp_loader_ops - has been used for APL/ CNL only. The SKL equivalent is a stub -> alloc_dma_buf / free_dma_buf which are shared across all platforms.
It's quite obvious that with proper design one does not need 12 functions and that many types to handle just the loading process. Removal of these has also learning value - anyone coming to help us with /skylake quickly hits the unreadability wall. Clearer the code, lower the learning curve.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com
sound/soc/intel/common/sst-dsp-priv.h | 1 - sound/soc/intel/skylake/bxt-sst.c | 5 ++-- sound/soc/intel/skylake/cnl-sst-dsp.h | 3 +- sound/soc/intel/skylake/cnl-sst.c | 5 ++-- sound/soc/intel/skylake/skl-messages.c | 40 +------------------------ sound/soc/intel/skylake/skl-sst-dsp.h | 24 ++------------- sound/soc/intel/skylake/skl-sst-utils.c | 3 +- sound/soc/intel/skylake/skl-sst.c | 5 ++-- sound/soc/intel/skylake/skl.h | 2 -- 9 files changed, 12 insertions(+), 76 deletions(-)
diff --git a/sound/soc/intel/common/sst-dsp-priv.h b/sound/soc/intel/common/sst-dsp-priv.h index 8a7009937d59..0fe9bebcfb38 100644 --- a/sound/soc/intel/common/sst-dsp-priv.h +++ b/sound/soc/intel/common/sst-dsp-priv.h @@ -306,7 +306,6 @@ struct sst_dsp { const char *fw_name; /* To allocate CL dma buffers */ - struct skl_dsp_loader_ops dsp_ops; struct skl_dsp_fw_ops fw_ops; int sst_state; struct skl_cl_dev cl_dev; diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c index 45da02f74f4d..c7961050c2ee 100644 --- a/sound/soc/intel/skylake/bxt-sst.c +++ b/sound/soc/intel/skylake/bxt-sst.c @@ -546,14 +546,13 @@ static struct sst_pdata skl_dev = { }; int bxt_sst_dsp_init(struct device *dev, void __iomem *mmio_base, int irq, - const char *fw_name, struct skl_dsp_loader_ops dsp_ops, - struct skl_dev **dsp) + const char *fw_name, struct skl_dev **dsp) { struct skl_dev *skl; struct sst_dsp *sst; int ret; - ret = skl_sst_ctx_init(dev, irq, fw_name, dsp_ops, dsp, &skl_dev); + ret = skl_sst_ctx_init(dev, irq, fw_name, dsp, &skl_dev); if (ret < 0) { dev_err(dev, "%s: no device\n", __func__); return ret; diff --git a/sound/soc/intel/skylake/cnl-sst-dsp.h b/sound/soc/intel/skylake/cnl-sst-dsp.h index 30b586acc858..70da4f312f53 100644 --- a/sound/soc/intel/skylake/cnl-sst-dsp.h +++ b/sound/soc/intel/skylake/cnl-sst-dsp.h @@ -94,7 +94,6 @@ void cnl_ipc_op_int_disable(struct sst_dsp *ctx); bool cnl_ipc_int_status(struct sst_dsp *ctx); int cnl_sst_dsp_init(struct device *dev, void __iomem *mmio_base, int irq, - const char *fw_name, struct skl_dsp_loader_ops dsp_ops, - struct skl_dev **dsp); + const char *fw_name, struct skl_dev **dsp); #endif /*__CNL_SST_DSP_H__*/ diff --git a/sound/soc/intel/skylake/cnl-sst.c b/sound/soc/intel/skylake/cnl-sst.c index 4d9a4d5886f4..2f10076cc906 100644 --- a/sound/soc/intel/skylake/cnl-sst.c +++ b/sound/soc/intel/skylake/cnl-sst.c @@ -423,14 +423,13 @@ static struct sst_pdata cnl_dev = { }; int cnl_sst_dsp_init(struct device *dev, void __iomem *mmio_base, int irq, - const char *fw_name, struct skl_dsp_loader_ops dsp_ops, - struct skl_dev **dsp) + const char *fw_name, struct skl_dev **dsp) { struct skl_dev *cnl; struct sst_dsp *sst; int ret; - ret = skl_sst_ctx_init(dev, irq, fw_name, dsp_ops, dsp, &cnl_dev); + ret = skl_sst_ctx_init(dev, irq, fw_name, dsp, &cnl_dev); if (ret < 0) { dev_err(dev, "%s: no device\n", __func__); return ret; diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c index 20ab980fe8a1..6fbceec67b53 100644 --- a/sound/soc/intel/skylake/skl-messages.c +++ b/sound/soc/intel/skylake/skl-messages.c @@ -140,72 +140,37 @@ int skl_dsp_cleanup(struct device *dev, return 0; } -static struct skl_dsp_loader_ops skl_get_loader_ops(void) -{ - struct skl_dsp_loader_ops loader_ops;
- memset(&loader_ops, 0, sizeof(struct skl_dsp_loader_ops));
- loader_ops.alloc_dma_buf = skl_alloc_dma_buf; - loader_ops.free_dma_buf = skl_free_dma_buf;
- return loader_ops; -};
-static struct skl_dsp_loader_ops bxt_get_loader_ops(void) -{ - struct skl_dsp_loader_ops loader_ops;
- memset(&loader_ops, 0, sizeof(loader_ops));
- loader_ops.alloc_dma_buf = skl_alloc_dma_buf; - loader_ops.free_dma_buf = skl_free_dma_buf; - loader_ops.prepare = skl_dsp_prepare; - loader_ops.trigger = skl_dsp_trigger; - loader_ops.cleanup = skl_dsp_cleanup;
- return loader_ops; -};
static const struct skl_dsp_ops dsp_ops[] = { { .id = 0x9d70, - .loader_ops = skl_get_loader_ops, .init = skl_sst_dsp_init, }, { .id = 0x9d71, - .loader_ops = skl_get_loader_ops, .init = skl_sst_dsp_init, }, { .id = 0x5a98, - .loader_ops = bxt_get_loader_ops, .init = bxt_sst_dsp_init, }, { .id = 0x3198, - .loader_ops = bxt_get_loader_ops, .init = bxt_sst_dsp_init, }, { .id = 0x9dc8, - .loader_ops = bxt_get_loader_ops, .init = cnl_sst_dsp_init, }, { .id = 0xa348, - .loader_ops = bxt_get_loader_ops, .init = cnl_sst_dsp_init, }, { .id = 0x02c8, - .loader_ops = bxt_get_loader_ops, .init = cnl_sst_dsp_init, }, { .id = 0x06c8, - .loader_ops = bxt_get_loader_ops, .init = cnl_sst_dsp_init, }, }; @@ -226,7 +191,6 @@ int skl_init_dsp(struct skl_dev *skl) { void __iomem *mmio_base; struct hdac_bus *bus = skl_to_bus(skl); - struct skl_dsp_loader_ops loader_ops; int irq = bus->irq; const struct skl_dsp_ops *ops; int ret; @@ -248,10 +212,8 @@ int skl_init_dsp(struct skl_dev *skl) goto unmap_mmio; } - loader_ops = ops->loader_ops(); ret = ops->init(bus->dev, mmio_base, irq, - skl->fw_name, loader_ops, - &skl); + skl->fw_name, &skl); if (ret < 0) goto unmap_mmio; diff --git a/sound/soc/intel/skylake/skl-sst-dsp.h b/sound/soc/intel/skylake/skl-sst-dsp.h index 97e16a602331..9f2dae92c1c9 100644 --- a/sound/soc/intel/skylake/skl-sst-dsp.h +++ b/sound/soc/intel/skylake/skl-sst-dsp.h @@ -150,22 +150,6 @@ struct skl_dsp_fw_ops { }; -struct skl_dsp_loader_ops { - int stream_tag;
- int (*alloc_dma_buf)(struct device *dev, - struct snd_dma_buffer *dmab, size_t size); - int (*free_dma_buf)(struct device *dev, - struct snd_dma_buffer *dmab); - int (*prepare)(struct device *dev, unsigned int format, - unsigned int byte_size, - struct snd_dma_buffer *bufp); - int (*trigger)(struct device *dev, bool start, int stream_tag);
- int (*cleanup)(struct device *dev, struct snd_dma_buffer *dmab, - int stream_tag); -};
#define MAX_INSTANCE_BUFF 2 struct uuid_module { @@ -227,11 +211,9 @@ int skl_dsp_put_core(struct sst_dsp *ctx, unsigned int core_id); int skl_dsp_boot(struct sst_dsp *ctx); int skl_sst_dsp_init(struct device *dev, void __iomem *mmio_base, int irq, - const char *fw_name, struct skl_dsp_loader_ops dsp_ops, - struct skl_dev **dsp); + const char *fw_name, struct skl_dev **dsp); int bxt_sst_dsp_init(struct device *dev, void __iomem *mmio_base, int irq, - const char *fw_name, struct skl_dsp_loader_ops dsp_ops, - struct skl_dev **dsp); + const char *fw_name, struct skl_dev **dsp); int bxt_load_library(struct sst_dsp *ctx, struct skl_lib_info *linfo, int lib_count); @@ -248,7 +230,7 @@ int skl_dsp_strip_extended_manifest(struct firmware *fw); void skl_dsp_set_astate_cfg(struct skl_dev *skl, u32 cnt, void *data); int skl_sst_ctx_init(struct device *dev, int irq, const char *fw_name, - struct skl_dsp_loader_ops dsp_ops, struct skl_dev **dsp, + struct skl_dev **dsp, struct sst_pdata *pdata); int skl_prepare_lib_load(struct skl_dev *skl, struct skl_lib_info *linfo, struct firmware *stripped_fw, diff --git a/sound/soc/intel/skylake/skl-sst-utils.c b/sound/soc/intel/skylake/skl-sst-utils.c index 8e03a10855c4..bbe67e298efe 100644 --- a/sound/soc/intel/skylake/skl-sst-utils.c +++ b/sound/soc/intel/skylake/skl-sst-utils.c @@ -355,7 +355,7 @@ int skl_dsp_strip_extended_manifest(struct firmware *fw) } int skl_sst_ctx_init(struct device *dev, int irq, const char *fw_name, - struct skl_dsp_loader_ops dsp_ops, struct skl_dev **dsp, + struct skl_dev **dsp, struct sst_pdata *pdata) { struct skl_dev *skl = *dsp; @@ -375,7 +375,6 @@ int skl_sst_ctx_init(struct device *dev, int irq, const char *fw_name, sst = skl->dsp; sst->fw_name = fw_name; - sst->dsp_ops = dsp_ops; init_waitqueue_head(&skl->mod_load_wait); INIT_LIST_HEAD(&sst->module_list); diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c index 6bb003add9e2..e9a7b2509b35 100644 --- a/sound/soc/intel/skylake/skl-sst.c +++ b/sound/soc/intel/skylake/skl-sst.c @@ -517,14 +517,13 @@ static struct sst_pdata skl_dev = { }; int skl_sst_dsp_init(struct device *dev, void __iomem *mmio_base, int irq, - const char *fw_name, struct skl_dsp_loader_ops dsp_ops, - struct skl_dev **dsp) + const char *fw_name, struct skl_dev **dsp) { struct skl_dev *skl; struct sst_dsp *sst; int ret; - ret = skl_sst_ctx_init(dev, irq, fw_name, dsp_ops, dsp, &skl_dev); + ret = skl_sst_ctx_init(dev, irq, fw_name, dsp, &skl_dev); if (ret < 0) { dev_err(dev, "%s: no device\n", __func__); return ret; diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h index 62e2f2d450e4..beeef67cb55f 100644 --- a/sound/soc/intel/skylake/skl.h +++ b/sound/soc/intel/skylake/skl.h @@ -155,10 +155,8 @@ struct skl_machine_pdata { struct skl_dsp_ops { int id; - struct skl_dsp_loader_ops (*loader_ops)(void); int (*init)(struct device *dev, void __iomem *mmio_base, int irq, const char *fw_name, - struct skl_dsp_loader_ops loader_ops, struct skl_dev **skl_sst); };