lkml.org 
[lkml]   [2023]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 09/11] ASoC: SOF: amd: Compute file paths on firmware load
    From
    On 12/10/23 05:50, Venkata Prasad Potturu wrote:
    >
    > On 12/10/23 02:23, Cristian Ciocaltea wrote:
    >> Commit 6c393ebbd74a ("ASoC: SOF: core: Implement IPC version fallback if
    >> firmware files are missing") changed the order of some operations and
    >> the firmware paths are not available anymore at snd_sof_probe() time.
    >>
    >> Precisely, fw_filename_prefix is set by sof_select_ipc_and_paths() via
    >>
    >>    plat_data->fw_filename_prefix = out_profile.fw_path;
    >>
    >> but sof_init_environment() which calls this function was moved from
    >> snd_sof_device_probe() to sof_probe_continue(). Moreover,
    >> snd_sof_probe() was moved from sof_probe_continue() to
    >> sof_init_environment(), but before the call to
    >> sof_select_ipc_and_paths().
    >>
    >> The problem here is that amd_sof_acp_probe() uses fw_filename_prefix to
    >> compute fw_code_bin and fw_data_bin paths, and because the field is not
    >> yet initialized, the paths end up containing (null):
    >>
    >> snd_sof_amd_vangogh 0000:04:00.5: Direct firmware load for
    >> (null)/sof-vangogh-code.bin failed with error -2
    >> snd_sof_amd_vangogh 0000:04:00.5: sof signed firmware code bin is missing
    >> snd_sof_amd_vangogh 0000:04:00.5: error: failed to load DSP firmware -2
    >> snd_sof_amd_vangogh: probe of 0000:04:00.5 failed with error -2
    >>
    >> Move usage of fw_filename_prefix right before request_firmware() calls
    >> in acp_sof_load_signed_firmware().
    >>
    >> Fixes: 6c393ebbd74a ("ASoC: SOF: core: Implement IPC version fallback
    >> if firmware files are missing")
    >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
    >> ---
    >>   sound/soc/sof/amd/acp-loader.c | 32 ++++++++++++++++++++++++++------
    >>   sound/soc/sof/amd/acp.c        |  7 ++-----
    >>   2 files changed, 28 insertions(+), 11 deletions(-)
    >>
    >> diff --git a/sound/soc/sof/amd/acp-loader.c
    >> b/sound/soc/sof/amd/acp-loader.c
    >> index e05eb7a86dd4..d2d21478399e 100644
    >> --- a/sound/soc/sof/amd/acp-loader.c
    >> +++ b/sound/soc/sof/amd/acp-loader.c
    >> @@ -267,29 +267,49 @@ int acp_sof_load_signed_firmware(struct
    >> snd_sof_dev *sdev)
    >>   {
    >>       struct snd_sof_pdata *plat_data = sdev->pdata;
    >>       struct acp_dev_data *adata = plat_data->hw_pdata;
    >> +    const char *fw_filename;
    >>       int ret;
    >>   -    ret = request_firmware(&sdev->basefw.fw, adata->fw_code_bin,
    >> sdev->dev);
    >> +    fw_filename = kasprintf(GFP_KERNEL, "%s/%s",
    >> +                plat_data->fw_filename_prefix,
    >> +                adata->fw_code_bin);
    > File path already saved in adata->fw_code_bin in amd_sof_acp_probe
    > function.
    > No need to get it again.

    As already stated in the patch description, fw_filename_prefix is not
    available anymore when amd_sof_acp_probe() gets invoked, and that is the
    root cause of ending up with (null) in the computed file path.

    Hence, the patch ensures amd_sof_acp_probe() computes the file name,
    without prefix, while acp_sof_load_signed_firmware() adds the prefix.

    >> +    if (!fw_filename)
    >> +        return -ENOMEM;
    >> +
    >> +    ret = request_firmware(&sdev->basefw.fw, fw_filename, sdev->dev);
    >>       if (ret < 0) {
    >> +        kfree(fw_filename);
    >>           dev_err(sdev->dev, "sof signed firmware code bin is
    >> missing\n");
    >>           return ret;
    >>       } else {
    >> -        dev_dbg(sdev->dev, "request_firmware %s successful\n",
    >> adata->fw_code_bin);
    >> +        dev_dbg(sdev->dev, "request_firmware %s successful\n",
    >> fw_filename);
    >>       }
    >> +    kfree(fw_filename);
    >> +
    >>       ret = snd_sof_dsp_block_write(sdev, SOF_FW_BLK_TYPE_IRAM, 0,
    >> -                      (void *)sdev->basefw.fw->data,
    >> sdev->basefw.fw->size);
    >> +                      (void *)sdev->basefw.fw->data,
    >> +                      sdev->basefw.fw->size);
    >> +
    >> +    fw_filename = kasprintf(GFP_KERNEL, "%s/%s",
    >> +                plat_data->fw_filename_prefix,
    >> +                adata->fw_data_bin);
    >> +    if (!fw_filename)
    >> +        return -ENOMEM;
    >>   -    ret = request_firmware(&adata->fw_dbin, adata->fw_data_bin,
    >> sdev->dev);
    >> +    ret = request_firmware(&adata->fw_dbin, fw_filename, sdev->dev);
    >>       if (ret < 0) {
    >> +        kfree(fw_filename);
    >>           dev_err(sdev->dev, "sof signed firmware data bin is
    >> missing\n");
    >>           return ret;
    >>         } else {
    >> -        dev_dbg(sdev->dev, "request_firmware %s successful\n",
    >> adata->fw_data_bin);
    >> +        dev_dbg(sdev->dev, "request_firmware %s successful\n",
    >> fw_filename);
    >>       }
    >> +    kfree(fw_filename);
    >>         ret = snd_sof_dsp_block_write(sdev, SOF_FW_BLK_TYPE_DRAM, 0,
    >> -                      (void *)adata->fw_dbin->data,
    >> adata->fw_dbin->size);
    >> +                      (void *)adata->fw_dbin->data,
    >> +                      adata->fw_dbin->size);
    >>       return ret;
    >>   }
    >>   EXPORT_SYMBOL_NS(acp_sof_load_signed_firmware, SND_SOC_SOF_AMD_COMMON);
    >> diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c
    >> index 1e9840ae8938..87c5c71eac68 100644
    >> --- a/sound/soc/sof/amd/acp.c
    >> +++ b/sound/soc/sof/amd/acp.c
    >> @@ -479,7 +479,6 @@ EXPORT_SYMBOL_NS(amd_sof_acp_resume,
    >> SND_SOC_SOF_AMD_COMMON);
    >>   int amd_sof_acp_probe(struct snd_sof_dev *sdev)
    >>   {
    >>       struct pci_dev *pci = to_pci_dev(sdev->dev);
    >> -    struct snd_sof_pdata *plat_data = sdev->pdata;
    >>       struct acp_dev_data *adata;
    >>       const struct sof_amd_acp_desc *chip;
    >>       const struct dmi_system_id *dmi_id;
    >> @@ -547,8 +546,7 @@ int amd_sof_acp_probe(struct snd_sof_dev *sdev)
    >>       dmi_id = dmi_first_match(acp_sof_quirk_table);
    >>       if (dmi_id && dmi_id->driver_data) {
    >>           adata->fw_code_bin = devm_kasprintf(sdev->dev, GFP_KERNEL,
    >> -                            "%s/sof-%s-code.bin",
    >> -                            plat_data->fw_filename_prefix,
    >> +                            "sof-%s-code.bin",
    >>                               chip->name);
    >>           if (!adata->fw_code_bin) {
    >>               ret = -ENOMEM;
    >> @@ -556,8 +554,7 @@ int amd_sof_acp_probe(struct snd_sof_dev *sdev)
    >>           }
    >>             adata->fw_data_bin = devm_kasprintf(sdev->dev, GFP_KERNEL,
    >> -                            "%s/sof-%s-data.bin",
    >> -                            plat_data->fw_filename_prefix,
    >> +                            "sof-%s-data.bin",
    >>                               chip->name);
    >>           if (!adata->fw_data_bin) {
    >>               ret = -ENOMEM;

    \
     
     \ /
      Last update: 2023-12-10 09:57    [W:5.566 / U:0.344 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site