2009-01-22 04:46:38

by Dave Airlie

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


Hi Linus,

Please pull the 'drm-fixes' branch from
ssh://master.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6.git drm-fixes

This includes a bunch of kms fixes, along one with regression with the
freeing of maps from the multi-master changes, along with make drm build
on MIPS work.

Dave.

drivers/gpu/drm/drm_agpsupport.c | 3 ++-
drivers/gpu/drm/drm_crtc.c | 14 +++++++++-----
drivers/gpu/drm/drm_drv.c | 4 ++++
drivers/gpu/drm/drm_edid.c | 2 +-
drivers/gpu/drm/drm_stub.c | 8 ++++++++
drivers/gpu/drm/i915/i915_dma.c | 7 ++++---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/intel_lvds.c | 13 +++++++++++++
include/drm/drm_crtc.h | 3 ++-
9 files changed, 44 insertions(+), 12 deletions(-)

commit 2906f0258770d3a9c4e65364df8acc904e148bbe
Author: Jesse Barnes <[email protected]>
Date: Tue Jan 20 19:10:54 2009 -0800

drm/i915: Fix cursor physical address choice to match the 2D driver.

Signed-off-by: Jesse Barnes <[email protected]>
Signed-off-by: Eric Anholt <[email protected]>

commit 1bb88edb7a3769992026f34fd648bb459b0469aa
Author: Eric Anholt <[email protected]>
Date: Thu Jan 15 01:16:25 2009 -0800

drm: stash AGP include under the do-we-have-AGP ifdef

This fixes the MIPS with DRM build.

Signed-off-by: Eric Anholt <[email protected]>
Tested-by: Martin Michlmayr <[email protected]>
Signed-off-by: Dave Airlie <[email protected]>

commit 4942f8b23b56a3f9a713d4436338710579329ffc
Author: Jesse Barnes <[email protected]>
Date: Thu Jan 22 22:23:53 2009 +1000

drm: don't whine about not reading EDID data

Make this message a little quieter, since it's common and not necessarily
indicative of a problem.

Signed-off-by: Jesse Barnes <[email protected]>
Signed-off-by: Dave Airlie <[email protected]>

commit 335041ed31d774391d9add49824d05e7d19d93e9
Author: Jesse Barnes <[email protected]>
Date: Thu Jan 22 22:22:06 2009 +1000

drm/i915: hook up LVDS DPMS property

The LVDS output supports DPMS calls, but we never hooked up the property code,
so set property calls didn't actually do anything. Implement a set_property
callback for the LVDS output so that the right thing happens.

Signed-off-by: Jesse Barnes <[email protected]>

commit ed2dd4b0cc1494c27478f4ea8452f68d2037a60c
Author: Jesse Barnes <[email protected]>
Date: Thu Jan 22 22:21:16 2009 +1000

drm/i915: remove unnecessary debug output in KMS init

We don't really need to print out the FB BAR...

Signed-off-by: Jesse Barnes <[email protected]>
Signed-off-by: Dave Airlie <[email protected]>

commit 260883c85611d3a7e27130af9aef15252856e14f
Author: Dave Airlie <[email protected]>
Date: Thu Jan 22 17:58:49 2009 +1000

i915: fix freeing path for gem phys objects.

This off-by-one was pointed out by Jesse Barnes.

Signed-off-by: Dave Airlie <[email protected]>

commit ad2563c2e42fc67b0976aeb70e9f3faf1c1196e8
Author: Jesse Barnes <[email protected]>
Date: Mon Jan 19 17:21:45 2009 +1000

drm: create mode_config idr lock

Create a separate mode_config IDR lock for simplicity. The core DRM
config structures (connector, mode, etc. lists) are still protected by
the mode_config mutex, but the CRTC IDR (used for the various identifier
IDs) is now protected by the mode_config idr_mutex. Simplifies the
locking a bit and removes a warning.

All objects are protected by the config mutex, we may in the future,
split the object further to have reference counts.

Signed-off-by: Jesse Barnes <[email protected]>
Signed-off-by: Dave Airlie <[email protected]>

commit c1ff85d97708550e634fb6fa099c463db90fc40d
Author: Dave Airlie <[email protected]>
Date: Mon Jan 19 17:17:58 2009 +1000

drm: fix leak of device mappings since multi-master changes.

Device maps now contain a link to the master that created them, so
when cleaning up the master, remove any maps that are connected to it.
Also delete any remaining maps at driver unload time.

Signed-off-by: Dave Airlie <[email protected]>


2009-01-26 18:24:18

by Linus Torvalds

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



Dave,
you have some odd and slightly git usage model, which shows up in various
commits. Lookie here as an example from comit 335041ed:

Author: Jesse Barnes <[email protected]> 2009-01-22 04:22:06
Committer: Dave Airlie <[email protected]> 2009-01-22 04:22:06

drm/i915: hook up LVDS DPMS property

The LVDS output supports DPMS calls, but we never hooked up the property code,
so set property calls didn't actually do anything. Implement a set_property
callback for the LVDS output so that the right thing happens.

Signed-off-by: Jesse Barnes <[email protected]>

and this has a few issues that trigger my "Dave is doing something wrong"
reaction:

- The signed-off-chain is incomplete from the author to the committer.

A _good_ sign-off will always have the sign-offs from the author and
the committer and everybody in between. This one does not. Clearly
Jesse did sign off on his work, but he is not listed as the committer:
you are. And that means that your sign-off is missing.

- You are clearly lying about dates and/or dropping them.

The dates for authorship and committing are the same, yet the author
and committer are clearly _not_ the same. You can try to convince me
that you committed Jesse's work the same second he sent it to you, but
quite frankly, I don't buy it. End result: you've done something to
drop the date information.

I don't know what tools you use, or what process the patches go through,
but I do know that whatever your process is, it's losing information.
Please fix it.

I've pulled, but I hope I won't have to see these issues in future pull
requests.

Linus

2009-01-26 21:44:51

by Dave Airlie

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

On Tue, Jan 27, 2009 at 4:23 AM, Linus Torvalds
<[email protected]> wrote:
>
>
> Dave,
> you have some odd and slightly git usage model, which shows up in various
> commits. Lookie here as an example from comit 335041ed:
>
> Author: Jesse Barnes <[email protected]> 2009-01-22 04:22:06
> Committer: Dave Airlie <[email protected]> 2009-01-22 04:22:06
>
> drm/i915: hook up LVDS DPMS property
>
> The LVDS output supports DPMS calls, but we never hooked up the property code,
> so set property calls didn't actually do anything. Implement a set_property
> callback for the LVDS output so that the right thing happens.
>
> Signed-off-by: Jesse Barnes <[email protected]>
>
> and this has a few issues that trigger my "Dave is doing something wrong"
> reaction:
>
> - The signed-off-chain is incomplete from the author to the committer.
>
> A _good_ sign-off will always have the sign-offs from the author and
> the committer and everybody in between. This one does not. Clearly
> Jesse did sign off on his work, but he is not listed as the committer:
> you are. And that means that your sign-off is missing.

This one is my fault, I think I just cut-n-pasted the commit message
in my sleep,
and forgot to add my signoff.

>
> - You are clearly lying about dates and/or dropping them.
>
> The dates for authorship and committing are the same, yet the author
> and committer are clearly _not_ the same. You can try to convince me
> that you committed Jesse's work the same second he sent it to you, but
> quite frankly, I don't buy it. End result: you've done something to
> drop the date information

Well I'm not 100% sure what happened for this patch, I suspect,
jbarnes sent patch a week
or two ago, it misapplied against the tree I had currently when
applied with git-am, it didn't work so I hand
applied the patch with patch and then did git commit
--author="jbarnes" as he did write it, I just munged it.

Now I'm unsure how I should best handle this, in a world where I can
devote a lot more time to
maintaining this, I would sent Jesse a mail saying, rebase, he'd reply
with a rebase and I'd apply it,
however I generally find it easier to just fix this stuff up on the
run as its synchronous. Should
I be specifying a date somewhere in the commit message?

Dave.

2009-01-26 21:51:03

by Sam Ravnborg

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

>
> Well I'm not 100% sure what happened for this patch, I suspect,
> jbarnes sent patch a week
> or two ago, it misapplied against the tree I had currently when
> applied with git-am, it didn't work so I hand
> applied the patch with patch and then did git commit
> --author="jbarnes" as he did write it, I just munged it.
>
> Now I'm unsure how I should best handle this, in a world where I can
> devote a lot more time to
> maintaining this, I would sent Jesse a mail saying, rebase, he'd reply
> with a rebase and I'd apply it,
> however I generally find it easier to just fix this stuff up on the
> run as its synchronous. Should
> I be specifying a date somewhere in the commit message?

My simple way to deal with this is to:

0) save the mail
1) fix subject and changelog and add my Signed-off-by:
2) apply the patch somehow
3) replace the origianl patch in the (saved) mail
4) git reset --hard (or patch -p1 -R < saved-mail)
5) git am <saved mail>

There must be easier ways to do so I think.
But the above is what my fingers know to do so I do it
routinely.

Sam

2009-01-26 22:08:57

by Linus Torvalds

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



On Mon, 26 Jan 2009, Sam Ravnborg wrote:

> >
> > Well I'm not 100% sure what happened for this patch, I suspect,
> > jbarnes sent patch a week
> > or two ago, it misapplied against the tree I had currently when
> > applied with git-am, it didn't work so I hand
> > applied the patch with patch and then did git commit
> > --author="jbarnes" as he did write it, I just munged it.
> >
> > Now I'm unsure how I should best handle this, in a world where I can
> > devote a lot more time to
> > maintaining this, I would sent Jesse a mail saying, rebase, he'd reply
> > with a rebase and I'd apply it,
> > however I generally find it easier to just fix this stuff up on the
> > run as its synchronous. Should
> > I be specifying a date somewhere in the commit message?
>
> My simple way to deal with this is to:
>
> 0) save the mail
> 1) fix subject and changelog and add my Signed-off-by:
> 2) apply the patch somehow
> 3) replace the origianl patch in the (saved) mail
> 4) git reset --hard (or patch -p1 -R < saved-mail)
> 5) git am <saved mail>
>
> There must be easier ways to do so I think.

Um. Yes. Something like

git am -s -C1

which ends up requiring just a single line of context for the patch to
apply, exactly like the GNU 'patch' program.

The difference being that GNU patch does that incredibly broken thing by
default, and makes it easy to apply a patch that was already applied quite
by mistake, because it still works. Git will require you to say explicitly
that it's ok to have just a single line of context.

Anyway, on its own I wouldn't have reacted - sure, you can just do the
whole "use patch and then commit as another user" and the times will be
odd, but the *big* issue with Dave's pull-request was that there were
multiple cases of just clearly lost information.

In other words, I don't care about the dates. I don't care _how_ you do
your git commits. And I don't even care if you use the broken "patch"
command that accepts random line noise as a patch, as long as you check it
later.

But I _do_ care when it looks like somebody is using a bad process that
clearly isn't working, and that clearly is dropping things on the floor.
If it's a "odd things happens very occasionally because I had to use a
special sequence for it", that's fine.

So when 3 out of 8 patches were somehow missing information (two without
sign-offs, and the third that had a sign-off but was obviously also done
by the odd sequence that dropped the timestamp), that's no longer just
some occasional issue and is a more endemic process problem.

Linus

2009-01-26 22:20:35

by Linus Torvalds

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



On Mon, 26 Jan 2009, Linus Torvalds wrote:
> >
> > There must be easier ways to do so I think.
>
> Um. Yes. Something like
>
> git am -s -C1
>
> which ends up requiring just a single line of context for the patch to
> apply, exactly like the GNU 'patch' program.
>
> The difference being that GNU patch does that incredibly broken thing by
> default, and makes it easy to apply a patch that was already applied quite
> by mistake, because it still works. Git will require you to say explicitly
> that it's ok to have just a single line of context.

Btw, I _really_ don't want people using -C1 by default. It really is a
broken default, and there is a reason why I dislike GNU patch for applying
almost any patch that makes any sense what-so-ever.

So please work with the stricter git defaults. You can still get patches
that silently apply even if they don't work (there's nothing magical about
three lines of context - a patch may still apply in the wrong place), but
it's a lot harder to get those silent screw-ups.

Then, when the strict model doesn't work, take a look by hand, and try to
figure out why it didn't work. If you decide that you really do want to
apply the patch, and the changes near-by that causes it to not apply any
more were really not relevant and don't affect the behaviour of the patch,
_that_ is when you can use -C1 and --whitespace=fix etc to make it apply
despite not being a 100% match.

So don't use -C1 and whitespace-fixup blindly. That way lies madness. It
may _seem_ to be much easier, and yes, 95% of the time it will do the
right thing, but when it doesn't, it silently does bad things. Please only
use those flags when you've literally spent a few seconds of a real human
brain to look at why they are needed.

Linus

2009-01-26 22:39:06

by Johannes Weiner

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

On Mon, Jan 26, 2009 at 10:52:35PM +0100, Sam Ravnborg wrote:
> >
> > Well I'm not 100% sure what happened for this patch, I suspect,
> > jbarnes sent patch a week
> > or two ago, it misapplied against the tree I had currently when
> > applied with git-am, it didn't work so I hand
> > applied the patch with patch and then did git commit
> > --author="jbarnes" as he did write it, I just munged it.
> >
> > Now I'm unsure how I should best handle this, in a world where I can
> > devote a lot more time to
> > maintaining this, I would sent Jesse a mail saying, rebase, he'd reply
> > with a rebase and I'd apply it,
> > however I generally find it easier to just fix this stuff up on the
> > run as its synchronous. Should
> > I be specifying a date somewhere in the commit message?
>
> My simple way to deal with this is to:
>
> 0) save the mail

1) git am --signoff --interactive mailfile

HTH,
Hannes