[Intel-gfx] [PATCH 3/5] drm/i915: In intel_dp_init, replace read of DPCD with intel_dp_get_dpcd
Eliminates an open-coded read and also gains the retry behaviour of
intel_dp_get_dpcd, which seems like a good idea.
Signed-off-by: Keith Packard kei...@keithp.com
---
drivers/gpu/drm/i915/intel_dp.c |8 +++-
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 41674e1..9f134d2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2015,7 +2015,7 @@ intel_dp_init(struct drm_device *dev, int output_reg)
/* Cache some DPCD data in the eDP case */
if (is_edp(intel_dp)) {
- int ret;
+ bool ret;
u32 pp_on, pp_div;
pp_on = I915_READ(PCH_PP_ON_DELAYS);
@@ -2028,11 +2028,9 @@ intel_dp_init(struct drm_device *dev, int output_reg)
dev_priv-panel_t12 *= 100; /* t12 in 100ms units */
ironlake_edp_panel_vdd_on(intel_dp);
- ret = intel_dp_aux_native_read(intel_dp, DP_DPCD_REV,
- intel_dp-dpcd,
- sizeof(intel_dp-dpcd));
+ ret = intel_dp_get_dpcd(intel_dp);
ironlake_edp_panel_vdd_off(intel_dp);
- if (ret == sizeof(intel_dp-dpcd)) {
+ if (ret) {
if (intel_dp-dpcd[DP_DPCD_REV] = 0x11)
dev_priv-no_aux_handshake =
intel_dp-dpcd[DP_MAX_DOWNSPREAD]
--
1.7.5.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915: DP_PIPE_ENABLED must check transcoder on CPT
Display port pipe selection on CPT is not done with a bit in the
output register, rather it is controlled by a couple of bits in the
separate transcoder register which indicate which display port output
is connected to the transcoder.
This patch replaces the simplistic macro DP_PIPE_ENABLED with the
rather more complicated function dp_pipe_enabled which checks the
output register to see if that is enabled, and then goes on to either
check the output register pipe selection bit (on non-CPT) or the
transcoder DP selection bits (on CPT).
Before this patch, any time the mode of pipe A was changed, any
display port outputs on pipe B would get disabled as
intel_disable_pch_ports would ensure that the mode setting operation
could occur on pipe A without interference from other outputs
connected to that pch port
Signed-off-by: Keith Packard kei...@keithp.com
---
drivers/gpu/drm/i915/i915_reg.h |3 --
drivers/gpu/drm/i915/intel_display.c | 45 +
2 files changed, 34 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5d5def7..f731565 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2083,9 +2083,6 @@
#define DP_PIPEB_SELECT (1 30)
#define DP_PIPE_MASK (1 30)
-#define DP_PIPE_ENABLED(V, P) \
- (((V) (DP_PIPE_MASK | DP_PORT_EN)) == ((P) 30 | DP_PORT_EN))
-
/* Link training mode - select a suitable mode for each stage */
#define DP_LINK_TRAIN_PAT_1 (0 28)
#define DP_LINK_TRAIN_PAT_2 (1 28)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5609c06..fc26cb4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -979,11 +979,29 @@ static void assert_transcoder_disabled(struct
drm_i915_private *dev_priv,
pipe_name(pipe));
}
+static bool dp_pipe_enabled(struct drm_i915_private *dev_priv, enum pipe pipe,
+ int reg, u32 port_sel, u32 val)
+{
+ if ((val DP_PORT_EN) == 0)
+ return false;
+
+ if (HAS_PCH_CPT(dev_priv-dev)) {
+ u32 trans_dp_ctl_reg = TRANS_DP_CTL(pipe);
+ u32 trans_dp_ctl = I915_READ(trans_dp_ctl_reg);
+ if ((trans_dp_ctl TRANS_DP_PORT_SEL_MASK) != port_sel)
+ return false;
+ } else {
+ if ((val DP_PIPE_MASK) != (pipe 30))
+ return false;
+ }
+ return true;
+}
+
static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv,
- enum pipe pipe, int reg)
+ enum pipe pipe, int reg, u32 port_sel)
{
u32 val = I915_READ(reg);
- WARN(DP_PIPE_ENABLED(val, pipe),
+ WARN(dp_pipe_enabled(dev_priv, pipe, reg, port_sel, val),
PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n,
reg, pipe_name(pipe));
}
@@ -1003,9 +1021,9 @@ static void assert_pch_ports_disabled(struct
drm_i915_private *dev_priv,
int reg;
u32 val;
- assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B);
- assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C);
- assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D);
+ assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_B, TRANS_DP_PORT_SEL_B);
+ assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_C, TRANS_DP_PORT_SEL_C);
+ assert_pch_dp_disabled(dev_priv, pipe, PCH_DP_D, TRANS_DP_PORT_SEL_D);
reg = PCH_ADPA;
val = I915_READ(reg);
@@ -1334,19 +1352,24 @@ static void intel_disable_plane(struct drm_i915_private
*dev_priv,
}
static void disable_pch_dp(struct drm_i915_private *dev_priv,
- enum pipe pipe, int reg)
+ enum pipe pipe, int reg, u32 port_sel)
{
u32 val = I915_READ(reg);
- if (DP_PIPE_ENABLED(val, pipe))
+ if (dp_pipe_enabled(dev_priv, pipe, reg, port_sel, val)) {
+ DRM_DEBUG_KMS(Disabling pch dp %x on pipe %d\n, reg, pipe);
I915_WRITE(reg, val ~DP_PORT_EN);
+ }
}
static void disable_pch_hdmi(struct drm_i915_private *dev_priv,
enum pipe pipe, int reg)
{
u32 val = I915_READ(reg);
- if (HDMI_PIPE_ENABLED(val, pipe))
+ if (HDMI_PIPE_ENABLED(val, pipe)) {
+ DRM_DEBUG_KMS(Disabling pch HDMI %x on pipe %d\n,
+ reg, pipe);
I915_WRITE(reg, val ~PORT_ENABLE);
+ }
}
/* Disable any ports connected to this transcoder */
@@ -1358,9 +1381,9 @@ static void intel_disable_pch_ports(struct
drm_i915_private *dev_priv,
val = I915_READ(PCH_PP_CONTROL);
I915_WRITE(PCH_PP_CONTROL, val | PANEL_UNLOCK_REGS);
- disable_pch_dp(dev_priv, pipe, PCH_DP_B);
- disable_pch_dp(dev_priv, pipe, PCH_DP_C);
[Intel-gfx] [PATCH 4/5] drm/i915: Delay 250ms before running the hotplug code
If the connector is inserted or removed slowly, the hotplug line may well change state before the data lines do. So, assume the user isn't trying to fool us and give them 250ms to get the connector plugged or unplugged. Signed-off-by: Keith Packard kei...@keithp.com --- drivers/gpu/drm/i915/i915_irq.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9da2a2c..e3ce1c3 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -306,6 +306,8 @@ static void i915_hotplug_work_func(struct work_struct *work) struct drm_mode_config *mode_config = dev-mode_config; struct intel_encoder *encoder; + /* Wait a bit so that the connector change can be completed */ + msleep(250); mutex_lock(mode_config-mutex); DRM_DEBUG_KMS(running encoder hotplug functions\n); -- 1.7.5.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
Two things I've noticed: - Why not dev-mode_config.mutex? I was under the impression that mode_config.mutex protects most of the modesetting state and dev-struct_mutex protects things related to the gpu execution cores (i.e. all things gem), with struct_mutex nested within mode_config.mutex. It's hazy at the edges and likely broken in tons of corner cases, but still ... This has also the benefit that it won't stall execbuf. - And a nitpick: Why the dev_priv-dev indirection, when dev is already lying around? -Daniel -- Daniel Vetter daniel.vet...@ffwll.ch - +41 (0) 79 364 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 4/5] drm/i915: Delay 250ms before running the hotplug code
queue_delayed_work? Plays nicer with other workqueue-items. -Daniel PS: Scrap my other mail, just noticed that the merged patched r-b'ed by Jesse uses the mode_config mutex. -- Daniel Vetter daniel.vet...@ffwll.ch - +41 (0) 79 364 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] 915GM: render error detected, EIR: 0x00000010
Dear graphics folks, Am Montag, den 25.07.2011, 22:33 +0200 schrieb Paul Menzel: using `linux-image-3.0.0-1-686-pae` from Debian Sid/unstable I noticed the following message in `dmesg`. [ 76.292185] [drm] capturing error event; look for more information in /debug/dri/0/i915_error_state [ 76.294236] render error detected, EIR: 0x0010 [ 76.294246] page table error [ 76.294251] PGTBL_ER: 0x0010 [ 76.294264] [drm:i915_report_and_clear_eir] *ERROR* EIR stuck: 0x0010, masking [ 76.294298] render error detected, EIR: 0x0010 [ 76.294304] page table error [ 76.294309] PGTBL_ER: 0x0010 I just booted, GDM was started, I started the window manager and opened a terminal. I mounted the debug filesystem as documented in [1] (after the message had appeared in `dmesg`) and just copied the `/debug/dri/0/i915_error_state` to the attached file. The device is an ASUS EeePC 701 4G [2]. Searching for that error message on the Web gives several hits suggesting this error has been there for a long time [3] but is supposed to be fixed since 2.6.33 and also 2.6.32 stable. I will try to find out if I can find it in the system logs with earlier Linux kernels. I found such messages also with Linux kernel 2.6.39. [ 114.355954] [drm] capturing error event; look for more information in /debug/dri/0/i915_error_state [ 114.356015] render error detected, EIR: 0x0010 [ 114.356015] page table error [ 114.356015] PGTBL_ER: 0x0010 [ 114.356015] [drm:i915_report_and_clear_eir] *ERROR* EIR stuck: 0x0010, masking [ 114.356015] render error detected, EIR: 0x0010 [ 114.356015] page table error [ 114.356015] PGTBL_ER: 0x0010 Other information I found on the Web with the same error message are [4– 7] where some of them suggest that this is an error which got fixed again and again. Please tell me, if that is a know problem and if you want me to submit a report/ticket to the bug tracker. Thanks, Paul [1] http://intellinuxgraphics.org/intel-gpu-dump.html [2] https://secure.wikimedia.org/wikipedia/en/wiki/ASUS_Eee_PC#Specifications [3] https://bugzilla.kernel.org/show_bug.cgi?id=15502 [4] http://lkml.indiana.edu/hypermail/linux/kernel/1005.1/02637.html [5] https://bugs.archlinux.org/task/22781 [6] http://us.generation-nt.com/answer/2-6-36-rc5-i915-regression-help-200514811.html [7] http://www.devheads.net/linux/ubuntu/user/t43-login-render-error-detected-eir-0x0010-page-table-error-pgtbl-er-0x00100-drm915-handle-e.htm signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Sat, Jul 23, 2011 at 12:23:36AM +0400, Kirill Smelkov wrote:
Keith,
first of all thanks for your prompt reply. Then...
On Fri, Jul 22, 2011 at 11:00:41AM -0700, Keith Packard wrote:
On Fri, 22 Jul 2011 15:08:06 +0400, Kirill Smelkov k...@mns.spb.ru wrote:
And now after v3.0 is out, I've tested it again, and yes, like it was
broken on v3.0-rc5, it is (now even more) broken on v3.0 -- after first
bad io access the system freezes completely:
I looked at this when I first saw it (a couple of weeks ago), and I
couldn't see any obvious reason this patch would cause this particular
problem. I didn't want to revert the patch at that point as I feared it
would cause other subtle problems. Given that you've got a work-around,
it seemed best to just push this off past 3.0.
What kind of a workaround are you talking about? Sorry, to me it all
looked like UMS is being ignored forever. Anyway, let's move on to try
to solve the issue.
Given the failing address passed to ioread32, this seems like it's
probably the call to READ_BREADCRUMB -- I915_BREADCRUMB_INDEX is 0x21,
which is an offset in 32-bit units within the hardware status page. If
the status_page.page_addr value was zero, then the computed address
would end up being 0x84.
And, it looks like status_page.page_addr *will* end up being zero as a
result of the patch in question. The patch resets the entire ring
structure contents back to the initial values, which includes smashing
the status_page structure to zero, clearing the value of
status_page.page_addr set in i915_init_phys_hws.
Here's an untested patch which moves the initialization of
status_page.page_addr into intel_render_ring_init_dri. I note that
intel_init_render_ring_buffer *already* has the setting of the
status_page.page_addr value, and so I've removed the setting of
status_page.page_addr from i915_init_phys_hws.
I suspect we could remove the memset from intel_init_render_ring_buffer;
it seems entirely superfluous given the memset in i915_init_phys_hws.
From 159ba1dd207fc52590ce8a3afd83f40bd2cedf46 Mon Sep 17 00:00:00 2001
From: Keith Packard kei...@keithp.com
Date: Fri, 22 Jul 2011 10:44:39 -0700
Subject: [PATCH] drm/i915: Initialize RCS ring status page address in
intel_render_ring_init_dri
Physically-addressed hardware status pages are initialized early in
the driver load process by i915_init_phys_hws. For UMS environments,
the ring structure is not initialized until the X server starts. At
that point, the entire ring structure is re-initialized with all new
values. Any values set in the ring structure (including
ring-status_page.page_addr) will be lost when the ring is
re-initialized.
This patch moves the initialization of the status_page.page_addr value
to intel_render_ring_init_dri.
Signed-off-by: Keith Packard kei...@keithp.com
---
drivers/gpu/drm/i915/i915_dma.c |6 ++
drivers/gpu/drm/i915/intel_ringbuffer.c |3 +++
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index 1271282..8a3942c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -61,7 +61,6 @@ static void i915_write_hws_pga(struct drm_device *dev)
static int i915_init_phys_hws(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = dev-dev_private;
- struct intel_ring_buffer *ring = LP_RING(dev_priv);
/* Program Hardware Status Page */
dev_priv-status_page_dmah =
@@ -71,10 +70,9 @@ static int i915_init_phys_hws(struct drm_device *dev)
DRM_ERROR(Can not allocate hardware status page\n);
return -ENOMEM;
}
- ring-status_page.page_addr =
- (void __force __iomem *)dev_priv-status_page_dmah-vaddr;
- memset_io(ring-status_page.page_addr, 0, PAGE_SIZE);
+ memset_io((void __force __iomem *)dev_priv-status_page_dmah-vaddr,
+ 0, PAGE_SIZE);
i915_write_hws_pga(dev);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index e961568..47b9b27 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1321,6 +1321,9 @@ int intel_render_ring_init_dri(struct drm_device
*dev, u64 start, u32 size)
ring-get_seqno = pc_render_get_seqno;
}
+ if (!I915_NEED_GFX_HWS(dev))
+ ring-status_page.page_addr = dev_priv-status_page_dmah-vaddr;
+
ring-dev = dev;
INIT_LIST_HEAD(ring-active_list);
INIT_LIST_HEAD(ring-request_list);
I can't tell whether this is correct, because intel gfx driver is
unknown to me, but from the first glance your description sounds reasonable.
I'm out of office till ~ next week's tuesday, and on return I'll try
to test it on the hardware in question.
Keith, thanks
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Rename i915_dp_detect_common to intel_dp_get_dpcd
On Mon, 25 Jul 2011 23:36:31 -0700 Keith Packard kei...@keithp.com wrote: This describes the function better, allowing it to be used where the DPCD value is relevant. Signed-off-by: Keith Packard kei...@keithp.com --- Ah I see you've addressed my previous comment already. :) You can add my Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org to 1/5 and 2/5. -- 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/5] drm/i915: In intel_dp_init, replace read of DPCD with intel_dp_get_dpcd
On Mon, 25 Jul 2011 23:36:32 -0700
Keith Packard kei...@keithp.com wrote:
Eliminates an open-coded read and also gains the retry behaviour of
intel_dp_get_dpcd, which seems like a good idea.
Signed-off-by: Keith Packard kei...@keithp.com
---
drivers/gpu/drm/i915/intel_dp.c |8 +++-
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 41674e1..9f134d2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2015,7 +2015,7 @@ intel_dp_init(struct drm_device *dev, int output_reg)
/* Cache some DPCD data in the eDP case */
if (is_edp(intel_dp)) {
- int ret;
+ bool ret;
u32 pp_on, pp_div;
pp_on = I915_READ(PCH_PP_ON_DELAYS);
@@ -2028,11 +2028,9 @@ intel_dp_init(struct drm_device *dev, int output_reg)
dev_priv-panel_t12 *= 100; /* t12 in 100ms units */
ironlake_edp_panel_vdd_on(intel_dp);
- ret = intel_dp_aux_native_read(intel_dp, DP_DPCD_REV,
-intel_dp-dpcd,
-sizeof(intel_dp-dpcd));
+ ret = intel_dp_get_dpcd(intel_dp);
ironlake_edp_panel_vdd_off(intel_dp);
- if (ret == sizeof(intel_dp-dpcd)) {
+ if (ret) {
if (intel_dp-dpcd[DP_DPCD_REV] = 0x11)
dev_priv-no_aux_handshake =
intel_dp-dpcd[DP_MAX_DOWNSPREAD]
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Now we just have to enable fast link training in the eDP case (and
optionally when we know the DP monitor hasn't changed, just DPMS'd).
--
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: DP_PIPE_ENABLED must check transcoder on CPT
On Mon, 25 Jul 2011 23:36:34 -0700 Keith Packard kei...@keithp.com wrote: Display port pipe selection on CPT is not done with a bit in the output register, rather it is controlled by a couple of bits in the separate transcoder register which indicate which display port output is connected to the transcoder. This patch replaces the simplistic macro DP_PIPE_ENABLED with the rather more complicated function dp_pipe_enabled which checks the output register to see if that is enabled, and then goes on to either check the output register pipe selection bit (on non-CPT) or the transcoder DP selection bits (on CPT). Before this patch, any time the mode of pipe A was changed, any display port outputs on pipe B would get disabled as intel_disable_pch_ports would ensure that the mode setting operation could occur on pipe A without interference from other outputs connected to that pch port Signed-off-by: Keith Packard kei...@keithp.com --- Ah nice catch. I expect one day we'll have all the chipset and PCH differences coded... 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] drm/i915: A selection of display port fixes
On Mon, 2011-07-25 at 23:36 -0700, Keith Packard wrote: [PATCH 1/5] drm/i915: Use dp_detect_common in hotplug helper [PATCH 2/5] drm/i915: Rename i915_dp_detect_common to [PATCH 3/5] drm/i915: In intel_dp_init, replace read of DPCD with These three are simple cleanups to centralize all places where the DPCD block was read from the device. Now everyone shares the same function, and that function retries the reads. Reviewed-by: Adam Jackson a...@redhat.com [PATCH 4/5] drm/i915: Delay 250ms before running the hotplug code I was experimenting with DP hotplugging -- moving the plug in and out of the jack very slowly and discovered that the hotplug interrupt occurred well before or after the link for the aux data channel was connected or disconnected. The result of this was that a sufficiently rapid cycle back through user mode could easily beat the motion of the plug and cause the hotplug detection to get the wrong status. Sticking a 250ms delay before doing anything gives the user sufficient time to actually get the plug connected or disconnected. At the very least this should instead be queue_delayed_work(). But if we're going to delay, can we instead set up PCH_PORT_HOTPLUG to do a 100ms delay for us? I'll try this locally, at any rate. [PATCH 5/5] drm/i915: DP_PIPE_ENABLED must check transcoder on CPT snip Turns out the bug wasn't that the mode setting code was doing it wrong and turning the DP2 output off intentionally as a part of the mode change. Instead, the intel driver was trying to adjust the PCH link for the LVDS1 output and thought it needed to turn the DP2 output off because it mistakenly believed the DP2 output was sharing the same pipe as the LVDS1 output. Just a matter of using the wrong mechanism to detect which pipe the DP2 output was connected to. Reviewed-by: Adam Jackson a...@redhat.com - ajax signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing
On Tue, 26 Jul 2011 08:23:13 -0700 Keith Packard kei...@keithp.com wrote: On Tue, 26 Jul 2011 09:24:39 +0200, Daniel Vetter dan...@ffwll.ch wrote: Two things I've noticed: - Why not dev-mode_config.mutex? You're right, of course. I noticed that just after posting that version and updated it; the updated version is on my drm-intel-fixes branch already (having been reviewed by Jesse). - And a nitpick: Why the dev_priv-dev indirection, when dev is already lying around? All nicely cleaned up by using mode_config-mutex instead :-) Thanks for looking it over! I'd like to amend my reviewed by and say the lock shouldn't be held around the call to the drm helper function. It queues some work that also takes the mode config lock, which will break. So you can drop it before that... Previously I had only checked our internal driver callbacks but missed that the lock was held across the helper too. -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915/dp: Use auxch precharge value of 5 everywhere
The default in the Sandybridge docs is 5, as on Ironlake, and I have no
reason to believe 3 would work any better.
Signed-off-by: Adam Jackson a...@redhat.com
---
drivers/gpu/drm/i915/intel_dp.c |7 +--
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3ad90f6..875da4e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -290,7 +290,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
int recv_bytes;
uint32_t status;
uint32_t aux_clock_divider;
- int try, precharge;
+ int try, precharge = 5;
/* The clock divider is based off the hrawclk,
* and would like to run at 2MHz. So, take the
@@ -309,11 +309,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
else
aux_clock_divider = intel_hrawclk(dev) / 2;
- if (IS_GEN6(dev))
- precharge = 3;
- else
- precharge = 5;
-
if (I915_READ(ch_ctl) DP_AUX_CH_CTL_SEND_BUSY) {
DRM_ERROR(dp_aux_ch not started status 0x%08x\n,
I915_READ(ch_ctl));
--
1.7.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/pch: Save/restore PCH_PORT_HOTPLUG across suspend
On Tue, 26 Jul 2011 16:53:06 -0400
Adam Jackson a...@redhat.com wrote:
At least on a Lenovo X220 the HPD bits of this are enabled at boot but
cleared after resume, which means plug interrupts stop working.
This also happens to fix DP displays re-lighting on resume. I'm quite
certain that's an accident: the first DP link train inevitably fails on
that machine, and it's only serendipity that we're getting multiple plug
interrupts and the second train works. But I shall take my victories
where I get them.
Signed-off-by: Adam Jackson a...@redhat.com
---
drivers/gpu/drm/i915/i915_drv.h |1 +
drivers/gpu/drm/i915/i915_suspend.c |2 ++
2 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cf30f99..6b93fa5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -541,6 +541,7 @@ typedef struct drm_i915_private {
u32 savePIPEB_LINK_M1;
u32 savePIPEB_LINK_N1;
u32 saveMCHBAR_RENDER_STANDBY;
+ u32 savePCH_PORT_HOTPLUG;
struct {
/** Bridge to intel-gtt-ko */
diff --git a/drivers/gpu/drm/i915/i915_suspend.c
b/drivers/gpu/drm/i915/i915_suspend.c
index 5257cfc..27693c0 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -814,6 +814,7 @@ int i915_save_state(struct drm_device *dev)
dev_priv-saveFDI_RXB_IMR = I915_READ(_FDI_RXB_IMR);
dev_priv-saveMCHBAR_RENDER_STANDBY =
I915_READ(RSTDBYCTL);
+ dev_priv-savePCH_PORT_HOTPLUG = I915_READ(PCH_PORT_HOTPLUG);
} else {
dev_priv-saveIER = I915_READ(IER);
dev_priv-saveIMR = I915_READ(IMR);
@@ -865,6 +866,7 @@ int i915_restore_state(struct drm_device *dev)
I915_WRITE(GTIMR, dev_priv-saveGTIMR);
I915_WRITE(_FDI_RXA_IMR, dev_priv-saveFDI_RXA_IMR);
I915_WRITE(_FDI_RXB_IMR, dev_priv-saveFDI_RXB_IMR);
+ I915_WRITE(PCH_PORT_HOTPLUG, dev_priv-savePCH_PORT_HOTPLUG);
} else {
I915_WRITE(IER, dev_priv-saveIER);
I915_WRITE(IMR, dev_priv-saveIMR);
We should be writing this reg. The only question is whether we should
be trusting the BIOS values (which may have custom pulse duration
settings) or just unconditionally enabling hot plug for ports we care
about with the default 2ms pulse width (per the DP spec).
If we assume the BIOS programs this reg to a good value (a very big
assumption) saving and restoring it is safest. I just wonder if we'll
find machines where it's broken by default leading to weird DP behavior.
--
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
