[alsa-devel] [PATCH v2] ALSA: x86: Select CONFIG_HDMI_LPE_AUDIO as default

[Takashi Iwai]

On Mon, 20 Mar 2017 10:41:07 +0100,
Ian W MORRISON wrote:
> 
> On 20 March 2017 at 19:41, Takashi Iwai <tiwai at suse.de> wrote:
> 
> > On Mon, 20 Mar 2017 09:17:30 +0100,
> > Ian W MORRISON wrote:
> > >
> > > Oops ... forgot to copy alsa-devel and Pierre-Louis.
> > >
> > > On 20 March 2017 at 18:59, Takashi Iwai <tiwai at suse.de> wrote:
> > >
> > > > On Mon, 20 Mar 2017 08:42:32 +0100,
> > > > Ian W MORRISON wrote:
> > > > >
> > > > > The upstream kernel builds for distributions such as Ubuntu which now
> > > > > includes binary packages for v4.11 mainline kernel release
> > candidates are
> > > > > promoted as a way of testing upstream kernels to to confirm that
> > upstream
> > > > > has fixed a specific issue (see https://wiki.ubuntu.com/
> > > > > Kernel/MainlineBuilds).
> > > > >
> > > > > Unfortunately the long awaited patch for providing HDMI audio
> > support for
> > > > > Bay Trail and Cherry Trail devices does not include this support
> > through
> > > > a
> > > > > module built by default.
> > > > >
> > > > > Through including by default of the two associated CONFIG settings
> > > > (SND_X86
> > > > > and HDMI_LPE_AUDIO), upstream kernel builds would automatically
> > provide
> > > > the
> > > > > much desired HDMI audio support by default.
> > > > >
> > > > > This patch uses a Kconfig 'default' statement to include the driver
> > as
> > > > > default.
> > > > >
> > > > > Changes in version 2: CONFIG_SND_X86 now a bool and changed default
> > m to
> > > > > default y
> > > > >
> > > > > Signed-off-by: Ian W Morrison <linuxium at linuxium.com.au>
> > > > > ---
> > > > >  sound/x86/Kconfig | 4 +++-
> > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/sound/x86/Kconfig b/sound/x86/Kconfig
> > > > > index 84c8f8fc..cac2270 100644
> > > > > --- a/sound/x86/Kconfig
> > > > > +++ b/sound/x86/Kconfig
> > > > > @@ -1,6 +1,7 @@
> > > > >  menuconfig SND_X86
> > > > > -       tristate "X86 sound devices"
> > > > > +       bool "X86 sound devices"
> > > > >         depends on X86
> > > > > +       default y
> > > >
> > > > This one is OK, but ...
> > > >
> > > > >         ---help---
> > > > >           X86 sound devices that don't fall under SoC or PCI
> > categories
> > > > >
> > > > > @@ -9,6 +10,7 @@ if SND_X86
> > > > >  config HDMI_LPE_AUDIO
> > > > >         tristate "HDMI audio without HDaudio on Intel Atom platforms"
> > > > >         depends on DRM_I915
> > > > > +       default y
> > > >
> > > > ... this is wrong.  Each driver config itself should be left
> > > > unspecified.
> > > >
> > > > It's distributor's job to choose the right config here.
> > > >
> > > > Actually this goes back to one of my earlier points: A distributor
> > doesn't
> > > have to set 'HDMI' as HDMI audio is automatically provided.
> >
> > Provided by who...?
> >
> 
> 
> I suppose it is provided as a result of the architecture one runs the
> primary Makefile on. With 'uname -m' returning 'x86_64' running 'make
> defconfig' results in 'CONFIG_HDMI=y' being set so 'drivers/video/Makefile'
> automatically makes 'drivers/video/hdmi.c' and as
> 'arch/x86/configs/x86_64_defconfig' includes 'CONFIG_DRM=y' and
> 'CONFIG_DRM_I915=y' and 'drivers/gpu/drm/i915/Makefile' makes
> 'intel_audio.c'.

The defconfig stuff supports only the limited device sets that are
supposed to be very common.  Is HDMI_LPE_AUDIO classified really as
such a thing?  I don't know...


> > > This is just an
> > > extension because by setting 'HDMI_LPE_AUDIO' the missing audio support
> > for
> > > BYT and CHT SoCs is then provided. Therefore, in this albeit unusual
> > > instance, I reason is it appropriate to set HDMI_LPE_AUDIO so that audio
> > is
> > > automatically provided regardless of distribution. If a distributor
> > didn't
> > > want to allow audio for BYT and CHT SoC based devices running their
> > distro
> > > then they could always remove it from their distro specific config.
> >
> > It's a wrong approach.  What we're discussing about is just a
> > configuration for a new individual driver, and the same rule should be
> > applied to it like others.
> >
> > Check other drivers.  See whether default=y (or =m) is set to
> > CONFIG_E1000E, as a random example.  With your argument, it should  be
> > set to y or m as default, since the Ethernet functionality is already
> > provided by the network core.
> >
> > In general, we don't set the default values to the driver configs
> > unless there is a VERY specific reason to do so.
> >
> >
> I'm trying to get HDMI audio by default for BYT & CHT SoC but currently
> this requires 'CONFIG_HDMI_LPE_AUDIO' set with a value of 'm'. I don't want
> to argue a 'special case' or promote a 'wrong approach' but just get HDMI
> audio working so is there another way to achieve this?

What's wrong with manually setting CONFIG_HDMI_LPE_AUDIO=m or =y?
IOW, do all features on your CHT/BYT machines work without adjusting
manually after defconfig?  If you have to do it in anyway, what makes
it special for HDMI_LPD_AUDIO?

> Would adding
> 'CONFIG_HDMI_LPE_AUDIO=y' to 'arch/x86/configs/x86_64_defconfig' be another
> mistake (assuming 'CONFIG_SND_X86' was set as per the first part of the
> revised patch)? Would anyone even accept a patch
> to 'arch/x86/configs/x86_64_defconfig' if this was acceptable?

The content in x86_64_defconfig is another story, and it's maintained
by x86 guys, so you can try it, of corse...


Takashi
> 
> 
> >
> > Takashi
> >