Message ID | 1391773652-25214-230-git-send-email-luis.henriques@canonical.com |
---|---|
State | New |
Headers | show |
Hi, On 02/07/2014 12:47 PM, Luis Henriques wrote: > 3.11.10.4 -stable review patch. If anyone has any objections, please let me know. > [...] > > +static int patch_nvhdmi(struct hda_codec *codec) > +{ > + struct hdmi_spec *spec; > + int err; > + > + err = patch_generic_hdmi(codec); > + if (err) > + return err; > + > + spec = codec->spec; > + > + return 0; > +} > [...] Is it just me or is it that the 'spec' variable has no use?
On Fri, Feb 07, 2014 at 12:58:47PM +0100, Levente Kurusa wrote: > Hi, > > On 02/07/2014 12:47 PM, Luis Henriques wrote: > > 3.11.10.4 -stable review patch. If anyone has any objections, please let me know. > > [...] > > > > +static int patch_nvhdmi(struct hda_codec *codec) > > +{ > > + struct hdmi_spec *spec; > > + int err; > > + > > + err = patch_generic_hdmi(codec); > > + if (err) > > + return err; > > + > > + spec = codec->spec; > > + > > + return 0; > > +} > > [...] > > Is it just me or is it that the 'spec' variable has no use? This does seem a little bit odd indeed. I've picked the backport provided by Stephen, which has been queued for the 3.10 and 3.12 stable kernels as well. The original patch actually modifies the ops field in the hdmi_spec struct, however this field doesn't exist in this kernel version. Stephen, could you please comment? Since this is just a partial backport of 611885bc963a ("ALSA: hda - hdmi: Disallow unsupported 2ch remapping on NVIDIA codecs"), I'm assuming this is correct -- although the spec variable could have been dropped. Cheers, -- Luis
On 02/07/2014 07:36 AM, Luis Henriques wrote: > On Fri, Feb 07, 2014 at 12:58:47PM +0100, Levente Kurusa wrote: >> Hi, >> >> On 02/07/2014 12:47 PM, Luis Henriques wrote: >>> 3.11.10.4 -stable review patch. If anyone has any objections, please let me know. >>> [...] >>> >>> +static int patch_nvhdmi(struct hda_codec *codec) >>> +{ >>> + struct hdmi_spec *spec; >>> + int err; >>> + >>> + err = patch_generic_hdmi(codec); >>> + if (err) >>> + return err; >>> + >>> + spec = codec->spec; >>> + >>> + return 0; >>> +} >>> [...] >> >> Is it just me or is it that the 'spec' variable has no use? > > This does seem a little bit odd indeed. I've picked the backport provided > by Stephen, which has been queued for the 3.10 and 3.12 stable kernels as > well. The original patch actually modifies the ops field in the hdmi_spec > struct, however this field doesn't exist in this kernel version. > > Stephen, could you please comment? Since this is just a partial backport > of 611885bc963a ("ALSA: hda - hdmi: Disallow unsupported 2ch remapping on > NVIDIA codecs"), I'm assuming this is correct -- although the spec > variable could have been dropped. The very next patch of mine to this file (in other stable releases) makes use of the spec variable. I kept the addition of the spec variable in this patch, even though it isn't used until the next patch, so that it stayed as part of the backport of this patch which originally added it, rather than moving it to the other patch I backported, and hence mixing up multiple upstream patches in the same backported patch.
On Fri, Feb 07, 2014 at 09:20:57AM -0700, Stephen Warren wrote: > On 02/07/2014 07:36 AM, Luis Henriques wrote: > > On Fri, Feb 07, 2014 at 12:58:47PM +0100, Levente Kurusa wrote: > >> Hi, > >> > >> On 02/07/2014 12:47 PM, Luis Henriques wrote: > >>> 3.11.10.4 -stable review patch. If anyone has any objections, please let me know. > >>> [...] > >>> > >>> +static int patch_nvhdmi(struct hda_codec *codec) > >>> +{ > >>> + struct hdmi_spec *spec; > >>> + int err; > >>> + > >>> + err = patch_generic_hdmi(codec); > >>> + if (err) > >>> + return err; > >>> + > >>> + spec = codec->spec; > >>> + > >>> + return 0; > >>> +} > >>> [...] > >> > >> Is it just me or is it that the 'spec' variable has no use? > > > > This does seem a little bit odd indeed. I've picked the backport provided > > by Stephen, which has been queued for the 3.10 and 3.12 stable kernels as > > well. The original patch actually modifies the ops field in the hdmi_spec > > struct, however this field doesn't exist in this kernel version. > > > > Stephen, could you please comment? Since this is just a partial backport > > of 611885bc963a ("ALSA: hda - hdmi: Disallow unsupported 2ch remapping on > > NVIDIA codecs"), I'm assuming this is correct -- although the spec > > variable could have been dropped. > > The very next patch of mine to this file (in other stable releases) > makes use of the spec variable. I kept the addition of the spec variable > in this patch, even though it isn't used until the next patch, so that > it stayed as part of the backport of this patch which originally added > it, rather than moving it to the other patch I backported, and hence > mixing up multiple upstream patches in the same backported patch. Doh! I've applied both of your two patches to the 3.11 kernel. And while I first reviewed them, I've seen that detail and it made perfect sense ;) Anyway, thank you for your clarification. Cheers, -- Luis
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 0b36c63..7f4c726 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -2562,6 +2562,20 @@ static int patch_nvhdmi_8ch_7x(struct hda_codec *codec) return 0; } +static int patch_nvhdmi(struct hda_codec *codec) +{ + struct hdmi_spec *spec; + int err; + + err = patch_generic_hdmi(codec); + if (err) + return err; + + spec = codec->spec; + + return 0; +} + /* * ATI-specific implementations * @@ -2634,30 +2648,30 @@ static const struct hda_codec_preset snd_hda_preset_hdmi[] = { { .id = 0x10de0005, .name = "MCP77/78 HDMI", .patch = patch_nvhdmi_8ch_7x }, { .id = 0x10de0006, .name = "MCP77/78 HDMI", .patch = patch_nvhdmi_8ch_7x }, { .id = 0x10de0007, .name = "MCP79/7A HDMI", .patch = patch_nvhdmi_8ch_7x }, -{ .id = 0x10de000a, .name = "GPU 0a HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de000b, .name = "GPU 0b HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de000c, .name = "MCP89 HDMI", .patch = patch_generic_hdmi }, -{ .id = 0x10de000d, .name = "GPU 0d HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0010, .name = "GPU 10 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0011, .name = "GPU 11 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0012, .name = "GPU 12 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0013, .name = "GPU 13 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0014, .name = "GPU 14 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0015, .name = "GPU 15 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0016, .name = "GPU 16 HDMI/DP", .patch = patch_generic_hdmi }, +{ .id = 0x10de000a, .name = "GPU 0a HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de000b, .name = "GPU 0b HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de000c, .name = "MCP89 HDMI", .patch = patch_nvhdmi }, +{ .id = 0x10de000d, .name = "GPU 0d HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0010, .name = "GPU 10 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0011, .name = "GPU 11 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0012, .name = "GPU 12 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0013, .name = "GPU 13 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0014, .name = "GPU 14 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0015, .name = "GPU 15 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0016, .name = "GPU 16 HDMI/DP", .patch = patch_nvhdmi }, /* 17 is known to be absent */ -{ .id = 0x10de0018, .name = "GPU 18 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0019, .name = "GPU 19 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de001a, .name = "GPU 1a HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de001b, .name = "GPU 1b HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de001c, .name = "GPU 1c HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0040, .name = "GPU 40 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0041, .name = "GPU 41 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0042, .name = "GPU 42 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0043, .name = "GPU 43 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0044, .name = "GPU 44 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0051, .name = "GPU 51 HDMI/DP", .patch = patch_generic_hdmi }, -{ .id = 0x10de0060, .name = "GPU 60 HDMI/DP", .patch = patch_generic_hdmi }, +{ .id = 0x10de0018, .name = "GPU 18 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0019, .name = "GPU 19 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de001a, .name = "GPU 1a HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de001b, .name = "GPU 1b HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de001c, .name = "GPU 1c HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0040, .name = "GPU 40 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0041, .name = "GPU 41 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0042, .name = "GPU 42 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0043, .name = "GPU 43 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0044, .name = "GPU 44 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0051, .name = "GPU 51 HDMI/DP", .patch = patch_nvhdmi }, +{ .id = 0x10de0060, .name = "GPU 60 HDMI/DP", .patch = patch_nvhdmi }, { .id = 0x10de0067, .name = "MCP67 HDMI", .patch = patch_nvhdmi_2ch }, { .id = 0x10de8001, .name = "MCP73 HDMI", .patch = patch_nvhdmi_2ch }, { .id = 0x11069f80, .name = "VX900 HDMI/DP", .patch = patch_via_hdmi },