[alsa-devel] [PATCH] ASoC: Intel: skl: extract common function
Extract common logic to a function to avoid duplicate code.
Signed-off-by: Tzung-Bi Shih tzungbi@google.com --- Refactor HDMI init to as most Intel machine drivers did, e.g. kbl_da7219_max98357a.c.
.../soc/intel/boards/skl_nau88l25_max98357a.c | 39 +++++------------- sound/soc/intel/boards/skl_nau88l25_ssm4567.c | 40 +++++-------------- 2 files changed, 18 insertions(+), 61 deletions(-)
diff --git a/sound/soc/intel/boards/skl_nau88l25_max98357a.c b/sound/soc/intel/boards/skl_nau88l25_max98357a.c index 3ce8efbeed12..485614c4fa47 100644 --- a/sound/soc/intel/boards/skl_nau88l25_max98357a.c +++ b/sound/soc/intel/boards/skl_nau88l25_max98357a.c @@ -179,7 +179,7 @@ static int skylake_nau8825_codec_init(struct snd_soc_pcm_runtime *rtd) return ret; }
-static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) +static int skylake_hdmi_init(struct snd_soc_pcm_runtime *rtd, int device) { struct skl_nau8825_private *ctx = snd_soc_card_get_drvdata(rtd->card); struct snd_soc_dai *dai = rtd->codec_dai; @@ -189,7 +189,7 @@ static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) if (!pcm) return -ENOMEM;
- pcm->device = SKL_DPCM_AUDIO_HDMI1_PB; + pcm->device = device; pcm->codec_dai = dai;
list_add_tail(&pcm->head, &ctx->hdmi_pcm_list); @@ -197,40 +197,19 @@ static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) return 0; }
-static int skylake_hdmi2_init(struct snd_soc_pcm_runtime *rtd) +static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) { - struct skl_nau8825_private *ctx = snd_soc_card_get_drvdata(rtd->card); - struct snd_soc_dai *dai = rtd->codec_dai; - struct skl_hdmi_pcm *pcm; - - pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL); - if (!pcm) - return -ENOMEM; - - pcm->device = SKL_DPCM_AUDIO_HDMI2_PB; - pcm->codec_dai = dai; - - list_add_tail(&pcm->head, &ctx->hdmi_pcm_list); + return skylake_hdmi_init(rtd, SKL_DPCM_AUDIO_HDMI1_PB); +}
- return 0; +static int skylake_hdmi2_init(struct snd_soc_pcm_runtime *rtd) +{ + return skylake_hdmi_init(rtd, SKL_DPCM_AUDIO_HDMI2_PB); }
static int skylake_hdmi3_init(struct snd_soc_pcm_runtime *rtd) { - struct skl_nau8825_private *ctx = snd_soc_card_get_drvdata(rtd->card); - struct snd_soc_dai *dai = rtd->codec_dai; - struct skl_hdmi_pcm *pcm; - - pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL); - if (!pcm) - return -ENOMEM; - - pcm->device = SKL_DPCM_AUDIO_HDMI3_PB; - pcm->codec_dai = dai; - - list_add_tail(&pcm->head, &ctx->hdmi_pcm_list); - - return 0; + return skylake_hdmi_init(rtd, SKL_DPCM_AUDIO_HDMI3_PB); }
static int skylake_nau8825_fe_init(struct snd_soc_pcm_runtime *rtd) diff --git a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c index 1a7ac8bdf543..772cbd6940db 100644 --- a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c +++ b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c @@ -198,7 +198,7 @@ static int skylake_nau8825_codec_init(struct snd_soc_pcm_runtime *rtd) return ret; }
-static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) +static int skylake_hdmi_init(struct snd_soc_pcm_runtime *rtd, int device) { struct skl_nau88125_private *ctx = snd_soc_card_get_drvdata(rtd->card); struct snd_soc_dai *dai = rtd->codec_dai; @@ -208,7 +208,7 @@ static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) if (!pcm) return -ENOMEM;
- pcm->device = SKL_DPCM_AUDIO_HDMI1_PB; + pcm->device = device; pcm->codec_dai = dai;
list_add_tail(&pcm->head, &ctx->hdmi_pcm_list); @@ -216,41 +216,19 @@ static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) return 0; }
-static int skylake_hdmi2_init(struct snd_soc_pcm_runtime *rtd) +static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) { - struct skl_nau88125_private *ctx = snd_soc_card_get_drvdata(rtd->card); - struct snd_soc_dai *dai = rtd->codec_dai; - struct skl_hdmi_pcm *pcm; - - pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL); - if (!pcm) - return -ENOMEM; - - pcm->device = SKL_DPCM_AUDIO_HDMI2_PB; - pcm->codec_dai = dai; - - list_add_tail(&pcm->head, &ctx->hdmi_pcm_list); - - return 0; + return skylake_hdmi_init(rtd, SKL_DPCM_AUDIO_HDMI1_PB); }
+static int skylake_hdmi2_init(struct snd_soc_pcm_runtime *rtd) +{ + return skylake_hdmi_init(rtd, SKL_DPCM_AUDIO_HDMI2_PB); +}
static int skylake_hdmi3_init(struct snd_soc_pcm_runtime *rtd) { - struct skl_nau88125_private *ctx = snd_soc_card_get_drvdata(rtd->card); - struct snd_soc_dai *dai = rtd->codec_dai; - struct skl_hdmi_pcm *pcm; - - pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL); - if (!pcm) - return -ENOMEM; - - pcm->device = SKL_DPCM_AUDIO_HDMI3_PB; - pcm->codec_dai = dai; - - list_add_tail(&pcm->head, &ctx->hdmi_pcm_list); - - return 0; + return skylake_hdmi_init(rtd, SKL_DPCM_AUDIO_HDMI3_PB); }
static int skylake_nau8825_fe_init(struct snd_soc_pcm_runtime *rtd)
On 6/18/19 9:05 AM, Tzung-Bi Shih wrote:
Extract common logic to a function to avoid duplicate code.
Signed-off-by: Tzung-Bi Shih tzungbi@google.com
Refactor HDMI init to as most Intel machine drivers did, e.g. kbl_da7219_max98357a.c.
HDMI support is a mess for sure, but I am not sure this is the right way to refactor the code. See e.g. bxt_da7219_max98357a, a single init callback was used. you can use dai->id to manage the right offset,
pcm->device = SKL_DPCM_AUDIO_HDMI1_PB + dai->id; or pcm->device = dai->id;
instead of hard-coding the values in a parameter.
.../soc/intel/boards/skl_nau88l25_max98357a.c | 39 +++++------------- sound/soc/intel/boards/skl_nau88l25_ssm4567.c | 40 +++++-------------- 2 files changed, 18 insertions(+), 61 deletions(-)
diff --git a/sound/soc/intel/boards/skl_nau88l25_max98357a.c b/sound/soc/intel/boards/skl_nau88l25_max98357a.c index 3ce8efbeed12..485614c4fa47 100644 --- a/sound/soc/intel/boards/skl_nau88l25_max98357a.c +++ b/sound/soc/intel/boards/skl_nau88l25_max98357a.c @@ -179,7 +179,7 @@ static int skylake_nau8825_codec_init(struct snd_soc_pcm_runtime *rtd) return ret; }
-static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) +static int skylake_hdmi_init(struct snd_soc_pcm_runtime *rtd, int device) { struct skl_nau8825_private *ctx = snd_soc_card_get_drvdata(rtd->card); struct snd_soc_dai *dai = rtd->codec_dai; @@ -189,7 +189,7 @@ static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) if (!pcm) return -ENOMEM;
- pcm->device = SKL_DPCM_AUDIO_HDMI1_PB;
pcm->device = device; pcm->codec_dai = dai;
list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
@@ -197,40 +197,19 @@ static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) return 0; }
-static int skylake_hdmi2_init(struct snd_soc_pcm_runtime *rtd) +static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) {
- struct skl_nau8825_private *ctx = snd_soc_card_get_drvdata(rtd->card);
- struct snd_soc_dai *dai = rtd->codec_dai;
- struct skl_hdmi_pcm *pcm;
- pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
- if (!pcm)
return -ENOMEM;
- pcm->device = SKL_DPCM_AUDIO_HDMI2_PB;
- pcm->codec_dai = dai;
- list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
- return skylake_hdmi_init(rtd, SKL_DPCM_AUDIO_HDMI1_PB);
+}
- return 0;
+static int skylake_hdmi2_init(struct snd_soc_pcm_runtime *rtd) +{
return skylake_hdmi_init(rtd, SKL_DPCM_AUDIO_HDMI2_PB); }
static int skylake_hdmi3_init(struct snd_soc_pcm_runtime *rtd) {
- struct skl_nau8825_private *ctx = snd_soc_card_get_drvdata(rtd->card);
- struct snd_soc_dai *dai = rtd->codec_dai;
- struct skl_hdmi_pcm *pcm;
- pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
- if (!pcm)
return -ENOMEM;
- pcm->device = SKL_DPCM_AUDIO_HDMI3_PB;
- pcm->codec_dai = dai;
- list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
- return 0;
return skylake_hdmi_init(rtd, SKL_DPCM_AUDIO_HDMI3_PB); }
static int skylake_nau8825_fe_init(struct snd_soc_pcm_runtime *rtd)
diff --git a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c index 1a7ac8bdf543..772cbd6940db 100644 --- a/sound/soc/intel/boards/skl_nau88l25_ssm4567.c +++ b/sound/soc/intel/boards/skl_nau88l25_ssm4567.c @@ -198,7 +198,7 @@ static int skylake_nau8825_codec_init(struct snd_soc_pcm_runtime *rtd) return ret; }
-static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) +static int skylake_hdmi_init(struct snd_soc_pcm_runtime *rtd, int device) { struct skl_nau88125_private *ctx = snd_soc_card_get_drvdata(rtd->card); struct snd_soc_dai *dai = rtd->codec_dai; @@ -208,7 +208,7 @@ static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) if (!pcm) return -ENOMEM;
- pcm->device = SKL_DPCM_AUDIO_HDMI1_PB;
pcm->device = device; pcm->codec_dai = dai;
list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
@@ -216,41 +216,19 @@ static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) return 0; }
-static int skylake_hdmi2_init(struct snd_soc_pcm_runtime *rtd) +static int skylake_hdmi1_init(struct snd_soc_pcm_runtime *rtd) {
- struct skl_nau88125_private *ctx = snd_soc_card_get_drvdata(rtd->card);
- struct snd_soc_dai *dai = rtd->codec_dai;
- struct skl_hdmi_pcm *pcm;
- pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
- if (!pcm)
return -ENOMEM;
- pcm->device = SKL_DPCM_AUDIO_HDMI2_PB;
- pcm->codec_dai = dai;
- list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
- return 0;
- return skylake_hdmi_init(rtd, SKL_DPCM_AUDIO_HDMI1_PB); }
+static int skylake_hdmi2_init(struct snd_soc_pcm_runtime *rtd) +{
- return skylake_hdmi_init(rtd, SKL_DPCM_AUDIO_HDMI2_PB);
+}
static int skylake_hdmi3_init(struct snd_soc_pcm_runtime *rtd) {
- struct skl_nau88125_private *ctx = snd_soc_card_get_drvdata(rtd->card);
- struct snd_soc_dai *dai = rtd->codec_dai;
- struct skl_hdmi_pcm *pcm;
- pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
- if (!pcm)
return -ENOMEM;
- pcm->device = SKL_DPCM_AUDIO_HDMI3_PB;
- pcm->codec_dai = dai;
- list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
- return 0;
return skylake_hdmi_init(rtd, SKL_DPCM_AUDIO_HDMI3_PB); }
static int skylake_nau8825_fe_init(struct snd_soc_pcm_runtime *rtd)
On Tue, Jun 18, 2019 at 3:46 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
HDMI support is a mess for sure, but I am not sure this is the right way to refactor the code. See e.g. bxt_da7219_max98357a, a single init callback was used. you can use dai->id to manage the right offset,
pcm->device = SKL_DPCM_AUDIO_HDMI1_PB + dai->id; or pcm->device = dai->id;
instead of hard-coding the values in a parameter.
Yeah, I noticed that and I am very confused.
But after following the call sequence, _late_probe() -> hdac_hdmi_jack_init() -> hdac_hdmi_get_pcm_from_id(), it seems the ID is merely a unique key to perform linear search in hdac_hdmi_get_pcm_from_id(). It looks like the follower snd_hdac_add_chmap_ctls() does not use the ID too seriously.
TBH, I am not very sure either. But if either way is fine, simpler is better (i.e. pcm->device = dai->id;).
participants (2)
-
Pierre-Louis Bossart
-
Tzung-Bi Shih