Finally, add some debugging output for ddb changes in the atomic debug
output. This makes it a lot easier to spot bugs from incorrect ddb
allocations.
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1476480722-13015-7-git-send-email-cpaul@redhat.com
Now that we've make skl_wm_levels make a little more sense, we can
remove all of the redundant wm information. Up until now we'd been
storing two copies of all of the skl watermarks: one being the
skl_pipe_wm structs, the other being the global wm struct in
drm_i915_private containing the raw register values. This is confusing
and problematic, since it means we're prone to accidentally letting the
two copies go out of sync. So, get rid of all of the functions
responsible for computing the register values and just use a single
helper, skl_write_wm_level(), to convert and write the new watermarks on
the fly.
Changes since v1:
- Fixup skl_write_wm_level()
- Fixup skl_wm_level_from_reg_val()
- Don't forget to copy *active to intel_crtc->wm.active.skl
Changes since v2:
- Fix usage of wrong cstate
Changes since v3 (by Paulo):
- Rebase
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (v2)
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Lyude <cpaul@redhat.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1476814189-6062-1-git-send-email-paulo.r.zanoni@intel.com
We used to call skl_pipe_pixel_rate(), which used to be a single
one-line return, but now we're calling ilk_pipe_pixel_rate() which is
not as simple, so it's better to just call it once and store the
computed value for reuse.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1475872138-16194-2-git-send-email-paulo.r.zanoni@intel.com
This function is a wreck, let's help it get its life back together and
cleanup all of the copy pasta here.
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Having skl_wm_level contain all of the watermarks for each plane is
annoying since it prevents us from having any sort of object to
represent a single watermark level, something we take advantage of in
the next commit to cut down on all of the copy paste code in here.
Changes since v1:
- Style nitpicks
- Fix accidental usage of i vs. PLANE_CURSOR
- Split out skl_pipe_wm_active_state simplification into separate patch
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Next part of cleaning up the watermark code for skl. This is easy, since
it seems that we never actually needed to keep track of the linetime in
the skl_wm_values struct anyway.
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
First part of cleaning up all of the skl watermark code. This moves the
structures for storing the ddb allocations of each pipe into
intel_crtc_state, along with moving the structures for storing the
current ddb allocations active on hardware into intel_crtc.
Changes since v1:
- Don't replace alloc->start = alloc->end = 0;
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Saves 944 bytes of .rodata strings and 128 bytes of .text.
v2: Add parantheses around dev_priv. (Ville Syrjala)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Jani Nikula <jani.nikula@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Saves 864 bytes of .rodata strings and ~100 of .text.
v2: Add parantheses around dev_priv. (Ville Syrjala)
v3: Rebase.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Jani Nikula <jani.nikula@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
This saves 3248 bytes of .rodata strings.
v2: Add parantheses around dev_priv. (Ville Syrjala)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Jani Nikula <jani.nikula@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
With the possibility of addition of many more number of rings in future,
the drm_i915_private structure could bloat as an array, of type
intel_engine_cs, is embedded inside it.
struct intel_engine_cs engine[I915_NUM_ENGINES];
Though this is still fine as generally there is only a single instance of
drm_i915_private structure used, but not all of the possible rings would be
enabled or active on most of the platforms. Some memory can be saved by
allocating intel_engine_cs structure only for the enabled/active engines.
Currently the engine/ring ID is kept static and dev_priv->engine[] is simply
indexed using the enums defined in intel_engine_id.
To save memory and continue using the static engine/ring IDs, 'engine' is
defined as an array of pointers.
struct intel_engine_cs *engine[I915_NUM_ENGINES];
dev_priv->engine[engine_ID] will be NULL for disabled engine instances.
There is a text size reduction of 928 bytes, from 1028200 to 1027272, for
i915.o file (but for i915.ko file text size remain same as 1193131 bytes).
v2:
- Remove the engine iterator field added in drm_i915_private structure,
instead pass a local iterator variable to the for_each_engine**
macros. (Chris)
- Do away with intel_engine_initialized() and instead directly use the
NULL pointer check on engine pointer. (Chris)
v3:
- Remove for_each_engine_id() macro, as the updated macro for_each_engine()
can be used in place of it. (Chris)
- Protect the access to Render engine Fault register with a NULL check, as
engine specific init is done later in Driver load sequence.
v4:
- Use !!dev_priv->engine[VCS] style for the engine check in getparam. (Chris)
- Kill the superfluous init_engine_lists().
v5:
- Cleanup the intel_engines_init() & intel_engines_setup(), with respect to
allocation of intel_engine_cs structure. (Chris)
v6:
- Rebase.
v7:
- Optimize the for_each_engine_masked() macro. (Chris)
- Change the type of 'iter' local variable to enum intel_engine_id. (Chris)
- Rebase.
v8: Rebase.
v9: Rebase.
v10:
- For index calculation use engine ID instead of pointer based arithmetic in
intel_engine_sync_index() as engine pointers are not contiguous now (Chris)
- For appropriateness, rename local enum variable 'iter' to 'id'. (Joonas)
- Use for_each_engine macro for cleanup in intel_engines_init() and remove
check for NULL engine pointer in cleanup() routines. (Joonas)
v11: Rebase.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Akash Goel <akash.goel@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1476378888-7372-1-git-send-email-akash.goel@intel.com
Use types of more appropriate size in struct
intel_watermark_params to save 512 bytes of .rodata.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
unsigned long is too wide - use smaller types in
struct cxsr_latency to save 800-something bytes of .rodata.
v2: All data even fits in u16 for even more saving. (Ville Syrjala)
v3: Move bitfields to the end of the struct. (Joonas Lahtinen)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
We were previously adding all the planes owned by the CRTC even when
the ddb partitioning didn't change for them. As a consequence, a lot
of functions were being called when we were just moving the cursor
around the screen, such as skylake_update_primary_plane().
This was causing flickering on the primary plane when moving the
cursor. I'm not 100% sure which operation caused the flickering, but
we were writing to a lot of registers, so it could be any of these
writes. With this patch, just moving the mouse won't add the primary
plane to the commit since it won't trigger a change in DDB
partitioning.
v2: Use skl_ddb_entry_equal() (Lyude).
v3: Change Reported-and-bisected-by: to Reported-by: for checkpatch
Fixes: 05a76d3d6a ("drm/i915/skl: Ensure pipes with changed wms get added to the state")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97888
Cc: Mike Lothian <mike@fireburn.co.uk>
Cc: stable@vger.kernel.org
Reported-by: Mike Lothian <mike@fireburn.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Lyude <cpaul@redhat.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1475177808-29955-1-git-send-email-paulo.r.zanoni@intel.com
(cherry picked from commit 7f60e200e2)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
We forgot the "res_blocks += y_tile_minimum" that's described on step
V of our documentation.
Again, this should only affect the Y tiling cases.
It looks like the relevant code was introduced in 0fda65680e, but
there's always the possibility that it matched our specification when
it was introduced, and then the specification changed while the code
stayed the same. So we can't really say this was a regression, but
let's try to add a "Fixes" tag anyway to help backporting.
v2: Try to add a "Fixes" tag (Maarten).
Fixes: 0fda65680e ("drm/i915/skl: Update watermarks for Y tiling")
Cc: stable@vger.kernel.org
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-8-git-send-email-paulo.r.zanoni@intel.com
(cherry picked from commit 75676ed423)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
The confusing thing is that plane_blocks_per_line is listed as part of
the method 2 calculation but is also used for other things. We
calculated it in two different places and different ways: one inside
skl_wm_method2() and the other inside skl_compute_plane_wm(). The
skl_wm_method2() implementation is the one that matches the
specification.
With this patch we fix the skl_compute_plane_wm() calculation and just
pass it as a parameter to skl_wm_method2(). We also take care to not
modify the value of plane_bytes_per_line since we're going to rely on
it having a correct value in later patches.
This should affect the watermarks for Linear and Y-tiled.
From my analysis, it looks like the two plane_blocks_per_line
variables got out of sync on 0fda65680e, but we can't really say
that commit was a regression, it looks like just an incomplete fix.
There's always the possibility that 0fda65680e matched our
specification at that time, and then later the specification changed.
v2: Try to add a "Fixes" tag (Maarten).
Fixes: 0fda65680e ("drm/i915/skl: Update watermarks for Y tiling")
Cc: stable@vger.kernel.org
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-7-git-send-email-paulo.r.zanoni@intel.com
(cherry picked from commit 7a1a8aed67)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
During watermarks calculations, this value is used in 3 different
places. Only one of them was not using a hardcoded 4. Move the code up
so everybody can benefit from the actual value.
This should only help on situations with Y tiling + 90/270 rotation +
1 or 2 bpp or NV12.
Cc: stable@vger.kernel.org
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-6-git-send-email-paulo.r.zanoni@intel.com
(cherry picked from commit 1186fa85eb)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Bspec says:
"The mailbox response data may not account for memory read latency.
If the mailbox response data for level 0 is 0us, add 2 microseconds
to the result for each valid level."
This means we should only do the +2 in case wm[0] == 0, not always.
So split the sanitizing implementation from the WA implementation and
fix the WA implementation.
v2: Add Fixes tag (Maarten).
Fixes: 367294be7c ("drm/i915/gen9: Add 2us read latency to WM level")
Cc: stable@vger.kernel.org
Cc: Vandana Kannan <vandana.kannan@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-5-git-send-email-paulo.r.zanoni@intel.com
(cherry picked from commit 0727e40a48)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
According to BSpec, it's the "core CPUs" that need the code, which
means SKL and KBL, but not BXT.
I don't have a KBL to test this patch on it.
v2: Only SKL should have I915_SAGV_NOT_CONTROLLED.
Cc: stable@vger.kernel.org
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-4-git-send-email-paulo.r.zanoni@intel.com
(cherry picked from commit 6e3100ec21)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
And use it to move knowledge about the SAGV-supporting platforms from
the callers to the SAGV code.
We'll add more platforms to intel_has_sagv(), so IMHO it makes more
sense to move all this to a single function instead of patching all
the callers every time we add SAGV support to a new platform.
v2: Move I915_SAGV_NOT_CONTROLLED to the new function (Lyude).
Cc: stable@vger.kernel.org
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-3-git-send-email-paulo.r.zanoni@intel.com
(cherry picked from commit 56feca9197)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
The plan is to introduce intel_has_sagv() and then use it to discover
which platforms actually support it.
I thought about keeping the functions with their current skl names,
but found two problems: (i) skl_has_sagv() would become a very
confusing name, and (ii) intel_atomic_commit_tail() doesn't seem to be
calling any functions whose name start with a platform name, so the
"intel_" naming scheme seems make more sense than the "firstplatorm_"
naming scheme here.
Cc: stable@vger.kernel.org
Reviewed-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-2-git-send-email-paulo.r.zanoni@intel.com
(cherry picked from commit 16dcdc4edb)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Fix sparse warning:
drivers/gpu/drm/i915/intel_pm.c:3970:1: warning: symbol
'skl_ddb_add_affected_planes' was not declared. Should it be static?
Fixes: 7f60e200e2 ("drm/i915/gen9: only add the planes actually affected by ddb changes")
Cc: Lyude <cpaul@redhat.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1475573357-30562-1-git-send-email-jani.nikula@intel.com
We were previously adding all the planes owned by the CRTC even when
the ddb partitioning didn't change for them. As a consequence, a lot
of functions were being called when we were just moving the cursor
around the screen, such as skylake_update_primary_plane().
This was causing flickering on the primary plane when moving the
cursor. I'm not 100% sure which operation caused the flickering, but
we were writing to a lot of registers, so it could be any of these
writes. With this patch, just moving the mouse won't add the primary
plane to the commit since it won't trigger a change in DDB
partitioning.
v2: Use skl_ddb_entry_equal() (Lyude).
v3: Change Reported-and-bisected-by: to Reported-by: for checkpatch
Fixes: 05a76d3d6a ("drm/i915/skl: Ensure pipes with changed wms get added to the state")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97888
Cc: Mike Lothian <mike@fireburn.co.uk>
Cc: stable@vger.kernel.org
Reported-by: Mike Lothian <mike@fireburn.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Lyude <cpaul@redhat.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1475177808-29955-1-git-send-email-paulo.r.zanoni@intel.com
Now that this code is part of the compute stage we can return -EINVAL
to prevent the modeset instead of giving a WARN and trying anyway.
v2:
- Fix typo (Paul Menzel).
- Add MISSING_CASE() (Ville, Maarten).
Reported-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-10-git-send-email-paulo.r.zanoni@intel.com
This should affect linear and X tiled planes on really small htotal
cases. It doesn't seem to be a very feasible case, but let's implement
it since it's on the specification and it's better to have it and
never need than not have it and realize we needed it.
Reviewed-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-9-git-send-email-paulo.r.zanoni@intel.com
We forgot the "res_blocks += y_tile_minimum" that's described on step
V of our documentation.
Again, this should only affect the Y tiling cases.
It looks like the relevant code was introduced in 0fda65680e, but
there's always the possibility that it matched our specification when
it was introduced, and then the specification changed while the code
stayed the same. So we can't really say this was a regression, but
let's try to add a "Fixes" tag anyway to help backporting.
v2: Try to add a "Fixes" tag (Maarten).
Fixes: 0fda65680e ("drm/i915/skl: Update watermarks for Y tiling")
Cc: stable@vger.kernel.org
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-8-git-send-email-paulo.r.zanoni@intel.com
The confusing thing is that plane_blocks_per_line is listed as part of
the method 2 calculation but is also used for other things. We
calculated it in two different places and different ways: one inside
skl_wm_method2() and the other inside skl_compute_plane_wm(). The
skl_wm_method2() implementation is the one that matches the
specification.
With this patch we fix the skl_compute_plane_wm() calculation and just
pass it as a parameter to skl_wm_method2(). We also take care to not
modify the value of plane_bytes_per_line since we're going to rely on
it having a correct value in later patches.
This should affect the watermarks for Linear and Y-tiled.
From my analysis, it looks like the two plane_blocks_per_line
variables got out of sync on 0fda65680e, but we can't really say
that commit was a regression, it looks like just an incomplete fix.
There's always the possibility that 0fda65680e matched our
specification at that time, and then later the specification changed.
v2: Try to add a "Fixes" tag (Maarten).
Fixes: 0fda65680e ("drm/i915/skl: Update watermarks for Y tiling")
Cc: stable@vger.kernel.org
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-7-git-send-email-paulo.r.zanoni@intel.com
During watermarks calculations, this value is used in 3 different
places. Only one of them was not using a hardcoded 4. Move the code up
so everybody can benefit from the actual value.
This should only help on situations with Y tiling + 90/270 rotation +
1 or 2 bpp or NV12.
Cc: stable@vger.kernel.org
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-6-git-send-email-paulo.r.zanoni@intel.com
Bspec says:
"The mailbox response data may not account for memory read latency.
If the mailbox response data for level 0 is 0us, add 2 microseconds
to the result for each valid level."
This means we should only do the +2 in case wm[0] == 0, not always.
So split the sanitizing implementation from the WA implementation and
fix the WA implementation.
v2: Add Fixes tag (Maarten).
Fixes: 367294be7c ("drm/i915/gen9: Add 2us read latency to WM level")
Cc: stable@vger.kernel.org
Cc: Vandana Kannan <vandana.kannan@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-5-git-send-email-paulo.r.zanoni@intel.com
And use it to move knowledge about the SAGV-supporting platforms from
the callers to the SAGV code.
We'll add more platforms to intel_has_sagv(), so IMHO it makes more
sense to move all this to a single function instead of patching all
the callers every time we add SAGV support to a new platform.
v2: Move I915_SAGV_NOT_CONTROLLED to the new function (Lyude).
Cc: stable@vger.kernel.org
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-3-git-send-email-paulo.r.zanoni@intel.com
The plan is to introduce intel_has_sagv() and then use it to discover
which platforms actually support it.
I thought about keeping the functions with their current skl names,
but found two problems: (i) skl_has_sagv() would become a very
confusing name, and (ii) intel_atomic_commit_tail() doesn't seem to be
calling any functions whose name start with a platform name, so the
"intel_" naming scheme seems make more sense than the "firstplatorm_"
naming scheme here.
Cc: stable@vger.kernel.org
Reviewed-by: Lyude <cpaul@redhat.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1474578035-424-2-git-send-email-paulo.r.zanoni@intel.com
Adding the ddb size into the devide info will avoid
platform checks while computing wm.
v2: Added comment and WARN_ON if ddb size is zero.(Jani)
v3: Added WARN_ON at the right place.(Jani)
Suggested-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Signed-off-by: Deepak M <m.deepak@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1473931870-7724-1-git-send-email-m.deepak@intel.com
Access to intel_init_emon() is strictly ordered by gt_powersave, using
struct_mutex around it is overkill (and will conflict with the caller
holding struct_mutex themselves).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20160909131201.16673-8-chris@chris-wilson.co.uk
Move all slice/subslice/eu related properties to the sseu_dev_info
struct.
No functional change.
v2:
- s/info/sseu/ based on the new struct name. (Ben)
Reviewed-by: Robert Bragg <robert@sixbynine.org> (v1)
Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com> (v1)
Tested-by: Ben Widawsky <benjamin.widawsky@intel.com> (v1)
Signed-off-by: Imre Deak <imre.deak@intel.com>
Now that we can hook into update_crtcs and control the order in which we
update CRTCs at each modeset, we can finish the final step of fixing
Skylake's watermark handling by performing DDB updates at the same time
as plane updates and watermark updates.
The first major change in this patch is skl_update_crtcs(), which
handles ensuring that we order each CRTC update in our atomic commits
properly so that they honor the DDB flush order.
The second major change in this patch is the order in which we flush the
pipes. While the previous order may have worked, it can't be used in
this approach since it no longer will do the right thing. For example,
using the old ddb flush order:
We have pipes A, B, and C enabled, and we're disabling C. Initial ddb
allocation looks like this:
| A | B |xxxxxxx|
Since we're performing the ddb updates after performing any CRTC
disablements in intel_atomic_commit_tail(), the space to the right of
pipe B is unallocated.
1. Flush pipes with new allocation contained into old space. None
apply, so we skip this
2. Flush pipes having their allocation reduced, but overlapping with a
previous allocation. None apply, so we also skip this
3. Flush pipes that got more space allocated. This applies to A and B,
giving us the following update order: A, B
This is wrong, since updating pipe A first will cause it to overlap with
B and potentially burst into flames. Our new order (see the code
comments for details) would update the pipes in the proper order: B, A.
As well, we calculate the order for each DDB update during the check
phase, and reference it later in the commit phase when we hit
skl_update_crtcs().
This long overdue patch fixes the rest of the underruns on Skylake.
Changes since v1:
- Add skl_ddb_entry_write() for cursor into skl_write_cursor_wm()
Changes since v2:
- Use the method for updating CRTCs that Ville suggested
- In skl_update_wm(), only copy the watermarks for the crtc that was
passed to us
Changes since v3:
- Small comment fix in skl_ddb_allocation_overlaps()
Changes since v4:
- Remove the second loop in intel_update_crtcs() and use Ville's
suggestion for updating the ddb allocations in the right order
- Get rid of the second loop and just use the ddb state as it updates
to determine what order to update everything in (thanks for the
suggestion Ville)
- Simplify skl_ddb_allocation_overlaps()
- Split actual overlap checking into it's own helper
Fixes: 0e8fb7ba7c ("drm/i915/skl: Flush the WM configuration")
Fixes: 8211bd5bdf ("drm/i915/skl: Program the DDB allocation")
[omitting CC for stable, since this patch will need to be changed for
such backports first]
Testcase: kms_cursor_legacy
Testcase: plane-all-modeset-transition
Signed-off-by: Lyude <cpaul@redhat.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1471961565-28540-2-git-send-email-cpaul@redhat.com
Thanks to Ville for suggesting this as a potential solution to pipe
underruns on Skylake.
On Skylake all of the registers for configuring planes, including the
registers for configuring their watermarks, are double buffered. New
values written to them won't take effect until said registers are
"armed", which is done by writing to the PLANE_SURF (or in the case of
cursor planes, the CURBASE register) register.
With this in mind, up until now we've been updating watermarks on skl
like this:
non-modeset {
- calculate (during atomic check phase)
- finish_atomic_commit:
- intel_pre_plane_update:
- intel_update_watermarks()
- {vblank happens; new watermarks + old plane values => underrun }
- drm_atomic_helper_commit_planes_on_crtc:
- start vblank evasion
- write new plane registers
- end vblank evasion
}
or
modeset {
- calculate (during atomic check phase)
- finish_atomic_commit:
- crtc_enable:
- intel_update_watermarks()
- {vblank happens; new watermarks + old plane values => underrun }
- drm_atomic_helper_commit_planes_on_crtc:
- start vblank evasion
- write new plane registers
- end vblank evasion
}
Now we update watermarks atomically like this:
non-modeset {
- calculate (during atomic check phase)
- finish_atomic_commit:
- intel_pre_plane_update:
- intel_update_watermarks() (wm values aren't written yet)
- drm_atomic_helper_commit_planes_on_crtc:
- start vblank evasion
- write new plane registers
- write new wm values
- end vblank evasion
}
modeset {
- calculate (during atomic check phase)
- finish_atomic_commit:
- crtc_enable:
- intel_update_watermarks() (actual wm values aren't written
yet)
- drm_atomic_helper_commit_planes_on_crtc:
- start vblank evasion
- write new plane registers
- write new wm values
- end vblank evasion
}
So this patch moves all of the watermark writes into the right place;
inside of the vblank evasion where we update all of the registers for
each plane. While this patch doesn't fix everything, it does allow us to
update the watermark values in the way the hardware expects us to.
Changes since original patch series:
- Remove mutex_lock/mutex_unlock since they don't do anything and we're
not touching global state
- Move skl_write_cursor_wm/skl_write_plane_wm functions into
intel_pm.c, make externally visible
- Add skl_write_plane_wm calls to skl_update_plane
- Fix conditional for for loop in skl_write_plane_wm (level < max_level
should be level <= max_level)
- Make diagram in commit more accurate to what's actually happening
- Add Fixes:
Changes since v1:
- Use IS_GEN9() instead of IS_SKYLAKE() since these fixes apply to more
then just Skylake
- Update description to make it clear this patch doesn't fix everything
- Check if pipes were actually changed before writing watermarks
Changes since v2:
- Write PIPE_WM_LINETIME during vblank evasion
Changes since v3:
- Rebase against new SAGV patch changes
Changes since v4:
- Add a parameter to choose what skl_wm_values struct to use when
writing new plane watermarks
Changes since v5:
- Remove cursor ddb entry write in skl_write_cursor_wm(), defer until
patch 6
- Write WM_LINETIME in intel_begin_crtc_commit()
Changes since v6:
- Remove redundant dirty_pipes check in skl_write_plane_wm (we check
this in all places where we call this function, and it was supposed
to have been removed earlier anyway)
- In i9xx_update_cursor(), use dev_priv->info.gen >= 9 instead of
IS_GEN9(dev_priv). We do this everywhere else and I'd imagine this
needs to be done for gen10 as well
Changes since v7:
- Fix rebase fail (unused variable obj)
- Make struct skl_wm_values *wm const
- Fix indenting
- Use INTEL_GEN() instead of dev_priv->info.gen
Changes since v8:
- Don't forget calls to skl_write_plane_wm() when disabling planes
- Use INTEL_GEN(), not INTEL_INFO()->gen in intel_begin_crtc_commit()
Fixes: 2d41c0b59a ("drm/i915/skl: SKL Watermark Computation")
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1471884608-10671-1-git-send-email-cpaul@redhat.com
Link: http://patchwork.freedesktop.org/patch/msgid/1471884608-10671-1-git-send-email-cpaul@redhat.com
If we're enabling a pipe, we'll need to modify the watermarks on all
active planes. Since those planes won't be added to the state on
their own, we need to add them ourselves.
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-6-git-send-email-cpaul@redhat.com
When we write watermark values to the hardware, those values are stored
in dev_priv->wm.skl_hw. However with recent watermark changes, the
results structure we're copying from only contains valid watermark and
DDB values for the pipes that are actually changing; the values for
other pipes remain 0. Thus a blind copy of the entire skl_wm_values
structure will clobber the values for unchanged pipes...we need to be
more selective and only copy over the values for the changing pipes.
This mistake was hidden until recently due to another bug that caused us
to erroneously re-calculate watermarks for all active pipes rather than
changing pipes. Only when that bug was fixed was the impact of this bug
discovered (e.g., modesets failing with "Requested display configuration
exceeds system watermark limitations" messages and leaving watermarks
non-functional, even ones initiated by intel_fbdev_restore_mode).
Changes since v1:
- Add a function for copying a pipe's wm values
(skl_copy_wm_for_pipe()) so we can reuse this later
Fixes: 734fa01f3a ("drm/i915/gen9: Calculate watermarks during atomic 'check' (v2)")
Fixes: 9b61302274 ("drm/i915/gen9: Re-allocate DDB only for changed pipes")
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Lyude <cpaul@redhat.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Cc: stable@vger.kernel.org
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-4-git-send-email-cpaul@redhat.com
Since the watermark calculations for Skylake are still broken, we're apt
to hitting underruns very easily under multi-monitor configurations.
While it would be lovely if this was fixed, it's not. Another problem
that's been coming from this however, is the mysterious issue of
underruns causing full system hangs. An easy way to reproduce this with
a skylake system:
- Get a laptop with a skylake GPU, and hook up two external monitors to
it
- Move the cursor from the built-in LCD to one of the external displays
as quickly as you can
- You'll get a few pipe underruns, and eventually the entire system will
just freeze.
After doing a lot of investigation and reading through the bspec, I
found the existence of the SAGV, which is responsible for adjusting the
system agent voltage and clock frequencies depending on how much power
we need. According to the bspec:
"The display engine access to system memory is blocked during the
adjustment time. SAGV defaults to enabled. Software must use the
GT-driver pcode mailbox to disable SAGV when the display engine is not
able to tolerate the blocking time."
The rest of the bspec goes on to explain that software can simply leave
the SAGV enabled, and disable it when we use interlaced pipes/have more
then one pipe active.
Sure enough, with this patchset the system hangs resulting from pipe
underruns on Skylake have completely vanished on my T460s. Additionally,
the bspec mentions turning off the SAGV with more then one pipe enabled
as a workaround for display underruns. While this patch doesn't entirely
fix that, it looks like it does improve the situation a little bit so
it's likely this is going to be required to make watermarks on Skylake
fully functional.
This will still need additional work in the future: we shouldn't be
enabling the SAGV if any of the currently enabled planes can't enable WM
levels that introduce latencies >= 30 µs.
Changes since v11:
- Add skl_can_enable_sagv()
- Make sure we don't enable SAGV when not all planes can enable
watermarks >= the SAGV engine block time. I was originally going to
save this for later, but I recently managed to run into a machine
that was having problems with a single pipe configuration + SAGV.
- Make comparisons to I915_SKL_SAGV_NOT_CONTROLLED explicit
- Change I915_SAGV_DYNAMIC_FREQ to I915_SAGV_ENABLE
- Move printks outside of mutexes
- Don't print error messages twice
Changes since v10:
- Apparently sandybridge_pcode_read actually writes values and reads
them back, despite it's misleading function name. This means we've
been doing this mostly wrong and have been writing garbage to the
SAGV control. Because of this, we no longer attempt to read the SAGV
status during initialization (since there are no helpers for this).
- mlankhorst noticed that this patch was breaking on some very early
pre-release Skylake machines, which apparently don't allow you to
disable the SAGV. To prevent machines from failing tests due to SAGV
errors, if the first time we try to control the SAGV results in the
mailbox indicating an invalid command, we just disable future attempts
to control the SAGV state by setting dev_priv->skl_sagv_status to
I915_SKL_SAGV_NOT_CONTROLLED and make a note of it in dmesg.
- Move mutex_unlock() a little higher in skl_enable_sagv(). This
doesn't actually fix anything, but lets us release the lock a little
sooner since we're finished with it.
Changes since v9:
- Only enable/disable sagv on Skylake
Changes since v8:
- Add intel_state->modeset guard to the conditional for
skl_enable_sagv()
Changes since v7:
- Remove GEN9_SAGV_LOW_FREQ, replace with GEN9_SAGV_IS_ENABLED (that's
all we use it for anyway)
- Use GEN9_SAGV_IS_ENABLED instead of 0x1 for clarification
- Fix a styling error that snuck past me
Changes since v6:
- Protect skl_enable_sagv() with intel_state->modeset conditional in
intel_atomic_commit_tail()
Changes since v5:
- Don't use is_power_of_2. Makes things confusing
- Don't use the old state to figure out whether or not to
enable/disable the sagv, use the new one
- Split the loop in skl_disable_sagv into it's own function
- Move skl_sagv_enable/disable() calls into intel_atomic_commit_tail()
Changes since v4:
- Use is_power_of_2 against active_crtcs to check whether we have > 1
pipe enabled
- Fix skl_sagv_get_hw_state(): (temp & 0x1) indicates disabled, 0x0
enabled
- Call skl_sagv_enable/disable() from pre/post-plane updates
Changes since v3:
- Use time_before() to compare timeout to jiffies
Changes since v2:
- Really apply minor style nitpicks to patch this time
Changes since v1:
- Added comments about this probably being one of the requirements to
fixing Skylake's watermark issues
- Minor style nitpicks from Matt Roper
- Disable these functions on Broxton, since it doesn't have an SAGV
Signed-off-by: Lyude <cpaul@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-3-git-send-email-cpaul@redhat.com
[mlankhorst: ENOSYS -> ENXIO, whitespace fixes]
In order to add proper support for the SAGV, we need to be able to know
what the cause of a failure to change the SAGV through the pcode mailbox
was. The reasoning for this is that some very early pre-release Skylake
machines don't actually allow you to control the SAGV on them, and
indicate an invalid mailbox command was sent.
This also might come in handy in the future for debugging.
Changes since v1:
- Add functions for interpreting gen6 mailbox error codes along with
gen7+ error codes, and actually interpret those codes properly
- Renamed patch to reflect new behavior
Signed-off-by: Lyude <cpaul@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-2-git-send-email-cpaul@redhat.com
[mlankhorst: -ENOSYS -> -ENXIO for checkpatch]
It's possible to have a non-zero plane mask and still wind up with a
total data rate of zero. There are two cases where this can happen:
* planes are active (from the KMS point of view), but are
all fully clipped (positioned offscreen)
* the only active plane on a CRTC is the cursor (which is handled
independently and not counted into the general data rate computations
These are both valid display setups (although unusual), so we need to
drop the WARN().
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Testcase: kms_universal_planes.cursor-only-pipe-*
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1466196140-16336-4-git-send-email-matthew.d.roper@intel.com
Cc: stable@vger.kernel.org #v4.7+
intel_state->active_crtcs is usually only initialized when doing a
modeset. During our first atomic commit after boot, we're effectively
faking a modeset to sanitize the DDB/wm setup, so ensure that this field
gets initialized before use.
v2:
- Don't clobber active_crtcs if our first commit really is a modeset
(Maarten)
- Grab connection_mutex when faking a modeset during sanitization
(Maarten)
Reported-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Tested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1466196140-16336-2-git-send-email-matthew.d.roper@intel.com
Cc: stable@vger.kernel.org #v4.7+
Backmerge because too many conflicts, and also we need to get at the
latest struct fence patches from Gustavo. Requested by Chris Wilson.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
- refactor ddi buffer programming a bit (Ville)
- large-scale renaming to untangle naming in the gem code (Chris)
- rework vma/active tracking for accurately reaping idle mappings of shared
objects (Chris)
- misc dp sst/mst probing corner case fixes (Ville)
- tons of cleanup&tunings all around in gem
- lockless (rcu-protected) request lookup, plus use it everywhere for
non(b)locking waits (Chris)
- pipe crc debugfs fixes (Rodrigo)
- random fixes all over
* tag 'drm-intel-next-2016-08-08' of git://anongit.freedesktop.org/drm-intel: (222 commits)
drm/i915: Update DRIVER_DATE to 20160808
drm/i915: fix aliasing_ppgtt leak
drm/i915: Update comment before i915_spin_request
drm/i915: Use drm official vblank_no_hw_counter callback.
drm/i915: Fix copy_to_user usage for pipe_crc
Revert "drm/i915: Track active streams also for DP SST"
drm/i915: fix WaInsertDummyPushConstPs
drm/i915: Assert that the request hasn't been retired
drm/i915: Repack fence tiling mode and stride into a single integer
drm/i915: Document and reject invalid tiling modes
drm/i915: Remove locking for get_tiling
drm/i915: Remove pinned check from madvise ioctl
drm/i915: Reduce locking inside swfinish ioctl
drm/i915: Remove (struct_mutex) locking for busy-ioctl
drm/i915: Remove (struct_mutex) locking for wait-ioctl
drm/i915: Do a nonblocking wait first in pread/pwrite
drm/i915: Remove unused no-shrinker-steal
drm/i915: Tidy generation of the GTT mmap offset
drm/i915/shrinker: Wait before acquiring struct_mutex under oom
drm/i915: Simplify do_idling() (Ironlake vt-d w/a)
...
- more fence destaging and cleanup (Gustavo&Sumit)
- DRIVER_LEGACY to untangle from DRIVER_MODESET
- drm_mm refactor (Chris)
- fbdev-less compile fies
- clipped plane src/dst rects (Ville)
- + a few mediatek patches that build on top of that (Bibby+Daniel)
- small stuff all over really
* tag 'topic/drm-misc-2016-08-12' of git://anongit.freedesktop.org/drm-intel: (43 commits)
dma-buf/fence: kerneldoc: remove spurious section header
dma-buf/fence: kerneldoc: remove unused struct members
Revert "gpu: drm: omapdrm: dss-of: add missing of_node_put after calling of_parse_phandle"
drm: Protect fb_defio in drivers with CONFIG_KMS_FBDEV_EMULATION
drm/radeon|amgpu: Make fbdev emulation optional
drm/vmwgfx: select CONFIG_FB
drm: Remove superflous linux/fb.h includes
drm/fb-helper: Add a dummy remove_conflicting_framebuffers
dma-buf/sync_file: only enable fence signalling on poll()
Documentation: add doc for sync_file_get_fence()
dma-buf/sync_file: add sync_file_get_fence()
dma-buf/sync_file: refactor fence storage in struct sync_file
dma-buf/fence-array: add fence_is_array()
drm/dp_helper: Rate limit timeout errors from drm_dp_i2c_do_msg()
drm/dp_helper: Print first error received on failure in drm_dp_dpcd_access()
drm: Add ratelimited versions of the DRM_DEBUG* macros
drm: Make sure drm_vblank_no_hw_counter isn't abused
drm/mediatek: Fix mtk_atomic_complete for runtime_pm
drm/mediatek: plane: Use FB's format's cpp to compute x offset
drm/mediatek: plane: Merge mtk_plane_enable into mtk_plane_atomic_update
...
During intel_gt_powersave_init() we take the RPS mutex to ensure that
all locking requirements are met as we talk to the punit, but we also
require the struct_mutex for allocating a slice of the global GTT for a
power context on Valleyview. struct_mutex must be the outer lock here,
as we nest rps.mutex inside later on.
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Fixes: 773ea9a801 ("drm/i915: Perform static RPS frequency setup before...")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1470833904-29886-1-git-send-email-chris@chris-wilson.co.uk
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
3 intel fixes.
* tag 'drm-intel-next-fixes-2016-08-05' of git://anongit.freedesktop.org/drm-intel:
drm/i915/fbdev: Check for the framebuffer before use
drm/i915: Never fully mask the the EI up rps interrupt on SNB/IVB
drm/i915: Wait up to 3ms for the pcu to ack the cdclk change request on SKL
In the previous commit, we moved the obj->tiling_mode out of a bitfield
and into its own integer so that we could safely use READ_ONCE(). Let us
now repair some of that damage by sharing the tiling_mode with its
companion, the fence stride.
v2: New magic
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1470388464-28458-18-git-send-email-chris@chris-wilson.co.uk
The principal motivation for this was to try and eliminate the
struct_mutex from i915_gem_suspend - but we still need to hold the mutex
current for the i915_gem_context_lost(). (The issue there is that there
may be an indirect lockdep cycle between cpu_hotplug (i.e. suspend) and
struct_mutex via the stop_machine().) For the moment, enabling last
request tracking for the engine, allows us to do busyness checking and
waiting without requiring the struct_mutex - which is useful in its own
right.
As a side-effect of having a robust means for tracking engine busyness,
we can replace our other busyness heuristic, that of comparing against
the last submitted seqno. For paranoid reasons, we have a semi-ordered
check of that seqno inside the hangchecker, which we can now improve to
an ordered check of the engine's busyness (removing a locked xchg in the
process).
v2: Pass along "bool interruptible" as being unlocked we cannot rely on
i915->mm.interruptible being stable or even under our control.
v3: Replace check Ironlake i915_gpu_busy() with the common precalculated value
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1470388464-28458-6-git-send-email-chris@chris-wilson.co.uk
Backmerge the 4.8 pull request state from Dave - conflicts were
getting out of hand, and Chris has some patches which outright don't
apply without everything merged together again.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
The bspec was updated a couple weeks ago to add an extra block per line
to plane watermark calculations for linear pixel formats.
Bspec update 115327 description:
"Gen9+ - Updated the plane blocks per line calculation for linear
cases. Adds +1 for all linear cases to handle the non-block aligned
stride cases."
Cc: Lyude <cpaul@redhat.com>
Cc: drm-intel-fixes@lists.freedesktop.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1470344880-27394-1-git-send-email-matthew.d.roper@intel.com
Reviewed-by: Lyude <cpaul@redhat.com>
Remove the CHV early bail out from intel_cleanup_gt_powersave() so that
we'll clean up the extra RPM reference held due to i915.enable_rc6=0.
Cc: Imre Deak <imre.deak@intel.com>
Fixes: b268c699ac ("drm/i915: refactor RPM disabling due to RC6 being disabled")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1470136053-23276-1-git-send-email-ville.syrjala@linux.intel.com
Reviewed-by: Imre Deak <imre.deak@intel.com>
According to Bspec FW_BLC_SELF exists on 915G also. Let's program it.
The only open question is whether there's is a memory self-refresh
enable bit somewhere as well. For 945G/GM it's in FW_BLC_SELF, for
915GM it's in INSTPM. For 915G I can't find one in the docs. Let's drop
a FIXME about this, in case someone with the hardware is ever bored
enough to look for it.
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1469804222-12650-2-git-send-email-ville.syrjala@linux.intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Bspec says:
"FW_BLC_SELF
...
Programming Note [DevALV] and [DevCST]: When calculating watermark
values for 15/16bpp, assume 32bpp for purposes of calculation using
the high priority bandwidth analysis spreadsheet."
Let's do that.
Perhaps this might even help with the problem that resulted in
commit 2ab1bc9df0 ("drm/i915: Disable self-refresh for untiled fbs on i915gm")
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1469804222-12650-1-git-send-email-ville.syrjala@linux.intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
In the middle of intel_gt_init_powersave() we have an if-chain that ends
with a universal else clause to read gen6+ registers. Older platforms
like Pineview that end up here do not like those registers and may even
OOPS whilst reading them!
Fixes: 3ea9a80132 ("drm/i915: Perform static RPS frequency setup ...")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1470132927-1821-1-git-send-email-chris@chris-wilson.co.uk
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
SNB (and IVB too I suppose) starts to misbehave if the GPU gets stuck
in an infinite batch buffer loop. The GPU apparently hogs something
critical and CPUs start to lose interrupts and whatnot. We can keep
the system limping along by unmasking some interrupts in
GEN6_PMINTRMSK. The EI up interrupt has been previously chosen for
that task, so let's never mask it.
v2: s/gen6_rps_pm_mask/gen6_sanitize_rps_pm_mask/ (Chris)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93122
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1464014568-4529-1-git-send-email-ville.syrjala@linux.intel.com
Cc: stable@vger.kernel.org
(cherry picked from commit 12c100bfa5)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
SNB (and IVB too I suppose) starts to misbehave if the GPU gets stuck
in an infinite batch buffer loop. The GPU apparently hogs something
critical and CPUs start to lose interrupts and whatnot. We can keep
the system limping along by unmasking some interrupts in
GEN6_PMINTRMSK. The EI up interrupt has been previously chosen for
that task, so let's never mask it.
v2: s/gen6_rps_pm_mask/gen6_sanitize_rps_pm_mask/ (Chris)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93122
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1464014568-4529-1-git-send-email-ville.syrjala@linux.intel.com
Cc: stable@vger.kernel.org
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQEcBAABAgAGBQJXlRXSAAoJEHm+PkMAQRiGG/gH/0Z8O4zWOsrwO+X1mRToRDBH
joFOjAmCVe83T1VpF5LYNB+9+owL/dEDt6+ZIswnhH7AfQPjs4RqwS4PcuMbCDVO
+mDm0PmfcKaYcQZrB2Z2OwIzRNnfCTVcsDPhIHwuIHk0m4z/xuGZonD8KoAj0+tO
3yJF6sbE1KubDVjOb+lmZZSP3cXA0pDXrNhkYhE4Tsr8fiihGjeXSNJ8t2zPLjxo
W3MPqo0rzDvQsOwoF4TWHHagVaFSJlhLBBgqu33fI7uO3jtfQD2G8wG68JCND1j3
qbMoBfTLFV/yQmSIJUt0Wv1axaCcwnjpweEB35A/GEeZ0mNB1rDdoBeI1eKEQkc=
=DGFC
-----END PGP SIGNATURE-----
Backmerge tag 'v4.7' into drm-next
Linux 4.7
As requested by Daniel Vetter as the conflicts were getting messy.
Now that PCU communication is reasonably fast, we do not need to defer
RC6 initialisation to a workqueue.
References: https://bugs.freedesktop.org/show_bug.cgi?id=97017
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Since commit a6f766f397 ("drm/i915: Limit ring synchronisation (sw
sempahores) RPS boosts") and commit bcafc4e38b ("drm/i915: Limit mmio
flip RPS boosts") we have limited the waitboosting for semaphores and
flips. Ideally we do not want to boost in either of these instances as no
userspace consumer is waiting upon the results (though a userspace producer
may be stalled trying to submit an execbuf - but in this case the
producer is being throttled due to the engine being saturated with
work). With the introduction of NO_WAITBOOST in the previous patch, we
can finally disable these needless boosts.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1469002875-2335-6-git-send-email-chris@chris-wilson.co.uk
Bspec states that we need to set nuke on modify all to prevent
screen corruption with fbc on skl and kbl.
v2: proper workaround name
References: HSD#2227109, HSDES#1404569388
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1465309159-30531-27-git-send-email-mika.kuoppala@intel.com
(cherry picked from commit 031cd8c85a)
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Set bit 8 in 0x43224 to prevent screen corruption and system
hangs on high memory bandwidth conditions. The same wa also suggest
setting bit 31 on ARB_CTL. According to another workaround we gain
better idle power savings when FBC is enabled.
v2: use correct workaround name
v3: split out overlapping wa for corruption avoidance (Ville)
References: HSD#2137218, HSD#2227171, HSD#2136579, BSID#883
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1465309159-30531-26-git-send-email-mika.kuoppala@intel.com
(cherry picked from commit 303d4ea522)
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
According to bspec this prevents screen corruption when fbc is
used.
v2: This workaround has a name, use it (Ville)
v3: remove bogus gen check on ilk/vlv wm path (Ville)
References: HSD#2135555, HSD#2137270, BSID#562
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1465309159-30531-25-git-send-email-mika.kuoppala@intel.com
(cherry picked from commit 0f78dee6f0)
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Workaround for display underrun issues with Y & Yf Tiling.
Set this on all gen9 as stated by bspec.
v2: proper workaround name
References: HSD#2136383, BSID#857
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1465309159-30531-22-git-send-email-mika.kuoppala@intel.com
(cherry picked from commit 590e8ff04b)
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Found this while browsing Bspec. Looks like it applies to both skl and
kbl.
v2: Also for bxt (Art).
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Sonika Jindal <sonika.jindal@intel.com>
Cc: Durgadoss R <durgadoss.r@intel.com>
Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Cc: "Runyan, Arthur J" <arthur.j.runyan@intel.com>
Reviewed-by: Sonika Jindal<sonika.jindal@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1463642060-30728-1-git-send-email-daniel.vetter@ffwll.ch
(cherry picked from commit dc00b6a07c)
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
This function is no longer used outside of intel_pm.c so we can stop
exposing it and rename the __gen6_update_ring_freq() to take its place.
Suggested-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1468397438-21226-8-git-send-email-chris@chris-wilson.co.uk
Some hardware requires a valid render context before it can initiate
rc6 power gating of the GPU; the default state of the GPU is not
sufficient and may lead to undefined behaviour. The first execution of
any batch will load the "golden render state", at which point it is safe
to enable rc6. As we do not forcibly load the kernel context at resume,
we have to hook into the batch submission to be sure that the render
state is setup before enabling rc6.
However, since we don't enable powersaving until that first batch, we
queued a delayed task in order to guarantee that the batch is indeed
submitted.
v2: Rearrange intel_disable_gt_powersave() to match.
v3: Apply user specified cur_freq (or idle_freq if not set).
v4: Give in, and supply a delayed work to autoenable rc6
v5: Mika suggested a couple of better names for delayed_resume_work
v6: Rebalance rpm_put around the autoenable task
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1468397438-21226-7-git-send-email-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
To allow the user finer control over waitboosting, allow them to set the
frequency we request for the boost. This also them allows to effectively
disable the boosting by setting the boost request to a low frequency.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1468397438-21226-5-git-send-email-chris@chris-wilson.co.uk
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Select idle frequency during initialisation, then reset the last known
frequency when re-enabling. This allows us to preserve the user selected
frequency across resets.
v2: Stop CHV from overriding the user's choice in cherryview_enable_rps()
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1468397438-21226-2-git-send-email-chris@chris-wilson.co.uk
Since drm_i915_private is now a subclass of drm_device we do not need to
chase the drm_i915_private->dev backpointer and can instead simply
access drm_i915_private->drm directly.
text data bss dec hex filename
1068757 4565 416 1073738 10624a drivers/gpu/drm/i915/i915.ko
1066949 4565 416 1071930 105b3a drivers/gpu/drm/i915/i915.ko
Created by the coccinelle script:
@@
struct drm_i915_private *d;
identifier i;
@@
(
- d->dev->i
+ d->drm.i
|
- d->dev
+ &d->drm
)
and for good measure the dev_priv->dev backpointer was removed entirely.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467711623-2905-4-git-send-email-chris@chris-wilson.co.uk
Since we now subclass struct drm_device, we can save pointer dances by
noting the equivalence of struct drm_device and struct drm_i915_private,
i.e. by using to_i915().
text data bss dec hex filename
1073824 4562 416 1078802 107612 drivers/gpu/drm/i915/i915.ko
1068976 4562 416 1073954 106322 drivers/gpu/drm/i915/i915.ko
Created by the coccinelle script:
@@
expression E;
identifier p;
@@
- struct drm_i915_private *p = E->dev_private;
+ struct drm_i915_private *p = to_i915(E);
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467628477-25379-1-git-send-email-chris@chris-wilson.co.uk
Make sure that the RPS bottom-half is flushed before we set the idle
frequency when we decide the GPU is idle. This should prevent any races
with the bottom-half and setting the idle frequency, and ensures that
the bottom-half is bounded by the GPU's rpm reference taken for when it
is active (i.e. between gen6_rps_busy() and gen6_rps_idle()).
v2: Avoid recursively using the i915->wq - RPS does not touch the
struct_mutex so has no place being on the ordered i915->wq.
v3: Enable/disable interrupts for RPS busy/idle in order to prevent
further HW access from RPS outside of the wakeref.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
References: https://bugs.freedesktop.org/show_bug.cgi?id=89728
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467616119-4093-6-git-send-email-chris@chris-wilson.co.uk
The retire worker is a low frequency task that makes sure we retire
outstanding requests if userspace is being lax. We only need to start it
once as it remains active until the GPU is idle, so do a cheap test
before the more expensive queue_work(). A consequence of this is that we
need correct locking in the worker to make the hot path of request
submission cheap. To keep the symmetry and keep hangcheck strictly bound
by the GPU's wakelock, we move the cancel_sync(hangcheck) to the idle
worker before dropping the wakelock.
v2: Guard against RCU fouling the breadcrumbs bottom-half whilst we kick
the waiter.
v3: Remove the wakeref assertion squelching (now we hold a wakeref for
the hangcheck, any rpm error there is genuine).
v4: To prevent excess work when retiring requests, we split the busy
flag into two, a boolean to denote whether we hold the wakeref and a
bitmask of active engines.
v5: Reorder cancelling hangcheck upon idling to avoid a race where we
might cancel a hangcheck after being preempted by a new task
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
References: https://bugs.freedesktop.org/show_bug.cgi?id=88437
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467616119-4093-1-git-send-email-chris@chris-wilson.co.uk
When waiting for an interrupt (waiting for the engine to complete some
work), we know we are the only waiter to be woken on this engine. We also
know when the GPU has nearly completed our request (or at least started
processing it), so after being woken and we detect that the GPU is
active and working on our request, allow us the bottom-half (the first
waiter who wakes up to handle checking the seqno after the interrupt) to
spin for a very short while to reduce client latencies.
The impact is minimal, there was an improvement to the realtime-vs-many
clients case, but exporting the function proves useful later. However,
it is tempting to adjust irq_seqno_barrier to include the spin. The
problem is first ensuring that the "start-of-request" seqno is coherent
as we use that as our basis for judging when it is ok to spin. If we
could, spinning there could dramatically shorten some sleeps, and allow
us to make the barriers more conservative to handle missed seqno writes
on more platforms (all gen7+ are known to have the occasional issue, at
least).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-7-git-send-email-chris@chris-wilson.co.uk
BXT BIOS has two options related to GPU power management: "RC6(Render
Standby)" and "GT PM Support". The assumption so far was that disabling
either of these options would leave RC6 uninitialized. According to my
tests this isn't so: for a proper RC6 setup we only need the "GT PM
Support" option to be enabled while the "RC6" option only controls
whether RC6 is left enabled or not by BIOS. OTOH we were missing a few
checks to ensure a proper RC6 setup. Add these now and don't fail the
sanity check if RC6 is disabled. This fixes a problem where RC6 remains
disabled after reloading the driver, since we explicitly disable RC6
during unloading.
v2:
- Print a debug message about the BIOS enabled RC state. (Sagar)
CC: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467216835-1086-2-git-send-email-imre.deak@intel.com
