[Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Tue, Aug 09, 2011 at 10:43:08AM -0700, Ray Lee wrote: > On Tue, Aug 9, 2011 at 10:40 AM, Kirill Smelkov wrote: > >> If you like, submit a patch. You may now be more up-to-date on those > >> particular code paths than most of the intel-gfx developers. > > > > Ray, I'd agree with you if the topic was about cleanups. > > At this point it is about cleanups unless Keith's patch upthread does > not work for you. Does it or not? I've already wrote two weeks ago it does, but if this needs to be restated one more time here it is: Keith's patch fixes the problem in a sense that X now starts and seemingly works (thanks), but several issues remain to be there imho. I've got the message, if it's ok for intel-gfx to leave them as is - it's ok for me. Peace, Kirill
Re: [Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Tue, Jul 26, 2011 at 05:48:27PM +0400, Kirill Smelkov wrote:
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
Re: [Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Tue, Aug 09, 2011 at 05:00:52PM +0300, Vasily Khoruzhick wrote:
On Tuesday 09 August 2011 15:08:03 Kirill Smelkov wrote:
On Tue, Jul 26, 2011 at 05:48:27PM +0400, Kirill Smelkov wrote:
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
Re: [Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Tue, Aug 09, 2011 at 06:09:57PM +0300, Vasily Khoruzhick wrote:
On Tuesday 09 August 2011 17:47:56 Kirill Smelkov wrote:
On Tue, Aug 09, 2011 at 05:00:52PM +0300, Vasily Khoruzhick wrote:
On Tuesday 09 August 2011 15:08:03 Kirill Smelkov wrote:
On Tue, Jul 26, 2011 at 05:48:27PM +0400, Kirill Smelkov wrote:
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
Re: [Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Tue, Aug 09, 2011 at 07:02:59PM +0300, Vasily Khoruzhick wrote:
On Tuesday 09 August 2011 18:34:46 Kirill Smelkov wrote:
On Tue, Aug 09, 2011 at 06:09:57PM +0300, Vasily Khoruzhick wrote:
On Tuesday 09 August 2011 17:47:56 Kirill Smelkov wrote:
On Tue, Aug 09, 2011 at 05:00:52PM +0300, Vasily Khoruzhick wrote:
On Tuesday 09 August 2011 15:08:03 Kirill Smelkov wrote:
On Tue, Jul 26, 2011 at 05:48:27PM +0400, Kirill Smelkov wrote:
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
Re: [Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Tue, Aug 09, 2011 at 09:56:01AM -0700, Ray Lee wrote: On Tue, Aug 9, 2011 at 9:32 AM, Kirill Smelkov k...@mns.spb.ru wrote: Quite frankly, I don't understand intel-gfx developers attitude: why is it me, just random user who is nitpicking here? Why there is no interest/will to analyze now obviously buggy/duplicate code and fix it? Because they don't have an infinite amount of manpower. Actual bugs hitting actual users take precedence over 'cleanups' which always have a chance of causing regressions, as you're well aware. Code churn for the sake of abstract prettiness is discouraged, as it has a potential cost for little potential gain. If you like, submit a patch. You may now be more up-to-date on those particular code paths than most of the intel-gfx developers. Ray, I'd agree with you if the topic was about cleanups. But here I was talking about copy-pasty commit which introduced regressions and bugs, and if now it's a user dilemma to either clean up it after developers himself, or accept that something is broken because developers lack manpower and so plug things in a hurry increasing entropy, I'd like to remind a good rule, at least to me one more time, not to break things in the first place. I'm not talking about cleanup here. I'm talking about original commit which introduced problems, and that there is no need to clean it up, but better revert and redo properly to avoid subsequent code churn in lots of fixes. Sorry, I won't submit a patch. If there is a need to find/fix/cleanup obvious things after company's developers, I have better things to do, and a todo item to re-evaluate hardware for my next project. Thanks, Kirill ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Tue, Aug 09, 2011 at 10:43:08AM -0700, Ray Lee wrote: On Tue, Aug 9, 2011 at 10:40 AM, Kirill Smelkov k...@mns.spb.ru wrote: If you like, submit a patch. You may now be more up-to-date on those particular code paths than most of the intel-gfx developers. Ray, I'd agree with you if the topic was about cleanups. At this point it is about cleanups unless Keith's patch upthread does not work for you. Does it or not? I've already wrote two weeks ago it does, but if this needs to be restated one more time here it is: Keith's patch fixes the problem in a sense that X now starts and seemingly works (thanks), but several issues remain to be there imho. I've got the message, if it's ok for intel-gfx to leave them as is - it's ok for me. Peace, Kirill ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Wed, Aug 10, 2011 at 10:41:44AM +0100, Alan Cox wrote: Sorry, I won't submit a patch. If there is a need to find/fix/cleanup obvious things after company's developers, I have better things to do, and a todo item to re-evaluate hardware for my next project. You seem confused. If you have a support contract of some form with a Linux supplier or Intel please contact your support. This mailing list isn't for support services. Thanks for clarifying. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Tue, Aug 09, 2011 at 09:56:01AM -0700, Ray Lee wrote: > On Tue, Aug 9, 2011 at 9:32 AM, Kirill Smelkov wrote: > > Quite frankly, I don't understand intel-gfx developers attitude: why is > > it me, just random user who is nitpicking here? Why there is no > > interest/will to analyze now obviously buggy/duplicate code and fix it? > > Because they don't have an infinite amount of manpower.?Actual bugs > hitting actual users take precedence over 'cleanups' which always have > a chance of causing regressions, as you're well aware. Code churn for > the sake of abstract prettiness is discouraged, as it has a potential > cost for little potential gain. > > If you like, submit a patch. You may now be more up-to-date on those > particular code paths than most of the intel-gfx developers. Ray, I'd agree with you if the topic was about cleanups. But here I was talking about copy-pasty commit which introduced regressions and bugs, and if now it's a user dilemma to either "clean up" it after developers himself, or accept that something is broken because developers lack manpower and so plug things in a hurry increasing entropy, I'd like to remind a good rule, at least to me one more time, not to break things in the first place. I'm not talking about cleanup here. I'm talking about original commit which introduced problems, and that there is no need to clean it up, but better revert and redo properly to avoid subsequent code churn in lots of fixes. Sorry, I won't submit a patch. If there is a need to find/fix/cleanup obvious things after company's developers, I have better things to do, and a todo item to re-evaluate hardware for my next project. Thanks, Kirill
[Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Tue, Aug 09, 2011 at 07:02:59PM +0300, Vasily Khoruzhick wrote: > On Tuesday 09 August 2011 18:34:46 Kirill Smelkov wrote: > > On Tue, Aug 09, 2011 at 06:09:57PM +0300, Vasily Khoruzhick wrote: > > > On Tuesday 09 August 2011 17:47:56 Kirill Smelkov wrote: > > > > On Tue, Aug 09, 2011 at 05:00:52PM +0300, Vasily Khoruzhick wrote: > > > > > On Tuesday 09 August 2011 15:08:03 Kirill Smelkov wrote: > > > > > > On Tue, Jul 26, 2011 at 05:48:27PM +0400, Kirill Smelkov wrote: > > > > > > > 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 > > > > > > > > > > > > > > > > > > > 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 > > > > > > > > > Date: Fri, 22 Jul 2011 10:44:39 -0700 > > > >
[Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Tue, Aug 09, 2011 at 06:09:57PM +0300, Vasily Khoruzhick wrote: > On Tuesday 09 August 2011 17:47:56 Kirill Smelkov wrote: > > On Tue, Aug 09, 2011 at 05:00:52PM +0300, Vasily Khoruzhick wrote: > > > On Tuesday 09 August 2011 15:08:03 Kirill Smelkov wrote: > > > > On Tue, Jul 26, 2011 at 05:48:27PM +0400, Kirill Smelkov wrote: > > > > > 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 > > > > > > > > > > > > > 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 > > > > > > > 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 t
[Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Tue, Aug 09, 2011 at 05:00:52PM +0300, Vasily Khoruzhick wrote: > On Tuesday 09 August 2011 15:08:03 Kirill Smelkov wrote: > > On Tue, Jul 26, 2011 at 05:48:27PM +0400, Kirill Smelkov wrote: > > > 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 > > > > 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 > > > > > 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 > > > > > --- > > > > > > > > > > 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/i91
[Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Tue, Jul 26, 2011 at 05:48:27PM +0400, Kirill Smelkov wrote:
> 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
> > > 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
> > > 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
> > > ---
> > > 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 _
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
[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
> > 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
> > 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
> > ---
> > 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
&
Re: [Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Sat, Jul 23, 2011 at 11:10:53AM -0400, Alex Deucher wrote: On Fri, Jul 22, 2011 at 5:31 PM, Kirill Smelkov k...@mns.spb.ru wrote: On Sat, Jul 23, 2011 at 01:08:14AM +0400, Kirill Smelkov wrote: On Fri, Jul 22, 2011 at 01:50:04PM -0700, Keith Packard wrote: You're right, of course -- UMS is a huge wart on the kernel driver at this point, keeping it working while also adding new functionality continues to cause challenges. We tend to expect that most people will run reasonably contemporaneous kernel and user space code, and so three years after the switch, it continues to surprise us when someone actually tries UMS. We are planning upgrade to KMS too. The kernel is upgraded more often compared to userspace, because of already mentioned (thanks!) no regression rule. Userspace is more complex and more work in my context, so it is lagging, but eventually we'll get there. Also wanted to say, that if whole X could be built, like the kernel, from one repo without multirepo-setup tool, with 100% reliable working incremental rebuild, etc... it would be a bit easier to upgrade X too. Sorry for being a bit offtopic, could not resist. I was keeping that though in my head for ~ 2 years already, and now had a chance to mention it. You don't have to rebuild all of X to use KMS. In most cases, you just need to update the ddx for your card. I meant the rebuilt not to use KMS, but general case. To me the kernel has one of the great advantage of being lots of self-consistent code because of being maintained in one repo + good build system + good development process. And as the result it is (relatively) easy to upgrade. Anyway, this is just a note from both kernel and X stranger, so whatever... Kirill ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Sat, Jul 23, 2011 at 11:10:53AM -0400, Alex Deucher wrote: > On Fri, Jul 22, 2011 at 5:31 PM, Kirill Smelkov wrote: > > On Sat, Jul 23, 2011 at 01:08:14AM +0400, Kirill Smelkov wrote: > >> On Fri, Jul 22, 2011 at 01:50:04PM -0700, Keith Packard wrote: > > > >> > You're right, of course -- UMS is a huge wart on the kernel driver at > >> > this point, keeping it working while also adding new functionality > >> > continues to cause challenges. We tend to expect that most people will > >> > run reasonably contemporaneous kernel and user space code, and so three > >> > years after the switch, it continues to surprise us when someone > >> > actually tries UMS. > >> > >> We are planning upgrade to KMS too. The kernel is upgraded more often > >> compared to userspace, because of already mentioned (thanks!) "no > >> regression" rule. Userspace is more complex and more work in my context, > >> so it is lagging, but eventually we'll get there. > > > > Also wanted to say, that if whole X could be built, like the kernel, from > > one > > repo without multirepo-setup tool, with 100% reliable working > > incremental rebuild, etc... it would be a bit easier to upgrade X too. > > > > Sorry for being a bit offtopic, could not resist. I was keeping that > > though in my head for ~ 2 years already, and now had a chance to mention it. > > You don't have to rebuild all of X to use KMS. In most cases, you > just need to update the ddx for your card. I meant the rebuilt not to use KMS, but general case. To me the kernel has one of the great advantage of being lots of self-consistent code because of being maintained in one repo + good build system + good development process. And as the result it is (relatively) easy to upgrade. Anyway, this is just a note from both kernel and X stranger, so whatever... Kirill
Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Fri, Jul 22, 2011 at 01:50:04PM -0700, Keith Packard wrote: > On Sat, 23 Jul 2011 00:23:36 +0400, Kirill Smelkov wrote: > > > What kind of a workaround are you talking about? > > Just reverting the commit -- that makes your machine work, even if it's > wrong for other machines. Yes, I could revert it. But since the driver is reasonably complex, it is better to know what I'm doing and that the change makes sense, especially when it's not "my machine", but lots of target boards located all over the country. That's why I wanted, and imho reasonably, because I did the homework, your feedback - to be not on my own, alone. > > Sorry, to me it all looked like "UMS is being ignored forever". > > You're right, of course -- UMS is a huge wart on the kernel driver at > this point, keeping it working while also adding new functionality > continues to cause challenges. We tend to expect that most people will > run reasonably contemporaneous kernel and user space code, and so three > years after the switch, it continues to surprise us when someone > actually tries UMS. We are planning upgrade to KMS too. The kernel is upgraded more often compared to userspace, because of already mentioned (thanks!) "no regression" rule. Userspace is more complex and more work in my context, so it is lagging, but eventually we'll get there. So I hope some day, when everyone upgrades, UMS support could be "cleaned up" out from the driver. > > I'm out of office till ~ next week's tuesday, and on return I'll try > > to test it on the hardware in question. > > Let me know; I've pushed this patch to my drm-intel-fixes tree on > kernel.org in the meantime; if it does solve the problem, I'd like to > add your Tested-by: line. Yes, sure, I'll let you know the results. Thanks, Kirill
Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
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 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
> 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
> ---
> 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(>active_list);
> INIT_LIST_HEAD(>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.
Thanks again,
Kirill
Re: Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Fri, Jul 22, 2011 at 01:50:04PM -0700, Keith Packard wrote: On Sat, 23 Jul 2011 00:23:36 +0400, Kirill Smelkov k...@mns.spb.ru wrote: What kind of a workaround are you talking about? Just reverting the commit -- that makes your machine work, even if it's wrong for other machines. Yes, I could revert it. But since the driver is reasonably complex, it is better to know what I'm doing and that the change makes sense, especially when it's not my machine, but lots of target boards located all over the country. That's why I wanted, and imho reasonably, because I did the homework, your feedback - to be not on my own, alone. Sorry, to me it all looked like UMS is being ignored forever. You're right, of course -- UMS is a huge wart on the kernel driver at this point, keeping it working while also adding new functionality continues to cause challenges. We tend to expect that most people will run reasonably contemporaneous kernel and user space code, and so three years after the switch, it continues to surprise us when someone actually tries UMS. We are planning upgrade to KMS too. The kernel is upgraded more often compared to userspace, because of already mentioned (thanks!) no regression rule. Userspace is more complex and more work in my context, so it is lagging, but eventually we'll get there. So I hope some day, when everyone upgrades, UMS support could be cleaned up out from the driver. I'm out of office till ~ next week's tuesday, and on return I'll try to test it on the hardware in question. Let me know; I've pushed this patch to my drm-intel-fixes tree on kernel.org in the meantime; if it does solve the problem, I'd like to add your Tested-by: line. Yes, sure, I'll let you know the results. Thanks, Kirill ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
On Sat, Jul 23, 2011 at 01:08:14AM +0400, Kirill Smelkov wrote: On Fri, Jul 22, 2011 at 01:50:04PM -0700, Keith Packard wrote: You're right, of course -- UMS is a huge wart on the kernel driver at this point, keeping it working while also adding new functionality continues to cause challenges. We tend to expect that most people will run reasonably contemporaneous kernel and user space code, and so three years after the switch, it continues to surprise us when someone actually tries UMS. We are planning upgrade to KMS too. The kernel is upgraded more often compared to userspace, because of already mentioned (thanks!) no regression rule. Userspace is more complex and more work in my context, so it is lagging, but eventually we'll get there. Also wanted to say, that if whole X could be built, like the kernel, from one repo without multirepo-setup tool, with 100% reliable working incremental rebuild, etc... it would be a bit easier to upgrade X too. Sorry for being a bit offtopic, could not resist. I was keeping that though in my head for ~ 2 years already, and now had a chance to mention it. Thanks, Kirill ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
[ Cc'ing Florian Mickler and Keith Packard ]
On Tue, Jul 12, 2011 at 09:07:47PM +0300, Pekka Enberg wrote:
> On Tue, Jul 12, 2011 at 8:17 PM, Kirill Smelkov wrote:
> > On Sat, May 28, 2011 at 05:19:20PM +0400, Kirill Smelkov wrote:
> >> Hello Chris, everyone,
> >>
> >> On Sat, May 21, 2011 at 04:40:17PM +0100, Chris Wilson wrote:
> >> > On Sat, 21 May 2011 11:23:53 -0400, "Luke-Jr" wrote:
> >> > > On Saturday, May 21, 2011 4:41:45 AM Chris Wilson wrote:
> >> > > > On Fri, 20 May 2011 11:08:56 -0700, Ray Lee >> > > > madrabbit.org> wrote:
> >> > > > > [ Adding Chris Wilson (author of the problematic patch) and Rafael
> >> > > > > Wysocki to the message ]
> >> > > > >
> >> > > > > On Fri, May 20, 2011 at 10:06 AM, Luke-Jr
> >> > > > > wrote:
> >> > > > > > I submitted https://bugzilla.kernel.org/show_bug.cgi?id=33662 a
> >> > > > > > month
> >> > > > > > ago against 2.6.38. Now 2.6.39 was just released without the
> >> > > > > > regression being addressed. This bug makes the system
> >> > > > > > unusable... Some
> >> > > > > > guys on IRC suggested I
> >> > > > > > email, so here it is.
> >> > > > >
> >> > > > > See the bugzilla entry for the bisection history.
> >> > > >
> >> > > > Which has nothing to do with Luke's bug. Considering the thousand
> >> > > > things
> >> > > > that can go wrong during X starting, without a hint as to which it
> >> > > > is nigh
> >> > > > on impossible to debug except by trial and error. If you set up
> >> > > > netconsole, does the kernel emit an OOPS with it's last dying breath?
> >> > >
> >> > > Why assume it's a different bug? I would almost wonder if it might
> >> > > affect
> >> > > all Sandy Bridge GPUs. In any case, I no longer have the original
> >> > > motherboard (it was recalled, as I said in the first post), nor even
> >> > > the
> >> > > revision of it (it had other issues that weren't being fixed). I
> >> > > *assume* I
> >> > > will have the same problem with my new motherboard (Intel DQ67SW), but
> >> > > I
> >> > > haven't verified that yet. I'll be sure to try a netconsole when I
> >> > > have to
> >> > > reboot next and get a chance to try the most recent 2.6.38 and .39
> >> > > kernels,
> >> > > but at the moment it seems reasonable to address the problem bisected
> >> > > in the
> >> > > bug, even if it turns out to be different.
> >> >
> >> > The bisection is into an old DRI1 bug on 945GM. That DRI has inadequate
> >> > locking between release and IRQ and so is prone to such races as befell
> >> > Kirill should not surprise anyone. As neither UMS nor DRI supported SNB,
> >> > I can quite confidently state they are separate bugs.
> >> > -Chris
> >>
> >> I see DRI1 is maybe buggy and old, but still, pre-kms X used to work ok
> >> on kernels < 2.6.38, and starting from 2.6.38 the system is just
> >> unusable because X either crashes the kernel (2.6.38), or does not start
> >> at all (2.6.39):
> >>
> >> https://bugzilla.kernel.org/show_bug.cgi?id=36052
> >>
> >>
> >> It's a regression. It's blocking me to upgrade to newer kernels. I've
> >> done my homework -- digged it and came with detailed OOPS on netconsole
> >> and bisected to single commit. Could this please be fixed?
> >
> > Silence...
> >
> > Still, reverting the bisected patch helps even for 3.0:
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=36052#c4
>
> Keith, Chris, what's up with this regression from 2.6.38? It seems
> commit e8616b6 ("drm/i915: Initialise ring vfuncs for old DRI paths")
> caused problems on other machines.
Silence again, and not surprising -- I was ringing this bell for 3
months already:
https://bugzilla.kernel.org/show_bug.cgi?id=33662#c10
https://bugzilla.kernel.org/show_bug.cgi?id=36052
(and on the list)
with detailed logs and bisected single patch, without even single reply
from intel-gfx people.
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
Re: Major 2.6.38 / 2.6.39 / 3.0 regression ignored?
[ Cc'ing Florian Mickler and Keith Packard ] On Tue, Jul 12, 2011 at 09:07:47PM +0300, Pekka Enberg wrote: On Tue, Jul 12, 2011 at 8:17 PM, Kirill Smelkov k...@mns.spb.ru wrote: On Sat, May 28, 2011 at 05:19:20PM +0400, Kirill Smelkov wrote: Hello Chris, everyone, On Sat, May 21, 2011 at 04:40:17PM +0100, Chris Wilson wrote: On Sat, 21 May 2011 11:23:53 -0400, Luke-Jr l...@dashjr.org wrote: On Saturday, May 21, 2011 4:41:45 AM Chris Wilson wrote: On Fri, 20 May 2011 11:08:56 -0700, Ray Lee ray...@madrabbit.org wrote: [ Adding Chris Wilson (author of the problematic patch) and Rafael Wysocki to the message ] On Fri, May 20, 2011 at 10:06 AM, Luke-Jr l...@dashjr.org wrote: I submitted https://bugzilla.kernel.org/show_bug.cgi?id=33662 a month ago against 2.6.38. Now 2.6.39 was just released without the regression being addressed. This bug makes the system unusable... Some guys on IRC suggested I email, so here it is. See the bugzilla entry for the bisection history. Which has nothing to do with Luke's bug. Considering the thousand things that can go wrong during X starting, without a hint as to which it is nigh on impossible to debug except by trial and error. If you set up netconsole, does the kernel emit an OOPS with it's last dying breath? Why assume it's a different bug? I would almost wonder if it might affect all Sandy Bridge GPUs. In any case, I no longer have the original motherboard (it was recalled, as I said in the first post), nor even the revision of it (it had other issues that weren't being fixed). I *assume* I will have the same problem with my new motherboard (Intel DQ67SW), but I haven't verified that yet. I'll be sure to try a netconsole when I have to reboot next and get a chance to try the most recent 2.6.38 and .39 kernels, but at the moment it seems reasonable to address the problem bisected in the bug, even if it turns out to be different. The bisection is into an old DRI1 bug on 945GM. That DRI has inadequate locking between release and IRQ and so is prone to such races as befell Kirill should not surprise anyone. As neither UMS nor DRI supported SNB, I can quite confidently state they are separate bugs. -Chris I see DRI1 is maybe buggy and old, but still, pre-kms X used to work ok on kernels 2.6.38, and starting from 2.6.38 the system is just unusable because X either crashes the kernel (2.6.38), or does not start at all (2.6.39): https://bugzilla.kernel.org/show_bug.cgi?id=36052 It's a regression. It's blocking me to upgrade to newer kernels. I've done my homework -- digged it and came with detailed OOPS on netconsole and bisected to single commit. Could this please be fixed? Silence... Still, reverting the bisected patch helps even for 3.0: https://bugzilla.kernel.org/show_bug.cgi?id=36052#c4 Keith, Chris, what's up with this regression from 2.6.38? It seems commit e8616b6 (drm/i915: Initialise ring vfuncs for old DRI paths) caused problems on other machines. Silence again, and not surprising -- I was ringing this bell for 3 months already: https://bugzilla.kernel.org/show_bug.cgi?id=33662#c10 https://bugzilla.kernel.org/show_bug.cgi?id=36052 (and on the list) with detailed logs and bisected single patch, without even single reply from intel-gfx people. 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: On netconsole: # X starts here, then [ 45.102377] [ cut here ] [ 45.102402] WARNING: at lib/iomap.c:43 bad_io_access+0x3d/0x40() [ 45.102411] Hardware name: PCISA-945GSE [ 45.102418] Bad IO access at port 0x84 (return inl(port)) [ 45.102425] Modules linked in: [ 45.102438] Pid: 2846, comm: sshd Not tainted 3.0.0--NAVY #33 [ 45.102445] Call Trace: [ 45.102460] [c118e9fd] ? bad_io_access+0x3d/0x40 [ 45.102473] [c10287ec] warn_slowpath_common+0x6c/0xa0 [ 45.102484] [c118e9fd] ? bad_io_access+0x3d/0x40 [ 45.102495] [c102889e] warn_slowpath_fmt+0x2e/0x30 [ 45.102506] [c118e9fd] bad_io_access+0x3d/0x40 [ 45.102516] [c118edb2] ioread32+0x22/0x40 [ 45.102528] [c122cc7d] i915_driver_irq_handler+0x1ad/0x660 [ 45.102541] [c12c6a7e] ? rtl8169_interrupt+0xee/0x370 [ 45.102554] [c105c396] handle_irq_event_percpu+0x36/0x140 [ 45.102565] [c105e490] ? handle_edge_irq+0x150/0x150 [ 45.102576] [c105c4d9] handle_irq_event+0x39/0x60 [ 45.102587] [c105e4d5] handle_fasteoi_irq+0x45/0xd0 [ 45.102594] IRQ [c1003c29] ? do_IRQ+0x39/0xb0 [ 45.102613] [c103c9b3] ? start_flush_work+0xc3/0x130 [ 45.102625
[Intel-gfx] Major 2.6.38 / 2.6.39 regression ignored?
On Sat, May 28, 2011 at 05:19:20PM +0400, Kirill Smelkov wrote: > Hello Chris, everyone, > > On Sat, May 21, 2011 at 04:40:17PM +0100, Chris Wilson wrote: > > On Sat, 21 May 2011 11:23:53 -0400, "Luke-Jr" wrote: > > > On Saturday, May 21, 2011 4:41:45 AM Chris Wilson wrote: > > > > On Fri, 20 May 2011 11:08:56 -0700, Ray Lee > > > > wrote: > > > > > [ Adding Chris Wilson (author of the problematic patch) and Rafael > > > > > Wysocki to the message ] > > > > > > > > > > On Fri, May 20, 2011 at 10:06 AM, Luke-Jr wrote: > > > > > > I submitted https://bugzilla.kernel.org/show_bug.cgi?id=33662 a > > > > > > month > > > > > > ago against 2.6.38. Now 2.6.39 was just released without the > > > > > > regression being addressed. This bug makes the system unusable... > > > > > > Some > > > > > > guys on IRC suggested I > > > > > > email, so here it is. > > > > > > > > > > See the bugzilla entry for the bisection history. > > > > > > > > Which has nothing to do with Luke's bug. Considering the thousand things > > > > that can go wrong during X starting, without a hint as to which it is > > > > nigh > > > > on impossible to debug except by trial and error. If you set up > > > > netconsole, does the kernel emit an OOPS with it's last dying breath? > > > > > > Why assume it's a different bug? I would almost wonder if it might affect > > > all Sandy Bridge GPUs. In any case, I no longer have the original > > > motherboard (it was recalled, as I said in the first post), nor even the > > > revision of it (it had other issues that weren't being fixed). I *assume* > > > I > > > will have the same problem with my new motherboard (Intel DQ67SW), but I > > > haven't verified that yet. I'll be sure to try a netconsole when I have > > > to > > > reboot next and get a chance to try the most recent 2.6.38 and .39 > > > kernels, > > > but at the moment it seems reasonable to address the problem bisected in > > > the > > > bug, even if it turns out to be different. > > > > The bisection is into an old DRI1 bug on 945GM. That DRI has inadequate > > locking between release and IRQ and so is prone to such races as befell > > Kirill should not surprise anyone. As neither UMS nor DRI supported SNB, > > I can quite confidently state they are separate bugs. > > -Chris > > I see DRI1 is maybe buggy and old, but still, pre-kms X used to work ok > on kernels < 2.6.38, and starting from 2.6.38 the system is just > unusable because X either crashes the kernel (2.6.38), or does not start > at all (2.6.39): > > https://bugzilla.kernel.org/show_bug.cgi?id=36052 > > > It's a regression. It's blocking me to upgrade to newer kernels. I've > done my homework -- digged it and came with detailed OOPS on netconsole > and bisected to single commit. Could this please be fixed? Silence... Still, reverting the bisected patch helps even for 3.0: https://bugzilla.kernel.org/show_bug.cgi?id=36052#c4
Re: [Intel-gfx] Major 2.6.38 / 2.6.39 regression ignored?
On Sat, May 28, 2011 at 05:19:20PM +0400, Kirill Smelkov wrote: Hello Chris, everyone, On Sat, May 21, 2011 at 04:40:17PM +0100, Chris Wilson wrote: On Sat, 21 May 2011 11:23:53 -0400, Luke-Jr l...@dashjr.org wrote: On Saturday, May 21, 2011 4:41:45 AM Chris Wilson wrote: On Fri, 20 May 2011 11:08:56 -0700, Ray Lee ray...@madrabbit.org wrote: [ Adding Chris Wilson (author of the problematic patch) and Rafael Wysocki to the message ] On Fri, May 20, 2011 at 10:06 AM, Luke-Jr l...@dashjr.org wrote: I submitted https://bugzilla.kernel.org/show_bug.cgi?id=33662 a month ago against 2.6.38. Now 2.6.39 was just released without the regression being addressed. This bug makes the system unusable... Some guys on IRC suggested I email, so here it is. See the bugzilla entry for the bisection history. Which has nothing to do with Luke's bug. Considering the thousand things that can go wrong during X starting, without a hint as to which it is nigh on impossible to debug except by trial and error. If you set up netconsole, does the kernel emit an OOPS with it's last dying breath? Why assume it's a different bug? I would almost wonder if it might affect all Sandy Bridge GPUs. In any case, I no longer have the original motherboard (it was recalled, as I said in the first post), nor even the revision of it (it had other issues that weren't being fixed). I *assume* I will have the same problem with my new motherboard (Intel DQ67SW), but I haven't verified that yet. I'll be sure to try a netconsole when I have to reboot next and get a chance to try the most recent 2.6.38 and .39 kernels, but at the moment it seems reasonable to address the problem bisected in the bug, even if it turns out to be different. The bisection is into an old DRI1 bug on 945GM. That DRI has inadequate locking between release and IRQ and so is prone to such races as befell Kirill should not surprise anyone. As neither UMS nor DRI supported SNB, I can quite confidently state they are separate bugs. -Chris I see DRI1 is maybe buggy and old, but still, pre-kms X used to work ok on kernels 2.6.38, and starting from 2.6.38 the system is just unusable because X either crashes the kernel (2.6.38), or does not start at all (2.6.39): https://bugzilla.kernel.org/show_bug.cgi?id=36052 It's a regression. It's blocking me to upgrade to newer kernels. I've done my homework -- digged it and came with detailed OOPS on netconsole and bisected to single commit. Could this please be fixed? Silence... Still, reverting the bisected patch helps even for 3.0: https://bugzilla.kernel.org/show_bug.cgi?id=36052#c4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Revert "drm/i915: Initialise ring vfuncs for old DRI paths"
This reverts commit e8616b6ced6137085e6657cc63bc2fe3900b8616.
See https://bugzilla.kernel.org/show_bug.cgi?id=36052
Cc: Herbert Xu
Cc: Florian Mickler
Cc: Pekka Enberg
Cc: Chris Wilson
Cc: Keith Packard
Cc: stable at kernel.org
---
drivers/gpu/drm/i915/i915_dma.c | 25 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 42 ---
drivers/gpu/drm/i915/intel_ringbuffer.h |3 --
3 files changed, 18 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 296fbd6..9300d18 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -160,7 +160,7 @@ static int i915_initialize(struct drm_device * dev,
drm_i915_init_t * init)
{
drm_i915_private_t *dev_priv = dev->dev_private;
struct drm_i915_master_private *master_priv =
dev->primary->master->driver_priv;
- int ret;
+ struct intel_ring_buffer *ring = LP_RING(dev_priv);
master_priv->sarea = drm_getsarea(dev);
if (master_priv->sarea) {
@@ -171,22 +171,33 @@ static int i915_initialize(struct drm_device * dev,
drm_i915_init_t * init)
}
if (init->ring_size != 0) {
- if (LP_RING(dev_priv)->obj != NULL) {
+ if (ring->obj != NULL) {
i915_dma_cleanup(dev);
DRM_ERROR("Client tried to initialize ringbuffer in "
"GEM mode\n");
return -EINVAL;
}
- ret = intel_render_ring_init_dri(dev,
-init->ring_start,
-init->ring_size);
- if (ret) {
+ ring->size = init->ring_size;
+
+ ring->map.offset = init->ring_start;
+ ring->map.size = init->ring_size;
+ ring->map.type = 0;
+ ring->map.flags = 0;
+ ring->map.mtrr = 0;
+
+ drm_core_ioremap_wc(>map, dev);
+
+ if (ring->map.handle == NULL) {
i915_dma_cleanup(dev);
- return ret;
+ DRM_ERROR("can not ioremap virtual address for"
+ " ring buffer\n");
+ return -ENOMEM;
}
}
+ ring->virtual_start = ring->map.handle;
+
dev_priv->cpp = init->cpp;
dev_priv->back_offset = init->back_offset;
dev_priv->front_offset = init->front_offset;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 95c4b14..8d2f610 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1304,48 +1304,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
return intel_init_ring_buffer(dev, ring);
}
-int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size)
-{
- drm_i915_private_t *dev_priv = dev->dev_private;
- struct intel_ring_buffer *ring = _priv->ring[RCS];
-
- *ring = render_ring;
- if (INTEL_INFO(dev)->gen >= 6) {
- ring->add_request = gen6_add_request;
- ring->irq_get = gen6_render_ring_get_irq;
- ring->irq_put = gen6_render_ring_put_irq;
- } else if (IS_GEN5(dev)) {
- ring->add_request = pc_render_add_request;
- ring->get_seqno = pc_render_get_seqno;
- }
-
- ring->dev = dev;
- INIT_LIST_HEAD(>active_list);
- INIT_LIST_HEAD(>request_list);
- INIT_LIST_HEAD(>gpu_write_list);
-
- ring->size = size;
- ring->effective_size = ring->size;
- if (IS_I830(ring->dev))
- ring->effective_size -= 128;
-
- ring->map.offset = start;
- ring->map.size = size;
- ring->map.type = 0;
- ring->map.flags = 0;
- ring->map.mtrr = 0;
-
- drm_core_ioremap_wc(>map, dev);
- if (ring->map.handle == NULL) {
- DRM_ERROR("can not ioremap virtual address for"
- " ring buffer\n");
- return -ENOMEM;
- }
-
- ring->virtual_start = (void __force __iomem *)ring->map.handle;
- return 0;
-}
-
int intel_init_bsd_ring_buffer(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 39ac2b6..b6b0fd4 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -197,7 +197,4 @@ static inline void i915_trace_irq_get(struct
intel_ring_buffer *ring, u32 seqno)
ring->trace_irq_seqno = seqno;
}
-/* DRI warts */
-int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size);
-
#endif /* _INTEL_RINGBUFFER_H_ */
--
1.7.6.233.gd79bc
Appendix 1. `X -verbose` log
Re: Major 2.6.38 / 2.6.39 regression ignored?
Hello Chris, everyone, On Sat, May 21, 2011 at 04:40:17PM +0100, Chris Wilson wrote: On Sat, 21 May 2011 11:23:53 -0400, Luke-Jr l...@dashjr.org wrote: On Saturday, May 21, 2011 4:41:45 AM Chris Wilson wrote: On Fri, 20 May 2011 11:08:56 -0700, Ray Lee ray...@madrabbit.org wrote: [ Adding Chris Wilson (author of the problematic patch) and Rafael Wysocki to the message ] On Fri, May 20, 2011 at 10:06 AM, Luke-Jr l...@dashjr.org wrote: I submitted https://bugzilla.kernel.org/show_bug.cgi?id=33662 a month ago against 2.6.38. Now 2.6.39 was just released without the regression being addressed. This bug makes the system unusable... Some guys on IRC suggested I email, so here it is. See the bugzilla entry for the bisection history. Which has nothing to do with Luke's bug. Considering the thousand things that can go wrong during X starting, without a hint as to which it is nigh on impossible to debug except by trial and error. If you set up netconsole, does the kernel emit an OOPS with it's last dying breath? Why assume it's a different bug? I would almost wonder if it might affect all Sandy Bridge GPUs. In any case, I no longer have the original motherboard (it was recalled, as I said in the first post), nor even the revision of it (it had other issues that weren't being fixed). I *assume* I will have the same problem with my new motherboard (Intel DQ67SW), but I haven't verified that yet. I'll be sure to try a netconsole when I have to reboot next and get a chance to try the most recent 2.6.38 and .39 kernels, but at the moment it seems reasonable to address the problem bisected in the bug, even if it turns out to be different. The bisection is into an old DRI1 bug on 945GM. That DRI has inadequate locking between release and IRQ and so is prone to such races as befell Kirill should not surprise anyone. As neither UMS nor DRI supported SNB, I can quite confidently state they are separate bugs. -Chris I see DRI1 is maybe buggy and old, but still, pre-kms X used to work ok on kernels 2.6.38, and starting from 2.6.38 the system is just unusable because X either crashes the kernel (2.6.38), or does not start at all (2.6.39): https://bugzilla.kernel.org/show_bug.cgi?id=36052 It's a regression. It's blocking me to upgrade to newer kernels. I've done my homework -- digged it and came with detailed OOPS on netconsole and bisected to single commit. Could this please be fixed? Thanks, Kirill ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Major 2.6.38 / 2.6.39 regression ignored?
Hello Chris, everyone, On Sat, May 21, 2011 at 04:40:17PM +0100, Chris Wilson wrote: > On Sat, 21 May 2011 11:23:53 -0400, "Luke-Jr" wrote: > > On Saturday, May 21, 2011 4:41:45 AM Chris Wilson wrote: > > > On Fri, 20 May 2011 11:08:56 -0700, Ray Lee > > > wrote: > > > > [ Adding Chris Wilson (author of the problematic patch) and Rafael > > > > Wysocki to the message ] > > > > > > > > On Fri, May 20, 2011 at 10:06 AM, Luke-Jr wrote: > > > > > I submitted https://bugzilla.kernel.org/show_bug.cgi?id=33662 a month > > > > > ago against 2.6.38. Now 2.6.39 was just released without the > > > > > regression being addressed. This bug makes the system unusable... Some > > > > > guys on IRC suggested I > > > > > email, so here it is. > > > > > > > > See the bugzilla entry for the bisection history. > > > > > > Which has nothing to do with Luke's bug. Considering the thousand things > > > that can go wrong during X starting, without a hint as to which it is nigh > > > on impossible to debug except by trial and error. If you set up > > > netconsole, does the kernel emit an OOPS with it's last dying breath? > > > > Why assume it's a different bug? I would almost wonder if it might affect > > all Sandy Bridge GPUs. In any case, I no longer have the original > > motherboard (it was recalled, as I said in the first post), nor even the > > revision of it (it had other issues that weren't being fixed). I *assume* I > > will have the same problem with my new motherboard (Intel DQ67SW), but I > > haven't verified that yet. I'll be sure to try a netconsole when I have to > > reboot next and get a chance to try the most recent 2.6.38 and .39 kernels, > > but at the moment it seems reasonable to address the problem bisected in > > the > > bug, even if it turns out to be different. > > The bisection is into an old DRI1 bug on 945GM. That DRI has inadequate > locking between release and IRQ and so is prone to such races as befell > Kirill should not surprise anyone. As neither UMS nor DRI supported SNB, > I can quite confidently state they are separate bugs. > -Chris I see DRI1 is maybe buggy and old, but still, pre-kms X used to work ok on kernels < 2.6.38, and starting from 2.6.38 the system is just unusable because X either crashes the kernel (2.6.38), or does not start at all (2.6.39): https://bugzilla.kernel.org/show_bug.cgi?id=36052 It's a regression. It's blocking me to upgrade to newer kernels. I've done my homework -- digged it and came with detailed OOPS on netconsole and bisected to single commit. Could this please be fixed? Thanks, Kirill
[PATCH 2/2] drm: Prefix info printk about registering panic notifier with 'drm'
Recently I've studied my system dmesg and seen this:
1 [0.478416] ACPI: Battery Slot [C1B4] (battery present)
2 [0.478648] ACPI: Battery Slot [C1B3] (battery absent)
3 [0.906678] [drm] initialized overlay support
4 [1.762304] Console: switching to colour frame buffer device 128x48
5 [1.765211] fb0: inteldrmfb frame buffer device
6 [1.765242] registered panic notifier
7 [1.765272] [drm] Initialized i915 1.6.0 20080730 for :00:02.0 on
minor 0
8 [1.765372] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
and it was not evident who registered that panic notifier on line 6.
I'd bought it as some low-level stuff needed by kernel itself, but the
time was inappropriate -- too late for such things.
So I had to study sources to see it was drm who was registering
switch-to-fb on panic.
Let's avoid possible confusion and mark this message as going from drm
subsystem.
(I'm a bit unsure whether to use '[drm]:' or 'drm:' -- the rest of the
kernel just uses 'topic:', and even in drm_fb_helper.c we use 'fb%d:'
without [] brackets. Either way is ok with me.)
Signed-off-by: Kirill Smelkov
---
drivers/gpu/drm/drm_fb_helper.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 288ea2f..90f1331 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -912,7 +912,7 @@ int drm_fb_helper_single_fb_probe(struct drm_device *dev,
/* Switch back to kernel console on panic */
/* multi card linked list maybe */
if (list_empty(_fb_helper_list)) {
- printk(KERN_INFO "registered panic notifier\n");
+ printk(KERN_INFO "drm: registered panic notifier\n");
atomic_notifier_chain_register(_notifier_list,
);
register_sysrq_key('v', _drm_fb_helper_restore_op);
@@ -926,7 +926,7 @@ void drm_fb_helper_free(struct drm_fb_helper *helper)
{
list_del(>kernel_fb_list);
if (list_empty(_fb_helper_list)) {
- printk(KERN_INFO "unregistered panic notifier\n");
+ printk(KERN_INFO "drm: unregistered panic notifier\n");
atomic_notifier_chain_unregister(_notifier_list,
);
unregister_sysrq_key('v', _drm_fb_helper_restore_op);
--
1.7.1.87.gcc8c60.dirty
[PATCH 1/2] drm/radeon: "unregistered panic notifier" printk is redundant
Because drm_fb_helper_free() prints the same. This redundant printk seems to be a forgotten leftover after 785b93ef (drm/kms: move driver specific fb common code to helper functions (v2)) -- let's remove it. Cc: Jerome Glisse Signed-off-by: Kirill Smelkov --- drivers/gpu/drm/radeon/radeon_fb.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index 9ac57a0..2a161d8 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -365,8 +365,6 @@ int radeonfb_remove(struct drm_device *dev, struct drm_framebuffer *fb) framebuffer_release(info); } - printk(KERN_INFO "unregistered panic notifier\n"); - return 0; } EXPORT_SYMBOL(radeonfb_remove); -- 1.7.1.87.gcc8c60.dirty
[PATCH 0/2] drm: registering panic notifier cosmetics
Just some minor tweaks from casual user. Please apply & thanks. Kirill Smelkov (2): drm/radeon: "unregistered panic notifier" printk is redundant drm: Prefix info printk about registering panic notifier with 'drm' drivers/gpu/drm/drm_fb_helper.c|4 ++-- drivers/gpu/drm/radeon/radeon_fb.c |2 -- 2 files changed, 2 insertions(+), 4 deletions(-)
[PATCH 1/2] drm/radeon: unregistered panic notifier printk is redundant
Because drm_fb_helper_free() prints the same. This redundant printk seems to be a forgotten leftover after 785b93ef (drm/kms: move driver specific fb common code to helper functions (v2)) -- let's remove it. Cc: Jerome Glisse jgli...@redhat.com Signed-off-by: Kirill Smelkov k...@landau.phys.spbu.ru --- drivers/gpu/drm/radeon/radeon_fb.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index 9ac57a0..2a161d8 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -365,8 +365,6 @@ int radeonfb_remove(struct drm_device *dev, struct drm_framebuffer *fb) framebuffer_release(info); } - printk(KERN_INFO unregistered panic notifier\n); - return 0; } EXPORT_SYMBOL(radeonfb_remove); -- 1.7.1.87.gcc8c60.dirty ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] drm: registering panic notifier cosmetics
Just some minor tweaks from casual user. Please apply thanks. Kirill Smelkov (2): drm/radeon: unregistered panic notifier printk is redundant drm: Prefix info printk about registering panic notifier with 'drm' drivers/gpu/drm/drm_fb_helper.c|4 ++-- drivers/gpu/drm/radeon/radeon_fb.c |2 -- 2 files changed, 2 insertions(+), 4 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
