| Message ID | 20210928082248.6081-1-peter.ujfalusi@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: SOF: Intel: hda-stream: limit PROCEN workaround | expand |
On Tue, Sep 28, 2021 at 11:22:48AM +0300, Peter Ujfalusi wrote: > From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > > The work-around enabled in hda-stream.c is only required on earlier > versions of SOCs/PCH (Skylake, KabyLake, ApolloLake, > GeminiLake). Before setting the format on the host DMA, it is required > to couple the host and link DMA - which as a consequence shall use the > same format. This breaks the build for me: /mnt/kernel/sound/soc/sof/intel/hda-stream.c: In function 'hda_dsp_stream_hw_params': /mnt/kernel/sound/soc/sof/intel/hda-stream.c:436:42: error: implicit declaration of function 'get_chip_info'; did you mean 'get_group_info'? [-Werror=implicit-function-declaration]
On 9/28/21 10:10 AM, Mark Brown wrote: > On Tue, Sep 28, 2021 at 11:22:48AM +0300, Peter Ujfalusi wrote: >> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> >> >> The work-around enabled in hda-stream.c is only required on earlier >> versions of SOCs/PCH (Skylake, KabyLake, ApolloLake, >> GeminiLake). Before setting the format on the host DMA, it is required >> to couple the host and link DMA - which as a consequence shall use the >> same format. > > This breaks the build for me: > > /mnt/kernel/sound/soc/sof/intel/hda-stream.c: In function 'hda_dsp_stream_hw_params': > /mnt/kernel/sound/soc/sof/intel/hda-stream.c:436:42: error: implicit declaration of function 'get_chip_info'; did you mean 'get_group_info'? [-Werror=implicit-function-declaration] Ack, we're missing a dependency. Peter, this function was moved to shim.h with Ranjani's patch "ASoC: SOF: Intel: hda: expose get_chip_info()" That patch was added in the SOF multi-core series.
Hi Mark, Pierre, On 30/09/2021 23:24, Pierre-Louis Bossart wrote: > > > On 9/28/21 10:10 AM, Mark Brown wrote: >> On Tue, Sep 28, 2021 at 11:22:48AM +0300, Peter Ujfalusi wrote: >>> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> >>> >>> The work-around enabled in hda-stream.c is only required on earlier >>> versions of SOCs/PCH (Skylake, KabyLake, ApolloLake, >>> GeminiLake). Before setting the format on the host DMA, it is required >>> to couple the host and link DMA - which as a consequence shall use the >>> same format. >> >> This breaks the build for me: >> >> /mnt/kernel/sound/soc/sof/intel/hda-stream.c: In function 'hda_dsp_stream_hw_params': >> /mnt/kernel/sound/soc/sof/intel/hda-stream.c:436:42: error: implicit declaration of function 'get_chip_info'; did you mean 'get_group_info'? [-Werror=implicit-function-declaration] Sorry about that. I'm going to script this from now. It was sort of therapeutic meditation type of flow: one slip of coffee find a patch / feature series git checkout -b to_upstream/<topic> <current_upstream_base> git cherry-pick <hash from sof-dev-rebase> if (fail) goto drop_patch; scripts/checkpatch.pl --strict -g HEAD if (fail) correct it(); compile (x64/aarch64, sparse for both) - this I have scripted if (fail) goto drop_patch; boot test if (fail) goto drop_patch; format-patch && send fail: drop patch or series and try to find another one. I must have skipped the compile phase. > Ack, we're missing a dependency. > > Peter, this function was moved to shim.h with Ranjani's patch > "ASoC: SOF: Intel: hda: expose get_chip_info()" > > That patch was added in the SOF multi-core series. Yes, it is and it does not apply without the dynamic pipelines and possibly have dependency on other patches from the muticore support for the dynamic-pipelines. Sorry for the trouble, I'll start with a coffee then the script.
On Fri, Oct 01, 2021 at 08:58:28AM +0300, Péter Ujfalusi wrote: > Sorry about that. > I'm going to script this from now. No worries. BTW while you're at scripting this I'd suggest looking at signing your patches with patatt/b4 attest, it's pretty trivial to integrate into the flow.
diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c index e6a1f6532547..16b7d8b1adff 100644 --- a/sound/soc/sof/intel/apl.c +++ b/sound/soc/sof/intel/apl.c @@ -143,5 +143,6 @@ const struct sof_intel_dsp_desc apl_chip_info = { .rom_init_timeout = 150, .ssp_count = APL_SSP_COUNT, .ssp_base_offset = APL_SSP_BASE_OFFSET, + .quirks = SOF_INTEL_PROCEN_FMT_QUIRK, }; EXPORT_SYMBOL_NS(apl_chip_info, SND_SOC_SOF_INTEL_HDA_COMMON); diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index 63c367478f1c..47a465b06da5 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -405,6 +405,7 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, struct snd_dma_buffer *dmab, struct snd_pcm_hw_params *params) { + const struct sof_intel_dsp_desc *chip = get_chip_info(sdev->pdata); struct hdac_bus *bus = sof_to_bus(sdev); struct hdac_stream *hstream = &stream->hstream; int sd_offset = SOF_STREAM_SD_OFFSET(hstream); @@ -538,6 +539,7 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, /* * Recommended hardware programming sequence for HDAudio DMA format + * on earlier platforms - this is not needed on newer platforms * * 1. Put DMA into coupled mode by clearing PPCTL.PROCEN bit * for corresponding stream index before the time of writing @@ -547,9 +549,11 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, * enable decoupled mode */ - /* couple host and link DMA, disable DSP features */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_PP_BAR, SOF_HDA_REG_PP_PPCTL, - mask, 0); + if (chip->quirks & SOF_INTEL_PROCEN_FMT_QUIRK) { + /* couple host and link DMA, disable DSP features */ + snd_sof_dsp_update_bits(sdev, HDA_DSP_PP_BAR, SOF_HDA_REG_PP_PPCTL, + mask, 0); + } /* program stream format */ snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, @@ -557,9 +561,11 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev, SOF_HDA_ADSP_REG_CL_SD_FORMAT, 0xffff, hstream->format_val); - /* decouple host and link DMA, enable DSP features */ - snd_sof_dsp_update_bits(sdev, HDA_DSP_PP_BAR, SOF_HDA_REG_PP_PPCTL, - mask, mask); + if (chip->quirks & SOF_INTEL_PROCEN_FMT_QUIRK) { + /* decouple host and link DMA, enable DSP features */ + snd_sof_dsp_update_bits(sdev, HDA_DSP_PP_BAR, SOF_HDA_REG_PP_PPCTL, + mask, mask); + } /* program last valid index */ snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR, diff --git a/sound/soc/sof/intel/shim.h b/sound/soc/sof/intel/shim.h index e9f7d4d7fcce..8e2a0f4c0f66 100644 --- a/sound/soc/sof/intel/shim.h +++ b/sound/soc/sof/intel/shim.h @@ -151,6 +151,9 @@ #define PCI_PMCS 0x84 #define PCI_PMCS_PS_MASK 0x3 +/* Intel quirks */ +#define SOF_INTEL_PROCEN_FMT_QUIRK BIT(0) + /* DSP hardware descriptor */ struct sof_intel_dsp_desc { int cores_num; @@ -166,6 +169,7 @@ struct sof_intel_dsp_desc { int ssp_base_offset; /* base address of the SSPs */ u32 sdw_shim_base; u32 sdw_alh_base; + u32 quirks; bool (*check_sdw_irq)(struct snd_sof_dev *sdev); };