The conditional is usually a recoverable driver bug, and so WARNing, and
preventing the drm_mm code from doing potential damage (BUG) is
desirable.
This issue was hit and fixed twice while developing the i915 multiple
address space code. The first fix is the patch just before this, and is
hit on an not frequently occuring error path. Another was fixed during
patch iteration, so it's hard to see from the patch:
commit c6cfb32567
Author: Ben Widawsky <ben@bwidawsk.net>
Date: Fri Jul 5 14:41:06 2013 -0700
drm/i915: Embed drm_mm_node in i915 gem obj
From the intel-gfx mailing list, we discussed this:
References: <20130705191235.GA3057@bwidawsk.net>
Cc: Dave Airlie <airlied@redhat.com>
CC: <dri-devel@lists.freedesktop.org>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Acked-by: Dave Airlie <airlied@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Use () to make for neater alignment of the split lines, too. With this
we ditch another jump through the obj_gtt_size/offset indirection
maze.
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cleanup the map and fenceable setting during bind to make more sense,
and not check i915_is_ggtt() 2 unnecessary times
v2: Move the bools into the if block (Chris) - There are ways to tidy
this function (fence calculations for instance) even further, but they
are quite invasive, so I am punting on those unless specifically asked.
v3: Add newline between variable declaration and logic (Chris)
Recommended-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
VMAs can be created and not bound. One may think of it as lazy cleanup,
and safely gloss over the conditions which manufacture it. In either
case, when the object backing the i915 vma is destroyed, we must cleanup
the vma without stumbling into a bunch of pitfalls that assume the vma
is bound.
NOTE: I was pretty certain the above condition could only happen when we
introduced the use of VMAs being looked up at execbuf, and already
existing. Paulo has hit this though, so I must be missing something. As
I believe the patch is correct anyway, therefore I won't scratch my head
too hard.
v2: use goto destroy as a compromise (Chris)
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Use the standard inversely ordered goto label stack for everything.
Spotted while reviewing place where we might need to to call
vma_destroy but failed to do so.
Cc: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Ideally we could use for_each_ring with the ring flags as I've done a
couple times
(http://lists.freedesktop.org/archives/intel-gfx/2013-June/029450.html).
Until Daniel merges that patch though, we can just use this.
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We require n-1 mailboxes for proper semaphore synchronization. All
semaphore synchronization code relies on proper values in these
mailboxes. The fact that we failed to touch the vebox ring by itself
was unlikely to be an issue since the HW should be initializing the
values to 0. However the error framework for testing seqno wrap
introduced by Mika, in addition to the hangcheck via seqno, and
i915_error_first_batchbuffer() combined caused a nice explosion.
The problem is caused by seqno wrap because the wrap condition is not
properly setup. The wrap code attempts to set the sync mailboxes all
to 0, and then set the current seqno to one less than 0. In all cases,
the vebox mailbox wasn't properly being initialized. This caused a
wrap to not occur. When hangcheck kicks in with the bogus seqno
values, the rest just doesn't work. It makes me wonder if we shouldn't
consider a dumber version of hangcheck...
How we messed this up: VECS support was written before the
aforementioned other features. Upon VECS being rebased, these facts
were missed.
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65387
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67198
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
It's basically the same deal as the RC6+ issues on ivy bridge
except this time with RC6 on sandy bridge. Like last time the
core of the issue is that the timings don't work 100% with our
voltage regulator. So from time to time, the kernel will print
a warning message about the GPU not getting out of RC6. In
particular, I found this fairly easy to reproduce during
suspend/resume.
Changing the threshold to 125000 instead of 50000 seems to fix
the issue. The previous patch used 150000 but as it turns out
this doesn't work everywhere. After getting such a machine, I
bisected the highest value which works, which is 125000, so here
it is.
I also measured the idle power usage before/after this patch and
didn't see a difference on a sandy bridge laptop. On haswell and
up, it makes a big difference, so we want to keep it at 50k
there. It also seems like haswell doesn't have the RC6 issues
that sandy bridge has so the 50k value is fine.
Signed-off-by: Stéphane Marchesin <marcheu@chromium.org>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The machines that fall in this category are the SDVs that have a PCI
ID starting with 0x0C. These are very early pre-production machines
and may not fully work. Other Haswell SDVs have PCI IDs that match the
real Haswell machines and we expect them to work better.
Even though they have problems, they still mostly work so I don't see
a reason to refuse loading our driver. But I do see a reason to reject
bug reports from these machines, so the message should help the bug
triagers.
As far as I know, we don't implement some workarounds that are
specific to these machines and suspend/resume may not work on most of
them, but besides this, they may work.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61508
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
After computing the stage changes for the set_config, record those in
the debug log.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Caught by "make W=1 drivers/gpu/drm/i915/".
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This is primarily for the benefit of the create2 ioctl so that the
caller can avoid the later step of rebinding the bo with new PTE bits.
After introducing WT (and possibly GFDT) cacheing for display targets,
not everything in the display is earmarked as UC, and more importantly
what is is controlled by the kernel.
Note that set_cache_level/get_cache_level for DISPLAY is not necessarily
idempotent; get_cache_level may return UC for architectures that have no
special cache domain for the display engine.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Haswell GT3e has the unique feature of supporting Write-Through cacheing
of objects within the eLLC/LLC. The purpose of this is to enable the display
plane to remain coherent whilst objects lie resident in the eLLC/LLC - so
that we, in theory, get the best of both worlds, perfect display and fast
access.
However, we still need to be careful as the CPU does not see the WT when
accessing the cache. In particular, this means that we need to flush the
cache lines after writing to an object through the CPU, and on
transitioning from a cached state to WT.
v2: Actually do the clflush on transition to WT, nagging by Ville.
v3: Flush the CPU cache after writes into WT objects.
v4: Rease onto LLC updates and report WT as "uncached" for
get_cache_level_ioctl to remain symmetric with set_cache_level_ioctl.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Although I could not reproduce this (different compiler version,
perhaps), reportedly we get:
drivers/gpu/drm/i915/i915_irq.c:1943:27: warning: ‘score’ may be used
uninitialized in this function [-Wuninitialized]
Drop the 'score' variable altogether as it's not really needed.
Reported-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The short lowercase names are bound to collide. The default warnings
don't even warn about shadowing.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
It's been there since i8xx_irq_handler() was added in
commit c2798b19ba
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Sun Apr 22 21:13:57 2012 +0100
drm/i915: i8xx interrupt handler
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Some Poulsbo cards seem to incorrectly report
SDVO_CMD_STATUS_TARGET_NOT_SPECIFIED instead of
SDVO_CMD_STATUS_PENDING, which causes the display to be turned off.
This could also happen to i915.
Signed-off-by: Guillaume Clement <gclement@baobob.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I just noticed in our code we don't really check the assertion, and
given some of the code I am changing in this area, I feel a WARN is very
nice to have.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: s/&/&&/ to fix typo on the check.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Now that we skip clflushes more often, return a boolean indicating
whether the clflush was actually performed, and only if it was do the
chipset flush. (Though on most of the architectures where the clflush will
be skipped, the chipset flush is a no-op!)
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Here's some gma500 unifying and cleanups for drm-next. There is more stuff in
the pipe for 3.12 but I'd like to get these out of the way first.
* 'gma500-next' of git://github.com/patjak/drm-gma500: (35 commits)
drm/gma500/cdv: Add and hook up chip op for disabling sr
drm/gma500/cdv: Add and hook up chip op for watermarks
drm/gma500: Rename psb_intel_encoder to gma_encoder
drm/gma500: Rename psb_intel_connector to gma_connector
drm/gma500: Rename psb_intel_crtc to gma_crtc
drm/gma500/cdv: Convert to generic set_config()
drm/gma500/psb: Convert to generic set_config()
drm/gma500: Add generic set_config() function
drm/gma500/cdv: Convert to generic save/restore
drm/gma500/psb: Convert to generic save/restore
drm/gma500: Add generic crtc save/restore funcs
drm/gma500: Convert to generic encoder funcs
drm/gma500: Add generic encoder functions
drm/gma500/psb: Convert to generic cursor funcs
drm/gma500/cdv: Convert to generic cursor funcs
drm/gma500: Add generic cursor functions
drm/gma500/psb: Convert to generic crtc->destroy
drm/gma500/mdfld: Use identical generic crtc funcs
drm/gma500/oak: Use identical generic crtc funcs
drm/gma500/psb: Convert to gma_crtc_dpms()
...
Some Poulsbo cards seem to incorrectly report SDVO_CMD_STATUS_TARGET_NOT_SPECIFIED instead of SDVO_CMD_STATUS_PENDING, which causes the display to be turned off.
Signed-off-by: Guillaume Clement <gclement@baobob.org>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
... not only when the dma-buf is freshly created. In contrived
examples someone else could have exported/imported the dma-buf already
and handed us the gem object with a flink name. If such on object gets
reexported as a dma_buf we won't have it in the handle cache already,
which breaks the guarantee that for dma-buf imports we always hand
back an existing handle if there is one.
This is exercised by igt/prime_self_import/with_one_bo_two_files
Now if we extend the locked sections just a notch more we can also
plug th racy buf/handle cache setup in handle_to_fd:
If evil userspace races a concurrent gem close against a prime export
operation we can end up tearing down the gem handle before the dma buf
handle cache is set up. When handle_to_fd gets around to adding the
handle to the cache there will be no one left to clean it up,
effectily leaking the bo (and the dma-buf, since the handle cache
holds a ref on the dma-buf):
Thread A Thread B
handle_to_fd:
lookup gem object from handle
creates new dma_buf
gem_close on the same handle
obj->dma_buf is set, but file priv buf
handle cache has no entry
obj->handle_count drops to 0
drm_prime_add_buf_handle sets up the handle cache
-> We have a dma-buf reference in the handle cache, but since the
handle_count of the gem object already dropped to 0 no on will clean
it up. When closing the drm device fd we'll hit the WARN_ON in
drm_prime_destroy_file_private.
The important change is to extend the critical section of the
filp->prime.lock to cover the gem handle lookup. This serializes with
a concurrent gem handle close.
This leak is exercised by igt/prime_self_import/export-vs-gem_close-race
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
... and move it to the top of the function to avoid a forward
declaration.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
with the reworking semantics and locking of the obj->dma_buf pointer
this pointer is always set as long as there's still a gem handle
around and a dma_buf associated with this gem object.
Also, the per file-priv lookup-cache for dma-buf importing is also
unified between foreign and native objects.
Hence we don't need to special case the clean any more and can simply
drop the clause which only runs for foreing objects, i.e. with
obj->import_attach set.
Note that with this change (actually with the previous one to always
set up obj->dma_buf even for foreign objects) it is no longer required
to set obj->import_attach when importing a foreing object. So update
comments accordingly, too.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The export dma-buf cache is semantically similar to an flink name. So
semantically it makes sense to treat it the same and remove the name
(i.e. the dma_buf pointer) and its references when the last gem handle
disappears.
Again we need to be careful, but double so: Not just could someone
race and export with a gem close ioctl (so we need to recheck
obj->handle_count again when assigning the new name), but multiple
exports can also race against each another. This is prevented by
holding the dev->object_name_lock across the entire section which
touches obj->dma_buf.
With the new scheme we also need to reinstate the obj->dma_buf link at
import time (in case the only reference userspace has held in-between
was through the dma-buf fd and not through any native gem handle). For
simplicity we don't check whether it's a native object but
unconditionally set up that link - with the new scheme of removing the
obj->dma_buf reference when the last handle disappears we can do that.
To make it clear that this is not just for exported buffers anymore
als rename it from export_dma_buf to dma_buf.
To make sure that now one can race a fd_to_handle or handle_to_fd with
gem_close we use the same tricks as in flink of extending the
dev->object_name_locking critical section. With this change we finally
have a guaranteed 1:1 relationship (at least for native objects)
between gem objects and dma-bufs, even accounting for races (which can
happen since the dma-buf itself holds a reference while in-flight).
This prevent igt/prime_self_import/export-vs-gem_close-race from
Oopsing the kernel. There is still a leak though since the per-file
priv dma-buf/handle cache handling is racy. That will be fixed in a
later patch.
v2: Remove the bogus dma_buf_put from the export_and_register_object
failure path if we've raced with the handle count dropping to 0.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The gem flink name holds a reference onto the object itself, and this
self-reference would prevent an flink'ed object from every being
freed. To break that loop we remove the flink name when the last
userspace handle disappears, i.e. when obj->handle_count reaches 0.
Now in gem_open we drop the dev->object_name_lock between the flink
name lookup and actually adding the handle. This means a concurrent
gem_close of the last handle could result in the flink name getting
reaped right inbetween, i.e.
Thread 1 Thread 2
gem_open gem_close
flink -> obj lookup
handle_count drops to 0
remove flink name
create_handle
handle_count++
If someone now flinks this object again, we'll get a new flink name.
We can close this race by removing the lock dropping and making the
entire lookup+handle_create sequence atomic. Unfortunately to still be
able to share the handle_create logic this requires a
handle_create_tail function which drops the lock - we can't hold the
object_name_lock while calling into a driver's ->gem_open callback.
Note that for flink fixing this race isn't really important, since
racing gem_open against gem_close is clearly a userspace bug. And no
matter how the race ends, we won't leak any references.
But with dma-buf where the userspace dma-buf fd itself is refcounted
this is a valid sequence and hence we should fix it. Therefore this
patch here is just a warm-up exercise (and for consistency between
flink buffer sharing and dma-buf buffer sharing with self-imports).
Also note that this extension of the critical section in gem_open
protected by dev->object_name_lock only works because it's now a
mutex: A spinlock would conflict with the potential memory allocation
in idr_preload().
This is exercises by igt/gem_flink_race/flink_name.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
I want to wrap the creation of a dma-buf from a gem object in it,
so that the obj->export_dma_buf cache can be atomically filled in.
Instead of creating a new mutex just for that variable I've figured
I can reuse the existing dev->object_name_lock, especially since
the new semantics will exactly mirror the flink obj->name already
protected by that lock.
v2: idr_preload/idr_preload_end is now an atomic section, so need to
move the mutex locking outside.
[airlied: fix up conflict with patch to make debugfs use lock]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
if (!ret) implies that ret == 0, so no need to clear it again. And
explicitly check for ret == 0 to indicate that we're checking an errno
integer.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
When exporting a gem object as a dma-buf the critical section for the
per-fd prime lock is just the adding (and in case of errors, removing)
of the handle to the per-fd lookup cache.
So restrict the critical section to just that part of the function.
This simplifies later reordering.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Part of the function uses the properly-typed dmabuf variable, the
other an untyped void *buf. Kill the later.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
No one outside of drm should use this, the official interfaces are
drm_gem_handle_create and drm_gem_handle_delete. The handle refcounting
is purely an implementation detail of gem.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
handle_unreference only clears up the obj->name and the reference,
but would leave a dangling handle in the idr. The right thing
to do is to call handle_delete.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This is the 2nd attempt, I've always been a bit dissatisified with the
tricky nature of the first one:
http://lists.freedesktop.org/archives/dri-devel/2012-July/025451.html
The issue is that the flink ioctl can race with calling gem_close on
the last gem handle. In that case we'll end up with a zero handle
count, but an flink name (and it's corresponding reference). Which
results in a neat space leak.
In my first attempt I've solved this by rechecking the handle count.
But fundamentally the issue is that ->handle_count isn't your usual
refcount - it can be resurrected from 0 among other things.
For those special beasts atomic_t often suggest way more ordering that
it actually guarantees. To prevent being tricked by those hairy
semantics take the easy way out and simply protect the handle with the
existing dev->object_name_lock.
With that change implemented it's dead easy to fix the flink vs. gem
close reace: When we try to create the name we simply have to check
whether there's still officially a gem handle around and if not refuse
to create the flink name. Since the handle count decrement and flink
name destruction is now also protected by that lock the reace is gone
and we can't ever leak the flink reference again.
Outside of the drm core only the exynos driver looks at the handle
count, and tbh I have no idea why (it's just for debug dmesg output
luckily).
I've considered inlining the drm_gem_object_handle_free, but I plan to
add more name-like things (like the exported dma_buf) to this scheme,
so it's clearer to leave the handle freeing in its own function.
This is exercised by the new gem_flink_race i-g-t testcase, which on
my snb leaks gem objects at a rate of roughly 1k objects/s.
v2: Fix up the error path handling in handle_create and make it more
robust by simply calling object_handle_unreference.
v3: Fix up the handle_unreference logic bug - atomic_dec_and_test
retursn 1 for 0. Oops.
v4: Squash in inlining of drm_gem_object_handle_reference as suggested
by Dave Airlie and add a note that we now have a testcase.
Cc: Dave Airlie <airlied@gmail.com>
Cc: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Daniel writes:
New pile of stuff for -next:
- Cleanup of the old crtc helper callbacks, all encoders are now converted
to the i915 modeset infrastructure.
- Massive amount of wm patches from Ville for ilk, snb, ivb, hsw, this is
prep work to eventually get things going for nuclear pageflips where we
need to adjust watermarks on the fly.
- More vm/vma patches from Ben. This refactoring isn't yet fully rolled
out, we miss the execbuf conversion and some of the low-level
bind/unbind support code.
- Convert our hdmi infoframe code to use the new common helper functions
(Damien). This contains some bugfixes for the common infoframe helpers.
- Some cruft removal from Damien.
- Various smaller bits&pieces all over, as usual.
* tag 'drm-intel-next-2013-08-09' of git://people.freedesktop.org/~danvet/drm-intel: (105 commits)
drm/i915: Fix FB WM for HSW
drm/i915: expose HDMI connectors on port C on BYT
drm/i915: fix a limit check in hsw_compute_wm_results()
drm/i915: unbreak i915_gem_object_ggtt_unbind()
drm/i915: Make intel_set_mode() static
drm/i915: Remove intel_modeset_disable()
drm/i915: Make intel_encoder_dpms() static
drm/i915: Make i915_hangcheck_elapsed() static
drm/i915: Fix #endif comment
drm/i915: Remove i915_gem_object_check_coherency()
drm/i915: Remove stale prototypes
drm/i915: List objects allocated from stolen memory in debugfs
drm/i915: Always call intel_update_sprite_watermarks() when disabling a plane
drm/i915: Pass plane and crtc to intel_update_sprite_watermarks
drm/i915: Don't try to disable plane if it's already disabled
drm/i915: Pass crtc to our update/disable_plane hooks
drm/i915: Split plane watermark parameters into a separate struct
drm/i915: Pull some watermarks state into a separate structure
drm/i915: Calculate max watermark levels for ILK+
drm/i915: Rename hsw_lp_wm_result to intel_wm_level
...
It's only used in drm_platform.c.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This function is only used in drm_fb_cma_helper.c.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
It's only used in drm_crtc.c.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The last user was removed in
commit 575dc34ee0
Author: Dave Airlie <airlied@redhat.com>
Date: Mon Sep 7 18:43:26 2009 +1000
drm/kms: remove old std mode fallback code.
The new code adds modes in the helper, which makes more sense
I disliked the non-driver code adding modes.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This was last used by nouveau, replaced by a driver-specific property
in:
commit de69185573
Author: Ben Skeggs <bskeggs@redhat.com>
Date: Mon Oct 17 12:23:41 2011 +1000
drm/nouveau: improve dithering properties, and implement proper auto mode
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
In commit 77145f1cbd was introduced
error which cause that reclocking on nv40 not working anymore.
There is missing assigment of return value from pll_calc to ret.
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Signed-off-by: Martin Peres <martin.peres@labri.fr>
Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
Allocating type=0 marks the memory as free. This allows the ltcg memory
to be allocated twice.
Add a BUG_ON in core/mm.c to prevent this ever happening again.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
Some registers were not initialized in init, this causes them to be
uninitialized after suspend.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
Commit dceef5d87 (drm/nouveau/fb: initialise vram controller as pfb
sub-object) moved some code around and introduced these null derefs.
pfb->ram is set to the new ram object outside of this ctor.
Reported-by: Ronald Uitermark <ronald645@gmail.com>
Tested-by: Ronald Uitermark <ronald645@gmail.com>
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
We kzalloc this structure, and for real kms devices we should never
loose track of things really.
But ums/legacy drivers rely on the drm core to clean up a bit of cruft
between lastclose and firstopen (i.e. when X is being restarted), so
keep this around. But give it a clear drm_legacy_ prefix and
conditionalize the code on !DRIVER_MODESET.
Cc: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
So almost two years ago I've tried to nuke the procfs code already
once before:
http://lists.freedesktop.org/archives/dri-devel/2011-October/015707.html
The conclusion was that userspace drivers (specifically libdrm device
node detection) stopped relying on procfs in 2001. But after some
digging it turned out that the drmstat tool in libdrm is still using
those files (but only when certain options are set). So we've decided
to keep profcs.
But I when I've started to dig around again what exactly this tool
does I've noticed that it tries to read the "mem", "vm", and "vma"
files from procfs. Now as far my git history digging shows "mem" never
did anything useful (at least in the version that first showed up in
upstream history in 2004) and the file was remove in
commit 955b12def4
Author: Ben Gamari <bgamari@gmail.com>
Date: Tue Feb 17 20:08:49 2009 -0500
drm: Convert proc files to seq_file and introduce debugfs
Which means that for over 4 years drmstat has been broken, and no one
cared. In my opinion that's proof enough that no one is actually using
drmstat, and so that we can savely nuke the procfs support from drm.
While at it fix up the error case cleanup for debugfs in drm_get_minor.
v2: Fix dates, libdrm stopped relying on procfs for drm node detection
in 2001.
v3: fixup compilation warning for !CONFIG_DEBUG_FS, reported by
Fengguang Wu.
Cc: kbuild test robot <fengguang.wu@intel.com>
Cc: Dave Airlie <airlied@linux.ie>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
It has way too much potential for driver writers to do stupid things
like delayed hw setup because the load sequence is somehow racy (e.g.
the imx driver in staging). So don't call it for modesetting drivers,
which reduces the complexity of the drm core -> driver interface a
notch.
v2: Don't forget to update DocBook.
v3: Go with Laurent's slightly more elaborate proposal for the DocBook
update. Add a few words on top of his diff to elaborate a bit on what
KMS drivers should and shouldn't do in lastclose. There was already a
paragraph present talking about restoring properties, I've simply
extended that one.
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
So if we survey kms drivers there's a bunch of things they commonly do
in ->lastclose
- delayed processing of vga switcheroo requests (i915, nouveau,
radeon)
- force-restoring the fbcon (most)
- resetting a bunch properties to make fbcon work better (omap)
- disabling all outputs (vmwgfx)
In short besides the semantically important vga switcheroo stuff they
all try very hard to keep fbcon working in case X dies.
But none of them try to not do this at driver unload time safe for
vmwgfx, and digging through logs I couldn't find any reason for why
vmwgfx is special.
Since ->firstopen has lots of potential for abuse with kms drivers
(like delaying driver setup to pamper over races in the load sequence)
it's imo very much worth it to remove this logic so that we can
stop using the ->firstopen callback for kms drivers.
Also module unloading is rather a debug feature and developers should
know how to restore the display to a sane configuration.
Cc: Jakob Bornecrantz <jakob@vmware.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
