As we may unwind incomplete requests (for preemption) prior to
processing the CSB and the schedule-out events, we may update rq->engine
(resetting it to point back to the parent virtual engine) prior to
calling execlists_schedule_out(), invalidating the assertion that the
request still points to the inflight engine. (The likelihood of this is
increased if the CSB interrupt processing is pushed to the ksoftirqd for
being too slow and direct submission overtakes it.)
Tvrtko summarised it as:
"So unwind from direct submission resets rq->engine and races with
process_csb from the tasklet which notices request has actually
completed."
Reported-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Fixes: df40306902 ("drm/i915/execlists: Lift process_csb() out of the irq-off spinlock")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190907105046.19934-1-chris@chris-wilson.co.uk
If we make sure we grab a strong reference to each object as we dump it,
we can reduce the locks outside of our iterators to an rcu_read_lock.
This should prevent errors like:
[ 2138.371911] BUG: KASAN: use-after-free in per_file_stats+0x43/0x380 [i915]
[ 2138.371924] Read of size 8 at addr ffff888223651000 by task cat/8293
[ 2138.371947] CPU: 0 PID: 8293 Comm: cat Not tainted 5.3.0-rc6-CI-Custom_4352+ #1
[ 2138.371953] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./J4205-ITX, BIOS P1.40 07/14/2017
[ 2138.371959] Call Trace:
[ 2138.371974] dump_stack+0x7c/0xbb
[ 2138.372099] ? per_file_stats+0x43/0x380 [i915]
[ 2138.372108] print_address_description+0x73/0x3a0
[ 2138.372231] ? per_file_stats+0x43/0x380 [i915]
[ 2138.372352] ? per_file_stats+0x43/0x380 [i915]
[ 2138.372362] __kasan_report+0x14e/0x192
[ 2138.372489] ? per_file_stats+0x43/0x380 [i915]
[ 2138.372502] kasan_report+0xe/0x20
[ 2138.372625] per_file_stats+0x43/0x380 [i915]
[ 2138.372751] ? i915_panel_show+0x110/0x110 [i915]
[ 2138.372761] idr_for_each+0xa7/0x160
[ 2138.372773] ? idr_get_next_ul+0x110/0x110
[ 2138.372782] ? do_raw_spin_lock+0x10a/0x1d0
[ 2138.372923] print_context_stats+0x264/0x510 [i915]
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: David Weinehall <david.weinehall@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190903062133.27360-1-chris@chris-wilson.co.uk
Tiger Lake has up to 4 pipes so the mask would need to be 0xf instead of
0x7. Do not hardcode the mask so it allows the fake MST encoders to
connect to all pipes no matter how many the platform has.
Iterating over all pipes to keep consistent with intel_ddi_init().
Initialy this patch was replaced by commit 4eaceea3a0 ("drm/i915:
Fix DP-MST crtc_mask") but userspace it not correctly using
encoder.possible_crtcs and it was reverted by
commit e838bfa8e1 ("Revert "drm/i915: Fix DP-MST crtc_mask"")
Userspace should be fixed but it might take a while, so bringing this
patch back for now.
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190904230241.20638-2-jose.souza@intel.com
Gen 12 onwards moves the DP_TP_* registers to be transcoder-based rather
than port-based. This adds the new register addresses and changes all
the callers to use the register saved in intel_dp->regs.*. This is
filled out when preparing to enable the port so we take into account if
we should use the transcoder or the port.
v2: reimplement by stashing the registers we want to access under
intel_dp->reg. Now they are initialized when enabling the port.
Ville suggested to store the transcoder to be used exclusively
by TGL+. After implementing I thought just storing the register directly
made it cleaner.
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190904213419.27547-5-jose.souza@intel.com
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
It was enabling and checking PSR interruptions in every transcoder
while it should keep the interruptions on the non-used transcoders
masked.
While doing this it gives us trouble on Tiger Lake if we are
reading/writing to registers of disabled transcoders since from gen12
onwards the registers are relative to the transcoder. Instead of forcing
them ON to access those registers, just avoid the accesses as they are
not needed.
v2 (Lucas):
- Explain why we can't keep accessing all transcoders
- Remove TODO about extending the irq handling to multiple instances:
when/if implementing multiple instances it's pretty clear by the
singleton psr that it needs to be extended
- Fix intel_psr_debug_set() calling psr_irq_control() with
psr.transcoder not set yet (from Imre). Now we only set the debug
register right away if psr is already enabled. Otherwise we just
record the value to be set when enabling the source.
- Do not depend on the value of TRANSCODER_A. Just be relative to it
(from Imre)
- handle psr error last so we don't schedule the work before handling
the other flags
v3:
- Adding a warning about setting reserverd bits on EDP_PSR_IMR
Cc: Imre Deak <imre.deak@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190904213419.27547-2-jose.souza@intel.com
For glk, add hw read out to create hw blob of gamma
lut values.
v4: -No need to initialize *blob [Jani]
-Removed right shifts [Jani]
-Dropped dev local var [Jani]
v5: -Returned blob instead of assigning it internally within the
function [Ville]
-Renamed glk_get_color_config() to glk_read_luts() [Ville]
-Added degamma validation [Ville]
v9: -80 character limit [Uma]
-Made read func para as const [Ville, Uma]
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1567538578-4489-8-git-send-email-swati2.sharma@intel.com
For ilk, add hw read out to create hw blob of gamma
lut values.
v4: -No need to initialize *blob [Jani]
-Removed right shifts [Jani]
-Dropped dev local var [Jani]
v5: -Returned blob instead of assigning it internally within the
function [Ville]
-Renamed ilk_get_color_config() to ilk_read_luts() [Ville]
v9: -80 character limit [Uma]
-Made read func para as const [Ville, Uma]
-Renamed ilk_read_gamma_lut() to ilk_read_lut_10() [Uma, Ville]
v10: -Made ilk_read_luts() static [Jani]
-ilk_load_lut_10 has lut_size, not (lut_size - 1) [Jani]
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1567538578-4489-7-git-send-email-swati2.sharma@intel.com
For the legacy(gen < 4) gamma, add hw read out to create hw blob of gamma
lut values. Also, add function intel_color_lut_pack to convert hw value
with given bit precision to lut property val.
v4: -No need to initialize *blob [Jani]
-Removed right shifts [Jani]
-Dropped dev local var [Jani]
v5: -Returned blob instead of assigning it internally within the
function [Ville]
-Renamed function i9xx_get_color_config() to i9xx_read_luts()
-Renamed i9xx_get_config_internal() to i9xx_read_lut_8() [Ville]
v9: -Change in commit message [Jani, Uma]
-Wrap commit within 75 characters [Uma]
-Use macro for 256 [Uma]
-Made read func para as const [Ville, Uma]
v10: -Made i9xx_read_luts() static [Jani]
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1567538578-4489-6-git-send-email-swati2.sharma@intel.com
Add func intel_color_lut_equal() to compare hw/sw gamma
lut values. Since hw/sw gamma lut sizes and lut entries comparison
will be different for different gamma modes, add gamma mode dependent
checks.
v3: -Rebase
v4: -Renamed intel_compare_color_lut() to intel_color_lut_equal() [Jani]
-Added the default label above the correct label [Jani]
-Corrected smatch warn "variable dereferenced before check"
[Dan Carpenter]
v5: -Added condition (!blob1 && !blob2) return true [Jani]
v6: -Made patch11 as patch3 [Jani]
v8: -Split patch 3 into 4 patches
-Optimized blob check condition [Ville]
v9: -Exclude spilt gamma mode (bdw and ivb platforms)
as there is exception in way gamma values are written in
hardware [Ville]
-Added exception made in commit [Uma]
-Dropped else, character limit and indentation [Uma]
-Added multi segmented gama mode for icl+ platforms [Uma]
v10: -Dropped multi segmented mode for icl+ platforms [Jani]
-Removed references of sw and hw state in compare code [Jani]
-Dropped inline from func [Jani]
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1567538578-4489-4-git-send-email-swati2.sharma@intel.com
Each platform supports different gamma modes and each gamma mode
has different bit precision. Here bit precision corresponds
to number of bits the hw LUT supports.
Add func per platform to return bit precision corresponding to gamma mode
which will be later used as a parameter in lut comparison function
intel_color_lut_equal().
This is done for legacy, ilk, glk and their variant platforms.
v6: -Added func intel_color_get_bit_precision() to get bit precision for
gamma and degamma lut readout depending upon platform and
corresponding to load_luts() [Ankit]
-Made patch11 as patch3 [Jani]
v7: -Renamed func intel_color_get_bit_precision() to
intel_color_get_gamma_bit_precision()
-Added separate function/platform for gamma bit precision [Ville]
-Corrected checkpatch warnings
v8: -Split patch 3 into 4 separate patches
v9: -Changed commit message, gave more info [Uma]
-Added precision func for icl+ platform
v10: -Removed precision func for chv and icl+ platforms [Jani]
-Added gamma_enable check once [Jani]
Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1567538578-4489-2-git-send-email-swati2.sharma@intel.com
obj->pin_global was originally used as a means to keep the shrinker off
the active scanout, but we use the vma->pin_count itself for that and
the obj->frontbuffer to delay shrinking active framebuffers. The other
role that obj->pin_global gained was for spotting display objects inside
GEM and working harder to keep those coherent; for which we can again
simply inspect obj->frontbuffer directly.
Coming up next, we will want to manipulate the pin_global counter
outside of the principle locks, so would need to make pin_global atomic.
However, since obj->frontbuffer is already managed atomically, it makes
sense to use that the primary key for display objects instead of having
pin_global.
Ville pointed out the principle difference is that obj->frontbuffer is
set for as long as an intel_framebuffer is attached to an object, but
obj->pin_global was only raised for as long as the object was active. In
practice, this means that we consider the object as being on the scanout
for longer than is strictly required, causing us to be more proactive in
flushing -- though it should be true that we would have flushed
eventually when the back became the front, except that on the flip path
that flush is async but when hit from another ioctl it will be
synchronous.
v2: i915_gem_object_is_framebuffer()
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190902040303.14195-5-chris@chris-wilson.co.uk
Reogranize the HDMI deep color state computation to just
loop over possible bpc values. Avoids having to maintain
so many variants of the clock etc.
The current code also looks confused w.r.t. port_clock vs.
bw_constrained. It would happily update port_clock for
deep color but then not actually enable deep color due to
bw_constrained being set. The new logic handles that case
correctly.
v2: Pull stuff into separate funcs (Jani)
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190828183424.7856-1-ville.syrjala@linux.intel.com
enum port is a mess now because it no longer matches the spec
at all. Let's start to dig ourselves out of this hole by
reducing our reliance on port_name(). This should at least make
a bunch of debug messages a bit more sensible while we think how
to fill the the hole properly.
Based on the following cocci script with a lot of manual cleanup
(all the format strings etc.):
@@
expression E;
@@
(
- port_name(E->port)
+ E->base.base.id, E->base.name
|
- port_name(E.port)
+ E.base.base.id, E.base.name
)
@@
enum port P;
expression E;
@@
P = E->port
<...
- port_name(P)
+ E->base.base.id, E->base.name
...>
@@
enum port P;
expression E;
@@
P = E.port
<...
- port_name(P)
+ E.base.base.id, E.base.name
...>
@@
expression E;
@@
{
- enum port P = E;
... when != P
}
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190830182719.32608-1-ville.syrjala@linux.intel.com
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
The bspec has just recently been updated with new cdclk values that
require the use of a /2 CD2X divider rather than a /1 divider. Once we
add the divider selection logic to ICL+ cdclk programming, we have
pretty much the same logic we were already using on CNL, so it's simpler
to drop icl_set_cdclk() completely and reuse cnl_set_cdclk() on gen11+
platforms as well.
v2:
- Using ICL_CDCLK_CD2X_PIPE_NONE + BXT_CDCLK_CD2X_PIPE(pipe) for TGL is
correct, but looks really confusing. Add some TGL_ macros that alias
these to avoid confusion. (Ville)
- Use DIV_ROUND_CLOSEST rather than / when applying the divider. (Ville)
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190830004828.19359-1-matthew.d.roper@intel.com
>From Gen12 onwards, HDCP HW block is implemented within transcoders.
Till Gen11 HDCP HW block was part of DDI.
Hence required changes in HW programming is handled here.
As ME FW needs the transcoder detail on which HDCP is enabled
on Gen12+ platform, we are populating the detail in hdcp_port_data.
v2:
_MMIO_TRANS is used [Lucas and Daniel]
platform check is moved into the caller [Lucas]
v3:
platform check is moved into a macro [Shashank]
v4:
Few optimizations in the coding [Shashank]
v5:
Fixed alignment in macro definition in i915_reg.h [Shashank]
unused variables "reg" is removed.
v6:
Configuring the transcoder at compute_config.
transcoder is used instead of pipe in macros.
Rebased.
v7:
transcoder is cached at intel_hdcp
hdcp_port_data is configured with transcoder index asper ME FW.
v8:
s/trans/cpu_transcoder
s/tc/cpu_transcoder
v9:
rep_ctl is prepared for TCD too.
return moved into deault of rep_ctl prepare function [Shashank]
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190828164216.405-7-ramalingam.c@intel.com
