There is an extra semi-colon here so we just leak and never unbind
anything.
This regression has been introduced in
commit 07fe0b1280
Author: Ben Widawsky <ben@bwidawsk.net>
Date: Wed Jul 31 17:00:10 2013 -0700
drm/i915: plumb VM into bind/unbind code
Cc: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
And also fix a small typo in the intel_encoder_dpms() comment.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This code was dead since:
commit 432e58edc9
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Thu Nov 25 19:32:06 2010 +0000
drm/i915: Avoid allocation for execbuffer object list
so just put it to rest for good.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I was curious as to what objects were currently allocated from stolen
memory, and so exported it from debugfs.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
ILK and VLV codepaths didn't update sprite watermarks when disabling a
sprite. Make them do that.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We're going to want to know the crtc in the watermark code to avoid
doing more work than we have to. We should also pass the plane we're
disabling so that we know where to stick our watermark parameters
without having to go look the plane up.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Check plane->fb in intel_disable_plane() to determine if the plane
is already disabled.
If the plane has an fb, then it must also have a crtc, so we can drop
the plane->crtc check and just call intel_enable_primary() directly.
v2: WARN and bail if the plane doesn't have a crtc when it should
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We're going to want to know which CRTC we're dealing with, so pass it
down to the update/disable_plane hooks.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Give a name to the plane watermark related data we have currently
stored under intel_plane->wm.
We also observe that this data is more or less the same that we have
in the hsw_pipe_wm_parameters structure, so use it there as well.
v2: Make pahole happier
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
There is a bunch of global state that needs to be considered when
checking watermarks for validity. Move most of that to a new
structure intel_wm_config, to avoid having to pass around so
many variables.
One notable thing left out is the DDB partitioning information,
since we often anyway need to check the same watermarks against
both 1/2 and 5/6 DDB partitioning layouts.
v2: s/pipes_active/num_pipes_active
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
There are quite a few variables we need to take into account to
determine the maximum watermark levels, so it feels a bit cleaner
to calculate those rather than just have a bunch of what look like
magic numbers.
v2: s/pipes_active/num_pipes_active
s/othwewise/otherwise
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Let's call hsw_lp_wm_result intel_wm_level from now on and move it to
i915_drv.h for later use.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Refactor the code a bit to split the watermark level validity check into
a separate function.
Also add hack there that allows us to use it even for LP0 watermarks.
ATM we don't pre-compute/check the LP0 watermarks, so we just have to
clamp them to the maximum and hope things work out.
v2: Add some debug prints when we exceed max WM0
Kill pointless ret = false' assignment.
Include the check for the already disabled 'result' which
got shuffled around when the patchs got reorderd
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
With the current code there shouldn't be a distinction - however with an
upcoming change we intend to allocate a vma much earlier, before it's
actually bound anywhere.
To do this we have to check node allocation as well for the _bound()
check.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: move list_del(&vma->vma_link) from vma_unbind to vma_destroy,
again fallout from the loss of "rm/i915: Cleanup more of VMA in
destroy".]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
fixup for drm/i915: Add vma to list at creation
formerly: "drm/i915: Create VMAs (part 4) - Error capture"
Since the active/inactive lists are per VM, we need to modify the error
capture code to be aware of this, and also extend it to capture the
buffers from all the VMs. For now all the code assumes only 1 VM, but it
will become more generic over the next few patches.
NOTE: If the number of VMs in a real world system grows significantly
we'll have to focus on only capturing the guilty VM, or else it's likely
there won't be enough space for error capture.
v2: Squashed in the "part 6" which had dependencies on the mm_list
change. Since I've moved the mm_list change to an earlier point in the
series, we were able to accomplish it here and now.
v3: Rebased over new error capture
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
formerly: "drm/i915: Create VMAs (part 5) - move mm_list"
The mm_list is used for the active/inactive LRUs. Since those LRUs are
per address space, the link should be per VMx .
Because we'll only ever have 1 VMA before this point, it's not incorrect
to defer this change until this point in the patch series, and doing it
here makes the change much easier to understand.
Shamelessly manipulated out of Daniel:
"active/inactive stuff is used by eviction when we run out of address
space, so needs to be per-vma and per-address space. Bound/unbound otoh
is used by the shrinker which only cares about the amount of memory used
and not one bit about in which address space this memory is all used in.
Of course to actual kick out an object we need to unbind it from every
address space, but for that we have the per-object list of vmas."
v2: only bump GGTT LRU in i915_gem_object_set_to_gtt_domain (Chris)
v3: Moved earlier in the series
v4: Add dropped message from v3
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: Frob patch to apply and use vma->node.size directly as
discused with Ben. Also drop a needles BUG_ON before move_to_inactive,
the function itself has the same check.]
[danvet 2nd: Rebase on top of the lost "drm/i915: Cleanup more of VMA
in destroy", specifically unlink the vma from the mm_list in
vma_unbind (to keep it symmetric with bind_to_vm) instead of
vma_destroy.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
formerly: "drm/i915: Create VMAs (part 3.5) - map and fenceable
tracking"
The map_and_fenceable tracking is per object. GTT mapping, and fences
only apply to global GTT. As such, object operations which are not
performed on the global GTT should not effect mappable or fenceable
characteristics.
Functionally, this commit could very well be squashed in to a previous
patch which updated object operations to take a VM argument. This
commit is split out because it's a bit tricky (or at least it was for
me).
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: Drop the bogus hunk in i915_vma_unbind as discussed with
Ben.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/i915/i915_debugfs.c:2136:3: warning: symbol
'i915_debugfs_files' was not declared. Should it be static?
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We're going to use the 1/2 vs. 5/6 split option already on IVB so the
HSW name is not proper. Just give it an intel_ prefix and move it to
i915_drv.h so that we can use it there later.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We don't need to store the FBC WM enabled status in each watermark
level. We anyway have to reduce it down to a single boolean, so just
delay checking the FBC WM limit until we're computing the final
value.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Refactor the watermarks computation for one level to a separate
function. This function will now set the ->enable flag to true,
even if the watermark level wasn't actually checked yet. In the
future we will delay the checking so we must consider all unchecked
watermarks as possibly valid.
v2: Preserve comment about latency units
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Let's be consistent and always call our variables 'enabled' insted of
the occasional 'enable'.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
[danvet: Spelling fix in the commit message, spotted by Chris.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
set_frame() wraps the write_frame() vfunc. Be consistent and name the
wrapping function like the vfunc being called.
It's doubly confusing as we also have a set_infoframes() vfunc and
set_infoframe() doesn't wrap it.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I cannot find any evidence what we shouldn't try to set those fields
when setting a non-CEA mode on an HDMI sink. So just kill that return.
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
All the HDMI infoframe code has been ported to use video/hdmi.c, so it's
time to say bye bye to this code.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
Signed-off-by: Thierry Reding <thierry.reding at avionic-design.de>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Let's use the drivers/video/hmdi.c and drm infoframe helpers to build
our infoframes.
v2: Simplify the logic to compute the buffer size. We can just take the
maximum infoframe size rounded to 32, which happens to be what the
hardware let us write anyway.
v3: Remove unnecessary memset() (Ville Syrjälä)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
First step in the move to the shared infoframe infrastructure, let's
move the different infoframe helpers and the write_infoframe() vfunc to
a type (enum hdmi_infoframe_type) and a buffer + len instead of using
our struct dip_infoframe.
v2: constify the infoframe pointer and don't mix signs (Ville Syrjälä)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
Signed-off-by: Thierry Reding <thierry.reding at avionic-design.de>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
From CEA-861:
Data Byte 1, bit A0 indicates whether Active Format Data is present in
Data Byte 2 bits R3 through R0. A source device shall set A0=1 when
any of the AFD bits are set.
ie. if we want to set active_aspect, we need to set the
active_info_valid bit to 1 as well.
Cc: Thierry Reding <thierry.reding@avionic-design.de>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In some places, we want to know if an object is bound in any address
space, and not just the global GTT. This often applies when there is a
single global resource (object, pages, etc.)
function | reason
--------------------------------------------------
i915_gem_object_is_inactive | global object
i915_gem_object_put_pages | object's pages
915_gem_object_unpin | global object
i915_gem_execbuffer_unreserve_object | temporary until we plumb vma
pread/pwrite | see the note below
Note: set_to_gtt_domain in pwrite/pread is abused as a wait_rendering
call - but that once only worked if the object is bound. We really
should replace this with a plain wait_rendering call, which would have
the upside that in pread it would be clearer that we actually only
wait for oustanding gpu writes.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: Explain the set_to_gtt_domain in pwrite/pread and volunteer
Ben to replace those with wait_rendering calls.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Eviction code, like the rest of the converted code needs to be aware of
the address space for which it is evicting (or the everything case, all
addresses). With the updated bind/unbind interfaces of the last patch,
we can now safely move the eviction code over.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
As alluded to in several patches, and it will be reiterated later... A
VMA is an abstraction for a GEM BO bound into an address space.
Therefore it stands to reason, that the existing bind, and unbind are
the ones which will be the most impacted. This patch implements this,
and updates all callers which weren't already updated in the series
(because it was too messy).
This patch represents the bulk of an earlier, larger patch. I've pulled
out a bunch of things by the request of Daniel. The history is preserved
for posterity with the email convention of ">" One big change from the
original patch aside from a bunch of cropping is I've created an
i915_vma_unbind() function. That is because we always have the VMA
anyway, and doing an extra lookup is useful. There is a caveat, we
retain an i915_gem_object_ggtt_unbind, for the global cases which might
not talk in VMAs.
> drm/i915: plumb VM into object operations
>
> This patch was formerly known as:
> "drm/i915: Create VMAs (part 3) - plumbing"
>
> This patch adds a VM argument, bind/unbind, and the object
> offset/size/color getters/setters. It preserves the old ggtt helper
> functions because things still need, and will continue to need them.
>
> Some code will still need to be ported over after this.
>
> v2: Fix purge to pick an object and unbind all vmas
> This was doable because of the global bound list change.
>
> v3: With the commit to actually pin/unpin pages in place, there is no
> longer a need to check if unbind succeeded before calling put_pages().
> Make put_pages only BUG() after checking pin count.
>
> v4: Rebased on top of the new hangcheck work by Mika
> plumbed eb_destroy also
> Many checkpatch related fixes
>
> v5: Very large rebase
>
> v6:
> Change BUG_ON to WARN_ON (Daniel)
> Rename vm to ggtt in preallocate stolen, since it is always ggtt when
> dealing with stolen memory. (Daniel)
> list_for_each will short-circuit already (Daniel)
> remove superflous space (Daniel)
> Use per object list of vmas (Daniel)
> Make obj_bound_any() use obj_bound for each vm (Ben)
> s/bind_to_gtt/bind_to_vm/ (Ben)
>
> Fixed up the inactive shrinker. As Daniel noticed the code could
> potentially count the same object multiple times. While it's not
> possible in the current case, since 1 object can only ever be bound into
> 1 address space thus far - we may as well try to get something more
> future proof in place now. With a prep patch before this to switch over
> to using the bound list + inactive check, we're now able to carry that
> forward for every address space an object is bound into.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: Rebase on top of the loss of "drm/i915: Cleanup more of VMA
in destroy".]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
In order to do this for all VMs, it's convenient to rework the logic a
bit. This should have no functional impact.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Rather than open-code the teardown of a framebuffer, export the routine
from intel_display.c. This then make intel_fbdev symmetric in its use of
the common intel_framebuffer routines to initialise and clean up the
struct intel_framebuffer. (And new features need only be added in one
location!)
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>
MLC_LLC was never validated for Sandybridge and was superseded by a new
level of cacheing for the GPU in Ivybridge. Update our names to be
consistent with usage, and in the process stop setting the unwanted bit
on Sandybridge.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
[danvet: s/BUG/WARN_ON(1) bikeshed.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We set the mode based on the port, and we already pass the port as an
argument.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
These messages are not really useful since it's very easy to check
which mode is used for each port: The values programmed are based on
the port type, then assigned to the ddi_translations variable.
Currently we use DP mode for ports A-D and FDI mode for port E.
Also, when we add the code to enable/disable PC8+,
intel_prepare_ddi_buffers will be called more often and will eat your
dmesg buffers.
While at it, fix the coding style of the "for" statement above.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
[danvet: Pimp commit message with Paulo's more detailed explanation of
how the ddi translation buffer settings are computed, to answer a
question from Chris.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The code itself is no longer accurate without updating once we have
multiple address space since clearing the domains of every object
requires scanning the inactive list for all VMs.
"This code is dead. Just remove it rather than port it to vma." - Chris
Wilson
Recommended-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
All the ILK+ WM compute functions take the latency values in 0.1us
units. Add a few comments to remind people about that.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Adjust the current ILK/SNB/IVB watermark codepaths to use the
pre-populated latency values from dev_priv instead of reading
them out from the registers every time.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Return UINT_MAX for the calculated WM level if the latency is zero.
This will lead to marking the WM level as disabled.
I'm not sure if latency==0 should mean that we want to disable the
level. But that's the implication I got from the fact that we don't
even enable the watermark code of the SSKDP register is 0.
v2: Use WARN() to scare people
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Seeing the watermark latency values in dmesg might help sometimes.
v2: Use DRM_ERROR() when expected latency values are missing
Note: We might hit the DRM_ERROR added in this patch and apparently
there's not much we can do about that. But I think it'd be interesting
to figure out whether that actually happens in the real world, so I
didn't apply a s/DRM_ERROR/DRM_DEBUG_KMS/ bikeshed while applying.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
[danvet: Add note about new error dmesg output.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Rather than pass around the plane latencies, just grab them from
dev_priv nearer to where they're needed. Do the same for cursor
latencies.
v2: Add some comments about latency units
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
