Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Libin Yang <libin.yang@linux.intel.com>
Cc: Libin Yang <libin.yang@intel.com>,
	alsa-devel@alsa-project.org, mengdong.lin@linux.intel.com
Subject: Re: [RFC PATCH 2/2] ALSA: hda - HDMI codec driver dynamically attach PCM
Date: Mon, 09 Nov 2015 08:55:11 +0100	[thread overview]
Message-ID: <s5hoaf3k3og.wl-tiwai@suse.de> (raw)
In-Reply-To: <563FF90A.3000206@linux.intel.com>

On Mon, 09 Nov 2015 02:38:18 +0100,
Libin Yang wrote:
> 
> Hi Takashi,
> 
> On 11/06/2015 05:12 PM, Takashi Iwai wrote:
> > On Fri, 06 Nov 2015 06:33:48 +0100,
> > Libin Yang wrote:
> >>
> >> Hi Takashi,
> >>
> >> On 11/04/2015 01:38 PM, Libin Yang wrote:
> >>> On 11/04/2015 12:44 AM, Takashi Iwai wrote:
> >>>> On Tue, 03 Nov 2015 09:22:53 +0100,
> >>>> libin.yang@linux.intel.com wrote:
> >>>>>
> >>>>> From: Libin Yang <libin.yang@intel.com>
> >>>>>
> >>>>> Allocate the PCM based on the number of pin and device entry.
> >>>>> The number of PCM is pin num plus device entry number per pin.
> >>>>> For example, on Intel platform, it will be 5 PCMs.
> >>>>>
> >>>>> Do not attach the PCM to pin in initialization.
> >>>>> Dynamically attach PCM to pin when monitor is connected
> >>>>> in HDMI audio codec driver.
> >>>>>
> >>>>> When monitor is disconnected, detach the PCM from the pin.
> >>>>> And if the audio is playing, stop the playback.
> >>>>>
> >>>>> The below is the example of device entry and PCM binding:
> >>>>> For a monitor is plugged in, we need to dynamically assign
> >>>>> this monitor to 5 PCM devices (on Intel platform):
> >>>>> For a monitor at pin nid 0x05, dev index 0, it will prefer PCM 3
> >>>>> For a monitor at pin nid 0x06, dev index 0, it will prefer PCM 7
> >>>>> For a monitor at pin nid 0x07, dev index 0, it will prefer PCM 8
> >>>>> For a monitor at dev index 1 (any pin), it will prefer PCM 9
> >>>>> For a monitor at dev index 2 (any pin), it will prefer PCM 10
> >>>>>
> >>>>> If the preferred PCM is not available, try PCM in this order:
> >>>>> 9, 10, 3, 7 ,8.
> >>>>>
> >>>>> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> >>>>
> >>>> We should split the changes to a few patches here.  For example,
> >>>> stopping the stream at discussion is basically an implementation
> >>>> independent from the MST itself.  IOW, the dynamic attach/detach can
> >>>> be implemented and tested even without MST support.  Let's code them
> >>>> at first, then add MST support.
> >>>
> >>> This patch actually doesn't depend on MST. I use the dev_num to
> >>> determine how many PCMs to be created.
> >>>
> >>> To Split the patch, how about I create the PCM number based the pin
> >>> number in the patch?
> >>>
> >>>>
> >>>> Also, it's an open question whether we apply the dynamic attach/detach
> >>>> to all devices that use the generic hdmi framework.  It means
> >>>> including AMD and Nvidia.  You hard-coded it to be applied
> >>>> unconditionally.  Would it break anything potentially...?
> >>>
> >>> I have no other verdors platforms to test. But it seems to be OK on
> >>> other platforms. The impact is, i think, it will stop the stream when
> >>> monitor is disconnected. Should we apply the code by judging whehter
> >>> it is Intel platform?
> >>
> >> If you are concerning the impact on other vendors, what about I create
> >> a separate mst codec driver? In the driver, I will still use virtual
> >> pin, but other vendors platform will not be supported at the moment.
> >> Other vendors can add their patches to the new codec driver to support
> >> their own mst audio.
> >
> > No, it's not the option.  Such a behavior change is the big burden to
> > user-space.  We should provide consistent behavior as much as
> > possible.
> 
> OK, I see. What do you think if I judge the platform before using DP 
> MST mode?

Yes, I'd love to have more tests before doing any radical changes.

As already mentioned, basically your work has to individual changes:
the stream validation and dynamic disconnect per monitor plug/unplug,
and the MST support via dynamic PCM assignment.

The former is definitely a generic framework, and it's worth to have
even without MST support.  OTOH, the former *is* the biggest change
from user-space POV.  User won't notice much about the latter -- the
usage pattern should be almost same.  So, it's the former change we
need to test and evaluate carefully.


Takashi

  reply	other threads:[~2015-11-09  7:55 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-03  8:22 [RFC PATCH 0/2] ALSA: hda - patch set for DP MST audio support libin.yang
2015-11-03  8:22 ` [RFC PATCH 1/2] ALSA: hda - add hda_dev_t typedef for DP MST audio libin.yang
2015-11-03 16:28   ` Takashi Iwai
2015-11-04  2:37     ` Libin Yang
2015-11-03  8:22 ` [RFC PATCH 2/2] ALSA: hda - HDMI codec driver dynamically attach PCM libin.yang
2015-11-03 16:44   ` Takashi Iwai
2015-11-04  5:38     ` Libin Yang
2015-11-04  7:55       ` Libin Yang
2015-11-06  5:33       ` Libin Yang
2015-11-06  9:12         ` Takashi Iwai
2015-11-09  1:38           ` Libin Yang
2015-11-09  7:55             ` Takashi Iwai [this message]
2015-11-09  8:45               ` Libin Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hoaf3k3og.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=libin.yang@intel.com \
    --cc=libin.yang@linux.intel.com \
    --cc=mengdong.lin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox