Re: [Intel-gfx] [PATCH] drm/i915: HDMI - Clear Audio Enable bit for Hot Plug unconditionally
-Original Message- From: Wu, Fengguang Sent: Thursday, September 13, 2012 11:14 AM To: Wang, Xingchao Cc: intel-gfx@lists.freedesktop.org; alsa-de...@alsa-project.org; daniel.vet...@ffwll.ch; ti...@suse.de; Zhao, Yakui; Fu, Michael Subject: Re: [PATCH] drm/i915: HDMI - Clear Audio Enable bit for Hot Plug unconditionally u32 enable_bits = SDVO_ENABLE; - if (intel_hdmi-has_audio) - enable_bits |= SDVO_AUDIO_ENABLE; + enable_bits |= SDVO_AUDIO_ENABLE; The two lines can be combined: u32 enable_bits = SDVO_ENABLE | SDVO_AUDIO_ENABLE; Looks better, I send out V2 patch for Daniel. -- Xingchao ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: optionally check GTT checksum across suspend/resume
On Wed, 12 Sep 2012 13:27:10 -0700 Jesse Barnes jbar...@virtuousgeek.org wrote: This adds a module parameter, gtt_suspend_verify, that will enable a simple GTT checksum across suspend and resume. We currently spend a good amount of time (20ms) clearing all the GTT PTEs at resume time, even though this may not be necessary on some machines. This debug feature is intended to help determine whether newer machines don't need the unconditional GTT clear on resume. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org It's a cool idea, but I'd vote against adding a module parameter to verify it. Daniel's trees should be well enough tested that you could just disable it on IVB+ or whatever and let it go through QA and developer testing. Aside from that, a debugfs flag to toggle it would probably be a little better. Also, as a bikeshed you could probably get a much better detection with a CRC or something similar. I dunno, just a thought, -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH v2] drm/i915: HDMI - Clear Audio Enable bit for Hot Plug unconditionally
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.w...@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-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [QA 09/13] Testing report for `drm-intel-testing` (was: Updated -next) on
Summary We finished a new round of kernel testing. Generally, in this circle,7 bugs are closed, 14 bugs are still open. Test Environment The Linux Kernel, the operating system core itself Kernel: (drm-intel-testing)c3c3d4e9c2daeca01c42040cda0e5e0579c5c80b Some additional commit info: Merge: e04190e 99d0b1d Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Sun Sep 9 12:07:07 2012 +0200 Hardware We covered the platform of IvyBridge, SandyBridge, IronLake,G45 and Pineview Findings closed bugs Bug 51490 - [SNB ILK] Some modes can’t work normally on VGA display Bug 51491 - [SNB] 1024x768 86Hz mode can’t work normally on HDMI display Bug 51499 - [SNB regression] Some modes can’t work normally on DVI port Bug 53577 - [IVB]I-G-T/prime_api failed Bug 53578 - [IVB]I-G-T/prime_api failed Bug 53579 - [IVB]I-G-T/prime_test failed Bug 54411 - [IVB]I-G-T/gem_tiled_swapping failed opened bugs Bug 36997 - [G45] TV can't display after loading drm driver Bug 41976 - [IVB] screen turn to be black while switching between console and x-window with 3-pipe active Bug 42194 - [IVB/SNB] coldplug new monitors for fbcon on lastclose() Bug 45729 - [bisected regression] Incorrect Mode Timing on DP Display, with 3.3-rc (due to interlaced CEA modes) Bug 47034 - [G45] mode set failure with testdisplay, low resolution modes fail and display doesn't come back Bug 50069 - [IVB]I-G-T/flip_test never finish Bug 50823 - [IVB]EDP can't show all modes correctly Bug 51055 - [IVB]I-G-T/gem_tiled_blits fails while running two or more times Bug 51493 - [ILK] Prefer mode(2560x1600) can’t light up for the second time with DP external display connected Bug 51677 - [IVB] Running module_reload after calling function drm_open_any() will cause GPU hang Bug 51975 - [IVB]can't find the HDMI audio device Bug 54111 - [IVB]I-G-T/module_reload fail with *ERROR* “Memory manager not clean. Delaying takedown” Bug 54253 - [SNB]eDP can't work while booting with miniVGA Bug 54410 - [IVB regression]I-G-T/gem_tiled_pread_pwrite failed Thanks --Sun, Yi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: rip out early dp port write for gm45/ilk
On Wed, 12 Sep 2012 23:24:09 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: It's bogus. If I've followed the history of this piece of code correctly, i.e. the initial register write with the following vblank wait, this goes all the way back to the original enabling of DP support in commit a4fc5ed69817c73e32571ad7837bb707f9890009 Author: Keith Packard kei...@keithp.com Date: Tue Apr 7 16:16:42 2009 -0700 drm/i915: Add Display Port support Unfortunately it seems to be nothing more than glorified duct-tape and sometimes actively harmful. Adam Jackson noticed this for CPT platforms with commit e85194641bec56179dcf5e1704ce5c6bf30340c6 Author: Adam Jackson a...@redhat.com Date: Thu Jul 21 17:48:38 2011 -0400 drm/i915/dp: Don't turn CPT DP ports on too early Unfortunately this kept the code around for ilk and gm45. The specific failure case I'm seeing here is that after a dpms off/on cycle we have the bits from the last link training (hopefully successful link training) set in intel_dp-DP. This is requiered so that complete_link_train can enable the port with the right tuning values. Unfortunately writing these again to the disabled port at dpms on time kills the port somehow until it's disabled - dp link training fails in an endless loop without this patch on my mobile ilk and gm45. Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch Works for me, only time will time if I no longer get the occasional failure in link training, but it definitely fixes the issue with dpms off. Tested-by: Chris Wilson ch...@chris-wilson.co.uk Probably https://bugs.freedesktop.org/show_bug.cgi?id=51493 -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] Stereo 3D modes support
On Wed, 12 Sep 2012 18:47:12 +0100, Damien Lespiau damien.lesp...@gmail.com wrote: Hi, This series introduces stereo 3D modes support and is split in 3 chunks: 1. 3 kernel patches to parse the 3D_present flag of the HDMI CEA vendor block, to expose 3D formats flags in modes and to add a new property on connectors supporting stereo 3D, I'm not convinced by the approach of using a stereo mode flag as a connector property for the simple reason that property has different lifetimes to the associated mode and userspace needs some form of synchronisation in order to be able to switch stereo modes on the fly without corruption. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: rip out early dp port write for gm45/ilk
On Thu, Sep 13, 2012 at 11:10:14AM +0100, Chris Wilson wrote: On Wed, 12 Sep 2012 23:24:09 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: It's bogus. If I've followed the history of this piece of code correctly, i.e. the initial register write with the following vblank wait, this goes all the way back to the original enabling of DP support in commit a4fc5ed69817c73e32571ad7837bb707f9890009 Author: Keith Packard kei...@keithp.com Date: Tue Apr 7 16:16:42 2009 -0700 drm/i915: Add Display Port support Unfortunately it seems to be nothing more than glorified duct-tape and sometimes actively harmful. Adam Jackson noticed this for CPT platforms with commit e85194641bec56179dcf5e1704ce5c6bf30340c6 Author: Adam Jackson a...@redhat.com Date: Thu Jul 21 17:48:38 2011 -0400 drm/i915/dp: Don't turn CPT DP ports on too early Unfortunately this kept the code around for ilk and gm45. The specific failure case I'm seeing here is that after a dpms off/on cycle we have the bits from the last link training (hopefully successful link training) set in intel_dp-DP. This is requiered so that complete_link_train can enable the port with the right tuning values. Unfortunately writing these again to the disabled port at dpms on time kills the port somehow until it's disabled - dp link training fails in an endless loop without this patch on my mobile ilk and gm45. Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch Works for me, only time will time if I no longer get the occasional failure in link training, but it definitely fixes the issue with dpms off. Tested-by: Chris Wilson ch...@chris-wilson.co.uk Probably https://bugs.freedesktop.org/show_bug.cgi?id=51493 Queued for -next, thanks for testing - imo this patch is too risky for -fixes at this point in the release cycle. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: rip out pre-production ilk cpu edp w/a
While reading docs I've noticed that this special workaround to select
the 1.6 GHz DP clock only applies to pre-production ilk machines.
Since the registers we're touching here are rather undocumented and
might be harmful on later chips, rip it out.
For the Bspec reference of this w/a look in vol4g CPU Display
Registers [DevILK], Section 4.1.7.1 DP_A—DisplayPort A
Control Register, DP_PLL_Frequency_Select.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_dp.c | 22 +++---
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 376ae28..e2794ca 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -818,27 +818,11 @@ static void ironlake_set_pll_edp(struct drm_crtc *crtc,
int clock)
dpa_ctl = I915_READ(DP_A);
dpa_ctl = ~DP_PLL_FREQ_MASK;
- if (clock 20) {
- u32 temp;
+ if (clock 20)
dpa_ctl |= DP_PLL_FREQ_160MHZ;
- /* workaround for 160Mhz:
- 1) program 0x4600c bits 15:0 = 0x8124
- 2) program 0x46010 bit 0 = 1
- 3) program 0x46034 bit 24 = 1
- 4) program 0x64000 bit 14 = 1
- */
- temp = I915_READ(0x4600c);
- temp = 0x;
- I915_WRITE(0x4600c, temp | 0x8124);
-
- temp = I915_READ(0x46010);
- I915_WRITE(0x46010, temp | 1);
-
- temp = I915_READ(0x46034);
- I915_WRITE(0x46034, temp | (1 24));
- } else {
+ else
dpa_ctl |= DP_PLL_FREQ_270MHZ;
- }
+
I915_WRITE(DP_A, dpa_ctl);
POSTING_READ(DP_A);
--
1.7.11.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: rip out pre-production ilk cpu edp w/a
On Thu, 13 Sep 2012 13:13:29 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: While reading docs I've noticed that this special workaround to select the 1.6 GHz DP clock only applies to pre-production ilk machines. Since the registers we're touching here are rather undocumented and might be harmful on later chips, rip it out. For the Bspec reference of this w/a look in vol4g CPU Display Registers [DevILK], Section 4.1.7.1 DP_AâDisplayPort A Control Register, DP_PLL_Frequency_Select. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch The spec definitely says DevILK-A, are we sure it got excised in production machines? Once a w/a is implemented by the Windows driver they have a tendency to linger... So can you instead replace the w/a with a message to remind us about the possibilty of failure of the PLL to change frequencies. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] Stereo 3D modes support
On Thu, Sep 13, 2012 at 12:37 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, 12 Sep 2012 18:47:12 +0100, Damien Lespiau damien.lesp...@gmail.com wrote: Hi, This series introduces stereo 3D modes support and is split in 3 chunks: 1. 3 kernel patches to parse the 3D_present flag of the HDMI CEA vendor block, to expose 3D formats flags in modes and to add a new property on connectors supporting stereo 3D, I'm not convinced by the approach of using a stereo mode flag as a connector property for the simple reason that property has different lifetimes to the associated mode and userspace needs some form of synchronisation in order to be able to switch stereo modes on the fly without corruption. That's something that makes me feel bit uneasy as well, it seems that the stereo format is really something associated with a mode. Note that the set_property() only stores the new value and it's taken into account the next time a mode is set. I'm not quite sure how to improve on this. The alternatives I can think about would break the current user space. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH v2] drm/i915: HDMI - Clear Audio Enable bit for Hot Plug unconditionally
Hi Takashi, 2012/9/13 Takashi Iwai ti...@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.w...@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-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH v2] drm/i915: HDMI - Clear Audio Enable bit for Hot Plug unconditionally
At Thu, 13 Sep 2012 21:53:12 +0800, Wang Xingchao wrote: Hi Takashi, 2012/9/13 Takashi Iwai ti...@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.w...@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-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH v2] drm/i915: HDMI - Clear Audio Enable bit for Hot Plug unconditionally
On Thu, Sep 13, 2012 at 4:00 PM, Takashi Iwai ti...@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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [alsa-devel] [PATCH v2] drm/i915: HDMI - Clear Audio Enable bit for Hot Plug unconditionally
At Thu, 13 Sep 2012 16:18:08 +0200, Daniel Vetter wrote: On Thu, Sep 13, 2012 at 4:00 PM, Takashi Iwai ti...@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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: optionally check GTT checksum across suspend/resume
Ben Widawsky b...@bwidawsk.net writes: Also, as a bikeshed you could probably get a much better detection with a CRC or something similar. Wonder if you could use the existing MD5 code in the kernel; would avoid all sorts of unfortunate false-positives. -- keith.pack...@intel.com pgpOfytF7Loyb.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: rip out pre-production ilk cpu edp w/a
While reading docs I've noticed that this special workaround to select
the 1.6 GHz DP clock only applies to pre-production ilk machines.
Since the registers we're touching here are rather undocumented and
might be harmful on later chips, rip it out.
For the Bspec reference of this w/a look in vol4g CPU Display
Registers [DevILK], Section 4.1.7.1 DP_A—DisplayPort A
Control Register, DP_PLL_Frequency_Select.
v2: Keep a debug message as a hint in case something regresses.
Requested by Chris Wilson.
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_dp.c | 21 +
1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 376ae28..1539c8c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -819,26 +819,15 @@ static void ironlake_set_pll_edp(struct drm_crtc *crtc,
int clock)
dpa_ctl = ~DP_PLL_FREQ_MASK;
if (clock 20) {
- u32 temp;
+ /* For a long time we've carried around a ILK-DevA w/a for the
+* 160MHz clock. If we're really unlucky, it's still required.
+*/
+ DRM_DEBUG_KMS(160MHz cpu eDP clock, might need ilk devA
w/a\n);
dpa_ctl |= DP_PLL_FREQ_160MHZ;
- /* workaround for 160Mhz:
- 1) program 0x4600c bits 15:0 = 0x8124
- 2) program 0x46010 bit 0 = 1
- 3) program 0x46034 bit 24 = 1
- 4) program 0x64000 bit 14 = 1
- */
- temp = I915_READ(0x4600c);
- temp = 0x;
- I915_WRITE(0x4600c, temp | 0x8124);
-
- temp = I915_READ(0x46010);
- I915_WRITE(0x46010, temp | 1);
-
- temp = I915_READ(0x46034);
- I915_WRITE(0x46034, temp | (1 24));
} else {
dpa_ctl |= DP_PLL_FREQ_270MHZ;
}
+
I915_WRITE(DP_A, dpa_ctl);
POSTING_READ(DP_A);
--
1.7.11.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915: clean up the cpu edp pll special case
On Wed, Sep 12, 2012 at 06:00:58PM -0300, Paulo Zanoni wrote: Hi 2012/9/6 Daniel Vetter daniel.vet...@ffwll.ch: By using the new pre_enabel/post_disable functions. s/enabel/enable To ensure that we only frob the cpu edp pll while the pipe is off add the relevant asserts. Thanks to the new output state staging, this is now really easy. This patch does more than it says. It creates the new pre/post enable/disable functions, but it also replaces the dpms connector function with the default intel_connector_dpms (because after removing the edp enable/disable code from the dpms function, it will look almost exactly like intel_connector_dpms). Ideally this should be 2 patches: first do the pre/post enable/disable dance, then switch the specialized dpms with the generic one. The main reason to split this in 2 patches is to make it easier to reviewers understand what's going on, so they can review faster without trying to discover why you switched the dpms function. But since you've already got a reviewer, you should at least write about the dpms change in the commit message. With the typo fixed and at least a small sentence on the commit message explaining the replacement of intel_dp_dpms with intel_connector_dpms: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com I've added a paragraph to explain why we can now simplify the dpms code, too. Thanks for the review, patches 12 applied to dinq. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915: robustify edp_pll_on/off
Hi
2012/9/6 Daniel Vetter daniel.vet...@ffwll.ch:
With the previous patch to clean up where exactly these two functions
are getting called, this patch can tackle the enable/disable code
itself:
This is the kind of patch that's hard to proof-read. Basically, IMHO,
the intel_dp-DP variable makes things harder to understand and
review. Sometimes we use intel_dp-DP directly, sometimes we do int
DP = intel_dp-DP and then just change the temporary DP (sometimes
not updating the original intel_dp-DP after changing the temporary
DP), sometimes we read/write directly from intel_dp-output_reg,
sometimes we pass int DP as a function argument instead of storing
the value inside intel_dp-DP. I have to admit that even after working
some time with this code I'm not exactly sure why intel_dp-DP is
really necessary. One of my plans was to try to write a patch that
removes the variable so I could at least maybe understand why it's
necessary (the plan was to read/write from/to intel_dp-output_reg
directly). You can do that if you want :)
Complaints aside, here are my comments:
- WARN if the port enable bit is in the wrong state or if the edp pll
bit is in the wrong state, just for paranoia's sake.
Looks good.
- Don't disable the edp pll harder in the modeset functions just for
fun.
Which part of the patch is this message about? Did you mean
intel_dp_link_down instead of modeset? I could not find anything on
our documentation saying that the PLL must be disabled when retraining
the link, so your patch looks correct, but this is the kind of thing
that we should really try to test somehow :)
- Don't set the edp pll enable flag in intel_dp-DP in modeset, do
that while changing the actual hw state. We do the same with the
actual port enable bit, so this is a bit more consistent.
Looks good.
- Track the current DP register value when setting things up and add
some comments how intel_dp-DP is used in the disable code.
v2: Be more careful with resetting intel_dp-DP - otherwise dpms
off-on will fail spectacularly, becuase we enable the eDP port when
we should only enable the eDP pll.
These 2 chunks are the ones my comment above is about. I'll just trust
our beloved maintainer on these chunks :)
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_dp.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c72d4f3..7c746d1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -887,7 +887,6 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct
drm_display_mode *mode,
intel_dp-DP |= intel_crtc-pipe 29;
/* don't miss out required setting for eDP */
- intel_dp-DP |= DP_PLL_ENABLE;
if (adjusted_mode-clock 20)
intel_dp-DP |= DP_PLL_FREQ_160MHZ;
else
@@ -909,7 +908,6 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct
drm_display_mode *mode,
if (is_cpu_edp(intel_dp)) {
/* don't miss out required setting for eDP */
- intel_dp-DP |= DP_PLL_ENABLE;
if (adjusted_mode-clock 20)
intel_dp-DP |= DP_PLL_FREQ_160MHZ;
else
@@ -1192,8 +1190,15 @@ static void ironlake_edp_pll_on(struct intel_dp
*intel_dp)
DRM_DEBUG_KMS(\n);
dpa_ctl = I915_READ(DP_A);
- dpa_ctl |= DP_PLL_ENABLE;
- I915_WRITE(DP_A, dpa_ctl);
+ WARN(dpa_ctl DP_PLL_ENABLE, dp pll on, should be off\n);
+ WARN(dpa_ctl DP_PORT_EN, dp port still on, should be off\n);
+
+ /* We don't adjust intel_dp-DP while tearing down the link, to
+* facilitate link retraining (e.g. after hotplug). Hence clear all
+* enable bits here to ensure that we don't enable too much. */
+ intel_dp-DP = ~(DP_PORT_EN | DP_AUDIO_OUTPUT_ENABLE);
+ intel_dp-DP |= DP_PLL_ENABLE;
+ I915_WRITE(DP_A, intel_dp-DP);
POSTING_READ(DP_A);
udelay(200);
}
@@ -1209,6 +1214,13 @@ static void ironlake_edp_pll_off(struct intel_dp
*intel_dp)
to_intel_crtc(crtc)-pipe);
dpa_ctl = I915_READ(DP_A);
+ WARN((dpa_ctl DP_PLL_ENABLE) == 0,
+dp pll off, should be on\n);
+ WARN(dpa_ctl DP_PORT_EN, dp port still on, should be off\n);
+
+ /* We can't rely on the value tracked for the DP register in
+* intel_dp-DP because link_down must not change that (otherwise link
+* re-training will fail. */
dpa_ctl = ~DP_PLL_ENABLE;
I915_WRITE(DP_A, dpa_ctl);
POSTING_READ(DP_A);
@@ -1906,13 +1918,6 @@ intel_dp_link_down(struct intel_dp *intel_dp)
Re: [Intel-gfx] [PATCH 4/5] drm/i915: rip out dp port enabling cludges^Wchecks
Hi
2012/9/6 Daniel Vetter daniel.vet...@ffwll.ch:
These have been added because dp links are fiddle things and don't
like it when we try to re-train an enabled output (or disable a
disabled output harder). And because the crtc helper code is
ridiculously bad add tracking the modeset state.
But with the new code in place it is simply a bug to disable a disabled
encoder or to enable an enabled encoder again. Hence convert these to
WARNs (and bail out for safety), but flatten all conditionals in the
code itself.
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7c746d1..07f9521 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1333,15 +1333,15 @@ static void intel_enable_dp(struct intel_encoder
*encoder)
struct drm_i915_private *dev_priv = dev-dev_private;
uint32_t dp_reg = I915_READ(intel_dp-output_reg);
+ if (WARN_ON(dp_reg DP_PORT_EN))
+ return;
+
ironlake_edp_panel_vdd_on(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
- if (!(dp_reg DP_PORT_EN)) {
- intel_dp_start_link_train(intel_dp);
- ironlake_edp_panel_on(intel_dp);
- ironlake_edp_panel_vdd_off(intel_dp, true);
- intel_dp_complete_link_train(intel_dp);
- } else
- ironlake_edp_panel_vdd_off(intel_dp, false);
+ intel_dp_start_link_train(intel_dp);
+ ironlake_edp_panel_on(intel_dp);
+ ironlake_edp_panel_vdd_off(intel_dp, true);
+ intel_dp_complete_link_train(intel_dp);
ironlake_edp_backlight_on(intel_dp);
}
@@ -1913,7 +1913,7 @@ intel_dp_link_down(struct intel_dp *intel_dp)
struct drm_i915_private *dev_priv = dev-dev_private;
uint32_t DP = intel_dp-DP;
- if ((I915_READ(intel_dp-output_reg) DP_PORT_EN) == 0)
+ if (WARN_ON((I915_READ(intel_dp-output_reg) DP_PORT_EN) == 0))
return;
DRM_DEBUG_KMS(\n);
--
1.7.11.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: disable the cpu edp port after the cpu pipe
On Thu, Sep 13, 2012 at 04:11:20PM -0300, Paulo Zanoni wrote:
Hi
2012/9/6 Daniel Vetter daniel.vet...@ffwll.ch:
See bspec, Vol3 Part2, Section 1.1.3 Display Mode Set Sequence. This
applies to all platforms where we currently support eDP on, i.e. ilk,
snb ivb.
Ok, so I looked at BSpec and the conclusion is: shouldn't we do this
for eDP _and_ DP instead of just eDP? The only things to disable
before the crtc are audio, the panel backlight, and then the panel
power.
Your patch looks correct, but maybe it could be even more correct by
including DP too? Yes, I can help testing.
The dp pll for cpu edp is special, since we set it in the DP_A register.
The pll for pch dp ports is just the regular pch pll iirc, and that is
handled by the common code. Also, dp pch seems to wfm, whereas cpu edp was
definitely broken.
In any case, frobbing the pch dp sequence would be a separate patch imo.
So can I still have an r-b for this on here?
-Daniel
Without this change we fail to light up the eDP port on previously
unused crtcs (likely because something is stuck on the old pipe), and
we also fail to properly disable the old pipe (i.e. bit 30 in the
PIPECONF register is stuck as set until the next reboot).
v2: Rebased on top of the edp panel off sequence changes in 3.6-rc2.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=44001
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_dp.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index 07f9521..f28353d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1315,15 +1315,20 @@ static void intel_disable_dp(struct intel_encoder
*encoder)
ironlake_edp_backlight_off(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
ironlake_edp_panel_off(intel_dp);
- intel_dp_link_down(intel_dp);
+
+ /* cpu edp my only be disable _after_ the cpu pipe/plane is
disabled. */
+ if (!is_cpu_edp(intel_dp))
+ intel_dp_link_down(intel_dp);
}
static void intel_post_disable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
- if (is_cpu_edp(intel_dp))
+ if (is_cpu_edp(intel_dp)) {
+ intel_dp_link_down(intel_dp);
ironlake_edp_pll_off(intel_dp);
+ }
}
static void intel_enable_dp(struct intel_encoder *encoder)
--
1.7.11.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915: robustify edp_pll_on/off
On Thu, Sep 13, 2012 at 02:46:19PM -0300, Paulo Zanoni wrote:
Hi
2012/9/6 Daniel Vetter daniel.vet...@ffwll.ch:
With the previous patch to clean up where exactly these two functions
are getting called, this patch can tackle the enable/disable code
itself:
This is the kind of patch that's hard to proof-read. Basically, IMHO,
the intel_dp-DP variable makes things harder to understand and
review. Sometimes we use intel_dp-DP directly, sometimes we do int
DP = intel_dp-DP and then just change the temporary DP (sometimes
not updating the original intel_dp-DP after changing the temporary
DP), sometimes we read/write directly from intel_dp-output_reg,
sometimes we pass int DP as a function argument instead of storing
the value inside intel_dp-DP. I have to admit that even after working
some time with this code I'm not exactly sure why intel_dp-DP is
really necessary. One of my plans was to try to write a patch that
removes the variable so I could at least maybe understand why it's
necessary (the plan was to read/write from/to intel_dp-output_reg
directly). You can do that if you want :)
Complaints aside, here are my comments:
- WARN if the port enable bit is in the wrong state or if the edp pll
bit is in the wrong state, just for paranoia's sake.
Looks good.
- Don't disable the edp pll harder in the modeset functions just for
fun.
Which part of the patch is this message about? Did you mean
intel_dp_link_down instead of modeset? I could not find anything on
our documentation saying that the PLL must be disabled when retraining
the link, so your patch looks correct, but this is the kind of thing
that we should really try to test somehow :)
See below.
- Don't set the edp pll enable flag in intel_dp-DP in modeset, do
that while changing the actual hw state. We do the same with the
actual port enable bit, so this is a bit more consistent.
Looks good.
- Track the current DP register value when setting things up and add
some comments how intel_dp-DP is used in the disable code.
v2: Be more careful with resetting intel_dp-DP - otherwise dpms
off-on will fail spectacularly, becuase we enable the eDP port when
we should only enable the eDP pll.
These 2 chunks are the ones my comment above is about. I'll just trust
our beloved maintainer on these chunks :)
Yeah, intel_dp-DP is hard to follow, and I have some more patches. After
all this has settled the rule is:
- intel_dp-DP is computed freshly in intel_dp_mode_set
- intel_dp-DP is update with the link training values in start_link_train
so that complete_link_train writes the right registers. Same applies to
edp_pll_on for similar reasons
- All the disable functions don't frob intel_dp-DP so that we can
re-enable after dpms off without going through -mode_set
So yeah, a bit a mess, but should work.
-Daniel
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_dp.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index c72d4f3..7c746d1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -887,7 +887,6 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct
drm_display_mode *mode,
intel_dp-DP |= intel_crtc-pipe 29;
/* don't miss out required setting for eDP */
- intel_dp-DP |= DP_PLL_ENABLE;
if (adjusted_mode-clock 20)
intel_dp-DP |= DP_PLL_FREQ_160MHZ;
else
@@ -909,7 +908,6 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct
drm_display_mode *mode,
if (is_cpu_edp(intel_dp)) {
/* don't miss out required setting for eDP */
- intel_dp-DP |= DP_PLL_ENABLE;
if (adjusted_mode-clock 20)
intel_dp-DP |= DP_PLL_FREQ_160MHZ;
else
@@ -1192,8 +1190,15 @@ static void ironlake_edp_pll_on(struct intel_dp
*intel_dp)
DRM_DEBUG_KMS(\n);
dpa_ctl = I915_READ(DP_A);
- dpa_ctl |= DP_PLL_ENABLE;
- I915_WRITE(DP_A, dpa_ctl);
+ WARN(dpa_ctl DP_PLL_ENABLE, dp pll on, should be off\n);
+ WARN(dpa_ctl DP_PORT_EN, dp port still on, should be off\n);
+
+ /* We don't adjust intel_dp-DP while tearing down the link, to
+* facilitate link retraining (e.g. after hotplug). Hence clear all
+* enable bits here to ensure that we don't enable too much. */
+ intel_dp-DP = ~(DP_PORT_EN | DP_AUDIO_OUTPUT_ENABLE);
+ intel_dp-DP |= DP_PLL_ENABLE;
+ I915_WRITE(DP_A, intel_dp-DP);
POSTING_READ(DP_A);
udelay(200);
}
@@ -1209,6 +1214,13
Re: [Intel-gfx] [PATCH 3/5] drm/i915: robustify edp_pll_on/off
On Thu, Sep 13, 2012 at 02:46:19PM -0300, Paulo Zanoni wrote:
Hi
2012/9/6 Daniel Vetter daniel.vet...@ffwll.ch:
With the previous patch to clean up where exactly these two functions
are getting called, this patch can tackle the enable/disable code
itself:
This is the kind of patch that's hard to proof-read. Basically, IMHO,
the intel_dp-DP variable makes things harder to understand and
review. Sometimes we use intel_dp-DP directly, sometimes we do int
DP = intel_dp-DP and then just change the temporary DP (sometimes
not updating the original intel_dp-DP after changing the temporary
DP), sometimes we read/write directly from intel_dp-output_reg,
sometimes we pass int DP as a function argument instead of storing
the value inside intel_dp-DP. I have to admit that even after working
some time with this code I'm not exactly sure why intel_dp-DP is
really necessary. One of my plans was to try to write a patch that
removes the variable so I could at least maybe understand why it's
necessary (the plan was to read/write from/to intel_dp-output_reg
directly). You can do that if you want :)
Oh, missed this one here a bit. Will blow up - the reason is that we need
to support re-training when the user unplugs, then replugs a dp screen.
Since that works perfectly fine with hdmi, dvi, vga it should work with
dp, too.
But on dp we need to retrain the link, which means disabling, then
re-enabling. Since that potentially clobbers the register, we keep the
value we've computed at -mode_set time around so that we can reuse it for
the re-training. See my other mail for the exact semantics.
Imo once you've worked with the code long enough, and now that it'll lose
a lot of the ugly hacks and special cases, intel_dp-DP make sense.
Cheers, Daniel
Complaints aside, here are my comments:
- WARN if the port enable bit is in the wrong state or if the edp pll
bit is in the wrong state, just for paranoia's sake.
Looks good.
- Don't disable the edp pll harder in the modeset functions just for
fun.
Which part of the patch is this message about? Did you mean
intel_dp_link_down instead of modeset? I could not find anything on
our documentation saying that the PLL must be disabled when retraining
the link, so your patch looks correct, but this is the kind of thing
that we should really try to test somehow :)
- Don't set the edp pll enable flag in intel_dp-DP in modeset, do
that while changing the actual hw state. We do the same with the
actual port enable bit, so this is a bit more consistent.
Looks good.
- Track the current DP register value when setting things up and add
some comments how intel_dp-DP is used in the disable code.
v2: Be more careful with resetting intel_dp-DP - otherwise dpms
off-on will fail spectacularly, becuase we enable the eDP port when
we should only enable the eDP pll.
These 2 chunks are the ones my comment above is about. I'll just trust
our beloved maintainer on these chunks :)
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_dp.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index c72d4f3..7c746d1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -887,7 +887,6 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct
drm_display_mode *mode,
intel_dp-DP |= intel_crtc-pipe 29;
/* don't miss out required setting for eDP */
- intel_dp-DP |= DP_PLL_ENABLE;
if (adjusted_mode-clock 20)
intel_dp-DP |= DP_PLL_FREQ_160MHZ;
else
@@ -909,7 +908,6 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct
drm_display_mode *mode,
if (is_cpu_edp(intel_dp)) {
/* don't miss out required setting for eDP */
- intel_dp-DP |= DP_PLL_ENABLE;
if (adjusted_mode-clock 20)
intel_dp-DP |= DP_PLL_FREQ_160MHZ;
else
@@ -1192,8 +1190,15 @@ static void ironlake_edp_pll_on(struct intel_dp
*intel_dp)
DRM_DEBUG_KMS(\n);
dpa_ctl = I915_READ(DP_A);
- dpa_ctl |= DP_PLL_ENABLE;
- I915_WRITE(DP_A, dpa_ctl);
+ WARN(dpa_ctl DP_PLL_ENABLE, dp pll on, should be off\n);
+ WARN(dpa_ctl DP_PORT_EN, dp port still on, should be off\n);
+
+ /* We don't adjust intel_dp-DP while tearing down the link, to
+* facilitate link retraining (e.g. after hotplug). Hence clear all
+* enable bits here to ensure that we don't enable too much. */
+ intel_dp-DP = ~(DP_PORT_EN | DP_AUDIO_OUTPUT_ENABLE);
+ intel_dp-DP |=
[Intel-gfx] Start a seperate driver instance for each screen.
Good evening. I have a config question about a multi monitor setup: I'm using the intel driver xf86-video-intel 2.20.0 and want to start a single driver instance for each screen in xorg in order to use xinerama. Because at the moment every window which i maximize spans over the whole screen, i want ti use xinerama. I have two device sections in my xorg.conf but only the first is used. Her's my xorg.conf: Section ServerLayout Identifier X.org Configured Screen 0 Screen0 0 0 Screen 1 Screen1 RightOf Screen0 Screen 2 Screen2 RightOf Screen1 Screen 3 Screen3 RightOf Screen2 InputDeviceMouse0 CorePointer InputDeviceKeyboard0 CoreKeyboard EndSection Section Files ModulePath /usr/lib64/xorg/modules FontPath/usr/share/fonts/misc/ FontPath/usr/share/fonts/TTF/ FontPath/usr/share/fonts/OTF/ FontPath/usr/share/fonts/Type1/ FontPath/usr/share/fonts/100dpi/ FontPath/usr/share/fonts/75dpi/ EndSection Section Module Load record Load glx Load extmod Load dbe Load RandR EndSection Section InputDevice Identifier Keyboard0 Driver kbd EndSection Section InputDevice Identifier Mouse0 Driver mouse Option Protocol auto Option Device /dev/input/mice Option ZAxisMapping 4 5 6 7 EndSection Section Monitor Identifier Monitor0 VendorName DELL ModelName U2410 HorizSync 30.0 - 81.0 VertRefresh 56.0 - 76.0 EndSection Section Monitor Identifier Monitor1 VendorName DELL ModelName U2410 Option Primary true HorizSync 30.0 - 81.0 VertRefresh 56.0 - 76.0 EndSection Section Monitor Identifier Monitor2 VendorName DELL ModelName 1908WFP Option dpms Option PreferredMode 1440x900 EndSection Section Monitor Identifier Monitor3 VendorName DELL ModelName LCD Option dpms Option PreferredMode 1920x1080 Option RightOf Monitor2 EndSection Section Device Identifier Device0 Driver intel VendorName Intel BoardName IGS BusID PCI:0:2:0 Option monitor-LVDS1 Monitor3 Option monitor-VGA1 Monitor2 Option MergedFB false Option ZaphodHeads LVDS1 EndSection Section Device Identifier Device1 Driver intel VendorName Intel BoardName IGS BusID PCI:0:2:0 Option monitor-LVDS1 Monitor3 Option monitor-VGA1 Monitor2 Option MergedFB false Option ZaphodHeads VGA1 EndSection Section Device Identifier Device2 Driver nvidia VendorName NVIDIA Corporation BoardName NVS 4200M BusID PCI:1:0:0 Screen 0 EndSection Section Device Identifier Device3 Driver nvidia VendorName NVIDIA Corporation BoardName NVS 4200M BusID PCI:1:0:0 Screen 1 EndSection Section Screen Identifier Screen0 Device Device2 MonitorMonitor0 DefaultDepth24 Option TwinView 0 Option TwinViewXineramaInfoOrder DFP-4, DFP-2 Option metamodes DFP-4: nvidia-auto-select +0+0 SubSection Display Depth 24 EndSubSection EndSection Section Screen Identifier Screen1 Device Device3 MonitorMonitor1 DefaultDepth24 Option TwinView 0 Option metamodes DFP-2: nvidia-auto-select +0+0 SubSection Display Depth 24 EndSubSection EndSection Section Screen Identifier Screen2 Device Device0 MonitorMonitor2 DefaultDepth24 SubSection Display Depth 24 Modes 1440x900 EndSubSection EndSection Section Screen Identifier Screen3 Device Device1 MonitorMonitor3 SubSection Display Depth 24 Modes 1920x1080 EndSubSection EndSection Thanks in advance. David___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: disable the cpu edp port after the cpu pipe
2012/9/13 Daniel Vetter dan...@ffwll.ch:
On Thu, Sep 13, 2012 at 04:11:20PM -0300, Paulo Zanoni wrote:
Hi
2012/9/6 Daniel Vetter daniel.vet...@ffwll.ch:
See bspec, Vol3 Part2, Section 1.1.3 Display Mode Set Sequence. This
applies to all platforms where we currently support eDP on, i.e. ilk,
snb ivb.
Ok, so I looked at BSpec and the conclusion is: shouldn't we do this
for eDP _and_ DP instead of just eDP? The only things to disable
before the crtc are audio, the panel backlight, and then the panel
power.
Your patch looks correct, but maybe it could be even more correct by
including DP too? Yes, I can help testing.
The dp pll for cpu edp is special, since we set it in the DP_A register.
The pll for pch dp ports is just the regular pch pll iirc, and that is
handled by the common code. Also, dp pch seems to wfm, whereas cpu edp was
definitely broken.
In any case, frobbing the pch dp sequence would be a separate patch imo.
So can I still have an r-b for this on here?
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
-Daniel
Without this change we fail to light up the eDP port on previously
unused crtcs (likely because something is stuck on the old pipe), and
we also fail to properly disable the old pipe (i.e. bit 30 in the
PIPECONF register is stuck as set until the next reboot).
v2: Rebased on top of the edp panel off sequence changes in 3.6-rc2.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=44001
Signed-Off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_dp.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index 07f9521..f28353d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1315,15 +1315,20 @@ static void intel_disable_dp(struct intel_encoder
*encoder)
ironlake_edp_backlight_off(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
ironlake_edp_panel_off(intel_dp);
- intel_dp_link_down(intel_dp);
+
+ /* cpu edp my only be disable _after_ the cpu pipe/plane is
disabled. */
+ if (!is_cpu_edp(intel_dp))
+ intel_dp_link_down(intel_dp);
}
static void intel_post_disable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
- if (is_cpu_edp(intel_dp))
+ if (is_cpu_edp(intel_dp)) {
+ intel_dp_link_down(intel_dp);
ironlake_edp_pll_off(intel_dp);
+ }
}
static void intel_enable_dp(struct intel_encoder *encoder)
--
1.7.11.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Start a seperate driver instance for each screen.
Good evening. Ii have a config question about a mulit monitor setup: I'm using the intel driver xf86-video-intel 2.20.0 and want to start a single driver instance for each screen in xorg in order to use xinerama. Because at the moment every window which i maximize spans over the whole screen, i want ti use xinerama. I have two device sections in my xorg.conf but only the first is used. Her's my xorg.conf: Section ServerLayout Identifier X.org Configured Screen 0 Screen0 0 0 Screen 1 Screen1 RightOf Screen0 Screen 2 Screen2 RightOf Screen1 Screen 3 Screen3 RightOf Screen2 InputDeviceMouse0 CorePointer InputDeviceKeyboard0 CoreKeyboard EndSection Section Files ModulePath /usr/lib64/xorg/modules FontPath/usr/share/fonts/misc/ FontPath/usr/share/fonts/TTF/ FontPath/usr/share/fonts/OTF/ FontPath/usr/share/fonts/Type1/ FontPath/usr/share/fonts/100dpi/ FontPath/usr/share/fonts/75dpi/ EndSection Section Module Load record Load glx Load extmod Load dbe Load RandR EndSection Section InputDevice Identifier Keyboard0 Driver kbd EndSection Section InputDevice Identifier Mouse0 Driver mouse Option Protocol auto Option Device /dev/input/mice Option ZAxisMapping 4 5 6 7 EndSection Section Monitor Identifier Monitor0 VendorName DELL ModelName U2410 HorizSync 30.0 - 81.0 VertRefresh 56.0 - 76.0 EndSection Section Monitor Identifier Monitor1 VendorName DELL ModelName U2410 Option Primary true HorizSync 30.0 - 81.0 VertRefresh 56.0 - 76.0 EndSection Section Monitor Identifier Monitor2 VendorName DELL ModelName 1908WFP Option dpms Option PreferredMode 1440x900 EndSection Section Monitor Identifier Monitor3 VendorName DELL ModelName LCD Option dpms Option PreferredMode 1920x1080 Option RightOf Monitor2 EndSection Section Device Identifier Device0 Driver intel VendorName Intel BoardName IGS BusID PCI:0:2:0 Option monitor-LVDS1 Monitor3 Option monitor-VGA1 Monitor2 Option MergedFB false Option ZaphodHeads LVDS1 EndSection Section Device Identifier Device1 Driver intel VendorName Intel BoardName IGS BusID PCI:0:2:0 Option monitor-LVDS1 Monitor3 Option monitor-VGA1 Monitor2 Option MergedFB false Option ZaphodHeads VGA1 EndSection Section Device Identifier Device2 Driver nvidia VendorName NVIDIA Corporation BoardName NVS 4200M BusID PCI:1:0:0 Screen 0 EndSection Section Device Identifier Device3 Driver nvidia VendorName NVIDIA Corporation BoardName NVS 4200M BusID PCI:1:0:0 Screen 1 EndSection Section Screen Identifier Screen0 Device Device2 MonitorMonitor0 DefaultDepth24 Option TwinView 0 Option TwinViewXineramaInfoOrder DFP-4, DFP-2 Option metamodes DFP-4: nvidia-auto-select +0+0 SubSection Display Depth 24 EndSubSection EndSection Section Screen Identifier Screen1 Device Device3 MonitorMonitor1 DefaultDepth24 Option TwinView 0 Option metamodes DFP-2: nvidia-auto-select +0+0 SubSection Display Depth 24 EndSubSection EndSection Section Screen Identifier Screen2 Device Device0 MonitorMonitor2 DefaultDepth24 SubSection Display Depth 24 Modes 1440x900 EndSubSection EndSection Section Screen Identifier Screen3 Device Device1 MonitorMonitor3 SubSection Display Depth 24 Modes 1920x1080 EndSubSection EndSection___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915: rip out intel_disable_pch_ports
On Thu, 6 Sep 2012 22:08:32 +0200 Daniel Vetter daniel.vet...@ffwll.ch wrote: Even with the old crtc helper code we should have disabled all encoders on that pipe by now, and with the new code this would definitely paper over a bug. We already have the necessary checks in place in intel_disable_transcoder, so if we accidentally leave a pch port on, this will be caught. Hence just rip this all out. Note that up to the patch in this giant modeset series that removes the LVDS special case to avoid disabling LVDS in the encoder-prepare callback (drm/i915/lvds: ditch -prepare special case), this was not the case for all outputs. Also note that in commit 1b3c7a47f993bf9ab6c4c7cc3bbf5588052b58f4 Author: Zhenyu Wang zhen...@linux.intel.com Date: Wed Nov 25 13:09:38 2009 +0800 drm/i915: Fix LVDS stability issue on Ironlake this was already discovered independently and worked around. How I bloody hate this entire mess of cludges piled on top of other cludges. Yes this was always an ugly layering violation too. I think I was worried about a mode set on one crtc getting stale config bits from another that hadn't been touched... Anyway now we've fixed it properly. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: don't disable fdi links harder in ilk_crtc_enable
On Thu, 6 Sep 2012 22:08:33 +0200
Daniel Vetter daniel.vet...@ffwll.ch wrote:
Because they should have been disabled when shutting down the display
pipe previously. To ensure that this is the case, add a few assserts
instead of unconditionally disabling the fdi link.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6c06109..0973797 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3182,10 +3182,12 @@ static void ironlake_crtc_enable(struct drm_crtc
*crtc)
is_pch_port = intel_crtc_driving_pch(crtc);
- if (is_pch_port)
+ if (is_pch_port) {
ironlake_fdi_pll_enable(intel_crtc);
- else
- ironlake_fdi_disable(crtc);
+ } else {
+ assert_fdi_tx_disabled(dev_priv, pipe);
+ assert_fdi_rx_disabled(dev_priv, pipe);
+ }
/* Enable panel fitting for LVDS */
if (dev_priv-pch_pf_size
Yep.
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: don't call dpms funcs after set_mode
On Thu, 6 Sep 2012 22:08:34 +0200
Daniel Vetter daniel.vet...@ffwll.ch wrote:
... because our current set_mode implementation doesn't bother to adjust
for the dpms state, we just forcefully update it. So stop pretending that
we're better than we are and rip out this extranous call.
Note that this totally confuses userspace, because the exposed connector
property isn't actually updated ...
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 0973797..805324d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7236,7 +7236,6 @@ static int intel_crtc_set_config(struct drm_mode_set
*set)
struct drm_mode_set save_set;
struct intel_set_config *config;
int ret;
- int i;
BUG_ON(!set);
BUG_ON(!set-crtc);
@@ -7300,15 +7299,6 @@ static int intel_crtc_set_config(struct drm_mode_set
*set)
ret = -EINVAL;
goto fail;
}
-
- if (set-crtc-enabled) {
- DRM_DEBUG_KMS(Setting connector DPMS state to on\n);
- for (i = 0; i set-num_connectors; i++) {
- DRM_DEBUG_KMS(\t[CONNECTOR:%d:%s] set DPMS
on\n, set-connectors[i]-base.id,
-
drm_get_connector_name(set-connectors[i]));
-
set-connectors[i]-funcs-dpms(set-connectors[i], DRM_MODE_DPMS_ON);
- }
- }
} else if (config-fb_changed) {
ret = intel_pipe_set_base(set-crtc,
set-x, set-y, set-fb);
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: update dpms property in set_mode
On Thu, 6 Sep 2012 22:08:35 +0200
Daniel Vetter daniel.vet...@ffwll.ch wrote:
Hopefully this makes userspace slightly less confused about us
frobbing the dpms state behind its back. Yeah, it would be better
to be more careful with not changing the dpms state, but that is
quite more invasive.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 805324d..bff0936 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6806,7 +6806,13 @@ intel_modeset_update_state(struct drm_device *dev,
unsigned prepare_pipes)
intel_crtc = to_intel_crtc(connector-encoder-crtc);
if (prepare_pipes (1 intel_crtc-pipe)) {
+ struct drm_property *dpms_property =
+ dev-mode_config.dpms_property;
+
connector-dpms = DRM_MODE_DPMS_ON;
+ drm_connector_property_set_value(connector,
+ dpms_property,
+ DRM_MODE_DPMS_ON);
intel_encoder = to_intel_encoder(connector-encoder);
intel_encoder-connectors_active = true;
Both this and the last one need lots of testing coming from different
DPMS states between fbcon and X, especially across different heads and
connectors.
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: disable the cpu edp port after the cpu pipe
On Thu, Sep 13, 2012 at 04:43:57PM -0300, Paulo Zanoni wrote: 2012/9/13 Daniel Vetter dan...@ffwll.ch: On Thu, Sep 13, 2012 at 04:11:20PM -0300, Paulo Zanoni wrote: Hi 2012/9/6 Daniel Vetter daniel.vet...@ffwll.ch: See bspec, Vol3 Part2, Section 1.1.3 Display Mode Set Sequence. This applies to all platforms where we currently support eDP on, i.e. ilk, snb ivb. Ok, so I looked at BSpec and the conclusion is: shouldn't we do this for eDP _and_ DP instead of just eDP? The only things to disable before the crtc are audio, the panel backlight, and then the panel power. Your patch looks correct, but maybe it could be even more correct by including DP too? Yes, I can help testing. The dp pll for cpu edp is special, since we set it in the DP_A register. The pll for pch dp ports is just the regular pch pll iirc, and that is handled by the common code. Also, dp pch seems to wfm, whereas cpu edp was definitely broken. In any case, frobbing the pch dp sequence would be a separate patch imo. So can I still have an r-b for this on here? Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com I've slurped in the other 3 patches, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: rip out edp special case from dp_link_down
Hi
2012/9/9 Daniel Vetter daniel.vet...@ffwll.ch:
This has been tons of fun to figure out with git blame. The first
notion of this code block goes back to the original cpu edp enabling
for ilk in
commit 32f9d658aee5be09ebdd28fc730630e61d0b46db
Author: Zhenyu Wang zhen...@linux.intel.com
Date: Fri Jul 24 01:00:32 2009 +0800
drm/i915: Add eDP support on IGDNG mobile chip
Two things are notable in this commit wrt to the this edp special
case:
- The IS_eDP check _only_ fires for DP A, i.e. cpu edp ports.
- The cpu edp port is disabled at the top of the dp_link_down function.
My theory is that these hacks was added to work around the completely
different modeset sequence for cpu edp ports compared to pch edp
ports. With the cpu edp confusion on ilk (and snb/ivb) now fixed up,
this shouldn't be required any more.
The really interesting question is how this special cases survived
this long in the code. The first step is declaring the pch port D as
eDP if it's used for an internal panel:
commit b329530ca7cdf6bf014f2124efd983e01265d623
Author: Adam Jackson a...@redhat.com
Date: Fri Jul 16 14:46:28 2010 -0400
drm/i915/dp: Correctly report eDP in the core connector type
This commit unfortunately failed to notice that not all edp ports are
created equal. Then follow a flurry of refactorings, culminating in a
patch from Keith Packard which resulted in the current logic (by
making it correct for all platforms that have edp):
commit 417e822deee1d2bcd8a8a60660c40a0903713f2b
Author: Keith Packard kei...@keithp.com
Date: Tue Nov 1 19:54:11 2011 -0700
drm/i915: Treat PCH eDP like DP in most places
None of these cleanups or refactorings supply any reason why we need
this code, they've simply carried it on as-is.
Hence presume it might be harmful with the current code and rip it
out. We do rewrite the link training bits completely anyway when
re-training the link.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
After looking for a long time I could not find a reason why we should
set the pattern to train off while disabling the eDP port. Notice
that train off means send normal pixels (see BSpec). All the docs
say is that when _enabling_ the port we need to set training pattern
1, which we should already be doing. After your patch, when we disable
the port we will disable it with training pattern already set to 1
(instead of send normal pixels/ train off), but there's nothing
saying we shouldn't do this. So yeah, your patch looks fine. If we
ever revert this patch, let's make sure we add a big comment
explaining it.
Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f28353d..8adad5e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1934,13 +1934,6 @@ intel_dp_link_down(struct intel_dp *intel_dp)
msleep(17);
- if (is_edp(intel_dp)) {
- if (HAS_PCH_CPT(dev) (IS_GEN7(dev) ||
!is_cpu_edp(intel_dp)))
- DP |= DP_LINK_TRAIN_OFF_CPT;
- else
- DP |= DP_LINK_TRAIN_OFF;
- }
-
if (HAS_PCH_IBX(dev)
I915_READ(intel_dp-output_reg) DP_PIPEB_SELECT) {
struct drm_crtc *crtc = intel_dp-base.base.crtc;
--
1.7.11.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
