[Intel-gfx] ✗ Fi.CI.BAT: warning for GEM/GuC Suspend/Resume/Reset fixes and restructuring (rev5)
== Series Details == Series: GEM/GuC Suspend/Resume/Reset fixes and restructuring (rev5) URL : https://patchwork.freedesktop.org/series/30802/ State : warning == Summary == Series 30802v5 GEM/GuC Suspend/Resume/Reset fixes and restructuring https://patchwork.freedesktop.org/api/1.0/series/30802/revisions/5/mbox/ Test chamelium: Subgroup hdmi-crc-fast: pass -> DMESG-WARN (fi-skl-6700k) Test kms_pipe_crc_basic: Subgroup nonblocking-crc-pipe-b-frame-sequence: dmesg-fail -> DMESG-WARN (fi-cfl-s) fdo#102294 Subgroup nonblocking-crc-pipe-c: incomplete -> SKIP (fi-cfl-s) Test drv_module_reload: Subgroup basic-reload-inject: pass -> DMESG-WARN (fi-glk-1) fdo#102777 fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294 fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777 fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:441s fi-bdw-gvtdvmtotal:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:472s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:420s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:518s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:278s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:507s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:494s fi-cfl-s total:289 pass:223 dwarn:34 dfail:0 fail:0 skip:32 time:534s fi-cnl-y total:289 pass:259 dwarn:0 dfail:0 fail:3 skip:27 time:651s fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:414s fi-glk-1 total:289 pass:259 dwarn:1 dfail:0 fail:0 skip:29 time:568s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:426s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:401s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:429s fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:489s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:462s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:469s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:579s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:588s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:542s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:450s fi-skl-6700k total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:747s fi-skl-6770hqtotal:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:496s fi-skl-gvtdvmtotal:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:479s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:566s fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:413s c1ed501217782b35094792696924a1d22ee83c0f drm-tip: 2017y-09m-28d-04h-52m-47s UTC integration manifest 33793ea7acaf drm/i915/guc: Fix GuC cleanup in unload path d0d5fde9d1de drm/i915/guc: Disable GuC submission and suspend it prior to i915 reset 19b890859702 drm/i915/guc: Update GuC functionality in intel_uc_suspend/intel_uc_resume afc823451d74 drm/i915/guc: Update GuC ggtt.invalidate/interrupts/communication across RPM suspend/resume 618c9c6ed8a3 drm/i915/guc: Update i915.enable_guc_loading check in intel_uc_fini_hw 4397c4f2e4cb drm/i915/guc: Make GuC related disable/destroy functions not depend on i915.enable_guc_submission 565b5d5937f0 drm/i915/guc: Introduce intel_uc_sanitize 54471d03302b drm/i915: Create uC runtime and system suspend/resume helpers df5c5562d597 drm/i915: Move i915_gem_restore_fences to i915_gem_resume 32506e3eaec1 drm/i915: Update GEM suspend/resume flows with GuC suspend/resume functions 56ce39172960 drm/i915: Create GEM runtime resume helper and handle GEM suspend/resume errors == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5843/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Allow optimized platform checks
On Wed, 27 Sep 2017, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> If we store the platform as a bitmask, and convert the
> IS_PLATFORM macro to use it, we allow the compiler to
> merge the IS_PLATFORM(a) || IS_PLATFORM(b) || ... checks
> into a single conditional.
>
> As a secondary benefit this saves almost 1k of text:
>
> text data bss dec hex filename
> -1460254 600143656 1523924 1740d4 drivers/gpu/drm/i915/i915.ko
> +1459260 600263656 1522942 173cfe drivers/gpu/drm/i915/i915.ko
>
> v2: Removed the infamous -1.
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Jani Nikula
Acked-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/i915_drv.c | 4
> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 03bbe23e4df8..187c0fad1b79 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -870,6 +870,10 @@ static int i915_driver_init_early(struct
> drm_i915_private *dev_priv,
> memcpy(device_info, match_info, sizeof(*device_info));
> device_info->device_id = dev_priv->drm.pdev->device;
>
> + BUILD_BUG_ON(INTEL_MAX_PLATFORMS >
> + sizeof(device_info->platform_mask) * BITS_PER_BYTE);
> + device_info->platform_mask = BIT(device_info->platform);
> +
> BUG_ON(device_info->gen > sizeof(device_info->gen_mask) *
> BITS_PER_BYTE);
> device_info->gen_mask = BIT(device_info->gen - 1);
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bda91db3fc46..3a6a34882427 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -861,6 +861,7 @@ struct intel_device_info {
> u8 ring_mask; /* Rings supported by the HW */
>
> enum intel_platform platform;
> + u32 platform_mask;
>
> u32 display_mmio_offset;
>
> @@ -2986,7 +2987,7 @@ intel_info(const struct drm_i915_private *dev_priv)
> #define IS_REVID(p, since, until) \
> (INTEL_REVID(p) >= (since) && INTEL_REVID(p) <= (until))
>
> -#define IS_PLATFORM(dev_priv, p) ((dev_priv)->info.platform == (p))
> +#define IS_PLATFORM(dev_priv, p) ((dev_priv)->info.platform_mask & BIT(p))
>
> #define IS_I830(dev_priv)IS_PLATFORM(dev_priv, INTEL_I830)
> #define IS_I845G(dev_priv) IS_PLATFORM(dev_priv, INTEL_I845G)
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for GEM/GuC Suspend/Resume/Reset fixes and restructuring (rev5)
On 9/28/2017 12:41 PM, Patchwork wrote: == Series Details == Series: GEM/GuC Suspend/Resume/Reset fixes and restructuring (rev5) URL : https://patchwork.freedesktop.org/series/30802/ State : warning == Summary == Series 30802v5 GEM/GuC Suspend/Resume/Reset fixes and restructuring https://patchwork.freedesktop.org/api/1.0/series/30802/revisions/5/mbox/ Test chamelium: Subgroup hdmi-crc-fast: pass -> DMESG-WARN (fi-skl-6700k) This looks sporadic. This test passed in trybot with same patchset. Filed below bug: https://bugs.freedesktop.org/show_bug.cgi?id=103019 Test kms_pipe_crc_basic: Subgroup nonblocking-crc-pipe-b-frame-sequence: dmesg-fail -> DMESG-WARN (fi-cfl-s) fdo#102294 Subgroup nonblocking-crc-pipe-c: incomplete -> SKIP (fi-cfl-s) Test drv_module_reload: Subgroup basic-reload-inject: pass -> DMESG-WARN (fi-glk-1) fdo#102777 fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294 fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777 fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:441s fi-bdw-gvtdvmtotal:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:472s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:420s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:518s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:278s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:507s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:494s fi-cfl-s total:289 pass:223 dwarn:34 dfail:0 fail:0 skip:32 time:534s fi-cnl-y total:289 pass:259 dwarn:0 dfail:0 fail:3 skip:27 time:651s fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:414s fi-glk-1 total:289 pass:259 dwarn:1 dfail:0 fail:0 skip:29 time:568s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:426s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:401s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:429s fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:489s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:462s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:469s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:579s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:588s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:542s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:450s fi-skl-6700k total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:747s fi-skl-6770hqtotal:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:496s fi-skl-gvtdvmtotal:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:479s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:566s fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:413s c1ed501217782b35094792696924a1d22ee83c0f drm-tip: 2017y-09m-28d-04h-52m-47s UTC integration manifest 33793ea7acaf drm/i915/guc: Fix GuC cleanup in unload path d0d5fde9d1de drm/i915/guc: Disable GuC submission and suspend it prior to i915 reset 19b890859702 drm/i915/guc: Update GuC functionality in intel_uc_suspend/intel_uc_resume afc823451d74 drm/i915/guc: Update GuC ggtt.invalidate/interrupts/communication across RPM suspend/resume 618c9c6ed8a3 drm/i915/guc: Update i915.enable_guc_loading check in intel_uc_fini_hw 4397c4f2e4cb drm/i915/guc: Make GuC related disable/destroy functions not depend on i915.enable_guc_submission 565b5d5937f0 drm/i915/guc: Introduce intel_uc_sanitize 54471d03302b drm/i915: Create uC runtime and system suspend/resume helpers df5c5562d597 drm/i915: Move i915_gem_restore_fences to i915_gem_resume 32506e3eaec1 drm/i915: Update GEM suspend/resume flows with GuC suspend/resume functions 56ce39172960 drm/i915: Create GEM runtime resume helper and handle GEM suspend/resume errors == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5843/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Also discard second CRC on gen8+ platforms.
This fixes my issue with GLK+MIPI/DSI when running IGT test
kms_frontbuffer_tracking --r basic
Tested-by: Mika Kahola
On Wed, 2017-09-27 at 17:20 -0700, Rodrigo Vivi wrote:
> One of the differences I spotted for GEN8+ platforms when
> compared to older platforms is that spec for BDW+ includes
> this sentence:
>
> "The first CRC done indication after CRC is first enabled is
> from only a partial frame, so it will not have the expected
> CRC result."
>
> This is an indication that on BDW+ platforms, by the time
> we receive the interrupt the CRC is not accurate yet for
> the full frame. That would be ok, because we are already
> skipping the first CRC for all platforms. However the comment
> on the code state that it is for some unknown reason. Also,
> on CHV (gen8 lp) we were already discarding the second CRC
> as well to make sure we have a reliable CRC on hand.
>
> So based on all ou tests and bugs it seems that it is not
> on CHV that needs to discard 2 first CRCs, but all BDW+
> platforms.
>
> Starting on SKL we have this CRC done bit (24), but the
> experiments around the use of this bit wasn't that stable
> as just discarding the second CRC. So, let's for now
> just move with CHV solution for all gen8+ platforms and
> make our CI a bit more stable.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102374
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101309
> Cc: Mika Kahola
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/i915_irq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 0b7562135d1c..efd7827ff181 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1647,11 +1647,11 @@ static void
> display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
> * bonkers. So let's just wait for the next vblank
> and read
> * out the buggy result.
> *
> - * On CHV sometimes the second CRC is bonkers as
> well, so
> + * On GEN8+ sometimes the second CRC is bonkers as
> well, so
> * don't trust that one either.
> */
> if (pipe_crc->skipped == 0 ||
> - (IS_CHERRYVIEW(dev_priv) && pipe_crc->skipped ==
> 1)) {
> + (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped
> == 1)) {
> pipe_crc->skipped++;
> spin_unlock(&pipe_crc->lock);
> return;
--
Mika Kahola - Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/8] drm/i915/bios: parse DDI ports also for CHV for HDMI DDC pin and DP AUX channel
While technically CHV isn't DDI, we do look at the VBT based DDI port
info for HDMI DDC pin and DP AUX channel. (We call these "alternate",
but they're really just something that aren't platform defaults.)
In commit e4ab73a13291 ("drm/i915: Respect alternate_ddc_pin for all DDI
ports") Ville writes, "IIRC there may be CHV system that might actually
need this."
I'm not sure why there couldn't be even more platforms that need this,
but start conservative, and parse the info for CHV in addition to DDI.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100553
Reported-by: Marek Wilczewski
Cc: sta...@vger.kernel.org
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 3747d8df0175..6b8d396ab605 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1247,7 +1247,7 @@ static void parse_ddi_ports(struct drm_i915_private
*dev_priv,
{
enum port port;
- if (!HAS_DDI(dev_priv))
+ if (!HAS_DDI(dev_priv) && !IS_CHERRYVIEW(dev_priv))
return;
if (!dev_priv->vbt.child_dev_num)
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/8] drm/i915/bios: refactor parse general definitions
Early return on failures. Rename the variable for later merging with
parse_device_mappings().
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.c | 34 --
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 6b8d396ab605..225e2ecde2a0 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -435,21 +435,27 @@ static void
parse_general_definitions(struct drm_i915_private *dev_priv,
const struct bdb_header *bdb)
{
- const struct bdb_general_definitions *general;
-
- general = find_section(bdb, BDB_GENERAL_DEFINITIONS);
- if (general) {
- u16 block_size = get_blocksize(general);
- if (block_size >= sizeof(*general)) {
- int bus_pin = general->crt_ddc_gmbus_pin;
- DRM_DEBUG_KMS("crt_ddc_bus_pin: %d\n", bus_pin);
- if (intel_gmbus_is_valid_pin(dev_priv, bus_pin))
- dev_priv->vbt.crt_ddc_pin = bus_pin;
- } else {
- DRM_DEBUG_KMS("BDB_GD too small (%d). Invalid.\n",
- block_size);
- }
+ const struct bdb_general_definitions *defs;
+ u16 block_size;
+ int bus_pin;
+
+ defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
+ if (!defs) {
+ DRM_DEBUG_KMS("General definitions block not found\n");
+ return;
+ }
+
+ block_size = get_blocksize(defs);
+ if (block_size < sizeof(*defs)) {
+ DRM_DEBUG_KMS("General definitions block too small (%u)\n",
+ block_size);
+ return;
}
+
+ bus_pin = defs->crt_ddc_gmbus_pin;
+ DRM_DEBUG_KMS("crt_ddc_bus_pin: %d\n", bus_pin);
+ if (intel_gmbus_is_valid_pin(dev_priv, bus_pin))
+ dev_priv->vbt.crt_ddc_pin = bus_pin;
}
static const struct child_device_config *
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/8] drm/i915/bios: VBT parsing fixes and cleanups
More VBT parsing fixes and refactoring. BR, Jani. Jani Nikula (8): drm/i915/bios: parse DDI ports also for CHV for HDMI DDC pin and DP AUX channel drm/i915/bios: refactor parse general definitions drm/i915/bios: don't initialize fields based on vbt version drm/i915/bios: remove an unnecessary temp variable drm/i915/bios: cleanup comments and useless return drm/i915/bios: merge parse_device_mapping() into parse_general_definitions() drm/i915/bios: parse SDVO device mapping from pre-parsed child devices drm/i915/bios: don't pass bdb to parsers that don't parse VBT directly drivers/gpu/drm/i915/intel_bios.c | 130 -- 1 file changed, 41 insertions(+), 89 deletions(-) -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/8] drm/i915/bios: don't initialize fields based on vbt version
In theory, these might clobber information for older VBT versions.
We might have to store the BDB version for later parsing, but currently
all code accessing these fields will only use them on newer platforms
with new enough BDB versions.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 225e2ecde2a0..435db26f5a1f 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1354,19 +1354,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
*/
memcpy(child_dev_ptr, child,
min_t(size_t, defs->child_dev_size, sizeof(*child)));
-
- /*
-* copied full block, now init values when they are not
-* available in current version
-*/
- if (bdb->version < 196) {
- /* Set default values for bits added from v196 */
- child_dev_ptr->iboost = 0;
- child_dev_ptr->hpd_invert = 0;
- }
-
- if (bdb->version < 192)
- child_dev_ptr->lspcon = 0;
}
return;
}
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/8] drm/i915/bios: remove an unnecessary temp variable
Prepare for merging parse_device_mapping() into
parse_general_definitions(). No functional changes.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 435db26f5a1f..d0fedac0322f 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1272,7 +1272,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
{
const struct bdb_general_definitions *defs;
const struct child_device_config *child;
- struct child_device_config *child_dev_ptr;
int i, child_device_num, count;
u8 expected_size;
u16 block_size;
@@ -1344,16 +1343,14 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
continue;
}
- child_dev_ptr = dev_priv->vbt.child_dev + count;
- count++;
-
/*
* Copy as much as we know (sizeof) and is available
* (child_dev_size) of the child device. Accessing the data must
* depend on VBT version.
*/
- memcpy(child_dev_ptr, child,
+ memcpy(dev_priv->vbt.child_dev + count, child,
min_t(size_t, defs->child_dev_size, sizeof(*child)));
+ count++;
}
return;
}
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 8/8] drm/i915/bios: don't pass bdb to parsers that don't parse VBT directly
Hint that you're not supposed to look at VBT in these functions.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.c | 20 +---
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 391487a2a0b0..e809a9c347d3 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -438,8 +438,7 @@ child_device_ptr(const struct bdb_general_definitions
*defs, int i)
}
static void
-parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
- const struct bdb_header *bdb)
+parse_sdvo_device_mapping(struct drm_i915_private *dev_priv, u8 bdb_version)
{
struct sdvo_device_mapping *mapping;
const struct child_device_config *child;
@@ -1073,7 +1072,7 @@ static void sanitize_aux_ch(struct drm_i915_private
*dev_priv,
}
static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
- const struct bdb_header *bdb)
+ u8 bdb_version)
{
struct child_device_config *it, *child = NULL;
struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port];
@@ -1184,7 +1183,7 @@ static void parse_ddi_port(struct drm_i915_private
*dev_priv, enum port port,
sanitize_aux_ch(dev_priv, port);
}
- if (bdb->version >= 158) {
+ if (bdb_version >= 158) {
/* The VBT HDMI level shift values match the table we have. */
hdmi_level_shift = child->hdmi_level_shifter_value;
DRM_DEBUG_KMS("VBT HDMI level shift for port %c: %d\n",
@@ -1194,7 +1193,7 @@ static void parse_ddi_port(struct drm_i915_private
*dev_priv, enum port port,
}
/* Parse the I_boost config for SKL and above */
- if (bdb->version >= 196 && child->iboost) {
+ if (bdb_version >= 196 && child->iboost) {
info->dp_boost_level = translate_iboost(child->dp_iboost_level);
DRM_DEBUG_KMS("VBT (e)DP boost level for port %c: %d\n",
port_name(port), info->dp_boost_level);
@@ -1204,8 +1203,7 @@ static void parse_ddi_port(struct drm_i915_private
*dev_priv, enum port port,
}
}
-static void parse_ddi_ports(struct drm_i915_private *dev_priv,
- const struct bdb_header *bdb)
+static void parse_ddi_ports(struct drm_i915_private *dev_priv, u8 bdb_version)
{
enum port port;
@@ -1215,11 +1213,11 @@ static void parse_ddi_ports(struct drm_i915_private
*dev_priv,
if (!dev_priv->vbt.child_dev_num)
return;
- if (bdb->version < 155)
+ if (bdb_version < 155)
return;
for (port = PORT_A; port < I915_MAX_PORTS; port++)
- parse_ddi_port(dev_priv, port, bdb);
+ parse_ddi_port(dev_priv, port, bdb_version);
}
static void
@@ -1503,8 +1501,8 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
parse_mipi_sequence(dev_priv, bdb);
/* Further processing on pre-parsed data */
- parse_sdvo_device_mapping(dev_priv, bdb);
- parse_ddi_ports(dev_priv, bdb);
+ parse_sdvo_device_mapping(dev_priv, bdb->version);
+ parse_ddi_ports(dev_priv, bdb->version);
out:
if (!vbt) {
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/8] drm/i915/bios: parse SDVO device mapping from pre-parsed child devices
We parse and store the child devices in
parse_general_definitions(). There is no need to parse the VBT block
again for SDVO device mapping. Do the same as we do in
parse_ddi_ports().
We no longer have access to child device size at this stage, but we also
don't need to worry about reading past the child device anymore. Instead
of a child device size check, do a mild optimization by limiting the
parsing to gens 3 through 7.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.c | 39 ---
1 file changed, 12 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 0845e13c59d1..391487a2a0b0 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -442,37 +442,21 @@ parse_sdvo_device_mapping(struct drm_i915_private
*dev_priv,
const struct bdb_header *bdb)
{
struct sdvo_device_mapping *mapping;
- const struct bdb_general_definitions *defs;
const struct child_device_config *child;
- int i, child_device_num, count;
- u16 block_size;
-
- defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
- if (!defs) {
- DRM_DEBUG_KMS("No general definition block is found, unable to
construct sdvo mapping.\n");
- return;
- }
+ int i, count = 0;
/*
-* Only parse SDVO mappings when the general definitions block child
-* device size matches that of the *legacy* child device config
-* struct. Thus, SDVO mapping will be skipped for newer VBT.
+* Only parse SDVO mappings on gens that could have SDVO. This isn't
+* accurate and doesn't have to be, as long as it's not too strict.
*/
- if (defs->child_dev_size != LEGACY_CHILD_DEVICE_CONFIG_SIZE) {
- DRM_DEBUG_KMS("Unsupported child device size for SDVO
mapping.\n");
+ if (!IS_GEN(dev_priv, 3, 7)) {
+ DRM_DEBUG_KMS("Skipping SDVO device mapping\n");
return;
}
- /* get the block size of general definitions */
- block_size = get_blocksize(defs);
- /* get the number of child device */
- child_device_num = (block_size - sizeof(*defs)) / defs->child_dev_size;
- count = 0;
- for (i = 0; i < child_device_num; i++) {
- child = child_device_ptr(defs, i);
- if (!child->device_type) {
- /* skip the device block if device type is invalid */
- continue;
- }
+
+ for (i = 0, count = 0; i < dev_priv->vbt.child_dev_num; i++) {
+ child = dev_priv->vbt.child_dev + i;
+
if (child->slave_addr != SLAVE_ADDR1 &&
child->slave_addr != SLAVE_ADDR2) {
/*
@@ -523,7 +507,6 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
/* No SDVO device info is found */
DRM_DEBUG_KMS("No SDVO device info is found in VBT\n");
}
- return;
}
static void
@@ -1513,12 +1496,14 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
parse_lfp_panel_data(dev_priv, bdb);
parse_lfp_backlight(dev_priv, bdb);
parse_sdvo_panel_data(dev_priv, bdb);
- parse_sdvo_device_mapping(dev_priv, bdb);
parse_driver_features(dev_priv, bdb);
parse_edp(dev_priv, bdb);
parse_psr(dev_priv, bdb);
parse_mipi_config(dev_priv, bdb);
parse_mipi_sequence(dev_priv, bdb);
+
+ /* Further processing on pre-parsed data */
+ parse_sdvo_device_mapping(dev_priv, bdb);
parse_ddi_ports(dev_priv, bdb);
out:
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/8] drm/i915/bios: merge parse_device_mapping() into parse_general_definitions()
They're both parsing the same block, and there's no need for them to be
split. The former also benefits from the range checks in the latter.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.c | 48 +--
1 file changed, 16 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 9cfe89eed501..0845e13c59d1 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -431,33 +431,6 @@ parse_general_features(struct drm_i915_private *dev_priv,
dev_priv->vbt.fdi_rx_polarity_inverted);
}
-static void
-parse_general_definitions(struct drm_i915_private *dev_priv,
- const struct bdb_header *bdb)
-{
- const struct bdb_general_definitions *defs;
- u16 block_size;
- int bus_pin;
-
- defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
- if (!defs) {
- DRM_DEBUG_KMS("General definitions block not found\n");
- return;
- }
-
- block_size = get_blocksize(defs);
- if (block_size < sizeof(*defs)) {
- DRM_DEBUG_KMS("General definitions block too small (%u)\n",
- block_size);
- return;
- }
-
- bus_pin = defs->crt_ddc_gmbus_pin;
- DRM_DEBUG_KMS("crt_ddc_bus_pin: %d\n", bus_pin);
- if (intel_gmbus_is_valid_pin(dev_priv, bus_pin))
- dev_priv->vbt.crt_ddc_pin = bus_pin;
-}
-
static const struct child_device_config *
child_device_ptr(const struct bdb_general_definitions *defs, int i)
{
@@ -1267,20 +1240,34 @@ static void parse_ddi_ports(struct drm_i915_private
*dev_priv,
}
static void
-parse_device_mapping(struct drm_i915_private *dev_priv,
-const struct bdb_header *bdb)
+parse_general_definitions(struct drm_i915_private *dev_priv,
+ const struct bdb_header *bdb)
{
const struct bdb_general_definitions *defs;
const struct child_device_config *child;
int i, child_device_num, count;
u8 expected_size;
u16 block_size;
+ int bus_pin;
defs = find_section(bdb, BDB_GENERAL_DEFINITIONS);
if (!defs) {
DRM_DEBUG_KMS("No general definition block is found, no devices
defined.\n");
return;
}
+
+ block_size = get_blocksize(defs);
+ if (block_size < sizeof(*defs)) {
+ DRM_DEBUG_KMS("General definitions block too small (%u)\n",
+ block_size);
+ return;
+ }
+
+ bus_pin = defs->crt_ddc_gmbus_pin;
+ DRM_DEBUG_KMS("crt_ddc_bus_pin: %d\n", bus_pin);
+ if (intel_gmbus_is_valid_pin(dev_priv, bus_pin))
+ dev_priv->vbt.crt_ddc_pin = bus_pin;
+
if (bdb->version < 106) {
expected_size = 22;
} else if (bdb->version < 111) {
@@ -1310,8 +1297,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
return;
}
- /* get the block size of general definitions */
- block_size = get_blocksize(defs);
/* get the number of child device */
child_device_num = (block_size - sizeof(*defs)) / defs->child_dev_size;
count = 0;
@@ -1529,7 +1514,6 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
parse_lfp_backlight(dev_priv, bdb);
parse_sdvo_panel_data(dev_priv, bdb);
parse_sdvo_device_mapping(dev_priv, bdb);
- parse_device_mapping(dev_priv, bdb);
parse_driver_features(dev_priv, bdb);
parse_edp(dev_priv, bdb);
parse_psr(dev_priv, bdb);
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/8] drm/i915/bios: cleanup comments and useless return
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index d0fedac0322f..9cfe89eed501 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1318,10 +1318,8 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
/* get the number of child device that is present */
for (i = 0; i < child_device_num; i++) {
child = child_device_ptr(defs, i);
- if (!child->device_type) {
- /* skip the device block if device type is invalid */
+ if (!child->device_type)
continue;
- }
count++;
}
if (!count) {
@@ -1338,10 +1336,8 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
count = 0;
for (i = 0; i < child_device_num; i++) {
child = child_device_ptr(defs, i);
- if (!child->device_type) {
- /* skip the device block if device type is invalid */
+ if (!child->device_type)
continue;
- }
/*
* Copy as much as we know (sizeof) and is available
@@ -1352,7 +1348,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
min_t(size_t, defs->child_dev_size, sizeof(*child)));
count++;
}
- return;
}
/* Common defaults which may be overridden by VBT. */
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v4 2/6] lib/igt_kms: Fix off-by-one bug on skip of missing pipe
On Wed, Sep 27, 2017 at 03:34:15PM -0300, Gabriel Krisman Bertazi wrote:
> display->n_pipes is zero-indexed, so N returned in
> igt_display_get_n_pipes is already not a valid pipe. This patch
> prevents kms_ccs from going nuts when testing the first unxesting pipe.
^unexisting
--
Petri Latvala
>
> Signed-off-by: Gabriel Krisman Bertazi
> Reviewed-by: Maarten Lankhorst
> ---
> lib/igt_kms.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 7bcafc072f70..c7b169afba54 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1865,7 +1865,7 @@ void igt_display_require_output_on_pipe(igt_display_t
> *display, enum pipe pipe)
> {
> igt_output_t *output;
>
> - igt_skip_on_f(igt_display_get_n_pipes(display) < pipe,
> + igt_skip_on_f(pipe >= igt_display_get_n_pipes(display),
> "Pipe %s does not exist.\n", kmstest_pipe_name(pipe));
>
> for_each_valid_output_on_pipe(display, pipe, output)
> --
> 2.11.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 1/3] Fix rlim_cur compiler warnings when building on ARM.
On Wed, Sep 27, 2017 at 11:37:33AM -0700, Eric Anholt wrote:
> Signed-off-by: Eric Anholt
For the series:
Reviewed-by: Petri Latvala
> ---
> benchmarks/prime_lookup.c | 2 +-
> tests/gem_exec_reuse.c| 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/benchmarks/prime_lookup.c b/benchmarks/prime_lookup.c
> index e995b766a173..d6c397299fcb 100644
> --- a/benchmarks/prime_lookup.c
> +++ b/benchmarks/prime_lookup.c
> @@ -143,7 +143,7 @@ static bool allow_files(unsigned min)
> return false;
>
> igt_info("Current file limit is %ld, estimated we need %d\n",
> - rlim.rlim_cur, min);
> + (long)rlim.rlim_cur, min);
>
> if (rlim.rlim_cur > min)
> return true;
> diff --git a/tests/gem_exec_reuse.c b/tests/gem_exec_reuse.c
> index cdfa9783f5b7..4e3907cf7b52 100644
> --- a/tests/gem_exec_reuse.c
> +++ b/tests/gem_exec_reuse.c
> @@ -122,7 +122,8 @@ static uint64_t max_open_files(void)
> if (getrlimit(RLIMIT_NOFILE, &rlim))
> rlim.rlim_cur = 64 << 10;
>
> - igt_info("Process limit for file descriptors is %lu\n", rlim.rlim_cur);
> + igt_info("Process limit for file descriptors is %lu\n",
> + (long)rlim.rlim_cur);
> return rlim.rlim_cur;
> }
>
> --
> 2.14.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] Fix compilation on some distros
On Wed, Sep 27, 2017 at 04:08:27PM -0700, James Ausmus wrote: > Some distros (such as Gentoo) are removing the include of > sys/sysmacros.h from sys/types.h. Explicitly include sysmacros.h in > files where we use the minor() and major() functions. > > Signed-off-by: James Ausmus Reviewed-by: Petri Latvala > --- > lib/igt_debugfs.c | 1 + > lib/igt_sysfs.c | 1 + > tools/aubdump.c | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c > index 1e8c8cc3cd44..60b29e3a025a 100644 > --- a/lib/igt_debugfs.c > +++ b/lib/igt_debugfs.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > #include > #include > #include > diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c > index 817678bc28ed..f4e306003b01 100644 > --- a/lib/igt_sysfs.c > +++ b/lib/igt_sysfs.c > @@ -24,6 +24,7 @@ > > #include > #include > +#include > #include > #include > #include > diff --git a/tools/aubdump.c b/tools/aubdump.c > index 78d183f49adc..ee4d99b06ed1 100644 > --- a/tools/aubdump.c > +++ b/tools/aubdump.c > @@ -30,6 +30,7 @@ > #include > #include > #include > +#include > #include > #include > #include > -- > 2.14.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/bios: VBT parsing fixes and cleanups
== Series Details == Series: drm/i915/bios: VBT parsing fixes and cleanups URL : https://patchwork.freedesktop.org/series/31051/ State : success == Summary == Series 31051v1 drm/i915/bios: VBT parsing fixes and cleanups https://patchwork.freedesktop.org/api/1.0/series/31051/revisions/1/mbox/ Test drv_module_reload: Subgroup basic-reload-inject: pass -> DMESG-WARN (fi-glk-1) fdo#102777 fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777 fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:444s fi-bdw-gvtdvmtotal:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:466s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:419s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:518s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:279s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:500s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:491s fi-cnl-y total:289 pass:258 dwarn:0 dfail:0 fail:4 skip:27 time:648s fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:416s fi-glk-1 total:289 pass:259 dwarn:1 dfail:0 fail:0 skip:29 time:561s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:430s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:403s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:431s fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:493s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:464s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:475s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:576s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:586s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:545s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:452s fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:762s fi-skl-6770hqtotal:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:488s fi-skl-gvtdvmtotal:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:482s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:575s fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:418s fi-bxt-dsi failed to connect after reboot c1ed501217782b35094792696924a1d22ee83c0f drm-tip: 2017y-09m-28d-04h-52m-47s UTC integration manifest 8b0e808a9c7e drm/i915/bios: don't pass bdb to parsers that don't parse VBT directly d8998f883ab5 drm/i915/bios: parse SDVO device mapping from pre-parsed child devices 9488a8a7070d drm/i915/bios: merge parse_device_mapping() into parse_general_definitions() 82f12f508663 drm/i915/bios: cleanup comments and useless return f60cb1a89dd3 drm/i915/bios: remove an unnecessary temp variable 3a41abe26c68 drm/i915/bios: don't initialize fields based on vbt version 5fcddd78ef61 drm/i915/bios: refactor parse general definitions 3027d6ddfb20 drm/i915/bios: parse DDI ports also for CHV for HDMI DDC pin and DP AUX channel == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5844/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt 1/3] benchmark/gem_busy: Compare polling with syncobj_wait
Quoting Tvrtko Ursulin (2017-09-28 07:53:56)
>
> On 25/09/2017 21:26, Chris Wilson wrote:
> > Signed-off-by: Chris Wilson
> > ---
> > benchmarks/gem_busy.c | 73
> > ++-
> > 1 file changed, 72 insertions(+), 1 deletion(-)
> >
> > diff --git a/benchmarks/gem_busy.c b/benchmarks/gem_busy.c
> > index f050454b..9649ea02 100644
> > --- a/benchmarks/gem_busy.c
> > +++ b/benchmarks/gem_busy.c
> > @@ -58,6 +58,15 @@
> > #define DMABUF 0x4
> > #define WAIT 0x8
> > #define SYNC 0x10
> > +#define SYNCOBJ 0x20
> > +
> > +#define LOCAL_I915_EXEC_FENCE_ARRAY (1 << 19)
> > +struct local_gem_exec_fence {
> > + uint32_t handle;
> > + uint32_t flags;
> > +#define LOCAL_EXEC_FENCE_WAIT (1 << 0)
> > +#define LOCAL_EXEC_FENCE_SIGNAL (1 << 1)
> > +};
> >
> > static void gem_busy(int fd, uint32_t handle)
> > {
> > @@ -109,11 +118,54 @@ static int sync_merge(int fd1, int fd2)
> > return data.fence;
> > }
> >
> > +static uint32_t __syncobj_create(int fd)
> > +{
> > + struct local_syncobj_create {
> > + uint32_t handle, flags;
> > + } arg;
> > +#define LOCAL_IOCTL_SYNCOBJ_CREATEDRM_IOWR(0xBF, struct
> > local_syncobj_create)
> > +
> > + memset(&arg, 0, sizeof(arg));
> > + ioctl(fd, LOCAL_IOCTL_SYNCOBJ_CREATE, &arg);
> > +
> > + return arg.handle;
> > +}
> > +
> > +static uint32_t syncobj_create(int fd)
> > +{
> > + uint32_t ret;
> > +
> > + igt_assert_neq((ret = __syncobj_create(fd)), 0);
> > +
> > + return ret;
> > +}
> > +
> > +#define LOCAL_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0)
> > +#define LOCAL_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
> > +struct local_syncobj_wait {
> > + __u64 handles;
> > + /* absolute timeout */
> > + __s64 timeout_nsec;
> > + __u32 count_handles;
> > + __u32 flags;
> > + __u32 first_signaled; /* only valid when not waiting all */
> > + __u32 pad;
> > +};
> > +#define LOCAL_IOCTL_SYNCOBJ_WAIT DRM_IOWR(0xC3, struct
> > local_syncobj_wait)
> > +static int __syncobj_wait(int fd, struct local_syncobj_wait *args)
> > +{
> > + int err = 0;
> > + if (drmIoctl(fd, LOCAL_IOCTL_SYNCOBJ_WAIT, args))
> > + err = -errno;
> > + return err;
> > +}
> > +
> > static int loop(unsigned ring, int reps, int ncpus, unsigned flags)
> > {
> > struct drm_i915_gem_execbuffer2 execbuf;
> > struct drm_i915_gem_exec_object2 obj[2];
> > struct drm_i915_gem_relocation_entry reloc[2];
> > + struct local_gem_exec_fence syncobj;
> > unsigned engines[16];
> > unsigned nengine;
> > uint32_t *batch;
> > @@ -126,6 +178,11 @@ static int loop(unsigned ring, int reps, int ncpus,
> > unsigned flags)
> > fd = drm_open_driver(DRIVER_INTEL);
> > gen = intel_gen(intel_get_drm_devid(fd));
> >
> > + if (flags & SYNCOBJ) {
> > + syncobj.handle = syncobj_create(fd);
> > + syncobj.flags = LOCAL_EXEC_FENCE_SIGNAL;
> > + }
> > +
> > memset(obj, 0, sizeof(obj));
> > obj[0].handle = gem_create(fd, 4096);
> > if (flags & WRITE)
> > @@ -144,6 +201,8 @@ static int loop(unsigned ring, int reps, int ncpus,
> > unsigned flags)
> > execbuf.buffer_count = 2;
> > execbuf.flags |= LOCAL_I915_EXEC_HANDLE_LUT;
> > execbuf.flags |= LOCAL_I915_EXEC_NO_RELOC;
> > + if (flags & SYNCOBJ)
> > + execbuf.flags |= LOCAL_I915_EXEC_FENCE_ARRAY;
>
> According to the comment in i915_drm.h, when this is specified, syncobj
> should be also passed in in cliprects_ptr but that's not happening?
You want -b support as well! :)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/31] drm/i915/debugfs: Create generic string tokenize function and update CRC control parsing
Thanks for the review Michal. Will update as suggested.
On 9/21/2017 8:42 PM, Michal Wajdeczko wrote:
On Tue, 19 Sep 2017 19:41:37 +0200, Sagar Arun Kamble
wrote:
Input string parsing used in CRC control parameter parsing is generic
and can be reused for other debugfs interfaces. Hence name it as
buffer_tokenize instead of tieing to display_crc. Also fix the function
desciption for CRC control parsing that was misplaced at tokenize
function.
Cc: Tomeu Vizoso
Signed-off-by: Sagar Arun Kamble
Acked-by: Radoslaw Szwichtenberg
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_pipe_crc.c | 88
+--
2 files changed, 45 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 6d7d871..4d5ffde 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3847,6 +3847,7 @@ u32 i915_gem_fence_alignment(struct
drm_i915_private *dev_priv, u32 size,
int i915_debugfs_register(struct drm_i915_private *dev_priv);
int i915_debugfs_connector_add(struct drm_connector *connector);
void intel_display_crc_init(struct drm_i915_private *dev_priv);
+int buffer_tokenize(char *buf, char *words[], int max_words);
#else
static inline int i915_debugfs_register(struct drm_i915_private
*dev_priv) {return 0;}
static inline int i915_debugfs_connector_add(struct drm_connector
*connector)
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c
b/drivers/gpu/drm/i915/intel_pipe_crc.c
index 96043a5..2e312b8 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -710,49 +710,6 @@ static int pipe_crc_set_source(struct
drm_i915_private *dev_priv,
return ret;
}
-/*
- * Parse pipe CRC command strings:
- * command: wsp* object wsp+ name wsp+ source wsp*
- * object: 'pipe'
- * name: (A | B | C)
- * source: (none | plane1 | plane2 | pf)
- * wsp: (#0x20 | #0x9 | #0xA)+
- *
- * eg.:
- * "pipe A plane1" -> Start CRC computations on plane1 of pipe A
- * "pipe A none" -> Stop CRC
- */
-static int display_crc_ctl_tokenize(char *buf, char *words[], int
max_words)
-{
- int n_words = 0;
-
- while (*buf) {
- char *end;
-
- /* skip leading white space */
- buf = skip_spaces(buf);
- if (!*buf)
- break; /* end of buffer */
-
- /* find end of word */
- for (end = buf; *end && !isspace(*end); end++)
- ;
-
- if (n_words == max_words) {
- DRM_DEBUG_DRIVER("too many words, allowed <= %d\n",
- max_words);
- return -EINVAL; /* ran out of words[] before bytes */
- }
-
- if (*end)
- *end++ = '\0';
- words[n_words++] = buf;
- buf = end;
- }
-
- return n_words;
-}
-
enum intel_pipe_crc_object {
PIPE_CRC_OBJECT_PIPE,
};
@@ -806,6 +763,49 @@ static int display_crc_ctl_parse_pipe(const char
*buf, enum pipe *pipe)
return -EINVAL;
}
+int buffer_tokenize(char *buf, char *words[], int max_words)
+{
+ int n_words = 0;
+
+ while (*buf) {
+ char *end;
+
+ /* skip leading white space */
+ buf = skip_spaces(buf);
+ if (!*buf)
+ break; /* end of buffer */
+
+ /* find end of word */
+ for (end = buf; *end && !isspace(*end); end++)
+ ;
+
+ if (n_words == max_words) {
+ DRM_DEBUG_DRIVER("too many words, allowed <= %d\n",
+ max_words);
+ return -EINVAL; /* ran out of words[] before bytes */
+ }
+
+ if (*end)
+ *end++ = '\0';
+ words[n_words++] = buf;
+ buf = end;
+ }
+
+ return n_words;
+}
You should move this function to i915_debugfs.c
+
+/*
+ * Parse pipe CRC command strings:
+ * command: wsp* object wsp+ name wsp+ source wsp*
+ * object: 'pipe'
+ * name: (A | B | C)
+ * source: (none | plane1 | plane2 | pf)
+ * wsp: (#0x20 | #0x9 | #0xA)+
+ *
+ * eg.:
+ * "pipe A plane1" -> Start CRC computations on plane1 of pipe A
+ * "pipe A none" -> Stop CRC
+ */
static int display_crc_ctl_parse(struct drm_i915_private *dev_priv,
char *buf, size_t len)
{
@@ -816,7 +816,7 @@ static int display_crc_ctl_parse(struct
drm_i915_private *dev_priv,
enum intel_pipe_crc_object object;
enum intel_pipe_crc_source source;
- n_words = display_crc_ctl_tokenize(buf, words, N_WORDS);
+ n_words = buffer_tokenize(buf, words, N_WORDS);
if (n_words != N_WORDS) {
DRM_DEBUG_DRIVER("tokenize failed, a command is %d words\n",
N_WORDS);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/31] drm/i915: Rename intel_enable_rc6 to intel_rc6_enabled
Thank you Radek, Ewelina for the reviews.
On 9/26/2017 1:11 PM, Ewelina Musial wrote:
On Tue, Sep 19, 2017 at 11:11:44PM +0530, Sagar Arun Kamble wrote:
This function gives the status of RC6, whether disabled or if
enabled then which state. intel_enable_rc6 will be used for
enabling RC6 in the next patch.
Cc: Chris Wilson
Cc: Imre Deak
Signed-off-by: Sagar Arun Kamble
Reviewed-by: Ewelina Musial
- Ewelina
---
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/i915/i915_sysfs.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h | 2 +-
drivers/gpu/drm/i915/intel_guc.c | 3 ++-
drivers/gpu/drm/i915/intel_pm.c | 12 ++--
5 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6cc1162..a6dbad3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2486,7 +2486,7 @@ static int intel_runtime_suspend(struct device *kdev)
struct drm_i915_private *dev_priv = to_i915(dev);
int ret;
- if (WARN_ON_ONCE(!(dev_priv->pm.rps.enabled && intel_enable_rc6(
+ if (WARN_ON_ONCE(!(dev_priv->pm.rps.enabled && intel_rc6_enabled(
return -ENODEV;
if (WARN_ON_ONCE(!HAS_RUNTIME_PM(dev_priv)))
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
b/drivers/gpu/drm/i915/i915_sysfs.c
index c16e907..8add849 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -49,7 +49,7 @@ static u32 calc_residency(struct drm_i915_private *dev_priv,
static ssize_t
show_rc6_mask(struct device *kdev, struct device_attribute *attr, char *buf)
{
- return snprintf(buf, PAGE_SIZE, "%x\n", intel_enable_rc6());
+ return snprintf(buf, PAGE_SIZE, "%x\n", intel_rc6_enabled());
}
static ssize_t
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f7db720..0cf04eb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1900,7 +1900,7 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc
*intel_crtc,
struct intel_crtc_state *cstate);
void intel_init_ipc(struct drm_i915_private *dev_priv);
void intel_enable_ipc(struct drm_i915_private *dev_priv);
-static inline int intel_enable_rc6(void)
+static inline int intel_rc6_enabled(void)
{
return i915.enable_rc6;
}
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index c731cff..f4dc708 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -128,7 +128,8 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
action[0] = INTEL_GUC_ACTION_SAMPLE_FORCEWAKE;
/* WaRsDisableCoarsePowerGating:skl,bxt */
- if (!intel_enable_rc6() || NEEDS_WaRsDisableCoarsePowerGating(dev_priv))
+ if (!intel_rc6_enabled() ||
+ NEEDS_WaRsDisableCoarsePowerGating(dev_priv))
action[1] = 0;
else
/* bit 0 and 1 are for Render and Media domain separately */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b89677a..b83751e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6606,7 +6606,7 @@ static void gen9_enable_rc6(struct drm_i915_private
*dev_priv)
I915_WRITE(GEN9_RENDER_PG_IDLE_HYSTERESIS, 25);
/* 3a: Enable RC6 */
- if (intel_enable_rc6() & INTEL_RC6_ENABLE)
+ if (intel_rc6_enabled() & INTEL_RC6_ENABLE)
rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
DRM_INFO("RC6 %s\n", onoff(rc6_mask & GEN6_RC_CTL_RC6_ENABLE));
I915_WRITE(GEN6_RC6_THRESHOLD, 37500); /* 37.5/125ms per EI */
@@ -6655,7 +6655,7 @@ static void gen8_enable_rc6(struct drm_i915_private
*dev_priv)
I915_WRITE(GEN6_RC6_THRESHOLD, 5); /* 50/125ms per EI */
/* 3: Enable RC6 */
- if (intel_enable_rc6() & INTEL_RC6_ENABLE)
+ if (intel_rc6_enabled() & INTEL_RC6_ENABLE)
rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
intel_print_rc6_info(dev_priv, rc6_mask);
I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
@@ -6749,7 +6749,7 @@ static void gen6_enable_rc6(struct drm_i915_private
*dev_priv)
I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */
/* Check if we are enabling RC6 */
- rc6_mode = intel_enable_rc6();
+ rc6_mode = intel_rc6_enabled();
if (rc6_mode & INTEL_RC6_ENABLE)
rc6_mask |= GEN6_RC_CTL_RC6_ENABLE;
@@ -7251,7 +7251,7 @@ static void cherryview_enable_rc6(struct drm_i915_private *dev_priv)
pcbr = I915_READ(VLV_PCBR);
/* 3: Enable RC6 */
- if ((intel_enable_rc6() & INTEL_RC6_ENABLE) &&
+ if ((intel_rc6_enabled() & INTEL_RC6_ENABLE) &&
(pcbr >> VLV_PCBR_ADDR_SHIFT))
rc6_mode = GEN7_RC_CTL_TO_MODE;
@@ -7345,7 +7345,7 @@ static void valleyview_enable_rc6(struct drm_i915_private *dev_priv)
Re: [Intel-gfx] [PATCH v2 08/11] drm/i915/execlists: Keep request->priority for its lifetime
On Wed, Sep 27, 2017 at 04:44:37PM +, Chris Wilson wrote:
> With preemption, we will want to "unsubmit" a request, taking it back
> from the hw and returning it to the priority sorted execution list. In
> order to know where to insert it into that list, we need to remember
> its adjust priority (which may change even as it was being executed).
This has a (positive IMO) side effect that should be mentioned here.
Starting from:
drm/i915/execlists: Unwind incomplete requests on resets
We were using the fact, that were overwriting the priority on submit to HW, to
resubmit the same requests on GPU reset.
Since now we keep track of the priority, we're going to 'resubmit' with correct
priority, making reset another "preemption" point. A quicker and more reliable
preemption! Although a bit sad for the requests that were "preempted" this way
:)
Please, add the same behavior to GuC submission path.
With that, and expanded commit msg:
Reviewed-by: Michał Winiarski
-Michał
>
> Signed-off-by: Chris Wilson
> Cc: Michal Winiarski
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 14 ++
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index e32109265eb9..7ac92a77aea8 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -585,8 +585,6 @@ static void execlists_dequeue(struct intel_engine_cs
> *engine)
> }
>
> INIT_LIST_HEAD(&rq->priotree.link);
> - rq->priotree.priority = INT_MAX;
> -
> __i915_gem_request_submit(rq);
> trace_i915_gem_request_in(rq, port_index(port,
> execlists));
> last = rq;
> @@ -794,6 +792,7 @@ static void intel_lrc_irq_handler(unsigned long data)
> execlists_context_status_change(rq,
> INTEL_CONTEXT_SCHEDULE_OUT);
>
> trace_i915_gem_request_out(rq);
> + rq->priotree.priority = INT_MAX;
> i915_gem_request_put(rq);
>
> execlists_port_complete(execlists, port);
> @@ -846,11 +845,15 @@ static void execlists_submit_request(struct
> drm_i915_gem_request *request)
> spin_unlock_irqrestore(&engine->timeline->lock, flags);
> }
>
> +static struct drm_i915_gem_request *pt_to_request(struct i915_priotree *pt)
> +{
> + return container_of(pt, struct drm_i915_gem_request, priotree);
> +}
> +
> static struct intel_engine_cs *
> pt_lock_engine(struct i915_priotree *pt, struct intel_engine_cs *locked)
> {
> - struct intel_engine_cs *engine =
> - container_of(pt, struct drm_i915_gem_request, priotree)->engine;
> + struct intel_engine_cs *engine = pt_to_request(pt)->engine;
>
> GEM_BUG_ON(!locked);
>
> @@ -904,6 +907,9 @@ static void execlists_schedule(struct
> drm_i915_gem_request *request, int prio)
>* engines.
>*/
> list_for_each_entry(p, &pt->signalers_list, signal_link) {
> + if
> (i915_gem_request_completed(pt_to_request(p->signaler)))
> + continue;
> +
> GEM_BUG_ON(p->signaler->priority < pt->priority);
> if (prio > READ_ONCE(p->signaler->priority))
> list_move_tail(&p->dfs_link, &dfs);
> --
> 2.14.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt 1/3] benchmark/gem_busy: Compare polling with syncobj_wait
On 28/09/2017 10:07, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2017-09-28 07:53:56)
On 25/09/2017 21:26, Chris Wilson wrote:
Signed-off-by: Chris Wilson
---
benchmarks/gem_busy.c | 73
++-
1 file changed, 72 insertions(+), 1 deletion(-)
diff --git a/benchmarks/gem_busy.c b/benchmarks/gem_busy.c
index f050454b..9649ea02 100644
--- a/benchmarks/gem_busy.c
+++ b/benchmarks/gem_busy.c
@@ -58,6 +58,15 @@
#define DMABUF 0x4
#define WAIT 0x8
#define SYNC 0x10
+#define SYNCOBJ 0x20
+
+#define LOCAL_I915_EXEC_FENCE_ARRAY (1 << 19)
+struct local_gem_exec_fence {
+ uint32_t handle;
+ uint32_t flags;
+#define LOCAL_EXEC_FENCE_WAIT (1 << 0)
+#define LOCAL_EXEC_FENCE_SIGNAL (1 << 1)
+};
static void gem_busy(int fd, uint32_t handle)
{
@@ -109,11 +118,54 @@ static int sync_merge(int fd1, int fd2)
return data.fence;
}
+static uint32_t __syncobj_create(int fd)
+{
+ struct local_syncobj_create {
+ uint32_t handle, flags;
+ } arg;
+#define LOCAL_IOCTL_SYNCOBJ_CREATEDRM_IOWR(0xBF, struct
local_syncobj_create)
+
+ memset(&arg, 0, sizeof(arg));
+ ioctl(fd, LOCAL_IOCTL_SYNCOBJ_CREATE, &arg);
+
+ return arg.handle;
+}
+
+static uint32_t syncobj_create(int fd)
+{
+ uint32_t ret;
+
+ igt_assert_neq((ret = __syncobj_create(fd)), 0);
+
+ return ret;
+}
+
+#define LOCAL_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0)
+#define LOCAL_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
+struct local_syncobj_wait {
+ __u64 handles;
+ /* absolute timeout */
+ __s64 timeout_nsec;
+ __u32 count_handles;
+ __u32 flags;
+ __u32 first_signaled; /* only valid when not waiting all */
+ __u32 pad;
+};
+#define LOCAL_IOCTL_SYNCOBJ_WAIT DRM_IOWR(0xC3, struct local_syncobj_wait)
+static int __syncobj_wait(int fd, struct local_syncobj_wait *args)
+{
+ int err = 0;
+ if (drmIoctl(fd, LOCAL_IOCTL_SYNCOBJ_WAIT, args))
+ err = -errno;
+ return err;
+}
+
static int loop(unsigned ring, int reps, int ncpus, unsigned flags)
{
struct drm_i915_gem_execbuffer2 execbuf;
struct drm_i915_gem_exec_object2 obj[2];
struct drm_i915_gem_relocation_entry reloc[2];
+ struct local_gem_exec_fence syncobj;
unsigned engines[16];
unsigned nengine;
uint32_t *batch;
@@ -126,6 +178,11 @@ static int loop(unsigned ring, int reps, int ncpus,
unsigned flags)
fd = drm_open_driver(DRIVER_INTEL);
gen = intel_gen(intel_get_drm_devid(fd));
+ if (flags & SYNCOBJ) {
+ syncobj.handle = syncobj_create(fd);
+ syncobj.flags = LOCAL_EXEC_FENCE_SIGNAL;
+ }
+
memset(obj, 0, sizeof(obj));
obj[0].handle = gem_create(fd, 4096);
if (flags & WRITE)
@@ -144,6 +201,8 @@ static int loop(unsigned ring, int reps, int ncpus,
unsigned flags)
execbuf.buffer_count = 2;
execbuf.flags |= LOCAL_I915_EXEC_HANDLE_LUT;
execbuf.flags |= LOCAL_I915_EXEC_NO_RELOC;
+ if (flags & SYNCOBJ)
+ execbuf.flags |= LOCAL_I915_EXEC_FENCE_ARRAY;
According to the comment in i915_drm.h, when this is specified, syncobj
should be also passed in in cliprects_ptr but that's not happening?
You want -b support as well! :)
I just failed to figure out where is the connection between syncobj
(local var) and execbuf. Flag is set, so what happens next? Execbuf
fails since cliprects_ptr is not set?
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 15/31] drm/i915/slpc: Sanitize GuC version
On 9/21/2017 6:22 PM, Michal Wajdeczko wrote:
On Tue, 19 Sep 2017 19:41:51 +0200, Sagar Arun Kamble
wrote:
From: Tom O'Rourke
The SLPC interface is dependent on GuC version.
Only GuC versions known to be compatible are supported here.
SLPC with GuC firmware v9 is supported with this series.
v1: Updated with modified sanitize_slpc_option in earlier patch.
v2-v3: Rebase.
v4: Updated support for GuC firmware v9.
v5: Commit subject updated.
v6: Commit subject and message update. Add support condition as >=v9.
v7: Sanitizing GuC version in intel_uc_init_fw for SLPC compatibility.
Added info. print for needed version and pointer to 01.org.
v8: s/FIRMWARE_URL/I915_FIRMWARE_URL, Macro added for SLPC required GuC
Major version and rearrangement for sanitization. (MichalW, Joonas)
v9: Checking major_ver_found to sanitize SLPC option enable_slpc post
fetching the firmware as with Custom firmware loaded through
guc_firmware_path parameter, major_ver_wanted are cleared. (Lukasz)
v10: Moved the I915_FIRMWARE_URL macro to intel_uc_common.h.
Signed-off-by: Tom O'Rourke
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/intel_csr.c | 15 ++-
drivers/gpu/drm/i915/intel_guc.h | 1 +
drivers/gpu/drm/i915/intel_guc_loader.c | 15 +++
drivers/gpu/drm/i915/intel_uc.c | 1 +
drivers/gpu/drm/i915/intel_uc_common.h | 2 ++
5 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_csr.c
b/drivers/gpu/drm/i915/intel_csr.c
index 965988f..56c56f5 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -52,11 +52,6 @@
MODULE_FIRMWARE(I915_CSR_BXT);
#define BXT_CSR_VERSION_REQUIRED CSR_VERSION(1, 7)
-#define FIRMWARE_URL "https://01.org/linuxgraphics/downloads/firmware";
-
-
-
-
#define CSR_MAX_FW_SIZE 0x2FFF
#define CSR_DEFAULT_FW_OFFSET 0x
@@ -309,11 +304,12 @@ static uint32_t *parse_csr_fw(struct
drm_i915_private *dev_priv,
if (csr->version != required_version) {
DRM_INFO("Refusing to load DMC firmware v%u.%u,"
- " please use v%u.%u [" FIRMWARE_URL "].\n",
+ " please use v%u.%u [%s].\n",
CSR_VERSION_MAJOR(csr->version),
CSR_VERSION_MINOR(csr->version),
CSR_VERSION_MAJOR(required_version),
- CSR_VERSION_MINOR(required_version));
+ CSR_VERSION_MINOR(required_version),
+ I915_FIRMWARE_URL);
Hmm, I'm not sure that including URL here is useful.
URL will be repeated in csr_load_work_fn() where we can explain
its purpose ;)
Sure. Will remove from here.
return NULL;
}
@@ -420,8 +416,9 @@ static void csr_load_work_fn(struct work_struct
*work)
} else {
dev_notice(dev_priv->drm.dev,
"Failed to load DMC firmware"
- " [" FIRMWARE_URL "],"
- " disabling runtime power management.\n");
+ " [%s],"
+ " disabling runtime power management.\n",
+ I915_FIRMWARE_URL);
Maybe more user friendly message should looks like:
"Failed to load DMC firmware, disabling runtime power management."
"DMC firmware can be downloaded from
https://01.org/linuxgraphics/downloads/firmware";
Yes. Will update like this.
}
release_firmware(fw);
diff --git a/drivers/gpu/drm/i915/intel_guc.h
b/drivers/gpu/drm/i915/intel_guc.h
index a894991..3821bf2 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -159,6 +159,7 @@ static inline void intel_guc_init_early(struct
intel_guc *guc)
int intel_guc_select_fw(struct intel_guc *guc);
int intel_guc_init_hw(struct intel_guc *guc);
u32 intel_guc_wopcm_size(struct intel_guc *guc);
+void intel_guc_fetch_sanitize_options(struct intel_guc *guc);
/* i915_guc_submission.c */
int i915_guc_submission_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 6ee7c16..4550620 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -64,6 +64,8 @@
#define GLK_FW_MAJOR 10
#define GLK_FW_MINOR 56
+#define I915_SLPC_REQUIRED_GUC_MAJOR 9
+
#define GUC_FW_PATH(platform, major, minor) \
"i915/" __stringify(platform) "_guc_ver" __stringify(major)
"_" __stringify(minor) ".bin"
@@ -418,3 +420,16 @@ int intel_guc_select_fw(struct intel_guc *guc)
return 0;
}
+
+void intel_guc_fetch_sanitize_options(struct intel_guc *guc)
+{
+ if (guc->fw.major_ver_found <
+ I915_SLPC_REQUIRED_GUC_MAJOR) {
Generally we do not allow Guc fw major "found" to be different than
"wanted"
so this check could be also done against "wanted".
With Custom firmware loaded through guc_firmware_path parameter,
major_ver_wanted is cleared. And
then check fails. Lukasz verified this.
+ DRM_INFO("SLPC not supported with GuC firmware"
+
Re: [Intel-gfx] [PATCH igt] lib: Capture the error state on an unexpected hang
Quoting Chris Wilson (2017-09-12 21:10:25)
> Dump debugfs/i915_error_state to the debug channel if we detect an ERROR
> uevent. This poses a few problems, not least that it is the auxiliary
> process doing the dumping (so the output may be interleaved with the
> test, but considering a hang occurred it is likely the test is blocked)
> and the average error state is around 60k, which may prove unwieldy!
>
> On the other hand, it may prove invaluable in debugging those impossible
> to reproduce bugs.
>
> Signed-off-by: Chris Wilson
Penguin???
> ---
> lib/igt_aux.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 65f832ab..ae076aaa 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -395,7 +395,7 @@ void igt_stop_shrink_helper(void)
>
> static struct igt_helper_process hang_detector;
> static void __attribute__((noreturn))
> -hang_detector_process(pid_t pid, dev_t rdev)
> +hang_detector_process(int fd, pid_t pid, dev_t rdev)
> {
> struct udev_monitor *mon =
> udev_monitor_new_from_netlink(udev_new(), "kernel");
> @@ -429,8 +429,10 @@ hang_detector_process(pid_t pid, dev_t rdev)
> const char *str;
>
> str = udev_device_get_property_value(dev, "ERROR");
> - if (str && atoi(str) == 1)
> + if (str && atoi(str) == 1) {
> + igt_debugfs_dump(fd, "i915_error_state");
> kill(pid, SIGIO);
> + }
> }
>
> udev_device_unref(dev);
> @@ -462,7 +464,7 @@ void igt_fork_hang_detector(int fd)
>
> signal(SIGIO, sig_abort);
> igt_fork_helper(&hang_detector)
> - hang_detector_process(getppid(), st.st_rdev);
> + hang_detector_process(fd, getppid(), st.st_rdev);
> }
>
> void igt_stop_hang_detector(void)
> --
> 2.14.1
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 16/31] drm/i915/slpc: Lay out SLPC init/enable/disable/cleanup helpers
On 9/21/2017 6:30 PM, Michal Wajdeczko wrote:
On Tue, 19 Sep 2017 19:41:52 +0200, Sagar Arun Kamble
wrote:
SLPC operates based on parameters setup in shared data between
i915 and GuC SLPC. This is to be created/initialized in intel_slpc_init.
From there onwards i915 can control the SLPC operations by Enabling,
Disabling complete SLPC or changing SLPC parameters. During cleanup
SLPC shared data has to be freed.
With this patch on platforms with SLPC support we call intel_slpc_*()
functions from GuC setup functions and do not use Host rps functions.
With SLPC, intel_enable_gt_powersave will only handle RC6. In the later
patch intel_init_gt_powersave will check if SLPC has started running
through shared data and update initial state that i915 needs like
frequency limits if needed.
v1: Return void instead of ignored error code (Paulo)
enable/disable RC6 in SLPC flows (Sagar)
replace HAS_SLPC() use with intel_slpc_enabled()
or intel_slpc_active() (Paulo)
Fix for renaming gen9_disable_rps to gen9_disable_rc6 in
"drm/i915/bxt: Explicitly clear the Turbo control register"
Defer RC6 and SLPC enabling to intel_gen6_powersave_work. (Sagar)
Performance drop with SLPC was happening as ring frequency table
was not programmed when SLPC was enabled. This patch programs ring
frequency table with SLPC. Initial reset of SLPC is based on kernel
parameter as planning to add slpc state in intel_slpc_active.
Cleanup
is also based on kernel parameter as SLPC gets disabled in
disable/suspend.(Sagar)
v2: Usage of INTEL_GEN instead of INTEL_INFO->gen (David)
Checkpatch update.
v3: Rebase
v4: Removed reset functions to comply with *_gt_powersave routines.
(Sagar)
v5: Removed intel_slpc_active. Relying on slpc.active for control flows
that are based on SLPC active status in GuC. State setup/cleanup
needed
for SLPC is handled using kernel parameter i915.enable_slpc.
Moved SLPC
init and enabling to GuC enable path as SLPC in GuC can start
doing the
setup post GuC init. Commit message update. (Sagar)
v6: Rearranged function definitions.
v7: Makefile rearrangement. Reducing usage of i915.enable_slpc and
relying
mostly on rps.rps_enabled to bypass Host RPS flows. Commit message
update.
v8: Changed parameters for SLPC functions to struct intel_slpc*.
v9: Reinstated intel_slpc_active and intel_slpc_enabled as they are more
meaningful.
v10: Rebase changes due to creation of intel_guc.h. Updates in
intel_guc_cleanup w.r.t slpc cleanup.
Signed-off-by: Tom O'Rourke
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.h | 12 ++
drivers/gpu/drm/i915/intel_guc.c | 3 +++
drivers/gpu/drm/i915/intel_guc.h | 3 +++
Hmm, this looks like cross dependency on other pending series
Yes. Will make this independent series.
drivers/gpu/drm/i915/intel_pm.c | 19 +++-
drivers/gpu/drm/i915/intel_slpc.c | 42
It looks that SLPC is tight with Guc so maybe better names would be:
intel_guc_slpc.c and struct intel_guc_slpc
ie. the same pattern as with Guc log.
Sure. Will update.
++
drivers/gpu/drm/i915/intel_slpc.h | 47
+++
drivers/gpu/drm/i915/intel_uc.c | 20 +
8 files changed, 142 insertions(+), 5 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_slpc.c
create mode 100644 drivers/gpu/drm/i915/intel_slpc.h
diff --git a/drivers/gpu/drm/i915/Makefile
b/drivers/gpu/drm/i915/Makefile
index d1327f6..62bf4f6e 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -64,6 +64,7 @@ i915-y += intel_uc.o \
intel_guc_log.o \
intel_guc_loader.o \
intel_huc.o \
+ intel_slpc.o \
i915_guc_submission.o
# autogenerated null render state
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 428cb1c..af633c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2748,6 +2748,18 @@ static inline struct drm_i915_private
*guc_to_i915(struct intel_guc *guc)
return container_of(guc, struct drm_i915_private, guc);
}
+static inline struct intel_guc *slpc_to_guc(struct intel_slpc *slpc)
+{
+ return container_of(slpc, struct intel_guc, slpc);
+}
+
+static inline struct drm_i915_private *slpc_to_i915(struct
intel_slpc *slpc)
+{
+ struct intel_guc *guc = slpc_to_guc(slpc);
+
+ return guc_to_i915(guc);
+}
+
static inline struct drm_i915_private *huc_to_i915(struct intel_huc
*huc)
{
return container_of(huc, struct drm_i915_private, huc);
diff --git a/drivers/gpu/drm/i915/intel_guc.c
b/drivers/gpu/drm/i915/intel_guc.c
index f4dc708..a92c7e8 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -226,6 +226,9 @@ void intel_guc_cleanup(struct intel_guc *guc)
if (i915.enable_guc_submission)
Re: [Intel-gfx] [PATCH v2 08/11] drm/i915/execlists: Keep request->priority for its lifetime
Quoting Michał Winiarski (2017-09-28 10:14:47) > On Wed, Sep 27, 2017 at 04:44:37PM +, Chris Wilson wrote: > > With preemption, we will want to "unsubmit" a request, taking it back > > from the hw and returning it to the priority sorted execution list. In > > order to know where to insert it into that list, we need to remember > > its adjust priority (which may change even as it was being executed). > > This has a (positive IMO) side effect that should be mentioned here. > > Starting from: > drm/i915/execlists: Unwind incomplete requests on resets > > We were using the fact, that were overwriting the priority on submit to HW, to > resubmit the same requests on GPU reset. > Since now we keep track of the priority, we're going to 'resubmit' with > correct > priority, making reset another "preemption" point. A quicker and more reliable > preemption! Although a bit sad for the requests that were "preempted" this > way :) Bah. I am not keen that reset acts as an accidental preemption point. Hence why I kept the behaviour of resubmitting exactly was on the hw at the time of reset to avoid the notion of preemption happening before we were expecting any. > Please, add the same behavior to GuC submission path. Why? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 08/11] drm/i915/execlists: Keep request->priority for its lifetime
Quoting Chris Wilson (2017-09-28 10:31:00) > Quoting Michał Winiarski (2017-09-28 10:14:47) > > Please, add the same behavior to GuC submission path. > > Why? To expand on this, there is a large negative cost in removing the dfs optimisation. What benefit does it bring to the execution when there is no preemption? Afaict, none. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnl: WaForceContextSaveRestoreNonCoherent
Stealing the thread for another gem_workarounds conundrum. After a reset, we lose the RING_FORCE_TO_NONPRIV registers. If they where in the context image as we presumed, the values would be retained and they can be read back from before reset, so it's not the case of write-only register! So are they the mythical powercontext but require an LRI after reset to restore the settings for all logical contexts? Or can we make those into regular MMIO? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 18/31] drm/i915/slpc: Add SLPC communication interfaces
On 9/21/2017 6:44 PM, Michal Wajdeczko wrote:
On Tue, 19 Sep 2017 19:41:54 +0200, Sagar Arun Kamble
wrote:
Communication with SLPC is via Host to GuC interrupt through
shared data and parameters. This patch defines the structure of
shared data, parameters, data structure to be passed as input and
received as output from SLPC. This patch also defines the events
to be sent as input and status values output by GuC on processing
SLPC events. SLPC shared data has details of SKU type, Slice count,
IA Perf MSR values, SLPC state, Power source/plan, SLPC tasks status.
Parameters allow overriding task control, frequency range etc.
v1: fix whitespace (Sagar)
v2-v3: Rebase.
v4: Updated with GuC firmware v9.
v5: Added definition of input and output data structures for SLPC
events. Updated commit message.
v6: Removed definition of host2guc_slpc. Will be added in the next
patch that uses it. Commit subject update. Rebase.
v7: Added definition of SLPC_RESET_FLAG_TDR_OCCURRED to be sent
throgh SLPC reset in case of engine reset. Moved all Host/SLPC
interfaces from later patches to this patch. Commit message update.
v8: Updated value of SLPC_RESET_FLAG_TDR_OCCURRED.
Signed-off-by: Tom O'Rourke
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/intel_slpc.c | 39 +++
drivers/gpu/drm/i915/intel_slpc.h | 207
++
2 files changed, 246 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_slpc.c
b/drivers/gpu/drm/i915/intel_slpc.c
index 06abda5..a3db63c 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -25,6 +25,45 @@
#include "i915_drv.h"
#include "intel_uc.h"
+struct slpc_param slpc_paramlist[SLPC_MAX_PARAM] = {
+ {SLPC_PARAM_TASK_ENABLE_GTPERF, "Enable task GTPERF"},
+ {SLPC_PARAM_TASK_DISABLE_GTPERF, "Disable task GTPERF"},
+ {SLPC_PARAM_TASK_ENABLE_BALANCER, "Enable task BALANCER"},
+ {SLPC_PARAM_TASK_DISABLE_BALANCER, "Disable task BALANCER"},
+ {SLPC_PARAM_TASK_ENABLE_DCC, "Enable task DCC"},
+ {SLPC_PARAM_TASK_DISABLE_DCC, "Disable task DCC"},
+ {SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
+ "Minimum GT frequency request for unslice"},
+ {SLPC_PARAM_GLOBAL_MAX_GT_UNSLICE_FREQ_MHZ,
+ "Maximum GT frequency request for unslice"},
+ {SLPC_PARAM_GLOBAL_MIN_GT_SLICE_FREQ_MHZ,
+ "Minimum GT frequency request for slice"},
+ {SLPC_PARAM_GLOBAL_MAX_GT_SLICE_FREQ_MHZ,
+ "Maximum GT frequency request for slice"},
+ {SLPC_PARAM_GTPERF_THRESHOLD_MAX_FPS,
+ "If non-zero, algorithm will slow down "
+ "frame-based applications to this frame-rate"},
+ {SLPC_PARAM_GLOBAL_DISABLE_GT_FREQ_MANAGEMENT,
+ "Lock GT frequency request to RPe"},
+ {SLPC_PARAM_GTPERF_ENABLE_FRAMERATE_STALLING,
+ "Set to TRUE to enable slowing framerate"},
+ {SLPC_PARAM_GLOBAL_DISABLE_RC6_MODE_CHANGE,
+ "Prevent from changing the RC mode"},
+ {SLPC_PARAM_GLOBAL_OC_UNSLICE_FREQ_MHZ,
+ "Override fused value of unslice RP0"},
+ {SLPC_PARAM_GLOBAL_OC_SLICE_FREQ_MHZ,
+ "Override fused value of slice RP0"},
+ {SLPC_PARAM_GLOBAL_ENABLE_IA_GT_BALANCING,
+ "TRUE means enable Intelligent Bias Control"},
+ {SLPC_PARAM_GLOBAL_ENABLE_ADAPTIVE_BURST_TURBO,
+ "TRUE = enable eval mode when transitioning "
+ "from idle to active."},
+ {SLPC_PARAM_GLOBAL_ENABLE_EVAL_MODE,
+ "FALSE = disable eval mode completely"},
+ {SLPC_PARAM_GLOBAL_ENABLE_BALANCER_IN_NON_GAMING_MODE,
+ "Enable IBC when non-Gaming Mode is enabled"}
+};
+
void intel_slpc_init(struct intel_slpc *slpc)
{
}
diff --git a/drivers/gpu/drm/i915/intel_slpc.h
b/drivers/gpu/drm/i915/intel_slpc.h
index f68671f..ac4cb65 100644
--- a/drivers/gpu/drm/i915/intel_slpc.h
+++ b/drivers/gpu/drm/i915/intel_slpc.h
@@ -38,6 +38,213 @@ static inline bool intel_slpc_active(struct
intel_slpc *slpc)
return slpc->active;
}
+enum slpc_status {
+ SLPC_STATUS_OK = 0,
+ SLPC_STATUS_ERROR = 1,
+ SLPC_STATUS_ILLEGAL_COMMAND = 2,
+ SLPC_STATUS_INVALID_ARGS = 3,
+ SLPC_STATUS_INVALID_PARAMS = 4,
+ SLPC_STATUS_INVALID_DATA = 5,
+ SLPC_STATUS_OUT_OF_RANGE = 6,
+ SLPC_STATUS_NOT_SUPPORTED = 7,
+ SLPC_STATUS_NOT_IMPLEMENTED = 8,
+ SLPC_STATUS_NO_DATA = 9,
+ SLPC_STATUS_EVENT_NOT_REGISTERED = 10,
+ SLPC_STATUS_REGISTER_LOCKED = 11,
+ SLPC_STATUS_TEMPORARILY_UNAVAILABLE = 12,
+ SLPC_STATUS_VALUE_ALREADY_SET = 13,
+ SLPC_STATUS_VALUE_ALREADY_UNSET = 14,
+ SLPC_STATUS_VALUE_NOT_CHANGED = 15,
+ SLPC_STATUS_MEMIO_ERROR = 16,
+ SLPC_STATUS_EVENT_QUEUED_REQ_DPC = 17,
+ SLPC_STATUS_EVENT_QUEUED_NOREQ_DPC = 18,
+ SLPC_STATUS_NO_EVENT_QUEUED = 19,
+ SLPC_STATUS_OUT_OF_SPACE = 20,
+ SLPC_STATUS_TIMEOUT = 21,
+ SLPC_STATUS_NO_LOCK = 22,
+};
I'm always wondering why everyone wants
Re: [Intel-gfx] [PATCH igt 1/3] benchmark/gem_busy: Compare polling with syncobj_wait
Quoting Tvrtko Ursulin (2017-09-28 10:16:58)
>
> On 28/09/2017 10:07, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2017-09-28 07:53:56)
> >>
> >> On 25/09/2017 21:26, Chris Wilson wrote:
> >>> Signed-off-by: Chris Wilson
> >>> ---
> >>>benchmarks/gem_busy.c | 73
> >>> ++-
> >>>1 file changed, 72 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/benchmarks/gem_busy.c b/benchmarks/gem_busy.c
> >>> index f050454b..9649ea02 100644
> >>> --- a/benchmarks/gem_busy.c
> >>> +++ b/benchmarks/gem_busy.c
> >>> @@ -58,6 +58,15 @@
> >>>#define DMABUF 0x4
> >>>#define WAIT 0x8
> >>>#define SYNC 0x10
> >>> +#define SYNCOBJ 0x20
> >>> +
> >>> +#define LOCAL_I915_EXEC_FENCE_ARRAY (1 << 19)
> >>> +struct local_gem_exec_fence {
> >>> + uint32_t handle;
> >>> + uint32_t flags;
> >>> +#define LOCAL_EXEC_FENCE_WAIT (1 << 0)
> >>> +#define LOCAL_EXEC_FENCE_SIGNAL (1 << 1)
> >>> +};
> >>>
> >>>static void gem_busy(int fd, uint32_t handle)
> >>>{
> >>> @@ -109,11 +118,54 @@ static int sync_merge(int fd1, int fd2)
> >>>return data.fence;
> >>>}
> >>>
> >>> +static uint32_t __syncobj_create(int fd)
> >>> +{
> >>> + struct local_syncobj_create {
> >>> + uint32_t handle, flags;
> >>> + } arg;
> >>> +#define LOCAL_IOCTL_SYNCOBJ_CREATEDRM_IOWR(0xBF, struct
> >>> local_syncobj_create)
> >>> +
> >>> + memset(&arg, 0, sizeof(arg));
> >>> + ioctl(fd, LOCAL_IOCTL_SYNCOBJ_CREATE, &arg);
> >>> +
> >>> + return arg.handle;
> >>> +}
> >>> +
> >>> +static uint32_t syncobj_create(int fd)
> >>> +{
> >>> + uint32_t ret;
> >>> +
> >>> + igt_assert_neq((ret = __syncobj_create(fd)), 0);
> >>> +
> >>> + return ret;
> >>> +}
> >>> +
> >>> +#define LOCAL_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0)
> >>> +#define LOCAL_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1)
> >>> +struct local_syncobj_wait {
> >>> + __u64 handles;
> >>> + /* absolute timeout */
> >>> + __s64 timeout_nsec;
> >>> + __u32 count_handles;
> >>> + __u32 flags;
> >>> + __u32 first_signaled; /* only valid when not waiting all */
> >>> + __u32 pad;
> >>> +};
> >>> +#define LOCAL_IOCTL_SYNCOBJ_WAIT DRM_IOWR(0xC3, struct
> >>> local_syncobj_wait)
> >>> +static int __syncobj_wait(int fd, struct local_syncobj_wait *args)
> >>> +{
> >>> + int err = 0;
> >>> + if (drmIoctl(fd, LOCAL_IOCTL_SYNCOBJ_WAIT, args))
> >>> + err = -errno;
> >>> + return err;
> >>> +}
> >>> +
> >>>static int loop(unsigned ring, int reps, int ncpus, unsigned flags)
> >>>{
> >>>struct drm_i915_gem_execbuffer2 execbuf;
> >>>struct drm_i915_gem_exec_object2 obj[2];
> >>>struct drm_i915_gem_relocation_entry reloc[2];
> >>> + struct local_gem_exec_fence syncobj;
> >>>unsigned engines[16];
> >>>unsigned nengine;
> >>>uint32_t *batch;
> >>> @@ -126,6 +178,11 @@ static int loop(unsigned ring, int reps, int ncpus,
> >>> unsigned flags)
> >>>fd = drm_open_driver(DRIVER_INTEL);
> >>>gen = intel_gen(intel_get_drm_devid(fd));
> >>>
> >>> + if (flags & SYNCOBJ) {
> >>> + syncobj.handle = syncobj_create(fd);
> >>> + syncobj.flags = LOCAL_EXEC_FENCE_SIGNAL;
> >>> + }
> >>> +
> >>>memset(obj, 0, sizeof(obj));
> >>>obj[0].handle = gem_create(fd, 4096);
> >>>if (flags & WRITE)
> >>> @@ -144,6 +201,8 @@ static int loop(unsigned ring, int reps, int ncpus,
> >>> unsigned flags)
> >>>execbuf.buffer_count = 2;
> >>>execbuf.flags |= LOCAL_I915_EXEC_HANDLE_LUT;
> >>>execbuf.flags |= LOCAL_I915_EXEC_NO_RELOC;
> >>> + if (flags & SYNCOBJ)
> >>> + execbuf.flags |= LOCAL_I915_EXEC_FENCE_ARRAY;
> >>
> >> According to the comment in i915_drm.h, when this is specified, syncobj
> >> should be also passed in in cliprects_ptr but that's not happening?
> >
> > You want -b support as well! :)
>
> I just failed to figure out where is the connection between syncobj
> (local var) and execbuf. Flag is set, so what happens next? Execbuf
> fails since cliprects_ptr is not set?
Your observation is correct. The code never set the syncobj to be
signaled. I was just saying that in the manner of polling on the idle
syncobj, it should be ok, though strictly it will be reporting that
there is no fence attached rather than idle. I need to add code to
support the '-b' mode of polling on a busy fence.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/bios: VBT parsing fixes and cleanups
== Series Details == Series: drm/i915/bios: VBT parsing fixes and cleanups URL : https://patchwork.freedesktop.org/series/31051/ State : success == Summary == Test perf: Subgroup blocking: pass -> FAIL (shard-hsw) fdo#102252 Test kms_cursor_legacy: Subgroup cursorA-vs-flipA-atomic-transitions: fail -> PASS (shard-hsw) Test gem_exec_reloc: Subgroup basic-write-read-noreloc: incomplete -> PASS (shard-hsw) fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 shard-hswtotal:2429 pass:1332 dwarn:4 dfail:0 fail:10 skip:1083 time:9910s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5844/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 20/31] drm/i915/slpc: Add parameter set/unset/get, task control/status functions
On 9/21/2017 7:17 PM, Michal Wajdeczko wrote:
On Tue, 19 Sep 2017 19:41:56 +0200, Sagar Arun Kamble
wrote:
SLPC behavior can be changed through set of parameters.
These parameters can be updated and queried from i915 though
Host to GuC SLPC events. This patch adds parameter update
events for setting/unsetting/getting parameters. SLPC has
various tasks for controlling different controls. This patch
adds functions to control and query the task status.
v1: Use host2guc_slpc
update slcp_param_id enum values for SLPC 2015.2.4
return void instead of ignored error code (Paulo)
v2: Checkpatch update.
v3: Rebase.
v4: Updated with GuC firmware v9.
v5: Updated input structure to host2guc_slpc. Added functions
to update only parameters in the SLPC shared memory. This
will allow to setup shared data with all parameters and send
single event to SLPC take them into effect. Commit message
update. (Sagar)
v6: Rearranged helpers to use them in slpc_shared_data_init.
Added definition of SLPC_KMD_MAX_PARAM.
v7: Added definition of host2guc_slpc with rearrangement of patches.
Added task control/status functions.
v8: Rebase w.r.t s/intel_guc_send/intel_guc_send_mmio.
Cc: Michal Wajdeczko
Signed-off-by: Tom O'Rourke
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/intel_guc.c | 21 -
drivers/gpu/drm/i915/intel_guc.h | 2 +
drivers/gpu/drm/i915/intel_slpc.c | 185
++
drivers/gpu/drm/i915/intel_slpc.h | 8 ++
4 files changed, 215 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_guc.c
b/drivers/gpu/drm/i915/intel_guc.c
index a92c7e8..656bae9 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -67,9 +67,11 @@ void intel_guc_init_send_regs(struct intel_guc *guc)
/*
* This function implements the MMIO based host to GuC interface.
*/
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action,
u32 len)
+int __intel_guc_send_mmio(struct intel_guc *guc, const u32 *action,
u32 len,
+ u32 *output)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ union slpc_event_output_header header;
Don't pollute generic send function with slpc specific code.
Ok. Will prepare new SLPC specific send function built on top of GUC CT
based send.
u32 status;
int i;
int ret;
@@ -115,12 +117,29 @@ int intel_guc_send_mmio(struct intel_guc *guc,
const u32 *action, u32 len)
action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
}
+ /*
+ * Output data from Host to GuC SLPC actions is populated in
scratch
+ * registers SOFT_SCRATCH(1) to SOFT_SCRATCH(14) based on event.
Note that receiving more data over MMIO will be handled by these
pending patches
https://patchwork.freedesktop.org/patch/170667/
https://patchwork.freedesktop.org/patch/170669/
The same series will also add support for responses over CT so stay
tuned!
Yes. Will base SLPC changes on top of these.
+ * Currently only SLPC action status in GuC is meaningful as Host
+ * can query only overridden parameters and that are fetched from
+ * Host-GuC SLPC shared data.
+ */
+ if (output && !ret) {
+ output[0] = header.value = I915_READ(SOFT_SCRATCH(1));
+ ret = header.status;
+ }
+
intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
mutex_unlock(&guc->send_mutex);
return ret;
}
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action,
u32 len)
+{
+ return __intel_guc_send_mmio(guc, action, len, NULL);
+}
+
int intel_guc_sample_forcewake(struct intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
diff --git a/drivers/gpu/drm/i915/intel_guc.h
b/drivers/gpu/drm/i915/intel_guc.h
index b835d30..c27d2dd 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -132,6 +132,8 @@ struct intel_guc {
int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32
len);
void gen8_guc_raise_irq(struct intel_guc *guc);
void intel_guc_init_send_regs(struct intel_guc *guc);
+int __intel_guc_send_mmio(struct intel_guc *guc, const u32 *action,
u32 len,
+ u32 *output);
int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action,
u32 len);
int intel_guc_sample_forcewake(struct intel_guc *guc);
int intel_guc_runtime_suspend(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_slpc.c
b/drivers/gpu/drm/i915/intel_slpc.c
index 73e7bf5..f47d81e 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -132,6 +132,191 @@ int slpc_mem_task_control(struct
slpc_shared_data *data, u64 val,
return ret;
}
+static void host2guc_slpc(struct intel_slpc *slpc,
+ struct slpc_event_input *input, u32 len)
+{
+ struct intel_guc *guc = slpc_to_guc(slpc);
+ u32 *data;
+ u32 output[SLPC_EVENT_MAX_OUTPUT_ARGS];
+ int ret = 0;
+
+
[Intel-gfx] [PATCH] drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled
Only init / reset the display interrupts during power well enabling /
disabling if the i915 interrupts are enabled. So far we did the
init / reset during driver loading / resuming too, where
initialization / enabling of the i915 interrupts happens only at a later
point. This didn't cause a problem due to GEN8_MASTER_IRQ_CONTROL being
cleared, but triggered gen3_assert_iir_is_zero() in GEN8_IRQ_INIT_NDX().
References: https://bugs.freedesktop.org/show_bug.cgi?id=102988
Cc: Chris Wilson
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_irq.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index af82bd721dbc..f048ac478355 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3132,10 +3132,17 @@ void gen8_irq_power_well_post_enable(struct
drm_i915_private *dev_priv,
enum pipe pipe;
spin_lock_irq(&dev_priv->irq_lock);
+
+ if (!intel_irqs_enabled(dev_priv)) {
+ spin_unlock_irq(&dev_priv->irq_lock);
+ return;
+ }
+
for_each_pipe_masked(dev_priv, pipe, pipe_mask)
GEN8_IRQ_INIT_NDX(DE_PIPE, pipe,
dev_priv->de_irq_mask[pipe],
~dev_priv->de_irq_mask[pipe] | extra_ier);
+
spin_unlock_irq(&dev_priv->irq_lock);
}
@@ -3145,8 +3152,15 @@ void gen8_irq_power_well_pre_disable(struct
drm_i915_private *dev_priv,
enum pipe pipe;
spin_lock_irq(&dev_priv->irq_lock);
+
+ if (!intel_irqs_enabled(dev_priv)) {
+ spin_unlock_irq(&dev_priv->irq_lock);
+ return;
+ }
+
for_each_pipe_masked(dev_priv, pipe, pipe_mask)
GEN8_IRQ_RESET_NDX(DE_PIPE, pipe);
+
spin_unlock_irq(&dev_priv->irq_lock);
/* make sure we're done processing display irqs */
--
2.13.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 21/31] drm/i915/slpc: Send RESET event to enable SLPC during Load/TDR
On 9/21/2017 7:36 PM, Michal Wajdeczko wrote:
On Tue, 19 Sep 2017 19:41:57 +0200, Sagar Arun Kamble
wrote:
Send host2guc SLPC reset event to GuC post GuC load.
Post this, i915 can ascertain if SLPC has started running successfully
through shared data. This check is done during intel_init_gt_powersave.
This allows to get initial configuration setup by SLPC and if needed
move to Host RPS if SLPC runs into issues.
On TDR/Engine reset i915 should send extra flag
SLPC_RESET_FLAG_TDR_OCCURREDto clear SLPC state as appropriate.
v1: Extract host2guc_slpc to handle slpc status code
coding style changes (Paulo)
Removed WARN_ON for checking msb of gtt address of
shared gem obj. (ChrisW)
host2guc_action to i915_guc_action change.(Sagar)
Updating SLPC enabled status. (Sagar)
v2: Commit message update. (David)
v3: Rebase.
v4: Added DRM_INFO message when SLPC is enabled.
v5: Updated patch as host2guc_slpc is moved to earlier patch.
SLPC activation status message put after checking the
state from shared data during intel_init_gt_powersave.
v6: Added definition of host2guc_slpc and clflush the shared data only
for required size. Setting state to NOT_RUNNING before sending RESET
event. Output data for SLPC actions is to be retrieved during
intel_guc_send with lock protection so created wrapper
__intel_guc_send that outputs GuC output data if needed. Clearing
pm_rps_events on confirming SLPC RUNNING status so that even if
host touches any of the PM registers by mistake it should not have
any effect. (Sagar)
v7: Added save/restore_default_rps as Uncore sanitize will clear the
RP_CONTROL setup by BIOS. s/i915_ggtt_offset/guc_ggtt_offset.
v8: Added support for handling TDR based SLPC reset. Added functions
host2guc_slpc_tdr_reset, intel_slpc_reset_prepare and
intel_slpc_tdr_reset to handle TDR based SLPC reset.
Cc: Michal Wajdeczko
Signed-off-by: Tom O'Rourke
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/i915_drv.c | 2 +
drivers/gpu/drm/i915/i915_irq.c | 7 +-
drivers/gpu/drm/i915/intel_pm.c | 10 +++
drivers/gpu/drm/i915/intel_slpc.c | 170
++
drivers/gpu/drm/i915/intel_slpc.h | 9 ++
drivers/gpu/drm/i915/intel_uc.c | 1 +
6 files changed, 198 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index f13a3de..932f9ef 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1074,6 +1074,8 @@ static int i915_driver_init_hw(struct
drm_i915_private *dev_priv)
intel_sanitize_options(dev_priv);
+ intel_slpc_save_default_rps(&dev_priv->guc.slpc);
+
ret = i915_ggtt_probe_hw(dev_priv);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 4a1554c..2d5ad13 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2838,8 +2838,13 @@ void i915_handle_error(struct drm_i915_private
*dev_priv,
}
}
- if (!engine_mask)
+ if (!engine_mask) {
+ if (intel_slpc_active(&dev_priv->guc.slpc)) {
+ intel_slpc_reset_prepare(&dev_priv->guc.slpc);
+ intel_slpc_tdr_reset(&dev_priv->guc.slpc);
+ }
Can you just jump to single slpc function that will hide slpc internals ?
we need to have these two calls as currently SLPC gets enabled during
intel_uc_init_hw and we can reset from two places.
1. engine reset 2. full gpu reset
In 1st case we want to disable SLPC and reset it
In 2nd case we just want to disable SLPC and let it get enabled through
intel_slpc_enable based on flag tdr_reset.
I will remove intel_slpc_active check here and probably create two
functions now to handle these different types of resets.
goto out;
+ }
/* Full reset needs the mutex, stop any other user trying to do
so. */
if (test_and_set_bit(I915_RESET_BACKOFF,
&dev_priv->gpu_error.flags)) {
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 6b2b7f8..c2065f2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7918,6 +7918,16 @@ void intel_init_gt_powersave(struct
drm_i915_private *dev_priv)
intel_runtime_pm_get(dev_priv);
}
+ if (intel_slpc_enabled()) {
+ dev_priv->guc.slpc.active =
+ intel_slpc_get_status(&dev_priv->guc.slpc);
+ if (!intel_slpc_active(&dev_priv->guc.slpc)) {
+ i915.enable_slpc = 0;
+ intel_sanitize_gt_powersave(dev_priv);
+ } else
+ dev_priv->pm_rps_events = 0;
+ }
+
Hmm, on one hand you're trying to use friendly wrappers like
enabled() active() but at the same time you're modifying data
which these helpers were trying to hide ...
This actually is point from where on intel_slpc_active() is determined.
SLPC enabling is asynchronous and we would need to poll on the shared
memory status to k
Re: [Intel-gfx] [PATCH 03/11] drm/i915: Include fence-hint for timeout warning
On Mon, 2017-09-11 at 09:41 +0100, Chris Wilson wrote: > If an asynchronous wait on a foriegn fence, we print a warning "foreign" Reviewed-by: Joonas Lahtinen Regards, Joonas > indicating which fence was not signaled. As i915_sw_fences become more > common, include the debug hint (the symbol-name of the target) to help > identify the waiter. E.g. > > [ 31.968144] Asynchronous wait on fence sw_sync:gem_eio:1 timed out for > hint:submit_notify [i915] > > Signed-off-by: Chris Wilson -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt] lib: Capture the error state on an unexpected hang
On Tue, Sep 12, 2017 at 09:10:25PM +0100, Chris Wilson wrote:
> Dump debugfs/i915_error_state to the debug channel if we detect an ERROR
> uevent. This poses a few problems, not least that it is the auxiliary
> process doing the dumping (so the output may be interleaved with the
> test, but considering a hang occurred it is likely the test is blocked)
> and the average error state is around 60k, which may prove unwieldy!
>
> On the other hand, it may prove invaluable in debugging those impossible
> to reproduce bugs.
>
> Signed-off-by: Chris Wilson
Reviewed-by: Petri Latvala
> ---
> lib/igt_aux.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 65f832ab..ae076aaa 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -395,7 +395,7 @@ void igt_stop_shrink_helper(void)
>
> static struct igt_helper_process hang_detector;
> static void __attribute__((noreturn))
> -hang_detector_process(pid_t pid, dev_t rdev)
> +hang_detector_process(int fd, pid_t pid, dev_t rdev)
> {
> struct udev_monitor *mon =
> udev_monitor_new_from_netlink(udev_new(), "kernel");
> @@ -429,8 +429,10 @@ hang_detector_process(pid_t pid, dev_t rdev)
> const char *str;
>
> str = udev_device_get_property_value(dev, "ERROR");
> - if (str && atoi(str) == 1)
> + if (str && atoi(str) == 1) {
> + igt_debugfs_dump(fd, "i915_error_state");
> kill(pid, SIGIO);
> + }
> }
>
> udev_device_unref(dev);
> @@ -462,7 +464,7 @@ void igt_fork_hang_detector(int fd)
>
> signal(SIGIO, sig_abort);
> igt_fork_helper(&hang_detector)
> - hang_detector_process(getppid(), st.st_rdev);
> + hang_detector_process(fd, getppid(), st.st_rdev);
> }
>
> void igt_stop_hang_detector(void)
> --
> 2.14.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/11] drm/i915: Try harder to finish the idle-worker
On Mon, 2017-09-11 at 09:41 +0100, Chris Wilson wrote: > If a worker requeues itself, it may switch to a different kworker pool, > which flush_work() considers as complete. To be strict, we then need to > keep flushing the work until it is no longer pending. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=102456 > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled
Quoting Imre Deak (2017-09-28 11:06:24) > Only init / reset the display interrupts during power well enabling / > disabling if the i915 interrupts are enabled. So far we did the > init / reset during driver loading / resuming too, where > initialization / enabling of the i915 interrupts happens only at a later > point. This didn't cause a problem due to GEN8_MASTER_IRQ_CONTROL being > cleared, but triggered gen3_assert_iir_is_zero() in GEN8_IRQ_INIT_NDX(). > > References: https://bugs.freedesktop.org/show_bug.cgi?id=102988 > Cc: Chris Wilson > Signed-off-by: Imre Deak Patch makes sense, so Reviewed-by: Chris Wilson There's an irq powerwell! When is it taken? We don't take it for GT as far as I am aware (we should for execlists plus whenever we enable the user interrupt). Should we? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 24/31] drm/i915/slpc: Add debugfs support to read/write/revert the parameters
On 9/21/2017 8:37 PM, Michal Wajdeczko wrote:
On Tue, 19 Sep 2017 19:42:00 +0200, Sagar Arun Kamble
wrote:
This patch adds two debugfs interfaces:
1. i915_slpc_paramlist: List of all parameters that Host can configure.
Currently listing id and description of each.
2. i915_slpc_param_ctl: This allows to change the parameters. Syntax is:
echo "write " > i915_slpc_param_ctl.
echo "read " > i915_slpc_param_ctl; cat i915_slpc_param_ctl
revert allows to set to default SLPC internal values. Syntax is:
echo "revert " > i915_slpc_param_ctl.
Added support to set/read parameters and unset the parameters which will
revert them to default SLPC internal values. Also added RPM ref. cover
around set/unset calls. Explicit SLPC reset is needed on
setting/unsetting
some of the parameters.
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/i915_debugfs.c | 19 +
drivers/gpu/drm/i915/intel_slpc.c | 158
drivers/gpu/drm/i915/intel_slpc.h | 6 ++
3 files changed, 183 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index dbfe185..0a04f3d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2352,6 +2352,23 @@ static int i915_huc_load_status_info(struct
seq_file *m, void *data)
return 0;
}
+static int i915_slpc_paramlist_info(struct seq_file *m, void *data)
I'm little confused that part of the debugfs functionality is done here
while other part in slpc.c
Will pull them together. It was to allow new interfaces to make use of
the functionality in slpc.c.
+{
+ struct drm_i915_private *dev_priv = node_to_i915(m->private);
+ int i;
+
+ if (!dev_priv->guc.slpc.active) {
intel_slpc_active() ?
yes. will update.
+ seq_puts(m, "SLPC not active\n");
+ return 0;
+ }
+
+ seq_puts(m, "Param id\tParam description\n");
+ for (i = 0; i < SLPC_MAX_PARAM; i++)
+ seq_printf(m, "%8d\t%s\n", slpc_paramlist[i].id,
+ slpc_paramlist[i].description);
What if size of slpc_paramlist[] will be smaller than i ?
will add the size checks.
+ return 0;
+}
+
static int i915_guc_load_status_info(struct seq_file *m, void *data)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4881,6 +4898,7 @@ static int i915_hpd_storm_ctl_open(struct inode
*inode, struct file *file)
{"i915_guc_load_err_log_dump", i915_guc_log_dump, 0, (void *)1},
{"i915_guc_stage_pool", i915_guc_stage_pool, 0},
{"i915_huc_load_status", i915_huc_load_status_info, 0},
+ {"i915_slpc_paramlist", i915_slpc_paramlist_info, 0},
{"i915_frequency_info", i915_frequency_info, 0},
{"i915_hangcheck_info", i915_hangcheck_info, 0},
{"i915_reset_info", i915_reset_info, 0},
@@ -4944,6 +4962,7 @@ static int i915_hpd_storm_ctl_open(struct inode
*inode, struct file *file)
{"i915_dp_test_type", &i915_displayport_test_type_fops},
{"i915_dp_test_active", &i915_displayport_test_active_fops},
{"i915_guc_log_control", &i915_guc_log_control_fops},
+ {"i915_slpc_param_ctl", &i915_slpc_param_ctl_fops},
{"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
{"i915_ipc_status", &i915_ipc_status_fops}
};
diff --git a/drivers/gpu/drm/i915/intel_slpc.c
b/drivers/gpu/drm/i915/intel_slpc.c
index d0fd402..0c094f0 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -25,6 +25,8 @@
#include
#include "i915_drv.h"
#include "intel_uc.h"
+#include
+#include
struct slpc_param slpc_paramlist[SLPC_MAX_PARAM] = {
{SLPC_PARAM_TASK_ENABLE_GTPERF, "Enable task GTPERF"},
@@ -684,3 +686,159 @@ void intel_slpc_disable(struct intel_slpc *slpc)
slpc->active = false;
}
+
+static int slpc_param_ctl_show(struct seq_file *m, void *data)
+{
+ struct drm_i915_private *dev_priv = m->private;
+ struct intel_slpc *slpc = &dev_priv->guc.slpc;
+
+ if (!slpc->active) {
intel_slpc_active() ?
yes. will update.
+ seq_puts(m, "SLPC not active\n");
+ return 0;
+ }
+
+ seq_printf(m, "%s=%u, override=%s\n",
+ slpc_paramlist[slpc->debug_param_id].description,
+ slpc->debug_param_value,
+ yesno(!!slpc->debug_param_override));
+
What if slpc->debug_param_id >= SLPC_MAX_PARAM or sizeof paramlist ?
will add the check.
+ return 0;
+}
+
+static int slpc_param_ctl_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, slpc_param_ctl_show, inode->i_private);
+}
+
+static const char *read_token = "read", *write_token = "write",
+ *revert_token = "revert";
+
+/*
+ * Parse SLPC parameter control strings: (Similar to Pipe CRC handling)
+ * command: wsp* op wsp+ param id wsp+ [value] wsp*
+ * op: "read"/"write"/"revert"
+ * param id: slpc_param_id
+ * value: u32 value
+ * wsp: (#0x20 | #0x9 | #0xA)+
+ *
+ * eg.:
+ * "read 0" -> read SLPC_PARAM_TASK_ENABLE_GTPERF
+
Re: [Intel-gfx] [PATCH 26/31] drm/i915/slpc: Add i915_slpc_info to debugfs
On 9/21/2017 8:43 PM, Michal Wajdeczko wrote:
On Tue, 19 Sep 2017 19:42:02 +0200, Sagar Arun Kamble
wrote:
From: Tom O'Rourke
i915_slpc_info shows the contents of SLPC shared data
parsed into text format.
v1: Reformat slpc info (Radek)
squashed query task state info
in slpc info, kunmap before seq_print (Paulo)
return void instead of ignored return value (Paulo)
Avoid magic numbers and use local variables (Jon Bloomfield)
Removed WARN_ON for checking msb of gtt address of
shared gem obj. (ChrisW)
Moved definition of power plan and power source to earlier
patch in the series.
drm/i915/slpc: Allocate/Release/Initialize SLPC shared data
(Akash)
v2-v3: Rebase.
v4: Updated with GuC firmware v9.
v5: Updated host2guc_slpc_query_task_state with struct slpc_input_event
structure. Removed unnecessary checks of vma from i915_slpc_info.
Created helpers for reading the SLPC shared data and string form of
SLPC state. (Sagar)
Signed-off-by: Tom O'Rourke
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/i915_debugfs.c | 165
1 file changed, 165 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index e6fd63f..7a0838f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1023,6 +1023,170 @@ static int i915_error_state_open(struct inode
*inode, struct file *file)
NULL, i915_next_seqno_set,
"0x%llx\n");
+static int i915_slpc_info(struct seq_file *m, void *unused)
+{
+ struct drm_i915_private *dev_priv = node_to_i915(m->private);
+ int i, value;
+ struct slpc_shared_data data;
+ enum slpc_global_state global_state;
+ enum slpc_platform_sku platform_sku;
+ struct slpc_task_state_data *task_data;
+ enum slpc_power_plan power_plan;
+ enum slpc_power_source power_source;
+
+ if (!dev_priv->guc.slpc.active)
+ return -ENODEV;
+
+ intel_runtime_pm_get(dev_priv);
+ mutex_lock(&dev_priv->pm.pcu_lock);
+
+ intel_slpc_read_shared_data(&dev_priv->guc.slpc, &data);
+
+ mutex_unlock(&dev_priv->pm.pcu_lock);
+ intel_runtime_pm_put(dev_priv);
+
+ seq_printf(m, "shared data size: %d\n", data.shared_data_size);
+
+ global_state = (enum slpc_global_state) data.global_state;
+ seq_printf(m, "global state: %d (", global_state);
+ seq_printf(m, "%s)\n", intel_slpc_get_state_str(global_state));
+
+ platform_sku = (enum slpc_platform_sku)
+ data.platform_info.platform_sku;
+ seq_printf(m, "sku: %d (", platform_sku);
+ switch (platform_sku) {
+ case SLPC_PLATFORM_SKU_UNDEFINED:
+ seq_puts(m, "undefined)\n");
+ break;
+ case SLPC_PLATFORM_SKU_ULX:
+ seq_puts(m, "ULX)\n");
+ break;
+ case SLPC_PLATFORM_SKU_ULT:
+ seq_puts(m, "ULT)\n");
+ break;
+ case SLPC_PLATFORM_SKU_T:
+ seq_puts(m, "T)\n");
+ break;
+ case SLPC_PLATFORM_SKU_MOBL:
+ seq_puts(m, "Mobile)\n");
+ break;
+ case SLPC_PLATFORM_SKU_DT:
+ seq_puts(m, "DT)\n");
+ break;
+ case SLPC_PLATFORM_SKU_UNKNOWN:
+ default:
+ seq_puts(m, "unknown)\n");
+ break;
+ }
Define platform_to_string() followed by single seq_puts()
Sure. Will update.
+ seq_printf(m, "slice count: %d\n",
+ data.platform_info.slice_count);
+
+ seq_printf(m, "power plan/source: 0x%x\n\tplan:\t",
+ data.platform_info.power_plan_source);
+ power_plan = (enum slpc_power_plan) SLPC_POWER_PLAN(
+ data.platform_info.power_plan_source);
+ power_source = (enum slpc_power_source) SLPC_POWER_SOURCE(
+ data.platform_info.power_plan_source);
+ switch (power_plan) {
+ case SLPC_POWER_PLAN_UNDEFINED:
+ seq_puts(m, "undefined");
+ break;
+ case SLPC_POWER_PLAN_BATTERY_SAVER:
+ seq_puts(m, "battery saver");
+ break;
+ case SLPC_POWER_PLAN_BALANCED:
+ seq_puts(m, "balanced");
+ break;
+ case SLPC_POWER_PLAN_PERFORMANCE:
+ seq_puts(m, "performance");
+ break;
+ case SLPC_POWER_PLAN_UNKNOWN:
+ default:
+ seq_puts(m, "unknown");
+ break;
+ }
Define power_plan_to_string() followed by single seq_puts()
Sure. Will update.
+ seq_puts(m, "\n\tsource:\t");
+ switch (power_source) {
+ case SLPC_POWER_SOURCE_UNDEFINED:
+ seq_puts(m, "undefined\n");
+ break;
+ case SLPC_POWER_SOURCE_AC:
+ seq_puts(m, "AC\n");
+ break;
+ case SLPC_POWER_SOURCE_DC:
+ seq_puts(m, "DC\n");
+ break;
+ case SLPC_POWER_SOURCE_UNKNOWN:
+ default:
+ seq_puts(m, "unknown\n");
+ break;
+ }
+
+ seq_printf(m, "IA frequency (MHz):\n\tP0: %d\n\tP1: %d\n\tPe:
%d\n\tPn: %d\n",
+ data.platform_info.P0_freq * 50,
+ data.platform_info.P1_freq * 50,
+ data.platform_info.Pe_freq * 50
Re: [Intel-gfx] [PATCH 06/11] drm/i915: Pin fence for iomap
On Mon, 2017-09-11 at 09:41 +0100, Chris Wilson wrote:
> Acquire the fence register for the iomap in i915_vma_pin_iomap() on
> behalf of the caller.
>
> We probably want for the caller to specify whether the fence should be
> pinned for their usage, but at the moment all callers do want the
> associated fence, or none, so take it on their behalf.
>
> Signed-off-by: Chris Wilson
> @@ -278,6 +278,7 @@ int i915_vma_bind(struct i915_vma *vma, enum
> i915_cache_level cache_level,
> void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
> {
> void __iomem *ptr;
> + int ret;
>
> /* Access through the GTT requires the device to be awake. */
> assert_rpm_wakelock_held(vma->vm->i915);
> @@ -301,9 +302,27 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
> }
>
> __i915_vma_pin(vma);
> +
> + ret = i915_vma_get_fence(vma);
> + if (ret) {
> + __i915_vma_unpin(vma);
> + return IO_ERR_PTR(ret);
Please consolidate the IO_ERR_PTR's into goto labels. Then this is;
Reviewed-by: Joonas Lahtinen
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2 5/8] lib/igt_kms: Rework plane properties to be more atomic, v3.
In the future I want to allow tests to commit more properties,
but for this to work I have to fix all properties to work better
with atomic commit. Instead of special casing each
property make a bitmask for all property changed flags, and try to
commit all properties.
Changes since v1:
- Remove special dumping of src and crtc coordinates.
- Dump all modified coordinates.
Changes since v2:
- Move igt_plane_set_prop_changed up slightly.
Signed-off-by: Maarten Lankhorst
---
lib/igt_kms.c| 293 +--
lib/igt_kms.h| 59
tests/kms_atomic_interruptible.c | 12 +-
tests/kms_rotation_crc.c | 4 +-
4 files changed, 159 insertions(+), 209 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 07d2074c2b1a..6e0052ebe7a7 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -192,11 +192,11 @@ const char
*igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
/*
* Retrieve all the properies specified in props_name and store them into
- * plane->atomic_props_plane.
+ * plane->props.
*/
static void
-igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
- int num_props, const char **prop_names)
+igt_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
+int num_props, const char **prop_names)
{
drmModeObjectPropertiesPtr props;
int i, j, fd;
@@ -214,7 +214,7 @@ igt_atomic_fill_plane_props(igt_display_t *display,
igt_plane_t *plane,
if (strcmp(prop->name, prop_names[j]) != 0)
continue;
- plane->atomic_props_plane[j] = props->props[i];
+ plane->props[j] = props->props[i];
break;
}
@@ -1659,7 +1659,6 @@ void igt_display_init(igt_display_t *display, int drm_fd)
drmModeRes *resources;
drmModePlaneRes *plane_resources;
int i;
- int is_atomic = 0;
memset(display, 0, sizeof(igt_display_t));
@@ -1679,7 +1678,9 @@ void igt_display_init(igt_display_t *display, int drm_fd)
igt_assert_f(display->pipes, "Failed to allocate memory for %d
pipes\n", display->n_pipes);
drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
- is_atomic = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1);
+ if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
+ display->is_atomic = 1;
+
plane_resources = drmModeGetPlaneResources(display->drm_fd);
igt_assert(plane_resources);
@@ -1776,19 +1777,19 @@ void igt_display_init(igt_display_t *display, int
drm_fd)
plane->type = type;
plane->pipe = pipe;
plane->drm_plane = drm_plane;
- plane->fence_fd = -1;
+ plane->values[IGT_PLANE_IN_FENCE_FD] = ~0ULL;
- if (is_atomic == 0) {
- display->is_atomic = 1;
- igt_atomic_fill_plane_props(display, plane,
IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
- }
+ igt_fill_plane_props(display, plane,
IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
get_plane_property(display->drm_fd, drm_plane->plane_id,
"rotation",
- &plane->rotation_property,
- &prop_value,
+ &plane->props[IGT_PLANE_ROTATION],
+ &plane->values[IGT_PLANE_ROTATION],
NULL);
- plane->rotation = (igt_rotation_t)prop_value;
+
+ /* Clear any residual framebuffer on first commit. */
+ igt_plane_set_prop_changed(plane, IGT_PLANE_FB_ID);
+ igt_plane_set_prop_changed(plane, IGT_PLANE_CRTC_ID);
}
/*
@@ -1805,9 +1806,6 @@ void igt_display_init(igt_display_t *display, int drm_fd)
pipe->n_planes = n_planes;
- for_each_plane_on_pipe(display, i, plane)
- plane->fb_changed = true;
-
pipe->mode_changed = true;
}
@@ -2070,18 +2068,7 @@ bool igt_pipe_get_property(igt_pipe_t *pipe, const char
*name,
static uint32_t igt_plane_get_fb_id(igt_plane_t *plane)
{
- if (plane->fb)
- return plane->fb->fb_id;
- else
- return 0;
-}
-
-static uint32_t igt_plane_get_fb_gem_handle(igt_plane_t *plane)
-{
- if (plane->fb)
- return plane->fb->gem_handle;
- else
- return 0;
+ return plane->values[IGT_PLANE_FB_ID];
}
#define CHECK_RETURN(r, fail) {\
@@ -2090,9 +2077,6 @@ static uint32_t igt_plane_ge
[Intel-gfx] [PATCH i-g-t v2 6/8] lib/igt_kms: Rework pipe properties to be more atomic, v3.
In the future I want to allow tests to commit more properties,
but for this to work I have to fix all properties to work better
with atomic commit. Instead of special casing each
property make a bitmask for all property changed flags, and try to
commit all properties.
This has been the most involved one, since legacy pipe commit still
handles a lot of the properties differently from the rest.
Changes since v1:
- Dump all changed properties on commit.
- Fix bug in igt_pipe_refresh().
Changes since v2:
- Set pipe ACTIVE property changed flag on init.
Signed-off-by: Maarten Lankhorst
---
lib/igt_kms.c | 217 --
lib/igt_kms.h | 77 ++
tests/kms_atomic_interruptible.c | 4 +-
tests/kms_atomic_transition.c | 2 +-
tests/kms_crtc_background_color.c | 2 +-
5 files changed, 147 insertions(+), 155 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 6e0052ebe7a7..a81d7998433a 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -259,8 +259,8 @@ igt_atomic_fill_connector_props(igt_display_t *display,
igt_output_t *output,
}
static void
-igt_atomic_fill_pipe_props(igt_display_t *display, igt_pipe_t *pipe,
- int num_crtc_props, const char **crtc_prop_names)
+igt_fill_pipe_props(igt_display_t *display, igt_pipe_t *pipe,
+ int num_crtc_props, const char **crtc_prop_names)
{
drmModeObjectPropertiesPtr props;
int i, j, fd;
@@ -278,7 +278,7 @@ igt_atomic_fill_pipe_props(igt_display_t *display,
igt_pipe_t *pipe,
if (strcmp(prop->name, crtc_prop_names[j]) != 0)
continue;
- pipe->atomic_props_crtc[j] = props->props[i];
+ pipe->props[j] = props->props[i];
break;
}
@@ -1690,7 +1690,6 @@ void igt_display_init(igt_display_t *display, int drm_fd)
int p = 1;
int j, type;
uint8_t last_plane = 0, n_planes = 0;
- uint64_t prop_value;
pipe->crtc_id = resources->crtcs[i];
pipe->display = display;
@@ -1700,29 +1699,16 @@ void igt_display_init(igt_display_t *display, int
drm_fd)
pipe->planes = NULL;
pipe->out_fence_fd = -1;
+ igt_fill_pipe_props(display, pipe, IGT_NUM_CRTC_PROPS,
igt_crtc_prop_names);
+
+ /* Force modeset disable on first commit */
+ igt_pipe_obj_set_prop_changed(pipe, IGT_CRTC_MODE_ID);
+ igt_pipe_obj_set_prop_changed(pipe, IGT_CRTC_ACTIVE);
+
get_crtc_property(display->drm_fd, pipe->crtc_id,
- "background_color",
- &pipe->background_property,
- &prop_value,
+ "background_color", NULL,
+ &pipe->values[IGT_CRTC_BACKGROUND],
NULL);
- pipe->background = (uint32_t)prop_value;
- get_crtc_property(display->drm_fd, pipe->crtc_id,
- "DEGAMMA_LUT",
- &pipe->degamma_property,
- NULL,
- NULL);
- get_crtc_property(display->drm_fd, pipe->crtc_id,
- "CTM",
- &pipe->ctm_property,
- NULL,
- NULL);
- get_crtc_property(display->drm_fd, pipe->crtc_id,
- "GAMMA_LUT",
- &pipe->gamma_property,
- NULL,
- NULL);
-
- igt_atomic_fill_pipe_props(display, pipe, IGT_NUM_CRTC_PROPS,
igt_crtc_prop_names);
/* count number of valid planes */
for (j = 0; j < plane_resources->count_planes; j++) {
@@ -1805,8 +1791,6 @@ void igt_display_init(igt_display_t *display, int drm_fd)
igt_assert_eq(p, n_planes);
pipe->n_planes = n_planes;
-
- pipe->mode_changed = true;
}
/*
@@ -2283,7 +2267,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
*primary,
if (!igt_plane_is_prop_changed(primary, IGT_PLANE_FB_ID) &&
!(primary->changed & IGT_PLANE_COORD_CHANGED_MASK) &&
- !primary->pipe->mode_changed)
+ !igt_pipe_obj_is_prop_changed(primary->pipe, IGT_CRTC_MODE_ID))
return 0;
crtc_id = pipe->crtc_id;
@@ -2352,6 +2336,16 @@ static int igt_plane_commit(igt_plane_t *plane,
}
}
+static bool is_atomic_prop(enum igt_atomic_crtc_properties prop)
+{
+ if (prop == IGT_CRTC_MODE_ID ||
+ prop == IGT_CRTC_ACTIVE ||
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled
== Series Details == Series: drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled URL : https://patchwork.freedesktop.org/series/31058/ State : warning == Summary == Series 31058v1 drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled https://patchwork.freedesktop.org/api/1.0/series/31058/revisions/1/mbox/ Test kms_force_connector_basic: Subgroup force-connector-state: pass -> SKIP (fi-ivb-3520m) Subgroup force-edid: pass -> SKIP (fi-ivb-3520m) Subgroup force-load-detect: pass -> SKIP (fi-ivb-3520m) Subgroup prune-stale-modes: pass -> SKIP (fi-ivb-3520m) fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:443s fi-bdw-gvtdvmtotal:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:473s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:415s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:511s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:281s fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:504s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:508s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:500s fi-cnl-y total:289 pass:258 dwarn:0 dfail:0 fail:4 skip:27 time:643s fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:413s fi-glk-1 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:562s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:427s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:402s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:433s fi-ivb-3520m total:289 pass:257 dwarn:0 dfail:0 fail:0 skip:32 time:485s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:470s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:470s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:620s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:589s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:544s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:452s fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:746s fi-skl-6770hqtotal:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:491s fi-skl-gvtdvmtotal:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:475s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:572s fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:417s c1ed501217782b35094792696924a1d22ee83c0f drm-tip: 2017y-09m-28d-04h-52m-47s UTC integration manifest 052c5c13b17f drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5845/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-misc-fixes
Hi Dave, Here's the latest from -misc-fixes. It's been a while since the last one due to plumbers and XDC. I'll be out of office next week, but will do my best to send -fixes amongst moving boxes and chaos. drm-misc-fixes-2017-09-28-1: Driver Changes: - qxl: fix primary surface and fb unpinning (Gerd) - sun41: fix CEC_PIN config gate now that media has been merged (Hans) - tegra: fix TRACE_INCLUDE_PATH (Thierry) Cc: Thierry Reding Cc: Hans Verkuil Cc: Gerd Hoffmann Cheers, Sean The following changes since commit 4a704d6db0ee4a349d5d523813a718e429b4914d: Merge tag 'kbuild-fixes-v4.14' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild (2017-09-21 06:01:03 -1000) are available in the git repository at: git://anongit.freedesktop.org/git/drm-misc tags/drm-misc-fixes-2017-09-28-1 for you to fetch changes up to a98c75fcd0ec02623f4f56d824d76e659410a52b: drm/tegra: trace: Fix path to include (2017-09-26 11:08:17 +0200) Driver Changes: - qxl: fix primary surface and fb unpinning (Gerd) - sun41: fix CEC_PIN config gate now that media has been merged (Hans) - tegra: fix TRACE_INCLUDE_PATH (Thierry) Cc: Thierry Reding Cc: Hans Verkuil Cc: Gerd Hoffmann Gerd Hoffmann (2): qxl: fix primary surface handling qxl: fix framebuffer unpinning Hans Verkuil (1): drm/sun4i: cec: Enable back CEC-pin framework Sean Paul (1): Merge remote-tracking branch 'origin/master' into drm-misc-fixes Thierry Reding (1): drm/tegra: trace: Fix path to include drivers/gpu/drm/qxl/qxl_display.c | 41 +- drivers/gpu/drm/sun4i/Kconfig | 2 +- drivers/gpu/drm/sun4i/sun4i_hdmi.h | 2 +- drivers/gpu/drm/tegra/trace.h | 2 +- 4 files changed, 26 insertions(+), 21 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/11] drm/i915: Check PIN_NONFAULT overlaps in evict_for_node
On Mon, 2017-09-25 at 09:40 +0100, Chris Wilson wrote:
> Quoting Chris Wilson (2017-09-19 13:43:41)
> > Quoting Chris Wilson (2017-09-11 09:41:34)
> > > If the caller says that he doesn't want to evict any other faulting
> > > vma, honour that flag. The logic was used in evict_something, but not
> > > the more specific evict_for_node, now being used as a preliminary probe
> > > since commit 606fec956c0e ("drm/i915: Prefer random replacement before
> > > eviction search").
> > >
> > > Fixes: 606fec956c0e ("drm/i915: Prefer random replacement before eviction
> > > search")
> > > Fixes: 821188778b9b ("drm/i915: Choose not to evict faultable objects
> > > from the GGTT")
> > > References: https://patchwork.freedesktop.org/patch/174781/
> > > Signed-off-by: Chris Wilson
> > > Cc: Joonas Lahtinen
> >
> > Any chance of getting review to this point?
>
> Any better suggestions for resolving the incompletes in CI? Is this good
> enough to land right now and then we can improve again with a happy CI?
Review provided for the CI/bxt relevant parts.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for lib/igt_kms: Convert properties to be more atomic-like. (rev4)
== Series Details == Series: lib/igt_kms: Convert properties to be more atomic-like. (rev4) URL : https://patchwork.freedesktop.org/series/30903/ State : success == Summary == IGT patchset tested on top of latest successful build 2885b10f99b4beeb046e75af8b8488c229f629d3 igt/gem_exec_schedule: Ignore set-priority failures on old kernels with latest DRM-Tip kernel build CI_DRM_3148 c1ed50121778 drm-tip: 2017y-09m-28d-04h-52m-47s UTC integration manifest fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:447s fi-bdw-gvtdvmtotal:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:473s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:423s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:517s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:282s fi-bxt-dsi total:289 pass:258 dwarn:1 dfail:0 fail:0 skip:30 time:498s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:510s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:495s fi-cnl-y total:289 pass:259 dwarn:0 dfail:0 fail:3 skip:27 time:663s fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:415s fi-glk-1 total:289 pass:259 dwarn:1 dfail:0 fail:0 skip:29 time:563s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:433s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:411s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:434s fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:484s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:469s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:475s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:583s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:596s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:547s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:454s fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:756s fi-skl-6770hqtotal:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:492s fi-skl-gvtdvmtotal:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:479s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:567s fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:418s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_264/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add has_psr-flag to gen9lp
On Thu, Sep 28, 2017 at 04:20:29AM +, Rodrigo Vivi wrote: > On Wed, Sep 27, 2017 at 5:14 AM David Weinehall < > david.weineh...@linux.intel.com> wrote: > > > On Tue, Aug 08, 2017 at 12:50:51PM -0700, Rodrigo Vivi wrote: > > > a long time ago I had agreed with Daniel that we would only add new > > > platforms after it was enabled by default on previous platforms. > > > a big reason for that is that we was willing to reduce the platforms > > > to validate and do better validation one by one before enabling. > > > > > > However now I believe it would be beneficial to have that supported > > > added so we can get more brave people using in different platforms so > > > we could capture more corner cases before we enable it by default. > > > Also we can still enable by default one platform at time if needed. > > > > > > So: > > > > > > Acked-by: Rodrigo Vivi > > > > > > I also checked the spec to see if there was anything else new or > > > different for these platforms and didn't find anything so: > > > > > > Reviewed-by: Rodrigo Vivi > > > > > > But let's wait a bit to merge to give Daniel or others a time to nack ;) > > > > An update: while testing revealed that our BXT-P RVP doesn't work with > > PSR, the GLK definitely does. CI would like to do PSR testing on GLK, > > which obviously isn't possible if PSR is reported as unsupported on GLK. > > > > Based on BSpec alone the PSR failure on BXT-P shouldn't be a > > Broxton/Apollo Lake issue, but rather an issue with the RVP board > > (or the panel), so I'd say that this patch still makes sense. > > > It would be very important if we could narrow down the issue on BXT. > Panel?! Bios?! Missing Workaround? Different user space? Agreed. I haven't been able to find any newer BIOS for that device, the user space should be the same. Missing workaround might well be the case, and the panel is definitely not the same as the one the GLK has. We have several other panels that could be tested with though. > One of the biggest problem with PSR is that when it works well in all > machines we have and we enable it we end up finding someone in the > community with a machine that does not work well. "Luckily" I own one of those machines :P > We have an opportunity to investigate and understand very well what > are the issues on this BXT. We shouldn't loose track of it. That opportunity is now rapidly fleeing, since the HW in question is a BXT B0, for which the "drop workarounds" patch series has already been submitted and gotten a R-B. > And maybe adding that to CI we will be forced to record the bug! ;) > > > > > After all it only changes gen9lp to report that they *can* support PSR > > (thus allowing for testing of PSR on such platforms), it doesn't enable > > it by default. > > > > So I'd like to nudge once more that this patch be merged. > > I agree. Let's add it. Also good to enable on CNL as well. If the panel > that you have there on CNL that is on CI doesn't support it you are about > to recurve some panels that does support PSR2. Yeah, enabling on CNL too makes sense and getting systematic PSR2 testing would be awesome. "recurve" => "receive"? Kind regards, David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2 5/8] lib/igt_kms: Rework plane properties to be more atomic, v4.
In the future I want to allow tests to commit more properties,
but for this to work I have to fix all properties to work better
with atomic commit. Instead of special casing each
property make a bitmask for all property changed flags, and try to
commit all properties.
Changes since v1:
- Remove special dumping of src and crtc coordinates.
- Dump all modified coordinates.
Changes since v2:
- Move igt_plane_set_prop_changed up slightly.
Changes since v3:
- Fix wrong ordering of set_position in kms_plane_lowres causing a test failure.
Signed-off-by: Maarten Lankhorst
---
lib/igt_kms.c| 293 +--
lib/igt_kms.h| 59
tests/kms_atomic_interruptible.c | 12 +-
tests/kms_plane_lowres.c | 2 +-
tests/kms_rotation_crc.c | 4 +-
5 files changed, 160 insertions(+), 210 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 07d2074c2b1a..6e0052ebe7a7 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -192,11 +192,11 @@ const char
*igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
/*
* Retrieve all the properies specified in props_name and store them into
- * plane->atomic_props_plane.
+ * plane->props.
*/
static void
-igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
- int num_props, const char **prop_names)
+igt_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
+int num_props, const char **prop_names)
{
drmModeObjectPropertiesPtr props;
int i, j, fd;
@@ -214,7 +214,7 @@ igt_atomic_fill_plane_props(igt_display_t *display,
igt_plane_t *plane,
if (strcmp(prop->name, prop_names[j]) != 0)
continue;
- plane->atomic_props_plane[j] = props->props[i];
+ plane->props[j] = props->props[i];
break;
}
@@ -1659,7 +1659,6 @@ void igt_display_init(igt_display_t *display, int drm_fd)
drmModeRes *resources;
drmModePlaneRes *plane_resources;
int i;
- int is_atomic = 0;
memset(display, 0, sizeof(igt_display_t));
@@ -1679,7 +1678,9 @@ void igt_display_init(igt_display_t *display, int drm_fd)
igt_assert_f(display->pipes, "Failed to allocate memory for %d
pipes\n", display->n_pipes);
drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
- is_atomic = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1);
+ if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
+ display->is_atomic = 1;
+
plane_resources = drmModeGetPlaneResources(display->drm_fd);
igt_assert(plane_resources);
@@ -1776,19 +1777,19 @@ void igt_display_init(igt_display_t *display, int
drm_fd)
plane->type = type;
plane->pipe = pipe;
plane->drm_plane = drm_plane;
- plane->fence_fd = -1;
+ plane->values[IGT_PLANE_IN_FENCE_FD] = ~0ULL;
- if (is_atomic == 0) {
- display->is_atomic = 1;
- igt_atomic_fill_plane_props(display, plane,
IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
- }
+ igt_fill_plane_props(display, plane,
IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
get_plane_property(display->drm_fd, drm_plane->plane_id,
"rotation",
- &plane->rotation_property,
- &prop_value,
+ &plane->props[IGT_PLANE_ROTATION],
+ &plane->values[IGT_PLANE_ROTATION],
NULL);
- plane->rotation = (igt_rotation_t)prop_value;
+
+ /* Clear any residual framebuffer on first commit. */
+ igt_plane_set_prop_changed(plane, IGT_PLANE_FB_ID);
+ igt_plane_set_prop_changed(plane, IGT_PLANE_CRTC_ID);
}
/*
@@ -1805,9 +1806,6 @@ void igt_display_init(igt_display_t *display, int drm_fd)
pipe->n_planes = n_planes;
- for_each_plane_on_pipe(display, i, plane)
- plane->fb_changed = true;
-
pipe->mode_changed = true;
}
@@ -2070,18 +2068,7 @@ bool igt_pipe_get_property(igt_pipe_t *pipe, const char
*name,
static uint32_t igt_plane_get_fb_id(igt_plane_t *plane)
{
- if (plane->fb)
- return plane->fb->fb_id;
- else
- return 0;
-}
-
-static uint32_t igt_plane_get_fb_gem_handle(igt_plane_t *plane)
-{
- if (plane->fb)
- return plane->fb->gem_handle;
- else
- return 0;
+
Re: [Intel-gfx] [PATCH v2 08/11] drm/i915/execlists: Keep request->priority for its lifetime
Quoting Chris Wilson (2017-09-27 17:44:37) > With preemption, we will want to "unsubmit" a request, taking it back > from the hw and returning it to the priority sorted execution list. In > order to know where to insert it into that list, we need to remember > its adjust priority (which may change even as it was being executed). s/adjust/adjusted/ "This also affects reset for execlists as we are now unsubmitting the requests following the reset (rather than directly writing the ELSP for the inflight contexts). This turns reset into an accidental preemption point, as after the reset we may choose a different pair of contexts to submit to hw. GuC is not updated as this series doesn't add preemption to the GuC submission, and so it can keep benefiting from the early pruning of the DFS inside execlists_schedule() for a little longer. We also need to find a way of reducing the cost of that DFS..." ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt] igt/gem_exec_schedule: Detect too slow setup in deep-*
Using vgem as our cork for building the request queue limits us to 10s
of setup (or else the fence autoexpires and we start executing too
early). Add timeouts to the setup loops and SKIP if we cannot establish
the workload within 10s, the machine and driver is too slow to evaluate
the expected results.
To avoid the artificial limit of 10s requires lifting the dependency on
vgem and switching to sw_sync and explicit fencing. Just a matter of
plumbing!
Signed-off-by: Chris Wilson
---
tests/gem_exec_schedule.c | 37 +
1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/tests/gem_exec_schedule.c b/tests/gem_exec_schedule.c
index ec0fd9ef..961dc9e2 100644
--- a/tests/gem_exec_schedule.c
+++ b/tests/gem_exec_schedule.c
@@ -466,10 +466,12 @@ static void preempt_self(int fd, unsigned ring)
static void deep(int fd, unsigned ring)
{
#define XS 8
- const unsigned int nctx = MAX_PRIO + 1;
+ const unsigned int nctx = MAX_PRIO - MIN_PRIO;
const unsigned size = ALIGN(4*nctx, 4096);
+ struct timespec tv = {};
struct cork cork;
uint32_t result, dep[XS];
+ uint32_t expected = 0;
uint32_t *ptr;
uint32_t *ctx;
@@ -483,21 +485,48 @@ static void deep(int fd, unsigned ring)
for (int m = 0; m < XS; m ++)
dep[m] = gem_create(fd, size);
+ /* Bind the surfaces into each of the contexts */
+ {
+ struct drm_i915_gem_exec_object2 obj[XS + 2];
+ struct drm_i915_gem_execbuffer2 execbuf;
+ const uint32_t bbe = MI_BATCH_BUFFER_END;
+
+ memset(obj, 0, sizeof(obj));
+ for (int n = 0; n < XS; n++)
+ obj[n].handle = dep[n];
+ obj[XS].handle = result;
+ obj[XS+1].handle = gem_create(fd, 4096);
+ gem_write(fd, obj[XS+1].handle, 0, &bbe, sizeof(bbe));
+
+ memset(&execbuf, 0, sizeof(execbuf));
+ execbuf.buffers_ptr = to_user_pointer(obj);
+ execbuf.buffer_count = XS + 2;
+ execbuf.flags = ring;
+ for (int n = 0; n < nctx; n++) {
+ execbuf.rsvd1 = ctx[n];
+ gem_execbuf(fd, &execbuf);
+ }
+ gem_close(fd, obj[XS+1].handle);
+ gem_sync(fd, result);
+ }
+
plug(fd, &cork);
/* Create a deep dependency chain, with a few branches */
- for (int n = 0; n < nctx; n++)
+ for (int n = 0; n < nctx && igt_seconds_elapsed(&tv) < 8; n++)
for (int m = 0; m < XS; m++)
store_dword(fd, ctx[n], ring, dep[m], 4*n, ctx[n],
cork.handle, I915_GEM_DOMAIN_INSTRUCTION);
- for (int n = 0; n < nctx; n++) {
+ for (int n = 0; n < nctx && igt_seconds_elapsed(&tv) < 6; n++) {
for (int m = 0; m < XS; m++) {
store_dword(fd, ctx[n], ring, result, 4*n, ctx[n],
dep[m], 0);
store_dword(fd, ctx[n], ring, result, 4*m, ctx[n], 0,
I915_GEM_DOMAIN_INSTRUCTION);
}
+ expected = ctx[n];
}
unplug_show_queue(fd, &cork, ring);
+ igt_require(expected); /* too slow */
for (int n = 0; n < nctx; n++)
gem_context_destroy(fd, ctx[n]);
@@ -520,7 +549,7 @@ static void deep(int fd, unsigned ring)
/* No reordering due to PI on all contexts because of the common dep */
for (int m = 0; m < XS; m++)
- igt_assert_eq_u32(ptr[m], ctx[nctx - 1]);
+ igt_assert_eq_u32(ptr[m], expected);
munmap(ptr, size);
free(ctx);
--
2.14.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for lib/igt_kms: Convert properties to be more atomic-like. (rev5)
== Series Details == Series: lib/igt_kms: Convert properties to be more atomic-like. (rev5) URL : https://patchwork.freedesktop.org/series/30903/ State : success == Summary == IGT patchset tested on top of latest successful build 2885b10f99b4beeb046e75af8b8488c229f629d3 igt/gem_exec_schedule: Ignore set-priority failures on old kernels with latest DRM-Tip kernel build CI_DRM_3148 c1ed50121778 drm-tip: 2017y-09m-28d-04h-52m-47s UTC integration manifest fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:449s fi-bdw-gvtdvmtotal:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:481s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:424s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:527s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:279s fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:503s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:505s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:498s fi-cnl-y total:289 pass:259 dwarn:0 dfail:0 fail:3 skip:27 time:657s fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:418s fi-glk-1 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:568s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:425s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:406s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:435s fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:493s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:468s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:476s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:581s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:589s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:545s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:456s fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:761s fi-skl-6770hqtotal:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:492s fi-skl-gvtdvmtotal:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:475s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:569s fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:417s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_265/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Also discard second CRC on gen8+ platforms.
On Wed, 2017-09-27 at 17:20 -0700, Rodrigo Vivi wrote:
> One of the differences I spotted for GEN8+ platforms when
> compared to older platforms is that spec for BDW+ includes
> this sentence:
>
> "The first CRC done indication after CRC is first enabled is
> from only a partial frame, so it will not have the expected
> CRC result."
>
> This is an indication that on BDW+ platforms, by the time
> we receive the interrupt the CRC is not accurate yet for
> the full frame. That would be ok, because we are already
> skipping the first CRC for all platforms. However the comment
> on the code state that it is for some unknown reason. Also,
> on CHV (gen8 lp) we were already discarding the second CRC
> as well to make sure we have a reliable CRC on hand.
>
> So based on all ou tests and bugs it seems that it is not
> on CHV that needs to discard 2 first CRCs, but all BDW+
> platforms.
>
> Starting on SKL we have this CRC done bit (24), but the
> experiments around the use of this bit wasn't that stable
> as just discarding the second CRC. So, let's for now
> just move with CHV solution for all gen8+ platforms and
> make our CI a bit more stable.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102374
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101309
> Cc: Mika Kahola
> Signed-off-by: Rodrigo Vivi
Reviewed-by: Mika Kahola
> ---
> drivers/gpu/drm/i915/i915_irq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 0b7562135d1c..efd7827ff181 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1647,11 +1647,11 @@ static void
> display_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
> * bonkers. So let's just wait for the next vblank
> and read
> * out the buggy result.
> *
> - * On CHV sometimes the second CRC is bonkers as
> well, so
> + * On GEN8+ sometimes the second CRC is bonkers as
> well, so
> * don't trust that one either.
> */
> if (pipe_crc->skipped == 0 ||
> - (IS_CHERRYVIEW(dev_priv) && pipe_crc->skipped ==
> 1)) {
> + (INTEL_GEN(dev_priv) >= 8 && pipe_crc->skipped
> == 1)) {
> pipe_crc->skipped++;
> spin_unlock(&pipe_crc->lock);
> return;
--
Mika Kahola - Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10 1/9] drm/i915: Create GEM runtime resume helper and handle GEM suspend/resume errors
Quoting Sagar Arun Kamble (2017-09-27 10:30:31)
> These changes are preparation to handle GuC suspend/resume. Prepared
> helper i915_gem_runtime_resume to reinitialize suspended gem setup.
> Returning status from i915_gem_runtime_suspend and i915_gem_resume.
> This will be placeholder for handling any errors from uC suspend/resume
> in upcoming patches. Restructured the suspend/resume routines w.r.t setup
> creation and rollback order.
> This also fixes issue of ordering of i915_gem_runtime_resume with
> intel_runtime_pm_enable_interrupts.
>
> v2: Fixed return from intel_runtime_resume. (Michał Winiarski)
>
> v3: Not returning status from gem_runtime_resume. (Chris)
>
> Signed-off-by: Sagar Arun Kamble
> Cc: Chris Wilson
> Cc: Imre Deak
> Cc: Paulo Zanoni
> Cc: Rodrigo Vivi
> Cc: Michal Wajdeczko
> Cc: Michał Winiarski
> ---
> drivers/gpu/drm/i915/i915_drv.c | 22 +-
> drivers/gpu/drm/i915/i915_drv.h | 5 +++--
> drivers/gpu/drm/i915/i915_gem.c | 18 --
> 3 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 7056bb2..d0a710d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1655,6 +1655,7 @@ static int i915_suspend_switcheroo(struct drm_device
> *dev, pm_message_t state)
> static int i915_drm_resume(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> + struct pci_dev *pdev = dev_priv->drm.pdev;
> int ret;
>
> disable_rpm_wakeref_asserts(dev_priv);
> @@ -1666,7 +1667,9 @@ static int i915_drm_resume(struct drm_device *dev)
>
> intel_csr_ucode_resume(dev_priv);
>
> - i915_gem_resume(dev_priv);
> + ret = i915_gem_resume(dev_priv);
> + if (ret)
> + dev_err(&pdev->dev, "GEM resume failed\n");
I am still uneasy about this. Later on in the resume we try to reinit
the hw under the assumption that this earlier step succeeded.
Previously we have tried to make sure that if GEM fails, we do not leave
the display in an unusable state. Is that still true?
>
> i915_restore_state(dev_priv);
> intel_pps_unlock_regs_wa(dev_priv);
> @@ -2495,7 +2498,11 @@ static int intel_runtime_suspend(struct device *kdev)
> * We are safe here against re-faults, since the fault handler takes
> * an RPM reference.
> */
> - i915_gem_runtime_suspend(dev_priv);
> + ret = i915_gem_runtime_suspend(dev_priv);
> + if (ret) {
> + enable_rpm_wakeref_asserts(dev_priv);
> + return ret;
> + }
>
> intel_guc_suspend(dev_priv);
>
> @@ -2515,6 +2522,8 @@ static int intel_runtime_suspend(struct device *kdev)
> DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
> intel_runtime_pm_enable_interrupts(dev_priv);
>
> + intel_guc_resume(dev_priv);
> + i915_gem_runtime_resume(dev_priv);
> enable_rpm_wakeref_asserts(dev_priv);
>
> return ret;
> @@ -2596,13 +2605,6 @@ static int intel_runtime_resume(struct device *kdev)
> ret = vlv_resume_prepare(dev_priv, true);
> }
>
> - /*
> -* No point of rolling back things in case of an error, as the best
> -* we can do is to hope that things will still work (and disable RPM).
> -*/
This comment shouldn't be attached to the following gem init operations
as they cannot fail. If they could, we should be throwing a warning here
as this may result in a change of swizzling/fencing state as seen by
userspace and therefore graphical corruption.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10 2/9] drm/i915: Update GEM suspend/resume flows considering GuC and GEM fences
Quoting Sagar Arun Kamble (2017-09-27 10:30:32) > @@ -4607,13 +4611,14 @@ int i915_gem_resume(struct drm_i915_private *dev_priv) > > mutex_lock(&dev->struct_mutex); > i915_gem_restore_gtt_mappings(dev_priv); > + i915_gem_restore_fences(dev_priv); Seconded Michal's suggestion, otherwise it looks very clean. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled
== Series Details == Series: drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled URL : https://patchwork.freedesktop.org/series/31058/ State : warning == Summary == Series 31058v1 drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled https://patchwork.freedesktop.org/api/1.0/series/31058/revisions/1/mbox/ Test chamelium: Subgroup hdmi-crc-fast: pass -> DMESG-WARN (fi-skl-6700k) fdo#103019 Test gem_exec_suspend: Subgroup basic-s3: pass -> DMESG-WARN (fi-cfl-s) Subgroup basic-s4-devices: dmesg-warn -> PASS (fi-cfl-s) fdo#102294 +20 Test kms_frontbuffer_tracking: Subgroup basic: dmesg-warn -> PASS (fi-cfl-s) fdo#102374 Test kms_pipe_crc_basic: Subgroup nonblocking-crc-pipe-c: incomplete -> SKIP (fi-cfl-s) fdo#103022 Test drv_module_reload: Subgroup basic-reload: pass -> DMESG-WARN (fi-glk-1) fdo#102777 fdo#103019 https://bugs.freedesktop.org/show_bug.cgi?id=103019 fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294 fdo#102374 https://bugs.freedesktop.org/show_bug.cgi?id=102374 fdo#103022 https://bugs.freedesktop.org/show_bug.cgi?id=103022 fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777 fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:443s fi-bdw-gvtdvmtotal:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:475s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:425s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:519s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:280s fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:505s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:509s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:503s fi-cfl-s total:289 pass:256 dwarn:1 dfail:0 fail:0 skip:32 time:546s fi-cnl-y total:289 pass:259 dwarn:0 dfail:0 fail:3 skip:27 time:671s fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:420s fi-glk-1 total:289 pass:259 dwarn:1 dfail:0 fail:0 skip:29 time:568s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:425s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:405s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:440s fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:495s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:469s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:471s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:582s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:589s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:554s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:455s fi-skl-6700k total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:756s fi-skl-6770hqtotal:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:494s fi-skl-gvtdvmtotal:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:476s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:570s fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:429s c1ed501217782b35094792696924a1d22ee83c0f drm-tip: 2017y-09m-28d-04h-52m-47s UTC integration manifest 3b58c2e42208 drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5846/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10 3/9] drm/i915: Create uC runtime and system suspend/resume helpers
Quoting Sagar Arun Kamble (2017-09-27 10:30:33) > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 174a7c5..f79646b 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1698,6 +1698,16 @@ static int i915_drm_resume(struct drm_device *dev) > } > mutex_unlock(&dev->struct_mutex); > > + /* > +* NB: Currently we know that at the end of suspend we have done Full > +* GPU reset, Hence GuC is loaded again during i915_gem_init_hw. > +* Now, send action to GuC to resume back again as earlier call to > +* intel_uc_resume from i915_gem_resume would have done nothing. > +*/ When you say "GuC is loaded again during i915_gem_init_hw" are you thinking of the i915_reset call or i915_drm_resume call? We are using intel_gpu_reset() to do the full device reset at suspend/resume, so bypassing the i915_gem_init_hw from i915_reset. Could you clarify what you mean by "hence"? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10 6/9] drm/i915/guc: Check execbuf client to disable submission and don't depend on enable_guc_submission
Quoting Sagar Arun Kamble (2017-09-27 10:30:36) > We should check dependent state setup by enable path to run disable path > and not depend on the user parameters. i915_guc_submission_disable now > checks if execbuf client is setup and then goes ahead with disabling. > > Suggested-by: Chris Wilson > Signed-off-by: Sagar Arun Kamble > Cc: Michal Wajdeczko > Cc: Michał Winiarski Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for lib/igt_kms: Convert properties to be more atomic-like. (rev4)
== Series Details == Series: lib/igt_kms: Convert properties to be more atomic-like. (rev4) URL : https://patchwork.freedesktop.org/series/30903/ State : failure == Summary == Test gem_close_race: Subgroup basic-process: skip -> PASS (shard-hsw) Test kms_frontbuffer_tracking: Subgroup fbcpsr-2p-primscrn-cur-indfb-draw-pwrite: pass -> SKIP (shard-hsw) Subgroup psr-1p-offscren-pri-shrfb-draw-blt: pass -> SKIP (shard-hsw) Subgroup fbcpsr-1p-primscrn-cur-indfb-draw-mmap-wc: pass -> SKIP (shard-hsw) Subgroup fbcpsr-2p-primscrn-cur-indfb-draw-render: pass -> SKIP (shard-hsw) Subgroup fbcpsr-1p-primscrn-pri-shrfb-draw-blt: pass -> SKIP (shard-hsw) Subgroup fbcpsr-2p-scndscrn-spr-indfb-fullscreen: pass -> SKIP (shard-hsw) Subgroup fbc-2p-primscrn-pri-shrfb-draw-pwrite: pass -> SKIP (shard-hsw) Subgroup fbc-2p-primscrn-cur-indfb-draw-mmap-gtt: pass -> SKIP (shard-hsw) Subgroup fbc-1p-offscren-pri-indfb-draw-pwrite: skip -> PASS (shard-hsw) Subgroup psr-2p-primscrn-shrfb-plflip-blt: pass -> SKIP (shard-hsw) Subgroup psr-2p-primscrn-spr-indfb-draw-mmap-wc: pass -> SKIP (shard-hsw) Subgroup fbcpsr-2p-shrfb-fliptrack: pass -> SKIP (shard-hsw) Subgroup fbc-2p-primscrn-indfb-pgflip-blt: pass -> SKIP (shard-hsw) Subgroup fbcpsr-2p-scndscrn-spr-indfb-draw-mmap-cpu: pass -> SKIP (shard-hsw) Subgroup fbcpsr-1p-primscrn-pri-indfb-draw-pwrite: pass -> SKIP (shard-hsw) Subgroup fbcpsr-2p-scndscrn-spr-indfb-draw-mmap-gtt: pass -> SKIP (shard-hsw) Subgroup fbc-1p-primscrn-cur-indfb-onoff: skip -> PASS (shard-hsw) Subgroup fbc-1p-rte: skip -> PASS (shard-hsw) Subgroup psr-rgb565-draw-mmap-wc: pass -> SKIP (shard-hsw) Subgroup fbc-2p-scndscrn-cur-indfb-draw-mmap-gtt: pass -> SKIP (shard-hsw) Subgroup fbc-1p-offscren-pri-indfb-draw-mmap-wc: dmesg-warn -> PASS (shard-hsw) Subgroup psr-1p-offscren-pri-indfb-draw-mmap-cpu: pass -> SKIP (shard-hsw) Subgroup fbcpsr-rgb565-draw-mmap-cpu: pass -> SKIP (shard-hsw) Subgroup fbcpsr-1p-primscrn-pri-shrfb-draw-render: pass -> SKIP (shard-hsw) Subgroup psr-1p-primscrn-indfb-plflip-blt: pass -> SKIP (shard-hsw) Subgroup fbcpsr-modesetfrombusy: pass -> SKIP (shard-hsw) Subgroup psr-1p-primscrn-pri-shrfb-draw-mmap-wc: pass -> SKIP (shard-hsw) Subgroup fbcpsr-2p-scndscrn-pri-indfb-draw-mmap-wc: pass -> SKIP (shard-hsw) Subgroup psr-1p-rte: pass -> SKIP (shard-hsw) Subgroup psr-1p-primscrn-pri-shrfb-draw-mmap-gtt: pass -> SKIP (shard-hsw) Subgroup psr-1p-offscren-pri-shrfb-draw-mmap-wc: pass -> SKIP (shard-hsw) Subgroup fbc-1p-offscren-pri-shrfb-draw-mmap-gtt: skip -> PASS (shard-hsw) Subgroup psr-2p-primscrn-pri-shrfb-draw-render: pass -> SKIP (shard-hsw) Subgroup fbc-1p-primscrn-spr-indfb-draw-mmap-gtt: skip -> PASS (shard-hsw) Subgroup fbc-1p-primscrn-cur-indfb-draw-mmap-wc: skip -> PASS (shard-hsw) Subgroup fbc-1p-primscrn-shrfb-pgflip-blt: skip -> PASS (shard-hsw) Subgroup psr-2p-primscrn-spr-indfb-draw-mmap-cpu: pass -> SKIP (shard-hsw) Subgroup fbcpsr-1p-primscrn-cur-indfb-onoff: pass -> SKIP (shard-hsw) Subgroup fbcpsr-2p-primscrn-spr-indfb-draw-pwrite: pass -> SKIP (shard-hsw) Subgroup fbcpsr-1p-primscrn-pri-indfb-draw-mmap-gtt: pass -> SKIP (shard-hsw) Subgroup psr-2p-scndscrn-cur-indfb-draw-blt: pass -> SKIP (shard-hsw) Subgroup psr-1p-primscrn-spr-indfb-draw-mmap-wc: pass -> SKIP (shard-hsw) Subgroup fbc-2p-primscrn-pri-shrfb-draw-mmap-wc: pass -> SKIP (shard-hsw) Subgroup fbcpsr-rgb565-draw-mmap-gtt: pass
Re: [Intel-gfx] [PATCH v2 01/11] drm/i915/execlists: Move request unwinding to a separate function
On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote: > In the future, we will want to unwind requests following a preemption > point. This requires the same steps as for unwinding upon a reset, so > extract the existing code to a separate function for later use. > > Signed-off-by: Chris Wilson > Reviewed-by: Mika Kuoppala Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Allow optimized platform checks
On 27/09/2017 18:07, Patchwork wrote: == Series Details == Series: drm/i915: Allow optimized platform checks URL : https://patchwork.freedesktop.org/series/30982/ State : success == Summary == Series 30982v1 drm/i915: Allow optimized platform checks https://patchwork.freedesktop.org/api/1.0/series/30982/revisions/1/mbox/ Test drv_module_reload: Subgroup basic-reload: dmesg-warn -> PASS (fi-glk-1) fdo#102777 fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777 fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:440s fi-bdw-gvtdvmtotal:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:479s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:419s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:519s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:282s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:499s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:492s fi-cfl-s total:289 pass:223 dwarn:34 dfail:0 fail:0 skip:32 time:544s fi-cnl-y total:289 pass:258 dwarn:0 dfail:0 fail:4 skip:27 time:637s fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:420s fi-glk-1 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:568s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:424s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:405s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:437s fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:493s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:468s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:471s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:584s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:593s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:546s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:446s fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:745s fi-skl-6770hqtotal:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:485s fi-skl-gvtdvmtotal:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:473s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:567s fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:410s aa884e1abdf2ffb7db8c524fc6269280734e5145 drm-tip: 2017y-09m-27d-15h-39m-07s UTC integration manifest 6ef30a4343e1 drm/i915: Allow optimized platform checks Pushed, thanks for the review and acks. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10 9/9] drm/i915/guc: Fix GuC cleanup in unload path
Quoting Sagar Arun Kamble (2017-09-27 10:30:39)
> -void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
> +void intel_uc_cleanup(struct drm_i915_private *dev_priv)
> {
> guc_free_load_err_log(&dev_priv->guc);
>
> if (!i915_modparams.enable_guc_loading)
> return;
>
> - guc_disable_communication(&dev_priv->guc);
> -
> - if (i915_modparams.enable_guc_submission) {
> - gen9_disable_guc_interrupts(dev_priv);
> - i915_guc_submission_fini(dev_priv);
> - }
> -
> - i915_ggtt_disable_guc(dev_priv);
> + if (i915_modparams.enable_guc_submission)
> + i915_guc_submission_cleanup(dev_priv);
My preference would be for if (!guc->stage_desc_pool) return; inside
i915_guc_submission_cleanup().
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10 3/9] drm/i915: Create uC runtime and system suspend/resume helpers
On 9/28/2017 5:15 PM, Chris Wilson wrote: Quoting Sagar Arun Kamble (2017-09-27 10:30:33) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 174a7c5..f79646b 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1698,6 +1698,16 @@ static int i915_drm_resume(struct drm_device *dev) } mutex_unlock(&dev->struct_mutex); + /* +* NB: Currently we know that at the end of suspend we have done Full +* GPU reset, Hence GuC is loaded again during i915_gem_init_hw. +* Now, send action to GuC to resume back again as earlier call to +* intel_uc_resume from i915_gem_resume would have done nothing. +*/ When you say "GuC is loaded again during i915_gem_init_hw" are you thinking of the i915_reset call or i915_drm_resume call? We are using intel_gpu_reset() to do the full device reset at suspend/resume, so bypassing the i915_gem_init_hw from i915_reset. Could you clarify what you mean by "hence"? -Chris Hi Chris, I've updated the comments in the latest revision. Please review v12 patches from https://patchwork.freedesktop.org/series/30802/. Sorry I have pushed few more revisions in past few days as one of the igt fail due to patch change was not caught by trybot. From Latest patch: @@ -1698,6 +1698,18 @@static int i915_drm_resume(struct drm_device *dev) } mutex_unlock(&dev->struct_mutex); +/* +* FIXME: Currently we know that at the end of suspend we have done Full +* GPU reset and GuC is loaded again during i915_gem_init_hw. +* Now, send action to GuC to resume back again as earlier call to +* intel_uc_resume from i915_gem_resume would have done nothing. +* This needs to be skipped if GuC was not loaded during resume as +* intel_uc_resume would have been already called from i915_gem_resume. +*/ +ret = intel_uc_resume(dev_priv); +if (ret) +i915_gem_set_wedged(dev_priv); + I referred to intel_gpu_reset from i915_gem_suspend here. Thanks Sagar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 02/11] drm/i915/execlists: Cache the last priolist lookup
On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote:
> From: Michał Winiarski
>
> Avoid the repeated rbtree lookup for each request as we unwind them by
> tracking the last priolist.
>
> v2: Fix up my unhelpful suggestion of using default_priolist.
>
> Signed-off-by: Michał Winiarski
> Signed-off-by: Chris Wilson
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -358,25 +358,31 @@ static void unwind_wa_tail(struct drm_i915_gem_request
> *rq)
> static void unwind_incomplete_requests(struct intel_engine_cs *engine)
> {
> struct drm_i915_gem_request *rq, *rn;
> + struct i915_priolist *uninitialized_var(p);
> + int last_prio = INT_MAX;
>
> lockdep_assert_held(&engine->timeline->lock);
>
> list_for_each_entry_safe_reverse(rq, rn,
>&engine->timeline->requests,
>link) {
> - struct i915_priolist *p;
> -
> if (i915_gem_request_completed(rq))
> return;
>
> __i915_gem_request_unsubmit(rq);
> unwind_wa_tail(rq);
>
> - p = lookup_priolist(engine,
> - &rq->priotree,
> - rq->priotree.priority);
> - list_add(&rq->priotree.link,
> - &ptr_mask_bits(p, 1)->requests);
> + GEM_BUG_ON(rq->priotree.priority == INT_MAX);
This doesn't read aloud too logically when coming from the ring reset
codepath, at first like would seem like we're not allowing maximum
priority tasks to be unwinded (which only makes sense on the pre-empt
odepath). #define INVALID_PRIORITY INT_MAX might help
Reviewed-by: Joonas Lahtinen
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled
On Thu, Sep 28, 2017 at 11:18:27AM +0100, Chris Wilson wrote: > Quoting Imre Deak (2017-09-28 11:06:24) > > Only init / reset the display interrupts during power well enabling / > > disabling if the i915 interrupts are enabled. So far we did the > > init / reset during driver loading / resuming too, where > > initialization / enabling of the i915 interrupts happens only at a later > > point. This didn't cause a problem due to GEN8_MASTER_IRQ_CONTROL being > > cleared, but triggered gen3_assert_iir_is_zero() in GEN8_IRQ_INIT_NDX(). > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=102988 > > Cc: Chris Wilson > > Signed-off-by: Imre Deak > > Patch makes sense, so > Reviewed-by: Chris Wilson Thanks. > > There's an irq powerwell! When is it taken? We don't take it for GT as > far as I am aware (we should for execlists plus whenever we enable the > user interrupt). Should we? Only the display interrupt registers have a power well that we toggle from the driver (display power well/power well 2). The rest of interrupt regs including the GT ones are backed by a power well that the HW/DMC FW toggles automatically (always-on power well/power well 0). So no need to take it explicitly; there is the slow-down problem where the context restore during such enabling by DMC adds overhead, which will be solved by Tvrtko's WA. --Imre > -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled
Quoting Imre Deak (2017-09-28 12:59:16) > On Thu, Sep 28, 2017 at 11:18:27AM +0100, Chris Wilson wrote: > > Quoting Imre Deak (2017-09-28 11:06:24) > > > Only init / reset the display interrupts during power well enabling / > > > disabling if the i915 interrupts are enabled. So far we did the > > > init / reset during driver loading / resuming too, where > > > initialization / enabling of the i915 interrupts happens only at a later > > > point. This didn't cause a problem due to GEN8_MASTER_IRQ_CONTROL being > > > cleared, but triggered gen3_assert_iir_is_zero() in GEN8_IRQ_INIT_NDX(). > > > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=102988 > > > Cc: Chris Wilson > > > Signed-off-by: Imre Deak > > > > Patch makes sense, so > > Reviewed-by: Chris Wilson > > Thanks. > > > > > There's an irq powerwell! When is it taken? We don't take it for GT as > > far as I am aware (we should for execlists plus whenever we enable the > > user interrupt). Should we? > > Only the display interrupt registers have a power well that we toggle > from the driver (display power well/power well 2). The rest of interrupt > regs including the GT ones are backed by a power well that the HW/DMC FW > toggles automatically (always-on power well/power well 0). So no need to > take it explicitly; there is the slow-down problem where the context > restore during such enabling by DMC adds overhead, which will be solved > by Tvrtko's WA. For that w/a can we make the powerwell request more explicit? Instead of POWERWLL_MODESET maybe POWERWELL_GT_IRQ? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for igt/gem_exec_schedule: Detect too slow setup in deep-*
== Series Details == Series: igt/gem_exec_schedule: Detect too slow setup in deep-* URL : https://patchwork.freedesktop.org/series/31061/ State : success == Summary == IGT patchset tested on top of latest successful build 2885b10f99b4beeb046e75af8b8488c229f629d3 igt/gem_exec_schedule: Ignore set-priority failures on old kernels with latest DRM-Tip kernel build CI_DRM_3148 c1ed50121778 drm-tip: 2017y-09m-28d-04h-52m-47s UTC integration manifest fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:445s fi-bdw-gvtdvmtotal:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:480s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:423s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:522s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:281s fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:502s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:508s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:499s fi-cfl-s total:289 pass:256 dwarn:1 dfail:0 fail:0 skip:32 time:560s fi-cnl-y total:289 pass:258 dwarn:0 dfail:0 fail:4 skip:27 time:660s fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:423s fi-glk-1 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:567s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:425s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:402s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:435s fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:493s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:466s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:472s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:580s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:591s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:545s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:454s fi-skl-6700k total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:750s fi-skl-6770hqtotal:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:492s fi-skl-gvtdvmtotal:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:473s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:570s fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:419s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_266/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 02/11] drm/i915/execlists: Cache the last priolist lookup
Quoting Joonas Lahtinen (2017-09-28 12:59:01)
> On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote:
> > From: Michał Winiarski
> >
> > Avoid the repeated rbtree lookup for each request as we unwind them by
> > tracking the last priolist.
> >
> > v2: Fix up my unhelpful suggestion of using default_priolist.
> >
> > Signed-off-by: Michał Winiarski
> > Signed-off-by: Chris Wilson
>
>
>
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -358,25 +358,31 @@ static void unwind_wa_tail(struct
> > drm_i915_gem_request *rq)
> > static void unwind_incomplete_requests(struct intel_engine_cs *engine)
> > {
> > struct drm_i915_gem_request *rq, *rn;
> > + struct i915_priolist *uninitialized_var(p);
> > + int last_prio = INT_MAX;
> >
> > lockdep_assert_held(&engine->timeline->lock);
> >
> > list_for_each_entry_safe_reverse(rq, rn,
> >&engine->timeline->requests,
> >link) {
> > - struct i915_priolist *p;
> > -
> > if (i915_gem_request_completed(rq))
> > return;
> >
> > __i915_gem_request_unsubmit(rq);
> > unwind_wa_tail(rq);
> >
> > - p = lookup_priolist(engine,
> > - &rq->priotree,
> > - rq->priotree.priority);
> > - list_add(&rq->priotree.link,
> > - &ptr_mask_bits(p, 1)->requests);
> > + GEM_BUG_ON(rq->priotree.priority == INT_MAX);
>
> This doesn't read aloud too logically when coming from the ring reset
> codepath, at first like would seem like we're not allowing maximum
> priority tasks to be unwinded (which only makes sense on the pre-empt
> odepath). #define INVALID_PRIORITY INT_MAX might help
And adding the GEM_BUG_ON() to execlists_schedule() to make sure we
never try to set it as well.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 03/11] drm/i915/preempt: Fix WaEnablePreemptionGranularityControlByUMD
On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote: > From: Jeff McGee > > The WA applies to all production Gen9 and requires both enabling and > whitelisting of the per-context preemption control register. > > Signed-off-by: Jeff McGee > Signed-off-by: Michał Winiarski > Signed-off-by: Chris Wilson > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 12 +++- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > b/drivers/gpu/drm/i915/intel_engine_cs.c > index a28e2a864cf1..af3fe494a429 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -1077,7 +1077,9 @@ static int gen9_init_workarounds(struct intel_engine_cs > *engine) > return ret; > > /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl */ To have the record correct, this applies to CNL too. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gen8+: Init/reset display interrupts only if i915 IRQs are enabled
On Thu, Sep 28, 2017 at 01:01:34PM +0100, Chris Wilson wrote: > Quoting Imre Deak (2017-09-28 12:59:16) > > On Thu, Sep 28, 2017 at 11:18:27AM +0100, Chris Wilson wrote: > > > Quoting Imre Deak (2017-09-28 11:06:24) > > > > Only init / reset the display interrupts during power well enabling / > > > > disabling if the i915 interrupts are enabled. So far we did the > > > > init / reset during driver loading / resuming too, where > > > > initialization / enabling of the i915 interrupts happens only at a later > > > > point. This didn't cause a problem due to GEN8_MASTER_IRQ_CONTROL being > > > > cleared, but triggered gen3_assert_iir_is_zero() in GEN8_IRQ_INIT_NDX(). > > > > > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=102988 > > > > Cc: Chris Wilson > > > > Signed-off-by: Imre Deak > > > > > > Patch makes sense, so > > > Reviewed-by: Chris Wilson > > > > Thanks. > > > > > > > > There's an irq powerwell! When is it taken? We don't take it for GT as > > > far as I am aware (we should for execlists plus whenever we enable the > > > user interrupt). Should we? > > > > Only the display interrupt registers have a power well that we toggle > > from the driver (display power well/power well 2). The rest of interrupt > > regs including the GT ones are backed by a power well that the HW/DMC FW > > toggles automatically (always-on power well/power well 0). So no need to > > take it explicitly; there is the slow-down problem where the context > > restore during such enabling by DMC adds overhead, which will be solved > > by Tvrtko's WA. > > For that w/a can we make the powerwell request more explicit? Instead of > POWERWLL_MODESET maybe POWERWELL_GT_IRQ? Yes, sounds good to have a power domain for this purpose. --Imre > -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 04/11] drm/i915/preempt: Default to disabled mid-command preemption levels
On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote: > From: Michał Winiarski > > Supporting fine-granularity preemption levels may require changes in > userspace batch buffer programming. Therefore, we need to fallback to > safe default values, rather that use hardware defaults. Userspace is > still able to enable fine-granularity, since we're whitelisting the > register controlling it in WaEnablePreemptionGranularityControlByUMD. > > Signed-off-by: Michał Winiarski > Signed-off-by: Chris Wilson > + /* Supporting preemption with fine-granularity requires changes in the > + * batch buffer programming. Since we can't break old userspace, we > + * need to set our default preemption level to safe value. Userspace is > + * still able to use more fine-grained preemption levels, since in > + * WaEnablePreemptionGranularityControlByUMD we're whitelisting the > + * per-ctx register. As such, WaDisableMidCmdPreemption is not a real > + * HW workaround, but merely a way to start using preemption while > + * maintaining old contract with userspace. > + */ > + > + /* WaDisable3DMidCmdPreemption:skl,bxt,glk,cfl */ > + WA_CLR_BIT_MASKED(GEN8_CS_CHICKEN1, GEN9_PREEMPT_3D_OBJECT_LEVEL); > + > + /* WaDisableGPGPUMidCmdPreemption:skl,bxt,blk,cfl */ > + WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1, GEN9_PREEMPT_GPGPU_LEVEL_MASK, > + GEN9_PREEMPT_GPGPU_COMMAND_LEVEL); Lets avoid confusion by not inventing Wa names (It's not in the database at least). This also applies to pretty much any new HW, including CNL like the previous actual W/A. Other than that, this is the correct thing to do so drop the false name, and just leave this as a remark which references the other W/A. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 05/11] drm/i915/execlists: Distinguish the incomplete context notifies
On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote: > Let the listener know that the context we just scheduled out was not > complete, and will be scheduled back in at a later point. > > v2: Handle CONTEXT_STATUS_PREEMPTED in gvt by aliasing it to > CONTEXT_STATUS_OUT for the moment, gvt can expand upon the difference > later. > > Signed-off-by: Chris Wilson > Cc: "Zhenyu Wang" > Cc: "Wang, Zhi A" > Cc: Michał Winiarski > Cc: Mika Kuoppala > Cc: Tvrtko Ursulin Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 06/11] drm/i915: Introduce a preempt context
On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote:
> Add another perma-pinned context for using for preemption at any time.
> We cannot just reuse the existing kernel context, as first and foremost
> we need to ensure that we can preempt the kernel context itself, so
> require a distinct context id. Similar to the kernel context, we may
> want to interrupt execution and switch to the preempt context at any
> time, and so it needs to be permanently pinned and available.
>
> To compensate for yet another permanent allocation, we shrink the
> existing context and the new context by reducing their ringbuffer to the
> minimum.
>
> v2: Assert that we never allocate a request from the preemption context.
> v3: Limit perma-pin to engines that may preempt.
>
> Signed-off-by: Chris Wilson
> Reviewed-by: Michał Winiarski
> @@ -2250,8 +2251,9 @@ struct drm_i915_private {
> wait_queue_head_t gmbus_wait_queue;
>
> struct pci_dev *bridge_dev;
> - struct i915_gem_context *kernel_context;
> struct intel_engine_cs *engine[I915_NUM_ENGINES];
First to touch old code gets to add kerneldoc, just formulate what's in
the commit message into here.
> + struct i915_gem_context *kernel_context;
> + struct i915_gem_context *preempt_context;
> struct i915_vma *semaphore;
>
> struct drm_dma_handle *status_page_dmah;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 921ee369c74d..1518e110fd04 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -416,14 +416,29 @@ i915_gem_context_create_gvt(struct drm_device *dev)
> return ctx;
> }
>
> +static struct i915_gem_context *
> +create_kernel_context(struct drm_i915_private *i915, int prio)
I may vote for 'create_internal_context' because 'kernel context' is
quite an established term. But I've got no hard option, just gotta keep
the terminology coherent (eg. in the above requested kerneldoc).
> +{
> + struct i915_gem_context *ctx;
> +
> + ctx = i915_gem_create_context(i915, NULL);
> + if (IS_ERR(ctx))
> + return ctx;
> +
> + i915_gem_context_clear_bannable(ctx);
> + ctx->priority = prio;
> + ctx->ring_size = PAGE_SIZE;
> +
> + return ctx;
> +}
> +
> int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
> {
> struct i915_gem_context *ctx;
>
> /* Init should only be called once per module load. Eventually the
>* restriction on the context_disabled check can be loosened. */
Commit seems to be out of date now?
> @@ -441,23 +456,30 @@ int i915_gem_contexts_init(struct drm_i915_private
> *dev_priv)
> BUILD_BUG_ON(MAX_CONTEXT_HW_ID > INT_MAX);
> ida_init(&dev_priv->contexts.hw_ida);
>
> - ctx = i915_gem_create_context(dev_priv, NULL);
> + /* lowest priority; idle task */
> + ctx = create_kernel_context(dev_priv, I915_PRIORITY_MIN);
> if (IS_ERR(ctx)) {
> DRM_ERROR("Failed to create default global context (error
> %ld)\n",
> PTR_ERR(ctx));
> return PTR_ERR(ctx);
> }
>
> - /* For easy recognisablity, we want the kernel context to be 0 and then
> + /*
> + * For easy recognisablity, we want the kernel context to be 0 and then
>* all user contexts will have non-zero hw_id.
>*/
> GEM_BUG_ON(ctx->hw_id);
> -
> - i915_gem_context_clear_bannable(ctx);
> - ctx->priority = I915_PRIORITY_MIN; /* lowest priority; idle task */
> + GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
> dev_priv->kernel_context = ctx;
>
> - GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
> + /* highest priority; preempting task */
> + ctx = create_kernel_context(dev_priv, INT_MAX);
> + if (IS_ERR(ctx)) {
> + DRM_ERROR("Failed to create default preempt context (error
> %ld)\n",
> + PTR_ERR(ctx));
There's no onion teardown so kernel_context is not freed. Pleas add
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -587,6 +587,13 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
>
> lockdep_assert_held(&dev_priv->drm.struct_mutex);
>
> + /*
> + * Preempt contexts are reserved for exclusive use to inject a
> + * preemption context switch. They are never to be used for any trivial
> + * request!
> + */
> + GEM_BUG_ON(ctx == dev_priv->preempt_context);
Maybe check the prio here too.
> +
> /* ABI: Before userspace accesses the GPU (e.g. execbuffer), report
>* EIO if the GPU is already wedged.
>*/
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c
> b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 21e037fc21b7..02eb25ed1064 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -613,9 +613,22 @@ int intel_engine_init_common(struct intel_engine_cs
> *engine)
> if (IS_E
[Intel-gfx] ✗ Fi.CI.IGT: failure for lib/igt_kms: Convert properties to be more atomic-like. (rev5)
== Series Details == Series: lib/igt_kms: Convert properties to be more atomic-like. (rev5) URL : https://patchwork.freedesktop.org/series/30903/ State : failure == Summary == Test gem_exec_schedule: Subgroup pi-ringfull-blt: pass -> SKIP (shard-hsw) Subgroup fifo-bsd: skip -> PASS (shard-hsw) Subgroup preempt-bsd2: pass -> SKIP (shard-hsw) Subgroup in-order-bsd1: pass -> SKIP (shard-hsw) Subgroup promotion-bsd: pass -> SKIP (shard-hsw) Subgroup preempt-other-bsd1: pass -> SKIP (shard-hsw) Subgroup preempt-self-blt: pass -> SKIP (shard-hsw) Subgroup reorder-wide-vebox: pass -> SKIP (shard-hsw) Subgroup preempt-vebox: pass -> SKIP (shard-hsw) Subgroup promotion-bsd1: pass -> SKIP (shard-hsw) Subgroup deep-vebox: pass -> SKIP (shard-hsw) Subgroup reorder-wide-render: pass -> SKIP (shard-hsw) Subgroup in-order-vebox: pass -> SKIP (shard-hsw) Subgroup in-order-bsd: pass -> SKIP (shard-hsw) Subgroup fifo-bsd1: pass -> SKIP (shard-hsw) Subgroup pi-ringfull-bsd: pass -> SKIP (shard-hsw) Subgroup pi-ringfull-default: pass -> SKIP (shard-hsw) Subgroup out-order-blt: pass -> SKIP (shard-hsw) Subgroup preempt-blt: pass -> SKIP (shard-hsw) Subgroup reorder-wide-blt: pass -> SKIP (shard-hsw) Subgroup preempt-contexts-vebox: pass -> SKIP (shard-hsw) Subgroup preempt-other-vebox: pass -> SKIP (shard-hsw) Subgroup pi-ringfull-bsd2: pass -> SKIP (shard-hsw) Subgroup preempt-contexts-render: pass -> SKIP (shard-hsw) Subgroup preempt-self-bsd1: pass -> SKIP (shard-hsw) Subgroup deep-render: pass -> SKIP (shard-hsw) Subgroup preempt-contexts-bsd1: pass -> SKIP (shard-hsw) Subgroup promotion-blt: pass -> SKIP (shard-hsw) Subgroup out-order-render: pass -> SKIP (shard-hsw) Subgroup preempt-render: pass -> SKIP (shard-hsw) Subgroup preempt-other-blt: pass -> SKIP (shard-hsw) Subgroup in-order-bsd2: pass -> SKIP (shard-hsw) Subgroup deep-bsd1: pass -> SKIP (shard-hsw) Subgroup in-order-render: pass -> SKIP (shard-hsw) Subgroup out-order-vebox: pass -> SKIP (shard-hsw) Subgroup preempt-other-bsd2: pass -> SKIP (shard-hsw) Subgroup reorder-wide-bsd2: pass -> SKIP (shard-hsw) Subgroup wide-blt: pass -> SKIP (shard-hsw) Subgroup fifo-render: fail -> PASS (shard-hsw) Subgroup reorder-wide-bsd1: pass -> SKIP (shard-hsw) Subgroup out-order-bsd2: pass -> SKIP (shard-hsw) Subgroup pi-ringfull-bsd1: pass -> SKIP (shard-hsw) Subgroup deep-bsd: pass -> SKIP (shard-hsw) Subgroup wide-bsd1: fail -> SKIP (shard-hsw) Subgroup pi-ringfull-render: pass -> SKIP (shard-hsw) Subgroup deep-blt: pass -> SKIP (shard-hsw) Test kms_setmode: Subgroup invalid-clone-exclusive-crtc: skip -> PASS (shard-hsw) Subgroup basic: skip -> PASS (shard-hsw) fdo#99912 Subgroup invalid-clone-single-crtc: skip -> PASS (shard-hsw) Test kms_busy: Subgroup basic-flip-D: pass -> SKIP (shard-hsw) Subgroup extended-pageflip-hang-oldfb-render-C: skip -> PASS (shard-hsw) Subgroup extended-modeset-hang-newfb-render-D: pass -> SKIP (shard-hsw) Subgroup basic-flip-A: skip -> PASS (shard-hsw) Subgroup extended-modeset-hang-oldfb-with-reset-render-B: skip -> PASS (shard-hsw
Re: [Intel-gfx] [PATCH v2 06/11] drm/i915: Introduce a preempt context
Quoting Joonas Lahtinen (2017-09-28 13:32:09)
> On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote:
> > Add another perma-pinned context for using for preemption at any time.
> > We cannot just reuse the existing kernel context, as first and foremost
> > we need to ensure that we can preempt the kernel context itself, so
> > require a distinct context id. Similar to the kernel context, we may
> > want to interrupt execution and switch to the preempt context at any
> > time, and so it needs to be permanently pinned and available.
> >
> > To compensate for yet another permanent allocation, we shrink the
> > existing context and the new context by reducing their ringbuffer to the
> > minimum.
> >
> > v2: Assert that we never allocate a request from the preemption context.
> > v3: Limit perma-pin to engines that may preempt.
> >
> > Signed-off-by: Chris Wilson
> > Reviewed-by: Michał Winiarski
>
>
>
> > @@ -2250,8 +2251,9 @@ struct drm_i915_private {
> > wait_queue_head_t gmbus_wait_queue;
> >
> > struct pci_dev *bridge_dev;
> > - struct i915_gem_context *kernel_context;
> > struct intel_engine_cs *engine[I915_NUM_ENGINES];
>
> First to touch old code gets to add kerneldoc, just formulate what's in
> the commit message into here.
/* Nothing to see here, please move along. */
> > + struct i915_gem_context *kernel_context;
> > + struct i915_gem_context *preempt_context;
> > struct i915_vma *semaphore;
> >
> > struct drm_dma_handle *status_page_dmah;
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> > b/drivers/gpu/drm/i915/i915_gem_context.c
> > index 921ee369c74d..1518e110fd04 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -416,14 +416,29 @@ i915_gem_context_create_gvt(struct drm_device *dev)
> > return ctx;
> > }
> >
> > +static struct i915_gem_context *
> > +create_kernel_context(struct drm_i915_private *i915, int prio)
>
> I may vote for 'create_internal_context' because 'kernel context' is
> quite an established term. But I've got no hard option, just gotta keep
> the terminology coherent (eg. in the above requested kerneldoc).
The contexts return by this function pass i915_gem_context_is_kernel().
Atm, I think kernel_context is still the consistent terminology.
> > +{
> > + struct i915_gem_context *ctx;
> > +
> > + ctx = i915_gem_create_context(i915, NULL);
> > + if (IS_ERR(ctx))
> > + return ctx;
> > +
> > + i915_gem_context_clear_bannable(ctx);
> > + ctx->priority = prio;
> > + ctx->ring_size = PAGE_SIZE;
> > +
> > + return ctx;
> > +}
> > +
> > int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
> > {
> > struct i915_gem_context *ctx;
> >
> > /* Init should only be called once per module load. Eventually the
> >* restriction on the context_disabled check can be loosened. */
>
> Commit seems to be out of date now?
Got to keep some humour around.
> > +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> > @@ -587,6 +587,13 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
> >
> > lockdep_assert_held(&dev_priv->drm.struct_mutex);
> >
> > + /*
> > + * Preempt contexts are reserved for exclusive use to inject a
> > + * preemption context switch. They are never to be used for any
> > trivial
> > + * request!
> > + */
> > + GEM_BUG_ON(ctx == dev_priv->preempt_context);
>
> Maybe check the prio here too.
More a task for ->schedule().
>
> > +
> > /* ABI: Before userspace accesses the GPU (e.g. execbuffer), report
> >* EIO if the GPU is already wedged.
> >*/
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c
> > b/drivers/gpu/drm/i915/intel_engine_cs.c
> > index 21e037fc21b7..02eb25ed1064 100644
> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > @@ -613,9 +613,22 @@ int intel_engine_init_common(struct intel_engine_cs
> > *engine)
> > if (IS_ERR(ring))
> > return PTR_ERR(ring);
> >
> > + /*
> > + * Similarly the preempt context must always be available so that
> > + * we can interrupt the engine at any time.
> > + */
> > + if (INTEL_INFO(engine->i915)->has_logical_ring_preemption) {
> > + ring = engine->context_pin(engine,
> > +engine->i915->preempt_context);
> > + if (IS_ERR(ring)) {
> > + ret = PTR_ERR(ring);
> > + goto err_unpin_kernel;
> > + }
> > + }
>
> Maybe add helper for the internal/kernel_context_pin and _free too,
> it's unnecessary code duplication.
What's the duplication? I'm not clear on what may be saved.
engine->context_pin()
engine->context_unpin()
??
The cleanup path I have more sympathy for, I considered doing it. But I
keep getting side tracked by the desire to eliminate the contex
Re: [Intel-gfx] [PATCH v2 07/11] drm/i915/execlists: Move bdw GPGPU w/a to emit_bb
On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote: > Move the re-enabling of MI arbitration from a per-bb w/a buffer to the > emission of the batch buffer itself. > > Signed-off-by: Chris Wilson There's something about 64b addressing mode requiring disabling ARB during the BB_START on pre-BDW machines, but shouldn't affect production machines. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 08/11] drm/i915/execlists: Keep request->priority for its lifetime
On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote: > With preemption, we will want to "unsubmit" a request, taking it back > from the hw and returning it to the priority sorted execution list. In > order to know where to insert it into that list, we need to remember > its adjust priority (which may change even as it was being executed). > > Signed-off-by: Chris Wilson > Cc: Michal Winiarski There seems to be a whole lot of discussion around it, but this patch itself does what it describes. Reviewed-by: Joonas Lahtinen The loop can be optimized once it's working. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for lib/igt_kms: Convert properties to be more atomic-like. (rev4)
== Series Details == Series: lib/igt_kms: Convert properties to be more atomic-like. (rev4) URL : https://patchwork.freedesktop.org/series/30903/ State : failure == Summary == Test kms_cursor_legacy: Subgroup cursorA-vs-flipA-atomic-transitions: fail -> PASS (shard-hsw) fdo#102723 Test prime_mmap: Subgroup test_userptr: pass -> DMESG-WARN (shard-hsw) fdo#102939 Test kms_atomic_interruptible: Subgroup atomic-setmode: pass -> FAIL (shard-hsw) Subgroup legacy-setmode: pass -> FAIL (shard-hsw) Test kms_rotation_crc: Subgroup sprite-rotation-180-flip: fail -> PASS (shard-hsw) fdo#102691 Test kms_plane_lowres: Subgroup pipe-A-tiling-x: pass -> FAIL (shard-hsw) Subgroup pipe-A-tiling-none: pass -> FAIL (shard-hsw) Test gem_exec_reloc: Subgroup basic-write-read-noreloc: incomplete -> PASS (shard-hsw) fdo#102332 Test gem_eio: Subgroup in-flight: pass -> DMESG-WARN (shard-hsw) fdo#102886 +2 Test perf: Subgroup blocking: pass -> FAIL (shard-hsw) fdo#102252 fdo#102723 https://bugs.freedesktop.org/show_bug.cgi?id=102723 fdo#102939 https://bugs.freedesktop.org/show_bug.cgi?id=102939 fdo#102691 https://bugs.freedesktop.org/show_bug.cgi?id=102691 fdo#102332 https://bugs.freedesktop.org/show_bug.cgi?id=102332 fdo#102886 https://bugs.freedesktop.org/show_bug.cgi?id=102886 fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 shard-hswtotal:2429 pass:1327 dwarn:6 dfail:0 fail:13 skip:1083 time:9878s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_264/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for lib/igt_kms: Convert properties to be more atomic-like. (rev5)
== Series Details == Series: lib/igt_kms: Convert properties to be more atomic-like. (rev5) URL : https://patchwork.freedesktop.org/series/30903/ State : failure == Summary == Test gem_eio: Subgroup in-flight-contexts: dmesg-warn -> PASS (shard-hsw) fdo#102886 +1 Test kms_atomic_interruptible: Subgroup atomic-setmode: pass -> FAIL (shard-hsw) Subgroup legacy-setmode: pass -> FAIL (shard-hsw) Test kms_rotation_crc: Subgroup sprite-rotation-180-flip: fail -> PASS (shard-hsw) fdo#102691 Test kms_busy: Subgroup extended-modeset-hang-oldfb-with-reset-render-A: pass -> DMESG-WARN (shard-hsw) fdo#102249 Test gem_exec_reloc: Subgroup basic-write-read-noreloc: incomplete -> PASS (shard-hsw) fdo#102332 Test kms_cursor_legacy: Subgroup cursorA-vs-flipA-atomic-transitions: fail -> PASS (shard-hsw) fdo#102723 fdo#102886 https://bugs.freedesktop.org/show_bug.cgi?id=102886 fdo#102691 https://bugs.freedesktop.org/show_bug.cgi?id=102691 fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249 fdo#102332 https://bugs.freedesktop.org/show_bug.cgi?id=102332 fdo#102723 https://bugs.freedesktop.org/show_bug.cgi?id=102723 shard-hswtotal:2429 pass:1331 dwarn:5 dfail:0 fail:10 skip:1083 time:9895s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_265/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 09/11] drm/i915: Expand I915_PARAM_HAS_SCHEDULER into a capability bitmask
On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote:
> In the next few patches, we wish to enable different features for the
> scheduler, some which may subtlety change ABI (e.g. allow requests to be
> reordered under different circumstances). So we need to make sure
> userspace is cognizant of the changes (if they care), by which we employ
> the usual method of a GETPARAM. We already have an
> I915_PARAM_HAS_SCHEDULER (which notes the existing ability to reorder
> requests to avoid bubbles), and now we wish to extend that to be a
> bitmask to describe the different capabilities implemented.
Please link the last Mesa series here, as this is the patch bringing in
the ABI.
> Signed-off-by: Chris Wilson
> +++ b/include/uapi/drm/i915_drm.h
> @@ -397,10 +397,17 @@ typedef struct drm_i915_irq_wait {
> #define I915_PARAM_MIN_EU_IN_POOL 39
> #define I915_PARAM_MMAP_GTT_VERSION 40
>
> -/* Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
> +/*
> + * Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
> * priorities and the driver will attempt to execute batches in priority
> order.
> + * The param returns a capability bitmask, nonzero implies that the scheduler
> + * is enabled, with different features present according to the mask.
> */
> #define I915_PARAM_HAS_SCHEDULER 41
> +#define I915_SCHEDULER_CAP_ENABLED (1ul << 0)
> +#define I915_SCHEDULER_CAP_PRIORITY(1ul << 1)
> +#define I915_SCHEDULER_CAP_PREEMPTION (1ul << 2)
There seems to be the occasional BIT() hanging under uapi (I seem to
have added the first for i915.h). So maybe we need to revert that or
convert these.
Either way;
Reviewed-by: Joonas Lahtinen
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 09/11] drm/i915: Expand I915_PARAM_HAS_SCHEDULER into a capability bitmask
Quoting Joonas Lahtinen (2017-09-28 14:07:35)
> On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote:
> > In the next few patches, we wish to enable different features for the
> > scheduler, some which may subtlety change ABI (e.g. allow requests to be
> > reordered under different circumstances). So we need to make sure
> > userspace is cognizant of the changes (if they care), by which we employ
> > the usual method of a GETPARAM. We already have an
> > I915_PARAM_HAS_SCHEDULER (which notes the existing ability to reorder
> > requests to avoid bubbles), and now we wish to extend that to be a
> > bitmask to describe the different capabilities implemented.
>
> Please link the last Mesa series here, as this is the patch bringing in
> the ABI.
>
> > Signed-off-by: Chris Wilson
>
>
>
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -397,10 +397,17 @@ typedef struct drm_i915_irq_wait {
> > #define I915_PARAM_MIN_EU_IN_POOL 39
> > #define I915_PARAM_MMAP_GTT_VERSION 40
> >
> > -/* Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
> > +/*
> > + * Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
> > * priorities and the driver will attempt to execute batches in priority
> > order.
> > + * The param returns a capability bitmask, nonzero implies that the
> > scheduler
> > + * is enabled, with different features present according to the mask.
> > */
> > #define I915_PARAM_HAS_SCHEDULER 41
> > +#define I915_SCHEDULER_CAP_ENABLED (1ul << 0)
> > +#define I915_SCHEDULER_CAP_PRIORITY(1ul << 1)
> > +#define I915_SCHEDULER_CAP_PREEMPTION (1ul << 2)
>
> There seems to be the occasional BIT() hanging under uapi (I seem to
> have added the first for i915.h). So maybe we need to revert that or
> convert these.
We don't have BIT() in uapi yet, afaik. Plenty of places where we could
benefit.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10 9/9] drm/i915/guc: Fix GuC cleanup in unload path
On 9/28/2017 5:25 PM, Chris Wilson wrote:
Quoting Sagar Arun Kamble (2017-09-27 10:30:39)
-void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
+void intel_uc_cleanup(struct drm_i915_private *dev_priv)
{
guc_free_load_err_log(&dev_priv->guc);
if (!i915_modparams.enable_guc_loading)
return;
- guc_disable_communication(&dev_priv->guc);
-
- if (i915_modparams.enable_guc_submission) {
- gen9_disable_guc_interrupts(dev_priv);
- i915_guc_submission_fini(dev_priv);
- }
-
- i915_ggtt_disable_guc(dev_priv);
+ if (i915_modparams.enable_guc_submission)
+ i915_guc_submission_cleanup(dev_priv);
My preference would be for if (!guc->stage_desc_pool) return; inside
i915_guc_submission_cleanup().
-Chris
Yes. I have taken similar input in the latest patch -
https://patchwork.freedesktop.org/patch/179405/
In i915_guc_submission_cleanup we can cover destroy of stage_ids and
stage_desc_pool based on check you have suggested.
guc_ads_destroy is always required data so should we link with
stage_desc_pool check?
intel_guc_log is optional so destroy need to be made dependent on
guc->log.vma
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10 6/9] drm/i915/guc: Check execbuf client to disable submission and don't depend on enable_guc_submission
On 9/28/2017 5:17 PM, Chris Wilson wrote: Quoting Sagar Arun Kamble (2017-09-27 10:30:36) We should check dependent state setup by enable path to run disable path and not depend on the user parameters. i915_guc_submission_disable now checks if execbuf client is setup and then goes ahead with disabling. Suggested-by: Chris Wilson Signed-off-by: Sagar Arun Kamble Cc: Michal Wajdeczko Cc: Michał Winiarski Reviewed-by: Chris Wilson -Chris Thanks for the review Chris. I have this change as part of latest patch - https://patchwork.freedesktop.org/patch/179405/ Could you please review this patch. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10 2/9] drm/i915: Update GEM suspend/resume flows considering GuC and GEM fences
On 9/28/2017 5:10 PM, Chris Wilson wrote: Quoting Sagar Arun Kamble (2017-09-27 10:30:32) @@ -4607,13 +4611,14 @@ int i915_gem_resume(struct drm_i915_private *dev_priv) mutex_lock(&dev->struct_mutex); i915_gem_restore_gtt_mappings(dev_priv); + i915_gem_restore_fences(dev_priv); Seconded Michal's suggestion, otherwise it looks very clean. -Chris This has been updated in the following patch in the latest revision of series: https://patchwork.freedesktop.org/patch/179403/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for igt/gem_exec_schedule: Detect too slow setup in deep-*
== Series Details == Series: igt/gem_exec_schedule: Detect too slow setup in deep-* URL : https://patchwork.freedesktop.org/series/31061/ State : success == Summary == Test gem_eio: Subgroup in-flight: pass -> DMESG-WARN (shard-hsw) fdo#102886 +2 Test gem_exec_reloc: Subgroup basic-write-read-noreloc: incomplete -> PASS (shard-hsw) fdo#102332 Test kms_flip: Subgroup plain-flip-fb-recreate: pass -> FAIL (shard-hsw) fdo#102504 Subgroup wf_vblank-vs-modeset: pass -> DMESG-WARN (shard-hsw) fdo#102614 Test perf: Subgroup blocking: pass -> FAIL (shard-hsw) fdo#102252 Test kms_cursor_legacy: Subgroup cursorA-vs-flipA-atomic-transitions: fail -> PASS (shard-hsw) fdo#102723 Test kms_setmode: Subgroup basic: pass -> FAIL (shard-hsw) fdo#99912 Test kms_atomic_transition: Subgroup plane-all-transition-nonblocking: pass -> FAIL (shard-hsw) fdo#102671 fdo#102886 https://bugs.freedesktop.org/show_bug.cgi?id=102886 fdo#102332 https://bugs.freedesktop.org/show_bug.cgi?id=102332 fdo#102504 https://bugs.freedesktop.org/show_bug.cgi?id=102504 fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 fdo#102723 https://bugs.freedesktop.org/show_bug.cgi?id=102723 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 fdo#102671 https://bugs.freedesktop.org/show_bug.cgi?id=102671 shard-hswtotal:2429 pass:1327 dwarn:6 dfail:0 fail:13 skip:1083 time:9894s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_266/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 08/11] drm/i915/execlists: Keep request->priority for its lifetime
On Thu, Sep 28, 2017 at 11:09:14AM +, Chris Wilson wrote: > Quoting Chris Wilson (2017-09-27 17:44:37) > > With preemption, we will want to "unsubmit" a request, taking it back > > from the hw and returning it to the priority sorted execution list. In > > order to know where to insert it into that list, we need to remember > > its adjust priority (which may change even as it was being executed). > s/adjust/adjusted/ > > "This also affects reset for execlists as we are now unsubmitting the > requests following the reset (rather than directly writing the ELSP for > the inflight contexts). This turns reset into an accidental preemption > point, as after the reset we may choose a different pair of contexts to > submit to hw. > > GuC is not updated as this series doesn't add preemption to the GuC > submission, and so it can keep benefiting from the early pruning of the > DFS inside execlists_schedule() for a little longer. We also need to > find a way of reducing the cost of that DFS..." > Reviewed-by: Michał Winiarski -Michał ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v10 1/9] drm/i915: Create GEM runtime resume helper and handle GEM suspend/resume errors
On 9/28/2017 5:07 PM, Chris Wilson wrote:
Quoting Sagar Arun Kamble (2017-09-27 10:30:31)
These changes are preparation to handle GuC suspend/resume. Prepared
helper i915_gem_runtime_resume to reinitialize suspended gem setup.
Returning status from i915_gem_runtime_suspend and i915_gem_resume.
This will be placeholder for handling any errors from uC suspend/resume
in upcoming patches. Restructured the suspend/resume routines w.r.t setup
creation and rollback order.
This also fixes issue of ordering of i915_gem_runtime_resume with
intel_runtime_pm_enable_interrupts.
v2: Fixed return from intel_runtime_resume. (Michał Winiarski)
v3: Not returning status from gem_runtime_resume. (Chris)
Signed-off-by: Sagar Arun Kamble
Cc: Chris Wilson
Cc: Imre Deak
Cc: Paulo Zanoni
Cc: Rodrigo Vivi
Cc: Michal Wajdeczko
Cc: Michał Winiarski
---
drivers/gpu/drm/i915/i915_drv.c | 22 +-
drivers/gpu/drm/i915/i915_drv.h | 5 +++--
drivers/gpu/drm/i915/i915_gem.c | 18 --
3 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7056bb2..d0a710d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1655,6 +1655,7 @@ static int i915_suspend_switcheroo(struct drm_device
*dev, pm_message_t state)
static int i915_drm_resume(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ struct pci_dev *pdev = dev_priv->drm.pdev;
int ret;
disable_rpm_wakeref_asserts(dev_priv);
@@ -1666,7 +1667,9 @@ static int i915_drm_resume(struct drm_device *dev)
intel_csr_ucode_resume(dev_priv);
- i915_gem_resume(dev_priv);
+ ret = i915_gem_resume(dev_priv);
+ if (ret)
+ dev_err(&pdev->dev, "GEM resume failed\n");
I am still uneasy about this. Later on in the resume we try to reinit
the hw under the assumption that this earlier step succeeded.
Previously we have tried to make sure that if GEM fails, we do not leave
the display in an unusable state. Is that still true?
As part of gem_resume we are resuming GuC and if that fails we are
declaring gem wedged.
Will that be okay or we ignore the GuC resume failure and go ahead with
reinit?
i915_restore_state(dev_priv);
intel_pps_unlock_regs_wa(dev_priv);
@@ -2495,7 +2498,11 @@ static int intel_runtime_suspend(struct device *kdev)
* We are safe here against re-faults, since the fault handler takes
* an RPM reference.
*/
- i915_gem_runtime_suspend(dev_priv);
+ ret = i915_gem_runtime_suspend(dev_priv);
+ if (ret) {
+ enable_rpm_wakeref_asserts(dev_priv);
+ return ret;
+ }
intel_guc_suspend(dev_priv);
@@ -2515,6 +2522,8 @@ static int intel_runtime_suspend(struct device *kdev)
DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
intel_runtime_pm_enable_interrupts(dev_priv);
+ intel_guc_resume(dev_priv);
+ i915_gem_runtime_resume(dev_priv);
enable_rpm_wakeref_asserts(dev_priv);
return ret;
@@ -2596,13 +2605,6 @@ static int intel_runtime_resume(struct device *kdev)
ret = vlv_resume_prepare(dev_priv, true);
}
- /*
-* No point of rolling back things in case of an error, as the best
-* we can do is to hope that things will still work (and disable RPM).
-*/
This comment shouldn't be attached to the following gem init operations
as they cannot fail. If they could, we should be throwing a warning here
as this may result in a change of swizzling/fencing state as seen by
userspace and therefore graphical corruption.
-Chris
Ok. Will remove this comment.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt] tests/gem_workarounds: Skip write only registers
We have no means to check write only registers as
this would need access through context image. For now we
know that cnl has a one such register, 0xe5f0 which is used
to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting
the context image without and with workaround applied:
0xa840: 0xe5f0 0x0800
0xa840: 0xe5f0 0x0820
we can conclude that the workaround setup is working right
in this particular case.
Add a write only list and add register 0xe5f0 into this list.
This is a temporary solution until a more capable context image
checker emerges.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
Cc: Oscar Mateo
Cc: Chris Wilson
Cc: Rodrigo Vivi
Signed-off-by: Mika Kuoppala
---
tests/gem_workarounds.c | 30 +-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
index d6641bd5..03b4dc6c 100644
--- a/tests/gem_workarounds.c
+++ b/tests/gem_workarounds.c
@@ -29,6 +29,8 @@
#include
+static int gen;
+
enum operation {
GPU_RESET,
SUSPEND_RESUME,
@@ -41,6 +43,13 @@ struct intel_wa_reg {
uint32_t mask;
};
+static struct write_only_list {
+ unsigned int gen;
+ uint32_t addr;
+} wo_list[] = {
+ { 10, 0xE5F0 } /* WaForceContextSaveRestoreNonCoherent:cnl */
+};
+
static struct intel_wa_reg *wa_regs;
static int num_wa_regs;
@@ -64,6 +73,21 @@ static void test_suspend_resume(void)
igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
}
+static bool write_only(const uint32_t addr)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(wo_list); i++) {
+ if (gen == wo_list[i].gen &&
+ addr == wo_list[i].addr) {
+ igt_info("Skipping check for 0x%x due to write only\n",
addr);
+ return true;
+ }
+ }
+
+ return false;
+}
+
static int workaround_fail_count(void)
{
int i, fail_count = 0;
@@ -85,6 +109,9 @@ static int workaround_fail_count(void)
wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
val, ok ? "OK" : "FAIL");
+ if (write_only(wa_regs[i].addr))
+ continue;
+
if (!ok) {
igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
wa_regs[i].addr, wa_regs[i].value,
@@ -124,7 +151,6 @@ igt_main
{
igt_fixture {
int device = drm_open_driver_master(DRIVER_INTEL);
- const int gen = intel_gen(intel_get_drm_devid(device));
struct pci_device *pci_dev;
FILE *file;
char *line = NULL;
@@ -133,6 +159,8 @@ igt_main
igt_require_gem(device);
+ gen = intel_gen(intel_get_drm_devid(device));
+
pci_dev = intel_get_pci_device();
igt_require(pci_dev);
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt] tests/gem_workarounds: Skip write only registers
Quoting Mika Kuoppala (2017-09-28 14:45:06) > We have no means to check write only registers as > this would need access through context image. For now we > know that cnl has a one such register, 0xe5f0 which is used > to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting > the context image without and with workaround applied: > > 0xa840: 0xe5f0 0x0800 > 0xa840: 0xe5f0 0x0820 > > we can conclude that the workaround setup is working right > in this particular case. > > Add a write only list and add register 0xe5f0 into this list. > This is a temporary solution until a more capable context image > checker emerges. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943 > Cc: Oscar Mateo > Cc: Chris Wilson > Cc: Rodrigo Vivi > Signed-off-by: Mika Kuoppala It makes sense given the discovery of the w/o register. I was thinking of how we could communicate these through the debugfs, but I think any changes in direction involve pulling this test into the kernel. So, this appears to be the pragmatic solution. Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 10/11] drm/i915/execlists: Preemption!
On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote:
> When we write to ELSP, it triggers a context preemption at the earliest
> arbitration point (3DPRIMITIVE, some PIPECONTROLs, a few other
> operations and the explicit MI_ARB_CHECK). If this is to the same
> context, it triggers a LITE_RESTORE where the RING_TAIL is merely
> updated (used currently to chain requests from the same context
> together, avoiding bubbles). However, if it is to a different context, a
> full context-switch is performed and it will start to execute the new
> context saving the image of the old for later execution.
>
> Previously we avoided preemption by only submitting a new context when
> the old was idle. But now we wish embrace it, and if the new request has
> a higher priority than the currently executing request, we write to the
> ELSP regardless, thus triggering preemption, but we tell the GPU to
> switch to our special preemption context (not the target). In the
> context-switch interrupt handler, we know that the previous contexts
> have finished execution and so can unwind all the incomplete requests
> and compute the new highest priority request to execute.
>
> It would be feasible to avoid the switch-to-idle intermediate by
> programming the ELSP with the target context. The difficulty is in
> tracking which request that should be whilst maintaining the dependency
> change, the error comes in with coalesced requests. As we only track the
> most recent request and its priority, we may run into the issue of being
> tricked in preempting a high priority request that was followed by a
> low priority request from the same context (e.g. for PI);
"followed" is bit ambiguous here, depending on how you view the
ordering, wall time or ports.
> worse still
> that earlier request may be our own dependency and the order then broken
> by preemption. By injecting the switch-to-idle and then recomputing the
> priority queue, we avoid the issue with tracking in-flight coalesced
> requests. Having tried the preempt-to-busy approach, and failed to find
> a way around the coalesced priority issue, Michal's original proposal to
> inject an idle context (based on handling GuC preemption) succeeds.
>
> The current heuristic for deciding when to preempt are only if the new
> request is of higher priority, and has the privileged priority of
> greater than 0. Note that the scheduler remains unfair!
>
> v2: Disable for gen8 (bdw/bsw) as we need additional w/a for GPGPU.
> Since, the feature is now conditional and not always available when we
> have a scheduler, make it known via the HAS_SCHEDULER GETPARAM (now a
> capability mask).
>
> Suggested-by: Michal Winiarski
> Signed-off-by: Chris Wilson
> Cc: Michal Winiarski
> Cc: Tvrtko Ursulin
> Cc: Arkadiusz Hiler
> Cc: Mika Kuoppala
> Cc: Ben Widawsky
> Cc: Zhenyu Wang
> Cc: Zhi Wang
> @@ -489,26 +489,44 @@ static void port_assign(struct execlist_port *port,
> port_set(port, port_pack(i915_gem_request_get(rq), port_count(port)));
> }
>
> +static void inject_preempt_context(struct intel_engine_cs *engine)
> +{
> + struct intel_context *ce =
> + &engine->i915->preempt_context->engine[engine->id];
> + u32 __iomem *elsp =
> + engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine));
engine_elsp() helper or so?
> + unsigned int n;
> +
> + GEM_BUG_ON(engine->i915->preempt_context->hw_id != PREEMPT_ID);
I think this could/should be done way earlier?
> +
> + memset(ce->ring->vaddr + ce->ring->tail, 0, 8);
> + ce->ring->tail += 8;
> + ce->ring->tail &= (ce->ring->size - 1);
> + ce->lrc_reg_state[CTX_RING_TAIL+1] = ce->ring->tail;
An awful lot of pre-expectations here, would be shame if somebody
documented them.
> +
> + for (n = execlists_num_ports(&engine->execlists); --n; ) {
This is fine detail compared to the other loop, "<=" vs "<" (or maybe
even <= -1) would make a more clear distinction, but I'm not arguing.
> + writel(0, elsp);
> + writel(0, elsp);
> + }
> + writel(upper_32_bits(ce->lrc_desc), elsp);
> + writel(lower_32_bits(ce->lrc_desc), elsp);
Could also be elsp_write inline helper.
> @@ -696,7 +746,7 @@ static void intel_lrc_irq_handler(unsigned long data)
> {
> struct intel_engine_cs * const engine = (struct intel_engine_cs *)data;
> struct intel_engine_execlists * const execlists = &engine->execlists;
> - struct execlist_port *port = execlists->port;
> + struct execlist_port * const port = execlists->port;
> struct drm_i915_private *dev_priv = engine->i915;
>
> /* We can skip acquiring intel_runtime_pm_get() here as it was taken
> @@ -781,6 +831,23 @@ static void intel_lrc_irq_handler(unsigned long data)
> if (!(status & GEN8_CTX_STATUS_COMPLETED_MASK))
> continue;
>
> + if (status & GEN8_CTX_STATUS_ACTIVE_IDLE &&
> + buf[2*
Re: [Intel-gfx] [PATCH igt] tests/gem_workarounds: Skip write only registers
On Thu, Sep 28, 2017 at 04:45:06PM +0300, Mika Kuoppala wrote:
> We have no means to check write only registers as
> this would need access through context image. For now we
> know that cnl has a one such register, 0xe5f0 which is used
> to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting
> the context image without and with workaround applied:
>
> 0xa840: 0xe5f0 0x0800
> 0xa840: 0xe5f0 0x0820
>
> we can conclude that the workaround setup is working right
> in this particular case.
>
> Add a write only list and add register 0xe5f0 into this list.
> This is a temporary solution until a more capable context image
> checker emerges.
According to old wisdom, nothing is as permanent as a temporary
solution. I'd like this temporary-ness noted in a comment in the code
as well to ease future generations who finally get to fix this
properly.
--
Petri Latvala
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
> Cc: Oscar Mateo
> Cc: Chris Wilson
> Cc: Rodrigo Vivi
> Signed-off-by: Mika Kuoppala
> ---
> tests/gem_workarounds.c | 30 +-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> index d6641bd5..03b4dc6c 100644
> --- a/tests/gem_workarounds.c
> +++ b/tests/gem_workarounds.c
> @@ -29,6 +29,8 @@
>
> #include
>
> +static int gen;
> +
> enum operation {
> GPU_RESET,
> SUSPEND_RESUME,
> @@ -41,6 +43,13 @@ struct intel_wa_reg {
> uint32_t mask;
> };
>
> +static struct write_only_list {
> + unsigned int gen;
> + uint32_t addr;
> +} wo_list[] = {
> + { 10, 0xE5F0 } /* WaForceContextSaveRestoreNonCoherent:cnl */
> +};
> +
> static struct intel_wa_reg *wa_regs;
> static int num_wa_regs;
>
> @@ -64,6 +73,21 @@ static void test_suspend_resume(void)
> igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
> }
>
> +static bool write_only(const uint32_t addr)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(wo_list); i++) {
> + if (gen == wo_list[i].gen &&
> + addr == wo_list[i].addr) {
> + igt_info("Skipping check for 0x%x due to write only\n",
> addr);
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> static int workaround_fail_count(void)
> {
> int i, fail_count = 0;
> @@ -85,6 +109,9 @@ static int workaround_fail_count(void)
> wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
> val, ok ? "OK" : "FAIL");
>
> + if (write_only(wa_regs[i].addr))
> + continue;
> +
> if (!ok) {
> igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
>wa_regs[i].addr, wa_regs[i].value,
> @@ -124,7 +151,6 @@ igt_main
> {
> igt_fixture {
> int device = drm_open_driver_master(DRIVER_INTEL);
> - const int gen = intel_gen(intel_get_drm_devid(device));
> struct pci_device *pci_dev;
> FILE *file;
> char *line = NULL;
> @@ -133,6 +159,8 @@ igt_main
>
> igt_require_gem(device);
>
> + gen = intel_gen(intel_get_drm_devid(device));
> +
> pci_dev = intel_get_pci_device();
> igt_require(pci_dev);
>
> --
> 2.11.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 11/11] drm/i915/scheduler: Support user-defined priorities
On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote: > Use a priority stored in the context as the initial value when > submitting a request. This allows us to change the default priority on a > per-context basis, allowing different contexts to be favoured with GPU > time at the expense of lower importance work. The user can adjust the > context's priority via I915_CONTEXT_PARAM_PRIORITY, with more positive > values being higher priority (they will be serviced earlier, after their > dependencies have been resolved). Any prerequisite work for an execbuf > will have its priority raised to match the new request as required. > > Normal users can specify any value in the range of -1023 to 0 [default], > i.e. they can reduce the priority of their workloads (and temporarily > boost it back to normal if so desired). > > Privileged users can specify any value in the range of -1023 to 1023, > [default is 0], i.e. they can raise their priority above all overs and > so potentially starve the system. > > Note that the existing schedulers are not fair, nor load balancing, the > execution is strictly by priority on a first-come, first-served basis, > and the driver may choose to boost some requests above the range > available to users. > > This priority was originally based around nice(2), but evolved to allow > clients to adjust their priority within a small range, and allow for a > privileged high priority range. > > For example, this can be used to implement EGL_IMG_context_priority > https://www.khronos.org/registry/egl/extensions/IMG/EGL_IMG_context_priority.txt > > EGL_CONTEXT_PRIORITY_LEVEL_IMG determines the priority level of > the context to be created. This attribute is a hint, as an > implementation may not support multiple contexts at some > priority levels and system policy may limit access to high > priority contexts to appropriate system privilege level. The > default value for EGL_CONTEXT_PRIORITY_LEVEL_IMG is > EGL_CONTEXT_PRIORITY_MEDIUM_IMG." > > so we can map > > PRIORITY_HIGH -> 1023 [privileged, will failback to 0] > PRIORITY_MED -> 0 [default] > PRIORITY_LOW -> -1023 > > They also map onto the priorities used by VkQueue (and a VkQueue is > essentially a timeline, our i915_gem_context under full-ppgtt). > > v2: s/CAP_SYS_ADMIN/CAP_SYS_NICE/ > v3: Report min/max user priorities as defines in the uapi, and rebase > internal priorities on the exposed values. > > Testcase: igt/gem_exec_schedule > Signed-off-by: Chris Wilson > Reviewed-by: Tvrtko Ursulin We should be good to go once Mesa is. Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v1] drm/i915: Enhanced for initialize partially filled pagetables
On Thu, 2017-09-28 at 10:09 +0800, Xiaolin Zhang wrote: > if vgpu active, the page table entry should be initialized after > allocation and then the hypersivor can ping pages succesuffly, > otherwise hypervisor will ping pages failed and the host will print > a lot of annoying errors such as “ERROR gvt: guest page write error -22, > gfn 0x7ada8, pa 0x7ada89a8, var 0x6, len 1” when create linux guest. > > Signed-off-by: Xiaolin Zhang Why does the hypervisor try to access the entries prior to them being made valid for hardware? Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for tests/gem_workarounds: Skip write only registers
== Series Details == Series: tests/gem_workarounds: Skip write only registers URL : https://patchwork.freedesktop.org/series/31073/ State : success == Summary == IGT patchset tested on top of latest successful build 2885b10f99b4beeb046e75af8b8488c229f629d3 igt/gem_exec_schedule: Ignore set-priority failures on old kernels with latest DRM-Tip kernel build CI_DRM_3150 f4bd0d12dbf2 drm-tip: 2017y-09m-28d-13h-39m-47s UTC integration manifest Test drv_module_reload: Subgroup basic-reload-inject: dmesg-warn -> PASS (fi-glk-1) fdo#102777 fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777 fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:443s fi-bdw-gvtdvmtotal:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:471s fi-blb-e6850 total:289 pass:224 dwarn:1 dfail:0 fail:0 skip:64 time:420s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:524s fi-bwr-2160 total:289 pass:184 dwarn:0 dfail:0 fail:0 skip:105 time:279s fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:503s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:507s fi-byt-j1900 total:289 pass:254 dwarn:1 dfail:0 fail:0 skip:34 time:494s fi-cfl-s total:289 pass:256 dwarn:1 dfail:0 fail:0 skip:32 time:557s fi-cnl-y total:289 pass:259 dwarn:0 dfail:0 fail:3 skip:27 time:667s fi-elk-e7500 total:289 pass:230 dwarn:0 dfail:0 fail:0 skip:59 time:417s fi-glk-1 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:572s fi-hsw-4770 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:428s fi-hsw-4770r total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:406s fi-ilk-650 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:444s fi-ivb-3520m total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:488s fi-ivb-3770 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:470s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:468s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:579s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:595s fi-pnv-d510 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:553s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:452s fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:753s fi-skl-6770hqtotal:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:491s fi-skl-gvtdvmtotal:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:479s fi-snb-2520m total:289 pass:251 dwarn:0 dfail:0 fail:0 skip:38 time:570s fi-snb-2600 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:416s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_267/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt v3] igt/gem_exec_scheduler: HAS_SCHEDULER no longer means HAS_PREEMPTION
On Wed, 2017-09-27 at 19:47 +0100, Chris Wilson wrote:
> Michal wants to limit machines that can do preemption, which means that
> we no longer can assume that if we have a scheduler for execbuf, that
> implies we have preemption.
>
> v2: Try a capability mask instead
> v3: Pretty print the caps.
>
> Signed-off-by: Chris Wilson
> +++ b/tests/gem_exec_schedule.c
> @@ -32,7 +32,12 @@
> #include "igt_rand.h"
> #include "igt_sysfs.h"
>
> +#define BIT(x) (1ul << (x))
> +
> #define LOCAL_PARAM_HAS_SCHEDULER 41
> +#define HAS_SCHEDULER BIT(0)
> +#define HAS_PRIORITY BIT(1)
> +#define HAS_PREEMPTION BIT(2)
This seems to be all spaces?
> +static unsigned int has_scheduler(int fd)
> {
> drm_i915_getparam_t gp;
> - int has = -1;
> + unsigned int caps = 0;
> + char buf[200];
> + size_t len = 0;
>
> gp.param = LOCAL_PARAM_HAS_SCHEDULER;
> - gp.value = &has;
> + gp.value = (int *)∩︀
> drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
>
> - return has > 0;
> + if (!caps)
> + return 0;
> +
> + len = snprintf(buf, sizeof(buf), "Has kernel scheduler");
> + if (caps & HAS_PRIORITY)
> + len += snprintf(buf + len, sizeof(buf) - len,
> + "%s context priorities",
> + caps & (HAS_PRIORITY - 2) ? "," : " with");
> +
> + if (caps & HAS_PREEMPTION)
> + len += snprintf(buf + len, sizeof(buf) - len,
> + "%s batchbuffer preemption",
> + caps & (HAS_PREEMPTION - 2) ? "," : " with");
The output is not going to be published in PEOPLE magazine, maybe we
can do a simpler indented "- with ..." prints :P
Reviewed-by: Joonas Lahtinen
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Inherit Kabylake platform features from Skylake
I recently tried to update the gen9 feature matrix and to my unpleasant
surprise found that Kabylake still acted like Broadwell and didn't
enable the feature. This is because kbl/cfl are inheriting their
defaults from Broadwell and not Skylake.
Signed-off-by: Chris Wilson
Cc: Rodrigo Vivi
Cc: Paulo Zanoni
Cc: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_pci.c | 21 +
1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index da60866b6628..01d4b569b2cc 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -495,13 +495,9 @@ static const struct intel_device_info
intel_geminilake_info __initconst = {
};
#define KBL_PLATFORM \
- BDW_FEATURES, \
- .gen = 9, \
+ SKL_PLATFORM, \
.platform = INTEL_KABYLAKE, \
- .has_csr = 1, \
- .has_guc = 1, \
- .has_ipc = 1, \
- .ddb_size = 896
+ .has_ipc = 1
static const struct intel_device_info intel_kabylake_gt1_info __initconst = {
KBL_PLATFORM,
@@ -520,13 +516,8 @@ static const struct intel_device_info
intel_kabylake_gt3_info __initconst = {
};
#define CFL_PLATFORM \
- BDW_FEATURES, \
- .gen = 9, \
- .platform = INTEL_COFFEELAKE, \
- .has_csr = 1, \
- .has_guc = 1, \
- .has_ipc = 1, \
- .ddb_size = 896
+ KBL_PLATFORM, \
+ .platform = INTEL_COFFEELAKE
static const struct intel_device_info intel_coffeelake_gt1_info __initconst = {
CFL_PLATFORM,
@@ -545,14 +536,12 @@ static const struct intel_device_info
intel_coffeelake_gt3_info __initconst = {
};
static const struct intel_device_info intel_cannonlake_gt2_info __initconst = {
- BDW_FEATURES,
+ SKL_PLATFORM,
.is_alpha_support = 1,
.platform = INTEL_CANNONLAKE,
.gen = 10,
.gt = 2,
.ddb_size = 1024,
- .has_csr = 1,
- .has_ipc = 1,
.color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
};
--
2.14.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt] tests/gem_workarounds: Skip write only registers
We have no means to check write only registers as
this would need access through context image. For now we
know that cnl has a one such register, 0xe5f0 which is used
to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting
the context image without and with workaround applied:
0xa840: 0xe5f0 0x0800
0xa840: 0xe5f0 0x0820
we can conclude that the workaround setup is working right
in this particular case.
Add a write only list and add register 0xe5f0 into this list.
This is a temporary solution until a more capable context image
checker emerges.
v2: add code comment about adhocness (Petri)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
Cc: Oscar Mateo
Cc: Chris Wilson
Cc: Rodrigo Vivi
Cc: Petri Latvala
Signed-off-by: Mika Kuoppala
Reviewed-by: Chris Wilson
---
tests/gem_workarounds.c | 38 +-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
index d6641bd5..5e30a7b8 100644
--- a/tests/gem_workarounds.c
+++ b/tests/gem_workarounds.c
@@ -29,6 +29,8 @@
#include
+static int gen;
+
enum operation {
GPU_RESET,
SUSPEND_RESUME,
@@ -41,6 +43,21 @@ struct intel_wa_reg {
uint32_t mask;
};
+static struct write_only_list {
+ unsigned int gen;
+ uint32_t addr;
+} wo_list[] = {
+ { 10, 0xE5F0 } /* WaForceContextSaveRestoreNonCoherent:cnl */
+
+ /*
+* FIXME: If you are contemplating adding stuff here
+* consider this as a temporary solution. You need to
+* manually check from context image that your workaround
+* is having an effect. Consider creating a context image
+* validator to act as a superior solution.
+*/
+};
+
static struct intel_wa_reg *wa_regs;
static int num_wa_regs;
@@ -64,6 +81,21 @@ static void test_suspend_resume(void)
igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
}
+static bool write_only(const uint32_t addr)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(wo_list); i++) {
+ if (gen == wo_list[i].gen &&
+ addr == wo_list[i].addr) {
+ igt_info("Skipping check for 0x%x due to write only\n",
addr);
+ return true;
+ }
+ }
+
+ return false;
+}
+
static int workaround_fail_count(void)
{
int i, fail_count = 0;
@@ -85,6 +117,9 @@ static int workaround_fail_count(void)
wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
val, ok ? "OK" : "FAIL");
+ if (write_only(wa_regs[i].addr))
+ continue;
+
if (!ok) {
igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
wa_regs[i].addr, wa_regs[i].value,
@@ -124,7 +159,6 @@ igt_main
{
igt_fixture {
int device = drm_open_driver_master(DRIVER_INTEL);
- const int gen = intel_gen(intel_get_drm_devid(device));
struct pci_device *pci_dev;
FILE *file;
char *line = NULL;
@@ -133,6 +167,8 @@ igt_main
igt_require_gem(device);
+ gen = intel_gen(intel_get_drm_devid(device));
+
pci_dev = intel_get_pci_device();
igt_require(pci_dev);
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt] tests/gem_workarounds: Skip write only registers
On Thu, Sep 28, 2017 at 06:00:03PM +0300, Mika Kuoppala wrote:
> We have no means to check write only registers as
> this would need access through context image. For now we
> know that cnl has a one such register, 0xe5f0 which is used
> to set WaForceContextSaveRestoreNonCoherent:cnl. By inspecting
> the context image without and with workaround applied:
>
> 0xa840: 0xe5f0 0x0800
> 0xa840: 0xe5f0 0x0820
>
> we can conclude that the workaround setup is working right
> in this particular case.
>
> Add a write only list and add register 0xe5f0 into this list.
> This is a temporary solution until a more capable context image
> checker emerges.
>
> v2: add code comment about adhocness (Petri)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102943
> Cc: Oscar Mateo
> Cc: Chris Wilson
> Cc: Rodrigo Vivi
> Cc: Petri Latvala
> Signed-off-by: Mika Kuoppala
> Reviewed-by: Chris Wilson
> ---
> tests/gem_workarounds.c | 38 +-
> 1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
> index d6641bd5..5e30a7b8 100644
> --- a/tests/gem_workarounds.c
> +++ b/tests/gem_workarounds.c
> @@ -29,6 +29,8 @@
>
> #include
>
> +static int gen;
> +
> enum operation {
> GPU_RESET,
> SUSPEND_RESUME,
> @@ -41,6 +43,21 @@ struct intel_wa_reg {
> uint32_t mask;
> };
>
> +static struct write_only_list {
> + unsigned int gen;
> + uint32_t addr;
> +} wo_list[] = {
> + { 10, 0xE5F0 } /* WaForceContextSaveRestoreNonCoherent:cnl */
> +
> + /*
> + * FIXME: If you are contemplating adding stuff here
> + * consider this as a temporary solution. You need to
> + * manually check from context image that your workaround
> + * is having an effect. Consider creating a context image
> + * validator to act as a superior solution.
> + */
> +};
Excellent.
Reviewed-by: Petri Latvala
> static struct intel_wa_reg *wa_regs;
> static int num_wa_regs;
>
> @@ -64,6 +81,21 @@ static void test_suspend_resume(void)
> igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
> }
>
> +static bool write_only(const uint32_t addr)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(wo_list); i++) {
> + if (gen == wo_list[i].gen &&
> + addr == wo_list[i].addr) {
> + igt_info("Skipping check for 0x%x due to write only\n",
> addr);
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> static int workaround_fail_count(void)
> {
> int i, fail_count = 0;
> @@ -85,6 +117,9 @@ static int workaround_fail_count(void)
> wa_regs[i].addr, wa_regs[i].value, wa_regs[i].mask,
> val, ok ? "OK" : "FAIL");
>
> + if (write_only(wa_regs[i].addr))
> + continue;
> +
> if (!ok) {
> igt_warn("0x%05X\t0x%08X\t0x%08X\t0x%08X\t%s\n",
>wa_regs[i].addr, wa_regs[i].value,
> @@ -124,7 +159,6 @@ igt_main
> {
> igt_fixture {
> int device = drm_open_driver_master(DRIVER_INTEL);
> - const int gen = intel_gen(intel_get_drm_devid(device));
> struct pci_device *pci_dev;
> FILE *file;
> char *line = NULL;
> @@ -133,6 +167,8 @@ igt_main
>
> igt_require_gem(device);
>
> + gen = intel_gen(intel_get_drm_devid(device));
> +
> pci_dev = intel_get_pci_device();
> igt_require(pci_dev);
>
> --
> 2.11.0
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/2] meson: Also build kms_atomic_interruptible
Signed-off-by: Petri Latvala --- tests/meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/meson.build b/tests/meson.build index 1c619179..53d02d13 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -152,6 +152,7 @@ test_progs = [ 'kms_3d', 'kms_addfb_basic', 'kms_atomic', + 'kms_atomic_interruptible', 'kms_atomic_transition', 'kms_busy', 'kms_ccs', -- 2.14.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 2/2] meson: Also build kms_atomic_interruptible
On Thu, Sep 28, 2017 at 06:10:48PM +0300, Petri Latvala wrote: > Signed-off-by: Petri Latvala > --- > tests/meson.build | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/meson.build b/tests/meson.build > index 1c619179..53d02d13 100644 > --- a/tests/meson.build > +++ b/tests/meson.build > @@ -152,6 +152,7 @@ test_progs = [ > 'kms_3d', > 'kms_addfb_basic', > 'kms_atomic', > + 'kms_atomic_interruptible', Yeah I guess that's a recent addition, the lists did match when I tested things. On both patches Reviewed-by: Daniel Vetter > 'kms_atomic_transition', > 'kms_busy', > 'kms_ccs', > -- > 2.14.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://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 https://lists.freedesktop.org/mailman/listinfo/intel-gfx
