[2/2] ALSA: hda - hdmi: Set infoframe and channel mapping even without sink
diff mbox

Message ID 1399246724-16611-2-git-send-email-anssi.hannula@iki.fi
State Accepted
Commit 561a7d6e853f170877a2ebf87f64e0e846b6a99c
Headers show

Commit Message

Anssi Hannula May 4, 2014, 11:38 p.m. UTC
Currently infoframe contents and channel mapping are only set when a
sink (monitor) is present.

However, this does not make much sense, since
1) We can make a very reasonable guess on CA after 18e391862c ("ALSA:
   hda - hdmi: Fallback to ALSA allocation when selecting CA") or by
   relying on a previously valid ELD (or we may be using a
   user-specified channel map).
2) Not setting infoframe contents and channel count simply means they
   are left at a possibly incorrect state - playback is still allowed
   to proceed (with missing or wrongly mapped channels).

Reasons for monitor_present being 0 include disconnected cable, video
driver issues, or codec not being spec-compliant. Note that in
actual disconnected-cable case it should not matter if these settings
are wrong as they will be re-set after jack detection, though.

Change the behavior to allow the infoframe contents and the channel
mapping to be set even without a sink/monitor, either based on the
previous valid ELD contents, if any, or based on sensible defaults
(standard channel layouts or provided custom map, sink type HDMI).

Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
Tested-by: Stephan Raue <stephan@openelec.tv>
---

This simply removes the monitor_present check, which is probably more
trouble than it is worth, as it may trip obscure setups even though we
can tolerate !monitor_present and !eld in 99%+ of all cases by using
defaults, nowadays post-18e391862c anyway.
Downside is that missing monitor_present may not be noticed by the user
as ~everything continues to work.


 sound/pci/hda/patch_hdmi.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Takashi Iwai May 5, 2014, 2:34 p.m. UTC | #1
At Mon,  5 May 2014 02:38:44 +0300,
Anssi Hannula wrote:
> 
> Currently infoframe contents and channel mapping are only set when a
> sink (monitor) is present.
> 
> However, this does not make much sense, since
> 1) We can make a very reasonable guess on CA after 18e391862c ("ALSA:
>    hda - hdmi: Fallback to ALSA allocation when selecting CA") or by
>    relying on a previously valid ELD (or we may be using a
>    user-specified channel map).
> 2) Not setting infoframe contents and channel count simply means they
>    are left at a possibly incorrect state - playback is still allowed
>    to proceed (with missing or wrongly mapped channels).
> 
> Reasons for monitor_present being 0 include disconnected cable, video
> driver issues, or codec not being spec-compliant. Note that in
> actual disconnected-cable case it should not matter if these settings
> are wrong as they will be re-set after jack detection, though.
> 
> Change the behavior to allow the infoframe contents and the channel
> mapping to be set even without a sink/monitor, either based on the
> previous valid ELD contents, if any, or based on sensible defaults
> (standard channel layouts or provided custom map, sink type HDMI).
> 
> Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
> Tested-by: Stephan Raue <stephan@openelec.tv>
> ---
> 
> This simply removes the monitor_present check, which is probably more
> trouble than it is worth, as it may trip obscure setups even though we
> can tolerate !monitor_present and !eld in 99%+ of all cases by using
> defaults, nowadays post-18e391862c anyway.
> Downside is that missing monitor_present may not be noticed by the user
> as ~everything continues to work.

Is this considered as a "fix" for 3.15 kernel?  If so, I find it a bit
distracting to queue the previous patch for stable while we're
removing the whole things here later.

If this is intended for testing for 3.16 kernel, I'm fine to take as
is.


thanks,

Takashi

> 
> 
>  sound/pci/hda/patch_hdmi.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 1edbb9c47c2d..016f785cdf45 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1127,10 +1127,6 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
>  					    AMP_OUT_UNMUTE);
>  
>  	eld = &per_pin->sink_eld;
> -	if (!eld->monitor_present) {
> -		hdmi_set_channel_count(codec, per_pin->cvt_nid, channels);
> -		return;
> -	}
>  
>  	if (!non_pcm && per_pin->chmap_set)
>  		ca = hdmi_manual_channel_allocation(channels, per_pin->chmap);
> -- 
> 1.8.4.5
>
Anssi Hannula May 5, 2014, 2:48 p.m. UTC | #2
Takashi Iwai kirjoitti 2014-05-05 17:34:
> At Mon,  5 May 2014 02:38:44 +0300,
> Anssi Hannula wrote:
>> 
>> Currently infoframe contents and channel mapping are only set when a
>> sink (monitor) is present.
>> 
>> However, this does not make much sense, since
>> 1) We can make a very reasonable guess on CA after 18e391862c ("ALSA:
>>    hda - hdmi: Fallback to ALSA allocation when selecting CA") or by
>>    relying on a previously valid ELD (or we may be using a
>>    user-specified channel map).
>> 2) Not setting infoframe contents and channel count simply means they
>>    are left at a possibly incorrect state - playback is still allowed
>>    to proceed (with missing or wrongly mapped channels).
>> 
>> Reasons for monitor_present being 0 include disconnected cable, video
>> driver issues, or codec not being spec-compliant. Note that in
>> actual disconnected-cable case it should not matter if these settings
>> are wrong as they will be re-set after jack detection, though.
>> 
>> Change the behavior to allow the infoframe contents and the channel
>> mapping to be set even without a sink/monitor, either based on the
>> previous valid ELD contents, if any, or based on sensible defaults
>> (standard channel layouts or provided custom map, sink type HDMI).
>> 
>> Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
>> Tested-by: Stephan Raue <stephan@openelec.tv>
>> ---
>> 
>> This simply removes the monitor_present check, which is probably more
>> trouble than it is worth, as it may trip obscure setups even though we
>> can tolerate !monitor_present and !eld in 99%+ of all cases by using
>> defaults, nowadays post-18e391862c anyway.
>> Downside is that missing monitor_present may not be noticed by the 
>> user
>> as ~everything continues to work.
> 
> Is this considered as a "fix" for 3.15 kernel?  If so, I find it a bit
> distracting to queue the previous patch for stable while we're
> removing the whole things here later.
> 
> If this is intended for testing for 3.16 kernel, I'm fine to take as
> is.

Well, I intended it for 3.16, since I don't have anything concrete that 
it
would fix that the previous AFAICS safer patch did not (for now, at 
least...).

Sorry for not making it explicit.

> thanks,
> 
> Takashi
> 
>> 
>> 
>>  sound/pci/hda/patch_hdmi.c | 4 ----
>>  1 file changed, 4 deletions(-)
>> 
>> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
>> index 1edbb9c47c2d..016f785cdf45 100644
>> --- a/sound/pci/hda/patch_hdmi.c
>> +++ b/sound/pci/hda/patch_hdmi.c
>> @@ -1127,10 +1127,6 @@ static void hdmi_setup_audio_infoframe(struct 
>> hda_codec *codec,
>>  					    AMP_OUT_UNMUTE);
>> 
>>  	eld = &per_pin->sink_eld;
>> -	if (!eld->monitor_present) {
>> -		hdmi_set_channel_count(codec, per_pin->cvt_nid, channels);
>> -		return;
>> -	}
>> 
>>  	if (!non_pcm && per_pin->chmap_set)
>>  		ca = hdmi_manual_channel_allocation(channels, per_pin->chmap);
>> --
>> 1.8.4.5
>>
Takashi Iwai May 5, 2014, 2:55 p.m. UTC | #3
At Mon, 05 May 2014 17:48:04 +0300,
Anssi Hannula wrote:
> 
> Takashi Iwai kirjoitti 2014-05-05 17:34:
> > At Mon,  5 May 2014 02:38:44 +0300,
> > Anssi Hannula wrote:
> >> 
> >> Currently infoframe contents and channel mapping are only set when a
> >> sink (monitor) is present.
> >> 
> >> However, this does not make much sense, since
> >> 1) We can make a very reasonable guess on CA after 18e391862c ("ALSA:
> >>    hda - hdmi: Fallback to ALSA allocation when selecting CA") or by
> >>    relying on a previously valid ELD (or we may be using a
> >>    user-specified channel map).
> >> 2) Not setting infoframe contents and channel count simply means they
> >>    are left at a possibly incorrect state - playback is still allowed
> >>    to proceed (with missing or wrongly mapped channels).
> >> 
> >> Reasons for monitor_present being 0 include disconnected cable, video
> >> driver issues, or codec not being spec-compliant. Note that in
> >> actual disconnected-cable case it should not matter if these settings
> >> are wrong as they will be re-set after jack detection, though.
> >> 
> >> Change the behavior to allow the infoframe contents and the channel
> >> mapping to be set even without a sink/monitor, either based on the
> >> previous valid ELD contents, if any, or based on sensible defaults
> >> (standard channel layouts or provided custom map, sink type HDMI).
> >> 
> >> Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
> >> Tested-by: Stephan Raue <stephan@openelec.tv>
> >> ---
> >> 
> >> This simply removes the monitor_present check, which is probably more
> >> trouble than it is worth, as it may trip obscure setups even though we
> >> can tolerate !monitor_present and !eld in 99%+ of all cases by using
> >> defaults, nowadays post-18e391862c anyway.
> >> Downside is that missing monitor_present may not be noticed by the 
> >> user
> >> as ~everything continues to work.
> > 
> > Is this considered as a "fix" for 3.15 kernel?  If so, I find it a bit
> > distracting to queue the previous patch for stable while we're
> > removing the whole things here later.
> > 
> > If this is intended for testing for 3.16 kernel, I'm fine to take as
> > is.
> 
> Well, I intended it for 3.16, since I don't have anything concrete that 
> it
> would fix that the previous AFAICS safer patch did not (for now, at 
> least...).
> 
> Sorry for not making it explicit.

Alright, I applied both, one to for-linus and another to for-next
branches.  Thanks.


Takashi

Patch
diff mbox

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 1edbb9c47c2d..016f785cdf45 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1127,10 +1127,6 @@  static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
 					    AMP_OUT_UNMUTE);
 
 	eld = &per_pin->sink_eld;
-	if (!eld->monitor_present) {
-		hdmi_set_channel_count(codec, per_pin->cvt_nid, channels);
-		return;
-	}
 
 	if (!non_pcm && per_pin->chmap_set)
 		ca = hdmi_manual_channel_allocation(channels, per_pin->chmap);