Re: [Intel-gfx] [PATCH igt] igt/gem_softpin: Remove false dependencies on esoteric features
On Wed, Jan 20, 2016 at 06:49:49PM +, Belgaumkar, Vinay wrote:
> Hi Chris,
> These tests were developed for testing buffered SVM(using userptr
> and soft pinning API). I think Dan wanted me to rename the tests to
> gem_softpin, since they were being checked in as tests which
> validated the softpin kernel patches. Can we rename the existing
> tests to gem_buffered_svm or something similar, and then push any
> targeted softpin only tests as gem_softpin? We were hoping to use
> these userptr+softpin tests as GFT tests for SVM(Android) as well,
> since buffered SVM is POR for BXT Android.
I agree with Chris, there's no need to unecessarily mix together features.
When the api is designed in an orthogonal way, so should be the testing.
i915.ko is already a mindboggling complex beast, no need to make our lives
harder by making the tests use features that aren't strictly needed.
In the end applications and UMDs will of course use all these features
together, but that's why we do integration testing on top of just running
igt.
Can you please review Chris' patch?
Thanks, Daniel
>
> Thanks,
> Vinay.
>
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
> Chris Wilson
> Sent: Thursday, January 14, 2016 3:03 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH igt] igt/gem_softpin: Remove false dependencies
> on esoteric features
>
> For softpinning, we do not require either userptr or extended ppgtt, so
> remove those requirements and make the tests work universally. (Certain ABI
> tests require large GTT, or per-process GTT.)
>
> In the process, make the tests more extensive - validate overlapping handling
> more careful, explicitly test no-relocation support, validate more ABI
> handling. And for fun, cause a kernel GPF.
>
> Signed-off-by: Chris Wilson
> ---
> tests/gem_softpin.c | 1313
> +--
> 1 file changed, 324 insertions(+), 989 deletions(-)
>
> diff --git a/tests/gem_softpin.c b/tests/gem_softpin.c index 1cbde4e..f188559
> 100644
> --- a/tests/gem_softpin.c
> +++ b/tests/gem_softpin.c
> @@ -26,80 +26,10 @@
> *
> */
>
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include "drm.h"
> -#include "ioctl_wrappers.h"
> -#include "drmtest.h"
> -#include "intel_chipset.h"
> -#include "intel_io.h"
> -#include "i915_drm.h"
> -#include
> -#include
> -#include
> -#include
> -#include "igt_kms.h"
> -#include
> -#include
> -#include
> -
> -#define BO_SIZE 4096
> -#define MULTIPAGE_BO_SIZE 2 * BO_SIZE
> -#define STORE_BATCH_BUFFER_SIZE 4
> +#include "igt.h"
> +
> #define EXEC_OBJECT_PINNED (1<<4)
> #define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3) -#define SHARED_BUFFER_SIZE
> 4096
> -
> -typedef struct drm_i915_gem_userptr i915_gem_userptr;
> -
> -static uint32_t init_userptr(int fd, i915_gem_userptr *, void *ptr, uint64_t
> size); -static void *create_mem_buffer(uint64_t size); -static int
> gem_call_userptr_ioctl(int fd, i915_gem_userptr *userptr); -static void
> gem_pin_userptr_test(void); -static void gem_pin_bo_test(void); -static void
> gem_pin_invalid_vma_test(bool test_decouple_flags, bool
> test_canonical_offset); -static void gem_pin_overlap_test(void); -static void
> gem_pin_high_address_test(void);
> -
> -#define NO_PPGTT 0
> -#define ALIASING_PPGTT 1
> -#define FULL_32_BIT_PPGTT 2
> -#define FULL_48_BIT_PPGTT 3
> -/* uses_full_ppgtt
> - * Finds supported PPGTT details.
> - * @fd DRM fd
> - * @min can be
> - * 0 - No PPGTT
> - * 1 - Aliasing PPGTT
> - * 2 - Full PPGTT (32b)
> - * 3 - Full PPGTT (48b)
> - * RETURNS true/false if min support is present -*/ -static bool
> uses_full_ppgtt(int fd, int min) -{
> - struct drm_i915_getparam gp;
> - int val = 0;
> -
> - memset(&gp, 0, sizeof(gp));
> - gp.param = 18; /* HAS_ALIASING_PPGTT */
> - gp.value = &val;
> -
> - if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
> - return 0;
> -
> - errno = 0;
> - return val >= min;
> -}
>
> /* has_softpin_support
> * Finds if softpin feature is supported @@ -121,83 +51,6 @@ static bool
> has_softpin_support(int fd)
> return (val == 1);
> }
>
> -/* gem_call_userptr_ioctl
> - * Helper to call ioctl - TODO: move to lib
> - * @fd - drm fd
> - * @userptr - pointer to initialised userptr
> - * RETURNS status of ioctl call
> -*/
> -static int gem_call_userptr_ioctl(int fd, i915_gem_userptr *userptr) -{
> - int ret;
> -
> - ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_USERPTR, userptr);
> -
> - if (ret)
> - ret = errno;
> -
> - return ret;
> -}
> -
> -/* init_userptr
> - * Helper that inits userptr an returns handle
> - * @fd - drm fd
> - * @userptr - pointer to empty userptr
> - * @ptr - buffer to be shared
> - * @size - size of buffer
> - * @ro -
Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for improve handling of the driver's default (kernel) context
On Wed, Jan 20, 2016 at 05:31:00PM +, Dave Gordon wrote: > On 20/01/16 07:49, Patchwork wrote: > >== Summary == > > > >Built on 7d26528d30b0d8119c858115b6e5e22deb74ec71 drm-intel-nightly: > >2016y-01m-19d-19h-38m-52s UTC integration manifest > > > >Test gem_storedw_loop: > > Subgroup basic-render: > > pass -> DMESG-WARN (bdw-nuci7) UNSTABLE > > https://bugs.freedesktop.org/show_bug.cgi?id=93693 > [BAT SKL BDW] missed interrupt in gem_storedw_loop/basic-render with *ERROR* > Hangcheck timer elapsed... > > >Test kms_flip: > > Subgroup basic-flip-vs-wf_vblank: > > dmesg-warn -> PASS (ilk-hp8440p) > >Test kms_pipe_crc_basic: > > Subgroup hang-read-crc-pipe-a: > > pass -> DMESG-WARN (ilk-hp8440p) > > https://bugs.freedesktop.org/show_bug.cgi?id=93787 > "[BAT ILK] sporadic fifo underruns" > > The two 'dmesg-warn' -> PASS changes are similar cases where the underrun > just happened to occur in the reference run and not in this patch test. > > Also compare Bug 93640 - [BAT ILK SNB IVB bisected] Fifo underruns since > CI_DRM_952 > > I'd consider marking these tests as UNSTABLE on ILK for now. > > (Although, this was against CI_DRM_987, and it looks like maybe it's fixed > after CI_DRM_989, at least on ILK). It's jumping around all over right now, which is why we can't really mark it as unstable. I thought this was just fallout from Matt breaking watermarks on ilk-ivb, but reverting that patch uncovered that we have some other fifo underruns still (or again, was 2 weeks without coverage due to Matt's regression) happening. I'm looking into this, if I can coax my ilk into coperation. -Daniel > > .Dave. > > > Subgroup read-crc-pipe-b: > > dmesg-warn -> PASS (ilk-hp8440p) > > > >bdw-nuci7total:140 pass:130 dwarn:1 dfail:0 fail:0 skip:9 > >bdw-ultratotal:140 pass:132 dwarn:1 dfail:1 fail:0 skip:6 > >byt-nuc total:143 pass:125 dwarn:3 dfail:0 fail:0 skip:15 > >hsw-brixbox total:143 pass:136 dwarn:0 dfail:0 fail:0 skip:7 > >hsw-gt2 total:143 pass:139 dwarn:0 dfail:0 fail:0 skip:4 > >ilk-hp8440p total:143 pass:102 dwarn:3 dfail:0 fail:0 skip:38 > >skl-i5k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 > >skl-i7k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 > >snb-dellxps total:143 pass:124 dwarn:5 dfail:0 fail:0 skip:14 > >snb-x220ttotal:143 pass:124 dwarn:5 dfail:0 fail:1 skip:13 > > > >Results at /archive/results/CI_IGT_test/Patchwork_1225/ > > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 0/3] improve handling of the driver's default (kernel) context
On Tue, Jan 19, 2016 at 07:02:52PM +, Dave Gordon wrote: > During startup, the driver creates a unique "intel_context" that will > provide a home for orphan requests (i.e. those generated by the driver > internally, not associated with user batchbuffers). > > However, one of the infelicities of the current code is that the driver > keeps a per-engine pointer to the default context for that engine (this > is probably from a decision made early in execlists implementation, > when it was thought that a context was engine-specific, and not revised > when it was decided that an "intel_context" represents a multiplex of > engine-level contexts). All these per-engine pointers actually end up > pointing to the same unique object. > > To compound this, the refcounting is bogus; the driver holds just one > reference to the default context, even though there are multiple pointers > to it (RCS is considered to hold this "on behalf of" the other engines, > but this can lead to problems during unload as it makes the code sensitive > to the order of engine teardown -- RCS should be done last, but it isn't). > > Also, some of the execlists batch submission code treats the default > context differently from any other; testing for it by comparing against > the per-engine pointer is quite clumsy, especially in debugfs where > contexts are enumerated from a global list and therefore not automatically > known to be associated with a particular engine. > > Therefore, we should replace the per-engine pointers with a single > driver-wide reference, which will make the refcounting sane and simplify > the code that uses this context for non-user-related requests. > > THIS patchset does NOT address the execlists code's special handling of > the default context, but it will simplify the planned future cleanup of > that code. > > v3: don't flag the context created during startup (and fully instantiated > on all engines) for driver-internal purposes as "is_global_default", > 1448630935-27377-1-git-send-email-ch...@chris-wilson.co.uk notwithstanding. > (We now call it "the kernel context", and compare against the device-wide > pointer). [Chris Wilson] > > v4: Rebased entire series vacuumed up, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/2 v2] lib/ioctl_wrappers: Add gem_gtt_type exposing raw HAS_ALIASING_PPGTT param
No functional changes.
While I'm here, let's also rename gem_uses_aliasing_ppgtt (since it's
being used to indicate if we are using ANY kind of ppgtt) and introduce
gem_uses_full_ppgtt to drop some unnecessary code from tests that were
previously calling getparam directly instead of using ioctl wrapper.
v2: drop gem_uses_full_48b_ppgtt since it's no longer used anywhere,
s/48b/64b (Chris)
Cc: Chris Wilson
Signed-off-by: Michał Winiarski
---
lib/ioctl_wrappers.c | 45 +---
lib/ioctl_wrappers.h | 4 +++-
tests/drv_hangman.c | 2 +-
tests/gem_bad_reloc.c| 11 +-
tests/gem_ctx_thrash.c | 20 ++
tests/gem_exec_parse.c | 2 +-
tests/gem_ppgtt.c| 20 ++
tests/gem_storedw_batches_loop.c | 2 +-
tests/gem_storedw_loop.c | 2 +-
tests/pm_rps.c | 2 +-
tests/pm_sseu.c | 2 +-
11 files changed, 51 insertions(+), 61 deletions(-)
diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 0024898..b534b03 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -924,18 +924,18 @@ bool gem_bo_busy(int fd, uint32_t handle)
/* feature test helpers */
/**
- * gem_uses_aliasing_ppgtt:
+ * gem_gtt_type:
* @fd: open i915 drm file descriptor
*
- * Feature test macro to check whether the kernel internally uses ppgtt to
- * execute batches. The /aliasing/ in the function name is a bit a misnomer,
- * this driver parameter is also true when full ppgtt address spaces are
- * available since for batchbuffer construction only ppgtt or global gtt is
- * relevant.
+ * Feature test macro to check what type of gtt is being used by the kernel:
+ * 0 - global gtt
+ * 1 - aliasing ppgtt
+ * 2 - full ppgtt, limited to 32bit address space
+ * 3 - full ppgtt, 64bit address space
*
- * Returns: Whether batches are run through ppgtt.
+ * Returns: Type of gtt being used.
*/
-bool gem_uses_aliasing_ppgtt(int fd)
+int gem_gtt_type(int fd)
{
struct drm_i915_getparam gp;
int val = 0;
@@ -952,6 +952,35 @@ bool gem_uses_aliasing_ppgtt(int fd)
}
/**
+ * gem_uses_ppgtt:
+ * @fd: open i915 drm file descriptor
+ *
+ * Feature test macro to check whether the kernel internally uses ppgtt to
+ * execute batches. Note that this is also true when we're using full ppgtt.
+ *
+ * Returns: Whether batches are run through ppgtt.
+ */
+bool gem_uses_ppgtt(int fd)
+{
+ return gem_gtt_type(fd) > 0;
+}
+
+/**
+ * gem_uses_full_ppgtt:
+ * @fd: open i915 drm file descriptor
+ *
+ * Feature test macro to check whether the kernel internally uses full
+ * per-process gtt to execute batches. Note that this is also true when we're
+ * using full 64b ppgtt.
+ *
+ * Returns: Whether batches are run through full ppgtt.
+ */
+bool gem_uses_full_ppgtt(int fd)
+{
+ return gem_gtt_type(fd) > 1;
+}
+
+/**
* gem_available_fences:
* @fd: open i915 drm file descriptor
*
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index 214ec78..a6bf700 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -125,7 +125,9 @@ bool gem_has_bsd(int fd);
bool gem_has_blt(int fd);
bool gem_has_vebox(int fd);
bool gem_has_bsd2(int fd);
-bool gem_uses_aliasing_ppgtt(int fd);
+int gem_gtt_type(int fd);
+bool gem_uses_ppgtt(int fd);
+bool gem_uses_full_ppgtt(int fd);
int gem_available_fences(int fd);
uint64_t gem_available_aperture_size(int fd);
uint64_t gem_aperture_size(int fd);
diff --git a/tests/drv_hangman.c b/tests/drv_hangman.c
index cd63b97..1498ddf 100644
--- a/tests/drv_hangman.c
+++ b/tests/drv_hangman.c
@@ -358,7 +358,7 @@ static bool uses_cmd_parser(int fd, int gen)
if (rc || parser_version == 0)
return false;
- if (!gem_uses_aliasing_ppgtt(fd))
+ if (!gem_uses_ppgtt(fd))
return false;
if (gen != 7)
diff --git a/tests/gem_bad_reloc.c b/tests/gem_bad_reloc.c
index e8701da..a9146e2 100644
--- a/tests/gem_bad_reloc.c
+++ b/tests/gem_bad_reloc.c
@@ -46,16 +46,7 @@ IGT_TEST_DESCRIPTION("Simulates SNA behaviour using negative
self-relocations"
static uint64_t get_page_table_size(int fd)
{
- struct drm_i915_getparam gp;
- int val = 0;
-
- memset(&gp, 0, sizeof(gp));
- gp.param = 18; /* HAS_ALIASING_PPGTT */
- gp.value = &val;
-
- if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
- return 0;
- errno = 0;
+ int val = gem_gtt_type(fd);
switch (val) {
case 0:
diff --git a/tests/gem_ctx_thrash.c b/tests/gem_ctx_thrash.c
index acfa8f5..ff752b7 100644
--- a/tests/gem_ctx_thrash.c
+++ b/tests/gem_ctx_thrash.c
@@ -117,22 +117,6 @@ static void *thread(void *bufmgr)
return NULL;
}
-static int uses_ppgtt(int _fd)
-{
- struct drm_i915_getparam gp;
- int val = 0;
-
- memset(&gp, 0, sizeof(gp));
- gp.param = 18; /* HAS_ALIASING_PPGTT */
-
Re: [Intel-gfx] [PATCH] drm/i915: Do not put big intel_crtc_state on the stack
On Tue, 19 Jan 2016, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Having this on stack triggers the -Wframe-larger-than=1024 and
> is not nice to put such big things on the kernel stack anyway.
>
> This required a little bit of refactoring to handle the new
> failure path from vlv_force_pll_on.
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Daniel Vetter
> Cc: John Harrison
> Cc: Ville Syrjälä
> ---
> Compile tested only!
> ---
> drivers/gpu/drm/i915/intel_display.c | 58
> +++-
> drivers/gpu/drm/i915/intel_dp.c | 8 +++--
> drivers/gpu/drm/i915/intel_drv.h | 4 +--
> 3 files changed, 45 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index ccb3e3f47450..7bf18658c659 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7635,26 +7635,34 @@ static void chv_prepare_pll(struct intel_crtc *crtc,
> * in cases where we need the PLL enabled even when @pipe is not going to
> * be enabled.
> */
> -void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> +int vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
> const struct dpll *dpll)
> {
> struct intel_crtc *crtc =
> to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> - struct intel_crtc_state pipe_config = {
> - .base.crtc = &crtc->base,
> - .pixel_multiplier = 1,
> - .dpll = *dpll,
> - };
> + struct intel_crtc_state *pipe_config;
> +
> + pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
> + if (!pipe_config)
> + return -ENOMEM;
> +
> + pipe_config->base.crtc = &crtc->base;
> + pipe_config->pixel_multiplier = 1;
> + pipe_config->dpll = *dpll;
>
> if (IS_CHERRYVIEW(dev)) {
> - chv_compute_dpll(crtc, &pipe_config);
> - chv_prepare_pll(crtc, &pipe_config);
> - chv_enable_pll(crtc, &pipe_config);
> + chv_compute_dpll(crtc, pipe_config);
> + chv_prepare_pll(crtc, pipe_config);
> + chv_enable_pll(crtc, pipe_config);
> } else {
> - vlv_compute_dpll(crtc, &pipe_config);
> - vlv_prepare_pll(crtc, &pipe_config);
> - vlv_enable_pll(crtc, &pipe_config);
> + vlv_compute_dpll(crtc, pipe_config);
> + vlv_prepare_pll(crtc, pipe_config);
> + vlv_enable_pll(crtc, pipe_config);
> }
> +
> + kfree(pipe_config);
> +
> + return 0;
> }
>
> /**
> @@ -10828,7 +10836,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct
> drm_device *dev,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> struct drm_display_mode *mode;
> - struct intel_crtc_state pipe_config;
> + struct intel_crtc_state *pipe_config;
> int htot = I915_READ(HTOTAL(cpu_transcoder));
> int hsync = I915_READ(HSYNC(cpu_transcoder));
> int vtot = I915_READ(VTOTAL(cpu_transcoder));
> @@ -10839,6 +10847,12 @@ struct drm_display_mode *intel_crtc_mode_get(struct
> drm_device *dev,
> if (!mode)
> return NULL;
>
> + pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
GFP_TEMPORARY
BR,
Jani.
> + if (!pipe_config) {
> + kfree(mode);
> + return NULL;
> + }
> +
> /*
>* Construct a pipe_config sufficient for getting the clock info
>* back out of crtc_clock_get.
> @@ -10846,14 +10860,14 @@ struct drm_display_mode *intel_crtc_mode_get(struct
> drm_device *dev,
>* Note, if LVDS ever uses a non-1 pixel multiplier, we'll need
>* to use a real value here instead.
>*/
> - pipe_config.cpu_transcoder = (enum transcoder) pipe;
> - pipe_config.pixel_multiplier = 1;
> - pipe_config.dpll_hw_state.dpll = I915_READ(DPLL(pipe));
> - pipe_config.dpll_hw_state.fp0 = I915_READ(FP0(pipe));
> - pipe_config.dpll_hw_state.fp1 = I915_READ(FP1(pipe));
> - i9xx_crtc_clock_get(intel_crtc, &pipe_config);
> -
> - mode->clock = pipe_config.port_clock / pipe_config.pixel_multiplier;
> + pipe_config->cpu_transcoder = (enum transcoder) pipe;
> + pipe_config->pixel_multiplier = 1;
> + pipe_config->dpll_hw_state.dpll = I915_READ(DPLL(pipe));
> + pipe_config->dpll_hw_state.fp0 = I915_READ(FP0(pipe));
> + pipe_config->dpll_hw_state.fp1 = I915_READ(FP1(pipe));
> + i9xx_crtc_clock_get(intel_crtc, pipe_config);
> +
> + mode->clock = pipe_config->port_clock / pipe_config->pixel_multiplier;
> mode->hdisplay = (htot & 0x) + 1;
> mode->htotal = ((htot & 0x) >> 16) + 1;
> mode->hsync_start = (hsync & 0x) + 1;
> @@ -10865,6 +10879,8 @@ struct drm_display_mode *intel_crtc_mode_get(struct
> drm_device *dev,
>
> drm_mode_set_name(mode);
>
> + kfree(pipe_conf
Re: [Intel-gfx] [PATCH] drm/i915: Tune down "GT register while GT waking disabled" message
Mika Kuoppala writes:
> Daniel Vetter writes:
>
>> We've had this since forever, and's randomly reporting issues and as
>> such causing piles&piles of CI noise. Mika is working on proper debug
>> infrastructure for this, and on fixing this properly.
>>
>> Meanwhile make CI more useful for everyone else.
>>
>> Cc: Mika Kuoppala
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93121
>> Signed-off-by: Daniel Vetter
>
> We should have soon way to do research on this without
> harming ci/bat. So for denoising:
>
> Reviewed-by: Mika Kuoppala
>
Applied to dinq. Thanks for patch.
-Mika
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> b/drivers/gpu/drm/i915/i915_drv.c
>> index 760e0ce4aa26..f1cab9f131ae 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1384,7 +1384,7 @@ static void vlv_check_no_gt_access(struct
>> drm_i915_private *dev_priv)
>> if (!(I915_READ(VLV_GTLC_PW_STATUS) & VLV_GTLC_ALLOWWAKEERR))
>> return;
>>
>> -DRM_ERROR("GT register access while GT waking disabled\n");
>> +DRM_DEBUG_DRIVER("GT register access while GT waking disabled\n");
>> I915_WRITE(VLV_GTLC_PW_STATUS, VLV_GTLC_ALLOWWAKEERR);
>> }
>>
>> --
>> 2.7.0.rc3
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Limit the auto arming of mmio debugs on vlv/chv
Daniel Vetter writes:
> On Wed, Jan 20, 2016 at 12:32:23PM +0200, Mika Kuoppala wrote:
>> The capability to detect unclaimed register access was
>> recently introduced for vlv/chv platforms. Apparently
>> there are plenty of unclaimed access on these platforms,
>> resulting in new dmesg warns. But as we are trying to form
>> a beachhead for CI/Bat, all new warns are adding to the
>> noise and thus not desirable at this point in time.
>>
>> Make it so that if in these platforms the automatic arming
>> was responsible for mmio_debug enabling, ignore the warns.
>>
>> If user/dev wants to fix these, he can still do so by
>> i915.mmio_debug=1234.
>>
>> Cc: Daniel Vetter
>> Signed-off-by: Mika Kuoppala
>
> Yeah it sucks, but otoh this will make CI more useful for everyone else
> and we can keep the original test coverage even.
>
> Reviewed-by: Daniel Vetter
Applied to dinq. Thanks for review.
-Mika
>
>> ---
>> drivers/gpu/drm/i915/intel_uncore.c | 9 +
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
>> b/drivers/gpu/drm/i915/intel_uncore.c
>> index c3c13dc929cb..bfa79e5c214e 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -631,6 +631,15 @@ __unclaimed_reg_debug(struct drm_i915_private *dev_priv,
>>const bool read,
>>const bool before)
>> {
>> +/* XXX. We limit the auto arming traces for mmio
>> + * debugs on these platforms. There are just too many
>> + * revealed by these and CI/Bat suffers from the noise.
>> + * Please fix and then re-enable the automatic traces.
>> + */
>> +if (i915.mmio_debug < 2 &&
>> +(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
>> +return;
>> +
>> if (WARN(check_for_unclaimed_mmio(dev_priv),
>> "Unclaimed register detected %s %s register 0x%x\n",
>> before ? "before" : "after",
>> --
>> 2.5.0
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Make LRC pinning own a reference to the context
From: Tvrtko Ursulin Will simplify the following fix and sounds logical. v2: Add some whitespace to separate logic better. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Reviewed-by: Chris Wilson Cc: Chris Wilson Cc: Nick Hoath --- drivers/gpu/drm/i915/intel_lrc.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index c1376fed30a5..b35788ee2f83 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1103,6 +1103,8 @@ static int intel_lr_context_pin(struct intel_context *ctx, ret = intel_lr_context_do_pin(ctx, engine); if (ret) goto reset_pin_count; + + i915_gem_context_reference(ctx); } return ret; @@ -1128,6 +1130,8 @@ void intel_lr_context_unpin(struct intel_context *ctx, ctx->engine[engine->id].lrc_vma = NULL; ctx->engine[engine->id].lrc_desc = 0; ctx->engine[engine->id].lrc_reg_state = NULL; + + i915_gem_context_unreference(ctx); } } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/3] drm/i915: Make LRC (un)pinning work on context and engine
From: Tvrtko Ursulin
Previously intel_lr_context_(un)pin were operating on requests
which is in conflict with their names.
If we make them take a context and an engine, it makes the names
make more sense and it also makes future fixes possible.
v2: Rebase for default_context/kernel_context change.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Chris Wilson
Cc: Chris Wilson
Cc: Nick Hoath
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/intel_lrc.c | 49
drivers/gpu/drm/i915/intel_lrc.h | 3 ++-
3 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6a3e4ee7f7e2..864fe2382875 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2680,7 +2680,7 @@ void i915_gem_request_free(struct kref *req_ref)
if (ctx) {
if (i915.enable_execlists && ctx != req->i915->kernel_context)
- intel_lr_context_unpin(req);
+ intel_lr_context_unpin(ctx, req->ring);
i915_gem_context_unreference(ctx);
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 134379dc4dd9..c1376fed30a5 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -225,7 +225,8 @@ enum {
#define GEN8_CTX_ID_SHIFT 32
#define CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT 0x17
-static int intel_lr_context_pin(struct drm_i915_gem_request *rq);
+static int intel_lr_context_pin(struct intel_context *ctx,
+ struct intel_engine_cs *engine);
static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
struct drm_i915_gem_object *default_ctx_obj);
@@ -599,7 +600,7 @@ static int execlists_context_queue(struct
drm_i915_gem_request *request)
int num_elements = 0;
if (request->ctx != request->i915->kernel_context)
- intel_lr_context_pin(request);
+ intel_lr_context_pin(request->ctx, ring);
i915_gem_request_reference(request);
@@ -704,7 +705,7 @@ int intel_logical_ring_alloc_request_extras(struct
drm_i915_gem_request *request
}
if (request->ctx != request->i915->kernel_context)
- ret = intel_lr_context_pin(request);
+ ret = intel_lr_context_pin(request->ctx, request->ring);
return ret;
}
@@ -1004,7 +1005,8 @@ void intel_execlists_retire_requests(struct
intel_engine_cs *ring)
ctx->engine[ring->id].state;
if (ctx_obj && (ctx != req->i915->kernel_context))
- intel_lr_context_unpin(req);
+ intel_lr_context_unpin(ctx, ring);
+
list_del(&req->execlist_link);
i915_gem_request_unreference(req);
}
@@ -1048,8 +1050,8 @@ int logical_ring_flush_all_caches(struct
drm_i915_gem_request *req)
return 0;
}
-static int intel_lr_context_do_pin(struct intel_engine_cs *ring,
- struct intel_context *ctx)
+static int intel_lr_context_do_pin(struct intel_context *ctx,
+ struct intel_engine_cs *ring)
{
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1092,41 +1094,40 @@ unpin_ctx_obj:
return ret;
}
-static int intel_lr_context_pin(struct drm_i915_gem_request *rq)
+static int intel_lr_context_pin(struct intel_context *ctx,
+ struct intel_engine_cs *engine)
{
int ret = 0;
- struct intel_engine_cs *ring = rq->ring;
- if (rq->ctx->engine[ring->id].pin_count++ == 0) {
- ret = intel_lr_context_do_pin(ring, rq->ctx);
+ if (ctx->engine[engine->id].pin_count++ == 0) {
+ ret = intel_lr_context_do_pin(ctx, engine);
if (ret)
goto reset_pin_count;
}
return ret;
reset_pin_count:
- rq->ctx->engine[ring->id].pin_count = 0;
+ ctx->engine[engine->id].pin_count = 0;
return ret;
}
-void intel_lr_context_unpin(struct drm_i915_gem_request *rq)
+void intel_lr_context_unpin(struct intel_context *ctx,
+ struct intel_engine_cs *engine)
{
- struct intel_engine_cs *ring = rq->ring;
- struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
- struct intel_ringbuffer *ringbuf = rq->ringbuf;
+ struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
- WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
+ WARN_ON(!mutex_is_locked(&engine->dev->struct_mutex));
- if (!ctx_obj)
+ if (WARN_ON_ONCE(!ctx_obj))
return;
- if (--rq->ctx->engine[ring->id].pin_count == 0) {
- kunmap(kmap_to_page(rq->ctx->engine[ring->id].lrc_reg_state));
-
[Intel-gfx] [PATCH v3 3/3] drm/i915: Fix premature LRC unpin in GuC mode
From: Tvrtko Ursulin
In GuC mode LRC pinning lifetime depends exclusively on the
request liftime. Since that is terminated by the seqno update
that opens up a race condition between GPU finishing writing
out the context image and the driver unpinning the LRC.
To extend the LRC lifetime we will employ a similar approach
to what legacy ringbuffer submission does.
We will start tracking the last submitted context per engine
and keep it pinned until it is replaced by another one.
Note that the driver unload path is a bit fragile and could
benefit greatly from efforts to unify the legacy and exec
list submission code paths.
At the moment i915_gem_context_fini has special casing for the
two which are potentialy not needed, and also depends on
i915_gem_cleanup_ringbuffer running before itself.
v2:
* Move pinning into engine->emit_request and actually fix
the reference/unreference logic. (Chris Wilson)
* ring->dev can be NULL on driver unload so use a different
route towards it.
v3:
* Rebase.
* Handle the reset path. (Chris Wilson)
* Exclude default context from the pinning - it is impossible
to get it right before default context special casing in
general is eliminated.
Signed-off-by: Tvrtko Ursulin
Issue: VIZ-4277
Cc: Chris Wilson
Cc: Nick Hoath
---
drivers/gpu/drm/i915/i915_gem_context.c | 15 +++
drivers/gpu/drm/i915/intel_lrc.c| 13 -
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 6a4f64b03db6..47870b463bbd 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -341,10 +341,14 @@ void i915_gem_context_reset(struct drm_device *dev)
struct intel_context *lctx = ring->last_context;
if (lctx) {
- if (lctx->legacy_hw_ctx.rcs_state && i == RCS)
-
i915_gem_object_ggtt_unpin(lctx->legacy_hw_ctx.rcs_state);
+ if (!i915.enable_execlists) {
+ if (lctx->legacy_hw_ctx.rcs_state && i == RCS)
+
i915_gem_object_ggtt_unpin(lctx->legacy_hw_ctx.rcs_state);
- i915_gem_context_unreference(lctx);
+ i915_gem_context_unreference(lctx);
+ } else {
+ intel_lr_context_unpin(lctx, ring);
+ }
ring->last_context = NULL;
}
}
@@ -432,7 +436,10 @@ void i915_gem_context_fini(struct drm_device *dev)
struct intel_engine_cs *ring = &dev_priv->ring[i];
if (ring->last_context) {
- i915_gem_context_unreference(ring->last_context);
+ if (i915.enable_execlists)
+ intel_lr_context_unpin(ring->last_context,
ring);
+ else
+
i915_gem_context_unreference(ring->last_context);
ring->last_context = NULL;
}
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b35788ee2f83..8619415f60c7 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1118,7 +1118,7 @@ void intel_lr_context_unpin(struct intel_context *ctx,
{
struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
- WARN_ON(!mutex_is_locked(&engine->dev->struct_mutex));
+ WARN_ON(!mutex_is_locked(&ctx->i915->dev->struct_mutex));
if (WARN_ON_ONCE(!ctx_obj))
return;
@@ -1857,6 +1857,17 @@ static int gen8_emit_request(struct drm_i915_gem_request
*request)
u32 cmd;
int ret;
+ if (ring->last_context != request->ctx) {
+ if (ring->last_context)
+ intel_lr_context_unpin(ring->last_context, ring);
+ if (request->ctx != request->i915->kernel_context) {
+ intel_lr_context_pin(request->ctx, ring);
+ ring->last_context = request->ctx;
+ } else {
+ ring->last_context = NULL;
+ }
+ }
+
/*
* Reserve space for 2 NOOPs at the end of each request to be
* used as a workaround for not being allowed to do lite
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 1/2 v2] lib/ioctl_wrappers: Add gem_gtt_type exposing raw HAS_ALIASING_PPGTT param
On Thu, Jan 21, 2016 at 10:27:26AM +0100, Michał Winiarski wrote: > No functional changes. > While I'm here, let's also rename gem_uses_aliasing_ppgtt (since it's > being used to indicate if we are using ANY kind of ppgtt) and introduce > gem_uses_full_ppgtt to drop some unnecessary code from tests that were > previously calling getparam directly instead of using ioctl wrapper. > > v2: drop gem_uses_full_48b_ppgtt since it's no longer used anywhere, > s/48b/64b (Chris) > > Cc: Chris Wilson > Signed-off-by: Michał Winiarski Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Use ordered seqno write interrupt generation on gen8+ execlists
Patchwork writes: > == Summary == > > Built on e9545b0b60b73d8be3d41048af5b8f2c1e2fc4c1 drm-intel-nightly: > 2016y-01m-20d-13h-55m-37s UTC integration manifest > > Test gem_storedw_loop: > Subgroup basic-render: > dmesg-warn -> PASS (bdw-ultra) UNSTABLE > Test gem_sync: > Subgroup basic-render: > dmesg-fail -> PASS (bdw-ultra) UNSTABLE > Test kms_flip: > Subgroup basic-flip-vs-dpms: > pass -> DMESG-WARN (ilk-hp8440p) > Subgroup basic-flip-vs-modeset: > pass -> DMESG-WARN (ilk-hp8440p) I broadened the scope of: https://bugs.freedesktop.org/show_bug.cgi?id=93787 to include vs-modeset too as it is same fifo underrun warning. Applied to dinq. Thanks for patch. -Mika > Test kms_pipe_crc_basic: > Subgroup read-crc-pipe-a-frame-sequence: > dmesg-warn -> PASS (skl-i5k-2) > > bdw-nuci7total:143 pass:134 dwarn:0 dfail:0 fail:0 skip:9 > bdw-ultratotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 > bsw-nuc-2total:143 pass:117 dwarn:2 dfail:0 fail:0 skip:24 > byt-nuc total:143 pass:125 dwarn:3 dfail:0 fail:0 skip:15 > hsw-brixbox total:143 pass:136 dwarn:0 dfail:0 fail:0 skip:7 > hsw-gt2 total:143 pass:139 dwarn:0 dfail:0 fail:0 skip:4 > ilk-hp8440p total:143 pass:102 dwarn:3 dfail:0 fail:0 skip:38 > ivb-t430stotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 > skl-i5k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 > skl-i7k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 > snb-dellxps total:143 pass:129 dwarn:0 dfail:0 fail:0 skip:14 > snb-x220ttotal:143 pass:129 dwarn:0 dfail:0 fail:1 skip:13 > > Results at /archive/results/CI_IGT_test/Patchwork_1233/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Decouple execbuf uAPI from internal implementation
On Fri, Jan 15, 2016 at 03:12:50PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> At the moment execbuf ring selection is fully coupled to
> internal ring ids which is not a good thing on its own.
>
> This dependency is also spread between two source files and
> not spelled out at either side which makes it hidden and
> fragile.
>
> This patch decouples this dependency by introducing an explicit
> translation table of execbuf uAPI to ring id close to the only
> call site (i915_gem_do_execbuffer).
>
> This way we are free to change driver internal implementation
> details without breaking userspace. All state relating to the
> uAPI is now contained in, or next to, i915_gem_do_execbuffer.
>
> As a side benefit, this patch decreases the compiled size
> of i915_gem_do_execbuffer.
>
> v2: Extract ring selection into eb_select_ring. (Chris Wilson)
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Daniel Vetter
> Cc: Chris Wilson
Yeah, this decoupling is nice, after all the point of include/uapi was to
make the uapi vs. internal headers clear.
Acked-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_drv.h| 4 +-
> drivers/gpu/drm/i915/i915_gem.c| 2 +
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 139
> +++--
> drivers/gpu/drm/i915/intel_ringbuffer.h| 10 +--
> 4 files changed, 81 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index eb7bb97f7316..35d5d6099a44 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -334,7 +334,7 @@ struct drm_i915_file_private {
> unsigned boosts;
> } rps;
>
> - struct intel_engine_cs *bsd_ring;
> + unsigned int bsd_ring;
> };
>
> enum intel_dpll_id {
> @@ -1300,7 +1300,7 @@ struct i915_gem_mm {
> bool busy;
>
> /* the indicator for dispatch video commands on two BSD rings */
> - int bsd_ring_dispatch_index;
> + unsigned int bsd_ring_dispatch_index;
>
> /** Bit 6 swizzling required for X tiling */
> uint32_t bit_6_swizzle_x;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ddc21d4b388d..26e6842f2df3 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5112,6 +5112,8 @@ int i915_gem_open(struct drm_device *dev, struct
> drm_file *file)
> spin_lock_init(&file_priv->mm.lock);
> INIT_LIST_HEAD(&file_priv->mm.request_list);
>
> + file_priv->bsd_ring = -1;
> +
> ret = i915_gem_context_open(dev, file);
> if (ret)
> kfree(file_priv);
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index d469c4779ff5..497a2f87836c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1328,33 +1328,23 @@ i915_gem_ringbuffer_submission(struct
> i915_execbuffer_params *params,
>
> /**
> * Find one BSD ring to dispatch the corresponding BSD command.
> - * The Ring ID is returned.
> + * The ring index is returned.
> */
> -static int gen8_dispatch_bsd_ring(struct drm_device *dev,
> - struct drm_file *file)
> +static unsigned int
> +gen8_dispatch_bsd_ring(struct drm_i915_private *dev_priv, struct drm_file
> *file)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_i915_file_private *file_priv = file->driver_priv;
>
> - /* Check whether the file_priv is using one ring */
> - if (file_priv->bsd_ring)
> - return file_priv->bsd_ring->id;
> - else {
> - /* If no, use the ping-pong mechanism to select one ring */
> - int ring_id;
> -
> - mutex_lock(&dev->struct_mutex);
> - if (dev_priv->mm.bsd_ring_dispatch_index == 0) {
> - ring_id = VCS;
> - dev_priv->mm.bsd_ring_dispatch_index = 1;
> - } else {
> - ring_id = VCS2;
> - dev_priv->mm.bsd_ring_dispatch_index = 0;
> - }
> - file_priv->bsd_ring = &dev_priv->ring[ring_id];
> - mutex_unlock(&dev->struct_mutex);
> - return ring_id;
> + /* Check whether the file_priv has already selected one ring. */
> + if ((int)file_priv->bsd_ring < 0) {
> + /* If not, use the ping-pong mechanism to select one. */
> + mutex_lock(&dev_priv->dev->struct_mutex);
> + file_priv->bsd_ring = dev_priv->mm.bsd_ring_dispatch_index;
> + dev_priv->mm.bsd_ring_dispatch_index ^= 1;
> + mutex_unlock(&dev_priv->dev->struct_mutex);
> }
> +
> + return file_priv->bsd_ring;
> }
>
> static struct drm_i915_gem_object *
> @@ -1377,6 +1367,63 @@ eb_get_batch(struct eb_vmas *eb)
> return vma->obj;
> }
>
> +#define I915_USER_RINGS (4)
> +
> +static const enu
Re: [Intel-gfx] [PATCH v2] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
On Fri, Jan 15, 2016 at 04:51:46PM +, Chris Wilson wrote:
> Tvrtko was looking through the execbuffer-ioctl and noticed that the
> uABI was tightly coupled to our internal engine identifiers. Close
> inspection also revealed that we leak those internal engine identifiers
> through the busy-ioctl, and those internal identifiers already do not
> match the user identifiers. Fortuitiously, there is only one user of the
> set of busy rings from the busy-ioctl, and they only wish to choose
> between the RENDER and the BLT engines.
>
> Let's fix the userspace ABI while we still can.
>
> v2: Update the uAPI documentation to explain the identifiers.
>
> Signed-off-by: Chris Wilson
> Cc: Tvrtko Ursulin
> Cc: Daniel Vetter
> Reviewed-by: Tvrtko Ursulin
Acked-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_gem.c | 18 ++
> drivers/gpu/drm/i915/intel_lrc.c| 5 +
> drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +
> drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
> include/uapi/drm/i915_drm.h | 33
> +
> 5 files changed, 54 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index bb44bad15403..85797813a3de 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void
> *data,
> if (ret)
> goto unref;
>
> - BUILD_BUG_ON(I915_NUM_RINGS > 16);
> - args->busy = obj->active << 16;
> - if (obj->last_write_req)
> - args->busy |= obj->last_write_req->ring->id;
> + args->busy = 0;
> + if (obj->active) {
> + int i;
> +
> + for (i = 0; i < I915_NUM_RINGS; i++) {
> + struct drm_i915_gem_request *req;
> +
> + req = obj->last_read_req[i];
> + if (req)
> + args->busy |= 1 << (16 + req->ring->exec_id);
> + }
> + if (obj->last_write_req)
> + args->busy |= obj->last_write_req->ring->exec_id;
> + }
>
> unref:
> drm_gem_object_unreference(&obj->base);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index f5d89c845ede..4aa209483237 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2024,6 +2024,7 @@ static int logical_render_ring_init(struct drm_device
> *dev)
>
> ring->name = "render ring";
> ring->id = RCS;
> + ring->exec_id = I915_EXEC_RENDER;
> ring->mmio_base = RENDER_RING_BASE;
>
> logical_ring_default_irqs(ring, GEN8_RCS_IRQ_SHIFT);
> @@ -2073,6 +2074,7 @@ static int logical_bsd_ring_init(struct drm_device *dev)
>
> ring->name = "bsd ring";
> ring->id = VCS;
> + ring->exec_id = I915_EXEC_BSD;
> ring->mmio_base = GEN6_BSD_RING_BASE;
>
> logical_ring_default_irqs(ring, GEN8_VCS1_IRQ_SHIFT);
> @@ -2088,6 +2090,7 @@ static int logical_bsd2_ring_init(struct drm_device
> *dev)
>
> ring->name = "bsd2 ring";
> ring->id = VCS2;
> + ring->exec_id = I915_EXEC_BSD;
> ring->mmio_base = GEN8_BSD2_RING_BASE;
>
> logical_ring_default_irqs(ring, GEN8_VCS2_IRQ_SHIFT);
> @@ -2103,6 +2106,7 @@ static int logical_blt_ring_init(struct drm_device *dev)
>
> ring->name = "blitter ring";
> ring->id = BCS;
> + ring->exec_id = I915_EXEC_BLT;
> ring->mmio_base = BLT_RING_BASE;
>
> logical_ring_default_irqs(ring, GEN8_BCS_IRQ_SHIFT);
> @@ -2118,6 +2122,7 @@ static int logical_vebox_ring_init(struct drm_device
> *dev)
>
> ring->name = "video enhancement ring";
> ring->id = VECS;
> + ring->exec_id = I915_EXEC_VEBOX;
> ring->mmio_base = VEBOX_RING_BASE;
>
> logical_ring_default_irqs(ring, GEN8_VECS_IRQ_SHIFT);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 8cd8aabcc3ff..310d151c0db2 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2680,6 +2680,7 @@ int intel_init_render_ring_buffer(struct drm_device
> *dev)
>
> ring->name = "render ring";
> ring->id = RCS;
> + ring->exec_id = I915_EXEC_RENDER;
> ring->mmio_base = RENDER_RING_BASE;
>
> if (INTEL_INFO(dev)->gen >= 8) {
> @@ -2828,6 +2829,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
>
> ring->name = "bsd ring";
> ring->id = VCS;
> + ring->exec_id = I915_EXEC_BSD;
>
> ring->write_tail = ring_write_tail;
> if (INTEL_INFO(dev)->gen >= 6) {
> @@ -2904,6 +2906,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
>
> ring->name = "bsd2 ring";
> ring->id = VCS2;
> + ring->exec_id = I915_EXEC_BSD;
>
> ring->write_tail = ring_write_tail;
> ring->mmio_base = GEN8_BSD2_
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Tune down "GT register while GT waking disabled" message
== Summary == Built on c783b5011894af49992f2095cb2848b6cf8ebc57 drm-intel-nightly: 2016y-01m-20d-10h-12m-03s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: pass -> DMESG-WARN (bdw-ultra) UNSTABLE Test gem_sync: Subgroup basic-render: pass -> DMESG-FAIL (skl-i5k-2) UNSTABLE Test kms_force_connector_basic: Subgroup force-connector-state: skip -> PASS (snb-dellxps) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: pass -> SKIP (hsw-gt2) Test pm_rpm: Subgroup basic-rte: dmesg-warn -> PASS (byt-nuc) UNSTABLE bdw-ultratotal:143 pass:135 dwarn:1 dfail:1 fail:0 skip:6 bsw-nuc-2total:143 pass:117 dwarn:2 dfail:0 fail:0 skip:24 byt-nuc total:143 pass:126 dwarn:2 dfail:0 fail:0 skip:15 hsw-brixbox total:143 pass:136 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:143 pass:138 dwarn:0 dfail:0 fail:0 skip:5 ilk-hp8440p total:143 pass:103 dwarn:2 dfail:0 fail:0 skip:38 ivb-t430stotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 skl-i5k-2total:143 pass:133 dwarn:1 dfail:1 fail:0 skip:8 skl-i7k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 snb-dellxps total:143 pass:129 dwarn:0 dfail:0 fail:0 skip:14 snb-x220ttotal:143 pass:129 dwarn:0 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1228/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/3] drm/i915: Fix premature LRC unpin in GuC mode
On Thu, Jan 21, 2016 at 10:10:42AM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> In GuC mode LRC pinning lifetime depends exclusively on the
> request liftime. Since that is terminated by the seqno update
> that opens up a race condition between GPU finishing writing
> out the context image and the driver unpinning the LRC.
>
> To extend the LRC lifetime we will employ a similar approach
> to what legacy ringbuffer submission does.
>
> We will start tracking the last submitted context per engine
> and keep it pinned until it is replaced by another one.
>
> Note that the driver unload path is a bit fragile and could
> benefit greatly from efforts to unify the legacy and exec
> list submission code paths.
>
> At the moment i915_gem_context_fini has special casing for the
> two which are potentialy not needed, and also depends on
> i915_gem_cleanup_ringbuffer running before itself.
>
> v2:
> * Move pinning into engine->emit_request and actually fix
>the reference/unreference logic. (Chris Wilson)
>
> * ring->dev can be NULL on driver unload so use a different
>route towards it.
>
> v3:
> * Rebase.
> * Handle the reset path. (Chris Wilson)
> * Exclude default context from the pinning - it is impossible
>to get it right before default context special casing in
>general is eliminated.
Since you have the refactoring in place, eliminating the special case is
trivial. So patch 2.5?
Looks like http://patchwork.freedesktop.org/patch/69972/ but you already
have half of that patch in place.
Also if we take a step to add patch 2.1:
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index c25083c78ba7..d43c886fd95a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -321,6 +321,14 @@ err_destroy:
return ERR_PTR(ret);
}
+static void i915_gem_context_unpin(struct intel_context *ctx,
+ struct intel_engine_cs *rcs)
+{
+ if (rcs->id == RCS && ctx->legacy_hw_ctx.rcs_state)
+ i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
+ i915_gem_context_unreference(ctx);
+}
+
void i915_gem_context_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -338,13 +346,9 @@ void i915_gem_context_reset(struct drm_device *dev)
for (i = 0; i < I915_NUM_RINGS; i++) {
struct intel_engine_cs *ring = &dev_priv->ring[i];
- struct intel_context *lctx = ring->last_context;
- if (lctx) {
- if (lctx->legacy_hw_ctx.rcs_state && i == RCS)
-
i915_gem_object_ggtt_unpin(lctx->legacy_hw_ctx.rcs_state);
-
- i915_gem_context_unreference(lctx);
+ if (ring->last_context) {
+ i915_gem_context_unpin(ring->last_context, ring);
ring->last_context = NULL;
}
@@ -424,25 +428,18 @@ void i915_gem_context_fini(struct drm_device *dev)
* to offset the do_switch part, so that
i915_gem_context_unreference()
* can then free the base object correctly. */
WARN_ON(!dev_priv->ring[RCS].last_context);
- if (dev_priv->ring[RCS].last_context == dctx) {
- /* Fake switch to NULL context */
- WARN_ON(dctx->legacy_hw_ctx.rcs_state->active);
-
i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
- i915_gem_context_unreference(dctx);
- dev_priv->ring[RCS].last_context = NULL;
- }
-
i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
}
for (i = 0; i < I915_NUM_RINGS; i++) {
struct intel_engine_cs *ring = &dev_priv->ring[i];
- if (ring->last_context)
- i915_gem_context_unreference(ring->last_context);
+ if (ring->last_context) {
+ i915_gem_context_unpin(ring->last_context, ring);
+ ring->last_context = NULL;
+ }
ring->default_context = NULL;
- ring->last_context = NULL;
}
i915_gem_context_unreference(dctx);
Then we just end up with
if (i915.enable_execlists)
intel_lr_context_unpin(ring->last_context, ring);
else
i915_gem_context_unpin(ring->last_context, ring);
which is much less messy in this patch and one step closer to unifying
engine->pin_context()/engine->unpin_context() vfuns.
The only tricky part there is having to disentangle the legacy paths,
but doing so should allow us to see clearly that the sequence is just
intel_gpu_reset();
for_each_engine() unpin(engine->last_context);
unpin(dev_priv->default_context);
After a few more passes, we should be able to call the reset and cleanup
functions higher
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Decouple execbuf uAPI from internal implementation (rev2)
On 15/01/16 17:20, Patchwork wrote: == Summary == Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 2016y-01m-15d-09h-46m-32s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: pass -> DMESG-WARN (skl-i5k-2) UNSTABLE pass -> DMESG-WARN (skl-i7k-2) UNSTABLE Known unrelated: https://bugs.freedesktop.org/show_bug.cgi?id=93693 Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (skl-i5k-2) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-b-frame-sequence: pass -> DMESG-WARN (byt-nuc) Known unrelated: https://bugs.freedesktop.org/show_bug.cgi?id=93121 bdw-nuci7total:138 pass:128 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:132 dwarn:0 dfail:0 fail:0 skip:6 bsw-nuc-2total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 byt-nuc total:141 pass:122 dwarn:4 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 ilk-hp8440p total:141 pass:101 dwarn:3 dfail:0 fail:0 skip:37 ivb-t430stotal:135 pass:122 dwarn:3 dfail:4 fail:0 skip:6 skl-i5k-2total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 HANGED hsw-gt2 in igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a Results at /archive/results/CI_IGT_test/Patchwork_1201/ Merged. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids
On 15/01/16 11:24, Patchwork wrote: == Summary == Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 2016y-01m-15d-09h-46m-32s UTC integration manifest Test gem_storedw_loop: Subgroup basic-render: dmesg-warn -> PASS (bdw-nuci7) pass -> DMESG-WARN (skl-i7k-2) UNSTABLE Known unrelated: https://bugs.freedesktop.org/show_bug.cgi?id=93693 Test kms_flip: Subgroup basic-flip-vs-modeset: dmesg-warn -> PASS (skl-i5k-2) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-a: pass -> DMESG-WARN (byt-nuc) Known unrelated: https://bugs.freedesktop.org/show_bug.cgi?id=93121 bdw-nuci7total:138 pass:129 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultratotal:138 pass:132 dwarn:0 dfail:0 fail:0 skip:6 bsw-nuc-2total:141 pass:115 dwarn:2 dfail:0 fail:0 skip:24 byt-nuc total:141 pass:122 dwarn:4 dfail:0 fail:0 skip:15 hsw-brixbox total:141 pass:134 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:141 pass:137 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:141 pass:101 dwarn:3 dfail:0 fail:0 skip:37 skl-i5k-2total:141 pass:132 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:141 pass:131 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:141 pass:122 dwarn:5 dfail:0 fail:0 skip:14 snb-x220ttotal:141 pass:122 dwarn:5 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1195/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx Patch merged. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 3/3] drm/i915: Fix premature LRC unpin in GuC mode
On Thu, Jan 21, 2016 at 11:02:50AM +, Chris Wilson wrote: > intel_gpu_reset(); > for_each_engine() unpin(engine->last_context); > unpin(dev_priv->default_context); > > After a few more passes, we should be able to call the reset and cleanup > functions higher up (as part of the stopping the GPU upon suspend > procedure before tearing down the sw structs) at which point we only > need the unpin(default_context) here. unref(dev_priv->default_context) ! whether or not the individual engines have a pin on it will be left to themselves to decide. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/25] drm/i915/fbc: extract intel_fbc_can_activate()
Op 19-01-16 om 14:35 schreef Paulo Zanoni:
> Extract all the code that checks if the FBC configuration is valid to
> its own function, making __intel_fbc_update() much simpler.
>
> Signed-off-by: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/intel_fbc.c | 92
> ++--
> 1 file changed, 50 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> b/drivers/gpu/drm/i915/intel_fbc.c
> index 6b43ec3..dff30e1 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -514,17 +514,6 @@ static bool crtc_can_fbc(struct intel_crtc *crtc)
> return true;
> }
>
> -static bool crtc_is_valid(struct intel_crtc *crtc)
> -{
> - if (!intel_crtc_active(&crtc->base))
> - return false;
> - if (!to_intel_plane_state(crtc->base.primary->state)->visible)
> - return false;
> -
> - return true;
> -}
> -
> static bool multiple_pipes_ok(struct drm_i915_private *dev_priv)
> {
> enum pipe pipe;
> @@ -750,48 +739,40 @@ static bool intel_fbc_hw_tracking_covers_screen(struct
> intel_crtc *crtc)
> return effective_w <= max_w && effective_h <= max_h;
> }
>
> -/**
> - * __intel_fbc_update - activate/deactivate FBC as needed, unlocked
> - * @crtc: the CRTC that triggered the update
> - *
> - * This function completely reevaluates the status of FBC, then activates,
> - * deactivates or maintains it on the same state.
> - */
> -static void __intel_fbc_update(struct intel_crtc *crtc)
> +static bool intel_fbc_can_activate(struct intel_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> + struct drm_plane *primary;
> struct drm_framebuffer *fb;
> + struct intel_plane_state *plane_state;
> struct drm_i915_gem_object *obj;
> const struct drm_display_mode *adjusted_mode;
It would be nice if this function had a plane_state and crtc_state passed into
it for validation.
I suppose it can wait though, maybe something for the future.
> - WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock));
> -
> - if (!multiple_pipes_ok(dev_priv)) {
> - set_no_fbc_reason(dev_priv, "more than one pipe active");
> - goto out_disable;
> - }
> -
> - if (!dev_priv->fbc.enabled || dev_priv->fbc.crtc != crtc)
> - return;
> -
> - if (!crtc_is_valid(crtc)) {
> - set_no_fbc_reason(dev_priv, "no output");
> - goto out_disable;
> + if (!intel_crtc_active(&crtc->base)) {
> + set_no_fbc_reason(dev_priv, "CRTC not active");
> + return false;
> }
This one has become redundant with plane_state->visible. You can't have a
visible plane with crtc off any more.
~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/8] drm/i915/gen9: Add GEN8_CS_CHICKEN1 to HW whitelist
On 13/01/2016 10:06, Arun Siluvery wrote:
Required for WaEnablePreemptionGranularityControlByUMD:skl,bxt
Signed-off-by: Arun Siluvery
Reviewed-by: Nick Hoath
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_ringbuffer.c | 6 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6668bb0..1067ff0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5998,6 +5998,8 @@ enum skl_disp_power_wells {
#define FF_SLICE_CS_CHICKEN2 _MMIO(0x20e4)
#define GEN9_TSG_BARRIER_ACK_DISABLE (1<<8)
+#define GEN8_CS_CHICKEN1 _MMIO(0x2580)
+
/* GEN7 chicken */
#define GEN7_COMMON_SLICE_CHICKEN1_MMIO(0x7010)
# define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC((1<<10) | (1<<26))
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 354da81..35e78ed 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -909,6 +909,7 @@ static int gen9_init_workarounds(struct intel_engine_cs
*ring)
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t tmp;
+ int ret;
/* WaEnableLbsSlaRetryTimerDecrement:skl */
I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) |
@@ -979,6 +980,11 @@ static int gen9_init_workarounds(struct intel_engine_cs
*ring)
/* WaDisableSTUnitPowerOptimization:skl,bxt */
WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE);
+ /* WaEnablePreemptionGranularityControlByUMD:skl,bxt */
+ ret= wa_ring_whitelist_reg(ring, GEN8_CS_CHICKEN1);
+ if (ret)
+ return ret;
+
return 0;
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Acquire RPM wakeref for KMS atomic commit
On to, 2016-01-14 at 18:50 +0200, Imre Deak wrote:
> On la, 2015-12-19 at 09:58 +, Chris Wilson wrote:
> > Once all the preparations are complete, we are ready to write the
> > modesetting to the hardware. During this phase, we will be making
> > lots
> > of HW register access, so take a top level wakeref to prevent an
> > unwarranted rpm suspend cycle mid-commit. Lower level functions
> > should
> > be waking the individual power wells as required.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93439
>
> I would separate here two things:
>
> The device level power flip-flopping you mention and the fix for the
> above bug. For the flip-flopping we could use what you suggest,
> perhaps
> by also avoiding waking up the device if nothing will change and
> everything will stay disabled.
>
> As for the fix I would go with what Ville suggested. By ensuring we
> keep an RPM reference we still allow for a display power domain
> reference to come and go in the middle of the HW readout. I went
> ahead
> and tried the following which got rid of the problem too, if people
> are
> ok with it I could convert the rest of the HW readout places
> accordingly and send out the patch. We can also
> get pm_runtime_get_if_in_use() into use once it's added, but it's
> not crucial for the fix.
>
Below patch looks fine. Just that I'd use s/if_enabled/if_in_use/ to
match the PM API better. We're already doing too much of a good job to
having many names for same thing.
Regards, Joonas
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 1f9a368..907377dc 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1910,13 +1910,16 @@ bool intel_ddi_connector_get_hw_state(struct
> intel_connector *intel_connector)
> enum transcoder cpu_transcoder;
> enum intel_display_power_domain power_domain;
> uint32_t tmp;
> + bool ret;
>
> power_domain =
> intel_display_port_power_domain(intel_encoder);
> - if (!intel_display_power_is_enabled(dev_priv, power_domain))
> + if (!intel_display_power_get_if_enabled(dev_priv,
> power_domain))
> return false;
>
> - if (!intel_encoder->get_hw_state(intel_encoder, &pipe))
> - return false;
> + if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
> + ret = false;
> + goto out;
> + }
>
> if (port == PORT_A)
> cpu_transcoder = TRANSCODER_EDP;
> @@ -1928,23 +1931,30 @@ bool intel_ddi_connector_get_hw_state(struct
> intel_connector *intel_connector)
> switch (tmp & TRANS_DDI_MODE_SELECT_MASK) {
> case TRANS_DDI_MODE_SELECT_HDMI:
> case TRANS_DDI_MODE_SELECT_DVI:
> - return (type == DRM_MODE_CONNECTOR_HDMIA);
> + ret = type == DRM_MODE_CONNECTOR_HDMIA;
> + goto out;
>
> case TRANS_DDI_MODE_SELECT_DP_SST:
> - if (type == DRM_MODE_CONNECTOR_eDP)
> - return true;
> - return (type == DRM_MODE_CONNECTOR_DisplayPort);
> + ret = type == DRM_MODE_CONNECTOR_eDP ||
> + type == DRM_MODE_CONNECTOR_DisplayPort;
> + goto out;
> case TRANS_DDI_MODE_SELECT_DP_MST:
> /* if the transcoder is in MST state then
> * connector isn't connected */
> - return false;
> + ret = false;
> + goto out;
>
> case TRANS_DDI_MODE_SELECT_FDI:
> - return (type == DRM_MODE_CONNECTOR_VGA);
> + ret = type == DRM_MODE_CONNECTOR_VGA;
> + goto out;
>
> default:
> - return false;
> + ret = false;
> }
> +out:
> + intel_display_power_put(dev_priv, power_domain);
> +
> + return ret;
> }
>
> bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 059b46e..3c84159 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1456,6 +1456,8 @@ bool __intel_display_power_is_enabled(struct
> drm_i915_private *dev_priv,
> enum
> intel_display_power_domain domain);
> void intel_display_power_get(struct drm_i915_private *dev_priv,
> enum intel_display_power_domain
> domain);
> +bool intel_display_power_get_if_enabled(struct drm_i915_private
> *dev_priv,
> + enum
> intel_display_power_domain domain);
> void intel_display_power_put(struct drm_i915_private *dev_priv,
> enum intel_display_power_domain
> domain);
>
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index bbca527..6c4f170 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1470,6 +1470,17 @@ void intel_display
Re: [Intel-gfx] [PATCH 3/8] drm/i915/gen9: Add HDC_CHICKEN1 to HW whitelist
On 13/01/2016 10:06, Arun Siluvery wrote:
Required for WaAllowUMDToModifyHDCChicken1:skl,bxt
Signed-off-by: Arun Siluvery
Reviewed-by: Nick Hoath
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1067ff0..16ef377 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6045,6 +6045,8 @@ enum skl_disp_power_wells {
#define HDC_FORCE_NON_COHERENT (1<<4)
#define HDC_BARRIER_PERFORMANCE_DISABLE (1<<10)
+#define GEN8_HDC_CHICKEN1 _MMIO(0x7304)
+
/* GEN9 chicken */
#define SLICE_ECO_CHICKEN0_MMIO(0x7308)
#define PIXEL_MASK_CAMMING_DISABLE (1 << 14)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 35e78ed..2241a92 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -985,6 +985,11 @@ static int gen9_init_workarounds(struct intel_engine_cs
*ring)
if (ret)
return ret;
+ /* WaAllowUMDToModifyHDCChicken1:skl,bxt */
+ ret = wa_ring_whitelist_reg(ring, GEN8_HDC_CHICKEN1);
+ if (ret)
+ return ret;
+
return 0;
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/8] drm/i915/bxt: Add GEN9_CS_DEBUG_MODE1 to HW whitelist
On 13/01/2016 10:06, Arun Siluvery wrote:
Required for,
WaDisableObjectLevelPreemptionForTrifanOrPolygon:bxt
WaDisableObjectLevelPreemptionForInstancedDraw:bxt
WaDisableObjectLevelPreemtionForInstanceId:bxt
According to WA database these are only applicable for BXT:A0 but since
A0 and A1 shares the same GT these are extended for A1 as well.
These are also required for SKL until B0 but not adding them because they
are pre-production steppings.
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 16ef377..eabd2af 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5998,6 +5998,7 @@ enum skl_disp_power_wells {
#define FF_SLICE_CS_CHICKEN2 _MMIO(0x20e4)
#define GEN9_TSG_BARRIER_ACK_DISABLE (1<<8)
+#define GEN9_CS_DEBUG_MODE1_MMIO(0x20EC)
The pattern seems to be lc for hex (0x20ec)
#define GEN8_CS_CHICKEN1 _MMIO(0x2580)
/* GEN7 chicken */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2241a92..7a46cf1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1132,6 +1132,15 @@ static int bxt_init_workarounds(struct intel_engine_cs
*ring)
GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
}
+ /* WaDisableObjectLevelPreemptionForTrifanOrPolygon:bxt */
+ /* WaDisableObjectLevelPreemptionForInstancedDraw:bxt */
+ /* WaDisableObjectLevelPreemtionForInstanceId:bxt */
+ if (IS_BXT_REVID(dev, 0, BXT_REVID_A1)) {
+ ret = wa_ring_whitelist_reg(ring, GEN9_CS_DEBUG_MODE1);
+ if (ret)
+ return ret;
+ }
+
return 0;
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Instrument PSR parameter for possible quirks with link standby.
Em Qui, 2016-01-21 às 08:35 +0530, Thulasimani, Sivakumar escreveu:
>
> On 1/20/2016 10:32 PM, Zanoni, Paulo R wrote:
> > Em Sex, 2015-12-11 às 08:39 -0800, Rodrigo Vivi escreveu:
> > > Unfortunately we don't know all panels and platforms out there
> > > and we
> > > found internal prototypes without VBT proper set but where only
> > > link in standby worked well.
> :) if it is internal i assume someone has to set the vbt ,we
> encountered
> an issue
> sometime back that blamed vbt as incorrect only to later learn that
> the person who created the setup didn't care to configure the VBT.
But in order to discover if the VBT is incorrect, the parameter can be
used.
> > >
> > > So, before enable PSR by default let's instrument the PSR
> > > parameter
> > > in a way that we can identify different panels out there that
> > > might
> > > require or work better with link standby mode.
> > >
> > > It is also useful to say that for backward compatibility I'm not
> > > changing the meaning of this flag. So "0" still means disabled
> > > and "1" means enabled with full support and maximum power
> > > savings.
> > >
> > > v2: Use positive value instead of negative for different
> > > operation
> > > mode
> > > as suggested by Daniel.
> > >
> > > v3: As Paulo suggested use 2 to force link standby and 3 to force
> > > link
> > > fully on. Also split the link_standby introduction in a
> > > separated
> > > patch.
> > >
> > > Cc: Paulo Zanoni
> > > Cc: Daniel Vetter
> > > Signed-off-by: Rodrigo Vivi
> > > ---
> > > drivers/gpu/drm/i915/i915_params.c | 7 ++-
> > > drivers/gpu/drm/i915/intel_psr.c | 17 +
> > > 2 files changed, 23 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_params.c
> > > b/drivers/gpu/drm/i915/i915_params.c
> > > index 835d609..f78ddf3 100644
> > > --- a/drivers/gpu/drm/i915/i915_params.c
> > > +++ b/drivers/gpu/drm/i915/i915_params.c
> > > @@ -126,7 +126,12 @@ MODULE_PARM_DESC(enable_execlists,
> > > "(-1=auto [default], 0=disabled, 1=enabled)");
> > >
> > > module_param_named_unsafe(enable_psr, i915.enable_psr, int,
> > > 0600);
> > > -MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)");
> > > +MODULE_PARM_DESC(enable_psr, "Enable PSR "
> > > + "(0=disabled [default], 1=enabled - link mode
> > > chosen per-platform, 2=force link-standby mode, 3=force link-off
> > > mode)"
> > > + "In case you needed to force any different
> > > option,
> > > please "
> > > + "report PCI device ID, subsystem vendor and
> > > subsystem device ID "
> > > + "to intel-gfx@lists.freedesktop.org, if your
> > > machine needs it. "
> > > + "It will then be included in an upcoming module
> > > version.");
> > Are we making a promise here? Isn't that dangerous? :P
> > I'd just tell the users to open bug reports.
> > (I'm not requiring you to change anything here, but something
> > something
> > lawyers something)
> >
> > >
> > > module_param_named_unsafe(preliminary_hw_support,
> > > i915.preliminary_hw_support, int, 0600);
> > > MODULE_PARM_DESC(preliminary_hw_support,
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index b84ec80..c3c2bb8 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -335,6 +335,12 @@ static bool
> > > intel_psr_match_conditions(struct
> > > intel_dp *intel_dp)
> > > return false;
> > > }
> > >
> > > + if ((IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) &&
> > > + dev_priv->psr.link_standby) {
> IS_VALLEYVIEW() will return true for both valleyview and cherryview
> so
> the above check for cherryview can be removed.
Not anymore:
commit 666a45379e2c29bc16e60648e5ad8f6f8b7fa6ce
Author: Wayne Boyer
Date: Wed Dec 9 12:29:35 2015 -0800
drm/i915: Separate cherryview from valleyview
> > s/dev_priv->psr.link_standby/!dev_priv->psr.link_standby/
> >
> >
> > Also, I'm not sure if this chunk belongs here or at
> > intel_psr_init(),
> > since it effectively disables PSR. This means that
> > i915.enable_psr=3
> > disables PSR on VLV/CHV. But maybe we shouldn't care since users
> > shouldn't be using the option anyway. On the other hand, users may
> > start claiming that i915.enable_psr=X "fixed PSR" for them while
> > effectively it just disabled PSR, so perhaps DRM_ERROR would be
> > better.
> > Anyway, I'm not requesting any change, just pointing things in case
> > you
> > or someone else has any idea, but maybe I'd go with DRM_ERROR since
> > users usually don't know which platform supports what, so the loud
> > message may help them.
> i agree, psr_match_conditions should check for parameters that can
> change
> dynamically post boot to decide if we can enable psr or not,
> if link_standby cannot be changed post boot we should check for it in
> in
Re: [Intel-gfx] [PATCH 14/18] drm/i915: Embed rotation_info under intel_framebuffer
On Wed, Jan 20, 2016 at 09:08:50PM +, Chris Wilson wrote: > On Wed, Jan 20, 2016 at 09:05:35PM +0200, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > Instead of repopulatin the rotation_info struct for the fb every time > > we try to use the fb, we can just populate it once when creating the fb, > > and later we can just copy the pre-populate struct into the gtt_view. > > But the rotation state isn't tied to the fb, but to the plane+fb. > i.e. I can show the same fb on an unrotated pipe at the same time as > showing it rotated, right? Yep. The rotation info basically just tells us how we need to shuffle the pages for this particular fb if we want to scan it out in the 90/270 degree orientation. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/25] drm/i915/fbc: extract intel_fbc_can_activate()
Em Qui, 2016-01-21 às 12:35 +0100, Maarten Lankhorst escreveu:
> Op 19-01-16 om 14:35 schreef Paulo Zanoni:
> > Extract all the code that checks if the FBC configuration is valid
> > to
> > its own function, making __intel_fbc_update() much simpler.
> >
> > Signed-off-by: Paulo Zanoni
> > ---
> > drivers/gpu/drm/i915/intel_fbc.c | 92 ++
> > --
> > 1 file changed, 50 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> > b/drivers/gpu/drm/i915/intel_fbc.c
> > index 6b43ec3..dff30e1 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -514,17 +514,6 @@ static bool crtc_can_fbc(struct intel_crtc
> > *crtc)
> > return true;
> > }
> >
> > -static bool crtc_is_valid(struct intel_crtc *crtc)
> > -{
> > - if (!intel_crtc_active(&crtc->base))
> > - return false;
> > - if (!to_intel_plane_state(crtc->base.primary->state)-
> > >visible)
> > - return false;
> > -
> > - return true;
> > -}
> > -
> > static bool multiple_pipes_ok(struct drm_i915_private *dev_priv)
> > {
> > enum pipe pipe;
> > @@ -750,48 +739,40 @@ static bool
> > intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
> > return effective_w <= max_w && effective_h <= max_h;
> > }
> >
> > -/**
> > - * __intel_fbc_update - activate/deactivate FBC as needed,
> > unlocked
> > - * @crtc: the CRTC that triggered the update
> > - *
> > - * This function completely reevaluates the status of FBC, then
> > activates,
> > - * deactivates or maintains it on the same state.
> > - */
> > -static void __intel_fbc_update(struct intel_crtc *crtc)
> > +static bool intel_fbc_can_activate(struct intel_crtc *crtc)
> > {
> > struct drm_i915_private *dev_priv = crtc->base.dev-
> > >dev_private;
> > + struct drm_plane *primary;
> > struct drm_framebuffer *fb;
> > + struct intel_plane_state *plane_state;
> > struct drm_i915_gem_object *obj;
> > const struct drm_display_mode *adjusted_mode;
> It would be nice if this function had a plane_state and crtc_state
> passed into it for validation.
> I suppose it can wait though, maybe something for the future.
The way we handle this will change a few times during the series. In
the end this function will use the state cache, and
intel_fbc_update_state_cache() will be responsible for finding the
state structures.
> > - WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock));
> > -
> > - if (!multiple_pipes_ok(dev_priv)) {
> > - set_no_fbc_reason(dev_priv, "more than one pipe
> > active");
> > - goto out_disable;
> > - }
> > -
> > - if (!dev_priv->fbc.enabled || dev_priv->fbc.crtc != crtc)
> > - return;
> > -
> > - if (!crtc_is_valid(crtc)) {
> > - set_no_fbc_reason(dev_priv, "no output");
> > - goto out_disable;
> > + if (!intel_crtc_active(&crtc->base)) {
> > + set_no_fbc_reason(dev_priv, "CRTC not active");
> > + return false;
> > }
> This one has become redundant with plane_state->visible. You can't
> have a visible plane with crtc off any more.
We fix this later in the series. If we just remove this right now, we
may have a NULL primary->fb causing a segfault, so we get rid of it
when the segfault won't be possible anymore.
>
> ~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/18] drm/i915: Embed rotation_info under intel_framebuffer
On Thu, Jan 21, 2016 at 02:06:27PM +0200, Ville Syrjälä wrote: > On Wed, Jan 20, 2016 at 09:08:50PM +, Chris Wilson wrote: > > On Wed, Jan 20, 2016 at 09:05:35PM +0200, ville.syrj...@linux.intel.com > > wrote: > > > From: Ville Syrjälä > > > > > > Instead of repopulatin the rotation_info struct for the fb every time > > > we try to use the fb, we can just populate it once when creating the fb, > > > and later we can just copy the pre-populate struct into the gtt_view. > > > > But the rotation state isn't tied to the fb, but to the plane+fb. > > i.e. I can show the same fb on an unrotated pipe at the same time as > > showing it rotated, right? > > Yep. The rotation info basically just tells us how we need to shuffle > the pages for this particular fb if we want to scan it out in the > 90/270 degree orientation. Ah, gotcha, thanks for the clarification. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: Extract context unpinning to its own function
From: Tvrtko Ursulin
Will enable cleaner implementation of a following fix and
easier code unification in the future.
Idea and code by Chris Wilson.
Signed-off-by: Tvrtko Ursulin
Cc: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_context.c | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 6a4f64b03db6..1a67e07b9e6a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -321,6 +321,14 @@ err_destroy:
return ERR_PTR(ret);
}
+static void i915_gem_context_unpin(struct intel_context *ctx,
+ struct intel_engine_cs *engine)
+{
+ if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
+ i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
+ i915_gem_context_unreference(ctx);
+}
+
void i915_gem_context_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -338,13 +346,9 @@ void i915_gem_context_reset(struct drm_device *dev)
for (i = 0; i < I915_NUM_RINGS; i++) {
struct intel_engine_cs *ring = &dev_priv->ring[i];
- struct intel_context *lctx = ring->last_context;
-
- if (lctx) {
- if (lctx->legacy_hw_ctx.rcs_state && i == RCS)
-
i915_gem_object_ggtt_unpin(lctx->legacy_hw_ctx.rcs_state);
- i915_gem_context_unreference(lctx);
+ if (ring->last_context) {
+ i915_gem_context_unpin(ring->last_context, ring);
ring->last_context = NULL;
}
}
@@ -417,13 +421,6 @@ void i915_gem_context_fini(struct drm_device *dev)
* to offset the do_switch part, so that
i915_gem_context_unreference()
* can then free the base object correctly. */
WARN_ON(!dev_priv->ring[RCS].last_context);
- if (dev_priv->ring[RCS].last_context == dctx) {
- /* Fake switch to NULL context */
- WARN_ON(dctx->legacy_hw_ctx.rcs_state->active);
-
i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
- i915_gem_context_unreference(dctx);
- dev_priv->ring[RCS].last_context = NULL;
- }
i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
}
@@ -432,7 +429,7 @@ void i915_gem_context_fini(struct drm_device *dev)
struct intel_engine_cs *ring = &dev_priv->ring[i];
if (ring->last_context) {
- i915_gem_context_unreference(ring->last_context);
+ i915_gem_context_unpin(ring->last_context, ring);
ring->last_context = NULL;
}
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/4] drm/i915: Make LRC (un)pinning work on context and engine
From: Tvrtko Ursulin
Previously intel_lr_context_(un)pin were operating on requests
which is in conflict with their names.
If we make them take a context and an engine, it makes the names
make more sense and it also makes future fixes possible.
v2: Rebase for default_context/kernel_context change.
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Chris Wilson
Cc: Chris Wilson
Cc: Nick Hoath
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/intel_lrc.c | 49
drivers/gpu/drm/i915/intel_lrc.h | 3 ++-
3 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 371bbb28c471..28a28ebb3f16 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2680,7 +2680,7 @@ void i915_gem_request_free(struct kref *req_ref)
if (ctx) {
if (i915.enable_execlists && ctx != req->i915->kernel_context)
- intel_lr_context_unpin(req);
+ intel_lr_context_unpin(ctx, req->ring);
i915_gem_context_unreference(ctx);
}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 73d4347429df..28128acf23a3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -225,7 +225,8 @@ enum {
#define GEN8_CTX_ID_SHIFT 32
#define CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT 0x17
-static int intel_lr_context_pin(struct drm_i915_gem_request *rq);
+static int intel_lr_context_pin(struct intel_context *ctx,
+ struct intel_engine_cs *engine);
static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
struct drm_i915_gem_object *default_ctx_obj);
@@ -599,7 +600,7 @@ static int execlists_context_queue(struct
drm_i915_gem_request *request)
int num_elements = 0;
if (request->ctx != request->i915->kernel_context)
- intel_lr_context_pin(request);
+ intel_lr_context_pin(request->ctx, ring);
i915_gem_request_reference(request);
@@ -704,7 +705,7 @@ int intel_logical_ring_alloc_request_extras(struct
drm_i915_gem_request *request
}
if (request->ctx != request->i915->kernel_context)
- ret = intel_lr_context_pin(request);
+ ret = intel_lr_context_pin(request->ctx, request->ring);
return ret;
}
@@ -1015,7 +1016,8 @@ void intel_execlists_retire_requests(struct
intel_engine_cs *ring)
ctx->engine[ring->id].state;
if (ctx_obj && (ctx != req->i915->kernel_context))
- intel_lr_context_unpin(req);
+ intel_lr_context_unpin(ctx, ring);
+
list_del(&req->execlist_link);
i915_gem_request_unreference(req);
}
@@ -1059,8 +1061,8 @@ int logical_ring_flush_all_caches(struct
drm_i915_gem_request *req)
return 0;
}
-static int intel_lr_context_do_pin(struct intel_engine_cs *ring,
- struct intel_context *ctx)
+static int intel_lr_context_do_pin(struct intel_context *ctx,
+ struct intel_engine_cs *ring)
{
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1103,41 +1105,40 @@ unpin_ctx_obj:
return ret;
}
-static int intel_lr_context_pin(struct drm_i915_gem_request *rq)
+static int intel_lr_context_pin(struct intel_context *ctx,
+ struct intel_engine_cs *engine)
{
int ret = 0;
- struct intel_engine_cs *ring = rq->ring;
- if (rq->ctx->engine[ring->id].pin_count++ == 0) {
- ret = intel_lr_context_do_pin(ring, rq->ctx);
+ if (ctx->engine[engine->id].pin_count++ == 0) {
+ ret = intel_lr_context_do_pin(ctx, engine);
if (ret)
goto reset_pin_count;
}
return ret;
reset_pin_count:
- rq->ctx->engine[ring->id].pin_count = 0;
+ ctx->engine[engine->id].pin_count = 0;
return ret;
}
-void intel_lr_context_unpin(struct drm_i915_gem_request *rq)
+void intel_lr_context_unpin(struct intel_context *ctx,
+ struct intel_engine_cs *engine)
{
- struct intel_engine_cs *ring = rq->ring;
- struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
- struct intel_ringbuffer *ringbuf = rq->ringbuf;
+ struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
- WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
+ WARN_ON(!mutex_is_locked(&engine->dev->struct_mutex));
- if (!ctx_obj)
+ if (WARN_ON_ONCE(!ctx_obj))
return;
- if (--rq->ctx->engine[ring->id].pin_count == 0) {
- kunmap(kmap_to_page(rq->ctx->engine[ring->id].lrc_reg_state));
-
[Intel-gfx] [PATCH v2 2/4] drm/i915: Make LRC pinning own a reference to the context
From: Tvrtko Ursulin Will simplify the following fix and sounds logical. v2: Add some whitespace to separate logic better. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Reviewed-by: Chris Wilson Cc: Chris Wilson Cc: Nick Hoath --- drivers/gpu/drm/i915/intel_lrc.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 28128acf23a3..8eb6e364fefc 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1114,6 +1114,8 @@ static int intel_lr_context_pin(struct intel_context *ctx, ret = intel_lr_context_do_pin(ctx, engine); if (ret) goto reset_pin_count; + + i915_gem_context_reference(ctx); } return ret; @@ -1139,6 +1141,8 @@ void intel_lr_context_unpin(struct intel_context *ctx, ctx->engine[engine->id].lrc_vma = NULL; ctx->engine[engine->id].lrc_desc = 0; ctx->engine[engine->id].lrc_reg_state = NULL; + + i915_gem_context_unreference(ctx); } } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 4/4] drm/i915: Fix premature LRC unpin in GuC mode
From: Tvrtko Ursulin
In GuC mode LRC pinning lifetime depends exclusively on the
request liftime. Since that is terminated by the seqno update
that opens up a race condition between GPU finishing writing
out the context image and the driver unpinning the LRC.
To extend the LRC lifetime we will employ a similar approach
to what legacy ringbuffer submission does.
We will start tracking the last submitted context per engine
and keep it pinned until it is replaced by another one.
Note that the driver unload path is a bit fragile and could
benefit greatly from efforts to unify the legacy and exec
list submission code paths.
At the moment i915_gem_context_fini has special casing for the
two which are potentialy not needed, and also depends on
i915_gem_cleanup_ringbuffer running before itself.
v2:
* Move pinning into engine->emit_request and actually fix
the reference/unreference logic. (Chris Wilson)
* ring->dev can be NULL on driver unload so use a different
route towards it.
v3:
* Rebase.
* Handle the reset path. (Chris Wilson)
* Exclude default context from the pinning - it is impossible
to get it right before default context special casing in
general is eliminated.
v4:
* Rebased & moved context tracking to
intel_logical_ring_advance_and_submit.
Signed-off-by: Tvrtko Ursulin
Issue: VIZ-4277
Cc: Chris Wilson
Cc: Nick Hoath
---
drivers/gpu/drm/i915/i915_gem_context.c | 10 +++---
drivers/gpu/drm/i915/intel_lrc.c| 16 ++--
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 1a67e07b9e6a..7e6f8c7b6d01 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -324,9 +324,13 @@ err_destroy:
static void i915_gem_context_unpin(struct intel_context *ctx,
struct intel_engine_cs *engine)
{
- if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
- i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
- i915_gem_context_unreference(ctx);
+ if (i915.enable_execlists) {
+ intel_lr_context_unpin(ctx, engine);
+ } else {
+ if (engine->id == RCS && ctx->legacy_hw_ctx.rcs_state)
+
i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
+ i915_gem_context_unreference(ctx);
+ }
}
void i915_gem_context_reset(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 8eb6e364fefc..0e215ea3f8ab 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -766,6 +766,7 @@ intel_logical_ring_advance_and_submit(struct
drm_i915_gem_request *request)
{
struct intel_ringbuffer *ringbuf = request->ringbuf;
struct drm_i915_private *dev_priv = request->i915;
+ struct intel_engine_cs *engine = request->ring;
intel_logical_ring_advance(ringbuf);
request->tail = ringbuf->tail;
@@ -780,9 +781,20 @@ intel_logical_ring_advance_and_submit(struct
drm_i915_gem_request *request)
intel_logical_ring_emit(ringbuf, MI_NOOP);
intel_logical_ring_advance(ringbuf);
- if (intel_ring_stopped(request->ring))
+ if (intel_ring_stopped(engine))
return 0;
+ if (engine->last_context != request->ctx) {
+ if (engine->last_context)
+ intel_lr_context_unpin(engine->last_context, engine);
+ if (request->ctx != request->i915->kernel_context) {
+ intel_lr_context_pin(request->ctx, engine);
+ engine->last_context = request->ctx;
+ } else {
+ engine->last_context = NULL;
+ }
+ }
+
if (dev_priv->guc.execbuf_client)
i915_guc_submit(dev_priv->guc.execbuf_client, request);
else
@@ -1129,7 +1141,7 @@ void intel_lr_context_unpin(struct intel_context *ctx,
{
struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
- WARN_ON(!mutex_is_locked(&engine->dev->struct_mutex));
+ WARN_ON(!mutex_is_locked(&ctx->i915->dev->struct_mutex));
if (WARN_ON_ONCE(!ctx_obj))
return;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/8] drm/i915/bxt: Add GEN8_L3SQCREG4 to HW whitelist
On 13/01/2016 10:06, Arun Siluvery wrote:
Required for WaDisableLSQCROPERFforOCL:bxt
According to WA database these are only applicable for BXT:A0 but since
A0 and A1 shares the same GT these are extended for A1 as well.
Signed-off-by: Arun Siluvery
Reviewed-by: Nick Hoath
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 7a46cf1..5eb4eea 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1135,10 +1135,15 @@ static int bxt_init_workarounds(struct intel_engine_cs
*ring)
/* WaDisableObjectLevelPreemptionForTrifanOrPolygon:bxt */
/* WaDisableObjectLevelPreemptionForInstancedDraw:bxt */
/* WaDisableObjectLevelPreemtionForInstanceId:bxt */
+ /* WaDisableLSQCROPERFforOCL:bxt */
if (IS_BXT_REVID(dev, 0, BXT_REVID_A1)) {
ret = wa_ring_whitelist_reg(ring, GEN9_CS_DEBUG_MODE1);
if (ret)
return ret;
+
+ ret = wa_ring_whitelist_reg(ring, GEN8_L3SQCREG4);
+ if (ret)
+ return ret;
}
return 0;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/8] drm/i915/skl: Add GEN8_L3SQCREG4 to HW whitelist
On 13/01/2016 10:06, Arun Siluvery wrote: Required for WaDisableLSQCROPERFforOCL:skl Signed-off-by: Arun Siluvery Reviewed-by: Nick Hoath --- drivers/gpu/drm/i915/intel_ringbuffer.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 5eb4eea..b8dbd2c 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1097,6 +1097,11 @@ static int skl_init_workarounds(struct intel_engine_cs *ring) GEN7_HALF_SLICE_CHICKEN1, GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE); + /* WaDisableLSQCROPERFforOCL:skl */ + ret = wa_ring_whitelist_reg(ring, GEN8_L3SQCREG4); + if (ret) + return ret; + return skl_tune_iz_hashing(ring); } ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915: edp resume/On time optimization.
On 1/20/2016 5:06 AM, Ville Syrjälä wrote:
On Wed, Jan 20, 2016 at 03:29:00PM +0530, Kumar, Shobhit wrote:
On 01/20/2016 02:30 PM, Daniel Vetter wrote:
On Tue, Jan 19, 2016 at 02:37:55PM -0800, Kumar, Abhay wrote:
On 1/12/2016 5:57 PM, Kumar, Abhay wrote:
From: Abhay Kumar
Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.
v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
delay calculation(Ville).
v3: Addressed below comments
1. Tracking time from where last powercycle is initiated.
2. Used ktime_get_bootime() wrapper for boottime clock.
3. Used ktime_ms_delta() to get time difference.
v4: Updated v3 change log in detail.
Cc: Ville Syrjälä
Signed-off-by: Abhay Kumar
I can't see the r-b here nor anywhere else in this thread. That's why I
didn't pick it up. Where is it?
V3 of this patch was r-b by Ville and had comment on the commit msg. I
updated commit msg and pushed V4.
So practically there is no code change and Ville r-b should hold good.
Sorry in case i missed to keep that r-b by Ville in V3 here.
I didn't see it either. Ville can you please have a look at the latest
changes.
I don't think I'll bother since I'm pretty sure I already gave the r-b
during the last round. People really should learn to hang on to the r-bs
with some vigor.
Regards
Shobhit
-Daniel
---
drivers/gpu/drm/i915/intel_dp.c | 19 ++-
drivers/gpu/drm/i915/intel_drv.h | 2 +-
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d3..0042693 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1812,12 +1812,21 @@ static void wait_panel_off(struct intel_dp *intel_dp)
static void wait_panel_power_cycle(struct intel_dp *intel_dp)
{
+ static ktime_t panel_power_on_time;
+ s64 panel_power_off_duration;
+
DRM_DEBUG_KMS("Wait for panel power cycle\n");
+ /* take the difference of currrent time and panel power off time
+* and then make panel wait for t11_t12 if needed. */
+ panel_power_on_time = ktime_get_boottime();
+ panel_power_off_duration = ktime_ms_delta(panel_power_on_time,
intel_dp->panel_power_off_time);
+
/* When we disable the VDD override bit last we have to do the manual
* wait. */
- wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
- intel_dp->panel_power_cycle_delay);
+ if (panel_power_off_duration < (s64)intel_dp->panel_power_cycle_delay)
+ wait_remaining_ms_from_jiffies(jiffies,
+ intel_dp->panel_power_cycle_delay -
panel_power_off_duration);
wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
}
@@ -1969,7 +1978,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp
*intel_dp)
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
if ((pp & POWER_TARGET_ON) == 0)
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
power_domain = intel_display_port_aux_power_domain(intel_encoder);
intel_display_power_put(dev_priv, power_domain);
@@ -2118,7 +2127,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */
@@ -5122,7 +5131,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct
drm_connector *connect
static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
{
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
intel_dp->last_power_on = jiffies;
intel_dp->last_backlight_off = jiffies;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bdfe403..06b37b8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -793,9 +793,9 @@ struct intel_dp {
int backlight_off_delay;
struct delayed_work panel_vdd_work;
bool want_panel_vdd;
- unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ ktime_t panel_power_off_time;
struct notifier_block edp_notifier;
Since this is already reviewed. Can we please get this Queued for -next ?
Regards,
Abhay Kumar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lis
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Instrument PSR parameter for possible quirks with link standby.
On 1/21/2016 5:26 PM, Zanoni, Paulo R wrote:
Em Qui, 2016-01-21 às 08:35 +0530, Thulasimani, Sivakumar escreveu:
On 1/20/2016 10:32 PM, Zanoni, Paulo R wrote:
Em Sex, 2015-12-11 às 08:39 -0800, Rodrigo Vivi escreveu:
Unfortunately we don't know all panels and platforms out there
and we
found internal prototypes without VBT proper set but where only
link in standby worked well.
:) if it is internal i assume someone has to set the vbt ,we
encountered
an issue
sometime back that blamed vbt as incorrect only to later learn that
the person who created the setup didn't care to configure the VBT.
But in order to discover if the VBT is incorrect, the parameter can be
used.
So, before enable PSR by default let's instrument the PSR
parameter
in a way that we can identify different panels out there that
might
require or work better with link standby mode.
It is also useful to say that for backward compatibility I'm not
changing the meaning of this flag. So "0" still means disabled
and "1" means enabled with full support and maximum power
savings.
v2: Use positive value instead of negative for different
operation
mode
as suggested by Daniel.
v3: As Paulo suggested use 2 to force link standby and 3 to force
link
fully on. Also split the link_standby introduction in a
separated
patch.
Cc: Paulo Zanoni
Cc: Daniel Vetter
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/i915_params.c | 7 ++-
drivers/gpu/drm/i915/intel_psr.c | 17 +
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c
b/drivers/gpu/drm/i915/i915_params.c
index 835d609..f78ddf3 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -126,7 +126,12 @@ MODULE_PARM_DESC(enable_execlists,
"(-1=auto [default], 0=disabled, 1=enabled)");
module_param_named_unsafe(enable_psr, i915.enable_psr, int,
0600);
-MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)");
+MODULE_PARM_DESC(enable_psr, "Enable PSR "
+"(0=disabled [default], 1=enabled - link mode
chosen per-platform, 2=force link-standby mode, 3=force link-off
mode)"
+"In case you needed to force any different
option,
please "
+"report PCI device ID, subsystem vendor and
subsystem device ID "
+"to intel-gfx@lists.freedesktop.org, if your
machine needs it. "
+"It will then be included in an upcoming module
version.");
Are we making a promise here? Isn't that dangerous? :P
I'd just tell the users to open bug reports.
(I'm not requiring you to change anything here, but something
something
lawyers something)
module_param_named_unsafe(preliminary_hw_support,
i915.preliminary_hw_support, int, 0600);
MODULE_PARM_DESC(preliminary_hw_support,
diff --git a/drivers/gpu/drm/i915/intel_psr.c
b/drivers/gpu/drm/i915/intel_psr.c
index b84ec80..c3c2bb8 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -335,6 +335,12 @@ static bool
intel_psr_match_conditions(struct
intel_dp *intel_dp)
return false;
}
+ if ((IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) &&
+ dev_priv->psr.link_standby) {
IS_VALLEYVIEW() will return true for both valleyview and cherryview
so
the above check for cherryview can be removed.
Not anymore:
commit 666a45379e2c29bc16e60648e5ad8f6f8b7fa6ce
Author: Wayne Boyer
Date: Wed Dec 9 12:29:35 2015 -0800
drm/i915: Separate cherryview from valleyview
Thanks for commit id of patch :). jumping on upstream every
now and then will lead to missing a lot of changes happening.
s/dev_priv->psr.link_standby/!dev_priv->psr.link_standby/
Also, I'm not sure if this chunk belongs here or at
intel_psr_init(),
since it effectively disables PSR. This means that
i915.enable_psr=3
disables PSR on VLV/CHV. But maybe we shouldn't care since users
shouldn't be using the option anyway. On the other hand, users may
start claiming that i915.enable_psr=X "fixed PSR" for them while
effectively it just disabled PSR, so perhaps DRM_ERROR would be
better.
Anyway, I'm not requesting any change, just pointing things in case
you
or someone else has any idea, but maybe I'd go with DRM_ERROR since
users usually don't know which platform supports what, so the loud
message may help them.
i agree, psr_match_conditions should check for parameters that can
change
dynamically post boot to decide if we can enable psr or not,
if link_standby cannot be changed post boot we should check for it in
init
so we can avoid psr being enabled in the first place.
Another check which we seem to be missing is "if (HAS_DDI(dev_priv)
&&
transcoder != TRANSCODER_EDP && !dev_priv->psr.link_standby)", but
this
depends on the result of the discussion of patch 1.
Everything else looks good, but it would be nice to see the
opinions of
maintainers here since they always have somethin
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: Fix premature LRC unpin in GuC mode
On Thu, Jan 21, 2016 at 12:14:10PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > In GuC mode LRC pinning lifetime depends exclusively on the > request liftime. Since that is terminated by the seqno update > that opens up a race condition between GPU finishing writing > out the context image and the driver unpinning the LRC. > > To extend the LRC lifetime we will employ a similar approach > to what legacy ringbuffer submission does. > > We will start tracking the last submitted context per engine > and keep it pinned until it is replaced by another one. > > Note that the driver unload path is a bit fragile and could > benefit greatly from efforts to unify the legacy and exec > list submission code paths. > > At the moment i915_gem_context_fini has special casing for the > two which are potentialy not needed, and also depends on > i915_gem_cleanup_ringbuffer running before itself. > > v2: > * Move pinning into engine->emit_request and actually fix >the reference/unreference logic. (Chris Wilson) > > * ring->dev can be NULL on driver unload so use a different >route towards it. > > v3: > * Rebase. > * Handle the reset path. (Chris Wilson) > * Exclude default context from the pinning - it is impossible >to get it right before default context special casing in >general is eliminated. > > v4: > * Rebased & moved context tracking to >intel_logical_ring_advance_and_submit. > > Signed-off-by: Tvrtko Ursulin > Issue: VIZ-4277 > Cc: Chris Wilson > Cc: Nick Hoath Whilst it saddens me to see yet another (impossible) special case added that will just have to be deleted again, the series is Reviewed-by: Chris Wilson I wonder if it is possible to poison the context objects before and after, then do a deferred check for stray writes, and use that mode for igt/gem_ctx_* (with some tests targetting active->idle vs context-close). Would still be susceptible to timing as we need to hit the interval between the seqno being complete and the delayed context save, but that seems like the most reliable way to detect the error? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/25] drm/i915/fbc: introduce struct intel_fbc_reg_params
Op 19-01-16 om 14:35 schreef Paulo Zanoni:
> The early return inside __intel_fbc_update does not completely check
> all the parameters that affect the FBC register values. For example,
> we currently lack looking at crtc->adjusted_y (for the fence Y offset)
> and all the parameters that affect the CFB size (for i8xx).
>
> Instead of just adding the missing parameters to the check and hoping
> that any changes to the fbc_activate functions also come with a
> matching change to the __intel_fbc_update check, introduce a new
> structure where we store these parameters and use the structure at the
> fbc_activate function. Of course, it's still possible to access
> everything from dev_priv in those functions, but IMHO the new code
> will be harder to break.
>
> Signed-off-by: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/i915_drv.h | 22 ++-
> drivers/gpu/drm/i915/intel_fbc.c | 131
> ++-
> 2 files changed, 93 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 33217a4..aa9c35e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -909,11 +909,9 @@ struct i915_fbc {
>* it's the outer lock when overlapping with stolen_lock. */
> struct mutex lock;
> unsigned threshold;
> - unsigned int fb_id;
> unsigned int possible_framebuffer_bits;
> unsigned int busy_bits;
> struct intel_crtc *crtc;
> - int y;
>
> struct drm_mm_node compressed_fb;
> struct drm_mm_node *compressed_llb;
> @@ -923,6 +921,24 @@ struct i915_fbc {
> bool enabled;
> bool active;
>
> + struct intel_fbc_reg_params {
> + struct {
> + enum pipe pipe;
> + enum plane plane;
> + unsigned int fence_y_offset;
> + } crtc;
> +
> + struct {
> + u64 ggtt_offset;
> + uint32_t id;
On a casual glance it looks like this fb.id is write-only now, but the whole
struct is being compared.
It might be worth making a note about that.
~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/25] drm/i915/fbc: introduce struct intel_fbc_reg_params
Em Qui, 2016-01-21 às 13:48 +0100, Maarten Lankhorst escreveu:
> Op 19-01-16 om 14:35 schreef Paulo Zanoni:
> > The early return inside __intel_fbc_update does not completely
> > check
> > all the parameters that affect the FBC register values. For
> > example,
> > we currently lack looking at crtc->adjusted_y (for the fence Y
> > offset)
> > and all the parameters that affect the CFB size (for i8xx).
> >
> > Instead of just adding the missing parameters to the check and
> > hoping
> > that any changes to the fbc_activate functions also come with a
> > matching change to the __intel_fbc_update check, introduce a new
> > structure where we store these parameters and use the structure at
> > the
> > fbc_activate function. Of course, it's still possible to access
> > everything from dev_priv in those functions, but IMHO the new code
> > will be harder to break.
> >
> > Signed-off-by: Paulo Zanoni
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 22 ++-
> > drivers/gpu/drm/i915/intel_fbc.c | 131 ++-
> >
> > 2 files changed, 93 insertions(+), 60 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 33217a4..aa9c35e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -909,11 +909,9 @@ struct i915_fbc {
> > * it's the outer lock when overlapping with stolen_lock.
> > */
> > struct mutex lock;
> > unsigned threshold;
> > - unsigned int fb_id;
> > unsigned int possible_framebuffer_bits;
> > unsigned int busy_bits;
> > struct intel_crtc *crtc;
> > - int y;
> >
> > struct drm_mm_node compressed_fb;
> > struct drm_mm_node *compressed_llb;
> > @@ -923,6 +921,24 @@ struct i915_fbc {
> > bool enabled;
> > bool active;
> >
> > + struct intel_fbc_reg_params {
> > + struct {
> > + enum pipe pipe;
> > + enum plane plane;
> > + unsigned int fence_y_offset;
> > + } crtc;
> > +
> > + struct {
> > + u64 ggtt_offset;
> > + uint32_t id;
> On a casual glance it looks like this fb.id is write-only now, but
> the whole struct is being compared.
It is read during intel_fbc_reg_params_equal(), so we're still
comparing it. I tried to be conservative and just remove the fb.id
comparison later, just to make bisecting easier in case something
happens. But we remove fb.id in patch 22/25.
>
> It might be worth making a note about that.
> ~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/25] drm/i915/fbc: unconditionally update FBC during atomic commits
Op 19-01-16 om 14:35 schreef Paulo Zanoni: > We unconditionally disable/update FBC even during the page flip > IOCTLs, and an unconditional disable/update at every atomic commit > touching the primary plane shouldn't impact PC state residency > noticeably. Besides, the code that checks for rotation is a good hint > that we may be forgetting something else, so let's leave all the > decisions to intel_fbc.c, making the code much safer. > > Once we have the code to properly make FBC enable/update decisions > based on atomic states, with proper locking, then we'll be able to > evaluate whether it will be worth trying to optimize the cases where a > disable isn't needed. > > Signed-off-by: Paulo Zanoni I would rather have this patch remove those 2 members entirely, but I can work with this for now. Could nuke at least disable_fbc though, being redundant with update_fbc. :) ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/skl/kbl: Add support for pipe fusing (rev3)
== Summary == Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest bdw-nuci7total:140 pass:131 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultratotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 bsw-nuc-2total:143 pass:119 dwarn:0 dfail:0 fail:0 skip:24 hsw-brixbox total:143 pass:136 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:143 pass:139 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:143 pass:104 dwarn:1 dfail:0 fail:0 skip:38 ivb-t430stotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 skl-i5k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 snb-dellxps total:143 pass:129 dwarn:0 dfail:0 fail:0 skip:14 snb-x220ttotal:143 pass:129 dwarn:0 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1234/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 14/22] drm/i915/slpc: Notification of Display mode change
Em Qua, 2016-01-20 às 18:26 -0800, tom.orou...@intel.com escreveu:
> From: Sagar Arun Kamble
>
> GuC SLPC need to be sent data related to Active pipes, refresh rates,
> widi pipes, fullscreen pipes related via host to GuC display mode
> change event.
> This patch defines the event and implements trigger of the event.
>
> Signed-off-by: Sagar Arun Kamble
> Acked-by: Tom O'Rourke
(this is just a comment from a quick look at the file, not a full
review)
> ---
> drivers/gpu/drm/i915/intel_display.c | 2 +
> drivers/gpu/drm/i915/intel_slpc.c| 92
>
> drivers/gpu/drm/i915/intel_slpc.h| 1 +
> 3 files changed, 95 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 06ab6df..7c3d902 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13638,6 +13638,8 @@ static int intel_atomic_commit(struct
> drm_device *dev,
> */
> intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
>
> + intel_slpc_update_display_mode_info(dev);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_slpc.c
> b/drivers/gpu/drm/i915/intel_slpc.c
> index f155d88..f5f7cad 100644
> --- a/drivers/gpu/drm/i915/intel_slpc.c
> +++ b/drivers/gpu/drm/i915/intel_slpc.c
> @@ -74,6 +74,27 @@ static int host2guc_slpc_shutdown(struct
> drm_i915_private *dev_priv)
> return ret;
> }
>
> +static int host2guc_slpc_display_mode_change(struct drm_i915_private
> *dev_priv)
> +{
> +u32 data[7];
> + int ret, i;
> +
> +data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
> +data[1] = SLPC_EVENT(SLPC_EVENT_DISPLAY_MODE_CHANGE,
> MAX_NUM_OF_PIPE + 1);
> + data[2] = dev_priv-
> >guc.slpc.display_mode_params.global_data;
> + for(i = 0; i < MAX_NUM_OF_PIPE; ++i)
> + data[3+i] = dev_priv-
> >guc.slpc.display_mode_params.per_pipe_info[i].data;
> +
> +ret = host2guc_action(&dev_priv->guc, data, 7);
> +
> + if (0 == ret) {
https://en.wikipedia.org/wiki/Yoda_conditions are not part of our usual
coding style.
> + ret = I915_READ(SOFT_SCRATCH(1));
> + ret &= 0xFF;
> + }
> +
> + return ret;
> +}
There are some tab/space issues in the function above.
Just a suggestion, not a request: for both functions you introduced,
since the only callers do not bother to check the return values, you
could just make the functions return void. Having unchecked return
values may give a false sense that errors would be caught by the upper
layer, when in fact they aren't. In a lot of cases it's better to just
print some error message instead of returning some value that is going
to be ignored.
> +
> static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
> {
> struct drm_device *dev = obj->base.dev;
> @@ -225,3 +246,74 @@ int intel_slpc_reset(struct drm_device *dev)
>
> return ret;
> }
> +
> +int intel_slpc_update_display_mode_info(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_crtc *crtc;
> + struct intel_crtc *intel_crtc;
> + struct drm_connector *connector;
> + struct intel_connector *intel_connector;
> + struct drm_encoder *encoder;
> + struct intel_encoder *intel_encoder;
> + struct intel_display_pipe_basic_info *per_pipe_info;
> + struct intel_slpc_display_mode_event_params *cur_params,
> old_params;
> + bool notify = false;
> +
> + if (!HAS_SLPC(dev))
> + return -EINVAL;
> +
> + cur_params = &dev_priv->guc.slpc.display_mode_params;
> +
> + /* Copy display mode parameters for comparison */
> + old_params.global_data = cur_params->global_data;
> + cur_params->global_data = 0;
> +
> + for_each_intel_crtc(dev, intel_crtc) {
> + per_pipe_info = &cur_params-
> >per_pipe_info[intel_crtc->pipe];
> + old_params.per_pipe_info[intel_crtc->pipe].data =
> per_pipe_info->data;
> + per_pipe_info->data = 0;
> + }
> +
> + for_each_intel_crtc(dev, intel_crtc) {
> + struct intel_crtc_state *pipe_config;
> +
> + per_pipe_info = &cur_params-
> >per_pipe_info[intel_crtc->pipe];
> + crtc = &intel_crtc->base;
> + pipe_config = to_intel_crtc_state(crtc->state);
> +
> + if (pipe_config->base.active) {
As far as I understand from the new atomic locking model, since this
code is called from intel_atomic_commit, it can't just iterate through
every crtc/encoder/connector checking state structures. During an
atomic commit we can only look at the objects affected by the commit,
so we have to use things such as for_each_crtc_in_state().
It seems to me that the model here could be:
- At driver load, during or after HW state readout, initialize some
state structure to make it match the hardware.
- Whenever there's an atomic commit, just look+update the status of the
affected
Re: [Intel-gfx] [PATCH 08/25] drm/i915/fbc: unconditionally update FBC during atomic commits
Em Qui, 2016-01-21 às 14:04 +0100, Maarten Lankhorst escreveu: > Op 19-01-16 om 14:35 schreef Paulo Zanoni: > > We unconditionally disable/update FBC even during the page flip > > IOCTLs, and an unconditional disable/update at every atomic commit > > touching the primary plane shouldn't impact PC state residency > > noticeably. Besides, the code that checks for rotation is a good > > hint > > that we may be forgetting something else, so let's leave all the > > decisions to intel_fbc.c, making the code much safer. > > > > Once we have the code to properly make FBC enable/update decisions > > based on atomic states, with proper locking, then we'll be able to > > evaluate whether it will be worth trying to optimize the cases > > where a > > disable isn't needed. > > > > Signed-off-by: Paulo Zanoni > I would rather have this patch remove those 2 members entirely, but I > can work with this for now. And what would be the new way to know whether a given atomic commit touches the primary plane of a given crtc? > > Could nuke at least disable_fbc though, being redundant with > update_fbc. :) Check patch 11 :) > > ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 15/25] drm/i915/fbc: rewrite the multiple_pipes_ok() code for locking
Op 19-01-16 om 14:35 schreef Paulo Zanoni: > Older FBC platforms have this restriction where FBC can't be enabled > if multiple pipes are enabled. In the current code, we disable FBC > before the second pipe becomes visible. > > One of the problems with this code is that the current > multiple_pipes_ok() implementation just iterates through all CRTCs > looking at their states, but it doesn't make sure that the state > locks are grabbed. It also can't just grab the locks for every CRTC > since this would kill one of the biggest advantages of atomic > modesetting. > > After the recent FBC changes, we now have the appropriate locks for > the given CRTC, so we can just try to maintain the state of each CRTC > and update it once intel_fbc_pre_update is called. > > As a last note, I don't have gen 2/3 machines to test this code. My > current plan is to enable FBC on just the newer platforms, so this > patch is just an attempt to get the gen 2/3 code at least looking > sane, so if one day someone decide to fix FBC on these platforms, they > may have less work to do. > It would still be nice if pre/post update took a crtc_state and plane_state so we wouldn't have to worry about lifetime issues. Right now it's pulled from various places. ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/25] drm/i915/fbc: unconditionally update FBC during atomic commits
Op 21-01-16 om 14:27 schreef Zanoni, Paulo R: > Em Qui, 2016-01-21 às 14:04 +0100, Maarten Lankhorst escreveu: >> Op 19-01-16 om 14:35 schreef Paulo Zanoni: >>> We unconditionally disable/update FBC even during the page flip >>> IOCTLs, and an unconditional disable/update at every atomic commit >>> touching the primary plane shouldn't impact PC state residency >>> noticeably. Besides, the code that checks for rotation is a good >>> hint >>> that we may be forgetting something else, so let's leave all the >>> decisions to intel_fbc.c, making the code much safer. >>> >>> Once we have the code to properly make FBC enable/update decisions >>> based on atomic states, with proper locking, then we'll be able to >>> evaluate whether it will be worth trying to optimize the cases >>> where a >>> disable isn't needed. >>> >>> Signed-off-by: Paulo Zanoni >> I would rather have this patch remove those 2 members entirely, but I >> can work with this for now. > And what would be the new way to know whether a given atomic commit > touches the primary plane of a given crtc? if (drm_atomic_get_existing_plane_state(old_crtc_state->state, crtc->primary)) >> Could nuke at least disable_fbc though, being redundant with >> update_fbc. :) > Check patch 11 :) > >> ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Handle fb->offsets[] and rewrite fb rotation handling to be more generic (v2)
== Summary == Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest bdw-nuci7total:140 pass:131 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultratotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 bsw-nuc-2total:143 pass:119 dwarn:0 dfail:0 fail:0 skip:24 byt-nuc total:143 pass:128 dwarn:0 dfail:0 fail:0 skip:15 hsw-brixbox total:143 pass:136 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:143 pass:139 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:143 pass:104 dwarn:1 dfail:0 fail:0 skip:38 ivb-t430stotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 skl-i5k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 snb-dellxps total:143 pass:129 dwarn:0 dfail:0 fail:0 skip:14 snb-x220ttotal:143 pass:129 dwarn:0 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1235/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for Add support for GuC-based SLPC
== Summary == Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest Test drv_getparams_basic: Subgroup basic-eu-total: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-subslice-total: pass -> DMESG-FAIL (skl-i5k-2) Test drv_hangman: Subgroup error-state-basic: pass -> DMESG-FAIL (skl-i5k-2) Test drv_module_reload_basic: pass -> DMESG-FAIL (skl-i5k-2) Test gem_basic: Subgroup bad-close: pass -> DMESG-FAIL (skl-i5k-2) Subgroup create-close: pass -> DMESG-FAIL (skl-i5k-2) Subgroup create-fd-close: pass -> DMESG-FAIL (skl-i5k-2) Test gem_cpu_reloc: Subgroup basic: pass -> DMESG-FAIL (skl-i5k-2) Test gem_ctx_basic: pass -> DMESG-FAIL (skl-i5k-2) Test gem_ctx_create: Subgroup basic: pass -> DMESG-FAIL (skl-i5k-2) Test gem_ctx_exec: Subgroup basic: pass -> DMESG-FAIL (skl-i5k-2) Test gem_ctx_param_basic: Subgroup basic: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-default: pass -> DMESG-FAIL (skl-i5k-2) Subgroup invalid-ctx-get: pass -> DMESG-FAIL (skl-i5k-2) Subgroup invalid-ctx-set: pass -> DMESG-FAIL (skl-i5k-2) Subgroup invalid-param-get: pass -> DMESG-FAIL (skl-i5k-2) Subgroup invalid-param-set: pass -> DMESG-FAIL (skl-i5k-2) Subgroup invalid-size-get: pass -> DMESG-FAIL (skl-i5k-2) Subgroup invalid-size-set: pass -> DMESG-FAIL (skl-i5k-2) Subgroup non-root-set: pass -> DMESG-FAIL (skl-i5k-2) Subgroup non-root-set-no-zeromap: pass -> DMESG-FAIL (skl-i5k-2) Subgroup root-set: pass -> DMESG-FAIL (skl-i5k-2) Subgroup root-set-no-zeromap-disabled: pass -> DMESG-FAIL (skl-i5k-2) Subgroup root-set-no-zeromap-enabled: pass -> DMESG-FAIL (skl-i5k-2) Test gem_exec_parse: Subgroup basic-allowed: skip -> DMESG-FAIL (skl-i5k-2) Subgroup basic-rejected: skip -> DMESG-FAIL (skl-i5k-2) Test gem_flink_basic: Subgroup bad-flink: pass -> DMESG-FAIL (skl-i5k-2) Subgroup bad-open: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic: pass -> DMESG-FAIL (skl-i5k-2) Subgroup double-flink: pass -> DMESG-FAIL (skl-i5k-2) Subgroup flink-lifetime: pass -> DMESG-FAIL (skl-i5k-2) Test gem_linear_blits: Subgroup basic: pass -> DMESG-FAIL (skl-i5k-2) Test gem_mmap: Subgroup basic: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-small-bo: pass -> DMESG-FAIL (skl-i5k-2) Test gem_mmap_gtt: Subgroup basic: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-copy: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-read: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-read-no-prefault: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-read-write: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-read-write-distinct: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-short: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-small-bo: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-small-bo-tiledx: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-small-bo-tiledy: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-small-copy: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-small-copy-xy: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-write: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-write-cpu-read-gtt: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-write-gtt: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-write-gtt-no-prefault: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-write-no-prefault: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-write-read: pass -> DMESG-FAIL (skl-i5k-2) Subgroup basic-write-read-distinct: pass -> DMESG-FAIL (skl-i5k-2) Test gem_pread: Subgroup basic:
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: Fix premature LRC unpin in GuC mode
On 21/01/16 12:32, Chris Wilson wrote: On Thu, Jan 21, 2016 at 12:14:10PM +, Tvrtko Ursulin wrote: From: Tvrtko Ursulin In GuC mode LRC pinning lifetime depends exclusively on the request liftime. Since that is terminated by the seqno update that opens up a race condition between GPU finishing writing out the context image and the driver unpinning the LRC. To extend the LRC lifetime we will employ a similar approach to what legacy ringbuffer submission does. We will start tracking the last submitted context per engine and keep it pinned until it is replaced by another one. Note that the driver unload path is a bit fragile and could benefit greatly from efforts to unify the legacy and exec list submission code paths. At the moment i915_gem_context_fini has special casing for the two which are potentialy not needed, and also depends on i915_gem_cleanup_ringbuffer running before itself. v2: * Move pinning into engine->emit_request and actually fix the reference/unreference logic. (Chris Wilson) * ring->dev can be NULL on driver unload so use a different route towards it. v3: * Rebase. * Handle the reset path. (Chris Wilson) * Exclude default context from the pinning - it is impossible to get it right before default context special casing in general is eliminated. v4: * Rebased & moved context tracking to intel_logical_ring_advance_and_submit. Signed-off-by: Tvrtko Ursulin Issue: VIZ-4277 Cc: Chris Wilson Cc: Nick Hoath Whilst it saddens me to see yet another (impossible) special case added that will just have to be deleted again, the series is Reviewed-by: Chris Wilson Thanks and sorry, hopefully it will get cleanup up soon. There seems to be a growing number of people who want it done. And I still need to get back to your VMA rewrite and breadcrumbs would be nice as well. I wonder if it is possible to poison the context objects before and after, then do a deferred check for stray writes, and use that mode for igt/gem_ctx_* (with some tests targetting active->idle vs context-close). Would still be susceptible to timing as we need to hit the interval between the seqno being complete and the delayed context save, but that seems like the most reliable way to detect the error? First it needs to be tested with GuC to check that it actually fixes the issue. And pass CI of course. But I can't really figure where would you put this poisoning? You could put something in in exec list mode after context complete and check it before it is used next time, but I did not think we can hit this in exec list mode, only in GuC. You think it is possible? And in GuC mode I have no idea at which point you would put "poisoning" in? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 0/8] Gen9 HW whitelist and Preemption WA patches
Resending all patches as told by Daniel because I didn't use correct message-id while replying updated version of Patch1 before which means patchwork won't pickup and we won't have CI results. Previous updates regarding Patch1 are available at https://patchwork.freedesktop.org/patch/70527/ Some of the WA patches are now reviewed so I added r-b tags for them. Reviewed: 1, 2, 3, 5, 6 Updated: 4 (to address review comment) To be reviewed: 4, 7, 8 Arun Siluvery (8): drm/i915/gen9: Add framework to whitelist specific GPU registers drm/i915/gen9: Add GEN8_CS_CHICKEN1 to HW whitelist drm/i915/gen9: Add HDC_CHICKEN1 to HW whitelist drm/i915/bxt: Add GEN9_CS_DEBUG_MODE1 to HW whitelist drm/i915/bxt: Add GEN8_L3SQCREG4 to HW whitelist drm/i915/skl: Add GEN8_L3SQCREG4 to HW whitelist drm/i915/skl: Enable Per context Preemption granularity control drm/i915/gen9: Add WaOCLCoherentLineFlush drivers/gpu/drm/i915/i915_debugfs.c | 15 +--- drivers/gpu/drm/i915/i915_drv.h | 9 - drivers/gpu/drm/i915/i915_reg.h | 11 ++ drivers/gpu/drm/i915/intel_ringbuffer.c | 61 + 4 files changed, 90 insertions(+), 6 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/8] drm/i915/gen9: Add GEN8_CS_CHICKEN1 to HW whitelist
Required for WaEnablePreemptionGranularityControlByUMD:skl,bxt
Reviewed-by: Nick Hoath
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_ringbuffer.c | 6 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7938814..511732e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5998,6 +5998,8 @@ enum skl_disp_power_wells {
#define FF_SLICE_CS_CHICKEN2 _MMIO(0x20e4)
#define GEN9_TSG_BARRIER_ACK_DISABLE (1<<8)
+#define GEN8_CS_CHICKEN1 _MMIO(0x2580)
+
/* GEN7 chicken */
#define GEN7_COMMON_SLICE_CHICKEN1 _MMIO(0x7010)
# define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC ((1<<10) | (1<<26))
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 56af736..47d8767 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -908,6 +908,7 @@ static int gen9_init_workarounds(struct intel_engine_cs
*ring)
struct drm_device *dev = ring->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t tmp;
+ int ret;
/* WaEnableLbsSlaRetryTimerDecrement:skl */
I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) |
@@ -978,6 +979,11 @@ static int gen9_init_workarounds(struct intel_engine_cs
*ring)
/* WaDisableSTUnitPowerOptimization:skl,bxt */
WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE);
+ /* WaEnablePreemptionGranularityControlByUMD:skl,bxt */
+ ret= wa_ring_whitelist_reg(ring, GEN8_CS_CHICKEN1);
+ if (ret)
+ return ret;
+
return 0;
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 5/8] drm/i915/bxt: Add GEN8_L3SQCREG4 to HW whitelist
Required for WaDisableLSQCROPERFforOCL:bxt
According to WA database these are only applicable for BXT:A0 but since
A0 and A1 shares the same GT these are extended for A1 as well.
Reviewed-by: Nick Hoath
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 72e89b6..1decaf1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1134,10 +1134,15 @@ static int bxt_init_workarounds(struct intel_engine_cs
*ring)
/* WaDisableObjectLevelPreemptionForTrifanOrPolygon:bxt */
/* WaDisableObjectLevelPreemptionForInstancedDraw:bxt */
/* WaDisableObjectLevelPreemtionForInstanceId:bxt */
+ /* WaDisableLSQCROPERFforOCL:bxt */
if (IS_BXT_REVID(dev, 0, BXT_REVID_A1)) {
ret = wa_ring_whitelist_reg(ring, GEN9_CS_DEBUG_MODE1);
if (ret)
return ret;
+
+ ret = wa_ring_whitelist_reg(ring, GEN8_L3SQCREG4);
+ if (ret)
+ return ret;
}
return 0;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/8] drm/i915/gen9: Add HDC_CHICKEN1 to HW whitelist
Required for WaAllowUMDToModifyHDCChicken1:skl,bxt
Reviewed-by: Nick Hoath
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 511732e..ed887cf 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6045,6 +6045,8 @@ enum skl_disp_power_wells {
#define HDC_FORCE_NON_COHERENT(1<<4)
#define HDC_BARRIER_PERFORMANCE_DISABLE (1<<10)
+#define GEN8_HDC_CHICKEN1 _MMIO(0x7304)
+
/* GEN9 chicken */
#define SLICE_ECO_CHICKEN0 _MMIO(0x7308)
#define PIXEL_MASK_CAMMING_DISABLE (1 << 14)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 47d8767..fea632f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -984,6 +984,11 @@ static int gen9_init_workarounds(struct intel_engine_cs
*ring)
if (ret)
return ret;
+ /* WaAllowUMDToModifyHDCChicken1:skl,bxt */
+ ret = wa_ring_whitelist_reg(ring, GEN8_HDC_CHICKEN1);
+ if (ret)
+ return ret;
+
return 0;
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/8] drm/i915/gen9: Add framework to whitelist specific GPU registers
Some of the HW registers are privileged and cannot be written to from
non-privileged batch buffers coming from userspace unless they are added to
the HW whitelist. This whitelist is maintained by HW and it is different from
SW whitelist. Userspace need write access to them to implement preemption
related WA.
The reason for using this approach is, the register bits that control
preemption granularity at the HW level are not context save/restored; so even
if we set these bits always in kernel they are going to change once the
context is switched out. We can consider making them non-privileged by
default but these registers also contain other chicken bits which should not
be allowed to be modified.
In the later revisions controlling bits are save/restored at context level but
in the existing revisions these are exported via other debug registers and
should be on the whitelist. This patch adds changes to provide HW with a list
of registers to be whitelisted. HW checks this list during execution and
provides access accordingly.
HW imposes a limit on the number of registers on whitelist and it is
per-engine. At this point we are only enabling whitelist for RCS and we don't
foresee any requirement for other engines.
The registers to be whitelisted are added using generic workaround list
mechanism, even these are only enablers for userspace workarounds. But by
sharing this mechanism we get some test assets without additional cost (Mika).
v2: rebase
v3: parameterize RING_FORCE_TO_NONPRIV() as _MMIO() should be limited to
i915_reg.h (Ville), drop inline for wa_ring_whitelist_reg (Mika).
v4: improvements suggested by Chris Wilson.
Clarify that this is HW whitelist and different from the one maintained in
driver. This list is engine specific but it gets initialized along with other
WA which is RCS specific thing, so make it clear that we are not doing any
cross engine setup during initialization.
Make HW whitelist count of each engine available in debugfs.
Reviewed-by: Chris Wilson
Reviewed-by: Mika Kuoppala
Cc: Mika Kuoppala
Cc: Chris Wilson
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/i915_debugfs.c | 15 ++-
drivers/gpu/drm/i915/i915_drv.h | 9 -
drivers/gpu/drm/i915/i915_reg.h | 3 +++
drivers/gpu/drm/i915/intel_ringbuffer.c | 17 +
4 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index e3377ab..7eb002c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3229,9 +3229,11 @@ static int i915_wa_registers(struct seq_file *m, void
*unused)
{
int i;
int ret;
+ struct intel_engine_cs *ring;
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct i915_workarounds *workarounds = &dev_priv->workarounds;
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
@@ -3239,15 +3241,18 @@ static int i915_wa_registers(struct seq_file *m, void
*unused)
intel_runtime_pm_get(dev_priv);
- seq_printf(m, "Workarounds applied: %d\n", dev_priv->workarounds.count);
- for (i = 0; i < dev_priv->workarounds.count; ++i) {
+ seq_printf(m, "Workarounds applied: %d\n", workarounds->count);
+ for_each_ring(ring, dev_priv, i)
+ seq_printf(m, "HW whitelist count for %s: %d\n",
+ ring->name, workarounds->hw_whitelist_count[i]);
+ for (i = 0; i < workarounds->count; ++i) {
i915_reg_t addr;
u32 mask, value, read;
bool ok;
- addr = dev_priv->workarounds.reg[i].addr;
- mask = dev_priv->workarounds.reg[i].mask;
- value = dev_priv->workarounds.reg[i].value;
+ addr = workarounds->reg[i].addr;
+ mask = workarounds->reg[i].mask;
+ value = workarounds->reg[i].value;
read = I915_READ(addr);
ok = (value & mask) == (read & mask);
seq_printf(m, "0x%X: 0x%08X, mask: 0x%08X, read: 0x%08x,
status: %s\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 104bd18..83fccc0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1653,11 +1653,18 @@ struct i915_wa_reg {
u32 mask;
};
-#define I915_MAX_WA_REGS 16
+/*
+ * RING_MAX_NONPRIV_SLOTS is per-engine but at this point we are only
+ * allowing it for RCS as we don't foresee any requirement of having
+ * a whitelist for other engines. When it is really required for
+ * other engines then the limit need to be increased.
+ */
+#define I915_MAX_WA_REGS (16 + RING_MAX_NONPRIV_SLOTS)
struct i915_workarounds {
struct i915_wa_reg reg[I915_MAX_WA_REGS];
u32 count;
+
[Intel-gfx] [PATCH v2 8/8] drm/i915/gen9: Add WaOCLCoherentLineFlush
This is mainly required for preemption. Cc: Dave Gordon Signed-off-by: Arun Siluvery --- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index e91fb70..f26f274 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -979,6 +979,10 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) /* WaDisableSTUnitPowerOptimization:skl,bxt */ WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE); + /* WaOCLCoherentLineFlush:skl,bxt */ + I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) | + GEN8_LQSC_FLUSH_COHERENT_LINES)); + /* WaEnablePreemptionGranularityControlByUMD:skl,bxt */ ret= wa_ring_whitelist_reg(ring, GEN8_CS_CHICKEN1); if (ret) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 7/8] drm/i915/skl: Enable Per context Preemption granularity control
Per context preemption granularity control is only available from SKL:E0+
Cc: Dave Gordon
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/i915_reg.h | 3 +++
drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++
2 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c51e7e9..65e32a3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5995,6 +5995,9 @@ enum skl_disp_power_wells {
#define SKL_DFSM_CDCLK_LIMIT_450 (2 << 23)
#define SKL_DFSM_CDCLK_LIMIT_337_5 (3 << 23)
+#define GEN7_FF_SLICE_CS_CHICKEN1 _MMIO(0x20e0)
+#define GEN9_FFSC_PERCTX_PREEMPT_CTRL(1<<14)
+
#define FF_SLICE_CS_CHICKEN2 _MMIO(0x20e4)
#define GEN9_TSG_BARRIER_ACK_DISABLE (1<<8)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ce64519..e91fb70 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1044,6 +1044,16 @@ static int skl_init_workarounds(struct intel_engine_cs
*ring)
if (ret)
return ret;
+ /*
+* Actual WA is to disable percontext preemption granularity control
+* until D0 which is the default case so this is equivalent to
+* !WaDisablePerCtxtPreemptionGranularityControl:skl
+*/
+ if (IS_SKL_REVID(dev, SKL_REVID_E0, REVID_FOREVER)) {
+ I915_WRITE(GEN7_FF_SLICE_CS_CHICKEN1,
+ _MASKED_BIT_ENABLE(GEN9_FFSC_PERCTX_PREEMPT_CTRL));
+ }
+
if (IS_SKL_REVID(dev, 0, SKL_REVID_D0)) {
/* WaDisableChickenBitTSGBarrierAckForFFSliceCS:skl */
I915_WRITE(FF_SLICE_CS_CHICKEN2,
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 6/8] drm/i915/skl: Add GEN8_L3SQCREG4 to HW whitelist
Required for WaDisableLSQCROPERFforOCL:skl Reviewed-by: Nick Hoath Signed-off-by: Arun Siluvery --- drivers/gpu/drm/i915/intel_ringbuffer.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 1decaf1..ce64519 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1096,6 +1096,11 @@ static int skl_init_workarounds(struct intel_engine_cs *ring) GEN7_HALF_SLICE_CHICKEN1, GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE); + /* WaDisableLSQCROPERFforOCL:skl */ + ret = wa_ring_whitelist_reg(ring, GEN8_L3SQCREG4); + if (ret) + return ret; + return skl_tune_iz_hashing(ring); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/8] drm/i915/bxt: Add GEN9_CS_DEBUG_MODE1 to HW whitelist
Required for,
WaDisableObjectLevelPreemptionForTrifanOrPolygon:bxt
WaDisableObjectLevelPreemptionForInstancedDraw:bxt
WaDisableObjectLevelPreemtionForInstanceId:bxt
According to WA database these are only applicable for BXT:A0 but since
A0 and A1 shares the same GT these are extended for A1 as well.
These are also required for SKL until B0 but not adding them because they
are pre-production steppings.
v2: use lower case in register defines (Nick)
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ed887cf..c51e7e9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5998,6 +5998,7 @@ enum skl_disp_power_wells {
#define FF_SLICE_CS_CHICKEN2 _MMIO(0x20e4)
#define GEN9_TSG_BARRIER_ACK_DISABLE (1<<8)
+#define GEN9_CS_DEBUG_MODE1_MMIO(0x20ec)
#define GEN8_CS_CHICKEN1 _MMIO(0x2580)
/* GEN7 chicken */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index fea632f..72e89b6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1131,6 +1131,15 @@ static int bxt_init_workarounds(struct intel_engine_cs
*ring)
GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
}
+ /* WaDisableObjectLevelPreemptionForTrifanOrPolygon:bxt */
+ /* WaDisableObjectLevelPreemptionForInstancedDraw:bxt */
+ /* WaDisableObjectLevelPreemtionForInstanceId:bxt */
+ if (IS_BXT_REVID(dev, 0, BXT_REVID_A1)) {
+ ret = wa_ring_whitelist_reg(ring, GEN9_CS_DEBUG_MODE1);
+ if (ret)
+ return ret;
+ }
+
return 0;
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v2,1/4] drm/i915: Make LRC (un)pinning work on context and engine
== Summary == Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> DMESG-WARN (ilk-hp8440p) bdw-nuci7total:140 pass:131 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultratotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 bsw-nuc-2total:143 pass:119 dwarn:0 dfail:0 fail:0 skip:24 byt-nuc total:143 pass:128 dwarn:0 dfail:0 fail:0 skip:15 hsw-brixbox total:143 pass:136 dwarn:0 dfail:0 fail:0 skip:7 ilk-hp8440p total:143 pass:103 dwarn:2 dfail:0 fail:0 skip:38 ivb-t430stotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 skl-i5k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 snb-dellxps total:143 pass:129 dwarn:0 dfail:0 fail:0 skip:14 snb-x220ttotal:143 pass:129 dwarn:0 dfail:0 fail:1 skip:13 HANGED hsw-gt2 in igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a Results at /archive/results/CI_IGT_test/Patchwork_1237/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: Fix premature LRC unpin in GuC mode
On Thu, Jan 21, 2016 at 01:51:30PM +, Tvrtko Ursulin wrote: > But I can't really figure where would you put this poisoning? You > could put something in in exec list mode after context complete and > check it before it is used next time, I was thinking just in context-free. Move the pages to a poisoned list and check at the end of the test. The issue is that the GPU may write to the pages as we release them, so instead of releasing them we just poison them (or CRC them). > but I did not think we can hit > this in exec list mode, only in GuC. You think it is possible? With the current code, no since the last context unreference is done from i915_gem_request_free(), and the request reference is only dropped after we see the context-switch/active->idle state change (i.e. the context save should always be flushed by the time we unpin and free the context). However, that ordering imposes the struct_mutex upon request-free which leads to fairly severe issues that can be eleviated by moving the context-unreference into the request-retire - which opens up the context-close race to normal execlists, as the context can be unreferenced before the next context-switch-interrupt, now fixed in this patch. And reducing the context-pin lifetime even further should help mitigate context thrashing and certain userspace stress tests (OpenGL microbenchmarks). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/25] drm/i915/fbc: wait for a vblank instead of 50ms when enabling
Op 19-01-16 om 14:35 schreef Paulo Zanoni:
> Instead of waiting for 50ms, just wait until the next vblank, since
> it's the minimum requirement. The whole infrastructure of FBC is based
> on vblanks, so waiting for X vblanks instead of X milliseconds sounds
> like the correct way to go. Besides, 50ms may be less than a vblank on
> super slow modes that may or may not exist.
>
> There are some small improvements in PC state residency (due to the
> fact that we're now using 16ms for the common modes instead of 50ms),
> but the biggest advantage is still the correctness of being
> vblank-based instead of time-based.
>
> v2:
> - Rebase after changing the patch order.
> - Update the commit message.
> v3:
> - Fix bogus vblank_get() instead of vblank_count() (Ville).
> - Don't forget to call drm_crtc_vblank_{get,put} (Chris, Ville)
> - Adjust the performance details on the commit message.
>
> Signed-off-by: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_fbc.c | 43
>
> 2 files changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index af30148..33217a4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -925,9 +925,9 @@ struct i915_fbc {
>
> struct intel_fbc_work {
> bool scheduled;
> + u32 scheduled_vblank;
> struct work_struct work;
> struct drm_framebuffer *fb;
> - unsigned long enable_jiffies;
> } work;
>
> const char *no_fbc_reason;
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> b/drivers/gpu/drm/i915/intel_fbc.c
> index a1988a4..6b43ec3 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -381,7 +381,15 @@ static void intel_fbc_work_fn(struct work_struct *__work)
> container_of(__work, struct drm_i915_private, fbc.work.work);
> struct intel_fbc_work *work = &dev_priv->fbc.work;
> struct intel_crtc *crtc = dev_priv->fbc.crtc;
> - int delay_ms = 50;
> + struct drm_vblank_crtc *vblank = &dev_priv->dev->vblank[crtc->pipe];
> +
> + mutex_lock(&dev_priv->fbc.lock);
> + if (drm_crtc_vblank_get(&crtc->base)) {
> + DRM_ERROR("vblank not available for FBC on pipe %c\n",
> + pipe_name(crtc->pipe));
> + goto out;
> + }
> + mutex_unlock(&dev_priv->fbc.lock);
What does the lock protect here?
~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/25] FBC crtc/fb locking + smaller fixes
Op 19-01-16 om 14:35 schreef Paulo Zanoni:
> Hi
>
> Here's yet another patch series randomly modifying the FBC code. We
> start by refactoring things in order to fix the locking problems, then
> fix a few other smaller problems and apply some polishing.
>
> Just to keep the tradition of the past 10 cover letters, I guess I
> should say that this series is the last one and that we may consider
> enabling FBC on HSW/BDW/SKL after it is merged :)
>
> For SKL specifically, I tested this series on a two-weeks old version
> of drm-intel-nightly since today's version is giving me a BUG() even
> without my patches applied.
>
> Thanks,
> Paulo
>
> Paulo Zanoni (25):
> drm/i915/fbc: wait for a vblank instead of 50ms when enabling
> drm/i915/fbc: extract intel_fbc_can_activate()
> drm/i915/fbc: extract intel_fbc_can_enable()
> drm/i915/fbc: introduce struct intel_fbc_reg_params
> drm/i915/fbc: replace frequent dev_priv->fbc.x with fbc->x
> drm/i915/fbc: don't use the frontbuffer tracking subsystem for flips
> drm/i915/fbc: don't flush for operations on the wrong frontbuffer
> drm/i915/fbc: unconditionally update FBC during atomic commits
> drm/i915/fbc: introduce struct intel_fbc_state_cache
> drm/i915/fbc: split intel_fbc_update into pre and post update
> drm/i915/fbc: fix the FBC state checking code
> drm/i915/fbc: unexport intel_fbc_deactivate
> drm/i915/fbc: rename the FBC disable functions
> drm/i915/fbc: make sure we cancel the work function at fbc_disable
> drm/i915/fbc: rewrite the multiple_pipes_ok() code for locking
> drm/i915: simplify struct drm_device access at intel_atomic_check()
> drm/i915/fbc: choose the new FBC CRTC during atomic check
> drm/i915/fbc: move intel_fbc_{enable,disable} call one level up
> drm/i915/fbc: make FBC work with fastboot
> drm/i915/fbc: don't try to deactivate FBC if it's not enabled
> drm/i915/fbc: don't print no_fbc_reason to dmesg
> drm/i915/fbc: don't store the fb_id on reg_params
> drm/i915/fbc: call intel_fbc_pre_update earlier during page flips
> drm/i915/fbc: don't store/check a pointer to the FB
> drm/i915/fbc: refactor some small functions called only once
>
For the whole series except patch 1
Reviewed-by: Maarten Lankhorst
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for Gen9 HW whitelist and Preemption WA patches (rev2)
== Summary == Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> DMESG-WARN (ilk-hp8440p) bdw-nuci7total:140 pass:131 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultratotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 bsw-nuc-2total:143 pass:119 dwarn:0 dfail:0 fail:0 skip:24 byt-nuc total:143 pass:128 dwarn:0 dfail:0 fail:0 skip:15 hsw-brixbox total:143 pass:136 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:143 pass:139 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:143 pass:103 dwarn:2 dfail:0 fail:0 skip:38 ivb-t430stotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 skl-i5k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 snb-dellxps total:143 pass:129 dwarn:0 dfail:0 fail:0 skip:14 snb-x220ttotal:143 pass:129 dwarn:0 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1238/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for Gen9 HW whitelist and Preemption WA patches (rev2)
On 21/01/2016 14:25, Patchwork wrote: == Summary == Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> DMESG-WARN (ilk-hp8440p) bdw-nuci7total:140 pass:131 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultratotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 bsw-nuc-2total:143 pass:119 dwarn:0 dfail:0 fail:0 skip:24 byt-nuc total:143 pass:128 dwarn:0 dfail:0 fail:0 skip:15 hsw-brixbox total:143 pass:136 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:143 pass:139 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:143 pass:103 dwarn:2 dfail:0 fail:0 skip:38 ivb-t430stotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 skl-i5k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 skl-i7k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 snb-dellxps total:143 pass:129 dwarn:0 dfail:0 fail:0 skip:14 snb-x220ttotal:143 pass:129 dwarn:0 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1238/ Known issue, this test has been failing on ilk-hp8440p even before, Bug 93787 - [BAT ILK] sporadic fifo underruns in igt@kms_flip@basic-flip-vs-* on ilk-hp8440p https://bugs.freedesktop.org/show_bug.cgi?id=93787 regards Arun ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/8] drm/i915/skl: Enable Per context Preemption granularity control
On 13/01/2016 10:06, Arun Siluvery wrote:
Per context preemption granularity control is only available from SKL:E0+
Cc: Dave Gordon
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/i915_reg.h | 3 +++
drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++
2 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index eabd2af..97774a3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5995,6 +5995,9 @@ enum skl_disp_power_wells {
#define SKL_DFSM_CDCLK_LIMIT_450 (2 << 23)
#define SKL_DFSM_CDCLK_LIMIT_337_5(3 << 23)
+#define GEN7_FF_SLICE_CS_CHICKEN1 _MMIO(0x20E0)
0x20e0?
+#define GEN9_FFSC_PERCTX_PREEMPT_CTRL(1<<14)
+
#define FF_SLICE_CS_CHICKEN2 _MMIO(0x20e4)
#define GEN9_TSG_BARRIER_ACK_DISABLE (1<<8)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index b8dbd2c..5a2ad10 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1045,6 +1045,16 @@ static int skl_init_workarounds(struct intel_engine_cs
*ring)
if (ret)
return ret;
+ /*
+* Actual WA is to disable percontext preemption granularity control
+* until D0 which is the default case so this is equivalent to
+* !WaDisablePerCtxtPreemptionGranularityControl:skl
+*/
+ if (IS_SKL_REVID(dev, SKL_REVID_E0, REVID_FOREVER)) {
+ I915_WRITE(GEN7_FF_SLICE_CS_CHICKEN1,
+ _MASKED_BIT_ENABLE(GEN9_FFSC_PERCTX_PREEMPT_CTRL));
+ }
+
if (IS_SKL_REVID(dev, 0, SKL_REVID_D0)) {
/* WaDisableChickenBitTSGBarrierAckForFFSliceCS:skl */
I915_WRITE(FF_SLICE_CS_CHICKEN2,
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 7/8] drm/i915/skl: Enable Per context Preemption granularity control
On 21/01/2016 14:00, Arun Siluvery wrote:
Per context preemption granularity control is only available from SKL:E0+
Cc: Dave Gordon
Signed-off-by: Arun Siluvery
Reviewed-by: Nick Hoath
---
drivers/gpu/drm/i915/i915_reg.h | 3 +++
drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++
2 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c51e7e9..65e32a3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5995,6 +5995,9 @@ enum skl_disp_power_wells {
#define SKL_DFSM_CDCLK_LIMIT_450 (2 << 23)
#define SKL_DFSM_CDCLK_LIMIT_337_5(3 << 23)
+#define GEN7_FF_SLICE_CS_CHICKEN1 _MMIO(0x20e0)
+#define GEN9_FFSC_PERCTX_PREEMPT_CTRL(1<<14)
+
#define FF_SLICE_CS_CHICKEN2 _MMIO(0x20e4)
#define GEN9_TSG_BARRIER_ACK_DISABLE (1<<8)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ce64519..e91fb70 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1044,6 +1044,16 @@ static int skl_init_workarounds(struct intel_engine_cs
*ring)
if (ret)
return ret;
+ /*
+* Actual WA is to disable percontext preemption granularity control
+* until D0 which is the default case so this is equivalent to
+* !WaDisablePerCtxtPreemptionGranularityControl:skl
+*/
+ if (IS_SKL_REVID(dev, SKL_REVID_E0, REVID_FOREVER)) {
+ I915_WRITE(GEN7_FF_SLICE_CS_CHICKEN1,
+ _MASKED_BIT_ENABLE(GEN9_FFSC_PERCTX_PREEMPT_CTRL));
+ }
+
if (IS_SKL_REVID(dev, 0, SKL_REVID_D0)) {
/* WaDisableChickenBitTSGBarrierAckForFFSliceCS:skl */
I915_WRITE(FF_SLICE_CS_CHICKEN2,
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 8/8] drm/i915/gen9: Add WaOCLCoherentLineFlush
On 21/01/2016 14:00, Arun Siluvery wrote: This is mainly required for preemption. Cc: Dave Gordon Signed-off-by: Arun Siluvery Reviewed-by: Nick Hoath --- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index e91fb70..f26f274 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -979,6 +979,10 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring) /* WaDisableSTUnitPowerOptimization:skl,bxt */ WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE); + /* WaOCLCoherentLineFlush:skl,bxt */ + I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) | + GEN8_LQSC_FLUSH_COHERENT_LINES)); + /* WaEnablePreemptionGranularityControlByUMD:skl,bxt */ ret= wa_ring_whitelist_reg(ring, GEN8_CS_CHICKEN1); if (ret) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 4/8] drm/i915/bxt: Add GEN9_CS_DEBUG_MODE1 to HW whitelist
On 21/01/2016 14:00, Arun Siluvery wrote:
Required for,
WaDisableObjectLevelPreemptionForTrifanOrPolygon:bxt
WaDisableObjectLevelPreemptionForInstancedDraw:bxt
WaDisableObjectLevelPreemtionForInstanceId:bxt
According to WA database these are only applicable for BXT:A0 but since
A0 and A1 shares the same GT these are extended for A1 as well.
These are also required for SKL until B0 but not adding them because they
are pre-production steppings.
v2: use lower case in register defines (Nick)
Signed-off-by: Arun Siluvery
Reviewed-by: Nick Hoath
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +
2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ed887cf..c51e7e9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5998,6 +5998,7 @@ enum skl_disp_power_wells {
#define FF_SLICE_CS_CHICKEN2 _MMIO(0x20e4)
#define GEN9_TSG_BARRIER_ACK_DISABLE (1<<8)
+#define GEN9_CS_DEBUG_MODE1_MMIO(0x20ec)
#define GEN8_CS_CHICKEN1 _MMIO(0x2580)
/* GEN7 chicken */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index fea632f..72e89b6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1131,6 +1131,15 @@ static int bxt_init_workarounds(struct intel_engine_cs
*ring)
GEN7_SBE_SS_CACHE_DISPATCH_PORT_SHARING_DISABLE);
}
+ /* WaDisableObjectLevelPreemptionForTrifanOrPolygon:bxt */
+ /* WaDisableObjectLevelPreemptionForInstancedDraw:bxt */
+ /* WaDisableObjectLevelPreemtionForInstanceId:bxt */
+ if (IS_BXT_REVID(dev, 0, BXT_REVID_A1)) {
+ ret = wa_ring_whitelist_reg(ring, GEN9_CS_DEBUG_MODE1);
+ if (ret)
+ return ret;
+ }
+
return 0;
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 6/9] drm/i915: Add sync framework support to execbuff IOCTL
Op 14-01-16 om 13:07 schreef Chris Wilson: > On Thu, Jan 14, 2016 at 11:47:17AM +, John Harrison wrote: >> On 13/01/2016 18:43, Chris Wilson wrote: >>> Use the upper s32 for the output, so again you are not overwriting user >>> state without good reason. >>> >> Makes sense. Will do. > It would also be useful (for nefarious reasons) to only copy back the > rsvd2 field, i.e. keep it as DRM_IOW(EXECBUFFER2) and do the small > fixed-sized copy_to_user of rsvd2 as required. That means we can avoid > the copy mostly, and treat args as a private copy of the user > parameters, i.e. space we can scribble over with impunity. Won't that be an abuse of abi? If we write memory we should mark the struct as writable, and make sure we don't touch the user execbuffer2 at all until the call succeeds. ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915: Implement color management on chv
Signed-off-by: Lionel Landwerlin
---
drivers/gpu/drm/i915/i915_drv.c| 3 +
drivers/gpu/drm/i915/i915_reg.h| 40 +++
drivers/gpu/drm/i915/intel_color.c | 139 -
3 files changed, 180 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1978fe5..1e7a567 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -65,6 +65,8 @@ static struct drm_driver driver;
#define BDW_COLORS \
.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
+#define CHV_COLORS \
+ .color = { .degamma_lut_size = 65, .gamma_lut_size = 257 }
static const struct intel_device_info intel_i830_info = {
.gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2,
@@ -322,6 +324,7 @@ static const struct intel_device_info intel_cherryview_info
= {
.display_mmio_offset = VLV_DISPLAY_BASE,
GEN_CHV_PIPEOFFSETS,
CURSOR_OFFSETS,
+ CHV_COLORS,
};
static const struct intel_device_info intel_skylake_info = {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ccbcf63..1b81b44 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7659,6 +7659,46 @@ enum skl_disp_power_wells {
#define PREC_PAL_GC_MAX(pipe, i) _MMIO(_PIPE3(pipe, _PAL_PREC_GC_MAX_A,
_PAL_PREC_GC_MAX_B, _PAL_PREC_GC_MAX_C) + (i) * 4)
#define PREC_PAL_EXT_GC_MAX(pipe, i) _MMIO(_PIPE3(pipe,
_PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B, _PAL_PREC_EXT_GC_MAX_C) + (i) *
4)
+/* pipe CSC & degamma/gamma LUTs on CHV */
+#define _CGM_PIPE_A_CSC_COEFF01(VLV_DISPLAY_BASE + 0x67900)
+#define _CGM_PIPE_A_CSC_COEFF23(VLV_DISPLAY_BASE + 0x67904)
+#define _CGM_PIPE_A_CSC_COEFF45(VLV_DISPLAY_BASE + 0x67908)
+#define _CGM_PIPE_A_CSC_COEFF67(VLV_DISPLAY_BASE + 0x6790C)
+#define _CGM_PIPE_A_CSC_COEFF8 (VLV_DISPLAY_BASE + 0x67910)
+#define _CGM_PIPE_A_DEGAMMA(VLV_DISPLAY_BASE + 0x66000)
+#define _CGM_PIPE_A_GAMMA (VLV_DISPLAY_BASE + 0x67000)
+#define _CGM_PIPE_A_MODE (VLV_DISPLAY_BASE + 0x67A00)
+#define CGM_PIPE_MODE_GAMMA (1 << 2)
+#define CGM_PIPE_MODE_CSC(1 << 1)
+#define CGM_PIPE_MODE_DEGAMMA(1 << 0)
+
+#define _CGM_PIPE_B_CSC_COEFF01(VLV_DISPLAY_BASE + 0x69900)
+#define _CGM_PIPE_B_CSC_COEFF23(VLV_DISPLAY_BASE + 0x69904)
+#define _CGM_PIPE_B_CSC_COEFF45(VLV_DISPLAY_BASE + 0x69908)
+#define _CGM_PIPE_B_CSC_COEFF67(VLV_DISPLAY_BASE + 0x6990C)
+#define _CGM_PIPE_B_CSC_COEFF8 (VLV_DISPLAY_BASE + 0x69910)
+#define _CGM_PIPE_B_DEGAMMA(VLV_DISPLAY_BASE + 0x68000)
+#define _CGM_PIPE_B_GAMMA (VLV_DISPLAY_BASE + 0x69000)
+#define _CGM_PIPE_B_MODE (VLV_DISPLAY_BASE + 0x69A00)
+
+#define _CGM_PIPE_C_CSC_COEFF01(VLV_DISPLAY_BASE + 0x6B900)
+#define _CGM_PIPE_C_CSC_COEFF23(VLV_DISPLAY_BASE + 0x6B904)
+#define _CGM_PIPE_C_CSC_COEFF45(VLV_DISPLAY_BASE + 0x6B908)
+#define _CGM_PIPE_C_CSC_COEFF67(VLV_DISPLAY_BASE + 0x6B90C)
+#define _CGM_PIPE_C_CSC_COEFF8 (VLV_DISPLAY_BASE + 0x6B910)
+#define _CGM_PIPE_C_DEGAMMA(VLV_DISPLAY_BASE + 0x6A000)
+#define _CGM_PIPE_C_GAMMA (VLV_DISPLAY_BASE + 0x6B000)
+#define _CGM_PIPE_C_MODE (VLV_DISPLAY_BASE + 0x6BA00)
+
+#define CGM_PIPE_CSC_COEFF01(pipe) _MMIO_PIPE3(pipe,
_CGM_PIPE_A_CSC_COEFF01, _CGM_PIPE_B_CSC_COEFF01, _CGM_PIPE_C_CSC_COEFF01)
+#define CGM_PIPE_CSC_COEFF23(pipe) _MMIO_PIPE3(pipe,
_CGM_PIPE_A_CSC_COEFF23, _CGM_PIPE_B_CSC_COEFF23, _CGM_PIPE_C_CSC_COEFF23)
+#define CGM_PIPE_CSC_COEFF45(pipe) _MMIO_PIPE3(pipe,
_CGM_PIPE_A_CSC_COEFF45, _CGM_PIPE_B_CSC_COEFF45, _CGM_PIPE_C_CSC_COEFF45)
+#define CGM_PIPE_CSC_COEFF67(pipe) _MMIO_PIPE3(pipe,
_CGM_PIPE_A_CSC_COEFF67, _CGM_PIPE_B_CSC_COEFF67, _CGM_PIPE_C_CSC_COEFF67)
+#define CGM_PIPE_CSC_COEFF8(pipe) _MMIO_PIPE3(pipe,
_CGM_PIPE_A_CSC_COEFF8, _CGM_PIPE_B_CSC_COEFF8, _CGM_PIPE_C_CSC_COEFF8)
+#define CGM_PIPE_DEGAMMA(pipe, i, w) _MMIO(_PIPE3(pipe, _CGM_PIPE_A_DEGAMMA,
_CGM_PIPE_B_DEGAMMA, _CGM_PIPE_C_DEGAMMA) + (i) * 4 + (w) * 4)
+#define CGM_PIPE_GAMMA(pipe, i, w) _MMIO(_PIPE3(pipe, _CGM_PIPE_A_GAMMA,
_CGM_PIPE_B_GAMMA, _CGM_PIPE_C_GAMMA) + (i) * 4 + (w) * 4)
+#define CGM_PIPE_MODE(pipe)_MMIO_PIPE3(pipe, _CGM_PIPE_A_MODE,
_CGM_PIPE_B_MODE, _CGM_PIPE_C_MODE)
+
/* MIPI DSI registers */
#define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */
diff --git a/drivers/gpu/drm/i915/intel_color.c
b/drivers/gpu/drm/i915/intel_color.c
index bbf84cc..baa7fe9 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -213,6 +213,54 @@ static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
}
}
+/*
+ * Set up the pipe CSC unit on CherryView.
+ */
+static void cherryview_load_csc_matrix(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_
[Intel-gfx] [PATCH 4/6] drm/i915: enable legacy palette for pipe C
Signed-off-by: Lionel Landwerlin
---
drivers/gpu/drm/i915/i915_reg.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 63c4283..46143f8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5753,7 +5753,8 @@ enum skl_disp_power_wells {
/* legacy palette */
#define _LGC_PALETTE_A 0x4a000
#define _LGC_PALETTE_B 0x4a800
-#define LGC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B)
+ (i) * 4)
+#define _LGC_PALETTE_C 0x4b000
+#define LGC_PALETTE(pipe, i) _MMIO(_PIPE3(pipe, _LGC_PALETTE_A,
_LGC_PALETTE_B, _LGC_PALETTE_C) + (i) * 4)
#define _GAMMA_MODE_A 0x4a480
#define _GAMMA_MODE_B 0x4ac80
--
2.6.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] drm/i915: enable CSC for pipe C
Signed-off-by: Lionel Landwerlin
---
drivers/gpu/drm/i915/i915_reg.h | 40 +++-
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0a98889..63c4283 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7608,19 +7608,33 @@ enum skl_disp_power_wells {
#define _PIPE_B_CSC_POSTOFF_ME 0x49144
#define _PIPE_B_CSC_POSTOFF_LO 0x49148
-#define PIPE_CSC_COEFF_RY_GY(pipe) _MMIO_PIPE(pipe,
_PIPE_A_CSC_COEFF_RY_GY, _PIPE_B_CSC_COEFF_RY_GY)
-#define PIPE_CSC_COEFF_BY(pipe)_MMIO_PIPE(pipe,
_PIPE_A_CSC_COEFF_BY, _PIPE_B_CSC_COEFF_BY)
-#define PIPE_CSC_COEFF_RU_GU(pipe) _MMIO_PIPE(pipe,
_PIPE_A_CSC_COEFF_RU_GU, _PIPE_B_CSC_COEFF_RU_GU)
-#define PIPE_CSC_COEFF_BU(pipe)_MMIO_PIPE(pipe,
_PIPE_A_CSC_COEFF_BU, _PIPE_B_CSC_COEFF_BU)
-#define PIPE_CSC_COEFF_RV_GV(pipe) _MMIO_PIPE(pipe,
_PIPE_A_CSC_COEFF_RV_GV, _PIPE_B_CSC_COEFF_RV_GV)
-#define PIPE_CSC_COEFF_BV(pipe)_MMIO_PIPE(pipe,
_PIPE_A_CSC_COEFF_BV, _PIPE_B_CSC_COEFF_BV)
-#define PIPE_CSC_MODE(pipe)_MMIO_PIPE(pipe, _PIPE_A_CSC_MODE,
_PIPE_B_CSC_MODE)
-#define PIPE_CSC_PREOFF_HI(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_PREOFF_HI,
_PIPE_B_CSC_PREOFF_HI)
-#define PIPE_CSC_PREOFF_ME(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_PREOFF_ME,
_PIPE_B_CSC_PREOFF_ME)
-#define PIPE_CSC_PREOFF_LO(pipe) _MMIO_PIPE(pipe, _PIPE_A_CSC_PREOFF_LO,
_PIPE_B_CSC_PREOFF_LO)
-#define PIPE_CSC_POSTOFF_HI(pipe) _MMIO_PIPE(pipe,
_PIPE_A_CSC_POSTOFF_HI, _PIPE_B_CSC_POSTOFF_HI)
-#define PIPE_CSC_POSTOFF_ME(pipe) _MMIO_PIPE(pipe,
_PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME)
-#define PIPE_CSC_POSTOFF_LO(pipe) _MMIO_PIPE(pipe,
_PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
+#define _PIPE_C_CSC_COEFF_RY_GY0x49210
+#define _PIPE_C_CSC_COEFF_BY 0x49214
+#define _PIPE_C_CSC_COEFF_RU_GU0x49218
+#define _PIPE_C_CSC_COEFF_BU 0x4921c
+#define _PIPE_C_CSC_COEFF_RV_GV0x49220
+#define _PIPE_C_CSC_COEFF_BV 0x49224
+#define _PIPE_C_CSC_MODE 0x49228
+#define _PIPE_C_CSC_PREOFF_HI 0x49230
+#define _PIPE_C_CSC_PREOFF_ME 0x49234
+#define _PIPE_C_CSC_PREOFF_LO 0x49238
+#define _PIPE_C_CSC_POSTOFF_HI 0x49240
+#define _PIPE_C_CSC_POSTOFF_ME 0x49244
+#define _PIPE_C_CSC_POSTOFF_LO 0x49248
+
+#define PIPE_CSC_COEFF_RY_GY(pipe) _MMIO_PIPE3(pipe,
_PIPE_A_CSC_COEFF_RY_GY, _PIPE_B_CSC_COEFF_RY_GY, _PIPE_C_CSC_COEFF_RY_GY)
+#define PIPE_CSC_COEFF_BY(pipe)_MMIO_PIPE3(pipe,
_PIPE_A_CSC_COEFF_BY, _PIPE_B_CSC_COEFF_BY, _PIPE_C_CSC_COEFF_BY)
+#define PIPE_CSC_COEFF_RU_GU(pipe) _MMIO_PIPE3(pipe,
_PIPE_A_CSC_COEFF_RU_GU, _PIPE_B_CSC_COEFF_RU_GU, _PIPE_C_CSC_COEFF_RU_GU)
+#define PIPE_CSC_COEFF_BU(pipe)_MMIO_PIPE3(pipe,
_PIPE_A_CSC_COEFF_BU, _PIPE_B_CSC_COEFF_BU, _PIPE_C_CSC_COEFF_BU)
+#define PIPE_CSC_COEFF_RV_GV(pipe) _MMIO_PIPE3(pipe,
_PIPE_A_CSC_COEFF_RV_GV, _PIPE_B_CSC_COEFF_RV_GV, _PIPE_C_CSC_COEFF_RV_GV)
+#define PIPE_CSC_COEFF_BV(pipe)_MMIO_PIPE3(pipe,
_PIPE_A_CSC_COEFF_BV, _PIPE_B_CSC_COEFF_BV, _PIPE_C_CSC_COEFF_BV)
+#define PIPE_CSC_MODE(pipe)_MMIO_PIPE3(pipe, _PIPE_A_CSC_MODE,
_PIPE_B_CSC_MODE, _PIPE_C_CSC_MODE)
+#define PIPE_CSC_PREOFF_HI(pipe) _MMIO_PIPE3(pipe,
_PIPE_A_CSC_PREOFF_HI, _PIPE_B_CSC_PREOFF_HI, _PIPE_C_CSC_PREOFF_HI)
+#define PIPE_CSC_PREOFF_ME(pipe) _MMIO_PIPE3(pipe,
_PIPE_A_CSC_PREOFF_ME, _PIPE_B_CSC_PREOFF_ME, _PIPE_C_CSC_PREOFF_ME)
+#define PIPE_CSC_PREOFF_LO(pipe) _MMIO_PIPE3(pipe,
_PIPE_A_CSC_PREOFF_LO, _PIPE_B_CSC_PREOFF_LO, _PIPE_C_CSC_PREOFF_LO)
+#define PIPE_CSC_POSTOFF_HI(pipe) _MMIO_PIPE3(pipe,
_PIPE_A_CSC_POSTOFF_HI, _PIPE_B_CSC_POSTOFF_HI, _PIPE_C_CSC_POSTOFF_HI)
+#define PIPE_CSC_POSTOFF_ME(pipe) _MMIO_PIPE3(pipe,
_PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME, _PIPE_C_CSC_POSTOFF_ME)
+#define PIPE_CSC_POSTOFF_LO(pipe) _MMIO_PIPE3(pipe,
_PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO, _PIPE_C_CSC_POSTOFF_LO)
/* MIPI DSI registers */
--
2.6.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm/i915: Extract out gamma table and CSC to their own file
Signed-off-by: Lionel Landwerlin
---
drivers/gpu/drm/i915/Makefile| 1 +
drivers/gpu/drm/i915/intel_color.c | 174 +++
drivers/gpu/drm/i915/intel_display.c | 163 +++-
drivers/gpu/drm/i915/intel_drv.h | 10 ++
4 files changed, 197 insertions(+), 151 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_color.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0851de07..0516300 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -55,6 +55,7 @@ i915-y += intel_audio.o \
intel_atomic.o \
intel_atomic_plane.o \
intel_bios.o \
+ intel_color.o \
intel_display.o \
intel_fbc.o \
intel_fifo_underrun.o \
diff --git a/drivers/gpu/drm/i915/intel_color.c
b/drivers/gpu/drm/i915/intel_color.c
new file mode 100644
index 000..39ca215
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -0,0 +1,174 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#include "intel_drv.h"
+
+/*
+ * Set up the pipe CSC unit.
+ *
+ * Currently only full range RGB to limited range RGB conversion
+ * is supported, but eventually this should handle various
+ * RGB<->YCbCr scenarios as well.
+ */
+static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ int pipe = intel_crtc->pipe;
+ uint16_t coeff = 0x7800; /* 1.0 */
+
+ /*
+* TODO: Check what kind of values actually come out of the pipe
+* with these coeff/postoff values and adjust to get the best
+* accuracy. Perhaps we even need to take the bpc value into
+* consideration.
+*/
+
+ if (intel_crtc->config->limited_color_range)
+ coeff = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */
+
+ /*
+* GY/GU and RY/RU should be the other way around according
+* to BSpec, but reality doesn't agree. Just set them up in
+* a way that results in the correct picture.
+*/
+ I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeff << 16);
+ I915_WRITE(PIPE_CSC_COEFF_BY(pipe), 0);
+
+ I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeff);
+ I915_WRITE(PIPE_CSC_COEFF_BU(pipe), 0);
+
+ I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), 0);
+ I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeff << 16);
+
+ I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
+ I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
+ I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0);
+
+ if (INTEL_INFO(dev)->gen > 6) {
+ uint16_t postoff = 0;
+
+ if (intel_crtc->config->limited_color_range)
+ postoff = (16 * (1 << 12) / 255) & 0x1fff;
+
+ I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff);
+ I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff);
+ I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff);
+
+ I915_WRITE(PIPE_CSC_MODE(pipe), 0);
+ } else {
+ uint32_t mode = CSC_MODE_YUV_TO_RGB;
+
+ if (intel_crtc->config->limited_color_range)
+ mode |= CSC_BLACK_SCREEN_OFFSET;
+
+ I915_WRITE(PIPE_CSC_MODE(pipe), mode);
+ }
+}
+
+void intel_color_set_csc(struct drm_crtc *crtc)
+{
+ i9xx_load_csc_matrix(crtc);
+}
+
+/** Loads the palette/gamma unit for the CRTC with the prepared values on */
+static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ enum pipe pipe = intel_crtc->pipe;
+
[Intel-gfx] [PATCH 0/6] Pipe level color management
Hi, This serie introduces pipe level color management through a set of properties attached to the CRTC. It also provides an implementation for some Intel platforms. This serie is based of a previous set of patches by Shashank Sharma and takes into account of the comments by Daniel Stone & Daniel Vetter. Cheers, Lionel Lionel Landwerlin (6): drm/i915: Extract out gamma table and CSC to their own file drm: introduce color correction properties drm/i915: enable CSC for pipe C drm/i915: enable legacy palette for pipe C drm/i915: Implement color management on bdw/skl/bxt/kbl drm/i915: Implement color management on chv Documentation/DocBook/gpu.tmpl | 50 ++- drivers/gpu/drm/drm_atomic.c | 102 ++- drivers/gpu/drm/drm_atomic_helper.c | 10 + drivers/gpu/drm/drm_crtc.c | 35 +++ drivers/gpu/drm/drm_crtc_helper.c| 33 ++ drivers/gpu/drm/i915/Makefile| 1 + drivers/gpu/drm/i915/i915_drv.c | 27 +- drivers/gpu/drm/i915/i915_drv.h | 9 + drivers/gpu/drm/i915/i915_reg.h | 105 ++- drivers/gpu/drm/i915/intel_color.c | 568 +++ drivers/gpu/drm/i915/intel_display.c | 175 ++- drivers/gpu/drm/i915/intel_drv.h | 12 + include/drm/drm_crtc.h | 43 ++- include/drm/drm_crtc_helper.h| 3 + include/uapi/drm/drm_mode.h | 15 + 15 files changed, 1011 insertions(+), 177 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_color.c -- 2.6.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ warning: Fi.CI.BAT
Hi, I got this message in reply to this patch (https://patchwork.freedesktop.org/patch/60736/). It looks like most of the warnings are related to 'PWM1 enabled' warnings that happen when the hardware is going into some power management state and BLM_PWM_ENABLE and/or BLM_PCH_PWM_ENABLE are enabled on the bdw-ultra platform. What is the best way to fix this? If my patch is used then the function that disables BLM_PWM_ENABLE and/or BLM_PCH_PWM_ENABLE (lpt_disable_backlight) will not be called. Should I disable this DPCD backlight control featured for BDW or specifically disable these bits in my intel_dp_aux_enable_backlight function? Thank you. Yetunde Dmesg warn output [ 357.655508] [ cut here ] [ 357.655536] WARNING: CPU: 1 PID: 43 at drivers/gpu/drm/i915/intel_display.c:9518 hsw_enable_pc8+0x609/0x730 [i915]() [ 357.655537] CPU PWM1 enabled [ 357.655539] Modules linked in: i915 ax88179_178a i2c_hid x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul cdc_ncm usbnet mii mei_me mei lpc_ich i2c_designware_platform i2c_designware_core e1000e sdhci_pci ptp pps_core sdhci_acpi sdhci mmc_core [last unloaded: i915] [ 357.69] CPU: 1 PID: 43 Comm: kworker/1:1 Tainted: G U W 4.4.0-gfxbench+ #1 [ 357.655560] Hardware name: Intel Corporation Broadwell Client platform/Wilson Beach SDS, BIOS BDW-E2R1.86C.0095.R09.141036 10/30/2014 [ 357.655565] Workqueue: pm pm_runtime_work [ 357.655567] a03e7ce8 8800ab8e3b68 813df90c 8800ab8e3bb0 [ 357.655570] 8800ab8e3ba0 810746e1 8802308c 880240341898 [ 357.655573] 8802403418a8 880240341148 880243bd3470 8800ab8e3c00 [ 357.655576] Call Trace: [ 357.655580] [] dump_stack+0x4e/0x82 [ 357.655583] [] warn_slowpath_common+0x81/0xc0 [ 357.655585] [] warn_slowpath_fmt+0x47/0x50 [ 357.655603] [] hsw_enable_pc8+0x609/0x730 [i915] [ 357.655610] [] intel_suspend_complete+0xca/0x6c0 [i915] [ 357.655617] [] intel_runtime_suspend+0xdb/0x2d0 [i915] [ 357.655620] [] pci_pm_runtime_suspend+0x56/0x190 [ 357.655623] [] ? pci_pm_runtime_resume+0xa0/0xa0 [ 357.655626] [] __rpm_callback+0x2d/0x70 [ 357.655628] [] ? pci_pm_runtime_resume+0xa0/0xa0 [ 357.655631] [] rpm_callback+0x1f/0x80 [ 357.655633] [] ? pci_pm_runtime_resume+0xa0/0xa0 [ 357.655635] [] rpm_suspend+0x148/0x780 [ 357.655638] [] pm_runtime_work+0x76/0xc0 [ 357.655641] [] process_one_work+0x1e5/0x620 [ 357.655642] [] ? process_one_work+0x149/0x620 [ 357.655645] [] worker_thread+0x49/0x450 [ 357.655646] [] ? process_one_work+0x620/0x620 [ 357.655648] [] ? process_one_work+0x620/0x620 [ 357.655651] [] kthread+0xea/0x100 [ 357.655653] [] ? _raw_spin_unlock_irq+0x27/0x50 [ 357.655656] [] ? kthread_create_on_node+0x1f0/0x1f0 [ 357.655658] [] ret_from_fork+0x3f/0x70 [ 357.655661] [] ? kthread_create_on_node+0x1f0/0x1f0 [ 357.655662] ---[ end trace d4f8f254173751a9 ]--- [ 357.655665] [ cut here ] [ 357.655682] WARNING: CPU: 1 PID: 43 at drivers/gpu/drm/i915/intel_display.c:9523 hsw_enable_pc8+0x701/0x730 [i915]() [ 357.655683] PCH PWM1 enabled [ 357.655684] Modules linked in: i915 ax88179_178a i2c_hid x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul cdc_ncm usbnet mii mei_me mei lpc_ich i2c_designware_platform i2c_designware_core e1000e sdhci_pci ptp pps_core sdhci_acpi sdhci mmc_core [last unloaded: i915] [ 357.655699] CPU: 1 PID: 43 Comm: kworker/1:1 Tainted: G U W 4.4.0-gfxbench+ #1 [ 357.655701] Hardware name: Intel Corporation Broadwell Client platform/Wilson Beach SDS, BIOS BDW-E2R1.86C.0095.R09.141036 10/30/2014 [ 357.655704] Workqueue: pm pm_runtime_work [ 357.655705] a03e7ce8 8800ab8e3b68 813df90c 8800ab8e3bb0 [ 357.655708] 8800ab8e3ba0 810746e1 8802308c 880240341898 [ 357.655711] 8802403418a8 880240341148 880243bd3470 8800ab8e3c00 [ 357.655714] Call Trace: [ 357.655716] [] dump_stack+0x4e/0x82 [ 357.655718] [] warn_slowpath_common+0x81/0xc0 [ 357.655720] [] warn_slowpath_fmt+0x47/0x50 [ 357.655736] [] hsw_enable_pc8+0x701/0x730 [i915] [ 357.655743] [] intel_suspend_complete+0xca/0x6c0 [i915] [ 357.655750] [] intel_runtime_suspend+0xdb/0x2d0 [i915] [ 357.655753] [] pci_pm_runtime_suspend+0x56/0x190 [ 357.655755] [] ? pci_pm_runtime_resume+0xa0/0xa0 [ 357.655758] [] __rpm_callback+0x2d/0x70 [ 357.655760] [] ? pci_pm_runtime_resume+0xa0/0xa0 [ 357.655763] [] rpm_callback+0x1f/0x80 [ 357.655765] [] ? pci_pm_runtime_resume+0xa0/0xa0 [ 357.655767] [] rpm_suspend+0x148/0x780 [ 357.655770] [] pm_runtime_work+0x76/0xc0 [ 357.655772] [] process_one_work+0x1e5/0x620 [ 357.655773] [] ? process_one_work+0x149/0x620 [ 357.655775] [] worker_thread+0x49/0x450 [ 357.655777] [] ? process_one_work+0x620/0x620 [ 357.655779] [] ? process_one_work+0x620/
Re: [Intel-gfx] [RFC 6/9] drm/i915: Add sync framework support to execbuff IOCTL
On Thu, Jan 21, 2016 at 03:47:40PM +0100, Maarten Lankhorst wrote: > Op 14-01-16 om 13:07 schreef Chris Wilson: > > On Thu, Jan 14, 2016 at 11:47:17AM +, John Harrison wrote: > >> On 13/01/2016 18:43, Chris Wilson wrote: > >>> Use the upper s32 for the output, so again you are not overwriting user > >>> state without good reason. > >>> > >> Makes sense. Will do. > > It would also be useful (for nefarious reasons) to only copy back the > > rsvd2 field, i.e. keep it as DRM_IOW(EXECBUFFER2) and do the small > > fixed-sized copy_to_user of rsvd2 as required. That means we can avoid > > the copy mostly, and treat args as a private copy of the user > > parameters, i.e. space we can scribble over with impunity. > Won't that be an abuse of abi? If we write memory we should mark the struct > as writable, and make sure we don't touch the user execbuffer2 at all until > the call succeeds. We only write into the structure when the user requests an output parameter, so they have to opt in (because the current ABI has no outputs through the ioctl atm). Whether or not userspace marks the ioctl argument as RW/RO is irrelevant to the kernel who uses its own table to control the ABI. If the memory really is read-only, then copy_to_user() will -EFAULT (and as the user requested an output we should check early and fail, whether to fail after we dispatch the request is an interesting conundrum). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm: introduce color correction properties
Signed-off-by: Lionel Landwerlin
---
Documentation/DocBook/gpu.tmpl | 48 +
drivers/gpu/drm/drm_atomic.c| 102 +++-
drivers/gpu/drm/drm_atomic_helper.c | 10
drivers/gpu/drm/drm_crtc.c | 35 +
drivers/gpu/drm/drm_crtc_helper.c | 33
include/drm/drm_crtc.h | 43 ++-
include/drm/drm_crtc_helper.h | 3 ++
include/uapi/drm/drm_mode.h | 15 ++
8 files changed, 286 insertions(+), 3 deletions(-)
diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 351e801..d2f682c 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -2092,6 +2092,54 @@ void intel_crt_init(struct drm_device *dev)
TBD
+ “DEGAMMA_LUT”
+ BLOB
+
+ CRTC
+ DRM property to set the degamma LUT mapping
+ pixel data from the framebuffer before it is given to the
+ transformation matrix. The data is an interpreted as an array
+ of struct drm_color_lut elements.
+
+
+ “DEGAMMA_LUT_SIZE”
+ IMMUTABLE
+
+ CRTC
+ DRM property to gives the size of the LUT to
+ be set on the DEGAMMA_LUT property (the size depends on the
+ underlying hardware).
+
+
+ “CTM_MATRIX”
+ BLOB
+
+ CRTC
+ DRM property to set the transformation
+ matrix apply to pixel data after the lookup through the
+ degamma LUT and before the lookup through the gamma LUT. The
+ data is an interpreted as a struct drm_color_ctm.
+
+
+ “GAMMA_LUT”
+ BLOB
+
+ CRTC
+ DRM property to set the gamma LUT mapping
+ pixel data after to the transformation matrix to data sent to
+ the connector. The data is an interpreted as an array
+ of struct drm_color_lut elements.
+
+
+ “GAMMA_LUT_SIZE”
+ IMMUTABLE
+
+ CRTC
+ DRM property to gives the size of the LUT to
+ be set on the GAMMA_LUT property (the size depends on the
+ underlying hardware).
+
+
i915
Generic
"Broadcast RGB"
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3f74193..5287416 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -28,6 +28,7 @@
#include
#include
+#include
#include
/**
@@ -388,6 +389,58 @@ int drm_atomic_set_mode_prop_for_crtc(struct
drm_crtc_state *state,
EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
/**
+ * drm_atomic_replace_property_blob - replace a blob property
+ * @blob: a pointer to the member blob to be replaced
+ * @new_blob: the new blob to replace with
+ * @expected_size: the expected size of the new blob
+ * @replaced: whether the blob has been replaced
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+static int
+drm_atomic_replace_property_blob(struct drm_property_blob **blob,
+struct drm_property_blob *new_blob,
+size_t expected_size,
+bool *replaced)
+{
+ struct drm_property_blob *old_blob = *blob;
+
+ if (old_blob == new_blob)
+ return 0;
+
+ if (new_blob != NULL && new_blob->length != expected_size)
+ return -EINVAL;
+
+ if (old_blob != NULL)
+ drm_property_unreference_blob(old_blob);
+ *blob = new_blob;
+ *replaced = true;
+
+ return 0;
+}
+
+static int
+drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
+struct drm_property_blob **blob,
+uint64_t blob_id,
+size_t expected_size,
+bool *replaced)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_property_blob *new_blob = NULL;
+
+ if (blob_id != 0) {
+ new_blob = drm_property_lookup_blob(dev, blob_id);
+ if (new_blob == NULL)
+ return -EINVAL;
+ }
+
+ return drm_atomic_replace_property_blob(blob, new_blob,
+ expected_size, replaced);
+}
+
+/**
* drm_atomic_crtc_set_property - set property on CRTC
* @crtc: the drm CRTC to set a property on
* @state: the state object to update with the new property value
@@ -419,8 +472,47 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
drm_property_unreference_blob(mode);
return ret;
- }
- else if (crtc->funcs->atomic_set_property)
+ } else if (property == config->degamma_lut_prop
[Intel-gfx] [PATCH 5/6] drm/i915: Implement color management on bdw/skl/bxt/kbl
Signed-off-by: Lionel Landwerlin
---
Documentation/DocBook/gpu.tmpl | 12 +-
drivers/gpu/drm/i915/i915_drv.c | 24 ++-
drivers/gpu/drm/i915/i915_drv.h | 9 +
drivers/gpu/drm/i915/i915_reg.h | 22 +++
drivers/gpu/drm/i915/intel_color.c | 345 ++-
drivers/gpu/drm/i915/intel_display.c | 22 ++-
drivers/gpu/drm/i915/intel_drv.h | 4 +-
7 files changed, 375 insertions(+), 63 deletions(-)
diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index d2f682c..e436376 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -2068,7 +2068,7 @@ void intel_crt_init(struct drm_device *dev)
property to suggest an Y offset for a connector
- Optional
+ Optional
“scaling mode”
ENUM
{ "None", "Full", "Center", "Full aspect" }
@@ -2094,7 +2094,7 @@ void intel_crt_init(struct drm_device *dev)
“DEGAMMA_LUT”
BLOB
-
+ 0
CRTC
DRM property to set the degamma LUT mapping
pixel data from the framebuffer before it is given to the
@@ -2104,7 +2104,7 @@ void intel_crt_init(struct drm_device *dev)
“DEGAMMA_LUT_SIZE”
IMMUTABLE
-
+ 0
CRTC
DRM property to gives the size of the LUT to
be set on the DEGAMMA_LUT property (the size depends on the
@@ -2113,7 +2113,7 @@ void intel_crt_init(struct drm_device *dev)
“CTM_MATRIX”
BLOB
-
+ 0
CRTC
DRM property to set the transformation
matrix apply to pixel data after the lookup through the
@@ -2123,7 +2123,7 @@ void intel_crt_init(struct drm_device *dev)
“GAMMA_LUT”
BLOB
-
+ 0
CRTC
DRM property to set the gamma LUT mapping
pixel data after to the transformation matrix to data sent to
@@ -2133,7 +2133,7 @@ void intel_crt_init(struct drm_device *dev)
“GAMMA_LUT_SIZE”
IMMUTABLE
-
+ 0
CRTC
DRM property to gives the size of the LUT to
be set on the GAMMA_LUT property (the size depends on the
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 706b8ea..1978fe5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -63,6 +63,9 @@ static struct drm_driver driver;
#define IVB_CURSOR_OFFSETS \
.cursor_offsets = { CURSOR_A_OFFSET, IVB_CURSOR_B_OFFSET,
IVB_CURSOR_C_OFFSET }
+#define BDW_COLORS \
+ .color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
+
static const struct intel_device_info intel_i830_info = {
.gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2,
.has_overlay = 1, .overlay_needs_physical = 1,
@@ -285,24 +288,28 @@ static const struct intel_device_info
intel_haswell_m_info = {
.is_mobile = 1,
};
+#define BDW_FEATURES \
+ HSW_FEATURES, \
+ BDW_COLORS
+
static const struct intel_device_info intel_broadwell_d_info = {
- HSW_FEATURES,
+ BDW_FEATURES,
.gen = 8,
};
static const struct intel_device_info intel_broadwell_m_info = {
- HSW_FEATURES,
+ BDW_FEATURES,
.gen = 8, .is_mobile = 1,
};
static const struct intel_device_info intel_broadwell_gt3d_info = {
- HSW_FEATURES,
+ BDW_FEATURES,
.gen = 8,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
};
static const struct intel_device_info intel_broadwell_gt3m_info = {
- HSW_FEATURES,
+ BDW_FEATURES,
.gen = 8, .is_mobile = 1,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
};
@@ -318,13 +325,13 @@ static const struct intel_device_info
intel_cherryview_info = {
};
static const struct intel_device_info intel_skylake_info = {
- HSW_FEATURES,
+ BDW_FEATURES,
.is_skylake = 1,
.gen = 9,
};
static const struct intel_device_info intel_skylake_gt3_info = {
- HSW_FEATURES,
+ BDW_FEATURES,
.is_skylake = 1,
.gen = 9,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
@@ -342,17 +349,18 @@ static const struct intel_device_info intel_broxton_info
= {
.has_fbc = 1,
GEN_DEFAULT_PIPEOFFSETS,
IVB_CURSOR_OFFSETS,
+ BDW_COLORS,
};
static const struct intel_device_info intel_kabylake_info = {
- HSW_FEATURES,
+ BDW_FEATURES,
.is_preliminary = 1,
.is_kabylake = 1,
.gen = 9,
};
static const struct intel_device_info intel_kabylake_gt3_info = {
- HSW_FEATURES,
+ BDW_FEATURES,
.is_preliminary = 1,
.is_kabylake = 1,
.gen = 9,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 204661f..4a38b15 100644
--- a/drivers/gpu/drm/i915/i915_
[Intel-gfx] [PATCH i-g-t 1/4] lib: kms: add crtc_id to igt_pipe_t
Signed-off-by: Lionel Landwerlin
---
lib/igt_kms.c | 1 +
lib/igt_kms.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 497118a..f5eef82 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1004,6 +1004,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
int p = IGT_PLANE_2;
int j, type;
+ pipe->crtc_id = resources->crtcs[i];
pipe->display = display;
pipe->pipe = i;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 94f315f..c12c2e1 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -212,6 +212,7 @@ struct igt_pipe {
uint64_t background; /* Background color MSB BGR 16bpc LSB */
uint32_t background_changed : 1;
uint32_t background_property;
+ uint32_t crtc_id;
};
typedef struct {
--
2.6.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/4] lib: kms: add helpers for color management properties on pipes
Signed-off-by: Lionel Landwerlin
---
lib/igt_kms.c | 74 +++
lib/igt_kms.h | 17 +-
2 files changed, 90 insertions(+), 1 deletion(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index f5eef82..984d2ea 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1136,6 +1136,21 @@ void igt_display_init(igt_display_t *display, int drm_fd)
&prop_value,
NULL);
pipe->background = (uint32_t)prop_value;
+ get_crtc_property(display->drm_fd,
output->config.crtc->crtc_id,
+ "DEGAMMA_LUT",
+ &pipe->degamma_property,
+ NULL,
+ NULL);
+ get_crtc_property(display->drm_fd,
output->config.crtc->crtc_id,
+ "CTM_MATRIX",
+ &pipe->ctm_property,
+ NULL,
+ NULL);
+ get_crtc_property(display->drm_fd,
output->config.crtc->crtc_id,
+ "GAMMA_LUT",
+ &pipe->gamma_property,
+ NULL,
+ NULL);
}
}
}
@@ -1285,6 +1300,16 @@ static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe,
enum igt_plane plane)
return &pipe->planes[idx];
}
+bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
+ uint32_t *prop_id, uint64_t *value,
+ drmModePropertyPtr *prop)
+{
+ return get_crtc_property(pipe->display->drm_fd,
+pipe->crtc_id,
+name,
+prop_id, value, prop);
+}
+
static uint32_t igt_plane_get_fb_id(igt_plane_t *plane)
{
if (plane->fb)
@@ -1592,6 +1617,17 @@ static int igt_output_commit(igt_output_t *output,
pipe->background_changed = false;
}
+ if (pipe->color_mgmt_changed) {
+ igt_crtc_set_property(output, pipe->degamma_property,
+ pipe->degamma_blob);
+ igt_crtc_set_property(output, pipe->ctm_property,
+ pipe->ctm_blob);
+ igt_crtc_set_property(output, pipe->gamma_property,
+ pipe->gamma_blob);
+
+ pipe->color_mgmt_changed = false;
+ }
+
for (i = 0; i < pipe->n_planes; i++) {
igt_plane_t *plane = &pipe->planes[i];
@@ -1924,6 +1960,44 @@ void igt_plane_set_rotation(igt_plane_t *plane,
igt_rotation_t rotation)
plane->rotation_changed = true;
}
+static void
+igt_pipe_replace_blob(igt_pipe_t *pipe, uint64_t *blob, void *ptr, size_t
length)
+{
+ igt_display_t *display = pipe->display;
+ uint32_t blob_id = 0;
+
+ if (*blob != 0)
+ igt_assert(drmModeDestroyPropertyBlob(display->drm_fd,
+ *blob) == 0);
+
+ if (length > 0)
+ igt_assert(drmModeCreatePropertyBlob(display->drm_fd,
+ptr, length, &blob_id) ==
0);
+
+ *blob = blob_id;
+}
+
+void
+igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
+{
+ igt_pipe_replace_blob(pipe, &pipe->degamma_blob, ptr, length);
+ pipe->color_mgmt_changed = 1;
+}
+
+void
+igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length)
+{
+ igt_pipe_replace_blob(pipe, &pipe->ctm_blob, ptr, length);
+ pipe->color_mgmt_changed = 1;
+}
+
+void
+igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
+{
+ igt_pipe_replace_blob(pipe, &pipe->gamma_blob, ptr, length);
+ pipe->color_mgmt_changed = 1;
+}
+
/**
* igt_crtc_set_background:
* @pipe: pipe pointer to which background color to be set
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index c12c2e1..dc87981 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -212,6 +212,15 @@ struct igt_pipe {
uint64_t background; /* Background color MSB BGR 16bpc LSB */
uint32_t background_changed : 1;
uint32_t background_property;
+
+ uint64_t degamma_blob;
+ uint32_t degamma_property;
+ uint64_t ctm_blob;
+ uint32_t ctm_property;
+ uint64_t gamma_blob;
+ uint32_t gamma_property;
+ uint32_t color_mgmt_changed : 1;
+
uint32_t crtc_id;
};
@@ -250,12 +259,19 @@ drmModeModeInfo *igt_output_get_mode(
[Intel-gfx] [PATCH i-g-t 3/4] lib: fb: add igt_paint_color_gradient_range
This is a helper to draw a gradient between 2 colors.
Signed-off-by: Lionel Landwerlin
---
lib/igt_fb.c | 34 ++
lib/igt_fb.h | 3 +++
2 files changed, 37 insertions(+)
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index c985824..2d5cf7a 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -200,6 +200,40 @@ igt_paint_color_gradient(cairo_t *cr, int x, int y, int w,
int h,
cairo_pattern_destroy(pat);
}
+/**
+ * igt_paint_color_gradient_range:
+ * @cr: cairo drawing context
+ * @x: pixel x-coordination of the fill rectangle
+ * @y: pixel y-coordination of the fill rectangle
+ * @w: width of the fill rectangle
+ * @h: height of the fill rectangle
+ * @sr: red value to use as start gradient color
+ * @sg: green value to use as start gradient color
+ * @sb: blue value to use as start gradient color
+ * @er: red value to use as end gradient color
+ * @eg: green value to use as end gradient color
+ * @eb: blue value to use as end gradient color
+ *
+ * This functions draws a gradient into the rectangle which fades in
+ * from one color to the other using the drawing context @cr.
+ */
+void
+igt_paint_color_gradient_range(cairo_t *cr, int x, int y, int w, int h,
+ double sr, double sg, double sb,
+ double er, double eg, double eb)
+{
+ cairo_pattern_t *pat;
+
+ pat = cairo_pattern_create_linear(x, y, x + w, y + h);
+ cairo_pattern_add_color_stop_rgba(pat, 1, sr, sg, sb, 1);
+ cairo_pattern_add_color_stop_rgba(pat, 0, er, eg, eb, 1);
+
+ cairo_rectangle(cr, x, y, w, h);
+ cairo_set_source(cr, pat);
+ cairo_fill(cr);
+ cairo_pattern_destroy(pat);
+}
+
static void
paint_test_patterns(cairo_t *cr, int width, int height)
{
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index 5cc8644..ee52c73 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -103,6 +103,9 @@ void igt_paint_color_alpha(cairo_t *cr, int x, int y, int
w, int h,
double r, double g, double b, double a);
void igt_paint_color_gradient(cairo_t *cr, int x, int y, int w, int h,
int r, int g, int b);
+void igt_paint_color_gradient_range(cairo_t *cr, int x, int y, int w, int h,
+ double sr, double sg, double sb,
+ double er, double eg, double eb);
void igt_paint_test_pattern(cairo_t *cr, int width, int height);
void igt_paint_image(cairo_t *cr, const char *filename,
int dst_x, int dst_y, int dst_width, int dst_height);
--
2.6.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 4/4] tests/kms_color: New test for pipe level color management
This test enables testing of :
* degamma LUTs
* csc matrix
* gamma LUTs
* legacy gamma LUTs
Signed-off-by: Lionel Landwerlin
---
tests/Makefile.sources | 1 +
tests/kms_pipe_color.c | 986 +
2 files changed, 987 insertions(+)
create mode 100644 tests/kms_pipe_color.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 3b2ae99..70d309e 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -81,6 +81,7 @@ TESTS_progs_M = \
kms_legacy_colorkey \
kms_mmio_vs_cs_flip \
kms_pipe_b_c_ivb \
+ kms_pipe_color \
kms_pipe_crc_basic \
kms_plane \
kms_psr_sink_crc \
diff --git a/tests/kms_pipe_color.c b/tests/kms_pipe_color.c
new file mode 100644
index 000..e540221
--- /dev/null
+++ b/tests/kms_pipe_color.c
@@ -0,0 +1,986 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include
+#include
+
+#include "drm.h"
+#include "drmtest.h"
+#include "igt_core.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+
+
+IGT_TEST_DESCRIPTION("Test Color Features at Pipe level");
+
+/* Data structures for gamma/degamma ramps & ctm matrix. */
+struct _drm_color_ctm {
+ /* Transformation matrix in S31.32 format. */
+ __s64 matrix[9];
+};
+
+struct _drm_color_lut {
+ /*
+* Data is U0.16 fixed point format.
+*/
+ __u16 red;
+ __u16 green;
+ __u16 blue;
+ __u16 reserved;
+};
+
+/* Internal */
+typedef struct {
+ float r, g, b;
+} color_t;
+
+typedef struct {
+ int drm_fd;
+ igt_display_t display;
+ igt_pipe_crc_t *pipe_crc;
+
+ uint32_t color_depth;
+ uint64_t degamma_lut_size;
+ uint64_t gamma_lut_size;
+} data_t;
+
+
+static void paint_gradient_rectangles(data_t *data,
+ drmModeModeInfo *mode,
+ color_t *colors,
+ struct igt_fb *fb)
+{
+ cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb);
+ int i, l = mode->hdisplay / 3;
+
+ /* Paint 3 gradient rectangles with red/green/blue between 1.0 and
+* 0.5. We want to avoid 0 so each max LUTs only affect their own
+* rectangle.
+*/
+ for (i = 0 ; i < 3; i++) {
+ igt_paint_color_gradient_range(cr, i * l, 0, l, mode->vdisplay,
+ colors[i].r != 0 ? 0.1 : 0,
+ colors[i].g != 0 ? 0.1 : 0,
+ colors[i].b != 0 ? 0.1 : 0,
+ colors[i].r * 255,
+ colors[i].g * 255,
+ colors[i].b * 255);
+ }
+
+ cairo_destroy(cr);
+}
+
+static void paint_rectangles(data_t *data,
+drmModeModeInfo *mode,
+color_t *colors,
+struct igt_fb *fb)
+{
+ cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb);
+ int i, l = mode->hdisplay / 3;
+
+ /* Paint 3 solid rectangles. */
+ for (i = 0 ; i < 3; i++) {
+ igt_paint_color(cr, i * l, 0, l, mode->vdisplay,
+ colors[i].r, colors[i].g, colors[i].b);
+ }
+
+ cairo_destroy(cr);
+}
+
+static float *generate_table(uint32_t lut_size, float exp)
+{
+ float *coeffs = malloc(sizeof(float) * lut_size);
+ uint32_t i;
+
+ for (i = 0; i < lut_size; i++)
+ coeffs[i] = powf((float) i * 1.0 / (float) (lut_size - 1), exp);
+
+ return coeffs;
+}
+
+static float *generate_table_max(uint32_t lut_size)
+{
+ float *coeffs = malloc(sizeof(float) * lut_size);
+ uint32_t i;
+
+ coeffs[
[Intel-gfx] [PATCH i-g-t 0/4] New pipe level color management tests
Hi, This series enables testing pipe level color management using kernel patches from this serie : http://lists.freedesktop.org/archives/intel-gfx/2016-January/085922.html Most of the tests use pipe CRCs to check the results by comparing the output with the expected output drawn using cairo. Cheers, Lionel Lionel Landwerlin (4): lib: kms: add crtc_id to igt_pipe_t lib: kms: add helpers for color management properties on pipes lib: fb: add igt_paint_color_gradient_range tests/kms_color: New test for pipe level color management lib/igt_fb.c | 34 ++ lib/igt_fb.h | 3 + lib/igt_kms.c | 75 lib/igt_kms.h | 18 +- tests/Makefile.sources | 1 + tests/kms_pipe_color.c | 986 + 6 files changed, 1116 insertions(+), 1 deletion(-) create mode 100644 tests/kms_pipe_color.c -- 2.6.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] drm/i915: Extract out gamma table and CSC to their own file
Hi Lionel,
On 21 January 2016 at 15:03, Lionel Landwerlin
wrote:
> + /* Workaround : Do not read or write the pipe palette/gamma data while
> +* GAMMA_MODE is configured for split gamma and IPS_CTL has IPS
> enabled.
> +*/
> + if (IS_HASWELL(dev) && intel_crtc->config->ips_enabled &&
> + ((I915_READ(GAMMA_MODE(pipe)) & GAMMA_MODE_MODE_MASK) ==
> +GAMMA_MODE_MODE_SPLIT)) {
> + hsw_disable_ips(intel_crtc);
> + reenable_ips = true;
> + }
Obviously this one isn't your fault, but this should be inspecting
software state rather than hardware register state.
Cheers,
Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/8] Gen9 HW whitelist and Preemption WA patches
On Thu, Jan 21, 2016 at 02:00:39PM +, Arun Siluvery wrote: > Resending all patches as told by Daniel because I didn't use correct > message-id while replying updated version of Patch1 before which means > patchwork won't pickup and we won't have CI results. Previous updates > regarding Patch1 are available at > https://patchwork.freedesktop.org/patch/70527/ > > Some of the WA patches are now reviewed so I added r-b tags for them. > > Reviewed: 1, 2, 3, 5, 6 > Updated: 4 (to address review comment) > To be reviewed: 4, 7, 8 > > Arun Siluvery (8): > drm/i915/gen9: Add framework to whitelist specific GPU registers > drm/i915/gen9: Add GEN8_CS_CHICKEN1 to HW whitelist > drm/i915/gen9: Add HDC_CHICKEN1 to HW whitelist > drm/i915/bxt: Add GEN9_CS_DEBUG_MODE1 to HW whitelist > drm/i915/bxt: Add GEN8_L3SQCREG4 to HW whitelist > drm/i915/skl: Add GEN8_L3SQCREG4 to HW whitelist > drm/i915/skl: Enable Per context Preemption granularity control > drm/i915/gen9: Add WaOCLCoherentLineFlush With bland changelogs, it is impossible to assess the severity of the issues being addressed. How many of these do we need to apply to kernel release to address potential user impact? Which do backporters need? Are they critical stability fixes or power tuning or enabling for future features? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for Pipe level color management
== Summary == Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest Test drv_module_reload_basic: pass -> DMESG-WARN (bsw-nuc-2) pass -> DMESG-WARN (skl-i5k-2) pass -> DMESG-WARN (hsw-gt2) pass -> DMESG-WARN (bdw-ultra) pass -> DMESG-WARN (ivb-t430s) pass -> DMESG-WARN (byt-nuc) pass -> DMESG-WARN (snb-x220t) pass -> DMESG-WARN (snb-dellxps) pass -> DMESG-WARN (hsw-brixbox) pass -> DMESG-WARN (bdw-nuci7) bdw-nuci7total:140 pass:130 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:143 pass:136 dwarn:1 dfail:0 fail:0 skip:6 bsw-nuc-2total:143 pass:118 dwarn:1 dfail:0 fail:0 skip:24 byt-nuc total:143 pass:127 dwarn:1 dfail:0 fail:0 skip:15 hsw-brixbox total:143 pass:135 dwarn:1 dfail:0 fail:0 skip:7 hsw-gt2 total:143 pass:138 dwarn:1 dfail:0 fail:0 skip:4 ilk-hp8440p total:143 pass:104 dwarn:1 dfail:0 fail:0 skip:38 ivb-t430stotal:143 pass:136 dwarn:1 dfail:0 fail:0 skip:6 skl-i5k-2total:143 pass:133 dwarn:2 dfail:0 fail:0 skip:8 snb-dellxps total:143 pass:128 dwarn:1 dfail:0 fail:0 skip:14 snb-x220ttotal:143 pass:128 dwarn:1 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1239/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/8] Gen9 HW whitelist and Preemption WA patches
On 21/01/2016 15:17, Chris Wilson wrote: On Thu, Jan 21, 2016 at 02:00:39PM +, Arun Siluvery wrote: Resending all patches as told by Daniel because I didn't use correct message-id while replying updated version of Patch1 before which means patchwork won't pickup and we won't have CI results. Previous updates regarding Patch1 are available at https://patchwork.freedesktop.org/patch/70527/ Some of the WA patches are now reviewed so I added r-b tags for them. Reviewed: 1, 2, 3, 5, 6 Updated: 4 (to address review comment) To be reviewed: 4, 7, 8 Arun Siluvery (8): drm/i915/gen9: Add framework to whitelist specific GPU registers drm/i915/gen9: Add GEN8_CS_CHICKEN1 to HW whitelist drm/i915/gen9: Add HDC_CHICKEN1 to HW whitelist drm/i915/bxt: Add GEN9_CS_DEBUG_MODE1 to HW whitelist drm/i915/bxt: Add GEN8_L3SQCREG4 to HW whitelist drm/i915/skl: Add GEN8_L3SQCREG4 to HW whitelist drm/i915/skl: Enable Per context Preemption granularity control drm/i915/gen9: Add WaOCLCoherentLineFlush With bland changelogs, it is impossible to assess the severity of the issues being addressed. How many of these do we need to apply to kernel release to address potential user impact? Which do backporters need? Are they critical stability fixes or power tuning or enabling for future features? -Chris All of these changes are required for Preemption so they affect Gen9 only. The whitelist changes are really to support for preemption WA that are going to be part of UMD. The WA data is sketchy and for some of them hsd links are not available so I don't really know the details. regards Arun ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 017/190] drm/i915: Remove forcewake dance from seqno/irq barrier on legacy gen6+
Dave Gordon writes:
> On 11/01/16 09:16, Chris Wilson wrote:
>> In order to ensure seqno/irq coherency, we current read a ring register.
>> We are not sure quite how it works, only that is does. Experiments show
>> that e.g. doing a clflush(seqno) instead is not sufficient, but we can
>> remove the forcewake dance from the mmio access.
>>
>> v2: Baytrail wants a clflush too.
>>
>> Signed-off-by: Chris Wilson
>> Cc: Daniel Vetter
>> ---
>> drivers/gpu/drm/i915/intel_ringbuffer.c | 15 +--
>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> index 99780b674311..a1d43b2c7077 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> @@ -1490,10 +1490,21 @@ gen6_ring_get_seqno(struct intel_engine_cs *ring,
>> bool lazy_coherency)
>> {
>> /* Workaround to force correct ordering between irq and seqno writes on
>> * ivb (and maybe also on snb) by reading from a CS register (like
>> - * ACTHD) before reading the status page. */
>> + * ACTHD) before reading the status page.
>> + *
>> + * Note that this effectively effectively stalls the read by the time
>> + * it takes to do a memory transaction, which more or less ensures
>> + * that the write from the GPU has sufficient time to invalidate
>> + * the CPU cacheline. Alternatively we could delay the interrupt from
>> + * the CS ring to give the write time to land, but that would incur
>> + * a delay after every batch i.e. much more frequent than a delay
>> + * when waiting for the interrupt (with the same net latency).
>> + */
>> if (!lazy_coherency) {
>> struct drm_i915_private *dev_priv = ring->dev->dev_private;
>> -POSTING_READ(RING_ACTHD(ring->mmio_base));
>> +POSTING_READ_FW(RING_ACTHD(ring->mmio_base));
>> +
>> +intel_flush_status_page(ring, I915_GEM_HWS_INDEX);
>> }
>>
>> return intel_read_status_page(ring, I915_GEM_HWS_INDEX);
>
> Well, I generally like this, but my previous questions of 2015-01-05
> were not answered:
>
>> Hmm ... would putting the flush /before/ the POSTING_READ be better?
>>
>> Depending on how the h/w implements the cacheline invalidation, it
> > might allow some overlap between the cache controller's internal
> > activities and the MMIO cycle ...
I thought of the sequence of events like this:
#1: read(acthd9) -> gpu flushes the write (if cpu snoop fails,
we still have the correct on in dram)
#2: flush_status_page() -> is actually going to be invalidate for cpu as
it is only written from cpu side on init. (explained in bxt_a_get_seqno)
#3: read status page will get coherent value from dram.
>>
>> Also, previously we only had the flush on BXT, whereas now you're
> > doing it on all gen6+. I think this is probably a good thing, but just
> > wondered whether there's any downside to it?
Perf hit. But when seqno coherence is essential, we need
to be absolutely sure due to way we handle irqs. The only
place where we force coherence is from __i915_wait_request,
and there only before final decision before going to sleep.
(well hangcheck also does the coherent read but that has no
relevance on perf).
Our irq handling is built on a principle that if
we enable interrupts with waiters, absolutely nothing can get lost.
There is no leeway after irq_get().
If we are going to put task to sleep, we can afford a cacheline
flush.
So guarantee a coherent read asked to be coherent. And relax
the rules per gen basis, if someone can show passed tests with
improved perf metrics.
>>
>> Also ... are we sure that no-one calls this without having a
>> forcewake in effect at the time, in particular debugfs? Or is it not
> > going to end up going through here once lazy_coherency is abolished?
>
I asked about this from Chris in irc. The actual forcewake
status doesn't matter. If it was not on, we just get zero
but the sideffect still happens: seqno is written.
Thanks,
-Mika
> .Dave.
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/25] drm/i915/fbc: wait for a vblank instead of 50ms when enabling
Em Qui, 2016-01-21 às 15:17 +0100, Maarten Lankhorst escreveu:
> Op 19-01-16 om 14:35 schreef Paulo Zanoni:
> > Instead of waiting for 50ms, just wait until the next vblank, since
> > it's the minimum requirement. The whole infrastructure of FBC is
> > based
> > on vblanks, so waiting for X vblanks instead of X milliseconds
> > sounds
> > like the correct way to go. Besides, 50ms may be less than a vblank
> > on
> > super slow modes that may or may not exist.
> >
> > There are some small improvements in PC state residency (due to the
> > fact that we're now using 16ms for the common modes instead of
> > 50ms),
> > but the biggest advantage is still the correctness of being
> > vblank-based instead of time-based.
> >
> > v2:
> > - Rebase after changing the patch order.
> > - Update the commit message.
> > v3:
> > - Fix bogus vblank_get() instead of vblank_count() (Ville).
> > - Don't forget to call drm_crtc_vblank_{get,put} (Chris, Ville)
> > - Adjust the performance details on the commit message.
> >
> > Signed-off-by: Paulo Zanoni
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> > drivers/gpu/drm/i915/intel_fbc.c | 43
> >
> > 2 files changed, 31 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index af30148..33217a4 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -925,9 +925,9 @@ struct i915_fbc {
> >
> > struct intel_fbc_work {
> > bool scheduled;
> > + u32 scheduled_vblank;
> > struct work_struct work;
> > struct drm_framebuffer *fb;
> > - unsigned long enable_jiffies;
> > } work;
> >
> > const char *no_fbc_reason;
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> > b/drivers/gpu/drm/i915/intel_fbc.c
> > index a1988a4..6b43ec3 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -381,7 +381,15 @@ static void intel_fbc_work_fn(struct
> > work_struct *__work)
> > container_of(__work, struct drm_i915_private,
> > fbc.work.work);
> > struct intel_fbc_work *work = &dev_priv->fbc.work;
> > struct intel_crtc *crtc = dev_priv->fbc.crtc;
> > - int delay_ms = 50;
> > + struct drm_vblank_crtc *vblank = &dev_priv->dev-
> > >vblank[crtc->pipe];
> > +
> > + mutex_lock(&dev_priv->fbc.lock);
> > + if (drm_crtc_vblank_get(&crtc->base)) {
> > + DRM_ERROR("vblank not available for FBC on pipe
> > %c\n",
> > + pipe_name(crtc->pipe));
> > + goto out;
> > + }
> > + mutex_unlock(&dev_priv->fbc.lock);
> What does the lock protect here?
Hmmm, yeah, it protects nothing... Well observed.
>
> ~Maarten
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/8] Gen9 HW whitelist and Preemption WA patches
On Thu, Jan 21, 2016 at 04:07:22PM +, Arun Siluvery wrote: > On 21/01/2016 15:17, Chris Wilson wrote: > >On Thu, Jan 21, 2016 at 02:00:39PM +, Arun Siluvery wrote: > >>Resending all patches as told by Daniel because I didn't use correct > >>message-id while replying updated version of Patch1 before which means > >>patchwork won't pickup and we won't have CI results. Previous updates > >>regarding Patch1 are available at > >>https://patchwork.freedesktop.org/patch/70527/ > >> > >>Some of the WA patches are now reviewed so I added r-b tags for them. > >> > >>Reviewed: 1, 2, 3, 5, 6 > >>Updated: 4 (to address review comment) > >>To be reviewed: 4, 7, 8 > >> > >>Arun Siluvery (8): > >> drm/i915/gen9: Add framework to whitelist specific GPU registers > >> drm/i915/gen9: Add GEN8_CS_CHICKEN1 to HW whitelist > >> drm/i915/gen9: Add HDC_CHICKEN1 to HW whitelist > >> drm/i915/bxt: Add GEN9_CS_DEBUG_MODE1 to HW whitelist > >> drm/i915/bxt: Add GEN8_L3SQCREG4 to HW whitelist > >> drm/i915/skl: Add GEN8_L3SQCREG4 to HW whitelist > >> drm/i915/skl: Enable Per context Preemption granularity control > >> drm/i915/gen9: Add WaOCLCoherentLineFlush > > > >With bland changelogs, it is impossible to assess the severity of the > >issues being addressed. How many of these do we need to apply to kernel > >release to address potential user impact? Which do backporters need? > >Are they critical stability fixes or power tuning or enabling for future > >features? > >-Chris > > > All of these changes are required for Preemption so they affect Gen9 > only. The whitelist changes are really to support for preemption WA > that are going to be part of UMD. The WA data is sketchy and for > some of them hsd links are not available so I don't really know the > details. Are you happy with adding "Required for future enabling of pre-emptive command execution" to each? It just helps clarify the purpose of the patch, and as I said helps answer the question of "do I need to backport this to stable kernels". -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Improve handling of overlapping objects
The generic interval tree we use to speed up range invalidation is an
augmented rbtree that can report all overlapping intervals for a given
range. Therefore we do not need to degrade to a linear list if we find
overlapping objects. Oops.
Signed-off-by: Chris Wilson
Cc: Tvrtko Ursulin
Cc: Michał Winiarski
---
drivers/gpu/drm/i915/i915_gem_userptr.c | 181 +---
1 file changed, 50 insertions(+), 131 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c
b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 59e45b3a6937..7107f2fd38f5 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -49,21 +49,18 @@ struct i915_mmu_notifier {
struct hlist_node node;
struct mmu_notifier mn;
struct rb_root objects;
- struct list_head linear;
- bool has_linear;
};
struct i915_mmu_object {
struct i915_mmu_notifier *mn;
+ struct drm_i915_gem_object *obj;
struct interval_tree_node it;
struct list_head link;
- struct drm_i915_gem_object *obj;
struct work_struct work;
- bool active;
- bool is_linear;
+ bool attached;
};
-static void __cancel_userptr__worker(struct work_struct *work)
+static void cancel_userptr(struct work_struct *work)
{
struct i915_mmu_object *mo = container_of(work, typeof(*mo), work);
struct drm_i915_gem_object *obj = mo->obj;
@@ -94,24 +91,22 @@ static void __cancel_userptr__worker(struct work_struct
*work)
mutex_unlock(&dev->struct_mutex);
}
-static unsigned long cancel_userptr(struct i915_mmu_object *mo)
+static void add_object(struct i915_mmu_object *mo)
{
- unsigned long end = mo->obj->userptr.ptr + mo->obj->base.size;
-
- /* The mmu_object is released late when destroying the
-* GEM object so it is entirely possible to gain a
-* reference on an object in the process of being freed
-* since our serialisation is via the spinlock and not
-* the struct_mutex - and consequently use it after it
-* is freed and then double free it.
-*/
- if (mo->active && kref_get_unless_zero(&mo->obj->base.refcount)) {
- schedule_work(&mo->work);
- /* only schedule one work packet to avoid the refleak */
- mo->active = false;
- }
+ if (mo->attached)
+ return;
+
+ interval_tree_insert(&mo->it, &mo->mn->objects);
+ mo->attached = true;
+}
- return end;
+static void del_object(struct i915_mmu_object *mo)
+{
+ if (!mo->attached)
+ return;
+
+ interval_tree_remove(&mo->it, &mo->mn->objects);
+ mo->attached = false;
}
static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier
*_mn,
@@ -122,28 +117,36 @@ static void
i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn,
struct i915_mmu_notifier *mn =
container_of(_mn, struct i915_mmu_notifier, mn);
struct i915_mmu_object *mo;
+ struct interval_tree_node *it;
+ LIST_HEAD(cancelled);
+
+ if (RB_EMPTY_ROOT(&mn->objects))
+ return;
/* interval ranges are inclusive, but invalidate range is exclusive */
end--;
spin_lock(&mn->lock);
- if (mn->has_linear) {
- list_for_each_entry(mo, &mn->linear, link) {
- if (mo->it.last < start || mo->it.start > end)
- continue;
-
- cancel_userptr(mo);
- }
- } else {
- struct interval_tree_node *it;
+ it = interval_tree_iter_first(&mn->objects, start, end);
+ while (it) {
+ /* The mmu_object is released late when destroying the
+* GEM object so it is entirely possible to gain a
+* reference on an object in the process of being freed
+* since our serialisation is via the spinlock and not
+* the struct_mutex - and consequently use it after it
+* is freed and then double free it. To prevent that
+* use-after-free we only acquire a reference on the
+* object if it is not in the process of being destroyed.
+*/
+ mo = container_of(it, struct i915_mmu_object, it);
+ if (kref_get_unless_zero(&mo->obj->base.refcount))
+ schedule_work(&mo->work);
- it = interval_tree_iter_first(&mn->objects, start, end);
- while (it) {
- mo = container_of(it, struct i915_mmu_object, it);
- start = cancel_userptr(mo);
- it = interval_tree_iter_next(it, start, end);
- }
+ list_add(&mo->link, &cancelled);
+ it = interval_tree_iter_next(it, start, end);
}
+ list_for_each_entry(mo, &canc
Re: [Intel-gfx] [PATCH v2 0/8] Gen9 HW whitelist and Preemption WA patches
On 21/01/2016 16:56, Chris Wilson wrote: On Thu, Jan 21, 2016 at 04:07:22PM +, Arun Siluvery wrote: On 21/01/2016 15:17, Chris Wilson wrote: On Thu, Jan 21, 2016 at 02:00:39PM +, Arun Siluvery wrote: Resending all patches as told by Daniel because I didn't use correct message-id while replying updated version of Patch1 before which means patchwork won't pickup and we won't have CI results. Previous updates regarding Patch1 are available at https://patchwork.freedesktop.org/patch/70527/ Some of the WA patches are now reviewed so I added r-b tags for them. Reviewed: 1, 2, 3, 5, 6 Updated: 4 (to address review comment) To be reviewed: 4, 7, 8 Arun Siluvery (8): drm/i915/gen9: Add framework to whitelist specific GPU registers drm/i915/gen9: Add GEN8_CS_CHICKEN1 to HW whitelist drm/i915/gen9: Add HDC_CHICKEN1 to HW whitelist drm/i915/bxt: Add GEN9_CS_DEBUG_MODE1 to HW whitelist drm/i915/bxt: Add GEN8_L3SQCREG4 to HW whitelist drm/i915/skl: Add GEN8_L3SQCREG4 to HW whitelist drm/i915/skl: Enable Per context Preemption granularity control drm/i915/gen9: Add WaOCLCoherentLineFlush With bland changelogs, it is impossible to assess the severity of the issues being addressed. How many of these do we need to apply to kernel release to address potential user impact? Which do backporters need? Are they critical stability fixes or power tuning or enabling for future features? -Chris All of these changes are required for Preemption so they affect Gen9 only. The whitelist changes are really to support for preemption WA that are going to be part of UMD. The WA data is sketchy and for some of them hsd links are not available so I don't really know the details. Are you happy with adding "Required for future enabling of pre-emptive command execution" to each? It just helps clarify the purpose of the patch, and as I said helps answer the question of "do I need to backport this to stable kernels". yes it helps, will add it and resend. regards Arun -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/2] drm/i915: Adding Broxton GuC Loader Support
This commits adds the Broxton target to the GuC loader
Issue: https://jira01.devtools.intel.com/browse/VIZ-6638
Signed-off-by: Peter Antoine
---
drivers/gpu/drm/i915/intel_guc_loader.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 550921f..8182d11 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -62,6 +62,9 @@
#define I915_SKL_GUC_UCODE "i915/skl_guc_ver4.bin"
MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
+#define I915_BXT_GUC_UCODE "i915/bxt_guc_ver3.bin"
+MODULE_FIRMWARE(I915_BXT_GUC_UCODE);
+
/* User-friendly representation of an enum */
const char *intel_guc_fw_status_repr(enum intel_guc_fw_status status)
{
@@ -587,6 +590,10 @@ void intel_guc_ucode_init(struct drm_device *dev)
fw_path = I915_SKL_GUC_UCODE;
guc_fw->guc_fw_major_wanted = 4;
guc_fw->guc_fw_minor_wanted = 3;
+ } else if (IS_BROXTON(dev)) {
+ fw_path = I915_BXT_GUC_UCODE;
+ guc_fw->guc_fw_major_wanted = 3;
+ guc_fw->guc_fw_minor_wanted = 0;
} else {
i915.enable_guc_submission = false;
fw_path = ""; /* unknown device */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 0/2] Enabling GuC Loading on Broxton
This set of patches will enable the GuC loading for BXT. There is also a fix that is required for GuC submission with the BXT GuC to make it reliable. v2: Remove patch that is to be added with a later patchset. remove erroneous write (merge error) - Jeff McGee Peter Antoine (2): drm/i915: Adding Broxton GuC Loader Support drm/i915: resize the GuC WOPCM for rc6 drivers/gpu/drm/i915/i915_guc_reg.h | 3 ++- drivers/gpu/drm/i915/intel_guc_loader.c | 13 - 2 files changed, 14 insertions(+), 2 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/2] drm/i915: resize the GuC WOPCM for rc6
This patch resizes the GuC WOPCM to so that the GuC and the RC6 memory spaces do not overlap. Issue: https://jira01.devtools.intel.com/browse/VIZ-6638 Signed-off-by: Peter Antoine --- drivers/gpu/drm/i915/i915_guc_reg.h | 3 ++- drivers/gpu/drm/i915/intel_guc_loader.c | 6 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h b/drivers/gpu/drm/i915/i915_guc_reg.h index 685c799..cb938b0 100644 --- a/drivers/gpu/drm/i915/i915_guc_reg.h +++ b/drivers/gpu/drm/i915/i915_guc_reg.h @@ -58,7 +58,8 @@ #define GUC_MAX_IDLE_COUNT _MMIO(0xC3E4) #define GUC_WOPCM_SIZE _MMIO(0xc050) -#define GUC_WOPCM_SIZE_VALUE (0x80 << 12) /* 512KB */ +#define GUC_WOPCM_SIZE_VALUE (0x80 << 12)/* 512KB */ +#define BXT_GUC_WOPCM_SIZE_VALUE (0x70 << 12)/* 448KB */ /* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */ #defineGUC_WOPCM_TOP (GUC_WOPCM_SIZE_VALUE) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 8182d11..69c85d1 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -304,7 +304,11 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv) intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); /* init WOPCM */ - I915_WRITE(GUC_WOPCM_SIZE, GUC_WOPCM_SIZE_VALUE); + if (IS_BROXTON(dev)) + I915_WRITE(GUC_WOPCM_SIZE, BXT_GUC_WOPCM_SIZE_VALUE); + else + I915_WRITE(GUC_WOPCM_SIZE, GUC_WOPCM_SIZE_VALUE); + I915_WRITE(DMA_GUC_WOPCM_OFFSET, GUC_WOPCM_OFFSET_VALUE); /* Enable MIA caching. GuC clock gating is disabled. */ -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib/igt_kms: Add COMIT_ATOMIC to igt_display_commit2()
On Fri, Jan 15, 2016 at 11:06:24AM +0200, Marius Vlad wrote:
> So far, we had only COMMIT_UNIVERSAL and COMMIT_LEGACY, using
> drmModeSetPlane()/drmSetCrtc(). This patch adds COMMIT_ATOMIC
> to igt_display_commit2() that makes use of drmModeAtomicCommit().
>
> Signed-off-by: Marius Vlad
Hmm, this looks pretty similar to the IGT infrastructure I added for
nuclear/atomic about a year ago:
https://github.com/mattrope/intel-gpu-tools/commits/nuclear_pageflip
Specifically the commit "lib/kms: Add nuclear pageflip commit style"
At the time atomic was still too new and the various pieces like libdrm
weren't readily available so we didn't actually merge it. Then I got
too busy and never updated/merged it once the API finalized.
One of the VPG validation teams was in the process of resurrecting my
old IGT work and updating it to match the small changes that snuck in as
the API finalized; it may make more sense for them to leverage your work
here instead since it's already up-to-date. +Cc Avinash.
Also, as noted by Maarten, you're calling drmModeAtomicCommit() at the
wrong layer and submitting every single plane update as its own
transaction (and missing the CRTC-level updates?). You either want to
call commit once per-top level update (for true atomic) or once per CRTC
update (for the "nuclear pageflip" subset). I don't think I had it in
my github repo, but I had some patches that made these two separate
commit styles, COMMIT_ATOMIC and COMMIT_NUCLEAR that would allow either
of these styles to be used.
Matt
> ---
> lib/igt_kms.c | 190
> +-
> lib/igt_kms.h | 33 +-
> 2 files changed, 221 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 497118a..61f7a39 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1306,6 +1306,191 @@ static uint32_t
> igt_plane_get_fb_gem_handle(igt_plane_t *plane)
> igt_assert(r == 0); \
> }
>
> +static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> + "SRC_X",
> + "SRC_Y",
> + "SRC_W",
> + "SRC_H",
> + "CRTC_X",
> + "CRTC_Y",
> + "CRTC_W",
> + "CRTC_H",
> + "FB_ID",
> + "CRTC_ID",
> + "type"
> +};
> +
> +/*
> + * Retrieve all the properies specified in props_name and store them into
> + * plane->atomic_props_plane.
> + */
> +static void
> +igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
> + int type, int num_props, const char **prop_names)
> +{
> + drmModeObjectPropertiesPtr props;
> + int i, j, fd;
> +
> + fd = display->drm_fd;
> +
> + props = drmModeObjectGetProperties(fd, plane->drm_plane->plane_id,
> type);
> + igt_assert(props);
> +
> + for (i = 0; i < props->count_props; i++) {
> + drmModePropertyPtr prop =
> + drmModeGetProperty(fd, props->props[i]);
> +
> + for (j = 0; j < num_props; j++) {
> + if (strcmp(prop->name, prop_names[j]) != 0)
> + continue;
> + plane->atomic_props_plane[j] = props->props[i];
> + break;
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + drmModeFreeObjectProperties(props);
> +}
> +
> +/*
> + * Commit position and fb changes to a DRM plane via the AtomicCommit()
> + * ioctl; if the DRM call to program the plane fails, we'll either fail
> + * immediately (for tests that expect the commit to succeed) or return the
> + * failure code (for tests that expect a specific error code).
> + */
> +static int
> +igt_atomic_plane_commit(igt_plane_t *plane, igt_output_t *output,
> + bool fail_on_error)
> +{
> + igt_display_t *display = output->display;
> +
> + uint32_t fb_id, crtc_id;
> + int ret;
> + uint32_t src_x;
> + uint32_t src_y;
> + uint32_t src_w;
> + uint32_t src_h;
> + int32_t crtc_x;
> + int32_t crtc_y;
> + uint32_t crtc_w;
> + uint32_t crtc_h;
> + drmModeAtomicReq *req;
> +
> + igt_assert(plane->drm_plane);
> +
> + do_or_die(drmSetClientCap(display->drm_fd, DRM_CLIENT_CAP_ATOMIC, 1));
> +
> + /* it's an error to try an unsupported feature */
> + igt_assert(igt_plane_supports_rotation(plane) ||
> + !plane->rotation_changed);
> +
> + fb_id = igt_plane_get_fb_id(plane);
> + crtc_id = output->config.crtc->crtc_id;
> +
> + if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
> +
> + LOG(display,
> + "%s: drmModeAtomicCommit pipe %s, plane %d, disabling\n",
> + igt_output_name(output),
> + kmstest_pipe_name(output->config.pipe),
> + plane->index);
> +
> + req = drmModeAtomicAlloc();
> + igt_atomic_fill_plane_props(display, plane,
> + DRM_MODE_OBJECT_PLANE,
> +
Re: [Intel-gfx] [PATCH 2/6] drm: introduce color correction properties
On Thu, Jan 21, 2016 at 03:03:49PM +, Lionel Landwerlin wrote: ... > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > index 351e801..d2f682c 100644 > --- a/Documentation/DocBook/gpu.tmpl > +++ b/Documentation/DocBook/gpu.tmpl > @@ -2092,6 +2092,54 @@ void intel_crt_init(struct drm_device *dev) > TBD > > > + “DEGAMMA_LUT” > + BLOB > + > + CRTC > + DRM property to set the degamma LUT mapping > + pixel data from the framebuffer before it is given to the > + transformation matrix. The data is an interpreted as an array > + of struct drm_color_lut elements. > + > + > + “DEGAMMA_LUT_SIZE” > + IMMUTABLE IMMUTABLE by itself isn't a property type, just a flag that you can apply in addition to the type. You probably want "RANGE | IMMUTABLE" here (and also on GAMMA_LUT_SIZE below). ...snip... > + “GAMMA_LUT_SIZE” > + IMMUTABLE > + > + CRTC > + DRM property to gives the size of the LUT to > + be set on the GAMMA_LUT property (the size depends on the > + underlying hardware). > + > + > i915 > Generic > "Broadcast RGB" ...snip... > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index d40bab2..6f96c04 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1542,6 +1542,41 @@ static int drm_mode_create_standard_properties(struct > drm_device *dev) > return -ENOMEM; > dev->mode_config.prop_mode_id = prop; > > + prop = drm_property_create(dev, > + DRM_MODE_PROP_BLOB, > + "DEGAMMA_LUT", 0); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.degamma_lut_property = prop; > + > + prop = drm_property_create(dev, > + DRM_MODE_PROP_IMMUTABLE, As noted above, you still need to OR in a valid property type (in this case DRM_MODE_PROP_RANGE); otherwise you'll trip over the the check in drm_property_type_valid() that gets called at the end of drm_property_create(). Matt -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: handle teardown of HWSP when context is freed
Existing code did a kunmap() on the wrong page, and didn't account for the
fact that the HWSP and the default context are different offsets within the
same GEM object.
This patch improves symmetry by creating lrc_teardown_hardware_status_page()
to complement lrc_setup_hardware_status_page(), making sure that they both
take the same argument (pointer to engine). They encapsulate all the details
of the relationship between the default context and the status page, so the
rest of the LRC code doesn't have to know about it.
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/intel_lrc.c | 57
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 73d4347..3914120 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -226,9 +226,8 @@ enum {
#define CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT 0x17
static int intel_lr_context_pin(struct drm_i915_gem_request *rq);
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
- struct drm_i915_gem_object *default_ctx_obj);
-
+static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring);
+static void lrc_teardown_hardware_status_page(struct intel_engine_cs *ring);
/**
* intel_sanitize_enable_execlists() - sanitize i915.enable_execlists
@@ -1536,8 +1535,7 @@ static int gen8_init_common_ring(struct intel_engine_cs
*ring)
struct drm_i915_private *dev_priv = dev->dev_private;
u8 next_context_status_buffer_hw;
- lrc_setup_hardware_status_page(ring,
-
dev_priv->kernel_context->engine[ring->id].state);
+ lrc_setup_hardware_status_page(ring);
I915_WRITE_IMR(ring, ~(ring->irq_enable_mask | ring->irq_keep_mask));
I915_WRITE(RING_HWSTAM(ring->mmio_base), 0x);
@@ -1992,10 +1990,9 @@ void intel_logical_ring_cleanup(struct intel_engine_cs
*ring)
i915_cmd_parser_fini_ring(ring);
i915_gem_batch_pool_fini(&ring->batch_pool);
- if (ring->status_page.obj) {
- kunmap(sg_page(ring->status_page.obj->pages->sgl));
- ring->status_page.obj = NULL;
- }
+ /* Status page should have gone already */
+ WARN_ON(ring->status_page.page_addr);
+ WARN_ON(ring->status_page.obj);
ring->disable_lite_restore_wa = false;
ring->ctx_desc_template = 0;
@@ -2434,6 +2431,11 @@ void intel_lr_context_free(struct intel_context *ctx)
continue;
if (ctx == ctx->i915->kernel_context) {
+ /*
+* The HWSP is part of the default context
+* object in LRC mode.
+*/
+ lrc_teardown_hardware_status_page(ringbuf->ring);
intel_unpin_ringbuffer_obj(ringbuf);
i915_gem_object_ggtt_unpin(ctx_obj);
}
@@ -2482,24 +2484,45 @@ uint32_t intel_lr_context_size(struct intel_engine_cs
*ring)
return ret;
}
-static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
- struct drm_i915_gem_object *default_ctx_obj)
+static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring)
{
- struct drm_i915_private *dev_priv = ring->dev->dev_private;
+ struct drm_i915_private *dev_priv = to_i915(ring->dev);
+ struct intel_context *dctx = dev_priv->kernel_context;
+ struct drm_i915_gem_object *dctx_obj = dctx->engine[ring->id].state;
+ u64 dctx_addr = i915_gem_obj_ggtt_offset(dctx_obj);
struct page *page;
- /* The HWSP is part of the default context object in LRC mode. */
- ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj)
- + LRC_PPHWSP_PN * PAGE_SIZE;
- page = i915_gem_object_get_page(default_ctx_obj, LRC_PPHWSP_PN);
+ /*
+* The HWSP is part of the default context object in LRC mode.
+* Note that it doesn't contribute to the refcount!
+*/
+ page = i915_gem_object_get_page(dctx_obj, LRC_PPHWSP_PN);
ring->status_page.page_addr = kmap(page);
- ring->status_page.obj = default_ctx_obj;
+ ring->status_page.gfx_addr = dctx_addr + LRC_PPHWSP_PN * PAGE_SIZE;
+ ring->status_page.obj = dctx_obj;
I915_WRITE(RING_HWS_PGA(ring->mmio_base),
(u32)ring->status_page.gfx_addr);
POSTING_READ(RING_HWS_PGA(ring->mmio_base));
}
+/* This should be called before the default context is destroyed */
+static void lrc_teardown_hardware_status_page(struct intel_engine_cs *ring)
+{
+ struct drm_i915_gem_object *dctx_obj = ring->status_page.obj;
+ struct page *page;
+
+ WARN_ON(!dctx_obj);
+
+ if (ring->status_page.page_addr) {
+ page = i915_gem_object_get_page(dctx_obj, LRC_PPHWSP_PN);
+
[Intel-gfx] A collection of cleanups
Also includes Nick Hoath's patch to swap context/engine teardown. In-Reply-To: ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: tidy up initialisation failure paths (GEM & LRC)
1. add call to i915_gem_context_fini() to deallocate the default
context(s) if the call to init_rings() fails, so that we don't
leak the context in that situation.
2. remove useless code in intel_logical_ring_cleanup(), presumably
copypasted from legacy ringbuffer version at creation.
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/i915_gem.c | 5 -
drivers/gpu/drm/i915/intel_lrc.c | 10 ++
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 799a53a..041e0e1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4986,8 +4986,11 @@ int i915_gem_init(struct drm_device *dev)
goto out_unlock;
ret = dev_priv->gt.init_rings(dev);
- if (ret)
+ if (ret) {
+ i915_gem_context_fini(dev);
+ /* XXX: anything else to be undone here? */
goto out_unlock;
+ }
ret = i915_gem_init_hw(dev);
if (ret == -EIO) {
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 3914120..c9b0917 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1972,17 +1972,11 @@ static int gen8_init_rcs_context(struct
drm_i915_gem_request *req)
*/
void intel_logical_ring_cleanup(struct intel_engine_cs *ring)
{
- struct drm_i915_private *dev_priv;
-
if (!intel_ring_initialized(ring))
return;
- dev_priv = ring->dev->dev_private;
-
- if (ring->buffer) {
- intel_logical_ring_stop(ring);
- WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0);
- }
+ /* should not be set in LRC mode */
+ WARN_ON(ring->buffer);
if (ring->cleanup)
ring->cleanup(ring);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: tidy up initialisation failure paths (legacy)
1. Fix intel_cleanup_ring_buffer() to handle the error cleanup
case where the ringbuffer has been allocated but map-and-pin
failed. Unpin it iff it's previously been mapped-and-pinned.
2. Fix the error path in intel_init_ring_buffer(), which already
called intel_destroy_ringbuffer_obj(), but failed to free the
actual ringbuffer structure. Calling intel_ringbuffer_free()
instead does both in one go.
3. With the above change, intel_destroy_ringbuffer_obj() is only
called in one place (intel_ringbuffer_free()), so flatten it
into that function.
4. move low-level register accesses from intel_cleanup_ring_buffer()
(which calls intel_stop_ring_buffer(ring) which calls stop_ring())
down into stop_ring() itself), which is already doing low-level
register accesses. Then, intel_cleanup_ring_buffer() no longer
needs 'dev_priv'.
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 47 +++--
1 file changed, 22 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 9030e2b..29de64e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -551,6 +551,8 @@ static bool stop_ring(struct intel_engine_cs *ring)
I915_WRITE_MODE(ring, _MASKED_BIT_DISABLE(STOP_RING));
}
+ WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & MODE_IDLE) == 0);
+
return (I915_READ_HEAD(ring) & HEAD_ADDR) == 0;
}
@@ -2071,12 +2073,6 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device
*dev,
return 0;
}
-static void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
-{
- drm_gem_object_unreference(&ringbuf->obj->base);
- ringbuf->obj = NULL;
-}
-
static int intel_alloc_ringbuffer_obj(struct drm_device *dev,
struct intel_ringbuffer *ringbuf)
{
@@ -2139,11 +2135,14 @@ struct intel_ringbuffer *
}
void
-intel_ringbuffer_free(struct intel_ringbuffer *ring)
+intel_ringbuffer_free(struct intel_ringbuffer *ringbuf)
{
- intel_destroy_ringbuffer_obj(ring);
- list_del(&ring->link);
- kfree(ring);
+ if (ringbuf->obj) {
+ drm_gem_object_unreference(&ringbuf->obj->base);
+ ringbuf->obj = NULL;
+ }
+ list_del(&ringbuf->link);
+ kfree(ringbuf);
}
static int intel_init_ring_buffer(struct drm_device *dev,
@@ -2171,6 +2170,13 @@ static int intel_init_ring_buffer(struct drm_device *dev,
}
ring->buffer = ringbuf;
+ ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
+ if (ret) {
+ DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n",
+ ring->name, ret);
+ goto error;
+ }
+
if (I915_NEED_GFX_HWS(dev)) {
ret = init_status_page(ring);
if (ret)
@@ -2182,14 +2188,6 @@ static int intel_init_ring_buffer(struct drm_device *dev,
goto error;
}
- ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
- if (ret) {
- DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n",
- ring->name, ret);
- intel_destroy_ringbuffer_obj(ringbuf);
- goto error;
- }
-
ret = i915_cmd_parser_init_ring(ring);
if (ret)
goto error;
@@ -2203,19 +2201,18 @@ static int intel_init_ring_buffer(struct drm_device
*dev,
void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
{
- struct drm_i915_private *dev_priv;
+ struct intel_ringbuffer *ringbuf;
if (!intel_ring_initialized(ring))
return;
- dev_priv = to_i915(ring->dev);
-
- if (ring->buffer) {
+ ringbuf = ring->buffer;
+ if (ringbuf) {
intel_stop_ring_buffer(ring);
- WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) &
MODE_IDLE) == 0);
- intel_unpin_ringbuffer_obj(ring->buffer);
- intel_ringbuffer_free(ring->buffer);
+ if (ringbuf->virtual_start)
+ intel_unpin_ringbuffer_obj(ringbuf);
+ intel_ringbuffer_free(ringbuf);
ring->buffer = NULL;
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
