Re: [PATCH 1/3] drm: fix print format of sequence in trace point

[Chris Wilson]

On Mon, Jul 01, 2013 at 07:06:31PM +0900, Seung-Woo Kim wrote:
 seq of a trace point is unsigned int but print format was %d. So
 it fixes the format as %u even the format can be not used.

I don't understand what you mean here. The patch itself looks fine.
 
 Signed-off-by: Seung-Woo Kim sw0312@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm: fix print format of sequence in trace point

[Seung-Woo Kim]

Hello Chris,

Thank you for reviewing.

On 2013년 07월 01일 19:23, Chris Wilson wrote:
 On Mon, Jul 01, 2013 at 07:06:31PM +0900, Seung-Woo Kim wrote:
 seq of a trace point is unsigned int but print format was %d. So
 it fixes the format as %u even the format can be not used.
 
 I don't understand what you mean here. The patch itself looks fine.

I can not find where the format is used or not, so I think the format is
not really used anywhere. If you want to fix the commit-msg, I'll update
and re-send this patch.

Regards,
- Seung-Woo Kim

  
 Signed-off-by: Seung-Woo Kim sw0312@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
 -Chris
 

-- 
Seung-Woo Kim
Samsung Software RD Center
--

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm: fix print format of sequence in trace point

[Chris Wilson]

On Mon, Jul 01, 2013 at 07:28:49PM +0900, Seung-Woo Kim wrote:
 Hello Chris,
 
 Thank you for reviewing.
 
 On 2013년 07월 01일 19:23, Chris Wilson wrote:
  On Mon, Jul 01, 2013 at 07:06:31PM +0900, Seung-Woo Kim wrote:
  seq of a trace point is unsigned int but print format was %d. So
  it fixes the format as %u even the format can be not used.
  
  I don't understand what you mean here. The patch itself looks fine.
 
 I can not find where the format is used or not, so I think the format is
 not really used anywhere. If you want to fix the commit-msg, I'll update
 and re-send this patch.

One of the tricks performed by the TRACE() macro is that it prepends
trace_ to the name of the tracepoint for use in the code.

git grep trace_drm_vblank_event:
drivers/gpu/drm/drm_irq.c:  trace_drm_vblank_event_delivered(e-base.pid, 
e-pipe,
drivers/gpu/drm/drm_irq.c:  trace_drm_vblank_event_queued(current-pid, 
pipe,
drivers/gpu/drm/drm_irq.c:  trace_drm_vblank_event(crtc, seq);
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/3] drm: fix print format of sequence in trace point

[Seung-Woo Kim]

seq of a trace point is unsigned int but print format was %d. So
it fixes the format as %u.

Signed-off-by: Seung-Woo Kim sw0312@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
change from v1
- remove wrong commit messageas Chris commented

 drivers/gpu/drm/drm_trace.h |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_trace.h b/drivers/gpu/drm/drm_trace.h
index 03ea964..27cc95f 100644
--- a/drivers/gpu/drm/drm_trace.h
+++ b/drivers/gpu/drm/drm_trace.h
@@ -21,7 +21,7 @@ TRACE_EVENT(drm_vblank_event,
__entry-crtc = crtc;
__entry-seq = seq;
),
-   TP_printk(crtc=%d, seq=%d, __entry-crtc, __entry-seq)
+   TP_printk(crtc=%d, seq=%u, __entry-crtc, __entry-seq)
 );
 
 TRACE_EVENT(drm_vblank_event_queued,
@@ -37,7 +37,7 @@ TRACE_EVENT(drm_vblank_event_queued,
__entry-crtc = crtc;
__entry-seq = seq;
),
-   TP_printk(pid=%d, crtc=%d, seq=%d, __entry-pid, __entry-crtc, \
+   TP_printk(pid=%d, crtc=%d, seq=%u, __entry-pid, __entry-crtc, \
  __entry-seq)
 );
 
@@ -54,7 +54,7 @@ TRACE_EVENT(drm_vblank_event_delivered,
__entry-crtc = crtc;
__entry-seq = seq;
),
-   TP_printk(pid=%d, crtc=%d, seq=%d, __entry-pid, __entry-crtc, \
+   TP_printk(pid=%d, crtc=%d, seq=%u, __entry-pid, __entry-crtc, \
  __entry-seq)
 );
 
-- 
1.7.4.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/3] drm: fix error routines in drm_open_helper

[Seung-Woo Kim]

From: YoungJun Cho yj44@samsung.com

There are wrong cases to handle error in drm_open_helper().
The priv-minor, assigned by idr_find() which can return NULL,
should be checked whether it is NULL or not before referencing it.
And if an error occurs after executing dev-driver-open() which
allocates driver specific per-file private data, then the private
data should be released.

Signed-off-by: YoungJun Cho yj44@samsung.com
Signed-off-by: Seung-Woo Kim sw0312@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
change from v1
- replace error value for failure to find the minor as ENODEV as Chris commented

 drivers/gpu/drm/drm_fops.c |   17 +
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 429e07d..0470261 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -271,6 +271,11 @@ static int drm_open_helper(struct inode *inode, struct 
file *filp,
priv-uid = current_euid();
priv-pid = get_pid(task_pid(current));
priv-minor = idr_find(drm_minors_idr, minor_id);
+   if (!priv-minor) {
+   ret = -ENODEV;
+   goto out_free;
+   }
+
priv-ioctl_count = 0;
/* for compatibility root is always authenticated */
priv-authenticated = capable(CAP_SYS_ADMIN);
@@ -304,7 +309,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
if (!priv-minor-master) {
mutex_unlock(dev-struct_mutex);
ret = -ENOMEM;
-   goto out_free;
+   goto out_close;
}
 
priv-is_master = 1;
@@ -322,7 +327,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
drm_master_put(priv-minor-master);
drm_master_put(priv-master);
mutex_unlock(dev-struct_mutex);
-   goto out_free;
+   goto out_close;
}
}
mutex_lock(dev-struct_mutex);
@@ -333,7 +338,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
drm_master_put(priv-minor-master);
drm_master_put(priv-master);
mutex_unlock(dev-struct_mutex);
-   goto out_free;
+   goto out_close;
}
}
mutex_unlock(dev-struct_mutex);
@@ -367,7 +372,11 @@ static int drm_open_helper(struct inode *inode, struct 
file *filp,
 #endif
 
return 0;
-  out_free:
+
+out_close:
+   if (dev-driver-postclose)
+   dev-driver-postclose(dev, priv);
+out_free:
kfree(priv);
filp-private_data = NULL;
return ret;
-- 
1.7.4.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/3] drm: fix print format of sequence in trace point

[Chris Wilson]

On Mon, Jul 01, 2013 at 07:44:14PM +0900, Seung-Woo Kim wrote:
 seq of a trace point is unsigned int but print format was %d. So
 it fixes the format as %u.
 
 Signed-off-by: Seung-Woo Kim sw0312@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/3] drm: fix error routines in drm_open_helper

[Chris Wilson]

On Mon, Jul 01, 2013 at 07:49:10PM +0900, Seung-Woo Kim wrote:
 +
 +out_close:
 + if (dev-driver-postclose)
 + dev-driver-postclose(dev, priv);
 +out_free:
   kfree(priv);
   filp-private_data = NULL;
   return ret;

Looks like we are also missing:

if (drm_core_check_feature(dev, DRIVER_PRIME))
drm_prime_destroy_file_private(file_priv-prime);

put_pid(file_priv-pid);

after out_free.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC 1/3] DRM: Armada: Add Armada DRM driver

[Russell King - ARM Linux]

On Mon, Jul 01, 2013 at 10:01:30AM +1000, Dave Airlie wrote:
 OMG I'm working in a subsystem where stuff is being developed, with only
 a few resources! I know my full time job isn't maintaining a 500,000
 line subsystem,
 and the sub maintainers and developers do a great job refactoring
 where required.
 
 lots of other driver authors have made substantial changes to the drm core as
 they've written drivers, you spot one bit of refactoring and its a
 major shortcoming
 of the entire subsystem and development community.
 
 how about instead of writing:
 However, at least I've taken the time to _think_ about what I'm doing
 and realise that there _is_ scope here for the DRM core to improve,
 rather than burying this stuff deep inside my driver like everyone else
 has.  That's no reason to penalise patches from the good guys who think
 
 you go with
 I noticed this piece of functionality could be refactored, here is a
 patch adding them to
 the core, does anyone think its a good idea?
 
 As Daniel pointed out every driver submitted has refactored things,
 even exynos did a
 lot of work to be the first ARM driver we shipped, the DRM core
 doesn't write itself and
 I fully expect driver authors to be the ones that tell me what needs
 refactoring, since
 they are on the pointy end, but to state that you are the only person
 ever to think about
 things is frankly being a dick.
 
 Lets try for less dick, and more productive in future.

And you can stick your dick back where you got it from David.  Really,
your response is totally uncalled for.

Let's try realising that I only have very limited time to put into this
and unlike the other submitters who have been _paid_ to get their code
sorted, this is being done purely for free - which means it's really
low priority.  As you should already realise because I've stated already
that I *really* don't care whether it gets into mainline or not.

I am *NOT* planning on spending any time on this during the next week
as I have *paid* work to do, and probably not next weekend nor the
following week either.  So the next time that I'm likely to do anything
on this is in a fortnight or longer.

If you want stuff to be refactored in DRM, you need to find someone
with more time to do it.

And before you continue making a mountain out of a molehill, as you
seem to like doing, the let's make it a core thing is REALLY BLOODY
SIMPLE.  All it takes is for the header file to go into include/drm.
It's now available to anyone who wants to use that - and all that's
left is to sort out all those drivers *WHICH I CANT TEST* to use
that stuff.

And, frankly, when I was going to post this latest RFC, I gave serious
consideration to quite simply not posting it to dri-devel and copying
you or any other person listed as a maintainer for DRM, and only posting
it to the people interested in it on the ARM side, because I'd already
given up hope of it ever being merged into mainline.

Your totally over the top and rediculous response just confirms that.

Further postings will now omit you and dri-devel.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC 1/3] DRM: Armada: Add Armada DRM driver

[Sebastian Hesselbarth]

On 07/01/13 02:01, Dave Airlie wrote:

how about instead of writing:
However, at least I've taken the time to_think_  about what I'm doing
and realise that there_is_  scope here for the DRM core to improve,
rather than burying this stuff deep inside my driver like everyone else
has.  That's no reason to penalise patches from the good guys who think

you go with
I noticed this piece of functionality could be refactored, here is a
patch adding them to
the core, does anyone think its a good idea?


Dave,

at least on this point I do share Russell's impression. I've sent
bunch of patches improving TDA998x and DRM+DT:
- TDA998x irq handling - ignored
- TDA998x sync fix - ignored
- Fix drm I2C slave encoder probing

I am aware that this is not an easy job nor one you get much
appreciation for. But, back when TDA998x driver was published,
all my comments were basically answered with Oh, I know. Maybe
someday somebody will fix it.

I am not being paid for any of this, but have a strong intrinsic
motivation here. But I am loosing interest in sending fixes for
DRM stuff because my (personal) impression is the same Russell
has: Depending on who sends patches, they get merged independent
of how broken they are - others are discussed to death.

Sebastian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: linux-next: Tree for Jul 1 [ drm-intel-next: Several call-traces ]

[Sedat Dilek]

On Mon, Jul 1, 2013 at 10:52 AM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 On Mon, Jul 1, 2013 at 10:49 AM, Sedat Dilek sedat.di...@gmail.com wrote:
 On Mon, Jul 1, 2013 at 9:59 AM, Stephen Rothwell s...@canb.auug.org.au 
 wrote:
 Hi all,

 Changes since 20130628:

 The regulator tree gained a build failure so I used the version from
 next-20130628.

 The trivial tree gained a conflict against the fbdev tree.

 The arm-soc tree gained a conflict against the net-next tree.

 The akpm tree lost a few patches that turned up elsewhere and I removed 2
 that were causing run time problems.


 [ CC drm and drm-intel folks ]

 [ Did not check any relevant MLs ]

 Please, see attached dmesg output.

 Clock mismatch, one for Jesse to figure out. Note that this patch is
 for 3.12, I simply haven't yet gotten around to properly split my
 patch queue so a few spilled into -next. I'll do that now.

I like lightspeed-fast replies :-).

Guess drm/i915: get mode clock when reading the pipe config v9 [1]
is the cause.

- Sedat -

[1] 
http://cgit.freedesktop.org/~danvet/drm-intel/commit/?h=drm-intel-next-queuedid=d325d8b4f351f9d45e7c8baabf581fd21f343133

 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC 1/3] DRM: Armada: Add Armada DRM driver

[Sebastian Hesselbarth]

On 07/01/13 11:42, Daniel Vetter wrote:

On Mon, Jul 01, 2013 at 10:52:03AM +0200, Sebastian Hesselbarth wrote:

at least on this point I do share Russell's impression. I've sent
bunch of patches improving TDA998x and DRM+DT:
- TDA998x irq handling - ignored
- TDA998x sync fix - ignored
- Fix drm I2C slave encoder probing

I am aware that this is not an easy job nor one you get much
appreciation for. But, back when TDA998x driver was published,
all my comments were basically answered with Oh, I know. Maybe
someday somebody will fix it.


I guess part of the problem here is that in the arm world we don't (yet)
have many full-blown drivers and not many people to fix up stuff or chime
in with good review. And sometimes that just means that someone puts down
his foot and says this is how we do it - at least for drm/i915 I
sometimes have to do that to unblock a massive bikeshed-fest.


I am not being paid for any of this, but have a strong intrinsic
motivation here. But I am loosing interest in sending fixes for
DRM stuff because my (personal) impression is the same Russell
has: Depending on who sends patches, they get merged independent
of how broken they are - others are discussed to death.


Hm, we run fairly extensive QA for drm/i915, and thus far the drm core
stuff hasn't really blown up badly for us. So can you please point at
examples where crap was merged and shouldn't have been?


Don't get me wrong, broken above didn't mean it doesn't work at all,
but with SoC graphic drivers arising, requirements shift from some
known implementations to you never know what will be combined with
e.g. a specific CRTC. So from that latter point-of-view, I consider
TDA998x for example as broken. It may work with tilcdc in some modes,
but as Darren Etheridge stated, it breaks as soon as you touch TDA998x
driver. At least for that, I confirmed the timings of Russell's driver
and the fixes posted with a scope and a bunch of modes and monitors/tv.

For the timing fix of TDA998x, Darren now is trying to also confirm
every single sync line of tilcdc and I really like to see his Tested-by
on the patch - just because Russell's driver and the CuBox are the only
testbeds I have on this.


Wrt to bikeshed to death I know that drm folks are a bit prone to that (me
included), but recently I haven't spotted a case where this happened. ARM
stuff excluded ofc since I don't follow that too closely. There's also
that Dave is sometimes a bit swamped, but pinging him on irc about lost
patches works well (at least for stuff I care about).


Hmm, I understand that it is _very_ time consuming work on your side.
But for me - with limited insight of DRM core - it is not a good starter
to make the API DT aware. The DRM API documentation _is_ limited wrt
intentions of the way it is done.

Of course, I share the idea of true, full-blown of_drm_something
helpers. But the DT patch, is not an improvement but a real fix as in
make DRM not break on some platforms. From that on, I can start
digging into DRM API and improve DT support here and there.

So for the three patches I mentioned above, they are all minor
improvements of the API. Minor, because I cannot ever sent _the_ single
perfect patch just because I don't know the API well enough, yet.

Just based on my personal experience: TDA998x driver got merged the way 
it is _although_ I addressed some concerns - the fixes are not merged

_although_ I provided experimental results. This is of course
disappointing for me, but I am sure it will work out soon and I will
get back to happily send improvements for the platforms I can test on.

Sebastian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/3] drm: fix error routines in drm_open_helper

[Seung-Woo Kim]

Hello Chris,

On 2013년 07월 01일 19:57, Chris Wilson wrote:
 On Mon, Jul 01, 2013 at 07:49:10PM +0900, Seung-Woo Kim wrote:
 +
 +out_close:
 +if (dev-driver-postclose)
 +dev-driver-postclose(dev, priv);
 +out_free:
  kfree(priv);
  filp-private_data = NULL;
  return ret;
 
 Looks like we are also missing:
 
 if (drm_core_check_feature(dev, DRIVER_PRIME))
   drm_prime_destroy_file_private(file_priv-prime);

Currently, file_priv-prime is just initialized, and
drm_prime_destroy_file_private() just checks the list is empty and at
the open time, prime list is always empty. So IMHO, it seems unnecessary
to call drm_prime_destroy_file_private().

If this is necessary, drm_gem_release() is also needed because the pair
function of drm_gem_open() is drm_gem_release().

 
 put_pid(file_priv-pid);

Yes, you are rignt. put_pid is also needed.

After discussion about above part, I'll post v3 for this.

Thanks and Regards,
- Seung-Woo Kim

 
 after out_free.
 -Chris
 

-- 
Seung-Woo Kim
Samsung Software RD Center
--

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 66450] New: JUNIPER UVD accelerated playback of MPEG 1/2 streams does not work

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=66450

  Priority: medium
Bug ID: 66450
  Assignee: dri-devel@lists.freedesktop.org
   Summary: JUNIPER UVD accelerated  playback of MPEG 1/2 streams
does not work
  Severity: normal
Classification: Unclassified
OS: Linux (All)
  Reporter: eugene.shaly...@gmail.com
  Hardware: x86-64 (AMD64)
Status: NEW
   Version: unspecified
 Component: DRM/Radeon
   Product: DRI

When playing MPEG 1 or 2 streams using mplayer (or mplayer2 or ffplay or mpv)
there is no output, only black rectangle. 

$vdpauinfo declares MPEG support:

MPEG116  9216  2048  1152
MPEG2_SIMPLE 16  9216  2048  1152
MPEG2_MAIN   16  9216  2048  1152


Output from ffmpeg shows:
Unsupported PixelFormat xvmcidct (16)
Unsupported PixelFormat xvmcmc (15)

acceleration of h264 works fine, so I conclude that UVD is initialized
succesfully

Radeon Mobility HD 5850

Kernel 3.10.0, libdrm and mesa are from git master (30.06.2013)

sample file: http://samples.mplayerhq.hu/MPEG2/vid_0x80.ts

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/3] drm: fix error routines in drm_open_helper

[Chris Wilson]

On Mon, Jul 01, 2013 at 08:14:42PM +0900, Seung-Woo Kim wrote:
 Hello Chris,
 
 On 2013년 07월 01일 19:57, Chris Wilson wrote:
  On Mon, Jul 01, 2013 at 07:49:10PM +0900, Seung-Woo Kim wrote:
  +
  +out_close:
  +  if (dev-driver-postclose)
  +  dev-driver-postclose(dev, priv);
  +out_free:
 kfree(priv);
 filp-private_data = NULL;
 return ret;
  
  Looks like we are also missing:
  
  if (drm_core_check_feature(dev, DRIVER_PRIME))
  drm_prime_destroy_file_private(file_priv-prime);
 
 Currently, file_priv-prime is just initialized, and
 drm_prime_destroy_file_private() just checks the list is empty and at
 the open time, prime list is always empty. So IMHO, it seems unnecessary
 to call drm_prime_destroy_file_private().
 
 If this is necessary, drm_gem_release() is also needed because the pair
 function of drm_gem_open() is drm_gem_release().

Hmm, could be a slight ordering bug in drm_release(), but yes I agree
that we should also call drm_gem_release() here in drm_open_helper. It
is better for the code to be symmetric in cleaning up anything that we
create so that we are correct for future changes. We might as well fix it
correctly, rather than having to rediscover this bug at some later date.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 66452] New: JUNIPER UVD accelerated playback of WMV3 streams does not work

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=66452

  Priority: medium
Bug ID: 66452
  Assignee: dri-devel@lists.freedesktop.org
   Summary: JUNIPER UVD accelerated playback of WMV3 streams does
not work
  Severity: normal
Classification: Unclassified
OS: Linux (All)
  Reporter: eugene.shalygin+bugzilla@gmail.com
  Hardware: x86-64 (AMD64)
Status: NEW
   Version: unspecified
 Component: DRM/Radeon
   Product: DRI

When playing WMV3 streams using mplayer (or mplayer2 or ffplay or mpv) the
output is corrupted: it looks like black rectangle with color mess at the top
(https://imageshack.com/a/img22/534/yw2a.png).

acceleration of h264 works fine, so I conclude that UVD is initialized
succesfully

Radeon Mobility HD 5850

Kernel 3.10.0, libdrm and mesa are from git master (30.06.2013)

sample file: http://samples.mplayerhq.hu/asf-wmv/asf_with_chapters.wmv

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/3] drm: fix error routines in drm_open_helper

[YoungJun Cho]

Hello Chris,

On Jul 1, 2013 8:53 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:

 On Mon, Jul 01, 2013 at 08:14:42PM +0900, Seung-Woo Kim wrote:
  Hello Chris,
 
  On 2013년 07월 01일 19:57, Chris Wilson wrote:
   On Mon, Jul 01, 2013 at 07:49:10PM +0900, Seung-Woo Kim wrote:
   +
   +out_close:
   +  if (dev-driver-postclose)
   +  dev-driver-postclose(dev, priv);
   +out_free:
  kfree(priv);
  filp-private_data = NULL;
  return ret;
  
   Looks like we are also missing:
  
   if (drm_core_check_feature(dev, DRIVER_PRIME))
   drm_prime_destroy_file_private(file_priv-prime);
 
  Currently, file_priv-prime is just initialized, and
  drm_prime_destroy_file_private() just checks the list is empty and at
  the open time, prime list is always empty. So IMHO, it seems unnecessary
  to call drm_prime_destroy_file_private().
 
  If this is necessary, drm_gem_release() is also needed because the pair
  function of drm_gem_open() is drm_gem_release().

 Hmm, could be a slight ordering bug in drm_release(), but yes I agree
 that we should also call drm_gem_release() here in drm_open_helper. It
 is better for the code to be symmetric in cleaning up anything that we
 create so that we are correct for future changes. We might as well fix it
 correctly, rather than having to rediscover this bug at some later date.
 -Chris


Thank you for quick reviews.
We'll post V3 patch with this also.

Best regards YJ

 --
 Chris Wilson, Intel Open Source Technology Centre
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 63520] r300g regression (RV380): Strange rendering of light sources in Penumbra (bisected)

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=63520

Fabio Pedretti fabio@libero.it changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #23 from Fabio Pedretti fabio@libero.it ---
Fixed in master:
http://cgit.freedesktop.org/mesa/mesa/commit/?id=24fa43675f32bc81c7252f3ddce4c80ed8c7737d

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 66425] failed testing IB on ring 5 when suspending to disk

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=66425

--- Comment #1 from Alex Deucher ag...@yahoo.com ---
This is a mac?

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/24] drm/rcar-du: Use devm_ioremap_resource()

[Laurent Pinchart]

Hi Sergei,

On Thursday 27 June 2013 17:04:45 Sergei Shtylyov wrote:
 On 27-06-2013 13:49, Laurent Pinchart wrote:
  Replace the devm_request_mem_region() and devm_ioremap_nocache() calls
  with devm_ioremap_resource().
  
  Signed-off-by: Laurent Pinchart
  laurent.pinchart+rene...@ideasonboard.com
  ---
  
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 20 ++--
1 file changed, 2 insertions(+), 18 deletions(-)
  
  diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
  b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 003b34e..24ab0ca 100644
  --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
  +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
 
 [...]
 
  @@ -129,24 +128,9 @@ static int rcar_du_load(struct drm_device *dev,
  unsigned long flags) 
  /* I/O resources and clocks */
  mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
 [...]
 
  +   rcdu-mmio = devm_ioremap_resource(pdev-dev, mem);
  +   if (IS_ERR(rcdu-mmio))
  
  return -ENOMEM;
 
 You should return PTR_ERR(rcdu-mmio).

Good point, I'll fix that. Thank you.

-- 
Regards,

Laurent Pinchart

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 63520] r300g regression (RV380): Strange rendering of light sources in Penumbra (bisected)

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=63520

--- Comment #24 from madbiologist s.j.tur...@uqconnect.net ---
The fix is now also in Mesa 9.1.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm: move edid null check to the first part of drm_edid_block_valid

[Daniel Vetter]

On Mon, Jul 1, 2013 at 12:21 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Mon, Jul 01, 2013 at 07:06:32PM +0900, Seung-Woo Kim wrote:
 If raw_edid is null, it will crash, so checking in bad label is
 meaningless.

 It would be an error on part of the caller, but the defense looks sane.
 As the function is a bool, I would have preferred it returned
 true/false, but your patch is correct wrt to the rest of the function.

If we consider passing a NULL raw_edid here a caller-error, shouldn't
this be a WARN on top? And I concur on the s/0/false/ bikeshed, return
0 could be misleading since for errno returning functions that reads
as success.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 57875] Second Life viewer bad rendering with git-ec83535

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=57875

--- Comment #29 from Stefan Dösinger stefandoesin...@gmx.at ---
Hmm, this seems like an idea worth thinking about. The D3D behavior the
proposed extension addresses is part of the D3DDECLUSAGE_POSITIONT /
D3DFVF_XYZRHW vertex input semantics.

For now I'm opposed to making this a vertex shader control though. The point of
POSITIONT / XYZRHW is to skip vertex processing altogether, so handling
POSITIONT semantics in a shader seems a bit off. Furthermore, the clipping
behavior of POSITIONT depends on the depth test, so this needs a separate
control anyway (or replicating the depth test interaction, which seems somewhat
ugly to me because its done by a different stage in the rendering pipeline).

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 2/6] x86: provide platform-devices for boot-framebuffers

[Stephen Warren]

On 06/28/2013 04:11 AM, David Herrmann wrote:
 Hi
 
 On Wed, Jun 26, 2013 at 10:49 PM, Stephen Warren swar...@wwwdotorg.org 
 wrote:
 On 06/24/2013 04:27 PM, David Herrmann wrote:
 The current situation regarding boot-framebuffers (VGA, VESA/VBE, EFI) on
 x86 causes troubles when loading multiple fbdev drivers. The global
 struct screen_info does not provide any state-tracking about which
 drivers use the FBs. request_mem_region() theoretically works, but
 unfortunately vesafb/efifb ignore it due to quirks for broken boards.

 Avoid this by creating a platform-framebuffer device with a pointer
 to the struct screen_info as platform-data. Drivers can now create
 platform-drivers and the driver-core will refuse multiple drivers being
 active simultaneously.

 We keep the screen_info available for backwards-compatibility. Drivers
 can be converted in follow-up patches.

 Apart from platform-framebuffer devices, this also introduces a
 compatibility option for simple-framebuffer drivers which recently got
 introduced for OF based systems. If CONFIG_X86_SYSFB is selected, we
 try to match the screen_info against a simple-framebuffer supported
 format. If we succeed, we create a simple-framebuffer device instead
 of a platform-framebuffer.
...

 diff --git a/arch/x86/kernel/sysfb.c b/arch/x86/kernel/sysfb.c
...
 +#else /* CONFIG_X86_SYSFB */
 +
 +static bool parse_mode(const struct screen_info *si,
 +struct simplefb_platform_data *mode)
 +{
 + return false;
 +}
 +
 +static int create_simplefb(const struct screen_info *si,
 +const struct simplefb_platform_data *mode)
 +{
 + return -EINVAL;
 +}
 +
 +#endif /* CONFIG_X86_SYSFB */

 Following on from my ifdef comment above, I believe those versions of
 those functions will always cause add_sysfb() to return -ENODEV, so you
 may as well provide a static inline for add_sysfb() instead.
 
 No. add_sysfb() is supposed to always succeed. However, if
 parse_mode/create_simplefb fail, it creates a platform-framebuffer
 as fallback. I don't see any way to avoid these ifdefs. Considering
 the explanation above, could you elaborate how you think this should
 work?

Ah, I wasn't getting the fallback mechanism; that if creating a simplefb
wasn't possible or didn't succeed, a platformfb device would be created
instead.

Perhaps the following might be slightly clearer; there are certainly
fewer nesting levels:

static __init int add_sysfb(void)
{
const struct screen_info *si = screen_info;
struct simplefb_platform_data mode;
struct platform_device *pd;
bool compatible = false;
int ret;

compatible = parse_mode(si, mode);
if (compatible) {
ret = create_simplefb(si, mode);
if (!ret)
return 0;
}

pd = platform_device_register_resndata(NULL,
platform-framebuffer, 0,
NULL, 0, si, sizeof(*si));
ret = IS_ERR(pd) ? PTR_ERR(pd) : 0;

return ret;
}

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[patch -next] drm/radeon/dpm/btc: off by one in btc_set_mc_special_registers()

[Dan Carpenter]

It should be = instead of  here.  The table-mc_reg_address[]
array has SMC_EVERGREEN_MC_REGISTER_ARRAY_SIZE (16) elements.

Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

diff --git a/drivers/gpu/drm/radeon/btc_dpm.c b/drivers/gpu/drm/radeon/btc_dpm.c
index bab0185..55491e7 100644
--- a/drivers/gpu/drm/radeon/btc_dpm.c
+++ b/drivers/gpu/drm/radeon/btc_dpm.c
@@ -1913,7 +1913,7 @@ static int btc_set_mc_special_registers(struct 
radeon_device *rdev,
}
j++;
 
-   if (j  SMC_EVERGREEN_MC_REGISTER_ARRAY_SIZE)
+   if (j = SMC_EVERGREEN_MC_REGISTER_ARRAY_SIZE)
return -EINVAL;
 
tmp = RREG32(MC_PMG_CMD_MRS);
@@ -1928,7 +1928,7 @@ static int btc_set_mc_special_registers(struct 
radeon_device *rdev,
}
j++;
 
-   if (j  SMC_EVERGREEN_MC_REGISTER_ARRAY_SIZE)
+   if (j = SMC_EVERGREEN_MC_REGISTER_ARRAY_SIZE)
return -EINVAL;
break;
case MC_SEQ_RESERVE_M  2:
@@ -1942,7 +1942,7 @@ static int btc_set_mc_special_registers(struct 
radeon_device *rdev,
}
j++;
 
-   if (j  SMC_EVERGREEN_MC_REGISTER_ARRAY_SIZE)
+   if (j = SMC_EVERGREEN_MC_REGISTER_ARRAY_SIZE)
return -EINVAL;
break;
default:
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[patch] drm/radeon: forever loop on error in radeon_do_test_moves()

[Dan Carpenter]

The error path does this:

for (--i; i = 0; --i) {

which is a forever loop because i is unsigned.

Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

diff --git a/drivers/gpu/drm/radeon/radeon_test.c 
b/drivers/gpu/drm/radeon/radeon_test.c
index f4d6bce..12e8099 100644
--- a/drivers/gpu/drm/radeon/radeon_test.c
+++ b/drivers/gpu/drm/radeon/radeon_test.c
@@ -36,8 +36,8 @@ static void radeon_do_test_moves(struct radeon_device *rdev, 
int flag)
struct radeon_bo *vram_obj = NULL;
struct radeon_bo **gtt_obj = NULL;
uint64_t gtt_addr, vram_addr;
-   unsigned i, n, size;
-   int r, ring;
+   unsigned n, size;
+   int i, r, ring;
 
switch (flag) {
case RADEON_TEST_COPY_DMA:
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 65310] [regression] failure in building nvc0_vbo.lo: /tmp/cclDjdRp.s:1270: Error: missing or invalid displacement expression `-8589934576

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=65310

Emil Velikov emil.l.veli...@gmail.com changed:

   What|Removed |Added

   Assignee|dri-devel@lists.freedesktop |nouveau@lists.freedesktop.o
   |.org|rg
  Component|Drivers/Gallium/r300|Drivers/DRI/nouveau

--- Comment #3 from Emil Velikov emil.l.veli...@gmail.com ---
Correcting component - nouveau (nvc0) related

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 0/6] DRM: Unified VMA Offset Manager

[David Herrmann]

Hi

I picked up the initial work from Dave [1], fixed several bugs, rewrote the
drm_mm node handling and adjusted the different drivers.
The series tries to replace the VMA-offset managers from GEM and TTM with a
single unified implementation. It uses the TTM RBTree idea to allow sub-mappings
(which wouldn't be feasible with hashtables).

Changes to Dave's v1:
 * Fixed a ref-count bug in TTM during object lookup
 * Use embedded drm_mm_node objects to avoid allocations
 * Document drm_vma_* API
 * Reviewed TTM locking
 * Fixed all new drivers
 * Use node-vm_pages instead of obj-size for GEM size calculations

Notes:
 * Tested on nouveau only! I will try to test i915 this week. However, the
   gem changes seem pretty trivial.
 * I couldn't even compile-test the ARM drivers. However, the omapdrm diffs
   are the only changes that are non-trivial. Is there any ongoing work to
   remove the arch-deps in DRM drivers?
 * _DRM_GEM is no longer used, but I guess we need to keep it for backwards
   compat?
 * If we replace node_list in drm_mm with an rbtree, we can drop it from
   drm_vma_offset_manager completely. However, I wanted to avoid heavy drm_mm
   changes and left this for follow up patches.
 * This is currently based on linux-3.10 from today. Next series will be
   rebased on drm-next/linux-next, but the current -next trees continously break
   my machines..
   But the only changes should be to fix additional drivers. I didn't see any
   other things to fix for drm-next.

Another series, which I will send later, adds struct file lists for each
drm-vma-offset so we can get access control over gem objects. Also, I have an
experimental series to remove the allocation helpers in drm_mm and let drivers
embed drm_mm_node instead. Lets see how that works out.

Comments welcome!
Cheers
David

[1]: http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-vma-manager

David Herrmann (6):
  drm: make drm_mm_init() return void
  drm: mm: add drm_mm_node_linked() helper
  drm: add unified vma offset manager
  drm: gem: convert to new unified vma manager
  drm: ttm: convert to unified vma offset manager
  drm: provide generic drm_vma_node_unmap() helper

 drivers/gpu/drm/Makefile   |   2 +-
 drivers/gpu/drm/ast/ast_main.c |   2 +-
 drivers/gpu/drm/cirrus/cirrus_main.c   |   2 +-
 drivers/gpu/drm/drm_gem.c  |  93 ++--
 drivers/gpu/drm/drm_gem_cma_helper.c   |   9 +-
 drivers/gpu/drm/drm_mm.c   |   5 +-
 drivers/gpu/drm/drm_vma_manager.c  | 224 +
 drivers/gpu/drm/exynos/exynos_drm_gem.c|   7 +-
 drivers/gpu/drm/gma500/gem.c   |   8 +-
 drivers/gpu/drm/i915/i915_gem.c|  13 +-
 drivers/gpu/drm/mgag200/mgag200_main.c |   2 +-
 drivers/gpu/drm/nouveau/nouveau_display.c  |   2 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c  |   2 +-
 drivers/gpu/drm/omapdrm/omap_gem.c |  11 +-
 drivers/gpu/drm/omapdrm/omap_gem_helpers.c |  49 +--
 drivers/gpu/drm/qxl/qxl_object.h   |   2 +-
 drivers/gpu/drm/qxl/qxl_release.c  |   2 +-
 drivers/gpu/drm/radeon/radeon_object.h |   5 +-
 drivers/gpu/drm/ttm/ttm_bo.c   |  82 ++-
 drivers/gpu/drm/ttm/ttm_bo_manager.c   |   8 +-
 drivers/gpu/drm/ttm/ttm_bo_util.c  |   3 +-
 drivers/gpu/drm/ttm/ttm_bo_vm.c|  81 ---
 drivers/gpu/drm/udl/udl_gem.c  |   6 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |   4 +-
 include/drm/drmP.h |   7 +-
 include/drm/drm_mm.h   |  11 +-
 include/drm/drm_vma_manager.h  | 122 
 include/drm/ttm/ttm_bo_api.h   |  15 +-
 include/drm/ttm/ttm_bo_driver.h|   7 +-
 include/uapi/drm/drm.h |   2 +-
 30 files changed, 464 insertions(+), 324 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_vma_manager.c
 create mode 100644 include/drm/drm_vma_manager.h

-- 
1.8.3.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/6] drm: make drm_mm_init() return void

[David Herrmann]

There is no reason to return int as this function never fails.
Furthermore, several drivers (ast, sis) already depend on this.

Signed-off-by: David Herrmann dh.herrm...@gmail.com
---
 drivers/gpu/drm/drm_gem.c| 8 ++--
 drivers/gpu/drm/drm_mm.c | 4 +---
 drivers/gpu/drm/ttm/ttm_bo.c | 6 +-
 drivers/gpu/drm/ttm/ttm_bo_manager.c | 8 +---
 include/drm/drm_mm.h | 6 +++---
 5 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index cf919e3..88f0322 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -108,12 +108,8 @@ drm_gem_init(struct drm_device *dev)
return -ENOMEM;
}
 
-   if (drm_mm_init(mm-offset_manager, DRM_FILE_PAGE_OFFSET_START,
-   DRM_FILE_PAGE_OFFSET_SIZE)) {
-   drm_ht_remove(mm-offset_hash);
-   kfree(mm);
-   return -ENOMEM;
-   }
+   drm_mm_init(mm-offset_manager, DRM_FILE_PAGE_OFFSET_START,
+   DRM_FILE_PAGE_OFFSET_SIZE);
 
return 0;
 }
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 07cf99c..7917729 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -669,7 +669,7 @@ int drm_mm_clean(struct drm_mm * mm)
 }
 EXPORT_SYMBOL(drm_mm_clean);
 
-int drm_mm_init(struct drm_mm * mm, unsigned long start, unsigned long size)
+void drm_mm_init(struct drm_mm * mm, unsigned long start, unsigned long size)
 {
INIT_LIST_HEAD(mm-hole_stack);
INIT_LIST_HEAD(mm-unused_nodes);
@@ -690,8 +690,6 @@ int drm_mm_init(struct drm_mm * mm, unsigned long start, 
unsigned long size)
list_add_tail(mm-head_node.hole_stack, mm-hole_stack);
 
mm-color_adjust = NULL;
-
-   return 0;
 }
 EXPORT_SYMBOL(drm_mm_init);
 
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 9b07b7d..e97c5a0 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1619,9 +1619,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
goto out_no_sys;
 
bdev-addr_space_rb = RB_ROOT;
-   ret = drm_mm_init(bdev-addr_space_mm, file_page_offset, 0x1000);
-   if (unlikely(ret != 0))
-   goto out_no_addr_mm;
+   drm_mm_init(bdev-addr_space_mm, file_page_offset, 0x1000);
 
INIT_DELAYED_WORK(bdev-wq, ttm_bo_delayed_workqueue);
INIT_LIST_HEAD(bdev-ddestroy);
@@ -1635,8 +1633,6 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
mutex_unlock(glob-device_list_mutex);
 
return 0;
-out_no_addr_mm:
-   ttm_bo_clean_mm(bdev, 0);
 out_no_sys:
return ret;
 }
diff --git a/drivers/gpu/drm/ttm/ttm_bo_manager.c 
b/drivers/gpu/drm/ttm/ttm_bo_manager.c
index 9212494..e4367f9 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_manager.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_manager.c
@@ -103,18 +103,12 @@ static int ttm_bo_man_init(struct ttm_mem_type_manager 
*man,
   unsigned long p_size)
 {
struct ttm_range_manager *rman;
-   int ret;
 
rman = kzalloc(sizeof(*rman), GFP_KERNEL);
if (!rman)
return -ENOMEM;
 
-   ret = drm_mm_init(rman-mm, 0, p_size);
-   if (ret) {
-   kfree(rman);
-   return ret;
-   }
-
+   drm_mm_init(rman-mm, 0, p_size);
spin_lock_init(rman-lock);
man-priv = rman;
return 0;
diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
index 88591ef..de92425 100644
--- a/include/drm/drm_mm.h
+++ b/include/drm/drm_mm.h
@@ -275,9 +275,9 @@ static inline  struct drm_mm_node 
*drm_mm_search_free_in_range_color(
return drm_mm_search_free_in_range_generic(mm, size, alignment, color,
   start, end, best_match);
 }
-extern int drm_mm_init(struct drm_mm *mm,
-  unsigned long start,
-  unsigned long size);
+extern void drm_mm_init(struct drm_mm *mm,
+   unsigned long start,
+   unsigned long size);
 extern void drm_mm_takedown(struct drm_mm *mm);
 extern int drm_mm_clean(struct drm_mm *mm);
 extern int drm_mm_pre_get(struct drm_mm *mm);
-- 
1.8.3.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/6] drm: mm: add drm_mm_node_linked() helper

[David Herrmann]

This helper tests whether a given node is currently linked into a drm_mm
manager. We use the node-mm pointer for that as it is set for all linked
objects.

We also reset node-mm whenever a node is removed. All such access is
currently safe as everyone calls kfree() on the object directly after
removing it. Furthermore, no-one is supposed to access node-mm from
outside drm_mm.c, anyway.

Signed-off-by: David Herrmann dh.herrm...@gmail.com
---
 drivers/gpu/drm/drm_mm.c | 1 +
 include/drm/drm_mm.h | 5 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 7917729..3bd43ce 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -374,6 +374,7 @@ void drm_mm_remove_node(struct drm_mm_node *node)
 
list_del(node-node_list);
node-allocated = 0;
+   node-mm = NULL;
 }
 EXPORT_SYMBOL(drm_mm_remove_node);
 
diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
index de92425..d83b966 100644
--- a/include/drm/drm_mm.h
+++ b/include/drm/drm_mm.h
@@ -85,6 +85,11 @@ static inline bool drm_mm_node_allocated(struct drm_mm_node 
*node)
return node-allocated;
 }
 
+static inline bool drm_mm_node_linked(struct drm_mm_node *node)
+{
+   return node-mm;
+}
+
 static inline bool drm_mm_initialized(struct drm_mm *mm)
 {
return mm-hole_stack.next;
-- 
1.8.3.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/6] drm: add unified vma offset manager

[David Herrmann]

If we want to map GPU memory into user-space, we need to linearize the
addresses to not confuse mm-core. Currently, GEM and TTM both implement
their own offset-managers to assign a pgoff to each object for user-space
CPU access. GEM uses a hash-table, TTM uses an rbtree.

This patch provides a unified implementation that can be used to replace
both. TTM allows partial mmaps with a given offset, so we cannot use
hashtables as the start address may not be known at mmap time. Hence, we
use the rbtree-implementation of TTM.

We could easily update drm_mm to use an rbtree instead of a linked list
for it's object list and thus drop the rbtree from the vma-manager.
However, this would slow down drm_mm object allocation for all other
use-cases (rbtree insertion) and add another 4-8 bytes to each mm node.
Hence, use the separate tree but allow for later migration.

This is a rewrite of the 2012-proposal by David Airlie airl...@linux.ie

Signed-off-by: David Herrmann dh.herrm...@gmail.com
---
 drivers/gpu/drm/Makefile  |   2 +-
 drivers/gpu/drm/drm_vma_manager.c | 224 ++
 include/drm/drm_vma_manager.h | 107 ++
 3 files changed, 332 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_vma_manager.c
 create mode 100644 include/drm/drm_vma_manager.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 1c9f2439..f8851cc 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -12,7 +12,7 @@ drm-y   :=drm_auth.o drm_buffer.o drm_bufs.o 
drm_cache.o \
drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
drm_crtc.o drm_modes.o drm_edid.o \
drm_info.o drm_debugfs.o drm_encoder_slave.o \
-   drm_trace_points.o drm_global.o drm_prime.o
+   drm_trace_points.o drm_global.o drm_prime.o drm_vma_manager.o
 
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/drm_vma_manager.c 
b/drivers/gpu/drm/drm_vma_manager.c
new file mode 100644
index 000..c28639f
--- /dev/null
+++ b/drivers/gpu/drm/drm_vma_manager.c
@@ -0,0 +1,224 @@
+/*
+ * Copyright (c) 2012 David Airlie airl...@linux.ie
+ * Copyright (c) 2013 David Herrmann dh.herrm...@gmail.com
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include drm/drmP.h
+#include drm/drm_mm.h
+#include drm/drm_vma_manager.h
+#include linux/mm.h
+#include linux/module.h
+#include linux/rbtree.h
+#include linux/slab.h
+#include linux/spinlock.h
+#include linux/types.h
+
+/** @file drm_vma_manager.c
+ *
+ * The vma-manager is responsible to map arbitrary driver-dependent memory
+ * regions into the linear user address-space. It provides offsets to the
+ * caller which can then be used on the address_space of the drm-device. It
+ * takes care to not overlap regions, size them appropriately and to not
+ * confuse mm-core by inconsistent fake vm_pgoff fields.
+ *
+ * We use drm_mm as backend to manage object allocations. But it is highly
+ * optimized for alloc/free calls, not lookups. Hence, we use an rb-tree to
+ * speed up offset lookups.
+ */
+
+/** drm_vma_offset_manager_init()
+ *
+ * Initialize a new offset-manager. The offset and area size available for the
+ * manager are given as @page_offset and @size. Both are interpreted as
+ * page-numbers, not bytes.
+ *
+ * Adding/removing nodes from the manager is locked internally and protected
+ * against concurrent access. However, node allocation and destruction is left
+ * for the caller. While calling into the vma-manager, a given node must
+ * always be guaranteed to be referenced.
+ */
+void drm_vma_offset_manager_init(struct drm_vma_offset_manager *mgr,
+unsigned long page_offset, unsigned long size)
+{
+   rwlock_init(mgr-vm_lock);
+   mgr-vm_addr_space_rb = RB_ROOT;
+   

[PATCH 5/6] drm: ttm: convert to unified vma offset manager

[David Herrmann]

Use the new vma-manager infrastructure. This doesn't change any
implementation details as the vma-offset-manager is nearly copied 1-to-1
from TTM.

Even though the vma-manager uses its own locks, we still need bo-vm_lock
to prevent bos from being destroyed before we can get a reference during
lookup. However, this lock is not needed during vm-setup as we still
hold a reference there.

This also drops the addr_space_offset member as it is a copy of vm_start
in vma_node objects. Use the accessor functions instead.

Signed-off-by: David Herrmann dh.herrm...@gmail.com
---
 drivers/gpu/drm/ast/ast_main.c|  2 +-
 drivers/gpu/drm/cirrus/cirrus_main.c  |  2 +-
 drivers/gpu/drm/mgag200/mgag200_main.c|  2 +-
 drivers/gpu/drm/nouveau/nouveau_display.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c |  2 +-
 drivers/gpu/drm/qxl/qxl_object.h  |  2 +-
 drivers/gpu/drm/qxl/qxl_release.c |  2 +-
 drivers/gpu/drm/radeon/radeon_object.h|  5 +-
 drivers/gpu/drm/ttm/ttm_bo.c  | 84 ++-
 drivers/gpu/drm/ttm/ttm_bo_util.c |  3 +-
 drivers/gpu/drm/ttm/ttm_bo_vm.c   | 81 -
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c  |  4 +-
 include/drm/ttm/ttm_bo_api.h  | 15 ++
 include/drm/ttm/ttm_bo_driver.h   |  7 +--
 14 files changed, 65 insertions(+), 148 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index f60fd7b..c195dc2 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -487,7 +487,7 @@ void ast_gem_free_object(struct drm_gem_object *obj)
 
 static inline u64 ast_bo_mmap_offset(struct ast_bo *bo)
 {
-   return bo-bo.addr_space_offset;
+   return drm_vma_node_offset_addr(bo-bo.vma_node);
 }
 int
 ast_dumb_mmap_offset(struct drm_file *file,
diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c 
b/drivers/gpu/drm/cirrus/cirrus_main.c
index 35cbae8..3a7a0ef 100644
--- a/drivers/gpu/drm/cirrus/cirrus_main.c
+++ b/drivers/gpu/drm/cirrus/cirrus_main.c
@@ -294,7 +294,7 @@ void cirrus_gem_free_object(struct drm_gem_object *obj)
 
 static inline u64 cirrus_bo_mmap_offset(struct cirrus_bo *bo)
 {
-   return bo-bo.addr_space_offset;
+   return drm_vma_node_offset_addr(bo-bo.vma_node);
 }
 
 int
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c 
b/drivers/gpu/drm/mgag200/mgag200_main.c
index 9905923..1b560a1 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -330,7 +330,7 @@ void mgag200_gem_free_object(struct drm_gem_object *obj)
 
 static inline u64 mgag200_bo_mmap_offset(struct mgag200_bo *bo)
 {
-   return bo-bo.addr_space_offset;
+   return drm_vma_node_offset_addr(bo-bo.vma_node);
 }
 
 int
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
b/drivers/gpu/drm/nouveau/nouveau_display.c
index f17dc2a..52498de 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -705,7 +705,7 @@ nouveau_display_dumb_map_offset(struct drm_file *file_priv,
gem = drm_gem_object_lookup(dev, file_priv, handle);
if (gem) {
struct nouveau_bo *bo = gem-driver_private;
-   *poffset = bo-bo.addr_space_offset;
+   *poffset = drm_vma_node_offset_addr(bo-bo.vma_node);
drm_gem_object_unreference_unlocked(gem);
return 0;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c 
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index b4b4d0c..357dace 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -192,7 +192,7 @@ nouveau_gem_info(struct drm_file *file_priv, struct 
drm_gem_object *gem,
}
 
rep-size = nvbo-bo.mem.num_pages  PAGE_SHIFT;
-   rep-map_handle = nvbo-bo.addr_space_offset;
+   rep-map_handle = drm_vma_node_offset_addr(nvbo-bo.vma_node);
rep-tile_mode = nvbo-tile_mode;
rep-tile_flags = nvbo-tile_flags;
return 0;
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index b4fd89f..1fc4e4b 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -64,7 +64,7 @@ static inline bool qxl_bo_is_reserved(struct qxl_bo *bo)
 
 static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo)
 {
-   return bo-tbo.addr_space_offset;
+   return drm_vma_node_offset_addr(bo-tbo.vma_node);
 }
 
 static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type,
diff --git a/drivers/gpu/drm/qxl/qxl_release.c 
b/drivers/gpu/drm/qxl/qxl_release.c
index b443d67..1a648e1 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -87,7 +87,7 @@ qxl_release_free(struct qxl_device *qdev,
 
for (i = 0 ; i  release-bo_count; ++i) {
QXL_INFO(qdev, release %llx\n,
-   release-bos[i]-tbo.addr_space_offset
+   

[PATCH 4/6] drm: gem: convert to new unified vma manager

[David Herrmann]

Use the new vma manager instead of the old hashtable. Also convert all
drivers to use the new convenience helpers. This drops all the
(map_list.hash.key  PAGE_SHIFT) non-sense.

Locking and access-management is exactly the same as before with an
additional lock inside of the vma-manager, which strictly wouldn't be
needed for gem.

Signed-off-by: David Herrmann dh.herrm...@gmail.com
---
 drivers/gpu/drm/drm_gem.c  | 89 +-
 drivers/gpu/drm/drm_gem_cma_helper.c   |  9 +--
 drivers/gpu/drm/exynos/exynos_drm_gem.c|  7 ++-
 drivers/gpu/drm/gma500/gem.c   |  8 +--
 drivers/gpu/drm/i915/i915_gem.c|  9 +--
 drivers/gpu/drm/omapdrm/omap_gem.c | 11 ++--
 drivers/gpu/drm/omapdrm/omap_gem_helpers.c | 49 +---
 drivers/gpu/drm/udl/udl_gem.c  |  6 +-
 include/drm/drmP.h |  7 +--
 include/drm/drm_vma_manager.h  |  1 -
 include/uapi/drm/drm.h |  2 +-
 11 files changed, 47 insertions(+), 151 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 88f0322..e1d0f67 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -37,6 +37,7 @@
 #include linux/shmem_fs.h
 #include linux/dma-buf.h
 #include drm/drmP.h
+#include drm/drm_vma_manager.h
 
 /** @file drm_gem.c
  *
@@ -102,14 +103,9 @@ drm_gem_init(struct drm_device *dev)
}
 
dev-mm_private = mm;
-
-   if (drm_ht_create(mm-offset_hash, 12)) {
-   kfree(mm);
-   return -ENOMEM;
-   }
-
-   drm_mm_init(mm-offset_manager, DRM_FILE_PAGE_OFFSET_START,
-   DRM_FILE_PAGE_OFFSET_SIZE);
+   drm_vma_offset_manager_init(mm-vma_manager,
+   DRM_FILE_PAGE_OFFSET_START,
+   DRM_FILE_PAGE_OFFSET_SIZE);
 
return 0;
 }
@@ -119,8 +115,7 @@ drm_gem_destroy(struct drm_device *dev)
 {
struct drm_gem_mm *mm = dev-mm_private;
 
-   drm_mm_takedown(mm-offset_manager);
-   drm_ht_remove(mm-offset_hash);
+   drm_vma_offset_manager_destroy(mm-vma_manager);
kfree(mm);
dev-mm_private = NULL;
 }
@@ -306,12 +301,8 @@ drm_gem_free_mmap_offset(struct drm_gem_object *obj)
 {
struct drm_device *dev = obj-dev;
struct drm_gem_mm *mm = dev-mm_private;
-   struct drm_map_list *list = obj-map_list;
 
-   drm_ht_remove_item(mm-offset_hash, list-hash);
-   drm_mm_put_block(list-file_offset_node);
-   kfree(list-map);
-   list-map = NULL;
+   drm_vma_offset_remove(mm-vma_manager, obj-vma_node);
 }
 EXPORT_SYMBOL(drm_gem_free_mmap_offset);
 
@@ -331,54 +322,9 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj)
 {
struct drm_device *dev = obj-dev;
struct drm_gem_mm *mm = dev-mm_private;
-   struct drm_map_list *list;
-   struct drm_local_map *map;
-   int ret;
-
-   /* Set the object up for mmap'ing */
-   list = obj-map_list;
-   list-map = kzalloc(sizeof(struct drm_map_list), GFP_KERNEL);
-   if (!list-map)
-   return -ENOMEM;
-
-   map = list-map;
-   map-type = _DRM_GEM;
-   map-size = obj-size;
-   map-handle = obj;
-
-   /* Get a DRM GEM mmap offset allocated... */
-   list-file_offset_node = drm_mm_search_free(mm-offset_manager,
-   obj-size / PAGE_SIZE, 0, false);
-
-   if (!list-file_offset_node) {
-   DRM_ERROR(failed to allocate offset for bo %d\n, obj-name);
-   ret = -ENOSPC;
-   goto out_free_list;
-   }
 
-   list-file_offset_node = drm_mm_get_block(list-file_offset_node,
-   obj-size / PAGE_SIZE, 0);
-   if (!list-file_offset_node) {
-   ret = -ENOMEM;
-   goto out_free_list;
-   }
-
-   list-hash.key = list-file_offset_node-start;
-   ret = drm_ht_insert_item(mm-offset_hash, list-hash);
-   if (ret) {
-   DRM_ERROR(failed to add to map hash\n);
-   goto out_free_mm;
-   }
-
-   return 0;
-
-out_free_mm:
-   drm_mm_put_block(list-file_offset_node);
-out_free_list:
-   kfree(list-map);
-   list-map = NULL;
-
-   return ret;
+   return drm_vma_offset_add(mm-vma_manager, obj-vma_node,
+ obj-size / PAGE_SIZE);
 }
 EXPORT_SYMBOL(drm_gem_create_mmap_offset);
 
@@ -660,9 +606,8 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct 
*vma)
struct drm_file *priv = filp-private_data;
struct drm_device *dev = priv-minor-dev;
struct drm_gem_mm *mm = dev-mm_private;
-   struct drm_local_map *map = NULL;
struct drm_gem_object *obj;
-   struct drm_hash_item *hash;
+   struct drm_vma_offset_node *node;
int ret = 0;
 
if (drm_device_is_unplugged(dev))
@@ -670,25 +615,19 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct 

[PATCH 6/6] drm: provide generic drm_vma_node_unmap() helper

[David Herrmann]

Instead of unmapping the nodes in TTM and GEM users manually, we provide
a generic wrapper which does the correct thing for all vma-nodes.

Signed-off-by: David Herrmann dh.herrm...@gmail.com
---
 drivers/gpu/drm/i915/i915_gem.c |  6 +-
 drivers/gpu/drm/ttm/ttm_bo.c|  8 +---
 include/drm/drm_vma_manager.h   | 16 
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1617166..43f9aaf 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1427,11 +1427,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
if (!obj-fault_mappable)
return;
 
-   if (obj-base.dev-dev_mapping)
-   unmap_mapping_range(obj-base.dev-dev_mapping,
-   
(loff_t)drm_vma_node_offset_addr(obj-base.vma_node),
-   obj-base.size, 1);
-
+   drm_vma_node_unmap(obj-base.vma_node, obj-base.dev-dev_mapping);
obj-fault_mappable = false;
 }
 
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 2b96a75..2979070 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1657,17 +1657,11 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, 
struct ttm_mem_reg *mem)
 void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo)
 {
struct ttm_bo_device *bdev = bo-bdev;
-   loff_t offset, holelen;
 
if (!bdev-dev_mapping)
return;
 
-   if (drm_vma_node_has_offset(bo-vma_node)) {
-   offset = (loff_t) drm_vma_node_offset_addr(bo-vma_node);
-   holelen = ((loff_t) bo-mem.num_pages)  PAGE_SHIFT;
-
-   unmap_mapping_range(bdev-dev_mapping, offset, holelen, 1);
-   }
+   drm_vma_node_unmap(bo-vma_node, bdev-dev_mapping);
ttm_mem_io_free_vm(bo);
 }
 
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
index 4d6a734..185e9aa 100644
--- a/include/drm/drm_vma_manager.h
+++ b/include/drm/drm_vma_manager.h
@@ -24,6 +24,7 @@
  */
 
 #include drm/drm_mm.h
+#include linux/mm.h
 #include linux/module.h
 #include linux/rbtree.h
 #include linux/spinlock.h
@@ -103,4 +104,19 @@ static inline __u64 drm_vma_node_offset_addr(struct 
drm_vma_offset_node *node)
return ((__u64)node-vm_node.start)  PAGE_SHIFT;
 }
 
+/** drm_vma_node_unmap()
+ *
+ * Unmap all userspace mappings for a given offset node. The mappings must be
+ * associated with the @file_mapping address-space. If no offset exists or
+ * the address-space is invalid, nothing is done.
+ */
+static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
+ struct address_space *file_mapping)
+{
+   if (file_mapping  drm_vma_node_has_offset(node))
+   unmap_mapping_range(file_mapping,
+   drm_vma_node_offset_addr(node),
+   node-vm_pages  PAGE_SHIFT, 1);
+}
+
 #endif /* __DRM_VMA_MANAGER_H__ */
-- 
1.8.3.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/6] DRM: Unified VMA Offset Manager

[Rob Clark]

On Mon, Jul 1, 2013 at 2:32 PM, David Herrmann dh.herrm...@gmail.com wrote:
 Hi

 I picked up the initial work from Dave [1], fixed several bugs, rewrote the
 drm_mm node handling and adjusted the different drivers.
 The series tries to replace the VMA-offset managers from GEM and TTM with a
 single unified implementation. It uses the TTM RBTree idea to allow 
 sub-mappings
 (which wouldn't be feasible with hashtables).

 Changes to Dave's v1:
  * Fixed a ref-count bug in TTM during object lookup
  * Use embedded drm_mm_node objects to avoid allocations
  * Document drm_vma_* API
  * Reviewed TTM locking
  * Fixed all new drivers
  * Use node-vm_pages instead of obj-size for GEM size calculations

 Notes:
  * Tested on nouveau only! I will try to test i915 this week. However, the
gem changes seem pretty trivial.
  * I couldn't even compile-test the ARM drivers. However, the omapdrm diffs
are the only changes that are non-trivial. Is there any ongoing work to
remove the arch-deps in DRM drivers?

I think most of the arm drivers should support ARCH_MULTIPLATFORM now,
so at least if you have a cross compiler it should be pretty easy to
compile-test most of the arm drm drivers..

BR,
-R

  * _DRM_GEM is no longer used, but I guess we need to keep it for backwards
compat?
  * If we replace node_list in drm_mm with an rbtree, we can drop it from
drm_vma_offset_manager completely. However, I wanted to avoid heavy drm_mm
changes and left this for follow up patches.
  * This is currently based on linux-3.10 from today. Next series will be
rebased on drm-next/linux-next, but the current -next trees continously 
 break
my machines..
But the only changes should be to fix additional drivers. I didn't see any
other things to fix for drm-next.

 Another series, which I will send later, adds struct file lists for each
 drm-vma-offset so we can get access control over gem objects. Also, I have an
 experimental series to remove the allocation helpers in drm_mm and let drivers
 embed drm_mm_node instead. Lets see how that works out.

 Comments welcome!
 Cheers
 David

 [1]: http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-vma-manager

 David Herrmann (6):
   drm: make drm_mm_init() return void
   drm: mm: add drm_mm_node_linked() helper
   drm: add unified vma offset manager
   drm: gem: convert to new unified vma manager
   drm: ttm: convert to unified vma offset manager
   drm: provide generic drm_vma_node_unmap() helper

  drivers/gpu/drm/Makefile   |   2 +-
  drivers/gpu/drm/ast/ast_main.c |   2 +-
  drivers/gpu/drm/cirrus/cirrus_main.c   |   2 +-
  drivers/gpu/drm/drm_gem.c  |  93 ++--
  drivers/gpu/drm/drm_gem_cma_helper.c   |   9 +-
  drivers/gpu/drm/drm_mm.c   |   5 +-
  drivers/gpu/drm/drm_vma_manager.c  | 224 
 +
  drivers/gpu/drm/exynos/exynos_drm_gem.c|   7 +-
  drivers/gpu/drm/gma500/gem.c   |   8 +-
  drivers/gpu/drm/i915/i915_gem.c|  13 +-
  drivers/gpu/drm/mgag200/mgag200_main.c |   2 +-
  drivers/gpu/drm/nouveau/nouveau_display.c  |   2 +-
  drivers/gpu/drm/nouveau/nouveau_gem.c  |   2 +-
  drivers/gpu/drm/omapdrm/omap_gem.c |  11 +-
  drivers/gpu/drm/omapdrm/omap_gem_helpers.c |  49 +--
  drivers/gpu/drm/qxl/qxl_object.h   |   2 +-
  drivers/gpu/drm/qxl/qxl_release.c  |   2 +-
  drivers/gpu/drm/radeon/radeon_object.h |   5 +-
  drivers/gpu/drm/ttm/ttm_bo.c   |  82 ++-
  drivers/gpu/drm/ttm/ttm_bo_manager.c   |   8 +-
  drivers/gpu/drm/ttm/ttm_bo_util.c  |   3 +-
  drivers/gpu/drm/ttm/ttm_bo_vm.c|  81 ---
  drivers/gpu/drm/udl/udl_gem.c  |   6 +-
  drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |   4 +-
  include/drm/drmP.h |   7 +-
  include/drm/drm_mm.h   |  11 +-
  include/drm/drm_vma_manager.h  | 122 
  include/drm/ttm/ttm_bo_api.h   |  15 +-
  include/drm/ttm/ttm_bo_driver.h|   7 +-
  include/uapi/drm/drm.h |   2 +-
  30 files changed, 464 insertions(+), 324 deletions(-)
  create mode 100644 drivers/gpu/drm/drm_vma_manager.c
  create mode 100644 include/drm/drm_vma_manager.h

 --
 1.8.3.2

 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/6] drm: make drm_mm_init() return void

[Daniel Vetter]

On Mon, Jul 01, 2013 at 08:32:58PM +0200, David Herrmann wrote:
 There is no reason to return int as this function never fails.
 Furthermore, several drivers (ast, sis) already depend on this.
 
 Signed-off-by: David Herrmann dh.herrm...@gmail.com

Back when I've reworked drm_mm I was still a rookie and didn't want to
touch all drivers, hence why I've left the int return type. Good riddance
to that!

Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/drm_gem.c| 8 ++--
  drivers/gpu/drm/drm_mm.c | 4 +---
  drivers/gpu/drm/ttm/ttm_bo.c | 6 +-
  drivers/gpu/drm/ttm/ttm_bo_manager.c | 8 +---
  include/drm/drm_mm.h | 6 +++---
  5 files changed, 8 insertions(+), 24 deletions(-)
 
 diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
 index cf919e3..88f0322 100644
 --- a/drivers/gpu/drm/drm_gem.c
 +++ b/drivers/gpu/drm/drm_gem.c
 @@ -108,12 +108,8 @@ drm_gem_init(struct drm_device *dev)
   return -ENOMEM;
   }
  
 - if (drm_mm_init(mm-offset_manager, DRM_FILE_PAGE_OFFSET_START,
 - DRM_FILE_PAGE_OFFSET_SIZE)) {
 - drm_ht_remove(mm-offset_hash);
 - kfree(mm);
 - return -ENOMEM;
 - }
 + drm_mm_init(mm-offset_manager, DRM_FILE_PAGE_OFFSET_START,
 + DRM_FILE_PAGE_OFFSET_SIZE);
  
   return 0;
  }
 diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
 index 07cf99c..7917729 100644
 --- a/drivers/gpu/drm/drm_mm.c
 +++ b/drivers/gpu/drm/drm_mm.c
 @@ -669,7 +669,7 @@ int drm_mm_clean(struct drm_mm * mm)
  }
  EXPORT_SYMBOL(drm_mm_clean);
  
 -int drm_mm_init(struct drm_mm * mm, unsigned long start, unsigned long size)
 +void drm_mm_init(struct drm_mm * mm, unsigned long start, unsigned long size)
  {
   INIT_LIST_HEAD(mm-hole_stack);
   INIT_LIST_HEAD(mm-unused_nodes);
 @@ -690,8 +690,6 @@ int drm_mm_init(struct drm_mm * mm, unsigned long start, 
 unsigned long size)
   list_add_tail(mm-head_node.hole_stack, mm-hole_stack);
  
   mm-color_adjust = NULL;
 -
 - return 0;
  }
  EXPORT_SYMBOL(drm_mm_init);
  
 diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
 index 9b07b7d..e97c5a0 100644
 --- a/drivers/gpu/drm/ttm/ttm_bo.c
 +++ b/drivers/gpu/drm/ttm/ttm_bo.c
 @@ -1619,9 +1619,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
   goto out_no_sys;
  
   bdev-addr_space_rb = RB_ROOT;
 - ret = drm_mm_init(bdev-addr_space_mm, file_page_offset, 0x1000);
 - if (unlikely(ret != 0))
 - goto out_no_addr_mm;
 + drm_mm_init(bdev-addr_space_mm, file_page_offset, 0x1000);
  
   INIT_DELAYED_WORK(bdev-wq, ttm_bo_delayed_workqueue);
   INIT_LIST_HEAD(bdev-ddestroy);
 @@ -1635,8 +1633,6 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
   mutex_unlock(glob-device_list_mutex);
  
   return 0;
 -out_no_addr_mm:
 - ttm_bo_clean_mm(bdev, 0);
  out_no_sys:
   return ret;
  }
 diff --git a/drivers/gpu/drm/ttm/ttm_bo_manager.c 
 b/drivers/gpu/drm/ttm/ttm_bo_manager.c
 index 9212494..e4367f9 100644
 --- a/drivers/gpu/drm/ttm/ttm_bo_manager.c
 +++ b/drivers/gpu/drm/ttm/ttm_bo_manager.c
 @@ -103,18 +103,12 @@ static int ttm_bo_man_init(struct ttm_mem_type_manager 
 *man,
  unsigned long p_size)
  {
   struct ttm_range_manager *rman;
 - int ret;
  
   rman = kzalloc(sizeof(*rman), GFP_KERNEL);
   if (!rman)
   return -ENOMEM;
  
 - ret = drm_mm_init(rman-mm, 0, p_size);
 - if (ret) {
 - kfree(rman);
 - return ret;
 - }
 -
 + drm_mm_init(rman-mm, 0, p_size);
   spin_lock_init(rman-lock);
   man-priv = rman;
   return 0;
 diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
 index 88591ef..de92425 100644
 --- a/include/drm/drm_mm.h
 +++ b/include/drm/drm_mm.h
 @@ -275,9 +275,9 @@ static inline  struct drm_mm_node 
 *drm_mm_search_free_in_range_color(
   return drm_mm_search_free_in_range_generic(mm, size, alignment, color,
  start, end, best_match);
  }
 -extern int drm_mm_init(struct drm_mm *mm,
 -unsigned long start,
 -unsigned long size);
 +extern void drm_mm_init(struct drm_mm *mm,
 + unsigned long start,
 + unsigned long size);
  extern void drm_mm_takedown(struct drm_mm *mm);
  extern int drm_mm_clean(struct drm_mm *mm);
  extern int drm_mm_pre_get(struct drm_mm *mm);
 -- 
 1.8.3.2
 
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org

Re: [PATCH 2/6] drm: mm: add drm_mm_node_linked() helper

[Daniel Vetter]

On Mon, Jul 01, 2013 at 08:32:59PM +0200, David Herrmann wrote:
 This helper tests whether a given node is currently linked into a drm_mm
 manager. We use the node-mm pointer for that as it is set for all linked
 objects.
 
 We also reset node-mm whenever a node is removed. All such access is
 currently safe as everyone calls kfree() on the object directly after
 removing it. Furthermore, no-one is supposed to access node-mm from
 outside drm_mm.c, anyway.
 
 Signed-off-by: David Herrmann dh.herrm...@gmail.com
 ---
  drivers/gpu/drm/drm_mm.c | 1 +
  include/drm/drm_mm.h | 5 +
  2 files changed, 6 insertions(+)
 
 diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
 index 7917729..3bd43ce 100644
 --- a/drivers/gpu/drm/drm_mm.c
 +++ b/drivers/gpu/drm/drm_mm.c
 @@ -374,6 +374,7 @@ void drm_mm_remove_node(struct drm_mm_node *node)
  
   list_del(node-node_list);
   node-allocated = 0;
 + node-mm = NULL;
  }
  EXPORT_SYMBOL(drm_mm_remove_node);
  
 diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
 index de92425..d83b966 100644
 --- a/include/drm/drm_mm.h
 +++ b/include/drm/drm_mm.h
 @@ -85,6 +85,11 @@ static inline bool drm_mm_node_allocated(struct 
 drm_mm_node *node)
   return node-allocated;
  }
  
 +static inline bool drm_mm_node_linked(struct drm_mm_node *node)
 +{
 + return node-mm;
 +}

I've only checked a few examples, but this seems to duplicate what the
drm_mm_node_allocated right above was meant for. Or do I miss something
here?
-Daniel

 +
  static inline bool drm_mm_initialized(struct drm_mm *mm)
  {
   return mm-hole_stack.next;
 -- 
 1.8.3.2
 
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/6] drm: add unified vma offset manager

[Daniel Vetter]

On Mon, Jul 01, 2013 at 08:33:00PM +0200, David Herrmann wrote:
 If we want to map GPU memory into user-space, we need to linearize the
 addresses to not confuse mm-core. Currently, GEM and TTM both implement
 their own offset-managers to assign a pgoff to each object for user-space
 CPU access. GEM uses a hash-table, TTM uses an rbtree.
 
 This patch provides a unified implementation that can be used to replace
 both. TTM allows partial mmaps with a given offset, so we cannot use
 hashtables as the start address may not be known at mmap time. Hence, we
 use the rbtree-implementation of TTM.
 
 We could easily update drm_mm to use an rbtree instead of a linked list
 for it's object list and thus drop the rbtree from the vma-manager.
 However, this would slow down drm_mm object allocation for all other
 use-cases (rbtree insertion) and add another 4-8 bytes to each mm node.
 Hence, use the separate tree but allow for later migration.
 
 This is a rewrite of the 2012-proposal by David Airlie airl...@linux.ie
 
 Signed-off-by: David Herrmann dh.herrm...@gmail.com

This seems to sprinkle a lot of likely/unlikely. Imo those are only
justified in two cases:
- When we have real performance data proving that they're worth it.
- Sometimes they're also useful as self-documenting code for the truly
  unlikely. But early do-nothing returns and error handling are
  established code patterns, so imo don't qualify.

I think none of the likely/unlikely below qualify, so imo better to remove
them.

Second high-level review request: Can you please convert the very nice
documentation you already added into proper kerneldoc and link it up at an
appropriate section in the drm DocBook?

A few more comments below. I'll postpone reviewing the gem/ttm conversion
patches until this is settled a bit.

 ---
  drivers/gpu/drm/Makefile  |   2 +-
  drivers/gpu/drm/drm_vma_manager.c | 224 
 ++
  include/drm/drm_vma_manager.h | 107 ++
  3 files changed, 332 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/drm_vma_manager.c
  create mode 100644 include/drm/drm_vma_manager.h
 
 diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
 index 1c9f2439..f8851cc 100644
 --- a/drivers/gpu/drm/Makefile
 +++ b/drivers/gpu/drm/Makefile
 @@ -12,7 +12,7 @@ drm-y   :=  drm_auth.o drm_buffer.o drm_bufs.o 
 drm_cache.o \
   drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
   drm_crtc.o drm_modes.o drm_edid.o \
   drm_info.o drm_debugfs.o drm_encoder_slave.o \
 - drm_trace_points.o drm_global.o drm_prime.o
 + drm_trace_points.o drm_global.o drm_prime.o drm_vma_manager.o
  
  drm-$(CONFIG_COMPAT) += drm_ioc32.o
  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
 diff --git a/drivers/gpu/drm/drm_vma_manager.c 
 b/drivers/gpu/drm/drm_vma_manager.c
 new file mode 100644
 index 000..c28639f
 --- /dev/null
 +++ b/drivers/gpu/drm/drm_vma_manager.c
 @@ -0,0 +1,224 @@
 +/*
 + * Copyright (c) 2012 David Airlie airl...@linux.ie
 + * Copyright (c) 2013 David Herrmann dh.herrm...@gmail.com
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a
 + * copy of this software and associated documentation files (the Software),
 + * to deal in the Software without restriction, including without limitation
 + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 + * and/or sell copies of the Software, and to permit persons to whom the
 + * Software is furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice shall be included in
 + * all copies or substantial portions of the Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
 + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
 + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
 + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
 + * OTHER DEALINGS IN THE SOFTWARE.
 + */
 +
 +#include drm/drmP.h
 +#include drm/drm_mm.h
 +#include drm/drm_vma_manager.h
 +#include linux/mm.h
 +#include linux/module.h
 +#include linux/rbtree.h
 +#include linux/slab.h
 +#include linux/spinlock.h
 +#include linux/types.h
 +
 +/** @file drm_vma_manager.c
 + *
 + * The vma-manager is responsible to map arbitrary driver-dependent memory
 + * regions into the linear user address-space. It provides offsets to the
 + * caller which can then be used on the address_space of the drm-device. It
 + * takes care to not overlap regions, size them appropriately and to not
 + * confuse mm-core by inconsistent fake vm_pgoff fields.
 + *
 + * We use drm_mm as backend to manage object allocations. But it is highly
 + * 

Re: [PATCH 6/6] drm: provide generic drm_vma_node_unmap() helper

[Daniel Vetter]

On Mon, Jul 01, 2013 at 08:33:03PM +0200, David Herrmann wrote:
 Instead of unmapping the nodes in TTM and GEM users manually, we provide
 a generic wrapper which does the correct thing for all vma-nodes.
 
 Signed-off-by: David Herrmann dh.herrm...@gmail.com

Nice. One nitpick below, otherwise:

Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch

 ---
  drivers/gpu/drm/i915/i915_gem.c |  6 +-
  drivers/gpu/drm/ttm/ttm_bo.c|  8 +---
  include/drm/drm_vma_manager.h   | 16 
  3 files changed, 18 insertions(+), 12 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index 1617166..43f9aaf 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -1427,11 +1427,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
   if (!obj-fault_mappable)
   return;
  
 - if (obj-base.dev-dev_mapping)
 - unmap_mapping_range(obj-base.dev-dev_mapping,
 - 
 (loff_t)drm_vma_node_offset_addr(obj-base.vma_node),
 - obj-base.size, 1);
 -
 + drm_vma_node_unmap(obj-base.vma_node, obj-base.dev-dev_mapping);
   obj-fault_mappable = false;
  }
  
 diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
 index 2b96a75..2979070 100644
 --- a/drivers/gpu/drm/ttm/ttm_bo.c
 +++ b/drivers/gpu/drm/ttm/ttm_bo.c
 @@ -1657,17 +1657,11 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, 
 struct ttm_mem_reg *mem)
  void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo)
  {
   struct ttm_bo_device *bdev = bo-bdev;
 - loff_t offset, holelen;
  
   if (!bdev-dev_mapping)
   return;

Can't we kill this check now since your helper checks for the mapping already?

  
 - if (drm_vma_node_has_offset(bo-vma_node)) {
 - offset = (loff_t) drm_vma_node_offset_addr(bo-vma_node);
 - holelen = ((loff_t) bo-mem.num_pages)  PAGE_SHIFT;
 -
 - unmap_mapping_range(bdev-dev_mapping, offset, holelen, 1);
 - }
 + drm_vma_node_unmap(bo-vma_node, bdev-dev_mapping);
   ttm_mem_io_free_vm(bo);
  }
  
 diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
 index 4d6a734..185e9aa 100644
 --- a/include/drm/drm_vma_manager.h
 +++ b/include/drm/drm_vma_manager.h
 @@ -24,6 +24,7 @@
   */
  
  #include drm/drm_mm.h
 +#include linux/mm.h
  #include linux/module.h
  #include linux/rbtree.h
  #include linux/spinlock.h
 @@ -103,4 +104,19 @@ static inline __u64 drm_vma_node_offset_addr(struct 
 drm_vma_offset_node *node)
   return ((__u64)node-vm_node.start)  PAGE_SHIFT;
  }
  
 +/** drm_vma_node_unmap()
 + *
 + * Unmap all userspace mappings for a given offset node. The mappings must be
 + * associated with the @file_mapping address-space. If no offset exists or
 + * the address-space is invalid, nothing is done.
 + */
 +static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
 +   struct address_space *file_mapping)
 +{
 + if (file_mapping  drm_vma_node_has_offset(node))
 + unmap_mapping_range(file_mapping,
 + drm_vma_node_offset_addr(node),
 + node-vm_pages  PAGE_SHIFT, 1);
 +}
 +
  #endif /* __DRM_VMA_MANAGER_H__ */
 -- 
 1.8.3.2
 
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/6] DRM: Unified VMA Offset Manager

[Daniel Vetter]

On Mon, Jul 01, 2013 at 08:32:57PM +0200, David Herrmann wrote:
 Hi
 
 I picked up the initial work from Dave [1], fixed several bugs, rewrote the
 drm_mm node handling and adjusted the different drivers.
 The series tries to replace the VMA-offset managers from GEM and TTM with a
 single unified implementation. It uses the TTM RBTree idea to allow 
 sub-mappings
 (which wouldn't be feasible with hashtables).
 
 Changes to Dave's v1:
  * Fixed a ref-count bug in TTM during object lookup
  * Use embedded drm_mm_node objects to avoid allocations
  * Document drm_vma_* API
  * Reviewed TTM locking
  * Fixed all new drivers
  * Use node-vm_pages instead of obj-size for GEM size calculations
 
 Notes:
  * Tested on nouveau only! I will try to test i915 this week. However, the
gem changes seem pretty trivial.
  * I couldn't even compile-test the ARM drivers. However, the omapdrm diffs
are the only changes that are non-trivial. Is there any ongoing work to
remove the arch-deps in DRM drivers?
  * _DRM_GEM is no longer used, but I guess we need to keep it for backwards
compat?

I seriously hope no piece of userspace ever used that. So if you can do
some history digging and that indeed turns out to be the case, then I'd
vote to kill _DRM_GEM. Makes it much clearer where the dri1 dungeons are
if there's no new-world stuff interspersed by accident ;-)
-Daniel

  * If we replace node_list in drm_mm with an rbtree, we can drop it from
drm_vma_offset_manager completely. However, I wanted to avoid heavy drm_mm
changes and left this for follow up patches.
  * This is currently based on linux-3.10 from today. Next series will be
rebased on drm-next/linux-next, but the current -next trees continously 
 break
my machines..
But the only changes should be to fix additional drivers. I didn't see any
other things to fix for drm-next.
 
 Another series, which I will send later, adds struct file lists for each
 drm-vma-offset so we can get access control over gem objects. Also, I have an
 experimental series to remove the allocation helpers in drm_mm and let drivers
 embed drm_mm_node instead. Lets see how that works out.
 
 Comments welcome!
 Cheers
 David
 
 [1]: http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-vma-manager
 
 David Herrmann (6):
   drm: make drm_mm_init() return void
   drm: mm: add drm_mm_node_linked() helper
   drm: add unified vma offset manager
   drm: gem: convert to new unified vma manager
   drm: ttm: convert to unified vma offset manager
   drm: provide generic drm_vma_node_unmap() helper
 
  drivers/gpu/drm/Makefile   |   2 +-
  drivers/gpu/drm/ast/ast_main.c |   2 +-
  drivers/gpu/drm/cirrus/cirrus_main.c   |   2 +-
  drivers/gpu/drm/drm_gem.c  |  93 ++--
  drivers/gpu/drm/drm_gem_cma_helper.c   |   9 +-
  drivers/gpu/drm/drm_mm.c   |   5 +-
  drivers/gpu/drm/drm_vma_manager.c  | 224 
 +
  drivers/gpu/drm/exynos/exynos_drm_gem.c|   7 +-
  drivers/gpu/drm/gma500/gem.c   |   8 +-
  drivers/gpu/drm/i915/i915_gem.c|  13 +-
  drivers/gpu/drm/mgag200/mgag200_main.c |   2 +-
  drivers/gpu/drm/nouveau/nouveau_display.c  |   2 +-
  drivers/gpu/drm/nouveau/nouveau_gem.c  |   2 +-
  drivers/gpu/drm/omapdrm/omap_gem.c |  11 +-
  drivers/gpu/drm/omapdrm/omap_gem_helpers.c |  49 +--
  drivers/gpu/drm/qxl/qxl_object.h   |   2 +-
  drivers/gpu/drm/qxl/qxl_release.c  |   2 +-
  drivers/gpu/drm/radeon/radeon_object.h |   5 +-
  drivers/gpu/drm/ttm/ttm_bo.c   |  82 ++-
  drivers/gpu/drm/ttm/ttm_bo_manager.c   |   8 +-
  drivers/gpu/drm/ttm/ttm_bo_util.c  |   3 +-
  drivers/gpu/drm/ttm/ttm_bo_vm.c|  81 ---
  drivers/gpu/drm/udl/udl_gem.c  |   6 +-
  drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |   4 +-
  include/drm/drmP.h |   7 +-
  include/drm/drm_mm.h   |  11 +-
  include/drm/drm_vma_manager.h  | 122 
  include/drm/ttm/ttm_bo_api.h   |  15 +-
  include/drm/ttm/ttm_bo_driver.h|   7 +-
  include/uapi/drm/drm.h |   2 +-
  30 files changed, 464 insertions(+), 324 deletions(-)
  create mode 100644 drivers/gpu/drm/drm_vma_manager.c
  create mode 100644 include/drm/drm_vma_manager.h
 
 -- 
 1.8.3.2
 
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/6] drm: add unified vma offset manager

[David Herrmann]

Hi

On Mon, Jul 1, 2013 at 9:42 PM, Daniel Vetter dan...@ffwll.ch wrote:
 On Mon, Jul 01, 2013 at 08:33:00PM +0200, David Herrmann wrote:
 If we want to map GPU memory into user-space, we need to linearize the
 addresses to not confuse mm-core. Currently, GEM and TTM both implement
 their own offset-managers to assign a pgoff to each object for user-space
 CPU access. GEM uses a hash-table, TTM uses an rbtree.

 This patch provides a unified implementation that can be used to replace
 both. TTM allows partial mmaps with a given offset, so we cannot use
 hashtables as the start address may not be known at mmap time. Hence, we
 use the rbtree-implementation of TTM.

 We could easily update drm_mm to use an rbtree instead of a linked list
 for it's object list and thus drop the rbtree from the vma-manager.
 However, this would slow down drm_mm object allocation for all other
 use-cases (rbtree insertion) and add another 4-8 bytes to each mm node.
 Hence, use the separate tree but allow for later migration.

 This is a rewrite of the 2012-proposal by David Airlie airl...@linux.ie

 Signed-off-by: David Herrmann dh.herrm...@gmail.com

 This seems to sprinkle a lot of likely/unlikely. Imo those are only
 justified in two cases:
 - When we have real performance data proving that they're worth it.
 - Sometimes they're also useful as self-documenting code for the truly
   unlikely. But early do-nothing returns and error handling are
   established code patterns, so imo don't qualify.

 I think none of the likely/unlikely below qualify, so imo better to remove
 them.

They are copied mostly from ttm_bo.c. However, I agree that they are
probably unneeded. Furthermore, these aren't code-paths I'd expect to
be performance-critical. I will dig into the git-history of TTM to see
whether they have been in-place since the beginning or whether there
is some real reason for them.

 Second high-level review request: Can you please convert the very nice
 documentation you already added into proper kerneldoc and link it up at an
 appropriate section in the drm DocBook?

Sure.

 A few more comments below. I'll postpone reviewing the gem/ttm conversion
 patches until this is settled a bit.

 ---
  drivers/gpu/drm/Makefile  |   2 +-
  drivers/gpu/drm/drm_vma_manager.c | 224 
 ++
  include/drm/drm_vma_manager.h | 107 ++
  3 files changed, 332 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/drm_vma_manager.c
  create mode 100644 include/drm/drm_vma_manager.h

 diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
 index 1c9f2439..f8851cc 100644
 --- a/drivers/gpu/drm/Makefile
 +++ b/drivers/gpu/drm/Makefile
 @@ -12,7 +12,7 @@ drm-y   :=  drm_auth.o drm_buffer.o drm_bufs.o 
 drm_cache.o \
   drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
   drm_crtc.o drm_modes.o drm_edid.o \
   drm_info.o drm_debugfs.o drm_encoder_slave.o \
 - drm_trace_points.o drm_global.o drm_prime.o
 + drm_trace_points.o drm_global.o drm_prime.o drm_vma_manager.o

  drm-$(CONFIG_COMPAT) += drm_ioc32.o
  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
 diff --git a/drivers/gpu/drm/drm_vma_manager.c 
 b/drivers/gpu/drm/drm_vma_manager.c
 new file mode 100644
 index 000..c28639f
 --- /dev/null
 +++ b/drivers/gpu/drm/drm_vma_manager.c
 @@ -0,0 +1,224 @@
 +/*
 + * Copyright (c) 2012 David Airlie airl...@linux.ie
 + * Copyright (c) 2013 David Herrmann dh.herrm...@gmail.com
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a
 + * copy of this software and associated documentation files (the 
 Software),
 + * to deal in the Software without restriction, including without limitation
 + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 + * and/or sell copies of the Software, and to permit persons to whom the
 + * Software is furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice shall be included 
 in
 + * all copies or substantial portions of the Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS 
 OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
 + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
 + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
 + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
 + * OTHER DEALINGS IN THE SOFTWARE.
 + */
 +
 +#include drm/drmP.h
 +#include drm/drm_mm.h
 +#include drm/drm_vma_manager.h
 +#include linux/mm.h
 +#include linux/module.h
 +#include linux/rbtree.h
 +#include linux/slab.h
 +#include linux/spinlock.h
 +#include linux/types.h
 +
 +/** @file drm_vma_manager.c
 + *
 + * The vma-manager is responsible to map 

[PATCH 1/3] drm/mm: fix debug table BUG

[Daniel Vetter]

In

commit 3a359f0b21ab218c1bf7a6a1b638b6fd143d0b99
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date:   Sat Apr 20 12:08:11 2013 +0200

drm/mm: fix dump table BUG

I've failed to fix both instances of the regression introduced in

commit 9e8944ab564f2e3dde90a518cd32048c58918608
Author: Chris Wilson ch...@chris-wilson.co.uk
Date:   Thu Nov 15 11:32:17 2012 +

   drm: Introduce an iterator over holes in the drm_mm range manager

Patch this up in the same way by extracting the hole debug logic
into it's own function, since that'll also clarify the logic a bit.

Cc: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/drm_mm.c | 37 +++--
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 07cf99c..f9d4873 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -716,36 +716,37 @@ void drm_mm_takedown(struct drm_mm * mm)
 }
 EXPORT_SYMBOL(drm_mm_takedown);
 
-void drm_mm_debug_table(struct drm_mm *mm, const char *prefix)
+static unsigned long drm_mm_debug_hole(struct drm_mm_node *entry,
+  const char *prefix)
 {
-   struct drm_mm_node *entry;
-   unsigned long total_used = 0, total_free = 0, total = 0;
unsigned long hole_start, hole_end, hole_size;
 
-   hole_start = drm_mm_hole_node_start(mm-head_node);
-   hole_end = drm_mm_hole_node_end(mm-head_node);
-   hole_size = hole_end - hole_start;
-   if (hole_size)
+   if (entry-hole_follows) {
+   hole_start = drm_mm_hole_node_start(entry);
+   hole_end = drm_mm_hole_node_end(entry);
+   hole_size = hole_end - hole_start;
printk(KERN_DEBUG %s 0x%08lx-0x%08lx: %8lu: free\n,
prefix, hole_start, hole_end,
hole_size);
-   total_free += hole_size;
+   return hole_size;
+   }
+
+   return 0;
+}
+
+void drm_mm_debug_table(struct drm_mm *mm, const char *prefix)
+{
+   struct drm_mm_node *entry;
+   unsigned long total_used = 0, total_free = 0, total = 0;
+
+   total_free += drm_mm_debug_hole(mm-head_node, prefix);
 
drm_mm_for_each_node(entry, mm) {
printk(KERN_DEBUG %s 0x%08lx-0x%08lx: %8lu: used\n,
prefix, entry-start, entry-start + entry-size,
entry-size);
total_used += entry-size;
-
-   if (entry-hole_follows) {
-   hole_start = drm_mm_hole_node_start(entry);
-   hole_end = drm_mm_hole_node_end(entry);
-   hole_size = hole_end - hole_start;
-   printk(KERN_DEBUG %s 0x%08lx-0x%08lx: %8lu: free\n,
-   prefix, hole_start, hole_end,
-   hole_size);
-   total_free += hole_size;
-   }
+   total_free += drm_mm_debug_hole(entry, prefix);
}
total = total_free + total_used;
 
-- 
1.8.3.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/i915: Don't try to tear down the stolen drm_mm if it's not there

[Daniel Vetter]

Every other place properly checks whether we've managed to set
up the stolen allocator at boot-up properly, with the exception
of the cleanup code. Which results in an ugly

*ERROR* Memory manager not clean. Delaying takedown

at module unload time since the drm_mm isn't initialized at all.

v2: While at it check whether the stolen drm_mm is initialized instead
of the more obscure stolen_base == 0 check.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65953
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 8e02344..32e63a8 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -147,7 +147,7 @@ int i915_gem_stolen_setup_compression(struct drm_device 
*dev, int size)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
 
-   if (dev_priv-mm.stolen_base == 0)
+   if (drm_mm_initialized(dev_priv-mm.stolen))
return -ENODEV;
 
if (size  dev_priv-fbc.size)
@@ -179,6 +179,9 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
 
+   if (drm_mm_initialized(dev_priv-mm.stolen))
+   return;
+
i915_gem_stolen_cleanup_compression(dev);
drm_mm_takedown(dev_priv-mm.stolen);
 }
@@ -300,7 +303,7 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 
size)
struct drm_i915_gem_object *obj;
struct drm_mm_node *stolen;
 
-   if (dev_priv-mm.stolen_base == 0)
+   if (drm_mm_initialized(dev_priv-mm.stolen))
return NULL;
 
DRM_DEBUG_KMS(creating stolen object: size=%x\n, size);
@@ -331,7 +334,7 @@ i915_gem_object_create_stolen_for_preallocated(struct 
drm_device *dev,
struct drm_i915_gem_object *obj;
struct drm_mm_node *stolen;
 
-   if (dev_priv-mm.stolen_base == 0)
+   if (drm_mm_initialized(dev_priv-mm.stolen))
return NULL;
 
DRM_DEBUG_KMS(creating preallocated stolen object: stolen_offset=%x, 
gtt_offset=%x, size=%x\n,
-- 
1.8.3.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/3] drm/mm: WARN for unclean mm takedown

[Daniel Vetter]

The usual drm driver has tons of different drm_mm memory managers so the drm
error message in dmesg is pretty useless. WARN instead so that we have the full
backtrace.

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/drm_mm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index f9d4873..d303e31 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -699,8 +699,8 @@ void drm_mm_takedown(struct drm_mm * mm)
 {
struct drm_mm_node *entry, *next;
 
-   if (!list_empty(mm-head_node.node_list)) {
-   DRM_ERROR(Memory manager not clean. Delaying takedown\n);
+   if (WARN(!list_empty(mm-head_node.node_list),
+Memory manager not clean. Delaying takedown\n)) {
return;
}
 
-- 
1.8.3.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/mm: kill color_search_free/get_block

[Daniel Vetter]

drm/i915 is the only user of the color allocation handling and
switched to insert_node a while ago. So we can ditch this.

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 include/drm/drm_mm.h | 31 ---
 1 file changed, 31 deletions(-)

diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
index 88591ef..ab240f1 100644
--- a/include/drm/drm_mm.h
+++ b/include/drm/drm_mm.h
@@ -177,17 +177,6 @@ static inline struct drm_mm_node *drm_mm_get_block_range(
return drm_mm_get_block_range_generic(parent, size, alignment, 0,
  start, end, 0);
 }
-static inline struct drm_mm_node *drm_mm_get_color_block_range(
-   struct drm_mm_node *parent,
-   unsigned long size,
-   unsigned alignment,
-   unsigned long color,
-   unsigned long start,
-   unsigned long end)
-{
-   return drm_mm_get_block_range_generic(parent, size, alignment, color,
- start, end, 0);
-}
 static inline struct drm_mm_node *drm_mm_get_block_atomic_range(
struct drm_mm_node *parent,
unsigned long size,
@@ -255,26 +244,6 @@ static inline  struct drm_mm_node 
*drm_mm_search_free_in_range(
return drm_mm_search_free_in_range_generic(mm, size, alignment, 0,
   start, end, best_match);
 }
-static inline struct drm_mm_node *drm_mm_search_free_color(const struct drm_mm 
*mm,
-  unsigned long size,
-  unsigned alignment,
-  unsigned long color,
-  bool best_match)
-{
-   return drm_mm_search_free_generic(mm,size, alignment, color, 
best_match);
-}
-static inline  struct drm_mm_node *drm_mm_search_free_in_range_color(
-   const struct drm_mm *mm,
-   unsigned long size,
-   unsigned alignment,
-   unsigned long color,
-   unsigned long start,
-   unsigned long end,
-   bool best_match)
-{
-   return drm_mm_search_free_in_range_generic(mm, size, alignment, color,
-  start, end, best_match);
-}
 extern int drm_mm_init(struct drm_mm *mm,
   unsigned long start,
   unsigned long size);
-- 
1.7.11.7

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/i915: use drm_mm_remove_node instead of put_block

[Daniel Vetter]

When converting to the preallocated drm_mm_node interfaces in

commit dc9dd7a20fde95aa81a8307cde79c2dff9f83f3d
Author: Chris Wilson ch...@chris-wilson.co.uk
Date:   Fri Dec 7 20:37:07 2012 +

drm/i915: Preallocate the drm_mm_node prior to manipulating the GTT drm_mm

only the allocation side was converted, but not the freeing. Fix this
up.

Note that the only difference between put_block and remove_node is
that the former fills up the preallocation cache. Which we don't need
anyway and hence is just wasted space.

Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_gem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4200c32..30fd694 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2620,7 +2620,7 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj)
/* Avoid an unnecessary call to unbind on rebind. */
obj-map_and_fenceable = true;
 
-   drm_mm_put_block(obj-gtt_space);
+   drm_mm_remove_node(obj-gtt_space);
obj-gtt_space = NULL;
obj-gtt_offset = 0;
 
@@ -3137,14 +3137,14 @@ search_free:
}
if (WARN_ON(!i915_gem_valid_gtt_space(dev, node, obj-cache_level))) {
i915_gem_object_unpin_pages(obj);
-   drm_mm_put_block(node);
+   drm_mm_remove_node(node);
return -EINVAL;
}
 
ret = i915_gem_gtt_prepare_object(obj);
if (ret) {
i915_gem_object_unpin_pages(obj);
-   drm_mm_put_block(node);
+   drm_mm_remove_node(node);
return ret;
}
 
-- 
1.7.11.7

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/i915: Don't try to tear down the stolen drm_mm if it's not there

[Chris Wilson]

On Mon, Jul 01, 2013 at 10:01:03PM +0200, Daniel Vetter wrote:
 Every other place properly checks whether we've managed to set
 up the stolen allocator at boot-up properly, with the exception
 of the cleanup code. Which results in an ugly
 
 *ERROR* Memory manager not clean. Delaying takedown
 
 at module unload time since the drm_mm isn't initialized at all.
 
 v2: While at it check whether the stolen drm_mm is initialized instead
 of the more obscure stolen_base == 0 check.
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65953
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_gem_stolen.c | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
 b/drivers/gpu/drm/i915/i915_gem_stolen.c
 index 8e02344..32e63a8 100644
 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
 +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
 @@ -147,7 +147,7 @@ int i915_gem_stolen_setup_compression(struct drm_device 
 *dev, int size)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
  
 - if (dev_priv-mm.stolen_base == 0)
 + if (drm_mm_initialized(dev_priv-mm.stolen))

Logically reversed? Which is more obscure now :-p
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/mm: kill color_search_free/get_block

[Chris Wilson]

On Mon, Jul 01, 2013 at 10:05:53PM +0200, Daniel Vetter wrote:
 drm/i915 is the only user of the color allocation handling and
 switched to insert_node a while ago. So we can ditch this.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch

If you build it, they will come. Lies!
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: use drm_mm_remove_node instead of put_block

[David Herrmann]

Hi

On Mon, Jul 1, 2013 at 10:05 PM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 When converting to the preallocated drm_mm_node interfaces in

 commit dc9dd7a20fde95aa81a8307cde79c2dff9f83f3d
 Author: Chris Wilson ch...@chris-wilson.co.uk
 Date:   Fri Dec 7 20:37:07 2012 +

 drm/i915: Preallocate the drm_mm_node prior to manipulating the GTT drm_mm

 only the allocation side was converted, but not the freeing. Fix this
 up.

 Note that the only difference between put_block and remove_node is
 that the former fills up the preallocation cache. Which we don't need
 anyway and hence is just wasted space.

 Cc: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Ben Widawsky b...@bwidawsk.net
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_gem.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index 4200c32..30fd694 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -2620,7 +2620,7 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj)
 /* Avoid an unnecessary call to unbind on rebind. */
 obj-map_and_fenceable = true;

 -   drm_mm_put_block(obj-gtt_space);
 +   drm_mm_remove_node(obj-gtt_space);

kfree(obj-gtt_space);

 obj-gtt_space = NULL;
 obj-gtt_offset = 0;

 @@ -3137,14 +3137,14 @@ search_free:
 }
 if (WARN_ON(!i915_gem_valid_gtt_space(dev, node, obj-cache_level))) {
 i915_gem_object_unpin_pages(obj);
 -   drm_mm_put_block(node);
 +   drm_mm_remove_node(node);

kfree(node);

 return -EINVAL;
 }

 ret = i915_gem_gtt_prepare_object(obj);
 if (ret) {
 i915_gem_object_unpin_pages(obj);
 -   drm_mm_put_block(node);
 +   drm_mm_remove_node(node);

kfree(node);

drm_mm_remove_node() does unlink the node but not remove it. Btw., I
have these fixes in my series, too. I will push it later and write the
git-link to #dri-devel.

Cheers
David

 return ret;
 }

 --
 1.7.11.7

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/6] drm: add unified vma offset manager

[Daniel Vetter]

On Mon, Jul 01, 2013 at 09:54:17PM +0200, David Herrmann wrote:
 On Mon, Jul 1, 2013 at 9:42 PM, Daniel Vetter dan...@ffwll.ch wrote:
  On Mon, Jul 01, 2013 at 08:33:00PM +0200, David Herrmann wrote:
  +/** drm_vma_offset_manager_destroy()
  + *
  + * Destroy an object manager which was previously created via
  + * drm_vma_offset_manager_init(). The caller must remove all allocated 
  nodes
  + * before destroying the manager. Otherwise, drm_mm will refuse to free 
  the
  + * requested resources.
  + *
  + * The manager must not be accessed after this function is called.
  + */
  +void drm_vma_offset_manager_destroy(struct drm_vma_offset_manager *mgr)
  +{
  + BUG_ON(!drm_mm_clean(mgr-vm_addr_space_mm));
 
  Please convert this to a WARN_ON - drm_mm has code to cobble over buggy
  drivers, and killing the driver with a BUG_ON if we could keep on going
  with just a WARN_ON is a real pain for development.
 
 Ok.

Actually I think the check in the takedown function should be good enough
already. I've just dumped a patch onto dri-devel which will convert that
to a WARN, so I think adding a second one here is redundant.

 
  +
  + /* take the lock to protect against buggy drivers */
  + write_lock(mgr-vm_lock);
  + drm_mm_takedown(mgr-vm_addr_space_mm);
  + write_unlock(mgr-vm_lock);
  +}
  +EXPORT_SYMBOL(drm_vma_offset_manager_destroy);
  +
  +/** drm_vma_offset_lookup()
  + *
  + * Find a node given a start address and object size. This returns the 
  _best_
  + * match for the given node. That is, @start may point somewhere into a 
  valid
  + * region and the given node will be returned, as long as the node spans 
  the
  + * whole requested area (given the size in number of pages as @pages).
  + *
  + * Returns NULL if no suitable node can be found. Otherwise, the best 
  match
  + * is returned. It's the caller's responsibility to make sure the node 
  doesn't
  + * get destroyed before the caller can access it.
  + */
  +struct drm_vma_offset_node *drm_vma_offset_lookup(struct 
  drm_vma_offset_manager *mgr,
  +   unsigned long start,
  +   unsigned long pages)
  +{
  + struct drm_vma_offset_node *node, *best;
  + struct rb_node *iter;
  + unsigned long offset;
  +
  + read_lock(mgr-vm_lock);
  +
  + iter = mgr-vm_addr_space_rb.rb_node;
  + best = NULL;
  +
  + while (likely(iter)) {
  + node = rb_entry(iter, struct drm_vma_offset_node, vm_rb);
  + offset = node-vm_node.start;
  + if (start = offset) {
  + iter = iter-rb_right;
  + best = node;
  + if (start == offset)
  + break;
  + } else {
  + iter = iter-rb_left;
  + }
  + }
  +
  + /* verify that the node spans the requested area */
  + if (likely(best)) {
  + offset = best-vm_node.start + best-vm_pages;
  + if (offset  start + pages)
  + best = NULL;
  + }
  +
  + read_unlock(mgr-vm_lock);
  +
  + return best;
  +}
  +EXPORT_SYMBOL(drm_vma_offset_lookup);
  +
  +/* internal helper to link @node into the rb-tree */
  +static void _drm_vma_offset_add_rb(struct drm_vma_offset_manager *mgr,
  +struct drm_vma_offset_node *node)
  +{
  + struct rb_node **iter = mgr-vm_addr_space_rb.rb_node;
  + struct rb_node *parent = NULL;
  + struct drm_vma_offset_node *iter_node;
  +
  + while (likely(*iter)) {
  + parent = *iter;
  + iter_node = rb_entry(*iter, struct drm_vma_offset_node, 
  vm_rb);
  +
  + if (node-vm_node.start  iter_node-vm_node.start)
  + iter = (*iter)-rb_left;
  + else if (node-vm_node.start  iter_node-vm_node.start)
  + iter = (*iter)-rb_right;
  + else
  + BUG();
  + }
  +
  + rb_link_node(node-vm_rb, parent, iter);
  + rb_insert_color(node-vm_rb, mgr-vm_addr_space_rb);
  +}
  +
  +/** drm_vma_offset_add()
  + *
  + * Add a node to the offset-manager. If the node was already added, this 
  does
  + * nothing and return 0. @pages is the size of the object given in number 
  of
  + * pages.
  + * After this call succeeds, you can access the offset of the node until 
  it
  + * is removed again.
  + *
  + * If this call fails, it is safe to retry the operation or call
  + * drm_vma_offset_remove(), anyway. However, no cleanup is required in 
  that
  + * case.
  + */
  +int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
  +struct drm_vma_offset_node *node, unsigned long pages)
  +{
  + int ret;
  +
  + write_lock(mgr-vm_lock);
  +
  + if (unlikely(drm_mm_node_linked(node-vm_node))) {
  + ret = 0;
  + goto out_unlock;
  + }
  +
  +

Re: [PATCH 2/2] drm/i915: use drm_mm_remove_node instead of put_block

[Daniel Vetter]

On Mon, Jul 01, 2013 at 09:16:56PM +0100, Chris Wilson wrote:
 On Mon, Jul 01, 2013 at 10:05:54PM +0200, Daniel Vetter wrote:
  When converting to the preallocated drm_mm_node interfaces in
  
  commit dc9dd7a20fde95aa81a8307cde79c2dff9f83f3d
  Author: Chris Wilson ch...@chris-wilson.co.uk
  Date:   Fri Dec 7 20:37:07 2012 +
  
  drm/i915: Preallocate the drm_mm_node prior to manipulating the GTT 
  drm_mm
  
  only the allocation side was converted, but not the freeing. Fix this
  up.
  
  Note that the only difference between put_block and remove_node is
  that the former fills up the preallocation cache. Which we don't need
  anyway and hence is just wasted space.
  
  Cc: Chris Wilson ch...@chris-wilson.co.uk
  Cc: Ben Widawsky b...@bwidawsk.net
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 
 You learn something new every day.
 Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk

The lack of kfree in this patch might be a problem ...

/me tries again
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/i915: Don't try to tear down the stolen drm_mm if it's not there

[Daniel Vetter]

Every other place properly checks whether we've managed to set
up the stolen allocator at boot-up properly, with the exception
of the cleanup code. Which results in an ugly

*ERROR* Memory manager not clean. Delaying takedown

at module unload time since the drm_mm isn't initialized at all.

v2: While at it check whether the stolen drm_mm is initialized instead
of the more obscure stolen_base == 0 check.

v3: Fix up the logic. Also we need to keep the stolen_base check in
i915_gem_object_create_stolen_for_preallocated since that can be
called before stolen memory is fully set up. Spotted by Chris Wilson.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65953
Cc: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 8e02344..fbfc2c7 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -147,7 +147,7 @@ int i915_gem_stolen_setup_compression(struct drm_device 
*dev, int size)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
 
-   if (dev_priv-mm.stolen_base == 0)
+   if (!drm_mm_initialized(dev_priv-mm.stolen))
return -ENODEV;
 
if (size  dev_priv-fbc.size)
@@ -179,6 +179,9 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
 
+   if (!drm_mm_initialized(dev_priv-mm.stolen))
+   return;
+
i915_gem_stolen_cleanup_compression(dev);
drm_mm_takedown(dev_priv-mm.stolen);
 }
@@ -300,7 +303,7 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 
size)
struct drm_i915_gem_object *obj;
struct drm_mm_node *stolen;
 
-   if (dev_priv-mm.stolen_base == 0)
+   if (!drm_mm_initialized(dev_priv-mm.stolen))
return NULL;
 
DRM_DEBUG_KMS(creating stolen object: size=%x\n, size);
-- 
1.8.3.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: use drm_mm_remove_node instead of put_block

[Daniel Vetter]

On Mon, Jul 01, 2013 at 10:21:57PM +0200, David Herrmann wrote:
 Hi
 
 On Mon, Jul 1, 2013 at 10:05 PM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
  When converting to the preallocated drm_mm_node interfaces in
 
  commit dc9dd7a20fde95aa81a8307cde79c2dff9f83f3d
  Author: Chris Wilson ch...@chris-wilson.co.uk
  Date:   Fri Dec 7 20:37:07 2012 +
 
  drm/i915: Preallocate the drm_mm_node prior to manipulating the GTT 
  drm_mm
 
  only the allocation side was converted, but not the freeing. Fix this
  up.
 
  Note that the only difference between put_block and remove_node is
  that the former fills up the preallocation cache. Which we don't need
  anyway and hence is just wasted space.
 
  Cc: Chris Wilson ch...@chris-wilson.co.uk
  Cc: Ben Widawsky b...@bwidawsk.net
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
  ---
   drivers/gpu/drm/i915/i915_gem.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_gem.c 
  b/drivers/gpu/drm/i915/i915_gem.c
  index 4200c32..30fd694 100644
  --- a/drivers/gpu/drm/i915/i915_gem.c
  +++ b/drivers/gpu/drm/i915/i915_gem.c
  @@ -2620,7 +2620,7 @@ i915_gem_object_unbind(struct drm_i915_gem_object 
  *obj)
  /* Avoid an unnecessary call to unbind on rebind. */
  obj-map_and_fenceable = true;
 
  -   drm_mm_put_block(obj-gtt_space);
  +   drm_mm_remove_node(obj-gtt_space);
 
 kfree(obj-gtt_space);
 
  obj-gtt_space = NULL;
  obj-gtt_offset = 0;
 
  @@ -3137,14 +3137,14 @@ search_free:
  }
  if (WARN_ON(!i915_gem_valid_gtt_space(dev, node, 
  obj-cache_level))) {
  i915_gem_object_unpin_pages(obj);
  -   drm_mm_put_block(node);
  +   drm_mm_remove_node(node);
 
 kfree(node);
 
  return -EINVAL;
  }
 
  ret = i915_gem_gtt_prepare_object(obj);
  if (ret) {
  i915_gem_object_unpin_pages(obj);
  -   drm_mm_put_block(node);
  +   drm_mm_remove_node(node);
 
 kfree(node);

Yeah, I fail ...

 drm_mm_remove_node() does unlink the node but not remove it. Btw., I
 have these fixes in my series, too. I will push it later and write the
 git-link to #dri-devel.

We have patches in-flight to convert over to embedded drm_mm_nodes anyway,
so I guess that part will solve itself automatically.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: use drm_mm_remove_node instead of put_block

[Ben Widawsky]

On Mon, Jul 01, 2013 at 10:39:03PM +0200, Daniel Vetter wrote:
 On Mon, Jul 01, 2013 at 10:21:57PM +0200, David Herrmann wrote:
  Hi
  
  On Mon, Jul 1, 2013 at 10:05 PM, Daniel Vetter daniel.vet...@ffwll.ch 
  wrote:
   When converting to the preallocated drm_mm_node interfaces in
  
   commit dc9dd7a20fde95aa81a8307cde79c2dff9f83f3d
   Author: Chris Wilson ch...@chris-wilson.co.uk
   Date:   Fri Dec 7 20:37:07 2012 +
  
   drm/i915: Preallocate the drm_mm_node prior to manipulating the GTT 
   drm_mm
  
   only the allocation side was converted, but not the freeing. Fix this
   up.
  
   Note that the only difference between put_block and remove_node is
   that the former fills up the preallocation cache. Which we don't need
   anyway and hence is just wasted space.
  
   Cc: Chris Wilson ch...@chris-wilson.co.uk
   Cc: Ben Widawsky b...@bwidawsk.net
   Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
   ---
drivers/gpu/drm/i915/i915_gem.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
  
   diff --git a/drivers/gpu/drm/i915/i915_gem.c 
   b/drivers/gpu/drm/i915/i915_gem.c
   index 4200c32..30fd694 100644
   --- a/drivers/gpu/drm/i915/i915_gem.c
   +++ b/drivers/gpu/drm/i915/i915_gem.c
   @@ -2620,7 +2620,7 @@ i915_gem_object_unbind(struct drm_i915_gem_object 
   *obj)
   /* Avoid an unnecessary call to unbind on rebind. */
   obj-map_and_fenceable = true;
  
   -   drm_mm_put_block(obj-gtt_space);
   +   drm_mm_remove_node(obj-gtt_space);
  
  kfree(obj-gtt_space);
  
   obj-gtt_space = NULL;
   obj-gtt_offset = 0;
  
   @@ -3137,14 +3137,14 @@ search_free:
   }
   if (WARN_ON(!i915_gem_valid_gtt_space(dev, node, 
   obj-cache_level))) {
   i915_gem_object_unpin_pages(obj);
   -   drm_mm_put_block(node);
   +   drm_mm_remove_node(node);
  
  kfree(node);
  
   return -EINVAL;
   }
  
   ret = i915_gem_gtt_prepare_object(obj);
   if (ret) {
   i915_gem_object_unpin_pages(obj);
   -   drm_mm_put_block(node);
   +   drm_mm_remove_node(node);
  
  kfree(node);
 
 Yeah, I fail ...
 
  drm_mm_remove_node() does unlink the node but not remove it. Btw., I
  have these fixes in my series, too. I will push it later and write the
  git-link to #dri-devel.
 
 We have patches in-flight to convert over to embedded drm_mm_nodes anyway,
 so I guess that part will solve itself automatically.
 -Daniel

I'm planning to get this out ASAP. I'm a bit confused now though what I
actually need to send.

-- 
Ben Widawsky, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915: use drm_mm_remove_node instead of put_block

[Daniel Vetter]

On Mon, Jul 1, 2013 at 10:46 PM, Ben Widawsky b...@bwidawsk.net wrote:
  drm_mm_remove_node() does unlink the node but not remove it. Btw., I
  have these fixes in my series, too. I will push it later and write the
  git-link to #dri-devel.

 We have patches in-flight to convert over to embedded drm_mm_nodes anyway,
 so I guess that part will solve itself automatically.

 I'm planning to get this out ASAP. I'm a bit confused now though what I
 actually need to send.

I think for now just the top-down allocator + the create_node stuff
for stolen memory. I guess the conversion to embed the gtt_space
drm_mm_node will take a bit longer.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[pull] radeon drm-next-3.11

[alexdeucher]

From: Alex Deucher alexander.deuc...@amd.com

Hi Dave,

A few more patches for 3.11:
- add debugfs interface to check current DPM state
- Fix a bug that caused problems with DPM on BTC+ asics.

The following changes since commit f7d452f4fd5d86f764807a1234a407deb5b105ef:

  Merge branch 'drm-nouveau-next' of 
git://anongit.freedesktop.org/git/nouveau/linux-2.6 into drm-next (2013-07-01 
14:10:20 +1000)

are available in the git repository at:

  git://people.freedesktop.org/~agd5f/linux drm-next-3.11

Alex Deucher (12):
  drm/radeon: remove sumo dpm/uvd bringup leftovers
  drm/radeon/atom: fix endian bug in radeon_atom_init_mc_reg_table()
  drm/radeon: fix typo in radeon_atom_init_mc_reg_table()
  drm/radeon/dpm: re-enable state transitions for BTC
  drm/radeon/dpm: re-enable state transitions for Cayman
  drm/radeon/dpm: add infrastructure to support debugfs info
  drm/radeon/dpm: add debugfs support for rv6xx
  drm/radeon/dpm: add debugfs support for 7xx/evergreen/btc
  drm/radeon/dpm: add debugfs support for ON/LN
  drm/radeon/dpm: add debugfs support for TN
  drm/radeon/dpm: add debugfs support for cayman
  drm/radeon/dpm: add debugfs support for SI

 drivers/gpu/drm/radeon/btc_dpm.c |3 --
 drivers/gpu/drm/radeon/ni_dpm.c  |   25 +---
 drivers/gpu/drm/radeon/nid.h |4 ++
 drivers/gpu/drm/radeon/radeon.h  |2 +
 drivers/gpu/drm/radeon/radeon_asic.c |8 +
 drivers/gpu/drm/radeon/radeon_asic.h |   12 
 drivers/gpu/drm/radeon/radeon_atombios.c |3 +-
 drivers/gpu/drm/radeon/radeon_pm.c   |   40 ++
 drivers/gpu/drm/radeon/rv6xx_dpm.c   |   25 
 drivers/gpu/drm/radeon/rv770_dpm.c   |   30 
 drivers/gpu/drm/radeon/rv770d.h  |4 ++
 drivers/gpu/drm/radeon/si_dpm.c  |   19 
 drivers/gpu/drm/radeon/sid.h |4 ++
 drivers/gpu/drm/radeon/sumo_dpm.c|   45 ++---
 drivers/gpu/drm/radeon/trinity_dpm.c |   21 ++
 15 files changed, 206 insertions(+), 39 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC 1/3] DRM: Armada: Add Armada DRM driver

[Dave Airlie]

 Of course, I share the idea of true, full-blown of_drm_something
 helpers. But the DT patch, is not an improvement but a real fix as in
 make DRM not break on some platforms. From that on, I can start
 digging into DRM API and improve DT support here and there.

 So for the three patches I mentioned above, they are all minor
 improvements of the API. Minor, because I cannot ever sent _the_ single
 perfect patch just because I don't know the API well enough, yet.

 Just based on my personal experience: TDA998x driver got merged the way it
 is _although_ I addressed some concerns - the fixes are not merged
 _although_ I provided experimental results. This is of course
 disappointing for me, but I am sure it will work out soon and I will
 get back to happily send improvements for the platforms I can test on.

To be honest I've no idea what those tda998x patches could fix or
break, so they go on my no ideas list and I hope if they get reposted
someone will tell me what they do.

I could probably be more motivated towards poking other people to
review stuff, but I mostly hope in vain that the ARM people will cross
review things for other ARM chips, I had a bit of that happening for a
while at the start, but it seems to have died off. Now I'm mostly
relying on Rob to have some clue what I'm merging is sane.

My current priority for merging stuff is:
core patches that affect all platforms,
core patches that affect x86
drivers that I maintain by default
core patches that affect ARM
misc ARM drivers that fall through cracks.

Maybe I can persuade Rob to become a sub maintainer for all of the SoC
drivers but I suspect he'd try and hurt me in real life.

I'll take another look at the tda patches but I may still have no idea
what they are doing.

Dave.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/nouveau: fix locking in nouveau_crtc_page_flip

[Dave Airlie]

On Mon, Jul 1, 2013 at 8:00 PM, Maarten Lankhorst
maarten.lankho...@canonical.com wrote:
 This is a bit messed up because chan-cli-mutex is a different class,
 depending on whether it is the global drm client or not. This is
 because the global cli-mutex lock can be taken for eviction,
 so locking it before pinning the buffer objects may result in a deadlock.

conditional mutex locking made me free a bit ill,

Ben any cleaner way to do this? if not I'll merge this.

Dave.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 66425] failed testing IB on ring 5 when suspending to disk

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=66425

--- Comment #2 from Austin Lund austin.l...@gmail.com ---
(In reply to comment #1)
 This is a mac?

Yes.  Macbookpro8,2

Also, this doesn't happen with a 3.9.7 kernel.

It seems to be related to the UVD stuff that was added to 3.10.  Ring 5 appears
to be related to this and it doesn't appear in the 3.9 kernels.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC 1/3] DRM: Armada: Add Armada DRM driver

[Rob Clark]

On Mon, Jul 1, 2013 at 4:52 AM, Sebastian Hesselbarth
sebastian.hesselba...@gmail.com wrote:
 On 07/01/13 02:01, Dave Airlie wrote:

 how about instead of writing:
 However, at least I've taken the time to_think_  about what I'm doing
 and realise that there_is_  scope here for the DRM core to improve,

 rather than burying this stuff deep inside my driver like everyone else
 has.  That's no reason to penalise patches from the good guys who think

 you go with
 I noticed this piece of functionality could be refactored, here is a
 patch adding them to
 the core, does anyone think its a good idea?


 Dave,

 at least on this point I do share Russell's impression. I've sent
 bunch of patches improving TDA998x and DRM+DT:
 - TDA998x irq handling - ignored
 - TDA998x sync fix - ignored

At least the sync fix, looks like I missed it (it probably is a good
idea to CC me if you want me to look at it).  Looks like there was
some follow-up discussion on both patches, unless I missed seeing a
newer version of those patches.

Sometimes if you think a patch has been ignored/forgotten, it doesn't
hurt to ping on mailing list or #dri-devel..  a lot of us are working
not just on kernel (the relatively small part in the whole linux
graphics stack), but also mesa and/or x11.  Some times things end up
several pages down in the mail folder.  It's not because we are all
sitting on a beach drinking margaritas, or because we don't like you.
It is just because we are busy and missed it.

Last few months I've been pretty buried in r/e + gallium driver for
new gpu, so I wasn't always checking dri-devel list every day.  At
least now I am in drm-driver mode again ;-)

 - Fix drm I2C slave encoder probing

 I am aware that this is not an easy job nor one you get much
 appreciation for. But, back when TDA998x driver was published,
 all my comments were basically answered with Oh, I know. Maybe
 someday somebody will fix it.

If you have a better idea about how to make the slave encoder probing
better (and/or more generic to support stuff other than i2c), please
send RFC patch.  (And if you already did this, please send updated
version, see previous point about sometimes missing patches.)


BR,
-R


 I am not being paid for any of this, but have a strong intrinsic
 motivation here. But I am loosing interest in sending fixes for
 DRM stuff because my (personal) impression is the same Russell
 has: Depending on who sends patches, they get merged independent
 of how broken they are - others are discussed to death.

 Sebastian


 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 66473] New: hdmi oops with mem sleep

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=66473

  Priority: medium
Bug ID: 66473
  Assignee: dri-devel@lists.freedesktop.org
   Summary: hdmi oops with mem sleep
  Severity: normal
Classification: Unclassified
OS: Linux (All)
  Reporter: adf.li...@gmail.com
  Hardware: x86-64 (AMD64)
Status: NEW
   Version: XOrg CVS
 Component: DRM/Radeon
   Product: DRI

Created attachment 81834
  -- https://bugs.freedesktop.org/attachment.cgi?id=81834action=edit
oops hdmi

RV790  running drm-next as it was on 260613

Have used echo mem  /sys/power/state for some time now, though I don't think
this is a regression - just a situation that I haven't hit before.

Haven't tested every senario but it seems that if I boot and use single DVI
monitor then someone else turns on TV which is connected DVI - hdmi then if I
use mem sleep I get an oops. If the TV is on already when I boot it doesn't
happen. I am not enabling the TV with xrandr as it's not me that is using it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [pull] radeon drm-next-3.11

[Markus Trippelsdorf]

On 2013.07.01 at 17:01 -0400, alexdeuc...@gmail.com wrote:
 From: Alex Deucher alexander.deuc...@amd.com
 
 Hi Dave,
 
 A few more patches for 3.11:
 - add debugfs interface to check current DPM state
 - Fix a bug that caused problems with DPM on BTC+ asics.
 
 The following changes since commit f7d452f4fd5d86f764807a1234a407deb5b105ef:
 
   Merge branch 'drm-nouveau-next' of 
 git://anongit.freedesktop.org/git/nouveau/linux-2.6 into drm-next (2013-07-01 
 14:10:20 +1000)
 
 are available in the git repository at:
 
   git://people.freedesktop.org/~agd5f/linux drm-next-3.11
 
 Alex Deucher (12):
   drm/radeon: remove sumo dpm/uvd bringup leftovers
   drm/radeon/atom: fix endian bug in radeon_atom_init_mc_reg_table()
   drm/radeon: fix typo in radeon_atom_init_mc_reg_table()
   drm/radeon/dpm: re-enable state transitions for BTC
   drm/radeon/dpm: re-enable state transitions for Cayman
   drm/radeon/dpm: add infrastructure to support debugfs info
   drm/radeon/dpm: add debugfs support for rv6xx
   drm/radeon/dpm: add debugfs support for 7xx/evergreen/btc

Looks like you forgot to add debugfs support for rs780:

diff --git a/drivers/gpu/drm/radeon/radeon_asic.c 
b/drivers/gpu/drm/radeon/radeon_asic.c
index a5b244d..ca4f928 100644
--- a/drivers/gpu/drm/radeon/radeon_asic.c
+++ b/drivers/gpu/drm/radeon/radeon_asic.c
@@ -1270,6 +1270,7 @@ static struct radeon_asic rs780_asic = {
.get_sclk = rs780_dpm_get_sclk,
.get_mclk = rs780_dpm_get_mclk,
.print_power_state = rs780_dpm_print_power_state,
+   .debugfs_print_current_performance_level = 
rv770_dpm_debugfs_print_current_performance_level,
},
.pflip = {
.pre_page_flip = rs600_pre_page_flip,

-- 
Markus
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [pull] radeon drm-next-3.11

[Deucher, Alexander]

 -Original Message-
 From: Markus Trippelsdorf [mailto:mar...@trippelsdorf.de]
 Sent: Monday, July 01, 2013 5:58 PM
 To: alexdeuc...@gmail.com
 Cc: dri-devel@lists.freedesktop.org; airl...@gmail.com; Deucher, Alexander
 Subject: Re: [pull] radeon drm-next-3.11
 
 On 2013.07.01 at 17:01 -0400, alexdeuc...@gmail.com wrote:
  From: Alex Deucher alexander.deuc...@amd.com
 
  Hi Dave,
 
  A few more patches for 3.11:
  - add debugfs interface to check current DPM state
  - Fix a bug that caused problems with DPM on BTC+ asics.
 
  The following changes since commit
 f7d452f4fd5d86f764807a1234a407deb5b105ef:
 
Merge branch 'drm-nouveau-next' of
 git://anongit.freedesktop.org/git/nouveau/linux-2.6 into drm-next (2013-07-
 01 14:10:20 +1000)
 
  are available in the git repository at:
 
git://people.freedesktop.org/~agd5f/linux drm-next-3.11
 
  Alex Deucher (12):
drm/radeon: remove sumo dpm/uvd bringup leftovers
drm/radeon/atom: fix endian bug in radeon_atom_init_mc_reg_table()
drm/radeon: fix typo in radeon_atom_init_mc_reg_table()
drm/radeon/dpm: re-enable state transitions for BTC
drm/radeon/dpm: re-enable state transitions for Cayman
drm/radeon/dpm: add infrastructure to support debugfs info
drm/radeon/dpm: add debugfs support for rv6xx
drm/radeon/dpm: add debugfs support for 7xx/evergreen/btc
 
 Looks like you forgot to add debugfs support for rs780:
 
 diff --git a/drivers/gpu/drm/radeon/radeon_asic.c
 b/drivers/gpu/drm/radeon/radeon_asic.c
 index a5b244d..ca4f928 100644
 --- a/drivers/gpu/drm/radeon/radeon_asic.c
 +++ b/drivers/gpu/drm/radeon/radeon_asic.c
 @@ -1270,6 +1270,7 @@ static struct radeon_asic rs780_asic = {
   .get_sclk = rs780_dpm_get_sclk,
   .get_mclk = rs780_dpm_get_mclk,
   .print_power_state = rs780_dpm_print_power_state,
 + .debugfs_print_current_performance_level =
 rv770_dpm_debugfs_print_current_performance_level,
   },
   .pflip = {
   .pre_page_flip = rs600_pre_page_flip,
 

Rs780 doesn't have a convenient way to look up the current power state so it's 
not implemented yet.  The method used on the other asics does not work for 
rs780.

Alex


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC 1/3] DRM: Armada: Add Armada DRM driver

[Rob Clark]

On Mon, Jul 1, 2013 at 5:26 PM, Dave Airlie airl...@gmail.com wrote:
 Of course, I share the idea of true, full-blown of_drm_something
 helpers. But the DT patch, is not an improvement but a real fix as in
 make DRM not break on some platforms. From that on, I can start
 digging into DRM API and improve DT support here and there.

 So for the three patches I mentioned above, they are all minor
 improvements of the API. Minor, because I cannot ever sent _the_ single
 perfect patch just because I don't know the API well enough, yet.

 Just based on my personal experience: TDA998x driver got merged the way it
 is _although_ I addressed some concerns - the fixes are not merged
 _although_ I provided experimental results. This is of course
 disappointing for me, but I am sure it will work out soon and I will
 get back to happily send improvements for the platforms I can test on.

 To be honest I've no idea what those tda998x patches could fix or
 break, so they go on my no ideas list and I hope if they get reposted
 someone will tell me what they do.

IIRC, a few of those patches were breaking things on tilcdc, which
someone needs to get to the bottom of..  and for that I'm relying a
bit on Darren as he is the one with both hw to test with tilcdc plus
an hdmi tester.  And I think understands the issue better than I do at
this point.

 I could probably be more motivated towards poking other people to
 review stuff, but I mostly hope in vain that the ARM people will cross
 review things for other ARM chips, I had a bit of that happening for a
 while at the start, but it seems to have died off. Now I'm mostly
 relying on Rob to have some clue what I'm merging is sane.

 My current priority for merging stuff is:
 core patches that affect all platforms,
 core patches that affect x86
 drivers that I maintain by default
 core patches that affect ARM
 misc ARM drivers that fall through cracks.

 Maybe I can persuade Rob to become a sub maintainer for all of the SoC
 drivers but I suspect he'd try and hurt me in real life.

/me runs :-P

well, anyways, I do at least try to review arm patches.  Things
sometimes do fall on the floor when I'm busy in other areas.

BR,
-R

 I'll take another look at the tda patches but I may still have no idea
 what they are doing.

 Dave.
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm: move edid null check to the first part of drm_edid_block_valid

[Seung-Woo Kim]

Hi Daniel,

On 2013년 07월 01일 23:56, Daniel Vetter wrote:
 On Mon, Jul 1, 2013 at 12:21 PM, Chris Wilson ch...@chris-wilson.co.uk 
 wrote:
 On Mon, Jul 01, 2013 at 07:06:32PM +0900, Seung-Woo Kim wrote:
 If raw_edid is null, it will crash, so checking in bad label is
 meaningless.

 It would be an error on part of the caller, but the defense looks sane.
 As the function is a bool, I would have preferred it returned
 true/false, but your patch is correct wrt to the rest of the function.
 
 If we consider passing a NULL raw_edid here a caller-error, shouldn't
 this be a WARN on top? And I concur on the s/0/false/ bikeshed, return
 0 could be misleading since for errno returning functions that reads
 as success.

Yes, you are right. WARN_ON() is better because there was no crash until
now. and I will also update all return values as false/true instead of 0/1.

 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
 ___
 dri-devel mailing list
 dri-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel
 

-- 
Seung-Woo Kim
Samsung Software RD Center
--

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 3/3] drm: fix error routines in drm_open_helper

[Seung-Woo Kim]

From: YoungJun Cho yj44@samsung.com

There are missing parts to handle error in drm_open_helper().
The priv-minor, assigned by idr_find() which can return NULL,
should be checked whether it is NULL or not before referencing it.
put_pid(), drm_gem_release(), and drm_prime_destory_file_private()
should be called when error happens after their pair functions are
called. If an error occurs after executing dev-driver-open()
which allocates driver specific per-file private data, then the
private data should be released.

Signed-off-by: YoungJun Cho yj44@samsung.com
Signed-off-by: Seung-Woo Kim sw0312@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
change from v2
- adds put_pid, drm_gem_release, drm_prime_destroy_file_private as Chris's 
review
change from v1
- replaces error value for failure to find the minor as ENODEV as Chris 
commented

 drivers/gpu/drm/drm_fops.c |   25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 429e07d..33b1125 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -271,6 +271,11 @@ static int drm_open_helper(struct inode *inode, struct 
file *filp,
priv-uid = current_euid();
priv-pid = get_pid(task_pid(current));
priv-minor = idr_find(drm_minors_idr, minor_id);
+   if (!priv-minor) {
+   ret = -ENODEV;
+   goto out_put_pid;
+   }
+
priv-ioctl_count = 0;
/* for compatibility root is always authenticated */
priv-authenticated = capable(CAP_SYS_ADMIN);
@@ -292,7 +297,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
if (dev-driver-open) {
ret = dev-driver-open(dev, priv);
if (ret  0)
-   goto out_free;
+   goto out_prime_destroy;
}
 
 
@@ -304,7 +309,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
if (!priv-minor-master) {
mutex_unlock(dev-struct_mutex);
ret = -ENOMEM;
-   goto out_free;
+   goto out_close;
}
 
priv-is_master = 1;
@@ -322,7 +327,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
drm_master_put(priv-minor-master);
drm_master_put(priv-master);
mutex_unlock(dev-struct_mutex);
-   goto out_free;
+   goto out_close;
}
}
mutex_lock(dev-struct_mutex);
@@ -333,7 +338,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
drm_master_put(priv-minor-master);
drm_master_put(priv-master);
mutex_unlock(dev-struct_mutex);
-   goto out_free;
+   goto out_close;
}
}
mutex_unlock(dev-struct_mutex);
@@ -367,7 +372,17 @@ static int drm_open_helper(struct inode *inode, struct 
file *filp,
 #endif
 
return 0;
-  out_free:
+
+out_close:
+   if (dev-driver-postclose)
+   dev-driver-postclose(dev, priv);
+out_prime_destroy:
+   if (drm_core_check_feature(dev, DRIVER_PRIME))
+   drm_prime_destroy_file_private(priv-prime);
+   if (dev-driver-driver_feature  DRIVER_GEM)
+   drm_gem_release(dev, priv);
+out_put_pid:
+   put_pid(priv-pid);
kfree(priv);
filp-private_data = NULL;
return ret;
-- 
1.7.9.5

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/3] drm: add assertion for checking null edid to drm_edid_block_valid

[Seung-Woo Kim]

If raw_edid of drm_edid_block_vaild() is null, it will crash, so
checking in bad label is removed and instead assertion is added at
the top of the function.
The type of return for the function is bool, so it fixes to return
true and false instead of 1 and 0.

Signed-off-by: Seung-Woo Kim sw0312@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
chages from v1
- NULL checking is replaced with WARN_ON() as Daniel commented
- all return value is replaced as true/false as Chris and Daniel commented

 drivers/gpu/drm/drm_edid.c |8 +---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 2dc1a60..1117f02 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -968,6 +968,8 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool 
print_bad_edid)
u8 csum = 0;
struct edid *edid = (struct edid *)raw_edid;
 
+   WARN_ON(!raw_edid);
+
if (edid_fixup  8 || edid_fixup  0)
edid_fixup = 6;
 
@@ -1010,15 +1012,15 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool 
print_bad_edid)
break;
}
 
-   return 1;
+   return true;
 
 bad:
-   if (raw_edid  print_bad_edid) {
+   if (print_bad_edid) {
printk(KERN_ERR Raw EDID:\n);
print_hex_dump(KERN_ERR,  \t, DUMP_PREFIX_NONE, 16, 1,
   raw_edid, EDID_LENGTH, false);
}
-   return 0;
+   return false;
 }
 EXPORT_SYMBOL(drm_edid_block_valid);
 
-- 
1.7.4.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v4 3/3] drm: fix error routines in drm_open_helper

[Seung-Woo Kim]

From: YoungJun Cho yj44@samsung.com

There are missing parts to handle error in drm_open_helper().
The priv-minor, assigned by idr_find() which can return NULL,
should be checked whether it is NULL or not before referencing it.
put_pid(), drm_gem_release(), and drm_prime_destory_file_private()
should be called when error happens after their pair functions are
called. If an error occurs after executing dev-driver-open()
which allocates driver specific per-file private data, then the
private data should be released.

Signed-off-by: YoungJun Cho yj44@samsung.com
Signed-off-by: Seung-Woo Kim sw0312@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
change from v3
- fix typo
change from v2
- adds put_pid, drm_gem_release, drm_prime_destroy_file_private as Chris's 
review
change from v1
- replaces error value for failure to find the minor as ENODEV as Chris 
commented

 drivers/gpu/drm/drm_fops.c |   25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 429e07d..33b1125 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -271,6 +271,11 @@ static int drm_open_helper(struct inode *inode, struct 
file *filp,
priv-uid = current_euid();
priv-pid = get_pid(task_pid(current));
priv-minor = idr_find(drm_minors_idr, minor_id);
+   if (!priv-minor) {
+   ret = -ENODEV;
+   goto out_put_pid;
+   }
+
priv-ioctl_count = 0;
/* for compatibility root is always authenticated */
priv-authenticated = capable(CAP_SYS_ADMIN);
@@ -292,7 +297,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
if (dev-driver-open) {
ret = dev-driver-open(dev, priv);
if (ret  0)
-   goto out_free;
+   goto out_prime_destroy;
}
 
 
@@ -304,7 +309,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
if (!priv-minor-master) {
mutex_unlock(dev-struct_mutex);
ret = -ENOMEM;
-   goto out_free;
+   goto out_close;
}
 
priv-is_master = 1;
@@ -322,7 +327,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
drm_master_put(priv-minor-master);
drm_master_put(priv-master);
mutex_unlock(dev-struct_mutex);
-   goto out_free;
+   goto out_close;
}
}
mutex_lock(dev-struct_mutex);
@@ -333,7 +338,7 @@ static int drm_open_helper(struct inode *inode, struct file 
*filp,
drm_master_put(priv-minor-master);
drm_master_put(priv-master);
mutex_unlock(dev-struct_mutex);
-   goto out_free;
+   goto out_close;
}
}
mutex_unlock(dev-struct_mutex);
@@ -367,7 +372,17 @@ static int drm_open_helper(struct inode *inode, struct 
file *filp,
 #endif
 
return 0;
-  out_free:
+
+out_close:
+   if (dev-driver-postclose)
+   dev-driver-postclose(dev, priv);
+out_prime_destroy:
+   if (drm_core_check_feature(dev, DRIVER_PRIME))
+   drm_prime_destroy_file_private(priv-prime);
+   if (dev-driver-driver_features  DRIVER_GEM)
+   drm_gem_release(dev, priv);
+out_put_pid:
+   put_pid(priv-pid);
kfree(priv);
filp-private_data = NULL;
return ret;
-- 
1.7.9.5

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 3/3] drm/mm: WARN for unclean mm takedown

[Ben Widawsky]

On Mon,  1 Jul 2013 22:01:04 +0200
Daniel Vetter daniel.vet...@ffwll.ch wrote:

 The usual drm driver has tons of different drm_mm memory managers so
 the drm error message in dmesg is pretty useless. WARN instead so
 that we have the full backtrace.
 
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch

I've written this patch myself a number of times.
Reviewed-by: Ben Widawsky b...@bwidawsk.net

 ---
  drivers/gpu/drm/drm_mm.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
 index f9d4873..d303e31 100644
 --- a/drivers/gpu/drm/drm_mm.c
 +++ b/drivers/gpu/drm/drm_mm.c
 @@ -699,8 +699,8 @@ void drm_mm_takedown(struct drm_mm * mm)
  {
   struct drm_mm_node *entry, *next;
  
 - if (!list_empty(mm-head_node.node_list)) {
 - DRM_ERROR(Memory manager not clean. Delaying
 takedown\n);
 + if (WARN(!list_empty(mm-head_node.node_list),
 +  Memory manager not clean. Delaying takedown\n)) {
   return;
   }
  

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/6] drm: make drm_mm_init() return void

[Dave Airlie]

On Tue, Jul 2, 2013 at 5:22 AM, Daniel Vetter dan...@ffwll.ch wrote:
 On Mon, Jul 01, 2013 at 08:32:58PM +0200, David Herrmann wrote:
 There is no reason to return int as this function never fails.
 Furthermore, several drivers (ast, sis) already depend on this.

 Signed-off-by: David Herrmann dh.herrm...@gmail.com

 Back when I've reworked drm_mm I was still a rookie and didn't want to
 touch all drivers, hence why I've left the int return type. Good riddance
 to that!

 Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch

Thanks, I've stuck this in -next as it looks like a nice cleanup I'd like now.

Dave.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel