Skip to content
  1. Apr 27, 2018
  3. Apr 26, 2018
  5. Apr 25, 2018
  7. Apr 24, 2018
  9. Apr 23, 2018
  11. Apr 21, 2018
    • Hans de Goede's avatar
      drm/i915: Do NOT skip the first 4k of stolen memory for pre-allocated buffers v2 · 011f22eb
      Hans de Goede authored 
      Before this commit the WaSkipStolenMemoryFirstPage workaround code was
skipping the first 4k by passing 4096 as start of the address range passed
to drm_mm_init(). This means that calling drm_mm_reserve_node() to try and
reserve the firmware framebuffer so that we can inherit it would always
fail, as the firmware framebuffer starts at address 0.

Commit d4353761

 ("drm/i915: skip the first 4k of stolen memory on
everything >= gen8") says in its commit message: "This is confirmed to fix
Skylake screen flickering issues (probably caused by the fact that we
initialized a ring in the first page of stolen, but I didn't 100% confirm
this theory)."

Which suggests that it is safe to use the first page for a linear
framebuffer as the firmware is doing (see note below).

This commit always passes 0 as start to drm_mm_init() and works around
WaSkipStolenMemoryFirstPage in i915_gem_stolen_insert_node_in_range()
by insuring the start address passed by to drm_mm_insert_node_in_range()
is always 4k or more. All entry points to i915_gem_stolen.c go through
i915_gem_stolen_insert_node_in_range(), so that any newly allocated
objects such as ring-buffers will not be allocated in the first 4k.

The one exception is i915_gem_object_create_stolen_for_preallocated()
which directly calls drm_mm_reserve_node() which now will be able to
use the first 4k.

This fixes the i915 driver no longer being able to inherit the firmware
framebuffer on gen8+, which fixes the video output changing from the
vendor logo to a black screen as soon as the i915 driver is loaded
(on systems without fbcon).

Some notes about the mapping of the BIOS framebuffer:

v1 led to some discussion if the assumption of the intel_display.c code
that the firmware framebuffer is a linear mapping of the stolen memory
starting at offset 0 is still correct, because that would mean that the
GOP does not implement the WaSkipStolenMemoryFirstPage workaround.

To verify this the following code was added at the end of
i915_gem_object_create_stolen_for_preallocated() :

pr_err("first ggtt entry before bind: 0x%016llx\n",
       readq(dev_priv->ggtt.gsm));
ret = i915_vma_bind(vma,
            HAS_LLC(dev_priv) ? I915_CACHE_LLC : I915_CACHE_NONE,
            PIN_UPDATE);
pr_err("i915_vma_bind ret %d\n", ret);
pr_err("first ggtt entry after bind: 0x%016llx\n",
       readq(dev_priv->ggtt.gsm));

Which prints the mapping of the first page, then does a vma_bind() to
force update the mapping with our linear view of the framebuffer and
then prints the mapping of the first page again.

On an Asrock B150M Pro4S/D3 mainboard with i5-6500 CPU this prints:

[    1.651141] first ggtt entry before bind: 0x0000000078c00001
[    1.651151] i915_vma_bind ret 0
[    1.651152] first ggtt entry after bind: 0x0000000078c00083

And "sudo cat /proc/iomem | grep Stolen" gives:
  78c00000-88bfffff : Graphics Stolen Memory

There are no visual changes with this patch (BIOS vendor logo still
stays in place when we inherit the BIOS framebuffer), so the vma_bind()
does not impact which memory is being scanned out.

The address of the first ggtt entry matches with the start of stolen
and the i915_vma_bind call only changes the first gtt entry's flags,
or-ing in _PAGE_RW (BIT(1)) and PPAT_CACHED (BIT(7)), which perfectly
matches what we would expect based on gen8_pte_encode()'s behavior.

So it seems that the GOP indeed does NOT implement the wa and the i915's
code assuming a linear mapping at the start of stolen for the BIOS fb
still holds true for gen8+.

I've also tested this on a Cherry Trail based device (a GPD Win)
with identical results (the flags are 0x1b after the vma_bind
on CHT, which matches with I915_CACHE_NONE).

Changed in v2: No code changes, extended the commit message with the
verification that the intel_display.c BIOS framebuffer mapping is still
correct.

Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180420095933.16442-1-hdegoede@redhat.com
      011f22eb
    • Dhinakaran Pandiyan's avatar
      drm/i915/psr: Timestamps for PSR entry and exit interrupts. · 3f983e54
      Dhinakaran Pandiyan authored 
      

Timestamps are useful for IGT tests that trigger PSR exit and/or wait for
PSR entry.

v2: Removed seqlock (Ville)
    Removed erroneous warning in irq loop (Chris)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: default avatarDhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: default avatarJose Roberto de Souza <jose.souza@intel.com>
Signed-off-by: default avatarRodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180403212420.25007-4-dhinakaran.pandiyan@intel.com
      3f983e54
    • Dhinakaran Pandiyan's avatar
      drm/i915/psr: Control PSR interrupts via debugfs · 54fd3149
      Dhinakaran Pandiyan authored 
      

Interrupts other than the one for AUX errors are required only for debug,
so unmask them via debugfs when the user requests debug.

User can make such a request with
echo 1 > <DEBUG_FS>/dri/0/i915_edp_psr_debug

There are no locks to serialize PSR debug enabling from
irq_postinstall() and debugfs for simplicity. As irq_postinstall() is
called only during module initialization/resume and IGT subtests
aren't expected to modify PSR debug at those times, we should be safe.

v2: Unroll loops (Ville)
    Avoid resetting error mask bits.

v3: Unmask interrupts in postinstall() if debug was still enabled.
    Avoid RMW (Ville)

v4: Avoid extra IMR write introduced in the previous version.(Jose)
    Style changes, renames (Jose).

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: default avatarDhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: default avatarJose Roberto de Souza <jose.souza@intel.com>
Signed-off-by: default avatarRodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180405013717.24254-1-dhinakaran.pandiyan@intel.com
      54fd3149
    • Ville Syrjälä's avatar
      drm/i915: Enable edp psr error interrupts on bdw+ · e04f7ece
      Ville Syrjälä authored 
      

Plug in the bdw+ irq handling for PSR interrupts. bdw+ supports psr on
any transcoder in theory, though the we don't currenty enable PSR except
on the EDP transcoder.

v2: From DK
 * Rebased on drm-tip
v3: Switched author to Ville based on IRC discussion.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: default avatarDhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: default avatarJose Roberto de Souza <jose.souza@intel.com>
Signed-off-by: default avatarRodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180403212420.25007-2-dhinakaran.pandiyan@intel.com
      e04f7ece
    • Daniel Vetter's avatar
      drm/i915: Enable edp psr error interrupts on hsw · fc340442
      Daniel Vetter authored 
      

The definitions for the error register should be valid on bdw/skl too,
but there we haven't even enabled DE_MISC handling yet.

Somewhat confusing the the moved register offset on bdw is only for
the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
on bdw.

v2: Fixes from Ville.

v3: From DK
 * Rebased on drm-tip
 * Removed BDW IIR bit definition, looks like an unintentional change that
should be in the following patch.

v4: From DK
 * Don't mask REG_WRITE.

References: bspec/11974 [SRD Interrupt Bit Definition DevHSW]
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Signed-off-by: default avatarDhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: default avatarJose Roberto de Souza <jose.souza@intel.com>
Signed-off-by: default avatarRodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180405220023.9449-1-dhinakaran.pandiyan@intel.com
      fc340442