Re: [Intel-gfx] [PATCH v6 13/20] drm/i915/guc: Provide register list to be saved/restored during engine reset
On 21/04/17 13:21, Chris Wilson wrote: On Fri, Apr 21, 2017 at 01:10:37PM -0700, Daniele Ceraolo Spurio wrote: On 21/04/17 13:07, Michel Thierry wrote: On 20/04/17 10:29, Michel Thierry wrote: On 20/04/17 09:39, Daniele Ceraolo Spurio wrote: On 20/04/17 04:33, Joonas Lahtinen wrote: On ke, 2017-04-19 at 11:35 -0700, Michel Thierry wrote: From: Arun SiluveryGuC expects a list of registers from the driver which are saved/restored during engine reset. The type of value to be saved is controlled by flags. We provide a minimal set of registers that we want GuC to save and restore. This is not an issue in case of engine reset as driver initializes most of them following an engine reset, but in case of media reset (aka watchdog reset) which is completely internal to GuC (including resubmission of hung workload), it is necessary to provide this list, otherwise GuC won't be able to schedule further workloads after a reset. This is the minimal set of registers identified for things to work as expected but if we see any new issues, this register list can be expanded. In order to not loose any existing workarounds, we have to let GuC know the registers and its values. These will be reapplied after the reset. Note that we can't just read the current value because most of these registers are masked (so we have a workaround for a workaround for a workaround). v2: REGSET_MASKED is too difficult for GuC, use REGSET_SAVE_DEFAULT_VALUE and current value from RING_MODE reg instead; no need to preserve head/tail either, be extra paranoid and save whitelisted registers (Daniele). v3: Workarounds added only once during _init_workarounds also have to been restored, or we risk loosing them after internal GuC reset (Daniele). Cc: Daniele Ceraolo Spurio Signed-off-by: Arun Siluvery Signed-off-by: Jeff McGee Signed-off-by: Michel Thierry @@ -732,15 +755,16 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) int ret; /* WaConextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk */ -I915_WRITE(GEN9_CSFE_CHICKEN1_RCS, _MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE)); +I915_GUC_REG_WRITE(GEN9_CSFE_CHICKEN1_RCS, + _MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE)); To make grepping easier, how about? I915_WRITE(GUC_REG(GEN9_CSFE_CHICKEN1_RCS), ...); Regards, Joonas GUC_REG makes it sound like it is somehow related to GuC, while it isn't, we just want GuC to restore its value. What about GUC_REG_RESTORE? Honestly, I dont care about names, pick one and I add it. Just a reminder, we not only need the reg offset, we want to save the value too. I915_WRITE_GUC_RESTORE(reg, value) ? That would be inline to the others we have, e.g. I915_WRITE_FW, I915_WRITE_CTL, I915_WRITE_HEAD/TAIL. I915_WRITE_FW is not the same class as I915_WRITE_CTL/HEAD/TAIL, and I can say from experience the I915_*_CTL/HEAD/TAIL were a mistake (special casing one particular access to the ring mmio, but we often deviate from that pattern). Looking at the above I see you are falling for the same trap as the ring shorthand... So are you sure the convenience will not be lost later? And in particular avoid using I915_WRITE_*() naming style as I would rather that was earmarked for the different mmio accessors. Ok, then can follow the pattern of the other workarounds & whitelist reg code? E.g. WA_REG_GUC_RESTORE or WA_MMIO_REG_GUC_RESTORE (to make it clearer that these are not registers in the ctx image). ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 13/20] drm/i915/guc: Provide register list to be saved/restored during engine reset
On Fri, Apr 21, 2017 at 01:10:37PM -0700, Daniele Ceraolo Spurio wrote: > > > On 21/04/17 13:07, Michel Thierry wrote: > > > > > >On 20/04/17 10:29, Michel Thierry wrote: > >> > >> > >>On 20/04/17 09:39, Daniele Ceraolo Spurio wrote: > >>> > >>> > >>>On 20/04/17 04:33, Joonas Lahtinen wrote: > On ke, 2017-04-19 at 11:35 -0700, Michel Thierry wrote: > >From: Arun Siluvery> > > >GuC expects a list of registers from the driver which are > >saved/restored > >during engine reset. The type of value to be saved is controlled by > >flags. We provide a minimal set of registers that we want GuC to save > >and > >restore. This is not an issue in case of engine reset as driver > >initializes > >most of them following an engine reset, but in case of media reset > >(aka > >watchdog reset) which is completely internal to GuC (including > >resubmission > >of hung workload), it is necessary to provide this list, otherwise > >GuC won't > >be able to schedule further workloads after a reset. This is the > >minimal > >set of registers identified for things to work as expected but if we > >see > >any new issues, this register list can be expanded. > > > >In order to not loose any existing workarounds, we have to let GuC > >know > >the registers and its values. These will be reapplied after the reset. > >Note that we can't just read the current value because most of these > >registers are masked (so we have a workaround for a workaround for a > >workaround). > > > >v2: REGSET_MASKED is too difficult for GuC, use > >REGSET_SAVE_DEFAULT_VALUE > >and current value from RING_MODE reg instead; no need to preserve > >head/tail either, be extra paranoid and save whitelisted registers > >(Daniele). > > > >v3: Workarounds added only once during _init_workarounds also have to > >been restored, or we risk loosing them after internal GuC reset > >(Daniele). > > > >Cc: Daniele Ceraolo Spurio > >Signed-off-by: Arun Siluvery > >Signed-off-by: Jeff McGee > >Signed-off-by: Michel Thierry > > > > >@@ -732,15 +755,16 @@ static int gen9_init_workarounds(struct > >intel_engine_cs *engine) > >> int ret; > > > > /* WaConextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk */ > >-I915_WRITE(GEN9_CSFE_CHICKEN1_RCS, > >_MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE)); > >+I915_GUC_REG_WRITE(GEN9_CSFE_CHICKEN1_RCS, > >+ > >_MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE)); > > To make grepping easier, how about? > > I915_WRITE(GUC_REG(GEN9_CSFE_CHICKEN1_RCS), > ...); > > Regards, Joonas > > >>> > >>>GUC_REG makes it sound like it is somehow related to GuC, while it > >>>isn't, we just want GuC to restore its value. What about > >>>GUC_REG_RESTORE? > >>> > >> > >>Honestly, I dont care about names, pick one and I add it. > >>Just a reminder, we not only need the reg offset, we want to save the > >>value too. > >> > > > >I915_WRITE_GUC_RESTORE(reg, value) ? > > > >That would be inline to the others we have, e.g. I915_WRITE_FW, > >I915_WRITE_CTL, I915_WRITE_HEAD/TAIL. I915_WRITE_FW is not the same class as I915_WRITE_CTL/HEAD/TAIL, and I can say from experience the I915_*_CTL/HEAD/TAIL were a mistake (special casing one particular access to the ring mmio, but we often deviate from that pattern). Looking at the above I see you are falling for the same trap as the ring shorthand... So are you sure the convenience will not be lost later? And in particular avoid using I915_WRITE_*() naming style as I would rather that was earmarked for the different mmio accessors. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 13/20] drm/i915/guc: Provide register list to be saved/restored during engine reset
On 21/04/17 13:07, Michel Thierry wrote: On 20/04/17 10:29, Michel Thierry wrote: On 20/04/17 09:39, Daniele Ceraolo Spurio wrote: On 20/04/17 04:33, Joonas Lahtinen wrote: On ke, 2017-04-19 at 11:35 -0700, Michel Thierry wrote: From: Arun SiluveryGuC expects a list of registers from the driver which are saved/restored during engine reset. The type of value to be saved is controlled by flags. We provide a minimal set of registers that we want GuC to save and restore. This is not an issue in case of engine reset as driver initializes most of them following an engine reset, but in case of media reset (aka watchdog reset) which is completely internal to GuC (including resubmission of hung workload), it is necessary to provide this list, otherwise GuC won't be able to schedule further workloads after a reset. This is the minimal set of registers identified for things to work as expected but if we see any new issues, this register list can be expanded. In order to not loose any existing workarounds, we have to let GuC know the registers and its values. These will be reapplied after the reset. Note that we can't just read the current value because most of these registers are masked (so we have a workaround for a workaround for a workaround). v2: REGSET_MASKED is too difficult for GuC, use REGSET_SAVE_DEFAULT_VALUE and current value from RING_MODE reg instead; no need to preserve head/tail either, be extra paranoid and save whitelisted registers (Daniele). v3: Workarounds added only once during _init_workarounds also have to been restored, or we risk loosing them after internal GuC reset (Daniele). Cc: Daniele Ceraolo Spurio Signed-off-by: Arun Siluvery Signed-off-by: Jeff McGee Signed-off-by: Michel Thierry @@ -732,15 +755,16 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) int ret; /* WaConextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk */ -I915_WRITE(GEN9_CSFE_CHICKEN1_RCS, _MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE)); +I915_GUC_REG_WRITE(GEN9_CSFE_CHICKEN1_RCS, + _MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE)); To make grepping easier, how about? I915_WRITE(GUC_REG(GEN9_CSFE_CHICKEN1_RCS), ...); Regards, Joonas GUC_REG makes it sound like it is somehow related to GuC, while it isn't, we just want GuC to restore its value. What about GUC_REG_RESTORE? Honestly, I dont care about names, pick one and I add it. Just a reminder, we not only need the reg offset, we want to save the value too. I915_WRITE_GUC_RESTORE(reg, value) ? That would be inline to the others we have, e.g. I915_WRITE_FW, I915_WRITE_CTL, I915_WRITE_HEAD/TAIL. -Michel LGTM. Daniele ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 13/20] drm/i915/guc: Provide register list to be saved/restored during engine reset
On 20/04/17 10:29, Michel Thierry wrote: On 20/04/17 09:39, Daniele Ceraolo Spurio wrote: On 20/04/17 04:33, Joonas Lahtinen wrote: On ke, 2017-04-19 at 11:35 -0700, Michel Thierry wrote: From: Arun SiluveryGuC expects a list of registers from the driver which are saved/restored during engine reset. The type of value to be saved is controlled by flags. We provide a minimal set of registers that we want GuC to save and restore. This is not an issue in case of engine reset as driver initializes most of them following an engine reset, but in case of media reset (aka watchdog reset) which is completely internal to GuC (including resubmission of hung workload), it is necessary to provide this list, otherwise GuC won't be able to schedule further workloads after a reset. This is the minimal set of registers identified for things to work as expected but if we see any new issues, this register list can be expanded. In order to not loose any existing workarounds, we have to let GuC know the registers and its values. These will be reapplied after the reset. Note that we can't just read the current value because most of these registers are masked (so we have a workaround for a workaround for a workaround). v2: REGSET_MASKED is too difficult for GuC, use REGSET_SAVE_DEFAULT_VALUE and current value from RING_MODE reg instead; no need to preserve head/tail either, be extra paranoid and save whitelisted registers (Daniele). v3: Workarounds added only once during _init_workarounds also have to been restored, or we risk loosing them after internal GuC reset (Daniele). Cc: Daniele Ceraolo Spurio Signed-off-by: Arun Siluvery Signed-off-by: Jeff McGee Signed-off-by: Michel Thierry @@ -732,15 +755,16 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) int ret; /* WaConextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk */ -I915_WRITE(GEN9_CSFE_CHICKEN1_RCS, _MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE)); +I915_GUC_REG_WRITE(GEN9_CSFE_CHICKEN1_RCS, + _MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE)); To make grepping easier, how about? I915_WRITE(GUC_REG(GEN9_CSFE_CHICKEN1_RCS), ...); Regards, Joonas GUC_REG makes it sound like it is somehow related to GuC, while it isn't, we just want GuC to restore its value. What about GUC_REG_RESTORE? Honestly, I dont care about names, pick one and I add it. Just a reminder, we not only need the reg offset, we want to save the value too. I915_WRITE_GUC_RESTORE(reg, value) ? That would be inline to the others we have, e.g. I915_WRITE_FW, I915_WRITE_CTL, I915_WRITE_HEAD/TAIL. -Michel ___ 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: Two stage watermarks for g4x
== Series Details == Series: drm/i915: Two stage watermarks for g4x URL : https://patchwork.freedesktop.org/series/23358/ State : success == Summary == Series 23358v1 drm/i915: Two stage watermarks for g4x https://patchwork.freedesktop.org/api/1.0/series/23358/revisions/1/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:433s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:429s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:579s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:515s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:554s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:486s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:479s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:408s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:405s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:492s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:471s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:459s fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:567s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:450s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:573s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:457s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:498s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:430s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:535s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:411s d473a03c132969f33a6f95456a4dbbfb36b99373 drm-tip: 2017y-04m-21d-15h-42m-02s UTC integration manifest ec327fa drm/i915: Add support for sprites on g4x 6fdd8f0 drm/i915: Add g4x watermark tracepoint dddf36a drm/i915: Enable HPLL watermarks on g4x 9a73d73 drm/i915: Two stage watermarks for g4x fe292bd drm/i915: Apply the g4x TLB miss w/a to SR watermarks as well 8adede1 drm/i915: Refactor wm calculations 79a7571 drm/i915: Refactor the g4x TLB miss w/a to a helper e680001 drm/i915: Fix the g4x watermark TLB miss workaround 76a8ff6 drm/i915: Fix cursor 'cpp' in watermark calculatins for old platforms 9df15fa drm/i915: Document CxSR 29a6f48 drm/i915: Make vlv/chv watermark debug print less cryptic abd09b7 drm/i915: Rename bunch of vlv_ watermark structures to g4x_ d05cc7f drm/i915: s/vlv_num_wm_levels/intel_wm_num_levels/ 73841ae drm/i915: Drop the debug message from vlv_get_fifo_size() d236600 drm/i915: s/vlv_plane_wm_compute/vlv_raw_plane_wm_compute/ etc. == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4531/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/15] drm/i915: Fix the g4x watermark TLB miss workaround
From: Ville SyrjäläThe g4x watermark TLB miss workaround requires that we bump up the watermark by the difference between 8 full lines and the FIFO size. Unfortunately the way we compute it at the moment ignores the size of the pixels. The code also used the primary plane width as the cursor width when computing the TLB miss w/a for the cursor. Let's fix both problems. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 7d0d1a0c4c63..09d4676419fb 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -811,7 +811,7 @@ static bool g4x_compute_wm0(struct drm_i915_private *dev_priv, struct intel_crtc *crtc; const struct drm_display_mode *adjusted_mode; const struct drm_framebuffer *fb; - int htotal, hdisplay, clock, cpp; + int htotal, plane_width, cursor_width, clock, cpp; int line_time_us, line_count; int entries, tlb_miss; @@ -826,12 +826,13 @@ static bool g4x_compute_wm0(struct drm_i915_private *dev_priv, fb = crtc->base.primary->state->fb; clock = adjusted_mode->crtc_clock; htotal = adjusted_mode->crtc_htotal; - hdisplay = crtc->config->pipe_src_w; + plane_width = crtc->config->pipe_src_w; + cursor_width = crtc->base.cursor->state->crtc_w; cpp = fb->format->cpp[0]; /* Use the small buffer method to calculate plane watermark */ entries = ((clock * cpp / 1000) * display_latency_ns) / 1000; - tlb_miss = display->fifo_size*display->cacheline_size - hdisplay * 8; + tlb_miss = display->fifo_size*display->cacheline_size - plane_width * cpp * 8; if (tlb_miss > 0) entries += tlb_miss; entries = DIV_ROUND_UP(entries, display->cacheline_size); @@ -842,8 +843,8 @@ static bool g4x_compute_wm0(struct drm_i915_private *dev_priv, /* Use the large buffer method to calculate cursor watermark */ line_time_us = max(htotal * 1000 / clock, 1); line_count = (cursor_latency_ns / line_time_us + 1000) / 1000; - entries = line_count * crtc->base.cursor->state->crtc_w * 4; - tlb_miss = cursor->fifo_size*cursor->cacheline_size - hdisplay * 8; + entries = line_count * cursor_width * 4; + tlb_miss = cursor->fifo_size*cursor->cacheline_size - cursor_width * 4 * 8; if (tlb_miss > 0) entries += tlb_miss; entries = DIV_ROUND_UP(entries, cursor->cacheline_size); -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/15] drm/i915: Two stage watermarks for g4x
From: Ville SyrjäläImplement proper two stage watermark programming for g4x. As with other pre-SKL platforms, the watermark registers aren't double buffered on g4x. Hence we must sequence the watermark update carefully around plane updates. The code is quite heavily modelled on the VLV/CHV code, with some fairly significant differences due to the different hardware architecture: * g4x doesn't use inverted watermark values * CxSR actually affects the watermarks since it controls memory self refresh in addition to the max FIFO mode * A further HPLL SR mode is possible with higher memory wakeup latency * g4x has FBC2 and so it also has FBC watermarks * max FIFO mode for primary plane only (cursor is allowed, sprite is not) * g4x has no manual FIFO repartitioning * some TLB miss related workarounds are needed for the watermarks Actually the hardware is quite similar to ILK+ in many ways. The most visible differences are in the actual watermakr register layout. ILK revamped that part quite heavily whereas g4x is still using the layout inherited from earlier platforms. Note that we didn't previously enable the HPLL SR on g4x. So in order to not introduce too many functional changes in this patch I've not actually enabled it here either, even though the code is now fully ready for it. We'll enable it separately later on. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_debugfs.c | 12 +- drivers/gpu/drm/i915/i915_drv.h | 12 + drivers/gpu/drm/i915/intel_display.c | 25 +- drivers/gpu/drm/i915/intel_drv.h | 28 ++ drivers/gpu/drm/i915/intel_pm.c | 942 +++ 5 files changed, 792 insertions(+), 227 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 870c470177b5..69550d31099e 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3898,6 +3898,8 @@ static void wm_latency_show(struct seq_file *m, const uint16_t wm[8]) num_levels = 3; else if (IS_VALLEYVIEW(dev_priv)) num_levels = 1; + else if (IS_G4X(dev_priv)) + num_levels = 3; else num_levels = ilk_wm_max_level(dev_priv) + 1; @@ -3910,8 +3912,10 @@ static void wm_latency_show(struct seq_file *m, const uint16_t wm[8]) * - WM1+ latency values in 0.5us units * - latencies are in us on gen9/vlv/chv */ - if (INTEL_GEN(dev_priv) >= 9 || IS_VALLEYVIEW(dev_priv) || - IS_CHERRYVIEW(dev_priv)) + if (INTEL_GEN(dev_priv) >= 9 || + IS_VALLEYVIEW(dev_priv) || + IS_CHERRYVIEW(dev_priv) || + IS_G4X(dev_priv)) latency *= 10; else if (level > 0) latency *= 5; @@ -3972,7 +3976,7 @@ static int pri_wm_latency_open(struct inode *inode, struct file *file) { struct drm_i915_private *dev_priv = inode->i_private; - if (INTEL_GEN(dev_priv) < 5) + if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv)) return -ENODEV; return single_open(file, pri_wm_latency_show, dev_priv); @@ -4014,6 +4018,8 @@ static ssize_t wm_latency_write(struct file *file, const char __user *ubuf, num_levels = 3; else if (IS_VALLEYVIEW(dev_priv)) num_levels = 1; + else if (IS_G4X(dev_priv)) + num_levels = 3; else num_levels = ilk_wm_max_level(dev_priv) + 1; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 0a393fdc53d1..6df8bff7f5a7 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1762,11 +1762,13 @@ struct ilk_wm_values { struct g4x_pipe_wm { uint16_t plane[I915_MAX_PLANES]; + uint16_t fbc; }; struct g4x_sr_wm { uint16_t plane; uint16_t cursor; + uint16_t fbc; }; struct vlv_wm_ddl_values { @@ -1781,6 +1783,15 @@ struct vlv_wm_values { bool cxsr; }; +struct g4x_wm_values { + struct g4x_pipe_wm pipe[2]; + struct g4x_sr_wm sr; + struct g4x_sr_wm hpll; + bool cxsr; + bool hpll_en; + bool fbc_en; +}; + struct skl_ddb_entry { uint16_t start, end;/* in number of blocks, 'end' is exclusive */ }; @@ -2410,6 +2421,7 @@ struct drm_i915_private { struct ilk_wm_values hw; struct skl_wm_values skl_hw; struct vlv_wm_values vlv; + struct g4x_wm_values g4x; }; uint8_t max_level; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 85b9e2f521a0..0f42263c3f76 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++
[Intel-gfx] [PATCH 13/15] drm/i915: Enable HPLL watermarks on g4x
From: Ville SyrjäläI don't see why we couldn't use the HPLL watermarks on g4x. So let's enable them. Let's assume a 35 usec memory latency for the HPLL mode. That's roughly what PNV uses. Based on the behaviour of the ELK box I have 35 usec is probably overkill. Actually all the current latency values used seem overkill as I can reduce them pretty drastically before I start to see underruns. But let's play things a bit safe for now. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 9b0a6a4572ce..957ef10b1569 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1020,8 +1020,9 @@ static void g4x_setup_wm_latency(struct drm_i915_private *dev_priv) /* all latencies in usec */ dev_priv->wm.pri_latency[G4X_WM_LEVEL_NORMAL] = 5; dev_priv->wm.pri_latency[G4X_WM_LEVEL_SR] = 12; + dev_priv->wm.pri_latency[G4X_WM_LEVEL_HPLL] = 35; - dev_priv->wm.max_level = G4X_WM_LEVEL_SR; + dev_priv->wm.max_level = G4X_WM_LEVEL_HPLL; } static int g4x_plane_fifo_size(enum plane_id plane_id, int level) -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 15/15] drm/i915: Add support for sprites on g4x
From: Ville SyrjäläNow that the watermarks are in order, it should be safe to enable sprite planes on g4x. We alreday have the code in fact, we just call it ilk_. Let's rename to g4x_ and let it loose. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_device_info.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 4 ++-- drivers/gpu/drm/i915/intel_sprite.c | 18 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 7d01dfe7faac..3718341662c2 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -337,7 +337,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv) } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { for_each_pipe(dev_priv, pipe) info->num_sprites[pipe] = 2; - } else if (INTEL_GEN(dev_priv) >= 5) { + } else if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) { for_each_pipe(dev_priv, pipe) info->num_sprites[pipe] = 1; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0f42263c3f76..3ccc39e46f8f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1277,7 +1277,7 @@ static void assert_sprites_disabled(struct drm_i915_private *dev_priv, I915_STATE_WARN(val & SPRITE_ENABLE, "sprite %c assertion failure, should be off on pipe %c but is still active\n", plane_name(pipe), pipe_name(pipe)); - } else if (INTEL_GEN(dev_priv) >= 5) { + } else if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) { u32 val = I915_READ(DVSCNTR(pipe)); I915_STATE_WARN(val & DVS_ENABLE, "sprite %c assertion failure, should be off on pipe %c but is still active\n", @@ -14429,7 +14429,7 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb, case DRM_FORMAT_UYVY: case DRM_FORMAT_YVYU: case DRM_FORMAT_VYUY: - if (INTEL_GEN(dev_priv) < 5) { + if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv)) { DRM_DEBUG_KMS("unsupported pixel format: %s\n", drm_get_format_name(mode_cmd->pixel_format, _name)); goto err; diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index f7d431427115..511f0e969f7a 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -629,7 +629,7 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) spin_unlock_irqrestore(_priv->uncore.lock, irqflags); } -static u32 ilk_sprite_ctl(const struct intel_crtc_state *crtc_state, +static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { struct drm_i915_private *dev_priv = @@ -683,7 +683,7 @@ static u32 ilk_sprite_ctl(const struct intel_crtc_state *crtc_state, } static void -ilk_update_plane(struct drm_plane *plane, +g4x_update_plane(struct drm_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { @@ -744,7 +744,7 @@ ilk_update_plane(struct drm_plane *plane, } static void -ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) +g4x_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) { struct drm_device *dev = plane->dev; struct drm_i915_private *dev_priv = to_i915(dev); @@ -964,7 +964,7 @@ intel_check_sprite_plane(struct drm_plane *plane, if (ret) return ret; - state->ctl = ilk_sprite_ctl(crtc_state, state); + state->ctl = g4x_sprite_ctl(crtc_state, state); } return 0; @@ -1024,7 +1024,7 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data, return ret; } -static const uint32_t ilk_plane_formats[] = { +static const uint32_t g4x_plane_formats[] = { DRM_FORMAT_XRGB, DRM_FORMAT_YUYV, DRM_FORMAT_YVYU, @@ -1128,15 +1128,15 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv, intel_plane->can_scale = true; intel_plane->max_downscale = 16; - intel_plane->update_plane = ilk_update_plane; - intel_plane->disable_plane = ilk_disable_plane; + intel_plane->update_plane = g4x_update_plane; + intel_plane->disable_plane = g4x_disable_plane; if (IS_GEN6(dev_priv)) { plane_formats = snb_plane_formats; num_plane_formats
[Intel-gfx] [PATCH 14/15] drm/i915: Add g4x watermark tracepoint
From: Ville SyrjäläAdd a tracepoint for watermark programming on g4x, similar to what we have on vlv/chv. Should help in debugging watermark programming sequence issues. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_trace.h | 49 +++ drivers/gpu/drm/i915/intel_pm.c | 5 2 files changed, 54 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index 66404c5aee82..b24a83d43559 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -89,6 +89,55 @@ TRACE_EVENT(intel_memory_cxsr, __entry->frame[PIPE_C], __entry->scanline[PIPE_C]) ); +TRACE_EVENT(g4x_wm, + TP_PROTO(struct intel_crtc *crtc, const struct g4x_wm_values *wm), + TP_ARGS(crtc, wm), + + TP_STRUCT__entry( +__field(enum pipe, pipe) +__field(u32, frame) +__field(u32, scanline) +__field(u16, primary) +__field(u16, sprite) +__field(u16, cursor) +__field(u16, sr_plane) +__field(u16, sr_cursor) +__field(u16, sr_fbc) +__field(u16, hpll_plane) +__field(u16, hpll_cursor) +__field(u16, hpll_fbc) +__field(bool, cxsr) +__field(bool, hpll) +__field(bool, fbc) +), + + TP_fast_assign( + __entry->pipe = crtc->pipe; + __entry->frame = crtc->base.dev->driver->get_vblank_counter(crtc->base.dev, + crtc->pipe); + __entry->scanline = intel_get_crtc_scanline(crtc); + __entry->primary = wm->pipe[crtc->pipe].plane[PLANE_PRIMARY]; + __entry->sprite = wm->pipe[crtc->pipe].plane[PLANE_SPRITE0]; + __entry->cursor = wm->pipe[crtc->pipe].plane[PLANE_CURSOR]; + __entry->sr_plane = wm->sr.plane; + __entry->sr_cursor = wm->sr.cursor; + __entry->sr_fbc = wm->sr.fbc; + __entry->hpll_plane = wm->hpll.plane; + __entry->hpll_cursor = wm->hpll.cursor; + __entry->hpll_fbc = wm->hpll.fbc; + __entry->cxsr = wm->cxsr; + __entry->hpll = wm->hpll_en; + __entry->fbc = wm->fbc_en; + ), + + TP_printk("pipe %c, frame=%u, scanline=%u, wm %d/%d/%d, sr %s/%d/%d/%d, hpll %s/%d/%d/%d, fbc %s", + pipe_name(__entry->pipe), __entry->frame, __entry->scanline, + __entry->primary, __entry->sprite, __entry->cursor, + yesno(__entry->cxsr), __entry->sr_plane, __entry->sr_cursor, __entry->sr_fbc, + yesno(__entry->hpll), __entry->hpll_plane, __entry->hpll_cursor, __entry->hpll_fbc, + yesno(__entry->fbc)) +); + TRACE_EVENT(vlv_wm, TP_PROTO(struct intel_crtc *crtc, const struct vlv_wm_values *wm), TP_ARGS(crtc, wm), diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 957ef10b1569..ef0e9f8d4dbd 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -913,6 +913,11 @@ static int g4x_tlb_miss_wa(int fifo_size, int width, int cpp) static void g4x_write_wm_values(struct drm_i915_private *dev_priv, const struct g4x_wm_values *wm) { + enum pipe pipe; + + for_each_pipe(dev_priv, pipe) + trace_g4x_wm(intel_get_crtc_for_pipe(dev_priv, pipe), wm); + I915_WRITE(DSPFW1, FW_WM(wm->sr.plane, SR) | FW_WM(wm->pipe[PIPE_B].plane[PLANE_CURSOR], CURSORB) | -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/15] drm/i915: Refactor wm calculations
From: Ville SyrjäläAll platforms until SKL compute their watermarks essentially using the same method1/small buffer and method2/large buffer formulas. Most just open code it in slightly different ways. Let's pull it all into common helpers. This makes it a little easier to spot the actual differences. While at it try to add some docs explainign what the formulas are trying to do. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 221 +++- 1 file changed, 149 insertions(+), 72 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c43fcd5d29b2..c07f3b2b0972 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -626,8 +626,104 @@ static const struct intel_watermark_params i845_wm_info = { }; /** + * intel_wm_method1 - Method 1 / "small buffer" watermark formula + * @pixel_rate: Pipe pixel rate in kHz + * @cpp: Plane bytes per pixel + * @latency: Memory wakeup latency in 0.1us units + * + * Compute the watermark using the method 1 or "small buffer" + * formula. The caller may additonally add extra cachelines + * to account for TLB misses and clock crossings. + * + * This method is concerned with the short term drain rate + * of the FIFO, ie. it does not account for blanking periods + * which would effectively reduce the average drain rate across + * a longer period. The name "small" refers to the fact the + * FIFO is relatively small compared to the amount of data + * fetched. + * + * The FIFO level vs. time graph might look something like: + * + * |\ |\ + * | \ | \ + * __---__---__ (- plane active, _ blanking) + * -> time + * + * or perhaps like this: + * + * |\|\ |\|\ + * ______ (- plane active, _ blanking) + * -> time + * + * Returns: + * The watermark in bytes + */ +static unsigned int intel_wm_method1(unsigned int pixel_rate, +unsigned int cpp, +unsigned int latency) +{ + uint64_t ret; + + ret = (uint64_t) pixel_rate * cpp * latency; + ret = DIV_ROUND_UP_ULL(ret, 1); + + return ret; +} + +/** + * intel_wm_method2 - Method 2 / "large buffer" watermark formula + * @pixel_rate: Pipe pixel rate in kHz + * @htotal: Pipe horizontal total + * @width: Plane width in pixels + * @cpp: Plane bytes per pixel + * @latency: Memory wakeup latency in 0.1us units + * + * Compute the watermark using the method 2 or "large buffer" + * formula. The caller may additonally add extra cachelines + * to account for TLB misses and clock crossings. + * + * This method is concerned with the long term drain rate + * of the FIFO, ie. it does account for blanking periods + * which effectively reduce the average drain rate across + * a longer period. The name "large" refers to the fact the + * FIFO is relatively large compared to the amount of data + * fetched. + * + * The FIFO level vs. time graph might look something like: + * + *|\___ |\___ + *|\___ |\___ + *|\ |\ + * __ --__--__--__--__--__--__ (- plane active, _ blanking) + * -> time + * + * Returns: + * The watermark in bytes + */ +static unsigned int intel_wm_method2(unsigned int pixel_rate, +unsigned int htotal, +unsigned int width, +unsigned int cpp, +unsigned int latency) +{ + unsigned int ret; + + /* +* FIXME remove once all users are computing +* watermarks in the correct place. +*/ + if (WARN_ON_ONCE(htotal == 0)) + htotal = 1; + + ret = (latency * pixel_rate) / (htotal * 1); + ret = (ret + 1) * width * cpp; + + return ret; +} + +/** * intel_calculate_wm - calculate watermark level - * @clock_in_khz: pixel clock + * @pixel_rate: pixel clock * @wm: chip FIFO params * @cpp: bytes per pixel * @latency_ns: memory latency for the platform @@ -643,12 +739,12 @@ static const struct intel_watermark_params i845_wm_info = { * past the watermark point. If the FIFO drains completely, a FIFO underrun * will occur, and a display engine hang could result. */ -static unsigned long intel_calculate_wm(unsigned long clock_in_khz, - const struct intel_watermark_params *wm, - int fifo_size, int cpp, - unsigned long latency_ns) +static unsigned int intel_calculate_wm(int pixel_rate, + const struct intel_watermark_params *wm, + int fifo_size, int cpp, + unsigned int latency_ns) { - long entries_required, wm_size; + int entries, wm_size; /* * Note: we
[Intel-gfx] [PATCH 11/15] drm/i915: Apply the g4x TLB miss w/a to SR watermarks as well
From: Ville SyrjäläThe documentation I've seen doesn't actually specify which watermarks need the TLB miss w/a. Currently we only apply the w/a to the normal watermarks for both primary and cursor planes. Since the documentation doesn't explicitly say anything I'm going to assume that the w/a should equally apply to the SR/HPLL watermarks. So let's do that. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c07f3b2b0972..61b67994c4a8 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1006,7 +1006,7 @@ static bool g4x_compute_srwm(struct drm_i915_private *dev_priv, struct intel_crtc *crtc; const struct drm_display_mode *adjusted_mode; const struct drm_framebuffer *fb; - int hdisplay, htotal, cpp, clock; + int plane_width, cursor_width, htotal, cpp, clock; int small, large; int entries; @@ -1020,20 +1020,23 @@ static bool g4x_compute_srwm(struct drm_i915_private *dev_priv, fb = crtc->base.primary->state->fb; clock = adjusted_mode->crtc_clock; htotal = adjusted_mode->crtc_htotal; - hdisplay = crtc->config->pipe_src_w; + plane_width = crtc->config->pipe_src_w; + cursor_width = crtc->base.cursor->state->crtc_w; cpp = fb->format->cpp[0]; /* Use the minimum of the small and large buffer method for primary */ small = intel_wm_method1(clock, cpp, latency_ns / 100); - large = intel_wm_method2(clock, htotal, hdisplay, cpp, + large = intel_wm_method2(clock, htotal, plane_width, cpp, latency_ns / 100); - entries = DIV_ROUND_UP(min(small, large), display->cacheline_size); + entries = min(small, large); + entries += g4x_tlb_miss_wa(display->fifo_size, plane_width, cpp); + entries = DIV_ROUND_UP(entries, display->cacheline_size); *display_wm = entries + display->guard_size; /* calculate the self-refresh watermark for display cursor */ - entries = intel_wm_method2(clock, htotal, - crtc->base.cursor->state->crtc_w, 4, + entries = intel_wm_method2(clock, htotal, cursor_width, 4, latency_ns / 100); + entries += g4x_tlb_miss_wa(cursor->fifo_size, cursor_width, 4); entries = DIV_ROUND_UP(entries, cursor->cacheline_size); *cursor_wm = entries + cursor->guard_size; -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/15] drm/i915: Refactor the g4x TLB miss w/a to a helper
From: Ville SyrjäläPull the g4x TLB miss w/a calculation into a small helper. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 27 --- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 09d4676419fb..c43fcd5d29b2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -799,6 +799,23 @@ static void pineview_update_wm(struct intel_crtc *unused_crtc) } } +/* + * Documentation says: + * "If the line size is small, the TLB fetches can get in the way of the + * data fetches, causing some lag in the pixel data return which is not + * accounted for in the above formulas. The following adjustment only + * needs to be applied if eight whole lines fit in the buffer at once. + * The WM is adjusted upwards by the difference between the FIFO size + * and the size of 8 whole lines. This adjustment is always performed + * in the actual pixel depth regardless of whether FBC is enabled or not." + */ +static int g4x_tlb_miss_wa(int fifo_size, int width, int cpp) +{ + int tlb_miss = fifo_size * 64 - width * cpp * 8; + + return max(0, tlb_miss); +} + static bool g4x_compute_wm0(struct drm_i915_private *dev_priv, int plane, const struct intel_watermark_params *display, @@ -813,7 +830,7 @@ static bool g4x_compute_wm0(struct drm_i915_private *dev_priv, const struct drm_framebuffer *fb; int htotal, plane_width, cursor_width, clock, cpp; int line_time_us, line_count; - int entries, tlb_miss; + int entries; crtc = intel_get_crtc_for_plane(dev_priv, plane); if (!intel_crtc_active(crtc)) { @@ -832,9 +849,7 @@ static bool g4x_compute_wm0(struct drm_i915_private *dev_priv, /* Use the small buffer method to calculate plane watermark */ entries = ((clock * cpp / 1000) * display_latency_ns) / 1000; - tlb_miss = display->fifo_size*display->cacheline_size - plane_width * cpp * 8; - if (tlb_miss > 0) - entries += tlb_miss; + entries += g4x_tlb_miss_wa(display->fifo_size, plane_width, cpp); entries = DIV_ROUND_UP(entries, display->cacheline_size); *plane_wm = entries + display->guard_size; if (*plane_wm > (int)display->max_wm) @@ -844,9 +859,7 @@ static bool g4x_compute_wm0(struct drm_i915_private *dev_priv, line_time_us = max(htotal * 1000 / clock, 1); line_count = (cursor_latency_ns / line_time_us + 1000) / 1000; entries = line_count * cursor_width * 4; - tlb_miss = cursor->fifo_size*cursor->cacheline_size - cursor_width * 4 * 8; - if (tlb_miss > 0) - entries += tlb_miss; + entries += g4x_tlb_miss_wa(cursor->fifo_size, cursor_width, 4); entries = DIV_ROUND_UP(entries, cursor->cacheline_size); *cursor_wm = entries + cursor->guard_size; if (*cursor_wm > (int)cursor->max_wm) -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/15] drm/i915: Document CxSR
From: Ville SyrjäläAdd some documentation explaining what CxSR actually is. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 37 + 1 file changed, 37 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index da2072728df2..7bd4c8688acb 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -386,6 +386,43 @@ static bool _intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enabl return was_enabled; } +/** + * intel_set_memory_cxsr - Configure CxSR state + * @dev_priv: i915 device + * @enable: Allow vs. disallow CxSR + * + * Allow or disallow the system to enter a special CxSR + * (C-state self refresh) state. What typically happens in CxSR mode + * is that several display FIFOs may get combined into a single larger + * FIFO for a particular plane (so called max FIFO mode) to allow the + * system to defer memory fetches longer, and the memory will enter + * self refresh. + * + * Note that enabling CxSR does not guarantee that the system enter + * this special mode, nor does it guarantee that the system stays + * in that mode once entered. So this just allows/disallows the system + * to autonomously utilize the CxSR mode. Other factors such as core + * C-states will affect when/if the system actually enters/exits the + * CxSR mode. + * + * Note that on VLV/CHV this actually only controls the max FIFO mode, + * and the system is free to enter/exit memory self refresh at any time + * even when the use of CxSR has been disallowed. + * + * While the system is actually in the CxSR/max FIFO mode, some plane + * control registers will not get latched on vblank. Thus in order to + * guarantee the system will respond to changes in the plane registers + * we must always disallow CxSR prior to making changes to those registers. + * Unfortunately the system will re-evaluate the CxSR conditions at + * frame start which happens after vblank start (which is when the plane + * registers would get latched), so we can't proceed with the plane update + * during the same frame where we disallowed CxSR. + * + * Certain platforms also have a deeper HPLL SR mode. Fortunately the + * HPLL SR mode depends on CxSR itself, so we don't have to hand hold + * the hardware w.r.t. HPLL SR when writing to plane registers. + * Disallowing just CxSR is sufficient. + */ bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable) { bool ret; -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/15] drm/i915: Fix cursor 'cpp' in watermark calculatins for old platforms
From: Ville SyrjäläThe watermark code for the old platforms (g4x and older) uses the primary plane cpp when computing cursor watermarks. To keep the fix simple let's just hardcode cpp=4 for the cursor on those platforms since that's all we support. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 7bd4c8688acb..7d0d1a0c4c63 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -768,7 +768,7 @@ static void pineview_update_wm(struct intel_crtc *unused_crtc) /* cursor SR */ wm = intel_calculate_wm(clock, _cursor_wm, pineview_display_wm.fifo_size, - cpp, latency->cursor_sr); + 4, latency->cursor_sr); reg = I915_READ(DSPFW3); reg &= ~DSPFW_CURSOR_SR_MASK; reg |= FW_WM(wm, CURSOR_SR); @@ -786,7 +786,7 @@ static void pineview_update_wm(struct intel_crtc *unused_crtc) /* cursor HPLL off SR */ wm = intel_calculate_wm(clock, _cursor_hplloff_wm, pineview_display_hplloff_wm.fifo_size, - cpp, latency->cursor_hpll_disable); + 4, latency->cursor_hpll_disable); reg = I915_READ(DSPFW3); reg &= ~DSPFW_HPLL_CURSOR_MASK; reg |= FW_WM(wm, HPLL_CURSOR); @@ -842,7 +842,7 @@ static bool g4x_compute_wm0(struct drm_i915_private *dev_priv, /* Use the large buffer method to calculate cursor watermark */ line_time_us = max(htotal * 1000 / clock, 1); line_count = (cursor_latency_ns / line_time_us + 1000) / 1000; - entries = line_count * crtc->base.cursor->state->crtc_w * cpp; + entries = line_count * crtc->base.cursor->state->crtc_w * 4; tlb_miss = cursor->fifo_size*cursor->cacheline_size - hdisplay * 8; if (tlb_miss > 0) entries += tlb_miss; @@ -930,7 +930,7 @@ static bool g4x_compute_srwm(struct drm_i915_private *dev_priv, *display_wm = entries + display->guard_size; /* calculate the self-refresh watermark for display cursor */ - entries = line_count * cpp * crtc->base.cursor->state->crtc_w; + entries = line_count * 4 * crtc->base.cursor->state->crtc_w; entries = DIV_ROUND_UP(entries, cursor->cacheline_size); *cursor_wm = entries + cursor->guard_size; @@ -1736,7 +1736,7 @@ static void i965_update_wm(struct intel_crtc *unused_crtc) entries, srwm); entries = (((sr_latency_ns / line_time_us) + 1000) / 1000) * - cpp * crtc->base.cursor->state->crtc_w; + 4 * crtc->base.cursor->state->crtc_w; entries = DIV_ROUND_UP(entries, i965_cursor_wm_info.cacheline_size); cursor_sr = i965_cursor_wm_info.fifo_size - -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/15] drm/i915: s/vlv_num_wm_levels/intel_wm_num_levels/
From: Ville SyrjäläRename the VLV/CHV max_level->num_levels helper to have an intel_ prefix since it's not VLV/CHV specific and I'll want to use it on other platforms as well. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2c63abe2039c..ee045be2a5e0 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -648,6 +648,11 @@ static unsigned long intel_calculate_wm(unsigned long clock_in_khz, return wm_size; } +static int intel_wm_num_levels(struct drm_i915_private *dev_priv) +{ + return dev_priv->wm.max_level + 1; +} + static bool intel_wm_plane_visible(const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { @@ -1136,18 +1141,13 @@ static int vlv_compute_fifo(struct intel_crtc_state *crtc_state) return 0; } -static int vlv_num_wm_levels(struct drm_i915_private *dev_priv) -{ - return dev_priv->wm.max_level + 1; -} - /* mark all levels starting from 'level' as invalid */ static void vlv_invalidate_wms(struct intel_crtc *crtc, struct vlv_wm_state *wm_state, int level) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - for (; level < vlv_num_wm_levels(dev_priv); level++) { + for (; level < intel_wm_num_levels(dev_priv); level++) { enum plane_id plane_id; for_each_plane_id_on_crtc(crtc, plane_id) @@ -1174,7 +1174,7 @@ static bool vlv_raw_plane_wm_set(struct intel_crtc_state *crtc_state, int level, enum plane_id plane_id, u16 value) { struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); - int num_levels = vlv_num_wm_levels(dev_priv); + int num_levels = intel_wm_num_levels(dev_priv); bool dirty = false; for (; level < num_levels; level++) { @@ -1192,7 +1192,7 @@ static bool vlv_raw_plane_wm_compute(struct intel_crtc_state *crtc_state, { struct intel_plane *plane = to_intel_plane(plane_state->base.plane); enum plane_id plane_id = plane->id; - int num_levels = vlv_num_wm_levels(to_i915(plane->base.dev)); + int num_levels = intel_wm_num_levels(to_i915(plane->base.dev)); int level; bool dirty = false; @@ -1306,7 +1306,7 @@ static int vlv_compute_pipe_wm(struct intel_crtc_state *crtc_state) } /* initially allow all levels */ - wm_state->num_levels = vlv_num_wm_levels(dev_priv); + wm_state->num_levels = intel_wm_num_levels(dev_priv); /* * Note that enabling cxsr with no primary/sprite planes * enabled can wedge the pipe. Hence we only allow cxsr -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/15] drm/i915: Make vlv/chv watermark debug print less cryptic
From: Ville SyrjäläThe magic numbers 0,1,2 aren't all that interesting for users perhaps. Since we know what these watermark levels mean for VLV/CHV let's print their names. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 1cc13ccadee6..da2072728df2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1218,7 +1218,7 @@ static bool vlv_raw_plane_wm_compute(struct intel_crtc_state *crtc_state, out: if (dirty) - DRM_DEBUG_KMS("%s wms: [0]=%d,[1]=%d,[2]=%d\n", + DRM_DEBUG_KMS("%s watermarks: PM2=%d, PM5=%d, DDR DVFS=%d\n", plane->base.name, crtc_state->wm.vlv.raw[VLV_WM_LEVEL_PM2].plane[plane_id], crtc_state->wm.vlv.raw[VLV_WM_LEVEL_PM5].plane[plane_id], -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/15] drm/i915: Rename bunch of vlv_ watermark structures to g4x_
From: Ville SyrjäläWe'll be wanting to share some of these watermark structures on g4x, so let's rename them to have a g4x_ prefix instead of vlv_. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.h | 8 drivers/gpu/drm/i915/intel_drv.h | 6 +++--- drivers/gpu/drm/i915/intel_pm.c | 14 +++--- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 357b6c6c2f04..0a393fdc53d1 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1760,11 +1760,11 @@ struct ilk_wm_values { enum intel_ddb_partitioning partitioning; }; -struct vlv_pipe_wm { +struct g4x_pipe_wm { uint16_t plane[I915_MAX_PLANES]; }; -struct vlv_sr_wm { +struct g4x_sr_wm { uint16_t plane; uint16_t cursor; }; @@ -1774,8 +1774,8 @@ struct vlv_wm_ddl_values { }; struct vlv_wm_values { - struct vlv_pipe_wm pipe[3]; - struct vlv_sr_wm sr; + struct g4x_pipe_wm pipe[3]; + struct g4x_sr_wm sr; struct vlv_wm_ddl_values ddl[3]; uint8_t level; bool cxsr; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 54f3ff840812..d1cdd10998fa 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -512,8 +512,8 @@ enum vlv_wm_level { }; struct vlv_wm_state { - struct vlv_pipe_wm wm[NUM_VLV_WM_LEVELS]; - struct vlv_sr_wm sr[NUM_VLV_WM_LEVELS]; + struct g4x_pipe_wm wm[NUM_VLV_WM_LEVELS]; + struct g4x_sr_wm sr[NUM_VLV_WM_LEVELS]; uint8_t num_levels; bool cxsr; }; @@ -549,7 +549,7 @@ struct intel_crtc_wm_state { struct { /* "raw" watermarks (not inverted) */ - struct vlv_pipe_wm raw[NUM_VLV_WM_LEVELS]; + struct g4x_pipe_wm raw[NUM_VLV_WM_LEVELS]; /* intermediate watermarks (inverted) */ struct vlv_wm_state intermediate; /* optimal watermarks (inverted) */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ee045be2a5e0..1cc13ccadee6 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1062,7 +1062,7 @@ static bool vlv_need_sprite0_fifo_workaround(unsigned int active_planes) static int vlv_compute_fifo(struct intel_crtc_state *crtc_state) { struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); - const struct vlv_pipe_wm *raw = + const struct g4x_pipe_wm *raw = _state->wm.vlv.raw[VLV_WM_LEVEL_PM2]; struct vlv_fifo_state *fifo_state = _state->wm.vlv.fifo_state; unsigned int active_planes = crtc_state->active_planes & ~BIT(PLANE_CURSOR); @@ -1178,7 +1178,7 @@ static bool vlv_raw_plane_wm_set(struct intel_crtc_state *crtc_state, bool dirty = false; for (; level < num_levels; level++) { - struct vlv_pipe_wm *raw = _state->wm.vlv.raw[level]; + struct g4x_pipe_wm *raw = _state->wm.vlv.raw[level]; dirty |= raw->plane[plane_id] != value; raw->plane[plane_id] = value; @@ -1202,7 +1202,7 @@ static bool vlv_raw_plane_wm_compute(struct intel_crtc_state *crtc_state, } for (level = 0; level < num_levels; level++) { - struct vlv_pipe_wm *raw = _state->wm.vlv.raw[level]; + struct g4x_pipe_wm *raw = _state->wm.vlv.raw[level]; int wm = vlv_compute_wm_level(crtc_state, plane_state, level); int max_wm = plane_id == PLANE_CURSOR ? 63 : 511; @@ -1230,7 +1230,7 @@ static bool vlv_raw_plane_wm_compute(struct intel_crtc_state *crtc_state, static bool vlv_raw_plane_wm_is_valid(const struct intel_crtc_state *crtc_state, enum plane_id plane_id, int level) { - const struct vlv_pipe_wm *raw = + const struct g4x_pipe_wm *raw = _state->wm.vlv.raw[level]; const struct vlv_fifo_state *fifo_state = _state->wm.vlv.fifo_state; @@ -1315,7 +1315,7 @@ static int vlv_compute_pipe_wm(struct intel_crtc_state *crtc_state) wm_state->cxsr = crtc->pipe != PIPE_C && num_active_planes == 1; for (level = 0; level < wm_state->num_levels; level++) { - const struct vlv_pipe_wm *raw = _state->wm.vlv.raw[level]; + const struct g4x_pipe_wm *raw = _state->wm.vlv.raw[level]; const int sr_fifo_size = INTEL_INFO(dev_priv)->num_pipes * 512 - 1; if (!vlv_raw_crtc_wm_is_valid(crtc_state, level)) @@ -4785,7 +4785,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev) active->cxsr = wm->cxsr; for (level = 0; level < active->num_levels; level++) { - struct
[Intel-gfx] [PATCH 01/15] drm/i915: s/vlv_plane_wm_compute/vlv_raw_plane_wm_compute/ etc.
From: Ville SyrjäläRename some of the vlv wm functions to reflect the fact that they operate on the "raw" watermarks. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index cacb65fa2dd5..23fff9167d77 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1194,8 +1194,8 @@ static bool vlv_raw_plane_wm_set(struct intel_crtc_state *crtc_state, return dirty; } -static bool vlv_plane_wm_compute(struct intel_crtc_state *crtc_state, -const struct intel_plane_state *plane_state) +static bool vlv_raw_plane_wm_compute(struct intel_crtc_state *crtc_state, +const struct intel_plane_state *plane_state) { struct intel_plane *plane = to_intel_plane(plane_state->base.plane); enum plane_id plane_id = plane->id; @@ -1234,8 +1234,8 @@ static bool vlv_plane_wm_compute(struct intel_crtc_state *crtc_state, return dirty; } -static bool vlv_plane_wm_is_valid(const struct intel_crtc_state *crtc_state, - enum plane_id plane_id, int level) +static bool vlv_raw_plane_wm_is_valid(const struct intel_crtc_state *crtc_state, + enum plane_id plane_id, int level) { const struct vlv_pipe_wm *raw = _state->wm.vlv.raw[level]; @@ -1245,12 +1245,12 @@ static bool vlv_plane_wm_is_valid(const struct intel_crtc_state *crtc_state, return raw->plane[plane_id] <= fifo_state->plane[plane_id]; } -static bool vlv_crtc_wm_is_valid(const struct intel_crtc_state *crtc_state, int level) +static bool vlv_raw_crtc_wm_is_valid(const struct intel_crtc_state *crtc_state, int level) { - return vlv_plane_wm_is_valid(crtc_state, PLANE_PRIMARY, level) && - vlv_plane_wm_is_valid(crtc_state, PLANE_SPRITE0, level) && - vlv_plane_wm_is_valid(crtc_state, PLANE_SPRITE1, level) && - vlv_plane_wm_is_valid(crtc_state, PLANE_CURSOR, level); + return vlv_raw_plane_wm_is_valid(crtc_state, PLANE_PRIMARY, level) && + vlv_raw_plane_wm_is_valid(crtc_state, PLANE_SPRITE0, level) && + vlv_raw_plane_wm_is_valid(crtc_state, PLANE_SPRITE1, level) && + vlv_raw_plane_wm_is_valid(crtc_state, PLANE_CURSOR, level); } static int vlv_compute_pipe_wm(struct intel_crtc_state *crtc_state) @@ -1279,7 +1279,7 @@ static int vlv_compute_pipe_wm(struct intel_crtc_state *crtc_state) old_plane_state->base.crtc != >base) continue; - if (vlv_plane_wm_compute(crtc_state, plane_state)) + if (vlv_raw_plane_wm_compute(crtc_state, plane_state)) dirty |= BIT(plane->id); } @@ -1325,7 +1325,7 @@ static int vlv_compute_pipe_wm(struct intel_crtc_state *crtc_state) const struct vlv_pipe_wm *raw = _state->wm.vlv.raw[level]; const int sr_fifo_size = INTEL_INFO(dev_priv)->num_pipes * 512 - 1; - if (!vlv_crtc_wm_is_valid(crtc_state, level)) + if (!vlv_raw_crtc_wm_is_valid(crtc_state, level)) break; for_each_plane_id_on_crtc(crtc, plane_id) { -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/15] drm/i915: Drop the debug message from vlv_get_fifo_size()
From: Ville SyrjäläSeeing the display FIFO sizes at driver load time doesn't really provide anything useful for us, so let's just drop the debug message. One can always use eg. intel_watermarks to dump out the hardware settings prior to loading the driver. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 23fff9167d77..2c63abe2039c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -454,13 +454,6 @@ static void vlv_get_fifo_size(struct intel_crtc_state *crtc_state) fifo_state->plane[PLANE_SPRITE0] = sprite1_start - sprite0_start; fifo_state->plane[PLANE_SPRITE1] = 511 - sprite1_start; fifo_state->plane[PLANE_CURSOR] = 63; - - DRM_DEBUG_KMS("Pipe %c FIFO size: %d/%d/%d/%d\n", - pipe_name(pipe), - fifo_state->plane[PLANE_PRIMARY], - fifo_state->plane[PLANE_SPRITE0], - fifo_state->plane[PLANE_SPRITE1], - fifo_state->plane[PLANE_CURSOR]); } static int i9xx_get_fifo_size(struct drm_i915_private *dev_priv, int plane) -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/15] drm/i915: Two stage watermarks for g4x
From: Ville SyrjäläThis makes g4x follow the two stage watermark programming approach as well, and as a bonus exposes the video sprites on g4x. There is one slight problem with merging the wms from multiple pipes; If one pipe is currently enabled and we're about to enabled another one, we should turn off CxSR before the second pipe gets enabled as the FIFO will get repartitioned. This could also happen when there's a parallel watermark update on the first pipe, so a dumb approach of just disabling CxSR in the modeset path doesn't really work. I think the proper fix will involve some shuffling of code in the modeset path because it's currently a bit of a mess. So with the current code there could be an occasional underrun reported when enabling the second pipe. Entire series available here: git://github.com/vsyrjala/linux.git g4x_atomic_wm_8 Ville Syrjälä (15): drm/i915: s/vlv_plane_wm_compute/vlv_raw_plane_wm_compute/ etc. drm/i915: Drop the debug message from vlv_get_fifo_size() drm/i915: s/vlv_num_wm_levels/intel_wm_num_levels/ drm/i915: Rename bunch of vlv_ watermark structures to g4x_ drm/i915: Make vlv/chv watermark debug print less cryptic drm/i915: Document CxSR drm/i915: Fix cursor 'cpp' in watermark calculatins for old platforms drm/i915: Fix the g4x watermark TLB miss workaround drm/i915: Refactor the g4x TLB miss w/a to a helper drm/i915: Refactor wm calculations drm/i915: Apply the g4x TLB miss w/a to SR watermarks as well drm/i915: Two stage watermarks for g4x drm/i915: Enable HPLL watermarks on g4x drm/i915: Add g4x watermark tracepoint drm/i915: Add support for sprites on g4x drivers/gpu/drm/i915/i915_debugfs.c | 12 +- drivers/gpu/drm/i915/i915_drv.h | 20 +- drivers/gpu/drm/i915/i915_trace.h| 49 ++ drivers/gpu/drm/i915/intel_device_info.c |2 +- drivers/gpu/drm/i915/intel_display.c | 29 +- drivers/gpu/drm/i915/intel_drv.h | 34 +- drivers/gpu/drm/i915/intel_pm.c | 1254 ++ drivers/gpu/drm/i915/intel_sprite.c | 18 +- 8 files changed, 1081 insertions(+), 337 deletions(-) -- 2.10.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PULL] drm-misc-next-fixes
On Thu, Apr 20, 2017 at 11:50:02PM +0200, Daniel Vetter wrote: > On Thu, Apr 20, 2017 at 10:11 PM, Sean Paulwrote: > > Hi Dave, > > A few fixes for you to pick up. The driver changes are trivial, and the > > maintainer change was necessitated by the sti fix. The headliner here is the > > dma_buf_ops rename, since it touches so many drivers. Everything looks sane > > and > > builds with that change, so it shouldn't cause problems. > > > > > > drm-misc-next-fixes-2017-04-20: > > > > Core changes: > > - Maintain sti via drm-misc (Vincent) > > - Rename dma_buf_ops->kmap_* to avoid naming collision (Logan) > > This one touches v4l and ion and is acked by the corresponding > maintainers (but Sumit forgot to record that when applying the patch, > and Sean didn't highlight it in the summary). > > Sean, should we augment the template that cross-subsystem stuff > (outside of what's on-topic for drm-misc) needs to be highlighted > specifically in the pull summary? Yeah, I think that's reasonable. I'd like to think that I would have done a better job highlighting this had it been a standard header. Going forward, I'll add UABI and cross-sub headers to force myself to be more careful :) > > Off for vacation for real now. Enjoy! Sean > > Cheers, Daniel > > > > > Driver changes: > > - Fix UHD displays on stih407 (Vincent) > > - Fix uninitialized var return in atmel-hlcdc (Dan) > > > > Take care, Sean > > > > > > The following changes since commit 8cb68c83ab99a474ae847602f0c514d0fe17cc82: > > > > drm: Fix get_property logic fumble (2017-04-12 18:11:32 +0200) > > > > are available in the git repository at: > > > > git://anongit.freedesktop.org/git/drm-misc > > tags/drm-misc-next-fixes-2017-04-20 > > > > for you to fetch changes up to f9b67f0014cba18f1aabb6fa9272335a043eb6fd: > > > > dma-buf: Rename dma-ops to prevent conflict with kunmap_atomic macro > > (2017-04-20 13:47:46 +0530) > > > > > > drm-misc-next-fixes-2017-04-20 > > > > Core changes: > > - Maintain sti via drm-misc (Vincent) > > - Rename dma_buf_ops->kmap_* to avoid naming collision (Logan) > > > > Driver changes: > > - Fix UHD displays on stih407 (Vincent) > > - Fix uninitialized var return in atmel-hlcdc (Dan) > > > > > > Dan Carpenter (1): > > drm: atmel-hlcdc: Uninitialized return in atmel_hlcdc_create_outputs() > > > > Logan Gunthorpe (1): > > dma-buf: Rename dma-ops to prevent conflict with kunmap_atomic macro > > > > Vincent Abriou (2): > > MAINTAINERS: add drm/sti driver into drm-misc > > drm/sti: fix GDP size to support up to UHD resolution > > > > MAINTAINERS | 2 +- > > drivers/dma-buf/dma-buf.c| 16 > > drivers/gpu/drm/armada/armada_gem.c | 8 > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 5 ++--- > > drivers/gpu/drm/drm_prime.c | 8 > > drivers/gpu/drm/i915/i915_gem_dmabuf.c | 8 > > drivers/gpu/drm/i915/selftests/mock_dmabuf.c | 8 > > drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c| 8 > > drivers/gpu/drm/sti/sti_gdp.c| 12 +++- > > drivers/gpu/drm/tegra/gem.c | 8 > > drivers/gpu/drm/udl/udl_dmabuf.c | 8 > > drivers/gpu/drm/vmwgfx/vmwgfx_prime.c| 8 > > drivers/media/v4l2-core/videobuf2-dma-contig.c | 4 ++-- > > drivers/media/v4l2-core/videobuf2-dma-sg.c | 4 ++-- > > drivers/media/v4l2-core/videobuf2-vmalloc.c | 4 ++-- > > drivers/staging/android/ion/ion.c| 8 > > include/linux/dma-buf.h | 22 > > +++--- > > 17 files changed, 71 insertions(+), 70 deletions(-) > > > > -- > > Sean Paul, Software Engineer, Google / Chromium OS > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Sean Paul, Software Engineer, Google / Chromium OS ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] tests/pm_sseu: Re-enable the test
On Tue, Apr 18, 2017 at 04:45:29PM -0700, Oscar Mateo wrote: > This test got inadvertently disabled by commit 83884e97 (Restore > "lib: Open debugfs files for the given DRM device"). > > Cc: Jeff McGee> Cc: Chris Wilson > Signed-off-by: Oscar Mateo Reviewed-by: Michał Winiarski -Michał > --- > tests/pm_sseu.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio (rev2)
On Fri, Apr 21, 2017 at 02:26:14PM -, Patchwork wrote: > == Series Details == > > Series: drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio (rev2) > URL : https://patchwork.freedesktop.org/series/23340/ > State : success > > == Summary == > > Series 23340v2 drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio > https://patchwork.freedesktop.org/api/1.0/series/23340/revisions/2/mbox/ > > Test drv_module_reload: > Subgroup basic-reload-inject: > incomplete -> PASS (fi-bdw-5557u) fdo#100750 > Test gem_exec_suspend: > Subgroup basic-s4-devices: > pass -> DMESG-WARN (fi-kbl-7560u) fdo#100125 > Test kms_flip: > Subgroup basic-flip-vs-wf_vblank: > pass -> FAIL (fi-skl-6700hq) fdo#99739 > > fdo#100750 https://bugs.freedesktop.org/show_bug.cgi?id=100750 > fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125 > fdo#99739 https://bugs.freedesktop.org/show_bug.cgi?id=99739 Applied, thanks for the review and testing. As it is in principle a revert of the change to intel_wait_for_register() it seems safe enough to apply for extended testing. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PULL] drm-misc-next-fixes
On 21 April 2017 at 03:20, Daniel Vetterwrote: >> - Rename dma_buf_ops->kmap_* to avoid naming collision (Logan) > > This one touches v4l and ion and is acked by the corresponding > maintainers (but Sumit forgot to record that when applying the patch, > and Sean didn't highlight it in the summary). > Hmm - I assumed that the mbox I downloaded (after all the acks got registered in patchworks) would've had the Acks? Didn't cross-check, my bad. Will of course take care from next time on. > Sean, should we augment the template that cross-subsystem stuff > (outside of what's on-topic for drm-misc) needs to be highlighted > specifically in the pull summary? > > Off for vacation for real now. > > Cheers, Daniel > Best, Sumit. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v5] benchmarks/gem_wsim: Command submission workload simulator
From: Tvrtko UrsulinTool which emits batch buffers to engines with configurable sequences, durations, contexts, dependencies and userspace waits. Unfinished but shows promise so sending out for early feedback. v2: * Load workload descriptors from files. (also -w) * Help text. * Calibration control if needed. (-t) * NORELOC | LUT to eb flags. * Added sample workload to wsim/workload1. v3: * Multiple parallel different workloads (-w -w ...). * Multi-context workloads. * Variable (random) batch length. * Load balancing (round robin and queue depth estimation). * Workloads delays and explicit sync steps. * Workload frequency (period) control. v4: * Fixed queue-depth estimation by creating separate batches per engine when qd load balancing is on. * Dropped separate -s cmd line option. It can turn itself on automatically when needed. * Keep a single status page and lie about the write hazard as suggested by Chris. * Use batch_start_offset for controlling the batch duration. (Chris) * Set status page object cache level. (Chris) * Moved workload description to a README. * Tidied example workloads. * Some other cleanups and refactorings. v5: * Master and background workloads (-W / -w). * Single batch per step is enough even when balancing. (Chris) * Use hars_petruska_f54_1_random IGT functions and see to zero at start. (Chris) * Use WC cache domain when WC mapping. (Chris) * Keep seqnos 64-bytes apart in the status page. (Chris) * Add workload throttling and queue-depth throttling commands. (Chris) TODO list: * Fence support. * Better error handling. * Less 1980's workload parsing. * Proper workloads. * Threads? * ... ? Signed-off-by: Tvrtko Ursulin Cc: Chris Wilson Cc: "Rogozhkin, Dmitry V" --- benchmarks/Makefile.sources |1 + benchmarks/gem_wsim.c| 1189 ++ benchmarks/wsim/README | 56 ++ benchmarks/wsim/media_17i7.wsim |7 + benchmarks/wsim/media_load_balance_17i7.wsim |7 + benchmarks/wsim/vcs1.wsim| 26 + benchmarks/wsim/vcs_balanced.wsim| 26 + lib/igt_core.c | 26 + lib/igt_core.h |1 + 9 files changed, 1339 insertions(+) create mode 100644 benchmarks/gem_wsim.c create mode 100644 benchmarks/wsim/README create mode 100644 benchmarks/wsim/media_17i7.wsim create mode 100644 benchmarks/wsim/media_load_balance_17i7.wsim create mode 100644 benchmarks/wsim/vcs1.wsim create mode 100644 benchmarks/wsim/vcs_balanced.wsim diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources index 3af54ebe36f2..3a941150abb3 100644 --- a/benchmarks/Makefile.sources +++ b/benchmarks/Makefile.sources @@ -14,6 +14,7 @@ benchmarks_prog_list =\ gem_prw \ gem_set_domain \ gem_syslatency \ + gem_wsim\ kms_vblank \ prime_lookup\ vgem_mmap \ diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c new file mode 100644 index ..3d6670fdb815 --- /dev/null +++ b/benchmarks/gem_wsim.c @@ -0,0 +1,1189 @@ +/* + * Copyright © 2017 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +#include "intel_chipset.h" +#include "drm.h" +#include "ioctl_wrappers.h" +#include "drmtest.h" +#include "intel_io.h" +#include "igt_rand.h" + +enum
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio (rev2)
== Series Details == Series: drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio (rev2) URL : https://patchwork.freedesktop.org/series/23340/ State : success == Summary == Series 23340v2 drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio https://patchwork.freedesktop.org/api/1.0/series/23340/revisions/2/mbox/ Test drv_module_reload: Subgroup basic-reload-inject: incomplete -> PASS (fi-bdw-5557u) fdo#100750 Test gem_exec_suspend: Subgroup basic-s4-devices: pass -> DMESG-WARN (fi-kbl-7560u) fdo#100125 Test kms_flip: Subgroup basic-flip-vs-wf_vblank: pass -> FAIL (fi-skl-6700hq) fdo#99739 fdo#100750 https://bugs.freedesktop.org/show_bug.cgi?id=100750 fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125 fdo#99739 https://bugs.freedesktop.org/show_bug.cgi?id=99739 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:435s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:430s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:573s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:515s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:566s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:484s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:481s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:414s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:405s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:427s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:488s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:466s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:452s fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:567s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:455s fi-skl-6700hqtotal:278 pass:260 dwarn:0 dfail:0 fail:1 skip:17 time:572s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:461s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:488s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:428s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:533s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:402s 0fb03930a694adb823f77a954d79905fdbe6d727 drm-tip: 2017y-04m-21d-08h-59m-17s UTC integration manifest 16d4e52 drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4530/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio
The busy-spin, as the first stage of intel_wait_for_register(), is currently under suspicion for causing: [ 62.034926] NMI watchdog: Watchdog detected hard LOCKUP on cpu 1 [ 62.034928] Modules linked in: i2c_dev i915 intel_gtt drm_kms_helper prime_numbers [ 62.034932] CPU: 1 PID: 183 Comm: kworker/1:2 Not tainted 4.11.0-rc7+ #471 [ 62.034933] Hardware name: /, BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015 [ 62.034934] Workqueue: pm pm_runtime_work [ 62.034936] task: 880275a04ec0 task.stack: c92d8000 [ 62.034936] RIP: 0010:__intel_wait_for_register_fw+0x77/0x1a0 [i915] [ 62.034937] RSP: 0018:c92dbc38 EFLAGS: 0082 [ 62.034939] RAX: c90003530094 RBX: 00130094 RCX: 0001 [ 62.034940] RDX: 00a1 RSI: 88027fd15e58 RDI: [ 62.034941] RBP: c92dbc78 R08: 0002 R09: [ 62.034942] R10: c92dbc18 R11: 880276429dd0 R12: 8802707c [ 62.034943] R13: 00a0 R14: R15: fffefc10 [ 62.034945] FS: () GS:88027fd0() knlGS: [ 62.034945] CS: 0010 DS: ES: CR0: 80050033 [ 62.034947] CR2: 7ffd3cd98ff8 CR3: 000274c19000 CR4: 001006e0 [ 62.034947] Call Trace: [ 62.034948] intel_wait_for_register+0x77/0x140 [i915] [ 62.034949] vlv_suspend_complete+0x23/0x5b0 [i915] [ 62.034950] intel_runtime_suspend+0x16c/0x2a0 [i915] [ 62.034950] pci_pm_runtime_suspend+0x50/0x180 [ 62.034951] ? pci_pm_runtime_resume+0xa0/0xa0 [ 62.034952] __rpm_callback+0xc5/0x210 [ 62.034953] rpm_callback+0x1f/0x80 [ 62.034953] ? pci_pm_runtime_resume+0xa0/0xa0 [ 62.034954] rpm_suspend+0x118/0x580 [ 62.034955] pm_runtime_work+0x64/0x90 [ 62.034956] process_one_work+0x1bb/0x3e0 [ 62.034956] worker_thread+0x46/0x4f0 [ 62.034957] ? __schedule+0x18b/0x610 [ 62.034958] kthread+0xff/0x140 [ 62.034958] ? process_one_work+0x3e0/0x3e0 [ 62.034959] ? kthread_create_on_node+ and related hard lockups in CI for byt and bsw. Note this effectively reverts commits 41ce405e6894 and b27366958869 ("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()") v2: Convert bool allow into a u32 mask for clarity and repeat the comment on vlv rc6 timing to justify the 3ms timeout used for the wait (Ville) Fixes: 41ce405e6894 ("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()") Fixes: b27366958869 ("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100718 Signed-off-by: Chris WilsonCc: Tvrtko Ursulin Cc: Ville Syrjälä Cc: Tomi Sarvela Reviewed-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_drv.c | 46 + 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index e4f902f61e57..dc48eae4072c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2179,6 +2179,20 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv) I915_WRITE(VLV_GUNIT_CLOCK_GATE2, s->clock_gate_dis2); } +static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv, + u32 mask, u32 val) +{ + /* The HW does not like us polling for PW_STATUS frequently, so +* use the sleeping loop rather than risk the busy spin within +* intel_wait_for_register(). +* +* Transitioning between RC6 states should be at most 2ms (see +* valleyview_enable_rps) so use a 3ms timeout. +*/ + return wait_for((I915_READ_NOTRACE(VLV_GTLC_PW_STATUS) & mask) == val, + 3); +} + int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on) { u32 val; @@ -2207,8 +2221,9 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on) static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow) { + u32 mask; u32 val; - int err = 0; + int err; val = I915_READ(VLV_GTLC_WAKE_CTRL); val &= ~VLV_GTLC_ALLOWWAKEREQ; @@ -2217,45 +2232,32 @@ static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow) I915_WRITE(VLV_GTLC_WAKE_CTRL, val); POSTING_READ(VLV_GTLC_WAKE_CTRL); - err = intel_wait_for_register(dev_priv, - VLV_GTLC_PW_STATUS, - VLV_GTLC_ALLOWWAKEACK, - allow, - 1); + mask = VLV_GTLC_ALLOWWAKEACK; + val = allow ? mask : 0; + + err =
Re: [Intel-gfx] [PATCH] drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio
On Fri, Apr 21, 2017 at 02:00:40PM +0100, Chris Wilson wrote: > The busy-spin, as the first stage of intel_wait_for_register, is > currently under suspicion for causing: > > [ 62.034926] NMI watchdog: Watchdog detected hard LOCKUP on cpu 1 > [ 62.034928] Modules linked in: i2c_dev i915 intel_gtt drm_kms_helper > prime_numbers > [ 62.034932] CPU: 1 PID: 183 Comm: kworker/1:2 Not tainted 4.11.0-rc7+ #471 > [ 62.034933] Hardware name: /, BIOS > PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015 > [ 62.034934] Workqueue: pm pm_runtime_work > [ 62.034936] task: 880275a04ec0 task.stack: c92d8000 > [ 62.034936] RIP: 0010:__intel_wait_for_register_fw+0x77/0x1a0 [i915] > [ 62.034937] RSP: 0018:c92dbc38 EFLAGS: 0082 > [ 62.034939] RAX: c90003530094 RBX: 00130094 RCX: > 0001 > [ 62.034940] RDX: 00a1 RSI: 88027fd15e58 RDI: > > [ 62.034941] RBP: c92dbc78 R08: 0002 R09: > > [ 62.034942] R10: c92dbc18 R11: 880276429dd0 R12: > 8802707c > [ 62.034943] R13: 00a0 R14: R15: > fffefc10 > [ 62.034945] FS: () GS:88027fd0() > knlGS: > [ 62.034945] CS: 0010 DS: ES: CR0: 80050033 > [ 62.034947] CR2: 7ffd3cd98ff8 CR3: 000274c19000 CR4: > 001006e0 > [ 62.034947] Call Trace: > [ 62.034948] intel_wait_for_register+0x77/0x140 [i915] > [ 62.034949] vlv_suspend_complete+0x23/0x5b0 [i915] > [ 62.034950] intel_runtime_suspend+0x16c/0x2a0 [i915] > [ 62.034950] pci_pm_runtime_suspend+0x50/0x180 > [ 62.034951] ? pci_pm_runtime_resume+0xa0/0xa0 > [ 62.034952] __rpm_callback+0xc5/0x210 > [ 62.034953] rpm_callback+0x1f/0x80 > [ 62.034953] ? pci_pm_runtime_resume+0xa0/0xa0 > [ 62.034954] rpm_suspend+0x118/0x580 > [ 62.034955] pm_runtime_work+0x64/0x90 > [ 62.034956] process_one_work+0x1bb/0x3e0 > [ 62.034956] worker_thread+0x46/0x4f0 > [ 62.034957] ? __schedule+0x18b/0x610 > [ 62.034958] kthread+0xff/0x140 > [ 62.034958] ? process_one_work+0x3e0/0x3e0 > [ 62.034959] ? kthread_create_on_node+ > > and related hard lockups in CI for byt and bsw. > > Note this effectively reverts commits 41ce405e6894 and b27366958869 > ("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()") > > Fixes: 41ce405e6894 ("drm/i915: Convert wait_for(I915_READ(reg)) to > intel_wait_for_register()") > Fixes: b27366958869 ("drm/i915: Convert wait_for(I915_READ(reg)) to > intel_wait_for_register()") > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100718 > Signed-off-by: Chris Wilson> Cc: Tvrtko Ursulin > Cc: Ville Syrjälä > Cc: Tomi Sarvela > --- > drivers/gpu/drm/i915/i915_drv.c | 39 +-- > 1 file changed, 17 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index e4f902f61e57..89b517c478e8 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -2179,6 +2179,17 @@ static void vlv_restore_gunit_s0ix_state(struct > drm_i915_private *dev_priv) > I915_WRITE(VLV_GUNIT_CLOCK_GATE2, s->clock_gate_dis2); > } > > +static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv, > + u32 mask, u32 val) > +{ > + /* The HW does not like us polling for PW_STATUS frequently, so > + * use the sleeping loop rather than risk the busy spin within > + * intel_wait_for_register(). > + */ > + return wait_for((I915_READ_NOTRACE(VLV_GTLC_PW_STATUS) & mask) == val, > + 3); > +} > + > int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on) > { > u32 val; > @@ -2208,7 +2219,7 @@ int vlv_force_gfx_clock(struct drm_i915_private > *dev_priv, bool force_on) > static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow) > { > u32 val; > - int err = 0; > + int err; > > val = I915_READ(VLV_GTLC_WAKE_CTRL); > val &= ~VLV_GTLC_ALLOWWAKEREQ; > @@ -2217,45 +2228,29 @@ static int vlv_allow_gt_wake(struct drm_i915_private > *dev_priv, bool allow) > I915_WRITE(VLV_GTLC_WAKE_CTRL, val); > POSTING_READ(VLV_GTLC_WAKE_CTRL); > > - err = intel_wait_for_register(dev_priv, > - VLV_GTLC_PW_STATUS, > - VLV_GTLC_ALLOWWAKEACK, > - allow, > - 1); > + err = vlv_wait_for_pw_status(dev_priv, VLV_GTLC_ALLOWWAKEACK, allow); Looks a bit funny to wait for a boolean. Maybe 'allow ? VLV_GTLC_ALLOWWAKEACK : 0' to make this a little less confusing? > if (err) >
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio
== Series Details == Series: drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio URL : https://patchwork.freedesktop.org/series/23340/ State : failure == Summary == Series 23340v1 drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio https://patchwork.freedesktop.org/api/1.0/series/23340/revisions/1/mbox/ Test drv_module_reload: Subgroup basic-reload-inject: incomplete -> PASS (fi-bdw-5557u) fdo#100750 Test gem_exec_suspend: Subgroup basic-s4-devices: pass -> DMESG-WARN (fi-kbl-7560u) fdo#100125 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: pass -> FAIL (fi-ivb-3520m) Subgroup suspend-read-crc-pipe-b: pass -> FAIL (fi-ivb-3520m) Subgroup suspend-read-crc-pipe-c: pass -> DMESG-WARN (fi-bsw-n3050) fdo#100651 pass -> FAIL (fi-ivb-3520m) Test pm_rpm: Subgroup basic-pci-d3-state: pass -> DMESG-WARN (fi-bsw-n3050) fdo#100718 fdo#100750 https://bugs.freedesktop.org/show_bug.cgi?id=100750 fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125 fdo#100651 https://bugs.freedesktop.org/show_bug.cgi?id=100651 fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:432s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:430s fi-bsw-n3050 total:278 pass:240 dwarn:2 dfail:0 fail:0 skip:36 time:562s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:512s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:567s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:487s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:482s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:407s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:413s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:421s fi-ivb-3520m total:278 pass:257 dwarn:0 dfail:0 fail:3 skip:18 time:442s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:484s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:457s fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:573s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:450s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:574s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:462s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:490s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:428s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:534s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:410s 0fb03930a694adb823f77a954d79905fdbe6d727 drm-tip: 2017y-04m-21d-08h-59m-17s UTC integration manifest 2d36703 drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4529/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio
The busy-spin, as the first stage of intel_wait_for_register, is currently under suspicion for causing: [ 62.034926] NMI watchdog: Watchdog detected hard LOCKUP on cpu 1 [ 62.034928] Modules linked in: i2c_dev i915 intel_gtt drm_kms_helper prime_numbers [ 62.034932] CPU: 1 PID: 183 Comm: kworker/1:2 Not tainted 4.11.0-rc7+ #471 [ 62.034933] Hardware name: /, BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015 [ 62.034934] Workqueue: pm pm_runtime_work [ 62.034936] task: 880275a04ec0 task.stack: c92d8000 [ 62.034936] RIP: 0010:__intel_wait_for_register_fw+0x77/0x1a0 [i915] [ 62.034937] RSP: 0018:c92dbc38 EFLAGS: 0082 [ 62.034939] RAX: c90003530094 RBX: 00130094 RCX: 0001 [ 62.034940] RDX: 00a1 RSI: 88027fd15e58 RDI: [ 62.034941] RBP: c92dbc78 R08: 0002 R09: [ 62.034942] R10: c92dbc18 R11: 880276429dd0 R12: 8802707c [ 62.034943] R13: 00a0 R14: R15: fffefc10 [ 62.034945] FS: () GS:88027fd0() knlGS: [ 62.034945] CS: 0010 DS: ES: CR0: 80050033 [ 62.034947] CR2: 7ffd3cd98ff8 CR3: 000274c19000 CR4: 001006e0 [ 62.034947] Call Trace: [ 62.034948] intel_wait_for_register+0x77/0x140 [i915] [ 62.034949] vlv_suspend_complete+0x23/0x5b0 [i915] [ 62.034950] intel_runtime_suspend+0x16c/0x2a0 [i915] [ 62.034950] pci_pm_runtime_suspend+0x50/0x180 [ 62.034951] ? pci_pm_runtime_resume+0xa0/0xa0 [ 62.034952] __rpm_callback+0xc5/0x210 [ 62.034953] rpm_callback+0x1f/0x80 [ 62.034953] ? pci_pm_runtime_resume+0xa0/0xa0 [ 62.034954] rpm_suspend+0x118/0x580 [ 62.034955] pm_runtime_work+0x64/0x90 [ 62.034956] process_one_work+0x1bb/0x3e0 [ 62.034956] worker_thread+0x46/0x4f0 [ 62.034957] ? __schedule+0x18b/0x610 [ 62.034958] kthread+0xff/0x140 [ 62.034958] ? process_one_work+0x3e0/0x3e0 [ 62.034959] ? kthread_create_on_node+ and related hard lockups in CI for byt and bsw. Note this effectively reverts commits 41ce405e6894 and b27366958869 ("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()") Fixes: 41ce405e6894 ("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()") Fixes: b27366958869 ("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100718 Signed-off-by: Chris WilsonCc: Tvrtko Ursulin Cc: Ville Syrjälä Cc: Tomi Sarvela --- drivers/gpu/drm/i915/i915_drv.c | 39 +-- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index e4f902f61e57..89b517c478e8 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2179,6 +2179,17 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv) I915_WRITE(VLV_GUNIT_CLOCK_GATE2, s->clock_gate_dis2); } +static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv, + u32 mask, u32 val) +{ + /* The HW does not like us polling for PW_STATUS frequently, so +* use the sleeping loop rather than risk the busy spin within +* intel_wait_for_register(). +*/ + return wait_for((I915_READ_NOTRACE(VLV_GTLC_PW_STATUS) & mask) == val, + 3); +} + int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on) { u32 val; @@ -2208,7 +2219,7 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on) static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow) { u32 val; - int err = 0; + int err; val = I915_READ(VLV_GTLC_WAKE_CTRL); val &= ~VLV_GTLC_ALLOWWAKEREQ; @@ -2217,45 +2228,29 @@ static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow) I915_WRITE(VLV_GTLC_WAKE_CTRL, val); POSTING_READ(VLV_GTLC_WAKE_CTRL); - err = intel_wait_for_register(dev_priv, - VLV_GTLC_PW_STATUS, - VLV_GTLC_ALLOWWAKEACK, - allow, - 1); + err = vlv_wait_for_pw_status(dev_priv, VLV_GTLC_ALLOWWAKEACK, allow); if (err) DRM_ERROR("timeout disabling GT waking\n"); return err; } -static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv, -bool wait_for_on) +static void vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv, + bool wait_for_on) { u32 mask; u32 val; -
Re: [Intel-gfx] [PATCH v8 1/2] ACPI / bus: Introduce a list of ids for "always present" devices
Hi, On 21-04-17 13:38, Andy Shevchenko wrote: On Fri, 2017-04-21 at 12:47 +0200, Hans de Goede wrote: Several Bay / Cherry Trail devices (all of which ship with Windows 10) hide the LPSS PWM controller in ACPI, typically the _STA method looks like this: Method (_STA, 0, NotSerialized) // _STA: Status { If (OSID == One) { Return (Zero) } Return (0x0F) } Where OSID is some dark magic seen in all Cherry Trail ACPI tables making the machine behave differently depending on which OS it *thinks* it is booting, this gets set in a number of ways which we cannot control, on some newer machines it simple hardcoded to "One" aka win10. This causes the PWM controller to get hidden, which means Linux cannot control the backlight level on cht based tablets / laptops. Since loading the driver for this does no harm (the only in kernel user of it is the i915 driver, which will only uses it when it needs it), this commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT for the LPSS PWM device, fixing the lack of backlight control. drivers/acpi/Makefile| 1 + drivers/acpi/bus.c | 5 +++ drivers/acpi/x86/x86_utils.c | 85 Perhaps .../x86/utils.c ? I thought that utils.c would be too generic, but that was mainly thinking about module kernel cmdline options which do not apply here, still having a somewhat unique basename seems useful. include/acpi/acpi_bus.h | 9 + diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 34fbe02..784bda6 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -114,6 +114,11 @@ int acpi_bus_get_status(struct acpi_device *device) acpi_status status; unsigned long long sta; + if (acpi_device_always_present(device)) { + acpi_set_device_status(device, ACPI_STA_DEFAULT); + return 0; + } + status = acpi_bus_get_status_handle(device->handle, ); if (ACPI_FAILURE(status)) return -ENODEV; +#define ICPU(model){ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, } + +#define ENTRY(hid, uid, cpu_models) { cpu_models -> cpu_model ? Or I missed that it's an array? It may be an array, e.g. : ENTRY("INT0002", "1", (ICPU(INTEL_FAM6_ATOM_SILVERMONT1), ICPU(INTEL_FAM6_ATOM_AIRMONT)) ), Note this is a theoretical example (for now). Regards, Hans ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v8 1/2] ACPI / bus: Introduce a list of ids for "always present" devices
On Fri, 2017-04-21 at 12:47 +0200, Hans de Goede wrote: > Several Bay / Cherry Trail devices (all of which ship with Windows 10) > hide > the LPSS PWM controller in ACPI, typically the _STA method looks like > this: > > Method (_STA, 0, NotSerialized) // _STA: Status > { > If (OSID == One) > { > Return (Zero) > } > > Return (0x0F) > } > > Where OSID is some dark magic seen in all Cherry Trail ACPI tables > making > the machine behave differently depending on which OS it *thinks* it is > booting, this gets set in a number of ways which we cannot control, on > some newer machines it simple hardcoded to "One" aka win10. > > This causes the PWM controller to get hidden, which means Linux cannot > control the backlight level on cht based tablets / laptops. > > Since loading the driver for this does no harm (the only in kernel > user > of it is the i915 driver, which will only uses it when it needs it), > this > commit makes acpi_bus_get_status() always set status to > ACPI_STA_DEFAULT > for the LPSS PWM device, fixing the lack of backlight control. > > drivers/acpi/Makefile| 1 + > drivers/acpi/bus.c | 5 +++ > drivers/acpi/x86/x86_utils.c | 85 > Perhaps .../x86/utils.c ? > include/acpi/acpi_bus.h | 9 + > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index 34fbe02..784bda6 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -114,6 +114,11 @@ int acpi_bus_get_status(struct acpi_device > *device) > acpi_status status; > unsigned long long sta; > > + if (acpi_device_always_present(device)) { > + acpi_set_device_status(device, ACPI_STA_DEFAULT); > + return 0; > + } > + > status = acpi_bus_get_status_handle(device->handle, ); > if (ACPI_FAILURE(status)) > return -ENODEV; > > +#define ICPU(model) { X86_VENDOR_INTEL, 6, model, > X86_FEATURE_ANY, } > + > +#define ENTRY(hid, uid, cpu_models) { cpu_models -> cpu_model ? Or I missed that it's an array? -- Andy ShevchenkoIntel Finland Oy ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 1/2] ACPI / bus: Introduce a list of ids for "always present" devices
Hi, On 21-04-17 12:40, Rafael J. Wysocki wrote: On Friday, April 21, 2017 12:42:31 PM Hans de Goede wrote: HI, [cut] And in that path, which I seem to have overlooked before, the acpi_set_device_status() call is too early for invoking acpi_device_always_present(adev), so the latter should be called directly from acpi_add_single_object() like acpi_init_device_object(device, handle, type, sta); if (acpi_device_always_present(adev)) acpi_set_device_status(adev, ACPI_STA_DEFAULT); That is not necessary, the place(s) where we care about status being fixed-up for always-present devices, so that they get scanned / their platform device initiated, is in acpi_bus_attach() which already calls acpi_bus_get_status() and thus gets the acpi_device_always_present() check applied before checking. For hotplugged devices there are acpi_scan_bus_check and acpi_scan_device_check but those both also already call acpi_bus_get_status() before checking adev->status. OK Which probably means that we don't need to initialize adev->status in acpi_init_device_object() to anything meaningful, do we? Right, I don't think that is necessary. But maybe it is necessary for some special cases (e.g. power resources) ? For power resources _STA is defined differently at all and we don't even call acpi_add_single_object() for them. :-) Are there any other special cases in which that may matter? Not that I'm aware of, but I'm in no way an expert when it comes to the ACPI subsystem. Regards, Hans ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v8 1/2] ACPI / bus: Introduce a list of ids for "always present" devices
Several Bay / Cherry Trail devices (all of which ship with Windows 10) hide the LPSS PWM controller in ACPI, typically the _STA method looks like this: Method (_STA, 0, NotSerialized) // _STA: Status { If (OSID == One) { Return (Zero) } Return (0x0F) } Where OSID is some dark magic seen in all Cherry Trail ACPI tables making the machine behave differently depending on which OS it *thinks* it is booting, this gets set in a number of ways which we cannot control, on some newer machines it simple hardcoded to "One" aka win10. This causes the PWM controller to get hidden, which means Linux cannot control the backlight level on cht based tablets / laptops. Since loading the driver for this does no harm (the only in kernel user of it is the i915 driver, which will only uses it when it needs it), this commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT for the LPSS PWM device, fixing the lack of backlight control. Signed-off-by: Hans de Goede--- Changes in v2: -Use pr_debug instead of ACPI_DEBUG_PRINT Changes in v3: -Un-inline acpi_set_device_status and do the always_present_device_ids table check inside the un-inlined version of it Changes in v4: -Use dev_info instead of pr_debug -Not only check for ACPI HID but also for CPU (SoC) model so as to not for devices present on other models then for which the quirk is intended and to avoid enabling unrelated ACPI devices which happen to use the same HID Changes in v5: -Only do the dev_info once per device (acpi_set_device_status gets called multiple times per device during boot) Changes in v6: -Allow specifying more then one CPU-model for a single HID -Not only match the HID but also the UID, like on Cherry Trail, on some Bay Trail Windows 10 tablets we need to enable the PWM controller to get working backlight even though _STA returns 0. The Bay Trail SoC has 2 PWM controllers and we only need the first one. UID matching will allows adding an entry for Bay Trail which only enables the first PWM controller Changes in v7: -Put the actual x86 specific matching code and table with always present device HID + UID + CPU model entries in a new x86/x86_utils.c file which provides an acpi_device_always_present() helper function on x86, on non x86 a stub which always returns false is provided -Squash in the addition of the Bay Trail PWM entry in the table as it is there for the same reasons as the Cherry Trail entry is there Changes in v8: -Move the acpi_device_always_present() check to acpi_bus_get_status(), leave acpi_set_device_status unchanged --- drivers/acpi/Makefile| 1 + drivers/acpi/bus.c | 5 +++ drivers/acpi/x86/x86_utils.c | 85 include/acpi/acpi_bus.h | 9 + 4 files changed, 100 insertions(+) create mode 100644 drivers/acpi/x86/x86_utils.c diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index d78065c..f3940ac 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -50,6 +50,7 @@ acpi-$(CONFIG_ACPI_REDUCED_HARDWARE_ONLY) += evged.o acpi-y += sysfs.o acpi-y += property.o acpi-$(CONFIG_X86) += acpi_cmos_rtc.o +acpi-$(CONFIG_X86) += x86/x86_utils.o acpi-$(CONFIG_DEBUG_FS)+= debugfs.o acpi-$(CONFIG_ACPI_NUMA) += numa.o acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 34fbe02..784bda6 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -114,6 +114,11 @@ int acpi_bus_get_status(struct acpi_device *device) acpi_status status; unsigned long long sta; + if (acpi_device_always_present(device)) { + acpi_set_device_status(device, ACPI_STA_DEFAULT); + return 0; + } + status = acpi_bus_get_status_handle(device->handle, ); if (ACPI_FAILURE(status)) return -ENODEV; diff --git a/drivers/acpi/x86/x86_utils.c b/drivers/acpi/x86/x86_utils.c new file mode 100644 index 000..74f1237 --- /dev/null +++ b/drivers/acpi/x86/x86_utils.c @@ -0,0 +1,85 @@ +/* + * X86 ACPI Utility Functions + * + * Copyright (C) 2017 Hans de Goede + * + * Based on various non upstream patches to support the CHT Whiskey Cove PMIC: + * Copyright (C) 2013-2015 Intel Corporation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include "../internal.h" + +/* + * Some ACPI devices are hidden (status == 0x0) in recent BIOS-es because + * some recent Windows drivers bind to one device but poke at multiple + * devices at the same time, so the others get hidden. + * We work around this by always reporting ACPI_STA_DEFAULT for
[Intel-gfx] [PATCH v8 2/2] ACPI / bus: Add INT0002 to list of always-present devices
The INT0002 device is necessary to clear wakeup interrupt sources on Cherry Trail devices, without it we get nobody cared IRQ msgs and some systems don't properly resume at all without it. Signed-off-by: Hans de Goede--- Changes in v6: -This is a new patch in v6 of this patch-set Changes in v7: -Adjust for the always present devices table being moved to drivers/acpi/x86/x86_utils.c --- drivers/acpi/x86/x86_utils.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/acpi/x86/x86_utils.c b/drivers/acpi/x86/x86_utils.c index 74f1237..98d875a 100644 --- a/drivers/acpi/x86/x86_utils.c +++ b/drivers/acpi/x86/x86_utils.c @@ -49,6 +49,11 @@ static const struct always_present_id always_present_ids[] = { */ ENTRY("80860F09", "1", ICPU(INTEL_FAM6_ATOM_SILVERMONT1)), ENTRY("80862288", "1", ICPU(INTEL_FAM6_ATOM_AIRMONT)), + /* +* The INT0002 device is necessary to clear wakeup interrupt sources +* on Cherry Trail devices, without it we get nobody cared IRQ msgs. +*/ + ENTRY("INT0002", "1", ICPU(INTEL_FAM6_ATOM_AIRMONT)), }; bool acpi_device_always_present(struct acpi_device *adev) -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 1/2] ACPI / bus: Introduce a list of ids for "always present" devices
On Friday, April 21, 2017 12:42:31 PM Hans de Goede wrote: > HI, > [cut] > >>> And in that path, which I seem to have overlooked before, the > >>> acpi_set_device_status() call is too early for invoking > >>> acpi_device_always_present(adev), so the latter should be called > >>> directly from acpi_add_single_object() like > >>> > >>> acpi_init_device_object(device, handle, type, sta); > >>> if (acpi_device_always_present(adev)) > >>> acpi_set_device_status(adev, ACPI_STA_DEFAULT); > >> > >> That is not necessary, the place(s) where we care about status being > >> fixed-up for always-present devices, so that they get scanned / their > >> platform device initiated, is in acpi_bus_attach() which > >> already calls acpi_bus_get_status() and thus gets the > >> acpi_device_always_present() check applied before checking. > >> > >> For hotplugged devices there are acpi_scan_bus_check and > >> acpi_scan_device_check but those both also already call > >> acpi_bus_get_status() before checking adev->status. > > > > OK > > > > Which probably means that we don't need to initialize adev->status > > in acpi_init_device_object() to anything meaningful, do we? > > Right, I don't think that is necessary. But maybe it is necessary > for some special cases (e.g. power resources) ? For power resources _STA is defined differently at all and we don't even call acpi_add_single_object() for them. :-) Are there any other special cases in which that may matter? Thanks, Rafael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 1/2] ACPI / bus: Introduce a list of ids for "always present" devices
On Friday, April 21, 2017 11:59:34 AM Hans de Goede wrote: > Hi, > > On 19-04-17 22:14, Rafael J. Wysocki wrote: > > On Wed, Apr 19, 2017 at 2:04 PM, Hans de Goedewrote: > >> Several Bay / Cherry Trail devices (all of which ship with Windows 10) hide > >> the LPSS PWM controller in ACPI, typically the _STA method looks like this: > >> > >> Method (_STA, 0, NotSerialized) // _STA: Status > >> { > >> If (OSID == One) > >> { > >> Return (Zero) > >> } > >> > >> Return (0x0F) > >> } > >> > >> Where OSID is some dark magic seen in all Cherry Trail ACPI tables making > >> the machine behave differently depending on which OS it *thinks* it is > >> booting, this gets set in a number of ways which we cannot control, on > >> some newer machines it simple hardcoded to "One" aka win10. > >> > >> This causes the PWM controller to get hidden, which means Linux cannot > >> control the backlight level on cht based tablets / laptops. > >> > >> Since loading the driver for this does no harm (the only in kernel user > >> of it is the i915 driver, which will only uses it when it needs it), this > >> commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT > >> for the LPSS PWM device, fixing the lack of backlight control. > >> > >> Signed-off-by: Hans de Goede > >> --- > >> Changes in v2: > >> -Use pr_debug instead of ACPI_DEBUG_PRINT > >> Changes in v3: > >> -Un-inline acpi_set_device_status and do the always_present_device_ids > >> table check inside the un-inlined version of it > >> Changes in v4: > >> -Use dev_info instead of pr_debug > >> -Not only check for ACPI HID but also for CPU (SoC) model so as to not > >> for devices present on other models then for which the quirk is intended > >> and > >> to avoid enabling unrelated ACPI devices which happen to use the same HID > >> Changes in v5: > >> -Only do the dev_info once per device (acpi_set_device_status gets called > >> multiple times per device during boot) > >> Changes in v6: > >> -Allow specifying more then one CPU-model for a single HID > >> -Not only match the HID but also the UID, like on Cherry Trail, on some Bay > >> Trail Windows 10 tablets we need to enable the PWM controller to get > >> working > >> backlight even though _STA returns 0. The Bay Trail SoC has 2 PWM > >> controllers > >> and we only need the first one. UID matching will allows adding an entry > >> for > >> Bay Trail which only enables the first PWM controller > >> Changes in v7: > >> -Put the actual x86 specific matching code and table with always present > >> device HID + UID + CPU model entries in a new x86/x86_utils.c file which > >> provides an acpi_device_always_present() helper function on x86, on > >> non x86 a stub which always returns false is provided > >> -Squash in the addition of the Bay Trail PWM entry in the table as it > >> is there for the same reasons as the Cherry Trail entry is there > > > > >> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > >> index 34fbe02..4d952cc 100644 > >> --- a/drivers/acpi/bus.c > >> +++ b/drivers/acpi/bus.c > >> @@ -132,6 +132,16 @@ int acpi_bus_get_status(struct acpi_device *device) > >> } > >> EXPORT_SYMBOL(acpi_bus_get_status); > >> > >> +void acpi_set_device_status(struct acpi_device *adev, u32 sta) > >> +{ > >> + u32 *status = (u32 *)>status; > >> + > >> + if (acpi_device_always_present(adev)) > >> + *status = ACPI_STA_DEFAULT; > >> + else > >> + *status = sta; > > > > *((u32 *)>status) = acpi_device_always_present(adev) ? > > ACPI_STA_DEFAULT : sta; > > > > and I guess it could still be static inline? > > > > But that said, evaluating _STA for the always present devices is > > pointless (as Lukas noticed), so why not to modify > > acpi_bus_get_status() to do something like: > > > > if (acpi_device_always_present(adev)) { > > acpi_set_device_status(adev, ACPI_STA_DEFAULT); > > return 0; > > } > > > > upfront > > Hehe, my v1 of this patch actually did the check in acpi_bus_get_status() > I moved it to acpi_set_device_status() on your request. Moving it back > is fine with me and indeed seems cleaner. > > > and modify the other path invoking acpi_set_device_status() accordingly. > > > > And in that path, which I seem to have overlooked before, the > > acpi_set_device_status() call is too early for invoking > > acpi_device_always_present(adev), so the latter should be called > > directly from acpi_add_single_object() like > > > > acpi_init_device_object(device, handle, type, sta); > > if (acpi_device_always_present(adev)) > > acpi_set_device_status(adev, ACPI_STA_DEFAULT); > > That is not necessary, the place(s) where we care about status being > fixed-up for always-present devices, so that they get scanned / their > platform device initiated, is in acpi_bus_attach() which >
Re: [Intel-gfx] [PATCH v7 1/2] ACPI / bus: Introduce a list of ids for "always present" devices
HI, On 21-04-17 12:33, Rafael J. Wysocki wrote: On Friday, April 21, 2017 11:59:34 AM Hans de Goede wrote: Hi, On 19-04-17 22:14, Rafael J. Wysocki wrote: On Wed, Apr 19, 2017 at 2:04 PM, Hans de Goedewrote: Several Bay / Cherry Trail devices (all of which ship with Windows 10) hide the LPSS PWM controller in ACPI, typically the _STA method looks like this: Method (_STA, 0, NotSerialized) // _STA: Status { If (OSID == One) { Return (Zero) } Return (0x0F) } Where OSID is some dark magic seen in all Cherry Trail ACPI tables making the machine behave differently depending on which OS it *thinks* it is booting, this gets set in a number of ways which we cannot control, on some newer machines it simple hardcoded to "One" aka win10. This causes the PWM controller to get hidden, which means Linux cannot control the backlight level on cht based tablets / laptops. Since loading the driver for this does no harm (the only in kernel user of it is the i915 driver, which will only uses it when it needs it), this commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT for the LPSS PWM device, fixing the lack of backlight control. Signed-off-by: Hans de Goede --- Changes in v2: -Use pr_debug instead of ACPI_DEBUG_PRINT Changes in v3: -Un-inline acpi_set_device_status and do the always_present_device_ids table check inside the un-inlined version of it Changes in v4: -Use dev_info instead of pr_debug -Not only check for ACPI HID but also for CPU (SoC) model so as to not for devices present on other models then for which the quirk is intended and to avoid enabling unrelated ACPI devices which happen to use the same HID Changes in v5: -Only do the dev_info once per device (acpi_set_device_status gets called multiple times per device during boot) Changes in v6: -Allow specifying more then one CPU-model for a single HID -Not only match the HID but also the UID, like on Cherry Trail, on some Bay Trail Windows 10 tablets we need to enable the PWM controller to get working backlight even though _STA returns 0. The Bay Trail SoC has 2 PWM controllers and we only need the first one. UID matching will allows adding an entry for Bay Trail which only enables the first PWM controller Changes in v7: -Put the actual x86 specific matching code and table with always present device HID + UID + CPU model entries in a new x86/x86_utils.c file which provides an acpi_device_always_present() helper function on x86, on non x86 a stub which always returns false is provided -Squash in the addition of the Bay Trail PWM entry in the table as it is there for the same reasons as the Cherry Trail entry is there diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 34fbe02..4d952cc 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -132,6 +132,16 @@ int acpi_bus_get_status(struct acpi_device *device) } EXPORT_SYMBOL(acpi_bus_get_status); +void acpi_set_device_status(struct acpi_device *adev, u32 sta) +{ + u32 *status = (u32 *)>status; + + if (acpi_device_always_present(adev)) + *status = ACPI_STA_DEFAULT; + else + *status = sta; *((u32 *)>status) = acpi_device_always_present(adev) ? ACPI_STA_DEFAULT : sta; and I guess it could still be static inline? But that said, evaluating _STA for the always present devices is pointless (as Lukas noticed), so why not to modify acpi_bus_get_status() to do something like: if (acpi_device_always_present(adev)) { acpi_set_device_status(adev, ACPI_STA_DEFAULT); return 0; } upfront Hehe, my v1 of this patch actually did the check in acpi_bus_get_status() I moved it to acpi_set_device_status() on your request. Moving it back is fine with me and indeed seems cleaner. and modify the other path invoking acpi_set_device_status() accordingly. And in that path, which I seem to have overlooked before, the acpi_set_device_status() call is too early for invoking acpi_device_always_present(adev), so the latter should be called directly from acpi_add_single_object() like acpi_init_device_object(device, handle, type, sta); if (acpi_device_always_present(adev)) acpi_set_device_status(adev, ACPI_STA_DEFAULT); That is not necessary, the place(s) where we care about status being fixed-up for always-present devices, so that they get scanned / their platform device initiated, is in acpi_bus_attach() which already calls acpi_bus_get_status() and thus gets the acpi_device_always_present() check applied before checking. For hotplugged devices there are acpi_scan_bus_check and acpi_scan_device_check but those both also already call acpi_bus_get_status() before checking adev->status. OK Which probably means that we don't need to initialize adev->status in acpi_init_device_object() to anything meaningful, do we? Right, I
Re: [Intel-gfx] [PATCH v7 1/2] ACPI / bus: Introduce a list of ids for "always present" devices
Hi, On 19-04-17 22:14, Rafael J. Wysocki wrote: On Wed, Apr 19, 2017 at 2:04 PM, Hans de Goedewrote: Several Bay / Cherry Trail devices (all of which ship with Windows 10) hide the LPSS PWM controller in ACPI, typically the _STA method looks like this: Method (_STA, 0, NotSerialized) // _STA: Status { If (OSID == One) { Return (Zero) } Return (0x0F) } Where OSID is some dark magic seen in all Cherry Trail ACPI tables making the machine behave differently depending on which OS it *thinks* it is booting, this gets set in a number of ways which we cannot control, on some newer machines it simple hardcoded to "One" aka win10. This causes the PWM controller to get hidden, which means Linux cannot control the backlight level on cht based tablets / laptops. Since loading the driver for this does no harm (the only in kernel user of it is the i915 driver, which will only uses it when it needs it), this commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT for the LPSS PWM device, fixing the lack of backlight control. Signed-off-by: Hans de Goede --- Changes in v2: -Use pr_debug instead of ACPI_DEBUG_PRINT Changes in v3: -Un-inline acpi_set_device_status and do the always_present_device_ids table check inside the un-inlined version of it Changes in v4: -Use dev_info instead of pr_debug -Not only check for ACPI HID but also for CPU (SoC) model so as to not for devices present on other models then for which the quirk is intended and to avoid enabling unrelated ACPI devices which happen to use the same HID Changes in v5: -Only do the dev_info once per device (acpi_set_device_status gets called multiple times per device during boot) Changes in v6: -Allow specifying more then one CPU-model for a single HID -Not only match the HID but also the UID, like on Cherry Trail, on some Bay Trail Windows 10 tablets we need to enable the PWM controller to get working backlight even though _STA returns 0. The Bay Trail SoC has 2 PWM controllers and we only need the first one. UID matching will allows adding an entry for Bay Trail which only enables the first PWM controller Changes in v7: -Put the actual x86 specific matching code and table with always present device HID + UID + CPU model entries in a new x86/x86_utils.c file which provides an acpi_device_always_present() helper function on x86, on non x86 a stub which always returns false is provided -Squash in the addition of the Bay Trail PWM entry in the table as it is there for the same reasons as the Cherry Trail entry is there diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 34fbe02..4d952cc 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -132,6 +132,16 @@ int acpi_bus_get_status(struct acpi_device *device) } EXPORT_SYMBOL(acpi_bus_get_status); +void acpi_set_device_status(struct acpi_device *adev, u32 sta) +{ + u32 *status = (u32 *)>status; + + if (acpi_device_always_present(adev)) + *status = ACPI_STA_DEFAULT; + else + *status = sta; *((u32 *)>status) = acpi_device_always_present(adev) ? ACPI_STA_DEFAULT : sta; and I guess it could still be static inline? But that said, evaluating _STA for the always present devices is pointless (as Lukas noticed), so why not to modify acpi_bus_get_status() to do something like: if (acpi_device_always_present(adev)) { acpi_set_device_status(adev, ACPI_STA_DEFAULT); return 0; } upfront Hehe, my v1 of this patch actually did the check in acpi_bus_get_status() I moved it to acpi_set_device_status() on your request. Moving it back is fine with me and indeed seems cleaner. and modify the other path invoking acpi_set_device_status() accordingly. And in that path, which I seem to have overlooked before, the acpi_set_device_status() call is too early for invoking acpi_device_always_present(adev), so the latter should be called directly from acpi_add_single_object() like acpi_init_device_object(device, handle, type, sta); if (acpi_device_always_present(adev)) acpi_set_device_status(adev, ACPI_STA_DEFAULT); That is not necessary, the place(s) where we care about status being fixed-up for always-present devices, so that they get scanned / their platform device initiated, is in acpi_bus_attach() which already calls acpi_bus_get_status() and thus gets the acpi_device_always_present() check applied before checking. For hotplugged devices there are acpi_scan_bus_check and acpi_scan_device_check but those both also already call acpi_bus_get_status() before checking adev->status. So just adding the check to acpi_bus_get_status(), as I did in v1 is sufficient. Note that the end result is still significantly cleaner then v1 as we're now using the acpi_device_always_present() which makes the drivers/acpi/bus.c changes a lot cleaner. I'll go and test this and
Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno()
On Fri, Apr 21, 2017 at 08:50:26AM -, Patchwork wrote: > == Series Details == > > Series: drm/i915: Stop touching hangcheck.seqno from > intel_engine_init_global_seqno() > URL : https://patchwork.freedesktop.org/series/23320/ > State : success > > == Summary == > > Series 23320v1 drm/i915: Stop touching hangcheck.seqno from > intel_engine_init_global_seqno() > https://patchwork.freedesktop.org/api/1.0/series/23320/revisions/1/mbox/ > > Test gem_exec_flush: > Subgroup basic-batch-kernel-default-uc: > pass -> FAIL (fi-snb-2600) fdo#17 > Test kms_pipe_crc_basic: > Subgroup suspend-read-crc-pipe-a: > fail -> PASS (fi-skl-6700k) fdo#100367 > Test pm_rpm: > Subgroup basic-rte: > incomplete -> PASS (fi-bsw-n3050) fdo#100718 > > fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 > fdo#100367 https://bugs.freedesktop.org/show_bug.cgi?id=100367 > fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718 Pushed, thanks for the review. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno()
== Series Details == Series: drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno() URL : https://patchwork.freedesktop.org/series/23320/ State : success == Summary == Series 23320v1 drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno() https://patchwork.freedesktop.org/api/1.0/series/23320/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#17 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: fail -> PASS (fi-skl-6700k) fdo#100367 Test pm_rpm: Subgroup basic-rte: incomplete -> PASS (fi-bsw-n3050) fdo#100718 fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fdo#100367 https://bugs.freedesktop.org/show_bug.cgi?id=100367 fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:431s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:430s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:576s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:508s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:561s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:492s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:479s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:406s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:409s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:483s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:461s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:459s fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:571s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:461s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:569s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:467s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:491s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:430s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:529s fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time:402s 9bb844000d9412c2758b861ce7aee1e9546fccb9 drm-tip: 2017y-04m-20d-14h-46m-59s UTC integration manifest a7239e1 drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno() == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4528/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno()
On Fri, Apr 21, 2017 at 11:12:21AM +0300, Mika Kuoppala wrote: > Chris Wilsonwrites: > > > The hangcheck runs independently to the main flow of seqno through the > > driver. However, we have an odd coupling of the seqno reset that is > > unwelcome, and if poked at just the right rate can cause spurious hangs > > (e.g. gem_exec_whisper) on an apparently idle engine. > > > > Signed-off-by: Chris Wilson > > Cc: Mika Kuoppala > > Reviewed-by: Mika Kuoppala Ta, I'll send this by itself to CI to confirm that it is safe :) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI] drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno()
The hangcheck runs independently to the main flow of seqno through the driver. However, we have an odd coupling of the seqno reset that is unwelcome, and if poked at just the right rate can cause spurious hangs (e.g. gem_exec_whisper) on an apparently idle engine. Signed-off-by: Chris WilsonCc: Mika Kuoppala Reviewed-by: Mika Kuoppala --- drivers/gpu/drm/i915/intel_engine_cs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 402769d9d840..82a274b336c5 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -265,6 +265,7 @@ void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno) struct drm_i915_private *dev_priv = engine->i915; GEM_BUG_ON(!intel_engine_is_idle(engine)); + GEM_BUG_ON(i915_gem_active_isset(>timeline->last_request)); /* Our semaphore implementation is strictly monotonic (i.e. we proceed * so long as the semaphore value in the register/page is greater @@ -296,9 +297,6 @@ void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno) intel_write_status_page(engine, I915_GEM_HWS_INDEX, seqno); clear_bit(ENGINE_IRQ_BREADCRUMB, >irq_posted); - GEM_BUG_ON(i915_gem_active_isset(>timeline->last_request)); - engine->hangcheck.seqno = seqno; - /* After manually advancing the seqno, fake the interrupt in case * there are any waiters for that seqno. */ -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno()
Chris Wilsonwrites: > The hangcheck runs independently to the main flow of seqno through the > driver. However, we have an odd coupling of the seqno reset that is > unwelcome, and if poked at just the right rate can cause spurious hangs > (e.g. gem_exec_whisper) on an apparently idle engine. > > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala Reviewed-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > b/drivers/gpu/drm/i915/intel_engine_cs.c > index 7681d17ce454..f3cb7e931317 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -265,6 +265,7 @@ void intel_engine_init_global_seqno(struct > intel_engine_cs *engine, u32 seqno) > struct drm_i915_private *dev_priv = engine->i915; > > GEM_BUG_ON(!intel_engine_is_idle(engine)); > + GEM_BUG_ON(i915_gem_active_isset(>timeline->last_request)); > > /* Our semaphore implementation is strictly monotonic (i.e. we proceed >* so long as the semaphore value in the register/page is greater > @@ -284,9 +285,6 @@ void intel_engine_init_global_seqno(struct > intel_engine_cs *engine, u32 seqno) > intel_write_status_page(engine, I915_GEM_HWS_INDEX, seqno); > clear_bit(ENGINE_IRQ_BREADCRUMB, >irq_posted); > > - GEM_BUG_ON(i915_gem_active_isset(>timeline->last_request)); > - engine->hangcheck.seqno = seqno; > - > /* After manually advancing the seqno, fake the interrupt in case >* there are any waiters for that seqno. >*/ > -- > 2.11.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 Adding driver-private objects to atomic state
== Series Details == Series: Adding driver-private objects to atomic state URL : https://patchwork.freedesktop.org/series/23308/ State : success == Summary == Series 23308v1 Adding driver-private objects to atomic state https://patchwork.freedesktop.org/api/1.0/series/23308/revisions/1/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#17 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: fail -> PASS (fi-skl-6700k) fdo#100367 Test pm_rpm: Subgroup basic-rte: incomplete -> PASS (fi-bsw-n3050) fdo#100718 fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17 fdo#100367 https://bugs.freedesktop.org/show_bug.cgi?id=100367 fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:437s fi-bdw-gvtdvmtotal:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:427s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:588s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:511s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:542s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:481s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:487s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:414s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:406s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:423s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:492s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:486s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:454s fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:568s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:448s fi-skl-6700hqtotal:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:568s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:456s fi-skl-6770hqtotal:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:499s fi-skl-gvtdvmtotal:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:428s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:541s fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time:405s 9bb844000d9412c2758b861ce7aee1e9546fccb9 drm-tip: 2017y-04m-20d-14h-46m-59s UTC integration manifest 3347ee1 drm/dp: Track MST link bandwidth 8fef895b4 drm/dp: Add DP MST helpers to atomically find and release vcpi slots 403e426 drm/dp: Introduce MST topology state to track available link bandwidth 4eb3832 drm: Add driver-private objects to atomic state == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4527/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx