Skip to content

topology: apl-pcm512x: align HDMI/DP PCM#1750

Closed
kv2019i wants to merge 1 commit intothesofproject:masterfrom
kv2019i:topic/hda-hdmi
Closed

topology: apl-pcm512x: align HDMI/DP PCM#1750
kv2019i wants to merge 1 commit intothesofproject:masterfrom
kv2019i:topic/hda-hdmi

Conversation

@kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Aug 23, 2019

Needs to be merged together with machine driver update: thesofproject/linux#1155

With apl-pcm512x driver moving to use shared codec driver for HDMI/DP,
also topology needs to follow the legacy numbering used by
snd-hda-codec-hdmi.

With apl-pcm512x driver moving to use shared codec driver for HDMI/DP,
also topology needs to follow the legacy numbering used by
snd-hda-codec-hdmi.

The numbering scheme is defined in hda_codec.c:get_empty_pcm_device().

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

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

wow. Do we have to do this for every driver?

@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 26, 2019

@plbossart wrote:

wow. Do we have to do this for every driver?

This is not nice I agree. But basicly the patch_hdmi.c driver hard codes the PCM interface numbers. With legacy driver, the codec driver also creates the PCM instances. With ASoC/SOF, this makes less sense, but if we want to reuse the patch_hdmi.c code, the PCM numbering should align.

So this is at least a version that works. I've spend this day on a PoC to dig up the PCM numbers from topology, and pass this to patch_hdmi.c at runtime. I'll post an update one I have this ready -- it's not working yet.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 27, 2019

@kv2019i wrote:

So this is at least a version that works. I've spend this day on a PoC to dig up the PCM numbers from topology, and pass this to patch_hdmi.c at runtime. I'll post an update one I have this ready -- it's not working yet.

@plbossart Ok, I might have a solution, please see thesofproject/linux@f405d36
... with this, I need to update every machine driver, but topology does not have to be changed.

@plbossart
Copy link
Member

@kv2019i wrote:

So this is at least a version that works. I've spend this day on a PoC to dig up the PCM numbers from topology, and pass this to patch_hdmi.c at runtime. I'll post an update one I have this ready -- it's not working yet.

@plbossart Ok, I might have a solution, please see thesofproject/linux@f405d36
... with this, I need to update every machine driver, but topology does not have to be changed.

I don't have enough background to figure out why the HDMI codec would need to know about PCM devices, as you wrote "PCM numbers used in topology from machine driver to the codec driver"

Is this for e.g. jack detection or to actually create the PCM devices? In the latter case we'd have a clear conflict between what the topology does and what the machine driver does.

Also wondering if we could pass those PCM devices from the topology to the machine drivers, we have a mach_params argument and passing such a list wouldn't be too hard, and would avoid hard-coded values in machine drivers.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 27, 2019

@plbossart wrote:

I don't have enough background to figure out why the HDMI codec would need to know about
PCM devices, as you wrote "PCM numbers used in topology from machine driver to the codec driver"

Is this for e.g. jack detection or to actually create the PCM devices? In the latter case we'd have a clear conflict between what the topology does and what the machine driver does.

Correct. The codec driver needs PCM device numbers to create the card-level jack controls. Second usage is to pass the "snd_pcm_t" object as well, the codec uses this to attach chmap controls to the correct PCM instance. I'm trying to remove the snd_pcm_t link, but the jack controls are harder to remove.

Also wondering if we could pass those PCM devices from the topology to the machine drivers, we have a mach_params argument and passing such a list wouldn't be too hard, and would avoid hard-coded values in machine drivers.

I spent a day trying different options, but I kept hitting a wall with ordering. The HDA codec driver wants the PCM numbers at probe. When the topology is parsed and we know the PCM numbers, the codec is already setup and it's too late to change the numbers.

But it's a good idea -- I didn't think of passing topology to machine drivers. Maybe that could work -- I need to study how to use the mach_params for this. I did already implement a mechanism/extension to postpone control creation in the codec driver, so I could wait for "late_probe()" in the machine driver.

Current patch (v2), avoids cross the board topology changes, but still has the big limitation that for a single machine driver, the PCM device numbers should be the same for all topologies. This does not hold e.g. for our variants of skl-hda-generic-dsp (sof-hda-generic-idisp.m4 versus sof-hda-generic.4 -> HDMI PCM numbers are different).

Thanks a lot for feedback, this is useful!

@plbossart
Copy link
Member

But it's a good idea -- I didn't think of passing topology to machine drivers. Maybe that could work -- I need to study how to use the mach_params for this. I did already implement a mechanism/extension to postpone control creation in the codec driver, so I could wait for "late_probe()" in the machine driver.

I didn't think of passing the topology completely, but rather a mask that tells you which PCM streams are related to topology. e.g. if you pass the hdmi_pcm_mask 0xA8 it tells you that the topology will create PCM 3,5,7
I don't know how easy it'd be to extract this information though, @ranj063 might want to chime in here...

@kv2019i
Copy link
Collaborator Author

kv2019i commented Aug 28, 2019

@plbossart wrote:

I didn't think of passing the topology completely, but rather a mask that tells you which PCM streams are related to topology. e.g. if you pass the hdmi_pcm_mask 0xA8 it tells you that the topology will create PCM 3,5,7
I don't know how easy it'd be to extract this information though, @ranj063 might want to chime in here...

I actually found another way. This looks pretty good. Basicly I parse the card PCMs as the last step of machine driver late_probe(), find all PCM devices with "HDMI" in the name, and record the IDs. And with all data gathered, I create the codec controls.

This still requires machine driver to be updated, but it does

  • not require changes to SOF topologies
  • it does require to hardcode specific PCM ids to the machine driver

Main beef is in thesofproject/linux@4ed40be#diff-cbb59462c9d50ac25d30da88550f0a37L37
... this helper can be used in all machine drivers.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Sep 23, 2019

No longer needed, closing.

@kv2019i kv2019i closed this Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants