ASoC: Intel: sof_rt5682 add support for HDMI_In capture #4470
Conversation
apoorvintel
requested review from
lgirdwood,
plbossart,
ranj063,
kv2019i,
dbaluta,
bardliao,
ujfalusi,
ajitkupandey,
kuanhsuncheng and
yaochunhung
as code owners
|
Can one of the admins verify this patch?
|
sound/soc/intel/boards/sof_rt5682.c
Outdated
| (((quirk) << SOF_NO_OF_HDMI_CAPTURE_SSP_SHIFT) & SOF_NO_OF_HDMI_CAPTURE_SSP_MASK) | ||
|
|
||
| #define SOF_HDMI_CAPTURE_1_SSP_SHIFT 30 | ||
| #define SOF_HDMI_CAPTURE_1_SSP_MASK (GENMASK(32, 30)) |
There was a problem hiding this comment.
we need to define sof_rt5682_quirk as u64 now... see line 83
There was a problem hiding this comment.
our observation from test is unsigned long sof_rt5682_quirk is supported 64 bit when its compiled with X64 flag enabled.
the issue is during macro assignment and shift , SOF_HDMI_CAPTURE_1_SSP(quirk)=> quirk is treated as 32 bit and throws complier warning. Hence we need to do temporary type casting to make it as 64 bit variable.
There was a problem hiding this comment.
this warning has been fixed with typecasting and code is working as expected.
There was a problem hiding this comment.
no need for typecast, use u64 for the quirk.
There was a problem hiding this comment.
We have tried with unsigned long long sof_rt5682_quirk but it doesn't help to fix the compiler warning. I understand that warning is not related to sof_rt5682_quirk data type.
looks Its related to macro variable SOF_HDMI_CAPTURE_1_SSP(quirk) => quirk is treated as 32 bit width default during shift. also here we are not passing sof_rt5682_quirk as argument.
just passing a integer value ex. SOF_HDMI_CAPTURE_1_SSP(0) . So its irrelevant of sof_rt5682_quirk data type. please confirm.
There was a problem hiding this comment.
Hi @bardliao,
As per your suggestion changed the code and uploaded. please check.
There was a problem hiding this comment.
@CBala21 Thanks, it looks good. Can you split the commit into 3?
- Add HDMI_IN SSP support
- Add adl_rt5682_c1_h02 id
- Add adl_rt5682_rt5682s_hp in snd_soc_acpi_intel_adl_machines[]
There was a problem hiding this comment.
Hi [bardliao],
All these three changes are connected without this HDMI -in capture use case wont work. Still you want to split it out?
There was a problem hiding this comment.
Hi [bardliao],
All these three changes are connected without this HDMI -in capture use case wont work. Still you want to split it out?
Yes, supporting HDMI -in capture is a new feature and adding board support is another topic though they can not work without each other. What I am not sure is whether adding adl_rt5682_c1_h02 id and adl_rt5682_rt5682s_hp should be in the same commit or not. @plbossart What do you think?
There was a problem hiding this comment.
it's fine to add in the same commit. I'll approve, let's move on with this.
sound/soc/intel/boards/sof_rt5682.c
Outdated
| (((quirk) << SOF_NO_OF_HDMI_CAPTURE_SSP_SHIFT) & SOF_NO_OF_HDMI_CAPTURE_SSP_MASK) | ||
|
|
||
| #define SOF_HDMI_CAPTURE_1_SSP_SHIFT 30 | ||
| #define SOF_HDMI_CAPTURE_1_SSP_MASK (GENMASK(32, 30)) |
There was a problem hiding this comment.
no need for typecast, use u64 for the quirk.
sound/soc/intel/boards/sof_rt5682.c
Outdated
| SOF_HDMI_CAPTURE_1_SSP(0) | | ||
| SOF_HDMI_CAPTURE_2_SSP(2) | | ||
| SOF_SSP_HDMI_CAPTURE_PRESENT), | ||
| }, |
There was a problem hiding this comment.
why would you put this last instead of grouping it with the rest of ADL stuff? This is just too messy.
There was a problem hiding this comment.
Agree. this messed up happen due to changes brought from 5.15 development kernel into mainline. Actually there is no RPL, MTL support on 5.15 kernel and no entry for other boards and its last entry.
We will correct it and update.
Adding support for 2 streams of HDMI-In capture via I2S with rt5682s codec variant Signed-off-by: apoorv <[email protected]>
sound/soc/intel/boards/sof_rt5682.c
Outdated
| (((quirk) << SOF_NO_OF_HDMI_CAPTURE_SSP_SHIFT) & SOF_NO_OF_HDMI_CAPTURE_SSP_MASK) | ||
|
|
||
| #define SOF_HDMI_CAPTURE_1_SSP_SHIFT 30 | ||
| #define SOF_HDMI_CAPTURE_1_SSP_MASK (GENMASK(32, 30)) |
There was a problem hiding this comment.
it's fine to add in the same commit. I'll approve, let's move on with this.
|
test this please |
Adding support for 2 streams of HDMI-In capture via I2S with rt5682s codec variant
HI @bardliao , @CBala21 Please review