Re: [Intel-gfx] [PATCH 10/10] drm/i915/uc: Add params for specifying firmware
>-Original Message- >From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of >Arkadiusz Hiler >Sent: Tuesday, March 7, 2017 7:25 AM >To: intel-gfx@lists.freedesktop.org >Subject: [Intel-gfx] [PATCH 10/10] drm/i915/uc: Add params for specifying >firmware > >`guc_firmware_path` and `huc_firmware_path` module parameters are added. > >Using the parameter disables version checks and loads desired firmware instead >of the default one. I see that the effort of this patch makes us test with different firmware versions and not just the default one. But is it worth introducing two new params ? We already have 3 parameters that are guc and huc related. Anusha >v2: make params unsafe && notice about disabled fw check (J. Lahtinen) > >Cc: Chris Wilson>Cc: Joonas Lahtinen >Cc: Michal Winiarski >Signed-off-by: Arkadiusz Hiler >--- > drivers/gpu/drm/i915/i915_params.c | 10 ++ > drivers/gpu/drm/i915/i915_params.h | 2 ++ > drivers/gpu/drm/i915/intel_guc_loader.c | 6 +- > drivers/gpu/drm/i915/intel_huc.c| 6 +- > drivers/gpu/drm/i915/intel_uc.c | 6 -- > 5 files changed, 26 insertions(+), 4 deletions(-) > >diff --git a/drivers/gpu/drm/i915/i915_params.c >b/drivers/gpu/drm/i915/i915_params.c >index 2e9645e..b6a7e36 100644 >--- a/drivers/gpu/drm/i915/i915_params.c >+++ b/drivers/gpu/drm/i915/i915_params.c >@@ -59,6 +59,8 @@ struct i915_params i915 __read_mostly = { > .enable_guc_loading = 0, > .enable_guc_submission = 0, > .guc_log_level = -1, >+ .guc_firmware_path = NULL, >+ .huc_firmware_path = NULL, > .enable_dp_mst = true, > .inject_load_failure = 0, > .enable_dpcd_backlight = false, >@@ -230,6 +232,14 @@ module_param_named(guc_log_level, >i915.guc_log_level, int, 0400); MODULE_PARM_DESC(guc_log_level, > "GuC firmware logging level (-1:disabled (default), 0-3:enabled)"); > >+module_param_named_unsafe(guc_firmware_path, i915.guc_firmware_path, >+charp, 0400); MODULE_PARM_DESC(guc_firmware_path, >+ "GuC firmware path to use instead of the default one"); >+ >+module_param_named_unsafe(huc_firmware_path, i915.huc_firmware_path, >+charp, 0400); MODULE_PARM_DESC(huc_firmware_path, >+ "HuC firmware path to use instead of the default one"); >+ > module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, >0600); MODULE_PARM_DESC(enable_dp_mst, > "Enable multi-stream transport (MST) for new DisplayPort sinks. > (default: >true)"); diff --git a/drivers/gpu/drm/i915/i915_params.h >b/drivers/gpu/drm/i915/i915_params.h >index 55d47ee..34148cc 100644 >--- a/drivers/gpu/drm/i915/i915_params.h >+++ b/drivers/gpu/drm/i915/i915_params.h >@@ -46,6 +46,8 @@ > func(int, enable_guc_loading); \ > func(int, enable_guc_submission); \ > func(int, guc_log_level); \ >+ func(char *, guc_firmware_path); \ >+ func(char *, huc_firmware_path); \ > func(int, use_mmio_flip); \ > func(int, mmio_debug); \ > func(int, edp_vswing); \ >diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c >b/drivers/gpu/drm/i915/intel_guc_loader.c >index 0478483..283b0ca 100644 >--- a/drivers/gpu/drm/i915/intel_guc_loader.c >+++ b/drivers/gpu/drm/i915/intel_guc_loader.c >@@ -474,7 +474,11 @@ int intel_guc_select_fw(struct intel_guc *guc) > guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; > guc->fw.fw = INTEL_UC_FW_TYPE_GUC; > >- if (IS_SKYLAKE(dev_priv)) { >+ if (i915.guc_firmware_path) { >+ guc->fw.path = i915.guc_firmware_path; >+ guc->fw.major_ver_wanted = 0; >+ guc->fw.minor_ver_wanted = 0; >+ } else if (IS_SKYLAKE(dev_priv)) { > guc->fw.path = I915_SKL_GUC_UCODE; > guc->fw.major_ver_wanted = SKL_FW_MAJOR; > guc->fw.minor_ver_wanted = SKL_FW_MINOR; diff --git >a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c >index ea67abc..ab4ee08 100644 >--- a/drivers/gpu/drm/i915/intel_huc.c >+++ b/drivers/gpu/drm/i915/intel_huc.c >@@ -153,7 +153,11 @@ void intel_huc_select_fw(struct intel_huc *huc) > huc->fw.load_status = INTEL_UC_FIRMWARE_NONE; > huc->fw.fw = INTEL_UC_FW_TYPE_HUC; > >- if (IS_SKYLAKE(dev_priv)) { >+ if (i915.huc_firmware_path) { >+ huc->fw.path = i915.huc_firmware_path; >+ huc->fw.major_ver_wanted = 0; >+ huc->fw.minor_ver_wanted = 0; >+ } else if (IS_SKYLAKE(dev_priv)) { > huc->fw.path = I915_SKL_HUC_UCODE; > huc->fw.major_ver_wanted = SKL_HUC_FW_MAJOR; > huc->fw.minor_ver_wanted = SKL_HUC_FW_MINOR; diff --git >a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index >8875647..1cf4590 100644 >--- a/drivers/gpu/drm/i915/intel_uc.c >+++ b/drivers/gpu/drm/i915/intel_uc.c >@@ -359,8 +359,10 @@ void
[Intel-gfx] linux-next: build failure after merge of the rcu tree
Hi Paul, After merging the rcu tree, today's linux-next build (x86_64 allmodconfig) failed like this: In file included from include/linux/resource_ext.h:19:0, from include/linux/pci.h:32, from include/drm/drmP.h:50, from drivers/gpu/drm/i915/i915_gem.c:28: drivers/gpu/drm/i915/selftests/mock_gem_device.c: In function 'mock_gem_device': drivers/gpu/drm/i915/selftests/mock_gem_device.c:177:9: error: 'SLAB_DESTROY_BY_RCU' undeclared (first use in this function) SLAB_DESTROY_BY_RCU); ^ include/linux/slab.h:149:4: note: in definition of macro 'KMEM_CACHE' (__flags), NULL) ^ drivers/gpu/drm/i915/selftests/mock_gem_device.c:177:9: note: each undeclared identifier is reported only once for each function it appears in SLAB_DESTROY_BY_RCU); ^ include/linux/slab.h:149:4: note: in definition of macro 'KMEM_CACHE' (__flags), NULL) ^ / Caused by commit 24b7cb25b8d1 ("mm: Rename SLAB_DESTROY_BY_RCU to SLAB_TYPESAFE_BY_RCU") interacting with commit 0daf0113cff6 ("drm/i915: Mock infrastructure for request emission") from the drm-intel tree. I added the following merge fix patch: From: Stephen RothwellDate: Wed, 8 Mar 2017 12:09:49 +1100 Subject: [PATCH] drm/i915: merge fix for "mm: Rename SLAB_DESTROY_BY_RCU to SLAB_TYPESAFE_BY_RCU" Signed-off-by: Stephen Rothwell --- drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c index 6a8258eacdcb..9f24c5da3f8d 100644 --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c @@ -174,7 +174,7 @@ struct drm_i915_private *mock_gem_device(void) i915->requests = KMEM_CACHE(mock_request, SLAB_HWCACHE_ALIGN | SLAB_RECLAIM_ACCOUNT | - SLAB_DESTROY_BY_RCU); + SLAB_TYPESAFE_BY_RCU); if (!i915->requests) goto err_vmas; -- 2.11.0 -- Cheers, Stephen Rothwell ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915/glk: Apply cdclk workaround for DP audio
== Series Details == Series: series starting with [v2,1/2] drm/i915/glk: Apply cdclk workaround for DP audio URL : https://patchwork.freedesktop.org/series/20862/ State : success == Summary == Series 20862v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/20862/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: fail -> PASS (fi-snb-2600) fdo#17 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-c: dmesg-warn -> PASS (fi-bsw-n3050) Test prime_vgem: Subgroup basic-wait-default: dmesg-warn -> PASS (fi-bsw-n3050) fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 464s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 615s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 535s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 630s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 508s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 499s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 437s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 439s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 441s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 491s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 487s fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 480s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 527s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 600s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 504s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 549s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 549s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 414s 22a3a8313335d0958175580a6212a97fc04b3b69 drm-tip: 2017y-03m-07d-22h-03m-11s UTC integration manifest 79b37ff drm/i915: Implement cdclk restrictions based on Azalia BCLK 8372d33 drm/i915/glk: Apply cdclk workaround for DP audio == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4091/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/2] drm/i915/glk: Apply cdclk workaround for DP audio
Implement the DP-Audio cdclk restriction for GLK, similar to what is implemented for BDW and other GEN9 platforms. The max. pixel clock adjustment for GLK, however factors in the 2 pixels per clock output that GLK generates. Separating min. cdclk and max. pixel_rate would be nicer, but let's defer that to future and fix the GLK bug for now. Signed-off-by: Dhinakaran Pandiyan--- drivers/gpu/drm/i915/intel_cdclk.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index de5ce6b..e8c1181 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1442,16 +1442,21 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state, if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95); - /* BSpec says "Do not use DisplayPort with CDCLK less than -* 432 MHz, audio enabled, port width x4, and link rate -* HBR2 (5.4 GHz), or else there may be audio corruption or -* screen corruption." + /* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz, +* audio enabled, port width x4, and link rate HBR2 (5.4 GHz), or else +* there may be audio corruption or screen corruption." This cdclk +* restriction for GLK is 316.8 MHz and since GLK can output two +* pixels per clock, the pixel rate becomes 2 * 316.8 MHz. */ if (intel_crtc_has_dp_encoder(crtc_state) && crtc_state->has_audio && crtc_state->port_clock >= 54 && - crtc_state->lane_count == 4) - pixel_rate = max(432000, pixel_rate); + crtc_state->lane_count == 4) { + if (IS_GEMINILAKE(dev_priv)) + pixel_rate = max(2 * 316800, pixel_rate); + else + pixel_rate = max(432000, pixel_rate); + } return pixel_rate; } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/2] drm/i915: Implement cdclk restrictions based on Azalia BCLK
According to BSpec, "The CD clock frequency must be at least twice the frequency of the Azalia BCLK." and BCLK is configured to 96 MHz by default. This check is needed because BXT and GLK support cdclk frequencies less than 192 MHz. Signed-off-by: Dhinakaran Pandiyan--- drivers/gpu/drm/i915/intel_cdclk.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index e8c1181..7b1ac1d 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1458,6 +1458,18 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state, pixel_rate = max(432000, pixel_rate); } + /* According to BSpec, "The CD clock frequency must be at least twice +* the frequency of the Azalia BCLK." and BCLK is 96 MHz by default. +* The check for GLK has to be adjusted as the platform can output +* two pixels per clock. +*/ + if (crtc_state->has_audio) { + if (IS_GEMINILAKE(dev_priv)) + pixel_rate = max(2 * 2 * 96000, pixel_rate); + if (IS_BROXTON(dev_priv)) + pixel_rate = max(2 * 96000, pixel_rate); + } + return pixel_rate; } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/10] drm/i915: Move cursor position and base handling into the platform specific functions
On Tue, Mar 07, 2017 at 05:27:05PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > Supposedly on some platforms we can get extra atomicity guarantees for > CURPOS if we write it between the CURCNTR and CURBASE. Let's move the > CURPOS handling into the platform specific hooks to make the possible > without having to pass the calculated CURPOS around. And while at it, > do the same for the CURBASE to avoid passing that either. > > Signed-off-by: Ville Syrjälä I'm convinced, Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/10] drm/i915: Clean up cursor junk from intel_crtc
On Tue, Mar 07, 2017 at 05:27:03PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > Move cursor_base, cursor_cntl, and cursor_size from intel_crtc > into intel_plane so that we don't need the crtc for cursor stuff > so much. > > Also entirely nuke cursor_addr which IMO doesn't provide any benefit > since it's not actually used by the cursor code itself. I'm not 100% > sure what the SKL+ DDB is code is after by looking at cursor_addr so > I just make it do its checks unconditionally. If that's not correct > then we should likely replace it with somehting like > plane_state->visible. Looks ok to me, but I'm going to bow out of checking atomic state and friends. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/10] drm/i915: Support variable cursor height on ivb+
On Tue, Mar 07, 2017 at 05:27:09PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > IVB introduced the CUR_FBC_CTL register which allows reducing the cursor > height down to 8 lines from the otherwise square cursor dimensions. > Implement support for it. CUR_FBC_CTL can't be used when the cursor > is rotated. > > Commandeer the otherwise unused cursor->cursor.size to track the > current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL > writes, and to notice when we need to arm the update via CURBASE if > just CUR_FBC_CTL changes. For userspace to discover this, they should just use trial and error during startup (using the legacy SetCursor)? Though the easiest way would cause the cursor to flicker - just hope the cursor is off/invisible on startup. Code makes a lot of sense after all the refactoring, but I'll leave the register checking to somebody else unless they are lazier than I am. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/10] drm/i915: Use fb->pitches[0] in cursor code
On Tue, Mar 07, 2017 at 05:27:08PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > The cursor code currently ignores fb->pitches[0] (except when creating > the fb itself), and just uses the cursor_width*4 as the stride. Let's > make sure fb->pitches[0] actually matches what we expect it to be. > > We can also relax the stride vs. cursor width relationship on 845/865 > since the stride is programmed separately. The only constraint is that > width*cpp doesn't exceed the stride, and that's already been checked > by the core since it makes sure the entire plane fits within the fb. > > We can also drop the bo size check as that's already checked when > we create the fb. That is the fb is guaranteed to fit within the bo. > > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/10] drm/i915: Split cursor check_plane into i845 and i9xx variants
On Tue, Mar 07, 2017 at 05:27:07PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > The 845/865 and 830/855/9xx+ style cursor don't have that > much in common with each other, so let's just split the > .check_plane() hook into two variants as well. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 232 > ++- > 1 file changed, 145 insertions(+), 87 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 222f54ffd113..41cbaee66f1b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9269,6 +9269,74 @@ static void i845_disable_cursor(struct intel_plane > *plane, > i845_update_cursor(plane, NULL, NULL); > } > > +static bool i845_cursor_size_ok(const struct intel_plane_state *plane_state) > +{ > + struct drm_i915_private *dev_priv = > + to_i915(plane_state->base.plane->dev); > + int width = plane_state->base.crtc_w; > + int height = plane_state->base.crtc_h; > + > + if (width == 0 || height == 0) > + return false; > + > + /* > + * 845g/865g are only limited by the width of their cursors, > + * the height is arbitrary up to the precision of the register. > + */ > + if ((width & 63) != 0) if (!IS_ALIGNED(width, 64)) ? > + return false; > + > + if (width > (IS_I845G(dev_priv) ? 64 : 512)) > + return false; > + > + if (height > 1023) > + return false; > + > + return true; > +} > static void i9xx_update_cursor(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state) > @@ -9328,41 +9396,92 @@ static void i9xx_disable_cursor(struct intel_plane > *plane, > i9xx_update_cursor(plane, NULL, NULL); > } > > -static bool cursor_size_ok(struct drm_i915_private *dev_priv, > -uint32_t width, uint32_t height) > +static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state) > { > + struct drm_i915_private *dev_priv = > + to_i915(plane_state->base.plane->dev); > + int width = plane_state->base.crtc_w; > + int height = plane_state->base.crtc_h; > + > if (width == 0 || height == 0) > return false; > > /* > - * 845g/865g are special in that they are only limited by > - * the width of their cursors, the height is arbitrary up to > - * the precision of the register. Everything else requires > - * square cursors, limited to a few power-of-two sizes. > + * Cursors are limited to a few power-of-two > + * sizes, and they must be square. >*/ > - if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) { > - if ((width & 63) != 0) > + switch (width | height) { > + case 256: > + case 128: > + if (IS_GEN2(dev_priv)) > return false; > + case 64: > + break; > + default: > + return false; Checks out ok. > + } There's still quite a bit of boilerplate duplication between the two check_plane functions. No chance for some sharing? I presume their is also an ulterior motive for the split in later patches. That would be worth mentioning in the changelog to quell some of the doubts over duplication. Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/10] drm/i915: Drop useless posting reads from cursor commit
On Tue, Mar 07, 2017 at 05:27:06PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > There should be no need to do posting reads between all the cursor > register accessess. Let's just drop them. > > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/10] drm/i915: Refactor CURBASE calculation
On Tue, Mar 07, 2017 at 05:27:02PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > The remaining cursor base address calculations are spread > around into several different locations. Just pull it all > into one place. > > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/10] drm/i915: Parametrize cursor/primary pipe select bits
On Tue, Mar 07, 2017 at 05:27:00PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/10] drm/i915: Pass intel_plane and intel_crtc to plane hooks
On Tue, Mar 07, 2017 at 05:27:01PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > Streamline things by passing intel_plane and intel_crtc instead of > the drm types to our plane hooks. > > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/10] drm/i915: Refactor CURPOS calculation
On Tue, Mar 07, 2017 at 05:27:04PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä> > Move the CURPOS calculations to seprate function. This will allow > sharing the code between the 845/865 vs. others codepaths when we > otherwise split them apart. > > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] linux-next: build failure after merge of the sunxi tree
Hi Stephen, Daniel, On Tue, Mar 07, 2017 at 11:10:19AM +1100, Stephen Rothwell wrote: > Hi all, > > After merging the sunxi tree, today's linux-next build (arm > multi_v7_defconfig) failed like this: > > drivers/gpu/drm/sun4i/sun4i_crtc.c: In function 'sun4i_crtc_enable_vblank': > drivers/gpu/drm/sun4i/sun4i_crtc.c:109:31: error: 'struct sun4i_crtc' has no > member named 'drv' > struct sun4i_drv *drv = scrtc->drv; >^ > drivers/gpu/drm/sun4i/sun4i_crtc.c: In function 'sun4i_crtc_disable_vblank': > drivers/gpu/drm/sun4i/sun4i_crtc.c:121:31: error: 'struct sun4i_crtc' has no > member named 'drv' > struct sun4i_drv *drv = scrtc->drv; >^ > > Caused by commit > > 50480a78e282 ("drm: sun4i: use vblank hooks in struct drm_crtc_funcs") > > from the drm-misc tree interacting with commit > > 1b8d109585df ("drm/sun4i: Add backend and tcon pointers to sun4i_crtc") > > from the sunxi tree. I just rebased my tree on top of the latest drm-misc tag (drm-misc-next-2017-03-06). It should compile, and not have merge conflicts anymore. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915/userptr: Deactivate a failed userptr if the worker reports an EFAULT
== Series Details == Series: series starting with [v2,1/3] drm/i915/userptr: Deactivate a failed userptr if the worker reports an EFAULT URL : https://patchwork.freedesktop.org/series/20855/ State : success == Summary == Series 20855v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/20855/revisions/1/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 468s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 615s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 539s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 600s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 509s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 496s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 438s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 433s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 437s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 510s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 491s fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 477s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 506s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 599s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 504s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 548s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 551s fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time: 422s dcca4ca8923adc61254f23eb66ca18a4c9e1ffd3 drm-tip: 2017y-03m-07d-18h-17m-03s UTC integration manifest 16415ac drm/i915/userptr: Disallow wrapping GTT into a userptr 6ae9f57 drm/i915/userptr: Only flush the workqueue if required c6bcac8 drm/i915/userptr: Deactivate a failed userptr if the worker reports an EFAULT == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4090/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Store a permanent error in obj->mm.pages
On Tue, Mar 07, 2017 at 07:47:29PM +0200, Joonas Lahtinen wrote: > On ti, 2017-03-07 at 13:20 +, Chris Wilson wrote: > > Once the object has been truncated, it is unrecoverable. To facilitate > > detection of this state store the error in obj->mm.pages. > > > > This is required for the next patch which should be applied to v4.10 > > (via stable), so we also need to mark this patch for backporting. In > > that regard, let's consider this to be a fix/improvement too. > > > > v2: Avoid dereferencing the ERR_PTR when freeing the object. > > > > Fixes: 1233e2db199d ("drm/i915: Move object backing storage manipulation to > > its own locking") > > Signed-off-by: Chris Wilson> > Cc: Joonas Lahtinen > > Cc: # v4.10+ > > I'd imagine we may want a couple more GEM_BUG_ON checks going forward. Have a gander at https://patchwork.freedesktop.org/series/20312/ > Regardless; > > Reviewed-by: Joonas Lahtinen Thanks for the review, pushed to stop vlc/libva misbehaving. One day the shrinker will dtrt :) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ 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: Nuke debug messages from the pipe update critical section
== Series Details == Series: drm/i915: Nuke debug messages from the pipe update critical section URL : https://patchwork.freedesktop.org/series/20853/ State : success == Summary == Series 20853v1 drm/i915: Nuke debug messages from the pipe update critical section https://patchwork.freedesktop.org/api/1.0/series/20853/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: fail -> PASS (fi-snb-2600) fdo#17 fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 471s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 613s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 534s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 620s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 505s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 501s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 436s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 432s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 441s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 506s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 490s fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 475s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 505s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 603s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 493s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 552s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 549s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 415s dcca4ca8923adc61254f23eb66ca18a4c9e1ffd3 drm-tip: 2017y-03m-07d-18h-17m-03s UTC integration manifest 92f5123 drm/i915: Nuke debug messages from the pipe update critical section == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4089/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/3] drm/i915/userptr: Disallow wrapping GTT into a userptr
If we allow the user to convert a GTT mmap address into a userptr, we may end up in recursion hell, where currently we hit a mutex deadlock but other possibilities include use-after-free during the unbind/cancel_userptr. [ 143.203989] gem_userptr_bli D0 902898 0x [ 143.204054] Call Trace: [ 143.204137] __schedule+0x511/0x1180 [ 143.204195] ? pci_mmcfg_check_reserved+0xc0/0xc0 [ 143.204274] schedule+0x57/0xe0 [ 143.204327] schedule_timeout+0x383/0x670 [ 143.204374] ? trace_hardirqs_on_caller+0x187/0x280 [ 143.204457] ? trace_hardirqs_on_thunk+0x1a/0x1c [ 143.204507] ? usleep_range+0x110/0x110 [ 143.204657] ? irq_exit+0x89/0x100 [ 143.204710] ? retint_kernel+0x2d/0x2d [ 143.204794] ? trace_hardirqs_on_caller+0x187/0x280 [ 143.204857] ? _raw_spin_unlock_irq+0x33/0x60 [ 143.204944] wait_for_common+0x1f0/0x2f0 [ 143.205006] ? out_of_line_wait_on_atomic_t+0x170/0x170 [ 143.205103] ? wake_up_q+0xa0/0xa0 [ 143.205159] ? flush_workqueue_prep_pwqs+0x15a/0x2c0 [ 143.205237] wait_for_completion+0x1d/0x20 [ 143.205292] flush_workqueue+0x2e9/0xbb0 [ 143.205339] ? flush_workqueue+0x163/0xbb0 [ 143.205418] ? __schedule+0x533/0x1180 [ 143.205498] ? check_flush_dependency+0x1a0/0x1a0 [ 143.205681] i915_gem_userptr_mn_invalidate_range_start+0x1c7/0x270 [i915] [ 143.205865] ? i915_gem_userptr_dmabuf_export+0x40/0x40 [i915] [ 143.205955] __mmu_notifier_invalidate_range_start+0xc6/0x120 [ 143.206044] ? __mmu_notifier_invalidate_range_start+0x51/0x120 [ 143.206123] zap_page_range_single+0x1c7/0x1f0 [ 143.206171] ? unmap_single_vma+0x160/0x160 [ 143.206260] ? unmap_mapping_range+0xa9/0x1b0 [ 143.206308] ? vma_interval_tree_subtree_search+0x75/0xd0 [ 143.206397] unmap_mapping_range+0x18f/0x1b0 [ 143.206444] ? zap_vma_ptes+0x70/0x70 [ 143.206524] ? __pm_runtime_resume+0x67/0xa0 [ 143.206723] i915_gem_release_mmap+0x1ba/0x1c0 [i915] [ 143.206846] i915_vma_unbind+0x5c2/0x690 [i915] [ 143.206925] ? __lock_is_held+0x52/0x100 [ 143.207076] i915_gem_object_set_tiling+0x1db/0x650 [i915] [ 143.207236] i915_gem_set_tiling_ioctl+0x1d3/0x3b0 [i915] [ 143.207377] ? i915_gem_set_tiling_ioctl+0x5/0x3b0 [i915] [ 143.207457] drm_ioctl+0x36c/0x670 [ 143.207535] ? debug_lockdep_rcu_enabled.part.0+0x1a/0x30 [ 143.207730] ? i915_gem_object_set_tiling+0x650/0x650 [i915] [ 143.207793] ? drm_getunique+0x120/0x120 [ 143.207875] ? __handle_mm_fault+0x996/0x14a0 [ 143.207939] ? vm_insert_page+0x340/0x340 [ 143.208028] ? up_write+0x28/0x50 [ 143.208086] ? vm_mmap_pgoff+0x160/0x190 [ 143.208163] do_vfs_ioctl+0x12c/0xa60 [ 143.208218] ? debug_lockdep_rcu_enabled+0x35/0x40 [ 143.208267] ? ioctl_preallocate+0x150/0x150 [ 143.208353] ? __do_page_fault+0x36a/0x6e0 [ 143.208400] ? mark_held_locks+0x23/0xc0 [ 143.208479] ? up_read+0x1f/0x40 [ 143.208526] ? entry_SYSCALL_64_fastpath+0x5/0xc6 [ 143.208669] ? __fget_light+0xa7/0xc0 [ 143.208747] SyS_ioctl+0x41/0x70 To prevent the possibility of a deadlock, we defer scheduling the worker until after we have proven that given the current mm, the userptr range does not overlap a GGTT mmaping. If another thread tries to remap the GGTT over the userptr before the worker is scheduled, it will be stopped by its invalidate-range flushing the current work, before the deadlock can occur. v2: Improve discussion of how we end up in the deadlock. Reported-by: Michał WiniarskiTestcase: igt/gem_userptr_blits/map-fixed-invalidate-gup Signed-off-by: Chris Wilson Cc: Michał Winiarski Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem_userptr.c | 76 ++--- 1 file changed, 52 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c index dc9bf5282071..7addbf08bcb9 100644 --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -488,6 +488,36 @@ __i915_gem_userptr_set_active(struct drm_i915_gem_object *obj, return ret; } +static bool overlaps_ggtt(struct drm_i915_gem_object *obj, struct mm_struct *mm) +{ + const struct vm_operations_struct *gem_vm_ops = + obj->base.dev->driver->gem_vm_ops; + unsigned long addr = obj->userptr.ptr; + const unsigned long end = addr + obj->base.size; + struct vm_area_struct *vma; + + /* Check for a contiguous set of vma covering the userptr, if any +* are absent, they will EFAULT. More importantly if any point back +* to a drm_i915_gem_object GTT mmaping, we may trigger a deadlock +* between the deferred gup of this userptr and the object being +* unbound calling invalidate_range -> cancel_userptr. +*/ + for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) { + if (vma->vm_start > addr) /* gap */ +
[Intel-gfx] [PATCH v2 2/3] drm/i915/userptr: Only flush the workqueue if required
To avoid waiting for work from other invalidate-range threads where not required, only wait on the userptr cancel workqueue if we have added some work to it. Signed-off-by: Chris WilsonCc: Michał Winiarski Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem_userptr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c index 6ef05d5b884d..dc9bf5282071 100644 --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -145,7 +145,8 @@ static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *_mn, del_object(mo); spin_unlock(>lock); - flush_workqueue(mn->wq); + if (!list_empty()) + flush_workqueue(mn->wq); } static const struct mmu_notifier_ops i915_gem_userptr_notifier = { -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/3] drm/i915/userptr: Deactivate a failed userptr if the worker reports an EFAULT
If the worker fails, it no longer has pages to release and can be immediately removed from the invalidate-tree. Signed-off-by: Chris WilsonCc: Michał Winiarski Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem_userptr.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c index 22b46398831e..6ef05d5b884d 100644 --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -541,6 +541,8 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work) } obj->userptr.work = ERR_CAST(pages); + if (IS_ERR(pages)) + __i915_gem_userptr_set_active(obj, false); } mutex_unlock(>mm.lock); -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Nuke debug messages from the pipe update critical section
From: Ville Syrjäläprintks are slow so we should not be doing them from the vblank evade critical section. These could explain why we sometimes seem to blow past our 100 usec deadline. The problem has been there ever since commit bfd16b2a23dc ("drm/i915: Make updating pipe without modeset atomic.") but it may not have been readily visible until commit e1edbd44e23b ("drm/i915: Complain if we take too long under vblank evasion.") increased our chances of noticing it. Cc: Maarten Lankhorst Fixes: bfd16b2a23dc ("drm/i915: Make updating pipe without modeset atomic.") Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 12 +--- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e77ca7dfa44d..726ae191076b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3650,10 +3650,6 @@ static void intel_update_pipe_config(struct intel_crtc *crtc, /* drm_atomic_helper_update_legacy_modeset_state might not be called. */ crtc->base.mode = crtc->base.state->mode; - DRM_DEBUG_KMS("Updating pipe size %ix%i -> %ix%i\n", - old_crtc_state->pipe_src_w, old_crtc_state->pipe_src_h, - pipe_config->pipe_src_w, pipe_config->pipe_src_h); - /* * Update pipe size and adjust fitter if needed: the reason for this is * that in compute_mode_changes we check the native mode (not the pfit @@ -4775,23 +4771,17 @@ static void skylake_pfit_enable(struct intel_crtc *crtc) struct intel_crtc_scaler_state *scaler_state = >config->scaler_state; - DRM_DEBUG_KMS("for crtc_state = %p\n", crtc->config); - if (crtc->config->pch_pfit.enabled) { int id; - if (WARN_ON(crtc->config->scaler_state.scaler_id < 0)) { - DRM_ERROR("Requesting pfit without getting a scaler first\n"); + if (WARN_ON(crtc->config->scaler_state.scaler_id < 0)) return; - } id = scaler_state->scaler_id; I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN | PS_FILTER_MEDIUM | scaler_state->scalers[id].mode); I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc->config->pch_pfit.pos); I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc->config->pch_pfit.size); - - DRM_DEBUG_KMS("for crtc_state = %p scaler_id = %d\n", crtc->config, id); } } -- 2.10.2 ___ 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 drm/i915: CCS prep stuff
On Tue, Mar 07, 2017 at 08:22:03PM -, Patchwork wrote: > == Series Details == > > Series: drm/i915: CCS prep stuff > URL : https://patchwork.freedesktop.org/series/20851/ > State : warning > > == Summary == > > Series 20851v1 drm/i915: CCS prep stuff > https://patchwork.freedesktop.org/api/1.0/series/20851/revisions/1/mbox/ > > Test gem_exec_flush: > Subgroup basic-batch-kernel-default-uc: > fail -> PASS (fi-snb-2600) fdo#17 > Subgroup basic-wb-rw-before-default: > pass -> INCOMPLETE (fi-skl-6700hq) fdo#100081 > Test gem_exec_suspend: > Subgroup basic-s4-devices: > pass -> DMESG-WARN (fi-bxt-t5700) [ 380.840188] [drm] Atomic update on pipe (A) took 110 us, max time under evasion is 100 us Hmm. I wonder what we're doing in there on BXT that takes so long. Oh. I see plenty of DRM_DEBUG_KMS() calls via intel_update_pipe_config() at least. Those really need to go. [ 381.221206] WARNING: CPU: 3 PID: 0 at net/sched/sch_generic.c:316 dev_watchdog+0x225/0x230 Presumably not our problem, and not the first time this has been seen either. > > fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 > fdo#100081 https://bugs.freedesktop.org/show_bug.cgi?id=100081 > > fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 > time: 470s > fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 > time: 611s > fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 > time: 538s > fi-bxt-t5700 total:278 pass:257 dwarn:1 dfail:0 fail:0 skip:20 > time: 609s > fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 > time: 511s > fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 > time: 499s > fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 > time: 437s > fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 > time: 435s > fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 > time: 440s > fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 > time: 495s > fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 > time: 494s > fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 > time: 475s > fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 > time: 503s > fi-skl-6700hqtotal:62 pass:54 dwarn:0 dfail:0 fail:0 skip:7 > time: 0s > fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 > time: 503s > fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 > time: 545s > fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 > time: 553s > fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 > time: 431s > > dcca4ca8923adc61254f23eb66ca18a4c9e1ffd3 drm-tip: 2017y-03m-07d-18h-17m-03s > UTC integration manifest > e02ab59 drm/i915: Use DRM_DEBUG_KMS() for framebuffer failure debug messages > 98e01f1 drm/i915: Pass the correct plane index to _intel_compute_tile_offset() > d44a4ff drm/i915: Avoid div-by-zero when computing aux_stride w/o an aux plane > 9f610c8 drm/i915: Move nv12 chroma plane handling into intel_surf_alignment() > 48539e6 drm/i915: Plumb drm_framebuffer into more places > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4088/ -- Ville Syrjälä Intel OTC ___ 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: CCS prep stuff
== Series Details == Series: drm/i915: CCS prep stuff URL : https://patchwork.freedesktop.org/series/20851/ State : warning == Summary == Series 20851v1 drm/i915: CCS prep stuff https://patchwork.freedesktop.org/api/1.0/series/20851/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: fail -> PASS (fi-snb-2600) fdo#17 Subgroup basic-wb-rw-before-default: pass -> INCOMPLETE (fi-skl-6700hq) fdo#100081 Test gem_exec_suspend: Subgroup basic-s4-devices: pass -> DMESG-WARN (fi-bxt-t5700) fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fdo#100081 https://bugs.freedesktop.org/show_bug.cgi?id=100081 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 470s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 611s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 538s fi-bxt-t5700 total:278 pass:257 dwarn:1 dfail:0 fail:0 skip:20 time: 609s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 511s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 499s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 437s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 435s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 440s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 495s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 494s fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 475s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 503s fi-skl-6700hqtotal:62 pass:54 dwarn:0 dfail:0 fail:0 skip:7 time: 0s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 503s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 545s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 553s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 431s dcca4ca8923adc61254f23eb66ca18a4c9e1ffd3 drm-tip: 2017y-03m-07d-18h-17m-03s UTC integration manifest e02ab59 drm/i915: Use DRM_DEBUG_KMS() for framebuffer failure debug messages 98e01f1 drm/i915: Pass the correct plane index to _intel_compute_tile_offset() d44a4ff drm/i915: Avoid div-by-zero when computing aux_stride w/o an aux plane 9f610c8 drm/i915: Move nv12 chroma plane handling into intel_surf_alignment() 48539e6 drm/i915: Plumb drm_framebuffer into more places == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4088/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/dp: Read link status more times when EQ not done
On Thu, 02 Mar 2017, "Lee, Shawn C"wrote: > From: "Lee, Shawn C" > > Display driver read DPCD register 0x202, 0x203 and 0x204 to identify > eDP sink status.If PSR exit is ongoing at eDP sink, and eDP source > read these registers at the same time. Panel will report EQ & symbol > lock not done. It will cause panel display flicking. > > Try to read link status more times if eDP EQ not done. Panel side > request at least 1000us for fast link train while doing PSR exit. > So wait more than 1000us then retrieve sink's status again. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99639 > TEST=Reboot DUT and no flicking on local display at login screen > > Cc: Cooper Chiou > Cc: Wei Shun Chen > Cc: Gary C Wang > Cc: Jani Nikula > > Signed-off-by: Lee, Shawn C > --- > drivers/gpu/drm/i915/intel_dp.c | 22 ++ > 1 file changed, 22 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 024798a9c016..f6229d616971 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4225,6 +4225,7 @@ static void intel_dp_handle_test_request(struct > intel_dp *intel_dp) > { > struct intel_encoder *intel_encoder = _to_dig_port(intel_dp)->base; > struct drm_device *dev = intel_dp_to_dev(intel_dp); > + struct drm_i915_private *dev_priv = dev->dev_private; > u8 link_status[DP_LINK_STATUS_SIZE]; > > WARN_ON(!drm_modeset_is_locked(>mode_config.connection_mutex)); > @@ -4247,6 +4248,27 @@ static void intel_dp_handle_test_request(struct > intel_dp *intel_dp) > > /* Retrain if Channel EQ or CR not ok */ > if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) { > + if (is_edp(intel_dp) && dev_priv->psr.enabled) { > + u8 retry; > + > + for (retry = 0; retry < 3; retry++) { > + /* > + * EQ not ok may caused by fast link train > while exit PSR active, > + * wait at least 1000 us then read it again. > + */ > + DRM_DEBUG_KMS("%s: channel EQ not ok, retry = > %d, DPCD 0x202 = 0x%x, 0x203 = 0x%x, 0x204 = 0x%x\n", > + intel_encoder->base.name, retry, > link_status[0], link_status[1], link_status[2]); > + usleep_range(1000, 1500); > + if (!intel_dp_get_link_status(intel_dp, > link_status)) { > + DRM_ERROR("Failed to get link > status\n"); > + return; > + } > + > + if (drm_dp_channel_eq_ok(link_status, > intel_dp->lane_count)) > + return; > + } > + } Sorry, I'm really not fond of how this basically duplicates the entire intel_dp_check_link_status() function for the retry. Looks like you should move the intel_dp_get_link_status() call closer to the drm_dp_channel_eq_ok() call in the function, and abstract them into a separate helper. Then call the helper in a loop, but only really repeat for eDP and PSR active, settle for one try otherwise. But before you do, please check with Rodrigo (Cc'd) if he thinks the idea is sane to begin with. BR, Jani. > + > DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n", > intel_encoder->base.name); -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 5/5] drm/i915: Use DRM_DEBUG_KMS() for framebuffer failure debug messages
From: Ville SyrjäläDRM_UT_CORE generates way too much noise usually, so having the framebuffer init failures use DRM_UT_CORE is a pain when trying to find out the reason why you failed in creating a framebuffer. Let's use DRM_UT_KMS for these debug messages instead. v2: s/at less than/at most/ in the debug message (Imre) Signed-off-by: Ville Syrjälä Reviewed-by: Imre Deak --- drivers/gpu/drm/i915/intel_display.c | 66 ++-- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 90e6403b0324..060b6989c4da 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2454,8 +2454,8 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv, */ if (i915_gem_object_is_tiled(intel_fb->obj) && (x + width) * cpp > fb->pitches[i]) { - DRM_DEBUG("bad fb plane %d offset: 0x%x\n", - i, fb->offsets[i]); + DRM_DEBUG_KMS("bad fb plane %d offset: 0x%x\n", + i, fb->offsets[i]); return -EINVAL; } @@ -2537,9 +2537,9 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv, max_size = max(max_size, offset + size); } - if (max_size * tile_size > to_intel_framebuffer(fb)->obj->base.size) { - DRM_DEBUG("fb too big for bo (need %u bytes, have %zu bytes)\n", - max_size * tile_size, to_intel_framebuffer(fb)->obj->base.size); + if (max_size * tile_size > intel_fb->obj->base.size) { + DRM_DEBUG_KMS("fb too big for bo (need %u bytes, have %zu bytes)\n", + max_size * tile_size, intel_fb->obj->base.size); return -EINVAL; } @@ -14308,14 +14308,14 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb, */ if (tiling != I915_TILING_NONE && tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) { - DRM_DEBUG("tiling_mode doesn't match fb modifier\n"); + DRM_DEBUG_KMS("tiling_mode doesn't match fb modifier\n"); goto err; } } else { if (tiling == I915_TILING_X) { mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED; } else if (tiling == I915_TILING_Y) { - DRM_DEBUG("No Y tiling for legacy addfb\n"); + DRM_DEBUG_KMS("No Y tiling for legacy addfb\n"); goto err; } } @@ -14325,16 +14325,16 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb, case I915_FORMAT_MOD_Y_TILED: case I915_FORMAT_MOD_Yf_TILED: if (INTEL_GEN(dev_priv) < 9) { - DRM_DEBUG("Unsupported tiling 0x%llx!\n", - mode_cmd->modifier[0]); + DRM_DEBUG_KMS("Unsupported tiling 0x%llx!\n", + mode_cmd->modifier[0]); goto err; } case DRM_FORMAT_MOD_NONE: case I915_FORMAT_MOD_X_TILED: break; default: - DRM_DEBUG("Unsupported fb modifier 0x%llx!\n", - mode_cmd->modifier[0]); + DRM_DEBUG_KMS("Unsupported fb modifier 0x%llx!\n", + mode_cmd->modifier[0]); goto err; } @@ -14344,17 +14344,17 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb, */ if (INTEL_INFO(dev_priv)->gen < 4 && tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) { - DRM_DEBUG("tiling_mode must match fb modifier exactly on gen2/3\n"); + DRM_DEBUG_KMS("tiling_mode must match fb modifier exactly on gen2/3\n"); goto err; } pitch_limit = intel_fb_pitch_limit(dev_priv, mode_cmd->modifier[0], mode_cmd->pixel_format); if (mode_cmd->pitches[0] > pitch_limit) { - DRM_DEBUG("%s pitch (%u) must be at less than %d\n", - mode_cmd->modifier[0] != DRM_FORMAT_MOD_NONE ? - "tiled" : "linear", - mode_cmd->pitches[0], pitch_limit); + DRM_DEBUG_KMS("%s pitch (%u) must be at most %d\n", + mode_cmd->modifier[0] != DRM_FORMAT_MOD_NONE ? + "tiled" : "linear", + mode_cmd->pitches[0], pitch_limit); goto err; } @@ -14362,9 +14362,9 @@
[Intel-gfx] [PATCH 4/5] drm/i915: Pass the correct plane index to _intel_compute_tile_offset()
From: Ville Syrjäläintel_fill_fb_info() should pass the correct plane index to _intel_compute_tile_offset() once we start to care about the AUX surface. Signed-off-by: Ville Syrjälä Reviewed-by: Imre Deak --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ffa1041e10b9..90e6403b0324 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2467,7 +2467,7 @@ intel_fill_fb_info(struct drm_i915_private *dev_priv, intel_fb->normal[i].y = y; offset = _intel_compute_tile_offset(dev_priv, , , - fb, 0, fb->pitches[i], + fb, i, fb->pitches[i], DRM_ROTATE_0, tile_size); offset /= tile_size; -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/i915: Move nv12 chroma plane handling into intel_surf_alignment()
From: Ville SyrjäläLet's try to keep the alignment requirements in one place, and so towards that end let's move the AUX_DIST alignment handling into intel_surf_alignment() alongside the main surface alignment stuff. Signed-off-by: Ville Syrjälä Reviewed-by: Imre Deak --- drivers/gpu/drm/i915/intel_display.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d97f3f3554e4..f0da21327198 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2102,6 +2102,10 @@ static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb, { struct drm_i915_private *dev_priv = to_i915(fb->dev); + /* AUX_DIST needs only 4K alignment */ + if (fb->format->format == DRM_FORMAT_NV12 && plane == 1) + return 4096; + switch (fb->modifier) { case DRM_FORMAT_MOD_NONE: return intel_linear_alignment(dev_priv); @@ -2386,13 +2390,7 @@ u32 intel_compute_tile_offset(int *x, int *y, const struct drm_framebuffer *fb = state->base.fb; unsigned int rotation = state->base.rotation; int pitch = intel_fb_pitch(fb, plane, rotation); - u32 alignment; - - /* AUX_DIST needs only 4K alignment */ - if (fb->format->format == DRM_FORMAT_NV12 && plane == 1) - alignment = 4096; - else - alignment = intel_surf_alignment(fb, plane); + u32 alignment = intel_surf_alignment(fb, plane); return _intel_compute_tile_offset(dev_priv, x, y, fb, plane, pitch, rotation, alignment); -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] drm/i915: Avoid div-by-zero when computing aux_stride w/o an aux plane
From: Ville SyrjäläTo make life easier let's allow skl_plane_stride() to be called for the AUX surface even when there is no AUX surface. Avoids special cases in the callers. Signed-off-by: Ville Syrjälä Reviewed-by: Imre Deak --- drivers/gpu/drm/i915/intel_display.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f0da21327198..ffa1041e10b9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3218,7 +3218,12 @@ static void skl_detach_scalers(struct intel_crtc *intel_crtc) u32 skl_plane_stride(const struct drm_framebuffer *fb, int plane, unsigned int rotation) { - u32 stride = intel_fb_pitch(fb, plane, rotation); + u32 stride; + + if (plane >= fb->format->num_planes) + return 0; + + stride = intel_fb_pitch(fb, plane, rotation); /* * The stride is either expressed as a multiple of 64 bytes chunks for -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/5] drm/i915: CCS prep stuff
From: Ville SyrjäläAlready reviewed prep stuff for CCS. Posting it separately so that I can push it if/when CI gives the green light. Ville Syrjälä (5): drm/i915: Plumb drm_framebuffer into more places drm/i915: Move nv12 chroma plane handling into intel_surf_alignment() drm/i915: Avoid div-by-zero when computing aux_stride w/o an aux plane drm/i915: Pass the correct plane index to _intel_compute_tile_offset() drm/i915: Use DRM_DEBUG_KMS() for framebuffer failure debug messages drivers/gpu/drm/i915/intel_display.c | 213 --- drivers/gpu/drm/i915/intel_drv.h | 11 +- drivers/gpu/drm/i915/intel_fbdev.c | 4 +- 3 files changed, 99 insertions(+), 129 deletions(-) -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] drm/i915: Plumb drm_framebuffer into more places
From: Ville SyrjäläNow that framebuffers can be used even before calling drm_framebuffer_init() we can start to plumb them into more places, instead of passing individual pieces for fb metadata. Signed-off-by: Ville Syrjälä Reviewed-by: Imre Deak --- drivers/gpu/drm/i915/intel_display.c | 132 ++- drivers/gpu/drm/i915/intel_drv.h | 11 +-- drivers/gpu/drm/i915/intel_fbdev.c | 4 +- 3 files changed, 57 insertions(+), 90 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e77ca7dfa44d..d97f3f3554e4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1990,10 +1990,13 @@ static unsigned int intel_tile_size(const struct drm_i915_private *dev_priv) return IS_GEN2(dev_priv) ? 2048 : 4096; } -static unsigned int intel_tile_width_bytes(const struct drm_i915_private *dev_priv, - uint64_t fb_modifier, unsigned int cpp) +static unsigned int +intel_tile_width_bytes(const struct drm_framebuffer *fb, int plane) { - switch (fb_modifier) { + struct drm_i915_private *dev_priv = to_i915(fb->dev); + unsigned int cpp = fb->format->cpp[plane]; + + switch (fb->modifier) { case DRM_FORMAT_MOD_NONE: return cpp; case I915_FORMAT_MOD_X_TILED: @@ -2022,43 +2025,38 @@ static unsigned int intel_tile_width_bytes(const struct drm_i915_private *dev_pr } break; default: - MISSING_CASE(fb_modifier); + MISSING_CASE(fb->modifier); return cpp; } } -unsigned int intel_tile_height(const struct drm_i915_private *dev_priv, - uint64_t fb_modifier, unsigned int cpp) +static unsigned int +intel_tile_height(const struct drm_framebuffer *fb, int plane) { - if (fb_modifier == DRM_FORMAT_MOD_NONE) + if (fb->modifier == DRM_FORMAT_MOD_NONE) return 1; else - return intel_tile_size(dev_priv) / - intel_tile_width_bytes(dev_priv, fb_modifier, cpp); + return intel_tile_size(to_i915(fb->dev)) / + intel_tile_width_bytes(fb, plane); } /* Return the tile dimensions in pixel units */ -static void intel_tile_dims(const struct drm_i915_private *dev_priv, +static void intel_tile_dims(const struct drm_framebuffer *fb, int plane, unsigned int *tile_width, - unsigned int *tile_height, - uint64_t fb_modifier, - unsigned int cpp) + unsigned int *tile_height) { - unsigned int tile_width_bytes = - intel_tile_width_bytes(dev_priv, fb_modifier, cpp); + unsigned int tile_width_bytes = intel_tile_width_bytes(fb, plane); + unsigned int cpp = fb->format->cpp[plane]; *tile_width = tile_width_bytes / cpp; - *tile_height = intel_tile_size(dev_priv) / tile_width_bytes; + *tile_height = intel_tile_size(to_i915(fb->dev)) / tile_width_bytes; } unsigned int -intel_fb_align_height(struct drm_i915_private *dev_priv, - unsigned int height, - uint32_t pixel_format, - uint64_t fb_modifier) +intel_fb_align_height(const struct drm_framebuffer *fb, + int plane, unsigned int height) { - unsigned int cpp = drm_format_plane_cpp(pixel_format, 0); - unsigned int tile_height = intel_tile_height(dev_priv, fb_modifier, cpp); + unsigned int tile_height = intel_tile_height(fb, plane); return ALIGN(height, tile_height); } @@ -2099,21 +2097,23 @@ static unsigned int intel_linear_alignment(const struct drm_i915_private *dev_pr return 0; } -static unsigned int intel_surf_alignment(const struct drm_i915_private *dev_priv, -uint64_t fb_modifier) +static unsigned int intel_surf_alignment(const struct drm_framebuffer *fb, +int plane) { - switch (fb_modifier) { + struct drm_i915_private *dev_priv = to_i915(fb->dev); + + switch (fb->modifier) { case DRM_FORMAT_MOD_NONE: return intel_linear_alignment(dev_priv); case I915_FORMAT_MOD_X_TILED: - if (INTEL_INFO(dev_priv)->gen >= 9) + if (INTEL_GEN(dev_priv) >= 9) return 256 * 1024; return 0; case I915_FORMAT_MOD_Y_TILED: case I915_FORMAT_MOD_Yf_TILED: return 1 * 1024 * 1024; default: - MISSING_CASE(fb_modifier); + MISSING_CASE(fb->modifier); return 0; } } @@ -2130,7 +2130,7 @@
Re: [Intel-gfx] [PATCH v3] drm/i915: Store a permanent error in obj->mm.pages
On ti, 2017-03-07 at 13:20 +, Chris Wilson wrote: > Once the object has been truncated, it is unrecoverable. To facilitate > detection of this state store the error in obj->mm.pages. > > This is required for the next patch which should be applied to v4.10 > (via stable), so we also need to mark this patch for backporting. In > that regard, let's consider this to be a fix/improvement too. > > v2: Avoid dereferencing the ERR_PTR when freeing the object. > > Fixes: 1233e2db199d ("drm/i915: Move object backing storage manipulation to > its own locking") > Signed-off-by: Chris Wilson> Cc: Joonas Lahtinen > Cc: # v4.10+ I'd imagine we may want a couple more GEM_BUG_ON checks going forward. Regardless; 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: Clear the VBT defaults for unused ports
On Fri, 03 Mar 2017, Ville Syrjäläwrote: > On Fri, Mar 03, 2017 at 05:01:42AM -0800, Manasi Navare wrote: >> If during VBT parsing we find that the port is unused, >> the driver code just bails out without clearing the >> defaults for that port. This can cause failures down >> the path through link training for unused Port. >> This patch fixes this issue by clearing the defaults >> before bailing out from the VBT parsing function. >> >> Cc: Ville Syrjala >> Signed-off-by: Manasi Navare >> --- >> drivers/gpu/drm/i915/intel_bios.c | 10 -- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_bios.c >> b/drivers/gpu/drm/i915/intel_bios.c >> index e144f03..3ac3d24 100644 >> --- a/drivers/gpu/drm/i915/intel_bios.c >> +++ b/drivers/gpu/drm/i915/intel_bios.c >> @@ -1148,9 +1148,15 @@ static void parse_ddi_port(struct drm_i915_private >> *dev_priv, enum port port, >> } >> } >> } >> -if (!child) >> -return; >> +if (!child) { >> +/* Clear the DDI VBT Port info values */ >> +info->supports_dvi = 0; >> +info->supports_hdmi = 0; >> +info->supports_dp = 0; >> +info->supports_edp = 0; > > I would s/0/false/ here. Although they are apparently of type 'uint8_t:1'. > As a followup someone might want to s/uint8_t:1/bool:1/ the definitions > so that the compiler will protect us against values >1 when we assign > these. > > Otherwise lgtm. And based on a cursory glance the bogus port A has now > disappaered from the ci results on fi-skl-6700k, so looks like my > analysis of the problem was correct. > > Reviewed-by: Ville Syrjälä Bikeshedding after review... I think the approach here is a bit magical, first setting defaults on all platforms, then removing same defaults on platforms that a) are DDI, b) have VBT, c) have a high enough VBT revision, and d) do not have child devices. I think I'd go with simplicity, and split out a function from init_vbt_defaults() to be called on platforms without VBT. It would initially just contain the loop to initialize info->supports_dvi = (port != PORT_A && port != PORT_E); info->supports_hdmi = info->supports_dvi; info->supports_dp = (port != PORT_E); BR, Jani. > >> >> +return; >> +} >> aux_channel = child->common.aux_channel; >> ddc_pin = child->common.ddc_pin; >> >> -- >> 2.1.4 -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Flush idle work when changing missed-irq fault injection (rev3)
== Series Details == Series: drm/i915: Flush idle work when changing missed-irq fault injection (rev3) URL : https://patchwork.freedesktop.org/series/20752/ State : failure == Summary == Series 20752v3 drm/i915: Flush idle work when changing missed-irq fault injection https://patchwork.freedesktop.org/api/1.0/series/20752/revisions/3/mbox/ Test gem_exec_suspend: Subgroup basic-s3: pass -> FAIL (fi-bxt-t5700) Subgroup basic-s4-devices: pass -> FAIL (fi-bxt-t5700) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: dmesg-warn -> PASS (fi-byt-n2820) pass -> FAIL (fi-bxt-t5700) Subgroup suspend-read-crc-pipe-b: pass -> FAIL (fi-bxt-t5700) Subgroup suspend-read-crc-pipe-c: pass -> FAIL (fi-bxt-t5700) fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 469s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 614s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 541s fi-bxt-t5700 total:278 pass:253 dwarn:0 dfail:0 fail:5 skip:20 time: 652s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 501s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 500s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 438s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 436s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 439s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 509s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 478s fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 473s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 514s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 591s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 509s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 557s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 558s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 422s ba93250bd451d62c73db7ed39242a625730424f2 drm-tip: 2017y-03m-07d-13h-23m-43s UTC integration manifest 7932bb5 drm/i915: Flush idle work when changing missed-irq fault injection == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4087/ ___ 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: Cursor code cleanup and cursor "FBC" support for IVB+
== Series Details == Series: drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ URL : https://patchwork.freedesktop.org/series/20835/ State : success == Summary == Series 20835v1 drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ https://patchwork.freedesktop.org/api/1.0/series/20835/revisions/1/mbox/ Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: dmesg-warn -> PASS (fi-byt-n2820) fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 475s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 609s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 533s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 603s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 504s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 501s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 433s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 435s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 439s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 511s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 474s fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 476s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 507s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 595s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 501s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 561s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 556s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 423s ba93250bd451d62c73db7ed39242a625730424f2 drm-tip: 2017y-03m-07d-13h-23m-43s UTC integration manifest b8e5718 drm/i915: Support variable cursor height on ivb+ bb9c59d drm/i915: Use fb->pitches[0] in cursor code cb8bd0f drm/i915: Split cursor check_plane into i845 and i9xx variants 647a321 drm/i915: Drop useless posting reads from cursor commit 0fe5163 drm/i915: Move cursor position and base handling into the platform specific functions 7f5b846 drm/i915: Refactor CURPOS calculation 395e755 drm/i915: Clean up cursor junk from intel_crtc 8b8b73c drm/i915: Refactor CURBASE calculation f8c341da drm/i915: Pass intel_plane and intel_crtc to plane hooks dd96657 drm/i915: Parametrize cursor/primary pipe select bits == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4086/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: suppress atomic commit error message under gvt-g env
On Tue, Mar 07, 2017 at 12:46:35PM -0500, bing@intel.com wrote: > From: Bing Niu> > under virtualization enviroment, it is possible guest update pipe > registers across vblank intervals due to overhead of mmio traps or vm > schedule out. However, it is safe since those pipe update happen in > virual registers and will not be committed to hardware. suppress that > atomic commit error message under virtualization case to avoid > confusing user. > > v2: per ville's comment: return early and against Maarten's patch > > Signed-off-by: Bing Niu > --- > drivers/gpu/drm/i915/intel_sprite.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > b/drivers/gpu/drm/i915/intel_sprite.c > index 375ca91..b7849ca 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -161,6 +161,7 @@ void intel_pipe_update_end(struct intel_crtc *crtc, > struct intel_flip_work *work > int scanline_end = intel_get_crtc_scanline(crtc); > u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc); > ktime_t end_vbl_time = ktime_get(); > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > if (work) { > work->flip_queued_vblank = end_vbl_count; > @@ -186,6 +187,9 @@ void intel_pipe_update_end(struct intel_crtc *crtc, > struct intel_flip_work *work > > local_irq_enable(); > > + if(intel_vgpu_active(dev_priv)) ^ missing space I don't understand why it's OK fail atomicity guarantees for vgpu, but I don't really care either. So I'm fine with this. Acked-by: Ville Syrjälä > + return; > + > if (crtc->debug.start_vbl_count && > crtc->debug.start_vbl_count != end_vbl_count) { > DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) > time %lld us, min %d, max %d, scanline start %d, end %d\n", > -- > 2.7.4 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests: Add gem_spin_batch
On Tue, Mar 07, 2017 at 06:02:29PM +0200, Mika Kuoppala wrote: > Add gem_spin_batch to test that the dummyload infra > is working properly. Can be also act as tool to force > a single engine to be busy for controlled period of time. > > v2: plenty of igt-fu improvements (Chris) > > Cc: Abdiel Janulgue> Cc: Chris Wilson > Signed-off-by: Mika Kuoppala > --- > tests/Makefile.sources | 1 + > tests/gem_spin_batch.c | 80 > ++ > 2 files changed, 81 insertions(+) > create mode 100644 tests/gem_spin_batch.c > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > index 6e07d93..1763b67 100644 > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -214,6 +214,7 @@ TESTS_progs = \ > gem_unfence_active_buffers \ > gem_unref_active_buffers \ > gem_wait \ > + gem_spin_batch \ > gem_workarounds \ > gen3_mixed_blits \ > gen3_render_linear_blits \ > diff --git a/tests/gem_spin_batch.c b/tests/gem_spin_batch.c > new file mode 100644 > index 000..9823f4c > --- /dev/null > +++ b/tests/gem_spin_batch.c > @@ -0,0 +1,80 @@ > +/* > + * Copyright © 2017 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > DEALINGS > + * IN THE SOFTWARE. > + * > + */ > + > +#include "igt.h" > + static void basic(int fd, unsigned engine, unsigned timeout_sec) { const int64_t timeout_100ms = 1LL; unsigned long loops = 0; igt_spin_t *spin; struct timespec tv = {}; int64_t elapsed; spin = igt_spin_batch_new(fd, engine, 0); while ((elapsed = igt_nanoseconds_elapsed()) >> 30 < timeout_sec) { igt_spin_t *next = igt_spin_bach_new(fd, engine, 0); igt_spin_batch_set_timeout(spin, timeout_100ms); gem_sync(fd, spin->handle); loops++; igt_spin_batch_free(fd, spin); spin = next; } igt_spin_batch_free(fd, spin); igt_assert_lte(loops * timeout_100ms, elapsed); igt_assert_lte(100 * elapsed < 105 * loops * timeout_100ms); igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0); } -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests: Add gem_spin_batch
Add gem_spin_batch to test that the dummyload infra is working properly. Can be also act as tool to force a single engine to be busy for controlled period of time. v2: plenty of igt-fu improvements (Chris) Cc: Abdiel JanulgueCc: Chris Wilson Signed-off-by: Mika Kuoppala --- tests/Makefile.sources | 1 + tests/gem_spin_batch.c | 80 ++ 2 files changed, 81 insertions(+) create mode 100644 tests/gem_spin_batch.c diff --git a/tests/Makefile.sources b/tests/Makefile.sources index 6e07d93..1763b67 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -214,6 +214,7 @@ TESTS_progs = \ gem_unfence_active_buffers \ gem_unref_active_buffers \ gem_wait \ + gem_spin_batch \ gem_workarounds \ gen3_mixed_blits \ gen3_render_linear_blits \ diff --git a/tests/gem_spin_batch.c b/tests/gem_spin_batch.c new file mode 100644 index 000..9823f4c --- /dev/null +++ b/tests/gem_spin_batch.c @@ -0,0 +1,80 @@ +/* + * Copyright © 2017 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +#include "igt.h" + +static int64_t elapsed(const struct timespec *start, const struct timespec *end) +{ + return (end->tv_sec - start->tv_sec) * 10LL + + (end->tv_nsec - start->tv_nsec); +} + +static void basic(int fd, unsigned engine, unsigned timeout_sec) +{ + const int64_t timeout_100ms = 1LL; + struct timespec s, e; + igt_spin_t *spin; + unsigned long loops = 0; + + clock_gettime(CLOCK_MONOTONIC, ); + igt_until_timeout(timeout_sec) { + spin = igt_spin_batch_new(fd, engine, 0); + igt_spin_batch_set_timeout(spin, timeout_100ms); + gem_sync(fd, spin->handle); + loops++; + igt_spin_batch_free(fd, spin); + } + clock_gettime(CLOCK_MONOTONIC, ); + + igt_assert_lte(loops * timeout_100ms, elapsed(, )); + igt_assert_lte(elapsed(, ), 105 * loops * timeout_100ms / 100); /* 5% */ + igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0); +} + +igt_main +{ + const struct intel_execution_engine *e; + int fd = -1; + + igt_skip_on_simulation(); + + igt_fixture { + fd = drm_open_driver(DRIVER_INTEL); + igt_require_gem(fd); + igt_fork_hang_detector(fd); + } + + for (e = intel_execution_engines; e->name; e++) { + if (e->exec_id == 0) + continue; + + igt_subtest_f("basic-%s", e->name) + basic(fd, e->exec_id, 3); + } + + igt_fixture { + igt_stop_hang_detector(); + close(fd); + } +} -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI] drm/i915: Flush idle work when changing missed-irq fault injection
In order for the missed-irq update to take effect, the device must be idle. So when the user updates the fault injection via debugfs, idle the device. v2: Idle is explicitly required for setting test_irq, and good behaviour for clearing the missed_irq. v3: Use matching types; expanding to more than ulong rings is left as an exercise to the reader. Testcase: igt/drv_missed_irq Signed-off-by: Chris WilsonReviewed-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_debugfs.c | 55 +++-- 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index aa2d726b4349..3a2ef08ed3a1 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -4125,6 +4125,41 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_wedged_fops, "%llu\n"); static int +fault_irq_set(struct drm_i915_private *i915, + unsigned long *irq, + unsigned long val) +{ + int err; + + err = mutex_lock_interruptible(>drm.struct_mutex); + if (err) + return err; + + err = i915_gem_wait_for_idle(i915, +I915_WAIT_LOCKED | +I915_WAIT_INTERRUPTIBLE); + if (err) + goto err_unlock; + + /* Retire to kick idle work */ + i915_gem_retire_requests(i915); + GEM_BUG_ON(i915->gt.active_requests); + + *irq = val; + mutex_unlock(>drm.struct_mutex); + + /* Flush idle worker to disarm irq */ + while (flush_delayed_work(>gt.idle_work)) + ; + + return 0; + +err_unlock: + mutex_unlock(>drm.struct_mutex); + return err; +} + +static int i915_ring_missed_irq_get(void *data, u64 *val) { struct drm_i915_private *dev_priv = data; @@ -4136,18 +4171,9 @@ i915_ring_missed_irq_get(void *data, u64 *val) static int i915_ring_missed_irq_set(void *data, u64 val) { - struct drm_i915_private *dev_priv = data; - struct drm_device *dev = _priv->drm; - int ret; + struct drm_i915_private *i915 = data; - /* Lock against concurrent debugfs callers */ - ret = mutex_lock_interruptible(>struct_mutex); - if (ret) - return ret; - dev_priv->gpu_error.missed_irq_rings = val; - mutex_unlock(>struct_mutex); - - return 0; + return fault_irq_set(i915, >gpu_error.missed_irq_rings, val); } DEFINE_SIMPLE_ATTRIBUTE(i915_ring_missed_irq_fops, @@ -4167,13 +4193,12 @@ i915_ring_test_irq_get(void *data, u64 *val) static int i915_ring_test_irq_set(void *data, u64 val) { - struct drm_i915_private *dev_priv = data; + struct drm_i915_private *i915 = data; - val &= INTEL_INFO(dev_priv)->ring_mask; + val &= INTEL_INFO(i915)->ring_mask; DRM_DEBUG_DRIVER("Masking interrupts on rings 0x%08llx\n", val); - dev_priv->gpu_error.test_irq_rings = val; - return 0; + return fault_irq_set(i915, >gpu_error.test_irq_rings, val); } DEFINE_SIMPLE_ATTRIBUTE(i915_ring_test_irq_fops, -- 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] lib/igt_gt: Remove duplicated PARAM_NO_ERROR_CAPTURE define
Added r-b and applied upstream. On 2017-03-06 05:10 PM, Michel Thierry wrote: LOCAL_CONTEXT_PARAM_NO_ERROR_CAPTURE exists in lib/ioctl_wrappers.h since commit 171b21d9f761 ("igt/gem_ctx_param: Update invalid parma number"). Cc: Chris WilsonSigned-off-by: Michel Thierry --- lib/igt_gt.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/igt_gt.c b/lib/igt_gt.c index 4bdb9114..88b86aff 100644 --- a/lib/igt_gt.c +++ b/lib/igt_gt.c @@ -39,8 +39,6 @@ #include "intel_reg.h" #include "intel_chipset.h" -#define LOCAL_CONTEXT_PARAM_NO_ERROR_CAPTURE 0x4 - /** * SECTION:igt_gt * @short_description: GT support library ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for GuC Scrub vol. 1 (rev10)
== Series Details == Series: GuC Scrub vol. 1 (rev10) URL : https://patchwork.freedesktop.org/series/16856/ State : success == Summary == Series 16856v10 GuC Scrub vol. 1 https://patchwork.freedesktop.org/api/1.0/series/16856/revisions/10/mbox/ Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: dmesg-warn -> PASS (fi-byt-n2820) fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 463s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 612s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 541s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 610s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 505s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 500s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 436s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 434s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 438s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 500s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 488s fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 483s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 505s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 595s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 497s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 551s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 554s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 418s ba93250bd451d62c73db7ed39242a625730424f2 drm-tip: 2017y-03m-07d-13h-23m-43s UTC integration manifest ba44d8a drm/i915/uc: Add params for specifying firmware 15c67b4 drm/i915/uc: Separate firmware selection and preparation edb4736 drm/i915/uc: Simplify firmware path handling bafb3bf drm/i915/guc: Simplify intel_guc_init_hw() d09d99c drm/i915/guc: Extract param logic form guc_init_fw() 71768d6 drm/i915/uc: Introduce intel_uc_init_fw() b6386a1 drm/i915/uc: Move intel_uc_fw_fetch() to intel_uc.c 4601d8f drm/i915/uc: Rename intel_?uc_{setup, load}() to _init_hw() e2d1541 drm/i915/huc: Add huc_to_i915 5d99ff1 drm/i915/uc: Drop superfluous externs in intel_uc.h == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4085/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/10] drm/i915: Move cursor position and base handling into the platform specific functions
From: Ville SyrjäläSupposedly on some platforms we can get extra atomicity guarantees for CURPOS if we write it between the CURCNTR and CURBASE. Let's move the CURPOS handling into the platform specific hooks to make the possible without having to pass the calculated CURPOS around. And while at it, do the same for the CURBASE to avoid passing that either. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 77 +--- 1 file changed, 37 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b6786fc1d883..a3ff816fb152 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9197,11 +9197,12 @@ static u32 intel_cursor_position(struct intel_plane *plane, return pos; } -static void i845_update_cursor(struct intel_plane *plane, u32 base, +static void i845_update_cursor(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); - uint32_t cntl = 0, size = 0; + u32 cntl = 0, base = 0, pos = 0, size = 0; if (plane_state && plane_state->base.visible) { unsigned int width = plane_state->base.crtc_w; @@ -9227,6 +9228,9 @@ static void i845_update_cursor(struct intel_plane *plane, u32 base, CURSOR_STRIDE(stride); size = (height << 12) | width; + + base = intel_cursor_base(plane, plane_state); + pos = intel_cursor_position(plane, plane_state); } if (plane->cursor.cntl != 0 && @@ -9251,6 +9255,9 @@ static void i845_update_cursor(struct intel_plane *plane, u32 base, plane->cursor.size = size; } + if (cntl) + I915_WRITE(CURPOS(PIPE_A), pos); + if (plane->cursor.cntl != cntl) { I915_WRITE(CURCNTR(PIPE_A), cntl); POSTING_READ(CURCNTR(PIPE_A)); @@ -9258,12 +9265,19 @@ static void i845_update_cursor(struct intel_plane *plane, u32 base, } } -static void i9xx_update_cursor(struct intel_plane *plane, u32 base, +static void i845_disable_cursor(struct intel_plane *plane, + struct intel_crtc *crtc) +{ + i845_update_cursor(plane, NULL, NULL); +} + +static void i9xx_update_cursor(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum pipe pipe = plane->pipe; - uint32_t cntl = 0; + u32 cntl = 0, base = 0, pos = 0; if (plane_state && plane_state->base.visible) { cntl = MCURSOR_GAMMA_ENABLE; @@ -9289,6 +9303,9 @@ static void i9xx_update_cursor(struct intel_plane *plane, u32 base, if (plane_state->base.rotation & DRM_ROTATE_180) cntl |= CURSOR_ROTATE_180; + + base = intel_cursor_base(plane, plane_state); + pos = intel_cursor_position(plane, plane_state); } if (plane->cursor.cntl != cntl) { @@ -9297,6 +9314,9 @@ static void i9xx_update_cursor(struct intel_plane *plane, u32 base, plane->cursor.cntl = cntl; } + if (cntl) + I915_WRITE(CURPOS(pipe), pos); + /* and commit changes on next vblank */ I915_WRITE(CURBASE(pipe), base); POSTING_READ(CURBASE(pipe)); @@ -9304,25 +9324,10 @@ static void i9xx_update_cursor(struct intel_plane *plane, u32 base, plane->cursor.base = base; } -/* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */ -static void intel_crtc_update_cursor(struct intel_plane *plane, -const struct intel_plane_state *plane_state) +static void i9xx_disable_cursor(struct intel_plane *plane, + struct intel_crtc *crtc) { - struct drm_i915_private *dev_priv = to_i915(plane->base.dev); - enum pipe pipe = plane->pipe; - u32 pos = 0, base = 0; - - if (plane_state) { - base = intel_cursor_base(plane, plane_state); - pos = intel_cursor_position(plane, plane_state); - } - - I915_WRITE(CURPOS(pipe), pos); - - if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) - i845_update_cursor(plane, base, plane_state); - else - i9xx_update_cursor(plane, base, plane_state); + i9xx_update_cursor(plane, NULL, NULL); } static bool cursor_size_ok(struct drm_i915_private *dev_priv, @@ -13730,23 +13735,9 @@ intel_check_cursor_plane(struct intel_plane *plane,
[Intel-gfx] [PATCH 08/10] drm/i915: Split cursor check_plane into i845 and i9xx variants
From: Ville SyrjäläThe 845/865 and 830/855/9xx+ style cursor don't have that much in common with each other, so let's just split the .check_plane() hook into two variants as well. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 232 ++- 1 file changed, 145 insertions(+), 87 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 222f54ffd113..41cbaee66f1b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9269,6 +9269,74 @@ static void i845_disable_cursor(struct intel_plane *plane, i845_update_cursor(plane, NULL, NULL); } +static bool i845_cursor_size_ok(const struct intel_plane_state *plane_state) +{ + struct drm_i915_private *dev_priv = + to_i915(plane_state->base.plane->dev); + int width = plane_state->base.crtc_w; + int height = plane_state->base.crtc_h; + + if (width == 0 || height == 0) + return false; + + /* +* 845g/865g are only limited by the width of their cursors, +* the height is arbitrary up to the precision of the register. +*/ + if ((width & 63) != 0) + return false; + + if (width > (IS_I845G(dev_priv) ? 64 : 512)) + return false; + + if (height > 1023) + return false; + + return true; +} + +static int i845_check_cursor(struct intel_plane *plane, +struct intel_crtc_state *crtc_state, +struct intel_plane_state *state) +{ + struct drm_framebuffer *fb = state->base.fb; + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + unsigned stride; + int ret; + + ret = drm_plane_helper_check_state(>base, + >clip, + DRM_PLANE_HELPER_NO_SCALING, + DRM_PLANE_HELPER_NO_SCALING, + true, true); + if (ret) + return ret; + + /* if we want to turn off the cursor ignore width and height */ + if (!obj) + return 0; + + /* Check for which cursor types we support */ + if (!i845_cursor_size_ok(state)) { + DRM_DEBUG("Cursor dimension %dx%d not supported\n", + state->base.crtc_w, state->base.crtc_h); + return -EINVAL; + } + + stride = roundup_pow_of_two(state->base.crtc_w) * 4; + if (obj->base.size < stride * state->base.crtc_h) { + DRM_DEBUG_KMS("buffer is too small\n"); + return -ENOMEM; + } + + if (fb->modifier != DRM_FORMAT_MOD_NONE) { + DRM_DEBUG_KMS("cursor cannot be tiled\n"); + return -EINVAL; + } + + return 0; +} + static void i9xx_update_cursor(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) @@ -9328,41 +9396,92 @@ static void i9xx_disable_cursor(struct intel_plane *plane, i9xx_update_cursor(plane, NULL, NULL); } -static bool cursor_size_ok(struct drm_i915_private *dev_priv, - uint32_t width, uint32_t height) +static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state) { + struct drm_i915_private *dev_priv = + to_i915(plane_state->base.plane->dev); + int width = plane_state->base.crtc_w; + int height = plane_state->base.crtc_h; + if (width == 0 || height == 0) return false; /* -* 845g/865g are special in that they are only limited by -* the width of their cursors, the height is arbitrary up to -* the precision of the register. Everything else requires -* square cursors, limited to a few power-of-two sizes. +* Cursors are limited to a few power-of-two +* sizes, and they must be square. */ - if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) { - if ((width & 63) != 0) + switch (width | height) { + case 256: + case 128: + if (IS_GEN2(dev_priv)) return false; + case 64: + break; + default: + return false; + } - if (width > (IS_I845G(dev_priv) ? 64 : 512)) - return false; + return true; +} - if (height > 1023) - return false; - } else { - switch (width | height) { - case 256: - case 128: - if (IS_GEN2(dev_priv)) - return false; - case 64: - break; -
[Intel-gfx] [PATCH 02/10] drm/i915/huc: Add huc_to_i915
Used to obtain "dev_priv" from huc struct pointer. We already have similar thing for guc. Cc: Michal WajdeczkoSigned-off-by: Arkadiusz Hiler Reviewed-by: Michal Wajdeczko --- drivers/gpu/drm/i915/i915_drv.h | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b7ec663..8b25975 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2546,6 +2546,11 @@ 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 drm_i915_private *huc_to_i915(struct intel_huc *huc) +{ + return container_of(huc, struct drm_i915_private, huc); +} + /* Simple iterator over all initialised engines */ #define for_each_engine(engine__, dev_priv__, id__) \ for ((id__) = 0; \ -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/10] drm/i915: Use fb->pitches[0] in cursor code
From: Ville SyrjäläThe cursor code currently ignores fb->pitches[0] (except when creating the fb itself), and just uses the cursor_width*4 as the stride. Let's make sure fb->pitches[0] actually matches what we expect it to be. We can also relax the stride vs. cursor width relationship on 845/865 since the stride is programmed separately. The only constraint is that width*cpp doesn't exceed the stride, and that's already been checked by the core since it makes sure the entire plane fits within the fb. We can also drop the bo size check as that's already checked when we create the fb. That is the fb is guaranteed to fit within the bo. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 30 -- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 41cbaee66f1b..17362dc9f438 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9300,8 +9300,6 @@ static int i845_check_cursor(struct intel_plane *plane, struct intel_plane_state *state) { struct drm_framebuffer *fb = state->base.fb; - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - unsigned stride; int ret; ret = drm_plane_helper_check_state(>base, @@ -9313,7 +9311,7 @@ static int i845_check_cursor(struct intel_plane *plane, return ret; /* if we want to turn off the cursor ignore width and height */ - if (!obj) + if (!fb) return 0; /* Check for which cursor types we support */ @@ -9323,10 +9321,16 @@ static int i845_check_cursor(struct intel_plane *plane, return -EINVAL; } - stride = roundup_pow_of_two(state->base.crtc_w) * 4; - if (obj->base.size < stride * state->base.crtc_h) { - DRM_DEBUG_KMS("buffer is too small\n"); - return -ENOMEM; + switch (fb->pitches[0]) { + case 256: + case 512: + case 1024: + case 2048: + break; + default: + DRM_DEBUG_KMS("Invalid cursor stride (%u)\n", + fb->pitches[0]); + return -EINVAL; } if (fb->modifier != DRM_FORMAT_MOD_NONE) { @@ -9430,9 +9434,7 @@ static int i9xx_check_cursor(struct intel_plane *plane, { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); struct drm_framebuffer *fb = state->base.fb; - struct drm_i915_gem_object *obj = intel_fb_obj(fb); enum pipe pipe = plane->pipe; - unsigned stride; int ret; ret = drm_plane_helper_check_state(>base, @@ -9444,7 +9446,7 @@ static int i9xx_check_cursor(struct intel_plane *plane, return ret; /* if we want to turn off the cursor ignore width and height */ - if (!obj) + if (!fb) return 0; /* Check for which cursor types we support */ @@ -9454,10 +9456,10 @@ static int i9xx_check_cursor(struct intel_plane *plane, return -EINVAL; } - stride = roundup_pow_of_two(state->base.crtc_w) * 4; - if (obj->base.size < stride * state->base.crtc_h) { - DRM_DEBUG_KMS("buffer is too small\n"); - return -ENOMEM; + if (fb->pitches[0] != state->base.crtc_w * fb->format->cpp[0]) { + DRM_DEBUG_KMS("Invalid cursor stride (%u) (cursor width %d)\n", + fb->pitches[0], state->base.crtc_w); + return -EINVAL; } if (fb->modifier != DRM_FORMAT_MOD_NONE) { -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/10] drm/i915: Clean up cursor junk from intel_crtc
From: Ville SyrjäläMove cursor_base, cursor_cntl, and cursor_size from intel_crtc into intel_plane so that we don't need the crtc for cursor stuff so much. Also entirely nuke cursor_addr which IMO doesn't provide any benefit since it's not actually used by the cursor code itself. I'm not 100% sure what the SKL+ DDB is code is after by looking at cursor_addr so I just make it do its checks unconditionally. If that's not correct then we should likely replace it with somehting like plane_state->visible. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_debugfs.c | 48 +- drivers/gpu/drm/i915/intel_display.c | 80 +++- drivers/gpu/drm/i915/intel_drv.h | 9 ++-- 3 files changed, 47 insertions(+), 90 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index aa2d726b4349..fc20580ecfc3 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3031,36 +3031,6 @@ static void intel_connector_info(struct seq_file *m, intel_seq_print_mode(m, 2, mode); } -static bool cursor_active(struct drm_i915_private *dev_priv, int pipe) -{ - u32 state; - - if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) - state = I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE; - else - state = I915_READ(CURCNTR(pipe)) & CURSOR_MODE; - - return state; -} - -static bool cursor_position(struct drm_i915_private *dev_priv, - int pipe, int *x, int *y) -{ - u32 pos; - - pos = I915_READ(CURPOS(pipe)); - - *x = (pos >> CURSOR_X_SHIFT) & CURSOR_POS_MASK; - if (pos & (CURSOR_POS_SIGN << CURSOR_X_SHIFT)) - *x = -*x; - - *y = (pos >> CURSOR_Y_SHIFT) & CURSOR_POS_MASK; - if (pos & (CURSOR_POS_SIGN << CURSOR_Y_SHIFT)) - *y = -*y; - - return cursor_active(dev_priv, pipe); -} - static const char *plane_type(enum drm_plane_type type) { switch (type) { @@ -3182,9 +3152,7 @@ static int i915_display_info(struct seq_file *m, void *unused) seq_printf(m, "CRTC info\n"); seq_printf(m, "-\n"); for_each_intel_crtc(dev, crtc) { - bool active; struct intel_crtc_state *pipe_config; - int x, y; pipe_config = to_intel_crtc_state(crtc->base.state); @@ -3195,14 +3163,18 @@ static int i915_display_info(struct seq_file *m, void *unused) yesno(pipe_config->dither), pipe_config->pipe_bpp); if (pipe_config->base.active) { + struct intel_plane *cursor = + to_intel_plane(crtc->base.cursor); + intel_crtc_info(m, crtc); - active = cursor_position(dev_priv, crtc->pipe, , ); - seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x, active? %s\n", - yesno(crtc->cursor_base), - x, y, crtc->base.cursor->state->crtc_w, - crtc->base.cursor->state->crtc_h, - crtc->cursor_addr, yesno(active)); + seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x\n", + yesno(cursor->base.state->visible), + cursor->base.state->crtc_x, + cursor->base.state->crtc_y, + cursor->base.state->crtc_w, + cursor->base.state->crtc_h, + cursor->cursor.base); intel_scaler_info(m, crtc); intel_plane_info(m, crtc); } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b884c0d5bbb3..191685a37200 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9153,8 +9153,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, return active; } -static u32 intel_cursor_base(struct intel_crtc *crtc, -struct intel_plane *plane, +static u32 intel_cursor_base(struct intel_plane *plane, const struct intel_plane_state *plane_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); @@ -9167,8 +9166,6 @@ static u32 intel_cursor_base(struct intel_crtc *crtc, else base = intel_plane_ggtt_offset(plane_state); - crtc->cursor_addr = base; - /* ILK+ do this automagically */ if (HAS_GMCH_DISPLAY(dev_priv) && plane_state->base.rotation & DRM_ROTATE_180) @@ -9178,12 +9175,10 @@ static u32
[Intel-gfx] [PATCH 00/10] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+
From: Ville SyrjäläHere's a pile of cleanups to the cursor code. I think it makes the code quite a bit more pleasant to look at. I wrote most of these probably one or two years ago, so I figured it's about time I try to get them in. I've also included support for the so called cursor "FBC" feature for IVB+, which allows non-square cursors. I think I've posted an earlier version of that (and perhaps some of the other patches too) at least once. Entire series available here: git://github.com/vsyrjala/linux.git cursor_improvements_4 Ville Syrjälä (10): drm/i915: Parametrize cursor/primary pipe select bits drm/i915: Pass intel_plane and intel_crtc to plane hooks drm/i915: Refactor CURBASE calculation drm/i915: Clean up cursor junk from intel_crtc drm/i915: Refactor CURPOS calculation drm/i915: Move cursor position and base handling into the platform specific functions drm/i915: Drop useless posting reads from cursor commit drm/i915: Split cursor check_plane into i845 and i9xx variants drm/i915: Use fb->pitches[0] in cursor code drm/i915: Support variable cursor height on ivb+ drivers/gpu/drm/i915/i915_debugfs.c | 48 +-- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 12 +- drivers/gpu/drm/i915/intel_atomic_plane.c | 6 +- drivers/gpu/drm/i915/intel_display.c | 572 +- drivers/gpu/drm/i915/intel_drv.h | 17 +- drivers/gpu/drm/i915/intel_sprite.c | 102 +++--- 7 files changed, 393 insertions(+), 365 deletions(-) -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/10] drm/i915: Drop useless posting reads from cursor commit
From: Ville SyrjäläThere should be no need to do posting reads between all the cursor register accessess. Let's just drop them. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 32 +++- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a3ff816fb152..222f54ffd113 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9241,28 +9241,26 @@ static void i845_update_cursor(struct intel_plane *plane, * whilst the cursor is disabled. */ I915_WRITE(CURCNTR(PIPE_A), 0); - POSTING_READ(CURCNTR(PIPE_A)); plane->cursor.cntl = 0; } - if (plane->cursor.base != base) { + if (plane->cursor.base != base) I915_WRITE(CURBASE(PIPE_A), base); - plane->cursor.base = base; - } - if (plane->cursor.size != size) { + if (plane->cursor.size != size) I915_WRITE(CURSIZE, size); - plane->cursor.size = size; - } if (cntl) I915_WRITE(CURPOS(PIPE_A), pos); - if (plane->cursor.cntl != cntl) { + if (plane->cursor.cntl != cntl) I915_WRITE(CURCNTR(PIPE_A), cntl); - POSTING_READ(CURCNTR(PIPE_A)); - plane->cursor.cntl = cntl; - } + + POSTING_READ(CURCNTR(PIPE_A)); + + plane->cursor.cntl = cntl; + plane->cursor.base = base; + plane->cursor.size = size; } static void i845_disable_cursor(struct intel_plane *plane, @@ -9308,19 +9306,19 @@ static void i9xx_update_cursor(struct intel_plane *plane, pos = intel_cursor_position(plane, plane_state); } - if (plane->cursor.cntl != cntl) { + if (plane->cursor.cntl != cntl) I915_WRITE(CURCNTR(pipe), cntl); - POSTING_READ(CURCNTR(pipe)); - plane->cursor.cntl = cntl; - } if (cntl) I915_WRITE(CURPOS(pipe), pos); - /* and commit changes on next vblank */ - I915_WRITE(CURBASE(pipe), base); + if (plane->cursor.cntl != cntl || + plane->cursor.base != base) + I915_WRITE(CURBASE(pipe), base); + POSTING_READ(CURBASE(pipe)); + plane->cursor.cntl = cntl; plane->cursor.base = base; } -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/10] drm/i915: Refactor CURPOS calculation
From: Ville SyrjäläMove the CURPOS calculations to seprate function. This will allow sharing the code between the 845/865 vs. others codepaths when we otherwise split them apart. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 38 ++-- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 191685a37200..b6786fc1d883 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9175,6 +9175,28 @@ static u32 intel_cursor_base(struct intel_plane *plane, return base; } +static u32 intel_cursor_position(struct intel_plane *plane, +const struct intel_plane_state *plane_state) +{ + int x = plane_state->base.crtc_x; + int y = plane_state->base.crtc_y; + u32 pos = 0; + + if (x < 0) { + pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT; + x = -x; + } + pos |= x << CURSOR_X_SHIFT; + + if (y < 0) { + pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT; + y = -y; + } + pos |= y << CURSOR_Y_SHIFT; + + return pos; +} + static void i845_update_cursor(struct intel_plane *plane, u32 base, const struct intel_plane_state *plane_state) { @@ -9291,22 +9313,8 @@ static void intel_crtc_update_cursor(struct intel_plane *plane, u32 pos = 0, base = 0; if (plane_state) { - int x = plane_state->base.crtc_x; - int y = plane_state->base.crtc_y; - - if (x < 0) { - pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT; - x = -x; - } - pos |= x << CURSOR_X_SHIFT; - - if (y < 0) { - pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT; - y = -y; - } - pos |= y << CURSOR_Y_SHIFT; - base = intel_cursor_base(plane, plane_state); + pos = intel_cursor_position(plane, plane_state); } I915_WRITE(CURPOS(pipe), pos); -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/10] drm/i915: Support variable cursor height on ivb+
From: Ville SyrjäläIVB introduced the CUR_FBC_CTL register which allows reducing the cursor height down to 8 lines from the otherwise square cursor dimensions. Implement support for it. CUR_FBC_CTL can't be used when the cursor is rotated. Commandeer the otherwise unused cursor->cursor.size to track the current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL writes, and to notice when we need to arm the update via CURBASE if just CUR_FBC_CTL changes. v2: Reverse the gen check to make it sane v3: Only enable CUR_FBC_CTL when cursor is enabled, adapt to earlier code changes which means we now actually turn off the cursor when we're supposed to unlike v2 v4: Add a comment about rotation vs. CUR_FBC_CTL, rebase due to 'dirty' (Chris) v5: Rebase to the atomic world Handle 180 degree rotation Add HAS_CUR_FBC() v6: Rebase Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 5 - drivers/gpu/drm/i915/intel_display.c | 36 +--- 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b7ec6636e2a0..c96b667d6e6e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2900,6 +2900,7 @@ intel_info(const struct drm_i915_private *dev_priv) #define HAS_FW_BLC(dev_priv) (INTEL_GEN(dev_priv) > 2) #define HAS_PIPE_CXSR(dev_priv) ((dev_priv)->info.has_pipe_cxsr) #define HAS_FBC(dev_priv) ((dev_priv)->info.has_fbc) +#define HAS_CUR_FBC(dev_priv) (!HAS_GMCH_DISPLAY(dev_priv) && INTEL_INFO(dev_priv)->gen >= 7) #define HAS_IPS(dev_priv) (IS_HSW_ULT(dev_priv) || IS_BROADWELL(dev_priv)) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 07cae03e8727..cfcb139e5952 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5449,7 +5449,9 @@ enum { #define CURSOR_POS_SIGN 0x8000 #define CURSOR_X_SHIFT0 #define CURSOR_Y_SHIFT16 -#define CURSIZE_MMIO(0x700a0) +#define CURSIZE_MMIO(0x700a0) /* 845/865 */ +#define _CUR_FBC_CTL_A 0x700a0 /* ivb+ */ +#define CUR_FBC_CTL_EN (1 << 31) #define _CURBCNTR 0x700c0 #define _CURBBASE 0x700c4 #define _CURBPOS 0x700c8 @@ -5465,6 +5467,7 @@ enum { #define CURCNTR(pipe) _CURSOR2(pipe, _CURACNTR) #define CURBASE(pipe) _CURSOR2(pipe, _CURABASE) #define CURPOS(pipe) _CURSOR2(pipe, _CURAPOS) +#define CUR_FBC_CTL(pipe) _CURSOR2(pipe, _CUR_FBC_CTL_A) #define CURSOR_A_OFFSET 0x70080 #define CURSOR_B_OFFSET 0x700c0 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 17362dc9f438..13b31e929631 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9347,7 +9347,7 @@ static void i9xx_update_cursor(struct intel_plane *plane, { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum pipe pipe = plane->pipe; - u32 cntl = 0, base = 0, pos = 0; + u32 cntl = 0, base = 0, pos = 0, size = 0; if (plane_state && plane_state->base.visible) { cntl = MCURSOR_GAMMA_ENABLE; @@ -9376,15 +9376,22 @@ static void i9xx_update_cursor(struct intel_plane *plane, base = intel_cursor_base(plane, plane_state); pos = intel_cursor_position(plane, plane_state); + + if (plane_state->base.crtc_h != plane_state->base.crtc_w) + size = CUR_FBC_CTL_EN | (plane_state->base.crtc_h - 1); } if (plane->cursor.cntl != cntl) I915_WRITE(CURCNTR(pipe), cntl); + if (plane->cursor.size != size) + I915_WRITE(CUR_FBC_CTL(pipe), size); + if (cntl) I915_WRITE(CURPOS(pipe), pos); if (plane->cursor.cntl != cntl || + plane->cursor.size != size || plane->cursor.base != base) I915_WRITE(CURBASE(pipe), base); @@ -9392,6 +9399,7 @@ static void i9xx_update_cursor(struct intel_plane *plane, plane->cursor.cntl = cntl; plane->cursor.base = base; + plane->cursor.size = size; } static void i9xx_disable_cursor(struct intel_plane *plane, @@ -9410,11 +9418,8 @@ static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state) if (width == 0 || height == 0) return false; - /* -* Cursors are limited to a few power-of-two -* sizes, and they must be square. -*/ - switch (width | height) { + /* Cursor width is limited to a few power-of-two sizes */ + switch (width) { case 256: case 128: if (IS_GEN2(dev_priv)) @@ -9425,6 +9430,21 @@ static bool
[Intel-gfx] [PATCH 01/10] drm/i915: Parametrize cursor/primary pipe select bits
From: Ville SyrjäläSigned-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_reg.h | 7 ++- drivers/gpu/drm/i915/intel_display.c | 6 +++--- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index cc843f96576f..07cae03e8727 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5439,9 +5439,7 @@ enum { #define CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX) #define CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX) #define CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX) -#define MCURSOR_PIPE_SELECT (1 << 28) -#define MCURSOR_PIPE_A 0x00 -#define MCURSOR_PIPE_B (1 << 28) +#define MCURSOR_PIPE_SELECT(pipe)((pipe) << 28) #define MCURSOR_GAMMA_ENABLE (1 << 26) #define CURSOR_ROTATE_180(1<<15) #define CURSOR_TRICKLE_FEED_DISABLE (1 << 14) @@ -5499,8 +5497,7 @@ enum { #define DISPPLANE_PIPE_CSC_ENABLE(1<<24) #define DISPPLANE_SEL_PIPE_SHIFT 24 #define DISPPLANE_SEL_PIPE_MASK (3< pipe == PIPE_B) - dspcntr |= DISPPLANE_SEL_PIPE_B; + dspcntr |= DISPPLANE_SEL_PIPE(intel_crtc->pipe); /* pipesrc and dspsize control the size that is scaled from, * which should always be the user's requested size. @@ -9247,7 +9246,8 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, MISSING_CASE(plane_state->base.crtc_w); return; } - cntl |= pipe << 28; /* Connect to correct pipe */ + + cntl |= MCURSOR_PIPE_SELECT(pipe); if (HAS_DDI(dev_priv)) cntl |= CURSOR_PIPE_CSC_ENABLE; -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/10] drm/i915: Pass intel_plane and intel_crtc to plane hooks
From: Ville SyrjäläStreamline things by passing intel_plane and intel_crtc instead of the drm types to our plane hooks. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_atomic_plane.c | 6 +- drivers/gpu/drm/i915/intel_display.c | 123 ++ drivers/gpu/drm/i915/intel_drv.h | 8 +- drivers/gpu/drm/i915/intel_sprite.c | 102 ++--- 4 files changed, 107 insertions(+), 132 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index cfb47293fd53..182dc2a36ed1 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -185,7 +185,7 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state, } intel_state->base.visible = false; - ret = intel_plane->check_plane(plane, crtc_state, intel_state); + ret = intel_plane->check_plane(intel_plane, crtc_state, intel_state); if (ret) return ret; @@ -235,14 +235,14 @@ static void intel_plane_atomic_update(struct drm_plane *plane, trace_intel_update_plane(plane, to_intel_crtc(crtc)); - intel_plane->update_plane(plane, + intel_plane->update_plane(intel_plane, to_intel_crtc_state(crtc->state), intel_state); } else { trace_intel_disable_plane(plane, to_intel_crtc(crtc)); - intel_plane->disable_plane(plane, crtc); + intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc)); } } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4e8351d89d10..c3e064af3b43 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2755,7 +2755,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, false); intel_pre_disable_primary_noatomic(_crtc->base); trace_intel_disable_plane(primary, intel_crtc); - intel_plane->disable_plane(primary, _crtc->base); + intel_plane->disable_plane(intel_plane, intel_crtc); return; @@ -2969,14 +2969,14 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state) return 0; } -static void i9xx_update_primary_plane(struct drm_plane *primary, +static void i9xx_update_primary_plane(struct intel_plane *primary, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { - struct drm_i915_private *dev_priv = to_i915(primary->dev); - struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc); + struct drm_i915_private *dev_priv = to_i915(primary->base.dev); + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); struct drm_framebuffer *fb = plane_state->base.fb; - int plane = intel_crtc->plane; + int plane = crtc->plane; u32 linear_offset; u32 dspcntr; i915_reg_t reg = DSPCNTR(plane); @@ -2989,7 +2989,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary, dspcntr |= DISPLAY_PLANE_ENABLE; if (INTEL_GEN(dev_priv) < 4) { - dspcntr |= DISPPLANE_SEL_PIPE(intel_crtc->pipe); + dspcntr |= DISPPLANE_SEL_PIPE(crtc->pipe); /* pipesrc and dspsize control the size that is scaled from, * which should always be the user's requested size. @@ -3048,7 +3048,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary, intel_add_fb_offsets(, , plane_state, 0); if (INTEL_GEN(dev_priv) >= 4) - intel_crtc->dspaddr_offset = + crtc->dspaddr_offset = intel_compute_tile_offset(, , plane_state, 0); if (rotation & DRM_ROTATE_180) { @@ -3061,10 +3061,10 @@ static void i9xx_update_primary_plane(struct drm_plane *primary, linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0); if (INTEL_GEN(dev_priv) < 4) - intel_crtc->dspaddr_offset = linear_offset; + crtc->dspaddr_offset = linear_offset; - intel_crtc->adjusted_x = x; - intel_crtc->adjusted_y = y; + crtc->adjusted_x = x; + crtc->adjusted_y = y; I915_WRITE(reg, dspcntr); @@ -3072,24 +3072,22 @@ static void i9xx_update_primary_plane(struct drm_plane *primary, if (INTEL_GEN(dev_priv) >= 4) { I915_WRITE(DSPSURF(plane), intel_plane_ggtt_offset(plane_state) + - intel_crtc->dspaddr_offset); +
[Intel-gfx] [PATCH 03/10] drm/i915: Refactor CURBASE calculation
From: Ville SyrjäläThe remaining cursor base address calculations are spread around into several different locations. Just pull it all into one place. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 54 +--- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c3e064af3b43..b884c0d5bbb3 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9153,6 +9153,31 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, return active; } +static u32 intel_cursor_base(struct intel_crtc *crtc, +struct intel_plane *plane, +const struct intel_plane_state *plane_state) +{ + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); + const struct drm_framebuffer *fb = plane_state->base.fb; + const struct drm_i915_gem_object *obj = intel_fb_obj(fb); + u32 base; + + if (INTEL_INFO(dev_priv)->cursor_needs_physical) + base = obj->phys_handle->busaddr; + else + base = intel_plane_ggtt_offset(plane_state); + + crtc->cursor_addr = base; + + /* ILK+ do this automagically */ + if (HAS_GMCH_DISPLAY(dev_priv) && + plane_state->base.rotation & DRM_ROTATE_180) + base += (plane_state->base.crtc_h * +plane_state->base.crtc_w - 1) * fb->format->cpp[0]; + + return base; +} + static void i845_update_cursor(struct drm_crtc *crtc, u32 base, const struct intel_plane_state *plane_state) { @@ -9266,14 +9291,14 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, /* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */ static void intel_crtc_update_cursor(struct drm_crtc *crtc, +struct intel_plane *plane, const struct intel_plane_state *plane_state) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = to_i915(dev); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int pipe = intel_crtc->pipe; - u32 base = intel_crtc->cursor_addr; - u32 pos = 0; + u32 pos = 0, base = 0; if (plane_state) { int x = plane_state->base.crtc_x; @@ -9291,12 +9316,9 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc, } pos |= y << CURSOR_Y_SHIFT; - /* ILK+ do this automagically */ - if (HAS_GMCH_DISPLAY(dev_priv) && - plane_state->base.rotation & DRM_ROTATE_180) { - base += (plane_state->base.crtc_h * -plane_state->base.crtc_w - 1) * 4; - } + base = intel_cursor_base(intel_crtc, plane, plane_state); + } else { + intel_crtc->cursor_addr = 0; } I915_WRITE(CURPOS(pipe), pos); @@ -13716,8 +13738,7 @@ static void intel_disable_cursor_plane(struct intel_plane *plane, struct intel_crtc *crtc) { - crtc->cursor_addr = 0; - intel_crtc_update_cursor(>base, NULL); + intel_crtc_update_cursor(>base, plane, NULL); } static void @@ -13725,20 +13746,9 @@ intel_update_cursor_plane(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *state) { - struct drm_i915_private *dev_priv = to_i915(plane->base.dev); struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); - struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb); - uint32_t addr; - - if (!obj) - addr = 0; - else if (!INTEL_INFO(dev_priv)->cursor_needs_physical) - addr = intel_plane_ggtt_offset(state); - else - addr = obj->phys_handle->busaddr; - crtc->cursor_addr = addr; - intel_crtc_update_cursor(>base, state); + intel_crtc_update_cursor(>base, plane, state); } static struct intel_plane * -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/10] drm/i915/uc: Add params for specifying firmware
`guc_firmware_path` and `huc_firmware_path` module parameters are added. Using the parameter disables version checks and loads desired firmware instead of the default one. v2: make params unsafe && notice about disabled fw check (J. Lahtinen) Cc: Chris WilsonCc: Joonas Lahtinen Cc: Michal Winiarski Signed-off-by: Arkadiusz Hiler --- drivers/gpu/drm/i915/i915_params.c | 10 ++ drivers/gpu/drm/i915/i915_params.h | 2 ++ drivers/gpu/drm/i915/intel_guc_loader.c | 6 +- drivers/gpu/drm/i915/intel_huc.c| 6 +- drivers/gpu/drm/i915/intel_uc.c | 6 -- 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 2e9645e..b6a7e36 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -59,6 +59,8 @@ struct i915_params i915 __read_mostly = { .enable_guc_loading = 0, .enable_guc_submission = 0, .guc_log_level = -1, + .guc_firmware_path = NULL, + .huc_firmware_path = NULL, .enable_dp_mst = true, .inject_load_failure = 0, .enable_dpcd_backlight = false, @@ -230,6 +232,14 @@ module_param_named(guc_log_level, i915.guc_log_level, int, 0400); MODULE_PARM_DESC(guc_log_level, "GuC firmware logging level (-1:disabled (default), 0-3:enabled)"); +module_param_named_unsafe(guc_firmware_path, i915.guc_firmware_path, charp, 0400); +MODULE_PARM_DESC(guc_firmware_path, + "GuC firmware path to use instead of the default one"); + +module_param_named_unsafe(huc_firmware_path, i915.huc_firmware_path, charp, 0400); +MODULE_PARM_DESC(huc_firmware_path, + "HuC firmware path to use instead of the default one"); + module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, 0600); MODULE_PARM_DESC(enable_dp_mst, "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 55d47ee..34148cc 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -46,6 +46,8 @@ func(int, enable_guc_loading); \ func(int, enable_guc_submission); \ func(int, guc_log_level); \ + func(char *, guc_firmware_path); \ + func(char *, huc_firmware_path); \ func(int, use_mmio_flip); \ func(int, mmio_debug); \ func(int, edp_vswing); \ diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 0478483..283b0ca 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -474,7 +474,11 @@ int intel_guc_select_fw(struct intel_guc *guc) guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; guc->fw.fw = INTEL_UC_FW_TYPE_GUC; - if (IS_SKYLAKE(dev_priv)) { + if (i915.guc_firmware_path) { + guc->fw.path = i915.guc_firmware_path; + guc->fw.major_ver_wanted = 0; + guc->fw.minor_ver_wanted = 0; + } else if (IS_SKYLAKE(dev_priv)) { guc->fw.path = I915_SKL_GUC_UCODE; guc->fw.major_ver_wanted = SKL_FW_MAJOR; guc->fw.minor_ver_wanted = SKL_FW_MINOR; diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c index ea67abc..ab4ee08 100644 --- a/drivers/gpu/drm/i915/intel_huc.c +++ b/drivers/gpu/drm/i915/intel_huc.c @@ -153,7 +153,11 @@ void intel_huc_select_fw(struct intel_huc *huc) huc->fw.load_status = INTEL_UC_FIRMWARE_NONE; huc->fw.fw = INTEL_UC_FW_TYPE_HUC; - if (IS_SKYLAKE(dev_priv)) { + if (i915.huc_firmware_path) { + huc->fw.path = i915.huc_firmware_path; + huc->fw.major_ver_wanted = 0; + huc->fw.minor_ver_wanted = 0; + } else if (IS_SKYLAKE(dev_priv)) { huc->fw.path = I915_SKL_HUC_UCODE; huc->fw.major_ver_wanted = SKL_HUC_FW_MAJOR; huc->fw.minor_ver_wanted = SKL_HUC_FW_MINOR; diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 8875647..1cf4590 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -359,8 +359,10 @@ void intel_uc_prepare_fw(struct drm_i915_private *dev_priv, goto fail; } - if (uc_fw->major_ver_found != uc_fw->major_ver_wanted || - uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) { + if (uc_fw->major_ver_wanted == 0 && uc_fw->minor_ver_wanted == 0) { + DRM_NOTE("Skipping uC firmware version check\n"); + } else if (uc_fw->major_ver_found != uc_fw->major_ver_wanted || + uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) { DRM_NOTE("uC firmware version %d.%d, required
[Intel-gfx] [PATCH 07/10] drm/i915/guc: Simplify intel_guc_init_hw()
Current version of intel_guc_init_hw() does a lot: - cares about submission - loads huc - implement WA This change offloads some of the logic to intel_uc_init_hw(), which now cares about the above. v2: rename guc_hw_reset and fix typo in define name (M. Wajdeczko) v3: rename once again v4: remove spurious comments and add some style (J. Lahtinen) v5: flow changes, got rid of dead checks (M. Wajdeczko) Cc: Anusha SrivatsaCc: Michal Winiarski Cc: Michal Wajdeczko Cc: Daniele Ceraolo Spurio Cc: Joonas Lahtinen Signed-off-by: Arkadiusz Hiler --- drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/intel_guc_loader.c | 147 +++- drivers/gpu/drm/i915/intel_uc.c | 114 + drivers/gpu/drm/i915/intel_uc.h | 3 + 4 files changed, 130 insertions(+), 136 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 2c3057c..ef9f8e5 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4454,7 +4454,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv) intel_mocs_init_l3cc_table(dev_priv); /* We can't enable contexts until all firmware is loaded */ - ret = intel_guc_init_hw(_priv->guc); + ret = intel_uc_init_hw(dev_priv); if (ret) goto out; diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 2761a76..f63e7e8 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -90,7 +90,7 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status) } }; -static void guc_interrupts_release(struct drm_i915_private *dev_priv) +void intel_guc_release_interrupts(struct drm_i915_private *dev_priv) { struct intel_engine_cs *engine; enum intel_engine_id id; @@ -108,7 +108,7 @@ static void guc_interrupts_release(struct drm_i915_private *dev_priv) I915_WRITE(GUC_WD_VECS_IER, 0); } -static void guc_interrupts_capture(struct drm_i915_private *dev_priv) +void intel_guc_capture_interrupts(struct drm_i915_private *dev_priv) { struct intel_engine_cs *engine; enum intel_engine_id id; @@ -408,24 +408,6 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv) return ret; } -static int guc_hw_reset(struct drm_i915_private *dev_priv) -{ - int ret; - u32 guc_status; - - ret = intel_guc_reset(dev_priv); - if (ret) { - DRM_ERROR("GuC reset failed, ret = %d\n", ret); - return ret; - } - - guc_status = I915_READ(GUC_STATUS); - WARN(!(guc_status & GS_MIA_IN_RESET), -"GuC status: 0x%x, MIA core expected to be in reset\n", guc_status); - - return ret; -} - /** * intel_guc_init_hw() - finish preparing the GuC for activity * @guc: intel_guc structure @@ -443,42 +425,22 @@ int intel_guc_init_hw(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); const char *fw_path = guc->fw.path; - int retries, ret, err; + int ret; DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n", fw_path, intel_uc_fw_status_repr(guc->fw.fetch_status), intel_uc_fw_status_repr(guc->fw.load_status)); - /* Loading forbidden, or no firmware to load? */ - if (!i915.enable_guc_loading) { - err = 0; - goto fail; - } else if (fw_path == NULL) { - /* Device is known to have no uCode (e.g. no GuC) */ - err = -ENXIO; - goto fail; + if (!fw_path) { + return -ENXIO; } else if (*fw_path == '\0') { - /* Device has a GuC but we don't know what f/w to load? */ WARN(1, "No GuC firmware known for this platform!\n"); - err = -ENODEV; - goto fail; + return -ENODEV; } - /* Fetch failed, or already fetched but failed to load? */ - if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS) { - err = -EIO; - goto fail; - } else if (guc->fw.load_status == INTEL_UC_FIRMWARE_FAIL) { - err = -ENOEXEC; - goto fail; - } - - guc_interrupts_release(dev_priv); - gen9_reset_guc_interrupts(dev_priv); - - /* We need to notify the guc whenever we change the GGTT */ - i915_ggtt_enable_guc(dev_priv); + if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS) + return -EIO; guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING; @@ -486,104 +448,19 @@ int intel_guc_init_hw(struct intel_guc *guc)
[Intel-gfx] [PATCH 03/10] drm/i915/uc: Rename intel_?uc_{setup, load}() to _init_hw()
GuC historically has two "startup" functions called _init() and _setup() Then HuC came with it's _init() and _load(). This commit renames intel_guc_setup() and intel_huc_load() to *uc_init_hw() as they called from the i915_gem_init_hw(). The aim is to be consistent in that entry points called during particular driver init phases (e.g. init_hw) are all suffixed by that phase. When reading the leaf functions, it should be clear at what stage during the driver load it is called and therefore what operations are legal at that point. Also, since the functions start with intel_guc and intel_huc they take appropiate structure. v2: commit message update (Chris Wilson) v3: change taken parameters to be more "semantic" (M. Wajdeczko) Cc: Chris WilsonCc: Michal Winiarski Cc: Michal Wajdeczko Signed-off-by: Arkadiusz Hiler Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/intel_guc_loader.c | 44 - drivers/gpu/drm/i915/intel_huc.c| 44 - drivers/gpu/drm/i915/intel_uc.h | 4 +-- 4 files changed, 47 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 7c20601..2c3057c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4454,7 +4454,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv) intel_mocs_init_l3cc_table(dev_priv); /* We can't enable contexts until all firmware is loaded */ - ret = intel_guc_setup(dev_priv); + ret = intel_guc_init_hw(_priv->guc); if (ret) goto out; diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 9885f76..cfffafd 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -428,28 +428,28 @@ static int guc_hw_reset(struct drm_i915_private *dev_priv) } /** - * intel_guc_setup() - finish preparing the GuC for activity - * @dev_priv: i915 device private + * intel_guc_init_hw() - finish preparing the GuC for activity + * @guc: intel_guc structure * - * Called from gem_init_hw() during driver loading and also after a GPU reset. + * Called during driver loading and also after a GPU reset. * * The main action required here it to load the GuC uCode into the device. * The firmware image should have already been fetched into memory by the - * earlier call to intel_guc_init(), so here we need only check that worked, - * and then transfer the image to the h/w. + * earlier call to intel_guc_init(), so here we need only check that + * worked, and then transfer the image to the h/w. * * Return: non-zero code on error */ -int intel_guc_setup(struct drm_i915_private *dev_priv) +int intel_guc_init_hw(struct intel_guc *guc) { - struct intel_uc_fw *guc_fw = _priv->guc.fw; - const char *fw_path = guc_fw->path; + struct drm_i915_private *dev_priv = guc_to_i915(guc); + const char *fw_path = guc->fw.path; int retries, ret, err; DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n", fw_path, - intel_uc_fw_status_repr(guc_fw->fetch_status), - intel_uc_fw_status_repr(guc_fw->load_status)); + intel_uc_fw_status_repr(guc->fw.fetch_status), + intel_uc_fw_status_repr(guc->fw.load_status)); /* Loading forbidden, or no firmware to load? */ if (!i915.enable_guc_loading) { @@ -467,10 +467,10 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) } /* Fetch failed, or already fetched but failed to load? */ - if (guc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS) { + if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS) { err = -EIO; goto fail; - } else if (guc_fw->load_status == INTEL_UC_FIRMWARE_FAIL) { + } else if (guc->fw.load_status == INTEL_UC_FIRMWARE_FAIL) { err = -ENOEXEC; goto fail; } @@ -481,11 +481,11 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) /* We need to notify the guc whenever we change the GGTT */ i915_ggtt_enable_guc(dev_priv); - guc_fw->load_status = INTEL_UC_FIRMWARE_PENDING; + guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING; DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n", - intel_uc_fw_status_repr(guc_fw->fetch_status), - intel_uc_fw_status_repr(guc_fw->load_status)); + intel_uc_fw_status_repr(guc->fw.fetch_status), + intel_uc_fw_status_repr(guc->fw.load_status)); err = i915_guc_submission_init(dev_priv); if (err) @@ -506,7 +506,7 @@ int intel_guc_setup(struct
[Intel-gfx] [PATCH 08/10] drm/i915/uc: Simplify firmware path handling
Currently fw->path values can represent one of three possible states: 1) NULL - device without the uC 2) '\0' - device with the uC but have no firmware 3) else - device with the uC and we have firmware Second case is used only to WARN at a later stage. We can WARN right away and merge cases 1 and 2. Code can be even further simplified and common (HuC/GuC logic) happening right before the fetch can be offloaded to the common function. v2: fewer temporary variables, more straightforward flow (M. Wajdeczko) v3: DRM_ERROR instead of WARN (M. Wajdeczko) v4: coding standard (J. Lahtinen) v5: non-trivial rebase Cc: Anusha SrivatsaCc: Tvrtko Ursulin Cc: Michal Winiarski Cc: Michal Wajdeczko Cc: Joonas Lahtinen Signed-off-by: Arkadiusz Hiler --- drivers/gpu/drm/i915/intel_guc_loader.c | 35 +++-- drivers/gpu/drm/i915/intel_huc.c| 21 ++-- drivers/gpu/drm/i915/intel_uc.c | 4 +++- 3 files changed, 21 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index f63e7e8..a36aaba 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -432,12 +432,8 @@ int intel_guc_init_hw(struct intel_guc *guc) intel_uc_fw_status_repr(guc->fw.fetch_status), intel_uc_fw_status_repr(guc->fw.load_status)); - if (!fw_path) { + if (!fw_path) return -ENXIO; - } else if (*fw_path == '\0') { - WARN(1, "No GuC firmware known for this platform!\n"); - return -ENODEV; - } if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS) return -EIO; @@ -463,7 +459,6 @@ int intel_guc_init_hw(struct intel_guc *guc) return 0; } - /** * intel_guc_init_fw() - select and prepare firmware for loading * @guc: intel_guc struct @@ -476,37 +471,31 @@ int intel_guc_init_hw(struct intel_guc *guc) void intel_guc_init_fw(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); - const char *fw_path; + + guc->fw.path = NULL; + guc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE; + guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; + guc->fw.fw = INTEL_UC_FW_TYPE_GUC; if (IS_SKYLAKE(dev_priv)) { - fw_path = I915_SKL_GUC_UCODE; + guc->fw.path = I915_SKL_GUC_UCODE; guc->fw.major_ver_wanted = SKL_FW_MAJOR; guc->fw.minor_ver_wanted = SKL_FW_MINOR; } else if (IS_BROXTON(dev_priv)) { - fw_path = I915_BXT_GUC_UCODE; + guc->fw.path = I915_BXT_GUC_UCODE; guc->fw.major_ver_wanted = BXT_FW_MAJOR; guc->fw.minor_ver_wanted = BXT_FW_MINOR; } else if (IS_KABYLAKE(dev_priv)) { - fw_path = I915_KBL_GUC_UCODE; + guc->fw.path = I915_KBL_GUC_UCODE; guc->fw.major_ver_wanted = KBL_FW_MAJOR; guc->fw.minor_ver_wanted = KBL_FW_MINOR; } else { - fw_path = ""; /* unknown device */ + DRM_ERROR("No GuC firmware known for platform with GuC!\n"); + i915.enable_guc_loading = 0; + return; } - guc->fw.path = fw_path; - guc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE; - guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; - - if (fw_path == NULL) - return; - if (*fw_path == '\0') - return; - - guc->fw.fetch_status = INTEL_UC_FIRMWARE_PENDING; - DRM_DEBUG_DRIVER("GuC firmware pending, path %s\n", fw_path); intel_uc_prepare_fw(dev_priv, >fw); - /* status must now be FAIL or SUCCESS */ } /** diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c index 168aab1..5fadd55 100644 --- a/drivers/gpu/drm/i915/intel_huc.c +++ b/drivers/gpu/drm/i915/intel_huc.c @@ -155,7 +155,6 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv) void intel_huc_init_fw(struct intel_huc *huc) { struct drm_i915_private *dev_priv = huc_to_i915(huc); - const char *fw_path = NULL; huc->fw.path = NULL; huc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE; @@ -163,29 +162,21 @@ void intel_huc_init_fw(struct intel_huc *huc) huc->fw.fw = INTEL_UC_FW_TYPE_HUC; if (IS_SKYLAKE(dev_priv)) { - fw_path = I915_SKL_HUC_UCODE; + huc->fw.path = I915_SKL_HUC_UCODE; huc->fw.major_ver_wanted = SKL_HUC_FW_MAJOR; huc->fw.minor_ver_wanted = SKL_HUC_FW_MINOR; } else if (IS_BROXTON(dev_priv)) { - fw_path = I915_BXT_HUC_UCODE; + huc->fw.path = I915_BXT_HUC_UCODE;
[Intel-gfx] [PATCH 09/10] drm/i915/uc: Separate firmware selection and preparation
intel_{h,g}uc_init_fw selects correct firmware and then triggers it's preparation (fetch + initial parsing). This change separates out select steps, so those can be called by the sanitize_options(). Then, during the init_fw(), we prepare the firmware if the firmware was selected. Cc: Michal WiniarskiCc: Joonas Lahtinen Signed-off-by: Arkadiusz Hiler Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_guc_loader.c | 14 +- drivers/gpu/drm/i915/intel_huc.c| 14 ++ drivers/gpu/drm/i915/intel_uc.c | 18 -- drivers/gpu/drm/i915/intel_uc.h | 4 ++-- 4 files changed, 21 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index a36aaba..0478483 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -460,15 +460,12 @@ int intel_guc_init_hw(struct intel_guc *guc) } /** - * intel_guc_init_fw() - select and prepare firmware for loading + * intel_guc_select_fw() - selects GuC firmware for loading * @guc: intel_guc struct * - * Called early during driver load, but after GEM is initialised. - * - * The firmware will be transferred to the GuC's memory later, - * when intel_guc_init_hw() is called. + * Return: zero when we know firmware, non-zero in other case */ -void intel_guc_init_fw(struct intel_guc *guc) +int intel_guc_select_fw(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); @@ -491,11 +488,10 @@ void intel_guc_init_fw(struct intel_guc *guc) guc->fw.minor_ver_wanted = KBL_FW_MINOR; } else { DRM_ERROR("No GuC firmware known for platform with GuC!\n"); - i915.enable_guc_loading = 0; - return; + return -ENOENT; } - intel_uc_prepare_fw(dev_priv, >fw); + return 0; } /** diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c index 5fadd55..ea67abc 100644 --- a/drivers/gpu/drm/i915/intel_huc.c +++ b/drivers/gpu/drm/i915/intel_huc.c @@ -141,18 +141,10 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv) } /** - * intel_huc_init_fw() - select and prepare firmware for loading + * intel_huc_select_fw() - selects HuC firmware for loading * @huc: intel_huc struct - * - * Called early during driver load, but after GEM is initialised. The loading - * will continue only when driver explicitly specify firmware name and version. - * All other cases are considered as INTEL_UC_FIRMWARE_NONE either because HW - * is not capable or driver yet support it. And there will be no error message - * for INTEL_UC_FIRMWARE_NONE cases. - * - * The DMA-copying to HW is done later when intel_huc_init_hw() is called. */ -void intel_huc_init_fw(struct intel_huc *huc) +void intel_huc_select_fw(struct intel_huc *huc) { struct drm_i915_private *dev_priv = huc_to_i915(huc); @@ -177,8 +169,6 @@ void intel_huc_init_fw(struct intel_huc *huc) DRM_ERROR("No HuC firmware known for platform with HuC!\n"); return; } - - intel_uc_prepare_fw(dev_priv, >fw); } /** diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 7f9aad7..8875647 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -66,6 +66,14 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) if (!i915.enable_guc_loading) i915.enable_guc_submission = 0; } + + if (i915.enable_guc_loading) { + if (HAS_HUC_UCODE(dev_priv)) + intel_huc_select_fw(_priv->huc); + + if (intel_guc_select_fw(_priv->guc)) + i915.enable_guc_loading = 0; + } } void intel_uc_init_early(struct drm_i915_private *dev_priv) @@ -75,13 +83,11 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv) void intel_uc_init_fw(struct drm_i915_private *dev_priv) { - if (!i915.enable_guc_loading) - return; + if (dev_priv->huc.fw.path) + intel_uc_prepare_fw(dev_priv, _priv->huc.fw); - if (HAS_HUC_UCODE(dev_priv)) - intel_huc_init_fw(_priv->huc); - - intel_guc_init_fw(_priv->guc); + if (dev_priv->guc.fw.path) + intel_uc_prepare_fw(dev_priv, _priv->guc.fw); } int intel_uc_init_hw(struct drm_i915_private *dev_priv) diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index a9a4188..50ed561 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -194,7 +194,7 @@ int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len); int intel_guc_sample_forcewake(struct intel_guc *guc); /*
[Intel-gfx] [PATCH 06/10] drm/i915/guc: Extract param logic form guc_init_fw()
Let intel_guc_init_fw() focus on determining and fetching the correct firmware. This patch introduces intel_uc_sanitize_options() that is called from intel_sanitize_options(). Then, if we have GuC, we can call intel_guc_init_fw() conditionally and we do not have to do the internal checks. v2: fix comment, notify when nuking GuC explicitly enabled (M. Wajdeczko) v3: fix comment again, change the nuke message (M. Wajdeczko) v4: update title to reflect new function name + rebase v5: text && remove 2 uneccessary checks (M. Wajdeczko) Cc: Michal WiniarskiCc: Michal Wajdeczko Cc: Daniele Ceraolo Spurio Cc: Joonas Lahtinen Signed-off-by: Arkadiusz Hiler Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_drv.c | 2 ++ drivers/gpu/drm/i915/intel_guc_loader.c | 18 +- drivers/gpu/drm/i915/intel_huc.c| 3 --- drivers/gpu/drm/i915/intel_uc.c | 28 +++- drivers/gpu/drm/i915/intel_uc.h | 1 + 5 files changed, 31 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 154ea2f..d546c61 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -991,6 +991,8 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv) i915.semaphores = intel_sanitize_semaphores(dev_priv, i915.semaphores); DRM_DEBUG_DRIVER("use GPU semaphores? %s\n", yesno(i915.semaphores)); + + intel_uc_sanitize_options(dev_priv); } /** diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 2e3339d..2761a76 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -601,20 +601,7 @@ void intel_guc_init_fw(struct intel_guc *guc) struct drm_i915_private *dev_priv = guc_to_i915(guc); const char *fw_path; - if (!HAS_GUC(dev_priv)) { - i915.enable_guc_loading = 0; - i915.enable_guc_submission = 0; - } else { - /* A negative value means "use platform default" */ - if (i915.enable_guc_loading < 0) - i915.enable_guc_loading = HAS_GUC_UCODE(dev_priv); - if (i915.enable_guc_submission < 0) - i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv); - } - - if (!HAS_GUC_UCODE(dev_priv)) { - fw_path = NULL; - } else if (IS_SKYLAKE(dev_priv)) { + if (IS_SKYLAKE(dev_priv)) { fw_path = I915_SKL_GUC_UCODE; guc->fw.major_ver_wanted = SKL_FW_MAJOR; guc->fw.minor_ver_wanted = SKL_FW_MINOR; @@ -634,9 +621,6 @@ void intel_guc_init_fw(struct intel_guc *guc) guc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE; guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; - /* Early (and silent) return if GuC loading is disabled */ - if (!i915.enable_guc_loading) - return; if (fw_path == NULL) return; if (*fw_path == '\0') diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c index fda473d..168aab1 100644 --- a/drivers/gpu/drm/i915/intel_huc.c +++ b/drivers/gpu/drm/i915/intel_huc.c @@ -162,9 +162,6 @@ void intel_huc_init_fw(struct intel_huc *huc) huc->fw.load_status = INTEL_UC_FIRMWARE_NONE; huc->fw.fw = INTEL_UC_FW_TYPE_HUC; - if (!HAS_HUC_UCODE(dev_priv)) - return; - if (IS_SKYLAKE(dev_priv)) { fw_path = I915_SKL_HUC_UCODE; huc->fw.major_ver_wanted = SKL_HUC_FW_MAJOR; diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index e5155de..f0a69d4 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -26,6 +26,27 @@ #include "intel_uc.h" #include +void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) +{ + if (!HAS_GUC(dev_priv)) { + if (i915.enable_guc_loading > 0) + DRM_INFO("Ignoring GuC options, no hardware"); + + i915.enable_guc_loading = 0; + i915.enable_guc_submission = 0; + } else { + /* A negative value means "use platform default" */ + if (i915.enable_guc_loading < 0) + i915.enable_guc_loading = HAS_GUC_UCODE(dev_priv); + if (i915.enable_guc_submission < 0) + i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv); + + /* Can't enable guc submission without guc loaded */ + if (!i915.enable_guc_loading) + i915.enable_guc_submission = 0; + } +} + void intel_uc_init_early(struct drm_i915_private *dev_priv) {
[Intel-gfx] [PATCH 05/10] drm/i915/uc: Introduce intel_uc_init_fw()
Instead of calling intel_guc_init() and intel_huc_init() one by one this patch introduces intel_uc_init_fw() function that calls them both. Called functions are renamed accordingly. Trying to have subject_verb_object ordering and more descriptive names, the intel_huc_init() and intel_guc_init() functions are renamed. For guc_init(): * `intel_guc` is the subject, so those functions now take intel_guc structure, instead of the dev_priv * init is the verb * fw is the object which better describes the function's role huc_init() change follows the same reasoning. v2: settle on intel_uc_fetch_fw name (M. Wajdeczko) v3: yet another rename - intel_uc_init_fw (J. Lahtinen) v4: non-trivial rebase Cc: Chris WilsonCc: Joonas Lahtinen Cc: Michal Wajdeczko Cc: Michal Winiarski Signed-off-by: Arkadiusz Hiler Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_drv.c | 3 +-- drivers/gpu/drm/i915/intel_guc_loader.c | 30 - drivers/gpu/drm/i915/intel_huc.c| 39 + drivers/gpu/drm/i915/intel_uc.c | 6 + drivers/gpu/drm/i915/intel_uc.h | 5 +++-- 5 files changed, 45 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index b1e9027..154ea2f 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -607,8 +607,7 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_irq; - intel_huc_init(dev_priv); - intel_guc_init(dev_priv); + intel_uc_init_fw(dev_priv); ret = i915_gem_init(dev_priv); if (ret) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 5fc10d2..2e3339d 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -588,17 +588,17 @@ int intel_guc_init_hw(struct intel_guc *guc) /** - * intel_guc_init() - define parameters and fetch firmware - * @dev_priv: i915 device private + * intel_guc_init_fw() - select and prepare firmware for loading + * @guc: intel_guc struct * * Called early during driver load, but after GEM is initialised. * * The firmware will be transferred to the GuC's memory later, * when intel_guc_init_hw() is called. */ -void intel_guc_init(struct drm_i915_private *dev_priv) +void intel_guc_init_fw(struct intel_guc *guc) { - struct intel_uc_fw *guc_fw = _priv->guc.fw; + struct drm_i915_private *dev_priv = guc_to_i915(guc); const char *fw_path; if (!HAS_GUC(dev_priv)) { @@ -616,23 +616,23 @@ void intel_guc_init(struct drm_i915_private *dev_priv) fw_path = NULL; } else if (IS_SKYLAKE(dev_priv)) { fw_path = I915_SKL_GUC_UCODE; - guc_fw->major_ver_wanted = SKL_FW_MAJOR; - guc_fw->minor_ver_wanted = SKL_FW_MINOR; + guc->fw.major_ver_wanted = SKL_FW_MAJOR; + guc->fw.minor_ver_wanted = SKL_FW_MINOR; } else if (IS_BROXTON(dev_priv)) { fw_path = I915_BXT_GUC_UCODE; - guc_fw->major_ver_wanted = BXT_FW_MAJOR; - guc_fw->minor_ver_wanted = BXT_FW_MINOR; + guc->fw.major_ver_wanted = BXT_FW_MAJOR; + guc->fw.minor_ver_wanted = BXT_FW_MINOR; } else if (IS_KABYLAKE(dev_priv)) { fw_path = I915_KBL_GUC_UCODE; - guc_fw->major_ver_wanted = KBL_FW_MAJOR; - guc_fw->minor_ver_wanted = KBL_FW_MINOR; + guc->fw.major_ver_wanted = KBL_FW_MAJOR; + guc->fw.minor_ver_wanted = KBL_FW_MINOR; } else { fw_path = ""; /* unknown device */ } - guc_fw->path = fw_path; - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE; - guc_fw->load_status = INTEL_UC_FIRMWARE_NONE; + guc->fw.path = fw_path; + guc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE; + guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; /* Early (and silent) return if GuC loading is disabled */ if (!i915.enable_guc_loading) @@ -642,9 +642,9 @@ void intel_guc_init(struct drm_i915_private *dev_priv) if (*fw_path == '\0') return; - guc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING; + guc->fw.fetch_status = INTEL_UC_FIRMWARE_PENDING; DRM_DEBUG_DRIVER("GuC firmware pending, path %s\n", fw_path); - intel_uc_prepare_fw(dev_priv, guc_fw); + intel_uc_prepare_fw(dev_priv, >fw); /* status must now be FAIL or SUCCESS */ } diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c index 36326ca..fda473d 100644 --- a/drivers/gpu/drm/i915/intel_huc.c +++
[Intel-gfx] [PATCH 04/10] drm/i915/uc: Move intel_uc_fw_fetch() to intel_uc.c
The file fits better. Additionally rename it to intel_uc_prepare_fw(), as the function does more than simple fetch. `obj` cleanup in the function is also fixed (i.e. removed). In the fail scenario it was always 'put' but there's no possible flow that initializes the obj properly and then goes to the fail label. v2: remove second declaration, reorder (M. Wajdeczko) v3: non-trivial rebase v4: remove obj cleanup in the fail scenario (C. Wilson) Cc: Chris WilsonCc: Michal Wajdeczko Signed-off-by: Arkadiusz Hiler Reviewed-by: Michal Wajdeczko --- drivers/gpu/drm/i915/intel_guc_loader.c | 137 +--- drivers/gpu/drm/i915/intel_huc.c| 2 +- drivers/gpu/drm/i915/intel_uc.c | 131 ++ drivers/gpu/drm/i915/intel_uc.h | 4 +- 4 files changed, 135 insertions(+), 139 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index cfffafd..5fc10d2 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -26,7 +26,6 @@ *Dave Gordon *Alex Dai */ -#include #include "i915_drv.h" #include "intel_uc.h" @@ -587,140 +586,6 @@ int intel_guc_init_hw(struct intel_guc *guc) return ret; } -void intel_uc_fw_fetch(struct drm_i915_private *dev_priv, -struct intel_uc_fw *uc_fw) -{ - struct pci_dev *pdev = dev_priv->drm.pdev; - struct drm_i915_gem_object *obj; - const struct firmware *fw = NULL; - struct uc_css_header *css; - size_t size; - int err; - - DRM_DEBUG_DRIVER("before requesting firmware: uC fw fetch status %s\n", - intel_uc_fw_status_repr(uc_fw->fetch_status)); - - err = request_firmware(, uc_fw->path, >dev); - if (err) - goto fail; - if (!fw) - goto fail; - - DRM_DEBUG_DRIVER("fetch uC fw from %s succeeded, fw %p\n", - uc_fw->path, fw); - - /* Check the size of the blob before examining buffer contents */ - if (fw->size < sizeof(struct uc_css_header)) { - DRM_NOTE("Firmware header is missing\n"); - goto fail; - } - - css = (struct uc_css_header *)fw->data; - - /* Firmware bits always start from header */ - uc_fw->header_offset = 0; - uc_fw->header_size = (css->header_size_dw - css->modulus_size_dw - - css->key_size_dw - css->exponent_size_dw) * sizeof(u32); - - if (uc_fw->header_size != sizeof(struct uc_css_header)) { - DRM_NOTE("CSS header definition mismatch\n"); - goto fail; - } - - /* then, uCode */ - uc_fw->ucode_offset = uc_fw->header_offset + uc_fw->header_size; - uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32); - - /* now RSA */ - if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) { - DRM_NOTE("RSA key size is bad\n"); - goto fail; - } - uc_fw->rsa_offset = uc_fw->ucode_offset + uc_fw->ucode_size; - uc_fw->rsa_size = css->key_size_dw * sizeof(u32); - - /* At least, it should have header, uCode and RSA. Size of all three. */ - size = uc_fw->header_size + uc_fw->ucode_size + uc_fw->rsa_size; - if (fw->size < size) { - DRM_NOTE("Missing firmware components\n"); - goto fail; - } - - /* -* The GuC firmware image has the version number embedded at a well-known -* offset within the firmware blob; note that major / minor version are -* TWO bytes each (i.e. u16), although all pointers and offsets are defined -* in terms of bytes (u8). -*/ - switch (uc_fw->fw) { - case INTEL_UC_FW_TYPE_GUC: - /* Header and uCode will be loaded to WOPCM. Size of the two. */ - size = uc_fw->header_size + uc_fw->ucode_size; - - /* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */ - if (size > intel_guc_wopcm_size(dev_priv)) { - DRM_ERROR("Firmware is too large to fit in WOPCM\n"); - goto fail; - } - uc_fw->major_ver_found = css->guc.sw_version >> 16; - uc_fw->minor_ver_found = css->guc.sw_version & 0x; - break; - - case INTEL_UC_FW_TYPE_HUC: - uc_fw->major_ver_found = css->huc.sw_version >> 16; - uc_fw->minor_ver_found = css->huc.sw_version & 0x; - break; - - default: - DRM_ERROR("Unknown firmware type %d\n", uc_fw->fw); - err = -ENOEXEC; - goto fail; - } - - if (uc_fw->major_ver_found != uc_fw->major_ver_wanted || -
[Intel-gfx] [PATCH v7 00/10] GuC Scrub vol. 1
Reasoning = General GuC/HuC cleanup simplifying logic and moving chunks around as the area got pretty rusty. This is the first part of effort to clean it up. A lot of logic were extracted from intel_guc_load() to other functions - it did not only handle the actual loading but had WA implementations and the code that enabled submission baked into it. Param sanitization and firmware selection are also extracted and streamlined. Naming: === I try to adhere to subject_verb_object naming and parameters. e.g. for intel_guc_init_fw: * intel_guc is the subject, it determines the first argument taken * init is the verb * fwis the object intel_guc_ functions take intel_guc struct pointer intel_huc_ functions take intel_guc struct pointer There's no `struct intel_uc`, so this family of functions take `dev_priv`. New structure looks like this: == 1. sanitize params - module params + selecting firmware happens at this stage * this is done mostly in patch 8, and 9 * some cleanups happen along th way (e.g. pushing check ups, and changeing prototypes) * patch 10 introduces new modules params to overload used firmware 2. init_fw - requesting and initial parsing of firmware * firmware is read and parsed, we also check for versions to match 3. init_hw - firmware is loaded into hardware and hardware is initalized * uc_init_hw has now all logic when it comes to retires and resetting fw to vanilla state * huc_init_hw and guc_init_hw no longer care about each other and submission v2: rebase after HuC merge + feedback v3: even more renaming that aims to make things more semantic v4: some naming improvements, some bikeshedding v5: coding style, some cleanup module params for huc and guc firmware path, separate fw select step from actual prepare v6: feedback + pushed a couple of patches with r-b down the stack v7: reorder, rename, rebase Arkadiusz Hiler (10): drm/i915/uc: Drop superfluous externs in intel_uc.h drm/i915/huc: Add huc_to_i915 drm/i915/uc: Rename intel_?uc_{setup,load}() to _init_hw() drm/i915/uc: Move intel_uc_fw_fetch() to intel_uc.c drm/i915/uc: Introduce intel_uc_init_fw() drm/i915/guc: Extract param logic form guc_init_fw() drm/i915/guc: Simplify intel_guc_init_hw() drm/i915/uc: Simplify firmware path handling drm/i915/uc: Separate firmware selection and preparation drm/i915/uc: Add params for specifying firmware drivers/gpu/drm/i915/i915_drv.c | 5 +- drivers/gpu/drm/i915/i915_drv.h | 5 + drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_params.c | 10 + drivers/gpu/drm/i915/i915_params.h | 2 + drivers/gpu/drm/i915/intel_guc_loader.c | 389 +--- drivers/gpu/drm/i915/intel_huc.c| 109 - drivers/gpu/drm/i915/intel_uc.c | 287 +++ drivers/gpu/drm/i915/intel_uc.h | 25 +- 9 files changed, 421 insertions(+), 413 deletions(-) -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/10] drm/i915/uc: Drop superfluous externs in intel_uc.h
Externs are implicit and we generally try to avoid them. Cc: Michal WajdeczkoSigned-off-by: Arkadiusz Hiler Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_uc.h | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index d74f4d3..bf72342 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -189,12 +189,12 @@ int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len); int intel_guc_sample_forcewake(struct intel_guc *guc); /* intel_guc_loader.c */ -extern void intel_guc_init(struct drm_i915_private *dev_priv); -extern int intel_guc_setup(struct drm_i915_private *dev_priv); -extern void intel_guc_fini(struct drm_i915_private *dev_priv); -extern const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status); -extern int intel_guc_suspend(struct drm_i915_private *dev_priv); -extern int intel_guc_resume(struct drm_i915_private *dev_priv); +void intel_guc_init(struct drm_i915_private *dev_priv); +int intel_guc_setup(struct drm_i915_private *dev_priv); +void intel_guc_fini(struct drm_i915_private *dev_priv); +const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status); +int intel_guc_suspend(struct drm_i915_private *dev_priv); +int intel_guc_resume(struct drm_i915_private *dev_priv); void intel_uc_fw_fetch(struct drm_i915_private *dev_priv, struct intel_uc_fw *uc_fw); u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv); -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915/guc: Break out the GuC log "extras"
On Fri, Feb 24, 2017 at 06:01:37AM -0800, Oscar Mateo wrote: > When initializing the GuC log struct, there is an object we need to > allocate always, since the GuC needs its address at fw load time. > The rest are "extras", in the sense that we only need them if we > actually enable GuC logging. Make that distinction explicit by > subdividing further the intel_guc_log struct. > > Cc: Daniele Ceraolo Spurio> Cc: Joonas Lahtinen > Signed-off-by: Oscar Mateo > --- Reviewed-by: Michal Wajdeczko Regards, Michal ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915/guc: s/ads_vma/addon
On Fri, Feb 24, 2017 at 06:01:36AM -0800, Oscar Mateo wrote: > This vma contains much more than just the additional data struct (ads) > and since we were already using the word "addon" as an object in > guc_addon_create, make it the preffered one. No need for the vma suffix > either, as that dependency is given by the type. Hmm, but almost all over the driver we're using "vma" as a name or suffix. Why do you want to change this here ? Also, GUC still is using term "ADS" for this blob so I'm not convinced that we need new name here. -Michal > > while at it, add an explanation of what things go inside the addon object. > > Cc: Daniele Ceraolo Spurio> Cc: Joonas Lahtinen > Signed-off-by: Oscar Mateo > --- > drivers/gpu/drm/i915/i915_guc_submission.c | 12 > drivers/gpu/drm/i915/intel_guc_loader.c| 4 ++-- > drivers/gpu/drm/i915/intel_uc.h| 2 +- > 3 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c > b/drivers/gpu/drm/i915/i915_guc_submission.c > index 7562343c..e1922fe 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -834,9 +834,13 @@ static int guc_addon_create(struct intel_guc *guc) > struct page *page; > u32 size; > > - GEM_BUG_ON(guc->ads_vma); > + GEM_BUG_ON(guc->addon); > > - /* The ads obj includes the struct itself and buffers passed to GuC */ > + /* The additional data struct (ADS) has pointers for different buffers > + * used by the GuC. The addon object contains the ADS itself (guc_ads), > + * the scheduling policies (guc_policies), a structure describing > + * a collection of register sets (guc_mmio_reg_state) and some extra > + * pages for the GuC to save its internal state for sleep */ > size = sizeof(struct guc_ads) + sizeof(struct guc_policies) + > sizeof(struct guc_mmio_reg_state) + > GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE; > @@ -845,7 +849,7 @@ static int guc_addon_create(struct intel_guc *guc) > if (IS_ERR(vma)) > return PTR_ERR(vma); > > - guc->ads_vma = vma; > + guc->addon = vma; > > page = i915_vma_first_page(vma); > ads = kmap(page); > @@ -894,7 +898,7 @@ static int guc_addon_create(struct intel_guc *guc) > > static void guc_addon_destroy(struct intel_guc *guc) > { > - i915_vma_unpin_and_release(>ads_vma); > + i915_vma_unpin_and_release(>addon); > } > > /* > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index 1f9ec54..1eb0c51 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -213,8 +213,8 @@ static void guc_params_init(struct drm_i915_private > *dev_priv) > } else > params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED; > > - if (guc->ads_vma) { > - u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT; > + if (guc->addon) { > + u32 ads = guc_ggtt_offset(guc->addon) >> PAGE_SHIFT; > params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT; > params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED; > } > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h > index 330d08f..d8897b5 100644 > --- a/drivers/gpu/drm/i915/intel_uc.h > +++ b/drivers/gpu/drm/i915/intel_uc.h > @@ -152,7 +152,7 @@ struct intel_guc { > /* intel_guc_recv interrupt related state */ > bool interrupts_enabled; > > - struct i915_vma *ads_vma; > + struct i915_vma *addon; > struct i915_vma *ctx_pool; > void *ctx_pool_vaddr; > struct ida ctx_ids; > -- > 1.9.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 series starting with [v3] drm/i915: Store a permanent error in obj->mm.pages (rev2)
== Series Details == Series: series starting with [v3] drm/i915: Store a permanent error in obj->mm.pages (rev2) URL : https://patchwork.freedesktop.org/series/20829/ State : success == Summary == Series 20829v2 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/20829/revisions/2/mbox/ Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: dmesg-warn -> PASS (fi-byt-n2820) fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 465s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 608s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 534s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 624s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 509s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 503s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 436s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 437s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 437s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 512s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 492s fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 477s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 516s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 598s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 501s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 556s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 553s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 406s ba93250bd451d62c73db7ed39242a625730424f2 drm-tip: 2017y-03m-07d-13h-23m-43s UTC integration manifest faada78 drm/i915: Use pagecache write to prepopulate shmemfs from pwrite-ioctl 8f3f9fb drm/i915: Store a permanent error in obj->mm.pages == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4084/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/15] drm/i915: add page_size_mask to dev_info
On Mon, Mar 06, 2017 at 11:54:03PM +, Matthew Auld wrote: > Signed-off-by: Matthew Auld> --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem_gtt.h | 14 ++ > drivers/gpu/drm/i915/i915_pci.c | 23 ++- > 3 files changed, 37 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 1fd4128a10b1..e45b8d74cebf 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -913,6 +913,7 @@ struct intel_device_info { > enum intel_platform platform; > u8 ring_mask; /* Rings supported by the HW */ > u8 num_rings; > + unsigned long page_size_mask; /* page sizes supported by the HW */ > #define DEFINE_FLAG(name) u8 name:1 > DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG); > #undef DEFINE_FLAG > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h > b/drivers/gpu/drm/i915/i915_gem_gtt.h > index fb15684c1d83..6c90a2ffd0e1 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -43,8 +43,18 @@ > #include "i915_selftest.h" > > #define I915_GTT_PAGE_SIZE 4096UL > +#define I915_GTT_PAGE_SIZE_64K 65536UL > +#define I915_GTT_PAGE_SIZE_2M 2097152UL > +#define I915_GTT_PAGE_SIZE_1G 1073741824UL I915_GTT_PAGE_SIZE_4K BIT(12) I915_GTT_PAGE_SIZE_64K BIT(16) I915_GTT_PAGE_SIZE_2M BIT(21) I915_GTT_PAGE_SIZE_1G BIT(30) #define I915_GTT_PAGE_SIZE I915_GTT_PAGE_SIZE_4K Still debating the relative merits of a tight enum. Note that you want to scatter #define assert_valid_gtt_page_size(page_size) \ GEM_BUG_ON(!is_power_of_2(page_size) || \ page_size & ~I915_GTT_PAGE_SIZE_MASK); around or GEM_BUG_ON(is_valid_gtt_page_size(page_size))? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/7] drm/i915/dsi: stop using drm_panel, refactor
On Mon, 06 Mar 2017, Jani Nikulawrote: > This is pretty natural continuation to what Hans started. We haven't > made use of the drm_panel framework much at all, and there's no need in > sight, really. It was good for ensuring a certain amount of separation > between the core and the VBT stuff. Let's keep things that way, but > without the interface. Pushed the lot, thanks for the review everyone! BR, Jani. > > BR, > Jani. > > Jani Nikula (7): > drm/i915/dsi: remove support for more than one panel driver > drm/i915/dsi: call vbt_panel_get_modes directly instead of via > drm_panel > drm/i915/dsi: stop using the drm_panel framework completely > drm/i915/dsi: rename intel_dsi_exec_vbt_sequence to > intel_dsi_vbt_exec_sequence > drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable > drm/i915/dsi: rename intel_dsi_panel_vbt.c to intel_dsi_vbt.c > drm/i915/dsi: arrange intel_dsi.h according to relevant files > > drivers/gpu/drm/i915/Makefile | 2 +- > drivers/gpu/drm/i915/intel_dsi.c | 76 > -- > drivers/gpu/drm/i915/intel_dsi.h | 14 ++-- > .../{intel_dsi_panel_vbt.c => intel_dsi_vbt.c} | 46 +++-- > 4 files changed, 43 insertions(+), 95 deletions(-) > rename drivers/gpu/drm/i915/{intel_dsi_panel_vbt.c => intel_dsi_vbt.c} (95%) -- 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] Implementing Miracast
On 07/03/17 05:00, Daniel Kasak wrote: Any news on this? I'm also interested :) Dan Hmm, good question! I will ping internally and see if we are ready to release something as an RFC. Martin ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915: Store a permanent error in obj->mm.pages
Once the object has been truncated, it is unrecoverable. To facilitate detection of this state store the error in obj->mm.pages. This is required for the next patch which should be applied to v4.10 (via stable), so we also need to mark this patch for backporting. In that regard, let's consider this to be a fix/improvement too. v2: Avoid dereferencing the ERR_PTR when freeing the object. Fixes: 1233e2db199d ("drm/i915: Move object backing storage manipulation to its own locking") Signed-off-by: Chris WilsonCc: Joonas Lahtinen Cc: # v4.10+ --- drivers/gpu/drm/i915/i915_gem.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 7c20601fe1de..7ec2881de710 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2130,6 +2130,7 @@ i915_gem_object_truncate(struct drm_i915_gem_object *obj) */ shmem_truncate_range(file_inode(obj->base.filp), 0, (loff_t)-1); obj->mm.madv = __I915_MADV_PURGED; + obj->mm.pages = ERR_PTR(-EFAULT); } /* Try to discard unwanted pages */ @@ -2229,7 +2230,9 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj, __i915_gem_object_reset_page_iter(obj); - obj->ops->put_pages(obj, pages); + if (!IS_ERR(pages)) + obj->ops->put_pages(obj, pages); + unlock: mutex_unlock(>mm.lock); } @@ -2449,7 +2452,7 @@ int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj) if (err) return err; - if (unlikely(!obj->mm.pages)) { + if (unlikely(IS_ERR_OR_NULL(obj->mm.pages))) { err = i915_gem_object_get_pages(obj); if (err) goto unlock; @@ -2527,7 +2530,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, pinned = true; if (!atomic_inc_not_zero(>mm.pages_pin_count)) { - if (unlikely(!obj->mm.pages)) { + if (unlikely(IS_ERR_OR_NULL(obj->mm.pages))) { ret = i915_gem_object_get_pages(obj); if (ret) goto err_unlock; -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v2,1/2] drm/i915: Store a permanent error in obj->mm.pages
On Tue, Mar 07, 2017 at 01:12:42PM +, Chris Wilson wrote: > On Tue, Mar 07, 2017 at 12:57:19PM -, Patchwork wrote: > > == Series Details == > > > > Series: series starting with [v2,1/2] drm/i915: Store a permanent error in > > obj->mm.pages > > URL : https://patchwork.freedesktop.org/series/20829/ > > State : failure > > > > == Summary == > > > > Series 20829v1 Series without cover letter > > https://patchwork.freedesktop.org/api/1.0/series/20829/revisions/1/mbox/ > > > > Test gem_exec_flush: > > Subgroup basic-batch-kernel-default-uc: > > fail -> PASS (fi-snb-2600) fdo#17 > > Test gem_exec_suspend: > > Subgroup basic-s3: > > dmesg-warn -> PASS (fi-snb-2600) > > Test gem_tiled_fence_blits: > > Subgroup basic: > > pass -> INCOMPLETE (fi-hsw-4770) > > pass -> INCOMPLETE (fi-bxt-t5700) > > pass -> INCOMPLETE (fi-byt-j1900) > > pass -> INCOMPLETE (fi-bdw-5557u) > > pass -> INCOMPLETE (fi-ivb-3520m) > > pass -> INCOMPLETE (fi-ilk-650) > > pass -> INCOMPLETE (fi-skl-6700hq) > > pass -> INCOMPLETE (fi-snb-2520m) > > pass -> INCOMPLETE (fi-skl-6770hq) > > pass -> INCOMPLETE (fi-skl-6260u) > > pass -> INCOMPLETE (fi-snb-2600) > > pass -> INCOMPLETE (fi-skl-6700k) > > I choose the wrong subset of tests to try with pages = -EFAULT! Ah, worse it works for me because my async-patches are prepared for pages = ERR_PTR. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v2,1/2] drm/i915: Store a permanent error in obj->mm.pages
On Tue, Mar 07, 2017 at 12:57:19PM -, Patchwork wrote: > == Series Details == > > Series: series starting with [v2,1/2] drm/i915: Store a permanent error in > obj->mm.pages > URL : https://patchwork.freedesktop.org/series/20829/ > State : failure > > == Summary == > > Series 20829v1 Series without cover letter > https://patchwork.freedesktop.org/api/1.0/series/20829/revisions/1/mbox/ > > Test gem_exec_flush: > Subgroup basic-batch-kernel-default-uc: > fail -> PASS (fi-snb-2600) fdo#17 > Test gem_exec_suspend: > Subgroup basic-s3: > dmesg-warn -> PASS (fi-snb-2600) > Test gem_tiled_fence_blits: > Subgroup basic: > pass -> INCOMPLETE (fi-hsw-4770) > pass -> INCOMPLETE (fi-bxt-t5700) > pass -> INCOMPLETE (fi-byt-j1900) > pass -> INCOMPLETE (fi-bdw-5557u) > pass -> INCOMPLETE (fi-ivb-3520m) > pass -> INCOMPLETE (fi-ilk-650) > pass -> INCOMPLETE (fi-skl-6700hq) > pass -> INCOMPLETE (fi-snb-2520m) > pass -> INCOMPLETE (fi-skl-6770hq) > pass -> INCOMPLETE (fi-skl-6260u) > pass -> INCOMPLETE (fi-snb-2600) > pass -> INCOMPLETE (fi-skl-6700k) I choose the wrong subset of tests to try with pages = -EFAULT! -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid clearing the base drm_crtc_state
On Mon, Mar 06, 2017 at 09:05:47PM +, Chris Wilson wrote: > On Mon, Mar 06, 2017 at 06:46:16PM +0100, Daniel Vetter wrote: > > On Fri, Mar 03, 2017 at 03:46:44PM +, Chris Wilson wrote: > > > @@ -11289,9 +11287,9 @@ clear_intel_crtc_state(struct intel_crtc_state > > > *crtc_state) > > > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > > > wm_state = crtc_state->wm; > > > > > > - memset(crtc_state, 0, sizeof *crtc_state); > > > + memset(_state->base + 1, 0, > > > +sizeof(*crtc_state) - sizeof(crtc_state->base)); > > > > Maybe add a comment like /* Only clear our part of the overall struct. */ > > or similar, since this is not entirely obvious what's going on. Also > > > > COMPILE_BUG_ON(offsetof(struct intel_crtc_state, base) != 0); > > > > maybe? Anyway, I'll let you decide on both, either way: > > I thought we had the BUILD_BUG_ON already -- the NULL correspondance is > widely used. I did one version with it, then thought that it would be > better in to_intel_crtc_state() and so left it out. I posted a patch for that once but I never finished it. In the end I was even thinking of having both const and non-const to_foo() macros. If someone wants to continue my work I pushed it here: git://github.com/vsyrjala/linux.git kms_obj_base_0_2 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v2,1/2] drm/i915: Store a permanent error in obj->mm.pages
== Series Details == Series: series starting with [v2,1/2] drm/i915: Store a permanent error in obj->mm.pages URL : https://patchwork.freedesktop.org/series/20829/ State : failure == Summary == Series 20829v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/20829/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: fail -> PASS (fi-snb-2600) fdo#17 Test gem_exec_suspend: Subgroup basic-s3: dmesg-warn -> PASS (fi-snb-2600) Test gem_tiled_fence_blits: Subgroup basic: pass -> INCOMPLETE (fi-hsw-4770) pass -> INCOMPLETE (fi-bxt-t5700) pass -> INCOMPLETE (fi-byt-j1900) pass -> INCOMPLETE (fi-bdw-5557u) pass -> INCOMPLETE (fi-ivb-3520m) pass -> INCOMPLETE (fi-ilk-650) pass -> INCOMPLETE (fi-skl-6700hq) pass -> INCOMPLETE (fi-snb-2520m) pass -> INCOMPLETE (fi-skl-6770hq) pass -> INCOMPLETE (fi-skl-6260u) pass -> INCOMPLETE (fi-snb-2600) pass -> INCOMPLETE (fi-skl-6700k) Test kms_frontbuffer_tracking: Subgroup basic: pass -> DMESG-FAIL (fi-bxt-j4205) pass -> DMESG-FAIL (fi-kbl-7500u) pass -> DMESG-FAIL (fi-hsw-4770r) pass -> DMESG-FAIL (fi-byt-n2820) pass -> DMESG-FAIL (fi-ivb-3770) Test kms_pipe_crc_basic: Subgroup bad-nb-words-1: pass -> INCOMPLETE (fi-bxt-j4205) pass -> INCOMPLETE (fi-kbl-7500u) pass -> INCOMPLETE (fi-hsw-4770r) pass -> INCOMPLETE (fi-byt-n2820) pass -> INCOMPLETE (fi-ivb-3770) fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fi-bdw-5557u total:153 pass:149 dwarn:0 dfail:0 fail:0 skip:3 time: 0s fi-bxt-j4205 total:216 pass:198 dwarn:0 dfail:1 fail:0 skip:16 time: 0s fi-bxt-t5700 total:153 pass:139 dwarn:0 dfail:0 fail:0 skip:13 time: 0s fi-byt-j1900 total:153 pass:139 dwarn:0 dfail:0 fail:0 skip:13 time: 0s fi-byt-n2820 total:216 pass:192 dwarn:0 dfail:1 fail:0 skip:22 time: 0s fi-hsw-4770 total:153 pass:144 dwarn:0 dfail:0 fail:0 skip:8 time: 0s fi-hsw-4770r total:216 pass:201 dwarn:0 dfail:1 fail:0 skip:13 time: 0s fi-ilk-650 total:153 pass:121 dwarn:0 dfail:0 fail:0 skip:31 time: 0s fi-ivb-3520m total:153 pass:140 dwarn:0 dfail:0 fail:0 skip:12 time: 0s fi-ivb-3770 total:216 pass:201 dwarn:0 dfail:1 fail:0 skip:13 time: 0s fi-kbl-7500u total:216 pass:198 dwarn:1 dfail:1 fail:0 skip:15 time: 0s fi-skl-6260u total:153 pass:149 dwarn:0 dfail:0 fail:0 skip:3 time: 0s fi-skl-6700hqtotal:153 pass:141 dwarn:0 dfail:0 fail:0 skip:11 time: 0s fi-skl-6700k total:153 pass:141 dwarn:0 dfail:0 fail:0 skip:11 time: 0s fi-skl-6770hqtotal:153 pass:149 dwarn:0 dfail:0 fail:0 skip:3 time: 0s fi-snb-2520m total:153 pass:136 dwarn:0 dfail:0 fail:0 skip:16 time: 0s fi-snb-2600 total:153 pass:136 dwarn:0 dfail:0 fail:0 skip:16 time: 0s c07cf6d486149608afb2d37a33a00775c3ac312b drm-tip: 2017y-03m-07d-11h-07m-45s UTC integration manifest 81123cc drm/i915: Use pagecache write to prepopulate shmemfs from pwrite-ioctl a47f52c drm/i915: Store a permanent error in obj->mm.pages == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4083/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Avoid clearing the base drm_crtc_state
On Mon, Mar 06, 2017 at 06:46:16PM +0100, Daniel Vetter wrote: > On Fri, Mar 03, 2017 at 03:46:44PM +, Chris Wilson wrote: > > To prevent having to preserve the drm_crtc_state as we clear the > > intel_crtc_state, only memset our extended state. > > > > Fixes: > > drivers/gpu/drm/i915/intel_display.c: In function ‘clear_intel_crtc_state’: > > drivers/gpu/drm/i915/intel_display.c:11301:1: error: the frame size of 1056 > > bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > > > > Signed-off-by: Chris Wilson> > --- > > drivers/gpu/drm/i915/intel_display.c | 6 ++ > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index e6881a69a88f..cfab4d135af3 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -11269,7 +11269,6 @@ clear_intel_crtc_state(struct intel_crtc_state > > *crtc_state) > > { > > struct drm_i915_private *dev_priv = > > to_i915(crtc_state->base.crtc->dev); > > - struct drm_crtc_state tmp_state; > > struct intel_crtc_scaler_state scaler_state; > > struct intel_dpll_hw_state dpll_hw_state; > > struct intel_shared_dpll *shared_dpll; > > @@ -11281,7 +11280,6 @@ clear_intel_crtc_state(struct intel_crtc_state > > *crtc_state) > > * fixed, so that the crtc_state can be safely duplicated. For now, > > * only fields that are know to not cause problems are preserved. */ > > > > - tmp_state = crtc_state->base; > > scaler_state = crtc_state->scaler_state; > > shared_dpll = crtc_state->shared_dpll; > > dpll_hw_state = crtc_state->dpll_hw_state; > > @@ -11289,9 +11287,9 @@ clear_intel_crtc_state(struct intel_crtc_state > > *crtc_state) > > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > > wm_state = crtc_state->wm; > > > > - memset(crtc_state, 0, sizeof *crtc_state); > > + memset(_state->base + 1, 0, > > + sizeof(*crtc_state) - sizeof(crtc_state->base)); > > Maybe add a comment like /* Only clear our part of the overall struct. */ > or similar, since this is not entirely obvious what's going on. Also > > COMPILE_BUG_ON(offsetof(struct intel_crtc_state, base) != 0); > > maybe? Anyway, I'll let you decide on both, either way: > > Reviewed-by: Daniel Vetter Did both and pushed. Thanks, a few of my machines were tripping over this which is a bit puzzling as to how they have different values for the stack warning. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/guc: Use formalized struct definition for ads object
On Tue, Mar 07, 2017 at 11:09:36AM +, Michal Wajdeczko wrote: > Manual pointer manipulation is error prone. Let compiler calculate > right offsets for us in case we need to change ads layout. > > v2: don't call it object (Chris) > > Signed-off-by: Michal Wajdeczko> Cc: Oscar Mateo > Cc: Joonas Lahtinen > Cc: Daniele Ceraolo Spurio > Cc: Chris Wilson Looks good. I would have done mine line splitting slightly differently. For > - ads->reg_state_buffer = ads->reg_state_addr + > - sizeof(struct guc_mmio_reg_state); > + blob->ads.reg_state_buffer = offset + > + offsetof(struct __guc_ads_blob, reg_state_buffer); I would have used blob->ads.reg_state_buffer = offset + offsetof(struct __guc_ads_blob, reg_state_buffer); I didn't see any mistakes in the conversion, and the packing is certainly more obvious now. Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ 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/guc: Use formalized struct definition for ads object (rev2)
== Series Details == Series: drm/i915/guc: Use formalized struct definition for ads object (rev2) URL : https://patchwork.freedesktop.org/series/20826/ State : success == Summary == Series 20826v2 drm/i915/guc: Use formalized struct definition for ads object https://patchwork.freedesktop.org/api/1.0/series/20826/revisions/2/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: fail -> PASS (fi-snb-2600) fdo#17 Test gem_exec_suspend: Subgroup basic-s3: dmesg-warn -> PASS (fi-snb-2600) fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 461s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 614s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 548s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 620s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 507s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 500s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 440s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 435s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 437s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 509s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 478s fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 485s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 511s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 595s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 506s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 552s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 552s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 423s c07cf6d486149608afb2d37a33a00775c3ac312b drm-tip: 2017y-03m-07d-11h-07m-45s UTC integration manifest ed1bc95 drm/i915/guc: Use formalized struct definition for ads object == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4082/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/2] drm/i915: Use pagecache write to prepopulate shmemfs from pwrite-ioctl
Before we instantiate/pin the backing store for our use, we can prepopulate the shmemfs filp efficiently using a write into the pagecache. We avoid the penalty of instantiating all the pages, important if the user is just writing to a few and never uses the object on the GPU, and using a direct write into shmemfs allows it to avoid the cost of retrieving a page (mostly the clear-before-use, but in theory we could curtail swapin) before it is overwritten. This can be extended later to provide additional specialisation for other backends (other than shmemfs). For now it provides a defense against very large write-only allocations from exhausting all of system memory. v2: Smelling fixes. Fixes: fe115628d567 ("drm/i915: Implement pwrite without struct-mutex") References: https://bugs.freedesktop.org/show_bug.cgi?id=99107 Signed-off-by: Chris WilsonCc: Matthew Auld Cc: Joonas Lahtinen Cc: Mika Kuoppala Cc: # v4.10+ Reviewed-by: Joonas Lahtinen Reviewed-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem.c| 78 ++ drivers/gpu/drm/i915/i915_gem_object.h | 3 ++ 2 files changed, 81 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 1f92d25ca27d..d00203d538d4 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1452,6 +1452,12 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, trace_i915_gem_object_pwrite(obj, args->offset, args->size); + ret = -ENODEV; + if (obj->ops->pwrite) + ret = obj->ops->pwrite(obj, args); + if (ret != -ENODEV) + goto err; + ret = i915_gem_object_wait(obj, I915_WAIT_INTERRUPTIBLE | I915_WAIT_ALL, @@ -2576,6 +2582,75 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, goto out_unlock; } +static int +i915_gem_object_pwrite_gtt(struct drm_i915_gem_object *obj, + const struct drm_i915_gem_pwrite *arg) +{ + struct address_space *mapping = obj->base.filp->f_mapping; + char __user *user_data = u64_to_user_ptr(arg->data_ptr); + u64 remain, offset; + unsigned int pg; + + /* Before we instantiate/pin the backing store for our use, we +* can prepopulate the shmemfs filp efficiently using a write into +* the pagecache. We avoid the penalty of instantiating all the +* pages, important if the user is just writing to a few and never +* uses the object on the GPU, and using a direct write into shmemfs +* allows it to avoid the cost of retrieving a page (either swapin +* or clearing-before-use) before it is overwritten. +*/ + if (READ_ONCE(obj->mm.pages)) + return -ENODEV; + + /* Before the pages are instantiated the object is treated as being +* in the CPU domain. The pages will be clflushed as required before +* use, and we can freely write into the pages directly. If userspace +* races pwrite with any other operation; corruption will ensue - +* that is userspace's prerogative! +*/ + + remain = arg->size; + offset = arg->offset; + pg = offset_in_page(offset); + + do { + unsigned int len, unwritten; + struct page *page; + void *data, *vaddr; + int err; + + len = PAGE_SIZE - pg; + if (len > remain) + len = remain; + + err = pagecache_write_begin(obj->base.filp, mapping, + offset, len, 0, + , ); + if (err < 0) + return err; + + vaddr = kmap(page); + unwritten = copy_from_user(vaddr + pg, user_data, len); + kunmap(page); + + err = pagecache_write_end(obj->base.filp, mapping, + offset, len, len - unwritten, + page, data); + if (err < 0) + return err; + + if (unwritten) + return -EFAULT; + + remain -= len; + user_data += len; + offset += len; + pg = 0; + } while (remain); + + return 0; +} + static bool ban_context(const struct i915_gem_context *ctx) { return (i915_gem_context_is_bannable(ctx) && @@ -3992,8 +4067,11 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj, static const struct drm_i915_gem_object_ops i915_gem_object_ops = { .flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE |
[Intel-gfx] [PATCH v2 1/2] drm/i915: Store a permanent error in obj->mm.pages
Once the object has been truncated, it is unrecoverable. To facilitate detection of this state store the error in obj->mm.pages. This is required for the next patch which should be applied to v4.10 (via stable), so we also need to mark this patch for backporting. In that regard, let's consider this to be a fix/improvement too. Fixes: 1233e2db199d ("drm/i915: Move object backing storage manipulation to its own locking") Signed-off-by: Chris WilsonCc: Joonas Lahtinen Cc: # v4.10+ --- drivers/gpu/drm/i915/i915_gem.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 7c20601fe1de..1f92d25ca27d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2130,6 +2130,7 @@ i915_gem_object_truncate(struct drm_i915_gem_object *obj) */ shmem_truncate_range(file_inode(obj->base.filp), 0, (loff_t)-1); obj->mm.madv = __I915_MADV_PURGED; + obj->mm.pages = ERR_PTR(-EFAULT); } /* Try to discard unwanted pages */ @@ -2449,7 +2450,7 @@ int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj) if (err) return err; - if (unlikely(!obj->mm.pages)) { + if (unlikely(IS_ERR_OR_NULL(obj->mm.pages))) { err = i915_gem_object_get_pages(obj); if (err) goto unlock; @@ -2527,7 +2528,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, pinned = true; if (!atomic_inc_not_zero(>mm.pages_pin_count)) { - if (unlikely(!obj->mm.pages)) { + if (unlikely(IS_ERR_OR_NULL(obj->mm.pages))) { ret = i915_gem_object_get_pages(obj); if (ret) goto err_unlock; -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Use pagecache write to prepopulate shmemfs from pwrite-ioctl
On Tue, Mar 07, 2017 at 01:46:36PM +0200, Petri Latvala wrote: > On Tue, Mar 07, 2017 at 10:22:09AM +, Chris Wilson wrote: > > > igt@gem_madvise@dontneed-before-exec: pass -> {'fail', 'incomplete'} > > That test is expected to fail, that it ever passed is a fluke. > > That subtest should be removed from IGT then? Or updated for consistency. It's the odd-one-out in that test set atm. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Use pagecache write to prepopulate shmemfs from pwrite-ioctl
On Tue, Mar 07, 2017 at 10:22:09AM +, Chris Wilson wrote: > > igt@gem_madvise@dontneed-before-exec: pass -> {'fail', 'incomplete'} > That test is expected to fail, that it ever passed is a fluke. That subtest should be removed from IGT then? > incomplete there should be a failure in the runner? That should quite > happily spot the purged object in execbuf. The test executed right before was igt@drv_missed_irq and the used versions of IGT and kernel did not have the recent fixes related to that test. Quite possible that the incomplete result was due to that one. 'incomplete' here means that the test didn't finish and the machine was killed a watchdog. 'fail' also being in the set is because the test order was different in that run. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/userptr: Disallow wrapping GTT into a userptr
On Tue, Mar 07, 2017 at 11:18:53AM +, Chris Wilson wrote: > Simplest patch is then fun with obj->userptr.work, i.e. something like > if (xchg(>userptr.work, NULL)) return 0; Overly simplistic. Too many holes to plug between potential users of the pages and the current cancel_userptr. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/userptr: Disallow wrapping GTT into a userptr
On Tue, Mar 07, 2017 at 11:03:03AM +, Tvrtko Ursulin wrote: > > On 07/03/2017 09:13, Chris Wilson wrote: > >On Tue, Mar 07, 2017 at 08:42:26AM +, Tvrtko Ursulin wrote: > >> > >>On 06/03/2017 15:36, Chris Wilson wrote: > >>>If we allow the user to convert a GTT mmap address into a userptr, we > >>>may end up in recursion hell, where currently we hit a mutex deadlock > >>>but other possibilities include use-after-free during the > >>>unbind/cancel_userptr. > >> > >>I thought we already disallowed that and indeed > >>igt/gem_userptr_blits/invalid-gtt-mapping succeeds here - what am I > >>missing? > > > >Michał presented this deadlock: > > > >[ 143.203989] gem_userptr_bli D0 902898 0x > >[ 143.204054] Call Trace: > >[ 143.204137] __schedule+0x511/0x1180 > >[ 143.204195] ? pci_mmcfg_check_reserved+0xc0/0xc0 > >[ 143.204274] schedule+0x57/0xe0 > >[ 143.204327] schedule_timeout+0x383/0x670 > >[ 143.204374] ? trace_hardirqs_on_caller+0x187/0x280 > >[ 143.204457] ? trace_hardirqs_on_thunk+0x1a/0x1c > >[ 143.204507] ? usleep_range+0x110/0x110 > >[ 143.204657] ? irq_exit+0x89/0x100 > >[ 143.204710] ? retint_kernel+0x2d/0x2d > >[ 143.204794] ? trace_hardirqs_on_caller+0x187/0x280 > >[ 143.204857] ? _raw_spin_unlock_irq+0x33/0x60 > >[ 143.204944] wait_for_common+0x1f0/0x2f0 > >[ 143.205006] ? out_of_line_wait_on_atomic_t+0x170/0x170 > >[ 143.205103] ? wake_up_q+0xa0/0xa0 > >[ 143.205159] ? flush_workqueue_prep_pwqs+0x15a/0x2c0 > >[ 143.205237] wait_for_completion+0x1d/0x20 > >[ 143.205292] flush_workqueue+0x2e9/0xbb0 > >[ 143.205339] ? flush_workqueue+0x163/0xbb0 > >[ 143.205418] ? __schedule+0x533/0x1180 > >[ 143.205498] ? check_flush_dependency+0x1a0/0x1a0 > >[ 143.205681] i915_gem_userptr_mn_invalidate_range_start+0x1c7/0x270 [i915] > >[ 143.205865] ? i915_gem_userptr_dmabuf_export+0x40/0x40 [i915] > >[ 143.205955] __mmu_notifier_invalidate_range_start+0xc6/0x120 > >[ 143.206044] ? __mmu_notifier_invalidate_range_start+0x51/0x120 > >[ 143.206123] zap_page_range_single+0x1c7/0x1f0 > >[ 143.206171] ? unmap_single_vma+0x160/0x160 > >[ 143.206260] ? unmap_mapping_range+0xa9/0x1b0 > >[ 143.206308] ? vma_interval_tree_subtree_search+0x75/0xd0 > >[ 143.206397] unmap_mapping_range+0x18f/0x1b0 > >[ 143.206444] ? zap_vma_ptes+0x70/0x70 > >[ 143.206524] ? __pm_runtime_resume+0x67/0xa0 > >[ 143.206723] i915_gem_release_mmap+0x1ba/0x1c0 [i915] > >[ 143.206846] i915_vma_unbind+0x5c2/0x690 [i915] > >[ 143.206925] ? __lock_is_held+0x52/0x100 > >[ 143.207076] i915_gem_object_set_tiling+0x1db/0x650 [i915] > >[ 143.207236] i915_gem_set_tiling_ioctl+0x1d3/0x3b0 [i915] > >[ 143.207377] ? i915_gem_set_tiling_ioctl+0x5/0x3b0 [i915] > >[ 143.207457] drm_ioctl+0x36c/0x670 > >[ 143.207535] ? debug_lockdep_rcu_enabled.part.0+0x1a/0x30 > >[ 143.207730] ? i915_gem_object_set_tiling+0x650/0x650 [i915] > >[ 143.207793] ? drm_getunique+0x120/0x120 > >[ 143.207875] ? __handle_mm_fault+0x996/0x14a0 > >[ 143.207939] ? vm_insert_page+0x340/0x340 > >[ 143.208028] ? up_write+0x28/0x50 > >[ 143.208086] ? vm_mmap_pgoff+0x160/0x190 > >[ 143.208163] do_vfs_ioctl+0x12c/0xa60 > >[ 143.208218] ? debug_lockdep_rcu_enabled+0x35/0x40 > >[ 143.208267] ? ioctl_preallocate+0x150/0x150 > >[ 143.208353] ? __do_page_fault+0x36a/0x6e0 > >[ 143.208400] ? mark_held_locks+0x23/0xc0 > >[ 143.208479] ? up_read+0x1f/0x40 > >[ 143.208526] ? entry_SYSCALL_64_fastpath+0x5/0xc6 > >[ 143.208669] ? __fget_light+0xa7/0xc0 > >[ 143.208747] SyS_ioctl+0x41/0x70 > >[ 143.208808] entry_SYSCALL_64_fastpath+0x23/0xc6 > > That would mean another process never exits cancel_userptr, correct? > Do we have a trace of the other end? Our worker, who is waiting on struct_mutex (in order to unbind) which we are holding ourselves as we are in the middle of an unbind. The nasty part here is that we can recurse into unbind on the same object (let alone the mutex deadlock). That's impossible if we prevent the object from activating itself on a GTT mmap. ARGH. The deadlock is between a non-userptr and a set of userptr (that explains how we get to the unbind). It is just that the other userptr are still in the process of running their workers when the time comes to cancel the work. (Avoids the dilemma of how we ended up with bound userptr of the GTT). Simplest patch is then fun with obj->userptr.work, i.e. something like if (xchg(>userptr.work, NULL)) return 0; -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/guc: Use formalized struct definition for ads object
Manual pointer manipulation is error prone. Let compiler calculate right offsets for us in case we need to change ads layout. v2: don't call it object (Chris) Signed-off-by: Michal WajdeczkoCc: Oscar Mateo Cc: Joonas Lahtinen Cc: Daniele Ceraolo Spurio Cc: Chris Wilson --- drivers/gpu/drm/i915/i915_guc_submission.c | 50 ++ 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index beb38e3..c95616e 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -810,22 +810,21 @@ static void guc_addon_create(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); struct i915_vma *vma; - struct guc_ads *ads; - struct guc_policies *policies; - struct guc_mmio_reg_state *reg_state; - struct intel_engine_cs *engine; - enum intel_engine_id id; struct page *page; - u32 size; - /* The ads obj includes the struct itself and buffers passed to GuC */ - size = sizeof(struct guc_ads) + sizeof(struct guc_policies) + - sizeof(struct guc_mmio_reg_state) + - GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE; + struct __guc_ads_blob { + struct guc_ads ads; + struct guc_policies policies; + struct guc_mmio_reg_state reg_state; + u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE]; + } __packed *blob; + u32 offset; + struct intel_engine_cs *engine; + enum intel_engine_id id; vma = guc->ads_vma; if (!vma) { - vma = intel_guc_allocate_vma(guc, PAGE_ALIGN(size)); + vma = intel_guc_allocate_vma(guc, PAGE_ALIGN(sizeof(*blob))); if (IS_ERR(vma)) return; @@ -833,7 +832,8 @@ static void guc_addon_create(struct intel_guc *guc) } page = i915_vma_first_page(vma); - ads = kmap(page); + blob = kmap(page); + offset = i915_ggtt_offset(vma); /* * The GuC requires a "Golden Context" when it reinitialises @@ -843,34 +843,32 @@ static void guc_addon_create(struct intel_guc *guc) * to find it. */ engine = dev_priv->engine[RCS]; - ads->golden_context_lrca = engine->status_page.ggtt_offset; + blob->ads.golden_context_lrca = engine->status_page.ggtt_offset; for_each_engine(engine, dev_priv, id) - ads->eng_state_size[engine->guc_id] = intel_lr_context_size(engine); + blob->ads.eng_state_size[engine->guc_id] = + intel_lr_context_size(engine); /* GuC scheduling policies */ - policies = (void *)ads + sizeof(struct guc_ads); - guc_policies_init(policies); + guc_policies_init(>policies); - ads->scheduler_policies = - guc_ggtt_offset(vma) + sizeof(struct guc_ads); + blob->ads.scheduler_policies = offset + + offsetof(struct __guc_ads_blob, policies); /* MMIO reg state */ - reg_state = (void *)policies + sizeof(struct guc_policies); - for_each_engine(engine, dev_priv, id) { - reg_state->mmio_white_list[engine->guc_id].mmio_start = + blob->reg_state.mmio_white_list[engine->guc_id].mmio_start = engine->mmio_base + GUC_MMIO_WHITE_LIST_START; /* Nothing to be saved or restored for now. */ - reg_state->mmio_white_list[engine->guc_id].count = 0; + blob->reg_state.mmio_white_list[engine->guc_id].count = 0; } - ads->reg_state_addr = ads->scheduler_policies + - sizeof(struct guc_policies); + blob->ads.reg_state_addr = offset + + offsetof(struct __guc_ads_blob, reg_state); - ads->reg_state_buffer = ads->reg_state_addr + - sizeof(struct guc_mmio_reg_state); + blob->ads.reg_state_buffer = offset + + offsetof(struct __guc_ads_blob, reg_state_buffer); kunmap(page); } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/userptr: Disallow wrapping GTT into a userptr
On 07/03/2017 09:13, Chris Wilson wrote: On Tue, Mar 07, 2017 at 08:42:26AM +, Tvrtko Ursulin wrote: On 06/03/2017 15:36, Chris Wilson wrote: If we allow the user to convert a GTT mmap address into a userptr, we may end up in recursion hell, where currently we hit a mutex deadlock but other possibilities include use-after-free during the unbind/cancel_userptr. I thought we already disallowed that and indeed igt/gem_userptr_blits/invalid-gtt-mapping succeeds here - what am I missing? Michał presented this deadlock: [ 143.203989] gem_userptr_bli D0 902898 0x [ 143.204054] Call Trace: [ 143.204137] __schedule+0x511/0x1180 [ 143.204195] ? pci_mmcfg_check_reserved+0xc0/0xc0 [ 143.204274] schedule+0x57/0xe0 [ 143.204327] schedule_timeout+0x383/0x670 [ 143.204374] ? trace_hardirqs_on_caller+0x187/0x280 [ 143.204457] ? trace_hardirqs_on_thunk+0x1a/0x1c [ 143.204507] ? usleep_range+0x110/0x110 [ 143.204657] ? irq_exit+0x89/0x100 [ 143.204710] ? retint_kernel+0x2d/0x2d [ 143.204794] ? trace_hardirqs_on_caller+0x187/0x280 [ 143.204857] ? _raw_spin_unlock_irq+0x33/0x60 [ 143.204944] wait_for_common+0x1f0/0x2f0 [ 143.205006] ? out_of_line_wait_on_atomic_t+0x170/0x170 [ 143.205103] ? wake_up_q+0xa0/0xa0 [ 143.205159] ? flush_workqueue_prep_pwqs+0x15a/0x2c0 [ 143.205237] wait_for_completion+0x1d/0x20 [ 143.205292] flush_workqueue+0x2e9/0xbb0 [ 143.205339] ? flush_workqueue+0x163/0xbb0 [ 143.205418] ? __schedule+0x533/0x1180 [ 143.205498] ? check_flush_dependency+0x1a0/0x1a0 [ 143.205681] i915_gem_userptr_mn_invalidate_range_start+0x1c7/0x270 [i915] [ 143.205865] ? i915_gem_userptr_dmabuf_export+0x40/0x40 [i915] [ 143.205955] __mmu_notifier_invalidate_range_start+0xc6/0x120 [ 143.206044] ? __mmu_notifier_invalidate_range_start+0x51/0x120 [ 143.206123] zap_page_range_single+0x1c7/0x1f0 [ 143.206171] ? unmap_single_vma+0x160/0x160 [ 143.206260] ? unmap_mapping_range+0xa9/0x1b0 [ 143.206308] ? vma_interval_tree_subtree_search+0x75/0xd0 [ 143.206397] unmap_mapping_range+0x18f/0x1b0 [ 143.206444] ? zap_vma_ptes+0x70/0x70 [ 143.206524] ? __pm_runtime_resume+0x67/0xa0 [ 143.206723] i915_gem_release_mmap+0x1ba/0x1c0 [i915] [ 143.206846] i915_vma_unbind+0x5c2/0x690 [i915] [ 143.206925] ? __lock_is_held+0x52/0x100 [ 143.207076] i915_gem_object_set_tiling+0x1db/0x650 [i915] [ 143.207236] i915_gem_set_tiling_ioctl+0x1d3/0x3b0 [i915] [ 143.207377] ? i915_gem_set_tiling_ioctl+0x5/0x3b0 [i915] [ 143.207457] drm_ioctl+0x36c/0x670 [ 143.207535] ? debug_lockdep_rcu_enabled.part.0+0x1a/0x30 [ 143.207730] ? i915_gem_object_set_tiling+0x650/0x650 [i915] [ 143.207793] ? drm_getunique+0x120/0x120 [ 143.207875] ? __handle_mm_fault+0x996/0x14a0 [ 143.207939] ? vm_insert_page+0x340/0x340 [ 143.208028] ? up_write+0x28/0x50 [ 143.208086] ? vm_mmap_pgoff+0x160/0x190 [ 143.208163] do_vfs_ioctl+0x12c/0xa60 [ 143.208218] ? debug_lockdep_rcu_enabled+0x35/0x40 [ 143.208267] ? ioctl_preallocate+0x150/0x150 [ 143.208353] ? __do_page_fault+0x36a/0x6e0 [ 143.208400] ? mark_held_locks+0x23/0xc0 [ 143.208479] ? up_read+0x1f/0x40 [ 143.208526] ? entry_SYSCALL_64_fastpath+0x5/0xc6 [ 143.208669] ? __fget_light+0xa7/0xc0 [ 143.208747] SyS_ioctl+0x41/0x70 [ 143.208808] entry_SYSCALL_64_fastpath+0x23/0xc6 That would mean another process never exits cancel_userptr, correct? Do we have a trace of the other end? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2] lib: Define a common bit operations library
On Thu, Mar 02, 2017 at 11:46:11AM -0800, Michel Thierry wrote: > > On 01/03/17 23:45, Daniel Vetter wrote: > > On Wed, Mar 01, 2017 at 04:14:31PM +, Chris Wilson wrote: > > > On Wed, Mar 01, 2017 at 07:52:26AM -0800, Michel Thierry wrote: > > > > Bring the test/set/clear bit functions we have into a single place. > > > > > > > > v2: Add gtk-doc comment blocks (Daniel) > > > > > > Hiss, boo! Will someone take gtk-doc and bury it? -flto to save the day? > > > > ... I thought it's perfectly ok with inline functions. Is it not? > > Yes, I was supposed to just remove _static_. Yeah, static it won't document because that's not a library interface. -Daniel -- 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
Re: [Intel-gfx] [PATCH v1] drm/i915/bxt: use NULL for GPIO connection ID
On Tue, Mar 07, 2017 at 12:48:26PM +0200, Andy Shevchenko wrote: > On Sun, 2017-02-26 at 22:45 +0100, Daniel Vetter wrote: > > On Tue, Feb 21, 2017 at 06:52:24PM +0200, Andy Shevchenko wrote: > > > On Tue, 2017-02-21 at 18:26 +0200, Jani Nikula wrote: > > > > On Tue, 21 Feb 2017, Andy Shevchenko> > > l.co > > > > m> wrote: > > > > > The commit 213e08ad60ba ("drm/i915/bxt: add bxt dsi gpio element > > > > > support") enables GPIO support for Broxton based platforms. > > > > > > > > > > While using that API we might get into troubles in the future, > > > > > because > > > > > we can't rely on label "panel" in the driver since vendor > > > > > firmware > > > > > might > > > > > provide any GPIO pin there, e.g. "reset", and even mark it in > > > > > _DSD > > > > > (in > > > > > which case the request will fail). > > > > > > > > > > To avoid inconsistency and potential issues we have two options: > > > > > a) generate GPIO ACPI mapping table and supply it via > > > > > acpi_dev_add_driver_gpios(), or > > > > > b) just pass NULL as connection ID. > > > > > > > > > > The b) approach is much simplier and would work since the driver > > > > > relies > > > > > on GPIO indeces only. Moreover, the _CRS fallback mechanism, > > > > > when > > > > > requesting GPIO, is going to be stricter, and supplying non-NULL > > > > > connection ID when neither _DSD, nor GPIO ACPI mapping is > > > > > present, > > > > > will > > > > > make request fail. > > > > > > > > The patch version log in the commit suggests otherwise; we'd tried > > > > and > > > > failed with NULL, > > > > > > Can I see DSDT excerpts of the platform that fails? > > > > > > > until Mika realized passing "panel" works: > > > > > > > > v2 by Mika: switch *NULL* to *"panel"* when requesting gpio > > > > for > > > > MIPI/DSI > > > > panel. > > > > > > > > See also [1]. What has changed since then that should make this > > > > work > > > > now? We shouldn't apply until we get Tested-by's. > > > > > > Not changed yet, but *going to be*. See my repository here [2]. > > > To fix the mess with GPIO ACPI stuff we are going to make request > > > stricter as I pointed in commit message above, i.e. asking for a > > > GPIO by > > > connection ID without _DSD present will guarantee -ENOENT since it > > > will > > > be no fallback to _CRS. You may follow discussion in our internal > > > mailing list for drivers. > > > > Why exactly is this being discussed on an internal mailing list? > > Upstream > > happens in public ... > > It was a prelininary discussion and it's sad you didn't notice it. The problem isn't that I didn't notice (I don't think I can provide anything of value here), but that technical discussion should happen in the open, on public mailing lists, because otherwise we just have a big coordination chaos. GFX is huge, and just the automatic public archiving mailing lists provides is super important to get people up to speed when suddenly you realize you need them. -Daniel -- 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
Re: [Intel-gfx] [PATCH v1] drm/i915/bxt: use NULL for GPIO connection ID
On Sun, 2017-02-26 at 22:45 +0100, Daniel Vetter wrote: > On Tue, Feb 21, 2017 at 06:52:24PM +0200, Andy Shevchenko wrote: > > On Tue, 2017-02-21 at 18:26 +0200, Jani Nikula wrote: > > > On Tue, 21 Feb 2017, Andy Shevchenko> > l.co > > > m> wrote: > > > > The commit 213e08ad60ba ("drm/i915/bxt: add bxt dsi gpio element > > > > support") enables GPIO support for Broxton based platforms. > > > > > > > > While using that API we might get into troubles in the future, > > > > because > > > > we can't rely on label "panel" in the driver since vendor > > > > firmware > > > > might > > > > provide any GPIO pin there, e.g. "reset", and even mark it in > > > > _DSD > > > > (in > > > > which case the request will fail). > > > > > > > > To avoid inconsistency and potential issues we have two options: > > > > a) generate GPIO ACPI mapping table and supply it via > > > > acpi_dev_add_driver_gpios(), or > > > > b) just pass NULL as connection ID. > > > > > > > > The b) approach is much simplier and would work since the driver > > > > relies > > > > on GPIO indeces only. Moreover, the _CRS fallback mechanism, > > > > when > > > > requesting GPIO, is going to be stricter, and supplying non-NULL > > > > connection ID when neither _DSD, nor GPIO ACPI mapping is > > > > present, > > > > will > > > > make request fail. > > > > > > The patch version log in the commit suggests otherwise; we'd tried > > > and > > > failed with NULL, > > > > Can I see DSDT excerpts of the platform that fails? > > > > > until Mika realized passing "panel" works: > > > > > > v2 by Mika: switch *NULL* to *"panel"* when requesting gpio > > > for > > > MIPI/DSI > > > panel. > > > > > > See also [1]. What has changed since then that should make this > > > work > > > now? We shouldn't apply until we get Tested-by's. > > > > Not changed yet, but *going to be*. See my repository here [2]. > > To fix the mess with GPIO ACPI stuff we are going to make request > > stricter as I pointed in commit message above, i.e. asking for a > > GPIO by > > connection ID without _DSD present will guarantee -ENOENT since it > > will > > be no fallback to _CRS. You may follow discussion in our internal > > mailing list for drivers. > > Why exactly is this being discussed on an internal mailing list? > Upstream > happens in public ... It was a prelininary discussion and it's sad you didn't notice it. Nevertheless, Hans started it in public mailing list here [1]. I would include you in Cc list for my further replies there. Mika K., my branch is here [2] with above patch included. Can you, please, test your use case? Because it sounds too strange to me that connection ID in there affects somehow the PM flow. It very likely unveils a bug somewhere else. [1] https://www.spinics.net/lists/linux-input/msg49127.html [2] http://bitbucket.org/andy-shev/linux/branch/topic%2Fuart%2frpm -- Andy Shevchenko Intel Finland Oy ___ 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/guc: Use formalized struct definition for ads object
== Series Details == Series: drm/i915/guc: Use formalized struct definition for ads object URL : https://patchwork.freedesktop.org/series/20826/ State : warning == Summary == Series 20826v1 drm/i915/guc: Use formalized struct definition for ads object https://patchwork.freedesktop.org/api/1.0/series/20826/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: fail -> PASS (fi-snb-2600) fdo#17 Test gem_exec_suspend: Subgroup basic-s4-devices: pass -> DMESG-WARN (fi-bxt-t5700) fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 462s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 610s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 544s fi-bxt-t5700 total:278 pass:257 dwarn:1 dfail:0 fail:0 skip:20 time: 620s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 503s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 509s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 444s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 435s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 442s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 505s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 491s fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 477s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 512s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 600s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 507s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 557s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 550s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 423s bf077370616c6c41acd0ab6d46158569244941a8 drm-tip: 2017y-03m-07d-07h-57m-10s UTC integration manifest 2b0d679 drm/i915/guc: Use formalized struct definition for ads object == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4081/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Use formalized struct definition for ads object
On Tue, Mar 07, 2017 at 10:39:46AM +, Chris Wilson wrote: > On Tue, Mar 07, 2017 at 10:29:35AM +, Michal Wajdeczko wrote: > > Manual pointer manipulation is error prone. Let compiler calculate > > right offsets for us in case we need to change ads layout. > > > > Signed-off-by: Michal Wajdeczko> > Cc: Oscar Mateo > > Cc: Joonas Lahtinen > > Cc: Daniele Ceraolo Spurio > > Cc: Chris Wilson > > --- > > drivers/gpu/drm/i915/i915_guc_submission.c | 49 > > ++ > > 1 file changed, 23 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c > > b/drivers/gpu/drm/i915/i915_guc_submission.c > > index beb38e3..f87649b 100644 > > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > > @@ -810,22 +810,21 @@ static void guc_addon_create(struct intel_guc *guc) > > { > > struct drm_i915_private *dev_priv = guc_to_i915(guc); > > struct i915_vma *vma; > > - struct guc_ads *ads; > > - struct guc_policies *policies; > > - struct guc_mmio_reg_state *reg_state; > > - struct intel_engine_cs *engine; > > - enum intel_engine_id id; > > struct page *page; > > - u32 size; > > - > > /* The ads obj includes the struct itself and buffers passed to GuC */ > > - size = sizeof(struct guc_ads) + sizeof(struct guc_policies) + > > - sizeof(struct guc_mmio_reg_state) + > > - GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE; > > + struct __guc_ads_object { > > + struct guc_ads ads; > > + struct guc_policies policies; > > + struct guc_mmio_reg_state reg_state; > > + u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE]; > > + } __packed *obj; > > A humble request not to call this obj. I was confused later and worrying > about what you were adding to drm_i915_gem_object. Agree, but I just followed the name used in existing comment. What about: struct __guc_ads_blob { ... } __packed *blob; -Michal ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Use formalized struct definition for ads object
On Tue, Mar 07, 2017 at 10:29:35AM +, Michal Wajdeczko wrote: > Manual pointer manipulation is error prone. Let compiler calculate > right offsets for us in case we need to change ads layout. > > Signed-off-by: Michal Wajdeczko> Cc: Oscar Mateo > Cc: Joonas Lahtinen > Cc: Daniele Ceraolo Spurio > Cc: Chris Wilson > --- > drivers/gpu/drm/i915/i915_guc_submission.c | 49 > ++ > 1 file changed, 23 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c > b/drivers/gpu/drm/i915/i915_guc_submission.c > index beb38e3..f87649b 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -810,22 +810,21 @@ static void guc_addon_create(struct intel_guc *guc) > { > struct drm_i915_private *dev_priv = guc_to_i915(guc); > struct i915_vma *vma; > - struct guc_ads *ads; > - struct guc_policies *policies; > - struct guc_mmio_reg_state *reg_state; > - struct intel_engine_cs *engine; > - enum intel_engine_id id; > struct page *page; > - u32 size; > - > /* The ads obj includes the struct itself and buffers passed to GuC */ > - size = sizeof(struct guc_ads) + sizeof(struct guc_policies) + > - sizeof(struct guc_mmio_reg_state) + > - GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE; > + struct __guc_ads_object { > + struct guc_ads ads; > + struct guc_policies policies; > + struct guc_mmio_reg_state reg_state; > + u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE]; > + } __packed *obj; A humble request not to call this obj. I was confused later and worrying about what you were adding to drm_i915_gem_object. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/guc: Use formalized struct definition for ads object
Manual pointer manipulation is error prone. Let compiler calculate right offsets for us in case we need to change ads layout. Signed-off-by: Michal WajdeczkoCc: Oscar Mateo Cc: Joonas Lahtinen Cc: Daniele Ceraolo Spurio Cc: Chris Wilson --- drivers/gpu/drm/i915/i915_guc_submission.c | 49 ++ 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index beb38e3..f87649b 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -810,22 +810,21 @@ static void guc_addon_create(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); struct i915_vma *vma; - struct guc_ads *ads; - struct guc_policies *policies; - struct guc_mmio_reg_state *reg_state; - struct intel_engine_cs *engine; - enum intel_engine_id id; struct page *page; - u32 size; - /* The ads obj includes the struct itself and buffers passed to GuC */ - size = sizeof(struct guc_ads) + sizeof(struct guc_policies) + - sizeof(struct guc_mmio_reg_state) + - GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE; + struct __guc_ads_object { + struct guc_ads ads; + struct guc_policies policies; + struct guc_mmio_reg_state reg_state; + u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE]; + } __packed *obj; + u32 offset; + struct intel_engine_cs *engine; + enum intel_engine_id id; vma = guc->ads_vma; if (!vma) { - vma = intel_guc_allocate_vma(guc, PAGE_ALIGN(size)); + vma = intel_guc_allocate_vma(guc, PAGE_ALIGN(sizeof(*obj))); if (IS_ERR(vma)) return; @@ -833,7 +832,8 @@ static void guc_addon_create(struct intel_guc *guc) } page = i915_vma_first_page(vma); - ads = kmap(page); + obj = kmap(page); + offset = i915_ggtt_offset(vma); /* * The GuC requires a "Golden Context" when it reinitialises @@ -843,34 +843,31 @@ static void guc_addon_create(struct intel_guc *guc) * to find it. */ engine = dev_priv->engine[RCS]; - ads->golden_context_lrca = engine->status_page.ggtt_offset; + obj->ads.golden_context_lrca = engine->status_page.ggtt_offset; for_each_engine(engine, dev_priv, id) - ads->eng_state_size[engine->guc_id] = intel_lr_context_size(engine); + obj->ads.eng_state_size[engine->guc_id] = intel_lr_context_size(engine); /* GuC scheduling policies */ - policies = (void *)ads + sizeof(struct guc_ads); - guc_policies_init(policies); + guc_policies_init(>policies); - ads->scheduler_policies = - guc_ggtt_offset(vma) + sizeof(struct guc_ads); + obj->ads.scheduler_policies = offset + + offsetof(struct __guc_ads_object, policies); /* MMIO reg state */ - reg_state = (void *)policies + sizeof(struct guc_policies); - for_each_engine(engine, dev_priv, id) { - reg_state->mmio_white_list[engine->guc_id].mmio_start = + obj->reg_state.mmio_white_list[engine->guc_id].mmio_start = engine->mmio_base + GUC_MMIO_WHITE_LIST_START; /* Nothing to be saved or restored for now. */ - reg_state->mmio_white_list[engine->guc_id].count = 0; + obj->reg_state.mmio_white_list[engine->guc_id].count = 0; } - ads->reg_state_addr = ads->scheduler_policies + - sizeof(struct guc_policies); + obj->ads.reg_state_addr = offset + + offsetof(struct __guc_ads_object, reg_state); - ads->reg_state_buffer = ads->reg_state_addr + - sizeof(struct guc_mmio_reg_state); + obj->ads.reg_state_buffer = offset + + offsetof(struct __guc_ads_object, reg_state_buffer); kunmap(page); } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Use pagecache write to prepopulate shmemfs from pwrite-ioctl
On Tue, Mar 07, 2017 at 11:55:20AM +0200, Petri Latvala wrote: > > For the record, manually launched extended test run on SKL > resulted in: > > commit f88cf1067cc499f4c9236c4e3ff7e410f9cc76d7 > Author: Chris Wilson> Date: Sat Mar 4 10:35:32 2017 + > > drm/i915: Use pagecache write to prepopulate shmemfs from pwrite-ioctl > > > igt@gem_madvise@dontneed-before-exec: pass -> {'fail', 'incomplete'} That test is expected to fail, that it ever passed is a fluke. incomplete there should be a failure in the runner? That should quite happily spot the purged object in execbuf. > igt@gem_madvise@dontneed-before-pwrite: pass -> fail Hmm. Here we are relying on shmemfs reporting the write to the truncated object as invalid. Looks like we need to do the filtering ourselves. It is not a huge problem as we still prevent any later instantiation of pages so just a temporary slip. Setting obj->mm.pages = -EFAULT; is likely a fun solution. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] lib/scatterlist: Avoid potential scatterlist entry overflow
Hi David, Chris noticed your "scatterlist: don't overflow length field" patch and pinged me, so I am copying you on another thread which tries to solve the same problem. My latest series is here: https://patchwork.freedesktop.org/series/18062/, but it has been going from some time November last year. I like your BUILD_BUG_ON safety, but otherwise our patches are pretty similar. i915 driver also benefits from the ability to create large sg chunks which saves us a few megabytes of RAM at runtime, but we do have to degrade to smaller chunks when running under a hypervisor. For that we are using the swiotlb_max_segment API Konrad recently added for this purpose. So what I did in addition to fixing the overflow is exported a new flavour of sg_alloc_table_from_pages which allows you to control the maximum chunk. Maybe you can have a look at my series and see if it would work for you? I've been trying to gain some traction for it for some months now. Regards, Tvrtko On 07/03/2017 08:58, Tvrtko Ursulin wrote: Hi, On 16/01/2017 14:12, Tvrtko Ursulin wrote: From: Tvrtko UrsulinSince the scatterlist length field is an unsigned int, make sure that sg_alloc_table_from_pages does not overflow it while coallescing pages to a single entry. v2: Drop reference to future use. Use UINT_MAX. v3: max_segment must be page aligned. v4: Do not rely on compiler to optimise out the rounddown. (Joonas Lahtinen) v5: Simplified loops and use post-increments rather than pre-increments. Use PAGE_MASK and fix comment typo. (Andy Shevchenko) Signed-off-by: Tvrtko Ursulin Cc: Masahiro Yamada Cc: linux-ker...@vger.kernel.org Reviewed-by: Chris Wilson (v2) Cc: Joonas Lahtinen Cc: Andy Shevchenko Anyone in the mood for reviewing from here to the end of the series? Regards, Tvrtko --- include/linux/scatterlist.h | 6 ++ lib/scatterlist.c | 31 --- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index c981bee1a3ae..4768eeeb7054 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -21,6 +21,12 @@ struct scatterlist { }; /* + * Since the above length field is an unsigned int, below we define the maximum + * length in bytes that can be stored in one scatterlist entry. + */ +#define SCATTERLIST_MAX_SEGMENT (UINT_MAX & PAGE_MASK) + +/* * These macros should be used after a dma_map_sg call has been done * to get bus addresses of each of the SG entries and their lengths. * You should only work with the number of sg entries dma_map_sg diff --git a/lib/scatterlist.c b/lib/scatterlist.c index e05e7fc98892..65f375645df5 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -394,17 +394,22 @@ int sg_alloc_table_from_pages(struct sg_table *sgt, unsigned int offset, unsigned long size, gfp_t gfp_mask) { -unsigned int chunks; -unsigned int i; -unsigned int cur_page; +const unsigned int max_segment = SCATTERLIST_MAX_SEGMENT; +unsigned int chunks, cur_page, seg_len, i; int ret; struct scatterlist *s; /* compute number of contiguous chunks */ chunks = 1; -for (i = 1; i < n_pages; ++i) -if (page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) + 1) -++chunks; +seg_len = 0; +for (i = 1; i < n_pages; i++) { +seg_len += PAGE_SIZE; +if (seg_len >= max_segment || +page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) + 1) { +chunks++; +seg_len = 0; +} +} ret = sg_alloc_table(sgt, chunks, gfp_mask); if (unlikely(ret)) @@ -413,17 +418,21 @@ int sg_alloc_table_from_pages(struct sg_table *sgt, /* merging chunks and putting them into the scatterlist */ cur_page = 0; for_each_sg(sgt->sgl, s, sgt->orig_nents, i) { -unsigned long chunk_size; -unsigned int j; +unsigned int j, chunk_size; /* look for the end of the current chunk */ -for (j = cur_page + 1; j < n_pages; ++j) -if (page_to_pfn(pages[j]) != +seg_len = 0; +for (j = cur_page + 1; j < n_pages; j++) { +seg_len += PAGE_SIZE; +if (seg_len >= max_segment || +page_to_pfn(pages[j]) != page_to_pfn(pages[j - 1]) + 1) break; +} chunk_size = ((j - cur_page) << PAGE_SHIFT) - offset; -sg_set_page(s, pages[cur_page], min(size, chunk_size), offset); +sg_set_page(s, pages[cur_page], +min_t(unsigned long, size, chunk_size), offset); size -= chunk_size; offset = 0; cur_page = j; ___ Intel-gfx mailing list
Re: [Intel-gfx] [PATCH v7 5/6] drm/i915: enable scrambling
On Fri, 2017-03-03 at 21:58 +0530, Shashank Sharma wrote: > Geminilake platform sports a native HDMI 2.0 controller, and is > capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec > mendates scrambling for these higher clocks, for reduced RF footprint. > > This patch checks if the monitor supports scrambling, and if required, > enables it during the modeset. > > V2: Addressed review comments from Ville: > - Do not track scrambling status in DRM layer, track somewhere in > driver like in intel_crtc_state. > - Don't talk to monitor at such a low layer, set monitor scrambling > in intel_enable_ddi() before enabling the port. > > V3: Addressed review comments from Jani > - In comments, function names, use "sink" instead of "monitor", >so that the implementation could be close to the language of >HDMI spec. > > V4: Addressed review comment from Maarten > - scrambling -> hdmi_scrambling >high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio > > V5: Addressed review comments from Ville and Ander > - Do not modifiy the crtc_state after compute_config. Move all >scrambling and tmds_clock_ratio calcutations to compute_config. > - While setting scrambling for source/sink, do not check the >conditions again, just go by the crtc_state flags. This will >simplyfy the condition checks. > > V6: Addressed review comments from Ville > - Do not add IS_GLK check in disable/enable function, instead add it >in compute_config, while setting state flags. > - Remove unnecessary paranthesis. > - Simplyfy handle_sink_scrambling function as suggested. > - Add readout code for scrambling status in get_ddi_config and add a >check for the same in pipe_config_compare. > > V7: Addressed review comments from Ander/Ville > - No separate function for source scrambling, make it inline > - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK > - Do not add platform check while setting source scrambling > - Use pipe_config instead of crtc->config to set sink scrambling > - To readout scrambling status, Compare with SCRAMBLING_MASK >not any of its bits > - Remove platform check in intel_pipe_config_compare while checking >scrambling status > > Signed-off-by: Shashank Sharma> --- > drivers/gpu/drm/i915/i915_reg.h | 7 + > drivers/gpu/drm/i915/intel_ddi.c | 33 +++ > drivers/gpu/drm/i915/intel_display.c | 3 +++ > drivers/gpu/drm/i915/intel_drv.h | 10 +++ > drivers/gpu/drm/i915/intel_hdmi.c| 52 > > 5 files changed, 105 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 4906ce4d..bfd988b 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7824,7 +7824,14 @@ enum { > #define TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12) > #define TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12) > #define TRANS_DDI_DP_VC_PAYLOAD_ALLOC (1<<8) > +#define TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7) > +#define TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6) > #define TRANS_DDI_BFI_ENABLE(1<<4) > +#define TRANS_DDI_HIGH_TMDS_CHAR_RATE (1<<4) > +#define TRANS_DDI_HDMI_SCRAMBLING (1<<0) > +#define TRANS_DDI_HDMI_SCRAMBLING_MASK (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE > \ > + | TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \ > + | TRANS_DDI_HDMI_SCRAMBLING) > > /* DisplayPort Transport Control */ > #define _DP_TP_CTL_A 0x64040 > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index a7c08d7..2159b2b 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1311,6 +1311,11 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc > *crtc) > temp |= TRANS_DDI_MODE_SELECT_HDMI; > else > temp |= TRANS_DDI_MODE_SELECT_DVI; > + > + if (intel_crtc->config->hdmi_scrambling) > + temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK; > + if (intel_crtc->config->hdmi_high_tmds_clock_ratio) > + temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE; > } else if (type == INTEL_OUTPUT_ANALOG) { > temp |= TRANS_DDI_MODE_SELECT_FDI; > temp |= (intel_crtc->config->fdi_lanes - 1) << 1; > @@ -1885,6 +1890,20 @@ static void intel_enable_ddi(struct intel_encoder > *intel_encoder, > struct intel_digital_port *intel_dig_port = > enc_to_dig_port(encoder); > > + if (IS_GEMINILAKE(dev_priv)) { > + /* > + * GLK sports a native HDMI 2.0 controller. If required > + * clock rate is > 340 Mhz && scrambling is supported > + * by sink, enable scrambling before enabling the > + *
Re: [Intel-gfx] [PATCH 03/15] drm/i915/selftests: exercise cache domain eviction
On Mon, Mar 06, 2017 at 11:54:02PM +, Matthew Auld wrote: Add a selftest to exercise evicting neighbouring nodes that conflict due to page colouring in the GTT. > v2: add a peppering of comments > > Signed-off-by: Matthew Auld> Cc: Chris Wilson Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/15] drm/i915: use correct node for handling cache domain eviction
On Mon, Mar 06, 2017 at 11:54:01PM +, Matthew Auld wrote: > It looks like we were incorrectly comparing vma->node against itself > instead of the target node, when evicting for a node on systems where we > need guard pages between regions with different cache domains. As a > consequence we can end up trying to needlessly evict neighbouring nodes, > even if they have the same cache domain, and if they were pinned we > would fail the eviction. > > Fixes: 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a > helper") > Signed-off-by: Matthew Auld> Cc: Chris Wilson > Cc: Joonas Lahtinen > Reviewed-by: Chris Wilson Lost my /o\ :) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 00/15] drm/i915: initial support for huge gtt pages
On Mon, Mar 06, 2017 at 11:53:59PM +, Matthew Auld wrote: > This series adds support for huge-pages for the gtt, where "huge" > is 64K, 2M and 1G. This isn't everything I have and there are still some > things which I have yet to implement, like handling evict-for-node with the > 64K/4K trickiness, but the hope here is to get some early feedback if > possible. > > One open question I still have is how the page-size should be handled at the > gem object level, should the page-size be an implementation detail of > whichever > backend the gem object uses, where the selected page-size would solely depend > on the size of the object and the availability of huge pages, or do we intend > to expose some kind of hinting, both within our driver and possibly to > userspace? I thought the intention was to use optimistic superpages in the GTT (and so optimistic large allocations). One question I have is how do you mix swiotlb limits? We currently have a trial-and-error approach to whittle down the sg chunks, which prevents having a predetermined pagesize (at least as far as the GTT is concerned). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Use pagecache write to prepopulate shmemfs from pwrite-ioctl
For the record, manually launched extended test run on SKL resulted in: commit f88cf1067cc499f4c9236c4e3ff7e410f9cc76d7 Author: Chris WilsonDate: Sat Mar 4 10:35:32 2017 + drm/i915: Use pagecache write to prepopulate shmemfs from pwrite-ioctl igt@gem_madvise@dontneed-before-exec: pass -> {'fail', 'incomplete'} igt@gem_madvise@dontneed-before-pwrite: pass -> fail -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx