[alsa-devel] [PATCH 1/3] ALSA: hda: Move common haswell init to a helper
Geminilake vendor nid is different from other Skylake variants, but rest of the initialization code is same.
So a variable is added in hdmi_spec to store the platform specific vendor nid and move the initialization code to a helper function to be used by both platform specific init.
Fixes: 126cfa2f5e15 ("ALSA: hda: Add Geminilake HDMI codec ID") Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Jaikrishna Nemallapudi jaikrishnax.nemallapudi@intel.com Cc: Senthilnathan Veppur senthilnathanx.veppur@intel.com Cc: Vinod Koul vinod.koul@intel.com Cc: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 48 ++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 37f11560186a..7413fff06d70 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -177,6 +177,7 @@ struct hdmi_spec { bool i915_bound; /* was i915 bound in this driver? */
struct hdac_chmap chmap; + hda_nid_t vendor_nid; };
#ifdef CONFIG_SND_HDA_I915 @@ -2381,14 +2382,15 @@ static void intel_haswell_enable_all_pins(struct hda_codec *codec, bool update_tree) { unsigned int vendor_param; + struct hdmi_spec *spec = codec->spec;
- vendor_param = snd_hda_codec_read(codec, INTEL_VENDOR_NID, 0, + vendor_param = snd_hda_codec_read(codec, spec->vendor_nid, 0, INTEL_GET_VENDOR_VERB, 0); if (vendor_param == -1 || vendor_param & INTEL_EN_ALL_PIN_CVTS) return;
vendor_param |= INTEL_EN_ALL_PIN_CVTS; - vendor_param = snd_hda_codec_read(codec, INTEL_VENDOR_NID, 0, + vendor_param = snd_hda_codec_read(codec, spec->vendor_nid, 0, INTEL_SET_VENDOR_VERB, vendor_param); if (vendor_param == -1) return; @@ -2400,8 +2402,9 @@ static void intel_haswell_enable_all_pins(struct hda_codec *codec, static void intel_haswell_fixup_enable_dp12(struct hda_codec *codec) { unsigned int vendor_param; + struct hdmi_spec *spec = codec->spec;
- vendor_param = snd_hda_codec_read(codec, INTEL_VENDOR_NID, 0, + vendor_param = snd_hda_codec_read(codec, spec->vendor_nid, 0, INTEL_GET_VENDOR_VERB, 0); if (vendor_param == -1 || vendor_param & INTEL_EN_DP12) return; @@ -2409,7 +2412,7 @@ static void intel_haswell_fixup_enable_dp12(struct hda_codec *codec) /* enable DP1.2 mode */ vendor_param |= INTEL_EN_DP12; snd_hdac_regmap_add_vendor_verb(&codec->core, INTEL_SET_VENDOR_VERB); - snd_hda_codec_write_cache(codec, INTEL_VENDOR_NID, 0, + snd_hda_codec_write_cache(codec, spec->vendor_nid, 0, INTEL_SET_VENDOR_VERB, vendor_param); }
@@ -2502,22 +2505,11 @@ static void i915_pin_cvt_fixup(struct hda_codec *codec, } }
-/* Intel Haswell and onwards; audio component with eld notifier */ -static int patch_i915_hsw_hdmi(struct hda_codec *codec) +static int intel_haswell_common_init(struct hda_codec *codec) { - struct hdmi_spec *spec; + struct hdmi_spec *spec = codec->spec; int err;
- /* HSW+ requires i915 binding */ - if (!codec->bus->core.audio_component) { - codec_info(codec, "No i915 binding for Intel HDMI/DP codec\n"); - return -ENODEV; - } - - err = alloc_generic_hdmi(codec); - if (err < 0) - return err; - spec = codec->spec; codec->dp_mst = true; spec->dyn_pcm_assign = true;
@@ -2548,6 +2540,28 @@ static int patch_i915_hsw_hdmi(struct hda_codec *codec) return 0; }
+/* Intel Haswell and onwards; audio component with eld notifier */ +static int patch_i915_hsw_hdmi(struct hda_codec *codec) +{ + struct hdmi_spec *spec; + int err; + + /* HSW+ requires i915 binding */ + if (!codec->bus->core.audio_component) { + codec_info(codec, "No i915 binding for Intel HDMI/DP codec\n"); + return -ENODEV; + } + + err = alloc_generic_hdmi(codec); + if (err < 0) + return err; + + spec = codec->spec; + spec->vendor_nid = INTEL_VENDOR_NID; + + return intel_haswell_common_init(codec); +} + /* Intel Baytrail and Braswell; with eld notifier */ static int patch_i915_byt_hdmi(struct hda_codec *codec) {
From: Ander Conselvan De Oliveira ander.conselvan.de.oliveira@intel.com
Separate out the Geminilake platform init and add vendor nid for it.
Fixes: 126cfa2f5e15 ("ALSA: hda: Add Geminilake HDMI codec ID") Signed-off-by: Ander Conselvan De Oliveira ander.conselvan.de.oliveira@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Cc: Senthilnathan Veppur senthilnathanx.veppur@intel.com Cc: Vinod Koul vinod.koul@intel.com Cc: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 7413fff06d70..843f5fe1dfb5 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2373,6 +2373,7 @@ static void intel_haswell_fixup_connect_list(struct hda_codec *codec, }
#define INTEL_VENDOR_NID 0x08 +#define INTEL_GLK_VENDOR_NID 0x0B #define INTEL_GET_VENDOR_VERB 0xf81 #define INTEL_SET_VENDOR_VERB 0x781 #define INTEL_EN_DP12 0x02 /* enable DP 1.2 features */ @@ -2562,6 +2563,27 @@ static int patch_i915_hsw_hdmi(struct hda_codec *codec) return intel_haswell_common_init(codec); }
+static int patch_i915_glk_hdmi(struct hda_codec *codec) +{ + struct hdmi_spec *spec; + int err; + + /* HSW+ requires i915 binding */ + if (!codec->bus->core.audio_component) { + codec_info(codec, "No i915 binding for Intel HDMI/DP codec\n"); + return -ENODEV; + } + + err = alloc_generic_hdmi(codec); + if (err < 0) + return err; + + spec = codec->spec; + spec->vendor_nid = INTEL_GLK_VENDOR_NID; + + return intel_haswell_common_init(codec); +} + /* Intel Baytrail and Braswell; with eld notifier */ static int patch_i915_byt_hdmi(struct hda_codec *codec) { @@ -3814,7 +3836,7 @@ static int patch_via_hdmi(struct hda_codec *codec) HDA_CODEC_ENTRY(0x80862809, "Skylake HDMI", patch_i915_hsw_hdmi), HDA_CODEC_ENTRY(0x8086280a, "Broxton HDMI", patch_i915_hsw_hdmi), HDA_CODEC_ENTRY(0x8086280b, "Kabylake HDMI", patch_i915_hsw_hdmi), -HDA_CODEC_ENTRY(0x8086280d, "Geminilake HDMI", patch_i915_hsw_hdmi), +HDA_CODEC_ENTRY(0x8086280d, "Geminilake HDMI", patch_i915_glk_hdmi), HDA_CODEC_ENTRY(0x80862880, "CedarTrail HDMI", patch_generic_hdmi), HDA_CODEC_ENTRY(0x80862882, "Valleyview2 HDMI", patch_i915_byt_hdmi), HDA_CODEC_ENTRY(0x80862883, "Braswell HDMI", patch_i915_byt_hdmi),
Geminilake is Skylake family platform. So add it's id to skl_plus check.
Fixes: 126cfa2f5e15 ("ALSA: hda: Add Geminilake HDMI codec ID") Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Cc: Senthilnathan Veppur senthilnathanx.veppur@intel.com Cc: Vinod Koul vinod.koul@intel.com Cc: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_intel.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 64db6698214c..5b9ffd70ae52 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -370,8 +370,10 @@ enum { #define IS_KBL_LP(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x9d71) #define IS_KBL_H(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0xa2f0) #define IS_BXT(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x5a98) +#define IS_GLK(pci) ((pci)->vendor == 0x8086 && (pci)->device == 0x3198) #define IS_SKL_PLUS(pci) (IS_SKL(pci) || IS_SKL_LP(pci) || IS_BXT(pci)) || \ - IS_KBL(pci) || IS_KBL_LP(pci) || IS_KBL_H(pci) + IS_KBL(pci) || IS_KBL_LP(pci) || IS_KBL_H(pci) || \ + IS_GLK(pci)
static char *driver_short_names[] = { [AZX_DRIVER_ICH] = "HDA Intel",
On Wed, 12 Apr 2017 06:23:58 +0200, Subhransu S. Prusty wrote:
Geminilake vendor nid is different from other Skylake variants, but rest of the initialization code is same.
So a variable is added in hdmi_spec to store the platform specific vendor nid and move the initialization code to a helper function to be used by both platform specific init.
Fixes: 126cfa2f5e15 ("ALSA: hda: Add Geminilake HDMI codec ID") Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Jaikrishna Nemallapudi jaikrishnax.nemallapudi@intel.com Cc: Senthilnathan Veppur senthilnathanx.veppur@intel.com Cc: Vinod Koul vinod.koul@intel.com Cc: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 48 ++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 37f11560186a..7413fff06d70 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -177,6 +177,7 @@ struct hdmi_spec { bool i915_bound; /* was i915 bound in this driver? */
struct hdac_chmap chmap;
- hda_nid_t vendor_nid;
};
#ifdef CONFIG_SND_HDA_I915 @@ -2381,14 +2382,15 @@ static void intel_haswell_enable_all_pins(struct hda_codec *codec, bool update_tree) { unsigned int vendor_param;
- struct hdmi_spec *spec = codec->spec;
- vendor_param = snd_hda_codec_read(codec, INTEL_VENDOR_NID, 0,
vendor_param = snd_hda_codec_read(codec, spec->vendor_nid, 0, INTEL_GET_VENDOR_VERB, 0); if (vendor_param == -1 || vendor_param & INTEL_EN_ALL_PIN_CVTS) return;
vendor_param |= INTEL_EN_ALL_PIN_CVTS;
- vendor_param = snd_hda_codec_read(codec, INTEL_VENDOR_NID, 0,
- vendor_param = snd_hda_codec_read(codec, spec->vendor_nid, 0, INTEL_SET_VENDOR_VERB, vendor_param); if (vendor_param == -1) return;
@@ -2400,8 +2402,9 @@ static void intel_haswell_enable_all_pins(struct hda_codec *codec, static void intel_haswell_fixup_enable_dp12(struct hda_codec *codec) { unsigned int vendor_param;
- struct hdmi_spec *spec = codec->spec;
- vendor_param = snd_hda_codec_read(codec, INTEL_VENDOR_NID, 0,
- vendor_param = snd_hda_codec_read(codec, spec->vendor_nid, 0, INTEL_GET_VENDOR_VERB, 0); if (vendor_param == -1 || vendor_param & INTEL_EN_DP12) return;
@@ -2409,7 +2412,7 @@ static void intel_haswell_fixup_enable_dp12(struct hda_codec *codec) /* enable DP1.2 mode */ vendor_param |= INTEL_EN_DP12; snd_hdac_regmap_add_vendor_verb(&codec->core, INTEL_SET_VENDOR_VERB);
- snd_hda_codec_write_cache(codec, INTEL_VENDOR_NID, 0,
- snd_hda_codec_write_cache(codec, spec->vendor_nid, 0, INTEL_SET_VENDOR_VERB, vendor_param);
}
@@ -2502,22 +2505,11 @@ static void i915_pin_cvt_fixup(struct hda_codec *codec, } }
-/* Intel Haswell and onwards; audio component with eld notifier */ -static int patch_i915_hsw_hdmi(struct hda_codec *codec) +static int intel_haswell_common_init(struct hda_codec *codec) {
- struct hdmi_spec *spec;
- struct hdmi_spec *spec = codec->spec; int err;
- /* HSW+ requires i915 binding */
- if (!codec->bus->core.audio_component) {
codec_info(codec, "No i915 binding for Intel HDMI/DP codec\n");return -ENODEV;- }
- err = alloc_generic_hdmi(codec);
- if (err < 0)
return err;- spec = codec->spec; codec->dp_mst = true; spec->dyn_pcm_assign = true;
@@ -2548,6 +2540,28 @@ static int patch_i915_hsw_hdmi(struct hda_codec *codec) return 0; }
+/* Intel Haswell and onwards; audio component with eld notifier */ +static int patch_i915_hsw_hdmi(struct hda_codec *codec) +{
- struct hdmi_spec *spec;
- int err;
- /* HSW+ requires i915 binding */
- if (!codec->bus->core.audio_component) {
codec_info(codec, "No i915 binding for Intel HDMI/DP codec\n");return -ENODEV;- }
- err = alloc_generic_hdmi(codec);
- if (err < 0)
return err;- spec = codec->spec;
- spec->vendor_nid = INTEL_VENDOR_NID;
- return intel_haswell_common_init(codec);
+}
It'd be easier to change like:
/* Intel Haswell and onwards; audio component with eld notifier */ static int intel_hsw_common_init(struct hda_codec *codec, int vendor_nid) { .... spec->vendor_nid = nid; .... }
static int patch_i915_hsw_hdmi(struct hda_codec *codec) { return intel_hsw_common_init(codec, INTEL_VENDOR_NID); }
static int patch_i915_glk_hdmi(struct hda_codec *codec) { return intel_hsw_common_init(codec, INTEL_GLK_VENDOR_NID); }
Also, there is no merit to split this change to two patches.
thanks,
Takashi
participants (2)
-
Subhransu S. Prusty -
Takashi Iwai