2011-02-23 23:17:43

by Dave Airlie

[permalink] [raw]
Subject: [git pull] drm fixes


Nothing too major,

Two regression fixers (one revert that got fixes properly elsewhere), some
timestamp fixes and an agp module reload fix.

Dave.

The following changes since commit d8204a37baf5474d3154eb536c936369be2bd5c0:

Merge branch 'urgent' of git://git.kernel.org/pub/scm/linux/kernel/git/brodo/pcmcia-2.6 (2011-02-22 09:26:54 -0800)

are available in the git repository at:

ssh://master.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6.git drm-fixes

Alex Deucher (1):
Revert "drm/radeon/kms: switch back to min->max pll post divider iteration"

Dave Airlie (2):
drm/radeon/kms: align height of fb allocation.
drm/radeon: fix regression with AA resolve checking

Florian Mickler (1):
amd64-agp: fix crash at second module load

Mario Kleiner (3):
drm/vblank: Use abs64(diff_ns) for s64 diff_ns instead of abs(diff_ns)
drm/vblank: Use memory barriers optimized for atomic_t instead of generics.
drm/vblank: Enable precise vblank timestamps for interlaced and doublescan modes.

Paul Bolle (1):
drm: drop commented out code and preceding comment

drivers/char/agp/amd64-agp.c | 9 +++++++--
drivers/gpu/drm/drm_irq.c | 26 ++++++++++++++------------
drivers/gpu/drm/radeon/r100.c | 4 +---
drivers/gpu/drm/radeon/radeon_display.c | 2 +-
drivers/gpu/drm/radeon/radeon_fb.c | 5 ++++-
5 files changed, 27 insertions(+), 19 deletions(-)


2011-02-23 23:58:54

by Linus Torvalds

[permalink] [raw]
Subject: Re: [git pull] drm fixes

On Wed, Feb 23, 2011 at 3:17 PM, Dave Airlie <[email protected]> wrote:
>
> Nothing too major,
>
> Two regression fixers (one revert that got fixes properly elsewhere), some
> timestamp fixes and an agp module reload fix.

Pulled. However, what about the report from Pavel Machek <[email protected]>:

> > drm/i915: Completely disable fence pipelining.
>
> Reverting this commit helps in v2.6.38-rc5+ when screen is not fully updated,
> or has a corrupted picture like horizontal black or white stripes. Using a
> compositor like compiz may help to avoid the problem.
>
> See bug https://bugzilla.kernel.org/show_bug.cgi?id=27572

Any update on that one? There's that whole "return -EINVAL for
I915_PARAM_HAS_RELAXED_FENCING" patch thing?

Linus

2011-02-24 00:13:59

by Chris Wilson

[permalink] [raw]
Subject: Re: [git pull] drm fixes

On Wed, 23 Feb 2011 15:58:02 -0800, Linus Torvalds <[email protected]> wrote:
> On Wed, Feb 23, 2011 at 3:17 PM, Dave Airlie <[email protected]> wrote:
> >
> > Nothing too major,
> >
> > Two regression fixers (one revert that got fixes properly elsewhere), some
> > timestamp fixes and an agp module reload fix.
>
> Pulled. However, what about the report from Pavel Machek <[email protected]>:
>
> > > drm/i915: Completely disable fence pipelining.
> >
> > Reverting this commit helps in v2.6.38-rc5+ when screen is not fully updated,
> > or has a corrupted picture like horizontal black or white stripes. Using a
> > compositor like compiz may help to avoid the problem.
> >
> > See bug https://bugzilla.kernel.org/show_bug.cgi?id=27572
>
> Any update on that one?

No, reverting that will cause just another bug elsewhere. I need to
work out how the gpu is not being flushed with a non-pipelined fence
change.

> There's that whole "return -EINVAL for
> I915_PARAM_HAS_RELAXED_FENCING" patch thing?

As it turns out this is a bug in the userspace components of the stack for
gen2 hardware, with lax kernel side enforcement. Daniel has a fix for both.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre

2011-02-24 08:27:05

by Alex Riesen

[permalink] [raw]
Subject: Re: [git pull] drm fixes

On Thu, Feb 24, 2011 at 01:13, Chris Wilson <[email protected]> wrote:
> On Wed, 23 Feb 2011 15:58:02 -0800, Linus Torvalds <[email protected]> wrote:
>> >
>> > See bug https://bugzilla.kernel.org/show_bug.cgi?id=27572
>>
>> Any update on that one?
>
> No, reverting that will cause just another bug elsewhere. I need to
> work out how the gpu is not being flushed with a non-pipelined fence
> change.
>
>> There's that whole "return -EINVAL for
>> I915_PARAM_HAS_RELAXED_FENCING" patch thing?
>
> As it turns out this is a bug in the userspace components of the stack for
> gen2 hardware, with lax kernel side enforcement. Daniel has a fix for both.

Chris, could you point us at the patch? I ask because Daniel left a
comment in bug discussion that we should ignore some patch from him,
and there was no mention of anything else.

I'll gladly test a better fix.

2011-02-24 09:19:07

by Indan Zupancic

[permalink] [raw]
Subject: Re: [git pull] drm fixes

On Thu, February 24, 2011 09:27, Alex Riesen wrote:
> On Thu, Feb 24, 2011 at 01:13, Chris Wilson <[email protected]> wrote:
>> On Wed, 23 Feb 2011 15:58:02 -0800, Linus Torvalds <[email protected]>
>> wrote:
>>> >
>>> > See bug https://bugzilla.kernel.org/show_bug.cgi?id=27572
>>>
>>> Any update on that one?
>>
>> No, reverting that will cause just another bug elsewhere. I need to
>> work out how the gpu is not being flushed with a non-pipelined fence
>> change.
>>
>>> There's that whole "return -EINVAL for
>>> I915_PARAM_HAS_RELAXED_FENCING" patch thing?
>>
>> As it turns out this is a bug in the userspace components of the stack for
>> gen2 hardware, with lax kernel side enforcement. Daniel has a fix for both.
>
> Chris, could you point us at the patch? I ask because Daniel left a
> comment in bug discussion that we should ignore some patch from him,
> and there was no mention of anything else.
>
> I'll gladly test a better fix.

See:

http://lists.freedesktop.org/archives/dri-devel/2011-February/008658.html
https://lkml.org/lkml/2011/2/23/34

2011-02-24 19:04:07

by Alex Riesen

[permalink] [raw]
Subject: Re: [git pull] drm fixes

On Thu, Feb 24, 2011 at 10:18, Indan Zupancic <[email protected]> wrote:
>>>
>>> As it turns out this is a bug in the userspace components of the stack for
>>> gen2 hardware, with lax kernel side enforcement. Daniel has a fix for both.
>>
>> Chris, could you point us at the patch? I ask because Daniel left a
>> comment in bug discussion that we should ignore some patch from him,
>> and there was no mention of anything else.
>>
>> I'll gladly test a better fix.
>
> See:
>
> http://lists.freedesktop.org/archives/dri-devel/2011-February/008658.html

This is precisely the link on which Daniel commented:

Comment #35 From Daniel Vetter 2011-02-23 10:38:25

> --- Comment #34 from Indan <[email protected]> 2011-02-23 01:53:25 ---
> Daniel has a real fix at:
> http://lists.freedesktop.org/archives/dri-devel/2011-February/008658.html

You can safely ignore this patch. It only fixes a very special corruption due
to relaxed tiling. This kind of corruption manifests itself in garbage in the
lower-left corner of pixmaps (think ui elements) if and only if the height
rounded up to the next multiple of 8 is not a multiple of 16. Your
corruptions look different.

[lower-left corner means: at most 8 pixels high, at most half the width of
the total pixmap]

And yes, it does not help at all to fix the corruption in the ticket.

> https://lkml.org/lkml/2011/2/23/34

This is just the discussion about the problem described in the ticket.
It does not even mention the patch from the previous link, BTW.
It does have the patch which returns -EINVAL for I915_PARAM_HAS_RELAXED_FENCING,
though.

So, AFAICS, at the moment there is no better patch than this:

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 17bd766..8f8a6a3 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -764,7 +764,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
break;
case I915_PARAM_HAS_RELAXED_FENCING:
value = 1;
- break;
+ return -EINVAL;
case I915_PARAM_HAS_COHERENT_RINGS:
value = 1;
break;

2011-02-24 19:25:22

by Alex Riesen

[permalink] [raw]
Subject: Re: [git pull] drm fixes

On Thu, Feb 24, 2011 at 20:04, Alex Riesen <[email protected]> wrote:
> So, AFAICS, at the moment there is no better patch than this:
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 17bd766..8f8a6a3 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -764,7 +764,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
>                break;
>        case I915_PARAM_HAS_RELAXED_FENCING:
>                value = 1;
> -               break;
> +               return -EINVAL;
>        case I915_PARAM_HAS_COHERENT_RINGS:
>                value = 1;
>                break;
>

Probably unrelated, but I managed to get a cursor corruption with
just this patch. So maybe, I still need that fix from Daniel: an X
cursor seem to meet that pixmap size requirements. And corruption
picture fits too: stripes and dots in lower left corner of where
a part of the cursor image should have been.

Can't reproduce, though.

2011-02-24 23:29:53

by Indan Zupancic

[permalink] [raw]
Subject: Re: [git pull] drm fixes

On Thu, February 24, 2011 20:04, Alex Riesen wrote:
> On Thu, Feb 24, 2011 at 10:18, Indan Zupancic <[email protected]> wrote:
>>>>
>>>> As it turns out this is a bug in the userspace components of the stack for
>>>> gen2 hardware, with lax kernel side enforcement. Daniel has a fix for both.
>>>
>>> Chris, could you point us at the patch? I ask because Daniel left a
>>> comment in bug discussion that we should ignore some patch from him,
>>> and there was no mention of anything else.
>>>
>>> I'll gladly test a better fix.
>>
>> See:
>>
>> http://lists.freedesktop.org/archives/dri-devel/2011-February/008658.html
>
> This is precisely the link on which Daniel commented:
[...]
> And yes, it does not help at all to fix the corruption in the ticket.

Because there are two (or three) corruption problems! One is fixed by
Daniels's patch, the other isn't fixed yet AFAIC.

You're grumbling that I'm giving you information about the fix for one
problem, the one Chris was talking about and you replied to, while you're
interested in the other one (or both).

>> https://lkml.org/lkml/2011/2/23/34
>
> This is just the discussion about the problem described in the ticket.
> It does not even mention the patch from the previous link, BTW.
> It does have the patch which returns -EINVAL for I915_PARAM_HAS_RELAXED_FENCING,
> though.
>
> So, AFAICS, at the moment there is no better patch than this:
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 17bd766..8f8a6a3 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -764,7 +764,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
> break;
> case I915_PARAM_HAS_RELAXED_FENCING:
> value = 1;
> - break;
> + return -EINVAL;
> case I915_PARAM_HAS_COHERENT_RINGS:
> value = 1;
> break;

Read those above links again! Daniel's patch fixes that one corruption, the
above snippet has the same effect and works around the same bug, but neither
do fix that other corruption mentioned in the ticket.

Greetings,

Indan

2011-02-25 10:18:27

by Alex Riesen

[permalink] [raw]
Subject: Re: [git pull] drm fixes

On Fri, Feb 25, 2011 at 00:29, Indan Zupancic <[email protected]> wrote:
>>> https://lkml.org/lkml/2011/2/23/34
>>
>> This is just the discussion about the problem described in the ticket.
>> It does not even mention the patch from the previous link, BTW.
>> It does have the patch which returns -EINVAL for I915_PARAM_HAS_RELAXED_FENCING,
>> though.
>>
>> So, AFAICS, at the moment there is no better patch than this:
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 17bd766..8f8a6a3 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -764,7 +764,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
>>               break;
>>       case I915_PARAM_HAS_RELAXED_FENCING:
>>               value = 1;
>> -             break;
>> +             return -EINVAL;
>>       case I915_PARAM_HAS_COHERENT_RINGS:
>>               value = 1;
>>               break;
>
> Read those above links again! Daniel's patch fixes that one corruption, the
> above snippet has the same effect and works around the same bug, but neither
> do fix that other corruption mentioned in the ticket.

Do you have any idea which patch fixes what? It's just I slowly begin to
doubt that you do. So far I found only two patches (and three changes):

This is the first:

Corruption caused by portions of the screen stopped updating (the Bug 27572).
Certainly worked around by returning -EINVAL for ..RELAXED_FENCING.

The second:

Corruption in small pixmaps, which has no bug number, and commented on by
Daniel in the Bug 27572 as being not the case there.
Fixed by his patch posted to dri-devel "fix corruptions on i8xx due to
relaxed fencing". It may be related to the Bug 27572, but it certainly
does not fix the problem.

There is also this change from the first Daniel's patch which I don't
know what to think about:

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cf4f74c..2e6b532 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1404,6 +1404,8 @@ i915_gem_get_unfenced_gtt_alignment(struct
drm_i915_gem_object *obj)
struct drm_device *dev = obj->base.dev;
int tile_height;

+ return i915_gem_get_gtt_alignment(obj);
+
/*
* Minimum alignment is 4k (GTT page size) for sane hw.
*/diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c

Now, may I ask you , Indan, to shut up for while and let the developers
speak? Because the matter is becoming a little bit confusing, and not without
your help.

2011-02-25 11:07:25

by Indan Zupancic

[permalink] [raw]
Subject: Re: [git pull] drm fixes

On Fri, February 25, 2011 11:18, Alex Riesen wrote:
> On Fri, Feb 25, 2011 at 00:29, Indan Zupancic <[email protected]> wrote:
>>>> https://lkml.org/lkml/2011/2/23/34
>>>
>>> This is just the discussion about the problem described in the ticket.
>>> It does not even mention the patch from the previous link, BTW.
>>> It does have the patch which returns -EINVAL for I915_PARAM_HAS_RELAXED_FENCING,
>>> though.
>>>
>>> So, AFAICS, at the moment there is no better patch than this:
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>>> index 17bd766..8f8a6a3 100644
>>> --- a/drivers/gpu/drm/i915/i915_dma.c
>>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>>> @@ -764,7 +764,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
>>>               break;
>>>       case I915_PARAM_HAS_RELAXED_FENCING:
>>>               value = 1;
>>> -             break;
>>> +             return -EINVAL;
>>>       case I915_PARAM_HAS_COHERENT_RINGS:
>>>               value = 1;
>>>               break;
>>
>> Read those above links again! Daniel's patch fixes that one corruption, the
>> above snippet has the same effect and works around the same bug, but neither
>> do fix that other corruption mentioned in the ticket.
>
> Do you have any idea which patch fixes what? It's just I slowly begin to
> doubt that you do.

There is none yet. Reverting "drm/i915: Completely disable fence pipelining."
fixes it, but causes other problems, quoting Chris:

"No, reverting that will cause just another bug elsewhere. I need to
work out how the gpu is not being flushed with a non-pipelined fence
change."

> So far I found only two patches (and three changes):
>
> This is the first:
>
> Corruption caused by portions of the screen stopped updating (the Bug 27572).
> Certainly worked around by returning -EINVAL for ..RELAXED_FENCING.

This is fixed in a better way by Daniel's latest patch.

> The second:
>
> Corruption in small pixmaps, which has no bug number, and commented on by
> Daniel in the Bug 27572 as being not the case there.

This is the above mentioned issue Chris is talking about in the quoted part.

> Fixed by his patch posted to dri-devel "fix corruptions on i8xx due to
> relaxed fencing".
> It may be related to the Bug 27572, but it certainly
> does not fix the problem.

No, that didn't fix that corruption. It's confusing because Tomas
first said it did fix it and later said it didn't. I muddles that
bug report with the corruption I saw, I thought maybe it had the
same source.

> There is also this change from the first Daniel's patch which I don't
> know what to think about:
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index cf4f74c..2e6b532 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1404,6 +1404,8 @@ i915_gem_get_unfenced_gtt_alignment(struct
> drm_i915_gem_object *obj)
> struct drm_device *dev = obj->base.dev;
> int tile_height;
>
> + return i915_gem_get_gtt_alignment(obj);
> +
> /*
> * Minimum alignment is 4k (GTT page size) for sane hw.
> */diff --git a/drivers/gpu/drm/i915/i915_dma.c
> b/drivers/gpu/drm/i915/i915_dma.c

That was just a patch to try out for me to pinpoint the source of the bug.
It's irrelevant now, as that bug is fixed by Daniel's patch. And I tested
it and it didn't help anyway, so just ignore it.

> Now, may I ask you , Indan, to shut up for while and let the developers
> speak? Because the matter is becoming a little bit confusing, and not without
> your help.

That's because you're getting frustrated and not reading properly what
everyone writes. Just relax.

Chris seems to be working on it. When he has progress or patches for us to
try I'm sure he'll speak up.

My corruption problems are gone, except for the ones I have without xcompmgr,
but as I get those with old kernels too I think it's a userspace bug. Only
reason I reply to you is so others don't have to. I'm trying to clarify things,
but apparently I'm doing a bad job of it.

Good luck,

Indan

2011-02-25 11:35:18

by Alex Riesen

[permalink] [raw]
Subject: Re: [git pull] drm fixes

On Fri, Feb 25, 2011 at 12:07, Indan Zupancic <[email protected]> wrote:
> On Fri, February 25, 2011 11:18, Alex Riesen wrote:
>> So far I found only two patches (and three changes):
>>
>> This is the first:
>>
>> Corruption caused by portions of the screen stopped updating (the Bug 27572).
>> Certainly worked around by returning -EINVAL for ..RELAXED_FENCING.
>
> This is fixed in a better way by Daniel's latest patch.
>

Precisely which is it? Just copy the text of the patch you have
in mind here. Because I don't fscking understand what are talking
about, man!

2011-02-27 09:46:12

by Pavel Machek

[permalink] [raw]
Subject: Re: [git pull] drm fixes

Hi!

> > Nothing too major,
> >
> > Two regression fixers (one revert that got fixes properly elsewhere), some
> > timestamp fixes and an agp module reload fix.
>
> Pulled. However, what about the report from Pavel Machek <[email protected]>:

Are you sure it was me?

> > > drm/i915: Completely disable fence pipelining.
> >
> > Reverting this commit helps in v2.6.38-rc5+ when screen is not fully updated,
> > or has a corrupted picture like horizontal black or white stripes. Using a
> > compositor like compiz may help to avoid the problem.
> >
> > See bug https://bugzilla.kernel.org/show_bug.cgi?id=27572

I don't see myself there, nor do I remember having that problem.

DRM_I915_KMS seems to have vesafb... but I'm afraid it always did..
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2011-02-27 10:57:23

by Alex Riesen

[permalink] [raw]
Subject: Re: [git pull] drm fixes

On Fri, Feb 25, 2011 at 12:35, Alex Riesen <[email protected]> wrote:
> On Fri, Feb 25, 2011 at 12:07, Indan Zupancic <[email protected]> wrote:
>> On Fri, February 25, 2011 11:18, Alex Riesen wrote:
>>> So far I found only two patches (and three changes):
>>>
>>> This is the first:
>>>
>>> Corruption caused by portions of the screen stopped updating (the Bug 27572).
>>> Certainly worked around by returning -EINVAL for ..RELAXED_FENCING.
>>
>> This is fixed in a better way by Daniel's latest patch.
>>
>
> Precisely which is it? Just copy the text of the patch you have
> in mind here. Because I don't fscking understand what are talking
> about, man!
>

Whatever is now in the master has fixed this corruption for me.