| Message ID | 1347506340-2683-1-git-send-email-xingchao.wang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
At Thu, 13 Sep 2012 11:19:00 +0800, Wang Xingchao wrote: > > Clear Audio Enable bit to trigger unsolicated event to notify Audio > Driver part the HDMI hot plug change. The patch fixed the bug when > remove HDMI cable the bit was not cleared correctly. > > In intel_enable_hdmi(), if intel_hdmi->has_audio been true, the "Audio enable bit" will > be set to trigger unsolicated event to notify Alsa driver the change. > > intel_hdmi->has_audio will be reset to false from intel_hdmi_detect() after > remove the hdmi cable, here's debug log: > > [ 187.494153] [drm:output_poll_execute], [CONNECTOR:17:HDMI-A-1] status updated from 1 to 2 > [ 187.525349] [drm:intel_hdmi_detect], HDMI: has_audio = 0 > > so when comes back to intel_disable_hdmi(), the "Audio enable bit" will not be cleared. And this > cause the eld infomation and pin presence doesnot update accordingly in alsa driver side. > > This patch will also trigger unsolicated event to alsa driver to notify the hot plug event: > > [ 187.853159] ALSA sound/pci/hda/patch_hdmi.c:772 HDMI hot plug event: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=1 > [ 187.853268] ALSA sound/pci/hda/patch_hdmi.c:990 HDMI status: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=0 > > Signed-off-by: Wang Xingchao <xingchao.wang@intel.com> We need the same fix for stable kernels, too, but unfortunately this can't be applied to 3.6 or earlier. Greg prefers still having a Cc to stable even in such a case, and then send a separate patch applicable to the existing kernels. http://lwn.net/Articles/515528/ So better to have a Cc to stable in the patch. thanks, Takashi > --- > drivers/gpu/drm/i915/intel_hdmi.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index 5d02aad..229897f 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -674,10 +674,7 @@ static void intel_disable_hdmi(struct intel_encoder *encoder) > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); > u32 temp; > - u32 enable_bits = SDVO_ENABLE; > - > - if (intel_hdmi->has_audio) > - enable_bits |= SDVO_AUDIO_ENABLE; > + u32 enable_bits = SDVO_ENABLE | SDVO_AUDIO_ENABLE; > > temp = I915_READ(intel_hdmi->sdvox_reg); > > -- > 1.7.9.5 > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
Hi Takashi, 2012/9/13 Takashi Iwai <tiwai@suse.de>: > At Thu, 13 Sep 2012 11:19:00 +0800, > Wang Xingchao wrote: >> >> Clear Audio Enable bit to trigger unsolicated event to notify Audio >> Driver part the HDMI hot plug change. The patch fixed the bug when >> remove HDMI cable the bit was not cleared correctly. >> >> In intel_enable_hdmi(), if intel_hdmi->has_audio been true, the "Audio enable bit" will >> be set to trigger unsolicated event to notify Alsa driver the change. >> >> intel_hdmi->has_audio will be reset to false from intel_hdmi_detect() after >> remove the hdmi cable, here's debug log: >> >> [ 187.494153] [drm:output_poll_execute], [CONNECTOR:17:HDMI-A-1] status updated from 1 to 2 >> [ 187.525349] [drm:intel_hdmi_detect], HDMI: has_audio = 0 >> >> so when comes back to intel_disable_hdmi(), the "Audio enable bit" will not be cleared. And this >> cause the eld infomation and pin presence doesnot update accordingly in alsa driver side. >> >> This patch will also trigger unsolicated event to alsa driver to notify the hot plug event: >> >> [ 187.853159] ALSA sound/pci/hda/patch_hdmi.c:772 HDMI hot plug event: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=1 >> [ 187.853268] ALSA sound/pci/hda/patch_hdmi.c:990 HDMI status: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=0 >> >> Signed-off-by: Wang Xingchao <xingchao.wang@intel.com> > > We need the same fix for stable kernels, too, but unfortunately this > can't be applied to 3.6 or earlier. I had another patch against 3.6 kernel, I send it out at first. This v2 version is based on Daniel's drm-intel-next branch. so i will resend that seperate patch to Greg. The patch for 3.6 kernel: http://lists.freedesktop.org/archives/intel-gfx/2012-September/020413.html thanks --xingchao > > Greg prefers still having a Cc to stable even in such a case, and then > send a separate patch applicable to the existing kernels. > http://lwn.net/Articles/515528/ > So better to have a Cc to stable in the patch. > > > thanks, > > Takashi > >> --- >> drivers/gpu/drm/i915/intel_hdmi.c | 5 +---- >> 1 file changed, 1 insertion(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c >> index 5d02aad..229897f 100644 >> --- a/drivers/gpu/drm/i915/intel_hdmi.c >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c >> @@ -674,10 +674,7 @@ static void intel_disable_hdmi(struct intel_encoder *encoder) >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); >> u32 temp; >> - u32 enable_bits = SDVO_ENABLE; >> - >> - if (intel_hdmi->has_audio) >> - enable_bits |= SDVO_AUDIO_ENABLE; >> + u32 enable_bits = SDVO_ENABLE | SDVO_AUDIO_ENABLE; >> >> temp = I915_READ(intel_hdmi->sdvox_reg); >> >> -- >> 1.7.9.5 >> >> _______________________________________________ >> Alsa-devel mailing list >> Alsa-devel@alsa-project.org >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >> > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
At Thu, 13 Sep 2012 21:53:12 +0800, Wang Xingchao wrote: > > Hi Takashi, > > 2012/9/13 Takashi Iwai <tiwai@suse.de>: > > At Thu, 13 Sep 2012 11:19:00 +0800, > > Wang Xingchao wrote: > >> > >> Clear Audio Enable bit to trigger unsolicated event to notify Audio > >> Driver part the HDMI hot plug change. The patch fixed the bug when > >> remove HDMI cable the bit was not cleared correctly. > >> > >> In intel_enable_hdmi(), if intel_hdmi->has_audio been true, the "Audio enable bit" will > >> be set to trigger unsolicated event to notify Alsa driver the change. > >> > >> intel_hdmi->has_audio will be reset to false from intel_hdmi_detect() after > >> remove the hdmi cable, here's debug log: > >> > >> [ 187.494153] [drm:output_poll_execute], [CONNECTOR:17:HDMI-A-1] status updated from 1 to 2 > >> [ 187.525349] [drm:intel_hdmi_detect], HDMI: has_audio = 0 > >> > >> so when comes back to intel_disable_hdmi(), the "Audio enable bit" will not be cleared. And this > >> cause the eld infomation and pin presence doesnot update accordingly in alsa driver side. > >> > >> This patch will also trigger unsolicated event to alsa driver to notify the hot plug event: > >> > >> [ 187.853159] ALSA sound/pci/hda/patch_hdmi.c:772 HDMI hot plug event: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=1 > >> [ 187.853268] ALSA sound/pci/hda/patch_hdmi.c:990 HDMI status: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=0 > >> > >> Signed-off-by: Wang Xingchao <xingchao.wang@intel.com> > > > > We need the same fix for stable kernels, too, but unfortunately this > > can't be applied to 3.6 or earlier. > > I had another patch against 3.6 kernel, I send it out at first. This > v2 version is based on Daniel's drm-intel-next branch. I know. But my point is to put a Cc line into the patch itself. > so i will resend that seperate patch to Greg. FYI, sending a separate stable patch would be better done once after the original patch gets merged to Linus tree and Greg pings you due to a cherry-pick failure. As a rule of thumb, all stable patches must be merged to Linus tree at first. But, this makes me wonder why this patch can't go into 3.6. It's a clear bug and the fix is trivial. Daniel, isn't it better to pick the fix (for 3.6) to drm-intel-fixes, then merge back to drm-intel-next? thanks, Takashi > > The patch for 3.6 kernel: > http://lists.freedesktop.org/archives/intel-gfx/2012-September/020413.html > > thanks > --xingchao > > > > Greg prefers still having a Cc to stable even in such a case, and then > > send a separate patch applicable to the existing kernels. > > http://lwn.net/Articles/515528/ > > So better to have a Cc to stable in the patch. > > > > > > thanks, > > > > Takashi > > > >> --- > >> drivers/gpu/drm/i915/intel_hdmi.c | 5 +---- > >> 1 file changed, 1 insertion(+), 4 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > >> index 5d02aad..229897f 100644 > >> --- a/drivers/gpu/drm/i915/intel_hdmi.c > >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c > >> @@ -674,10 +674,7 @@ static void intel_disable_hdmi(struct intel_encoder *encoder) > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); > >> u32 temp; > >> - u32 enable_bits = SDVO_ENABLE; > >> - > >> - if (intel_hdmi->has_audio) > >> - enable_bits |= SDVO_AUDIO_ENABLE; > >> + u32 enable_bits = SDVO_ENABLE | SDVO_AUDIO_ENABLE; > >> > >> temp = I915_READ(intel_hdmi->sdvox_reg); > >> > >> -- > >> 1.7.9.5 > >> > >> _______________________________________________ > >> Alsa-devel mailing list > >> Alsa-devel@alsa-project.org > >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > >> > > _______________________________________________ > > Alsa-devel mailing list > > Alsa-devel@alsa-project.org > > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
On Thu, Sep 13, 2012 at 4:00 PM, Takashi Iwai <tiwai@suse.de> wrote: > But, this makes me wonder why this patch can't go into 3.6. It's a > clear bug and the fix is trivial. > > Daniel, isn't it better to pick the fix (for 3.6) to drm-intel-fixes, > then merge back to drm-intel-next? Well, on a quick read-through it didn't sound serious (just a bit of noise in dmesg and a few spurious reconfigs), and we're rather late in the -rc cycle. Applying this on top of -fixes will cause a conflict, so didn't looked like I should bother. Also, I _want_ the patch for -next so that git blame shows the commit and not some random conflicting merge. -Daniel
At Thu, 13 Sep 2012 16:18:08 +0200, Daniel Vetter wrote: > > On Thu, Sep 13, 2012 at 4:00 PM, Takashi Iwai <tiwai@suse.de> wrote: > > But, this makes me wonder why this patch can't go into 3.6. It's a > > clear bug and the fix is trivial. > > > > Daniel, isn't it better to pick the fix (for 3.6) to drm-intel-fixes, > > then merge back to drm-intel-next? > > Well, on a quick read-through it didn't sound serious (just a bit of > noise in dmesg and a few spurious reconfigs), For the graphics side, yes. But for the sound driver side, you'll miss the jack-detection unplug event without this fix. > and we're rather late in > the -rc cycle. Yes, but a bug is a bug is a bug :) And the fix is two-liners. If there were no conflict, would you mind it? I don't think so. > Applying this on top of -fixes will cause a conflict, so didn't looked > like I should bother. Yeah, that's bad. > Also, I _want_ the patch for -next so that git > blame shows the commit and not some random conflicting merge. With parallel development, you can't always get straight lines, as you know well... Takashi
On Thu, Sep 13, 2012 at 08:51:22AM +0200, Takashi Iwai wrote: > At Thu, 13 Sep 2012 11:19:00 +0800, > Wang Xingchao wrote: > > > > Clear Audio Enable bit to trigger unsolicated event to notify Audio > > Driver part the HDMI hot plug change. The patch fixed the bug when > > remove HDMI cable the bit was not cleared correctly. > > > > In intel_enable_hdmi(), if intel_hdmi->has_audio been true, the "Audio enable bit" will > > be set to trigger unsolicated event to notify Alsa driver the change. > > > > intel_hdmi->has_audio will be reset to false from intel_hdmi_detect() after > > remove the hdmi cable, here's debug log: > > > > [ 187.494153] [drm:output_poll_execute], [CONNECTOR:17:HDMI-A-1] status updated from 1 to 2 > > [ 187.525349] [drm:intel_hdmi_detect], HDMI: has_audio = 0 > > > > so when comes back to intel_disable_hdmi(), the "Audio enable bit" will not be cleared. And this > > cause the eld infomation and pin presence doesnot update accordingly in alsa driver side. > > > > This patch will also trigger unsolicated event to alsa driver to notify the hot plug event: > > > > [ 187.853159] ALSA sound/pci/hda/patch_hdmi.c:772 HDMI hot plug event: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=1 > > [ 187.853268] ALSA sound/pci/hda/patch_hdmi.c:990 HDMI status: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=0 > > > > Signed-off-by: Wang Xingchao <xingchao.wang@intel.com> > > We need the same fix for stable kernels, too, but unfortunately this > can't be applied to 3.6 or earlier. > > Greg prefers still having a Cc to stable even in such a case, and then > send a separate patch applicable to the existing kernels. > http://lwn.net/Articles/515528/ > So better to have a Cc to stable in the patch. I've merged both this patch for -next and the previous patch for -fixes (with cc: stable added for -fixes). Linus/Dave will have some fun with the conflict ;-) Thanks, Daniel > > > thanks, > > Takashi > > > --- > > drivers/gpu/drm/i915/intel_hdmi.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > > index 5d02aad..229897f 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -674,10 +674,7 @@ static void intel_disable_hdmi(struct intel_encoder *encoder) > > struct drm_i915_private *dev_priv = dev->dev_private; > > struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); > > u32 temp; > > - u32 enable_bits = SDVO_ENABLE; > > - > > - if (intel_hdmi->has_audio) > > - enable_bits |= SDVO_AUDIO_ENABLE; > > + u32 enable_bits = SDVO_ENABLE | SDVO_AUDIO_ENABLE; > > > > temp = I915_READ(intel_hdmi->sdvox_reg); > > > > -- > > 1.7.9.5 > > > > _______________________________________________ > > Alsa-devel mailing list > > Alsa-devel@alsa-project.org > > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > >
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 5d02aad..229897f 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -674,10 +674,7 @@ static void intel_disable_hdmi(struct intel_encoder *encoder) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); u32 temp; - u32 enable_bits = SDVO_ENABLE; - - if (intel_hdmi->has_audio) - enable_bits |= SDVO_AUDIO_ENABLE; + u32 enable_bits = SDVO_ENABLE | SDVO_AUDIO_ENABLE; temp = I915_READ(intel_hdmi->sdvox_reg);
Clear Audio Enable bit to trigger unsolicated event to notify Audio Driver part the HDMI hot plug change. The patch fixed the bug when remove HDMI cable the bit was not cleared correctly. In intel_enable_hdmi(), if intel_hdmi->has_audio been true, the "Audio enable bit" will be set to trigger unsolicated event to notify Alsa driver the change. intel_hdmi->has_audio will be reset to false from intel_hdmi_detect() after remove the hdmi cable, here's debug log: [ 187.494153] [drm:output_poll_execute], [CONNECTOR:17:HDMI-A-1] status updated from 1 to 2 [ 187.525349] [drm:intel_hdmi_detect], HDMI: has_audio = 0 so when comes back to intel_disable_hdmi(), the "Audio enable bit" will not be cleared. And this cause the eld infomation and pin presence doesnot update accordingly in alsa driver side. This patch will also trigger unsolicated event to alsa driver to notify the hot plug event: [ 187.853159] ALSA sound/pci/hda/patch_hdmi.c:772 HDMI hot plug event: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=1 [ 187.853268] ALSA sound/pci/hda/patch_hdmi.c:990 HDMI status: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=0 Signed-off-by: Wang Xingchao <xingchao.wang@intel.com> --- drivers/gpu/drm/i915/intel_hdmi.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)