Re: [Intel-gfx] [PATCH 01/10] include: Move ascii85 functions from i915 to linux/ascii85.h
Hi Jordan, Thank you for the patch! Yet something to improve: [auto build test ERROR on robclark/msm-next] [also build test ERROR on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Jordan-Crouse/drm-msm-GPU-crash-state/20180406-170513 base: git://people.freedesktop.org/~robclark/linux msm-next config: x86_64-rhel (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/gpu//drm/i915/i915_gpu_error.c: In function 'print_error_obj': >> drivers/gpu//drm/i915/i915_gpu_error.c:549:4: error: implicit declaration of >> function 'error_puts'; did you mean 'err_puts'? >> [-Werror=implicit-function-declaration] error_puts(m, ascii85_encode(obj->pages[page][i], out)); ^~ err_puts cc1: some warnings being treated as errors vim +549 drivers/gpu//drm/i915/i915_gpu_error.c 520 521 static void print_error_obj(struct drm_i915_error_state_buf *m, 522 struct intel_engine_cs *engine, 523 const char *name, 524 struct drm_i915_error_object *obj) 525 { 526 char out[ASCII85_BUFSZ]; 527 int page; 528 529 if (!obj) 530 return; 531 532 if (name) { 533 err_printf(m, "%s --- %s = 0x%08x %08x\n", 534 engine ? engine->name : "global", name, 535 upper_32_bits(obj->gtt_offset), 536 lower_32_bits(obj->gtt_offset)); 537 } 538 539 err_compression_marker(m); 540 for (page = 0; page < obj->page_count; page++) { 541 int i, len; 542 543 len = PAGE_SIZE; 544 if (page == obj->page_count - 1) 545 len -= obj->unused; 546 len = ascii85_encode_len(len); 547 548 for (i = 0; i < len; i++) > 549 error_puts(m, > ascii85_encode(obj->pages[page][i], out)); 550 } 551 err_puts(m, "\n"); 552 } 553 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/4] drm/i915: Check hdcp key loadability
On Fri, Apr 06, 2018 at 07:02:02PM +0300, Ville Syrjälä wrote: > On Mon, Apr 02, 2018 at 02:35:42PM +0530, Ramalingam C wrote: > > > > > > On Thursday 29 March 2018 07:54 PM, Ville Syrjälä wrote: > > > On Thu, Mar 29, 2018 at 07:39:07PM +0530, Ramalingam C wrote: > > >> HDCP1.4 key can be loaded, only when Power well #1 is enabled and cdclk > > >> is enabled. Using the I915 power well infrastruture, above requirement > > >> is verified. > > >> > > >> This patch enables the hdcp initialization for HSW, BDW, and BXT. > > >> > > >> v2: > > >>Choose the PW# based on the platform. > > >> > > >> Signed-off-by: Ramalingam C > > >> Reviewed-by: Sean Paul > > >> --- > > >> drivers/gpu/drm/i915/intel_hdcp.c | 41 > > >> +-- > > >> 1 file changed, 39 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > > >> b/drivers/gpu/drm/i915/intel_hdcp.c > > >> index f77d956b2b18..d8af09b46a44 100644 > > >> --- a/drivers/gpu/drm/i915/intel_hdcp.c > > >> +++ b/drivers/gpu/drm/i915/intel_hdcp.c > > >> @@ -37,6 +37,43 @@ static int intel_hdcp_poll_ksv_fifo(struct > > >> intel_digital_port *intel_dig_port, > > >> return 0; > > >> } > > >> > > >> +static bool hdcp_key_loadable(struct drm_i915_private *dev_priv) > > >> +{ > > >> +struct i915_power_domains *power_domains = > > >> &dev_priv->power_domains; > > >> +struct i915_power_well *power_well; > > >> +enum i915_power_well_id id; > > >> +bool enabled = false; > > >> + > > >> +/* > > >> + * On HSW and BDW, Display HW loads the Key as soon as Display > > >> resumes. > > >> + * On all BXT+, SW can load the keys only when the PW#1 is > > >> turned on. > > >> + */ > > >> +if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > > >> +id = HSW_DISP_PW_GLOBAL; > > >> +else > > >> +id = SKL_DISP_PW_1; > > >> + > > >> +mutex_lock(&power_domains->lock); > > >> + > > >> +/* PG1 (power well #1) needs to be enabled */ > > >> +for_each_power_well(dev_priv, power_well) { > > >> +if (power_well->id == id) { > > >> +enabled = power_well->ops->is_enabled(dev_priv, > > >> + > > >> power_well); > > >> +break; > > >> +} > > >> +} > > >> +mutex_unlock(&power_domains->lock); > > >> + > > >> +/* > > >> + * Another req for hdcp key loadability is enabled state of pll > > >> for > > >> + * cdclk. Without active crtc we wont land here. So we are > > >> assuming that > > >> + * cdclk is already on. > > >> + */ > > >> + > > >> +return enabled; > > >> +} > > >> + > > >> static void intel_hdcp_clear_keys(struct drm_i915_private *dev_priv) > > >> { > > >> I915_WRITE(HDCP_KEY_CONF, HDCP_CLEAR_KEYS_TRIGGER); > > >> @@ -615,8 +652,8 @@ static int _intel_hdcp_enable(struct intel_connector > > >> *connector) > > >> DRM_DEBUG_KMS("[%s:%d] HDCP is being enabled...\n", > > >>connector->base.name, connector->base.base.id); > > >> > > >> -if (!(I915_READ(SKL_FUSE_STATUS) & SKL_FUSE_PG_DIST_STATUS(1))) > > >> { > > >> -DRM_ERROR("PG1 is disabled, cannot load keys\n"); > > >> +if (!hdcp_key_loadable(dev_priv)) { > > >> +DRM_ERROR("HDCP key Load is not possible\n"); > > >> return -ENXIO; > > >> } > > > If you need the power well then why aren't you grabbing the correct > > > power domain reference somewhere? > > > > Ville, > > > > As HDCP is supposed to be enabled after the basic display is up, power well > > #1 is supposed to be enabled. > > If not that is mostly an error w.r.t HDCP. > > > > So at this point we just want to verify whether required PW is up and HDCP > > key can be loaded from the HW. Else fail the HDCP request. > > So this is in practice dead code to deal with a hypothetical bug > elsewhere in the driver? Yeah looks like it should be wrapped in a WARN_ON, or maybe outright thrown out. The unclaimed mmio debug stuff will catch when this happens (or well, should). -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] drm/i915: Do NOT skip the first 4k of stolen memory for pre-allocated buffers
On Sat, Apr 07, 2018 at 11:34:57AM +0200, Hans de Goede wrote: > Hi, > > On 06-04-18 18:06, Ville Syrjälä wrote: > > On Thu, Apr 05, 2018 at 01:37:31PM +0200, Hans de Goede wrote: > > > Hi, > > > > > > On 04-04-18 22:49, Ville Syrjälä wrote: > > > > On Wed, Apr 04, 2018 at 10:06:29PM +0200, Hans de Goede wrote: > > > > > Hi, > > > > > > > > > > On 04-04-18 17:50, Ville Syrjälä wrote: > > > > > > On Fri, Mar 30, 2018 at 04:26:53PM +0200, Hans de Goede wrote: > > > > > > > Hi, > > > > > > > > > > > > > > On 30-03-18 15:25, Hans de Goede wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > On 30-03-18 14:44, Chris Wilson wrote: > > > > > > > > > Quoting Hans de Goede (2018-03-30 13:37:40) > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > On 30-03-18 14:30, Chris Wilson wrote: > > > > > > > > > > > Quoting Hans de Goede (2018-03-30 13:27:15) > > > > > > > > > > > > Before this commit the WaSkipStolenMemoryFirstPage > > > > > > > > > > > > workaround code was > > > > > > > > > > > > skipping the first 4k by passing 4096 as start of the > > > > > > > > > > > > address range passed > > > > > > > > > > > > to drm_mm_init(). This means that calling > > > > > > > > > > > > drm_mm_reserve_node() to try and > > > > > > > > > > > > reserve the firmware framebuffer so that we can inherit > > > > > > > > > > > > it would always > > > > > > > > > > > > fail, as the firmware framebuffer starts at address 0. > > > > > > > > > > > > > > > > > > > > > > > > Commit d43537610470 ("drm/i915: skip the first 4k of > > > > > > > > > > > > stolen memory on > > > > > > > > > > > > everything >= gen8") says in its commit message: "This > > > > > > > > > > > > is confirmed to fix > > > > > > > > > > > > Skylake screen flickering issues (probably caused by > > > > > > > > > > > > the fact that we > > > > > > > > > > > > initialized a ring in the first page of stolen, but I > > > > > > > > > > > > didn't 100% confirm > > > > > > > > > > > > this theory)." > > > > > > > > > > > > > > > > > > > > > > > > Which suggests that it is safe to use the first page > > > > > > > > > > > > for a linear > > > > > > > > > > > > framebuffer as the firmware is doing. > > > > > > > > > > > > > > > > > > > > > > > > This commit always passes 0 as start to drm_mm_init() > > > > > > > > > > > > and works around > > > > > > > > > > > > WaSkipStolenMemoryFirstPage in > > > > > > > > > > > > i915_gem_stolen_insert_node_in_range() > > > > > > > > > > > > by insuring the start address passed by to > > > > > > > > > > > > drm_mm_insert_node_in_range() > > > > > > > > > > > > is always 4k or more. All entry points to > > > > > > > > > > > > i915_gem_stolen.c go through > > > > > > > > > > > > i915_gem_stolen_insert_node_in_range(), so that any > > > > > > > > > > > > newly allocated > > > > > > > > > > > > objects such as ring-buffers will not be allocated in > > > > > > > > > > > > the first 4k. > > > > > > > > > > > > > > > > > > > > > > > > The one exception is > > > > > > > > > > > > i915_gem_object_create_stolen_for_preallocated() > > > > > > > > > > > > which directly calls drm_mm_reserve_node() which now > > > > > > > > > > > > will be able to > > > > > > > > > > > > use the first 4k. > > > > > > > > > > > > > > > > > > > > > > > > This fixes the i915 driver no longer being able to > > > > > > > > > > > > inherit the firmware > > > > > > > > > > > > framebuffer on gen8+, which fixes the video output > > > > > > > > > > > > changing from the > > > > > > > > > > > > vendor logo to a black screen as soon as the i915 > > > > > > > > > > > > driver is loaded > > > > > > > > > > > > (on systems without fbcon). > > > > > > > > > > > > > > > > > > > > > > We've been told by the HW guys not to use the first page. > > > > > > > > > > > (That's my > > > > > > > > > > > understanding from every time this gets questioned.) > > > > > > > > > > > > > > > > > > > > Yet the GOP is happily using the first page. I think we may > > > > > > > > > > need to make > > > > > > > > > > a difference here between the GPU not using the first page > > > > > > > > > > and the > > > > > > > > > > display engine/pipeline not using the first page. Note that > > > > > > > > > > my patch > > > > > > > > > > only influences the inheriting of the initial framebuffer > > > > > > > > > > as allocated > > > > > > > > > > by the GOP. It does not influence any other allocations > > > > > > > > > > from the > > > > > > > > > > reserved range, those will still all avoid the first page. > > > > > > > > > > > > > > > > > > > > Without this patch fastboot / flickerfree support is > > > > > > > > > > essentially broken > > > > > > > > > > on any gen8+ hardware given that one of the goals of atomic > > > > > > > > > > is to be > > > > > > > > > > able to do flickerfree transitions I think that this > > > > > > > > > > warrants a closer > > > > > > > > > > look then just simply saying never use the first page. > > > > > > > > > > > > > > > > >
Re: [Intel-gfx] [PATCH v1 00/14] Preparation patches for NV12
> -Original Message- > From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] > Sent: Monday, April 9, 2018 2:04 PM > To: Srinivas, Vidya ; intel- > g...@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v1 00/14] Preparation patches for NV12 > > Op 09-04-18 om 05:40 schreef Vidya Srinivas: > > Series contain preparation patches for NV12 support Enabling NV12 KMD > > support will follow the series > > > > Chandra Konduru (3): > > drm/i915: Set scaler mode for NV12 > > drm/i915: Update format_is_yuv() to include NV12 > > drm/i915: Upscale scaler max scale for NV12 > > > > Mahesh Kumar (9): > > drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values > > drm/i915/skl+: refactor WM calculation for NV12 > > drm/i915/skl+: add NV12 in skl_format_to_fourcc > > drm/i915/skl+: support verification of DDB HW state for NV12 > > drm/i915/skl+: NV12 related changes for WM > > drm/i915/skl+: pass skl_wm_level struct to wm compute func > > drm/i915/skl+: make sure higher latency level has higher wm value > > drm/i915/skl+: nv12 workaround disable WM level 1-7 > > drm/i915/skl: split skl_compute_ddb function > > > > Vidya Srinivas (2): > > drm/i915: Display WA 827 > > drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg > > > > drivers/gpu/drm/i915/i915_drv.h | 10 +- > > drivers/gpu/drm/i915/i915_reg.h | 5 + > > drivers/gpu/drm/i915/intel_atomic.c | 14 +- > > drivers/gpu/drm/i915/intel_display.c | 93 ++-- > > drivers/gpu/drm/i915/intel_drv.h | 11 +- > > drivers/gpu/drm/i915/intel_pm.c | 438 ++- > > > drivers/gpu/drm/i915/intel_sprite.c | 7 +- > > 7 files changed, 393 insertions(+), 185 deletions(-) > > This series looks good, so for any patches I missed: > > Reviewed-by: Maarten Lankhorst > > Do you have commit rights, or should I push them? I don’t have commit rights I think. Could you please push it? Thank you. Regards Vidya ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v1 00/14] Preparation patches for NV12
Op 09-04-18 om 05:40 schreef Vidya Srinivas: > Series contain preparation patches for NV12 support > Enabling NV12 KMD support will follow the series > > Chandra Konduru (3): > drm/i915: Set scaler mode for NV12 > drm/i915: Update format_is_yuv() to include NV12 > drm/i915: Upscale scaler max scale for NV12 > > Mahesh Kumar (9): > drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values > drm/i915/skl+: refactor WM calculation for NV12 > drm/i915/skl+: add NV12 in skl_format_to_fourcc > drm/i915/skl+: support verification of DDB HW state for NV12 > drm/i915/skl+: NV12 related changes for WM > drm/i915/skl+: pass skl_wm_level struct to wm compute func > drm/i915/skl+: make sure higher latency level has higher wm value > drm/i915/skl+: nv12 workaround disable WM level 1-7 > drm/i915/skl: split skl_compute_ddb function > > Vidya Srinivas (2): > drm/i915: Display WA 827 > drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg > > drivers/gpu/drm/i915/i915_drv.h | 10 +- > drivers/gpu/drm/i915/i915_reg.h | 5 + > drivers/gpu/drm/i915/intel_atomic.c | 14 +- > drivers/gpu/drm/i915/intel_display.c | 93 ++-- > drivers/gpu/drm/i915/intel_drv.h | 11 +- > drivers/gpu/drm/i915/intel_pm.c | 438 > ++- > drivers/gpu/drm/i915/intel_sprite.c | 7 +- > 7 files changed, 393 insertions(+), 185 deletions(-) This series looks good, so for any patches I missed: Reviewed-by: Maarten Lankhorst Do you have commit rights, or should I push them? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/13] drm/omapdrm: Nuke omap_framebuffer_get_next_connector()
On Fri, Apr 06, 2018 at 09:10:43AM +0300, Tomi Valkeinen wrote: > On 05/04/18 19:50, Daniel Vetter wrote: > > On Thu, Apr 05, 2018 at 06:13:58PM +0300, Ville Syrjala wrote: > >> From: Ville Syrjälä > >> > >> omap_framebuffer_get_next_connector() uses plane->fb which we want to > >> deprecate for atomic drivers. As omap_framebuffer_get_next_connector() > >> is unused just nuke the entire function. > >> > >> Cc: Tomi Valkeinen > >> Signed-off-by: Ville Syrjälä > > > > Yeah was slightly worried how to fix up this one, but we're lucky! > > > > Reviewed-by: Daniel Vetter > > I tried to remove it just a week ago, but Sebastian said that it's used > by a unmerged series about DSI command mode displays, so I dropped the > patch. > > In the unmerged series, it's used by omap_framebuffer_dirty() ([PATCHv3 > 3/8] drm/omap: add support for manually updated displays). So we have a > framebuffer, and we want to know which crtcs need to be flushed. You can't do that in atomic drivers. You need to take appropriate locks and do the full ->state->fb deref. Also, there's a gazillion of copies for generic framebuffer_dirty implementations floating around, pleas try to coordinate with those. -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 00/14] Preparation patches for NV12
> -Original Message- > From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] > Sent: Monday, April 9, 2018 2:04 PM > To: Srinivas, Vidya ; intel- > g...@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v1 00/14] Preparation patches for NV12 > > Op 09-04-18 om 05:40 schreef Vidya Srinivas: > > Series contain preparation patches for NV12 support Enabling NV12 KMD > > support will follow the series > > > > Chandra Konduru (3): > > drm/i915: Set scaler mode for NV12 > > drm/i915: Update format_is_yuv() to include NV12 > > drm/i915: Upscale scaler max scale for NV12 > > > > Mahesh Kumar (9): > > drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values > > drm/i915/skl+: refactor WM calculation for NV12 > > drm/i915/skl+: add NV12 in skl_format_to_fourcc > > drm/i915/skl+: support verification of DDB HW state for NV12 > > drm/i915/skl+: NV12 related changes for WM > > drm/i915/skl+: pass skl_wm_level struct to wm compute func > > drm/i915/skl+: make sure higher latency level has higher wm value > > drm/i915/skl+: nv12 workaround disable WM level 1-7 > > drm/i915/skl: split skl_compute_ddb function > > > > Vidya Srinivas (2): > > drm/i915: Display WA 827 > > drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg > > > > drivers/gpu/drm/i915/i915_drv.h | 10 +- > > drivers/gpu/drm/i915/i915_reg.h | 5 + > > drivers/gpu/drm/i915/intel_atomic.c | 14 +- > > drivers/gpu/drm/i915/intel_display.c | 93 ++-- > > drivers/gpu/drm/i915/intel_drv.h | 11 +- > > drivers/gpu/drm/i915/intel_pm.c | 438 ++- > > > drivers/gpu/drm/i915/intel_sprite.c | 7 +- > > 7 files changed, 393 insertions(+), 185 deletions(-) > > This series looks good, so for any patches I missed: > > Reviewed-by: Maarten Lankhorst > > Do you have commit rights, or should I push them? Thank you. I don’t have commit rights I think. Also, Should I add your RB for all the patches and push them again? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v1 00/14] Preparation patches for NV12
Op 09-04-18 om 10:57 schreef Srinivas, Vidya: > >> -Original Message- >> From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] >> Sent: Monday, April 9, 2018 2:04 PM >> To: Srinivas, Vidya ; intel- >> g...@lists.freedesktop.org >> Subject: Re: [Intel-gfx] [PATCH v1 00/14] Preparation patches for NV12 >> >> Op 09-04-18 om 05:40 schreef Vidya Srinivas: >>> Series contain preparation patches for NV12 support Enabling NV12 KMD >>> support will follow the series >>> >>> Chandra Konduru (3): >>> drm/i915: Set scaler mode for NV12 >>> drm/i915: Update format_is_yuv() to include NV12 >>> drm/i915: Upscale scaler max scale for NV12 >>> >>> Mahesh Kumar (9): >>> drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values >>> drm/i915/skl+: refactor WM calculation for NV12 >>> drm/i915/skl+: add NV12 in skl_format_to_fourcc >>> drm/i915/skl+: support verification of DDB HW state for NV12 >>> drm/i915/skl+: NV12 related changes for WM >>> drm/i915/skl+: pass skl_wm_level struct to wm compute func >>> drm/i915/skl+: make sure higher latency level has higher wm value >>> drm/i915/skl+: nv12 workaround disable WM level 1-7 >>> drm/i915/skl: split skl_compute_ddb function >>> >>> Vidya Srinivas (2): >>> drm/i915: Display WA 827 >>> drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg >>> >>> drivers/gpu/drm/i915/i915_drv.h | 10 +- >>> drivers/gpu/drm/i915/i915_reg.h | 5 + >>> drivers/gpu/drm/i915/intel_atomic.c | 14 +- >>> drivers/gpu/drm/i915/intel_display.c | 93 ++-- >>> drivers/gpu/drm/i915/intel_drv.h | 11 +- >>> drivers/gpu/drm/i915/intel_pm.c | 438 ++- >> >>> drivers/gpu/drm/i915/intel_sprite.c | 7 +- >>> 7 files changed, 393 insertions(+), 185 deletions(-) >> This series looks good, so for any patches I missed: >> >> Reviewed-by: Maarten Lankhorst >> >> Do you have commit rights, or should I push them? > Thank you. I don’t have commit rights I think. > Also, Should I add your RB for all the patches and push them again? > I'll push them. :) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Keep a count of requests submitted from userspace
On 06/04/2018 21:17, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-04-05 13:39:19) From: Tvrtko Ursulin Keep a count of requests submitted from userspace and not yet runnable due unresolved dependencies. v2: Rename and move under the container struct. (Chris Wilson) v3: Rebase. Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_request.c | 3 +++ drivers/gpu/drm/i915/intel_engine_cs.c | 3 ++- drivers/gpu/drm/i915/intel_ringbuffer.h | 8 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 5c01291ad1cc..152321655fe6 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -640,6 +640,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) rcu_read_lock(); request->engine->submit_request(request); rcu_read_unlock(); + atomic_dec(&request->engine->request_stats.queued); But we use atomic here? Might as well use atomic for request_stats.runnable here as well? I admit it can read a bit uneven. For runnable I wanted to avoid another atomic by putting it under the engine timeline lock. But for queued I did not want to start taking the same lock when adding a request. Your proposal to make runnable atomic_t and move to submit_notify would indeed simplify things, but at a cost of one more atomic in that path. Perhaps the code path is heavy enough for one new atomic to be completely hidden in it, and code simplification to win? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/7] drm/i915/pmu: Add running counter
On 06/04/2018 21:24, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-04-05 13:39:22) From: Tvrtko Ursulin We add a PMU counter to expose the number of requests currently executing on the GPU. This is useful to analyze the overall load of the system. v2: * Rebase. * Drop floating point constant. (Chris Wilson) v3: * Change scale to 1024 for faster arithmetics. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Reviewed-by: Chris Wilson Do we want these separate in the final push? Is there value in reverting one but not the others? They seem a triumvirate. I think the only benefit to have them separate for me was that rebasing was marginally easier. I can just as well squash them if that is preferred. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Keep a count of requests submitted from userspace
Quoting Tvrtko Ursulin (2018-04-09 10:11:53) > > On 06/04/2018 21:17, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-04-05 13:39:19) > >> From: Tvrtko Ursulin > >> > >> Keep a count of requests submitted from userspace and not yet runnable due > >> unresolved dependencies. > >> > >> v2: Rename and move under the container struct. (Chris Wilson) > >> v3: Rebase. > >> > >> Signed-off-by: Tvrtko Ursulin > >> --- > >> drivers/gpu/drm/i915/i915_request.c | 3 +++ > >> drivers/gpu/drm/i915/intel_engine_cs.c | 3 ++- > >> drivers/gpu/drm/i915/intel_ringbuffer.h | 8 > >> 3 files changed, 13 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_request.c > >> b/drivers/gpu/drm/i915/i915_request.c > >> index 5c01291ad1cc..152321655fe6 100644 > >> --- a/drivers/gpu/drm/i915/i915_request.c > >> +++ b/drivers/gpu/drm/i915/i915_request.c > >> @@ -640,6 +640,7 @@ submit_notify(struct i915_sw_fence *fence, enum > >> i915_sw_fence_notify state) > >> rcu_read_lock(); > >> request->engine->submit_request(request); > >> rcu_read_unlock(); > >> + atomic_dec(&request->engine->request_stats.queued); > > > > But we use atomic here? Might as well use atomic for > > request_stats.runnable here as well? > > I admit it can read a bit uneven. > > For runnable I wanted to avoid another atomic by putting it under the > engine timeline lock. > > But for queued I did not want to start taking the same lock when adding > a request. > > Your proposal to make runnable atomic_t and move to submit_notify would > indeed simplify things, but at a cost of one more atomic in that path. > Perhaps the code path is heavy enough for one new atomic to be > completely hidden in it, and code simplification to win? It also solidifies that we are moving from one counter to the next. (There must be some common 64b cmpxchg for doing that!) Going from +1 locked operations to +2 here isn't the end of the world, but I can certainly appreciated trying to keep the number down (especially for aux information like stats). now = atomic64_read(&stats.queued_runnable); do { old = now; new_queued = upper_32_bits(old) - 1; new_runnable = lower_32_bits(old) + 1; now = atomic64_cmpxchg(&stats.queued_runnable, old, (new_runnable | (u64)new_queued << 32)); } while (now != old); Downside being that we either then use atomic64 throughout or we mix atomic32/atomic64 knowing that we're on x86. (I feel like someone else must have solved this problem in a much neater way, before they went to per-cpu stats ;) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v1 00/14] Preparation patches for NV12
> -Original Message- > From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] > Sent: Monday, April 9, 2018 2:38 PM > To: Srinivas, Vidya ; intel- > g...@lists.freedesktop.org > Cc: Kamath, Sunil > Subject: Re: [Intel-gfx] [PATCH v1 00/14] Preparation patches for NV12 > > Op 09-04-18 om 10:57 schreef Srinivas, Vidya: > > > >> -Original Message- > >> From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] > >> Sent: Monday, April 9, 2018 2:04 PM > >> To: Srinivas, Vidya ; intel- > >> g...@lists.freedesktop.org > >> Subject: Re: [Intel-gfx] [PATCH v1 00/14] Preparation patches for > >> NV12 > >> > >> Op 09-04-18 om 05:40 schreef Vidya Srinivas: > >>> Series contain preparation patches for NV12 support Enabling NV12 > >>> KMD support will follow the series > >>> > >>> Chandra Konduru (3): > >>> drm/i915: Set scaler mode for NV12 > >>> drm/i915: Update format_is_yuv() to include NV12 > >>> drm/i915: Upscale scaler max scale for NV12 > >>> > >>> Mahesh Kumar (9): > >>> drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values > >>> drm/i915/skl+: refactor WM calculation for NV12 > >>> drm/i915/skl+: add NV12 in skl_format_to_fourcc > >>> drm/i915/skl+: support verification of DDB HW state for NV12 > >>> drm/i915/skl+: NV12 related changes for WM > >>> drm/i915/skl+: pass skl_wm_level struct to wm compute func > >>> drm/i915/skl+: make sure higher latency level has higher wm value > >>> drm/i915/skl+: nv12 workaround disable WM level 1-7 > >>> drm/i915/skl: split skl_compute_ddb function > >>> > >>> Vidya Srinivas (2): > >>> drm/i915: Display WA 827 > >>> drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg > >>> > >>> drivers/gpu/drm/i915/i915_drv.h | 10 +- > >>> drivers/gpu/drm/i915/i915_reg.h | 5 + > >>> drivers/gpu/drm/i915/intel_atomic.c | 14 +- > >>> drivers/gpu/drm/i915/intel_display.c | 93 ++-- > >>> drivers/gpu/drm/i915/intel_drv.h | 11 +- > >>> drivers/gpu/drm/i915/intel_pm.c | 438 ++-- > --- > >> > >>> drivers/gpu/drm/i915/intel_sprite.c | 7 +- > >>> 7 files changed, 393 insertions(+), 185 deletions(-) > >> This series looks good, so for any patches I missed: > >> > >> Reviewed-by: Maarten Lankhorst > >> > >> Do you have commit rights, or should I push them? > > Thank you. I don’t have commit rights I think. > > Also, Should I add your RB for all the patches and push them again? > > > I'll push them. :) Thank you so much :) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnp: Properly handle VBT ddc pin out of bounds.
On Fri, 23 Mar 2018, Timo Aaltonen wrote: > On 30.01.2018 00:12, Rodrigo Vivi wrote: >> On Mon, Jan 29, 2018 at 05:42:53AM +, Kai Heng Feng wrote: >>> On 26 Jan 2018, at 6:25 AM, Rodrigo Vivi wrote: If the table result is out of bounds on the array map there is something really wrong with VBT pin so we don't return that vbt_pin, but only return 0 instead. This basically reverts commit 'a8e6f3888b05 ("drm/i915/cnp: Ignore VBT request for know invalid DDC pin.")' Also this properly fixes commit 9c3b2689d01f ("drm/i915/cnl: Map VBT DDC Pin to BSpec DDC Pin.") v2: Do in a way that we don't break other platforms. (Jani) v3: Keep debug message (Jani) v4: Don't mess with 0 mapping was noticed by Jani and addressed with a simple solution suggested by Lucas that makes this even simpler. Fixes: a8e6f3888b05 ("drm/i915/cnp: Ignore VBT request for know invalid DDC pin.") Fixes: 9c3b2689d01f ("drm/i915/cnl: Map VBT DDC Pin to BSpec DDC Pin.") Cc: Radhakrishna Sripada Cc: Jani Nikula Cc: Kai Heng Feng Cc: Lucas De Marchi Suggested-by: Lucas De Marchi Signed-off-by: Rodrigo Vivi >>> >>> Tested-by: Kai-Heng Feng >> >> merged. thanks for suggestions, reviews, tests and patience ;) > > Shouldn't this and > > drm/i915/cnp: Ignore VBT request for know invalid DDC pin. > > be cc:stable? Though they aren't even in 4.16 yet. Apologies for nobody replying. The commits are in v4.16, and I've made a stable backport request for v4.15. BR, Jani. -- 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] drm/i915: Silence debugging DRM_ERROR for failing to suspend vlv powerwells
If we try to suspend a wedged device following a GPU reset failure, we will also fail to turn off the rc6 powerwells (on vlv), leading to a *ERROR*. This is quite expected in this case, so the best we can do is shake our heads and reduce the *ERROR* to a debug so CI stops complaining. Testcase: igt/gem_eio/in-flight-suspend #vlv Signed-off-by: Chris Wilson Cc: Imre Deak Cc: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index f770be18b2d7..db6fc176ec3c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2462,10 +2462,13 @@ static void vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv, /* * RC6 transitioning can be delayed up to 2 msec (see * valleyview_enable_rps), use 3 msec for safety. +* +* This can fail to turn off the rc6 if the GPU is stuck after a failed +* reset and we are trying to force the machine to sleep. */ if (vlv_wait_for_pw_status(dev_priv, mask, val)) - DRM_ERROR("timeout waiting for GT wells to go %s\n", - onoff(wait_for_on)); + DRM_DEBUG_DRIVER("timeout waiting for GT wells to go %s\n", +onoff(wait_for_on)); } static void vlv_check_no_gt_access(struct drm_i915_private *dev_priv) -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Silence debugging DRM_ERROR for failing to suspend vlv powerwells
Quoting Chris Wilson (2018-04-09 10:49:05) > If we try to suspend a wedged device following a GPU reset failure, we > will also fail to turn off the rc6 powerwells (on vlv), leading to a > *ERROR*. This is quite expected in this case, so the best we can do is > shake our heads and reduce the *ERROR* to a debug so CI stops > complaining. > > Testcase: igt/gem_eio/in-flight-suspend #vlv Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105583 -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Keep a count of requests submitted from userspace
On 09/04/2018 10:25, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-04-09 10:11:53) On 06/04/2018 21:17, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-04-05 13:39:19) From: Tvrtko Ursulin Keep a count of requests submitted from userspace and not yet runnable due unresolved dependencies. v2: Rename and move under the container struct. (Chris Wilson) v3: Rebase. Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_request.c | 3 +++ drivers/gpu/drm/i915/intel_engine_cs.c | 3 ++- drivers/gpu/drm/i915/intel_ringbuffer.h | 8 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 5c01291ad1cc..152321655fe6 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -640,6 +640,7 @@ submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state) rcu_read_lock(); request->engine->submit_request(request); rcu_read_unlock(); + atomic_dec(&request->engine->request_stats.queued); But we use atomic here? Might as well use atomic for request_stats.runnable here as well? I admit it can read a bit uneven. For runnable I wanted to avoid another atomic by putting it under the engine timeline lock. But for queued I did not want to start taking the same lock when adding a request. Your proposal to make runnable atomic_t and move to submit_notify would indeed simplify things, but at a cost of one more atomic in that path. Perhaps the code path is heavy enough for one new atomic to be completely hidden in it, and code simplification to win? It also solidifies that we are moving from one counter to the next. (There must be some common 64b cmpxchg for doing that!) Going from +1 locked operations to +2 here isn't the end of the world, but I can certainly appreciated trying to keep the number down (especially for aux information like stats). now = atomic64_read(&stats.queued_runnable); do { old = now; new_queued = upper_32_bits(old) - 1; new_runnable = lower_32_bits(old) + 1; now = atomic64_cmpxchg(&stats.queued_runnable, old, (new_runnable | (u64)new_queued << 32)); } while (now != old); Hm don't know, have to be careful with these retry loops. More importantly I am not sure if it isn't an overkill. Downside being that we either then use atomic64 throughout or we mix atomic32/atomic64 knowing that we're on x86. (I feel like someone else must have solved this problem in a much neater way, before they went to per-cpu stats ;) Is the winky implying you know who and where? :) We have three potential solutions now, even for if the winky is suggesting something. For me it is still a choice between what I have versus simplifying the code paths by going another atomic_t. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Park before resetting the submission backend
As different backends may have different park/unpark callbacks, we should only ever switch backends (reset_default_submission on wedge recovery, or on enabling the guc) while parked. v2: Remove the assert from the guc code, as we are currently trying to modify the engine vfuncs pointer on a live system after reset (not just wedging). We will just have to hope that the system is balanced. v3: Rebase onto __i915_gem_park and improve grammar. Signed-off-by: Chris Wilson Cc: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Tvrtko Ursulin Cc: Mika Kuoppala --- drivers/gpu/drm/i915/i915_gem.c| 15 --- drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 28ab0beff86c..dd3e292ba243 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -144,8 +144,6 @@ static u32 __i915_gem_park(struct drm_i915_private *i915) if (!i915->gt.awake) return I915_EPOCH_INVALID; - GEM_BUG_ON(i915->gt.epoch == I915_EPOCH_INVALID); - /* * Be paranoid and flush a concurrent interrupt to make sure * we don't reactivate any irq tasklets after parking. @@ -173,6 +171,7 @@ static u32 __i915_gem_park(struct drm_i915_private *i915) intel_runtime_pm_put(i915); + GEM_BUG_ON(i915->gt.epoch == I915_EPOCH_INVALID); return i915->gt.epoch; } @@ -3435,7 +3434,17 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915) } } i915_retire_requests(i915); - GEM_BUG_ON(i915->gt.active_requests); + + /* +* Park before disengaging the old submit mechanism as different +* backends may have different park/unpack callbacks. +* +* We are idle; the idle-worker will be queued, but we need to run +* it now. As we already hold the struct mutex, we can park the GPU +* right away, letting the lazy worker see that we are already active +* again by the time it acquires the mutex. +*/ + __i915_gem_park(i915); /* * Undo nop_submit_request. We prevent all new i915 requests from diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 12486d8f534b..b4ea77a2896c 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1651,6 +1651,9 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915) struct intel_engine_cs *engine; enum intel_engine_id id; + /* Must be parked first! */ + GEM_BUG_ON(i915->gt.awake); + for_each_engine(engine, i915, id) engine->set_default_submission(engine); } -- 2.17.0 ___ 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: Silence debugging DRM_ERROR for failing to suspend vlv powerwells
== Series Details == Series: drm/i915: Silence debugging DRM_ERROR for failing to suspend vlv powerwells URL : https://patchwork.freedesktop.org/series/41350/ State : success == Summary == Series 41350v1 drm/i915: Silence debugging DRM_ERROR for failing to suspend vlv powerwells https://patchwork.freedesktop.org/api/1.0/series/41350/revisions/1/mbox/ Known issues: Test debugfs_test: Subgroup read_all_entries: incomplete -> PASS (fi-snb-2520m) fdo#103713 Test gem_ctx_param: Subgroup basic-default: incomplete -> PASS (fi-cnl-y3) fdo#105086 Test kms_chamelium: Subgroup dp-edid-read: fail -> PASS (fi-kbl-7500u) fdo#102505 Test kms_flip: Subgroup basic-flip-vs-wf_vblank: fail -> PASS (fi-cfl-s3) fdo#100368 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-c: pass -> INCOMPLETE (fi-bxt-dsi) fdo#103927 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086 fdo#102505 https://bugs.freedesktop.org/show_bug.cgi?id=102505 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:434s fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:446s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:379s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:542s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:299s fi-bxt-dsi total:243 pass:216 dwarn:0 dfail:0 fail:0 skip:26 fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:512s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:521s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:509s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:410s fi-cfl-s3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:561s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:510s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:588s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:429s fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:317s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:540s fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:487s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:403s fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:420s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:462s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:441s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:470s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:463s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:508s fi-pnv-d510 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:666s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:438s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:544s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:503s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:499s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:426s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:446s fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:584s fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:403s 1be073153147c5c39cdcbdfdeb4e2595ba595bf7 drm-tip: 2018y-04m-07d-22h-26m-31s UTC integration manifest a64a9e12899b drm/i915: Silence debugging DRM_ERROR for failing to suspend vlv powerwells == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8636/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Keep a count of requests submitted from userspace
Quoting Tvrtko Ursulin (2018-04-09 11:17:04) > > On 09/04/2018 10:25, Chris Wilson wrote: > > Downside being that we either then use atomic64 throughout or we mix > > atomic32/atomic64 knowing that we're on x86. (I feel like someone else > > must have solved this problem in a much neater way, before they went to > > per-cpu stats ;) > > Is the winky implying you know who and where? :) We have three potential > solutions now, even for if the winky is suggesting something. Nah, just that atomic/locked counters are so old hat. Not sure if there remain any good examples for hotpath counters that remain applicable to our code. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Keep a count of requests submitted from userspace
Quoting Chris Wilson (2018-04-09 11:27:17) > Quoting Tvrtko Ursulin (2018-04-09 11:17:04) > > > > On 09/04/2018 10:25, Chris Wilson wrote: > > > Downside being that we either then use atomic64 throughout or we mix > > > atomic32/atomic64 knowing that we're on x86. (I feel like someone else > > > must have solved this problem in a much neater way, before they went to > > > per-cpu stats ;) > > > > Is the winky implying you know who and where? :) We have three potential > > solutions now, even for if the winky is suggesting something. > > Nah, just that atomic/locked counters are so old hat. Not sure if there > remain any good examples for hotpath counters that remain applicable to > our code. With an underlying nudge that perhaps our hotpath code isn't so hot, and if we can squeeze out a few more cycles. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: set minimum CD clock to twice the BCLK.
On Fri, Apr 06, 2018 at 04:47:08PM +0300, Jani Nikula wrote: > On Thu, 05 Apr 2018, Abhay Kumar wrote: > > In glk when device boots with 1366x768 panel, HDA codec doesn't comeup. > > This result in no audio forever as cdclk is < 96Mhz. > > This chagne will ensure CD clock to be twice of BCLK. > > In short, we can't poke around CDCLK like this. It needs a full modeset, > and it's non-trivial from the path you're calling this from. I tried to cobble something together quickly: git://github.com/vsyrjala/linux.git glk_cnl_cdclk_audio Not tested at all, and I have no idea if my assumptions about snd_hda_intel actually hold. > > I'm considering pushing the original patch [1], because we haven't come > up with working alternatives. Please confirm that the patch reliably > fixes the issue. > > (Though I think if you boot with *all* outputs disabled, we'll choose > the lowest CDCLK possible regardless of the patch, reproducing the same > issue.) > > What's the CDCLK frequency set by the BIOS/GOP at boot? There are no > logs with drm.debug=14 attached to the referenced bug. > > I see that bspec says, "158.4 MHz CD (Display Audio enumeration and > playback OK)" but that's *not* twice the BCLK. I'm inclined to lean > towards 192 MHz min leading to max CDCLK on GLK. > > BR, > Jani. > > > [1] https://patchwork.freedesktop.org/patch/184778/ > > > > > > > v2: > > - Address comment (Jani) > > - New design approach > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937 > > Signed-off-by: Abhay Kumar > > --- > > drivers/gpu/drm/i915/intel_audio.c | 33 ++--- > > drivers/gpu/drm/i915/intel_cdclk.c | 21 + > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > 3 files changed, 44 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_audio.c > > b/drivers/gpu/drm/i915/intel_audio.c > > index 709d6ca68074..f7dd3d532e93 100644 > > --- a/drivers/gpu/drm/i915/intel_audio.c > > +++ b/drivers/gpu/drm/i915/intel_audio.c > > @@ -723,15 +723,37 @@ static void i915_audio_component_put_power(struct > > device *kdev) > > intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO); > > } > > > > +/* Get CDCLK in kHz */ > > +static int i915_audio_component_get_cdclk_freq(struct device *kdev) > > +{ > > + struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > > + > > + if (WARN_ON_ONCE(!HAS_DDI(dev_priv))) > > + return -ENODEV; > > + > > + return dev_priv->cdclk.hw.cdclk; > > +} > > + > > static void i915_audio_component_codec_wake_override(struct device *kdev, > > bool enable) > > { > > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > > u32 tmp; > > + int current_cdclk; > > > > if (!IS_GEN9_BC(dev_priv)) > > return; > > > > + current_cdclk = i915_audio_component_get_cdclk_freq(kdev); > > + > > + /* > > +* Before probing for HDA Codec we need to make sure > > +* "The CD clock frequency must be at least twice > > +* the frequency of the Azalia BCLK." > > +*/ > > + if (INTEL_GEN(dev_priv) >= 9 && current_cdclk <= 192000) > > + intel_cdclk_bump(dev_priv); > > + > > i915_audio_component_get_power(kdev); > > > > /* > > @@ -753,17 +775,6 @@ static void > > i915_audio_component_codec_wake_override(struct device *kdev, > > i915_audio_component_put_power(kdev); > > } > > > > -/* Get CDCLK in kHz */ > > -static int i915_audio_component_get_cdclk_freq(struct device *kdev) > > -{ > > - struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > > - > > - if (WARN_ON_ONCE(!HAS_DDI(dev_priv))) > > - return -ENODEV; > > - > > - return dev_priv->cdclk.hw.cdclk; > > -} > > - > > /* > > * get the intel_encoder according to the parameter port and pipe > > * intel_encoder is saved by the index of pipe > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c > > b/drivers/gpu/drm/i915/intel_cdclk.c > > index dc7db8a2caf8..9426e1b7badc 100644 > > --- a/drivers/gpu/drm/i915/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > > @@ -1516,6 +1516,27 @@ void bxt_init_cdclk(struct drm_i915_private > > *dev_priv) > > } > > > > /** > > + * intel_cdclk_bump - Increase cdclk to 2* BCLK > > + * @dev_priv: i915 device > > + * > > + * Increase CDCLK for GKL and CNL. This is done only > > + * during HDA codec probe. > > + */ > > +void intel_cdclk_bump(struct drm_i915_private *dev_priv) > > +{ > > + struct intel_cdclk_state cdclk_state; > > + > > + cdclk_state = dev_priv->cdclk.hw; > > + > > + if (IS_GEMINILAKE(dev_priv)) { > > + cdclk_state.cdclk = glk_calc_cdclk((2*96000)); > > + cdclk_state.vco = glk_de_pll_vco(dev_priv, cdclk_state.cdclk); > > + cdclk_state.voltage_level = > > bxt_calc_voltage_level(cdclk_state.cdclk); > > + bxt_set_cdclk(dev_priv, &cdclk_state); > > + } > > +} > > + > > +/** > > * bxt_uninit_cdclk -
Re: [Intel-gfx] [PATCH] drm/i915: Park before resetting the submission backend
On 4/9/2018 3:48 PM, Chris Wilson wrote: As different backends may have different park/unpark callbacks, we should only ever switch backends (reset_default_submission on wedge recovery, or on enabling the guc) while parked. v2: Remove the assert from the guc code, as we are currently trying to modify the engine vfuncs pointer on a live system after reset (not just wedging). We will just have to hope that the system is balanced. v3: Rebase onto __i915_gem_park and improve grammar. Signed-off-by: Chris Wilson Cc: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Tvrtko Ursulin Cc: Mika Kuoppala Reviewed-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/i915_gem.c| 15 --- drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 28ab0beff86c..dd3e292ba243 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -144,8 +144,6 @@ static u32 __i915_gem_park(struct drm_i915_private *i915) if (!i915->gt.awake) return I915_EPOCH_INVALID; - GEM_BUG_ON(i915->gt.epoch == I915_EPOCH_INVALID); - /* * Be paranoid and flush a concurrent interrupt to make sure * we don't reactivate any irq tasklets after parking. @@ -173,6 +171,7 @@ static u32 __i915_gem_park(struct drm_i915_private *i915) intel_runtime_pm_put(i915); + GEM_BUG_ON(i915->gt.epoch == I915_EPOCH_INVALID); return i915->gt.epoch; } @@ -3435,7 +3434,17 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915) } } i915_retire_requests(i915); - GEM_BUG_ON(i915->gt.active_requests); + + /* +* Park before disengaging the old submit mechanism as different +* backends may have different park/unpack callbacks. +* +* We are idle; the idle-worker will be queued, but we need to run +* it now. As we already hold the struct mutex, we can park the GPU +* right away, letting the lazy worker see that we are already active +* again by the time it acquires the mutex. +*/ + __i915_gem_park(i915); /* * Undo nop_submit_request. We prevent all new i915 requests from diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 12486d8f534b..b4ea77a2896c 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1651,6 +1651,9 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915) struct intel_engine_cs *engine; enum intel_engine_id id; + /* Must be parked first! */ + GEM_BUG_ON(i915->gt.awake); + for_each_engine(engine, i915, id) engine->set_default_submission(engine); } -- Thanks, Sagar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Keep a count of requests submitted from userspace
On 09/04/2018 11:27, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-04-09 11:17:04) On 09/04/2018 10:25, Chris Wilson wrote: Downside being that we either then use atomic64 throughout or we mix atomic32/atomic64 knowing that we're on x86. (I feel like someone else must have solved this problem in a much neater way, before they went to per-cpu stats ;) Is the winky implying you know who and where? :) We have three potential solutions now, even for if the winky is suggesting something. Nah, just that atomic/locked counters are so old hat. Not sure if there remain any good examples for hotpath counters that remain applicable to our code. Leave it as is then for now and improve if we discover it is not good enough? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Park before resetting the submission backend
Quoting Sagar Arun Kamble (2018-04-09 11:38:34) > > > On 4/9/2018 3:48 PM, Chris Wilson wrote: > > As different backends may have different park/unpark callbacks, we > > should only ever switch backends (reset_default_submission on wedge > > recovery, or on enabling the guc) while parked. > > > > v2: Remove the assert from the guc code, as we are currently trying to > > modify the engine vfuncs pointer on a live system after reset (not just > > wedging). We will just have to hope that the system is balanced. > > v3: Rebase onto __i915_gem_park and improve grammar. > > > > Signed-off-by: Chris Wilson > > Cc: Michal Wajdeczko > > Cc: Sagar Arun Kamble > > Cc: Tvrtko Ursulin > > Cc: Mika Kuoppala > Reviewed-by: Sagar Arun Kamble Michal, do you want to take this and merge it into your series? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Park before resetting the submission backend
On Mon, 09 Apr 2018 12:41:22 +0200, Chris Wilson wrote: Quoting Sagar Arun Kamble (2018-04-09 11:38:34) On 4/9/2018 3:48 PM, Chris Wilson wrote: > As different backends may have different park/unpark callbacks, we > should only ever switch backends (reset_default_submission on wedge > recovery, or on enabling the guc) while parked. > > v2: Remove the assert from the guc code, as we are currently trying to > modify the engine vfuncs pointer on a live system after reset (not just > wedging). We will just have to hope that the system is balanced. > v3: Rebase onto __i915_gem_park and improve grammar. > > Signed-off-by: Chris Wilson > Cc: Michal Wajdeczko > Cc: Sagar Arun Kamble > Cc: Tvrtko Ursulin > Cc: Mika Kuoppala Reviewed-by: Sagar Arun Kamble Michal, do you want to take this and merge it into your series? Yes, I can take it. Then we will see results from CI.IGT with GuC enabled. /Michal ps. this patch is also Reviewed-by: Michal Wajdeczko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Keep a count of requests submitted from userspace
Quoting Tvrtko Ursulin (2018-04-09 11:40:08) > > On 09/04/2018 11:27, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-04-09 11:17:04) > >> > >> On 09/04/2018 10:25, Chris Wilson wrote: > >>> Downside being that we either then use atomic64 throughout or we mix > >>> atomic32/atomic64 knowing that we're on x86. (I feel like someone else > >>> must have solved this problem in a much neater way, before they went to > >>> per-cpu stats ;) > >> > >> Is the winky implying you know who and where? :) We have three potential > >> solutions now, even for if the winky is suggesting something. > > > > Nah, just that atomic/locked counters are so old hat. Not sure if there > > remain any good examples for hotpath counters that remain applicable to > > our code. > > Leave it as is then for now and improve if we discover it is not good > enough? I did have an ulterior motive in that the cmpxchg did resolve one issue that irked me with the two counters being updated out of sync. Minor, minor glitches :) I don't have a strong preference either way. These instructions on the submit are not likely to stand out, as compared to the biggest fish of ksoftirqd, execlists_schedule() and execlists_dequeue(). -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Park before resetting the submission backend (rev2)
== Series Details == Series: drm/i915: Park before resetting the submission backend (rev2) URL : https://patchwork.freedesktop.org/series/41202/ State : warning == Summary == Series 41202v2 drm/i915: Park before resetting the submission backend https://patchwork.freedesktop.org/api/1.0/series/41202/revisions/2/mbox/ Possible new issues: Test gem_exec_gttfill: Subgroup basic: pass -> SKIP (fi-pnv-d510) Known issues: Test debugfs_test: Subgroup read_all_entries: incomplete -> PASS (fi-snb-2520m) fdo#103713 +1 Test gem_ctx_param: Subgroup basic-default: incomplete -> PASS (fi-cnl-y3) fdo#105086 Test kms_chamelium: Subgroup dp-edid-read: fail -> PASS (fi-kbl-7500u) fdo#102505 Test kms_flip: Subgroup basic-flip-vs-wf_vblank: fail -> PASS (fi-cfl-s3) fdo#100368 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086 fdo#102505 https://bugs.freedesktop.org/show_bug.cgi?id=102505 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:434s fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:444s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:383s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:543s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:296s fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:513s fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:514s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:518s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:505s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:412s fi-cfl-s3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:562s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:512s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:590s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:426s fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:314s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:535s fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:485s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:405s fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:422s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:464s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:438s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:474s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:466s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:507s fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:644s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:442s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:532s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:500s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:506s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:426s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:444s fi-snb-2520m total:242 pass:208 dwarn:0 dfail:0 fail:0 skip:33 fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:408s 1be073153147c5c39cdcbdfdeb4e2595ba595bf7 drm-tip: 2018y-04m-07d-22h-26m-31s UTC integration manifest 03338d1167d7 drm/i915: Park before resetting the submission backend == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8637/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/18] drm/i915/execlists: Set queue priority from secondary port
We can refine our current execlists->queue_priority if we inspect ELSP[1] rather than the head of the unsubmitted queue. Currently, we use the unsubmitted queue and say that if a subsequent request is more than important than the current queue, we will rerun the submission tasklet to evaluate the need for preemption. However, we only want to preempt if we need to jump ahead of a currently executing request in ELSP. The second reason for running the submission tasklet is amalgamate requests into the active context on ELSP[0] to avoid a stall when ELSP[0] drains. (Though repeatedly amalgamating requests into the active context and triggering many lite-restore is off question gain, the goal really is to put a context into ELSP[1] to cover the interrupt.) So if instead of looking at the head of the queue, we look at the context in ELSP[1] we can answer both of the questions more accurately -- we don't need to rerun the submission tasklet unless our new request is important enough to feed into, at least, ELSP[1]. References: f6322eddaff7 ("drm/i915/preemption: Allow preemption between submission ports") Signed-off-by: Chris Wilson Cc: Michał Winiarski Cc: Michel Thierry Cc: Mika Kuoppala Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_lrc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 3592288e4696..b47d53d284ca 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -713,7 +713,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine) kmem_cache_free(engine->i915->priorities, p); } done: - execlists->queue_priority = rb ? to_priolist(rb)->priority : INT_MIN; + execlists->queue_priority = + port != execlists->port ? rq_prio(last) : INT_MIN; execlists->first = rb; if (submit) port_assign(port, last); -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Still trying for context->preempt_timeout
Now with i915_reset_engine() marking the stalled request as guilty, preemption timeout doesn't lead into a GPU hang death spiral; at the loss of potentially resetting a context with no harm (in practice that didn't work out!). A bit ambivalent on the flip forcing reset, both the stutter and glitch can be seen under load. I'm not sold on the UX; it's bad either way. If we don't break the deadlock, the user can't interact with the system; if we do, they may see a glitch in one app. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/18] drm/i915/execlists: Refactor out complete_preempt_context()
As a complement to inject_preempt_context(), follow up with the function to handle its completion. This will be useful should we wish to extend the duties of the preempt-context for execlists. v2: And do the same for the guc. Signed-off-by: Chris Wilson Cc: Jeff McGee Cc: Michał Winiarski Reviewed-by: Jeff McGee #v1 --- drivers/gpu/drm/i915/intel_guc_submission.c | 26 ++--- drivers/gpu/drm/i915/intel_lrc.c| 23 ++ 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 97121230656c..04125e8cec6b 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -621,6 +621,21 @@ static void wait_for_guc_preempt_report(struct intel_engine_cs *engine) report->report_return_status = INTEL_GUC_REPORT_STATUS_UNKNOWN; } +static void complete_preempt_context(struct intel_engine_cs *engine) +{ + struct intel_engine_execlists *execlists = &engine->execlists; + + GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT)); + + execlists_cancel_port_requests(execlists); + execlists_unwind_incomplete_requests(execlists); + + wait_for_guc_preempt_report(engine); + intel_write_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX, 0); + + execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT); +} + /** * guc_submit() - Submit commands through GuC * @engine: engine associated with the commands @@ -775,15 +790,8 @@ static void guc_submission_tasklet(unsigned long data) if (execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT) && intel_read_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX) == - GUC_PREEMPT_FINISHED) { - execlists_cancel_port_requests(&engine->execlists); - execlists_unwind_incomplete_requests(execlists); - - wait_for_guc_preempt_report(engine); - - execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT); - intel_write_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX, 0); - } + GUC_PREEMPT_FINISHED) + complete_preempt_context(engine); if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT)) guc_dequeue(engine); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index b47d53d284ca..5773a0e8dc51 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -546,8 +546,18 @@ static void inject_preempt_context(struct intel_engine_cs *engine) if (execlists->ctrl_reg) writel(EL_CTRL_LOAD, execlists->ctrl_reg); - execlists_clear_active(&engine->execlists, EXECLISTS_ACTIVE_HWACK); - execlists_set_active(&engine->execlists, EXECLISTS_ACTIVE_PREEMPT); + execlists_clear_active(execlists, EXECLISTS_ACTIVE_HWACK); + execlists_set_active(execlists, EXECLISTS_ACTIVE_PREEMPT); +} + +static void complete_preempt_context(struct intel_engine_execlists *execlists) +{ + GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT)); + + execlists_cancel_port_requests(execlists); + execlists_unwind_incomplete_requests(execlists); + + execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT); } static void execlists_dequeue(struct intel_engine_cs *engine) @@ -995,14 +1005,7 @@ static void execlists_submission_tasklet(unsigned long data) if (status & GEN8_CTX_STATUS_COMPLETE && buf[2*head + 1] == execlists->preempt_complete_status) { GEM_TRACE("%s preempt-idle\n", engine->name); - - execlists_cancel_port_requests(execlists); - execlists_unwind_incomplete_requests(execlists); - - GEM_BUG_ON(!execlists_is_active(execlists, - EXECLISTS_ACTIVE_PREEMPT)); - execlists_clear_active(execlists, - EXECLISTS_ACTIVE_PREEMPT); + complete_preempt_context(execlists); continue; } -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/18] drm/i915: Move engine reset prepare/finish to backends
In preparation to more carefully handling incomplete preemption during reset by execlists, we move the existing code wholesale to the backends under a couple of new reset vfuncs. Signed-off-by: Chris Wilson Cc: Michał Winiarski CC: Michel Thierry Cc: Jeff McGee Reviewed-by: Jeff McGee --- drivers/gpu/drm/i915/i915_gem.c | 47 +++- drivers/gpu/drm/i915/intel_lrc.c| 59 +++-- drivers/gpu/drm/i915/intel_ringbuffer.c | 23 -- drivers/gpu/drm/i915/intel_ringbuffer.h | 9 +++- 4 files changed, 88 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 28ab0beff86c..61aca602c3a9 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2997,7 +2997,7 @@ i915_gem_find_active_request(struct intel_engine_cs *engine) struct i915_request * i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) { - struct i915_request *request = NULL; + struct i915_request *request; /* * During the reset sequence, we must prevent the engine from @@ -3020,40 +3020,7 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) */ kthread_park(engine->breadcrumbs.signaler); - /* -* Prevent request submission to the hardware until we have -* completed the reset in i915_gem_reset_finish(). If a request -* is completed by one engine, it may then queue a request -* to a second via its execlists->tasklet *just* as we are -* calling engine->init_hw() and also writing the ELSP. -* Turning off the execlists->tasklet until the reset is over -* prevents the race. -* -* Note that this needs to be a single atomic operation on the -* tasklet (flush existing tasks, prevent new tasks) to prevent -* a race between reset and set-wedged. It is not, so we do the best -* we can atm and make sure we don't lock the machine up in the more -* common case of recursively being called from set-wedged from inside -* i915_reset. -*/ - if (!atomic_read(&engine->execlists.tasklet.count)) - tasklet_kill(&engine->execlists.tasklet); - tasklet_disable(&engine->execlists.tasklet); - - /* -* We're using worker to queue preemption requests from the tasklet in -* GuC submission mode. -* Even though tasklet was disabled, we may still have a worker queued. -* Let's make sure that all workers scheduled before disabling the -* tasklet are completed before continuing with the reset. -*/ - if (engine->i915->guc.preempt_wq) - flush_workqueue(engine->i915->guc.preempt_wq); - - if (engine->irq_seqno_barrier) - engine->irq_seqno_barrier(engine); - - request = i915_gem_find_active_request(engine); + request = engine->reset.prepare(engine); if (request && request->fence.error == -EIO) request = ERR_PTR(-EIO); /* Previous reset failed! */ @@ -3204,13 +3171,8 @@ void i915_gem_reset_engine(struct intel_engine_cs *engine, if (request) request = i915_gem_reset_request(engine, request, stalled); - if (request) { - DRM_DEBUG_DRIVER("resetting %s to restart from tail of request 0x%x\n", -engine->name, request->global_seqno); - } - /* Setup the CS to resume from the breadcrumb of the hung request */ - engine->reset_hw(engine, request); + engine->reset.reset(engine, request); } void i915_gem_reset(struct drm_i915_private *dev_priv, @@ -3265,7 +3227,8 @@ void i915_gem_reset(struct drm_i915_private *dev_priv, void i915_gem_reset_finish_engine(struct intel_engine_cs *engine) { - tasklet_enable(&engine->execlists.tasklet); + engine->reset.finish(engine); + kthread_unpark(engine->breadcrumbs.signaler); intel_uncore_forcewake_put(engine->i915, FORCEWAKE_ALL); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 5773a0e8dc51..0fe872f8b6a3 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1741,8 +1741,48 @@ static int gen9_init_render_ring(struct intel_engine_cs *engine) return init_workarounds_ring(engine); } -static void reset_common_ring(struct intel_engine_cs *engine, - struct i915_request *request) +static struct i915_request * +execlists_reset_prepare(struct intel_engine_cs *engine) +{ + struct intel_engine_execlists * const execlists = &engine->execlists; + + GEM_TRACE("%s\n", engine->name); + + /* +* Prevent request submission to the hardware until we have +* completed the reset in i915_gem_reset_finish(). If a request +* is completed by one engine, it may then queue a request +* to a second via it
[Intel-gfx] [PATCH 04/18] drm/i915: Split execlists/guc reset preparations
In the next patch, we will make the execlists reset prepare callback take into account preemption by flushing the context-switch handler. This is not applicable to the GuC submission backend, so split the two into their own backend callbacks. Signed-off-by: Chris Wilson Cc: Michał Winiarski CC: Michel Thierry Cc: Jeff McGee Reviewed-by: Jeff McGee --- drivers/gpu/drm/i915/intel_guc_submission.c | 41 + drivers/gpu/drm/i915/intel_lrc.c| 11 +- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 04125e8cec6b..dc6782391f9f 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -797,6 +797,46 @@ static void guc_submission_tasklet(unsigned long data) guc_dequeue(engine); } +static struct i915_request * +guc_reset_prepare(struct intel_engine_cs *engine) +{ + struct intel_engine_execlists * const execlists = &engine->execlists; + + GEM_TRACE("%s\n", engine->name); + + /* +* Prevent request submission to the hardware until we have +* completed the reset in i915_gem_reset_finish(). If a request +* is completed by one engine, it may then queue a request +* to a second via its execlists->tasklet *just* as we are +* calling engine->init_hw() and also writing the ELSP. +* Turning off the execlists->tasklet until the reset is over +* prevents the race. +* +* Note that this needs to be a single atomic operation on the +* tasklet (flush existing tasks, prevent new tasks) to prevent +* a race between reset and set-wedged. It is not, so we do the best +* we can atm and make sure we don't lock the machine up in the more +* common case of recursively being called from set-wedged from inside +* i915_reset. +*/ + if (!atomic_read(&execlists->tasklet.count)) + tasklet_kill(&execlists->tasklet); + tasklet_disable(&execlists->tasklet); + + /* +* We're using worker to queue preemption requests from the tasklet in +* GuC submission mode. +* Even though tasklet was disabled, we may still have a worker queued. +* Let's make sure that all workers scheduled before disabling the +* tasklet are completed before continuing with the reset. +*/ + if (engine->i915->guc.preempt_wq) + flush_workqueue(engine->i915->guc.preempt_wq); + + return i915_gem_find_active_request(engine); +} + /* * Everything below here is concerned with setup & teardown, and is * therefore not part of the somewhat time-critical batch-submission @@ -1256,6 +1296,7 @@ int intel_guc_submission_enable(struct intel_guc *guc) &engine->execlists; execlists->tasklet.func = guc_submission_tasklet; + engine->reset.prepare = guc_reset_prepare; engine->park = guc_submission_park; engine->unpark = guc_submission_unpark; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 0fe872f8b6a3..12f5abbadd3c 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1768,16 +1768,6 @@ execlists_reset_prepare(struct intel_engine_cs *engine) tasklet_kill(&execlists->tasklet); tasklet_disable(&execlists->tasklet); - /* -* We're using worker to queue preemption requests from the tasklet in -* GuC submission mode. -* Even though tasklet was disabled, we may still have a worker queued. -* Let's make sure that all workers scheduled before disabling the -* tasklet are completed before continuing with the reset. -*/ - if (engine->i915->guc.preempt_wq) - flush_workqueue(engine->i915->guc.preempt_wq); - return i915_gem_find_active_request(engine); } @@ -2167,6 +2157,7 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine) engine->cancel_requests = execlists_cancel_requests; engine->schedule = execlists_schedule; engine->execlists.tasklet.func = execlists_submission_tasklet; + engine->reset.prepare = execlists_reset_prepare; engine->park = NULL; engine->unpark = NULL; -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/18] drm/i915/guc: Make submission tasklet hardirq safe
Prepare to allow the GuC submission to be run from underneath a hardirq timer context (and not just the current softirq context) as is required for fast preemption resets. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_guc_submission.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index dc6782391f9f..994082712181 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -689,10 +689,11 @@ static void guc_dequeue(struct intel_engine_cs *engine) struct i915_request *last = NULL; const struct execlist_port * const last_port = &execlists->port[execlists->port_mask]; + unsigned long flags; bool submit = false; struct rb_node *rb; - spin_lock_irq(&engine->timeline->lock); + spin_lock_irqsave(&engine->timeline->lock, flags); rb = execlists->first; GEM_BUG_ON(rb_first(&execlists->queue) != rb); @@ -763,7 +764,7 @@ static void guc_dequeue(struct intel_engine_cs *engine) GEM_BUG_ON(execlists->first && !port_isset(execlists->port)); unlock: - spin_unlock_irq(&engine->timeline->lock); + spin_unlock_irqrestore(&engine->timeline->lock, flags); } static void guc_submission_tasklet(unsigned long data) -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/18] drm/i915: Combine tasklet_kill and tasklet_disable
Ideally, we want to atomically flush and disable the tasklet before resetting the GPU. At present, we rely on being the only part to touch our tasklet and serialisation of the reset process to ensure that we can suspend the tasklet from the mix of reset/wedge pathways. In this patch, we move the tasklet abuse into its own function and tweak it such that we only do a synchronous operation the first time it is disabled around the reset. This allows us to avoid the sync inside a softirq context in subsequent patches. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Michał Winiarski CC: Michel Thierry Cc: Jeff McGee --- drivers/gpu/drm/i915/intel_lrc.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index bbcc6439a2a1..d5640f3d5276 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1754,6 +1754,16 @@ static int gen9_init_render_ring(struct intel_engine_cs *engine) return init_workarounds_ring(engine); } +static void tasklet_kill_and_disable(struct tasklet_struct *t) +{ + if (!atomic_read(&t->count)) + tasklet_kill(t); + + if (atomic_inc_return(&t->count) == 1) + tasklet_unlock_wait(t); + smp_mb(); +} + static struct i915_request * execlists_reset_prepare(struct intel_engine_cs *engine) { @@ -1778,9 +1788,7 @@ execlists_reset_prepare(struct intel_engine_cs *engine) * common case of recursively being called from set-wedged from inside * i915_reset. */ - if (!atomic_read(&execlists->tasklet.count)) - tasklet_kill(&execlists->tasklet); - tasklet_disable(&execlists->tasklet); + tasklet_kill_and_disable(&execlists->tasklet); /* * We want to flush the pending context switches, having disabled -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/18] drm/i915/execlists: Flush pending preemption events during reset
Catch up with the inflight CSB events, after disabling the tasklet before deciding which request was truly guilty of hanging the GPU. v2: Restore checking of use_csb_mmio on every loop, don't forget old vgpu. Signed-off-by: Chris Wilson Cc: Michał Winiarski CC: Michel Thierry Cc: Jeff McGee --- drivers/gpu/drm/i915/intel_lrc.c | 127 +-- 1 file changed, 87 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 12f5abbadd3c..bbcc6439a2a1 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -890,34 +890,14 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine) local_irq_restore(flags); } -/* - * Check the unread Context Status Buffers and manage the submission of new - * contexts to the ELSP accordingly. - */ -static void execlists_submission_tasklet(unsigned long data) +static void process_csb(struct intel_engine_cs *engine) { - struct intel_engine_cs * const engine = (struct intel_engine_cs *)data; struct intel_engine_execlists * const execlists = &engine->execlists; struct execlist_port *port = execlists->port; - struct drm_i915_private *dev_priv = engine->i915; + struct drm_i915_private *i915 = engine->i915; bool fw = false; - /* -* We can skip acquiring intel_runtime_pm_get() here as it was taken -* on our behalf by the request (see i915_gem_mark_busy()) and it will -* not be relinquished until the device is idle (see -* i915_gem_idle_work_handler()). As a precaution, we make sure -* that all ELSP are drained i.e. we have processed the CSB, -* before allowing ourselves to idle and calling intel_runtime_pm_put(). -*/ - GEM_BUG_ON(!dev_priv->gt.awake); - - /* -* Prefer doing test_and_clear_bit() as a two stage operation to avoid -* imposing the cost of a locked atomic transaction when submitting a -* new request (outside of the context-switch interrupt). -*/ - while (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) { + do { /* The HWSP contains a (cacheable) mirror of the CSB */ const u32 *buf = &engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX]; @@ -925,28 +905,27 @@ static void execlists_submission_tasklet(unsigned long data) if (unlikely(execlists->csb_use_mmio)) { buf = (u32 * __force) - (dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0))); - execlists->csb_head = -1; /* force mmio read of CSB ptrs */ + (i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0))); + execlists->csb_head = -1; /* force mmio read of CSB */ } /* Clear before reading to catch new interrupts */ clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted); smp_mb__after_atomic(); - if (unlikely(execlists->csb_head == -1)) { /* following a reset */ + if (unlikely(execlists->csb_head == -1)) { /* after a reset */ if (!fw) { - intel_uncore_forcewake_get(dev_priv, - execlists->fw_domains); + intel_uncore_forcewake_get(i915, execlists->fw_domains); fw = true; } - head = readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine))); + head = readl(i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine))); tail = GEN8_CSB_WRITE_PTR(head); head = GEN8_CSB_READ_PTR(head); execlists->csb_head = head; } else { const int write_idx = - intel_hws_csb_write_index(dev_priv) - + intel_hws_csb_write_index(i915) - I915_HWS_CSB_BUF0_INDEX; head = execlists->csb_head; @@ -954,8 +933,8 @@ static void execlists_submission_tasklet(unsigned long data) } GEM_TRACE("%s cs-irq head=%d [%d%s], tail=%d [%d%s]\n", engine->name, - head, GEN8_CSB_READ_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine, fw ? "" : "?", - tail, GEN8_CSB_WRITE_PTR(readl(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine, fw ? "" : "?"); + head, GEN8_CSB_READ_PTR(readl(i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine, fw ? ""
[Intel-gfx] [PATCH 08/18] drm/i915: Stop parking the signaler around reset
We cannot call kthread_park() from softirq context, so let's avoid it entirely during the reset. We wanted to suspend the signaler so that it would not mark a request as complete at the same time as we marked it as being in error. Instead of parking the signaling, stop the engine from advancing so that the GPU doesn't emit the breadcrumb for our chosen "guilty" request. Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Michał Winiarski CC: Michel Thierry Cc: Jeff McGee Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem.c | 14 -- drivers/gpu/drm/i915/intel_lrc.c| 21 + drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++ 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 61aca602c3a9..0a3f6a41525b 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3008,18 +3008,6 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine) */ intel_uncore_forcewake_get(engine->i915, FORCEWAKE_ALL); - /* -* Prevent the signaler thread from updating the request -* state (by calling dma_fence_signal) as we are processing -* the reset. The write from the GPU of the seqno is -* asynchronous and the signaler thread may see a different -* value to us and declare the request complete, even though -* the reset routine have picked that request as the active -* (incomplete) request. This conflict is not handled -* gracefully! -*/ - kthread_park(engine->breadcrumbs.signaler); - request = engine->reset.prepare(engine); if (request && request->fence.error == -EIO) request = ERR_PTR(-EIO); /* Previous reset failed! */ @@ -3229,8 +3217,6 @@ void i915_gem_reset_finish_engine(struct intel_engine_cs *engine) { engine->reset.finish(engine); - kthread_unpark(engine->breadcrumbs.signaler); - intel_uncore_forcewake_put(engine->i915, FORCEWAKE_ALL); } diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index d5640f3d5276..bfafa2e4bbfc 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1764,6 +1764,21 @@ static void tasklet_kill_and_disable(struct tasklet_struct *t) smp_mb(); } +static void set_stop_engine(struct intel_engine_cs *engine) +{ + struct drm_i915_private *dev_priv = engine->i915; + const u32 base = engine->mmio_base; + const i915_reg_t mode = RING_MI_MODE(base); + + GEM_TRACE("%s\n", engine->name); + I915_WRITE_FW(mode, _MASKED_BIT_ENABLE(STOP_RING)); + if (__intel_wait_for_register_fw(dev_priv, +mode, MODE_IDLE, MODE_IDLE, +1000, 0, +NULL)) + GEM_TRACE("%s: timed out on STOP_RING -> IDLE\n", engine->name); +} + static struct i915_request * execlists_reset_prepare(struct intel_engine_cs *engine) { @@ -1810,6 +1825,12 @@ execlists_reset_prepare(struct intel_engine_cs *engine) if (request) { unsigned long flags; + /* +* Prevent the breadcrumb from advancing before we decide +* which request is currently active. +*/ + set_stop_engine(engine); + spin_lock_irqsave(&engine->timeline->lock, flags); list_for_each_entry_from_reverse(request, &engine->timeline->requests, diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 5dadbc435c0e..226c00aecd8a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -530,8 +530,26 @@ static int init_ring_common(struct intel_engine_cs *engine) return ret; } +static void set_stop_engine(struct intel_engine_cs *engine) +{ + struct drm_i915_private *dev_priv = engine->i915; + const u32 base = engine->mmio_base; + const i915_reg_t mode = RING_MI_MODE(base); + + I915_WRITE_FW(mode, _MASKED_BIT_ENABLE(STOP_RING)); + if (__intel_wait_for_register_fw(dev_priv, +mode, MODE_IDLE, MODE_IDLE, +1000, 0, +NULL)) + DRM_DEBUG_DRIVER("%s: timed out on STOP_RING\n", +engine->name); +} + static struct i915_request *reset_prepare(struct intel_engine_cs *engine) { + if (INTEL_GEN(engine->i915) >= 3) + set_stop_engine(engine); + if (engine->irq_seqno_barrier) engine->irq_seqno_barrier(engine); -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org
[Intel-gfx] [PATCH 09/18] drm/i915: Be irqsafe inside reset
As we want to be able to call i915_reset_engine and co from a softirq or timer context, we need to be irqsafe at all timers. So we have to forgo the simple spin_lock_irq for the full spin_lock_irqsave. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 0a3f6a41525b..42a387ff0eaa 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3130,15 +3130,17 @@ i915_gem_reset_request(struct intel_engine_cs *engine, */ request = i915_gem_find_active_request(engine); if (request) { + unsigned long flags; + i915_gem_context_mark_innocent(request->ctx); dma_fence_set_error(&request->fence, -EAGAIN); /* Rewind the engine to replay the incomplete rq */ - spin_lock_irq(&engine->timeline->lock); + spin_lock_irqsave(&engine->timeline->lock, flags); request = list_prev_entry(request, link); if (&request->link == &engine->timeline->requests) request = NULL; - spin_unlock_irq(&engine->timeline->lock); + spin_unlock_irqrestore(&engine->timeline->lock, flags); } } -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/18] drm/i915/breadcrumbs: Keep the fake irq armed across reset
Instead of synchronously cancelling the timer and re-enabling it inside the reset callbacks, keep the timer enabled and let it die on its next wakeup if no longer required. This allows intel_engine_reset_breadcrumbs() to be used from an atomic (timer/softirq) context such as required for resetting an engine. It also allows us to react better to the user poking around debugfs for testing missed irqs. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Mika Kuoppala --- drivers/gpu/drm/i915/intel_breadcrumbs.c | 27 +--- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index 671a6d61e29d..0a2128c6b418 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -130,11 +130,12 @@ static void intel_breadcrumbs_hangcheck(struct timer_list *t) static void intel_breadcrumbs_fake_irq(struct timer_list *t) { - struct intel_engine_cs *engine = from_timer(engine, t, - breadcrumbs.fake_irq); + struct intel_engine_cs *engine = + from_timer(engine, t, breadcrumbs.fake_irq); struct intel_breadcrumbs *b = &engine->breadcrumbs; - /* The timer persists in case we cannot enable interrupts, + /* +* The timer persists in case we cannot enable interrupts, * or if we have previously seen seqno/interrupt incoherency * ("missed interrupt" syndrome, better known as a "missed breadcrumb"). * Here the worker will wake up every jiffie in order to kick the @@ -148,6 +149,12 @@ static void intel_breadcrumbs_fake_irq(struct timer_list *t) if (!b->irq_armed) return; + /* If the user has disabled the fake-irq, restore the hangchecking */ + if (!test_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings)) { + mod_timer(&b->hangcheck, wait_timeout()); + return; + } + mod_timer(&b->fake_irq, jiffies + 1); } @@ -840,15 +847,22 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) { struct intel_breadcrumbs *b = &engine->breadcrumbs; - cancel_fake_irq(engine); spin_lock_irq(&b->irq_lock); + /* +* Leave the fake_irq timer enabled (if it is running), but clear the +* bit so that it turns itself off on its next wake up and goes back +* to the long hangcheck interval if still required. +*/ + clear_bit(engine->id, &engine->i915->gpu_error.missed_irq_rings); + if (b->irq_enabled) irq_enable(engine); else irq_disable(engine); - /* We set the IRQ_BREADCRUMB bit when we enable the irq presuming the + /* +* We set the IRQ_BREADCRUMB bit when we enable the irq presuming the * GPU is active and may have already executed the MI_USER_INTERRUPT * before the CPU is ready to receive. However, the engine is currently * idle (we haven't started it yet), there is no possibility for a @@ -857,9 +871,6 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) */ clear_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted); - if (b->irq_armed) - enable_fake_irq(b); - spin_unlock_irq(&b->irq_lock); } -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 14/18] drm/i915/execlists: Force preemption via reset on timeout
Install a timer when trying to preempt on behalf of an important context such that if the active context does not honour the preemption request within the desired timeout, then we reset the GPU to allow the important context to run. v2: Install the timer on scheduling the preempt request; long before we even try to inject preemption into the ELSP, as the tasklet/injection may itself be blocked. v3: Update the guc to handle the preemption/tasklet timer. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_guc_submission.c | 1 + drivers/gpu/drm/i915/intel_lrc.c| 88 +++-- drivers/gpu/drm/i915/intel_ringbuffer.h | 8 +- drivers/gpu/drm/i915/selftests/intel_lrc.c | 60 ++ 4 files changed, 148 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 994082712181..5577d6f717e3 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -750,6 +750,7 @@ static void guc_dequeue(struct intel_engine_cs *engine) kmem_cache_free(engine->i915->priorities, p); } done: + execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT_TIMEOUT); execlists->queue_priority = rb ? to_priolist(rb)->priority : INT_MIN; execlists->first = rb; if (submit) { diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 73fb941a675e..ca1c54af2877 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -550,6 +550,52 @@ static void inject_preempt_context(struct intel_engine_cs *engine) execlists_set_active(execlists, EXECLISTS_ACTIVE_PREEMPT); } +static enum hrtimer_restart preempt_timeout(struct hrtimer *hrtimer) +{ + struct intel_engine_execlists *execlists = + container_of(hrtimer, typeof(*execlists), preempt_timer); + + GEM_TRACE("%s\n", + container_of(execlists, + struct intel_engine_cs, + execlists)->name); + + if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT_TIMEOUT)) + return HRTIMER_NORESTART; + + if (GEM_SHOW_DEBUG()) { + struct intel_engine_cs *engine = + container_of(execlists, typeof(*engine), execlists); + struct drm_printer p = drm_debug_printer(__func__); + + intel_engine_dump( engine, &p, "%s\n", engine->name); + } + + queue_work(system_highpri_wq, &execlists->preempt_reset); + + return HRTIMER_NORESTART; +} + +static void preempt_reset(struct work_struct *work) +{ + struct intel_engine_execlists *execlists = + container_of(work, typeof(*execlists), preempt_reset); + struct intel_engine_cs *engine = + container_of(execlists, struct intel_engine_cs, execlists); + + GEM_TRACE("%s\n", engine->name); + + tasklet_disable(&execlists->tasklet); + + execlists->tasklet.func(execlists->tasklet.data); + + if (execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT_TIMEOUT)) + i915_handle_error(engine->i915, BIT(engine->id), 0, + "preemption time out on %s", engine->name); + + tasklet_enable(&execlists->tasklet); +} + static void complete_preempt_context(struct intel_engine_execlists *execlists) { GEM_BUG_ON(!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT)); @@ -724,6 +770,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) kmem_cache_free(engine->i915->priorities, p); } done: + execlists_clear_active(execlists, EXECLISTS_ACTIVE_PREEMPT_TIMEOUT); execlists->queue_priority = port != execlists->port ? rq_prio(last) : INT_MIN; execlists->first = rb; @@ -1109,16 +1156,38 @@ static void queue_request(struct intel_engine_cs *engine, list_add_tail(&pt->link, &lookup_priolist(engine, pt, prio)->requests); } -static void __submit_queue(struct intel_engine_cs *engine, int prio) +static void __submit_queue(struct intel_engine_cs *engine, + int prio, unsigned int timeout) { - engine->execlists.queue_priority = prio; - tasklet_hi_schedule(&engine->execlists.tasklet); + struct intel_engine_execlists * const execlists = &engine->execlists; + int old = execlists->queue_priority; + + GEM_TRACE("%s prio=%d (previous=%d)\n", engine->name, prio, old); + + if (unlikely(execlists_is_active(execlists, +EXECLISTS_ACTIVE_PREEMPT_TIMEOUT))) + hrtimer_cancel(&execlists->preempt_timer); + + execlists->queue_priority = prio; + tasklet_hi_schedule(&execlists->tasklet); + + /* Set a timer to force preemption vs hostile userspace */ + if (timeout && __execlists_
[Intel-gfx] [PATCH 15/18] drm/i915/execlists: Try preempt-reset from hardirq timer context
When circumstances allow, trying resetting the engine directly from the preemption timeout handler. As this is softirq context, we have to be careful both not to sleep and not to spin on anything we may be interrupting (e.g. the submission tasklet). Signed-off-by: Chris Wilson Cc: Mika Kuoppala Cc: Michał Winiarski CC: Michel Thierry Cc: Jeff McGee Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/intel_lrc.c | 35 +- drivers/gpu/drm/i915/selftests/intel_lrc.c | 123 + 2 files changed, 157 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index ca1c54af2877..4f1e985b3cdb 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -550,6 +550,38 @@ static void inject_preempt_context(struct intel_engine_cs *engine) execlists_set_active(execlists, EXECLISTS_ACTIVE_PREEMPT); } +static int try_preempt_reset(struct intel_engine_execlists *execlists) +{ + int err = -EBUSY; + + if (tasklet_trylock(&execlists->tasklet)) { + struct intel_engine_cs *engine = + container_of(execlists, typeof(*engine), execlists); + const unsigned int bit = I915_RESET_ENGINE + engine->id; + unsigned long *lock = &engine->i915->gpu_error.flags; + + execlists->tasklet.func(execlists->tasklet.data); + + if (!execlists_is_active(execlists, +EXECLISTS_ACTIVE_PREEMPT_TIMEOUT)) { + /* Nothing to do; the tasklet was just delayed. */ + err = 0; + } else if (!test_and_set_bit(bit, lock)) { + tasklet_disable_nosync(&execlists->tasklet); + err = i915_reset_engine(engine, + "preemption time out"); + tasklet_enable(&execlists->tasklet); + + clear_bit(bit, lock); + wake_up_bit(lock, bit); + } + + tasklet_unlock(&execlists->tasklet); + } + + return err; +} + static enum hrtimer_restart preempt_timeout(struct hrtimer *hrtimer) { struct intel_engine_execlists *execlists = @@ -571,7 +603,8 @@ static enum hrtimer_restart preempt_timeout(struct hrtimer *hrtimer) intel_engine_dump( engine, &p, "%s\n", engine->name); } - queue_work(system_highpri_wq, &execlists->preempt_reset); + if (try_preempt_reset(execlists)) + queue_work(system_highpri_wq, &execlists->preempt_reset); return HRTIMER_NORESTART; } diff --git a/drivers/gpu/drm/i915/selftests/intel_lrc.c b/drivers/gpu/drm/i915/selftests/intel_lrc.c index aed2502419ee..72d2770e5d71 100644 --- a/drivers/gpu/drm/i915/selftests/intel_lrc.c +++ b/drivers/gpu/drm/i915/selftests/intel_lrc.c @@ -555,6 +555,128 @@ static int live_preempt_timeout(void *arg) return err; } +static void __softirq_begin(void) +{ + local_bh_disable(); +} + +static void __softirq_end(void) +{ + local_bh_enable(); +} + +static void __hardirq_begin(void) +{ + local_irq_disable(); +} + +static void __hardirq_end(void) +{ + local_irq_enable(); +} + +static int live_preempt_reset(void *arg) +{ + struct drm_i915_private *i915 = arg; + struct intel_engine_cs *engine; + struct i915_gem_context *ctx; + enum intel_engine_id id; + struct spinner spin; + int err = -ENOMEM; + + if (!HAS_LOGICAL_RING_PREEMPTION(i915)) + return 0; + + mutex_lock(&i915->drm.struct_mutex); + + if (spinner_init(&spin, i915)) + goto err_unlock; + + ctx = kernel_context(i915); + if (!ctx) + goto err_spin; + + for_each_engine(engine, i915, id) { + static const struct { + const char *name; + void (*critical_section_begin)(void); + void (*critical_section_end)(void); + } phases[] = { + { "softirq", __softirq_begin, __softirq_end }, + { "hardirq", __hardirq_begin, __hardirq_end }, + { } + }; + const typeof(*phases) *p; + + for (p = phases; p->name; p++) { + struct i915_request *rq; + + rq = spinner_create_request(&spin, ctx, engine, + MI_NOOP); + if (IS_ERR(rq)) { + err = PTR_ERR(rq); + goto err_ctx; + } + + i915_request_add(rq); + if (!wait_for_spinner(&spin, rq)) { + i915_gem_set_wedged(i915); + err = -EIO; +
[Intel-gfx] [PATCH 10/18] drm/i915/execlists: Make submission tasklet hardirq safe
Prepare to allow the execlists submission to be run from underneath a hardirq timer context (and not just the current softirq context) as is required for fast preemption resets. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_lrc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index bfafa2e4bbfc..73fb941a675e 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -568,6 +568,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) &execlists->port[execlists->port_mask]; struct i915_request *last = port_request(port); struct rb_node *rb; + unsigned long flags; bool submit = false; /* Hardware submission is through 2 ports. Conceptually each port @@ -591,7 +592,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) * and context switches) submission. */ - spin_lock_irq(&engine->timeline->lock); + spin_lock_irqsave(&engine->timeline->lock, flags); rb = execlists->first; GEM_BUG_ON(rb_first(&execlists->queue) != rb); @@ -733,7 +734,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine) GEM_BUG_ON(execlists->first && !port_isset(execlists->port)); unlock: - spin_unlock_irq(&engine->timeline->lock); + spin_unlock_irqrestore(&engine->timeline->lock, flags); if (submit) { execlists_user_begin(execlists, execlists->port); -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/18] drm/i915: Allow init_breadcrumbs to be used from irq context
In order to support engine reset from irq (timer) context, we need to be able to re-initialise the breadcrumbs. So we need to promote the plain spin_lock_irq to a safe spin_lock_irqsave. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_breadcrumbs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index 0a2128c6b418..ca0b04d9747c 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -846,8 +846,9 @@ static void cancel_fake_irq(struct intel_engine_cs *engine) void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) { struct intel_breadcrumbs *b = &engine->breadcrumbs; + unsigned long flags; - spin_lock_irq(&b->irq_lock); + spin_lock_irqsave(&b->irq_lock, flags); /* * Leave the fake_irq timer enabled (if it is running), but clear the @@ -871,7 +872,7 @@ void intel_engine_reset_breadcrumbs(struct intel_engine_cs *engine) */ clear_bit(ENGINE_IRQ_BREADCRUMB, &engine->irq_posted); - spin_unlock_irq(&b->irq_lock); + spin_unlock_irqrestore(&b->irq_lock, flags); } void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine) -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 18/18] drm/i915: Allow user control over preempt timeout on their important context
One usecase would be to couple in via EGL_NV_context_priority_realtime in userspace to provide some QoS guarantees in conjunction with setting the highest priority. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem_context.c| 22 ++ drivers/gpu/drm/i915/i915_gem_context.h| 13 drivers/gpu/drm/i915/i915_request.c| 8 +- drivers/gpu/drm/i915/intel_lrc.c | 2 +- drivers/gpu/drm/i915/selftests/intel_lrc.c | 85 ++ include/uapi/drm/i915_drm.h| 12 +++ 6 files changed, 139 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 5cfac0255758..932ca1082b26 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -749,6 +749,15 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data, case I915_CONTEXT_PARAM_PRIORITY: args->value = ctx->priority; break; + case I915_CONTEXT_PARAM_PREEMPT_TIMEOUT: + if (!(to_i915(dev)->caps.scheduler & I915_SCHEDULER_CAP_PREEMPTION)) + ret = -ENODEV; + else if (args->size) + ret = -EINVAL; + else + args->value = ctx->preempt_timeout; + break; + default: ret = -EINVAL; break; @@ -824,6 +833,19 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data, } break; + case I915_CONTEXT_PARAM_PREEMPT_TIMEOUT: + if (args->size) + ret = -EINVAL; + else if (args->value > U32_MAX) + ret = -EINVAL; + else if (!(to_i915(dev)->caps.scheduler & I915_SCHEDULER_CAP_PREEMPTION)) + ret = -ENODEV; + else if (args->value && !capable(CAP_SYS_ADMIN)) + ret = -EPERM; + else + ctx->preempt_timeout = args->value; + break; + default: ret = -EINVAL; break; diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h index 7854262ddfd9..1fc7181edd2d 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -150,6 +150,19 @@ struct i915_gem_context { */ int priority; + /** +* @preempt_timeout: QoS guarantee for the high priority context +* +* Some clients need a guarantee that they will start executing +* within a certain window, even at the expense of others. This entails +* that if a preemption request is not honoured by the active context +* within the timeout, we will reset the GPU to evict the hog and +* run the high priority context instead. +* +* Timeout is stored in nanoseconds. +*/ + u32 preempt_timeout; + /** ggtt_offset_bias: placement restriction for context objects */ u32 ggtt_offset_bias; diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index ddb829ef..b9dfd49a173e 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1061,8 +1061,12 @@ void __i915_request_add(struct i915_request *request, bool flush_caches) * run at the earliest possible convenience. */ rcu_read_lock(); - if (engine->schedule) - engine->schedule(request, request->ctx->priority, 0); + if (engine->schedule) { + unsigned int timeout = request->ctx->preempt_timeout; + int priority = request->ctx->priority; + + engine->schedule(request, priority, timeout); + } rcu_read_unlock(); local_bh_disable(); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 877340afb63d..18cc83520e95 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1232,7 +1232,7 @@ static void execlists_submit_request(struct i915_request *request) spin_lock_irqsave(&engine->timeline->lock, flags); queue_request(engine, &request->priotree, rq_prio(request)); - submit_queue(engine, rq_prio(request), 0); + submit_queue(engine, rq_prio(request), request->ctx->preempt_timeout); GEM_BUG_ON(!engine->execlists.first); GEM_BUG_ON(list_empty(&request->priotree.link)); diff --git a/drivers/gpu/drm/i915/selftests/intel_lrc.c b/drivers/gpu/drm/i915/selftests/intel_lrc.c index 5e4d6d07fff5..4fac9c552595 100644 --- a/drivers/gpu/drm/i915/selftests/intel_lrc.c +++ b/drivers/gpu/drm/i915/selftests/intel_lrc.c @@ -780,6 +780,90 @@ static int live_late_preempt_timeout(void *arg) goto err_ctx_lo; } +static int live_context_preempt_timeout(void *a
[Intel-gfx] [PATCH 17/18] drm/i915: Use a preemption timeout to enforce interactivity
Use a liberal timeout of 20ms to ensure that the rendering for an interactive pageflip is started in a timely fashion, and that user interaction is not blocked by GPU, or CPU, hogs. This is at the cost of resetting whoever was blocking the preemption, likely leading to that context/process being banned from submitting future requests. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.h | 3 ++- drivers/gpu/drm/i915/i915_gem.c | 18 ++ drivers/gpu/drm/i915/intel_display.c | 18 +- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9bca104c409e..732d375ab330 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3155,8 +3155,9 @@ int i915_gem_object_wait(struct drm_i915_gem_object *obj, struct intel_rps_client *rps); int i915_gem_object_wait_priority(struct drm_i915_gem_object *obj, unsigned int flags, - int priority); + int priority, unsigned int timeout); #define I915_PRIORITY_DISPLAY I915_PRIORITY_MAX +#define I915_PREEMPTION_TIMEOUT_DISPLAY (100 * 1000 * 1000) /* 100 ms / 10Hz */ int __must_check i915_gem_object_set_to_wc_domain(struct drm_i915_gem_object *obj, bool write); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index e8c1a077e223..38d8f6aebfbb 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -563,7 +563,8 @@ i915_gem_object_wait_reservation(struct reservation_object *resv, return timeout; } -static void __fence_set_priority(struct dma_fence *fence, int prio) +static void __fence_set_priority(struct dma_fence *fence, +int prio, unsigned int timeout) { struct i915_request *rq; struct intel_engine_cs *engine; @@ -576,11 +577,12 @@ static void __fence_set_priority(struct dma_fence *fence, int prio) rcu_read_lock(); if (engine->schedule) - engine->schedule(rq, prio, 0); + engine->schedule(rq, prio, timeout); rcu_read_unlock(); } -static void fence_set_priority(struct dma_fence *fence, int prio) +static void fence_set_priority(struct dma_fence *fence, + int prio, unsigned int timeout) { /* Recurse once into a fence-array */ if (dma_fence_is_array(fence)) { @@ -588,16 +590,16 @@ static void fence_set_priority(struct dma_fence *fence, int prio) int i; for (i = 0; i < array->num_fences; i++) - __fence_set_priority(array->fences[i], prio); + __fence_set_priority(array->fences[i], prio, timeout); } else { - __fence_set_priority(fence, prio); + __fence_set_priority(fence, prio, timeout); } } int i915_gem_object_wait_priority(struct drm_i915_gem_object *obj, unsigned int flags, - int prio) + int prio, unsigned int timeout) { struct dma_fence *excl; @@ -612,7 +614,7 @@ i915_gem_object_wait_priority(struct drm_i915_gem_object *obj, return ret; for (i = 0; i < count; i++) { - fence_set_priority(shared[i], prio); + fence_set_priority(shared[i], prio, timeout); dma_fence_put(shared[i]); } @@ -622,7 +624,7 @@ i915_gem_object_wait_priority(struct drm_i915_gem_object *obj, } if (excl) { - fence_set_priority(excl, prio); + fence_set_priority(excl, prio, timeout); dma_fence_put(excl); } return 0; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9c6156216e5e..ba850b59530f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12785,7 +12785,23 @@ intel_prepare_plane_fb(struct drm_plane *plane, ret = intel_plane_pin_fb(to_intel_plane_state(new_state)); - i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY); + /* +* Reschedule our dependencies, and ensure we run within a timeout. +* +* Note that if the timeout is exceeded, then whoever was running that +* prevented us from acquiring the GPU is declared rogue and reset. An +* unresponsive process will then be banned in order to preserve +* interactivity. Since this can be seen as a bit heavy-handed, we +* select a timeout for when the dropped frames start to become a +* noticeable nuisance for the user (100 ms, i.e. preemption was +* blocked for more than a few frames). Note, this is only a timeout +* for a delay in preem
[Intel-gfx] [PATCH 16/18] drm/i915/preemption: Select timeout when scheduling
The choice of preemption timeout is determined by the context from which we trigger the preemption, as such allow the caller to specify the desired timeout. Effectively the other choice would be to use the shortest timeout along the dependency chain. However, given that we would have already triggered preemption for the dependency chain, we can assume that no preemption along that chain is more important than the current request, ergo we need only consider the current timeout. Realising this, we can then pass control of the preemption timeout to the caller for greater flexibility. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c| 2 +- drivers/gpu/drm/i915/i915_request.c| 2 +- drivers/gpu/drm/i915/intel_lrc.c | 5 +- drivers/gpu/drm/i915/intel_ringbuffer.h| 6 +- drivers/gpu/drm/i915/selftests/intel_lrc.c | 106 - 5 files changed, 114 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index cec52bbd1b41..e8c1a077e223 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -576,7 +576,7 @@ static void __fence_set_priority(struct dma_fence *fence, int prio) rcu_read_lock(); if (engine->schedule) - engine->schedule(rq, prio); + engine->schedule(rq, prio, 0); rcu_read_unlock(); } diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 629f3e860592..ddb829ef 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1062,7 +1062,7 @@ void __i915_request_add(struct i915_request *request, bool flush_caches) */ rcu_read_lock(); if (engine->schedule) - engine->schedule(request, request->ctx->priority); + engine->schedule(request, request->ctx->priority, 0); rcu_read_unlock(); local_bh_disable(); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 4f1e985b3cdb..877340afb63d 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1260,7 +1260,8 @@ pt_lock_engine(struct i915_priotree *pt, struct intel_engine_cs *locked) return engine; } -static void execlists_schedule(struct i915_request *request, int prio) +static void execlists_schedule(struct i915_request *request, + int prio, unsigned int timeout) { struct intel_engine_cs *engine; struct i915_dependency *dep, *p; @@ -1356,7 +1357,7 @@ static void execlists_schedule(struct i915_request *request, int prio) if (prio > engine->execlists.queue_priority && i915_sw_fence_done(&pt_to_request(pt)->submit)) - __submit_queue(engine, prio, 0); + __submit_queue(engine, prio, timeout); } spin_unlock_irq(&engine->timeline->lock); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 25147a877518..04d25d1d4c2f 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -463,13 +463,15 @@ struct intel_engine_cs { */ void(*submit_request)(struct i915_request *rq); - /* Call when the priority on a request has changed and it and its + /* +* Call when the priority on a request has changed and it and its * dependencies may need rescheduling. Note the request itself may * not be ready to run! * * Called under the struct_mutex. */ - void(*schedule)(struct i915_request *request, int priority); + void(*schedule)(struct i915_request *request, + int priority, unsigned int timeout); /* * Cancel all requests on the hardware, or queued for execution. diff --git a/drivers/gpu/drm/i915/selftests/intel_lrc.c b/drivers/gpu/drm/i915/selftests/intel_lrc.c index 72d2770e5d71..5e4d6d07fff5 100644 --- a/drivers/gpu/drm/i915/selftests/intel_lrc.c +++ b/drivers/gpu/drm/i915/selftests/intel_lrc.c @@ -458,7 +458,7 @@ static int live_late_preempt(void *arg) goto err_wedged; } - engine->schedule(rq, I915_PRIORITY_MAX); + engine->schedule(rq, I915_PRIORITY_MAX, 0); if (!wait_for_spinner(&spin_hi, rq)) { pr_err("High priority context failed to preempt the low priority context\n"); @@ -677,6 +677,109 @@ static int live_preempt_reset(void *arg) return err; } +static int live_late_preempt_timeout(void *arg) +{ + struct drm_i915_private *i915 = arg; + struct i915_gem_context *ctx_hi, *ctx_lo; + struct spinner spin_hi, spin_lo; + struct intel_engine_cs *engine; + enum intel_engine_id id; + int
[Intel-gfx] [PATCH 13/18] drm/i915: Compile out engine debug for release
The majority of the engine state dumping is too voluminous to be useful outside of a controlled setup, though a few do accompany severe errors. Keep the debug dumps next to the errors, but hide the others behind a CI compile flag. This becomes more useful when adding more dumps to latency sensitive paths. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_gem.h | 6 ++ drivers/gpu/drm/i915/intel_breadcrumbs.c | 2 +- drivers/gpu/drm/i915/intel_hangcheck.c | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 42a387ff0eaa..cec52bbd1b41 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3267,7 +3267,7 @@ void i915_gem_set_wedged(struct drm_i915_private *i915) GEM_TRACE("start\n"); - if (drm_debug & DRM_UT_DRIVER) { + if (GEM_SHOW_DEBUG()) { struct drm_printer p = drm_debug_printer(__func__); for_each_engine(engine, i915, id) diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h index deaf78d2ae8b..525920404ede 100644 --- a/drivers/gpu/drm/i915/i915_gem.h +++ b/drivers/gpu/drm/i915/i915_gem.h @@ -30,6 +30,9 @@ struct drm_i915_private; #ifdef CONFIG_DRM_I915_DEBUG_GEM + +#define GEM_SHOW_DEBUG() (drm_debug & DRM_UT_DRIVER) + #define GEM_BUG_ON(condition) do { if (unlikely((condition))) {\ pr_err("%s:%d GEM_BUG_ON(%s)\n", \ __func__, __LINE__, __stringify(condition)); \ @@ -45,6 +48,9 @@ struct drm_i915_private; #define GEM_DEBUG_BUG_ON(expr) GEM_BUG_ON(expr) #else + +#define GEM_SHOW_DEBUG() (0) + #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr) #define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0) diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c index ca0b04d9747c..ad761b8d843d 100644 --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c @@ -82,7 +82,7 @@ static unsigned long wait_timeout(void) static noinline void missed_breadcrumb(struct intel_engine_cs *engine) { - if (drm_debug & DRM_UT_DRIVER) { + if (GEM_SHOW_DEBUG()) { struct drm_printer p = drm_debug_printer(__func__); intel_engine_dump(engine, &p, diff --git a/drivers/gpu/drm/i915/intel_hangcheck.c b/drivers/gpu/drm/i915/intel_hangcheck.c index fd0ffb8328d0..309e38b00e95 100644 --- a/drivers/gpu/drm/i915/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/intel_hangcheck.c @@ -356,7 +356,7 @@ static void hangcheck_accumulate_sample(struct intel_engine_cs *engine, break; case ENGINE_DEAD: - if (drm_debug & DRM_UT_DRIVER) { + if (GEM_SHOW_DEBUG()) { struct drm_printer p = drm_debug_printer("hangcheck"); intel_engine_dump(engine, &p, "%s\n", engine->name); } -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Silence debugging DRM_ERROR for failing to suspend vlv powerwells
== Series Details == Series: drm/i915: Silence debugging DRM_ERROR for failing to suspend vlv powerwells URL : https://patchwork.freedesktop.org/series/41350/ State : success == Summary == Possible new issues: Test gem_pwrite: Subgroup big-gtt-backwards: skip -> PASS (shard-apl) Known issues: Test kms_flip: Subgroup 2x-flip-vs-expired-vblank-interruptible: fail -> PASS (shard-hsw) fdo#102887 +1 Subgroup 2x-plain-flip-fb-recreate: fail -> PASS (shard-hsw) fdo#100368 Test kms_sysfs_edid_timing: pass -> WARN (shard-apl) fdo#100047 fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047 shard-apltotal:2680 pass:1835 dwarn:1 dfail:0 fail:7 skip:836 time:12699s shard-hswtotal:2680 pass:1785 dwarn:1 dfail:0 fail:2 skip:891 time:11415s Blacklisted hosts: shard-kbltotal:2680 pass:1962 dwarn:2 dfail:0 fail:7 skip:709 time:9180s shard-snbtotal:2680 pass:1377 dwarn:1 dfail:0 fail:3 skip:1299 time:6908s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8636/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915/selftests: Compare mappable vma against instance in unmappable region
Add an additional comparison to check the entire vma created in the mappable region of the global GTT against the one in the unmappable range. Further test with an assert_partial as well to ensure the VMA corresponds to the original object's backing store. Signed-off-by: Abdiel Janulgue Cc: Joonas Lahtinen Cc: Chris Wilson --- drivers/gpu/drm/i915/selftests/i915_vma.c | 44 ++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c index ea48bac..10cf4df 100644 --- a/drivers/gpu/drm/i915/selftests/i915_vma.c +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c @@ -554,6 +554,30 @@ static bool assert_partial(struct drm_i915_gem_object *obj, return true; } +static bool assert_vma_compare(struct i915_vma *vma1, struct i915_vma *vma2) +{ + struct sgt_iter sgt; + dma_addr_t dma; + unsigned long offset = 0; + + for_each_sgt_dma(dma, sgt, vma1->pages) { + dma_addr_t src; + + src = sg_dma_address(vma2->pages->sgl) + + (offset << PAGE_SHIFT); + + if (src != dma) { + pr_err("VMA comparison failed. DMA mismatch for partial " + "page offset %lu\n", offset); + return false; + } + + offset++; + } + + return true; +} + static bool assert_pin(struct i915_vma *vma, struct i915_ggtt_view *view, u64 size, @@ -621,7 +645,7 @@ static int igt_vma_partial(void *arg) { }, }, *p; unsigned int sz, offset; - struct i915_vma *vma; + struct i915_vma *vma, *vma_unmapped = NULL; int err = -ENOMEM; /* @@ -660,6 +684,8 @@ static int igt_vma_partial(void *arg) err = i915_vma_pin(vma, 0, 0, PIN_GLOBAL | PIN_HIGH); if (err) goto out_object; + + vma_unmapped = vma; } nvma = 0; @@ -748,6 +774,22 @@ static int igt_vma_partial(void *arg) goto out_object; } + if (!assert_partial(obj, vma, 0, npages)) { + pr_err("(%s) Inconsistent partial pages for (offset=%d, size=%d)\n", + p->name, offset, sz); + err = -EINVAL; + goto out_object; + } + + if (vma_unmapped) { + if (!assert_vma_compare(vma, vma_unmapped)) { + pr_err("(%s) Inconsistent vma from unmapped region\n", + p->name); + err = -EINVAL; + goto out_object; + } + } + i915_vma_unpin(vma); } } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915/selftests: Extend partial vma coverage to check parallel creation
From: Chris Wilson One important use of partial vma is to provide mappable access to the object while it is being used elsewhere (pinned entirely into the unmappable portion of the Global GTT, i.e. for use as a display scanout). Signed-off-by: Chris Wilson Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/selftests/i915_vma.c | 59 +++ 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c index eb89e30..ea48bac 100644 --- a/drivers/gpu/drm/i915/selftests/i915_vma.c +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c @@ -606,11 +606,17 @@ static int igt_vma_partial(void *arg) struct drm_i915_private *i915 = arg; struct i915_address_space *vm = &i915->ggtt.base; const unsigned int npages = 1021; /* prime! */ - struct drm_i915_gem_object *obj; + struct drm_i915_gem_object *obj = NULL; const struct phase { const char *name; + unsigned int flags; +#define CREATE BIT(0) +#define WHOLE BIT(1) } phases[] = { - { "create" }, + { "create", CREATE }, + { "lookup" }, + { "whole", WHOLE }, + { "recreate", CREATE | WHOLE }, { "lookup" }, { }, }, *p; @@ -618,17 +624,44 @@ static int igt_vma_partial(void *arg) struct i915_vma *vma; int err = -ENOMEM; - /* Create lots of different VMA for the object and check that + /* +* Create lots of different VMA for the object and check that * we are returned the same VMA when we later request the same range. */ - obj = i915_gem_object_create_internal(i915, npages*PAGE_SIZE); - if (IS_ERR(obj)) - goto out; - for (p = phases; p->name; p++) { /* exercise both create/lookup */ unsigned int count, nvma; + if (p->flags & CREATE) { + if (obj) + i915_gem_object_put(obj); + + obj = i915_gem_object_create_internal(i915, + npages*PAGE_SIZE); + if (IS_ERR(obj)) + goto out; + } + + if (p->flags & WHOLE) { + /* +* Make sure we can create mappable partial vma +* while the whole object is in use elsewhere. +*/ + vma = i915_vma_instance(obj, vm, NULL); + if (IS_ERR(vma)) { + err = PTR_ERR(vma); + goto out_object; + } + + err = i915_vma_unbind(vma); + if (err) + goto out_object; + + err = i915_vma_pin(vma, 0, 0, PIN_GLOBAL | PIN_HIGH); + if (err) + goto out_object; + } + nvma = 0; for_each_prime_number_from(sz, 1, npages) { for_each_prime_number_from(offset, 0, npages - sz) { @@ -707,12 +740,24 @@ static int igt_vma_partial(void *arg) err = -EINVAL; goto out_object; } + + if (p->flags & WHOLE) { + vma = i915_vma_instance(obj, vm, NULL); + if (IS_ERR(vma)) { + err = PTR_ERR(vma); + goto out_object; + } + + i915_vma_unpin(vma); + } } out_object: i915_gem_object_put(obj); out: return err; +#undef CREATE +#undef WHOLE } int i915_vma_mock_selftests(void) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/18] drm/i915/execlists: Set queue priority from secondary port
== Series Details == Series: series starting with [01/18] drm/i915/execlists: Set queue priority from secondary port URL : https://patchwork.freedesktop.org/series/41357/ State : warning == Summary == $ dim checkpatch origin/drm-tip fe34e7d49f28 drm/i915/execlists: Set queue priority from secondary port -:25: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #25: References: f6322eddaff7 ("drm/i915/preemption: Allow preemption between submission ports") -:25: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("")' - ie: 'commit f6322eddaff7 ("drm/i915/preemption: Allow preemption between submission ports")' #25: References: f6322eddaff7 ("drm/i915/preemption: Allow preemption between submission ports") total: 1 errors, 1 warnings, 0 checks, 9 lines checked 7c05a3c646d6 drm/i915/execlists: Refactor out complete_preempt_context() b7ecf299a4f9 drm/i915: Move engine reset prepare/finish to backends 2b9aff94cf13 drm/i915: Split execlists/guc reset preparations 69d92c7b2ef7 drm/i915/execlists: Flush pending preemption events during reset -:69: WARNING:LONG_LINE: line over 100 characters #69: FILE: drivers/gpu/drm/i915/intel_lrc.c:908: + (i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0))); -:87: WARNING:LONG_LINE: line over 100 characters #87: FILE: drivers/gpu/drm/i915/intel_lrc.c:922: + head = readl(i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine))); -:104: WARNING:LONG_LINE: line over 100 characters #104: FILE: drivers/gpu/drm/i915/intel_lrc.c:936: + head, GEN8_CSB_READ_PTR(readl(i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine, fw ? "" : "?", -:105: WARNING:LONG_LINE: line over 100 characters #105: FILE: drivers/gpu/drm/i915/intel_lrc.c:937: + tail, GEN8_CSB_WRITE_PTR(readl(i915->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_PTR(engine, fw ? "" : "?"); total: 0 errors, 4 warnings, 0 checks, 192 lines checked 8584674c645e drm/i915/breadcrumbs: Keep the fake irq armed across reset 5333a7b17913 drm/i915: Combine tasklet_kill and tasklet_disable -:39: WARNING:MEMORY_BARRIER: memory barrier without comment #39: FILE: drivers/gpu/drm/i915/intel_lrc.c:1764: + smp_mb(); total: 0 errors, 1 warnings, 0 checks, 26 lines checked 69c64b744f61 drm/i915: Stop parking the signaler around reset 3b598171e856 drm/i915: Be irqsafe inside reset 26a3324ce9c0 drm/i915/execlists: Make submission tasklet hardirq safe 277f79ddc081 drm/i915/guc: Make submission tasklet hardirq safe c3667f5a4ad6 drm/i915: Allow init_breadcrumbs to be used from irq context c9c18eae9f33 drm/i915: Compile out engine debug for release 68bac469bf86 drm/i915/execlists: Force preemption via reset on timeout -:56: ERROR:SPACING: space prohibited after that open parenthesis '(' #56: FILE: drivers/gpu/drm/i915/intel_lrc.c:571: + intel_engine_dump( engine, &p, "%s\n", engine->name); total: 1 errors, 0 warnings, 0 checks, 228 lines checked df4b8d8c35f6 drm/i915/execlists: Try preempt-reset from hardirq timer context d8144187ca0e drm/i915/preemption: Select timeout when scheduling 0aeab6cbe712 drm/i915: Use a preemption timeout to enforce interactivity cd1c93c17baf drm/i915: Allow user control over preempt timeout on their important context ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915/selftests: Compare mappable vma against instance in unmappable region
Quoting Abdiel Janulgue (2018-04-09 12:28:04) > Add an additional comparison to check the entire vma created in the mappable > region of the global GTT against the one in the unmappable range. > > Further test with an assert_partial as well to ensure the VMA corresponds > to the original object's backing store. > > Signed-off-by: Abdiel Janulgue > Cc: Joonas Lahtinen > Cc: Chris Wilson It seems like instead of comparing two vma, you just need to compare the vma->pages against the parent obj->pages. Don't we already do that? We do check we can write through the vma and read it via the obj. That should be checking the pages do map correctly, and are also tiled correctly. And we do check partial vs object dma addresses. Adding this pass to igt_vma_partial() looks redundant as you have already established both vma point to the same obj->pages. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [01/18] drm/i915/execlists: Set queue priority from secondary port
== Series Details == Series: series starting with [01/18] drm/i915/execlists: Set queue priority from secondary port URL : https://patchwork.freedesktop.org/series/41357/ State : success == Summary == Series 41357v1 series starting with [01/18] drm/i915/execlists: Set queue priority from secondary port https://patchwork.freedesktop.org/api/1.0/series/41357/revisions/1/mbox/ Known issues: Test debugfs_test: Subgroup read_all_entries: incomplete -> PASS (fi-snb-2520m) fdo#103713 Test gem_ctx_param: Subgroup basic-default: incomplete -> PASS (fi-cnl-y3) fdo#105086 Test kms_chamelium: Subgroup dp-edid-read: fail -> PASS (fi-kbl-7500u) fdo#102505 Test kms_flip: Subgroup basic-flip-vs-wf_vblank: fail -> PASS (fi-cfl-s3) fdo#100368 Test kms_pipe_crc_basic: Subgroup read-crc-pipe-b-frame-sequence: pass -> FAIL (fi-skl-6700k2) fdo#103191 Subgroup suspend-read-crc-pipe-c: pass -> INCOMPLETE (fi-bxt-dsi) fdo#103927 Test prime_vgem: Subgroup basic-fence-flip: pass -> FAIL (fi-ilk-650) fdo#104008 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086 fdo#102505 https://bugs.freedesktop.org/show_bug.cgi?id=102505 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:430s fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:445s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:385s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:546s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:297s fi-bxt-dsi total:243 pass:216 dwarn:0 dfail:0 fail:0 skip:26 fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:515s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:524s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:513s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:413s fi-cfl-s3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:559s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:512s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:584s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:422s fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:314s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:534s fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:491s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:409s fi-ilk-650 total:285 pass:224 dwarn:0 dfail:0 fail:1 skip:60 time:423s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:469s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:434s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:472s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:465s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:510s fi-pnv-d510 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:672s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:440s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:539s fi-skl-6700k2total:285 pass:260 dwarn:0 dfail:0 fail:1 skip:24 time:511s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:497s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:427s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:446s fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:555s fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:395s 1be073153147c5c39cdcbdfdeb4e2595ba595bf7 drm-tip: 2018y-04m-07d-22h-26m-31s UTC integration manifest cd1c93c17baf drm/i915: Allow user control over preempt timeout on their important context 0aeab6cbe712 drm/i915: Use a preemption timeout to enforce interactivity d8144187ca0e drm/i915/preemption: Select timeout when scheduling df4b8d8c35f6 drm/i915/execlists: Try preempt-reset from hardirq timer context 68bac469bf86 drm/i915/execlists: Force
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Keep a count of requests submitted from userspace
On 09/04/2018 11:51, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-04-09 11:40:08) On 09/04/2018 11:27, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-04-09 11:17:04) On 09/04/2018 10:25, Chris Wilson wrote: Downside being that we either then use atomic64 throughout or we mix atomic32/atomic64 knowing that we're on x86. (I feel like someone else must have solved this problem in a much neater way, before they went to per-cpu stats ;) Is the winky implying you know who and where? :) We have three potential solutions now, even for if the winky is suggesting something. Nah, just that atomic/locked counters are so old hat. Not sure if there remain any good examples for hotpath counters that remain applicable to our code. Leave it as is then for now and improve if we discover it is not good enough? I did have an ulterior motive in that the cmpxchg did resolve one issue that irked me with the two counters being updated out of sync. Minor, minor glitches :) I don't have a strong preference either way. These instructions on the submit are not likely to stand out, as compared to the biggest fish of ksoftirqd, execlists_schedule() and execlists_dequeue(). I could move the queued decrement from submit_notify to backends, right next to runnable++? Then both would be under the engine->timeline->lock so any inconsistencies in readout I'd hope should be dismissable? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v1 00/14] Preparation patches for NV12
Op 09-04-18 om 11:41 schreef Srinivas, Vidya: > >> -Original Message- >> From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] >> Sent: Monday, April 9, 2018 2:38 PM >> To: Srinivas, Vidya ; intel- >> g...@lists.freedesktop.org >> Cc: Kamath, Sunil >> Subject: Re: [Intel-gfx] [PATCH v1 00/14] Preparation patches for NV12 >> >> Op 09-04-18 om 10:57 schreef Srinivas, Vidya: -Original Message- From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com] Sent: Monday, April 9, 2018 2:04 PM To: Srinivas, Vidya ; intel- g...@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v1 00/14] Preparation patches for NV12 Op 09-04-18 om 05:40 schreef Vidya Srinivas: > Series contain preparation patches for NV12 support Enabling NV12 > KMD support will follow the series > > Chandra Konduru (3): > drm/i915: Set scaler mode for NV12 > drm/i915: Update format_is_yuv() to include NV12 > drm/i915: Upscale scaler max scale for NV12 > > Mahesh Kumar (9): > drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values > drm/i915/skl+: refactor WM calculation for NV12 > drm/i915/skl+: add NV12 in skl_format_to_fourcc > drm/i915/skl+: support verification of DDB HW state for NV12 > drm/i915/skl+: NV12 related changes for WM > drm/i915/skl+: pass skl_wm_level struct to wm compute func > drm/i915/skl+: make sure higher latency level has higher wm value > drm/i915/skl+: nv12 workaround disable WM level 1-7 > drm/i915/skl: split skl_compute_ddb function > > Vidya Srinivas (2): > drm/i915: Display WA 827 > drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg > > drivers/gpu/drm/i915/i915_drv.h | 10 +- > drivers/gpu/drm/i915/i915_reg.h | 5 + > drivers/gpu/drm/i915/intel_atomic.c | 14 +- > drivers/gpu/drm/i915/intel_display.c | 93 ++-- > drivers/gpu/drm/i915/intel_drv.h | 11 +- > drivers/gpu/drm/i915/intel_pm.c | 438 ++-- >> --- > drivers/gpu/drm/i915/intel_sprite.c | 7 +- > 7 files changed, 393 insertions(+), 185 deletions(-) This series looks good, so for any patches I missed: Reviewed-by: Maarten Lankhorst Do you have commit rights, or should I push them? >>> Thank you. I don’t have commit rights I think. >>> Also, Should I add your RB for all the patches and push them again? >>> >> I'll push them. :) > Thank you so much :) > Pushed! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/selftests: Extend partial vma coverage to check parallel creation
== Series Details == Series: series starting with [1/2] drm/i915/selftests: Extend partial vma coverage to check parallel creation URL : https://patchwork.freedesktop.org/series/41359/ State : warning == Summary == $ dim checkpatch origin/drm-tip 6a326fc76a03 drm/i915/selftests: Extend partial vma coverage to check parallel creation -:60: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV) #60: FILE: drivers/gpu/drm/i915/selftests/i915_vma.c:640: + npages*PAGE_SIZE); ^ total: 0 errors, 0 warnings, 1 checks, 92 lines checked d9b8dd829776 drm/i915/selftests: Compare mappable vma against instance in unmappable region -:7: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line) #7: Add an additional comparison to check the entire vma created in the mappable total: 0 errors, 1 warnings, 0 checks, 68 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Keep a count of requests submitted from userspace
Quoting Tvrtko Ursulin (2018-04-09 12:43:50) > > On 09/04/2018 11:51, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-04-09 11:40:08) > >> > >> On 09/04/2018 11:27, Chris Wilson wrote: > >>> Quoting Tvrtko Ursulin (2018-04-09 11:17:04) > > On 09/04/2018 10:25, Chris Wilson wrote: > > Downside being that we either then use atomic64 throughout or we mix > > atomic32/atomic64 knowing that we're on x86. (I feel like someone else > > must have solved this problem in a much neater way, before they went to > > per-cpu stats ;) > > Is the winky implying you know who and where? :) We have three potential > solutions now, even for if the winky is suggesting something. > >>> > >>> Nah, just that atomic/locked counters are so old hat. Not sure if there > >>> remain any good examples for hotpath counters that remain applicable to > >>> our code. > >> > >> Leave it as is then for now and improve if we discover it is not good > >> enough? > > > > I did have an ulterior motive in that the cmpxchg did resolve one issue > > that irked me with the two counters being updated out of sync. Minor, > > minor glitches :) > > > > I don't have a strong preference either way. These instructions on the > > submit are not likely to stand out, as compared to the biggest fish of > > ksoftirqd, execlists_schedule() and execlists_dequeue(). > > I could move the queued decrement from submit_notify to backends, right > next to runnable++? Then both would be under the engine->timeline->lock > so any inconsistencies in readout I'd hope should be dismissable? Fair. I have this itch to add a request->state, switch (request->state) { case QUEUED: stats->queued--; switch (now) { case QUEUED: BUG(); case: READY: stats->runnable++; case EXEC: break; } break; case ... } request->state = now; Stop me. Please, stop me. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/execlists: Log fence context & seqno throughout GEM_TRACE
Quoting Tvrtko Ursulin (2018-04-06 13:35:14) > From: Tvrtko Ursulin > > Include fence context and seqno in low level tracing so it is easier to > follow flows of individual requests when things go bad. > > Also added tracing on the reset side of things. > > v2: > Chris Wilson: > * Standardize global_seqno and seqno as global. > * Include current hws seqno in execlists_cancel_port_requests. > > v3: > * Fix port printk format for all builds. > > Signed-off-by: Tvrtko Ursulin > Reviewed-by: Chris Wilson # v2 Since I didn't spot the printk kerfuffle, it's fair to assume my rb would stand for a minor change :) Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/selftests: Extend partial vma coverage to check parallel creation
== Series Details == Series: series starting with [1/2] drm/i915/selftests: Extend partial vma coverage to check parallel creation URL : https://patchwork.freedesktop.org/series/41359/ State : success == Summary == Series 41359v1 series starting with [1/2] drm/i915/selftests: Extend partial vma coverage to check parallel creation https://patchwork.freedesktop.org/api/1.0/series/41359/revisions/1/mbox/ Known issues: Test debugfs_test: Subgroup read_all_entries: incomplete -> PASS (fi-snb-2520m) fdo#103713 Test gem_ctx_param: Subgroup basic-default: incomplete -> PASS (fi-cnl-y3) fdo#105086 Test kms_chamelium: Subgroup dp-edid-read: fail -> PASS (fi-kbl-7500u) fdo#102505 Test kms_flip: Subgroup basic-flip-vs-wf_vblank: fail -> PASS (fi-cfl-s3) fdo#100368 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086 fdo#102505 https://bugs.freedesktop.org/show_bug.cgi?id=102505 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:429s fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:440s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:380s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:538s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:297s fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:515s fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:514s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:519s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:506s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:411s fi-cfl-s3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:559s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:511s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:583s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:431s fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:314s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:535s fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:486s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:404s fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:428s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:476s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:440s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:472s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:462s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:511s fi-pnv-d510 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:670s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:447s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:533s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:501s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:502s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:426s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:445s fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:581s fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:398s 1be073153147c5c39cdcbdfdeb4e2595ba595bf7 drm-tip: 2018y-04m-07d-22h-26m-31s UTC integration manifest d9b8dd829776 drm/i915/selftests: Compare mappable vma against instance in unmappable region 6a326fc76a03 drm/i915/selftests: Extend partial vma coverage to check parallel creation == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8639/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 01/12] drm/i915: Park before resetting the submission backend
From: Chris Wilson As different backends may have different park/unpark callbacks, we should only ever switch backends (reset_default_submission on wedge recovery, or on enabling the guc) while parked. v2: Remove the assert from the guc code, as we are currently trying to modify the engine vfuncs pointer on a live system after reset (not just wedging). We will just have to hope that the system is balanced. v3: Rebase onto __i915_gem_park and improve grammar. Signed-off-by: Chris Wilson Cc: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Tvrtko Ursulin Cc: Mika Kuoppala Reviewed-by: Sagar Arun Kamble Reviewed-by: Michal Wajdeczko --- drivers/gpu/drm/i915/i915_gem.c| 15 --- drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 28ab0be..dd3e292 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -144,8 +144,6 @@ static u32 __i915_gem_park(struct drm_i915_private *i915) if (!i915->gt.awake) return I915_EPOCH_INVALID; - GEM_BUG_ON(i915->gt.epoch == I915_EPOCH_INVALID); - /* * Be paranoid and flush a concurrent interrupt to make sure * we don't reactivate any irq tasklets after parking. @@ -173,6 +171,7 @@ static u32 __i915_gem_park(struct drm_i915_private *i915) intel_runtime_pm_put(i915); + GEM_BUG_ON(i915->gt.epoch == I915_EPOCH_INVALID); return i915->gt.epoch; } @@ -3435,7 +3434,17 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915) } } i915_retire_requests(i915); - GEM_BUG_ON(i915->gt.active_requests); + + /* +* Park before disengaging the old submit mechanism as different +* backends may have different park/unpack callbacks. +* +* We are idle; the idle-worker will be queued, but we need to run +* it now. As we already hold the struct mutex, we can park the GPU +* right away, letting the lazy worker see that we are already active +* again by the time it acquires the mutex. +*/ + __i915_gem_park(i915); /* * Undo nop_submit_request. We prevent all new i915 requests from diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 12486d8..b4ea77a 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1651,6 +1651,9 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915) struct intel_engine_cs *engine; enum intel_engine_id id; + /* Must be parked first! */ + GEM_BUG_ON(i915->gt.awake); + for_each_engine(engine, i915, id) engine->set_default_submission(engine); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 09/12] drm/i915/uc: Use correct error code for GuC initialization failure
Since commit 6ca9a2beb54a ("drm/i915: Unwind i915_gem_init() failure") we believed that we correctly handle all errors encountered during GuC initialization, including special one that indicates request to run driver with disabled GPU submission (-EIO). Unfortunately since commit 121981fafe69 ("drm/i915/guc: Combine enable_guc_loading|submission modparams") we stopped using that error code to avoid unwanted fallback to execlist submission mode. In result any GuC initialization failure was treated as non-recoverable error leading to driver load abort, so we could not even read related GuC error log to investigate cause of the problem. Fix that by always returning -EIO on uC hardware related failure. v2: don't allow -EIO from uc_init don't call uc_fini[_misc] on -EIO mark guc fw as failed on hw init failure prepare uc_fini_hw to run after earlier -EIO v3: update comments (Sagar) use sanitize functions on failure in init_hw (Michal) and also sanitize guc/huc fw in fini_hw (Michal) v4: rebase v5: rebase Signed-off-by: Michal Wajdeczko Cc: Chris Wilson Cc: Michal Winiarski Cc: Daniele Ceraolo Spurio Cc: Sagar Arun Kamble Reviewed-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/i915_gem.c| 17 ++--- drivers/gpu/drm/i915/intel_guc.h | 5 + drivers/gpu/drm/i915/intel_uc.c| 15 +++ drivers/gpu/drm/i915/intel_uc_fw.h | 5 + 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index decda1a..532246a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5444,8 +5444,10 @@ int i915_gem_init(struct drm_i915_private *dev_priv) intel_init_gt_powersave(dev_priv); ret = intel_uc_init(dev_priv); - if (ret) + if (ret) { + GEM_BUG_ON(ret == -EIO); goto err_pm; + } ret = i915_gem_init_hw(dev_priv); if (ret) @@ -5492,7 +5494,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv) i915_gem_contexts_lost(dev_priv); i915_gem_fini_hw(dev_priv); err_uc_init: - intel_uc_fini(dev_priv); + if (ret != -EIO) + intel_uc_fini(dev_priv); err_pm: if (ret != -EIO) { intel_cleanup_gt_powersave(dev_priv); @@ -5506,15 +5509,15 @@ int i915_gem_init(struct drm_i915_private *dev_priv) intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); mutex_unlock(&dev_priv->drm.struct_mutex); - intel_uc_fini_misc(dev_priv); - - if (ret != -EIO) + if (ret != -EIO) { + intel_uc_fini_misc(dev_priv); i915_gem_cleanup_userptr(dev_priv); + } if (ret == -EIO) { /* -* Allow engine initialisation to fail by marking the GPU as -* wedged. But we only want to do this where the GPU is angry, +* Allow engines or uC initialization to fail by marking the GPU +* as wedged. But we only want to do this when the GPU is angry, * for all other failure, such as an allocation failure, bail. */ if (!i915_terminally_wedged(&dev_priv->gpu_error)) { diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h index f1265e1..c587068 100644 --- a/drivers/gpu/drm/i915/intel_guc.h +++ b/drivers/gpu/drm/i915/intel_guc.h @@ -176,6 +176,11 @@ static inline int intel_guc_sanitize(struct intel_guc *guc) return 0; } +static inline bool intel_guc_is_loaded(struct intel_guc *guc) +{ + return intel_uc_fw_is_loaded(&guc->fw); +} + static inline void intel_guc_enable_msg(struct intel_guc *guc, u32 mask) { spin_lock_irq(&guc->irq_lock); diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 0439966..7862731 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -332,6 +332,8 @@ static void __uc_sanitize(struct drm_i915_private *i915) intel_huc_sanitize(huc); intel_guc_sanitize(guc); + GEM_BUG_ON(intel_guc_is_loaded(guc)); + __intel_uc_reset_hw(i915); } @@ -420,11 +422,13 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv) * Note that there is no fallback as either user explicitly asked for * the GuC or driver default option was to run with the GuC enabled. */ - if (GEM_WARN_ON(ret == -EIO)) - ret = -EINVAL; - dev_err(dev_priv->drm.dev, "GuC initialization failed %d\n", ret); - return ret; + + /* Sanitize GuC/HuC to avoid clean-up on wedged */ + __uc_sanitize(dev_priv); + + /* We want to disable GPU submission but keep KMS alive */ + return -EIO; } void intel_uc_fini_hw(struct drm_i915_private *dev_priv) @@ -436,6 +440,9 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv) GEM_BUG_ON(!HAS_GUC(dev_priv)); +
[Intel-gfx] [PATCH v8 02/12] drm/i915: Correctly handle error path in i915_gem_init_hw
In function gem_init_hw() we are calling uc_init_hw() but in case of error later in function, we missed to call matching uc_fini_hw() Signed-off-by: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Chris Wilson Reviewed-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/i915_gem.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index dd3e292..26294e8 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5246,9 +5246,15 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv) /* Only when the HW is re-initialised, can we replay the requests */ ret = __i915_gem_restart_engines(dev_priv); + if (ret) + goto cleanup_uc; out: intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); return ret; + +cleanup_uc: + intel_uc_fini_hw(dev_priv); + goto out; } static int __intel_engines_record_defaults(struct drm_i915_private *i915) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 03/12] drm/i915: Move i915_gem_fini to i915_gem.c
We should keep i915_gem_init/fini functions together for easier tracking of their symmetry. Signed-off-by: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.c | 20 drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 20 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index f770be1..854b26c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -625,26 +625,6 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev) .can_switch = i915_switcheroo_can_switch, }; -static void i915_gem_fini(struct drm_i915_private *dev_priv) -{ - /* Flush any outstanding unpin_work. */ - i915_gem_drain_workqueue(dev_priv); - - mutex_lock(&dev_priv->drm.struct_mutex); - intel_uc_fini_hw(dev_priv); - intel_uc_fini(dev_priv); - i915_gem_cleanup_engines(dev_priv); - i915_gem_contexts_fini(dev_priv); - mutex_unlock(&dev_priv->drm.struct_mutex); - - intel_uc_fini_misc(dev_priv); - i915_gem_cleanup_userptr(dev_priv); - - i915_gem_drain_freed_objects(dev_priv); - - WARN_ON(!list_empty(&dev_priv->contexts.list)); -} - static int i915_load_modeset_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9bca104..f8bc276 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3143,6 +3143,7 @@ void i915_gem_reset_engine(struct intel_engine_cs *engine, int __must_check i915_gem_init(struct drm_i915_private *dev_priv); int __must_check i915_gem_init_hw(struct drm_i915_private *dev_priv); void i915_gem_init_swizzling(struct drm_i915_private *dev_priv); +void i915_gem_fini(struct drm_i915_private *dev_priv); void i915_gem_cleanup_engines(struct drm_i915_private *dev_priv); int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv, unsigned int flags); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 26294e8..fb99485 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5520,6 +5520,26 @@ int i915_gem_init(struct drm_i915_private *dev_priv) return ret; } +void i915_gem_fini(struct drm_i915_private *dev_priv) +{ + /* Flush any outstanding unpin_work. */ + i915_gem_drain_workqueue(dev_priv); + + mutex_lock(&dev_priv->drm.struct_mutex); + intel_uc_fini_hw(dev_priv); + intel_uc_fini(dev_priv); + i915_gem_cleanup_engines(dev_priv); + i915_gem_contexts_fini(dev_priv); + mutex_unlock(&dev_priv->drm.struct_mutex); + + intel_uc_fini_misc(dev_priv); + i915_gem_cleanup_userptr(dev_priv); + + i915_gem_drain_freed_objects(dev_priv); + + WARN_ON(!list_empty(&dev_priv->contexts.list)); +} + void i915_gem_init_mmio(struct drm_i915_private *i915) { i915_gem_sanitize(i915); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 04/12] drm/i915: Introduce i915_gem_fini_hw for symmetry with i915_gem_init_hw
We have i915_gem_init_hw function that on failure requires some cleanup in uC and then in other places we were trying to do such cleanup directly. Let's fix that by adding i915_gem_fini_hw for nice symmetry with init_hw and call it on cleanup paths. Signed-off-by: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 13 +++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f8bc276..dbd95a4 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3144,6 +3144,7 @@ void i915_gem_reset_engine(struct intel_engine_cs *engine, int __must_check i915_gem_init_hw(struct drm_i915_private *dev_priv); void i915_gem_init_swizzling(struct drm_i915_private *dev_priv); void i915_gem_fini(struct drm_i915_private *dev_priv); +void i915_gem_fini_hw(struct drm_i915_private *dev_priv); void i915_gem_cleanup_engines(struct drm_i915_private *dev_priv); int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv, unsigned int flags); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index fb99485..6f71099 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5257,6 +5257,15 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv) goto out; } +void i915_gem_fini_hw(struct drm_i915_private *dev_priv) +{ + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); + + intel_uc_fini_hw(dev_priv); + + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); +} + static int __intel_engines_record_defaults(struct drm_i915_private *i915) { struct i915_gem_context *ctx; @@ -5482,7 +5491,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv) err_init_hw: i915_gem_wait_for_idle(dev_priv, I915_WAIT_LOCKED); i915_gem_contexts_lost(dev_priv); - intel_uc_fini_hw(dev_priv); + i915_gem_fini_hw(dev_priv); err_uc_init: intel_uc_fini(dev_priv); err_pm: @@ -5526,7 +5535,7 @@ void i915_gem_fini(struct drm_i915_private *dev_priv) i915_gem_drain_workqueue(dev_priv); mutex_lock(&dev_priv->drm.struct_mutex); - intel_uc_fini_hw(dev_priv); + i915_gem_fini_hw(dev_priv); intel_uc_fini(dev_priv); i915_gem_cleanup_engines(dev_priv); i915_gem_contexts_fini(dev_priv); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 07/12] drm/i915/guc: Restore symmetric doorbell cleanup
In commit 9192d4fb811e ("drm/i915/guc: Extract doorbell creation from client allocation") we introduced asymmetry in doorbell cleanup to avoid warnings due to failed communication with already reset GuC. As we improved our reset/unload paths, we can restore symmetry in doorbell cleanup, as GuC should be still active by now. Suggested-by: Sagar Arun Kamble Signed-off-by: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Michal Winiarski Cc: Chris Wilson Reviewed-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/intel_guc_submission.c | 15 +++ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 9712123..3b6d7f5 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -848,18 +848,9 @@ static int guc_clients_doorbell_init(struct intel_guc *guc) static void guc_clients_doorbell_fini(struct intel_guc *guc) { - /* -* By the time we're here, GuC has already been reset. -* Instead of trying (in vain) to communicate with it, let's just -* cleanup the doorbell HW and our internal state. -*/ - if (guc->preempt_client) { - __destroy_doorbell(guc->preempt_client); - __update_doorbell_desc(guc->preempt_client, - GUC_DOORBELL_INVALID); - } - __destroy_doorbell(guc->execbuf_client); - __update_doorbell_desc(guc->execbuf_client, GUC_DOORBELL_INVALID); + if (guc->preempt_client) + destroy_doorbell(guc->preempt_client); + destroy_doorbell(guc->execbuf_client); } /** -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 08/12] drm/i915/uc: Fully sanitize uC within intel_uc_fini_hw
As we always call intel_uc_sanitize after every call to intel_uc_fini_hw we may drop redundant call and sanitize uC from the fini_hw function. Signed-off-by: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 2 -- drivers/gpu/drm/i915/intel_uc.c | 9 +++-- drivers/gpu/drm/i915/intel_uc.h | 1 - 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ceec5a0..decda1a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3077,7 +3077,6 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv) } i915_gem_revoke_fences(dev_priv); - intel_uc_sanitize(dev_priv); return err; } @@ -5062,7 +5061,6 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv) * machine in an unusable condition. */ i915_gem_fini_hw(dev_priv); - intel_uc_sanitize(dev_priv); i915_gem_sanitize(dev_priv); intel_runtime_pm_put(dev_priv); diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 1cffaf7..0439966 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -322,18 +322,13 @@ void intel_uc_fini(struct drm_i915_private *dev_priv) intel_guc_fini(guc); } -void intel_uc_sanitize(struct drm_i915_private *i915) +static void __uc_sanitize(struct drm_i915_private *i915) { struct intel_guc *guc = &i915->guc; struct intel_huc *huc = &i915->huc; - if (!USES_GUC(i915)) - return; - GEM_BUG_ON(!HAS_GUC(i915)); - guc_disable_communication(guc); - intel_huc_sanitize(huc); intel_guc_sanitize(guc); @@ -445,6 +440,8 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv) intel_guc_submission_disable(guc); guc_disable_communication(guc); + + __uc_sanitize(dev_priv); } int intel_uc_suspend(struct drm_i915_private *i915) diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index 25d73ad..64aaf93 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -33,7 +33,6 @@ void intel_uc_init_mmio(struct drm_i915_private *dev_priv); int intel_uc_init_misc(struct drm_i915_private *dev_priv); void intel_uc_fini_misc(struct drm_i915_private *dev_priv); -void intel_uc_sanitize(struct drm_i915_private *dev_priv); int intel_uc_init_hw(struct drm_i915_private *dev_priv); void intel_uc_fini_hw(struct drm_i915_private *dev_priv); int intel_uc_init(struct drm_i915_private *dev_priv); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 06/12] drm/i915: Add i915_gem_fini_hw to i915_reset
By calling in i915_reset only i915_gem_init_hw without previous i915_gem_fini_hw we introduced asymmetry. Let's fix that. Signed-off-by: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 854b26c..a0a5af0 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1902,6 +1902,8 @@ void i915_reset(struct drm_i915_private *i915, goto error; } + i915_gem_fini_hw(i915); + for (i = 0; i < 3; i++) { ret = intel_gpu_reset(i915, ALL_ENGINES); if (ret == 0) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 05/12] drm/i915: Add i915_gem_fini_hw to i915_gem_suspend
By calling i915_gem_init_hw in i915_gem_resume and not calling i915_gem_fini_hw in i915_gem_suspend we introduced asymmetry in init_hw/fini_hw calls. Let's fix that. Signed-off-by: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Chris Wilson --- drivers/gpu/drm/i915/i915_gem.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 6f71099..ceec5a0 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5061,6 +5061,7 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv) * machines is a good idea, we don't - just in case it leaves the * machine in an unusable condition. */ + i915_gem_fini_hw(dev_priv); intel_uc_sanitize(dev_priv); i915_gem_sanitize(dev_priv); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 11/12] drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c
Some functions already use i915 name instead of dev_priv. Let's rename this param in all remaining functions, except those that still use legacy macros. v2: don't forget about function descriptions (Sagar) v3: rebased Signed-off-by: Michal Wajdeczko Reviewed-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/intel_uc.c | 133 1 file changed, 66 insertions(+), 67 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index ed39273..ccf75aa 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -50,10 +50,10 @@ static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv) return ret; } -static int __get_platform_enable_guc(struct drm_i915_private *dev_priv) +static int __get_platform_enable_guc(struct drm_i915_private *i915) { - struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; - struct intel_uc_fw *huc_fw = &dev_priv->huc.fw; + struct intel_uc_fw *guc_fw = &i915->guc.fw; + struct intel_uc_fw *huc_fw = &i915->huc.fw; int enable_guc = 0; /* Default is to enable GuC/HuC if we know their firmwares */ @@ -67,11 +67,11 @@ static int __get_platform_enable_guc(struct drm_i915_private *dev_priv) return enable_guc; } -static int __get_default_guc_log_level(struct drm_i915_private *dev_priv) +static int __get_default_guc_log_level(struct drm_i915_private *i915) { int guc_log_level; - if (!HAS_GUC(dev_priv) || !intel_uc_is_using_guc()) + if (!HAS_GUC(i915) || !intel_uc_is_using_guc()) guc_log_level = GUC_LOG_LEVEL_DISABLED; else if (IS_ENABLED(CONFIG_DRM_I915_DEBUG) || IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) @@ -86,7 +86,7 @@ static int __get_default_guc_log_level(struct drm_i915_private *dev_priv) /** * sanitize_options_early - sanitize uC related modparam options - * @dev_priv: device private + * @i915: device private * * In case of "enable_guc" option this function will attempt to modify * it only if it was initially set to "auto(-1)". Default value for this @@ -101,14 +101,14 @@ static int __get_default_guc_log_level(struct drm_i915_private *dev_priv) * unless GuC is enabled on given platform and the driver is compiled with * debug config when this modparam will default to "enable(1..4)". */ -static void sanitize_options_early(struct drm_i915_private *dev_priv) +static void sanitize_options_early(struct drm_i915_private *i915) { - struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; - struct intel_uc_fw *huc_fw = &dev_priv->huc.fw; + struct intel_uc_fw *guc_fw = &i915->guc.fw; + struct intel_uc_fw *huc_fw = &i915->huc.fw; /* A negative value means "use platform default" */ if (i915_modparams.enable_guc < 0) - i915_modparams.enable_guc = __get_platform_enable_guc(dev_priv); + i915_modparams.enable_guc = __get_platform_enable_guc(i915); DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n", i915_modparams.enable_guc, @@ -119,28 +119,28 @@ static void sanitize_options_early(struct drm_i915_private *dev_priv) if (intel_uc_is_using_guc() && !intel_uc_fw_is_selected(guc_fw)) { DRM_WARN("Incompatible option detected: %s=%d, %s!\n", "enable_guc", i915_modparams.enable_guc, -!HAS_GUC(dev_priv) ? "no GuC hardware" : - "no GuC firmware"); +!HAS_GUC(i915) ? "no GuC hardware" : + "no GuC firmware"); } /* Verify HuC firmware availability */ if (intel_uc_is_using_huc() && !intel_uc_fw_is_selected(huc_fw)) { DRM_WARN("Incompatible option detected: %s=%d, %s!\n", "enable_guc", i915_modparams.enable_guc, -!HAS_HUC(dev_priv) ? "no HuC hardware" : - "no HuC firmware"); +!HAS_HUC(i915) ? "no HuC hardware" : + "no HuC firmware"); } /* A negative value means "use platform/config default" */ if (i915_modparams.guc_log_level < 0) i915_modparams.guc_log_level = - __get_default_guc_log_level(dev_priv); + __get_default_guc_log_level(i915); if (i915_modparams.guc_log_level > 0 && !intel_uc_is_using_guc()) { DRM_WARN("Incompatible option detected: %s=%d, %s!\n", "guc_log_level", i915_modparams.guc_log_level, -!HAS_GUC(dev_priv) ? "no GuC hardware" : - "GuC not enabled"); +!HAS_GUC(i915) ? "no GuC hardware" : + "GuC not enabled"); i
[Intel-gfx] [PATCH v8 12/12] HAX: Enable GuC for CI
Signed-off-by: Michal Wajdeczko --- drivers/gpu/drm/i915/i915_params.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index c963603..53037b5 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -47,7 +47,7 @@ param(int, disable_power_well, -1) \ param(int, enable_ips, 1) \ param(int, invert_brightness, 0) \ - param(int, enable_guc, 0) \ + param(int, enable_guc, -1) \ param(int, guc_log_level, -1) \ param(char *, guc_firmware_path, NULL) \ param(char *, huc_firmware_path, NULL) \ -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 10/12] drm/i915/uc: Use helper functions to detect fw load status
We don't have to check load status values. Signed-off-by: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Chris Wilson Reviewed-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/intel_huc.c | 2 +- drivers/gpu/drm/i915/intel_uc.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c index 2912852..975ae61 100644 --- a/drivers/gpu/drm/i915/intel_huc.c +++ b/drivers/gpu/drm/i915/intel_huc.c @@ -51,7 +51,7 @@ int intel_huc_auth(struct intel_huc *huc) u32 status; int ret; - if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) + if (!intel_uc_fw_is_loaded(&huc->fw)) return -ENOEXEC; vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0, diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 7862731..ed39273 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -459,7 +459,7 @@ int intel_uc_suspend(struct drm_i915_private *i915) if (!USES_GUC(i915)) return 0; - if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) + if (!intel_guc_is_loaded(guc)) return 0; err = intel_guc_suspend(guc); @@ -481,7 +481,7 @@ int intel_uc_resume(struct drm_i915_private *i915) if (!USES_GUC(i915)) return 0; - if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS) + if (!intel_guc_is_loaded(guc)) return 0; gen9_enable_guc_interrupts(i915); -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t v6] intel-gpu-top: Rewrite the tool to be safe to use
[Adding some people to Cc for more ack/nack type feedback.] Executive question is ack or nack on replacing intel_gpu_top with a new implementation which uses only perf PMU for counter gathering. A short history on how this came to be: There was a recent external patch contribution from Rinat Ibragimov to support more platforms from the existing intel_gpu_top. But as the tool is not safe to use Chris Wilson suggested to maybe just replace it. As it happens I had a good start to do this quickly and cheaply, in the form of one prototype I did recently, which only needed ripping some bits out, and polishing the rest. Eero and Rinat kindly did a lot of platform coverage testing and the rewrite seems ready for next steps. I need to stress that as the commit notes, the new tool has a slightly different scope as that it doesn't expose GPU functional level data, but only overall stats like power, frequencies, RC6, interrupts, IMC memory bandwidth and per command streamer busyness, mi_semaphore and mi_event waits. My thinking was that for more functional level profiling gpu-top (OA) should be used. Also the "run a command" and CSV output features are not not supported since both can be done directly via perf stat. Regards, Tvrtko On 04/04/2018 16:26, Tvrtko Ursulin wrote: From: Tvrtko Ursulin intel-gpu-top is a dangerous tool which can hang machines due unsafe mmio register access. This patch rewrites it to use only PMU. Only overall command streamer busyness and GPU global data such as power and frequencies are included in this new version. For access to more GPU functional unit level data, an OA metric based tool like gpu-top should be used instead. v2: * Sort engines by class and instance. * Do not wait for one sampling period to display something on screen. * Move code out of the asserts. (Rinat Ibragimov) * Continuously adapt to terminal size. (Rinat Ibragimov) v3: * Change layout and precision of some field. (Chris Wilson) Eero Tamminen: * Use more user friendly engine names. * Don't error out if a counter is missing. * Add IMC read/write bandwidth. * Report minimum required kernel version. v4: * Really support 4.16 by skipping of missing engines. * Simpler and less hacky float printing. * Preserve copyright header. (Antonio Argenziano) * Simplify engines_ptr macro. (Rinat Ibragimov) v5: * Get RAPL unit from sysfs. * Consolidate sysfs paths with a macro. * Tidy error handling by carrying over and reporting errno. * Check against console height on all prints. * More readable minimum kernel version message. (Eero Tamminen) * Column banner for per engine stats. (Eero Tamminen) v6: * Man page update. (Eero Tamminen) Signed-off-by: Tvrtko Ursulin Cc: Chris Wilson Cc: Lionel Landwerlin Cc: Petri Latvala Cc: Eero Tamminen Cc: Rinat Ibragimov Reviewed-by: Lionel Landwerlin # v1 Reviewed-by: Chris Wilson # v0.5 --- lib/igt_perf.c|6 + lib/igt_perf.h|1 + man/intel_gpu_top.rst | 41 +- tools/Makefile.am |2 + tools/intel_gpu_top.c | 1250 +++-- tools/meson.build |6 +- 6 files changed, 719 insertions(+), 587 deletions(-) diff --git a/lib/igt_perf.c b/lib/igt_perf.c index 99d82ea51c9b..e3dec2cc29c7 100644 --- a/lib/igt_perf.c +++ b/lib/igt_perf.c @@ -69,3 +69,9 @@ int igt_perf_open(uint64_t type, uint64_t config) return _perf_open(type, config, -1, PERF_FORMAT_TOTAL_TIME_ENABLED); } + +int igt_perf_open_group(uint64_t type, uint64_t config, int group) +{ + return _perf_open(type, config, group, + PERF_FORMAT_TOTAL_TIME_ENABLED | PERF_FORMAT_GROUP); +} diff --git a/lib/igt_perf.h b/lib/igt_perf.h index 614ea5d23fa6..e00718f4769a 100644 --- a/lib/igt_perf.h +++ b/lib/igt_perf.h @@ -55,5 +55,6 @@ uint64_t i915_type_id(void); int perf_i915_open(uint64_t config); int perf_i915_open_group(uint64_t config, int group); int igt_perf_open(uint64_t type, uint64_t config); +int igt_perf_open_group(uint64_t type, uint64_t config, int group); #endif /* I915_PERF_H */ diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst index a5f7175bb1a0..19c712307d28 100644 --- a/man/intel_gpu_top.rst +++ b/man/intel_gpu_top.rst @@ -7,9 +7,9 @@ Display a top-like summary of Intel GPU usage - .. include:: defs.rst :Author: IGT Developers -:Date: 2016-03-01 +:Date: 2018-04-04 :Version: |PACKAGE_STRING| -:Copyright: 2009,2011,2012,2016 Intel Corporation +:Copyright: 2009,2011,2012,2016,2018 Intel Corporation :Manual section: |MANUAL_SECTION| :Manual group: |MANUAL_GROUP| @@ -21,42 +21,25 @@ SYNOPSIS DESCRIPTION === -**intel_gpu_top** is a tool to display usage information of an Intel GPU. It -requires root privilege to map the graphics device. +**intel_gpu_top** is a tool to display usage information on Intel GPU
[Intel-gfx] [PATCH] drm/i915/gen9_lp: Increase DDI PHY0 power well enabling timeout
On GLK sporadic timeouts occur during PHY0 enabling. Based on logs it looks like they happen sometime after a system suspend/resume cycle, with the same power well enabling succeeding both before and after the failed one and no other problems observed. The current timeout in the code is not actually specified by BSpec, so let's try to increase that until a BSpec update. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105771 Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/intel_dpio_phy.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c b/drivers/gpu/drm/i915/intel_dpio_phy.c index c8e9e44e5981..00b3ab656b06 100644 --- a/drivers/gpu/drm/i915/intel_dpio_phy.c +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c @@ -380,13 +380,14 @@ static void _bxt_ddi_phy_init(struct drm_i915_private *dev_priv, * all 1s. Eventually they become accessible as they power up, then * the reserved bit will give the default 0. Poll on the reserved bit * becoming 0 to find when the PHY is accessible. -* HW team confirmed that the time to reach phypowergood status is -* anywhere between 50 us and 100us. +* The flag should get set in 100us according to the HW team, but +* use 1ms due to occasional timeouts observed with that. */ - if (wait_for_us(((I915_READ(BXT_PORT_CL1CM_DW0(phy)) & - (PHY_RESERVED | PHY_POWER_GOOD)) == PHY_POWER_GOOD), 100)) { + if (intel_wait_for_register_fw(dev_priv, BXT_PORT_CL1CM_DW0(phy), + PHY_RESERVED | PHY_POWER_GOOD, + PHY_POWER_GOOD, + 1)) DRM_ERROR("timeout during PHY%d power on\n", phy); - } /* Program PLL Rcomp code offset */ val = I915_READ(BXT_PORT_CL1CM_DW9(phy)); -- 2.13.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v8 06/12] drm/i915: Add i915_gem_fini_hw to i915_reset
Quoting Michal Wajdeczko (2018-04-09 13:23:26) > By calling in i915_reset only i915_gem_init_hw without previous > i915_gem_fini_hw we introduced asymmetry. Let's fix that. > > Signed-off-by: Michal Wajdeczko > Cc: Sagar Arun Kamble > Cc: Chris Wilson Reviewed-by: Chris Wilson > --- > drivers/gpu/drm/i915/i915_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 854b26c..a0a5af0 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1902,6 +1902,8 @@ void i915_reset(struct drm_i915_private *i915, > goto error; > } > > + i915_gem_fini_hw(i915); > + > for (i = 0; i < 3; i++) { > ret = intel_gpu_reset(i915, ALL_ENGINES); > if (ret == 0) I still have a feeling that i915_gem_reset() will cause trouble. Hmm, the wedged -> recovery path should be triggering the submission from inside i915_gem_reset. So it should be exploding already... I think where we use GEM_BUG_ON(!gt.awake) in execlists, we want a GEM_BUG_ON(!irq_pinned) in guc_submission_tasklet(). -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/guc: Check that the breadcrumb irq is enabled
Our execlists emulation for GuC requires use of the breadcrumb following every request as a simulcrum for the context-switch interrupt, which we then use to drive the submission tasklet. Therefore, when we unpark the engine for use with the GuC, we pin the breadcrumb interrupt to keep it enabled for the duration. This has to be remain so across all resets, wedging and resume, so check we do have the irq enabled when we start submitting requests to the GuC and on all submissions thereafter. Signed-off-by: Chris Wilson Cc: Michal Wajdeczko Cc: Michał Winiarski --- drivers/gpu/drm/i915/intel_guc_submission.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 97121230656c..a7957b669b68 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -758,6 +758,8 @@ static void guc_submission_tasklet(unsigned long data) struct execlist_port *port = execlists->port; struct i915_request *rq; + GEM_BUG_ON(!READ_ONCE(engine->breadcrumbs.irq_enabled)); + rq = port_request(port); while (rq && i915_request_completed(rq)) { trace_i915_request_out(rq); -- 2.17.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: Remove last references to drm_atomic_get_existing* macros
All the references to get_existing_state can be converted to get_new_state or get_old_state, which means that i915 is now get_existing_state free. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_display.c | 51 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d42b635c6807..487a6e235222 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5148,8 +5148,8 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) intel_atomic_get_new_crtc_state(to_intel_atomic_state(old_state), crtc); struct drm_plane *primary = crtc->base.primary; - struct drm_plane_state *old_pri_state = - drm_atomic_get_existing_plane_state(old_state, primary); + struct drm_plane_state *old_primary_state = + drm_atomic_get_old_plane_state(old_state, primary); intel_frontbuffer_flip(to_i915(crtc->base.dev), pipe_config->fb_bits); @@ -5159,19 +5159,16 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) if (hsw_post_update_enable_ips(old_crtc_state, pipe_config)) hsw_enable_ips(pipe_config); - if (old_pri_state) { - struct intel_plane_state *primary_state = - intel_atomic_get_new_plane_state(to_intel_atomic_state(old_state), - to_intel_plane(primary)); - struct intel_plane_state *old_primary_state = - to_intel_plane_state(old_pri_state); - struct drm_framebuffer *fb = primary_state->base.fb; + if (old_primary_state) { + struct drm_plane_state *new_primary_state = + drm_atomic_get_new_plane_state(old_state, primary); + struct drm_framebuffer *fb = new_primary_state->fb; intel_fbc_post_update(crtc); - if (primary_state->base.visible && + if (new_primary_state->visible && (needs_modeset(&pipe_config->base) || -!old_primary_state->base.visible)) +!old_primary_state->visible)) intel_post_enable_primary(&crtc->base, pipe_config); /* Display WA 827 */ @@ -5192,8 +5189,8 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, struct drm_i915_private *dev_priv = to_i915(dev); struct drm_atomic_state *old_state = old_crtc_state->base.state; struct drm_plane *primary = crtc->base.primary; - struct drm_plane_state *old_pri_state = - drm_atomic_get_existing_plane_state(old_state, primary); + struct drm_plane_state *old_primary_state = + drm_atomic_get_old_plane_state(old_state, primary); bool modeset = needs_modeset(&pipe_config->base); struct intel_atomic_state *old_intel_state = to_intel_atomic_state(old_state); @@ -5201,13 +5198,11 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, if (hsw_pre_update_disable_ips(old_crtc_state, pipe_config)) hsw_disable_ips(old_crtc_state); - if (old_pri_state) { - struct intel_plane_state *primary_state = + if (old_primary_state) { + struct intel_plane_state *new_primary_state = intel_atomic_get_new_plane_state(old_intel_state, to_intel_plane(primary)); - struct intel_plane_state *old_primary_state = - to_intel_plane_state(old_pri_state); - struct drm_framebuffer *fb = primary_state->base.fb; + struct drm_framebuffer *fb = new_primary_state->base.fb; /* Display WA 827 */ if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) || @@ -5216,13 +5211,13 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, skl_wa_clkgate(dev_priv, crtc->pipe, true); } - intel_fbc_pre_update(crtc, pipe_config, primary_state); + intel_fbc_pre_update(crtc, pipe_config, new_primary_state); /* * Gen2 reports pipe underruns whenever all planes are disabled. * So disable underrun reporting before all the planes get disabled. */ - if (IS_GEN2(dev_priv) && old_primary_state->base.visible && - (modeset || !primary_state->base.visible)) + if (IS_GEN2(dev_priv) && old_primary_state->visible && + (modeset || !new_primary_state->base.visible)) intel_set_cpu_fifo_unde
[Intel-gfx] [PATCH 1/4] drm/i915: Change use get_new_plane_state instead of existing plane state
The get_existing macros are deprecated and should be replaced by get_old/new_state for clarity. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_atomic.c | 5 +++-- drivers/gpu/drm/i915/intel_drv.h| 11 --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index bb8c1687823e..40285d1b91b7 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -227,6 +227,7 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, struct intel_crtc_scaler_state *scaler_state = &crtc_state->scaler_state; struct drm_atomic_state *drm_state = crtc_state->base.state; + struct intel_atomic_state *intel_state = to_intel_atomic_state(drm_state); int num_scalers_need; int i, j; @@ -304,8 +305,8 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, continue; } - plane_state = intel_atomic_get_existing_plane_state(drm_state, - intel_plane); + plane_state = intel_atomic_get_new_plane_state(intel_state, + intel_plane); scaler_id = &plane_state->scaler_id; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index b2e0fa04ef5b..e545aa673bd9 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -2109,17 +2109,6 @@ intel_atomic_get_existing_crtc_state(struct drm_atomic_state *state, return NULL; } -static inline struct intel_plane_state * -intel_atomic_get_existing_plane_state(struct drm_atomic_state *state, - struct intel_plane *plane) -{ - struct drm_plane_state *plane_state; - - plane_state = drm_atomic_get_existing_plane_state(state, &plane->base); - - return to_intel_plane_state(plane_state); -} - int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 007a12ebe725..fb24b44ec37f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5037,7 +5037,7 @@ skl_ddb_add_affected_planes(struct intel_crtc_state *cstate) struct drm_plane *plane; enum pipe pipe = intel_crtc->pipe; - WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc)); + WARN_ON(!drm_atomic_get_new_crtc_state(state, crtc)); drm_for_each_plane_mask(plane, dev, cstate->base.plane_mask) { enum plane_id plane_id = to_intel_plane(plane)->id; -- 2.16.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: Enable display workaround 827 for all planes.
The workaround was applied only to the primary plane, but is required on all planes. Iterate over all planes in the crtc atomic check to see if the workaround is enabled, and only perform the actual toggling in the pre/post plane update functions. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_display.c | 40 +--- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 487a6e235222..829b593a3cee 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5162,7 +5162,6 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) if (old_primary_state) { struct drm_plane_state *new_primary_state = drm_atomic_get_new_plane_state(old_state, primary); - struct drm_framebuffer *fb = new_primary_state->fb; intel_fbc_post_update(crtc); @@ -5170,15 +5169,11 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) (needs_modeset(&pipe_config->base) || !old_primary_state->visible)) intel_post_enable_primary(&crtc->base, pipe_config); - - /* Display WA 827 */ - if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) || - IS_CANNONLAKE(dev_priv)) { - if (fb && fb->format->format == DRM_FORMAT_NV12) - skl_wa_clkgate(dev_priv, crtc->pipe, false); - } - } + + /* Display WA 827 */ + if (old_crtc_state->nv12_wa && !pipe_config->nv12_wa) + skl_wa_clkgate(dev_priv, crtc->pipe, false); } static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, @@ -5202,14 +5197,6 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, struct intel_plane_state *new_primary_state = intel_atomic_get_new_plane_state(old_intel_state, to_intel_plane(primary)); - struct drm_framebuffer *fb = new_primary_state->base.fb; - - /* Display WA 827 */ - if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) || - IS_CANNONLAKE(dev_priv)) { - if (fb && fb->format->format == DRM_FORMAT_NV12) - skl_wa_clkgate(dev_priv, crtc->pipe, true); - } intel_fbc_pre_update(crtc, pipe_config, new_primary_state); /* @@ -5221,6 +5208,10 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); } + /* Display WA 827 */ + if (!old_crtc_state->nv12_wa && pipe_config->nv12_wa) + skl_wa_clkgate(dev_priv, crtc->pipe, true); + /* * Vblank time updates from the shadow to live plane control register * are blocked if the memory self-refresh mode is active at that @@ -10485,6 +10476,21 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, return ret; } + pipe_config->nv12_wa = false; + + /* Apply display WA 827 if required. */ + if (crtc_state->active && + ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) || +IS_CANNONLAKE(dev_priv))) { + struct drm_plane *plane; + const struct drm_plane_state *plane_state; + + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, crtc_state) + if (plane_state->fb && + plane_state->fb->format->format == DRM_FORMAT_NV12) + pipe_config->nv12_wa = true; + } + if (crtc_state->color_mgmt_changed) { ret = intel_color_check(crtc, crtc_state); if (ret) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 9969309132d0..9c2b7f78d5dd 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -723,6 +723,7 @@ struct intel_crtc_state { bool update_wm_pre, update_wm_post; /* watermarks are updated */ bool fb_changed; /* fb on any of the planes is changed */ bool fifo_changed; /* FIFO split is changed */ + bool nv12_wa; /* Whether display WA 827 needs to be enabled. */ /* Pipe source size (ie. panel fitter input size) * All planes will be positioned inside this space, -- 2.16.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v8 08/12] drm/i915/uc: Fully sanitize uC within intel_uc_fini_hw
Quoting Michal Wajdeczko (2018-04-09 13:23:28) > As we always call intel_uc_sanitize after every call to > intel_uc_fini_hw we may drop redundant call and sanitize > uC from the fini_hw function. > > Signed-off-by: Michal Wajdeczko > Cc: Sagar Arun Kamble > Cc: Chris Wilson Not that it matters, since doing it before losing control or on resume is the same from our pov, but I've always pencilled in sanitize as being done on takeover (i.e. before init). Why do you favour after fini? Gut feeling prefers keeping it as a separate step rather rolling it up into init/fini. But that's just because before we did sanitize elsewhere, we had many strange bugs and those bugs have left their scars (so I like seeing sanitize, it reminds me of dead bugs). -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: Remove get_existing_crtc_state
get_existing_crtc_state is currently unused, get rid of it. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_drv.h | 14 -- 1 file changed, 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index e545aa673bd9..9969309132d0 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -2095,20 +2095,6 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state, return to_intel_crtc_state(crtc_state); } -static inline struct intel_crtc_state * -intel_atomic_get_existing_crtc_state(struct drm_atomic_state *state, -struct intel_crtc *crtc) -{ - struct drm_crtc_state *crtc_state; - - crtc_state = drm_atomic_get_existing_crtc_state(state, &crtc->base); - - if (crtc_state) - return to_intel_crtc_state(crtc_state); - else - return NULL; -} - int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state); -- 2.16.3 ___ 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 [v8,01/12] drm/i915: Park before resetting the submission backend
== Series Details == Series: series starting with [v8,01/12] drm/i915: Park before resetting the submission backend URL : https://patchwork.freedesktop.org/series/41365/ State : success == Summary == Series 41365v1 series starting with [v8,01/12] drm/i915: Park before resetting the submission backend https://patchwork.freedesktop.org/api/1.0/series/41365/revisions/1/mbox/ Known issues: Test gem_mmap_gtt: Subgroup basic-small-bo-tiledx: fail -> PASS (fi-gdg-551) fdo#102575 Test kms_flip: Subgroup basic-flip-vs-wf_vblank: pass -> FAIL (fi-cfl-s3) fdo#100368 Test prime_vgem: Subgroup basic-fence-flip: fail -> PASS (fi-ilk-650) fdo#104008 fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:435s fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:447s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:380s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:541s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:296s fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:513s fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:517s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:526s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:506s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:410s fi-cfl-s3total:285 pass:258 dwarn:0 dfail:0 fail:1 skip:26 time:554s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:511s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:583s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:426s fi-gdg-551 total:285 pass:177 dwarn:0 dfail:0 fail:0 skip:108 time:314s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:536s fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:488s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:409s fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:420s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:471s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:434s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:474s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:461s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:508s fi-pnv-d510 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:670s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:444s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:540s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:498s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:508s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:431s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:443s fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:586s fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:405s 6eef2aa99feb6e37e6252c0a0ddb78966a0b23dd drm-tip: 2018y-04m-09d-11h-44m-50s UTC integration manifest 058d4d58c467 HAX: Enable GuC for CI 2f326fb7 drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c 38d4bebc3c54 drm/i915/uc: Use helper functions to detect fw load status ecd487066952 drm/i915/uc: Use correct error code for GuC initialization failure 174c75f27d90 drm/i915/uc: Fully sanitize uC within intel_uc_fini_hw 8f30a658e3ce drm/i915/guc: Restore symmetric doorbell cleanup 2298ffab92ec drm/i915: Add i915_gem_fini_hw to i915_reset 2d5745d4f14e drm/i915: Add i915_gem_fini_hw to i915_gem_suspend d70c468442f3 drm/i915: Introduce i915_gem_fini_hw for symmetry with i915_gem_init_hw 2ae7ae86b3c4 drm/i915: Move i915_gem_fini to i915_gem.c bcc6baad8bda drm/i915: Correctly handle error path in i915_gem_init_hw 7d8134ca6ba3 drm/i915: Park before resetting the submission backend == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8640/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915: Change use get_new_plane_state instead of existing plane state
On Mon, Apr 09, 2018 at 02:46:53PM +0200, Maarten Lankhorst wrote: > The get_existing macros are deprecated and should be replaced by > get_old/new_state for clarity. > > Signed-off-by: Maarten Lankhorst > --- > drivers/gpu/drm/i915/intel_atomic.c | 5 +++-- > drivers/gpu/drm/i915/intel_drv.h| 11 --- > drivers/gpu/drm/i915/intel_pm.c | 2 +- > 3 files changed, 4 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c > b/drivers/gpu/drm/i915/intel_atomic.c > index bb8c1687823e..40285d1b91b7 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -227,6 +227,7 @@ int intel_atomic_setup_scalers(struct drm_i915_private > *dev_priv, > struct intel_crtc_scaler_state *scaler_state = > &crtc_state->scaler_state; > struct drm_atomic_state *drm_state = crtc_state->base.state; > + struct intel_atomic_state *intel_state = > to_intel_atomic_state(drm_state); > int num_scalers_need; > int i, j; > > @@ -304,8 +305,8 @@ int intel_atomic_setup_scalers(struct drm_i915_private > *dev_priv, > continue; > } > > - plane_state = > intel_atomic_get_existing_plane_state(drm_state, > - > intel_plane); > + plane_state = > intel_atomic_get_new_plane_state(intel_state, > + > intel_plane); > scaler_id = &plane_state->scaler_id; > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index b2e0fa04ef5b..e545aa673bd9 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -2109,17 +2109,6 @@ intel_atomic_get_existing_crtc_state(struct > drm_atomic_state *state, > return NULL; > } > > -static inline struct intel_plane_state * > -intel_atomic_get_existing_plane_state(struct drm_atomic_state *state, > - struct intel_plane *plane) > -{ > - struct drm_plane_state *plane_state; > - > - plane_state = drm_atomic_get_existing_plane_state(state, &plane->base); > - > - return to_intel_plane_state(plane_state); > -} > - > int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, > struct intel_crtc *intel_crtc, > struct intel_crtc_state *crtc_state); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 007a12ebe725..fb24b44ec37f 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -5037,7 +5037,7 @@ skl_ddb_add_affected_planes(struct intel_crtc_state > *cstate) > struct drm_plane *plane; > enum pipe pipe = intel_crtc->pipe; > > - WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc)); > + WARN_ON(!drm_atomic_get_new_crtc_state(state, crtc)); This assert seems rather pointless since we got the drm_atomic_state via cstate->base.state. Anyways, patch is Reviewed-by: Ville Syrjälä > > drm_for_each_plane_mask(plane, dev, cstate->base.plane_mask) { > enum plane_id plane_id = to_intel_plane(plane)->id; > -- > 2.16.3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 2/4] drm/i915: Remove get_existing_crtc_state
On Mon, Apr 09, 2018 at 02:46:54PM +0200, Maarten Lankhorst wrote: > get_existing_crtc_state is currently unused, get rid of it. > > Signed-off-by: Maarten Lankhorst Reviewed-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_drv.h | 14 -- > 1 file changed, 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index e545aa673bd9..9969309132d0 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -2095,20 +2095,6 @@ intel_atomic_get_crtc_state(struct drm_atomic_state > *state, > return to_intel_crtc_state(crtc_state); > } > > -static inline struct intel_crtc_state * > -intel_atomic_get_existing_crtc_state(struct drm_atomic_state *state, > - struct intel_crtc *crtc) > -{ > - struct drm_crtc_state *crtc_state; > - > - crtc_state = drm_atomic_get_existing_crtc_state(state, &crtc->base); > - > - if (crtc_state) > - return to_intel_crtc_state(crtc_state); > - else > - return NULL; > -} > - > int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv, > struct intel_crtc *intel_crtc, > struct intel_crtc_state *crtc_state); > -- > 2.16.3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 3/4] drm/i915: Remove last references to drm_atomic_get_existing* macros
On Mon, Apr 09, 2018 at 02:46:55PM +0200, Maarten Lankhorst wrote: > All the references to get_existing_state can be converted to > get_new_state or get_old_state, which means that i915 is now > get_existing_state free. > > Signed-off-by: Maarten Lankhorst > --- > drivers/gpu/drm/i915/intel_display.c | 51 > > 1 file changed, 23 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index d42b635c6807..487a6e235222 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5148,8 +5148,8 @@ static void intel_post_plane_update(struct > intel_crtc_state *old_crtc_state) > > intel_atomic_get_new_crtc_state(to_intel_atomic_state(old_state), > crtc); > struct drm_plane *primary = crtc->base.primary; > - struct drm_plane_state *old_pri_state = > - drm_atomic_get_existing_plane_state(old_state, primary); > + struct drm_plane_state *old_primary_state = > + drm_atomic_get_old_plane_state(old_state, primary); > > intel_frontbuffer_flip(to_i915(crtc->base.dev), pipe_config->fb_bits); > > @@ -5159,19 +5159,16 @@ static void intel_post_plane_update(struct > intel_crtc_state *old_crtc_state) > if (hsw_post_update_enable_ips(old_crtc_state, pipe_config)) > hsw_enable_ips(pipe_config); > > - if (old_pri_state) { > - struct intel_plane_state *primary_state = > - > intel_atomic_get_new_plane_state(to_intel_atomic_state(old_state), > - > to_intel_plane(primary)); > - struct intel_plane_state *old_primary_state = > - to_intel_plane_state(old_pri_state); > - struct drm_framebuffer *fb = primary_state->base.fb; > + if (old_primary_state) { > + struct drm_plane_state *new_primary_state = > + drm_atomic_get_new_plane_state(old_state, primary); > + struct drm_framebuffer *fb = new_primary_state->fb; > > intel_fbc_post_update(crtc); > > - if (primary_state->base.visible && > + if (new_primary_state->visible && > (needs_modeset(&pipe_config->base) || > - !old_primary_state->base.visible)) > + !old_primary_state->visible)) > intel_post_enable_primary(&crtc->base, pipe_config); > > /* Display WA 827 */ > @@ -5192,8 +5189,8 @@ static void intel_pre_plane_update(struct > intel_crtc_state *old_crtc_state, > struct drm_i915_private *dev_priv = to_i915(dev); > struct drm_atomic_state *old_state = old_crtc_state->base.state; > struct drm_plane *primary = crtc->base.primary; > - struct drm_plane_state *old_pri_state = > - drm_atomic_get_existing_plane_state(old_state, primary); > + struct drm_plane_state *old_primary_state = > + drm_atomic_get_old_plane_state(old_state, primary); > bool modeset = needs_modeset(&pipe_config->base); > struct intel_atomic_state *old_intel_state = > to_intel_atomic_state(old_state); > @@ -5201,13 +5198,11 @@ static void intel_pre_plane_update(struct > intel_crtc_state *old_crtc_state, > if (hsw_pre_update_disable_ips(old_crtc_state, pipe_config)) > hsw_disable_ips(old_crtc_state); > > - if (old_pri_state) { > - struct intel_plane_state *primary_state = > + if (old_primary_state) { > + struct intel_plane_state *new_primary_state = > intel_atomic_get_new_plane_state(old_intel_state, > > to_intel_plane(primary)); > - struct intel_plane_state *old_primary_state = > - to_intel_plane_state(old_pri_state); > - struct drm_framebuffer *fb = primary_state->base.fb; > + struct drm_framebuffer *fb = new_primary_state->base.fb; > > /* Display WA 827 */ > if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) || > @@ -5216,13 +5211,13 @@ static void intel_pre_plane_update(struct > intel_crtc_state *old_crtc_state, > skl_wa_clkgate(dev_priv, crtc->pipe, true); > } > > - intel_fbc_pre_update(crtc, pipe_config, primary_state); > + intel_fbc_pre_update(crtc, pipe_config, new_primary_state); > /* >* Gen2 reports pipe underruns whenever all planes are disabled. >* So disable underrun reporting before all the planes get > disabled. >*/ > - if (IS_GEN2(dev_priv) && old_primary_state->base.visible && > - (modeset || !primary_state->base.visible)) > + if (IS_GEN2(dev_priv) &
[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [01/18] drm/i915/execlists: Set queue priority from secondary port
== Series Details == Series: series starting with [01/18] drm/i915/execlists: Set queue priority from secondary port URL : https://patchwork.freedesktop.org/series/41357/ State : failure == Summary == Possible new issues: Test gem_ctx_param: Subgroup invalid-param-get: pass -> FAIL (shard-apl) pass -> FAIL (shard-hsw) Subgroup invalid-param-set: pass -> FAIL (shard-apl) pass -> FAIL (shard-hsw) Test gem_pwrite: Subgroup big-gtt-backwards: skip -> PASS (shard-apl) Known issues: Test kms_flip: Subgroup 2x-flip-vs-expired-vblank-interruptible: fail -> PASS (shard-hsw) fdo#102887 +1 Subgroup plain-flip-fb-recreate-interruptible: pass -> FAIL (shard-hsw) fdo#100368 +2 fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 shard-apltotal:2680 pass:1833 dwarn:1 dfail:1 fail:9 skip:836 time:12708s shard-hswtotal:2680 pass:1781 dwarn:1 dfail:0 fail:6 skip:891 time:11341s Blacklisted hosts: shard-kbltotal:2680 pass:1960 dwarn:1 dfail:1 fail:9 skip:709 time:9215s shard-snbtotal:2680 pass:1375 dwarn:1 dfail:0 fail:5 skip:1299 time:6916s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8638/shards.html ___ 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: Enable display workaround 827 for all planes.
On Mon, Apr 09, 2018 at 02:46:56PM +0200, Maarten Lankhorst wrote: > The workaround was applied only to the primary plane, but is required > on all planes. Iterate over all planes in the crtc atomic check to see > if the workaround is enabled, and only perform the actual toggling in > the pre/post plane update functions. > > Signed-off-by: Maarten Lankhorst > --- > drivers/gpu/drm/i915/intel_display.c | 40 > +--- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 2 files changed, 24 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 487a6e235222..829b593a3cee 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5162,7 +5162,6 @@ static void intel_post_plane_update(struct > intel_crtc_state *old_crtc_state) > if (old_primary_state) { > struct drm_plane_state *new_primary_state = > drm_atomic_get_new_plane_state(old_state, primary); > - struct drm_framebuffer *fb = new_primary_state->fb; > > intel_fbc_post_update(crtc); > > @@ -5170,15 +5169,11 @@ static void intel_post_plane_update(struct > intel_crtc_state *old_crtc_state) > (needs_modeset(&pipe_config->base) || >!old_primary_state->visible)) > intel_post_enable_primary(&crtc->base, pipe_config); > - > - /* Display WA 827 */ > - if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) || > - IS_CANNONLAKE(dev_priv)) { > - if (fb && fb->format->format == DRM_FORMAT_NV12) > - skl_wa_clkgate(dev_priv, crtc->pipe, false); > - } > - > } > + > + /* Display WA 827 */ > + if (old_crtc_state->nv12_wa && !pipe_config->nv12_wa) > + skl_wa_clkgate(dev_priv, crtc->pipe, false); > } > > static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > @@ -5202,14 +5197,6 @@ static void intel_pre_plane_update(struct > intel_crtc_state *old_crtc_state, > struct intel_plane_state *new_primary_state = > intel_atomic_get_new_plane_state(old_intel_state, > > to_intel_plane(primary)); > - struct drm_framebuffer *fb = new_primary_state->base.fb; > - > - /* Display WA 827 */ > - if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) || > - IS_CANNONLAKE(dev_priv)) { > - if (fb && fb->format->format == DRM_FORMAT_NV12) > - skl_wa_clkgate(dev_priv, crtc->pipe, true); > - } > > intel_fbc_pre_update(crtc, pipe_config, new_primary_state); > /* > @@ -5221,6 +5208,10 @@ static void intel_pre_plane_update(struct > intel_crtc_state *old_crtc_state, > intel_set_cpu_fifo_underrun_reporting(dev_priv, > crtc->pipe, false); > } > > + /* Display WA 827 */ > + if (!old_crtc_state->nv12_wa && pipe_config->nv12_wa) > + skl_wa_clkgate(dev_priv, crtc->pipe, true); > + > /* >* Vblank time updates from the shadow to live plane control register >* are blocked if the memory self-refresh mode is active at that > @@ -10485,6 +10476,21 @@ static int intel_crtc_atomic_check(struct drm_crtc > *crtc, > return ret; > } > > + pipe_config->nv12_wa = false; > + > + /* Apply display WA 827 if required. */ > + if (crtc_state->active && > + ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) || > + IS_CANNONLAKE(dev_priv))) { GLK doesn't need this? That seems odd to say the least. > + struct drm_plane *plane; > + const struct drm_plane_state *plane_state; > + > + drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, > crtc_state) I'd prefer a bitmask instead of that sneaky peeking of the plane states. It should also avoid polluting this sort of central/generic piece of code with platform specific w/as. > + if (plane_state->fb && > + plane_state->fb->format->format == DRM_FORMAT_NV12) > + pipe_config->nv12_wa = true; > + } > + > if (crtc_state->color_mgmt_changed) { > ret = intel_color_check(crtc, crtc_state); > if (ret) > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 9969309132d0..9c2b7f78d5dd 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -723,6 +723,7 @@ struct intel_crtc_state { > bool update_wm_pre, update_wm_post; /* watermarks are updated */ > bool fb_changed; /* fb on any of the planes is changed */ > bool fifo_changed; /* FIFO spli
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gen9_lp: Increase DDI PHY0 power well enabling timeout
== Series Details == Series: drm/i915/gen9_lp: Increase DDI PHY0 power well enabling timeout URL : https://patchwork.freedesktop.org/series/41366/ State : success == Summary == Series 41366v1 drm/i915/gen9_lp: Increase DDI PHY0 power well enabling timeout https://patchwork.freedesktop.org/api/1.0/series/41366/revisions/1/mbox/ Known issues: Test gem_mmap_gtt: Subgroup basic-small-bo-tiledx: pass -> FAIL (fi-gdg-551) fdo#102575 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-c: pass -> INCOMPLETE (fi-bxt-dsi) fdo#103927 Test prime_vgem: Subgroup basic-fence-flip: pass -> FAIL (fi-ilk-650) fdo#104008 fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:430s fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:446s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:382s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:536s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:299s fi-bxt-dsi total:243 pass:216 dwarn:0 dfail:0 fail:0 skip:26 fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:512s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:521s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:508s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:409s fi-cfl-s3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:562s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:511s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:587s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:425s fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:317s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:539s fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:487s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:411s fi-ilk-650 total:285 pass:224 dwarn:0 dfail:0 fail:1 skip:60 time:421s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:481s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:433s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:472s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:463s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:519s fi-pnv-d510 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:668s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:446s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:531s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:500s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:504s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:430s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:442s fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:572s fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:406s e5a01dd0c5d224beec064e40184cc63a82ae79ce drm-tip: 2018y-04m-09d-12h-37m-09s UTC integration manifest 031e10c34d8a drm/i915/gen9_lp: Increase DDI PHY0 power well enabling timeout == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8641/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915/selftests: Extend partial vma coverage to check parallel creation
== Series Details == Series: series starting with [1/2] drm/i915/selftests: Extend partial vma coverage to check parallel creation URL : https://patchwork.freedesktop.org/series/41359/ State : failure == Summary == Possible new issues: Test drv_selftest: Subgroup mock_vma: pass -> DMESG-FAIL (shard-hsw) Test gem_pwrite: Subgroup big-gtt-backwards: skip -> PASS (shard-apl) Known issues: Test kms_flip: Subgroup 2x-flip-vs-expired-vblank-interruptible: fail -> PASS (shard-hsw) fdo#102887 Subgroup 2x-plain-flip-fb-recreate: fail -> PASS (shard-hsw) fdo#100368 Subgroup modeset-vs-vblank-race-interruptible: pass -> FAIL (shard-hsw) fdo#103060 +1 Test kms_sysfs_edid_timing: pass -> WARN (shard-apl) fdo#100047 fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060 fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047 shard-apltotal:2680 pass:1835 dwarn:1 dfail:0 fail:7 skip:836 time:12777s shard-hswtotal:2680 pass:1783 dwarn:1 dfail:1 fail:3 skip:891 time:11456s Blacklisted hosts: shard-kbltotal:2680 pass:1960 dwarn:1 dfail:0 fail:9 skip:710 time:9198s shard-snbtotal:2680 pass:1377 dwarn:1 dfail:0 fail:3 skip:1299 time:6941s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8639/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/36] drm/i915: Reduce RPS update frequency on Valleyview/Cherryview
Quoting Sagar Arun Kamble (2018-03-15 09:23:25) > > > On 3/14/2018 3:07 PM, Chris Wilson wrote: > > Valleyview and Cherryview update the GPU frequency via the punit, which > > is very expensive as we have to ensure the cores do not sleep during the > > comms. > But the patch 5 applies this workaround to only VLV. Still using an indirect method that uses a RTT, so still true that the punit access is noticeable. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/36] drm/i915: Replace pcu_lock with sb_lock
Quoting Sagar Arun Kamble (2018-03-15 12:06:57) > On 3/14/2018 3:07 PM, Chris Wilson wrote: > > struct intel_rps { > > + struct mutex lock; > > + > I think this lock can now become part of struct intel_gt_pm. Maybe, haven't decided yet. Anything but rps is so infrequent as not to really matter... And rps by the same metric deserves its own locking. > > /* > >* work, interrupts_enabled and pm_iir are protected by > >* dev_priv->irq_lock > > @@ -1783,14 +1785,6 @@ struct drm_i915_private { > > /* Cannot be determined by PCIID. You must always read a register. */ > > u32 edram_cap; > > > > - /* > > - * Protects RPS/RC6 register access and PCU communication. > > - * Must be taken after struct_mutex if nested. Note that > > - * this lock may be held for long periods of time when > > - * talking to hw - so only take it when talking to hw! > > - */ > > - struct mutex pcu_lock; > > - > > /* gen6+ GT PM state */ > > struct intel_gen6_power_mgmt gt_pm; > > > ... > > -int sandybridge_pcode_write_timeout(struct drm_i915_private *dev_priv, > > - u32 mbox, u32 val, > > - int fast_timeout_us, int slow_timeout_ms) > > +static int __sandybridge_pcode_write_timeout(struct drm_i915_private > > *dev_priv, > > + u32 mbox, u32 val, > > + int fast_timeout_us, > > + int slow_timeout_ms) > > { > > int status; > > > > - WARN_ON(!mutex_is_locked(&dev_priv->pcu_lock)); > > - > lockdep_assert is missed here. Because it is now static with its only pair of users immediately after, so easy to verify both callers take the sb_lock (pair when we reduce this to the common rw routine). -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/psr: vbt change for psr
On Fri, 06 Apr 2018, Rodrigo Vivi wrote: > On Fri, Apr 06, 2018 at 10:58:51PM +0530, vathsala nagaraju wrote: >> From: Vathsala Nagaraju >> >> For psr block #9, the vbt description has moved to options [0-3] for >> TP1,TP2,TP3 Wakeup time from decimal value without any change to vbt >> structure. Since spec does not mention from which VBT version this >> change was added to vbt.bsf file, we cannot depend on bdb->version check >> to change for all the platforms. >> >> There is RCR inplace for GOP team to provide the version number >> to make generic change. Since Kabylake with bdb version 209 is having this >> change, limiting this change to kbl and version 209+ to unblock google. >> >> bspec 20131 >> >> Cc: Rodrigo Vivi >> CC: Puthikorn Voravootivat >> >> Signed-off-by: Maulik V Vaghela >> Signed-off-by: Vathsala Nagaraju >> --- >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> drivers/gpu/drm/i915/intel_bios.c | 2 +- >> drivers/gpu/drm/i915/intel_psr.c | 84 >> ++- >> 3 files changed, 59 insertions(+), 28 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index 5373b17..a47be19b 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -1075,6 +1075,7 @@ struct intel_vbt_data { >> enum psr_lines_to_wait lines_to_wait; >> int tp1_wakeup_time; >> int tp2_tp3_wakeup_time; >> +int bdb_version; > > please keep the vbt stuff inside intel_bios.c > > so there at intel_bios.c you parse the vbt and based on the vbt version > you export in a standard way to intel_psr.c Exactly! struct intel_vbt_data is supposed to be an abstraction. BR, Jani. > >> } psr; >> >> struct { >> diff --git a/drivers/gpu/drm/i915/intel_bios.c >> b/drivers/gpu/drm/i915/intel_bios.c >> index c5c7530..cfefd32 100644 >> --- a/drivers/gpu/drm/i915/intel_bios.c >> +++ b/drivers/gpu/drm/i915/intel_bios.c >> @@ -658,7 +658,7 @@ static int intel_bios_ssc_frequency(struct >> drm_i915_private *dev_priv, >> DRM_DEBUG_KMS("No PSR BDB found.\n"); >> return; >> } >> - >> +dev_priv->vbt.psr.bdb_version = bdb->version; >> psr_table = &psr->psr_table[panel_type]; >> >> dev_priv->vbt.psr.full_link = psr_table->full_link; >> diff --git a/drivers/gpu/drm/i915/intel_psr.c >> b/drivers/gpu/drm/i915/intel_psr.c >> index 2d53f73..e470d5e 100644 >> --- a/drivers/gpu/drm/i915/intel_psr.c >> +++ b/drivers/gpu/drm/i915/intel_psr.c >> @@ -353,24 +353,45 @@ static void hsw_activate_psr1(struct intel_dp >> *intel_dp) >> if (dev_priv->psr.link_standby) >> val |= EDP_PSR_LINK_STANDBY; >> >> -if (dev_priv->vbt.psr.tp1_wakeup_time > 5) >> -val |= EDP_PSR_TP1_TIME_2500us; >> -else if (dev_priv->vbt.psr.tp1_wakeup_time > 1) >> -val |= EDP_PSR_TP1_TIME_500us; >> -else if (dev_priv->vbt.psr.tp1_wakeup_time > 0) >> -val |= EDP_PSR_TP1_TIME_100us; >> -else >> -val |= EDP_PSR_TP1_TIME_0us; >> - >> -if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5) >> -val |= EDP_PSR_TP2_TP3_TIME_2500us; >> -else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1) >> -val |= EDP_PSR_TP2_TP3_TIME_500us; >> -else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0) >> -val |= EDP_PSR_TP2_TP3_TIME_100us; >> -else >> -val |= EDP_PSR_TP2_TP3_TIME_0us; >> +if (dev_priv->vbt.psr.bdb_version >= 209 && IS_KABYLAKE(dev_priv)) { >> +if (dev_priv->vbt.psr.tp1_wakeup_time == 0) >> +val |= EDP_PSR_TP1_TIME_500us; >> +else if (dev_priv->vbt.psr.tp1_wakeup_time == 1) >> +val |= EDP_PSR_TP1_TIME_100us; >> +else if (dev_priv->vbt.psr.tp1_wakeup_time == 2) >> +val |= EDP_PSR_TP1_TIME_2500us; >> +else >> +val |= EDP_PSR_TP1_TIME_0us; >> +} else { >> +if (dev_priv->vbt.psr.tp1_wakeup_time > 5) >> +val |= EDP_PSR_TP1_TIME_2500us; >> +else if (dev_priv->vbt.psr.tp1_wakeup_time > 1) >> +val |= EDP_PSR_TP1_TIME_500us; >> +else if (dev_priv->vbt.psr.tp1_wakeup_time > 0) >> +val |= EDP_PSR_TP1_TIME_100us; >> +else >> +val |= EDP_PSR_TP1_TIME_0us; >> +} >> >> +if (dev_priv->vbt.psr.bdb_version >= 209 && IS_KABYLAKE(dev_priv)) { >> +if (dev_priv->vbt.psr.tp2_tp3_wakeup_time == 0) >> +val |= EDP_PSR_TP2_TP3_TIME_500us; >> +else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time == 1) >> +val |= EDP_PSR_TP2_TP3_TIME_100us; >> +else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time == 2) >> +val |= EDP_PSR_TP2_TP3_TIME_2500us; >> +else >> +val |= EDP_PSR_TP2_TP3_TIM
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/guc: Check that the breadcrumb irq is enabled
== Series Details == Series: drm/i915/guc: Check that the breadcrumb irq is enabled URL : https://patchwork.freedesktop.org/series/41368/ State : success == Summary == Series 41368v1 drm/i915/guc: Check that the breadcrumb irq is enabled https://patchwork.freedesktop.org/api/1.0/series/41368/revisions/1/mbox/ Known issues: Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-c: pass -> INCOMPLETE (fi-bxt-dsi) fdo#103927 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:427s fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:441s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:380s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:537s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:295s fi-bxt-dsi total:243 pass:216 dwarn:0 dfail:0 fail:0 skip:26 fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:515s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:525s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:509s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:410s fi-cfl-s3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:560s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:514s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:586s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:426s fi-gdg-551 total:285 pass:177 dwarn:0 dfail:0 fail:0 skip:108 time:317s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:538s fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:487s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:404s fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:421s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:475s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:435s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:473s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:466s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:511s fi-pnv-d510 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:668s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:439s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:530s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:500s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:505s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:433s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:446s fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:578s fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:402s e5a01dd0c5d224beec064e40184cc63a82ae79ce drm-tip: 2018y-04m-09d-12h-37m-09s UTC integration manifest efbbf2bfa79b drm/i915/guc: Check that the breadcrumb irq is enabled == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8642/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gen9_lp: Increase DDI PHY0 power well enabling timeout
On Mon, Apr 09, 2018 at 03:27:16PM +0300, Imre Deak wrote: > On GLK sporadic timeouts occur during PHY0 enabling. Based on logs it looks > like they happen sometime after a system suspend/resume cycle, with the > same power well enabling succeeding both before and after the failed > one and no other problems observed. The current timeout in the code is > not actually specified by BSpec, so let's try to increase that until a > BSpec update. Looks like we've always used 1ms on CHV. I couldn't find any specific notes on how long we should poll in any of the CHV PHY docs. So I assume we just picked 1ms since it seemed sufficient. Doing the same for BXT/GLK seems reasonable to me. Reviewed-by: Ville Syrjälä > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105771 > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/intel_dpio_phy.c | 11 ++- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dpio_phy.c > b/drivers/gpu/drm/i915/intel_dpio_phy.c > index c8e9e44e5981..00b3ab656b06 100644 > --- a/drivers/gpu/drm/i915/intel_dpio_phy.c > +++ b/drivers/gpu/drm/i915/intel_dpio_phy.c > @@ -380,13 +380,14 @@ static void _bxt_ddi_phy_init(struct drm_i915_private > *dev_priv, >* all 1s. Eventually they become accessible as they power up, then >* the reserved bit will give the default 0. Poll on the reserved bit >* becoming 0 to find when the PHY is accessible. > - * HW team confirmed that the time to reach phypowergood status is > - * anywhere between 50 us and 100us. > + * The flag should get set in 100us according to the HW team, but > + * use 1ms due to occasional timeouts observed with that. >*/ > - if (wait_for_us(((I915_READ(BXT_PORT_CL1CM_DW0(phy)) & > - (PHY_RESERVED | PHY_POWER_GOOD)) == PHY_POWER_GOOD), 100)) { > + if (intel_wait_for_register_fw(dev_priv, BXT_PORT_CL1CM_DW0(phy), > +PHY_RESERVED | PHY_POWER_GOOD, > +PHY_POWER_GOOD, > +1)) > DRM_ERROR("timeout during PHY%d power on\n", phy); > - } > > /* Program PLL Rcomp code offset */ > val = I915_READ(BXT_PORT_CL1CM_DW9(phy)); > -- > 2.13.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 2/2] drm/i915/fbc: Resize CFB in non-full modeset paths
On Fri, 06 Apr 2018, José Roberto de Souza wrote: > A simple page flip can cause the CFB required size to increase and > if it is bigger than the currently allocated CFB it needs to be > resized to activate FBC again. > > Until now this case was not being handled but CI is starting to > get some of this errors. > > So here it will free the old CFB and a try to allocate the required > CFB in the schedule activation work, it will happen after one vblank > so is guarantee that FBC was completed disabled and is not using CFB. > > Also in case that there is no enough stolen memory to allocate the > new CFB it will try 3 times per full modeset as the CFB requirement > could be reduced in the next non-full modeset. > > Cc: Paulo Zanoni > Signed-off-by: José Roberto de Souza > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105683 > --- > drivers/gpu/drm/i915/i915_drv.h | 3 +++ > drivers/gpu/drm/i915/intel_fbc.c | 46 +--- > 2 files changed, 33 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 5373b171bb96..4ce19b45f67d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -566,6 +566,9 @@ struct intel_fbc { > } work; > > const char *no_fbc_reason; > + > + bool cfb_try_resize; > + u8 cfb_resize_tries_left; > }; > > /* > diff --git a/drivers/gpu/drm/i915/intel_fbc.c > b/drivers/gpu/drm/i915/intel_fbc.c > index 573b034a02fd..7d77936db3ec 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -41,6 +41,9 @@ > #include "intel_drv.h" > #include "i915_drv.h" > > +static void __intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv); > +static int intel_fbc_alloc_cfb(struct intel_crtc *crtc); > + > static inline bool fbc_supported(struct drm_i915_private *dev_priv) > { > return HAS_FBC(dev_priv); > @@ -446,6 +449,15 @@ static void intel_fbc_work_fn(struct work_struct *__work) > goto retry; > } > > + if (fbc->cfb_try_resize && fbc->cfb_resize_tries_left) { > + __intel_fbc_cleanup_cfb(dev_priv); > + if (intel_fbc_alloc_cfb(crtc)) { > + fbc->no_fbc_reason = "not enough stolen memory"; > + fbc->cfb_resize_tries_left--; > + goto out; > + } > + } > + > intel_fbc_hw_activate(dev_priv); > > work->scheduled = false; > @@ -850,22 +862,6 @@ static bool intel_fbc_can_activate(struct intel_crtc > *crtc) > return false; > } > > - /* It is possible for the required CFB size change without a > - * crtc->disable + crtc->enable since it is possible to change the > - * stride without triggering a full modeset. Since we try to > - * over-allocate the CFB, there's a chance we may keep FBC enabled even > - * if this happens, but if we exceed the current CFB size we'll have to > - * disable FBC. Notice that it would be possible to disable FBC, wait > - * for a frame, free the stolen node, then try to reenable FBC in case > - * we didn't get any invalidate/deactivate calls, but this would require > - * a lot of tracking just for a specific case. If we conclude it's an > - * important case, we can implement it later. */ > - if (intel_fbc_calculate_cfb_size(dev_priv, &fbc->state_cache) > > - fbc->compressed_fb.size * fbc->threshold) { > - fbc->no_fbc_reason = "CFB requirements changed"; > - return false; > - } > - > /* >* Work around a problem on GEN9+ HW, where enabling FBC on a plane >* having a Y offset that isn't divisible by 4 causes FIFO underrun > @@ -877,6 +873,23 @@ static bool intel_fbc_can_activate(struct intel_crtc > *crtc) > return false; > } > > + /* It is possible for the required CFB size change without a > + * crtc->disable + crtc->enable since it is possible to change the > + * stride without triggering a full modeset. Since we try to > + * over-allocate the CFB, there's a chance we may keep FBC enabled even > + * if this happens, but if we exceed the current CFB size we'll have to > + * resize CFB. > + */ > + if (!drm_mm_node_allocated(&fbc->compressed_fb) || > + (intel_fbc_calculate_cfb_size(dev_priv, &fbc->state_cache) > > + fbc->compressed_fb.size * fbc->threshold)) { > + fbc->cfb_try_resize = true; > + DRM_DEBUG_KMS("CFB requirements have changed, activation \ > + work will try to resize it"); Either the string entirely on one line, or the string split to two, but please never line continuation within the string. BR, Jani. > + } else { > + fbc->cfb_try_resize = false; > + } > + > return true; > } > > @@ -1208,6 +1221,7 @@ void intel_fbc_enable(struct intel_crtc *crtc, >
Re: [Intel-gfx] [PATCH 12/36] drm/i915: Merge sbi read/write into a single accessor
Quoting Sagar Arun Kamble (2018-03-16 03:39:56) > > > On 3/14/2018 3:07 PM, Chris Wilson wrote: > > Since intel_sideband_read and intel_sideband_write differ by only a > > couple of lines (depending on whether we feed the value in or out), > > merge the two into a single common accessor. > > > > Signed-off-by: Chris Wilson > > > -u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, u32 reg) > vlv_flisdsi_read declaration can be removed from sideband.h Oops, no, that was a rebase mistake. The API should not be affected by this patch. That we have unused API.. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: Change use get_new_plane_state instead of existing plane state
== Series Details == Series: series starting with [1/4] drm/i915: Change use get_new_plane_state instead of existing plane state URL : https://patchwork.freedesktop.org/series/41370/ State : warning == Summary == $ dim checkpatch origin/drm-tip 591b58b45998 drm/i915: Change use get_new_plane_state instead of existing plane state 2c8e55a34368 drm/i915: Remove get_existing_crtc_state 78e0056cb594 drm/i915: Remove last references to drm_atomic_get_existing* macros -:142: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #142: FILE: drivers/gpu/drm/i915/intel_display.c:12795: + drm_atomic_get_new_crtc_state(new_state->state, plane->state->crtc); total: 0 errors, 0 warnings, 1 checks, 115 lines checked 90d0615ca37a drm/i915: Enable display workaround 827 for all planes. ___ 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: Remove last references to drm_atomic_get_existing* macros
Op 09-04-18 om 15:04 schreef Ville Syrjälä: > On Mon, Apr 09, 2018 at 02:46:55PM +0200, Maarten Lankhorst wrote: >> All the references to get_existing_state can be converted to >> get_new_state or get_old_state, which means that i915 is now >> get_existing_state free. >> >> Signed-off-by: Maarten Lankhorst >> --- >> drivers/gpu/drm/i915/intel_display.c | 51 >> >> 1 file changed, 23 insertions(+), 28 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index d42b635c6807..487a6e235222 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -5148,8 +5148,8 @@ static void intel_post_plane_update(struct >> intel_crtc_state *old_crtc_state) >> >> intel_atomic_get_new_crtc_state(to_intel_atomic_state(old_state), >> crtc); >> struct drm_plane *primary = crtc->base.primary; >> -struct drm_plane_state *old_pri_state = >> -drm_atomic_get_existing_plane_state(old_state, primary); >> +struct drm_plane_state *old_primary_state = >> +drm_atomic_get_old_plane_state(old_state, primary); >> >> intel_frontbuffer_flip(to_i915(crtc->base.dev), pipe_config->fb_bits); >> >> @@ -5159,19 +5159,16 @@ static void intel_post_plane_update(struct >> intel_crtc_state *old_crtc_state) >> if (hsw_post_update_enable_ips(old_crtc_state, pipe_config)) >> hsw_enable_ips(pipe_config); >> >> -if (old_pri_state) { >> -struct intel_plane_state *primary_state = >> - >> intel_atomic_get_new_plane_state(to_intel_atomic_state(old_state), >> - >> to_intel_plane(primary)); >> -struct intel_plane_state *old_primary_state = >> -to_intel_plane_state(old_pri_state); >> -struct drm_framebuffer *fb = primary_state->base.fb; >> +if (old_primary_state) { >> +struct drm_plane_state *new_primary_state = >> +drm_atomic_get_new_plane_state(old_state, primary); >> +struct drm_framebuffer *fb = new_primary_state->fb; >> >> intel_fbc_post_update(crtc); >> >> -if (primary_state->base.visible && >> +if (new_primary_state->visible && >> (needs_modeset(&pipe_config->base) || >> - !old_primary_state->base.visible)) >> + !old_primary_state->visible)) >> intel_post_enable_primary(&crtc->base, pipe_config); >> >> /* Display WA 827 */ >> @@ -5192,8 +5189,8 @@ static void intel_pre_plane_update(struct >> intel_crtc_state *old_crtc_state, >> struct drm_i915_private *dev_priv = to_i915(dev); >> struct drm_atomic_state *old_state = old_crtc_state->base.state; >> struct drm_plane *primary = crtc->base.primary; >> -struct drm_plane_state *old_pri_state = >> -drm_atomic_get_existing_plane_state(old_state, primary); >> +struct drm_plane_state *old_primary_state = >> +drm_atomic_get_old_plane_state(old_state, primary); >> bool modeset = needs_modeset(&pipe_config->base); >> struct intel_atomic_state *old_intel_state = >> to_intel_atomic_state(old_state); >> @@ -5201,13 +5198,11 @@ static void intel_pre_plane_update(struct >> intel_crtc_state *old_crtc_state, >> if (hsw_pre_update_disable_ips(old_crtc_state, pipe_config)) >> hsw_disable_ips(old_crtc_state); >> >> -if (old_pri_state) { >> -struct intel_plane_state *primary_state = >> +if (old_primary_state) { >> +struct intel_plane_state *new_primary_state = >> intel_atomic_get_new_plane_state(old_intel_state, >> >> to_intel_plane(primary)); >> -struct intel_plane_state *old_primary_state = >> -to_intel_plane_state(old_pri_state); >> -struct drm_framebuffer *fb = primary_state->base.fb; >> +struct drm_framebuffer *fb = new_primary_state->base.fb; >> >> /* Display WA 827 */ >> if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) || >> @@ -5216,13 +5211,13 @@ static void intel_pre_plane_update(struct >> intel_crtc_state *old_crtc_state, >> skl_wa_clkgate(dev_priv, crtc->pipe, true); >> } >> >> -intel_fbc_pre_update(crtc, pipe_config, primary_state); >> +intel_fbc_pre_update(crtc, pipe_config, new_primary_state); >> /* >> * Gen2 reports pipe underruns whenever all planes are disabled. >> * So disable underrun reporting before all the planes get >> disabled. >> */ >> -if (IS_GEN2(dev_priv) && old_primary_state->base.visible && >> -
Re: [Intel-gfx] [PATCH] drm/i915/audio: Fix audio detection issue on GLK
On Sun, 08 Apr 2018, Gaurav K Singh wrote: > On Geminilake, sometimes audio card is not getting > detected after reboot. This is a spurious issue happening on > Geminilake. HW codec and HD audio controller link was going > out of sync for which there was a fix in i915 driver but > was not getting invoked for GLK. Extending this fix to GLK as well. > > Tested by Du,Wenkai on GLK board. > > Signed-off-by: Gaurav K Singh > --- > drivers/gpu/drm/i915/intel_audio.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_audio.c > b/drivers/gpu/drm/i915/intel_audio.c > index 656f6c931341..73b1e0b96f88 100644 > --- a/drivers/gpu/drm/i915/intel_audio.c > +++ b/drivers/gpu/drm/i915/intel_audio.c > @@ -729,7 +729,8 @@ static void > i915_audio_component_codec_wake_override(struct device *kdev, > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > u32 tmp; > > - if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv)) > + if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv) && > + !IS_GEMINILAKE(dev_priv)) That could be written as if (!IS_GEN9_BC(dev_priv) && !IS_GEN9_LP(dev_priv)) which in turn could just be written as if (!IS_GEN9(dev_priv)) ...but since GLK has gen 10 display, so I'm wondering if the same issue will be present in gen 10 too, and whether this should just become if (INTEL_GEN(dev_priv) < 9) BR, Jani. > return; > > i915_audio_component_get_power(kdev); -- 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] [PATCH 15/36] drm/i915: Mark up Ironlake ips with rpm wakerefs
Quoting Sagar Arun Kamble (2018-03-16 04:58:22) > > > On 3/14/2018 3:07 PM, Chris Wilson wrote: > > Currently Ironlake operates under the assumption that rpm awake (and its > > error checking is disabled). As such, we have missed a few places where we > > access registers without taking the rpm wakeref and thus trigger > > warnings. intel_ips being one culprit. > > > > As this involved adding a potentially sleeping rpm_get, we have to > > rearrange the spinlocks slightly and so switch to acquiring a device-ref > > under the spinlock rather than hold the spinlock for the whole > > operation. To be consistent, we make the change in pattern common to the > > intel_ips interface even though this adds a few more atomic operations > > than necessary in a few cases. > > > > Signed-off-by: Chris Wilson > > --- > > drivers/gpu/drm/i915/i915_drv.c | 3 + > > drivers/gpu/drm/i915/intel_pm.c | 138 > > > > 2 files changed, 73 insertions(+), 68 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index 3d0b7353fb09..5c28990aab7f 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -1440,6 +1440,9 @@ void i915_driver_unload(struct drm_device *dev) > > > > i915_driver_unregister(dev_priv); > > > > + /* Flush any external code that still may be under the RCU lock */ > > + synchronize_rcu(); > > + > Hi Chris, > > Will this rcu change be equivalent to > > rcu_assign_pointer(i915_mch_dev, dev_priv) in gpu_ips_init > rcu_assign_pointer(i915_mch_dev, NULL) in gpu_ips_teardown > > eliminating smp_store_mb from init/teardown and synchronize_rcu here. We still have to go through the RCU period on teardown to be sure we flush all readers, but yes, the store_mb can be reduce to RCU_INIT_POINTER() and the mb are overkill as all we really need is the ordering on init, and the explicit rcu sync on teardown. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Check that the breadcrumb irq is enabled
On Mon, 09 Apr 2018 14:42:19 +0200, Chris Wilson wrote: Our execlists emulation for GuC requires use of the breadcrumb following every request as a simulcrum for the context-switch interrupt, which we then use to drive the submission tasklet. Therefore, when we unpark the engine for use with the GuC, we pin the breadcrumb interrupt to keep it enabled for the duration. This has to be remain so across all resets, wedging and resume, so check we do have the irq enabled when we start submitting requests to the GuC and on all submissions thereafter. Signed-off-by: Chris Wilson Cc: Michal Wajdeczko Cc: Michał Winiarski --- drivers/gpu/drm/i915/intel_guc_submission.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c index 97121230656c..a7957b669b68 100644 --- a/drivers/gpu/drm/i915/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/intel_guc_submission.c @@ -758,6 +758,8 @@ static void guc_submission_tasklet(unsigned long data) struct execlist_port *port = execlists->port; struct i915_request *rq; + GEM_BUG_ON(!READ_ONCE(engine->breadcrumbs.irq_enabled)); + rq = port_request(port); while (rq && i915_request_completed(rq)) { trace_i915_request_out(rq); LGTM, but can you run this with GuC enabled ? Thanks, Michal ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v8 08/12] drm/i915/uc: Fully sanitize uC within intel_uc_fini_hw
On Mon, 09 Apr 2018 14:47:24 +0200, Chris Wilson wrote: Quoting Michal Wajdeczko (2018-04-09 13:23:28) As we always call intel_uc_sanitize after every call to intel_uc_fini_hw we may drop redundant call and sanitize uC from the fini_hw function. Signed-off-by: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Chris Wilson Not that it matters, since doing it before losing control or on resume is the same from our pov, but I've always pencilled in sanitize as being done on takeover (i.e. before init). In intel_uc_init_hw we are already doing some semi-sanitization (thanks to __intel_uc_reset_hw), but maybe to be more explicit, we should add call to __uc_sanitize() in intel_uc_init_mmio() ? Why do you favour after fini? Hmm, not at all, I would call it just more explicit Gut feeling prefers keeping it as a separate step rather rolling it up into init/fini. But that's just because before we did sanitize elsewhere, we had many strange bugs and those bugs have left their scars (so I like seeing sanitize, it reminds me of dead bugs). As now we have symmetrical inits/finis that cover all critical paths, I don't think we need separate 'sanitize' step that could be called any time/any place. I assume extra __sanitize in uc_init_mmio should be enough reminder. /michal ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx