Skip to content
This repository was archived by the owner on Jan 17, 2019. It is now read-only.

Conversation

@ghost
Copy link

@ghost ghost commented Aug 3, 2018

add more tokens for ssp register config.

the kernel and firmware should be changed to support
this feature.

Signed-off-by: Wu Zhigang [email protected]

@ghost ghost closed this Aug 3, 2018
@ghost ghost reopened this Aug 3, 2018
` SOF_TKN_INTEL_SSP_MCLK_ID' ifelse($4, `', "0", STR($4))
` }'
` tuples."short" {'
` SOF_TKN_INTEL_SSP_FRAME_PULSE_WIDTH' ifelse($5, `', "0", STR($5))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we combine the two short type tokens within the same tuples."short" {} array?

Copy link
Author

Choose a reason for hiding this comment

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

If we combine them together, how the token parser to process them in kernel side?
maybe we need add more code in the sof_link_ssp_load() function to do this.
I think this would cause the code too complicated to read.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we could put the frame width in the fs_sync configuration? It'd be more logical.

Copy link
Author

Choose a reason for hiding this comment

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

I will think about it. the PR in the kernel will be updated accordingly.

Copy link
Author

Choose a reason for hiding this comment

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

@plbossart
I did not find the fs_sync configuration, you mean the SSP_CLOCK(fsync, ....)?
what about put the parameter "frame_pulse_width" in SSP_TDM() in ssp.m4 file?
bit clock and fs clock both use the SSP_CLOCK() macro.

Copy link
Author

Choose a reason for hiding this comment

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

@plbossart
I checked this, the alsa_lib did not support this.
we have to use this in the token at the moment.
we could update the alsa_lib in future to support this parameter.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, putting the frame_pulse_width in the SSP_TDM macro would make sense, but I am not sure it's feasible since it's a new token.
You may want to keep it in SSP_CONFIG_DATA but you want to reorder the parameters so that we get mclk_id, quirks, frame_pulse_width. The latter makes no sense for I2S and LeftJ so we want to have it last, while quirks could be common to all modes

Copy link
Author

Choose a reason for hiding this comment

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

@plbossart
OK, I will update the order of the parameters.

add more tokens for ssp register config.

the kernel and firmware should be changed to support
this feature.

Signed-off-by: Wu Zhigang <[email protected]>
@ghost
Copy link
Author

ghost commented Aug 8, 2018

Update the order of the parameter in SSP_CONFIG_DATA() already.

dnl SSP_CONFIG_DATA(type, idx, valid bits, mclk_id)
dnl mclk_id is optional
dnl SSP_CONFIG_DATA(type, idx, valid bits, mclk_id, quirks, frame_pulse_width)
dnl mclk_id quirks frame_pulse_width are optional
Copy link
Member

Choose a reason for hiding this comment

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

@xiulipan is working on something similar which will restart after 1.2. Please align.

Copy link
Author

Choose a reason for hiding this comment

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

this PR should be align with the kernel's PR "ssp:support additional SSP register bits #64".
the kernel's PR is merged.
I hope the tplg's update will not take much time.
the incompatible would cause problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

@lgirdwood
I would like to use some more vendor token for this. Because the alse-tplg have no such more config we have. we need to keep these config as bespoke vendor tuple.

@ranj063
Copy link
Collaborator

ranj063 commented Oct 30, 2018

@lgirdwood @plbossart this has been taken care of in #114. Should we close this now?

@lgirdwood lgirdwood closed this Oct 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants