commit 565602d7 ("drm/i915: Do not acquire crtc state to check clock during modeset, v4.")
removed the possibility that intel_mode_max_pixclk() or
ilk_max_pixel_rate() might return an error, so let's get rid of the
error checks in the callers as well.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1462995892-32416-2-git-send-email-ville.syrjala@linux.intel.comReviewed-by: NJani Nikula <jani.nikula@intel.com>

We calculate the watermark config into intel_atomic_state and then save
it into dev_priv, but never actually use it from there.  This is
left-over from some early ILK-style watermark programming designs that
got changed over time.
Signed-off-by: NMatt Roper <matthew.d.roper@intel.com>
Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1463061971-19638-18-git-send-email-matthew.d.roper@intel.com

Moving watermark calculation into the check phase will allow us to to
reject display configurations for which there are no valid watermark
values before we start trying to program the hardware (although those
tests will come in a subsequent patch).

Another advantage of moving this calculation to the check phase is that
we can calculate the watermarks in a single shot as part of the atomic
transaction.  The watermark interfaces we inherited from our legacy
modesetting days are a bit broken in the atomic design because they use
per-crtc entry points but actually re-calculate and re-program something
that is really more of a global state.  That worked okay in the legacy
modesetting world because operations only ever updated a single CRTC at
a time.  However in the atomic world, a transaction can involve multiple
CRTC's, which means we wind up computing and programming the watermarks
NxN times (where N is the number of CRTC's involved).  With this patch
we eliminate the redundant re-calculation of watermark data for atomic
states (which was the cause of the WARN_ON(!wm_changed) problems that
have plagued us for a while).

We still need to work on the 'commit' side of watermark handling so that
we aren't doing redundant NxN programming of watermarks, but that's
content for future patches.

v2:
 - Bail out of skl_write_wm_values() if the CRTC isn't active.  Now that
   we set dirty_pipes to ~0 if the active pipes change (because
   we need to deal with DDB changes), we can now wind up here for
   disabled pipes, whereas we couldn't before.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89055
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92181
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: NMatt Roper <matthew.d.roper@intel.com>
Tested-by: NDaniel Stone <daniels@collabora.com>
Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1463091100-13747-1-git-send-email-matthew.d.roper@intel.com

Once we move watermark calculation to the atomic check phase, we'll want
to start rejecting display configurations that exceed out watermark
limits.  At the moment we just assume that there's always a valid set of
watermarks, even though this may not actually be true.  Let's prepare by
passing return codes up through the call stack in preparation.
Signed-off-by: NMatt Roper <matthew.d.roper@intel.com>
Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1463061971-19638-15-git-send-email-matthew.d.roper@intel.com

Now that we're properly pre-allocating the DDB during the atomic check
phase and we trust that the allocation is appropriate, let's actually
use the allocation computed and not duplicate that work during the
commit phase.

v2:
 - Significant rebasing now that we can use cached data rates and
   minimum block allocations to avoid grabbing additional plane states.
Signed-off-by: NMatt Roper <matthew.d.roper@intel.com>
Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1463061971-19638-11-git-send-email-matthew.d.roper@intel.com

Calculate the DDB blocks needed to satisfy the current atomic
transaction at atomic check time.  This is a prerequisite to calculating
SKL watermarks during the 'check' phase and rejecting any configurations
that we can't find valid watermarks for.

Due to the nature of DDB allocation, it's possible for the addition of a
new CRTC to make the watermark configuration already in use on another,
unchanged CRTC become invalid.  A change in which CRTC's are active
triggers a recompute of the entire DDB, which unfortunately means we
need to disallow any other atomic commits from racing with such an
update.  If the active CRTC's change, we need to grab the lock on all
CRTC's and run all CRTC's through their 'check' handler to recompute and
re-check their per-CRTC DDB allocations.

Note that with this patch we only compute the DDB allocation but we
don't actually use the computed values during watermark programming yet.
For ease of review/testing/bisecting, we still recompute the DDB at
watermark programming time and just WARN() if it doesn't match the
precomputed values.  A future patch will switch over to using the
precomputed values once we're sure they're being properly computed.

Another clarifying note:  DDB allocation itself shouldn't ever fail with
the algorithm we use today (i.e., we have enough DDB blocks on BXT to
support the minimum needs of the worst-case scenario of every pipe/plane
enabled at full size).  However the watermarks calculations based on the
DDB may fail and we'll be moving those to the atomic check as well in
future patches.

v2:
 - Skip DDB calculations in the rare case where our transaction doesn't
   actually touch any CRTC's at all.  Assuming at least one CRTC state
   is present in our transaction, then it means we can't race with any
   transactions that would update dev_priv->active_crtcs (which requires
   _all_ CRTC locks).

v3:
 - Also calculate DDB during initial hw readout, to prevent using
   incorrect bios values. (Maarten)

v4:
 - Use new distrust_bios_wm flag instead of skip_initial_wm (which was
   never actually set).
 - Set intel_state->active_pipe_changes instead of just realloc_pipes

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Lyude Paul <cpaul@redhat.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Signed-off-by: NMatt Roper <matthew.d.roper@intel.com>
Signed-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1463061971-19638-10-git-send-email-matthew.d.roper@intel.com

SKL-style platforms can't fully trust the watermark/DDB settings
programmed by the BIOS and need to do extra sanitization on their first
atomic update.  Add a flag to dev_priv that is set during hardware
readout and cleared at the end of the first commit.

Note that for the somewhat common case where everything is turned off
when the driver starts up, we don't need to bother with a recompute...we
know exactly what the DDB should be (all zero's) so just setup the DDB
directly in that case.

v2:
 - Move clearing of distrust_bios_wm up below the swap_state call since
   it's a more natural / self-explanatory location.  (Maarten)
 - Use dev_priv->active_crtcs to test whether any CRTC's are turned on
   during HW WM readout rather than trying to count the active CRTC's
   again ourselves.  (Maarten)

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: NMatt Roper <matthew.d.roper@intel.com>
Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1463061971-19638-9-git-send-email-matthew.d.roper@intel.com

For the purposes of DDB re-allocation we need to know whether a
transaction changes the list of CRTC's that are active.  While
state->modeset could be used for this purpose, that would be slightly
too aggressive since it would lead us to re-allocate the DDB when a
CRTC's mode changes, but not its final active state.
Signed-off-by: NMatt Roper <matthew.d.roper@intel.com>
Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1463061971-19638-7-git-send-email-matthew.d.roper@intel.com

Code checkers may complain about the explicit casts between different
enum types, so add comments for known-valid cases to help future
triaging of such complaints.

v2:
- Make the comments more logical (Ville).
Signed-off-by: NImre Deak <imre.deak@intel.com>
Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1463059132-1720-3-git-send-email-imre.deak@intel.com

The coding style documentation says the following about typedefs:

"In general, a pointer, or a struct that has elements that can
reasonably be directly accessed should _never_ be a typedef."

intel_limit_t falls in that category, so just use "struct intel_limit"
instead.
Signed-off-by: NAnder Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: NJani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1462353119-9738-3-git-send-email-ander.conselvan.de.oliveira@intel.com

Those are only used for defining struct intel_limit, so use anonymous
structs instead.
Signed-off-by: NAnder Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: NJani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1462353119-9738-2-git-send-email-ander.conselvan.de.oliveira@intel.com

Just use "struct dpll" everywhere. That's actually shorter than
intel_clock_t.
Signed-off-by: NAnder Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: NJani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1462353119-9738-1-git-send-email-ander.conselvan.de.oliveira@intel.com

Another day, another long overdue conversion. Not much to update inside
intel_overlay.c, but still

   text	   data	    bss	    dec	    hex	filename
6309547	3578778	 696320	10584645	 a18245	vmlinux
6309291	3578778	 696320	10584389	 a18145	vmlinux

a couple of hundred bytes of pointer misdirection.

Whilst here, rename the ioctl entry points to include the _ioctl suffix
so that the user entry points are clear (following the idiom).
Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1463053403-25086-1-git-send-email-chris@chris-wilson.co.ukReviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com>

This way optimization from a previous patch works even better.

v2: Rebase.
Signed-off-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: NJani Nikula <jani.nikula@intel.com>

Pass drm_i915_private to the uncore init/fini routines and their
subservients as it is their native type.

   text    data     bss     dec     hex filename
6309978 3578778  696320 10585076         a183f4 vmlinux
6309530 3578778  696320 10584628         a18234 vmlinux

a modest 400 bytes of saving, but 60 lines of code deleted!
Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: NJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1462885804-26750-1-git-send-email-chris@chris-wilson.co.uk

When the crtc is enabled but !active, we should still compute the
watermarks as if the planes were visible. That would make it more
likely that the we can later transition to active without errors.

Add a FIXME to remind people that we're doing the wrong thing now.
We should perhaps just move the wm computation for each individual plane
into the .check_plane hook, and later we'd just combine the results from
all active planes.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1461940278-17122-2-git-send-email-ville.syrjala@linux.intel.comReviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>

   text	   data	    bss	    dec	    hex	filename
6309351	3578714	 696320	10584385	 a18141	vmlinux
6308391	3578714	 696320	10583425	 a17d81	vmlinux

Almost 1KiB of code reduction.

v2: More s/INTEL_INFO()->gen/INTEL_GEN()/ and IS_GENx() conversions

   text	   data	    bss	    dec	    hex	filename
6304579	3578778	 696320	10579677	 a16edd	vmlinux
6303427	3578778	 696320	10578525	 a16a5d	vmlinux

Now over 1KiB!
Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1462545621-30125-3-git-send-email-chris@chris-wilson.co.uk

I have noticed some of our interrupt handlers use both dev and
dev_priv while they could get away with only dev_priv in the
huge majority of cases.

Tidying that up had a cascading effect on changing functions
prototypes, so relatively big churn factor, but I think it is
for the better.

For example even where changes cascade out of i915_irq.c, for
functions prefixed with intel_, genX_ or <plat>_, it makes more
sense to take dev_priv directly anyway.

This allows us to eliminate local variables and intermixed usage
of dev and dev_priv where only one is good enough.

End result is shrinkage of both source and the resulting binary.

i915.ko:

 - .text         000b0899
 + .text         000b0619

Or if we look at the Gen8 display irq chain:

 -00000000000006ad t gen8_irq_handler
 +0000000000000663 t gen8_irq_handler
   -0000000000000028 T intel_opregion_asle_intr
   +0000000000000024 T intel_opregion_asle_intr
   -000000000000008c t ilk_hpd_irq_handler
   +000000000000007f t ilk_hpd_irq_handler
   -0000000000000116 T intel_check_page_flip
   +0000000000000112 T intel_check_page_flip
   -000000000000011a T intel_prepare_page_flip
   +0000000000000119 T intel_prepare_page_flip
   -0000000000000014 T intel_finish_page_flip_plane
   +0000000000000013 T intel_finish_page_flip_plane
   -0000000000000053 t hsw_pipe_crc_irq_handler
   +000000000000004c t hsw_pipe_crc_irq_handler
   -000000000000022e t cpt_irq_handler
   +0000000000000213 t cpt_irq_handler

So small shrinkage but it is all fast paths so doesn't harm.

Situation is similar in other interrupt handlers as well.

v2: Tidy intel_queue_rps_boost_for_request as well. (Chris Wilson)
Signed-off-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk>

This makes it easier to debug issues like https://bugs.freedesktop.org/show_bug.cgi?id=93477Signed-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Chris Wilson <|chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/843f4327-1574-cf8e-0776-adbb0d58c2c0@mblankhorst.nlReviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch>

They use dev_priv exclusively so pass it in instead of dev
for smaller source and binary.
Signed-off-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1461844620-35360-1-git-send-email-tvrtko.ursulin@linux.intel.com

The LVDS border enable is independent from the panel fitter. Move the
readout of the "border bits" from i9xx_get_pfit_config() to
intel_lvds_get_config(), where it will be read if LVDS is enabled even
if the panel fitter is not.

This fixes the state checker warning:

[drm:intel_pipe_config_compare [i915]] *ERROR* mismatch in
gmch_pfit.lvds_border_bits (expected 0x00008000, found 0x00000000)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: drm-intel-fixes@lists.freedesktop.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87632Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Tested-by: NSitsofe Wheeler <sitsofe@yahoo.com>
Signed-off-by: NJani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1461933243-2140-1-git-send-email-jani.nikula@intel.com

The comment about GMBUSFREQ is confused. The spec actually explains
the 4MHz thing perfectly by noting that the 4MHz divider values is
actually just bits [9:2] not [9:0], hence the divide by 1000 correct.
Replace the confused note with a quote from the spec, and eliminate
the duplicated comment that snuck in.
Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1461689194-6079-4-git-send-email-ville.syrjala@linux.intel.comReviewed-by: NMika Kahola <mika.kahola@intel.com>

Update CDCLK_FREQ on BDW after changing the cdclk frequency. Not sure
if this is a late addition to the spec, or if I simply overlooked this
step when writing the original code.

This is what Bspec has to say about CDCLK_FREQ:
"Program this field to the CD clock frequency minus one. This is used to
 generate a divided down clock for miscellaneous timers in display."

And the "Broadwell Sequences for Changing CD Clock Frequency" section
clarifies this further:
"For CD clock 337.5 MHz, program 337 decimal.
 For CD clock 450 MHz, program 449 decimal.
 For CD clock 540 MHz, program 539 decimal.
 For CD clock 675 MHz, program 674 decimal."

Cc: stable@vger.kernel.org
Cc: Mika Kahola <mika.kahola@intel.com>
Fixes: b432e5cf ("drm/i915: BDW clock change support")
Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1461689194-6079-2-git-send-email-ville.syrjala@linux.intel.comReviewed-by: NMika Kahola <mika.kahola@intel.com>

If we move the release of the GEM request (i.e. decoupling it from the
various lists used for client and context tracking) after it is complete
(either by the GPU retiring the request, or by the caller cancelling the
request), we can remove the requirement that the final unreference of
the GEM request need to be under the struct_mutex.

The careful reader may notice that one or two impossible NULL pointer
tests are dropped for readability. These pointers cannot be NULL since
they are assigned during request construction and never unset.

v2,v3: Rebalance execlists by moving the context unpinning.
v4: Rebase onto -nightly
v5: Avoid trying to rebalance execlist/GuC context pinning, leave that
to the next step
Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: NJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1461833819-3991-21-git-send-email-chris@chris-wilson.co.uk

When releasing the intel_fbdev, we should unpin the framebuffer that we
pinned during construction.
Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
Link: http://patchwork.freedesktop.org/patch/msgid/1461833819-3991-1-git-send-email-chris@chris-wilson.co.uk

Propagate the real error from drm_gem_object_init(). Note this also
fixes some confusion in the error return from i915_gem_alloc_object...

v2:
(Matthew Auld)
  - updated new users of gem_alloc_object from latest drm-nightly
  - replaced occurrences of IS_ERR_OR_NULL() with IS_ERR()
v3:
(Joonas Lahtinen)
  - fix double "From:" in commit message
  - add goto teardown path
v4:
(Matthew Auld)
  - rebase with i915_gem_alloc_object name change
Signed-off-by: NMatthew Auld <matthew.auld@intel.com>
Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: NJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1461587533-8841-1-git-send-email-matthew.auld@intel.com
[Joonas: Removed spurious " = NULL" from _init() function]
Signed-off-by: NJoonas Lahtinen <joonas.lahtinen@linux.intel.com>

I just noticed that VLV/CHV have a RAWCLK_FREQ register just like PCH
platforms. It lives in the display power well, so we should update it
when enabling the power well.

Interestingly the BIOS seems to leave it at the reset value (125) which
doesn't match the rawclk frequency on VLV/CHV (200 MHz). As always with
these register, the spec is extremely vague what the register does. All
it says is: "This is used to generate a divided down clock for
miscellaneous timers in display." Based on a quick test, at least AUX
and PWM appear to be unaffected by this.

But since the register is there, let's configure it in accordance with
the spec.

Note that we have to move intel_update_rawclk() to occur before we
touch the power wells, so that the dev_priv->rawclk_freq is already
populated when the disp2 enable hook gets called for the first time.
I think this should be safe to do on other platforms as well.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1461768202-17544-1-git-send-email-ville.syrjala@linux.intel.comReviewed-by: NRodrigo Vivi <rodrigo.vivi@intel.com>

Because having both i915_gem_object_alloc() and i915_gem_alloc_object()
(with different return conventions) is just too confusing!

(i915_gem_object_alloc() is the low-level memory allocator, and remains
unchanged, whereas i915_gem_alloc_object() is a constructor that ALSO
initialises the newly-allocated object.)
Signed-off-by: NDave Gordon <david.s.gordon@intel.com>
Reviewed-by: NJoonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: NTvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1461348872-4702-1-git-send-email-david.s.gordon@intel.com

intel_pipe_will_have_type() doesn't just look at the passied in
pipe_config, instead it expects there to be a full atomic state behind
it. Obviously that won't go so well when vlv_force_pll_on() just uses a
temp pipe_config. Fix things by using pipe_config->has_dsi_encoder
instead intel_pipe_will_have_type(INTEL_OUTPUT_DSI) to check if we need
to actually enable the DPLL.

Here's an example oops for reference:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
IP: [<ffffffffa0389a5b>] intel_pipe_will_have_type+0x15/0x7b [i915]
PGD 7acda067 PUD 72696067 PMD 0
Oops: 0000 [#1] PREEMPT SMP
Modules linked in: i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm intel_gtt agpgart netconsole psmouse atkbd iTCO_wdt libps2 coretemp hwmon efi_pstore intel_rapl punit_atom_debug efivars pcspkr i2c_i801 r8169 lpc_ich mii processor_thermal_device snd_soc_rt5670 intel_soc_dts_iosf snd_soc_rl6231 i2c_hid hid snd_intel_sst_acpi snd_intel_sst_core snd_soc_sst_mfld_platform snd_soc_sst_match snd_soc_core i8042 serio snd_compress snd_pcm snd_timer snd i2c_designware_platform sdhci_acpi i2c_designware_core soundcore sdhci pwm_lpss_platform mmc_core pwm_lpss spi_pxa2xx_platform evdev int3403_thermal int3400_thermal int340x_thermal_zone acpi_thermal_rel sch_fq_codel ip_tables x_tables ipv6 autofs4
CPU: 3 PID: 290 Comm: Xorg Tainted: G     U          4.6.0-rc4-bsw+ #2876
Hardware name: Intel Corporation CHERRYVIEW C0 PLATFORM/Braswell CRB, BIOS BRAS.X64.X088.R00.1510270350 10/27/2015
task: ffff88007a8dd200 ti: ffff880173ac4000 task.ti: ffff880173ac4000
RIP: 0010:[<ffffffffa0389a5b>]  [<ffffffffa0389a5b>] intel_pipe_will_have_type+0x15/0x7b [i915]
RSP: 0018:ffff880173ac7928  EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff880176594000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000009 RDI: ffff880176594000
RBP: ffff880173ac7930 R08: 0000000000019290 R09: 0000000000000000
R10: ffff880173ac7890 R11: 00000000000080cf R12: ffff88017fbd4000
R13: ffffffffa03e3c44 R14: ffff88007492c000 R15: ffff88007492c000
FS:  00007ff8936a6940(0000) GS:ffff88017ef80000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000030 CR3: 0000000177e08000 CR4: 00000000001006e0
Stack:
 ffff880176594000 ffff880173ac7948 ffffffffa0389b42 ffff880176594000
 ffff880173ac7978 ffffffffa0396e02 ffff8801765b0000 ffff88007af660d8
 0000000000000000 0000000000000004 ffff880173ac79c0 ffffffffa03b6b64
Call Trace:
 [<ffffffffa0389b42>] chv_compute_dpll.isra.39+0x33/0x55 [i915]
 [<ffffffffa0396e02>] vlv_force_pll_on+0x80/0xc6 [i915]
 [<ffffffffa03b6b64>] vlv_power_sequencer_pipe+0x29b/0x3dd [i915]
 [<ffffffffa03b6cd4>] _pp_stat_reg+0x2e/0x38 [i915]
 [<ffffffffa03b6dc1>] wait_panel_status+0x4c/0x1ec [i915]
 [<ffffffffa03b6fcb>] wait_panel_power_cycle+0x6a/0xb4 [i915]
 [<ffffffffa03b70da>] edp_panel_vdd_on+0xc5/0x1d1 [i915]
 [<ffffffffa03b861b>] intel_dp_aux_ch+0x55/0x572 [i915]
 [<ffffffff810af5c8>] ? mark_held_locks+0x5d/0x74
 [<ffffffff81518e61>] ? mutex_lock_nested+0x321/0x346
 [<ffffffff81094007>] ? preempt_count_sub+0xf2/0x102
 [<ffffffffa03b8cb4>] intel_dp_aux_transfer+0x17c/0x1b5 [i915]
 [<ffffffffa03028ef>] drm_dp_dpcd_access+0x62/0xed [drm_kms_helper]
 [<ffffffffa0302995>] drm_dp_dpcd_read+0x1b/0x1f [drm_kms_helper]
 [<ffffffffa03b5147>] intel_dp_dpcd_read_wake+0x31/0x69 [i915]
 [<ffffffffa03bb36a>] intel_dp_long_pulse+0x15f/0x5ed [i915]
 [<ffffffffa03bbb09>] intel_dp_detect+0x79/0x95 [i915]
 [<ffffffffa030340e>] drm_helper_probe_single_connector_modes+0xc7/0x3db [drm_kms_helper]
 [<ffffffffa029de23>] drm_mode_getconnector+0xe9/0x333 [drm]
 [<ffffffff810b1cfb>] ? lock_acquire+0x137/0x1df
 [<ffffffffa0292364>] drm_ioctl+0x266/0x3ae [drm]
 [<ffffffffa029dd3a>] ? drm_mode_getcrtc+0x126/0x126 [drm]
 [<ffffffff811af082>] vfs_ioctl+0x18/0x34
 [<ffffffff811af682>] do_vfs_ioctl+0x547/0x5fe
 [<ffffffff811b9acb>] ? __fget_light+0x62/0x71
 [<ffffffff811af77c>] SyS_ioctl+0x43/0x61
 [<ffffffff81001a82>] do_syscall_64+0x63/0xf8
 [<ffffffff8151bc9a>] entry_SYSCALL64_slow_path+0x25/0x25
Code: 35 00 40 a0 e8 97 4b ce e0 b8 17 00 00 00 5d c3 b8 17 00 00 00 c3 0f 1f 44 00 00 55 31 c0 31 d2 48 89 e5 53 48 8b 8f e8 01 00 00 <44> 8b 49 30 41 39 c1 7e 2d 4c 8b 51 38 4c 8b 41 40 49 83 3c c2
RIP  [<ffffffffa0389a5b>] intel_pipe_will_have_type+0x15/0x7b [i915]
 RSP <ffff880173ac7928>
CR2: 0000000000000030

The regressing patch wasn't exactly new (as in first posted more than
six months ago), so I'm a bit baffled how I didn't manage to hit this
myself so far.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Marius Vlad <marius.c.vlad@intel.com>
Reported-by: NMarius Vlad <marius.c.vlad@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94995
Fixes: cd2d34d9 ("drm/i915: Setup DPLL/DPLLMD for DSI too on VLV/CHV")
Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1461000844-20543-1-git-send-email-ville.syrjala@linux.intel.comTested-by: NMarius Vlad <marius.c.vlad@intel.com>
Reviewed-by: NJani Nikula <jani.nikula@intel.com>

When a vblank wait times out in intel_atomic_wait_for_vblanks() we just
get a cryptic 'WARN_ON(!ret)' backtrace in dmesg. Repace it with
something that tells you what actually happened.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1460978973-24945-1-git-send-email-ville.syrjala@linux.intel.comReviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>

The legacy cursor ioctl expects to be asynchronous with respect to other
screen updates, in particular page flips. As X updates the cursor from a
signal context, if the cursor blocks then it will stall both the input
and output chains causing bad stuttering and horrible UX.
Reported-and-tested-by: NRafael Ristovski <rafael.ristovski@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94980
Fixes: 5008e874 ("drm/i915: Make wait_for_flips interruptible.")
Suggested-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: stable@vger.kernel.org
Link: http://patchwork.freedesktop.org/patch/msgid/1460922166-20292-1-git-send-email-chris@chris-wilson.co.ukAcked-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>

Compute the DSI PLL parameters during .compute_config() rather than
.pre_pll_enable() so that we can fail gracefully if we can't find
suitable parameters.

In order to do that we need to store the DSI PLL parameters in
pipe_config.

v2: Handle BXT too
Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: NJani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1460488478-18311-3-git-send-email-ville.syrjala@linux.intel.comTested-by: NJani Nikula <jani.nikula@intel.com>

Set up DPLL and DPLL_MD even when driving DSI output on VLV/CHV. While
the DPLL isn't used to provide the clock we still need the refclock, and
it appears that the pixel repeat factor also has an effect on DSI
output. So set up eveyrhing in DPLL and DPLL_MD as we would do for
DP/HDMI/VGA, but don't actually enable the DPLL or configure the
dividers via DPIO.
Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1460488478-18311-2-git-send-email-ville.syrjala@linux.intel.comReviewed-by: NJani Nikula <jani.nikula@intel.com>
Tested-by: NJani Nikula <jani.nikula@intel.com>

I caught a few errors in our current PHY/CDCLK programming by sanity
checking the actual programmed state, so I thought it would be also
useful for the future. In addition to verifying the state after
programming it also verify it after exiting DC5, to make sure DMC
restored/kept intact everything related.

v2:
- Inlining __phy_reg_verify_state() doesn't make sense and also
  incorrect, so don't do it (PW/CI gcc)
v3:
- Rebase on latest -nightly
Signed-off-by: NImre Deak <imre.deak@intel.com>
Reviewed-by: NDavid Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1459780030-15781-1-git-send-email-imre.deak@intel.com

When determining whether CDCLK is enabled by BIOS and so we should skip
reprogramming it, we didn't check the related DBUF power request and
state. In theory BIOS could enable one without the other so check for
this case and reprogram things if something is amiss.
Signed-off-by: NImre Deak <imre.deak@intel.com>
Reviewed-by: NMika Kuoppala <mika.kuoppala@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1459515767-29228-13-git-send-email-imre.deak@intel.com

Power well 1 is managed by the DMC firmware so don't toggle it on-demand
from the driver. This means we need to follow the BSpec display
initialization sequence during driver loading and resuming (both system
and runtime) and enable power well 1 only once there. Afterwards DMC
will toggle power well 1 whenever entering/exiting DC5.

For this to work we also need to do away getting the PLL power domain,
since that just kept runtime PM disabled for good.
Signed-off-by: NImre Deak <imre.deak@intel.com>
Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1459515767-29228-12-git-send-email-imre.deak@intel.com

For internal APIs passing dev_priv is preferred to reduce indirections,
so convert over a few DDI PHY, CDCLK helpers.

No functional change.
Signed-off-by: NImre Deak <imre.deak@intel.com>
Acked-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: NDavid Weinehall <david.weinehall@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1459515767-29228-10-git-send-email-imre.deak@intel.com

Replace the legacy drm_send_vblank_event() with the new helper function.
Signed-off-by: NGustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1460656118-16766-4-git-send-email-gustavo@padovan.org

Conceptually, each request is a record of a hardware transaction - we
build up a list of pending commands and then either commit them to
hardware, or cancel them. However, whilst building up the list of
pending commands, we may modify state outside of the request and make
references to the pending request. If we do so and then cancel that
request, external objects then point to the deleted request leading to
both graphical and memory corruption.

The easiest example is to consider object/VMA tracking. When we mark an
object as active in a request, we store a pointer to this, the most
recent request, in the object. Then we want to free that object, we wait
for the most recent request to be idle before proceeding (otherwise the
hardware will write to pages now owned by the system, or we will attempt
to read from those pages before the hardware is finished writing). If
the request was cancelled instead, that wait completes immediately. As a
result, all requests must be committed and not cancelled if the external
state is unknown.

All that remains of i915_gem_request_cancel() users are just a couple of
extremely unlikely allocation failures, so remove the API entirely.

A consequence of committing all incomplete requests is that we generate
excess breadcrumbs and fill the ring much more often with dummy work. We
have completely undone the outstanding_last_seqno optimisation.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93907Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: stable@vger.kernel.org
Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1460565315-7748-16-git-send-email-chris@chris-wilson.co.uk

Reporting -EIO from i915_wait_request() has proven very troublematic
over the years, with numerous hard-to-reproduce bugs cropping up in the
corner case of where a reset occurs and the code wasn't expecting such
an error.

If the we reset the GPU or have detected a hang and wish to reset the
GPU, the request is forcibly complete and the wait broken. Currently, we
report either -EAGAIN or -EIO in order for the caller to retreat and
restart the wait (if appropriate) after dropping and then reacquiring
the struct_mutex (essential to allow the GPU reset to proceed). However,
if we take the view that the request is complete (no further work will
be done on it by the GPU because it is dead and soon to be reset), then
we can proceed with the task at hand and then drop the struct_mutex
allowing the reset to occur. This transfers the burden of checking
whether it is safe to proceed to the caller, which in all but one
instance it is safe - completely eliminating the source of all spurious
-EIO.

Of note, we only have two API entry points where we expect that
userspace can observe an EIO. First is when submitting an execbuf, if
the GPU is terminally wedged, then the operation cannot succeed and an
-EIO is reported. Secondly, existing userspace uses the throttle ioctl
to detect an already wedged GPU before starting using HW acceleration
(or to confirm that the GPU is wedged after an error condition). So if
the GPU is wedged when the user calls throttle, also report -EIO.

v2: Split more carefully the change to i915_wait_request() and assorted
ABI from the reset handling.
v3: Add a couple of WARN_ON(EIO) to the interruptible modesetting code
so that we don't start to leak EIO there in future (and break our hang
resistant modesetting).
Signed-off-by: NChris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1460565315-7748-9-git-send-email-chris@chris-wilson.co.uk
Link: http://patchwork.freedesktop.org/patch/msgid/1460565315-7748-1-git-send-email-chris@chris-wilson.co.uk