Well, at least step 1. The goal here is that framebuffer objects can
survive outside of the mode_config lock, with just a reference held
as protection. The first step to get there is to introduce a special
fb_lock which protects fb lookup, creation and destruction, to make
them appear atomic.
This new fb_lock can nest within the mode_config lock. But the idea is
(once the reference counting part is completed) that we only quickly
take that fb_lock to lookup a framebuffer and grab a reference,
without any other locks involved.
vmwgfx is the only driver which does framebuffer lookups itself, also
wrap those calls to drm_mode_object_find with the new lock.
Also protect the fb_list walking in i915 and omapdrm with the new lock.
As a slight complication there's also the list of user-created fbs
attached to the file private. The problem now is that at fclose() time
we need to walk that list, eventually do a modeset call to remove the
fb from active usage (and are required to be able to take the
mode_config lock), but in the end we need to grab the new fb_lock to
remove the fb from the list. The easiest solution is to add another
mutex to protect this per-file list.
Currently that new fbs_lock nests within the modeset locks and so
appears redudant. But later patches will switch around this sequence
so that taking the modeset locks in the fb destruction path is
optional in the fastpath. Ultimately the goal is that addfb and rmfb
do not require the mode_config lock, since otherwise they have the
potential to introduce stalls in the pageflip sequence of a compositor
(if the compositor e.g. switches to a fullscreen client or if it
enables a plane). But that requires a few more steps and hoops to jump
through.
Note that framebuffer creation/destruction is now double-protected -
once by the fb_lock and in parts by the idr_lock. The later would be
unnecessariy if framebuffers would have their own idr allocator. But
that's material for another patch (series).
v2: Properly initialize the fb->filp_head list in _init, otherwise the
newly added WARN to check whether the fb isn't on a fpriv list any
more will fail for driver-private objects.
v3: Fixup two error-case unlock bugs spotted by Richard Wilbur.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
->cursor_move uses mostly the same facilities in drivers as
->cursor_set, so pretty much nothing to fix up:
- ast/gma500/i915: They all use per-crtc registers to update the
cursor position. ast again touches the global cursor cache, but
that's ok since there's only one crtc.
- nouveau: nv50+ is again special, updates happen through the per-crtc
channel (without pushbufs), so it's not protected by the new evo
lock introduced earlier. But since this channel is per-crtc, we
should be fine anyway.
- radeon: A bit a mess: avivo asics need a workaround when both output
pipes are enabled, which means it'll access the crtc list. Just
reading that flag is ok though as long as radeon _always_ grabs all
locks when changing the crtc configuration. Which means with the
current scheme it cannot do an optimized modeset which only locks
the relevant crtcs. This can be fixed though by introducing a bit of
global state with separate locks and ensure in the modeset code that
the cursor will be updated appropriately when enabling the 2nd pipe
(on affected asics).
- vmwgfx: I still don't understand what it's doing exactly, so apply
the same trick for now.
v2: Fixup unlocking for the error cases, spotted by Richard Wilbur.
v3: Another error-case fixup.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
First convert ->cursor_set to only take the crtc lock, since that
seems to be the function with the least amount of state - the core
ioctl function doesn't check anything which can change at runtime, so
we don't have any object lifetime issues to contend.
The only thing which is important is that the driver's implementation
doesn't touch any state outside of that single crtc which is not yet
properly protected by other locking:
- ast: access the global ast->cache_kmap. Luckily we only have on crtc
on this driver, so this is fine. Add a comment.
- gma500: calls gma_power_begin|and and psb_gtt_pin|unpin, both which
have their own locking to protect their state. Everything else is
crtc-local.
- i915: touches a bit of global gem state, all protected by the One
Lock to Rule Them All (dev->struct_mutex).
- nouveau: Pre-nv50 is all nice, nv50+ uses the evo channels to queue
up all display changes. And some of these channels are device
global. But this is fine now since the previous patch introduced an
evo channel mutex.
- radeon: Uses some indirect register access for cursor updates, but
with the previous patches to protect these indirect 2-register
access patterns with a spinlock, this should be fine now, too.
- vmwgfx: I have no idea how that works - update_cursor_position
doesn't take any per-crtc argument and I haven't figured out any
other place where this could be set in some form of a side-channel.
But vmwgfx definitely has more than one crtc (or at least can
register more than one), so I have no idea how this is supposed to
not fail with the current code already. Hence take the easy way out
and simply acquire all locks (which requires dropping the crtc lock
the core acquired for us). That way it's not worse off for
consistency than the old code.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
*drumroll*
The basic idea is to protect per-crtc state which can change without
touching the output configuration with separate mutexes, i.e. all the
input side state to a crtc like framebuffers, cursor settings or plane
configuration. Holding such a crtc lock gives a read-lock on all the
other crtc state which can be changed by e.g. a modeset.
All non-crtc state is still protected by the mode_config mutex.
Callers that need to change modeset state of a crtc (e.g. dpms or
set_mode) need to grab both the mode_config lock and nested within any
crtc locks.
Note that since there can only ever be one holder of the mode_config
lock we can grab the subordinate crtc locks in any order (if we need
to grab more than one of them). Lockdep can handle such nesting with
the mutex_lock_nest_lock call correctly.
With this functions that only touch connectors/encoders but not crtcs
only need to take the mode_config lock. The biggest such case is the
output probing, which means that we can now pageflip and move cursors
while the output probe code is reading an edid.
Most cases neatly fall into the three buckets:
- Only touches connectors and similar output state and so only needs
the mode_config lock.
- Touches the global configuration and so needs all locks.
- Only touches the crtc input side and so only needs the crtc lock.
But a few cases that need special consideration:
- Load detection which requires a crtc. The mode_config lock already
prevents a modeset change, so we can use any unused crtc as we like
to do load detection. The only thing to consider is that such
temporary state changes don't leak out to userspace through ioctls
that only take the crtc look (like a pageflip). Hence the load
detect code needs to grab the crtc of any output pipes it touches
(but only if it touches state used by the pageflip or cursor
ioctls).
- Atomic pageflip when moving planes. The first case is sane hw, where
planes have a fixed association with crtcs - nothing needs to be
done there. More insane^Wflexible hw needs to have plane->crtc
mapping which is separately protect with a lock that nests within
the crtc lock. If the plane is unused we can just assign it to the
current crtc and continue. But if a plane is already in use by
another crtc we can't just reassign it.
Two solution present themselves: Either go back to a slow-path which
takes all modeset locks, potentially incure quite a hefty delay. Or
simply disallowing such changes in one atomic pageflip - in general
the vblanks of two crtcs are not synced, so there's no sane way to
atomically flip such plane changes accross more than one crtc. I'd
heavily favour the later approach, going as far as mandating it as
part of the ABI of such a new a nuclear pageflip.
And if we _really_ want such semantics, we can always get them by
introducing another pageflip mutex between the mode_config.mutex and
the individual crtc locks. Pageflips crossing more than one crtc
would then need to take that lock first, to lock out concurrent
multi-crtc pageflips.
- Optimized global modeset operations: We could just take the
mode_config lock and then lazily lock all crtc which are affected by
a modeset operation. This has the advantage that pageflip could
continue unhampered on unaffected crtc. But if e.g. global resources
like plls need to be reassigned and so affect unrelated crtcs we can
still do that - nested locking works in any order.
This patch just adds the locks and takes them in drm_modeset_lock_all,
no real locking changes yet.
v2: Need to initialize the new lock in crtc_init and lock it righ
away, for otherwise the modeset_unlock_all below will try to unlock a
not-locked mutex.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I've left the locking in the debugfs code as-is, it's essentially just
used to keep the framebuffer object alive (which won't be necessary
any more later on). We don't need fb refcounting either, since the new
mode_config.fb_lock ensures that the framebuffers can't disappear
(once mode_config.mutex doesn't guarantee this any more later on in
the series).
The fbcon restore needs all modeset locks. The crtc callbacks seem to
only need the crtc locks, but I've quickly discussed things with Rob
Clark and he's fine with just using modeset_lock_all for those, too.
He'll look into converting things over later.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Ok, this one here is a bit more complicated, and I can't really claim
to fully understand the locking and lifetime rules of the vmwgfx
driver. So just convert ever mutex_lock call, including the
interruptible one. Since other places (e.g. in the execbuf ioctl) take
the mode_config.mutex without bothering with interruptible handling,
I've figured I should be able to get away with this in a few more
places ...
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Only two places:
- suspend/resume
- Some really strange mode validation tool with too much funny-lucking
hand-rolled conversion code.
- The recently-added lastclose fbdev restore code.
Better safe than sorry, so convert both places to keep the locking
semantics as much as possible.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Two exceptions:
- debugfs files only read information which is not related to crtc, so
can stay on the modeset_config lock.
- Same holds for the edp vdd work in intel_dp.c. Add a corresponding
WARN_ON and a comment next to the intel_dp struct fields for
documentation.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This is the first step towards introducing the new modeset locking
scheme. The plan is to put helper functions into place at all the
right places step-by-step, so that the final patch to switch on the
new locking scheme doesn't need to touch every single driver.
This helper here will serve as the shotgun solutions for all places
where a more fine-grained locking isn't (yet) implemented.
v2: Fixup kerneldoc for unlock_all.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The ia64 function "thread_matches()" has no users since commit
e868a55c2a ("[IA64] remove find_thread_for_addr()"). Remove it.
This allows us to make ptrace_check_attach() static to kernel/ptrace.c,
which is good since we'll need to change the semantics of it and fix up
all the callers.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
With refcounting we need to adjust framebuffer refcounts at each
callsite - much easier to do if they all call the same little helper
function.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Some drivers don't have real ->create_handle callbacks.
- cirrus/ast/mga200: Returns either 0 or -EINVAL.
- udl: Didn't even bother with a callback, leading to a nice
userspace-triggerable OOPS.
- vmwgfx: This driver bothered with an implementation to return 0 as
the handle (which is the canonical no-obj gem handle).
All have in common that ->create_handle doesn't really make too much
sense for them - that ioctl is used only for seamless fb takeover in
the radeon/nouveau/i915 ddx drivers. So allow drivers to not implement
this and return a consistent -ENODEV.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
... by moving the bo_pin/bo_unpin manipulation of the pin_refcount
under the protection of the ttm reservation lock. pin/unpin seems
to get called from all over the place, so atm this is completely racy.
After this patch there are only a few places in cleanup functions
left which access ->pin_refcount without locking. But I'm hoping that
those are safe and some other code invariant guarantees that this
won't blow up.
In any case, I only need to fix up pin/unpin to make ->pageflip work
safely, so let's keep it at that.
Add a comment to the header to explain the new locking rule.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With per-crtc locks modeset operations can run in parallel, and the
cursor code uses the device-global evo master channel for hw frobbing.
But the pageflip code can also sync with the master under some
circumstances. Hence just wrap things up in a mutex to ensure that
pushbuf access doesn't intermingle.
The approach here is a bit overkill since the per-crtc channels used
to schedule the pageflips could probably be used without this pushbuf
locking, but I'm not familiar enough with the nouveau codebase to be
sure of that.
v2: Add missing mutex_init to avoid angering lockdep.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Doing this within the fb->destroy callback leads to a locking
nightmare. And all other drm drivers that restore the fbcon do
it in lastclose, too.
With this adjustments all fb->destroy callbacks optionally drop
references to any gem objects used as backing storage, call
drm_framebuffer_cleanup and then kfree the struct. Which nicely
simplifies the locking for framebuffer unreferencing and freeing,
since this doesn't require that we hold the mode_config lock. A
slight exception is the vmwgfx surface backed framebuffer, it also
calls drm_master_put and removes the object from a device-private
framebuffer list. Both seem to have solid locking in place already.
Conclusion is that now it is no longer required to hold the
mode_config lock while freeing a framebuffer.
v2: Drop the corresponding mutex_lock WARN check from
drm_framebuffer_unreference.
v3: Use just the mode_config lock not modeset_lock_all, due to patch
reordering.
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
vmwgfx has an oddity, when failing to reference the surface it'll
return 0, since that's what the successfull drm_framebuffer_init will
leave behind in ret. Fix this up by returning -EINVAL.
Split out from all the other driver updates due to the above tiny
semantic change. Shouldn't matter though since the reference grabbing
seemingly can't fail.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With more fine-grained locking we can no longer rely on the big
mode_config lock to prevent concurrent access to mode resources
like framebuffers. Instead a framebuffer becomes accessible to
other threads as soon as it is added to the relevant lookup
structures. Hence it needs to be fully set up by the time drivers
call drm_framebuffer_init.
This patch here is the drivers part of that reorg. Nothing really fancy
going on safe for three special cases.
- exynos needs to be careful to properly unref all handles.
- nouveau gets a resource leak fixed for free: one of the error
cases didn't cleanup the framebuffer, which is now moot since
the framebuffer is only registered once it is fully set up.
- vmwgfx requires a slight reordering of operations, I'm hoping I didn't
break anything (but it's refcount management only, so should be safe).
v2: Split out exynos, since it's a bit more hairy than expected.
v3: Drop bogus cirrus hunk noticed by Richard Wilbur.
v4: Split out vmwgfx since there's a small change in return values.
Reviewed-by: Rob Clark <rob@ti.com> (core + omapdrm)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
And do a quick pass to adjust them to the last few (years?) of changes
...
This time actually compile-tested ;-)
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
- config_cleanup was confused: It claimed that callers need to hold
the modeset lock, but the connector|encoder_cleanup helpers grabbed
that themselves (note that crtc_cleanup did _not_ grab the modeset
lock). Which resulted in all drivers _not_ hodling the lock. Since
this is for single-threaded cleanup code, drop the requirement from
docs and also drop the lock_grabbing from all _cleanup functions.
- Kill the LOCKING section in the doctype, since clearly we're not
good enough to keep them up-to-date. And misleading locking
documentation is worse than useless (see e.g. the comment in the
vmgfx driver about the cleanup mess). And since for most functions
the very first line either grabs the lock or has a WARN_ON(!locked)
the documentation doesn't really add anything.
- Instead put in some effort into explaining the only two special
cases a bit better: config_init and config_cleanup are both called
from single-threaded setup/teardown code, so don't do any locking.
It's the driver's job though to enforce this.
- Where lacking, add a WARN_ON(!is_locked). Not many places though,
since locking around fbdev setup/teardown is through-roughly screwed
up, and so will break almost every single WARN annotation I've tried
to add.
- Add a drm_modeset_is_locked helper - the Grate Modset Locking Rework
will use the compiler to assist in the big reorg by renaming the
mode lock, so start encapsulating things. Unfortunately this ended
up in the "wrong" header file since it needs the definition of
struct drm_device.
v2: Drop most WARNS again - we hit them all over the place, mostly in
the setup and teardown sequences. And trying to fix it up leads to
nice deadlocks, since the locking in the setup code is really
inconsistent.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Operation-specific check (whether subvol is readonly or not) should go
after the mutual exclusiveness check.
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
The error code that is returned in response to starting a mutually
exclusive operation when there is one already running got silently
changed from EINVAL to EINPROGRESS by 5ac00add. Returning EINPROGRESS
to, say, add_dev, when rm_dev is running is misleading. Furthermore,
the operation itself may want to use EINPROGRESS for other purposes.
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Balance pause/resume logic got broken by 5ac00add (went in into 3.8-rc1
as part of dev-replace merge). Offending commit took a stab at making
mutually exclusive volume operations (add_dev, rm_dev, resize, balance,
replace_dev) not block behind volume_mutex if another such operation is
in progress and instead return an error right away. Balancing front-end
relied on the blocking behaviour, so the fix is ugly, but short of a
complete rework, it's the best we can do.
Reported-by: Liu Bo <bo.li.liu@oracle.com>
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
the variable chan is dereferenced in line 635, so it is no reason to check
null again in line 641.
Signed-off-by: Cong Ding <dinggnu@gmail.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
the pointer cfg is dereferenced in line 594, so it's no reason to check null
again in line 620.
Signed-off-by: Cong Ding <dinggnu@gmail.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Normally kmalloc() returns things that are DMA safe so not visible on all
platforms but we do need to explicitly request DMA safe memory.
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Normally kmalloc() returns things that are DMA safe so not visible on all
platforms but we do need to explicitly request DMA safe memory.
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Now that we seem to have brought order to the GTT barriers, the last one
to review is the terminal barrier before we unbind the buffer from the
GTT. This needs to only be performed if the buffer still resides in the
GTT domain, and so we can skip some needless barriers otherwise.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With a fence, we only need to insert a memory barrier around the actual
fence alteration for CPU accesses through the GTT. Performing the
barrier in flush-fence was inserting unnecessary and expensive barriers
for never fenced objects.
Note removing the barriers from flush-fence, which was effectively a
barrier before every direct access through the GTT, revealed that we
where missing a barrier before the first access through the GTT. Lack of
that barrier was sufficient to cause GPU hangs.
v2: Add a couple more comments to explain the new barriers
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We have two important transitions of the wedged state in the current
code:
- 0 -> 1: This means a hang has been detected, and signals to everyone
that they please get of any locks, so that the reset work item can
do its job.
- 1 -> 0: The reset handler has completed.
Now the last transition mixes up two states: "Reset completed and
successful" and "Reset failed". To distinguish these two we do some
tricks with the reset completion, but I simply could not convince
myself that this doesn't race under odd circumstances.
Hence split this up, and add a new terminal state indicating that the
hw is gone for good.
Also add explicit #defines for both states, update comments.
v2: Split out the reset handling bugfix for the throttle ioctl.
v3: s/tmp/wedged/ sugested by Chris Wilson. Also fixup up a rebase
error which prevented this patch from actually compiling.
v4: To unify the wedged state with the reset counter, keep the
reset-in-progress state just as a flag. The terminally-wedged state is
now denoted with a big number.
v5: Add a comment to the reset_counter special values explaining that
WEDGED & RESET_IN_PROGRESS needs to be true for the code to be
correct.
v6: Fixup logic errors introduced with the wedged+reset_counter
unification. Since WEDGED implies reset-in-progress (in a way we're
terminally stuck in the dead-but-reset-not-completed state), we need
ensure that we check for this everywhere. The specific bug was in
wait_for_error, which would simply have timed out.
v7: Extract an inline i915_reset_in_progress helper to make the code
more readable. Also annote the reset-in-progress case with an
unlikely, to help the compiler optimize the fastpath. Do the same for
the terminally wedged case with i915_terminally_wedged.
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
While auditing the code I've noticed one place (the throttle ioctl)
which does not yet wait for the reset handler to complete and doesn't
properly decode the wedge state into -EAGAIN/-EIO. Fix this up by
calling the right helpers. This might explain the oddball "my
compositor just died in a successfull gpu reset" reports. Or maybe not, since
current mesa doesn't use this ioctl to throttle command submission.
The throttle ioctl doesn't take the struct_mutex, so to avoid busy-looping
with -EAGAIN while a reset is in process, check for errors first and wait
for the handler to complete if a reset is pending by calling
i915_gem_wait_for_error.
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
And to make Ben Widawsky happier, use the gpu_error instead of
the entire device as the argument in some functions.
Drop the outdated comment on ->wedged for now, a follow-up patch will
change the semantics and add a proper comment again.
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This has been sprinkled all over the place in dev_priv. I think
it'd be good to also move all the code into a separate file like
i915_gem_error.c, but that's for another patch.
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Tha one is really big, since it contains tons of comments explaining
how things work. Which is nice ;-)
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
It is no longer used in the i915 code, so isolate it from the shared
struct.
This was originally part of:
commit 0e275518f325418d559c05327775bff894b237f7
Author: Ben Widawsky <ben@bwidawsk.net>
Date: Mon Jan 14 13:35:33 2013 -0800
agp/intel: decouple more of the agp-i915 sharing
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
That commit had some other hunks which can't be used due to issues
Daniel found in previous commits.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: drop squash notice from the commit since it's imo ok to keep
this one separate.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The reasoning behind our code taking two paths depending upon whether or
not we may have been configured for IOMMU isn't clear to me. It should
always be safe to use the pci mapping functions as they are designed to
abstract the decision we were handling in i915.
Aside from simpler code, removing another member for the intel_gtt
struct is a nice motivation.
I ran this by Chris, and he wasn't concerned about the extra kzalloc,
and memory references vs. page_to_phys calculation in the case without
IOMMU.
v2: Update commit message
v3: Remove needs_dmar addition from Zhenyu upstream
This reverts (and then other stuff)
commit 20652097da
Author: Zhenyu Wang <zhenyuw@linux.intel.com>
Date: Thu Dec 13 23:47:47 2012 +0800
drm/i915: Fix missed needs_dmar setting
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> (v2)
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: Squash in follow-up fix to remove the bogus hunk which
deleted the dma_mask configuration for gen6+.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We already had a mapping in both (minus the phys_addr in AGP).
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
And, move it to where the rest of the logic is.
There is some slight functionality changes. There was extra paranoid
checks in AGP code making sure we never do idle maps on gen2 parts. That
was not duplicated as the simple PCI id check should do the right thing.
v2: use IS_GEN5 && IS_MOBILE check instead. For now, this is the same as
IS_IRONLAKE_M but is more future proof. The workaround docs hint that
more than one platform may be effected, but we've never seen such a
platform in the wild. (Rodrigo, Daniel)
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> (v1)
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The AVI infoframe is able to inform the display whether the source is
sending full or limited range RGB data.
As per CEA-861 [1] we must first check whether the display reports the
quantization range as selectable, and if so we can set the approriate
bits in the AVI inforframe.
[1] CEA-861-E - 6.4 Format of Version 2 AVI InfoFrame
v2: Give the Q bits better names, add spec chapter information
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drm_rgb_quant_range_selectable() will report whether the monitor
claims to support for RGB quantization range selection.
The information can be found in the CEA Video capability block.
v2: s/quantzation/quantization/ in the comment
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: David Airlie <airlied@linux.ie>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Add a new "Automatic" mode to the "Broadcast RGB" range property.
When selected the driver automagically selects between full range and
limited range output.
Based on CEA-861 [1] guidelines, limited range output is selected if the
mode is a CEA mode, except 640x480. Otherwise full range output is used.
Additionally DVI monitors should most likely default to full range
always.
As per DP1.2a [2] DisplayPort should always use full range for 18bpp, and
otherwise will follow CEA-861 rules.
NOTE: The default value for the property will now be "Automatic"
so some people may be affected in case they're relying on the
current full range default.
[1] CEA-861-E - 5.1 Default Encoding Parameters
[2] VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry
v2: Use has_hdmi_sink to check if a HDMI monitor is present
v3: Add information about relevant spec chapters
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The RGB color range select bit on the DP/SDVO/HDMI registers
disappeared when PCH was introduced, and instead a new PIPECONF bit
was added that performs the same function.
Add a new INTEL_MODE_LIMITED_COLOR_RANGE private mode flag, and set
it in the encoder mode_fixup if limited color range is requested.
Set the the PIPECONF bit 13 based on the flag.
Experimentation showed that simply toggling the bit while the pipe is
active doesn't work. We need to restart the pipe, which luckily already
happens.
The DP/SDVO/HDMI bit 8 is marked MBZ in the docs, so avoid setting it,
although it doesn't seem to do any harm in practice.
TODO:
- the PIPECONF bit too seems to have disappeared from HSW. Need a
volunteer to test if it's just a documentation issue or if it's really
gone. If the bit is gone and no easy replacement is found, then I suppose
we may need to use the pipe CSC unit to perform the range compression.
v2: Use mode private_flags instead of intel_encoder virtual functions
v3: Moved the intel_dp color_range handling after bpc check to help
later patches
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46800
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Mappable_end, ie. size is almost always what you want as opposed to the
number of entries. Since we already have that information, we can scrap
the number of entries and only calculate it when needed.
If gtt_start is !0, this will have slightly different behavior. This
difference can only occur in DRI1, and exists when we try to kick out
the firmware fb. The new code seems like a bugfix to me.
The other case where we've changed the behavior is during init we check
the mappable region against our current known upper and lower limits
(64MB, and 512MB). This now matches the comment, and makes things more
convenient after removing gtt_mappable_entries.
Also worth noting is the setting of mappable_end is taken out of setup
because we do it earlier now in the DRI2 case and therefore need to add
that tiny hunk to support the DRI1 IOCTL.
v2: Move up mappable end to before legacy AGP init
v3: Add the dev_priv inclusion here from previous rebase error in patch
5
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> (v2)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: squash in fix for a printk format flag mismatch warning.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The edma_slave_config() implementation depends on the
direction field such that it will not properly configure
a slave channel when called without direction set.
This fixes the implementation so that the slave config
is copied as is and prep_slave_sg() handles the
direction dependent handling. spi-omap2-mcspi and
omap_hsmmc both expose this bug as they configure the
slave channel config from a common path with an unconfigured
direction field.
Signed-off-by: Matt Porter <mporter@ti.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
