Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1340864 > unrolled thread

Re: [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915 Takashi Iwai <tiwai@suse.de> - 2016-02-23 18:00 +0100
    Re: [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for  i915 Martin Kepplinger <martink@posteo.de> - 2016-02-23 18:20 +0100
    Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over  HDMI for i915 Ville Syrjälä <ville.syrjala@linux.intel.com> - 2016-02-23 18:20 +0100
      Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over  HDMI for i915 Martin Kepplinger <martink@posteo.de> - 2016-02-23 20:20 +0100
        Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915 Takashi Iwai <tiwai@suse.de> - 2016-02-24 09:00 +0100
          Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915 Takashi Iwai <tiwai@suse.de> - 2016-02-24 10:20 +0100
            Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over  HDMI for i915 Martin Kepplinger <martink@posteo.de> - 2016-02-24 13:30 +0100
              Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915 Takashi Iwai <tiwai@suse.de> - 2016-02-24 15:30 +0100

#1340864 — Re: [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915

On Mon, 22 Feb 2016 22:37:28 +0100,
Martin Kepplinger wrote:
> 
> Am 2016-02-22 um 20:10 schrieb Takashi Iwai:
> > On Mon, 22 Feb 2016 19:58:18 +0100,
> > Martin Kepplinger wrote:
> >>
> >> Am 2016-02-22 um 15:12 schrieb Takashi Iwai:
> >>> On Mon, 22 Feb 2016 15:02:56 +0100,
> >>> Martin Kepplinger wrote:
> >>>>> And how about my questions in the previous mail?  Does
> >>>>> i915_audio_component_get_eld() is called and returns 0?
> >>>>> And is monitor_present set true or false?
> >>>>
> >>>> i915_audio_component_get_eld() returns 0 and monitor_present is 0.
> >>>>>
> >>>>> If i915_audio_component_get_eld() is called but returns zero, track
> >>>>> the code flow there.  It means either intel_encoder is NULL or
> >>>>> intel_dig_port->audio_connector is NULL.
> >>>>
> >>>> intel_dig_port->audio_connector is NULL!
> >>>>
> >>>> (when called during boot and during HDMI plugin)
> >>>
> >>> Interesting.  The relevant code flow should be like:
> >>>
> >>>   intel_audio_codec_enable()
> >>>   -> acomp->audio_ops->pin_eld_notify()
> >>>     -> intel_pin_eld_notify()
> >>>       -> check_presence_and_report()
> >>>         -> hdmi_present_sense()
> >>> 	  -> sync_eld_via_acomp()
> >>> 	    -> snd_hdac_acomp_get_eld()
> >>> 	      -> i915_audio_component_get_eld()
> >>>
> >>> So, at first, check whether intel_dig_port in both
> >>> intel_audio_codec_enable() and i915_audio_component_get_eld() points
> >>> to the same object address.  The audio_connector must be set in
> >>> intel_audio_codec_enable(), thus basically it must be non-NULL at
> >>> i915_audio_component_get_eld(), too.
> >>>
> >>
> >> intel_dig_port is *not* the same object in these 2 places. During
> >> plugin, see:
> >>
> >> [  146.934091] in intel_audio_codec_enable: intel_dig_port is
> >> ffff8800a1f54000
> >> [  146.934121] in i915_audio_component_get_eld: intel_dig_port is
> >> ffff880244f7d000
> >>
> >> sorry for the slow responses. I'll try to go back that direction unless
> >> again someone comes up with other suggestions.
> > 
> > Thanks, this makes sense.  It implies that the digital port mapping is
> > somehow wrong.  There are three places setting dig_port_map[], one in
> > intel_ddi_init(), one in intel_dp_init() and another in
> > intel_hdmi_init().  Try to check which function creates which object
> > assigned to which port number, together with drm.debug=0x0e debug
> > messages.
> > 
> without using drm.debug=0x0e, but by printing the kmalloc'ed objects in
> those 3 functions with ports, I found:
> 
> 2 of them are running, only during boot:
> 
> [    2.322865] intel_hdmi_init: intel_dig_port is ffff880242564000  port 1
> [    2.322999] intel_dp_init: intel_dig_port is ffff880242f30000  port 1
> 
> is is correct for them to have both port 1? Any more ideas?

Adding intel-gfx ML to Cc.

Martin, is the machine SandyBridge or IvyBridge?

In anyway it's PCH_SPLIT and there can call both intel_hdmi_init() and 
intel_dp_init() for the same port although both functions map
intel_dig_port[].  The assumption of intel_dig_port[] reverse mapping
is that there is only a single intel_dig_port assigned to a port, but
this doesn't look correct...


Takashi

[toc] | [next] | [standalone]

#1340885 — Re: [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915

Am 2016-02-23 um 17:57 schrieb Takashi Iwai:
> On Mon, 22 Feb 2016 22:37:28 +0100,
> Martin Kepplinger wrote:
>>
>> Am 2016-02-22 um 20:10 schrieb Takashi Iwai:
>>> On Mon, 22 Feb 2016 19:58:18 +0100,
>>> Martin Kepplinger wrote:
>>>>
>>>> Am 2016-02-22 um 15:12 schrieb Takashi Iwai:
>>>>> On Mon, 22 Feb 2016 15:02:56 +0100,
>>>>> Martin Kepplinger wrote:
>>>>>>> And how about my questions in the previous mail?  Does
>>>>>>> i915_audio_component_get_eld() is called and returns 0?
>>>>>>> And is monitor_present set true or false?
>>>>>>
>>>>>> i915_audio_component_get_eld() returns 0 and monitor_present is 0.
>>>>>>>
>>>>>>> If i915_audio_component_get_eld() is called but returns zero, track
>>>>>>> the code flow there.  It means either intel_encoder is NULL or
>>>>>>> intel_dig_port->audio_connector is NULL.
>>>>>>
>>>>>> intel_dig_port->audio_connector is NULL!
>>>>>>
>>>>>> (when called during boot and during HDMI plugin)
>>>>>
>>>>> Interesting.  The relevant code flow should be like:
>>>>>
>>>>>   intel_audio_codec_enable()
>>>>>   -> acomp->audio_ops->pin_eld_notify()
>>>>>     -> intel_pin_eld_notify()
>>>>>       -> check_presence_and_report()
>>>>>         -> hdmi_present_sense()
>>>>> 	  -> sync_eld_via_acomp()
>>>>> 	    -> snd_hdac_acomp_get_eld()
>>>>> 	      -> i915_audio_component_get_eld()
>>>>>
>>>>> So, at first, check whether intel_dig_port in both
>>>>> intel_audio_codec_enable() and i915_audio_component_get_eld() points
>>>>> to the same object address.  The audio_connector must be set in
>>>>> intel_audio_codec_enable(), thus basically it must be non-NULL at
>>>>> i915_audio_component_get_eld(), too.
>>>>>
>>>>
>>>> intel_dig_port is *not* the same object in these 2 places. During
>>>> plugin, see:
>>>>
>>>> [  146.934091] in intel_audio_codec_enable: intel_dig_port is
>>>> ffff8800a1f54000
>>>> [  146.934121] in i915_audio_component_get_eld: intel_dig_port is
>>>> ffff880244f7d000
>>>>
>>>> sorry for the slow responses. I'll try to go back that direction unless
>>>> again someone comes up with other suggestions.
>>>
>>> Thanks, this makes sense.  It implies that the digital port mapping is
>>> somehow wrong.  There are three places setting dig_port_map[], one in
>>> intel_ddi_init(), one in intel_dp_init() and another in
>>> intel_hdmi_init().  Try to check which function creates which object
>>> assigned to which port number, together with drm.debug=0x0e debug
>>> messages.
>>>
>> without using drm.debug=0x0e, but by printing the kmalloc'ed objects in
>> those 3 functions with ports, I found:
>>
>> 2 of them are running, only during boot:
>>
>> [    2.322865] intel_hdmi_init: intel_dig_port is ffff880242564000  port 1
>> [    2.322999] intel_dp_init: intel_dig_port is ffff880242f30000  port 1
>>
>> is is correct for them to have both port 1? Any more ideas?
> 
> Adding intel-gfx ML to Cc.

yes thanks, maybe people there have noticed this too. I can't be the
only one.

> 
> Martin, is the machine SandyBridge or IvyBridge?

SandyBridge.

> 
> In anyway it's PCH_SPLIT and there can call both intel_hdmi_init() and 
> intel_dp_init() for the same port although both functions map
> intel_dig_port[].  The assumption of intel_dig_port[] reverse mapping
> is that there is only a single intel_dig_port assigned to a port, but
> this doesn't look correct...
> 
> 
> Takashi
> 

[toc] | [prev] | [next] | [standalone]

#1340889 — Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915

On Tue, Feb 23, 2016 at 05:57:40PM +0100, Takashi Iwai wrote:
> On Mon, 22 Feb 2016 22:37:28 +0100,
> Martin Kepplinger wrote:
> > 
> > Am 2016-02-22 um 20:10 schrieb Takashi Iwai:
> > > On Mon, 22 Feb 2016 19:58:18 +0100,
> > > Martin Kepplinger wrote:
> > >>
> > >> Am 2016-02-22 um 15:12 schrieb Takashi Iwai:
> > >>> On Mon, 22 Feb 2016 15:02:56 +0100,
> > >>> Martin Kepplinger wrote:
> > >>>>> And how about my questions in the previous mail?  Does
> > >>>>> i915_audio_component_get_eld() is called and returns 0?
> > >>>>> And is monitor_present set true or false?
> > >>>>
> > >>>> i915_audio_component_get_eld() returns 0 and monitor_present is 0.
> > >>>>>
> > >>>>> If i915_audio_component_get_eld() is called but returns zero, track
> > >>>>> the code flow there.  It means either intel_encoder is NULL or
> > >>>>> intel_dig_port->audio_connector is NULL.
> > >>>>
> > >>>> intel_dig_port->audio_connector is NULL!
> > >>>>
> > >>>> (when called during boot and during HDMI plugin)
> > >>>
> > >>> Interesting.  The relevant code flow should be like:
> > >>>
> > >>>   intel_audio_codec_enable()
> > >>>   -> acomp->audio_ops->pin_eld_notify()
> > >>>     -> intel_pin_eld_notify()
> > >>>       -> check_presence_and_report()
> > >>>         -> hdmi_present_sense()
> > >>> 	  -> sync_eld_via_acomp()
> > >>> 	    -> snd_hdac_acomp_get_eld()
> > >>> 	      -> i915_audio_component_get_eld()
> > >>>
> > >>> So, at first, check whether intel_dig_port in both
> > >>> intel_audio_codec_enable() and i915_audio_component_get_eld() points
> > >>> to the same object address.  The audio_connector must be set in
> > >>> intel_audio_codec_enable(), thus basically it must be non-NULL at
> > >>> i915_audio_component_get_eld(), too.
> > >>>
> > >>
> > >> intel_dig_port is *not* the same object in these 2 places. During
> > >> plugin, see:
> > >>
> > >> [  146.934091] in intel_audio_codec_enable: intel_dig_port is
> > >> ffff8800a1f54000
> > >> [  146.934121] in i915_audio_component_get_eld: intel_dig_port is
> > >> ffff880244f7d000
> > >>
> > >> sorry for the slow responses. I'll try to go back that direction unless
> > >> again someone comes up with other suggestions.
> > > 
> > > Thanks, this makes sense.  It implies that the digital port mapping is
> > > somehow wrong.  There are three places setting dig_port_map[], one in
> > > intel_ddi_init(), one in intel_dp_init() and another in
> > > intel_hdmi_init().  Try to check which function creates which object
> > > assigned to which port number, together with drm.debug=0x0e debug
> > > messages.
> > > 
> > without using drm.debug=0x0e, but by printing the kmalloc'ed objects in
> > those 3 functions with ports, I found:
> > 
> > 2 of them are running, only during boot:
> > 
> > [    2.322865] intel_hdmi_init: intel_dig_port is ffff880242564000  port 1
> > [    2.322999] intel_dp_init: intel_dig_port is ffff880242f30000  port 1
> > 
> > is is correct for them to have both port 1? Any more ideas?
> 
> Adding intel-gfx ML to Cc.
> 
> Martin, is the machine SandyBridge or IvyBridge?
> 
> In anyway it's PCH_SPLIT and there can call both intel_hdmi_init() and 
> intel_dp_init() for the same port although both functions map
> intel_dig_port[].  The assumption of intel_dig_port[] reverse mapping
> is that there is only a single intel_dig_port assigned to a port, but
> this doesn't look correct...

On pre-HSW there can indeed be two encoders for the same port.
And I'm planning to change HSW+ to follow that model as well,
but I've been busy with other stuff to finish off that work [1]

[1] https://lists.freedesktop.org/archives/intel-gfx/2015-December/082384.html

-- 
Ville Syrjälä
Intel OTC

[toc] | [prev] | [next] | [standalone]

#1340960 — Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915

Am 2016-02-23 um 18:14 schrieb Ville Syrjälä:
> On Tue, Feb 23, 2016 at 05:57:40PM +0100, Takashi Iwai wrote:
>> On Mon, 22 Feb 2016 22:37:28 +0100,
>> Martin Kepplinger wrote:
>>>
>>> Am 2016-02-22 um 20:10 schrieb Takashi Iwai:
>>>> On Mon, 22 Feb 2016 19:58:18 +0100,
>>>> Martin Kepplinger wrote:
>>>>>
>>>>> Am 2016-02-22 um 15:12 schrieb Takashi Iwai:
>>>>>> On Mon, 22 Feb 2016 15:02:56 +0100,
>>>>>> Martin Kepplinger wrote:
>>>>>>>> And how about my questions in the previous mail?  Does
>>>>>>>> i915_audio_component_get_eld() is called and returns 0?
>>>>>>>> And is monitor_present set true or false?
>>>>>>>
>>>>>>> i915_audio_component_get_eld() returns 0 and monitor_present is 0.
>>>>>>>>
>>>>>>>> If i915_audio_component_get_eld() is called but returns zero, track
>>>>>>>> the code flow there.  It means either intel_encoder is NULL or
>>>>>>>> intel_dig_port->audio_connector is NULL.
>>>>>>>
>>>>>>> intel_dig_port->audio_connector is NULL!
>>>>>>>
>>>>>>> (when called during boot and during HDMI plugin)
>>>>>>
>>>>>> Interesting.  The relevant code flow should be like:
>>>>>>
>>>>>>   intel_audio_codec_enable()
>>>>>>   -> acomp->audio_ops->pin_eld_notify()
>>>>>>     -> intel_pin_eld_notify()
>>>>>>       -> check_presence_and_report()
>>>>>>         -> hdmi_present_sense()
>>>>>> 	  -> sync_eld_via_acomp()
>>>>>> 	    -> snd_hdac_acomp_get_eld()
>>>>>> 	      -> i915_audio_component_get_eld()
>>>>>>
>>>>>> So, at first, check whether intel_dig_port in both
>>>>>> intel_audio_codec_enable() and i915_audio_component_get_eld() points
>>>>>> to the same object address.  The audio_connector must be set in
>>>>>> intel_audio_codec_enable(), thus basically it must be non-NULL at
>>>>>> i915_audio_component_get_eld(), too.
>>>>>>
>>>>>
>>>>> intel_dig_port is *not* the same object in these 2 places. During
>>>>> plugin, see:
>>>>>
>>>>> [  146.934091] in intel_audio_codec_enable: intel_dig_port is
>>>>> ffff8800a1f54000
>>>>> [  146.934121] in i915_audio_component_get_eld: intel_dig_port is
>>>>> ffff880244f7d000
>>>>>
>>>>> sorry for the slow responses. I'll try to go back that direction unless
>>>>> again someone comes up with other suggestions.
>>>>
>>>> Thanks, this makes sense.  It implies that the digital port mapping is
>>>> somehow wrong.  There are three places setting dig_port_map[], one in
>>>> intel_ddi_init(), one in intel_dp_init() and another in
>>>> intel_hdmi_init().  Try to check which function creates which object
>>>> assigned to which port number, together with drm.debug=0x0e debug
>>>> messages.
>>>>
>>> without using drm.debug=0x0e, but by printing the kmalloc'ed objects in
>>> those 3 functions with ports, I found:
>>>
>>> 2 of them are running, only during boot:
>>>
>>> [    2.322865] intel_hdmi_init: intel_dig_port is ffff880242564000  port 1
>>> [    2.322999] intel_dp_init: intel_dig_port is ffff880242f30000  port 1
>>>
>>> is is correct for them to have both port 1? Any more ideas?
>>
>> Adding intel-gfx ML to Cc.
>>
>> Martin, is the machine SandyBridge or IvyBridge?
>>
>> In anyway it's PCH_SPLIT and there can call both intel_hdmi_init() and 
>> intel_dp_init() for the same port although both functions map
>> intel_dig_port[].  The assumption of intel_dig_port[] reverse mapping
>> is that there is only a single intel_dig_port assigned to a port, but
>> this doesn't look correct...
> 
> On pre-HSW there can indeed be two encoders for the same port.
> And I'm planning to change HSW+ to follow that model as well,
> but I've been busy with other stuff to finish off that work [1]
> 
> [1] https://lists.freedesktop.org/archives/intel-gfx/2015-December/082384.html
> 

So your work wouldn't fix hdmi-audio pre-HSW here?


Anyways, a quick fix would be good, and if not that, remember marking
future changes that fix this, for stable.

What implications would something like the following, that Takashi
suggested, have on other systems?

commit 7b20983c02928c55377b3cfa927257a17896ecee
Author: Martin Kepplinger <martink@posteo.de>
Date:   Tue Feb 23 18:29:05 2016 +0100

    sound: pci: hda: set codec_has_acomp to false due to pending regression

    v4.5 and it's significant intel hda changes break audio over HDMI
    for some cases. This is a temporary workaround to be reverted in
    the future.

    Signed-off-by: Martin Kepplinger <martink@posteo.de>

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 8ee78db..6d6f104 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -156,7 +156,7 @@ struct hdmi_spec {
        bool i915_bound; /* was i915 bound in this driver? */
 };

-#ifdef CONFIG_SND_HDA_I915
+#if 0
 #define codec_has_acomp(codec) \
        ((codec)->bus->core.audio_component != NULL)
 #else

[toc] | [prev] | [next] | [standalone]

#1341626 — Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915

On Tue, 23 Feb 2016 20:09:02 +0100,
Martin Kepplinger wrote:
> 
> Am 2016-02-23 um 18:14 schrieb Ville Syrjälä:
> > On Tue, Feb 23, 2016 at 05:57:40PM +0100, Takashi Iwai wrote:
> >> On Mon, 22 Feb 2016 22:37:28 +0100,
> >> Martin Kepplinger wrote:
> >>>
> >>> Am 2016-02-22 um 20:10 schrieb Takashi Iwai:
> >>>> On Mon, 22 Feb 2016 19:58:18 +0100,
> >>>> Martin Kepplinger wrote:
> >>>>>
> >>>>> Am 2016-02-22 um 15:12 schrieb Takashi Iwai:
> >>>>>> On Mon, 22 Feb 2016 15:02:56 +0100,
> >>>>>> Martin Kepplinger wrote:
> >>>>>>>> And how about my questions in the previous mail?  Does
> >>>>>>>> i915_audio_component_get_eld() is called and returns 0?
> >>>>>>>> And is monitor_present set true or false?
> >>>>>>>
> >>>>>>> i915_audio_component_get_eld() returns 0 and monitor_present is 0.
> >>>>>>>>
> >>>>>>>> If i915_audio_component_get_eld() is called but returns zero, track
> >>>>>>>> the code flow there.  It means either intel_encoder is NULL or
> >>>>>>>> intel_dig_port->audio_connector is NULL.
> >>>>>>>
> >>>>>>> intel_dig_port->audio_connector is NULL!
> >>>>>>>
> >>>>>>> (when called during boot and during HDMI plugin)
> >>>>>>
> >>>>>> Interesting.  The relevant code flow should be like:
> >>>>>>
> >>>>>>   intel_audio_codec_enable()
> >>>>>>   -> acomp->audio_ops->pin_eld_notify()
> >>>>>>     -> intel_pin_eld_notify()
> >>>>>>       -> check_presence_and_report()
> >>>>>>         -> hdmi_present_sense()
> >>>>>> 	  -> sync_eld_via_acomp()
> >>>>>> 	    -> snd_hdac_acomp_get_eld()
> >>>>>> 	      -> i915_audio_component_get_eld()
> >>>>>>
> >>>>>> So, at first, check whether intel_dig_port in both
> >>>>>> intel_audio_codec_enable() and i915_audio_component_get_eld() points
> >>>>>> to the same object address.  The audio_connector must be set in
> >>>>>> intel_audio_codec_enable(), thus basically it must be non-NULL at
> >>>>>> i915_audio_component_get_eld(), too.
> >>>>>>
> >>>>>
> >>>>> intel_dig_port is *not* the same object in these 2 places. During
> >>>>> plugin, see:
> >>>>>
> >>>>> [  146.934091] in intel_audio_codec_enable: intel_dig_port is
> >>>>> ffff8800a1f54000
> >>>>> [  146.934121] in i915_audio_component_get_eld: intel_dig_port is
> >>>>> ffff880244f7d000
> >>>>>
> >>>>> sorry for the slow responses. I'll try to go back that direction unless
> >>>>> again someone comes up with other suggestions.
> >>>>
> >>>> Thanks, this makes sense.  It implies that the digital port mapping is
> >>>> somehow wrong.  There are three places setting dig_port_map[], one in
> >>>> intel_ddi_init(), one in intel_dp_init() and another in
> >>>> intel_hdmi_init().  Try to check which function creates which object
> >>>> assigned to which port number, together with drm.debug=0x0e debug
> >>>> messages.
> >>>>
> >>> without using drm.debug=0x0e, but by printing the kmalloc'ed objects in
> >>> those 3 functions with ports, I found:
> >>>
> >>> 2 of them are running, only during boot:
> >>>
> >>> [    2.322865] intel_hdmi_init: intel_dig_port is ffff880242564000  port 1
> >>> [    2.322999] intel_dp_init: intel_dig_port is ffff880242f30000  port 1
> >>>
> >>> is is correct for them to have both port 1? Any more ideas?
> >>
> >> Adding intel-gfx ML to Cc.
> >>
> >> Martin, is the machine SandyBridge or IvyBridge?
> >>
> >> In anyway it's PCH_SPLIT and there can call both intel_hdmi_init() and 
> >> intel_dp_init() for the same port although both functions map
> >> intel_dig_port[].  The assumption of intel_dig_port[] reverse mapping
> >> is that there is only a single intel_dig_port assigned to a port, but
> >> this doesn't look correct...
> > 
> > On pre-HSW there can indeed be two encoders for the same port.
> > And I'm planning to change HSW+ to follow that model as well,
> > but I've been busy with other stuff to finish off that work [1]
> > 
> > [1] https://lists.freedesktop.org/archives/intel-gfx/2015-December/082384.html
> > 
> 
> So your work wouldn't fix hdmi-audio pre-HSW here?

The only question is how to pass intel_dig_port.  The current code is
a kind of optimization suggested after my initial patch.

Since dig_port_map[] is used only for the audio callback, we can
assign it dynamically just before the callbacks.

Could you try the patch below?  (It's totally untested.)

> Anyways, a quick fix would be good, and if not that, remember marking
> future changes that fix this, for stable.
> 
> What implications would something like the following, that Takashi
> suggested, have on other systems?

We'd take that action as a last resort, but it should be limited to
pre-HSW.  So if any, we'd need the check of codec ID.


thanks,

Takashi

---
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 31f6d212fb1b..e2bee6957cc0 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -521,6 +521,9 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
 
 	connector->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
 
+	/* referred in audio callbacks */
+	dev_priv->dig_port_map[port] = intel_encoder;
+
 	if (dev_priv->display.audio_codec_enable)
 		dev_priv->display.audio_codec_enable(connector, intel_encoder,
 						     adjusted_mode);
@@ -558,6 +561,8 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
 
 	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
 		acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, (int) port);
+
+	dev_priv->dig_port_map[port] = NULL;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6408e5583d7..63ba42aa2985 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3311,7 +3311,6 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 	intel_encoder->get_config = intel_ddi_get_config;
 
 	intel_dig_port->port = port;
-	dev_priv->dig_port_map[port] = intel_encoder;
 	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
 					  (DDI_BUF_PORT_REVERSAL |
 					   DDI_A_4_LANES);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d313cb9..ac6a37cbd323 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6035,7 +6035,6 @@ intel_dp_init(struct drm_device *dev,
 	}
 
 	intel_dig_port->port = port;
-	dev_priv->dig_port_map[port] = intel_encoder;
 	intel_dig_port->dp.output_reg = output_reg;
 
 	intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 4a77639a489d..23ee48dc765f 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2146,7 +2146,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 void intel_hdmi_init(struct drm_device *dev,
 		     i915_reg_t hdmi_reg, enum port port)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_digital_port *intel_dig_port;
 	struct intel_encoder *intel_encoder;
 	struct intel_connector *intel_connector;
@@ -2215,7 +2214,6 @@ void intel_hdmi_init(struct drm_device *dev,
 		intel_encoder->cloneable |= 1 << INTEL_OUTPUT_HDMI;
 
 	intel_dig_port->port = port;
-	dev_priv->dig_port_map[port] = intel_encoder;
 	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
 	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
 

[toc] | [prev] | [next] | [standalone]

#1341690 — Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915

On Wed, 24 Feb 2016 08:51:32 +0100,
Takashi Iwai wrote:
> 
> Since dig_port_map[] is used only for the audio callback, we can
> assign it dynamically just before the callbacks.
> 
> Could you try the patch below?  (It's totally untested.)

The one below might be safer, it's setting in the mutex lock.
Please give this a try instead.


Takashi

---
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 31f6d212fb1b..30f921421b0c 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -527,6 +527,8 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
 
 	mutex_lock(&dev_priv->av_mutex);
 	intel_dig_port->audio_connector = connector;
+	/* referred in audio callbacks */
+	dev_priv->dig_port_map[port] = intel_encoder;
 	mutex_unlock(&dev_priv->av_mutex);
 
 	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
@@ -554,6 +556,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
 
 	mutex_lock(&dev_priv->av_mutex);
 	intel_dig_port->audio_connector = NULL;
+	dev_priv->dig_port_map[port] = NULL;
 	mutex_unlock(&dev_priv->av_mutex);
 
 	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6408e5583d7..63ba42aa2985 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3311,7 +3311,6 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 	intel_encoder->get_config = intel_ddi_get_config;
 
 	intel_dig_port->port = port;
-	dev_priv->dig_port_map[port] = intel_encoder;
 	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
 					  (DDI_BUF_PORT_REVERSAL |
 					   DDI_A_4_LANES);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d313cb9..ac6a37cbd323 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6035,7 +6035,6 @@ intel_dp_init(struct drm_device *dev,
 	}
 
 	intel_dig_port->port = port;
-	dev_priv->dig_port_map[port] = intel_encoder;
 	intel_dig_port->dp.output_reg = output_reg;
 
 	intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 4a77639a489d..23ee48dc765f 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2146,7 +2146,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 void intel_hdmi_init(struct drm_device *dev,
 		     i915_reg_t hdmi_reg, enum port port)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_digital_port *intel_dig_port;
 	struct intel_encoder *intel_encoder;
 	struct intel_connector *intel_connector;
@@ -2215,7 +2214,6 @@ void intel_hdmi_init(struct drm_device *dev,
 		intel_encoder->cloneable |= 1 << INTEL_OUTPUT_HDMI;
 
 	intel_dig_port->port = port;
-	dev_priv->dig_port_map[port] = intel_encoder;
 	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
 	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
 

[toc] | [prev] | [next] | [standalone]

#1341939 — Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915

Am 2016-02-24 um 10:13 schrieb Takashi Iwai:
> On Wed, 24 Feb 2016 08:51:32 +0100,
> Takashi Iwai wrote:
>>
>> Since dig_port_map[] is used only for the audio callback, we can
>> assign it dynamically just before the callbacks.
>>
>> Could you try the patch below?  (It's totally untested.)

now it's tested :) this works! Also sounds like a reasonable fix for
now, doesn't it?

thanks
                       martin

ps. sorry I couldn't send to intel-gfx@lists.freedesktop.org it rejected
my mail.

> 
> The one below might be safer, it's setting in the mutex lock.
> Please give this a try instead.
> 
> 
> Takashi
> 
> ---
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 31f6d212fb1b..30f921421b0c 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -527,6 +527,8 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  	intel_dig_port->audio_connector = connector;
> +	/* referred in audio callbacks */
> +	dev_priv->dig_port_map[port] = intel_encoder;
>  	mutex_unlock(&dev_priv->av_mutex);
>  
>  	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
> @@ -554,6 +556,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  	intel_dig_port->audio_connector = NULL;
> +	dev_priv->dig_port_map[port] = NULL;
>  	mutex_unlock(&dev_priv->av_mutex);
>  
>  	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e6408e5583d7..63ba42aa2985 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3311,7 +3311,6 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  	intel_encoder->get_config = intel_ddi_get_config;
>  
>  	intel_dig_port->port = port;
> -	dev_priv->dig_port_map[port] = intel_encoder;
>  	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
>  					  (DDI_BUF_PORT_REVERSAL |
>  					   DDI_A_4_LANES);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 796e3d313cb9..ac6a37cbd323 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6035,7 +6035,6 @@ intel_dp_init(struct drm_device *dev,
>  	}
>  
>  	intel_dig_port->port = port;
> -	dev_priv->dig_port_map[port] = intel_encoder;
>  	intel_dig_port->dp.output_reg = output_reg;
>  
>  	intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4a77639a489d..23ee48dc765f 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2146,7 +2146,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  void intel_hdmi_init(struct drm_device *dev,
>  		     i915_reg_t hdmi_reg, enum port port)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_digital_port *intel_dig_port;
>  	struct intel_encoder *intel_encoder;
>  	struct intel_connector *intel_connector;
> @@ -2215,7 +2214,6 @@ void intel_hdmi_init(struct drm_device *dev,
>  		intel_encoder->cloneable |= 1 << INTEL_OUTPUT_HDMI;
>  
>  	intel_dig_port->port = port;
> -	dev_priv->dig_port_map[port] = intel_encoder;
>  	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
>  	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
>  
> 

[toc] | [prev] | [next] | [standalone]

#1342071 — Re: [Intel-gfx] [BUG] [REGRESSION] [BISECTED] -rc1 breaks audio over HDMI for i915

On Wed, 24 Feb 2016 13:18:43 +0100,
Martin Kepplinger wrote:
> 
> Am 2016-02-24 um 10:13 schrieb Takashi Iwai:
> > On Wed, 24 Feb 2016 08:51:32 +0100,
> > Takashi Iwai wrote:
> >>
> >> Since dig_port_map[] is used only for the audio callback, we can
> >> assign it dynamically just before the callbacks.
> >>
> >> Could you try the patch below?  (It's totally untested.)
> 
> now it's tested :) this works! Also sounds like a reasonable fix for
> now, doesn't it?

Good to hear!

> thanks
>                        martin
> 
> ps. sorry I couldn't send to intel-gfx@lists.freedesktop.org it rejected
> my mail.

I thought intel-gfx ML has manual approval for non-subscribers.  But
don't worry, I'm going to submit the proper patch with Cc'ed you.


thanks,

Takashi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web