Re: [PATCH] ALSA: hda: Fix tegra hda dp infoframe struct

[Mohan Kumar D]

On 9/13/2022 11:10 AM, Takashi Iwai wrote:
> External email: Use caution opening links or attachments
>
>
> On Tue, 13 Sep 2022 05:44:10 +0200,
> Mohan Kumar wrote:
> > Tegra HDA HW expects infoframe data bytes order same for both
> > HDMI and DP i.e infoframe data starts from 5th bytes offset.
> > This hw behavior mandates to have dummy bytes for dp infoframe
> > structure for Tegra.
> >
> > Signed-off-by: Mohan Kumar <mkumard@xxxxxxxxxx>
> > ---
> >   sound/pci/hda/patch_hdmi.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> > index 6c209cd26c0c..a52e764db2e0 100644
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -218,6 +218,9 @@ struct dp_audio_infoframe {
> >        u8 type; /* 0x84 */
> >        u8 len;  /* 0x1b */
> >        u8 ver;  /* 0x11 << 2 */
> > +#if IS_ENABLED(CONFIG_SND_HDA_TEGRA)
> > +     u8 checksum; /* Tegra HW expects infoframe bytes from 5th offset */
> > +#endif
> I'm afraid that we can't use ifdef here, as this is another module
> that is used not only by snd-hda-tegra.  In theory, snd-hda-intel can
> run on Arm using the same codec driver.
>
> That is, the check has to be dynamically.  Maybe we need to set a flag
> at tegra_hdmi_init().
>
> If I understand correctly, Tegra uses the same byte layout for both DP
> and HDMI?  If so, the patch like below should work instead.
>
> Of course, if that's really specific to Tegra, not generically for
> Nvidia graphics, the flag has to be set in a different way...
>
>
> thanks,
>
> Takashi
>
> -- 8< --
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -171,6 +171,7 @@ struct hdmi_spec {
>          /* hdmi interrupt trigger control flag for Nvidia codec */
>          bool hdmi_intr_trig_ctrl;
>          bool intel_hsw_fixup;   /* apply Intel platform-specific fixups */
> +       bool tegra_dp_workaround; /* workaround DP audio infoframe for Tegra */
>          /*
>           * Non-generic VIA/NVIDIA specific
>           */
> @@ -679,15 +680,24 @@ static void hdmi_pin_setup_infoframe(struct hda_codec *codec,
>                                       int ca, int active_channels,
>                                       int conn_type)
>   {
> +       struct hdmi_spec *spec = codec->spec;
>          union audio_infoframe ai;
>
>          memset(&ai, 0, sizeof(ai));
> -       if (conn_type == 0) { /* HDMI */
> +       if (conn_type == 0 || /* HDMI */
> +           /* DisplayPort for Tegra: Tegra HW expects same layout as HDMI */
> +           (conn_type == 1 && spec->tegra_dp_workaround)) {
>                  struct hdmi_audio_infoframe *hdmi_ai = &ai.hdmi;
>
> -               hdmi_ai->type           = 0x84;
> -               hdmi_ai->ver            = 0x01;
> -               hdmi_ai->len            = 0x0a;
> +               if (conn_type == 0) { /* HDMI */
> +                       hdmi_ai->type           = 0x84;
> +                       hdmi_ai->ver            = 0x01;
> +                       hdmi_ai->len            = 0x0a;
> +               } else { /* Tegra DP */
> +                       hdmi_ai->type           = 0x84;
> +                       hdmi_ai->len            = 0x1b;
> +                       hdmi_ai->ver            = 0x11 << 2;
> +               }
>                  hdmi_ai->CC02_CT47      = active_channels - 1;
>                  hdmi_ai->CA             = ca;
>                  hdmi_checksum_audio_infoframe(hdmi_ai);
> @@ -3992,6 +4002,7 @@ static int tegra_hdmi_init(struct hda_codec *codec)
>          spec->chmap.ops.chmap_cea_alloc_validate_get_type =
>                  nvhdmi_chmap_cea_alloc_validate_get_type;
>          spec->chmap.ops.chmap_validate = nvhdmi_chmap_validate;
> +       spec->tegra_dp_workaround = true;
>
>          return 0;
>   }
Agreed, will do as per suggestion and upload patch v2.